Skip to content
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

Feat/#9 닉네임 중복 체크 기능 구현 #40

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ public ResponseEntity<AuthTokens> renew(@Login AuthInfo authInfo) {
@PostMapping("/logout")
public ResponseEntity<Void> logout(@Login @RequestBody AuthInfo authInfo) {
authService.logout(authInfo.userId());
return ResponseEntity.noContent().build();
return ResponseEntity.ok().build();
}

}
14 changes: 11 additions & 3 deletions src/main/java/coffeemeet/server/auth/service/AuthService.java
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,12 @@ public String getAuthCodeRequestUrl(OAuthProvider oAuthProvider) {
public AuthTokens signup(SignupRequest request) {
OAuthInfoResponse response = oauthMemberClientComposite.fetch(request.oAuthProvider(),
request.authCode());
checkDuplicateUser(response);
checkDuplicatedUser(response);
String profileImage = checkProfileImage(response.profileImage());
String nickname = checkDuplicatedNickname(request.nickname());

User user = new User(new OAuthInfo(response.oAuthProvider(), response.oAuthProviderId()),
Profile.builder().name(response.name()).nickname(request.nickname()).email(response.email())
Profile.builder().name(response.name()).nickname(nickname).email(response.email())
.profileImageUrl(profileImage).birth(response.birth()).build());

User newUser = userRepository.save(user);
Expand Down Expand Up @@ -80,7 +81,7 @@ public void logout(Long userId) {
refreshTokenRepository.deleteById(userId);
}

private void checkDuplicateUser(OAuthInfoResponse response) {
private void checkDuplicatedUser(OAuthInfoResponse response) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이건 AuthService보다 UserService에 두는게 어때요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

그러면 AuthService가 너무 많은 클래스에 의존하게 되어 AuthService에 해당 로직을 넣었는데, 그만큼 많은 책임을 가지고 있다는 의미같아서 말씀하신대로 해당 메서드는 UserService에 두고, 추후에 AuthService의 책임을 분리하려고 합니다 ! 🙆🏻‍♀️🫡

if (userRepository.existsUserByOauthInfo_oauthProviderAndOauthInfo_oauthProviderId(
Copy link
Collaborator

@smartandhandsome smartandhandsome Oct 24, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이거 OAuthInfo 에 OAuthProvider oauthProvider, String oauthProviderId 이렇게 두 개만 있고 조회도 그 두 개로 해서 userRepository.existsUserByOauthInfo 까지맨 해도 될 것 같은데 안되나용?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

메서드명을 단순화하면 스프링 데이터 JPA가 어떤 필드를 기준으로 사용자를 조회할지 알 수 없어 해당 이름은 줄이기 어렵습니다 !
너무 길긴하지만,, 추후에 QueryDSL로 개선하려고 일단은 놔뒀습니다 ! 🫡

response.oAuthProvider(), response.oAuthProviderId())) {
throw new IllegalArgumentException(ALREADY_REGISTERED_MESSAGE);
Expand All @@ -94,6 +95,13 @@ private String checkProfileImage(String profileImage) {
return profileImage;
}

private String checkDuplicatedNickname(String nickname) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

굳이 nickname을 반환할 필요 있나요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

checkDuplicatedUser 반환값이 void라 검증 메서드들을 void로 통일시키는게 좋을 것 같네요 ! 🙆🏻‍♀️🙆🏻‍♀️

if (userRepository.findUserByProfileNickname(nickname).isPresent()) {
throw new IllegalArgumentException("중복된 닉네임입니다.");
}
return nickname;
}

Comment on lines +98 to +104
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 부분 UserService에 checkDuplicatedNickname 랑 중복인데 UserService를 필드로 갖고 쓰는거 어때요?

private void generateInterests(SignupRequest request, User newUser) {
List<Interest> interests = new ArrayList<>();
for (Keyword value : request.keywords()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import org.springframework.web.bind.annotation.PathVariable;
import org.springframework.web.bind.annotation.RequestBody;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RequestParam;
import org.springframework.web.bind.annotation.RestController;

@RestController
Expand All @@ -38,14 +39,20 @@ public ResponseEntity<MyProfileResponse> findMyProfile(@Login AuthInfo authInfo)
public ResponseEntity<Void> updateProfileImage(@Login AuthInfo authInfo,
@Valid @RequestBody UpdateProfileImageUrlRequest request) {
userService.updateProfileImage(authInfo.userId(), request.profileImageUrl());
return ResponseEntity.noContent().build();
return ResponseEntity.ok().build();
}

@PatchMapping("/me")
public ResponseEntity<Void> updateProfileInfo(@Login AuthInfo authInfo,
@Valid @RequestBody UpdateProfileRequest request) {
userService.updateProfileInfo(authInfo.userId(), request.nickname(), request.interests());
return ResponseEntity.noContent().build();
return ResponseEntity.ok().build();
}

@GetMapping("/duplicate")
public ResponseEntity<Void> checkDuplicatedNickname(@RequestParam String nickname) {
userService.checkDuplicatedNickname(nickname);
return ResponseEntity.ok().build();
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -66,10 +66,18 @@ public void updateProfileImage(Long userId, String profileImageUrl) {
@Transactional
public void updateProfileInfo(Long userId, String nickname, List<Keyword> interests) {
User user = getUserById(userId);
checkDuplicatedNickname(nickname);

user.updateNickname(nickname);
interestService.updateInterests(userId, interests);
}

public void checkDuplicatedNickname(String nickname) {
if (userRepository.findUserByProfileNickname(nickname).isPresent()) {
throw new IllegalArgumentException("이미 존재하는 닉네임입니다.");
}
}

private User getUserById(Long userId) {
return userRepository.findById(userId)
.orElseThrow(IllegalArgumentException::new);
Expand Down