[Team-25][BE][Jay] 예약 가능한 숙소 조회 및 위시 리스트 추가/삭제 API PR #270
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
비즈니스 예외 클래스들이 HttpMethod 를 반환하도록 하여, 각 예외에 맞는 HTTP 상태코드를 반환할 수 있도록 함
프로젝트 전역에서 발생하는 예외를 처리하는 로직 추가 - 비즈니스 예외 발생시에는 예외 클래스에 있는 HTTP 상태코드 반환 - 나머지 분류되지 않은 예외는 INTERNAL_SERVER_ERROR 로 반환
[BE] 전역 예외처리 구조 구현 PR
- 위시 리스트 추가 여부는, 레코드가 있는지 여부로 판단 - Wish 엔티티는 숙소와 회원의 id 만 참조하도록 변경
[BE] �Entity, DB 설계 완료 PR
- 미완성된 FeesPolicy 커밋된 내역 제거
[BE] 불필요 항목 제거
- mysql : 런타임에만 참조 - h2 : 테스트 런타임에만 참조
- test/resources/application.properties : 테스트 환경 분리를 위해 추가 - QueryDslConfig : JPAQueryFactory 빈 등록
* feat: 숙소 가격 통계 API URL 복수형으로 변경 * feat: 숙소제공항목 row 로 저장 및 명칭 단수로 변경 - Provides -> Provide - AccommodationProvide 추가 - 숙소제공항목을 enum 으로 관리 - enum 을 column 으로 변환하는 ProvideElementsConverter 추가 * feat: 변경 불가능한 컬렉션 대신, 복사하여 반환하도록 변경 * style: static 변수 대문자로 변경 * style: import * 대신, 필요한 것만 import 하도록 변경 * feat: 시작 종료일자 검증로직 추가 * feat: AccommodationRepository, ReservationRepository 추가 * feat: Accommodation 생성자 추가 - @AllArgsConstructor 추가 - @entity 를 위해 기본 생성자를 @NoArgsConstructor 로 변경 * refactor: ProvideElement -> ProvideName 으로 명칭 변경 - ProvideElement -> ProvideName - ProvideElementConverter -> ProvideNameConverter * feat: Schedule 에 정적 생성 메소드 추가 * feat: Reservation 에 Getter 및 AllArgsConstructor 추가 * feat: 예약 가능한 숙소 조회 쿼리 추가 * feat: 예약 일정을 저장하는 변수 타입 변경 (LocalDateTime -> LocalDate) * feat: 예약 가능한 숙소 조회 서비스 추가 * build: MySQL Point 를 처리를 위해 hibernate-spatial 의존성 추가 및 Dialect 설정 * refactor: 전역 예외처리 로직 에러 로깅 추가 * refactor: Accommodation 의 Point 의존성 변경 * refactor: StartEndDate null 방어코드 추가 * refactor: Address toString 추가 * refactor: AccommodationService 에서 조회시 Lazy loading 되도록 수정 * style: 미사용 import 제거 * refactor: 일급컬렉션 객체 제거 * feat: 예약 가능한 숙소 조회 API 추가
- 위시 리스트 저장 기능 추가 - 위시 리스트 삭제 기능 추가
새로 추가할 엔티티를 만들 경우에 사용
# Conflicts: # backend/Dockerfile # backend/build.gradle # backend/docker-compose.yml # backend/src/docs/asciidoc/index.adoc # backend/src/main/java/codesquad/airbnb/accomodation/AccommodationController.java # backend/src/main/java/codesquad/airbnb/accomodation/domain/Accommodation.java # backend/src/main/java/codesquad/airbnb/accomodation/domain/Address.java # backend/src/main/java/codesquad/airbnb/accomodation/dto/PriceAndCountStatistics.java # backend/src/main/java/codesquad/airbnb/accomodation/repository/AccommodationQueryRepository.java # backend/src/main/java/codesquad/airbnb/accomodation/service/AmountUnitPolicy.java # backend/src/main/java/codesquad/airbnb/exception/GlobalExceptionHandler.java # backend/src/main/java/codesquad/airbnb/member/Member.java # backend/src/main/java/codesquad/airbnb/reservation/Reservation.java # backend/src/main/java/codesquad/airbnb/reservation/Schedule.java # backend/src/main/java/codesquad/airbnb/reservation/StartEndDate.java # backend/src/main/java/codesquad/airbnb/wish/Wish.java # backend/src/main/resources/application-prod.properties # backend/src/main/resources/application.properties # backend/src/main/resources/sql/airbnb-schema.sql # backend/src/test/java/codesquad/airbnb/ApiDocumentUtils.java # backend/src/test/java/codesquad/airbnb/accomodation/AccommodationControllerTest.java # backend/src/test/resources/application.properties
* docs: API 문서 오타 수정 * feat: 숙소 가격 통계 API 가 더미 데이터를 반환하도록 변경 * feat: 숙소 가격 통계 API 에 전체 숙소 평균 가격 추가
* feat: 숙소 가격 통계 API URL 복수형으로 변경 * feat: 숙소제공항목 row 로 저장 및 명칭 단수로 변경 - Provides -> Provide - AccommodationProvide 추가 - 숙소제공항목을 enum 으로 관리 - enum 을 column 으로 변환하는 ProvideElementsConverter 추가 * feat: 변경 불가능한 컬렉션 대신, 복사하여 반환하도록 변경 * style: static 변수 대문자로 변경 * style: import * 대신, 필요한 것만 import 하도록 변경 * feat: 시작 종료일자 검증로직 추가 * feat: AccommodationRepository, ReservationRepository 추가 * feat: Accommodation 생성자 추가 - @AllArgsConstructor 추가 - @entity 를 위해 기본 생성자를 @NoArgsConstructor 로 변경 * refactor: ProvideElement -> ProvideName 으로 명칭 변경 - ProvideElement -> ProvideName - ProvideElementConverter -> ProvideNameConverter * feat: Schedule 에 정적 생성 메소드 추가 * feat: Reservation 에 Getter 및 AllArgsConstructor 추가 * feat: 예약 가능한 숙소 조회 쿼리 추가 * feat: 예약 일정을 저장하는 변수 타입 변경 (LocalDateTime -> LocalDate) * feat: 예약 가능한 숙소 조회 서비스 추가 * build: MySQL Point 를 처리를 위해 hibernate-spatial 의존성 추가 및 Dialect 설정 * refactor: 전역 예외처리 로직 에러 로깅 추가 * refactor: Accommodation 의 Point 의존성 변경 * refactor: StartEndDate null 방어코드 추가 * refactor: Address toString 추가 * refactor: AccommodationService 에서 조회시 Lazy loading 되도록 수정 * style: 미사용 import 제거 * refactor: 일급컬렉션 객체 제거 * feat: 예약 가능한 숙소 조회 API 추가
jinan159
changed the title
jinan159
commented
Jun 8, 2022
| public List<Accommodation> findNotReservedAccommodations(Schedule schedule) { | ||
| JPAQuery<Accommodation> accommodationJPAQuery = queryFactory.selectFrom(Q_ACCOMMODATION); | ||
|
|
||
| if (!isVisitorsNull(schedule.getVisitors())) { |
There was a problem hiding this comment.
인원수를 설정하지 않아도 검색이 가능해야 한다는 요구사항이 있어서 이에 대응하기 위해서 다이나믹 쿼리 를 사용했는데요.
이러한 형태는 유지보수가 힘들 수 있다는 생각이 들었습니다.
이런 형태의 다이나믹 쿼리 보다는 코드 중복이 조금 있더라도, 각각의 케이스에 대해 다른 메소드에서 기능을 제공하는게 맞을까요?
There was a problem hiding this comment.
동적 쿼리는 재사용성을 높일 수 있지만, 복잡한 동적 쿼리는 가독성이 많이 떨어집니다.
때문에 동적쿼리는 어드민처럼 모든 컬럼에 대해 조건을 부여하는 경우가 아니면 대부분 사용하지 않습니다(제 경험상)
isVisitorsNull 여부에 따라서 각각 다른 메소드를 호출하는건 어떨까요?
There was a problem hiding this comment.
Suggested change
| if (!isVisitorsNull(schedule.getVisitors())) { | |
| if (schedule.hasVisitors()) { |
jinan159
commented
Jun 8, 2022
|
|
||
| @SpringBootTest | ||
| @Transactional | ||
| @Sql("classpath:sql/accommodation-reservation.sql") |
There was a problem hiding this comment.
Service 테스트를 위해서 데이터 set up 을 해야하는데요.
많은 도메인들이 연관되어있어서, 코드로 데이터를 생성하려면 Repository 를 너무 많이 알고 있어야 합니다.
그래서 다른 Repository 와의 의존성을 줄이기 위해 SQL 파일로 set up 하였습니다.
이 방법에 대한 리뷰어님의 의견이 궁금합니다.
There was a problem hiding this comment.
보통 service 레이어 테스트 코드의 경우 repository는 mock을 사용하고 서비스 로직만을 테스트 합니다.
repository의 알맞은 메소드를 호출했는가를 검증합니다.
transactional을 활용한 테스트는 repository 레이어에 테스트코드를 생성하는 경우 사용합니다 :)
Min-92
approved these changes
Jun 8, 2022
| public class AccommodationController { | ||
|
|
||
| private static final DateTimeFormatter DATE_FORMAT = DateTimeFormatter.ofPattern("yyyy-MM-dd"); |
Comment on lines
+35
to
+42
| @RequestParam(value = "startDate", required = false) String startDate, | ||
| @RequestParam(value = "endDate", required = false) String endDate, | ||
| @RequestParam(value = "visitors", required = false) Integer visitors) { | ||
|
|
||
| AccommodationSearchRequest searchRequest = new AccommodationSearchRequest( | ||
| LocalDate.parse(startDate, DATE_FORMAT), | ||
| LocalDate.parse(endDate, DATE_FORMAT), | ||
| visitors); |
There was a problem hiding this comment.
다수의 Request param을 하나의 dto로 수신하면 코드가 더 간결해질것 같네요 :)
참고
|
|
||
| @GetMapping("/statistics/price") | ||
| public PriceAndCountStatistics getAccommodationPriceStatistics() { | ||
| return dummyAccommodationStatisticsService.getCachedPriceAndCountStatistics(); | ||
| } | ||
|
|
||
| @GetMapping() | ||
| public List<WishedAccommodationResponse> getNotReservedAccommodations( |
There was a problem hiding this comment.
필수는 아니고 참고사항 입니다.
not~~같은 부정문 대신 긍정문을 사용하면 좀 더 간결하고 명확한 경우가 있습니다 :)
Suggested change
| public List<WishedAccommodationResponse> getNotReservedAccommodations( | |
| public List<WishedAccommodationResponse> getAvailableAccommodations( |
Comment on lines
+47
to
+48
| @OneToMany(mappedBy = "accommodation", fetch = FetchType.LAZY) | ||
| private List<AccommodationProvide> accommodationProvides; |
Comment on lines
+50
to
+52
| public static Accommodation createNewAccommodation(String title, long price, Image image, Point point, Address address) { | ||
| return new Accommodation(null, title, price, image, point, address, new ArrayList<>()); | ||
| } |
There was a problem hiding this comment.
테스트 에서만 사용되는 정적 메소드로 보이네요.
테스트만을 위한 메소드가 프로덕선 코드에 있는건 좋지 않습니다.
테스트 클래스의 createNewAccommodation에서 생성자를 직접 호출하는 방식으로 변경해봅시다 :)
Comment on lines
+28
to
+30
| public static Member createNewMember(String email, String name) { | ||
| return new Member(null, email, name); | ||
| } |
There was a problem hiding this comment.
테스트 에서만 사용되는 정적 메소드로 보이네요.
테스트만을 위한 메소드가 프로덕선 코드에 있는건 좋지 않습니다.
Comment on lines
+42
to
+44
| public static Reservation createNewReservation(Accommodation accommodation,Member member, Schedule schedule) { | ||
| return new Reservation(null, accommodation, member, schedule); | ||
| } |
There was a problem hiding this comment.
테스트 에서만 사용되는 정적 메소드로 보이네요.
테스트만을 위한 메소드가 프로덕선 코드에 있는건 좋지 않습니다.
Comment on lines
+18
to
+23
| public StartEndDate(LocalDate startDate, LocalDate endDate) { | ||
| this.startDate = startDate; | ||
| this.endDate = endDate; | ||
|
|
||
| if ((startDate != null && endDate != null) && startDate.isAfter(endDate)) throw new InvalidStartEndDateException(); | ||
| } |
There was a problem hiding this comment.
검증을 생성자 상단에서 하면 더 좋을듯 합니다 :)
Suggested change
| public StartEndDate(LocalDate startDate, LocalDate endDate) { | |
| this.startDate = startDate; | |
| this.endDate = endDate; | |
| if ((startDate != null && endDate != null) && startDate.isAfter(endDate)) throw new InvalidStartEndDateException(); | |
| } | |
| public StartEndDate(LocalDate startDate, LocalDate endDate) { | |
| if ((startDate != null && endDate != null) && startDate.isAfter(endDate)) throw new InvalidStartEndDateException(); | |
| this.startDate = startDate; | |
| this.endDate = endDate; | |
| } |
There was a problem hiding this comment.
검증 메소드를 분리해주면 더 좋을거 같구요
Suggested change
| public StartEndDate(LocalDate startDate, LocalDate endDate) { | |
| this.startDate = startDate; | |
| this.endDate = endDate; | |
| if ((startDate != null && endDate != null) && startDate.isAfter(endDate)) throw new InvalidStartEndDateException(); | |
| } | |
| public StartEndDate(LocalDate startDate, LocalDate endDate) { | |
| validate(startDate, endDate); | |
| this.startDate = startDate; | |
| this.endDate = endDate; | |
| } |
Comment on lines
+25
to
+32
| @Transactional | ||
| public WishResponse addWish(WishAddRequest request) { | ||
| validateAccommodationExists(request.getAccommodationId()); | ||
| validateMemberExists(request.getMemberId()); | ||
|
|
||
| Wish savedWish = wishRepository.save(Wish.createNewWish(request.getAccommodationId(), request.getMemberId())); | ||
| return WishResponse.from(savedWish); | ||
| } |
There was a problem hiding this comment.
이미 wish를 추가한 상태에서 다시 wish 추가 요청이 들어오면 어떻게 될까요?
|
|
||
| @SpringBootTest | ||
| @Transactional | ||
| @Sql("classpath:sql/accommodation-reservation.sql") |
There was a problem hiding this comment.
보통 service 레이어 테스트 코드의 경우 repository는 mock을 사용하고 서비스 로직만을 테스트 합니다.
repository의 알맞은 메소드를 호출했는가를 검증합니다.
transactional을 활용한 테스트는 repository 레이어에 테스트코드를 생성하는 경우 사용합니다 :)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
왕민안녕하세요 Jay 입니다.지난주에 개인 사정으로 개발을 많이 하지 못하여, 주말과 이번주 초에 빠르게 개발하여 PR 요청드립니다.
정리하는데 시간이 걸려 조금 늦어진 점 죄송합니다😢
지난 리뷰 피드백 적용 현황 및 신규 기능 적용 PR 입니다.
신규 기능
숙소 조회 API
숙소 위시 리스트 추가/삭제 API
참고 : API 문서
지난 리뷰 피드백
1. API 명사 복수형 사용
2. 제공 품목을 column -> row 로 관리
+ Provides -> Provide 단수 명사로 변경
Provide.java
ProvideName.java
3. 정적 변수 UPPER_SNAKE_CASE 로 변경
4. asterisk 없이 필요한 정보만 import 하도록 변경
추가로 여쭤보고싶은 부분은
File changes에 댓글로 남겨놓겠습니다!감사합니다😃