[REFACTOR] DB Flush 호출 최소화 및 전역 Soft Delete 적용#102
Merged
ydking0911 merged 6 commits intoMay 6, 2026
Hidden character warning
The head ref may contain hidden characters: "refactor/#101/db-flush-\ud638\ucd9c-\ucd5c\uc18c\ud654-\ubc0f-\uc804\uc5ed-soft-delete-\uc801\uc6a9"
Conversation
ydking0911
requested review from
KoungQ and
Copilot
and removed request for
KoungQ
Walkthrough여러 도메인 엔티티에 Hibernate 수준의 ChangesSoft-Delete 통합 (Entity → Service → Repository → Schema → Tests)
Sequence DiagramsequenceDiagram
participant Client
participant UseCase as DeleteRoomUseCase
participant RoomService as Room Service
participant RoommateService as Roommate Service
participant Repo as JPA Repository
participant DB as Database
Client->>UseCase: execute(requesterNo, roomNo)
UseCase->>RoomService: room.delete()
RoomService->>Repo: save(room)
Repo->>DB: UPDATE room SET deleted_at = NOW() WHERE ... AND deleted_at IS NULL
UseCase->>RoommateService: leaveRoom(requesterNo, roomNo)
RoommateService->>Repo: save(roommate)
Repo->>DB: UPDATE roommate SET deleted_at = NOW() WHERE ... AND deleted_at IS NULL
UseCase->>Repo: deleteAllByRoom (RoomRequest/RoomRule/RoomLike)
Repo->>DB: UPDATE ... SET deleted_at = NOW() WHERE ... AND deleted_at IS NULL
UseCase->>UseCase: publishEvent(RoomDeletedEvent)
DB-->>Client: Commit success
sequenceDiagram
participant Query
participant SQLRestriction as SQLRestriction Filter
participant DB as Database
Query->>DB: SELECT * FROM chat_message
DB-->>SQLRestriction: apply filter
SQLRestriction->>DB: SELECT * FROM chat_message WHERE deleted_at IS NULL
DB-->>Query: filtered rows
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 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 |
There was a problem hiding this comment.
Pull request overview
This PR refactors the room/chat deletion flows to rely on JPA-triggered flushes instead of explicit flush() calls, and converts several previously hard-deleted entities to deletedAt-based soft delete with @SQLRestriction filtering. In the codebase, this mainly affects room lifecycle, chat membership cleanup, and the repository/query contract for entities that can now be logically deleted.
Changes:
- Removed explicit
roomService.flush()calls from room deletion/kick flows and updated tests around the new ordering assumptions. - Switched multiple delete paths from hard delete to soft delete (
entity.delete()/ bulkUPDATE ... SET deletedAt) across room, chat, request, like, roommate, and rule-related entities. - Added schema/index updates and new integration/unit tests intended to verify flush timing and soft-delete behavior.
Reviewed changes
Copilot reviewed 28 out of 28 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/test/java/com/project/dorumdorum/domain/room/unit/usecase/KickRoommateUseCaseTest.java | Updated unit test expectations after removing explicit flush from kick flow. |
| src/test/java/com/project/dorumdorum/domain/room/unit/usecase/DeleteRoomUseCaseTest.java | Updated room deletion tests for new cascade ordering without explicit flush. |
| src/test/java/com/project/dorumdorum/domain/room/unit/service/RoomRequestServiceTest.java | Adjusted tests to expect soft delete/save instead of repository hard delete. |
| src/test/java/com/project/dorumdorum/domain/room/integration/FlushRemovalIntegrationTest.java | Added integration test for flush removal and DB persistence behavior. |
| src/test/java/com/project/dorumdorum/domain/chat/unit/service/ChatRoomServiceTest.java | Updated chat room service test to expect soft delete/save semantics. |
| src/test/java/com/project/dorumdorum/domain/chat/unit/service/ChatRoomMemberServiceTest.java | Updated chat room member service test to expect soft delete/save semantics. |
| src/main/resources/schema.sql | Replaced several global unique indexes with partial indexes for soft-deleted rows. |
| src/main/java/com/project/dorumdorum/domain/roommate/domain/service/RoommateService.java | Changed room leave behavior from hard delete to soft delete + save. |
| src/main/java/com/project/dorumdorum/domain/roommate/domain/entity/Roommate.java | Added global SQL restriction and removed table-level unique constraint mapping. |
| src/main/java/com/project/dorumdorum/domain/room/domain/service/RoomService.java | Removed explicit repository flush helper. |
| src/main/java/com/project/dorumdorum/domain/room/domain/service/RoomRequestService.java | Converted request deletion/cancel flows to soft delete. |
| src/main/java/com/project/dorumdorum/domain/room/domain/repository/RoomRequestRepository.java | Replaced bulk hard delete with bulk soft-delete update. |
| src/main/java/com/project/dorumdorum/domain/room/domain/repository/RoomLikeRepository.java | Replaced hard deletes with soft-delete JPQL updates for likes. |
| src/main/java/com/project/dorumdorum/domain/room/domain/entity/RoomRequest.java | Added SQL restriction and removed hard unique constraint mapping. |
| src/main/java/com/project/dorumdorum/domain/room/domain/entity/RoomLike.java | Added SQL restriction and removed hard unique constraint mapping. |
| src/main/java/com/project/dorumdorum/domain/room/application/usecase/KickRoommateUseCase.java | Removed explicit flush from roommate kick flow. |
| src/main/java/com/project/dorumdorum/domain/room/application/usecase/DeleteRoomUseCase.java | Removed explicit flush from room deletion flow and updated docs. |
| src/main/java/com/project/dorumdorum/domain/checklist/domain/repository/RoomRuleRepository.java | Switched room rule bulk delete to soft-delete update. |
| src/main/java/com/project/dorumdorum/domain/checklist/domain/entity/RoomRule.java | Added SQL restriction for soft-deleted room rules. |
| src/main/java/com/project/dorumdorum/domain/checklist/domain/entity/ChecklistBase.java | Added deletedAt state and delete helpers to checklist-based entities. |
| src/main/java/com/project/dorumdorum/domain/chat/domain/service/ChatRoomService.java | Converted chat room deletion to soft delete + save. |
| src/main/java/com/project/dorumdorum/domain/chat/domain/service/ChatRoomMemberService.java | Converted chat member leave flow to soft delete + save. |
| src/main/java/com/project/dorumdorum/domain/chat/domain/repository/ChatRoomRepository.java | Replaced chat room hard delete with soft-delete update. |
| src/main/java/com/project/dorumdorum/domain/chat/domain/repository/ChatRoomMemberRepository.java | Replaced chat room member bulk delete with soft-delete update. |
| src/main/java/com/project/dorumdorum/domain/chat/domain/repository/ChatMessageRepository.java | Added flushAutomatically to delete path and switched to soft delete for messages. |
| src/main/java/com/project/dorumdorum/domain/chat/domain/entity/ChatRoomMember.java | Added SQL restriction and removed hard unique constraint mapping. |
| src/main/java/com/project/dorumdorum/domain/chat/domain/entity/ChatRoom.java | Added SQL restriction for soft-deleted chat rooms. |
| src/main/java/com/project/dorumdorum/domain/chat/domain/entity/ChatMessage.java | Added SQL restriction for soft-deleted chat messages. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+126
to
+127
| @Column(name = "deleted_at") | ||
| private LocalDateTime deletedAt; |
Comment on lines
+7
to
+28
| DROP INDEX IF EXISTS uk_roommate_user_no; | ||
| CREATE UNIQUE INDEX IF NOT EXISTS uk_roommate_user_no | ||
| ON roommate (user_no); | ||
| ON roommate (user_no) | ||
| WHERE deleted_at IS NULL; | ||
|
|
||
| DROP INDEX IF EXISTS uk_room_like_user_room; | ||
| CREATE UNIQUE INDEX IF NOT EXISTS uk_room_like_user_room | ||
| ON room_like (user_no, room_no); | ||
| ON room_like (user_no, room_no) | ||
| WHERE deleted_at IS NULL; | ||
|
|
||
| CREATE UNIQUE INDEX IF NOT EXISTS uk_device_user_device | ||
| ON devices (user_no, device_id); | ||
|
|
||
| DROP INDEX IF EXISTS uk_room_request_user_room_direction; | ||
| CREATE UNIQUE INDEX IF NOT EXISTS uk_room_request_user_room_direction | ||
| ON room_request (user_no, room_no, direction); | ||
| ON room_request (user_no, room_no, direction) | ||
| WHERE deleted_at IS NULL; | ||
|
|
||
| DROP INDEX IF EXISTS uk_chat_room_member_room_user; | ||
| CREATE UNIQUE INDEX IF NOT EXISTS uk_chat_room_member_room_user | ||
| ON chat_room_member (chat_room_no, user_no) | ||
| WHERE deleted_at IS NULL; |
Comment on lines
+58
to
+70
| * @ActiveProfiles("local-db"): 로컬 Docker PostgreSQL(dorumdorum_test DB)에 직접 연결. | ||
| * Testcontainers 없이 실제 DB로 동작을 검증한다. | ||
| */ | ||
| @SpringBootTest(properties = { | ||
| "spring.datasource.driver-class-name=org.postgresql.Driver", | ||
| "spring.datasource.url=jdbc:postgresql://localhost:5432/dorumdorum_test", | ||
| "spring.datasource.username=dorumdorum", | ||
| "spring.datasource.password=test1234", | ||
| "spring.jpa.defer-datasource-initialization=true", | ||
| "spring.jpa.hibernate.ddl-auto=create-drop", | ||
| "spring.sql.init.mode=always", | ||
| "spring.sql.init.schema-locations=classpath:schema.sql" | ||
| }) |
Comment on lines
66
to
+70
| public void leaveRoom(String userNo, String roomNo) { | ||
| Roommate roommate = roommateRepository.findByUserNoAndRoomNo(userNo, roomNo) | ||
| .orElseThrow(() -> new RestApiException(_NOT_FOUND)); | ||
| // uk_roommate_user_no 유니크 제약으로 인해 소프트 삭제 불가 — 하드 삭제 유지 | ||
| roommateRepository.delete(roommate); | ||
| roommate.delete(); | ||
| roommateRepository.save(roommate); |
| ON room_request (user_no, room_no, direction) | ||
| WHERE deleted_at IS NULL; | ||
|
|
||
| DROP INDEX IF EXISTS uk_chat_room_member_room_user; |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/com/project/dorumdorum/domain/checklist/domain/entity/RoomRule.java (1)
33-33:⚠️ Potential issue | 🟠 Major | ⚡ Quick win소프트 삭제와 충돌하는 일반 유니크 제약이 Partial Index로 교체되지 않았습니다.
@Index(unique = true)(Line 33)와@JoinColumn(unique = true)(Line 51) 두 곳 모두 일반 유니크 제약으로 남아 있습니다. PR 설명에서 "연관 유니크 제약은WHERE deleted_at IS NULL을 포함한 Partial Index로 교체함"이라고 명시했지만, 실제 코드에는 반영되지 않았습니다."Both deleted and live entities will share unique constraints and indexes. Hence, creating regular indexes will not work any longer."
소프트 삭제된
RoomRule행은room_no값을 그대로 유지하면서 테이블에 남아 있기 때문에, 같은 방에 대해 새로운RoomRule을 생성하려 하면 유니크 제약 위반이 발생합니다.ddl-auto=create-drop을 사용하는 통합 테스트 환경에서도 Hibernate가 일반 유니크 제약을 그대로 생성하므로 동일한 문제가 발생합니다.🛠️ 수정 방향 제안
1. 어노테이션에서
unique = true제거`@Index`(name = "idx_room_rule_updated_at", columnList = "updated_at"), - `@Index`(name = "uk_room_rule_room_no", columnList = "room_no", unique = true) + `@Index`(name = "idx_room_rule_room_no", columnList = "room_no")`@JoinColumn`( name = "room_no", nullable = false, - unique = true, foreignKey = `@ForeignKey`(name = "fk_room_rule_room") )2. DB 마이그레이션 스크립트(Flyway/Liquibase)에 Partial Index 추가
-- 기존 일반 유니크 제약 제거 후 Partial Index 생성 DROP INDEX IF EXISTS uk_room_rule_room_no; CREATE UNIQUE INDEX uk_room_rule_room_no ON room_rule(room_no) WHERE deleted_at IS NULL;Also applies to: 47-53
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/project/dorumdorum/domain/checklist/domain/entity/RoomRule.java` at line 33, The RoomRule entity still declares regular unique constraints which conflict with soft-delete: remove unique=true from the `@Index` on class RoomRule (the `@Index`(name = "uk_room_rule_room_no", columnList = "room_no", unique = true)) and from the `@JoinColumn`(unique = true) on the field referencing room_no so Hibernate does not create a normal UNIQUE constraint; then add a DB migration (Flyway/Liquibase) that drops the old uk_room_rule_room_no constraint if present and creates a partial unique index on room_rule(room_no) WHERE deleted_at IS NULL to enforce uniqueness only for non-deleted rows.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/main/java/com/project/dorumdorum/domain/checklist/domain/entity/ChecklistBase.java`:
- Around line 126-135: ChecklistBase currently reimplements the soft-delete
fields/methods (deletedAt, isDeleted(), delete()) already present in BaseEntity;
refactor by either making class ChecklistBase extend BaseEntity and removing the
duplicated deletedAt/isDeleted()/delete() members, or extract those members into
a new `@MappedSuperclass` (e.g., SoftDeletableEntity) and have ChecklistBase
extend that; ensure the `@Column`(name = "deleted_at") mapping and any
auditing/timezone handling are moved to the chosen superclass and remove the
duplicate definitions from ChecklistBase so only the superclass owns the
soft-delete behavior.
In
`@src/main/java/com/project/dorumdorum/domain/room/domain/entity/RoomLike.java`:
- Around line 15-18: RoomLike 엔티티의 `@Table` 어노테이션에 빈 indexes 속성이 남아 있어 불필요하게 보입니다;
RoomLike 클래스에서 `@Table`(...)을 찾아 indexes 속성이 비어있으면 해당 속성 전체를 제거해 어노테이션을 간결하게 하거나,
만약 DB 스키마에서 Partial Index를 별도 관리 중이라 유지해야 한다면 주석으로 이유를 명시해두세요.
In
`@src/test/java/com/project/dorumdorum/domain/room/integration/FlushRemovalIntegrationTest.java`:
- Around line 63-65: The test currently hardcodes DB credentials in the embedded
properties array inside FlushRemovalIntegrationTest (the
"spring.datasource.username=dorumdorum" and
"spring.datasource.password=test1234" entries); remove these plaintext values
and externalize them by one of the following: load from
application-test.properties placeholders (e.g.
"spring.datasource.username=${DB_USERNAME}" and
"spring.datasource.password=${DB_PASSWORD}") and set env vars in CI/local, or
inject values dynamically via Testcontainers or a `@DynamicPropertySource` method
in FlushRemovalIntegrationTest that registers the JDBC URL/username/password
programmatically from System.getenv() or the Testcontainer instance; update the
test setup to reference the new placeholders or dynamic properties instead of
the hardcoded strings.
- Around line 61-70: The current SpringBootTest properties hardcode
localhost:5432 causing CI failures; update FlushRemovalIntegrationTest to use
Testcontainers by annotating the class with `@Testcontainers`, adding a static
PostgreSQLContainer<?> (e.g., named postgresContainer) and a
`@DynamicPropertySource` method that registers spring.datasource.url,
spring.datasource.username, and spring.datasource.password from
postgresContainer.getJdbcUrl()/getUsername()/getPassword(); ensure you remove or
stop using the hardcoded properties block so the test spins up an isolated DB in
CI. As a short-term alternative, wrap the test class with
`@EnabledIfEnvironmentVariable` or `@EnabledIfSystemProperty` to skip the test when
no external Postgres is available.
- Around line 176-183: The test
delete_SoftDeletedAt_PersistedWithoutExplicitFlush currently uses
roomRepository.findById(...).orElseThrow() which could break silently if a
future `@SQLRestriction` hides soft-deleted rows; change the verification to query
the DB directly via jdbcTemplate (bypass JPA filters) after calling
deleteRoomUseCase.execute(DELETE_HOST_NO, deleteRoom.getRoomNo()) and assert the
deleted_at column is not null for the given room_no. Locate the test method
(delete_SoftDeletedAt_PersistedWithoutExplicitFlush) and replace the
roomRepository lookup with a jdbcTemplate query that selects deleted_at by
room_no and assert its non-nullness to avoid orElseThrow masking the intended
assertion.
---
Outside diff comments:
In
`@src/main/java/com/project/dorumdorum/domain/checklist/domain/entity/RoomRule.java`:
- Line 33: The RoomRule entity still declares regular unique constraints which
conflict with soft-delete: remove unique=true from the `@Index` on class RoomRule
(the `@Index`(name = "uk_room_rule_room_no", columnList = "room_no", unique =
true)) and from the `@JoinColumn`(unique = true) on the field referencing room_no
so Hibernate does not create a normal UNIQUE constraint; then add a DB migration
(Flyway/Liquibase) that drops the old uk_room_rule_room_no constraint if present
and creates a partial unique index on room_rule(room_no) WHERE deleted_at IS
NULL to enforce uniqueness only for non-deleted rows.
🪄 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: 2141d1bd-04da-414a-ac3c-4a40e262313c
📒 Files selected for processing (28)
src/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/schema.sqlsrc/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/FlushRemovalIntegrationTest.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.java
💤 Files with no reviewable changes (3)
- src/test/java/com/project/dorumdorum/domain/room/unit/usecase/KickRoommateUseCaseTest.java
- src/main/java/com/project/dorumdorum/domain/room/domain/service/RoomService.java
- src/main/java/com/project/dorumdorum/domain/room/application/usecase/KickRoommateUseCase.java
Comment on lines
+126
to
+135
| @Column(name = "deleted_at") | ||
| private LocalDateTime deletedAt; | ||
|
|
||
| public boolean isDeleted() { | ||
| return deletedAt != null; | ||
| } | ||
|
|
||
| public void delete() { | ||
| deletedAt = LocalDateTime.now(); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚖️ Poor tradeoff
BaseEntity와 동일한 소프트 삭제 패턴이 중복 구현되어 있습니다.
deletedAt, isDeleted(), delete()는 BaseEntity에 이미 동일하게 정의된 패턴입니다. ChecklistBase가 BaseEntity를 상속하지 않기 때문에 부득이하게 중복이 발생했지만, 향후 두 구현이 따로 관리되면 타임존 처리나 감사(Auditing) 로직 변경 시 한 쪽이 누락될 위험이 있습니다.
ChecklistBase가 BaseEntity를 상속하도록 리팩토링하거나, 소프트 삭제 로직을 별도의 @MappedSuperclass(예: SoftDeletableEntity)로 추출하는 방향을 장기적으로 검토해 보시면 좋겠습니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/com/project/dorumdorum/domain/checklist/domain/entity/ChecklistBase.java`
around lines 126 - 135, ChecklistBase currently reimplements the soft-delete
fields/methods (deletedAt, isDeleted(), delete()) already present in BaseEntity;
refactor by either making class ChecklistBase extend BaseEntity and removing the
duplicated deletedAt/isDeleted()/delete() members, or extract those members into
a new `@MappedSuperclass` (e.g., SoftDeletableEntity) and have ChecklistBase
extend that; ensure the `@Column`(name = "deleted_at") mapping and any
auditing/timezone handling are moved to the chosen superclass and remove the
duplicate definitions from ChecklistBase so only the superclass owns the
soft-delete behavior.
Comment on lines
15
to
18
| @Table( | ||
| uniqueConstraints = { | ||
| @UniqueConstraint(name = "uk_room_like_user_room", columnNames = {"user_no", "room_no"}) | ||
| }, | ||
| indexes = { | ||
| } | ||
| ) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | 💤 Low value
빈 indexes 블록 정리를 고려해 주세요.
현재 인덱스가 정의되어 있지 않다면 @Table 어노테이션에서 indexes 속성을 제거하여 코드를 간결하게 유지할 수 있습니다. 다만 스키마에서 Partial Index를 별도로 관리하고 있다면 현재 상태로 두셔도 무방합니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/com/project/dorumdorum/domain/room/domain/entity/RoomLike.java`
around lines 15 - 18, RoomLike 엔티티의 `@Table` 어노테이션에 빈 indexes 속성이 남아 있어 불필요하게
보입니다; RoomLike 클래스에서 `@Table`(...)을 찾아 indexes 속성이 비어있으면 해당 속성 전체를 제거해 어노테이션을 간결하게
하거나, 만약 DB 스키마에서 Partial Index를 별도 관리 중이라 유지해야 한다면 주석으로 이유를 명시해두세요.
Comment on lines
+176
to
+183
| void delete_SoftDeletedAt_PersistedWithoutExplicitFlush() { | ||
| deleteRoomUseCase.execute(DELETE_HOST_NO, deleteRoom.getRoomNo()); | ||
|
|
||
| Room persisted = roomRepository.findById(deleteRoom.getRoomNo()).orElseThrow(); | ||
| assertThat(persisted.getDeletedAt()) | ||
| .as("room.delete()가 flush 없이도 deletedAt이 DB에 반영되어야 한다") | ||
| .isNotNull(); | ||
| } |
There was a problem hiding this comment.
findById().orElseThrow() 호출이 향후 @SQLRestriction 추가 시 오해를 유발할 수 있습니다
현재 Room 엔티티에는 @SQLRestriction이 없으므로 소프트 삭제 후에도 findById가 정상적으로 Row를 반환합니다. 그러나 PR 방향성을 고려할 때 추후 Room에도 @SQLRestriction("deleted_at is null")이 추가될 경우, orElseThrow()가 NoSuchElementException을 던지고 의도한 assertion 메시지 없이 테스트가 실패하게 됩니다.
deletedAt 값을 검증할 때는 JPA 필터를 우회하는 jdbcTemplate을 사용하는 것이 더 안정적입니다.
🛡️ jdbcTemplate으로 직접 검증하는 방식
`@Test`
`@DisplayName`("방 삭제 후 room.deletedAt이 DB에 반영된다 (explicit flush 없이)")
void delete_SoftDeletedAt_PersistedWithoutExplicitFlush() {
deleteRoomUseCase.execute(DELETE_HOST_NO, deleteRoom.getRoomNo());
- Room persisted = roomRepository.findById(deleteRoom.getRoomNo()).orElseThrow();
- assertThat(persisted.getDeletedAt())
- .as("room.delete()가 flush 없이도 deletedAt이 DB에 반영되어야 한다")
- .isNotNull();
+ String deletedAt = jdbcTemplate.queryForObject(
+ "SELECT deleted_at FROM room WHERE room_no = ?",
+ String.class, deleteRoom.getRoomNo());
+ assertThat(deletedAt)
+ .as("room.delete()가 flush 없이도 deletedAt이 DB에 반영되어야 한다")
+ .isNotNull();
}📝 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
| void delete_SoftDeletedAt_PersistedWithoutExplicitFlush() { | |
| deleteRoomUseCase.execute(DELETE_HOST_NO, deleteRoom.getRoomNo()); | |
| Room persisted = roomRepository.findById(deleteRoom.getRoomNo()).orElseThrow(); | |
| assertThat(persisted.getDeletedAt()) | |
| .as("room.delete()가 flush 없이도 deletedAt이 DB에 반영되어야 한다") | |
| .isNotNull(); | |
| } | |
| `@Test` | |
| `@DisplayName`("방 삭제 후 room.deletedAt이 DB에 반영된다 (explicit flush 없이)") | |
| void delete_SoftDeletedAt_PersistedWithoutExplicitFlush() { | |
| deleteRoomUseCase.execute(DELETE_HOST_NO, deleteRoom.getRoomNo()); | |
| String deletedAt = jdbcTemplate.queryForObject( | |
| "SELECT deleted_at FROM room WHERE room_no = ?", | |
| String.class, deleteRoom.getRoomNo()); | |
| assertThat(deletedAt) | |
| .as("room.delete()가 flush 없이도 deletedAt이 DB에 반영되어야 한다") | |
| .isNotNull(); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/test/java/com/project/dorumdorum/domain/room/integration/FlushRemovalIntegrationTest.java`
around lines 176 - 183, The test
delete_SoftDeletedAt_PersistedWithoutExplicitFlush currently uses
roomRepository.findById(...).orElseThrow() which could break silently if a
future `@SQLRestriction` hides soft-deleted rows; change the verification to query
the DB directly via jdbcTemplate (bypass JPA filters) after calling
deleteRoomUseCase.execute(DELETE_HOST_NO, deleteRoom.getRoomNo()) and assert the
deleted_at column is not null for the given room_no. Locate the test method
(delete_SoftDeletedAt_PersistedWithoutExplicitFlush) and replace the
roomRepository lookup with a jdbcTemplate query that selects deleted_at by
room_no and assert its non-nullness to avoid orElseThrow masking the intended
assertion.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@src/main/resources/schema.sql`:
- Around line 9-30: The partial-index replacement for roommate, room_like,
room_request, and chat_room_member is unsafe because you only DROP CONSTRAINT IF
EXISTS (e.g. uk_roommate_user_no, uk_room_like_user_room,
uk_room_request_user_room_direction, uk_chat_room_member_room_user) but not DROP
INDEX IF EXISTS, which leaves an old full UNIQUE INDEX in place; update the
migration to mirror chat_room by also issuing DROP INDEX IF EXISTS for each
corresponding index name (uk_roommate_user_no, uk_room_like_user_room,
uk_room_request_user_room_direction, uk_chat_room_member_room_user) before
creating the new CREATE UNIQUE INDEX ... WHERE deleted_at IS NULL so that either
prior constraint or prior standalone index is removed.
In
`@src/test/java/com/project/dorumdorum/domain/chat/integration/ChatTransactionAtomicityIntegrationTest.java`:
- Around line 349-362: Multiple integration tests duplicate the same
truncateTables() implementation; extract it into a shared helper to avoid
repetitive maintenance. Create a common test base class (e.g.,
AbstractIntegrationTest) or a utility class (e.g., TestDbUtils) containing the
truncateTables() method and make it accessible (protected instance method on the
base class or a public static method on the util); then remove the duplicated
truncateTables() implementations from ChatTransactionAtomicityIntegrationTest,
KickRoommatePersistenceIntegrationTest, DeleteRoomPersistenceIntegrationTest,
and FlushRemovalIntegrationTest and call the shared method from each test class
(reference method name: truncateTables()).
🪄 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: d00a5483-6a3a-4c4a-af08-fe0b134e7408
📒 Files selected for processing (8)
.github/workflows/cicd.ymlbuild.gradlesrc/main/resources/schema.sqlsrc/test/java/com/project/dorumdorum/domain/chat/integration/ChatTransactionAtomicityIntegrationTest.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/roommate/unit/service/RoommateServiceTest.java
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() 메서드가 4개 통합 테스트 클래스에 그대로 복사되어 있습니다
ChatTransactionAtomicityIntegrationTest, KickRoommatePersistenceIntegrationTest, DeleteRoomPersistenceIntegrationTest, FlushRemovalIntegrationTest 모두 동일한 truncateTables() 구현을 갖고 있습니다. 테이블 목록이 변경될 때 4곳을 일일이 수정해야 하는 유지보수 부담이 생깁니다.
공통 부모 클래스 또는 유틸리티 클래스로 추출하시는 것을 권장드립니다.
♻️ 공통 부모 클래스 추출 예시
// 신규: src/test/java/com/project/dorumdorum/testsupport/IntegrationTestBase.java
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.jdbc.core.JdbcTemplate;
+
+public abstract class IntegrationTestBase {
+
+ `@Autowired`
+ protected JdbcTemplate jdbcTemplate;
+
+ protected void truncateTables() {
+ jdbcTemplate.execute("""
+ TRUNCATE TABLE
+ chat_message,
+ chat_room_member,
+ chat_room,
+ room_request,
+ room_rule,
+ room_like,
+ roommate,
+ room
+ RESTART IDENTITY CASCADE
+ """);
+ }
+}-class ChatTransactionAtomicityIntegrationTest {
+class ChatTransactionAtomicityIntegrationTest extends IntegrationTestBase {
- `@Autowired` private JdbcTemplate jdbcTemplate;
// ...
- 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, Multiple integration tests duplicate the same
truncateTables() implementation; extract it into a shared helper to avoid
repetitive maintenance. Create a common test base class (e.g.,
AbstractIntegrationTest) or a utility class (e.g., TestDbUtils) containing the
truncateTables() method and make it accessible (protected instance method on the
base class or a public static method on the util); then remove the duplicated
truncateTables() implementations from ChatTransactionAtomicityIntegrationTest,
KickRoommatePersistenceIntegrationTest, DeleteRoomPersistenceIntegrationTest,
and FlushRemovalIntegrationTest and call the shared method from each test class
(reference method name: truncateTables()).
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@src/main/resources/schema.sql`:
- Around line 47-58: The GROUP unique must be defensively dropped like DIRECT:
add a DROP CONSTRAINT IF EXISTS uk_chat_room_group before (or alongside) the
existing DROP INDEX IF EXISTS uk_chat_room_group in the chat_room migration SQL
so both constraint and index removal are handled (mirror the pattern used for
uk_chat_room_direct); update the block that references uk_chat_room_group to
include that DROP CONSTRAINT IF EXISTS statement to ensure consistency and
safety across environments.
🪄 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: d077b4f3-4110-474f-81ee-377d88a85f7b
📒 Files selected for processing (1)
src/main/resources/schema.sql
Comment on lines
47
to
+58
| ALTER TABLE IF EXISTS chat_room | ||
| DROP CONSTRAINT IF EXISTS uk_chat_room_direct; | ||
|
|
||
| DROP INDEX IF EXISTS uk_chat_room_group; | ||
| CREATE UNIQUE INDEX IF NOT EXISTS uk_chat_room_group | ||
| ON chat_room (room_no) | ||
| WHERE chat_room_type = 'GROUP'; | ||
| WHERE chat_room_type = 'GROUP' AND deleted_at IS NULL; | ||
|
|
||
| DROP INDEX IF EXISTS uk_chat_room_direct; | ||
| CREATE UNIQUE INDEX IF NOT EXISTS uk_chat_room_direct | ||
| ON chat_room (room_no, applicant_user_no) | ||
| WHERE chat_room_type = 'DIRECT'; | ||
| WHERE chat_room_type = 'DIRECT' AND deleted_at IS NULL; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | 💤 Low value
uk_chat_room_group도 방어적으로 DROP CONSTRAINT IF EXISTS를 함께 추가하는 것이 어떨까요?
라인 9-34에서 적용한 패턴(이전 리뷰 반영)과 비교했을 때, chat_room의 경우 uk_chat_room_direct는 라인 47-48에서 DROP CONSTRAINT, 라인 55에서 DROP INDEX로 양쪽 케이스를 모두 처리하지만, uk_chat_room_group은 라인 50의 DROP INDEX IF EXISTS만 존재합니다.
현재 코드베이스 기준으로는 GROUP 인덱스가 INDEX 형태로만 존재했을 가능성이 높지만, 과거 마이그레이션 이력이나 환경별 차이를 고려하면 direct와 동일하게 양쪽 모두 방어적으로 처리해두는 편이 일관성 측면에서도, 운영 안정성 측면에서도 더 좋을 것 같습니다.
🛡️ 제안 수정
ALTER TABLE IF EXISTS chat_room
- DROP CONSTRAINT IF EXISTS uk_chat_room_direct;
+ DROP CONSTRAINT IF EXISTS uk_chat_room_direct;
+ALTER TABLE IF EXISTS chat_room
+ DROP CONSTRAINT IF EXISTS uk_chat_room_group;
DROP INDEX IF EXISTS uk_chat_room_group;
CREATE UNIQUE INDEX IF NOT EXISTS uk_chat_room_group
ON chat_room (room_no)
WHERE chat_room_type = 'GROUP' AND deleted_at IS NULL;📝 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
| ALTER TABLE IF EXISTS chat_room | |
| DROP CONSTRAINT IF EXISTS uk_chat_room_direct; | |
| DROP INDEX IF EXISTS uk_chat_room_group; | |
| CREATE UNIQUE INDEX IF NOT EXISTS uk_chat_room_group | |
| ON chat_room (room_no) | |
| WHERE chat_room_type = 'GROUP'; | |
| WHERE chat_room_type = 'GROUP' AND deleted_at IS NULL; | |
| DROP INDEX IF EXISTS uk_chat_room_direct; | |
| CREATE UNIQUE INDEX IF NOT EXISTS uk_chat_room_direct | |
| ON chat_room (room_no, applicant_user_no) | |
| WHERE chat_room_type = 'DIRECT'; | |
| WHERE chat_room_type = 'DIRECT' AND deleted_at IS NULL; | |
| ALTER TABLE IF EXISTS chat_room | |
| DROP CONSTRAINT IF EXISTS uk_chat_room_direct; | |
| ALTER TABLE IF EXISTS chat_room | |
| DROP CONSTRAINT IF EXISTS uk_chat_room_group; | |
| DROP INDEX IF EXISTS uk_chat_room_group; | |
| CREATE UNIQUE INDEX IF NOT EXISTS uk_chat_room_group | |
| ON chat_room (room_no) | |
| WHERE chat_room_type = 'GROUP' AND deleted_at IS NULL; | |
| DROP INDEX IF EXISTS uk_chat_room_direct; | |
| CREATE UNIQUE INDEX IF NOT EXISTS uk_chat_room_direct | |
| ON chat_room (room_no, applicant_user_no) | |
| WHERE chat_room_type = 'DIRECT' AND deleted_at IS 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/main/resources/schema.sql` around lines 47 - 58, The GROUP unique must be
defensively dropped like DIRECT: add a DROP CONSTRAINT IF EXISTS
uk_chat_room_group before (or alongside) the existing DROP INDEX IF EXISTS
uk_chat_room_group in the chat_room migration SQL so both constraint and index
removal are handled (mirror the pattern used for uk_chat_room_direct); update
the block that references uk_chat_room_group to include that DROP CONSTRAINT IF
EXISTS statement to ensure consistency and safety across environments.
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
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.
📌 제목
📢 요약
Roommate,ChatRoomMember,RoomRequest,RoomLike,RoomRule,ChatRoom,ChatMessage등 하드 삭제(Hard Delete)로 처리되던 모든 엔티티를deletedAt기반 소프트 삭제(Soft Delete)로 전환@SQLRestriction("deleted_at is null")을 적용해 SELECT 쿼리에서 삭제된 레코드가 자동 필터링되도록 함WHERE deleted_at IS NULL조건을 포함한 Partial Index로 교체🔗 연관 이슈: Resolves #101
🚀 PR 유형
✅ PR 체크리스트
📜 상세 설명
1. 명시적
flush()제거기존에
flush()가 필요했던 이유KickRoommateUseCase와DeleteRoomUseCase에서 엔티티 변경(room.minusCurrentMate(),room.delete()) 후 바로@Modifying벌크 JPQL을 실행하는 구조였습니다.JPA의 기본
FlushMode.AUTO는 SELECT 직전에 동일 테이블이 연관된 경우에만 자동 flush합니다. 벌크 UPDATE/DELETE JPQL이 실행되는 시점에는 flush가 보장되지 않기 때문에, 1차 캐시에만 존재하는 변경 사항이 DB에 반영되지 않은 상태로 벌크 쿼리가 실행됩니다.이로 인해 발생하던 문제:
KickRoommateUseCaseroom.minusCurrentMate()가 캐시에만 존재하는 상태에서BEFORE_COMMIT리스너의decreaseUnreadCount(JPQL)가 실행되어 DB의 stale 값을 기준으로 처리됨DeleteRoomUseCaseroom.delete()(deletedAt 설정)가 캐시에만 존재하는 상태에서 이후 벌크 삭제 쿼리가 실행되어 정합성 깨질 수 있음이를 임시로 해결하기 위해 PR #93 에서 각 변경 직후
roomService.flush()를 명시적으로 호출했습니다.이번에 제거할 수 있었던 이유
@Modifying(flushAutomatically = true)은FlushMode.AUTO와 다르게, 쿼리 실행 전 EntityManager 전체를 무조건 flush합니다. 이를 활용해 flush 타이밍을 보장하는 구조로 전환했습니다.KickRoommateUseCase@TransactionalEventListener(BEFORE_COMMIT)내decreaseUnreadCount(@Modifying flushAutomatically=true)가 실행되기 전 EntityManager 전체 flush →minusCurrentMate()변경이 DB에 반영된 뒤 쿼리 실행DeleteRoomUseCase@Modifying(flushAutomatically=true)쿼리(deleteAllByRoom) 실행 시room.delete()포함 전체 flushRoomService.flush()메서드 자체도 함께 제거했습니다.검증은 Testcontainers 대신 로컬 Docker PostgreSQL(
dorumdorum_test)에 직접 연결하는 통합 테스트(FlushRemovalIntegrationTest)를 신규 작성해 수행했습니다. (Testcontainers 1.19.7 + Docker Desktop 환경에서 docker-java 클라이언트가 HTTP 400을 반환하는 호환성 문제로 Testcontainers를 사용하지 않았습니다.)2. 전역 Soft Delete 전환
하드 삭제로 처리되던 모든 엔티티 및 연산을
deletedAt기반 소프트 삭제로 전환했습니다.엔티티별 변경 사항
@SQLRestriction추가@UniqueConstraint제거Roommateuk_roommate_user_no WHERE deleted_at IS NULLChatRoomMemberuk_chat_room_member_room_user WHERE deleted_at IS NULLRoomRequestuk_room_request_user_room_direction WHERE deleted_at IS NULLRoomLikeuk_room_like_user_room WHERE deleted_at IS NULLRoomRuleChatRoomuk_chat_room_group/direct에AND deleted_at IS NULL조건 추가ChatMessageRoomRule은ChecklistBase(@MappedSuperclass)를 상속하며BaseEntity를 상속하지 않아deletedAt필드가 없었습니다.ChecklistBase에deletedAt필드와delete()메서드를 추가했습니다.서비스 레이어 변경 (개별 삭제)
// Before
roommateRepository.delete(roommate)
// After
roommate.delete()
roommateRepository.save(roommate)
동일 패턴을
RoommateService.leaveRoom(),ChatRoomMemberService.leave(),ChatRoomService.delete(),RoomRequestService.delete(),RoomRequestService.cancelJoinRequest()에 적용했습니다.Repository 레이어 변경 (벌크 삭제)
동일 패턴을 RoomLikeRepository, ChatRoomMemberRepository, ChatRoomRepository, ChatMessageRepository, RoomRuleRepository에 적용했습니다.
RoomLikeRepository.deleteByUserNoAndRoom은 Spring Data 파생 메서드(하드 삭제)에서 @Modifying @query 소프트 삭제로 교체했습니다.
3. @SQLRestriction 적용 효과
@SQLRestriction("deleted_at is null")은 해당 엔티티에 대한 모든 SELECT 쿼리에 AND deleted_at IS NULL 조건을 자동으로 추가합니다. 벌크 DML(UPDATE/DELETE JPQL)에는 적용되지 않으므로, 벌크 쿼리에는 조건을 명시적으로 추가했습니다.주의: Room 엔티티는 기존 커스텀 쿼리에 이미 deleted_at IS NULL 필터가 포함되어 있어 @SQLRestriction을 추가하지 않았습니다. (findById로 소프트 삭제된 Room을 직접 조회해야 하는 케이스가 존재하기 때문)
Summary by CodeRabbit
Refactor
Tests
Chores