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

Fake DB 제거 #835

Open
wants to merge 2 commits into
base: dev/be
Choose a base branch
from
Open

Fake DB 제거 #835

wants to merge 2 commits into from

Conversation

jminkkk
Copy link
Contributor

@jminkkk jminkkk commented Oct 20, 2024

⚡️ 관련 이슈

close #740

📍주요 변경 사항

  • Fake DB를 제거
    • 다른 부분은 이전 ServiceTest 리팩토링 시에 Fake를 사용하지 않도록 개선했었습니다.
      다만 BasicAuthCredentialProvider 테스트는 여전히 Member Fake를 사용하고 있었고 실제 구현과 stub 중 고민했습니다.
      해당 테스트에서는 stub을 사용하더라도 사용되는 메서드가 하나이기에 거짓양성이 발생할 확률이 적고 관리 비용이 많지 않다고 판단하여 stub을 사용했습니다~

🎸기타

QueryDSL로 변경을 하기 위해서는 기존 Repository 추상화가 필요하고 관계가 많이 달라집니다
따라서 현재 Fake DB가 제거되어야지만 변경이 가능합니다.

+) 또한 템플릿 조회 시에 성능 문제로 전체 페이지 카운팅을 제거하기로 했는데, 기존처럼 동적 쿼리를 위해 Specification을 사용하고 페이징 처리를 위해 Pageable을 둘 다 사용하려면 JpaSpecificationExecutor 상속받도록 해야하는데, JpaSpecificationExecutor 는 Page 타입밖에 반환을 못받습니다. 관련 spring repo issue
따라서 QueryDSL을 사용하여 쿼리문을 작성하고 List로 반환받도록 변경해야합니다.
기존에는 QueryDSL을 단순 코드 가독성 개선 때문에 사용했다면, 성능적으로 필요한 이유가 하나 더 생겼습니다. 참고 부탁드립니다~

불가피하게 빠른 리뷰 부탁드립니다~!

🍗 PR 첫 리뷰 마감 기한

10/22 15:00

@jminkkk jminkkk added refactor 요구사항이 바뀌지 않은 변경사항 BE 백엔드 labels Oct 20, 2024
@jminkkk jminkkk added this to the 6차 스프린트 🦴 milestone Oct 20, 2024
@jminkkk jminkkk self-assigned this Oct 20, 2024
Copy link
Contributor

@HoeSeong123 HoeSeong123 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 66 to 74
Member unsaverdMember = MemberFixture.memberFixture();
String credential = basicAuthCredentialProvider.createCredential(unsaverdMember);

doThrow(new CodeZapException(ErrorCode.INVALID_REQUEST, "존재하지 않는 아이디 " + unsaverdMember.getName() + " 입니다."))
.when(memberRepository).fetchByName(any());

assertThatThrownBy(() -> basicAuthCredentialProvider.extractMember(credential))
.isInstanceOf(CodeZapException.class)
.hasMessage("존재하지 않는 아이디 " + unsaverdMember.getName() + " 입니다.");
Copy link
Contributor

Choose a reason for hiding this comment

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

이 테스트는 해당 클래스에서 진행하지 않아도 되지 않나요? memberRepository에서 진행해야 하는 테스트인 것 같아서요

Copy link
Contributor Author

@jminkkk jminkkk Oct 20, 2024

Choose a reason for hiding this comment

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

이 테스트는 해당 클래스에서 진행하지 않아도 되지 않나요?

MemberRepository를 말하는 걸까요?
BasicAuthCredentialProvider의 내부 구현에 대한 테스트라고 생각되어 따로 제거하지 않았습니다.
MemberRepository가 BasicAuthCredentialProvider를 의존하지 않고, 모르기 때문에 BasicAuthCredentialProvider에 대한 내용을 MemberRepository에서 테스트하는 건 어색할 것 같은데 어떻게 생각하시나요?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BE 백엔드 refactor 요구사항이 바뀌지 않은 변경사항
Projects
Status: Todo
Development

Successfully merging this pull request may close these issues.

2 participants