-
Notifications
You must be signed in to change notification settings - Fork 4
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
탈퇴 기능 추가 #378
탈퇴 기능 추가 #378
Changes from 8 commits
854c900
fa53da7
692abb0
4436508
ed70865
8635682
5aff1f4
e20b6e4
621590a
059f81c
6cbadb8
01cccdc
ea5d5f1
e63f83c
72ed2e1
94433a3
898cefb
797ceca
ba0ab60
75fd058
559c0b7
af0f74e
f70c71a
de24d84
662b4f7
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 |
---|---|---|
@@ -1,23 +1,26 @@ | ||
package com.ddang.ddang.authentication.application; | ||
|
||
import com.ddang.ddang.authentication.application.dto.TokenDto; | ||
import com.ddang.ddang.authentication.domain.exception.InvalidTokenException; | ||
import com.ddang.ddang.authentication.application.exception.InaccessibleWithdrawalException; | ||
import com.ddang.ddang.authentication.application.util.RandomNameGenerator; | ||
import com.ddang.ddang.authentication.domain.Oauth2UserInformationProviderComposite; | ||
import com.ddang.ddang.authentication.infrastructure.jwt.PrivateClaims; | ||
import com.ddang.ddang.authentication.domain.TokenDecoder; | ||
import com.ddang.ddang.authentication.domain.TokenEncoder; | ||
import com.ddang.ddang.authentication.domain.TokenType; | ||
import com.ddang.ddang.authentication.domain.dto.UserInformationDto; | ||
import com.ddang.ddang.authentication.domain.exception.InvalidTokenException; | ||
import com.ddang.ddang.authentication.infrastructure.jwt.PrivateClaims; | ||
import com.ddang.ddang.authentication.infrastructure.oauth2.OAuth2UserInformationProvider; | ||
import com.ddang.ddang.authentication.infrastructure.oauth2.Oauth2Type; | ||
import com.ddang.ddang.user.domain.User; | ||
import com.ddang.ddang.user.infrastructure.persistence.JpaUserRepository; | ||
import java.time.LocalDateTime; | ||
import java.util.Map; | ||
import lombok.RequiredArgsConstructor; | ||
import org.springframework.stereotype.Service; | ||
import org.springframework.transaction.annotation.Transactional; | ||
|
||
import java.time.LocalDateTime; | ||
import java.util.Map; | ||
|
||
@Service | ||
@Transactional(readOnly = true) | ||
@RequiredArgsConstructor | ||
|
@@ -31,7 +34,7 @@ public class AuthenticationService { | |
private final TokenDecoder tokenDecoder; | ||
|
||
@Transactional | ||
public TokenDto login(final Oauth2Type oauth2Type, final String oauth2AccessToken) { | ||
public TokenDto login(final Oauth2Type oauth2Type, final String oauth2AccessToken){ | ||
final OAuth2UserInformationProvider provider = providerComposite.findProvider(oauth2Type); | ||
final UserInformationDto userInformationDto = provider.findUserInformation(oauth2AccessToken); | ||
final User persistUser = findOrPersistUser(oauth2Type, userInformationDto); | ||
|
@@ -40,10 +43,10 @@ public TokenDto login(final Oauth2Type oauth2Type, final String oauth2AccessToke | |
} | ||
|
||
private User findOrPersistUser(final Oauth2Type oauth2Type, final UserInformationDto userInformationDto) { | ||
return userRepository.findByOauthId(userInformationDto.findUserId()) | ||
return userRepository.findByOauthIdAndDeletedIsFalse(userInformationDto.findUserId()) | ||
.orElseGet(() -> { | ||
final User user = User.builder() | ||
.name(oauth2Type.calculateNickname(userInformationDto)) | ||
.name(oauth2Type.calculateNickname(calculateRandomNumber())) | ||
.profileImage(null) | ||
.reliability(0.0d) | ||
.oauthId(userInformationDto.findUserId()) | ||
|
@@ -53,6 +56,20 @@ private User findOrPersistUser(final Oauth2Type oauth2Type, final UserInformatio | |
}); | ||
} | ||
|
||
private String calculateRandomNumber() { | ||
String name; | ||
|
||
do { | ||
name = RandomNameGenerator.generate(); | ||
} while (isAlreadyExist(name)); | ||
|
||
return name; | ||
} | ||
|
||
private boolean isAlreadyExist(final String name) { | ||
return userRepository.existsByNameEndingWith(name); | ||
} | ||
|
||
private TokenDto convertTokenDto(final User persistUser) { | ||
final String accessToken = tokenEncoder.encode( | ||
LocalDateTime.now(), | ||
|
@@ -86,4 +103,18 @@ public boolean validateToken(final String accessToken) { | |
return tokenDecoder.decode(TokenType.ACCESS, accessToken) | ||
.isPresent(); | ||
} | ||
|
||
@Transactional | ||
public void withdrawal( | ||
final Oauth2Type oauth2Type, | ||
final String oauth2AccessToken | ||
) throws InaccessibleWithdrawalException { | ||
final OAuth2UserInformationProvider provider = providerComposite.findProvider(oauth2Type); | ||
final UserInformationDto userInformationDto = provider.findUserInformation(oauth2AccessToken); | ||
final User user = userRepository.findByOauthIdAndDeletedIsFalse(userInformationDto.findUserId()) | ||
.orElseThrow(() -> new InaccessibleWithdrawalException("탈퇴에 대한 권한 없습니다.")); | ||
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. 저 몰래 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. 일단 저는 아닙니다 |
||
|
||
provider.unlinkUserBy(oauth2AccessToken, user.getOauthId()); | ||
user.withdrawal(); | ||
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. 근데 순서를 탈퇴 -> 탈퇴로 인한 사용자 레코드 DB 변경 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,8 @@ | ||
package com.ddang.ddang.authentication.application.exception; | ||
|
||
public class InaccessibleWithdrawalException extends IllegalArgumentException { | ||
|
||
public InaccessibleWithdrawalException(final String message) { | ||
super(message); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
package com.ddang.ddang.authentication.application.util; | ||
|
||
import java.util.Random; | ||
|
||
public class RandomNameGenerator { | ||
|
||
private static final int NAME_LENGTH = 10; | ||
|
||
private static final Random random = new Random(); | ||
|
||
private RandomNameGenerator() { | ||
} | ||
|
||
public static String generate() { | ||
StringBuilder name = new StringBuilder(); | ||
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. final이 누락되었습니다! 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. 해당 클래스에 전체적으로 final이 누락되었습니다 |
||
|
||
for (int i = 0; i < NAME_LENGTH; i++) { | ||
int digit = random.nextInt(10); | ||
name.append(digit); | ||
} | ||
|
||
return name.toString(); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -23,6 +23,7 @@ | |
public class User extends BaseTimeEntity { | ||
|
||
private static final boolean DELETED_STATUS = true; | ||
private static final String UNKOWN_NAME = "알 수 없음"; | ||
|
||
@Id | ||
@GeneratedValue(strategy = GenerationType.IDENTITY) | ||
|
@@ -35,7 +36,6 @@ public class User extends BaseTimeEntity { | |
|
||
private double reliability; | ||
|
||
@Column(unique = true) | ||
private String oauthId; | ||
|
||
@Column(name = "is_deleted") | ||
|
@@ -57,4 +57,14 @@ private User( | |
public void withdrawal() { | ||
this.deleted = DELETED_STATUS; | ||
} | ||
|
||
// TODO: 2023/09/15 [고민] getName() 메서드를 통해 가져오게 된다면 헷갈릴까요? | ||
// TODO: 2023/09/15 [고민] 출력을 위한 로직이 여기 있다면 어색할까요? 사실 귀찮아서 여기 두긴 했습니다. | ||
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.
|
||
public String getName() { | ||
if (isDeleted()) { | ||
return UNKOWN_NAME; | ||
} | ||
|
||
return name; | ||
} | ||
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 클래스에 unknownUser를 반환하는 static 메서드가 있고, 레포지토리에서 findById()시 deleted가 true면 unknownUser를 반환하는 형태를 생각하긴 했는데, 이렇게 해도 괜찮을 것 같아요. 어차피 dto에 넣을 때 getName() 하니까 로직상 문제는 없을 것 같고.. 출력을 위한 로직이 있는게 어색하다면 아예 delete할 때 유저 이름을 바꾸는 것은 별로일까요? 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 |
---|---|---|
|
@@ -7,7 +7,6 @@ | |
import com.ddang.ddang.user.presentation.dto.ReadUserResponse; | ||
import lombok.RequiredArgsConstructor; | ||
import org.springframework.http.ResponseEntity; | ||
import org.springframework.web.bind.annotation.DeleteMapping; | ||
import org.springframework.web.bind.annotation.GetMapping; | ||
import org.springframework.web.bind.annotation.RequestMapping; | ||
import org.springframework.web.bind.annotation.RestController; | ||
|
@@ -26,12 +25,4 @@ public ResponseEntity<ReadUserResponse> readById(@AuthenticateUser final Authent | |
|
||
return ResponseEntity.ok(response); | ||
} | ||
|
||
@DeleteMapping("/withdrawal") | ||
public ResponseEntity<Void> delete(@AuthenticateUser final AuthenticationUserInfo userInfo) { | ||
Comment on lines
-51
to
-52
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.deleteById(userInfo.userId()); | ||
|
||
return ResponseEntity.noContent() | ||
.build(); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
ALTER TABLE users DROP INDEX uq_oauth_id; |
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.
do-while문은 가독성에 좋지 않다고 생각합니다
그리고 calculateRandomNumber()라는 메서드 네이밍에 비해 반환하는건 name이라 좀 애매한 것 같습니다
name도 값을 초기화하지 않는건 위험하다고 생각합니다
하지만 이건 다른 분들의 의견도 궁금하네요
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.
대충 전 이렇게 할 것 같기는 합니다