[refactor] #64 Redis 캐싱 적용 및 Critical/High 보안·정합성 이슈 수정#64
Merged
[refactor] #64 Redis 캐싱 적용 및 Critical/High 보안·정합성 이슈 수정#64
Conversation
like == null 분기에서 return으로 조기 종료하여 TransactionUtils.afterCommit() 호출이 누락되는 버그 수정. if/else-if/else 구조로 변경하여 모든 분기에서 캐시 무효화 보장.
…체하여 역직렬화 RCE 취약점 제거 LaissezFaireSubTypeValidator는 클래스패스의 모든 non-final 클래스 역직렬화를 허용하여 Jackson 가젯 체인을 통한 RCE 위험이 있음. BasicPolymorphicTypeValidator로 교체하여 tave.crezipsa.crezipsa.*, java.util.*, java.time.* 패키지만 허용하도록 제한.
다른 쓰기 메서드(createComment, deleteComment)와 달리 updateComment만 직접 evictCommentsAndDetail을 호출하여 트랜잭션 커밋 전에 캐시가 제거되는 race condition 발생 가능. afterCommit으로 연기하여 일관성 확보.
[High] 댓글 이중 캐싱 제거 - CommunityDetailCacheDto에서 List<CommentCacheDto> comments 필드 제거 - getCommunity에서 댓글을 comment-list 캐시(getCommentsCache)로 분리 조회 - Redis 메모리 절감 및 comment-list/community-detail 간 정합성 불일치 리스크 제거 [High] likeCount 실시간 조회 - 캐시된 likeCount는 좋아요 동시 변경 시 TTL 동안 stale 가능 - likeRepository.countByCommunityIdAndIsLikedTrue()로 항상 최신 값 반환 - toResponse()에 likeCount 파라미터 추가하여 캐시 필드 override
[갱신] CommunityDetailCacheDto에서 comments 필드 제거에 따른 생성자 호출 수정 [갱신] getCommentsCache() / countByCommunityIdAndIsLikedTrue() mock stub 추가 [추가] likeCount는 캐시 값이 아닌 likeRepository 실시간 조회 값을 사용함을 검증 [추가] 댓글 목록은 getCommentsCache를 통해 comment-list 캐시에서 별도 조회됨을 검증
📝 WalkthroughWalkthroughRefactors community caching to remove embedded comments, fetch comments separately, compute live like counts, defer cache eviction to after transaction commit, tighten Jackson polymorphic validation, add AI-agent role docs for code modification/review/writing/testing, update tests and .gitignore. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant UseCase as CommunityUseCaseImpl
participant CacheSvc as CommunityCacheService
participant LikeRepo as LikeRepository
participant CommentCache as CommentCacheService
participant Cache as CacheStore
Client->>UseCase: getCommunityDetail(id)
UseCase->>CacheSvc: getCommunityDetailCache(id)
CacheSvc->>Cache: retrieve CommunityDetailCacheDto (no comments)
Cache-->>CacheSvc: CommunityDetailCacheDto
CacheSvc-->>UseCase: CommunityDetailCacheDto
par Fetch live like count
UseCase->>LikeRepo: countByCommunityIdAndIsLikedTrue(id)
LikeRepo-->>UseCase: likeCount
and Fetch comments separately
UseCase->>CommentCache: getCommentsCache(id)
CommentCache->>Cache: retrieve comments list
Cache-->>CommentCache: List<CommentResponse>
CommentCache-->>UseCase: comments
end
UseCase->>UseCase: toResponse(isWriter, isLiked, likeCount, comments)
UseCase-->>Client: CommunityDetailResponse
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ 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 |
seokjun01
changed the title
seokjun01
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/main/java/tave/crezipsa/crezipsa/application/community/dto/cache/CommunityDetailCacheDto.java (1)
41-47:CommunityDetailCacheDto의 캐시likeCount필드는 이제 사실상 중복 상태입니다.현재
toResponse(...)는 전달받은 실시간likeCount를 사용하므로, record 내부likeCount를 유지하면 의미가 충돌합니다. 캐시 payload와 혼동을 줄이기 위해 제거를 권장합니다.♻️ 정리 리팩터링 예시
public record CommunityDetailCacheDto( Long communityId, Long writerId, String title, String content, CommunityField field, List<String> imageUrls, WriterResponse writer, - long likeCount, long commentCount, LocalDateTime createdAt ) { @@ return new CommunityDetailCacheDto( community.getCommunityId(), community.getWriterId(), community.getTitle(), community.getContent(), community.getField(), community.getImageUrls(), writer, - community.getLikeCount(), commentCount, community.getCreatedAt() ); } - public CommunityDetailResponse toResponse(boolean isWriter, boolean isLiked, long likeCount, List<CommentResponse> comments) { + public CommunityDetailResponse toResponse(boolean isWriter, boolean isLiked, long liveLikeCount, List<CommentResponse> comments) { return new CommunityDetailResponse( @@ - likeCount, + liveLikeCount, commentCount, TimeUtils.convertToRelativeTime(createdAt), comments ); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/tave/crezipsa/crezipsa/application/community/dto/cache/CommunityDetailCacheDto.java` around lines 41 - 47, CommunityDetailCacheDto currently stores a redundant likeCount component that conflicts with the real-time likeCount passed into toResponse(boolean isWriter, boolean isLiked, long likeCount, List<CommentResponse> comments); remove the likeCount component from the record declaration and its constructor usage, update all places that construct CommunityDetailCacheDto to stop supplying a cached likeCount, and ensure toResponse continues to use the method parameter likeCount (not any removed field); also remove any getters/accessors or serialization mappings referencing the removed likeCount to avoid compilation/serialization errors.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.claude/agents/code-reviewer.md:
- Line 163: The file .claude/agents/code-reviewer.md contains a hardcoded
absolute user-specific path "/Users/zuny/…/agent-memory/code-reviewer/" which
will break for other developers; replace this literal with a repository-root or
home-agnostic construct (e.g., use process.cwd()/repository root or OS home via
path.join(os.homedir(), '.claude', 'agent-memory', 'code-reviewer') or an env
var) and ensure the memory directory is resolved dynamically and/or path is
relative to the repo; update any references to that string in the repo to use
the new resolution approach and add the memory folder to .gitignore if it should
not be committed.
In @.claude/agents/test-generator.md:
- Around line 92-94: The example static imports shown (import static
com.tave.common.fixture.UserFixture.*; import static
com.tave.community.fixture.CommunityFixture.*;) do not match our current package
layout; update the example to reference the actual fixture package structure
used in the repo (e.g., static imports from fixture.UserFixture,
fixture.ChatFixture, fixture.StoryboardFixture) and adjust the guidance to
instruct readers to place domain-specific fixture classes under the fixture
package and to use static imports of their factory methods (refer to
UserFixture, ChatFixture, StoryboardFixture as the canonical class names to
locate and correct the imports).
---
Nitpick comments:
In
`@src/main/java/tave/crezipsa/crezipsa/application/community/dto/cache/CommunityDetailCacheDto.java`:
- Around line 41-47: CommunityDetailCacheDto currently stores a redundant
likeCount component that conflicts with the real-time likeCount passed into
toResponse(boolean isWriter, boolean isLiked, long likeCount,
List<CommentResponse> comments); remove the likeCount component from the record
declaration and its constructor usage, update all places that construct
CommunityDetailCacheDto to stop supplying a cached likeCount, and ensure
toResponse continues to use the method parameter likeCount (not any removed
field); also remove any getters/accessors or serialization mappings referencing
the removed likeCount to avoid compilation/serialization errors.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
.claude/agents/code-modifier.md.claude/agents/code-reviewer.md.claude/agents/code-writer.md.claude/agents/test-generator.md.gitignoresrc/main/java/tave/crezipsa/crezipsa/application/community/cache/CommunityCacheService.javasrc/main/java/tave/crezipsa/crezipsa/application/community/dto/cache/CommunityDetailCacheDto.javasrc/main/java/tave/crezipsa/crezipsa/application/community/usecase/CommentUseCaseImpl.javasrc/main/java/tave/crezipsa/crezipsa/application/community/usecase/CommunityUseCaseImpl.javasrc/main/java/tave/crezipsa/crezipsa/application/community/usecase/LikeUseCaseImpl.javasrc/main/java/tave/crezipsa/crezipsa/global/config/CacheConfig.javasrc/test/java/tave/crezipsa/crezipsa/application/community/usecase/CommunityUseCaseImplTest.java
|
|
||
| # Persistent Agent Memory | ||
|
|
||
| You have a persistent Persistent Agent Memory directory at `/Users/zuny/Tave_Project/crezipsa/.claude/agent-memory/code-reviewer/`. Its contents persist across conversations. |
There was a problem hiding this comment.
Hardcoded absolute path with username will break for other developers.
The path /Users/zuny/Tave_Project/crezipsa/.claude/agent-memory/code-reviewer/ contains a user-specific home directory. This will not work for other team members.
Consider using a relative path from the repository root:
-You have a persistent Persistent Agent Memory directory at `/Users/zuny/Tave_Project/crezipsa/.claude/agent-memory/code-reviewer/`. Its contents persist across conversations.
+You have a persistent Persistent Agent Memory directory at `.claude/agent-memory/code-reviewer/` (relative to repository root). Its contents persist across conversations.📝 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.
Suggested change
| You have a persistent Persistent Agent Memory directory at `/Users/zuny/Tave_Project/crezipsa/.claude/agent-memory/code-reviewer/`. Its contents persist across conversations. | |
| You have a persistent Persistent Agent Memory directory at `.claude/agent-memory/code-reviewer/` (relative to repository root). Its contents persist across conversations. |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.claude/agents/code-reviewer.md at line 163, The file
.claude/agents/code-reviewer.md contains a hardcoded absolute user-specific path
"/Users/zuny/…/agent-memory/code-reviewer/" which will break for other
developers; replace this literal with a repository-root or home-agnostic
construct (e.g., use process.cwd()/repository root or OS home via
path.join(os.homedir(), '.claude', 'agent-memory', 'code-reviewer') or an env
var) and ensure the memory directory is resolved dynamically and/or path is
relative to the repo; update any references to that string in the repo to use
the new resolution approach and add the memory folder to .gitignore if it should
not be committed.
Comment on lines
+92
to
+94
| import static com.tave.common.fixture.UserFixture.*; | ||
| import static com.tave.community.fixture.CommunityFixture.*; | ||
|
|
There was a problem hiding this comment.
예시 fixture import 경로가 현재 코드베이스 패키지와 불일치합니다.
이 예시를 그대로 따를 경우 테스트 생성 코드가 컴파일 실패할 가능성이 큽니다.
📘 문서 예시 수정안
-import static com.tave.common.fixture.UserFixture.*;
-import static com.tave.community.fixture.CommunityFixture.*;
+import static tave.crezipsa.crezipsa.fixture.UserFixture.*;
+import static tave.crezipsa.crezipsa.fixture.CommunityFixture.*;Based on learnings: Applies to **/fixture/**Fixture.java : Create domain-specific fixture classes in fixture/ package (e.g., UserFixture, ChatFixture, StoryboardFixture) and use static import for factory methods.
📝 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.
Suggested change
| import static com.tave.common.fixture.UserFixture.*; | |
| import static com.tave.community.fixture.CommunityFixture.*; | |
| import static tave.crezipsa.crezipsa.fixture.UserFixture.*; | |
| import static tave.crezipsa.crezipsa.fixture.CommunityFixture.*; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.claude/agents/test-generator.md around lines 92 - 94, The example static
imports shown (import static com.tave.common.fixture.UserFixture.*; import
static com.tave.community.fixture.CommunityFixture.*;) do not match our current
package layout; update the example to reference the actual fixture package
structure used in the repo (e.g., static imports from fixture.UserFixture,
fixture.ChatFixture, fixture.StoryboardFixture) and adjust the guidance to
instruct readers to place domain-specific fixture classes under the fixture
package and to use static imports of their factory methods (refer to
UserFixture, ChatFixture, StoryboardFixture as the canonical class names to
locate and correct the imports).
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.claude/agents/test-generator.md (1)
10-10: Consider varying sentence beginnings for better readability.Three consecutive sentences begin with "You," which can feel repetitive. While acceptable for instructional tone, varying the structure could improve flow.
✍️ Optional rewording suggestion
-You are a **test generation expert for the CreZipsa project**. You generate high-quality, comprehensive test code following project conventions (JUnit 5, Mockito, AssertJ, H2, GWT structure, domain fixtures). You focus on maximum coverage including success paths AND exception paths. +You are a **test generation expert for the CreZipsa project**. Generate high-quality, comprehensive test code following project conventions (JUnit 5, Mockito, AssertJ, H2, GWT structure, domain fixtures). Focus on maximum coverage including success paths AND exception paths.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/agents/test-generator.md at line 10, The opening guidance in .claude/agents/test-generator.md repeats "You" at the start of three consecutive sentences; rewrite those latter sentences to vary their openings while preserving meaning and tone—for example, begin one with a gerund ("Generating high-quality..."), another with a clause ("Following project conventions such as JUnit 5, Mockito..."), or use passive voice—ensure the lines that reference "test generation expert for the CreZipsa project", "JUnit 5, Mockito, AssertJ, H2, GWT structure, domain fixtures", and "maximum coverage including success paths AND exception paths" remain accurate and included.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.claude/agents/test-generator.md:
- Line 92: The documentation example has an invalid Java import: "import
static.tave.crezipsa.crezipsa.fixture.*;" — remove the erroneous dot after
"static" and replace the wildcard with explicit fixture classes; update the
import to use normal static or regular imports as appropriate and reference
specific fixtures like UserFixture and CommunityFixture (e.g., import
tave.crezipsa.crezipsa.fixture.UserFixture; import
tave.crezipsa.crezipsa.fixture.CommunityFixture;) so generated tests compile and
follow project conventions.
---
Nitpick comments:
In @.claude/agents/test-generator.md:
- Line 10: The opening guidance in .claude/agents/test-generator.md repeats
"You" at the start of three consecutive sentences; rewrite those latter
sentences to vary their openings while preserving meaning and tone—for example,
begin one with a gerund ("Generating high-quality..."), another with a clause
("Following project conventions such as JUnit 5, Mockito..."), or use passive
voice—ensure the lines that reference "test generation expert for the CreZipsa
project", "JUnit 5, Mockito, AssertJ, H2, GWT structure, domain fixtures", and
"maximum coverage including success paths AND exception paths" remain accurate
and included.
| } | ||
|
|
||
| ### ✅ Fixture Usage with Static Import | ||
| import static.tave.crezipsa.crezipsa.fixture.*; |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
correct Java syntax for static imports
💡 Result:
Java static-import declarations come in two forms:
1) Import one specific static member
import static fully.qualified.TypeName.memberName;Formal syntax: import static TypeName . Identifier ; (where TypeName is the canonical name of a class/interface). (docs.oracle.com)
Example:
import static java.lang.Math.PI;
import static java.lang.Math.sqrt;2) Import all static members of a type (on-demand)
import static fully.qualified.TypeName.*;Formal syntax: import static TypeName . * ;. (docs.oracle.com)
Example:
import static java.lang.Math.*;Placement rule
Static imports go in the normal import section: after the optional package line and before any type declarations. (docs.oracle.com)
Example using System.out:
import static java.lang.System.out;
class Demo {
void f() { out.println("hi"); }
}
``` ([qualitestgroup.com](https://www.qualitestgroup.com/insights/technical-hub/how-to-use-static-imports-in-java/?utm_source=openai))
Citations:
- 1: https://docs.oracle.com/javase/specs/jls/se20/html/jls-7.html?utm_source=openai
- 2: https://docs.oracle.com/javase/specs/jls/se20/html/jls-7.html?utm_source=openai
- 3: https://docs.oracle.com/javase/specs/jls/se20/html/jls-7.html?utm_source=openai
- 4: https://www.qualitestgroup.com/insights/technical-hub/how-to-use-static-imports-in-java/?utm_source=openai
---
🏁 Script executed:
```shell
# Check if the markdown file exists and examine its context
fd "test-generator.md" 2>/dev/null | head -5Repository: CREJIPSA/CreJipsa-BE
Length of output: 46
🏁 Script executed:
# If found, examine the lines around line 92 to understand context
if [ -f ".claude/agents/test-generator.md" ]; then
echo "=== File found, examining lines 85-100 ===";
sed -n '85,100p' ".claude/agents/test-generator.md" | cat -n;
else
echo "File not found at .claude/agents/test-generator.md";
find . -name "test-generator.md" 2>/dev/null;
fiRepository: CREJIPSA/CreJipsa-BE
Length of output: 611
Fix invalid Java import syntax in documentation example.
The import statement contains a syntax error: there's an invalid dot immediately after the static keyword. This will cause compilation failures in any test code generated following this example.
Additionally, per project learnings, fixture imports should reference specific fixture classes (e.g., UserFixture, CommunityFixture) rather than wildcard package imports.
Corrected import statements
-import static.tave.crezipsa.crezipsa.fixture.*;
+import static tave.crezipsa.crezipsa.fixture.UserFixture.*;
+import static tave.crezipsa.crezipsa.fixture.CommunityFixture.*;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.claude/agents/test-generator.md at line 92, The documentation example has
an invalid Java import: "import static.tave.crezipsa.crezipsa.fixture.*;" —
remove the erroneous dot after "static" and replace the wildcard with explicit
fixture classes; update the import to use normal static or regular imports as
appropriate and reference specific fixtures like UserFixture and
CommunityFixture (e.g., import tave.crezipsa.crezipsa.fixture.UserFixture;
import tave.crezipsa.crezipsa.fixture.CommunityFixture;) so generated tests
compile and follow project conventions.
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
1 participant
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.
배경 (Why)
커뮤니티 목록·상세·댓글 조회는 읽기 요청이 집중되는 핫패스입니다.
매 요청마다 DB를 조회하면 트래픽이 몰릴수록 응답 지연과 DB 부하가 증가합니다.
이를 해소하기 위해 Redis 기반 캐싱을 도입했고, 이후 코드 리뷰에서 발견된
보안 취약점과 정합성 버그들을 함께 수정했습니다.
변경 사항
1단계 — 캐싱 인프라 구축
CacheConfigRedisCacheManager설정, TTL 5분, 캐시 이름 4종 등록CommunityCacheService@Cacheable/@CacheEvict집중 관리, self-invocation 문제 방지를 위한 별도 빈CommunitySummaryCacheDtoCommunityDetailCacheDtoCommentCacheDtoTimeUtilsTransactionUtils.afterCommit()2단계 — Critical 보안·버그 수정
🔴 [Critical] LaissezFaireSubTypeValidator → BasicPolymorphicTypeValidator 교체
LaissezFaireSubTypeValidator는 클래스패스의 모든 non-final 클래스 역직렬화를 허용합니다.BasicPolymorphicTypeValidator로 교체해tave.crezipsa.crezipsa.*,java.util.*,java.time.*패키지만 허용하도록 제한했습니다.🔴 [Critical]
like()첫 좋아요 시 캐시 무효화 누락like == null(첫 좋아요) 분기에서return으로 조기 종료되어afterCommit콜백이 등록되지 않았습니다.likeCount가 0으로 고정되는 데이터 불일치가 발생했습니다.if/else-if/else구조로 변경해 모든 경로에서 캐시 퇴거를 보장합니다.3단계 — High 정합성 이슈 수정
🟠 [High]
updateComment()직접 evict →afterCommit래핑으로 통일createComment,deleteComment는 모두afterCommit을 사용했지만,updateComment만 직접 호출했습니다.🟠 [High]
community-detail캐시에서 댓글 제거 →comment-list단일 출처 유지community-detail캐시가List<CommentCacheDto>를 포함하고,comment-list캐시도 동일 데이터 저장 → Redis 이중 저장community-detail은 메타데이터만 저장, 댓글은comment-list캐시에서만 조회🟠 [High]
likeCount실시간 조회로 교체likeCount는 좋아요 동시 변경 시 TTL 내에 stale 값이 노출될 수 있습니다.getCommunity()호출 시likeRepository.countByCommunityIdAndIsLikedTrue()로 항상 최신 값을 반환합니다.toResponse()에likeCount파라미터를 추가해 캐시 필드를 override하도록 변경했습니다.캐시 구조 요약
community-list,community-list-by-fieldcommunity-list,community-list-by-field,community-detailcommunity-list,community-list-by-field,community-detailcommunity-detail,comment-list테스트
CommunityUseCaseImplTest:getCommunity— likeCount 실시간 조회 검증, 댓글 별도 캐시 조회 검증 케이스 추가CommentUseCaseImplTest:updateComment— afterCommit 퇴거 호출 검증LikeUseCaseImplTest: like/unlike — 모든 경로에서 캐시 퇴거 호출 검증Summary by CodeRabbit
Release Notes
New Features
Improvements
Security
Tests