유저 탈퇴시 정상적으로 데이터 삭제되지 않는 버그 개선 / 구조 개선 / 테스트 코드 추가 / 일부 authentication 처리 로직 개선 #109

akalswl14 · 2023-02-22T08:51:18Z

구현 내용

API 수정

[DELETE] /user 유저 탈퇴 API : 버그 수정
- URI의 오타 수정 ( /user/ -> /user )
- jwtService를 UserService.delete 메소드의 파라미터로 받지 않도록 개선
- User 엔티티는 다른 엔티티와의 개인 소유 관계가 아니기 때문에, cascade 삭제가 부적합한 것으로 보여, 수동적으로 삭제하도록 개선
- 해당 유저의 카테고리 데이터를 먼저 삭제하고, 카테고리 삭제시 cascade 삭제로 전시, 작품, 태그가 삭제되도록 함
- 카테고리 데이터가 삭제된 이후, 유저 데이터를 삭제함
유저 API의 런칭 목적 authentication 처리 로직 교체
- 기존에는 개발상 편의를 위해 단순히 1L으로 처리하였음
- 런칭을 위해, 테스트 계정이 아닌 요청은 정상적으로 처리하도록 메소드 적용하여 처리

테스트

통합 테스트 base class 추가
테스트 상 편의를 위해, User 엔티티의 id를 입력하는 생성자 추가
테스트 상 편의를 위해, 테스트용 유저 정적 변수 선언
Category Repository Unit Test 추가
유저 기반 카테고리를 삭제하는 CategoryRepository.deleteAllByUser메소드를 검증하는 레포지토리 테스트 추가
카테고리를 삭제하면 전시데이터도 삭제되는지에 대해, CategoryService.delete 메소드를 검증하는 테스트 추가
기존의 User Service Test를 단위 테스트로 처리하도록 변경
UserService.updateUserName 메소드의 서비스 단위 테스트 추가
유저 삭제 API의 통합 테스트 추가
UserRepository의 단위 테스트 추가
UserRepository.delete를 검증하는 레포지토리 단위 테스트 추가

기타

일부 예외의 메세지 오타 수정

구현 내용

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)

akalswl14 · 2023-02-22T08:52:51Z

src/main/java/com/yapp/artie/domain/archive/domain/category/Category.java

@@ -35,7 +35,7 @@ public class Category extends BaseEntity {
  @Column(nullable = false)
  private String name;

-  @OneToMany(mappedBy = "category", cascade = CascadeType.ALL, orphanRemoval = true)
+  @OneToMany(mappedBy = "category", cascade = CascadeType.REMOVE)


CascadeType을 Remove로 통일하였습니다.
현재는 CascadeType.Persist 등을 이용한 코드가 없고, 삭제를 목적으로 cascadetype을 사용했으므로, Remove로 처리하는 것이 좋다고 판단하였습니다.

적절한 판단이라고 생각해요👍👍 처음에 ALL로 둔 이유는 상위 객체의 메서드를 이용해서 하위 객체를 저장할 수도 있겠다 싶겠다는 맥락이 있었는데요.. 지금 저희 코드를 보면 Remove만 주는 것이 좋아보이네요!

akalswl14 · 2023-02-22T08:52:58Z

src/main/java/com/yapp/artie/domain/archive/domain/exhibit/Exhibit.java

@@ -44,7 +44,7 @@ public class Exhibit extends BaseEntity {
  @JoinColumn(name = "category_id", nullable = false)
  private Category category;

-  @OneToMany(mappedBy = "exhibit", cascade = CascadeType.ALL, orphanRemoval = true)
+  @OneToMany(mappedBy = "exhibit", cascade = CascadeType.REMOVE)


CascadeType을 Remove로 통일하였습니다.
현재는 CascadeType.Persist 등을 이용한 코드가 없고, 삭제를 목적으로 cascadetype을 사용했으므로, Remove로 처리하는 것이 좋다고 판단하였습니다.

akalswl14 · 2023-02-22T08:53:47Z

src/main/java/com/yapp/artie/domain/archive/repository/CategoryRepository.java

@@ -27,5 +27,8 @@ public interface CategoryRepository extends JpaRepository<Category, Long> {
  int countCategoriesByUser(@Param("user") User user);

  List<Category> findCategoriesByUserOrderBySequence(User user);
+
+  @Modifying(clearAutomatically = true)
+  void deleteAllByUser(User user);


특정 유저에 대한 카테고리 데이터를 모두 삭제하는 레포지토리 메소드 입니다. cascade 설정이 되어있어, 해당 메소드가 처리될때, 해당하는 전시, 작품, 태그도 삭제됩니다.

akalswl14 · 2023-02-22T08:54:37Z

src/main/java/com/yapp/artie/domain/user/controller/UserController.java

-    // Long userId = Long.parseLong(authentication.getName());
-    Long userId = 1L;
+
+    Long userId = getUserId(authentication);
    return ResponseEntity.ok().body(userThumbnailService.getUserThumbnail(userId));
  }



기존에는 개발상 편의를 위해 단순히 1L으로 처리하였으나, 런칭을 위해, 테스트 계정이 아닌 요청은 정상적으로 처리하도록 메소드 적용하였습니다.

akalswl14 · 2023-02-22T08:55:17Z

src/main/java/com/yapp/artie/domain/user/domain/User.java

+    user.name = name;
+    user.profileImage = profileImage;
+    return user;
+  }


테스트상 편의를 위해 id를 받아 User를 생성할 수 있도록 생성자를 추가하였습니다. 빌더패턴을 써도 좋지 않을까 하는 생각이 들긴했으나, 시간상 생략하였습니다.

akalswl14 · 2023-02-22T09:02:26Z

src/test/java/com/yapp/artie/domain/user/domain/UserTest.java

+  public static final User TEST_USER = User.create("test-uid", "test-name", "test-profile");
+  public static final User TEST_SAVED_USER = User.create(1L, "test-uid", "test-name",
+      "test-profile");
+}


테스트상 편의를 위해 유저를 static으로 선언하였습니다. 1L의 id를 갖는 다는 것을 보장하게 하고 싶었습니다.

akalswl14 · 2023-02-22T09:02:41Z

src/test/java/com/yapp/artie/domain/user/repository/UserRepositoryTest.java

+
+@DataJpaTest
+@AutoConfigureTestDatabase(replace = Replace.NONE)
+public class UserRepositoryTest {


User Repository Unit Test를 추가하였습니다.

akalswl14 · 2023-02-22T09:03:01Z

src/test/java/com/yapp/artie/domain/user/service/UserServiceTest.java

 import org.springframework.transaction.annotation.Transactional;

 @Transactional
-@SpringBootTest
+@ExtendWith(MockitoExtension.class)
 class UserServiceTest {


기존의 테스트를 유닛테스트로 변경하였습니다.

akalswl14 · 2023-02-22T09:03:25Z

src/test/java/com/yapp/artie/domain/user/service/UserServiceTest.java

-
-    assertThat(em.find(User.class, user.getId()).getName()).isEqualTo("sample-name-2");
+    assertThatThrownBy(() -> userService.updateUserName(user.getId(), "new-name")).isInstanceOf(
+        UserNotFoundException.class);
  }


일단은 시간상 유저 닉네임 변경 서비스만 유닛테스트로 추가하였습니다.

akalswl14 · 2023-02-22T09:04:05Z

src/test/java/com/yapp/artie/domain/user/service/UserServiceTest.java

-
-    assertThat(captor.getValue()).isEqualTo(expectedUid);
-  }
+  // TODO: delete 서비스 메소드가, jwtService.withDraw를, FirebaseAuth.deleteUser를 잘 호출하는지 확인하는 테스트 추가 필요


mockito에 spy를 쓰면 정상적으로 호출하였는지, 적용할 수 있을 텐데 런칭 코앞이라 못했습니다 흑흑

akalswl14 · 2023-02-22T09:05:43Z

@le2sky 코드 리뷰리뷰 요청요청 이요~! 🤩🤩🤩

le2sky

확인했습니다! 저는 처음에 컨트롤러 테스트를 servie에 대한 통합 테스트로 대체해도 괜찮지 않을까? 생각했었는데 민지님 코드 보고 생각이 많이 달라졌어요!

controller => 통합 테스트
service => 단위 테스트

지금 서비스 테스트 코드 중에서 단위 테스트로 분리할 수 있는 경우가 있거든요? 예를 들어 , "~~~하면 예외를 발생.." 처럼 예외 발생 검증 코드는 단위 테스트로 분리가 가능할 것 같아요! 다른 예로는 기존 delete 테스트는 controller 통합 테스트 + service#delete의 단위 테스트로 분리해서 생각할 수 있겠네요! 덕분에 시야가 넓어졌어요 🙇‍♂️🙇‍♂️

le2sky · 2023-02-22T14:05:42Z

src/main/java/com/yapp/artie/domain/archive/domain/category/Category.java

@@ -35,7 +35,7 @@ public class Category extends BaseEntity {
  @Column(nullable = false)
  private String name;

-  @OneToMany(mappedBy = "category", cascade = CascadeType.ALL, orphanRemoval = true)
+  @OneToMany(mappedBy = "category", cascade = CascadeType.REMOVE)


적절한 판단이라고 생각해요👍👍 처음에 ALL로 둔 이유는 상위 객체의 메서드를 이용해서 하위 객체를 저장할 수도 있겠다 싶겠다는 맥락이 있었는데요.. 지금 저희 코드를 보면 Remove만 주는 것이 좋아보이네요!

le2sky · 2023-02-22T14:11:03Z

src/main/java/com/yapp/artie/domain/user/service/UserService.java

@@ -42,10 +45,12 @@ public CreateUserResponseDto register(String uid, String username, String pictur
  }

  @Transactional
-  public void delete(Long id, JwtService jwtService) {
+  public void delete(Long id) {


controller에서 이미 jwtService를 주입받고 있어서 service를 전달해줬던 맥락이 있었습니다! 저도 이부분이 찜찜하긴 했는데요... controller의 jwtService에 대한 코드를 userService 내부로 이동시키면 좀 더 좋을 것 같다는 생각이 들었어요!

le2sky · 2023-02-22T14:24:32Z

src/main/java/com/yapp/artie/domain/user/service/UserService.java

-    userRepository.deleteById(id);
+
+    categoryRepository.deleteAllByUser(user);
+    userRepository.delete(user);


이렇게 구현하셨군요 고생하셨습니다! 😀😀 참고로 처음에 회원 탈퇴할 때, 연관 데이터 삭제를 구현하지 않았던 맥락을 설명드리자면요... (일단 user에서 casecade로 지우는게 어려운지 몰랐었음..!)

아래 두가지 방법에 대해서 :

user에서 영속성 전이(첫 시도, 복잡하고 잘안됨)

user에서 다른 패키지에 삭제 요청(현재)

어떤 문제가 있냐면, 패키지간 의존성 사이클이 발생해요! categoryService -> userService -> categoryService.. 비슷한 맥락으로 userThumnailService도 현재 패키지간의 의존성 사이클이 발생합니다!

지금 당장으로는 개선할 방법이 없어보이지만, 저번에 공유드렸던 우아한객체지향 세미나에서 이러한 문제를 이벤트를 사용해서 처리했던 걸 봤어요!

le2sky · 2023-02-22T14:24:50Z

src/main/java/com/yapp/artie/global/exception/response/ErrorCode.java

@@ -22,7 +22,7 @@ public enum ErrorCode {
  FIREBASE_INVALID_TOKEN(401, "F004", "JWT 토큰 파싱에 실패했습니다."),
  FIREBASE_REVOKED_TOKEN(401, "F005", "취소된 토큰입니다."),
  FIREBASE_NOT_FOUND_USER(404, "F006", "존재하지 않는 파이어베이스 사용자입니다."),
-  FIREBASE_INVALID_UID(401, "F007", "유요하지 않은 파이어베이스 식별자입니다."),
+  FIREBASE_INVALID_UID(401, "F007", "유효하지 않은 파이어베이스 식별자입니다."),


창피하네요.. 바꿔주셔서 감사합니다.. ㅎㅎㅎ

le2sky · 2023-02-22T14:26:23Z

src/test/java/com/yapp/artie/BaseIntegrationTest.java

+  protected MockMvc mvc;
+  @Autowired
+  protected ObjectMapper objectMapper;
+}


Base Class 만드신거 너무 좋은거 같아요! 제 생각에는 BaseIntegrationTest는 테스트 대상이 아니기 때문에 abstract가 맞을거 같아요!

le2sky and others added 22 commits February 19, 2023 14:12

feat : user - category 영속성 전이 설정

700ced5

test : 사용자 삭제 시 데이터 삭제 검증 테스트

b28fdb1

feat : jwtService를 파라미터로 받지 않도록 개선

3614c2b

feat : User 수동 cascade 삭제

fc75b0c

test : 통합 테스트 base class 추가

b3cdcbd

feat : id와 함께 생성되는 User 생성자 추가

fda6491

feat : User 자동 cascade 삭제를 적용하지 않음에 따라, cascade 관련 필드 삭제

decc073

feat : �삭제를 위한 CascadeType 설정 개선

c29f291

feat : jwtService를 파라미터로 받지 않도록 개선

ce2f1dd

bug : URI 오타 개선

7c5e216

feat : Category Repository Unit Test 추가

17a625e

feat : Category Service Unit Test 추가

50d2f90

feat : 런칭 목적 authentication 처리 로직 교체

07f413a

test : 테스트용 유저 정적 변수 선언

bc19e64

feat : User Service Unit Test로 변경

17295b7

feat : 수동 cascade 삭제를 위한, 유저 기반 카테고리 데이터 삭제 메소드 추가

051c1b6

feat : 유저 삭제 API 통합 테스트 추가

7baf6e3

feat : User Repository Unit Test 추가

f47bd8b

feat : user repository delete 메소드 단위 테스트 추가

3198f7d

refactor : 오타 수정

65d1d40

feat : Artwork 또한 CascadeType REMOVE로 통일

4053371

feat : 런칭 목적 authentication 처리 로직 교체

ed62d95

akalswl14 requested a review from le2sky February 22, 2023 08:51

akalswl14 self-assigned this Feb 22, 2023

akalswl14 commented Feb 22, 2023

View reviewed changes

akalswl14 added 버그 버그 수정 기능 구현 기능 구현 테스트 테스트 관련 기능 수정 허걱 labels Feb 22, 2023

akalswl14 merged commit 544dfd9 into develop Feb 22, 2023

le2sky reviewed Feb 22, 2023

View reviewed changes

akalswl14 deleted the feature/109_v2 branch April 27, 2023 05:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

유저 탈퇴시 정상적으로 데이터 삭제되지 않는 버그 개선 / 구조 개선 / 테스트 코드 추가 / 일부 authentication 처리 로직 개선 #109

유저 탈퇴시 정상적으로 데이터 삭제되지 않는 버그 개선 / 구조 개선 / 테스트 코드 추가 / 일부 authentication 처리 로직 개선 #109

akalswl14 commented Feb 22, 2023 •

edited

Loading

akalswl14 Feb 22, 2023

le2sky Feb 22, 2023

akalswl14 Feb 22, 2023

akalswl14 Feb 22, 2023

akalswl14 Feb 22, 2023

akalswl14 Feb 22, 2023

akalswl14 Feb 22, 2023

akalswl14 Feb 22, 2023

akalswl14 Feb 22, 2023

akalswl14 Feb 22, 2023

akalswl14 Feb 22, 2023

akalswl14 commented Feb 22, 2023

le2sky left a comment

le2sky Feb 22, 2023

le2sky Feb 22, 2023

le2sky Feb 22, 2023

le2sky Feb 22, 2023

le2sky Feb 22, 2023

유저 탈퇴시 정상적으로 데이터 삭제되지 않는 버그 개선 / 구조 개선 / 테스트 코드 추가 / 일부 authentication 처리 로직 개선 #109

유저 탈퇴시 정상적으로 데이터 삭제되지 않는 버그 개선 / 구조 개선 / 테스트 코드 추가 / 일부 authentication 처리 로직 개선 #109

Conversation

akalswl14 commented Feb 22, 2023 • edited Loading

구현 내용

API 수정

테스트

기타

관련 이슈

구현 내용

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

akalswl14 commented Feb 22, 2023

le2sky 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

akalswl14 commented Feb 22, 2023 •

edited

Loading