-
Notifications
You must be signed in to change notification settings - Fork 0
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
유저 탈퇴시 정상적으로 데이터 삭제되지 않는 버그 개선 / 구조 개선 / 테스트 코드 추가 / 일부 authentication 처리 로직 개선 #109
Changes from all commits
700ced5
b28fdb1
3614c2b
fc75b0c
b3cdcbd
fda6491
decc073
c29f291
ce2f1dd
7c5e216
17a625e
50d2f90
07f413a
bc19e64
17295b7
051c1b6
7baf6e3
f47bd8b
3198f7d
65d1d40
4053371
ed62d95
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. CascadeType을 Remove로 통일하였습니다. |
||
List<Artwork> artworks = new ArrayList<>(); | ||
|
||
@Embedded | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 특정 유저에 대한 카테고리 데이터를 모두 삭제하는 레포지토리 메소드 입니다. cascade 설정이 되어있어, 해당 메소드가 처리될때, 해당하는 전시, 작품, 태그도 삭제됩니다. |
||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -71,8 +71,8 @@ public ResponseEntity<CreateUserResponseDto> register( | |
}) | ||
@GetMapping("/me") | ||
public ResponseEntity<User> me(Authentication authentication) { | ||
// Long userId = Long.parseLong(authentication.getName()); | ||
Long userId = 1L; | ||
|
||
Long userId = Long.parseLong(authentication.getName()); | ||
User user = userService.findById(userId); | ||
|
||
return ResponseEntity.ok().body(user); | ||
|
@@ -85,11 +85,11 @@ public ResponseEntity<User> me(Authentication authentication) { | |
description = "유저가 성공적으로 삭제됨", | ||
content = @Content(mediaType = "application/json", schema = @Schema(implementation = ResponseEntity.class))), | ||
}) | ||
@DeleteMapping("/") | ||
@DeleteMapping() | ||
public ResponseEntity<? extends HttpEntity> deleteUser(Authentication authentication) { | ||
// Long userId = Long.parseLong(authentication.getName()); | ||
Long userId = 1L; | ||
userService.delete(userId, jwtService); | ||
|
||
Long userId = getUserId(authentication); | ||
userService.delete(userId); | ||
return ResponseEntity.noContent().build(); | ||
} | ||
|
||
|
@@ -102,8 +102,8 @@ public ResponseEntity<? extends HttpEntity> deleteUser(Authentication authentica | |
}) | ||
@GetMapping("/my-page") | ||
public ResponseEntity<UserThumbnailResponseDto> my(Authentication authentication) { | ||
// Long userId = Long.parseLong(authentication.getName()); | ||
Long userId = 1L; | ||
|
||
Long userId = getUserId(authentication); | ||
return ResponseEntity.ok().body(userThumbnailService.getUserThumbnail(userId)); | ||
} | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 기존에는 개발상 편의를 위해 단순히 1L으로 처리하였으나, 런칭을 위해, 테스트 계정이 아닌 요청은 정상적으로 처리하도록 메소드 적용하였습니다. |
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -35,4 +35,13 @@ public static User create(String uid, String name, String picture) { | |
user.setProfileImage(picture); | ||
return user; | ||
} | ||
|
||
public static User create(Long id, String uid, String name, String profileImage) { | ||
User user = new User(); | ||
user.id = id; | ||
user.uid = uid; | ||
user.name = name; | ||
user.profileImage = profileImage; | ||
return user; | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 테스트상 편의를 위해 id를 받아 User를 생성할 수 있도록 생성자를 추가하였습니다. 빌더패턴을 써도 좋지 않을까 하는 생각이 들긴했으나, 시간상 생략하였습니다. |
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,6 +2,7 @@ | |
|
||
import static org.springframework.security.core.userdetails.User.builder; | ||
|
||
import com.yapp.artie.domain.archive.repository.CategoryRepository; | ||
import com.yapp.artie.domain.user.domain.User; | ||
import com.yapp.artie.domain.user.dto.response.CreateUserResponseDto; | ||
import com.yapp.artie.domain.user.exception.UserNotFoundException; | ||
|
@@ -20,6 +21,8 @@ | |
public class UserService implements UserDetailsService { | ||
|
||
private final UserRepository userRepository; | ||
private final CategoryRepository categoryRepository; | ||
private final JwtService jwtService; | ||
|
||
public Optional<User> findByUid(String uid) { | ||
return userRepository.findByUid(uid); | ||
|
@@ -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) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 다른 리뷰에서도 말씀 드렸지만, 파라미터로 서비스를 넘기는 것이 좋지않다고 생각하며, 그럴 이유가 없다고 생각합니다. 해당 리뷰, 혹은 이 리뷰에 의견 남겨주시면 이야기해봐도 좋을 것 같습니다. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. controller에서 이미 jwtService를 주입받고 있어서 service를 전달해줬던 맥락이 있었습니다! 저도 이부분이 찜찜하긴 했는데요... controller의 jwtService에 대한 코드를 userService 내부로 이동시키면 좀 더 좋을 것 같다는 생각이 들었어요! |
||
User user = findById(id); | ||
jwtService.withdraw(user.getUid()); | ||
userRepository.deleteById(id); | ||
|
||
categoryRepository.deleteAllByUser(user); | ||
userRepository.delete(user); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. cascade 되어 자동으로 삭제처리를 해주는 작업을 하지 않기 때문에, 해당 유저와 관련이 있는 카테고리, 전시, 작품, 태그 데이터를 먼저 삭제하고, 유저 데이터를 삭제하는 식으로 작업하였습니다. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 이렇게 구현하셨군요 고생하셨습니다! 😀😀 참고로 처음에 회원 탈퇴할 때, 연관 데이터 삭제를 구현하지 않았던 맥락을 설명드리자면요... (일단 user에서 casecade로 지우는게 어려운지 몰랐었음..!) 아래 두가지 방법에 대해서 :
어떤 문제가 있냐면, 패키지간 지금 당장으로는 개선할 방법이 없어보이지만, 저번에 공유드렸던 우아한객체지향 세미나에서 이러한 문제를 이벤트를 사용해서 처리했던 걸 봤어요! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 오... 이벤트에 대해 한번 세미나 열어주시는 건가요 기다리겠습니다 ㅎㅎ There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 제가... 세미나는 무리구요 ㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋ.. 기능 구현 상으로는 문제가 없습니다! 다만, 더 좋은 설계는 user <-> archive 간에 의존성이 단방향으로 흘러야 해요! 이벤트로 해결하는 방법을 간략하게 말씀드리자면 user 패키지의 소속된 Delete 이벤트 객체를 발행하면, archive에서 Delete 이벤트 객체를 사용하는 방법이었던 걸로 기억해요.. 그렇게 되면 archive->user쪽으로 단방향 패키지 의존을 유지할 수 있게 됩니다! 이거 적용해보면 재밌겠네요 👍👍 |
||
} | ||
|
||
@Override | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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", "유효하지 않은 파이어베이스 식별자입니다."), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 창피하네요.. 바꿔주셔서 감사합니다.. ㅎㅎㅎ |
||
|
||
// User | ||
USER_NOT_FOUND(404, "U001", "회원을 찾을 수 없습니다."), | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
package com.yapp.artie; | ||
|
||
import com.fasterxml.jackson.databind.ObjectMapper; | ||
import org.junit.jupiter.api.Disabled; | ||
import org.springframework.beans.factory.annotation.Autowired; | ||
import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc; | ||
import org.springframework.boot.test.context.SpringBootTest; | ||
import org.springframework.test.web.servlet.MockMvc; | ||
import org.springframework.transaction.annotation.Transactional; | ||
|
||
@SpringBootTest | ||
@Disabled | ||
@AutoConfigureMockMvc | ||
@Transactional | ||
public class BaseIntegrationTest { | ||
|
||
@Autowired | ||
protected MockMvc mvc; | ||
@Autowired | ||
protected ObjectMapper objectMapper; | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 보다 높은 검증성을 보장하기 위해, 통합테스트와 유닛테스트를 도입해야합니다. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Base Class 만드신거 너무 좋은거 같아요! 제 생각에는 BaseIntegrationTest는 테스트 대상이 아니기 때문에 abstract가 맞을거 같아요! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 근데 abstract로 하면 동작하긴 할까?? 라는 생각이 드네요 ... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ㅋㅋㅋㅋㅋㅋ 이거 라이브 코딩 시간에 바로 해봐요~! |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,64 @@ | ||
package com.yapp.artie.domain.archive.repository; | ||
|
||
import static org.assertj.core.api.Assertions.assertThat; | ||
|
||
import com.yapp.artie.domain.archive.domain.artwork.Artwork; | ||
import com.yapp.artie.domain.archive.domain.category.Category; | ||
import com.yapp.artie.domain.archive.domain.exhibit.Exhibit; | ||
import com.yapp.artie.domain.archive.domain.tag.Tag; | ||
import com.yapp.artie.domain.user.domain.User; | ||
import com.yapp.artie.domain.user.domain.UserTest; | ||
import com.yapp.artie.domain.user.repository.UserRepository; | ||
import java.time.LocalDate; | ||
import javax.persistence.EntityManager; | ||
import org.junit.jupiter.api.DisplayName; | ||
import org.junit.jupiter.api.Test; | ||
import org.springframework.beans.factory.annotation.Autowired; | ||
import org.springframework.boot.test.autoconfigure.jdbc.AutoConfigureTestDatabase; | ||
import org.springframework.boot.test.autoconfigure.jdbc.AutoConfigureTestDatabase.Replace; | ||
import org.springframework.boot.test.autoconfigure.orm.jpa.DataJpaTest; | ||
|
||
@DataJpaTest | ||
@AutoConfigureTestDatabase(replace = Replace.NONE) | ||
public class CategoryRepositoryTest { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Category Repository Unit Test를 추가하였습니다. |
||
|
||
@Autowired | ||
EntityManager em; | ||
|
||
@Autowired | ||
UserRepository userRepository; | ||
|
||
@Autowired | ||
CategoryRepository categoryRepository; | ||
|
||
@Autowired | ||
ExhibitRepository exhibitRepository; | ||
|
||
@Autowired | ||
ArtworkRepository artworkRepository; | ||
|
||
@Autowired | ||
TagRepository tagRepository; | ||
|
||
@Test | ||
@DisplayName("유저 기반 카테고리 삭제 - 카테고리가 삭제되면 해당하는 전시, 작품, 태그도 삭제되어야합니다") | ||
void deleteAllByUser() { | ||
User user = userRepository.save(UserTest.TEST_SAVED_USER); | ||
Category category = categoryRepository.save(Category.create(user, "category-name", 1)); | ||
Exhibit exhibit = exhibitRepository.save( | ||
Exhibit.create("exhibit-name", LocalDate.now(), category, user, "exhibit-link")); | ||
Artwork artwork = artworkRepository.save(Artwork.create(exhibit, true, "artwork-uri")); | ||
Tag tag = tagRepository.save(new Tag(user, artwork, 1, "tag-name")); | ||
em.flush(); | ||
em.clear(); | ||
|
||
categoryRepository.deleteAllByUser(user); | ||
em.flush(); | ||
|
||
assertThat(userRepository.findById(user.getId()).isPresent()).isTrue(); | ||
assertThat(categoryRepository.findById(category.getId()).isPresent()).isFalse(); | ||
assertThat(exhibitRepository.findById(exhibit.getId()).isPresent()).isFalse(); | ||
assertThat(artworkRepository.findById(artwork.getId()).isPresent()).isFalse(); | ||
assertThat(tagRepository.findById(tag.getId()).isPresent()).isFalse(); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,8 +3,10 @@ | |
import static org.assertj.core.api.Assertions.assertThat; | ||
import static org.assertj.core.api.Assertions.assertThatThrownBy; | ||
|
||
import com.yapp.artie.domain.archive.domain.artwork.Artwork; | ||
import com.yapp.artie.domain.archive.domain.category.Category; | ||
import com.yapp.artie.domain.archive.domain.exhibit.Exhibit; | ||
import com.yapp.artie.domain.archive.domain.tag.Tag; | ||
import com.yapp.artie.domain.archive.dto.cateogry.CategoryDto; | ||
import com.yapp.artie.domain.archive.dto.cateogry.CreateCategoryRequestDto; | ||
import com.yapp.artie.domain.archive.dto.cateogry.UpdateCategoryRequestDto; | ||
|
@@ -13,8 +15,10 @@ | |
import com.yapp.artie.domain.archive.exception.ChangeCategoryWrongLengthException; | ||
import com.yapp.artie.domain.archive.exception.ExceededCategoryCountException; | ||
import com.yapp.artie.domain.archive.exception.NotOwnerOfCategoryException; | ||
import com.yapp.artie.domain.archive.repository.ArtworkRepository; | ||
import com.yapp.artie.domain.archive.repository.CategoryRepository; | ||
import com.yapp.artie.domain.archive.repository.ExhibitRepository; | ||
import com.yapp.artie.domain.archive.repository.TagRepository; | ||
import com.yapp.artie.domain.user.domain.User; | ||
import com.yapp.artie.domain.user.repository.UserRepository; | ||
import java.time.LocalDate; | ||
|
@@ -52,6 +56,12 @@ class CategoryServiceTest { | |
@Autowired | ||
ExhibitRepository exhibitRepository; | ||
|
||
@Autowired | ||
ArtworkRepository artworkRepository; | ||
|
||
@Autowired | ||
TagRepository tagRepository; | ||
|
||
@Autowired | ||
CategoryService categoryService; | ||
|
||
|
@@ -165,6 +175,25 @@ private Category findCategory(Long id) { | |
assertThat(Optional.ofNullable(findCategory(created))).isNotPresent(); | ||
} | ||
|
||
@Test | ||
public void delete_카테고리를_삭제하면_전시데이터도_삭제된다() throws Exception { | ||
User user = findUser("1"); | ||
Long created = createCategory(user, "test"); | ||
Exhibit exhibit = exhibitRepository.save( | ||
Exhibit.create("test", LocalDate.now(), findCategory(created), user, "link")); | ||
Artwork artwork = artworkRepository.save(Artwork.create(exhibit, true, "url")); | ||
Tag tag = tagRepository.save(new Tag(user, artwork, 1, "tagName")); | ||
em.clear(); | ||
|
||
categoryService.delete(created, user.getId()); | ||
em.flush(); | ||
|
||
assertThat(Optional.ofNullable(findCategory(created))).isNotPresent(); | ||
assertThat(exhibitRepository.findById(exhibit.getId()).isEmpty()).isTrue(); | ||
assertThat(artworkRepository.findById(artwork.getId()).isEmpty()).isTrue(); | ||
assertThat(tagRepository.findById(tag.getId()).isEmpty()).isTrue(); | ||
} | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 시간 상, Category Service Unit Test로는 구성하지 못했습니다. 기존의 비 유닛 테스트 방식을 유지하되, 검증 로직은 필요하므로 테스트를 추가하였습니다. |
||
@Test | ||
public void delete_존재하지_않는_카테고리를_삭제하면_예외를_발생한다() throws Exception { | ||
assertThatThrownBy(() -> { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,78 @@ | ||
package com.yapp.artie.domain.user.controller; | ||
|
||
import static org.assertj.core.api.Assertions.assertThat; | ||
import static org.mockito.ArgumentMatchers.anyString; | ||
import static org.mockito.Mockito.doNothing; | ||
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.delete; | ||
import static org.springframework.test.web.servlet.result.MockMvcResultHandlers.print; | ||
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; | ||
|
||
import com.yapp.artie.BaseIntegrationTest; | ||
import com.yapp.artie.domain.archive.domain.artwork.Artwork; | ||
import com.yapp.artie.domain.archive.domain.category.Category; | ||
import com.yapp.artie.domain.archive.domain.exhibit.Exhibit; | ||
import com.yapp.artie.domain.archive.domain.tag.Tag; | ||
import com.yapp.artie.domain.archive.repository.ArtworkRepository; | ||
import com.yapp.artie.domain.archive.repository.CategoryRepository; | ||
import com.yapp.artie.domain.archive.repository.ExhibitRepository; | ||
import com.yapp.artie.domain.archive.repository.TagRepository; | ||
import com.yapp.artie.domain.user.domain.User; | ||
import com.yapp.artie.domain.user.domain.UserTest; | ||
import com.yapp.artie.domain.user.repository.UserRepository; | ||
import com.yapp.artie.global.authentication.JwtServiceImpl; | ||
import java.time.LocalDate; | ||
import javax.persistence.EntityManager; | ||
import org.junit.jupiter.api.DisplayName; | ||
import org.junit.jupiter.api.Test; | ||
import org.springframework.beans.factory.annotation.Autowired; | ||
import org.springframework.boot.test.mock.mockito.MockBean; | ||
|
||
class UserControllerTest extends BaseIntegrationTest { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. User API를 검증하는 통합 테스트 입니다. |
||
|
||
@Autowired | ||
EntityManager em; | ||
|
||
@Autowired | ||
UserRepository userRepository; | ||
|
||
@Autowired | ||
CategoryRepository categoryRepository; | ||
|
||
@Autowired | ||
ExhibitRepository exhibitRepository; | ||
|
||
@Autowired | ||
ArtworkRepository artworkRepository; | ||
|
||
@Autowired | ||
TagRepository tagRepository; | ||
|
||
@MockBean | ||
JwtServiceImpl jwtService; | ||
|
||
// TODO : 헤더 토큰 ( Authentication ) 모킹 필요. 현재는 getUserId 메소드에 의존하고 있음 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 헤더 토큰 모킹은 시간상 도입하지 못했습니다. 시도해봤는데, 실패해서ㅜㅜ 같이 방법 찾아서 도입하고 싶습니다. Spring Security에 대한 지식이 부족한 것도 한 몫... 합니다 ㅎㅎㅎ |
||
@Test | ||
@DisplayName("유저 삭제(탈퇴) API - 유저 데이터가 삭제되면 해당 유저의 카테고리, 전시, 작품, 태그 데이터가 함께 삭제되어야합니다.") | ||
public void deleteUser() throws Exception { | ||
User user = userRepository.save(UserTest.TEST_SAVED_USER); | ||
Category category = categoryRepository.save(Category.create(user, "category-name", 1)); | ||
Exhibit exhibit = exhibitRepository.save( | ||
Exhibit.create("exhibit-name", LocalDate.now(), category, user, "exhibit-link")); | ||
Artwork artwork = artworkRepository.save(Artwork.create(exhibit, true, "artwork-uri")); | ||
Tag tag = tagRepository.save(new Tag(user, artwork, 1, "tag-name")); | ||
em.clear(); | ||
|
||
doNothing().when(jwtService).withdraw(anyString()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 외부 서비스는 호출되어서는 안되므로, Firebase의 유저 탈퇴 처리를 하는 jwtServiceImple은 Mockito로 모킹처리하였습니다. |
||
|
||
mvc.perform(delete("/user")) | ||
.andExpect(status().isNoContent()) | ||
.andDo(print()); | ||
|
||
em.flush(); | ||
assertThat(userRepository.findById(user.getId()).isPresent()).isFalse(); | ||
assertThat(categoryRepository.findById(category.getId()).isPresent()).isFalse(); | ||
assertThat(exhibitRepository.findById(exhibit.getId()).isPresent()).isFalse(); | ||
assertThat(artworkRepository.findById(artwork.getId()).isPresent()).isFalse(); | ||
assertThat(tagRepository.findById(tag.getId()).isPresent()).isFalse(); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
package com.yapp.artie.domain.user.domain; | ||
|
||
import org.junit.jupiter.api.DisplayName; | ||
|
||
@DisplayName("User 테스트") | ||
public class UserTest { | ||
|
||
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"); | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 테스트상 편의를 위해 유저를 static으로 선언하였습니다. 1L의 id를 갖는 다는 것을 보장하게 하고 싶었습니다. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,37 @@ | ||
package com.yapp.artie.domain.user.repository; | ||
|
||
import static org.assertj.core.api.Assertions.assertThat; | ||
|
||
import com.yapp.artie.domain.user.domain.User; | ||
import com.yapp.artie.domain.user.domain.UserTest; | ||
import javax.persistence.EntityManager; | ||
import org.junit.jupiter.api.DisplayName; | ||
import org.junit.jupiter.api.Test; | ||
import org.springframework.beans.factory.annotation.Autowired; | ||
import org.springframework.boot.test.autoconfigure.jdbc.AutoConfigureTestDatabase; | ||
import org.springframework.boot.test.autoconfigure.jdbc.AutoConfigureTestDatabase.Replace; | ||
import org.springframework.boot.test.autoconfigure.orm.jpa.DataJpaTest; | ||
|
||
@DataJpaTest | ||
@AutoConfigureTestDatabase(replace = Replace.NONE) | ||
public class UserRepositoryTest { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. User Repository Unit Test를 추가하였습니다. |
||
|
||
@Autowired | ||
EntityManager em; | ||
|
||
@Autowired | ||
UserRepository userRepository; | ||
|
||
@Test | ||
@DisplayName("유저 삭제") | ||
void delete() { | ||
User user = userRepository.save(UserTest.TEST_SAVED_USER); | ||
em.clear(); | ||
|
||
userRepository.delete(user); | ||
em.flush(); | ||
|
||
assertThat(userRepository.findById(user.getId()).isPresent()).isFalse(); | ||
} | ||
|
||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CascadeType을 Remove로 통일하였습니다.
현재는 CascadeType.Persist 등을 이용한 코드가 없고, 삭제를 목적으로 cascadetype을 사용했으므로, Remove로 처리하는 것이 좋다고 판단하였습니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
적절한 판단이라고 생각해요👍👍 처음에 ALL로 둔 이유는 상위 객체의 메서드를 이용해서 하위 객체를 저장할 수도 있겠다 싶겠다는 맥락이 있었는데요.. 지금 저희 코드를 보면 Remove만 주는 것이 좋아보이네요!