[fix] 모임방 참여 api 5xx 에러 발생 해결 #331
Open
Conversation
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
- 데이터 정합성이 맞는 결과도 포함할 수 있도록 assert 코드 수정
- 모임방 참여 service 코드에 추가된 'RESOURCE_LOCKED exception throw' 를 커버하기 위해, HTTP 423 error도 캐치할 수 있도록 스크립트 수정
2 tasks
Walkthrough비관적 락(findByRoomIdForUpdate)과 Spring Retry( Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API
participant RoomJoinService
participant Repo as RoomJpaRepository
participant DB as Database
Note over RoomJoinService,Repo: 변경점: getByIdForUpdate + `@Retryable/`@Recover
Client->>API: POST /rooms/{id}/join
API->>RoomJoinService: changeJoinState(command)
alt 첫 시도
RoomJoinService->>Repo: findByRoomIdForUpdate(id)
Repo->>DB: SELECT ... FOR UPDATE (pessimistic write)
DB-->>Repo: row locked or lock timeout
alt row locked
Repo-->>RoomJoinService: Room entity
RoomJoinService->>DB: 변경 후 commit
RoomJoinService-->>API: 성공 응답 (2xx)
else lock timeout
Repo-->>RoomJoinService: LockTimeoutException
RoomJoinService->>RoomJoinService: Retry (exponential backoff)
par 재시도 흐름
RoomJoinService->>Repo: findByRoomIdForUpdate(id) (retry)
Repo->>DB: SELECT ... FOR UPDATE
DB-->>Repo: locked or timeout
end
alt retries exhausted
RoomJoinService-->>API: RESOURCE_LOCKED 예외 처리 -> 423
end
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
시
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
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 |
Test Results488 tests 488 ✅ 45s ⏱️ Results for commit 7bfe838. ♻️ This comment has been updated with latest results. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/test/java/konkuk/thip/common/persistence/JpaRepositoryMethodTest.java (2)
120-138: 테스트 검증 방식 개선 및 PR 목적과의 연관성 확인 필요이 테스트는 JPA flush 시 dirty 엔티티에 대한 UPDATE 쿼리 실행을 확인하는 교육적 목적의 테스트로 보입니다. 몇 가지 개선이 필요합니다:
검증 방식: 현재 주석으로만 기대 동작을 설명하고 있으나, 실제 UPDATE 쿼리 실행을 검증하는 assertion이 없습니다. 수동 로그 확인에 의존하는 테스트는 자동화된 CI/CD 파이프라인에서 회귀를 감지하지 못합니다.
PR 연관성: 이 PR의 핵심 목적은 비관적 락(x-lock)과 Spring Retry를 통한 모임방 참여 API의 동시성 문제 해결입니다. 그러나 이 테스트는 일반적인 JPA flush 동작을 다루고 있어, PR의 주요 변경사항(pessimistic locking, retry logic, lock timeout)과 직접적인 연관성이 보이지 않습니다.
제안:
//then em.flush(); + em.clear(); // 영속성 컨텍스트 초기화 + TestUser reloaded = testUserRepository.findById(id).orElseThrow(); + assertThat(reloaded.getNickname()).isEqualTo("김희용"); // UPDATE가 실제로 DB에 반영되었는지 검증 /** - * flush 시점에 영속성 컨텍스트 상에 dirty 상태인 엔티티에 대하여 UPDATE 쿼리가 발생하는 것 확인 + * flush 시점에 영속성 컨텍스트 상에 dirty 상태인 엔티티에 대하여 UPDATE 쿼리가 발생하고 DB에 반영됨을 검증 */또한, PR의 핵심 변경사항(비관적 락, 재시도 로직)에 대한 테스트가 누락된 것으로 보이는데, 이러한 테스트 추가가 필요한지 검토 부탁드립니다.
140-152: 테스트 검증 방식 개선 필요이 테스트도 마찬가지로 JPA flush 시 신규 엔티티에 대한 INSERT 쿼리 실행을 문서화하고 있지만, 실제 검증 로직이 없습니다.
제안:
//when //then em.flush(); + em.clear(); // 영속성 컨텍스트 초기화 + TestUser reloaded = testUserRepository.findById(u2.getUserId()).orElseThrow(); + assertThat(reloaded.getNickname()).isEqualTo("노성준"); // INSERT가 실제로 DB에 반영되었는지 검증 /** - * flush 시점에 영속성 컨텍스트 상에 새로 추가된 엔티티에 대하여 INSERT 쿼리가 발생하는 것 확인 + * flush 시점에 영속성 컨텍스트 상에 새로 추가된 엔티티에 대하여 INSERT 쿼리가 발생하고 DB에 반영됨을 검증 */이러한 assertion은 테스트의 자동화된 검증을 가능하게 하여, 향후 JPA 설정 변경이나 버전 업그레이드 시 회귀를 감지할 수 있습니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (11)
build.gradle(2 hunks)loadtest/room_join_load_test.js(1 hunks)src/main/java/konkuk/thip/common/exception/code/ErrorCode.java(1 hunks)src/main/java/konkuk/thip/config/RetryConfig.java(1 hunks)src/main/java/konkuk/thip/room/adapter/out/persistence/RoomCommandPersistenceAdapter.java(1 hunks)src/main/java/konkuk/thip/room/adapter/out/persistence/repository/RoomJpaRepository.java(2 hunks)src/main/java/konkuk/thip/room/application/port/out/RoomCommandPort.java(1 hunks)src/main/java/konkuk/thip/room/application/service/RoomJoinService.java(4 hunks)src/test/java/konkuk/thip/common/persistence/JpaRepositoryMethodTest.java(1 hunks)src/test/java/konkuk/thip/common/util/TestEntityFactory.java(1 hunks)src/test/java/konkuk/thip/room/concurrency/RoomJoinConcurrencyTest.java(1 hunks)
🧰 Additional context used
🧠 Learnings (8)
📓 Common learnings
Learnt from: seongjunnoh
Repo: THIP-TextHip/THIP-Server PR: 113
File: src/main/java/konkuk/thip/recentSearch/adapter/out/persistence/RecentSearchCommandPersistenceAdapter.java:38-44
Timestamp: 2025-07-30T14:05:04.945Z
Learning: seongjunnoh는 코드 최적화 제안에 대해 구체적인 기술적 근거와 효율성 차이를 이해하고 싶어하며, 성능 개선 방식에 대한 상세한 설명을 선호한다.
Learnt from: seongjunnoh
Repo: THIP-TextHip/THIP-Server PR: 93
File: src/main/java/konkuk/thip/room/adapter/out/persistence/RoomQueryPersistenceAdapter.java:49-114
Timestamp: 2025-07-28T16:44:31.224Z
Learning: seongjunnoh는 코드 중복 문제에 대한 리팩토링 제안을 적극적으로 수용하고 함수형 인터페이스를 활용한 해결책을 선호한다.
Learnt from: seongjunnoh
Repo: THIP-TextHip/THIP-Server PR: 285
File: src/main/java/konkuk/thip/room/adapter/out/jpa/RoomStatus.java:1-7
Timestamp: 2025-08-31T05:25:14.845Z
Learning: seongjunnoh는 enum 의존성에 대해 유연한 접근을 선호하며, 도메인→어댑터 레이어 참조와 같은 아키텍처 layering 원칙보다 실용적인 구현을 우선시한다.
Learnt from: seongjunnoh
Repo: THIP-TextHip/THIP-Server PR: 195
File: src/main/java/konkuk/thip/feed/application/mapper/FeedQueryMapper.java:0-0
Timestamp: 2025-08-13T05:22:32.287Z
Learning: seongjunnoh는 데이터 무결성과 중복 방지에 대한 고민이 깊으며, LinkedHashSet을 활용한 중복 제거와 순서 보장을 동시에 달성하는 솔루션을 선호한다.
Learnt from: seongjunnoh
Repo: THIP-TextHip/THIP-Server PR: 305
File: src/main/java/konkuk/thip/notification/adapter/out/persistence/repository/NotificationQueryRepository.java:9-13
Timestamp: 2025-09-17T06:40:49.863Z
Learning: seongjunnoh는 메서드 네이밍 시 구현 세부사항보다 비즈니스 의도를 반영하는 것을 선호하며, 미래 확장성과 아키텍처 변화를 고려한 설계 철학을 가지고 있다.
Learnt from: seongjunnoh
Repo: THIP-TextHip/THIP-Server PR: 180
File: src/main/java/konkuk/thip/room/application/service/AttendanceCheckCreateService.java:22-40
Timestamp: 2025-08-14T09:15:31.371Z
Learning: seongjunnoh는 기술적 완벽성보다 실제 비즈니스 시나리오와 사용자 행동 패턴을 우선 고려하며, 발생 가능성이 낮은 엣지 케이스보다는 실용적인 구현을 선호한다.
Learnt from: seongjunnoh
Repo: THIP-TextHip/THIP-Server PR: 278
File: src/main/java/konkuk/thip/common/exception/code/ErrorCode.java:210-211
Timestamp: 2025-08-24T09:40:48.595Z
Learning: seongjunnoh는 HTTP 상태 코드 선택에 대해 기술적 근거와 코드베이스 내 일관성을 중요하게 생각하며, 구체적인 사례 분석을 통한 설명을 선호한다.
Learnt from: seongjunnoh
Repo: THIP-TextHip/THIP-Server PR: 278
File: src/main/java/konkuk/thip/TestTokenController.java:0-0
Timestamp: 2025-08-24T09:33:52.982Z
Learning: seongjunnoh는 Spring의 ConditionalOnProperty 동작 원리를 정확히 이해하고 있으며, 보안 이슈에 대해서도 실질적인 위험성을 기준으로 판단하는 실용적 접근을 선호한다.
Learnt from: seongjunnoh
Repo: THIP-TextHip/THIP-Server PR: 112
File: src/main/java/konkuk/thip/feed/adapter/out/persistence/repository/FeedQueryRepositoryImpl.java:272-272
Timestamp: 2025-07-30T10:44:34.115Z
Learning: seongjunnoh는 피드 커서 페이지네이션에서 LocalDateTime 단일 커서 방식을 선호하며, 복합 키 기반 커서보다 구현 단순성과 성능을 우선시한다.
Learnt from: seongjunnoh
Repo: THIP-TextHip/THIP-Server PR: 166
File: src/main/java/konkuk/thip/book/adapter/out/persistence/BookCommandPersistenceAdapter.java:70-82
Timestamp: 2025-08-07T18:19:55.908Z
Learning: seongjunnoh는 Clean Architecture 원칙을 중시하며, 어댑터는 순수하게 저장/조회 기능만 담당하고 비즈니스 로직은 서비스 레이어에서 처리하는 것을 선호한다.
📚 Learning: 2025-07-29T08:11:23.599Z
Learnt from: seongjunnoh
Repo: THIP-TextHip/THIP-Server PR: 109
File: src/main/java/konkuk/thip/user/adapter/in/web/UserQueryController.java:37-46
Timestamp: 2025-07-29T08:11:23.599Z
Learning: THIP 프로젝트에서 ExceptionDescription 어노테이션은 비즈니스 로직에서 발생하는 커스텀 에러 코드가 있는 API에만 사용하며, bean validation만 수행하는 API에는 사용하지 않는다.
Applied to files:
src/main/java/konkuk/thip/common/exception/code/ErrorCode.java
📚 Learning: 2025-09-01T13:18:13.652Z
Learnt from: seongjunnoh
Repo: THIP-TextHip/THIP-Server PR: 287
File: src/main/java/konkuk/thip/user/adapter/out/persistence/repository/UserJpaRepository.java:8-14
Timestamp: 2025-09-01T13:18:13.652Z
Learning: seongjunnoh는 JpaRepository의 findById 메서드 재정의보다는 도메인별 명시적 메서드(findByUserId, findByRoomId 등)를 정의하여 Hibernate Filter 적용을 보장하는 방식을 선호하며, 이를 통해 더 안전하고 의도가 명확한 코드 구조를 구축한다.
Applied to files:
src/main/java/konkuk/thip/room/adapter/out/persistence/repository/RoomJpaRepository.javasrc/main/java/konkuk/thip/room/adapter/out/persistence/RoomCommandPersistenceAdapter.java
📚 Learning: 2025-09-13T06:46:25.082Z
Learnt from: seongjunnoh
Repo: THIP-TextHip/THIP-Server PR: 300
File: src/main/java/konkuk/thip/room/application/service/RoomJoinService.java:37-42
Timestamp: 2025-09-13T06:46:25.082Z
Learning: seongjunnoh 프로젝트에서 방 참여 상태 변경 API(RoomJoinService.changeJoinState)는 모집중인 방(RECRUITING)에서만 JOIN과 CANCEL이 모두 가능하며, 공통 검증에서 room.validateRoomRecruitExpired()를 사용하는 것이 올바른 구현이다.
Applied to files:
src/test/java/konkuk/thip/room/concurrency/RoomJoinConcurrencyTest.javasrc/main/java/konkuk/thip/room/application/service/RoomJoinService.java
📚 Learning: 2025-10-13T08:39:43.833Z
Learnt from: buzz0331
Repo: THIP-TextHip/THIP-Server PR: 323
File: build.gradle:102-104
Timestamp: 2025-10-13T08:39:43.833Z
Learning: Spring AI 1.0.0-M6에서 Google AI Gemini 전용 스타터가 빈 등록에 실패하는 경우, spring-ai-openai-spring-boot-starter를 사용하고 외부 설정(환경 변수 등)으로 spring.ai.openai.base-url을 Google의 OpenAI 호환 엔드포인트로, spring.ai.openai.api-key를 Google Cloud 액세스 토큰으로, spring.ai.openai.chat.options.model을 Gemini 모델명으로 지정하여 우회할 수 있습니다.
Applied to files:
build.gradle
📚 Learning: 2025-07-26T06:09:00.850Z
Learnt from: seongjunnoh
Repo: THIP-TextHip/THIP-Server PR: 101
File: src/main/java/konkuk/thip/comment/application/service/CommentCreateService.java:36-39
Timestamp: 2025-07-26T06:09:00.850Z
Learning: THIP 프로젝트에서는 "사용자가 방에 속하는지 검증" 로직을 RoomParticipantPolicy 도메인 서비스로 캡슐화하여 재사용성을 높이고 비즈니스 로직의 중복을 방지하는 방식을 선호한다.
Applied to files:
src/main/java/konkuk/thip/room/application/service/RoomJoinService.java
📚 Learning: 2025-09-05T04:11:01.456Z
Learnt from: buzz0331
Repo: THIP-TextHip/THIP-Server PR: 293
File: src/main/java/konkuk/thip/common/scheduler/RoomStateScheduler.java:20-0
Timestamp: 2025-09-05T04:11:01.456Z
Learning: RoomStateScheduler에서 changeRoomStateToExpired()와 changeRoomStateToProgress()는 각각 다른 트랜잭션에서 실행되며, 날짜 기반으로 서로 다른 방들을 대상으로 하므로 실행 순서가 바뀌어도 데이터 정합성에 문제가 없다.
Applied to files:
src/main/java/konkuk/thip/room/application/service/RoomJoinService.java
📚 Learning: 2025-07-03T03:05:05.031Z
Learnt from: seongjunnoh
Repo: THIP-TextHip/THIP-Server PR: 43
File: src/main/java/konkuk/thip/book/application/port/out/BookCommandPort.java:0-0
Timestamp: 2025-07-03T03:05:05.031Z
Learning: THIP 프로젝트에서는 CQRS Port 분리 시 다음 컨벤션을 따름: CommandPort에는 findByXXX를 통해 도메인 엔티티를 찾아오는 메서드를 추가하고, QueryPort에는 조회 API의 response에 해당하는 데이터들을 DB로부터 조회하는 메서드를 추가함.
Applied to files:
src/main/java/konkuk/thip/room/application/port/out/RoomCommandPort.java
🧬 Code graph analysis (2)
src/test/java/konkuk/thip/room/concurrency/RoomJoinConcurrencyTest.java (2)
src/test/java/konkuk/thip/common/util/TestEntityFactory.java (1)
TestEntityFactory(35-417)loadtest/room_join_load_test.js (3)
body(120-120)i(72-72)userIds(69-69)
src/main/java/konkuk/thip/room/application/service/RoomJoinService.java (3)
src/main/java/konkuk/thip/room/application/service/RoomShowRecruitingDetailViewService.java (1)
Service(20-82)src/main/java/konkuk/thip/room/application/service/RoomRecruitCloseService.java (1)
Service(17-64)src/main/java/konkuk/thip/room/application/service/RoomVerifyPasswordService.java (1)
Service(12-30)
Comment on lines
81
to
94
| mockMvc.perform(post("/rooms/{roomId}/join", room.getRoomId()) | ||
| .contentType("application/json") | ||
| .content(json) | ||
| .requestAttr("userId", userId)) | ||
| .andExpect(status().isOk()); | ||
| return 200; | ||
| } catch (AssertionError e) { | ||
| return 400; | ||
| } finally { | ||
| finish.countDown(); | ||
| } | ||
| })); | ||
| } | ||
|
|
There was a problem hiding this comment.
동시성 테스트가 5xx 재발을 검출하지 못합니다
andExpect(status().isOk())에서 5xx가 발생하면 AssertionError가 발생하는데, 현재 catch 블록이 이를 400으로 변환해버려 이후 검증이 실패를 감지하지 못합니다. 이렇게 되면 다시 5xx가 발생해도 테스트가 통과하여 회귀를 막을 수 없습니다. 응답 코드를 그대로 읽어 500 이상이면 즉시 실패하도록 바꿔 주세요. 예시는 아래와 같습니다.
- results.add(pool.submit(() -> {
+ results.add(pool.submit(() -> {
ready.countDown();
start.await();
- try {
- mockMvc.perform(post("/rooms/{roomId}/join", room.getRoomId())
- .contentType("application/json")
- .content(json)
- .requestAttr("userId", userId))
- .andExpect(status().isOk());
- return 200;
- } catch (AssertionError e) {
- return 400;
- } finally {
+ try {
+ int status = mockMvc.perform(post("/rooms/{roomId}/join", room.getRoomId())
+ .contentType("application/json")
+ .content(json)
+ .requestAttr("userId", userId))
+ .andReturn()
+ .getResponse()
+ .getStatus();
+ if (status >= 500) {
+ throw new AssertionError("방 참여 요청이 5xx로 응답했습니다. status=" + status);
+ }
+ return status;
+ } finally {
finish.countDown();
}
}));- Room 엔티티 조회하는 메서드 변경으로 인한 테스트 코드 수정
- 테스트 클래스의 트랜잭션 어노테이션 제거(수정된 모임방 참여 service 메서드가 매번 새로운 트랜잭션을 열기 떄문) - MySQL, H2 DB의 락 획득 방식 차이로 인해 기존 테스트 코드 주석 처리 - test yml에 TIMEOUT 값 명시적으로 설정해도 테스트 코드 제대로 동작하지 X - 일단 깨지는 테스트 메서드는 주석 처리 - 운영 환경과의 동일한 테스트를 위해 test DB를 MySQL로 변경하는게 좋아보임
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/test/java/konkuk/thip/room/application/service/RoomJoinServiceTest.java (1)
68-69: 주석 처리된 코드를 제거하세요.네 개의 테스트 메서드 모두에서 이전
findById()호출이 주석으로 남아있습니다. 이미getByIdForUpdate()로 변경이 완료되었으므로 주석 처리된 코드는 삭제하는 것이 좋습니다.다음 diff를 적용하여 주석 처리된 코드를 제거하세요:
-// given(roomCommandPort.findById(ROOM_ID)).willReturn(Optional.of(room)); given(roomCommandPort.getByIdForUpdate(ROOM_ID)).willReturn(room);위 패턴을 네 곳 모두에 적용해주세요.
Also applies to: 83-84, 115-116, 131-132
src/test/java/konkuk/thip/room/adapter/in/web/RoomJoinApiTest.java (1)
196-197: 주석 처리된em.flush()/em.clear()를 제거하세요.EntityManager의 flush와 clear 호출이 주석 처리되어 있습니다.
@Transactional제거로 인해 불필요해진 것으로 보이지만, 주석 처리된 코드는 제거하는 것이 좋습니다.만약 향후 필요하다면 버전 관리 시스템에서 복원할 수 있습니다.
-// em.flush(); -// em.clear();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
src/test/java/konkuk/thip/room/adapter/in/web/RoomJoinApiTest.java(7 hunks)src/test/java/konkuk/thip/room/application/service/RoomJoinServiceTest.java(4 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: seongjunnoh
Repo: THIP-TextHip/THIP-Server PR: 113
File: src/main/java/konkuk/thip/recentSearch/adapter/out/persistence/RecentSearchCommandPersistenceAdapter.java:38-44
Timestamp: 2025-07-30T14:05:04.945Z
Learning: seongjunnoh는 코드 최적화 제안에 대해 구체적인 기술적 근거와 효율성 차이를 이해하고 싶어하며, 성능 개선 방식에 대한 상세한 설명을 선호한다.
Learnt from: seongjunnoh
Repo: THIP-TextHip/THIP-Server PR: 93
File: src/main/java/konkuk/thip/room/adapter/out/persistence/RoomQueryPersistenceAdapter.java:49-114
Timestamp: 2025-07-28T16:44:31.224Z
Learning: seongjunnoh는 코드 중복 문제에 대한 리팩토링 제안을 적극적으로 수용하고 함수형 인터페이스를 활용한 해결책을 선호한다.
Learnt from: seongjunnoh
Repo: THIP-TextHip/THIP-Server PR: 285
File: src/main/java/konkuk/thip/room/adapter/out/jpa/RoomStatus.java:1-7
Timestamp: 2025-08-31T05:25:14.845Z
Learning: seongjunnoh는 enum 의존성에 대해 유연한 접근을 선호하며, 도메인→어댑터 레이어 참조와 같은 아키텍처 layering 원칙보다 실용적인 구현을 우선시한다.
Learnt from: seongjunnoh
Repo: THIP-TextHip/THIP-Server PR: 195
File: src/main/java/konkuk/thip/feed/application/mapper/FeedQueryMapper.java:0-0
Timestamp: 2025-08-13T05:22:32.287Z
Learning: seongjunnoh는 데이터 무결성과 중복 방지에 대한 고민이 깊으며, LinkedHashSet을 활용한 중복 제거와 순서 보장을 동시에 달성하는 솔루션을 선호한다.
Learnt from: seongjunnoh
Repo: THIP-TextHip/THIP-Server PR: 305
File: src/main/java/konkuk/thip/notification/adapter/out/persistence/repository/NotificationQueryRepository.java:9-13
Timestamp: 2025-09-17T06:40:49.863Z
Learning: seongjunnoh는 메서드 네이밍 시 구현 세부사항보다 비즈니스 의도를 반영하는 것을 선호하며, 미래 확장성과 아키텍처 변화를 고려한 설계 철학을 가지고 있다.
Learnt from: seongjunnoh
Repo: THIP-TextHip/THIP-Server PR: 180
File: src/main/java/konkuk/thip/room/application/service/AttendanceCheckCreateService.java:22-40
Timestamp: 2025-08-14T09:15:31.371Z
Learning: seongjunnoh는 기술적 완벽성보다 실제 비즈니스 시나리오와 사용자 행동 패턴을 우선 고려하며, 발생 가능성이 낮은 엣지 케이스보다는 실용적인 구현을 선호한다.
Learnt from: seongjunnoh
Repo: THIP-TextHip/THIP-Server PR: 278
File: src/main/java/konkuk/thip/TestTokenController.java:0-0
Timestamp: 2025-08-24T09:33:52.982Z
Learning: seongjunnoh는 Spring의 ConditionalOnProperty 동작 원리를 정확히 이해하고 있으며, 보안 이슈에 대해서도 실질적인 위험성을 기준으로 판단하는 실용적 접근을 선호한다.
Learnt from: seongjunnoh
Repo: THIP-TextHip/THIP-Server PR: 278
File: src/main/java/konkuk/thip/common/exception/code/ErrorCode.java:210-211
Timestamp: 2025-08-24T09:40:48.595Z
Learning: seongjunnoh는 HTTP 상태 코드 선택에 대해 기술적 근거와 코드베이스 내 일관성을 중요하게 생각하며, 구체적인 사례 분석을 통한 설명을 선호한다.
Learnt from: seongjunnoh
Repo: THIP-TextHip/THIP-Server PR: 112
File: src/main/java/konkuk/thip/feed/adapter/out/persistence/repository/FeedQueryRepositoryImpl.java:272-272
Timestamp: 2025-07-30T10:44:34.115Z
Learning: seongjunnoh는 피드 커서 페이지네이션에서 LocalDateTime 단일 커서 방식을 선호하며, 복합 키 기반 커서보다 구현 단순성과 성능을 우선시한다.
Learnt from: seongjunnoh
Repo: THIP-TextHip/THIP-Server PR: 298
File: src/main/java/konkuk/thip/notification/application/service/template/room/RoomVoteStartedTemplate.java:10-12
Timestamp: 2025-09-12T08:02:38.082Z
Learning: seongjunnoh는 Java Record 문법에 대해 정확한 이해를 가지고 있으며, 컴파일 에러에 대한 실질적인 검증을 통해 기술적 이슈를 판단한다.
📚 Learning: 2025-09-13T06:46:25.082Z
Learnt from: seongjunnoh
Repo: THIP-TextHip/THIP-Server PR: 300
File: src/main/java/konkuk/thip/room/application/service/RoomJoinService.java:37-42
Timestamp: 2025-09-13T06:46:25.082Z
Learning: seongjunnoh 프로젝트에서 방 참여 상태 변경 API(RoomJoinService.changeJoinState)는 모집중인 방(RECRUITING)에서만 JOIN과 CANCEL이 모두 가능하며, 공통 검증에서 room.validateRoomRecruitExpired()를 사용하는 것이 올바른 구현이다.
Applied to files:
src/test/java/konkuk/thip/room/application/service/RoomJoinServiceTest.javasrc/test/java/konkuk/thip/room/adapter/in/web/RoomJoinApiTest.java
📚 Learning: 2025-08-14T12:29:00.150Z
Learnt from: buzz0331
Repo: THIP-TextHip/THIP-Server PR: 183
File: src/main/java/konkuk/thip/roompost/application/service/helper/RoomProgressHelper.java:33-45
Timestamp: 2025-08-14T12:29:00.150Z
Learning: 방 생성 시 항상 HOST 역할의 RoomParticipant가 생성되며, RoomProgressHelper.updateUserAndRoomProgress() 메서드는 사용자가 해당 방의 참여자임이 사전 검증된 상황에서만 호출되므로 참여자 목록이 비어있을 수 없다.
Applied to files:
src/test/java/konkuk/thip/room/adapter/in/web/RoomJoinApiTest.java
🧬 Code graph analysis (1)
src/test/java/konkuk/thip/room/adapter/in/web/RoomJoinApiTest.java (1)
src/test/java/konkuk/thip/room/application/service/RoomJoinServiceTest.java (1)
DisplayName(29-144)
🔇 Additional comments (2)
src/test/java/konkuk/thip/room/adapter/in/web/RoomJoinApiTest.java (2)
67-71: 알림 서비스 모킹 처리가 적절합니다.
RoomNotificationOrchestrator를 MockitoBean으로 선언하고@BeforeEach에서 적절히 모킹 처리하여 테스트 중 부작용을 방지했습니다. 통합 테스트에서 외부 의존성을 격리하는 좋은 패턴입니다.
48-48:tearDown()메서드의 예외 처리 누락으로 인한 테스트 격리 문제를 해결하세요.
@Transactional주석 처리는 H2 비관적 락 타임아웃 문제에 대한 정당한 해결책입니다. 그러나 현재tearDown()메서드는 예외 처리가 없어서, 첫 번째deleteAllInBatch()실패 시 나머지 정리 작업이 실행되지 않아 테스트 데이터가 누적될 수 있습니다. 다음과 같이 개선하세요:@AfterEach void tearDown() { try { roomParticipantJpaRepository.deleteAllInBatch(); roomJpaRepository.deleteAllInBatch(); bookJpaRepository.deleteAllInBatch(); userJpaRepository.deleteAllInBatch(); } catch (Exception e) { // 로그 및 재시도 로직, 또는 각 저장소별 독립적 예외 처리 } }또한 주석 처리된 테스트(
joinRoom_alreadyParticipated, 줄 158-179)는 완전히 제거하거나 GitHub 이슈로 추적하여 나중에 해결할 수 있도록 하세요. 코드에 설명이 있지만 주석 처리된 테스트는 유지보수 부담이 됩니다.
src/test/java/konkuk/thip/room/adapter/in/web/RoomJoinApiTest.java
Outdated
Show resolved
Hide resolved
- 멀티쓰레드 환경에서 동시에 모임방 참여 요청을 보낼 때, 200 OK 응답 수와 DB 데이터 정합성을 확인하는게 목적이라 4xx, 5xx error의 구분은 X
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/test/java/konkuk/thip/room/concurrency/RoomJoinConcurrencyTest.java (1)
81-88: 5xx 에러 재발을 감지하지 못하는 구조가 여전히 유지됩니다이전 리뷰에서 지적된 문제가 해결되지 않았습니다.
andExpect(status().isOk())에서 5xx 응답 시 발생하는AssertionError를 catch하여 -1을 반환하면, 실제 5xx 에러가 발생해도 테스트가 실패하지 않습니다.비관적 락 적용 후 5xx 에러가 재발하는지 확인하려면, 응답 코드를 직접 읽어서 500 이상일 경우 즉시 테스트를 실패시켜야 합니다.
이전 리뷰에서 제안된 수정안을 적용하세요:
results.add(pool.submit(() -> { ready.countDown(); start.await(); try { - mockMvc.perform(post("/rooms/{roomId}/join", room.getRoomId()) + int status = mockMvc.perform(post("/rooms/{roomId}/join", room.getRoomId()) .contentType("application/json") .content(json) .requestAttr("userId", userId)) - .andExpect(status().isOk()); - return 200; - } catch (AssertionError e) { - return -1; // 응답이 200이 아닌 경우 (4xx, 5xx error 모두) + .andReturn() + .getResponse() + .getStatus(); + if (status >= 500) { + throw new AssertionError("방 참여 요청이 5xx로 응답했습니다. status=" + status); + } + return status; } finally { finish.countDown(); } }));
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/test/java/konkuk/thip/room/concurrency/RoomJoinConcurrencyTest.java(3 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: seongjunnoh
Repo: THIP-TextHip/THIP-Server PR: 113
File: src/main/java/konkuk/thip/recentSearch/adapter/out/persistence/RecentSearchCommandPersistenceAdapter.java:38-44
Timestamp: 2025-07-30T14:05:04.945Z
Learning: seongjunnoh는 코드 최적화 제안에 대해 구체적인 기술적 근거와 효율성 차이를 이해하고 싶어하며, 성능 개선 방식에 대한 상세한 설명을 선호한다.
Learnt from: seongjunnoh
Repo: THIP-TextHip/THIP-Server PR: 93
File: src/main/java/konkuk/thip/room/adapter/out/persistence/RoomQueryPersistenceAdapter.java:49-114
Timestamp: 2025-07-28T16:44:31.224Z
Learning: seongjunnoh는 코드 중복 문제에 대한 리팩토링 제안을 적극적으로 수용하고 함수형 인터페이스를 활용한 해결책을 선호한다.
Learnt from: seongjunnoh
Repo: THIP-TextHip/THIP-Server PR: 285
File: src/main/java/konkuk/thip/room/adapter/out/jpa/RoomStatus.java:1-7
Timestamp: 2025-08-31T05:25:14.845Z
Learning: seongjunnoh는 enum 의존성에 대해 유연한 접근을 선호하며, 도메인→어댑터 레이어 참조와 같은 아키텍처 layering 원칙보다 실용적인 구현을 우선시한다.
Learnt from: seongjunnoh
Repo: THIP-TextHip/THIP-Server PR: 195
File: src/main/java/konkuk/thip/feed/application/mapper/FeedQueryMapper.java:0-0
Timestamp: 2025-08-13T05:22:32.287Z
Learning: seongjunnoh는 데이터 무결성과 중복 방지에 대한 고민이 깊으며, LinkedHashSet을 활용한 중복 제거와 순서 보장을 동시에 달성하는 솔루션을 선호한다.
Learnt from: seongjunnoh
Repo: THIP-TextHip/THIP-Server PR: 305
File: src/main/java/konkuk/thip/notification/adapter/out/persistence/repository/NotificationQueryRepository.java:9-13
Timestamp: 2025-09-17T06:40:49.863Z
Learning: seongjunnoh는 메서드 네이밍 시 구현 세부사항보다 비즈니스 의도를 반영하는 것을 선호하며, 미래 확장성과 아키텍처 변화를 고려한 설계 철학을 가지고 있다.
Learnt from: seongjunnoh
Repo: THIP-TextHip/THIP-Server PR: 180
File: src/main/java/konkuk/thip/room/application/service/AttendanceCheckCreateService.java:22-40
Timestamp: 2025-08-14T09:15:31.371Z
Learning: seongjunnoh는 기술적 완벽성보다 실제 비즈니스 시나리오와 사용자 행동 패턴을 우선 고려하며, 발생 가능성이 낮은 엣지 케이스보다는 실용적인 구현을 선호한다.
Learnt from: seongjunnoh
Repo: THIP-TextHip/THIP-Server PR: 278
File: src/main/java/konkuk/thip/TestTokenController.java:0-0
Timestamp: 2025-08-24T09:33:52.982Z
Learning: seongjunnoh는 Spring의 ConditionalOnProperty 동작 원리를 정확히 이해하고 있으며, 보안 이슈에 대해서도 실질적인 위험성을 기준으로 판단하는 실용적 접근을 선호한다.
Learnt from: seongjunnoh
Repo: THIP-TextHip/THIP-Server PR: 278
File: src/main/java/konkuk/thip/common/exception/code/ErrorCode.java:210-211
Timestamp: 2025-08-24T09:40:48.595Z
Learning: seongjunnoh는 HTTP 상태 코드 선택에 대해 기술적 근거와 코드베이스 내 일관성을 중요하게 생각하며, 구체적인 사례 분석을 통한 설명을 선호한다.
Learnt from: seongjunnoh
Repo: THIP-TextHip/THIP-Server PR: 112
File: src/main/java/konkuk/thip/feed/adapter/out/persistence/repository/FeedQueryRepositoryImpl.java:272-272
Timestamp: 2025-07-30T10:44:34.115Z
Learning: seongjunnoh는 피드 커서 페이지네이션에서 LocalDateTime 단일 커서 방식을 선호하며, 복합 키 기반 커서보다 구현 단순성과 성능을 우선시한다.
Learnt from: seongjunnoh
Repo: THIP-TextHip/THIP-Server PR: 298
File: src/main/java/konkuk/thip/notification/application/service/template/room/RoomVoteStartedTemplate.java:10-12
Timestamp: 2025-09-12T08:02:38.082Z
Learning: seongjunnoh는 Java Record 문법에 대해 정확한 이해를 가지고 있으며, 컴파일 에러에 대한 실질적인 검증을 통해 기술적 이슈를 판단한다.
📚 Learning: 2025-09-13T06:46:25.082Z
Learnt from: seongjunnoh
Repo: THIP-TextHip/THIP-Server PR: 300
File: src/main/java/konkuk/thip/room/application/service/RoomJoinService.java:37-42
Timestamp: 2025-09-13T06:46:25.082Z
Learning: seongjunnoh 프로젝트에서 방 참여 상태 변경 API(RoomJoinService.changeJoinState)는 모집중인 방(RECRUITING)에서만 JOIN과 CANCEL이 모두 가능하며, 공통 검증에서 room.validateRoomRecruitExpired()를 사용하는 것이 올바른 구현이다.
Applied to files:
src/test/java/konkuk/thip/room/concurrency/RoomJoinConcurrencyTest.java
📚 Learning: 2025-09-01T13:18:13.652Z
Learnt from: seongjunnoh
Repo: THIP-TextHip/THIP-Server PR: 287
File: src/main/java/konkuk/thip/user/adapter/out/persistence/repository/UserJpaRepository.java:8-14
Timestamp: 2025-09-01T13:18:13.652Z
Learning: seongjunnoh는 JpaRepository의 findById 메서드 재정의보다는 도메인별 명시적 메서드(findByUserId, findByRoomId 등)를 정의하여 Hibernate Filter 적용을 보장하는 방식을 선호하며, 이를 통해 더 안전하고 의도가 명확한 코드 구조를 구축한다.
Applied to files:
src/test/java/konkuk/thip/room/concurrency/RoomJoinConcurrencyTest.java
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
Comment on lines
+135
to
+139
| assertThat(participantRows).isGreaterThanOrEqualTo(recruit); | ||
|
|
||
| // 2) memberCount가 실제 participants 수보다 작을 수 있음 | ||
| // memberCount 값은 Room 도메인 규칙에 의해 recruitCount를 초과하여 증가하지 않음 | ||
| assertThat(memberCountInRoom).isLessThan((int) participantRows); | ||
| assertThat(memberCountInRoom).isLessThanOrEqualTo((int) participantRows); |
There was a problem hiding this comment.
🧩 Analysis chain
비관적 락 적용 후 데이터 정합성 검증이 불충분합니다
PR 목표에서 "동시성 문제(데이터 정합성 불일치) 해결"을 명시했는데, 테스트 assertion은 여전히 데이터 불일치를 허용하고 있습니다:
participantRows >= recruit: participant 수가 recruit를 초과하는 것을 허용memberCountInRoom <= participantRows: memberCount와 participant 수의 불일치를 허용
비관적 락(select for update)이 제대로 작동한다면, Room 엔티티의 memberCount 증가와 RoomParticipant 엔티티 저장이 순차적으로 처리되어 데이터 정합성이 보장되어야 합니다.
다음 중 하나를 명확히 해주세요:
- 비관적 락이 데이터 정합성을 완전히 보장한다면, assertion을
assertEquals로 변경하여 엄격하게 검증 - HTTP 423 응답으로 인해 일부 요청이 실패할 수 있다면, 성공한 요청들에 대해서만 정합성 검증 (예:
okCount == participantRows - 1,memberCountInRoom == participantRows - 1)
현재 주석(lines 127-132)에서 "동기화 보장 X"라고 명시되어 있는데, 이것이 비관적 락 적용 이후에도 여전히 유효한지 확인이 필요합니다. 다음 스크립트로 실제 동작을 검증해보세요:
🏁 Script executed:
#!/bin/bash
# 비관적 락 적용 코드에서 실제 동작 확인
# RoomJoinService에서 select for update 패턴 확인
ast-grep --pattern $'class RoomJoinService {
$$$
getByIdForUpdate($$$) {
$$$
}
$$$
}'
# Room 엔티티 조회 시 락 적용 여부 확인
rg -nP --type=java -C5 'getByIdForUpdate|FOR UPDATE|@Lock'Length of output: 11341
추가 검증을 위해 RoomJoinConcurrencyTest의 전체 맥락을 확인하겠습니다.
[/run_scripts]
🏁 Script executed:
#!/bin/bash
# RoomJoinConcurrencyTest 전체 코드 확인 - 테스트 목표와 로직 파악
cat src/test/java/konkuk/thip/room/concurrency/RoomJoinConcurrencyTest.java | head -200Length of output: 6620
🏁 Script executed:
#!/bin/bash
# 동기화 보장 관련 주석 확인
rg -nP --type=java -B5 -A5 '동기화|synchroni|보장'Length of output: 13410
🏁 Script executed:
#!/bin/bash
# HTTP 423 재시도 로직 확인
rg -nP --type=java -C5 '423|LOCKED|retry|재시도'Length of output: 9134
추가로 Room 도메인의 memberCount 증가 로직과 실제 데이터 정합성을 확인하겠습니다.
[/run_scripts]
🏁 Script executed:
#!/bin/bash
# Room 도메인의 memberCount 증가 로직 확인
cat src/main/java/konkuk/thip/room/domain/Room.java | grep -A10 -B2 "increaseMemberCount\|memberCount"Length of output: 1728
🏁 Script executed:
#!/bin/bash
# RoomJoinService의 전체 changeJoinState 로직 확인
cat src/main/java/konkuk/thip/room/application/service/RoomJoinService.java | sed -n '37,80p'Length of output: 1733
🏁 Script executed:
#!/bin/bash
# RoomParticipant 저장과 Room memberCount 증가 순서 확인
ast-grep --pattern $'class RoomJoinService {
$$$
changeJoinState($$$) {
$$$
roomParticipantCommandPort.$_($_);
$$$
room.$_($_);
$$$
}
$$$
}'Length of output: 0
handleJoin/handleCancel 로직을 확인하겠습니다.
[/run_scripts]
🏁 Script executed:
#!/bin/bash
# handleJoin과 handleCancel 메서드 전체 확인
rg -nP --type=java -A20 'private void handleJoin|private void handleCancel'Length of output: 4063
🏁 Script executed:
#!/bin/bash
# RoomParticipantCommandPort의 save 구현 확인
cat src/main/java/konkuk/thip/room/application/port/out/RoomParticipantCommandPort.javaLength of output: 1043
🏁 Script executed:
#!/bin/bash
# RoomParticipant 저장 시점에 실제 DB INSERT가 발생하는지 확인
rg -nP --type=java -B3 -A8 '@Override.*save.*RoomParticipant|roomParticipantJpaRepository.save'Length of output: 107228
비관적 락이 적용되었지만 데이터 정합성이 완전히 보장되지 않습니다
현재 구현의 근본 문제:
-
실행 순서 (RoomJoinService.handleJoin):
roomParticipantCommandPort.save()→ RoomParticipant 저장room.increaseMemberCount()→ Room의 memberCount 증가
-
보호 범위의 한계:
- 비관적 락 (
@Lock(PESSIMISTIC_WRITE))은 Room 엔티티만 보호 - RoomParticipant 테이블은 보호 대상이 아님
- 두 엔티티 간 일관성을 보장하지 못함
- 비관적 락 (
-
동시성 문제 발생 경로:
- 스레드 A, B가 동시에 Room.memberCount < recruitCount 확인 통과
- A가 RoomParticipant INSERT 후 memberCount 증가 시도
- B도 동일 수행 → 경쟁 상태로 인해
checkJoinPossible()예외 발생 - InvalidStateException → 재시도 안 함 → HTTP 423 (락 타임아웃)
- 하지만 A의 RoomParticipant는 이미 DB에 저장됨
- 결과:
participantRows > memberCount불일치
-
테스트 검증의 문제:
// 현재 (약한 assertion)
assertThat(participantRows).isGreaterThanOrEqualTo(recruit); // 초과 허용
assertThat(memberCountInRoom).isLessThanOrEqualTo((int) participantRows); // 미달 허용다음과 같이 수정하세요:
- 성공한 요청만 검증:
okCount만큼만 정합성 확인 - 또는 memberCount 도메인 규칙을 비관적 락 범위에 포함
필수 수정 사항:
Line 131 주석 "동기화 보장 X"는 현재도 여전히 유효합니다. 비관적 락이 두 엔티티의 consistency를 보장하도록 범위를 확대하거나, 테스트 목표를 명확히 해주세요.
🤖 Prompt for AI Agents
In src/test/java/konkuk/thip/room/concurrency/RoomJoinConcurrencyTest.java
around lines 135 to 139, the test and comment reveal a race where only the Room
entity is pessimistically locked while RoomParticipant inserts are unprotected,
causing participantRows > memberCount; either change the test to assert
consistency only for successfully completed joins (verify only okCount entries
and that memberCount == okCount and participantRows == okCount) or change the
implementation so the pessimistic lock scope includes participant writes
(perform checkJoinPossible(), increment memberCount and persist participant
within the same PESSIMISTIC_WRITE-locked transaction or additionally lock the
participant rows/table and add retry on InvalidStateException) so the two
entities remain consistent.
Collaborator
Author
There was a problem hiding this comment.
@coderabbitai 현재 서비스 메서드에서 최초로 Room 엔티티를 조회할 때 x-lock을 획득하게 강제해서 동시성 이슈가 발생할 여지는 없지 않나?
그리고 커밋 메시지에 작성한 것처럼, 현재 테스트 메서드의 목적은 멀티쓰레드 환경으로 모임방 참여 요청이 동시에 몰리는 상황을 재현했을 때, 200 Ok 응답 개수와 DB에 기록된 데이터를 통해 데이터 정합성이 맞는지를 확인하기 위함이야
따라서 200 Ok가 아닌 다른 응답의 HTTP Status는 굳이 분류하지 않았어
There was a problem hiding this comment.
@seongjunnoh, 정확하게 이해하셨습니다! 제 이전 코멘트에서 혼란을 드린 점 사과드립니다.
비관적 락이 데이터 정합성을 보장하는 이유
말씀하신 대로, PESSIMISTIC_WRITE는 exclusive lock을 획득하여 데이터가 읽히거나 수정·삭제되는 것을 방지합니다. 현재 구현의 동작 방식은 다음과 같습니다:
-
스레드 A가
getByIdForUpdate()로 Room 엔티티에 x-lock 획득 -
스레드 A가 트랜잭션 내에서:
- RoomParticipant 저장
room.increaseMemberCount()호출 (성공)- Room 엔티티 업데이트
-
스레드 A가 커밋하고 락 해제
-
스레드 B가 x-lock 획득 대기 (최대 5초)
-
스레드 A 커밋 후 스레드 B가 락 획득 → 업데이트된 상태 조회
-
스레드 B가 트랜잭션 내에서:
- RoomParticipant 저장
checkJoinPossible()실패 (memberCount >= recruitCount)InvalidStateException발생
-
트랜잭션이 커밋 또는 롤백될 때 락이 해제됩니다 → 트랜잭션 전체 롤백
-
RoomParticipant 저장도 함께 롤백됨
결과: 데이터 정합성 보장 ✅
테스트 Assertion 변경 의도
// 변경 전: 불일치를 "예상"
assertThat(participantRows).isGreaterThan(recruit);
assertThat(memberCountInRoom).isLessThan((int) participantRows);
// 변경 후: 정합성 달성 "가능"하도록 허용
assertThat(participantRows).isGreaterThanOrEqualTo(recruit);
assertThat(memberCountInRoom).isLessThanOrEqualTo((int) participantRows);>=, <=로 변경한 것은:
- 비관적 락 적용 이전: 불일치가 필수적으로 발생 (
>,<) - 비관적 락 적용 이후: 완벽한 정합성 달성 가능 (
==케이스 허용)
테스트 목적이 "200 OK 응답과 DB 데이터 정합성 확인"이라는 말씀도 맞습니다. 다만 HTTP 423 (RESOURCE_LOCKED) 응답이 발생한 요청들은 DB에 아무 변경도 남기지 않으므로, 최종적으로는:
participantRows = 1 (HOST) + okCount (성공한 JOIN 요청)
memberCountInRoom = okCount
형태로 완벽한 정합성이 유지될 것으로 예상됩니다.
제 이전 분석이 틀렸던 부분을 정리하면, "RoomParticipant 테이블이 락으로 보호받지 못한다"고 했지만, 실제로는 트랜잭션 경계 안에서 함께 롤백되므로 결과적으로 양쪽 엔티티의 일관성이 보장됩니다. 혼란을 드려 죄송합니다! 🙏
/)_/)
(◕ᴥ◕) 정확한 이해에 박수!
/ >📚
🧠 Learnings used
Learnt from: seongjunnoh
Repo: THIP-TextHip/THIP-Server PR: 300
File: src/main/java/konkuk/thip/room/application/service/RoomJoinService.java:37-42
Timestamp: 2025-09-13T06:46:25.082Z
Learning: seongjunnoh 프로젝트에서 방 참여 상태 변경 API(RoomJoinService.changeJoinState)는 모집중인 방(RECRUITING)에서만 JOIN과 CANCEL이 모두 가능하며, 공통 검증에서 room.validateRoomRecruitExpired()를 사용하는 것이 올바른 구현이다.
Learnt from: buzz0331
Repo: THIP-TextHip/THIP-Server PR: 183
File: src/main/java/konkuk/thip/roompost/application/service/helper/RoomProgressHelper.java:33-45
Timestamp: 2025-08-14T12:29:00.150Z
Learning: 방 생성 시 항상 HOST 역할의 RoomParticipant가 생성되며, RoomProgressHelper.updateUserAndRoomProgress() 메서드는 사용자가 해당 방의 참여자임이 사전 검증된 상황에서만 호출되므로 참여자 목록이 비어있을 수 없다.
Learnt from: buzz0331
Repo: THIP-TextHip/THIP-Server PR: 293
File: src/main/java/konkuk/thip/common/scheduler/RoomStateScheduler.java:20-0
Timestamp: 2025-09-05T04:11:01.456Z
Learning: RoomStateScheduler의 changeRoomStateToExpired()와 changeRoomStateToProgress()는 서로 다른 날짜 조건으로 완전히 다른 방들을 대상으로 하므로(end_date < today vs start_date <= today AND end_date >= today), 비동기 실행 순서와 관계없이 데이터 정합성에 문제가 없다.
Learnt from: seongjunnoh
Repo: THIP-TextHip/THIP-Server PR: 113
File: src/main/java/konkuk/thip/recentSearch/adapter/out/persistence/RecentSearchCommandPersistenceAdapter.java:38-44
Timestamp: 2025-07-30T14:05:04.945Z
Learning: seongjunnoh는 코드 최적화 제안에 대해 구체적인 기술적 근거와 효율성 차이를 이해하고 싶어하며, 성능 개선 방식에 대한 상세한 설명을 선호한다.
Learnt from: seongjunnoh
Repo: THIP-TextHip/THIP-Server PR: 93
File: src/main/java/konkuk/thip/room/adapter/out/persistence/RoomQueryPersistenceAdapter.java:49-114
Timestamp: 2025-07-28T16:44:31.224Z
Learning: seongjunnoh는 코드 중복 문제에 대한 리팩토링 제안을 적극적으로 수용하고 함수형 인터페이스를 활용한 해결책을 선호한다.
Learnt from: seongjunnoh
Repo: THIP-TextHip/THIP-Server PR: 285
File: src/main/java/konkuk/thip/room/adapter/out/jpa/RoomStatus.java:1-7
Timestamp: 2025-08-31T05:25:14.845Z
Learning: seongjunnoh는 enum 의존성에 대해 유연한 접근을 선호하며, 도메인→어댑터 레이어 참조와 같은 아키텍처 layering 원칙보다 실용적인 구현을 우선시한다.
Learnt from: seongjunnoh
Repo: THIP-TextHip/THIP-Server PR: 305
File: src/main/java/konkuk/thip/notification/adapter/out/persistence/repository/NotificationQueryRepository.java:9-13
Timestamp: 2025-09-17T06:40:49.863Z
Learning: seongjunnoh는 메서드 네이밍 시 구현 세부사항보다 비즈니스 의도를 반영하는 것을 선호하며, 미래 확장성과 아키텍처 변화를 고려한 설계 철학을 가지고 있다.
Learnt from: seongjunnoh
Repo: THIP-TextHip/THIP-Server PR: 195
File: src/main/java/konkuk/thip/feed/application/mapper/FeedQueryMapper.java:0-0
Timestamp: 2025-08-13T05:22:32.287Z
Learning: seongjunnoh는 데이터 무결성과 중복 방지에 대한 고민이 깊으며, LinkedHashSet을 활용한 중복 제거와 순서 보장을 동시에 달성하는 솔루션을 선호한다.
Learnt from: seongjunnoh
Repo: THIP-TextHip/THIP-Server PR: 180
File: src/main/java/konkuk/thip/room/application/service/AttendanceCheckCreateService.java:22-40
Timestamp: 2025-08-14T09:15:31.371Z
Learning: seongjunnoh는 기술적 완벽성보다 실제 비즈니스 시나리오와 사용자 행동 패턴을 우선 고려하며, 발생 가능성이 낮은 엣지 케이스보다는 실용적인 구현을 선호한다.
Learnt from: seongjunnoh
Repo: THIP-TextHip/THIP-Server PR: 278
File: src/main/java/konkuk/thip/common/exception/code/ErrorCode.java:210-211
Timestamp: 2025-08-24T09:40:48.595Z
Learning: seongjunnoh는 HTTP 상태 코드 선택에 대해 기술적 근거와 코드베이스 내 일관성을 중요하게 생각하며, 구체적인 사례 분석을 통한 설명을 선호한다.
Learnt from: seongjunnoh
Repo: THIP-TextHip/THIP-Server PR: 278
File: src/main/java/konkuk/thip/TestTokenController.java:0-0
Timestamp: 2025-08-24T09:33:52.982Z
Learning: seongjunnoh는 Spring의 ConditionalOnProperty 동작 원리를 정확히 이해하고 있으며, 보안 이슈에 대해서도 실질적인 위험성을 기준으로 판단하는 실용적 접근을 선호한다.
Learnt from: seongjunnoh
Repo: THIP-TextHip/THIP-Server PR: 112
File: src/main/java/konkuk/thip/feed/adapter/out/persistence/repository/FeedQueryRepositoryImpl.java:272-272
Timestamp: 2025-07-30T10:44:34.115Z
Learning: seongjunnoh는 피드 커서 페이지네이션에서 LocalDateTime 단일 커서 방식을 선호하며, 복합 키 기반 커서보다 구현 단순성과 성능을 우선시한다.
Learnt from: seongjunnoh
Repo: THIP-TextHip/THIP-Server PR: 298
File: src/main/java/konkuk/thip/notification/application/service/template/room/RoomVoteStartedTemplate.java:10-12
Timestamp: 2025-09-12T08:02:38.082Z
Learning: seongjunnoh는 Java Record 문법에 대해 정확한 이해를 가지고 있으며, 컴파일 에러에 대한 실질적인 검증을 통해 기술적 이슈를 판단한다.
hd0rable
previously approved these changes
Nov 10, 2025
Member
hd0rable
left a comment
hd0rable
left a comment
There was a problem hiding this comment.
수고하셨습니닷!! 노션도 확인했습니다!! 단순궁금증 질문 남겨보았는데 확인 부탁드립니다!!
| // .orElseThrow(() -> new BusinessException(ErrorCode.USER_CANNOT_JOIN_OR_CANCEL)); | ||
|
|
||
| /** 락 타잉아웃 발생 포인트 **/ | ||
| Room room = roomCommandPort.getByIdForUpdate(roomJoinCommand.roomId()); |
Member
There was a problem hiding this comment.
p3: 존재하지 않는 방에대해 방참여/취소 요청에대한 예외를 기존에는 USER_CANNOT_JOIN_OR_CANCEL로 처리했었는데 수정된 코드에서는 ROOM_NOT_FOUND로 처리하고있는데 이렇게 수정하신 이유가 따로있나용??
Collaborator
Author
There was a problem hiding this comment.
Room 엔티티 조회 시 x-lock을 걸고 조회하게끔 수정하는데 집중하다보니, 기존 코드처럼 조회가 되지 않을 경우에 대한 고려를 깊게 하지는 않았습니다!
현재 작업 중인 브랜치에서 이 부분 또한 고려해보도록 하겠습니다!
| @Override | ||
| @Transactional | ||
| @Retryable( | ||
| noRetryFor = {InvalidStateException.class, BusinessException.class}, // 재시도 제외 예외 |
src/test/java/konkuk/thip/room/adapter/in/web/RoomJoinApiTest.java
Outdated
Show resolved
Hide resolved
buzz0331
previously approved these changes
Nov 11, 2025
Contributor
buzz0331
left a comment
buzz0331
left a comment
There was a problem hiding this comment.
너무 좋은 것 같습니다! 테스트 코드나 부하테스트 시나리오 의도가 명확해서 이해도 잘되고 잘 와닿는 것 같습니다~
src/test/java/konkuk/thip/room/adapter/in/web/RoomJoinApiTest.java
Outdated
Show resolved
Hide resolved
- InvalidException, BusinessException에 대한 @recover 메서드 추가 - @retryable 메서드 내에서 발생한 exception은 모두 @recover 설정해줘야함
- InvalidException, BusinessException에 대한 @recover 메서드 추가로 인해 테스트 코드 주석 해제
- 테스트 수행 결과의 400, 423 응답 구분
- (user, room) 쌍에 대해 unique 제약 추가
seongjunnoh
commented
Dec 29, 2025
Comment on lines
+88
to
+96
| @Recover | ||
| public RoomJoinResult recoverInvalidStateException(InvalidStateException e, RoomJoinCommand roomJoinCommand) { | ||
| throw e; | ||
| } | ||
|
|
||
| @Recover | ||
| public RoomJoinResult recoverBusinessException(BusinessException e, RoomJoinCommand roomJoinCommand) { | ||
| throw e; | ||
| } |
Collaborator
Author
There was a problem hiding this comment.
@buzz0331 @hd0rable 이전에 방 참여 API 실행 중 도메인 로직으로 인해 발생한 InvalidStateException, BusinessException 이 @recover 메서드로 흡수되어 모두 423 error로 응답되었던 이슈를 해결했습니다
spring @retryable 로 정의한 메서드내에서 발생하는 exception을 재시도 여부와 관계없이 각 exception에 대한 @recover 메서드로 exception handling 이 필요하다고 하네요
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
loadtest/room_join_load_test.js (1)
109-112: Line 111의 체크가 모든 4xx를 수용하지만, 404(방 없음) 응답이 실제로 발생 가능하므로 별도 추적이 필요합니다.
getByIdForUpdate에서 방을 찾을 수 없을 때EntityNotFoundException(ROOM_NOT_FOUND)을 던지며, 이는 404 NOT_FOUND로 응답됩니다. 현재는 400과 423만 추적 중이므로 404도 별도 메트릭(예:http404)으로 기록하거나, 체크 조건을 더 명확하게 제한하는 것이 좋습니다.
♻️ Duplicate comments (1)
src/test/java/konkuk/thip/room/adapter/in/web/RoomJoinApiTest.java (1)
48-48: 테스트 격리를 위해 트랜잭션을 유지하고 락 동작을 선택적으로 제어하세요.
@Transactional을 주석 처리하면 테스트 간 데이터 격리가 깨집니다:
@AfterEach의deleteAllInBatch()로 정리하더라도, 테스트 실행 중 다른 테스트에 영향을 줄 수 있습니다- 트랜잭션 롤백이 없어 테스트 실패 시 데이터가 남을 수 있습니다
대안:
- H2의 락 타임아웃 동작 차이가 문제라면, 락 관련 테스트만
@Transactional(propagation = Propagation.NOT_SUPPORTED)를 사용하고 나머지는 트랜잭션을 유지하세요- 또는 과거 리뷰에서 논의된 것처럼 TestContainers로 실제 MySQL 환경에서 테스트하는 것을 권장합니다
Based on learnings: 팀에서 TestContainers 도입을 긍정적으로 검토 중인 것으로 보이며, 이것이 근본적인 해결책입니다.
🧹 Nitpick comments (4)
src/test/java/konkuk/thip/room/adapter/in/web/RoomPlayingOrExpiredDetailViewApiTest.java (1)
151-156: 변경 사항 승인: 엔티티 재생성에서 인플레이스 업데이트로 개선되었습니다.이전의 삭제 후 재생성 방식에서 기존 엔티티를 직접 업데이트하는 방식으로 변경되어 더 효율적이고 명확한 테스트 데이터 설정이 가능해졌습니다.
flush()와em.clear()조합으로 변경사항이 데이터베이스에 반영되고 영속성 컨텍스트가 초기화되어 테스트 격리성이 보장됩니다.선택적 제안: 공통 패턴을 헬퍼 메서드로 추출
6개의 테스트 메서드에서 동일한 패턴이 반복됩니다. 가독성과 유지보수성 향상을 위해 헬퍼 메서드로 추출할 수 있습니다:
private RoomParticipantJpaEntity setupParticipantActivity( RoomJpaEntity room, int currentPage, double userPercentage) { RoomParticipantJpaEntity participant = roomParticipantJpaRepository.findAllByRoomId(room.getRoomId()).get(0); participant.updateCurrentPage(currentPage); participant.updateUserPercentage(userPercentage); roomParticipantJpaRepository.flush(); em.clear(); return participant; } private RoomParticipantJpaEntity setupHostActivity( RoomJpaEntity room, int currentPage, double userPercentage) { RoomParticipantJpaEntity participant = setupParticipantActivity(room, currentPage, userPercentage); // flush/clear 이후이므로 다시 조회 필요 participant = roomParticipantJpaRepository.findAllByRoomId(room.getRoomId()).get(0); participant.updateRoleToHost(); roomParticipantJpaRepository.flush(); em.clear(); return roomParticipantJpaRepository.findAllByRoomId(room.getRoomId()).get(0); }사용 예시:
RoomParticipantJpaEntity joiningMember = setupParticipantActivity(room, 50, 10.6);Also applies to: 197-201, 242-247, 289-293, 312-316, 361-365
loadtest/room_join_load_test.js (1)
102-106: 응답 처리 로직의 효율성 개선을 고려할 수 있습니다.현재 구조는 정확하지만, 범위 비교를 중복으로 수행합니다. 다만, VU당 단일 API 호출이므로 성능 영향은 미미합니다.
🔎 선택적 리팩터링 제안
- if (res.status >= 200 && res.status < 300) http2xx.add(1); - else if (res.status >= 400 && res.status < 500) { - if (res.status === 400) http400.add(1); - else if (res.status === 423) http423.add(1); - } - else if (res.status >= 500) http5xx.add(1); + if (res.status >= 500) { + http5xx.add(1); + } else if (res.status === 423) { + http423.add(1); + } else if (res.status === 400) { + http400.add(1); + } else if (res.status >= 200 && res.status < 300) { + http2xx.add(1); + }이 방식은 특정 값 비교를 먼저 수행하여 범위 체크 횟수를 줄입니다.
src/main/java/konkuk/thip/room/application/service/RoomJoinService.java (2)
83-96: 재시도 제외 예외를 위한 복구 메서드는 호출되지 않아 불필요합니다.
noRetryFor에 명시된InvalidStateException,BusinessException은 재시도되지 않으므로, 이들을 위한@Recover메서드는 Spring Retry에 의해 호출되지 않습니다:
- Lines 88-91:
recoverInvalidStateException- Lines 93-96:
recoverBusinessException이 메서드들은 단순히 예외를 재발생시킬 뿐이며,
noRetryFor에 의해 이미 즉시 예외가 발생하므로 중복입니다.권장사항:
- 이 두 메서드를 제거하세요
- 또는 과거 리뷰 코멘트에서 논의된 것처럼, 모든 예외가
RESOURCE_LOCKED로 변환되는 것을 방지하려는 의도라면 주석으로 명확히 설명하세요🔎 불필요한 복구 메서드 제거
- @Recover - public RoomJoinResult recoverInvalidStateException(InvalidStateException e, RoomJoinCommand roomJoinCommand) { - throw e; - } - - @Recover - public RoomJoinResult recoverBusinessException(BusinessException e, RoomJoinCommand roomJoinCommand) { - throw e; - }
60-60: 비관적 락을 통한 동시성 제어가 적용되었습니다.
getByIdForUpdate()로SELECT ... FOR UPDATE패턴의 비관적 락이 적용되어 동시 참여 요청의 데이터 정합성을 보장합니다.다만 PR 설명에서 언급한 것처럼, 모든 스레드가 락 획득을 대기하는 구조의 성능 영향(p95 지연 증가)을 모니터링하고, 제안하신 낙관적 락 + 이벤트 기반 아키텍처로의 개선을 검토하시길 권장합니다.
Based on learnings: seongjunnoh님은 성능 개선에 대한 구체적인 기술적 근거를 선호하시므로, 추후 낙관적 락 전환 시 처리량(TPS) 개선 수치와 함께 논의하시면 좋을 것 같습니다.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (7)
loadtest/room_join_load_test.jssrc/main/java/konkuk/thip/room/adapter/out/jpa/RoomParticipantJpaEntity.javasrc/main/java/konkuk/thip/room/application/service/RoomJoinService.javasrc/main/resources/db/migration/V251229__Add_unique_room_participants.sqlsrc/test/java/konkuk/thip/room/adapter/in/web/RoomJoinApiTest.javasrc/test/java/konkuk/thip/room/adapter/in/web/RoomPlayingOrExpiredDetailViewApiTest.javasrc/test/java/konkuk/thip/room/adapter/in/web/RoomRecruitingDetailViewApiTest.java
🧰 Additional context used
🧠 Learnings (9)
📓 Common learnings
Learnt from: seongjunnoh
Repo: THIP-TextHip/THIP-Server PR: 113
File: src/main/java/konkuk/thip/recentSearch/adapter/out/persistence/RecentSearchCommandPersistenceAdapter.java:38-44
Timestamp: 2025-07-30T14:05:04.945Z
Learning: seongjunnoh는 코드 최적화 제안에 대해 구체적인 기술적 근거와 효율성 차이를 이해하고 싶어하며, 성능 개선 방식에 대한 상세한 설명을 선호한다.
Learnt from: seongjunnoh
Repo: THIP-TextHip/THIP-Server PR: 93
File: src/main/java/konkuk/thip/room/adapter/out/persistence/RoomQueryPersistenceAdapter.java:49-114
Timestamp: 2025-07-28T16:44:31.224Z
Learning: seongjunnoh는 코드 중복 문제에 대한 리팩토링 제안을 적극적으로 수용하고 함수형 인터페이스를 활용한 해결책을 선호한다.
Learnt from: seongjunnoh
Repo: THIP-TextHip/THIP-Server PR: 285
File: src/main/java/konkuk/thip/room/adapter/out/jpa/RoomStatus.java:1-7
Timestamp: 2025-08-31T05:25:14.845Z
Learning: seongjunnoh는 enum 의존성에 대해 유연한 접근을 선호하며, 도메인→어댑터 레이어 참조와 같은 아키텍처 layering 원칙보다 실용적인 구현을 우선시한다.
Learnt from: seongjunnoh
Repo: THIP-TextHip/THIP-Server PR: 195
File: src/main/java/konkuk/thip/feed/application/mapper/FeedQueryMapper.java:0-0
Timestamp: 2025-08-13T05:22:32.287Z
Learning: seongjunnoh는 데이터 무결성과 중복 방지에 대한 고민이 깊으며, LinkedHashSet을 활용한 중복 제거와 순서 보장을 동시에 달성하는 솔루션을 선호한다.
Learnt from: seongjunnoh
Repo: THIP-TextHip/THIP-Server PR: 305
File: src/main/java/konkuk/thip/notification/adapter/out/persistence/repository/NotificationQueryRepository.java:9-13
Timestamp: 2025-09-17T06:40:49.863Z
Learning: seongjunnoh는 메서드 네이밍 시 구현 세부사항보다 비즈니스 의도를 반영하는 것을 선호하며, 미래 확장성과 아키텍처 변화를 고려한 설계 철학을 가지고 있다.
Learnt from: seongjunnoh
Repo: THIP-TextHip/THIP-Server PR: 278
File: src/main/java/konkuk/thip/TestTokenController.java:0-0
Timestamp: 2025-08-24T09:33:52.982Z
Learning: seongjunnoh는 Spring의 ConditionalOnProperty 동작 원리를 정확히 이해하고 있으며, 보안 이슈에 대해서도 실질적인 위험성을 기준으로 판단하는 실용적 접근을 선호한다.
Learnt from: seongjunnoh
Repo: THIP-TextHip/THIP-Server PR: 278
File: src/main/java/konkuk/thip/common/exception/code/ErrorCode.java:210-211
Timestamp: 2025-08-24T09:40:48.595Z
Learning: seongjunnoh는 HTTP 상태 코드 선택에 대해 기술적 근거와 코드베이스 내 일관성을 중요하게 생각하며, 구체적인 사례 분석을 통한 설명을 선호한다.
Learnt from: seongjunnoh
Repo: THIP-TextHip/THIP-Server PR: 180
File: src/main/java/konkuk/thip/room/application/service/AttendanceCheckCreateService.java:22-40
Timestamp: 2025-08-14T09:15:31.371Z
Learning: seongjunnoh는 기술적 완벽성보다 실제 비즈니스 시나리오와 사용자 행동 패턴을 우선 고려하며, 발생 가능성이 낮은 엣지 케이스보다는 실용적인 구현을 선호한다.
Learnt from: seongjunnoh
Repo: THIP-TextHip/THIP-Server PR: 112
File: src/main/java/konkuk/thip/feed/adapter/out/persistence/repository/FeedQueryRepositoryImpl.java:272-272
Timestamp: 2025-07-30T10:44:34.115Z
Learning: seongjunnoh는 피드 커서 페이지네이션에서 LocalDateTime 단일 커서 방식을 선호하며, 복합 키 기반 커서보다 구현 단순성과 성능을 우선시한다.
Learnt from: seongjunnoh
Repo: THIP-TextHip/THIP-Server PR: 278
File: src/main/java/konkuk/thip/TestTokenController.java:0-0
Timestamp: 2025-08-24T09:36:25.323Z
Learning: seongjunnoh는 Spring의 조건부 빈 등록(ConditionalOnProperty) 메커니즘을 정확히 이해하고 있으며, 이미 조건부 등록으로 기능이 보장되는 상황에서 중복적인 어노테이션(Hidden 등)을 추가하는 것보다 실용적이고 간결한 구현을 선호한다.
📚 Learning: 2025-08-14T12:29:00.150Z
Learnt from: buzz0331
Repo: THIP-TextHip/THIP-Server PR: 183
File: src/main/java/konkuk/thip/roompost/application/service/helper/RoomProgressHelper.java:33-45
Timestamp: 2025-08-14T12:29:00.150Z
Learning: 방 생성 시 항상 HOST 역할의 RoomParticipant가 생성되며, RoomProgressHelper.updateUserAndRoomProgress() 메서드는 사용자가 해당 방의 참여자임이 사전 검증된 상황에서만 호출되므로 참여자 목록이 비어있을 수 없다.
Applied to files:
src/test/java/konkuk/thip/room/adapter/in/web/RoomRecruitingDetailViewApiTest.javasrc/test/java/konkuk/thip/room/adapter/in/web/RoomPlayingOrExpiredDetailViewApiTest.javasrc/test/java/konkuk/thip/room/adapter/in/web/RoomJoinApiTest.javasrc/main/java/konkuk/thip/room/application/service/RoomJoinService.java
📚 Learning: 2025-09-13T06:46:25.082Z
Learnt from: seongjunnoh
Repo: THIP-TextHip/THIP-Server PR: 300
File: src/main/java/konkuk/thip/room/application/service/RoomJoinService.java:37-42
Timestamp: 2025-09-13T06:46:25.082Z
Learning: seongjunnoh 프로젝트에서 방 참여 상태 변경 API(RoomJoinService.changeJoinState)는 모집중인 방(RECRUITING)에서만 JOIN과 CANCEL이 모두 가능하며, 공통 검증에서 room.validateRoomRecruitExpired()를 사용하는 것이 올바른 구현이다.
Applied to files:
src/test/java/konkuk/thip/room/adapter/in/web/RoomRecruitingDetailViewApiTest.javasrc/test/java/konkuk/thip/room/adapter/in/web/RoomPlayingOrExpiredDetailViewApiTest.javasrc/test/java/konkuk/thip/room/adapter/in/web/RoomJoinApiTest.javasrc/main/java/konkuk/thip/room/application/service/RoomJoinService.java
📚 Learning: 2025-07-26T06:09:00.850Z
Learnt from: seongjunnoh
Repo: THIP-TextHip/THIP-Server PR: 101
File: src/main/java/konkuk/thip/comment/application/service/CommentCreateService.java:36-39
Timestamp: 2025-07-26T06:09:00.850Z
Learning: THIP 프로젝트에서는 "사용자가 방에 속하는지 검증" 로직을 RoomParticipantPolicy 도메인 서비스로 캡슐화하여 재사용성을 높이고 비즈니스 로직의 중복을 방지하는 방식을 선호한다.
Applied to files:
src/test/java/konkuk/thip/room/adapter/in/web/RoomRecruitingDetailViewApiTest.javasrc/test/java/konkuk/thip/room/adapter/in/web/RoomPlayingOrExpiredDetailViewApiTest.javasrc/main/java/konkuk/thip/room/application/service/RoomJoinService.java
📚 Learning: 2025-08-14T07:19:27.310Z
Learnt from: buzz0331
Repo: THIP-TextHip/THIP-Server PR: 180
File: src/main/java/konkuk/thip/room/application/port/out/AttendanceCheckQueryPort.java:5-5
Timestamp: 2025-08-14T07:19:27.310Z
Learning: AttendanceCheck 엔티티는 BaseJpaEntity를 상속받아 status 필드(StatusType.ACTIVE/INACTIVE/EXPIRED)로 소프트 딜리트를 구현하므로, 카운트 쿼리에서 status = 'ACTIVE' 조건을 포함해야 삭제된 레코드를 제외할 수 있음.
Applied to files:
src/main/java/konkuk/thip/room/adapter/out/jpa/RoomParticipantJpaEntity.java
📚 Learning: 2025-09-05T04:11:01.456Z
Learnt from: buzz0331
Repo: THIP-TextHip/THIP-Server PR: 293
File: src/main/java/konkuk/thip/common/scheduler/RoomStateScheduler.java:20-0
Timestamp: 2025-09-05T04:11:01.456Z
Learning: RoomStateScheduler에서 changeRoomStateToExpired()와 changeRoomStateToProgress()는 각각 다른 트랜잭션에서 실행되며, 날짜 기반으로 서로 다른 방들을 대상으로 하므로 실행 순서가 바뀌어도 데이터 정합성에 문제가 없다.
Applied to files:
src/test/java/konkuk/thip/room/adapter/in/web/RoomPlayingOrExpiredDetailViewApiTest.javasrc/main/java/konkuk/thip/room/application/service/RoomJoinService.java
📚 Learning: 2025-07-26T06:09:00.850Z
Learnt from: seongjunnoh
Repo: THIP-TextHip/THIP-Server PR: 101
File: src/main/java/konkuk/thip/comment/application/service/CommentCreateService.java:36-39
Timestamp: 2025-07-26T06:09:00.850Z
Learning: THIP 프로젝트에서 Record와 Vote는 Room에 속하지만 Feed는 Room에 속하지 않는 구조이며, 댓글 작성 시 Record/Vote에 대해서만 사용자가 해당 Room의 참가자인지 검증이 필요하다.
Applied to files:
src/test/java/konkuk/thip/room/adapter/in/web/RoomPlayingOrExpiredDetailViewApiTest.java
📚 Learning: 2025-09-05T05:19:29.893Z
Learnt from: buzz0331
Repo: THIP-TextHip/THIP-Server PR: 293
File: src/main/java/konkuk/thip/notification/adapter/out/persistence/FcmTokenLoadPersistenceAdapter.java:18-0
Timestamp: 2025-09-05T05:19:29.893Z
Learning: THIP 팀은 Transactional 어노테이션을 서비스 메서드에 붙이는 것으로 통일했다. 퍼시스턴스 어댑터나 리포지토리 레벨이 아닌 서비스 레이어에서 트랜잭션 경계를 관리한다.
Applied to files:
src/test/java/konkuk/thip/room/adapter/in/web/RoomJoinApiTest.java
📚 Learning: 2025-06-29T09:17:39.787Z
Learnt from: hd0rable
Repo: THIP-TextHip/THIP-Server PR: 36
File: build.gradle:30-30
Timestamp: 2025-06-29T09:17:39.787Z
Learning: THIP-Server 프로젝트에서 Redis 의존성(spring-boot-starter-data-redis)은 향후 사용 예정으로 의도적으로 추가되었음. 현재는 사용하지 않지만 추후 개발에서 활용할 계획임.
Applied to files:
src/test/java/konkuk/thip/room/adapter/in/web/RoomJoinApiTest.java
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (9)
src/test/java/konkuk/thip/room/adapter/in/web/RoomPlayingOrExpiredDetailViewApiTest.java (1)
3-3: LGTM: EntityManager 주입이 적절합니다.테스트 데이터 설정 후 영속성 컨텍스트를 클리어하기 위한
em.clear()호출에 필요한EntityManager주입이 올바르게 추가되었습니다. 이를 통해 실제 API 호출 시 데이터베이스에서 최신 상태를 조회하도록 보장합니다.Also applies to: 53-53
loadtest/room_join_load_test.js (2)
17-18: 423 응답 추적을 위한 메트릭 추가가 적절합니다.비관적 락 재시도 소진 시 반환되는 423 응답을 별도로 추적하기 위한 메트릭 추가가 PR 목적과 일치합니다. Threshold
count>=0은 기록 목적으로 적절합니다.Also applies to: 35-36
43-72: 토큰 발급 실패 시 인덱스 정렬 유지 로직이 올바릅니다.Line 62에서 토큰 발급 실패 시에도 빈 문자열을 추가하여 인덱스를 유지하는 방식은 Line 76의 VU-사용자 1:1 매핑을 보장합니다.
src/main/java/konkuk/thip/room/adapter/out/jpa/RoomParticipantJpaEntity.java (1)
66-69: LGTM!
@VisibleForTesting어노테이션과 함께 사용된 테스트 헬퍼 메서드입니다. 프로덕션 코드에서는 사용되지 않도록 적절히 제한되어 있습니다.src/test/java/konkuk/thip/room/adapter/in/web/RoomRecruitingDetailViewApiTest.java (1)
188-190: LGTM!삭제 후 재생성하는 방식보다 직접 역할을 변경하는 것이 더 명확하고 효율적입니다. 테스트 의도가 더 분명해졌습니다.
src/test/java/konkuk/thip/room/adapter/in/web/RoomJoinApiTest.java (1)
67-71: LGTM!알림 서비스를 모킹하여 테스트 범위를 명확히 제한했습니다. 적절한 접근입니다.
src/main/java/konkuk/thip/room/application/service/RoomJoinService.java (2)
51-51: LGTM!
Propagation.REQUIRES_NEW는 재시도 시나리오에 적합합니다. 각 재시도가 독립적인 트랜잭션에서 실행되어, 이전 시도의 트랜잭션 상태에 영향받지 않습니다.
41-50: 락 타임아웃 설정은 올바르게 구성되어 있습니다.JPA 레이어에서
jakarta.persistence.lock.timeout힌트가 5000ms(5초)로 정확히 설정되어 있으므로 추가 검증이 불필요합니다.다만
maxAttempts = 2는 최초 1회 + 재시도 1회로 총 2회 실행을 의미합니다. PR 설명에서 "최대 3회 재시도"를 언급한다면maxAttempts = 3으로 조정이 필요한지 확인하세요.또한
InvalidStateException과BusinessException은noRetryFor목록에 포함되어 있어 이들 예외에 대한@Recover메서드(lines 88-96)는 호출되지 않으므로 제거 검토가 필요합니다.src/main/resources/db/migration/V251229__Add_unique_room_participants.sql (1)
1-4: 재참여 시나리오에 대한 유니크 제약의 영향을 확인하세요.이 마이그레이션은 JPA 엔티티에 이미 정의된 유니크 제약을 데이터베이스 수준에서도 강제합니다. 그러나
room_participants는 soft delete (status = 'INACTIVE')를 사용하므로, INACTIVE 상태의 레코드도 제약 조건에 포함되어 사용자가 탈퇴한 방에 다시 참여하는 것을 차단합니다.이는 코드의 TODO 주석(RoomParticipantJpaEntity line 17-18)에서도 명시적으로 인정된 제한사항입니다. 재참여를 지원해야 한다면 제약 조건에 status 필터링을 추가하거나, 이를 의도적인 설계로 명시적으로 문서화하는 것이 좋습니다.
Comment on lines
+13
to
+23
| @Table( | ||
| name = "room_participants", | ||
| uniqueConstraints = { | ||
| @UniqueConstraint( | ||
| // TODO : room_participant가 soft delete 된 경우에도 unique 제약조건은 여전히 유효 | ||
| // room_participant가 삭제된 방에 다시 참여하는 경우는 일단 고려 X | ||
| name = "uk_room_participant_user_room", | ||
| columnNames = {"user_id", "room_id"} | ||
| ) | ||
| } | ||
| ) |
There was a problem hiding this comment.
소프트 딜리트와 유니크 제약 조건 조합의 데이터 정합성 문제를 해결해야 합니다.
TODO 주석에서 언급한 것처럼, 현재 구현은 중대한 제약사항이 있습니다:
status = 'INACTIVE'로 소프트 딜리트된 참여자도 유니크 제약 조건에 포함됩니다- 사용자가 방을 나갔다가 다시 참여하려 하면 유니크 제약 위반으로 실패합니다
- "일단 고려 X"라고 했지만, 이는 실제로 발생 가능한 일반적인 시나리오입니다
해결 방안:
- PostgreSQL의 경우 partial unique index 사용:
WHERE status = 'ACTIVE' - MySQL의 경우 별도 컬럼으로 유니크 제약 관리 (예:
unique_key = CASE WHEN status='ACTIVE' THEN CONCAT(user_id,'_',room_id) ELSE NULL END) - Hard delete 전환 고려 (감사 로그가 필요하면 별도 이력 테이블 사용)
이 문제에 대한 구체적인 해결 방안을 제시해 드릴까요?
🤖 Prompt for AI Agents
In src/main/java/konkuk/thip/room/adapter/out/jpa/RoomParticipantJpaEntity.java
around lines 13–23 the current JPA @UniqueConstraint on (user_id, room_id)
conflicts with soft-delete (status='INACTIVE') and prevents rejoining; remove
the static UniqueConstraint and instead implement a DB-compatible fix: for
PostgreSQL create a partial unique index on (user_id, room_id) WHERE
status='ACTIVE'; for MySQL add a computed/auxiliary column (e.g., unique_key)
that is populated only when status='ACTIVE' and add a unique constraint on that
column (or use a functional index if supported); update the JPA entity to
include the status field and the auxiliary column mapping (or keep the index
managed by migrations), and add a migration script to drop the old unique
constraint and create the new index/column so active participants remain unique
while inactive rows do not cause violations.
Comment on lines
+190
to
+191
| // em.flush(); | ||
| // em.clear(); |
There was a problem hiding this comment.
주석 처리된 영속성 컨텍스트 초기화로 인한 검증 누락 가능성이 있습니다.
em.flush()와 em.clear()가 주석 처리되어:
- 영속성 컨텍스트에 캐시된 엔티티가 그대로 사용될 수 있습니다
- Lines 194-195의
findById()가 실제 DB가 아닌 1차 캐시에서 조회할 수 있습니다 - 실제 DB 변경사항이 제대로 반영되었는지 검증하지 못할 수 있습니다
@Transactional을 제거했다면 이 코드가 필요 없을 수도 있지만, 명시적인 검증을 위해 em.flush(); em.clear();를 복원하거나, 별도의 트랜잭션에서 조회하는 것이 더 안전합니다.
🤖 Prompt for AI Agents
In src/test/java/konkuk/thip/room/adapter/in/web/RoomJoinApiTest.java around
lines 190-191, the commented out em.flush() and em.clear() can leave entities in
the persistence context so subsequent findById() may read from the 1st-level
cache instead of the DB; restore em.flush(); em.clear(); after
persisting/updating to force SQL execution and clear the persistence context, or
alternatively perform the verification in a new transaction/EntityManager to
ensure the findById() reads the actual database state.
seongjunnoh
commented
Dec 29, 2025
Collaborator
Author
seongjunnoh
left a comment
seongjunnoh
left a comment
There was a problem hiding this comment.
@buzz0331 @hd0rable MySQL x-lock 적용하는 것으로 해당 pr merge 하고 조회 로직 개선으로 넘어가보겠습니다~~
현재 구현은 [DB x-lock + 락 타임아웃 발생 시 재시도] 로 동시에 여러 방 참여 요청이 들어오더라도 데이터 정합성을 유지할 수 있습니다.
[락 획득을 위한 대기시간 설정 + 타임아웃 시 재시도 로직]을 설정해 클라이언트가 [방 참여 성공 or API 제약 조건으로 인한 정상적인 방 참여 실패] 이렇게 2가지 응답만을 받을 수 있도록 했습니다.
-> 재시도 횟수가 초과됐음에도 락 획득에 실패한다면 423 error를 응답하도록 했습니다 (과도한 고트래픽이 아닌 이상 여기까지 오지는 않을 것으로 예상)
이때 잦은 재시도로 인해 톰캣 쓰레드의 점유 시간이 늘어나 서버 전체의 API 요청 처리량이 줄어드는 것을 방지하기 위해, 락 타임아웃 시간을 비교적 넉넉하게(= 5초) 로 설정했습니다.
하지만 현재 구현은 락 경합이 심할 경우 락 해제를 대기하는 쓰레드들의 DB 커넥션 점유 시간이 늘어나고, DB 커넥션 풀 고갈로 인해 방 참여 APi 뿐만아니라 다른 API 응답에서도 문제가 생길 수 있습니다.
-> 최대한 트랜잭션 점유 시간을 줄여 빠른 락 해제를 보장해야하는데, 이를 위한 방법은 고민이 필요할 것 같습니다
추후에 분산 환경을 고려한다면 DB에게 동시성 처리를 맡기는게 아니라, 어플리케이션단에서 redis 등을 활용한 분산 락을 적용하는 등 다른 해결책도 생각해보면 좋을 듯 합니다.
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.
#️⃣ 연관된 이슈
📝 작업 내용
'모임방 참여 api' 에 비관적 락을 적용해, '인기 작가 모임방에 여러 유저들이 동시에 모임방 참여 요청을 보낼 경우' 를 재현한 테스트 코드와 부하 테스트에서의 5xx 에러 발생 이슈를 해결했습니다.
<주요 수정 부분>
<수정 결과>
📸 스크린샷
https://separate-snowplow-d8b.notion.site/API-2-2a3b701eb0b8809db7dbfafeda3596e0?source=copy_link
-> 정리한 노션 링크 입니다!
💬 리뷰 요구사항
위 개선 방향에 대해 리뷰 달아주시면 감사하겠습니다!!
📌 PR 진행 시 이러한 점들을 참고해 주세요
Summary by CodeRabbit
릴리스 노트
새로운 기능
개선 사항
테스트/운영
✏️ Tip: You can customize this high-level summary in your review settings.