-
Notifications
You must be signed in to change notification settings - Fork 0
[feat] 방생성 api 개발 #51
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
829d0ce
fddfc67
7d1dfa4
1856c55
4aca1e7
41fdad1
8b779b0
7acbb2c
b8b9ce7
8235fa1
538b12c
54139ee
e106de5
aad2e93
ca99132
3349ae6
77852a8
c248b15
fb3b5c2
bbacd4e
11dffd2
b56f48c
acd5976
0ea0179
9539c14
e13e5e6
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 |
|---|---|---|
| @@ -0,0 +1,53 @@ | ||
| package konkuk.thip.book.adapter.out.api; | ||
|
|
||
| import konkuk.thip.book.adapter.out.api.aladin.AladinApiClient; | ||
| import konkuk.thip.book.adapter.out.api.dto.NaverBookParseResult; | ||
| import konkuk.thip.book.adapter.out.api.dto.NaverDetailBookParseResult; | ||
| import konkuk.thip.book.application.port.out.BookApiQueryPort; | ||
| import konkuk.thip.book.domain.Book; | ||
| import lombok.RequiredArgsConstructor; | ||
| import org.springframework.stereotype.Component; | ||
|
|
||
| @Component | ||
| @RequiredArgsConstructor | ||
| public class CompositeBookApiAdapter implements BookApiQueryPort { | ||
|
|
||
| private final NaverApiClient naverApiClient; | ||
| private final AladinApiClient aladinApiClient; | ||
|
|
||
| @Override | ||
| public NaverBookParseResult findBooksByKeyword(String keyword, int start) { | ||
| return naverApiClient.findBooksByKeyword(keyword, start); | ||
| } | ||
|
|
||
| @Override | ||
| public NaverDetailBookParseResult findDetailBookByIsbn(String isbn) { | ||
| return naverApiClient.findDetailBookByIsbn(isbn); | ||
| } | ||
|
|
||
| @Override | ||
| public Integer findPageCountByIsbn(String isbn) { | ||
| return aladinApiClient.findPageCountByIsbn(isbn); | ||
| } | ||
|
|
||
| @Override | ||
| public Book loadBookWithPageByIsbn(String isbn) { | ||
| // 1. naver 상세정보 조회 api 로 책 상세정보(without page) load | ||
| NaverDetailBookParseResult detailBookByKeyword = findDetailBookByIsbn(isbn); | ||
|
|
||
| // 2. 알라딘으로부터 책 page 정보 load | ||
| Integer pageCount = findPageCountByIsbn(isbn); | ||
|
|
||
| // 3. pageCount 정보를 포함한 Book 반환 | ||
| return Book.withoutId( | ||
| detailBookByKeyword.title(), | ||
| isbn, | ||
| detailBookByKeyword.author(), | ||
| false, // TODO : 추후 BestSeller 도입되면 고려해야함 | ||
|
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. LGTM |
||
| detailBookByKeyword.publisher(), | ||
| detailBookByKeyword.imageUrl(), | ||
| pageCount, | ||
| detailBookByKeyword.description() | ||
| ); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| package konkuk.thip.book.adapter.out.api.aladin; | ||
|
|
||
| import lombok.RequiredArgsConstructor; | ||
| import org.springframework.stereotype.Component; | ||
|
|
||
| @Component | ||
| @RequiredArgsConstructor | ||
| public class AladinApiClient { | ||
|
|
||
| private final AladinApiUtil aladinApiUtil; | ||
|
|
||
| public Integer findPageCountByIsbn(String isbn) { | ||
| return aladinApiUtil.getPageCount(isbn); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| package konkuk.thip.book.adapter.out.api.aladin; | ||
|
|
||
| public enum AladinApiParam { | ||
|
|
||
| ITEM_ID_TYPE("ISBN"), | ||
| OUTPUT("js"), | ||
| API_VERSION("20131101"), | ||
| SUB_INFO_PARSING_KEY("subInfo"), | ||
| PAGE_COUNT_PARSING_KEY("itemPage"); | ||
|
|
||
|
Comment on lines
+3
to
+10
Contributor
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. 👍🏻 |
||
| private final String value; | ||
|
|
||
| AladinApiParam(String value) { | ||
| this.value = value; | ||
| } | ||
|
|
||
| public String getValue() { | ||
| return value; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,67 @@ | ||
| package konkuk.thip.book.adapter.out.api.aladin; | ||
|
|
||
| import com.fasterxml.jackson.databind.JsonNode; | ||
| import com.fasterxml.jackson.databind.ObjectMapper; | ||
| import konkuk.thip.common.exception.BusinessException; | ||
| import lombok.RequiredArgsConstructor; | ||
| import lombok.extern.slf4j.Slf4j; | ||
| import org.springframework.beans.factory.annotation.Value; | ||
| import org.springframework.stereotype.Component; | ||
| import org.springframework.web.client.RestTemplate; | ||
|
|
||
| import java.io.IOException; | ||
|
|
||
| import static konkuk.thip.book.adapter.out.api.aladin.AladinApiParam.*; | ||
| import static konkuk.thip.common.exception.code.ErrorCode.BOOK_ALADIN_API_ISBN_NOT_FOUND; | ||
| import static konkuk.thip.common.exception.code.ErrorCode.BOOK_ALADIN_API_PARSING_ERROR; | ||
|
|
||
| @Component | ||
| @RequiredArgsConstructor | ||
| @Slf4j | ||
| public class AladinApiUtil { | ||
|
|
||
| private final RestTemplate restTemplate; | ||
| private final ObjectMapper objectMapper; | ||
|
|
||
| @Value("${aladin.ttbKey}") | ||
| private String ttbKey; | ||
|
|
||
| @Value("${aladin.baseUrl}") | ||
| private String baseUrl; | ||
|
Comment on lines
+29
to
+30
Contributor
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. 👍🏻 |
||
|
|
||
|
|
||
| private String buildLookupUrl(String isbn) { | ||
| return String.format( | ||
| baseUrl + "ttbkey=%s&itemIdType=%s&itemId=%s&output=%s&Version=%s", | ||
| ttbKey, | ||
| ITEM_ID_TYPE.getValue(), | ||
| isbn, | ||
| OUTPUT.getValue(), | ||
| API_VERSION.getValue() | ||
| ); | ||
| } | ||
|
|
||
| public Integer getPageCount(String isbn) { | ||
| String url = buildLookupUrl(isbn); | ||
| String response = restTemplate.getForObject(url, String.class); | ||
|
Comment on lines
+44
to
+46
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 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 |
||
|
|
||
| try { | ||
| JsonNode jsonNode = objectMapper.readTree(response); | ||
| JsonNode items = jsonNode.path("item"); | ||
|
|
||
| // json 응답 결과에 item 키값이 없는 경우 | ||
| if (!items.isArray() || items.isEmpty()) { | ||
| // TODO : 알라딘으로부터 page 정보가 없으면 ?? | ||
|
seongjunnoh marked this conversation as resolved.
|
||
| // 보상 시나리오 : 유저에게 "page 정보를 찾을 수 없는 책입니다. 직접 page 정보를 입력하세요" 라고 안내 | ||
| // 일단 지금은 exception throw 만 진행 | ||
| throw new BusinessException(BOOK_ALADIN_API_ISBN_NOT_FOUND); | ||
| } | ||
|
|
||
| JsonNode subInfo = items.get(0).path(SUB_INFO_PARSING_KEY.getValue()); | ||
|
|
||
| return subInfo.path(PAGE_COUNT_PARSING_KEY.getValue()).asInt(); | ||
| } catch (IOException e) { | ||
| throw new BusinessException(BOOK_ALADIN_API_PARSING_ERROR); | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -39,4 +39,14 @@ public Book findById(Long id) { | |||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| return bookMapper.toDomainEntity(bookJpaEntity); | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| @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); | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+43
to
+51
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 트랜잭션 처리 및 메서드명 개선 필요
다음과 같이 개선하는 것을 권장합니다: +@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
Suggested change
🤖 Prompt for AI Agents
Contributor
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. 코래 말대로 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. @buzz0331 맞습니다!
특히 페이지 수 업데이트와 같은 중요한 책 정보 변경 작업은 원자성이 보장되어야 하므로 If you found this review helpful, would you consider giving us a shout-out on X? Thank you for using CodeRabbit!
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. 서비스에서 @ Transactional 처리되고있지 않나요?? 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.
Contributor
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. 아 머야 adapter구나 예리하시다
Collaborator
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. 넵 하나의 api 에 해당하는 로직의 원자성을 지키는 것이 좋지않나 싶어서 api 를 처리하는 Service 클래스의 public 메서드에 @transactional 어노테이션을 추가해두었습니다! |
||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,8 +2,15 @@ | |
|
|
||
| import konkuk.thip.book.adapter.out.api.dto.NaverBookParseResult; | ||
| import konkuk.thip.book.adapter.out.api.dto.NaverDetailBookParseResult; | ||
| import konkuk.thip.book.domain.Book; | ||
|
|
||
| public interface BookApiQueryPort { | ||
|
|
||
| NaverBookParseResult findBooksByKeyword(String keyword, int start); | ||
|
|
||
| NaverDetailBookParseResult findDetailBookByIsbn(String isbn); | ||
|
|
||
| Integer findPageCountByIsbn(String isbn); | ||
|
|
||
| Book loadBookWithPageByIsbn(String isbn); | ||
|
Comment on lines
+13
to
+15
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 chainCQRS 포트 분리 컨벤션 검토 필요
다음 스크립트로 기존 포트 인터페이스의 패턴을 확인해보겠습니다: 🏁 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 분리 컨벤션 위반 확인 필요
현재 구현은 이 컨벤션을 따르고 있지 않으므로 아래 중 하나로 수정이 필요합니다:
수정 대상:
🤖 Prompt for AI Agents
Contributor
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. 융통성 챙겨~ 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. @buzz0331 알겠습니다! 프로젝트의 맥락에 따라 유연하게 적용하는 것이 좋겠네요. 현재 🧠 Learnings used
Collaborator
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. 이 부분은 Service가 Port로부터 알라딘, 네이버 api 의 통신결과에 해당하는 dto가 아니라, Book 도메인 엔티티를 반환받아 그 다음 로직을 수행하는게 어떤가 싶어서 Book 도메인 엔티티를 반환하는 메서드를 추가해두었습니다. 기존 Port 의 네이밍이 BookAPIQueryPort 이고, 해당 port 를 내부에 희진님이 네이버 api 와의 통신 결과에 해당하는 dto 를 반환하는 메서드들을 정의해주셨지만,
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.
Contributor
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
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.
이렇다면 괜찮은 것 같습니다 (다만 query를 위한 dto를 반환하는 메서드와, command 를 위한 도메인 엔티티를 반환하는 메서드가 구분되는게 좀 더 좋지 않나? 라는 생각이 있긴 합니다!) |
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -56,7 +56,8 @@ public BookIsSavedResult changeSavedBook(String isbn, boolean isSave, Long userI | |
| naverResult.description()); | ||
|
|
||
| Long newBookId = bookCommandPort.save(newBook); | ||
| book = newBook.withId(newBookId); | ||
|
|
||
| book = bookCommandPort.findById(newBookId); | ||
|
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. ㅎㅎ |
||
| } | ||
|
|
||
| // 유저가 저장한 책 목록 조회 | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,14 @@ | ||||||||||||||||||||||||||||||||
| package konkuk.thip.config; | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| import org.springframework.context.annotation.Bean; | ||||||||||||||||||||||||||||||||
| import org.springframework.context.annotation.Configuration; | ||||||||||||||||||||||||||||||||
| import org.springframework.web.client.RestTemplate; | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| @Configuration | ||||||||||||||||||||||||||||||||
| public class RestTemplateConfig { | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| @Bean | ||||||||||||||||||||||||||||||||
| public RestTemplate restTemplate() { | ||||||||||||||||||||||||||||||||
| return new RestTemplate(); | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
|
Comment on lines
+10
to
+13
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 프로덕션 환경을 위한 RestTemplate 설정 개선 필요 현재 기본 RestTemplate 설정만 있어 프로덕션 환경에서 성능 및 안정성 문제가 발생할 수 있습니다. 다음 설정들을 고려해보세요:
@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
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,10 +1,26 @@ | ||
| package konkuk.thip.room.adapter.in.web; | ||
|
|
||
| import jakarta.validation.Valid; | ||
| import konkuk.thip.common.dto.BaseResponse; | ||
| import konkuk.thip.common.security.annotation.UserId; | ||
| import konkuk.thip.room.adapter.in.web.request.RoomCreateRequest; | ||
| import konkuk.thip.room.adapter.in.web.response.RoomCreateResponse; | ||
| import konkuk.thip.room.application.port.in.RoomCreateUseCase; | ||
| import lombok.RequiredArgsConstructor; | ||
| import org.springframework.web.bind.annotation.PostMapping; | ||
| import org.springframework.web.bind.annotation.RequestBody; | ||
| import org.springframework.web.bind.annotation.RestController; | ||
|
|
||
| @RestController | ||
| @RequiredArgsConstructor | ||
| public class RoomCommandController { | ||
|
|
||
| private final RoomCreateUseCase roomCreateUseCase; | ||
|
|
||
| @PostMapping("/rooms") | ||
| public BaseResponse<RoomCreateResponse> createRoom(@Valid @RequestBody RoomCreateRequest request, @UserId Long userId) { | ||
| return BaseResponse.ok(RoomCreateResponse.of( | ||
| roomCreateUseCase.createRoom(request.toCommand(), userId) | ||
| )); | ||
| } | ||
| } |
This file was deleted.
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.
👍🏻