Merged
Conversation
WalkthroughSpring AOP의 자기 자신 호출 문제를 해결하기 위해 캐시 무효화 및 갱신 로직을 별도 Bean으로 분리했습니다. AuthV2ServiceImpl에서 캐시 관련 메서드를 제거하고 외부 서비스로 위임하는 방식으로 리팩토링되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client
participant AuthV2ServiceImpl
participant AuthV2UserCacheInvalidationService
participant AuthV2UserCacheCommandService
participant Cache as Spring Cache<br/>(AOP)
participant DB as UserRepository
rect rgba(200, 150, 100, 0.5)
Note over AuthV2ServiceImpl: 이전 문제: 자기 자신 호출
Client->>AuthV2ServiceImpl: loginUser(userId)
AuthV2ServiceImpl->>AuthV2ServiceImpl: clearCacheForUser()<br/>(AOP 미적용)
Note over Cache: ❌ 캐시 제거 안 됨
end
rect rgba(100, 200, 100, 0.5)
Note over AuthV2ServiceImpl: 개선된 방식: 외부 Bean 호출
Client->>AuthV2ServiceImpl: loginUser(userId)
AuthV2ServiceImpl->>AuthV2UserCacheInvalidationService: refreshCachesAfterUserUpdate()
AuthV2UserCacheInvalidationService->>AuthV2UserCacheCommandService: evictMeetingLeaderCache(userId)
AuthV2UserCacheCommandService->>Cache: `@CacheEvict` 실행<br/>(AOP 적용 ✓)
AuthV2UserCacheCommandService->>DB: 필요 시 데이터 조회
AuthV2UserCacheCommandService->>Cache: `@CachePut` 실행<br/>(AOP 적용 ✓)
Note over Cache: ✅ 캐시 정상 갱신
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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.
🧹 Nitpick comments (3)
main/src/test/java/org/sopt/makers/crew/main/auth/v2/service/AuthV2UserCacheInvalidationServiceTest.java (1)
48-52: 중복된 검증 제거 고려Line 49의
verify(authV2UserCacheCommandService).evictCoLeadersCache(100)와 Line 51의verify(..., times(1)).evictCoLeadersCache(100)는 동일한 검증입니다. 기본verify()는 이미 정확히 1회 호출을 검증합니다.♻️ 제안된 수정
verify(authV2UserCacheCommandService).evictMeetingLeaderCache(1); verify(authV2UserCacheCommandService).evictCoLeadersCache(100); verify(authV2UserCacheCommandService).evictCoLeadersCache(200); - verify(authV2UserCacheCommandService, times(1)).evictCoLeadersCache(100); verify(authV2UserCacheCommandService).refreshOrgIdCache();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@main/src/test/java/org/sopt/makers/crew/main/auth/v2/service/AuthV2UserCacheInvalidationServiceTest.java` around lines 48 - 52, The test contains duplicate verifications for authV2UserCacheCommandService.evictCoLeadersCache(100): both verify(authV2UserCacheCommandService).evictCoLeadersCache(100) and verify(authV2UserCacheCommandService, times(1)).evictCoLeadersCache(100) assert the same single invocation; remove one of them (keep either the default verify(...) or the explicit times(1)) to avoid redundant assertions in AuthV2UserCacheInvalidationServiceTest.main/src/main/java/org/sopt/makers/crew/main/auth/v2/service/AuthV2UserCacheInvalidationServiceImpl.java (1)
23-26: 스트림 체이닝 들여쓰기 일관성
.distinct()의 들여쓰기가 다른 메서드 체이닝과 일관되지 않습니다.♻️ 제안된 수정
coLeaderRepository.findAllByUserIdWithMeeting(userId).stream() .map(coLeader -> coLeader.getMeeting().getId()) - .distinct() + .distinct() .forEach(authV2UserCacheCommandService::evictCoLeadersCache);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@main/src/main/java/org/sopt/makers/crew/main/auth/v2/service/AuthV2UserCacheInvalidationServiceImpl.java` around lines 23 - 26, The method chain starting at coLeaderRepository.findAllByUserIdWithMeeting(userId).stream() has inconsistent indentation for .distinct(); align the `.distinct()` call with the other chained methods (same indentation level as `.map(...)` and `.forEach(...)`) so the chain is consistently formatted—locate the stream chain in AuthV2UserCacheInvalidationServiceImpl (the calls: coLeaderRepository.findAllByUserIdWithMeeting, .stream(), .map(coLeader -> coLeader.getMeeting().getId()), .distinct(), .forEach(authV2UserCacheCommandService::evictCoLeadersCache)) and adjust whitespace/indentation accordingly.main/src/main/java/org/sopt/makers/crew/main/auth/v2/service/AuthV2ServiceImpl.java (1)
34-36: 트랜잭션-캐시 정합성에 대한 향후 개선 고려PR 설명에서 언급하신대로, 현재 캐시 갱신이 같은 트랜잭션 내에서 수행됩니다. 트랜잭션 롤백 시 DB는 원복되지만 캐시는 이미 갱신되어 정합성 문제가 발생할 수 있습니다.
향후
@TransactionalEventListener(phase = TransactionPhase.AFTER_COMMIT)기반으로 커밋 이후 캐시 처리로 변경하면 이 문제를 해결할 수 있습니다.Also applies to: 51-59
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@main/src/main/java/org/sopt/makers/crew/main/auth/v2/service/AuthV2ServiceImpl.java` around lines 34 - 36, Replace the in-transaction direct cache refresh call to avoid rollback inconsistencies: in AuthV2ServiceImpl stop calling authV2UserCacheInvalidationService.refreshCachesAfterUserUpdate(...) inside the transactional flow (where updateUserIfChanged(...) is used) and instead publish a UserUpdatedEvent (or similar) with the user's id when updateUserIfChanged returns true; implement a separate listener component that listens for that event with `@TransactionalEventListener`(phase = TransactionPhase.AFTER_COMMIT) and invokes authV2UserCacheInvalidationService.refreshCachesAfterUserUpdate(userId) from the listener so cache invalidation only runs after the DB transaction commits.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@main/src/main/java/org/sopt/makers/crew/main/auth/v2/service/AuthV2ServiceImpl.java`:
- Around line 34-36: Replace the in-transaction direct cache refresh call to
avoid rollback inconsistencies: in AuthV2ServiceImpl stop calling
authV2UserCacheInvalidationService.refreshCachesAfterUserUpdate(...) inside the
transactional flow (where updateUserIfChanged(...) is used) and instead publish
a UserUpdatedEvent (or similar) with the user's id when updateUserIfChanged
returns true; implement a separate listener component that listens for that
event with `@TransactionalEventListener`(phase = TransactionPhase.AFTER_COMMIT)
and invokes
authV2UserCacheInvalidationService.refreshCachesAfterUserUpdate(userId) from the
listener so cache invalidation only runs after the DB transaction commits.
In
`@main/src/main/java/org/sopt/makers/crew/main/auth/v2/service/AuthV2UserCacheInvalidationServiceImpl.java`:
- Around line 23-26: The method chain starting at
coLeaderRepository.findAllByUserIdWithMeeting(userId).stream() has inconsistent
indentation for .distinct(); align the `.distinct()` call with the other chained
methods (same indentation level as `.map(...)` and `.forEach(...)`) so the chain
is consistently formatted—locate the stream chain in
AuthV2UserCacheInvalidationServiceImpl (the calls:
coLeaderRepository.findAllByUserIdWithMeeting, .stream(), .map(coLeader ->
coLeader.getMeeting().getId()), .distinct(),
.forEach(authV2UserCacheCommandService::evictCoLeadersCache)) and adjust
whitespace/indentation accordingly.
In
`@main/src/test/java/org/sopt/makers/crew/main/auth/v2/service/AuthV2UserCacheInvalidationServiceTest.java`:
- Around line 48-52: The test contains duplicate verifications for
authV2UserCacheCommandService.evictCoLeadersCache(100): both
verify(authV2UserCacheCommandService).evictCoLeadersCache(100) and
verify(authV2UserCacheCommandService, times(1)).evictCoLeadersCache(100) assert
the same single invocation; remove one of them (keep either the default
verify(...) or the explicit times(1)) to avoid redundant assertions in
AuthV2UserCacheInvalidationServiceTest.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2f8596d2-8729-4404-80ff-4969efd27110
📒 Files selected for processing (8)
main/src/main/java/org/sopt/makers/crew/main/auth/v2/service/AuthV2ServiceImpl.javamain/src/main/java/org/sopt/makers/crew/main/auth/v2/service/AuthV2UserCacheCommandService.javamain/src/main/java/org/sopt/makers/crew/main/auth/v2/service/AuthV2UserCacheCommandServiceImpl.javamain/src/main/java/org/sopt/makers/crew/main/auth/v2/service/AuthV2UserCacheInvalidationService.javamain/src/main/java/org/sopt/makers/crew/main/auth/v2/service/AuthV2UserCacheInvalidationServiceImpl.javamain/src/test/java/org/sopt/makers/crew/main/auth/v2/service/AuthV2ServiceTest.javamain/src/test/java/org/sopt/makers/crew/main/auth/v2/service/AuthV2UserCacheCommandServiceIntegrationTest.javamain/src/test/java/org/sopt/makers/crew/main/auth/v2/service/AuthV2UserCacheInvalidationServiceTest.java
huncozyboy
changed the title
Member
|
너무 좋은데요! 바로 올리면 될 거 같아요! |
khyojun
approved these changes
Mar 9, 2026
Member
Author
|
크루 파이팅 🥐 🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
👩💻 Contents
1.
AuthV2ServiceImpl내부 호출로 인해 적용되지 않던 캐시 AOP 경계를 외부 Bean으로 분리AuthV2UserCacheCommandService와AuthV2UserCacheInvalidationService로 분리하면서AuthV2ServiceImpl은 유저 동기화만 담당하도록 해주었습니다.2. 사용자 정보 변경 시 leader, coLeader, orgId 캐시가 올바르게 무효화 + 갱신되는지 검증하는 테스트 코드 구현
📝 Review Note
1. 테스트 코드 형식 관련
기존에 구현되어있던 테스트 코드중에서
AuthV2ServiceImpl와 관련하여updateOrgIds(),clearCacheForUser()를 직접 검증하던 테스트를 찾지 못하였습니다. 만약 직접 검증하던 테스트가 있었다면 노티해주시면 좋을거같습니다 !+추가로 테스트 코드는 기존에 있던 작성 형식과 네이밍을 최대한 참고해서 맞춰보려고 했는데, 아직 컨벤션이 익숙하지 않아 변수명이나 테스트 방식에서 컨벤션과 어긋난 부분이 없는지 봐주시면 감사하겠습니다 🙇🏻♂️
2. 트랜잭션-캐시 정합성 관련
현재 구조에서는 한가지 트랜잭션 안에서 사용자 정보 변경과 캐시 무효화, 갱신이 함께 수행되는 것으로 이해했습니다.
기능적으로는 문제 없지만, 트랜잭션이 롤백되는 경우 DB와 캐시 간 정합성이 어긋날 수도 있지 않을까하는 걱정이 들기도 했습니다.
따라서 추후에는
@TransactionalEventListener(phase = AFTER_COMMIT)기반으로 변경 이벤트를 발행하고, 커밋 이후에 캐시를 처리하는 방향을 고려해보면 좋을거같다는 개인적인 생각이 들었습니다.📣 Related Issue
✅ 점검사항