Vegan-Life / VeganLife-Backend

채식주의자를 위한 식단 및 영양관리 앱 BE
2 stars 0 forks source link

회원 탈퇴시 연관된 엔티티로 인한 문제 해결 #200

Closed O-Wensu closed 10 months ago

O-Wensu commented 10 months ago

이슈 번호 (#129 )

요약

회원 탈퇴시 연관된 엔티티로 인한 문제 해결 Member를 참조하는 Post, Comment, PostLike, CommentLike의 member 필드를 null로 변경 Member를 참조하는 MealData, MealLog, Notification 데이터 삭제

트러블슈팅

문제 상황

    @Modifying
    @Query("UPDATE Comment c SET c.member = NULL WHERE c.member.id = :memberId")
    void setMemberToNull(Long memberId);

@Modifying 애노테이션은 @Query 애노테이션으로 작성된 INSERT, UPDATE, DELETE와 같은 쿼리에 사용됩니다. (SELECT제외)

테스트 코드

    @Test
    @DisplayName("회원이 작성한 댓글의 Member를 null로 변경")
    void setMemberToNullTest() {
        // given
        Member otherMember = MemberFixture.DEFAULT_F.get();
        memberRepository.save(otherMember);
        Post otherPost = PostFixture.CHALLENGE.get();
        postRepository.save(otherPost);
        Comment otherComment = CommentFixture.DEFAULT.getTopComment(otherMember, otherPost);
        commentRepository.save(otherComment);
        // when
        commentRepository.setMemberToNull(member.getId());
        // then
        assertThat(commentRepository.findById(comment.getId()).get().getMember()).isNull();
        assertThat(commentRepository.findById(otherComment.getId()).get().getMember()).isNotNull();
    }

다음과 같이 테스트 코드를 작성했을 때, assertThat이 실패하는 문제가 있었습니다.

원인

기존 JPA를 통해 조회를 할 때 순서는 다음과 같습니다.

1차 캐시 조회 > 존재하면 반환 > 없으면 DB 접근하여 반환

하지만 @Query로 정의된 JPQL은 JPA처럼 영속성 컨텍스트를 거쳐 쓰기 지연SQL로 동작하는 것이 아닌, 데이터베이스에 바로 질의하게 됩니다.

그렇게 되면, 영속성 컨텍스트의 1차 캐시와 DB의 데이터 싱크가 맞지 않게 됩니다.

테스트 코드에서 findById로 데이터를 가져왔지만 기존 데이터가 사용되는 이유는, 이미 같은 Id로 영속성 컨텍스트에 데이터가 존재하기 때문에 DB에 접근하지 않고, 1차 캐시를 통해 가져오기 때문에 기존 데이터가 사용되는 것입니다.

해결

@Modifying의 속성 중 clearAutomatically 라는 속성이 있습니다. 이 속성을 true로 변경하면 문제를 해결할 수 있습니다.

public interface CommentRepository extends JpaRepository<Comment, Long> {
    Optional<Comment> findById(Long commentId);

    @Modifying(clearAutomatically = true)
    @Query("UPDATE Comment c SET c.member = NULL WHERE c.member.id = :memberId")
    void setMemberToNull(Long memberId);
}

clearAutomatically@Query로 정의된 JPQL을 실행한 후에 자동으로 영속성 컨텍스트를 비워줍니다.

그래서 findById를 수행하게 되면 1차 캐시에 데이터가 존재하지 않기 때문에 DB 조회 쿼리를 수행하게 됩니다. 그리고 이는 다시 영속성 컨텍스트로 관리되어 최신 상태를 유지할 수 있습니다.

soun997 commented 10 months ago

많은 삭제 연산을 구현하느라 고생하셨습니다!

항상 변경 감지만 사용했었는데, 일괄 수정에 대하여 알아볼 수 있는 좋은 기회였습니다😊

몇 가지 피드백을 드리자면,

1. clearAutomatically 설정이 꼭 필요한가?

코드를 살펴보니, Member와 연관된 Comment를 일괄수정하는 연산은 하나의 트랜잭션 안에서 진행되더군요! 일괄 수정하기 전의 영속성 컨텍스트에는 엔티티가 없기 때문에 통합테스트 시에는 기존의 코드로도 문제가 발생하지 않을 것이라고 생각합니다.

테스트 로직은 이미 영속성 컨텍스트 내에 엔티티가 존재한 상태로 일괄 수정하기 때문에 문제가 발생한 것으로 보입니다. 그렇다는 것은 테스트를 위해 clearAutomatically를 추가했다는 것이 아닐까요?

테스트 코드는 다음과 같이 직접 em.clear()를 호출해주어 영속성 컨텍스트를 초기화하면 정상적으로 작동할 것 같습니다.

    // CommentRepositoryTest.java
    @BeforeEach
    void setup() {
        memberRepository.save(member);
        postRepository.save(post);
        commentRepository.save(comment);
        em.clear();
    }

물론 현재의 로직 또한 문제가 발생하는 것은 아니기 때문에 그대로 진행하셔도 좋을 것 같습니다!

2. deleteAll은 일괄 삭제가 아니다.

MealLogMealData 삭제에 대해서는 JPA가 제공하는 deleteAll 메서드를 사용하신 것 같습니다. 다만 JPA의 deleteAll은 내부적으로 for문을 돌면서 delete를 수행하는 것이기 때문에 일괄 삭제가 되지 않습니다.

이쪽 도메인은 제 담당이기 때문에 추후 쿼리 최적화 단계에서 해결해보도록 하겠습니다!

O-Wensu commented 10 months ago
  1. 현재는 테스트 코드 한정이지만, @Modifying을 사용하면 결국 1차 캐시와의 데이터 불일치가 발생합니다! 해당 옵션을 사용하지 않으면 다른 로직에서 수정한 데이터를 가져올 때 수정되지 않은 기존 데이터를 가져오게 되기 때문에 옵션은 필요하다고 생각합니다. 😆

  2. 아하 그렇군요! 그렇다면 deleteAllInBatch 사용해서 해당 메서드들은 수정해두겠습니다! 해당 부분은 나중에 쿼리문으로 성능 개선을 할 수 있을 것 같습니다!

soun997 commented 10 months ago

일괄 수정은 최대한 1차 영속성 캐시에 엔티티가 없는 상태에서 진행하는 것을 권장한다고 합니다! clearAutomatically를 사용하면 결과적으로는 데이터 불일치를 해결할 수 있다는 것에는 동의합니다. 다만 일괄 수정이 다른 jpa 로직과 동반되면 추적하기 어려운 문제가 발생할 가능성이 있기 때문에, 정말 필요할 때가 아니면 사용을 지양하는 것이 좋을 것 같다는 저의 생각이였습니다!!!

O-Wensu commented 10 months ago

현재 코드에서는 dirty checking 로직이 업데이트 되지 않을 수 있는 문제도 발생할 수 있을 것 같습니다.

이 문제는 @Modifying의 flushAutomatically 속성을 true로 변경 (default false) 하여 해결할 수 있습니다. 쿼리를 수행하기 전에, 영속성 컨텍스트 변경사항을 flush 하게 됩니다!

soun997 commented 10 months ago

그렇군요! 일단 저는 적용 과정에 조금 더 의의를 둔 것이기 때문에 저희 사이에 의견 불일치가 있었던 것 같네요. flushAutomatically 속성과 함께 사용한다면 제가 우려하는 문제는 발생하지 않을 것 같습니다! 좋습니다😊

서치하다가 발견한건데 중요한 건 아니구 과거 유물같은 것,,, 원래 flushAutomatically는없는 옵션이었는데 한 사용자의 이슈로 추가가 된 듯합니다! 그만큼 충분히 필요한 옵션이라고 생각되네요!! https://github.com/spring-projects/spring-data-jpa/issues/1167