Skip to content
Closed

Dev #154

Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 2 additions & 1 deletion build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ dependencies {
implementation 'org.springframework.boot:spring-boot-starter-oauth2-client'
implementation 'org.springframework.boot:spring-boot-starter-validation'
implementation 'org.springframework.boot:spring-boot-starter-data-redis'
implementation 'software.amazon.awssdk:s3:2.25.40'

// Swagger 설정
implementation 'org.springdoc:springdoc-openapi-starter-webmvc-ui:2.2.0'
Expand Down Expand Up @@ -75,4 +76,4 @@ sourceSets {

tasks.named('test') {
useJUnitPlatform() // JUnit5 명시적 활성화
}
}
26 changes: 26 additions & 0 deletions src/main/java/com/valanse/valanse/common/config/R2Config.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
package com.valanse.valanse.common.config;

import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import software.amazon.awssdk.auth.credentials.AwsBasicCredentials;
import software.amazon.awssdk.auth.credentials.StaticCredentialsProvider;
import software.amazon.awssdk.regions.Region;
import software.amazon.awssdk.services.s3.S3Client;

import java.net.URI;

@Configuration
public class R2Config {

@Bean
public S3Client s3Client(R2Properties properties) {
return S3Client.builder()
.region(Region.of("auto"))
.endpointOverride(URI.create(properties.getEndpoint()))
.credentialsProvider(StaticCredentialsProvider.create(
AwsBasicCredentials.create(properties.getAccessKey(), properties.getSecretKey())
))
.forcePathStyle(true)
.build();
}
}
64 changes: 64 additions & 0 deletions src/main/java/com/valanse/valanse/common/config/R2Properties.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
package com.valanse.valanse.common.config;

import org.springframework.boot.context.properties.ConfigurationProperties;
import org.springframework.stereotype.Component;

@Component
@ConfigurationProperties(prefix = "cloudflare.r2")
public class R2Properties {

private String accountId;
private String endpoint;
private String bucket;
private String accessKey;
private String secretKey;
private String publicUrl;
Comment on lines +6 to +15
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# R2Properties의 필수값 검증 어노테이션 적용 여부 확인
fd -i R2Properties.java src | xargs -I{} sh -c '
  echo "== {} ==";
  rg -n "`@ConfigurationProperties`|`@Validated`|`@NotBlank`|private String (accountId|endpoint|bucket|accessKey|secretKey|publicUrl)" "{}"
'

Repository: ValanSee/ValanSe_Server

Length of output: 386


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# 1) R2Properties.java 전체 확인
echo "---- R2Properties.java (head/tail with line numbers) ----"
FILE="src/main/java/com/valanse/valanse/common/config/R2Properties.java"
if [ -f "$FILE" ]; then
  nl -ba "$FILE" | sed -n '1,220p'
else
  echo "Missing $FILE"
fi

# 2) 이 프로퍼티(클래스)를 참조하는 곳 확인
echo
echo "---- Usages of R2Properties ----"
rg -n "R2Properties" src || true

# 3) 공용 URL/endpoint/accountId 등을 실제로 쓰는 곳에서 URI.create(등) 여부 확인
echo
echo "---- URI.create usages around R2/endpoint/publicUrl ----"
rg -n "URI\.create|publicUrl|endpoint|accountId|secretKey|accessKey" src/main/java/com/valanse/valanse || true

Repository: ValanSee/ValanSe_Server

Length of output: 186


🏁 Script executed:

#!/bin/bash
set -euo pipefail

FILE="src/main/java/com/valanse/valanse/common/config/R2Properties.java"

echo "---- R2Properties.java (line-numbered) ----"
if [ -f "$FILE" ]; then
  cat -n "$FILE" | sed -n '1,220p'
else
  echo "Missing $FILE"
fi

echo
echo "---- Usages of R2Properties ----"
rg -n "R2Properties" src || true

echo
echo "---- URI.create usages and related fields ----"
rg -n "URI\.create" src || true
rg -n "publicUrl|endpoint|accountId|accessKey|secretKey|bucket" src/main/java/com/valanse/valanse || true

Repository: ValanSee/ValanSe_Server

Length of output: 6433


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# show relevant sections
echo "---- R2Config.java (key parts) ----"
FILE="src/main/java/com/valanse/valanse/common/config/R2Config.java"
if [ -f "$FILE" ]; then
  sed -n '1,120p' "$FILE" | nl -ba
else
  echo "Missing $FILE"
fi

echo
echo "---- R2StorageService.java (key parts) ----"
FILE="src/main/java/com/valanse/valanse/service/StorageService/R2StorageService.java"
if [ -f "$FILE" ]; then
  sed -n '1,120p' "$FILE" | nl -ba
else
  echo "Missing $FILE"
fi

Repository: ValanSee/ValanSe_Server

Length of output: 164


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "---- R2Config.java (selected lines) ----"
FILE="src/main/java/com/valanse/valanse/common/config/R2Config.java"
if [ -f "$FILE" ]; then
  cat -n "$FILE" | sed -n '1,140p'
else
  echo "Missing $FILE"
fi

echo
echo "---- R2StorageService.java (selected lines) ----"
FILE="src/main/java/com/valanse/valanse/service/StorageService/R2StorageService.java"
if [ -f "$FILE" ]; then
  cat -n "$FILE" | sed -n '1,140p'
else
  echo "Missing $FILE"
fi

Repository: ValanSee/ValanSe_Server

Length of output: 4918


🏁 Script executed:

#!/bin/bash
set -euo pipefail

rg -n "import (jakarta\.validation|javax\.validation)\.constraints\.NotBlank|`@NotBlank`|import org\.springframework\.validation\.annotation\.Validated|`@Validated`" src || true

Repository: ValanSee/ValanSe_Server

Length of output: 49


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "---- root build files ----"
ls -la

# pom.xml / build.gradle 계열 확인
rg -n "spring-boot-starter-validation|validation-api|jakarta\.validation|javax\.validation" -S . || true

Repository: ValanSee/ValanSe_Server

Length of output: 1136


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "---- build.gradle (validation-related section) ----"
sed -n '1,120p' build.gradle | cat -n

echo
echo "---- jakarta/javax validation imports in src ----"
rg -n "jakarta\.validation|javax\.validation" src || true

echo
echo "---- Spring `@Validated` usage in src ----"
rg -n "org\.springframework\.validation\.annotation\.Validated|`@Validated`" src || true

Repository: ValanSee/ValanSe_Server

Length of output: 3036


R2 필수 설정값을 부팅 시점에 검증해 주세요.

  • R2Properties는 @ConfigurationProperties만 있고 @Validated/@notblank 제약이 없어 null/빈 문자열이 그대로 바인딩됩니다.
  • 실제로 R2Config.s3Client에서 URI.create(properties.getEndpoint())와 AwsBasicCredentials.create(properties.getAccessKey(), properties.getSecretKey())를 즉시 호출하므로 null/형식 불일치 시 부팅 중 NPE/IllegalArgumentException 같은 모호한 예외로 실패할 수 있습니다.
제안 diff
+import jakarta.validation.constraints.NotBlank;
 import org.springframework.boot.context.properties.ConfigurationProperties;
+import org.springframework.validation.annotation.Validated;
 import org.springframework.stereotype.Component;

 `@Component`
 `@ConfigurationProperties`(prefix = "cloudflare.r2")
+@Validated
 public class R2Properties {

+    `@NotBlank`
     private String accountId;
+    `@NotBlank`
     private String endpoint;
+    `@NotBlank`
     private String bucket;
+    `@NotBlank`
     private String accessKey;
+    `@NotBlank`
     private String secretKey;
+    `@NotBlank`
     private String publicUrl;
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/main/java/com/valanse/valanse/common/config/R2Properties.java` around
lines 6 - 15, R2Properties lacks validation so null/empty values cause ambiguous
exceptions when R2Config.s3Client calls URI.create and
AwsBasicCredentials.create; add validation to enforce required fields by
annotating the class with `@Validated` (or switching to constructor binding with
`@ConstructorBinding`) and mark each required String field (accountId, endpoint,
bucket, accessKey, secretKey, publicUrl if required) with `@NotBlank/`@NotNull (or
use `@NotEmpty`) and provide clear validation messages so Spring fails fast with a
readable error on startup; ensure R2Config.s3Client continues to reference
properties.getEndpoint(), getAccessKey(), getSecretKey() but rely on validated
properties to avoid NPE/IllegalArgumentException.


public String getAccountId() {
return accountId;
}

public void setAccountId(String accountId) {
this.accountId = accountId;
}

public String getEndpoint() {
return endpoint;
}

public void setEndpoint(String endpoint) {
this.endpoint = endpoint;
}

public String getBucket() {
return bucket;
}

public void setBucket(String bucket) {
this.bucket = bucket;
}

public String getAccessKey() {
return accessKey;
}

public void setAccessKey(String accessKey) {
this.accessKey = accessKey;
}

public String getSecretKey() {
return secretKey;
}

public void setSecretKey(String secretKey) {
this.secretKey = secretKey;
}

public String getPublicUrl() {
return publicUrl;
}

public void setPublicUrl(String publicUrl) {
this.publicUrl = publicUrl;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ public SecurityFilterChain securityFilterChain(HttpSecurity http) throws Excepti
.requestMatchers(HttpMethod.POST, "/votes/*/vote-options/*").authenticated()
.requestMatchers(HttpMethod.POST, "/votes/*/comments").authenticated()
.requestMatchers(HttpMethod.POST, "/comments/*/like").authenticated()
.requestMatchers(HttpMethod.POST, "/storage/images").authenticated()

// PUT - 인증 필요 (수정)
.requestMatchers(HttpMethod.PUT, "/votes/*").authenticated()
Expand Down Expand Up @@ -101,13 +102,7 @@ public CorsConfigurationSource corsConfigurationSource() {
"https://valan-se-web.vercel.app",
"https://valanse.kr",
"https://develop.valanse.kr",
"https://backendbase.store",
"http://backendbase.store:8080",
"http://backendbase.store:8081",
"http://backendbase.store:8082",
"https://backendbase.store:8080",
"https://backendbase.store:8081",
"https://backendbase.store:8082",
"https://valanserver.store",
"http://valanserver.store",
"http://valanserver.store:8080",
"http://valanserver.store:8081",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
package com.valanse.valanse.controller;

import com.valanse.valanse.dto.Storage.ImageUploadResponse;
import com.valanse.valanse.service.StorageService.StorageService;
import io.swagger.v3.oas.annotations.Operation;
import io.swagger.v3.oas.annotations.tags.Tag;
import lombok.RequiredArgsConstructor;
import org.springframework.http.ResponseEntity;
import org.springframework.web.bind.annotation.PostMapping;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RequestPart;
import org.springframework.web.bind.annotation.RestController;
import org.springframework.web.multipart.MultipartFile;

@Tag(name = "파일 업로드 API", description = "이미지 업로드 관련 기능")
@RestController
@RequestMapping("/storage")
@RequiredArgsConstructor
public class StorageController {

private final StorageService storageService;

@Operation(
summary = "이미지 업로드",
description = "이미지 파일을 Cloudflare R2에 업로드하고 공개 URL을 반환합니다."
)
@PostMapping(value = "/images", consumes = "multipart/form-data")
public ResponseEntity<ImageUploadResponse> uploadImage(@RequestPart("file") MultipartFile file) {
String imageUrl = storageService.uploadImage(file, "images");
return ResponseEntity.ok(new ImageUploadResponse(imageUrl));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
package com.valanse.valanse.dto.Storage;

public record ImageUploadResponse(
String imageUrl
) {
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
package com.valanse.valanse.service.StorageService;

import com.valanse.valanse.common.api.ApiException;
import com.valanse.valanse.common.config.R2Properties;
import lombok.RequiredArgsConstructor;
import org.springframework.http.HttpStatus;
import org.springframework.stereotype.Service;
import org.springframework.util.StringUtils;
import org.springframework.web.multipart.MultipartFile;
import software.amazon.awssdk.core.sync.RequestBody;
import software.amazon.awssdk.services.s3.S3Client;
import software.amazon.awssdk.services.s3.model.PutObjectRequest;
import software.amazon.awssdk.services.s3.model.S3Exception;

import java.io.IOException;
import java.util.Set;
import java.util.UUID;

@Service
@RequiredArgsConstructor
public class R2StorageService implements StorageService {

private static final long MAX_IMAGE_SIZE = 5 * 1024 * 1024;
private static final Set<String> ALLOWED_IMAGE_TYPES = Set.of(
"image/jpeg",
"image/png",
"image/webp",
"image/gif"
);

private final S3Client s3Client;
private final R2Properties properties;

@Override
public String uploadImage(MultipartFile file, String directory) {
validateImage(file);

String objectKey = buildObjectKey(file, directory);
PutObjectRequest request = PutObjectRequest.builder()
.bucket(properties.getBucket())
.key(objectKey)
.contentType(file.getContentType())
.contentLength(file.getSize())
.build();

try {
s3Client.putObject(request, RequestBody.fromInputStream(file.getInputStream(), file.getSize()));
} catch (IOException e) {
throw new ApiException("이미지 파일을 읽을 수 없습니다.", HttpStatus.BAD_REQUEST);
} catch (S3Exception e) {
throw new ApiException("이미지 업로드에 실패했습니다.", HttpStatus.BAD_GATEWAY);
}

return properties.getPublicUrl().replaceAll("/+$", "") + "/" + objectKey;
}

private void validateImage(MultipartFile file) {
if (file == null || file.isEmpty()) {
throw new ApiException("업로드할 이미지 파일을 선택해주세요.", HttpStatus.BAD_REQUEST);
}

if (file.getSize() > MAX_IMAGE_SIZE) {
throw new ApiException("이미지는 5MB 이하만 업로드할 수 있습니다.", HttpStatus.BAD_REQUEST);
}

if (!ALLOWED_IMAGE_TYPES.contains(file.getContentType())) {
throw new ApiException("jpg, png, webp, gif 이미지 파일만 업로드할 수 있습니다.", HttpStatus.BAD_REQUEST);
}
Comment on lines +57 to +68
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# MIME-only 검증 여부 확인 (바이너리 시그니처 검증 코드 유무 탐색)
fd -i R2StorageService.java src | xargs -I{} sh -c '
  echo "== {} ==";
  rg -n "ALLOWED_IMAGE_TYPES|getContentType|ImageIO|Tika|magic" "{}"
'

Repository: ValanSee/ValanSe_Server

Length of output: 348


🏁 Script executed:

#!/bin/bash
set -euo pipefail

FILE="src/main/java/com/valanse/valanse/service/StorageService/R2StorageService.java"

echo "===== validateImage() snippet ====="
nl -ba "$FILE" | sed -n '40,110p'

echo "===== allowed types set ====="
nl -ba "$FILE" | sed -n '15,55p'

echo "===== repo-wide image signature validation hints ====="
rg -n "ImageIO|Tika|magic|contentType\\(file\\.getContentType\\)|getInputStream\\(" src/main/java || true

echo "===== validateImage 호출/흐름 ====="
rg -n "validateImage\\(" -S src/main/java || true

Repository: ValanSee/ValanSe_Server

Length of output: 164


🏁 Script executed:

#!/bin/bash
set -euo pipefail

FILE="src/main/java/com/valanse/valanse/service/StorageService/R2StorageService.java"

echo "===== validateImage() snippet (line 40-110) ====="
awk 'NR>=40 && NR<=110 {printf "%5d:%s\n", NR, $0}' "$FILE" || true

echo "===== allowed types set (line 15-55) ====="
awk 'NR>=15 && NR<=55 {printf "%5d:%s\n", NR, $0}' "$FILE" || true

echo "===== repo-wide image signature validation hints ====="
rg -n "ImageIO|Tika|magic|getInputStream\\(" src/main/java || true
rg -n "contentType\\(file\\.getContentType\\)|getContentType\\(\\)" src/main/java || true

echo "===== validateImage 호출/흐름 ====="
rg -n "validateImage\\(" src/main/java || true

Repository: ValanSee/ValanSe_Server

Length of output: 4568


MIME 타입만으로는 이미지 검증이 부족합니다.
src/main/java/com/valanse/valanse/service/StorageService/R2StorageService.java의 validateImage()file.getContentType()와 크기만 검사하고(라인 66-68), 매직 바이트/디코딩 가능 여부 같은 바이너리 시그니처 검증이 없습니다. 따라서 Content-Type을 위조한 비이미지 업로드가 통과할 수 있으니, 최소한 ImageIO.read(...) 또는 매직 바이트 기반 검사로 유효한 이미지인지 확인을 추가해 주세요.

제안 diff
+import javax.imageio.ImageIO;
+import java.io.InputStream;
@@
     private void validateImage(MultipartFile file) {
@@
         if (!ALLOWED_IMAGE_TYPES.contains(file.getContentType())) {
             throw new ApiException("jpg, png, webp, gif 이미지 파일만 업로드할 수 있습니다.", HttpStatus.BAD_REQUEST);
         }
+
+        try (InputStream in = file.getInputStream()) {
+            if (ImageIO.read(in) == null) {
+                throw new ApiException("유효한 이미지 파일이 아닙니다.", HttpStatus.BAD_REQUEST);
+            }
+        } catch (IOException e) {
+            throw new ApiException("이미지 파일을 읽을 수 없습니다.", HttpStatus.BAD_REQUEST);
+        }
     }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@src/main/java/com/valanse/valanse/service/StorageService/R2StorageService.java`
around lines 57 - 68, validateImage currently only checks file size and MIME
type (ALLOWED_IMAGE_TYPES, MAX_IMAGE_SIZE) which can be spoofed; update
R2StorageService.validateImage to also verify the binary is a real image by
attempting to decode it (e.g., ImageIO.read(file.getInputStream()) and throwing
ApiException if it returns null) and, because ImageIO doesn't support all
formats (e.g., webp), add a fallback magic-byte check for JPEG/PNG/GIF/WEBP by
peeking the first bytes of file.getInputStream() (ensure the stream is
resettable or use a buffered copy) and throw ApiException on mismatch; wrap
IOExceptions into ApiException and close streams properly.

}

private String buildObjectKey(MultipartFile file, String directory) {
String cleanDirectory = StringUtils.hasText(directory)
? directory.replaceAll("^/+|/+$", "")
: "images";
String extension = StringUtils.getFilenameExtension(file.getOriginalFilename());
String filename = extension == null
? UUID.randomUUID().toString()
: UUID.randomUUID() + "." + extension.toLowerCase();

return cleanDirectory + "/" + filename;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package com.valanse.valanse.service.StorageService;

import org.springframework.web.multipart.MultipartFile;

public interface StorageService {
String uploadImage(MultipartFile file, String directory);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
package com.valanse.valanse.service.StorageService;

import com.valanse.valanse.common.api.ApiException;
import com.valanse.valanse.common.config.R2Properties;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.springframework.mock.web.MockMultipartFile;
import software.amazon.awssdk.core.sync.RequestBody;
import software.amazon.awssdk.services.s3.S3Client;
import software.amazon.awssdk.services.s3.model.PutObjectRequest;

import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;

class R2StorageServiceTest {

private S3Client s3Client;
private R2StorageService storageService;

@BeforeEach
void setUp() {
s3Client = mock(S3Client.class);

R2Properties properties = new R2Properties();
properties.setBucket("test-bucket");
properties.setPublicUrl("https://cdn.example.com/");

storageService = new R2StorageService(s3Client, properties);
}

@Test
void 이미지를_R2에_업로드하고_공개_URL을_반환한다() {
MockMultipartFile file = new MockMultipartFile(
"file",
"sample.png",
"image/png",
"image".getBytes()
);

String imageUrl = storageService.uploadImage(file, "images");

assertThat(imageUrl).startsWith("https://cdn.example.com/images/");
assertThat(imageUrl).endsWith(".png");
verify(s3Client).putObject(any(PutObjectRequest.class), any(RequestBody.class));
}
Comment on lines +34 to +48
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy lift

S3 호출 검증이 불충분하고 파일 크기 검증 테스트가 누락되었습니다.

현재 테스트는 any() 매처만 사용하여 putObject 호출 여부만 확인하고 있습니다. 컨텍스트에 따르면 서비스는 파일 크기 및 MIME 타입을 검증해야 하지만, 이에 대한 테스트가 없습니다.

다음 개선사항을 권장합니다:

  1. putObject 호출 시 올바른 버킷명, 키(경로), Content-Type이 전달되는지 검증
  2. 파일 크기 제한 초과 시 예외 발생 테스트 추가
  3. 빈 파일, null 파일 등 경계 케이스 테스트 추가
🧪 검증 강화 예시
 `@Test`
 void 이미지를_R2에_업로드하고_공개_URL을_반환한다() {
     MockMultipartFile file = new MockMultipartFile(
             "file",
             "sample.png",
             "image/png",
             "image".getBytes()
     );

     String imageUrl = storageService.uploadImage(file, "images");

     assertThat(imageUrl).startsWith("https://cdn.example.com/images/");
     assertThat(imageUrl).endsWith(".png");
-    verify(s3Client).putObject(any(PutObjectRequest.class), any(RequestBody.class));
+    
+    ArgumentCaptor<PutObjectRequest> requestCaptor = ArgumentCaptor.forClass(PutObjectRequest.class);
+    verify(s3Client).putObject(requestCaptor.capture(), any(RequestBody.class));
+    
+    PutObjectRequest request = requestCaptor.getValue();
+    assertThat(request.bucket()).isEqualTo("test-bucket");
+    assertThat(request.key()).startsWith("images/");
+    assertThat(request.contentType()).isEqualTo("image/png");
 }

추가 테스트 케이스:

`@Test`
void 파일_크기가_제한을_초과하면_예외가_발생한다() {
    byte[] largeContent = new byte[11 * 1024 * 1024]; // 11MB
    MockMultipartFile file = new MockMultipartFile(
            "file",
            "large.png",
            "image/png",
            largeContent
    );

    assertThrows(ApiException.class, () -> storageService.uploadImage(file, "images"));
}

`@Test`
void 빈_파일은_업로드하지_않는다() {
    MockMultipartFile file = new MockMultipartFile(
            "file",
            "empty.png",
            "image/png",
            new byte[0]
    );

    assertThrows(ApiException.class, () -> storageService.uploadImage(file, "images"));
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@src/test/java/com/valanse/valanse/service/StorageService/R2StorageServiceTest.java`
around lines 34 - 48, Update the R2StorageServiceTest to strengthen S3 call
verification and add boundary tests: when asserting the putObject call
(s3Client.putObject) capture the PutObjectRequest and RequestBody (or use
argument matchers) and verify the request's bucket name, key/path contains the
"images" prefix, and the PutObjectRequest's metadata/content-type equals the
file's MIME type; also assert the RequestBody/byte content length equals the
uploaded byte length. Add tests calling storageService.uploadImage for oversized
input (e.g., 11MB) and empty/null payloads asserting ApiException is thrown (use
assertThrows(ApiException.class, ...)) and ensure the existing positive test
still verifies URL format and that putObject was invoked with the correct
captured values.


@Test
void 이미지가_아닌_파일은_업로드하지_않는다() {
MockMultipartFile file = new MockMultipartFile(
"file",
"sample.txt",
"text/plain",
"text".getBytes()
);

assertThrows(ApiException.class, () -> storageService.uploadImage(file, "images"));
}
Comment on lines +50 to +60
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy lift

예외 검증이 불충분하고 예외 매핑 테스트가 누락되었습니다.

현재 테스트는 ApiException이 발생하는지만 확인하고, 예외의 상세 정보(에러 코드, 메시지)나 S3 클라이언트가 호출되지 않았는지 검증하지 않습니다. 컨텍스트에 따르면 서비스는 IOException → BAD_REQUEST, S3Exception → BAD_GATEWAY로 매핑해야 하지만 이에 대한 테스트가 없습니다.

다음 개선사항을 권장합니다:

  1. ApiException의 에러 코드 및 메시지 검증
  2. 검증 실패 시 S3 호출이 발생하지 않았는지 확인
  3. S3Exception 발생 시 BAD_GATEWAY 매핑 테스트 추가
  4. IOException 발생 시 BAD_REQUEST 매핑 테스트 추가
🧪 예외 검증 강화 및 추가 테스트 예시
 `@Test`
 void 이미지가_아닌_파일은_업로드하지_않는다() {
     MockMultipartFile file = new MockMultipartFile(
             "file",
             "sample.txt",
             "text/plain",
             "text".getBytes()
     );

-    assertThrows(ApiException.class, () -> storageService.uploadImage(file, "images"));
+    ApiException exception = assertThrows(ApiException.class, 
+        () -> storageService.uploadImage(file, "images"));
+    
+    assertThat(exception.getErrorCode()).isEqualTo(ErrorCode.BAD_REQUEST);
+    verify(s3Client, never()).putObject(any(PutObjectRequest.class), any(RequestBody.class));
 }

추가 테스트 케이스:

`@Test`
void S3_업로드_실패_시_BAD_GATEWAY_예외가_발생한다() {
    MockMultipartFile file = new MockMultipartFile(
            "file",
            "sample.png",
            "image/png",
            "image".getBytes()
    );
    
    when(s3Client.putObject(any(PutObjectRequest.class), any(RequestBody.class)))
        .thenThrow(S3Exception.builder().message("S3 error").build());

    ApiException exception = assertThrows(ApiException.class, 
        () -> storageService.uploadImage(file, "images"));
    
    assertThat(exception.getErrorCode()).isEqualTo(ErrorCode.BAD_GATEWAY);
}

`@Test`
void IOException_발생_시_BAD_REQUEST_예외가_발생한다() throws Exception {
    MultipartFile file = mock(MultipartFile.class);
    when(file.getContentType()).thenReturn("image/png");
    when(file.getOriginalFilename()).thenReturn("sample.png");
    when(file.getSize()).thenReturn(100L);
    when(file.getBytes()).thenThrow(new IOException("Read error"));

    ApiException exception = assertThrows(ApiException.class, 
        () -> storageService.uploadImage(file, "images"));
    
    assertThat(exception.getErrorCode()).isEqualTo(ErrorCode.BAD_REQUEST);
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@src/test/java/com/valanse/valanse/service/StorageService/R2StorageServiceTest.java`
around lines 50 - 60, Update the tests for storageService.uploadImage to assert
detailed ApiException properties and S3 interactions: in the non-image test
assert the thrown ApiException's ErrorCode and message, and verify the mocked
s3Client.putObject (or equivalent) was never invoked; add a test that stubs
s3Client.putObject to throw S3Exception and assert the service maps it to
ErrorCode.BAD_GATEWAY; add a test that stubs MultipartFile.getBytes() to throw
IOException and assert the service maps it to ErrorCode.BAD_REQUEST (use
ApiException exception = assertThrows(...) and
assertThat(exception.getErrorCode()).isEqualTo(...), and use verify(s3Client,
never()).putObject(...) to confirm no S3 call on validation failure).

}
9 changes: 9 additions & 0 deletions src/test/resources/application-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -26,3 +26,12 @@ oauth:
kakao:
client-id: test-client-id
redirect-uri: http://localhost

cloudflare:
r2:
account-id: test-account-id
endpoint: http://localhost:9000
bucket: test-bucket
access-key: test-access-key
secret-key: test-secret-key
public-url: http://localhost:9000/test-bucket
Loading