[Refactor] External Api Isolation in 방 생성#350
Open
seongjunnoh wants to merge 14 commits intodevelopfrom
Open
Conversation
- Naver API 통신은 RestTemplate 이 아니라, 기존 코드(java net HttpURLConnection 활용) 유지
- 알라딘 API 통신으로 얻는 pageCount 값은 방 생성 시 부가 정보로 판단 - 따라서 해당 과정에서 예외 발생 시 전체 방 생성 로직을 롤백시키지 않도록 수정
- 알라딘 API 통신 실패 시 동작하는 비동기 알림 전송 메서드 추가 - Discord 서버와 동기/비동기 통신하는 헬퍼 메서드 추가 - 기존 500 error 발생 시 동작하는 디스코드 알림은 기존 동기적 통신 유지
- 실제 디스코드 웹훅 메시지 전송되는지 확인 - 테스트 실행시마다 디스코드 메시지 전송되는 것 막기 위해 해당 코드 주석처리
- WebClient 싱글톤 스프링 빈 활용하도록 config 파일 추가 - 매번 WebClient create 하는 것 방지
- 외부 API 호출 부분을 spring transactional 범위에서 제거 - TransactionTemplate 스프링 빈 추가 - TransactionTemplate 활용해 [방 저장 & 방장 저장] 원자적으로 실행
- 방 생성 시 알라딘 API 장애/지연 으로 pageCount = null로 세팅된 데이터를 백그라운드 업데이트 하는 스케줄러
- 알라딘에 등록되지 않은 책일 경우, 해당 flag를 업데이트 - 스케줄러가 필요없는 API 통신을 수행하지 않도록 하기 위함
6 tasks
Walkthroughnull 페이지 수를 가진 책의 정보를 알라딘 API로 정기적으로 채우는 스케줄러를 추가합니다. API 실패 시 Discord로 알림을 보내고, 찾을 수 없는 책은 별도 플래그로 표시합니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Scheduler as BookPageCountFillScheduler
participant Service as BookPageCountFillService
participant QueryPort as BookQueryPort
participant CommandPort as BookCommandPort
participant AladinApi as AladinApiClient
participant Discord as DiscordClient
Scheduler->>Service: fillNullPageCounts()
Service->>QueryPort: findBooksWithNullPageCountLinkedToRooms()
QueryPort-->>Service: List<Book>
loop for each book
Service->>AladinApi: findPageCountByIsbn(isbn)
alt Success
AladinApi-->>Service: pageCount
Service->>CommandPort: updateForPageCount(book)
CommandPort-->>Service: OK
else ISBN Not Found
AladinApi-->>Service: ExternalApiException(ISBN_NOT_FOUND)
Service->>CommandPort: updateForUnfindable(book)
CommandPort-->>Service: OK
else Transient Failure
AladinApi-->>Service: ResourceAccessException
Service->>Service: Skip update (retry later)
end
Service->>Service: throttle(500ms)
end
Service->>Discord: sendPageCountFillResult(total, success, unfindable, transientFail)
Discord-->>Service: OK
sequenceDiagram
participant Client as Client
participant ApiAdapter as CompositeBookApiAdapter
participant AladinApi as AladinApiClient
participant Discord as DiscordClient
Client->>ApiAdapter: findPageCountByIsbn(isbn)
rect rgba(200, 150, 100, 0.5)
ApiAdapter->>AladinApi: findPageCountByIsbn(isbn)
alt Success
AladinApi-->>ApiAdapter: pageCount
ApiAdapter-->>Client: pageCount
else Exception
AladinApi-->>ApiAdapter: Exception
ApiAdapter->>Discord: sendAladinApiFailureAlert(isbn, exception)
Discord-->>ApiAdapter: OK (async)
ApiAdapter-->>Client: null
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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 Results508 tests 508 ✅ 48s ⏱️ Results for commit 5748293. |
There was a problem hiding this comment.
Pull request overview
이 PR은 방 생성 과정에서 외부 API(Naver/Aladin) 지연·장애가 DB 커넥션/서버 스레드 고갈로 번지는 연쇄 장애를 줄이기 위해, 트랜잭션 경계를 외부 호출과 분리하고 타임아웃/Fail-soft/백필 스케줄러를 도입하는 리팩터링입니다.
Changes:
- 방 생성 트랜잭션 경계를
TransactionTemplate로 최소화하고, 외부 API 호출 구간을 트랜잭션 밖으로 이동 - RestTemplate/WebClient 타임아웃 명시 + 알라딘 장애 시 null 반환 및 Discord 알림(비동기)로 격리
page_count_unfindable플래그/인덱스 및BookPageCountFillScheduler/Service로 pageCount 백필(최종적 일관성) 구축 + 테스트 추가
Reviewed changes
Copilot reviewed 14 out of 26 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/main/java/konkuk/thip/room/application/service/RoomCreateService.java | 외부 API 호출을 트랜잭션 밖으로 분리하고, Room/Participant 저장만 템플릿 트랜잭션으로 묶음 |
| src/main/java/konkuk/thip/config/AppConfig.java | TransactionTemplate Bean 등록 |
| src/main/java/konkuk/thip/config/RestTemplateConfig.java | RestTemplate connect/read timeout 설정 추가 |
| src/main/java/konkuk/thip/config/WebClientConfig.java | WebClient(Netty) connect/response timeout 설정 추가 |
| src/main/java/konkuk/thip/book/adapter/out/api/CompositeBookApiAdapter.java | 알라딘 실패 시 Discord 알림 후 null 반환(Fail-soft), 네이버는 예외 전파 유지 |
| src/main/java/konkuk/thip/book/adapter/out/api/naver/NaverApiUtil.java | HttpURLConnection connect/read timeout 적용 및 포맷 정리 |
| src/main/java/konkuk/thip/common/discord/DiscordClient.java | WebClient 주입 기반으로 변경, 500 알림(동기) vs 알라딘 실패(비동기) 및 스케줄러 결과 알림 추가 |
| src/main/java/konkuk/thip/common/exception/handler/GlobalExceptionHandler.java | 500 에러 Discord 전송 try/catch 제거(DiscordClient 내부에서 처리하도록 위임) |
| src/main/java/konkuk/thip/common/exception/ExternalApiException.java | errorCode getter 추가 |
| src/main/java/konkuk/thip/book/domain/Book.java | pageCountUnfindable 필드 및 markAsUnfindable() 추가 |
| src/main/java/konkuk/thip/book/application/service/BookPageCountFillService.java | pageCount null 백필 서비스(스로틀/실패 유형 분류/Discord 결과 알림) 추가 |
| src/main/java/konkuk/thip/common/scheduler/BookPageCountFillScheduler.java | 매일 03:00 백필 스케줄러 추가 |
| src/main/java/konkuk/thip/book/application/port/in/BookPageCountFillUseCase.java | 백필 유스케이스 인터페이스 추가 |
| src/main/java/konkuk/thip/book/application/port/out/BookQueryPort.java | 스케줄러용 조회 포트 메서드 추가 |
| src/main/java/konkuk/thip/book/application/port/out/BookCommandPort.java | 스케줄러용 unfindable 업데이트 포트 메서드 추가 |
| src/main/java/konkuk/thip/book/adapter/out/persistence/repository/BookJpaRepository.java | 스케줄러 대상 조회 JPQL 추가 |
| src/main/java/konkuk/thip/book/adapter/out/persistence/BookQueryPersistenceAdapter.java | 스케줄러 대상 조회 구현 추가 |
| src/main/java/konkuk/thip/book/adapter/out/persistence/BookCommandPersistenceAdapter.java | updateForUnfindable 구현 추가 |
| src/main/java/konkuk/thip/book/adapter/out/mapper/BookMapper.java | pageCountUnfindable 매핑 추가 |
| src/main/java/konkuk/thip/book/adapter/out/jpa/BookJpaEntity.java | page_count_unfindable 컬럼 및 mark 메서드 추가 |
| src/main/resources/db/migration/V260302__Add_page_count_unfindable_and_scheduler_index_to_book.sql | unfindable 컬럼/스케줄러 인덱스 추가 |
| src/test/java/konkuk/thip/room/application/service/RoomCreateServiceTest.java | 트랜잭션 분리/알라딘 fail-soft/동시성 충돌 처리에 대한 단위 테스트 추가 |
| src/test/java/konkuk/thip/book/application/service/BookPageCountFillServiceTest.java | 백필 서비스 성공/미등록/일시실패/혼합 케이스 단위 테스트 추가 |
| src/test/java/konkuk/thip/book/adapter/out/api/naver/NaverApiUtilTest.java | IOException → ExternalApiException 변환 경로 테스트 추가 |
| src/test/java/konkuk/thip/book/adapter/out/api/CompositeBookApiAdapterTest.java | 알라딘 fail-soft + Discord 알림 호출 여부 테스트 추가 |
| src/test/java/konkuk/thip/book/adapter/out/api/AladinApiDiscordAlertIntegrationTest.java | (수동 검증용) 실제 Discord 웹훅 전송 확인 테스트 추가 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| .bodyToMono(Void.class) | ||
| .block(); | ||
| .subscribe( | ||
| null, |
There was a problem hiding this comment.
sendAsync()에서 subscribe(null, ...)로 구독하고 있는데, Reactor subscribe(Consumer, Consumer) 오버로드는 첫 번째 Consumer가 null이면 런타임에 NPE가 발생할 수 있습니다. 값 consumer는 사용하지 않더라도 빈 람다(unused -> {})처럼 non-null로 전달해 주세요.
Suggested change
| null, | |
| unused -> { }, |
Comment on lines
69
to
76
| private void updateBookPageCount(Book book) { | ||
| // 알라딘 API 실패 시 null 반환 (Discord 알림은 어댑터에서 처리) | ||
| Integer pageCount = bookApiQueryPort.findPageCountByIsbn(book.getIsbn()); | ||
| if (pageCount == null) return; | ||
|
|
||
| book.changePageCount(pageCount); | ||
| bookCommandPort.updateForPageCount(book); | ||
| } |
There was a problem hiding this comment.
알라딘 API가 pageCount 필드를 내려주지 않는 경우 AladinApiUtil#getPageCount()가 asInt() 기본값으로 0을 반환할 수 있습니다. 현재 로직은 0도 정상 값처럼 DB에 업데이트해버려 스케줄러(page_count IS NULL)로도 복구가 불가능해지고, 이후에도 hasPageCount()가 false라 계속 외부 API를 재호출하게 됩니다. pageCount == null || pageCount <= 0 인 경우는 업데이트하지 않도록 방어 로직을 추가해 주세요.
Comment on lines
+52
to
+55
| book.changePageCount(pageCount); | ||
| bookCommandPort.updateForPageCount(book); | ||
| successCount++; | ||
| log.info("[스케줄러] pageCount 업데이트 완료 - isbn: {}, pageCount: {}", book.getIsbn(), pageCount); |
There was a problem hiding this comment.
aladinApiClient.findPageCountByIsbn()가 pageCount를 못 찾는 응답을 받으면(예: JSON 필드 누락) 0이 반환될 수 있는데, 현재는 0도 성공으로 카운트하고 DB에 저장합니다. 이렇게 되면 이후 스케줄러 조회 조건(page_count IS NULL)에 걸리지 않아 영구적으로 복구가 막힐 수 있으니, pageCount == null || pageCount <= 0이면 성공 처리/DB 업데이트를 건너뛰고 transient failure로 집계하는 식으로 처리해 주세요.
Suggested change
| book.changePageCount(pageCount); | |
| bookCommandPort.updateForPageCount(book); | |
| successCount++; | |
| log.info("[스케줄러] pageCount 업데이트 완료 - isbn: {}, pageCount: {}", book.getIsbn(), pageCount); | |
| // pageCount를 찾지 못했거나 비정상 값(0 이하)인 경우 → 일시 실패로 간주, DB 미반영 | |
| if (pageCount == null || pageCount <= 0) { | |
| transientFailCount++; | |
| log.warn("[스케줄러] 유효하지 않은 pageCount 응답 (재시도 예정) - isbn: {}, pageCount: {}", book.getIsbn(), pageCount); | |
| } else { | |
| book.changePageCount(pageCount); | |
| bookCommandPort.updateForPageCount(book); | |
| successCount++; | |
| log.info("[스케줄러] pageCount 업데이트 완료 - isbn: {}, pageCount: {}", book.getIsbn(), pageCount); | |
| } |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (10)
src/main/java/konkuk/thip/book/application/port/out/BookCommandPort.java (1)
35-36: 메서드명을 비즈니스 의도 중심으로 다듬는 것을 권장합니다.
updateForUnfindable보다markPageCountUnfindable처럼 “무엇을 보장하는지”가 드러나는 이름이 포트 계약 이해에 더 유리합니다.이름 개선 예시
- void updateForUnfindable(Book book); + void markPageCountUnfindable(Book book);Based on learnings: seongjunnoh는 메서드 네이밍 시 구현 세부사항보다 비즈니스 의도를 반영하는 것을 선호하며, 미래 확장성을 고려한 설계를 선호한다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/konkuk/thip/book/application/port/out/BookCommandPort.java` around lines 35 - 36, The port method name updateForUnfindable on the BookCommandPort interface is implementation-centric; rename it to a business-intent name such as markPageCountUnfindable(Book book) to make the contract explicit, then update the BookCommandPort interface declaration, all implementing classes and any callers/usages, plus Javadoc and tests to use markPageCountUnfindable(Book) while keeping the method signature (void, accepting Book) unchanged semantically.src/main/java/konkuk/thip/config/WebClientConfig.java (1)
17-19: 타임아웃 값을 상수/공통 설정으로 통일하는 것을 권장합니다.현재 숫자 리터럴이 직접 들어가 있어
RestTemplate설정과 드리프트가 생기기 쉽습니다. 공통 상수(또는 프로퍼티)로 묶어 유지보수 비용을 줄이는 편이 좋습니다.간단한 상수화 예시
public class WebClientConfig { + private static final int CONNECT_TIMEOUT_MS = 3_000; + private static final int READ_TIMEOUT_MS = 5_000; @@ HttpClient httpClient = HttpClient.create() - .option(ChannelOption.CONNECT_TIMEOUT_MILLIS, 3_000) - .responseTimeout(Duration.ofSeconds(5)); + .option(ChannelOption.CONNECT_TIMEOUT_MILLIS, CONNECT_TIMEOUT_MS) + .responseTimeout(Duration.ofMillis(READ_TIMEOUT_MS));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/konkuk/thip/config/WebClientConfig.java` around lines 17 - 19, Replace the numeric literals in the HttpClient setup in WebClientConfig (the HttpClient httpClient = HttpClient.create()... chain) with named constants or configurable properties so timeouts are centralized; introduce e.g. CONNECT_TIMEOUT_MS and RESPONSE_TIMEOUT_SEC (as private static final constants or `@Value-injected` properties) and use them in ChannelOption.CONNECT_TIMEOUT_MILLIS and Duration.ofSeconds(...) respectively, then update any other HTTP client or RestTemplate timeout configuration to reference the same constants/properties to avoid drift.src/main/java/konkuk/thip/common/discord/DiscordClient.java (1)
29-117: embed 생성 로직 중복 - 리팩토링 고려 가능.
sendErrorMessage,sendAladinApiFailureAlert,sendPageCountFillResult각각에서Map<String, String>필드 생성과 payload 조립 패턴이 반복됩니다. 현재 가독성은 좋으나, 향후 유지보수를 위해 헬퍼 메서드 추출을 고려해볼 수 있습니다.♻️ 선택적 리팩토링 예시
private Map<String, String> createField(String name, String value) { Map<String, String> field = new HashMap<>(); field.put("name", name); field.put("value", value); return field; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/konkuk/thip/common/discord/DiscordClient.java` around lines 29 - 117, The three methods sendErrorMessage, sendAladinApiFailureAlert, and sendPageCountFillResult duplicate embed/field creation and payload assembly; extract small helper methods (e.g., createField(String name, String value), buildEmbed(String title, List<Map<String,String>> fields) that returns the embed map, and createPayload(Map<String,Object> embed) that returns the final payload) and replace the repeated Map construction in those three methods to call the helpers to assemble fields, embedData and payload before calling sendSync/sendAsync.src/test/java/konkuk/thip/book/adapter/out/api/AladinApiDiscordAlertIntegrationTest.java (1)
56-58:Thread.sleep사용에 대한 참고사항.수동 검증용 테스트로서 의도는 이해되지만, CI 환경에서는
discord.env=test로 인해 실제 Discord 전송이 발생하지 않습니다.@TestPropertySource가 주석 처리되어 있어 현재는 null 반환 검증만 수행됩니다.비동기 동작 검증이 필요한 경우,
DiscordClient를 모킹하고verify()로 호출 여부를 확인하는 것이 더 안정적인 테스트가 될 수 있습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/konkuk/thip/book/adapter/out/api/AladinApiDiscordAlertIntegrationTest.java` around lines 56 - 58, The test currently uses Thread.sleep after calling sendAladinApiFailureAlert() to wait for the fire-and-forget subscribe(), which is unreliable; remove the Thread.sleep and instead mock the asynchronous collaborator (e.g., DiscordClient) in AladinApiDiscordAlertIntegrationTest and inject it into the class under test (or use `@MockBean`), then call sendAladinApiFailureAlert() and use Mockito.verify() (or equivalent) to assert that the DiscordClient method was invoked (or that the reactive subscribe path was executed) to deterministically validate the async behavior; also ensure any commented `@TestPropertySource` is either restored or test-specific properties are set so the mock is actually used.src/main/java/konkuk/thip/common/scheduler/BookPageCountFillScheduler.java (1)
18-21: 예외 발생 시 종료 로그가 누락됩니다.실행 중 예외가 발생하면 종료 로그가 남지 않아서 운영 추적이 어려워질 수 있습니다.
finally로 종료 로그를 보장하는 편이 안전합니다.개선 예시
public void fillNullPageCounts() { log.info("[스케줄러] pageCount 업데이트 시작"); - bookPageCountFillUseCase.fillNullPageCounts(); - log.info("[스케줄러] pageCount 업데이트 종료"); + try { + bookPageCountFillUseCase.fillNullPageCounts(); + } finally { + log.info("[스케줄러] pageCount 업데이트 종료"); + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/konkuk/thip/common/scheduler/BookPageCountFillScheduler.java` around lines 18 - 21, The fillNullPageCounts method in BookPageCountFillScheduler currently logs only on start and after successful completion, so if bookPageCountFillUseCase.fillNullPageCounts() throws the end log is missing; wrap the call in a try/finally (or try/catch/finally) around bookPageCountFillUseCase.fillNullPageCounts() so that the "[스케줄러] pageCount 업데이트 종료" log is executed in the finally block, and if you add a catch, log the exception (e.g., with log.error) and rethrow if needed to preserve behavior.src/test/java/konkuk/thip/room/application/service/RoomCreateServiceTest.java (2)
218-223: 중복 스텁이 테스트 의도를 흐립니다.
findByIsbn첫 스텁은 바로 아래 순차 스텁에 의해 덮입니다. 하나로 합치면 시나리오가 더 명확해집니다.정리 예시
- given(bookCommandPort.findByIsbn(ISBN)).willReturn(Optional.empty()); given(bookApiQueryPort.loadBookWithPageByIsbn(ISBN)).willReturn(bookWithPage()); @@ - given(bookCommandPort.findByIsbn(ISBN)).willReturn(Optional.empty(), Optional.of(bookWithPage())); + given(bookCommandPort.findByIsbn(ISBN)) + .willReturn(Optional.empty(), Optional.of(bookWithPage()));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/konkuk/thip/room/application/service/RoomCreateServiceTest.java` around lines 218 - 223, In RoomCreateServiceTest, remove the redundant initial stubbing of bookCommandPort.findByIsbn and replace it with a single sequential stub so the first invocation returns Optional.empty() and the subsequent invocation returns Optional.of(bookWithPage()); keep the existing stubs for bookApiQueryPort.loadBookWithPageByIsbn(ISBN) to return bookWithPage() and for bookCommandPort.save(any()) to throw new DataIntegrityViolationException("duplicate key") to preserve the race-condition save-then-requery scenario.
57-61: 트랜잭션 경계 회귀를 잡는 검증이 부족합니다.
TransactionTemplate.execute(...)를 단순 pass-through로만 스텁해 두면, 외부 API 호출이 트랜잭션 안으로 다시 들어가도 테스트가 통과할 수 있습니다. PR 목표를 고정하려면 호출 순서 검증을 추가해 주세요.테스트 보강 예시
`@Test` `@DisplayName`("알라딘 API 성공 시 pageCount가 업데이트되고 방이 생성된다") void createRoom_aladinSuccess_pageCountUpdated() { @@ Long roomId = roomCreateService.createRoom(createCommand(), USER_ID); assertThat(roomId).isEqualTo(ROOM_ID); then(bookCommandPort).should().updateForPageCount(any()); then(roomCommandPort).should().save(any()); + + InOrder inOrder = inOrder(bookApiQueryPort, transactionTemplate, roomCommandPort, roomParticipantCommandPort); + inOrder.verify(bookApiQueryPort).findPageCountByIsbn(ISBN); // 외부 API + inOrder.verify(transactionTemplate).execute(any()); // DB write 트랜잭션 }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/konkuk/thip/room/application/service/RoomCreateServiceTest.java` around lines 57 - 61, The test currently stubs transactionTemplate.execute to invoke the TransactionCallback but lacks call-order checks, so add an InOrder verification to assert that transactionTemplate.execute(...) (and the TransactionCallback execution) happens before the external API mock is invoked; use Mockito InOrder with the transactionTemplate mock and the external API mock (e.g., externalApiClient) and verify transactionTemplate.execute(any(TransactionCallback.class)) was called prior to externalApiClient.<method>() to prevent regressions where the external API is accidentally executed inside the transaction.src/main/java/konkuk/thip/book/application/port/out/BookQueryPort.java (1)
23-24: CQRS 포트 경계 일관성을 한 번 더 점검해 주세요.
BookQueryPort가 도메인 엔티티(List<Book>)를 직접 반환하면, 조회 응답 포트와 도메인 조회 포트의 책임이 섞일 수 있습니다. 전용 조회 DTO를 두거나, 도메인 엔티티 조회 성격이면 Command 측 포트로 이동하는 방식을 검토해 주세요.Based on learnings: THIP 프로젝트에서는 CQRS Port 분리 시 CommandPort에는 findByXXX로 도메인 엔티티를 조회하고, QueryPort에는 조회 API의 response 데이터를 DB에서 조회하는 메서드를 추가하는 컨벤션을 따릅니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/konkuk/thip/book/application/port/out/BookQueryPort.java` around lines 23 - 24, BookQueryPort currently returns domain entities (List<Book>) from findBooksWithNullPageCountLinkedToRooms which mixes query-port responsibility with domain model; change this to return a dedicated query DTO (e.g., BookNullPageCountDto) and update the method signature on BookQueryPort to List<BookNullPageCountDto> findBooksWithNullPageCountLinkedToRooms(); create the new DTO containing only the fields needed by the scheduler (id, roomId, any pageCount-related flags) and move any domain-centric findBy... semantics back to the Command/Repository port (e.g., BookCommandPort with findBy... returning List<Book>) if you need the full entity elsewhere.src/main/java/konkuk/thip/book/application/service/BookPageCountFillService.java (1)
77-79: Discord 알림 실패 시 예외 전파 가능성
discordClient.sendPageCountFillResult()호출이 예외를 던지면 스케줄러 로그에 에러가 기록되고 작업이 실패한 것처럼 보일 수 있습니다. 실제 DB 업데이트는 이미 완료된 상태이므로, Discord 알림 실패가 전체 작업 결과에 영향을 주지 않도록 try-catch로 감싸는 것을 고려해보세요.♻️ Discord 알림 실패 격리 제안
log.info("[스케줄러] 실행 완료 - 대상: {}건 / 성공: {}건 / 미등록: {}건 / 일시실패: {}건", books.size(), successCount, unfindableCount, transientFailCount); - discordClient.sendPageCountFillResult(books.size(), successCount, unfindableCount, transientFailCount); + try { + discordClient.sendPageCountFillResult(books.size(), successCount, unfindableCount, transientFailCount); + } catch (Exception e) { + log.warn("[스케줄러] Discord 알림 전송 실패 - error: {}", e.getMessage()); + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/konkuk/thip/book/application/service/BookPageCountFillService.java` around lines 77 - 79, The call to discordClient.sendPageCountFillResult in BookPageCountFillService can throw and bubble up, making the scheduler appear to fail after DB updates; wrap the sendPageCountFillResult(...) invocation in a try-catch that logs the Discord delivery failure (using log.error) but does not rethrow so the job remains successful, and optionally include the exception details in the log to aid debugging; ensure the log.info([...]) call stays before or is not suppressed by the catch so DB result counts (books.size(), successCount, unfindableCount, transientFailCount) are preserved in logs even if Discord fails.src/test/java/konkuk/thip/book/application/service/BookPageCountFillServiceTest.java (1)
40-44: 테스트 헬퍼 메서드가 최소한의 필드만 설정합니다.
bookWithIsbn헬퍼가 ISBN만 설정하고 있습니다. 현재 테스트에서는BookCommandPortmock을 통해 검증하므로 문제없지만, 향후book.changePageCount()나book.markAsUnfindable()등 도메인 메서드의 실제 동작을 검증하려면 Book 도메인 객체의 상태 변경도 assert하는 것이 좋습니다.현재 구조에서는 Book 도메인 로직이 정상 동작하는지 별도 단위 테스트로 커버되어야 합니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/konkuk/thip/book/application/service/BookPageCountFillServiceTest.java` around lines 40 - 44, The helper method bookWithIsbn only sets isbn; update it to initialize a fuller valid Book state (e.g., set id/title/pageCount/status/author or other required fields used by domain methods) so domain behavior like Book.changePageCount() and Book.markAsUnfindable() can be meaningfully exercised in tests; modify the bookWithIsbn helper (and any related test factories) to construct a Book with those minimal fields consistent with the Book class invariants and use the Book builder/constructors used in production so subsequent assertions against Book instance state will be valid.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/main/java/konkuk/thip/book/adapter/out/persistence/repository/BookJpaRepository.java`:
- Around line 35-41: The repository method
findBooksWithNullPageCountLinkedToRooms currently returns a List and loads all
matches into memory; change its signature in BookJpaRepository to accept a
Pageable (e.g., Pageable pageable) and return a Page<BookJpaEntity> or
Slice<BookJpaEntity> so queries can be paginated, update the `@Query` to keep the
same WHERE but remove ordering assumptions if using Slice, and then in the
service that calls findBooksWithNullPageCountLinkedToRooms use
PageRequest.of(page, size) in a loop (increment page until the returned page is
empty) to process results in bounded chunks rather than loading everything at
once.
In
`@src/main/java/konkuk/thip/book/application/service/BookPageCountFillService.java`:
- Around line 49-55: The loop in BookPageCountFillService currently treats any
Integer returned from aladinApiClient.findPageCountByIsbn as a success and
applies book.changePageCount even when the value may be 0 due to JSON parsing;
add defensive validation after calling findPageCountByIsbn (e.g., if pageCount
== null || pageCount <= 0) and skip updating the book (do not call
book.changePageCount or bookCommandPort.updateForPageCount and do not increment
successCount) when the pageCount is invalid so only valid positive page counts
are recorded.
In `@src/main/java/konkuk/thip/common/scheduler/BookPageCountFillScheduler.java`:
- Around line 17-18: The scheduled method fillNullPageCounts in
BookPageCountFillScheduler (and similar scheduled methods in BookDeleteScheduler
and RoomStateScheduler) will run on every app instance; add a distributed lock
using ShedLock: add the ShedLock dependency and a Redis LockProvider bean (e.g.,
RedisLockProvider) wired to your existing Redis client, then annotate each
scheduled method (fillNullPageCounts and the corresponding methods in
BookDeleteScheduler and RoomStateScheduler) with `@SchedulerLock`(name =
"BookPageCountFillScheduler.fillNullPageCounts" / appropriate unique names) and
set lockAtLeastFor and lockAtMostFor to safe durations so only one instance runs
the job at a time.
---
Nitpick comments:
In `@src/main/java/konkuk/thip/book/application/port/out/BookCommandPort.java`:
- Around line 35-36: The port method name updateForUnfindable on the
BookCommandPort interface is implementation-centric; rename it to a
business-intent name such as markPageCountUnfindable(Book book) to make the
contract explicit, then update the BookCommandPort interface declaration, all
implementing classes and any callers/usages, plus Javadoc and tests to use
markPageCountUnfindable(Book) while keeping the method signature (void,
accepting Book) unchanged semantically.
In `@src/main/java/konkuk/thip/book/application/port/out/BookQueryPort.java`:
- Around line 23-24: BookQueryPort currently returns domain entities
(List<Book>) from findBooksWithNullPageCountLinkedToRooms which mixes query-port
responsibility with domain model; change this to return a dedicated query DTO
(e.g., BookNullPageCountDto) and update the method signature on BookQueryPort to
List<BookNullPageCountDto> findBooksWithNullPageCountLinkedToRooms(); create the
new DTO containing only the fields needed by the scheduler (id, roomId, any
pageCount-related flags) and move any domain-centric findBy... semantics back to
the Command/Repository port (e.g., BookCommandPort with findBy... returning
List<Book>) if you need the full entity elsewhere.
In
`@src/main/java/konkuk/thip/book/application/service/BookPageCountFillService.java`:
- Around line 77-79: The call to discordClient.sendPageCountFillResult in
BookPageCountFillService can throw and bubble up, making the scheduler appear to
fail after DB updates; wrap the sendPageCountFillResult(...) invocation in a
try-catch that logs the Discord delivery failure (using log.error) but does not
rethrow so the job remains successful, and optionally include the exception
details in the log to aid debugging; ensure the log.info([...]) call stays
before or is not suppressed by the catch so DB result counts (books.size(),
successCount, unfindableCount, transientFailCount) are preserved in logs even if
Discord fails.
In `@src/main/java/konkuk/thip/common/discord/DiscordClient.java`:
- Around line 29-117: The three methods sendErrorMessage,
sendAladinApiFailureAlert, and sendPageCountFillResult duplicate embed/field
creation and payload assembly; extract small helper methods (e.g.,
createField(String name, String value), buildEmbed(String title,
List<Map<String,String>> fields) that returns the embed map, and
createPayload(Map<String,Object> embed) that returns the final payload) and
replace the repeated Map construction in those three methods to call the helpers
to assemble fields, embedData and payload before calling sendSync/sendAsync.
In `@src/main/java/konkuk/thip/common/scheduler/BookPageCountFillScheduler.java`:
- Around line 18-21: The fillNullPageCounts method in BookPageCountFillScheduler
currently logs only on start and after successful completion, so if
bookPageCountFillUseCase.fillNullPageCounts() throws the end log is missing;
wrap the call in a try/finally (or try/catch/finally) around
bookPageCountFillUseCase.fillNullPageCounts() so that the "[스케줄러] pageCount 업데이트
종료" log is executed in the finally block, and if you add a catch, log the
exception (e.g., with log.error) and rethrow if needed to preserve behavior.
In `@src/main/java/konkuk/thip/config/WebClientConfig.java`:
- Around line 17-19: Replace the numeric literals in the HttpClient setup in
WebClientConfig (the HttpClient httpClient = HttpClient.create()... chain) with
named constants or configurable properties so timeouts are centralized;
introduce e.g. CONNECT_TIMEOUT_MS and RESPONSE_TIMEOUT_SEC (as private static
final constants or `@Value-injected` properties) and use them in
ChannelOption.CONNECT_TIMEOUT_MILLIS and Duration.ofSeconds(...) respectively,
then update any other HTTP client or RestTemplate timeout configuration to
reference the same constants/properties to avoid drift.
In
`@src/test/java/konkuk/thip/book/adapter/out/api/AladinApiDiscordAlertIntegrationTest.java`:
- Around line 56-58: The test currently uses Thread.sleep after calling
sendAladinApiFailureAlert() to wait for the fire-and-forget subscribe(), which
is unreliable; remove the Thread.sleep and instead mock the asynchronous
collaborator (e.g., DiscordClient) in AladinApiDiscordAlertIntegrationTest and
inject it into the class under test (or use `@MockBean`), then call
sendAladinApiFailureAlert() and use Mockito.verify() (or equivalent) to assert
that the DiscordClient method was invoked (or that the reactive subscribe path
was executed) to deterministically validate the async behavior; also ensure any
commented `@TestPropertySource` is either restored or test-specific properties are
set so the mock is actually used.
In
`@src/test/java/konkuk/thip/book/application/service/BookPageCountFillServiceTest.java`:
- Around line 40-44: The helper method bookWithIsbn only sets isbn; update it to
initialize a fuller valid Book state (e.g., set id/title/pageCount/status/author
or other required fields used by domain methods) so domain behavior like
Book.changePageCount() and Book.markAsUnfindable() can be meaningfully exercised
in tests; modify the bookWithIsbn helper (and any related test factories) to
construct a Book with those minimal fields consistent with the Book class
invariants and use the Book builder/constructors used in production so
subsequent assertions against Book instance state will be valid.
In
`@src/test/java/konkuk/thip/room/application/service/RoomCreateServiceTest.java`:
- Around line 218-223: In RoomCreateServiceTest, remove the redundant initial
stubbing of bookCommandPort.findByIsbn and replace it with a single sequential
stub so the first invocation returns Optional.empty() and the subsequent
invocation returns Optional.of(bookWithPage()); keep the existing stubs for
bookApiQueryPort.loadBookWithPageByIsbn(ISBN) to return bookWithPage() and for
bookCommandPort.save(any()) to throw new
DataIntegrityViolationException("duplicate key") to preserve the race-condition
save-then-requery scenario.
- Around line 57-61: The test currently stubs transactionTemplate.execute to
invoke the TransactionCallback but lacks call-order checks, so add an InOrder
verification to assert that transactionTemplate.execute(...) (and the
TransactionCallback execution) happens before the external API mock is invoked;
use Mockito InOrder with the transactionTemplate mock and the external API mock
(e.g., externalApiClient) and verify
transactionTemplate.execute(any(TransactionCallback.class)) was called prior to
externalApiClient.<method>() to prevent regressions where the external API is
accidentally executed inside the transaction.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (26)
src/main/java/konkuk/thip/book/adapter/out/api/CompositeBookApiAdapter.javasrc/main/java/konkuk/thip/book/adapter/out/api/naver/NaverApiUtil.javasrc/main/java/konkuk/thip/book/adapter/out/jpa/BookJpaEntity.javasrc/main/java/konkuk/thip/book/adapter/out/mapper/BookMapper.javasrc/main/java/konkuk/thip/book/adapter/out/persistence/BookCommandPersistenceAdapter.javasrc/main/java/konkuk/thip/book/adapter/out/persistence/BookQueryPersistenceAdapter.javasrc/main/java/konkuk/thip/book/adapter/out/persistence/repository/BookJpaRepository.javasrc/main/java/konkuk/thip/book/application/port/in/BookPageCountFillUseCase.javasrc/main/java/konkuk/thip/book/application/port/out/BookCommandPort.javasrc/main/java/konkuk/thip/book/application/port/out/BookQueryPort.javasrc/main/java/konkuk/thip/book/application/service/BookPageCountFillService.javasrc/main/java/konkuk/thip/book/domain/Book.javasrc/main/java/konkuk/thip/common/discord/DiscordClient.javasrc/main/java/konkuk/thip/common/exception/ExternalApiException.javasrc/main/java/konkuk/thip/common/exception/handler/GlobalExceptionHandler.javasrc/main/java/konkuk/thip/common/scheduler/BookPageCountFillScheduler.javasrc/main/java/konkuk/thip/config/AppConfig.javasrc/main/java/konkuk/thip/config/RestTemplateConfig.javasrc/main/java/konkuk/thip/config/WebClientConfig.javasrc/main/java/konkuk/thip/room/application/service/RoomCreateService.javasrc/main/resources/db/migration/V260302__Add_page_count_unfindable_and_scheduler_index_to_book.sqlsrc/test/java/konkuk/thip/book/adapter/out/api/AladinApiDiscordAlertIntegrationTest.javasrc/test/java/konkuk/thip/book/adapter/out/api/CompositeBookApiAdapterTest.javasrc/test/java/konkuk/thip/book/adapter/out/api/naver/NaverApiUtilTest.javasrc/test/java/konkuk/thip/book/application/service/BookPageCountFillServiceTest.javasrc/test/java/konkuk/thip/room/application/service/RoomCreateServiceTest.java
Comment on lines
+35
to
+41
| @Query( | ||
| "SELECT b FROM BookJpaEntity b " + | ||
| "WHERE b.pageCount IS NULL AND b.pageCountUnfindable = false " + | ||
| "AND EXISTS (SELECT r FROM RoomJpaEntity r WHERE r.bookJpaEntity = b) " + | ||
| "ORDER BY b.bookId ASC" | ||
| ) | ||
| List<BookJpaEntity> findBooksWithNullPageCountLinkedToRooms(); |
There was a problem hiding this comment.
스케줄러 입력 쿼리가 무제한 조회라서 대량 데이터에서 위험합니다.
현재 List 전체 적재 방식은 건수가 커질수록 메모리 점유와 배치 지연을 키워, 외부 API 보정 작업의 안정성을 떨어뜨릴 수 있습니다. 페이징/청크 기반으로 바꾸는 게 안전합니다.
페이징 적용 예시
+import org.springframework.data.domain.Pageable;
import java.util.List;
@@
- List<BookJpaEntity> findBooksWithNullPageCountLinkedToRooms();
+ List<BookJpaEntity> findBooksWithNullPageCountLinkedToRooms(Pageable pageable);서비스에서는 PageRequest.of(0, N)부터 반복 조회하는 방식으로 처리량을 제한하는 것을 권장합니다.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| @Query( | |
| "SELECT b FROM BookJpaEntity b " + | |
| "WHERE b.pageCount IS NULL AND b.pageCountUnfindable = false " + | |
| "AND EXISTS (SELECT r FROM RoomJpaEntity r WHERE r.bookJpaEntity = b) " + | |
| "ORDER BY b.bookId ASC" | |
| ) | |
| List<BookJpaEntity> findBooksWithNullPageCountLinkedToRooms(); | |
| `@Query`( | |
| "SELECT b FROM BookJpaEntity b " + | |
| "WHERE b.pageCount IS NULL AND b.pageCountUnfindable = false " + | |
| "AND EXISTS (SELECT r FROM RoomJpaEntity r WHERE r.bookJpaEntity = b) " + | |
| "ORDER BY b.bookId ASC" | |
| ) | |
| List<BookJpaEntity> findBooksWithNullPageCountLinkedToRooms(Pageable pageable); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/konkuk/thip/book/adapter/out/persistence/repository/BookJpaRepository.java`
around lines 35 - 41, The repository method
findBooksWithNullPageCountLinkedToRooms currently returns a List and loads all
matches into memory; change its signature in BookJpaRepository to accept a
Pageable (e.g., Pageable pageable) and return a Page<BookJpaEntity> or
Slice<BookJpaEntity> so queries can be paginated, update the `@Query` to keep the
same WHERE but remove ordering assumptions if using Slice, and then in the
service that calls findBooksWithNullPageCountLinkedToRooms use
PageRequest.of(page, size) in a loop (increment page until the returned page is
empty) to process results in bounded chunks rather than loading everything at
once.
Comment on lines
+49
to
+55
| for (Book book : books) { | ||
| try { | ||
| Integer pageCount = aladinApiClient.findPageCountByIsbn(book.getIsbn()); | ||
| book.changePageCount(pageCount); | ||
| bookCommandPort.updateForPageCount(book); | ||
| successCount++; | ||
| log.info("[스케줄러] pageCount 업데이트 완료 - isbn: {}, pageCount: {}", book.getIsbn(), pageCount); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "*AladinApiClient*"Repository: THIP-TextHip/THIP-Server
Length of output: 144
🏁 Script executed:
rg -n "findPageCountByIsbn" --type java -B 2 -A 15Repository: THIP-TextHip/THIP-Server
Length of output: 41269
🏁 Script executed:
rg -n "changePageCount" --type java -B 2 -A 10Repository: THIP-TextHip/THIP-Server
Length of output: 6686
🏁 Script executed:
find . -type f -name "*AladinApiUtil*"Repository: THIP-TextHip/THIP-Server
Length of output: 142
🏁 Script executed:
cat -n ./src/main/java/konkuk/thip/book/adapter/out/api/aladin/AladinApiClient.javaRepository: THIP-TextHip/THIP-Server
Length of output: 538
🏁 Script executed:
cat -n ./src/main/java/konkuk/thip/book/adapter/out/api/aladin/AladinApiUtil.javaRepository: THIP-TextHip/THIP-Server
Length of output: 2848
🏁 Script executed:
cat -n ./src/main/java/konkuk/thip/book/application/service/BookPageCountFillService.java | head -80Repository: THIP-TextHip/THIP-Server
Length of output: 3981
AladinApiUtil.getPageCount()이 예외를 던지지 않을 때도 방어적 검증 필요
현재 AladinApiUtil.getPageCount()는 item이 없으면 ExternalApiException을 던지고, 그 외의 경우 JsonNode.asInt()로 pageCount를 반환합니다. 그런데 asInt()는 경로가 존재하지 않을 때 null이 아닌 0을 반환하기 때문에, 데이터 파싱 이상으로 0이 반환되어도 book.changePageCount(0)이 호출되고 성공으로 카운트됩니다.
RoomCreateService에서는 if (pageCount == null) return;로 방어하는 것처럼, BookPageCountFillService도 Integer 반환값에 대한 방어적 검증(null 체크 또는 유효성 검사)을 추가해야 합니다:
Integer pageCount = aladinApiClient.findPageCountByIsbn(book.getIsbn());
if (pageCount == null || pageCount <= 0) return; // 또는 적절한 처리
book.changePageCount(pageCount);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/konkuk/thip/book/application/service/BookPageCountFillService.java`
around lines 49 - 55, The loop in BookPageCountFillService currently treats any
Integer returned from aladinApiClient.findPageCountByIsbn as a success and
applies book.changePageCount even when the value may be 0 due to JSON parsing;
add defensive validation after calling findPageCountByIsbn (e.g., if pageCount
== null || pageCount <= 0) and skip updating the book (do not call
book.changePageCount or bookCommandPort.updateForPageCount and do not increment
successCount) when the pageCount is invalid so only valid positive page counts
are recorded.
Comment on lines
+17
to
+18
| @Scheduled(cron = "0 0 3 * * *", zone = "Asia/Seoul") | ||
| public void fillNullPageCounts() { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== 스케줄링/락 관련 코드 확인 =="
rg -n -C2 '@Scheduled|@SchedulerLock|EnableSchedulerLock|LockProvider|ShedLock|Quartz'
echo
echo "== 배포 스케일아웃 힌트 확인 =="
rg -n -C2 'replicas:|HorizontalPodAutoscaler|maxReplicas|minReplicas|autoscaling'
echo
echo "== BookPageCountFillScheduler 파일 내용 확인 =="
cat -n src/main/java/konkuk/thip/common/scheduler/BookPageCountFillScheduler.java
echo
echo "== pom.xml 또는 build.gradle에서 ShedLock 의존성 확인 =="
rg -n 'shedlock|ShedLock' --type xml --type gradleRepository: THIP-TextHip/THIP-Server
Length of output: 2288
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== BookPageCountFillScheduler.java 파일 내용 =="
cat -n src/main/java/konkuk/thip/common/scheduler/BookPageCountFillScheduler.java
echo
echo "== 배포 설정 파일 확인 (Kubernetes, Docker Compose, etc.) =="
fd -e yaml -e yml -e json | grep -E '(deployment|docker-compose|k8s|manifest)' | head -20
echo
echo "== application.yml/properties에서 스케줄링 관련 설정 =="
find . -name "application*.yml" -o -name "application*.yaml" -o -name "application*.properties" | head -10 | xargs cat -n 2>/dev/null || true
echo
echo "== 분산 락 설정이 있는지 Spring 설정 클래스 확인 =="
rg -n 'LockConfiguration|@Configuration|LockProvider|SchedulerLock' -A5 --type java | head -50Repository: THIP-TextHip/THIP-Server
Length of output: 1078
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== application 설정 파일 찾기 =="
find . -type f \( -name "application*.yml" -o -name "application*.yaml" -o -name "application*.properties" \) 2>/dev/null
echo
echo "== 프로젝트 루트의 설정/배포 파일 =="
ls -la | grep -E '\.(yml|yaml|docker|k8s|compose)' || echo "No deployment files in root"
echo
echo "== Docker/Kubernetes 관련 파일 =="
find . -type f \( -name "Dockerfile*" -o -name "*docker-compose*" -o -name "*.k8s.*" -o -name "*deployment*" -o -name "*statefulset*" \) 2>/dev/null | head -20
echo
echo "== pom.xml에서 분산 락/클러스터링 관련 의존성 =="
if [ -f pom.xml ]; then
cat -n pom.xml | grep -i -A2 -B2 'shedlock\|redisson\|lettuce\|hazelcast\|spring-cloud\|spring-boot-starter-data-redis' || echo "No distributed lock dependencies found"
else
echo "pom.xml not found"
fi
echo
echo "== application.yml 내용 확인 =="
find . -name "application.yml" -o -name "application.yaml" | head -1 | xargs cat -n 2>/dev/null || echo "No application.yml found"Repository: THIP-TextHip/THIP-Server
Length of output: 273
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== build.gradle 확인 =="
find . -name "build.gradle" -o -name "build.gradle.kts" | head -5 | xargs cat -n 2>/dev/null || echo "No build.gradle found"
echo
echo "== Dockerfile 내용 =="
if [ -f ./Dockerfile ]; then
cat -n ./Dockerfile
else
echo "Dockerfile not found"
fi
echo
echo "== src/main/resources에서 application 설정 파일 찾기 =="
find src/main/resources -type f \( -name "application*.yml" -o -name "application*.yaml" -o -name "application*.properties" \) 2>/dev/null | head -5
echo
echo "== 발견된 application 설정 파일 내용 =="
find src/main/resources -type f \( -name "application*.yml" -o -name "application*.yaml" -o -name "application*.properties" \) 2>/dev/null | head -1 | xargs cat -n 2>/dev/null || echo "No application config found"
echo
echo "== 모든 스케줄러 파일 목록 =="
find . -path "*/scheduler/*.java" -type f | head -20Repository: THIP-TextHip/THIP-Server
Length of output: 5248
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== 모든 위치에서 application 파일 찾기 =="
find . -type f -name "application*" 2>/dev/null | head -20
echo
echo "== 코드베이스에서 `@SchedulerLock` 재검색 =="
rg -n '@SchedulerLock|@Lock|@Transactional.*readOnly|synchronized' --type java
echo
echo "== 다른 스케줄러 파일들의 내용 (분산 락 여부 확인) =="
echo "### BookDeleteScheduler.java ###"
cat -n ./src/main/java/konkuk/thip/common/scheduler/BookDeleteScheduler.java
echo
echo "### RoomStateScheduler.java ###"
cat -n ./src/main/java/konkuk/thip/common/scheduler/RoomStateScheduler.java
echo
echo "== 설정 클래스에서 스케줄링 설정 확인 =="
rg -n '@Configuration|@EnableScheduling|SchedulingConfigurer' --type java -A5 | head -40Repository: THIP-TextHip/THIP-Server
Length of output: 11959
다중 인스턴스 환경에서 모든 스케줄러의 중복 실행 위험이 있습니다.
@Scheduled는 애플리케이션 인스턴스마다 독립적으로 실행됩니다. 코드베이스에 분산 락 구성(@SchedulerLock, ShedLock 등)이 없으며, Dockerfile을 통해 컨테이너화되어 있어 scale-out 환경에서 인스턴스 수만큼 중복 실행될 수 있습니다.
이는 BookPageCountFillScheduler뿐 아니라 BookDeleteScheduler, RoomStateScheduler 모두에 해당합니다. Redis가 의존성에 포함되어 있으므로 ShedLock을 추가하여 분산 락을 구성하는 것을 권장합니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/konkuk/thip/common/scheduler/BookPageCountFillScheduler.java`
around lines 17 - 18, The scheduled method fillNullPageCounts in
BookPageCountFillScheduler (and similar scheduled methods in BookDeleteScheduler
and RoomStateScheduler) will run on every app instance; add a distributed lock
using ShedLock: add the ShedLock dependency and a Redis LockProvider bean (e.g.,
RedisLockProvider) wired to your existing Redis client, then annotate each
scheduled method (fillNullPageCounts and the corresponding methods in
BookDeleteScheduler and RoomStateScheduler) with `@SchedulerLock`(name =
"BookPageCountFillScheduler.fillNullPageCounts" / appropriate unique names) and
set lockAtLeastFor and lockAtMostFor to safe durations so only one instance runs
the job at a time.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
#️⃣ 연관된 이슈
📝 작업 내용
외부 API(Naver, Aladin) 지연이 내부 시스템 자원(DB 커넥션, 톰캣 쓰레드) 고갈로 이어지는 연쇄 장애(Cascading Failure)를 차단하고, 방 생성 도메인의 가용성을 확보하기 위해 아키텍처를 개선했습니다.
트랜잭션 경계 분리 및 타임아웃 적용
@Transactional을 제거하고TransactionTemplate을 도입하여 외부 API 호출 구간을 DB 트랜잭션 밖으로 분리RestTemplate및WebClient에 명시적 Timeout(Connect 3s, Read 5s) 설정도메인 특성을 반영한 장애 격리 (Fault Isolation)
pageCount = null로 격리하여 방 생성 속행(Fail-soft)subscribe()) 알림 전송최종적 일관성(Eventual Consistency) 보장
BookPageCountFillScheduler도입으로 누락된pageCount복구 (외부 API 쓰로틀링 적용)ISBN_NOT_FOUND)는pageCountUnfindable = true플래그로 업데이트하여 무의미한 API 반복 호출(좀비 프로세스) 영구 차단동시성 예외 제어
DataIntegrityViolationException예외를 캐치하여 기존 데이터 재조회로 처리📸 스크린샷
💬 리뷰 요구사항
TransactionTemplate내부로 묶인 DB 쓰기 작업의 트랜잭션 경계가 적절히 최소화되었는지 검토 바랍니다.📌 PR 진행 시 이러한 점들을 참고해 주세요
Summary by CodeRabbit
릴리스 노트
새로운 기능
버그 수정