[REFACTOR] 사용자, 무드, 장소 관련 도메인 Annotation 재정비#250
Merged
Conversation
📝 WalkthroughWalkthrough스프링 웹 매핑( Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/org/sopt/snappinserver/api/v1/curation/controller/CurationController.java (1)
25-27:⚠️ Potential issue | 🔴 Critical
@Validated누락 — 파라미터 검증이 런타임에 동작하지 않습니다
@NotNull,@Min,@Max와 같은 메서드 파라미터 레벨 Bean Validation은 Spring의 AOP 프록시를 통해 동작합니다. 이 프록시를 활성화하려면 컨트롤러 클래스에@Validated가 선언되어야 하는데, 현재 누락되어 있습니다. 결과적으로step파라미터에 부착된 제약 조건들은 런타임에 전혀 적용되지 않습니다.코딩 가이드라인에도
@Validated는 컨트롤러 구현체에 위치해야 한다고 명시되어 있습니다.🛠️ 제안 수정
`@RequestMapping`("/api/v1/curation") `@RequiredArgsConstructor` +@Validated `@RestController` public class CurationController implements CurationApi {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/sopt/snappinserver/api/v1/curation/controller/CurationController.java` around lines 25 - 27, The CurationController class is missing the `@Validated` annotation so method-level Bean Validation on parameters like the step parameter (e.g., constraints such as `@NotNull/`@Min/@Max) will not be enforced at runtime; add the Spring `@Validated` annotation to the CurationController class declaration (alongside `@RestController/`@RequestMapping/@RequiredArgsConstructor) to enable AOP-based parameter validation and ensure existing parameter constraint annotations on methods in CurationController are activated.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/main/java/org/sopt/snappinserver/api/v1/curation/controller/CurationApi.java`:
- Around line 34-35: The step parameter in the CurationApi interface must carry
the validation annotations (`@NotNull`, `@Min`, `@Max`) instead of relying on the
implementation; update the step `@RequestParam` declaration in CurationApi (method
parameter named step) to include the appropriate annotations and remove any
duplicated validation annotations from CurationController so only the interface
defines the validation contract.
In
`@src/main/java/org/sopt/snappinserver/api/v1/curation/controller/CurationController.java`:
- Around line 38-41: Remove the validation annotations (`@NotNull`, `@Min`, `@Max`)
from the controller implementation (CurationController) and place them on the
corresponding method signature/parameter in the API contract interface
(CurationApi) so the validation metadata lives on the interface for the
parameter "Integer step"; ensure the controller method parameters no longer
carry those annotations and the CurationApi method has the same parameter
name/type annotated with `@NotNull`(message="단계는 필수입니다."), `@Min`(value=1,
message="단계는 1 이상이어야 합니다."), and `@Max`(value=5, message="단계는 5 이하여야 합니다.") to
comply with the coding guideline.
In
`@src/main/java/org/sopt/snappinserver/api/v1/photographer/controller/PhotographerApi.java`:
- Around line 21-25: Add the missing parameter validation annotations to the
interface method getPhotographerProfile in PhotographerApi: restore `@NotNull` and
`@Positive` on the photographerId `@PathVariable` so the validation lives on the
interface (not in PhotographerController). Update the method signature for
ApiResponseBody<GetPhotographerProfileResponse, Void>
getPhotographerProfile(...) to include `@NotNull` and `@Positive` on the Long
photographerId parameter and keep the existing `@PathVariable` and `@Schema`
annotations.
In
`@src/main/java/org/sopt/snappinserver/api/v1/photographer/controller/PhotographerController.java`:
- Around line 3-4: Move the parameter validation annotations (`@NotNull`,
`@Positive`, etc.) out of the PhotographerController implementation methods and
place them on the corresponding method parameters in the PhotographerApi
interface (add the same annotations to the interface method signatures), then
remove those annotations and their imports from PhotographerController so
validation is declared on the interface (PhotographerApi) not the
implementation; ensure you do not duplicate mapping/validation annotations on
both interface and controller methods.
In
`@src/main/java/org/sopt/snappinserver/api/v1/place/controller/PlaceController.java`:
- Around line 22-26: Move the parameter validation annotations from
PlaceController.getPlaces to the PlaceApi.getPlaces declaration: add `@NotBlank`
and `@Length` (and the `@RequestParam/`@Schema metadata) on the interface method
parameter so validation metadata lives on the API contract; then remove those
validation annotations from PlaceController.getPlaces and annotate the
PlaceController class with `@Validated` so Spring's method-level validation
(MethodValidationInterceptor) runs at runtime. Ensure you reference the same
parameter name and keep the method signature getPlaces in both PlaceApi and
PlaceController consistent.
---
Outside diff comments:
In
`@src/main/java/org/sopt/snappinserver/api/v1/curation/controller/CurationController.java`:
- Around line 25-27: The CurationController class is missing the `@Validated`
annotation so method-level Bean Validation on parameters like the step parameter
(e.g., constraints such as `@NotNull/`@Min/@Max) will not be enforced at runtime;
add the Spring `@Validated` annotation to the CurationController class declaration
(alongside `@RestController/`@RequestMapping/@RequiredArgsConstructor) to enable
AOP-based parameter validation and ensure existing parameter constraint
annotations on methods in CurationController are activated.
src/main/java/org/sopt/snappinserver/api/v1/curation/controller/CurationApi.java
Show resolved
Hide resolved
src/main/java/org/sopt/snappinserver/api/v1/curation/controller/CurationController.java
Outdated
Show resolved
Hide resolved
src/main/java/org/sopt/snappinserver/api/v1/photographer/controller/PhotographerApi.java
Show resolved
Hide resolved
src/main/java/org/sopt/snappinserver/api/v1/photographer/controller/PhotographerController.java
Outdated
Show resolved
Hide resolved
src/main/java/org/sopt/snappinserver/api/v1/place/controller/PlaceController.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/java/org/sopt/snappinserver/api/v1/place/controller/PlaceApi.java`:
- Line 22: 현재 파라미터에 붙어있는 `@Schema`(description = "장소 검색어")를 제거하고 Springdoc 권장 및
프로젝트 가이드라인에 맞게 `@Parameter로` 대체하세요; 구체적으로 PlaceApi 인터페이스의 해당 요청 파라미터에 적용된 `@Schema를`
`@Parameter`(...)로 바꾸고 (프로젝트 규약에 따라 인터페이스 레이어라면 `@Parameter`(hidden = true) 형태를
사용하거나 필요한 경우 description 속성을 제공) 관련 어노테이션 충돌(예: `@NotBlank`, `@Length` 등)이 발생하지 않도록
검토해서 문서가 덮어써지지 않게 수정하세요.
- Line 20: In PlaceApi update the method parameter annotation for the search
term: replace the use of `@Schema`(...) on the parameter named keyword with
`@Parameter`(description = "장소 검색어") while keeping `@NotBlank`(...) and
`@RequestParam`, and add the import for io.swagger.v3.oas.annotations.Parameter
(and remove unused `@Schema` import if applicable) so the Swagger UI shows
validation metadata and the description correctly.
src/main/java/org/sopt/snappinserver/api/v1/place/controller/PlaceApi.java
Show resolved
Hide resolved
src/main/java/org/sopt/snappinserver/api/v1/place/controller/PlaceApi.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/org/sopt/snappinserver/api/v1/curation/controller/CurationApi.java (1)
23-23:⚠️ Potential issue | 🟡 Minor
@Validated는 인터페이스가 아닌 컨트롤러 구현체에 위치해야 합니다.코딩 가이드라인에 따르면
@Validated는 "AOP 기반 검증 활성화" 목적으로 컨트롤러 구현체에 선언해야 합니다. 현재CurationApi인터페이스에만 있고CurationController구현체에는 없으므로, 다음과 같이 정리해 주시기 바랍니다.✏️ 수정 제안
CurationApi.java(인터페이스에서 제거):`@Tag`(name = "03 - Curation", description = "무드 큐레이션 관련 API") -@Validated public interface CurationApi {
CurationController.java(구현체에 추가):+@Validated `@RequestMapping`("/api/v1/curation") `@RequiredArgsConstructor` `@RestController` public class CurationController implements CurationApi {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/sopt/snappinserver/api/v1/curation/controller/CurationApi.java` at line 23, Remove the `@Validated` annotation from the CurationApi interface and add it to the controller implementation class CurationController (the class that implements CurationApi) so AOP-based validation is activated on the concrete controller; update imports if necessary and ensure only CurationController is annotated with `@Validated` while CurationApi no longer carries that annotation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/main/java/org/sopt/snappinserver/api/v1/curation/controller/CurationApi.java`:
- Line 35: The Swagger `@Parameter` annotation for the step parameter in
CurationApi currently lacks required=true even though `@NotNull` enforces it;
update the `@Parameter` on the method parameter (the one annotated with
`@Parameter`(description = "조회할 단계", example = "1") and `@NotNull`) to include
required = true so the OpenAPI/Swagger docs correctly reflect that the parameter
is mandatory.
In `@src/main/java/org/sopt/snappinserver/api/v1/place/controller/PlaceApi.java`:
- Line 22: The `@Parameter` annotation on the method parameter 'keyword' in
PlaceApi.java doesn't declare required=true so Swagger may show it as optional
even though `@NotBlank` enforces it; update the `@Parameter` on the 'keyword'
RequestParam in the PlaceApi controller to include required = true (and
optionally an example/description) so the OpenAPI docs match the `@NotBlank`
validation.
---
Outside diff comments:
In
`@src/main/java/org/sopt/snappinserver/api/v1/curation/controller/CurationApi.java`:
- Line 23: Remove the `@Validated` annotation from the CurationApi interface and
add it to the controller implementation class CurationController (the class that
implements CurationApi) so AOP-based validation is activated on the concrete
controller; update imports if necessary and ensure only CurationController is
annotated with `@Validated` while CurationApi no longer carries that annotation.
| CustomUserInfo userInfo, | ||
|
|
||
| @Schema(description = "조회할 단계", example = "1") | ||
| @Parameter(description = "조회할 단계", example = "1") |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
@Parameter에 required = true 추가를 고려해 주세요.
@NotNull로 필수 파라미터임을 검증하고 있지만, Swagger 문서에서는 @Parameter의 required 속성이 기본값 false로 표시됩니다. API 명세의 정확성을 위해 추가해 주시면 좋을 것 같습니다.
✏️ 수정 제안
- `@Parameter`(description = "조회할 단계", example = "1")
+ `@Parameter`(description = "조회할 단계", example = "1", required = true)📝 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
| @Parameter(description = "조회할 단계", example = "1") | |
| `@Parameter`(description = "조회할 단계", example = "1", required = true) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/org/sopt/snappinserver/api/v1/curation/controller/CurationApi.java`
at line 35, The Swagger `@Parameter` annotation for the step parameter in
CurationApi currently lacks required=true even though `@NotNull` enforces it;
update the `@Parameter` on the method parameter (the one annotated with
`@Parameter`(description = "조회할 단계", example = "1") and `@NotNull`) to include
required = true so the OpenAPI/Swagger docs correctly reflect that the parameter
is mandatory.
| ) | ||
| @GetMapping | ||
| ApiResponseBody<GetPlaceListResponse, Void> getPlaces( | ||
| @Parameter(description = "장소 검색어") |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
@Parameter에 required = true 속성 추가를 고려해 주세요.
keyword 파라미터는 @NotBlank로 필수임을 나타내고 있지만, @Parameter에 required 속성이 명시되지 않아 Swagger UI에서 해당 파라미터가 선택 사항으로 보일 수 있습니다. 문서와 실제 검증 동작이 일치하도록 아래와 같이 보완하면 좋을 것 같습니다.
✨ 제안 수정
- `@Parameter`(description = "장소 검색어")
+ `@Parameter`(description = "장소 검색어", required = true)Based on learnings: "메서드 파라미터(RequestParam, PathVariable, RequestHeader 등)에는 @Parameter 어노테이션을 사용하고, description, required, example 속성을 적절히 활용해야 합니다."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/org/sopt/snappinserver/api/v1/place/controller/PlaceApi.java`
at line 22, The `@Parameter` annotation on the method parameter 'keyword' in
PlaceApi.java doesn't declare required=true so Swagger may show it as optional
even though `@NotBlank` enforces it; update the `@Parameter` on the 'keyword'
RequestParam in the PlaceApi controller to include required = true (and
optionally an example/description) so the OpenAPI docs match the `@NotBlank`
validation.
earl9rey
approved these changes
Feb 23, 2026
Contributor
earl9rey
left a comment
earl9rey
left a comment
There was a problem hiding this comment.
확인했습니다!! 수고 많으셨어요 ~~
소연 ♡ 개행
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
2 participants
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
🖇️ Tasks
🔍 To Reviewer
❶ 가독성에 관한 고민
컨벤션을 다시 정하면서 API 인터페이스 쪽에 많은 어노테이션이 붙습니다. 하나의 파라미터에 대해 여러 어노테이션이 붙는 경우가 많고, 여러 파라미터가 있어 인터페이스 측에서는 한 줄 당 하나의 어노테이션만 붙였습니다. 또한, 파라미터 간 개행으로 파라미터를 조금 더 명확하게 작성하기 위해 노력했습니다.
반면, 컨트롤러 구현체에서는 인증 어노테이션을 제외하면 거의 모든 어노테이션이 붙지 않기 때문에 파라미터 간에 개행을 하지 않았고, 각 파라미터에 들어가는 어노테이션은 파라미터와 함께 한 줄에 작성했습니다.