[Refactor] pickeat 플로우에서 인메모리 캐싱 적용#417
Conversation
- 캐시 활용을 위한 spring-boot-starter-cache 및 caffeine 의존성 추가
- 로깅 간결화를 위해 body 필드 제거
- LoggingEventCompositeJsonEncoder를 활용하여 JSON 형식의 로그 출력 적용 - 로깅 타임스탬프, 레벨, 쓰레드명, 메시지 등의 정보를 구조화하여 기록
- ParticipantTokenProvider와 ProviderTokenProvider 생성 및 로직 분리 - JwtProvider 리팩토링으로 추가 정보를 포함한 토큰 생성 기능 도입 - 관련 컨트롤러 및 서비스 로직 수정
- Pickeat 엔티티의 participantCount 필드 삭제
- hasEqualLike 관련 로직 삭제 - RestaurantResultResponse -> PickeatResultResponse로 클래스명 변경 - PickeatResult에 동점 여부 필드 제거 - SQL 및 테스트 코드 수정 - 불필요한 코드를 간소화하고 가독성을 개선
…간접참조로 대체 - Pickeat 및 Restaurant를 ID로만 저장하도록 수정 - Repository 및 Service 로직 변경 - 테스트 코드 수정 및 불필요한 OneToOne 매핑 제거
- Pickeat을 ID로만 저장하도록 수정 - Repository 및 Service 로직 개선 - 테스트 코드 수정 및 불필요한 ManyToOne 매핑 제거 - 코드 가독성 향상 및 성능 최적화
- Pickeat을 ID로만 저장하도록 수정 - Restaurant의 likeCount 및 관련 로직 제거 - RestaurantService 및 Repository 로직 변경 및 개선 - Tests 내부 로직 및 픽스처 생성 방식 수정 - 기존 Restaurants 도메인 클래스 제거 - 코드 가독성 향상 및 무결성 검증 로직 단순화
- 단위 테스트 불가한 도메인 제거 후 애플리케이션 레이어로 이동
- CacheManagerConfig 클래스 생성 및 글로벌 캐시 매니저 설정 - CacheType enum 정의 및 캐시 타입별 속성 관리 - RESTAURANT 캐시 타입 추가 (TTL, 최대 크기, 초기 용량 설정 포함)
- RestaurantRepository를 JpaRepository와 JdbcTemplate 기반으로 분리해 RestaurantJpaRepository와 RestaurantJdbcRepository로 구현 - Pickeat 관련 캐싱 설정 도입 및 삭제 시 캐시 무효화 로직 적용
- Pickeat 비활성화 시 발생하는 PickeatDeactivatedEvent 생성 - 해당 이벤트를 처리하여 Restaurant 캐시를 무효화하는 PickeatDeactivatedEventHandler 추가
- RestaurantService에서 사용자 좋아요 데이터 조회 최적화 - RestaurantLike 케싱 관련 테스트 코드 추가
- PickeatRepository 인터페이스 분리 및 PickeatRepositoryImpl 클래스 생성 - Pickeat 검색 결과 캐싱 로직 추가 (Caffeine 활용) - Pickeat 캐시 사용 테스트 코드 추가 - PickeatJpaRepository로 리팩토링 및 관련 로직 최적화 - Pickeat 비활성화 시 캐시 무효화 처리 로직 강화 (PickeatDeactivatedEventHandler 수정)
- ParticipantRepository 인터페이스 분리 및 ParticipantRepositoryImpl 클래스 생성 - 참가자 검색 결과 캐싱 로직 추가 (Caffeine 활용) - 참가자 캐시 사용 테스트 코드 추가 - ParticipantJpaRepository로 리팩토링 및 관련 코드 최적화
- ParticipantStateUpdatedEvent 도메인 이벤트 클래스 생성 - ParticipantStateUpdatedEventHandler 구현 및 트랜잭션 후 이벤트 처리 로직 추가 - 참가자 캐시 무효화 로직 적용 (ParticipantRepositoryImpl 활용)
- PickeatDeactivatedEventHandler에 참가자 캐시 무효화 처리 로직 적용 - PickeatRepositoryImpl의 캐싱 키 표현식 수정 (value.toString() 사용)
- PickeatResult에서 has_equal_like 컬럼 제거 - PickeatResult 테이블의 FK 제약 조건 삭제 및 pickeat_id, restaurant_id 필드 NOT NULL로 수정 - Restaurant에서 like_count, type 컬럼 제거 및 기존 외래키 제약 조건 삭제 - Participant 및 Restaurant 테이블에 대한 새로운 인덱스 생성 - 간접 참조 처리 기반 데이터 무결성 유지 및 불필요한 컬럼 정리
|
@coderabbitai |
|
@jinu0328 네, PR을 리뷰하겠습니다! 커밋 단위로 잘 정리해주셔서 변경 사항을 파악하기 수월할 것 같습니다. 특히 다음 부분들을 중점적으로 확인하겠습니다:
바로 리뷰를 시작하겠습니다! 🐰 ✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 34
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
backend/src/main/java/com/pickeat/backend/global/log/dto/ResponseLog.java (1)
1-33: 테스트 코드가 필요합니다!ResponseLog의 구조가 변경되었으므로, 다음 테스트 케이스들이 필요합니다:
1. 단위 테스트 (
ResponseLogTest.java)@Test void of_메서드로_ResponseLog를_생성한다() { // given ContentCachingResponseWrapper response = mock(ContentCachingResponseWrapper.class); when(response.getStatus()).thenReturn(200); String requestURI = "/api/test"; // when ResponseLog responseLog = ResponseLog.of(response, requestURI); // then assertThat(responseLog.logType()).isEqualTo(LogType.RESPONSE); assertThat(responseLog.uri()).isEqualTo(requestURI); assertThat(responseLog.status()).isEqualTo(200); } @Test void fields_메서드는_body를_포함하지_않는다() { // given ResponseLog responseLog = new ResponseLog(LogType.RESPONSE, "/api/test", 200); // when Map<String, Object> fields = responseLog.fields(); // then assertThat(fields).containsKeys("logType", "uri", "status"); assertThat(fields).doesNotContainKey("body"); assertThat(fields.get("status")).isEqualTo(200); } @Test void summary_메서드는_올바른_형식으로_문자열을_반환한다() { // given ResponseLog responseLog = new ResponseLog(LogType.RESPONSE, "/api/test", 404); // when String summary = responseLog.summary(); // then assertThat(summary).isEqualTo("[RESPONSE] /api/test 404"); }2. 통합 테스트 또는 필터 테스트
- 로깅 필터/인터셉터와 함께 ResponseLog가 올바르게 생성되고 기록되는지 검증
- body가 로그에 포함되지 않는지 확인
#!/bin/bash # 기존 테스트 코드 확인 echo "=== ResponseLog 관련 테스트 파일 찾기 ===" fd -t f -e java "ResponseLog" backend/src/test echo -e "\n=== 로깅 관련 테스트 파일 찾기 ===" fd -t f -e java backend/src/test | xargs rg -l "ResponseLog|LogType"backend/src/main/java/com/pickeat/backend/restaurant/application/RestaurantService.java (1)
85-95: 좋아요 동시성 문제: 데이터 무결성 보장 필요레이스 컨디션이 실제로 존재합니다.
RestaurantLike엔티티와restaurant_like테이블 모두에(restaurant_id, participant_id)복합 유니크 제약이 없어서, 두 스레드가 동시에existsLike()체크를 통과한 후save()를 호출하면 중복된 좋아요가 데이터베이스에 저장될 수 있습니다.해결 방법:
데이터베이스 제약 추가 (필수)
- 마이그레이션:
ALTER TABLE restaurant_like ADD UNIQUE KEY uk_restaurant_participant (restaurant_id, participant_id);- JPA 엔티티:
@Table(uniqueConstraints = @UniqueConstraint(columnNames = {"restaurantId", "participantId"}))예외 처리 (필수)
try { restaurantLikeRepository.save(new RestaurantLike(participant.getId(), restaurant.getId())); } catch (DataIntegrityViolationException e) { throw new BusinessException(ErrorCode.PARTICIPANT_RESTAURANT_ALREADY_LIKED); }이렇게 하면
existsLike()실패 후 누군가 먼저 저장해도 idempotent하게 처리됩니다.테스트 케이스 추가 제안
- 동시성 테스트: 두 스레드가 동시에
like()호출 시 중복 저장 방지 확인- 예외 처리 테스트:
DataIntegrityViolationException발생 시BusinessException변환 확인이것은 single point of truth(데이터베이스 제약)에 application logic(예외 처리)을 덧붙이는 방어 전략입니다. 같은 방식을
unlike()메서드에도 적용해주세요.backend/src/main/java/com/pickeat/backend/tobe/restaurant/ui/api/RestaurantApiSpec.java (1)
120-123: RequestBody 충돌로 인한 바인딩 실패 가능성(치명적)이 위치의
@RequestBody는 현재 파일 상단에서 임포트한io.swagger.v3.oas.annotations.parameters.RequestBody로 해석됩니다. 따라서 Spring MVC가 본 파라미터를 HTTP 바디로 바인딩하지 못할 수 있어 400/Null 바인딩이 발생합니다. 위/아래 메서드들처럼 Spring의@org.springframework.web.bind.annotation.RequestBody로 통일해주세요.권장 수정안:
- @Valid @RequestBody WishRestaurantRequest request); + @Valid @org.springframework.web.bind.annotation.RequestBody WishRestaurantRequest request);또는 Swagger
RequestBody는 OpenAPI 문서의operation.requestBody에만 사용하고, 파라미터에는 반드시 Spring의@RequestBody를 사용하도록 일관성을 유지해주세요.backend/src/main/java/com/pickeat/backend/pickeat/ui/api/PickeatApiSpec.java (1)
240-254: createPickeatResult의 응답 코드 정책을 문서에서 명확히 통일해 주세요컨트롤러는 201만 반환합니다. 스펙은 신규 201/기존 200을 함께 기재합니다. 운영 혼선을 막기 위해
- A안: 201만 반환하도록 스키마·예시를 단일화, 또는
- B안: 코드 변경으로 201/200 분기
중 하나로 결정하여 문서/코드 동기화가 필요합니다.Also applies to: 278-282
♻️ Duplicate comments (1)
backend/src/main/java/com/pickeat/backend/restaurant/ui/RestaurantController.java (1)
57-64: (v2와 동일) 서비스 인자 형태 일관화 권장위 v2 컨트롤러 코멘트와 동일한 맥락입니다. 두 버전이 공존하므로 규칙을 맞춰두면 마이그레이션에 유리합니다.
🧹 Nitpick comments (96)
backend/src/main/resources/db/migration/V13__drop_participant_pickeat_foreign_key.sql (1)
21-35: 인덱스 생성 후 검증 단계 추가를 고려해보세요.현재 마이그레이션은
idx_participant_pickeat_id인덱스를 조건부로 생성하지만, 생성 후 실제로 성공했는지 확인하지 않습니다. 특히 프로덕션 환경에서 대규모 테이블에 인덱스를 추가할 때 타임아웃이나 메모리 부족 문제가 발생할 수 있습니다.고려사항:
- 마이그레이션 전
participant테이블의 예상 행 수를 문서로 남기거나, 성능 영향도를 별도 이슈/ADR로 기록해두세요.pickeat_id컬럼이 실제로 쿼리 성능 개선에 필요한지, 그리고 다른 컬럼과 복합 인덱스가 더 나을지 확인하셨나요?이는 필수 수정은 아니지만, 향후 마이그레이션 문제 추적 시 도움이 될 것 같습니다.
backend/src/main/resources/logback-local.xml (1)
4-24: 구조화된 로깅 필드 설정 — MDC 및 민감 정보 필터링 검토 제안제공자(providers) 구성이 명확하고 표준에 맞게 설정되었습니다. 몇 가지 고려 사항을 제안합니다:
MDC(Mapped Diagnostic Context) 활용 정책:
<mdc/>가 모든 MDC 값을 로그에 포함하도록 설정되어 있습니다. 현재 코드에서 MDC에 어떤 정보를 담고 있는지, 그리고 요청별 추적(request tracing)이나 사용자 정보 등이 포함되는지 확인해주세요.민감 정보 보호: 특히
<arguments/>와<mdc/>는 의도치 않게 비밀번호, 토큰, 개인정보(이메일, 전화번호 등)를 노출할 수 있습니다. 로그 필터링 또는 마스킹 정책을 수립하면 좋습니다. 예를 들어:
- logstash-logback-encoder의 JsonGeneratorDecorator를 활용한 필터
- 또는 Spring Boot의 로그 필터 구현
로그 크기 및 성능 영향:
<arguments/>를 통해 모든 메서드 인자를 로깅하면, 특히 큰 객체나 대용량 데이터가 포함될 경우 로그 크기가 증가하고 성능에 영향을 줄 수 있습니다. 필요한 정보만 선택적으로 로깅하는 방식도 검토해보세요.참고: Logback 공식 문서 - Configuration, LogstashEncoder
backend/src/main/resources/db/migration/V12__update_pickeat_result_indirect_reference.sql (2)
1-9: 마이그레이션 작업 순서와 원자성을 검토하세요.현재 마이그레이션은 외래 키 제거와 NOT NULL 제약 추가를 분리된 ALTER 문으로 수행하고 있습니다. 동시성 환경에서 이 두 작업 사이의 간격은 데이터 정합성 위험을 초래할 수 있습니다.
개선 방법:
하나의 원자적 트랜잭션으로 통합하는 것을 권장합니다:
START TRANSACTION; ALTER TABLE `pickeat_result` DROP FOREIGN KEY `FKg5rkj84lmqukrfiuhos82thdx`, DROP FOREIGN KEY `FKlkhf7ufetto289l3lyx35f39o`, MODIFY COLUMN `pickeat_id` bigint NOT NULL, MODIFY COLUMN `restaurant_id` bigint NOT NULL; COMMIT;또는 Flyway의
executeInTransaction설정 확인:# application.properties 또는 application.yml spring.flyway.execute-in-transaction=true참고: 우테코의 공식 컨벤션 문서(docs/back-code-convention.md)에서 DB 마이그레이션 안내를 확인하세요.
7-9: 롤백(Downgrade) 전략이 없습니다.이 마이그레이션이 실패하거나 롤백해야 하는 경우, 반대 작업을 수행할 방법이 없습니다. 프로덕션 환경에서 긴급 롤백이 필요할 때 큰 부담이 됩니다.
권장 사항:
마이그레이션 전에 데이터베이스 백업 수행:
mysqldump -h ${DB_HOST} -u ${DB_USER} -p${DB_PASSWORD} ${DB_NAME} > backup_v12.sql롤백 계획 수립:
- V12가 실패하면 이전 버전(V11)으로 자동 롤백
- 또는 명시적인 Undo 마이그레이션 파일(V12_undo.sql) 준비
변경 영향도 검증:
-- 외래 키 제거 후 참조 무결성 위반 확인 SELECT pr.id, pr.pickeat_id, pr.restaurant_id FROM pickeat_result pr LEFT JOIN pickeat p ON pr.pickeat_id = p.id LEFT JOIN restaurant r ON pr.restaurant_id = r.id WHERE p.id IS NULL OR r.id IS NULL;점진적 롤아웃 전략:
- 스테이징 환경에서 충분히 테스트
- 피크 시간 외(야간/휴일)에 프로덕션 배포
- 모니터링 대기 후 다음 마이그레이션 진행
backend/src/main/resources/db/migration/V14__update_restaurant_schema_for_indirect_reference.sql (1)
14-14:SELECT 1을 no-op으로 사용하는 것은 작동하지만, 가독성과 관례를 위해 개선을 권장합니다.현재 코드는 조건부 실행에서 "아무것도 하지 않아야 할 때"
SELECT 1을 사용합니다(lines 14, 30, 46). 이는 동작하지만 SQL 관례상 다음과 같은 문제가 있습니다:
- 의도 불명확:
SELECT 1이 실제로 "no-op"인지 명시적이지 않습니다.- 성능 고려: MySQL은 실제로 쿼리를 실행해야 합니다.
개선 방안:
방법 1 - 주석 추가 (현재 방식 유지):
'SELECT 1' -- 아무 작업 안 함 (no-op)방법 2 - MySQL 표준 문법 (권장):
-- MySQL 5.7.4+ 에서 지원 IF NOT EXISTS (...) THEN -- 작업 수행 END IF;다만 현재 마이그레이션 스타일(PREPARE/EXECUTE 패턴)을 유지한다면, 명시적 주석을 추가하는 것이 충분합니다:
SET @drop_like_count_sql := ( SELECT IF(EXISTS (...), 'ALTER TABLE `restaurant` DROP COLUMN `like_count`', 'SELECT 1' /* 컬럼이 이미 없으면 스킵 */) );참고: 이 부분은 기능상 문제는 아니지만, 향후 유지보수 시 의도를 명확히 하는 데 도움이 됩니다.
Also applies to: 30-30, 46-46
backend/src/main/java/com/pickeat/backend/global/log/dto/ResponseLog.java (1)
12-18: 팩토리 메서드 시그니처를 더 일반적으로 개선할 수 있겠네요.코드는 깔끔하게 정리되었지만, 한 가지 개선할 점이 있습니다.
개선 제안:
현재
ResponseLog.of()는response.getStatus()하나만 사용하고 있습니다.ContentCachingResponseWrapper의 바디 캐싱 기능을 활용하지 않는다면, 더 일반적인HttpServletResponse인터페이스로 매개변수를 변경하는 것이 좋습니다.public static ResponseLog of(HttpServletResponse response, String requestURI) { return new ResponseLog( LogType.RESPONSE, requestURI, response.getStatus() ); }왜 이렇게 하면 좋을까요?
ResponseLog가 불필요하게 구체적인 타입(ContentCachingResponseWrapper)에 의존하지 않음- 호출 시 더 유연한 Response 객체를 전달 가능
- 테스트 시에도 Mock이나 다른 HttpServletResponse 구현체 사용 가능
- 관심사 분리: ResponseLog는 상태 정보만 관심, 바디 캐싱 메커니즘과 무관
참고:
LogFilter에서는 여전히ContentCachingResponseWrapper가 필요합니다.copyBodyToResponse()를 호출하여 response body를 클라이언트에 전달해야 하기 때문입니다. 따라서 filter의cacheResponse래핑 자체는 유지하되,ResponseLog.of()로 전달할 때만 인터페이스 타입으로 변환하면 됩니다.backend/src/main/java/com/pickeat/backend/global/auth/principal/ParticipantPrincipal.java (1)
1-4: Record 사용이 적절합니다! 불변 데이터 전달에 딱이네요. 👏Principal 타입으로 인증 컨텍스트를 명확히 표현한 점이 좋습니다. 몇 가지 개선 제안:
JavaDoc 추가: Principal의 역할과 사용 시점을 문서화하면 팀원들이 이해하기 쉽습니다.
/** * 픽잇 참가자의 인증 정보를 담는 Principal 객체 * @param id 참가자 식별자 * @param rawPickeatCode 참가자가 속한 픽잇의 코드 */ public record ParticipantPrincipal(Long id, String rawPickeatCode) { }유효성 검증 고려:
rawPickeatCode가 null이거나 빈 문자열일 수 있나요? 필요하다면 compact constructor에서 검증을 추가할 수 있습니다.public record ParticipantPrincipal(Long id, String rawPickeatCode) { public ParticipantPrincipal { if (id == null || id <= 0) { throw new IllegalArgumentException("id는 양수여야 합니다"); } if (rawPickeatCode == null || rawPickeatCode.isBlank()) { throw new IllegalArgumentException("rawPickeatCode는 필수입니다"); } } }현재 설계가 의도된 것이라면 무시하셔도 됩니다!
backend/src/test/java/com/pickeat/backend/pickeat/domain/ParticipantTest.java (1)
24-29: Assertion 검증 로직이 명확합니다! ✨
getPickeatId()를 사용한 검증으로 깔끔하게 업데이트되었네요.추가 테스트 제안:
- 유효하지 않은 pickeatId 테스트:
pickeatId가 null이거나 음수일 때의 동작을 검증하는 테스트를 추가하면 좋겠습니다.@Test void pickeatId가_null이면_생성_실패() { assertThatThrownBy(() -> new Participant("테스트", null)) .isInstanceOf(IllegalArgumentException.class); }현재 Participant 생성자에 validation이 없다면 추가를 고려해보세요!
backend/src/test/java/com/pickeat/backend/fixture/ParticipantFixture.java (1)
7-8: 고정 닉네임은 충돌 가능 — 랜덤/파라미터화 권장여러 테스트에서 동일 닉네임("참가자")을 쓰면 유니크 제약이나 시나리오 간 간섭이 생길 수 있어요. 기본 닉네임을 랜덤하게 만들거나 파라미터 오버로드를 추가해 주세요. 또한 Pickeat 객체를 가진 테스트를 위해 ID를 추출하는 편의 오버로드도 있으면 좋습니다. 깔끔 리팩 좋았습니다!
예시 diff:
public class ParticipantFixture { - public static Participant create(Long pickeatId) { - return new Participant("참가자", pickeatId); - } + public static Participant create(Long pickeatId) { + return new Participant("참가자-" + java.util.UUID.randomUUID(), pickeatId); + } + + public static Participant createWithNickname(Long pickeatId, String nickname) { + return new Participant(nickname, pickeatId); + } + + // 테스트 가독성용 편의 오버로드 + public static Participant create(com.pickeat.backend.pickeat.domain.Pickeat pickeat) { + return create(pickeat.getId()); + }테스트 케이스 제안:
- 동일 Pickeat 내 동일 닉네임 중복 저장 시 예외 발생 확인(유니크 제약 존재 시).
(backend/** 가이드라인 준수)backend/src/main/java/com/pickeat/backend/pickeat/application/PickeatScheduler.java (4)
35-37: 스케줄 타이밍과 타임존 명시서버 JVM 타임존에 의존하면 일자 경계가 흔들릴 수 있어요. @scheduled에 zone을 명시하거나 설정으로 주입해 주세요. 주기 역시 설정값으로 추출하면 운영 편의성이 올라갑니다. 잘 쪼갠 책임 좋습니다!
예시 diff:
- @Scheduled(cron = "0 0 0 * * *") // 매일 00:00에 실행 + @Scheduled(cron = "${pickeat.cleanup.cron:0 0 0 * * *}", zone = "${app.timezone:Asia/Seoul}") // 매일 00:00 (KST 기본)추가로 Clock 주입을 고려하면 테스트 용이성과 일관성이 좋아집니다.
38-44: 삭제 대상 날짜 범위: ‘해당 일 하루’ vs ‘기준 이전 전체’ 의도 확인현재는 기준일(LocalDate.now().minusDays(3))의 00:00~23:59:59.999 사이만 삭제합니다. 매일 정상 동작하면 “딱 3일 전 것만” 제거되지만, 스케줄이 실패/중단되면 누락분이 계속 남을 수 있어요. 더 견고하게 하려면 “기준일의 다음날 00:00 미만”(< endExclusive) 조건을 권장합니다.
대안:
- PickeatJpaRepository에
findByUpdatedAtBefore(LocalDateTime endExclusive)추가 후endExclusive = targetDate.plusDays(1).atStartOfDay()사용.- 또는 대량 삭제 쿼리(
deleteByUpdatedAtBefore(...)) 도입으로 읽기 없이 바로 삭제.경계/타임스탬프 정밀도(DB millis/ micros vs Java nanos)도 고려해 < nextDayStart가 안전합니다.
(backend/** 가이드라인)Also applies to: 42-42
57-61: ID 조회 API는 Long으로 반환하도록 정리
findIdsByPickeatIdIn호출 후BaseEntity::getId로 매핑하고 있어 반환 타입과 메서드명이 어긋납니다. 메서드는List<Long>를 직접 반환하는 게 명확하고, 불필요한 엔티티 로딩도 피할 수 있어요.예시:
- List<Long> restaurantIds = restaurantJpaRepository.findIdsByPickeatIdIn(expiredPickeatIds).stream() - .map(BaseEntity::getId) - .toList(); + List<Long> restaurantIds = restaurantJpaRepository.findIdsByPickeatIdIn(expiredPickeatIds);리포지토리 시그니처도 프로젝션(Long)으로 수정 권장.
(backend/** 가이드라인)
53-55: 레스토랑/픽잇 삭제도 배치 처리 및 결과 검증 로그 권장오래된 데이터가 많으면
deleteByPickeatIds와deleteAll도 배치/카운트 로깅이 유용합니다. 또한 스케줄 1회 트랜잭션 크기가 커지지 않도록 페이징/배치를 고려해 주세요.운영 팁:
- 삭제 건수 로깅, 실패 시 재시도 전략(다음 스케줄에서 누락분 포착), 성능 메트릭 수집.
backend/src/main/java/com/pickeat/backend/pickeat/domain/repository/PickeatRepository.java (1)
13-17: 파라미터 명 오타(room) 정정으로 가독성 개선
rooId,roodIds오타는 혼란을 줍니다. 의미 있는 이름으로 정리해 주세요. 인터페이스는 팀의 ‘계약’이라 더 중요해요. 리팩 멋집니다!예시 diff:
- List<Pickeat> findByRoomId(Long rooId); + List<Pickeat> findByRoomId(Long roomId); @@ - List<Pickeat> findByRoomIdIn(List<Long> roodIds); + List<Pickeat> findByRoomIdIn(List<Long> roomIds);추가 제안:
- 스케줄 전용 쿼리(
findByUpdatedAtBetween)는 JpaRepository 쪽으로 한정하고, 도메인 리포지토리는 유스케이스 중심 API만 노출하는 것도 고려해 보세요.
(backend/** 가이드라인)backend/src/test/java/com/pickeat/backend/restaurant/domain/repository/RestaurantJpaRepositoryTest.java (1)
35-37: 테스트 의도 명확화 좋습니다ID 기반 조회로 전환한 부분 깔끔합니다. 추가로 존재하지 않는 pickeatId 조회 시 빈 리스트 반환 케이스도 하나 있으면 안정감이 더 생깁니다.
(backend/** 가이드라인)
backend/src/main/java/com/pickeat/backend/restaurant/domain/RestaurantLike.java (1)
15-24: 좋아요 중복/탐색 성능 대비: 유니크 제약 + 인덱스 권장ID 스칼라화는 좋지만, (participantId, restaurantId) 중복 방지와 탐색 성능을 위해 제약/인덱스를 추가하는 게 안전합니다. 또한 DB FK는 JPA 매핑 없이도 Flyway로 걸어두면 데이터 무결성에 도움이 됩니다.
예시 diff:
-@Entity -@Getter -@NoArgsConstructor(access = AccessLevel.PROTECTED) +@Entity +@Getter +@NoArgsConstructor(access = AccessLevel.PROTECTED) +@jakarta.persistence.Table( + name = "restaurant_like", + uniqueConstraints = { + @jakarta.persistence.UniqueConstraint( + name = "uk_restaurant_like_participant_restaurant", + columnNames = {"participantId", "restaurantId"} + ) + }, + indexes = { + @jakarta.persistence.Index(name = "idx_restaurant_like_participant_id", columnList = "participantId"), + @jakarta.persistence.Index(name = "idx_restaurant_like_restaurant_id", columnList = "restaurantId") + } +) public class RestaurantLike extends BaseEntity { @@ - public RestaurantLike(Long participantId, Long restaurantId) { - this.participantId = participantId; - this.restaurantId = restaurantId; - } + public RestaurantLike(Long participantId, Long restaurantId) { + this.participantId = java.util.Objects.requireNonNull(participantId, "participantId"); + this.restaurantId = java.util.Objects.requireNonNull(restaurantId, "restaurantId"); + }마이그레이션 제안(Flyway):
- UK(참가자ID, 식당ID) 추가
- FK(옵션): participantId → participant(id), restaurantId → restaurant(id)
테스트 제안:
- 동일 (participantId, restaurantId) 중복 저장 시 예외 발생 검증
- participantId/restaurantId 각각으로 조회 성능(인덱스 사용 여부)은 통계로 확인 가능(운영 시).
(backend/** 가이드라인)backend/src/main/java/com/pickeat/backend/pickeat/domain/repository/PickeatJpaRepository.java (2)
14-14: 파라미터명 오타(rooId) 정정 권장기능엔 영향 없지만 가독성과 일관성을 위해 roomId로 맞춰주세요.
- List<Pickeat> findByRoomId(Long rooId); + List<Pickeat> findByRoomId(Long roomId);As per coding guidelines.
18-18: 파라미터명 오타(roodIds) 및 컬렉션 타입 개선
- 변수명: roomIds로 정정
- IN 절 파라미터는 Collection이 유연합니다.
- List<Pickeat> findByRoomIdIn(List<Long> roodIds); + List<Pickeat> findByRoomIdIn(Collection<Long> roomIds);운영 팁: updated_at, room_id 기반 조회가 빈번하면 적절한 인덱스(예: updated_at, room_id+is_active)를 Flyway에 반영하세요.
As per coding guidelines.backend/src/main/java/com/pickeat/backend/restaurant/domain/Restaurant.java (2)
23-24: FK 컬럼 인덱스 확인pickeat_id로 조회/정리가 빈번하므로 DB 인덱스가 있는지 확인해주세요(Flyway 반영). 캐시 미스/만료 시에도 비용을 줄일 수 있습니다.
As per coding guidelines.
77-82: 메서드 명세-의미 불일치(nit):getPictureUrls가 단수 URL 반환현재는 단일 String을 반환합니다. 과거 호환이 필요 없다면 getPictureUrl로 이름을 다듬는 것도 가독성에 좋아요. 유지가 필요하면 Javadoc으로 형식(콤마 구분 등)을 명시해주세요.
As per coding guidelines.backend/src/test/java/com/pickeat/backend/pickeat/application/PickeatSchedulerTest.java (2)
64-75: 테스트 구성 좋아요! 정리 전/후 count와 존재 여부를 함께 검증한 점 굿추가 아이디어(선택):
- 연관 데이터가 없는 오래된 Pickeat도 안전하게 정리되는지(빈 케이스)
- 최근 Pickeat 경계값(예: D-2, D-3 기준 변곡점)
86-91: ID 기반 리팩토링 테스트도 깔끔합니다Participant/RestaurantLike/PickeatResult를 ID로 연결하는 흐름을 잘 반영했습니다. 캐시 무효화까지 포함한 통합 테스트가 있다면(이벤트 발행→캐시 clear 확인) 더 탄탄해집니다.
As per coding guidelines.backend/src/test/java/com/pickeat/backend/tobe/user/application/UserServiceTest.java (1)
70-74: ProviderPrincipal 전환 테스트 적절createUser 시그니처 변경을 잘 검증했습니다. 닉네임 트리밍/길이 제약 등 경계 케이스도 후속으로 추가하면 좋겠습니다.
backend/src/main/java/com/pickeat/backend/global/auth/principal/ProviderPrincipal.java (1)
3-5: Principal 객체의 null 안정성 검토 필요record 타입으로 간결하게 정의한 점은 좋습니다! 다만 인증 Principal로 사용되는 핵심 객체이므로, 필드에 대한 null 검증을 고려해보면 어떨까요?
제안:
public record ProviderPrincipal(Long providerId, String provider) { public ProviderPrincipal { if (providerId == null || provider == null) { throw new IllegalArgumentException("providerId와 provider는 null일 수 없습니다."); } } }compact constructor를 활용하면 생성 시점에 불변식을 보장할 수 있습니다. 인증 컨텍스트에서 예상치 못한 null로 인한 NPE를 사전에 방지할 수 있어요.
참고: Java record의 compact constructor 패턴은 불변 객체의 유효성 검증에 적합합니다.
backend/src/main/java/com/pickeat/backend/pickeat/domain/ParticipantStateUpdatedEvent.java (1)
3-4: 도메인 이벤트 정의 완료, 문서화 추가 권장record 타입으로 간결하게 이벤트를 정의한 점은 좋습니다. 다만 도메인 이벤트는 시스템의 흐름을 이해하는 데 중요한 요소이므로, 추가 개선을 제안합니다.
제안 1: 문서화 추가
/** * 참가자의 상태가 변경되었을 때 발행되는 도메인 이벤트. * <p> * 이 이벤트는 참가자의 선택 완료/취소 시 발행되며, * Pickeat 관련 캐시를 무효화하는 데 사용됩니다. * * @param pickeatId 상태가 변경된 참가자가 속한 Pickeat의 ID */ public record ParticipantStateUpdatedEvent(Long pickeatId) { }제안 2: null 안정성 보장
public record ParticipantStateUpdatedEvent(Long pickeatId) { public ParticipantStateUpdatedEvent { if (pickeatId == null) { throw new IllegalArgumentException("pickeatId는 null일 수 없습니다."); } } }도메인 이벤트는 여러 컴포넌트가 의존하는 계약(contract)이므로, 명확한 문서화와 불변식 보장이 중요합니다.
참고: Spring의
@EventListener나@TransactionalEventListener와 함께 사용될 경우, 이벤트 발행 시점과 트랜잭션 경계도 문서화하면 더욱 좋습니다.backend/src/test/java/com/pickeat/backend/fixture/RestaurantFixture.java (1)
19-21: 픽잇 ID null 방지 + 중복 제거(DRY)
- 테스트 픽스처라도
pickeat.getId()가 null이면 NPE가 납니다. 픽스처에서 빠르게 실패하도록 선행 검증을 넣어 주세요.create와createWithName가 거의 동일합니다. 한쪽에서 다른쪽을 호출해 중복을 제거하면 유지보수성이 좋아집니다.예시:
public static Restaurant create(Pickeat pickeat) { - return new Restaurant( - "식당", - FoodCategory.KOREAN, - 10, - "도로명 주소", - "URL", - "태그1,태그2", - null, - null, - pickeat.getId() - ); + return createWithName(pickeat, "식당"); } public static Restaurant createWithName(Pickeat pickeat, String name) { + if (pickeat.getId() == null) { + throw new IllegalStateException("Fixture mis-use: pickeat must be persisted and have an id"); + } return new Restaurant( name, FoodCategory.KOREAN, 10, "도로명 주소", "URL", "태그1,태그2", null, null, pickeat.getId() ); }Also applies to: 23-35
backend/src/test/java/com/pickeat/backend/tobe/acceptance_test/scenario/PickeatByTemplateScenarioTest.java (5)
61-66: 테스트 이름 오타 수정
텤플릿→템플릿으로 정정하면 검색성과 가독성이 좋아집니다.- void 텤플릿_기반_픽잇_플로우() { + void 템플릿_기반_픽잇_플로우() {
64-69: 전제 조건 검증 추가(예상 데이터 보장)템플릿/식당 목록 크기에 대한 전제 조건을 명시적으로 확인하면 시나리오 테스트의 플레이키를 줄일 수 있어요.
List<TemplateResponse> templates = 템플릿_목록_조회(); +assertThat(templates).isNotEmpty(); ... List<RestaurantResponse> restaurants = 픽잇의_식당_조회(pickeat.code(), null, participant1Token.token()); +assertThat(restaurants).hasSizeGreaterThanOrEqualTo(3);
115-120: 제외 식당 검증 강화(부분 포함 → 전부 포함)
containsAnyElementsOf는 일부만 포함돼도 통과합니다. 정확히 제외한 3개 모두가isExcluded=true인지 검증하도록 강화해주세요.-assertThat(restaurants) - .filteredOn(RestaurantResponse::isExcluded) - .extracting(RestaurantResponse::id) - .containsAnyElementsOf(excludedRestaurantIds); +assertThat(restaurants) + .filteredOn(RestaurantResponse::isExcluded) + .extracting(RestaurantResponse::id) + .containsAll(excludedRestaurantIds);
94-99: 인덱스 접근 전 크기 확인으로 IOB 방지좋아요/취소에서
noneExcludedRestaurant.get(0/1)을 사용하므로 최소 2개 존재를 보장해 주세요.List<RestaurantResponse> noneExcludedRestaurant = 픽잇의_식당_조회(pickeat.code(), false, participant1Token.token()); +assertThat(noneExcludedRestaurant).hasSizeGreaterThanOrEqualTo(2);
126-128: 결과 비교: 구조적 동등성 외에 핵심 필드도 명시 검증동등성 비교가 DTO
equals에 의존합니다. 회귀를 줄이려면 핵심 필드를 명시적으로도 비교하면 좋아요(예: 우승 식당 ID, 좋아요 수 합산 등).private void checkoutPickeatResult(PickeatResultResponse actual, PickeatResultResponse expected) { assertThat(actual).isEqualTo(expected); + assertThat(actual.restaurantId()).isEqualTo(expected.restaurantId()); + assertThat(actual.likeCount()).isEqualTo(expected.likeCount()); }backend/src/main/java/com/pickeat/backend/global/auth/annotation/ParticipantInPickeat.java (1)
12-15: 주석으로 기대 타입 명시 + 이전 어노테이션 이행 경로 제공
- 이 파라미터 어노테이션이 바인딩하는 기대 타입(예:
ParticipantPrincipal)을 Javadoc에 명시하면 IDE 힌트와 협업에 도움이 됩니다.- 기존
@ParticipantId를 사용하던 코드가 있다면, 동일한 리졸버를 재사용하는@Deprecated어노테이션을 한 버전 유지해 점진적 마이그레이션 경로를 제공하는 것도 좋아요. (차기 릴리스에서 제거 예고)예시:
-@Documented -public @interface ParticipantInPickeat { +/** + * 컨트롤러 메서드 파라미터를 인증된 참가자 컨텍스트로 주입합니다. + * 기대 타입: ParticipantPrincipal + */ +@Documented +public @interface ParticipantInPickeat {backend/src/main/java/com/pickeat/backend/user/application/UserService.java (1)
30-35: ---DB 유니크 제약이 이미 존재하므로 마이그레이션 제안 제거, 예외 처리는 선택적 개선안으로 재분류
좋은 소식입니다!
V1__init.sql을 확인해보니 라인 23에UNIQUE KEYUK2ty1xmrrgtn89xt7kyxx6ta7h(nickname)가 이미 적용되어 있네요. 따라서 Flyway 마이그레이션 추가 제안은 불필요합니다.다만 TOCTOU 경합 위험은 여전히 고려할 만합니다. DB 유니크 제약 +
validateDuplicateNickname선행 검사가 있더라도, 동시 요청에서 수동 검사를 우회하는 경우를 대비하면 좋습니다:예외 매핑 추가 (선택사항)
@Transactional public UserResponse createUser(SignupRequest request, ProviderPrincipal providerPrincipal) { validateDuplicateNickname(request.nickname()); - User user = new User(request.nickname(), providerPrincipal.providerId(), providerPrincipal.provider()); - userRepository.save(user); - return UserResponse.from(user); + try { + User user = new User(request.nickname(), providerPrincipal.providerId(), providerPrincipal.provider()); + userRepository.save(user); + return UserResponse.from(user); + } catch (DataIntegrityViolationException e) { + throw new BusinessException(ErrorCode.ALREADY_NICKNAME_EXISTS); + } }추가로,
provider필드를 도메인 타입(예: enum)으로 일치시키면 문자열 오입력을 방지할 수 있습니다.
backend/src/main/java/com/pickeat/backend/pickeat/application/PickeatResultGenerator.java (1)
16-23: 랜덤 선택 로직이 명확합니다. 다만 테스트 용이성 개선을 고려해보세요.
Collections.shuffle()을 사용한 랜덤 선택은 비즈니스 요구사항에 잘 부합하지만, 테스트에서 재현 가능성(reproducibility)이 떨어질 수 있습니다.개선 제안 (선택사항):
- 테스트 용이성을 위해
Random인스턴스를 주입받도록 변경하면, 테스트에서 시드를 고정할 수 있습니다.@Component class PickeatResultGenerator { private final Random random; PickeatResultGenerator() { this(new Random()); } @VisibleForTesting PickeatResultGenerator(Random random) { this.random = random; } public Restaurant generate(Map<Restaurant, Integer> likeCounts) { validateNotEmpty(likeCounts); List<Restaurant> topRatedRestaurants = getTopRatedRestaurants(likeCounts); Collections.shuffle(topRatedRestaurants, random); return topRatedRestaurants.getFirst(); } }참고: 현재 테스트가
isIn()같은 방식으로 검증하고 있다면 현재 구현도 충분합니다!backend/src/test/java/com/pickeat/backend/pickeat/application/PickeatResultGeneratorTest.java (1)
30-132: 테스트 커버리지가 훌륭합니다! 추가 엣지 케이스를 고려해보세요.주요 시나리오(단건 최고, 동점, 빈 리스트, 모두 0점, 다양한 선호도)를 모두 커버했네요. 랜덤성은
isIn()으로 적절히 검증했습니다.추가 고려 사항:
- 음수 좋아요 수:
likeCount가 음수인 경우 (방어 로직 필요 여부)- 대용량 데이터: 매우 많은 식당이 동점일 때 성능 (현재는 shuffle이 있어서 OK)
- null 값: Map에 null 키나 값이 있을 때 (현재 Java는 Map.of()로 null 불가하지만 일반 Map의 경우)
현재 테스트로도 충분하지만, 방어 로직이 추가된다면 해당 케이스도 테스트하면 좋겠습니다!
backend/src/test/java/com/pickeat/backend/pickeat/application/ParticipantServiceTest.java (1)
40-52: 테스트 검증을 좀 더 충실하게 다져보세요.현재는 response가 null이 아닌지만 확인하고 있네요. 생성된 참가자가 실제로 저장되었는지, 토큰이 유효한지까지 검증하면 더 견고한 테스트가 될 겁니다.
개선 제안:
// then assertThat(response).isNotNull(); assertThat(response.token()).isNotBlank(); // 추가: 실제 Participant 저장 검증 testEntityManager.flush(); testEntityManager.clear(); Participant createdParticipant = testEntityManager.find(Participant.class, participant.getId()); assertThat(createdParticipant.getNickname()).isEqualTo("테스트유저");참고로
pastCount필드는 현재 Participant 엔티티에 없으므로 그 부분은 신경 쓰지 않으셔도 좋습니다. 위의updateCompletion테스트처럼 실제 상태 변화를 DB에서 직접 조회하여 검증하는 패턴이 우테코의 통상적인 접근이니 참고하세요! 🎯backend/src/main/java/com/pickeat/backend/restaurant/infrastructure/RestaurantJdbcRepository.java (3)
47-54: JDBC null 처리: 드라이버 추론에 의존하지 말고 타입을 명시하세요.
ps.setObject(3, r.getDistance())는 null 시 드라이버가 타입을 추론해야 합니다. DB/드라이버에 따라 NPE·SQLType 불일치가 날 수 있어요. 안전하게 SQL 타입을 명시해 주세요.예시 Diff:
- ps.setObject(3, r.getDistance()); // NULL 허용(Integer) + if (r.getDistance() != null) { + ps.setObject(3, r.getDistance(), java.sql.Types.INTEGER); + } else { + ps.setNull(3, java.sql.Types.INTEGER); + }테스트: 거리(null/값) 각각으로 batchInsert 수행해 insert 성공 및 값 검증(Repository 통합 테스트). [backend 컨벤션 1,6 기반]
35-36: 서버시간 vs DB시간: created_at/updated_at는 DB 기본값 혹은 CURRENT_TIMESTAMP 활용 고려애플리케이션 서버 시간(
LocalDateTime.now())과 DB 시간대가 어긋나면 정렬/만료 로직이 흔들릴 수 있어요. 컬럼에 기본값(now())가 있다면 SQL에서 생략하거나CURRENT_TIMESTAMP사용을 고려해 주세요.대안:
- 컬럼 default(now()) 설정 + INSERT 컬럼에서 생성/수정시각 제거
- 또는
VALUES (..., CURRENT_TIMESTAMP, CURRENT_TIMESTAMP, ?)형태추가로
deleted도 DB 기본값이 false면 코드에서 명시 제거 가능(간결성↑). [backend 컨벤션 1,3]Also applies to: 67-69
64-66: NPE 우려는 현재 코드 구조상 과도함 - 일관성 관점의 선택적 개선 제안검증 결과 분석:
pickeatId (Long):
create()메서드에서pickeat.getId()는 DB 조회 결과로 반드시 유효합니다. Restaurant 생성자의 필수 파라미터이므로 null이 될 수 없습니다.isExcluded (Boolean): 필드 초기값이
false로 설정되어 있고, 생성자에서 명시적으로 할당하지 않아도 항상 false 또는 true 값을 가집니다.NoArgsConstructor가 실제로 사용되지 않으므로 초기값 미적용 문제도 없습니다.코드 일관성 측면의 제안:
비록 NPE 위험은 낮지만, 같은
batchInsert()메서드 내에서getTags()(6번줄)와Picture(7-8번줄)는 null 체크를 수행하는데,isExcluded와pickeatId는 체크하지 않습니다. 방어적 프로그래밍 관점에서 일관성 있게 처리하는 것을 권장합니다:// 현재 ps.setBoolean(9, r.getIsExcluded()); ps.setLong(10, r.getPickeatId()); // 제안 (선택적) if (r.getIsExcluded() != null) { ps.setBoolean(9, r.getIsExcluded()); } else { ps.setNull(9, java.sql.Types.BOOLEAN); } if (r.getPickeatId() != null) { ps.setLong(10, r.getPickeatId()); } else { ps.setNull(10, java.sql.Types.BIGINT); }현재 코드가 동작하지만, 향후 도메인 설계 변경 시 문제를 조기에 방지할 수 있습니다. 🎯
backend/src/main/java/com/pickeat/backend/global/cache/CacheType.java (2)
8-12: 캐시 이름 규칙 통일 제안(colon 사용 일관화)
restaurant:like-count,participant:likes는 콜론 네이밍,pickeat,restaurant는 단일 토큰이에요. 미터링/모니터링, 키 프리픽스 관리에 유리하도록 네이밍 규칙을 통일해 주세요(예: 모두도메인:리소스).예:
RESTAURANT("restaurant", ...)→RESTAURANT("restaurant:by-id", ...)PICKEAT("pickeat", ...)→PICKEAT("pickeat:by-id", ...)변경 시 CacheManagerConfig와 @Cacheable/@CacheEvict 사용처 전반 동기화 필요. 테스트: 캐시 이름 변경에 따른 스파이/통합 테스트 업데이트. [backend 컨벤션 1,5]
15-25: 시간/용량 단위 명확화: Duration 활용 및 외부 설정화 권장
ttlMinutes(Long)와maxSize(Long)는 단위 오해 소지가 큽니다.Duration를 사용하고, 환경별 튜닝을 위해@ConfigurationProperties로 외부 설정화하세요.예시 Diff:
- private final Long ttlMinutes; + private final java.time.Duration ttl; - CacheType(String name, Long ttlMinutes, Long maxSize, Integer initialCapacity) { + CacheType(String name, java.time.Duration ttl, Long maxSize, Integer initialCapacity) { this.name = name; - this.ttlMinutes = ttlMinutes; + this.ttl = ttl; this.maxSize = maxSize; this.initialCapacity = initialCapacity; }그리고 CacheManagerConfig에서
Caffeine.newBuilder().expireAfterWrite(ttl)로 직접 전달. 운영 팁: prod/stage에서 다른 TTL·size를 application-*.yml로 분리. [backend 컨벤션 1,3,4]backend/src/main/java/com/pickeat/backend/pickeat/infrastructure/ParticipantStateUpdatedEventHandler.java (1)
11-18: 핸들러의 구현체 의존 및 관찰 가능성 개선여기도
ParticipantRepositoryImpl직접 의존입니다. 위 파일과 동일하게 포트/어댑터로 추상화해 주세요. 캐시 무효화가 발생했는지 추적할 수 있도록 debug 로그 또는 카운터 메트릭을 추가하면 운영에서 유용합니다.예시:
participantCachePort.evictByPickeatId(pickeatId);log.debug("Evicted participant cache for pickeatId={}", pickeatId);
테스트: 이벤트 후 해당 키 재조회 시 DB hit를 검증. [backend 컨벤션 1,4]backend/src/main/java/com/pickeat/backend/pickeat/application/ParticipantService.java (1)
47-51: 불필요한 캐시 무효화 방지: 상태 변경 시에만 이벤트 발행완료 여부가 이미 동일한 값일 때도 이벤트를 발행하면 캐시 스톰이 날 수 있어요. 변경이 실제로 발생한 경우에만 이벤트를 발행해 주세요.
예시 Diff(메서드 시그니처 가정):
- participant.updateCompletionAs(isCompleted); - applicationEventPublisher.publishEvent(new ParticipantStateUpdatedEvent(participant.getPickeatId())); + boolean changed = participant.updateCompletionAs(isCompleted); // 변경 여부 리턴하도록 도메인 메서드 조정 권장 + if (changed) { + applicationEventPublisher.publishEvent(new ParticipantStateUpdatedEvent(participant.getPickeatId())); + }대안:
if (participant.isCompleted() != isCompleted) { ... }로 가드. 테스트: 동일 입력 2회 호출 시 이벤트 1회만 발행. [backend 컨벤션 2,6]backend/src/main/java/com/pickeat/backend/pickeat/application/ParticipantTokenProvider.java (2)
38-41: 널 안전성: 클레임 미존재 시 NPE 방지외부에서 들어온 토큰에
pickeatCode가 없을 수도 있어요.getRawPickeatCode에서 null 반환을 허용하되, 사용하는 쪽에서 명확히 가드하거나 Optional로 래핑을 고려해 주세요.예시:
- public String getRawPickeatCode(String token) { - Claims claims = jwtProvider.getClaims(token); - return claims.get(PICKEAT_CODE_CLAIM_KEY, String.class); - } + public java.util.Optional<String> getRawPickeatCode(String token) { + Claims claims = jwtProvider.getClaims(token); + return java.util.Optional.ofNullable(claims.get(PICKEAT_CODE_CLAIM_KEY, String.class)); + }사용처에서
orElseThrow등으로 명시 처리. [backend 컨벤션 2,3]
27-31: 현재 코드 설계는 안전합니다 — 단위 일치 및 상수 관리 검증 완료검증 결과를 정리하면:
expirationMillis 단위: ✓ 문제 없음
JwtProvider.createTokenWithClaims:new Date(now.getTime() + expirationMillis)- 모두 밀리초(ms) 기준으로 정렬되어 있습니다
PICKEAT_CODE_CLAIM_KEY 관리: ✓ 구조적으로 안전
- 상수는
ParticipantTokenProvider내부에서만 정의되고 사용됨 (라인 16, 29, 40)ParticipantIdArgumentResolver는getParticipantId(),getRawPickeatCode()같은 public 메서드를 통해 간접 접근하므로, 상수에 직접 의존하지 않음현재 설계의 장점:
- 토큰 생성/파싱 로직이
ParticipantTokenProvider에 집중되어 일관성 있음- ArgumentResolver는 TokenProvider의 계약(public 메서드)만 의존하여 느슨한 결합 유지
- 각 TokenProvider(
ParticipantTokenProvider,ProviderTokenProvider)가 자신의 상수를 관리하는 것도 책임 분리 측면에서 명확함선택적 개선 제안:
만약 향후 새로운 모듈이 토큰 클레임 키에 직접 접근해야 한다면 (예: 다른 서비스 모듈), 그때AuthClaims같은 공용 상수 인터페이스를 고려해도 좋습니다. 하지만 현재 구조는 충분히 안전하므로 급하게 리팩토링할 필요는 없습니다.backend/src/main/java/com/pickeat/backend/global/cache/CacheManagerConfig.java (2)
31-35: TTL 전략: expireAfterAccess → expireAfterWrite 검토 요청현재 접근 기반 만료(expireAfterAccess)를 사용하면, 무효화 누락 시 “자주 조회되는 오래된 값”이 계속 살아남을 수 있어요. 저장/삭제 이벤트 무효화가 대부분을 커버하더라도, 장애·크론·백필 등 비정형 갱신이 있을 때는 쓰기 기준 만료(expireAfterWrite)가 더 안전한 상한을 보장합니다.
권장:
- 캐시별 정책 필요 시 CacheType에 만료 방식 필드를 추가하고, 기본값은 expireAfterWrite로 두기.
- 단일 정책으로 단순화하려면 expireAfterWrite로 전환.
예시(단일 정책 전환):
- .expireAfterAccess(Duration.ofMinutes(cacheType.getTtlMinutes())) + .expireAfterWrite(Duration.ofMinutes(cacheType.getTtlMinutes()))장단점:
- expireAfterWrite: 신선도 상한 명확(+), 핫 키도 최대 TTL 초과 시 교체(±)
- expireAfterAccess: 핫 키 적중률↑(+), 갱신 누락 시 오래된 데이터 유지(−)
31-35: 캐시 메트릭 비용 관리: recordStats 토글 가능화recordStats는 오버헤드가 있습니다. 운영/부하 테스트에서만 켜도록 프로퍼티로 게이트해주세요.
간단 제안:
@Value("${cache.metrics.enabled:true}")주입 필드 추가if (cacheMetricsEnabled) builder.recordStats();운영에서는 Micrometer CacheMetrics 바인딩으로 자동 노출됩니다(스프링 캐시 메트릭스). Based on learnings
backend/src/main/java/com/pickeat/backend/login/application/ProviderTokenProvider.java (1)
24-28: 만료 단위 명확화 및 프로퍼티 네이밍
@Value("${provider.jwt.expiration}")가 밀리초 단위임을 코드만 보고 추론해야 해요. 설정 오해를 줄이려면 프로퍼티 키를provider.jwt.expiration-millis로 바꾸거나, JSR-310 Duration 타입을 받아Duration.parse(예:PT30M)로 처리하는 방식을 권장합니다. 테스트 구성에도 동일 단위를 명시해주세요.
또한 PROVIDER_CLAIM_KEY가 누락되면(null) 이후 플로우에 영향이 있으니, null 허용/불허 정책을 정하고 검증을 추가하는 것도 좋습니다.backend/src/test/java/com/pickeat/backend/pickeat/infrastructure/PickeatRepositoryImplCachingTest.java (3)
91-101: 프로덕션 캐시와 테스트 캐시의 동작 차이(ConcurrentMapCache vs Caffeine)현재 테스트는 TTL·사이즈·통계가 없는 ConcurrentMapCache를 사용합니다. 운영은 Caffeine이므로 TTL 관련 회귀를 놓칠 수 있어요. 테스트에서도 CaffeineCache를 써서 동작을 맞추는 걸 권장합니다.
예시(간단 교체):
@Bean CacheManager cacheManager() { SimpleCacheManager manager = new SimpleCacheManager(); CaffeineCache cache = new CaffeineCache( PickeatRepositoryImpl.PICKEAT_CACHE_NAME, Caffeine.newBuilder() .expireAfterWrite(Duration.ofMinutes(5)) // 혹은 CacheType의 TTL 참조 .build() ); manager.setCaches(List.of(cache)); return manager; }
54-68: 미존재(EMPTY) 결과 캐싱 정책 테스트 추가 제안
Optional.empty()를 캐시할지 여부가 성능/정합성에 영향을 줍니다. 현재 정책을 명시하고 테스트로 고정해 주세요.추천 케이스:
- findByCode가 empty일 때 재호출 시 JPA 호출 횟수(1회 유지인지 2회인지) 검증
- 추후 정책 변경(negative caching on/off)에 대비한 명시적 테스트 이름 부여
70-89: 무효화 키 검증(코드 기반 키인지 확인)
evictPickeatCache가 코드 기반으로 키를 생성한다면, 동일 코드의 다른 인스턴스로도 캐시가 무효화되어야 합니다. 현재 Mockito로when(cachedPickeat.getCode()).thenReturn(PICKEAT_CODE)만 검증하고 있으니, “서로 다른 객체(동일 코드)”로도 무효화되는지 추가 검증을 부탁드려요.backend/src/test/java/com/pickeat/backend/pickeat/application/PickeatServiceTest.java (3)
181-183: 메서드 네이밍/시그니처 불일치: getPickeatByParticipant인데 participant 인자가 없습니다테스트에서
getPickeatByParticipant(String pickeatCode)를 호출하지만, 이름상 참가자 맥락(Principal/ID)이 필요해 보입니다.
옵션:
- 실제 의도가 “코드로 픽잇 조회”라면
getPickeatByCode(또는getPickeat)로 리네이밍- 참가자 컨텍스트가 필요한 API라면
getPickeatByParticipant(Long participantId, String code)등으로 시그니처 조정이 부분은 서비스/컨트롤러 전반 용어 정합성에 직결됩니다. PR 전체에서 메서드/엔드포인트 명칭을 한 번 정리해보면 좋겠습니다.
58-66: 도메인 생성 일관성: Fixture/Builder 사용 권장루프 내에서
new Participant(nickname, pickeat.getId())로 직접 생성하면 향후 도메인 제약 변경 시 테스트가 쉽게 깨질 수 있어요.ParticipantFixture.create(id)또는 Builder를 일관되게 사용해 도메인 불변식을 캡슐화해 주세요.
424-431: 중복 toString 호출 정리
pickeat.getCode().getValue().toString()에서getValue()가 이미 String/UUID라면toString()중복일 수 있어요. 호출 체인을 간결하게 정리하면 가독성이 좋아집니다.Also applies to: 463-467
backend/src/main/java/com/pickeat/backend/global/auth/JwtProvider.java (2)
66-71: 파서 키 사용 단순화(선택): 기존 secretKey 재사용
verifyWith(Keys.hmacShaKeyFor(secretKey.getEncoded()))대신verifyWith(secretKey)로 단순화 가능합니다. 의미 동일, 가독성↑.- return Jwts.parser() - .verifyWith(Keys.hmacShaKeyFor(secretKey.getEncoded())) + return Jwts.parser() + .verifyWith(secretKey) .build() .parseSignedClaims(token) .getPayload();
29-41: 보안 강화 제안: iss/aud 등 컨텍스트 클레임 추가 검증토큰 혼용 방지를 위해 발급자(iss), 대상자(aud) 등 컨텍스트 클레임을 포함하고 파서에서 검증하는 것을 권장합니다. 만료·서명 외에도 컨텍스트가 맞는지 확인하면 “토큰 혼동(confusion)” 위험이 줄어듭니다. 구성 예:
- 발급:
.issuer(issuer).audience().add(audience)...- 검증: 파서에서 요구 클레임 매칭(필요 시 커스텀 검증)
테스트: 잘못된 iss/aud 토큰이 INVALID_TOKEN으로 처리되는지. Based on learnings
backend/src/main/java/com/pickeat/backend/pickeat/application/dto/response/PickeatResultResponse.java (1)
34-45: Null-안전성 점검 권장(FoodCategory 등)
restaurant.getFoodCategory()가 null일 가능성이 있다면 NPE가 납니다. 도메인 제약으로 null 불가라면 괜찮지만, 그게 아니라면 방어 로직을 추가하거나 사전 검증이 필요합니다.backend/src/test/java/com/pickeat/backend/restaurant/infrastructure/RestaurantRepositoryImplCachingTest.java (1)
44-51: 캐시 이름 하드코딩을 Repository 상수/중앙 CacheType으로 통일하세요.현재
"restaurant"리터럴을 사용합니다. 운영 코드의 캐시 이름 변경 시 테스트가 조용히 깨지거나(혹은 반대로) 드리프트가 발생할 수 있어요. Repository 상수(예:RestaurantRepositoryImpl.RESTAURANT_CACHE_NAME) 또는 중앙CacheType을 사용하면 단일 소스 오브 트루스로 관리됩니다.- Cache cache = cacheManager.getCache("restaurant"); + Cache cache = cacheManager.getCache(RestaurantRepositoryImpl.RESTAURANT_CACHE_NAME); ... - manager.setCaches(List.of(new ConcurrentMapCache("restaurant"))); + manager.setCaches(List.of(new ConcurrentMapCache(RestaurantRepositoryImpl.RESTAURANT_CACHE_NAME)));이 PR에서 CacheManager 중앙화가 도입되었다면 그 네이밍을 참조하도록 맞추는 것을 권장합니다. As per coding guidelines.
Also applies to: 98-103
backend/src/main/java/com/pickeat/backend/restaurant/application/RestaurantService.java (5)
58-65: 좋아요 카운트 N+1 쿼리 발생반복문 내
countAllByRestaurantId(...)호출로 레스토랑 수만큼 쿼리가 발생합니다. 배치 집계로 바꾸면 효율이 크게 개선됩니다.권장 방향:
- Repository에 배치 집계 메서드 추가(예:
Map<Long,Integer> countAllByRestaurantIdIn(Set<Long> ids)), 한 번의 쿼리로 집계.- 없으면 JDBC/JPQL로 프로젝션 반환해서 매핑.
예시:
- Set<Long> likedRestaurantIds = getLikedRestaurantIdsByParticipantId(participantId); + Set<Long> likedRestaurantIds = (participantId == null) ? Set.of() : getLikedRestaurantIdsByParticipantId(participantId); - List<RestaurantResponse> response = new ArrayList<>(); - for (Restaurant restaurant : targets) { - boolean isLiked = likedRestaurantIds.contains(restaurant.getId()); - Integer likeCount = restaurantLikeRepository.countAllByRestaurantId(restaurant.getId()); - response.add(RestaurantResponse.of(restaurant, likeCount, isLiked)); - } + Set<Long> targetIds = targets.stream().map(Restaurant::getId).collect(Collectors.toSet()); + Map<Long, Integer> likeCounts = restaurantLikeRepository.countAllByRestaurantIdIn(targetIds); // 신규 배치 메서드 + List<RestaurantResponse> response = new ArrayList<>(targets.size()); + for (Restaurant restaurant : targets) { + boolean isLiked = likedRestaurantIds.contains(restaurant.getId()); + int likeCount = likeCounts.getOrDefault(restaurant.getId(), 0); + response.add(RestaurantResponse.of(restaurant, likeCount, isLiked)); + }테스트 케이스: 레스토랑 100개 시 단 1쿼리만 나가는지, 빈 리스트/누락 ID 처리. As per coding guidelines.
58-59: participantId null 처리 누락 가능성비로그인/참가자 미지정 시
findAllByParticipantId(null)호출 위험이 있습니다. null이면 빈 Set으로 처리하면 안전합니다.- Set<Long> likedRestaurantIds = getLikedRestaurantIdsByParticipantId(participantId); + Set<Long> likedRestaurantIds = (participantId == null) ? Set.of() : getLikedRestaurantIdsByParticipantId(participantId);
81-83: 불필요한 전체 saveAll 호출변경된 대상만 저장하는 것이 I/O·경합 측면에서 유리합니다.
- targets.forEach(Restaurant::exclude); - restaurantRepository.saveAll(restaurants); + targets.forEach(Restaurant::exclude); + restaurantRepository.saveAll(targets);
145-148: 에러 코드 의미가 도메인 동작과 어긋납니다접근 검증 실패 시
RESTAURANT_ELIMINATION_FORBIDDEN는 “제외/삭제” 맥락입니다. 좋아요/조회 등 공통 접근 제어에는 보다 일반적인 코드(예:FORBIDDEN또는RESTAURANT_ACCESS_FORBIDDEN)를 쓰면 가독성과 유지보수성이 좋아집니다.
69-83: 참가자 권한 검증 범위 재확인 권장
exclude(...)는ParticipantPrincipal에서 pickeatCode만 사용합니다. 참가자 소속/권한(방장 등)을 컨트롤러/리졸버에서 이미 강제한다면 OK지만, 서비스 레이어에서도 방어 검증(요청한 참가자가 해당 픽잇 소속인지, 권한이 있는지)을 추가하면 안전합니다.backend/src/main/java/com/pickeat/backend/restaurant/ui/api/RestaurantApiSpec.java (1)
3-3: ParticipantPrincipal 기반으로의 전환 좋습니다!스펙과 실제 인증 흐름이 일치합니다. Swagger 문서에 인증 실패 케이스를 드러내면 클라이언트 UX가 좋아집니다.
- 제안: 보안이 필요한 4개 엔드포인트에 401/403 응답 스키마 추가
예시:
@ApiResponses({ @ApiResponse(responseCode = "204", description = "성공"), @ApiResponse(responseCode = "401", description = "인증 필요"), @ApiResponse(responseCode = "403", description = "권한 없음") })또한 컨트롤러에서는 서비스로
participantPrincipal.id()만 전달해 서비스 레이어가 Principal에 종속되지 않게 유지하는 것을 권장합니다.As per coding guidelines
Also applies to: 147-147, 180-181, 234-235, 279-279
backend/src/test/java/com/pickeat/backend/restaurant/application/RestaurantServiceTest.java (2)
85-103: 소거 흐름 테스트 보강 제안(필터·권한)현재는 소거 성공/비활성 예외만 검증합니다. 아래 보강을 추천해요.
- isExcluded 필터 3가지 모드(null/true/false) 각각 검증
- 다른 픽잇의 참가자가 소거를 시도할 때 권한 거부 확인
원하시면 케이스별 템플릿 제공해 드릴게요.
As per coding guidelines
Also applies to: 120-125
135-148: 좋아요/취소 경계 케이스 보강이미 중복 좋아요 예외는 검증되어 있어요. 추가로:
- 좋아요하지 않은 상태에서 취소 시 예외
- 동시성(같은 참가자/다른 참가자) 더블클릭 방지 보장
getPickeatRestaurants가 좋아요 플래그를 정확히 반영하는지 캐시 적중/미적중 상황 각각 확인필요 시 테스트 스캐폴딩 드리겠습니다.
As per coding guidelines
Also applies to: 171-189, 208-234
backend/src/main/java/com/pickeat/backend/pickeat/ui/ParticipantController.java (1)
39-42: 서비스 시그니처 일관화 제안(Principal vs ID)
getParticipant은 ID를, 완료/취소는ParticipantPrincipal을 서비스에 전달합니다. 서비스 레이어가 인증 세부사항에 종속되지 않도록 한 방향으로 통일(가급적 Principal 수용)하면 확장·테스트가 쉬워집니다.예:
participantService.getParticipant(participantPrincipal)형태로 일원화.As per coding guidelines
Also applies to: 47-56
backend/src/main/java/com/pickeat/backend/restaurant/domain/repository/RestaurantLikeRepository.java (1)
13-13: 카운트 타입은long권장좋아요 수는 누적치가 커질 수 있어
Integer보다long이 안전합니다. JPA/DB 카운트도 보통long을 반환합니다.- Integer countAllByRestaurantId(Long restaurantId); + long countAllByRestaurantId(Long restaurantId);테스트/서비스에서 타입만 맞춰주면 영향은 제한적입니다.
As per coding guidelines
backend/src/test/java/com/pickeat/backend/acceptance_test/scenario/PickeatByPositionScenarioTest.java (1)
99-106: 결과 DTO 전환 굿! + 하드코딩된 subList 경계 보강
PickeatResultResponse로의 전환 👍 깔끔합니다.- 다만
subList(0,4),(3,7),(6,10)은 식당 수가 기대보다 적을 때 테스트가 깨질 수 있어요. 크기에 따라 안전하게 컷팅하도록 보강을 권장합니다.아이디어:
int size = restaurantIds.size(); int e1 = Math.min(4, size); int e2 = Math.min(7, size); int e3 = Math.min(10, size); // 시작 인덱스도 size 체크 후 조건부 실행 또는 max(?, size)로 방어필요하시면 안전한 분할 헬퍼 제공하겠습니다.
Also applies to: 120-122
backend/src/main/java/com/pickeat/backend/pickeat/application/PickeatService.java (3)
68-71: 메서드 명/의미 불일치
getPickeatByParticipant(String pickeatCode)는 참가자 검증 없이 코드로만 조회합니다. 혼동을 피하려면:
- 실제 의미에 맞게
getPickeat(String pickeatCode)로 rename, 또는- 참가자(Principal/ID)를 인자로 받아 접근 검증을 수행
두 번째가 보안·일관성 측면에서 더 안전합니다.
As per coding guidelines
90-99: 쿼리 1회로 축소 제안(Rejoin 가능 여부)현재 2쿼리(참가자→픽잇)입니다. 아래와 같이 “존재 여부”로 한 번에 확인 가능합니다.
- Repository에 다음 형태 추가(예시):
boolean existsByParticipantIdAndPickeatCode(Long participantId, PickeatCode code);- JPQL:
select count(p)>0 from Participant p join Pickeat k on k.id=p.pickeatId where p.id=:pid and k.code=:code그 후 서비스:
boolean rejoinAvailable = participantId != null && participantPickeatRepository.existsByParticipantIdAndPickeatCode(participantId, new PickeatCode(pickeatCode)); return new PickeatRejoinAvailableResponse(rejoinAvailable);또한
Boolean rejoinAvailable→boolean으로 원시 타입 사용을 권장합니다.As per coding guidelines
Also applies to: 114-117
108-112: 메서드 네이밍 정합성단수 반환인데
getPickeatsByParticipant(복수)입니다.getPickeatByParticipant로 변경하면 읽기가 좋아집니다.backend/src/main/java/com/pickeat/backend/tobe/restaurant/application/RestaurantService.java (3)
41-55: 생성 후 캐시 전략 명확화 필요 (+TODO 대응)“생성 시점에 미리 캐싱” TODO가 있는데, 현재는 무효화/워밍이 없습니다. 대량 생성 후 첫 요청이 N번의 DB 조회/카운팅을 유발할 수 있어요.
권장:
- Pickeat 기준으로 레스토랑 목록 캐시를 워밍하거나,
- 결과 이벤트를 발행해 관련 캐시를 일괄 무효화/리프레시.
Spring Cache 레퍼런스의 CacheEvict(allEntries, beforeInvocation) 및 Caffeine refreshAfterWrite를 참고해보세요.
59-68: N+1 형태의 좋아요 카운트 조회레스토랑마다 countAllByRestaurantId 호출은 리스트 크기만큼 DB/캐시 조회를 유발합니다. 캐시가 비어있거나 TTL 만료 시 비용이 커집니다.
대안:
- repository에 countAllByRestaurantIdIn(Set) 형태의 집계 API를 추가해 한 번에 그룹바이 조회.
- 최소한 likeCount 조회를 병렬 스트림 대신 배치(한 쿼리)로 바꾸기.
추가로, 이 메서드도 ParticipantPrincipal을 받아 일관된 인증 컨텍스트를 유지하는 것이 안전합니다.
149-151: 에러 코드 의미 불일치(경미)좋아요 접근 검증에서 RESTAURANT_ELIMINATION_FORBIDDEN를 던지면, API 소비자에게 혼동을 줄 수 있습니다. 맥락에 맞는 PICKEAT_ACCESS_DENIED 등으로 통일하는 것을 권장합니다.
backend/src/main/java/com/pickeat/backend/pickeat/application/PickeatResultService.java (1)
98-105: 좋아요 카운트 집계의 배치화 제안레스토랑 개수만큼 count 호출이 발생합니다. 캐시가 비었을 때 비용이 큽니다.
대안:
- repository에 countAllByRestaurantIdIn(Collection) 추가 후 한 번의 그룹바이로 조회.
- 생성된 Map은 Restaurant 대신 Long id를 키로 사용하면 equals/hashCode 의존도를 낮출 수 있습니다.
예시:
Map<Long, Integer> likeCounts = restaurantLikeRepository.countAllByRestaurantIdIn(ids);backend/src/main/java/com/pickeat/backend/global/auth/ParticipantIdArgumentResolver.java (1)
24-27: 전반적으로 잘 정리됐습니다 👏Annotation + 타입 체크, 토큰 파싱 로직이 명확합니다. 소소한 개선만 제안해요.
- 메서드 명확성: getParticipantIdByHeader → getPrincipalByHeader로 변경하면 반환 타입과 일치합니다.
- 상수 재사용: 헤더 이름 "Pickeat-Participant-Token"과 "Bearer "는 AuthConstants 등으로 중앙화하면 컨트롤러/필터와 불일치가 줄어듭니다.
- 테스트 보강:
- required=true에서 헤더 미존재 시 예외,
- 잘못된 prefix,
- 정상 케이스에서 participantId/rawPickeatCode 매핑 검증.
원하시면 슬라이스 테스트 예시를 드릴게요.
Also applies to: 31-47, 53-58
backend/src/test/java/com/pickeat/backend/restaurant/infrastructure/RestaurantLikeRepositoryImplCachingTest.java (3)
88-118: save 이후 캐시 무효화 검증 좋습니다 — 키 검증 추가 제안현재 상호작용 횟수로 무효화를 확인하고 있어 충분합니다. 추가로 cacheManager.getCache("restaurant:like-count").get(restaurantId) 등을 점검하면 키 수준에서도 확실히 드러납니다(구현 키 전략이 기본이라면 파라미터 값).
또한 테스트에서 캐시 이름은 상수화(CacheType 사용)하면 운영 코드와 드리프트를 줄일 수 있어요.
120-148: delete 경로도 잘 커버됐습니다삭제 시 양쪽 캐시가 무효화되는 것을 상호작용 카운트로 검증하는 접근이 합리적입니다. 보완으로 “삭제가 이미 없는 경우(idempotent)에도 예외 없이 캐시 무효화가 수행되는지” 테스트를 추가해보세요.
150-169: 테스트 구성 적절 — 매직 스트링 최소화 권장SimpleCacheManager 구성과 Mockito 주입이 간결합니다. 운영 코드의 CacheType/CacheManager 설정과 이름을 맞추기 위해, 테스트도 동일한 상수를 참조하도록 정리하면 추후 리팩토링에 강해집니다.
backend/src/main/java/com/pickeat/backend/restaurant/domain/repository/RestaurantRepository.java (1)
16-21: 메서드 명과 반환 타입 불일치findIdsByPickeatIdIn은 “Ids”를 암시하지만 List를 반환합니다. 혼란을 유발해요.
대안(둘 중 하나):
- 실제 의도대로 반환 타입을 List로 변경.
- 혹은 메서드명을 findByPickeatIdIn으로 바꾸고 Restaurant 반환을 유지.
점진적 이전을 원하면 새 메서드 추가 후 기존은 @deprecated로 표시하세요.
backend/src/main/java/com/pickeat/backend/tobe/restaurant/ui/api/RestaurantApiSpec.java (1)
245-246: ParticipantPrincipal 주입 방식 일관화 제안
@Parameter(hidden = true) ParticipantPrincipal사용은 문서에는 숨기면서 실제 주입은 커스텀 리졸버(예:@ParticipantInPickeat)로 처리하는 의도 같아요. 스펙 인터페이스에도 의미를 드러내기 위해 메타 애너테이션(예:@ParticipantInPickeat)을 함께 표기하거나, 컨트롤러 구현부 파라미터에만 리졸버 애너테이션을 부여하는 방식 중 하나로 일관성을 맞추면 혼선을 줄일 수 있습니다. 또한 SecurityRequirement 이름(ParticipantAuth)과 실제 스키마 설정이 매칭되는지 한 번 더 점검해주세요.Also applies to: 298-299, 372-373, 436-437
backend/src/main/java/com/pickeat/backend/restaurant/infrastructure/RestaurantRepositoryImpl.java (1)
30-33: 캐시 스탬피드 완화 제안고동시 요청에서 동일
pickeatId미스가 겹치면 DB 부하가 급증할 수 있어요.@Cacheable(sync = true)로 단일 키에 대해 하나의 쓰레드만 로딩하게 하면 완화됩니다(Caffeine 지원).- @Cacheable(value = RESTAURANT_CACHE_NAME, key = "#pickeatId") + @Cacheable(value = RESTAURANT_CACHE_NAME, key = "#pickeatId", sync = true)backend/src/main/java/com/pickeat/backend/restaurant/application/dto/response/RestaurantResponse.java (1)
38-40: likeCount의 null 안전성 보강
Integer likeCount가 null이면 응답 스키마가 불안정해집니다. 클라이언트 단 처리 부담을 줄이기 위해 기본값 0을 보장하면 좋아요.간단 수정(팩토리에서 디폴트):
- public static RestaurantResponse of(Restaurant restaurant, Integer likeCount, boolean isLiked) { + public static RestaurantResponse of(Restaurant restaurant, Integer likeCount, boolean isLiked) { + int safeLikeCount = likeCount == null ? 0 : likeCount; return new RestaurantResponse( @@ - parsePictureUrls(restaurant.getPictureUrls()), - likeCount, + parsePictureUrls(restaurant.getPictureUrls()), + safeLikeCount, isLiked); }장기적으로는 레코드 필드를
int likeCount로 두어 스키마를 강제하는 것도 고려해주세요(마이그레이션 비용 有).Also applies to: 45-58
backend/src/main/java/com/pickeat/backend/login/ui/LoginController.java (1)
35-41: 의도 확인: 미가입 사용자에 401과 함께 토큰 반환
processCode에서 사용자가 없을 때도 토큰을 바디에 담아 401을 반환합니다. 가입 플로우(프론트가 이 토큰으로 가입 진행)라면 OK지만, 보안·제품 관점에서 합의된 정책인지 재확인 부탁드려요. 선택적으로 실패 케이스에 경량 감사 로그를 남겨 추적성을 높이면 운영에 도움이 됩니다.backend/src/main/java/com/pickeat/backend/restaurant/infrastructure/RestaurantLikeRepositoryImpl.java (2)
57-59: 캐시 이름 상수 일관성여기만 문자열 리터럴(
"participant:likes")을 사용하고 있어요. 상수로 통일하면 오타·불일치 위험이 줄어듭니다.- @Cacheable(value = "participant:likes", key = "#participantId") + @Cacheable(value = PARTICIPANT_LIKES_CACHE_NAME, key = "#participantId")
46-49: 스탬피드 완화 및 카운트 타입
- 다중 요청 동시 미스 시 DB 부하를 줄이기 위해
sync = true를 고려해주세요.- @Cacheable(value = RESTAURANT_LIKE_COUNT_CACHE_NAME, key = "#restaurantId") + @Cacheable(value = RESTAURANT_LIKE_COUNT_CACHE_NAME, key = "#restaurantId", sync = true)
Integer대신int(또는 상한을 고려하면long) 반환을 권장합니다. 카운트는 null이 될 수 없고, 장기적으로 누적치가 커질 수 있습니다.backend/src/test/java/com/pickeat/backend/pickeat/application/PickeatResultServiceTest.java (3)
59-77: 동률/무득표 케이스 테스트도 추가해 랜덤성으로 인한 플래키를 방지해요현재는 좋아요 1건(restaurant2)만 검증합니다. 다음 케이스를 추가하면 생성 로직의 경계값을 튼튼히 커버할 수 있어요.
- 동률 최고 likeCount(예: r1, r2가 동일 최댓값): 결과 id가 {r1.id, r2.id} 중 하나임을 검증. 랜덤성은 테스트에서 시드 고정 또는 허용 집합 검증으로 제어.
- 전원 0 likeCount: 전체 후보군 중 하나를 반환하는지 허용 집합 검증.
장점: 생성기(PickeatResultGenerator)의 정책 변경에도 테스트가 의미를 잃지 않습니다. 필요하면 예시 테스트 코드 드릴게요.
89-100: 멱등성 검증을 DB 차원까지 확장해 주세요같은 입력으로 2회 호출 시 동일 응답을 확인하셨는데, DB에 PickeatResult가 1건만 존재하는지도 함께 보장하면 더 좋습니다. 예: 엔티티 매니저로 count 쿼리 추가 후 1건임을 단언(assert).
172-188: 비활성화 플로우 검증 굿 — 캐시 무효화까지 별도 테스트 권장픽잇 비활성화 상태 단언은 명확합니다. 추가로, 비활성화 이벤트가 캐시 무효화까지 일으키는지(예: findByCode 캐시가 갱신됨) 통합 테스트를 분리해 두면 장애 대응력이 높아집니다. 캐시 매니저 주입 후 측정/검증하는 패턴을 추천드립니다.
backend/src/main/java/com/pickeat/backend/tobe/restaurant/ui/RestaurantController.java (1)
65-72: 서비스 시그니처를 일관화해 가독성과 사용성을 높여요exclude는 ParticipantPrincipal 전체를, like/unlike/조회는 id만 전달합니다. 한 컨트롤러에서 혼합되면 인지 부하가 큽니다.
- 제안: 서비스 계층에서 통일된 형태(ParticipantPrincipal 또는 long id)로 받도록 정리하세요. 인증/인가 검증이 필요하면 Principal 쪽이 확장성에 유리합니다.
장점: API 사용 규칙이 명확해지고, 파라미터 확장 시(예: code, roles) 변경 파급을 줄일 수 있어요.backend/src/main/java/com/pickeat/backend/pickeat/infrastructure/PickeatRepositoryImpl.java (2)
31-44: 파라미터 네이밍 오타(rooId/roodIds) 수정으로 혼란을 줄여요오탈자는 IDE 검색성, 가독성, 온보딩에 비용을 만듭니다. 아래처럼 정리해 주세요.
- public List<Pickeat> findByRoomId(Long rooId) { - return jpaRepository.findByRoomId(rooId); - } + public List<Pickeat> findByRoomId(Long roomId) { + return jpaRepository.findByRoomId(roomId); + } - public List<Pickeat> findByRoomIdIn(List<Long> roodIds) { - return jpaRepository.findByRoomIdIn(roodIds); - } + public List<Pickeat> findByRoomIdIn(List<Long> roomIds) { + return jpaRepository.findByRoomIdIn(roomIds); + }
25-29: 캐시 키·부정 캐싱 정책을 명시해 주세요현재 키는
#pickeatCode.value.toString()입니다. 의도는 “코드 문자열 기준 캐싱”으로 보이는데, 다음을 점검하면 안전합니다.
- 부정 캐싱(Optional.empty())을 허용할지 여부와 TTL. 허용 시 404 폭주 방지에 유리하나, 생성 직후 가시성 지연이 생길 수 있어요.
- 키 생성 유틸(예: PickeatCode.toCacheKey())로 중복 SpEL을 줄이면 유지보수가 편합니다.
결정한 정책을 javadoc/코멘트로 남겨 주세요. 필요하면
unless조건 활용도 검토해요.backend/src/main/java/com/pickeat/backend/pickeat/ui/PickeatController.java (2)
66-71: 메소드 네이밍과 실제 사용 맥락 점검
getPickeatByParticipant(pickeatCode)를 참가자 컨텍스트 없이 호출합니다. 네이밍과 사용 맥락이 어긋나 혼동 여지가 있어요. 실제 요구가 “참여자 맥락 없이 코드로 조회”라면 이름을getPickeatByCode등으로 조정하는 것을 권장합니다.
144-151: TODO 정리 제안
//TODO: 안씀 지우자는 PR 병합 전 정리하거나, 이슈로 승격해 추적 가능하게 남겨 주세요. 작은 빚을 일찍 갚으면 큽니다 🙂backend/src/main/java/com/pickeat/backend/pickeat/ui/api/PickeatApiSpec.java (1)
540-544: 비회원 재참여 가능 여부 API의 보안 요구사항 재검토엔드포인트 설명은 “비회원”이지만
security = ParticipantAuth가 설정되어 있습니다. 토큰이 선택 사항이라면 스펙에서 보안을 제거하거나 문서에 “선택적 인증(있으면 개인화)”로 명시해 주세요. 컨트롤러도required=false이므로 문서와 맞추는 것이 좋습니다.
backend/src/main/java/com/pickeat/backend/global/auth/JwtProvider.java
Outdated
Show resolved
Hide resolved
backend/src/main/java/com/pickeat/backend/global/auth/ProviderArgumentResolver.java
Show resolved
Hide resolved
backend/src/main/java/com/pickeat/backend/global/cache/CacheManagerConfig.java
Show resolved
Hide resolved
backend/src/main/java/com/pickeat/backend/login/application/ProviderTokenProvider.java
Show resolved
Hide resolved
| //TODO: 결과 만드는중에 좋아요되면 어카지? (2025-10-20, 월, 20:25) | ||
| private PickeatResultResponse createNewResultWithConcurrencyHandling(Pickeat pickeat) { | ||
| try { | ||
| pickeat.deactivate(); | ||
| RestaurantResultResponse newResult = createNewPickeatResult(pickeat); | ||
| return new PickeatResultCreationResponse(newResult, true); | ||
| PickeatResultResponse pickeatResult = createNewPickeatResult(pickeat); | ||
| applicationEventPublisher.publishEvent(new PickeatDeactivatedEvent(pickeat)); | ||
| return pickeatResult; | ||
| } catch (DataIntegrityViolationException e) { |
There was a problem hiding this comment.
결과 생성 중 좋아요 경쟁 상태(TODO 처리 필요)
deactivate() 이후 동일 트랜잭션에서 좋아요가 계속 반영될 수 있어 결과 합산 기준이 흔들립니다. 현재 like(...) 쪽에 활성 상태 검증이 없어 더 취약합니다.
개선 제안:
- like(...)에 피킷 활성 상태 검증 추가(서비스 간 일관성).
- 결과 생성 경로에서 deactivation을 먼저 영속/flush하여 다른 트랜잭션이 isActive=false를 즉시 보도록 처리.
- 필요 시 Pickeat 행에 대한 비관적 락(SELECT ... FOR UPDATE) 또는 결과 테이블의 유니크 제약 + 재조회 패턴 유지.
이렇게 하면 Race로 인한 결과 왜곡을 줄일 수 있어요.
Also applies to: 80-90
🤖 Prompt for AI Agents
backend/src/main/java/com/pickeat/backend/pickeat/application/PickeatResultService.java
around lines 67-74 (also apply same changes to 80-90): the current flow
deactivates a Pickeat in-memory and then creates results, but concurrent
like(...) calls can still record likes in parallel transactions causing
inconsistent result totals; fix by 1) enforcing active-state validation in the
like(...) service method so likes are rejected if pickeat is inactive, 2) after
calling pickeat.deactivate() persist and flush the Pickeat entity within the
same transaction so DB state shows isActive=false immediately (or acquire a
pessimistic lock on the Pickeat row via SELECT ... FOR UPDATE before
deactivation), and 3) keep or add a unique constraint on the results and
implement a retry/requery pattern on DataIntegrityViolationException to ensure
idempotent result creation under concurrency.
| restaurantLikeRepository.save(new RestaurantLike(participant.getId(), restaurant.getId())); | ||
| } |
There was a problem hiding this comment.
비활성화된 피킷에 대한 좋아요 허용 가능성
like(...)에서 피킷 활성 상태 검증이 없습니다. PickeatResultService가 비활성화 처리를 하더라도, 같은 시점에 좋아요가 계속 들어오면 결과 생성 시점의 합산과 불일치가 발생할 수 있어요.
대안:
- restaurant.getPickeatId()로 Pickeat을 조회 후 validatePickeatState(pickeat) 수행.
- 또는 DB 제약은 어려우니, 서비스 레벨에서 일관되게 isActive 검증을 추가.
원하시면 안전하게 적용 가능한 패치 제안을 더 드릴게요.
backend/src/main/java/com/pickeat/backend/tobe/restaurant/application/RestaurantService.java
Show resolved
Hide resolved
...end/src/main/resources/db/migration/V15__update_restaurant_schema_for_indirect_reference.sql
Show resolved
Hide resolved
...end/src/main/resources/db/migration/V15__update_restaurant_schema_for_indirect_reference.sql
Show resolved
Hide resolved
| @DataJpaTest | ||
| @Import({ParticipantService.class, ParticipantTokenProvider.class, JwtProvider.class}) | ||
| @Import({ParticipantService.class, ParticipantTokenProvider.class, JwtProvider.class, PickeatRepositoryImpl.class, | ||
| ParticipantRepositoryImpl.class}) |
There was a problem hiding this comment.
🧩 Analysis chain
Repository 구현체를 포함한 테스트 설정이 적절합니다! 🔧
ParticipantRepositoryImpl과 PickeatRepositoryImpl을 직접 import하여 캐싱 동작을 함께 테스트할 수 있게 했네요.
확인 포인트:
PR 목표가 "인메모리 캐싱 적용"인데, 캐싱 동작을 검증하는 테스트가 별도로 있는지 확인이 필요합니다. 현재 테스트는 기능 검증에 집중되어 있는데, 캐시 히트/미스를 검증하는 테스트도 있으면 좋겠습니다!
🏁 Script executed:
#!/bin/bash
# 캐싱 관련 테스트 확인
rg -n "@Cacheable|@CacheEvict|@CachePut" backend/src/test/ -A 5 -B 5Length of output: 0
추가 검증이 필요합니다. 테스트 파일 내용과 캐싱 구현 여부를 확인하겠습니다.
🏁 Script executed:
#!/bin/bash
# 1. 테스트 파일 전체 내용 확인
echo "=== ParticipantServiceTest.java 내용 ==="
cat backend/src/test/java/com/pickeat/backend/pickeat/application/ParticipantServiceTest.java
echo -e "\n=== 캐싱 어노테이션 전체 위치 ==="
rg -n "@Cacheable|@CacheEvict|@CachePut" backend/src/ -B 2 -A 2
echo -e "\n=== 캐싱 검증 관련 메소드 (테스트) ==="
rg -n "cache|Cache" backend/src/test/java/com/pickeat/backend/pickeat/application/ParticipantServiceTest.java -iLength of output: 11568
캐싱 동작 검증 테스트가 필요합니다! 📝
검증 결과, ParticipantServiceTest에는 현재 캐싱 동작을 검증하는 테스트가 없습니다. 구현체(ParticipantRepositoryImpl, PickeatRepositoryImpl)에는 @Cacheable과 @CacheEvict가 적용되어 있지만, 테스트에서는 기능만 검증하고 있네요.
추가해야 할 테스트 케이스:
- 캐시 히트 검증: 동일한 픽잇의 참가자 목록을 두 번 조회할 때, 두 번째는 DB 접근 없이 캐시에서 반환되는지 확인
- 캐시 미스 검증: 다른 픽잇의 참가자 목록 조회 시 새로운 캐시 키로 저장되는지 확인
- 캐시 무효화 검증: 참가자 생성/수정 후
evictParticipantCache()호출로 캐시가 정상 삭제되는지 확인
예시 방향:
@Test
void 동일_픽잇의_참가자_목록_조회시_캐시_히트() {
// 첫 번째 조회 (캐시 미스, DB 접근)
List<Participant> first = participantService.getParticipants(pickeatId);
// 두 번째 조회 (캐시 히트, DB 접근 없음)
List<Participant> second = participantService.getParticipants(pickeatId);
// 같은 객체 참조인지 확인 또는 DB 쿼리 횟수 검증
}Spring의 CacheManager를 테스트에 주입하거나, @SpyBean으로 Repository 메소드 호출 횟수를 검증할 수 있습니다. 공식 문서를 참고하세요: Spring Boot Caching Testing
- 사용자 정의 클레임(extraClaims) 추가 시 예약 클레임(sub, iat, exp) 덮어쓰기 방지 로직 적용 - JwtBuilder의 생성 순서를 명확히 구분 (사용자 정의 클레임 → JWT 예약 클레임 → 최종 토큰 생성) - 토큰 생성 관련 코드의 가독성과 안정성 향상
- 캐시 이름을 상수로 관리하기 위한 CacheNames 인터페이스 추가 - 기존 캐시 이름을 CacheNames 상수로 대체하여 코드 일관성 및 유지보수성 향상 - 관련된 Repository 구현체의 캐싱 키 설정 코드 리팩토링 (Restaurant, Participant, Pickeat 등)
- Boolean.FALSE.equals 사용으로 NPE 방지 및 코드 가독성 향상
- createPickeatResult 명세 최신화
- RestaurantLikeJpaRepository에서 countAllByRestaurantId를 countByRestaurantId로 변경 - RestaurantJpaRepository에서 findIdsByPickeatIdIn 메서드 제거 및 findByPickeatIdIn으로 변경 - 관련 테스트에서 메서드 이름 변경에 따른 수정
supernovaMK
left a comment
supernovaMK
left a comment
There was a problem hiding this comment.
에드~ 수고 많으셨습니다!
요구사항이 많았는데 모두 잘 해결해주셨네요 . 몇가지 코멘트 달아두었습니다!
| } | ||
|
|
||
| @Override | ||
| @PatchMapping("/restaurants/{restaurantId}/like") |
There was a problem hiding this comment.
[P2]
like,cacelLikeRestaurants 가가 Patch인게 이제 어색해진 것 같다는 생각이 듭니다.
어떻게 생각하시나요?
| targets.forEach(Restaurant::exclude); | ||
| restaurantRepository.saveAll(targets); |
There was a problem hiding this comment.
현재 cache를 적용 후에 restaurantRepository.saveAll()을 해주시고 계신데, 트랜잭션에 캐시가 롤백되지 않을 것 같다고 생각이 듭니다. saveAll()이 실패했을때 cache와의 데이터와 정합성이 맞지 않을 것 같아요. 🤔
만약 의도하신게 아니라면
- TransactionAwareCacheManagerProxy
- After commit 이벤트 기반 처리
등등의 키워드가 있네요!
| @Override | ||
| public Optional<Restaurant> findById(Long restaurantId) { | ||
| return jpaRepository.findById(restaurantId); | ||
| } |
There was a problem hiding this comment.
단일 검색에서는 cache를 사용하고 있지 않네요! 이유가 궁금합니다!
cahche되어있는 List 에서 검색하는 것보다 이게 낫다고 판단하신 걸까요?
- 좋아요 수를 관리하기 위해 RestaurantLikeCount 클래스 추가 - 기존 int 기반 카운트를 AtomicLong 기반의 RestaurantLikeCount로 대체 - 좋아요 및 좋아요 취소 시 카운트 증가/감소 처리 로직 추가 - 캐싱 로직 수정으로 데이터 일관성 강화 및 코드 가독성 향상 - 관련 테스트 코드 및 Repository 구현체 수정
- RestaurantLikeCount 클래스 및 관련 로직 제거 - 좋아요 수 관리를 ParticipantLikesRepository로 통합 - ParticipantLikes를 활용한 좋아요 카운트 및 상태 관리로 코드 간소화 - 캐시 및 테스트 코드 수정으로 불필요한 캐싱 제거 및 유지보수성 개선 - Pickeat 생성 시 캐시 초기화 로직 적용 - 관련 인터페이스 및 Repository 리팩토링
|
@CodeRabbit 코드 리뷰해줘 |
|
@wodnd0131 네, PR을 리뷰하겠습니다! 이번 PR은 pickeat 플로우에 인메모리 캐싱을 적용한 대규모 리팩토링이네요. 주요 변경 사항들을 확인해보겠습니다: 주요 검토 포인트:
커밋이 잘 정리되어 있어 변경 사항을 단계별로 검토하기 좋겠습니다. 바로 리뷰 시작하겠습니다! 🚀 ✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 12
♻️ Duplicate comments (9)
backend/src/main/resources/db/migration/V15__update_restaurant_schema_for_indirect_reference.sql (2)
1-2: 외래키 삭제 전 조건부 처리가 필요합니다.라인 1-2에서 외래키를 무조건 삭제하고 있는데, 다음과 같은 문제가 발생할 수 있습니다:
- 멱등성 위반: 마이그레이션을 재실행하면 이미 삭제된 외래키에 대해 에러 발생
- 환경별 제약 조건 이름 차이: 데이터베이스나 스키마 생성 방식에 따라 FK 이름이 다를 수 있음
- 라인 4-19와 불일치: 같은 파일에서 FK 인덱스는 조건부로 삭제하면서 FK 자체는 무조건 삭제하는 것은 패턴 불일치
라인 4-19에서 이미 구현한 조건부 삭제 패턴을 FK 삭제에도 적용하세요:
-ALTER TABLE `restaurant` - DROP FOREIGN KEY `FK37vnutmwssokg056o36w5qax`; - -SET @restaurant_fk_index := 'FK37vnutmwssokg056o36w5qax'; -SET @drop_restaurant_index_sql := ( - SELECT IF(EXISTS ( - SELECT 1 - FROM INFORMATION_SCHEMA.STATISTICS - WHERE TABLE_SCHEMA = DATABASE() - AND TABLE_NAME = 'restaurant' - AND INDEX_NAME = @restaurant_fk_index - ), - CONCAT('ALTER TABLE `restaurant` DROP INDEX `', @restaurant_fk_index, '`'), - 'SELECT 1') -); +SET @restaurant_fk_name := 'FK37vnutmwssokg056o36w5qax'; +SET @drop_fk_sql := ( + SELECT IF(EXISTS ( + SELECT 1 + FROM INFORMATION_SCHEMA.KEY_COLUMN_USAGE + WHERE TABLE_SCHEMA = DATABASE() + AND TABLE_NAME = 'restaurant' + AND CONSTRAINT_NAME = @restaurant_fk_name + ), + CONCAT('ALTER TABLE `restaurant` DROP FOREIGN KEY `', @restaurant_fk_name, '`'), + 'SELECT 1') +); + +PREPARE drop_fk_stmt FROM @drop_fk_sql; +EXECUTE drop_fk_stmt; +DEALLOCATE PREPARE drop_fk_stmt; + +SET @restaurant_fk_index := @restaurant_fk_name; +SET @drop_restaurant_index_sql := ( + SELECT IF(EXISTS ( + SELECT 1 + FROM INFORMATION_SCHEMA.STATISTICS + WHERE TABLE_SCHEMA = DATABASE() + AND TABLE_NAME = 'restaurant' + AND INDEX_NAME = @restaurant_fk_index + ), + CONCAT('ALTER TABLE `restaurant` DROP INDEX `', @restaurant_fk_index, '`'), + 'SELECT 1') +);참고: MySQL INFORMATION_SCHEMA.KEY_COLUMN_USAGE를 사용하여 외래키 존재 여부를 안전하게 검증할 수 있습니다.
53-67:pickeat_id컬럼 존재 여부를 인덱스 생성 전에 검증하세요.라인 21-51에서는 컬럼 삭제 전
INFORMATION_SCHEMA.COLUMNS로 존재 여부를 확인하지만, 라인 61-63에서는 컬럼 존재 여부를 검증하지 않고 인덱스를 생성합니다. 이는 패턴 불일치이며 다음과 같은 위험이 있습니다:
- 일관성 부재: 같은 파일 내 방어적 프로그래밍 패턴이 불일치
- 마이그레이션 체인 의존성: 이전 마이그레이션에서
pickeat_id컬럼 추가가 실패했다면 인덱스 생성 실패- 미래 유지보수: 스키마 변경 시 마이그레이션이 깨질 수 있음
라인 21-35의 패턴을 따라 다음과 같이 개선하세요:
SET @restaurant_index_exists = ( SELECT COUNT(*) FROM INFORMATION_SCHEMA.STATISTICS WHERE TABLE_SCHEMA = DATABASE() AND TABLE_NAME = 'restaurant' AND INDEX_NAME = 'idx_restaurant_pickeat_id' ); -SET @create_restaurant_index_sql = IF(@restaurant_index_exists = 0, - 'CREATE INDEX `idx_restaurant_pickeat_id` ON `restaurant` (`pickeat_id`)', +SET @pickeat_id_exists = ( + SELECT COUNT(*) + FROM INFORMATION_SCHEMA.COLUMNS + WHERE TABLE_SCHEMA = DATABASE() + AND TABLE_NAME = 'restaurant' + AND COLUMN_NAME = 'pickeat_id' +); + +SET @create_restaurant_index_sql = IF(@restaurant_index_exists = 0 AND @pickeat_id_exists > 0, + 'CREATE INDEX `idx_restaurant_pickeat_id` ON `restaurant` (`pickeat_id`)', 'SELECT 1');이렇게 수정하면:
- ✅ 컬럼 존재 여부를 먼저 검증 → 안전한 마이그레이션
- ✅ 같은 파일 내 방어적 로직 패턴 일관화 → 가독성 향상
- ✅ 미래 스키마 변경에 강건한 마이그레이션 → 유지보수성 개선
backend/src/main/java/com/pickeat/backend/pickeat/domain/repository/ParticipantRepository.java (1)
7-14: [이전 리뷰 지적사항 미반영] 캐시 무효화 메서드를 인터페이스에 노출하세요이전 리뷰에서 지적되었던
evictParticipantCache(Long pickeatId)메서드가 여전히 인터페이스에 없습니다. 현재 이벤트 핸들러(PickeatDeactivatedEventHandler,ParticipantStateUpdatedEventHandler)들이ParticipantRepositoryImpl구현체를 직접 주입받아 캐시 무효화를 수행하고 있다면, 이는 의존성 역전 원칙(DIP) 위반입니다.문제점:
- 도메인 계층(이벤트 핸들러)이 인프라 계층(구현체)을 직접 의존 → 계층 구조 위반
- 단위 테스트 시 실제 캐시 인프라까지 필요 → 테스트 복잡도 증가
- 구현체 교체 시 이벤트 핸들러까지 수정 필요 → 확장성 저하
해결 방법:
public interface ParticipantRepository { List<Participant> findByPickeatId(Long pickeatId); Participant save(Participant participant); Optional<Participant> findById(Long participantId); + + /** + * 특정 픽잇의 참가자 캐시를 무효화합니다. + * + * @param pickeatId 캐시를 무효화할 픽잇 ID + */ + void evictParticipantCache(Long pickeatId); }추가로, 여러 픽잇의 참가자를 한 번에 조회하는 배치 메서드도 고려해보세요:
/** * 여러 픽잇의 참가자 목록을 조회합니다. * * @param pickeatIds 조회할 픽잇 ID 모음 * @return 해당 픽잇들의 참가자 목록 */ List<Participant> findByPickeatIdIn(Collection<Long> pickeatIds);배치 조회 메서드는 N+1 문제를 방지하고 성능을 개선할 수 있습니다.
다음 스크립트로 실제 의존 관계를 확인해주세요:
#!/bin/bash # 이벤트 핸들러들이 구현체를 직접 의존하는지 확인 rg -nP 'ParticipantRepositoryImpl' --type=java -C3 # evictParticipantCache 호출부 확인 rg -nP '\bevictParticipantCache\s*\(' --type=java -C3 # findByPickeatIdIn 사용 여부 확인 rg -nP '\bfindByPickeatIdIn\s*\(' --type=java -C3참고 자료:
- SOLID 원칙 - DIP(의존성 역전 원칙)
- 우테코 백엔드 코드 컨벤션의 '계층 간 의존성 관리' 섹션
As per coding guidelines
backend/src/main/java/com/pickeat/backend/restaurant/domain/repository/RestaurantJpaRepository.java (1)
17-19: 과거 리뷰 이슈 해결 여부 확인 필요이전 리뷰에서 다음 두 가지 중요한 이슈가 지적되었습니다:
- @where 필터 미적용:
Restaurant엔티티에@Where(clause = "deleted = false")추가 필요- @transactional 보장: 호출하는 코드에서 트랜잭션 보장 필요
현재 코드에서
@Modifying설정은 적절하게 되어 있습니다. 이전 리뷰에서PickeatScheduler의 트랜잭션 설정은 확인되었다고 하니,Restaurant엔티티의@Where필터만 추가되었는지 최종 확인이 필요합니다.#!/bin/bash # Restaurant 엔티티에 @Where 필터 적용 여부 확인 rg -A 10 "class Restaurant" backend/src/main/java/com/pickeat/backend/restaurant/domain/Restaurant.java | rg -i "@Where|@SQLDelete|deleted"backend/src/main/java/com/pickeat/backend/restaurant/domain/repository/RestaurantLikeJpaRepository.java (2)
16-16: count 반환 타입 일관성(long)으로 변경 권장Spring Data count 파생쿼리는 long이 표준입니다. Integer는 범위/캐스팅 이슈가 있어요.
- Integer countByRestaurantId(Long restaurantId); + long countByRestaurantId(Long restaurantId);(이전 코멘트와 동일 제안입니다) As per coding guidelines.
10-26: 중복 좋아요 방지 및 조회 성능 위한 인덱스/제약 권장
- (restaurant_id, participant_id) UNIQUE 제약으로 중복 좋아요 방지
- 조회 패턴에 맞춰 restaurant_id, participant_id 인덱스 추가
Flyway에 마이그레이션을 추가해 주세요.
필요하시면 DDL 초안 드리겠습니다.
backend/src/main/java/com/pickeat/backend/tobe/restaurant/application/RestaurantService.java (2)
38-54: 배치 삽입 후 캐시 무효화 검증 필요이전 리뷰에서 지적된 배치 삽입의 캐시 무효화 이슈를 확인해야 합니다.
restaurantRepository.batchInsert()가 Pickeat ID 기반 캐시를 자동으로 무효화하는지 검증이 필요해요.확인이 필요한 사항:
RestaurantRepositoryImpl.batchInsert()에서@CacheEvict또는 이벤트 기반 캐시 무효화가 적용되어 있는가?- 생성 직후
restaurantRepository.findByPickeatId(pickeat.getId())호출 시 캐시된 빈 리스트가 반환되지 않는가?다음 스크립트로 RestaurantRepository의 batchInsert 구현을 확인해주세요:
#!/bin/bash # RestaurantRepository와 그 구현체에서 batchInsert의 캐시 처리 확인 ast-grep --pattern $'class $_ implements RestaurantRepository { $$$ batchInsert($$$) { $$$ } $$$ }' # 또는 캐시 관련 어노테이션 확인 rg -n -A 10 "batchInsert" --type=java -g '**/RestaurantRepository*.java'
92-104: 비활성 피킷에 대한 좋아요 허용 문제 여전이전 리뷰에서 지적되었던 피킷 활성 상태 검증이 여전히 누락되어 있습니다. 비활성화된 피킷에도 좋아요가 가능하면, 결과 생성 시점과 좋아요 집계 간 불일치가 발생할 수 있어요.
개선 방안:
@Transactional public void like(Long restaurantId, Long participantId) { if (existsLike(restaurantId, participantId)) { throw new BusinessException(ErrorCode.PARTICIPANT_RESTAURANT_ALREADY_LIKED); } Participant participant = getParticipant(participantId); Restaurant restaurant = getRestaurantById(restaurantId); validateParticipantAccessToRestaurants(List.of(restaurant), participant); // 피킷 활성 상태 검증 추가 Pickeat pickeat = getPickeatById(restaurant.getPickeatId()); validatePickeatState(pickeat); // 이미 구현된 메서드 재사용 ParticipantLikes participantLikes = participantLikesRepository.findByRestaurantId(restaurantId); restaurantLikeRepository.save(new RestaurantLike(participant.getId(), restaurant.getId())); participantLikes.addParticipantId(participantId); }비활성 피킷에 대한 좋아요를 막아 데이터 정합성을 보장해주세요.
backend/src/main/java/com/pickeat/backend/pickeat/ui/PickeatController.java (1)
74-83: 멱등성 지원 시 HTTP 상태 코드 개선 고려현재 결과 생성 API는 항상
201 CREATED를 반환하는데, API 스펙 Line 234에서 "멱등성을 지원하여 이미 결과가 있는 경우 기존 결과를 반환"한다고 명시되어 있습니다.REST 모범 사례:
- 신규 생성:
201 Created- 기존 결과 반환:
200 OK선택지:
- 간단한 방법: 문서를 "항상 201 반환"으로 수정
- 표준적인 방법: 서비스에서 생성 여부를 반환받아 분기 처리
CreateResultResponse response = pickeatResultService.createPickeatResult(...); HttpStatus status = response.isNewlyCreated() ? HttpStatus.CREATED : HttpStatus.OK; return ResponseEntity.status(status).body(response.data());팀 내에서 REST API 설계 원칙을 논의하고 일관된 정책을 적용하시길 권장합니다.
🧹 Nitpick comments (35)
backend/src/main/resources/db/migration/V11__drop_participant_count_column_from_pickeat.sql (1)
1-2: 마이그레이션 기본 구조는 올바르지만, 재실행 안전성을 위해IF EXISTS절 추가를 권장합니다.현재 마이그레이션은 SQL 문법이 정확하고 PR 의도(participantCount 필드 제거)와도 완벽히 부합합니다. 다만 Flyway 마이그레이션의 모범 사례 관점에서, 마이그레이션이 재실행되거나 실패 후 재시도되는 상황에 대비하면 좋습니다.
개선 제안:
ALTER TABLE `pickeat` DROP COLUMN IF EXISTS `participant_count`;배경:
- Flyway는 체크섬으로 마이그레이션 중복 실행을 방지하지만, DB 상태가 예상과 다른 경우(수동 개입, 복구 시나리오 등)를 대비할 수 있습니다.
IF EXISTS절은 컬럼이 없으면 무시하므로 멱등성(idempotency)을 확보합니다.참고: 팀의 Flyway 정책이나 기존 마이그레이션 관례가 있다면 그에 맞춰 진행하세요. 현재 마이그레이션 시리즈(V10~V14)가 일관되게 IF EXISTS를 사용하지 않는다면, 이 파일도 그 관례를 따르는 것이 통일성 면에서 좋습니다.
backend/src/test/java/com/pickeat/backend/tobe/acceptance_test/scenario/PickeatByTemplateScenarioTest.java (2)
105-112: 타입 변경이 일관되게 적용되었습니다.
PickeatResultResponse타입으로의 전환이 모든 변수에 일관성 있게 반영되었습니다.다만,
checkoutPickeatResult에서 단순 equality 검증만 수행하는데, 픽잇 결과의 핵심 필드(선택된 식당 정보, 투표 결과 등)를 명시적으로 검증하는 것도 고려해보세요. 이는 테스트의 의도를 더 명확히 하고, 예상치 못한 변경을 감지하는 데 도움이 됩니다.
126-128: 메서드 시그니처 변경이 적절합니다.타입 변경에 따라 파라미터 타입이 올바르게 업데이트되었습니다.
참고로, 메서드명
checkoutPickeatResult에서 "checkout"이라는 단어가 "검증"의 의미로 사용되고 있는데, 테스트 컨텍스트에서는verifyPickeatResult또는assertPickeatResult가 의도를 더 명확하게 전달할 수 있습니다. 하지만 프로젝트 전반의 네이밍 컨벤션을 따르는 것이라면 현재 상태도 괜찮습니다.backend/src/main/java/com/pickeat/backend/global/auth/JwtProvider.java (3)
49-57: 예약 클레임 보호 로직 굿! 범위를 등록 클레임 전체로 확장해주세요.sub/iat/exp 제거는 훌륭합니다. 다만 JWT 등록 클레임(iss, sub, aud, exp, nbf, iat, jti)까지 방어하지 않으면, 추후 iss·aud 등을 도입할 때 충돌 위험이 있어요. 상수 화이트리스트로 관리하면 안전하고 가독성도 좋아집니다.
권장 수정안:
- safeClaims.keySet().removeAll(Set.of("sub", "iat", "exp")); + safeClaims.keySet().removeAll(RESERVED_CLAIMS);클래스 상단(필드 영역)에 추가:
private static final Set<String> RESERVED_CLAIMS = Set.of("iss", "sub", "aud", "exp", "nbf", "iat", "jti");테스트 제안:
- extraClaims에 {"exp": 0, "iss": "x"}를 넣어도 sub/iat/exp/iss가 최종 토큰에서 기대대로 설정되는지 검증.
As per coding guidelines
60-63: 서명 알고리즘을 명시해 기본값 변경 리스크 제거기본값에 의존하면 라이브러리 업그레이드 시 알고리즘이 바뀔 수 있습니다. 명시적으로 HS256을 지정해주세요.
- .signWith(secretKey); + .signWith(secretKey, Jwts.SIG.HS256);(동일 패턴을 createToken(...)에도 적용 권장)
테스트 제안:
- 헤더 alg가 "HS256"인지, 서명 검증이 통과하는지 확인.
As per coding guidelines
27-29: 시크릿은 Base64로 관리하고 decode하여 Key 생성 권장UTF-8 문자열 바이트를 그대로 키로 쓰면 엔트로피·길이 관리가 어렵습니다. Base64로 저장하면 길이 보장이 쉽고 운영 관리도 명확합니다.
권장 수정안:
-import java.nio.charset.StandardCharsets; +import io.jsonwebtoken.io.Decoders; @@ - this.secretKey = Keys.hmacShaKeyFor(secret.getBytes(StandardCharsets.UTF_8)); + this.secretKey = Keys.hmacShaKeyFor(Decoders.BASE64.decode(secret));운영 가이드:
- application 설정의 jwt.secretKey를 최소 256비트 랜덤 바이트를 Base64 인코딩한 값으로 교체.
- 잘못된 길이일 때 부트 시점에 명확히 실패하는지 확인.
As per coding guidelines
backend/src/main/java/com/pickeat/backend/pickeat/domain/PickeatDeactivatedEvent.java (1)
5-10: 팩토리 메서드의 타입 변환 체인 검토 권장
pickeat.getCode().getValue().toString()호출 체인이 다소 길어 보입니다.확인 사항:
PickeatCode.getValue()의 반환 타입이 무엇인가요?- 이미 String이라면
.toString()은 불필요합니다.- UUID라면
.toString()은 적절하지만, 메서드명을toStringValue()같이 명확하게 하는 것도 고려해볼 수 있습니다.개선 제안:
public static PickeatDeactivatedEvent from(Pickeat pickeat) { return new PickeatDeactivatedEvent( pickeat.getId(), pickeat.getCode().getValueAsString() // 또는 더 명확한 메서드명 ); }과거 리뷰에서 제안된 ID 기반 페이로드로 잘 변경되었고,
pickeatId와pickeatCode모두 포함하여 핸들러에서 재조회 없이 캐시 무효화가 가능하게 된 점은 훌륭합니다! 👍backend/src/main/java/com/pickeat/backend/restaurant/domain/ParticipantLikes.java (1)
7-29: 도메인 객체의 방어적 프로그래밍 및 불변성 강화 제안
ParticipantLikes가 도메인 객체로서 몇 가지 개선이 가능합니다.1. Null 안전성 부재
현재 모든 public 메서드가 null 체크를 하지 않아 NPE 위험이 있습니다.public void addAll(List<Long> participantIds) { if (participantIds == null) { return; // 또는 throw new IllegalArgumentException } this.participantIds.addAll(participantIds); } public void addParticipantId(Long participantId) { if (participantId == null) { throw new IllegalArgumentException("participantId는 null일 수 없습니다"); } participantIds.add(participantId); }2. ConcurrentHashMap 사용 근거 확인 필요
ConcurrentHashMap.newKeySet()은 멀티스레드 환경에서 유용하지만, 추가 오버헤드가 있습니다.질문:
- 이 객체가 여러 스레드에서 동시 접근되나요?
- 캐시에 저장되어 공유되나요?
- 단일 스레드 서비스 레이어에서만 사용된다면
new HashSet<>()으로 충분합니다.3. 불변성 고려
도메인 객체라면 외부 변경을 최소화하는 것이 좋습니다. 빌더 패턴이나 생성자를 통한 초기화를 권장합니다.public class ParticipantLikes { private final Set<Long> participantIds; public ParticipantLikes(List<Long> participantIds) { this.participantIds = ConcurrentHashMap.newKeySet(); if (participantIds != null) { this.participantIds.addAll(participantIds); } } // add/remove는 신중하게 제공 }4. 비즈니스 로직 캡슐화 기회
현재는 단순 래퍼지만, 도메인 규칙을 추가할 수 있습니다:
- 최대 참여자 수 제한
- 중복 참여 검증
- 특정 상태에서만 추가/삭제 허용
Based on coding guidelines (객체지향, 최신 트렌드, 클린코드 확인 요청)
backend/src/test/java/com/pickeat/backend/pickeat/domain/repository/ParticipantJpaRepositoryTest.java (1)
29-61: 테스트 커버리지 확대 제안 - 엣지 케이스 추가 필요현재 Happy Path는 잘 작성되어 있습니다! 👍 Given-When-Then 구조가 명확하고
assertAll을 활용한 다중 검증도 좋습니다.추가 테스트 케이스 제안 (코딩 가이드라인 #6 준수):
@Nested class 참가자_삭제 { @Test void 픽잇_ID_목록으로_참가자_일괄_삭제() { // 기존 테스트 유지 } @Test void 빈_목록으로_삭제_시도시_아무것도_삭제되지_않음() { // given Pickeat pickeat = testEntityManager.persist(PickeatFixture.createWithoutRoom()); testEntityManager.persist(ParticipantFixture.create(pickeat.getId())); testEntityManager.flush(); long beforeCount = participantJpaRepository.count(); // when int deletedCount = participantJpaRepository.bulkSoftDeleteByPickeatIdIn(List.of()); // then assertThat(deletedCount).isZero(); assertThat(participantJpaRepository.count()).isEqualTo(beforeCount); } @Test void 존재하지_않는_픽잇_ID로_삭제_시도시_0_반환() { // when int deletedCount = participantJpaRepository.bulkSoftDeleteByPickeatIdIn( List.of(999L, 1000L) ); // then assertThat(deletedCount).isZero(); } @Test void 이미_삭제된_참가자는_카운트에_포함되지_않음() { // 소프트 삭제 멱등성 검증 } }검증해야 할 추가 시나리오:
- Null 안전성:
null파라미터 처리 (현재 DB 레이어에서 예외 발생할 것으로 예상)- 트랜잭션 롤백: 삭제 중 예외 발생 시 롤백 동작
- 대용량 데이터: 많은 ID 목록 처리 (IN 절 제한)
- 동시성: 동일 참가자를 동시에 삭제하는 경우
코딩 가이드라인에 따라 Repository 단위 테스트 커버리지를 높이는 것을 권장드립니다.
As per coding guidelines (#6: 미작성 테스트 케이스 제안)
backend/src/main/java/com/pickeat/backend/pickeat/application/PickeatService.java (5)
58-60: 이벤트는 커밋 이후에 발행하세요 (캐시 일관성·사이드이펙트 안정화).동일 트랜잭션 내 동기 발행은 리스너가 DB를 조회할 때 미커밋 상태·구버전 캐시와 레이스가 날 수 있어요.
@TransactionalEventListener(phase = AFTER_COMMIT)로 리스너를 전환하거나, 서비스에서 afterCommit 훅으로 발행을 지연시키는 방식을 권장합니다. 캐시 제거/전파(특히 Caffeine)는 커밋 이후가 안전합니다.
As per coding guidelines
68-71: 메서드 명이 의미와 달라요 — 코드로 조회인데 이름은 ‘Participant’.
getPickeatByParticipant(String pickeatCode)는 참가자와 무관하게 코드로만 조회합니다. 혼동을 줄이기 위해getPickeatByCode로 이름을 바꾸는 것을 제안합니다. 테스트도 함께 치환하세요.적용 예시(diff):
- public PickeatResponse getPickeatByParticipant(String pickeatCode) { + public PickeatResponse getPickeatByCode(String pickeatCode) { Pickeat pickeat = getPickeatByCode(pickeatCode); return PickeatResponse.from(pickeat); }
94-99: 재입장 여부 조회: 2쿼리를 1쿼리로 줄일 수 있어요.현재
participant -> pickeatId 조회+pickeatById 조회의 2번 접근입니다.existsByIdAndCode(id, code)를 리포지토리에 추가하면 단일 쿼리로 끝납니다. 박싱(Boolean)도 불필요하니 primitive로 정리하세요.적용 예시(diff):
- //TODO: 한번만 쿼리가 나가도록 할 수 있지 않을까? (2025-10-20, 월, 17:44) - Participant participant = getParticipant(participantId); - Pickeat pickeat = getPickeatByParticipant(participant); - Boolean rejoinAvailable = pickeat.isEqualPickeatCode(pickeatCode); - return new PickeatRejoinAvailableResponse(rejoinAvailable); + // 단일 쿼리: pickeatId + code 매칭 여부 + Participant participant = getParticipant(participantId); + boolean rejoinAvailable = pickeatRepository + .existsByIdAndCode(participant.getPickeatId(), new PickeatCode(pickeatCode)); + return new PickeatRejoinAvailableResponse(rejoinAvailable);리포지토리 시그니처 추가는
PickeatRepository코멘트 참고. As per coding guidelines
101-105: 빈 방 목록 처리로 불필요 쿼리 회피.
getAllRoomIdsByUserId가 빈 리스트면 즉시 빈 결과를 반환하면IN ()쿼리 생성·오류 가능성을 줄일 수 있어요.public List<PickeatResponse> getPickeatsByUser(Long userId) { List<Long> allRoomIds = roomUserRepository.getAllRoomIdsByUserId(userId); + if (allRoomIds.isEmpty()) { + return List.of(); + } List<Pickeat> roomPickeats = pickeatRepository.findByRoomIdIn(allRoomIds); return PickeatResponse.from(roomPickeats); }
107-111: 단수/복수 혼용 네이밍 수정 제안.
getPickeatsByParticipant(Long)가 단일Pickeat을 반환합니다.getPickeatByParticipantId등 단수형으로 맞추면 API 탐색성이 좋아져요. 테스트 호출부도 함께 치환 부탁드립니다.
As per coding guidelinesbackend/src/test/java/com/pickeat/backend/pickeat/application/PickeatServiceTest.java (2)
176-183: 불필요한 이중 persist 호출 제거.
createWithoutRoomPickeat()내에서 이미persist를 수행합니다. 반환 엔티티에 다시persist를 호출하면 혼동을 유발하고 구현체에 따라 예외가 날 수 있어요. 단순 대입으로 정리하세요.- Pickeat pickeat = testEntityManager.persist(createWithoutRoomPickeat()); + Pickeat pickeat = createWithoutRoomPickeat(); testEntityManager.flush(); testEntityManager.clear();
182-183: 서비스 메서드 리네이밍 반영.서비스를
getPickeatByCode로 변경 시 테스트도 함께 수정하세요.- PickeatResponse pickeatResponse = pickeatService.getPickeatByParticipant(pickeat.getCode().toString()); + PickeatResponse pickeatResponse = pickeatService.getPickeatByCode(pickeat.getCode().toString());backend/src/main/java/com/pickeat/backend/global/cache/CacheNames.java (1)
3-8: 상수 전용 ‘interface’ 대신final class권장.‘상수 인터페이스’는 안티패턴입니다. 인스턴스화 방지와 IDE 네비게이션을 위해
final class + private constructor로 전환을 제안해요. 콜론(participant:like)은 캐시 이름·메트릭 태그 등에서 의미가 섞일 수 있으니 팀 컨벤션을 명시해 두면 좋아요.-package com.pickeat.backend.global.cache; - -public interface CacheNames { - String RESTAURANT = "restaurant"; - String PICKEAT = "pickeat"; - String PARTICIPANT = "participant"; - String PARTICIPANT_LIKE = "participant:like"; -} +package com.pickeat.backend.global.cache; + +public final class CacheNames { + private CacheNames() {} + public static final String RESTAURANT = "restaurant"; + public static final String PICKEAT = "pickeat"; + public static final String PARTICIPANT = "participant"; + public static final String PARTICIPANT_LIKE = "participant:like"; +}As per coding guidelines
backend/src/main/java/com/pickeat/backend/pickeat/domain/repository/PickeatRepository.java (1)
12-16: 파라미터명 오타 정정 제안 (rooId/roodIds).가독성과 IDE 검색 정확도를 위해
roomId/roomIds로 통일하세요.- List<Pickeat> findByRoomId(Long rooId); + List<Pickeat> findByRoomId(Long roomId); ... - List<Pickeat> findByRoomIdIn(List<Long> roodIds); + List<Pickeat> findByRoomIdIn(List<Long> roomIds);As per coding guidelines
backend/src/main/java/com/pickeat/backend/restaurant/application/RestaurantService.java (3)
71-90: exclude: 유효하지 않은 ID 요청 처리와 캐시 무효화 확인
- 요청 ID 중 현재 Pickeat에 속하지 않는 ID가 섞여 있어도 무시됩니다. 클라이언트 오류를 조기에 드러내려면 유효성 검증 후 예외를 던지는 편이 안전합니다(데이터 일관성).
- 저장 이후 캐시 무효화(식당 목록/단건, 좋아요 집계 캐시 등)가 리포지토리 레벨에서 보장되는지 확인이 필요합니다.
예시:
- List<Restaurant> targets = restaurants.stream() - .filter(restaurant -> targetIds.contains(restaurant.getId())) - .toList(); + Set<Long> scope = restaurants.stream().map(Restaurant::getId).collect(Collectors.toSet()); + List<Long> invalid = targetIds.stream().filter(id -> !scope.contains(id)).toList(); + if (!invalid.isEmpty()) { + throw new BusinessException(ErrorCode.RESTAURANT_ELIMINATION_FORBIDDEN); + } + List<Restaurant> targets = restaurants.stream() + .filter(r -> targetIds.contains(r.getId())) + .toList();테스트 제안: (1) 유효/무효 ID 혼합 시 예외, (2) 저장 후 동일 요청 재실행 시 idempotent, (3) 캐시 적중률/무효화 검증(Repository 캐시 단위 테스트).
106-111: cancelLike의 의미를 명확히: 무상태(idempotent) vs 대칭적 예외현재 존재 여부 확인 없이 삭제합니다.
like()가 이미 좋아요인 경우 예외를 던지는 것과 대칭을 맞추려면 “좋아요하지 않았음”에 대한 예외/반환값 정책을 정리해 주세요. 서버-클라이언트 계약을 명시하면 UX/로깅이 깔끔해집니다.테스트 제안: (1) 미좋아요 상태에서 cancelLike 호출 시 기대 동작(예외/정상)을 명시, (2) 중복 호출 idempotent 보장 여부.
149-151: null‑세이프 비교로 통일해 가독성과 안정성 향상
Long.equals는 좌변 null 시 NPE 위험이 있습니다(도메인 제약으로 null 불가라도 일관성 측면에서 통일 권장). 위에서Objects.equals를 이미 사용하니 동일하게 맞추면 읽기 좋습니다.- if (restaurants.stream() - .anyMatch((restaurant -> !restaurant.getPickeatId().equals(participant.getPickeatId())))) { + if (restaurants.stream() + .anyMatch(r -> !Objects.equals(r.getPickeatId(), participant.getPickeatId()))) {backend/src/main/java/com/pickeat/backend/restaurant/domain/repository/RestaurantLikeRepository.java (1)
5-12: 도메인 저장소 계약 명확화: 삭제 반환값/벌크 지원
deleteByRestaurantIdAndParticipantId가void면 호출 측에서 성공/부재를 구분하기 어렵습니다. 불변성(대칭성) 확보를 위해boolean또는 삭제 건수 반환을 고려해 주세요.- 비활성화/정리 시 대량 정리 필요가 잦다면 도메인 저장소에도
deleteByRestaurantIdIn또는bulkSoftDeleteByRestaurantIdIn을 노출해 애플리케이션 레이어가 JPA 저장소에 직접 의존하지 않도록 경계를 정리하는 것이 좋습니다.테스트 제안: 존재/부재 케이스 각각에서 반환값/예외를 검증하고, 대량 삭제 시 트랜잭션 경계와 캐시 무효화가 일관되게 작동하는지 확인.
backend/src/main/java/com/pickeat/backend/global/cache/CacheType.java (1)
7-12: 캐시 정책 표현 개선: Duration 사용·이름 중복 제거·운영 가시성 확보
- 단위 혼동 방지를 위해
Long ttlMinutes대신Duration을 권장합니다.CacheNames와 문자열이 이중으로 존재합니다. 한 곳이 소스 오브 트루스가 되도록 중복 제거를 추천합니다.- Caffeine
recordStats()+ 지표(export)로 적중률/만료/축출을 관찰하면 사이징(TTL/size) 튜닝이 쉬워집니다. TODO 대신 프로퍼티 외부화도 고려해 주세요.예시:
- private final String name; - private final Long ttlMinutes; + private final String name; + private final Duration ttl; private final Long maxSize; private final Integer initialCapacity; - CacheType(String name, Long ttlMinutes, Long maxSize, Integer initialCapacity) { + CacheType(String name, Duration ttl, Long maxSize, Integer initialCapacity) { this.name = name; - this.ttlMinutes = ttlMinutes; + this.ttl = ttl; this.maxSize = maxSize; this.initialCapacity = initialCapacity; }운영 팁:
- 초기 값: PICKEAT(코드→Pickeat) 조회는 히트율이 높으므로 TTL↑, size↑; PARTICIPANT_LIKE는 변경 이벤트가 잦아 TTL↓. 실제 트래픽 메트릭으로 보정하세요. As per coding guidelines.
Also applies to: 14-25
backend/src/test/java/com/pickeat/backend/restaurant/domain/repository/RestaurantLikeJpaRepositoryTest.java (1)
54-70: 좋아요 벌크 소프트삭제 시나리오 테스트 훌륭해요 👏삭제 건수·잔존 데이터 검증이 명확합니다. 몇 가지 케이스를 추가하면 더 탄탄해집니다.
추가 제안:
- 빈 목록/존재하지 않는 ID 목록 입력 시 0건 반환 확인.
- 중복 ID 포함 시 중복 처리 안전성.
- 소프트삭제 후 유니크 제약(restaurant_id, participant_id, deleted=false)이 유지되는지 재삽입 테스트.
backend/src/main/java/com/pickeat/backend/restaurant/domain/repository/ParticipantLikesRepository.java (1)
23-26: Projection으로 I/O 줄이기participantId만 필요합니다. 전체 RestaurantLike를 조회하지 말고, ID 전용 프로젝션/쿼리로 전환하면 메모리·I/O를 줄일 수 있어요.
- List<RestaurantLike> findAllByRestaurantId(Long restaurantId); + @Query("select rl.participantId from RestaurantLike rl where rl.restaurantId = :restaurantId") + List<Long> findParticipantIdsByRestaurantId(@Param("restaurantId") Long restaurantId)그에 따라 본 메서드도 participantIds = repository.findParticipantIdsByRestaurantId(…)로 교체.
backend/src/test/java/com/pickeat/backend/restaurant/domain/repository/RestaurantJpaRepositoryTest.java (1)
73-105: 엣지/회귀 테스트 보강 제안현재 케이스 훌륭합니다. 아래 2가지를 추가하면 회귀 방지에 도움됩니다.
- 빈 리스트 삭제 시: 예외 없이 0 반환 확인 (서비스 가드 로직 기대 동작)
- 멱등성: 동일 ID로 2회 연속 soft-delete 호출 시 2번째 호출의 deletedCount가 0인지 확인 (
AND deleted = false조건 추가 시 보장)예시 검증:
// given List<Long> ids = List.of(pickeat1.getId(), pickeat2.getId()); // when int first = restaurantJpaRepository.bulkSoftDeleteByPickeatIdIn(ids); int second = restaurantJpaRepository.bulkSoftDeleteByPickeatIdIn(ids); // then assertThat(second).isZero();backend/src/main/java/com/pickeat/backend/restaurant/domain/repository/RestaurantLikeJpaRepository.java (2)
20-23: Bulk 소프트 삭제에도AND deleted = false조건 추가멱등성과 불필요한 쓰기 최소화를 위해 다음과 같이 변경하세요.
-@Query(value = "UPDATE restaurant_like SET deleted = true WHERE restaurant_id IN :restaurantIds", +@Query(value = "UPDATE restaurant_like SET deleted = true WHERE deleted = false AND restaurant_id IN :restaurantIds", nativeQuery = true) int bulkSoftDeleteByRestaurantIdIn(@Param("restaurantIds") List<Long> restaurantIds);
12-26: ---
deleteByRestaurantIdAndParticipantId를 명시적 소프트 삭제 쿼리로 통일하기좋은 소식입니다! 프로젝트가 이미
@SoftDelete로 소프트 삭제를 표준화했기 때문에, 현재 코드는 기능적으로 문제없습니다. 하지만 코드 의도를 더욱 명확히 하기 위해 약간의 개선을 제안합니다.현황 분석:
- BaseEntity에
@SoftDelete적용됨 → RestaurantLike가 자동으로 상속bulkSoftDeleteByRestaurantIdIn()은 명시적 UPDATE 쿼리로 구현됨 ✅deleteByRestaurantIdAndParticipantId()는 Spring Data 파생 메서드 →@SoftDelete로 인해 자동으로 UPDATE로 변환됨개선 권장사항:
deleteByRestaurantIdAndParticipantId→ 명시적 @query로 변경@Modifying(clearAutomatically = true, flushAutomatically = true) @Query("update RestaurantLike rl set rl.deleted = true " + "where rl.restaurantId = :restaurantId and rl.participantId = :participantId") int softDeleteByRestaurantIdAndParticipantId(@Param("restaurantId") Long restaurantId, @Param("participantId") Long participantId);이유:
bulkSoftDeleteByRestaurantIdIn()과의 일관성, 의도 명확성
bulkSoftDeleteByRestaurantIdIn()개선 (멱등성 강화)@Query(value = "UPDATE restaurant_like SET deleted = true " + "WHERE restaurant_id IN :restaurantIds AND deleted = false", nativeQuery = true)이유: 이미 삭제된 행의 중복 업데이트 방지
deleteByRestaurantIdAndParticipantId동작을 검증하는 테스트 추가@Test void 선호_취소가_소프트_삭제하는지_검증() { // 삭제 후 findAll()에서 제외되는지 확인 restaurantLikeJpaRepository.deleteByRestaurantIdAndParticipantId(restaurantId, participantId); List<RestaurantLike> remaining = restaurantLikeJpaRepository.findAll(); assertThat(remaining).doesNotContain(deletedLike); // 소프트 삭제 검증 }이렇게 하면 모든 삭제 메서드의 의도가 명확하고, 팀원들이 소프트 삭제 패턴을 한눈에 이해할 수 있습니다. 문서도 코드로 말하는 것이 최고의 코드 컨벤션이니까요! 👌
backend/src/main/java/com/pickeat/backend/pickeat/domain/repository/PickeatResultRepository.java (1)
15-18: 쿼리에AND deleted = false조건을 추가하여 성능 최적화 필요좋은 소식입니다! 빈 리스트 처리와 트랜잭션 보장이 이미 잘 구현되어 있네요. 다만, 쿼리 성능 개선을 위해 한 가지 수정이 남아있습니다.
PickeatScheduler.cleanupOldPickeats()에서 호출되는bulkSoftDeleteByPickeatIdIn()메서드의 쿼리에AND deleted = false조건을 추가하면, 이미 삭제된 행에 불필요한 UPDATE 시도를 방지할 수 있습니다. 이를 통해 row lock 경합을 줄이고 undo log 생성량도 감소시킬 수 있습니다.-@Query(value = "UPDATE pickeat_result SET deleted = true WHERE pickeat_id IN :pickeatIds", +@Query(value = "UPDATE pickeat_result SET deleted = true WHERE deleted = false AND pickeat_id IN :pickeatIds", nativeQuery = true) int bulkSoftDeleteByPickeatIdIn(@Param("pickeatIds") List<Long> pickeatIds);관련 리포지토리에도 동일하게 적용하세요:
RestaurantJpaRepository.bulkSoftDeleteByPickeatIdIn()ParticipantJpaRepository.bulkSoftDeleteByPickeatIdIn()이 패턴은 데이터베이스 레벨의 멱등성(idempotency)을 높이는 좋은 실천 방법입니다. 특히 스케줄러처럼 반복 실행되는 작업에서는 더욱 중요합니다.
backend/src/main/java/com/pickeat/backend/restaurant/domain/repository/RestaurantRepository.java (1)
7-16: 쓰기 경로 단일화: saveAll 하나로 추상화하고 내부에서 배치 전략을 선택하세요batchInsert와 saveAll이 공존하면 “서비스 레이어가 어느 것을 언제 호출할지”가 퍼지고, 캐시 무효화/이벤트 발행 책임이 흩어집니다. 공개 API는 saveAll 하나로 두고, 구현체 내부에서 데이터 크기 등에 따라 JDBC 배치 여부를 결정하는 편이 안전합니다. 또한 saveAll이 void인 것도 검증/관측에 불리하니 반영 개수 또는 저장된 엔티티를 반환하는 방향을 권장합니다.
예시(개념 스케치):
-public interface RestaurantRepository { - void batchInsert(List<Restaurant> restaurants); - List<Restaurant> findByPickeatId(Long pickeatId); - void saveAll(List<Restaurant> restaurants); - Optional<Restaurant> findById(Long restaurantId); -} +public interface RestaurantRepository { + List<Restaurant> saveAll(List<Restaurant> restaurants); // 내부에서 JPA or JDBC 배치 선택 + List<Restaurant> findByPickeatId(Long pickeatId); + Optional<Restaurant> findById(Long restaurantId); +}구현체는 캐시 무효화/도메인 이벤트를 단일 경로에 모아 주세요. (팀 컨벤션의 모듈 책임 일원화 원칙에 부합)
backend/src/main/java/com/pickeat/backend/pickeat/application/PickeatResultService.java (2)
81-90: N+1 쿼리: 좋아요 카운트 일괄 조회로 치환availableRestaurants 각각에 대해 findByRestaurantId를 호출하면서 N+1이 발생합니다. IN 절 기반 일괄 조회로 바꿔주세요. 성능과 캐시 적중률이 크게 개선됩니다.
예시(서비스 측 변경):
- Map<Restaurant, Integer> likeCounts = getLikeCounts(availableRestaurants); + Map<Restaurant, Integer> likeCounts = getLikeCountsBulk(availableRestaurants);예시(새 메서드):
+ private Map<Restaurant, Integer> getLikeCountsBulk(List<Restaurant> availableRestaurants) { + Map<Long, Integer> counts = participantLikesRepository.findCountsByRestaurantIds( + availableRestaurants.stream().map(Restaurant::getId).toList() + ); + Map<Restaurant, Integer> map = new HashMap<>(); + for (Restaurant r : availableRestaurants) { + map.put(r, counts.getOrDefault(r.getId(), 0)); + } + return map; + }리포지토리 예시:
// backend/.../ParticipantLikesRepository.java Map<Long, Integer> findCountsByRestaurantIds(List<Long> restaurantIds);쿼리 레이어에서 GROUP BY restaurant_id 형태로 구현하면 됩니다.
45-56: 기존 결과 단락 처리 OK + 사전 조건 명시를 고려Optional 기반의 기존 결과 단락 처리는 명확합니다. 추가로, availableRestaurants가 비면 즉시 RESTAURANTS_IS_EMPTY를 던져 서비스 계층에서 책임을 명시하는 것도 선택지입니다(현재는 생성기에서 처리될 수도 있으므로 팀 합의에 맞춰 결정).
backend/src/main/java/com/pickeat/backend/pickeat/application/ParticipantService.java (1)
47-53: 완료 상태 변경 없을 때 이벤트 생략(노이즈 줄이기)현재는 동일 상태로 요청해도 매번 ParticipantStateUpdatedEvent를 발행합니다. 변경 발생 시에만 이벤트를 발행해 캐시 스로틀링/부하를 줄이세요.
예시:
- public void updateCompletion(ParticipantPrincipal participantPrincipal, boolean isCompleted) { - Participant participant = getParticipant(participantPrincipal.id()); - participant.updateCompletionAs(isCompleted); - //Todo: 인메모리 도입 / 식당 소거와 같이 핸들링 가능하지 않을까? [2025-10-21 14:58:44] - applicationEventPublisher.publishEvent(new ParticipantStateUpdatedEvent(participant.getPickeatId())); - } + public void updateCompletion(ParticipantPrincipal participantPrincipal, boolean isCompleted) { + Participant participant = getParticipant(participantPrincipal.id()); + boolean before = participant.getIsCompleted(); // 도메인에 맞는 접근자 사용 + participant.updateCompletionAs(isCompleted); + if (before != participant.getIsCompleted()) { + applicationEventPublisher.publishEvent(new ParticipantStateUpdatedEvent(participant.getPickeatId())); + } + }도메인에 반환값(boolean changed)을 주도록 바꾸는 것도 좋은 대안입니다.
backend/src/main/java/com/pickeat/backend/tobe/restaurant/application/RestaurantSearchFacade.java (1)
49-56: 캐시 워밍 실패 시 사용자 경험 저하
warmUpCache()가 실패하면 전체 API 요청이 실패합니다. 하지만 캐시 워밍은 성능 최적화를 위한 것이므로, 실패해도 서비스 자체는 동작해야 하지 않을까요?개선 제안:
private void warmUpCache(String pickeatCode) { try { Pickeat pickeat = pickeatRepository.findByCode(new PickeatCode(pickeatCode)) .orElseThrow(() -> new BusinessException(ErrorCode.PICKEAT_NOT_FOUND)); List<Restaurant> restaurants = restaurantRepository.findByPickeatId(pickeat.getId()); for (Restaurant restaurant : restaurants) { participantLikesRepository.initial(restaurant.getId()); } } catch (Exception e) { // 로깅만 하고 예외는 전파하지 않음 log.warn("Cache warm-up failed for pickeatCode: {}", pickeatCode, e); } }캐시 워밍은 best-effort로 처리하고, 실패해도 조회 시점에 cache-miss로 처리되도록 하는 것이 resilient한 설계입니다.
추가로, 식당이 많을 경우 루프가 병목이 될 수 있으니
participantLikesRepository.batchInitial(restaurantIds)같은 배치 API를 고려해보세요.backend/src/main/java/com/pickeat/backend/pickeat/ui/PickeatController.java (1)
123-132: 호환성 유지 전략 좋습니다만, 마이그레이션 계획 추가 권장오타로 인한 엔드포인트 이슈를 호환성을 유지하며 해결한 것은 좋은 판단입니다.
개선 제안:
TODO 주석에 구체적인 마이그레이션 일정을 추가해주세요://Todo: 오타로 인한 호환성 문제 해결 이후 단일화 필요 // - 2025-11 말까지 클라이언트 마이그레이션 완료 후 // - 2025-12 초에 /deactive 경로 제거 예정 [2025-10-21] @PatchMapping({"/pickeats/{pickeatCode}/deactivate", "/pickeats/{pickeatCode}/deactive"})또한
/deactive경로 사용 시 응답 헤더에Deprecated: true나Warning헤더를 추가하면 클라이언트가 마이그레이션을 인지하기 쉽습니다:response.addHeader("Warning", "299 - \"Deprecated endpoint. Use /deactivate instead\"");
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (54)
backend/src/main/java/com/pickeat/backend/global/auth/JwtProvider.java(2 hunks)backend/src/main/java/com/pickeat/backend/global/cache/CacheManagerConfig.java(1 hunks)backend/src/main/java/com/pickeat/backend/global/cache/CacheNames.java(1 hunks)backend/src/main/java/com/pickeat/backend/global/cache/CacheType.java(1 hunks)backend/src/main/java/com/pickeat/backend/pickeat/application/ParticipantService.java(3 hunks)backend/src/main/java/com/pickeat/backend/pickeat/application/PickeatResultService.java(3 hunks)backend/src/main/java/com/pickeat/backend/pickeat/application/PickeatScheduler.java(3 hunks)backend/src/main/java/com/pickeat/backend/pickeat/application/PickeatService.java(5 hunks)backend/src/main/java/com/pickeat/backend/pickeat/domain/Participant.java(1 hunks)backend/src/main/java/com/pickeat/backend/pickeat/domain/PickeatDeactivatedEvent.java(1 hunks)backend/src/main/java/com/pickeat/backend/pickeat/domain/repository/ParticipantJpaRepository.java(1 hunks)backend/src/main/java/com/pickeat/backend/pickeat/domain/repository/ParticipantRepository.java(1 hunks)backend/src/main/java/com/pickeat/backend/pickeat/domain/repository/PickeatJpaRepository.java(1 hunks)backend/src/main/java/com/pickeat/backend/pickeat/domain/repository/PickeatRepository.java(1 hunks)backend/src/main/java/com/pickeat/backend/pickeat/domain/repository/PickeatResultRepository.java(1 hunks)backend/src/main/java/com/pickeat/backend/pickeat/infrastructure/ParticipantRepositoryImpl.java(1 hunks)backend/src/main/java/com/pickeat/backend/pickeat/infrastructure/PickeatDeactivatedEventHandler.java(1 hunks)backend/src/main/java/com/pickeat/backend/pickeat/infrastructure/PickeatRepositoryImpl.java(1 hunks)backend/src/main/java/com/pickeat/backend/pickeat/ui/PickeatController.java(4 hunks)backend/src/main/java/com/pickeat/backend/pickeat/ui/api/PickeatApiSpec.java(8 hunks)backend/src/main/java/com/pickeat/backend/restaurant/application/RestaurantService.java(6 hunks)backend/src/main/java/com/pickeat/backend/restaurant/domain/ParticipantLikes.java(1 hunks)backend/src/main/java/com/pickeat/backend/restaurant/domain/repository/ParticipantLikesRepository.java(1 hunks)backend/src/main/java/com/pickeat/backend/restaurant/domain/repository/RestaurantJpaRepository.java(1 hunks)backend/src/main/java/com/pickeat/backend/restaurant/domain/repository/RestaurantLikeJpaRepository.java(1 hunks)backend/src/main/java/com/pickeat/backend/restaurant/domain/repository/RestaurantLikeRepository.java(1 hunks)backend/src/main/java/com/pickeat/backend/restaurant/domain/repository/RestaurantRepository.java(1 hunks)backend/src/main/java/com/pickeat/backend/restaurant/infrastructure/RestaurantLikeRepositoryImpl.java(1 hunks)backend/src/main/java/com/pickeat/backend/restaurant/infrastructure/RestaurantRepositoryImpl.java(1 hunks)backend/src/main/java/com/pickeat/backend/tobe/restaurant/application/RestaurantSearchFacade.java(2 hunks)backend/src/main/java/com/pickeat/backend/tobe/restaurant/application/RestaurantService.java(6 hunks)backend/src/main/java/com/pickeat/backend/tobe/user/application/UserService.java(2 hunks)backend/src/main/java/com/pickeat/backend/user/application/UserService.java(2 hunks)backend/src/main/resources/db/migration/V11__drop_participant_count_column_from_pickeat.sql(1 hunks)backend/src/main/resources/db/migration/V12__drop_has_equal_like_column_from_pickeat_result.sql(1 hunks)backend/src/main/resources/db/migration/V13__update_pickeat_result_indirect_reference.sql(1 hunks)backend/src/main/resources/db/migration/V14__drop_participant_pickeat_foreign_key.sql(1 hunks)backend/src/main/resources/db/migration/V15__update_restaurant_schema_for_indirect_reference.sql(1 hunks)backend/src/test/java/com/pickeat/backend/pickeat/application/PickeatResultServiceTest.java(9 hunks)backend/src/test/java/com/pickeat/backend/pickeat/application/PickeatSchedulerTest.java(6 hunks)backend/src/test/java/com/pickeat/backend/pickeat/application/PickeatServiceTest.java(8 hunks)backend/src/test/java/com/pickeat/backend/pickeat/domain/repository/ParticipantJpaRepositoryTest.java(1 hunks)backend/src/test/java/com/pickeat/backend/pickeat/domain/repository/PickeatJpaRepositoryTest.java(1 hunks)backend/src/test/java/com/pickeat/backend/pickeat/domain/repository/PickeatResultRepositoryTest.java(1 hunks)backend/src/test/java/com/pickeat/backend/pickeat/infrastructure/ParticipantRepositoryImplCachingTest.java(1 hunks)backend/src/test/java/com/pickeat/backend/pickeat/infrastructure/PickeatRepositoryImplCachingTest.java(1 hunks)backend/src/test/java/com/pickeat/backend/restaurant/application/RestaurantServiceTest.java(13 hunks)backend/src/test/java/com/pickeat/backend/restaurant/domain/repository/RestaurantJpaRepositoryTest.java(1 hunks)backend/src/test/java/com/pickeat/backend/restaurant/domain/repository/RestaurantLikeJpaRepositoryTest.java(1 hunks)backend/src/test/java/com/pickeat/backend/restaurant/infrastructure/RestaurantRepositoryImplCachingTest.java(1 hunks)backend/src/test/java/com/pickeat/backend/tobe/acceptance_test/scenario/PickeatByTemplateScenarioTest.java(4 hunks)backend/src/test/java/com/pickeat/backend/tobe/restaurant/RestaurantServiceTest.java(3 hunks)backend/src/test/java/com/pickeat/backend/tobe/user/application/UserServiceTest.java(3 hunks)backend/src/test/java/com/pickeat/backend/user/application/UserServiceTest.java(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (12)
- backend/src/main/java/com/pickeat/backend/tobe/user/application/UserService.java
- backend/src/test/java/com/pickeat/backend/tobe/restaurant/RestaurantServiceTest.java
- backend/src/test/java/com/pickeat/backend/pickeat/infrastructure/ParticipantRepositoryImplCachingTest.java
- backend/src/main/java/com/pickeat/backend/pickeat/infrastructure/ParticipantRepositoryImpl.java
- backend/src/main/java/com/pickeat/backend/restaurant/infrastructure/RestaurantRepositoryImpl.java
- backend/src/main/java/com/pickeat/backend/pickeat/infrastructure/PickeatDeactivatedEventHandler.java
- backend/src/main/java/com/pickeat/backend/global/cache/CacheManagerConfig.java
- backend/src/main/java/com/pickeat/backend/pickeat/infrastructure/PickeatRepositoryImpl.java
- backend/src/test/java/com/pickeat/backend/restaurant/infrastructure/RestaurantRepositoryImplCachingTest.java
- backend/src/main/java/com/pickeat/backend/pickeat/application/PickeatScheduler.java
- backend/src/main/java/com/pickeat/backend/pickeat/domain/Participant.java
- backend/src/test/java/com/pickeat/backend/tobe/user/application/UserServiceTest.java
🧰 Additional context used
📓 Path-based instructions (1)
backend/**
⚙️ CodeRabbit configuration file
backend/**: 1. 팀 및 공식 컨벤션(docs/back-code-convention.md), 가독성, 예외처리, 테스트/확장/유지보수성, 모듈화, API/DB/보안 설계 기준을 기반으로 리뷰해주세요.
2. 객체지향, 최신 트렌드, 불필요한 로직, 클린코드, 리팩토링, 서비스/도메인 설계, 공통 예외 처리, 확장성도 함께 확인해주세요.
3. 각 리뷰 포인트별로 문제점과 대안, 장단점을 논리적으로 제시하고, 필요한 경우 예시 코드도 추가해 주세요.
4. 리뷰가 너무 많아서 피로감을 줄 수 있으니, 꼭 필요한 부분에 집중해주고, 나머지는 캡션으로 설명해주세요.
5. 리뷰 남겨주는 부분은 해당 라인 범위의 코멘트에 작성해주세요.
6. 미작성한 테스트 코드 케이스가 있다면, 어떤 테스트가 필요한지 제안해주세요. (예: API 테스트, Service 단위 테스트, Repository 단위 테스트)
Files:
backend/src/main/java/com/pickeat/backend/restaurant/domain/repository/RestaurantLikeRepository.javabackend/src/main/resources/db/migration/V13__update_pickeat_result_indirect_reference.sqlbackend/src/main/java/com/pickeat/backend/pickeat/domain/PickeatDeactivatedEvent.javabackend/src/main/java/com/pickeat/backend/restaurant/infrastructure/RestaurantLikeRepositoryImpl.javabackend/src/main/java/com/pickeat/backend/global/cache/CacheNames.javabackend/src/main/java/com/pickeat/backend/global/auth/JwtProvider.javabackend/src/main/java/com/pickeat/backend/pickeat/application/PickeatResultService.javabackend/src/main/java/com/pickeat/backend/global/cache/CacheType.javabackend/src/main/java/com/pickeat/backend/pickeat/application/ParticipantService.javabackend/src/test/java/com/pickeat/backend/user/application/UserServiceTest.javabackend/src/main/java/com/pickeat/backend/tobe/restaurant/application/RestaurantService.javabackend/src/main/java/com/pickeat/backend/pickeat/domain/repository/PickeatJpaRepository.javabackend/src/main/resources/db/migration/V15__update_restaurant_schema_for_indirect_reference.sqlbackend/src/main/java/com/pickeat/backend/restaurant/domain/repository/RestaurantJpaRepository.javabackend/src/main/java/com/pickeat/backend/restaurant/domain/repository/RestaurantLikeJpaRepository.javabackend/src/main/java/com/pickeat/backend/user/application/UserService.javabackend/src/main/java/com/pickeat/backend/restaurant/domain/ParticipantLikes.javabackend/src/main/java/com/pickeat/backend/tobe/restaurant/application/RestaurantSearchFacade.javabackend/src/main/java/com/pickeat/backend/pickeat/domain/repository/PickeatResultRepository.javabackend/src/main/java/com/pickeat/backend/pickeat/domain/repository/ParticipantJpaRepository.javabackend/src/main/resources/db/migration/V14__drop_participant_pickeat_foreign_key.sqlbackend/src/main/resources/db/migration/V12__drop_has_equal_like_column_from_pickeat_result.sqlbackend/src/test/java/com/pickeat/backend/pickeat/application/PickeatResultServiceTest.javabackend/src/test/java/com/pickeat/backend/restaurant/application/RestaurantServiceTest.javabackend/src/test/java/com/pickeat/backend/restaurant/domain/repository/RestaurantJpaRepositoryTest.javabackend/src/test/java/com/pickeat/backend/pickeat/domain/repository/PickeatJpaRepositoryTest.javabackend/src/main/java/com/pickeat/backend/restaurant/domain/repository/ParticipantLikesRepository.javabackend/src/test/java/com/pickeat/backend/restaurant/domain/repository/RestaurantLikeJpaRepositoryTest.javabackend/src/test/java/com/pickeat/backend/tobe/acceptance_test/scenario/PickeatByTemplateScenarioTest.javabackend/src/main/resources/db/migration/V11__drop_participant_count_column_from_pickeat.sqlbackend/src/main/java/com/pickeat/backend/restaurant/domain/repository/RestaurantRepository.javabackend/src/main/java/com/pickeat/backend/pickeat/domain/repository/PickeatRepository.javabackend/src/test/java/com/pickeat/backend/pickeat/application/PickeatServiceTest.javabackend/src/test/java/com/pickeat/backend/pickeat/application/PickeatSchedulerTest.javabackend/src/test/java/com/pickeat/backend/pickeat/infrastructure/PickeatRepositoryImplCachingTest.javabackend/src/main/java/com/pickeat/backend/pickeat/domain/repository/ParticipantRepository.javabackend/src/main/java/com/pickeat/backend/restaurant/application/RestaurantService.javabackend/src/main/java/com/pickeat/backend/pickeat/ui/api/PickeatApiSpec.javabackend/src/test/java/com/pickeat/backend/pickeat/domain/repository/ParticipantJpaRepositoryTest.javabackend/src/test/java/com/pickeat/backend/pickeat/domain/repository/PickeatResultRepositoryTest.javabackend/src/main/java/com/pickeat/backend/pickeat/ui/PickeatController.javabackend/src/main/java/com/pickeat/backend/pickeat/application/PickeatService.java
🧠 Learnings (1)
📚 Learning: 2025-10-21T07:49:05.932Z
Learnt from: wodnd0131
PR: woowacourse-teams/2025-pick-eat#417
File: backend/src/main/java/com/pickeat/backend/pickeat/domain/repository/PickeatResultRepository.java:10-12
Timestamp: 2025-10-21T07:49:05.932Z
Learning: Flyway를 사용하는 프로젝트에서는 JPA 엔티티의 `Column(unique = true)` 어노테이션이 자동으로 DB에 제약 조건을 생성하지 않습니다. Flyway 마이그레이션 파일에서 명시적으로 UNIQUE 제약 조건을 생성해야 합니다.
Applied to files:
backend/src/main/java/com/pickeat/backend/pickeat/domain/repository/PickeatResultRepository.java
🔇 Additional comments (26)
backend/src/test/java/com/pickeat/backend/tobe/acceptance_test/scenario/PickeatByTemplateScenarioTest.java (2)
27-27: 타입 변경이 일관되게 적용되었습니다! 👍도메인 모델 리팩토링에 따라
RestaurantResultResponse에서PickeatResultResponse로의 import 변경이 적절하게 반영되었습니다.
62-62: 오타 수정 감사합니다! ✨
텤플릿→템플릿수정으로 메서드명의 가독성이 개선되었습니다.backend/src/test/java/com/pickeat/backend/user/application/UserServiceTest.java (2)
65-80: 테스트 업데이트가 정확합니다! 🎯
ProviderPrincipal로의 마이그레이션이 테스트에도 일관되게 반영되었네요. 검증 로직도 모두 유효합니다.
83-92: 테스트는 적절하지만, 검증 책임의 계층이 명확한지 확인이 필요합니다.좋은 질문을 던지셨는데, 웹 검색 결과를 바탕으로 조금 더 구체적으로 안내하겠습니다.
Spring Security 모범 사례에 따르면, Principal 객체의 유효성 검증은 인증 레이어(AuthenticationProvider, UserDetailsService)에서 먼저 이루어져야 합니다. 현재 테스트 코드에서
ProviderPrincipal(2L, "google")을 직접 생성해서 통과시키는 것은:
- ✅ 비즈니스 로직 테스트는 좋음: 닉네임 중복 검증 로직이 제대로 작동하는지 확인
⚠️ 하지만 고려할 점:providerId나provider가 null/invalid인 경우는 실제 인증 과정에서 걸러져야 하는 사항따라서 다음을 확인해보세요:
- 인증 레이어에서 검증 중인가?
ProviderPrincipal생성/검증을 담당하는AuthenticationProvider구현체가 있다면, 그곳에서 null/빈값 체크가 이루어져야 합니다.- UserService에서 방어적 검증이 필요한가? 계층 간 신뢰도에 따라,
createUser메서드에서providerPrincipal != null정도의 최소한의 검증이 있으면 더 안전합니다.현재 테스트는 "정상 케이스"를 검증하고 있으므로 그대로 두고, 원본 리뷰의 ProviderPrincipal 엣지 케이스는 인증 레이어의 단위 테스트로 분리하시는 것을 추천드립니다. 이렇게 하면 각 계층의 책임이 명확해집니다.
backend/src/main/java/com/pickeat/backend/user/application/UserService.java (1)
30-35: 깔끔한 타입 마이그레이션입니다! ✨ 다만 원래 리뷰의 확인사항들은 이미 충분히 안전하니 참고하세요.검증 결과:
ProviderPrincipal 불변성: 해결됨 ✓
- Java record는 기본적으로 불변성을 언어 수준에서 보장합니다. 모든 필드가
private final로 자동 생성되므로 추가 검증이 불필요합니다.Null 안전성: 이미 관리 중 ✓
LoginController.signup()에서@Provider어노테이션으로 주입되므로, ArgumentResolver 계층에서 이미 검증됩니다. 메서드 내부의 null 체크는 오버엔지니어링입니다.결론: 코드가 정상이므로 원래 리뷰는 승인해도 됩니다. 다만 향후 리뷰 시 참고할 점:
- Java record = 자동 불변성 보장 (별도 검증 불필요)
@Provider같은 framework annotation = 주입 계층에서의 검증 담당 (메서드 중복 검증 불필요)backend/src/main/resources/db/migration/V12__drop_has_equal_like_column_from_pickeat_result.sql (1)
1-2:has_equal_like컬럼 삭제 안전성 확인됨 ✅코드베이스 전체 검색 결과, 해당 컬럼이 마이그레이션 파일(
V1__init.sql에서 생성, 현재 파일에서 삭제) 외에 다른 곳에서 참조되지 않는 것을 확인했습니다.Java 애플리케이션 코드에서도
hasEqualLike필드가 사용되지 않으므로, 안전하게 삭제할 수 있습니다. 도메인 모델 정합성도 맞는 상태네요. 👍배포 시 권장사항:
- 운영 DB는 마이그레이션 전 백업 진행 (표준 운영 절차)
- 마이그레이션은 불가역적이므로 스테이징 환경에서 1회 검증 후 배포
코드 변경은 문제없습니다.
backend/src/main/java/com/pickeat/backend/pickeat/domain/repository/ParticipantJpaRepository.java (1)
14-17: 원본 리뷰 코멘트는 잘못된 가정에 기반하고 있습니다. 코드는 올바르게 구현되어 있습니다.원본 리뷰에서
Participant엔티티에@Where절이 필요하다고 지적했는데, 실제로는 그럴 필요가 없습니다.🎯 실제 상황:
BaseEntity에 Hibernate의@SoftDelete애노테이션이 적용되어 있습니다- 이 기능은 Hibernate 6.2+의 최신 soft delete 패턴으로, 자동으로 모든 쿼리에서
deleted = true레코드를 필터링합니다- 따라서
findByPickeatId()같은 메서드도 자동으로 soft-deleted 레코드를 제외합니다✅ 현재 구현이 우수한 이유:
- 모든 엔티티가
BaseEntity를 상속받아 일관된 soft delete 처리- 수동으로
@Where절을 추가할 필요 없음- 레포지토리의 bulk soft delete 패턴이 모두 동일하게 구현됨
- Hibernate의 현대적인 soft delete 기능 활용
결론: 원본 리뷰 코멘트는 무시해도 됩니다.
ParticipantJpaRepository의bulkSoftDeleteByPickeatIdIn구현이 모두 올바릅니다.Likely an incorrect or invalid review comment.
backend/src/main/java/com/pickeat/backend/pickeat/domain/repository/PickeatJpaRepository.java (1)
25-27: 검증 완료: 소프트 삭제 패턴이 올바르게 구현되어 있습니다 ✅원본 리뷰 코멘트의 우려사항이 해소되었습니다.
Pickeat엔티티가 상속하는BaseEntity에 Hibernate의@SoftDelete애노테이션이 적용되어 있으며, 이는 다음을 자동으로 처리합니다:
- deleted 컬럼 매핑
- 모든 쿼리에 WHERE 필터 자동 적용
- 소프트 삭제 생명주기 관리
따라서
bulkSoftDeleteByIdIn메서드는 현대적이고 일관된 패턴으로 올바르게 구현되었습니다. 추가 수정이 필요하지 않습니다.참고:
@Where필터는 레거시 패턴이며,@SoftDelete는 Spring Data JPA 3.1+에서 지원하는 더 우수한 접근 방식입니다.backend/src/main/java/com/pickeat/backend/pickeat/application/PickeatService.java (1)
124-129: ID 비교를 통한 접근 검증 좋습니다.프록시/영속성 컨텍스트와 무관하게 안정적으로 비교돼요. 현재 방식 유지에 동의합니다.
backend/src/test/java/com/pickeat/backend/pickeat/application/PickeatSchedulerTest.java (1)
64-78: 플러시/클리어 타이밍과 존재 여부 검증이 명확합니다.카운트·존재성 체크로 스케줄러 동작을 깔끔히 보증합니다. 테스트 구조 좋아요.
backend/src/test/java/com/pickeat/backend/pickeat/domain/repository/PickeatJpaRepositoryTest.java (1)
47-53: 대량 소프트 삭제 케이스 커버리지 적절합니다.삭제 건수·잔존 엔티티 식별까지 검증해 회귀에 강합니다. 굿.
backend/src/test/java/com/pickeat/backend/pickeat/domain/repository/PickeatResultRepositoryTest.java (1)
58-64: 연관 결과 일괄 삭제 시나리오 테스트 좋습니다.카운트·잔존 항목의 pickeatId 일치까지 체크해 신뢰도가 높습니다.
backend/src/test/java/com/pickeat/backend/pickeat/application/PickeatServiceTest.java (1)
246-246: ---Java 21 요구사항 확인 완료 — 변경 불필요
프로젝트의
build.gradle에서languageVersion = JavaLanguageVersion.of(21)로 명시적으로 설정되어 있습니다.List.getFirst()는 Java 21에서 완벽하게 지원되므로, 현재 코드는 호환성 문제가 없습니다. 안심하고 진행하셔도 됩니다! 👍backend/src/main/java/com/pickeat/backend/restaurant/application/RestaurantService.java (1)
61-66: N+1 문제는 실제로 존재하지만, 제안된 솔루션이 구현되지 않았습니다현재 코드를 분석한 결과, 리뷰 코멘트의 접근은 방향은 맞지만 몇 가지 중요한 문제가 있습니다:
N+1은 캐싱으로 이미 완화됨:
ParticipantLikesRepository.findByRestaurantId()에@Cacheable이 적용되어 있고, Caffeine 캐시로 10분 TTL 관리 중입니다. 같은 restaurantId 재요청 시 캐시에서 즉시 반환됩니다.제안된 메서드가 존재하지 않음:
findByRestaurantIdIn()메서드가ParticipantLikesRepository에 없습니다ParticipantLikes.empty()팩토리 메서드가 없습니다- 따라서 제시된 예시 코드는 현 상태에서 실행 불가능합니다
더 시급한 문제: 캐시 일관성 이슈 🚨
like()라인 89에서 캐시된ParticipantLikes객체를 직접 수정:participantLikes.addParticipantId(participantId)cancelLike()라인 98에서도 마찬가지:participantLikes.removeParticipantId(participantId)RestaurantLikeDB는 저장/삭제되지만, 캐시된 객체 상태는 동기화되지 않음@CacheEvict어노테이션이 이 두 메서드에 없어 캐시 무효화가 발생하지 않습니다권장 개선:
like(),cancelLike()메서드에@CacheEvict(value = "participant:like", key = "#restaurantId")추가- 또는 더 안전하게, 캐시된 객체 직접 수정 대신 DB 상태 변경 후 캐시 갱신
Likely an incorrect or invalid review comment.
backend/src/main/java/com/pickeat/backend/pickeat/application/PickeatResultService.java (5)
92-96: null‑safe 필터 좋습니다Boolean.FALSE.equals 비교로 NPE 리스크 제거 깔끔합니다. “가끔은 정석이 최고” 👍
58-66: 레스토랑 조회 예외 처리 적절ID 기반 조회 + 도메인 예외 매핑이 명확합니다. 유지하세요. “심플 이즈 베스트” 👏
69-75: 이벤트 발행 타이밍: AFTER_COMMIT이 이미 적용되어 있습니다 ✓검증 결과,
PickeatDeactivatedEventHandler와ParticipantStateUpdatedEventHandler모두@TransactionalEventListener(phase = TransactionPhase.AFTER_COMMIT)으로 설정되어 있습니다. 따라서 캐시 무효화 작업이 트랜잭션 커밋 후에 안전하게 실행되며, 커밋 전 상태를 읽을 위험은 없습니다. 현재 구현이 이미 올바르게 처리되어 있으므로 추가 조치는 필요 없습니다.
75-79: 검증 완료: 유니크 제약이 Flyway에 명시적으로 정의되어 있습니다. 예외 처리가 안전합니다. ✓좋은 소식입니다. 검증 결과 다음을 확인했습니다:
- Flyway 마이그레이션 확인:
V1__init.sqlline 131에UNIQUE KEY \UK44xseqcxipx2ss3f0jyfwuvy4` (`pickeat_id`)`가 명시적으로 정의되어 있습니다.- 마이그레이션 히스토리: 이후
V12,V13마이그레이션에서도 이 제약을 제거하지 않아 현재 활성 상태입니다.- 예외 처리 안전성: 데이터베이스 레벨의 유니크 제약이 보장되므로, 중복 생성 시도 시
DataIntegrityViolationException이 확실히 발생하며, fallback 로직은 안전합니다.예외 메시지나 SQLState에 의존하지 않는 패턴으로 구현된 점도 훌륭합니다. 데이터베이스 제약 + 조용한 조회 패턴의 조합은 멀티스레드 환경에서 매우 안정적입니다.
68-79: 리뷰 의견이 부정확합니다. Pickeat 엔티티에는 like/cancelLike 메커니즘이 없습니다.코드 검증 결과, 이 리뷰의 핵심 가정이 맞지 않습니다:
like/cancelLike는 Restaurant에만 존재
RestaurantService에만like(),cancelLike()메서드 있음Pickeat도메인에는 like 기능 전혀 없음- 따라서 "like() 경로에 활성 픽잇만 허용 가드 추가" 제안은 관련 없음
TODO의 실제 의미
- "결과 만드는중에 좋아요되면 어카지?" → Restaurant에 대한 좋아요를 의미
- Pickeat 결과 생성 중에 참가자들이 레스토랑에 좋아요를 추가하면, 그 집계에 포함될 수도 안 될 수도 있다는 뜻
실제 문제는 다릅니다
private Map<Restaurant, Integer> getLikeCounts(List<Restaurant> availableRestaurants) { Map<Restaurant, Integer> restaurantLikeCounts = new HashMap<>(); for (Restaurant restaurant : availableRestaurants) { // 각 레스토랑마다 개별 쿼리 실행 → N+1 문제! ParticipantLikes participantLikes = participantLikesRepository.findByRestaurantId(restaurant.getId()); restaurantLikeCounts.put(restaurant, participantLikes.getCount()); } return restaurantLikeCounts; }
- 이게 실제 동시성/성능 이슈
원본 리뷰의 제안사항들은 코드 구조와 맞지 않으므로 무시하셔도 됩니다.
Likely an incorrect or invalid review comment.
backend/src/main/java/com/pickeat/backend/pickeat/application/ParticipantService.java (1)
33-39: 이벤트 핸들러의 AFTER_COMMIT 설정이 이미 적용되어 있습니다. ✨검증 결과,
ParticipantStateUpdatedEventHandler가 이미@TransactionalEventListener(phase = TransactionPhase.AFTER_COMMIT)으로 올바르게 구현되어 있네요.
createParticipant()에서 발행되는ParticipantStateUpdatedEvent가 트랜잭션 커밋 이후에 처리되어 캐시 정합성이 보장되고 있습니다. 좋은 설계입니다!backend/src/test/java/com/pickeat/backend/restaurant/application/RestaurantServiceTest.java (1)
40-43: 리뷰 코멘트가 잘못된 가정을 기반으로 작성되었습니다.코드 검증 결과,
ParticipantLikesRepository는 인터페이스가 아니라@Component어노테이션이 붙은 일반 클래스입니다. 따라서 원래 리뷰에서 우려한 "인터페이스 타입이 빈으로 등록되지 않을 수 있다"는 문제는 해당되지 않습니다.실제 코드 상태:
ParticipantLikesRepository:@Component+@RequiredArgsConstructor선언된 구체적인 클래스@DataJpaTest의@Import(ParticipantLikesRepository.class)는 해당 클래스를 빈으로 정상 등록- 테스트에서
@Autowired private ParticipantLikesRepository participantLikesRepository;로 주입받아 실제 사용 중결론: 테스트 컨텍스트 로딩에 문제가 없으며, 현재 구성은 의도대로 작동합니다. 코드를 수정할 필요가 없습니다.
Likely an incorrect or invalid review comment.
backend/src/main/java/com/pickeat/backend/tobe/restaurant/application/RestaurantService.java (3)
72-90: 이전 리뷰 피드백 반영 확인 완료! 👍이전에 지적되었던 두 가지 이슈가 모두 개선되었네요:
- ✅ 참가자-피킷 일치 검증 추가 (Lines 76-79)
- ✅ 변경된 타겟만 저장 (Line 89)
권한 검증과 불필요한 쓰기 방지가 모두 적용되어 안전하고 효율적인 코드가 되었습니다.
다만, supernovaMK님이 언급한 캐시-트랜잭션 정합성 문제는 여전히 남아있습니다.
saveAll()실패 시 캐시가 롤백되지 않는 이슈는TransactionAwareCacheManagerProxy또는 커밋 후 이벤트 기반 캐시 무효화로 해결할 수 있어요.
113-121: Null 안전성 개선 완료! 🎯이전에 지적된 NPE 위험이
Objects.equals()로 깔끔하게 해결되었습니다. Null-safe한 비교로 안정성이 향상되었네요.
123-127: 피킷 상태 검증 헬퍼 메서드 추가 우수별도의
validatePickeatState()메서드로 검증 로직을 분리한 것은 좋은 설계입니다. 다만like()메서드에서도 이 검증을 활용하면 더욱 일관성 있는 코드가 될 것 같아요.backend/src/main/java/com/pickeat/backend/pickeat/ui/api/PickeatApiSpec.java (1)
3-3: API 스펙 일관성 있게 업데이트 완료 ✨
ParticipantPrincipal기반 인증 모델과PickeatResultResponse타입 변경이 모든 엔드포인트에 일관되게 적용되었습니다.특히 다음 부분들이 잘 정리되었네요:
- 결과 생성/조회 API의 응답 타입 통일
- 참가자 인증이 필요한 엔드포인트들의 파라미터 타입 통일
- API 문서화 예시와 설명이 변경사항을 잘 반영
명세와 구현 간 일관성이 유지되어 클라이언트 개발자들이 혼란 없이 사용할 수 있겠습니다!
Also applies to: 8-8, 234-234, 270-274, 314-317, 429-432, 490-492, 532-536
backend/src/main/java/com/pickeat/backend/pickeat/ui/PickeatController.java (1)
134-145: NPE 방어 로직 추가 완벽! 🛡️이전 리뷰에서 지적된
@ParticipantInPickeat(required = false)사용 시 NPE 위험이 Line 140의 null 체크로 깔끔하게 해결되었습니다.Long participantId = (participantPrincipal != null) ? participantPrincipal.id() : null;Optional 파라미터 처리의 모범 사례입니다! 👏
backend/src/main/java/com/pickeat/backend/pickeat/domain/repository/PickeatJpaRepository.java
Outdated
Show resolved
Hide resolved
backend/src/main/java/com/pickeat/backend/pickeat/domain/repository/PickeatJpaRepository.java
Outdated
Show resolved
Hide resolved
| public interface PickeatRepository { | ||
|
|
||
| Optional<Pickeat> findByCode(PickeatCode uuid); | ||
| Optional<Pickeat> findByCode(PickeatCode pickeatCode); | ||
|
|
||
| List<Pickeat> findByRoomId(Long rooId); | ||
|
|
||
| List<Pickeat> findByRoomIdAndIsActive(Long roomId, Boolean isActive); | ||
|
|
||
| List<Pickeat> findByRoomIdIn(List<Long> roodIds); | ||
|
|
||
| List<Pickeat> findByUpdatedAtBetween(LocalDateTime startOfDay, LocalDateTime endOfDay); | ||
| Optional<Pickeat> findById(Long pickeatId); | ||
|
|
||
| Pickeat save(Pickeat pickeat); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
서비스 최적화를 위한 리포지토리 API 보강 (existsByIdAndCode).
getRejoinAvailableToPickeat 단일 쿼리화를 위해 존재 여부 체크 메서드를 추가해 주세요. 구현은 JPA/Querydsl/네이티브 중 팀 표준에 맞춰 주세요.
public interface PickeatRepository {
Optional<Pickeat> findByCode(PickeatCode pickeatCode);
List<Pickeat> findByRoomId(Long roomId);
List<Pickeat> findByRoomIdAndIsActive(Long roomId, Boolean isActive);
List<Pickeat> findByRoomIdIn(List<Long> roomIds);
Optional<Pickeat> findById(Long pickeatId);
+ boolean existsByIdAndCode(Long pickeatId, PickeatCode pickeatCode);
Pickeat save(Pickeat pickeat);
}- 장점: 쿼리 2→1, 불필요한 엔티티 로딩 제거, 캐시 영향 최소화
- 트레이드오프: 리포지토리 구현·테스트 추가 필요 (API 명세 합의 요망)
As per coding guidelines
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
backend/src/main/java/com/pickeat/backend/pickeat/domain/repository/PickeatRepository.java
lines 8-21: add a repository API to check existence by id and code to allow
single-query rejoin availability checks; add a method signature boolean
existsByIdAndCode(Long pickeatId, PickeatCode pickeatCode) to the interface and
implement it according to team standard (preferred: a JPA implementation using a
derived query or @Query, or Querydsl/native if that’s the repo convention) so
the service can replace the two-query flow with one exists check and avoid
loading the full entity.
| public List<RestaurantResponse> getPickeatRestaurants(String pickeatCode, Boolean isExcluded, Long participantId) { | ||
| Pickeat pickeat = getPickeatByCode(pickeatCode); | ||
| List<Restaurant> restaurants = restaurantRepository.findByPickeatAndIsExcludedIfProvided(pickeat, isExcluded); | ||
| List<Restaurant> restaurants = restaurantRepository.findByPickeatId(pickeat.getId()); | ||
| List<Restaurant> targets = getTargets(restaurants, isExcluded); | ||
| List<RestaurantResponse> response = new ArrayList<>(); |
There was a problem hiding this comment.
권한 검증 누락: participantId가 다른 Pickeat에 속해도 isLiked 노출 가능
현재 getPickeatRestaurants는 participantId에 대한 소속 검증이 없어, 다른 방의 참가자 ID로 호출하면 좋아요 여부를 조회할 수 있습니다(정보 노출). exclude와 동일하게 소속 검증을 적용해 주세요. 또한 participantId가 null일 수 있다면 안전하게 처리하세요.
예시:
- public List<RestaurantResponse> getPickeatRestaurants(String pickeatCode, Boolean isExcluded, Long participantId) {
+ public List<RestaurantResponse> getPickeatRestaurants(String pickeatCode, Boolean isExcluded, Long participantId) {
Pickeat pickeat = getPickeatByCode(pickeatCode);
+ if (participantId != null) {
+ Participant participant = getParticipant(participantId);
+ if (!Objects.equals(participant.getPickeatId(), pickeat.getId())) {
+ throw new BusinessException(ErrorCode.PICKEAT_ACCESS_DENIED);
+ }
+ }테스트 제안: (1) 동일 방 참가자 OK, (2) 다른 방 참가자 -> ACCESS_DENIED, (3) participantId=null -> isLiked=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.
| public List<RestaurantResponse> getPickeatRestaurants(String pickeatCode, Boolean isExcluded, Long participantId) { | |
| Pickeat pickeat = getPickeatByCode(pickeatCode); | |
| List<Restaurant> restaurants = restaurantRepository.findByPickeatAndIsExcludedIfProvided(pickeat, isExcluded); | |
| List<Restaurant> restaurants = restaurantRepository.findByPickeatId(pickeat.getId()); | |
| List<Restaurant> targets = getTargets(restaurants, isExcluded); | |
| List<RestaurantResponse> response = new ArrayList<>(); | |
| public List<RestaurantResponse> getPickeatRestaurants(String pickeatCode, Boolean isExcluded, Long participantId) { | |
| Pickeat pickeat = getPickeatByCode(pickeatCode); | |
| if (participantId != null) { | |
| Participant participant = getParticipant(participantId); | |
| if (!Objects.equals(participant.getPickeatId(), pickeat.getId())) { | |
| throw new BusinessException(ErrorCode.PICKEAT_ACCESS_DENIED); | |
| } | |
| } | |
| List<Restaurant> restaurants = restaurantRepository.findByPickeatId(pickeat.getId()); | |
| List<Restaurant> targets = getTargets(restaurants, isExcluded); | |
| List<RestaurantResponse> response = new ArrayList<>(); |
🤖 Prompt for AI Agents
In
backend/src/main/java/com/pickeat/backend/restaurant/application/RestaurantService.java
around lines 56 to 60, add ownership verification for participantId like the
exclude flow: if participantId is null, treat as anonymous and ensure isLiked is
always false; if participantId is provided, fetch the Participant and verify
participant.getPickeatId() (or participant.getPickeat().getId()) equals the
current pickeat.getId(), throwing an AccessDeniedException (or equivalent) when
it does not match; only after this check, compute and expose isLiked for
restaurants. Ensure null-safe handling and reuse existing participant
lookup/error types used elsewhere in the service.
| Participant participant = getParticipant(participantId); | ||
| Restaurant restaurant = getRestaurantById(restaurantId); | ||
| validateParticipantAccessToRestaurants(List.of(restaurant), participant); | ||
| restaurantLikeRepository.save(new RestaurantLike(participant, restaurant)); | ||
| restaurant.like(); | ||
| ParticipantLikes participantLikes = participantLikesRepository.findByRestaurantId(restaurantId); | ||
| restaurantLikeRepository.save(new RestaurantLike(participant.getId(), restaurant.getId())); | ||
| participantLikes.addParticipantId(participantId); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
좋아요 동시성: 중복 삽입 레이스 대비(DB 유니크 + 예외 매핑)
existsLike → save 사이에 경쟁이 생기면 중복 좋아요가 저장될 수 있습니다. (소프트삭제라면 부분 인덱스 필요) DB 유니크 제약과 DataIntegrityViolationException 매핑으로 방어하세요.
예시:
- restaurantLikeRepository.save(new RestaurantLike(participant.getId(), restaurant.getId()));
+ try {
+ restaurantLikeRepository.save(new RestaurantLike(participant.getId(), restaurant.getId()));
+ } catch (DataIntegrityViolationException e) {
+ throw new BusinessException(ErrorCode.PARTICIPANT_RESTAURANT_ALREADY_LIKED);
+ }DDL 제안(소프트삭제 컬럼이 deleted인 경우):
- UNIQUE (restaurant_id, participant_id) WHERE deleted = false
테스트 제안: 멀티 스레드/멀티 요청에서 하나만 성공하는지, 실패는 동일 비즈니스 예외로 매핑되는지. As per coding guidelines.
🏁 Script executed:
#!/bin/bash
# Find the full likeRestaurant method in RestaurantService
find backend -name "RestaurantService.java" -type f | head -5Length of output: 180
🏁 Script executed:
#!/bin/bash
# Get the full method context (lines around 98-104)
fd RestaurantService.java --type f && cat "$(fd RestaurantService.java --type f)" | head -150 | tail -80Length of output: 401
🏁 Script executed:
#!/bin/bash
# Search for RestaurantLike entity definition
fd -e java -g "RestaurantLike.java" Length of output: 79
🏁 Script executed:
#!/bin/bash
# Check for DataIntegrityViolationException handling in the codebase
rg -n "DataIntegrityViolationException" backend --type java -B 3 -A 3Length of output: 1992
🏁 Script executed:
#!/bin/bash
# Look for UniqueConstraint or unique index on RestaurantLike
rg -n "@UniqueConstraint|@Unique|unique.*restaurant.*participant" backend --type java -B 2 -A 2Length of output: 0
🏁 Script executed:
#!/bin/bash
# Check ParticipantLikes entity and addParticipantId method
fd ParticipantLikes.java --type fLength of output: 81
🏁 Script executed:
#!/bin/bash
# Search for existing concurrency tests related to likes
rg -n "like.*concurr|concurr.*like|thread|Thread" backend --type java -g "*test*" -iLength of output: 0
🏁 Script executed:
#!/bin/bash
# Check for soft delete implementation (deleted column)
rg -n "deleted.*=.*false|WHERE.*deleted" backend --type java -B 2 -A 2Length of output: 0
🏁 Script executed:
#!/bin/bash
# Look for existsLike or similar existence check
rg -n "existsLike|exists.*Like" backend --type java -B 3 -A 3Length of output: 3682
🏁 Script executed:
#!/bin/bash
# Check RestaurantLike repository methods
fd RestaurantLikeRepository.java --type fLength of output: 100
🏁 Script executed:
#!/bin/bash
# Get the full content of RestaurantService around the method
find backend -name "RestaurantService.java" -exec cat {} \; | grep -n -A 20 "def likeRestaurant\|void likeRestaurant\|public.*likeRestaurant"Length of output: 0
🏁 Script executed:
#!/bin/bash
# More comprehensive search with proper Java syntax
rg -n "likeRestaurant" backend/src/main/java --type java -B 2 -A 15Length of output: 14352
🏁 Script executed:
#!/bin/bash
# Find all saveRestaurantLike or similar methods
rg -n "save.*[Ll]ike|add.*[Ll]ike" backend/src/main --type java -C 5Length of output: 8948
🏁 Script executed:
#!/bin/bash
# Check DDL files for UNIQUE constraint
fd -e sql -e ddl | xargs rg -l "UNIQUE.*restaurant.*participant|restaurant_id.*participant_id.*unique" 2>/dev/nullLength of output: 0
🏁 Script executed:
#!/bin/bash
# Look for application.yml or properties files defining JPA behavior
fd -e yml -e yaml -e properties | xargs grep -l "jpa\|hibernate" 2>/dev/null | head -3Length of output: 0
🏁 Script executed:
#!/bin/bash
# Check for exception handling patterns in service layer
rg -n "catch.*DataIntegrity|GlobalExceptionHandler|ExceptionHandler" backend/src/main/java --type java -B 1 -A 5Length of output: 10593
🏁 Script executed:
#!/bin/bash
# Find ErrorCode enum to check if PARTICIPANT_RESTAURANT_ALREADY_LIKED exists
fd ErrorCode.java --type f && cat "$(fd ErrorCode.java --type f)"Length of output: 2939
🏁 Script executed:
#!/bin/bash
# Look at the complete RestaurantService file
cat $(fd RestaurantService.java --type f --path "*/application/*")Length of output: 233
🏁 Script executed:
#!/bin/bash
# Check imports in RestaurantService for exception handling
rg -n "import.*Exception|import.*javax.persistence|import.*org.springframework" "$(fd RestaurantService.java --type f)" | head -20Length of output: 220
🏁 Script executed:
#!/bin/bash
# Search for findByRestaurantId method signature
rg -n "findByRestaurantId" backend --type java -B 2 -A 2Length of output: 9781
🏁 Script executed:
#!/bin/bash
# Look for participantLikesRepository operations
rg -n "participantLikesRepository\." backend --type java -B 1 -A 1Length of output: 6247
🏁 Script executed:
#!/bin/bash
# Check RestaurantLike entity definition comprehensively
fd RestaurantLike.java && cat "$(fd RestaurantLike.java)"Length of output: 729
🏁 Script executed:
#!/bin/bash
# Search for all Like-related repositories and their methods
fd -e java "*LikeRepository.java" backend --type f -exec cat {} \;Length of output: 446
🏁 Script executed:
#!/bin/bash
# Check test files for concurrency scenarios
find backend -path "*/test/*" -name "*RestaurantService*Test.java" -o -name "*Like*Test.java" | head -5Length of output: 284
🏁 Script executed:
#!/bin/bash
# List test files related to restaurants or likes
fd -e java -g "*test*" -g "*Like*" backend | head -10Length of output: 967
🏁 Script executed:
#!/bin/bash
# Get basic structure of the repository to understand paths
find backend -type f -name "*.java" -path "*/entity/*" -o -path "*/domain/*" | grep -i like | head -5Length of output: 469
🏁 Script executed:
#!/bin/bash
# Final comprehensive search of the likeRestaurant method with full context
rg -n "def\s+likeRestaurant|void\s+likeRestaurant|public.*likeRestaurant" backend --type java -A 30Length of output: 8027
🏁 Script executed:
#!/bin/bash
# Search for save method calls in restaurants to understand pattern
rg -n "restaurantLikeRepository\.save|participantLikesRepository\." backend/src/main/java --type java -B 5 -A 5 | head -100Length of output: 11549
🏁 Script executed:
#!/bin/bash
# Check the exact structure - java files with specific patterns
rg -nU "(?s).*restaurantLikeRepository\.save.*participantLikes\.add" backend --type java -B 10 -A 5Length of output: 30777
🏁 Script executed:
#!/bin/bash
# Direct inspection of RestaurantService methods around line 98-104
awk 'NR>=80 && NR<=150' "$(fd RestaurantService.java)" 2>/dev/null || echo "File not found, searching..."
fd RestaurantService.java && awk 'NR>=80 && NR<=150' "$(fd RestaurantService.java)"Length of output: 460
🏁 Script executed:
#!/bin/bash
# Get the full RestaurantService file to understand complete context
fd "RestaurantService.java" -path "*/restaurant/application/*" --exec wc -l {} \;Length of output: 168
🏁 Script executed:
#!/bin/bash
# Show first 200 lines to find the method
fd "RestaurantService.java" -path "*/restaurant/application/*" --exec head -200 {} \;Length of output: 168
🏁 Script executed:
#!/bin/bash
# Search for the complete method with context using line numbers
file=$(fd "RestaurantService.java" -path "*/restaurant/application/*")
if [ -n "$file" ]; then
echo "=== RestaurantService content (lines 80-130) ==="
sed -n '80,130p' "$file"
echo ""
echo "=== Full file line count ==="
wc -l "$file"
fiLength of output: 168
🏁 Script executed:
#!/bin/bash
# Look for any like-related methods
file=$(fd "RestaurantService.java" -path "*/restaurant/application/*")
if [ -n "$file" ]; then
echo "=== Methods containing 'like' ==="
grep -n "like\|Like" "$file" -i
fiLength of output: 168
🏁 Script executed:
#!/bin/bash
# Check entity definitions
rg -n "class RestaurantLike|class ParticipantLikes" backend --type java -A 20 | head -80Length of output: 9973
🏁 Script executed:
#!/bin/bash
# Check for UniqueConstraint annotation more specifically
rg -n "@UniqueConstraint|@Column.*unique" backend --type java -B 5 -A 2 | grep -A 7 -B 5 "RestaurantLike\|restaurant_like"Length of output: 0
🏁 Script executed:
#!/bin/bash
# Final direct approach - list all java files in restaurant package
find backend -path "*/restaurant/*" -name "*.java" -type f | sortLength of output: 5783
동시성 제어: DB 유니크 제약과 예외 매핑으로 레이스 컨디션 방어 필수
existsLike() 체크(94번 라인)와 save() 호출(102번 라인) 사이에 경쟁 상태가 발생합니다. 두 요청이 동시에 existsLike()를 호출하면 모두 false를 반환받아 두 번째 체크를 통과하고, 결과적으로 중복된 좋아요가 저장될 수 있습니다.
문제점:
- RestaurantLike 엔티티에
@UniqueConstraint미설정 restaurantLikeRepository.save()호출 시DataIntegrityViolationException처리 부재- 멀티스레드 시나리오의 테스트 케이스 부재
해결방안:
-
DB 수준 제약 추가 (소프트삭제 컬럼이
deleted인 경우):CREATE UNIQUE INDEX idx_restaurant_like_unique ON restaurant_like (restaurant_id, participant_id) WHERE deleted = false
-
예외 매핑 추가 (RestaurantService의 like 메서드):
try { restaurantLikeRepository.save(new RestaurantLike(participant.getId(), restaurant.getId())); } catch (DataIntegrityViolationException e) { throw new BusinessException(ErrorCode.PARTICIPANT_RESTAURANT_ALREADY_LIKED); }
이는
PickeatResultService의 패턴과 일치합니다. -
테스트 추가:
- 멀티스레드에서 동일 레스토랑에 동시 좋아요 시도 시 정확히 하나만 성공하고 나머지는 실패하는지 검증
- 실패 응답이
PARTICIPANT_RESTAURANT_ALREADY_LIKED에러코드로 통일되는지 확인
🤖 Prompt for AI Agents
In
backend/src/main/java/com/pickeat/backend/restaurant/application/RestaurantService.java
around lines 98 to 104, there is a race between the existsLike() check and
restaurantLikeRepository.save() that can create duplicate likes; add a DB-level
unique constraint/index enforcing uniqueness on (restaurant_id, participant_id)
for non-deleted rows (e.g., WHERE deleted = false), and wrap the save() call in
a try/catch that catches DataIntegrityViolationException and rethrows a
BusinessException(ErrorCode.PARTICIPANT_RESTAURANT_ALREADY_LIKED) to map the DB
error to the domain error (consistent with PickeatResultService); finally add a
multithreaded test that concurrently attempts to like the same restaurant and
asserts exactly one success and others fail with
PARTICIPANT_RESTAURANT_ALREADY_LIKED.
| public void searchByLocation(LocationRestaurantRequest request, String pickeatCode) { | ||
| restaurantService.create(locationRestaurantSearchService.searchByLocation(request), pickeatCode); | ||
| warmUpCache(pickeatCode); | ||
| } | ||
|
|
||
| // Todo: 앞서 생성된 픽잇 삭제하는 작업에 대한 보상 필요[2025-09-25 17:33:13] | ||
| public void searchByWish(WishRestaurantRequest request, String pickeatCode) { | ||
| restaurantService.create(wishRestaurantSearchService.searchByWish(request), pickeatCode); | ||
| restaurantService.create(wishRestaurantSearchService.searchByWish(request), | ||
| pickeatCode); | ||
| warmUpCache(pickeatCode); | ||
| } | ||
|
|
||
| // Todo: 앞서 생성된 픽잇 삭제하는 작업에 대한 보상 필요[2025-09-25 17:33:13] | ||
| public void searchByTemplate(TemplateRestaurantRequest request, String pickeatCode) { | ||
| restaurantService.create(templateRestaurantSearchService.searchByTemplate(request), pickeatCode); | ||
| warmUpCache(pickeatCode); |
There was a problem hiding this comment.
트랜잭션 경계 밖 캐시 워밍으로 인한 정합성 문제 가능성
restaurantService.create()가 실패하여 롤백되더라도 warmUpCache()는 이미 실행되어 캐시가 생성됩니다. 이는 실제 DB에 없는 데이터가 캐시에 남는 문제를 유발할 수 있어요.
해결 방안:
restaurantService.create()내부에서 저장 성공 후 캐시 워밍 수행- Spring의
@TransactionalEventListener(phase = TransactionPhase.AFTER_COMMIT)를 활용해 커밋 성공 후에만 캐시 워밍 - 또는 캐시 워밍을 조회 시점으로 지연 (cache-aside 패턴)
예시 (이벤트 기반 접근):
// RestaurantService 내부
@Transactional
public void create(...) {
// 저장 로직
restaurantRepository.batchInsert(restaurants);
// 이벤트 발행
eventPublisher.publishEvent(new RestaurantsCreatedEvent(pickeat.getId()));
}
// 별도 리스너
@TransactionalEventListener(phase = TransactionPhase.AFTER_COMMIT)
public void warmUpCacheAfterCommit(RestaurantsCreatedEvent event) {
// 캐시 워밍
}현재 구조에서는 create가 실패해도 캐시는 워밍되어 불일치가 발생합니다. 트랜잭션 성공을 보장한 후 캐시 워밍을 수행하도록 개선해주세요.
| ALTER TABLE `pickeat_result` | ||
| DROP FOREIGN KEY `FKg5rkj84lmqukrfiuhos82thdx`; | ||
|
|
||
| ALTER TABLE `pickeat_result` | ||
| DROP FOREIGN KEY `FKlkhf7ufetto289l3lyx35f39o`; | ||
|
|
||
| ALTER TABLE `pickeat_result` | ||
| MODIFY COLUMN `pickeat_id` bigint NOT NULL, | ||
| MODIFY COLUMN `restaurant_id` bigint NOT NULL; |
There was a problem hiding this comment.
🧩 Analysis chain
NOT NULL 제약 추가 전 데이터 정합성 검증 필요
현재 마이그레이션은 pickeat_id와 restaurant_id를 NOT NULL로 변경하고 있는데, 기존 테이블에 NULL 값이 존재할 경우 마이그레이션이 실패합니다.
개선 제안:
- NOT NULL 제약 추가 전에 NULL 값 존재 여부를 검증하는 체크 추가
- NULL 값이 있다면 기본값 할당 또는 데이터 정리 로직 선행
-- 안전한 마이그레이션 예시
-- Step 1: 데이터 검증 (선택적)
-- SELECT COUNT(*) FROM pickeat_result WHERE pickeat_id IS NULL OR restaurant_id IS NULL;
-- Step 2: NULL 값 처리 (필요시)
-- UPDATE pickeat_result SET pickeat_id = 0 WHERE pickeat_id IS NULL;
-- Step 3: 제약 조건 변경
ALTER TABLE `pickeat_result`
MODIFY COLUMN `pickeat_id` bigint NOT NULL,
MODIFY COLUMN `restaurant_id` bigint NOT NULL;참고: 로컬에서 Flyway 테스트 완료하셨다고 하셨는데, 프로덕션 데이터 특성상 예외 케이스가 있을 수 있으니 배포 전 스테이징 환경에서 한 번 더 검증하시는 걸 추천드립니다.
또한 FK 제약을 제거한 후 재생성하지 않는 것이 의도적인지 확인이 필요합니다. 성능 최적화를 위한 것이라면 문서화하고, 아니라면 V14나 이후 마이그레이션에서 재생성하는 것을 고려해주세요.
🏁 Script executed:
#!/bin/bash
# V14 이후 마이그레이션에서 FK 재생성 여부 확인
fd "V1[4-9].*\.sql$" backend/src/main/resources/db/migration --exec cat {}Length of output: 3821
NOT NULL 제약 추가 전 NULL 값 검증이 필수입니다
V13 마이그레이션이 실행될 때 pickeat_id와 restaurant_id 컬럼에 NULL 값이 존재하면 마이그레이션이 실패합니다. 현재 스크립트에는 이에 대한 대응책이 없네요.
V14 마이그레이션에서 보이는 패턴처럼, 조건부 검증과 안전한 데이터 처리를 V13에 추가하세요:
-- Step 1: 데이터 상태 확인
SELECT COUNT(*) as null_count
FROM pickeat_result
WHERE pickeat_id IS NULL OR restaurant_id IS NULL;
-- Step 2: NULL 값 처리 (필요시)
UPDATE pickeat_result
SET pickeat_id = -1, restaurant_id = -1
WHERE pickeat_id IS NULL OR restaurant_id IS NULL;
-- Step 3: 제약 조건 변경
ALTER TABLE `pickeat_result`
MODIFY COLUMN `pickeat_id` bigint NOT NULL,
MODIFY COLUMN `restaurant_id` bigint NOT NULL;참고: V14 마이그레이션을 확인한 결과, FK 제약이 재생성되지 않고 인덱스만 생성되고 있습니다. 이것이 의도적인 설계라면 (예: 성능 최적화), 커밋 메시지나 마이그레이션 주석에 명확히 기록해주세요. 그렇지 않다면 적절한 마이그레이션에서 FK를 재생성하는 것을 검토하세요.
배포 전에 반드시 스테이징 환경에서 전체 마이그레이션 시퀀스를 테스트하시기 바랍니다.
🤖 Prompt for AI Agents
backend/src/main/resources/db/migration/V13__update_pickeat_result_indirect_reference.sql
lines 1-9: 현재 V13 마이그레이션이 pickeat_id 와 restaurant_id 를 NOT NULL로 변경하지만 NULL 값 존재
시 실패하므로, 마이그레이션에 NULL 검사와 안전 처리 로직을 추가하세요 — 먼저 NULL 건수 확인 쿼리를 실행하여 존재 여부를
판단하고(배포/검토 시 출력용), NULL이 있으면 안전한 기본값으로 대체하거나(예: 임시 -1 또는 적절한 FK 연계값) 또는 명시적으로
실패하도록 처리한 후에 NOT NULL 변경을 적용하되 데이터 정합성 유지가 필요하면 FK 재생성 여부도 검토하여 주석으로 의도 기록 또는 이후
마이그레이션에서 FK를 재생성하도록 명시하세요.
| ALTER TABLE `participant` | ||
| DROP FOREIGN KEY `FKt0fy0xdd97i4u9om07urapp6g`; |
There was a problem hiding this comment.
🧩 Analysis chain
외래키 DROP도 인덱스 로직처럼 조건부 처리가 필요합니다.
좋은 소식부터: 라인 4-19의 인덱스 DROP과 21-35의 CREATE 로직에서 INFORMATION_SCHEMA를 통해 조건부로 처리하신 점이 탁월합니다! 🎯
다만 라인 1-2의 외래키 DROP은 여전히 무조건 실행되고 있습니다. 이렇게 하면:
- 개발 환경에서 마이그레이션을 재실행할 때 오류 발생
- 배포 재시도 시 같은 문제 재현
- 마이그레이션의 멱등성(idempotency) 위반
인덱스 로직과 동일한 패턴으로 외래키도 보호해주세요:
SET @fk_name := 'FKt0fy0xdd97i4u9om07urapp6g';
SET @drop_fk_sql := (
SELECT IF(EXISTS (
SELECT 1
FROM INFORMATION_SCHEMA.KEY_COLUMN_USAGE
WHERE TABLE_SCHEMA = DATABASE()
AND TABLE_NAME = 'participant'
AND CONSTRAINT_NAME = @fk_name
AND REFERENCED_TABLE_NAME IS NOT NULL
),
CONCAT('ALTER TABLE `participant` DROP FOREIGN KEY `', @fk_name, '`'),
'SELECT 1')
);
PREPARE drop_fk_stmt FROM @drop_fk_sql;
EXECUTE drop_fk_stmt;
DEALLOCATE PREPARE drop_fk_stmt;그럼 모든 DDL 작업이 일관되게 안전해집니다.
추가 정보: MySQL은 외래키를 제거할 때 자동으로 관련 인덱스를 제거하므로, 라인 4-19의 명시적 인덱스 DROP이 실제로 필요한지도 한 번 확인해보시길 권합니다.
검증을 위해 마이그레이션 파일 내용과 관련 정보를 확인하겠습니다.
🏁 Script executed:
#!/bin/bash
# 마이그레이션 파일 전체 내용 확인
cat backend/src/main/resources/db/migration/V14__drop_participant_pickeat_foreign_key.sqlLength of output: 1389
🏁 Script executed:
#!/bin/bash
# 비교용으로 V13 마이그레이션 파일 확인
if [ -f "backend/src/main/resources/db/migration/V13__drop_participant_pickeat_foreign_key.sql" ]; then
cat backend/src/main/resources/db/migration/V13__drop_participant_pickeat_foreign_key.sql
fiLength of output: 0
🏁 Script executed:
#!/bin/bash
# 참여자 테이블 관련 마이그레이션 파일 목록
fd -e sql . backend/src/main/resources/db/migration/ | grep -E "(participant|pickeat)" | sortLength of output: 363
🌐 Web query:
MySQL외래키 제거 자동 인덱스 제거 동작
💡 Result:
요약: InnoDB는 FOREIGN KEY 제약을 만들 때(참조(자식) 테이블에) 해당 컬럼들로 시작하는 인덱스가 없으면 자동으로 인덱스를 생성합니다. 하지만 FOREIGN KEY를 제거(ALTER TABLE ... DROP FOREIGN KEY ...)해도 그 인덱스를 자동으로 삭제하지 않습니다. (다만 MySQL은 내부적으로 “해당 FK를 유지할 수 있는 다른 인덱스가 만들어지면 자동으로 생성된 인덱스를 조용히 제거할 수 있음”이라고 문서에 명시함.) 필요하면 ALTER TABLE ... DROP INDEX로 인덱스를 수동으로 제거하세요. [1][2]
참고: 자동 생성 인덱스에 대한 동작 설명(및 자동 삭제 예외)은 MySQL 공식 문서를 참조하세요. [1][2]
참고문헌
[1] MySQL Reference Manual — FOREIGN KEY Constraints. [2] 설명/실무 가이드(예: TablePlus 블로그).
라인 1-2 외래키 DROP은 반드시 조건부로 처리해야 합니다.
좋은 소식부터: 라인 4-19의 인덱스 DROP과 21-35의 CREATE 로직에서 INFORMATION_SCHEMA를 통해 탁월하게 조건부 처리하셨습니다! 🎯
문제: 라인 1-2의 외래키 DROP은 여전히 무조건 실행되고 있습니다.
이로 인한 영향:
- 개발 환경에서 마이그레이션 재실행 시
FOREIGN KEY ... does not exist오류 - 배포 자동화 도구(Flyway/Liquibase)의 재시도 메커니즘 동작 불가
- 다른 개발자의 로컬 환경에서 예측 불가능한 오류 발생
해결책: 인덱스 로직과 동일한 패턴으로 외래키도 보호하세요.
SET @fk_name := 'FKt0fy0xdd97i4u9om07urapp6g';
SET @drop_fk_sql := (
SELECT IF(EXISTS (
SELECT 1
FROM INFORMATION_SCHEMA.REFERENTIAL_CONSTRAINTS
WHERE CONSTRAINT_SCHEMA = DATABASE()
AND CONSTRAINT_NAME = @fk_name
AND TABLE_NAME = 'participant'
),
CONCAT('ALTER TABLE `participant` DROP FOREIGN KEY `', @fk_name, '`'),
'SELECT 1')
);
PREPARE drop_fk_stmt FROM @drop_fk_sql;
EXECUTE drop_fk_stmt;
DEALLOCATE PREPARE drop_fk_stmt;추가 학습: InnoDB는 FOREIGN KEY 제약을 만들 때 자동으로 인덱스를 생성하지만, FK를 제거해도 그 인덱스를 자동으로 삭제하지 않습니다. 따라서 라인 4-19의 명시적 인덱스 DROP이 필수이며, 현재 라인 21-35에서 idx_participant_pickeat_id를 새로 생성하시는 것도 정확한 설계입니다.
마이그레이션은 멱등성(idempotency)이 핵심이니, 모든 DDL 작업을 이 패턴으로 통일해 주세요. 🚀
| @Import({PickeatResultGenerator.class, PickeatResultService.class, RestaurantJdbcRepository.class, | ||
| RestaurantRepositoryImpl.class, RestaurantLikeRepositoryImpl.class, PickeatRepositoryImpl.class, | ||
| ParticipantRepositoryImpl.class, ParticipantLikesRepository.class, ParticipantLikesRepository.class}) | ||
| public class PickeatResultServiceTest { |
There was a problem hiding this comment.
@import 중복 제거
ParticipantLikesRepository.class가 두 번 등록되어 있습니다. 불필요한 중복이니 제거해 주세요. 테스트 동작에는 영향 없습니다.
적용 예:
-@Import({PickeatResultGenerator.class, PickeatResultService.class, RestaurantJdbcRepository.class,
- RestaurantRepositoryImpl.class, RestaurantLikeRepositoryImpl.class, PickeatRepositoryImpl.class,
- ParticipantRepositoryImpl.class, ParticipantLikesRepository.class, ParticipantLikesRepository.class})
+@Import({PickeatResultGenerator.class, PickeatResultService.class, RestaurantJdbcRepository.class,
+ RestaurantRepositoryImpl.class, RestaurantLikeRepositoryImpl.class, PickeatRepositoryImpl.class,
+ ParticipantRepositoryImpl.class, ParticipantLikesRepository.class})🤖 Prompt for AI Agents
In
backend/src/test/java/com/pickeat/backend/pickeat/application/PickeatResultServiceTest.java
around lines 33 to 36, the @Import annotation contains a duplicate entry of
ParticipantLikesRepository.class; remove the redundant second
ParticipantLikesRepository.class from the list so each repository/class is
imported only once, leaving the rest of the imports unchanged.
| Pickeat cachedPickeat = Mockito.mock(Pickeat.class); | ||
| when(cachedPickeat.getCode()).thenReturn(PICKEAT_CODE); | ||
|
|
||
| // when | ||
| repository.evictPickeatCache(cachedPickeat.getCode().toString()); | ||
| repository.findByCode(PICKEAT_CODE); | ||
|
|
||
| // then | ||
| verify(pickeatJpaRepository, times(2)).findByCode(PICKEAT_CODE); |
There was a problem hiding this comment.
🧩 Analysis chain
캐시 키 일치성 보장: PickeatCode vs String 혼용은 잠재 리스크
테스트에서 findByCode(PickeatCode)로 저장하고 evictPickeatCache(String)으로 무효화합니다. 실제 구현의 @Cacheable/@CacheEvict 키 전략이 완전히 동일하지 않으면(예: #code vs #code.toString()) 운영에서 캐시가 남아 있을 수 있습니다. 현재 테스트는 통과하지만, 키 정합성을 타입 레벨로 고정하면 안전합니다.
테스트/구현 동시 정렬 예시:
- 구현:
@Cacheable(value=..., key="#code")/@CacheEvict(value=..., key="#code"), 시그니처 모두PickeatCode. - 테스트:
- repository.evictPickeatCache(cachedPickeat.getCode().toString());
+ repository.evictPickeatCache(PICKEAT_CODE);추가 테스트 제안:
- 서로 다른 타입 키(String vs PickeatCode)로는 무효화되지 않음을 검증(의도적 실패 케이스)하여 회귀 방지. As per coding guidelines.
원하시면 구현/테스트 쌍으로 키 전략을 통일하는 PR 보조 패치를 제안드릴게요.
Also applies to: 97-108
검증을 위해 실제 구현 코드를 확인하겠습니다.
캐시 키 정합성을 검증하기 위해 구현 코드를 살펴보겠습니다.<!-- [scratchpad_update]
[web_search result]
Spring 문서에 따르면: @Cacheable과 @CacheEvict의 키가 일치해야 캐시 무효화가 제대로 동작한다. 구체적으로:
- 키 생성 전략이 동일해야 함 (#code vs #code.toString() 차이 중요)
- 기본 KeyGenerator는 모든 파라미터를 SimpleKey로 생성
- SpEL로 명시적 키 정의 시 양쪽이 정확히 일치해야 함
이제 실제 구현 코드를 확인해야 함.
[scratchpad_end] --><!-- [scratchpad_update]
[web_search result 2]
핵심 정보:
- Spring 캐시 키는 hashCode()와 equals()에 의존한다.
- PickeatCode와 String은 다른 타입이므로 equals()가 다르다.
- 기본 KeyGenerator는 SimpleKey를 사용하는데, 타입이 다르면 캐시 키가 다르다.
- @Cacheable과 @CacheEvict의 키가 일치하지 않으면 캐시 무효화가 실패한다.
리뷰 코멘트는 유효해 보인다. 하지만 실제 구현을 확인해야 한다:
- PickeatRepositoryImpl의 @Cacheable/@CacheEvict 구현
- PickeatCode의 equals()/hashCode() 구현
- 테스트 코드 라인 81-89, 97-108
[scratchpad_end] -->
이제 실제 코드를 검증하기 위해 저장소를 살펴보겠습니다.
🏁 Script executed:
#!/bin/bash
# 1. 테스트 파일의 전체 컨텍스트 확인 (70-110 라인)
echo "=== Test file context (lines 70-110) ==="
find . -type f -name "PickeatRepositoryImplCachingTest.java" -exec head -120 {} \; | tail -60
echo ""
echo "=== Searching for PickeatRepositoryImpl ==="
# 2. PickeatRepositoryImpl 파일 찾기
find . -type f -name "PickeatRepositoryImpl.java"
echo ""
echo "=== Searching for @Cacheable/@CacheEvict annotations ==="
# 3. Cacheable/CacheEvict 어노테이션 찾기
rg -n "@Cacheable|@CacheEvict" --type=java -C 2 backend/src/mainLength of output: 8453
🏁 Script executed:
#!/bin/bash
# 1. 테스트 파일에서 해당 라인 범위 확인
echo "=== Test file lines 70-110 ==="
find . -type f -name "PickeatRepositoryImplCachingTest.java" -exec sed -n '70,110p' {} \;
echo ""
echo "=== Searching for PickeatRepositoryImpl ==="
find . -type f -name "PickeatRepositoryImpl.java"
echo ""
echo "=== Searching for Pickeat cache annotation usage ==="
rg -n "@Cacheable|@CacheEvict" --type=java backend/src/main -A 2 -B 1 | head -50Length of output: 6765
캐시 키 정합성 개선 필수: 타입 혼용과 키 표현식 불일치 해소
검증 결과, 리뷰 코멘트의 지적이 정확합니다. 현재 구현에서 캐시 키 전략이 일치하지 않습니다:
- @Cacheable (라인 26):
key = "#pickeatCode.value.toString()"← PickeatCode 타입 파라미터 - @CacheEvict (라인 56):
key = "#pickeatCode"← String 타입 파라미터
메서드 시그니처도 다르므로 (PickeatCode vs String), 두 메서드의 캐시 무효화 계약이 불명확합니다. 지금은 String 변환이 일치하여 우연히 작동하지만, 유지보수 시 회귀 위험이 큽니다.
개선 방향 (둘 중 선택):
- PickeatCode 통일안 (권장 - 타입 안정성):
@Cacheable(key = "#pickeatCode.value") // .value만 사용하도록 단순화
public Optional<Pickeat> findByCode(PickeatCode pickeatCode)
@CacheEvict(key = "#pickeatCode.value") // String 대신 PickeatCode
public void evictPickeatCache(PickeatCode pickeatCode)- String 통일안 (심플함):
@Cacheable(key = "#pickeatCode") // String 직접 사용으로 단순화
public Optional<Pickeat> findByCode(String pickeatCode)
@CacheEvict(key = "#pickeatCode")
public void evictPickeatCache(String pickeatCode)테스트도 함께 정렬하여, 타입 일치성을 명시적으로 보장하세요. 참고로, 스프링 캐시는 SpEL 표현식을 기반으로 키를 생성하므로, 비명시적 변환은 예상 밖의 동작을 초래할 수 있습니다.
🤖 Prompt for AI Agents
In
backend/src/test/java/com/pickeat/backend/pickeat/infrastructure/PickeatRepositoryImplCachingTest.java
lines 81-89, the cache key expressions and method parameter types are
inconsistent between @Cacheable and @CacheEvict (one uses PickeatCode vs
String), causing fragile behavior; standardize to the PickeatCode-based
approach: change repository method signatures and their cache annotations to use
PickeatCode and the same SpEL key (e.g., key = "#pickeatCode.value"), update the
evict method to accept PickeatCode and use the identical key expression, and
adjust the test to pass/mocK a PickeatCode instance (or its value) so verify
calls use matching types/keys; ensure both annotation keys and method signatures
are aligned and update test assertions accordingly.
3 participants
Issue Number
#407
기존 객체참조 부분을 모두 간접참조로 바꾸다보니 변경이 전파된 부분이 많습니다 ㅠㅠ
커밋별로 확인해주시는 편이 파악하시기 수월할 것 같아요.
커밋별로 한 작업 간단히 정리해봤습니당
flyway 관련해서는 로컬에서 테스트 완료하긴했는데 검수 한번씩 꼭 부탁드려요!!
질문 지적 환영합니다~~
기반 세팅
2935a85 Spring Cache + Caffeine 의존성 추가
→ 이후 전역 캐싱 도입을 위한 기반 세팅입니다.
전역 캐시 구성
a79ad1c Caffeine 기반 글로벌 CacheManager 구성
→ TTL·사이즈·캐시 이름을 한 곳에서 관리하도록 중앙화했습니다.
도메인 구조 정리
c52ad63 Restaurant 계열 엔티티가 Pickeat을 Long ID로 간접 참조하도록 리팩토링
→ 양방향 연관 제거로 JPA 로딩/삭제 부담을 줄였습니다.
4df9574 Participant 엔티티도 Pickeat을 Long ID로 간접 참조하도록 변경
→ 순환 참조 위험을 줄이고 구조를 단순화했습니다.
830b09b PickeatResult에서 Pickeat/Restaurant 직접 필드를 Long ID로 교체
→ 도메인 전반을 간접 참조 방식으로 통일했습니다.
6974468 Pickeat의 참가자 수 필드 및 로직 제거
→ 스키마와 도메인 상태를 일관되게 유지했습니다.
508c2ed PickeatResult의 동점 여부 필드 제거 및 관련 로직 정리
→ 사용되지 않는 필드를 정리해 도메인 단순화.
0b22cee Restaurants 도메인을 PickeatResultGenerator로 대체
→ 레스토랑 선택 로직을 별도 서비스로 분리하여 테스트 용이성 및 응집도 향상.
캐시 도입 및 최적화
bc6a006 RestaurantRepositoryImpl에 Pickeat별 식당 조회 캐시 적용
→ 기존 도메인 기반 조회 대신 캐시 결과를 사용하도록 개선.
20e32f8 RestaurantLikeRepositoryImpl 캐싱 도입
→ 좋아요 수·참여자별 좋아요 목록 캐싱, save/delete 시 캐시 무효화 처리 추가.
13ca77e PickeatRepositoryImpl 캐싱 적용
→ 코드 기반 Pickeat 조회에 캐시 추가 및 비활성화 시 캐시 무효화.
ab464e9 ParticipantRepositoryImpl에 Spring Cache 적용
→ Pickeat별 참가자 목록 캐싱 및 저장/조회 로직 리팩토링.
이벤트 기반 캐시 무효화
eeb93d6 Pickeat 비활성화 이벤트 신설
→ Pickeat 비활성화 시 식당·Pickeat 캐시를 트랜잭션 이벤트로 통합 처리.
a047a2a Pickeat 비활성화 시 참가자 캐시도 함께 비우도록 이벤트 핸들러 보강
→ 기존 Pickeat/식당 캐시만 비워지던 문제를 해결.
9e44b1c 참가자 완료 상태 변경 이벤트 및 캐시 무효화 핸들러 추가
→ 참가자 상태 업데이트 시 캐시가 즉시 갱신되도록 개선.
Flyway 및 DB 정합성
fd54f64 PickeatResult·Restaurant 간 직접 연관을 Long ID 기반으로 변경
→ Flyway V10~V14 스키마 정리 및 인덱스 재구성 포함.
Check List
(Optional) Additional Description
Summary by CodeRabbit
릴리스 노트
새로운 기능
버그 수정
개선 사항