Refactor/#38 유저 도메인 리팩터링 #60

1o18z · 2023-10-29T02:48:02Z

이슈번호

close: #38

작업 내용 설명

유저 도메인 리팩터링

리뷰어에게 한마디

변경사항들 코멘트로 달아두었습니다 ! 🙆🏻‍♀️

github-actions · 2023-10-29T02:49:39Z

Test Results

13 files 13 suites 2s ⏱️
24 tests 24 ✔️ 0 💤 0 ❌
25 runs 25 ✔️ 0 💤 0 ❌

Results for commit b639277.

♻️ This comment has been updated with latest results.

1o18z · 2023-10-29T02:51:57Z

src/main/java/coffeemeet/server/user/controller/UserController.java

-      @Valid @RequestBody UpdateProfileRequest request) {
-    userService.updateProfileInfo(authInfo.userId(), request.nickname(), request.name(),
-        request.interests());
+      @Valid @RequestBody UpdateProfileDto.Request request) {


이름 수정은 불가하다는 정책에 따라 UpdateProfileDto의 name 필드를 제거 후 수정하였습니다!

1o18z · 2023-10-29T02:55:30Z

src/main/java/coffeemeet/server/user/service/UserService.java

+  private void updateInterests(User user, List<Keyword> keywords) {
+    List<Interest> currentInterests = interestCommand.findAllByUserId(user.getId());
+    interestCommand.deleteAll(currentInterests);
+    interestCommand.saveAll(keywords, user);


단순히 가독성을 위해 관심사를 업데이트 하는 로직을 메서드로 뺐습니다!
해당 메서드는 사용자 프로필 정보 수정 기능에서 사용됩니다.

1o18z · 2023-10-29T02:58:43Z

src/main/java/coffeemeet/server/user/service/UserService.java

        request.authCode());

-    checkDuplicatedUser(response);
-    checkDuplicatedNickname(request.nickname());
+    userQuery.hasDuplicatedUser(response.oAuthProvider(), response.oAuthProviderId());


Request 전체를 넘겨주지 않고 필요한 값만 빼서 전달하도록 수정하였습니다!

1o18z · 2023-10-29T03:04:51Z

src/main/java/coffeemeet/server/user/service/cq/UserQuery.java

+  public void hasDuplicatedNickname(String nickname) {
+    if (userRepository.existsUserByProfile_Nickname(nickname)) {
+      throw new IllegalArgumentException("이미 존재하는 닉네임입니다.");
+    }
+  }
+
+  public void hasDuplicatedUser(OAuthProvider oAuthProvider, String oAuthProviderId) {
+    if (userRepository.existsUserByOauthInfo_oauthProviderAndOauthInfo_oauthProviderId(
+        oAuthProvider, oAuthProviderId)) {
+      throw new IllegalArgumentException(ALREADY_REGISTERED_MESSAGE);
+    }
+  }
+


사용자와 닉네임 중복 체크를 일단 UserQuery에 두긴 했는데,, UserCommand와 UserQuery 중 어디에 두는게 더 적절한 것 같은지 의견 궁금합니다🙇🏻‍♀️🙇🏻‍♀️👍🏻

UserRepository는 쿼리지만, 이 메서드를 사용하는 사용자입장에서는 Command라고 생각했어요

그래서 제 생각엔 hasDuplicatedUser, hasDuplicatedNickname 둘 다 Command로 생각됩니다.

저는 반대로 사용자 입장에서 생각하는건 서비스 레이어고,
이 커맨드 쿼리 부분은 객체 내부상태 입장에서 생각하는게 맞아 보여요
객체의 내부 상태를 변환하면서 반환 상태가 없으면, 커맨드
그렇지 않으면 쿼리

즉, 이 부분은 쿼리가 맞는거 같아요

smartandhandsome

수고하셨습니다

src/main/java/coffeemeet/server/user/domain/OAuthInfo.java

src/main/java/coffeemeet/server/user/domain/Profile.java

src/main/java/coffeemeet/server/user/dto/SignupDto.java

smartandhandsome · 2023-10-29T11:19:27Z

src/main/java/coffeemeet/server/user/service/UserService.java

+    Long userId = userCommand.saveUser(user);
+    User newUser = userQuery.getUserById(userId);
+
+    interestCommand.saveAll(request.keywords(), newUser);


아 id값을 DB에 의존하게 되서 이렇게 코딩하신거 같은데 userId 리턴이 아니라 걍 User리턴으로 해도 될거 같은데, 월요일에 다 같이 한 번 얘기 하시죠

smartandhandsome · 2023-10-29T11:28:12Z

src/main/java/coffeemeet/server/user/service/UserService.java

+  public void checkDuplicatedNickname(String nickname) {
+    userQuery.hasDuplicatedNickname(nickname);
+  }
+
+  public void deleteUser(Long userId) {
+    userCommand.deleteUser(userId);
  }


제 생각엔 이거 메서드로 따로 뺄 필요는 없어 보이네용?

상민님, 이 부분 만약 따로 안빼면 컨트롤러에서 직접 커맨드 또는 쿼리 레이어에 의존하나요?

smartandhandsome · 2023-10-29T11:37:09Z

src/main/java/coffeemeet/server/user/service/UserService.java

+    User user = userQuery.getUserById(userId);
+    userQuery.hasDuplicatedNickname(nickname);
+    user.updateNickname(nickname);


더티 체킹이 동작하지 않아서 업데이트가 안될 것 같아요

제 생각엔 위 세줄을 묶어서 UserCommnad로 만들어도 좋을 것 같습니다.

smartandhandsome · 2023-10-29T11:41:46Z

src/main/java/coffeemeet/server/user/service/cq/UserQuery.java

+  public void hasDuplicatedNickname(String nickname) {
+    if (userRepository.existsUserByProfile_Nickname(nickname)) {
+      throw new IllegalArgumentException("이미 존재하는 닉네임입니다.");
+    }
+  }
+
+  public void hasDuplicatedUser(OAuthProvider oAuthProvider, String oAuthProviderId) {
+    if (userRepository.existsUserByOauthInfo_oauthProviderAndOauthInfo_oauthProviderId(
+        oAuthProvider, oAuthProviderId)) {
+      throw new IllegalArgumentException(ALREADY_REGISTERED_MESSAGE);
+    }
+  }
+


UserRepository는 쿼리지만, 이 메서드를 사용하는 사용자입장에서는 Command라고 생각했어요

그래서 제 생각엔 hasDuplicatedUser, hasDuplicatedNickname 둘 다 Command로 생각됩니다.

yumyeonghan

수고하셨습니다. 지금 저희 프로젝트 구조가 조금씩 의견이 갈리는 부분이 있어서 회의하고 차근차근 정해봅시다.!

yumyeonghan · 2023-10-29T12:27:55Z

src/main/java/coffeemeet/server/user/service/UserService.java

 import java.util.List;
 import lombok.RequiredArgsConstructor;
 import org.springframework.stereotype.Service;
-import org.springframework.transaction.annotation.Transactional;



서비스에 트랜잭션 있는게 좋을거같아 보여요!

yumyeonghan · 2023-10-29T12:31:35Z

src/main/java/coffeemeet/server/user/service/cq/UserQuery.java

+  public void hasDuplicatedNickname(String nickname) {
+    if (userRepository.existsUserByProfile_Nickname(nickname)) {
+      throw new IllegalArgumentException("이미 존재하는 닉네임입니다.");
+    }
+  }
+
+  public void hasDuplicatedUser(OAuthProvider oAuthProvider, String oAuthProviderId) {
+    if (userRepository.existsUserByOauthInfo_oauthProviderAndOauthInfo_oauthProviderId(
+        oAuthProvider, oAuthProviderId)) {
+      throw new IllegalArgumentException(ALREADY_REGISTERED_MESSAGE);
+    }
+  }
+


저는 반대로 사용자 입장에서 생각하는건 서비스 레이어고,
이 커맨드 쿼리 부분은 객체 내부상태 입장에서 생각하는게 맞아 보여요
객체의 내부 상태를 변환하면서 반환 상태가 없으면, 커맨드
그렇지 않으면 쿼리

즉, 이 부분은 쿼리가 맞는거 같아요

yumyeonghan · 2023-10-29T12:36:38Z

src/main/java/coffeemeet/server/user/service/UserService.java

+  public void checkDuplicatedNickname(String nickname) {
+    userQuery.hasDuplicatedNickname(nickname);
+  }
+
+  public void deleteUser(Long userId) {
+    userCommand.deleteUser(userId);
  }


상민님, 이 부분 만약 따로 안빼면 컨트롤러에서 직접 커맨드 또는 쿼리 레이어에 의존하나요?

smartandhandsome · 2023-10-30T06:20:12Z

src/main/java/coffeemeet/server/user/service/UserService.java

+  private void updateInterests(User user, List<Keyword> keywords) {
+    List<Interest> currentInterests = interestCommand.findAllByUserId(user.getId());
+    interestCommand.deleteAll(currentInterests);
+    interestCommand.saveAll(keywords, user);


InterestCommand에 updateInterests라는 메서드로 쫙 묶어도 될 것 같습니다.

1o18z added 13 commits October 28, 2023 23:11

refactor: User 도메인 검증 로직 수정

b379dfd

refactor: UserController url 변경

8b75e76

refactor: UserService 분리

de67395

refactor: InterestService 분리

c5fdd55

refactor: 유저 이름 수정 로직 제거

72e4a19

refactor: 유저 DTO를 sealed class로 변경

a8d39bf

refactor: 계층 간 DTO 분리를 위한 OAuthUserInfo 생성

33abbe5

refactor: CertificationService가 UserQuery를 의존하도록 변경

8b0a9cc

fix: 리베이스 시 충돌 해결

e289142

refactor: 메서드명 변경 및 이동

ba55e23

refactor: UserCommand 유저 저장 메서드의 반환타입 변경

c64540d

refactor: 불필요한 메서드 제거 및 잘못된 파라미터 수정

2abf270

refactor: 관심사 수정 메서드 분리

4d65471

1o18z added the refactor 리팩터링 label Oct 29, 2023

1o18z added this to the 1차 스프린트 milestone Oct 29, 2023

1o18z self-assigned this Oct 29, 2023

1o18z requested review from smartandhandsome and yumyeonghan as code owners October 29, 2023 02:48

1o18z commented Oct 29, 2023

View reviewed changes

refactor: 변경된 메서드명에 따라 테스트코드 수정

f8feab2

smartandhandsome reviewed Oct 29, 2023

View reviewed changes

yumyeonghan reviewed Oct 29, 2023

View reviewed changes

yumyeonghan approved these changes Oct 29, 2023

View reviewed changes

refactor: 누락된 @NonNull 적용

5ba3fc4

smartandhandsome reviewed Oct 30, 2023

View reviewed changes

refactor: 유저 업데이트 로직 분리

b639277

1o18z force-pushed the feat/#38 branch from 2a98a8b to b639277 Compare October 30, 2023 07:06

1o18z merged commit 254377f into dev Oct 30, 2023
3 checks passed

1o18z deleted the feat/#38 branch October 30, 2023 07:23

yumyeonghan modified the milestones: 1차 스프린트, 2차 스프린트 Nov 16, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor/#38 유저 도메인 리팩터링 #60

Refactor/#38 유저 도메인 리팩터링 #60

1o18z commented Oct 29, 2023

github-actions bot commented Oct 29, 2023 •

edited

Loading

1o18z Oct 29, 2023

1o18z Oct 29, 2023 •

edited

Loading

1o18z Oct 29, 2023

1o18z Oct 29, 2023

smartandhandsome Oct 29, 2023

yumyeonghan Oct 29, 2023

smartandhandsome left a comment

smartandhandsome Oct 29, 2023

smartandhandsome Oct 29, 2023

yumyeonghan Oct 29, 2023

smartandhandsome Oct 29, 2023

smartandhandsome Oct 29, 2023

yumyeonghan left a comment

yumyeonghan Oct 29, 2023

yumyeonghan Oct 29, 2023

yumyeonghan Oct 29, 2023

smartandhandsome Oct 30, 2023

Refactor/#38 유저 도메인 리팩터링 #60

Refactor/#38 유저 도메인 리팩터링 #60

Conversation

1o18z commented Oct 29, 2023

이슈번호

작업 내용 설명

리뷰어에게 한마디

github-actions bot commented Oct 29, 2023 • edited Loading

Test Results

Choose a reason for hiding this comment

1o18z Oct 29, 2023 • edited Loading

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

smartandhandsome 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

yumyeonghan 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

github-actions bot commented Oct 29, 2023 •

edited

Loading

1o18z Oct 29, 2023 •

edited

Loading