Conversation
|
Warning Rate limit exceeded@hd0rable has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 10 minutes and 30 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
""" Walkthrough책 저장 상태 변경 API가 도입되었습니다. 새로운 컨트롤러 엔드포인트와 서비스 계층, 도메인 객체, 예외 코드, 영속성 계층의 확장 및 테스트가 추가되어, 사용자가 책을 저장하거나 저장 해제할 수 있도록 전체적인 흐름이 구현되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Controller
participant BookSavedService
participant BookCommandPort
participant SavedQueryPort
participant SavedCommandPort
participant ExternalBookApi
Client->>Controller: POST /books/{isbn}/saved (type, JWT)
Controller->>BookSavedService: changeSavedBook(isbn, type, userId)
BookSavedService->>BookCommandPort: findByIsbn(isbn)
alt Book not found & type==save
BookSavedService->>ExternalBookApi: fetchBookDetail(isbn)
BookSavedService->>BookCommandPort: save(book)
else Book not found & type==delete
BookSavedService-->>Controller: throw BOOK_NOT_SAVED_DB_CANNOT_DELETE
end
BookSavedService->>SavedQueryPort: findByUserId(userId)
alt type==save
BookSavedService->>SavedCommandPort: saveBook(userId, bookId)
else type==delete
BookSavedService->>SavedCommandPort: deleteBook(userId, bookId)
end
BookSavedService-->>Controller: BookIsSavedResult
Controller-->>Client: PostBookIsSavedResponse
Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Suggested reviewers
Poem
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (17)
src/main/java/konkuk/thip/common/security/util/JwtUtil.java (1)
26-26: 환경 변수 설정 작업 완료 필요TODO 주석에서 언급한 대로 토큰 만료 시간을 환경 변수로 설정하는 작업을 완료해야 합니다. 이는 보안 설정의 유연성을 위해 중요합니다.
환경 변수 설정 코드를 생성해 드릴까요?
src/main/java/konkuk/thip/book/adapter/in/web/request/PostBookIsSavedRequest.java (1)
6-9: 필드명 개선 제안 및 구현 승인Record 클래스와 validation 구현이 적절합니다. 다만
type필드명보다는isSaved또는saveType같이 더 명확한 이름을 고려해보세요.더 명확한 필드명 제안:
public record PostBookIsSavedRequest( - @NotNull(message = "type은 필수입니다.") - boolean type + @NotNull(message = "saveType은 필수입니다.") + boolean saveType ) { }src/main/java/konkuk/thip/book/application/port/in/dto/BookIsSavedResult.java (1)
4-12: 간단한 스타일 개선이 필요합니다.Record 구현과 factory method는 잘 설계되었지만, 코드 스타일을 개선할 수 있습니다.
다음과 같이 포맷팅을 개선하세요:
- public static BookIsSavedResult of( String isbn,boolean isSaved) { + public static BookIsSavedResult of(String isbn, boolean isSaved) {src/main/java/konkuk/thip/book/adapter/in/web/response/PostBookIsSavedResponse.java (1)
3-3: 사용하지 않는 import를 제거해주세요.
BookDetailSearchResultimport가 사용되지 않습니다.-import konkuk.thip.book.application.port.in.dto.BookDetailSearchResult;src/main/java/konkuk/thip/saved/adapter/out/persistence/SavedQueryPersistenceAdapter.java (1)
18-21: 새로운 존재 여부 확인 메서드가 올바르게 구현되었습니다.책 저장 상태 확인 기능이 적절히 추가되었습니다. 다만 JPA 메서드명이 상당히 길어 가독성이 떨어집니다.
JPA 리포지토리에서 더 간결한 메서드명을 고려해보세요:
- return savedBookJpaRepository.existsByUserJpaEntity_UserIdAndBookJpaEntity_BookId(userId, bookId); + return savedBookJpaRepository.existsByUserIdAndBookId(userId, bookId);이를 위해 SavedBookJpaRepository에서 사용자 정의 쿼리나 더 간단한 메서드명을 사용할 수 있습니다.
src/main/java/konkuk/thip/room/adapter/out/persistence/RoomQueryPersistenceAdapter.java (1)
17-20: 모집 중인 방 개수 조회 메서드가 올바르게 구현되었습니다.책 상세 정보에 필요한 모집 중인 방 개수를 조회하는 기능이 적절히 추가되었습니다.
다만 JPA 메서드명이 길어 보입니다. 필요시 더 간결한 메서드명 사용을 고려해보세요:
- return roomJpaRepository.countByBookJpaEntity_BookIdAndStartDateAfter(bookId, currentDate); + return roomJpaRepository.countRecruitingByBookIdAndStartDateAfter(bookId, currentDate);src/main/java/konkuk/thip/feed/adapter/out/persistence/FeedQueryRepositoryImpl.java (1)
18-29: 중복된 중복 제거 로직과 null 체크 부재
distinct()와HashSet사용으로 중복 제거가 이중으로 수행되고 있습니다. QueryDSL의distinct()만으로도 충분합니다.bookId가 null일 경우에 대한 방어 코드가 없습니다.@Override public Set<Long> findUserIdsByBookId(Long bookId) { + if (bookId == null) { + return Collections.emptySet(); + } QFeedJpaEntity feed = QFeedJpaEntity.feedJpaEntity; - Set<Long> userIds = new HashSet<>( - jpaQueryFactory - .select(feed.userJpaEntity.userId) - .distinct() - .from(feed) - .where(feed.bookJpaEntity.bookId.eq(bookId)) - .fetch() - ); - return userIds; + return jpaQueryFactory + .select(feed.userJpaEntity.userId) + .distinct() + .from(feed) + .where(feed.bookJpaEntity.bookId.eq(bookId)) + .fetch() + .stream() + .collect(Collectors.toSet()); }src/main/java/konkuk/thip/book/application/port/in/dto/BookDetailSearchResult.java (1)
14-23: 정적 팩토리 메소드의 필요성 검토현재
of()메소드는 단순히 생성자를 호출하는 역할만 하고 있어 불필요해 보입니다. 레코드 클래스의 기본 생성자만으로도 충분할 수 있습니다.하지만 향후 확장성이나 팀 내 코딩 컨벤션을 고려한다면 유지하는 것도 합리적입니다. 팀의 일관된 패턴을 따르는 것을 권장합니다.
src/main/java/konkuk/thip/book/adapter/out/api/dto/NaverDetailBookParseResult.java (1)
22-23: 베스트셀러 하드코딩 개선 필요
bestSeller필드를false로 하드코딩하는 것은 임시 해결책으로 이해되지만, 향후 네이버 API 응답에서 베스트셀러 정보를 추출하거나 별도의 로직으로 판단하는 기능을 구현하는 것을 고려해보세요.- .bestSeller(false) //베스트셀러 구현 어떻게 할지 확정되면 나중에 수정, - // 지금은 베스트셀러 조회안하고 무조건 저장되는 책은 베스트셀러가아닌걸로 가정 + .bestSeller(determineBestSellerStatus(naverDetailBookParseResult)) // TODO: 베스트셀러 판단 로직 구현 필요src/test/java/konkuk/thip/book/adapter/in/web/BookIsSavedControllerTest.java (1)
213-213: 오타 수정 필요테스트 메서드명에 오타가 있습니다.
- @DisplayName("저장하지 않은 책을 삭제하려고 하면 [400 애러 발생]") + @DisplayName("저장하지 않은 책을 삭제하려고 하면 [400 에러 발생]")src/main/java/konkuk/thip/book/application/service/BookSavedService.java (1)
39-39: boolean 대신 enum 사용을 고려해보세요.
postBookIsSavedRequest.type()이 boolean을 반환하여 true/false로 저장/삭제를 구분하는 것은 가독성이 떨어집니다.enum을 사용하여 명확하게 표현하는 것을 제안합니다:
public enum BookSavedActionType { SAVE, DELETE }src/main/java/konkuk/thip/common/exception/validation/SizeViolationStrategy.java (1)
27-27: 중복된 ErrorCode 참조를 제거하세요.이미 12번 줄에서
API_INVALID_SIZE를 static import 했으므로 명시적인ErrorCode.접두사는 불필요합니다.- ErrorCode.API_INVALID_SIZE, + API_INVALID_SIZE,src/main/java/konkuk/thip/book/adapter/in/web/BookQueryController.java (1)
34-35: 불필요한 빈 줄을 제거하세요.메서드 내부의 빈 줄이 불필요합니다.
- - - return BaseResponse.ok(GetBookDetailSearchResponse.of(bookSearchUseCase.searchDetailBooks(isbn,userId)));src/main/java/konkuk/thip/common/exception/handler/GlobalExceptionHandler.java (1)
122-150: 제약 조건 위반 처리 구현이 우수합니다전략 패턴을 사용한 확장 가능한 구현과 적절한 폴백 처리가 잘 되어 있습니다.
첫 번째 위반만 처리하는 것에 대한 주석을 더 명확히 하면 좋겠습니다:
- // 첫 번째 위반을 꺼내서 + // 첫 번째 위반만 처리 (다중 위반 시 가장 먼저 발견된 것만 반환) ConstraintViolation<?> violation = e.getConstraintViolations().stream().findFirst().orElse(null);src/test/java/konkuk/thip/book/adapter/in/web/BookQueryControllerTest.java (1)
164-194: 최근 검색어 저장 테스트가 잘 구현되었습니다테스트가 명확하고 검증이 적절합니다.
테스트 가독성 향상을 위해 검증 부분을 별도 메서드로 추출하면 좋겠습니다:
private void assertRecentSearchSaved(String expectedKeyword, Long userId) { RecentSearchJpaEntity recentSearch = recentSearchJpaRepository.findAll().stream() .filter(rs -> rs.getUserJpaEntity().getUserId().equals(userId)) .filter(rs -> rs.getSearchTerm().equals(expectedKeyword)) .findFirst() .orElse(null); assertThat(recentSearch).isNotNull(); assertThat(recentSearch.getSearchTerm()).isEqualTo(expectedKeyword); assertThat(recentSearch.getType()).isEqualTo(SearchType.BOOK_SEARCH); }src/main/java/konkuk/thip/book/application/service/BookSearchService.java (1)
67-71: 최근 검색어 저장 시점 재검토 필요현재 구현은 검색 결과와 관계없이 최근 검색어를 저장합니다. 검색이 성공적으로 완료된 경우에만 저장하는 것이 더 적절할 수 있습니다.
예외가 발생하지 않은 경우에만 저장하도록 위치를 조정하거나, 검색 결과가 있는 경우에만 저장하는 로직 추가를 고려해보세요:
if ( totalElements!=0 && page > totalPages) { throw new BusinessException(BOOK_SEARCH_PAGE_OUT_OF_RANGE); } -//최근검색어 추가 -User user = userCommandPort.findById(userId); -recentSearchCommandPort.save(user.getId(),keyword,BOOK_SEARCH); +// 검색이 성공적으로 완료된 경우에만 최근검색어 추가 +if (totalElements > 0) { + User user = userCommandPort.findById(userId); + recentSearchCommandPort.save(user.getId(), keyword, BOOK_SEARCH); +} return result;src/test/java/konkuk/thip/book/adapter/in/web/BookDetailSearchControllerTest.java (1)
141-154: 컨트롤러 테스트에서 서비스 직접 호출
BookDetailSearchControllerTest라는 이름의 테스트 클래스이지만bookSearchService를 직접 호출하고 있습니다. 컨트롤러 계층을 테스트하려면 MockMvc를 사용하여 HTTP 엔드포인트를 호출하는 것이 더 적절할 것 같습니다.-var result = bookSearchService.searchDetailBooks(isbn, user.getUserId()); +var result = mockMvc.perform(get("/books/" + isbn) + .header("Authorization", "Bearer " + jwtUtil.generateToken(user))) + .andExpect(status().isOk()) + .andReturn();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (63)
src/main/java/konkuk/thip/book/adapter/in/web/BookCommandController.java(1 hunks)src/main/java/konkuk/thip/book/adapter/in/web/BookQueryController.java(1 hunks)src/main/java/konkuk/thip/book/adapter/in/web/request/PostBookIsSavedRequest.java(1 hunks)src/main/java/konkuk/thip/book/adapter/in/web/response/DummyResponse.java(0 hunks)src/main/java/konkuk/thip/book/adapter/in/web/response/GetBookDetailSearchResponse.java(1 hunks)src/main/java/konkuk/thip/book/adapter/in/web/response/GetBookSearchListResponse.java(0 hunks)src/main/java/konkuk/thip/book/adapter/in/web/response/PostBookIsSavedResponse.java(1 hunks)src/main/java/konkuk/thip/book/adapter/out/api/BookApiAdapter.java(0 hunks)src/main/java/konkuk/thip/book/adapter/out/api/BookApiNaverApiAdapter.java(1 hunks)src/main/java/konkuk/thip/book/adapter/out/api/NaverApiUtil.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/dto/NaverDetailBookParseResult.java(1 hunks)src/main/java/konkuk/thip/book/adapter/out/persistence/BookCommandPersistenceAdapter.java(1 hunks)src/main/java/konkuk/thip/book/adapter/out/persistence/BookJpaRepository.java(1 hunks)src/main/java/konkuk/thip/book/adapter/out/persistence/BookQueryPersistenceAdapter.java(0 hunks)src/main/java/konkuk/thip/book/application/port/in/BookSavedUseCase.java(1 hunks)src/main/java/konkuk/thip/book/application/port/in/BookSearchUseCase.java(1 hunks)src/main/java/konkuk/thip/book/application/port/in/dto/BookDetailSearchResult.java(1 hunks)src/main/java/konkuk/thip/book/application/port/in/dto/BookIsSavedResult.java(1 hunks)src/main/java/konkuk/thip/book/application/port/in/dto/DummyResult.java(0 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/application/service/BookSavedService.java(1 hunks)src/main/java/konkuk/thip/book/application/service/BookSearchService.java(2 hunks)src/main/java/konkuk/thip/common/exception/ExternalApiException.java(1 hunks)src/main/java/konkuk/thip/common/exception/code/ErrorCode.java(3 hunks)src/main/java/konkuk/thip/common/exception/handler/GlobalExceptionHandler.java(3 hunks)src/main/java/konkuk/thip/common/exception/validation/ConstraintViolationResult.java(1 hunks)src/main/java/konkuk/thip/common/exception/validation/ConstraintViolationStrategy.java(1 hunks)src/main/java/konkuk/thip/common/exception/validation/PatternViolationStrategy.java(1 hunks)src/main/java/konkuk/thip/common/exception/validation/SizeViolationStrategy.java(1 hunks)src/main/java/konkuk/thip/common/security/util/JwtUtil.java(1 hunks)src/main/java/konkuk/thip/feed/adapter/out/persistence/FeedJpaRepository.java(1 hunks)src/main/java/konkuk/thip/feed/adapter/out/persistence/FeedQueryPersistenceAdapter.java(1 hunks)src/main/java/konkuk/thip/feed/adapter/out/persistence/FeedQueryRepository.java(1 hunks)src/main/java/konkuk/thip/feed/adapter/out/persistence/FeedQueryRepositoryImpl.java(1 hunks)src/main/java/konkuk/thip/feed/application/port/out/FeedQueryPort.java(1 hunks)src/main/java/konkuk/thip/recentSearch/adapter/out/jpa/SearchType.java(2 hunks)src/main/java/konkuk/thip/recentSearch/adapter/out/persistence/RecentSearchCommandPersistenceAdapter.java(1 hunks)src/main/java/konkuk/thip/recentSearch/application/port/out/RecentSearchCommandPort.java(1 hunks)src/main/java/konkuk/thip/room/adapter/out/jpa/RoomJpaEntity.java(1 hunks)src/main/java/konkuk/thip/room/adapter/out/persistence/RoomJpaRepository.java(1 hunks)src/main/java/konkuk/thip/room/adapter/out/persistence/RoomQueryPersistenceAdapter.java(1 hunks)src/main/java/konkuk/thip/room/application/port/out/RoomQueryPort.java(1 hunks)src/main/java/konkuk/thip/saved/adapter/out/persistence/SavedBookJpaRepository.java(1 hunks)src/main/java/konkuk/thip/saved/adapter/out/persistence/SavedBookRepository.java(0 hunks)src/main/java/konkuk/thip/saved/adapter/out/persistence/SavedCommandPersistenceAdapter.java(1 hunks)src/main/java/konkuk/thip/saved/adapter/out/persistence/SavedFeedJpaRepository.java(1 hunks)src/main/java/konkuk/thip/saved/adapter/out/persistence/SavedQueryPersistenceAdapter.java(1 hunks)src/main/java/konkuk/thip/saved/application/port/out/SavedCommandPort.java(1 hunks)src/main/java/konkuk/thip/saved/application/port/out/SavedQueryPort.java(1 hunks)src/main/java/konkuk/thip/user/adapter/out/jpa/UserJpaEntity.java(1 hunks)src/main/java/konkuk/thip/user/adapter/out/persistence/UserCommandPersistenceAdapter.java(2 hunks)src/main/java/konkuk/thip/user/adapter/out/persistence/UserJpaRepository.java(1 hunks)src/main/java/konkuk/thip/user/adapter/out/persistence/UserQueryPersistenceAdapter.java(2 hunks)src/main/java/konkuk/thip/user/adapter/out/persistence/UserQueryRepository.java(1 hunks)src/main/java/konkuk/thip/user/adapter/out/persistence/UserQueryRepositoryImpl.java(1 hunks)src/main/java/konkuk/thip/user/adapter/out/persistence/UserRoomJpaRepository.java(1 hunks)src/main/java/konkuk/thip/user/application/port/out/UserCommandPort.java(1 hunks)src/main/java/konkuk/thip/user/application/port/out/UserQueryPort.java(1 hunks)src/test/java/konkuk/thip/book/adapter/in/web/BookDetailSearchControllerTest.java(1 hunks)src/test/java/konkuk/thip/book/adapter/in/web/BookIsSavedControllerTest.java(1 hunks)src/test/java/konkuk/thip/book/adapter/in/web/BookQueryControllerTest.java(5 hunks)
💤 Files with no reviewable changes (6)
- src/main/java/konkuk/thip/book/adapter/in/web/response/DummyResponse.java
- src/main/java/konkuk/thip/book/adapter/out/persistence/BookQueryPersistenceAdapter.java
- src/main/java/konkuk/thip/book/adapter/in/web/response/GetBookSearchListResponse.java
- src/main/java/konkuk/thip/saved/adapter/out/persistence/SavedBookRepository.java
- src/main/java/konkuk/thip/book/application/port/in/dto/DummyResult.java
- src/main/java/konkuk/thip/book/adapter/out/api/BookApiAdapter.java
🧰 Additional context used
🧠 Learnings (3)
src/main/java/konkuk/thip/user/adapter/out/persistence/UserJpaRepository.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/book/adapter/out/persistence/BookJpaRepository.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/user/adapter/out/persistence/UserCommandPersistenceAdapter.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 안전성을 위해 권장됩니다.
🧬 Code Graph Analysis (7)
src/main/java/konkuk/thip/recentSearch/adapter/out/persistence/RecentSearchCommandPersistenceAdapter.java (2)
src/main/java/konkuk/thip/common/exception/EntityNotFoundException.java (1)
EntityNotFoundException(5-10)src/main/java/konkuk/thip/recentSearch/adapter/out/persistence/RecentSearchQueryPersistenceAdapter.java (1)
Repository(8-15)
src/main/java/konkuk/thip/common/exception/validation/PatternViolationStrategy.java (1)
src/main/java/konkuk/thip/common/exception/validation/SizeViolationStrategy.java (1)
Component(15-32)
src/main/java/konkuk/thip/common/exception/validation/SizeViolationStrategy.java (1)
src/main/java/konkuk/thip/common/exception/validation/PatternViolationStrategy.java (1)
Component(13-30)
src/main/java/konkuk/thip/book/adapter/in/web/BookQueryController.java (2)
src/main/java/konkuk/thip/book/adapter/out/api/NaverApiUtil.java (1)
RequiredArgsConstructor(18-123)src/main/java/konkuk/thip/book/adapter/in/web/BookCommandController.java (1)
Validated(13-29)
src/main/java/konkuk/thip/saved/adapter/out/persistence/SavedCommandPersistenceAdapter.java (2)
src/main/java/konkuk/thip/common/exception/EntityNotFoundException.java (1)
EntityNotFoundException(5-10)src/main/java/konkuk/thip/saved/adapter/out/persistence/SavedQueryPersistenceAdapter.java (1)
Repository(9-22)
src/main/java/konkuk/thip/book/adapter/out/api/BookApiNaverApiAdapter.java (2)
src/main/java/konkuk/thip/book/adapter/out/api/NaverApiUtil.java (1)
RequiredArgsConstructor(18-123)src/main/java/konkuk/thip/book/adapter/out/api/NaverBookXmlParser.java (1)
NaverBookXmlParser(17-124)
src/main/java/konkuk/thip/book/adapter/out/api/NaverBookXmlParser.java (1)
src/main/java/konkuk/thip/common/exception/ExternalApiException.java (1)
ExternalApiException(5-17)
🔇 Additional comments (66)
src/main/java/konkuk/thip/recentSearch/application/port/out/RecentSearchCommandPort.java (1)
6-7: 최근 검색 저장 인터페이스 추가 승인메서드 시그니처가 명확하고 적절합니다. 사용자 ID, 키워드, 검색 타입을 받아 최근 검색 기록을 저장하는 역할이 잘 정의되어 있습니다.
src/main/java/konkuk/thip/feed/application/port/out/FeedQueryPort.java (1)
7-7: 북 ID로 사용자 ID 조회 메서드 승인메서드명이 명확하고 반환 타입으로
Set<Long>을 사용한 것이 적절합니다. 중복된 사용자 ID를 방지하고 효율적인 조회를 지원합니다.src/main/java/konkuk/thip/saved/adapter/out/persistence/SavedFeedJpaRepository.java (1)
6-7: JPA 저장소 인터페이스 승인표준 JPA 저장소 인터페이스 구조를 따르고 있으며, 제네릭 타입이 올바르게 지정되어 있습니다.
src/main/java/konkuk/thip/user/adapter/out/persistence/UserQueryRepository.java (1)
5-7: 사용자 조회 인터페이스 일관성 확인
FeedQueryPort와 동일한 메서드명과 반환 타입을 사용하여 일관성이 잘 유지되고 있습니다. 북 ID로 사용자 ID를 조회하는 기능이 여러 모듈에서 일관되게 구현되었습니다.src/main/java/konkuk/thip/saved/application/port/out/SavedQueryPort.java (1)
4-4: 메서드 시그니처가 명확하고 적절합니다.
existsByUserIdAndBookId메서드명이 기능을 명확히 표현하며, 책 저장 상태 변경 로직에서 중복 저장 방지를 위한 존재 여부 확인에 적합합니다.src/main/java/konkuk/thip/user/adapter/out/persistence/UserJpaRepository.java (1)
11-11: 중복 선언 검토 필요:findById메서드
JpaRepository<UserJpaEntity, Long>가 이미Optional<T> findById(ID id)메서드를 제공하므로, 아래 선언이 중복일 가능성이 높습니다. 특별한 커스텀 로직이나 의도가 없다면 해당 선언을 제거하거나, 명시적으로 오버라이드하는 이유를 명확히 해주세요.
- 파일:
src/main/java/konkuk/thip/user/adapter/out/persistence/UserJpaRepository.java- 라인: 11 (
Optional<UserJpaEntity> findById(Long userId);)src/main/java/konkuk/thip/user/application/port/out/UserCommandPort.java (1)
8-8: 도메인 계층에 적합한 메서드 시그니처입니다.
User findById(Long userId)메서드가Optional을 반환하지 않고 도메인 엔티티를 직접 반환하는 것은 적절합니다. 사용자가 존재하지 않을 경우 예외를 발생시키는 비즈니스 로직에 부합합니다.src/main/java/konkuk/thip/book/adapter/out/persistence/BookJpaRepository.java (2)
6-7: Optional 임포트 추가가 적절합니다.새로운 메서드의 반환 타입을 위한
Optional임포트가 올바르게 추가되었습니다.
9-9: Spring Data JPA 규칙을 잘 따른 메서드 시그니처입니다.
findByIsbn메서드는 Spring Data JPA의 쿼리 메서드 명명 규칙을 따르며,Optional반환 타입으로 null 안전성을 보장합니다. ISBN을 통한 책 조회 기능에 적합합니다.src/main/java/konkuk/thip/user/application/port/out/UserQueryPort.java (2)
3-3: Set 임포트 추가가 적절합니다.새로운 메서드의 반환 타입을 위한
Set임포트가 올바르게 추가되었습니다.
7-7: 메서드명이 상세하고 반환 타입이 적절합니다.
findUserIdsParticipatedInRoomsByBookId메서드명이 길지만 기능을 명확히 표현합니다.Set<Long>반환 타입은 중복 없는 사용자 ID 컬렉션에 적합하며, 책별 참여 사용자 수 집계 로직에 잘 부합합니다.src/main/java/konkuk/thip/user/adapter/out/persistence/UserQueryPersistenceAdapter.java (2)
8-8: Import 추가 승인Set 타입 사용을 위한 적절한 import 추가입니다.
22-25: 새로운 조회 메서드 구현 승인책 ID로 참여한 사용자 ID 목록을 조회하는 메서드가 올바르게 구현되었습니다. 메서드명이 명확하며 repository로의 위임 패턴도 적절합니다.
src/main/java/konkuk/thip/user/adapter/out/persistence/UserRoomJpaRepository.java (1)
1-7: 표준 JPA Repository 인터페이스 승인UserRoomJpaEntity를 위한 표준 JPA Repository 인터페이스가 올바르게 정의되었습니다. Spring Data JPA 컨벤션을 잘 따르고 있습니다.
src/main/java/konkuk/thip/feed/adapter/out/persistence/FeedJpaRepository.java (1)
6-6: 커스텀 쿼리 Repository 확장 승인
FeedQueryRepository인터페이스 확장을 통해 커스텀 쿼리 기능을 추가한 것이 적절합니다. Spring Data JPA의 표준 패턴을 잘 따르고 있습니다.src/main/java/konkuk/thip/common/exception/validation/ConstraintViolationResult.java (1)
1-9: 검증 제약 조건 위반 결과를 위한 레코드 클래스가 올바르게 구현되었습니다.불변 데이터 캐리어로서 적절한 구조를 가지고 있으며, 검증 예외 처리 메커니즘의 표준화된 결과 객체 역할을 잘 수행할 것으로 보입니다.
src/main/java/konkuk/thip/feed/adapter/out/persistence/FeedQueryPersistenceAdapter.java (3)
8-8: 필요한 import 추가가 적절합니다.새로운 메서드의 반환 타입
Set<Long>을 위한 import가 올바르게 추가되었습니다.
14-15: 필드명 변경으로 코드 가독성이 향상되었습니다.
jpaRepository→feedJpaRepository,userMapper→feedMapper로 변경하여 필드의 역할이 더 명확해졌습니다.
17-20: 포트 인터페이스 구현이 올바르게 되었습니다.
findUserIdsByBookId메서드가 적절하게 repository에 위임하여 구현되었으며, 어댑터 패턴을 잘 따르고 있습니다.src/main/java/konkuk/thip/feed/adapter/out/persistence/FeedQueryRepository.java (1)
1-7: 피드 쿼리를 위한 인터페이스가 잘 설계되었습니다.단일 책임 원칙을 따르는 간단하고 명확한 인터페이스입니다. 메서드 시그니처가 적절하고, 고유한 사용자 ID를 반환하기 위한
Set<Long>타입 선택이 합리적입니다.src/main/java/konkuk/thip/room/application/port/out/RoomQueryPort.java (2)
3-3: 필요한 import가 적절하게 추가되었습니다.새로운 메서드의
LocalDate파라미터를 위한 import가 올바르게 추가되었습니다.
5-7: 룸 쿼리 메서드가 명확하게 정의되었습니다.
countRecruitingRoomsByBookAndStartDateAfter메서드명이 기능을 잘 설명하고 있으며, 파라미터와 반환 타입이 적절합니다. 특정 책과 날짜 조건에 따른 모집 중인 룸 개수를 조회하는 목적이 명확합니다.src/main/java/konkuk/thip/saved/application/port/out/SavedCommandPort.java (1)
5-6: 책 저장/삭제를 위한 커맨드 메서드가 적절하게 정의되었습니다.
saveBook과deleteBook메서드가 명확한 목적을 가지고 있으며, 일관된 파라미터 타입(Long userId, Long bookId)을 사용하고 있습니다. 커맨드 패턴을 따라void반환 타입을 사용한 것이 적절합니다.src/main/java/konkuk/thip/room/adapter/out/jpa/RoomJpaEntity.java (1)
51-53: @manytoone 매핑 변경 영향 검토 결과 및 추가 확인 사항
현재 코드상에서는@OneToOne가정 로직이 전혀 발견되지 않아,@ManyToOne으로의 변경이 코드 레벨에서는 안전합니다.
그러나 데이터베이스 스키마 측면에서 기존 1:1 제약(unique constraint)이 남아 있을 수 있으니, 아래를 반드시 검토해주세요:
- 마이그레이션 파일(src/main/resources/db/migration)에서
room.category_id에 걸린UNIQUE제약이 제거되었는지- 실제 배포된 스키마 혹은 DDL 스크립트에 남아 있는 인덱스·제약 사항 여부
- 테스트용 엔티티 생성 로직(TestEntityFactory) 및 통합 테스트에서 unique 제약이 반영되지 않아 예외가 발생하지 않는지
src/main/java/konkuk/thip/room/adapter/out/persistence/RoomJpaRepository.java (2)
6-6: 올바른 import 추가입니다.새로운 메서드에서
LocalDate파라미터를 사용하기 위한 적절한 import입니다.
9-9: Spring Data JPA 네이밍 컨벤션을 올바르게 따르고 있습니다.메서드명이 Spring Data JPA의 쿼리 메서드 네이밍 컨벤션을 정확하게 따르고 있으며, 특정 책에 대해 현재 날짜 이후 시작하는 방의 수를 세는 기능이 명확하게 표현되어 있습니다.
src/main/java/konkuk/thip/book/application/port/out/BookApiQueryPort.java (2)
4-4: 적절한 import 추가입니다.새로운 메서드의 반환 타입을 위한 import가 올바르게 추가되었습니다.
6-8: 인터페이스명 변경과 메서드 추가가 적절합니다.
BookApiQueryPort로의 이름 변경이 더 명확한 의미를 제공하며, 새로운findDetailBookByKeyword메서드는 기존 메서드와 일관된 네이밍 패턴을 따르고 있습니다.src/main/java/konkuk/thip/user/adapter/out/persistence/UserCommandPersistenceAdapter.java (2)
13-13: 적절한 예외 코드 import입니다.새로운 메서드에서 사용할
USER_NOT_FOUND예외 코드가 올바르게 import되었습니다.
33-39: 일관성 있는 구현입니다.
findById메서드가 기존 코드 패턴과 일관되게 구현되었습니다:
- Optional을 사용한 null 안전성 확보
- 적절한 예외 처리 (
EntityNotFoundExceptionwithUSER_NOT_FOUND)- 도메인 매퍼를 통한 일관된 객체 변환
이전 학습에서 확인된 Spring Data JPA의 Optional 사용 권장사항도 잘 적용되었습니다.
src/main/java/konkuk/thip/book/application/port/out/BookCommandPort.java (2)
4-6: 새로운 import 선언이 적절합니다.Book 도메인 클래스와 Optional 타입 import가 추가된 메서드 시그니처에 맞게 적절히 추가되었습니다.
10-12: 메서드 시그니처가 적절하게 설계되었습니다.
findByIsbn: ISBN으로 책을 조회하는 메서드로 Optional 반환 타입이 적합합니다.save: 책을 저장하고 ID를 반환하는 일반적인 패턴을 따릅니다.두 메서드 모두 명확한 의미를 가지며 헥사고날 아키텍처의 포트 패턴을 잘 따르고 있습니다.
src/main/java/konkuk/thip/book/application/port/in/BookSavedUseCase.java (1)
1-9: 유스케이스 인터페이스가 잘 설계되었습니다.
- 메서드명
isSavedBook이 기능을 명확히 표현합니다.- 필요한 모든 매개변수(ISBN, 요청 객체, 사용자 ID)를 포함하고 있습니다.
- 클린 아키텍처의 유스케이스 패턴을 적절히 따르고 있습니다.
헥사고날 아키텍처에서 인바운드 포트 역할을 잘 수행할 것으로 보입니다.
src/main/java/konkuk/thip/recentSearch/adapter/out/jpa/SearchType.java (1)
27-27: 예외 처리 개선이 우수합니다.
null반환 대신BusinessException을 던지도록 변경한 것은 매우 좋은 개선사항입니다:
- NullPointerException 발생 가능성을 제거합니다.
- 실패-빠른(fail-fast) 원칙을 따릅니다.
- 적절한 에러 코드(
INVALID_SEARCH_TYPE)를 사용합니다.- 예외 상황을 명시적으로 처리합니다.
이 변경으로 코드의 안정성과 가독성이 향상되었습니다.
src/main/java/konkuk/thip/user/adapter/out/persistence/UserQueryRepositoryImpl.java (2)
13-16: 의존성 주입이 적절히 구현되었습니다.
@RequiredArgsConstructor와 final 필드를 사용한 생성자 주입 방식이 Spring의 모범 사례를 따르고 있습니다.
18-32: QueryDSL 구현이 잘 작성되었습니다.
- 적절한 조인을 통해 책 ID로 사용자 ID를 조회하는 로직이 정확합니다.
distinct()를 사용하여 중복 사용자 ID를 제거합니다.HashSet반환 타입이 고유한 사용자 ID 집합을 나타내기에 적합합니다.- Q클래스를 올바르게 사용하여 타입 안전한 쿼리를 구성했습니다.
복잡한 조인 쿼리임에도 불구하고 가독성이 좋고 논리적으로 올바릅니다.
src/main/java/konkuk/thip/book/adapter/in/web/response/PostBookIsSavedResponse.java (1)
7-15: Response 레코드가 잘 설계되었습니다.
@Builder어노테이션으로 객체 생성이 편리합니다.- 정적 팩토리 메서드
of()로 DTO 변환이 깔끔합니다.- 레코드 구조가 응답 데이터 전송 목적에 적합합니다.
- 필드명이 명확하고 의미가 분명합니다.
API 응답 구조로서 적절한 설계입니다.
src/main/java/konkuk/thip/saved/adapter/out/persistence/SavedQueryPersistenceAdapter.java (1)
13-14: 리포지토리 필드 변경이 일관성을 향상시킵니다.필드명을 더 구체적인 JPA 리포지토리 타입으로 변경한 것이 코드의 명확성을 높입니다.
src/main/java/konkuk/thip/room/adapter/out/persistence/RoomQueryPersistenceAdapter.java (2)
8-8: 필요한 import가 적절히 추가되었습니다.LocalDate import 추가가 새로운 메서드 구현에 필요합니다.
14-15: 필드명 변경이 코드 가독성을 향상시킵니다.더 구체적이고 명확한 필드명으로 변경된 것이 코드의 가독성을 높입니다.
src/main/java/konkuk/thip/common/exception/validation/ConstraintViolationStrategy.java (1)
1-10: 전략 패턴을 활용한 우수한 설계입니다.검증 제약 위반 처리를 위한 전략 패턴 인터페이스가 잘 설계되었습니다. 이 접근 방식은 다양한 어노테이션 타입별로 맞춤형 처리를 가능하게 하며, 확장성과 유지보수성을 높입니다.
supports메서드로 지원하는 어노테이션 타입을 확인handle메서드로 실제 위반 처리 수행깔끔하고 SOLID 원칙을 잘 따른 인터페이스 설계입니다.
src/main/java/konkuk/thip/book/application/port/in/BookSearchUseCase.java (1)
8-10: 사용자 컨텍스트를 포함한 책 검색 인터페이스 개선이 우수합니다.기존
searchBooks메서드에userId파라미터를 추가하고, 새로운searchDetailBooks메서드를 추가한 것이 사용자별 개인화된 검색 기능을 지원하는 좋은 설계입니다.
- 검색 기록 저장을 위한 사용자 컨텍스트 추가
- ISBN 기반 상세 검색 기능 추가
- 명확한 메서드 시그니처
인터페이스 확장이 깔끔하고 목적이 명확합니다.
src/main/java/konkuk/thip/saved/adapter/out/persistence/SavedBookJpaRepository.java (1)
6-9: 잘 구현된 JPA 레포지토리 인터페이스JPA 쿼리 메소드 네이밍 규칙을 올바르게 따르고 있으며, 복합 조건을 사용한 존재 여부 확인과 삭제 메소드가 적절하게 정의되어 있습니다.
src/main/java/konkuk/thip/book/adapter/in/web/BookCommandController.java (1)
21-27: 잘 설계된 REST API 엔드포인트ISBN 유효성 검사, 사용자 인증, 요청/응답 구조가 모두 적절하게 구현되어 있습니다. RESTful 설계 원칙을 잘 따르고 있습니다.
src/main/java/konkuk/thip/book/adapter/out/persistence/BookCommandPersistenceAdapter.java (3)
16-17: 필드명 변경으로 명확성 개선
jpaRepository에서bookJpaRepository로,userMapper에서bookMapper로 필드명을 변경한 것은 클래스의 역할을 더 명확하게 나타내어 가독성을 향상시킵니다.
19-23: ISBN 기반 도서 조회 메서드 구현 확인
findByIsbn메서드가 올바르게 구현되었습니다. Repository 계층에서 JPA 엔티티를 조회하고 매퍼를 통해 도메인 엔티티로 변환하여 Optional로 반환하는 로직이 정확합니다.
25-29: 도서 저장 메서드 구현 확인
save메서드가 적절하게 구현되었습니다. 도메인 엔티티를 JPA 엔티티로 변환하여 저장하고 생성된 ID를 반환하는 로직이 명확하고 올바릅니다.src/main/java/konkuk/thip/book/adapter/out/api/dto/NaverDetailBookParseResult.java (1)
7-15: 레코드 클래스 구조 확인네이버 API 상세 도서 정보를 담는 레코드 클래스 구조가 적절하며,
@Builder어노테이션을 통한 빌더 패턴 지원도 좋습니다.src/main/java/konkuk/thip/common/exception/validation/PatternViolationStrategy.java (1)
13-30: 패턴 검증 전략 구현 확인
PatternViolationStrategy클래스가SizeViolationStrategy와 동일한 패턴으로 일관성 있게 구현되었습니다.@Pattern어노테이션 지원 여부 확인과 에러 응답 생성 로직이 적절합니다.src/test/java/konkuk/thip/book/adapter/in/web/BookIsSavedControllerTest.java (4)
65-96: 테스트 설정 로직 확인테스트용 사용자, 별명, 도서 데이터 생성과 JWT 토큰 생성 로직이 적절하게 구현되었습니다. 테스트 환경 구성이 명확합니다.
106-128: DB 존재 도서 저장 테스트 확인기존에 DB에 있는 도서를 저장하는 테스트 케이스가 잘 구현되었습니다. API 응답 검증과 실제 DB 상태 확인을 모두 포함하여 신뢰성이 높습니다.
130-154: 외부 API 연동 저장 테스트 확인DB에 없는 도서를 외부 API를 통해 조회하여 저장하는 테스트가 적절하게 구현되었습니다. 실제 ISBN을 사용한 외부 API 호출 테스트로 실용적입니다.
156-180: 예외 상황 테스트 케이스 확인중복 저장, 저장되지 않은 책 삭제, 존재하지 않는 책 삭제에 대한 예외 상황 테스트들이 모두 적절하게 구현되었습니다. 각 케이스마다 정확한 에러 코드를 검증하고 있어 신뢰성이 높습니다.
Also applies to: 182-210, 212-246
src/main/java/konkuk/thip/book/adapter/out/api/BookApiNaverApiAdapter.java (3)
9-14: 어댑터 클래스 구조 확인
BookApiQueryPort인터페이스를 구현하는 어댑터 클래스가 적절하게 구성되었습니다.@Component와@RequiredArgsConstructor를 통한 의존성 주입 설정이 올바릅니다.
15-19: 키워드 기반 도서 검색 메서드 확인
findBooksByKeyword메서드가 네이버 API 호출과 XML 파싱을 적절하게 수행합니다. 코드가 간결하고 명확합니다.
21-25: ISBN 기반 상세 검색 메서드 확인
findDetailBookByKeyword메서드가 ISBN을 통한 상세 도서 정보 검색을 올바르게 구현했습니다. 네이버 API 호출과 상세 정보 파싱 로직이 적절합니다.src/main/java/konkuk/thip/book/adapter/in/web/response/GetBookDetailSearchResponse.java (1)
6-32: LGTM!Record 클래스와 정적 팩토리 메서드가 적절하게 구현되었습니다.
src/main/java/konkuk/thip/book/adapter/out/api/NaverApiUtil.java (1)
28-59: 리팩토링이 잘 되었습니다!상세 검색 기능 추가와 함께 공통 로직을
callNaverApi로 추출하여 코드 중복을 제거한 것이 좋습니다.src/main/java/konkuk/thip/common/exception/code/ErrorCode.java (1)
49-56: 에러 코드 번호 중복 없음 확인 완료
스크립트를 통해 실제 enum 상수 정의에서 중복된 에러 코드 번호가 없음을 확인했습니다. 추가 조치가 필요하지 않습니다.src/main/java/konkuk/thip/book/application/service/BookSearchService.java (1)
118-134: 읽기 참여 사용자 수 계산 로직이 우수합니다Set을 사용한 중복 제거와 명확한 주석이 좋습니다. 피드 작성자와 모임 참여자를 모두 고려한 구현이 적절합니다.
src/test/java/konkuk/thip/book/adapter/in/web/BookDetailSearchControllerTest.java (3)
31-33: 보안 필터 비활성화에 대한 확인 필요
@AutoConfigureMockMvc(addFilters = false)로 보안 필터를 비활성화하고 있습니다. 이로 인해 실제 환경에서 발생할 수 있는 인증/인가 관련 문제를 테스트에서 놓칠 수 있습니다.보안 필터를 비활성화한 특별한 이유가 있나요? 가능하다면 필터를 활성화한 상태에서 적절한 인증 토큰을 사용하여 테스트하는 것이 더 안전할 것 같습니다.
62-127: 테스트 데이터 설정이 깔끔합니다엔티티 간의 의존관계를 고려하여 올바른 순서로 테스트 데이터를 생성하고 있으며, 모든 필수 필드가 적절히 설정되어 있습니다.
129-139: 적절한 테스트 데이터 정리의존관계의 역순으로 엔티티를 삭제하여 외래 키 제약 조건 위반을 방지하고 있습니다. 테스트 격리를 위한 좋은 구현입니다.
src/main/java/konkuk/thip/book/adapter/out/api/NaverBookXmlParser.java (3)
21-22: 초기값 설정에 대한 확인
total과start의 초기값을 -1로 설정했는데, 파싱 실패 시 이 값들이 그대로 반환될 수 있습니다. 0이나 다른 의미 있는 기본값을 사용하는 것이 더 적절하지 않을까요?
90-98: 우수한 XML 보안 설정XXE(XML External Entity) 공격을 방지하기 위해 적절한 보안 설정을 적용했습니다. DTD 선언과 외부 엔티티 참조를 비활성화하여 안전한 XML 파싱을 보장합니다.
116-122: 안전한 태그 값 추출태그가 없거나 값이 없을 때 null 대신 빈 문자열을 반환하여 NullPointerException을 방지하고 있습니다. 좋은 방어적 프로그래밍 패턴입니다.
| private final long tokenExpiredMs = 2592000000L; // 30일 | ||
| private final long signupTokenExpiredMs = 2592000000L; // 30일 |
There was a problem hiding this comment.
JWT 토큰 만료 시간 연장의 보안 위험성
토큰 만료 시간을 30일로 연장하는 것은 심각한 보안 위험을 초래할 수 있습니다:
- 액세스 토큰 30일: 일반적인 웹 애플리케이션 표준(15-60분)보다 훨씬 길어 토큰 탈취 시 피해가 클 수 있습니다.
- 가입 토큰 30일: 5분에서 30일로 변경은 특히 위험합니다. 가입 토큰은 단기간만 유효해야 합니다.
- 동일한 만료 시간: 두 토큰의 만료 시간이 같으면 단기 토큰의 보안 이점이 사라집니다.
다음과 같은 개선을 권장합니다:
- private final long tokenExpiredMs = 2592000000L; // 30일
- private final long signupTokenExpiredMs = 2592000000L; // 30일
+ private final long tokenExpiredMs = 900000L; // 15분
+ private final long signupTokenExpiredMs = 300000L; // 5분또는 리프레시 토큰 패턴을 구현하여 보안성을 개선하는 것을 고려해보세요.
📝 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.
| private final long tokenExpiredMs = 2592000000L; // 30일 | |
| private final long signupTokenExpiredMs = 2592000000L; // 30일 | |
| private final long tokenExpiredMs = 900000L; // 15분 | |
| private final long signupTokenExpiredMs = 300000L; // 5분 |
🤖 Prompt for AI Agents
In src/main/java/konkuk/thip/common/security/util/JwtUtil.java around lines 27
to 28, the token expiration times for both access and signup tokens are set to
30 days, which poses security risks. Reduce the access token expiration time to
a much shorter period, such as 15 to 60 minutes, and set the signup token
expiration to a brief duration like 5 minutes. Ensure the two tokens have
distinct expiration times to maintain security benefits. Optionally, consider
implementing a refresh token mechanism for improved security.
| @ManyToOne(fetch = FetchType.LAZY) | ||
| @JoinColumn(name = "user_alias_id", nullable = false) | ||
| private AliasJpaEntity aliasForUserJpaEntity; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
엔티티 관계 변경에 대한 검증 요청
@OneToOne에서 @ManyToOne으로의 변경은 데이터베이스 스키마에 영향을 주는 중요한 변경사항입니다. 이 변경이 비즈니스 요구사항에 맞는지, 그리고 관련 데이터베이스 마이그레이션 스크립트가 준비되었는지 확인이 필요합니다.
다음 스크립트로 관련 마이그레이션 파일과 사용 패턴을 확인해보세요:
🏁 Script executed:
#!/bin/bash
# 데이터베이스 마이그레이션 파일 확인
fd -e sql -e xml | grep -i migration | head -10
# UserJpaEntity와 AliasJpaEntity 관계 사용 패턴 확인
rg -A 3 -B 3 "aliasForUserJpaEntity"Length of output: 5279
DB 마이그레이션 스크립트 미발견 및 관계 변경 검증 필요
현재 @OneToOne에서 @ManyToOne으로 변경된 aliasForUserJpaEntity 필드의 스키마 변경을 담당할 마이그레이션 파일(SQL/XML)을 리포지토리 내에서 찾을 수 없습니다.
fd -e sql -e xml | grep -i migration검색 결과 없음- 테스트 및 매퍼 코드에서는 정상 참조됨
- 관계 변경은 다수의 사용자(UserJpaEntity) → 단일 별칭(AliasJpaEntity)로 비즈니스 로직이 맞아야 함
[조치 요청]
- 이 관계 변경이 비즈니스 요구사항에 부합하는지 확인
- DB 스키마에 반영할 마이그레이션 스크립트(V__*.sql 또는 Flyway/Liquibase 설정) 추가 및 검증
🤖 Prompt for AI Agents
In src/main/java/konkuk/thip/user/adapter/out/jpa/UserJpaEntity.java around
lines 34 to 36, the relationship annotation for aliasForUserJpaEntity was
changed from @OneToOne to @ManyToOne, but no corresponding database migration
script was found. Verify that this relationship change aligns with business
requirements, then create and add the appropriate migration script (SQL or XML)
to update the DB schema accordingly, ensuring it reflects the many-to-one
relationship and test it thoroughly.
| package konkuk.thip.common.exception; | ||
|
|
||
| import konkuk.thip.common.exception.code.ErrorCode; | ||
|
|
||
| public class ExternalApiException extends RuntimeException { | ||
|
|
||
| private final ErrorCode errorCode; | ||
|
|
||
| public ExternalApiException(ErrorCode errorCode) { | ||
| super(errorCode.getMessage()); | ||
| this.errorCode = errorCode; | ||
| } | ||
| public ExternalApiException(ErrorCode errorCode, Exception e) { | ||
| super(errorCode.getMessage(), e); | ||
| this.errorCode = errorCode; | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
외부 API 예외 처리를 위한 적절한 클래스입니다.
외부 API 연동 시 발생하는 예외를 처리하기 위한 클래스가 잘 설계되었습니다. ErrorCode를 활용한 일관된 에러 처리 방식을 따르고 있습니다.
다만 ErrorCode 접근을 위한 getter 메서드가 누락되었습니다:
+ public ErrorCode getErrorCode() {
+ return errorCode;
+ }이 메서드가 있어야 예외 처리 시 ErrorCode에 접근할 수 있습니다.
📝 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.
| package konkuk.thip.common.exception; | |
| import konkuk.thip.common.exception.code.ErrorCode; | |
| public class ExternalApiException extends RuntimeException { | |
| private final ErrorCode errorCode; | |
| public ExternalApiException(ErrorCode errorCode) { | |
| super(errorCode.getMessage()); | |
| this.errorCode = errorCode; | |
| } | |
| public ExternalApiException(ErrorCode errorCode, Exception e) { | |
| super(errorCode.getMessage(), e); | |
| this.errorCode = errorCode; | |
| } | |
| } | |
| package konkuk.thip.common.exception; | |
| import konkuk.thip.common.exception.code.ErrorCode; | |
| public class ExternalApiException extends RuntimeException { | |
| private final ErrorCode errorCode; | |
| public ExternalApiException(ErrorCode errorCode) { | |
| super(errorCode.getMessage()); | |
| this.errorCode = errorCode; | |
| } | |
| public ExternalApiException(ErrorCode errorCode, Exception e) { | |
| super(errorCode.getMessage(), e); | |
| this.errorCode = errorCode; | |
| } | |
| public ErrorCode getErrorCode() { | |
| return errorCode; | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/main/java/konkuk/thip/common/exception/ExternalApiException.java lines 1
to 17, the class lacks a getter method for the private field errorCode. Add a
public getter method named getErrorCode() that returns the errorCode field to
allow external access to the ErrorCode associated with the exception.
.../konkuk/thip/recentSearch/adapter/out/persistence/RecentSearchCommandPersistenceAdapter.java
Outdated
Show resolved
Hide resolved
src/main/java/konkuk/thip/book/application/service/BookSavedService.java
Outdated
Show resolved
Hide resolved
src/main/java/konkuk/thip/saved/adapter/out/persistence/SavedCommandPersistenceAdapter.java
Show resolved
Hide resolved
src/main/java/konkuk/thip/saved/adapter/out/persistence/SavedCommandPersistenceAdapter.java
Show resolved
Hide resolved
…search-add' into feat/#42-post-book-saved # Conflicts: # src/main/java/konkuk/thip/common/exception/code/ErrorCode.java
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
src/main/java/konkuk/thip/book/application/service/BookSavedService.java (1)
57-65: 중복 저장 검증 로직이 적절히 구현되었습니다과거 리뷰에서 지적된 중복 저장 검증 문제가
savedBooks.validateNotAlreadySaved(book)호출을 통해 해결되었습니다. 도메인 객체SavedBooks에서 비즈니스 로직을 처리하는 것이 좋은 접근 방식입니다.src/test/java/konkuk/thip/book/adapter/in/web/BookIsSavedControllerTest.java (1)
106-247: 단위 테스트 분리를 고려해보세요현재 모든 테스트가 통합 테스트로 구성되어 있습니다. 과거 리뷰 코멘트에서 언급된 대로, 비즈니스 로직 검증은 단위 테스트로 분리하는 것이 좋겠습니다.
다음과 같은 구조로 테스트를 분리하는 것을 제안합니다:
- 통합 테스트 (현재 클래스): HTTP 요청/응답 검증
- 단위 테스트 (새로 생성):
BookSavedService의 비즈니스 로직 검증
- 예외 발생 조건 테스트
- 도메인 객체 상호작용 테스트
- 외부 API 호출 모킹 테스트
🧹 Nitpick comments (1)
src/test/java/konkuk/thip/book/adapter/in/web/BookIsSavedControllerTest.java (1)
213-213: 오타 수정이 필요합니다"애러"는 "에러"로 수정해야 합니다.
-@DisplayName("저장하지 않은 책을 삭제하려고 하면 [400 애러 발생]") +@DisplayName("저장하지 않은 책을 삭제하려고 하면 [400 에러 발생]")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
src/main/java/konkuk/thip/book/adapter/in/web/BookCommandController.java(1 hunks)src/main/java/konkuk/thip/book/adapter/out/api/dto/NaverDetailBookParseResult.java(0 hunks)src/main/java/konkuk/thip/book/application/port/in/BookSavedUseCase.java(1 hunks)src/main/java/konkuk/thip/book/application/service/BookSavedService.java(1 hunks)src/main/java/konkuk/thip/book/domain/Book.java(2 hunks)src/main/java/konkuk/thip/book/domain/SavedBooks.java(1 hunks)src/main/java/konkuk/thip/common/exception/code/ErrorCode.java(1 hunks)src/main/java/konkuk/thip/saved/adapter/out/persistence/SavedBookJpaRepository.java(1 hunks)src/main/java/konkuk/thip/saved/adapter/out/persistence/SavedCommandPersistenceAdapter.java(1 hunks)src/main/java/konkuk/thip/saved/adapter/out/persistence/SavedQueryPersistenceAdapter.java(1 hunks)src/main/java/konkuk/thip/saved/application/port/out/SavedQueryPort.java(1 hunks)src/test/java/konkuk/thip/book/adapter/in/web/BookIsSavedControllerTest.java(1 hunks)
💤 Files with no reviewable changes (1)
- src/main/java/konkuk/thip/book/adapter/out/api/dto/NaverDetailBookParseResult.java
🚧 Files skipped from review as they are similar to previous changes (4)
- src/main/java/konkuk/thip/book/adapter/in/web/BookCommandController.java
- src/main/java/konkuk/thip/common/exception/code/ErrorCode.java
- src/main/java/konkuk/thip/saved/adapter/out/persistence/SavedBookJpaRepository.java
- src/main/java/konkuk/thip/saved/adapter/out/persistence/SavedCommandPersistenceAdapter.java
🧰 Additional context used
🧠 Learnings (2)
src/main/java/konkuk/thip/saved/application/port/out/SavedQueryPort.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/saved/adapter/out/persistence/SavedQueryPersistenceAdapter.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 (1)
- GitHub Check: build
🔇 Additional comments (14)
src/main/java/konkuk/thip/saved/application/port/out/SavedQueryPort.java (2)
3-3: import 추가 적절함
SavedBooks도메인 클래스에 대한 import가 새로 추가된 메서드의 반환 타입으로 적절하게 사용되었습니다.
7-7: CQRS 패턴 준수
findByUserId메서드가 QueryPort에 적절히 추가되었습니다. 기존 학습 내용에 따르면 QueryPort에는 조회 API의 response에 해당하는 데이터를 DB로부터 조회하는 메서드를 추가하는 것이 컨벤션에 맞습니다.src/main/java/konkuk/thip/book/application/port/in/BookSavedUseCase.java (1)
5-7: UseCase 인터페이스 설계 우수함이전 리뷰 댓글에서 언급된 controller 계층의 DTO 의존성 제거 제안이 잘 반영되었습니다. 메서드 시그니처가 primitive type 파라미터를 사용하여 application 계층의 독립성을 유지하고 있습니다.
src/main/java/konkuk/thip/saved/adapter/out/persistence/SavedQueryPersistenceAdapter.java (3)
27-29: 의존성 추가 적절함새로운 기능 구현을 위한
UserJpaRepository와BookMapper의존성이 적절하게 추가되었습니다.
40-42: 사용자 검증 로직 우수함사용자 존재 여부를 확인하고 적절한 예외 처리를 수행하는 로직이 잘 구현되어 있습니다.
EntityNotFoundException을 사용하여 일관된 예외 처리 패턴을 따르고 있습니다.
44-49: 엔티티-도메인 매핑 로직 우수함Stream API를 사용하여 JPA 엔티티를 도메인 객체로 깔끔하게 변환하고 있습니다.
BookMapper를 활용한 매핑과 최종적으로SavedBooks도메인 객체로 래핑하는 로직이 적절합니다.src/main/java/konkuk/thip/book/domain/SavedBooks.java (3)
14-22: 생성자 검증 로직 우수함Set 변환을 통한 중복 검증과 불변 컬렉션 사용이 잘 구현되어 있습니다. 도메인 객체의 불변성을 보장하면서 비즈니스 규칙을 적절히 검증하고 있습니다.
24-29: 비즈니스 규칙 검증 메서드 우수함중복 저장 방지를 위한 검증 로직이 명확하게 구현되어 있습니다. 메서드명이 의도를 잘 표현하고 있으며, 적절한 예외 처리를 수행합니다.
31-36: 삭제 가능성 검증 로직 우수함저장되지 않은 책의 삭제 시도를 방지하는 검증 로직이 명확하게 구현되어 있습니다. 비즈니스 규칙을 도메인 레벨에서 적절히 강제하고 있습니다.
src/main/java/konkuk/thip/book/domain/Book.java (3)
31-42: ISBN 기반 동등성 구현 우수함ISBN을 기준으로 한
equals와hashCode메서드 구현이 적절합니다. 도메인 객체의 식별성을 명확히 정의하여SavedBooks의 Set 연산이 올바르게 동작할 수 있도록 지원합니다.
44-56: 정적 팩토리 메서드 우수함ID 없이 Book 인스턴스를 생성하는
withoutId메서드가 잘 구현되어 있습니다. 도메인 객체 생성 패턴을 명확히 하고 가독성을 향상시킵니다.
58-71: 불변성 보장 팩토리 메서드 우수함기존 인스턴스를 기반으로 ID를 설정한 새로운 인스턴스를 생성하는
withId메서드가 적절하게 구현되어 있습니다. 불변성을 유지하면서 객체 생성 워크플로우를 지원합니다.src/main/java/konkuk/thip/book/application/service/BookSavedService.java (1)
32-52: Book 조회 및 생성 로직이 개선되었습니다과거 리뷰에서 제안된 정적 팩토리 메서드
Book.withoutId()를 사용하여 코드 가독성과 도메인 책임 분리가 개선되었습니다. 또한 삭제 요청 시 책이 존재하지 않을 때BOOK_NOT_FOUND예외를 명확하게 던지는 로직도 적절합니다.src/test/java/konkuk/thip/book/adapter/in/web/BookIsSavedControllerTest.java (1)
65-96: 테스트 데이터 설정이 적절합니다테스트에 필요한 사용자, 별칭, 책 데이터를 적절히 설정하고 JWT 토큰을 생성하는 로직이 잘 구성되어 있습니다.
src/main/java/konkuk/thip/book/application/service/BookSavedService.java
Outdated
Show resolved
Hide resolved
| @Test | ||
| @DisplayName("DB에 책이 존재하지 않을 때 해당 책을 DB에 저장하고, 해당 책을 저장하려고 할 때 [책 저장 성공]") | ||
| void saveBook_whenBookNotExist_thenSaveAndSuccess() throws Exception { | ||
|
|
||
| // given | ||
| String newIsbn = "9791195710447"; // DB에 없고 실제 존재하는 책 ISBN | ||
| PostBookIsSavedRequest request = new PostBookIsSavedRequest(true); | ||
|
|
||
| // when | ||
| ResultActions result = mockMvc.perform(post("/books/{isbn}/saved", newIsbn) | ||
| .header("Authorization", "Bearer " + testToken) | ||
| .contentType(MediaType.APPLICATION_JSON) | ||
| .content(objectMapper.writeValueAsString(request))); | ||
|
|
||
| // then | ||
| result.andExpect(status().isOk()) | ||
| .andExpect(jsonPath("$.data.isbn").value(newIsbn)) | ||
| .andExpect(jsonPath("$.data.isSaved").value(true)); | ||
|
|
||
| // 실제 저장됐는지 검증 | ||
| Optional<BookJpaEntity> bookJpaEntity = bookJpaRepository.findByIsbn(newIsbn); | ||
| assertThat(bookJpaEntity).isPresent(); | ||
| boolean exists = savedBookJpaRepository.existsByUserJpaEntity_UserIdAndBookJpaEntity_BookId(userId, bookJpaEntity.get().getBookId()); | ||
| assertThat(exists).isTrue(); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
실제 외부 API 호출에 대한 테스트 안정성 문제
실제 네이버 API를 호출하는 테스트는 네트워크 상태나 외부 API 변경에 따라 불안정할 수 있습니다.
다음과 같은 개선 방안을 제안합니다:
- 외부 API 호출을 모킹하는 별도의 테스트 작성
- 또는 테스트 프로파일에서 외부 API 호출을 스텁으로 대체
- 실제 API 호출 테스트는 별도의 통합 테스트 슈트로 분리
@MockBean
private BookApiQueryPort bookApiQueryPort;
@Test
void saveBook_whenBookNotExist_thenSaveAndSuccess() throws Exception {
// given
given(bookApiQueryPort.findDetailBookByKeyword(anyString()))
.willReturn(mockNaverDetailBookParseResult);
// ... 나머지 테스트 로직
}🤖 Prompt for AI Agents
In src/test/java/konkuk/thip/book/adapter/in/web/BookIsSavedControllerTest.java
around lines 130 to 154, the test currently performs an actual external API call
which can cause instability due to network or API changes. To fix this, mock the
external API call by injecting a mock of the BookApiQueryPort and stub its
method (e.g., findDetailBookByKeyword) to return a predefined mock response.
This will isolate the test from real API calls and improve reliability.
Alternatively, move real API call tests to a separate integration test suite and
use test profiles or mocks for unit tests.
| @AfterEach | ||
| void tearDown() { | ||
| savedBookJpaRepository.deleteAll(); | ||
| bookJpaRepository.deleteAll(); | ||
| userJpaRepository.deleteAll(); | ||
| aliasJpaRepository.deleteAll(); | ||
| } |
There was a problem hiding this comment.
데이터 정리 순서를 개선해야 합니다
현재 테스트 데이터 정리 순서가 외래 키 제약 조건에 위배될 수 있습니다. 자식 테이블부터 삭제해야 합니다.
다음과 같이 삭제 순서를 변경하는 것을 제안합니다:
@AfterEach
void tearDown() {
savedBookJpaRepository.deleteAll();
- bookJpaRepository.deleteAll();
- userJpaRepository.deleteAll();
- aliasJpaRepository.deleteAll();
+ userJpaRepository.deleteAll();
+ aliasJpaRepository.deleteAll();
+ bookJpaRepository.deleteAll();
}📝 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.
| @AfterEach | |
| void tearDown() { | |
| savedBookJpaRepository.deleteAll(); | |
| bookJpaRepository.deleteAll(); | |
| userJpaRepository.deleteAll(); | |
| aliasJpaRepository.deleteAll(); | |
| } | |
| @AfterEach | |
| void tearDown() { | |
| savedBookJpaRepository.deleteAll(); | |
| userJpaRepository.deleteAll(); | |
| aliasJpaRepository.deleteAll(); | |
| bookJpaRepository.deleteAll(); | |
| } |
🤖 Prompt for AI Agents
In src/test/java/konkuk/thip/book/adapter/in/web/BookIsSavedControllerTest.java
around lines 98 to 104, the tearDown method deletes test data in an order that
may violate foreign key constraints. To fix this, reorder the deletions to start
from child tables to parent tables, ensuring that dependent records are deleted
before their references. Adjust the deleteAll calls so that aliasJpaRepository
and savedBookJpaRepository are cleared before bookJpaRepository and
userJpaRepository.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/test/java/konkuk/thip/book/adapter/in/web/BookChangeSavedControllerTest.java (2)
230-246: DB 미존재 책 삭제 테스트 승인 및 예외 처리 전략 제안DB에 존재하지 않는 책을 삭제하려고 할 때의 예외 처리가 테스트되었습니다.
현재
BOOK_NOT_SAVED_DB_CANNOT_DELETE에러 코드를 사용하고 있는데, 이는 "책이 DB에 저장되지 않아서 삭제할 수 없다"는 의미로 해석됩니다. 이는 사용자의 관점에서 볼 때 적절한 에러 메시지로 보입니다.BOOK_NOT_FOUND보다는 현재 사용하는 에러 코드가 더 명확한 의미를 전달합니다.
212-228: 예외 상황 테스트 구현이 올바릅니다.저장하지 않은 책을 삭제하려 할 때 적절한 에러 코드를 반환하는지 확인하는 테스트입니다. 다만 213행의 주석에서 "애러"를 "에러"로 수정하는 것이 올바른 맞춤법입니다.
- @DisplayName("저장하지 않은 책을 삭제하려고 하면 [400 애러 발생]") + @DisplayName("저장하지 않은 책을 삭제하려고 하면 [400 에러 발생]")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.github/workflows/ci-workflow.yml(1 hunks)src/main/java/konkuk/thip/book/adapter/out/persistence/BookCommandPersistenceAdapter.java(2 hunks)src/main/java/konkuk/thip/book/application/port/out/BookCommandPort.java(1 hunks)src/main/java/konkuk/thip/book/application/service/BookSavedService.java(1 hunks)src/main/java/konkuk/thip/book/application/service/BookSearchService.java(2 hunks)src/main/java/konkuk/thip/common/exception/code/ErrorCode.java(1 hunks)src/test/java/konkuk/thip/book/adapter/in/web/BookChangeSavedControllerTest.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/main/java/konkuk/thip/book/application/port/out/BookCommandPort.java
- src/main/java/konkuk/thip/book/application/service/BookSavedService.java
- src/main/java/konkuk/thip/common/exception/code/ErrorCode.java
🧰 Additional context used
🧠 Learnings (2)
src/main/java/konkuk/thip/book/application/service/BookSearchService.java (2)
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 안전성을 위해 권장됩니다.
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/persistence/BookCommandPersistenceAdapter.java (2)
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 안전성을 위해 권장됩니다.
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 (2)
src/main/java/konkuk/thip/book/application/service/BookSearchService.java (1)
src/main/java/konkuk/thip/common/exception/EntityNotFoundException.java (1)
EntityNotFoundException(5-10)
src/main/java/konkuk/thip/book/adapter/out/persistence/BookCommandPersistenceAdapter.java (1)
src/main/java/konkuk/thip/common/exception/EntityNotFoundException.java (1)
EntityNotFoundException(5-10)
🔇 Additional comments (19)
.github/workflows/ci-workflow.yml (1)
48-52: Redis 설정 추가 승인새로운 책 저장 기능의 테스트를 위해 Redis 서버 설정이 적절히 추가되었습니다. 액션 버전과 Redis 버전이 명시되어 있어 좋습니다.
src/main/java/konkuk/thip/book/adapter/out/persistence/BookCommandPersistenceAdapter.java (4)
28-32: save 메서드 추가 승인도메인 객체를 JPA 엔티티로 변환하여 저장하는 로직이 깔끔하게 구현되었습니다. BookMapper에 변환 책임을 위임한 것이 적절합니다.
21-25: findByIsbn 메서드 변경 영향 없음 확인
Book→Optional에서 Book을 반환하고 EntityNotFoundException을 던지도록 수정된 이후, 호출부와 레포지토리·테스트 모두 정상 동작함을 검증했습니다.
- BookSearchService.java, BookSavedService.java: EntityNotFoundException을 catch하여 예외 처리
- BookJpaRepository.java: Optional 반환 타입 유지
- BookChangeSavedControllerTest.java: 직접 JPA 리포지토리 호출로 영향 없음
위 변경은 CQRS CommandPort 컨벤션에 부합하며, 추가 조치가 필요하지 않습니다.
21-25: Optional 패턴에서 예외 처리 패턴으로의 변경이 적절합니다.CommandPort의 컨벤션에 따라 Optional 대신 EntityNotFoundException을 던지는 방식으로 변경된 것이 올바릅니다. 이는 검색된 학습 내용과 일치하며, 호출하는 쪽에서 명확한 예외 처리가 가능합니다.
28-32: 도메인 객체 저장 구현이 올바릅니다.도메인 Book을 JPA 엔티티로 변환하고 저장한 후 생성된 ID를 반환하는 로직이 적절합니다. 매퍼를 통한 변환과 JPA 레포지토리를 통한 저장이 올바르게 구현되었습니다.
src/main/java/konkuk/thip/book/application/service/BookSearchService.java (2)
88-100: 예외 처리 방식 변경 승인BookCommandPersistenceAdapter의 findByIsbn 메서드 변경에 따라 적절히 예외 처리 방식으로 변경되었습니다. 책이 DB에 없는 경우 기본값을 반환하는 로직이 명확하게 구현되었습니다.
88-100: 예외 처리를 통한 기본값 반환 로직이 적절합니다.DB에서 책을 찾을 수 없을 때 EntityNotFoundException을 잡아서 기본값(모집 중인 방 0개, 참여자 0명, 저장 안함)을 반환하는 것이 검색 서비스의 역할에 적합합니다. 사용자 경험 측면에서도 예외로 인한 오류보다는 기본 정보를 제공하는 것이 더 나은 접근입니다.
src/test/java/konkuk/thip/book/adapter/in/web/BookChangeSavedControllerTest.java (12)
65-104: 테스트 설정 및 정리 메서드 승인테스트 데이터 설정과 정리가 적절히 구현되었습니다. 외래키 관계를 고려한 삭제 순서와 테스트 격리를 위한 완전한 데이터 정리가 잘 되어 있습니다.
106-128: 책 저장 성공 테스트 승인기존 책을 저장하는 시나리오가 올바르게 테스트되었습니다. HTTP 응답 검증과 실제 DB 상태 검증이 모두 포함되어 있어 완전한 테스트입니다.
130-154: 책 미존재 시 저장 테스트 승인DB에 책이 없는 경우 자동으로 책을 생성하고 저장하는 중요한 비즈니스 로직이 적절히 테스트되었습니다. 실제 ISBN을 사용한 것도 좋은 접근입니다.
156-180: 중복 저장 방지 테스트 승인이미 저장된 책을 다시 저장하려고 할 때의 예외 처리가 올바르게 테스트되었습니다. 에러 코드 검증까지 포함되어 완전한 테스트입니다.
182-210: 책 삭제 성공 테스트 승인저장된 책을 삭제하는 정상 시나리오가 올바르게 테스트되었습니다. HTTP 응답과 실제 DB 상태 변경 모두 검증되어 있습니다.
212-228: 저장되지 않은 책 삭제 방지 테스트 승인저장하지 않은 책을 삭제하려고 할 때의 예외 처리가 올바르게 테스트되었습니다. 적절한 에러 코드가 사용되었습니다.
65-96: 테스트 설정이 잘 구성되어 있습니다.사용자, 별명, 책 엔티티를 적절히 생성하고 JWT 토큰도 준비하여 통합 테스트를 위한 완전한 환경을 구성했습니다. 테스트 간 격리를 위한 설정이 잘 되어 있습니다.
106-128: 기존 책 저장 테스트가 포괄적으로 구현되었습니다.HTTP 응답 검증과 실제 데이터베이스 상태 확인을 모두 포함하여 엔드투엔드 테스트가 올바르게 구현되었습니다. MockMvc와 직접 레포지토리 접근을 통한 이중 검증이 효과적입니다.
130-154: DB에 없는 책 저장 시나리오 테스트가 우수합니다.실제 존재하는 ISBN을 사용하여 책이 DB에 없을 때 네이버 API를 통해 정보를 가져와 저장하는 전체 플로우를 테스트하는 것이 현실적이고 포괄적입니다.
156-180: 중복 저장 방지 테스트가 적절합니다.이미 저장된 책을 다시 저장하려 할 때 적절한 에러 코드(BOOK_ALREADY_SAVED)를 반환하는지 확인하는 테스트가 비즈니스 로직을 올바르게 검증합니다.
182-210: 저장된 책 삭제 테스트가 완전합니다.책 삭제 성공 시나리오에서 HTTP 응답과 실제 데이터베이스에서의 삭제 여부를 모두 검증하여 완전한 테스트를 구현했습니다.
230-246: DB에 없는 책 삭제 시나리오 테스트가 완전합니다.존재하지 않는 책을 삭제하려 할 때 적절한 에러 코드(BOOK_NOT_SAVED_DB_CANNOT_DELETE)를 반환하는지 확인하는 테스트로, 모든 예외 상황을 포괄적으로 다루고 있습니다.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/ci-workflow.yml (1)
42-44: trailing space 제거 및 가독성 개선 제안
44번째 줄 끝에 공백이 남아 있어 YAMLlint 오류가 발생합니다. 의미 없는 공백은 제거해주세요. 또한printf뒤에\n을 추가하면 사람이 파일을 열어봤을 때 줄바꿈이 적용돼 가독성이 조금 더 좋아집니다.- printf "%s" "${{ secrets.APPLICATION_YML_TEST }}" > ${{ env.TEST_RESOURCE_PATH }}/application-test.yml␠ + printf "%s\n" "${{ secrets.APPLICATION_YML_TEST }}" > ${{ env.TEST_RESOURCE_PATH }}/application-test.yml
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/ci-workflow.yml(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.github/workflows/ci-workflow.yml
[error] 44-44: trailing spaces
(trailing-spaces)
🔇 Additional comments (1)
.github/workflows/ci-workflow.yml (1)
49-53: Redis 단계 주석 처리 확인 필요
Redis 기동 단계가 주석 처리되었습니다. 통합 테스트나 애플리케이션 구동 시 Redis 의존성이 있는 경우, CI 빌드가 실패할 수 있으니 확인 부탁드립니다. 만약 선택적으로 Redis를 띄워야 한다면if: ${{ env.USE_REDIS == 'true' }}같은 조건부 실행을 고려해볼 수 있습니다.
|
3 participants
#️⃣ 연관된 이슈
📝 작업 내용
BookCommandController에서BookSavedUseCase를 구현한BookSavedService로 Request가 들어옴.DB에 책이 존재하면 사용자의 책 저장테이블(saveBook)에 책 저장 삭제.
DB에 책이 존재하지 않을경우 예외처리 (.. 여기서 고민 리뷰 요구사항 확인해주세욧)
각 저장,삭제 요청일 시 사용자의 책 저장테이블을 먼저 검사한 뒤, 이미 저장된 책이라면 중복 저장 예외처리, 이미 삭제된 책이라면(사용자와 책이 관계가 없다면) 삭제 예외처리가 되어있습니다.
5.서비스가 반환결과 반환 Response 타입으로 변경후 Response반환.
📸 스크린샷
💬 리뷰 요구사항
구현 하면서 생긴 고민이 하나 있는데요..
BOOK_NOT_SAVED_CANNOT_DELETE,"저장되어있지 않은 책은 저장삭제 할 수 없습니다."로 하고있는데요. 이걸BOOK_NOT_FOUND,"존재하지 않는 책입니다."로 예외코드를 작성해야할지 고민입니다.제가 생각했을때는 저희 서비스 로직에서 사용자가 책을 저장하려고할때 DB에 책을 저장하는데 DB에 저장하지않은 책을 저장 삭제하는 요청이라면, 책 저장시에 정상적으로 책 저장을 하지않았을 경우라고 생각해서
BOOK_NOT_SAVED_CANNOT_DELETE로 작성했는데요. 그러면BOOK_NOT_FOUND로 정상적으로 책이 저장되지않았음을 알리는게 더 나은건지? 의문이 듭니다..서비스 로직에서는
BOOK_NOT_SAVED_CANNOT_DELETE예외처리가 맞지만 실제 서버 내부 에러로 인해 책이 정상적으로 저장되지 않았을 경우BOOK_NOT_FOUND로 예외의 원인을 바로 찾을 수 있도록 버그 발견시에 수정에 용이 하도록 예외처리를 해야할지...여러분들의 의견이 궁금합니다..😭📌 PR 진행 시 이러한 점들을 참고해 주세요
Summary by CodeRabbit
신규 기능
버그 수정
테스트
CI/환경