Conversation
|
Caution Review failedThe pull request is closed. Walkthrough이번 변경에서는 인증 및 인가 로직과 카카오/구글 소셜 로그인, JWT 기반 인증, Spring Security 설정, 사용자 식별을 위한 커스텀 argument resolver, 예외 처리, 관련 DTO 및 테스트 코드 리팩토링이 대규모로 이루어졌습니다. User 엔티티는 이메일 대신 OAuth2 ID 기반으로 변경되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant JwtAuthenticationFilter
participant JwtUtil
participant SecurityContext
participant Controller
Client->>JwtAuthenticationFilter: HTTP 요청 (Authorization 헤더 포함)
JwtAuthenticationFilter->>JwtUtil: 토큰 추출 및 검증
JwtUtil-->>JwtAuthenticationFilter: LoginUser 반환/예외 발생
alt 토큰 유효
JwtAuthenticationFilter->>SecurityContext: 인증 정보 저장
JwtAuthenticationFilter->>Controller: 요청 전달 (UserId/Oauth2Id argument resolver 작동)
else 토큰 없음/유효하지 않음
JwtAuthenticationFilter->>Controller: 요청 전달 (인증 정보 없음)
end
sequenceDiagram
participant Client
participant OAuth2Provider
participant CustomOAuth2UserService
participant UserJpaRepository
participant JwtUtil
participant CustomSuccessHandler
Client->>OAuth2Provider: OAuth2 인증 요청
OAuth2Provider->>CustomOAuth2UserService: 인증 결과 전달
CustomOAuth2UserService->>UserJpaRepository: Oauth2Id로 사용자 조회
alt 신규 사용자
CustomOAuth2UserService-->>CustomSuccessHandler: LoginUser(isNewUser)
CustomSuccessHandler->>JwtUtil: Signup 토큰 생성
CustomSuccessHandler->>Client: Signup 토큰 응답 헤더+바디
else 기존 사용자
CustomOAuth2UserService-->>CustomSuccessHandler: LoginUser(userId)
CustomSuccessHandler->>JwtUtil: Access 토큰 생성
CustomSuccessHandler->>Client: Access 토큰 응답 헤더+바디
end
Assessment against linked issues
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
🧹 Nitpick comments (15)
src/main/java/konkuk/thip/common/security/annotation/Oauth2Id.java (1)
10-10: 어노테이션 네이밍 일관성 개선을 고려해보세요.OAuth2 표준 네이밍에 따라
@interface OAuth2Id로 변경하는 것을 권장합니다. 대문자 'O'와 숫자 '2'를 사용하는 것이 일반적인 OAuth2 명명 규칙입니다.-public @interface Oauth2Id { +public @interface OAuth2Id {src/main/java/konkuk/thip/user/application/port/in/dto/UserSignupCommand.java (1)
9-9: OAuth2 ID 필드에 대한 검증 또는 문서화 추가를 고려하세요.email에서 oauth2Id로의 전환은 OAuth2 통합에 적절합니다. 하지만 oauth2Id 필드에 대한 유효성 검증이나 JavaDoc 주석을 추가하여 필드의 목적과 형식을 명확히 하는 것을 권장합니다.
+ /** + * OAuth2 provider로부터 받은 고유 사용자 식별자 + * @param oauth2Id OAuth2 provider의 사용자 고유 ID + */ String oauth2Idsrc/test/java/konkuk/thip/user/adapter/out/jpa/UserJpaEntityTest.java (1)
43-45: OAuth2 ID 관련 검증 추가를 고려해 주세요.현재 테스트는 기본적인 저장/조회만 검증하고 있습니다. OAuth2 ID 필드가 새로 추가된 만큼 관련 검증을 추가하는 것이 좋겠습니다.
다음과 같은 검증 추가를 제안합니다:
// then assertThat(foundUser.getNickname()).isEqualTo("테스터"); + assertThat(foundUser.getOauth2Id()).isEqualTo("kakao_12345678"); assertThat(foundUser.getAliasForUserJpaEntity().getValue()).isEqualTo("칭호"); assertThat(foundUser.getRole()).isEqualTo(UserRole.USER);src/test/java/konkuk/thip/feed/adapter/out/jpa/FeedJpaEntityTest.java (1)
24-25: @PersistenceContext 대신 @Autowired 사용에 대한 검토 필요EntityManager에 대해
@PersistenceContext에서@Autowired로 변경했습니다. JPA 테스트에서는@PersistenceContext가 더 명시적이고 적절한 선택입니다.- @Autowired + @PersistenceContext private EntityManager em;src/main/java/konkuk/thip/common/security/constant/AuthParameters.java (1)
11-12: OAuth2 제공자별 ID 키에 대한 문서화 고려Kakao와 Google의 프로바이더별 ID 키가 정의되어 있습니다. 이런 외부 API 의존적인 상수들은 주석으로 출처를 명시하는 것이 좋겠습니다.
+ // Kakao OAuth2 API에서 사용자 ID를 가져오는 키 KAKAO_PROVIDER_ID_KEY("id"), + // Google OAuth2 API에서 사용자 ID를 가져오는 키 GOOGLE_PROVIDER_ID_KEY("sub"),src/main/java/konkuk/thip/common/security/oauth2/KakaoUserDetails.java (1)
26-31: 주석 처리된 코드를 제거하는 것을 고려해주세요.이메일 관련 코드가 주석 처리되어 있는데, OAuth2 ID 기반 인증으로 전환된다면 이 코드를 완전히 제거하는 것이 좋겠습니다.
-// @Override -// public String getEmail() { -// Object object = attributes.get("kakao_account"); -// LinkedHashMap accountMap = (LinkedHashMap) object; -// return accountMap.get("email").toString(); -// }src/main/java/konkuk/thip/common/security/oauth2/GoogleUserDetails.java (1)
29-34: 주석 처리된 코드를 제거해주세요.
KakaoUserDetails와 마찬가지로 이메일 관련 주석 코드를 제거하는 것이 좋겠습니다.-// @Override -// public String getEmail() { -// -// return attribute.get("email").toString(); -// }src/main/java/konkuk/thip/common/security/oauth2/CustomOAuth2User.java (1)
18-21: getAttributes() 메서드에서 null 반환이 잠재적 문제를 야기할 수 있습니다.일부 OAuth2 제공자나 Spring Security 구성 요소는 attributes가 null이 아닐 것으로 예상할 수 있습니다. 빈 Map을 반환하는 것을 고려해보세요.
@Override public Map<String, Object> getAttributes() { - return null; + return Map.of(); }src/main/java/konkuk/thip/common/security/util/JwtUtil.java (2)
26-28: 토큰 만료 시간을 설정 파일로 외부화해야 합니다.하드코딩된 만료 시간을 환경변수나 설정 파일로 이동해야 합니다.
TODO 주석이 있는 부분을 해결하기 위해 설정 외부화 작업을 도와드릴까요?
- //todo 확정 후 환경변수로 변경 - private final long tokenExpiredMs = 1000 * 60 * 60 * 24; // 24시간 - private final long signupTokenExpiredMs = 5 * 60 * 1000; // 5분 + @Value("${jwt.access-token.expiration:86400000}") + private final long tokenExpiredMs; + + @Value("${jwt.signup-token.expiration:300000}") + private final long signupTokenExpiredMs;
68-70: 중복된 만료 확인 로직을 제거하는 것을 고려해보세요.
validateToken메서드에서 이미ExpiredJwtException을 처리하고 있어isExpired메서드가 중복될 수 있습니다.이 메서드가 필요한 특별한 사용 사례가 있는지 확인해보세요. 없다면 제거하고
validateToken만 사용하는 것이 좋겠습니다.src/main/java/konkuk/thip/common/security/filter/JwtAuthenticationFilter.java (1)
41-47: JWT 검증 로직을 통합할 수 있습니다.
validateToken과isExpired를 별도로 호출하는 것은 중복된 파싱을 야기합니다.
JwtUtil.validateToken메서드가 만료도 함께 확인하므로isExpired호출을 제거할 수 있습니다.if (!jwtUtil.validateToken(token)) { throw new AuthException(AUTH_INVALID_TOKEN); } - -if (jwtUtil.isExpired(token)) { - throw new AuthException(AUTH_EXPIRED_TOKEN); -}src/main/java/konkuk/thip/common/security/oauth2/CustomOAuth2UserService.java (2)
43-43: OAuth2 ID 형식을 상수로 정의하세요.OAuth2 ID 형식이 하드코딩되어 있습니다. 유지보수성을 위해 상수로 정의하는 것이 좋습니다.
AuthParameters 클래스에 추가:
public static final String OAUTH2_ID_DELIMITER = "_";그리고 이 라인을 다음과 같이 수정:
-String oauth2Id = oAuth2UserDetails.getProvider() + "_" + oAuth2UserDetails.getProviderId(); +String oauth2Id = oAuth2UserDetails.getProvider() + OAUTH2_ID_DELIMITER + oAuth2UserDetails.getProviderId();
23-52: 트랜잭션 처리를 추가하세요.데이터베이스 읽기 작업이지만, 동시성 문제를 방지하기 위해 @transactional(readOnly = true) 어노테이션 추가를 고려하세요.
클래스나 메서드 레벨에 추가:
+@Transactional(readOnly = true) @Override public OAuth2User loadUser(OAuth2UserRequest userRequest) throws OAuth2AuthenticationException {src/test/java/konkuk/thip/user/adapter/in/web/UserCommandControllerTest.java (2)
90-93: 일관된 테스트 데이터 사용을 권장합니다.기존 테스트들에서 닉네임으로 다양한 값들("테스트 유저", "테스트유저", "11자_닉네임_입니다")을 사용하고 있습니다. 테스트의 일관성과 가독성을 위해 표준화된 테스트 데이터 사용을 고려해보세요.
- "테스트 유저" + "테스트유저"또는 TestEntityFactory 같은 팩토리 클래스를 활용하여 일관된 테스트 데이터를 생성하는 것을 권장합니다.
Also applies to: 96-100, 110-113, 116-120, 130-133, 136-140, 192-195
66-67: 테스트 코드 중복을 줄이기 위한 헬퍼 메서드 도입을 고려해보세요.여러 테스트에서 동일한 패턴으로 임시 토큰을 생성하고 있습니다. 테스트 유지보수성을 위해 헬퍼 메서드를 도입하는 것을 권장합니다.
private String createTestSignupToken() { return jwtUtil.createSignupToken("kakao_12345678"); } private String createTestSignupToken(String oauth2Id) { return jwtUtil.createSignupToken(oauth2Id); }그리고 테스트에서 다음과 같이 사용:
- String testToken = jwtUtil.createSignupToken("kakao_12345678"); + String testToken = createTestSignupToken();Also applies to: 96-97, 116-117, 136-137, 165-166
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (36)
build.gradle(1 hunks)src/main/java/konkuk/thip/common/exception/AuthException.java(1 hunks)src/main/java/konkuk/thip/common/exception/code/ErrorCode.java(1 hunks)src/main/java/konkuk/thip/common/exception/handler/GlobalExceptionHandler.java(3 hunks)src/main/java/konkuk/thip/common/security/annotation/Oauth2Id.java(1 hunks)src/main/java/konkuk/thip/common/security/annotation/UserId.java(1 hunks)src/main/java/konkuk/thip/common/security/argument_resolver/Oauth2IdArgumentResolver.java(1 hunks)src/main/java/konkuk/thip/common/security/argument_resolver/UserIdArgumentResolver.java(1 hunks)src/main/java/konkuk/thip/common/security/constant/AuthParameters.java(1 hunks)src/main/java/konkuk/thip/common/security/filter/JwtAuthenticationEntryPoint.java(1 hunks)src/main/java/konkuk/thip/common/security/filter/JwtAuthenticationFilter.java(1 hunks)src/main/java/konkuk/thip/common/security/oauth2/CustomOAuth2User.java(1 hunks)src/main/java/konkuk/thip/common/security/oauth2/CustomOAuth2UserService.java(1 hunks)src/main/java/konkuk/thip/common/security/oauth2/CustomSuccessHandler.java(1 hunks)src/main/java/konkuk/thip/common/security/oauth2/GoogleUserDetails.java(1 hunks)src/main/java/konkuk/thip/common/security/oauth2/KakaoUserDetails.java(1 hunks)src/main/java/konkuk/thip/common/security/oauth2/LoginUser.java(1 hunks)src/main/java/konkuk/thip/common/security/oauth2/OAuth2UserDetails.java(1 hunks)src/main/java/konkuk/thip/common/security/util/JwtUtil.java(1 hunks)src/main/java/konkuk/thip/config/SecurityConfig.java(1 hunks)src/main/java/konkuk/thip/config/WebMvcConfig.java(1 hunks)src/main/java/konkuk/thip/user/adapter/in/web/UserCommandController.java(1 hunks)src/main/java/konkuk/thip/user/adapter/in/web/request/UserSignupRequest.java(1 hunks)src/main/java/konkuk/thip/user/adapter/out/jpa/UserJpaEntity.java(1 hunks)src/main/java/konkuk/thip/user/adapter/out/mapper/UserMapper.java(1 hunks)src/main/java/konkuk/thip/user/adapter/out/persistence/UserJpaRepository.java(1 hunks)src/main/java/konkuk/thip/user/application/port/in/dto/UserSignupCommand.java(1 hunks)src/main/java/konkuk/thip/user/application/service/UserSignupService.java(1 hunks)src/main/java/konkuk/thip/user/domain/User.java(1 hunks)src/test/java/konkuk/thip/common/util/TestEntityFactory.java(1 hunks)src/test/java/konkuk/thip/feed/adapter/out/jpa/FeedJpaEntityTest.java(3 hunks)src/test/java/konkuk/thip/room/adapter/out/jpa/RecordJpaEntityTest.java(2 hunks)src/test/java/konkuk/thip/room/adapter/out/jpa/RoomJpaEntityTest.java(2 hunks)src/test/java/konkuk/thip/room/adapter/out/jpa/VoteJpaEntityTest.java(3 hunks)src/test/java/konkuk/thip/user/adapter/in/web/UserCommandControllerTest.java(7 hunks)src/test/java/konkuk/thip/user/adapter/out/jpa/UserJpaEntityTest.java(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (7)
src/test/java/konkuk/thip/user/adapter/out/jpa/UserJpaEntityTest.java (1)
src/test/java/konkuk/thip/common/util/TestEntityFactory.java (1)
TestEntityFactory(13-75)
src/test/java/konkuk/thip/room/adapter/out/jpa/RecordJpaEntityTest.java (1)
src/test/java/konkuk/thip/common/util/TestEntityFactory.java (1)
TestEntityFactory(13-75)
src/test/java/konkuk/thip/room/adapter/out/jpa/RoomJpaEntityTest.java (1)
src/test/java/konkuk/thip/common/util/TestEntityFactory.java (1)
TestEntityFactory(13-75)
src/main/java/konkuk/thip/common/security/argument_resolver/Oauth2IdArgumentResolver.java (1)
src/main/java/konkuk/thip/common/security/argument_resolver/UserIdArgumentResolver.java (1)
Component(16-38)
src/test/java/konkuk/thip/feed/adapter/out/jpa/FeedJpaEntityTest.java (1)
src/test/java/konkuk/thip/common/util/TestEntityFactory.java (1)
TestEntityFactory(13-75)
src/main/java/konkuk/thip/common/security/argument_resolver/UserIdArgumentResolver.java (1)
src/main/java/konkuk/thip/common/security/argument_resolver/Oauth2IdArgumentResolver.java (1)
Component(16-38)
src/main/java/konkuk/thip/common/security/oauth2/CustomOAuth2User.java (1)
src/main/java/konkuk/thip/user/domain/User.java (1)
Getter(7-34)
🪛 Gitleaks (8.26.0)
src/test/java/konkuk/thip/user/adapter/in/web/UserCommandControllerTest.java
164-164: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🔇 Additional comments (39)
src/main/java/konkuk/thip/common/security/annotation/UserId.java (1)
8-11: 어노테이션 구조가 올바르게 설계되었습니다.Spring의 argument resolver 패턴을 따라
@Target(ElementType.PARAMETER)와@Retention(RetentionPolicy.RUNTIME)설정이 적절합니다. 컨트롤러 메서드에서 인증된 사용자 ID 주입을 위한 최소한의 구현이 잘 되어 있습니다.src/main/java/konkuk/thip/common/security/oauth2/OAuth2UserDetails.java (1)
3-7: OAuth2 제공자 추상화를 위한 인터페이스가 잘 설계되었습니다.Google과 Kakao 소셜 로그인 통합을 위한 깔끔한 추상화입니다.
getProvider()와getProviderId()메서드는 현재 요구사항을 충족하며, 향후 다른 OAuth2 제공자 추가 시에도 확장 가능한 구조입니다.src/test/java/konkuk/thip/user/adapter/out/jpa/UserJpaEntityTest.java (1)
33-34: TestEntityFactory 사용은 좋은 개선입니다.팩토리 패턴을 사용한 리팩토링으로 테스트 데이터 생성의 일관성과 유지보수성이 향상되었습니다.
src/main/java/konkuk/thip/user/domain/User.java (2)
21-21: OAuth2 ID 필드 추가는 적절합니다.도메인 엔티티에서 email을 oauth2Id로 대체하는 것은 OAuth2 기반 인증 전환에 적합한 변경입니다.
23-31: 팩토리 메서드User.withoutId호출부 검증 완료
src/main/java/konkuk/thip/user/application/service/UserSignupService.java에서
User.withoutId(command.nickname(), alias.getImageUrl(), USER.getType(), alias.getId(), command.oauth2Id())
호출부의 인자 순서가(nickname, imageUrl, userRole, aliasId, oauth2Id)시그니처와 일치함을 확인했습니다. 다른 호출부는 존재하지 않습니다.build.gradle (2)
34-43: 보안 관련 의존성 추가가 적절합니다.OAuth2 로그인 및 JWT 기반 인증을 위한 필요한 의존성들이 올바르게 추가되었습니다:
- Spring Security: 기본 보안 프레임워크
- JWT 라이브러리 (0.12.3): 최신 버전으로 적절
- OAuth2 클라이언트: 소셜 로그인 지원
50-50: 테스트 의존성 추가가 적절합니다.Spring Security 테스트 의존성 추가로 보안 관련 기능들을 적절히 테스트할 수 있게 되었습니다.
src/main/java/konkuk/thip/user/adapter/out/mapper/UserMapper.java (2)
17-17: JPA 엔티티 매핑이 올바르게 업데이트되었습니다.email에서 oauth2Id로의 필드 매핑 변경이 적절하게 이루어졌습니다.
29-29: 도메인 엔티티 매핑이 일관되게 업데이트되었습니다.JPA 엔티티에서 도메인 엔티티로의 매핑도 oauth2Id 필드로 일관되게 변경되어 데이터 흐름이 올바르게 유지됩니다.
src/main/java/konkuk/thip/config/WebMvcConfig.java (1)
1-25: Spring MVC 설정이 올바르게 구현되었습니다.인증 관련 argument resolver들이 적절히 등록되어 있고, 표준적인 Spring MVC 설정 패턴을 잘 따르고 있습니다.
src/test/java/konkuk/thip/room/adapter/out/jpa/RoomJpaEntityTest.java (3)
6-6: TestEntityFactory 도입으로 테스트 코드가 개선되었습니다.새로운 import 추가가 적절하며, 특히 AssertJ static import를 통해 더 읽기 쉬운 assertion을 사용할 수 있게 되었습니다.
Also applies to: 10-10, 18-18
42-46: 테스트 데이터 생성이 표준화되었습니다.수동으로 엔티티를 생성하던 방식에서 TestEntityFactory를 사용하는 방식으로 변경되어 테스트 코드의 일관성과 유지보수성이 향상되었습니다.
55-61: TestEntityFactory의 기본값과 일치하도록 assertion이 올바르게 업데이트되었습니다.팩토리에서 제공하는 값("방이름", "설명", 3, "책제목" 등)과 일치하도록 테스트 검증 로직이 적절히 수정되었습니다.
src/main/java/konkuk/thip/common/exception/code/ErrorCode.java (1)
19-24: 인증 관련 에러코드가 체계적으로 정의되었습니다.HTTP 401 상태코드와 40100번대 에러코드를 사용하여 인증 관련 오류를 명확하게 분류했습니다. 각 에러 유형(토큰 무효, 만료, 미인증, 토큰 없음, 로그인 실패)이 적절히 구분되어 있습니다.
src/main/java/konkuk/thip/common/exception/AuthException.java (1)
1-19: 인증 예외 클래스가 적절하게 구현되었습니다.ErrorCode를 포함하는 커스텀 예외 클래스로, 인증 관련 오류를 체계적으로 처리할 수 있도록 설계되었습니다. 두 가지 생성자를 통해 원인 예외 포함 여부를 선택할 수 있어 유연성을 제공합니다.
src/test/java/konkuk/thip/room/adapter/out/jpa/RecordJpaEntityTest.java (3)
6-6: TestEntityFactory import가 추가되어 테스트 코드 표준화에 기여합니다.다른 테스트 클래스들과 동일한 패턴으로 리팩토링되어 프로젝트 전체의 테스트 코드 일관성이 향상되었습니다.
51-57: 엔티티 생성 로직이 TestEntityFactory로 표준화되었습니다.복잡한 엔티티 관계 설정(alias → user, category, book → room → record)이 팩토리 메서드를 통해 간결하게 처리되어 테스트 코드의 가독성과 유지보수성이 크게 향상되었습니다.
62-62: 테스트 구조가 개선되고 assertion이 팩토리 값과 일치하도록 업데이트되었습니다.given-when-then 주석으로 테스트 단계가 명확해졌고, nickname 검증이 TestEntityFactory의 기본값("테스터")과 일치하도록 올바르게 수정되었습니다.
Also applies to: 65-65, 69-69
src/main/java/konkuk/thip/user/adapter/in/web/request/UserSignupRequest.java (1)
16-22: OAuth2 ID 기반 변경사항이 올바르게 구현됨이메일 필드 제거 및 OAuth2 ID 파라미터 추가가 깔끔하게 처리되었습니다. 기존 검증 로직도 적절하게 유지되고 있습니다.
src/test/java/konkuk/thip/feed/adapter/out/jpa/FeedJpaEntityTest.java (1)
43-52: TestEntityFactory 사용으로 테스트 일관성 향상중앙화된
TestEntityFactory를 사용하여 테스트 데이터 생성이 일관되고 유지보수가 용이해졌습니다. 좋은 리팩토링입니다.src/main/java/konkuk/thip/common/security/constant/AuthParameters.java (1)
6-22: 상수 중앙화로 타입 안전성 및 유지보수성 향상인증 관련 상수들을 enum으로 중앙화하여 타입 안전성과 유지보수성을 크게 향상시켰습니다. 좋은 설계 패턴입니다.
src/main/java/konkuk/thip/common/security/oauth2/LoginUser.java (2)
3-7: 레코드 구조가 깔끔하고 잘 설계되었습니다.OAuth2 로그인 사용자 정보를 캡슐화하는 불변 데이터 구조로 적절히 구현되었습니다.
8-14: 정적 팩토리 메서드가 명확하고 사용하기 쉽게 구현되었습니다.
createNewUser와createExistingUser메서드가 명확한 의도를 보여주며, 각각의 사용 시나리오를 잘 표현하고 있습니다.src/main/java/konkuk/thip/user/application/service/UserSignupService.java (1)
27-29: User.withoutId 호출부 일관성 확인 완료실행한 검색 결과
User.withoutId호출은UserSignupService.java한 곳뿐이며, 새 시그니처(닉네임, 이미지 URL, USER 타입, aliasId, oauth2Id)가 일관되게 적용된 것을 확인했습니다. 추가 검토나 수정은 불필요합니다.src/main/java/konkuk/thip/common/security/oauth2/KakaoUserDetails.java (1)
8-14: 카카오 OAuth2 사용자 세부정보 구현이 적절합니다.
OAuth2UserDetails인터페이스를 올바르게 구현하였고, 생성자를 통해 속성을 주입받는 구조가 명확합니다.src/test/java/konkuk/thip/room/adapter/out/jpa/VoteJpaEntityTest.java (3)
27-28: EntityManager 주입 방식 변경이 적절합니다.
@PersistenceContext에서@Autowired로 변경한 것이 일관성 있고 명확합니다.
51-57: TestEntityFactory 사용으로 테스트 코드가 크게 개선되었습니다.중앙화된 엔티티 팩토리를 사용하여 테스트 설정이 간소화되고 일관성이 향상되었습니다. given/when/then 구조도 명확합니다.
75-76: 추가된 검증이 테스트 커버리지를 향상시켰습니다.투표 내용과 사용자 닉네임에 대한 추가적인 assert가 테스트의 완성도를 높였습니다.
src/main/java/konkuk/thip/user/adapter/in/web/UserCommandController.java (1)
27-34: OAuth2 기반 회원가입 로직이 올바르게 구현되었습니다.JWT 토큰 생성과 헤더 설정이 적절히 처리되고 있습니다. OAuth2 ID 주입도 커스텀 어노테이션을 통해 깔끔하게 구현되었네요.
src/main/java/konkuk/thip/common/security/filter/JwtAuthenticationEntryPoint.java (1)
14-30: 인증 진입점 구현이 잘 되어있습니다.HandlerExceptionResolver를 활용한 중앙집중식 예외 처리가 적절합니다. request 속성에서 예외를 우선 확인하고 fallback하는 로직도 올바릅니다.
src/main/java/konkuk/thip/common/exception/handler/GlobalExceptionHandler.java (2)
75-82: AuthException 핸들러 추가가 적절합니다.인증/인가 관련 예외를 별도로 처리하는 것은 좋은 접근입니다. 기존 패턴을 잘 따르고 있습니다.
94-109: 예외 핸들러 분리가 적절합니다.RuntimeException과 IllegalStateException을 분리하여 처리하는 것은 더 명확한 예외 처리를 가능하게 합니다.
src/main/java/konkuk/thip/common/security/oauth2/CustomSuccessHandler.java (1)
26-43: 로그인 성공 처리 로직이 잘 구현되었습니다.새 사용자와 기존 사용자를 적절히 구분하여 처리하고 있습니다.
src/test/java/konkuk/thip/common/util/TestEntityFactory.java (1)
13-75: 테스트 엔티티 팩토리 구현이 훌륭합니다.테스트 데이터 생성을 중앙화하여 일관성을 높이고 중복을 줄이는 좋은 접근입니다.
src/test/java/konkuk/thip/user/adapter/in/web/UserCommandControllerTest.java (5)
5-5: 의존성 및 상수 추가가 적절하게 구성되었습니다.JWT 기반 인증 시스템 도입에 따른 필요한 의존성들이 올바르게 추가되었습니다.
Also applies to: 22-22, 46-47
60-63: 회원가입 요청 구조 변경이 올바르게 반영되었습니다.이메일 필드 제거와 OAuth2 기반 인증을 위한 임시 토큰 사용이 적절하게 구현되었습니다.
Also applies to: 66-70
147-186: OAuth2 ID 추출 로직 검증 테스트가 잘 구현되었습니다.임시 토큰에서 OAuth2 ID를 올바르게 추출하여 사용자 엔티티에 저장되는지 검증하는 중요한 테스트 케이스입니다. 테스트 로직이 명확하고 검증 부분이 적절합니다.
189-204: 인증 실패 시나리오 테스트가 적절하게 추가되었습니다.토큰 없이 요청할 때 401 Unauthorized 응답과 적절한 에러 코드/메시지를 반환하는지 검증하는 보안 관련 테스트가 잘 구현되었습니다.
164-164: 테스트용 OAuth2 ID 사용에 대한 정적 분석 경고를 무시해도 됩니다.정적 분석 도구가 "kakao_12345678"을 API 키로 감지했지만, 이는 테스트용 가짜 OAuth2 ID이므로 보안상 문제가 없습니다.
src/main/java/konkuk/thip/user/adapter/out/persistence/UserJpaRepository.java
Outdated
Show resolved
Hide resolved
| @Column(name = "oauth2_id", length = 50, nullable = false) | ||
| private String oauth2Id; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
데이터 마이그레이션 및 제약조건 고려가 필요합니다.
OAuth2 ID 필드 추가는 적절하지만, 다음 사항들을 고려해 주세요:
- Unique 제약조건 추가: OAuth2 ID는 사용자 식별자로 사용되므로 unique 제약조건이 필요합니다.
- 필드 길이 검증: 다양한 OAuth2 제공자(Google, Kakao, Naver 등)의 ID 길이를 확인하여 50자가 충분한지 검증해 주세요.
- 기존 데이터 마이그레이션: 현재 사용자 데이터가 있다면 마이그레이션 전략이 필요합니다.
다음 수정을 제안합니다:
- @Column(name = "oauth2_id", length = 50, nullable = false)
+ @Column(name = "oauth2_id", length = 100, nullable = false, unique = true)
private String oauth2Id;다양한 OAuth2 제공자의 ID 길이를 확인하는 스크립트:
🏁 Script executed:
#!/bin/bash
# OAuth2 제공자별 ID 형식 및 길이 확인
echo "OAuth2 제공자별 ID 형식 및 최대 길이 확인이 필요합니다."
echo "Google: 숫자 21자리 (예: 123456789012345678901)"
echo "Kakao: 숫자 최대 15자리 정도"
echo "Naver: 문자+숫자 조합"Length of output: 269
OAuth2 ID 필드에 대한 unique 제약조건 및 마이그레이션 전략 적용 필요
- 수정 위치:
src/main/java/konkuk/thip/user/adapter/out/jpa/UserJpaEntity.java (27–28줄) - 고려 사항:
• OAuth2 ID는 사용자 식별자로 사용되므로 반드시 unique 제약조건을 걸어야 합니다.
• Google(21자리), Kakao(15자리), Naver(알파벳·숫자 조합) 등 모든 제공자의 ID를 수용할 수 있도록 길이를 100자로 여유 있게 설정하세요.
• 이미 운영 중인 테이블에 NOT NULL·UNIQUE 제약을 추가할 때는 데이터 마이그레이션 스크립트(ALTER TABLE, 기존 NULL 처리 등) 를 별도로 준비해야 합니다.
제안 차이(diff):
- @Column(name = "oauth2_id", length = 50, nullable = false)
+ @Column(name = "oauth2_id", length = 100, nullable = false, unique = true)
private String oauth2Id;추가로, 복합 제약 설정이 필요할 경우 엔티티 클래스 상단에 다음과 같이 @Table 의 uniqueConstraints를 활용할 수도 있습니다:
@Table(
name = "users",
uniqueConstraints = @UniqueConstraint(columnNames = "oauth2_id")
)데이터 마이그레이션 계획도 함께 수립해 주세요.
🤖 Prompt for AI Agents
In src/main/java/konkuk/thip/user/adapter/out/jpa/UserJpaEntity.java around
lines 27 to 28, update the oauth2Id field to have a length of 100 and add a
unique constraint to ensure uniqueness at the database level. Modify the @Column
annotation accordingly and add a @Table annotation at the class level with
uniqueConstraints set on the oauth2_id column. Additionally, prepare a data
migration script to handle existing NULL values and apply NOT NULL and UNIQUE
constraints safely on the production database.
src/main/java/konkuk/thip/common/security/argument_resolver/Oauth2IdArgumentResolver.java
Outdated
Show resolved
Hide resolved
src/main/java/konkuk/thip/common/security/argument_resolver/UserIdArgumentResolver.java
Outdated
Show resolved
Hide resolved
| } catch (Exception e) { | ||
| log.error("JWT 필터에서 오류 발생: {}", e.getMessage()); | ||
| request.setAttribute("exception", e); | ||
| } finally { | ||
| filterChain.doFilter(request, response); | ||
| } |
There was a problem hiding this comment.
예외 처리 시 보안 정보 노출을 주의해야 합니다.
로그에 전체 예외 메시지를 출력하면 민감한 정보가 노출될 수 있습니다.
} catch (Exception e) {
- log.error("JWT 필터에서 오류 발생: {}", e.getMessage());
+ log.error("JWT 필터에서 오류 발생", e);
request.setAttribute("exception", e);
}📝 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.
| } catch (Exception e) { | |
| log.error("JWT 필터에서 오류 발생: {}", e.getMessage()); | |
| request.setAttribute("exception", e); | |
| } finally { | |
| filterChain.doFilter(request, response); | |
| } | |
| } catch (Exception e) { | |
| log.error("JWT 필터에서 오류 발생", e); | |
| request.setAttribute("exception", e); | |
| } finally { | |
| filterChain.doFilter(request, response); | |
| } |
🤖 Prompt for AI Agents
In src/main/java/konkuk/thip/common/security/filter/JwtAuthenticationFilter.java
around lines 62 to 67, the current exception logging outputs the full exception
message, which may expose sensitive information. Modify the log statement to
avoid printing detailed exception messages; instead, log a generic error message
or only the exception type without sensitive details. This prevents potential
security risks from information leakage in logs.
src/main/java/konkuk/thip/common/security/oauth2/CustomOAuth2UserService.java
Show resolved
Hide resolved
| private void writeResponse(HttpServletResponse response, Object value) throws IOException { | ||
| response.setStatus(HttpServletResponse.SC_OK); | ||
| response.setContentType(CONTENT_TYPE); | ||
| response.setCharacterEncoding(ENCODING); | ||
| ObjectMapper objectMapper = new ObjectMapper(); | ||
| String body = objectMapper.writeValueAsString(value); | ||
| response.getWriter().write(body); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
ObjectMapper를 필드로 선언하여 재사용하세요.
ObjectMapper 인스턴스를 매번 생성하는 것은 비효율적입니다. 클래스 필드로 선언하여 재사용하는 것이 좋습니다.
@Component
@RequiredArgsConstructor
public class CustomSuccessHandler extends SimpleUrlAuthenticationSuccessHandler {
private static final String CONTENT_TYPE = "application/json";
private static final String ENCODING = "UTF-8";
+ private final ObjectMapper objectMapper = new ObjectMapper();
private final JwtUtil jwtUtil;
private void writeResponse(HttpServletResponse response, Object value) throws IOException {
response.setStatus(HttpServletResponse.SC_OK);
response.setContentType(CONTENT_TYPE);
response.setCharacterEncoding(ENCODING);
- ObjectMapper objectMapper = new ObjectMapper();
String body = objectMapper.writeValueAsString(value);
response.getWriter().write(body);
}📝 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.
| private void writeResponse(HttpServletResponse response, Object value) throws IOException { | |
| response.setStatus(HttpServletResponse.SC_OK); | |
| response.setContentType(CONTENT_TYPE); | |
| response.setCharacterEncoding(ENCODING); | |
| ObjectMapper objectMapper = new ObjectMapper(); | |
| String body = objectMapper.writeValueAsString(value); | |
| response.getWriter().write(body); | |
| } | |
| @Component | |
| @RequiredArgsConstructor | |
| public class CustomSuccessHandler extends SimpleUrlAuthenticationSuccessHandler { | |
| private static final String CONTENT_TYPE = "application/json"; | |
| private static final String ENCODING = "UTF-8"; | |
| private final ObjectMapper objectMapper = new ObjectMapper(); | |
| private final JwtUtil jwtUtil; | |
| private void writeResponse(HttpServletResponse response, Object value) throws IOException { | |
| response.setStatus(HttpServletResponse.SC_OK); | |
| response.setContentType(CONTENT_TYPE); | |
| response.setCharacterEncoding(ENCODING); | |
| String body = objectMapper.writeValueAsString(value); | |
| response.getWriter().write(body); | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/main/java/konkuk/thip/common/security/oauth2/CustomSuccessHandler.java
around lines 45 to 52, the ObjectMapper is instantiated inside the writeResponse
method every time it is called, which is inefficient. To fix this, declare
ObjectMapper as a private final field in the class and initialize it once, then
reuse this instance in the writeResponse method instead of creating a new one
each time.
| // //테스트를 위한 url | ||
| "/api/test/public", | ||
| "/api/test/auth-status", | ||
| "/api/test/protected", | ||
| "/auth/kakao/**", | ||
| "/kakao-login-test.html", | ||
| "/google-login-test.html", | ||
| "/index.html", | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
프로덕션 환경에서 테스트 엔드포인트를 제거하세요.
WHITELIST에 테스트용 엔드포인트가 포함되어 있습니다. 프로파일 기반으로 조건부 설정을 고려하세요.
+@Value("${spring.profiles.active:default}")
+private String activeProfile;
private static final String[] WHITELIST = {
"/swagger-ui/**", "/api-docs/**", "/swagger-ui.html",
"/v3/api-docs/**","/oauth2/authorization/**",
"/login/oauth2/code/**"
-
-// //테스트를 위한 url
- "/api/test/public",
- "/api/test/auth-status",
- "/api/test/protected",
- "/auth/kakao/**",
- "/kakao-login-test.html",
- "/google-login-test.html",
- "/index.html",
};
+private String[] getWhitelist() {
+ if ("dev".equals(activeProfile) || "local".equals(activeProfile)) {
+ return ArrayUtils.addAll(WHITELIST,
+ "/api/test/public",
+ "/api/test/auth-status",
+ "/api/test/protected",
+ "/auth/kakao/**",
+ "/kakao-login-test.html",
+ "/google-login-test.html",
+ "/index.html"
+ );
+ }
+ return WHITELIST;
+}그리고 설정에서 사용:
.authorizeHttpRequests(auth -> auth
- .requestMatchers(WHITELIST).permitAll()
+ .requestMatchers(getWhitelist()).permitAll()
.anyRequest().authenticated()
)🤖 Prompt for AI Agents
In src/main/java/konkuk/thip/config/SecurityConfig.java around lines 42 to 50,
the WHITELIST array includes test endpoints that should not be accessible in
production. Refactor the code to conditionally include these test URLs only when
the application is running in a non-production profile, such as 'dev' or 'test'.
Use Spring's profile-based configuration or conditional logic to exclude these
test endpoints from the whitelist in the production environment.
| CorsConfiguration config = new CorsConfiguration(); | ||
| config.setAllowedOrigins(Collections.singletonList("*")); // 배포 시 도메인 명시 | ||
| config.setAllowedMethods(List.of("GET", "POST", "PUT", "DELETE", "PATCH", "OPTIONS")); | ||
| config.setAllowedHeaders(Collections.singletonList("*")); | ||
| config.setAllowCredentials(true); | ||
| config.setMaxAge(3600L); |
There was a problem hiding this comment.
CORS 설정에 보안 문제가 있습니다.
setAllowedOrigins("*")와 setAllowCredentials(true)는 함께 사용할 수 없습니다. 이는 보안상 위험합니다.
다음 중 하나를 선택하세요:
옵션 1 (권장): 특정 도메인만 허용
-config.setAllowedOrigins(Collections.singletonList("*"));
+config.setAllowedOrigins(List.of("http://localhost:3000", "https://yourdomain.com"));
config.setAllowedMethods(List.of("GET", "POST", "PUT", "DELETE", "PATCH", "OPTIONS"));
config.setAllowedHeaders(Collections.singletonList("*"));
config.setAllowCredentials(true);옵션 2: 모든 origin 허용하되 credentials 비활성화
-config.setAllowedOrigins(Collections.singletonList("*"));
+config.setAllowedOriginPatterns(Collections.singletonList("*"));
config.setAllowedMethods(List.of("GET", "POST", "PUT", "DELETE", "PATCH", "OPTIONS"));
config.setAllowedHeaders(Collections.singletonList("*"));
-config.setAllowCredentials(true);
+config.setAllowCredentials(false);📝 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.
| CorsConfiguration config = new CorsConfiguration(); | |
| config.setAllowedOrigins(Collections.singletonList("*")); // 배포 시 도메인 명시 | |
| config.setAllowedMethods(List.of("GET", "POST", "PUT", "DELETE", "PATCH", "OPTIONS")); | |
| config.setAllowedHeaders(Collections.singletonList("*")); | |
| config.setAllowCredentials(true); | |
| config.setMaxAge(3600L); | |
| CorsConfiguration config = new CorsConfiguration(); | |
| config.setAllowedOrigins(List.of("http://localhost:3000", "https://yourdomain.com")); | |
| config.setAllowedMethods(List.of("GET", "POST", "PUT", "DELETE", "PATCH", "OPTIONS")); | |
| config.setAllowedHeaders(Collections.singletonList("*")); | |
| config.setAllowCredentials(true); | |
| config.setMaxAge(3600L); |
| CorsConfiguration config = new CorsConfiguration(); | |
| config.setAllowedOrigins(Collections.singletonList("*")); // 배포 시 도메인 명시 | |
| config.setAllowedMethods(List.of("GET", "POST", "PUT", "DELETE", "PATCH", "OPTIONS")); | |
| config.setAllowedHeaders(Collections.singletonList("*")); | |
| config.setAllowCredentials(true); | |
| config.setMaxAge(3600L); | |
| CorsConfiguration config = new CorsConfiguration(); | |
| config.setAllowedOriginPatterns(Collections.singletonList("*")); | |
| config.setAllowedMethods(List.of("GET", "POST", "PUT", "DELETE", "PATCH", "OPTIONS")); | |
| config.setAllowedHeaders(Collections.singletonList("*")); | |
| config.setAllowCredentials(false); | |
| config.setMaxAge(3600L); |
🤖 Prompt for AI Agents
In src/main/java/konkuk/thip/config/SecurityConfig.java around lines 91 to 96,
the CORS configuration uses setAllowedOrigins("*") together with
setAllowCredentials(true), which is a security risk. To fix this, either specify
a list of allowed origins explicitly instead of "*", or if you want to allow all
origins, setAllowCredentials(false) to disable credentials. Choose one of these
options and update the code accordingly.
seongjunnoh
left a comment
seongjunnoh
left a comment
There was a problem hiding this comment.
고생하셨습니다! oauth2 + spring security 의 조합은 처음이어서 저도 공부하면서 재미있게 리뷰할 수 있었습니다! 리뷰 단 부분 확인 부탁드립니다!
| public enum AuthParameters { | ||
| JWT_HEADER_KEY("Authorization"), | ||
| JWT_PREFIX("Bearer "), | ||
| KAKAO("kakao"), | ||
| GOOGLE("google"), | ||
| KAKAO_PROVIDER_ID_KEY("id"), | ||
| GOOGLE_PROVIDER_ID_KEY("sub"), | ||
| JWT_ACCESS_TOKEN_KEY("userId"), | ||
| JWT_SIGNUP_TOKEN_KEY("oauth2Id"), | ||
| ; |
There was a problem hiding this comment.
LGTM! authorization 관련 string value 들을 enum으로 모아두니 관리하기 편할 것 같습니다!
| public static LoginUser createNewUser(String oauth2Id) { | ||
| return new LoginUser(oauth2Id, null, true); | ||
| } | ||
|
|
||
| public static LoginUser createExistingUser(String oauth2Id, Long userId) { | ||
| return new LoginUser(oauth2Id, userId, false); | ||
| } |
| .csrf(AbstractHttpConfigurer::disable) | ||
| .formLogin(AbstractHttpConfigurer::disable) | ||
| .httpBasic(AbstractHttpConfigurer::disable) | ||
| .addFilterBefore(new JwtAuthenticationFilter(jwtUtil), UsernamePasswordAuthenticationFilter.class) |
There was a problem hiding this comment.
p3 : SecurityConfig가 JwtAuthenticationFilter 를 private attribute로 가지도록 하고, addFilterBefore 메서드에서 해당 인스턴스 변수를 파라미터로 주입하는건 어떤가요?
SecurityConfig 가 굳이 JwtUtil 과의 의존성을 가질 필요 없이 이미 @component를 통해 빈으로 등록된 JwtAuthenticationFilter 를 SecurityConfig 에서 사용하는 구조를 말씀드린 겁니다!
이렇게 수정하면 JwtAuthenticationFilter 만이 JwtUtil 과의 의존성을 맺고 있으므로, JWT관련 모든 책임들을 JwtAuthenticationFilter 가 담당하도록 강제할 수 있는 효과가 있을 것 같습니다!
There was a problem hiding this comment.
오호 너무 좋은 리뷰네요!! 굿굿 👍🏻
src/main/java/konkuk/thip/user/adapter/out/persistence/UserJpaRepository.java
Outdated
Show resolved
Hide resolved
| //when : 회원가입 api 호출 + 임시 토큰 발급 | ||
| String testToken = jwtUtil.createSignupToken("kakao_12345678"); | ||
| ResultActions result = mockMvc.perform(post("/users/signup") | ||
| .header("Authorization", "Bearer " + testToken) //헤더 추가 |
hd0rable
left a comment
hd0rable
left a comment
There was a problem hiding this comment.
수고하셨어요🤩🤩 시큐리티내용은 언제나봐도 어질어질하네요,,
| import lombok.Getter; | ||
|
|
||
| @Getter | ||
| public class AuthException extends RuntimeException { |
| import lombok.Getter; | ||
|
|
||
| @Getter | ||
| public enum AuthParameters { |
src/main/java/konkuk/thip/common/security/oauth2/CustomOAuth2UserService.java
Show resolved
Hide resolved
| } | ||
|
|
||
| String oauth2Id = oAuth2UserDetails.getProvider() + "_" + oAuth2UserDetails.getProviderId(); //kakao_1234567890 | ||
| UserJpaEntity existingUser = userJpaRepository.findByOauth2Id(oauth2Id); |
There was a problem hiding this comment.
p2: null로반환하지않고, optional로 안전하게 반환하는게 낫지않을까요?
|
|
||
| private final SecretKey secretKey; | ||
|
|
||
| //todo 확정 후 환경변수로 변경 |
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (10)
src/main/java/konkuk/thip/book/application/port/out/SearchBookQueryPort.java (1)
5-7: 도메인 모델 추상화 개선을 고려해보세요.현재 인터페이스가
NaverBookParseResult라는 구체적인 API 응답 모델에 의존하고 있습니다. 포트 인터페이스는 외부 시스템에 독립적인 도메인 모델을 사용하는 것이 헥사고날 아키텍처의 원칙에 더 부합합니다.-import konkuk.thip.book.adapter.out.api.dto.NaverBookParseResult; +import konkuk.thip.book.domain.BookSearchResult; public interface SearchBookQueryPort { - NaverBookParseResult findBooksByKeyword(String keyword, int start); + BookSearchResult findBooksByKeyword(String keyword, int start); }src/main/java/konkuk/thip/book/application/service/BookSearchService.java (2)
30-30: TODO 주석을 처리해 주세요.최근 검색어 로직 추가가 계획되어 있습니다. 향후 구현 일정을 명확히 하거나 이슈로 등록하는 것을 권장합니다.
최근 검색어 기능 구현에 도움이 필요하시면 별도 이슈로 등록해 드릴까요?
37-37: 코딩 스타일 일관성을 개선해 주세요.
!=연산자 앞뒤에 공백을 추가하여 가독성을 향상시켜 주세요.- if ( totalElements!=0 && page > totalPages) { + if (totalElements != 0 && page > totalPages) {src/main/java/konkuk/thip/book/adapter/out/api/NaverBookXmlParser.java (2)
16-16: 변수명 네이밍 컨벤션을 수정해 주세요.Java 네이밍 컨벤션에 따라 변수명은 camelCase로 작성해야 합니다.
- List<NaverBookParseResult.NaverBook> Naverbooks = new ArrayList<>(); + List<NaverBookParseResult.NaverBook> naverBooks = new ArrayList<>();그리고 46번 라인의
Naverbooks.add(naverBook);도naverBooks.add(naverBook);로 수정해 주세요.
49-51: 예외 처리 범위를 구체화하는 것을 고려해 주세요.현재 모든 예외를 catch하고 있어 구체적인 파싱 오류 정보가 손실될 수 있습니다. 필요에 따라 로깅을 추가하거나 더 구체적인 예외 처리를 고려해 보세요.
} catch (Exception e) { + // 로깅 추가 고려 + log.warn("XML 파싱 중 오류 발생: {}", e.getMessage(), e); throw new BusinessException(ErrorCode.BOOK_NAVER_API_PARSING_ERROR); }src/main/java/konkuk/thip/book/adapter/out/api/NaverApiUtil.java (1)
47-47: 변수명을 더 의미있게 작성해 주세요.
text변수명이 용도를 명확히 나타내지 못합니다.- String text = null; + String encodedKeyword = null; try { - text = URLEncoder.encode(keyword, "UTF-8"); + encodedKeyword = URLEncoder.encode(keyword, "UTF-8"); } catch (UnsupportedEncodingException e) { throw new BusinessException(BOOK_KEYWORD_ENCODING_FAILED); } - return text; + return encodedKeyword;src/main/java/konkuk/thip/book/adapter/in/web/response/GetBookSearchListResponse.java (2)
10-19: 레코드 구조가 잘 설계되었습니다.페이지네이션에 필요한 모든 정보가 포함되어 있고, 필드명이 명확합니다. 하지만
@Builder어노테이션이 레코드에 필요한지 검토해보세요. 레코드는 이미 모든 필드를 받는 생성자가 있어서 빌더 패턴이 불필요할 수 있습니다.-@Builder public record GetBookSearchListResponse(
10-19: record에서 @builder 어노테이션 사용을 재검토하세요.record는 이미 모든 필드를 받는 canonical constructor를 제공하므로 @builder 어노테이션이 불필요할 수 있습니다. record의 간결성을 유지하기 위해 @builder 제거를 고려해보세요.
-@Builder public record GetBookSearchListResponse(src/main/java/konkuk/thip/common/exception/code/ErrorCode.java (2)
37-37: 오타 수정이 필요합니다.오류 메시지에서 쉼표와 따옴표 사이에 공백이 누락되었습니다.
- BOOK_NAVER_API_REQUEST_ERROR(HttpStatus.BAD_REQUEST, 80001,"네이버 API 요청에 실패하였습니다."), + BOOK_NAVER_API_REQUEST_ERROR(HttpStatus.BAD_REQUEST, 80001, "네이버 API 요청에 실패하였습니다."),
36-44: 도서 검색 관련 에러 코드 설계를 개선할 수 있습니다.일부 에러의 HTTP 상태 코드가 부적절할 수 있습니다. 네이버 API 연결 실패나 응답 오류는 서버 측 문제이므로 5xx 상태 코드가 더 적절할 수 있습니다.
다음과 같이 수정을 고려해보세요:
-BOOK_NAVER_API_URL_HTTP_CONNECT_FAILED(HttpStatus.BAD_REQUEST, 80004,"네이버 API 요청 중, HTTP 연결에 실패하였습니다."), -BOOK_NAVER_API_RESPONSE_ERROR(HttpStatus.BAD_REQUEST, 80005,"네이버 API 응답에 실패하였습니다."), +BOOK_NAVER_API_URL_HTTP_CONNECT_FAILED(HttpStatus.BAD_GATEWAY, 80004,"네이버 API 요청 중, HTTP 연결에 실패하였습니다."), +BOOK_NAVER_API_RESPONSE_ERROR(HttpStatus.BAD_GATEWAY, 80005,"네이버 API 응답에 실패하였습니다."),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (24)
src/main/java/konkuk/thip/book/adapter/in/web/BookQueryController.java(1 hunks)src/main/java/konkuk/thip/book/adapter/in/web/request/DummyRequest.java(0 hunks)src/main/java/konkuk/thip/book/adapter/in/web/response/GetBookSearchListResponse.java(1 hunks)src/main/java/konkuk/thip/book/adapter/out/api/BookApiAdapter.java(1 hunks)src/main/java/konkuk/thip/book/adapter/out/api/NaverApiUtil.java(1 hunks)src/main/java/konkuk/thip/book/adapter/out/api/NaverBookXmlParser.java(1 hunks)src/main/java/konkuk/thip/book/adapter/out/api/dto/NaverBookParseResult.java(1 hunks)src/main/java/konkuk/thip/book/application/port/in/BookSearchUseCase.java(1 hunks)src/main/java/konkuk/thip/book/application/port/in/DummyUseCase.java(0 hunks)src/main/java/konkuk/thip/book/application/port/out/SearchBookQueryPort.java(1 hunks)src/main/java/konkuk/thip/book/application/service/BookSearchService.java(1 hunks)src/main/java/konkuk/thip/book/application/service/BookService.java(0 hunks)src/main/java/konkuk/thip/common/dto/ErrorResponse.java(1 hunks)src/main/java/konkuk/thip/common/exception/BusinessException.java(1 hunks)src/main/java/konkuk/thip/common/exception/code/ErrorCode.java(1 hunks)src/main/java/konkuk/thip/common/security/argument_resolver/Oauth2IdArgumentResolver.java(1 hunks)src/main/java/konkuk/thip/common/security/argument_resolver/UserIdArgumentResolver.java(1 hunks)src/main/java/konkuk/thip/common/security/oauth2/CustomOAuth2UserService.java(1 hunks)src/main/java/konkuk/thip/common/security/oauth2/GoogleUserDetails.java(1 hunks)src/main/java/konkuk/thip/common/security/oauth2/KakaoUserDetails.java(1 hunks)src/main/java/konkuk/thip/config/SecurityConfig.java(1 hunks)src/main/java/konkuk/thip/user/adapter/out/persistence/UserJpaRepository.java(1 hunks)src/test/java/konkuk/thip/book/adapter/in/web/BookQueryControllerTest.java(1 hunks)src/test/java/konkuk/thip/book/adapter/out/api/NaverApiUtilTest.java(1 hunks)
💤 Files with no reviewable changes (3)
- src/main/java/konkuk/thip/book/application/service/BookService.java
- src/main/java/konkuk/thip/book/application/port/in/DummyUseCase.java
- src/main/java/konkuk/thip/book/adapter/in/web/request/DummyRequest.java
✅ Files skipped from review due to trivial changes (1)
- src/main/java/konkuk/thip/book/application/port/in/BookSearchUseCase.java
🚧 Files skipped from review as they are similar to previous changes (7)
- src/main/java/konkuk/thip/user/adapter/out/persistence/UserJpaRepository.java
- src/main/java/konkuk/thip/common/security/argument_resolver/UserIdArgumentResolver.java
- src/main/java/konkuk/thip/common/security/oauth2/GoogleUserDetails.java
- src/main/java/konkuk/thip/common/security/argument_resolver/Oauth2IdArgumentResolver.java
- src/main/java/konkuk/thip/config/SecurityConfig.java
- src/main/java/konkuk/thip/common/security/oauth2/KakaoUserDetails.java
- src/main/java/konkuk/thip/common/security/oauth2/CustomOAuth2UserService.java
🧰 Additional context used
🧬 Code Graph Analysis (2)
src/main/java/konkuk/thip/book/adapter/in/web/BookQueryController.java (1)
src/main/java/konkuk/thip/book/adapter/out/api/NaverApiUtil.java (1)
RequiredArgsConstructor(18-108)
src/main/java/konkuk/thip/book/adapter/out/api/BookApiAdapter.java (2)
src/main/java/konkuk/thip/book/adapter/out/api/NaverApiUtil.java (1)
RequiredArgsConstructor(18-108)src/main/java/konkuk/thip/book/adapter/out/api/NaverBookXmlParser.java (1)
NaverBookXmlParser(13-63)
🔇 Additional comments (23)
src/main/java/konkuk/thip/common/dto/ErrorResponse.java (1)
11-11: 중요한 JSON 속성명 버그 수정 완료JSON 속성명에서 불필요한 콜론을 제거한 것은 올바른 수정입니다. 이전의
"isSuccess:"는 잘못된 JSON 속성명을 생성하여 API 응답 형식에 문제를 일으킬 수 있었습니다.src/main/java/konkuk/thip/common/exception/BusinessException.java (1)
10-18: 예외 메시지 전파 개선이 잘 되었습니다.
ErrorCode의 메시지를 상위 클래스로 전달하여 예외 메시지가 일관되게 표시되도록 개선된 점이 좋습니다. 로깅과 디버깅 시 더 명확한 오류 정보를 제공할 수 있습니다.src/main/java/konkuk/thip/book/adapter/out/api/BookApiAdapter.java (1)
8-19: 어댑터 구현이 깔끔하게 잘 되었습니다.관심사가 잘 분리되어 있고 어댑터 패턴을 적절히 구현했습니다:
- API 호출 로직은
NaverApiUtil에 위임- XML 파싱 로직은
NaverBookXmlParser에 위임- 어댑터는 단순히 두 컴포넌트를 연결하는 역할만 수행
NaverApiUtil과NaverBookXmlParser에서 이미 적절한 예외 처리가 되어 있어 안전합니다.src/test/java/konkuk/thip/book/adapter/out/api/NaverApiUtilTest.java (1)
36-69: 테스트 케이스 구성이 적절합니다.성공 케이스와 예외 케이스를 모두 다루고 있어 좋습니다. 테스트 메서드명도 명확하고 한국어로 의도가 잘 드러나 있습니다. AssertJ를 활용한 검증도 적절합니다.
src/main/java/konkuk/thip/book/application/service/BookSearchService.java (1)
22-24: 입력 유효성 검사가 잘 구현되었습니다.키워드가 null이거나 빈 문자열인 경우를 적절히 처리하고 있습니다.
src/main/java/konkuk/thip/book/adapter/out/api/dto/NaverBookParseResult.java (1)
8-23: 잘 설계된 데이터 구조입니다.레코드 클래스를 적절히 활용하여 불변 객체로 설계되었고, Builder 패턴과 정적 팩토리 메서드를 제공하여 객체 생성의 편의성을 높였습니다.
src/test/java/konkuk/thip/book/adapter/in/web/BookQueryControllerTest.java (1)
26-77: 포괄적인 테스트 커버리지를 제공합니다.성공 시나리오와 다양한 실패 시나리오(키워드 누락, 잘못된 페이지 번호, 페이지 범위 초과)를 모두 테스트하고 있어 API의 견고성을 보장합니다.
src/main/java/konkuk/thip/book/adapter/in/web/response/GetBookSearchListResponse.java (9)
26-28: 스트림 처리가 효율적입니다.
toList()메서드 사용으로 간결하고 명확한 변환 로직을 구현했습니다.
40-57: BookDto 레코드 구조가 적절합니다.네이버 API 응답을 클라이언트 응답으로 변환하는 매핑이 명확합니다. 필드명도 직관적이고 책 정보를 나타내기에 충분합니다.
20-39: PAGE_SIZE 고정값(10)으로 0으로 나누기 오류 없음
NaverApiUtil.PAGE_SIZE가public static final int PAGE_SIZE = 10으로 정의되어 있어 0이 될 수 없습니다. 따라서 나누기 0 예외를 방지하기 위한 추가 검증은 필요하지 않습니다.
또한 빈 결과(totalElements == 0) 시totalPages = 0,first = last = true가 되는데, 이 동작이 의도된 것이라면 별도 수정 없이도 괜찮습니다.
22-23: 페이지네이션 로직이 올바르게 구현되었습니다.totalPages 계산과 last 페이지 판단 로직이 정확합니다. Math.ceil을 사용한 페이지 수 계산이 적절합니다.
26-28: 스트림 처리가 효율적으로 구현되었습니다.NaverBook을 BookDto로 변환하는 로직이 깔끔하게 구현되었습니다.
40-56: BookDto 중첩 레코드 설계가 적절합니다.정적 팩토리 메서드를 통한 객체 생성 패턴이 일관성 있게 적용되었습니다.
26-28: 스트림 처리 로직이 올바르게 구현되었습니다.NaverBook을 BookDto로 변환하는 스트림 처리가 깔끔하게 구현되어 있습니다.
40-56: nested record 구현이 적절합니다.BookDto record와 factory method가 명확하고 일관성 있게 구현되어 있습니다. 필드 매핑도 올바릅니다.
20-24: totalPages 계산에서 잠재적 오버플로우를 방지하세요.
Math.ceil((double) totalElements / PAGE_SIZE)계산에서 매우 큰 숫자의 경우 정밀도 손실이나 오버플로우가 발생할 수 있습니다.- int totalPages = (int) Math.ceil((double) totalElements / PAGE_SIZE); + int totalPages = (totalElements + PAGE_SIZE - 1) / PAGE_SIZE; // 정수 연산으로 ceiling 계산Likely an incorrect or invalid review comment.
src/main/java/konkuk/thip/common/exception/code/ErrorCode.java (7)
19-25: 인증 관련 오류 코드가 잘 정의되었습니다.모든 인증 오류가 HTTP 401 상태 코드를 사용하여 일관성이 있고, 오류 메시지도 명확합니다. 오류 코드 번호 체계(40100번대)도 적절합니다.
36-44: 책 검색 관련 오류 코드가 체계적으로 정의되었습니다.네이버 API와의 연동에서 발생할 수 있는 다양한 오류 상황을 잘 커버하고 있습니다. 오류 코드 번호(80000번대)와 HTTP 400 상태 코드 사용도 적절합니다.
30-30: ALIAS_NOT_FOUND 오류 코드 변경 내역 재확인 요청원격 기본 브랜치와의 diff 확인이 실패하여,
ErrorCode.java에서ALIAS_NOT_FOUND가 실제로 변경되었는지 다시 한번 검토가 필요합니다. 기존 API 응답에 영향이 없는지 아래 명령어로 확인해 주세요.# 원격 브랜치 이름이 main이 아닐 경우 적절히 변경 git fetch origin git diff origin/main -- src/main/java/konkuk/thip/common/exception/code/ErrorCode.java만약 여전히 diff가 보이지 않는다면, 먼저 원격 브랜치 목록을 확인한 뒤 올바른 브랜치 이름으로 재실행 부탁드립니다.
19-24: 인증 관련 에러 코드가 적절하게 추가되었습니다.모든 인증 에러 코드가 HTTP 401 상태코드를 사용하며, 코드 번호 체계(40100번대)가 일관성 있게 적용되었습니다. 메시지도 명확하고 이해하기 쉽습니다.
30-30: ALIAS_NOT_FOUND enum 상수 위치 변경은 안전합니다.
- Java enum 상수는 선언 순서와 무관하게 이름으로 참조되므로, 위치 변경이 코드호환성에 영향을 주지 않습니다.
- 실제로
AliasCommandPersistenceAdapter와UserCommandPersistenceAdapter에서 정상적으로import static …ErrorCode.ALIAS_NOT_FOUND;후 사용되고 있습니다.
19-24: 인증 관련 에러 코드들이 적절하게 정의되었습니다.모든 인증 에러가 HTTP 401 상태 코드를 사용하고, 에러 코드 번호체계(40100-40105)도 일관성이 있습니다. 에러 메시지도 명확합니다.
33-35: 에러 코드 번호체계의 일관성을 확인하세요.60000번대에서 80000번대로 점프하는 것이 프로젝트의 에러 코드 체계와 일치하는지 확인해주세요. 중간 번호대(70000번대)가 다른 용도로 예약되어 있는지 검토가 필요합니다.
| @GetMapping("/books") | ||
| public BaseResponse<GetBookSearchListResponse> getBookSearchList(@RequestParam final String keyword, | ||
| @RequestParam final int page) { | ||
| NaverBookParseResult result = bookSearchUseCase.searchBooks(keyword, page); | ||
| return BaseResponse.ok(GetBookSearchListResponse.of(result, page)); | ||
| } | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
입력 검증 및 API 문서화 추가를 권장합니다.
현재 컨트롤러에 입력 검증과 API 문서화가 부족합니다. 다음 개선사항을 고려해보세요:
+import io.swagger.v3.oas.annotations.Operation;
+import io.swagger.v3.oas.annotations.Parameter;
+import javax.validation.constraints.Min;
+import javax.validation.constraints.NotBlank;
@GetMapping("/books")
+@Operation(summary = "도서 검색", description = "키워드로 도서를 검색합니다")
public BaseResponse<GetBookSearchListResponse> getBookSearchList(
- @RequestParam final String keyword,
- @RequestParam final int page) {
+ @Parameter(description = "검색 키워드") @RequestParam @NotBlank final String keyword,
+ @Parameter(description = "페이지 번호 (1부터 시작)") @RequestParam @Min(1) final int page) {🤖 Prompt for AI Agents
In src/main/java/konkuk/thip/book/adapter/in/web/BookQueryController.java around
lines 16 to 22, the getBookSearchList method lacks input validation and API
documentation. Add validation annotations to the method parameters to ensure
keyword is not empty and page is a positive integer. Also, include API
documentation annotations such as Swagger's @Operation and @Parameter to
describe the endpoint and its parameters for better clarity and maintainability.
| private NaverApiUtil createTestUtil() { | ||
| NaverApiUtil util = Mockito.spy(new NaverApiUtil()); | ||
| // @Value로 주입되는 필드를 직접 세팅 | ||
| try { | ||
| Field clientIdField = NaverApiUtil.class.getDeclaredField("clientId"); | ||
| clientIdField.setAccessible(true); | ||
| clientIdField.set(util, "dummy-client-id"); | ||
|
|
||
| Field clientSecretField = NaverApiUtil.class.getDeclaredField("clientSecret"); | ||
| clientSecretField.setAccessible(true); | ||
| clientSecretField.set(util, "dummy-client-secret"); | ||
|
|
||
| Field bookSearchUrlField = NaverApiUtil.class.getDeclaredField("bookSearchUrl"); | ||
| bookSearchUrlField.setAccessible(true); | ||
| bookSearchUrlField.set(util, "https://dummy-url.com/search?query="); | ||
| } catch (Exception e) { | ||
| throw new RuntimeException(e); | ||
| } | ||
| return util; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
테스트 설정 방식 개선을 권장합니다.
리플렉션을 사용한 필드 설정 방식은 취약하고 유지보수가 어렵습니다. 다음 대안들을 고려해보세요:
Option 1: 테스트 전용 생성자 추가
// NaverApiUtil에 패키지 가시성 생성자 추가
NaverApiUtil(String clientId, String clientSecret, String bookSearchUrl) {
this.clientId = clientId;
this.clientSecret = clientSecret;
this.bookSearchUrl = bookSearchUrl;
}Option 2: @TestConfiguration 사용
@TestConfiguration
static class TestConfig {
@Bean
@Primary
NaverApiUtil testNaverApiUtil() {
return new NaverApiUtil("test-id", "test-secret", "test-url");
}
}🤖 Prompt for AI Agents
In src/test/java/konkuk/thip/book/adapter/out/api/NaverApiUtilTest.java around
lines 15 to 34, the test setup uses reflection to set private fields, which is
fragile and hard to maintain. Instead, add a package-private constructor in
NaverApiUtil that accepts clientId, clientSecret, and bookSearchUrl as
parameters and use it in the test to instantiate the object with test values.
Alternatively, create a @TestConfiguration class with a @Bean method that
returns a preconfigured NaverApiUtil instance for testing, avoiding reflection
altogether.
|
|
||
|
|
||
| private String readBody(InputStream body){ | ||
| InputStreamReader streamReader = new InputStreamReader(body); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
문자 인코딩을 명시적으로 지정해 주세요.
InputStreamReader에 문자 인코딩을 명시하지 않으면 플랫폼 기본 인코딩을 사용하여 예상치 못한 문자 깨짐이 발생할 수 있습니다.
- InputStreamReader streamReader = new InputStreamReader(body);
+ InputStreamReader streamReader = new InputStreamReader(body, StandardCharsets.UTF_8);그리고 파일 상단에 import를 추가해 주세요:
import java.nio.charset.StandardCharsets;🤖 Prompt for AI Agents
In src/main/java/konkuk/thip/book/adapter/out/api/NaverApiUtil.java at line 92,
the InputStreamReader is created without specifying a character encoding, which
can cause character corruption due to platform default encoding. Fix this by
explicitly specifying StandardCharsets.UTF_8 when constructing the
InputStreamReader. Also, add the import statement 'import
java.nio.charset.StandardCharsets;' at the top of the file.
| } else { // 오류 발생 | ||
| return readBody(con.getErrorStream()); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
에러 응답 처리를 개선해 주세요.
HTTP 오류 상황에서도 응답 본문을 반환하고 있는데, 이는 예상치 못한 동작을 야기할 수 있습니다. 오류 상황에서는 적절한 예외를 발생시키는 것이 좋습니다.
int responseCode = con.getResponseCode();
if (responseCode == HttpURLConnection.HTTP_OK) { // 정상 호출
return readBody(con.getInputStream());
} else { // 오류 발생
- return readBody(con.getErrorStream());
+ String errorResponse = readBody(con.getErrorStream());
+ throw new BusinessException(BOOK_NAVER_API_REQUEST_ERROR);
}📝 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.
| } else { // 오류 발생 | |
| return readBody(con.getErrorStream()); | |
| } | |
| int responseCode = con.getResponseCode(); | |
| if (responseCode == HttpURLConnection.HTTP_OK) { // 정상 호출 | |
| return readBody(con.getInputStream()); | |
| } else { // 오류 발생 | |
| String errorResponse = readBody(con.getErrorStream()); | |
| throw new BusinessException(BOOK_NAVER_API_REQUEST_ERROR); | |
| } |
🤖 Prompt for AI Agents
In src/main/java/konkuk/thip/book/adapter/out/api/NaverApiUtil.java around lines
68 to 70, the code currently returns the error response body directly when an
HTTP error occurs, which can cause unexpected behavior. Instead of returning the
error stream, modify the code to throw a meaningful exception that includes the
error details from the error stream. This will ensure proper error handling and
clearer failure signals to the caller.
| public static GetBookSearchListResponse of(NaverBookParseResult result, int page) { | ||
| int totalElements = result.total(); | ||
| int totalPages = (int) Math.ceil((double) totalElements / PAGE_SIZE); | ||
| boolean last = (page >= totalPages); | ||
| boolean first = (page == 1); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
타입 안전성과 null 체크 개선이 필요합니다.
result.total() 반환 타입이 int가 아닐 경우 캐스팅 오류가 발생할 수 있으며, result 매개변수에 대한 null 체크가 없습니다.
다음과 같이 개선할 수 있습니다:
public static GetBookSearchListResponse of(NaverBookParseResult result, int page) {
+ if (result == null) {
+ throw new IllegalArgumentException("NaverBookParseResult cannot be null");
+ }
+ if (page < 1) {
+ throw new IllegalArgumentException("Page number must be greater than 0");
+ }
int totalElements = result.total();🤖 Prompt for AI Agents
In
src/main/java/konkuk/thip/book/adapter/in/web/response/GetBookSearchListResponse.java
around lines 20 to 24, improve type safety and add null checks for the result
parameter. Ensure that result is not null before accessing its total method, and
verify or safely convert the return type of result.total() to int to avoid
casting errors. Add appropriate null checks and handle potential type mismatches
to prevent runtime exceptions.
| BOOK_KEYWORD_ENCODING_FAILED(HttpStatus.BAD_REQUEST, 80000, "검색어 인코딩에 실패했습니다."), | ||
| BOOK_NAVER_API_REQUEST_ERROR(HttpStatus.BAD_REQUEST, 80001,"네이버 API 요청에 실패하였습니다."), | ||
| BOOK_NAVER_API_PARSING_ERROR(HttpStatus.BAD_REQUEST, 80002,"네이버 API 응답 파싱에 실패하였습니다."), | ||
| BOOK_NAVER_API_URL_ERROR(HttpStatus.BAD_REQUEST, 80003,"네이버 API URL이 잘못되었습니다."), | ||
| BOOK_NAVER_API_URL_HTTP_CONNECT_FAILED(HttpStatus.BAD_REQUEST, 80004,"네이버 API 요청 중, HTTP 연결에 실패하였습니다."), | ||
| BOOK_NAVER_API_RESPONSE_ERROR(HttpStatus.BAD_REQUEST, 80005,"네이버 API 응답에 실패하였습니다."), | ||
| BOOK_SEARCH_PAGE_OUT_OF_RANGE(HttpStatus.BAD_REQUEST, 80006,"검색어 페이지가 범위를 벗어났습니다."), | ||
| BOOK_KEYWORD_REQUIRED(HttpStatus.BAD_REQUEST, 80007, "검색어는 필수 입력값입니다."), | ||
| BOOK_PAGE_NUMBER_INVALID(HttpStatus.BAD_REQUEST, 80008, "페이지 번호는 1 이상의 값이어야 합니다."); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
일부 책 관련 에러의 HTTP 상태 코드를 재검토하세요.
네이버 API 연결 실패나 응답 에러의 경우 클라이언트 요청 문제가 아닌 서버 측 문제일 수 있으므로 HTTP 500 상태 코드가 더 적절할 수 있습니다.
- BOOK_NAVER_API_REQUEST_ERROR(HttpStatus.BAD_REQUEST, 80001,"네이버 API 요청에 실패하였습니다."),
+ BOOK_NAVER_API_REQUEST_ERROR(HttpStatus.INTERNAL_SERVER_ERROR, 80001,"네이버 API 요청에 실패하였습니다."),
- BOOK_NAVER_API_URL_HTTP_CONNECT_FAILED(HttpStatus.BAD_REQUEST, 80004,"네이버 API 요청 중, HTTP 연결에 실패하였습니다."),
+ BOOK_NAVER_API_URL_HTTP_CONNECT_FAILED(HttpStatus.INTERNAL_SERVER_ERROR, 80004,"네이버 API 요청 중, HTTP 연결에 실패하였습니다."),
- BOOK_NAVER_API_RESPONSE_ERROR(HttpStatus.BAD_REQUEST, 80005,"네이버 API 응답에 실패하였습니다."),
+ BOOK_NAVER_API_RESPONSE_ERROR(HttpStatus.INTERNAL_SERVER_ERROR, 80005,"네이버 API 응답에 실패하였습니다."),📝 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.
| BOOK_KEYWORD_ENCODING_FAILED(HttpStatus.BAD_REQUEST, 80000, "검색어 인코딩에 실패했습니다."), | |
| BOOK_NAVER_API_REQUEST_ERROR(HttpStatus.BAD_REQUEST, 80001,"네이버 API 요청에 실패하였습니다."), | |
| BOOK_NAVER_API_PARSING_ERROR(HttpStatus.BAD_REQUEST, 80002,"네이버 API 응답 파싱에 실패하였습니다."), | |
| BOOK_NAVER_API_URL_ERROR(HttpStatus.BAD_REQUEST, 80003,"네이버 API URL이 잘못되었습니다."), | |
| BOOK_NAVER_API_URL_HTTP_CONNECT_FAILED(HttpStatus.BAD_REQUEST, 80004,"네이버 API 요청 중, HTTP 연결에 실패하였습니다."), | |
| BOOK_NAVER_API_RESPONSE_ERROR(HttpStatus.BAD_REQUEST, 80005,"네이버 API 응답에 실패하였습니다."), | |
| BOOK_SEARCH_PAGE_OUT_OF_RANGE(HttpStatus.BAD_REQUEST, 80006,"검색어 페이지가 범위를 벗어났습니다."), | |
| BOOK_KEYWORD_REQUIRED(HttpStatus.BAD_REQUEST, 80007, "검색어는 필수 입력값입니다."), | |
| BOOK_PAGE_NUMBER_INVALID(HttpStatus.BAD_REQUEST, 80008, "페이지 번호는 1 이상의 값이어야 합니다."); | |
| BOOK_KEYWORD_ENCODING_FAILED(HttpStatus.BAD_REQUEST, 80000, "검색어 인코딩에 실패했습니다."), | |
| BOOK_NAVER_API_REQUEST_ERROR(HttpStatus.INTERNAL_SERVER_ERROR, 80001,"네이버 API 요청에 실패하였습니다."), | |
| BOOK_NAVER_API_PARSING_ERROR(HttpStatus.BAD_REQUEST, 80002,"네이버 API 응답 파싱에 실패하였습니다."), | |
| BOOK_NAVER_API_URL_ERROR(HttpStatus.BAD_REQUEST, 80003,"네이버 API URL이 잘못되었습니다."), | |
| BOOK_NAVER_API_URL_HTTP_CONNECT_FAILED(HttpStatus.INTERNAL_SERVER_ERROR, 80004,"네이버 API 요청 중, HTTP 연결에 실패하였습니다."), | |
| BOOK_NAVER_API_RESPONSE_ERROR(HttpStatus.INTERNAL_SERVER_ERROR, 80005,"네이버 API 응답에 실패하였습니다."), | |
| BOOK_SEARCH_PAGE_OUT_OF_RANGE(HttpStatus.BAD_REQUEST, 80006,"검색어 페이지가 범위를 벗어났습니다."), | |
| BOOK_KEYWORD_REQUIRED(HttpStatus.BAD_REQUEST, 80007, "검색어는 필수 입력값입니다."), | |
| BOOK_PAGE_NUMBER_INVALID(HttpStatus.BAD_REQUEST, 80008, "페이지 번호는 1 이상의 값이어야 합니다."); |
🤖 Prompt for AI Agents
In src/main/java/konkuk/thip/common/exception/code/ErrorCode.java around lines
36 to 44, some book-related error codes use HttpStatus.BAD_REQUEST (400) for
issues like Naver API connection failure or response errors, which are likely
server-side problems. Change the HTTP status codes for
BOOK_NAVER_API_REQUEST_ERROR, BOOK_NAVER_API_PARSING_ERROR,
BOOK_NAVER_API_URL_HTTP_CONNECT_FAILED, and BOOK_NAVER_API_RESPONSE_ERROR from
HttpStatus.BAD_REQUEST to HttpStatus.INTERNAL_SERVER_ERROR (500) to better
reflect server-side errors.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (9)
src/main/java/konkuk/thip/user/adapter/out/persistence/UserQueryRepositoryImpl.java (1)
5-11: QueryDSL 기반 리포지토리 구조가 적절히 준비되었습니다.향후 복잡한 조회 로직 구현을 위한 기반이 잘 마련되었습니다. 다른 QueryDSL 구현체들과 일관된 패턴을 따르고 있습니다.
향후 QueryDSL 쿼리 구현 시 JPAQueryFactory 의존성 주입을 미리 준비하는 것을 고려해보세요:
+import com.querydsl.jpa.impl.JPAQueryFactory; +import lombok.RequiredArgsConstructor; + @Repository +@RequiredArgsConstructor public class UserQueryRepositoryImpl implements UserQueryRepository { + + private final JPAQueryFactory jpaQueryFactory; /** * QueryDsl 을 활용한 복잡한 조회 로직 구현 */ }src/main/java/konkuk/thip/user/adapter/out/persistence/AliasQueryRepositoryImpl.java (1)
20-40: 빈 결과에 대한 처리를 고려해보세요.현재 구현은 정상적이지만, 데이터가 없을 때의 동작을 명시적으로 문서화하거나 로깅을 추가하는 것을 고려해볼 수 있습니다.
선택적으로 다음과 같은 개선을 고려할 수 있습니다:
@Override public UserViewAliasChoiceResult getAllAliasesAndCategories() { QAliasJpaEntity alias = QAliasJpaEntity.aliasJpaEntity; QCategoryJpaEntity category = QCategoryJpaEntity.categoryJpaEntity; List<UserViewAliasChoiceResult.AliasChoice> aliasChoices = jpaQueryFactory .select(Projections.constructor( UserViewAliasChoiceResult.AliasChoice.class, alias.aliasId, alias.value, category.value, alias.imageUrl, alias.color )) .from(alias) .leftJoin(category) .on(category.aliasForCategoryJpaEntity.eq(alias)) .orderBy(alias.aliasId.asc()) .fetch(); + // 로깅이나 빈 결과 처리 로직 추가 고려 return new UserViewAliasChoiceResult(aliasChoices); }src/test/java/konkuk/thip/user/adapter/in/web/UserVerifyNicknameControllerTest.java (4)
67-71: JSON 응답 파싱 로직 개선 제안응답을 파싱하기 위해
JsonNode를 직접 사용하고 있습니다. 더 간결하고 타입 안전한 방법으로 개선할 수 있습니다.다음과 같이 리팩토링을 제안합니다:
- String json = result.andReturn().getResponse().getContentAsString(); - JsonNode jsonNode = objectMapper.readTree(json); - boolean isVerified = jsonNode.path("data").path("isVerified").asBoolean(); - - assertThat(isVerified).isTrue(); + result.andExpect(jsonPath("$.data.isVerified").value(true));
78-92: 테스트 데이터 설정 로직 중복사용자 엔티티 생성 로직이 반복되고 있습니다. 테스트 헬퍼 메서드로 추출하여 중복을 제거할 수 있습니다.
다음과 같이 헬퍼 메서드를 추가하는 것을 제안합니다:
+ private UserJpaEntity createTestUser(String nickname, String oauth2Id) { + AliasJpaEntity aliasJpaEntity = AliasJpaEntity.builder() + .value("칭호") + .color("blue") + .imageUrl("http://image.url") + .build(); + aliasJpaRepository.save(aliasJpaEntity); + + UserJpaEntity userJpaEntity = UserJpaEntity.builder() + .nickname(nickname) + .imageUrl("http://image.url") + .role(USER) + .oauth2Id(oauth2Id) + .aliasForUserJpaEntity(aliasJpaEntity) + .build(); + return userJpaRepository.save(userJpaEntity); + }
105-109: JSON 응답 파싱 로직 일관성앞선 테스트와 동일한 JSON 파싱 패턴이 반복되고 있습니다. 일관성을 위해 동일한 개선 방법을 적용하는 것을 권장합니다.
앞서 제안한 것과 동일하게
jsonPath매처를 사용하여 개선할 수 있습니다:- String json = result.andReturn().getResponse().getContentAsString(); - JsonNode jsonNode = objectMapper.readTree(json); - boolean isVerified = jsonNode.path("data").path("isVerified").asBoolean(); - - assertThat(isVerified).isFalse(); + result.andExpect(jsonPath("$.data.isVerified").value(false));
124-124: 에러 메시지 검증 개선에러 메시지를
containsString으로 부분 검증하고 있습니다. 더 정확한 검증을 위해 전체 메시지를 확인하는 것을 고려해보세요.- .andExpect(jsonPath("$.message", containsString("닉네임은 한글, 영어, 숫자로만 구성되어야 합니다.(공백불가)"))); + .andExpect(jsonPath("$.message").value("닉네임은 한글, 영어, 숫자로만 구성되어야 합니다.(공백불가)"));src/test/java/konkuk/thip/user/adapter/in/web/UserViewAliasChoiceControllerTest.java (2)
66-77: 응답 검증 로직 개선복잡한 JSON 파싱과 객체 변환 로직을 사용하고 있습니다.
jsonPath를 더 활용하여 간소화할 수 있습니다.다음과 같이 개선을 제안합니다:
result.andExpect(status().isOk()) - .andExpect(jsonPath("$.data.aliasChoices").exists()); - - String json = result.andReturn().getResponse().getContentAsString(); - JsonNode jsonNode = objectMapper.readTree(json); - UserViewAliasChoiceResponse showResponse = objectMapper.treeToValue(jsonNode.get("data"), UserViewAliasChoiceResponse.class); - List<UserViewAliasChoiceResponse.AliasChoice> choices = showResponse.aliasChoices(); - - assertThat(choices).hasSize(2); - assertThat(choices) - .extracting("aliasName", "categoryName", "imageUrl", "color") - .containsExactlyInAnyOrder( - tuple("문학가", "문학", "문학가_image", "red"), - tuple("과학자", "과학/IT", "과학자_image", "blue") - ); + .andExpect(jsonPath("$.data.aliasChoices").exists()) + .andExpect(jsonPath("$.data.aliasChoices").isArray()) + .andExpect(jsonPath("$.data.aliasChoices", hasSize(2))) + .andExpect(jsonPath("$.data.aliasChoices[?(@.aliasName == '문학가' && @.categoryName == '문학')]").exists()) + .andExpect(jsonPath("$.data.aliasChoices[?(@.aliasName == '과학자' && @.categoryName == '과학/IT')]").exists());
80-106: 테스트 데이터 생성 헬퍼 메서드 구조 개선헬퍼 메서드가 좋은 접근이지만, 매직 넘버와 하드코딩된 값들을 개선할 수 있습니다.
상수를 도입하여 가독성을 높이는 것을 제안합니다:
+ private static final String TEST_ALIAS_1 = "문학가"; + private static final String TEST_CATEGORY_1 = "문학"; + private static final String TEST_ALIAS_2 = "과학자"; + private static final String TEST_CATEGORY_2 = "과학/IT"; + private void saveAliasesAndCategories() { AliasJpaEntity alias1 = AliasJpaEntity.builder() - .value("문학가") + .value(TEST_ALIAS_1) .imageUrl("문학가_image") .color("red") .build();src/test/java/konkuk/thip/user/adapter/in/web/UserSignupControllerTest.java (1)
171-171: 테스트 데이터의 보안 검토정적 분석 도구에서 API 키로 감지된 "kakao_12345678"은 실제로는 테스트용 OAuth2 ID입니다. 하지만 더 명확하게 테스트 데이터임을 표시할 수 있습니다.
테스트 데이터임을 더 명확하게 하기 위해 다음과 같이 개선을 제안합니다:
- String expectedOauth2Id = "kakao_12345678"; + String expectedOauth2Id = "test_kakao_12345678"; // 테스트용 OAuth2 ID
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (35)
build.gradle(1 hunks)src/main/java/konkuk/thip/config/QuerydslConfig.java(1 hunks)src/main/java/konkuk/thip/user/adapter/in/web/UserCommandController.java(1 hunks)src/main/java/konkuk/thip/user/adapter/in/web/UserQueryController.java(1 hunks)src/main/java/konkuk/thip/user/adapter/in/web/request/UserSignupRequest.java(1 hunks)src/main/java/konkuk/thip/user/adapter/in/web/request/UserVerifyNicknameRequest.java(1 hunks)src/main/java/konkuk/thip/user/adapter/in/web/response/UserVerifyNicknameResponse.java(1 hunks)src/main/java/konkuk/thip/user/adapter/in/web/response/UserViewAliasChoiceResponse.java(1 hunks)src/main/java/konkuk/thip/user/adapter/out/persistence/AliasJpaRepository.java(1 hunks)src/main/java/konkuk/thip/user/adapter/out/persistence/AliasQueryPersistenceAdapter.java(1 hunks)src/main/java/konkuk/thip/user/adapter/out/persistence/AliasQueryRepository.java(1 hunks)src/main/java/konkuk/thip/user/adapter/out/persistence/AliasQueryRepositoryImpl.java(1 hunks)src/main/java/konkuk/thip/user/adapter/out/persistence/UserJpaRepository.java(1 hunks)src/main/java/konkuk/thip/user/adapter/out/persistence/UserQueryPersistenceAdapter.java(1 hunks)src/main/java/konkuk/thip/user/adapter/out/persistence/UserQueryRepository.java(1 hunks)src/main/java/konkuk/thip/user/adapter/out/persistence/UserQueryRepositoryImpl.java(1 hunks)src/main/java/konkuk/thip/user/application/port/in/UserVerifyNicknameUseCase.java(1 hunks)src/main/java/konkuk/thip/user/application/port/in/UserViewAliasChoiceUseCase.java(1 hunks)src/main/java/konkuk/thip/user/application/port/in/dto/DummyCommand.java(0 hunks)src/main/java/konkuk/thip/user/application/port/in/dto/DummyQuery.java(0 hunks)src/main/java/konkuk/thip/user/application/port/in/dto/DummyResult.java(0 hunks)src/main/java/konkuk/thip/user/application/port/in/dto/UserViewAliasChoiceResult.java(1 hunks)src/main/java/konkuk/thip/user/application/port/out/AliasQueryPort.java(1 hunks)src/main/java/konkuk/thip/user/application/port/out/UserQueryPort.java(1 hunks)src/main/java/konkuk/thip/user/application/service/UserVerifyNicknameService.java(1 hunks)src/main/java/konkuk/thip/user/application/service/UserViewAliasChoiceService.java(1 hunks)src/test/java/konkuk/thip/config/TestQuerydslConfig.java(1 hunks)src/test/java/konkuk/thip/feed/adapter/out/jpa/FeedJpaEntityTest.java(2 hunks)src/test/java/konkuk/thip/room/adapter/out/jpa/RecordJpaEntityTest.java(2 hunks)src/test/java/konkuk/thip/room/adapter/out/jpa/RoomJpaEntityTest.java(2 hunks)src/test/java/konkuk/thip/room/adapter/out/jpa/VoteJpaEntityTest.java(3 hunks)src/test/java/konkuk/thip/user/adapter/in/web/UserSignupControllerTest.java(6 hunks)src/test/java/konkuk/thip/user/adapter/in/web/UserVerifyNicknameControllerTest.java(1 hunks)src/test/java/konkuk/thip/user/adapter/in/web/UserViewAliasChoiceControllerTest.java(1 hunks)src/test/java/konkuk/thip/user/adapter/out/jpa/UserJpaEntityTest.java(2 hunks)
💤 Files with no reviewable changes (3)
- src/main/java/konkuk/thip/user/application/port/in/dto/DummyQuery.java
- src/main/java/konkuk/thip/user/application/port/in/dto/DummyResult.java
- src/main/java/konkuk/thip/user/application/port/in/dto/DummyCommand.java
✅ Files skipped from review due to trivial changes (8)
- src/main/java/konkuk/thip/user/adapter/out/persistence/UserQueryRepository.java
- src/main/java/konkuk/thip/user/adapter/in/web/response/UserVerifyNicknameResponse.java
- src/main/java/konkuk/thip/user/adapter/in/web/request/UserVerifyNicknameRequest.java
- src/main/java/konkuk/thip/config/QuerydslConfig.java
- src/test/java/konkuk/thip/config/TestQuerydslConfig.java
- src/main/java/konkuk/thip/user/application/port/in/dto/UserViewAliasChoiceResult.java
- src/main/java/konkuk/thip/user/adapter/out/persistence/AliasQueryPersistenceAdapter.java
- src/main/java/konkuk/thip/user/adapter/in/web/response/UserViewAliasChoiceResponse.java
🚧 Files skipped from review as they are similar to previous changes (6)
- src/test/java/konkuk/thip/user/adapter/out/jpa/UserJpaEntityTest.java
- src/test/java/konkuk/thip/room/adapter/out/jpa/RoomJpaEntityTest.java
- src/test/java/konkuk/thip/feed/adapter/out/jpa/FeedJpaEntityTest.java
- src/test/java/konkuk/thip/room/adapter/out/jpa/VoteJpaEntityTest.java
- src/test/java/konkuk/thip/room/adapter/out/jpa/RecordJpaEntityTest.java
- src/main/java/konkuk/thip/user/adapter/in/web/request/UserSignupRequest.java
🧰 Additional context used
🧠 Learnings (2)
src/main/java/konkuk/thip/user/adapter/out/persistence/UserJpaRepository.java (1)
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#36
File: src/main/java/konkuk/thip/user/adapter/out/persistence/UserJpaRepository.java:7-7
Timestamp: 2025-06-29T09:47:31.299Z
Learning: Spring Data JPA에서 findBy{FieldName} 패턴의 메서드는 명시적 선언 없이 자동으로 생성되며, Optional<Entity> 반환 타입을 사용하는 것이 null 안전성을 위해 권장됩니다.
build.gradle (1)
Learnt from: hd0rable
PR: THIP-TextHip/THIP-Server#36
File: build.gradle:30-30
Timestamp: 2025-06-29T09:17:39.787Z
Learning: THIP-Server 프로젝트에서 Redis 의존성(spring-boot-starter-data-redis)은 향후 사용 예정으로 의도적으로 추가되었음. 현재는 사용하지 않지만 추후 개발에서 활용할 계획임.
🧬 Code Graph Analysis (1)
src/main/java/konkuk/thip/user/adapter/out/persistence/UserQueryRepositoryImpl.java (1)
src/main/java/konkuk/thip/user/adapter/out/persistence/AliasQueryRepositoryImpl.java (1)
Repository(13-41)
🪛 Gitleaks (8.26.0)
src/test/java/konkuk/thip/user/adapter/in/web/UserSignupControllerTest.java
171-171: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🔇 Additional comments (23)
src/main/java/konkuk/thip/user/application/port/out/UserQueryPort.java (1)
5-5: 닉네임 중복 확인 메서드가 적절히 추가되었습니다.메서드명과 시그니처가 명확하고, 포트 인터페이스로서 적절한 추상화 수준을 유지하고 있습니다.
src/main/java/konkuk/thip/user/application/port/in/UserVerifyNicknameUseCase.java (1)
1-6: 닉네임 검증 유스케이스가 잘 정의되었습니다.단일 책임 원칙을 따르며, 메서드명이 의도를 명확히 표현하고 있습니다. 헥사고날 아키텍처의 인바운드 포트 역할을 적절히 수행합니다.
src/main/java/konkuk/thip/user/application/port/in/UserViewAliasChoiceUseCase.java (1)
1-8: 별칭 조회 유스케이스가 명확하게 정의되었습니다.메서드명이 기능을 잘 표현하고 있으며, 적절한 DTO를 리턴타입으로 사용하고 있습니다. 헥사고날 아키텍처 패턴을 잘 따르고 있습니다.
src/main/java/konkuk/thip/user/application/port/out/AliasQueryPort.java (1)
1-8: 별칭 조회 포트가 적절히 정의되었습니다.인바운드 포트와 일관된 메서드명을 사용하고 있으며, 포트 인터페이스로서 적절한 추상화 수준을 유지하고 있습니다.
src/main/java/konkuk/thip/user/adapter/out/persistence/AliasQueryRepository.java (1)
5-8: 인터페이스 설계가 깔끔합니다.리포지토리 패턴을 잘 따르고 있으며, 메서드명이 명확하고 반환 타입도 적절합니다.
src/main/java/konkuk/thip/user/adapter/out/persistence/UserQueryPersistenceAdapter.java (2)
12-12: 필드명 변경이 코드 가독성을 향상시킵니다.
jpaRepository에서userJpaRepository로 변경하여 필드의 목적이 더 명확해졌습니다.
15-18: 닉네임 존재 여부 확인 로직이 올바릅니다.리포지토리에 위임하는 구조가 적절하며, 메서드 구현이 정확합니다.
src/main/java/konkuk/thip/user/adapter/out/persistence/AliasJpaRepository.java (1)
6-6: 인터페이스 확장이 Spring Data JPA 모범 사례를 따릅니다.표준 JPA 리포지토리 기능과 커스텀 쿼리 기능을 적절히 결합했습니다.
src/main/java/konkuk/thip/user/application/service/UserVerifyNicknameService.java (1)
8-18: 서비스 구현이 클린 아키텍처 원칙을 잘 따릅니다.의존성 주입과 포트 활용이 적절하며, 닉네임 유일성 검증 로직(
!userQueryPort.existsByNickname)이 정확합니다.src/main/java/konkuk/thip/user/adapter/out/persistence/AliasQueryRepositoryImpl.java (1)
24-37: QueryDSL 쿼리 구현이 올바르게 작성되었습니다.LEFT JOIN을 사용하여 카테고리가 없는 별칭도 포함시키는 것이 적절하며, Projections.constructor를 통한 DTO 매핑도 깔끔합니다. 조인 조건(
category.aliasForCategoryJpaEntity.eq(alias))도 엔티티 관계를 올바르게 반영하고 있습니다.src/main/java/konkuk/thip/user/adapter/in/web/UserQueryController.java (1)
14-21: 새로운 별명 선택 API 엔드포인트가 잘 구현되었습니다.Clean Architecture 원칙을 잘 따르고 있으며, 기존 코드 패턴과 일관성을 유지하고 있습니다. BaseResponse로 응답을 래핑하는 것도 좋은 설계입니다.
src/main/java/konkuk/thip/user/application/service/UserViewAliasChoiceService.java (1)
9-18: Clean Architecture 원칙을 잘 준수한 서비스 구현입니다.Use Case 인터페이스를 구현하여 애플리케이션 계층의 역할을 명확히 하고, 포트를 통한 위임으로 관심사 분리가 잘 되어 있습니다.
src/main/java/konkuk/thip/user/adapter/out/persistence/UserJpaRepository.java (1)
8-10: Spring Data JPA 모범 사례를 잘 적용했습니다.과거 리뷰 논의에서 제기된 Optional 반환 타입 사용 권장사항이 잘 반영되었습니다.
findByOauth2Id에서Optional<UserJpaEntity>을 사용하여 null 안전성을 확보했으며,existsByNickname은 표준 Spring Data JPA 패턴을 따르고 있습니다.src/main/java/konkuk/thip/user/adapter/in/web/UserCommandController.java (2)
30-38: OAuth2 기반 회원가입 API가 잘 구현되었습니다.
@Oauth2Id어노테이션을 통한 OAuth2 ID 주입과 JWT 토큰 생성 및 응답 헤더 설정이 적절하게 구현되어 있습니다. RESTful API 설계 원칙에도 부합합니다.
40-45: 별명 중복 검사 API가 간결하게 구현되었습니다.요청 유효성 검사와 응답 래핑이 일관된 패턴으로 잘 구현되어 있습니다.
build.gradle (2)
37-46: 보안 관련 의존성이 적절하게 추가되었습니다.Spring Security, JWT (0.12.3), OAuth2 Client 의존성이 모두 최신 버전으로 추가되어 있어 보안 기능 구현에 필요한 라이브러리가 잘 준비되었습니다.
59-84: Querydsl 설정이 매우 상세하고 올바르게 구성되었습니다.Jakarta 네임스페이스 사용, Q-클래스 생성 디렉토리 설정, 테스트 컴파일 시 중복 생성 방지, 소스 세트 포함, clean 시 정리 등 모든 측면이 체계적으로 설정되어 있습니다. 이는 Querydsl을 프로덕션 환경에서 안정적으로 사용할 수 있는 좋은 기반을 제공합니다.
src/test/java/konkuk/thip/user/adapter/in/web/UserViewAliasChoiceControllerTest.java (1)
29-31: 보안 필터 비활성화 검증 완료
UserQueryController.showAliasChoiceView()에 별도의 인증·인가 어노테이션이 없어 해당 엔드포인트는 공개 API로 동작합니다.
테스트에서@AutoConfigureMockMvc(addFilters = false)로 보안 필터를 비활성화해도 프로덕션 동작과 일치하므로 추가 수정은 필요 없습니다.src/test/java/konkuk/thip/user/adapter/in/web/UserSignupControllerTest.java (5)
47-48: JWT 유틸리티 의존성 추가 확인
JwtUtil이 테스트에 새로 추가되었습니다. 테스트 전용 설정이나 모킹이 필요한지 확인이 필요합니다.JWT 기반 인증 시스템 도입에 따른 자연스러운 의존성 추가입니다.
50-54: 테스트 격리를 위한 정리 로직 추가
@AfterEach메서드를 추가하여 테스트 간 격리를 보장하고 있습니다. 좋은 개선사항입니다.테스트 격리를 위한 적절한 정리 로직이 추가되었습니다.
72-75: JWT 임시 토큰 기반 인증 플로우OAuth2 기반 회원가입 플로우에 맞춘 임시 토큰 생성 및 헤더 설정이 적절히 구현되어 있습니다.
새로운 인증 플로우에 맞는 올바른 구현입니다.
154-193: OAuth2 ID 추출 검증 테스트JWT 토큰에서 OAuth2 ID를 정확히 추출하는지 검증하는 중요한 테스트입니다. 로직이 올바르게 구현되어 있습니다.
새로운 토큰 기반 인증 시스템의 핵심 기능을 잘 테스트하고 있습니다.
196-211: 인증 실패 케이스 테스트토큰 없이 요청했을 때의 401 에러 처리를 적절히 테스트하고 있습니다.
보안 요구사항에 맞는 적절한 에러 처리 테스트입니다.
| @SpringBootTest | ||
| @ActiveProfiles("test") | ||
| @AutoConfigureMockMvc(addFilters = false) |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
테스트 설정 검토: 보안 필터 비활성화
@AutoConfigureMockMvc(addFilters = false) 설정으로 보안 필터를 비활성화하고 있습니다. 이는 닉네임 검증 API가 인증이 필요 없는 공개 API임을 의미하는 것으로 보입니다. 실제 API 명세와 일치하는지 확인이 필요합니다.
다음 스크립트를 실행하여 닉네임 검증 API의 보안 설정을 확인해보겠습니다:
🏁 Script executed:
#!/bin/bash
# 닉네임 검증 API의 보안 설정 확인
ast-grep --pattern $'@PostMapping("/users/nickname")
$$$'Length of output: 58
🏁 Script executed:
#!/bin/bash
# 닉네임 검증 API 컨트롤러 매핑 및 보안 어노테이션 확인
rg "@PostMapping\\(\"/users/nickname\"\\)" -C5 -n srcLength of output: 1436
🏁 Script executed:
#!/bin/bash
# 보안 구성 파일에서 인증 및 인가 설정 검색
echo "> WebSecurityConfigurerAdapter 여부"
rg "WebSecurityConfigurerAdapter" -n src || true
echo "> SecurityFilterChain 빈 정의 여부"
rg "SecurityFilterChain" -n src || true
echo "> HttpSecurity 설정 (authorize, antMatchers 등)"
rg "HttpSecurity" -n src || true
rg "authorizeRequests" -n src || true
rg "authorizeHttpRequests" -n src || true
rg "antMatchers" -n src || true
rg "anyRequest" -n src || trueLength of output: 1220
🏁 Script executed:
#!/bin/bash
# SecurityConfig에서 공개 API 설정 확인
rg "permitAll" -n src
# users/nickname 경로에 대한 보안 설정 확인
rg "\"/users/nickname\"" -n srcLength of output: 1009
🏁 Script executed:
#!/bin/bash
# WHITELIST 상수 정의 및 경로 확인
rg "WHITELIST" -n -C5 src/main/java/konkuk/thip/config/SecurityConfig.javaLength of output: 1275
보안 필터 비활성화 검토: /users/nickname 엔드포인트 보안 조건 확인 필요
현재 SecurityConfig에서 POST /users/nickname은 WHITELIST에 포함되지 않아 인증이 필요한 상태입니다.
테스트에서 @AutoConfigureMockMvc(addFilters = false)로 보안 필터를 비활성화하면 실제 보안 설정과 불일치하여 테스트가 놓치는 부분이 생길 수 있습니다.
– 다음 중 하나를 선택해 조치해주세요:
• API가 공개(public) 엔드포인트여야 한다면
- SecurityConfig.WHITELIST 배열에 "/users/nickname" 경로 추가
• API가 인증(protected) 엔드포인트여야 한다면
- 테스트에서 addFilters = false 제거 후 보안 필터 적용하여 인증/인가 시나리오 추가
확인 파일:
- src/main/java/konkuk/thip/config/SecurityConfig.java
- src/test/java/konkuk/thip/user/adapter/in/web/UserVerifyNicknameControllerTest.java
🤖 Prompt for AI Agents
In
src/test/java/konkuk/thip/user/adapter/in/web/UserVerifyNicknameControllerTest.java
around lines 29 to 31, the test disables security filters with
@AutoConfigureMockMvc(addFilters = false), which causes a mismatch with the
actual security configuration where POST /users/nickname requires
authentication. To fix this, either add "/users/nickname" to the WHITELIST array
in src/main/java/konkuk/thip/config/SecurityConfig.java if the endpoint should
be public, or remove addFilters = false from the test annotation and update the
test to include authentication and authorization scenarios to reflect the
protected status of the endpoint.
- JWT - OAuth2 - Spring Security
- interface : xxxQueryRepository - 구현체 : xxxQueryRepositoryImpl
팀 컨벤션에 맞추어 수정
buzz0331
force-pushed
the
feat/#30-authentication-authorization
branch
from
a4374c2 to
533775d
Compare
|
3 participants
#️⃣ 연관된 이슈
📝 작업 내용
구글과 카카오톡 소셜 로그인 모두 연동 완료 해두었습니다. 로그인 처리흐름은 다음과 같습니다. 리프레쉬 토큰은 일단은 보류해두었습니다. 추후에 회의해서 결정하는 걸로 하죠!
테스트 코드는 성준님이 작성해둔 회원가입 api 테스트 코드에서 헤더를 검증하는 식으로 간단하게 작성해두었습니다.
[Oauth2 로그인 처리 흐름]
클라이언트에서 소셜 로그인 요청
/oauth2/authorization/{provider}302 리다이렉트 요청 보내주셔야 합니다!kakao또는googleSpring Security가 OAuth2 로그인 요청을 가로채고,
해당 provider의 인가 코드 요청 URL로 리다이렉트 '/login/oauth2/code/kakao'
사용자가 로그인 후 인가 코드를 받은 뒤,
GET /login/oauth2/code/{provider}로 리다이렉트되어 코드 수신OAuth2UserService가 인가 코드를 바탕으로 access token 발급 및 사용자 정보 조회조회한 사용자 정보로 이미 가입된 회원인지 판단
클라이언트는 JWT를 저장하고 이후 요청 시
Authorization헤더에Bearer {access_token}형식으로 전송📸 스크린샷
2025-06-28.6.47.53.mov
2025-06-28.6.48.35.mov
💬 리뷰 요구사항
📌 PR 진행 시 이러한 점들을 참고해 주세요
Summary by CodeRabbit
신규 기능
버그 수정
리팩터
테스트
문서화