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

[REFACTOR/#169] 성지순례 카테고리 조회 리팩토링 및 서비스 테스트 추가 #175

Merged
merged 18 commits into from
Feb 11, 2025

Conversation

ppparkta
Copy link
Contributor

@ppparkta ppparkta commented Jan 23, 2025

📄 Work Description

성지순례 카테고리 조회 관련 서비스 레이어를 리팩토링하고
테스트코드를 추가했습니다.

변경 사항

  • 인메모리 DB 추가
    • 실제 데이터를 추가하여 테스트하는 경우를 대비하여 테스트용 인메모리 DB 설정을 추가했습니다.
  • PilgrimageQueryService 분리 및 리팩토링
    • PilgrimageCategoryQueryService / PilgrimageQueryService 분리
    • 기존 인터페이스 메서드의 가독성을 높이기 위해 메서드 분리를 진행했습니다.
  • 서비스 레이어 테스트 코드 추가
    • Mockito 사용하여 의존성을 가능한 배제하고 테스트 코드를 작성했습니다.

⚙️ ISSUE

📷 Screenshot

image

public 메서드 가독성 위주로 리팩토링 진행했습니다. ex)

PilgrimageCategoryQueryService 리팩토링 진행 전

    public List<RallyResponseDto.PilgrimageCategoryAnimeDto> getCategoryAnime(Member member) {
        // 전체 랠리 최신 순으로 조회하기
        List<Rally> rallyList = rallyRepository.findAllOrderByCreatedAt();
        if (member == null) {
            return rallyList.stream()
                    .map(rally -> RallyConverter.toPilgrimageCategoryAnimeDto(rally, 0L))
                    .collect(Collectors.toList());
        }
        return rallyList.stream().map(rally->{
            Long visitedPilgrimages = visitedPilgrimageRepository.findByDistinctCount(member.getId(), rally.getId());
            return RallyConverter.toPilgrimageCategoryAnimeDto(rally, visitedPilgrimages);
        }).collect(Collectors.toList());
    }

PilgrimageCategoryQueryService 리팩토링 진행 후

    public List<PilgrimageCategoryAnimeDto> getCategoryAnime(@UserEmail String email) {
        List<Rally> rallyList = rallyRepository.findAllOrderByCreatedAt();

        if (email == null || email.isEmpty()) {
            return getPilgrimageAnimeCategoryWithoutMember(rallyList);
        }

        Member member = getMemberByEmail(email);
        return getPilgrimageAnimeCategoryWithMember(rallyList, member);
    }

💬 To Reviewers

Mockito 사용해서 의존성 최대한 배제하고 테스트 코드를 작성했으나,
모킹 놓친 부분이 있을 수 있습니다. 혹시 발견하신다면 피드백 부탁드립니다.

그 외 모든 사항 피드백 환영합니당

🔗 Reference

문제를 해결하면서 도움이 되었거나, 참고했던 사이트(코드링크)
https://bcp0109.tistory.com/317

추가 사항
- Nested class로 분리
- 로그인 상태에서 성지순례 애니별 카테고리 조회 성공 테스트 추가
변경 내역
- Category 관련 조회 서비스 분리
변경 내역
- 인터페이스 메서드 간략하게 수정
- 메서드 분리
변경 내역
- 기존 하드코딩에서 한 눈에 추가되는 값을 확인할 수 있도록 수정
@ppparkta ppparkta requested a review from JungYoonShin January 23, 2025 08:17
@ppparkta ppparkta self-assigned this Jan 23, 2025
@ppparkta ppparkta linked an issue Jan 23, 2025 that may be closed by this pull request
5 tasks
Copy link
Member

@JungYoonShin JungYoonShin left a comment

Choose a reason for hiding this comment

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

수고하셨어요!!! Service 분리 한 거 좋은 시도인 거 같아요오
테스트코드 열심히 더 공부해올게여 .. 총총👣


// Lombok
compileOnly 'org.projectlombok:lombok'
Copy link
Member

Choose a reason for hiding this comment

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

요거 implementation으로 바꾸신 이유가 있나요?? (단순 궁금증)

Copy link
Contributor Author

@ppparkta ppparkta Jan 30, 2025

Choose a reason for hiding this comment

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

테스트코드 작성할 때 롬복이 필요해서 수정했어요!
찾아보니까 compileOnly + testCompileOnly 쓰는것보다 implementation만 쓰는게 더 무겁다고 하네요...
gradle 파일 길어지면 관리가 까다로워질까봐 수정한건데 원래대로 되돌리는게 더 좋을거 같아요!

+) 수정했습니다 :)

Copy link
Member

Choose a reason for hiding this comment

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

오..새로운 사실 덕분에 알아가요 좋습니다!!

@@ -79,24 +77,26 @@ public RallyResponseDto.RallyDetailResponseDto getRallyDetail(Long rallyId, Memb
*/
public RallyResponseDto.RallyAddressListDto getRallyAddressList(Long rallyId, Member member) {
Rally rally = rallyRepository.findById(rallyId).orElseThrow(
()-> new RestApiException(ErrorCode.RALLY_NOT_FOUND));
() -> new RestApiException(ErrorCode.RALLY_NOT_FOUND));

// 주소, 성지순례 리스트 정보 담은 주소 리스트 생성
List<Address> addressList = addressRepository.findByPilgrimages_Rally(rally);
System.out.println(addressList.size());
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.

헉 맞습니다 지울게요!

Comment on lines +43 to +49
private PilgrimageCategoryQueryService pilgrimageService;

@BeforeEach
void setup() {
this.pilgrimageService = new PilgrimageCategoryQueryService(memberRepository, rallyRepository,
visitedPilgrimageRepository, addressRepository, pilgrimageRepository);
}
Copy link
Member

Choose a reason for hiding this comment

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

@InjectMocks를 붙이는 대신에 setUp() 메서드로 @Mock으로 만든 객체들을 주입한 후 pilgrimageService 인스턴스를 생성하신 이유가 궁금해요!

Copy link
Contributor Author

@ppparkta ppparkta Jan 30, 2025

Choose a reason for hiding this comment

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

injectMocks 사용이 낯설어서 직접 주입하는 방식을 선택했습니다
덕분에 편리한 방법을 알았어요 감사합니다!

@ppparkta ppparkta merged commit ca9431e into develop Feb 11, 2025
1 check passed
@ppparkta ppparkta deleted the refactor/#169 branch February 11, 2025 01:10
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.

[REFACTOR] 성지순례 카테고리 조회 리팩토링 및 테스트코드
2 participants