[FEAT] 푸드트럭과 메뉴 이미지 presiged URL 발급 api 구현#37
Conversation
…-url # Conflicts: # src/main/java/konkuk/chacall/domain/foodtruck/application/FoodTruckService.java # src/main/java/konkuk/chacall/domain/foodtruck/presentation/FoodTruckController.java
Walkthrough푸드트럭·메뉴 이미지 업로드용 presigned URL 생성 기능을 추가했습니다. S3 presigner 기반 URL 생성 로직(S3Service), 이미지 키 생성(KeyUtils), 확장자 검증(AllowedFileExtension), FoodTruckImageService와 FoodTruckController 엔드포인트/DTO, 관련 오류 코드 및 Swagger 응답 설명이 도입되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client
participant Controller
participant FoodTruckService
participant OwnerValidator
participant ImageService
participant AllowedExt
participant KeyUtils
participant S3Service
participant S3Presigner
Client->>Controller: POST /food-trucks/images or /food-trucks/menus/images (ImageRequest)
Controller->>FoodTruckService: create{FoodTruck|Menu}ImagePresignedUrl(request, ownerId)
FoodTruckService->>OwnerValidator: validateOwner(ownerId)
OwnerValidator-->>FoodTruckService: User(owner)
FoodTruckService->>ImageService: create{FoodTruck|Menu}ImagePresignedUrl(request, owner)
ImageService->>AllowedExt: checkAllowedExtension(request.fileExtensions)
alt food-truck images (max 9)
ImageService->>KeyUtils: buildFoodTruckImageKey(owner.id) x N
else menu images
ImageService->>KeyUtils: buildMenuImageKey(owner.id) x N
end
loop per image
ImageService->>S3Service: generatePresignedUrl(key)
S3Service->>S3Presigner: presign PUT (10min)
S3Presigner-->>S3Service: presigned URL
S3Service-->>ImageService: presignedUrl + fileUrl
end
ImageService-->>FoodTruckService: ImageResponse(presignedUrls)
FoodTruckService-->>Controller: ImageResponse
Controller-->>Client: BaseResponse<ImageResponse>
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ 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: 8
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/konkuk/chacall/global/config/S3Config.java (2)
23-28: S3Client와 S3Presigner의 리전 설정이 불일치합니다.
S3Client빈은 하드코딩된Region.AP_NORTHEAST_2를 사용하고,S3Presigner빈은 주입된region변수를 사용합니다. 일관성을 위해 두 빈 모두 동일한 리전 소스를 사용해야 합니다.다음 diff를 적용하여 일관성을 확보하세요:
@Bean public S3Client s3Client() { return S3Client.builder() - .region(Region.AP_NORTHEAST_2) + .region(Region.of(region)) .credentialsProvider(EnvironmentVariableCredentialsProvider.create()) .build(); }
23-28: 리전 설정 불일치를 수정하세요.S3Client 빈은
Region.AP_NORTHEAST_2를 하드코딩하고 있지만(line 25), S3Presigner 빈은 주입된region변수를 사용합니다(line 33). 일관성을 위해 두 빈 모두 동일한 리전 설정 방식을 사용해야 합니다.다음 diff를 적용하여 S3Client도 주입된 region을 사용하도록 수정하세요:
@Bean public S3Client s3Client() { return S3Client.builder() - .region(Region.AP_NORTHEAST_2) + .region(Region.of(region)) .credentialsProvider(EnvironmentVariableCredentialsProvider.create()) .build(); }Also applies to: 30-36
🧹 Nitpick comments (3)
src/main/java/konkuk/chacall/global/common/storage/util/KeyUtils.java (1)
24-40: 코드 중복 제거를 고려해보세요
buildFoodTruckImageKey와buildMenuImageKey메서드가 거의 동일한 로직을 포함하고 있습니다. prefix와 id를 파라미터로 받는 private 헬퍼 메서드를 추출하여 중복을 제거할 수 있습니다.+private static String buildImageKey(String prefix, Long id) { + if (id == null) { + throw new IllegalArgumentException("id는 null일 수 없습니다."); + } + String timestamp = LocalDateTime.now().format(DATE_TIME_FORMATTER); + String uuid = UUID.randomUUID().toString(); + return String.format("%s/%d/%s/%s", prefix, id, timestamp, uuid); +} + public static String buildFoodTruckImageKey(Long userId) { - String timestamp = LocalDateTime.now().format(DATE_TIME_FORMATTER); - String uuid = UUID.randomUUID().toString(); - return String.format("foodtrucks/%d/%s/%s", - userId, - timestamp, - uuid); + return buildImageKey("foodtrucks", userId); } public static String buildMenuImageKey(Long foodTruckId) { - String timestamp = LocalDateTime.now().format(DATE_TIME_FORMATTER); - String uuid = UUID.randomUUID().toString(); - return String.format("menus/%d/%s/%s", - foodTruckId, - timestamp, - uuid); + return buildImageKey("menus", foodTruckId); }src/main/java/konkuk/chacall/global/common/storage/util/AllowedFileExtension.java (1)
30-31: 예외 wrapping 패턴 재검토 필요
BusinessException에IllegalArgumentException을 cause로 wrapping하는 것은 일반적이지 않은 패턴입니다.BusinessException은 비즈니스 로직 오류를 나타내므로, cause 없이 직접 메시지나 ErrorCode만으로 던지는 것이 더 적절합니다.if (!isMatch) { - throw new BusinessException(ErrorCode.INVALID_FILE_EXTENSION, - new IllegalArgumentException("허용되지 않은 파일 확장자: " + extension)); + throw new BusinessException(ErrorCode.INVALID_FILE_EXTENSION); }ErrorCode에 메시지 포맷을 포함시키거나, BusinessException이 동적 메시지를 지원하도록 개선하는 것도 고려할 수 있습니다.
src/main/java/konkuk/chacall/domain/foodtruck/presentation/dto/response/ImageResponse.java (1)
11-13: 정적 팩토리 메서드가 선택적입니다.Record는 이미 모든 필드를 받는 정규 생성자를 제공하므로,
of()정적 팩토리 메서드는 추가적인 가치를 제공하지 않습니다. 생성자를 직접 사용해도 무방하지만, 팀의 코드 컨벤션에 따라 일관성을 위해 유지할 수도 있습니다.만약 제거하려면:
- public static ImageResponse of(List<String> presignedUrls) { - return new ImageResponse(presignedUrls); - }호출부에서는
ImageResponse.of(urls)대신new ImageResponse(urls)를 사용하면 됩니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
src/main/java/konkuk/chacall/domain/foodtruck/application/FoodTruckService.java(3 hunks)src/main/java/konkuk/chacall/domain/foodtruck/application/image/FoodTruckImageService.java(1 hunks)src/main/java/konkuk/chacall/domain/foodtruck/presentation/FoodTruckController.java(2 hunks)src/main/java/konkuk/chacall/domain/foodtruck/presentation/dto/request/ImageRequest.java(1 hunks)src/main/java/konkuk/chacall/domain/foodtruck/presentation/dto/response/ImageResponse.java(1 hunks)src/main/java/konkuk/chacall/global/common/exception/code/ErrorCode.java(1 hunks)src/main/java/konkuk/chacall/global/common/storage/S3Service.java(3 hunks)src/main/java/konkuk/chacall/global/common/storage/util/AllowedFileExtension.java(1 hunks)src/main/java/konkuk/chacall/global/common/storage/util/KeyUtils.java(2 hunks)src/main/java/konkuk/chacall/global/common/swagger/SwaggerResponseDescription.java(1 hunks)src/main/java/konkuk/chacall/global/config/S3Config.java(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/java/konkuk/chacall/domain/foodtruck/application/image/FoodTruckImageService.java (1)
src/main/java/konkuk/chacall/domain/foodtruck/application/FoodTruckService.java (1)
RequiredArgsConstructor(19-56)
🔇 Additional comments (14)
src/main/java/konkuk/chacall/global/config/S3Config.java (2)
18-20: 버킷 설정이 적절하게 구성되었습니다.필드 레벨
@Getter와@Value주입을 통해 버킷 이름을 외부 설정으로 관리하고 있어 좋습니다.
30-36: S3Presigner 빈 설정이 올바릅니다.환경 변수 기반 인증과 주입된 리전을 사용하여 presigner를 구성하고 있어 적절합니다.
src/main/java/konkuk/chacall/domain/foodtruck/presentation/FoodTruckController.java (4)
60-71: 푸드트럭 이미지 presigned URL 엔드포인트가 잘 구현되었습니다.엔드포인트 설계가 명확하고 보안 어노테이션(
@UserId)과 유효성 검증(@Valid)이 적절하게 적용되었습니다.참고: SwaggerResponseDescription의 오타 수정 후 line 64의
GET_FOOD_TRUCK_PRESGIEND_URL을GET_FOOD_TRUCK_PRESIGNED_URL로 업데이트해야 합니다.
73-84: 메뉴 이미지 presigned URL 엔드포인트가 잘 구현되었습니다.엔드포인트 설계에 대한 의견: 현재 분리된 엔드포인트 접근 방식(
/food-trucks/images와/food-trucks/menus/images)이 적절합니다. 이유는:
- 리소스 지향적: 각 엔드포인트가 명확한 리소스(푸드트럭 이미지 vs 메뉴 이미지)를 나타냅니다.
- 확장성: 향후 각 이미지 타입별로 다른 제약사항(개수 제한, 크기 등)을 적용하기 용이합니다.
- 명확성: API 사용자가 용도를 즉시 파악할 수 있습니다.
단일 엔드포인트에 타입 파라미터를 사용하는 방식보다 현재 방식이 RESTful 원칙에 더 부합합니다.
참고: SwaggerResponseDescription의 오타 수정 후 line 77의
GET_MENU_PRESGIEND_URL을GET_MENU_PRESIGNED_URL로 업데이트해야 합니다.
60-71: 인증 및 유효성 검증이 올바르게 적용되었습니다.presigned URL 발급 엔드포인트에
@UserId인증과@Valid요청 검증이 적절히 적용되어 있습니다.
64-64: SwaggerResponseDescription enum 상수명 수정 후 이 참조를 업데이트하세요.
SwaggerResponseDescription의 enum 상수명이 수정되면 (PRESGIEND → PRESIGNED), 여기의@ExceptionDescription어노테이션도 함께 업데이트해야 합니다.다음 diff를 적용하세요:
- @ExceptionDescription(SwaggerResponseDescription.GET_FOOD_TRUCK_PRESGIEND_URL) + @ExceptionDescription(SwaggerResponseDescription.GET_FOOD_TRUCK_PRESIGNED_URL)- @ExceptionDescription(SwaggerResponseDescription.GET_MENU_PRESGIEND_URL) + @ExceptionDescription(SwaggerResponseDescription.GET_MENU_PRESIGNED_URL)Also applies to: 77-77
src/main/java/konkuk/chacall/domain/foodtruck/presentation/dto/response/ImageResponse.java (1)
1-14: ImageResponse DTO가 깔끔하게 구현되었습니다.불변 record 타입과 정적 팩토리 메서드를 사용한 설계가 적절하며, Swagger 문서화도 잘 되어 있습니다.
src/main/java/konkuk/chacall/global/common/exception/code/ErrorCode.java (2)
82-83: 이미지 관련 에러 코드가 적절하게 추가되었습니다.에러 코드 번호(115xxx)와 메시지가 명확하며, 기존 패턴과 일관성을 유지하고 있습니다.
82-83: 새로운 에러 코드가 적절합니다.이미지 유효성 검증을 위한 에러 코드가 올바르게 정의되었습니다. HTTP 상태 코드(BAD_REQUEST)와 에러 메시지가 적절합니다.
src/main/java/konkuk/chacall/global/common/storage/util/AllowedFileExtension.java (1)
9-16: SVG 및 PDF 파일 허용에 따른 보안 위험 검토 필요SVG 파일은 스크립트를 포함할 수 있어 XSS 공격의 벡터가 될 수 있으며, PDF 파일은 이미지가 아닌 문서 형식입니다.
- SVG: 업로드를 허용할 경우 sanitization 처리 또는 Content-Type 헤더를
image/svg+xml이 아닌text/plain으로 제공하는 등의 추가 보안 조치가 필요합니다.- PDF: 메뉴 이미지 업로드에 PDF가 필요한지 확인하고, 불필요하다면 제거를 권장합니다.
presigned URL 생성 시 또는 S3 버킷 정책에서 Content-Type이 적절히 제한되는지 확인해주세요.
src/main/java/konkuk/chacall/domain/foodtruck/application/FoodTruckService.java (1)
43-53: LGTM! 적절한 검증과 위임 패턴Owner 검증 후
FoodTruckImageService로 위임하는 구조가 명확하고 적절합니다. 트랜잭션 read-only 설정도 presigned URL 생성 로직에 적합합니다.src/main/java/konkuk/chacall/global/common/storage/S3Service.java (2)
52-64: Content-Type 제한 없는 presigned URL 보안 검토현재 presigned URL 생성 시 Content-Type에 대한 제한이 없습니다. 이는 클라이언트가 허용되지 않은 파일 타입을 업로드할 수 있는 위험이 있습니다.
AllowedFileExtension에서 검증한 확장자에 해당하는 Content-Type만 업로드하도록 제한하려면, PutObjectRequest에contentType을 지정하거나 S3 버킷 정책에서 제한하는 것을 고려해야 합니다.클라이언트가 presigned URL로 업로드할 때 Content-Type 헤더를 올바르게 설정하는지, 그리고 서버 측에서 이를 강제할 방법이 있는지 확인해주세요.
27-27: 매직 넘버를 상수로 정의한 점 좋습니다presigned URL 만료 시간을 상수로 정의하여 가독성과 유지보수성을 높인 점이 좋습니다.
src/main/java/konkuk/chacall/domain/foodtruck/presentation/dto/request/ImageRequest.java (1)
9-14: 기본 검증 어노테이션 적절히 사용
@NotNull과@Size(min=1)을 사용하여 필수 입력과 최소 개수를 검증하는 것은 적절합니다. Swagger 문서화도 잘 되어 있습니다.
| public record ImageRequest( | ||
| @Schema(description = "파일 확장자 리스트", example = "[\"png\", \"jpg\"]") | ||
| @NotNull(message = "파일 확장자는 필수입니다.") | ||
| @Size(min = 1, message = "최소 하나 이상의 파일 확장자를 제공해야 합니다.") | ||
| List<String> fileExtensions | ||
| ) { |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
최대 이미지 개수 제한 누락
@Size(min = 1) 어노테이션으로 최소 개수는 검증하지만, 최대 개수 제한이 없습니다. ErrorCode에 INVALID_IMAGE_COUNT가 추가된 것으로 보아 개수 제한이 필요한 것으로 보입니다.
DOS 공격이나 과도한 리소스 사용을 방지하기 위해 최대 개수 제한을 추가하는 것을 권장합니다.
public record ImageRequest(
@Schema(description = "파일 확장자 리스트", example = "[\"png\", \"jpg\"]")
@NotNull(message = "파일 확장자는 필수입니다.")
- @Size(min = 1, message = "최소 하나 이상의 파일 확장자를 제공해야 합니다.")
+ @Size(min = 1, max = 10, message = "파일 확장자는 최소 1개, 최대 10개까지 가능합니다.")
List<String> fileExtensions
) {적절한 max 값은 비즈니스 요구사항에 따라 조정해주세요.
📝 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.
| public record ImageRequest( | |
| @Schema(description = "파일 확장자 리스트", example = "[\"png\", \"jpg\"]") | |
| @NotNull(message = "파일 확장자는 필수입니다.") | |
| @Size(min = 1, message = "최소 하나 이상의 파일 확장자를 제공해야 합니다.") | |
| List<String> fileExtensions | |
| ) { | |
| public record ImageRequest( | |
| @Schema(description = "파일 확장자 리스트", example = "[\"png\", \"jpg\"]") | |
| @NotNull(message = "파일 확장자는 필수입니다.") | |
| @Size(min = 1, max = 10, message = "파일 확장자는 최소 1개, 최대 10개까지 가능합니다.") | |
| List<String> fileExtensions | |
| ) { | |
| } |
🤖 Prompt for AI Agents
In
src/main/java/konkuk/chacall/domain/foodtruck/presentation/dto/request/ImageRequest.java
around lines 9 to 14, the @Size annotation only enforces a minimum count; add a
maximum limit to prevent excessive image counts by changing the annotation to
include max (for example @Size(min = 1, max = 10, message = "파일 확장자는 1개 이상, 최대
10개까지 허용됩니다.") — adjust the max value to your business requirement) and ensure
any validation error maps to the INVALID_IMAGE_COUNT error code if present in
your validation/error handling.
| public String generatePresignedUrl(String key) { | ||
| PutObjectRequest putObjectRequest = PutObjectRequest.builder() | ||
| .bucket(s3Config.getBucket()) | ||
| .key(key) | ||
| .build(); | ||
|
|
||
| PresignedPutObjectRequest presignedRequest = s3Presigner.presignPutObject(builder -> builder | ||
| .putObjectRequest(putObjectRequest) | ||
| .signatureDuration(Duration.ofMinutes(PRESIGNED_URL_EXPIRATION_MINUTES)) | ||
| ); | ||
|
|
||
| return presignedRequest.url().toString(); | ||
| } |
There was a problem hiding this comment.
key 파라미터 검증 누락
key 파라미터에 대한 null 체크 및 유효성 검증이 없습니다. null이거나 빈 문자열, 또는 부적절한 문자를 포함할 경우 문제가 발생할 수 있습니다.
public String generatePresignedUrl(String key) {
+ if (key == null || key.isBlank()) {
+ throw new IllegalArgumentException("key는 null이거나 비어있을 수 없습니다.");
+ }
PutObjectRequest putObjectRequest = PutObjectRequest.builder()
.bucket(s3Config.getBucket())🤖 Prompt for AI Agents
In src/main/java/konkuk/chacall/global/common/storage/S3Service.java around
lines 52 to 64, the generatePresignedUrl method lacks validation for the key
parameter which can be null, empty, or contain invalid characters; add input
validation at the start of the method: null-check (throw
IllegalArgumentException with a clear message), trim and ensure non-empty, and
enforce allowed character rules (or a whitelist/regex) and acceptable length; if
validation fails throw a meaningful exception so callers know the reason; only
proceed to build the PutObjectRequest and presign when the key passes
validation.
| public static void checkAllowedExtension(List<String> extensions) { | ||
| for (String extension : extensions) { | ||
| boolean isMatch = Arrays.stream(AllowedFileExtension.values()) | ||
| .anyMatch(allowedExtension -> allowedExtension.value.equalsIgnoreCase(extension)); | ||
|
|
||
| if (!isMatch) { | ||
| throw new BusinessException(ErrorCode.INVALID_FILE_EXTENSION, | ||
| new IllegalArgumentException("허용되지 않은 파일 확장자: " + extension)); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
null 입력에 대한 방어 로직 필요
extensions 파라미터나 리스트 내부의 null 요소에 대한 검증이 없습니다. null이 전달되면 NPE가 발생할 수 있습니다.
public static void checkAllowedExtension(List<String> extensions) {
+ if (extensions == null || extensions.isEmpty()) {
+ throw new BusinessException(ErrorCode.INVALID_FILE_EXTENSION,
+ new IllegalArgumentException("파일 확장자 리스트는 null이거나 비어있을 수 없습니다."));
+ }
for (String extension : extensions) {
+ if (extension == null || extension.isBlank()) {
+ throw new BusinessException(ErrorCode.INVALID_FILE_EXTENSION,
+ new IllegalArgumentException("파일 확장자는 null이거나 비어있을 수 없습니다."));
+ }
boolean isMatch = Arrays.stream(AllowedFileExtension.values())📝 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.
| public static void checkAllowedExtension(List<String> extensions) { | |
| for (String extension : extensions) { | |
| boolean isMatch = Arrays.stream(AllowedFileExtension.values()) | |
| .anyMatch(allowedExtension -> allowedExtension.value.equalsIgnoreCase(extension)); | |
| if (!isMatch) { | |
| throw new BusinessException(ErrorCode.INVALID_FILE_EXTENSION, | |
| new IllegalArgumentException("허용되지 않은 파일 확장자: " + extension)); | |
| } | |
| } | |
| } | |
| public static void checkAllowedExtension(List<String> extensions) { | |
| if (extensions == null || extensions.isEmpty()) { | |
| throw new BusinessException(ErrorCode.INVALID_FILE_EXTENSION, | |
| new IllegalArgumentException("파일 확장자 리스트는 null이거나 비어있을 수 없습니다.")); | |
| } | |
| for (String extension : extensions) { | |
| if (extension == null || extension.isBlank()) { | |
| throw new BusinessException(ErrorCode.INVALID_FILE_EXTENSION, | |
| new IllegalArgumentException("파일 확장자는 null이거나 비어있을 수 없습니다.")); | |
| } | |
| boolean isMatch = Arrays.stream(AllowedFileExtension.values()) | |
| .anyMatch(allowedExtension -> allowedExtension.value.equalsIgnoreCase(extension)); | |
| if (!isMatch) { | |
| throw new BusinessException(ErrorCode.INVALID_FILE_EXTENSION, | |
| new IllegalArgumentException("허용되지 않은 파일 확장자: " + extension)); | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In
src/main/java/konkuk/chacall/global/common/storage/util/AllowedFileExtension.java
around lines 24-34, add defensive checks so a null List or null elements don't
cause an NPE: first validate the extensions parameter is not null and if it is
throw a BusinessException (use an appropriate ErrorCode consistent with the
class, e.g., INVALID_FILE_EXTENSION or a more specific error code if available)
with a clear message; then iterate safely (or stream) while skipping or
explicitly rejecting null elements—if a null element is found throw a
BusinessException with a descriptive message; keep the existing extension
matching logic otherwise.
| public static String buildFoodTruckImageKey(Long userId) { | ||
| String timestamp = LocalDateTime.now().format(DATE_TIME_FORMATTER); // ex) 20250923-163045 | ||
| String uuid = UUID.randomUUID().toString(); | ||
| return String.format("foodtrucks/%d/%s/%s", | ||
| userId, | ||
| timestamp, | ||
| uuid); // foodtrucks/{userId}/{timestamp}/{uuid} | ||
| } |
There was a problem hiding this comment.
null 체크 누락으로 인한 NPE 가능성
userId가 null인 경우 String.format에서 NPE가 발생할 수 있습니다. 메서드 시작 부분에 null 검증을 추가하거나, @NonNull 애노테이션을 통해 계약을 명시하는 것을 권장합니다.
public static String buildFoodTruckImageKey(Long userId) {
+ if (userId == null) {
+ throw new IllegalArgumentException("userId는 null일 수 없습니다.");
+ }
String timestamp = LocalDateTime.now().format(DATE_TIME_FORMATTER);
String uuid = UUID.randomUUID().toString();
return String.format("foodtrucks/%d/%s/%s",📝 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.
| public static String buildFoodTruckImageKey(Long userId) { | |
| String timestamp = LocalDateTime.now().format(DATE_TIME_FORMATTER); // ex) 20250923-163045 | |
| String uuid = UUID.randomUUID().toString(); | |
| return String.format("foodtrucks/%d/%s/%s", | |
| userId, | |
| timestamp, | |
| uuid); // foodtrucks/{userId}/{timestamp}/{uuid} | |
| } | |
| public static String buildFoodTruckImageKey(Long userId) { | |
| if (userId == null) { | |
| throw new IllegalArgumentException("userId는 null일 수 없습니다."); | |
| } | |
| String timestamp = LocalDateTime.now().format(DATE_TIME_FORMATTER); // ex) 20250923-163045 | |
| String uuid = UUID.randomUUID().toString(); | |
| return String.format("foodtrucks/%d/%s/%s", | |
| userId, | |
| timestamp, | |
| uuid); // foodtrucks/{userId}/{timestamp}/{uuid} | |
| } |
🤖 Prompt for AI Agents
In src/main/java/konkuk/chacall/global/common/storage/util/KeyUtils.java around
lines 24 to 31, the method buildFoodTruckImageKey can throw an NPE when userId
is null; add an explicit null check at the start (e.g.,
Objects.requireNonNull(userId, "userId must not be null") or throw new
IllegalArgumentException("userId must not be null")) or annotate the parameter
with @NonNull and document the contract so callers cannot pass null; ensure the
method fails fast with a clear error message rather than allowing String.format
to throw a NullPointerException.
| public static String buildMenuImageKey(Long foodTruckId) { | ||
| String timestamp = LocalDateTime.now().format(DATE_TIME_FORMATTER); // ex) 20250923-163045 | ||
| String uuid = UUID.randomUUID().toString(); | ||
| return String.format("menus/%d/%s/%s", | ||
| foodTruckId, | ||
| timestamp, | ||
| uuid); // menus/{foodTruckId}/{timestamp}/{uuid} | ||
| } |
There was a problem hiding this comment.
null 체크 누락으로 인한 NPE 가능성
foodTruckId가 null인 경우 String.format에서 NPE가 발생할 수 있습니다. 메서드 시작 부분에 null 검증을 추가하거나, @NonNull 애노테이션을 통해 계약을 명시하는 것을 권장합니다.
public static String buildMenuImageKey(Long foodTruckId) {
+ if (foodTruckId == null) {
+ throw new IllegalArgumentException("foodTruckId는 null일 수 없습니다.");
+ }
String timestamp = LocalDateTime.now().format(DATE_TIME_FORMATTER);
String uuid = UUID.randomUUID().toString();
return String.format("menus/%d/%s/%s",📝 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.
| public static String buildMenuImageKey(Long foodTruckId) { | |
| String timestamp = LocalDateTime.now().format(DATE_TIME_FORMATTER); // ex) 20250923-163045 | |
| String uuid = UUID.randomUUID().toString(); | |
| return String.format("menus/%d/%s/%s", | |
| foodTruckId, | |
| timestamp, | |
| uuid); // menus/{foodTruckId}/{timestamp}/{uuid} | |
| } | |
| public static String buildMenuImageKey(Long foodTruckId) { | |
| if (foodTruckId == null) { | |
| throw new IllegalArgumentException("foodTruckId는 null일 수 없습니다."); | |
| } | |
| String timestamp = LocalDateTime.now().format(DATE_TIME_FORMATTER); // ex) 20250923-163045 | |
| String uuid = UUID.randomUUID().toString(); | |
| return String.format("menus/%d/%s/%s", | |
| foodTruckId, | |
| timestamp, | |
| uuid); // menus/{foodTruckId}/{timestamp}/{uuid} | |
| } |
🤖 Prompt for AI Agents
In src/main/java/konkuk/chacall/global/common/storage/util/KeyUtils.java around
lines 33–40, the buildMenuImageKey method can throw an NPE when foodTruckId is
null; add a null check at the start of the method (e.g., use
Objects.requireNonNull(foodTruckId, "foodTruckId must not be null") or if you
prefer validate and throw an IllegalArgumentException with a clear message), or
alternatively annotate the parameter with @NonNull and ensure callers respect
it; ensure the method fails fast with a clear error message rather than allowing
String.format to produce an NPE.
| GET_FOOD_TRUCK_PRESGIEND_URL(new LinkedHashSet<>(Set.of( | ||
| USER_NOT_FOUND, | ||
| USER_FORBIDDEN, | ||
| INVALID_IMAGE_COUNT, | ||
| INVALID_FILE_EXTENSION | ||
| ))), | ||
| GET_MENU_PRESGIEND_URL(new LinkedHashSet<>(Set.of( | ||
| USER_NOT_FOUND, | ||
| USER_FORBIDDEN, | ||
| INVALID_IMAGE_COUNT, | ||
| INVALID_FILE_EXTENSION | ||
| ))), |
There was a problem hiding this comment.
Enum 상수 이름에 오타가 있습니다.
GET_FOOD_TRUCK_PRESGIEND_URL과 GET_MENU_PRESGIEND_URL의 "PRESGIEND"는 "PRESIGNED"의 오타입니다.
다음 diff를 적용하여 수정하세요:
- GET_FOOD_TRUCK_PRESGIEND_URL(new LinkedHashSet<>(Set.of(
+ GET_FOOD_TRUCK_PRESIGNED_URL(new LinkedHashSet<>(Set.of(
USER_NOT_FOUND,
USER_FORBIDDEN,
INVALID_IMAGE_COUNT,
INVALID_FILE_EXTENSION
))),
- GET_MENU_PRESGIEND_URL(new LinkedHashSet<>(Set.of(
+ GET_MENU_PRESIGNED_URL(new LinkedHashSet<>(Set.of(
USER_NOT_FOUND,
USER_FORBIDDEN,
INVALID_IMAGE_COUNT,
INVALID_FILE_EXTENSION
))),참고: FoodTruckController.java의 @ExceptionDescription 어노테이션도 함께 업데이트해야 합니다.
📝 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.
| GET_FOOD_TRUCK_PRESGIEND_URL(new LinkedHashSet<>(Set.of( | |
| USER_NOT_FOUND, | |
| USER_FORBIDDEN, | |
| INVALID_IMAGE_COUNT, | |
| INVALID_FILE_EXTENSION | |
| ))), | |
| GET_MENU_PRESGIEND_URL(new LinkedHashSet<>(Set.of( | |
| USER_NOT_FOUND, | |
| USER_FORBIDDEN, | |
| INVALID_IMAGE_COUNT, | |
| INVALID_FILE_EXTENSION | |
| ))), | |
| GET_FOOD_TRUCK_PRESIGNED_URL(new LinkedHashSet<>(Set.of( | |
| USER_NOT_FOUND, | |
| USER_FORBIDDEN, | |
| INVALID_IMAGE_COUNT, | |
| INVALID_FILE_EXTENSION | |
| ))), | |
| GET_MENU_PRESIGNED_URL(new LinkedHashSet<>(Set.of( | |
| USER_NOT_FOUND, | |
| USER_FORBIDDEN, | |
| INVALID_IMAGE_COUNT, | |
| INVALID_FILE_EXTENSION | |
| ))), |
🤖 Prompt for AI Agents
In
src/main/java/konkuk/chacall/global/common/swagger/SwaggerResponseDescription.java
around lines 185-196 the enum constant names GET_FOOD_TRUCK_PRESGIEND_URL and
GET_MENU_PRESGIEND_URL contain a typo ("PRESGIEND" should be "PRESIGNED");
rename both constants to GET_FOOD_TRUCK_PRESIGNED_URL and GET_MENU_PRESIGNED_URL
(preserve their associated LinkedHashSet values) and update all references;
additionally, update the @ExceptionDescription usage in FoodTruckController.java
to refer to the new constant names so callers/annotations compile correctly.
enum 상수명의 오타를 수정하세요.
두 enum 상수명에 오타가 있습니다: PRESGIEND는 PRESIGNED가 되어야 합니다.
다음 diff를 적용하여 수정하세요:
- GET_FOOD_TRUCK_PRESGIEND_URL(new LinkedHashSet<>(Set.of(
+ GET_FOOD_TRUCK_PRESIGNED_URL(new LinkedHashSet<>(Set.of(
USER_NOT_FOUND,
USER_FORBIDDEN,
INVALID_IMAGE_COUNT,
INVALID_FILE_EXTENSION
))),
- GET_MENU_PRESGIEND_URL(new LinkedHashSet<>(Set.of(
+ GET_MENU_PRESIGNED_URL(new LinkedHashSet<>(Set.of(
USER_NOT_FOUND,
USER_FORBIDDEN,
INVALID_IMAGE_COUNT,
INVALID_FILE_EXTENSION
))),참고: 이 수정 후 FoodTruckController.java의 @ExceptionDescription 어노테이션도 함께 업데이트해야 합니다.
📝 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.
| GET_FOOD_TRUCK_PRESGIEND_URL(new LinkedHashSet<>(Set.of( | |
| USER_NOT_FOUND, | |
| USER_FORBIDDEN, | |
| INVALID_IMAGE_COUNT, | |
| INVALID_FILE_EXTENSION | |
| ))), | |
| GET_MENU_PRESGIEND_URL(new LinkedHashSet<>(Set.of( | |
| USER_NOT_FOUND, | |
| USER_FORBIDDEN, | |
| INVALID_IMAGE_COUNT, | |
| INVALID_FILE_EXTENSION | |
| ))), | |
| GET_FOOD_TRUCK_PRESIGNED_URL(new LinkedHashSet<>(Set.of( | |
| USER_NOT_FOUND, | |
| USER_FORBIDDEN, | |
| INVALID_IMAGE_COUNT, | |
| INVALID_FILE_EXTENSION | |
| ))), | |
| GET_MENU_PRESIGNED_URL(new LinkedHashSet<>(Set.of( | |
| USER_NOT_FOUND, | |
| USER_FORBIDDEN, | |
| INVALID_IMAGE_COUNT, | |
| INVALID_FILE_EXTENSION | |
| ))), |
🤖 Prompt for AI Agents
In
src/main/java/konkuk/chacall/global/common/swagger/SwaggerResponseDescription.java
around lines 185-196, two enum constant names contain a typo: change
GET_FOOD_TRUCK_PRESGIEND_URL to GET_FOOD_TRUCK_PRESIGNED_URL and
GET_MENU_PRESGIEND_URL to GET_MENU_PRESIGNED_URL; update any references/usages
to the renamed constants (including the @ExceptionDescription annotation in
FoodTruckController.java) so compilation and annotations refer to the new names.
There was a problem hiding this comment.
굿굿 좋습니다! 깔끔하네요~
언급하신 것처럼 저도 메뉴, 푸드트럭 용 이미지 업로드는 구분해두는 것이 API 엔드포인트 면에서 보았을 때는 restful 하다고 볼 수도 있을 것 같긴 합니다.
다만 추후에 사업자 등록증, 기타 서류등을 어드민 페이지에서 조회해야하는 요구사항이 추가될 것 같아서, 그렇게 되면 그러한 이미지들을 위한 API를 또 별도로 만들어야할 것 같다는 생각이 들었습니다.
만약 제 예상이 맞다면, 거의 중복된 형식의 API 반복적으로 3~4개 만들어져야할 것 같아서 그 부분에서는 유지보수가 오히려 귀찮아질 수 있겠다라는 생각도 드는데 어떻게 생각하시나요?
또, 한가지 더 고민이 되었던게 아직 저는 presigend URL을 통해 업로드 한 후 접근가능한 이미지 URL {cdn도메인 + 이미지 객체 key}을 파싱하지 않고 있는데, 이를 presigned URL을 발급하는 api에서 담당해서 presignedURL과 함께 세트로 반환해주는게 좋을까요? 아니면 푸드트럭 등록 또는 메뉴 등록 api 요청시에 key를 추출해서 내부적으로 파싱해서 테이블에 넣는게 좋을까요??
그럼 제가 이해하기로는,
- Presigned URL 을 발급하는 시점에 DB 에 저장할 때 활용해야하는 url ( = {cdn 도메인 + 이미지 객체 키} ) 도 같이 알려주어서, 메뉴 등록이나 푸드트럭 등록시에 그 url 을 요청에 포함시키기만 하면 되도록 구현
- 프론트는 서버로부터 받은 presigned url 을 다시 메뉴 등록이나 푸드트럭 등록 요청에 포함시켜서 전송하면, 서버쪽에서 알아서 이 presigned url 을 파싱한 후 DB에 저장한다.
이 두 방식 중에서 고민하고 계신 것 같은데 맞을까요?
맞다면 전 1번 방식이 맞다고 생각합니다! 일단 파싱보다는 url 을 만들어서 반환해주는 것이 좀 더 로직적으로도 간단할 것 같다는 생각이 들어요. 어차피 성능 이슈도 딱히 없을 것 같아서요!
| if(fileExtensions == null || fileExtensions.isEmpty() || fileExtensions.size() > MAX_FOOD_TRUCK_IMAGE_COUNT) { | ||
| throw new BusinessException(ErrorCode.INVALID_IMAGE_COUNT, |
There was a problem hiding this comment.
p2 : fileExtensions == null || fileExtensions.isEmpty 는 Bean validation 으로 검증되고있기 때문에 중복 검증인 것 같습니다! 제거해도 괜찮을 것 같아요
| public ImageResponse createMenuImagePresignedUrl(ImageRequest request, User owner) { | ||
| List<String> fileExtensions = request.fileExtensions(); | ||
|
|
||
| if(fileExtensions == null || fileExtensions.isEmpty()) { |
| public static void checkAllowedExtension(List<String> extensions) { | ||
| for (String extension : extensions) { | ||
| boolean isMatch = Arrays.stream(AllowedFileExtension.values()) | ||
| .anyMatch(allowedExtension -> allowedExtension.value.equalsIgnoreCase(extension)); | ||
|
|
||
| if (!isMatch) { | ||
| throw new BusinessException(ErrorCode.INVALID_FILE_EXTENSION, | ||
| new IllegalArgumentException("허용되지 않은 파일 확장자: " + extension)); | ||
| } |
There was a problem hiding this comment.
enum 내부 검증 좋습니다~
다만 예외를 터뜨리는 void 형식의 메서드라면
Arrays.stream(AllowedFileExtension.values())
.filter(ext -> ext.value.equalsIgnoreCase(extension))
.findAny()
.orElseThrow(() -> new BusinessException(
ErrorCode.INVALID_FILE_EXTENSION,
new IllegalArgumentException("허용되지 않은 파일 확장자: " + extension)
));이런 형태로 스트림 하나로 처리도 가능할 것 같아요!
There was a problem hiding this comment.
넵 좋슴다 Optional로 굳이 감싸고 싶지 않아서 이렇게 구현하긴 했는데 성능적으로는 별로 차이가 없을 것 같고 한번에 처리하는게 가독성이 훨씬 좋을 것 같네요! 수정하겠슴다~
| public static String buildFoodTruckImageKey(Long userId) { | ||
| String timestamp = LocalDateTime.now().format(DATE_TIME_FORMATTER); // ex) 20250923-163045 | ||
| String uuid = UUID.randomUUID().toString(); | ||
| return String.format("foodtrucks/%d/%s/%s", | ||
| userId, |
| GET_FOOD_TRUCK_PRESGIEND_URL(new LinkedHashSet<>(Set.of( | ||
| USER_NOT_FOUND, | ||
| USER_FORBIDDEN, | ||
| INVALID_IMAGE_COUNT, | ||
| INVALID_FILE_EXTENSION | ||
| ))), | ||
| GET_MENU_PRESGIEND_URL(new LinkedHashSet<>(Set.of( |
There was a problem hiding this comment.
네이밍을 PRESIGNED 로 바꿔야할 것 같습니다!
저도 상균님과 마찬가지로 API 3~4개가 모두 중복 로직을 가지고 있을 것이라고 생각되긴 했는데 결국 같은 API로 한다고 해도 하나의 서비스 메서드 내부에서 특정 플래그를 통해서 분기처리를 하여 유효성 검증(갯수)을 해줘야 하는 상황인 것 같아서 차라리 API는 분리(controller 중복은 감수), 하나의 서비스 메서드 내부에서 중복된 로직을 메서드화 시키는건 어떨까 싶습니다! 어떤가욥??
네넵 이해하신게 정확합니다! 그러면 2번 방식으로 API 반환형태를 수정해보겠습니다! |
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
src/main/java/konkuk/chacall/global/common/storage/S3Service.java (1)
55-67:key유효성 검사가 여전히 없습니다.
이전에 지적된 것처럼key가null이거나 공백이면PutObjectRequest빌드 단계에서 즉시 예외가 터집니다. 외부 입력을 그대로 넘기는 구조라면 곧바로 500이 발생하고, 유효하지 않은 key가 만들어지는 경우에도 문제를 조기에 발견할 수 없습니다. 최소한 null/blank 차단은 메서드 안에서 보강해주세요.다음과 같이 보완하면 됩니다:
public String generatePresignedUrl(String key) { + if (key == null || key.isBlank()) { + throw new IllegalArgumentException("S3 key는 비어 있을 수 없습니다."); + } PutObjectRequest putObjectRequest = PutObjectRequest.builder() .bucket(s3Config.getBucket()) .key(key)
🧹 Nitpick comments (1)
src/main/java/konkuk/chacall/domain/foodtruck/application/image/FoodTruckImageService.java (1)
22-22: 주석 처리된 상수를 제거하거나 활성화해 주세요.
MAX_MENU_IMAGE_COUNT가 주석 처리되어 있지만 메뉴 이미지 생성 메서드(createMenuImagePresignedUrl)에서는 이미지 개수 제한 검증을 하지 않고 있습니다. 만약 메뉴 이미지에도 개수 제한이 필요하다면 주석을 해제하고 검증 로직을 추가해야 하며, 불필요하다면 이 주석을 제거하는 것이 코드 정리 측면에서 좋을 것 같습니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/main/java/konkuk/chacall/domain/foodtruck/application/image/FoodTruckImageService.java(1 hunks)src/main/java/konkuk/chacall/domain/foodtruck/presentation/FoodTruckController.java(2 hunks)src/main/java/konkuk/chacall/domain/foodtruck/presentation/dto/response/ImageResponse.java(1 hunks)src/main/java/konkuk/chacall/global/common/security/filter/JwtAuthenticationFilter.java(3 hunks)src/main/java/konkuk/chacall/global/common/storage/S3Service.java(3 hunks)src/main/java/konkuk/chacall/global/common/storage/util/AllowedFileExtension.java(1 hunks)src/main/java/konkuk/chacall/global/common/swagger/SwaggerResponseDescription.java(1 hunks)src/main/java/konkuk/chacall/global/config/SecurityConfig.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main/java/konkuk/chacall/global/common/swagger/SwaggerResponseDescription.java
- src/main/java/konkuk/chacall/global/common/storage/util/AllowedFileExtension.java
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/java/konkuk/chacall/domain/foodtruck/application/image/FoodTruckImageService.java (1)
src/main/java/konkuk/chacall/domain/foodtruck/application/FoodTruckService.java (1)
RequiredArgsConstructor(19-56)
🔇 Additional comments (1)
src/main/java/konkuk/chacall/domain/foodtruck/application/image/FoodTruckImageService.java (1)
34-44: 확장자가 키에 정상적으로 반영되었습니다.이전 리뷰에서 지적된 확장자 미반영 이슈가 수정되어, 이제 각 presigned URL과 객체 키에 요청된 확장자(예:
.png)가 올바르게 포함됩니다. 클라이언트가 기대하는<uuid>.png형태의 키가 정상적으로 생성될 것입니다.
| public ImageResponse createFoodTruckImagePresignedUrl(ImageRequest request, User owner) { | ||
| List<String> fileExtensions = request.fileExtensions(); | ||
|
|
||
| if(fileExtensions.size() > MAX_FOOD_TRUCK_IMAGE_COUNT) { | ||
| throw new BusinessException(ErrorCode.INVALID_IMAGE_COUNT, | ||
| new IllegalArgumentException("푸드트럭 이미지는 1장 이상 " + MAX_FOOD_TRUCK_IMAGE_COUNT + "장 이하로 등록해야 합니다. 입력된 이미지 개수: " + fileExtensions.size())); | ||
| } | ||
|
|
||
| AllowedFileExtension.checkAllowedExtension(fileExtensions); | ||
|
|
||
| var imageInfos = fileExtensions.stream() | ||
| .map(extension -> { | ||
| String baseKey = KeyUtils.buildFoodTruckImageKey(owner.getUserId()); | ||
| String keyWithExt = baseKey + "." + extension.toLowerCase(); | ||
|
|
||
| String presignedUrl = s3Service.generatePresignedUrl(keyWithExt); | ||
| String fileUrl = s3Service.getFileUrl(keyWithExt); | ||
|
|
||
| return ImageResponse.ImageInfo.of(presignedUrl, fileUrl); | ||
| }) | ||
| .toList(); | ||
|
|
||
| return ImageResponse.of(imageInfos); | ||
| } | ||
|
|
||
| public ImageResponse createMenuImagePresignedUrl(ImageRequest request, User owner) { | ||
| List<String> fileExtensions = request.fileExtensions(); | ||
|
|
||
| AllowedFileExtension.checkAllowedExtension(fileExtensions); | ||
|
|
||
| var imageInfos = fileExtensions.stream() | ||
| .map(extension -> { | ||
| String baseKey = KeyUtils.buildMenuImageKey(owner.getUserId()); | ||
| String keyWithExt = baseKey + "." + extension.toLowerCase(); | ||
|
|
||
| String presignedUrl = s3Service.generatePresignedUrl(keyWithExt); | ||
| String fileUrl = s3Service.getFileUrl(keyWithExt); | ||
|
|
||
| return ImageResponse.ImageInfo.of(presignedUrl, fileUrl); | ||
| }) | ||
| .toList(); | ||
|
|
||
| return ImageResponse.of(imageInfos); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
중복 코드를 공통 메서드로 추출해 주세요.
두 메서드(createFoodTruckImagePresignedUrl, createMenuImagePresignedUrl) 간에 presigned URL 생성 로직이 거의 동일하게 반복되고 있습니다. PR 코멘트에서도 언급되었듯이, 향후 추가 이미지 타입(사업자 등록증 등)이 필요할 경우 동일한 코드가 계속 복사될 우려가 있습니다. 키 생성 부분만 다르므로, 공통 로직을 private 헬퍼 메서드로 추출하고 키 생성 함수를 파라미터로 받는 방식으로 리팩토링하면 유지보수성이 크게 개선됩니다.
리팩토링 예시:
+ private ImageResponse generateImagePresignedUrls(
+ List<String> fileExtensions,
+ java.util.function.Function<Long, String> keyGenerator) {
+
+ AllowedFileExtension.checkAllowedExtension(fileExtensions);
+
+ var imageInfos = fileExtensions.stream()
+ .map(extension -> {
+ String baseKey = keyGenerator.apply(owner.getUserId());
+ String keyWithExt = baseKey + "." + extension.toLowerCase();
+
+ String presignedUrl = s3Service.generatePresignedUrl(keyWithExt);
+ String fileUrl = s3Service.getFileUrl(keyWithExt);
+
+ return ImageResponse.ImageInfo.of(presignedUrl, fileUrl);
+ })
+ .toList();
+
+ return ImageResponse.of(imageInfos);
+ }
+
public ImageResponse createFoodTruckImagePresignedUrl(ImageRequest request, User owner) {
List<String> fileExtensions = request.fileExtensions();
if(fileExtensions.size() > MAX_FOOD_TRUCK_IMAGE_COUNT) {
throw new BusinessException(ErrorCode.INVALID_IMAGE_COUNT,
new IllegalArgumentException("푸드트럭 이미지는 1장 이상 " + MAX_FOOD_TRUCK_IMAGE_COUNT + "장 이하로 등록해야 합니다. 입력된 이미지 개수: " + fileExtensions.size()));
}
- AllowedFileExtension.checkAllowedExtension(fileExtensions);
-
- var imageInfos = fileExtensions.stream()
- .map(extension -> {
- String baseKey = KeyUtils.buildFoodTruckImageKey(owner.getUserId());
- String keyWithExt = baseKey + "." + extension.toLowerCase();
-
- String presignedUrl = s3Service.generatePresignedUrl(keyWithExt);
- String fileUrl = s3Service.getFileUrl(keyWithExt);
-
- return ImageResponse.ImageInfo.of(presignedUrl, fileUrl);
- })
- .toList();
-
- return ImageResponse.of(imageInfos);
+ return generateImagePresignedUrls(fileExtensions, owner, KeyUtils::buildFoodTruckImageKey);
}
public ImageResponse createMenuImagePresignedUrl(ImageRequest request, User owner) {
List<String> fileExtensions = request.fileExtensions();
- AllowedFileExtension.checkAllowedExtension(fileExtensions);
-
- var imageInfos = fileExtensions.stream()
- .map(extension -> {
- String baseKey = KeyUtils.buildMenuImageKey(owner.getUserId());
- String keyWithExt = baseKey + "." + extension.toLowerCase();
-
- String presignedUrl = s3Service.generatePresignedUrl(keyWithExt);
- String fileUrl = s3Service.getFileUrl(keyWithExt);
-
- return ImageResponse.ImageInfo.of(presignedUrl, fileUrl);
- })
- .toList();
-
- return ImageResponse.of(imageInfos);
+ return generateImagePresignedUrls(fileExtensions, owner, KeyUtils::buildMenuImageKey);
}또는 개수 검증도 함께 추상화하려면:
private ImageResponse generateImagePresignedUrls(
List<String> fileExtensions,
User owner,
Integer maxCount,
String imageType,
java.util.function.Function<Long, String> keyGenerator) {
if (maxCount != null && fileExtensions.size() > maxCount) {
throw new BusinessException(ErrorCode.INVALID_IMAGE_COUNT,
new IllegalArgumentException(imageType + " 이미지는 최대 " + maxCount + "장까지 등록 가능합니다. 입력된 이미지 개수: " + fileExtensions.size()));
}
AllowedFileExtension.checkAllowedExtension(fileExtensions);
// ... (동일한 stream 로직)
}🤖 Prompt for AI Agents
In
src/main/java/konkuk/chacall/domain/foodtruck/application/image/FoodTruckImageService.java
around lines 24-67, extract the duplicated presigned URL generation logic into a
private helper method that accepts the fileExtensions, owner, an optional
maxCount, an image type label, and a keyGenerator Function<Long,String>; the
helper should (1) validate maxCount and throw the same BusinessException when
fileExtensions.size() exceeds maxCount, (2) call
AllowedFileExtension.checkAllowedExtension, (3) map each extension to a key by
lowercasing the extension and calling keyGenerator.apply(owner.getUserId()),
append the extension, call s3Service.generatePresignedUrl and
s3Service.getFileUrl, build ImageResponse.ImageInfo and return
ImageResponse.of(list); then replace createFoodTruckImagePresignedUrl to call
the helper with MAX_FOOD_TRUCK_IMAGE_COUNT and KeyUtils::buildFoodTruckImageKey
and replace createMenuImagePresignedUrl to call the helper with no maxCount (or
null) and KeyUtils::buildMenuImageKey.
| if(fileExtensions.size() > MAX_FOOD_TRUCK_IMAGE_COUNT) { | ||
| throw new BusinessException(ErrorCode.INVALID_IMAGE_COUNT, | ||
| new IllegalArgumentException("푸드트럭 이미지는 1장 이상 " + MAX_FOOD_TRUCK_IMAGE_COUNT + "장 이하로 등록해야 합니다. 입력된 이미지 개수: " + fileExtensions.size())); | ||
| } |
There was a problem hiding this comment.
이미지 개수 검증 로직에서 최소값 검증이 누락되었습니다.
에러 메시지에서 "1장 이상"이라고 명시하고 있지만, 실제로는 최댓값(9장)만 검증하고 있습니다. fileExtensions.isEmpty() 케이스는 Bean Validation(@NotEmpty 등)에서 이미 검증되고 있을 가능성이 높지만, 만약 비즈니스 로직상 명시적으로 최솟값(1장)도 검증해야 한다면 조건을 추가하거나, 그렇지 않다면 에러 메시지를 "9장 이하로 등록해야 합니다"로 수정하는 것이 정확할 것 같습니다.
적용 예시 (최솟값도 검증하는 경우):
- if(fileExtensions.size() > MAX_FOOD_TRUCK_IMAGE_COUNT) {
+ if(fileExtensions.isEmpty() || fileExtensions.size() > MAX_FOOD_TRUCK_IMAGE_COUNT) {
throw new BusinessException(ErrorCode.INVALID_IMAGE_COUNT,
new IllegalArgumentException("푸드트럭 이미지는 1장 이상 " + MAX_FOOD_TRUCK_IMAGE_COUNT + "장 이하로 등록해야 합니다. 입력된 이미지 개수: " + fileExtensions.size()));
}또는 에러 메시지만 수정하는 경우:
if(fileExtensions.size() > MAX_FOOD_TRUCK_IMAGE_COUNT) {
throw new BusinessException(ErrorCode.INVALID_IMAGE_COUNT,
- new IllegalArgumentException("푸드트럭 이미지는 1장 이상 " + MAX_FOOD_TRUCK_IMAGE_COUNT + "장 이하로 등록해야 합니다. 입력된 이미지 개수: " + fileExtensions.size()));
+ new IllegalArgumentException("푸드트럭 이미지는 최대 " + MAX_FOOD_TRUCK_IMAGE_COUNT + "장까지 등록 가능합니다. 입력된 이미지 개수: " + fileExtensions.size()));
}| public ImageResponse createMenuImagePresignedUrl(ImageRequest request, User owner) { | ||
| List<String> fileExtensions = request.fileExtensions(); | ||
|
|
||
| AllowedFileExtension.checkAllowedExtension(fileExtensions); | ||
|
|
||
| var imageInfos = fileExtensions.stream() | ||
| .map(extension -> { | ||
| String baseKey = KeyUtils.buildMenuImageKey(owner.getUserId()); | ||
| String keyWithExt = baseKey + "." + extension.toLowerCase(); | ||
|
|
||
| String presignedUrl = s3Service.generatePresignedUrl(keyWithExt); | ||
| String fileUrl = s3Service.getFileUrl(keyWithExt); | ||
|
|
||
| return ImageResponse.ImageInfo.of(presignedUrl, fileUrl); | ||
| }) | ||
| .toList(); | ||
|
|
||
| return ImageResponse.of(imageInfos); | ||
| } |
There was a problem hiding this comment.
메뉴 이미지 개수 검증이 누락되었습니다.
createFoodTruckImagePresignedUrl과 달리 메뉴 이미지 생성 메서드에서는 이미지 개수 제한 검증이 없습니다. Line 22의 주석 처리된 MAX_MENU_IMAGE_COUNT 상수를 보면 원래 개수 제한이 계획되었던 것으로 보입니다. 메뉴 이미지에도 개수 제한이 필요하다면 푸드트럭 이미지와 동일한 검증 로직을 추가해 주세요.
적용 예시:
public ImageResponse createMenuImagePresignedUrl(ImageRequest request, User owner) {
List<String> fileExtensions = request.fileExtensions();
+ if(fileExtensions.size() > MAX_MENU_IMAGE_COUNT) {
+ throw new BusinessException(ErrorCode.INVALID_IMAGE_COUNT,
+ new IllegalArgumentException("메뉴 이미지는 최대 " + MAX_MENU_IMAGE_COUNT + "장까지 등록 가능합니다. 입력된 이미지 개수: " + fileExtensions.size()));
+ }
+
AllowedFileExtension.checkAllowedExtension(fileExtensions);📝 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.
| public ImageResponse createMenuImagePresignedUrl(ImageRequest request, User owner) { | |
| List<String> fileExtensions = request.fileExtensions(); | |
| AllowedFileExtension.checkAllowedExtension(fileExtensions); | |
| var imageInfos = fileExtensions.stream() | |
| .map(extension -> { | |
| String baseKey = KeyUtils.buildMenuImageKey(owner.getUserId()); | |
| String keyWithExt = baseKey + "." + extension.toLowerCase(); | |
| String presignedUrl = s3Service.generatePresignedUrl(keyWithExt); | |
| String fileUrl = s3Service.getFileUrl(keyWithExt); | |
| return ImageResponse.ImageInfo.of(presignedUrl, fileUrl); | |
| }) | |
| .toList(); | |
| return ImageResponse.of(imageInfos); | |
| } | |
| public ImageResponse createMenuImagePresignedUrl(ImageRequest request, User owner) { | |
| List<String> fileExtensions = request.fileExtensions(); | |
| if (fileExtensions.size() > MAX_MENU_IMAGE_COUNT) { | |
| throw new BusinessException( | |
| ErrorCode.INVALID_IMAGE_COUNT, | |
| new IllegalArgumentException( | |
| "메뉴 이미지는 최대 " + MAX_MENU_IMAGE_COUNT + | |
| "장까지 등록 가능합니다. 입력된 이미지 개수: " + | |
| fileExtensions.size() | |
| ) | |
| ); | |
| } | |
| AllowedFileExtension.checkAllowedExtension(fileExtensions); | |
| var imageInfos = fileExtensions.stream() | |
| .map(extension -> { | |
| String baseKey = KeyUtils.buildMenuImageKey(owner.getUserId()); | |
| String keyWithExt = baseKey + "." + extension.toLowerCase(); | |
| String presignedUrl = s3Service.generatePresignedUrl(keyWithExt); | |
| String fileUrl = s3Service.getFileUrl(keyWithExt); | |
| return ImageResponse.ImageInfo.of(presignedUrl, fileUrl); | |
| }) | |
| .toList(); | |
| return ImageResponse.of(imageInfos); | |
| } |
🤖 Prompt for AI Agents
In
src/main/java/konkuk/chacall/domain/foodtruck/application/image/FoodTruckImageService.java
around lines 49 to 67, the method createMenuImagePresignedUrl lacks validation
for the number of menu images; add the same max-count check used in
createFoodTruckImagePresignedUrl (use the intended MAX_MENU_IMAGE_COUNT constant
or reintroduce it if commented out) to validate request.fileExtensions().size()
and throw the same domain exception or validation error when the count exceeds
the limit, before proceeding to build keys and generate presigned URLs.
| @Schema(description = "생성된 presigned URL 리스트", example = "[\"https://example.com/presigned-url1\", \"https://example.com/presigned-url2\"]") | ||
| List<ImageInfo> presignedUrls | ||
| ) { | ||
| public record ImageInfo( | ||
| @Schema(description = "생성된 presigned URL", example = "https://example.com/presigned-url") | ||
| String presignedUrl, | ||
|
|
||
| @Schema(description = "파일 접근 URL", example = "https://example.com/file-url") | ||
| String fileUrl |
There was a problem hiding this comment.
Swagger 예시가 실제 구조와 다릅니다.
presignedUrls는 ImageInfo 객체 리스트인데 예시는 문자열 배열이라 문서 소비자가 오해할 수 있습니다. 실제 응답 형태(예: [{ "presignedUrl": "...", "fileUrl": "..." }])에 맞춰 예시를 수정해주세요.
🤖 Prompt for AI Agents
In
src/main/java/konkuk/chacall/domain/foodtruck/presentation/dto/response/ImageResponse.java
around lines 8 to 16, the @Schema example for presignedUrls shows a plain string
array but the field is List<ImageInfo>; update the example to an array of
objects that match ImageInfo (each object with presignedUrl and fileUrl), e.g.
[{"presignedUrl":"https://example.com/presigned-url","fileUrl":"https://example.com/file-url"},
...], so the Swagger docs reflect the actual response structure.
좋습니다~~ 그렇게 구현하는 것이 더 좋을 것 같아유 |
2 participants
#️⃣연관된 이슈
📝작업 내용
이미지 업로드 흐름
presigned URL 발급 api 호출스크린샷 (선택)
💬리뷰 요구사항(선택)
같은 api를 가지고 파라미터로 구분할 수 있지만, 엔드포인트를 분리하는게 조금더 RESTful 하다고 판단해서 현재는 분리된채로 구현해둔 상황입니다. 이에 대해서 상균님 의견이 궁금합니다!
또, 한가지 더 고민이 되었던게 아직 저는 presigend URL을 통해 업로드 한 후 접근가능한 이미지 URL {cdn도메인 + 이미지 객체 key}을 파싱하지 않고 있는데, 이를 presigned URL을 발급하는 api에서 담당해서 presignedURL과 함께 세트로 반환해주는게 좋을까요? 아니면 푸드트럭 등록 또는 메뉴 등록 api 요청시에 key를 추출해서 내부적으로 파싱해서 테이블에 넣는게 좋을까요??
말이 좀 두서가 없는데 허헣 이해 안되시면 카톡 부탁쓰,,
Summary by CodeRabbit