Skip to content

[FEAT] 이미지 경로 제거 기능 구현 #45

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
LimdaeIl merged 5 commits into from
Dec 9, 2025
Merged

[FEAT] 이미지 경로 제거 기능 구현 #45

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
0793997
✨feat: 이미지 루트에 저장하도록 메서드 오버로딩[#44]
LimdaeIl Dec 9, 2025
aa786dd
✨feat: 단 건 이미지 루트에 저장하도록 메서드 오버로딩[#44]
LimdaeIl Dec 9, 2025
4f0a3bc
✨feat: 단 건 이미지 루트에 저장하도록 메서드 오버로딩[#44]
LimdaeIl Dec 9, 2025
d8824e5
🔥remove: 불필요한 임포트 제거[#44]
LimdaeIl Dec 9, 2025
8d76a18
♻️refactor: 이미지 테스트 재수정 .http[#44]
LimdaeIl Dec 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
91 changes: 83 additions & 8 deletions src/main/java/team/wego/wegobackend/image/application/service/ImageUploadService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,8 @@ public ImageFile uploadOriginal(String dir, MultipartFile file, int index) {
validateExtension(file.getOriginalFilename());

String originalFilename = file.getOriginalFilename();
String key = buildKey(dir, originalFilename, index);
// String key = buildKey(dir, originalFilename, index);
Copy link

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commented-out code should be removed rather than left in the codebase. If this code is needed for reference, it should be documented in a TODO comment or commit message instead. The PR description mentions implementing the feature with method overloading to preserve previous functionality, so the old implementation can be recovered from git history if needed.

Suggested change
// String key = buildKey(dir, originalFilename, index);

Copilot uses AI. Check for mistakes.
String key = buildKey(originalFilename, index);
Comment on lines +55 to +56
Copy link

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The uploadOriginal method still validates the dir parameter (line 49) and accepts it as an argument, but then ignores it when building the key (line 56), using the overloaded buildKey(originalFilename, index) method instead. This creates inconsistent behavior where:

  1. The method still enforces directory validation rules
  2. But the validated directory is never used

This breaks the existing API contract. Since GroupImageService at line 68-74 still calls uploadAsWebpWithSizes(dir, file, i, widths, heights) with a directory path, and the PR description states method overloading should "preserve existing functionality", this implementation is incorrect.

Either:

  • Create a new overloaded method uploadOriginal(MultipartFile file, int index) without the dir parameter for the new behavior
  • Or update all callers to use the new API
Suggested change
// String key = buildKey(dir, originalFilename, index);
String key = buildKey(originalFilename, index);
String key = buildKey(dir, originalFilename, index);

Copilot uses AI. Check for mistakes.

byte[] bytes = resizeIfNeededKeepFormat(file);

Expand Down Expand Up @@ -93,6 +94,29 @@ public ImageFile uploadAsWebpWithSize(
return new ImageFile(key, url);
}


// TODO: 회원에서 사용할 단 건 이미지 저장 기능
public ImageFile uploadAsWebpWithSize(
MultipartFile file,
Integer width,
Integer height
) {
validateImageSize(file);
validateImageContentType(file);
validateExtension(file.getOriginalFilename());
ImageSize size = new ImageSize(width, height);

String baseName = buildBaseName();
String key = baseName + "_" + size.width() + "x" + size.height() + ".webp";

byte[] bytes = convertToWebpWithSize(file, size);

putToS3(key, bytes, "image/webp");
String url = awsS3Properties.getPublicEndpoint() + "/" + key;

return new ImageFile(key, url);
}

public List<ImageFile> uploadAsWebpWithSizes(
String dir,
MultipartFile file,
Expand Down Expand Up @@ -128,6 +152,39 @@ public List<ImageFile> uploadAsWebpWithSizes(
return result;
}

public List<ImageFile> uploadAsWebpWithSizes(
MultipartFile file,
int index,
List<Integer> widths,
List<Integer> heights
) {
if (widths.size() != heights.size()) {
// 해당 예외는 이미지 예외 처리로 수행하지 않는다.
throw new IllegalArgumentException("widths와 heights의 길이가 일치해야 합니다.");
}

validateImageSize(file);
validateImageContentType(file);
validateExtension(file.getOriginalFilename());

String baseName = buildBaseName(index);
List<ImageFile> result = new ArrayList<>();

for (int i = 0; i < widths.size(); i++) {
ImageSize size = new ImageSize(widths.get(i), heights.get(i));

String key = baseName + "_" + size.width() + "x" + size.height() + ".webp";
byte[] bytes = convertToWebpWithSize(file, size);

putToS3(key, bytes, "image/webp");
String url = awsS3Properties.getPublicEndpoint() + "/" + key;

result.add(new ImageFile(key, url));
}

return result;
}

public void delete(String key) {
s3Client.deleteObject(builder -> builder
.bucket(awsS3Properties.getBucket())
Expand Down Expand Up @@ -197,9 +254,14 @@ private String extractExtension(String originalFilename) {
}

private void validateDir(String dir) {
if (dir == null || dir.isBlank()) {
throw new ImageException(ImageExceptionCode.DIR_REQUIRED);
}
// TODO: 모임 경로: FE 요청으로 루트 디렉토리로 개선했습니다.
// if (dir == null || dir.isBlank()) {
// throw new ImageException(ImageExceptionCode.DIR_REQUIRED);
// }

// if (!dir.matches("[a-zA-Z0-9_\\-/]+")) {
// throw new ImageException(ImageExceptionCode.DIR_INVALID_PATTERN);
// }
Comment on lines +257 to +264
Copy link

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The validateDir method has most of its validation logic commented out (lines 258-264), but still validates for path traversal (line 266-268) and trailing slashes (line 270-272). This creates an inconsistent state where:

  1. The method is still called by existing methods like uploadAsWebpWithSizes(String dir, ...) at line 132
  2. But it only performs partial validation

This makes the code confusing and error-prone. Consider either:

  • Removing the commented code if directory validation is no longer needed
  • Or creating a separate validation method for the new root-only functionality

The commented-out validation checks are still being called by the existing dir-based methods, so they may cause unexpected behavior.

Copilot uses AI. Check for mistakes.

if (dir.contains("..") || dir.startsWith("/")) {
throw new ImageException(ImageExceptionCode.DIR_INVALID_TRAVERSAL);
Expand All @@ -208,10 +270,6 @@ private void validateDir(String dir) {
if (dir.endsWith("/")) {
throw new ImageException(ImageExceptionCode.DIR_TRAILING_SLASH);
}

if (!dir.matches("[a-zA-Z0-9_\\-/]+")) {
throw new ImageException(ImageExceptionCode.DIR_INVALID_PATTERN);
}
}

private String buildKey(String dir, String originalFilename, int index) {
Expand All @@ -223,13 +281,30 @@ private String buildKey(String dir, String originalFilename, int index) {
return dir + "/" + timestamp + "_" + index + "_" + uuid + extension;
}

private String buildKey(String originalFilename, int index) {
String extension = extractExtension(originalFilename);
String timestamp = LocalDateTime.now()
.format(DateTimeFormatter.ofPattern("yyyyMMddHHmmss"));
String uuid = UUID.randomUUID().toString();

return timestamp + "_" + index + "_" + uuid + extension;
}


private String buildBaseName(int index) {
String timestamp = LocalDateTime.now()
.format(DateTimeFormatter.ofPattern("yyyyMMddHHmmss"));
String uuid = UUID.randomUUID().toString();
return timestamp + "_" + index + "_" + uuid;
}

private String buildBaseName() {
String timestamp = LocalDateTime.now()
.format(DateTimeFormatter.ofPattern("yyyyMMddHHmmss"));
String uuid = UUID.randomUUID().toString();
return timestamp + "_" + uuid;
}

private byte[] resizeIfNeededKeepFormat(MultipartFile file) {
return resizeToBox(
file,
Expand Down
1 change: 0 additions & 1 deletion src/main/java/team/wego/wegobackend/image/presentation/ImageController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package team.wego.wegobackend.image.presentation;

import java.util.ArrayList;
import java.util.List;
import lombok.RequiredArgsConstructor;
import org.springframework.http.HttpStatus;
Expand Down
8 changes: 4 additions & 4 deletions src/test/http/group/create.http
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,22 +140,22 @@ POST http://localhost:8080/api/v1/groups/images/{{groupId}}/upload
?userId=1
Content-Type: multipart/form-data; boundary=boundary

--boundary
--boundary--
Copy link

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The boundary separator syntax is incorrect. In multipart/form-data, boundary separators should use --boundary (two hyphens at the start), not --boundary-- (which is only used for the final closing boundary). This will cause the HTTP request to fail or be malformed.

Each part should start with --boundary and only the very last line after all parts should be --boundary--.

Copilot uses AI. Check for mistakes.
Content-Disposition: form-data; name="images"; filename="test-webp1.webp"
Content-Type: image/webp

< ../image/resources/test-webp1.webp
--boundary
--boundary--
Copy link

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The boundary separator syntax is incorrect. In multipart/form-data, boundary separators should use --boundary (two hyphens at the start), not --boundary-- (which is only used for the final closing boundary). This will cause the HTTP request to fail or be malformed.

Each part should start with --boundary and only the very last line after all parts should be --boundary--.

Copilot uses AI. Check for mistakes.
Content-Disposition: form-data; name="images"; filename="test-webp1.webp"
Content-Type: image/webp

< ../image/resources/test-webp1.webp
--boundary
--boundary--
Copy link

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The boundary separator syntax is incorrect. In multipart/form-data, boundary separators should use --boundary (two hyphens at the start), not --boundary-- (which is only used for the final closing boundary). This will cause the HTTP request to fail or be malformed.

Each part should start with --boundary and only the very last line after all parts should be --boundary--.

Copilot uses AI. Check for mistakes.
Content-Disposition: form-data; name="images"; filename="img1.png"
Content-Type: image/png

< ../image/resources/img1.png
--boundary
--boundary--
Copy link

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The boundary separator syntax is incorrect. In multipart/form-data, boundary separators should use --boundary (two hyphens at the start), not --boundary-- (which is only used for the final closing boundary). This will cause the HTTP request to fail or be malformed.

Each part should start with --boundary and only the very last line after all parts should be --boundary--.

Copilot uses AI. Check for mistakes.
Content-Disposition: form-data; name="images"; filename="img2.jpg"
Content-Type: image/jpeg

Expand Down
Loading