Skip to content
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

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

Merged
merged 12 commits into from
Jul 21, 2024
Merged
Original file line number Diff line number Diff line change
@@ -1,14 +1,24 @@
package woowacourse.touroot.travelogue.controller;

import io.swagger.v3.oas.annotations.Operation;
import io.swagger.v3.oas.annotations.Parameter;
import io.swagger.v3.oas.annotations.tags.Tag;
import jakarta.validation.Valid;
import java.net.URI;
import lombok.RequiredArgsConstructor;
import org.springdoc.core.converters.models.PageableAsQueryParam;
import org.springframework.data.domain.Page;
import org.springframework.data.domain.Pageable;
import org.springframework.data.domain.Sort.Direction;
import org.springframework.data.web.PageableDefault;
import org.springframework.http.ResponseEntity;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.PathVariable;
import org.springframework.web.bind.annotation.PostMapping;
import org.springframework.web.bind.annotation.RequestBody;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RestController;
import woowacourse.touroot.travelogue.dto.TravelogueRequest;
import woowacourse.touroot.travelogue.dto.TravelogueResponse;
import woowacourse.touroot.travelogue.service.TravelogueFacadeService;

Expand All @@ -20,9 +30,29 @@ public class TravelogueController {

private final TravelogueFacadeService travelogueFacadeService;

@Operation(description = "여행기 작성")
@PostMapping
public ResponseEntity<TravelogueResponse> createTravelogue(@Valid @RequestBody TravelogueRequest request) {
TravelogueResponse response = travelogueFacadeService.createTravelogue(request);

return ResponseEntity.created(URI.create("/api/v1/travelogues/" + response.id()))
.body(response);
Comment on lines +38 to +39

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

}

@Operation(description = "여행기 상세 조회")
@GetMapping("/{id}")
public ResponseEntity<TravelogueResponse> findTravelogue(@Valid @PathVariable Long id) {
return ResponseEntity.ok(travelogueFacadeService.findTravelogueById(id));
}

@Operation(description = "여행기 메인 페이지 조회")
@PageableAsQueryParam
@GetMapping
public ResponseEntity<Page<TravelogueResponse>> findMainPageTravelogues(
@Parameter(hidden = true)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@PageableDefault(size = 5, sort = "id", direction = Direction.DESC)
Pageable pageable
) {
return ResponseEntity.ok(travelogueFacadeService.findTravelogues(pageable));
}
}
Original file line number Diff line number Diff line change
@@ -1,20 +1,40 @@
package woowacourse.touroot.travelogue.domain.day.service;

import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import lombok.RequiredArgsConstructor;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;
import woowacourse.touroot.global.exception.BadRequestException;
import woowacourse.touroot.travelogue.domain.Travelogue;
import woowacourse.touroot.travelogue.domain.day.domain.TravelogueDay;
import woowacourse.touroot.travelogue.domain.day.repository.TravelogueDayRepository;
import woowacourse.touroot.travelogue.dto.TravelogueDayRequest;
import woowacourse.touroot.travelogue.dto.TraveloguePlaceRequest;

@RequiredArgsConstructor
@Service
public class TravelogueDayService {

private final TravelogueDayRepository travelogueDayRepository;

@Transactional
public Map<TravelogueDay, List<TraveloguePlaceRequest>> createDays(
List<TravelogueDayRequest> requests,
Travelogue travelogue
) {
Map<TravelogueDay, List<TraveloguePlaceRequest>> daysWithPlaceRequests = new LinkedHashMap<>();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

@nak-honest nak-honest Jul 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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

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

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

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


for (int i = 0; i < requests.size(); i++) {
TravelogueDayRequest request = requests.get(i);
TravelogueDay travelogueDay = request.toTravelogueDay(i, travelogue);
daysWithPlaceRequests.put(travelogueDayRepository.save(travelogueDay), request.places());
}

return daysWithPlaceRequests;
}

@Transactional(readOnly = true)
public List<TravelogueDay> findDaysByTravelogue(Travelogue travelogue) {
return travelogueDayRepository.findByTravelogue(travelogue);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ public class TraveloguePhoto extends BaseEntity {
@ManyToOne(fetch = FetchType.LAZY)
private TraveloguePlace traveloguePlace;

public TraveloguePhoto(String key, Integer order, TraveloguePlace traveloguePlace) {
public TraveloguePhoto(Integer order, String key, TraveloguePlace traveloguePlace) {
this(null, key, order, traveloguePlace);
}
}
Original file line number Diff line number Diff line change
@@ -1,19 +1,36 @@
package woowacourse.touroot.travelogue.domain.photo.service;

import java.util.ArrayList;
import java.util.Comparator;
import java.util.List;
import lombok.RequiredArgsConstructor;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;
import woowacourse.touroot.travelogue.domain.photo.domain.TraveloguePhoto;
import woowacourse.touroot.travelogue.domain.photo.repository.TraveloguePhotoRepository;
import woowacourse.touroot.travelogue.domain.place.domain.TraveloguePlace;
import woowacourse.touroot.travelogue.dto.TraveloguePhotoRequest;

@RequiredArgsConstructor
@Service
public class TraveloguePhotoService {

private final TraveloguePhotoRepository traveloguePhotoRepository;

@Transactional
public List<TraveloguePhoto> createPhotos(List<TraveloguePhotoRequest> requests, TraveloguePlace place) {
List<TraveloguePhoto> photos = new ArrayList<>();

for (int i = 0; i < requests.size(); i++) {
TraveloguePhotoRequest request = requests.get(i);
TraveloguePhoto photo = request.toTraveloguePhoto(i, place);
photos.add(traveloguePhotoRepository.save(photo));
}

return photos;
}

@Transactional(readOnly = true)
public List<String> findPhotoUrlsByPlace(TraveloguePlace traveloguePlace) {
List<TraveloguePhoto> photos = traveloguePhotoRepository.findByTraveloguePlace(traveloguePlace);

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package woowacourse.touroot.travelogue.domain.place.dto;

import io.swagger.v3.oas.annotations.media.Schema;
import jakarta.validation.constraints.NotBlank;
import lombok.Builder;

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

String lat,
@Schema(description = "여행기 장소 설명", example = "127.0867236")
@NotBlank(message = "여행기 장소 경도는 비어있을 수 없습니다.")
String lng
) {
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,7 @@ public record TraveloguePlaceResponse(
@Schema(description = "여행기 장소 설명", example = "성담 빌딩에 위치한 선릉 캠퍼스입니다.")
@NotBlank(message = "여행기 장소 설명은 비어있을 수 없습니다.")
String description,
@Schema(description = "여행기 장소 위도", example = "37.5175896")
@NotBlank(message = "여행기 장소 위도는 비어있을 수 없습니다.")
String lat,
@Schema(description = "여행기 장소 설명", example = "127.0867236")
@NotBlank(message = "여행기 장소 경도는 비어있을 수 없습니다.")
String lng,
@NotNull TravelogueLocationResponse location,
List<String> photoUrls
) {
}
Original file line number Diff line number Diff line change
@@ -1,20 +1,53 @@
package woowacourse.touroot.travelogue.domain.place.service;

import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import lombok.RequiredArgsConstructor;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;
import woowacourse.touroot.global.exception.BadRequestException;
import woowacourse.touroot.place.domain.Place;
import woowacourse.touroot.place.repository.PlaceRepository;
import woowacourse.touroot.travelogue.domain.day.domain.TravelogueDay;
import woowacourse.touroot.travelogue.domain.place.domain.TraveloguePlace;
import woowacourse.touroot.travelogue.domain.place.repsitory.TraveloguePlaceRepository;
import woowacourse.touroot.travelogue.dto.TraveloguePhotoRequest;
import woowacourse.touroot.travelogue.dto.TraveloguePlaceRequest;

@RequiredArgsConstructor
@Service
public class TraveloguePlaceService {

private final PlaceRepository placeRepository;
private final TraveloguePlaceRepository traveloguePlaceRepository;

@Transactional
public Map<TraveloguePlace, List<TraveloguePhotoRequest>> createPlaces(
List<TraveloguePlaceRequest> requests,
TravelogueDay day
) {
Map<TraveloguePlace, List<TraveloguePhotoRequest>> places = new LinkedHashMap<>();

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());
}

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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

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

return places;
}

private Place getPlace(TraveloguePlaceRequest request) {
return placeRepository.findByNameAndLatitudeAndLongitude(
request.name(),
request.location().lat(),
request.location().lng()
).orElseGet(() -> placeRepository.save(request.toPlace()));
}

@Transactional(readOnly = true)
public List<TraveloguePlace> findTraveloguePlaceByDay(TravelogueDay travelogueDay) {
return traveloguePlaceRepository.findByTravelogueDay(travelogueDay);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package woowacourse.touroot.travelogue.dto;

import io.swagger.v3.oas.annotations.media.Schema;
import jakarta.validation.constraints.NotNull;
import java.util.List;
import woowacourse.touroot.travelogue.domain.Travelogue;
import woowacourse.touroot.travelogue.domain.day.domain.TravelogueDay;

public record TravelogueDayRequest(

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
List<TraveloguePlaceRequest> places
@Valid
List<TraveloguePlaceRequest> places

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

) {

public TravelogueDay toTravelogueDay(int order, Travelogue travelogue) {
return new TravelogueDay(order, travelogue);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package woowacourse.touroot.travelogue.dto;

import io.swagger.v3.oas.annotations.media.Schema;
import jakarta.validation.constraints.NotNull;

public record TravelogueLocationRequest(
@Schema(description = "여행기 장소 위도", example = "37.5175896")
@NotNull(message = "여행기 장소 위도는 비어있을 수 없습니다.")
String lat,
@Schema(description = "여행기 장소 경도", example = "127.0867236")
@NotNull(message = "여행기 장소 경도는 비어있을 수 없습니다.")
String lng
) {
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
package woowacourse.touroot.travelogue.dto;

import io.swagger.v3.oas.annotations.media.Schema;
import jakarta.validation.constraints.NotNull;
import woowacourse.touroot.travelogue.domain.photo.domain.TraveloguePhoto;
import woowacourse.touroot.travelogue.domain.place.domain.TraveloguePlace;

public record TraveloguePhotoRequest(
@Schema(description = "여행기 장소 사진 Key", example = "photo.png")
@NotNull(message = "여행기 장소 사진 Key 값은 비어있을 수 없습니다.")
String key
) {

public TraveloguePhoto toTraveloguePhoto(int order, TraveloguePlace traveloguePlace) {
return new TraveloguePhoto(order, key, traveloguePlace);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
package woowacourse.touroot.travelogue.dto;

import io.swagger.v3.oas.annotations.media.Schema;
import jakarta.validation.constraints.NotNull;
import java.util.List;
import woowacourse.touroot.place.domain.Place;
import woowacourse.touroot.travelogue.domain.day.domain.TravelogueDay;
import woowacourse.touroot.travelogue.domain.place.domain.TraveloguePlace;

public record TraveloguePlaceRequest(
@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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

) {

public TraveloguePlace toTraveloguePlace(int order, Place place, TravelogueDay travelogueDay) {
return new TraveloguePlace(order, description, place, travelogueDay);
}

public Place toPlace() {
return new Place(name, location.lat(), location.lng());
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
package woowacourse.touroot.travelogue.dto;

import io.swagger.v3.oas.annotations.media.Schema;
import jakarta.validation.constraints.NotNull;
import java.util.List;
import woowacourse.touroot.travelogue.domain.Travelogue;

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 부분도 위와 같습니다!

) {

public Travelogue toTravelogue() {
return new Travelogue(title, thumbnail);
}
}
Loading
Loading