Skip to content

Feat: 비밀번호 초기화 시 인증코드 전송 로직 분리#247#249

Merged
rlagkswn00 merged 15 commits intodevelopfrom
feat/verify-for-reset-password-#247
Apr 24, 2025
Merged

Feat: 비밀번호 초기화 시 인증코드 전송 로직 분리#247#249
rlagkswn00 merged 15 commits intodevelopfrom
feat/verify-for-reset-password-#247

Conversation

@rlagkswn00
Copy link
Member

구현

  • 인증 이메일 전송 시 password-resetsignup으로 구분하여 회원가입 시와 비밀번호 초기화 시 사용하는 API를 분리했습니다.
  • RandomGenerator를 수정하여 VerificationCodeGenerator와 NicknameGenerator를 구분하여 용도에 맞는 생성 클래스를 추가하였습니다.
  • Generator클래스의 테스트 코드를 추가했습니다.

@rlagkswn00 rlagkswn00 self-assigned this Apr 21, 2025
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR separates email verification endpoints for user signup and password reset, introduces new generator classes for verification codes and nicknames, and updates corresponding tests.

  • Updated email API endpoints and test cases to distinguish between signup and password reset verification flows.
  • Introduced VerificationCodeGenerator and NicknameGenerator to replace legacy random generation logic.
  • Updated service, acceptance tests, and API controllers to accommodate these changes.

Reviewed Changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
EmailCommandUseCaseTest.java Updated test to call sendSignupVerificationEmail instead of sendVerificationEmail.
VerificationCodeGeneratorTest.java Added tests validating the new six-digit verification code generation.
RandomGeneratorTest.java Reused tests with minor adjustments for generatePositiveNumber logic.
NicknameGeneratorTest.java Added tests to verify nickname generation using the new NicknameGenerator.
UserAcceptanceTest.java Updated email verification steps to call the new signup endpoint.
EmailStep.java Renamed methods to differentiate between signup and password reset email requests.
EmailAcceptanceTest.java Refactored tests to cover both signup and password reset email verification scenarios.
UserCommandService.java Changed nickname generation to use NicknameGenerator.generateNickname.
EmailCommandService.java Separated signup and password reset email verification processing and added input validation.
EmailCommandUseCase.java Updated interface with new method signatures for email verification.
EmailCommandApiV2.java Added a new endpoint for password reset email verification with JWT validation.
VerificationCodeGenerator.java Introduced a new generator class for verification codes using RandomGenerator.
RandomGenerator.java Removed nickname generation logic; added generatePositiveNumber for controlled random numbers.
NicknameGenerator.java Added a new generator class for nicknames based on a random index and a fixed prefix array.

}

private static String pickRandomNicknamePrefix() {
int index = RandomGenerator.generatePositiveNumber(NICKNAME_PREFIXES.length);
Copy link

Copilot AI Apr 21, 2025

Choose a reason for hiding this comment

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

Using generatePositiveNumber to select an array index results in a range of [1, length-1] for bounds greater than 1, which skips the first prefix and may lead to an off-by-one error. Consider using a 0-indexed random generator to correctly cover the full array range.

Copilot uses AI. Check for mistakes.
@github-actions
Copy link

github-actions bot commented Apr 21, 2025

Unit Test Results

  57 files  +  3    57 suites  +3   1m 9s ⏱️ +52s
307 tests +10  304 ✔️ +121  3 💤 ±0  0  - 111 
310 runs  +10  307 ✔️ +121  3 💤 ±0  0  - 111 

Results for commit 4cba81e. ± Comparison against base commit 6493d9d.

♻️ This comment has been updated with latest results.

Copy link
Member

@zbqmgldjfh zbqmgldjfh left a comment

Choose a reason for hiding this comment

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

2부분 정도 고민해볼만 한 부분 남겨두었습니다~~

String nickname = "";
do {
nickname = RandomGenerator.generateRandomNickname(6);
nickname = NicknameGenerator.generateNickname();
Copy link
Member

Choose a reason for hiding this comment

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

지금의 방식은 계속 한번씩 생성하면서 확인하는 방식인데, 혹시 한번에 한 5개 정도를 생성하고, 존제하지 않는 값만 리턴받아 그중 하나를 사용하면 어떨가요?

n번의 쿼리가 발생할 수 있는 지점인것 같아, 조금 더 사이즈를 크게 한번에 물어봐도 될것 같아요.

(DB : A, B)
닉네임 랜덤 생성 -> (A, B, C, D, E) -> DB 에 subquery로 not exsist만 list로 반환하도록
DB가 (C, D, E) 반환 -> 이중 암거나 사용

Copy link
Member

@zbqmgldjfh zbqmgldjfh Apr 22, 2025

Choose a reason for hiding this comment

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

대충 다음같은 느낌으로 QueryDsl로 동적 쿼리 만들어 주면 될듯요!

@Query("SELECT n FROM Nickname n WHERE n.nickname NOT IN (SELECT u.nickname FROM User u WHERE u.nickname IN :nicknames)")
List<String> findAvailableNicknames(@Param("nicknames") List<String> nicknames);

전체 코드는 대충 다음같은 느낌

private String createNickname() {
    final int BATCH_SIZE = 10;

    while (true) {
        List<String> candidates = new ArrayList<>();
        for (int i = 0; i < BATCH_SIZE; i++) {
            candidates.add(NicknameGenerator.generateNickname());
        }

        // DB에 존재하지 않는 닉네임 목록 조회
        List<String> availableNicknames = userQueryPort.findNonExistingNicknames(candidates);

        // 하나라도 존재하지 않는 닉네임이 있다면 반환
        if (!availableNicknames.isEmpty()) {
            return availableNicknames.get(0);
        }
        // 없으면 다음 배치 시도
    }
}

Copy link
Member Author

Choose a reason for hiding this comment

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

당연한 부분인데 확률이 낮다고 생각해서 너무 가볍게 생각했던거 같습니다!
말씀해주신대로 미리 일정 개수 정해두고 한번에 체크해보는게 더 좋을거 같습니다!
반영해서 수정해볼게요~~!!😀

assertThat(nickname).isNotNull();
assertThat(nickname).isNotEmpty();

boolean isMatch = false;
Copy link
Member

Choose a reason for hiding this comment

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

정규 표현식 사용은 어떤가요?

// 접두사 목록을 정규식으로 결합
String regex = "^(Cool|Happy|Smart)\d{6}$";
assertThat(nickname).matches(regex);

Copy link
Member Author

Choose a reason for hiding this comment

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

너무 좋은 방법인거같습니닷
수정해서 올려보겠습니다!

@rlagkswn00 rlagkswn00 changed the title Feat/verify for reset password #247 Feat: 비밀번호 초기화 시 인증코드 전송 로직 분리#247 Apr 22, 2025
@sonarqubecloud
Copy link

@rlagkswn00
Copy link
Member Author

rlagkswn00 commented Apr 23, 2025

변경사항

  • 테스트 구조 정규화로 적용했습니다.
  • 닉네임 생성시 5개까지 생성 후 한번에 DB에서 조회하는 방법으로 처리했습니다. 생성된 닉네임이 존재하는 것들만 불러오고 이를 필터링하도록 구현했습니다.

추가 수정

  • testcontainer 생성 시 Chromadb에서 heartbeat하다가 타임아웃 발생하는 현상으로 인해 통합테스트 구성이 되지 않아 테스트 오류가 발생했습니다.
  • 찾다보니 크로마DB를 latest버전으로 사용하면서 heartbeat api 경로가 /api/v1/heartbeat에서 /api/v2/heartbeat로 수정된걸로 이해해서 이 부분을 수정했습니다.(FastAPI init할 때 v1, v2둘다 setup하는거 같아서 안될 이유는 없는거같은데 CI에서만 왜 터지는지는 잘... 몰겠슴다)
  • 추후에 v1가 삭제될 예정으로 확인해서 testcontainers에서 지원하는 내용이 업데이트 되는게 확인되면 testcontainers 버전 자체를 올릴 필요가 있을거 같습니다.

관련

Copy link
Member

@zbqmgldjfh zbqmgldjfh left a comment

Choose a reason for hiding this comment

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

LGTM!~~


static {
chroma = new ChromaDBContainer(DockerImageName.parse("chromadb/chroma:latest"))
.waitingFor(Wait.forHttp("/api/v2/heartbeat"))
Copy link
Member

Choose a reason for hiding this comment

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

오 수정 감사합니다 굳굳~!

Copy link
Member Author

@rlagkswn00 rlagkswn00 Apr 24, 2025

Choose a reason for hiding this comment

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

chroma 1.0.0 버전업에 따른 testcontainers 수정

위 내용을 이제 봤는데 testcontainers 버전을 올렸으면 쉽게 해결됐을거같네유 하핫 🥲

@rlagkswn00 rlagkswn00 merged commit ef61606 into develop Apr 24, 2025
5 checks passed
zbqmgldjfh pushed a commit that referenced this pull request Apr 26, 2025
* feat : 비밀번호 초기화 인증 코드 전송 API 분리

* feat : RandomGenerator 랜덤 양수 생성 메서드 추가 및 닉네임 생성 로직 제거

* feat : NicknameGenerator 추가

* feat : VerificationCodeGenerator 추가

* refactor : 회원가입 시 닉네임 생성 시 NicknameGenerator 사용하도록 변경

* test : 비밀번호 초기화 이메일 인증코드 전송 인수 테스트 추가

* test : 비밀번호 초기화 이메일 인증코드 전송 인수 테스트 추가

* test : Generator 클래스 테스트 추가

* fix : 닉네임 생성 시 닉네임 생성 안되는 이슈 해결

* test : 랜덤 양수 생성 테스트 제거

* feat : 존재하는 닉네임 조회 PersistenceAdapter 로직 추가

* refactor : 닉네임 생성 시 한번에 일정 개수 생성 후 DB 비교 로직 추가

* test : 정규식 사용하여 테스트 하도록 변경

* fix : 통합 테스트 시 ChromaDB 컨테이너 동작 확인 heartbeat API 변경

* feat : RootUserQueryRepository 추가

(cherry picked from commit ef61606)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants