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

refactor: 스타카토 수정 시 삭제된 이미지 수동 삭제 #547

Open
2 tasks done
linirini opened this issue Nov 9, 2024 · 3 comments
Open
2 tasks done
Assignees
Labels
confirm need confirmation! refactor 리팩토링 (변수 및 메서드 네이밍 변경)
Milestone

Comments

@linirini
Copy link
Contributor

linirini commented Nov 9, 2024

🤮 As Is

리팩터링하고자 하는 파트와 이유를 구체적으로 설명해주세요.

이전에 CascadeType을 PERSIST로 변경하면서 자식 삭제를 수동으로 하도록 리팩터링 했습니다. 그 과정에서 스타카토 수정 시 삭제된 이미지를 수동 삭제하는 작업을 놓쳤습니다.
급한대로 CascadeType을 ALL로 수정했습니다. 해당 작업을 다른 삭제 작업처럼 수동 삭제하는 방식으로 리팩터링이 필요하다고 판단했습니다.

CascadeType.PERSIST 를 활용하여 부모 엔티티(Moment)에서 자식 엔티티(MomentImage)의 생성주기를 관리하게끔 구성하고,
update, delete 시에 이미지 삭제는 수동으로 서비스에서 MomentImageRepository에 직접 쿼리를 날리는 형식으로 구성될 것 같습니다.

고민인 부분

CascadeType.PERSIST 를 삭제하여, 생성 또한 서비스 로직내에서 관리하게 된다면 양방향 연관관계를 끊을 수 있겠다는 생각이 들었습니다.

장점

  • 기존에 Image 5장을 저장할 때 디비에 나가는 쿼리가 5개였는데, insert 쿼리 또한 1개의 BulkInsert로 구성될 수 있습니다.
  • 양방향으로 발생할 수 있는 여러 문제들이 자동으로 해소됩니다.

단점

  • 관리포인트가 늘어나므로, 서비스 로직이 복잡해질 수 있습니다
    • -> 해당 부분은 퍼사드 패턴을 도입하는 이유가 될 수도 있겠습니다.
  • 휴먼에러가 일어날 가능성이 높습니다.
    • MomentImage는 최대 5개로 구성되어야 하며, 해당 비즈니스로직을 검증하는 부분을 기존에는 Moment를 생성할 때 이루어졌습니다. 이로 인해 서비스 로직에서 MomentImages 일급컬렉션을 이용해야 합니다.

🤩 To Be

리팩터링 방향을 구체적으로 공유해주세요.

우선은 전에 회의에서 다루었던 내용과 같이 양방향 연관관계를 끊는 형식으로 구성하는 것으로 할 예정입니다.
기존에 Moment가 가지고 있던 MomentImages는 필드상에서 제거되어 Embedded 타입으로 사용되지 않을 것 같습니다.

  • Moment에서 MomentImages 삭제
  • MomentService에서 Moment 생성 시 MomentImages 일급컬렉션을 이용해서 비즈니스로직 검증 후 저장
  • insert가 BulkInsert가 될 수 있게끔 MomentRepository 에서 커스텀 Query 작성 필요
  • update 시 기존에 존재하는 모든 이미지 삭제 후 다시 모든 이미지 생성

😇 이때까지 끝낼게요!

리팩터링 완료 예상 날짜를 작성해주세요. 신중하게 생각해요!

😵 참고 자료(선택)

🙇‍♀️이슈 확인했어요:)

팀원에게 이슈 확인을 부탁해요!

@linirini linirini added the refactor 리팩토링 (변수 및 메서드 네이밍 변경) label Nov 9, 2024
@BurningFalls
Copy link
Contributor

@linirini
확인했습니다. 혹시 급한대로 CascadeType을 ALL로 수정했습니다. 이걸 반영한 시점이 언제인가요?

@linirini
Copy link
Contributor Author

@linirini 확인했습니다. 혹시 급한대로 CascadeType을 ALL로 수정했습니다. 이걸 반영한 시점이 언제인가요?

반영 pr 링크 남깁니다!

@linirini linirini added the confirm need confirmation! label Nov 10, 2024
@Ho-Tea Ho-Tea moved this to In Progress in 2024-staccato Nov 11, 2024
@Ho-Tea Ho-Tea added this to the sprint-7 milestone Nov 11, 2024
@Ho-Tea Ho-Tea added confirm need confirmation! and removed confirm need confirmation! labels Nov 11, 2024
@Ho-Tea
Copy link
Contributor

Ho-Tea commented Nov 11, 2024

@linirini @BurningFalls
Issue 한번 확인해주시면 감사하겠습니다.
새롭게 건의한 부분은 MomentMomentImage 양방향 연관관계를 끊는 것입니다.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirm need confirmation! refactor 리팩토링 (변수 및 메서드 네이밍 변경)
Projects
Status: In Progress
Development

No branches or pull requests

3 participants