Conversation
|
Caution Review failedThe pull request is closed. Walkthrough리뷰 작성 흐름을 단계별(초기화 → 1차 응답 → 2차 응답 → AI 생성 → 이미지 업로드 → 제출)으로 재구성하고, OpenAI 기반 AI 리뷰 생성, S3 업로드 분기 추가, 리뷰 관련 엔티티/DTO/검증/스케줄러/레포지토리 전면 도입 및 주문 상태 단순화가 포함된 대규모 기능 추가입니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant Controller as ReviewController
participant CmdService as ReviewCommandService
participant Repo as ReviewRepository
participant AIService as AiReviewGeneratorService
participant OpenAI as OpenAiService
participant S3 as S3ImageService
participant UserSrv as UserService
Client->>Controller: initializeReview(userId, orderItemId)
Controller->>CmdService: initializeReview(...)
CmdService->>Repo: save(Review)
Repo-->>CmdService: reviewId
CmdService-->>Controller: ReviewIdResponse
Controller-->>Client: reviewId
Client->>Controller: updateReviewStep1(userId, reviewId, Step1)
Controller->>CmdService: updateReviewStep1(...)
CmdService->>Repo: findById(reviewId)
Repo-->>CmdService: Review
CmdService->>Repo: save(Review)
CmdService-->>Controller: Step1Response
Controller-->>Client: step1 완료
Client->>Controller: generateSizeAiReview(userId, reviewId)
Controller->>CmdService: generateSizeAiReview(...)
CmdService->>AIService: generateSizeReview(request)
AIService->>OpenAI: callOpenAi(systemPrompt, userMsg)
OpenAI-->>AIService: aiText
AIService-->>CmdService: AiReviewResponse
CmdService-->>Controller: AiReviewResponse
Controller-->>Client: AI 문장 목록
Client->>Controller: uploadReviewImages(images[])
Controller->>S3: uploadReviewImage(file) xN
S3-->>Controller: imageUrl[]
Controller-->>Client: imageUrl 목록
Client->>Controller: submitReview(userId, reviewId, FinalSubmit)
Controller->>CmdService: submitReview(...)
CmdService->>Repo: findById(reviewId)
Repo-->>CmdService: Review
CmdService->>UserSrv: creditPoints(userId, points)
UserSrv-->>CmdService: ok
CmdService->>Repo: save(Review)
CmdService-->>Controller: success
Controller-->>Client: 제출 완료
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
🔍 핵심 리뷰 포인트
🚥 Pre-merge checks | ✅ 4 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
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/ongil/backend/domain/review/service/ReviewQueryService.java (1)
152-176:⚠️ Potential issue | 🟠 Major
getPendingReviews와getPendingReviewCount모두 DRAFT 상태 리뷰 필터링이 필요합니다.
getPendingReviews(line 163, 169)에서existsByOrderItemIdAndReviewType를 사용하고,getPendingReviewCount(line 192-193)에서findReviewedOrderItemIds를 사용하는데, 두 메서드 모두ReviewStatus를 검사하지 않습니다. 따라서 사용자가 작성 중(DRAFT)인 리뷰가 있으면 완료되지 않은 리뷰도 "이미 작성됨"으로 판단되어, 사용자가 다시 리뷰를 작성할 수 없게 됩니다. 스케줄러가 30분 후 DRAFT를 삭제하거나 사용자가 리뷰를 완료 제출할 때까지 리뷰 작성 옵션이 노출되지 않습니다.두 메서드 모두 새로 추가된
existsByOrderItemIdAndReviewTypeAndReviewStatus를 사용하여ReviewStatus.COMPLETED상태만 체크하거나,findReviewedOrderItemIds의 쿼리에AND r.reviewStatus = 'COMPLETED'조건을 추가하는 것이 필요합니다.src/main/java/com/ongil/backend/domain/review/controller/ReviewController.java (1)
103-111: 🛠️ Refactor suggestion | 🟠 Major
toggleHelpful은 상태를 변경하는 write 연산인데reviewQueryService에 위치해 있습니다.CQRS 분리 원칙에 따르면 도움돼요 토글은 데이터를 변경하므로
reviewCommandService에 위치해야 합니다. 현재 구조는 Query 서비스가 쓰기 작업을 수행하게 되어 CQRS의 목적을 희석시킵니다.
🤖 Fix all issues with AI agents
In `@src/main/java/com/ongil/backend/domain/order/converter/OrderConverter.java`:
- Around line 32-36: The current OrderConverter uses
UUID.randomUUID().toString().replace("-", "").substring(0, 10).toUpperCase() to
build orderNumber which is too short and risks unique-constraint collisions;
update OrderConverter to stop truncating to 10 chars (use the full 32-char hex
string or a longer substring) OR implement a bounded retry safe-generation: in
the orderNumber generation path (the code around orderNumber, UUID.randomUUID,
replace, substring) loop to regenerate and check uniqueness via the Order
repository (e.g., existsByOrderNumber) with a limited number of attempts and
throw a clear exception if exceeded; ensure the final chosen approach removes
the unsafe substring(0, 10) usage or adds the retry+repository check to prevent
DataIntegrityViolationException.
In
`@src/main/java/com/ongil/backend/domain/review/controller/ReviewController.java`:
- Around line 124-154: Add javax.validation by annotating the controller request
bodies with `@Valid` so Bean Validation runs: put `@Valid` on the `@RequestBody`
parameters of updateReviewStep1 (ReviewStep1Request), updateReviewStep2Size
(ReviewStep2SizeRequest), and updateReviewStep2Material
(ReviewStep2MaterialRequest), and likewise add `@Valid` to the controller method
that accepts ReviewFinalSubmitRequest; this ensures the
`@NotNull/`@NotBlank/@Min/@Max annotations on those DTOs are enforced before
calling the service methods.
- Around line 176-189: The uploadReviewImages method lacks the authenticated
user id parameter; update the controller method uploadReviewImages to accept
`@AuthenticationPrincipal` Long userId and pass that userId into the S3 upload
flow (use the appropriate s3ImageService.uploadReviewImage overload or update it
to accept userId and the MultipartFile) so uploaded images are associated with
the authenticated user, and keep the same validation (max 5) and return behavior
as before to maintain consistency with other endpoints like init/step1.
In
`@src/main/java/com/ongil/backend/domain/review/converter/ReviewConverter.java`:
- Around line 68-74: The three methods buildAnswerSummary,
buildInitialFirstAnswers, and buildMyReviewAnswerSummary call getDisplayName()
on enum fields (sizeAnswer, colorAnswer, materialAnswer) which can be null and
cause NPEs; update each method to null-safe access by checking each
review.getSizeAnswer()/getColorAnswer()/getMaterialAnswer() before calling
getDisplayName() and pass a safe default (e.g., null or a constant like
"UNKNOWN" or empty string) into the ReviewListResponse.AnswerSummary builder
when the enum is null (use ternary/Optional or helper method to map enum ->
display string safely), ensuring every builder field (sizeAnswer, colorAnswer,
materialAnswer) is populated without invoking getDisplayName() on a null
reference.
In
`@src/main/java/com/ongil/backend/domain/review/converter/ReviewWriteConverter.java`:
- Around line 32-46: toStep1Response can NPE when review.getSizeAnswer() or
review.getMaterialAnswer() is null (e.g., reviews created by
toInitialReviewEntity). Defensively check for nulls: compute needsSizeQ =
review.getSizeAnswer() != null &&
review.getSizeAnswer().isNeedsSecondaryQuestion(), needsMaterialQ =
review.getMaterialAnswer() != null &&
review.getMaterialAnswer().isNeedsSecondaryQuestion(), and only populate
availableBodyParts when needsSizeQ is true (use Collections.emptyList()
otherwise). Update the toStep1Response method to use these null-safe booleans
before calling ReviewStep1Response.of and keep references to
getClothingCategory().getBodyParts() as the source for body parts.
- Around line 48-66: The split(",") results in items that may contain
leading/trailing spaces (e.g., " 무게감"); update both toMaterialAiRequest and
toSizeAiRequest to trim each element after splitting and remove empty strings
before collecting into a List for materialFeatures and fitIssueParts. Replace
the Arrays.asList(...split(",")) usage with a stream-based approach (e.g.,
Stream.of(...split(",")) or Arrays.stream(...)) that maps each item with
String::trim, filters out blank/empty strings, and collects to a List; keep the
existing null check and return Collections.emptyList() when the source string is
null.
In
`@src/main/java/com/ongil/backend/domain/review/dto/request/AiReviewGenerateRequest.java`:
- Around line 27-29: The List fields fitIssueParts and materialFeatures in
AiReviewGenerateRequest can be null when built with Lombok's `@Builder`; add
`@Builder.Default` to each and initialize them to an empty list (e.g., new
ArrayList<>()) so that consumers like SizeReviewPrompter and
MaterialReviewPrompter can safely call .isEmpty() without NPE; update the field
declarations for fitIssueParts and materialFeatures in the
AiReviewGenerateRequest class to include `@Builder.Default` and an empty-list
initializer.
In
`@src/main/java/com/ongil/backend/domain/review/dto/request/ReviewStep1Request.java`:
- Around line 22-37: Replace the invalid `@NotBlank` constraints on enum fields
with `@NotNull`: change the annotation on clothingCategory (ClothingCategory),
sizeAnswer (SizeAnswer), colorAnswer (ColorAnswer), and materialAnswer
(MaterialAnswer) to `@NotNull` so Jakarta Validation applies correctly to enum
types; update imports if needed to use jakarta.validation.constraints.NotNull
and remove NotBlank usage to avoid UnexpectedTypeException at runtime.
In `@src/main/java/com/ongil/backend/domain/review/enums/ColorAnswer.java`:
- Around line 1-14: The request DTO uses `@NotBlank` on enum-typed fields which is
incorrect; replace `@NotBlank` with `@NotNull` on the enum fields in
ReviewStep1Request (specifically colorAnswer, clothingCategory, sizeAnswer,
materialAnswer) so Bean Validation validates these enum properties properly;
update import if needed and keep the existing messages (e.g., "색감 답변은 필수입니다.")
on the `@NotNull` annotations.
In
`@src/main/java/com/ongil/backend/domain/review/service/AiReviewGeneratorService.java`:
- Around line 58-71: The code hardcodes the model string and unsafely parses the
OpenAI response; change the hardcoded "gpt-4o-mini" in AiReviewGeneratorService
to a configurable property (read from application.yml via `@Value` or a
`@ConfigurationProperties` field) and use that variable in
ChatCompletionRequest.builder(), and replace the blind chained access after
openAiService.createChatCompletion(...) with explicit null/empty checks: ensure
getChoices() is non-empty and the selected Choice's getMessage() and
getContent() are non-null/blank before trimming, and throw a distinct
AppException (e.g., ErrorCode.AI_RESPONSE_FORMAT_ERROR) when parsing fails; also
narrow exception handling to catch OpenAI client/network exceptions separately
from parsing validation so you can log/handle OpenAI errors (e.g., wrap or
rethrow with ErrorCode.AI_GENERATION_ERROR) versus response format errors for
easier debugging while still logging the original causes and including them in
the thrown exceptions.
In
`@src/main/java/com/ongil/backend/domain/review/service/prompter/SizeReviewPrompter.java`:
- Around line 114-116: The user-facing prompt in SizeReviewPrompter uses the
banned term "실루엣" within the request.getFitIssueParts() branch (the
prompt.append call that adds "- '전반적' 항목에 대해서는 특정 신체 부위 언급 없이 전체적인 실루엣이나 조이는 느낌에
대해 2문장을 생성할 것."); change that appended string to remove any system-prohibited
terms (e.g., replace "전체적인 실루엣" with an allowed phrase like "전체적인 인상" or "전반적인
착용감") so the user message no longer conflicts with the system prompt, and scan
SizeReviewPrompter for any other occurrences of the banned words ("핏", "옷매무새",
"Y존", "라인 부각", "실루엣") and update them similarly.
In
`@src/main/java/com/ongil/backend/domain/review/service/ReviewCommandService.java`:
- Around line 43-56: Reorder and null-protect the category parent access in
initializeReview: first retrieve OrderItem via getOrderItemOrThrow(orderItemId),
call reviewValidator.validateReviewAuthority(orderItem, userId) before loading
User to avoid an unnecessary user DB fetch on unauthorized requests, then call
getUserOrThrow(userId); when resolving category name, avoid chaining
getParentCategory() unguarded—retrieve Product's Category into a local (e.g.,
category), check Category parent = category.getParentCategory(); use parent !=
null ? parent.getName() : category.getName() as the display name passed into
ClothingCategory.fromDisplayName, and proceed to build the Review via
reviewWriteConverter.toInitialReviewEntity(...) and save with
reviewRepository.save(...).
- Around line 126-152: Call the existing step-completion validator before
submitting and remove the hardcoded reward: invoke
reviewValidator.validateReviewStepCompletion(...) (pass the Review or the
Review's orderItem and the ReviewFinalSubmitRequest as required by the existing
method signature) right after validateReviewAuthority and before assembling
joined fields, then replace the literal 500 with a class-level constant (e.g.,
private static final int REVIEW_REWARD_AMOUNT = 500) and use
REVIEW_REWARD_AMOUNT for both review.submit(...) and user.restorePoints(...).
In `@src/main/java/com/ongil/backend/global/config/OpenAiConfig.java`:
- Around line 9-19: The OpenAiConfig currently uses the deprecated
com.theokanning.openai library and doesn't validate the injected apiKey; update
OpenAiConfig to use the official OpenAI Java SDK (com.openai:openai-java)
instead of com.theokanning.openai.service.OpenAiService and change the
openAiService() bean to construct the new SDK client, and before construction
validate the apiKey field (the apiKey member and inside openAiService()) to
ensure it's not null or empty and throw a clear IllegalStateException (or
similar) if invalid; update imports and any timeout/configuration usage to the
API of the new SDK when replacing the OpenAiService instantiation.
In `@src/main/java/com/ongil/backend/global/config/s3/S3ImageService.java`:
- Around line 39-54: The upload(MultipartFile file, String directory) method
should be made private to prevent external callers from supplying arbitrary
directories; change its access modifier from public to private, leaving the
public wrapper methods uploadProfileImage(MultipartFile) and
uploadReviewImage(MultipartFile) intact so all uploads go through their
controlled PROFILE_DIRECTORY and REVIEW_DIRECTORY values (ensure any internal
usages still call upload(...) and update visibility accordingly).
🧹 Nitpick comments (22)
src/main/java/com/ongil/backend/domain/order/converter/OrderConverter.java (1)
105-108: 이미지 URL 파싱 로직이toOrderItemDto와toItemSummaryDto에 중복됩니다.동일한 이미지 URL 추출 로직이 두 메서드에 복사되어 있습니다. private 헬퍼 메서드로 추출하면 DRY 원칙을 준수하고, 향후 URL 파싱 규칙 변경 시 한 곳만 수정하면 됩니다.
♻️ 헬퍼 메서드 추출 제안
+ private String extractFirstImageUrl(Product product) { + if (product.getImageUrls() != null && !product.getImageUrls().isBlank()) { + return product.getImageUrls().split(",")[0].trim(); + } + return "default-image-url"; + }Also applies to: 172-175
src/main/java/com/ongil/backend/domain/order/service/OrderService.java (2)
250-257:validateCancelable: 2-상태 체계에서 두 조건이 사실상 중복입니다.현재
OrderStatus가CONFIRMED와CANCELED두 값만 존재하므로, Line 251의== CANCELED체크와 Line 254의!= CONFIRMED체크는 동일한 상태를 가리킵니다. 향후 상태가 추가될 가능성을 고려하면 현재 구조도 방어적으로 괜찮지만, 지금 시점에서는 에러 코드(ORDER_ALREADY_CANCELEDvsORDER_CANCEL_NOT_ALLOWED)가 CANCELED 상태에 대해 어떤 것이 반환되는지 첫 번째 조건에 의존하므로, 의도가 명확히 드러나도록 코멘트를 남겨두면 좋겠습니다.
67-97: 기존 코드 참고:decreasePoints호출(Line 71)이 유효성 검증(Line 86)보다 앞에 있습니다.이번 PR 변경 범위는 아니지만,
user.decreasePoints()가 포인트 유효성 검증보다 먼저 실행됩니다.@Transactional롤백으로 데이터 정합성은 보장되지만, validation-first 패턴이 더 명확합니다. 향후 리팩토링 시 참고해 주세요.src/main/java/com/ongil/backend/domain/review/dto/response/ReviewStep1Response.java (2)
12-27:@Schema어노테이션 누락 — 동일 패키지 내 DTO와 일관성 불일치
ReviewIdResponse에는@Schema(description = ..., example = ...)어노테이션이 사용되고 있으나, 이 DTO에는 누락되어 있습니다. API 문서 자동 생성(Swagger/OpenAPI) 시 필드 설명이 표시되지 않습니다.또한
availableBodyParts가needsSizeSecondaryQuestion == false일 때null인지 빈 리스트인지에 대한 계약이 명확하지 않습니다. 프론트엔드 연동 시 혼란 방지를 위해 빈 리스트(Collections.emptyList()) 반환을 권장합니다.
19-26: 팩토리 메서드 파라미터명 축약 — 가독성 저하
needsSizeQ,needsMaterialQ는 필드명(needsSizeSecondaryQuestion,needsMaterialSecondaryQuestion)과 불일치하여 코드 추적 시 혼란을 줄 수 있습니다.♻️ 파라미터명 일관성 개선 제안
- public static ReviewStep1Response of(Long reviewId, boolean needsSizeQ, boolean needsMaterialQ, List<String> availableBodyParts) { + public static ReviewStep1Response of(Long reviewId, boolean needsSizeSecondaryQuestion, boolean needsMaterialSecondaryQuestion, List<String> availableBodyParts) { return ReviewStep1Response.builder() .reviewId(reviewId) - .needsSizeSecondaryQuestion(needsSizeQ) - .needsMaterialSecondaryQuestion(needsMaterialQ) + .needsSizeSecondaryQuestion(needsSizeSecondaryQuestion) + .needsMaterialSecondaryQuestion(needsMaterialSecondaryQuestion) .availableBodyParts(availableBodyParts) .build(); }src/main/java/com/ongil/backend/domain/review/dto/request/ReviewStep2SizeRequest.java (1)
14-19:fitIssueParts에 타입 안전성 및 요소 수준 검증 부재
ReviewStep2MaterialRequest는List<MaterialFeatureType>(enum)을 사용하여 타입 안전성을 확보하고 있으나, 이 DTO는List<String>을 사용합니다. 신체 부위가 사전 정의된 값이라면 enum 사용이 일관적이며, 잘못된 입력(오타, 빈 문자열 등)을 컴파일 타임에 방지할 수 있습니다.또한,
@NotEmpty는 리스트가 비어있지 않은지만 검증하며, 리스트 내 개별 요소가 빈 문자열("")이거나 공백인 경우는 통과합니다.♻️ 요소 수준 검증 추가 제안
+import jakarta.validation.constraints.NotBlank; + `@Getter` `@NoArgsConstructor`(access = AccessLevel.PROTECTED) `@AllArgsConstructor` public class ReviewStep2SizeRequest { `@NotEmpty`(message = "불편했던 부위를 최소 1개 이상 선택해주세요.") - private List<String> fitIssueParts; + private List<@NotBlank(message = "부위명은 비어있을 수 없습니다.") String> fitIssueParts; }src/main/java/com/ongil/backend/domain/review/enums/MaterialFeatureType.java (1)
12-20:Arrays.asList()대신List.of()사용 권장,values필드명 혼동 가능성
Arrays.asList()는 고정 크기이지만 요소 교체가 가능한 리스트를 반환합니다. enum 상수에는 완전 불변인List.of()가 더 적합합니다.
values필드명은 Java enum의 내장values()정적 메서드와 혼동될 수 있습니다. LombokgetValues()와MaterialFeatureType.values()가 공존하므로,options또는choices등으로 변경하면 명확해집니다.♻️ 불변 리스트 및 필드명 개선 제안
-import java.util.Arrays; -import java.util.List; +import java.util.List; `@Getter` `@RequiredArgsConstructor` public enum MaterialFeatureType { - TEXTURE("촉감", Arrays.asList("부드러움", "거칠음")), - WEIGHT("무게감", Arrays.asList("가벼움", "무거움")), - WRINKLE("구김 정도", Arrays.asList("없음", "많음")), - THICKNESS("두께감", Arrays.asList("두꺼움", "얇음")), - PILLING("보풀", Arrays.asList("없음", "있음")), - TRANSPARENCY("비침 정도", Arrays.asList("안비쳐요", "비쳐요")); + TEXTURE("촉감", List.of("부드러움", "거칠음")), + WEIGHT("무게감", List.of("가벼움", "무거움")), + WRINKLE("구김 정도", List.of("없음", "많음")), + THICKNESS("두께감", List.of("두꺼움", "얇음")), + PILLING("보풀", List.of("없음", "있음")), + TRANSPARENCY("비침 정도", List.of("안비쳐요", "비쳐요")); private final String displayName; - private final List<String> values; + private final List<String> options; }src/main/java/com/ongil/backend/domain/review/enums/ClothingCategory.java (2)
15-29:Arrays.asList대신List.of를 사용하여 불변성을 보장하세요.
Arrays.asList는set()호출을 통한 요소 변경이 가능합니다.@Getter로 외부에 노출되는 enum 상수의 리스트이므로List.of()를 사용하면 완전한 불변 리스트가 되어 방어적 복사가 불필요합니다.♻️ List.of() 적용 예시
- OUTER("아우터", Arrays.asList( - "전반적", "어깨&목", "가슴&몸통", "겨드랑이&팔" - )), - TOP("상의", Arrays.asList( - "전반적", "어깨&목", "가슴&몸통", "겨드랑이&팔" - )), - SKIRT("스커트", Arrays.asList( - "전반적", "허리&복부", "엉덩이&가랑이", "허벅지&종아리" - )), - DRESS("원피스", Arrays.asList( - "전반적", "목&어깨", "가슴&몸통", "겨드랑이&팔", "엉덩이&다리", "기장" - )), - PANTS("팬츠", Arrays.asList( - "전반적", "허리&복부", "엉덩이&가랑이", "허벅지&종아리" - )); + OUTER("아우터", List.of( + "전반적", "어깨&목", "가슴&몸통", "겨드랑이&팔" + )), + TOP("상의", List.of( + "전반적", "어깨&목", "가슴&몸통", "겨드랑이&팔" + )), + SKIRT("스커트", List.of( + "전반적", "허리&복부", "엉덩이&가랑이", "허벅지&종아리" + )), + DRESS("원피스", List.of( + "전반적", "목&어깨", "가슴&몸통", "겨드랑이&팔", "엉덩이&다리", "기장" + )), + PANTS("팬츠", List.of( + "전반적", "허리&복부", "엉덩이&가랑이", "허벅지&종아리" + ));
List.of()사용 시import java.util.Arrays;제거도 가능합니다.
34-39: 리뷰 도메인의ClothingCategory와 상품 도메인의Category에서CATEGORY_NOT_FOUND에러코드 공유 — 에러 식별 개선 검토.
fromDisplayName()은 ReviewCommandService에서 의류 카테고리 조회 실패 시 실행되며, ProductService와 AdminService의 상품 카테고리 조회 실패와 동일한CATEGORY_NOT_FOUND에러코드를 사용합니다. 두 도메인이 서로 다른 개념(의류 타입 vs. 상품 카테고리)임에도 같은 에러 메시지("카테고리를 찾을 수 없습니다.")를 공유하면, API 클라이언트가 에러 원인을 구분하기 어려울 수 있습니다. 의류 카테고리 조회 실패에 전용 에러코드 추가를 검토해 보세요.src/main/java/com/ongil/backend/domain/review/entity/Review.java (2)
27-28:@AllArgsConstructor의 접근 제한자를PRIVATE으로 설정하세요.
@NoArgsConstructor(access = PROTECTED)로 기본 생성자를 보호하고 있지만,@AllArgsConstructor는 기본적으로public생성자를 생성합니다.@Builder만 해당 생성자를 사용하도록 접근 수준을 제한하는 것이 좋습니다.♻️ 접근 제한자 적용
-@AllArgsConstructor +@AllArgsConstructor(access = AccessLevel.PRIVATE)As per coding guidelines,
src/main/java/**/*.java: SOLID 원칙, 의존성 주입 패턴에 중점을 둔다.
120-128:helpfulCount동시성 이슈 — DB 레벨 보호 검토 권장.
incrementHelpfulCount/decrementHelpfulCount는 인메모리에서 값을 변경하는데, 동시 요청 시 lost update가 발생할 수 있습니다. 트래픽이 높은 기능이라면@Version기반 낙관적 잠금이나 DB 쿼리(UPDATE ... SET helpful_count = helpful_count + 1)로 원자적 갱신을 고려해 보세요.src/main/java/com/ongil/backend/domain/review/dto/response/ReviewDetailResponse.java (1)
132-142:InitialSecondAnswers의fitIssueParts/materialFeatures도List<String>타입 검토.
sizeReview와materialReview는List<String>으로 변경되었는데, 같은 응답 내fitIssueParts와materialFeatures는 여전히String입니다. 프론트엔드에서 이 값을 파싱해야 한다면List<String>으로 통일하는 것이 일관성 면에서 유리할 수 있습니다. 의도된 설계라면 무시해 주세요.As per coding guidelines,
**/dto/**/*.java: 필드명 일관성 확인.src/main/java/com/ongil/backend/domain/review/scheduler/ReviewCleanupScheduler.java (1)
21-35:@Scheduled설정 및 트랜잭션 처리 — 전반적으로 적절합니다.로직 자체는 깔끔하고,
@EnableScheduling도Application클래스에 설정되어 있어 스케줄러가 정상 동작합니다.
fixedRatevsfixedDelay검토:fixedRate는 이전 실행 시작 기준으로 30분 후 재실행되므로, 삭제 작업이 길어지면 실행이 겹칠 수 있습니다. 현 수준에서는 문제없겠지만, 안전하게fixedDelay로 변경하는 것도 고려해 보세요.src/main/java/com/ongil/backend/domain/review/dto/response/MyReviewResponse.java (1)
89-93:fitIssueParts와materialFeatures가String타입인 점 확인 필요.이 필드들은 복수의 값을 가질 수 있는 데이터(부위 목록, 소재 특징 목록)인데,
String타입으로 노출되고 있습니다. 프론트엔드에서 파싱 없이 바로 사용하려면List<String>이 더 적합할 수 있습니다. 기존 코드이므로 후속 리팩토링에서 고려해 주세요.src/main/java/com/ongil/backend/domain/review/dto/request/AiReviewGenerateRequest.java (1)
23-23: 필드명clothingType과 타입ClothingCategory의 네이밍 불일치.필드 이름은
clothingType이지만 실제 타입은ClothingCategory입니다.clothingCategory로 통일하면 코드 가독성이 향상됩니다. 다만 프롬프터에서 이미 사용 중이므로 후속 리팩토링에서 고려해 주세요.src/main/java/com/ongil/backend/domain/review/repository/ReviewRepository.java (1)
195-197:@Modifying에clearAutomatically = true추가는 선택사항입니다.현재 코드는 이미 올바르게 작동합니다. ReviewCleanupScheduler가 메서드 레벨에서
@Transactional을 관리하고 있으며, ReviewStatus는@Enumerated(EnumType.STRING)으로 매핑되어 있어 JPQL의 문자열 리터럴 'DRAFT'가 정상 동작합니다. 30분 간격으로 실행되는 스케줄러는 각 실행이 고립되므로 persistence context의 stale data 위험도 낮습니다.다만
clearAutomatically = true를 추가하면 벌크 DELETE 후 persistence context를 자동으로 초기화하여 방어적 프로그래밍 관점에서 더 안전합니다. 필요에 따라 적용하세요.src/main/java/com/ongil/backend/domain/review/dto/request/ReviewFinalSubmitRequest.java (1)
16-29: 최종 제출 DTO에 유효성 검증 어노테이션이 누락되어 있습니다.
ReviewStep1Request에는@NotBlank,@NotNull,@Min/@Max등의 검증이 적용되어 있지만, 이 최종 제출 DTO에는 아무런 검증이 없습니다.
reviewImageUrls에@Size(max = 5)적용 필요 — 컨트롤러(ReviewControllerLine 181)에서 이미지 업로드 시 5장 제한을 하지만, 최종 제출 시에는 임의 URL 목록을 전달받으므로 여기서도 검증이 필요합니다.sizeReview/materialReview에 대한 null 또는 빈 목록 처리 정책이 명확하지 않습니다. 의도적으로 nullable이라면 서비스 레이어에서 방어 처리가 필요합니다.🛡️ 유효성 검증 추가 제안
+import jakarta.validation.constraints.Size; + public class ReviewFinalSubmitRequest { `@Schema`(description = "기타 후기") private String textReview; `@Schema`(description = "S3에서 받은 이미지 URL 리스트 (최대 5장)", example = "[\"https://s3.../1.jpg\"]") + `@Size`(max = 5, message = "리뷰 이미지는 최대 5장까지 가능합니다.") private List<String> reviewImageUrls;src/main/java/com/ongil/backend/domain/review/service/prompter/MaterialReviewPrompter.java (1)
130-152:"두께감:선택지전체"매직 스트링을 상수 또는 Enum 값으로 추출하세요.Line 135의
"두께감:선택지전체"는MaterialFeatureType등 관련 Enum에서 정의된 토큰을 참조해야 합니다. 매직 스트링은 오타 시 컴파일 타임에 잡히지 않으며, 값이 변경될 때 여러 곳을 수정해야 합니다.또한, Line 130에서
request.getMaterialFeatures()가 null이면isEmpty()호출 시 NPE가 발생합니다.ReviewWriteConverter.toMaterialAiRequest(Line 53-54)에서 null일 때Collections.emptyList()를 설정하지만, 다른 경로로 이 메서드가 호출될 경우를 대비해 방어 코드를 고려하세요.#!/bin/bash # MaterialFeatureType Enum에서 "두께감:선택지전체" 토큰이 정의되어 있는지 확인 rg -n "선택지전체" --type=javasrc/main/java/com/ongil/backend/domain/review/converter/ReviewConverter.java (1)
284-293:parseToList와parseImageUrls의 리스트 생성 방식 불일치
parseToList(Line 292)는.toList()(불변 리스트)를,parseImageUrls(Line 264)는Collectors.toList()(가변 리스트)를 사용합니다. 동작에 문제는 없지만, 일관성을 위해 통일하는 것이 좋습니다. 응답 DTO 빌더에서 리스트를 수정하지 않으므로.toList()로 통일하는 것을 권장합니다.src/main/java/com/ongil/backend/domain/review/service/AiReviewGeneratorService.java (1)
52-71: 재시도 전략 추가 권장
OpenAiConfig에서 타임아웃은 이미 60초로 설정되어 있으므로 무한 대기 이슈는 없습니다. 다만callOpenAi메서드에서 일시적 네트워크 오류나 API 요청 실패 시 재시도 로직이 없어서, 연속된 외부 API 호출이 필요한 워크플로우에서 취약할 수 있습니다. Spring Retry(@Retryable) 또는 Resilience4j 같은 라이브러리를 활용하여 재시도 메커니즘을 추가하는 것이 좋습니다.src/main/java/com/ongil/backend/domain/review/service/ReviewCommandService.java (2)
31-41: 클래스 레벨@Transactional과 메서드 레벨@Transactional중복Line 33에서 클래스 레벨
@Transactional이 선언되어 있으므로, Line 43, 58, 75, 84, 126의@Transactional어노테이션은 중복입니다.readOnly = true처럼 속성이 다른 경우에만 메서드 레벨 어노테이션이 필요합니다.♻️ 중복 `@Transactional` 제거
- `@Transactional` public Long initializeReview(Long userId, Long orderItemId) {- `@Transactional` public ReviewStep1Response updateReviewStep1(Long userId, Long reviewId, ReviewStep1Request request) {- `@Transactional` public void updateReviewStep2Size(Long userId, Long reviewId, ReviewStep2SizeRequest request) {- `@Transactional` public void updateReviewStep2Material(Long userId, Long reviewId, ReviewStep2MaterialRequest request) {- `@Transactional` public void submitReview(Long userId, Long reviewId, ReviewFinalSubmitRequest request) {
108-124:readOnly = true메서드가 CommandService에 위치 — CQRS 분리 의도와 불일치
generateSizeAiReview와generateMaterialAiReview는@Transactional(readOnly = true)로 선언되어 DB 상태를 변경하지 않습니다. CQRS 패턴에 따라ReviewCommandService와ReviewQueryService를 분리했다면, 읽기 전용 메서드는ReviewQueryService에 배치하는 것이 일관성에 맞습니다.다만, AI 생성 결과를 이후 단계에서 저장하는 흐름상 Command 쪽에 두는 것이 편리할 수 있으므로, 향후 리팩토링 시 고려해 주세요.
| String orderNumber = UUID.randomUUID() | ||
| .toString() | ||
| .replace("-", "") | ||
| .substring(0, 10) | ||
| .toUpperCase(); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# 1. OrderConverter.java 파일 확인 및 라인 32-36 검증
cat -n src/main/java/com/ongil/backend/domain/order/converter/OrderConverter.java | sed -n '25,45p'Repository: IT-Cotato/12th-OnGil-BE
Length of output: 819
🏁 Script executed:
# 2. Order 엔티티 조회 - orderNumber 컬럼 정의 및 unique 제약 확인
find . -type f -name "Order.java" | grep -E "entity|domain" | head -5Repository: IT-Cotato/12th-OnGil-BE
Length of output: 131
🏁 Script executed:
# 3. 데이터베이스 마이그레이션/스키마 파일 검색
fd -t f "\.sql$|\.xml$" | grep -iE "schema|migration|flyway|liquibase" | head -20Repository: IT-Cotato/12th-OnGil-BE
Length of output: 49
🏁 Script executed:
# Read Order entity to check orderNumber field and constraints
cat -n src/main/java/com/ongil/backend/domain/order/entity/Order.javaRepository: IT-Cotato/12th-OnGil-BE
Length of output: 4037
🏁 Script executed:
# Search for database schema/JPA annotations related to orderNumber constraint
rg -n "orderNumber|order_number" src/main/java/com/ongil/backend/domain/order/entity/Order.java -A 3 -B 1Repository: IT-Cotato/12th-OnGil-BE
Length of output: 273
🏁 Script executed:
# Look for application properties or schema generation config
find . -type f \( -name "application*.yml" -o -name "application*.properties" -o -name "*.sql" \) | head -10Repository: IT-Cotato/12th-OnGil-BE
Length of output: 225
🏁 Script executed:
# Check if there's any retry logic or exception handling in OrderController/OrderService
rg -n "orderNumber|OrderConverter" src/main/java/com/ongil/backend/domain/order/service/ -A 5 -B 2 | head -50Repository: IT-Cotato/12th-OnGil-BE
Length of output: 2142
🏁 Script executed:
# Check application.yml for any relevant config
cat src/main/resources/application.ymlRepository: IT-Cotato/12th-OnGil-BE
Length of output: 1243
주문번호 생성의 고유성 위반 위험: 데이터베이스 제약 충돌 발생 가능
UUID(128비트)를 hex 10자(40비트)로 절삭하면 약 1조 개 조합만 남습니다. Order 엔티티에 @Column(unique = true) 제약이 설정되어 있으므로, 약 3천만 건의 주문이 누적되면 충돌 확률이 50%에 도달하고, 이는 DataIntegrityViolationException으로 이어져 주문 생성이 실패합니다. 현재 코드에는 충돌 시 재시도 로직이 없으므로 프로덕션 환경에서 안정성 문제가 됩니다.
개선 방안:
- UUID 전체(32자) 또는 더 긴 substring 사용
- DB sequence 기반 주문번호 생성
- 충돌 발생 시 재시도 로직 추가 (제한된 횟수)
🤖 Prompt for AI Agents
In `@src/main/java/com/ongil/backend/domain/order/converter/OrderConverter.java`
around lines 32 - 36, The current OrderConverter uses
UUID.randomUUID().toString().replace("-", "").substring(0, 10).toUpperCase() to
build orderNumber which is too short and risks unique-constraint collisions;
update OrderConverter to stop truncating to 10 chars (use the full 32-char hex
string or a longer substring) OR implement a bounded retry safe-generation: in
the orderNumber generation path (the code around orderNumber, UUID.randomUUID,
replace, substring) loop to regenerate and check uniqueness via the Order
repository (e.g., existsByOrderNumber) with a limited number of attempts and
throw a clear exception if exceeded; ensure the final chosen approach removes
the unsafe substring(0, 10) usage or adds the retry+repository check to prevent
DataIntegrityViolationException.
src/main/java/com/ongil/backend/domain/review/controller/ReviewController.java
Show resolved
Hide resolved
| @Operation(summary = "리뷰 사진 업로드", description = "리뷰용 사진을 최대 5장까지 S3에 업로드합니다.") | ||
| @PostMapping(value = "/api/reviews/images", consumes = MediaType.MULTIPART_FORM_DATA_VALUE) | ||
| public DataResponse<List<String>> uploadReviewImages( | ||
| @RequestPart("images") List<MultipartFile> images | ||
| ) { | ||
| if (images.size() > 5) { | ||
| throw new ValidationException(ErrorCode.INVALID_PARAMETER); | ||
| } | ||
|
|
||
| List<String> imageUrls = images.stream() | ||
| .map(s3ImageService::uploadReviewImage) | ||
| .toList(); | ||
| return DataResponse.from(imageUrls); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "ReviewController.java" -type fRepository: IT-Cotato/12th-OnGil-BE
Length of output: 147
🏁 Script executed:
# ReviewController 파일의 전체 구조 확인
git ls-files "*ReviewController.java"Repository: IT-Cotato/12th-OnGil-BE
Length of output: 145
🏁 Script executed:
# Spring Security 설정 파일 탐색
fd "Security|Config" --type f -e java | head -20Repository: IT-Cotato/12th-OnGil-BE
Length of output: 600
🏁 Script executed:
# /api/reviews 경로에 대한 보안 설정 확인
rg "reviews" -t java | head -30Repository: IT-Cotato/12th-OnGil-BE
Length of output: 4010
🏁 Script executed:
cat -n src/main/java/com/ongil/backend/global/config/SecurityConfig.javaRepository: IT-Cotato/12th-OnGil-BE
Length of output: 4867
🏁 Script executed:
# Read ReviewController to check all write endpoints and auth patterns
cat -n src/main/java/com/ongil/backend/domain/review/controller/ReviewController.javaRepository: IT-Cotato/12th-OnGil-BE
Length of output: 9864
이미지 업로드 엔드포인트에서 userId 파라미터가 누락되었습니다.
Spring Security의 catch-all 규칙(.anyRequest().authenticated())으로 경로가 보호되고 있으나, 컨트롤러 메서드에 @AuthenticationPrincipal Long userId 파라미터가 없습니다. 이로 인해 인증된 사용자를 구분할 수 없으며, 다른 모든 write 엔드포인트(/api/reviews/init, /api/reviews/{reviewId}/step1 등)와 일관성이 깨집니다. 업로드된 이미지를 사용자와 연결하려면 반드시 필요합니다.
💡 수정 제안
public DataResponse<List<String>> uploadReviewImages(
+ `@AuthenticationPrincipal` Long userId,
`@RequestPart`("images") List<MultipartFile> images
) {🤖 Prompt for AI Agents
In
`@src/main/java/com/ongil/backend/domain/review/controller/ReviewController.java`
around lines 176 - 189, The uploadReviewImages method lacks the authenticated
user id parameter; update the controller method uploadReviewImages to accept
`@AuthenticationPrincipal` Long userId and pass that userId into the S3 upload
flow (use the appropriate s3ImageService.uploadReviewImage overload or update it
to accept userId and the MultipartFile) so uploaded images are associated with
the authenticated user, and keep the same validation (max 5) and return behavior
as before to maintain consistency with other endpoints like init/step1.
| private ReviewListResponse.AnswerSummary buildAnswerSummary(Review review) { | ||
| return ReviewListResponse.AnswerSummary.builder() | ||
| .sizeAnswer(review.getSizeAnswer()) | ||
| .colorAnswer(review.getColorAnswer()) | ||
| .materialAnswer(review.getMaterialAnswer()) | ||
| .sizeAnswer(review.getSizeAnswer().getDisplayName()) | ||
| .colorAnswer(review.getColorAnswer().getDisplayName()) | ||
| .materialAnswer(review.getMaterialAnswer().getDisplayName()) | ||
| .build(); | ||
| } |
There was a problem hiding this comment.
getDisplayName() 호출 시 Enum 필드가 null이면 NPE 발생
Line 70-72에서 review.getSizeAnswer(), getColorAnswer(), getMaterialAnswer()가 null인 경우 NPE가 발생합니다. 동일한 패턴이 buildInitialFirstAnswers (Line 143-145)와 buildMyReviewAnswerSummary (Line 208-210)에도 반복됩니다.
COMPLETED 상태의 리뷰만 조회된다면 문제없겠지만, 데이터 마이그레이션이나 스케줄러의 DRAFT 삭제 타이밍 등 엣지 케이스에서 null이 들어올 수 있습니다.
🛡️ null-safe 처리 예시
private ReviewListResponse.AnswerSummary buildAnswerSummary(Review review) {
return ReviewListResponse.AnswerSummary.builder()
- .sizeAnswer(review.getSizeAnswer().getDisplayName())
- .colorAnswer(review.getColorAnswer().getDisplayName())
- .materialAnswer(review.getMaterialAnswer().getDisplayName())
+ .sizeAnswer(review.getSizeAnswer() != null ? review.getSizeAnswer().getDisplayName() : null)
+ .colorAnswer(review.getColorAnswer() != null ? review.getColorAnswer().getDisplayName() : null)
+ .materialAnswer(review.getMaterialAnswer() != null ? review.getMaterialAnswer().getDisplayName() : null)
.build();
}📝 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.
| private ReviewListResponse.AnswerSummary buildAnswerSummary(Review review) { | |
| return ReviewListResponse.AnswerSummary.builder() | |
| .sizeAnswer(review.getSizeAnswer()) | |
| .colorAnswer(review.getColorAnswer()) | |
| .materialAnswer(review.getMaterialAnswer()) | |
| .sizeAnswer(review.getSizeAnswer().getDisplayName()) | |
| .colorAnswer(review.getColorAnswer().getDisplayName()) | |
| .materialAnswer(review.getMaterialAnswer().getDisplayName()) | |
| .build(); | |
| } | |
| private ReviewListResponse.AnswerSummary buildAnswerSummary(Review review) { | |
| return ReviewListResponse.AnswerSummary.builder() | |
| .sizeAnswer(review.getSizeAnswer() != null ? review.getSizeAnswer().getDisplayName() : null) | |
| .colorAnswer(review.getColorAnswer() != null ? review.getColorAnswer().getDisplayName() : null) | |
| .materialAnswer(review.getMaterialAnswer() != null ? review.getMaterialAnswer().getDisplayName() : null) | |
| .build(); | |
| } |
🤖 Prompt for AI Agents
In `@src/main/java/com/ongil/backend/domain/review/converter/ReviewConverter.java`
around lines 68 - 74, The three methods buildAnswerSummary,
buildInitialFirstAnswers, and buildMyReviewAnswerSummary call getDisplayName()
on enum fields (sizeAnswer, colorAnswer, materialAnswer) which can be null and
cause NPEs; update each method to null-safe access by checking each
review.getSizeAnswer()/getColorAnswer()/getMaterialAnswer() before calling
getDisplayName() and pass a safe default (e.g., null or a constant like
"UNKNOWN" or empty string) into the ReviewListResponse.AnswerSummary builder
when the enum is null (use ternary/Optional or helper method to map enum ->
display string safely), ensuring every builder field (sizeAnswer, colorAnswer,
materialAnswer) is populated without invoking getDisplayName() on a null
reference.
src/main/java/com/ongil/backend/domain/review/converter/ReviewWriteConverter.java
Show resolved
Hide resolved
src/main/java/com/ongil/backend/domain/review/service/prompter/SizeReviewPrompter.java
Show resolved
Hide resolved
src/main/java/com/ongil/backend/domain/review/service/ReviewCommandService.java
Show resolved
Hide resolved
| @Transactional | ||
| public void submitReview(Long userId, Long reviewId, ReviewFinalSubmitRequest request) { | ||
| Review review = getReviewOrThrow(reviewId); | ||
| User user = getUserOrThrow(userId); | ||
|
|
||
| reviewValidator.validateReviewAuthority(review.getOrderItem(), userId); | ||
|
|
||
| String joinedSizeReview = (request.getSizeReview() != null && !request.getSizeReview().isEmpty()) | ||
| ? String.join("\n", request.getSizeReview()) : null; | ||
|
|
||
| String joinedMaterialReview = (request.getMaterialReview() != null && !request.getMaterialReview().isEmpty()) | ||
| ? String.join("\n", request.getMaterialReview()) : null; | ||
|
|
||
| String joinedImages = (request.getReviewImageUrls() != null && !request.getReviewImageUrls().isEmpty()) | ||
| ? String.join(",", request.getReviewImageUrls()) : null; | ||
|
|
||
| int rewardAmount = 500; | ||
| review.submit( | ||
| request.getTextReview(), | ||
| joinedImages, | ||
| joinedSizeReview, | ||
| joinedMaterialReview, | ||
| rewardAmount | ||
| ); | ||
|
|
||
| user.restorePoints(rewardAmount); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find ReviewValidator class
find . -name "ReviewValidator*" -type fRepository: IT-Cotato/12th-OnGil-BE
Length of output: 145
🏁 Script executed:
# Search for step validation methods
rg -n "validateStep" --type=java -A 2Repository: IT-Cotato/12th-OnGil-BE
Length of output: 49
🏁 Script executed:
# Search for error codes mentioned
rg -n "REVIEW_STEP1_INCOMPLETE|REVIEW_STEP2_INCOMPLETE" --type=javaRepository: IT-Cotato/12th-OnGil-BE
Length of output: 984
🏁 Script executed:
# Check Review entity for step tracking
rg -n "step1|step2|Step1|Step2" --type=java -i | head -50Repository: IT-Cotato/12th-OnGil-BE
Length of output: 7051
🏁 Script executed:
# Verify hardcoded 500 usage in reward
rg -n "500|rewardAmount" --type=java | grep -i rewardRepository: IT-Cotato/12th-OnGil-BE
Length of output: 396
🏁 Script executed:
# Examine ReviewValidator class to see actual validation methods
cat -n ./src/main/java/com/ongil/backend/domain/review/validator/ReviewValidator.javaRepository: IT-Cotato/12th-OnGil-BE
Length of output: 2725
🏁 Script executed:
# Check Review entity for completion status fields
cat -n ./src/main/java/com/ongil/backend/domain/review/entity/Review.java | head -100Repository: IT-Cotato/12th-OnGil-BE
Length of output: 3678
🏁 Script executed:
# Check if reward amount is defined as a constant elsewhere
rg -n "500|REWARD|reward" --type=java src/main/java/com/ongil/backend/domain/review | head -30Repository: IT-Cotato/12th-OnGil-BE
Length of output: 396
🏁 Script executed:
# Check the full submitReview method and surrounding context
cat -n ./src/main/java/com/ongil/backend/domain/review/service/ReviewCommandService.java | sed -n '120,160p'Repository: IT-Cotato/12th-OnGil-BE
Length of output: 1812
🏁 Script executed:
# Check how submitReview is called from the controller
rg -n "submitReview" --type=java -B 3 -A 3Repository: IT-Cotato/12th-OnGil-BE
Length of output: 2251
🏁 Script executed:
# Check Review entity's submit method to see what it does
rg -n "public void submit" --type=java -A 10Repository: IT-Cotato/12th-OnGil-BE
Length of output: 2746
🏁 Script executed:
# Look for any configuration files or constants related to rewards
rg -n "reward|REWARD" --type=java | grep -i "const\|final\|config"Repository: IT-Cotato/12th-OnGil-BE
Length of output: 49
Step 완료 검증 누락 및 매직 넘버 리워드 금액
두 가지 문제가 있습니다:
-
Step 완료 검증 누락:
submitReview에서 작성 권한만 검증하고, 실제로 Step1(sizeAnswer, materialAnswer)과 Step2(fitIssueParts, materialFeatures)가 완료되었는지 확인하지 않습니다.ReviewValidator에validateReviewStepCompletion메서드가 존재하지만 호출되지 않아, 불완전한 리뷰가 제출될 수 있습니다. -
매직 넘버: Line 142의
rewardAmount = 500은 하드코딩된 값입니다. 상수로 분리하여 유지보수성을 높여야 합니다.
제안 수정
public void submitReview(Long userId, Long reviewId, ReviewFinalSubmitRequest request) {
Review review = getReviewOrThrow(reviewId);
User user = getUserOrThrow(userId);
reviewValidator.validateReviewAuthority(review.getOrderItem(), userId);
+ reviewValidator.validateReviewStepCompletion(review.getSizeAnswer(), review.getFitIssueParts());
+ reviewValidator.validateReviewStepCompletion(review.getMaterialAnswer(), review.getMaterialFeatures());📝 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.
| @Transactional | |
| public void submitReview(Long userId, Long reviewId, ReviewFinalSubmitRequest request) { | |
| Review review = getReviewOrThrow(reviewId); | |
| User user = getUserOrThrow(userId); | |
| reviewValidator.validateReviewAuthority(review.getOrderItem(), userId); | |
| String joinedSizeReview = (request.getSizeReview() != null && !request.getSizeReview().isEmpty()) | |
| ? String.join("\n", request.getSizeReview()) : null; | |
| String joinedMaterialReview = (request.getMaterialReview() != null && !request.getMaterialReview().isEmpty()) | |
| ? String.join("\n", request.getMaterialReview()) : null; | |
| String joinedImages = (request.getReviewImageUrls() != null && !request.getReviewImageUrls().isEmpty()) | |
| ? String.join(",", request.getReviewImageUrls()) : null; | |
| int rewardAmount = 500; | |
| review.submit( | |
| request.getTextReview(), | |
| joinedImages, | |
| joinedSizeReview, | |
| joinedMaterialReview, | |
| rewardAmount | |
| ); | |
| user.restorePoints(rewardAmount); | |
| } | |
| `@Transactional` | |
| public void submitReview(Long userId, Long reviewId, ReviewFinalSubmitRequest request) { | |
| Review review = getReviewOrThrow(reviewId); | |
| User user = getUserOrThrow(userId); | |
| reviewValidator.validateReviewAuthority(review.getOrderItem(), userId); | |
| reviewValidator.validateReviewStepCompletion(review.getSizeAnswer(), review.getFitIssueParts()); | |
| reviewValidator.validateReviewStepCompletion(review.getMaterialAnswer(), review.getMaterialFeatures()); | |
| String joinedSizeReview = (request.getSizeReview() != null && !request.getSizeReview().isEmpty()) | |
| ? String.join("\n", request.getSizeReview()) : null; | |
| String joinedMaterialReview = (request.getMaterialReview() != null && !request.getMaterialReview().isEmpty()) | |
| ? String.join("\n", request.getMaterialReview()) : null; | |
| String joinedImages = (request.getReviewImageUrls() != null && !request.getReviewImageUrls().isEmpty()) | |
| ? String.join(",", request.getReviewImageUrls()) : null; | |
| int rewardAmount = 500; | |
| review.submit( | |
| request.getTextReview(), | |
| joinedImages, | |
| joinedSizeReview, | |
| joinedMaterialReview, | |
| rewardAmount | |
| ); | |
| user.restorePoints(rewardAmount); | |
| } |
🤖 Prompt for AI Agents
In
`@src/main/java/com/ongil/backend/domain/review/service/ReviewCommandService.java`
around lines 126 - 152, Call the existing step-completion validator before
submitting and remove the hardcoded reward: invoke
reviewValidator.validateReviewStepCompletion(...) (pass the Review or the
Review's orderItem and the ReviewFinalSubmitRequest as required by the existing
method signature) right after validateReviewAuthority and before assembling
joined fields, then replace the literal 500 with a class-level constant (e.g.,
private static final int REVIEW_REWARD_AMOUNT = 500) and use
REVIEW_REWARD_AMOUNT for both review.submit(...) and user.restorePoints(...).
| import com.theokanning.openai.service.OpenAiService; | ||
|
|
||
| @Configuration | ||
| public class OpenAiConfig { | ||
| @Value("${openai.api-key}") | ||
| private String apiKey; | ||
|
|
||
| @Bean | ||
| public OpenAiService openAiService() { | ||
| return new OpenAiService(apiKey, Duration.ofSeconds(60)); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's find and read the full OpenAiConfig.java file
find . -name "OpenAiConfig.java" -type fRepository: IT-Cotato/12th-OnGil-BE
Length of output: 132
🌐 Web query:
theokanning openai-java library status 2025 archived deprecated
💡 Result:
- TheoKanning
openai-javastatus (in 2025): Archived / unmaintained. The GitHub repo was archived on June 6, 2024 and is read-only; the README states it is “no longer maintained” and will receive no further updates or support. [1] - “Deprecated” angle: The library isn’t “deprecated by OpenAI” per se, but it contains/mentions OpenAI API areas that OpenAI deprecated (notably engine-based endpoints). [1][2]
- Artifacts still exist but won’t be updated: Maven coordinates under
com.theokanning.openai-gpt3-javaremain published, but the project is flagged as archived/unmaintained. [3] - Recommended path forward: Migrate to the official OpenAI Java SDK (
com.openai:openai-java). [4]
Sources: [1] TheoKanning/openai-java GitHub repo (archived notice + README) • [2] OpenAI API deprecations docs • [3] Libraries.io Maven page for com.theokanning.openai-gpt3-java:service • [4] openai/openai-java GitHub repo (official SDK)
🌐 Web query:
OpenAI official Java SDK 2025
💡 Result:
OpenAI does have an official Java SDK (listed as “currently in beta” in the official OpenAI API docs). The docs show the Maven coordinates as:
groupId:com.openaiartifactId:openai-java- Example version shown:
4.0.0
and a basic usage example viaOpenAIOkHttpClient.fromEnv()and the Responses API. [1]
OpenAI also announced new official Go and Java SDKs (beta) as part of its “OpenAI o1 and new tools for developers” release post. [2]
Sources:
[1] OpenAI API docs — “Libraries” page (Java section)
[2] OpenAI blog — “OpenAI o1 and new tools for developers” (Go and Java SDKs in beta)
🏁 Script executed:
# Read the full OpenAiConfig.java file to check for upload method and other details
cat -n ./src/main/java/com/ongil/backend/global/config/OpenAiConfig.javaRepository: IT-Cotato/12th-OnGil-BE
Length of output: 712
theokanning/openai-java 라이브러리는 2024년 6월부터 아카이브 상태이며, 공식 OpenAI Java SDK로의 마이그레이션 권장
com.theokanning.openai 라이브러리는 2024년 6월 6일에 아카이브되었으며 더 이상 유지보수되지 않습니다. OpenAI 공식 Java SDK(com.openai:openai-java, 현재 베타)로의 마이그레이션을 고려해 주세요.
또한 @Value("${openai.api-key}")로 주입된 apiKey에 대한 검증이 필요합니다. 프로퍼티가 누락되면 Spring 시작 시 실패하지만, 명시적으로 빈 문자열("")이 설정된 경우 통과하게 됩니다. OpenAiService 생성 전에 null 및 빈 문자열 체크를 권장합니다.
🤖 Prompt for AI Agents
In `@src/main/java/com/ongil/backend/global/config/OpenAiConfig.java` around lines
9 - 19, The OpenAiConfig currently uses the deprecated com.theokanning.openai
library and doesn't validate the injected apiKey; update OpenAiConfig to use the
official OpenAI Java SDK (com.openai:openai-java) instead of
com.theokanning.openai.service.OpenAiService and change the openAiService() bean
to construct the new SDK client, and before construction validate the apiKey
field (the apiKey member and inside openAiService()) to ensure it's not null or
empty and throw a clear IllegalStateException (or similar) if invalid; update
imports and any timeout/configuration usage to the API of the new SDK when
replacing the OpenAiService instantiation.
| @RequiredArgsConstructor | ||
| @Transactional(readOnly = true) | ||
| public class ReviewService { | ||
| public class ReviewQueryService { |
There was a problem hiding this comment.
리뷰 기능을 CQRS 패턴으로 나눈부분 좋은거 같아요!
| Object key = row[0]; | ||
| String answerLabel; | ||
|
|
||
| if (key instanceof SizeAnswer size) answerLabel = size.getDisplayName(); |
There was a problem hiding this comment.
답변 마다 enum 따로 만들었어야했는데 제가 하기 애매해서 안했었는데 만든 부분 좋은거 같습니당 !
|
CQRS 패턴으로 리뷰 서비스 코드 나눈 부분 좋은거 같습니다 !! 리뷰 생성 기능 까다로웠을텐데 수고하셨습니당 ~~ |
2 participants
🔍️ 작업 내용
✨ 상세 설명
ClothingCategory및 사용자 응답(Size/Material)에 따라 시스템/유저 메시지를 동적으로 구성하는 로직 구현.|기호로 강제하여 서버에서List<String>으로 안전하게 파싱 후, 프론트엔드에 선택지로 제공하는 구조 설계.DRAFT상태인 임시 저장 리뷰를 자동 삭제하는ReviewCleanupScheduler구현.🛠️ 추후 리팩토링 및 고도화 계획
📸 스크린샷 (선택)
💬 리뷰 요구사항
ReviewCommandService와ReviewQueryService로 분리CONFIRMED/CANCLED2개로 축소하고 관련 로직 수정Summary by CodeRabbit
New Features
Bug Fixes
Chores