-
Notifications
You must be signed in to change notification settings - Fork 0
[feat] 저장한 책 또는 참여 중 모임의 책 조회 api 구현 및 패키지 정리 #166
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
5910157
7945fe0
cda7aab
a534943
1384b40
5a2bfb7
c2de19a
44216df
d100c52
c043a82
d111626
280b589
89dd638
cc24720
f5ad9a2
1f54669
eb5f197
10b2123
bb52613
4e6b00f
3587660
5b48353
2d330a3
11055e0
9531892
cccc547
1593313
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -8,6 +8,8 @@ on: | |||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| permissions: | ||||||||||||||||||||||||||||||||||||||||||||||
| contents: read | ||||||||||||||||||||||||||||||||||||||||||||||
| checks: write | ||||||||||||||||||||||||||||||||||||||||||||||
| pull-requests: write | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| env: | ||||||||||||||||||||||||||||||||||||||||||||||
| RESOURCE_PATH: src/main/resources | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -19,13 +21,6 @@ jobs: | |||||||||||||||||||||||||||||||||||||||||||||
| steps: | ||||||||||||||||||||||||||||||||||||||||||||||
| - uses: actions/checkout@v4 | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| # - name: Cache Gradle packages | ||||||||||||||||||||||||||||||||||||||||||||||
| # uses: actions/cache@v3 | ||||||||||||||||||||||||||||||||||||||||||||||
| # with: | ||||||||||||||||||||||||||||||||||||||||||||||
| # path: ~/.gradle/caches | ||||||||||||||||||||||||||||||||||||||||||||||
| # key: gradle-${{ runner.os }}-${{ hashFiles('**/*.gradle*', '**/gradle-wrapper.properties') }} | ||||||||||||||||||||||||||||||||||||||||||||||
| # restore-keys: gradle-${{ runner.os }}- | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| - name: Set up JDK 17 | ||||||||||||||||||||||||||||||||||||||||||||||
| uses: actions/setup-java@v3 | ||||||||||||||||||||||||||||||||||||||||||||||
| with: | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -50,4 +45,17 @@ jobs: | |||||||||||||||||||||||||||||||||||||||||||||
| redis-version: 7 | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| - name: 🐘 build with Gradle | ||||||||||||||||||||||||||||||||||||||||||||||
| run: ./gradlew build | ||||||||||||||||||||||||||||||||||||||||||||||
| run: ./gradlew build --parallel --stacktrace | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| - name: ✉️ Post test results as a comment on the PR | ||||||||||||||||||||||||||||||||||||||||||||||
| uses: EnricoMi/publish-unit-test-result-action@v2 | ||||||||||||||||||||||||||||||||||||||||||||||
| if: always() | ||||||||||||||||||||||||||||||||||||||||||||||
| with: | ||||||||||||||||||||||||||||||||||||||||||||||
| files: '**/build/test-results/test/TEST-*.xml' | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| - name: 🙁 Annotate failed test cases on changed lines | ||||||||||||||||||||||||||||||||||||||||||||||
| uses: mikepenz/action-junit-report@v3 | ||||||||||||||||||||||||||||||||||||||||||||||
| if: always() | ||||||||||||||||||||||||||||||||||||||||||||||
| with: | ||||||||||||||||||||||||||||||||||||||||||||||
| report_paths: '**/build/test-results/test/TEST-*.xml' | ||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+50
to
+60
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion 액션 버전을 커밋 SHA 로 고정하여 서플라이 체인 리스크를 줄이세요 예시 diff: - uses: EnricoMi/publish-unit-test-result-action@v2
+ uses: EnricoMi/publish-unit-test-result-action@v2.15.0 # 0e9f0b1
...
- uses: mikepenz/action-junit-report@v3
+ uses: mikepenz/action-junit-report@v3.8.0 # a1b2c3d📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||
| token: ${{ github.token }} | ||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+50
to
+61
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 오호 |
||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| package konkuk.thip.book.adapter.in.web.response; | ||
|
|
||
| import konkuk.thip.book.application.port.in.dto.BookSelectableResult; | ||
|
|
||
| import java.util.List; | ||
|
|
||
| public record BookSelectableListResponse( | ||
| List<BookSelectableResult> bookList | ||
| ) { | ||
| public static BookSelectableListResponse of(List<BookSelectableResult> bookSelectableResults) { | ||
| return new BookSelectableListResponse(bookSelectableResults); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -7,20 +7,25 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import konkuk.thip.book.domain.Book; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import konkuk.thip.common.exception.EntityNotFoundException; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import konkuk.thip.room.adapter.out.persistence.repository.RoomJpaRepository; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import konkuk.thip.book.adapter.out.jpa.SavedBookJpaEntity; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import konkuk.thip.book.adapter.out.persistence.repository.SavedBookJpaRepository; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import konkuk.thip.user.adapter.out.jpa.UserJpaEntity; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import konkuk.thip.user.adapter.out.persistence.repository.UserJpaRepository; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import lombok.RequiredArgsConstructor; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import org.springframework.stereotype.Repository; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import java.util.Optional; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import static konkuk.thip.common.exception.code.ErrorCode.BOOK_NOT_FOUND; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import static konkuk.thip.common.exception.code.ErrorCode.ROOM_NOT_FOUND; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import static konkuk.thip.common.exception.code.ErrorCode.*; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| @Repository | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| @RequiredArgsConstructor | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| public class BookCommandPersistenceAdapter implements BookCommandPort { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private final UserJpaRepository userJpaRepository; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private final RoomJpaRepository roomJpaRepository; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private final BookJpaRepository bookJpaRepository; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private final SavedBookJpaRepository savedBookJpaRepository; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private final BookMapper bookMapper; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| @Override | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -61,4 +66,24 @@ public Book findBookByRoomId(Long roomId) { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ).getBookJpaEntity(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return bookMapper.toDomainEntity(bookJpaEntity); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // 사용자가 책을 저장 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| @Override | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| public void saveSavedBook(Long userId, Long bookId) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| UserJpaEntity user = userJpaRepository.findById(userId) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .orElseThrow(() -> new EntityNotFoundException(USER_NOT_FOUND)); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| BookJpaEntity book = bookJpaRepository.findById(bookId) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .orElseThrow(() -> new EntityNotFoundException(BOOK_NOT_FOUND)); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| SavedBookJpaEntity entity = SavedBookJpaEntity.builder() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .userJpaEntity(user) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .bookJpaEntity(book) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .build(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| savedBookJpaRepository.save(entity); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+70
to
+82
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion 중복 저장 처리 로직 추가를 고려해보세요. 현재 구현에서는 이미 저장된 책을 다시 저장할 때의 중복 처리가 없습니다. 동일한 사용자-책 조합이 중복으로 저장될 가능성이 있습니다. @Override
public void saveSavedBook(Long userId, Long bookId) {
+ // 이미 저장된 책인지 확인
+ if (savedBookJpaRepository.existsByUserIdAndBookId(userId, bookId)) {
+ return; // 이미 저장되어 있으면 무시하거나 예외 처리
+ }
UserJpaEntity user = userJpaRepository.findById(userId)
.orElseThrow(() -> new EntityNotFoundException(USER_NOT_FOUND));
BookJpaEntity book = bookJpaRepository.findById(bookId)
.orElseThrow(() -> new EntityNotFoundException(BOOK_NOT_FOUND));📝 Committable suggestion
Suggested change
🤖 Prompt for AI AgentsThere was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain@seongjunnoh 맞습니다! Clean Architecture 관점에서 볼 때 어댑터는 순수하게 저장/조회 기능만 담당해야 하고, 비즈니스 로직(중복 검사 포함)은 서비스 레이어에서 처리하는 것이 올바른 접근입니다. 실제로 private void validateSaveBookAction(boolean isSaveRequest, boolean isAlreadySaved) {
if (isSaveRequest && isAlreadySaved) {
throw new BusinessException(BOOK_ALREADY_SAVED);
}
// ...
}어댑터는 단순히 데이터 저장 책임만 가져야 한다는 의견에 동의합니다. 제안을 철회하겠습니다. 🏁 Script executed: Length of output: 3010 @seongjunnoh 완전히 맞습니다! 실제 boolean alreadySaved = bookQueryPort.existsSavedBookByUserIdAndBookId(userId, book.getId());
validateSaveBookAction(isSaveRequest, alreadySaved);
private void validateSaveBookAction(boolean isSaveRequest, boolean alreadySaved) {
if (isSaveRequest && alreadySaved) {
throw new BusinessException(BOOK_ALREADY_SAVED);
}
// ...
}서비스 레이어에서 비즈니스 로직(중복 검사, 유효성 검증)을 담당하고, 어댑터는 순수하게 저장/삭제 동작만 수행하는 Clean Architecture 구조가 잘 구현되어 있습니다. 어댑터에 비즈니스 로직을 추가하자는 제안은 부적절했습니다. 현재 구조가 올바른 레이어 분리를 보여주고 있네요. ✏️ Learnings added
🧠 Learnings used |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // 사용자가 저장한 책을 삭제 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| @Override | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| public void deleteSavedBook(Long userId, Long bookId) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| savedBookJpaRepository.deleteByUserIdAndBookId(userId, bookId); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -2,15 +2,52 @@ | |||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| import konkuk.thip.book.adapter.out.mapper.BookMapper; | ||||||||||||||||||||||||||||||||||||||
| import konkuk.thip.book.adapter.out.persistence.repository.BookJpaRepository; | ||||||||||||||||||||||||||||||||||||||
| import konkuk.thip.book.adapter.out.persistence.repository.SavedBookJpaRepository; | ||||||||||||||||||||||||||||||||||||||
| import konkuk.thip.book.application.port.out.BookQueryPort; | ||||||||||||||||||||||||||||||||||||||
| import konkuk.thip.book.domain.Book; | ||||||||||||||||||||||||||||||||||||||
| import konkuk.thip.common.exception.EntityNotFoundException; | ||||||||||||||||||||||||||||||||||||||
| import konkuk.thip.user.adapter.out.jpa.UserJpaEntity; | ||||||||||||||||||||||||||||||||||||||
| import konkuk.thip.user.adapter.out.persistence.repository.UserJpaRepository; | ||||||||||||||||||||||||||||||||||||||
| import lombok.RequiredArgsConstructor; | ||||||||||||||||||||||||||||||||||||||
| import org.springframework.stereotype.Repository; | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| import java.util.List; | ||||||||||||||||||||||||||||||||||||||
| import java.util.stream.Collectors; | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| import static konkuk.thip.common.exception.code.ErrorCode.USER_NOT_FOUND; | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| @Repository | ||||||||||||||||||||||||||||||||||||||
| @RequiredArgsConstructor | ||||||||||||||||||||||||||||||||||||||
| public class BookQueryPersistenceAdapter implements BookQueryPort { | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| private final UserJpaRepository userJpaRepository; | ||||||||||||||||||||||||||||||||||||||
| private final BookJpaRepository bookJpaRepository; | ||||||||||||||||||||||||||||||||||||||
| private final SavedBookJpaRepository savedBookJpaRepository; | ||||||||||||||||||||||||||||||||||||||
| private final BookMapper bookMapper; | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| @Override | ||||||||||||||||||||||||||||||||||||||
| public boolean existsSavedBookByUserIdAndBookId(Long userId, Long bookId) { | ||||||||||||||||||||||||||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 옿 감사합니다 |
||||||||||||||||||||||||||||||||||||||
| return savedBookJpaRepository.existsByUserIdAndBookId(userId, bookId); | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| @Override | ||||||||||||||||||||||||||||||||||||||
| public List<Book> findSavedBooksByUserId(Long userId) { | ||||||||||||||||||||||||||||||||||||||
| UserJpaEntity user = userJpaRepository.findById(userId) | ||||||||||||||||||||||||||||||||||||||
| .orElseThrow(() -> new EntityNotFoundException(USER_NOT_FOUND)); | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| return bookJpaRepository.findSavedBooksByUserId(user.getUserId()).stream() | ||||||||||||||||||||||||||||||||||||||
| .map(bookMapper::toDomainEntity) | ||||||||||||||||||||||||||||||||||||||
| .collect(Collectors.toList()); | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+34
to
+41
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion User 존재 검증 최적화 및 불필요한 엔티티 로딩 제거 제안 현재 적용 예시(diff): - UserJpaEntity user = userJpaRepository.findById(userId)
- .orElseThrow(() -> new EntityNotFoundException(USER_NOT_FOUND));
-
- return bookJpaRepository.findSavedBooksByUserId(user.getUserId()).stream()
- .map(bookMapper::toDomainEntity)
- .collect(Collectors.toList());
+ if (!userJpaRepository.existsById(userId)) {
+ throw new EntityNotFoundException(USER_NOT_FOUND);
+ }
+ return bookJpaRepository.findSavedBooksByUserId(userId).stream()
+ .map(bookMapper::toDomainEntity)
+ .collect(Collectors.toList());추가로, 도메인 매핑 시 연관 로딩으로 인한 N+1 위험이 있습니다. JPA 쿼리에서 필요한 연관만 fetch join하거나 Projection을 사용해 주세요(아래 검증 스크립트 참고). 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| @Override | ||||||||||||||||||||||||||||||||||||||
| public List<Book> findJoiningRoomsBooksByUserId(Long userId) { | ||||||||||||||||||||||||||||||||||||||
| UserJpaEntity user = userJpaRepository.findById(userId) | ||||||||||||||||||||||||||||||||||||||
| .orElseThrow(() -> new EntityNotFoundException(USER_NOT_FOUND)); | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| return bookJpaRepository.findJoiningRoomsBooksByUserId(user.getUserId()) | ||||||||||||||||||||||||||||||||||||||
| .stream() | ||||||||||||||||||||||||||||||||||||||
| .map(bookMapper::toDomainEntity) | ||||||||||||||||||||||||||||||||||||||
| .collect(Collectors.toList()); | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+44
to
+52
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Verification agent 🧩 Analysis chainJoin 방 책 조회도 동일 최적화 및 쿼리 조건 보장 검증 필요
다음 스크립트로 해당 메서드 쿼리 정의를 찾아 상태 필터와 DISTINCT(ISBN 기준) 적용 여부를 확인해 주세요: 🏁 Script executed: #!/bin/bash
# repository 메서드 정의 탐색
rg -n -A5 -B3 $'findSavedBooksByUserId\\(|findJoiningRoomsBooksByUserId\\('
# JPQL/Query 어노테이션 유무와 where 조건, distinct/Group by(ISBN) 사용 확인
# Projection 또는 fetch join 여부 확인Length of output: 7053 Join 방 책 조회 – User 검증 최적화 제안 및 쿼리 검증 완료 BookQueryPersistenceAdapter.java (44–52):
제안되는 코드 수정 예시: public List<Book> findJoiningRoomsBooksByUserId(Long userId) {
- UserJpaEntity user = userJpaRepository.findById(userId)
- .orElseThrow(() -> new EntityNotFoundException(USER_NOT_FOUND));
-
- return bookJpaRepository.findJoiningRoomsBooksByUserId(user.getUserId())
+ if (!userJpaRepository.existsById(userId)) {
+ throw new EntityNotFoundException(USER_NOT_FOUND);
+ }
+ return bookJpaRepository.findJoiningRoomsBooksByUserId(userId)
.stream()
.map(bookMapper::toDomainEntity)
.collect(Collectors.toList());
}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,11 +2,26 @@ | |
|
|
||
| import konkuk.thip.book.adapter.out.jpa.BookJpaEntity; | ||
| import org.springframework.data.jpa.repository.JpaRepository; | ||
| import org.springframework.data.jpa.repository.Query; | ||
|
|
||
| import java.util.List; | ||
| import java.util.Optional; | ||
|
|
||
| public interface BookJpaRepository extends JpaRepository<BookJpaEntity, Long> { | ||
| Optional<BookJpaEntity> findByIsbn(String isbn); | ||
| List<BookJpaEntity> findByIsbnIn(List<String> isbnList); | ||
|
|
||
| @Query("SELECT DISTINCT b FROM BookJpaEntity b " + | ||
| "JOIN SavedBookJpaEntity s ON s.bookJpaEntity.bookId = b.bookId " + | ||
| "WHERE s.userJpaEntity.userId = :userId " + | ||
| "ORDER BY s.createdAt DESC") | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. p3 : ordering 결과를 사용하지 않는 것 같은데, 최신순으로 정렬한 이유가 있으실까요?
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. p2 : 추가로 유저가 저장한 책을 조회하는 메서드를 SavedBookJpaRepository 가 아니라 BookJpaRepository 에서 구현하신 이유가 있을까요??
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
우선 이 부분은 처음에 저도 SavedBookJpaRepository에서 조회하여 List를 반환받은 후 savedBook.getBookJpaEntity()를 호출하여 Book이라는 Domain으로 변환하시는 방식으로 구현했습니다. 물론 이렇게 하면 N+1 문제가 발생하니 SavedBookJpaRepository에서 SavedBookJpaEntity를 조회할때 EntityGraph를 통해 bookJpaEntity를 fetch join 해서 사용했습니다. 이렇게 구현하고 보니 굳이 fetch join을 통해 BookJpaEntity와 SavedBookJpaEntity를 모두 메모리에 올려두는 것이 맞을까? 라는 의문이 들었습니다. 따라서, 조회 로직을 bookJpaRepository로 옮기고 BookJpaEntity를 바로 반환받는 것으로 수정하였습니다!
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
엇 정렬 조건이 사용자가 책을 저장한 최신순이라고 생각해서 구현했는데 어떤 부분때문에 그러시죠?
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
아아 조회하는 대상이 SavedBook이 아니라 Book 이었군요! 제가 착각했네요 하하
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
이 메서드를 반환받는 어댑터에서 set 으로 만들어서 서비스에 던지길래 이러면 정렬한 이유가 없어지지않나? 라는 생각에 리뷰 남겨보았습니다 |
||
| List<BookJpaEntity> findSavedBooksByUserId(Long userId); | ||
|
|
||
| @Query("SELECT DISTINCT b FROM BookJpaEntity b " + | ||
| "JOIN RoomJpaEntity r ON r.bookJpaEntity.bookId = b.bookId " + | ||
| "JOIN RoomParticipantJpaEntity rp ON rp.roomJpaEntity.roomId = r.roomId " + | ||
| "WHERE rp.userJpaEntity.userId = :userId " + | ||
| "AND r.status = 'ACTIVE' " + | ||
| "AND r.startDate <= CURRENT_TIMESTAMP " + // 진행 중인 방만 조회 (모집 중 / 만료된 방 x) | ||
| "ORDER BY r.roomPercentage DESC") // 방의 진행률이 높은 순서로 정렬 | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. p3 : 여기도 마찬가지로 ordering 결과를 사용하지 않는 것 같은데, 최신순으로 정렬한 이유가 있으실까요? 영속성 adapter 에서 이 메서드 호출하고 set으로 만드는 거 같아서 여쭤봅니다
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 이 메서드는 "유저가 참가 중인 방 중, 진행중인 방에서의 책들을 조회" 하는 메서드인 것 같으니, BookJpaRepository 에 위치하는게 맞는 것 같습니다! 굳굳
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
앗 생각해보니 Set으로 변환하는 것을 깜빡했네요! DISTINCT 키워드를 추가하고 List로 변환해야 될 것 같네요,, |
||
| List<BookJpaEntity> findJoiningRoomsBooksByUserId(Long userId); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| package konkuk.thip.book.adapter.out.persistence.repository; | ||
|
|
||
| import konkuk.thip.book.adapter.out.jpa.SavedBookJpaEntity; | ||
| import org.springframework.data.jpa.repository.JpaRepository; | ||
| import org.springframework.data.jpa.repository.Modifying; | ||
| import org.springframework.data.jpa.repository.Query; | ||
|
|
||
| public interface SavedBookJpaRepository extends JpaRepository<SavedBookJpaEntity, Long> { | ||
| @Query("SELECT CASE WHEN COUNT(s) > 0 THEN true ELSE false END FROM SavedBookJpaEntity s " + | ||
| "WHERE s.userJpaEntity.userId = :userId AND s.bookJpaEntity.bookId = :bookId") | ||
| boolean existsByUserIdAndBookId(Long userId, Long bookId); | ||
|
|
||
| @Modifying | ||
| @Query("DELETE FROM SavedBookJpaEntity s WHERE s.userJpaEntity.userId = :userId AND s.bookJpaEntity.bookId = :bookId") | ||
| void deleteByUserIdAndBookId(Long userId, Long bookId); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| package konkuk.thip.book.application.port.in; | ||
|
|
||
| import konkuk.thip.book.application.port.in.dto.BookSelectableResult; | ||
| import konkuk.thip.book.application.port.in.dto.BookSelectableType; | ||
|
|
||
| import java.util.List; | ||
|
|
||
| public interface BookSelectableListUseCase { | ||
| List<BookSelectableResult> getSelectableBookList(BookSelectableType bookSelectableType, Long userId); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| package konkuk.thip.book.application.port.in.dto; | ||
|
|
||
| public record BookSelectableResult( | ||
| Long bookId, | ||
| String bookTitle, | ||
| String authorName, | ||
| String publisher, | ||
| String bookImageUrl, | ||
| String isbn | ||
| ) { | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| package konkuk.thip.book.application.port.in.dto; | ||
|
|
||
| import konkuk.thip.common.exception.InvalidStateException; | ||
| import konkuk.thip.common.exception.code.ErrorCode; | ||
| import lombok.Getter; | ||
|
|
||
| @Getter | ||
| public enum BookSelectableType { | ||
| SAVED("SAVED"), // 저장된 책 | ||
| JOINING("JOINING") // 참여 중인 모임 방의 책 | ||
| ; | ||
|
Comment on lines
+7
to
+11
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. api 의 request param을 검증하기 위한 enum 좋습니다 패키지 위치도 좋네요 |
||
|
|
||
| private final String type; | ||
|
|
||
| BookSelectableType(String type) { | ||
| this.type = type; | ||
| } | ||
|
|
||
| public static BookSelectableType from(String type) { | ||
| for (BookSelectableType bookSelectableType : BookSelectableType.values()) { | ||
| if (bookSelectableType.type.equals(type)) { | ||
| return bookSelectableType; | ||
| } | ||
| } | ||
| throw new InvalidStateException(ErrorCode.API_INVALID_PARAM, new IllegalArgumentException("타입은 SAVED 또는 JOINING이어야 합니다. 현재 타입: " + type)); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,14 @@ | ||
| package konkuk.thip.book.application.port.out; | ||
|
|
||
| import konkuk.thip.book.domain.Book; | ||
|
|
||
| import java.util.List; | ||
|
|
||
| public interface BookQueryPort { | ||
|
|
||
| boolean existsSavedBookByUserIdAndBookId(Long userId, Long bookId); | ||
|
|
||
| List<Book> findSavedBooksByUserId(Long userId); | ||
|
|
||
| List<Book> findJoiningRoomsBooksByUserId(Long userId); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
오 이러면 병렬적으로 빌드가 되나요?? 궁금
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
저희 테스트 코드가 서로 의존성이 없어서 가능한 것 같네요..! 로컬에서 통과되어서 추가해봤습니다!