[RELEASE] v0.2.1#108
Open
KoungQ wants to merge 24 commits into
Open
Conversation
…-및-전역-soft-delete-적용 [REFACTOR] DB Flush 호출 최소화 및 전역 Soft Delete 적용
[REFACTOR] 서버 리소스 스펙 최적화
…위한-환경-설정 [FEAT] GC 로그 및 JVM 프로파일링 환경 설정
Walkthrough이 PR은 채팅·방 도메인의 삭제를 소프트 삭제로 마이그레이션하고, Java Flight Recording 모니터링과 PostgreSQL 지원을 추가하며, k6 로드 테스트 인프라를 구성하는 대규모 변경입니다. 소프트 삭제 패턴 구현부터 인프라 업그레이드까지 여러 독립적인 기능 영역을 포함합니다. Changes소프트 삭제 패턴 구현
인프라 및 배포 업그레이드
🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
🚥 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 docstrings
🧪 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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/main/java/com/project/dorumdorum/domain/chat/domain/repository/ChatMessageRepository.java (1)
13-21:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
decreaseUnreadCount에 소프트 삭제 필터가 빠져 있어요.이번 변경으로
ChatMessage에 소프트 삭제가 도입되었지만, 바로 위의decreaseUnreadCountUPDATE 쿼리는m.deletedAt IS NULL조건이 없어서 이미 소프트 삭제된 메시지의unreadCount까지 함께 감소시킬 수 있어요. 동일 채팅방에 과거 삭제된 이력이 남아 있는 경우 불필요한 쓰기가 발생하고, 만약 향후 복원 정책이 생긴다면 카운터가 음수로 떨어진 채 노출될 위험도 있습니다.
AND m.deletedAt IS NULL한 줄을 추가해 두면 의미상 일관되고 안전합니다. (참고:@SQLRestriction은 Hibernate가 생성하는 SELECT에만 적용되고 JPQLUPDATE에는 자동 적용되지 않아, 명시적 조건이 필요합니다.)♻️ 제안 수정
`@Modifying`(clearAutomatically = true, flushAutomatically = true) `@Query`("UPDATE ChatMessage m SET m.unreadCount = m.unreadCount - 1 " + "WHERE m.chatRoom.chatRoomNo = :chatRoomNo " + "AND m.createdAt > :fromTime " + "AND m.senderNo != :userNo " + - "AND m.unreadCount > 0") + "AND m.unreadCount > 0 " + + "AND m.deletedAt IS NULL") int decreaseUnreadCount(`@Param`("chatRoomNo") String chatRoomNo, `@Param`("fromTime") LocalDateTime fromTime, `@Param`("userNo") String userNo);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/project/dorumdorum/domain/chat/domain/repository/ChatMessageRepository.java` around lines 13 - 21, The JPQL UPDATE in decreaseUnreadCount (in ChatMessageRepository) is missing the soft-delete filter and can modify unreadCount for soft-deleted ChatMessage rows; update the query in the decreaseUnreadCount method (the `@Query` on ChatMessage) to include "AND m.deletedAt IS NULL" so the UPDATE only affects non-deleted messages..github/workflows/cicd.yml (2)
100-107:⚠️ Potential issue | 🟠 Major | ⚡ Quick win배포 대상을 mutable tag/branch head가 아니라 커밋 SHA로 고정해 주세요.
지금은 이미지를
:pinpoint로만 push/pull하고, 배포 시점에는production브랜치 HEAD를 다시 clone 하고 있습니다. 이 조합이면 production에 연속 push가 들어오거나 workflow를 재실행할 때, 이번 run이 빌드한 결과물이 아니라 더 최신/이전 커밋의 이미지·compose를 섞어서 배포할 수 있습니다.pinpoint-${{ github.sha }}같은 불변 태그와${{ github.sha }}checkout으로 같은 스냅샷을 배포하도록 맞추는 편이 안전합니다.예시
- name: Build and push pinpoint image uses: docker/build-push-action@v6 with: context: . file: ./Dockerfile.pinpoint push: true - tags: ${{ secrets.DOCKERHUB_USERNAME }}/dorumdorum-be:pinpoint + tags: | + ${{ secrets.DOCKERHUB_USERNAME }}/dorumdorum-be:pinpoint + ${{ secrets.DOCKERHUB_USERNAME }}/dorumdorum-be:pinpoint-${{ github.sha }} - name: Deploy over SSH uses: appleboy/ssh-action@v1.2.0 env: - IMAGE: ${{ secrets.DOCKERHUB_USERNAME }}/dorumdorum-be:pinpoint + IMAGE: ${{ secrets.DOCKERHUB_USERNAME }}/dorumdorum-be:pinpoint-${{ github.sha }} + GITHUB_SHA: ${{ github.sha }} with: - envs: IMAGE,DOCKERHUB_USERNAME,DOCKERHUB_TOKEN,CONTAINER_NAME,GITHUB_TOKEN,REPO_FULL_NAME + envs: IMAGE,DOCKERHUB_USERNAME,DOCKERHUB_TOKEN,CONTAINER_NAME,GITHUB_TOKEN,REPO_FULL_NAME,GITHUB_SHA script: | - git clone --depth 1 -b production "https://x-access-token:${GITHUB_TOKEN}`@github.com/`${REPO_FULL_NAME}.git" temp_repo + git clone --depth 1 "https://x-access-token:${GITHUB_TOKEN}`@github.com/`${REPO_FULL_NAME}.git" temp_repo ( cd temp_repo + git fetch --depth 1 origin "${GITHUB_SHA}" + git checkout --detach "${GITHUB_SHA}" git -c url."https://x-access-token:${GITHUB_TOKEN}`@github.com/`".insteadOf="https://github.com/" \ submodule update --init --recursive --depth 1 )Also applies to: 121-132, 165-170
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/cicd.yml around lines 100 - 107, The workflow pushes images using docker/build-push-action@v6 with a mutable tag (tags: ${{ secrets.DOCKERHUB_USERNAME }}/dorumdorum-be:pinpoint) which can cause mismatched deploys; change the image tag to an immutable one using the commit SHA (e.g., pinpoint-${{ github.sha }}) when building/pushing, update any other docker/build-push-action steps likewise, and ensure subsequent deployment steps that checkout code or reference the image use actions/checkout with ref: ${{ github.sha }} (or otherwise reference the same ${{ github.sha }} tag) so the built image and the checked-out compose/manifests match the exact same commit.
125-125:⚠️ Potential issue | 🟠 Major | ⚡ Quick win별도 private submodule이면
GITHUB_TOKEN인증 실패 가능성이 큽니다현재 워크플로우가
GITHUB_TOKEN만으로git submodule update --init --recursive까지 수행하는데, submodule이 워크플로우가 실행되는 저장소와 다른 private 저장소라면 기본GITHUB_TOKEN권한 범위 때문에 클론/가져오기가 실패할 수 있습니다(보통 “접근 권한 없음/Repository not found”). 따라서 submodule 저장소까지 읽을 수 있는 PAT 또는 GitHub App 토큰을 별도 secret으로 두고 submodule fetch 단계에 사용하도록 구성해 주세요. (동일 이슈는 165-170 라인에도 적용됩니다.)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/cicd.yml at line 125, The workflow currently uses GITHUB_TOKEN for submodule operations which can fail for private submodules; create a new secret (e.g., SUBMODULE_PAT or SUBMODULE_ACCESS_TOKEN) and update the steps that run git submodule update --init --recursive and any actions/checkout steps to pass that secret as the token input instead of GITHUB_TOKEN (or call git with an authenticated URL using the PAT); specifically replace uses of GITHUB_TOKEN around the git submodule update --init --recursive step and the other occurrence noted (lines ~165-170) so the submodule fetch uses secrets.SUBMODULE_PAT or an equivalent GitHub App token. Ensure the new secret is documented and referenced in the job/env where token is needed.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/cicd.yml:
- Around line 44-47: The CI is always skipping integration tests by passing the
flag "-PskipIntegrationTests" in the "Run tests" step (./gradlew test
-PskipIntegrationTests --no-daemon); remove that unconditional skip and instead
either (a) make the flag conditional based on the pipeline context (e.g., only
add -PskipIntegrationTests for non-production branches/PR types using a GitHub
Actions if/conditional or an environment variable) or (b) create a separate
required job (e.g., "integration-tests") that runs ./gradlew test without
-PskipIntegrationTests for production-targeted pushes/PRs; update the "Run
tests" step and/or add the new job so integration tests run for
production-relevant runs while still allowing faster checks on other branches.
In @.gitmodules:
- Line 3: Update the submodule URL in .gitmodules from the HTTPS entry (url =
https://github.com/DorumDorum/secrets) to the SSH form (e.g., url =
git@github.com:DorumDorum/secrets.git), then update the repo to use the new URL
by running git submodule sync && git submodule update --init --recursive; also
ensure your CI is configured with the corresponding GitHub deploy key or SSH key
with proper access and that any CI job clones submodules using SSH (or add the
key to the runner) so authentication works in pipeline runs.
In `@docker-compose.yml`:
- Around line 162-163: Compose 파일의 k6 서비스가 이미지 태그 grafana/k6:latest 를 사용해 릴리스
재현성을 깨뜨리고 있으니 k6 서비스의 image 설정(서비스 이름 "k6"과 image 항목)을 의도한 안정적인 버전으로 고정하세요; 예를
들어 image 값을 최신 대신 버전 고정 문자열으로 바꾸거나 환경변수 K6_IMAGE 기본값을
grafana/k6:<desired-version>처럼 명시적으로 설정해 재현 가능한 빌드를 보장하도록 수정하세요.
In `@Dockerfile`:
- Line 22: The ENTRYPOINT line sets JFR_SETTINGS default to "profile" which
incurs runtime overhead; update the DEFAULT_JAVA_OPTS substitution to use
settings=${JFR_SETTINGS:-default} instead of profile so production containers
use the lightweight JFR "default" by default and tests can still enable
profiling via JFR_SETTINGS=profile; modify the ENTRYPOINT string where
DEFAULT_JAVA_OPTS and JFR_SETTINGS are referenced (the DEFAULT_JAVA_OPTS
variable construction in the ENTRYPOINT) to make this change.
- Line 22: The ENTRYPOINT's DEFAULT_JAVA_OPTS uses a static default for
JFR_RUN_ID which causes JFR output filename
(filename=/app/logs/jfr/dorumdorum-${JFR_RUN_ID:-cloud-load-test}.jfr) to be
overwritten on restarts; change the default expansion for JFR_RUN_ID in the
ENTRYPOINT (and related DEFAULT_JAVA_OPTS/EFFECTIVE_JAVA_OPTS) to include a
unique suffix such as a timestamp or container identifier (e.g. date
+%Y%m%d-%H%M%S or hostname/CONTAINER_ID) so the generated filename is unique per
run and preserves previous recordings.
In `@Dockerfile.pinpoint`:
- Around line 43-51: Extract the long DEFAULT_JAVA_OPTS/EFFECTIVE_JAVA_OPTS
construction and java launch logic out of the Dockerfile ENTRYPOINT into a
shared entrypoint script (entrypoint.sh) and update this Dockerfile to COPY that
script and use ENTRYPOINT ["sh","-c","/app/entrypoint.sh"] so both images share
one JFR/JAVA_OPTS source; move the -javaagent and -D... flags into that script
as written (refer to DEFAULT_JAVA_OPTS, EFFECTIVE_JAVA_OPTS, ENTRYPOINT and the
java launch command) and ensure the JFR filename pattern (currently
dorumdorum-${JFR_RUN_ID:-cloud-load-test}.jfr) generates a true unique filename
(e.g., prefer JFR_RUN_ID with a timestamp fallback) to avoid overwriting JFR
files when refactoring.
In `@monitoring/grafana/provisioning/dashboards/json/k6-jvm-gc-load-test.json`:
- Around line 175-180: The PromQL in this dashboard uses k6 metric names that
may include incorrect `_seconds_` infixes; update any queries referencing
metrics like k6_http_req_duration_seconds_p95 or
k6_http_req_duration_seconds_p99 to use k6_http_req_duration_p95 and
k6_http_req_duration_p99 (and similarly for other duration quantiles) while
keeping k6_http_req_failed_rate as-is; also verify the
K6_PROMETHEUS_RW_TREND_STATS export settings so the p95/p99 quantiles are
actually exported and adjust the panel queries to match the exported metric
names (e.g., replace *_duration_seconds_* → *_duration_* and ensure testid label
selectors remain unchanged).
- Line 240: 현재 대시보드의 PromQL 식
(sum(jvm_memory_used_bytes{application="dorumdorum",area="heap"}) /
sum(jvm_memory_max_bytes{application="dorumdorum",area="heap"}))는
application="dorumdorum" 라벨에 하드코딩되어 있는데, 프로메테우스의
static_configs.labels.application에 의해 이미 붙는지(또는 다른 환경에서 라벨 키가 다른지)를 확인하고,
Grafana가 바라보는 Prometheus 인스턴스가 어떤 라벨을 넣는지 확인한 뒤 식을 다음 중 하나로 수정하세요: 1) 애플리케이션을
대시보드 변수(예: $application)로 치환해 여러 환경에 대응하거나, 2) application 라벨을 제거해 라벨 없는 집계로
만들거나, 3) 필요하면 실제 라벨 키(예: job 또는 다른 키)로 교체하여 jvm_memory_used_bytes 및
jvm_memory_max_bytes 선택자가 올바른 라벨과 일치하게 하세요; 관련 식 문자열과
메트릭명(jvm_memory_used_bytes, jvm_memory_max_bytes, area="heap")을 찾아 변경하면 됩니다.
In
`@src/main/java/com/project/dorumdorum/domain/checklist/domain/entity/ChecklistBase.java`:
- Around line 133-135: delete() currently overwrites deletedAt every call losing
the original deletion timestamp; modify ChecklistBase.delete() to be idempotent
by setting deletedAt only if it is null (i.e., guard with a check like “if
(deletedAt == null) { deletedAt = LocalDateTime.now(); }”) so repeated calls
preserve the first deletion time while keeping the same method and field names
(ChecklistBase.delete, deletedAt).
In
`@src/test/java/com/project/dorumdorum/domain/chat/integration/ChatTransactionAtomicityIntegrationTest.java`:
- Around line 349-362: The duplicated truncateTables() helper (containing the
multi-table TRUNCATE ... RESTART IDENTITY CASCADE SQL) should be extracted to a
single reusable test utility to avoid sync errors; create a shared component
(e.g., testsupport.DatabaseCleaner with a TRUNCATE_SQL constant and a
truncateAll() method) or an abstract base test class (e.g.,
AbstractPersistenceCleanupTest) that has JdbcTemplate injected and exposes
truncateTables()/truncateAll(), then replace the local truncateTables()
implementations in ChatTransactionAtomicityIntegrationTest,
DeleteRoomPersistenceIntegrationTest, and KickRoommatePersistenceIntegrationTest
with calls to the shared DatabaseCleaner.truncateAll() or the base class method.
In
`@src/test/java/com/project/dorumdorum/domain/room/integration/FlushRemovalIntegrationTest.java`:
- Line 82: The test class FlushRemovalIntegrationTest currently declares
chatRoomMemberService with `@MockitoSpyBean` but never stubs or verifies it;
remove the unused partial spy to avoid breaking ApplicationContext caching by
either (A) change the annotation to `@Autowired` on the chatRoomMemberService
field if you intend to use the real bean, or (B) if you intended to control
behavior, keep `@MockitoSpyBean` and add explicit stubbing
(doReturn(...).when(chatRoomMemberService)...) or verify(...) calls in the test;
pick one of these and update the chatRoomMemberService declaration and tests
accordingly.
- Around line 186-194: 테스트의 현재 assertion은 roommateRepository.findByUserNo가 결과를
비우는지를 확인해 소프트 삭제 필터링 여부만 검증하므로, 검증 의도를 명확히 하세요: либо DisplayName과 assertion
메시지에서 "제거된다"를 "조회되지 않는다"로 바꾸어 findByUserNo의 동작을 명세하고 그대로 유지하거나,
deleteRoomUseCase.execute 호출 후 JdbcTemplate를 사용해 Roommate 테이블의 해당 row를 직접 조회(예:
SELECT deleted_at FROM roommate WHERE user_no = ?)하여 deleted_at이 NOT NULL인지 확인해
실질적 소프트 삭제가 이루어졌음을 검증하세요; 관련 식별자는
delete_RoommateCleanup_PersistedWithoutExplicitFlush 테스트 메소드,
roommateRepository.findByUserNo 호출, 그리고 deleteRoomUseCase.execute입니다.
- Around line 214-227: The truncateTables method in FlushRemovalIntegrationTest
currently omits the users (user) table which can leave residual User rows
affecting assertions like findByUserNo; update truncateTables (the
jdbcTemplate.execute TRUNCATE block) to include the users (or user) table name
in the list and then extract this method into a shared testsupport utility
(e.g., a common truncateTables helper) so other tests can reuse it and avoid
per-test maintenance whenever entities with `@SQLRestriction` are added.
- Around line 110-124: The fixture sets kickRoom.plusCurrentMate() only once but
then persists two Roommate entries (HOST and MEMBER), causing currentMateCount
to be 1 while two roommates exist; update the setup so kickRoom.currentMateCount
matches the two saved roommates—e.g., call kickRoom.plusCurrentMate() twice (or
otherwise set/increment currentMateCount to 2) before saving with
roomRepository.saveAndFlush(kickRoom) so the test
kick_CurrentMateCountDecrement_PersistedWithoutExplicitFlush accurately reflects
the HOST+MEMBER state.
---
Outside diff comments:
In @.github/workflows/cicd.yml:
- Around line 100-107: The workflow pushes images using
docker/build-push-action@v6 with a mutable tag (tags: ${{
secrets.DOCKERHUB_USERNAME }}/dorumdorum-be:pinpoint) which can cause mismatched
deploys; change the image tag to an immutable one using the commit SHA (e.g.,
pinpoint-${{ github.sha }}) when building/pushing, update any other
docker/build-push-action steps likewise, and ensure subsequent deployment steps
that checkout code or reference the image use actions/checkout with ref: ${{
github.sha }} (or otherwise reference the same ${{ github.sha }} tag) so the
built image and the checked-out compose/manifests match the exact same commit.
- Line 125: The workflow currently uses GITHUB_TOKEN for submodule operations
which can fail for private submodules; create a new secret (e.g., SUBMODULE_PAT
or SUBMODULE_ACCESS_TOKEN) and update the steps that run git submodule update
--init --recursive and any actions/checkout steps to pass that secret as the
token input instead of GITHUB_TOKEN (or call git with an authenticated URL using
the PAT); specifically replace uses of GITHUB_TOKEN around the git submodule
update --init --recursive step and the other occurrence noted (lines ~165-170)
so the submodule fetch uses secrets.SUBMODULE_PAT or an equivalent GitHub App
token. Ensure the new secret is documented and referenced in the job/env where
token is needed.
In
`@src/main/java/com/project/dorumdorum/domain/chat/domain/repository/ChatMessageRepository.java`:
- Around line 13-21: The JPQL UPDATE in decreaseUnreadCount (in
ChatMessageRepository) is missing the soft-delete filter and can modify
unreadCount for soft-deleted ChatMessage rows; update the query in the
decreaseUnreadCount method (the `@Query` on ChatMessage) to include "AND
m.deletedAt IS NULL" so the UPDATE only affects non-deleted messages.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8ed2d60d-b10f-41e6-a4fd-1d13c4437a0a
⛔ Files ignored due to path filters (1)
load-testing/README.mdis excluded by!**/*.md
📒 Files selected for processing (44)
.dockerignore.github/workflows/cicd.yml.gitignore.gitmodulesDockerfileDockerfile.pinpointbuild.gradledocker-compose.ymlload-testing/k6/.gitkeepmonitoring/grafana/provisioning/dashboards/json/k6-jvm-gc-load-test.jsonsecretssrc/main/java/com/project/dorumdorum/domain/chat/domain/entity/ChatMessage.javasrc/main/java/com/project/dorumdorum/domain/chat/domain/entity/ChatRoom.javasrc/main/java/com/project/dorumdorum/domain/chat/domain/entity/ChatRoomMember.javasrc/main/java/com/project/dorumdorum/domain/chat/domain/repository/ChatMessageRepository.javasrc/main/java/com/project/dorumdorum/domain/chat/domain/repository/ChatRoomMemberRepository.javasrc/main/java/com/project/dorumdorum/domain/chat/domain/repository/ChatRoomRepository.javasrc/main/java/com/project/dorumdorum/domain/chat/domain/service/ChatRoomMemberService.javasrc/main/java/com/project/dorumdorum/domain/chat/domain/service/ChatRoomService.javasrc/main/java/com/project/dorumdorum/domain/checklist/domain/entity/ChecklistBase.javasrc/main/java/com/project/dorumdorum/domain/checklist/domain/entity/RoomRule.javasrc/main/java/com/project/dorumdorum/domain/checklist/domain/repository/RoomRuleRepository.javasrc/main/java/com/project/dorumdorum/domain/room/application/usecase/DeleteRoomUseCase.javasrc/main/java/com/project/dorumdorum/domain/room/application/usecase/KickRoommateUseCase.javasrc/main/java/com/project/dorumdorum/domain/room/domain/entity/RoomLike.javasrc/main/java/com/project/dorumdorum/domain/room/domain/entity/RoomRequest.javasrc/main/java/com/project/dorumdorum/domain/room/domain/repository/RoomLikeRepository.javasrc/main/java/com/project/dorumdorum/domain/room/domain/repository/RoomRequestRepository.javasrc/main/java/com/project/dorumdorum/domain/room/domain/service/RoomRequestService.javasrc/main/java/com/project/dorumdorum/domain/room/domain/service/RoomService.javasrc/main/java/com/project/dorumdorum/domain/roommate/domain/entity/Roommate.javasrc/main/java/com/project/dorumdorum/domain/roommate/domain/service/RoommateService.javasrc/main/resources/application-prod.ymlsrc/main/resources/schema.sqlsrc/test/java/com/project/dorumdorum/domain/chat/integration/ChatTransactionAtomicityIntegrationTest.javasrc/test/java/com/project/dorumdorum/domain/chat/unit/service/ChatRoomMemberServiceTest.javasrc/test/java/com/project/dorumdorum/domain/chat/unit/service/ChatRoomServiceTest.javasrc/test/java/com/project/dorumdorum/domain/room/integration/DeleteRoomPersistenceIntegrationTest.javasrc/test/java/com/project/dorumdorum/domain/room/integration/FlushRemovalIntegrationTest.javasrc/test/java/com/project/dorumdorum/domain/room/integration/KickRoommatePersistenceIntegrationTest.javasrc/test/java/com/project/dorumdorum/domain/room/unit/service/RoomRequestServiceTest.javasrc/test/java/com/project/dorumdorum/domain/room/unit/usecase/DeleteRoomUseCaseTest.javasrc/test/java/com/project/dorumdorum/domain/room/unit/usecase/KickRoommateUseCaseTest.javasrc/test/java/com/project/dorumdorum/domain/roommate/unit/service/RoommateServiceTest.java
💤 Files with no reviewable changes (3)
- src/main/java/com/project/dorumdorum/domain/room/domain/service/RoomService.java
- src/main/java/com/project/dorumdorum/domain/room/application/usecase/KickRoommateUseCase.java
- src/test/java/com/project/dorumdorum/domain/room/unit/usecase/KickRoommateUseCaseTest.java
Comment on lines
44
to
+47
| - name: Run tests | ||
| run: | | ||
| echo ">>> [CI] Running tests..." | ||
| ./gradlew test --no-daemon | ||
| echo ">>> [CI] Running tests without integration tests..." | ||
| ./gradlew test -PskipIntegrationTests --no-daemon |
There was a problem hiding this comment.
기본 CI에서 통합 테스트를 항상 건너뛰면 이번 PR 유형의 회귀를 놓치기 쉽습니다.
현재 test job이 유일한 검증 단계인데 여기서 항상 -PskipIntegrationTests를 넘기고 있습니다. 이번 릴리스에는 DB 마이그레이션과 배포 변경이 함께 들어가 있어서 통합 테스트가 핵심 방어선인데, 이 설정이면 CI에서 전혀 실행되지 않습니다. 최소한 production 대상 PR/push에서는 별도 required job으로 돌리거나 조건부로만 skip 하시는 편이 안전합니다.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/cicd.yml around lines 44 - 47, The CI is always skipping
integration tests by passing the flag "-PskipIntegrationTests" in the "Run
tests" step (./gradlew test -PskipIntegrationTests --no-daemon); remove that
unconditional skip and instead either (a) make the flag conditional based on the
pipeline context (e.g., only add -PskipIntegrationTests for non-production
branches/PR types using a GitHub Actions if/conditional or an environment
variable) or (b) create a separate required job (e.g., "integration-tests") that
runs ./gradlew test without -PskipIntegrationTests for production-targeted
pushes/PRs; update the "Run tests" step and/or add the new job so integration
tests run for production-relevant runs while still allowing faster checks on
other branches.
| @@ -0,0 +1,3 @@ | |||
| [submodule "secrets"] | |||
| path = secrets | |||
| url = https://github.com/DorumDorum/secrets | |||
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
CI/CD 파이프라인을 위해 SSH URL 사용을 고려해보세요.
현재 HTTPS URL을 사용하고 계시는데, CI/CD 환경에서는 SSH URL과 deploy key를 사용하는 것이 인증 관리 측면에서 더 안전하고 편리합니다.
🔐 SSH URL로 변경하는 방법
[submodule "secrets"]
path = secrets
- url = https://github.com/DorumDorum/secrets
+ url = git@github.com:DorumDorum/secrets.git참고: SSH URL 사용 시 CI/CD 환경에 GitHub deploy key를 설정해주셔야 합니다.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.gitmodules at line 3, Update the submodule URL in .gitmodules from the
HTTPS entry (url = https://github.com/DorumDorum/secrets) to the SSH form (e.g.,
url = git@github.com:DorumDorum/secrets.git), then update the repo to use the
new URL by running git submodule sync && git submodule update --init
--recursive; also ensure your CI is configured with the corresponding GitHub
deploy key or SSH key with proper access and that any CI job clones submodules
using SSH (or add the key to the runner) so authentication works in pipeline
runs.
Comment on lines
+162
to
+163
| k6: | ||
| image: ${K6_IMAGE:-grafana/k6:latest} |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
grafana/k6:latest는 릴리스 재현성을 깨뜨립니다.
릴리스용 Compose에서 latest 태그를 쓰면 다음 이미지 공개 시점에 테스트 동작이나 experimental-prometheus-rw 호환성이 예고 없이 바뀔 수 있습니다. 의도한 k6 버전으로 고정해주시는 편이 안전합니다.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docker-compose.yml` around lines 162 - 163, Compose 파일의 k6 서비스가 이미지 태그
grafana/k6:latest 를 사용해 릴리스 재현성을 깨뜨리고 있으니 k6 서비스의 image 설정(서비스 이름 "k6"과 image
항목)을 의도한 안정적인 버전으로 고정하세요; 예를 들어 image 값을 최신 대신 버전 고정 문자열으로 바꾸거나 환경변수 K6_IMAGE
기본값을 grafana/k6:<desired-version>처럼 명시적으로 설정해 재현 가능한 빌드를 보장하도록 수정하세요.
| EXPOSE 8080 | ||
|
|
||
| ENTRYPOINT ["sh", "-c", "exec java ${JAVA_OPTS} -Dspring.profiles.active=${SPRING_PROFILES_ACTIVE:-prod} -jar /app/app.jar"] | ||
| ENTRYPOINT ["sh", "-c", "DEFAULT_JAVA_OPTS=\"-XX:StartFlightRecording=name=dorumdorum,settings=${JFR_SETTINGS:-profile},disk=true,dumponexit=true,filename=/app/logs/jfr/dorumdorum-${JFR_RUN_ID:-cloud-load-test}.jfr,maxage=${JFR_MAX_AGE:-30m},maxsize=${JFR_MAX_SIZE:-512m}\"; EFFECTIVE_JAVA_OPTS=\"${DEFAULT_JAVA_OPTS} ${JAVA_OPTS:-}\"; exec java ${EFFECTIVE_JAVA_OPTS} -Dspring.profiles.active=${SPRING_PROFILES_ACTIVE:-prod} -jar /app/app.jar"] |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | 💤 Low value
settings=profile을 상시 운영에 적용하기엔 오버헤드가 부담될 수 있습니다.
profile 설정은 일반적으로 ~1–2% 수준의 런타임 오버헤드(특히 alloc/lock 프로파일링)가 발생합니다. 상시 운영 컨테이너에서는 settings=${JFR_SETTINGS:-default}로 두고, 로드 테스트 시에만 JFR_SETTINGS=profile로 오버라이드하는 방식이 더 안전합니다. 본 변경의 의도가 k6 부하 시험용이라면 기본값을 default로 두시고 환경별로 명시적으로 활성화하시는 걸 권장드립니다.
🧰 Tools
🪛 Checkov (3.2.529)
[low] 1-22: Ensure that HEALTHCHECK instructions have been added to container images
(CKV_DOCKER_2)
[low] 1-22: Ensure that a user for the container has been created
(CKV_DOCKER_3)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Dockerfile` at line 22, The ENTRYPOINT line sets JFR_SETTINGS default to
"profile" which incurs runtime overhead; update the DEFAULT_JAVA_OPTS
substitution to use settings=${JFR_SETTINGS:-default} instead of profile so
production containers use the lightweight JFR "default" by default and tests can
still enable profiling via JFR_SETTINGS=profile; modify the ENTRYPOINT string
where DEFAULT_JAVA_OPTS and JFR_SETTINGS are referenced (the DEFAULT_JAVA_OPTS
variable construction in the ENTRYPOINT) to make this change.
JFR 출력 파일명이 재시작 시 덮어쓰여집니다.
JFR_RUN_ID의 기본값이 정적 문자열 cloud-load-test로 설정되어 있어, 환경변수를 매번 다르게 주입하지 않는 한 컨테이너 재시작이나 재배포 시 /app/logs/jfr/dorumdorum-cloud-load-test.jfr 파일이 그대로 덮어쓰여 이전 레코딩이 유실됩니다. dumponexit=true로 종료 시 덤프를 보존하려는 의도와도 충돌할 수 있어요.
기본값에 타임스탬프($(date +%Y%m%d-%H%M%S))나 호스트네임/컨테이너 ID 등 고유 식별자를 포함하시는 걸 권장드립니다.
🛠️ 제안 패치
-ENTRYPOINT ["sh", "-c", "DEFAULT_JAVA_OPTS=\"-XX:StartFlightRecording=name=dorumdorum,settings=${JFR_SETTINGS:-profile},disk=true,dumponexit=true,filename=/app/logs/jfr/dorumdorum-${JFR_RUN_ID:-cloud-load-test}.jfr,maxage=${JFR_MAX_AGE:-30m},maxsize=${JFR_MAX_SIZE:-512m}\"; EFFECTIVE_JAVA_OPTS=\"${DEFAULT_JAVA_OPTS} ${JAVA_OPTS:-}\"; exec java ${EFFECTIVE_JAVA_OPTS} -Dspring.profiles.active=${SPRING_PROFILES_ACTIVE:-prod} -jar /app/app.jar"]
+ENTRYPOINT ["sh", "-c", "JFR_RUN_ID=${JFR_RUN_ID:-$(date +%Y%m%d-%H%M%S)-$$}; DEFAULT_JAVA_OPTS=\"-XX:StartFlightRecording=name=dorumdorum,settings=${JFR_SETTINGS:-profile},disk=true,dumponexit=true,filename=/app/logs/jfr/dorumdorum-${JFR_RUN_ID}.jfr,maxage=${JFR_MAX_AGE:-30m},maxsize=${JFR_MAX_SIZE:-512m}\"; EFFECTIVE_JAVA_OPTS=\"${DEFAULT_JAVA_OPTS} ${JAVA_OPTS:-}\"; exec java ${EFFECTIVE_JAVA_OPTS} -Dspring.profiles.active=${SPRING_PROFILES_ACTIVE:-prod} -jar /app/app.jar"]📝 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
| ENTRYPOINT ["sh", "-c", "DEFAULT_JAVA_OPTS=\"-XX:StartFlightRecording=name=dorumdorum,settings=${JFR_SETTINGS:-profile},disk=true,dumponexit=true,filename=/app/logs/jfr/dorumdorum-${JFR_RUN_ID:-cloud-load-test}.jfr,maxage=${JFR_MAX_AGE:-30m},maxsize=${JFR_MAX_SIZE:-512m}\"; EFFECTIVE_JAVA_OPTS=\"${DEFAULT_JAVA_OPTS} ${JAVA_OPTS:-}\"; exec java ${EFFECTIVE_JAVA_OPTS} -Dspring.profiles.active=${SPRING_PROFILES_ACTIVE:-prod} -jar /app/app.jar"] | |
| ENTRYPOINT ["sh", "-c", "JFR_RUN_ID=${JFR_RUN_ID:-$(date +%Y%m%d-%H%M%S)-$$}; DEFAULT_JAVA_OPTS=\"-XX:StartFlightRecording=name=dorumdorum,settings=${JFR_SETTINGS:-profile},disk=true,dumponexit=true,filename=/app/logs/jfr/dorumdorum-${JFR_RUN_ID}.jfr,maxage=${JFR_MAX_AGE:-30m},maxsize=${JFR_MAX_SIZE:-512m}\"; EFFECTIVE_JAVA_OPTS=\"${DEFAULT_JAVA_OPTS} ${JAVA_OPTS:-}\"; exec java ${EFFECTIVE_JAVA_OPTS} -Dspring.profiles.active=${SPRING_PROFILES_ACTIVE:-prod} -jar /app/app.jar"] |
🧰 Tools
🪛 Checkov (3.2.529)
[low] 1-22: Ensure that HEALTHCHECK instructions have been added to container images
(CKV_DOCKER_2)
[low] 1-22: Ensure that a user for the container has been created
(CKV_DOCKER_3)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Dockerfile` at line 22, The ENTRYPOINT's DEFAULT_JAVA_OPTS uses a static
default for JFR_RUN_ID which causes JFR output filename
(filename=/app/logs/jfr/dorumdorum-${JFR_RUN_ID:-cloud-load-test}.jfr) to be
overwritten on restarts; change the default expansion for JFR_RUN_ID in the
ENTRYPOINT (and related DEFAULT_JAVA_OPTS/EFFECTIVE_JAVA_OPTS) to include a
unique suffix such as a timestamp or container identifier (e.g. date
+%Y%m%d-%H%M%S or hostname/CONTAINER_ID) so the generated filename is unique per
run and preserves previous recordings.
Comment on lines
+43
to
51
| ENTRYPOINT ["sh", "-c", "DEFAULT_JAVA_OPTS=\"-XX:StartFlightRecording=name=dorumdorum,settings=${JFR_SETTINGS:-profile},disk=true,dumponexit=true,filename=/app/logs/jfr/dorumdorum-${JFR_RUN_ID:-cloud-load-test}.jfr,maxage=${JFR_MAX_AGE:-30m},maxsize=${JFR_MAX_SIZE:-512m}\"; EFFECTIVE_JAVA_OPTS=\"${DEFAULT_JAVA_OPTS} ${JAVA_OPTS:-}\"; exec java \ | ||
| -javaagent:/app/pinpoint-agent/pinpoint-bootstrap-${PINPOINT_VERSION}.jar \ | ||
| -Dpinpoint.agentId=${PINPOINT_AGENT_ID} \ | ||
| -Dpinpoint.applicationName=${PINPOINT_APPLICATION_NAME} \ | ||
| -Dprofiler.transport.grpc.collector.ip=${PINPOINT_COLLECTOR_IP} \ | ||
| -Dpinpoint.container=true \ | ||
| ${JAVA_OPTS} \ | ||
| ${EFFECTIVE_JAVA_OPTS} \ | ||
| -Dspring.profiles.active=${SPRING_PROFILES_ACTIVE:-prod} \ | ||
| -jar /app/app.jar"] |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy lift
Dockerfile과 JFR 옵션 구성이 그대로 중복되어 유지보수 위험이 있습니다.
DEFAULT_JAVA_OPTS 구성 문자열이 Dockerfile(Line 22)과 완전히 동일하게 복제되어 있어, 한쪽만 수정될 경우 두 이미지 간 JFR 동작이 서로 달라질 수 있습니다. 공통 entrypoint 스크립트(예: docker/entrypoint.sh)를 추가하고 두 Dockerfile에서 COPY + ENTRYPOINT ["/app/entrypoint.sh"] 형태로 호출하시면 단일 출처에서 JFR/JAVA_OPTS 정책을 관리할 수 있습니다.
또한 앞 파일에서 지적한 JFR 파일명 덮어쓰기 이슈가 본 파일에도 동일하게 적용되니, 수정 시 함께 반영 부탁드립니다.
🧰 Tools
🪛 Checkov (3.2.529)
[low] 1-51: Ensure that HEALTHCHECK instructions have been added to container images
(CKV_DOCKER_2)
[low] 1-51: Ensure that a user for the container has been created
(CKV_DOCKER_3)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Dockerfile.pinpoint` around lines 43 - 51, Extract the long
DEFAULT_JAVA_OPTS/EFFECTIVE_JAVA_OPTS construction and java launch logic out of
the Dockerfile ENTRYPOINT into a shared entrypoint script (entrypoint.sh) and
update this Dockerfile to COPY that script and use ENTRYPOINT
["sh","-c","/app/entrypoint.sh"] so both images share one JFR/JAVA_OPTS source;
move the -javaagent and -D... flags into that script as written (refer to
DEFAULT_JAVA_OPTS, EFFECTIVE_JAVA_OPTS, ENTRYPOINT and the java launch command)
and ensure the JFR filename pattern (currently
dorumdorum-${JFR_RUN_ID:-cloud-load-test}.jfr) generates a true unique filename
(e.g., prefer JFR_RUN_ID with a timestamp fallback) to avoid overwriting JFR
files when refactoring.
Comment on lines
+349
to
+362
| private void truncateTables() { | ||
| jdbcTemplate.execute(""" | ||
| TRUNCATE TABLE | ||
| chat_message, | ||
| chat_room_member, | ||
| chat_room, | ||
| room_request, | ||
| room_rule, | ||
| room_like, | ||
| roommate, | ||
| room | ||
| RESTART IDENTITY CASCADE | ||
| """); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
중복되는 truncateTables() 헬퍼는 공용 베이스/유틸리티로 추출하시는 것을 권장드립니다.
동일한 TRUNCATE ... RESTART IDENTITY CASCADE SQL 블록과 헬퍼 메서드가 DeleteRoomPersistenceIntegrationTest, KickRoommatePersistenceIntegrationTest 세 파일에 그대로 복제되어 있습니다. 새 테이블이 추가되거나 테이블명이 바뀔 때 세 곳을 모두 동기화해야 하므로 누락 위험이 있습니다.
추상 베이스 클래스(예: AbstractPersistenceCleanupTest)에 JdbcTemplate 주입과 truncateTables()를 한 곳에 두거나, testsupport 패키지에 DatabaseCleaner 컴포넌트를 만들어 @Autowired로 주입해 호출하시면 DRY가 유지되고 스키마 변경 시 단일 지점에서만 갱신하면 됩니다.
♻️ 예시: `testsupport`에 공용 cleaner 도입
// src/test/java/com/project/dorumdorum/testsupport/DatabaseCleaner.java
`@Component`
public class DatabaseCleaner {
private static final String TRUNCATE_SQL = """
TRUNCATE TABLE
chat_message,
chat_room_member,
chat_room,
room_request,
room_rule,
room_like,
roommate,
room
RESTART IDENTITY CASCADE
""";
private final JdbcTemplate jdbcTemplate;
public DatabaseCleaner(JdbcTemplate jdbcTemplate) {
this.jdbcTemplate = jdbcTemplate;
}
public void truncateAll() {
jdbcTemplate.execute(TRUNCATE_SQL);
}
}각 테스트에서는 다음과 같이 사용 가능합니다:
- `@Autowired` private JdbcTemplate jdbcTemplate;
+ `@Autowired` private DatabaseCleaner databaseCleaner;
...
- truncateTables();
+ databaseCleaner.truncateAll();
...
- private void truncateTables() {
- jdbcTemplate.execute("""
- TRUNCATE TABLE
- ...
- """);
- }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@src/test/java/com/project/dorumdorum/domain/chat/integration/ChatTransactionAtomicityIntegrationTest.java`
around lines 349 - 362, The duplicated truncateTables() helper (containing the
multi-table TRUNCATE ... RESTART IDENTITY CASCADE SQL) should be extracted to a
single reusable test utility to avoid sync errors; create a shared component
(e.g., testsupport.DatabaseCleaner with a TRUNCATE_SQL constant and a
truncateAll() method) or an abstract base test class (e.g.,
AbstractPersistenceCleanupTest) that has JdbcTemplate injected and exposes
truncateTables()/truncateAll(), then replace the local truncateTables()
implementations in ChatTransactionAtomicityIntegrationTest,
DeleteRoomPersistenceIntegrationTest, and KickRoommatePersistenceIntegrationTest
with calls to the shared DatabaseCleaner.truncateAll() or the base class method.
|
|
||
| @MockitoSpyBean private UserService userService; | ||
| @MockitoSpyBean private ChatMessageService chatMessageService; | ||
| @MockitoSpyBean private ChatRoomMemberService chatRoomMemberService; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | 💤 Low value
사용되지 않는 @MockitoSpyBean 정의를 검토해주세요.
chatRoomMemberService가 @MockitoSpyBean으로 선언되어 있지만, 테스트 본문이나 stub* 메서드 어디에서도 doReturn(...).when(chatRoomMemberService)... 형태로 동작을 재정의하거나 verify(chatRoomMemberService)로 호출을 검증하는 부분이 없습니다. 단순 부분 스파이만 등록해두면 실제 빈을 그대로 사용하는 것과 차이가 없으면서, ApplicationContext 캐시 키만 달라져 다른 통합 테스트와의 컨텍스트 재사용이 깨질 수 있어요. 의도된 스텁/검증이 빠진 것이 아니라면 일반 @Autowired로 변경하는 것을 고려해주세요.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@src/test/java/com/project/dorumdorum/domain/room/integration/FlushRemovalIntegrationTest.java`
at line 82, The test class FlushRemovalIntegrationTest currently declares
chatRoomMemberService with `@MockitoSpyBean` but never stubs or verifies it;
remove the unused partial spy to avoid breaking ApplicationContext caching by
either (A) change the annotation to `@Autowired` on the chatRoomMemberService
field if you intend to use the real bean, or (B) if you intended to control
behavior, keep `@MockitoSpyBean` and add explicit stubbing
(doReturn(...).when(chatRoomMemberService)...) or verify(...) calls in the test;
pick one of these and update the chatRoomMemberService declaration and tests
accordingly.
Comment on lines
+110
to
+124
| kickRoom = roomRepository.save(Room.builder() | ||
| .roomType(RoomType.TYPE_1).capacity(2).title("kick-flush-test") | ||
| .hostUserNo(KICK_HOST_NO).residencePeriod(ResidencePeriod.SEMESTER) | ||
| .gender(Gender.MALE).build()); | ||
| kickRoom.plusCurrentMate(); | ||
| kickRoom = roomRepository.saveAndFlush(kickRoom); | ||
|
|
||
| roommateRepository.save(Roommate.builder().room(kickRoom).userNo(KICK_HOST_NO) | ||
| .roomRole(RoomRole.HOST).confirmStatus(ConfirmStatus.ACCEPTED).build()); | ||
| roommateRepository.save(Roommate.builder().room(kickRoom).userNo(KICK_MEMBER_NO) | ||
| .roomRole(RoomRole.MEMBER).confirmStatus(ConfirmStatus.ACCEPTED).build()); | ||
|
|
||
| kickChatRoom = chatRoomRepository.save(ChatRoom.builder().roomNo(kickRoom.getRoomNo()).build()); | ||
| chatRoomMemberRepository.save(ChatRoomMember.builder().chatRoom(kickChatRoom).userNo(KICK_HOST_NO).build()); | ||
| chatRoomMemberRepository.save(ChatRoomMember.builder().chatRoom(kickChatRoom).userNo(KICK_MEMBER_NO).build()); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
kickRoom의 currentMateCount와 실제 Roommate 행 수가 불일치합니다.
plusCurrentMate()를 1회만 호출해 currentMateCount = 1로 시작하지만, 실제로는 HOST + MEMBER 두 명의 Roommate가 저장됩니다. kick_CurrentMateCountDecrement_PersistedWithoutExplicitFlush에서는 beforeCount - 1 = 0이 되어 어설션은 통과하지만, 실제 프로덕션 시나리오(호스트와 멤버 둘 다 입장한 상태 → 멤버 강퇴 후 1명 남음)와 의미가 달라져 회귀 보호력이 떨어집니다. 픽스처를 다음과 같이 맞춰 두는 편이 안전합니다.
♻️ 제안 패치
kickRoom = roomRepository.save(Room.builder()
.roomType(RoomType.TYPE_1).capacity(2).title("kick-flush-test")
.hostUserNo(KICK_HOST_NO).residencePeriod(ResidencePeriod.SEMESTER)
.gender(Gender.MALE).build());
kickRoom.plusCurrentMate();
+ kickRoom.plusCurrentMate();
kickRoom = roomRepository.saveAndFlush(kickRoom);📝 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
| kickRoom = roomRepository.save(Room.builder() | |
| .roomType(RoomType.TYPE_1).capacity(2).title("kick-flush-test") | |
| .hostUserNo(KICK_HOST_NO).residencePeriod(ResidencePeriod.SEMESTER) | |
| .gender(Gender.MALE).build()); | |
| kickRoom.plusCurrentMate(); | |
| kickRoom = roomRepository.saveAndFlush(kickRoom); | |
| roommateRepository.save(Roommate.builder().room(kickRoom).userNo(KICK_HOST_NO) | |
| .roomRole(RoomRole.HOST).confirmStatus(ConfirmStatus.ACCEPTED).build()); | |
| roommateRepository.save(Roommate.builder().room(kickRoom).userNo(KICK_MEMBER_NO) | |
| .roomRole(RoomRole.MEMBER).confirmStatus(ConfirmStatus.ACCEPTED).build()); | |
| kickChatRoom = chatRoomRepository.save(ChatRoom.builder().roomNo(kickRoom.getRoomNo()).build()); | |
| chatRoomMemberRepository.save(ChatRoomMember.builder().chatRoom(kickChatRoom).userNo(KICK_HOST_NO).build()); | |
| chatRoomMemberRepository.save(ChatRoomMember.builder().chatRoom(kickChatRoom).userNo(KICK_MEMBER_NO).build()); | |
| kickRoom = roomRepository.save(Room.builder() | |
| .roomType(RoomType.TYPE_1).capacity(2).title("kick-flush-test") | |
| .hostUserNo(KICK_HOST_NO).residencePeriod(ResidencePeriod.SEMESTER) | |
| .gender(Gender.MALE).build()); | |
| kickRoom.plusCurrentMate(); | |
| kickRoom.plusCurrentMate(); | |
| kickRoom = roomRepository.saveAndFlush(kickRoom); | |
| roommateRepository.save(Roommate.builder().room(kickRoom).userNo(KICK_HOST_NO) | |
| .roomRole(RoomRole.HOST).confirmStatus(ConfirmStatus.ACCEPTED).build()); | |
| roommateRepository.save(Roommate.builder().room(kickRoom).userNo(KICK_MEMBER_NO) | |
| .roomRole(RoomRole.MEMBER).confirmStatus(ConfirmStatus.ACCEPTED).build()); | |
| kickChatRoom = chatRoomRepository.save(ChatRoom.builder().roomNo(kickRoom.getRoomNo()).build()); | |
| chatRoomMemberRepository.save(ChatRoomMember.builder().chatRoom(kickChatRoom).userNo(KICK_HOST_NO).build()); | |
| chatRoomMemberRepository.save(ChatRoomMember.builder().chatRoom(kickChatRoom).userNo(KICK_MEMBER_NO).build()); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@src/test/java/com/project/dorumdorum/domain/room/integration/FlushRemovalIntegrationTest.java`
around lines 110 - 124, The fixture sets kickRoom.plusCurrentMate() only once
but then persists two Roommate entries (HOST and MEMBER), causing
currentMateCount to be 1 while two roommates exist; update the setup so
kickRoom.currentMateCount matches the two saved roommates—e.g., call
kickRoom.plusCurrentMate() twice (or otherwise set/increment currentMateCount to
2) before saving with roomRepository.saveAndFlush(kickRoom) so the test
kick_CurrentMateCountDecrement_PersistedWithoutExplicitFlush accurately reflects
the HOST+MEMBER state.
Comment on lines
+186
to
+194
| @Test | ||
| @DisplayName("방 삭제 후 Roommate가 DB에서 제거된다 (explicit flush 없이)") | ||
| void delete_RoommateCleanup_PersistedWithoutExplicitFlush() { | ||
| deleteRoomUseCase.execute(DELETE_HOST_NO, deleteRoom.getRoomNo()); | ||
|
|
||
| assertThat(roommateRepository.findByUserNo(DELETE_HOST_NO)) | ||
| .as("방 삭제 시 Roommate가 DB에서 제거되어야 한다") | ||
| .isEmpty(); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | 💤 Low value
findByUserNo 어설션이 소프트 삭제의 의미와 어긋날 수 있습니다.
이 어설션이 비어 있음을 보장하는 근거는 Roommate 엔티티의 @SQLRestriction(또는 deletedAt IS NULL 조건)에 의존합니다. 즉, "DB에서 제거됨"이 아니라 "조회 시 필터링됨"을 검증하는 셈입니다. 표현과 의미를 정확히 일치시키기 위해 (1) DisplayName/메시지에서 "제거된다"를 "조회되지 않는다" 정도로 다듬거나, (2) JdbcTemplate로 row의 deleted_at IS NOT NULL을 직접 확인하면 검증 의도가 더 명확해집니다.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@src/test/java/com/project/dorumdorum/domain/room/integration/FlushRemovalIntegrationTest.java`
around lines 186 - 194, 테스트의 현재 assertion은 roommateRepository.findByUserNo가 결과를
비우는지를 확인해 소프트 삭제 필터링 여부만 검증하므로, 검증 의도를 명확히 하세요: либо DisplayName과 assertion
메시지에서 "제거된다"를 "조회되지 않는다"로 바꾸어 findByUserNo의 동작을 명세하고 그대로 유지하거나,
deleteRoomUseCase.execute 호출 후 JdbcTemplate를 사용해 Roommate 테이블의 해당 row를 직접 조회(예:
SELECT deleted_at FROM roommate WHERE user_no = ?)하여 deleted_at이 NOT NULL인지 확인해
실질적 소프트 삭제가 이루어졌음을 검증하세요; 관련 식별자는
delete_RoommateCleanup_PersistedWithoutExplicitFlush 테스트 메소드,
roommateRepository.findByUserNo 호출, 그리고 deleteRoomUseCase.execute입니다.
Comment on lines
+214
to
+227
| private void truncateTables() { | ||
| jdbcTemplate.execute(""" | ||
| TRUNCATE TABLE | ||
| chat_message, | ||
| chat_room_member, | ||
| chat_room, | ||
| room_request, | ||
| room_rule, | ||
| room_like, | ||
| roommate, | ||
| room | ||
| RESTART IDENTITY CASCADE | ||
| """); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | 💤 Low value
TRUNCATE 대상 테이블 누락 여부 확인이 필요해요.
현재 목록에 users/user 테이블이 빠져 있는데, 본 테스트의 픽스처는 User를 직접 저장하지 않으므로 문제는 없어 보입니다. 다만 동일한 컨텍스트에서 다른 테스트가 User를 저장하고 종료될 경우 잔여 데이터가 남아 본 테스트의 어설션(특히 findByUserNo)에 영향이 갈 가능성이 있습니다. 또 @SQLRestriction이 걸린 엔티티가 추가될 때마다 이 리스트를 유지보수해야 하니, 가능하면 공통 truncateTables 유틸(testsupport)로 추출해두는 편을 권합니다.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@src/test/java/com/project/dorumdorum/domain/room/integration/FlushRemovalIntegrationTest.java`
around lines 214 - 227, The truncateTables method in FlushRemovalIntegrationTest
currently omits the users (user) table which can leave residual User rows
affecting assertions like findByUserNo; update truncateTables (the
jdbcTemplate.execute TRUNCATE block) to include the users (or user) table name
in the list and then extract this method into a shared testsupport utility
(e.g., a common truncateTables helper) so other tests can reuse it and avoid
per-test maintenance whenever entities with `@SQLRestriction` are added.
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.
📝 Pull Request Template
📌 제목
✅ PR 체크리스트
📜 기타
Summary by CodeRabbit
릴리스 노트
New Features
Bug Fixes
Chores