Open
Conversation
Exception 사용시 상황에 맞는 메서드를 생성하는 과정을 삭제하고, 대신 에러코드를 Exception을 생성할때 사용해서 나타내는 방식 사용
common : 공통 응답 형식 exception : 예외처리 관련 security : 시큐리티 설정 관련 로 정리함
Security를 통해서 로그인을 한 사람의 Id를 사용할 수 있도록 util클래스 생성. 추후 Security를 통해서 무언가를 자여와야 할 때 이 클래스를 사용
폴더의 조회 관련 메서드에서 필요한 폴더내 활성 북마크들을 전부 불러오는 시능 추가
폴더의 비지니스 로직 마저 완성 자세한건 컨플루언스 문서로 기록
로그인 관련 에러코드 및 SecurityUtils 내용 추가 자세한 내용은 컨플루언스 문서로 작성
전체 요약이 변경사항은 패키지 구조를 재정리하여 시퀀스 다이어그램sequenceDiagram
actor Client
participant Controller as MemberFolderController
participant SecurityUtils
participant Service as MemberFolderServiceImpl
participant Dao as MemberFolderJpaDao
participant BookmarkDao
Client->>Controller: 폴더 작업 요청<br/>(create/update/delete)
Controller->>SecurityUtils: extractMemberId(Authentication)
SecurityUtils-->>Controller: loginId (멤버ID)
Controller->>Service: 작업 수행<br/>(loginId 포함)
Service->>Dao: 소유권 검증<br/>(loginId로 폴더 조회)
Dao-->>Service: 폴더 정보
alt 작업이 delete인 경우
Service->>BookmarkDao: existsActiveBookmarkInFolder()<br/>(활성 북마크 확인)
BookmarkDao-->>Service: boolean
Service->>Dao: 폴더 삭제 가능 여부 확인<br/>(자식 폴더 존재 여부)
Dao-->>Service: boolean
else 작업이 move인 경우
Service->>Dao: 순환 참조 검증<br/>(새로운 부모 폴더)
Dao-->>Service: 유효성 결과
Service->>Dao: 중복 폴더명 검사<br/>(새 위치에서)
Dao-->>Service: 중복 여부
end
Service->>Dao: 작업 수행<br/>(DB 변경)
Dao-->>Service: 결과
Service-->>Controller: 작업 완료
Controller-->>Client: ApiResponse
코드 리뷰 난이도🎯 4 (Complex) | ⏱️ ~45분 시
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/main/java/com/web/SearchWeb/config/security/SecurityConfig.java (1)
110-118:⚠️ Potential issue | 🟠 Major중복 Bean 생성 문제
CustomAuthenticationFailureHandler와CustomAuthenticationSuccessHandler는 이미@Component로 선언되어 Spring이 자동으로 Bean을 생성합니다. 여기서new로 추가 인스턴스를 생성하면 중복 Bean이 만들어집니다. 생성자 주입으로 기존 Bean을 사용하거나, 핸들러 클래스에서@Component를 제거하세요.♻️ 제안하는 수정 사항 (생성자 주입 방식)
public class SecurityConfig { private final CustomOAuth2UserService customOAuth2UserService; + private final CustomAuthenticationSuccessHandler customAuthenticationSuccessHandler; + private final CustomAuthenticationFailureHandler customAuthenticationFailureHandler; - public SecurityConfig(CustomOAuth2UserService customOAuth2UserService) { + public SecurityConfig(CustomOAuth2UserService customOAuth2UserService, + CustomAuthenticationSuccessHandler customAuthenticationSuccessHandler, + CustomAuthenticationFailureHandler customAuthenticationFailureHandler) { this.customOAuth2UserService = customOAuth2UserService; + this.customAuthenticationSuccessHandler = customAuthenticationSuccessHandler; + this.customAuthenticationFailureHandler = customAuthenticationFailureHandler; }그리고
filterChain메서드에서:- .successHandler(customAuthenticationSuccessHandler()) - .failureHandler(customAuthenticationFailureHandler()) + .successHandler(customAuthenticationSuccessHandler) + .failureHandler(customAuthenticationFailureHandler)마지막으로
@Bean메서드들을 제거하세요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/web/SearchWeb/config/security/SecurityConfig.java` around lines 110 - 118, SecurityConfig currently defines beans customAuthenticationFailureHandler() and customAuthenticationSuccessHandler() which duplicate the existing `@Component-scanned` CustomAuthenticationFailureHandler and CustomAuthenticationSuccessHandler; remove these `@Bean` methods and instead inject the existing beans into SecurityConfig via constructor injection (add constructor parameters CustomAuthenticationFailureHandler and CustomAuthenticationSuccessHandler) and use those injected instances inside the filterChain method; alternatively, if you prefer explicit bean factory methods, remove the `@Component` annotation from the handler classes so only the `@Bean` factory methods produce instances.src/main/java/com/web/SearchWeb/config/security/CustomAuthenticationSuccessHandler.java (1)
17-21:⚠️ Potential issue | 🔴 Critical오픈 리다이렉트 취약점 위험
redirect파라미터가 검증 없이sendRedirect()에 직접 전달되고 있습니다. 공격자가redirect=https://malicious-site.com형태로 악용하여 피싱 공격에 사용할 수 있습니다. 내부 경로 또는 허용된 도메인인지 검증이 필요합니다.🔒 제안하는 수정 사항
`@Override` public void onAuthenticationSuccess(HttpServletRequest request, HttpServletResponse response, Authentication authentication) throws IOException { // 리다이렉트 URL 설정 String redirectUrl = request.getParameter("redirect"); - if (redirectUrl != null && !redirectUrl.isEmpty()) { + if (redirectUrl != null && !redirectUrl.isEmpty() && isValidRedirectUrl(redirectUrl)) { // 리디렉트 파라미터가 있을 경우 그 URL로 리다이렉트 response.sendRedirect(redirectUrl); } else { // 없을 경우 기본 페이지로 이동 response.sendRedirect("/mainList"); } } + +private boolean isValidRedirectUrl(String url) { + // 상대 경로만 허용하거나, 허용된 도메인 목록과 비교 + return url.startsWith("/") && !url.startsWith("//"); +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/web/SearchWeb/config/security/CustomAuthenticationSuccessHandler.java` around lines 17 - 21, The redirectUrl parameter in CustomAuthenticationSuccessHandler is used directly in response.sendRedirect, causing an open-redirect risk; update the onAuthenticationSuccess logic to validate redirectUrl before redirecting: parse the redirectUrl (variable redirectUrl) and only allow redirection if it is a safe relative path (starts with "/" but not "//" or contains protocol) or matches an explicit allowlist of trusted domains, otherwise fall back to the default target (e.g., defaultTargetUrl or savedRequestRedirect). Ensure this validation occurs immediately before the call to response.sendRedirect and reject/ignore unsafe values.src/main/java/com/web/SearchWeb/tag/error/TagErrorCode.java (1)
11-11: 🧹 Nitpick | 🔵 TrivialEnum 상수 네이밍 컨벤션 위반
Tag_NOT_FOUND는 Java enum 네이밍 컨벤션(SCREAMING_SNAKE_CASE)을 따르지 않습니다. 다른 ErrorCode enum들(DUPLICATE_BOOKMARK,FOLDER_NOT_FOUND등)과 일관성을 위해TAG_NOT_FOUND로 변경하는 것을 권장합니다.♻️ 제안하는 수정
- Tag_NOT_FOUND(HttpStatus.NOT_FOUND, "T001", "태그를 찾을 수 없습니다."); + TAG_NOT_FOUND(HttpStatus.NOT_FOUND, "T001", "태그를 찾을 수 없습니다.");참고:
TagException.java에서TagErrorCode.Tag_NOT_FOUND참조도 함께 수정이 필요합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/web/SearchWeb/tag/error/TagErrorCode.java` at line 11, Rename the enum constant Tag_NOT_FOUND to follow SCREAMING_SNAKE_CASE as TAG_NOT_FOUND in TagErrorCode (change the symbol Tag_NOT_FOUND -> TAG_NOT_FOUND) and update all references, e.g., in TagException (replace TagErrorCode.Tag_NOT_FOUND with TagErrorCode.TAG_NOT_FOUND) so naming is consistent with other constants like DUPLICATE_BOOKMARK and FOLDER_NOT_FOUND.
🤖 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/com/web/SearchWeb/config/security/SecurityUtils.java`:
- Around line 25-30: In SecurityUtils (the method that extracts memberId from
the authentication principal), ensure you fail-closed by validating the returned
memberId from CustomUserDetails.getMemberId() and
CustomOAuth2User.getMemberId(): if either is null throw an UNAUTHORIZED response
(e.g. throw ResponseStatusException(HttpStatus.UNAUTHORIZED) or your app's
equivalent) instead of returning null; also handle the fallback branch (when
principal is not one of those types) to throw UNAUTHORIZED so no null memberId
propagates.
In
`@src/main/java/com/web/SearchWeb/folder/controller/dto/MemberFolderRequests.java`:
- Around line 17-27: Change the DTO fields from public to private so external
code cannot mutate them after validation; in the nested request class Create
make parentFolderId, folderName, and description private and rely on the
existing `@Getter` (and default constructor) for Jackson/Bean Validation, and
apply the same change to the other nested request classes in this file (e.g.,
Update/other request classes that currently have public fields).
In `@src/main/java/com/web/SearchWeb/folder/error/FolderErrorCode.java`:
- Line 13: The enum constant FOLDER_FORBIDDEN in FolderErrorCode currently uses
the incorrect code string "Foo3"; update the enum entry for FOLDER_FORBIDDEN to
use the correct code "F003" so the exposed error code matches the response
contract and client documentation.
In `@src/main/java/com/web/SearchWeb/folder/error/FolderException.java`:
- Line 10: The FolderException constructor currently accepts the generic
ErrorCode which allows non-folder error codes to be passed; change the
constructor signature in class FolderException to accept FolderErrorCode instead
(replace ErrorCode parameter with FolderErrorCode) and update all call sites
that construct new FolderException(...) to pass a FolderErrorCode (adjust
imports/usages as needed) so the API boundary is type-safe; keep the rest of
FolderException behavior unchanged.
In `@src/main/java/com/web/SearchWeb/folder/service/MemberFolderService.java`:
- Around line 13-15: Remove the redundant ownerMemberId parameter from the
MemberFolderService API: change the signatures of listRootFolders and
listChildren to accept only Long loginId (and Long parentFolderId for
listChildren), and update all implementations to derive the owner from loginId
instead of using ownerMemberId; also remove or adapt calls to
validateOwner(loginId, ownerMemberId) so they validate ownership using loginId
alone (e.g., validateOwner(loginId) or inline owner resolution) and adjust any
controllers/routes to stop passing ownerMemberId.
In `@src/main/java/com/web/SearchWeb/folder/service/MemberFolderServiceImpl.java`:
- Around line 220-235: validateNoCycle currently walks parent links and issues a
DB call per level via memberFolderJpaRepository.findById, which can be slow for
deep hierarchies; replace or supplement this with a single recursive query (CTE)
in the repository to fetch the ancestor chain (or perform a direct cycle check)
and use that result to detect folderId in the ancestors, updating
validateNoCycle to call the new repository method instead of looping; reference
validateNoCycle, memberFolderJpaRepository.findById, and the
FolderException/FOLDER_NOT_FOUND and INVALID_FOLDER_MOVE error codes when adding
the repository method and adjusting the service logic.
- Around line 80-91: normalizeDescription currently trims and null-checks but
lacks the max-length validation present in normalizeFolderName; add a length
check to ensure the returned description does not exceed the MemberFolder
`@Column`(length = 500) constraint. Modify normalizeDescription to trim, return
null for empty, then if length > 500 either truncate to 500 or throw an
IllegalArgumentException (match the approach used in normalizeFolderName), and
prefer introducing/using a named constant (e.g., MAX_DESCRIPTION_LENGTH = 500)
to make the limit explicit.
- Around line 68-78: The service validation in normalizeFolderName enforces a
50-character limit that conflicts with MemberFolder.folderName which is
annotated `@Column`(length = 100); fix by making the constraints consistent:
either update normalizeFolderName to allow up to 100 characters (replace the 50
check) or change the MemberFolder.folderName column length to 50 and update any
DB migrations; ensure FolderException uses FolderErrorCode.INVALID_FOLDER_NAME
as before and, if the shorter 50 limit is intentional, add a clear comment or
Javadoc near MemberFolder.folderName and normalizeFolderName documenting the
business rule.
---
Outside diff comments:
In
`@src/main/java/com/web/SearchWeb/config/security/CustomAuthenticationSuccessHandler.java`:
- Around line 17-21: The redirectUrl parameter in
CustomAuthenticationSuccessHandler is used directly in response.sendRedirect,
causing an open-redirect risk; update the onAuthenticationSuccess logic to
validate redirectUrl before redirecting: parse the redirectUrl (variable
redirectUrl) and only allow redirection if it is a safe relative path (starts
with "/" but not "//" or contains protocol) or matches an explicit allowlist of
trusted domains, otherwise fall back to the default target (e.g.,
defaultTargetUrl or savedRequestRedirect). Ensure this validation occurs
immediately before the call to response.sendRedirect and reject/ignore unsafe
values.
In `@src/main/java/com/web/SearchWeb/config/security/SecurityConfig.java`:
- Around line 110-118: SecurityConfig currently defines beans
customAuthenticationFailureHandler() and customAuthenticationSuccessHandler()
which duplicate the existing `@Component-scanned`
CustomAuthenticationFailureHandler and CustomAuthenticationSuccessHandler;
remove these `@Bean` methods and instead inject the existing beans into
SecurityConfig via constructor injection (add constructor parameters
CustomAuthenticationFailureHandler and CustomAuthenticationSuccessHandler) and
use those injected instances inside the filterChain method; alternatively, if
you prefer explicit bean factory methods, remove the `@Component` annotation from
the handler classes so only the `@Bean` factory methods produce instances.
In `@src/main/java/com/web/SearchWeb/tag/error/TagErrorCode.java`:
- Line 11: Rename the enum constant Tag_NOT_FOUND to follow SCREAMING_SNAKE_CASE
as TAG_NOT_FOUND in TagErrorCode (change the symbol Tag_NOT_FOUND ->
TAG_NOT_FOUND) and update all references, e.g., in TagException (replace
TagErrorCode.Tag_NOT_FOUND with TagErrorCode.TAG_NOT_FOUND) so naming is
consistent with other constants like DUPLICATE_BOOKMARK and FOLDER_NOT_FOUND.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6f51df56-6d63-40ce-b49a-be78265abe37
📒 Files selected for processing (26)
src/main/java/com/web/SearchWeb/bookmark/controller/BookmarkApiController.javasrc/main/java/com/web/SearchWeb/bookmark/dao/BookmarkDao.javasrc/main/java/com/web/SearchWeb/bookmark/dao/MybatisBookmarkDao.javasrc/main/java/com/web/SearchWeb/bookmark/error/BookmarkErrorCode.javasrc/main/java/com/web/SearchWeb/bookmark/service/BookmarkServiceImpl.javasrc/main/java/com/web/SearchWeb/config/common/ApiResponse.javasrc/main/java/com/web/SearchWeb/config/exception/BusinessException.javasrc/main/java/com/web/SearchWeb/config/exception/CommonErrorCode.javasrc/main/java/com/web/SearchWeb/config/exception/ErrorCode.javasrc/main/java/com/web/SearchWeb/config/security/CustomAuthenticationFailureHandler.javasrc/main/java/com/web/SearchWeb/config/security/CustomAuthenticationSuccessHandler.javasrc/main/java/com/web/SearchWeb/config/security/GlobalExceptionHandler.javasrc/main/java/com/web/SearchWeb/config/security/SecurityConfig.javasrc/main/java/com/web/SearchWeb/config/security/SecurityUtils.javasrc/main/java/com/web/SearchWeb/folder/controller/MemberFolderController.javasrc/main/java/com/web/SearchWeb/folder/controller/dto/MemberFolderRequests.javasrc/main/java/com/web/SearchWeb/folder/dao/MemberFolderJpaDao.javasrc/main/java/com/web/SearchWeb/folder/error/FolderErrorCode.javasrc/main/java/com/web/SearchWeb/folder/error/FolderException.javasrc/main/java/com/web/SearchWeb/folder/service/MemberFolderService.javasrc/main/java/com/web/SearchWeb/folder/service/MemberFolderServiceImpl.javasrc/main/java/com/web/SearchWeb/mypage/controller/MyPageController.javasrc/main/java/com/web/SearchWeb/tag/controller/MemberTagController.javasrc/main/java/com/web/SearchWeb/tag/error/TagErrorCode.javasrc/main/java/com/web/SearchWeb/tag/error/TagException.javasrc/main/resources/mapper/bookmark-mapper.xml
Comment on lines
+25
to
+30
| if (principal instanceof CustomUserDetails userDetails) { | ||
| return userDetails.getMemberId(); | ||
| } | ||
|
|
||
| if (principal instanceof CustomOAuth2User oauth2User) { | ||
| return oauth2User.getMemberId(); |
There was a problem hiding this comment.
memberId가 null인 경우도 여기서 차단하세요.
CustomUserDetails#getMemberId()와 CustomOAuth2User#getMemberId()가 값을 그대로 반환하므로, 인증 객체가 만들어졌더라도 memberId가 null이면 이후 권한 체크가 500으로 깨질 수 있습니다. 이 메서드에서 fail-closed로 UNAUTHORIZED를 던지는 편이 안전합니다.
🔒 제안 수정
if (principal instanceof CustomUserDetails userDetails) {
- return userDetails.getMemberId();
+ Long memberId = userDetails.getMemberId();
+ if (memberId != null) {
+ return memberId;
+ }
+ throw BusinessException.from(CommonErrorCode.UNAUTHORIZED);
}
if (principal instanceof CustomOAuth2User oauth2User) {
- return oauth2User.getMemberId();
+ Long memberId = oauth2User.getMemberId();
+ if (memberId != null) {
+ return memberId;
+ }
+ throw BusinessException.from(CommonErrorCode.UNAUTHORIZED);
}📝 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
| if (principal instanceof CustomUserDetails userDetails) { | |
| return userDetails.getMemberId(); | |
| } | |
| if (principal instanceof CustomOAuth2User oauth2User) { | |
| return oauth2User.getMemberId(); | |
| if (principal instanceof CustomUserDetails userDetails) { | |
| Long memberId = userDetails.getMemberId(); | |
| if (memberId != null) { | |
| return memberId; | |
| } | |
| throw BusinessException.from(CommonErrorCode.UNAUTHORIZED); | |
| } | |
| if (principal instanceof CustomOAuth2User oauth2User) { | |
| Long memberId = oauth2User.getMemberId(); | |
| if (memberId != null) { | |
| return memberId; | |
| } | |
| throw BusinessException.from(CommonErrorCode.UNAUTHORIZED); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/com/web/SearchWeb/config/security/SecurityUtils.java` around
lines 25 - 30, In SecurityUtils (the method that extracts memberId from the
authentication principal), ensure you fail-closed by validating the returned
memberId from CustomUserDetails.getMemberId() and
CustomOAuth2User.getMemberId(): if either is null throw an UNAUTHORIZED response
(e.g. throw ResponseStatusException(HttpStatus.UNAUTHORIZED) or your app's
equivalent) instead of returning null; also handle the fallback branch (when
principal is not one of those types) to throw UNAUTHORIZED so no null memberId
propagates.
Comment on lines
17
to
27
| public static class Create { | ||
| public Long ownerMemberId; | ||
| public Long parentFolderId; // null이면 루트 | ||
| // null이면 루트 폴더 | ||
| @Positive(message = "parentFolderId는 양수여야 합니다.") | ||
| public Long parentFolderId; | ||
|
|
||
| @NotBlank(message = "folderName은 비어 있을 수 없습니다.") | ||
| @Size(max = 50, message = "folderName은 최대 50자까지 가능합니다.") | ||
| public String folderName; | ||
|
|
||
| @Size(max = 200, message = "description은 최대 200자까지 가능합니다.") | ||
| public String description; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
요청 DTO 필드는 private로 두세요.
@Getter를 붙였는데 필드가 여전히 public이라 검증/바인딩 이후에도 외부 코드가 값을 직접 바꿀 수 있습니다. Jackson + Bean Validation 조합에서는 기본 생성자와 private 필드만으로 충분합니다.
♻️ 제안 수정
public static class Create {
// null이면 루트 폴더
`@Positive`(message = "parentFolderId는 양수여야 합니다.")
- public Long parentFolderId;
+ private Long parentFolderId;
`@NotBlank`(message = "folderName은 비어 있을 수 없습니다.")
`@Size`(max = 50, message = "folderName은 최대 50자까지 가능합니다.")
- public String folderName;
+ private String folderName;
`@Size`(max = 200, message = "description은 최대 200자까지 가능합니다.")
- public String description;
+ private String description;
}
@@
public static class Update {
`@Size`(max = 50, message = "folderName은 최대 50자까지 가능합니다.")
- public String folderName;
+ private String folderName;
`@Size`(max = 200, message = "description은 최대 200자까지 가능합니다.")
- public String description;
+ private String description;
}
@@
public static class Move {
// null이면 루트로 이동
`@Positive`(message = "newParentFolderId는 양수여야 합니다.")
- public Long newParentFolderId;
+ private Long newParentFolderId;
}Also applies to: 35-40, 48-51
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/com/web/SearchWeb/folder/controller/dto/MemberFolderRequests.java`
around lines 17 - 27, Change the DTO fields from public to private so external
code cannot mutate them after validation; in the nested request class Create
make parentFolderId, folderName, and description private and rely on the
existing `@Getter` (and default constructor) for Jackson/Bean Validation, and
apply the same change to the other nested request classes in this file (e.g.,
Update/other request classes that currently have public fields).
| FOLDER_NOT_FOUND(HttpStatus.NOT_FOUND, "F001", "폴더를 찾을 수 없습니다."), | ||
| DUPLICATE_FOLDER_NAME(HttpStatus.BAD_REQUEST, "F002", "이미 존재하는 폴더명입니다."); | ||
| DUPLICATE_FOLDER_NAME(HttpStatus.BAD_REQUEST, "F002", "이미 존재하는 폴더명입니다."), | ||
| FOLDER_FORBIDDEN(HttpStatus.FORBIDDEN,"Foo3" ,"접근이 제한된 폴더입니다." ), |
There was a problem hiding this comment.
에러 코드 값 오타를 수정하세요.
FOLDER_FORBIDDEN의 코드가 Foo3로 들어가 있습니다. 이 값이 응답 계약에 노출되면 클라이언트 분기와 문서가 모두 어긋나므로 F003로 바로잡아야 합니다.
🐛 제안 수정
- FOLDER_FORBIDDEN(HttpStatus.FORBIDDEN,"Foo3" ,"접근이 제한된 폴더입니다." ),
+ FOLDER_FORBIDDEN(HttpStatus.FORBIDDEN, "F003", "접근이 제한된 폴더입니다."),📝 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
| FOLDER_FORBIDDEN(HttpStatus.FORBIDDEN,"Foo3" ,"접근이 제한된 폴더입니다." ), | |
| FOLDER_FORBIDDEN(HttpStatus.FORBIDDEN, "F003", "접근이 제한된 폴더입니다."), |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/com/web/SearchWeb/folder/error/FolderErrorCode.java` at line
13, The enum constant FOLDER_FORBIDDEN in FolderErrorCode currently uses the
incorrect code string "Foo3"; update the enum entry for FOLDER_FORBIDDEN to use
the correct code "F003" so the exposed error code matches the response contract
and client documentation.
| public class FolderException extends BusinessException { | ||
|
|
||
| private FolderException(ErrorCode errorCode) { | ||
| public FolderException(ErrorCode errorCode) { |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
FolderException 생성자는 FolderErrorCode로 좁히는 편이 안전합니다.
지금 시그니처면 폴더 도메인 예외에 다른 도메인의 ErrorCode를 실수로 태워도 컴파일이 통과합니다. 현재 사용 패턴도 전부 FolderErrorCode라서 타입을 좁혀 두는 쪽이 API 경계를 더 명확하게 만듭니다.
♻️ 제안 수정
-import com.web.SearchWeb.config.exception.ErrorCode;
import lombok.Getter;
`@Getter`
public class FolderException extends BusinessException {
- public FolderException(ErrorCode errorCode) {
+ public FolderException(FolderErrorCode errorCode) {
super(errorCode);
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/com/web/SearchWeb/folder/error/FolderException.java` at line
10, The FolderException constructor currently accepts the generic ErrorCode
which allows non-folder error codes to be passed; change the constructor
signature in class FolderException to accept FolderErrorCode instead (replace
ErrorCode parameter with FolderErrorCode) and update all call sites that
construct new FolderException(...) to pass a FolderErrorCode (adjust
imports/usages as needed) so the API boundary is type-safe; keep the rest of
FolderException behavior unchanged.
Comment on lines
+13
to
+15
| List<MemberFolder> listRootFolders(Long loginId, Long ownerMemberId); | ||
|
|
||
| List<MemberFolder> listChildren(Long ownerMemberId, Long parentFolderId); | ||
| List<MemberFolder> listChildren(Long loginId, Long ownerMemberId, Long parentFolderId); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
fd 'MemberFolder(Controller|ServiceImpl)\.java$' src/main/java -x sh -c '
echo "== $1 ==";
rg -n -C3 "listRootFolders|listChildren|ownerMemberId|loginId" "$1";
' sh {}Repository: Searchweb-Dev/Searchweb-Back
Length of output: 8312
조회 API의 ownerMemberId 파라미터는 이미 검증이 적용되어 있습니다.
listRootFolders/listChildren는 ownerMemberId를 받지만, validateOwner(loginId, ownerMemberId) 호출 시 ownerMemberId와 loginId가 일치하는지 검증합니다. 일치하지 않으면 즉시 FolderException(FOLDER_FORBIDDEN)을 발생시키므로, 컨트롤러가 요청값을 그대로 넘겨도 인증 경계는 보호됩니다.
다만 API 설계상 ownerMemberId 파라미터를 제거하고 loginId에서만 소유자를 취득하면 의도가 더 명확해질 수 있습니다. 예를 들어 /root와 /children/{parentFolderId} 엔드포인트로 단순화하면 URL 경로에서 명시적 검증 로직이 필요 없게 됩니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/com/web/SearchWeb/folder/service/MemberFolderService.java`
around lines 13 - 15, Remove the redundant ownerMemberId parameter from the
MemberFolderService API: change the signatures of listRootFolders and
listChildren to accept only Long loginId (and Long parentFolderId for
listChildren), and update all implementations to derive the owner from loginId
instead of using ownerMemberId; also remove or adapt calls to
validateOwner(loginId, ownerMemberId) so they validate ownership using loginId
alone (e.g., validateOwner(loginId) or inline owner resolution) and adjust any
controllers/routes to stop passing ownerMemberId.
Comment on lines
+68
to
+78
| private String normalizeFolderName(String folderName) { | ||
| if (folderName == null) { | ||
| throw new FolderException(FolderErrorCode.INVALID_FOLDER_NAME); | ||
| } | ||
|
|
||
| String normalizedFolderName = folderName.trim(); | ||
| if (normalizedFolderName.isEmpty() || normalizedFolderName.length() > 50) { | ||
| throw new FolderException(FolderErrorCode.INVALID_FOLDER_NAME); | ||
| } | ||
| return normalizedFolderName; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check MemberFolder entity for folderName column length definition
ast-grep --pattern $'@Column($$$length = $_$$$)
private String folderName;'Repository: Searchweb-Dev/Searchweb-Back
Length of output: 54
🏁 Script executed:
fd -name "MemberFolder.java" -type fRepository: Searchweb-Dev/Searchweb-Back
Length of output: 302
🏁 Script executed:
fd "MemberFolder.java"Repository: Searchweb-Dev/Searchweb-Back
Length of output: 135
🏁 Script executed:
cat -n "src/main/java/com/web/SearchWeb/folder/domain/MemberFolder.java"Repository: Searchweb-Dev/Searchweb-Back
Length of output: 1846
서비스 검증 로직과 엔티티 컬럼 길이 불일치
MemberFolder 엔티티의 folderName 필드가 @Column(length = 100)으로 정의되어 있으나, normalizeFolderName 메서드에서는 50자 제한을 적용하고 있습니다. 데이터베이스 제약과 서비스 로직 간 일관성을 맞추거나, 차이가 의도된 비즈니스 규칙이라면 명시적으로 문서화하세요.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/com/web/SearchWeb/folder/service/MemberFolderServiceImpl.java`
around lines 68 - 78, The service validation in normalizeFolderName enforces a
50-character limit that conflicts with MemberFolder.folderName which is
annotated `@Column`(length = 100); fix by making the constraints consistent:
either update normalizeFolderName to allow up to 100 characters (replace the 50
check) or change the MemberFolder.folderName column length to 50 and update any
DB migrations; ensure FolderException uses FolderErrorCode.INVALID_FOLDER_NAME
as before and, if the shorter 50 limit is intentional, add a clear comment or
Javadoc near MemberFolder.folderName and normalizeFolderName documenting the
business rule.
Comment on lines
+80
to
+91
| private String normalizeDescription(String description) { | ||
| if (description == null) { | ||
| return null; | ||
| } | ||
|
|
||
| String normalizedDescription = description.trim(); | ||
| if (normalizedDescription.isEmpty()) { | ||
| return null; | ||
| } | ||
|
|
||
| return normalizedDescription; | ||
| } |
There was a problem hiding this comment.
설명(description) 최대 길이 검증 누락
normalizeFolderName은 최대 길이를 검증하지만, normalizeDescription은 길이 검증이 없습니다. MemberFolder 엔티티에서 description은 @Column(length = 500)으로 정의되어 있으므로, 500자를 초과하면 DB 제약 조건 위반으로 런타임 예외가 발생할 수 있습니다.
🛡️ 길이 검증 추가 제안
private String normalizeDescription(String description) {
if (description == null) {
return null;
}
String normalizedDescription = description.trim();
if (normalizedDescription.isEmpty()) {
return null;
}
+ if (normalizedDescription.length() > 500) {
+ throw new FolderException(FolderErrorCode.INVALID_DESCRIPTION);
+ }
+
return normalizedDescription;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/com/web/SearchWeb/folder/service/MemberFolderServiceImpl.java`
around lines 80 - 91, normalizeDescription currently trims and null-checks but
lacks the max-length validation present in normalizeFolderName; add a length
check to ensure the returned description does not exceed the MemberFolder
`@Column`(length = 500) constraint. Modify normalizeDescription to trim, return
null for empty, then if length > 500 either truncate to 500 or throw an
IllegalArgumentException (match the approach used in normalizeFolderName), and
prefer introducing/using a named constant (e.g., MAX_DESCRIPTION_LENGTH = 500)
to make the limit explicit.
Comment on lines
+220
to
235
| private void validateNoCycle(Long folderId, MemberFolder parentFolder) { | ||
| MemberFolder current = parentFolder; | ||
| while (current != null) { | ||
| if (current.getMemberFolderId().equals(folderId)) { | ||
| throw new FolderException(FolderErrorCode.INVALID_FOLDER_MOVE); | ||
| } | ||
|
|
||
| Long nextParentId = current.getParentFolderId(); | ||
| if (nextParentId == null) { | ||
| return; | ||
| } | ||
|
|
||
| current = memberFolderJpaRepository.findById(nextParentId) | ||
| .orElseThrow(() -> new FolderException(FolderErrorCode.FOLDER_NOT_FOUND)); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
순환 참조 검증 로직 적절함, 성능 고려 권장
validateNoCycle의 로직이 정확합니다. 다만, 깊은 폴더 계층 구조에서는 여러 번의 DB 조회가 발생할 수 있습니다. 현재 구현은 일반적인 사용 사례에서 충분하지만, 폴더 계층이 깊어질 경우 재귀 CTE 쿼리로 최적화를 고려할 수 있습니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/com/web/SearchWeb/folder/service/MemberFolderServiceImpl.java`
around lines 220 - 235, validateNoCycle currently walks parent links and issues
a DB call per level via memberFolderJpaRepository.findById, which can be slow
for deep hierarchies; replace or supplement this with a single recursive query
(CTE) in the repository to fetch the ancestor chain (or perform a direct cycle
check) and use that result to detect folderId in the ancestors, updating
validateNoCycle to call the new repository method instead of looping; reference
validateNoCycle, memberFolderJpaRepository.findById, and the
FolderException/FOLDER_NOT_FOUND and INVALID_FOLDER_MOVE error codes when adding
the repository method and adjusting the service logic.
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.
💡 이슈
resolve {#SW-61}
🤩 개요
PR의 개요를 적어주세요.
폴더 CRUD 기능의 비지니스 로직 및 검증 로직 보완
규칙에 맞는 예외처리
🧑💻 작업 사항
https://searchweb.atlassian.net/wiki/x/A4BJAQ
📖 참고 사항
공유할 내용, 레퍼런스, 추가로 발생할 것으로 예상되는 이슈, 스크린샷 등을 넣어 주세요.
Summary by CodeRabbit
릴리스 노트
New Features
Bug Fixes
Refactor