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

닉네임 랜덤 숫자 추가 및 수정 시 닉네임 중복 체크 #565

Merged
merged 4 commits into from
Sep 15, 2023

Conversation

mcodnjs
Copy link
Collaborator

@mcodnjs mcodnjs commented Sep 12, 2023

📄 Summary

#564

🙋🏻 More

여러분 고민이 많아요 .. 같이 고민해보려고 .. PR 먼저 올려요 ..
고민되는 부분은 커멘트에 남겨놓을게요 .. 다들 꼭 답글 달아줘요 ..

@github-actions
Copy link

github-actions bot commented Sep 12, 2023

📝 Jacoco Test Coverage

Total Project Coverage 81.74% 🍏
File Coverage [75.77%]
ExceptionCode.java 100% 🍏
Member.java 79.31%
MemberService.java 77.61%
AuthService.java 18.78%

Comment on lines 71 to 70
private String generateRandomNumbers() {
final StringBuilder randomNumbers = new StringBuilder();
for (int i = 0; i < 4; i++) {
final int randomNumber = random.nextInt(10);
randomNumbers.append(randomNumber);
}
return randomNumbers.toString();
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

4자리의 RandomNumber를 생성하는 로직을

  • Member 객체를 생성할 때 넣을 것인지 (Member 정팩메 안에 들어가겠죠 구럼?)
  • 혹은 또다른 유틸성 클래스에서 생성하고 static 메서드로 불러올 것인지
  • 지금 이대로 갈지

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Random 인스턴스를 static 하게 관리하는게 좋을거 같은데 어떻게 관리하면 좋을지?

Copy link
Member

Choose a reason for hiding this comment

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

Math.random 쓰면 안되나요?

(int)(Math.random() * 8999) + 1000 (1000 ~ 9999)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

맞아요! 저도 이 방법을 인지하고 있었는데, 어떤 로직인지 좀 파악하기 어렵다 ?? 싶었어요
글고 그냥 0492 이런 값도 하고 싶었어서 .. 근데 그 방법도 좋아요 ㅎㅎ

Copy link
Collaborator

Choose a reason for hiding this comment

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

ㅎㅎ 혹은 또다른 유틸성 클래스에서 생성하고 static 메서드로 불러올 것인지 좋아요. 랜덤 숫자 모음 생성하는 로직 유틸로 분리하는 거 좋아용

Copy link
Collaborator

Choose a reason for hiding this comment

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

String.format("%04d", random.nextInt(10000)) 이건 어떤가요? 이러면 라온이 말한 0492이런 숫자도 나오는데

Copy link
Collaborator Author

@mcodnjs mcodnjs Sep 14, 2023

Choose a reason for hiding this comment

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

random 객체를 사용한다면 유틸성 클래스로 분리하려했는데, 아래와 같이 해결가능해서 그냥 AuthService에 넣었습니다

private String generateRandomFourDigitCode() {
    final int randomNumber = (int) (Math.random() * 10000);
    return String.format("%04d", randomNumber);
}

Comment on lines 59 to 65
private Member createMember(final String socialLoginId, final String nickname, final String imageUrl) {
int tryCount = 0;
while (tryCount < 5) {
final String nicknameWithRandomNumber = nickname + generateRandomNumbers();
if (!memberRepository.existsByNickname(nicknameWithRandomNumber)) {
return memberRepository.save(new Member(socialLoginId, nicknameWithRandomNumber, imageUrl));
}
tryCount += 1;
}
throw new BadRequestException(FAIL_TO_GENERATE_RANDOM_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.

멤버 생성할 때, 난수를 넣는다고 해도 중복의 가능성이 있을 수 있는데, (중복될 가능성이 낮기도함)

  • 무시할지 -> 무시한다면, DB에서 exist로 검사 안하고 unique 제약조건으로 인해 오류가 발생할 수 있음
  • 지금처럼 tryCount로 존재하는지 확인할지

Copy link
Collaborator

Choose a reason for hiding this comment

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

지금처럼 tryCount로 존재하는지 확인하는 거 좋아요~ ~

Copy link
Collaborator

Choose a reason for hiding this comment

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

존재하는지 확인 필요하다고 생각합니다. 중복이 허용된다면 난수를 붙이는 이유가 퇴색된다고 느껴져서요!

Copy link
Collaborator

Choose a reason for hiding this comment

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

저도 이오와 같은 이유로 tryCount로 확인하는게 좋습니다.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

이 부분도 tryCount로 확인하는걸로 결정!
일단 5로 설정했는데 더 늘리는게 좋을거 같다? 싶으면 말씀 주십셔

Copy link
Member

@jjongwa jjongwa left a comment

Choose a reason for hiding this comment

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

네 해드렸습니다~

Comment on lines 60 to 63
int tryCount = 0;
while (tryCount < 5) {
final String nicknameWithRandomNumber = nickname + generateRandomNumbers();
if (!memberRepository.existsByNickname(nicknameWithRandomNumber)) {
return memberRepository.save(new Member(socialLoginId, nicknameWithRandomNumber, imageUrl));
}
tryCount += 1;
}
Copy link
Member

Choose a reason for hiding this comment

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

멤버 닉네임 존재하는지 확인
-> 없으면 바로 save
없으면 4자리 랜덤값 붙여서
존재하는지 확인
...
의 루프 도는 식으로 하는건 어떤가요

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

저는 일관성 있게 모두 4자리 붙이는 편이 좋다인데 다른 사람들 의견도 들어봅시당

Copy link
Collaborator

Choose a reason for hiding this comment

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

저도 전부 붙인다!

Copy link
Collaborator

Choose a reason for hiding this comment

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

저도 일관성있게 모두 4자리 한표 하겠습니다

Copy link
Collaborator

Choose a reason for hiding this comment

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

저는 모두 붙이는 것 보다 있는지 확인하고 붙이는게 좋을 것 같습니다. 유니크한 닉네임이 맞는데 번호가 붙으면 불필요하게 번호가 붙는거 같아서

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

그럼 다수결로 일관성있게 4자리로 갑니다 🚀

Comment on lines 71 to 70
private String generateRandomNumbers() {
final StringBuilder randomNumbers = new StringBuilder();
for (int i = 0; i < 4; i++) {
final int randomNumber = random.nextInt(10);
randomNumbers.append(randomNumber);
}
return randomNumbers.toString();
}
Copy link
Member

Choose a reason for hiding this comment

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

Math.random 쓰면 안되나요?

(int)(Math.random() * 8999) + 1000 (1000 ~ 9999)

Copy link
Collaborator

@hgo641 hgo641 left a comment

Choose a reason for hiding this comment

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

Speed~

Comment on lines 60 to 63
int tryCount = 0;
while (tryCount < 5) {
final String nicknameWithRandomNumber = nickname + generateRandomNumbers();
if (!memberRepository.existsByNickname(nicknameWithRandomNumber)) {
return memberRepository.save(new Member(socialLoginId, nicknameWithRandomNumber, imageUrl));
}
tryCount += 1;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

저도 전부 붙인다!

Comment on lines 59 to 65
private Member createMember(final String socialLoginId, final String nickname, final String imageUrl) {
int tryCount = 0;
while (tryCount < 5) {
final String nicknameWithRandomNumber = nickname + generateRandomNumbers();
if (!memberRepository.existsByNickname(nicknameWithRandomNumber)) {
return memberRepository.save(new Member(socialLoginId, nicknameWithRandomNumber, imageUrl));
}
tryCount += 1;
}
throw new BadRequestException(FAIL_TO_GENERATE_RANDOM_NICKNAME);
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

지금처럼 tryCount로 존재하는지 확인하는 거 좋아요~ ~

Comment on lines 71 to 70
private String generateRandomNumbers() {
final StringBuilder randomNumbers = new StringBuilder();
for (int i = 0; i < 4; i++) {
final int randomNumber = random.nextInt(10);
randomNumbers.append(randomNumber);
}
return randomNumbers.toString();
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

ㅎㅎ 혹은 또다른 유틸성 클래스에서 생성하고 static 메서드로 불러올 것인지 좋아요. 랜덤 숫자 모음 생성하는 로직 유틸로 분리하는 거 좋아용

Copy link
Collaborator Author

@mcodnjs mcodnjs left a comment

Choose a reason for hiding this comment

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

로컬에서 테스트 완 ~
image

Comment on lines 60 to 63
int tryCount = 0;
while (tryCount < 5) {
final String nicknameWithRandomNumber = nickname + generateRandomNumbers();
if (!memberRepository.existsByNickname(nicknameWithRandomNumber)) {
return memberRepository.save(new Member(socialLoginId, nicknameWithRandomNumber, imageUrl));
}
tryCount += 1;
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

그럼 다수결로 일관성있게 4자리로 갑니다 🚀

Comment on lines 59 to 65
private Member createMember(final String socialLoginId, final String nickname, final String imageUrl) {
int tryCount = 0;
while (tryCount < 5) {
final String nicknameWithRandomNumber = nickname + generateRandomNumbers();
if (!memberRepository.existsByNickname(nicknameWithRandomNumber)) {
return memberRepository.save(new Member(socialLoginId, nicknameWithRandomNumber, imageUrl));
}
tryCount += 1;
}
throw new BadRequestException(FAIL_TO_GENERATE_RANDOM_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.

이 부분도 tryCount로 확인하는걸로 결정!
일단 5로 설정했는데 더 늘리는게 좋을거 같다? 싶으면 말씀 주십셔

Comment on lines 71 to 70
private String generateRandomNumbers() {
final StringBuilder randomNumbers = new StringBuilder();
for (int i = 0; i < 4; i++) {
final int randomNumber = random.nextInt(10);
randomNumbers.append(randomNumber);
}
return randomNumbers.toString();
}
Copy link
Collaborator Author

@mcodnjs mcodnjs Sep 14, 2023

Choose a reason for hiding this comment

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

random 객체를 사용한다면 유틸성 클래스로 분리하려했는데, 아래와 같이 해결가능해서 그냥 AuthService에 넣었습니다

private String generateRandomFourDigitCode() {
    final int randomNumber = (int) (Math.random() * 10000);
    return String.format("%04d", randomNumber);
}

@mcodnjs mcodnjs merged commit 429cdd1 into develop Sep 15, 2023
1 check passed
waterricecake pushed a commit that referenced this pull request Sep 15, 2023
* feat: member의 nickname에 unique 제약조건 추가

* feat: 회원가입 시 닉네임 랜덤넘버 추가

* feat: 닉네임 수정 시 중복 체크 로직 추가

* refactor: 랜덤 생성 로직 수정 및 상수화
waterricecake pushed a commit that referenced this pull request Sep 15, 2023
* feat: member의 nickname에 unique 제약조건 추가

* feat: 회원가입 시 닉네임 랜덤넘버 추가

* feat: 닉네임 수정 시 중복 체크 로직 추가

* refactor: 랜덤 생성 로직 수정 및 상수화
waterricecake pushed a commit that referenced this pull request Sep 15, 2023
* feat: member의 nickname에 unique 제약조건 추가

* feat: 회원가입 시 닉네임 랜덤넘버 추가

* feat: 닉네임 수정 시 중복 체크 로직 추가

* refactor: 랜덤 생성 로직 수정 및 상수화
waterricecake pushed a commit that referenced this pull request Sep 15, 2023
* feat: member의 nickname에 unique 제약조건 추가

* feat: 회원가입 시 닉네임 랜덤넘버 추가

* feat: 닉네임 수정 시 중복 체크 로직 추가

* refactor: 랜덤 생성 로직 수정 및 상수화
jjongwa pushed a commit that referenced this pull request Sep 15, 2023
* feat: member의 nickname에 unique 제약조건 추가

* feat: 회원가입 시 닉네임 랜덤넘버 추가

* feat: 닉네임 수정 시 중복 체크 로직 추가

* refactor: 랜덤 생성 로직 수정 및 상수화
@LJW25 LJW25 deleted the feature/#564-nickname branch September 19, 2023 06:48
jjongwa added a commit that referenced this pull request Sep 20, 2023
* city 패키지 위치 변경 (#527)

* ExpensePage 리팩토링 (#533)

* refactor: 상수화

* refactor: ExpensePage 관련 코드 리팩토링

* refactor: 오타 수정

* refactor: createTrip 테스트 코드 수정

* TripPage, TripEditPage 리팩토링 (#537)

* fix: TripEditPage 데이터 패칭 waterfall 문제 해결

* refactor: useQuery 옵션 defaultOptions로 옮기기

* fix: Header cursor pointer 문제 해결

* refactor: Footer 수정

* refactor: 회원가입, 로그인 버튼 하나로 합치기

* refactor: props로 넘겨주는 대신 TripInformation에서 직접 데이터 가져오기

* refactor: TripInformation 수정 버튼 네이밍 수정에서 완료로 변경

* refactor: TripItem 제목 스타일링 수정

* refactor: 모바일일때 image carousel navigation 보이게 변경

* refactor: 수정, 삭제 버튼 더보기 메뉴에서 보여주는 대신 아이콘으로 변경

* refactor: 기타 탭에서 아이템 생성 시 디폴트 카테고리 기타로 설정

* refactor: 이미지 용량 10mb 이상으로 인한 에러 발생 시 상황에 맞는 에러 메세지 보여주기

* refactor: 아이템 추가할 때 제목에 공백 입력 후 아이템 추가 시 에러 메세지 보여주기

* refactor: TripItemList 컴포넌트 수정

* fix: 테스트 코드 수정

* 삭제 컨펌 경고 모달 추가 (#540)

* refactor: 여행 삭제 시 삭제 컨펌 모달 보여주기

* refactor: 여행 아이템 삭제 시 삭제 컨펌 모달 보여주기

* MyPage 리팩토링 (#541)

* refactor: 탈퇴 버튼 스타일링 수정

* refactor: 계정 탈퇴 클릭 시 컨펌 모달 보여주기

* 금액순으로 카테고리 정렬 구현 (#531)

* feat: 금액순으로 카테고리 정렬 구현

* style : 코드 컨밴션 적용

* 번들 사이즈 최적화 (#546)

* chore: webpack bundle analyzer 설치

* chore: compress webpack plugin 설치

* refactor: 이미지 파일 최적화

* refactor: tree shaking 추가

* refactor: tsconfig.json module commonjs에서 esnext로 변경

* refactor: lazy loading을 활용한 코드 스플리팅

* refactor: IntroPage 이미지 png에서 jpg으로 변경

* refactor: 탈퇴 버튼 수정

* fix: npm run build 시 오래 걸리는 문제 해결 (#549)

* 모바일 환경에서 StarRatingInput 호버 문제 수정 (#550)

* refactor: 디자인시스템 1.3.0 버전업

* refactor: 모바일 환경에서 StarRatingInput 호버 문제 수정

* FLYWAY 적용 (#552)

* chore: flyway 적용

* chore: submodule 변경 적용

* chore: submodule 변경 적용

* 모바일 버전에서 지도 볼 수 있는 기능 (#544)

* feat: 모바일에서 지도보기 버튼 추가, 지도 보는 기능

* refactor: 트립페이지 모바일 버전 컴포넌트 분리

* feat: 모바일에서 지도볼때도 daylog tab보이도록 수정

* refactor: 버튼 스타일 수정, 패딩 수정

* chore: 리베이스로 인한 버그 수정

* 비공유 상태일 시 code null값 반영 및 sharedTrip soft delete 추가 (#530)

* feat: soft delete 추가

* refactor: 메서드 반환 타입 변경

* style : 코드 컨밴션 적용

* 토글버튼 부활, UI관련 수정  (#557)

* feat:데이로그 아이템 토글 기능 부활

* feat:이미지 클릭시 확대 모달창 여는 기능

* refactor: 지도 로딩시 스피너 중앙에 위치

* refactor: 이미지 사이즈 조정 로직 훅으로 분리

* refactor: 제목길이 25로 늘림

* refactor: api통신 횟수 조정, queryClient 함수 분리

* refactor: 여행설명, 아이템메모 사용자입력 줄바꿈 적용

* sharedtrip 변경사항 flyway script 작성 (#556)

* feat: sharedtrip 변경사항 flyway script 작성

* feat: sharedtrip 변경사항 flyway script 작성

* refactor: 스크립트 명 변경

* TripCity 삭제 로직 추가 (#529)

* refactor: 테스트코드 패키지 변경 (#563)

* webpack 최적화 및 common, prod, dev 파일 분리 (#560)

* refactor: webpack 최적화, dev/prod파일 분리

* refactor: webpack serve, build script수정

* refactor: webpack파일 분리로 인한 cypress 자동화 yml 수정

* 접속자별 접근 권한 검증 구현 (#571)

* feat: AuthArgumentResolver 로직 추가

Co-authored-by: mcodnjs <codnjs8989@gmail.com>

* feat: MemberOnly 어노테이션 구현

Co-authored-by: mcodnjs <codnjs8989@gmail.com>

* feat: MemberOnly 어노테이션 적용

Co-authored-by: mcodnjs <codnjs8989@gmail.com>

* fix: cookie null 예외처리 추가

Co-authored-by: mcodnjs <codnjs8989@gmail.com>

---------

Co-authored-by: mcodnjs <codnjs8989@gmail.com>

* 닉네임 랜덤 숫자 추가 및 수정 시 닉네임 중복 체크 (#565)

* feat: member의 nickname에 unique 제약조건 추가

* feat: 회원가입 시 닉네임 랜덤넘버 추가

* feat: 닉네임 수정 시 중복 체크 로직 추가

* refactor: 랜덤 생성 로직 수정 및 상수화

* Trip, Like, PublishedTrip 테이블 생성 (#569)

* feat: like 엔티티 구현

* feat: publishedTrip 엔티티 구현

* feat: flyway 마이그레이션 스크립트 작성

* feat: 기본 생성시 unpublished 추가

* style: 코드 컨벤션 적용

* style: 코드 컨벤션 적용

* refactor: flyway 스크립트 버전 수정

* 이미지 압축 (#570)

* chore: browser image compression 설치

* feat: 이미지 최적화 추가

* refactor: preview 이미지 보여주는 방법 수정

* 마이페이지 수정 시 닉네임 변경되지 않은 경우 예외 발생 버그 해결 (#576)

* fix: 닉네임 변경 시에만 중복 닉네임 검사 로직 추가

* refactor: 멤버 수정 날짜 업데이트 로직 추가

* 여행 아이템 사진 확대해서 볼 수 있는 기능 추가 (#580)

* chore: 행록 디자인 시스템 버전 업데이트

* feat: 이미지 확대 모달 구현

* 성능 최적화 - 사용자 이미지 아이콘 width, height 추가 (#586)

* fix: 사용자 이미지 아이콘 width, height 추가

* chore: compression 관련 코드 삭제

* 경비 공유 페이지 생성 (#559)

* feat: 공유된 페이지에 가계부 버튼 노출

* fix: 메모, 상세설명 줄바꿈 되도록 수정

* feat: 가계부 공부 페이지 api 구현

* feat: 공유 가계부에서 공유 트립으로 이동

* refactor:버그 수정, 여행추가 버튼 위치 변경

* refactor:지도 확대 가능, 지도 클릭 불가능 기능

* feat:공유페이지 모바일 버전 적용

* refactor: 파일 명 변경

* refactor: 로그인시 메인이 아닌 전 페이지로 이동

* fix: 비로그인 시 공유경비 페이지 못보는 오류 해결

* refactor:디자인 변경, 변수명 리네임

* refactor: 여행정보사진수정 커스텀 훅 multi->single로 변경

* refactor:버그로 인한 이미지 업로드 롤백

* fix:CF배포로 인한 api 주소 변경 (#588)

* fix:dev서버에서 백서버로 api요청 안가는 오류 해결 ->axios base url환경변수 변경 (#594)

* 공유된 여행 가계부 조회 기능 추가 (#595)

* refactor: sharedCode index 설정 추가 (#591)

Co-authored-by: hgo641 <hgo641@gmail.com>
Co-authored-by: jjongwa <troas96@naver.com>

* feat: cache-control 메타데이터 추가 (#597)

* refactor: cache-control 메타데이터 값 변경 (#600)

* cors origin과 method 추가 (#603)

* RefreshToken memberId Unique 해제 (#606)

---------

Co-authored-by: Ashley Heo <51967731+ashleysyheo@users.noreply.github.com>
Co-authored-by: waterricecake <91263263+waterricecake@users.noreply.github.com>
Co-authored-by: 임우찬 <dlaxodud1217@naver.com>
Co-authored-by: 이지우 <49433615+LJW25@users.noreply.github.com>
Co-authored-by: Dahye Yun <102305630+Dahyeeee@users.noreply.github.com>
Co-authored-by: mcodnjs <codnjs8989@gmail.com>
Co-authored-by: Chaewon Moon <64852591+mcodnjs@users.noreply.github.com>
Co-authored-by: hgo641 <hgo641@gmail.com>
jjongwa pushed a commit that referenced this pull request Oct 8, 2023
* feat: member의 nickname에 unique 제약조건 추가

* feat: 회원가입 시 닉네임 랜덤넘버 추가

* feat: 닉네임 수정 시 중복 체크 로직 추가

* refactor: 랜덤 생성 로직 수정 및 상수화
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants