Open
Conversation
안내워크스루배너 관리 기능을 위한 완전한 계층 구조를 도입합니다. JPA 엔티티(Banner), 저장소(BannerRepository), 비즈니스 로직(BannerService), REST 컨트롤러(BannerController), 데이터 전송 객체(DTOs), 그리고 테스트 케이스를 포함합니다. 활성 배너 최대 4개 제한, 디스플레이 순서 자동 조정, 시간 기반 필터링을 구현합니다. 변경 사항
시퀀스 다이어그램sequenceDiagram
participant Client
participant BannerController
participant BannerService
participant BannerRepository
participant Database
rect rgba(100, 200, 150, 0.5)
Note over Client,Database: 배너 생성
Client->>BannerController: POST /admin/banners<br/>(BannerCreateRequest)
BannerController->>BannerService: create(request)
BannerService->>BannerRepository: countByActiveTrue()
BannerRepository->>Database: SELECT COUNT(*)
Database-->>BannerRepository: count
BannerRepository-->>BannerService: activeCount
alt activeCount >= 4
BannerService-->>Client: 예외 발생
else activeCount < 4
BannerService->>BannerRepository: findByActiveTrueAndDisplayOrderGreaterThanEqual()
BannerRepository->>Database: SELECT * WHERE active=true
Database-->>BannerRepository: banners
BannerRepository-->>BannerService: clashingBanners
BannerService->>BannerService: increment displayOrder
BannerService->>BannerRepository: saveAll(clashingBanners)
BannerRepository->>Database: UPDATE banners
BannerService->>BannerRepository: save(newBanner)
BannerRepository->>Database: INSERT banner
Database-->>BannerRepository: saved banner
BannerRepository-->>BannerService: banner
BannerService-->>Client: 200 OK
end
end
rect rgba(150, 200, 200, 0.5)
Note over Client,Database: 활성 배너 조회
Client->>BannerController: GET /banners
BannerController->>BannerService: getActiveBanners()
BannerService->>BannerRepository: findActiveBanners(now)
BannerRepository->>Database: SELECT * WHERE startAt<=now AND endAt>now<br/>AND active=true ORDER BY displayOrder
Database-->>BannerRepository: activeBanners
BannerRepository-->>BannerService: banners (limit 4)
BannerService->>BannerService: map to BannerResponse
BannerService-->>Client: 200 OK [BannerResponse]
end
예상 코드 리뷰 노력🎯 3 (중간) | ⏱️ ~25분 제안된 레이블
제안된 검토자
시
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Test Results 39 files 39 suites 12s ⏱️ Results for commit e4fa1f1. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Fix all issues with AI agents
In `@src/main/java/ku_rum/backend/domain/banner/application/BannerService.java`:
- Around line 36-41: When shifting existing banners' displayOrder in
BannerService (using
bannerRepository.findByActiveTrueAndDisplayOrderGreaterThanEqual and the stream
that sorts by Banner::getDisplayOrder then calls changeDisplayOrder), you must
prevent any banner from being pushed past MAX_BANNER_COUNT; before performing
the shift, check the targets collection for any Banner whose getDisplayOrder()
>= MAX_BANNER_COUNT and if found reject the insert (throw a domain/validation
exception) or return an error; alternatively, filter the shift to only update
banners with getDisplayOrder() < MAX_BANNER_COUNT and still fail the operation
if any banner would be truncated — update the logic around activeCount and the
shifting loop to enforce this constraint and reference MAX_BANNER_COUNT,
bannerRepository.findByActiveTrueAndDisplayOrderGreaterThanEqual,
Banner::getDisplayOrder and changeDisplayOrder when applying the fix.
- Around line 25-53: The create method (BannerService.create) is vulnerable to a
race where bannerRepository.countByActiveTrue() can be stale; change the flow to
obtain a DB-level lock or constraint before deciding to insert: either (A)
perform a locking select of active rows (e.g., add a repository method like
findByActiveTrueForUpdate / custom query using SELECT ... FOR UPDATE or JPA
PESSIMISTIC_WRITE) and base the count/shift logic on that locked result so
concurrent creators block, or (B) enforce the limit at the DB (unique/partial
index or check constraint that prevents more than MAX_BANNER_COUNT active=true)
and implement a retry/duplicate-handling path that catches the DB constraint
exception and retries or fails gracefully; apply the lock/query and then run the
existing findByActiveTrueAndDisplayOrderGreaterThanEqual,
Banner.changeDisplayOrder, and bannerRepository.save within the same
`@Transactional` boundary to ensure correctness.
- Around line 55-59: The update(Long id, BannerUpdateRequest req) currently
assigns displayOrder via banner.update(req) without validation or collision
handling; modify BannerService.update to (1) validate req.getDisplayOrder() is
within 1..4, (2) if the incoming displayOrder differs from the banner's current
value and banner.isActive() (and only consider active banners), run the same
shifting logic used in create(): call
bannerRepository.findByActiveTrueAndDisplayOrderGreaterThanEqual(newOrder) and
increment displayOrder for those existing banners (persisting changes via
repository/saveAll or equivalent) to avoid duplicates, then apply
banner.update(req); keep references to the existing methods banner.update(req),
bannerRepository.findByActiveTrueAndDisplayOrderGreaterThanEqual(...) and
bannerRepository.saveAll(...) so the behavior mirrors create().
In `@src/main/java/ku_rum/backend/domain/banner/domain/Banner.java`:
- Around line 43-48: The onCreate `@PrePersist` in Banner currently forces active
= true and overwrites any builder-supplied value; update the Banner.onCreate
method so it does not unconditionally overwrite active—either remove the active
assignment entirely or change the field to a nullable Boolean and set active =
true only when active == null (i.e., if (active == null) active = true); modify
Banner.onCreate accordingly and keep createdAt/updatedAt logic intact.
In
`@src/main/java/ku_rum/backend/domain/banner/domain/repository/BannerRepository.java`:
- Around line 12-20: The JPQL uses the named parameter :now but the repository
method findActiveBanners(LocalDateTime now) lacks the `@Param` binding; update
BannerRepository by annotating the method parameter with `@Param`("now") so Spring
Data can reliably bind the LocalDateTime now argument to the :now placeholder
regardless of compiler parameter name retention.
In `@src/main/java/ku_rum/backend/domain/banner/dto/BannerCreateRequest.java`:
- Around line 14-21: Add Bean Validation annotations to the BannerCreateRequest
DTO: annotate String fields (title, imageUrl, linkUrl) with `@NotBlank`, the
integer displayOrder with `@Min`(0) or `@NotNull` as appropriate, and the
LocalDateTime fields (startAt, endAt) with `@NotNull` and any temporal constraints
if needed; also annotate the class with Lombok/constructor/getters if required
for validation to work. Then update the controller method that accepts
BannerCreateRequest to annotate the parameter with `@Valid` (and `@RequestBody` if
missing) so Spring triggers validation and returns 400 on invalid input;
reference BannerCreateRequest and the controller method handling banner creation
when making the changes.
In `@src/main/java/ku_rum/backend/domain/banner/dto/BannerUpdateRequest.java`:
- Around line 8-18: BannerUpdateRequest currently only has `@AllArgsConstructor`
which prevents Jackson from deserializing JSON into the DTO for `@RequestBody`;
add a no-args constructor by annotating the class with `@NoArgsConstructor`
(alongside the existing `@AllArgsConstructor` and `@Getter`) so Jackson can
instantiate BannerUpdateRequest during JSON deserialization; update the class
declaration to include `@NoArgsConstructor` and ensure import for
lombok.NoArgsConstructor is present.
In
`@src/main/java/ku_rum/backend/domain/banner/presentation/BannerController.java`:
- Around line 18-21: Add javax.validation validation to the controller method
parameters: in BannerController annotate the `@RequestBody` parameters with `@Valid`
for both the create method (public void create(`@RequestBody` BannerCreateRequest
request)) and the update endpoint (the update method that accepts
BannerUpdateRequest) so Bean Validation on BannerCreateRequest and
BannerUpdateRequest is executed before calling bannerService.
In
`@src/main/java/ku_rum/backend/domain/user/dto/request/ProfileChangeRequest.java`:
- Around line 3-5: ProfileChangeRequest의 imageUrl이 `@NotNull` 제거로 null 허용되어
User.changeImage()가 null을 그대로 저장할 수 있으니, 한 가지 방법을 선택해 수정하세요: (1) imageUrl에
`@NotNull을` 복원하여 null 입력을 방지하거나, (2) 프로필 삭제를 허용하려면 User.changeImage(String
imageUrl) 내부에 명시적 null 처리 로직(예: null이면 기존 이미지 삭제 또는 기본값 사용)과 관련 동작을 주석/문서로
명시하십시오; 변경 대상 식별자는 ProfileChangeRequest.imageUrl 필드와 User.changeImage 메서드입니다.
🧹 Nitpick comments (5)
src/main/java/ku_rum/backend/domain/banner/domain/Banner.java (1)
4-4: 도메인 엔티티가 DTO에 직접 의존하고 있습니다.
Banner엔티티가BannerUpdateRequestDTO를 직접 import하여update()메서드의 파라미터로 사용하고 있습니다. 이는 도메인 계층이 프레젠테이션/애플리케이션 계층의 DTO에 결합되는 레이어 위반입니다. DTO 구조가 변경되면 도메인 엔티티도 함께 수정해야 합니다.
update()메서드가 개별 필드 값을 파라미터로 받거나, 서비스 계층에서 값을 풀어서 전달하는 방식을 권장합니다.제안하는 수정 사항
-import ku_rum.backend.domain.banner.dto.BannerUpdateRequest; - public void update(BannerUpdateRequest req) { - this.title = req.getTitle(); - this.imageUrl = req.getImageUrl(); - this.linkUrl = req.getLinkUrl(); - this.displayOrder = req.getDisplayOrder(); - this.startAt = req.getStartAt(); - this.endAt = req.getEndAt(); - this.active = req.isActive(); + public void update(String title, String imageUrl, String linkUrl, + int displayOrder, LocalDateTime startAt, LocalDateTime endAt, + boolean active) { + this.title = title; + this.imageUrl = imageUrl; + this.linkUrl = linkUrl; + this.displayOrder = displayOrder; + this.startAt = startAt; + this.endAt = endAt; + this.active = active; }Also applies to: 55-63
src/test/java/ku_rum/backend/domain/banner/presentation/BannerControllerTest.java (2)
43-84: 서비스 호출 검증이 누락되어 있습니다.
createBanner,updateBanner,deleteBanner테스트에서bannerService의 메서드가 실제로 호출되었는지verify()로 검증하고 있지 않습니다. 현재 테스트는 HTTP 상태 코드만 확인하므로, 컨트롤러가 서비스를 호출하지 않더라도 테스트가 통과합니다.// 예시: createBanner 테스트에 추가 verify(bannerService).create(any(BannerCreateRequest.class));
23-24: Spring Boot 3.4부터@MockBean이 deprecated 될 예정입니다.현재 프로젝트는 Spring Boot 3.3.4를 사용하고 있어 문제가 없지만, 향후 Spring Boot 3.4 이상으로 마이그레이션할 때는
org.springframework.test.context.bean.override.mockito.@MockitoBean으로 변경하시기 바랍니다.src/main/java/ku_rum/backend/domain/banner/presentation/BannerController.java (1)
18-21: POST 엔드포인트가 201 Created 대신 200 OK를 반환합니다.RESTful 관례에 따르면 리소스 생성 시
201 Created를 반환하는 것이 적절합니다.@ResponseStatus(HttpStatus.CREATED)추가 또는ResponseEntity를 사용하는 것을 고려해 주세요.제안하는 수정 사항
+import org.springframework.http.HttpStatus; +import org.springframework.web.bind.annotation.ResponseStatus; + `@PostMapping`("/admin/banners") + `@ResponseStatus`(HttpStatus.CREATED) public void create(`@Valid` `@RequestBody` BannerCreateRequest request) {src/main/java/ku_rum/backend/domain/banner/application/BannerService.java (1)
61-65: 삭제 후displayOrder갭(gap) 발생
deactivate()호출로 소프트 삭제 처리 시, 남은 활성 배너들의displayOrder에 빈 번호가 생깁니다 (예: 1, 3, 4). 프론트엔드에서 순서를displayOrder기준으로 렌더링한다면 문제가 없을 수 있지만, 연속된 1~N 순서를 기대하는 경우 재정렬 로직이 필요합니다.
Comment on lines
+25
to
+53
| public void create(BannerCreateRequest req) { | ||
| long activeCount = bannerRepository.countByActiveTrue(); | ||
| if (activeCount >= MAX_BANNER_COUNT) { | ||
| throw new IllegalStateException("활성 배너는 최대 4개까지 가능합니다."); | ||
| } | ||
|
|
||
| int order = req.getDisplayOrder(); | ||
| if (order < 1 || order > MAX_BANNER_COUNT) { | ||
| throw new IllegalArgumentException("displayOrder는 1~4만 가능합니다."); | ||
| } | ||
|
|
||
| List<Banner> targets = | ||
| bannerRepository.findByActiveTrueAndDisplayOrderGreaterThanEqual(order); | ||
|
|
||
| targets.stream() | ||
| .sorted(Comparator.comparingInt(Banner::getDisplayOrder).reversed()) | ||
| .forEach(b -> b.changeDisplayOrder(b.getDisplayOrder() + 1)); | ||
|
|
||
| Banner banner = Banner.builder() | ||
| .title(req.getTitle()) | ||
| .imageUrl(req.getImageUrl()) | ||
| .linkUrl(req.getLinkUrl()) | ||
| .displayOrder(order) | ||
| .startAt(req.getStartAt()) | ||
| .endAt(req.getEndAt()) | ||
| .build(); | ||
|
|
||
| bannerRepository.save(banner); | ||
| } |
There was a problem hiding this comment.
동시 요청 시 MAX_BANNER_COUNT 초과 가능 (Race Condition)
countByActiveTrue()로 활성 배너 수를 확인한 후 save()하는 사이에, 다른 트랜잭션이 동시에 배너를 생성하면 최대 개수를 초과할 수 있습니다. @Transactional만으로는 기본 격리 수준(READ_COMMITTED)에서 이를 방지할 수 없습니다.
해결 방안:
- DB 레벨에서
active = true인 배너 수에 대한 제약(예: 비관적 잠금,SELECT ... FOR UPDATE)을 적용 - 또는 unique constraint + retry 패턴 활용
🤖 Prompt for AI Agents
In `@src/main/java/ku_rum/backend/domain/banner/application/BannerService.java`
around lines 25 - 53, The create method (BannerService.create) is vulnerable to
a race where bannerRepository.countByActiveTrue() can be stale; change the flow
to obtain a DB-level lock or constraint before deciding to insert: either (A)
perform a locking select of active rows (e.g., add a repository method like
findByActiveTrueForUpdate / custom query using SELECT ... FOR UPDATE or JPA
PESSIMISTIC_WRITE) and base the count/shift logic on that locked result so
concurrent creators block, or (B) enforce the limit at the DB (unique/partial
index or check constraint that prevents more than MAX_BANNER_COUNT active=true)
and implement a retry/duplicate-handling path that catches the DB constraint
exception and retries or fails gracefully; apply the lock/query and then run the
existing findByActiveTrueAndDisplayOrderGreaterThanEqual,
Banner.changeDisplayOrder, and bannerRepository.save within the same
`@Transactional` boundary to ensure correctness.
Comment on lines
+36
to
+41
| List<Banner> targets = | ||
| bannerRepository.findByActiveTrueAndDisplayOrderGreaterThanEqual(order); | ||
|
|
||
| targets.stream() | ||
| .sorted(Comparator.comparingInt(Banner::getDisplayOrder).reversed()) | ||
| .forEach(b -> b.changeDisplayOrder(b.getDisplayOrder() + 1)); |
There was a problem hiding this comment.
displayOrder 시프트 시 상한 초과 버그
기존 배너의 displayOrder를 +1로 밀어낼 때, order 4인 배너가 5로 밀려나면서 1~4 제약 조건을 위반하게 됩니다. 예를 들어 order 1, 2, 3, 4에 활성 배너가 있는 상태에서 activeCount >= MAX_BANNER_COUNT 체크는 통과하지 못하지만, 활성 배너가 3개(order 1, 3, 4)인 상태에서 order 2에 삽입하면 order 3→4, order 4→5로 밀려납니다.
시프트 전에 최대 order가 MAX_BANNER_COUNT를 초과하지 않는지 검증하거나, 시프트 대상 중 order 4인 배너가 있으면 삽입을 거부하는 로직이 필요합니다.
🤖 Prompt for AI Agents
In `@src/main/java/ku_rum/backend/domain/banner/application/BannerService.java`
around lines 36 - 41, When shifting existing banners' displayOrder in
BannerService (using
bannerRepository.findByActiveTrueAndDisplayOrderGreaterThanEqual and the stream
that sorts by Banner::getDisplayOrder then calls changeDisplayOrder), you must
prevent any banner from being pushed past MAX_BANNER_COUNT; before performing
the shift, check the targets collection for any Banner whose getDisplayOrder()
>= MAX_BANNER_COUNT and if found reject the insert (throw a domain/validation
exception) or return an error; alternatively, filter the shift to only update
banners with getDisplayOrder() < MAX_BANNER_COUNT and still fail the operation
if any banner would be truncated — update the logic around activeCount and the
shifting loop to enforce this constraint and reference MAX_BANNER_COUNT,
bannerRepository.findByActiveTrueAndDisplayOrderGreaterThanEqual,
Banner::getDisplayOrder and changeDisplayOrder when applying the fix.
Comment on lines
+55
to
+59
| public void update(Long id, BannerUpdateRequest req) { | ||
| Banner banner = bannerRepository.findById(id) | ||
| .orElseThrow(() -> new IllegalArgumentException("배너 없음")); | ||
| banner.update(req); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd "BannerUpdateRequest.java" --type fRepository: KU-rum/backend
Length of output: 129
🏁 Script executed:
fd "Banner.java" --type f --not --path '*Test*'Repository: KU-rum/backend
Length of output: 305
🏁 Script executed:
cd src && find . -name "*Banner*" -type f | head -20Repository: KU-rum/backend
Length of output: 631
🏁 Script executed:
cat src/main/java/ku_rum/backend/domain/banner/dto/BannerUpdateRequest.javaRepository: KU-rum/backend
Length of output: 464
🏁 Script executed:
cat src/main/java/ku_rum/backend/domain/banner/domain/Banner.javaRepository: KU-rum/backend
Length of output: 1857
🏁 Script executed:
cat src/main/java/ku_rum/backend/domain/banner/application/BannerService.javaRepository: KU-rum/backend
Length of output: 2655
🏁 Script executed:
rg "findByActiveTrueAndDisplayOrderGreaterThanEqual" src/Repository: KU-rum/backend
Length of output: 387
update()에서 displayOrder 변경 시 중복 방지 로직 추가 필요
BannerUpdateRequest에 displayOrder 필드가 포함되어 있고, banner.update(req) 호출 시 이 값이 직접 할당됩니다. 그러나 create() 메서드에서 수행하는 다음 로직이 update()에는 완전히 없습니다:
displayOrder범위 검증 (1~4)- 기존 활성 배너와의 충돌 처리 (shifting)
- 활성 배너만 고려
현재 update()는 다른 활성 배너와 동일한 displayOrder가 설정되도록 허용하며, 이는 데이터 무결성을 위반합니다. create()의 findByActiveTrueAndDisplayOrderGreaterThanEqual() 및 shifting 로직을 update()에도 적용해야 합니다.
🤖 Prompt for AI Agents
In `@src/main/java/ku_rum/backend/domain/banner/application/BannerService.java`
around lines 55 - 59, The update(Long id, BannerUpdateRequest req) currently
assigns displayOrder via banner.update(req) without validation or collision
handling; modify BannerService.update to (1) validate req.getDisplayOrder() is
within 1..4, (2) if the incoming displayOrder differs from the banner's current
value and banner.isActive() (and only consider active banners), run the same
shifting logic used in create(): call
bannerRepository.findByActiveTrueAndDisplayOrderGreaterThanEqual(newOrder) and
increment displayOrder for those existing banners (persisting changes via
repository/saveAll or equivalent) to avoid duplicates, then apply
banner.update(req); keep references to the existing methods banner.update(req),
bannerRepository.findByActiveTrueAndDisplayOrderGreaterThanEqual(...) and
bannerRepository.saveAll(...) so the behavior mirrors create().
Comment on lines
+43
to
+48
| @PrePersist | ||
| void onCreate() { | ||
| this.createdAt = LocalDateTime.now(); | ||
| this.updatedAt = createdAt; | ||
| this.active = true; | ||
| } |
There was a problem hiding this comment.
@PrePersist에서 active = true로 강제 설정하면 빌더로 설정한 값이 무시됩니다.
Banner.builder().active(false).build()로 생성해도 @PrePersist에 의해 항상 active = true로 덮어씌워집니다. 의도적이라면 빌더에서 active 필드 설정을 제거하는 것이 혼동을 줄일 수 있습니다.
🤖 Prompt for AI Agents
In `@src/main/java/ku_rum/backend/domain/banner/domain/Banner.java` around lines
43 - 48, The onCreate `@PrePersist` in Banner currently forces active = true and
overwrites any builder-supplied value; update the Banner.onCreate method so it
does not unconditionally overwrite active—either remove the active assignment
entirely or change the field to a nullable Boolean and set active = true only
when active == null (i.e., if (active == null) active = true); modify
Banner.onCreate accordingly and keep createdAt/updatedAt logic intact.
Comment on lines
+12
to
+20
| @Query(""" | ||
| SELECT b | ||
| FROM Banner b | ||
| WHERE b.active = true | ||
| AND (b.startAt IS NULL OR b.startAt <= :now) | ||
| AND (b.endAt IS NULL OR b.endAt >= :now) | ||
| ORDER BY b.displayOrder ASC | ||
| """) | ||
| List<Banner> findActiveBanners(LocalDateTime now); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
@Param("now") 어노테이션 추가를 권장합니다.
JPQL 쿼리에서 :now로 바인딩하고 있지만 메서드 파라미터에 @Param("now") 어노테이션이 없습니다. 컴파일 옵션(-parameters)에 따라 파라미터 이름 추론이 실패하여 런타임 오류가 발생할 수 있습니다.
제안하는 수정 사항
+import org.springframework.data.repository.query.Param;
+
List<Banner> findActiveBanners(`@Param`("now") LocalDateTime now);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| @Query(""" | |
| SELECT b | |
| FROM Banner b | |
| WHERE b.active = true | |
| AND (b.startAt IS NULL OR b.startAt <= :now) | |
| AND (b.endAt IS NULL OR b.endAt >= :now) | |
| ORDER BY b.displayOrder ASC | |
| """) | |
| List<Banner> findActiveBanners(LocalDateTime now); | |
| `@Query`(""" | |
| SELECT b | |
| FROM Banner b | |
| WHERE b.active = true | |
| AND (b.startAt IS NULL OR b.startAt <= :now) | |
| AND (b.endAt IS NULL OR b.endAt >= :now) | |
| ORDER BY b.displayOrder ASC | |
| """) | |
| List<Banner> findActiveBanners(`@Param`("now") LocalDateTime now); |
🤖 Prompt for AI Agents
In
`@src/main/java/ku_rum/backend/domain/banner/domain/repository/BannerRepository.java`
around lines 12 - 20, The JPQL uses the named parameter :now but the repository
method findActiveBanners(LocalDateTime now) lacks the `@Param` binding; update
BannerRepository by annotating the method parameter with `@Param`("now") so Spring
Data can reliably bind the LocalDateTime now argument to the :now placeholder
regardless of compiler parameter name retention.
Comment on lines
+14
to
+21
| public class BannerCreateRequest { | ||
| private String title; | ||
| private String imageUrl; | ||
| private String linkUrl; | ||
| private int displayOrder; | ||
| private LocalDateTime startAt; | ||
| private LocalDateTime endAt; | ||
| } |
There was a problem hiding this comment.
요청 DTO에 Bean Validation 어노테이션이 누락되어 있습니다.
title, imageUrl 등 필수 필드에 @NotBlank, @NotNull 등의 검증 어노테이션이 없고, 컨트롤러에서도 @Valid가 사용되지 않고 있습니다. 빈 문자열이나 null 값이 그대로 엔티티에 저장될 수 있습니다.
제안하는 수정 사항
+import jakarta.validation.constraints.NotBlank;
+import jakarta.validation.constraints.NotNull;
+import jakarta.validation.constraints.Positive;
+
`@Getter`
`@Builder`
`@NoArgsConstructor`
`@AllArgsConstructor`
public class BannerCreateRequest {
+ `@NotBlank`
private String title;
+ `@NotBlank`
private String imageUrl;
private String linkUrl;
+ `@Positive`
private int displayOrder;
private LocalDateTime startAt;
private LocalDateTime endAt;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| public class BannerCreateRequest { | |
| private String title; | |
| private String imageUrl; | |
| private String linkUrl; | |
| private int displayOrder; | |
| private LocalDateTime startAt; | |
| private LocalDateTime endAt; | |
| } | |
| import jakarta.validation.constraints.NotBlank; | |
| import jakarta.validation.constraints.NotNull; | |
| import jakarta.validation.constraints.Positive; | |
| `@Getter` | |
| `@Builder` | |
| `@NoArgsConstructor` | |
| `@AllArgsConstructor` | |
| public class BannerCreateRequest { | |
| `@NotBlank` | |
| private String title; | |
| `@NotBlank` | |
| private String imageUrl; | |
| private String linkUrl; | |
| `@Positive` | |
| private int displayOrder; | |
| private LocalDateTime startAt; | |
| private LocalDateTime endAt; | |
| } |
🤖 Prompt for AI Agents
In `@src/main/java/ku_rum/backend/domain/banner/dto/BannerCreateRequest.java`
around lines 14 - 21, Add Bean Validation annotations to the BannerCreateRequest
DTO: annotate String fields (title, imageUrl, linkUrl) with `@NotBlank`, the
integer displayOrder with `@Min`(0) or `@NotNull` as appropriate, and the
LocalDateTime fields (startAt, endAt) with `@NotNull` and any temporal constraints
if needed; also annotate the class with Lombok/constructor/getters if required
for validation to work. Then update the controller method that accepts
BannerCreateRequest to annotate the parameter with `@Valid` (and `@RequestBody` if
missing) so Spring triggers validation and returns 400 on invalid input;
reference BannerCreateRequest and the controller method handling banner creation
when making the changes.
Comment on lines
+8
to
+18
| @Getter | ||
| @AllArgsConstructor | ||
| public class BannerUpdateRequest { | ||
| private String title; | ||
| private String imageUrl; | ||
| private String linkUrl; | ||
| private int displayOrder; | ||
| private boolean active; | ||
| private LocalDateTime startAt; | ||
| private LocalDateTime endAt; | ||
| } |
There was a problem hiding this comment.
@NoArgsConstructor 누락으로 JSON 역직렬화가 실패합니다.
BannerUpdateRequest에는 @AllArgsConstructor만 있고 @NoArgsConstructor가 없습니다. Jackson은 기본적으로 기본 생성자를 통해 객체를 생성하므로, @RequestBody로 이 DTO를 받을 때 역직렬화 오류(HttpMessageNotReadableException)가 발생합니다. BannerCreateRequest에는 @NoArgsConstructor가 있는 것과 비교하면 누락된 것으로 보입니다.
제안하는 수정 사항
import lombok.AllArgsConstructor;
import lombok.Getter;
+import lombok.NoArgsConstructor;
import java.time.LocalDateTime;
`@Getter`
`@AllArgsConstructor`
+@NoArgsConstructor
public class BannerUpdateRequest {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| @Getter | |
| @AllArgsConstructor | |
| public class BannerUpdateRequest { | |
| private String title; | |
| private String imageUrl; | |
| private String linkUrl; | |
| private int displayOrder; | |
| private boolean active; | |
| private LocalDateTime startAt; | |
| private LocalDateTime endAt; | |
| } | |
| `@Getter` | |
| `@AllArgsConstructor` | |
| `@NoArgsConstructor` | |
| public class BannerUpdateRequest { | |
| private String title; | |
| private String imageUrl; | |
| private String linkUrl; | |
| private int displayOrder; | |
| private boolean active; | |
| private LocalDateTime startAt; | |
| private LocalDateTime endAt; | |
| } |
🤖 Prompt for AI Agents
In `@src/main/java/ku_rum/backend/domain/banner/dto/BannerUpdateRequest.java`
around lines 8 - 18, BannerUpdateRequest currently only has `@AllArgsConstructor`
which prevents Jackson from deserializing JSON into the DTO for `@RequestBody`;
add a no-args constructor by annotating the class with `@NoArgsConstructor`
(alongside the existing `@AllArgsConstructor` and `@Getter`) so Jackson can
instantiate BannerUpdateRequest during JSON deserialization; update the class
declaration to include `@NoArgsConstructor` and ensure import for
lombok.NoArgsConstructor is present.
Comment on lines
+18
to
+21
| @PostMapping("/admin/banners") | ||
| public void create(@RequestBody BannerCreateRequest request) { | ||
| bannerService.create(request); | ||
| } |
There was a problem hiding this comment.
@RequestBody에 @Valid 어노테이션이 누락되어 있습니다.
DTO에 Bean Validation 어노테이션을 추가하더라도 컨트롤러에서 @Valid가 없으면 검증이 수행되지 않습니다. create와 update 엔드포인트 모두에 추가가 필요합니다.
제안하는 수정 사항
+import jakarta.validation.Valid;
+
`@PostMapping`("/admin/banners")
- public void create(`@RequestBody` BannerCreateRequest request) {
+ public void create(`@Valid` `@RequestBody` BannerCreateRequest request) {
bannerService.create(request);
}
`@PutMapping`("/admin/banners/{id}")
public void update(`@PathVariable` Long id,
- `@RequestBody` BannerUpdateRequest request) {
+ `@Valid` `@RequestBody` BannerUpdateRequest request) {
bannerService.update(id, request);
}🤖 Prompt for AI Agents
In
`@src/main/java/ku_rum/backend/domain/banner/presentation/BannerController.java`
around lines 18 - 21, Add javax.validation validation to the controller method
parameters: in BannerController annotate the `@RequestBody` parameters with `@Valid`
for both the create method (public void create(`@RequestBody` BannerCreateRequest
request)) and the update endpoint (the update method that accepts
BannerUpdateRequest) so Bean Validation on BannerCreateRequest and
BannerUpdateRequest is executed before calling bannerService.
Comment on lines
3
to
5
| public record ProfileChangeRequest | ||
| (@NotNull String imageUrl) { | ||
| (String imageUrl) { | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for ProfileChangeRequest usage in the codebase
rg -n -C3 'ProfileChangeRequest' --type=java -g '!**/dto/**' | head -100Repository: KU-rum/backend
Length of output: 5712
🏁 Script executed:
#!/bin/bash
# Search for service methods that handle profile changes to see null handling
rg -n 'imageUrl' --type=java -g '!**/dto/**' | grep -i profile | head -50Repository: KU-rum/backend
Length of output: 341
🏁 Script executed:
#!/bin/bash
# Look at the ProfileChangeRequest file and its surrounding context
find . -name "ProfileChangeRequest.java" -type f | head -1 | xargs cat -nRepository: KU-rum/backend
Length of output: 206
🏁 Script executed:
#!/bin/bash
# Check if there are other validation annotations or changes in the file
rg -n '@NotNull|@NotBlank|@NotEmpty' --type=java src/main/java/ku_rum/backend/domain/user/dto/request/ | head -20Repository: KU-rum/backend
Length of output: 3455
🏁 Script executed:
#!/bin/bash
# Find and display the changeImage method in the User entity
rg -n 'changeImage' --type=java -A10 | head -50Repository: KU-rum/backend
Length of output: 2292
🏁 Script executed:
#!/bin/bash
# Check the User domain class for changeImage implementation
find . -name "User.java" -path "*/domain/*" -type f | xargs cat -n 2>/dev/null | grep -A15 'changeImage'Repository: KU-rum/backend
Length of output: 567
imageUrl 필드에서 @NotNull 검증 제거로 인한 null 값 허용
@NotNull 어노테이션이 제거되어 imageUrl에 null 값이 전달될 수 있습니다. 현재 User.changeImage() 메서드는 null 체크 없이 직접 값을 할당하므로, null이 데이터베이스에 저장될 수 있습니다.
프로필 이미지 삭제를 의도한 변경이라면 명시적인 null 처리 로직을 추가하고 문서화하세요. 그렇지 않다면 @NotNull 어노테이션을 복원하세요.
🤖 Prompt for AI Agents
In
`@src/main/java/ku_rum/backend/domain/user/dto/request/ProfileChangeRequest.java`
around lines 3 - 5, ProfileChangeRequest의 imageUrl이 `@NotNull` 제거로 null 허용되어
User.changeImage()가 null을 그대로 저장할 수 있으니, 한 가지 방법을 선택해 수정하세요: (1) imageUrl에
`@NotNull을` 복원하여 null 입력을 방지하거나, (2) 프로필 삭제를 허용하려면 User.changeImage(String
imageUrl) 내부에 명시적 null 처리 로직(예: null이면 기존 이미지 삭제 또는 기본값 사용)과 관련 동작을 주석/문서로
명시하십시오; 변경 대상 식별자는 ProfileChangeRequest.imageUrl 필드와 User.changeImage 메서드입니다.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features