-
Notifications
You must be signed in to change notification settings - Fork 3
코스 추가 기능 구현 #652
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
base: main
Are you sure you want to change the base?
코스 추가 기능 구현 #652
Changes from all commits
ae277cc
d420d8e
fc9786e
bafc3cc
5c198bd
c3d4257
4a2f571
9a41d6b
e98e861
3b3c198
4d894f4
c1407b2
c0e04ae
3e24484
2ece92b
edadfd9
b7d0cd1
6e59f0c
62b9e5d
30a5403
30a1d60
4df7ff6
bd52bcd
cd56ef9
86771e3
1f7f1be
51f9582
6f85977
ab1c5cc
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 |
|---|---|---|
|
|
@@ -2,7 +2,10 @@ | |
|
|
||
| import coursepick.coursepick.application.dto.CourseResponse; | ||
| import coursepick.coursepick.application.dto.CoursesResponse; | ||
| import coursepick.coursepick.application.dto.SnapResponse; | ||
| import coursepick.coursepick.domain.course.*; | ||
| import coursepick.coursepick.domain.user.User; | ||
| import coursepick.coursepick.domain.user.UserRepository; | ||
| import lombok.RequiredArgsConstructor; | ||
| import lombok.extern.slf4j.Slf4j; | ||
| import org.jspecify.annotations.Nullable; | ||
|
|
@@ -13,6 +16,7 @@ | |
| import java.util.List; | ||
|
|
||
| import static coursepick.coursepick.application.exception.ErrorType.NOT_EXIST_COURSE; | ||
| import static coursepick.coursepick.application.exception.ErrorType.NOT_EXIST_USER; | ||
|
|
||
| @Slf4j | ||
| @Service | ||
|
|
@@ -21,6 +25,9 @@ public class CourseApplicationService { | |
|
|
||
| private final CourseRepository courseRepository; | ||
| private final RouteFinder routeFinder; | ||
| private final CoordinateSnapper coordinateSnapper; | ||
| private final UserRepository userRepository; | ||
| private final UserCreatedCourseRepository userCreatedCourseRepository; | ||
|
|
||
| @Transactional(readOnly = true) | ||
| public CoursesResponse findNearbyCourses(CourseFindCondition condition, @Nullable Double userLatitude, @Nullable Double userLongitude) { | ||
|
|
@@ -67,4 +74,24 @@ private void loggingForNotExistsCourse(List<String> ids, List<Course> courses) { | |
| } | ||
| } | ||
| } | ||
|
|
||
| @Transactional(readOnly = true) | ||
| public SnapResponse snapCoordinates(List<Coordinate> coordinates) { | ||
| SnapResult snapResult = coordinateSnapper.snap(coordinates); | ||
| return new SnapResponse(snapResult.coordinates(), snapResult.length()); | ||
| } | ||
|
Comment on lines
+78
to
+82
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. 이 로직이 CourseApplicationService에 있는게 좀 부자연스러운 것 같아요.
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. 저희가 우테코에서 프로젝트를 하면서 초기에 했던 얘기가 생각나는 대목이네요. 그래서 해당 철학을 기반으로 했을 때 Coordinate의 접근은 Course에서 이뤄지는게 자연스럽다고 생각했습니다.
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. 이것도 수정하려면 너무 커질 것 같아서, 일단 넘어가도 좋을 것 같습니다. 다만 고민해보면 좋을 점이, 모든 연산이 꼭 애그리거트 루트를 통할 필요가 없다는 지점입니다. DDD에서도 그것을 강제하지 않고, 다만 CUD 작업은 그것을 통하라고 말합니다. 애그리거트는 복잡한 시스템에서 데이터 일관성을 유지하는 경계이기 때문입니다. 이런 사실을 통해 제가 생각하기로는, |
||
|
|
||
| @Transactional | ||
| public CourseResponse create(String userId, String name, List<Coordinate> coordinates) { | ||
| User user = userRepository.findById(userId) | ||
| .orElseThrow(() -> NOT_EXIST_USER.create(userId)); | ||
|
|
||
| Course course = new Course(null, name, coordinates); | ||
| Course savedCourse = courseRepository.save(course); | ||
|
|
||
| UserCreatedCourse userCreatedCourse = new UserCreatedCourse(user.id(), savedCourse.id(), false); | ||
| userCreatedCourseRepository.save(userCreatedCourse); | ||
|
|
||
| return CourseResponse.from(savedCourse); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| package coursepick.coursepick.application.dto; | ||
|
|
||
| import coursepick.coursepick.domain.course.Coordinate; | ||
|
|
||
| import java.util.List; | ||
|
|
||
| public record SnapResponse( | ||
| List<Coordinate> coordinates, | ||
| double length | ||
| ) { | ||
| } |
dompoo marked this conversation as resolved.
Show resolved
Hide resolved
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| package coursepick.coursepick.domain.course; | ||
|
|
||
| import java.util.List; | ||
|
|
||
| public record SnapResult( | ||
| List<Coordinate> coordinates, | ||
| double length | ||
| ) { | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| package coursepick.coursepick.domain.course; | ||
|
|
||
| import lombok.AccessLevel; | ||
| import lombok.AllArgsConstructor; | ||
| import org.springframework.data.annotation.Id; | ||
| import org.springframework.data.annotation.PersistenceCreator; | ||
| import org.springframework.data.mongodb.core.mapping.Document; | ||
|
|
||
| @Document | ||
| @AllArgsConstructor(access = AccessLevel.PROTECTED, onConstructor_ = @PersistenceCreator) | ||
| public class UserCreatedCourse { | ||
|
|
||
| @Id | ||
| private final String id; | ||
| private final String userId; | ||
| private final String courseId; | ||
| private boolean isPublic; | ||
|
|
||
|
Comment on lines
+9
to
+18
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. UserCreatedCourse가 id로 구분될 필요가 있을까요? |
||
| public UserCreatedCourse(String userId, String courseId, boolean isPublic) { | ||
| this(null, userId, courseId, isPublic); | ||
| } | ||
kkiseug marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
|
Comment on lines
+9
to
+22
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. 이 데이터들을 Course에 추가하지 않으신 이유가 '소유'의 관계가 Course의 것이 아니라고 하셨는데요. 또 반대로 User 쪽에 추가하는 것도 괜찮을 것 같은데, 이것은 어떨까요?
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. 어떤 Course를 응답할 때, 그냥 그 Course만 조회하는게 아니라
라는 복잡한 규칙이 생기네요.
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.
짧게 해결 방안을 생각했을 때는 즐겨찾기까지 고려하여 isPublic 정도만 Course에 넘겨주는 방식은 어떤가요? 단점도 해결되고, 확장성도 그대로 유지될 거 같긴합니다.
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. 네! 일단 그렇게 구현하면 두 장점 모두 적절히 챙기는 것 같아서 좋은 것 같아요. 추후에 시간이 되시면 DocumentDB에서 관계를 어떻게 표현해야 하는지 좀 더 고민해보면 좋을 것 같습니다. 모델을 나누는 방식은 좀 더 RDB스럽다고 생각했어요. DocumentDB를 이왕 쓰고 있으니 데이터를 중복해서 임베딩 해두는게 기초가 되는 설계라고 생각합니다.
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. 아, 제가 너무 도메인에만 집중했던 거 같습니다. 해당 코멘트를 계기로 다시 한번 MongoDB에 대해서 학습을 해봤어요. 조회의 경우에 돔푸가 말한 것처럼 다음과 같은 상황이 생길 수 있고,
또, 추가로 유저가 자신만의 코스를 조회하는 상황에서도 이렇게 조회가 될 거 같아요.
혹은 간단한 $lookup 같은 연산을 통해서 찾을 수도 있겠죠? 하지만, 코스 추가 기능의 특성이
등을 고려하면 $lookup이나 저런 조회로 인한 다수 쿼리 발생이 코스 하나하나의 데이터가 큰 만큼 애플리케이션 성능에 꽤나 문제를 일으킬 것 같습니다. 또한, MongoDB를 '잘' 사용하는 것이 아니라는 점도 들 수 있겠네요. 이와 관련해서 MongoDB Docs에도 기준을 만들어 놨더라구요. 네.. 그래서 결론은 MongoDB에 맞게 설계하고, 성능이나 복잡성을 생각하면 Embed 하는게 좋다고 생각합니다. 다만 User 자체를 Embed 하기보다는 CreatorInfo 같은 객체를 만들어 넣어주는 것이 좋을 거 같네요. ㅎㅎ 쓰다보니 코멘트가 길어졌는데, 결론에 대한 돔푸의 의견도 궁금합니다. |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| package coursepick.coursepick.domain.course; | ||
|
|
||
| import org.springframework.data.repository.Repository; | ||
|
|
||
| public interface UserCreatedCourseRepository extends Repository<UserCreatedCourse, String> { | ||
|
|
||
| void save(UserCreatedCourse userCreatedCourse); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,6 +2,7 @@ | |
|
|
||
| import coursepick.coursepick.domain.course.Coordinate; | ||
| import coursepick.coursepick.domain.course.CoordinateSnapper; | ||
| import coursepick.coursepick.domain.course.SnapResult; | ||
| import coursepick.coursepick.logging.LogContent; | ||
| import lombok.RequiredArgsConstructor; | ||
| import lombok.extern.slf4j.Slf4j; | ||
|
|
@@ -17,6 +18,8 @@ | |
| import java.util.stream.Collectors; | ||
| import java.util.stream.IntStream; | ||
|
|
||
| import static coursepick.coursepick.application.exception.ErrorType.INVALID_SNAP_COORDINATE_SIZE; | ||
|
|
||
| @Slf4j | ||
| @Component | ||
| @Profile({"dev", "prod"}) | ||
|
|
@@ -26,9 +29,9 @@ public class OsrmCoordinateSnapper implements CoordinateSnapper { | |
| private final RestClient osrmRestClient; | ||
|
|
||
| @Override | ||
| public List<Coordinate> snap(List<Coordinate> coordinates) { | ||
| public SnapResult snap(List<Coordinate> coordinates) { | ||
| if (coordinates.size() < 2) { | ||
| return coordinates; | ||
| throw INVALID_SNAP_COORDINATE_SIZE.create(); | ||
| } | ||
|
|
||
| String coordinatesParam = coordinates.stream() | ||
|
|
@@ -52,26 +55,29 @@ public List<Coordinate> snap(List<Coordinate> coordinates) { | |
| .body(new ParameterizedTypeReference<>() { | ||
| }); | ||
|
|
||
| log.info("OSRM Match 결과: {}", response); | ||
| return parseMatchResponse(response, coordinates); | ||
| log.info("[OSRM Snap 결과] {}", response); | ||
| return parseSnapResponse(response, coordinates); | ||
| } catch (Exception e) { | ||
| log.warn("[EXCEPTION] OSRM 좌표 매칭 실패", LogContent.exception(e)); | ||
| return coordinates; | ||
| log.warn("[EXCEPTION] OSRM 좌표 Snap 실패", LogContent.exception(e)); | ||
| return new SnapResult(coordinates, 0); | ||
| } | ||
|
Comment on lines
+58
to
63
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. (prod 포함) OSRM 응답 전체 info 로깅은 위치정보/로그 용량 측면에서 위험합니다.
제안 diff (요약 로깅 + debug 레벨)- log.info("[OSRM Snap 결과] {}", response);
+ // Avoid logging full geometry/coordinates in prod logs.
+ log.debug("[OSRM Snap 결과] matchings_size={}", ((List<?>) response.getOrDefault("matchings", List.of())).size());
return parseSnapResponse(response, coordinates);🤖 Prompt for AI Agents |
||
| } | ||
|
|
||
| private List<Coordinate> parseMatchResponse(Map<String, Object> response, List<Coordinate> originals) { | ||
| private SnapResult parseSnapResponse(Map<String, Object> response, List<Coordinate> originals) { | ||
| try { | ||
| List<Map<String, Object>> matchings = (List<Map<String, Object>>) response.get("matchings"); | ||
| Map<String, Object> geometry = (Map<String, Object>) matchings.get(0).get("geometry"); | ||
| List<List<Double>> coordinates = (List<List<Double>>) geometry.get("coordinates"); | ||
| Double length = (Double) matchings.get(0).get("distance"); | ||
|
|
||
| return coordinates.stream() | ||
| List<Coordinate> snappedCoordinates = coordinates.stream() | ||
| .map(coord -> new Coordinate(coord.get(1), coord.get(0))) | ||
| .toList(); | ||
|
|
||
| return new SnapResult(snappedCoordinates, length); | ||
| } catch (Exception e) { | ||
| log.warn("[EXCEPTION] OSRM Match 응답 파싱 실패", LogContent.exception(e)); | ||
| return originals; | ||
| log.warn("[EXCEPTION] OSRM Snap 응답 파싱 실패", LogContent.exception(e)); | ||
| return new SnapResult(originals, 0); | ||
| } | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,34 @@ | ||
| package coursepick.coursepick.presentation; | ||
|
|
||
| import coursepick.coursepick.application.CourseApplicationService; | ||
| import coursepick.coursepick.application.dto.SnapResponse; | ||
| import coursepick.coursepick.domain.course.Coordinate; | ||
| import coursepick.coursepick.presentation.api.CoordinateWebApi; | ||
| import coursepick.coursepick.presentation.dto.SnapWebRequest; | ||
| import coursepick.coursepick.presentation.dto.SnapWebResponse; | ||
| import coursepick.coursepick.security.Login; | ||
| import lombok.RequiredArgsConstructor; | ||
| import org.springframework.web.bind.annotation.PostMapping; | ||
| import org.springframework.web.bind.annotation.RequestBody; | ||
| import org.springframework.web.bind.annotation.RestController; | ||
|
|
||
| import java.util.List; | ||
|
|
||
| @RestController | ||
| @RequiredArgsConstructor | ||
| public class CoordinateWebController implements CoordinateWebApi { | ||
|
|
||
| private final CourseApplicationService courseApplicationService; | ||
|
|
||
| @Override | ||
| @Login | ||
| @PostMapping("/coordinates/snap") | ||
| public SnapWebResponse snapCoordinates(@RequestBody SnapWebRequest snapWebRequest) { | ||
| List<Coordinate> coordinates = snapWebRequest.coordinates().stream() | ||
| .map(dto -> new Coordinate(dto.latitude(), dto.longitude())) | ||
| .toList(); | ||
|
|
||
| SnapResponse snapResponse = courseApplicationService.snapCoordinates(coordinates); | ||
| return SnapWebResponse.from(snapResponse); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,13 +1,17 @@ | ||||||||||
| package coursepick.coursepick.presentation; | ||||||||||
|
|
||||||||||
| import coursepick.coursepick.application.CourseApplicationService; | ||||||||||
| import coursepick.coursepick.application.dto.CourseResponse; | ||||||||||
| import coursepick.coursepick.application.dto.CoursesResponse; | ||||||||||
| import coursepick.coursepick.domain.course.Coordinate; | ||||||||||
| import coursepick.coursepick.domain.course.CourseFindCondition; | ||||||||||
| import coursepick.coursepick.presentation.api.CourseWebApi; | ||||||||||
| import coursepick.coursepick.presentation.dto.CoordinateWebResponse; | ||||||||||
| import coursepick.coursepick.presentation.dto.CourseCreateWebRequest; | ||||||||||
| import coursepick.coursepick.presentation.dto.CourseWebResponse; | ||||||||||
| import coursepick.coursepick.presentation.dto.CoursesWebResponse; | ||||||||||
| import coursepick.coursepick.security.Login; | ||||||||||
| import coursepick.coursepick.security.UserId; | ||||||||||
| import lombok.RequiredArgsConstructor; | ||||||||||
| import org.springframework.web.bind.annotation.*; | ||||||||||
|
|
||||||||||
|
|
@@ -66,4 +70,17 @@ public List<CourseWebResponse> findFavoriteCourses(@RequestParam("courseIds") Li | |||||||||
| .map(CourseWebResponse::from) | ||||||||||
| .toList(); | ||||||||||
| } | ||||||||||
|
|
||||||||||
| @Override | ||||||||||
| @Login | ||||||||||
| @PostMapping("/courses/create") | ||||||||||
| public CourseWebResponse create(@UserId String userId, @RequestBody CourseCreateWebRequest courseCreateWebRequest) { | ||||||||||
|
Comment on lines
+76
to
+77
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. 🧩 Analysis chain🏁 Script executed: fd "CourseCreateWebRequest.java" --exec cat {}Repository: woowacourse-teams/2025-course-pick Length of output: 1144 🏁 Script executed: fd "CourseWebController.java" -x head -n 90 {} | tail -n 30Repository: woowacourse-teams/2025-course-pick Length of output: 1169 🏁 Script executed: fd "AdminWebController.java" -x grep -A 5 "login"Repository: woowacourse-teams/2025-course-pick Length of output: 713 🏁 Script executed: rg "@PostMapping" -A 3 --type java | grep -A 3 "@RequestBody" | head -50Repository: woowacourse-teams/2025-course-pick Length of output: 2586 🏁 Script executed: fd "SignWebRequest.java|SnapWebRequest.java" --exec cat {}Repository: woowacourse-teams/2025-course-pick Length of output: 800
수정 방법 @Login
@PostMapping("/courses/create")
-public CourseWebResponse create(@UserId String userId, @RequestBody CourseCreateWebRequest courseCreateWebRequest) {
+public CourseWebResponse create(@UserId String userId, @RequestBody @Valid CourseCreateWebRequest courseCreateWebRequest) {📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||
| List<Coordinate> coordinates = courseCreateWebRequest.coordinates().stream() | ||||||||||
| .map(dto -> new Coordinate(dto.latitude(), dto.longitude())) | ||||||||||
| .toList(); | ||||||||||
|
|
||||||||||
|
Comment on lines
+78
to
+81
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.
계층을 정말 지키고 싶다면 따로 DTO를 만들었을 것 같아요.
하지만 이러면 공이 너무 들어가는 것 같기도 하고, 큰 의미가 없다고 판단되어서 지금같은 형태가 더 좋아보입니다. |
||||||||||
| CourseResponse courseResponse = courseApplicationService.create(userId, courseCreateWebRequest.name(), coordinates); | ||||||||||
|
|
||||||||||
| return CourseWebResponse.from(courseResponse); | ||||||||||
| } | ||||||||||
| } | ||||||||||

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.
이거
SnapResult와SnapReponse를 분리하신 이유가 궁금합니다.SnapResult를 그대로 응답해도 될 것 같거든요.