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(#15): 카카오 로그인 #18

Merged
merged 35 commits into from
Jan 30, 2025
Merged

Conversation

kimyu0218
Copy link
Collaborator

@kimyu0218 kimyu0218 commented Jan 28, 2025

작업 개요

카카오 로그인을 위한 http 클라이언트, service, controller를 작성했습니다.

작업 사항

  • 카카오 로그인 api 엔드포인트를 호출하기 위한 http 클라이언트 작성
  • http 클라이언트를 위한 http 프록시 객체 생성 로직 작성
  • 카카오 로그인을 위한 dto 객체, 서비스, 컨트롤러 작성

고민한 점들(필수 X)

  • 프로젝트 구조를 어떻게 가져하야 할지 모르겠습니다..! 더 좋은 방법이 있다면 말씀해주세요.

    • common > http, httpinterface
    • common > support > model
    • domain > auth > restclient
  • 테스트 코드 클래스 외부에 테스트 데이터를 정의하고 싶지 않은데 어떻게 하면 좋을까요? (테스트 코드 내부에 테스트 데이터를 정의해야 테스트 코드를 한눈에 파악하고, 파일 삭제로 인한 테스트 실패가 발생하지 않을 거라고 생각했습니다)

    • http interface를 조회할 때 접근 제어자를 고려하지 않아 base package 내 모든 http interface를 조회하고 있습니다
    • http interface를 찾는 base package를 수정하여 테스트 클래스가 위치한 클래스에서 http interface를 찾도록 했지만 http interface와 관련된 테스트 클래스가 같은 패키지에 존재하면서 테스트에 실패했습니다 (a와 b 테스트 클래스에 http interface가 존재)
      • HttpInterfaceFinder : @HttpExchange가 붙어있는 인터페이스 조회
      • HttpInterfaceFactory : 찾은 http interface를 위한 프록시 객체 생성 (+ 유효성 검사)
      • HttpInterfaceFactory : http interface 빈 객체 생성
  • api 문서 작성 시 depth가 많이 깊어지는데 다음과 같이 작성하는 게 좋을까요?

// [MockHttpServletRequestBuilder] 별도 선언
// [ResourceSnippetParameters] 별도 선언

mockMvc.perform([MockHttpServletRequestBuilder])
  .andExpect(status().isOk())
  .andDo([ResourceSnippetParameters]);
  • api 명명 규칙(ex. 버저닝)도 말씀해주시면 반영하겠습니다. 현재는 /api만 앞에 붙여줬습니다!
  • ddl은 어디에 어떤 파일명으로 작성하면 좋을까요? 👀

@NaMinhyeok @pythonstrup
jwt provider 호출 로직을 제외하고 작성했습니다. 리베이스 해서 작업하면 될까요? (아니면 작업하시는 동안 리뷰 반영하거나 애플 로그인 공부하고 있겠습니다!!)

@kimyu0218 kimyu0218 self-assigned this Jan 28, 2025
Comment on lines 37 to 54
static boolean filterFixture(final BeanDefinition beanDefinition) {
try {
Class<?> clazz = Class.forName(beanDefinition.getBeanClassName());
return !clazz.getPackage().getName().contains("fixture");
} catch (ClassNotFoundException e) {
return false;
}
}

static boolean isTestEnvironment() {
StackTraceElement[] stackTrace = Thread.currentThread().getStackTrace();
for (StackTraceElement element : stackTrace) {
if (element.getClassName().contains("org.springframework.boot.test.context")) {
return true;
}
}
return false;
}
Copy link
Collaborator Author

@kimyu0218 kimyu0218 Jan 28, 2025

Choose a reason for hiding this comment

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

보내주신 블로그 글에 없던 내용입니다.

  • HttpInterfaceFinder가 base package를 중심으로 접근제어자, 테스트 파일과 관계없이 모든 http interface를 스캔하기 때문에 넣어줬습니다.
  • 테스트 코드를 작성하지 않는다면 없어도 되는 부분입니다.

https://mangkyu.tistory.com/291

Copy link
Member

@pythonstrup pythonstrup left a comment

Choose a reason for hiding this comment

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

  • 고생하셨습니다! 금방 하셨네요!
  • HttpInterfaceRestClient 구성과 카카오 로그인을 나눠서 작업할 걸 그랬나보네요.. 다음부터는 티켓을 좀 더 쪼개볼까요?

질문에 대한 답변

1. 프로젝트 구조

  1. common/http에 모여 있는 게 좋을 것 같습니다!
  2. BaseEntitycommon/model에 들어가도 괜찮을 것 같아요.
  3. domain/auth/http는 어떨까요? common과 일관성있게 하면 코드를 이해하는 데 좋을 것 같아서요.

2. 테스트 코드

  • 테스트 코드 클래스 외부에 테스트 데이터를 정의하지 않는 것이 좋다는 것에는 동의합니다.
  • 이 부분은 조금 더 확인해보고 리뷰 달겠습니다.

3. api 테스트 depth

  • 저는 이정도 depth는 괜찮다고 생각합니다.
  • 보기 불편하시면 변수로 나누는 것도 괜찮다고 생각해요.

4. api versioning

  • /api/v1로 할까요? @NaMinhyeok 의견 부탁드립니다.

5. ddl 정의

  • jaknaeso-coresrc/java/resources/sql 디렉토리에 정리해둘까요?
  • 파일명은 {issueNumber}/{순서}_{작업유형}_{대상}.sql 제안드립니다.
  • ex) 15/001_create_table_member.sql

@kimyu0218 kimyu0218 linked an issue Jan 28, 2025 that may be closed by this pull request
Copy link
Member

@NaMinhyeok NaMinhyeok left a comment

Choose a reason for hiding this comment

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

고생하셨습니다 ! 엄청 빠르게 하셨네요

일단 리베이스 하지 마시고.. 필요한 부분 수정하시고,제 작업이 너무 늦다면.. 애플로그인 공부 하시면 될 것 같아요 ! 제 작업은 내일아침이 오기전까지 마무리해두겠습니다.. 죄송해여 😭

제가 HttpInterface에 대해서는 잘 몰라서 리뷰를 제대로 못달겠네요 공부를 해보도록 하겠습니다 ㅎㅎ

저는 api 테스트에 대한 뎁스는 근데 깊어져도 크게 가독성을 해친다거나 하진않는다고 생각해요 실제로 mockMvc테스트 시에는 어느부분에서 문제 발생하는지 쉽게 찾을 수 있고 코드가 문서가 된다기보단, 테스트 성공으로 문서가 되기 때문에 가독성 부분은 괜찮은 것 같습니다 !

저는 API 버저닝 api/v1 하면 좋을 것 같습니다 !

void kakaoLoginFail() throws Exception {
KakaoLoginRequest request = new KakaoLoginRequest("invalid access token");

given(authService.kakaoLogin(request.toServiceDto())).willThrow(RestClientException.class);
Copy link
Member

Choose a reason for hiding this comment

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

이렇게도 예외를 던질 수가 있군요... willThrow로 하는 방법이 있는지 몰랐네요

저는 테스트 할 때 사실 Request 부분에서 예외를 발생시켜서 실제로 프론트한테 내려가는 응답에 대해서 전부 테스트(예외 메시지, 에러코드, http status 코드 등)했었는데 해당 방법도 좋은 것 같네요!

- 디렉토리 위치 변경
- 불필요한 Column 어노테이션 삭제
- createdId, updatedId 추가
BaseEntity - BaseTimeEntity - BaseAuditableEntity
- BaseEntity : id
- BaseTimeEntity : 타임스탬프 추적
- BaseAuditableEntity : 사용자 정보 추적
@kimyu0218 kimyu0218 force-pushed the feature/#15-카카오-로그인 branch from 8c325e7 to 371cf00 Compare January 29, 2025 18:23
Copy link
Member

@pythonstrup pythonstrup 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 +13 to +18
@CreatedBy
@Column(updatable = false)
private Long createdBy;

@LastModifiedBy private Long updatedBy;
}
Copy link
Member

Choose a reason for hiding this comment

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

이런 형태로 @CreatedBy 를 사용하려면 AuditorAware의 구현체를 만들어야된다고 하는데 없이도 사용이 가능한가요 ??

아니면 아직 관련 설정을 안해두신걸까요 ??

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

제가 @CreatedBy를 처음 사용해봐서 필요한 부분인지 몰랐습니다 😂
구글링 해보니 시큐리티에서 authentication 객체를 가져오고 principal의 username을 사용하는 것 같은데 이렇게 구현하셨었나요??

Copy link
Member

Choose a reason for hiding this comment

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

저는 @CreatedBy를 사용해본 적이 없어서 잘 모르겠지만, 조금 찾아보니 유정님이 제안하신대로 시큐리티에서 authentication 객체를 가져오고 principal의 username을 사용 하면 될 것 같아요!

Copy link
Member

Choose a reason for hiding this comment

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

사실 저도 사용해본적이 없어서 검색해보다가 알게되었는데 없는것 같아서 리뷰남겼습니다 🤣
principalusernameuserid가 들어있으니 이용하면 될 것 같아요 !!

@kimyu0218 kimyu0218 merged commit 64699ab into main Jan 30, 2025
1 check passed
@NaMinhyeok NaMinhyeok deleted the feature/#15-카카오-로그인 branch February 15, 2025 17:21
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.

feat: 카카오 로그인
3 participants