스프링 이벤트를 사용한 S3 이미지 파일 삭제 기능 추가 #714

jjongwa · 2023-10-16T07:31:56Z

📄 Summary

테스트를 하기가 애매하더라구요.. 그래서 젠킨스 브랜치 바꿔서 삭제되는지 시도해 봤습니다.

있었는데,

없어졌습니다.

🙋🏻 More

close #323 #646

github-actions · 2023-10-16T07:34:30Z

📝 Jacoco Test Coverage

Total Project Coverage	79.66%	❌

File	Coverage [88.32%]
S3ImageEvent.java	100%	🍏
TripUpdateRequest.java	100%	🍏
DeleteEventListener.java	100%	🍏
Trip.java	98.44%	🍏
TripService.java	97.49%	🍏
ItemService.java	91.86%	🍏
MemberService.java	59.66%	❌
ImageService.java	53.49%	❌
S3ImageEventListener.java	47.37%	❌
ImageUploader.java	9.68%	❌

waterricecake

고생하셨습니다 디노!

waterricecake · 2023-10-17T02:20:38Z

backend/src/main/java/hanglog/listener/S3ImageEventListener.java

+    private String folder;
+
+    @Async
+    @Transactional(propagation = Propagation.REQUIRES_NEW)


static import하는건 어떤가요?

waterricecake · 2023-10-17T02:28:18Z

backend/src/main/java/hanglog/image/infrastructure/ImageUploader.java

+    @Value("${cloud.aws.s3.bucket}")
+    private String bucket;
+
+    @Value("${cloud.aws.s3.folder}")
+    private String folder;


@Value때문에 분리해준 건가요? 분리한 이유는 무엇인가요? 어차피 서비스에서 밖에 사용 안되던데...

레이어드 아키텍처 관점에서 분리했습니다.!

LJW25

코멘트 남겼는데 별건 없어서 Aㅏ프루-브

LJW25 · 2023-10-17T04:34:01Z

backend/src/main/java/hanglog/member/service/MemberService.java

            throw new BadRequestException(DUPLICATED_MEMBER_NICKNAME);
        }
    }
+
+    private void deleteBeforeImage(final String beforeUrl, final String updatedUrl) {


before은 뭔가 안와닿아서 old는 어떤가 하는 작은 건의? 무시하셔도 됩니다.

old도 좋습니다~

LJW25 · 2023-10-17T04:36:42Z

backend/src/main/java/hanglog/member/service/MemberService.java

+            if (targetUrl.getHost().equals(KAKAO_HOST) || targetUrl.getHost().equals(GOOGLE_HOST)) {
+                return;
+            }
+        } catch (MalformedURLException e) {


LJW25 · 2023-10-17T04:38:10Z

backend/src/main/java/hanglog/trip/service/TripService.java

+        if (beforeImageName.equals(updateImageName)) {
+            return;
+        }
+        publisher.publishEvent((new S3ImageEvent(beforeImageName)));


publishEvent는 원래 (()) 괄호가 두겹인가요? 신기하네

기현상입니다

mcodnjs

수고하셨습니다 ~~
코멘트 확인 부탁드립니다 ~ ~

mcodnjs · 2023-10-17T03:36:15Z

backend/src/main/java/hanglog/member/service/MemberService.java

            throw new BadRequestException(DUPLICATED_MEMBER_NICKNAME);
        }
    }
+
+    private void deleteBeforeImage(final String beforeUrl, final String updatedUrl) {


deletePreviousImage, deleteOldImage 제안드립니다 ..

댓츠 그뤠잇

mcodnjs · 2023-10-17T03:36:52Z

backend/src/main/java/hanglog/member/service/MemberService.java

+                return;
+            }
+        } catch (MalformedURLException e) {
+            throw new BadRequestException(FAIL_IMAGE_NAME_HASH);


FAIL_IMAGE_NAME_HASH 이 예외 맞나요?

새로운 에러코드까지 만들기는 애매하다고 생각해서 기존에 존재하는 코드 중에 가장 알맞아 보이는 친구로 골라봤는데..
새로 만드는게 나을까요..?

해싱하는데 발생하는 예외가 아니라고 생각해서 ㅎㅎ ..
전 예외 하나 더 만드는거 좋습니다

어어 INVALID_IMAGE_URL(5005, "요청한 이미지 URL의 형식이 잘못되었습니다.")
이걸 가져온다는걸 잘못 넣었군요
이 친구는 어떤가요?

참고로 해당 예외는
http://
https://
file://

이 세 가지 이외로 시작되는 문자열일 때 발생하는 에러입니다.

mcodnjs · 2023-10-17T03:42:44Z

backend/src/main/java/hanglog/member/service/MemberService.java

+            if (targetUrl.getHost().equals(KAKAO_HOST) || targetUrl.getHost().equals(GOOGLE_HOST)) {
+                return;
+            }


행록 S3 URL로 검증하는 방법은 어떤가요?

행록 URL이 환경마다 달라져서 총 3개인데 이걸로 검증하는게 나을까요?

hgo641

디노고생고생하셨습니다!

hgo641 · 2023-10-17T04:53:58Z

backend/src/main/java/hanglog/trip/service/TripService.java

@@ -144,6 +146,13 @@ private void updateDayLog(final TripUpdateRequest updateRequest, final Trip trip
        }
    }

+    private void updateImage(final String beforeImageName, final String updateImageName) {


beforeImageName.........이름 좋군요
그런데 Item에서는 originalPlace같은 네이밍을 사용하고 있어서 둘 중 하나로 통일하면 좋을 것 같아요

오리진 의견도 나와버렸군요

hgo641 · 2023-10-17T07:47:52Z

backend/src/main/java/hanglog/member/service/MemberService.java

        final Member updateMember = new Member(
                memberId,
                member.getSocialLoginId(),
                myPageRequest.getNickname(),
                myPageRequest.getImageUrl()
        );
+        deletePreviousImage(member.getImageUrl(), updateMember.getImageUrl());


얘는 previous네요?
previous vs before vs original
하나 통일할까요?

인정해버렸습니다

오리진으로 통일하겠습니다.

hgo641 · 2023-10-17T07:51:32Z

backend/src/main/java/hanglog/member/service/MemberService.java

@@ -17,7 +22,10 @@
 @Transactional
 public class MemberService {

+    private static final String HANG_LOG_HOST = "image.hanglog.com";


IMAGE_URL_HOST 또는 PROFILE_URL_HOST 어떤가요? (PROFILE이 명확해ㅓ 좋긴한데, Member도메인에서 imageUrl이라는 변수명을 사용하고 있어서 통일하려면 IMAGE_URL_HOST가 좋겠네요 ㅠㅠ)
변수명만 봤을때는 이게 멤버 프로필 이미지의 url host name이라는 것을 모를 것 같습니다.

날카로운 지적입니다 홍고라니
변경하였습니다.

hgo641 · 2023-10-17T07:59:32Z

backend/src/main/java/hanglog/listener/S3ImageEventListener.java

+        final String imageName = event.getImageName();
+        s3Client.deleteObject(bucket, folder + imageName);


여기는 예외처리 필요없을까요?
s3에서 delete중 예외가 발생할 경우

여기서 catch를 받아 customException을 던져 줄까요.?

* refactor: 이미지 업로드를 담당하는 클래스로 분리 * refactor: 사용하지 않는 & 중복된 Repository 삭제 * feat: S3ImageEvent와 listener 구현 * feat: 이미지 네임 삭제 완료시 S3에서 해당 파일 삭제하는 기능 추가 * refactor: trip 대표이미지 url -> name 변경 * feat: 여행 대표이미지 변경 시 이전에 존재하던 이미지 파일 S3에서 삭제하는 기능 추가 * refactor: 사용하지 않는 상수 제거 * feat: 마이페이지 프로필 사진 변경 시 S3에 존재하는 기존 파일 삭제 기능 추가 * refactor: static import 적용 * refactor: 변수 네이밍 변경 및 컨벤션 적용 * refactor: 삭제 여부 조건 및 예외 코드 변경 * feat: 아이템 삭제 시 이미지 파일 삭제 기능 추가 * refactor: 네이밍 변경

jjongwa added 7 commits October 16, 2023 16:20

refactor: 이미지 업로드를 담당하는 클래스로 분리

9181f38

refactor: 사용하지 않는 & 중복된 Repository 삭제

fc40a6a

feat: S3ImageEvent와 listener 구현

575dd23

feat: 이미지 네임 삭제 완료시 S3에서 해당 파일 삭제하는 기능 추가

eda8458

refactor: trip 대표이미지 url -> name 변경

40b5682

feat: 여행 대표이미지 변경 시 이전에 존재하던 이미지 파일 S3에서 삭제하는 기능 추가

0319c60

refactor: 사용하지 않는 상수 제거

b81c316

jjongwa added ✨ Feature BE labels Oct 16, 2023

jjongwa requested review from LJW25, mcodnjs, hgo641 and waterricecake October 16, 2023 07:31

jjongwa self-assigned this Oct 16, 2023

feat: 마이페이지 프로필 사진 변경 시 S3에 존재하는 기존 파일 삭제 기능 추가

3db499b

waterricecake reviewed Oct 17, 2023

View reviewed changes

refactor: static import 적용

aa4b8cf

jjongwa linked an issue Oct 17, 2023 that may be closed by this pull request

스프링 이벤트를 사용한 S3 이미지 파일 삭제 #646

Closed

3 tasks

waterricecake approved these changes Oct 17, 2023

View reviewed changes

LJW25 approved these changes Oct 17, 2023

View reviewed changes

refactor: 변수 네이밍 변경 및 컨벤션 적용

003eeff

mcodnjs reviewed Oct 17, 2023

View reviewed changes

jjongwa added 2 commits October 17, 2023 15:26

refactor: 삭제 여부 조건 및 예외 코드 변경

ff85e4d

feat: 아이템 삭제 시 이미지 파일 삭제 기능 추가

09713b7

hgo641 requested changes Oct 17, 2023

View reviewed changes

refactor: 네이밍 변경

efa5c8d

hgo641 approved these changes Oct 17, 2023

View reviewed changes

jjongwa merged commit 1a9ebe0 into develop Oct 17, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

스프링 이벤트를 사용한 S3 이미지 파일 삭제 기능 추가 #714

스프링 이벤트를 사용한 S3 이미지 파일 삭제 기능 추가 #714

jjongwa commented Oct 16, 2023

github-actions bot commented Oct 16, 2023 •

edited

Loading

waterricecake left a comment

waterricecake Oct 17, 2023

jjongwa Oct 17, 2023

waterricecake Oct 17, 2023

jjongwa Oct 17, 2023

LJW25 left a comment

LJW25 Oct 17, 2023

jjongwa Oct 17, 2023

LJW25 Oct 17, 2023

jjongwa Oct 17, 2023

LJW25 Oct 17, 2023

jjongwa Oct 17, 2023

mcodnjs left a comment

mcodnjs Oct 17, 2023

jjongwa Oct 17, 2023

mcodnjs Oct 17, 2023

jjongwa Oct 17, 2023

mcodnjs Oct 17, 2023

jjongwa Oct 17, 2023

mcodnjs Oct 17, 2023

jjongwa Oct 17, 2023

hgo641 left a comment

hgo641 Oct 17, 2023

jjongwa Oct 17, 2023

hgo641 Oct 17, 2023

jjongwa Oct 17, 2023

hgo641 Oct 17, 2023

jjongwa Oct 17, 2023

hgo641 Oct 17, 2023

jjongwa Oct 17, 2023

		final String imageName = event.getImageName();
		s3Client.deleteObject(bucket, folder + imageName);

스프링 이벤트를 사용한 S3 이미지 파일 삭제 기능 추가 #714

스프링 이벤트를 사용한 S3 이미지 파일 삭제 기능 추가 #714

Conversation

jjongwa commented Oct 16, 2023

📄 Summary

🙋🏻 More

github-actions bot commented Oct 16, 2023 • edited Loading

📝 Jacoco Test Coverage

waterricecake 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

LJW25 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

mcodnjs 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

Choose a reason for hiding this comment

hgo641 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

Choose a reason for hiding this comment

github-actions bot commented Oct 16, 2023 •

edited

Loading