[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
개요모임방 참여 API의 동시성 문제를 해결하기 위해 Spring Retry, 비관적 락(pessimistic lock), 그리고 RESOURCE_LOCKED 오류 코드를 추가하여 대량의 동시 참여 요청을 안정적으로 처리합니다. 변경사항
시퀀스 다이어그램sequenceDiagram
participant Client
participant API
participant RoomJoinService
participant Retry as Spring Retry
participant Repository
participant Database
Client->>API: POST /rooms/{id}/join (1000개 동시 요청)
par Concurrent Requests
API->>RoomJoinService: changeJoinState
Note over RoomJoinService: `@Retryable`: 3 attempts<br/>Exponential backoff
RoomJoinService->>Repository: getByIdForUpdate(roomId)
Repository->>Database: SELECT ... FOR UPDATE<br/>(Pessimistic Write Lock)
alt Lock Acquired
Database-->>Repository: Room with lock
Repository-->>RoomJoinService: Room entity
RoomJoinService->>RoomJoinService: Process join logic
RoomJoinService-->>Database: Commit transaction
RoomJoinService-->>API: Success (2xx)
API-->>Client: 200 OK
else Lock Timeout/Conflict
Database-->>Repository: Lock timeout (after 5s)
Repository-->>RoomJoinService: Optional.empty()
RoomJoinService->>Retry: Retry attempt (exponential backoff)
alt Max Retries Exceeded
Retry->>RoomJoinService: `@Recover` handler
RoomJoinService-->>API: BusinessException(RESOURCE_LOCKED)
API-->>Client: 423 Locked
else Retry Succeeds
RoomJoinService->>Repository: getByIdForUpdate (retry)
Repository->>Database: SELECT ... FOR UPDATE (다시 시도)
Database-->>Repository: Lock acquired
Repository-->>RoomJoinService: Room entity
RoomJoinService-->>API: Success
API-->>Client: 200 OK
end
end
end
예상 코드 리뷰 소요 시간🎯 4 (복잡함) | ⏱️ ~60분 주의 깊게 검토할 영역:
관련 PR
추천 검토자
시
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 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 Results486 tests 486 ✅ 47s ⏱️ Results for commit a182cc8. ♻️ 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 이슈로 추적하여 나중에 해결할 수 있도록 하세요. 코드에 설명이 있지만 주석 처리된 테스트는 유지보수 부담이 됩니다.
Comment on lines
+158
to
+179
| /** | ||
| * 400 error 가 아니라 재시도 횟수 초과로 인해 423 error 발생 | ||
| * H2 DB에서 select for update 패턴으로 락 획득할 시에 계속 예외 발생 -> 재시도 반복하여 테스트 의도대로 400 error 응답 X | ||
| * test yml에 LOCK_TIMEOUT 명시적으로 설정해도 해결 X | ||
| * 일단 주석 처리 | ||
| */ | ||
| // @Test | ||
| // @DisplayName("방 중복 참여 실패") | ||
| // void joinRoom_alreadyParticipated() throws Exception { | ||
| // // 이미 참여한 상태로 설정 | ||
| // setUpWithParticipant(); | ||
| // | ||
| // Map<String, Object> request = new HashMap<>(); | ||
| // request.put("type", JOIN.getType()); | ||
| // | ||
| // ResultActions result = mockMvc.perform(post("/rooms/" + room.getRoomId() + "/join") | ||
| // .requestAttr("userId", participant.getUserId()) | ||
| // .contentType(MediaType.APPLICATION_JSON) | ||
| // .content(objectMapper.writeValueAsString(request))); | ||
| // | ||
| // result.andExpect(status().isBadRequest()); | ||
| // } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
주석 처리된 테스트를 제거하거나 이슈로 추적하세요.
H2 DB의 락 타임아웃 동작 차이로 인해 두 개의 테스트가 주석 처리되었습니다. 설명에 따르면 예상된 400 에러 대신 423 에러가 발생한다고 합니다.
주석 처리된 테스트를 코드에 남겨두면 유지보수성이 떨어지고 코드베이스가 지저분해집니다. 다음 중 하나를 선택하세요:
- 추천: 주석 처리된 테스트를 완전히 제거하고 별도 이슈를 생성하여 추적
- TestContainers 등을 활용해 실제 MySQL/PostgreSQL 환경에서 테스트하도록 개선
- H2 호환 모드에서 동작하도록 테스트 로직 수정
주석 처리된 테스트 제거와 추적용 이슈 생성을 원하시면 도움을 드릴 수 있습니다.
Also applies to: 208-228
🤖 Prompt for AI Agents
In src/test/java/konkuk/thip/room/adapter/in/web/RoomJoinApiTest.java around
lines 158-179 (and similarly 208-228), there are commented-out tests that were
disabled due to H2 lock-timeout behavior causing 423 instead of the expected
400; remove these commented tests from the codebase and open a tracking issue
that documents the failing test names, the observed H2 vs production DB
behavior, reproduction steps, and suggested remediation options (run tests
against TestContainers MySQL/Postgres, adjust test logic for H2 compatibility,
or configure H2 lock timeout if feasible); alternatively, if you prefer to keep
an automated check, convert the tests to an @Disabled JUnit test with a short
message linking to the created issue instead of leaving commented code.
Collaborator
Author
There was a problem hiding this comment.
@hd0rable @buzz0331 커밋 메시지에 작성한 것처럼, 현재 브랜치에서 모임방 참여 코드의 동시성 이슈를 해결하기 위해 [비관적 락 + spring retry를 통한 재시도(매번 새로운 트랜잭션으로) + @recover] 로직을 추가했는데, 기존 테스트 코드가 깨지는 부분 중 해결되지 않는 테스트 코드가 있는 상황입니다.
실패하는 테스트 메서드(= 4xx error 발생 확인을 위한 테스트) 의 빌드 로그를 확인하니, 트랜잭션 시작 이후 최초 Room 엔티티 조회 시 select for update sql 로그가 찍히고, 이후 @recover로 흘러가 423 error 가 발생한 것을 확인했습니다.
(validation 과정까지 코드가 흘러가지 못해 400 error가 발생하지 않아서 테스트가 실패한 것임)
구글링을 통해 H2 DB는 제가 jpa repository에 명시한 @QueryHint의 timeout 설정값을 읽지 못한다는 정보를 얻어서, test yml의 H2 url에 LOCK_TIMEOUT 값을 명시했는데도 여전히 테스트 메서드가 깨졌습니다.
ref : https://rudaks.tistory.com/entry/Spring-Data-Jpa%EC%97%90%EC%84%9C-%EB%8D%B0%EC%9D%B4%ED%84%B0%EB%B2%A0%EC%9D%B4%EC%8A%A4%EB%B3%84-lock-timeout-%EC%84%A4%EC%A0%95-%ED%85%8C%EC%8A%A4%ED%8A%B8#google_vignette
일단 현재 브랜치의 모임방 참여 로직은 추후 개선될 예정이라, 깨지는 테스트 메서드를 일단 주석처리 하고 넘어가도 괜찮지 않나 라고 생각했는데, 다른 의견있으면 코멘트 부탁드립니다!!
추가로 현재 THIP이 실서비스 운영중이니, 서버의 테스트 메서드 환경의 DB을 운영 환경과 동일하게 MySQL로 변경하는 게 어떨지 제안드립니다!
(DB 환경이 달라서 마주하는 이슈들이 조금씩 크게 다가오는거 같습니다)
ref : https://dami97.tistory.com/73
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
Member
There was a problem hiding this comment.
동시성이 있는 상황이아닌데 도대체 왜 423에러가 뜨는지궁금했었는데 단순 h2 스펙때문에 나는 오류였군요 확인했습니다!!
실제 운영 db와 테스트 db의환경이 달라서 저번에도 ci만을위해서 수정한 코드가 종종있었는데 Testcontainers라는 라이브러리가있었균요 좋은것같습니다!! 우선은 일단 주석처리하고 넘어가도 될 것같습니다
Contributor
There was a problem hiding this comment.
제 생각은 현재 @Recover와 @Retryable어노테이션으로 인해 원래 BusinessException이나 InvalidException으로 발생해야 하는 예외상황들이 모두 RESOURCE_LOCKED 에러로 치환되어서 throw되기 때문에 기존 테스트 케이스가 깨지는 것이 아닐까 라고 생각되는데 이부분은 어떻게 생각하시나요? (카톡에서 이야기한 내용과 일맥상통하는 이야기입니다!)
- 멀티쓰레드 환경에서 동시에 모임방 참여 요청을 보낼 때, 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
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}, // 재시도 제외 예외 |
Comment on lines
+158
to
+179
| /** | ||
| * 400 error 가 아니라 재시도 횟수 초과로 인해 423 error 발생 | ||
| * H2 DB에서 select for update 패턴으로 락 획득할 시에 계속 예외 발생 -> 재시도 반복하여 테스트 의도대로 400 error 응답 X | ||
| * test yml에 LOCK_TIMEOUT 명시적으로 설정해도 해결 X | ||
| * 일단 주석 처리 | ||
| */ | ||
| // @Test | ||
| // @DisplayName("방 중복 참여 실패") | ||
| // void joinRoom_alreadyParticipated() throws Exception { | ||
| // // 이미 참여한 상태로 설정 | ||
| // setUpWithParticipant(); | ||
| // | ||
| // Map<String, Object> request = new HashMap<>(); | ||
| // request.put("type", JOIN.getType()); | ||
| // | ||
| // ResultActions result = mockMvc.perform(post("/rooms/" + room.getRoomId() + "/join") | ||
| // .requestAttr("userId", participant.getUserId()) | ||
| // .contentType(MediaType.APPLICATION_JSON) | ||
| // .content(objectMapper.writeValueAsString(request))); | ||
| // | ||
| // result.andExpect(status().isBadRequest()); | ||
| // } |
Member
There was a problem hiding this comment.
동시성이 있는 상황이아닌데 도대체 왜 423에러가 뜨는지궁금했었는데 단순 h2 스펙때문에 나는 오류였군요 확인했습니다!!
실제 운영 db와 테스트 db의환경이 달라서 저번에도 ci만을위해서 수정한 코드가 종종있었는데 Testcontainers라는 라이브러리가있었균요 좋은것같습니다!! 우선은 일단 주석처리하고 넘어가도 될 것같습니다
buzz0331
approved these changes
Nov 11, 2025
Contributor
buzz0331
left a comment
buzz0331
left a comment
There was a problem hiding this comment.
너무 좋은 것 같습니다! 테스트 코드나 부하테스트 시나리오 의도가 명확해서 이해도 잘되고 잘 와닿는 것 같습니다~
Comment on lines
+158
to
+179
| /** | ||
| * 400 error 가 아니라 재시도 횟수 초과로 인해 423 error 발생 | ||
| * H2 DB에서 select for update 패턴으로 락 획득할 시에 계속 예외 발생 -> 재시도 반복하여 테스트 의도대로 400 error 응답 X | ||
| * test yml에 LOCK_TIMEOUT 명시적으로 설정해도 해결 X | ||
| * 일단 주석 처리 | ||
| */ | ||
| // @Test | ||
| // @DisplayName("방 중복 참여 실패") | ||
| // void joinRoom_alreadyParticipated() throws Exception { | ||
| // // 이미 참여한 상태로 설정 | ||
| // setUpWithParticipant(); | ||
| // | ||
| // Map<String, Object> request = new HashMap<>(); | ||
| // request.put("type", JOIN.getType()); | ||
| // | ||
| // ResultActions result = mockMvc.perform(post("/rooms/" + room.getRoomId() + "/join") | ||
| // .requestAttr("userId", participant.getUserId()) | ||
| // .contentType(MediaType.APPLICATION_JSON) | ||
| // .content(objectMapper.writeValueAsString(request))); | ||
| // | ||
| // result.andExpect(status().isBadRequest()); | ||
| // } |
Contributor
There was a problem hiding this comment.
제 생각은 현재 @Recover와 @Retryable어노테이션으로 인해 원래 BusinessException이나 InvalidException으로 발생해야 하는 예외상황들이 모두 RESOURCE_LOCKED 에러로 치환되어서 throw되기 때문에 기존 테스트 케이스가 깨지는 것이 아닐까 라고 생각되는데 이부분은 어떻게 생각하시나요? (카톡에서 이야기한 내용과 일맥상통하는 이야기입니다!)
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
릴리스 노트
새로운 기능
개선 사항