Conversation
|
Caution Review failedThe pull request is closed. Walkthrough코스 상세 로드/저장 DTO가 재구성되고 ContentUrlProvider 의존이 도입되었으며, 학습 목표(CourseStudyGoal) 엔티티·동기화 로직과 CourseDetailModifier가 추가되었습니다. 이미지 업로드 API에 consumes/produces 명시와 인터뷰 응답에 sequence 필드가 추가되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Admin
participant API as AdminCourseServiceImpl
participant Repo as CourseRepository
participant Util as ContentUrlProvider
Admin->>API: loadDetail(courseId)
API->>Repo: findById(courseId)
API->>Util: resolve introduction image URLs
API-->>Admin: AdminCourseDetailLoadResponse (status, introductionImageList, interviewList, courseStudyGoalList, marketingUrlList)
sequenceDiagram
autonumber
actor Admin
participant API as AdminCourseServiceImpl
participant Mod as CourseDetailModifier
participant Domain as Course
Admin->>API: modifyDetail(courseId, AdminCourseDetailSaveRequest)
API->>Mod: CourseDetailModifier.from(request)
API->>Domain: modifyCourseDetail(modifier)
activate Domain
Domain->>Domain: update CourseDetail (marketingUrls)
Domain->>Domain: update introductions (images)
Domain->>Domain: updateCourseStudyGoals(request.courseStudyGoalList)
deactivate Domain
API-->>Admin: 완료
sequenceDiagram
autonumber
actor Client
participant ResourceApi
participant UploadSvc as ImageUploadService
Client->>ResourceApi: POST /image (multipart/form-data)
ResourceApi->>UploadSvc: upload(file)
UploadSvc-->>ResourceApi: ImageIdResponse
ResourceApi-->>Client: 200 application/json (ImageIdResponse)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/main/java/com/unretired/admin/course/api/payload/AdminCourseInfoLoadResponse.java (1)
45-45: 오탈자: “강사 자개소개” → “강사 자기소개”사용자/운영자 노출 문구이므로 즉시 수정 권장합니다.
- @Schema(description = "강사 자개소개") + @Schema(description = "강사 자기소개")src/main/java/com/unretired/core/course/domain/CourseDetail.java (1)
75-84: CourseDetail.modify()의 price(int) 필드 부분 업데이트 시 0으로 초기화되는 치명적 버그 확인
CourseDetail.modify에서 update 유틸을 primitive(int)인 price에 그대로 적용하면, courseDetail.getPrice()가 제공되지 않은 경우 기본값 0이 전달되어 기존 가격이 0으로 덮어써집니다.수정할 위치
- 파일: src/main/java/com/unretired/core/course/domain/CourseDetail.java
- 메서드: public void modify(CourseDetail courseDetail) (라인 75–84)
권장 수정 방향 중 하나를 선택해 주세요:
- A안 (가장 안전): price 타입을 Integer(null 허용) 또는 BigDecimal(null 허용)로 변경하여 null 기반 부분 갱신 패턴을 유지합니다.
- B안 (신속 회피): 이번 PR 범위에서는 modify(...)에서 price 갱신 코드를 제거하고, 가격 변경은 별도 전용 메서드(e.g. modifyPrice) 또는 CourseDetailModifier 흐름에서만 처리하도록 분리합니다.
- C안 (임시): Course.modifyCourseDetail에서 CourseDetail.modify를 호출하지 않고, Modifier가 보유한 필드만 직접 반영하는 로직으로 변경합니다.
임시 회피용 예시(diff):
public void modify(CourseDetail courseDetail) { update(this::setMainTitle, courseDetail.getMainTitle()); update(this::setSubTitle, courseDetail.getSubTitle()); update(this::setDescription, courseDetail.getDescription()); update(this::setBenefit, courseDetail.getBenefit()); update(this::setReason, courseDetail.getReason()); - update(this::setPrice, courseDetail.getPrice()); + // price(int)는 null 체크가 불가하여 부분 수정 시 0으로 리셋될 수 있음. + // 가격 변경은 별도 전용 흐름으로만 처리할 것을 권장. if (courseDetail.getInterviews() != null) { this.interviews.clear(); this.interviews.addAll(courseDetail.getInterviews()); } if (courseDetail.getMarketingUrls() != null) { if (this.marketingUrls == null) this.marketingUrls = new java.util.ArrayList<>(); this.marketingUrls.clear(); this.marketingUrls.addAll(courseDetail.getMarketingUrls()); } if (courseDetail.getCourseStudyGoalList() != null) { this.courseStudyGoalList.clear(); this.courseStudyGoalList.addAll(courseDetail.getCourseStudyGoalList()); } }
🧹 Nitpick comments (30)
src/main/java/com/unretired/admin/course/api/payload/AdminCourseInfoLoadResponse.java (6)
60-63: List 가변성 제거로 방어적 복사 강화현재
new ArrayList<>(...)는 외부에서 참조를 보관하면 여전히 변경 가능성이 있습니다. 불변 복사로 바꾸는 편이 안전합니다.- var marketingUrls = detail.getMarketingUrls() != null - ? new ArrayList<>(detail.getMarketingUrls()) - : new ArrayList<String>(); + var marketingUrls = detail.getMarketingUrls() != null + ? List.copyOf(detail.getMarketingUrls()) + : List.of();
64-67: 이미지 URL 변환부 NPE 가드 추가 권장
courseImage,courseImage.getMain()/getPreview(),creator.getProfile()가 null일 경우 NPE 위험이 있습니다. 관리자 상세 로드 특성상 데이터 편차가 있을 수 있으므로 null 가드를 권장합니다.예시(개념 제안):
- .mainImage(contentUrlProvider.getContentUrlNoDelimiter(courseImage.getMain().getPath())) - .previewImage(contentUrlProvider.getContentUrlNoDelimiter(courseImage.getPreview().getPath())) + .mainImage(courseImage != null && courseImage.getMain() != null + ? contentUrlProvider.getContentUrlNoDelimiter(courseImage.getMain().getPath()) + : null) + .previewImage(courseImage != null && courseImage.getPreview() != null + ? contentUrlProvider.getContentUrlNoDelimiter(courseImage.getPreview().getPath()) + : null) ... - .profile(contentUrlProvider.getContentUrlNoDelimiter(creator.getProfile().getPath())) + .profile(creator.getProfile() != null + ? contentUrlProvider.getContentUrlNoDelimiter(creator.getProfile().getPath()) + : null)Also applies to: 73-73
78-79: teacherBio/teacherTitle 매핑 상식적 불일치 가능성현재
teacherBio <- profile.getJob(),teacherTitle <- profile.getIntroduction()로 보이는데, 일반적으로 “Bio(자기소개)”는 소개문, “Title”은 직함/직무에 해당합니다. 스왑 필요성 검토 바랍니다.제안:
- .teacherBio(profile.getJob()) - .teacherTitle(profile.getIntroduction()) + .teacherTitle(profile.getJob()) + .teacherBio(profile.getIntroduction())
59-59: 로컬 변수명profile과 빌더 세터profile(...)혼동 가능 — 가독성 개선
var profile = creator.getCreatorProfile()와.profile(...)세터가 혼재되어 가독성이 떨어집니다. 변수명을creatorProfile등으로 바꾸면 혼동이 줄어듭니다.- var profile = creator.getCreatorProfile(); + var creatorProfile = creator.getCreatorProfile(); ... - .name(profile.getName()) - .phoneNumber(profile.getPhoneNumber()) - .email(profile.getEmail()) - .birthDate(profile.getBirthDate()) - .teacherBio(profile.getJob()) - .teacherTitle(profile.getIntroduction()) + .name(creatorProfile.getName()) + .phoneNumber(creatorProfile.getPhoneNumber()) + .email(creatorProfile.getEmail()) + .birthDate(creatorProfile.getBirthDate()) + .teacherBio(creatorProfile.getJob()) + .teacherTitle(creatorProfile.getIntroduction())Also applies to: 74-79
55-55: 정적 팩토리 메서드 호출부 검증 완료
rg검색 결과, 호출부는
src/main/java/com/unretired/admin/course/service/AdminCourseServiceImpl.java:130
에 단 한 곳만 존재하며, 모두from(course, contentUrlProvider)시그니처로 정상 반영된 것을 확인했습니다.아키텍처 제언:
- CDN URL 변환 로직을 현재
AdminCourseInfoLoadResponse내부에서 처리하는 대신, 전용 Mapper/Assembler (예:AdminCourseInfoMapper)로 분리하면 응집도가 높아지고 단위 테스트 작성이 수월해질 것입니다.
52-54: Swagger 스키마 개선 및 필드명 통일 제안프로젝트 내 Admin API 페이로드 간 마케팅 URL 필드명이 불일치하고, Swagger 설명이 배열임을 명시하지 않아 가독성이 떨어집니다. 아래 두 파일을 수정해 주세요.
- src/main/java/com/unretired/admin/course/api/payload/AdminCourseInfoLoadResponse.java
•marketingUrls필드의 @Schema 설명을 “마케팅 URL 목록”으로 변경하고, 배열 예시를 추가- src/main/java/com/unretired/admin/course/api/payload/AdminCourseDetailLoadResponse.java
• 필드명marketingUrlList→marketingUrls로 통일
• @Schema 설명을 “마케팅 URL 목록”으로 변경적용 예시:
--- a/src/main/java/com/unretired/admin/course/api/payload/AdminCourseInfoLoadResponse.java @@ - @Schema(description = "마케팅 주소") - List<String> marketingUrls + @Schema( + description = "마케팅 URL 목록", + example = "[\"https://cdn.unretired.co.kr/mkt/a\",\"https://cdn.unretired.co.kr/mkt/b\"]" + ) + List<String> marketingUrls--- a/src/main/java/com/unretired/admin/course/api/payload/AdminCourseDetailLoadResponse.java @@ - @Schema(description = "마케팅 주소") - List<String> marketingUrlList + @Schema(description = "마케팅 URL 목록") + List<String> marketingUrls설정 변경 후 JSON 직렬화, 테스트 코드(
AdminCourseDetailLoadResponseTest등) 및 매핑 로직에서 필드명을 함께 업데이트해야 합니다.src/main/java/com/unretired/core/resource/api/ResourceApi.java (1)
23-27: 201 Created 응답 및 파일 유효성/콘텐츠 타입 검증을 추가하면 운영 안정성이 높아집니다
- 업로드 성공 시 200 OK 대신 201 Created를 쓰면 REST 의미론에 부합합니다.
- 빈 파일/미지원 MIME 타입을 조기에 차단하면 서비스 단의 예외/리소스 낭비를 줄일 수 있습니다.
아래와 같이 보완을 제안드립니다.
- imports (선택): 파일 상단 import 추가
import org.springframework.http.HttpStatus; import org.springframework.web.bind.annotation.ResponseStatus; import org.springframework.web.server.ResponseStatusException; import java.util.Set;
- 변경 포인트 diff:
@PostMapping( value = "/image", consumes = MediaType.MULTIPART_FORM_DATA_VALUE, produces = MediaType.APPLICATION_JSON_VALUE ) +@ResponseStatus(HttpStatus.CREATED) public ImageIdResponse upload( @Auth User user, @RequestPart("image") MultipartFile file ) { - var image = imageUploadService.upload(file); + if (file == null || file.isEmpty()) { + throw new ResponseStatusException(HttpStatus.BAD_REQUEST, "Empty image file"); + } + var contentType = file.getContentType(); + var allowed = Set.of("image/jpeg", "image/png", "image/webp"); + if (contentType != null && !allowed.contains(contentType)) { + throw new ResponseStatusException(HttpStatus.UNSUPPORTED_MEDIA_TYPE, "Unsupported image type: " + contentType); + } + var image = imageUploadService.upload(file); return new ImageIdResponse(image.getFileId(), image.getFileType()); }Also applies to: 28-34
src/main/java/com/unretired/admin/course/api/payload/AdminCourseStudyGoalResponse.java (1)
3-7: DTO 명명/중복 정리 제안: AdminCourseDetailStudyGoal과 구조 중복
- 동일 PR에
AdminCourseDetailStudyGoal(동일 필드 구조)이 존재합니다. 로드/세이브/상세 등 용도에 따라 이름이 갈리는 경우가 많지만, 구조가 동일하면 단일 타입으로 통일하거나 접미사를 Request/Response로만 구분하는 편이 유지보수에 유리합니다.- 제안:
- 로드용:
AdminCourseStudyGoalResponse- 저장용:
AdminCourseStudyGoalRequest- 별도 목적이 없다면
AdminCourseDetailStudyGoal는 제거/대체원하시면 호출부 영향도 점검 스크립트를 드리겠습니다.
src/main/java/com/unretired/core/course/domain/CourseInfoModifier.java (1)
43-49: 접근자 호출과 필드 직접 참조 혼용 — 일관성 정리 제안
- 같은 메서드에서
mainTitle()(accessor)와subTitle/description(필드)을 혼용하고 있습니다. record에서는 accessor 사용을 일관화하는 편이 가독성에 좋습니다.return CourseDetail.builder() - .mainTitle(mainTitle()) - .subTitle(subTitle) - .description(description) + .mainTitle(mainTitle()) + .subTitle(subTitle()) + .description(description()) .build();src/main/java/com/unretired/admin/course/api/payload/AdminCourseDetailStudyGoal.java (1)
3-7:AdminCourseStudyGoalResponse와의 구조 중복 — 타입 통합 검토
- 필드 구성이 완전히 동일합니다. 불가피한 사유(예: 다른 계층의 직렬화 규약)가 없다면 중복 DTO를 줄여 단일 타입으로 통일하는 편이 안전합니다. 이후 변경 시 누락/불일치 리스크를 줄일 수 있습니다.
src/main/java/com/unretired/core/course/domain/CourseImage.java (3)
58-60: 소개 이미지만 부분 업데이트하는 오버로드 추가 — 방향성 좋음 + 방어적 복사 제안
- introductions에 외부 리스트 참조가 직접 보관되면 호출 측에서 리스트를 변경할 때 예기치 않은 사이드 이펙트가 발생할 수 있습니다. 불변 복사를 권장합니다.
부분 업데이트 메서드만 최소 변경으로 보완하는 diff:
public void modify(List<Image> introductions) { - update(this::setIntroductions, introductions); + // update는 null 입력을 무시하므로, non-null일 때만 불변 복사하여 세팅 + update(l -> this.setIntroductions(List.copyOf(l)), introductions); }추가로, 생성자/전체 modify에서도 동일 방식을 고려해 일관성을 맞추면 더 안전합니다. 필요하시면 전체 적용용 패치도 드리겠습니다.
51-56: NPE 안전성 소폭 향상 제안: null 인자 방어
modify(CourseImage image)가 null을 받을 가능성이 있으면 즉시 리턴 가드가 간단하고 효과적입니다.참고 구현(선택, 변경 라인 밖 지원 코드 예시):
public void modify(CourseImage image) { if (image == null) return; update(this::setMain, image.main); update(this::setPreview, image.preview); update(this::setSub, image.sub); update(this::setIntroductions, image.introductions); }
39-45: 리스트 필드의 불변성 보장 고려
- 생성자에서
this.introductions = introductions;는 외부 참조를 그대로 보관합니다. 대용량/동시성/불변성 요구가 있으면 방어적 복사를 권장합니다.지원 예시(선택, 변경 라인 밖 지원 코드):
this.introductions = introductions == null ? null : List.copyOf(introductions);src/main/java/com/unretired/admin/course/api/payload/AdminCourseStudyGoalRequest.java (2)
7-9: 입력값 검증 어노테이션 추가 권장(@PositiveOrZero, @notblank)sequence가 음수이거나 answer가 빈 문자열로 들어오면 도메인 제약과 불일치할 수 있습니다. 레코드 컴포넌트에 Bean Validation을 붙여 런타임/컨트롤러 레벨에서 조기에 방지하는 편이 안전합니다.
다음 변경을 제안드립니다:
public record AdminCourseStudyGoalRequest( - int sequence, - String answer + @jakarta.validation.constraints.PositiveOrZero int sequence, + @jakarta.validation.constraints.NotBlank String answer ) {
18-24: 널 가드 추가로 계약 강화(Objects.requireNonNull)도메인 불변식을 명시적으로 보장하기 위해 course 파라미터에 널 가드를 추가하세요.
public CourseStudyGoal toCourseStudyGoal(Course course) { + java.util.Objects.requireNonNull(course, "course must not be null"); return CourseStudyGoal.builder() .answer(answer) .sequence(sequence) .course(course) .build(); }src/main/java/com/unretired/core/course/domain/CourseStudyGoal.java (4)
30-31: 답변 길이 대비 컬럼 타입 보강(TEXT/Lob 고려)answer는 서술형 텍스트로 길이가 길 수 있습니다. DB 기본 varchar 길이를 초과할 위험을 줄이기 위해 TEXT(@lob)로 저장하는 편이 안전합니다.
- private String answer; + @jakarta.persistence.Column(columnDefinition = "TEXT") + private String answer;
32-35: 필드 가시성 일관성: course에 private 누락다른 필드는 private인데 course만 패키지 가시성입니다. 캡슐화 일관성을 위해 private로 통일하세요.
- Course course; + private Course course;
18-23: (코스, 시퀀스) 유니크 제약 추가 제안코스별 시퀀스가 중복되면 정렬/표시 로직이 흔들립니다. DB 레벨에서 (course_id, sequence) 유니크 제약을 두면 데이터 정합성을 강하게 보장할 수 있습니다.
-@Table(name = "COURSE_STUDY_GOAL") +@Table( + name = "COURSE_STUDY_GOAL", + uniqueConstraints = @jakarta.persistence.UniqueConstraint( + name = "uk_course_study_goal_course_seq", + columnNames = {"course_id", "sequence"} + ) +)
32-35: 연관관계 매핑 중복 가능성 점검 필요(CourseDetail의 @OneToMany/@joincolumn와 동시 사용 여부)현재 자식 쪽에 @manytoone(fetch = LAZY) @joincolumn(name="course_id", nullable=false)가 있습니다. 동시에 CourseDetail 쪽에서 @onetomany + @joincolumn(name = "course_id")로 같은 FK를 관리한다면, JPA에서 동일 컬럼을 양쪽에서 소유하려는 중복 매핑/소유권 충돌 문제가 발생할 수 있습니다. 일반적으로:
- 자식 쪽(@manytoone)이 FK를 소유,
- 부모 쪽은 @onetomany(mappedBy = "course")로 읽기 전용 관계를 명시
형태가 안전합니다. CourseDetail 매핑을 재확인해 주시고, 필요 시 mappedBy로 전환해 소유권을 단일화하세요.
src/main/java/com/unretired/admin/course/api/payload/AdminCourseDetailSaveRequest.java (2)
11-16: 널 방지를 위한 컬렉션 필수화(@NotNull) 및 입력 유효성 최소 보강입력이 null이면 도메인 로직에서 NPE가 발생할 수 있습니다. 세 리스트에 @NotNull을 부여하고, 서비스 계층에서 빈 리스트로 정규화하는 것을 권장합니다.
public record AdminCourseDetailSaveRequest( - @Schema(description = "세부 이미지 URL") - List<ImageRequest> introductions, - @Schema(description = "학습 목표") - List<AdminCourseStudyGoalRequest> courseStudyGoalList, - @Schema(description = "마케팅 주소") - List<String> marketingUrls + @Schema(description = "세부 이미지 URL") + @jakarta.validation.constraints.NotNull List<ImageRequest> introductions, + @Schema(description = "학습 목표") + @jakarta.validation.constraints.NotNull List<AdminCourseStudyGoalRequest> courseStudyGoalList, + @Schema(description = "마케팅 주소") + @jakarta.validation.constraints.NotNull List<String> marketingUrls ) {추가로, 마케팅 URL의 형식을 엄격히 하려면 Hibernate Validator 사용 시 @org.hibernate.validator.constraints.URL 리스트 요소 제약을 고려해볼 수 있습니다.
12-12: 네이밍 가독성 개선(introductions → introductionImages 등)컬렉션의 의미를 더 명확히 하려면 introductions 대신 introductionImages 또는 introductionImageList 같은 명칭이 의도를 잘 전달합니다. API 소비자 관점에서 직관성이 좋아집니다.
src/main/java/com/unretired/core/course/domain/Course.java (1)
144-149: modifyCourseDetail의 NPE 가능성: goal 리스트가 null일 때 방어 필요courseDetailModifier.courseStudyGoalList()가 null이면 updateCourseStudyGoals 호출에서 NPE 발생 가능성이 있습니다. 메서드 내부에서 null을 빈 리스트로 정규화하거나, 호출 전 방어 로직을 추가하세요.
간단한 방어는 updateCourseStudyGoals 쪽에 추가하는 편이 중복이 줄어듭니다(아래 별도 코멘트의 diff 참조).
src/main/java/com/unretired/core/course/domain/CourseDetail.java (3)
43-46: courseStudyGoalList 갱신 경로 누락 가능성새 필드(List)가 추가되었지만 modify(...)에 반영되지 않아, 이 메서드를 사용하는 기존 경로에서는 학습 목표 변경이 저장되지 않을 수 있습니다. 위의 패치처럼 in-place 갱신을 추가해 주세요. 만약 Course.modifyCourseDetail(...)에서만 학습 목표를 다룬다면, 이 메서드를 더 이상 외부에서 사용하지 않도록 하거나 JavaDoc으로 사용 제약을 문서화하는 것을 권장합니다.
Also applies to: 75-84
55-59: 불필요한 생성자 파라미터 및 import 정리생성자(@builder)에
BigDecimal badgeDiscountRate파라미터가 있으나 대응 필드가 존재하지 않아 혼란을 야기합니다. 사용하지 않는 파라미터와 관련 import(BigDecimal)를 제거해 주세요.- BigDecimal badgeDiscountRate,-import java.math.BigDecimal;Also applies to: 16-16
39-46: 컬렉션 기본값과 Builder 상호작용으로 인한 NPE 가능성필드에 기본값(ArrayList)을 부여했지만, 생성자 @builder를 통해 null이 주입되면 해당 기본값이 덮여져 null이 될 수 있습니다. Lombok @Builder.Default를 필드에 적용(타입 레벨 @builder 사용)하거나, 생성자에서
Objects.requireNonNullElse(...)로 방어 코드를 두는 것을 권장합니다.Also applies to: 47-69
src/main/java/com/unretired/core/course/domain/CourseDetailModifier.java (3)
13-21: 도메인 레이어가 Admin DTO에 직접 의존(레이어링 침식)
CourseDetailModifier(core/domain)가AdminCourseDetailSaveRequest,AdminCourseStudyGoalRequest(admin/api)를 직접 참조합니다. 이는 의존 방향이 역전되어 레이어 결합도를 높입니다.
권장:
- Modifier를 admin 레이어로 이동하거나,
- 도메인 순수 타입(예: CourseStudyGoalCommand 등)으로 변경 후, admin DTO → 도메인 타입 매핑을 admin 레이어에서 수행.
이렇게 하면 도메인 모듈이 admin API 변경의 파급을 받지 않습니다.
31-35: toCourseDetail가 marketingUrls만 설정: modify(...)와 결합 시 가격 0 리셋 트리거 가능현재 toCourseDetail()은 다른 필드를 비워둔 채 CourseDetail 인스턴스를 생성합니다. 이 객체가 CourseDetail.modify(...)로 전달되면 price가 0으로 덮일 수 있습니다(상세는 CourseDetail 코멘트 참고).
해결책:
- Course.modifyCourseDetail(...)에서 CourseDetail.modify(...) 호출을 피하고, Modifier의 필드만 직접 반영.
- 또는 CourseDetail.modify(...)를 Modifier 기반 시그니처로 교체.
단기적으로는 Course.modifyCourseDetail에서 marketingUrls/학습목표만 명시적으로 갱신하는 방식을 권장합니다.
37-42: null 반환 전략 명시
toIntroductionImages()가 introductions==null이면 null을 반환하고, 빈 리스트 요청 시 비우는 시맨틱을 주는 설계는 합리적입니다. 주석 또는 JavaDoc으로 “null=미변경, []=전체 삭제” 계약을 명시해 주세요.src/main/java/com/unretired/admin/course/api/payload/AdminCourseDetailLoadResponse.java (2)
45-49: 소개 이미지 리스트 null 안전성 보강
courseImage.getIntroductions()가 null일 가능성을 방어해 주세요. 안전하게 빈 리스트를 반환하도록 처리하면 NPE 리스크를 줄일 수 있습니다.- var introductionImageList = courseImage.getIntroductions() - .stream() - .map(introduction -> contentUrlProvider.getContentUrlNoDelimiter(introduction.getPath())) - .toList(); + var introductionImageList = + courseImage.getIntroductions() == null ? List.<String>of() + : courseImage.getIntroductions().stream() + .map(introduction -> contentUrlProvider.getContentUrlNoDelimiter(introduction.getPath())) + .toList();
50-57: 학습 목표 리스트 null 안전성 및 방어적 복사
detail.getCourseStudyGoalList()가 null일 때 NPE 위험이 있습니다. 또한 응답 생성 시 불변/방어적 리스트 사용을 권장합니다.- var courseStudyGoalList = course.getDetail().getCourseStudyGoalList() - .stream() - .map(courseStudyGoal -> new AdminCourseStudyGoalResponse( - courseStudyGoal.getSequence(), - courseStudyGoal.getAnswer() - )) - .toList(); + var goals = detail.getCourseStudyGoalList(); + var courseStudyGoalList = goals == null ? List.<AdminCourseStudyGoalResponse>of() + : goals.stream() + .map(courseStudyGoal -> new AdminCourseStudyGoalResponse( + courseStudyGoal.getSequence(), + courseStudyGoal.getAnswer() + )) + .toList();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
src/main/java/com/unretired/admin/course/api/payload/AdminCourseDetailLoadResponse.java(1 hunks)src/main/java/com/unretired/admin/course/api/payload/AdminCourseDetailSaveRequest.java(1 hunks)src/main/java/com/unretired/admin/course/api/payload/AdminCourseDetailStudyGoal.java(1 hunks)src/main/java/com/unretired/admin/course/api/payload/AdminCourseInfoLoadResponse.java(2 hunks)src/main/java/com/unretired/admin/course/api/payload/AdminCourseStudyGoalRequest.java(1 hunks)src/main/java/com/unretired/admin/course/api/payload/AdminCourseStudyGoalResponse.java(1 hunks)src/main/java/com/unretired/admin/course/service/AdminCourseServiceImpl.java(4 hunks)src/main/java/com/unretired/core/course/api/payload/CourseInterviewResponse.java(1 hunks)src/main/java/com/unretired/core/course/domain/Course.java(3 hunks)src/main/java/com/unretired/core/course/domain/CourseDetail.java(4 hunks)src/main/java/com/unretired/core/course/domain/CourseDetailModifier.java(1 hunks)src/main/java/com/unretired/core/course/domain/CourseImage.java(2 hunks)src/main/java/com/unretired/core/course/domain/CourseInfoModifier.java(1 hunks)src/main/java/com/unretired/core/course/domain/CourseStudyGoal.java(1 hunks)src/main/java/com/unretired/core/resource/api/ResourceApi.java(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
src/main/java/com/unretired/core/course/domain/CourseImage.java (1)
src/main/java/com/unretired/global/utils/Functions.java (1)
Functions(6-21)
src/main/java/com/unretired/core/course/domain/Course.java (2)
src/main/java/com/unretired/core/course/domain/CourseStudyGoal.java (1)
Entity(18-47)src/main/java/com/unretired/core/notice/domain/Notice.java (1)
Entity(20-52)
src/main/java/com/unretired/core/course/domain/CourseDetail.java (1)
src/main/java/com/unretired/global/utils/Functions.java (1)
Functions(6-21)
🔇 Additional comments (10)
src/main/java/com/unretired/admin/course/api/payload/AdminCourseInfoLoadResponse.java (2)
7-9: 신규 import 추가 타당 — 사용처 일치
ContentUrlProvider,Schema,Builder모두 실제 사용처가 존재합니다. 불필요 import 없음으로 LGTM입니다.
37-43: 개인정보 필드 노출 범위 재확인(관리자 전용 여부)이 레코드에 이름/휴대폰/이메일/생년월일이 포함됩니다. 관리자 전용 API에서만 사용되는지, 외부 공개 경로가 없는지 점검 바랍니다. 필요 시 마스킹/권한검증 레이어를 고려해 주세요.
src/main/java/com/unretired/core/resource/api/ResourceApi.java (1)
23-27: 명시적 consumes/produces 선언으로 업로드 호환성 개선 — LGTM
- multipart/form-data 소비와 application/json 응답을 명시해 클라이언트/게이트웨이(예: API GW, 프록시) 간 협업성이 좋아졌습니다.
src/main/java/com/unretired/admin/course/api/payload/AdminCourseStudyGoalResponse.java (1)
3-7: 단순/불변 DTO로 적절 — LGTM
- sequence, answer만 포함한 record 형태가 가볍고 명확합니다.
src/main/java/com/unretired/core/course/domain/CourseInfoModifier.java (1)
51-56: toCreatorProfile는 명확하고 깔끔 — LGTM
- record 필드를 그대로 매핑하는 단순 변환 메서드로 적절합니다.
src/main/java/com/unretired/core/course/domain/CourseImage.java (1)
35-37: introductions 컬럼 길이 제한 검토 요청
CourseImage.java(35–37행)의@Column(length = 1000)설정은ImageListConverter로 직렬화된 이미지 메타 리스트가 1000자 이상이 될 수 있는 잠재적 병목입니다. 특히 Base64 인코딩된 데이터나 긴 URL을 저장할 경우 여유가 부족할 수 있으니, 운영 환경 샘플 데이터를 기준으로 반드시 검증해 주세요.검토 사항:
- 운영 DB의
course_image테이블에서introductions컬럼에 실제 저장된 문자열 길이의 최대값 확인ImageListConverter.convertToDatabaseColumn(List<Image>)호출 결과 문자열 길이 측정- 최대 길이가 1000자를 초과한다면,
length값을 상향(예: 2000 이상)하거나TEXT등 가변 길이 타입으로 매핑 변경 검토검증을 위한 샘플 스크립트:
#!/bin/bash # 운영 DB에서 최대 문자열 길이 조회 (예시, PostgreSQL) psql -d your_db -c " SELECT MAX(octet_length(introductions)) FROM course_image; " # 로컬 Java 코드로 변환 로직 출력 길이 측정 python3 - <<EOF from com.unretired.core.unit.domain.ImageListConverter import ImageListConverter # 예시 데이터 로드 images = [...] # 실제 Image 객체 리스트 샘플 s = ImageListConverter().convertToDatabaseColumn(images) print('Serialized length:', len(s)) EOF위 과정을 통해 1000자가 충분한지 확인 후, 필요 시 컬럼 정의를 확장해 주시기 바랍니다.
src/main/java/com/unretired/core/course/api/payload/CourseInterviewResponse.java (1)
16-24: CourseInterviewResponse.sequence는 질문 순서를 나타냅니다—매핑 로직은 올바르나 설명을 명확히 해주세요CourseInterviewQuestion enum의
id(1L, 2L, …)는 질문 순서용 값이며,interview.getQuestion().getId()호출로 순서 정보를 가져오는 것은 의도한 바와 일치합니다. 다만 현재@Schema(description = "인터뷰 번호")로 표기되어 있어 “인터뷰 식별자”로 오해될 소지가 있습니다.– 변경 제안
• 순서 정보를 정확히 설명하도록 애노테이션만 수정- @Schema(description = "인터뷰 번호") - Long sequence + @Schema(description = "질문 순서") + Long sequence• (필요 시) “sequence” 대신 “order” 혹은 “sortOrder” 등으로 필드명을 바꾸는 것도 검토 가능
– 만약 실제로는 질문 식별자(questionId)가 필요하다면, 필드명·매핑 모두 questionId로 변경해야 합니다.
위 두 가지 중 실제 요구사항에 맞게 선택 후 반영 부탁드립니다.
src/main/java/com/unretired/admin/course/service/AdminCourseServiceImpl.java (2)
133-138: ContentUrlProvider를 사용하도록 시그니처 변경 적용 확인
AdminCourseDetailLoadResponse.from(course, contentUrlProvider)로의 변경 반영 좋습니다. 이미지 URL이 CDN 경유로 노출되는 요구사항과 부합합니다.
151-155: Course.modifyCourseDetail 내부 버그 및 orphanRemoval 처리 수동 검증 필요새로운 Detail 수정 플로우(
CourseDetailModifier.from(request)→course.modifyCourseDetail(detailModifier)) 전환은 의도에 부합하지만, 아래 사항을 코드베이스에서 직접 확인해 주세요.
price(int) 리셋 버그 확인
Course.modifyCourseDetail(...)내부에서CourseDetail.modify(...)호출 시, 기존 가격 필드가 의도치 않게 0으로 초기화되거나 변경되지 않는 문제가 재현될 수 있습니다. 해당 메서드 구현부를 열어, 가격 필드(price)가 새 값으로만 설정되고 기존 값이 보존되는지 검증해 주세요.학습 목표 List orphanRemoval 처리
@OneToMany(orphanRemoval = true)로 설정된 학습 목표 리스트를 업데이트할 때,clear()후addAll()방식으로 in-place 수정이 이루어지는지, 그리고 기존에 제거된 엔티티가 실제로 DB에서 삭제되는지 확인이 필요합니다. 수동 테스트 또는 통합 테스트를 통해 orphanRemoval 기능이 정상 작동하는지 검증 부탁드립니다.테스트 커버리지 보강 권장
위 두 시나리오를 포함하는 단위·통합 테스트를 추가하여, 가격 변경과 학습 목표 삭제 로직이 안정적으로 동작함을 증명하는 것이 좋습니다.위 사항을 로컬에서 직접 검토·테스트한 후 문제 없으면 이대로 머지해도 괜찮습니다.
src/main/java/com/unretired/admin/course/api/payload/AdminCourseDetailLoadResponse.java (1)
36-39: 마케팅 URL 복사와 명명 일관성은 적절합니다
marketingUrls를 새 ArrayList로 복사하고, builder에는marketingUrlList로 전달하는 패턴이 명확하며 부작용을 줄입니다. 그대로 유지해도 좋습니다.Also applies to: 58-66
Docstrings generation was requested by @jickDo. * #377 (comment) The following files were modified: * `src/main/java/com/unretired/admin/course/api/payload/AdminCourseDetailLoadResponse.java` * `src/main/java/com/unretired/admin/course/api/payload/AdminCourseInfoLoadResponse.java` * `src/main/java/com/unretired/admin/course/api/payload/AdminCourseStudyGoalRequest.java` * `src/main/java/com/unretired/admin/course/service/AdminCourseServiceImpl.java` * `src/main/java/com/unretired/core/course/api/payload/CourseInterviewResponse.java` * `src/main/java/com/unretired/core/course/domain/Course.java` * `src/main/java/com/unretired/core/course/domain/CourseDetail.java` * `src/main/java/com/unretired/core/course/domain/CourseDetailModifier.java` * `src/main/java/com/unretired/core/course/domain/CourseImage.java` * `src/main/java/com/unretired/core/course/domain/CourseInfoModifier.java` * `src/main/java/com/unretired/core/course/domain/CourseStudyGoal.java` * `src/main/java/com/unretired/core/resource/api/ResourceApi.java`
|
Note Generated docstrings for this pull request at #378 |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/main/java/com/unretired/core/course/domain/CourseInfoModifier.java (1)
51-56: teacherBio/teacherTitle 역매핑 불일치(조회 DTO와 반대 매핑).저장 측(toCreatorProfile)은 job ← teacherTitle, introduction ← teacherBio로 설정하고 있는데, 조회 측(AdminCourseInfoLoadResponse.from)에서는 teacherBio ← profile.getJob(), teacherTitle ← profile.getIntroduction()로 반대로 내려주고 있습니다. 저장/조회가 서로 상쇄되어 관리 화면에서 값이 뒤바뀌어 보일 가능성이 큽니다. 조회 DTO 쪽 매핑을 아래처럼 교정해 주세요.
다음 diff를 AdminCourseInfoLoadResponse에 적용 권장:
- .teacherBio(profile.getJob()) - .teacherTitle(profile.getIntroduction()) + .teacherBio(profile.getIntroduction()) + .teacherTitle(profile.getJob())src/main/java/com/unretired/admin/course/api/payload/AdminCourseInfoLoadResponse.java (1)
78-79: teacherBio/teacherTitle 역매핑 오류.현재 조회 시 teacherBio에 job, teacherTitle에 introduction이 들어가고 있어 필드 의미가 반전됩니다. 저장 로직(CourseInfoModifier.toCreatorProfile)과 대칭되도록 아래처럼 수정해 주세요.
- .teacherBio(profile.getJob()) - .teacherTitle(profile.getIntroduction()) + .teacherBio(profile.getIntroduction()) + .teacherTitle(profile.getJob())src/main/java/com/unretired/core/resource/api/ResourceApi.java (1)
28-34: 업로드 서비스에 파일 유효성 검증 및 악성 콘텐츠 스캔 로직 추가 필요현재
ResourceApi.upload에서 전달된MultipartFile은 컨트롤러에서 인증만 거치고 바로ImageUploadService.upload로 전달되며, 서비스 구현부(AwsImageUploadService.upload)를 확인한 결과 다음과 같은 검증 로직은 없음을 확인했습니다.
- 파일 확장자 또는 MIME 타입 검사
- 최대/최소 파일 크기 제한
- 악성코드(바이러스) 스캔
- 허용(화이트리스트) 또는 차단(블랙리스트) 정책 적용
구체적으로 다음 위치에서 검증 로직이 부재합니다:
src/main/java/com/unretired/admin/resource/service/ImageUploadService.java(인터페이스 선언)src/main/java/com/unretired/admin/resource/service/AwsImageUploadService.java
public Image upload(MultipartFile multipartFile)메서드 (라인 26)위치에 보안·정책 검증을 반드시 추가해주세요.
예시:
multipartFile.getContentType()기반 MIME 타입 화이트리스트 검사multipartFile.getSize()기반 최대 파일 크기 제한 (설정값 사용)- 서드파티 또는 자체 바이러스 스캔 호출
- 허용 가능한 확장자 목록 검증 등
이러한 검증은 서비스 계층에서 꼭 처리되어야 하는 보안상 필수 로직이므로, 구현 후 재검토 부탁드립니다.
♻️ Duplicate comments (4)
src/main/java/com/unretired/admin/course/api/payload/AdminCourseStudyGoalRequest.java (1)
13-19: 과거 영속성 제약 이슈 해소 — course 주입 방식으로 안전해졌습니다이전 커밋에서 지적되었던
course미설정 팩토리 문제를toCourseStudyGoal(Course course)로 해결했습니다. 이제 FK null 제약 위반 위험이 제거되어 안전합니다.src/main/java/com/unretired/core/course/domain/Course.java (2)
3-3: 도메인 → Admin DTO 직접 의존: 레이어 결합도 과함 (기존 지적 반복)
Course엔티티가AdminCourseStudyGoalRequest(admin API DTO)에 직접 의존합니다. 애플리케이션/프레젠테이션 경계에서 도메인 타입(예:CourseStudyGoalCommand/Editor)으로 변환하도록 리팩터링해 주세요. 아래updateCourseStudyGoals(...)의 시그니처도 도메인 전용 타입으로 교체 권장합니다.
221-258: 학습목표 동기화: 인덱스 기반 갱신은 시퀀스 재배열 시 오동작 위험 (기존 지적 반복)현재 로직은 양쪽 리스트를 정렬한 뒤 “같은 인덱스”끼리 매칭합니다. 요청/기존 항목의 시퀀스가 재배치되면 잘못된 항목이 덮어써질 수 있습니다(동일 시퀀스 매칭이 아니라 위치 매칭). 또한 요청 내 시퀀스 중복/누락 검증이 없습니다. 시퀀스 기반 매핑(Map)으로 교체하고, 중복 검증 및 삭제 동기화를 명시적으로 처리해 주세요.
Apply this diff to replace the index-based logic with sequence-keyed sync:
- private void updateCourseStudyGoals(List<AdminCourseStudyGoalRequest> requests) { - List<CourseStudyGoal> existingGoals = this.detail.getCourseStudyGoalList(); - - existingGoals.sort(Comparator.comparingInt(CourseStudyGoal::getSequence)); - - List<AdminCourseStudyGoalRequest> safeRequests = - Optional.ofNullable(requests) - .orElse(Collections.emptyList()) - .stream() - .filter(Objects::nonNull) - .sorted(Comparator.comparingInt(AdminCourseStudyGoalRequest::sequence)) - .toList(); - - for (int i = 0; i < Math.min(existingGoals.size(), safeRequests.size()); i++) { - CourseStudyGoal existing = existingGoals.get(i); - AdminCourseStudyGoalRequest request = safeRequests.get(i); - - existing.modify(CourseStudyGoal.builder() - .sequence(request.sequence()) - .answer(request.answer()) - .build()); - } - - for (int i = existingGoals.size(); i < safeRequests.size(); i++) { - AdminCourseStudyGoalRequest request = safeRequests.get(i); - CourseStudyGoal newGoal = CourseStudyGoal.builder() - .sequence(request.sequence()) - .answer(request.answer()) - .course(this) - .build(); - existingGoals.add(newGoal); - } - - if (existingGoals.size() > safeRequests.size()) { - existingGoals.subList(safeRequests.size(), existingGoals.size()).clear(); - } - } + private void updateCourseStudyGoals(List<AdminCourseStudyGoalRequest> requests) { + List<CourseStudyGoal> existingGoals = this.detail.getCourseStudyGoalList(); + // 요청 정규화: null 필터링 + 시퀀스 오름차순 + List<AdminCourseStudyGoalRequest> safeRequests = Optional.ofNullable(requests) + .orElse(Collections.emptyList()) + .stream() + .filter(Objects::nonNull) + .sorted(Comparator.comparingInt(AdminCourseStudyGoalRequest::sequence)) + .toList(); + + // 중복 시퀀스 검증 + for (int i = 1; i < safeRequests.size(); i++) { + if (safeRequests.get(i - 1).sequence() == safeRequests.get(i).sequence()) { + throw new IllegalArgumentException("Duplicated sequence in CourseStudyGoal requests: " + safeRequests.get(i).sequence()); + } + } + + // 기존 목록을 시퀀스 기준으로 조회/삭제 동기화 + existingGoals.sort(Comparator.comparingInt(CourseStudyGoal::getSequence)); + int reqIdx = 0, existIdx = 0; + while (reqIdx < safeRequests.size() && existIdx < existingGoals.size()) { + var req = safeRequests.get(reqIdx); + var exist = existingGoals.get(existIdx); + if (exist.getSequence() == req.sequence()) { + // 수정 + exist.modify(CourseStudyGoal.builder() + .sequence(req.sequence()) + .answer(req.answer()) + .build()); + reqIdx++; existIdx++; + } else if (exist.getSequence() < req.sequence()) { + // 삭제 + existingGoals.remove(existIdx); + } else { + // 추가 + existingGoals.add(existIdx, CourseStudyGoal.builder() + .sequence(req.sequence()) + .answer(req.answer()) + .course(this) + .build()); + reqIdx++; existIdx++; + } + } + // 나머지 요청 추가 + while (reqIdx < safeRequests.size()) { + var req = safeRequests.get(reqIdx++); + existingGoals.add(CourseStudyGoal.builder() + .sequence(req.sequence()) + .answer(req.answer()) + .course(this) + .build()); + } + // 나머지 기존 항목 삭제 + while (existIdx < existingGoals.size()) { + existingGoals.remove(existIdx); + } + }추가 제안:
AdminCourseStudyGoalRequest.toCourseStudyGoal(Course)팩토리를 재사용하면 생성 코드 중복이 줄어듭니다.- DB 제약으로
(course_id, sequence)유니크 인덱스를 고려하세요(아래CourseStudyGoal코멘트 참조).src/main/java/com/unretired/admin/course/api/payload/AdminCourseDetailLoadResponse.java (1)
40-44: 오탈자(inderviewList) 수정 확인이전 코멘트의 변수명 오탈자 이슈가
interviewList로 정상 반영되었습니다. 문제 없습니다.
🧹 Nitpick comments (17)
src/main/java/com/unretired/admin/course/api/payload/AdminCourseStudyGoalResponse.java (1)
3-7: 학습목표 DTO 문서화 보강 및 명명 일관성 점검.
- OpenAPI 문서 품질을 위해 필드에 @Schema 예시/설명을 추가하면 좋습니다.
- 유사 DTO(AdminCourseDetailStudyGoal)와의 중복/명명 불일치가 있습니다. Response/Request 페어로 통일(예: AdminCourseStudyGoalRequest/Response)하거나 공용 DTO로 재사용을 고려해 주세요.
다음과 같이 스키마 주석을 추가할 수 있습니다:
+import io.swagger.v3.oas.annotations.media.Schema; public record AdminCourseStudyGoalResponse( - int sequence, - String answer + @Schema(description = "표시 순서(1부터 시작)", example = "1", minimum = "1") + int sequence, + @Schema(description = "학습 목표(문장)", example = "데이터 분석의 기본 개념을 이해한다") + String answer ) { }src/main/java/com/unretired/admin/course/api/payload/AdminCourseDetailStudyGoal.java (1)
3-7: DTO 중복: 이름 통일 또는 재사용 고려.이 타입은 AdminCourseStudyGoalResponse와 구조가 동일합니다. 저장/조회 용도를 구분하려면 Request/Response 명명으로 통일하거나, 하나의 공용 DTO를 재사용하고 컨텍스트로 구분하는 편이 유지보수에 유리합니다. 또한 스키마 주석 추가를 권장합니다.
+import io.swagger.v3.oas.annotations.media.Schema; public record AdminCourseDetailStudyGoal( - int sequence, - String answer + @Schema(description = "표시 순서(1부터 시작)", example = "2", minimum = "1") + int sequence, + @Schema(description = "학습 목표(문장)", example = "실습을 통해 파이썬으로 데이터를 다룬다") + String answer ) { }src/main/java/com/unretired/admin/course/api/payload/AdminCourseInfoLoadResponse.java (2)
65-67: 이미지 필드 NPE 가능성(보수적 가드 권장).courseImage.getMain()/getPreview(), creator.getProfile()가 null일 경우 NPE가 발생합니다. 운영 데이터 편차를 고려해 null-safe 가드를 두는 편이 안전합니다.
- .mainImage(contentUrlProvider.getContentUrlNoDelimiter(courseImage.getMain().getPath())) - .previewImage(contentUrlProvider.getContentUrlNoDelimiter(courseImage.getPreview().getPath())) + .mainImage(courseImage.getMain() != null + ? contentUrlProvider.getContentUrlNoDelimiter(courseImage.getMain().getPath()) + : null) + .previewImage(courseImage.getPreview() != null + ? contentUrlProvider.getContentUrlNoDelimiter(courseImage.getPreview().getPath()) + : null) ... - .profile(contentUrlProvider.getContentUrlNoDelimiter(creator.getProfile().getPath())) + .profile(creator.getProfile() != null + ? contentUrlProvider.getContentUrlNoDelimiter(creator.getProfile().getPath()) + : null)추가로, 빈 문자열 반환 정책이 필요하다면 null 대신 ""를 반환하도록 합의가 필요합니다.
Also applies to: 73-73
52-54: List 필드의 OpenAPI 표현 개선(@ArraySchema 사용 제안).배열 예시를 문서에 명확히 드러내려면 @ArraySchema가 더 적합합니다.
-import io.swagger.v3.oas.annotations.media.Schema; +import io.swagger.v3.oas.annotations.media.ArraySchema; +import io.swagger.v3.oas.annotations.media.Schema; ... - @Schema(description = "마케팅 주소") - List<String> marketingUrls + @ArraySchema(arraySchema = @Schema(description = "마케팅 주소"), + schema = @Schema(implementation = String.class, example = "https://landing.unretired.co.kr/abc")) + List<String> marketingUrlssrc/main/java/com/unretired/core/course/domain/CourseImage.java (1)
58-60: List 참조 방어적 복사로 더티체킹/공유 참조 이슈 예방현재는 전달받은
introductions리스트 인스턴스를 그대로 보관합니다. 외부에서 동일 인스턴스를 변형하면 JPA 더티체킹이 안정적으로 동작하지 않거나(컨버터 사용 시 equals 비교 불안정), 의도치 않은 사이드이펙트가 생길 수 있습니다. 불변 복사(List.copyOf)로 교체 저장을 권장합니다. 메서드 명도 명확성을 위해modifyIntroductions로 바꾸는 것을 고려해볼 수 있습니다.적용 예시:
- public void modify(List<Image> introductions) { - update(this::setIntroductions, introductions); - } + public void modify(List<Image> introductions) { + if (introductions != null) { + // 방어적 복사 및 불변 보장 + this.setIntroductions(List.copyOf(introductions)); + } + }src/main/java/com/unretired/admin/course/api/payload/AdminCourseStudyGoalRequest.java (1)
6-9: 입력 유효성 강화 제안: sequence/answer 제약 추가관리자 입력값에 대한 기본 제약을 명시하면 도메인 이전 단계에서 조기 검증이 가능해집니다.
@Positive(또는@PositiveOrZero)와@NotBlank를 고려해주세요.예시 패치:
+import jakarta.validation.constraints.NotBlank; +import jakarta.validation.constraints.Positive; @@ -public record AdminCourseStudyGoalRequest( - int sequence, - String answer +public record AdminCourseStudyGoalRequest( + @Positive int sequence, + @NotBlank String answer ) {컨트롤러 메서드 파라미터에
@Valid가 함께 적용되어야 유효성 검증이 동작합니다(별도 파일).src/main/java/com/unretired/admin/course/api/payload/AdminCourseDetailSaveRequest.java (2)
11-16: 중첩 DTO 유효성/크기 제약 명시로 방어적 프로그래밍중첩 리스트에 대한
@Valid(중첩 요소 검증), 항목 수/문자 길이 제약을 추가하면 불량 데이터 유입을 API 레이어에서 차단할 수 있습니다. null 입력은 “부분 수정” 의미로 해석될 테니, 그 계약을 Javadoc/Schema에 명시하는 것도 추천합니다.예시 패치:
+import jakarta.validation.Valid; +import jakarta.validation.constraints.Size; @@ - @Schema(description = "세부 이미지 URL") - List<ImageRequest> introductions, + @Schema(description = "세부 이미지 URL") + @Valid @Size(max = 20) + List<ImageRequest> introductions, @@ - @Schema(description = "학습 목표") - List<AdminCourseStudyGoalRequest> courseStudyGoalList, + @Schema(description = "학습 목표") + @Valid @Size(max = 20) + List<AdminCourseStudyGoalRequest> courseStudyGoalList, @@ - @Schema(description = "마케팅 주소") - List<String> marketingUrls + @Schema(description = "마케팅 주소") + @Size(max = 20) + List<String> marketingUrls필요 시, 허용 최대 개수는 실제 비즈니스 요구에 맞춰 조정하세요.
4-8: CDN 주소 사용 정책과 정합성 확인이 DTO를 소비하는
CourseDetailModifier.from(request)가marketingUrls/introductions를 CDN 규칙에 맞게 정규화(스킴 강제, 도메인 화이트리스트)하는지 확인이 필요합니다. 미정이면 서비스 레이어에서ContentUrlProvider를 활용한 정규화를 고려하세요.src/main/java/com/unretired/admin/course/service/AdminCourseServiceImpl.java (1)
153-155: 세부 정보 수정 플로우 전환 확인 — 부분 업데이트 계약 재검토 권장
CourseDetailModifier.from(request)→course.modifyCourseDetail(...)전환은 도메인 캡슐화에 적합합니다. 다만, DTO 필드가 null일 때 “미변경”, 빈 리스트일 때 “초기화”로 해석되는 계약이 지켜지는지 확인 필요합니다(특히introductions,courseStudyGoalList).계약 보장을 위해 통합 테스트 추가를 제안합니다. 원하시면 테스트 스켈레톤을 제공하겠습니다.
src/main/java/com/unretired/core/course/domain/Course.java (2)
148-153: introductions 갱신 호출은 null-관용 설계 전제 — 호출부 주석 보강 제안
toIntroductionImages()가 null을 반환하면CourseImage.modify(List<Image>)내부update(...)로 인해 “변경 없음”으로 처리됩니다. 설계 의도를 명확히 하기 위해 “null = no-op, empty = clear” 계약을 메서드 주석/JavaDoc에 명시해 주세요.
245-253: 생성 시 DTO → 도메인 매핑 팩토리 재사용
AdminCourseStudyGoalRequest에toCourseStudyGoal(Course)가 있다면 이를 활용해 생성 중복/결합을 줄여 주세요.- CourseStudyGoal newGoal = CourseStudyGoal.builder() - .sequence(request.sequence()) - .answer(request.answer()) - .course(this) - .build(); + CourseStudyGoal newGoal = request.toCourseStudyGoal(this);src/main/java/com/unretired/core/course/domain/CourseDetail.java (3)
43-46: 조회 순서 보장을 위해 정렬 애너테이션 추가 권장클라이언트 일관성을 위해
sequence ASC정렬을 보장하세요.- @OneToMany(cascade = CascadeType.ALL, orphanRemoval = true) + @OneToMany(cascade = CascadeType.ALL, orphanRemoval = true) + @jakarta.persistence.OrderBy("sequence ASC") @JoinColumn(name = "course_id") private List<CourseStudyGoal> courseStudyGoalList = new ArrayList<>();
47-69: Builder에서 null 리스트 방어: NPE 및 예기치 않은 상태 예방빌더 인자로 null이 들어오면 필드가 null이 되어 이후
.stream()에서 NPE가 납니다(조회단AdminCourseDetailLoadResponse에서 특히). 안전하게 빈 리스트로 치환하세요.- this.interviews = interviews; - this.marketingUrls = marketingUrls; - this.courseStudyGoalList = courseStudyGoalList; + this.interviews = interviews == null ? new ArrayList<>() : interviews; + this.marketingUrls = marketingUrls == null ? new ArrayList<>() : marketingUrls; + this.courseStudyGoalList = courseStudyGoalList == null ? new ArrayList<>() : courseStudyGoalList;
75-84: modify(...)가 새 필드(courseStudyGoalList)를 무시 — 의도 명시/분리 유지 필요현재
modify(...)는courseStudyGoalList를 갱신하지 않습니다. 상위Course.modifyCourseDetail(...)에서 별도 동기화하므로 “의도된 무시”라면 주석으로 설명을 남기거나, 명시적 no-op 메서드로 의도를 드러내 주세요.src/main/java/com/unretired/core/course/domain/CourseDetailModifier.java (1)
31-35: toCourseDetail()가 marketingUrls만 반영 — 의도 확인 필요해당 API가 “기획 상세정보 등록”에서 마케팅 URL만 수정하는 계약이라면 ok입니다. 아니라면
benefit/reason/description...등도 반영하도록 확장하거나, 메서드명을 더 구체화(toCourseDetailForMarketing())해 주세요.src/main/java/com/unretired/core/course/domain/CourseStudyGoal.java (1)
28-31: DB 제약 및 컬럼 정의 보강: 무결성/길이 제한 명시 권장
sequence/answer의 not-null 및 길이 제한을 명시해 데이터 품질을 보장하세요. 또한(course_id, sequence)유니크 인덱스 권장.- private int sequence; + @jakarta.persistence.Column(nullable = false) + private int sequence; - private String answer; + @jakarta.persistence.Column(nullable = false, length = 1000) + private String answer;추가(선택):
- 테이블 수준 유니크 제약:
@Table(uniqueConstraints = @UniqueConstraint(name="uk_course_goal_sequence", columnNames={"course_id","sequence"}))- equals/hashCode(식별자 기반) 구현으로 컬렉션 동작 안정화
src/main/java/com/unretired/admin/course/api/payload/AdminCourseDetailLoadResponse.java (1)
50-56: 학습목표/인터뷰 정렬 일관성 보장 제안응답 순서를 시퀀스 기준으로 고정하면 클라이언트 단 정렬 의존성을 제거할 수 있습니다.
courseStudyGoalList는sequence ASC로,interviewList도 동일 기준으로 정렬 권장.- var courseStudyGoalList = course.getDetail().getCourseStudyGoalList() - .stream() + var courseStudyGoalList = course.getDetail().getCourseStudyGoalList() + .stream() + .sorted(java.util.Comparator.comparingInt(com.unretired.core.course.domain.CourseStudyGoal::getSequence)) .map(courseStudyGoal -> new AdminCourseStudyGoalResponse( courseStudyGoal.getSequence(), courseStudyGoal.getAnswer() )) .toList();인터뷰도 유사하게:
var interviewList = detail.getInterviews() .stream() .sorted(Comparator.comparingInt(CourseInterviewResponse::sequence)) // sequence 노출 시 .map(CourseInterviewResponse::from) .toList();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
src/main/java/com/unretired/admin/course/api/payload/AdminCourseDetailLoadResponse.java(1 hunks)src/main/java/com/unretired/admin/course/api/payload/AdminCourseDetailSaveRequest.java(1 hunks)src/main/java/com/unretired/admin/course/api/payload/AdminCourseDetailStudyGoal.java(1 hunks)src/main/java/com/unretired/admin/course/api/payload/AdminCourseInfoLoadResponse.java(2 hunks)src/main/java/com/unretired/admin/course/api/payload/AdminCourseStudyGoalRequest.java(1 hunks)src/main/java/com/unretired/admin/course/api/payload/AdminCourseStudyGoalResponse.java(1 hunks)src/main/java/com/unretired/admin/course/service/AdminCourseServiceImpl.java(4 hunks)src/main/java/com/unretired/core/course/api/payload/CourseInterviewResponse.java(1 hunks)src/main/java/com/unretired/core/course/domain/Course.java(3 hunks)src/main/java/com/unretired/core/course/domain/CourseDetail.java(4 hunks)src/main/java/com/unretired/core/course/domain/CourseDetailModifier.java(1 hunks)src/main/java/com/unretired/core/course/domain/CourseImage.java(2 hunks)src/main/java/com/unretired/core/course/domain/CourseInfoModifier.java(1 hunks)src/main/java/com/unretired/core/course/domain/CourseStudyGoal.java(1 hunks)src/main/java/com/unretired/core/resource/api/ResourceApi.java(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
src/main/java/com/unretired/core/course/domain/Course.java (3)
src/main/java/com/unretired/core/course/domain/CourseStudyGoal.java (1)
Entity(18-47)src/main/java/com/unretired/core/course/domain/CourseDetail.java (1)
Getter(22-85)src/main/java/com/unretired/core/course/domain/CourseImage.java (1)
Getter(19-61)
src/main/java/com/unretired/core/course/domain/CourseImage.java (1)
src/main/java/com/unretired/global/utils/Functions.java (1)
Functions(6-21)
src/main/java/com/unretired/core/course/domain/CourseStudyGoal.java (2)
src/main/java/com/unretired/core/course/domain/Course.java (1)
Getter(35-259)src/main/java/com/unretired/core/course/domain/CourseDetail.java (1)
Getter(22-85)
src/main/java/com/unretired/core/course/domain/CourseDetail.java (1)
src/main/java/com/unretired/global/utils/Functions.java (1)
Functions(6-21)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-and-deploy
🔇 Additional comments (8)
src/main/java/com/unretired/core/resource/api/ResourceApi.java (1)
23-27: consumes/produces 명시: 좋습니다.멀티파트 업로드의 콘텐츠 협상 문제가 예방됩니다. Swagger UI 상의 테스트 호환성도 좋아집니다.
src/main/java/com/unretired/core/course/domain/CourseImage.java (1)
15-18: 필요한 의존성 정리 확인 — 문제 없음
List및update정적 임포트 추가는 새 오버로드와 일관됩니다.src/main/java/com/unretired/admin/course/service/AdminCourseServiceImpl.java (3)
15-20: 와일드카드 제거로 명시적 임포트 — 가독성 개선도메인 의존이 명확해져 추적성이 좋아졌습니다.
36-40: 스트림 유틸 관련 임포트 추가 확인 — 문제 없음
Collectors/Function/LinkedHashMap사용과 일치합니다.
137-137: 다른 호출부 없음 확인 – 추가 변경 불필요
AdminCourseDetailLoadResponse.from(course, contentUrlProvider)호출은
src/main/java/com/unretired/admin/course/service/AdminCourseServiceImpl.java내loadDetail메서드 한 곳에만 존재함이 확인되었습니다.따라서 다른 호출부에 대한 동일한 변경 적용은 필요하지 않습니다.
src/main/java/com/unretired/core/course/domain/CourseDetailModifier.java (2)
37-42: null 반환 계약 유지하되 호출부 NPE 방어 필요여기서 null을 반환하는 설계(“null = 변경 없음”)는 괜찮습니다. 다만 조회단(
AdminCourseDetailLoadResponse)에서getIntroductions().stream()을 바로 호출해 NPE가 발생합니다. 조회부에서 null-관용 처리를 추가해야 합니다(아래 해당 파일 코멘트 참조).
6-7: 리뷰 코멘트 재검토: Image 패키지 변경 제안 불필요제안하신
import com.unretired.core.course.domain.Image;는 오히려 컴파일 에러를 발생시킵니다.
com.unretired.core.course.domain.Image클래스는 존재하지 않으며,CourseImage및 관련 도메인에서는 모두com.unretired.core.unit.domain.Image를 사용하고 있습니다.따라서 기존의
import com.unretired.core.unit.domain.Image;를 유지하시면 되며, 본 변경 제안은 무시해 주세요.Likely an incorrect or invalid review comment.
src/main/java/com/unretired/admin/course/api/payload/AdminCourseDetailLoadResponse.java (1)
58-66: 필드 매핑은 적절 — 추천사유/상태/마케팅 URL 명시적 매핑 LGTM응답 필드 리네이밍/추가에 맞춘 매핑이 명확합니다.
| var introductionImageList = courseImage.getIntroductions() | ||
| .stream() | ||
| .map(introduction -> contentUrlProvider.getContentUrlNoDelimiter(introduction.getPath())) | ||
| .toList(); | ||
|
|
There was a problem hiding this comment.
introductionImageList 구성부 NPE 가능성
courseImage.getIntroductions()가 null일 수 있어 .stream()에서 NPE가 납니다. null-관용 처리를 추가해 주세요.
- var introductionImageList = courseImage.getIntroductions()
- .stream()
- .map(introduction -> contentUrlProvider.getContentUrlNoDelimiter(introduction.getPath()))
- .toList();
+ var introductionImageList = java.util.Optional.ofNullable(courseImage.getIntroductions())
+ .orElseGet(java.util.ArrayList::new)
+ .stream()
+ .map(introduction -> contentUrlProvider.getContentUrlNoDelimiter(introduction.getPath()))
+ .toList();필요한 import(파일 상단에 추가):
import java.util.Optional;
import java.util.ArrayList;🤖 Prompt for AI Agents
In
src/main/java/com/unretired/admin/course/api/payload/AdminCourseDetailLoadResponse.java
around lines 45 to 49, courseImage.getIntroductions() can be null which will
cause an NPE at .stream(); wrap the call with null-tolerant logic such as using
Optional.ofNullable(courseImage.getIntroductions()).orElseGet(ArrayList::new).stream()
to ensure a non-null list before mapping and toList(), and add the requested
imports (java.util.Optional and java.util.ArrayList) at the file top.
| @Schema(description = "인터뷰 번호") | ||
| Long sequence |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
“sequence” 의미 확인 필요: 정렬순서 vs 질문 ID 혼용 가능성
필드명이 sequence인데 매핑은 interview.getQuestion().getId()로, 실제로는 “질문 ID”를 담고 있습니다. PR 요약의 “sequence 정보 포함”과 어긋날 수 있어 API 의미 혼동 우려가 큽니다. 도메인에 진짜 정렬 순서가 있다면 그 값을 쓰고, 아니라면 필드명을 questionId로 명확히 하는 것을 권장합니다.
대안 A: 실제 정렬 순서를 노출
- .sequence(interview.getQuestion().getId())
+ .sequence(interview.getSequence())대안 B: 질문 ID를 노출하는 것이 의도라면 필드명/스키마를 명확화
- @Schema(description = "인터뷰 번호")
- Long sequence
+ @Schema(description = "인터뷰 질문 ID")
+ Long questionId
@@
- .sequence(interview.getQuestion().getId())
+ .questionId(interview.getQuestion().getId())검증 스크립트(도메인에 getSequence() 존재 여부 확인):
Also applies to: 23-23
🏁 Script executed:
#!/bin/bash
# CourseInterview에 sequence 속성/접근자 존재 여부 탐색
rg -nP -C3 --type=java '\bclass\s+CourseInterview\b|\bgetSequence\s*\(|\bsequence\b'
# 질문 ID와 구분되는 순서 개념이 있는지 확인
rg -nP -C3 --type=java 'CourseInterview.*\b(getQuestion\(\)|getId\(\))'Length of output: 93093
추가 검증: CourseInterview 클래스 내에 sequence 속성 또는 접근자가 실제로 존재하는지 확인하기 위해 아래 스크립트를 실행해 주세요.
🏁 Script executed:
#!/bin/bash
# CourseInterview.java 첫 200줄 출력
sed -n '1,200p' src/main/java/com/unretired/core/course/domain/CourseInterview.javaLength of output: 1073
CourseInterviewResponse API 필드명·스키마 오류 수정 필요
CourseInterview 도메인에는 ‘정렬 순서(sequence)’ 속성이 존재하지 않고, 현재 .sequence(interview.getQuestion().getId())로 질문 ID를 노출하고 있습니다. 따라서 API 필드명이 잘못되어 클라이언트 혼동을 일으킬 우려가 높습니다.
아래 위치를 수정해 주세요:
• 파일: src/main/java/com/unretired/core/course/api/payload/CourseInterviewResponse.java
– 16–17행:
java @Schema(description = "인터뷰 번호") Long sequence
– 23행(Builder 매핑):
java .sequence(interview.getQuestion().getId())
제안하는 수정(diff 예시):
- @Schema(description = "인터뷰 번호")
- Long sequence
+ @Schema(description = "인터뷰 질문 ID")
+ Long questionId
@@
- .sequence(interview.getQuestion().getId())
+ .questionId(interview.getQuestion().getId())이렇게 하면 API 스펙이 실제 의도(질문 ID 노출)와 일치하여 명확성을 확보할 수 있습니다.
🤖 Prompt for AI Agents
In
src/main/java/com/unretired/core/course/api/payload/CourseInterviewResponse.java
around lines 16–17 and 23, the DTO exposes interview.getQuestion().getId() as
sequence which is incorrect and confusing; change the field name and schema to
reflect that it is the question ID (e.g., replace "Long sequence" and its
@Schema(description = "인터뷰 번호") with "Long questionId" and @Schema(description =
"질문 ID"), and in the builder mapping replace
.sequence(interview.getQuestion().getId()) with
.questionId(interview.getQuestion().getId()); also update any corresponding
getter/setter/builder method names to match questionId to keep the class
consistent.
1 participant
개요
강연 상세 조회, 및 등록
작업 내용
관련 이슈
#353
Summary by CodeRabbit