-
Notifications
You must be signed in to change notification settings - Fork 3
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.
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
๐ ์ด๋ฏธ์ง ๊ด๋ จ validation ์ถ๊ฐ์ ๊ฐ ์ํฉ์ ๋ง๋ ํ ์คํธ ์ฝ๋ ์์ฑ #66
Conversation
10MB -> 100MB
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
์ PR ๋ฉ์ธ ๋ธ๋์น๋ก ๋จธ์ง๋๊ฒ ํด๋์ ๊ฒ ๊ฐ์ต๋๋ค! ํ์ธ ํ ๋ฒ ํด์ฃผ์๋ฉด ๊ฐ์ฌํ๊ฒ ์ต๋๋ค!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
๐
src/main/java/com/backendoori/ootw/avatar/validation/AvatarImageValidator.java
Outdated
Show resolved
Hide resolved
src/main/java/com/backendoori/ootw/common/image/exception/SaveException.java
Show resolved
Hide resolved
src/main/java/com/backendoori/ootw/common/image/exception/ImageControllerAdvice.java
Outdated
Show resolved
Hide resolved
src/main/java/com/backendoori/ootw/common/validation/Image.java
Outdated
Show resolved
Hide resolved
if (!postImg.isEmpty()) { | ||
ImageFile imgFile = imageService.uploadImage(postImg); | ||
try { | ||
Post savedPost = postRepository.save(Post.from(user, request, imgFile.url(), temperatureArrange)); | ||
return PostSaveResponse.from(savedPost); | ||
} catch (Exception e) { | ||
imageService.deleteImage(imgFile.fileName()); | ||
throw new SaveException(); | ||
} | ||
} | ||
Post savedPost = postRepository.save(Post.from(user, request, null, temperatureArrange)); | ||
|
||
return PostSaveResponse.from(savedPost); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
์กฐ๊ฑด๋ฌธ์ ๋ฐ๋๋กํด์ ๊ตฌ์กฐ๋ฅผ ๋ฐ๊พธ๋ฉด ๋ ๊น๋ํ ๊ฒ ๊ฐ์ต๋๋ค!
src/test/java/com/backendoori/ootw/avatar/domain/AvatarItemTest.java
Outdated
Show resolved
Hide resolved
src/test/java/com/backendoori/ootw/avatar/service/AvatarItemServiceTest.java
Outdated
Show resolved
Hide resolved
src/test/java/com/backendoori/ootw/avatar/service/AvatarItemServiceTest.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
๊ณ ์ํ์
จ์ต๋๋ค!
๊ฐ์ด ์ด์ผ๊ธฐํ ๋ถ๋ถ๋ง ์์ ํ๊ณ ๋จธ์งํ๋ฉด ๋ ๊ฒ ๊ฐ์์!
๐ Jacoco ํ ์คํธ ๊ฒฐ๊ณผ
|
๐ Jacoco ํ ์คํธ ๊ฒฐ๊ณผ
|
๐ Jacoco ํ ์คํธ ๊ฒฐ๊ณผ
|
Image domain์ ๋ฉ์๋ ๋ช ์ ํฌํจํ์ง ์๋ ๋ฐฉ์์ผ๋ก ๋ณ๊ฒฝ
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
๊ฐ์ด ๋ฆฌ๋ทฐ ํ๋ ๋ถ๋ถ์ด ๋ฐ์์ด ์ ๋ง ์ ๋๋ค์!
ImageFile uploadImage(MultipartFile file); | ||
|
||
void deleteImage(String fileName); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
์ด ๋ถ๋ถ์ Image ๋จ์ด๊ฐ ์์ด๋ ์ข์ ๊ฒ ๊ฐ์์
Assert.notNull(sex, SEX_ESSENTIAL); | ||
Assert.isTrue(!sex.isBlank(), SEX_ESSENTIAL); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AssertUtil
์จ์ ํ ์ค๋ก ํ๋ ๊ฒ๋ ์ข์ ๊ฒ ๊ฐ์์
AssertUtil.notBlank()
๊ฐ ๋๊ฒ ๋ค์
@Slf4j | ||
@RestControllerAdvice | ||
public class ImageControllerAdvice { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
์ด ๋ถ๋ถ์ Order๋ฅผ ์ง์ ์ํด์ค๋ ์ ๋๋์?
์ ๋ UserContollerAdvice
์์ ์ฐ์ ์์๊ฐ ๋ฐ๋ฆฌ๋ ๋ฌธ์ ๊ฐ ์์๊ฑฐ๋ ์
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
test์์ ์ ์์ ์ผ๋ก ์๋ํด์ ๋ฌธ์ ์์ด๋ณด์ ๋๋ค!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
์ ๊ฐ ์กฐ๊ธ ์ฐพ์๋ณด๋ @ControllerAdvice
์ ์ค๋๋ง์ ๊ธฐ๋ณธ์ ์ผ๋ก class hierarchy๋ฅผ ๋ฐ๋ผ๊ฐ๋ค๊ณ ํ๋ค์ ์ถํ ํจํค์ง ๋ช
์ด ๋ฐ๋์์ ๋ ์์์น ๋ชป ํ status ๋ฐํ์ด ๋ ์ ์์ ๊ฒ ๊ฐ์์ ๋์ ํด๋์ค ์ง์ ๊ณผ ์ค๋๋ง์ ์ํ ์์ ์ด ํ์ํ ๊ฒ ๊ฐ์ต๋๋ค
} | ||
|
||
String contentType = img.getContentType(); | ||
if (!contentType.startsWith("image")) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
image
๋ฌธ์์ด์ ์์๋ก ํ๋ ๊ฒ๋ ์ข์ ๊ฒ ๊ฐ์์
public static void validateImage(MultipartFile img) { | ||
if (img == null) { | ||
throw new IllegalArgumentException(EMPTY_FILE); | ||
} | ||
if (img.isEmpty()) { | ||
throw new IllegalArgumentException(EMPTY_FILE); | ||
} | ||
|
||
if (img.getSize() > 10_000_000) { | ||
return false; | ||
throw new IllegalArgumentException(FILE_OVER_SIZE); | ||
} | ||
|
||
String contentType = img.getContentType(); | ||
if (!contentType.startsWith("image")) { | ||
return false; | ||
throw new IllegalArgumentException(INVALID_FILE_TYPE); | ||
} | ||
|
||
return true; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
์ด ๋ถ๋ถ๋ Assert๋ฅผ ์ฌ์ฉํ๋ฉด ์ข์ ๊ฒ ๊ฐ์์
if (postImg.isEmpty()) { | ||
Post savedPost = postRepository.save(Post.from(user, request, null, temperatureArrange)); | ||
return PostSaveResponse.from(savedPost); | ||
} | ||
|
||
ImageFile imgFile = imageService.uploadImage(postImg); | ||
try { | ||
Post savedPost = postRepository.save(Post.from(user, request, imgFile.url(), temperatureArrange)); | ||
|
||
return PostSaveResponse.from(savedPost); | ||
return PostSaveResponse.from(savedPost); | ||
} catch (Exception e) { | ||
imageService.deleteImage(imgFile.fileName()); | ||
throw new SaveException(); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
๐
@@ -21,14 +28,18 @@ | |||
import org.springframework.mock.web.MockMultipartFile; | |||
import org.springframework.security.test.context.support.WithMockUser; | |||
import org.springframework.test.web.servlet.MockMvc; | |||
import org.springframework.web.multipart.MultipartFile; | |||
|
|||
@WithMockUser | |||
@AutoConfigureMockMvc |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
์ด ์ด๋ ธํ ์ด์ ์ ์ด๋ค ์ญํ ์ ํ๋์?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@AutoConfigureMockMvc
๋ฅผ ์จ์ MockMvc๋ฅผ ๋ถ๋ฌ์ค๊ณ ์์ด์ :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
์ํ WebMvcTest
๊ฐ ์๋๋ผ SpringBootTest
๋ก ํ์
์ ๊ทธ๋ ๊ตฐ์
@ParameterizedTest(name = "[{index}]: ์์ดํ ํ์ ์ด {0}์ธ ๊ฒฝ์ฐ์ ์ ์ฅ์ ์ฑ๊ณตํ๋ค.") | ||
@ValueSource(strings = {"image/jpeg", "image/gif", "image/png"}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
์ ๋ง ์ข๋ค์ ๐ image/jpg
๋ ๋ค์ด๊ฐ๋ฉด ์ข์ ๊ฒ ๊ฐ์์!
MockMultipartFile postImg = | ||
new MockMultipartFile("postImg", "filename.txt", MediaType.MULTIPART_FORM_DATA_VALUE, | ||
new MockMultipartFile("postImg", "filename.jpeg", MediaType.IMAGE_JPEG_VALUE, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Sehee-Lee-01 Post ํ
์คํธ์์๋ @ParameterizedTest
๋ก ์ด๋ฏธ์ง ํ์์ ํ
์คํธํ๋ฉด ์ข์ ๊ฒ ๊ฐ์์!
return java.util.stream.Stream.of( | ||
org.junit.jupiter.params.provider.Arguments.of(null, validSex), | ||
org.junit.jupiter.params.provider.Arguments.of(validType, null), | ||
org.junit.jupiter.params.provider.Arguments.of("", validSex), | ||
org.junit.jupiter.params.provider.Arguments.of(validType, ""), | ||
org.junit.jupiter.params.provider.Arguments.of(" ", validSex), | ||
org.junit.jupiter.params.provider.Arguments.of(validType, " "), | ||
org.junit.jupiter.params.provider.Arguments.of("hair", validSex), | ||
org.junit.jupiter.params.provider.Arguments.of(validType, "female") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
static import๋ก ๋ฐ๊ถ์ฃผ์๋ฉด ๊ฐ์ฌํ๊ฒ ์ต๋๋ค!
๐ Jacoco ํ ์คํธ ๊ฒฐ๊ณผ
|
๐ Jacoco ํ ์คํธ ๊ฒฐ๊ณผ
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
์ธํฌ๋ PR์ ๋ค์ ๋ณด๋ค ๋ณด๋ ์ค์๋ PR๊ณผ ๊ด๋ จ๋๊ฒ ๋ง์ ๊ฒ ๊ฐ๋ค์
์ถํ ๋ฆฌํํ ๋ง์์ ๊ฐ์ด ์ด์ผ๊ธฐํด๋ณด๋ฉด ์ข์ ๊ฒ ๊ฐ์ต๋๋ค
validateImage(image); | ||
validateItemType(type); | ||
validateSex(sex); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
์ด ๋ถ๋ถ์ด static import method ์ธ ๊ฑธ ๋์ณค์๋ค์
์ธํฌ๋ PR์์ ์ด์ผ๊ธฐ ํ๋ ๊ฑด๋ฐ ๊ณ ๋ฏผํด๋ณผ ์ฃผ์ ์ธ ๊ฒ ๊ฐ์ต๋๋ค
public static boolean checkValue(String itemType) { | ||
return Arrays.stream(ItemType.values()) | ||
.anyMatch(e -> e.name().equals(itemType)); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
์ง๊ธ ์๊ฐํด๋ณด๋ฉด boolean ๋ฉ์๋๋ค ๋ณด๋ hasValue
๋ Enum ํด๋์ค๋๊น hasName
์ด ๋ ์ข์ ๋ฉ์๋ ๋ช
์ผ ๊ฒ ๊ฐ๋ค์
๋ค์ refactoring ๋ ๊ณ ๋ คํด๋ณด๋ฉด ์ข์ ๊ฒ ๊ฐ์ต๋๋ค
try { | ||
String url = imageFile.url(); | ||
AvatarItem savedItem = avatarItemRepository.save(AvatarItem.create(requestDto, url)); | ||
|
||
return AvatarItemResponse.from(savedItem); | ||
return AvatarItemResponse.from(savedItem); | ||
} catch (Exception e) { | ||
imageService.delete(imageFile.fileName()); | ||
throw new SaveException(); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
์ธํฌ๋ PR์ ๋ณด๋ image ์ฒ๋ฆฌ transaction ๋ฉ์๋๊ฐ ํ์ํ๋ค๋ ์๊ฐ์ด ๋ค๋๋ผ๊ณ ์
Supplier๋ฅผ ๋ฐ์์ ๋ฉ์๋๋ฅผ ์ฒ๋ฆฌํ ์ ์๋๋ก transaction ๊ธฐ๋ฅ ๋ฉ์๋๋ฅผ ๋ง๋ค ํ์๋ ์์ด๋ณด์
๋๋ค!
import static com.backendoori.ootw.common.image.exception.ImageException.IMAGE_ROLLBACK_FAIL_MESSAGE; | ||
import static com.backendoori.ootw.common.image.exception.ImageException.IMAGE_UPLOAD_FAIL_MESSAGE; | ||
import static com.backendoori.ootw.common.validation.ImageValidator.validateImage; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
์ด๋ ๊ฒ ๋ฉ์์ง๋ฅผ static import ํ๋ ๋ถ๋ถ๋ ์ถํ ๋ฆฌํฉํ ๋ง ๋ ๊ฐ์ด ์ด์ผ๊ธฐํด๋ด์!
@Slf4j | ||
@RestControllerAdvice | ||
public class ImageControllerAdvice { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
์ ๊ฐ ์กฐ๊ธ ์ฐพ์๋ณด๋ @ControllerAdvice
์ ์ค๋๋ง์ ๊ธฐ๋ณธ์ ์ผ๋ก class hierarchy๋ฅผ ๋ฐ๋ผ๊ฐ๋ค๊ณ ํ๋ค์ ์ถํ ํจํค์ง ๋ช
์ด ๋ฐ๋์์ ๋ ์์์น ๋ชป ํ status ๋ฐํ์ด ๋ ์ ์์ ๊ฒ ๊ฐ์์ ๋์ ํด๋์ค ์ง์ ๊ณผ ์ค๋๋ง์ ์ํ ์์ ์ด ํ์ํ ๊ฒ ๊ฐ์ต๋๋ค
public class ImageAnnotationValidator implements ConstraintValidator<Image, MultipartFile> { | ||
|
||
private static final String IMAGE_PREFIX = "image"; | ||
private Image annotation; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
9๋ฒ๊ณผ 10๋ฒ ํ ์ฌ์ด์ ๊ฐํ์ ํด์ฃผ๋ฉด ์ข์ ๊ฒ ๊ฐ์ต๋๋ค
if (postImg.isEmpty()) { | ||
Post savedPost = postRepository.save(Post.from(user, request, null, temperatureArrange)); | ||
return PostSaveResponse.from(savedPost); | ||
} | ||
|
||
ImageFile imgFile = imageService.upload(postImg); | ||
try { | ||
Post savedPost = postRepository.save(Post.from(user, request, imgFile.url(), temperatureArrange)); | ||
|
||
return PostSaveResponse.from(savedPost); | ||
return PostSaveResponse.from(savedPost); | ||
} catch (Exception e) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Sehee-Lee-01 ์ธํฌ๋๊ป์ ํด๋น ๋ก์ง์ ์ฐธ๊ณ ํ์ จ๋ ๊ฒ ๊ฐ๋ค์
Post savedPost = postRepository.save(Post.from(user, request, imgFile.url(), temperatureArrange));
return PostSaveResponse.from(savedPost);
ํด๋น ๋ถ๋ถ์ด ์ค๋ณต๋๋ ๋ถ๋ถ์ด ์๋๋ฐ private ๋ฉ์๋๋ก ๋ถ๋ฆฌํ๊ณ
private ๋ฉ์๋์์ postImage ์ฌ๋ถ์ ๋ฐ๋ผ null
์ด๋ imgFile.url()
์ ๋ฃ์ด์ฃผ๋ ์์ผ๋ก ๊ฐ์ ํ ์ ์์ ๊ฒ ๊ฐ์์
@BeforeEach | ||
void setup() { | ||
avatarItemRepository.deleteAll(); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
setup ๋ณด๋ค๋ cleanup์ ๊ฐ๊น์ ๋ณด์ด๋ค์!
๊ทธ๋ฆฌ๊ณ ๋ง์ง๋ง ํ
์คํธ ๊ฒฐ๊ณผ๊ฐ ์ง์์ง์ง ์์์ ๋ค๋ฅธ ํ
์คํธ์ ์ํฅ์ ์ค ์ ์์ ๊ฒ ๊ฐ์์
โ ์๊ตฌ์ฌํญ
๐ ์ฃผ์ ๋ณ๊ฒฝ์ฌํญ