Skip to content

Conversation

@Invidam
Copy link
Contributor

@Invidam Invidam commented Mar 17, 2024

▶ Request

Content

#353

as-is

  • 이메일 중복 검사, race condition에 의해 서버 에러(이메일 중복)가 발생함.

to-be

  • 발생하더라도 예외처리로 클라이언트 에러를 발생시킴.

✅ Check List

  • 의도치 않은 변경이 일어나지 않았는지.
    • 실수로 라이브러리(pom.xml) 변경이 일어나지 않았는지
    • 병합시 컴파일 & 런타임 에러가 발생하지 않는지
    • 기존 클라이언트와의 호환성 고려가 잘 이루어졌는지
  • 작성한 코드가 프로젝트에 반영됨을 명심하였는지
    • 타인도 알아보고 변경할 수 있는 코드를 작성하였는지
    • 코드 & 커밋 컨벤션을 준수하였는지
    • (필요한) 문서화가 진행되었는지
  • (기능 추가의 경우) 클라이언트의 입장에 대한 충분한 고려가 이루어졌는지
    • 클라이언트 측과 협의가 된 내용인 경우
    • 클라이언트의 요구사항을 잘 반영하는지
    • API 문서에 논리적인 오류 & 가시성이 떨어지는 내용이 없는지

📸 API Document ScreenShot

🧪 Test

  • 디버깅을 통해 이미 저장된 이메일을 만드려고 할 때 예외를 잡아 클라이언트 에러로 반환함을 확인

@Invidam Invidam self-assigned this Mar 17, 2024
@Invidam Invidam changed the base branch from production to develop March 17, 2024 14:06
Copy link
Member

@Choi-JJunho Choi-JJunho left a comment

Choose a reason for hiding this comment

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

race condition.. 클라이언트에서 디바운싱으로 막아주면 더 좋겠네요

코멘트 하나만 확인해주세용

Comment on lines 156 to 164
// redisOwnerMapper.validateOwner(ownerEmailAddress, ownerAuthPrefix);

encodePassword(owner);

createInDBFor(owner);

slackNotiSender.noticeRegisterComplete(owner);

redisOwnerMapper.removeRedisFrom(ownerEmailAddress, ownerAuthPrefix);
// redisOwnerMapper.removeRedisFrom(ownerEmailAddress, ownerAuthPrefix);
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
Contributor Author

Choose a reason for hiding this comment

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

헐 되돌려야대요.. 테스트용

Copy link
Collaborator

@johnny19991006 johnny19991006 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 313 to 315
} catch (DuplicateKeyException e
) {
throw new BaseException(ExceptionInformation.EMAIL_DUPLICATED);
Copy link
Collaborator

Choose a reason for hiding this comment

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

여기Key중복 되었을떄의 Exception처리가 되었네요 Good

Copy link
Member

@Choi-JJunho Choi-JJunho left a comment

Choose a reason for hiding this comment

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

굿~
라인 포맷팅은 애교로 넘어가드리겠습니다

@Invidam Invidam merged commit 765a6f1 into develop Mar 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants