Skip to content

Conversation

@sapiens2000
Copy link
Collaborator

@sapiens2000 sapiens2000 commented Apr 9, 2025

📌 PR 제목

프로필 생성 API

✨ 변경 사항

  • 리디렉션 위치 서브도메인으로 변경
  • 프로필 생성 위한 dto 추가
  • 프로필 생성 시 역할 변경 (ROLE_GUEST -> ROLE_USER)
  • 유저 서비스 단위 테스트 추가
  • 프로젝트 전반의 checkstyle 경고 제거

🔍 변경 이유

프론트 개발자님 요청에 따른 리디렉션 url 변경

✅ 체크리스트

  • 코드가 정상적으로 동작하는지 확인
  • 관련 테스트 코드 작성 및 통과 여부 확인
  • 문서화(README 등) 필요 여부 확인 및 반영
  • 리뷰어가 알아야 할 사항 추가 설명

📌 참고 사항

@github-actions
Copy link

github-actions bot commented Apr 9, 2025

Claude의 전체 변경사항 및 관련 파일에 대한 리뷰:

개선된 사항:

  • JPA Auditing 설정을 별도의 설정 클래스로 분리하여 모듈화를 개선했습니다.
  • 사용자 프로필 생성 및 업데이트 기능이 추가되었습니다.
  • 코드 중복을 줄이고 재사용성을 높이기 위해 몇 가지 메서드가 리팩토링되었습니다.

주요 이슈:

  1. OAuth2 인증 성공 후 리다이렉션 로직 개선

    • 제안: 리다이렉션 URL을 enum으로 관리하여 유지보수성을 높이고 오타를 방지할 수 있습니다.
    public enum RedirectUrl {
        GUEST(UrlConstants.PROFILE_CREATE_URL),
        USER(UrlConstants.FRONT_SUB_DOMAIN_URL),
        DEFAULT(UrlConstants.LOGIN_URL);
    
        private final String url;
    
        RedirectUrl(String url) {
            this.url = url;
        }
    
        public static String getUrl(String role) {
            return Arrays.stream(values())
                .filter(r -> r.name().equals(role))
                .findFirst()
                .orElse(DEFAULT)
                .url;
        }
    }
    
    // OAuth2AuthenticationSuccessHandler에서 사용
    String redirectUrl = RedirectUrl.getUrl(customOAuth2User.getRole());
  2. 사용자 프로필 업데이트 메서드 반환 값 불일치

    • 제안: UserService의 updateMyProfile 메서드가 void를 반환하도록 변경되었지만, UserProfileTest에서는 여전히 UserProfileResponseDto를 반환하는 것으로 가정하고 있습니다. 테스트 코드를 수정해야 합니다.
    @Test
    @DisplayName("내 프로필이 업데이트 되어야 한다.")
    void updateMyProfileSuccess() {
        User previous = userRepository.save(UserFixture.createUserFixture());
        UserProfileUpdateRequestDto updateRequestDto = new UserProfileUpdateRequestDto("업데이트이미지경로", "업데이트상태메시지");
    
        userService.updateMyProfile(previous.getUserId(), updateRequestDto);
    
        User updatedUser = userRepository.findById(previous.getUserId()).orElseThrow();
        assertEquals(updateRequestDto.profileImage(), updatedUser.getProfileImage());
        assertEquals(updateRequestDto.statusMessage(), updatedUser.getStatusMessage());
    }

전반적인 의견:
코드 품질이 전반적으로 향상되었으며, 사용자 프로필 관리 기능이 잘 구현되었습니다. 하지만 일부 테스트 코드와 리다이렉션 로직에 개선의 여지가 있습니다.

관련 파일에 대한 영향 분석:

  • JpaAuditingConfig 추가로 인해 Log4UApplication에서 @EnableJpaAuditing 어노테이션이 제거되었습니다. 이는 JPA Auditing 설정의 모듈화를 개선했지만, 다른 JPA 관련 설정 파일들과의 관계를 확인해야 합니다.
  • UserProfileMakeRequestDto 추가로 User 엔티티, UserService, UserController에 새로운 메서드가 추가되었습니다. 이는 사용자 프로필 생성 플로우에 영향을 미치며, 관련 테스트 코드도 업데이트해야 합니다.
  • UrlConstants 변경으로 OAuth2AuthenticationSuccessHandler와 SecurityConfig의 CORS 설정이 영향을 받았습니다. 프론트엔드 애플리케이션의 URL 변경 시 이 부분을 주의깊게 관리해야 합니다.
  • UserService의 메서드 시그니처 변경(특히 updateMyProfile)으로 인해 UserController와 관련 테스트 코드를 수정해야 합니다.

@github-actions
Copy link

github-actions bot commented Apr 9, 2025

Claude의 전체 변경사항 및 관련 파일에 대한 리뷰:

개선된 사항:

  • JPA Auditing 설정을 별도의 설정 클래스로 분리하여 관심사를 분리했습니다.
  • 사용자 프로필 생성 및 업데이트 로직이 추가되었습니다.
  • 테스트 코드가 추가되어 코드의 신뢰성이 향상되었습니다.

주요 이슈:

  1. 보안 설정 개선

    • 제안: CORS 설정을 별도의 메서드로 분리하여 가독성을 높이고 유지보수성을 개선합니다.
      private CorsConfiguration corsConfiguration() {
          CorsConfiguration configuration = new CorsConfiguration();
          configuration.setAllowedOrigins(Arrays.asList(UrlConstants.FRONT_ORIGIN_URL, UrlConstants.FRONT_SUB_DOMAIN_URL));
          configuration.setAllowedMethods(Collections.singletonList("*"));
          configuration.setAllowCredentials(true);
          configuration.setAllowedHeaders(Collections.singletonList("*"));
          configuration.setMaxAge(3600L);
          configuration.setExposedHeaders(Arrays.asList("Set-Cookie", "access", "refresh"));
          return configuration;
      }
  2. 사용자 프로필 업데이트 로직 개선

    • 제안: 트랜잭션 관리를 서비스 레벨에서 수행하도록 변경합니다.
      @Transactional
      public void updateMyProfile(Long userId, UserProfileUpdateRequestDto dto) {
          User user = getUserById(userId);
          user.updateMyProfile(dto);
          // userRepository.save(user); // 트랜잭션 내에서 자동으로 저장됨
      }
  3. 테스트 코드 개선

    • 제안: 테스트 케이스에 더 다양한 시나리오를 추가하고, 경계값 테스트를 포함시킵니다.
      @Test
      @DisplayName("닉네임 최대 길이 초과 시 예외 발생")
      void shouldThrowExceptionWhenNicknameTooLong() {
          String longNickname = "a".repeat(51); // 가정: 최대 길이가 50
          assertThrows(IllegalArgumentException.class, () -> 
              userService.validateNickname(longNickname));
      }

관련 파일에 대한 영향 분석:

  • User 엔티티 변경으로 인해 UserService, CustomOAuth2User, UserRepository 등에서 새로운 필드나 메서드를 사용할 수 있게 되었습니다. 이에 따라 관련 테스트 코드도 업데이트가 필요할 수 있습니다.
  • UserProfileResponseDto의 변경으로 UserController와 관련 테스트 코드의 수정이 필요할 수 있습니다.
  • UrlConstants의 변경으로 SecurityConfig와 OAuth2AuthenticationSuccessHandler의 CORS 설정 및 리다이렉트 로직이 영향을 받을 수 있습니다.

전반적인 의견:
코드 품질이 전반적으로 향상되었으며, 특히 사용자 프로필 관리 기능이 강화되었습니다. 보안 설정과 테스트 커버리지를 더욱 개선하면 좋을 것 같습니다.

@github-actions
Copy link

github-actions bot commented Apr 9, 2025

Claude의 전체 변경사항 및 관련 파일에 대한 리뷰:

개선된 사항:

  • JPA Auditing 설정이 별도의 설정 클래스로 분리되어 관심사 분리가 잘 이루어졌습니다.
  • 사용자 프로필 생성 및 업데이트 기능이 추가되어 기능이 확장되었습니다.
  • 단위 테스트가 추가되어 코드의 신뢰성이 향상되었습니다.

주요 이슈:

  1. SecurityConfig의 CORS 설정 중복

    • 제안: CORS 설정을 별도의 메서드로 분리하여 중복을 제거하세요.
    private CorsConfiguration getCorsConfiguration() {
        CorsConfiguration configuration = new CorsConfiguration();
        configuration.setAllowedOrigins(Arrays.asList(UrlConstants.FRONT_ORIGIN_URL, UrlConstants.FRONT_SUB_DOMAIN_URL));
        // ... 기타 설정 ...
        return configuration;
    }
  2. UserService의 중복 코드

    • 제안: getUserById와 getUserByNickname 메서드의 중복 로직을 제거하세요.
    private User getUser(Function<String, Optional<User>> finder, String identifier) {
        return finder.apply(identifier).orElseThrow(UserNotFoundException::new);
    }
    
    public User getUserById(Long userId) {
        return getUser(userRepository::findById, userId.toString());
    }
    
    public User getUserByNickname(String nickname) {
        return getUser(userRepository::findByNickname, nickname);
    }
  3. OAuth2AuthenticationSuccessHandler의 복잡성

    • 제안: 토큰 생성 및 쿠키 설정 로직을 별도의 메서드로 분리하세요.
    private void createAndSetTokenCookies(HttpServletResponse response, Long userId, String name, String role) {
        String access = jwtUtil.createJwt(ACCESS_TOKEN, userId, name, role, accessTokenValidityInSeconds);
        String refresh = jwtUtil.createJwt(REFRESH_TOKEN, userId, name, role, refreshTokenValidityInSeconds);
        response.addCookie(createCookie(ACCESS_TOKEN, access));
        response.addCookie(createCookie(REFRESH_TOKEN, refresh));
        refreshTokenService.saveRefreshToken(name, refresh);
    }

관련 파일에 대한 영향 분석:

  • User 엔티티 변경: UserProfileResponseDto, OAuth2AuthenticationSuccessHandler, CustomOAuth2UserService 등에 영향을 줄 수 있습니다. 특히 프로필 관련 필드가 변경된 경우 DTO와 서비스 로직을 업데이트해야 합니다.
  • UserRepository 변경: UserService와 CustomOAuth2UserService에서 사용하는 쿼리 메서드가 변경되었다면, 해당 서비스들의 로직을 확인하고 필요시 수정해야 합니다.
  • UrlConstants 변경: SecurityConfig와 OAuth2AuthenticationSuccessHandler에서 사용하는 URL 상수가 변경되었으므로, 해당 클래스들의 설정을 확인하고 업데이트해야 합니다.

전반적인 의견:
코드 품질이 전반적으로 향상되었습니다. 특히 테스트 추가와 관심사 분리가 잘 이루어졌습니다. 하지만 일부 중복 코드와 복잡한 로직이 있어 리팩토링의 여지가 있습니다. 제안된 개선사항을 적용하면 코드의 가독성과 유지보수성이 더욱 향상될 것입니다.

@github-actions
Copy link

github-actions bot commented Apr 9, 2025

Claude의 전체 변경사항 및 관련 파일에 대한 리뷰:

개선된 사항:

  • JPA Auditing 설정이 별도의 설정 클래스로 분리되어 관심사 분리가 잘 이루어졌습니다.
  • CORS 설정이 CorsConfig 클래스로 분리되어 재사용성과 유지보수성이 향상되었습니다.
  • UserService에 프로필 생성 및 업데이트 기능이 추가되어 기능이 확장되었습니다.
  • UserServiceTest가 추가되어 서비스 로직의 신뢰성이 향상되었습니다.

주요 이슈:

  1. OAuth2AuthenticationSuccessHandler의 리다이렉션 로직 개선

    • 제안: switch 문을 Map으로 대체하여 가독성과 확장성을 높일 수 있습니다.
    private static final Map<String, String> ROLE_REDIRECT_MAP = Map.of(
        "ROLE_GUEST", PROFILE_CREATE_URL,
        "ROLE_USER", FRONT_SUB_DOMAIN_URL
    );
    
    private void redirectTo(HttpServletResponse response, CustomOAuth2User customOAuth2User) throws IOException {
        String redirectUrl = ROLE_REDIRECT_MAP.getOrDefault(customOAuth2User.getRole(), LOGIN_URL);
        response.sendRedirect(redirectUrl);
    }
  2. UserService의 중복 코드 제거

    • 제안: getMyProfile과 getUserProfile 메서드의 중복 로직을 제거합니다.
    private UserProfileResponseDto getUserProfileResponseDto(User user) {
        Long userId = user.getUserId();
        return UserProfileResponseDto.fromUser(
            user,
            followRepository.countByTargetId(userId),
            followRepository.countByInitiatorId(userId)
        );
    }
    
    public UserProfileResponseDto getMyProfile(Long userId) {
        User me = getUserById(userId);
        return getUserProfileResponseDto(me);
    }
    
    public UserProfileResponseDto getUserProfile(String nickname) {
        User target = getUserByNickname(nickname);
        return getUserProfileResponseDto(target);
    }
  3. UserController의 예외 처리 개선

    • 제안: @ControllerAdvice를 사용하여 전역 예외 처리를 구현합니다.
    @ControllerAdvice
    public class GlobalExceptionHandler {
        @ExceptionHandler(UserNotFoundException.class)
        public ResponseEntity<ErrorResponse> handleUserNotFoundException(UserNotFoundException ex) {
            ErrorResponse error = new ErrorResponse("User not found", ex.getMessage());
            return new ResponseEntity<>(error, HttpStatus.NOT_FOUND);
        }
    }

관련 파일에 대한 영향 분석:

  • UserService 변경으로 UserController, FollowService, JwtAuthenticationFilter 등에서 새로운 메서드를 사용할 수 있게 되었습니다.
  • UrlConstants 변경으로 OAuth2AuthenticationSuccessHandler, CorsConfig 등에서 새로운 URL 상수를 사용할 수 있습니다.
  • User 엔티티 변경으로 CustomOAuth2User, UserFixture 등에서 새로운 필드나 메서드를 사용해야 할 수 있습니다.
  • SecurityConfig 변경으로 전체적인 보안 설정이 영향을 받을 수 있으며, 다른 컴포넌트들의 인증/인가 동작이 변경될 수 있습니다.

전반적인 의견:
코드 품질이 전반적으로 향상되었으며, 기능 확장과 테스트 추가로 안정성이 개선되었습니다. 다만, 일부 중복 코드와 복잡한 로직에 대해 리팩토링이 필요해 보입니다.

@github-actions
Copy link

github-actions bot commented Apr 9, 2025

Claude의 전체 변경사항 및 관련 파일에 대한 리뷰:

개선된 사항:

  • JPA Auditing 설정이 별도의 설정 클래스로 분리되어 관심사 분리가 잘 이루어졌습니다.
  • CORS 설정이 분리되어 더 명확하고 재사용 가능한 형태로 개선되었습니다.
  • 유저 프로필 생성 및 업데이트 로직이 추가되어 기능이 확장되었습니다.
  • 테스트 코드가 추가되어 코드의 신뢰성이 향상되었습니다.

주요 이슈:

  1. OAuth2AuthenticationSuccessHandler의 리다이렉션 로직 개선

    • 제안: enum을 사용하여 role에 따른 리다이렉션 URL 매핑을 더 명확하게 관리할 수 있습니다.
    private enum RoleRedirect {
        ROLE_GUEST(PROFILE_CREATE_URL),
        ROLE_USER(FRONT_SUB_DOMAIN_URL),
        DEFAULT(LOGIN_URL);
    
        private final String url;
    
        RoleRedirect(String url) {
            this.url = url;
        }
    
        public static String getUrlForRole(String role) {
            try {
                return valueOf(role).url;
            } catch (IllegalArgumentException e) {
                return DEFAULT.url;
            }
        }
    }
    
    private void redirectTo(HttpServletResponse response, CustomOAuth2User customOAuth2User) throws IOException {
        String redirectUrl = RoleRedirect.getUrlForRole(customOAuth2User.getRole());
        response.sendRedirect(redirectUrl);
    }
  2. UserService의 중복 코드 제거

    • 제안: getMyProfile과 getUserProfile 메서드의 중복 로직을 제거할 수 있습니다.
    private UserProfileResponseDto getUserProfileResponseDto(User user) {
        Long userId = user.getUserId();
        return UserProfileResponseDto.fromUser(
            user,
            followRepository.countByTargetId(userId),
            followRepository.countByInitiatorId(userId)
        );
    }
    
    public UserProfileResponseDto getMyProfile(Long userId) {
        User me = getUserById(userId);
        return getUserProfileResponseDto(me);
    }
    
    public UserProfileResponseDto getUserProfile(String nickname) {
        User target = getUserByNickname(nickname);
        return getUserProfileResponseDto(target);
    }

관련 파일에 대한 영향 분석:

  • User 엔티티의 변경은 UserRepository, UserService, 그리고 관련 테스트 코드에 영향을 줄 수 있습니다. 특히 새로운 필드나 메서드가 추가된 경우, 이를 사용하는 모든 곳에서 적절히 처리되어야 합니다.
  • CORS 설정 변경은 전체 애플리케이션의 보안에 영향을 줄 수 있으므로, 프론트엔드 애플리케이션과의 통합 테스트가 필요할 수 있습니다.
  • UrlConstants의 변경은 OAuth2AuthenticationSuccessHandler와 같은 다른 클래스에서 사용되는 URL에 영향을 줄 수 있으므로, 관련 로직을 검토해야 합니다.
  • UserService의 변경은 UserController와 같은 다른 서비스나 컨트롤러에 영향을 줄 수 있으므로, 연관된 메서드 호출을 확인해야 합니다.

전반적인 의견:
코드 품질이 전반적으로 향상되었습니다. 관심사 분리, 코드 재사용성, 그리고 테스트 커버리지가 개선되었습니다. 다만, 일부 중복 코드와 복잡한 로직에 대해 추가적인 리팩토링을 고려해볼 수 있습니다.

@github-actions
Copy link

github-actions bot commented Apr 9, 2025

Claude의 전체 변경사항 및 관련 파일에 대한 리뷰:

개선된 사항:

  • CORS 설정의 분리 및 재사용성 향상 (CorsConfig 클래스 생성)
  • JPA Auditing 설정의 분리 (JpaAuditingConfig 클래스 생성)
  • UserService의 기능 확장 (프로필 생성, 업데이트 등)
  • 쿠키 관련 유틸리티 클래스 추가 (CookieUtil)
  • UserServiceTest 추가로 테스트 커버리지 향상

주요 이슈:

  1. SecurityConfig의 CORS 설정 중복 제거

    • 제안: CorsConfig 클래스를 활용하여 중복 코드를 제거하고 재사용성을 높입니다.
    @Configuration
    public class SecurityConfig {
        @Bean
        public SecurityFilterChain filterChain(HttpSecurity http) throws Exception {
            // ...
            http.cors(corsCustomizer -> corsCustomizer.configurationSource(CorsConfig.corsConfigurationSource()));
            // ...
        }
    }
  2. UserService의 트랜잭션 관리

    • 제안: 프로필 생성 및 업데이트 메서드에 @transactional 어노테이션을 추가하여 데이터 일관성을 보장합니다.
    @Service
    public class UserService {
        @Transactional
        public void createMyProfile(Long userId, UserProfileMakeRequestDto dto) {
            // ...
        }
    
        @Transactional
        public void updateMyProfile(Long userId, UserProfileUpdateRequestDto dto) {
            // ...
        }
    }
  3. OAuth2AuthenticationSuccessHandler의 리다이렉션 로직 개선

    • 제안: switch 문을 사용하여 코드의 가독성과 유지보수성을 높입니다.
    private void redirectTo(HttpServletResponse response, CustomOAuth2User customOAuth2User) throws IOException {
        String redirectUrl = switch (customOAuth2User.getRole()) {
            case "ROLE_GUEST" -> UrlConstants.PROFILE_CREATE_URL;
            case "ROLE_USER" -> UrlConstants.FRONT_SUB_DOMAIN_URL;
            default -> UrlConstants.LOGIN_URL;
        };
        response.sendRedirect(redirectUrl);
    }

관련 파일에 대한 영향 분석:

  • UserService 변경: FollowService, UserController, CustomOAuth2UserService 등에 영향을 줄 수 있습니다. 특히 프로필 생성 및 업데이트 로직이 변경되었으므로, 이를 사용하는 다른 서비스나 컨트롤러의 동작을 확인해야 합니다.
  • UrlConstants 변경: CorsConfig, OAuth2AuthenticationSuccessHandler, UserController 등에서 URL 상수를 사용하는 부분을 확인하고 필요시 수정해야 합니다.
  • User 엔티티 변경: UserProfileResponseDto, CustomOAuth2User 등 User 엔티티를 사용하는 클래스들의 동작을 확인해야 합니다.
  • CorsConfig 추가: SecurityConfig에서 CORS 설정을 사용하는 방식이 변경되었으므로, 전체적인 CORS 동작을 테스트해야 합니다.

전반적인 의견:
코드의 구조화와 모듈화가 개선되었으며, 특히 CORS 설정과 JPA Auditing 설정의 분리는 좋은 변화입니다. 트랜잭션 관리와 예외 처리에 더 주의를 기울이고, 테스트 커버리지를 더욱 높이는 것이 좋을 것 같습니다.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Apr 9, 2025

Quality Gate Failed Quality Gate failed

Failed conditions
3 Security Hotspots

See analysis details on SonarQube Cloud

@github-actions
Copy link

github-actions bot commented Apr 9, 2025

Claude의 전체 변경사항 및 관련 파일에 대한 리뷰:

개선된 사항:

  • 코드 구조와 가독성이 전반적으로 개선되었습니다.
  • CORS 설정이 별도의 클래스로 분리되어 관리가 용이해졌습니다.
  • JPA Auditing 설정이 분리되어 애플리케이션의 구조가 개선되었습니다.
  • 사용자 프로필 생성 및 업데이트 로직이 추가되었습니다.

주요 이슈:

  1. 보안 관련 개선 필요:

    • 제안: JWT 토큰 관리와 관련된 보안을 강화해야 합니다. 특히 개발용 토큰 발급 부분은 제거하거나 보안을 강화해야 합니다.
    // UserController.java
    @GetMapping("/dev")
    public ResponseEntity<Void> loginAsDevUser(HttpServletResponse response) {
        // 이 부분을 제거하거나 개발 환경에서만 동작하도록 수정
    }
  2. 예외 처리 개선:

    • 제안: 글로벌 예외 처리기를 더욱 세분화하고, 커스텀 예외를 추가하여 더 명확한 오류 메시지를 제공해야 합니다.
    // GlobalExceptionHandler.java
    @ExceptionHandler(CustomBusinessException.class)
    public ResponseEntity<ApiErrorResponse> handleCustomBusinessException(CustomBusinessException e) {
        // 비즈니스 로직 관련 예외 처리
    }
  3. 테스트 코드 확장:

    • 제안: 새로 추가된 기능, 특히 사용자 프로필 관련 로직에 대한 단위 테스트와 통합 테스트를 추가해야 합니다.
    // UserServiceTest.java
    @Test
    void createMyProfile_ShouldUpdateUserRoleToUser() {
        // 프로필 생성 시 사용자 역할이 ROLE_USER로 변경되는지 확인하는 테스트
    }

관련 파일에 대한 영향 분석:

  • UserService와 UserController의 변경으로 인해 인증 및 권한 관련 로직(SecurityConfig, OAuth2AuthenticationSuccessHandler 등)에 영향을 줄 수 있습니다. 이들 간의 일관성을 유지해야 합니다.
  • DiaryRepository의 변경은 DiaryService와 MapService에 영향을 줄 수 있으므로, 관련 기능들이 정상적으로 작동하는지 확인해야 합니다.
  • CorsConfig의 변경으로 인해 전체 애플리케이션의 CORS 정책이 변경될 수 있으므로, 프론트엔드와의 통신에 문제가 없는지 확인해야 합니다.

전반적인 의견:
코드베이스가 전반적으로 개선되었지만, 보안 관련 사항과 테스트 커버리지를 더욱 강화할 필요가 있습니다. 또한, 새로 추가된 기능들이 기존 시스템과 잘 통합되는지 확인하고, 필요한 경우 문서화를 업데이트해야 합니다.

@sapiens2000 sapiens2000 merged commit 1b9f174 into develop Apr 9, 2025
3 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants