fix(user,admin): n+1해결 및 getUser 로직 변경 #332
Conversation
- 사용자 주문 조회 시 n+1 문제 해결(@entitygraph) - getuser 로직으로 인한 user 2회 중복 조회 로직 1회 수정으로 변
WalkthroughOAuth2 의존성과 테스트 구성 변경, MemberDetails에 Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Controller
participant Service
participant MemberDetails
participant User
Note over Service,MemberDetails: 기존 흐름 (헬퍼 사용)
Client->>Controller: 요청
Controller->>Service: 처리 호출
Service->>Service: getUser(memberDetails) 호출 (private helper)
Service->>MemberDetails: 내부로 user 조회
MemberDetails->>Service: User 반환
Note over Service,MemberDetails: 변경된 흐름 (직접 접근)
Client->>Controller: 요청
Controller->>Service: 처리 호출
Service->>MemberDetails: memberDetails.getUser()
MemberDetails->>Service: User 반환
sequenceDiagram
participant Manager
participant Controller
participant Service
participant DTO
participant User
Manager->>Controller: 매니저 가입 요청(ManagerSignupRequestDto)
Controller->>Service: DTO 전달
Service->>DTO: toEntity() 호출
Note over DTO: 추가 필드 적용<br/>isMarketingAgree, phoneEntered(false), profileImage="no"
DTO->>Service: User 엔티티 반환
Service->>Service: User 저장
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/menu/service/MenuService.java (1)
129-129: 리팩토링이 누락된 부분이 있습니다.
updateMenuSortOrder메서드에서는 여전히userRepository.findById(memberDetails.getId())를 사용하고 있습니다. 일관성을 위해 다른 메서드들과 동일하게memberDetails.getUser()로 변경해야 합니다.다음과 같이 수정하세요:
- User user = userRepository.findById(memberDetails.getId()).orElseThrow(UserNotFoundException::new); + User user = memberDetails.getUser();
🧹 Nitpick comments (7)
nowait-app-user-api/build.gradle (1)
31-36: 모듈 의존성 구조는 좋으나, 한글 인라인 주석은 문서화 파일로 분리하길 권장합니다.라인 33의 한글 주석은 domain-user-rdb 의존성의 중요성을 명확히 나타내고 있지만, Gradle 파일에서는 일반적으로 프로그래밍 언어별 주석 규칙을 따릅니다. 이 정보를 README나 ARCHITECTURE.md 같은 별도 문서에 옮기면 팀 온보딩 시 더 효과적일 것 같습니다.
현재 모듈 의존성 순서(core-rdb → user-rdb → redis → event → infra)는 명확한 계층 구조를 반영하고 있어 좋습니다.
nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/reservation/service/ReservationService.java (1)
545-550: 주석처리된 코드는 제거해야 합니다.더 이상 사용되지 않는
getUser()헬퍼 메서드는 주석 대신 완전히 삭제하는 것이 코드 가독성과 유지보수성에 좋습니다.다음과 같이 주석처리된 코드를 제거하세요:
- // private User getUser(MemberDetails memberDetails) { - // if (memberDetails == null) { - // throw new ReservationViewUnauthorizedException(); - // } - // return userRepository.findById(memberDetails.getId()).orElseThrow(UserNotFoundException::new); - // }nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/storePayment/service/StorePaymentServiceImpl.java (1)
89-91: 주석처리된 코드는 제거가 필요합니다.더 이상 사용되지 않는 헬퍼 메서드를 주석으로 남기는 대신 완전히 삭제하세요.
- // private User getUser(MemberDetails memberDetails) { - // return userRepository.findById(memberDetails.getId()).orElseThrow(UserNotFoundException::new); - // }nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/store/service/StoreServiceImpl.java (1)
153-158: 주석처리된 코드 제거를 권장합니다.사용되지 않는
getUser()헬퍼 메서드는 삭제하여 코드를 정리하세요.- // private User getUser(MemberDetails memberDetails) { - // if (memberDetails == null) { - // throw new StoreViewUnauthorizedException(); - // } - // return userRepository.findById(memberDetails.getId()).orElseThrow(UserNotFoundException::new); - // }nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/menu/service/MenuService.java (1)
213-215: 주석처리된 코드는 제거하세요.더 이상 사용되지 않는 헬퍼 메서드를 삭제하여 코드를 정리하세요.
- // private User getUser(MemberDetails memberDetails) { - // return userRepository.findById(memberDetails.getId()).orElseThrow(UserNotFoundException::new); - // }nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/order/service/OrderService.java (2)
51-67: 디버그 로그 제거 권장 및 리팩토링 승인
memberDetails.getUser()를 직접 호출하여 중복 DB 조회를 제거한 것은 올바른 개선입니다. 이를 통해 N+1 문제와 불필요한 I/O를 효과적으로 해결했습니다.다만, 추가된 디버그 로그(51, 53, 55, 66번 라인)는 성능 측정 목적의 임시 코드로 보입니다. 성능 테스트 완료 후 이러한 로그는 제거하거나, 필요하다면 보다 의미 있는 메시지와 적절한 로그 레벨(예: TRACE)로 변경하시기 바랍니다.
다음 diff를 적용하여 디버그 로그를 제거하세요:
@Transactional(readOnly = true) public List<OrderResponseDto> findAllOrders(Long storeId, MemberDetails memberDetails) { - log.info("getUser 호출 전"); User user = memberDetails.getUser(); - log.info("getUser 호출 완료"); storeRepository.findByStoreIdAndDeletedFalse(storeId).orElseThrow(StoreNotFoundException::new); - log.info("Store 조회 완료"); validateViewAuthorization(user, storeId); LocalDate today = LocalDate.now(ZoneId.of("Asia/Seoul")); LocalDateTime startDateTime = today.atStartOfDay(); LocalDateTime endDateTime = today.plusDays(1).atStartOfDay(); List<OrderResponseDto> order = orderRepository.findAllByStore_StoreIdAndCreatedAtBetween(storeId, startDateTime, endDateTime) .stream() .map(OrderResponseDto::fromEntity) .collect(Collectors.toList()); - log.info("Order 조회 완료"); return order; }
156-162: 주석 처리된 코드 제거 권장주석 처리된 기존
getUser헬퍼 메서드는 완전히 제거하는 것이 좋습니다. Git 히스토리에 이전 코드가 보관되므로, 주석으로 남겨둘 필요가 없습니다.다음 diff를 적용하여 주석 처리된 코드를 제거하세요:
- // private User getUser(MemberDetails memberDetails) { - // if (memberDetails == null) { - // throw new OrderViewUnauthorizedException(); - // } - // // findById로 Select 쿼리 나가는 것을 getReferenceById로 변경하여 방지 - // return userRepository.getReferenceById(memberDetails.getId()); - // } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
nowait-app-admin-api/build.gradle(4 hunks)nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/menu/service/MenuService.java(6 hunks)nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/order/service/OrderService.java(6 hunks)nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/reservation/service/ReservationService.java(6 hunks)nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/store/service/StoreServiceImpl.java(4 hunks)nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/storePayment/controller/StorePaymentController.java(1 hunks)nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/storePayment/service/StorePaymentService.java(1 hunks)nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/storePayment/service/StorePaymentServiceImpl.java(5 hunks)nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/user/dto/ManagerSignupRequestDto.java(1 hunks)nowait-app-user-api/build.gradle(3 hunks)nowait-domain/domain-core-rdb/src/main/java/com/nowait/domaincorerdb/order/repository/OrderRepository.java(1 hunks)nowait-domain/domain-core-rdb/src/main/java/com/nowait/domaincorerdb/user/entity/MemberDetails.java(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/order/service/OrderService.java (3)
nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/menu/service/MenuService.java (1)
Service(39-221)nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/reservation/service/ReservationService.java (1)
Service(50-551)nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/store/service/StoreServiceImpl.java (1)
Service(37-159)
🔇 Additional comments (18)
nowait-app-admin-api/build.gradle (4)
52-55: OAuth2 의존성과 jakarta.servlet-api 명시 추가 확인OAuth2 리소스 서버 의존성(
spring-boot-starter-oauth2-resource-server)이 추가되었습니다. PR 요약에서 주요 변경사항으로 명시되지 않았는데, admin-api에서 리소스 서버 역할이 필요한지 확인이 필요합니다. 일반적으로 OAuth2 클라이언트만으로 충분할 수 있습니다.또한
jakarta.servlet-api:6.0.0는spring-boot-starter-web에 이미 transitively 포함되어 있으므로, 명시적 추가가 필요한 특별한 이유가 있는지 확인하시기 바랍니다.
37-37: nowait-event 프로젝트 의존성 추가새로운 프로젝트 의존성
nowait-event가 추가되었습니다. 이는 PR #331 요구사항과 일치합니다. 단, 이 의존성이 기존nowait-domain모듈들과의 순환 참조나 버전 불일치를 야기하지 않는지 확인하시기 바랍니다.
86-91: 테스트 의존성 재구성Testcontainers BOM 기반 관리에서 명시적 버전 선언으로 변경되었습니다. JUnit Jupiter, Mockito, Testcontainers Redis 의존성이 명시적으로 추가되었으며, 이는 테스트 환경 설정의 명확성을 높입니다.
61-65: Swagger/OpenAPI 및 Redis 의존성 추가API 문서화를 위한 Swagger/OpenAPI(
springdoc-openapi-starter-webmvc-ui:2.6.0)와 캐싱/세션 관리를 위한 Redis 스타터가 추가되었습니다. 이는 PR 요약의memberDetail과 Spring Security 세션 저장 로직과 일치합니다.nowait-app-user-api/build.gradle (3)
43-45: OAuth2 클라이언트 의존성 추가가 PR 목표와 일치합니다.사용자 인증 처리 개선에 필요한 OAuth2 클라이언트를 추가하셨으며, 기존 JWT 라이브러리와 함께 하이브리드 인증 방식을 지원하는 구조로 보입니다. PR #331의 N+1 문제 해결 및 memberDetail.getUser() 로직 개선과 일관성 있게 적용되었네요.
81-83: 테스트 의존성을 Testcontainers BOM으로 관리하는 것은 모범 사례입니다.이전의 개별 의존성 선언에서 BOM(Bill of Materials) 기반 관리로 변경하셨는데, 이는 버전 충돌을 방지하고 유지보수성을 크게 높입니다. Spring Cloud BOM 관리 패턴(라인 26)과 일관성 있게 적용되어 있습니다.
74-74: Actuator와 Prometheus 의존성이 PR 목표 범위를 벗어나 보입니다.라인 74(actuator)와 76-77(prometheus)의 모니터링 의존성 추가는 관찰성(observability) 개선에 좋은 변화지만, PR #331의 목표인 "N+1 문제 해결 및 getUser 로직 변경"과는 직접적인 연관이 없어 보입니다. 이 변경사항이 의도적인 별도 개선인지, 아니면 다른 PR과 혼합되었는지 확인 부탁드립니다.
Also applies to: 76-77
nowait-domain/domain-core-rdb/src/main/java/com/nowait/domaincorerdb/order/repository/OrderRepository.java (1)
16-17: N+1 문제 해결을 위한 EntityGraph 적용이 적절합니다.
@EntityGraph를 통해store,orderItems,orderItems.menu연관관계를 즉시 로딩하여 N+1 쿼리 문제를 효과적으로 해결했습니다.nowait-domain/domain-core-rdb/src/main/java/com/nowait/domaincorerdb/user/entity/MemberDetails.java (2)
31-33: getUser() 메서드 구현이 적절합니다.User 객체에 대한 직접 접근을 제공하여 중복 조회를 제거하는 목적을 달성했습니다. 단, Line 19의 잠재적 이슈에 대한 검증이 선행되어야 합니다.
19-19: 검증 결과: Lazy Loading 문제는 없으나 아키텍처 개선 권고 유지
User엔티티 검토 결과, 현재 코드의 실제 위험도는 낮습니다:
Lazy Loading 문제 없음: User 엔티티에는 지연로딩 연관관계가 없습니다. 모든 필드가 스칼라 타입(String, Long, Boolean, Enum)이므로 LazyInitializationException 발생 위험이 없습니다.
Detached Entity 위험 완화됨: 모든
getUser()호출이@Transactional메서드 내에서 발생하므로, User 객체는 트랜잭션 내 접근 시점에 영속성 컨텍스트에 존재합니다.현재 코드는 안전: 기본 필드(id, email, role, storeId 등)만 접근하므로 즉시 로드되어 문제가 없습니다.
다만 아키텍처 권고: Security Context에 JPA 엔티티를 직접 저장하는 것은 장기적으로 다음 이유로 DTO 패턴 도입을 고려할 만합니다:
- 향후 User에 지연로딩 연관관계 추가 시 자동으로 문제 발생
- Security Context의 생명주기와 Hibernate 세션 생명주기 독립성 확보
- 보안 컨텍스트 데이터와 영속성 계층 분리
nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/storePayment/controller/StorePaymentController.java (1)
15-15: 패키지 이름 일관성 개선이 적절합니다.
storepayment에서storePayment로 변경하여 camelCase 명명 규칙을 준수했습니다.nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/storePayment/service/StorePaymentService.java (1)
1-1: 패키지 이름 변경이 적절합니다.패키지명을 camelCase로 통일하여 코드 일관성을 향상시켰습니다.
nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/reservation/service/ReservationService.java (1)
64-64: 중복 DB 조회 제거가 적절합니다.
memberDetails.getUser()를 직접 호출하여 불필요한userRepository조회를 제거했습니다. 이는 PR 목적인 I/O 중복 제거를 달성합니다.Also applies to: 99-99, 198-198, 217-217, 367-367
nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/storePayment/service/StorePaymentServiceImpl.java (1)
41-41: 중복 DB 조회 제거가 적절합니다.
memberDetails.getUser()를 통해 직접 User 객체에 접근하여 불필요한 데이터베이스 조회를 제거했습니다.Also applies to: 59-59, 72-72
nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/store/service/StoreServiceImpl.java (1)
62-62: 중복 조회 제거로 성능이 개선되었습니다.
memberDetails.getUser()를 사용하여 UserRepository 호출을 제거했습니다.Also applies to: 85-85, 118-118
nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/menu/service/MenuService.java (1)
50-50: 중복 DB 조회 제거가 적절합니다.대부분의 메서드에서
memberDetails.getUser()를 통해 불필요한 데이터베이스 호출을 제거했습니다.Also applies to: 64-64, 89-89, 105-105, 171-171
nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/order/service/OrderService.java (2)
37-41: 로깅 지원 추가를 승인하되, 디버그 로그는 임시용으로 보임Lombok
@Slf4j어노테이션을 추가하여 로깅 기능을 활성화한 것은 좋습니다. 다만, 이후 추가된 디버그 로그들이 성능 테스트 목적으로 보이므로, 테스트 완료 후 제거하거나 적절한 로그 레벨로 조정하는 것을 권장합니다.
73-73: 최적화 검증 완료 - 안전성 확인됨
MemberDetails.getUser()구현을 확인한 결과, 다음이 확인되었습니다:
완전히 로드된 엔티티 반환: getUser()는 Spring Security 인증 컨텍스트에서 설정된 User 객체를 반환합니다. MemberDetails.create() 팩토리 메서드를 통해 인증 시점에 User가 저장되며, private final 필드로 유지되므로 항상 동일한 완전히 로드된 인스턴스입니다.
null 안전성: 이 메서드들이 수신하는 MemberDetails는 Spring Security의 요청 인터셉터를 통과한 인증된 컨텍스트에서만 제공되므로, getUser()가 null을 반환하지 않음이 보장됩니다. 인증 실패 시 요청이 서비스 메서드에 도달하지 않습니다.
최적화 유효성: userRepository 호출 제거는 정당한 최적화입니다. 기존의 getReferenceById() DB 호출은 불필요했으며, 인증 시점에 이미 로드된 User 객체를 직접 사용하는 것이 더 효율적입니다.
| @Schema(description = "폰 번호 입력 여부", example = "true") | ||
| private boolean phoneEntered; | ||
|
|
||
| public User toEntity() { | ||
| return User.builder() | ||
| .profileImage("no") | ||
| .email(email) | ||
| .phoneNumber("") | ||
| .password(password) | ||
| .nickname(nickname) | ||
| .socialType(SocialType.LOCAL) | ||
| .role(Role.MANAGER) | ||
| .isMarketingAgree(isMarketingAgree) | ||
| .phoneEntered(false) | ||
| .createdAt(LocalDateTime.now()) |
There was a problem hiding this comment.
phoneEntered 값이 무조건 false로 저장됩니다.
요청 DTO에서 phoneEntered를 true로 보내더라도 toEntity()에서 항상 false를 주입해버려 실제 사용자 데이터가 왜곡됩니다. 이 필드는 방금 DTO에 추가한 신규 정보인데, 엔티티에 반영되지 않으면 의미가 없습니다. User.builder().phoneEntered(phoneEntered)처럼 DTO 값을 그대로 전달하도록 수정해 주세요.
🤖 Prompt for AI Agents
In
nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/user/dto/ManagerSignupRequestDto.java
around lines 42 to 56, the toEntity() builder hardcodes phoneEntered(false)
causing DTO phoneEntered values to be ignored; change the builder call to pass
the DTO field (phoneEntered) instead of the hardcoded false so the entity
reflects the incoming value.
| implementation project(':nowait-domain:domain-core-rdb') | ||
| implementation project(':nowait-domain:domain-redis') | ||
| implementation project(':nowait-event') | ||
| implementation project(":nowait-event") |
There was a problem hiding this comment.
''에서 ""로 바꾼 이유가 있을까요?! ''로 통일해서 사용하는게 좋을 것 같습니다
| // private User getUser(MemberDetails memberDetails) { | ||
| // return userRepository.findById(memberDetails.getId()).orElseThrow(UserNotFoundException::new); | ||
| // } |
There was a problem hiding this comment.
OAuth 측에서 예외 터진다면 해당 로직 삭제해도 될 것 같습니다
| User user = memberDetails.getUser(); | ||
| log.info("getUser 호출 완료"); | ||
| storeRepository.findByStoreIdAndDeletedFalse(storeId).orElseThrow(StoreNotFoundException::new); | ||
| log.info("Store 조회 완료"); |
There was a problem hiding this comment.
log.info 배포때는 없어도 될 것 같습니다!
nowait-app-user-api/build.gradle
Outdated
| implementation project(":nowait-domain:domain-core-rdb") | ||
| implementation project(":nowait-domain:domain-user-rdb") // ✅ 이 줄 꼭 있어야 합니다 | ||
| implementation project(":nowait-domain:domain-redis") | ||
| implementation project(":nowait-event") |
There was a problem hiding this comment.
user 쪽에는 이벤트 사용하는 로직 없어서 삭제 부탁드립니다
작업 요약
Issue Link
#331
문제점 및 어려움
해결 방안
memberDetail이 이미 Spring security에서 user 정보를 세션에 저장 후 userRepository와 비교하기 떄문에 굳이 중복할 필요 없다고 판단
Reference
Summary by CodeRabbit
새로운 기능
개선 사항