🚀 Feature image enhancement #85

shoeone96 · 2024-01-19T14:57:37Z

✅ 요구사항

사용하고 있지않는 이미지에 대한 관리 필요
- 현재 수정, 삭제, 저장(저장 시 이미지만 저장되는 경우) 등 다양한 케이스에서 이미지가 삭제되거나 저장되고 있다.
- 이 과정에서 이미지는 RDB에 직접 저장되는 정보가 아니다 보니 사용하지 않는 이미지가 버켓 안에 남이 있을 수 있다.
- 사용하지 않는 이미지를 저장하고 있는 비용문제를 해결하기 위해 일정 주기로 확인해서 사용하지 않는 이미지에 대한 삭제가 필요하다.
- 스케쥴링을 이용해 사용하고 있지 않는 이미지를 삭제할 필요가 있다.
file 이름 관리
- 현재 로직에서는 파일에 사용된 이름 그대로 버킷에 저장하고 있는데 동일한 이름의 파일을 업로드 하는 경우 문제가 발생할 수 있다.
- 파일 이름을 고유값으로 넣어 저장할 필요가 있다.
이미지 directory 분리
- 이미지 서비스 비중이 커지다 보니 한 directory에 분리할 필요성 존재

🚀 주요 변경사항

스케쥴링을 이용해 사용하고 있지 않는 이미지를 삭제 기능
- 사용하지 않는 이미지를 저장하고 있는 비용문제를 해결하기 위해 일정 주기로 확인해서 사용하지 않는 이미지를 스케쥴링을 이용하여 삭제
file 이름 unique 값으로 관리
- 동일한 파일 이름을 분리하기 위해 앞에 UUID를 붙여 고유성을 부여
이미지 directory 분리 및 테이블 생성
- image directory 생성 및 table 생성하여 관리

💡 기타사항

현재 변경한 로직에 대해 전반적인 평가와 방향성 재고를 부탁드립니다.
추가적으로 현재 EnumValidator가 jacoco 범위에 포함되지 않아 문제가 발생하는데 새로 추가한 테스트에서도 debuging을 찍을 시 항상 거쳐 가는 것을 확인해 어떤 문제인지 같이 고민해보고 싶습니다.
마지막으로 scheduling 부분을 테스트하는 방법이 잘 떠오르지 않아 같이 고민해보고 싶습니다.

Sehee-Lee-01 · 2024-01-21T06:30:23Z

혹시 빌드쪽에서 에러가 나는 것 같은데 확인 한 번만 부탁드려도 괜찮을까욥!

shoeone96 · 2024-01-21T06:35:29Z

혹시 빌드쪽에서 에러가 나는 것 같은데 확인 한 번만 부탁드려도 괜찮을까욥!

적어놓긴 했는데 지금 jacoco 통과를 못하고 있고 테스트에 대한 여러분의 생각을 듣고 싶어요
기타 사항에 언급했듯 현재 테스트로직에서 EnumValidator를 거치는 테스트가 존재함에도 통과가 안되고 있고 스케쥴러에 대한 좋은 테스트 방법이 떠오르지 않아 의견을 여쭙고 있습니다 :) 두 부분이 jacoco를 통과하지 못해서 나는 오류에요!

Sehee-Lee-01

전반적으로 코드 리팩토링한 내용이었는데 클래스 분리, Enum validator 설정 덕분에 더 코드가 깔끔해진 것 같네요!
빌드 에러가 나는 부분 써주신 것을 못봤네요ㅠㅠㅠ 지금 확인했습니다!

Sehee-Lee-01 · 2024-01-21T07:06:08Z

src/main/java/com/backendoori/ootw/image/service/ImageUpdateService.java

+    @Scheduled(cron = CRON, zone = TIMEZONE) // 매월 20일 오전 00시 10분에 실행
+    @Async
+    @Transactional
+    public void deleteUnused(){


Sehee-Lee-01 · 2024-01-21T07:07:12Z

src/main/java/com/backendoori/ootw/image/service/MiniOImageServiceImpl.java

+    private static String getUniqueFileName(MultipartFile file) {
+        String randomUUID = UUID.randomUUID().toString();
+        String originalFileName = file.getOriginalFilename();
+
+        return randomUUID + originalFileName;
+    }


Sehee-Lee-01 · 2024-01-21T08:09:12Z

src/main/java/com/backendoori/ootw/avatar/dto/AvatarItemRequest.java

@@ -3,7 +3,6 @@
 import com.backendoori.ootw.avatar.domain.ItemType;
 import com.backendoori.ootw.avatar.domain.Sex;
 import com.backendoori.ootw.common.validation.Enum;


원래는 여기서 Enum 어노테이션 정의된 클래스가 지금 바뀐 PR에는 src/main/java/com/backendoori/ootw/image/validation/Enum.java 여기인 걸로 아는데 경로가 안바뀌어서 그런 것일까요..?

확인이 필요하겠네요

Sehee-Lee-01 · 2024-01-21T08:11:16Z

적어놓긴 했는데 지금 jacoco 통과를 못하고 있고 테스트에 대한 여러분의 생각을 듣고 싶어요
기타 사항에 언급했듯 현재 테스트로직에서 EnumValidator를 거치는 테스트가 존재함에도 통과가 안되고 있고 스케쥴러에 대한 좋은 테스트 방법이 떠오르지 않아 의견을 여쭙고 있습니다 :) 두 부분이 jacoco를 통과하지 못해서 나는 오류에요!

제가 생각해본 테스트 방법에 대해서 아래 의견 남겨둡니다!

스케쥴러
- 중원님께서도 찾아보셨을테지만 스케쥴러 테스트 방법을 찾아보니 되게 방법이 많더라구요,,! 원본코드를 바꿔서 cron식을 프로퍼티로 관리해서 개발 환경이랑 테스트 환경이랑 다르게 한다던가, 테스트용 라이브러리를 추가한다던가,,,
- 그래서 스케줄링이 잘 되는지 자체를 테스트 하고 싶으시다면 위 방법들 중에서 중원님이 구현하기 편한 방법으로 해보셔도 괜찮을 것 같기도합니다!
- 근데 생각해보니 스케쥴링은 작은 설정만 저희가 해주고 프레임워크에서 관리해주는 것이니 따로 스케줄링이 잘 되는지 테스트를 하는 것이 의미 있나 생각도 들더라구요!(제가 안써봐서 그런 것일 수도 있지만...)
- 그래서 우선적으로는 deleteUnused()의 로직 자체만을 테스트하는 방향으로 가도 좋을 것 같다는 생각이 들었습니다!
EnumValidator
- EnumValidator는 혹시 import 경로가 잘못 되었을 수도 있을 것 같은데 확인 한 번만 해주시면 감사하겠습니다!

ASak1104

고생하셨습니다!
존재하지 않는 이미지가 언제 생성될까? 생각해보면 게시글의 업데이트가 가장 보편적일 것 같은데
사실 프로덕션 전이어서 서비스 차원에서 도입이 필요한지 정하기는 애매한 문제인 것 같아요
스케줄링에 대한 주기를 판단하기 어렵기도 하고요!
테스트 통과 과정에서 어려움이 있으니 추후 논의를 빠르게 가져봐야겠네요

ASak1104 · 2024-01-24T02:18:23Z

src/main/java/com/backendoori/ootw/image/service/ImageUpdateService.java

+        List<String> urls = new ArrayList<>();
+
+        postRepository.findAll()
+            .stream()
+            .map(Post::getImageUrl)
+            .forEach(urls::add);
+
+        avatarItemRepository.findAll()
+            .stream()
+            .map(AvatarItem::getImageUrl)
+            .forEach(urls::add);
+
+        List<Image> images = imageRepository.findAll();
+
+        for(Image image : images){
+            if(!urls.contains(image.getImageUrl())){
+                imageService.delete(image.getFileName());
+            }
+        }


배열을 사용하다 보니 이미지 수가 많을 경우에는 시간이 상당히 오래걸릴 수 있겠네요
그리고 47번 행에서의 for문 로직도 stream으로 처리하면 좋을 것 같아요
filter에서 delete를 하면 되니까요

ASak1104 · 2024-01-24T02:18:53Z

src/main/java/com/backendoori/ootw/avatar/dto/AvatarItemRequest.java

@@ -3,7 +3,6 @@
 import com.backendoori.ootw.avatar.domain.ItemType;
 import com.backendoori.ootw.avatar.domain.Sex;
 import com.backendoori.ootw.common.validation.Enum;


확인이 필요하겠네요

ASak1104 · 2024-01-24T02:21:11Z

src/main/java/com/backendoori/ootw/image/service/MiniOImageServiceImpl.java

+    @NotNull
+    private static String getUniqueFileName(MultipartFile file) {
+        String randomUUID = UUID.randomUUID().toString();
+        String originalFileName = file.getOriginalFilename();
+
+        return randomUUID + originalFileName;
+    }


해당 부분의 파라미터로 file 자체가 아닌 file name을 바로 주는 것도 좋을 것 같아요
그리고 uuid랑 original file name 사이에 구분자가 들어가면 좋을 것 같아요
최종적으로 만든 file name은 s3 name convention을 준수하는지 확인해보면 좋을 것 같습니다

ASak1104 · 2024-01-24T02:22:25Z

src/main/java/com/backendoori/ootw/image/validation/Enum.java

+    String message() default "유효하지 않은 값입니다 다시 입력해주세요";
+    Class<?>[] groups() default {};
+    Class<? extends Payload>[] payload() default {};
+    Class<? extends java.lang.Enum<?>> enumClass();


인터페이스에서는 모든 행에 대한 개행이 필요할 것 같아요

ASak1104 · 2024-01-24T02:26:03Z

src/test/java/com/backendoori/ootw/avatar/controller/AvatarItemControllerTest.java

+    static Stream<Arguments> provideInvalidAvatarImageInfo() {
+        String validType = "HAIR";
+        String validSex = "MALE";
+        return java.util.stream.Stream.of(
+            Arguments.of(null, validSex),
+            Arguments.of(validType, null),
+            Arguments.of("", validSex),
+            Arguments.of(validType, ""),
+            Arguments.of(" ", validSex),
+            Arguments.of(validType, " "),
+            Arguments.of("hair", validSex),
+            Arguments.of(validType, "female")
+        );
+    }


여기서 full package 를 명시해주신 이유가 있나요?

ASak1104 · 2024-01-24T02:26:43Z

src/main/java/com/backendoori/ootw/image/domain/Image.java

+    @Column(name = "filename")
+    private String fileName;


fileName도 unique면 좋을 것 같습니다

ASak1104 · 2024-01-24T02:27:16Z

mysql/schema.sql

+    image_url  varchar(500) NOT NULL,
+    filename   varchar(255) NOT NULL,


두 컬럼 모두 unique인 것 같은데 맞을까요?

shoeone96 added 10 commits January 18, 2024 11:18

refactor: image 관련 로직 image directory로 migration

82f51ea

feat: image table 및 entity 생성

0bd8a1f

refactor: Image db 저장 로직 생성과 file 이름 unique 하게 생성하는 로직 추가

88c5d92

feat: ImageFile 정적 static method 추가

e83f62c

feat: image 저장하는 repository 추가

bebc18a

feat: 한 달에 한번 사용안하는 image 삭제 로직 추가

300b564

feat: schema, Entity 맞추기 및 사용안하는 import 문 삭제

ed3bb10

fix: ImageException 을 못잡아내는 문제 해결

de94e13

Merge branch 'feature' into feature-image-enhancement

e16d0a5

test: 아바타 ItemRequest validation test

ff039e8

shoeone96 added the enhancement New feature or request label Jan 19, 2024

shoeone96 self-assigned this Jan 19, 2024

shoeone96 requested review from ASak1104 and Sehee-Lee-01 as code owners January 19, 2024 14:57

shoeone96 had a problem deploying to build January 19, 2024 14:57 — with GitHub Actions Failure

shoeone96 linked an issue Jan 19, 2024 that may be closed by this pull request

🔩 이미지 기능 Refactoring #82

Open

3 tasks

Sehee-Lee-01 reviewed Jan 21, 2024

View reviewed changes

ASak1104 reviewed Jan 24, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

🚀 Feature image enhancement #85

🚀 Feature image enhancement #85

shoeone96 commented Jan 19, 2024

Sehee-Lee-01 commented Jan 21, 2024

shoeone96 commented Jan 21, 2024

Sehee-Lee-01 left a comment •

edited

Loading

Sehee-Lee-01 Jan 21, 2024

Sehee-Lee-01 Jan 21, 2024

Sehee-Lee-01 Jan 21, 2024

ASak1104 Jan 24, 2024

Sehee-Lee-01 commented Jan 21, 2024 •

edited

Loading

ASak1104 left a comment

ASak1104 Jan 24, 2024

ASak1104 Jan 24, 2024

ASak1104 Jan 24, 2024

ASak1104 Jan 24, 2024

ASak1104 Jan 24, 2024

ASak1104 Jan 24, 2024

ASak1104 Jan 24, 2024

		image_url varchar(500) NOT NULL,
		filename varchar(255) NOT NULL,

🚀 Feature image enhancement #85

Are you sure you want to change the base?

🚀 Feature image enhancement #85

Conversation

shoeone96 commented Jan 19, 2024

✅ 요구사항

🚀 주요 변경사항

💡 기타사항

Sehee-Lee-01 commented Jan 21, 2024

shoeone96 commented Jan 21, 2024

Sehee-Lee-01 left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Sehee-Lee-01 commented Jan 21, 2024 • edited Loading

ASak1104 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Sehee-Lee-01 left a comment •

edited

Loading

Sehee-Lee-01 commented Jan 21, 2024 •

edited

Loading