Conversation
# Conflicts: # backend/turip-app/src/main/java/turip/favorite/repository/FavoriteFolderAccountRepository.java # backend/turip-app/src/main/java/turip/favorite/service/FavoriteFolderAccountService.java # backend/turip-app/src/main/java/turip/favorite/service/FavoriteFolderService.java
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
Walkthrough계정 조회 API( Changes
Sequence DiagramssequenceDiagram
participant Client
participant Controller as FavoriteFolderController
participant Service as FavoriteFolderService
participant AccountSvc as FavoriteFolderAccountService
participant Repo as FavoriteFolderRepository
participant EventPub as ApplicationEventPublisher
participant Listener as FavoriteFolderEventListener
participant StreamSvc as FavoriteFolderStreamService
Client->>Controller: POST /api/v1/turips/{id}/join
Controller->>Service: joinMember(folderId, member)
Service->>Repo: findByIdWithLock(folderId)
Repo-->>Service: FavoriteFolder
Service->>AccountSvc: findOrCreate(folder, account)
AccountSvc-->>Service: FavoriteFolderAccount
Service->>EventPub: publishEvent(FavoriteFolderUpdateEvent(MEMBER_JOINED))
EventPub->>Listener: handleFavoriteFolderUpdateEvent(event)
Listener->>StreamSvc: sendMemberUpdateEvents(folderId, MEMBER_JOINED)
StreamSvc->>StreamSvc: build MemberUpdateStreamResponse
StreamSvc-->>Client: SSE push to subscribers
Service-->>Controller: FavoriteFolderJoinResponse
Controller-->>Client: 200 OK
sequenceDiagram
participant Client
participant Controller as FavoriteFolderController
participant Service as FavoriteFolderService
participant AccountSvc as FavoriteFolderAccountService
participant PlaceRepo as FavoritePlaceRepository
participant Repo as FavoriteFolderRepository
participant EventPub as ApplicationEventPublisher
participant Listener as FavoriteFolderEventListener
participant StreamSvc as FavoriteFolderStreamService
Client->>Controller: DELETE /api/v1/turips/{id}/exit
Controller->>Service: exitFolder(account, folderId)
Service->>AccountSvc: deleteByFavoriteFolderAndAccount(folder, account)
AccountSvc-->>Service: deletion result
alt last member removed
Service->>PlaceRepo: delete places for folder
Service->>Repo: delete FavoriteFolder
Service->>EventPub: publishEvent(FavoriteFolderUpdateEvent(FOLDER_DELETED))
else remaining members
Service->>EventPub: publishEvent(FavoriteFolderUpdateEvent(MEMBER_EXITED))
end
EventPub->>Listener: handleFavoriteFolderUpdateEvent(event)
Listener->>StreamSvc: sendMemberUpdateEvents(...) or sendFolderUpdateEvents(...)
StreamSvc-->>Client: SSE push to subscribers
Service-->>Controller: FavoriteFolderExitResponse
Controller-->>Client: 200 OK
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (1 warning, 2 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (8)
backend/turip-app/src/main/java/turip/favorite/service/FavoritePlaceService.java-146-152 (1)
146-152:⚠️ Potential issue | 🟡 Minor
Collectors.toSet()반환 타입의 가변성이 보장되지 않습니다Line 150의
ids.addAll(requestIds)는Collectors.toSet()의 결과를 가변 컬렉션으로 가정합니다. 그러나 Java 공식 스펙은 반환된Set의 "type, mutability, serializability, or thread-safety" 를 보장하지 않으므로, 이후 JDK 버전에서UnsupportedOperationException이 발생할 수 있습니다.🛡️ 제안 수정
- Set<Long> ids = existingPlaces.stream() - .map(fp -> fp.getFavoriteFolder().getId()) - .collect(Collectors.toSet()); - ids.addAll(requestIds); - return ids; + Set<Long> ids = new java.util.HashSet<>(requestIds); + existingPlaces.stream() + .map(fp -> fp.getFavoriteFolder().getId()) + .forEach(ids::add); + return ids;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/turip-app/src/main/java/turip/favorite/service/FavoritePlaceService.java` around lines 146 - 152, calculateAffectedFolderIds assumes the Set produced by existingPlaces.stream().map(...).collect(Collectors.toSet()) is mutable before calling ids.addAll(requestIds); to fix, collect into a known mutable set (e.g., use .collect(Collectors.toCollection(HashSet::new)) or wrap the result in new HashSet<>(...)) so ids is a mutable HashSet, ensuring ids.addAll(requestIds) will not throw UnsupportedOperationException; update the stream collection in calculateAffectedFolderIds accordingly and keep variable name ids as-is.backend/src/main/java/turip/auth/token/GoogleTokenParser.java-50-62 (1)
50-62:⚠️ Potential issue | 🟡 Minor
null체크에서 던진UnauthorizedException이catch (Exception e)블록에 재포착됨
idToken == null일 때 54-56번 라인에서UnauthorizedException을 던지지만, 이 예외는 곧바로 59번 라인의catch (Exception e)블록에 잡힙니다.UnauthorizedException이HttpStatusException→RuntimeException→Exception으로 상속되므로, 예외 처리 흐름에서 재포착되어 60번 라인에서 새로운UnauthorizedException으로 재생성됩니다. 실질적 결과는 동일하지만,null체크 분기가 예외 처리와 독립적으로 동작한다는 의도가 모호합니다.제안:
catch블록에서UnauthorizedException을 구분하여 재-throw하거나,null체크를 try-catch 외부로 분리하세요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/main/java/turip/auth/token/GoogleTokenParser.java` around lines 50 - 62, The parseIdToken method currently throws UnauthorizedException when verifier.verify returns null but then immediately catches all Exceptions and creates a new UnauthorizedException, obscuring intent; update parseIdToken to either perform the null check outside the try block (call verifier.verify(idTokenString) and if result is null throw UnauthorizedException, then wrap only the verification/parsing logic in try/catch for other exceptions) or in the existing catch block re-throw any caught UnauthorizedException (if e instanceof UnauthorizedException) and only map other exceptions to a new UnauthorizedException; reference the parseIdToken method and the verifier.verify(idTokenString) null-check to locate where to apply the change.backend/turip-app/src/main/java/turip/favorite/controller/FavoriteFolderController.java-619-622 (1)
619-622:⚠️ Potential issue | 🟡 MinorSwagger
description에 오탈자
"함께 튜립의 참여들의 정보를 조회한다."→"함께 튜립의 참여자들의 정보를 조회한다."로 수정이 필요합니다.📝 수정 제안
- description = "함께 튜립의 참여들의 정보를 조회한다." + description = "함께 튜립의 참여자들의 정보를 조회한다."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/turip-app/src/main/java/turip/favorite/controller/FavoriteFolderController.java` around lines 619 - 622, In FavoriteFolderController locate the `@Operation` annotation on the method (the annotation with summary "튜립 참여자 목록 조회 api") and correct the description string by replacing "함께 튜립의 참여들의 정보를 조회한다." with the proper wording "함께 튜립의 참여자들의 정보를 조회한다." so the Swagger description contains the fixed Korean term.backend/src/main/java/turip/auth/resolver/AuthMemberArgumentResolver.java-46-58 (1)
46-58:⚠️ Potential issue | 🟡 Minor
accountId가 null일 경우 명시적 처리가 누락되어 있습니다.
jwtProvider.parseToken(accessToken).get("accountId", Long.class)에서 토큰에accountId클레임이 없으면null이 반환됩니다. 이null이memberService.getByAccountId(null)로 전달되면 예측하기 어려운 동작(DB 쿼리 오류 등)이 발생할 수 있습니다. 현재는 포괄 catch 블록에서UnauthorizedException으로 변환되지만, 명시적 null 체크를 추가하면 디버깅이 용이해집니다.🛡️ null 체크 추가 제안
private Member getMember(String accessToken) { try { Long accountId = jwtProvider.parseToken(accessToken).get("accountId", Long.class); + if (accountId == null) { + throw new UnauthorizedException(ErrorTag.UNAUTHORIZED); + } return memberService.getByAccountId(accountId); } catch (ExpiredJwtException e) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/main/java/turip/auth/resolver/AuthMemberArgumentResolver.java` around lines 46 - 58, In getMember, explicitly handle a null accountId returned by jwtProvider.parseToken(accessToken).get("accountId", Long.class): after retrieving accountId, add a null check and if null throw new UnauthorizedException(ErrorTag.UNAUTHORIZED) (or another appropriate ErrorTag), otherwise call memberService.getByAccountId(accountId); this prevents passing null into memberService and makes the failure explicit.backend/src/main/java/turip/account/service/AccountService.java-3-3 (1)
3-3:⚠️ Potential issue | 🟡 Minor
jakarta.transaction.Transactional대신org.springframework.transaction.annotation.Transactional을 사용하세요.
jakarta.transaction.Transactional은 Spring에서도 동작하지만, 프로젝트 전역에서org.springframework.transaction.annotation.Transactional을 표준으로 사용하고 있습니다. Spring의@Transactional이readOnly,propagation,rollbackFor등 더 많은 제어 옵션을 제공하므로, 일관성을 위해 Spring 애노테이션으로 변경이 필요합니다.♻️ import 변경
-import jakarta.transaction.Transactional; +import org.springframework.transaction.annotation.Transactional;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/main/java/turip/account/service/AccountService.java` at line 3, Replace the Jakarta Transaction import with Spring's transaction annotation: remove the existing import jakarta.transaction.Transactional and add org.springframework.transaction.annotation.Transactional; update any usages of `@Transactional` in AccountService (and related classes in this file) to use the Spring annotation so you can leverage Spring-specific attributes like readOnly, propagation, and rollbackFor for consistency with the rest of the project.backend/src/main/java/turip/account/controller/MemberController.java-36-87 (1)
36-87:⚠️ Potential issue | 🟡 Minor마이그레이션 엔드포인트의 Swagger에 400 응답이 누락되어 있습니다
/members/migration은@AuthGuest를 통해device-fid헤더를 필요로 합니다.MemberApiTest.migrationWithoutDeviceFidHeader테스트에서 확인되듯 헤더 미포함 시 400이 반환되지만, Swagger 문서에는 401만 정의되어 있습니다.📄 제안: 400 응답 예시 추가
`@ApiResponses`(value = { `@ApiResponse`(responseCode = "204", description = "성공 예시"), + `@ApiResponse`( + responseCode = "400", + description = "실패 예시", + content = `@Content`( + mediaType = "application/json", + schema = `@Schema`(implementation = ErrorResponse.class), + examples = `@ExampleObject`( + name = "device fid required", + summary = "요청 헤더에 device-fid가 존재하지 않는 경우", + value = """ + { + "tag": "DEVICE_FID_REQUIRED", + "message": "요청 헤더에 device_fid가 존재하지 않습니다." + } + """ + ) + ) + ), `@ApiResponse`(responseCode = "401", ...) })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/main/java/turip/account/controller/MemberController.java` around lines 36 - 87, Add a 400 ApiResponse example to the migrate endpoint's Swagger annotations in MemberController so Swagger reflects the actual behavior when device-fid is missing; update the `@ApiResponses` on the migrate method (public ResponseEntity<Void> migrate(...)) to include an `@ApiResponse` with responseCode = "400", description like "Bad Request - missing device-fid", content using mediaType = "application/json" and schema = `@Schema`(implementation = ErrorResponse.class) plus an `@ExampleObject` showing the JSON tag/message used by the code (matching the behavior verified in MemberApiTest.migrationWithoutDeviceFidHeader).backend/turip-app/src/main/java/turip/favorite/service/FavoriteFolderService.java-63-76 (1)
63-76:⚠️ Potential issue | 🟡 Minor
joinMember에서 동시 요청 시MEMBER_JOINED이벤트가 중복 발행될 수 있습니다.
isMember검사(Line 68)와findOrCreate(Line 69-70) 사이에 다른 스레드가 동일한 member로 join을 완료할 수 있습니다. 두 스레드 모두isAlreadyJoined=false를 반환받아 이벤트가 두 번 발행될 수 있습니다.
findOrCreate의 반환값에서 신규 생성 여부를 판별하거나,isMember검사를findOrCreate내부로 옮기는 방식을 고려해 주세요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/turip-app/src/main/java/turip/favorite/service/FavoriteFolderService.java` around lines 63 - 76, joinMember currently checks favoriteFolderAccountService.isMember before calling favoriteFolderAccountService.findOrCreate which can allow a race where two threads both see not-member and both cause MEMBER_JOINED to be published; change the flow so the creation result is the source of truth: modify findOrCreate (or add a new method like findOrCreateReturningCreatedFlag) to indicate whether a new FavoriteFolderAccount was created, call that from joinMember (instead of relying on isMember), and only publish FavoriteFolderUpdateEvent.of(favoriteFolderId, ActionType.MEMBER_JOINED) when the findOrCreate result indicates a new creation; alternatively move the membership check logic into favoriteFolderAccountService.findOrCreate to perform an atomic check-and-create and return the created/already-existed status so joinMember can publish the event only when created.backend/src/test/java/turip/auth/api/AuthApiTest.java-60-86 (1)
60-86:⚠️ Potential issue | 🟡 Minor테스트
@DisplayName과 실제 검증 상태 코드가 불일치합니다.Line 61의
@DisplayName에는 "201 Created"로 기술되어 있지만, Line 82에서 실제로는statusCode(200)을 검증하고 있습니다. 컨트롤러의login메서드도ResponseEntity.ok()(200)를 반환하므로, DisplayName을 수정해야 합니다.🐛 수정 제안
- `@DisplayName`("신규 회원 로그인 성공 시 201 Created와 토큰을 응답한다") + `@DisplayName`("신규 회원 로그인 성공 시 200 OK와 토큰을 응답한다")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/test/java/turip/auth/api/AuthApiTest.java` around lines 60 - 86, The test DisplayName is wrong: update the `@DisplayName` on the loginNewMemberSuccess test to reflect the actual expected status (200 OK) instead of "201 Created" so it matches the assertion statusCode(200) and the controller's ResponseEntity.ok() response; locate the loginNewMemberSuccess method and change the DisplayName text accordingly.
🧹 Nitpick comments (31)
backend/turip-app/src/main/java/turip/favorite/service/FavoritePlaceService.java (1)
154-158: 이벤트 생성 방식 불일치 — 생성자 대신 팩토리 메서드 사용 권장Line 54, 115, 128에서는
FavoriteFolderUpdateEvent.of(...)팩토리 메서드를 사용하는 반면,publishFolderUpdateEvents()의 Line 156에서는new FavoriteFolderUpdateEvent(...)생성자를 직접 호출합니다. 일관성을 위해 팩토리 메서드를 사용하세요.♻️ 제안 수정
private void publishFolderUpdateEvents(Set<Long> folderIds) { folderIds.forEach(folderId -> - eventPublisher.publishEvent(new FavoriteFolderUpdateEvent(folderId, ActionType.FOLDER_PLACE_CHANGED)) + eventPublisher.publishEvent(FavoriteFolderUpdateEvent.of(folderId, ActionType.FOLDER_PLACE_CHANGED)) ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/turip-app/src/main/java/turip/favorite/service/FavoritePlaceService.java` around lines 154 - 158, publishFolderUpdateEvents currently constructs events with new FavoriteFolderUpdateEvent(folderId, ActionType.FOLDER_PLACE_CHANGED); change it to use the factory method FavoriteFolderUpdateEvent.of(folderId, ActionType.FOLDER_PLACE_CHANGED) and call eventPublisher.publishEvent(...) with that result to match usage elsewhere (e.g., lines using FavoriteFolderUpdateEvent.of) for consistent event creation.backend/src/main/java/turip/auth/controller/dto/response/RefreshTokenResponse.java (1)
3-4:LoginResponse와 팩토리 메서드 유무의 사소한 일관성 차이
LoginResponse에는 정적of()팩토리 메서드가 있지만RefreshTokenResponse에는 없습니다. 프로젝트 컨벤션에 따라 통일 여부를 고려하세요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/main/java/turip/auth/controller/dto/response/RefreshTokenResponse.java` around lines 3 - 4, RefreshTokenResponse lacks the static factory method that LoginResponse provides; add a public static factory method named of(...) to the RefreshTokenResponse record (matching the signature String accessToken, String refreshToken) so callers can construct instances via RefreshTokenResponse.of(accessToken, refreshToken), keeping API consistency with LoginResponse and project conventions.backend/src/main/java/turip/auth/controller/dto/request/RefreshTokenRequest.java (1)
3-4:refreshToken필드에 입력 유효성 검사 추가 고려
LoginRequest.idToken과 마찬가지로@NotBlank없이 null/빈 문자열이 그대로 토큰 검증 로직으로 전달될 수 있습니다.🛡️ 제안하는 수정
+import jakarta.validation.constraints.NotBlank; + public record RefreshTokenRequest( - String refreshToken + `@NotBlank` String refreshToken ) { }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/main/java/turip/auth/controller/dto/request/RefreshTokenRequest.java` around lines 3 - 4, The RefreshTokenRequest record exposes a refreshToken that currently lacks input validation; add a validation annotation (e.g., `@NotBlank`) to the refreshToken component in the RefreshTokenRequest record and import the corresponding Jakarta/Javax validation annotation, and ensure the controller method that accepts RefreshTokenRequest is annotated with `@Valid` so blank or null tokens are rejected before reaching the token verification logic.backend/src/main/java/turip/auth/controller/dto/request/LoginRequest.java (1)
3-4:idToken필드에 입력 유효성 검사 추가 고려현재
idToken에@NotBlank같은 제약 조건이 없어 null 또는 빈 문자열이GoogleTokenParser/AuthService까지 전달될 수 있습니다.🛡️ 제안하는 수정
+import jakarta.validation.constraints.NotBlank; + public record LoginRequest( - String idToken + `@NotBlank` String idToken ) { }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/main/java/turip/auth/controller/dto/request/LoginRequest.java` around lines 3 - 4, The LoginRequest record allows null/empty idToken to flow to GoogleTokenParser/AuthService; add bean validation by annotating the idToken parameter in the LoginRequest record with `@NotBlank` (import javax.validation.constraints.NotBlank) and ensure the controller endpoint that consumes LoginRequest uses `@Valid` on the method parameter so validation is triggered; this prevents null/empty tokens from reaching GoogleTokenParser and AuthService and returns a 400 on invalid input.backend/turip-app/src/main/java/turip/common/configuration/AsyncConfiguration.java (1)
22-22:CallerRunsPolicy가 HTTP 요청 스레드를 블로킹할 수 있음큐(100)와 최대 스레드 풀(8)이 모두 포화 상태일 때
CallerRunsPolicy는 호출자(HTTP 요청 스레드)가 SSE 이벤트 작업을 직접 실행하도록 합니다. SSE 이벤트는 비동기로 처리되는 게 목적이므로, 이 경우 HTTP 응답 지연이 발생합니다.포화 시 이벤트를 드랍(로깅)하거나 별도 처리하는 정책이 더 적합합니다.
♻️ 제안 수정
-executor.setRejectedExecutionHandler(new ThreadPoolExecutor.CallerRunsPolicy()); +executor.setRejectedExecutionHandler((r, exec) -> { + // 요청 스레드 블로킹 방지: 포화 시 이벤트 드랍 및 로깅 + log.warn("SSE event executor saturated, task dropped: {}", r); +});또는 표준
AbortPolicy를 사용하여@Async호출부에서RejectedExecutionException을 catch해 처리할 수도 있습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/turip-app/src/main/java/turip/common/configuration/AsyncConfiguration.java` at line 22, 현재 AsyncConfiguration의 executor.setRejectedExecutionHandler(...)에 ThreadPoolExecutor.CallerRunsPolicy를 사용하면 큐(100)와 최대스레드(8) 포화 시 HTTP 요청(호출자) 스레드를 블로킹하여 SSE 응답 지연을 초래합니다; 대신 ThreadPoolExecutor.AbortPolicy를 사용하거나, 더 나은 선택으로는 로그 후 작업을 드롭하는 커스텀 RejectedExecutionHandler를 설정하여 포화 시 이벤트를 버리고 로그만 남기게 변경하고(예: 클래스 AsyncConfiguration의 executor.setRejectedExecutionHandler 호출 지점), 동시에 `@Async를` 호출하는 코드에서 RejectedExecutionException을 잡아 적절히 처리(로깅/메트릭 증가/대체 경로)하도록 보완하세요.backend/turip-app/src/test/java/turip/favorite/service/FavoritePlaceServiceTest.java (1)
397-403:assertAll내부에서 실제 메서드 호출과 검증을 혼합하는 패턴
assertDoesNotThrow(실제remove호출)와verify(이벤트 검증)를 같은assertAll안에 두면 실행 순서에 암묵적으로 의존하게 됩니다. JUnit 5의 현재 구현은 순서대로 실행하므로 지금은 동작하지만,when블록에서 실제 호출을 분리하는 것이 더 명확합니다.♻️ 수정 제안
- // when & then - assertAll( - () -> assertDoesNotThrow(() -> favoritePlaceService.remove(account, favoriteFolderId, placeId)), - () -> verify(eventPublisher).publishEvent( - FavoriteFolderUpdateEvent.of(favoriteFolderId, ActionType.PLACE_DELETED)) - ); + // when + assertDoesNotThrow(() -> favoritePlaceService.remove(account, favoriteFolderId, placeId)); + + // then + verify(eventPublisher).publishEvent( + FavoriteFolderUpdateEvent.of(favoriteFolderId, ActionType.PLACE_DELETED));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/turip-app/src/test/java/turip/favorite/service/FavoritePlaceServiceTest.java` around lines 397 - 403, Split the actual invocation and verification: first call favoritePlaceService.remove(account, favoriteFolderId, placeId) inside its own assertDoesNotThrow (or plain call if preferred) so the side-effect happens deterministically, then perform the verification of eventPublisher.publishEvent(FavoriteFolderUpdateEvent.of(favoriteFolderId, ActionType.PLACE_DELETED)) separately (for example inside assertAll or as a separate verify statement). This removes the implicit ordering dependency caused by mixing assertDoesNotThrow and verify inside the same assertAll and references favoritePlaceService.remove, assertDoesNotThrow, eventPublisher.publishEvent, FavoriteFolderUpdateEvent.of, and ActionType.PLACE_DELETED.backend/turip-app/src/main/java/turip/favorite/stream/service/FavoriteFolderStreamService.java (1)
38-39:heartbeatInterval필드 주입 시 단위 테스트에서NullPointerException위험
@Value필드 주입은 Spring 컨텍스트 없이 생성자를 직접 호출하는 단위 테스트에서heartbeatInterval이null로 남습니다.startHeartbeat에서scheduleAtFixedRate(..., heartbeatInterval, ...)호출 시Long → long언박싱으로NullPointerException이 발생합니다. 생성자 주입으로 옮기거나 기본 타입long으로 변경하는 것을 권장합니다.♻️ 수정 제안 (생성자 주입으로 변경)
- `@Value`("${sse.heartbeat.interval:30}") - private Long heartbeatInterval; + private final long heartbeatInterval; public FavoriteFolderStreamService( FavoriteFolderService favoriteFolderService, FavoriteFolderAccountService favoriteFolderAccountService, - ScheduledExecutorService scheduler + ScheduledExecutorService scheduler, + `@Value`("${sse.heartbeat.interval:30}") long heartbeatInterval ) { this.favoriteFolderService = favoriteFolderService; this.favoriteFolderAccountService = favoriteFolderAccountService; this.scheduler = scheduler; + this.heartbeatInterval = heartbeatInterval; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/turip-app/src/main/java/turip/favorite/stream/service/FavoriteFolderStreamService.java` around lines 38 - 39, The heartbeatInterval field in FavoriteFolderStreamService is injected with `@Value` and can be null in unit tests causing an NPE when startHeartbeat calls scheduleAtFixedRate(..., heartbeatInterval, ...). Convert the `@Value` field injection to constructor injection: add a constructor parameter (Long or primitive long) for heartbeatInterval and assign it to the field (or change the field type to primitive long), and update usages in startHeartbeat to rely on the non-null constructor-initialized value so scheduleAtFixedRate cannot receive a null due to unboxing.backend/turip-app/src/main/java/turip/favorite/stream/service/FavoriteFolderEventListener.java (1)
17-29:@Async핸들러의 예외가 조용히 삭제될 수 있음
sendMemberUpdateEvents내부의favoriteFolderAccountService.findMembersByFavoriteFolder()등에서 DB 예외가 발생하면,sendMemberUpdateEvent·sendFolderUpdateEvent내부의 개별 emitter 예외 처리는 우회되고 예외가 비동기 executor까지 전파됩니다. Spring의 기본AsyncUncaughtExceptionHandler는 이를 로그만 찍고 삭제하므로, SSE 이벤트 누락 시 명확한 피드백이 없습니다.♻️ 수정 제안
`@Async`("sseEventExecutor") `@TransactionalEventListener`(phase = TransactionPhase.AFTER_COMMIT) public void handleFavoriteFolderUpdateEvent(FavoriteFolderUpdateEvent event) { - Long folderId = event.favoriteFolderId(); - ActionType action = event.actionType(); - - if (action == ActionType.MEMBER_JOINED || action == ActionType.MEMBER_EXITED) { - favoriteFolderStreamService.sendMemberUpdateEvents(folderId, action); - return; + try { + Long folderId = event.favoriteFolderId(); + ActionType action = event.actionType(); + + if (action == ActionType.MEMBER_JOINED || action == ActionType.MEMBER_EXITED) { + favoriteFolderStreamService.sendMemberUpdateEvents(folderId, action); + return; + } + + favoriteFolderStreamService.sendFolderUpdateEvents(folderId, action); + } catch (Exception e) { + log.error("[SSE] 이벤트 처리 중 오류 발생, event: {}", event, e); } - - favoriteFolderStreamService.sendFolderUpdateEvents(folderId, action); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/turip-app/src/main/java/turip/favorite/stream/service/FavoriteFolderEventListener.java` around lines 17 - 29, In FavoriteFolderEventListener.handleFavoriteFolderUpdateEvent wrap the calls to favoriteFolderStreamService.sendMemberUpdateEvents(...) and sendFolderUpdateEvents(...) in a try-catch that catches Exception, logs a clear error including folderId and action and the exception stack, and prevents the exception from propagating to the async executor; this ensures DB errors from methods like favoriteFolderAccountService.findMembersByFavoriteFolder() do not get swallowed by Spring's AsyncUncaughtExceptionHandler—optionally increment a metric or emit a fallback/error event inside the catch to surface the failure.backend/turip-app/src/main/java/turip/favorite/service/FavoriteFolderAccountService.java (1)
61-63: 읽기 전용 메서드에@Transactional(readOnly = true)추가를 권장합니다.
findMembersByFavoriteFolder는 조회만 수행하므로@Transactional(readOnly = true)를 명시하면 JPA dirty checking 비용을 절감하고 의도를 명확히 할 수 있습니다. 같은 맥락에서countByFavoriteFolder,isMember메서드도 동일합니다.♻️ 수정 제안
+ `@Transactional`(readOnly = true) public List<Member> findMembersByFavoriteFolder(Long favoriteFolderId) { return favoriteFolderAccountRepository.findMembersByFavoriteFolderId(favoriteFolderId); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/turip-app/src/main/java/turip/favorite/service/FavoriteFolderAccountService.java` around lines 61 - 63, Add `@Transactional`(readOnly = true) to the read-only service methods in FavoriteFolderAccountService to avoid unnecessary JPA dirty-checking and clarify intent: annotate findMembersByFavoriteFolder(Long favoriteFolderId), countByFavoriteFolder(...), and isMember(...) with `@Transactional`(readOnly = true) on their method declarations (or the class-level annotations if you prefer to cover all read-only operations), keeping existing return logic and repository calls intact.backend/turip-app/src/main/java/turip/favorite/repository/FavoriteFolderAccountRepository.java (1)
21-31: 두findMembers쿼리 간 중복이 존재합니다.
findMembersByFavoriteFolder와findMembersByFavoriteFolderId는 WHERE 절만 다르고 나머지 JPQL이 동일합니다.FavoriteFolder엔티티를 인자로 받는 버전은 내부적으로favoriteFolder.id를 사용하므로,findMembersByFavoriteFolderId만 유지하고 서비스 레이어에서folder.getId()를 전달하는 방식으로 통합할 수 있습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/turip-app/src/main/java/turip/favorite/repository/FavoriteFolderAccountRepository.java` around lines 21 - 31, Remove the duplicated JPQL by deleting the method findMembersByFavoriteFolder from FavoriteFolderAccountRepository and keep the single query method findMembersByFavoriteFolderId; update any service or caller code that passed a FavoriteFolder to instead call findMembersByFavoriteFolderId(folder.getId()) (where folder is a FavoriteFolder) so the repository only accepts Long favoriteFolderId and the JPQL remains unchanged.backend/turip-app/src/test/java/turip/favorite/repository/FavoriteFolderAccountRepositoryTest.java (1)
39-61:findMembersByFavoriteFolderId메서드에 대한 테스트가 누락되어 있습니다.현재
findMembersByFavoriteFolder(FavoriteFolder)메서드만 테스트하고 있으며,findMembersByFavoriteFolderId(Long)메서드에 대한 테스트가 없습니다. 서비스 레이어(FavoriteFolderAccountService.findMembersByFavoriteFolder)에서 실제로 사용하는 것은 ID 기반 메서드이므로, 해당 메서드도 테스트하는 것을 권장합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/turip-app/src/test/java/turip/favorite/repository/FavoriteFolderAccountRepositoryTest.java` around lines 39 - 61, Add a test in FavoriteFolderAccountRepositoryTest that mirrors findMembersByFavoriteFolder1 but calls the ID-based repository method: use favoriteFolder.getId() and invoke favoriteFolderAccountRepository.findMembersByFavoriteFolderId(folder.getId()), then assert the returned Member list size and emails match the saved members (same assertions as in findMembersByFavoriteFolder1); this ensures the repository method used by FavoriteFolderAccountService.findMembersByFavoriteFolder is covered.backend/src/main/java/turip/auth/service/RefreshTokenService.java (1)
3-3:jakarta.transaction.Transactional대신org.springframework.transaction.annotation.Transactional사용 권장이 파일에서는
jakarta.transaction.Transactional을 사용하고 있지만, 같은 프로젝트의 다른 서비스(예:GuestService.java)에서는org.springframework.transaction.annotation.Transactional을 사용하고 있습니다. Spring의@Transactional은readOnly,propagation,rollbackFor등 추가 옵션을 지원하며, 프로젝트 전반의 일관성을 위해 Spring 어노테이션을 사용하는 것이 좋습니다.♻️ import 수정 제안
-import jakarta.transaction.Transactional; +import org.springframework.transaction.annotation.Transactional;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/main/java/turip/auth/service/RefreshTokenService.java` at line 3, RefreshTokenService에서 현재 jakarta.transaction.Transactional을 사용하고 있어 프로젝트의 다른 서비스(예: GuestService)와 일관성이 없으니 jakarta.transaction.Transactional을 org.springframework.transaction.annotation.Transactional로 교체하고 클래스의 `@Transactional` 애너테이션이 있다면 필요한 옵션(readOnly, propagation, rollbackFor 등)이 있다면 Spring 애너테이션 문법으로 옮겨 반영하세요; 대상 식별자: RefreshTokenService 클래스와 해당 import 문을 찾아 org.springframework.transaction.annotation.Transactional로 변경하고 필요한 옵션을 명시적으로 설정합니다.backend/turip-app/src/test/java/turip/favorite/api/FavoriteFolderApiTest.java (2)
549-696: Join / ExitFolder 테스트에서 로그인 토큰 획득 로직이 중복됩니다
Join클래스와ExitFolder클래스의 각 테스트 메서드마다 accessToken을 얻기 위한 동일한 RestAssured 로그인 호출 패턴이 반복되고 있습니다. 헬퍼 메서드로 추출하면 가독성과 유지보수성이 향상됩니다.♻️ 헬퍼 메서드 추출 예시
private String loginAndGetAccessToken(String loginId, String loginPassword, String deviceFid) { Map<String, String> loginRequest = new HashMap<>( Map.of("loginId", loginId, "loginPassword", loginPassword)); return RestAssured.given() .port(port) .header("device-fid", deviceFid) .contentType(ContentType.JSON) .body(loginRequest) .when().post("/api/v1/auth/login/turip") .then() .statusCode(200) .extract() .cookie("accessToken"); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/turip-app/src/test/java/turip/favorite/api/FavoriteFolderApiTest.java` around lines 549 - 696, Tests in the Join nested class duplicate the RestAssured login call to obtain accessToken across multiple test methods; extract that repeated logic into a private helper method (e.g., loginAndGetAccessToken) and replace the inline login blocks in Join (and ExitFolder if present) with calls to that helper to return the accessToken, ensuring it accepts loginId/loginPassword/deviceFid and still asserts statusCode(200) and extracts the "accessToken" cookie.
526-546:readMembersById3테스트에서 사용되지 않는 테스트 데이터 셋업
nonOwnerAccountId,nonOwnerMemberId와ownerMemberId를 생성하고 있지만, 요청 시"nonMemberDeviceFid"헤더를 사용합니다. 이 device-fid에 대한 guest가 등록되지 않았으므로findOrCreateByDeviceFid에 의해 새로운 account/guest가 생성됩니다. 결과적으로nonOwnerAccountId관련 셋업은 전혀 사용되지 않으며, 테스트 의도를 파악하기 어렵습니다.
nonOwnerAccountId에 대해 guest를 등록하고 해당 device-fid를 사용하거나, 불필요한 셋업 코드를 제거하는 것을 권장합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/turip-app/src/test/java/turip/favorite/api/FavoriteFolderApiTest.java` around lines 526 - 546, The test readMembersById3 sets up nonOwnerAccountId, nonOwnerMemberId and ownerMemberId via testDataHelper.insertAccount/insertMember but then sends a request with header "device-fid" = "nonMemberDeviceFid" which is not associated with any guest, causing findOrCreateByDeviceFid to create a new account and making the nonOwner* setup unused; fix by either (A) register a guest for nonOwnerAccountId with device-fid "nonMemberDeviceFid" (use the helper that links device FIDs to accounts) so the request is performed as that non-owner account, or (B) remove the unused nonOwnerAccountId/nonOwnerMemberId setup lines to reflect that the request simulates an unknown device; update assertions/comments accordingly in readMembersById3.backend/src/test/java/turip/account/service/AccountServiceTest.java (1)
95-114: 삭제 순서 검증 고려
deleteAccountAndFavorites테스트에서verify호출로 각 메서드가 호출되었는지 확인하고 있지만, 삭제 순서(찜 콘텐츠 → 찜 폴더 → 계정)가 중요한 경우InOrder를 사용하여 순서까지 검증하는 것이 좋습니다. FK 제약 조건이 있다면 순서가 바뀌면 런타임에 오류가 발생할 수 있습니다.♻️ InOrder 적용 예시
+import static org.mockito.Mockito.inOrder; +import org.mockito.InOrder; ... void deleteAccountAndFavorites() { // given Long accountId = 1L; Account account = new Account(accountId); // when accountService.deleteAccountAndFavorites(account); // then - verify(favoriteContentRepository).deleteByAccount(account); - verify(favoriteFolderService).removeByAccount(account); - verify(accountRepository).delete(account); + InOrder inOrder = inOrder(favoriteContentRepository, favoriteFolderService, accountRepository); + inOrder.verify(favoriteContentRepository).deleteByAccount(account); + inOrder.verify(favoriteFolderService).removeByAccount(account); + inOrder.verify(accountRepository).delete(account); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/test/java/turip/account/service/AccountServiceTest.java` around lines 95 - 114, The test DeleteAccountAndFavorites::deleteAccountAndFavorites currently verifies each deletion call but not the order; update the test to assert the required deletion sequence by creating an InOrder for favoriteContentRepository, favoriteFolderService, and accountRepository and use inOrder.verify(...) to ensure accountService.deleteAccountAndFavorites(account) triggers favoriteContentRepository.deleteByAccount(account) first, then favoriteFolderService.removeByAccount(account), then accountRepository.delete(account).backend/src/main/java/turip/account/service/AccountService.java (1)
28-31:getById는 읽기 전용 메서드이므로@Transactional(readOnly = true)추가를 고려하세요.Spring의
@Transactional(readOnly = true)를 적용하면 Hibernate의 dirty checking을 건너뛰어 약간의 성능 이점이 있고, 코드 의도도 명확해집니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/main/java/turip/account/service/AccountService.java` around lines 28 - 31, The getById method in AccountService is a read-only lookup and should be annotated with `@Transactional`(readOnly = true) to express intent and gain potential Hibernate optimizations; add the annotation to the getById(Long accountId) method (and import org.springframework.transaction.annotation.Transactional if not already present) so the method signature reads accordingly while leaving existing logic (accountRepository.findById(...).orElseThrow(...)) unchanged.backend/src/main/java/turip/auth/domain/RefreshToken.java (1)
60-62:isExpired()에서LocalDateTime.now()직접 사용은 테스트 어려움을 유발할 수 있습니다.현재 시스템 클럭에 의존하므로 단위 테스트에서 만료 시점을 제어하기 어렵습니다.
Clock을 주입받거나, 외부에서 현재 시각을 파라미터로 전달하는 방식을 고려해 볼 수 있습니다.♻️ 파라미터 방식 예시
- public boolean isExpired() { - return expiredAt.isBefore(LocalDateTime.now()); + public boolean isExpired(LocalDateTime now) { + return expiredAt.isBefore(now); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/main/java/turip/auth/domain/RefreshToken.java` around lines 60 - 62, The isExpired() method in RefreshToken uses LocalDateTime.now() which makes tests time-dependent; modify RefreshToken.isExpired to accept the current time as a parameter (e.g., isExpired(LocalDateTime now)) or have the class take a java.time.Clock (inject via constructor) and use LocalDateTime.now(clock) instead, then update callers/tests to supply a fixed time or a fixed Clock so expiration behavior can be deterministically asserted; ensure references to expiredAt remain and compare expiredAt.isBefore(providedNow) or expiredAt.isBefore(LocalDateTime.now(clock)).backend/turip-app/src/test/java/turip/account/api/AccountApiTest.java (1)
41-72: 테이블 정리 로직을 공유 유틸리티로 추출하는 것을 권장합니다.13개 테이블의 DELETE 및 ID RESTART를
@BeforeEach에서 직접 수행하고 있습니다. 다른 API 테스트(예: AuthApiTest, MemberApiTest)에서도 동일한 패턴이 반복될 가능성이 높으므로,DatabaseCleaner같은 공통 컴포넌트로 추출하면 유지보수성이 향상됩니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/turip-app/src/test/java/turip/account/api/AccountApiTest.java` around lines 41 - 72, The setUp() in AccountApiTest duplicates table DELETEs and sequence resets via jdbcTemplate; extract this logic into a reusable DatabaseCleaner class with public methods like cleanTables(List<String> tables) and resetSequences(List<String> sequences) (or a single cleanAll() that encapsulates the 13-table list), inject or instantiate DatabaseCleaner in test classes, replace the current jdbcTemplate calls in AccountApiTest.setUp() with a single DatabaseCleaner.cleanAll() call, and update other test classes (e.g., AuthApiTest, MemberApiTest) to call the same DatabaseCleaner to avoid repetition and centralize maintenance.backend/turip-app/src/test/java/turip/favorite/service/FavoriteFolderServiceTest.java (1)
693-719: 경계값 테스트 보완을 고려해 보세요.
exitFolder1에서countByFavoriteFolder가 2를 반환하지만, 폴더 삭제 여부의 경계값은 0과 1 사이입니다.count=1(현재 멤버 외 한 명만 남은 경우)인 케이스를 추가하면 경계 조건 검증이 더 정확해집니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/turip-app/src/test/java/turip/favorite/service/FavoriteFolderServiceTest.java` around lines 693 - 719, Add a boundary test where countByFavoriteFolder returns 1 to verify behavior when the leaving member is the last other participant: create a new test (e.g., exitFolder_countOne_deletesFolder) that stubs favoriteFolderRepository.findByIdWithLock(folderId) to return the folder and favoriteFolderAccountService.countByFavoriteFolder(favoriteFolder) to return 1, call favoriteFolderService.exitFolder(account, folderId), then assert that FavoriteFolderExitResponse.isDeleted() is true and verify the favoriteFolderRepository deletion method (e.g., delete or deleteById) was called and eventPublisher.publishEvent(...) was invoked with the expected FavoriteFolderUpdateEvent for folder deletion.backend/turip-app/src/main/java/turip/account/controller/AccountController.java (1)
21-22:@RequiredArgsConstructor가 필요하지 않습니다클래스에 주입받는
final필드가 없으므로 이 어노테이션은 Lombok이 빈 생성자를 생성하게 하는데, Java가 이미 기본으로 제공합니다.♻️ 제안: 불필요한 어노테이션 제거
`@RestController` -@RequiredArgsConstructor `@RequestMapping`("/api/v1/accounts") `@Tag`(name = "Account", description = "계정 API") public class AccountController {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/turip-app/src/main/java/turip/account/controller/AccountController.java` around lines 21 - 22, 클래스 AccountController에 현재 주입되는 final 필드가 없으므로 Lombok의 `@RequiredArgsConstructor` 어노테이션을 제거하세요; AccountController 클래스 선언부에서 `@RequiredArgsConstructor` 애노테이션을 삭제하면 Java 기본 생성자가 그대로 사용되며 불필요한 Lombok 의존성을 없앨 수 있습니다.backend/src/test/java/turip/auth/service/AuthServiceTest.java (1)
60-66:testSecretKey필드를static final로 선언하세요테스트 전반에 걸쳐 변경되지 않는 상수이므로
private static final String으로 선언하는 것이 적절합니다.♻️ 제안 수정
- private String testSecretKey = "turip-test-jwt-secret-key-for-testing-purposes-only-32bytes"; + private static final String TEST_SECRET_KEY = "turip-test-jwt-secret-key-for-testing-purposes-only-32bytes"; private SecretKey signingKey; `@BeforeEach` void setUp() { - signingKey = Keys.hmacShaKeyFor(testSecretKey.getBytes(StandardCharsets.UTF_8)); + signingKey = Keys.hmacShaKeyFor(TEST_SECRET_KEY.getBytes(StandardCharsets.UTF_8)); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/test/java/turip/auth/service/AuthServiceTest.java` around lines 60 - 66, Change the testSecretKey field to a constant by declaring it as private static final String testSecretKey = "turip-test-jwt-secret-key-for-testing-purposes-only-32bytes"; leave signingKey as an instance field and keep the setUp() method using Keys.hmacShaKeyFor(testSecretKey.getBytes(StandardCharsets.UTF_8)) to initialize signingKey; no other logic changes needed.backend/src/main/java/turip/auth/token/JwtProvider.java (3)
19-21: 필드 이름의 접미사가 일관되지 않습니다 (ExpiredvsExpire)
accessTokenExpiredMs와refreshTokenExpireMs는 의미는 동일하지만 접미사가 다릅니다.@Value프로퍼티 키(access-token-expiration-ms,refresh-token-expiration-ms)도Expiration을 공통으로 사용하고 있으므로 필드명을 맞춰주는 것이 가독성에 좋습니다.♻️ 제안 수정
- private final long accessTokenExpiredMs; - private final long refreshTokenExpireMs; + private final long accessTokenExpirationMs; + private final long refreshTokenExpirationMs;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/main/java/turip/auth/token/JwtProvider.java` around lines 19 - 21, Rename the inconsistent field suffixes in JwtProvider to match the property naming convention: change accessTokenExpiredMs and refreshTokenExpireMs to accessTokenExpirationMs and refreshTokenExpirationMs, then update all references (constructors, getters, setters, usages) and any `@Value` injectors or property bindings to use the new names so they align with access-token-expiration-ms and refresh-token-expiration-ms.
85-102: Java 17의HexFormat으로 수동 hex 인코딩을 대체할 수 있습니다Spring Boot 3.x는 Java 17+를 요구하므로
HexFormat이 사용 가능합니다.♻️ 제안 수정
public String hashToken(String refreshToken) { try { MessageDigest digest = MessageDigest.getInstance("SHA-256"); byte[] hash = digest.digest(refreshToken.getBytes(StandardCharsets.UTF_8)); - - StringBuilder hexString = new StringBuilder(); - for (byte b : hash) { - String hex = Integer.toHexString(0xff & b); - if (hex.length() == 1) { - hexString.append('0'); - } - hexString.append(hex); - } - return hexString.toString(); + return HexFormat.of().formatHex(hash); } catch (NoSuchAlgorithmException e) { throw new RuntimeException("SHA-256 algorithm not found", e); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/main/java/turip/auth/token/JwtProvider.java` around lines 85 - 102, The manual byte-to-hex loop in JwtProvider.hashToken should be replaced with Java 17's HexFormat for clarity and performance: after computing the SHA-256 byte[] `hash`, return HexFormat.of().formatHex(hash) instead of building hexString in the loop; ensure you add the import for java.util.HexFormat and keep the existing MessageDigest instantiation and exception handling around MessageDigest.getInstance("SHA-256") unchanged.
71-83: 동일 토큰에 대해parseToken()을 두 번 호출합니다
getIssuedAt(token)과getExpiration(token)을 연속 호출하면AuthService에서 토큰이 두 번 파싱되고 서명 검증도 두 번 수행됩니다.Claims를 먼저 한 번만 얻어 공유하는 방식으로 개선할 수 있습니다.♻️ 제안 수정
public LocalDateTime getIssuedAt(String token) { - Claims claims = parseToken(token); - return claims.getIssuedAt().toInstant() + return parseToken(token).getIssuedAt().toInstant() .atZone(ZoneOffset.UTC) .toLocalDateTime(); } public LocalDateTime getExpiration(String token) { - Claims claims = parseToken(token); - return claims.getExpiration().toInstant() + return parseToken(token).getExpiration().toInstant() .atZone(ZoneOffset.UTC) .toLocalDateTime(); }또는
AuthService에서parseToken(token)결과를 한 번 받아issuedAt과expiration을 함께 꺼내는 방식도 고려할 수 있습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/main/java/turip/auth/token/JwtProvider.java` around lines 71 - 83, getIssuedAt(token) and getExpiration(token) each call parseToken(token) causing double parsing/signature verification; fix by calling parseToken once and sharing the Claims: add a new method (e.g., getIssuedAndExpiration or getTokenTimes) that calls parseToken(token) a single time, converts claims.getIssuedAt() and claims.getExpiration() to LocalDateTime and returns both (via a small DTO/record or Pair), then update callers (e.g., AuthService) to use this new method instead of calling getIssuedAt and getExpiration separately; alternatively provide overloaded getIssuedAt(Claims) and getExpiration(Claims) helpers and change AuthService to call parseToken(token) once and pass the Claims into those helpers.backend/src/main/java/turip/auth/resolver/AuthAccountArgumentResolver.java (1)
49-61:catch (Exception e)블록이 예기치 않은 예외를 숨길 수 있습니다
accountService.getById(accountId)호출 중 발생하는NullPointerException이나DataAccessException같은 예외도 모두UNAUTHORIZED로 변환됩니다. 운영 환경에서 인프라 장애가 인증 실패로 위장되어 디버깅이 어려워질 수 있습니다. 알려진 JWT 파싱 예외 외의 경우에는 재처리 또는 로그를 남기는 것을 권장합니다.♻️ 제안 수정
private Account getMemberAccount(String accessToken) { try { Long accountId = jwtProvider.parseToken(accessToken).get("accountId", Long.class); return accountService.getById(accountId); } catch (ExpiredJwtException e) { throw new UnauthorizedException(ErrorTag.ACCESS_TOKEN_EXPIRED); } catch (SignatureException e) { throw new UnauthorizedException(ErrorTag.ACCESS_TOKEN_SIGNATURE_INVALID); + } catch (JwtException e) { + throw new UnauthorizedException(ErrorTag.UNAUTHORIZED); } catch (Exception e) { + // 계정 조회 실패 등 예기치 않은 오류는 로깅 후 재처리 + log.error("Unexpected error while resolving member account", e); throw new UnauthorizedException(ErrorTag.UNAUTHORIZED); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/main/java/turip/auth/resolver/AuthAccountArgumentResolver.java` around lines 49 - 61, 현재 getMemberAccount에서 catch (Exception e)로 모든 예외를 UnauthorizedException(ErrorTag.UNAUTHORIZED)로 변환해 accountService.getById나 DB/인프라 예외가 인증 실패로 은폐됩니다; jwtProvider.parseToken에서 처리한 ExpiredJwtException/SignatureException 등 알려진 JWT 예외만 잡고, 그 외 예기치 않은 예외는 잡지 말고 로그를 남기한 뒤 그대로 재던지거나 적절한 내부 서버 오류로 변환하도록 변경하세요—구체적으로 getMemberAccount 내부에서 catch (Exception e)를 제거하거나 대체하여 jwtProvider.parseToken 관련 예외(ExpiredJwtException, SignatureException 등)만 처리하고, accountService.getById에서 발생할 수 있는 NullPointerException/DataAccessException 등은 로깅(accountId 포함) 후 rethrow 하거나 새로운 RuntimeException으로 감싸서 던지도록 구현하세요.backend/src/main/java/turip/auth/controller/AuthController.java (2)
30-33:@Controller대신@RestController사용을 권장합니다.REST API 컨트롤러에서
@Controller를 사용하고 있습니다. 현재 모든 메서드가ResponseEntity를 반환하므로 동작상 문제는 없지만,@RestController를 사용하는 것이 REST API 컨트롤러의 관례적 패턴입니다.♻️ 수정 제안
-import org.springframework.stereotype.Controller; +import org.springframework.web.bind.annotation.RestController;-@Controller +@RestController `@RequiredArgsConstructor` `@Tag`(name = "Auth", description = "인증 API") public class AuthController {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/main/java/turip/auth/controller/AuthController.java` around lines 30 - 33, Replace the class-level `@Controller` annotation on AuthController with `@RestController` (keep `@RequiredArgsConstructor` and `@Tag` intact) so the REST controller stereotype is used; update imports to import org.springframework.web.bind.annotation.RestController instead of org.springframework.stereotype.Controller and remove the unused Controller import if necessary.
37-87: 로그인 및 토큰 갱신 엔드포인트에도device-fid누락 시 에러 응답 문서화가 필요합니다.
/logout엔드포인트에는device-fid헤더 누락 시 400 에러(DEVICE_FID_REQUIRED)가 Swagger에 문서화되어 있지만,/login/google과/token엔드포인트에는 동일한 에러 케이스가 문서화되어 있지 않습니다. 세 엔드포인트 모두@RequestHeader("device-fid")를 필수로 요구하므로, 일관성을 위해 모든 엔드포인트에 해당 에러 응답을 문서화하는 것이 좋습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/main/java/turip/auth/controller/AuthController.java` around lines 37 - 87, Add a 400 ApiResponse for missing device-fid to the Swagger docs for the AuthController endpoints that require `@RequestHeader`("device-fid"): update the `@ApiResponses` on the login method (public ResponseEntity<LoginResponse> login(...)) and the token refresh handler to include an `@ApiResponse`(responseCode = "400", description = "device-fid missing", content = `@Content`(mediaType = "application/json", schema = `@Schema`(implementation = ErrorResponse.class), examples = `@ExampleObject`(name = "device-fid required", summary = "DEVICE_FID_REQUIRED", value = "{ \"tag\": \"DEVICE_FID_REQUIRED\", \"message\": \"device-fid header is required.\" }"))), ensuring the new response matches the existing ErrorResponse schema and example style used for other error responses.backend/src/test/java/turip/auth/api/AuthApiTest.java (1)
127-131: 예외 클래스의 FQN(Fully Qualified Name) 대신 import 사용을 권장합니다.
turip.common.exception.custom.UnauthorizedException과turip.common.exception.ErrorTag를 인라인 FQN으로 사용하고 있습니다. import문을 추가하면 가독성이 향상됩니다.♻️ 수정 제안
파일 상단 import 영역에 추가:
import turip.common.exception.custom.UnauthorizedException; import turip.common.exception.ErrorTag;그 후:
when(googleTokenParser.getProviderId(anyString())) - .thenThrow(new turip.common.exception.custom.UnauthorizedException( - turip.common.exception.ErrorTag.ID_TOKEN_NOT_VALID)); + .thenThrow(new UnauthorizedException(ErrorTag.ID_TOKEN_NOT_VALID));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/test/java/turip/auth/api/AuthApiTest.java` around lines 127 - 131, Replace the inline fully-qualified exception class names with imports: add imports for turip.common.exception.custom.UnauthorizedException and turip.common.exception.ErrorTag at the top of the test file, then update the when(...) stub that throws new turip.common.exception.custom.UnauthorizedException(turip.common.exception.ErrorTag.ID_TOKEN_NOT_VALID) to use the short names UnauthorizedException and ErrorTag (i.e., throw new UnauthorizedException(ErrorTag.ID_TOKEN_NOT_VALID)) so the call site (googleTokenParser.getProviderId(...)) is more readable.backend/src/main/java/turip/auth/service/AuthService.java (1)
108-120:saveRefreshToken에서ExpiredJwtException/SignatureException이 발생할 가능성이 없습니다.이 메서드는
jwtProvider.generateRefreshToken()으로 방금 생성한 토큰을 대상으로getIssuedAt,getExpiration,hashToken을 호출합니다. 방금 생성된 토큰이 만료되거나 서명이 잘못될 수 없으므로, 이 try-catch 블록은 불필요한 방어 코드입니다. 만약 여기서 예외가 발생한다면 그것은JwtProvider자체의 버그이므로 서버 에러로 전파되어야 합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/main/java/turip/auth/service/AuthService.java` around lines 108 - 120, The try-catch in saveRefreshToken is overly defensive because the method operates on a freshly generated token (jwtProvider.generateRefreshToken), so ExpiredJwtException and SignatureException cannot occur; remove the try-catch (or at least remove the specific catches for ExpiredJwtException and SignatureException and the generic conversion to UnauthorizedException) so any JwtProvider errors bubble up as server errors; leave the core calls (jwtProvider.getIssuedAt, jwtProvider.getExpiration, jwtProvider.hashToken, and refreshTokenService.save) intact in saveRefreshToken so failures from JwtProvider surface instead of being mapped to UnauthorizedException.backend/turip-app/src/main/java/turip/favorite/service/FavoriteFolderService.java (1)
78-89:findAllByAccount에서 폴더별로 N+1 쿼리가 발생합니다.각 폴더에 대해
favoritePlaceRepository.countByFavoriteFolder()와favoriteFolderAccountService.countByFavoriteFolder()를 개별 호출하므로, 폴더 수 N에 대해 2N개의 추가 쿼리가 실행됩니다. 폴더 수가 많아지면 성능에 영향을 줄 수 있습니다.향후 단일 쿼리로 placeCount와 memberCount를 함께 조회하는 리팩터링을 고려해 볼 수 있습니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/turip-app/src/main/java/turip/favorite/service/FavoriteFolderService.java` around lines 78 - 89, findAllByAccount currently triggers N+1 queries because it calls favoritePlaceRepository.countByFavoriteFolder(...) and favoriteFolderAccountService.countByFavoriteFolder(...) for each folder returned by favoriteFolderRepository.findAllByAccountOrderByFavoriteFolderAccountIdAsc(...); fix this by replacing per-folder counts with two batched/grouped queries that return maps of favoriteFolderId->placeCount and favoriteFolderId->memberCount (e.g., add repository methods that count grouped by favoriteFolder IDs or use a single DTO query), then iterate the favoriteFolder list and call FavoriteFolderDetailResponse.of(favoriteFolder, account, placeCountMap.get(id), memberCountMap.get(id)) to build the response without per-folder DB calls.backend/src/main/java/turip/account/service/MemberService.java (1)
3-3:jakarta.transaction.Transactional대신org.springframework.transaction.annotation.Transactional사용을 권장합니다.이 파일에서는
jakarta.transaction.Transactional을 사용하고 있지만, 같은 PR의FavoriteFolderService에서는org.springframework.transaction.annotation.Transactional을 사용하고 있습니다. Spring의@Transactional이readOnly,propagation,isolation등 추가 속성을 지원하므로, 일관성과 확장성을 위해 Spring 어노테이션 사용을 권장합니다.♻️ 수정 제안
-import jakarta.transaction.Transactional; +import org.springframework.transaction.annotation.Transactional;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/main/java/turip/account/service/MemberService.java` at line 3, Replace the Jakarta Transactional import in MemberService with Spring's transactional annotation to match FavoriteFolderService: change the import from jakarta.transaction.Transactional to org.springframework.transaction.annotation.Transactional and ensure any `@Transactional` annotations on the MemberService class or its methods use the Spring annotation so you can leverage Spring-specific attributes like readOnly, propagation, and isolation.
android/app/src/main/java/com/on/turip/ui/compose/trip/TripDetailScreen.kt
Outdated
Show resolved
Hide resolved
| @Getter | ||
| @AllArgsConstructor | ||
| @Table(name = "account") | ||
| @NoArgsConstructor(access = AccessLevel.PUBLIC) |
There was a problem hiding this comment.
JPA no-args 생성자 접근 레벨을 PROTECTED로 변경하세요
JPA는 protected 이상의 no-args 생성자만 요구합니다. PUBLIC으로 설정하면 new Account()로 id = null인 인스턴스를 쉽게 생성할 수 있어 의도치 않은 NPE를 유발할 수 있습니다. Member와 RefreshToken 엔티티는 모두 AccessLevel.PROTECTED를 사용하여 일관성을 유지하고 있습니다.
🛡️ 제안 수정
-@NoArgsConstructor(access = AccessLevel.PUBLIC)
+@NoArgsConstructor(access = AccessLevel.PROTECTED)📝 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.
| @NoArgsConstructor(access = AccessLevel.PUBLIC) | |
| `@NoArgsConstructor`(access = AccessLevel.PROTECTED) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/src/main/java/turip/account/domain/Account.java` at line 18, Change
the no-args constructor access for the Account entity from public to protected:
replace the annotation `@NoArgsConstructor`(access = AccessLevel.PUBLIC) on the
Account class with AccessLevel.PROTECTED so the JPA-required no-args constructor
is present but prevents accidental public instantiation (matching Member and
RefreshToken); ensure the class-level annotation is updated accordingly.
| public boolean isFirstLogin(Provider provider, String providerId) { | ||
| return !memberRepository.existsByProviderAndProviderId(provider, providerId); | ||
| } | ||
|
|
||
| @Transactional | ||
| public Member findOrCreate(Provider provider, String providerId, String email) { | ||
| return memberRepository.findByProviderAndProviderId(provider, providerId) | ||
| .orElseGet(() -> { | ||
| Account savedAccount = accountService.create(); | ||
| Member member = new Member(savedAccount, provider, providerId, email); | ||
| return memberRepository.save(member); | ||
| }); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "Member.java" | head -5Repository: woowacourse-teams/2025-Turip
Length of output: 195
🏁 Script executed:
rg -n "class Member" --type=java -A 30 | head -100Repository: woowacourse-teams/2025-Turip
Length of output: 10213
🏁 Script executed:
fd -e sql -e yml -e yaml | xargs grep -l -i "member" 2>/dev/null | head -10Repository: woowacourse-teams/2025-Turip
Length of output: 748
🏁 Script executed:
fd -type f \( -name "*.sql" -o -name "*.yml" -o -name "*.yaml" \) | xargs grep -i "provider.*providerId\|unique.*member" 2>/dev/null | head -20Repository: woowacourse-teams/2025-Turip
Length of output: 242
🏁 Script executed:
cat -n backend/src/main/java/turip/account/domain/Member.javaRepository: woowacourse-teams/2025-Turip
Length of output: 2258
🏁 Script executed:
cat -n backend/turip-app/src/main/resources/db/migration/V12__add_unique_constraint.mysql.sqlRepository: woowacourse-teams/2025-Turip
Length of output: 252
🏁 Script executed:
cat -n backend/turip-app/src/main/resources/db/migration/V8__restructure_member.mysql.sqlRepository: woowacourse-teams/2025-Turip
Length of output: 4915
🏁 Script executed:
cat -n backend/turip-app/src/main/resources/db/migration/V9__expand_member.mysql.sqlRepository: woowacourse-teams/2025-Turip
Length of output: 1890
provider+providerId 조합에 DB unique constraint가 존재합니다.
Member 엔티티에서 @UniqueConstraint(name = "uq_member__provider_provider_id", columnNames = {"provider", "provider_id"})로 정의되어 있으며, V8 마이그레이션에서 생성되었습니다.
그러나 isFirstLogin과 findOrCreate 사이의 TOCTOU 경합 조건은 여전히 존재합니다. 동일한 provider/providerId로 동시에 두 건의 로그인 요청이 들어올 경우, 둘 다 isFirstLogin=true를 받은 후 findOrCreate를 호출할 수 있습니다. Unique constraint가 있으므로 중복 생성은 방지되지만, 한쪽 요청에서 데이터베이스 무결성 예외가 발생합니다. 이를 해결하려면 findOrCreate 메서드를 원자적 작업으로 변경하거나(예: INSERT ... ON DUPLICATE KEY UPDATE), 데이터베이스 레벨의 행 잠금을 활용하는 것이 권장됩니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/src/main/java/turip/account/service/MemberService.java` around lines
28 - 40, The TOCTOU between isFirstLogin(...) and findOrCreate(...) can cause
DB-constraint exceptions when two threads race; make findOrCreate(...) atomic by
attempting to create & save the Member and, if the save throws a DB-uniqueness
exception (e.g., DataIntegrityViolationException / PersistenceException), catch
it, re-query memberRepository.findByProviderAndProviderId(provider, providerId)
and return the existing Member; ensure the method remains `@Transactional` (or
mark it to retry appropriately) and handle Account creation so you don't leak
partially created accounts (create account lazily only when save succeeds or
delete it in the catch path). Use the existing symbols:
MemberService.findOrCreate, memberRepository.save,
memberRepository.findByProviderAndProviderId, and accountService.create.
| public LocalDateTime getIssuedAt(String token) { | ||
| Claims claims = parseToken(token); | ||
| return claims.getIssuedAt().toInstant() | ||
| .atZone(ZoneId.systemDefault()) | ||
| .toLocalDateTime(); | ||
| } | ||
|
|
||
| public LocalDateTime getExpiration(String token) { | ||
| Claims claims = parseToken(token); | ||
| return claims.getExpiration().toInstant() | ||
| .atZone(ZoneId.systemDefault()) | ||
| .toLocalDateTime(); | ||
| } |
There was a problem hiding this comment.
ZoneId.systemDefault() 사용으로 인한 시간대 불일치 위험
getIssuedAt/getExpiration 모두 시스템 기본 시간대를 사용하여 LocalDateTime으로 변환합니다. 컨테이너/클라우드 환경에서 JVM 시간대가 달라지면 동일한 토큰에서 추출한 LocalDateTime이 다른 값을 갖게 되고, DB에 저장된 expiredAt과 LocalDateTime.now()의 비교(RefreshToken.isExpired())가 깨질 수 있습니다. ZoneOffset.UTC로 고정하는 것이 안전합니다.
🛡️ 제안 수정
public LocalDateTime getIssuedAt(String token) {
Claims claims = parseToken(token);
return claims.getIssuedAt().toInstant()
- .atZone(ZoneId.systemDefault())
+ .atZone(ZoneOffset.UTC)
.toLocalDateTime();
}
public LocalDateTime getExpiration(String token) {
Claims claims = parseToken(token);
return claims.getExpiration().toInstant()
- .atZone(ZoneId.systemDefault())
+ .atZone(ZoneOffset.UTC)
.toLocalDateTime();
}import java.time.ZoneOffset; 추가도 필요합니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/src/main/java/turip/auth/token/JwtProvider.java` around lines 71 -
83, The getIssuedAt and getExpiration methods currently convert claim timestamps
using ZoneId.systemDefault(), which can vary across environments; change both
conversions to use ZoneOffset.UTC instead (i.e., convert
claims.getIssuedAt().toInstant().atOffset(ZoneOffset.UTC).toLocalDateTime() or
equivalent using ZoneOffset.UTC) so token times are interpreted in UTC; update
imports to include java.time.ZoneOffset and modify getIssuedAt/getExpiration in
JwtProvider to use ZoneOffset.UTC consistently when converting from Instant.
| @BeforeEach | ||
| void setUp() { | ||
| RestAssured.port = port; | ||
|
|
||
| jdbcTemplate.update("DELETE FROM refresh_token"); | ||
| jdbcTemplate.update("DELETE FROM favorite_content"); | ||
| jdbcTemplate.update("DELETE FROM favorite_folder"); | ||
| jdbcTemplate.update("DELETE FROM member"); | ||
| jdbcTemplate.update("DELETE FROM guest"); | ||
| jdbcTemplate.update("DELETE FROM account"); | ||
|
|
||
| jdbcTemplate.update("ALTER TABLE refresh_token ALTER COLUMN id RESTART WITH 1"); | ||
| jdbcTemplate.update("ALTER TABLE member ALTER COLUMN id RESTART WITH 1"); | ||
| jdbcTemplate.update("ALTER TABLE guest ALTER COLUMN id RESTART WITH 1"); | ||
| jdbcTemplate.update("ALTER TABLE account ALTER COLUMN id RESTART WITH 1"); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# favorite_folder_account 테이블 및 FavoriteFolderAccount 엔티티 검색
echo "=== Searching for favorite_folder_account table/entity ==="
rg -i "favorite_folder_account" --type=java -A5 | head -100
echo ""
echo "=== Searching for FavoriteFolderAccount class ==="
fd "FavoriteFolderAccount" --type=java
echo ""
echo "=== Searching for SQL schema files ==="
fd --type f --extension sql | head -20Repository: woowacourse-teams/2025-Turip
Length of output: 12412
🏁 Script executed:
#!/bin/bash
# AccountService.create() 메서드 확인
echo "=== Searching for AccountService class ==="
fd "AccountService" --type=java
echo ""
echo "=== Searching for createDefaultFavoriteFolder or similar ==="
rg "createDefaultFavoriteFolder|default.*folder|기본.*폴더" --type=java -B2 -A5Repository: woowacourse-teams/2025-Turip
Length of output: 50078
🏁 Script executed:
#!/bin/bash
# 게스트 계정 생성 및 회원 생성 로직 확인
echo "=== Searching for guest account creation ==="
rg "guest" --type=java -i -B3 -A3 | head -150
echo ""
echo "=== Searching for member signup/creation ==="
rg "signup|create.*member|신규.*회원" --type=java -i -B2 -A5 | head -150Repository: woowacourse-teams/2025-Turip
Length of output: 32471
favorite_folder_account 테이블 정리 필요
AccountService.create()가 Account 생성 시 favoriteFolderService.createDefaultFavoriteFolder()를 호출하여 기본 폴더와 함께 FavoriteFolderAccount 레코드를 생성합니다. 로그인 시 신규 게스트 계정이 생성되면 이 테이블에 데이터가 추가되므로, setUp의 테이블 정리 순서에 favorite_folder_account 삭제를 포함해야 합니다. 다른 테스트 클래스들(FavoriteFolderApiTest, FavoritePlaceApiTest, MemberApiTest 등)은 모두 이 테이블을 정리하고 있습니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/src/test/java/turip/auth/api/AuthApiTest.java` around lines 39 - 54,
In AuthApiTest.setUp add deletion of the favorite_folder_account table (e.g.,
jdbcTemplate.update("DELETE FROM favorite_folder_account")) to match other tests
and ensure cleanup of records created by AccountService.create() which calls
favoriteFolderService.createDefaultFavoriteFolder(); place this DELETE alongside
the other jdbcTemplate.update(...) table clears before the ALTER TABLE ...
RESTART WITH 1 statements so IDs reset cleanly.
| testImplementation "org.springframework.boot:spring-boot-testcontainers" | ||
| testImplementation "org.testcontainers:junit-jupiter" | ||
| testImplementation "org.testcontainers:testcontainers" | ||
| testImplementation "org.testcontainers:mysql" | ||
| testImplementation "org.testcontainers:testcontainers-junit-jupiter:2.0.2" | ||
| testImplementation "org.testcontainers:testcontainers:2.0.2" | ||
| testImplementation "org.testcontainers:testcontainers-mysql:2.0.2" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd backend/turip-app && pwd && ls -laRepository: woowacourse-teams/2025-Turip
Length of output: 334
🏁 Script executed:
# Check for root build.gradle or settings.gradle to find Spring Boot version
fd -g 'build.gradle' --exclude '**/build/**' --exclude '.gradle/**' -iRepository: woowacourse-teams/2025-Turip
Length of output: 156
🏁 Script executed:
# Look for gradle.properties and other configuration files that might define Spring Boot version
fd -g 'gradle.properties' --exclude '**/build/**' --exclude '.gradle/**'Repository: woowacourse-teams/2025-Turip
Length of output: 97
🏁 Script executed:
# Check the project structure
cat -n backend/turip-app/build.gradle | head -40Repository: woowacourse-teams/2025-Turip
Length of output: 1520
🏁 Script executed:
# Search for Spring Boot version references in all gradle files in the project
rg -n 'springBootVersion|spring.boot.version|org.springframework.boot.*version' --type gradleRepository: woowacourse-teams/2025-Turip
Length of output: 144
🏁 Script executed:
# Check for test files that might use old Testcontainers package paths
find backend -name '*Test.java' -o -name '*Tests.java' | head -20Repository: woowacourse-teams/2025-Turip
Length of output: 1484
🏁 Script executed:
# Search for old Testcontainers container class imports in test files
rg -n 'org\.testcontainers\.containers\.(MySQLContainer|MariaDBContainer|PostgreSQLContainer|GenericContainer)' --type javaRepository: woowacourse-teams/2025-Turip
Length of output: 54
🏁 Script executed:
# Check all current testcontainers references in the project
rg -n 'testcontainers' --type gradleRepository: woowacourse-teams/2025-Turip
Length of output: 499
🏁 Script executed:
# Check a few test files to see if they actually use Testcontainers
head -50 backend/turip-app/src/test/java/turip/account/api/AccountApiTest.javaRepository: woowacourse-teams/2025-Turip
Length of output: 1778
🏁 Script executed:
# Search for any Testcontainers usage in test files (imports or annotations)
rg -n 'import.*testcontainers|@.*Container|TestContainers' --type javaRepository: woowacourse-teams/2025-Turip
Length of output: 719
🏁 Script executed:
# Check if any test extends AbstractTestcontainersTest or uses `@Testcontainers` annotation
rg -n 'class.*Test.*\{|@Testcontainers|@SpringBootTest' --type java -A 3 | head -60Repository: woowacourse-teams/2025-Turip
Length of output: 5565
🌐 Web query:
Testcontainers 2.0 Spring Boot 3.5 compatibility
💡 Result:
Spring Boot 3.5.x is not “Testcontainers 2.0 compatible” out of the box.
- Spring Boot 3.5.x manages Testcontainers 1.x (currently 1.21.x) in its dependency management (e.g., Boot 3.5.9 uses 1.21.4)—so the Boot 3.5 line is built and tested against Testcontainers 1.x, not 2.x. [1]
- Spring Boot 4.x is the line that moves to the Testcontainers 2.x release train, and Testcontainers 2.0 includes breaking changes (module/artifact naming and package moves) that typically require code/dependency updates. [2]
Practical guidance:
- If you’re on Spring Boot 3.5.x, stick with Testcontainers 1.21.x (the version Boot manages) unless you’re ready to do an unsupported override/migration. [1]
- If you need the “Testcontainers 2.x” world, plan to upgrade to Spring Boot 4.x. [2]
- If your motivation is Docker Engine API/version breakage: Spring noted a case where a fix was only expected in Testcontainers 2.x, affecting Boot 3.5.x users; Testcontainers 2.0.2 also set a default Docker API version (1.44). [3][4]
Sources:
[1] Spring Boot 3.5.9 managed dependency coordinates (org.testcontainers = 1.21.4) (docs.spring.io)
[2] Spring Boot 4 / Spring Framework 7 testing notes mentioning “Adopting Testcontainers 2.0” and breaking changes (rieckpil.de)
[3] Spring Boot issue discussing Boot 3.5.x integration impacted and TC fix only in 2.x (github.com)
[4] Testcontainers Java GitHub releases (2.0.2 notes default Docker API version 1.44) (github.com)
Citations:
- 1: https://docs.spring.io/spring-boot/3.5.9/appendix/dependency-versions/coordinates.html?utm_source=openai
- 2: https://rieckpil.de/whats-new-for-testing-in-spring-boot-4-0-and-spring-framework-7/?utm_source=openai
- 3: Testcontainers integration fails on Docker 29.0.0 spring-projects/spring-boot#48104
- 4: https://github.com/testcontainers/testcontainers-java/releases?utm_source=openai
🌐 Web query:
Testcontainers 2.0.3 latest release version 2024 2025
💡 Result:
Which “Testcontainers” do you mean (Java/Maven, .NET/NuGet, Python/PyPI, etc.)?
If you mean Java (Maven Central) org.testcontainers:testcontainers:
- Latest 2.x: 2.0.3 (published Dec 15, 2025) [1]
- Latest 1.x: 1.21.4 (published Dec 16, 2025) [1]
- Latest version released in 2024: 1.20.4 (published Nov 20, 2024) [1]
If you mean .NET (NuGet) Testcontainers:
- Latest overall: 4.10.0 (updated Jan 1, 2026) [2]
- Latest released in 2025: 4.9.0 (updated Nov 23, 2025) [2]
- Latest released in 2024: 4.1.0 (updated Dec 9, 2024) [2]
Sources: [1] [2]
Spring Boot 3.5.3은 Testcontainers 2.0.2와 호환되지 않습니다
springdoc-openapi-starter-webmvc-ui:2.8.9에서 확인되는 대로 이 프로젝트는 Spring Boot 3.5.3을 사용 중입니다. Spring Boot 3.5.x는 의존성 관리를 통해 Testcontainers 1.21.x를 관리하며, Testcontainers 2.x는 Spring Boot 4.x 라인과 함께 사용되도록 설계되었습니다.
현재 명시적으로 선언된 Testcontainers 2.0.2는 spring-boot-testcontainers(line 26)가 기대하는 BOM 관리 Testcontainers 1.21.x와 버전 충돌을 일으킵니다. Spring Boot 3.5.x 통합 계층은 Testcontainers 1.x API에 맞춰 빌드되었으므로, 2.x 명시는 런타임 호환성 문제를 초래할 수 있습니다.
해결 방안:
- Testcontainers 1.21.x를 사용하도록 되돌리거나 (권장)
- Spring Boot 4.x로 업그레이드할 때까지 대기
또한 현재 최신 Testcontainers 패치 버전은 2.0.3입니다(2025년 12월 15일 릴리스). 2.0.2 대신 2.0.3을 사용 중이라면 업데이트를 검토하세요.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/turip-app/build.gradle` around lines 26 - 29, The explicit
Testcontainers 2.0.2 dependencies conflict with Spring Boot 3.5.3's managed
Testcontainers 1.21.x; update the dependency declarations referenced
(org.springframework.boot:spring-boot-testcontainers and the three
org.testcontainers entries: testcontainers-junit-jupiter, testcontainers,
testcontainers-mysql) to align with Spring Boot's BOM—either remove the explicit
:2.0.2 version specifiers so the Spring Boot dependency management picks 1.21.x,
or explicitly set them to a compatible 1.21.x version if you must declare
versions (recommended), or alternatively upgrade the project to Spring Boot 4.x
before keeping 2.x Testcontainers.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Fetch details from PR `#587` to understand the referenced discussion
gh pr view 587 --repo woowacourse-teams/2025-Turip --json title,body,comments --jq '.'
Length of output: 13650
🏁 Script executed:
#!/bin/bash
# Fetch review comments from PR `#587` to find the specific discussion
gh api repos/woowacourse-teams/2025-Turip/pulls/587/comments --jq '.[] | select(.id == 2804862035) | {id, body, user: .user.login, created_at, path, line}'
Length of output: 440
@seaniiio, PR #587의 논의 내용을 확인했습니다.
Gradle 의존성 해결 메커니즘을 통해 명시적으로 선언된 2.0.2가 우선순위를 가지며, 실제로 2.0.2만 사용되는 것을 확인하셨고, API 불일치 문제는 CI 테스트 과정에서 감지할 수 있다는 점을 이미 검증하셨네요.
Spring Boot 3.5.3이 공식적으로 Testcontainers 1.21.x를 관리하지만, 팀에서 명시적으로 2.0.2로 업그레이드하고 의존성 해결 및 테스트를 통해 검증하신 만큼, 현재 설정을 유지하셔도 될 것 같습니다. 다만 향후 Spring Boot나 Testcontainers 업데이트 시 호환성 이슈가 발생할 수 있다는 점만 인지하고 계시면 좋을 것 같습니다.
✏️ Learnings added
Learnt from: seaniiio
Repo: woowacourse-teams/2025-Turip PR: 598
File: backend/turip-app/build.gradle:26-29
Timestamp: 2026-02-20T10:13:59.681Z
Learning: The woowacourse-teams/2025-Turip project intentionally uses Testcontainers 2.0.2 with Spring Boot 3.5.3 (which officially manages Testcontainers 1.21.x). The team has verified through Gradle dependency resolution that 2.0.2 takes priority and relies on CI tests to detect any API incompatibility issues.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
.../src/main/java/turip/favorite/stream/controller/dto/response/MemberUpdateStreamResponse.java
Show resolved
Hide resolved
| public MySQLContainer<?> mySQLContainer() { | ||
| return new MySQLContainer<>("mysql:8.0.42") | ||
| public MySQLContainer mySQLContainer() { | ||
| return new MySQLContainer("mysql:8.0.42") |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if mysql:8.0.42 tag exists on Docker Hub
# Using Docker Hub v2 API
echo "Checking mysql:8.0.42 tag availability..."
curl -s "https://hub.docker.com/v2/repositories/library/mysql/tags/?name=8.0.42" | python3 << 'EOF'
import sys
import json
try:
data = json.load(sys.stdin)
results = data.get('results', [])
if results:
tag = results[0]
print(f"Tag Found: YES")
print(f"Tag Name: {tag.get('name')}")
print(f"Status: Available for pull")
else:
print(f"Tag Found: NO - 8.0.42 does not exist on Docker Hub")
except Exception as e:
print(f"Error checking tag: {e}")
sys.exit(1)
EOFRepository: woowacourse-teams/2025-Turip
Length of output: 175
🏁 Script executed:
#!/bin/bash
# Try alternative approach to check mysql:8.0.42 tag
# Using Docker Hub manifest API
echo "Attempting to fetch mysql:8.0.42 manifest..."
# Try the v2 manifest endpoint which doesn't require authentication for public images
response=$(curl -s -w "\n%{http_code}" \
-H "Accept: application/vnd.docker.distribution.manifest.v2+json" \
"https://registry-1.docker.io/v2/library/mysql/manifests/8.0.42")
http_code=$(echo "$response" | tail -n1)
body=$(echo "$response" | head -n-1)
echo "HTTP Response Code: $http_code"
if [ "$http_code" = "200" ]; then
echo "Tag Status: FOUND - mysql:8.0.42 exists and is pullable"
echo "Manifest retrieved successfully"
else
echo "Tag Status: NOT FOUND or ERROR"
echo "Response body: $body"
fi
# Also list some available 8.0.x tags for comparison
echo ""
echo "Checking available MySQL 8.0.x tags (sample)..."
curl -s "https://hub.docker.com/v2/repositories/library/mysql/tags/?page_size=100" 2>/dev/null | python3 << 'EOF'
import sys, json
try:
data = json.load(sys.stdin)
results = data.get('results', [])
tags_8_0 = [r['name'] for r in results if r['name'].startswith('8.0.')]
tags_8_0.sort()
print(f"Available MySQL 8.0.x tags (sample): {tags_8_0[:10]}")
if '8.0.42' in tags_8_0:
print("✓ 8.0.42 is in the available tags list")
else:
print("✗ 8.0.42 is NOT in the available tags list")
except:
print("Could not parse tag list")
EOFRepository: woowacourse-teams/2025-Turip
Length of output: 416
🏁 Script executed:
#!/bin/bash
# Check if there's any Maven or Gradle config that specifies MySQL versions
fd -t f "pom.xml|build.gradle|docker-compose" backend/
echo ""
echo "Checking TestContainers configuration and dependencies..."
# Look for Testcontainers MySQL configuration in the test file
cat -n backend/turip-app/src/test/java/turip/container/TestContainerConfig.java 2>/dev/null | head -30
echo ""
echo "Searching for other MySQL version references in the codebase..."
rg "mysql:" backend/ -i --max-count=10Repository: woowacourse-teams/2025-Turip
Length of output: 1882
🌐 Web query:
MySQL 8.0.42 Docker image tag Docker Hub official
💡 Result:
On Docker Hub, the Docker Official Image for MySQL is mysql (namespace library), so the versioned tag format is:
mysql:8.0.42
However, as of the current Docker Hub “mysql” official-image documentation, the supported/published 8.0 line is 8.0.45 (tags like mysql:8.0.45 and mysql:8.0), and 8.0.42 is not listed there. [1]
If you need to verify whether mysql:8.0.42 still exists in the registry, you can query Docker Hub’s tags API (pattern shown here): [2]
https://registry.hub.docker.com/v2/repositories/library/mysql/tags/?name=8.0.42
Sources:
[1] Docker Hub – mysql (Docker Official Image), “Supported tags …” https://hub.docker.com/_/mysql/
[2] “Looking up docker image tags” (explains Docker Hub tags API format) https://0ink.net/posts/2022/2022-12-01-docker-image-tags.html
🌐 Web query:
MySQL 8.0.42 release version history official
💡 Result:
Official MySQL Server 8.0.42 version history is published in the MySQL 8.0 Release Notes (online documentation), which includes a table of contents listing each 8.0 GA release and links to their “Changes in …” pages (including “Changes in MySQL 8.0.42 (2025-04-15, General Availability)”). [1]
The specific official release-notes page for MySQL 8.0.42 is “Changes in MySQL 8.0.42 (2025-04-15, General Availability)”, which contains the detailed change log (Audit Log notes, Compilation notes, InnoDB notes, functionality changes, bug fixes, etc.). [2]
Sources: MySQL 8.0 Release Notes (index/TOC) [1]; MySQL 8.0.42 Release Notes page [2].
🌐 Web query:
mysql:8.0.42 Docker image available registry
💡 Result:
mysql:8.0.42 (if it exists in your registry) would be pulled from the Docker Hub Official Image repo:
- Registry/repo:
docker.io/library/mysql(akamysqlon Docker Hub) [1]
However, Docker Hub’s currently documented/supported MySQL 8.0.x tag is 8.0.45 / 8.0 (not 8.0.42) [1].
For the Oracle/MySQL-team supported container images, use Oracle Container Registry (OCR):
- Registry/repo:
container-registry.oracle.com/mysql/community-server[2][3] - Pull pattern:
docker pull container-registry.oracle.com/mysql/community-server:<tag>[3]
Sources: Docker Hub MySQL Official Image page [1], Oracle MySQL blog on official images/OCR [2], Oracle docs showing OCR pull command for MySQL Community Server images [3].
mysql:8.0.42 이미지는 Docker Hub의 공식 MySQL 저장소에서 지원하지 않는 태그입니다
Docker Hub 공식 문서에 따르면 현재 지원되는 8.0.x 태그는 8.0.45이며, 8.0.42는 지원 목록에 없습니다. MySQL 8.0.42는 공식 릴리스로 존재하지만, 해당 Docker 이미지가 Docker Hub에 발행되지 않아 테스트 컨테이너 실행 시 이미지 pull 오류가 발생할 수 있습니다. mysql:8.0.45 또는 Oracle Container Registry의 이미지 사용을 권장합니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/turip-app/src/test/java/turip/container/TestContainerConfig.java` at
line 14, The TestContainerConfig is instantiating MySQLContainer with an
unsupported image tag ("mysql:8.0.42"); update the image tag to a supported one
(e.g., "mysql:8.0.45") in the MySQLContainer constructor used in
TestContainerConfig (the return new MySQLContainer(...) expression), or make the
image tag configurable via an environment/property so tests can switch to
"mysql:8.0.45" or an alternative registry image if needed.
# Conflicts: # backend/turip-app/src/main/java/turip/common/exception/ErrorTag.java # backend/turip-app/src/main/java/turip/favorite/controller/FavoriteFolderController.java # backend/turip-app/src/main/java/turip/favorite/service/FavoriteFolderAccountService.java # backend/turip-app/src/main/java/turip/favorite/service/FavoriteFolderService.java # backend/turip-app/src/test/java/turip/favorite/api/FavoriteFolderApiTest.java # backend/turip-app/src/test/java/turip/favorite/service/FavoriteFolderServiceTest.java
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
backend/turip-app/src/test/java/turip/favorite/service/FavoriteFolderServiceTest.java (1)
726-754:exitFolder2(마지막 참여자) 테스트에서deleteByFavoriteFolderAndAccount호출 순서 검증 누락서비스 코드에서
deleteByFavoriteFolderAndAccount→countByFavoriteFolder→removeFavoriteFolderWithFavoritePlaces순서가 중요합니다. 현재 테스트는 각 메서드 호출 여부만 확인하지만, 호출 순서를InOrder로 검증하면 로직 변경 시 더 안전합니다. 다만, 현재 구현이 정확하므로 이는 선택적 개선입니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/turip-app/src/test/java/turip/favorite/service/FavoriteFolderServiceTest.java` around lines 726 - 754, Add an InOrder verification to the exitFolder2 test to assert the expected call sequence when the last participant exits: after invoking favoriteFolderService.exitFolder in the test, verify with an org.mockito.InOrder that favoriteFolderAccountService.deleteByFavoriteFolderAndAccount(favoriteFolder, account) is called first, then favoriteFolderAccountService.countByFavoriteFolder(favoriteFolder), then favoritePlaceRepository.deleteAllByFavoriteFolder(favoriteFolder) and favoriteFolderRepository.deleteById(folderId), and finally eventPublisher.publishEvent(FavoriteFolderUpdateEvent.of(folderId, ActionType.FOLDER_DELETED)); this ensures the test validates the correct invocation order for those methods.backend/turip-app/src/main/java/turip/favorite/stream/service/FavoriteFolderStreamService.java (1)
60-61: 로그 메시지에서 문자열 연결 대신 파라미터 치환 사용 권장
SSE_LOG_PREFIX + "..."패턴은 로그 레벨과 무관하게 항상 문자열 연결이 발생합니다. SLF4J 파라미터 방식을 사용하면 해당 로그 레벨이 비활성화된 경우 연결 비용을 절약할 수 있습니다.예시
- log.info(SSE_LOG_PREFIX + "폴더 업데이트 이벤트 전송 시작, folderId: {}, 연결된 사용자 수: {}", favoriteFolderId, - folderEmitters.size()); + log.info("[SSE] 폴더 업데이트 이벤트 전송 시작, folderId: {}, 연결된 사용자 수: {}", favoriteFolderId, + folderEmitters.size());또는 prefix를 포함하는 로그 포맷을 별도로 정의하세요.
Also applies to: 74-74
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/turip-app/src/main/java/turip/favorite/stream/service/FavoriteFolderStreamService.java` around lines 60 - 61, The log statements in FavoriteFolderStreamService use string concatenation with SSE_LOG_PREFIX (e.g., log.info(SSE_LOG_PREFIX + "폴더 업데이트 이벤트 전송 시작, folderId: {}, 연결된 사용자 수: {}", ...)), which forces string construction even when the log level is disabled; change these to use SLF4J parameter substitution including the prefix as a parameter (or include the prefix in the logging format) so the message becomes log.info("{} 폴더 업데이트 이벤트 전송 시작, folderId: {}, 연결된 사용자 수: {}", SSE_LOG_PREFIX, favoriteFolderId, folderEmitters.size()) and make the same change for the other occurrence around the folder completion/send log (the other log at line ~74) in FavoriteFolderStreamService.backend/turip-app/src/main/java/turip/favorite/service/FavoriteFolderService.java (3)
147-161:findById와findMembersById의 폴더 조회 방식 불일치
findById는getById를 통해 폴더 엔티티를 조회하고validateMembership(account, favoriteFolder)를 호출합니다. 반면findMembersById는validateFolderExists+validateMembership(account, favoriteFolderId)(Long 오버로드)를 사용합니다. 두 방식 모두 동작하지만,findMembersById에서는 폴더 존재 확인과 멤버십 검증이 각각 별도 쿼리를 실행합니다.getById를 사용하면 단일 조회로 존재 확인과 엔티티 획득을 동시에 처리할 수 있습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/turip-app/src/main/java/turip/favorite/service/FavoriteFolderService.java` around lines 147 - 161, The two methods use different folder lookup patterns causing extra queries; in findMembersById replace validateFolderExists(favoriteFolderId) and favoriteFolderAccountService.validateMembership(account, favoriteFolderId) with a single entity load FavoriteFolder favoriteFolder = getById(favoriteFolderId) followed by favoriteFolderAccountService.validateMembership(account, favoriteFolder) so you reuse getById (like in findById) to both verify existence and provide the entity; then call favoriteFolderAccountService.findMembersByFavoriteFolder(favoriteFolder.getId()) and return FavoriteFolderMembersResponse.of(...) as before.
108-119:findAllByAccount에서 N+1 쿼리 발생각 폴더마다
countByFavoriteFolder와countByFavoriteFolder(member count)를 개별 쿼리로 실행합니다. 폴더 수가 많아지면 성능에 영향을 줄 수 있습니다. 현재 규모에서는 문제없을 수 있지만, 향후 단일 쿼리로 집계하는 방식을 고려해볼 수 있습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/turip-app/src/main/java/turip/favorite/service/FavoriteFolderService.java` around lines 108 - 119, findAllByAccount currently triggers N+1 queries by calling favoritePlaceRepository.countByFavoriteFolder and favoriteFolderAccountService.countByFavoriteFolder inside the stream for each FavoriteFolder; replace these per-folder counts with bulk aggregation queries: add/use repository methods that accept a list of favoriteFolder ids (e.g., countByFavoriteFolderIn or a custom query returning (favoriteFolderId, count) pairs) for both places and members, call favoriteFolderRepository.findAllByAccountOrderByFavoriteFolderAccountIdAsc to get the folders, collect their ids, run the two bulk count queries once, build maps from folderId->count, and then in the stream pass the mapped counts into FavoriteFolderDetailResponse.of(favoriteFolder, account, placeCount, memberCount) before returning FavoriteFoldersDetailResponse.from(...)
238-245:validateShareAndCustomFolder네이밍이 실제 검증 로직과 불일치이 메서드는 "공유 폴더이면서 기본 폴더가 아닌" 경우만 통과시킵니다. 하지만 메서드 이름
validateShareAndCustomFolder는 "공유이면서 커스텀"을 검증한다는 의미인데, 실제로는!isShared()이면 예외를 던지고isDefault()이면 예외를 던집니다. 이름에서 "Custom"이라는 단어가 혼란을 줄 수 있습니다 — 실제 검증은 "shared이면서 non-default"입니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/turip-app/src/main/java/turip/favorite/service/FavoriteFolderService.java` around lines 238 - 245, The method FavoriteFolderService.validateShareAndCustomFolder currently enforces "shared AND non-default" but the name ("Custom") is misleading; rename the method to something clearer like validateSharedNonDefaultFolder (and update all call sites) or, alternatively, change the logic to match the original name by checking isShared() && !isDefault(); also update any Javadoc/comments and keep the same exception behavior (throw BadRequestException with ErrorTag.PERSONAL_FAVORITE_FOLDER_OPERATION_NOT_ALLOWED when !isShared() and ErrorTag.DEFAULT_FAVORITE_FOLDER_OPERATION_NOT_ALLOWED when isDefault()) so callers and logs reflect the corrected intent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@backend/turip-app/src/main/java/turip/favorite/service/FavoriteFolderAccountService.java`:
- Around line 22-32: In FavoriteFolderAccountService, handle concurrent-create
races in findOrCreate by catching DB constraint exceptions when calling save;
specifically, wrap the call to save(favoriteFolder, account, AccountRole.MEMBER)
(and any direct favoriteFolderAccountRepository.save usage) in a try/catch for
DataIntegrityViolationException (or the repository's
IntegrityConstraintViolationException) and on catch re-query
favoriteFolderAccountRepository.findByFavoriteFolderAndAccount(favoriteFolder,
account) and return the existing entity (or rethrow if still absent);
alternatively document switching to a pessimistic lock via
favoriteFolderAccountRepository with a PESSIMISTIC_WRITE lock around the
favoriteFolder lookup to prevent simultaneous saves.
In
`@backend/turip-app/src/test/java/turip/favorite/api/FavoriteFolderApiTest.java`:
- Around line 525-545: The test readMembersById3 currently sends a request with
header "device-fid" = "nonMemberDeviceFid" which does not map to the previously
created nonOwnerAccountId/nonOwnerMemberId, so the request authenticates as a
newly auto-created guest instead of the intended non-owner account; fix by
either registering a guest/device record tied to nonOwnerAccountId before the
request (using your test helper that creates guest/device entries) or change the
request header to use the existing device-fid associated with nonOwnerMemberId
so the request is truly from the non-owner account; update the test to reference
nonOwnerAccountId/nonOwnerMemberId and the device-fid consistently (in method
readMembersById3) so the assertion of 403 reflects the intended actor.
---
Nitpick comments:
In
`@backend/turip-app/src/main/java/turip/favorite/service/FavoriteFolderService.java`:
- Around line 147-161: The two methods use different folder lookup patterns
causing extra queries; in findMembersById replace
validateFolderExists(favoriteFolderId) and
favoriteFolderAccountService.validateMembership(account, favoriteFolderId) with
a single entity load FavoriteFolder favoriteFolder = getById(favoriteFolderId)
followed by favoriteFolderAccountService.validateMembership(account,
favoriteFolder) so you reuse getById (like in findById) to both verify existence
and provide the entity; then call
favoriteFolderAccountService.findMembersByFavoriteFolder(favoriteFolder.getId())
and return FavoriteFolderMembersResponse.of(...) as before.
- Around line 108-119: findAllByAccount currently triggers N+1 queries by
calling favoritePlaceRepository.countByFavoriteFolder and
favoriteFolderAccountService.countByFavoriteFolder inside the stream for each
FavoriteFolder; replace these per-folder counts with bulk aggregation queries:
add/use repository methods that accept a list of favoriteFolder ids (e.g.,
countByFavoriteFolderIn or a custom query returning (favoriteFolderId, count)
pairs) for both places and members, call
favoriteFolderRepository.findAllByAccountOrderByFavoriteFolderAccountIdAsc to
get the folders, collect their ids, run the two bulk count queries once, build
maps from folderId->count, and then in the stream pass the mapped counts into
FavoriteFolderDetailResponse.of(favoriteFolder, account, placeCount,
memberCount) before returning FavoriteFoldersDetailResponse.from(...)
- Around line 238-245: The method
FavoriteFolderService.validateShareAndCustomFolder currently enforces "shared
AND non-default" but the name ("Custom") is misleading; rename the method to
something clearer like validateSharedNonDefaultFolder (and update all call
sites) or, alternatively, change the logic to match the original name by
checking isShared() && !isDefault(); also update any Javadoc/comments and keep
the same exception behavior (throw BadRequestException with
ErrorTag.PERSONAL_FAVORITE_FOLDER_OPERATION_NOT_ALLOWED when !isShared() and
ErrorTag.DEFAULT_FAVORITE_FOLDER_OPERATION_NOT_ALLOWED when isDefault()) so
callers and logs reflect the corrected intent.
In
`@backend/turip-app/src/main/java/turip/favorite/stream/service/FavoriteFolderStreamService.java`:
- Around line 60-61: The log statements in FavoriteFolderStreamService use
string concatenation with SSE_LOG_PREFIX (e.g., log.info(SSE_LOG_PREFIX + "폴더
업데이트 이벤트 전송 시작, folderId: {}, 연결된 사용자 수: {}", ...)), which forces string
construction even when the log level is disabled; change these to use SLF4J
parameter substitution including the prefix as a parameter (or include the
prefix in the logging format) so the message becomes log.info("{} 폴더 업데이트 이벤트 전송
시작, folderId: {}, 연결된 사용자 수: {}", SSE_LOG_PREFIX, favoriteFolderId,
folderEmitters.size()) and make the same change for the other occurrence around
the folder completion/send log (the other log at line ~74) in
FavoriteFolderStreamService.
In
`@backend/turip-app/src/test/java/turip/favorite/service/FavoriteFolderServiceTest.java`:
- Around line 726-754: Add an InOrder verification to the exitFolder2 test to
assert the expected call sequence when the last participant exits: after
invoking favoriteFolderService.exitFolder in the test, verify with an
org.mockito.InOrder that
favoriteFolderAccountService.deleteByFavoriteFolderAndAccount(favoriteFolder,
account) is called first, then
favoriteFolderAccountService.countByFavoriteFolder(favoriteFolder), then
favoritePlaceRepository.deleteAllByFavoriteFolder(favoriteFolder) and
favoriteFolderRepository.deleteById(folderId), and finally
eventPublisher.publishEvent(FavoriteFolderUpdateEvent.of(folderId,
ActionType.FOLDER_DELETED)); this ensures the test validates the correct
invocation order for those methods.
backend/turip-app/src/main/java/turip/favorite/service/FavoriteFolderAccountService.java
Show resolved
Hide resolved
backend/turip-app/src/test/java/turip/favorite/api/FavoriteFolderApiTest.java
Show resolved
Hide resolved
There was a problem hiding this comment.
🧹 Nitpick comments (1)
backend/turip-app/src/test/java/turip/favorite/stream/service/FavoriteFolderStreamServiceTest.java (1)
95-119:sendFolderUpdateEvents예외 경로 테스트 보완 고려현재 테스트는 정상 경로(emitter 2개에 이벤트 전송 성공)만 검증합니다. 다음 두 시나리오에 대한 테스트가 없습니다.
emitter.send()가IOException을 던질 때의 동작 (emitter 제거 등 후처리 여부)folderId에 등록된 emitter가 없을 때 NPE/빈 맵 처리 여부🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/turip-app/src/test/java/turip/favorite/stream/service/FavoriteFolderStreamServiceTest.java` around lines 95 - 119, Add tests covering the two missing exception paths for sendFolderUpdateEvents: (1) create a test in FavoriteFolderStreamServiceTest where one mocked SseEmitter throws an IOException on send (mock emitter.send(...) to throw IOException) and assert that sendFolderUpdateEvents still calls send on other emitters and that the failing emitter is removed from the internal "emitters" map (use ReflectionTestUtils to read the "emitters" field and verify removal); (2) create a test where the provided folderId has no entry in the "emitters" map (do not put a map for that folderId), call sendFolderUpdateEvents and assert it does not throw (no NPE) and that the "emitters" map remains unchanged; reference sendFolderUpdateEvents, FavoriteFolderStreamServiceTest, the "emitters" field, SseEmitter.send and IOException when implementing these tests.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@backend/turip-app/src/test/java/turip/favorite/stream/service/FavoriteFolderStreamServiceTest.java`:
- Around line 95-119: Add tests covering the two missing exception paths for
sendFolderUpdateEvents: (1) create a test in FavoriteFolderStreamServiceTest
where one mocked SseEmitter throws an IOException on send (mock
emitter.send(...) to throw IOException) and assert that sendFolderUpdateEvents
still calls send on other emitters and that the failing emitter is removed from
the internal "emitters" map (use ReflectionTestUtils to read the "emitters"
field and verify removal); (2) create a test where the provided folderId has no
entry in the "emitters" map (do not put a map for that folderId), call
sendFolderUpdateEvents and assert it does not throw (no NPE) and that the
"emitters" map remains unchanged; reference sendFolderUpdateEvents,
FavoriteFolderStreamServiceTest, the "emitters" field, SseEmitter.send and
IOException when implementing these tests.
|
@coderabbitai sse 이벤트 기능 관련 리뷰를 남겨주세요 |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 11797
✅ 잘 구현된 점
🔍 개선이 필요한 사항1. AsyncConfiguration - RejectedExecutionHandler 변경 권장Line 18: executor.setRejectedExecutionHandler(new ThreadPoolExecutor.CallerRunsPolicy());이유: 큐가 가득 차면 메인 스레드(이벤트를 발행한 스레드)에서 실행되어 응답 지연 발생 권장사항: // 옵션 1: 이벤트를 버리고 로그 남기기 (권장)
executor.setRejectedExecutionHandler((r, executor) -> {
log.warn("SSE 이벤트 전송 거부됨 - Thread pool 포화 상태");
});
// 옵션 2: 큐 크기를 늘리고 AbortPolicy 사용
executor.setQueueCapacity(500);
executor.setRejectedExecutionHandler(new ThreadPoolExecutor.AbortPolicy());2. FavoriteFolderStreamService - 에러 발생 시 Emitter 정리Lines 152-163, 165-176: 이벤트 전송 실패 시 private void sendFolderUpdateEvent(...) {
try {
// ...
emitter.send(event);
} catch (Exception e) {
log.error(...);
emitter.completeWithError(e);
// ❌ removeEmitter 호출 없음
}
}문제: 권장사항: private void sendFolderUpdateEvent(Long favoriteFolderId, FolderUpdateStreamResponse response, SseEmitter emitter) {
try {
SseEventBuilder event = SseEmitter.event()
.id(String.valueOf(System.currentTimeMillis()))
.name(StreamEventType.FOLDER_UPDATE.getName())
.data(response);
emitter.send(event);
} catch (Exception e) {
log.error(SSE_LOG_PREFIX + "폴더 업데이트 이벤트 전송 실패, folderId: {}", favoriteFolderId, e);
try {
emitter.completeWithError(e);
} catch (Exception completeException) {
log.error(SSE_LOG_PREFIX + "completeWithError 실패, folderId: {}", favoriteFolderId, completeException);
}
// ✅ emitter 수동 정리 필요 (emitterKey를 파라미터로 받아야 함)
}
}또는 메서드 시그니처 변경: private void sendFolderUpdateEvent(Long favoriteFolderId, String emitterKey,
FolderUpdateStreamResponse response, SseEmitter emitter) {
// ... 에러 처리에서 removeEmitter(favoriteFolderId, emitterKey) 호출
}3. Heartbeat 실패 시 정리 로직 확인Lines 201-211: private void sendHeartbeatEvent(SseEmitter emitter) {
try {
// ...
emitter.send(event);
} catch (Exception e) {
log.warn(SSE_LOG_PREFIX + "하트비트 전송 실패", e);
emitter.completeWithError(e); // ✅ onError 콜백 트리거 확인됨
}
}현재 구현은 정상적으로 동작하지만, 명확성을 위해 주석 추가를 권장합니다. 4. Timeout 설정 검토Line 35: private static final Long DEFAULT_TIMEOUT = 3 * 60 * 1000L; // 3분고려사항:
5. FOLDER_DELETED 이벤트 후 Emitter 정리폴더 삭제 시 해당 폴더의 모든 emitter를 명시적으로 정리하는 로직이 있으면 좋습니다. // FavoriteFolderStreamService에 추가 고려
public void closeAllEmittersForFolder(Long favoriteFolderId) {
Map<String, SseEmitter> folderEmitters = emitters.remove(favoriteFolderId);
if (folderEmitters != null) {
folderEmitters.values().forEach(emitter -> {
try {
emitter.complete();
} catch (Exception e) {
log.warn("Emitter close 실패", e);
}
});
}
}📝 기타 권장사항
전반적으로 SSE 기능이 견고하게 구현되었습니다! 위 개선사항들을 반영하시면 더욱 안정적인 서비스가 될 것 같습니다. 추가로 궁금하신 점이 있으시면 말씀해 주세요! 🚀 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
backend/turip-app/src/main/java/turip/favorite/repository/FavoriteFolderAccountRepository.java (1)
21-25:DISTINCT추가 및ON절의 엔티티 비교로 개선 권장
DISTINCT누락:JOIN FETCH와 추가 JOIN을 함께 사용할 때, 데이터베이스에(favoriteFolder, account)중복 행이 존재할 경우Member가 중복 반환될 수 있습니다.DISTINCT를 추가하면 이러한 상황에 대한 방어적 처리가 가능합니다.ON 절 비교 방식 개선:
ffa.account.id = a.id로 ID를 비교하면 Hibernate가Account테이블을 암묵적으로 추가 조인할 수 있습니다. 엔티티를 직접 비교하는 방식(ffa.account = a)이 더 명확하고 효율적입니다.♻️ 개선 제안
- `@Query`("SELECT m FROM Member m " + - "JOIN FETCH m.account a " + - "JOIN FavoriteFolderAccount ffa ON ffa.account.id = a.id " + - "WHERE ffa.favoriteFolder.id = :favoriteFolderId") + `@Query`("SELECT DISTINCT m FROM Member m " + + "JOIN FETCH m.account a " + + "JOIN FavoriteFolderAccount ffa ON ffa.account = a " + + "WHERE ffa.favoriteFolder.id = :favoriteFolderId")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/turip-app/src/main/java/turip/favorite/repository/FavoriteFolderAccountRepository.java` around lines 21 - 25, Update the JPQL in FavoriteFolderAccountRepository.findMembersByFavoriteFolderId to add DISTINCT to the select and change the ON clause entity comparison from "ffa.account.id = a.id" to "ffa.account = a"; specifically, make the query "SELECT DISTINCT m FROM Member m JOIN FETCH m.account a JOIN FavoriteFolderAccount ffa ON ffa.account = a WHERE ffa.favoriteFolder.id = :favoriteFolderId" so duplicate Member rows are eliminated and Hibernate uses entity comparison rather than an implicit ID-based join.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@backend/turip-app/src/main/java/turip/favorite/repository/FavoriteFolderAccountRepository.java`:
- Around line 21-25: Update the JPQL in
FavoriteFolderAccountRepository.findMembersByFavoriteFolderId to add DISTINCT to
the select and change the ON clause entity comparison from "ffa.account.id =
a.id" to "ffa.account = a"; specifically, make the query "SELECT DISTINCT m FROM
Member m JOIN FETCH m.account a JOIN FavoriteFolderAccount ffa ON ffa.account =
a WHERE ffa.favoriteFolder.id = :favoriteFolderId" so duplicate Member rows are
eliminated and Hibernate uses entity comparison rather than an implicit ID-based
join.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@backend/turip-app/src/main/java/turip/common/configuration/AsyncConfiguration.java`:
- Around line 23-28: The log message in the rejected handler for executor (the
lambda passed to executor.setRejectedExecutionHandler) incorrectly labels and
emits getQueue().size() as "잔여 큐" (remaining capacity); replace or augment this
to report the correct value using
executorInstance.getQueue().remainingCapacity() (or log both size() and
remainingCapacity() with correct labels) so the message reflects actual queue
occupancy vs remaining space when rejection occurs.
- Line 33: Remove the manual call to initialize() on the ThreadPoolTaskExecutor
to avoid double initialization; in AsyncConfiguration locate the bean creation
where you call executor.initialize() (the ThreadPoolTaskExecutor instance
returned from the method) and delete that explicit initialize() invocation so
Spring can perform afterPropertiesSet()/initialize() automatically via the
lifecycle.
backend/turip-app/src/main/java/turip/common/configuration/AsyncConfiguration.java
Show resolved
Hide resolved
backend/turip-app/src/main/java/turip/common/configuration/AsyncConfiguration.java
Outdated
Show resolved
Hide resolved
|
@coderabbitai sse 기능에 대한 Sequence Diagrams을 그려주세요 |
|
Tip For best results, initiate chat on the files or code changes.
1. SSE 연결 수립 (Connection Establishment)sequenceDiagram
actor Client
participant Controller as FavoriteFolderController
participant StreamService as FavoriteFolderStreamService
participant FolderService as FavoriteFolderService
participant Emitter as SseEmitter
participant Scheduler
Client->>Controller: GET /api/v1/turips/{turipId}/subscribe
Controller->>StreamService: subscribeToFavoriteFolder(turipId, member)
StreamService->>FolderService: validateFolderMembership(turipId, member)
FolderService-->>StreamService: validation success
StreamService->>Emitter: new SseEmitter(timeout=3min)
Emitter-->>StreamService: emitter instance
StreamService->>Emitter: onCompletion(() -> removeEmitter)
StreamService->>Emitter: onTimeout(() -> removeEmitter)
StreamService->>Emitter: onError(() -> removeEmitter)
StreamService->>StreamService: emitters.put(turipId, emitterKey, emitter)
StreamService->>Scheduler: scheduleHeartbeat(emitter)
activate Scheduler
StreamService-->>Controller: emitter
Controller-->>Client: SSE Connection Established (200 OK)
loop Every 30 seconds
Scheduler->>Emitter: send(heartbeat event)
Emitter-->>Client: ❤️ heartbeat
end
deactivate Scheduler
2. 폴더 업데이트 이벤트 전파 (Folder Update Event Flow)sequenceDiagram
actor Client
participant Controller as FavoriteFolderController
participant FolderService as FavoriteFolderService
participant EventPublisher as ApplicationEventPublisher
participant DB as Database
participant EventListener as FavoriteFolderEventListener
participant ThreadPool as SSE-EVT Thread Pool
participant StreamService as FavoriteFolderStreamService
participant Emitters as Connected Emitters
Client->>Controller: PUT /api/v1/turips/{id}/name
Controller->>FolderService: updateName(account, id, request)
activate FolderService
FolderService->>DB: update folder name
DB-->>FolderService: updated
FolderService->>EventPublisher: publishEvent(FavoriteFolderUpdateEvent)
Note over EventPublisher: Event queued until commit
FolderService-->>Controller: response
deactivate FolderService
Controller-->>Client: 200 OK
Note over DB,EventPublisher: Transaction Commit
EventPublisher->>EventListener: handleEvent(FavoriteFolderUpdateEvent)
Note over EventListener: `@TransactionalEventListener`<br/>(AFTER_COMMIT)
EventListener->>ThreadPool: `@Async`("sseEventExecutor")
activate ThreadPool
ThreadPool->>StreamService: sendFolderUpdateEvents(folderId, actionType)
StreamService->>StreamService: getFolderEmitters(folderId)
loop For each connected emitter
StreamService->>Emitters: send(FolderUpdateStreamResponse)
Emitters-->>Client: SSE Event: FOLDER_UPDATE
end
deactivate ThreadPool
3. 멤버 업데이트 이벤트 전파 (Member Update Event Flow)sequenceDiagram
actor NewMember as New Member
participant Controller as FavoriteFolderController
participant FolderService as FavoriteFolderService
participant EventPublisher as ApplicationEventPublisher
participant DB as Database
participant EventListener as FavoriteFolderEventListener
participant ThreadPool as SSE-EVT Thread Pool
participant StreamService as FavoriteFolderStreamService
participant FolderAccountService as FavoriteFolderAccountService
participant MemberRepo as MemberRepository
participant Emitters as Connected Members
NewMember->>Controller: POST /api/v1/turips/{id}/join
Controller->>FolderService: joinMember(id, member)
activate FolderService
FolderService->>DB: validate folder exists & is shared
FolderService->>FolderAccountService: findOrCreate(folder, account)
FolderAccountService->>DB: insert or fetch association
FolderAccountService-->>FolderService: folderAccount
FolderService->>EventPublisher: publishEvent(MEMBER_JOINED)
FolderService-->>Controller: FavoriteFolderJoinResponse
deactivate FolderService
Controller-->>NewMember: 200 OK
Note over DB,EventPublisher: Transaction Commit
EventPublisher->>EventListener: handleEvent(actionType=MEMBER_JOINED)
EventListener->>ThreadPool: `@Async`("sseEventExecutor")
activate ThreadPool
ThreadPool->>StreamService: sendMemberUpdateEvents(folderId, MEMBER_JOINED)
StreamService->>FolderAccountService: findMembersByFavoriteFolder(folderId)
FolderAccountService->>MemberRepo: findMembersByFavoriteFolderId(folderId)
MemberRepo-->>FolderAccountService: members list
FolderAccountService-->>StreamService: members list
StreamService->>StreamService: build MemberUpdateStreamResponse<br/>(memberCount, nicknames, timestamp)
loop For each connected member
StreamService->>Emitters: send(MemberUpdateStreamResponse)
Emitters-->>NewMember: SSE Event: MEMBER_UPDATE
Emitters-->>Emitters: 📢 "{nickname} joined"
end
deactivate ThreadPool
4. Emitter 정리 흐름 (Emitter Cleanup Flow)sequenceDiagram
participant Client
participant Emitter as SseEmitter
participant StreamService as FavoriteFolderStreamService
participant Scheduler
alt Client Disconnects
Client->>Emitter: close connection
Emitter->>Emitter: trigger onCompletion callback
Emitter->>StreamService: removeEmitter(turipId, emitterKey)
StreamService->>StreamService: emitters.remove(turipId, emitterKey)
StreamService->>Scheduler: cancel heartbeat
else Timeout (3 minutes)
Note over Emitter: No activity for 3 minutes
Emitter->>Emitter: trigger onTimeout callback
Emitter->>StreamService: removeEmitter(turipId, emitterKey)
StreamService->>StreamService: emitters.remove(turipId, emitterKey)
StreamService->>Scheduler: cancel heartbeat
else Error Occurs
Note over Emitter: Send failure
Emitter->>Emitter: completeWithError(exception)
Emitter->>Emitter: trigger onError callback
Emitter->>StreamService: removeEmitter(turipId, emitterKey)
StreamService->>StreamService: emitters.remove(turipId, emitterKey)
StreamService->>Scheduler: cancel heartbeat
end
5. 전체 아키텍처 흐름 (Complete Architecture Flow)sequenceDiagram
participant Client1 as Client 1
participant Client2 as Client 2
participant API as REST API
participant Domain as Domain Service
participant TxManager as Transaction Manager
participant EventBus as Spring Event Bus
participant Listener as Event Listener
participant ThreadPool as SSE Thread Pool<br/>(Core:2, Max:4, Queue:100)
participant StreamService as Stream Service
participant EmitterMap as ConcurrentHashMap<br/>(turipId → emitters)
Note over Client1,Client2: 1️⃣ 연결 수립 단계
Client1->>API: Subscribe (turipId=1)
API->>StreamService: create emitter
StreamService->>EmitterMap: store emitter1
StreamService-->>Client1: SSE Connected
Client2->>API: Subscribe (turipId=1)
API->>StreamService: create emitter
StreamService->>EmitterMap: store emitter2
StreamService-->>Client2: SSE Connected
Note over Client1,EmitterMap: 2️⃣ 도메인 이벤트 발생
Client1->>API: Update Folder Name
API->>Domain: updateName()
activate Domain
Domain->>TxManager: begin transaction
Domain->>Domain: update database
Domain->>EventBus: publishEvent(FOLDER_NAME_CHANGED)
Note over EventBus: Event queued
Domain-->>API: success
deactivate Domain
API-->>Client1: 200 OK
Note over TxManager,EventBus: 3️⃣ 트랜잭션 커밋 후 이벤트 전파
TxManager->>EventBus: commit transaction
EventBus->>Listener: `@TransactionalEventListener`<br/>(AFTER_COMMIT)
Note over Listener,ThreadPool: 4️⃣ 비동기 SSE 전송
Listener->>ThreadPool: `@Async`("sseEventExecutor")
activate ThreadPool
ThreadPool->>StreamService: sendFolderUpdateEvents(turipId=1)
StreamService->>EmitterMap: getFolderEmitters(turipId=1)
EmitterMap-->>StreamService: [emitter1, emitter2]
par Broadcast to all emitters
StreamService->>Client1: SSE: FOLDER_UPDATE
and
StreamService->>Client2: SSE: FOLDER_UPDATE
end
deactivate ThreadPool
Note over Client1,Client2: 5️⃣ 실시간 알림 수신
Client1->>Client1: ✅ Update UI
Client2->>Client2: ✅ Update UI
📊 주요 특징 정리
추가로 궁금하신 점이나 특정 시나리오에 대한 상세 다이어그램이 필요하시면 말씀해 주세요! 🎨 |
seaniiio
left a comment
seaniiio
left a comment
There was a problem hiding this comment.
고생했어요 하루! 역시 이벤트의 달인~~ દ=๑๑( ε:)
- 동시성 이슈는 코드래빗이 두 부분 짚어줬는데, 별도의 이슈에서 모아서 처리하는 것도 좋을 것 같아요.
- 코드래빗의 sse 리뷰 중 "폴더 삭제 시 해당 폴더의 모든 emitter를 명시적으로 정리하는 로직이 있으면 좋습니다." 이거 반영하면 좋을 것 같습니다!
- TransactionPhase.AFTER_COMMIT,
@Async사용 좋습니다 ~~ - 거부 정책 관련은 회의때 언급했던 것처럼, 저도 테스트를 통해 어느정도의 부하까지 처리 가능한지 확인해보는 과정이 필요할 것 같아요. 일단 지금 하루가 적용한 내용 좋습니다!
| List<FavoritePlace> existingFavoritePlaces = favoritePlaceRepository.findAllByPlaceAndAccount(place, account); | ||
| List<FavoritePlace> existingPlaces = favoritePlaceRepository.findAllByPlaceAndAccount(place, account); | ||
|
|
||
| Set<Long> affectedFolderIds = calculateAffectedFolderIds(existingPlaces, requestIds); |
.../src/main/java/turip/favorite/stream/controller/dto/response/MemberUpdateStreamResponse.java
Show resolved
Hide resolved
backend/src/main/java/turip/auth/controller/dto/response/LoginResponse.java
Show resolved
Hide resolved
|
|
||
| new ArrayList<>(folderEmitters.values()).forEach(emitter -> | ||
| sendFolderUpdateEvent(favoriteFolderId, response, emitter) | ||
| ); |
There was a problem hiding this comment.
folderEmitters.values()를 사용해 직접 참조하게 되면, 다른 스레드에서 요소를 추가하거나 삭제하는 경우 맵이 변경되어 일부 사용자에게는 이벤트가 발행되지 않는 문제가 생길 수 있기 때문입니다
| testImplementation "org.springframework.boot:spring-boot-testcontainers" | ||
| testImplementation "org.testcontainers:junit-jupiter" | ||
| testImplementation "org.testcontainers:testcontainers" | ||
| testImplementation "org.testcontainers:mysql" | ||
| testImplementation "org.testcontainers:testcontainers-junit-jupiter:2.0.2" | ||
| testImplementation "org.testcontainers:testcontainers:2.0.2" | ||
| testImplementation "org.testcontainers:testcontainers-mysql:2.0.2" |
backend/turip-app/src/test/java/turip/favorite/api/FavoriteFolderApiTest.java
Show resolved
Hide resolved
| @@ -49,8 +49,17 @@ public void validateMembership(Account account, FavoriteFolder favoriteFolder) { | |||
| } | |||
There was a problem hiding this comment.
c: 새로 생긴 validateMembership(Account account, Long favoriteFolderId)를 이용하도록 하는거 어떤가요?? 중복 코드를 줄일 수 있을 것 같습니당
There was a problem hiding this comment.
코드 내부에서 validateMembership(Account account, Long favoriteFolderId)를 호출하도록 하자는 건가유?
아니면 저 메서드를 사용하는 곳을 validateMembership(Account account, Long favoriteFolderId)로 대체하자는 건가요??
There was a problem hiding this comment.
코드 내부에서 validateMembership(Account account, Long favoriteFolderId)를 호출하자는 것을 의도하긴 했어요! 둘 다 상관 없긴 한데, validateMembership(Account account, FavoriteFolder favoriteFolder) 를 사용하고 있는 부분이 많아서, 시그니처는 유지시키되 내부만 변경해도 괜찮을 것 같습니당
boolean isMember = favoriteFolderAccountRepository.existsByFavoriteFolderAndAccount(favoriteFolder, account);
if (!isMember) {
throw new ForbiddenException(ErrorTag.FORBIDDEN);
}이 동작을 하는 코드를 한 곳으로 줄이고 싶었어요!
|
|
||
| import com.fasterxml.jackson.annotation.JsonProperty; | ||
| import java.time.LocalDateTime; | ||
| import turip.favorite.domain.event.ActionType; |
There was a problem hiding this comment.
c: FavoriteFolderEventListener, ActionType, FavoriteFolderUpdateEvent를 한 곳에 모아두는거 어떤가요? 개인적으로는 turip/favorite/stream/event 하위에 두는게 명확할 것 같다고 생각합니다!
There was a problem hiding this comment.
ActionType, FavoriteFolderUpdateEvent는 도메인 상태 변경을 나타내는 클래스이기 때문에 도메인 패키지가 더 적절하다고 생각합니다! ActionType이 sse관련 응답을 위해 생성된 점이 애매하긴 하지만 FavoriteFolderService같은 도메인 서비스 로직이 stream 패키지에 의존하지 않아야 된다고 생각했어요. (기술에 의존하지 않아야 된다고 생각했습니다!)
There was a problem hiding this comment.
기술에 의존하지 않아야 된다고 생각했습니다!
저는 FavoriteFolderService에서 ApplicationEventPublisher에 의존하는 순간 "기술에 의존하지 않는 순수한 도메인 서비스"라고 보기는 어렵다고 생각했어요. 그래서 순수한 도메인 서비스가 목표인거면, 이벤트를 publish하는 로직까지 포함해 이벤트 관련 로직을 애플리케이션 서비스로 분리해야 하지 않을까 싶습니다!
// 순수 도메인 서비스
@Service
class FavoriteFolderService {
// ApplicationEventPublisher 의존 제거
// 순수 비즈니스 로직
public FavoriteFolder create(...) {
// 검증, 생성 로직
return favoriteFolder;
}
}
// 기술 의존 서비스
@Service
class FavoriteFolderEventService {
private final FavoriteFolderService folderService;
private final ApplicationEventPublisher eventPublisher;
public FavoriteFolder createAndPublish(...) {
FavoriteFolder folder = folderService.create(...);
eventPublisher.publishEvent(new FavoriteFolderUpdateEvent(...));
return folder;
}
}ActionType, FavoriteFolderUpdateEvent는 도메인 상태 변경을 나타내는 클래스이기 때문에 도메인 패키지가 더 적절하다고 생각합니다!
도메인 상태 변경이긴 하지만, 이벤트라는 기술에 사용하기 위해 필요하기 때문에 도메인 패키지에 위치시키기 애매하다고 생각했던 것 같아요. (처음에 FavoriteFolderUpdateEvent 찾을 때 stream 하위에서 찾았던 제 경험 기반..ㅎ)
지금처럼 도메인 서비스에서 기술에 대한 적당한 의존을 허용한다면 패키지 이동을 해도 괜찮을 것 같은데 어떻게 생각하나요? 패키지 이동에 대한 생각 변화가 없다면 유지해도 상관 없습니다~!
There was a problem hiding this comment.
ApplicationEventPublisher는 이벤트를 발행하는 역할이고 제가 생각하는 '기술에 의존한다는 것'은 sse(현재 구조에서는 stream 패키지)를 직접적으로 사용하지 않는 것을 의미합니다!
폴더 서비스에서 이벤트를 발행하는 것 자체는 도메인의 비즈니스 요구사항에 따른 흐름이라고 생각했고, 그 이벤트를 받아서 FavoriteFolderEventListener에서 sse를 사용해 처리한 것(여기서만 기술에 의존)이라고 생각했습니다!
그래서 패키지 구조를 유지해도 좋을 것 같다는 생각입니다~
…urip into feature/#585 # Conflicts: # backend/turip-app/src/main/java/turip/favorite/repository/FavoriteFolderAccountRepository.java # backend/turip-app/src/main/java/turip/favorite/service/FavoriteFolderAccountService.java # backend/turip-app/src/main/java/turip/favorite/service/FavoriteFolderService.java
2 participants
Issues
✔️ Check-list
🗒️ Work Description
이번 기능 구현 커밋입니다!
알림을 보내는 시점, 대상, 방법(SSE)에 대한 정보는 도메인 서비스가 알 필요가 없도록
ApplicationEventPublisher를 구현했습니다.@TransactionalEventListener를 사용했습니다. DB 데이터가 실제로 커밋되어 확정된 후에만 SSE 이벤트를 발행하게 됩니다.phase = TransactionPhase.AFTER_COMMIT이 기본 옵션이지만 흐름 이해를 위해 명시적으로 작성해두었습니다.알림 전송을 다른 작업이 기다릴 필요가 없다고 생각해
@Async를 사용해 비동기 처리 했습니다.MaxPoolSize까지 스레드가 늘어나고, 너무 크면 작업이 뒤로 밀려 실시간성이 떨어지기 때문에 메모리를 너무 많이 차지하지 않으면서도 여유있는 개수를 설정했습니다.거부 정책
📷 Screenshot
📚 Reference
Summary by CodeRabbit
릴리스 노트
새로운 기능
개선사항