Skip to content
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

[BE] feat: 리뷰 삭제 기능 구현 #735

Merged
merged 30 commits into from
Oct 16, 2023
Merged

[BE] feat: 리뷰 삭제 기능 구현 #735

merged 30 commits into from
Oct 16, 2023

Conversation

hanueleee
Copy link
Collaborator

@hanueleee hanueleee commented Oct 9, 2023

Issue

✨ 구현한 기능

  • 리뷰 삭제 api 구현

📢 논의하고 싶은 내용

  1. updateProductImage 메소드 관련 ✅

기존 updateProductImage 메소드 질문 (리뷰 좋아요시 상품 이미지 업데이트)

  • update 로직을 따로 빼서 controller에서 service 로직 두번 호출하는 이유?
  • PageRequest 사용

우선 product를 파라미터로 받는 updateProductImage 메소드 추가해둔 상태
(기존 메소드는 reviewId를 파라미터로 받음)

  1. S3 delete
    @Transactional
    public void deleteReview(final Long reviewId, final Long memberId) {
        final Member member = memberRepository.findById(memberId)
                .orElseThrow(() -> new MemberNotFoundException(MEMBER_NOT_FOUND, memberId));
        final Review review = reviewRepository.findById(reviewId)
                .orElseThrow(() -> new ReviewNotFoundException(REVIEW_NOT_FOUND, reviewId));
        final Product product = review.getProduct();
        final String image = review.getImage();

        if (review.checkAuthor(member)) {
            deleteThingsRelatedToReview(reviewId, review);
            updateProductImage(product);
            imageUploader.delete(image);
        } else {
            throw new NotAuthorOfReviewException(NOT_AUTHOR_OF_REVIEW, memberId);
        }
    }

이런식으로
db에서 review 관련 데이터 지우기,
product의 대표 이미지 갱신,
s3에서 관련 이미지 삭제가
한 트랜잭션에서 이루어지고 있습니다.

s3 관련 로직을 다른 트랜잭션으로 분리해야할까요?

  1. NotAuthorReviewException ✅

리뷰를 삭제하려는 사용자가 리뷰 작성자가 아닐 때 던지는 에러입니다.
info로 memberId만 넘겨도 될까요?

🎸 기타

  • DB의 image컬럼에 값이 어떻게 들어가는지 보고(아마 cloudfront url이 들어가고 있는 것 같음) s3 delete 로직 조금 고칠 예정

⏰ 일정

  • 추정 시간 : 2
  • 걸린 시간 : 3

@github-actions
Copy link

github-actions bot commented Oct 9, 2023

Test Results

249 tests   249 ✔️  21s ⏱️
126 suites      0 💤
126 files        0

Results for commit 05b0030.

♻️ This comment has been updated with latest results.

Copy link
Collaborator

@wugawuga wugawuga left a comment

Choose a reason for hiding this comment

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

연휴동안 고생하셨어요 오잉!
코멘트 확인해주세요

Copy link
Member

@70825 70825 left a comment

Choose a reason for hiding this comment

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

고생하셨어요 오잉 🥳
우가 코멘트랑 겹치지 않는 부분 코멘트 남겨서 확인해주세요~
#728 에서 여기에서 만든 메서드가 필요한데, 오잉의 코드 변경점이 많아서 오잉꺼 먼저 머지하고, 제꺼 PR 머지하는게 좋아보입니다!

@hanueleee
Copy link
Collaborator Author

(기존)
image
review에 연관된 reviewTag조회
reviewTag 삭제 * N
review에 연관된 reviewFavorite조회
reviewFavorite 삭제 * M
review삭제
product의 가장 좋아요가 많은 review 조회
(+변경사항이 있다면 product 업데이트)
= 1 + N + 1 + M + 1 + 1 (+1)

(개선)
image
review에 연관된 reviewTag조회
reviewTag 삭제
review에 연관된 reviewFavorite조회
reviewFavorite 삭제
review삭제
product의 가장 좋아요가 많은 review 조회
(+변경사항이 있다면 product 업데이트)
= 1 + 1 + 1 + 1 + 1 + 1 (+1)

참고 블로그 : https://aljjabaegi.tistory.com/681

Copy link
Collaborator

@wugawuga wugawuga left a comment

Choose a reason for hiding this comment

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

image

잉오가 올려준 블로그를 잘 보고 왔는데용!
사용한 메소드는 복합키가 연관이 되어있는 것으로 이해를 했는데요!
deleteBatch vs jpql 선택한 이유가 궁금합니당

backend/src/main/java/com/funeat/review/domain/Review.java Outdated Show resolved Hide resolved
@hanueleee hanueleee changed the title [BE] 리뷰 삭제 기능 구현 [BE] feat:리뷰 삭제 기능 구현 Oct 12, 2023
@hanueleee hanueleee changed the title [BE] feat:리뷰 삭제 기능 구현 [BE] feat: 리뷰 삭제 기능 구현 Oct 12, 2023
@hanueleee
Copy link
Collaborator Author

사용한 메소드는 복합키가 연관이 되어있는 것으로 이해를 했는데요!
deleteBatch vs jpql 선택한 이유가 궁금합니당

deleteAllByIdInBatch는 복합키일 경우에는 deleteAllInBatch로 동작하고,
단일키일 경우에는 쿼리 한번 호출로 삭제할 수 있습니다.

    delete 
    from
        review_tag 
    where
        id in (
            ? , ? , ?
        )

현재 reviewTag와 reviewFavorite은 다 단일키라 해당 메소드를 써도 괜찮다고 생각했습니다.
(+ deleteAllInBatch로도 해봤는데

delete 
    from
        review_tag 
    where
        id=? 
        or id=? 
        or id=?

이렇게 나가더라구요??)

deleteAllByIdInBatch vs JPQL 중 deleteAllByIdInBatch를 사용한 이유는
이미 존재하는 메소드로 목표를 충족할 수 있다고 생각해서 굳이 새로운 jpql을 만들지 않았습니다!

@hanueleee
Copy link
Collaborator Author

hanueleee commented Oct 13, 2023

s3 이미지 삭제 로직에 스프링 이벤트 적용

  • 리뷰 삭제가 가능할 때 (리뷰 작성자가 맞을 때) ReviewDeleteEvent 발행
  • 리뷰 삭제 로직(reviewTag삭제, reviewFavorite삭제, review삭제)가 정상적으로 커밋되었을 때 이미지 삭제가 실행되어야 하기 때문에 @TransactionalEventListener(phase = TransactionPhase.AFTER_COMMIT)
  • @Async를 사용해 비동기 처리 (+@EnableAsync)
    • Listener 로직이 실행되는 시간이 사용자의 응답을 느리게 만들지 않는다.
    • 별도로 @Async에 대한 설정이 없으면 새로운 비동기 작업을 스레드 풀에서 처리하는 게 아니라 새로운 스레드를 매번 생성해서 작업을 수행시키는 것이 디폴트 설정 -> application.yml에 스레드풀 설정 추가
  • 테스트 관련
    • @RecordApplicationEventsApplicationEvents로 이벤트가 몇번 발행됐는지 셀 수 있다
    • 비동기 테스트
      • 방법1) 비동기 로직이 끝날 때까지 대기
      • 방법2) 테스트 코드에서만 동기로 작동하게
      • => '비동기로 동작'하는 것을 검증하고 싶어서 방법1을 사용했습니다
    • ImageUploader속 로직은 외부api이기 때문에(S3) 모킹처리했습니다

기타

  • 이미지 삭제 실패시 일단 로그만 찍어두었습니다. 추후에 이슈 따로 파서 보완할 예정 (db에 저장해두고 나중에 스케줄러로 삭제?)
  • updateProductImage도 이벤트 처리하는거 어떨까 (productService의 역할이 아닐까)
  • 이벤트 관련 클래스들 위치 어디에 둬야할까?

Copy link
Member

@70825 70825 left a comment

Choose a reason for hiding this comment

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

final 키워드 안붙인게 좀 많아서 여기에만 대표적으로 코멘트 남길게요~

나머지는 코멘트는 빠르게 반영할 수 있을 것 같아서 approve 할게요

고생하셨어요~

Copy link
Collaborator

@wugawuga wugawuga left a comment

Choose a reason for hiding this comment

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

이벤트 도입이 처음이었는데, 너무 잘 구현해주신 것 같아요!!!
고생하셨어요 코멘트 한번 확인 부탁드려요

Copy link
Member

@70825 70825 left a comment

Choose a reason for hiding this comment

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

코멘트 하나 더 남겼어요~
conflict 수정되면 다시 확인해보면 될 것 같아여

backend/src/main/java/com/funeat/common/s3/S3Uploader.java Outdated Show resolved Hide resolved
Copy link
Collaborator

@Go-Jaecheol Go-Jaecheol left a comment

Choose a reason for hiding this comment

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

앞에서 리뷰 잘 해줘서 개행 관련 코멘트 하나 말고는 피드백 할 내용 없네요!
이벤트 적용하느라 고생하셨슴다~~~!

backend/src/test/java/com/funeat/common/EventTest.java Outdated Show resolved Hide resolved
Copy link
Collaborator

@wugawuga wugawuga left a comment

Choose a reason for hiding this comment

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

코멘트가 많았는데 열심히 고민하신 것 같습니다 ㅎㅎ
고생하셨어요

@hanueleee hanueleee merged commit 5d2e717 into develop Oct 16, 2023
3 checks passed
@70825 70825 deleted the feat/issue-734 branch October 18, 2023 11:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BE] 리뷰 삭제 기능 구현
4 participants