[Feature] - 여행기 작성 API 및 여행기 전체 조회 API 구현 #68

hangillee · 2024-07-19T08:13:24Z

✅ 작업 내용

여행기 작성 API Controller 및 Service 구현
메인 페이지를 위한 여행기 전체 조회 Pagination API 구현

📸 스크린샷

🙈 참고 사항

Facade 패턴의 필요성이 조금 모호해지고 있습니다. 코드 리뷰 과정에서 의견을 듣고 싶습니다.
테스트는 추후 작업 예정!

github-actions · 2024-07-19T08:14:24Z

Unit Test Results

0 tests 0 ✔️ 0s ⏱️
0 suites 0 💤
0 files 0 ❌

Results for commit 4853904.

♻️ This comment has been updated with latest results.

slimsha2dy

고생하셨습니다 리건.
몇개 간단한 것들 달아봤는데 한 번 읽어보시고 괜찮으면 반영해주세요!

slimsha2dy · 2024-07-19T08:17:09Z

backend/src/main/java/woowacourse/touroot/travelogue/controller/TravelogueController.java

+        return ResponseEntity.created(URI.create("/api/v1/travelogues/" + response.id()))
+                .body(response);


여행 계획 작성 api에서는 ResponseEntity.ok()를 반환하는데, 어느 쪽을 바꾸든 맞추는게 좋을 것 같습니다.

201 Created 사용하기로 결정했습니다!

slimsha2dy · 2024-07-19T08:45:56Z

backend/src/main/java/woowacourse/touroot/travelogue/dto/TraveloguePlaceRequest.java

+        @Schema(description = "여행기 장소 이름", example = "선릉 캠퍼스")
+        @NotNull(message = "여행기 장소 이름은 비어있을 수 없습니다.")
+        String name,
+        @Schema(description = "여행기 장소 위치 정보")
+        @NotNull(message = "여행기 장소 위치 정보는 비어있을 수 없습니다.")
+        TravelogueLocationRequest location,
+        @Schema(description = "여행기 장소 설명", example = "성담 빌딩에 위치한 선릉 캠퍼스입니다.")
+        String description,
+        @Schema(description = "여행기 장소 사진")
+        @NotNull(message = "여행기 장소 사진은 비어있을 수 없습니다.")
+        List<TraveloguePhotoRequest> photos


어노테이션이 많아져서 한 눈에 안 들어오는데 변수마다 개행을 넣어주는건 어떻게 생각하시나요?

Suggested change

@Schema(description = "여행기 장소 이름", example = "선릉 캠퍼스")

@NotNull(message = "여행기 장소 이름은 비어있을 수 없습니다.")

String name,

@Schema(description = "여행기 장소 위치 정보")

@NotNull(message = "여행기 장소 위치 정보는 비어있을 수 없습니다.")

TravelogueLocationRequest location,

@Schema(description = "여행기 장소 설명", example = "성담 빌딩에 위치한 선릉 캠퍼스입니다.")

String description,

@Schema(description = "여행기 장소 사진")

@NotNull(message = "여행기 장소 사진은 비어있을 수 없습니다.")

List<TraveloguePhotoRequest> photos

@Schema(description = "여행기 장소 이름", example = "선릉 캠퍼스")

@NotNull(message = "여행기 장소 이름은 비어있을 수 없습니다.")

String name,

@Schema(description = "여행기 장소 위치 정보")

@NotNull(message = "여행기 장소 위치 정보는 비어있을 수 없습니다.")

TravelogueLocationRequest location,

@Schema(description = "여행기 장소 설명", example = "성담 빌딩에 위치한 선릉 캠퍼스입니다.")

String description,

@Schema(description = "여행기 장소 사진")

@NotNull(message = "여행기 장소 사진은 비어있을 수 없습니다.")

List<TraveloguePhotoRequest> photos

record 파라미터를 개행해도 무방할까요? 보통 파라미터는 개행하질 않아서 이렇게 작성했습니다!

깃헙에서는 가독성이 떨어지긴 한데, 인텔리제이에서는 띄어진채로 보여서 그대로 둬도 괜찮지 않을까 생각합니닷

slimsha2dy · 2024-07-19T08:46:45Z

backend/src/main/java/woowacourse/touroot/travelogue/dto/TravelogueRequest.java

+        @Schema(description = "여행기 제목", example = "서울 강남 여행기")
+        @NotNull(message = "여행기 제목은 비어있을 수 없습니다.")
+        String title,
+        @Schema(description = "여행기 섬네일", example = "https://thumbnail.png")
+        @NotNull(message = "여행기 섬네일은 비어있을 수 없습니다.")
+        String thumbnail,
+        @Schema(description = "여행기 일자 목록")
+        @NotNull(message = "여행기 일자 목록은 비어있을 수 없습니다.")
+        List<TravelogueDayRequest> days


이 부분도 위와 같습니다!

eunjungL

API 만드느라 수고하셨습니다 리건!!
FacadeService에서 고쳐졌으면 하는 부분이 조금 눈에 보여서 RC 한 번 드립니다 🙇

eunjungL · 2024-07-20T16:09:08Z

backend/src/main/java/woowacourse/touroot/travelogue/controller/TravelogueController.java

+    @PageableAsQueryParam
+    @GetMapping
+    public ResponseEntity<Page<TravelogueResponse>> findMainPageTravelogues(
+            @Parameter(hidden = true)


이거 hidden이면 swagger에서 표시가 안되는걸로 아는데 숨겨두신 이유가 있을까요?
파라미터로 받는만큼 클라이언트 쪽에서 테스트 가능성도 열어두는게 좋을 것 같아요! 디폴트를 설정해주셔서 숨기지 않아도 테스트에 불편함은 없을 것 같습니다

해당 파라미터를 hidden으로 해두지 않으면 JSON 오브젝트 형태의 body로 받게 됩니다. 그래서 @PageableAsQueryParam을 달아둔건데, 이렇게 하면 쿼리 파라미터로 페이지네이션 옵션을 각각 입력 받을 수 있습니다. 즉, 충분히 테스트 가능합니다!

방금 확인했는데 깔끔하네요!! 지금 방식 그대로가 좋은 것 같습니닷

eunjungL · 2024-07-20T16:11:26Z

...rc/main/java/woowacourse/touroot/travelogue/domain/place/dto/TravelogueLocationResponse.java

+@Builder
+public record TravelogueLocationResponse(
+        @Schema(description = "여행기 장소 위도", example = "37.5175896")
+        @NotBlank(message = "여행기 장소 위도는 비어있을 수 없습니다.")


질문) response에도 NoBlank와 같은 검증을 넣는 이유가 있나요? 휴먼에러 방지용?.?

의도는 그랬는데, 사실 저 부분이 validation에 걸리는 상황은 DB에 잘못된 데이터가 저장되어 있는 상황 밖에 없다고 생각합니다. 굳이 필요 없어보이니 제거하겠습니다!

eunjungL · 2024-07-20T16:14:10Z

backend/src/main/java/woowacourse/touroot/travelogue/dto/TravelogueDayRequest.java

+import woowacourse.touroot.travelogue.domain.Travelogue;
+import woowacourse.touroot.travelogue.domain.day.domain.TravelogueDay;
+
+public record TravelogueDayRequest(


Request가 TravelPlan쪽이랑 완전 동일한데 (Day, Place..) Request DTO 정도는 공유해보는 것도 생각해보면 좋을 것 같아요!

eunjungL · 2024-07-20T16:15:39Z

backend/src/main/java/woowacourse/touroot/travelogue/dto/TravelogueDayRequest.java

+public record TravelogueDayRequest(
+        @Schema(description = "여행기 장소 목록")
+        @NotNull(message = "여행기 장소 목록은 비어있을 수 없습니다.")
+        List<TraveloguePlaceRequest> places


Suggested change

List<TraveloguePlaceRequest> places

@Valid

List<TraveloguePlaceRequest> places

DTO 내부에서 또 다시 DTO로 들어가는 경우 Bean Validation이 정상 작동하려면 여기서도 Valid 명시가 필요한 걸로 알고있습니다! 잘 검증되고 있는지 테스트 한 번 해주시고 만약 안되고 있다면 고쳐주세용 👍

eunjungL · 2024-07-20T16:17:12Z

backend/src/main/java/woowacourse/touroot/travelogue/service/TravelogueFacadeService.java

+        Travelogue travelogue = travelogueService.createTravelogue(request);
+        List<TravelogueDayRequest> dayRequests = request.days();
+
+        return TravelogueResponse.builder()


이건 Response 객체 내부에 정팩메 or 생성자로 들어가있어도 괜찮을 것 같습니다!
파사드 서비스가 객체 생성 로직을 알고 있을 필요는 없을 것 같아요

eunjungL · 2024-07-20T16:22:14Z

backend/src/main/java/woowacourse/touroot/travelogue/service/TravelogueFacadeService.java

+        return getTravelogueResponse(travelogue);
+    }
+
+    private TravelogueResponse getTravelogueResponse(final Travelogue travelogue) {


Response가 스스로 생성할 수 있게 정팩메가 생성자를 갖는다면 이런게 FacadeService에서 제거되고 더 가독성 좋은 코드가 될 것 같네요!

eunjungL · 2024-07-20T16:23:44Z

backend/src/main/java/woowacourse/touroot/travelogue/service/TravelogueService.java

    @Transactional(readOnly = true)
    public Travelogue getTravelogueById(Long id) {
        return travelogueRepository.findById(id)
                .orElseThrow(() -> new BadRequestException("존재하지 않는 여행기입니다."));
    }
+
+    public Page<Travelogue> findAll(final Pageable pageable) {


질문) 요기 인자에 혼자 final 붙은 이유가 있나요?

자동 final parameter 옵션을 활성화해놨었는데 놓친 부분이네요. 제거하겠습니다!

nak-honest

리건씌 고생하셨습니다!!
다른분들이 리뷰 잘 남겨주셔서 approve 드릴게욥

nak-honest · 2024-07-21T11:21:30Z

backend/src/main/java/woowacourse/touroot/travelogue/controller/TravelogueController.java

+    @PageableAsQueryParam
+    @GetMapping
+    public ResponseEntity<Page<TravelogueResponse>> findMainPageTravelogues(
+            @Parameter(hidden = true)


방금 확인했는데 깔끔하네요!! 지금 방식 그대로가 좋은 것 같습니닷

nak-honest · 2024-07-21T11:37:28Z

...nd/src/main/java/woowacourse/touroot/travelogue/domain/day/service/TravelogueDayService.java

+            List<TravelogueDayRequest> requests,
+            Travelogue travelogue
+    ) {
+        Map<TravelogueDay, List<TraveloguePlaceRequest>> daysWithPlaceRequests = new LinkedHashMap<>();


현재 TravelogueDay에는 equals와 hashCode가 오버라이딩 되어 있지 않은데,
id 로 비교하도록 오버라이딩 하는 것은 어떠신가요??

JPA를 활용하는 우리 입장에선 이미 영속성 컨텍스트의 1차 캐시에서 관리하는 엔티티 인스턴스는 동일성을 확인할 수 있습니다.
id 필드를 활용해 equals()와 hashCode()를 재정의해야할지 의문이 듭니다. 낙낙은 어떻게 생각하시나용?

사실 제 의견이 조금 과한 면이 있다고 생각이 되는데요..!

createDays() 메소드 내부에서는 리건 말씀대로 동일한 엔티티만을 참조하겠지만,
createDays() 메소드를 호출하는 외부에서는 TravelogueDay 엔티티들이 준영속 상태가 되기 때문에 자칫하면 문제가 발생할 수 있을 것 같아요!

당장은 createDays() 메소드 외부에서 새로 TravelogueDay 엔티티를 가져올 일이 없지만,
경우에 따라 퍼사드 서비스 등에서 새로 엔티티를 가져오는 경우 Map에서 제대로 조회하지 못하게 되니까요..!

물론 그런 상황이 찾아올까 싶기는 해서 그냥 넘어가셔도 좋습니다! id 만으로 equals()와 hashCode()를 재정의할 때 비영속 엔티티와 프록시 객체를 처리하기 까다롭기도 하니까요 ㅎㅎ

좋은 말씀 감사합니다! 내일 다른 팀원 분들과도 공유해서 함께 고민해봐야할 내용인 것 같네요!

nak-honest · 2024-07-21T11:47:58Z

backend/src/main/java/woowacourse/touroot/travelogue/dto/TraveloguePlaceRequest.java

+        @Schema(description = "여행기 장소 이름", example = "선릉 캠퍼스")
+        @NotNull(message = "여행기 장소 이름은 비어있을 수 없습니다.")
+        String name,
+        @Schema(description = "여행기 장소 위치 정보")
+        @NotNull(message = "여행기 장소 위치 정보는 비어있을 수 없습니다.")
+        TravelogueLocationRequest location,
+        @Schema(description = "여행기 장소 설명", example = "성담 빌딩에 위치한 선릉 캠퍼스입니다.")
+        String description,
+        @Schema(description = "여행기 장소 사진")
+        @NotNull(message = "여행기 장소 사진은 비어있을 수 없습니다.")
+        List<TraveloguePhotoRequest> photos


깃헙에서는 가독성이 떨어지긴 한데, 인텔리제이에서는 띄어진채로 보여서 그대로 둬도 괜찮지 않을까 생각합니닷

Libienz

안녕하세요 리건 🙌
order 관련해서 반드시 얘기되었으면 하는 부분있어서 RC드립니다.

참고로 파사드는 현재 운용 이유가 충분하다고 저는 생각합니다.
장소를 생성하는 것 등등을 travelouguePlaceService에 안전하게 (같은 계층에서의 순환 없이) 위임할 수 있으니까요.

다만 순환 참조의 우려가 없다고 확신할 수 있으면 서비스 계층간의 의존으로 해결하고 파사드를 지울 수 있겠네요.

도메인 계층관련해서 단순화하는 방식으로 돌아가겠다고 말씀하신 것도 확인했습니다.

반영되면 다시한번 리뷰 요청 주세요!

Libienz · 2024-07-21T14:20:33Z

...rc/main/java/woowacourse/touroot/travelogue/domain/place/service/TraveloguePlaceService.java

+        for (int i = 0; i < requests.size(); i++) {
+            TraveloguePlaceRequest request = requests.get(i);
+            Place place = getPlace(request);
+
+            TraveloguePlace traveloguePlace = request.toTraveloguePlace(i, place, day);
+            places.put(traveloguePlaceRepository.save(traveloguePlace), request.photos());
+        }
+


현재는 json으로 넘어와 직렬화된 리스트의 순서를 방문하는 순서라고 확신하고 있어요.
장소를 생성하는 request에 장소 순서도 받도록 하는 편이 자연스럽지 않을까요?

명세를 바꾸는 편이 가독성에서도, 안전성에서도 좋을 것이라고 생각합니다.

프론트엔드단에서 order를 받지 않기로 합의했던 것 같은데 아니었나요? 제가 놓친 부분이 있는지 궁금합니다!

요 부분에 대해서 대신 답변 드리겠습니당!

여행계획 생성 관련해서 request 스키마를 논의할 때 나왔던 이야긴데요, 어차피 Json array로 전달되어 순서가 보장되기 때문에 굳이 order를 포함해주지 않아도 된다는 의견이 있었습니다.
논의할 때는 저도 리비처럼 만약을 위해 request에 포함되는 것이 좋을 것이라 생각해 order를 받았었는데 이렇게 되면 order가 제대로 들어왔는지 검증이 필요했습니다. 이 검증을 위해서는 모든 request를 돌면서 order가 올바른 순서인지 검증 후 또 다시 모든 request에 대해 반복문을 돌면서 생성을 진행해야 했고 이는 굉장히 비효율적이라고 판단, array의 순서를 그대로 사용하는 방향으로 결론이 나왔습니다.

컨텍스트 공유 차 답변 달아요~!

Libienz · 2024-07-21T14:27:33Z

backend/src/main/java/woowacourse/touroot/travelogue/service/TravelogueFacadeService.java

+                .map(travelogueDay -> TravelogueDayResponse.builder()
+                        .id(travelogueDay.getId())
+                        .places(createPlaces(days.get(travelogueDay), travelogueDay))
+                        .build())
+                .toList();


도메인을 response로 매핑하는 부분이 아래에서도 계속 이어지고 있는데 Response 객체에 from등의 팩토리 메서드 운용하면 가독성 지킬 수 있을 것 같습니다.
클로버의 코멘트와 비슷한 맥락이에요

해당 파사드의 매핑부분 전체적으로 검토되었으면 합니다.

Libienz

패키지 구조 변경 및 응답 객체 생성 캡슐화 작업하신 것 확인했습니다!
order 관련해서는 제가 놓치고 있는 부분이 있었나보네요 🤔

List가 순서를 보장하도록 추가적인 책임을 가지는 부분이 마음에 들지 않아서 저는 바뀌어야 한다는 입장이긴 한데요, 우선 develop 브랜치에 머지하고 추후 리팩토링에서 생각해보아도 좋을 것 같습니다.

우선 approve 남겨드립니다! 고생하셨어요 👍

eunjungL

FacadeService가 아주 홀쭉해졌군요 👍 👍
주말 늦은 시간까지 작업하시느라 고생 많았습니다 리건쓰~

hangillee added 4 commits July 19, 2024 16:17

feat: 여행기 작성 API를 위한 DTO 구현

efb9362

feat: 여행기 사진 엔티티 생성자 매개변수 순서 변경

2bfb749

feat: 여행기 작성 API 구현

2bbbda6

feat: 메인 페이지를 위한 여행기 전체 목록 조회 API 구현

583bf62

hangillee added the BE label Jul 19, 2024

hangillee added this to the sprint 2 milestone Jul 19, 2024

hangillee requested review from eunjungL, Libienz, nak-honest and slimsha2dy July 19, 2024 08:13

hangillee self-assigned this Jul 19, 2024

chore: EOL 추가

4bffdff

slimsha2dy reviewed Jul 19, 2024

View reviewed changes

eunjungL requested changes Jul 20, 2024

View reviewed changes

hangillee added 4 commits July 21, 2024 13:26

Merge branch 'develop/be' into feature/be/#64

409a3b3

refactor: 불필요한 Bean validation 제거

7c34365

refactor: embedded DTO bean validation 추가

0d17f0f

chore: 실수로 제거한 필드 추가

24b2bdd

nak-honest approved these changes Jul 21, 2024

View reviewed changes

hangillee added 2 commits July 21, 2024 21:32

refactor: 패키지 구조 변경

12b278d

chore: 불필요한 final 제거

6d3062e

Libienz requested changes Jul 21, 2024

View reviewed changes

refactor: 여행기 관련 도메인 조회 기능아 정적 팩토리 메소드를 활용하도록 개선

4853904

hangillee requested review from eunjungL, nak-honest and Libienz July 21, 2024 15:50

Libienz approved these changes Jul 21, 2024

View reviewed changes

eunjungL approved these changes Jul 21, 2024

View reviewed changes

hangillee merged commit 4de3070 into develop/be Jul 21, 2024
3 checks passed

hangillee deleted the feature/be/#64 branch July 21, 2024 16:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Feature] - 여행기 작성 API 및 여행기 전체 조회 API 구현 #68

[Feature] - 여행기 작성 API 및 여행기 전체 조회 API 구현 #68

hangillee commented Jul 19, 2024

github-actions bot commented Jul 19, 2024 •

edited

Loading

slimsha2dy left a comment

slimsha2dy Jul 19, 2024

hangillee Jul 19, 2024

slimsha2dy Jul 19, 2024

hangillee Jul 19, 2024

nak-honest Jul 21, 2024

slimsha2dy Jul 19, 2024

eunjungL left a comment

eunjungL Jul 20, 2024

hangillee Jul 21, 2024

nak-honest Jul 21, 2024

eunjungL Jul 20, 2024

hangillee Jul 21, 2024

eunjungL Jul 20, 2024

eunjungL Jul 20, 2024

eunjungL Jul 20, 2024

eunjungL Jul 20, 2024

eunjungL Jul 20, 2024

hangillee Jul 21, 2024

nak-honest left a comment

nak-honest Jul 21, 2024

nak-honest Jul 21, 2024

hangillee Jul 21, 2024

nak-honest Jul 21, 2024 •

edited

Loading

hangillee Jul 21, 2024

nak-honest Jul 21, 2024

Libienz left a comment

Libienz Jul 21, 2024

hangillee Jul 21, 2024

eunjungL Jul 21, 2024

Libienz Jul 21, 2024

Libienz left a comment

eunjungL left a comment

		return ResponseEntity.created(URI.create("/api/v1/travelogues/" + response.id()))
		.body(response);

	List<TraveloguePlaceRequest> places
	@Valid
	List<TraveloguePlaceRequest> places

[Feature] - 여행기 작성 API 및 여행기 전체 조회 API 구현 #68

[Feature] - 여행기 작성 API 및 여행기 전체 조회 API 구현 #68

Conversation

hangillee commented Jul 19, 2024

✅ 작업 내용

📸 스크린샷

🙈 참고 사항

github-actions bot commented Jul 19, 2024 • edited Loading

Unit Test Results

slimsha2dy left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eunjungL left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nak-honest left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nak-honest Jul 21, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Libienz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Libienz left a comment

Choose a reason for hiding this comment

eunjungL left a comment

Choose a reason for hiding this comment

github-actions bot commented Jul 19, 2024 •

edited

Loading

nak-honest Jul 21, 2024 •

edited

Loading