Conversation
- Course에서 유저가 생성함을 의도하는 팩토리 메서드 구현
- AbstractIntegrationTest에서 AbstractMockServerTest 상속하도록 변경
- 문서화 API에 AUTH 명시
|
Caution Review failedFailed to post review comments Walkthrough좌표에서 고도를 제거하고 코스 도메인 모델을 단순화하는 대규모 리팩토링입니다. 세그먼트 기반의 도로 타입 및 난이도 정보를 제거하고, 좌표 기반 모델로 전환합니다. 동시에 사용자 코스 생성 및 좌표 스냅 기능을 추가합니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant CourseV1WebController
participant CourseApplicationService
participant UserRepository
participant CourseRepository
participant UserCreatedCourseRepository
Client->>CourseV1WebController: POST /courses/create<br/>(userId, CourseCreateWebRequest)
CourseV1WebController->>CourseApplicationService: create(userId, coordinates, name)
CourseApplicationService->>UserRepository: findById(userId)
UserRepository-->>CourseApplicationService: Optional<User>
alt User Not Found
CourseApplicationService-->>Client: 404 NOT_EXIST_USER
else User Found
CourseApplicationService->>CourseRepository: save(Course)
CourseRepository-->>CourseApplicationService: Course
CourseApplicationService->>UserCreatedCourseRepository: save(UserCreatedCourse)
UserCreatedCourseRepository-->>CourseApplicationService: void
CourseApplicationService-->>CourseV1WebController: CourseResponse
CourseV1WebController-->>Client: 200 CourseWebResponse
end
sequenceDiagram
participant Client
participant CoordinateWebController
participant CourseApplicationService
participant CoordinateSnapper
participant OSRM API
Client->>CoordinateWebController: POST /coordinates/snap<br/>(SnapWebRequest)
CoordinateWebController->>CourseApplicationService: snapCoordinates(List<Coordinate>)
CourseApplicationService->>CoordinateSnapper: snap(List<Coordinate>)
CoordinateSnapper->>OSRM API: POST /match/v1/driving<br/>(coordinates)
OSRM API-->>CoordinateSnapper: { matchings, distance }
CoordinateSnapper-->>CourseApplicationService: SnapResult
CourseApplicationService-->>CoordinateWebController: SnapResponse
CoordinateWebController-->>Client: 200 SnapWebResponse
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In
@backend/src/main/java/coursepick/coursepick/domain/course/UserCreatedCourse.java:
- Around line 9-21: Add accessors and optional indexes to UserCreatedCourse:
annotate the class UserCreatedCourse with Lombok's @Getter (or add explicit
getters for getUserId(), getCourseId(), isPublic()) so external code can read
userId, courseId and isPublic; additionally add @Indexed(name = "idx_user_id")
to the userId field and @Indexed(name = "idx_course_id") to the courseId field
to improve lookup performance (ensure imports for
org.springframework.data.mongodb.core.index.Indexed and lombok.Getter or
implement methods manually).
In
@backend/src/main/java/coursepick/coursepick/infrastructure/snapper/OsrmCoordinateSnapper.java:
- Line 67: The code in OsrmCoordinateSnapper casts the "distance" value directly
to Double which can throw ClassCastException if the API returns an Integer;
instead retrieve it as a Number from matchings.get(0).get("distance"),
null-check it, and call number.doubleValue() (e.g., Number n = (Number)
matchings.get(0).get("distance"); double length = n != null ? n.doubleValue() :
0.0) to safely handle both Integer and Double values.
In
@backend/src/main/java/coursepick/coursepick/presentation/CourseWebController.java:
- Around line 71-72: CourseCreateWebRequest의 필드 제약 어노테이션이 동작하지 않으므로
CourseWebController의 create 메서드 시그니처에 요청 바디 검증을 활성화하도록 @Valid를 추가하세요; 구체적으로
public CourseWebResponse create(@UserId String userId, @RequestBody
CourseCreateWebRequest courseCreateWebRequest) 선언에서 CourseCreateWebRequest 파라미터에
@Valid를 함께 적용해 CourseCreateWebRequest의 @NotNull 등 검증 어노테이션이 트리거되도록 수정하세요 (참조:
CourseWebController.create, CourseCreateWebRequest).
In
@backend/src/test/java/coursepick/coursepick/test_util/AbstractIntegrationTest.java:
- Line 10: AbstractIntegrationTest currently extends AbstractMockServerTest
causing every integration test to pay MockWebServer startup/teardown overhead;
change the inheritance so AbstractIntegrationTest no longer extends
AbstractMockServerTest, create a new AbstractMockServerIntegrationTest that
extends AbstractMockServerTest (and contains the mock server setup/teardown),
update CourseRepositoryTest to extend AbstractIntegrationTest (no mock server)
and update CourseApplicationServiceTest to extend
AbstractMockServerIntegrationTest (since it uses mock()), and ensure only tests
that call mock() or rely on MockWebServer inherit from the mock-enabled abstract
class.
🧹 Nitpick comments (11)
backend/src/main/java/coursepick/coursepick/domain/course/Course.java (1)
53-65: 사용자 생성 코스를 위한 팩토리 메서드가 잘 구현되었습니다.사용자가 직접 난이도를 지정할 수 있도록 분리된 생성 경로를 제공한 설계가 적절합니다.
기존 생성자(43-51 라인)와 새 생성자(53-61 라인) 간에 중복 로직이 있습니다. 향후 유지보수를 고려해 공통 초기화 로직을 private 헬퍼 메서드로 추출하는 것을 고려해 볼 수 있습니다.
♻️ 선택적 리팩토링 제안
private Course(String id, String name, RoadType roadType, List<Coordinate> rawCoordinates) { - this.id = id; - this.name = new CourseName(name); - this.roadType = roadType; - this.segments = refineCoordinates(rawCoordinates); - this.length = calculateLength(segments); - this.inclineSummary = InclineSummary.of(segments); + this(id, name, roadType, rawCoordinates, null); this.difficulty = Difficulty.fromLengthAndRoadType(length(), roadType); } -private Course(String name, RoadType roadType, Difficulty difficulty, List<Coordinate> coordinates) { - this.id = null; - this.name = new CourseName(name); - this.roadType = roadType; - this.segments = refineCoordinates(coordinates); - this.length = calculateLength(segments); - this.inclineSummary = InclineSummary.of(segments); +private Course(String id, String name, RoadType roadType, List<Coordinate> coordinates, Difficulty explicitDifficulty) { + this.id = id; + this.name = new CourseName(name); + this.roadType = roadType; + this.segments = refineCoordinates(coordinates); + this.length = calculateLength(segments); + this.inclineSummary = InclineSummary.of(segments); + this.difficulty = explicitDifficulty != null ? explicitDifficulty : Difficulty.fromLengthAndRoadType(length(), roadType); +} + +private Course(String name, RoadType roadType, Difficulty difficulty, List<Coordinate> coordinates) { + this(null, name, roadType, coordinates, difficulty); this.difficulty = difficulty; }backend/src/main/java/coursepick/coursepick/presentation/dto/SnapWebRequest.java (1)
8-11: 입력 유효성 검증 어노테이션 추가를 고려해 주세요.
coordinates필드에@NotNull과@Size어노테이션을 추가하면 API 레벨에서 조기에 잘못된 요청을 거부할 수 있습니다. 현재는 null이나 빈 리스트가 서비스 레이어까지 전달될 수 있습니다.♻️ 유효성 검증 추가 제안
package coursepick.coursepick.presentation.dto; import io.swagger.v3.oas.annotations.media.Schema; +import jakarta.validation.Valid; +import jakarta.validation.constraints.NotNull; +import jakarta.validation.constraints.Size; import java.util.List; @Schema(description = "좌표 스냅 요청") public record SnapWebRequest( @Schema(description = "도로에 스냅할 좌표 목록", requiredMode = Schema.RequiredMode.REQUIRED) + @NotNull(message = "좌표 목록은 필수입니다") + @Size(min = 2, message = "최소 2개의 좌표가 필요합니다") + @Valid List<CoordinateWebRequest> coordinates ) { }backend/src/main/java/coursepick/coursepick/domain/course/SnapResult.java (1)
5-8: length 필드에 대한 문서화를 추가하는 것을 고려해보세요.
length필드가 무엇을 나타내는지(예: 미터 단위 거리, 킬로미터 단위 거리 등) 명확하지 않습니다. JavaDoc이나 필드 주석을 추가하면 향후 유지보수 시 혼란을 방지할 수 있습니다.📝 문서화 추가 제안
+/** + * 좌표 스냅 결과를 나타냅니다. + * + * @param coordinates 스냅된 좌표 목록 + * @param length 경로의 총 길이 (미터 단위) + */ public record SnapResult( List<Coordinate> coordinates, double length ) { }backend/src/main/java/coursepick/coursepick/presentation/CoordinateWebController.java (1)
26-26: 요청 본문 검증 추가를 권장합니다.
@RequestBody에@Valid또는@Validated어노테이션이 누락되어 있습니다. DTO의 제약 조건(예:@NotNull)을 자동으로 검증하려면 추가해야 합니다.♻️ 제안하는 수정
+import jakarta.validation.Valid; + @Override @Login @PostMapping("/coordinate/snap") -public SnapWebResponse snapCoordinates(@RequestBody SnapWebRequest snapWebRequest) { +public SnapWebResponse snapCoordinates(@Valid @RequestBody SnapWebRequest snapWebRequest) {backend/src/main/java/coursepick/coursepick/presentation/dto/CourseCreateWebRequest.java (3)
9-27: 필수 필드에 검증 어노테이션 추가를 권장합니다.
name,roadType,difficulty필드에@NotNull또는@NotBlank검증이 누락되었습니다. 현재는coordinates만 검증되고 있어 null 값이 서비스 계층까지 전달될 수 있습니다.♻️ 제안하는 수정
+import jakarta.validation.constraints.NotBlank; + @Schema(description = "코스 생성 요청") public record CourseCreateWebRequest( @Schema(description = "코스 이름", example = "한강 러닝 코스", requiredMode = Schema.RequiredMode.REQUIRED) + @NotBlank String name, @Schema( description = "도로 타입 (트랙, 트레일, 보도, 알수없음)", example = "트레일", allowableValues = {"트랙", "트레일", "보도", "알수없음"}, requiredMode = Schema.RequiredMode.REQUIRED ) + @NotBlank String roadType, @Schema( description = "난이도 (쉬움, 보통, 어려움)", example = "보통", allowableValues = {"쉬움", "보통", "어려움"}, requiredMode = Schema.RequiredMode.REQUIRED ) + @NotBlank String difficulty,
29-31: 좌표 목록 크기 검증 추가를 권장합니다.문서화에는 "최소 2개 이상"이라고 명시되어 있지만,
@Size(min = 2)검증이 누락되었습니다. 이로 인해 빈 리스트나 단일 좌표가 서비스 계층까지 전달될 수 있습니다.♻️ 제안하는 수정
+import jakarta.validation.constraints.Size; + @Schema(description = "코스를 구성하는 좌표 목록 (최소 2개 이상)", requiredMode = Schema.RequiredMode.REQUIRED) @NotNull +@Size(min = 2) List<CoordinateWebRequest> coordinates
13-27: 허용 값 검증 추가를 고려하세요.
roadType과difficulty는 Swagger 문서에 허용 값이 명시되어 있지만, 런타임에 이를 강제하는 검증이 없습니다.@Pattern또는 별도의 커스텀 검증을 추가하거나, 서비스 계층에서 검증하는 것이 명확한지 확인하세요.💡 선택적 개선 제안
+import jakarta.validation.constraints.Pattern; + @Schema( description = "도로 타입 (트랙, 트레일, 보도, 알수없음)", example = "트레일", allowableValues = {"트랙", "트레일", "보도", "알수없음"}, requiredMode = Schema.RequiredMode.REQUIRED ) +@Pattern(regexp = "^(트랙|트레일|보도|알수없음)$", message = "유효하지 않은 도로 타입입니다") String roadType, @Schema( description = "난이도 (쉬움, 보통, 어려움)", example = "보통", allowableValues = {"쉬움", "보통", "어려움"}, requiredMode = Schema.RequiredMode.REQUIRED ) +@Pattern(regexp = "^(쉬움|보통|어려움)$", message = "유효하지 않은 난이도입니다") String difficulty,backend/src/main/java/coursepick/coursepick/presentation/api/CourseWebApi.java (1)
3-3: 와일드카드 import 사용을 지양하는 것이 좋습니다.
import coursepick.coursepick.presentation.dto.*;보다는 실제 사용하는 클래스를 명시적으로 import하는 것이 코드 가독성과 유지보수성에 도움이 됩니다.♻️ 명시적 import 제안
-import coursepick.coursepick.presentation.dto.*; +import coursepick.coursepick.presentation.dto.CourseCreateWebRequest; +import coursepick.coursepick.presentation.dto.CourseWebResponse; +import coursepick.coursepick.presentation.dto.CoursesWebResponse; +import coursepick.coursepick.presentation.dto.CoordinateWebResponse;backend/src/main/java/coursepick/coursepick/application/CourseApplicationService.java (2)
110-124: 중복된 검증 패턴을 추출할 수 있습니다.
getRoadType과getDifficulty메서드가 동일한 try-catch 패턴을 사용하고 있습니다. 이를 제네릭 헬퍼 메서드로 추출하면 코드 중복을 줄이고 유지보수성을 개선할 수 있습니다.♻️ 제네릭 검증 헬퍼 추출 제안
+ private <T extends Enum<T>> T parseEnum(String value, Class<T> enumClass, ErrorType errorType) { + try { + return Enum.valueOf(enumClass, value); + } catch (IllegalArgumentException e) { + throw errorType.create(value); + } + } + private RoadType getRoadType(String roadType) { - try { - return RoadType.valueOf(roadType); - } catch (IllegalArgumentException e) { - throw INVALID_ROAD_TYPE.create(roadType); - } + return parseEnum(roadType, RoadType.class, INVALID_ROAD_TYPE); } private Difficulty getDifficulty(String difficulty) { - try { - return Difficulty.valueOf(difficulty); - } catch (IllegalArgumentException e) { - throw INVALID_DIFFICULTY.create(difficulty); - } + return parseEnum(difficulty, Difficulty.class, INVALID_DIFFICULTY); }
104-104: 매직 넘버(boolean)의 의미를 명확히 해주세요.
UserCreatedCourse생성 시 하드코딩된false값의 의미가 코드상에서 명확하지 않습니다. 이것이 무엇을 나타내는지(예: isDeleted, isPublic 등) 주석으로 설명하거나, 명명된 상수 또는 빌더 패턴을 사용하는 것이 좋습니다.// 예시 1: 주석 추가 UserCreatedCourse userCreatedCourse = new UserCreatedCourse( user.id(), savedCourse.id(), false // isDeleted: 새로 생성된 코스는 삭제되지 않음 ); // 예시 2: 명명된 상수 사용 private static final boolean NOT_DELETED = false; UserCreatedCourse userCreatedCourse = new UserCreatedCourse( user.id(), savedCourse.id(), NOT_DELETED );UserCreatedCourse 도메인 객체에 명명된 생성자나 팩토리 메서드를 제공하는 것도 고려해보세요.
backend/src/main/java/coursepick/coursepick/presentation/CourseWebController.java (1)
71-71: REST 엔드포인트 네이밍 컨벤션 고려현재
POST /courses/create로 정의되어 있습니다. RESTful 컨벤션에서는 HTTP 메서드가 이미 생성 의도를 표현하므로POST /courses가 더 일반적입니다. 다만, 프로젝트 내 기존 컨벤션이 있다면 그에 따르셔도 됩니다.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (29)
backend/src/main/java/coursepick/coursepick/application/CourseApplicationService.javabackend/src/main/java/coursepick/coursepick/application/dto/SnapResponse.javabackend/src/main/java/coursepick/coursepick/application/exception/ErrorType.javabackend/src/main/java/coursepick/coursepick/domain/course/CoordinateSnapper.javabackend/src/main/java/coursepick/coursepick/domain/course/Course.javabackend/src/main/java/coursepick/coursepick/domain/course/CourseRepository.javabackend/src/main/java/coursepick/coursepick/domain/course/SnapResult.javabackend/src/main/java/coursepick/coursepick/domain/course/UserCreatedCourse.javabackend/src/main/java/coursepick/coursepick/domain/course/UserCreatedCourseRepository.javabackend/src/main/java/coursepick/coursepick/domain/user/UserRepository.javabackend/src/main/java/coursepick/coursepick/infrastructure/mongodb/CourseRepositoryMongoTemplateImpl.javabackend/src/main/java/coursepick/coursepick/infrastructure/snapper/DummyCoordinateSnapper.javabackend/src/main/java/coursepick/coursepick/infrastructure/snapper/OsrmCoordinateSnapper.javabackend/src/main/java/coursepick/coursepick/presentation/AdminWebController.javabackend/src/main/java/coursepick/coursepick/presentation/CoordinateWebController.javabackend/src/main/java/coursepick/coursepick/presentation/CourseWebController.javabackend/src/main/java/coursepick/coursepick/presentation/api/CoordinateWebApi.javabackend/src/main/java/coursepick/coursepick/presentation/api/CourseWebApi.javabackend/src/main/java/coursepick/coursepick/presentation/api/OpenApiConfig.javabackend/src/main/java/coursepick/coursepick/presentation/dto/CoordinateWebRequest.javabackend/src/main/java/coursepick/coursepick/presentation/dto/CourseCreateWebRequest.javabackend/src/main/java/coursepick/coursepick/presentation/dto/SnapWebRequest.javabackend/src/main/java/coursepick/coursepick/presentation/dto/SnapWebResponse.javabackend/src/main/java/coursepick/coursepick/security/PathAllowlistFilter.javabackend/src/test/java/coursepick/coursepick/application/CourseApplicationServiceTest.javabackend/src/test/java/coursepick/coursepick/infrastructure/OsrmCoordinateSnapperTest.javabackend/src/test/java/coursepick/coursepick/test_util/AbstractIntegrationTest.javabackend/src/test/java/coursepick/coursepick/test_util/AbstractSecurityTest.javabackend/src/test/java/coursepick/coursepick/test_util/DatabaseTestUtil.java
🧰 Additional context used
🧬 Code graph analysis (4)
backend/src/main/java/coursepick/coursepick/presentation/CoordinateWebController.java (1)
backend/src/main/java/coursepick/coursepick/presentation/CourseWebController.java (1)
RestController(19-87)
backend/src/main/java/coursepick/coursepick/domain/course/UserCreatedCourse.java (1)
backend/src/main/java/coursepick/coursepick/domain/course/Course.java (1)
Document(16-131)
backend/src/main/java/coursepick/coursepick/domain/course/Course.java (1)
android/app/src/main/java/io/coursepick/coursepick/presentation/filter/CourseFilterAction.kt (1)
difficulty(17-19)
backend/src/main/java/coursepick/coursepick/presentation/CourseWebController.java (1)
backend/src/main/java/coursepick/coursepick/application/dto/CourseResponse.java (1)
RequiredArgsConstructor(12-46)
🔇 Additional comments (27)
backend/src/main/java/coursepick/coursepick/presentation/api/OpenApiConfig.java (2)
4-4: LGTM! 필요한 import가 올바르게 추가되었습니다.새로운
@SecurityScheme어노테이션에 필요한 import가 정확하게 추가되었습니다.Also applies to: 6-6
25-31: Bearer JWT 인증 스키마가 올바르게 구현되었습니다.OpenAPI 문서에 JWT Bearer 인증을 위한 보안 스키마가 적절하게 추가되었으며, 실제 엔드포인트에도
@SecurityRequirement(name = "BearerAuth")가 적용되어 있습니다. 설정값들이 모두 올바르며, 한국어 설명도 명확합니다.backend/src/main/java/coursepick/coursepick/domain/user/UserRepository.java (1)
11-12: LGTM!Spring Data 컨벤션을 따르는 표준적인
findById메서드 추가입니다.Optional반환 타입이 적절하게 사용되었습니다.backend/src/main/java/coursepick/coursepick/domain/course/CourseRepository.java (1)
11-11: LGTM!
save메서드가 저장된Course객체를 반환하도록 변경되어, 생성된 ID를 활용할 수 있게 되었습니다. Spring Data 표준 패턴을 따르며UserCreatedCourse생성 시courseId를 얻는 데 필요한 변경입니다.backend/src/main/java/coursepick/coursepick/presentation/dto/SnapWebResponse.java (1)
1-24: LGTM!레이어 간 DTO 분리가 잘 되어 있고,
from팩토리 메서드를 통한 변환 로직이 명확합니다. OpenAPI 스키마 어노테이션도 적절하게 적용되었습니다.backend/src/main/java/coursepick/coursepick/security/PathAllowlistFilter.java (1)
42-44: 불안(concern)이 확인되지 않았습니다./courses/create엔드포인트는@Login어노테이션으로 인증이 올바르게 보호되고 있습니다.
/courses/create엔드포인트는 다음과 같이 인증이 적용되어 있습니다:
@Login어노테이션으로 인증 필요@UserId String userId파라미터로 인증된 사용자만 접근 가능LoginInterceptor가@Login어노테이션을 통해 인증을 강제PathAllowlistFilter의 화이트리스트는 인증 필터가 아니라 경로 기반 접근 제어입니다. 요청이 LoginInterceptor의 인증 검사에 도달하려면 먼저 PathAllowlistFilter를 통과해야 하므로, 인증된 엔드포인트를 화이트리스트에 추가하는 것이 올바른 구현입니다.
backend/src/main/java/coursepick/coursepick/infrastructure/mongodb/CourseRepositoryMongoTemplateImpl.java (1)
26-27: 저장된 엔티티 반환으로의 변경이 적절합니다.save 메서드가 void 대신 Course를 반환하도록 변경된 것은 Spring Data의 일반적인 패턴과 일치하며, 호출자가 저장된 엔티티(생성된 ID 등 포함)를 받을 수 있게 합니다.
mongoTemplate.save()가 이미 저장된 객체를 반환하므로 구현도 정확합니다.backend/src/main/java/coursepick/coursepick/domain/course/CoordinateSnapper.java (1)
7-7: SnapResult 반환으로의 변경이 적절합니다.
List<Coordinate>대신SnapResult를 반환하도록 변경한 것은 좌표뿐만 아니라 경로 길이 정보도 함께 제공할 수 있게 하여 API를 풍부하게 만듭니다. PR 전반에 걸쳐 일관되게 적용된 변경사항입니다.backend/src/main/java/coursepick/coursepick/domain/course/UserCreatedCourseRepository.java (1)
5-7: 깔끔한 리포지토리 인터페이스입니다.Spring Data Repository 패턴을 잘 따르고 있으며, 사용 사례에 맞게 최소한의 메서드만 노출하고 있습니다. 구조가 명확하고 적절합니다.
backend/src/main/java/coursepick/coursepick/presentation/dto/CoordinateWebRequest.java (1)
6-12: 좌표 범위 검증은 이미 도메인 계층에서 구현되어 있습니다.
Coordinate도메인 생성자에서validateLatitudeRange()와validateLongitudeRange()메서드를 통해 범위 검증이 수행되고 있습니다.CourseWebController의 좌표 변환 로직(line 73-75)에서CoordinateWebRequest가Coordinate도메인 객체로 변환될 때 자동으로 검증되므로, DTO 레벨의@Min/@Max애노테이션이 없어도 문제없습니다.Likely an incorrect or invalid review comment.
backend/src/main/java/coursepick/coursepick/presentation/CoordinateWebController.java (1)
27-29: DTO → 도메인 변환 위치가 적절합니다.컨트롤러에서 DTO를 도메인 객체로 변환하는 방식은 올바른 설계 선택입니다. 이는
CourseWebController의 코스 생성 로직(lines 56-58)과 일관된 패턴이며, 서비스 계층이 프레젠테이션 계층의 DTO에 의존하지 않도록 보장합니다.backend/src/main/java/coursepick/coursepick/infrastructure/snapper/DummyCoordinateSnapper.java (1)
16-17: 반환 타입 변경이 적절합니다.
SnapResult를 반환하도록 변경한 것은 도메인 API 업데이트와 일치합니다. 하드코딩된 길이 값100은 더미 구현체이므로 허용되지만, 이 값이 임의로 선택되었음을 인지하고 있어야 합니다. 실제 운영 환경에서는@Fallback어노테이션에 따라 기본 구현체가 사용 불가능할 때만 호출됩니다.backend/src/main/java/coursepick/coursepick/application/dto/SnapResponse.java (1)
7-10: LGTM!간단하고 명확한 애플리케이션 계층 DTO입니다. 도메인의
SnapResult와 프레젠테이션 계층 사이의 깔끔한 분리를 제공합니다.backend/src/test/java/coursepick/coursepick/test_util/AbstractIntegrationTest.java (1)
17-19: MongoDB에서는 FK 제약 조건이 없으므로 cleanup 순서는 관계없습니다.이 애플리케이션은 MongoDB를 사용하고 있으며, User, Course, UserCreatedCourse 모두 @document 엔티티입니다. UserCreatedCourse의 userId와 courseId는 단순 String 필드이며, @manytoone이나 @joincolumn과 같은 FK 관계 어노테이션이 없습니다. MongoDB는 SQL 데이터베이스처럼 FK 제약 조건을 강제하지 않으므로, 현재의 cleanup 순서는 문제가 되지 않습니다.
Likely an incorrect or invalid review comment.
backend/src/test/java/coursepick/coursepick/application/CourseApplicationServiceTest.java (2)
235-268: 테스트 커버리지가 잘 구성되어 있습니다.snap 기능에 대한 테스트가 적절하게 작성되었으며, mock 응답도 실제 OSRM API 구조를 잘 반영하고 있습니다.
270-302: 코스 생성 테스트가 포괄적입니다.Course와 UserCreatedCourse 관계가 모두 올바르게 저장되는지 검증하고 있어 좋습니다. 통합 테스트로서 적절한 범위를 커버하고 있습니다.
backend/src/main/java/coursepick/coursepick/presentation/api/CourseWebApi.java (1)
107-189: API 문서화가 훌륭합니다.새로운 코스 생성 엔드포인트에 대한 OpenAPI 문서가 매우 상세하게 작성되어 있습니다. 요청 예시, 에러 응답, 보안 요구사항이 모두 명확하게 명시되어 있어 API 사용자에게 큰 도움이 될 것입니다.
backend/src/main/java/coursepick/coursepick/application/exception/ErrorType.java (1)
28-67: 새로운 에러 타입이 적절하게 정의되었습니다.
INVALID_ROAD_TYPE,INVALID_DIFFICULTY,NOT_EXIST_USER에러 타입이 명확한 메시지와 함께 추가되었으며, 기존 패턴과 일관성 있게 구현되어 있습니다.backend/src/main/java/coursepick/coursepick/application/CourseApplicationService.java (2)
96-108: DTO→도메인 변환 위치의 일관성을 검토해주세요.PR 목표에서 언급하신 대로, 좌표는 Controller에서
List<CoordinateDto>→List<Coordinate>로 변환되어 전달됩니다. 하지만roadType과difficulty는 여전히 String으로 전달되어 서비스 레이어에서 enum으로 변환됩니다 (lines 110-124).이러한 불일치는 다음과 같은 문제를 야기할 수 있습니다:
- 변환 책임이 여러 레이어에 분산되어 일관성이 떨어짐
- 컨트롤러에서 일부 검증만 수행하고 나머지는 서비스에서 수행하게 되어 관심사 분리가 명확하지 않음
권장사항: roadType과 difficulty도 컨트롤러에서 enum으로 변환하여 서비스 메서드 시그니처를
create(String userId, List<Coordinate> coordinates, String name, RoadType roadType, Difficulty difficulty)로 변경하는 것을 고려해보세요. 이렇게 하면 서비스는 검증된 도메인 객체만 받게 되어 책임이 더 명확해집니다.PR 목표에서 언급하신 "DTO → 도메인 변환 책임 위치"에 대한 검토 요청사항과 관련하여, 현재 접근 방식에 대한 팀 내 논의를 권장합니다.
90-94: snap 기능 구현이 깔끔합니다.도메인 서비스에 위임하고 application DTO로 변환하는 구조가 적절합니다. 책임 분리가 명확하게 이루어져 있습니다.
backend/src/test/java/coursepick/coursepick/infrastructure/OsrmCoordinateSnapperTest.java (1)
15-152: SnapResult 반환 타입 변경에 따른 테스트 업데이트가 적절합니다.모든 테스트가 새로운
SnapResult반환 타입에 맞게.coordinates()메서드를 사용하도록 일관되게 업데이트되었습니다. 테스트 로직은 동일하게 유지되면서 새로운 API 구조를 올바르게 반영하고 있습니다.backend/src/test/java/coursepick/coursepick/test_util/DatabaseTestUtil.java (1)
39-58: LGTM!새로운 테스트 유틸리티 메서드들이 기존 패턴(
saveCourse,deleteCourses등)과 일관되게 구현되어 있습니다.findUserCourse에서 복합 쿼리를 사용한 조회 방식도 적절합니다.backend/src/main/java/coursepick/coursepick/presentation/AdminWebController.java (1)
146-150: LGTM!
SnapResultAPI 변경에 맞게 업데이트되었습니다. PR 목표에서 이 파일은 이전 PR 오류 수정으로 리뷰 제외 요청이 있었으나, 변경 사항이 새로운SnapResult구조와 일관되게 적용되어 있습니다.backend/src/main/java/coursepick/coursepick/presentation/CourseWebController.java (1)
73-75: DTO → 도메인 변환 위치에 대한 설계 결정 검토PR 목표에서 언급하신 대로, Controller에서
List<CoordinateWebRequest>→List<Coordinate>변환을 수행하여 서비스 계층이 Presentation 계층에 의존하지 않도록 하는 설계는 적절합니다. 이 방식은 계층 간 책임 분리를 명확히 합니다.backend/src/main/java/coursepick/coursepick/presentation/api/CoordinateWebApi.java (1)
1-74: LGTM!OpenAPI 문서화가 잘 되어 있습니다. 보안 요구사항(
BearerAuth), 에러 응답(400, 401), 그리고 요청 예시가 명확하게 정의되어 있어 API 사용자에게 유용한 정보를 제공합니다.backend/src/main/java/coursepick/coursepick/infrastructure/snapper/OsrmCoordinateSnapper.java (2)
28-31: LGTM!좌표가 2개 미만일 때 조기 반환하는 로직이 적절합니다. 스냅핑이 불가능한 경우에도 원본 좌표를 유지하면서 길이를 0으로 설정하는 일관된 에러 처리 패턴을 따르고 있습니다.
56-59: LGTM!외부 서비스(OSRM) 호출 실패 시 원본 좌표를 반환하는 graceful degradation 패턴이 잘 적용되어 있습니다. 사용자 경험을 유지하면서 로그를 통해 문제를 추적할 수 있습니다.
backend/src/main/java/coursepick/coursepick/domain/course/UserCreatedCourse.java
Show resolved
Hide resolved
| @PostMapping("/courses/create") | ||
| public CourseWebResponse create(@UserId String userId, @RequestBody CourseCreateWebRequest courseCreateWebRequest) { |
There was a problem hiding this comment.
🧩 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
@Valid 어노테이션 추가 필요
CourseCreateWebRequest는 @NotNull 검증 어노테이션이 정의되어 있으나, 엔드포인트에 @Valid가 누락되어 유효성 검증이 실행되지 않습니다. AdminWebController의 다른 엔드포인트들(login, snapCoordinates)에서는 @RequestBody @Valid를 함께 사용하고 있으므로 동일한 패턴을 적용해야 합니다.
수정 방법
@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
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @PostMapping("/courses/create") | |
| public CourseWebResponse create(@UserId String userId, @RequestBody CourseCreateWebRequest courseCreateWebRequest) { | |
| @PostMapping("/courses/create") | |
| public CourseWebResponse create(@UserId String userId, @RequestBody @Valid CourseCreateWebRequest courseCreateWebRequest) { |
🤖 Prompt for AI Agents
In
@backend/src/main/java/coursepick/coursepick/presentation/CourseWebController.java
around lines 71 - 72, CourseCreateWebRequest의 필드 제약 어노테이션이 동작하지 않으므로
CourseWebController의 create 메서드 시그니처에 요청 바디 검증을 활성화하도록 @Valid를 추가하세요; 구체적으로
public CourseWebResponse create(@UserId String userId, @RequestBody
CourseCreateWebRequest courseCreateWebRequest) 선언에서 CourseCreateWebRequest 파라미터에
@Valid를 함께 적용해 CourseCreateWebRequest의 @NotNull 등 검증 어노테이션이 트리거되도록 수정하세요 (참조:
CourseWebController.create, CourseCreateWebRequest).
| @Import({GpxTestUtil.class, DatabaseTestUtil.class}) | ||
| @SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.NONE) | ||
| public abstract class AbstractIntegrationTest { | ||
| public abstract class AbstractIntegrationTest extends AbstractMockServerTest { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find all test classes that extend AbstractIntegrationTest
echo "=== Test classes extending AbstractIntegrationTest ==="
find . -name "*.java" -type f | xargs grep -l "extends AbstractIntegrationTest" 2>/dev/null | head -20
echo -e "\n=== Count of such classes ==="
find . -name "*.java" -type f | xargs grep -l "extends AbstractIntegrationTest" 2>/dev/null | wc -lRepository: woowacourse-teams/2025-course-pick
Length of output: 343
🏁 Script executed:
#!/bin/bash
# Check which test classes actually use MockServer functionality
echo "=== Integration test files using MockServer-related code ==="
find . -name "*Test.java" -type f -path "*/test/*" | xargs grep -l "mockServer\|MockServerClient\|MockServer" 2>/dev/null | head -20
echo -e "\n=== Count of test files using MockServer ==="
find . -name "*Test.java" -type f -path "*/test/*" | xargs grep -l "mockServer\|MockServerClient\|MockServer" 2>/dev/null | wc -lRepository: woowacourse-teams/2025-course-pick
Length of output: 622
🏁 Script executed:
#!/bin/bash
# Examine AbstractMockServerTest to understand initialization overhead
echo "=== AbstractMockServerTest contents ==="
find . -name "AbstractMockServerTest.java" -type f | head -1 | xargs cat
echo -e "\n=== AbstractIntegrationTest contents ==="
find . -name "AbstractIntegrationTest.java" -type f | head -1 | xargs catRepository: woowacourse-teams/2025-course-pick
Length of output: 2428
🏁 Script executed:
#!/bin/bash
# Check entity FK relationships
echo "=== Looking for entity relationship definitions ==="
find . -name "*.java" -type f -path "*/entity/*" | grep -E "(User|Course|UserCreatedCourse)" | head -10Repository: woowacourse-teams/2025-course-pick
Length of output: 129
🏁 Script executed:
#!/bin/bash
# Check if the 2 AbstractIntegrationTest subclasses actually use MockServer
echo "=== CourseApplicationServiceTest ==="
cat ./backend/src/test/java/coursepick/coursepick/application/CourseApplicationServiceTest.java
echo -e "\n=== CourseRepositoryTest ==="
cat ./backend/src/test/java/coursepick/coursepick/domain/course/CourseRepositoryTest.javaRepository: woowacourse-teams/2025-course-pick
Length of output: 16241
🏁 Script executed:
#!/bin/bash
# Check what the MockServer-using tests extend
echo "=== KakaoOauthProviderTest extends ==="
grep -A 2 "public class KakaoOauthProviderTest" ./backend/src/test/java/coursepick/coursepick/infrastructure/KakaoOauthProviderTest.java | head -5
echo -e "\n=== OsrmRouteFinderTest extends ==="
grep -A 2 "public class OsrmRouteFinderTest" ./backend/src/test/java/coursepick/coursepick/infrastructure/OsrmRouteFinderTest.java | head -5
echo -e "\n=== OsrmCoordinateSnapperTest extends ==="
grep -A 2 "public class OsrmCoordinateSnapperTest" ./backend/src/test/java/coursepick/coursepick/infrastructure/OsrmCoordinateSnapperTest.java | head -5Repository: woowacourse-teams/2025-course-pick
Length of output: 196
🏁 Script executed:
#!/bin/bash
# Check full class declaration for the MockServer tests
echo "=== KakaoOauthProviderTest full declaration ==="
head -20 ./backend/src/test/java/coursepick/coursepick/infrastructure/KakaoOauthProviderTest.java | grep -E "class|extends"
echo -e "\n=== OsrmRouteFinderTest full declaration ==="
head -20 ./backend/src/test/java/coursepick/coursepick/infrastructure/OsrmRouteFinderTest.java | grep -E "class|extends"
echo -e "\n=== OsrmCoordinateSnapperTest full declaration ==="
head -20 ./backend/src/test/java/coursepick/coursepick/infrastructure/OsrmCoordinateSnapperTest.java | grep -E "class|extends"Repository: woowacourse-teams/2025-course-pick
Length of output: 409
🏁 Script executed:
#!/bin/bash
# Check if CourseRepositoryTest actually uses MockServer methods
echo "=== Searching for MockServer usage in CourseRepositoryTest ==="
grep -i "mock\|mockwebserver\|mockserver" ./backend/src/test/java/coursepick/coursepick/domain/course/CourseRepositoryTest.java && echo "Found MockServer usage" || echo "No MockServer usage found"
echo -e "\n=== Searching for MockServer usage in CourseApplicationServiceTest ==="
grep -i "mockwebserver\|MockServerClient" ./backend/src/test/java/coursepick/coursepick/application/CourseApplicationServiceTest.java && echo "Found MockWebServer/MockServerClient" || echo "Not found (uses mock() method only)"Repository: woowacourse-teams/2025-course-pick
Length of output: 274
MockServer 상속으로 인한 불필요한 초기화 오버헤드 확인됨.
AbstractIntegrationTest가 AbstractMockServerTest를 상속하면서 모든 통합 테스트가 MockWebServer 초기화 오버헤드를 받고 있습니다. 특히 CourseRepositoryTest는 MockServer 기능을 전혀 사용하지 않음에도 불구하고 매 테스트 메서드마다 MockWebServer를 시작/종료하고 있습니다. 반면 CourseApplicationServiceTest는 단 1개의 테스트 메서드에서만 mock() 메서드를 사용합니다.
권장사항: MockServer가 필요한 테스트만 별도의 추상 클래스(예: AbstractMockServerIntegrationTest)를 통해 상속받도록 분리하세요:
AbstractIntegrationTest→ MockServer 없는 기본 통합 테스트CourseRepositoryTest→AbstractIntegrationTest상속 유지 (MockServer 불필요)CourseApplicationServiceTest→AbstractMockServerIntegrationTest상속 변경 (MockServer 필요)
현재 설계에서는 불필요한 초기화 비용이 발생하고 있습니다.
🤖 Prompt for AI Agents
In
@backend/src/test/java/coursepick/coursepick/test_util/AbstractIntegrationTest.java
at line 10, AbstractIntegrationTest currently extends AbstractMockServerTest
causing every integration test to pay MockWebServer startup/teardown overhead;
change the inheritance so AbstractIntegrationTest no longer extends
AbstractMockServerTest, create a new AbstractMockServerIntegrationTest that
extends AbstractMockServerTest (and contains the mock server setup/teardown),
update CourseRepositoryTest to extend AbstractIntegrationTest (no mock server)
and update CourseApplicationServiceTest to extend
AbstractMockServerIntegrationTest (since it uses mock()), and ensure only tests
that call mock() or rely on MockWebServer inherit from the mock-enabled abstract
class.
| @EnableWebMvc | ||
| @AutoConfigureMockMvc | ||
| public class AbstractSecurityTest { | ||
| public abstract class AbstractSecurityTest { |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
네이밍 컨벤션에 맞게 추상 클래스로 변경한 점 좋습니다!
클래스명이 Abstract로 시작하는데 구체 클래스였던 것을 추상 클래스로 변경하여 의도를 명확히 했습니다. 이를 통해 직접 인스턴스화를 방지하고, 테스트 베이스 클래스로만 사용되도록 강제할 수 있습니다.
| @SecurityScheme( | ||
| name = "BearerAuth", | ||
| type = SecuritySchemeType.HTTP, | ||
| scheme = "bearer", | ||
| bearerFormat = "JWT", | ||
| description = "JWT 토큰을 입력하세요. 'Bearer ' 접두사는 자동으로 추가됩니다." | ||
| ) |
There was a problem hiding this comment.
해당 부분이 인증에 대한 스키마를 지정하는 것입니다.
아까 사진에서 보신 것처럼 Security: Bearer Auth 블럭을 만드는 부분이라고 생각하시면 됩니다.
| @Operation( | ||
| summary = "사용자 코스 추가", | ||
| description = "사용자가 직접 새로운 러닝 코스를 생성합니다. 좌표 목록, 코스명, 도로 타입, 난이도를 입력받아 코스를 저장하고, 사용자와의 소유 관계를 생성합니다.", | ||
| security = @SecurityRequirement(name = "BearerAuth") |
There was a problem hiding this comment.
인증이 필요한 경우 @Operation 내 security 속성을 다음과 같이 지정하면 끝입니다!
dompoo
left a comment
dompoo
left a comment
There was a problem hiding this comment.
PR이 너무 커서, 일단 보이는 것만 빠르게 남겼습니다.
리뷰 코멘트는 계속해서 추가될 것 같아요!
| @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; | ||
|
|
||
| public UserCreatedCourse(String userId, String courseId, boolean isPublic) { | ||
| this(null, userId, courseId, isPublic); | ||
| } | ||
| } |
There was a problem hiding this comment.
이 데이터들을 Course에 추가하지 않으신 이유가 '소유'의 관계가 Course의 것이 아니라고 하셨는데요.
그러면 그냥 유저가 생성한 코스에만 user_id가 존재하고, 관리자에 의해 생성된 코스에는 user_id가 없어도 되지 않을까요?
소유의 관계가 Course의 것이 아니라고 해도, isPublic은 너무나 Course 것 같기도 하고요.
또 반대로 User 쪽에 추가하는 것도 괜찮을 것 같은데, 이것은 어떨까요?
There was a problem hiding this comment.
어떤 Course를 응답할 때, 그냥 그 Course만 조회하는게 아니라
- Course와 연관된 UserCreatedCourse가 있는지 전체 다 뒤진다.
- 없으면 public이다. (맞나요? 헷갈립니다)
- 있으면 UserCreatedCourse의 isPublic에 따른다.
라는 복잡한 규칙이 생기네요.
There was a problem hiding this comment.
그러면 그냥 유저가 생성한 코스에만 user_id가 존재하고, 관리자에 의해 생성된 코스에는 user_id가 없어도 되지 않을까요?
저도 처음에 그렇게 생각했는데요. 즐겨찾기 기능이 생기면 이것도 어떻게 설계를 풀어내느냐에 따라 좀 다르겠지만 현재 구조라면 UserCreatedCourse처럼 UserFavoriteCourse와 같은 중간 객체를 하나 더 만들면 됩니다. 오히려 확장성면에서 좋다고 생각했어요.
하지만, 확실히 돔푸가 말한 응답면에서는 단점이 명확하긴 하네요.
짧게 해결 방안을 생각했을 때는 즐겨찾기까지 고려하여 isPublic 정도만 Course에 넘겨주는 방식은 어떤가요? 단점도 해결되고, 확장성도 그대로 유지될 거 같긴합니다.
There was a problem hiding this comment.
네! 일단 그렇게 구현하면 두 장점 모두 적절히 챙기는 것 같아서 좋은 것 같아요.
추후에 시간이 되시면 DocumentDB에서 관계를 어떻게 표현해야 하는지 좀 더 고민해보면 좋을 것 같습니다. 모델을 나누는 방식은 좀 더 RDB스럽다고 생각했어요. DocumentDB를 이왕 쓰고 있으니 데이터를 중복해서 임베딩 해두는게 기초가 되는 설계라고 생각합니다.
There was a problem hiding this comment.
아, 제가 너무 도메인에만 집중했던 거 같습니다.
DocumentDB 특성도 고려해야 했던 거 같아요.
이 지점이 저에겐 도메인 vs 데이터베이스 성능 트레이드 오프 지점 같습니다.
해당 코멘트를 계기로 다시 한번 MongoDB에 대해서 학습을 해봤어요.
조회의 경우에 돔푸가 말한 것처럼 다음과 같은 상황이 생길 수 있고,
- Course와 연관된 UserCreatedCourse가 있는지 전체 다 뒤진다.
- 없으면 public이다. (맞나요? 헷갈립니다)
- 있으면 UserCreatedCourse의 isPublic에 따른다.
또, 추가로 유저가 자신만의 코스를 조회하는 상황에서도 이렇게 조회가 될 거 같아요.
- User를 찾고,
- UserCreatedCourse에서 userId를 통해 찾는다.
- 그리고 받아온 List-UserCreatedCourse에 있는 courseId를 통해 코스를 조회한 후 응답
혹은 간단한 $lookup 같은 연산을 통해서 찾을 수도 있겠죠?
하지만, 코스 추가 기능의 특성이
- 조회가 많고
- 쓰기가 적으며
- userId에 대한 업데이트가 없다는 점
- MongoDB의 기본적인 철학이 '역정규화'인 것
- 데이터 중첩, 중복 저장을 통해 조인 필요성을 줄이는 설계를 권장한다는 점
등을 고려하면 $lookup이나 저런 조회로 인한 다수 쿼리 발생이 코스 하나하나의 데이터가 큰 만큼 애플리케이션 성능에 꽤나 문제를 일으킬 것 같습니다. 또한, MongoDB를 '잘' 사용하는 것이 아니라는 점도 들 수 있겠네요.
이와 관련해서 MongoDB Docs에도 기준을 만들어 놨더라구요.
네.. 그래서 결론은 MongoDB에 맞게 설계하고, 성능이나 복잡성을 생각하면 Embed 하는게 좋다고 생각합니다. 다만 User 자체를 Embed 하기보다는 CreatorInfo 같은 객체를 만들어 넣어주는 것이 좋을 거 같네요. ㅎㅎ
쓰다보니 코멘트가 길어졌는데, 결론에 대한 돔푸의 의견도 궁금합니다.
backend/src/main/java/coursepick/coursepick/infrastructure/snapper/OsrmCoordinateSnapper.java
Show resolved
Hide resolved
| if (coordinates.size() < 2) { | ||
| return coordinates; | ||
| return new SnapResult(coordinates, 0); | ||
| } |
There was a problem hiding this comment.
예외 상황에서 null도 아니고, 예외도 아니고, 특정 값을 반환하는 형태인데, 저는 직관적이지 않은 것 같아요.
어떻게 생각하시나요?
There was a problem hiding this comment.
해당 로직이 코스 보정 기능을 구현하면서 만들어졌던 거 였는데요.
프론트 단에서 2개 이하로 호출하면 그냥 보정이 안된 것처럼 보이도록 하기 위해 이렇게 했던 거 같아요. 근데, 지금 생각해보니 돔푸 의견이 더 자연스러운 거 같네요.
There was a problem hiding this comment.
오잉 적용하신건가요? ㅋㅋㅋ 관련된 커밋이 보이지 않습니다.
There was a problem hiding this comment.
깜빡하고 푸시를 안했네요~
backend/src/main/java/coursepick/coursepick/presentation/api/CoordinateWebApi.java
Show resolved
Hide resolved
| public abstract class AbstractIntegrationTest extends AbstractMockServerTest { | ||
|
|
||
| @Autowired | ||
| protected DatabaseTestUtil dbUtil; | ||
|
|
||
| @AfterEach | ||
| void tearDown() { | ||
| dbUtil.deleteUserCreatedCourses(); | ||
| dbUtil.deleteCourses(); | ||
| dbUtil.deleteUsers(); | ||
| } | ||
| } |
There was a problem hiding this comment.
모든 통합테스트가 목 서버를 사용하는 것이 아닌데, 통합테스트가 목서버테스트를 상속받아도 될까요? 좀 더 유연하게 목 서버를 사용하고 싶을 때만 사용할 수 있는 유연한 방법이 없을까요?
There was a problem hiding this comment.
모든 통합테스트가 목 서버를 사용하는 것이 아닌데, 통합테스트가 목서버테스트를 상속받아도 될까요?
저도 이 부분에 대해서 우려했는데요. 그래서 생각한 방안이 본문에 적힌 것들이 있었습니다. 근데 지금 생각난건데, 저 방법 말고도 Helper 클래스로 분리해서 필드를 통해 Mock 서버가 관리되도록 할 수 있을 거 같습니다. 돔푸 생각은 어떤가요?
- MockServerHelper를 통해 필드 주입하여 사용하도록 한다.
- 해당 방법은 사용 전 전처리 과정을 개발자가 직접 해야할 것 같습니다.
- 예를 들면 코드에 이 부분을 사용 전, 후로 호출하는 식의 변화가 있을 것 같네여.
@BeforeEach
void setUp() throws IOException {
mockWebServer = new MockWebServer();
mockWebServer.start();
}
@AfterEach
void tearDown() {
mockWebServer.close();
}- 현재 구현 방식으로 둔다.
- 돔푸 의견대로 모든 통합 테스트가 목 서버를 사용하지 않는다는 단점이 존재합니다.
There was a problem hiding this comment.
음 지금 보니까 테스트 구조로 풀어낼 수 있을 것 같아요.
OsrmSnapper 관련된 단위테스트를 MockServerTest 를 상속받아서 구현하고,
ApplicationService 관련된 통합테스트에서는 그대로 IntegrationTest 를 상속받되, OsrmSnapper를 목으로 대체하는 것은 어떨까요?
backend/src/main/java/coursepick/coursepick/domain/course/SnapResult.java
Show resolved
Hide resolved
| public CourseResponse create(String userId, List<Coordinate> coordinates, String name, String roadType, String difficulty) { | ||
| User user = userRepository.findById(userId) | ||
| .orElseThrow(() -> NOT_EXIST_USER.create(userId)); | ||
|
|
||
| Course course = Course.createFromUser(coordinates, name, getRoadType(roadType), getDifficulty(difficulty)); | ||
| Course savedCourse = courseRepository.save(course); | ||
|
|
||
| UserCreatedCourse userCreatedCourse = new UserCreatedCourse(user.id(), savedCourse.id(), false); | ||
| userCreatedCourseRepository.save(userCreatedCourse); | ||
|
|
||
| return CourseResponse.from(savedCourse); | ||
| } |
There was a problem hiding this comment.
난이도를 유저에게 받는 이유가 뭔가요? 저희가 이미 가지고 있는 로직을 사용해도 되지 않나요?
There was a problem hiding this comment.
안드로이드와 회의하면서 정해진 내용인데요, 유저가 직접 코스를 추가한다면 본인이 잘 아는 길이나 뛰어본 경로에 대해서 추가할 가능성이 높다고 생각을 해서 유저가 직접 받도록 했습니다.
그리고 저희가 갖고있는 로직을 사용해도 되지만, 이는 경사도를 기반으로 계산하고 있기 때문에 고도 api가 붙어서 정확한 경사도를 계산할 수 있기전에는 유저를 통해서 받는게 자연스럽다고 생각해서 이렇게 했습니다.
There was a problem hiding this comment.
아하 지금은 고도 정보가 없군요?? 어쩔 수 없겠네요.
추후에 확장되면 난이도가 넘어오면 그거 쓰고, 없으면 고도 기준으로 만들어내도 좋을 것 같네요!
| @Transactional(readOnly = true) | ||
| public SnapResponse snapCoordinates(List<Coordinate> coordinates) { | ||
| SnapResult snapResult = coordinateSnapper.snap(coordinates); | ||
| return new SnapResponse(snapResult.coordinates(), snapResult.length()); | ||
| } |
There was a problem hiding this comment.
이 로직이 CourseApplicationService에 있는게 좀 부자연스러운 것 같아요.
좌표들에 대한 연산이고, Course라는 단어는 보이지 않는데, 여기 있는 이유가 뭔가요?
There was a problem hiding this comment.
저희가 우테코에서 프로젝트를 하면서 초기에 했던 얘기가 생각나는 대목이네요.
MongoDB로 전환을 하면서 가장 큰 이유가 Course라는 큰 도메인 객체에서 Segment, GeoLine, Coordinate이 각자 MySQL에서 말하는 관계를 맺는다기보다는 하나의 큰 틀에서 포함되는 것이다. 그래서 이를 하나의 어떻게보면 애그리거트로 보는 것이 자연스럽다. 라는 느낌으로 결론이 났었던 거 같아요. 맞나요?
그래서 해당 철학을 기반으로 했을 때 Coordinate의 접근은 Course에서 이뤄지는게 자연스럽다고 생각했습니다.
There was a problem hiding this comment.
이것도 수정하려면 너무 커질 것 같아서, 일단 넘어가도 좋을 것 같습니다.
다만 고민해보면 좋을 점이, 모든 연산이 꼭 애그리거트 루트를 통할 필요가 없다는 지점입니다. DDD에서도 그것을 강제하지 않고, 다만 CUD 작업은 그것을 통하라고 말합니다. 애그리거트는 복잡한 시스템에서 데이터 일관성을 유지하는 경계이기 때문입니다.
이런 사실을 통해 제가 생각하기로는,
좌표에 대한 수정은 Course에서 수행하는 것이 맞습니다. 하지만 좌표를 조회하는 것은 별도의 객체에서 수행될 수 있는 거죠!
더구나, 해당 로직의 Coordinate은 Course의 Coordinate과 사뭇 다릅니다. 해당 좌표들이 Course를 구성하지도 않고, 그냥 내부 값만 비슷한 전혀 다른 개념인 거죠.
지금은 전혀 그럴 필요가 없지만, 율무가 DDD를 좋아하시니 이런 지점을 고민해보시면 좀 더 재밌지 않을까 싶습니다 ㅋㅋ
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
backend/src/main/java/coursepick/coursepick/presentation/CoordinateWebController.java (1)
27-32: DTO 변환 위치 결정이 적절하며, 선택적 리팩터링을 고려할 수 있습니다.컨트롤러에서 DTO → 도메인 객체 변환을 수행한 설계 결정은 타당합니다:
- 서비스 계층이 프레젠테이션 계층에 의존하지 않아 클린 아키텍처 원칙을 따릅니다
CourseWebController(lines 59-61)와 패턴이 일관되게 유지됩니다다만, 동일한 좌표 변환 로직이 여러 컨트롤러에 중복됩니다. DRY 원칙 관점에서 선택적으로 공통 유틸리티나 매퍼로 추출할 수 있습니다.
♻️ 선택적 리팩터링: 좌표 변환 로직 공통화
좌표 DTO 리스트를 도메인 객체로 변환하는 헬퍼 메서드를 추가할 수 있습니다:
// CoordinateMapper.java (새 유틸 클래스) public class CoordinateMapper { public static List<Coordinate> toDomain(List<CoordinateDto> dtos) { return dtos.stream() .map(dto -> new Coordinate(dto.latitude(), dto.longitude())) .toList(); } }그런 다음 컨트롤러에서:
- List<Coordinate> coordinates = snapWebRequest.coordinates().stream() - .map(dto -> new Coordinate(dto.latitude(), dto.longitude())) - .toList(); + List<Coordinate> coordinates = CoordinateMapper.toDomain(snapWebRequest.coordinates());참고: 현재 코드도 충분히 명확하므로 이는 선택적 개선사항입니다.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
backend/src/main/java/coursepick/coursepick/presentation/CoordinateWebController.java
🧰 Additional context used
🧬 Code graph analysis (1)
backend/src/main/java/coursepick/coursepick/presentation/CoordinateWebController.java (2)
backend/src/main/java/coursepick/coursepick/application/dto/CourseResponse.java (1)
RequiredArgsConstructor(12-46)backend/src/main/java/coursepick/coursepick/presentation/CourseWebController.java (1)
RestController(19-87)
🔇 Additional comments (2)
backend/src/main/java/coursepick/coursepick/presentation/CoordinateWebController.java (2)
17-21: 컨트롤러 구조가 적절합니다.표준적인 Spring REST 컨트롤러 패턴을 따르고 있으며, 인터페이스 구현과 생성자 주입을 통한 의존성 관리가 잘 되어 있습니다.
23-26: 검증 어노테이션이 없으므로@Valid추가는 불필요합니다.
SnapWebRequest와CoordinateWebRequest모두 JSR-303/380 검증 어노테이션(@NotNull,@NotEmpty등)이 정의되어 있지 않습니다. 따라서@Valid를 추가해도 실제 검증 효과가 없으므로 현재 상태가 적절합니다.Likely an incorrect or invalid review comment.
dompoo
left a comment
dompoo
left a comment
There was a problem hiding this comment.
코멘트가 또 잔뜩 추가되었습니다 ㅋㅋ 이게 마지막일 것 같네요.
| List<Coordinate> coordinates = courseCreateWebRequest.coordinates().stream() | ||
| .map(dto -> new Coordinate(dto.latitude(), dto.longitude())) | ||
| .toList(); | ||
|
|
There was a problem hiding this comment.
DTO → 도메인 변환 책임의 위치
List → List 변환을 Controller에서 수행하도록 했습니다.
배경:
기존에는 서비스에서 원시값을 받아 내부에서 변환
List의 경우 원시값으로 전달하려면 CoordinateDto 또는 변환된 Coordinate를 넘겨야 함
CoordinateDto는 Presentation 계층 소속이므로, 서비스에서 받으면 의존 관계가 역전됨
질문: 이 변환 책임을 Controller에 두는 것이 적절할까요?
-> 이 부분이 돔푸 PR인 코스 조회 API에 필터링 기능 추가에서 만들어주신 CourseFindCondition과 비슷한 맥락입니다.
계층을 정말 지키고 싶다면 따로 DTO를 만들었을 것 같아요.
- Controller에서는 DTO -> DTO 변환
- Service에서는 DTO -> Coordinate 변환
하지만 이러면 공이 너무 들어가는 것 같기도 하고, 큰 의미가 없다고 판단되어서 지금같은 형태가 더 좋아보입니다.
| SnapResult snapResult = coordinateSnapper.snap(coordinates); | ||
| return new SnapResponse(snapResult.coordinates(), snapResult.length()); |
There was a problem hiding this comment.
이거 SnapResult와 SnapReponse를 분리하신 이유가 궁금합니다.
SnapResult를 그대로 응답해도 될 것 같거든요.
| } | ||
|
|
||
| @Transactional | ||
| public CourseResponse create(String userId, List<Coordinate> coordinates, String name, String roadType, String difficulty) { |
There was a problem hiding this comment.
작은건데, 메서드를 CRUD 순으로 정렬하는 것은 어떨까요?
원하는 기능을 찾기가 점점 어려워지는 것 같아요.
| Course course = Course.createFromUser(coordinates, name, getRoadType(roadType), getDifficulty(difficulty)); | ||
| Course savedCourse = courseRepository.save(course); | ||
|
|
||
| UserCreatedCourse userCreatedCourse = new UserCreatedCourse(user.id(), savedCourse.id(), false); | ||
| userCreatedCourseRepository.save(userCreatedCourse); | ||
|
|
There was a problem hiding this comment.
DDD를 좋아하시니까 또 이 부분이 문제가 되는 것 같은데요 ㅋㅋ
UserCreatedCourse는 별도의 애그리거트인가요? 이렇게 따로 저장하는 것이 그것을 표방하고 있는 것 같은데, 어떤가요?
다른 애그리거트라면, UserCreateCourse가 다른 애그리거트가 되어야 하는 이유가 궁금하고, 같은 애그리거트라면 이렇게 두번 저장하는 것이 애플리케이션 서비스에 있어도 될까요?
| private RoadType getRoadType(String roadType) { | ||
| try { | ||
| return RoadType.valueOf(roadType); | ||
| } catch (IllegalArgumentException e) { | ||
| throw INVALID_ROAD_TYPE.create(roadType); | ||
| } | ||
| } | ||
|
|
||
| private Difficulty getDifficulty(String difficulty) { | ||
| try { | ||
| return Difficulty.valueOf(difficulty); | ||
| } catch (IllegalArgumentException e) { | ||
| throw INVALID_DIFFICULTY.create(difficulty); | ||
| } | ||
| } |
| private Course(String name, RoadType roadType, Difficulty difficulty, List<Coordinate> coordinates) { | ||
| this.id = null; | ||
| this.name = new CourseName(name); | ||
| this.roadType = roadType; | ||
| this.segments = refineCoordinates(coordinates); | ||
| this.length = calculateLength(segments); | ||
| this.inclineSummary = InclineSummary.of(segments); | ||
| this.difficulty = difficulty; | ||
| } | ||
|
|
There was a problem hiding this comment.
Difficulty를 따로 받는 것이 Course의 일관성을 망가트릴 수 있다고 생각하는데 어떻게 생각하시나요?
마치 coordinates와 length를 따로 받아서, 실제 좌표와 총 길이가 달라지는 것과 비슷한 이치로, coordinates에 의해 결정되는 값들이 이런 방식이면 일관적이지 않은 것 같아요.
| public static Course createFromUser(List<Coordinate> coordinates, String name, RoadType roadType, Difficulty difficulty) { | ||
| return new Course(name, roadType, difficulty, coordinates); | ||
| } | ||
|
|
There was a problem hiding this comment.
파라미터 순서와 사용 순서를 통일 시키면 더 읽기 쉬워집니다!
| @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; | ||
|
|
There was a problem hiding this comment.
UserCreatedCourse가 id로 구분될 필요가 있을까요?
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
@backend/src/main/java/coursepick/coursepick/infrastructure/snapper/OsrmCoordinateSnapper.java:
- Around line 65-83: In parseSnapResponse, avoid casting distance directly to
Double (which can throw ClassCastException when OSRM returns an integer);
instead retrieve distance as a Number from matchings.get(0).get("distance") and
call doubleValue() (e.g., Number lengthNum = (Number) ...; double length =
lengthNum != null ? lengthNum.doubleValue() : 0.0) so valid numeric responses
aren’t treated as failures; apply this change inside
OsrmCoordinateSnapper.parseSnapResponse and keep the existing exception fallback
behavior.
- Around line 57-62: The code currently logs the full OSRM response at info
level in OsrmCoordinateSnapper (log.info("[OSRM Snap 결과] {}", response)) which
may expose location PII and bloat logs; change that to log.debug for the full
response and replace the info-level log with a concise summary (e.g., number of
matched points, total distance or snappedCount) before calling
parseSnapResponse(response, coordinates); ensure exception handling still
returns new SnapResult(coordinates, 0) and keep parseSnapResponse usage
unchanged.
🧹 Nitpick comments (2)
backend/src/main/java/coursepick/coursepick/application/exception/ErrorType.java (1)
28-35: 에러 메시지의 허용값 문자열을 “단일 소스”로 유지하면 좋아요.현재는 허용값 목록이 문자열로 박혀 있어(예: “[트랙, 트레일, …]”) 도메인 enum/검증 로직과 불일치가 나기 쉽습니다. 가능하면 enum(또는 상수)에서 허용값을 조합해 메시지를 만들거나, 최소한 동일 상수를 재사용하는 방향을 고려해주세요.
backend/src/main/java/coursepick/coursepick/infrastructure/snapper/OsrmCoordinateSnapper.java (1)
3-6: static wildcard import는 여기서는 과하지 않아서 무난합니다.다만 팀 컨벤션이 “wildcard 금지”라면
INVALID_SNAP_COORDINATE_SIZE만 명시 import 하는 쪽이 더 읽기 쉬울 수는 있어요.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
backend/src/main/java/coursepick/coursepick/application/exception/ErrorType.javabackend/src/main/java/coursepick/coursepick/infrastructure/snapper/OsrmCoordinateSnapper.java
🧰 Additional context used
🧬 Code graph analysis (1)
backend/src/main/java/coursepick/coursepick/infrastructure/snapper/OsrmCoordinateSnapper.java (1)
backend/src/main/java/coursepick/coursepick/logging/LogContent.java (1)
LogContent(15-85)
🔇 Additional comments (3)
backend/src/main/java/coursepick/coursepick/application/exception/ErrorType.java (2)
60-63: INVALID_SNAP_COORDINATE_SIZE 추가는 동작 변경(예외)과 잘 맞습니다.포맷 문자열이 아니라서
create()를 인자 없이 호출하는 것도 안전합니다. (placeholder 없음)
68-71: NOT_EXIST_USER 예외 타입/메시지 포맷 일관성 좋습니다.
NoSuchElementException+유저id=%s형태가 다른 NOT_EXIST_* 계열과도 톤이 맞습니다.backend/src/main/java/coursepick/coursepick/infrastructure/snapper/OsrmCoordinateSnapper.java (1)
31-34: HTTP 상태코드 매핑이 올바르게 구성되어 있습니다.전역 예외 처리(
WebExceptionHandler)에서IllegalArgumentException을@ExceptionHandler로 처리하고 있으며,ResponseEntity.badRequest()를 통해 HTTP 400으로 응답합니다.INVALID_SNAP_COORDINATE_SIZE가IllegalArgumentException을 생성하므로 API 계약이 명확하고, 예외 매핑도 정상 작동합니다.
| 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); | ||
| } |
There was a problem hiding this comment.
(prod 포함) OSRM 응답 전체 info 로깅은 위치정보/로그 용량 측면에서 위험합니다.
response에는 좌표/geometry가 포함될 수 있어 개인정보(위치) 성격의 데이터가 로그로 남을 가능성이 큽니다. 최소한 debug로 내리거나, 건수/거리 등 요약 필드만 로깅하는 형태를 권장합니다.
제안 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
In
@backend/src/main/java/coursepick/coursepick/infrastructure/snapper/OsrmCoordinateSnapper.java
around lines 57 - 62, The code currently logs the full OSRM response at info
level in OsrmCoordinateSnapper (log.info("[OSRM Snap 결과] {}", response)) which
may expose location PII and bloat logs; change that to log.debug for the full
response and replace the info-level log with a concise summary (e.g., number of
matched points, total distance or snappedCount) before calling
parseSnapResponse(response, coordinates); ensure exception handling still
returns new SnapResult(coordinates, 0) and keep parseSnapResponse usage
unchanged.
| 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 -> createCoordinateWithElevation( | ||
| new Coordinate(coord.get(1), coord.get(0)), | ||
| originals) | ||
| ) | ||
| .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); | ||
| } |
There was a problem hiding this comment.
distance 타입 캐스팅은 런타임 실패 가능성이 있어 Number 기반으로 파싱하는 게 안전합니다.
OSRM의 distance가 정수로 내려오면 (Double) 캐스팅으로 ClassCastException이 나서(현재는 catch로 length=0 처리) 정상 응답도 “침묵 실패”가 될 수 있습니다. Number#doubleValue()로 처리하는 쪽이 안전합니다.
제안 diff (Number 기반 파싱)
- Double length = (Double) matchings.get(0).get("distance");
+ Number lengthNumber = (Number) matchings.get(0).get("distance");
+ double length = (lengthNumber == null) ? 0.0 : lengthNumber.doubleValue();
@@
- return new SnapResult(snappedCoordinates, length);
+ return new SnapResult(snappedCoordinates, length);🤖 Prompt for AI Agents
In
@backend/src/main/java/coursepick/coursepick/infrastructure/snapper/OsrmCoordinateSnapper.java
around lines 65 - 83, In parseSnapResponse, avoid casting distance directly to
Double (which can throw ClassCastException when OSRM returns an integer);
instead retrieve distance as a Number from matchings.get(0).get("distance") and
call doubleValue() (e.g., Number lengthNum = (Number) ...; double length =
lengthNum != null ? lengthNum.doubleValue() : 0.0) so valid numeric responses
aren’t treated as failures; apply this change inside
OsrmCoordinateSnapper.parseSnapResponse and keep the existing exception fallback
behavior.
# Conflicts: # backend/src/main/java/coursepick/coursepick/application/CourseApplicationService.java # backend/src/main/java/coursepick/coursepick/domain/course/Course.java # backend/src/main/java/coursepick/coursepick/infrastructure/snapper/OsrmCoordinateSnapper.java # backend/src/main/java/coursepick/coursepick/presentation/api/OpenApiConfig.java # backend/src/test/java/coursepick/coursepick/application/CourseApplicationServiceTest.java # backend/src/test/java/coursepick/coursepick/infrastructure/OsrmCoordinateSnapperTest.java
2 participants
🛠️ 설명
CourseApplicationService변경이라서 보는데 오랜 시간이 걸리지는 않을 거 같습니다!코스 추가 기능 설계 고민
❌ Course에 user_id 컬럼 추가
✅ 중간 테이블 도입
/osrm/~에서 영감을 받았습니다.이외에도 싸인 관련 테스트 유틸 중
AbstractSecurityTest가 추상 클래스를 의도하셨던 거 같은데, 추상 클래스가 되어있지 않아 수정했습니다. 그리고 문서에도 로그인 기능이 필요함을 명시하는 것이 필요했는데요. 이를 지원하는 어노테이션을 활용했어요. 해당 부분은 따로 코멘트로 남겨놓겠습니다. 아래는 적용된 사진이에요.사진에서 볼 수 있듯이 Request 상단에 Security: Bearer Auth 블럭이 생기고, 우측에는 요청 시 토큰을 넣을 수 있습니다 ㅎㅎ
🔍 리뷰 요청사항
질문: 이 변환 책임을 Controller에 두는 것이 적절할까요?
-> 이 부분이 돔푸 PR인 코스 조회 API에 필터링 기능 추가에서 만들어주신
CourseFindCondition과 비슷한 맥락입니다.선택 이유: 통합 테스트 특성상 Mock 활용 가능성이 높아 상속 구조가 재사용성 측면에서 유리하다고 판단했습니다.
🔗 관련 이슈
참고
아래 파일은 이전 PR 오류 수정 내용이므로 리뷰에서 제외해주세요:
Summary by CodeRabbit
릴리스 노트
새로운 기능
개선사항
✏️ Tip: You can customize this high-level summary in your review settings.