-
Notifications
You must be signed in to change notification settings - Fork 1
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
[jinyoungchoi95-issue16] 현재 유저 정보 반환 및 업데이트 기능 구현 #33
Changes from 14 commits
ed3c067
4afceb9
beadb72
9bb2f86
061f34e
2c1f363
8768ac9
e97bff4
b2b4397
fe92682
730749f
b6f5143
7b0e701
28f49ab
d2158e4
a1e2048
7862de0
8f32d22
15c4dfa
7616243
f7eb279
3b1a5b2
ca19bfd
d02b358
845a389
6bb273f
c1fbfad
2aabc23
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 |
---|---|---|
@@ -0,0 +1,64 @@ | ||
package com.study.realworld.user.controller.request; | ||
|
||
import com.fasterxml.jackson.annotation.JsonProperty; | ||
import com.fasterxml.jackson.annotation.JsonTypeInfo; | ||
import com.fasterxml.jackson.annotation.JsonTypeName; | ||
import com.study.realworld.user.domain.Email; | ||
import com.study.realworld.user.domain.Password; | ||
import com.study.realworld.user.domain.Username; | ||
import com.study.realworld.user.service.model.UserUpdateModel; | ||
import java.util.Optional; | ||
|
||
@JsonTypeName(value = "user") | ||
@JsonTypeInfo(include = JsonTypeInfo.As.WRAPPER_OBJECT, use = JsonTypeInfo.Id.NAME) | ||
public class UserUpdateRequest { | ||
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. 요청 별 DTO 분리 좋습니다~ 👍 |
||
|
||
@JsonProperty("username") | ||
private String username; | ||
|
||
@JsonProperty("email") | ||
private String email; | ||
|
||
@JsonProperty("password") | ||
private String password; | ||
|
||
@JsonProperty("bio") | ||
private String bio; | ||
|
||
@JsonProperty("image") | ||
private String image; | ||
|
||
protected UserUpdateRequest() { | ||
} | ||
Comment on lines
+31
to
+32
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. jsonpath를 사용하고있기는한데 object mapper가 기본적으로 요구하는 부분에 대해서는 놔두기로 했어요. |
||
|
||
public String getUsername() { | ||
return username; | ||
} | ||
|
||
public String getEmail() { | ||
return email; | ||
} | ||
|
||
public String getPassword() { | ||
return password; | ||
} | ||
|
||
public String getBio() { | ||
return bio; | ||
} | ||
|
||
public String getImage() { | ||
return image; | ||
} | ||
|
||
public UserUpdateModel toUserUpdateModel() { | ||
return new UserUpdateModel( | ||
Optional.ofNullable(getUsername()).map(Username::new).orElse(null), | ||
Optional.ofNullable(getEmail()).map(Email::new).orElse(null), | ||
Optional.ofNullable(getPassword()).map(Password::new).orElse(null), | ||
DolphaGo marked this conversation as resolved.
Show resolved
Hide resolved
|
||
getBio(), | ||
getImage() | ||
); | ||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,6 +5,7 @@ | |
import com.study.realworld.user.domain.User; | ||
import com.study.realworld.user.domain.UserRepository; | ||
import com.study.realworld.user.domain.Username; | ||
import com.study.realworld.user.service.model.UserUpdateModel; | ||
import java.util.Optional; | ||
import javax.validation.Valid; | ||
import org.springframework.security.crypto.password.PasswordEncoder; | ||
|
@@ -26,7 +27,8 @@ public UserService(UserRepository userRepository, PasswordEncoder passwordEncode | |
|
||
@Transactional | ||
public User join(@Valid User user) { | ||
checkDuplicatedByUsernameOrEmail(user.getUsername(), user.getEmail()); | ||
checkDuplicatedByUsername(user.getUsername()); | ||
checkDuplicatedByEmail(user.getEmail()); | ||
|
||
user.encodePassword(passwordEncoder); | ||
return userRepository.save(user); | ||
|
@@ -39,11 +41,55 @@ public User login(@Valid Email email, @Valid Password password) { | |
return user; | ||
} | ||
|
||
private void checkDuplicatedByUsernameOrEmail(Username username, Email email) { | ||
@Transactional(readOnly = true) | ||
public Optional<User> findById(Long userId) { | ||
return userRepository.findById(userId); | ||
} | ||
|
||
@Transactional | ||
public User update(UserUpdateModel updateUser, Long 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. 해당 부분의 내부 구현 수정이 필요할 듯 합니다. 유선으로 말씀드린 대로 하나의 메소드에서 너무 많은 구현이 되고 있고, 그중 VO 단으로 넘길 수 있는 구현부도 있어 고민해보면서 수정하시면 좋을 듯 합니다! 👍 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. 처음에는 null한 부분을 처리하는데 급급해서 지금처럼 한군데 모아놓고 로직을 설계했는데 다시보니까 update 메소드 자체에서 하고있는 역할이 너무 많더라구요 😢 |
||
User user = userRepository.findById(userId).orElseThrow(RuntimeException::new); | ||
|
||
updateUser.getUsername() | ||
.filter(username -> !user.getUsername().equals(username)) | ||
.ifPresent( | ||
username -> { | ||
checkDuplicatedByUsername(username); | ||
user.changeUsername(username); | ||
}); | ||
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. 유선으로 이야기했던 것처럼 UserService 자체를 변경해보았는데 확인한번 부탁드려요 :) |
||
updateUser.getEmail() | ||
.filter(email -> !user.getEmail().equals(email)) | ||
.ifPresent( | ||
email -> { | ||
checkDuplicatedByEmail(email); | ||
user.changeEmail(email); | ||
}); | ||
updateUser.getPassword() | ||
.filter( | ||
password -> !passwordEncoder | ||
.matches(password.getPassword(), user.getPassword().getPassword())) | ||
.ifPresent( | ||
password -> { | ||
user.changePassword(Password.encode(password, passwordEncoder)); | ||
}); | ||
updateUser.getBio() | ||
.filter(bio -> !user.getBio().equals(bio)) | ||
.ifPresent(user::changeBio); | ||
updateUser.getImage() | ||
.filter(image -> !user.getImage().equals(image)) | ||
.ifPresent(user::changeImage); | ||
|
||
return user; | ||
} | ||
|
||
private void checkDuplicatedByUsername(@Valid Username username) { | ||
findByUsername(username) | ||
.ifPresent(param -> { | ||
throw new RuntimeException("already user username"); | ||
DolphaGo marked this conversation as resolved.
Show resolved
Hide resolved
|
||
}); | ||
} | ||
|
||
private void checkDuplicatedByEmail(@Valid Email email) { | ||
findByEmail(email) | ||
.ifPresent(param -> { | ||
throw new RuntimeException("already user email"); | ||
DolphaGo marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,45 @@ | ||
package com.study.realworld.user.service.model; | ||
|
||
import com.study.realworld.user.domain.Email; | ||
import com.study.realworld.user.domain.Password; | ||
import com.study.realworld.user.domain.Username; | ||
import java.util.Optional; | ||
|
||
public class UserUpdateModel { | ||
|
||
private final Username username; | ||
private final Email email; | ||
private final Password password; | ||
private final String bio; | ||
private final String image; | ||
|
||
public UserUpdateModel(Username username, Email email, Password password, String bio, | ||
String image) { | ||
this.username = username; | ||
this.email = email; | ||
this.password = password; | ||
this.bio = bio; | ||
this.image = image; | ||
} | ||
|
||
public Optional<Username> getUsername() { | ||
return Optional.ofNullable(username); | ||
} | ||
|
||
public Optional<Email> getEmail() { | ||
return Optional.ofNullable(email); | ||
} | ||
|
||
public Optional<Password> getPassword() { | ||
return Optional.ofNullable(password); | ||
} | ||
|
||
public Optional<String> getBio() { | ||
return Optional.ofNullable(bio); | ||
} | ||
|
||
public Optional<String> getImage() { | ||
return Optional.ofNullable(image); | ||
} | ||
DolphaGo marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
} |
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.
위 2방법 중 하나로 하면 가독성이 좋을 것 같아요!
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.
ide 내 code style 적용이 잘못되어서 폭이 짧아 어쩔수없이 현재와 같이 설정했었는데 두번째 말씀드린 방법으로 수정하였습니다 :)