[jinyoungchoi95-issue16] 현재 유저 정보 반환 및 업데이트 기능 구현 #33

jinyoungchoi95 · 2021-08-21T10:26:24Z

Issue: #16

작업 내용

GET /api/user
PUT /api/user

생성/변경 로직

업데이트 요청 값이 null한 값이 들어올 수 있어 null에 대한 핸들링이 필요하여 service에 update용 model추가
업데이트 불가한 상황들에 대해서 exception 혹은 업데이트 금지 로직 구현
user entity내 필드 변경 메소드(change...()) 추가
token 값 가져오는 부분이 공통되어 해당 메소드 분리(UserController.class)

개인 코멘트

두 가지가 제일 어려웠던 것 같아요. @AuthenticationPrincipal에 대한 mocking, Validation에 대한 처리
@AuthenticationPrincipal의 경우 현재 단위테스트만 하고 있는 구조상 내부 spring security filter에 대한 더 자세한 모킹이 필요했으나 진행을하다보니 굳이 이걸 하는데 공수가 들어야하나?라는 생각을 했고 단순히 테스팅하였습니다.
Spring Validation의 가장 큰 단점을 느꼈던 issue였던것 같아요. 파라메터에서만 받을 수 있는 validation의 특성상 update를 구현하는데 있어서 파일을 다 validate하면서 가져오자니 null한값이 들어올 수도 있는 아이러니한 상황이었기 때문이에요. 어쩔 수 없이 valid메소드를 분리하였는데 조금 이상해서 거슬리긴하네요 ... ㅎㅎ

- findById() > Optional<User> 반환

- UserService.update() : 현재 유저가 존재하지 않을 경우 exception - username : 현재 username 중복, repository 중복 값 불가 - email : 현재 email 중복, repository 중복 값 불가 - password : 현재 password 중복 불가 - bio : 현재 bio 중복 불가 - image : 현재 image 중복 불가 - UserUpdateModel : 유저 업데이트용 파라메터 모델

codecov-commenter · 2021-08-21T10:28:21Z

Codecov Report

Merging #33 (2aabc23) into jinyoungchoi95 (8b4be91) will increase coverage by 1.11%.
The diff coverage is 100.00%.

@@                 Coverage Diff                  @@
##             jinyoungchoi95      #33      +/-   ##
====================================================
+ Coverage             94.93%   96.05%   +1.11%     
- Complexity               81      118      +37     
====================================================
  Files                    14       16       +2     
  Lines                   237      304      +67     
  Branches                  4        6       +2     
====================================================
+ Hits                    225      292      +67     
  Misses                    6        6              
  Partials                  6        6

Impacted Files	Coverage Δ
...tudy/realworld/user/controller/UserController.java	`100.00% <100.00%> (ø)`
...rld/user/controller/request/UserUpdateRequest.java	`100.00% <100.00%> (ø)`
...alworld/user/controller/response/UserResponse.java	`100.00% <100.00%> (ø)`
...in/java/com/study/realworld/user/domain/Email.java	`77.77% <100.00%> (+8.54%)`	⬆️
...java/com/study/realworld/user/domain/Password.java	`100.00% <100.00%> (ø)`
...ain/java/com/study/realworld/user/domain/User.java	`93.33% <100.00%> (+1.33%)`	⬆️
...java/com/study/realworld/user/domain/Username.java	`78.94% <100.00%> (+9.71%)`	⬆️
.../com/study/realworld/user/service/UserService.java	`100.00% <100.00%> (ø)`
.../realworld/user/service/model/UserUpdateModel.java	`100.00% <100.00%> (ø)`
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8b4be91...2aabc23. Read the comment docs.

- rest docs 생성 시 null이 들어가는 것을 금지하므로 optional()을 통해 이를 허용

kwj1270 · 2021-08-25T11:27:30Z

👀

povia

역시 잘 짜셨네요! 👍 다시 고려해봐야 할 부분, 잘 되어있다고 보이는 부분만 코멘트 드렸습니다.

povia · 2021-08-25T11:48:13Z

src/main/java/com/study/realworld/user/controller/request/UserUpdateRequest.java

+
+@JsonTypeName(value = "user")
+@JsonTypeInfo(include = JsonTypeInfo.As.WRAPPER_OBJECT, use = JsonTypeInfo.Id.NAME)
+public class UserUpdateRequest {


요청 별 DTO 분리 좋습니다~ 👍

povia · 2021-08-25T15:08:42Z

src/main/java/com/study/realworld/user/service/UserService.java

+    private void validByPassword(@Valid Password password) {
+    }


Validation을 사용하는데 있어서 각각의 validate 로직이 vo들에 있다보니 따로 처리하기가 힘들어 지금과 같은 이상한 구조가 생겨버렸네요 🤣

default한 validation이 파라메터에서의 사용을 강제하는 부분이 있어 고민하던 중이었는데 구아바로 변경함으로써 지금과 같은 코드는 걷어내었습니다 :)

povia · 2021-08-25T18:13:50Z

src/main/java/com/study/realworld/user/service/UserService.java

+    }
+
+    @Transactional
+    public User update(UserUpdateModel updateUser, Long userId) {


해당 부분의 내부 구현 수정이 필요할 듯 합니다. 유선으로 말씀드린 대로 하나의 메소드에서 너무 많은 구현이 되고 있고, 그중 VO 단으로 넘길 수 있는 구현부도 있어 고민해보면서 수정하시면 좋을 듯 합니다! 👍

처음에는 null한 부분을 처리하는데 급급해서 지금처럼 한군데 모아놓고 로직을 설계했는데 다시보니까 update 메소드 자체에서 하고있는 역할이 너무 많더라구요 😢
최대한 구현 변경을 진행해 보았습니다 :)

kwj1270

크게 리뷰 드릴건 없는 것 같아요
같이 생각해볼 점 몇가지만 남겼으니 참고 부탁드려요 😉

kwj1270 · 2021-08-26T06:26:16Z

src/main/java/com/study/realworld/user/controller/UserController.java

+        User user = userService
+            .findById(loginId).orElseThrow(RuntimeException::new);   // 임시


Suggested change

User user = userService

.findById(loginId).orElseThrow(RuntimeException::new); // 임시

User user = userService.findById(loginId)

.orElseThrow(RuntimeException::new); // 임시

Suggested change

User user = userService

.findById(loginId).orElseThrow(RuntimeException::new); // 임시

User user = userService.findById(loginId).orElseThrow(RuntimeException::new); // 임시

위 2방법 중 하나로 하면 가독성이 좋을 것 같아요!

ide 내 code style 적용이 잘못되어서 폭이 짧아 어쩔수없이 현재와 같이 설정했었는데 두번째 말씀드린 방법으로 수정하였습니다 :)

kwj1270 · 2021-08-26T06:27:19Z

src/main/java/com/study/realworld/user/controller/request/UserUpdateRequest.java

+    protected UserUpdateRequest() {
+    }


상속 받는 구조가 없는 것 같고,
jsonpath() 사용하시는걸로 알아서 제거해도 좋을 것 같아요!

jsonpath를 사용하고있기는한데 object mapper가 기본적으로 요구하는 부분에 대해서는 놔두기로 했어요.
어짜피 기본생성자는 컴파일러가 생성해주고, 기본생성자를 컴파일러가 생성해준다고해도, 직접 기본생성자를 생성해두는 편이기는 해서요 :)

kwj1270 · 2021-08-26T06:30:37Z

src/main/java/com/study/realworld/user/service/UserService.java

+        updateUser.getUsername()
+            .filter(username -> !user.getUsername().equals(username))
+            .ifPresent(
+                username -> {
+                    checkDuplicatedByUsername(username);
+                    user.changeUsername(username);
+                });


이 부분은 개선 가능할 것 같아요

유선으로 이야기했던 것처럼 UserService 자체를 변경해보았는데 확인한번 부탁드려요 :)
관련 커밋링크는 여러가지가 분리되어있어서 드리기가 어려울거같네요 ㅜㅜ

DolphaGo

몇 가지 의견만 드렸습니다.
자유롭게 의견 교환해요~

src/main/java/com/study/realworld/user/controller/request/UserUpdateRequest.java

src/main/java/com/study/realworld/user/domain/Password.java

src/main/java/com/study/realworld/user/service/UserService.java

src/main/java/com/study/realworld/user/service/model/UserUpdateModel.java

povia

👍

povia · 2021-08-28T15:59:54Z

src/main/java/com/study/realworld/user/domain/Email.java

+    }
+
+    private static boolean checkEmailPattern(String address) {
+        return matches("[\\w~\\-.+]+@[\\w~\\-]+(\\.[\\w~\\-]+)+", address);


패턴 매칭 잘되네요 좋습니다 👍

패턴 매칭도 해야하는데.. 다음에 적용해봐야겠군요

povia · 2021-08-28T16:01:17Z

src/main/java/com/study/realworld/user/service/UserService.java

+        updateUser.getUsername().ifPresent(username -> checkDuplicatedUsernameAndChangeUsername(user, username));
+        updateUser.getEmail().ifPresent(email -> checkDuplicatedEmailAndChangeEmail(user, email));
+        updateUser.getPassword().ifPresent(password -> user.changePassword(password, passwordEncoder));
+        updateUser.getBio().ifPresent(user::changeBio);
+        updateUser.getImage().ifPresent(user::changeImage);


변경 구조 좋습니다 👍

povia · 2021-08-28T16:01:47Z

src/test/java/com/study/realworld/user/controller/UserControllerTest.java

+            .andExpect(jsonPath("$.user.bio", is(nullValue())))
+            .andExpect(jsonPath("$.user.image", is(nullValue())))


DolphaGo

💯

jinyoungchoi95 added 8 commits August 19, 2021 23:59

[#16] feat: UserService findById 검색 기능 구현

ed3c067

- findById() > Optional<User> 반환

[#16] test: join test docs optional 추가

4afceb9

[#16] feat: "GET /api/user" api 추가 및 테스트

beadb72

[#16] refactor

9bb2f86

[#16] refactor: Duplicated 메소드 분리 (username, email)

061f34e

[#16] feat: 'PUT /api/user' user update api 구현 및 테스트

8768ac9

[#16] refactor: security context holder 내에 token 생성 기능 메소드 분리

e97bff4

jinyoungchoi95 added feature ori 오리의 이슈 user labels Aug 21, 2021

jinyoungchoi95 self-assigned this Aug 21, 2021

jinyoungchoi95 force-pushed the jinyoungchoi95-issue16 branch from 7717c88 to 1e0cb6f Compare August 21, 2021 16:07

[#16] feat: user response json 내에 null값 들어가도록 변경

b2b4397

- rest docs 생성 시 null이 들어가는 것을 금지하므로 optional()을 통해 이를 허용

jinyoungchoi95 force-pushed the jinyoungchoi95-issue16 branch from 1e0cb6f to b2b4397 Compare August 21, 2021 16:09

[#16] test: beforeEach SecurityContextHolder clear 추가

fe92682

jinyoungchoi95 linked an issue Aug 25, 2021 that may be closed by this pull request

[jinyoungchoi95] 현재 유저 정보 반환 및 업데이트 기능 구현 #16

Closed

2 tasks

jinyoungchoi95 removed a link to an issue Aug 25, 2021

[jinyoungchoi95] 현재 유저 정보 반환 및 업데이트 기능 구현 #16

Closed

2 tasks

povia approved these changes Aug 25, 2021

View reviewed changes

povia reviewed Aug 25, 2021

View reviewed changes

kwj1270 reviewed Aug 26, 2021

View reviewed changes

jinyoungchoi95 added 7 commits August 27, 2021 00:45

chore: 구아바 implementation 추가

730749f

chore: apache commons library 추가

b6f5143

feat: password valid guava 전환 및 테스트 수정

7b0e701

feat: password valid guava 전환으로 관련 클래스 로직 변경

28f49ab

[#16] refactor

d2158e4

[#16] refactor: 더티체킹으로 필요없는 로직 제거

a1e2048

[#16] refactor

7862de0

jinyoungchoi95 added 6 commits August 27, 2021 09:18

[#16] refactor

8f32d22

[#16] refactor

15c4dfa

[#16] refactor

7616243

[#16] refactor: password change 메소드 수정

f7eb279

[#16] refactor

3b1a5b2

[#16] feat: username validation guava 전환

ca19bfd

DolphaGo reviewed Aug 27, 2021

View reviewed changes

jinyoungchoi95 added 5 commits August 28, 2021 18:23

[#16] feat: email validation guava 전환

d02b358

[#16] refactor: check duplicate 및 change 메소드 통합

845a389

[#16] refactor

6bb273f

[#16] refactor

c1fbfad

[#16] refactor: UserUpdateRequest dto Optional 제거

2aabc23

kwj1270 approved these changes Aug 28, 2021

View reviewed changes

povia approved these changes Aug 28, 2021

View reviewed changes

DolphaGo approved these changes Aug 30, 2021

View reviewed changes

jinyoungchoi95 merged commit 45d1da6 into jinyoungchoi95 Aug 30, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[jinyoungchoi95-issue16] 현재 유저 정보 반환 및 업데이트 기능 구현 #33

[jinyoungchoi95-issue16] 현재 유저 정보 반환 및 업데이트 기능 구현 #33

jinyoungchoi95 commented Aug 21, 2021

codecov-commenter commented Aug 21, 2021 •

edited

kwj1270 commented Aug 25, 2021

povia left a comment

povia Aug 25, 2021

povia Aug 25, 2021

jinyoungchoi95 Aug 26, 2021

povia Aug 25, 2021

jinyoungchoi95 Aug 28, 2021

kwj1270 left a comment

kwj1270 Aug 26, 2021

jinyoungchoi95 Aug 27, 2021

kwj1270 Aug 26, 2021

jinyoungchoi95 Aug 28, 2021

kwj1270 Aug 26, 2021

jinyoungchoi95 Aug 28, 2021

DolphaGo left a comment

povia left a comment

povia Aug 28, 2021

kwj1270 Aug 28, 2021

povia Aug 28, 2021

povia Aug 28, 2021

kwj1270 Aug 28, 2021

DolphaGo left a comment

		User user = userService
		.findById(loginId).orElseThrow(RuntimeException::new); // 임시

	User user = userService
	.findById(loginId).orElseThrow(RuntimeException::new); // 임시
	User user = userService.findById(loginId).orElseThrow(RuntimeException::new); // 임시

		.andExpect(jsonPath("$.user.bio", is(nullValue())))
		.andExpect(jsonPath("$.user.image", is(nullValue())))

[jinyoungchoi95-issue16] 현재 유저 정보 반환 및 업데이트 기능 구현 #33

[jinyoungchoi95-issue16] 현재 유저 정보 반환 및 업데이트 기능 구현 #33

Conversation

jinyoungchoi95 commented Aug 21, 2021

Issue: #16

작업 내용

생성/변경 로직

개인 코멘트

codecov-commenter commented Aug 21, 2021 • edited

Codecov Report

kwj1270 commented Aug 25, 2021

povia 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

kwj1270 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

Choose a reason for hiding this comment

DolphaGo left a comment

Choose a reason for hiding this comment

povia 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

DolphaGo left a comment

Choose a reason for hiding this comment

codecov-commenter commented Aug 21, 2021 •

edited