Conversation
- 생성할 방과 연관된 책의 pageCount 값을 로드하기 위한 메서드 시그니처 추가
- CompositionBookApiAdapter 내부에서 네이버, 알라딘 api와의 통신 결과를 결합할 수 있도록 api adapter 구성
- restTemplate 활용하여 통신하도록 구현
- @nested 어노테이션 활용해서 테스트코드 모듈화 진행
- 방 공개 여부, 비밀번호 설정 여부의 일관성을 검증한 후, 방 생성 진행할 수 있도록 테스트 코드 작성
|
""" Walkthrough방 생성 API가 새롭게 도입되어, 도메인, 어댑터, 서비스, 포트, 예외, 테스트 등 전반적으로 관련 코드가 추가·수정되었습니다. 방 생성 요청의 유효성 검증, 도메인 생성 규칙, 외부 도서 API 연동, JPA 엔티티 매핑, 통합 및 단위 테스트까지 전체적인 방 생성 플로우가 구현되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant RoomCommandController
participant RoomCreateService
participant CategoryCommandPort
participant BookCommandPort
participant BookApiQueryPort
participant RoomCommandPort
Client->>RoomCommandController: POST /rooms (RoomCreateRequest)
RoomCommandController->>RoomCreateService: createRoom(RoomCreateCommand)
RoomCreateService->>CategoryCommandPort: findByValue(category)
RoomCreateService->>BookCommandPort: findByIsbn(isbn)
alt Book exists
alt Book has no pageCount
RoomCreateService->>BookApiQueryPort: findPageCountByIsbn(isbn)
RoomCreateService->>BookCommandPort: updateForPageCount(book)
end
else Book not exists
RoomCreateService->>BookApiQueryPort: loadBookWithPageByIsbn(isbn)
RoomCreateService->>BookCommandPort: save(book)
end
RoomCreateService->>RoomCommandPort: save(room)
RoomCommandController-->>Client: BaseResponse<RoomCreateResponse>
Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
(범위 외 변경 없음) Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (8)
src/main/java/konkuk/thip/book/adapter/out/jpa/BookJpaEntity.java (1)
42-44: 변경 메서드의 도메인 의도 명확화 필요현재
changePageCount메서드는 단순한 setter 역할을 하고 있습니다. 도메인 의도를 더 명확히 드러내는 메서드명을 고려해보세요.-public void changePageCount(Integer pageCount) { +public void updatePageCountFromExternalApi(Integer pageCount) { this.pageCount = pageCount; }또는 도메인 이벤트를 발생시키는 것도 고려해볼 수 있습니다.
src/main/java/konkuk/thip/book/application/port/out/BookCommandPort.java (1)
14-14: 메서드 반환 타입 개선 고려
updateForPageCount메서드가 void를 반환하여 업데이트 성공/실패를 확인하기 어렵습니다. 업데이트 결과를 반환하는 것을 고려해보세요.-void updateForPageCount(Book book); +boolean updateForPageCount(Book book);또는 예외를 통한 에러 처리를 명확히 문서화하는 것도 좋습니다.
src/main/java/konkuk/thip/room/application/port/out/RoomCommandPort.java (1)
9-9: save 메서드 시그니처 개선 제안현재 save 메서드가 Long ID만 반환하고 있습니다. 저장된 엔티티의 완전한 정보가 필요할 경우를 고려해보세요.
-Long save(Room room); +Room save(Room room);또는 저장 실패 시 예외 처리 방식을 명확히 문서화하는 것을 권장합니다.
src/main/java/konkuk/thip/book/adapter/out/api/aladin/AladinApiClient.java (1)
12-14: 메서드명 오타 수정 필요메서드명
findPageCountByIsb에서 "Isb"는 "Isbn"의 오타로 보입니다. 일관성을 위해 수정이 필요합니다.-public Integer findPageCountByIsb(String isbn) { +public Integer findPageCountByIsbn(String isbn) { return aladinApiUtil.getPageCount(isbn); }src/main/java/konkuk/thip/room/application/port/in/dto/RoomCreateCommand.java (1)
22-22: Boolean 타입 대신 boolean 기본 타입 사용을 고려하세요.
isPublic필드에 Boolean 래퍼 타입을 사용하고 있는데, 이 필드는 필수 값이므로 null이 될 이유가 없습니다. boolean 기본 타입을 사용하는 것이 더 적절합니다.- Boolean isPublic + boolean isPublicsrc/test/java/konkuk/thip/room/domain/RoomTest.java (1)
11-44: 성공 케이스 테스트 추가를 고려하세요.현재 검증 실패 케이스만 테스트하고 있습니다. 정상적인 방 생성 케이스(공개방-비밀번호 null, 비공개방-비밀번호 존재)도 테스트로 추가하면 더 완전한 테스트 커버리지를 확보할 수 있습니다.
src/main/java/konkuk/thip/room/adapter/out/persistence/RoomCommandPersistenceAdapter.java (1)
37-43: 연관 엔티티 조회 성능 개선 검토Book과 Category 엔티티를 순차적으로 조회하고 있습니다. 동시 조회나 배치 조회를 통해 성능을 개선할 수 있습니다.
- BookJpaEntity bookJpaEntity = bookJpaRepository.findById(room.getBookId()).orElseThrow( - () -> new EntityNotFoundException(BOOK_NOT_FOUND) - ); - - CategoryJpaEntity categoryJpaEntity = categoryJpaRepository.findById(room.getCategoryId()).orElseThrow( - () -> new EntityNotFoundException(CATEGORY_NOT_FOUND) - ); + // CompletableFuture를 사용한 병렬 조회 + CompletableFuture<BookJpaEntity> bookFuture = CompletableFuture.supplyAsync(() -> + bookJpaRepository.findById(room.getBookId()).orElseThrow( + () -> new EntityNotFoundException(BOOK_NOT_FOUND) + )); + + CompletableFuture<CategoryJpaEntity> categoryFuture = CompletableFuture.supplyAsync(() -> + categoryJpaRepository.findById(room.getCategoryId()).orElseThrow( + () -> new EntityNotFoundException(CATEGORY_NOT_FOUND) + )); + + BookJpaEntity bookJpaEntity = bookFuture.join(); + CategoryJpaEntity categoryJpaEntity = categoryFuture.join();src/main/java/konkuk/thip/book/adapter/out/api/aladin/AladinApiUtil.java (1)
52-55: TODO: 페이지 정보 없음 처리 구현 필요알라딘 API에서 페이지 정보를 찾을 수 없는 경우의 처리 방안이 필요합니다. 사용자에게 직접 입력을 요청하는 보상 시나리오가 제안되어 있습니다.
이 TODO를 처리하는 구현을 작성해드릴까요? 새로운 이슈로 등록하시겠습니까?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (34)
src/main/java/konkuk/thip/book/adapter/out/api/BookApiNaverApiAdapter.java(1 hunks)src/main/java/konkuk/thip/book/adapter/out/api/CompositeBookApiAdapter.java(1 hunks)src/main/java/konkuk/thip/book/adapter/out/api/NaverBookXmlParser.java(2 hunks)src/main/java/konkuk/thip/book/adapter/out/api/aladin/AladinApiClient.java(1 hunks)src/main/java/konkuk/thip/book/adapter/out/api/aladin/AladinApiUtil.java(1 hunks)src/main/java/konkuk/thip/book/adapter/out/jpa/BookJpaEntity.java(1 hunks)src/main/java/konkuk/thip/book/adapter/out/persistence/BookCommandPersistenceAdapter.java(1 hunks)src/main/java/konkuk/thip/book/application/port/out/BookApiQueryPort.java(1 hunks)src/main/java/konkuk/thip/book/application/port/out/BookCommandPort.java(1 hunks)src/main/java/konkuk/thip/book/domain/Book.java(1 hunks)src/main/java/konkuk/thip/common/exception/code/ErrorCode.java(1 hunks)src/main/java/konkuk/thip/config/RestTemplateConfig.java(1 hunks)src/main/java/konkuk/thip/room/adapter/in/web/RoomCommandController.java(1 hunks)src/main/java/konkuk/thip/room/adapter/in/web/request/DummyRequest.java(0 hunks)src/main/java/konkuk/thip/room/adapter/in/web/request/RoomCreateRequest.java(1 hunks)src/main/java/konkuk/thip/room/adapter/in/web/response/DummyResponse.java(0 hunks)src/main/java/konkuk/thip/room/adapter/in/web/response/RoomCreateResponse.java(1 hunks)src/main/java/konkuk/thip/room/adapter/out/jpa/RoomJpaEntity.java(1 hunks)src/main/java/konkuk/thip/room/adapter/out/mapper/RoomMapper.java(1 hunks)src/main/java/konkuk/thip/room/adapter/out/persistence/CategoryCommandPersistenceAdapter.java(1 hunks)src/main/java/konkuk/thip/room/adapter/out/persistence/CategoryJpaRepository.java(1 hunks)src/main/java/konkuk/thip/room/adapter/out/persistence/RoomCommandPersistenceAdapter.java(2 hunks)src/main/java/konkuk/thip/room/application/port/in/DummyUseCase.java(0 hunks)src/main/java/konkuk/thip/room/application/port/in/RoomCreateUseCase.java(1 hunks)src/main/java/konkuk/thip/room/application/port/in/dto/DummyCommand.java(0 hunks)src/main/java/konkuk/thip/room/application/port/in/dto/RoomCreateCommand.java(1 hunks)src/main/java/konkuk/thip/room/application/port/out/CategoryCommandPort.java(1 hunks)src/main/java/konkuk/thip/room/application/port/out/RoomCommandPort.java(1 hunks)src/main/java/konkuk/thip/room/application/service/RoomCreateService.java(1 hunks)src/main/java/konkuk/thip/room/application/service/RoomService.java(0 hunks)src/main/java/konkuk/thip/room/domain/Room.java(3 hunks)src/test/java/konkuk/thip/room/adapter/in/web/RoomCreateAPITest.java(1 hunks)src/test/java/konkuk/thip/room/adapter/in/web/RoomCreateControllerTest.java(1 hunks)src/test/java/konkuk/thip/room/domain/RoomTest.java(1 hunks)
💤 Files with no reviewable changes (5)
- src/main/java/konkuk/thip/room/application/port/in/DummyUseCase.java
- src/main/java/konkuk/thip/room/application/port/in/dto/DummyCommand.java
- src/main/java/konkuk/thip/room/adapter/in/web/request/DummyRequest.java
- src/main/java/konkuk/thip/room/adapter/in/web/response/DummyResponse.java
- src/main/java/konkuk/thip/room/application/service/RoomService.java
🧰 Additional context used
🧠 Learnings (9)
src/main/java/konkuk/thip/book/application/port/out/BookCommandPort.java (1)
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#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로부터 조회하는 메서드를 추가함.
src/main/java/konkuk/thip/room/application/port/out/RoomCommandPort.java (1)
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#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로부터 조회하는 메서드를 추가함.
src/main/java/konkuk/thip/room/application/port/in/RoomCreateUseCase.java (1)
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#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로부터 조회하는 메서드를 추가함.
src/main/java/konkuk/thip/room/application/port/out/CategoryCommandPort.java (1)
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#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로부터 조회하는 메서드를 추가함.
src/main/java/konkuk/thip/room/adapter/out/persistence/CategoryJpaRepository.java (1)
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#36
File: src/main/java/konkuk/thip/user/adapter/out/persistence/UserJpaRepository.java:7-7
Timestamp: 2025-06-29T09:47:31.299Z
Learning: Spring Data JPA에서 findBy{FieldName} 패턴의 메서드는 명시적 선언 없이 자동으로 생성되며, Optional<Entity> 반환 타입을 사용하는 것이 null 안전성을 위해 권장됩니다.
src/main/java/konkuk/thip/room/adapter/out/persistence/CategoryCommandPersistenceAdapter.java (1)
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#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로부터 조회하는 메서드를 추가함.
src/main/java/konkuk/thip/book/application/port/out/BookApiQueryPort.java (1)
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#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로부터 조회하는 메서드를 추가함.
src/main/java/konkuk/thip/book/adapter/out/api/CompositeBookApiAdapter.java (1)
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#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로부터 조회하는 메서드를 추가함.
src/main/java/konkuk/thip/room/adapter/out/persistence/RoomCommandPersistenceAdapter.java (1)
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#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로부터 조회하는 메서드를 추가함.
🧬 Code Graph Analysis (5)
src/main/java/konkuk/thip/room/adapter/out/persistence/CategoryCommandPersistenceAdapter.java (1)
src/main/java/konkuk/thip/common/exception/EntityNotFoundException.java (1)
EntityNotFoundException(5-10)
src/main/java/konkuk/thip/room/application/service/RoomCreateService.java (2)
src/main/java/konkuk/thip/common/exception/EntityNotFoundException.java (1)
EntityNotFoundException(5-10)src/main/java/konkuk/thip/book/adapter/out/api/NaverApiUtil.java (1)
RequiredArgsConstructor(18-123)
src/test/java/konkuk/thip/room/adapter/in/web/RoomCreateAPITest.java (1)
src/test/java/konkuk/thip/room/domain/RoomTest.java (1)
DisplayName(11-44)
src/test/java/konkuk/thip/room/adapter/in/web/RoomCreateControllerTest.java (1)
src/test/java/konkuk/thip/room/domain/RoomTest.java (1)
DisplayName(11-44)
src/main/java/konkuk/thip/book/adapter/out/api/aladin/AladinApiUtil.java (1)
src/main/java/konkuk/thip/book/adapter/out/api/NaverApiUtil.java (1)
RequiredArgsConstructor(18-123)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (41)
src/main/java/konkuk/thip/book/adapter/out/api/NaverBookXmlParser.java (2)
14-14: 에러 코드 개선이 적절합니다.기존
BOOK_ISBN_NOT_FOUND에서BOOK_NAVER_API_ISBN_NOT_FOUND로 변경한 것은 에러의 출처를 명확히 하여 디버깅과 에러 처리를 개선합니다.
60-60: 에러 코드 사용이 일관성 있게 변경되었습니다.새로운 에러 코드를 정확하게 사용하여 Naver API에서 ISBN을 찾을 수 없는 상황을 명확히 표현합니다.
src/main/java/konkuk/thip/room/adapter/out/persistence/CategoryJpaRepository.java (2)
6-6: 적절한 import 추가입니다.
Optional타입을 사용하기 위한 import가 올바르게 추가되었습니다.
10-10: Spring Data JPA 컨벤션에 맞는 메서드 정의입니다.
findByValue메서드는 Spring Data JPA의 쿼리 메서드 네이밍 컨벤션을 따르고 있으며,Optional<CategoryJpaEntity>반환 타입을 사용하여 null 안전성을 보장합니다. 이는 이전 학습 내용과도 일치합니다.src/main/java/konkuk/thip/room/application/port/out/CategoryCommandPort.java (1)
1-8: CQRS 패턴에 맞는 CommandPort 설계입니다.이 인터페이스는 프로젝트의 CQRS 컨벤션을 잘 따르고 있습니다.
findByValue메서드를 통해 도메인 엔티티를 찾아오는 기능을 제공하며, CommandPort의 역할에 적합합니다.src/main/java/konkuk/thip/room/application/port/in/RoomCreateUseCase.java (1)
1-8: 클린 아키텍처 원칙을 잘 따른 UseCase 인터페이스입니다.
createRoom메서드는 명확한 입력(RoomCreateCommand)과 출력(Long)을 가지고 있으며, 방 생성 유스케이스의 계약을 잘 표현합니다. 반환 타입이Long인 것으로 보아 생성된 방의 식별자를 반환하는 것으로 보입니다.src/main/java/konkuk/thip/book/adapter/out/api/BookApiNaverApiAdapter.java (3)
11-11: 인터페이스 구현을 제거한 리팩토링이 적절합니다.
BookApiQueryPort인터페이스 구현을 제거하고CompositeBookApiAdapter에 위임하는 구조로 변경한 것은 좋은 설계입니다. 이를 통해 여러 책 API를 조합할 수 있는 유연성을 확보했습니다.
15-18: 메서드 구현 로직은 그대로 유지되어 안정성을 보장합니다.
@Override어노테이션만 제거되고 기존 로직은 그대로 유지되어 기능적 변경 없이 아키텍처만 개선되었습니다.
20-23: 상세 검색 메서드도 안전하게 리팩토링되었습니다.
findDetailBookByKeyword메서드 역시 기존 로직을 유지하면서 인터페이스 구현 책임만 분리되어 좋은 구조적 개선을 보여줍니다.src/main/java/konkuk/thip/room/adapter/in/web/response/RoomCreateResponse.java (1)
1-7: 깔끔한 Record 클래스 구현Record 클래스와 정적 팩토리 메서드를 사용한 간결하고 명확한 구현입니다. 불변성이 보장되고 의도가 분명합니다.
src/main/java/konkuk/thip/room/adapter/out/mapper/RoomMapper.java (1)
12-12: 메서드명 개선으로 가독성 향상
roomJpaEntity에서toJpaEntity로 변경하여 매퍼 클래스의 일관된 네이밍 컨벤션을 적용한 좋은 개선입니다.src/main/java/konkuk/thip/room/application/port/in/dto/RoomCreateCommand.java (1)
5-24: 잘 구성된 커맨드 DTO입니다.방 생성에 필요한 모든 필드가 적절히 정의되어 있고, 레코드 클래스를 사용하여 불변 객체로 설계된 점이 좋습니다. 필드 타입과 명명도 적절합니다.
src/main/java/konkuk/thip/room/adapter/out/persistence/CategoryCommandPersistenceAdapter.java (2)
13-18: 적절한 의존성 구성과 어노테이션 사용입니다.Spring의 @repository 어노테이션과 Lombok의 @requiredargsconstructor를 사용하여 깔끔하게 의존성을 구성했습니다. CategoryJpaRepository와 CategoryMapper의 조합도 적절합니다.
20-27: 예외 처리와 매핑 로직이 잘 구현되었습니다.Optional.orElseThrow를 사용한 예외 처리가 깔끔하고, 커스텀 예외 타입과 에러 코드를 사용한 점이 좋습니다. 도메인 매핑 로직도 적절히 분리되어 있습니다. 또한 학습된 CQRS 컨벤션에 따라 CommandPort에 findByXXX 메서드가 적절히 구현되었습니다.
src/test/java/konkuk/thip/room/domain/RoomTest.java (2)
17-29: 공개방-비밀번호 검증 로직이 잘 테스트되었습니다.공개방에서 비밀번호가 설정된 경우의 예외 처리가 정확히 검증되었습니다. 예외 타입, 원인 예외, 메시지 내용까지 상세하게 검증하는 점이 좋습니다.
31-43: 비공개방-비밀번호 검증 로직이 잘 테스트되었습니다.비공개방에서 비밀번호가 없는 경우의 예외 처리가 정확히 검증되었습니다. 첫 번째 테스트와 일관된 패턴으로 작성되어 있습니다.
src/main/java/konkuk/thip/room/adapter/in/web/RoomCommandController.java (2)
17-17: 적절한 의존성 주입입니다.RoomCreateUseCase를 주입받아 비즈니스 로직을 위임하는 구조가 좋습니다. 컨트롤러의 역할을 명확히 분리했습니다.
19-24: REST API 엔드포인트가 잘 구현되었습니다.@PostMapping을 사용한 엔드포인트 정의, @Valid를 통한 요청 검증, BaseResponse를 사용한 일관된 응답 래핑이 모두 적절합니다. request.toCommand() 메서드를 통한 DTO 변환도 깔끔합니다.
src/main/java/konkuk/thip/book/domain/Book.java (4)
58-62: toBuilder() 헬퍼 메서드 활용이 좋습니다.기존 withId() 메서드를 toBuilder()를 사용하도록 리팩토링한 점이 좋습니다. 코드 중복을 줄이고 일관성을 높였습니다.
64-66: 페이지 수 존재 여부 확인 로직을 검토하세요.hasPageCount() 메서드에서
pageCount > 0조건을 사용하고 있는데, 0페이지 책이 유효한 경우가 있는지 도메인 전문가와 확인해보세요. 일반적으로 책의 페이지 수는 1페이지 이상이어야 합니다.
68-72: 불변 객체 업데이트 패턴이 잘 구현되었습니다.changePageCount() 메서드가 기존 객체를 변경하지 않고 새 인스턴스를 반환하는 불변 객체 패턴을 올바르게 구현했습니다.
74-85: 코드 중복을 효과적으로 제거했습니다.toBuilder() 헬퍼 메서드를 도입하여 객체 복사 로직의 중복을 제거한 점이 훌륭합니다. 모든 필드를 빠뜨리지 않고 복사하여 안전성도 보장했습니다.
src/main/java/konkuk/thip/room/application/service/RoomCreateService.java (2)
41-41: roomPercentage 초기값 확인 필요roomPercentage가 0으로 고정되어 있습니다. 이것이 의도된 초기값인지 확인이 필요합니다.
방 생성 시 진행률이 항상 0%로 시작하는 것이 비즈니스 요구사항과 일치하는지 확인해주세요.
52-73: 깔끔한 헬퍼 메서드 구현책 조회 및 페이지 수 확인 로직이 잘 분리되어 있고, 예외 처리가 적절합니다.
src/main/java/konkuk/thip/room/domain/Room.java (1)
38-67: 우수한 도메인 검증 로직 구현공개/비공개 방과 비밀번호 설정 간의 일관성을 검증하는 로직이 명확하고 에러 메시지가 상세합니다.
src/main/java/konkuk/thip/book/adapter/out/api/aladin/AladinApiUtil.java (1)
28-28: 보안 경고: 알라딘 OpenAPI는 현재 HTTP만 지원알라딘 OpenAPI는 공식 문서에서 HTTP 프로토콜(예:
http://www.aladin.co.kr/ttb/api/…)로만 제공되는 것으로 확인되었습니다. HTTPS 지원 여부가 명확하지 않으므로, 보안을 강화하려면 아래 항목을 검토해 주세요:
- 알라딘 개발자 지원팀 또는 고객센터에 HTTPS 지원 계획 문의
- 자체 프록시 서버 또는 API 게이트웨이를 도입해 HTTP 호출을 HTTPS로 래핑
대상 위치
- 파일: src/main/java/konkuk/thip/book/adapter/out/api/aladin/AladinApiUtil.java
- 상수:
BASE_URLsrc/main/java/konkuk/thip/book/adapter/out/api/CompositeBookApiAdapter.java (1)
46-46: bestSeller 매개변수 고정값(false) 확인 필요CompositeBookApiAdapter에서 호출하는
Book.withoutId의 네 번째 인자는 도메인 상의bestSeller필드를 의미합니다. 현재 Naver API 결과에는 베스트셀러 여부 정보가 없어 기본값으로false를 설정한 것으로 보입니다.
이 값이 항상false여야 하는지, 혹은 추후 베스트셀러 정보 수집 로직이 추가될 때 변경이 필요할지 확인 부탁드립니다.
- 파일: src/main/java/konkuk/thip/book/adapter/out/api/CompositeBookApiAdapter.java
- 위치: 42–46행,
Book.withoutId(..., false, ...)src/test/java/konkuk/thip/room/adapter/in/web/RoomCreateAPITest.java (6)
1-42: 통합 테스트 설정이 적절합니다.클래스 구조와 어노테이션 설정이 통합 테스트에 적합하게 구성되어 있습니다.
@AutoConfigureMockMvc(addFilters = false)설정으로 인증 필터를 비활성화한 것도 테스트 환경에 적절합니다.
65-72: 엔티티 삭제 순서가 참조 무결성을 고려하고 있습니다.tearDown 메서드에서 외래키 관계를 고려하여 room → book → user → category → alias 순서로 삭제하는 것이 적절합니다.
121-133: 테스트 데이터 생성 메서드가 잘 구성되어 있습니다.buildRoomCreateRequest() 메서드가 재사용 가능하고 명확하게 구성되어 있습니다. 공개 방에 password가 null인 것도 도메인 규칙에 맞습니다.
135-178: 첫 번째 테스트 시나리오 구현이 적절합니다.페이지 수가 있는 책이 존재하는 경우의 테스트가 잘 구현되어 있습니다. 날짜 파싱과 엔티티 검증 로직이 정확합니다.
180-227: 두 번째 테스트 시나리오와 책 업데이트 검증이 우수합니다.페이지 수가 없는 책의 경우, 외부 API를 통해 페이지 정보를 업데이트하는 시나리오가 잘 테스트되어 있습니다. 특히 line 225-226의 업데이트된 책 검증이 중요한 테스트 케이스입니다.
229-280: 세 번째 테스트 시나리오가 완전한 플로우를 검증합니다.책이 존재하지 않는 경우, 외부 API를 통해 책 정보를 가져와 저장하는 플로우가 잘 테스트되어 있습니다. 새로 저장된 책의 검증 로직(lines 272-279)이 적절합니다.
src/main/java/konkuk/thip/room/adapter/in/web/request/RoomCreateRequest.java (1)
10-44: Record 클래스 구조와 유효성 검증이 적절합니다.필드별 Bean Validation 어노테이션이 적절히 적용되어 있습니다. 특히 날짜 형식 패턴 검증과 비밀번호 형식 검증이 명확하게 정의되어 있습니다.
src/test/java/konkuk/thip/room/adapter/in/web/RoomCreateControllerTest.java (2)
22-56: 테스트 클래스 구조와 도우미 메서드가 잘 구성되어 있습니다.MockMvc를 사용한 컨트롤러 테스트 설정이 적절하고,
buildValidRequest()와assertBad()도우미 메서드가 코드 중복을 줄이고 가독성을 높입니다.
58-186: 각 필드별 입력 검증 테스트가 체계적으로 구성되어 있습니다.Nested 클래스를 사용하여 각 필드별로 테스트를 분리한 것이 좋습니다. 모든 필드의 기본적인 검증 규칙이 적절히 테스트되고 있습니다.
src/main/java/konkuk/thip/common/exception/code/ErrorCode.java (5)
43-43: 네이버 API 파싱 에러의 HTTP 상태 코드 변경이 적절합니다.
BOOK_NAVER_API_PARSING_ERROR를BAD_REQUEST에서INTERNAL_SERVER_ERROR로 변경한 것이 적절합니다. 파싱 오류는 클라이언트 요청 문제가 아닌 서버 내부 처리 문제입니다.
50-50: 네이버 API 관련 에러 코드 이름 변경이 명확합니다.
BOOK_ISBN_NOT_FOUND를BOOK_NAVER_API_ISBN_NOT_FOUND로 변경하여 어떤 API에서 발생한 오류인지 명확히 구분됩니다.
56-57: 알라딘 API 관련 에러 코드 추가가 적절합니다.새로운 알라딘 API 통합에 맞춰
BOOK_ALADIN_API_PARSING_ERROR와BOOK_ALADIN_API_ISBN_NOT_FOUND에러 코드를 추가한 것이 적절합니다. HTTP 상태 코드도 네이버 API 에러와 일관성 있게 설정되어 있습니다.
68-68: 방 생성 관련 에러 코드 추가가 적절합니다.
INVALID_ROOM_CREATE에러 코드 추가가 새로운 방 생성 기능에 적절합니다.BAD_REQUEST상태 코드도 적절합니다.
77-80: 카테고리 관련 에러 코드 추가가 적절합니다.새로운 카테고리 섹션과
CATEGORY_NOT_FOUND에러 코드 추가가 방 생성 기능에서 카테고리 검증을 위해 필요한 추가입니다.NOT_FOUND상태 코드도 적절합니다.
| @Bean | ||
| public RestTemplate restTemplate() { | ||
| return new RestTemplate(); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
프로덕션 환경을 위한 RestTemplate 설정 개선 필요
현재 기본 RestTemplate 설정만 있어 프로덕션 환경에서 성능 및 안정성 문제가 발생할 수 있습니다. 다음 설정들을 고려해보세요:
- Connection timeout, Read timeout 설정
- Connection pool 설정
- Error handling 개선
- SSL 인증서 검증 설정
@Bean
public RestTemplate restTemplate() {
- return new RestTemplate();
+ HttpComponentsClientHttpRequestFactory factory = new HttpComponentsClientHttpRequestFactory();
+ factory.setConnectionRequestTimeout(5000);
+ factory.setConnectTimeout(5000);
+ factory.setReadTimeout(10000);
+
+ RestTemplate restTemplate = new RestTemplate(factory);
+ restTemplate.setErrorHandler(new DefaultResponseErrorHandler());
+ return restTemplate;
}📝 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.
| @Bean | |
| public RestTemplate restTemplate() { | |
| return new RestTemplate(); | |
| } | |
| @Bean | |
| public RestTemplate restTemplate() { | |
| HttpComponentsClientHttpRequestFactory factory = new HttpComponentsClientHttpRequestFactory(); | |
| factory.setConnectionRequestTimeout(5000); | |
| factory.setConnectTimeout(5000); | |
| factory.setReadTimeout(10000); | |
| RestTemplate restTemplate = new RestTemplate(factory); | |
| restTemplate.setErrorHandler(new DefaultResponseErrorHandler()); | |
| return restTemplate; | |
| } |
🤖 Prompt for AI Agents
In src/main/java/konkuk/thip/config/RestTemplateConfig.java around lines 10 to
13, the current RestTemplate bean uses default settings which may cause
performance and stability issues in production. Enhance the RestTemplate
configuration by setting connection and read timeouts, configuring a connection
pool using a suitable HttpClient, improving error handling with a custom
ResponseErrorHandler, and ensuring proper SSL certificate validation. Implement
these configurations within the RestTemplate bean method to make it
production-ready.
| @Column(length = 4) | ||
| private Integer password; | ||
| private String password; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
비밀번호 보안 개선 필요
현재 비밀번호를 평문으로 저장하고 있어 보안상 위험합니다. 비밀번호 해싱을 고려해보세요.
@Column(length = 4)
-private String password;
+private String password; // 해싱된 비밀번호 저장 필요또는 도메인 레벨에서 비밀번호 해싱 로직을 구현하는 것을 권장합니다.
🤖 Prompt for AI Agents
In src/main/java/konkuk/thip/room/adapter/out/jpa/RoomJpaEntity.java at lines
31-32, the password field is stored as plain text, which is a security risk. To
fix this, remove storing the raw password directly in the entity and instead
store a hashed version of the password. Implement password hashing either before
setting the password in this entity or at the domain level, ensuring only hashed
passwords are persisted.
| @Override | ||
| public void updateForPageCount(Book book) { | ||
| BookJpaEntity bookJpaEntity = bookJpaRepository.findById(book.getId()).orElseThrow( | ||
| () -> new EntityNotFoundException(BOOK_NOT_FOUND) | ||
| ); | ||
|
|
||
| bookJpaEntity.changePageCount(book.getPageCount()); | ||
| bookJpaRepository.save(bookJpaEntity); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
트랜잭션 처리 및 메서드명 개선 필요
- 메서드에
@Transactional어노테이션이 누락되어 있어 데이터 일관성 문제가 발생할 수 있습니다. - 메서드명
updateForPageCount가 비즈니스 의도를 명확히 표현하지 못합니다.
다음과 같이 개선하는 것을 권장합니다:
+@Transactional
@Override
-public void updateForPageCount(Book book) {
+public void updateBookPageCount(Book book) {
BookJpaEntity bookJpaEntity = bookJpaRepository.findById(book.getId()).orElseThrow(
() -> new EntityNotFoundException(BOOK_NOT_FOUND)
);
bookJpaEntity.changePageCount(book.getPageCount());
bookJpaRepository.save(bookJpaEntity);
}📝 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.
| @Override | |
| public void updateForPageCount(Book book) { | |
| BookJpaEntity bookJpaEntity = bookJpaRepository.findById(book.getId()).orElseThrow( | |
| () -> new EntityNotFoundException(BOOK_NOT_FOUND) | |
| ); | |
| bookJpaEntity.changePageCount(book.getPageCount()); | |
| bookJpaRepository.save(bookJpaEntity); | |
| } | |
| @Transactional | |
| @Override | |
| public void updateBookPageCount(Book book) { | |
| BookJpaEntity bookJpaEntity = bookJpaRepository.findById(book.getId()).orElseThrow( | |
| () -> new EntityNotFoundException(BOOK_NOT_FOUND) | |
| ); | |
| bookJpaEntity.changePageCount(book.getPageCount()); | |
| bookJpaRepository.save(bookJpaEntity); | |
| } |
🤖 Prompt for AI Agents
In
src/main/java/konkuk/thip/book/adapter/out/persistence/BookCommandPersistenceAdapter.java
around lines 43 to 51, add the @Transactional annotation to the
updateForPageCount method to ensure data consistency during the update
operation. Also, rename the method to a more descriptive name like
updatePageCount to clearly express its business intent.
There was a problem hiding this comment.
코래 말대로 @Transactional 필요해보이네요!
There was a problem hiding this comment.
@buzz0331 맞습니다!
@Transactional 어노테이션이 중요한 이유는 이 메서드가 데이터베이스에서 엔티티를 조회하고 수정한 후 저장하는 두 단계의 작업을 수행하기 때문입니다. 트랜잭션 없이는 동시성 문제나 부분 업데이트 상황에서 데이터 일관성이 보장되지 않을 수 있어요.
특히 페이지 수 업데이트와 같은 중요한 책 정보 변경 작업은 원자성이 보장되어야 하므로 @Transactional을 추가하는 것이 좋겠습니다! 🎯
If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!
There was a problem hiding this comment.
서비스에서 @ Transactional 처리되고있지 않나요??
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!
There was a problem hiding this comment.
넵 하나의 api 에 해당하는 로직의 원자성을 지키는 것이 좋지않나 싶어서 api 를 처리하는 Service 클래스의 public 메서드에 @transactional 어노테이션을 추가해두었습니다!
| Integer findPageCountByIsbn(String isbn); | ||
|
|
||
| Book loadBookWithPageByIsbn(String isbn); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
CQRS 포트 분리 컨벤션 검토 필요
loadBookWithPageByIsbn 메서드가 Book 도메인 엔티티를 반환하는데, 학습된 컨벤션에 따르면 QueryPort는 조회 API response 데이터 조회용이고, CommandPort는 도메인 엔티티 조회용입니다. 현재 구현이 프로젝트의 CQRS 분리 컨벤션과 일치하는지 확인이 필요합니다.
다음 스크립트로 기존 포트 인터페이스의 패턴을 확인해보겠습니다:
🏁 Script executed:
#!/bin/bash
# 기존 QueryPort와 CommandPort의 메서드 반환 타입 패턴 확인
echo "=== BookApiQueryPort 메서드 확인 ==="
ast-grep --pattern 'interface BookApiQueryPort {
$$$
}'
echo "=== BookCommandPort 메서드 확인 ==="
ast-grep --pattern 'interface BookCommandPort {
$$$
}'
echo "=== 다른 QueryPort 인터페이스들의 패턴 확인 ==="
fd -t f -e java | xargs grep -l "QueryPort" | head -5Length of output: 823
🏁 Script executed:
#!/bin/bash
echo "=== BookApiQueryPort.java ==="
sed -n '1,200p' src/main/java/konkuk/thip/book/application/port/out/BookApiQueryPort.java || true
echo ""
echo "=== BookCommandPort.java ==="
sed -n '1,200p' src/main/java/konkuk/thip/book/application/port/out/BookCommandPort.java || true
echo ""
echo "=== AttendnaceCheckQueryPort.java ==="
sed -n '1,200p' src/main/java/konkuk/thip/attendanceCheck/application/port/out/AttendnaceCheckQueryPort.java || trueLength of output: 1389
CQRS Port 분리 컨벤션 위반 확인 필요
BookApiQueryPort에 도메인 엔티티(Book)를 반환하는 loadBookWithPageByIsbn 메서드가 추가되어 있습니다. 학습된 컨벤션에 따르면
- QueryPort: 조회 API 응답용 DTO/VO 반환
- CommandPort: 도메인 엔티티 조회용 메서드
현재 구현은 이 컨벤션을 따르고 있지 않으므로 아래 중 하나로 수정이 필요합니다:
loadBookWithPageByIsbn(String isbn)메서드를BookCommandPort로 이동- QueryPort에 남겨야 할 경우, 반환 타입을 도메인이 아닌 DTO/VO 형태로 변경
수정 대상:
- src/main/java/konkuk/thip/book/application/port/out/BookApiQueryPort.java
- (이동 시) src/main/java/konkuk/thip/book/application/port/out/BookCommandPort.java
🤖 Prompt for AI Agents
In src/main/java/konkuk/thip/book/application/port/out/BookApiQueryPort.java
around lines 11 to 13, the method loadBookWithPageByIsbn returns a domain entity
Book, violating the CQRS port separation convention. To fix this, either move
the loadBookWithPageByIsbn method to BookCommandPort.java if it needs to return
the domain entity, or if it must remain in BookApiQueryPort.java, change its
return type to a DTO or VO instead of the domain entity. Adjust the code
accordingly in the specified files to comply with the CQRS convention.
There was a problem hiding this comment.
@buzz0331 알겠습니다! 프로젝트의 맥락에 따라 유연하게 적용하는 것이 좋겠네요.
현재 BookApiQueryPort에서 loadBookWithPageByIsbn 메서드가 도메인 엔티티를 반환하는 구조로 두셔도 괜찮습니다. 실제 사용 목적과 팀의 개발 편의성을 고려한 판단이라고 생각합니다.
🧠 Learnings used
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#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로부터 조회하는 메서드를 추가함.
There was a problem hiding this comment.
이 부분은 Service가 Port로부터 알라딘, 네이버 api 의 통신결과에 해당하는 dto가 아니라, Book 도메인 엔티티를 반환받아 그 다음 로직을 수행하는게 어떤가 싶어서 Book 도메인 엔티티를 반환하는 메서드를 추가해두었습니다.
기존 Port 의 네이밍이 BookAPIQueryPort 이고, 해당 port 를 내부에 희진님이 네이버 api 와의 통신 결과에 해당하는 dto 를 반환하는 메서드들을 정의해주셨지만,
- Port의 네이밍을 '외부 api와의 통신과정을 통해 Book 애그리거트에 관한 정보를 반환한다' 라는 의미로 BookApiPort 로 수정
- 기존 네이버 api 와의 통신결과를 반환하는 메서드의 반환 타입을 Book 도메인 엔티티로 수정 (이건 수정한다면 추후 리펙토링시에 진행해도 될 것 같습니다)
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!
There was a problem hiding this comment.
- book 패키지 내부의 application.out에는
BookQueryPort,BookCommandPort,BookApiPort이렇게 3개의 port로 네이밍하자는 말로 이해했는데 맞나요? 맞다면 좋은 생각인 것 같습니다! - 이 부분이 잘 이해가 안되는데 현재 코드 상에서는 네이버 api의 통신결과로 반환되는 dto(
NaverBookParseResult,NaverDetailBookParseResult) 내부에 조회된 책 목록의 List를 가지고 있는 형태인데 이걸 모두 Book으로 바꾼다면 하나씩 naver api로부터 책을 하나씩 가져와서 서비스 클래스에서 List로 합치자는 말씀이신가요..?
There was a problem hiding this comment.
- book 패키지 내부의 application.out에는
BookQueryPort,BookCommandPort,BookApiPort이렇게 3개의 port로 네이밍하자는 말로 이해했는데 맞나요? 맞다면 좋은 생각인 것 같습니다!- 이 부분이 잘 이해가 안되는데 현재 코드 상에서는 네이버 api의 통신결과로 반환되는 dto(
NaverBookParseResult,NaverDetailBookParseResult) 내부에 조회된 책 목록의 List를 가지고 있는 형태인데 이걸 모두 Book으로 바꾼다면 하나씩 naver api로부터 책을 하나씩 가져와서 서비스 클래스에서 List로 합치자는 말씀이신가요..?
- 맞습니다
- 엇 이건 Service 에서 반환받은 dto를 가지고 다시 Book 도메인 엔티티를 만들어서 다음 로직을 수행하는 코드가 있는거 같아 말씀드린 거였습니다!
그런데 코드를 다시 확인해보니 '책 상세조회' api 와, '저장된 책 상태 변경' api 에서 모두 NaverDetailBookParseResult 를 반환하는 메서드를 활용하는데, 조회 api에서는 이 결과를 바로 반환하고, 변경 api 에서는 이 결과를 Book 도메인 엔티티로 변경하는 플로우네요!
이렇다면 괜찮은 것 같습니다 (다만 query를 위한 dto를 반환하는 메서드와, command 를 위한 도메인 엔티티를 반환하는 메서드가 구분되는게 좀 더 좋지 않나? 라는 생각이 있긴 합니다!)
| public Integer getPageCount(String isbn) { | ||
| String url = buildLookupUrl(isbn); | ||
| String response = restTemplate.getForObject(url, String.class); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
RestTemplate 타임아웃 설정 추가 권장
외부 API 호출 시 타임아웃이 설정되지 않아 응답이 없을 경우 무한 대기할 수 있습니다.
RestTemplateConfig에서 타임아웃을 설정하거나, 호출 시 타임아웃을 명시적으로 설정하는 것을 권장합니다.
// RestTemplateConfig.java에 추가
@Bean
public RestTemplate restTemplate() {
RestTemplate restTemplate = new RestTemplate();
restTemplate.setRequestFactory(requestFactory());
return restTemplate;
}
@Bean
public ClientHttpRequestFactory requestFactory() {
HttpComponentsClientHttpRequestFactory factory = new HttpComponentsClientHttpRequestFactory();
factory.setConnectTimeout(5000);
factory.setReadTimeout(5000);
return factory;
}🤖 Prompt for AI Agents
In src/main/java/konkuk/thip/book/adapter/out/api/aladin/AladinApiUtil.java
around lines 42 to 44, the RestTemplate used for external API calls does not
have timeout settings, which can cause indefinite waiting if the API does not
respond. To fix this, configure a RestTemplate bean with connection and read
timeouts set (e.g., 5000 ms) in a RestTemplateConfig class, and inject this
configured RestTemplate into AladinApiUtil instead of using a default instance.
src/main/java/konkuk/thip/book/adapter/out/api/CompositeBookApiAdapter.java
Outdated
Show resolved
Hide resolved
| private void saveBookWithPageCount() { | ||
| bookJpaRepository.save(BookJpaEntity.builder() | ||
| .title("작별하지 않는다") | ||
| .isbn("9788954682152") // 실제 isbn 값 | ||
| .authorName("한강") | ||
| .bestSeller(false) | ||
| .publisher("문학동네") | ||
| .imageUrl("https://image1.jpg") | ||
| .pageCount(332) // pageCount 값이 null이 아닌 책 | ||
| .description("한강의 소설") | ||
| .build()); | ||
| } | ||
|
|
||
| private void saveBookWithoutPageCount() { | ||
| bookJpaRepository.save(BookJpaEntity.builder() | ||
| .title("작별하지 않는다") | ||
| .isbn("9788954682152") // 실제 isbn 값 | ||
| .authorName("한강") | ||
| .bestSeller(false) | ||
| .publisher("문학동네") | ||
| .imageUrl("https://image1.jpg") | ||
| .pageCount(null) // pageCount 값이 null 인 책 -> 실제 페이지 정보 332 | ||
| .description("한강의 소설") | ||
| .build()); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
외부 API 의존성으로 인한 테스트 불안정성 우려
실제 ISBN (9788954682152)을 사용하여 네이버/알라딘 API를 호출하는 통합 테스트는 다음과 같은 문제가 있을 수 있습니다:
- 외부 API 장애 시 테스트 실패
- 네트워크 연결 문제로 인한 불안정성
- API 응답 데이터 변경으로 인한 테스트 실패
다음 해결 방안을 고려해 보세요:
- WireMock을 사용한 외부 API 모킹
- 테스트 프로필에서 BookApiPort를 Mock으로 교체
- 별도의 End-to-End 테스트 분리
🤖 Prompt for AI Agents
In src/test/java/konkuk/thip/room/adapter/in/web/RoomCreateAPITest.java around
lines 95 to 119, the test uses a real ISBN to call external APIs, causing
instability due to network or API issues. To fix this, mock the external API
calls by using WireMock or replace the BookApiPort with a mock implementation in
the test profile. Alternatively, separate these into dedicated end-to-end tests
to isolate integration concerns from unit tests.
| public RoomCreateCommand toCommand() { | ||
| DateTimeFormatter formatter = DateTimeFormatter.ofPattern("yyyy.MM.dd"); | ||
| return new RoomCreateCommand( | ||
| isbn, | ||
| category, | ||
| roomName, | ||
| description, | ||
| LocalDate.parse(progressStartDate, formatter), | ||
| LocalDate.parse(progressEndDate, formatter), | ||
| recruitCount, | ||
| password, | ||
| isPublic | ||
| ); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
날짜 파싱 예외 처리가 누락되었습니다.
LocalDate.parse() 메서드는 DateTimeParseException을 발생시킬 수 있지만, 현재 예외 처리가 없습니다. 패턴 검증을 통과한 문자열이라도 실제 날짜로 파싱할 때 예외가 발생할 수 있습니다 (예: 2025.02.30).
public RoomCreateCommand toCommand() {
DateTimeFormatter formatter = DateTimeFormatter.ofPattern("yyyy.MM.dd");
try {
return new RoomCreateCommand(
isbn,
category,
roomName,
description,
LocalDate.parse(progressStartDate, formatter),
LocalDate.parse(progressEndDate, formatter),
recruitCount,
password,
isPublic
);
} catch (DateTimeParseException e) {
throw new IllegalArgumentException("날짜 형식이 올바르지 않습니다.", e);
}
}🤖 Prompt for AI Agents
In src/main/java/konkuk/thip/room/adapter/in/web/request/RoomCreateRequest.java
around lines 45 to 58, the LocalDate.parse calls can throw
DateTimeParseException which is not currently handled. Wrap the parsing logic
inside a try-catch block that catches DateTimeParseException and rethrows it as
an IllegalArgumentException with a clear message indicating invalid date format
to ensure proper exception handling.
src/main/java/konkuk/thip/room/adapter/in/web/request/RoomCreateRequest.java
Show resolved
Hide resolved
src/test/java/konkuk/thip/room/adapter/in/web/RoomCreateControllerTest.java
Show resolved
Hide resolved
buzz0331
left a comment
There was a problem hiding this comment.
고생하셨습니다~ 방 - 기록 - 투표 생각보다 머리 아프네요 🥲
src/main/java/konkuk/thip/book/adapter/out/api/CompositeBookApiAdapter.java
Show resolved
Hide resolved
src/main/java/konkuk/thip/book/adapter/out/api/CompositeBookApiAdapter.java
Outdated
Show resolved
Hide resolved
src/main/java/konkuk/thip/book/adapter/out/api/aladin/AladinApiUtil.java
Outdated
Show resolved
Hide resolved
| @Override | ||
| public void updateForPageCount(Book book) { | ||
| BookJpaEntity bookJpaEntity = bookJpaRepository.findById(book.getId()).orElseThrow( | ||
| () -> new EntityNotFoundException(BOOK_NOT_FOUND) | ||
| ); | ||
|
|
||
| bookJpaEntity.changePageCount(book.getPageCount()); | ||
| bookJpaRepository.save(bookJpaEntity); | ||
| } |
There was a problem hiding this comment.
코래 말대로 @Transactional 필요해보이네요!
src/main/java/konkuk/thip/book/adapter/out/api/CompositeBookApiAdapter.java
Show resolved
Hide resolved
src/main/java/konkuk/thip/room/application/service/RoomCreateService.java
Show resolved
Hide resolved
src/test/java/konkuk/thip/room/adapter/in/web/RoomCreateControllerTest.java
Show resolved
Hide resolved
hd0rable
left a comment
There was a problem hiding this comment.
수고하셨습니다~~ 현준님 리뷰처럼 중요한 서비스 로직들이 몇몇 누락되어있는것같습니다! 확인 부탁드립니당
src/main/java/konkuk/thip/book/adapter/out/api/CompositeBookApiAdapter.java
Outdated
Show resolved
Hide resolved
src/main/java/konkuk/thip/book/adapter/out/api/aladin/AladinApiUtil.java
Show resolved
Hide resolved
| @Override | ||
| public void updateForPageCount(Book book) { | ||
| BookJpaEntity bookJpaEntity = bookJpaRepository.findById(book.getId()).orElseThrow( | ||
| () -> new EntityNotFoundException(BOOK_NOT_FOUND) | ||
| ); | ||
|
|
||
| bookJpaEntity.changePageCount(book.getPageCount()); | ||
| bookJpaRepository.save(bookJpaEntity); | ||
| } |
There was a problem hiding this comment.
서비스에서 @ Transactional 처리되고있지 않나요??
엇 리뷰 요구사항을 미처 확인하지 못했네요. 죄송합니다! 말씀하신 "도메인 엔티티를 변경한 뒤 해당 엔티티 자체를 adapter로 넘길지, 아니면 변경할 데이터만 추려서 adapter로 넘길지"에 대해서는, 현재 저희 설계 구조상 전자의 방식이 더 적합하지 않을까 생각합니다. 그 이유는, 따라서 전자의 방식이 설계 일관성과 유지보수 측면에서 더 적절하다고 생각합니다! |
|
@buzz0331 @hd0rable 넵 답변 감사합니다! 여쭤본 이유는 변경된 도메인 엔티티 전체를 adapter에 넘기면 이를 가지고 DB에서 해당 jpa entity를 조회한 후, 그리고 제가 작성한 코드처럼 도메인 엔티티의 특정 값의 변경사실을 DB에 반영하는 메서드로 update를 구현할 수도 있고, 아니면 도메인 엔티티 자체의 변경사실을 DB에 반영하는 메서드로도 update를 구현할 수도 있을 것 같습니다 첫번째 방식은 도메인 엔티티의 변경가능한 필드값마다 메서드를 추가해야하는 이슈가 있을 것 같고, 두번째 방식은 메서드 내부에서 도메인 엔티티의 모든 필드를 jpa 엔티티의 필드와 매핑한 후, jpa 엔티티를 save 해야하는 이슈가 있을 것 같습니다 전반적으로 고려해서 저희만의 update 전략을 생각해보면 좋을 것 같습니다! |
- withoutId 는 새로운 방을 처음 생성하는 것이므로, 방 퍼센티지를 직접적으로 0으로 설정하도록 코드 수정
- Port로부터 도메인 엔티티 전달받는 플로우로 Service 코드 수정
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/main/java/konkuk/thip/book/domain/Book.java (1)
62-64: 불변성 설계 결정에 대한 일관성 확인과거 리뷰에서 도메인 엔티티의 불변성에 대한 논의가 있었는데, 이번
changePageCount()메서드는 가변 접근 방식을 채택했습니다. 이는 이전withId,changePageCount메서드들이 새 객체를 반환하던 방식과 다른 설계입니다.현재 구현은 JPA와의 호환성 측면에서 더 적절하다고 판단되지만, 도메인 엔티티 내에서 일관된 설계 철학을 유지하는 것이 중요합니다.
🧹 Nitpick comments (2)
src/main/java/konkuk/thip/book/domain/Book.java (1)
58-60:hasPageCount()메서드 로직 검토 필요
pageCount > 0조건이 비즈니스 요구사항과 일치하는지 확인해주세요. 페이지 수가 0인 경우(예: 전자책이나 특수한 형태의 책)를 어떻게 처리할지 고려가 필요합니다.public boolean hasPageCount() { - return pageCount != null && pageCount > 0; + return pageCount != null && pageCount >= 0; }src/main/java/konkuk/thip/book/application/service/BookSavedService.java (1)
60-60: 데이터 일관성을 위한 적절한 변경새로 저장된 책을 ID로 다시 조회하는 방식으로 변경한 것은 좋은 접근입니다. 이를 통해 데이터베이스에 실제로 저장된 완전한 엔티티(audit 필드 포함)를 확보할 수 있습니다.
다만, 성능 최적화가 필요한 경우
bookCommandPort.save()메서드가 저장된 엔티티를 직접 반환하도록 수정하는 것도 고려해볼 수 있습니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
src/main/java/konkuk/thip/book/adapter/out/api/CompositeBookApiAdapter.java(1 hunks)src/main/java/konkuk/thip/book/adapter/out/api/NaverApiClient.java(1 hunks)src/main/java/konkuk/thip/book/adapter/out/api/aladin/AladinApiClient.java(1 hunks)src/main/java/konkuk/thip/book/adapter/out/api/aladin/AladinApiParam.java(1 hunks)src/main/java/konkuk/thip/book/adapter/out/api/aladin/AladinApiUtil.java(1 hunks)src/main/java/konkuk/thip/book/application/port/out/BookApiQueryPort.java(1 hunks)src/main/java/konkuk/thip/book/application/service/BookSavedService.java(1 hunks)src/main/java/konkuk/thip/book/domain/Book.java(1 hunks)src/main/java/konkuk/thip/common/exception/code/ErrorCode.java(2 hunks)src/main/java/konkuk/thip/room/application/service/RoomCreateService.java(1 hunks)src/main/java/konkuk/thip/room/domain/Room.java(3 hunks)src/test/java/konkuk/thip/room/domain/RoomTest.java(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- src/main/java/konkuk/thip/book/adapter/out/api/aladin/AladinApiParam.java
- src/main/java/konkuk/thip/book/adapter/out/api/CompositeBookApiAdapter.java
🚧 Files skipped from review as they are similar to previous changes (5)
- src/main/java/konkuk/thip/book/adapter/out/api/aladin/AladinApiClient.java
- src/main/java/konkuk/thip/book/application/port/out/BookApiQueryPort.java
- src/main/java/konkuk/thip/book/adapter/out/api/aladin/AladinApiUtil.java
- src/main/java/konkuk/thip/room/application/service/RoomCreateService.java
- src/main/java/konkuk/thip/common/exception/code/ErrorCode.java
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#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로부터 조회하는 메서드를 추가함.
src/main/java/konkuk/thip/book/application/service/BookSavedService.java (1)
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#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로부터 조회하는 메서드를 추가함.
⏰ 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 (12)
src/main/java/konkuk/thip/book/adapter/out/api/NaverApiClient.java (1)
10-10: 좋은 리팩토링입니다!클래스명이
BookApiNaverApiAdapter에서NaverApiClient로 변경되어 클래스의 역할이 더 명확해졌습니다. 직접적인 포트 구현체에서 낮은 수준의 API 클라이언트로 역할이 변경되면서 단일 책임 원칙을 더 잘 따르게 되었고, 컴포지트 패턴을 통해 여러 API를 조합할 수 있는 구조로 개선되었습니다.src/test/java/konkuk/thip/room/domain/RoomTest.java (5)
14-16: 테스트 데이터 상수 정의가 적절합니다.테스트에서 사용되는 날짜 데이터를 상수로 정의하여 가독성과 유지보수성을 향상시켰습니다.
17-29: 공개 방 비밀번호 검증 테스트가 올바르게 구현되었습니다.테스트가 예상되는 예외 타입과 메시지 내용을 정확히 검증하고 있습니다. 비즈니스 룰을 명확히 테스트하고 있어 좋습니다.
31-43: 비공개 방 비밀번호 검증 테스트가 올바르게 구현되었습니다.앞의 테스트와 일관된 패턴으로 구현되어 있고, 다른 시나리오를 적절히 테스트하고 있습니다.
45-60: 날짜 순서 검증 테스트가 올바르게 구현되었습니다.시작일과 종료일의 순서가 올바른지 검증하는 테스트가 잘 구현되어 있습니다. 예외 메시지에 실제 날짜 값을 포함하여 디버깅에 도움이 됩니다.
62-81: 현재 날짜 이전 시작일 검증 테스트가 올바르게 구현되었습니다.
LocalDate.now()를 사용하여 현재 시점 기준으로 테스트하는 것이 적절합니다. 테스트 케이스가 비즈니스 요구사항을 정확히 반영하고 있습니다.src/main/java/konkuk/thip/room/domain/Room.java (6)
4-4: 임포트 문 추가가 적절합니다.새로운 검증 로직을 위해
InvalidStateException과ErrorCode임포트를 추가한 것이 적절합니다.Also applies to: 11-11
25-25: 비밀번호 필드 타입 변경이 적절합니다.
Integer에서String으로 변경하여 실제 비밀번호 요구사항에 더 적합한 타입을 사용하게 되었습니다.
39-56: 정적 팩토리 메서드 구현이 우수합니다.도메인 객체 생성 시 비즈니스 룰을 강제하는 정적 팩토리 메서드 패턴을 올바르게 적용했습니다. 생성 시점에 검증을 수행하여 잘못된 상태의 객체가 생성되는 것을 방지합니다.
58-69: 비밀번호와 공개 여부 검증 로직이 정확합니다.비즈니스 룰을 명확히 구현했습니다: 공개 방은 비밀번호가 없어야 하고, 비공개 방은 비밀번호가 있어야 합니다. 에러 메시지도 디버깅에 도움이 되도록 상세하게 작성되었습니다.
71-90: 날짜 검증 로직이 올바르게 구현되었습니다.두 가지 중요한 비즈니스 룰을 검증합니다:
- 시작일이 종료일보다 이전이어야 함
- 시작일이 현재 날짜 이후여야 함
검증 로직이 명확하고 에러 메시지가 구체적이어서 좋습니다.
82-89: 현재 날짜 기준 검증에 대한 확인이 필요합니다.코드 주석에 "현재 날짜 포함"이라고 되어 있지만, 실제 코드는
startDate.isBefore(today)를 사용하여 현재 날짜는 포함하지 않고 있습니다.비즈니스 요구사항을 확인해주세요:
- 현재 날짜에 시작하는 방을 생성할 수 있어야 하는가?
- 아니면 무조건 미래 날짜여야 하는가?
만약 현재 날짜 포함이 맞다면 다음과 같이 수정해야 합니다:
- if (startDate.isBefore(today)) { + if (startDate.isBefore(today) || startDate.isEqual(today)) {또는 더 간단하게:
- if (startDate.isBefore(today)) { + if (!startDate.isAfter(today)) {
- 비밀번호 매칭 메서드 테스트 코드 추가 - withoutId 메서드를 통해 비밀번호가 해싱되는 것을 보여주기 위한 테스트 코드 추가 - 기존 테스트 코드의 비밀번호를 4자리 숫자로 변경
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/java/konkuk/thip/room/adapter/out/mapper/RoomMapper.java(2 hunks)src/main/java/konkuk/thip/room/domain/Room.java(3 hunks)src/test/java/konkuk/thip/room/domain/RoomTest.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main/java/konkuk/thip/room/adapter/out/mapper/RoomMapper.java
- src/test/java/konkuk/thip/room/domain/RoomTest.java
🔇 Additional comments (6)
src/main/java/konkuk/thip/room/domain/Room.java (6)
4-4: 새로운 import 추가가 적절합니다.보안 강화를 위한 BCrypt 암호화와 도메인 검증을 위한 예외 처리 관련 import가 적절하게 추가되었습니다.
Also applies to: 8-9, 13-13
19-19: 정적 PasswordEncoder 인스턴스 사용이 적절합니다.BCryptPasswordEncoder는 스레드 안전하므로 정적 인스턴스로 사용하는 것이 성능상 효율적입니다.
29-29: 비밀번호 필드 타입 변경이 보안을 향상시킵니다.Integer 타입에서 String 타입의 hashedPassword로 변경하여 해시된 비밀번호를 저장하는 것은 보안상 올바른 접근입니다.
43-63: 정적 팩토리 메서드 구현이 우수합니다.도메인 객체 생성 시 유효성 검증과 비밀번호 해싱을 적절히 처리하고 있으며, 빌더 패턴을 활용한 구현이 깔끔합니다.
65-76: 공개/비공개 방과 비밀번호 규칙 검증이 정확합니다.공개 방은 비밀번호가 없어야 하고, 비공개 방은 비밀번호가 있어야 한다는 비즈니스 규칙이 정확하게 구현되었습니다.
107-112: 비밀번호 검증 메서드가 안전하게 구현되었습니다.null 체크와 BCrypt 검증을 적절히 수행하고 있어 안전한 비밀번호 검증이 가능합니다.
hd0rable
left a comment
There was a problem hiding this comment.
수고하셨습니다!! 유저검증만 추가되면 될것같아욥!!
| detailBookByKeyword.title(), | ||
| isbn, | ||
| detailBookByKeyword.author(), | ||
| false, // TODO : 추후 BestSeller 도입되면 고려해야함 |
| Long newBookId = bookCommandPort.save(newBook); | ||
| book = newBook.withId(newBookId); | ||
|
|
||
| book = bookCommandPort.findById(newBookId); |
| private final RoomCreateUseCase roomCreateUseCase; | ||
|
|
||
| @PostMapping("/rooms") | ||
| public BaseResponse<RoomCreateResponse> createRoom(@Valid @RequestBody RoomCreateRequest request) { |
There was a problem hiding this comment.
p2: 유저검증 필요없을까요? userId가 필요할것같습니다!
| validateDates(startDate, endDate); | ||
|
|
||
| // 비밀번호 해싱 | ||
| String hashedPassword = (password != null) ? PASSWORD_ENCODER.encode(password) : null; |
- 'Room 활동 시작일은 Room을 생성하려는 시각보다 최소 1일 이후여야 한다' 라는 요구사항 반영
- request dto 내부에서 모집 인원과 관련된 bean validation 추가 - 관련해서 controller 단위 테스트 코드 추가
| public class CompositeBookApiAdapter implements BookApiQueryPort { | ||
|
|
||
| private final BookApiNaverApiAdapter bookApiNaverApiAdapter; | ||
| private final NaverApiClient naverApiClient; |
| public enum AladinApiParam { | ||
|
|
||
| ITEM_ID_TYPE("ISBN"), | ||
| OUTPUT("js"), | ||
| API_VERSION("20131101"), | ||
| SUB_INFO_PARSING_KEY("subInfo"), | ||
| PAGE_COUNT_PARSING_KEY("itemPage"); | ||
|
|
| @Value("${aladin.baseUrl}") | ||
| private String baseUrl; |
| .hashedPassword(hashedPassword) | ||
| .roomPercentage(0) // 처음 Room 생성 시 -> 0% |
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/main/java/konkuk/thip/room/application/port/in/RoomCreateUseCase.java (1)
5-8: JavaDoc 문서 추가를 권장합니다.인터페이스와 메소드에 대한 설명, 매개변수, 반환값, 그리고 발생 가능한 예외에 대한 JavaDoc을 추가하면 코드의 가독성과 유지보수성이 향상됩니다.
+/** + * 방 생성 기능을 정의하는 인바운드 포트 인터페이스 + */ public interface RoomCreateUseCase { + /** + * 새로운 방을 생성합니다. + * + * @param command 방 생성에 필요한 정보를 담은 커맨드 객체 + * @param userId 방을 생성하는 사용자의 ID + * @return 생성된 방의 ID + * @throws IllegalArgumentException 잘못된 입력값이 제공된 경우 + * @throws BusinessException 비즈니스 규칙 위반 시 + */ Long createRoom(RoomCreateCommand command, Long userId); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/java/konkuk/thip/room/adapter/in/web/RoomCommandController.java(1 hunks)src/main/java/konkuk/thip/room/application/port/in/RoomCreateUseCase.java(1 hunks)src/main/java/konkuk/thip/room/application/service/RoomCreateService.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main/java/konkuk/thip/room/adapter/in/web/RoomCommandController.java
- src/main/java/konkuk/thip/room/application/service/RoomCreateService.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 (1)
src/main/java/konkuk/thip/room/application/port/in/RoomCreateUseCase.java (1)
7-7: 메소드 설계가 우수합니다.커맨드 패턴을 사용하여 매개변수를 캡슐화하고, 사용자 ID를 별도로 받는 설계가 적절합니다. 이는 관심사의 분리와 확장성 측면에서 좋은 설계입니다.



#️⃣ 연관된 이슈
📝 작업 내용
방 생성 api 를 개발하였습니다
방 생성 api 플로우
이때 책 load 시에 고려해야하는 경우와, 제가 구현 방식은 아래와 같습니다.
📸 스크린샷
💬 리뷰 요구사항
@hd0rable 님이 구현하신 네이버 api 와의 통신 기능을 adapter 단에서 활용하기 위해 BookApiPort 를 구현하는 구현체를 facade 패턴을 적용하였습니다.
이제 BookApiAdapter 에서 네이버, 알라딘 api 와의 통신 기능을 모두 활용할 수 있습니다.
Book update 플로우를
이렇게 구성하였는데, update 관련 내용에 대해 다음 회의에서 한번 얘기 나눠보면 좋을 것 같습니다.
(도메인 엔티티를 변경 후에 변경된 도메인 엔티티를 adapter 로 던질지, 아니면 변경할 데이터들만 뽑아서 바로 adapter로 던질지?? 등등)
위 부분들 참고해서 리뷰해주시면 감사하겠습니다
📌 PR 진행 시 이러한 점들을 참고해 주세요
Summary by CodeRabbit
신규 기능
버그 수정
리팩터링/개선
테스트
기타