[refactor] 불필요하게 join을 활용하는 조회 로직을 개선한다. #738

hyeonic · 2022-10-08T07:22:38Z

🔀 PR 제목의 형식을 잘 작성했나요? e.g. [feat] PR을 등록한다.
💯 테스트는 잘 통과했나요?
🏗️ 빌드는 성공했나요?
🧹 불필요한 코드는 제거했나요?
💭 이슈는 등록했나요?
🏷️ 라벨은 등록했나요?
💻 git rebase를 사용했나요?
🌈 알록달록한가요?

작업 내용

불필요하게 join을 활용하는 조회 로직을 개선한다.

예시

Optional<OAuthToken> findByMemberId(final Long memberId);

개선 전

Hibernate: 
    select
        oauthtoken0_.id as id1_4_,
        oauthtoken0_.created_at as created_2_4_,
        oauthtoken0_.updated_at as updated_3_4_,
        oauthtoken0_.members_id as members_5_4_,
        oauthtoken0_.refresh_token as refresh_4_4_ 
    from
        oauth_tokens oauthtoken0_ 
    left outer join
        members member1_ 
            on oauthtoken0_.members_id=member1_.id 
    where
        member1_.id=?

개선 후

Hibernate: 
    select
        oauthtoken0_.id as id1_4_,
        oauthtoken0_.created_at as created_2_4_,
        oauthtoken0_.updated_at as updated_3_4_,
        oauthtoken0_.members_id as members_5_4_,
        oauthtoken0_.refresh_token as refresh_4_4_ 
    from
        oauth_tokens oauthtoken0_ 
    where
        oauthtoken0_.members_id=?

스크린샷

주의사항

Closes #725

gudonghee2000

안녕하세요 매트! 작성해주신 개선로직 정말 잘보았어요!!
불필요하게 join 되는 로직들이 사라졌군요👍👍

매트의 꼼꼼한 리팩터링에 다시한번 놀라네요ㅎㅎ
바로 approve하였습니다!

그런데, 한가지 궁금한 점이있어요!
작업내용에 첨부해주신 쿼리와 같이 Entity에 LazyLoading을 설정하여도 select시에 join` 쿼리가 발생하는것일까요?!
발생한다면 원인이 어떤것인가요?!😮😮

gudonghee2000 · 2022-10-09T11:58:17Z

backend/src/main/java/com/allog/dallog/domain/subscription/domain/SubscriptionRepository.java

-    List<Subscription> findByMemberIdIn(final List<Long> memberIds);
-


불필요한 메서드가 제거되었군요!

gudonghee2000 · 2022-10-09T11:59:35Z

.../src/test/java/com/allog/dallog/domain/categoryrole/application/CategoryRoleServiceTest.java

@@ -58,7 +58,7 @@ class CategoryRoleServiceTest extends ServiceTest {
        CategoryRoleUpdateRequest request = new CategoryRoleUpdateRequest(ADMIN);
        categoryRoleService.updateRole(관리자.getId(), 후디.getId(), BE_일정.getId(), request);

-        CategoryRole actual = categoryRoleRepository.findByMemberIdAndCategoryId(후디.getId(), BE_일정.getId()).get();
+        CategoryRole actual = categoryRoleRepository.getByMemberIdAndCategoryId(후디.getId(), BE_일정.getId());


테스트 코드에서 default 메서드를 사용하게끔 변경해주셨군요!!👍👍

gudonghee2000 · 2022-10-09T12:00:33Z

...nd/src/test/java/com/allog/dallog/domain/subscription/domain/SubscriptionRepositoryTest.java

-    @DisplayName("member id 리스트를 기반으로 구독 정보를 조회한다.")
-    @Test
-    void member_id_리스트를_기반으로_구독_정보를_조회한다() {
-        // given
-        Member 관리자 = memberRepository.save(관리자());
-        Category 공통_일정 = categoryRepository.save(공통_일정(관리자));
-
-        Member 매트 = memberRepository.save(매트());
-        Member 리버 = memberRepository.save(리버());
-        Member 파랑 = memberRepository.save(파랑());
-        Member 후디 = memberRepository.save(후디());
-
-        subscriptionRepository.save(색상1_구독(매트, 공통_일정));
-        subscriptionRepository.save(색상1_구독(리버, 공통_일정));
-        subscriptionRepository.save(색상1_구독(파랑, 공통_일정));
-        subscriptionRepository.save(색상1_구독(후디, 공통_일정));
-
-        List<Long> memberIds = Stream.of(매트, 리버, 파랑, 후디)
-                .map(Member::getId)
-                .collect(Collectors.toList());
-
-        // when
-        List<Subscription> actual = subscriptionRepository.findByMemberIdIn(memberIds);
-
-        // then
-        assertThat(actual).hasSize(4);


불필요한 테스트코드가 제거되었군요!!👍👍

gudonghee2000 · 2022-10-09T12:07:54Z

backend/src/main/java/com/allog/dallog/domain/categoryrole/domain/CategoryRoleRepository.java

+    @Query("SELECT cr "
+            + "FROM CategoryRole cr "
+            + "WHERE cr.member.id = :memberId AND cr.category.id = :categoryId")


이 메서드는 lazy로딩이 걸려있어서 join로직이 발생하지 않을것 같은데
제가 잘못 알고있는 부분이있는걸까요?!

그거와 별개로 CategoryRole이 member를 필드로 가지고 있어서 memberId로 명시하게 되면 필드를 못 찾고 값을 얻기 위해 outer join을 진행하더라구요 ㅜㅠ

[Spring Data JPA] findByXXXId 는 불필요한 join을 유발한다

참고해주시면 감사하겠습니다!

gudonghee2000 · 2022-10-09T12:09:39Z

backend/src/main/java/com/allog/dallog/domain/category/domain/CategoryRepository.java

+    @Query("SELECT c "
+            + "FROM Category c "
+            + "WHERE c.member.id = :memberId AND c.categoryType = :categoryType")


불필요한 join 로직이 제거 되었군요!!

summerlunaa

find 메서드를 사용할 때 저런 방식으로 조인이 되는지 전혀 몰랐네요 🥹 변경사항 확인했습니다!! 수고하셨습니다

summerlunaa · 2022-10-10T01:35:30Z

backend/src/test/java/com/allog/dallog/domain/category/application/CategoryServiceTest.java

@@ -593,7 +593,7 @@ class CategoryServiceTest extends ServiceTest {
        Member 후디 = memberRepository.save(후디());
        subscriptionService.save(후디.getId(), 공통_일정.getId());

-        CategoryRole 역할 = categoryRoleRepository.findByMemberIdAndCategoryId(후디.getId(), 공통_일정.getId()).get();
+        CategoryRole 역할 = categoryRoleRepository.getByMemberIdAndCategoryId(후디.getId(), 공통_일정.getId());


summerlunaa · 2022-10-10T01:36:30Z

...nd/src/test/java/com/allog/dallog/domain/categoryrole/domain/CategoryRoleRepositoryTest.java

+import org.junit.jupiter.api.Test;
+import org.springframework.beans.factory.annotation.Autowired;
+
+class CategoryRoleRepositoryTest extends RepositoryTest {


a: 테스트 추가 굿

hyeonic added backend 백엔드 refactor 리팩터링 labels Oct 8, 2022

hyeonic requested review from devHudi, gudonghee2000 and summerlunaa October 8, 2022 07:22

hyeonic self-assigned this Oct 8, 2022

gudonghee2000 approved these changes Oct 9, 2022

View reviewed changes

summerlunaa approved these changes Oct 10, 2022

View reviewed changes

hyeonic added 5 commits October 10, 2022 10:42

refactor: OAuthTokenRepository 불필요 조인 제거

08d1b72

refactor: CategoryRepository 불필요 조인 제거

3969518

test: 검증 메서드 변경

7ddf1d8

refactor: CategoryRoleRepository 불필요 join 제거

1a81256

refactor: 사용하지 않는 불필요 메서드 제거

f81733c

hyeonic force-pushed the feature/725-unnecessary-join branch from 87b4796 to f81733c Compare October 10, 2022 01:42

hyeonic merged commit b05c9d9 into woowacourse-teams:develop Oct 10, 2022

hyeonic deleted the feature/725-unnecessary-join branch October 10, 2022 01:46

hyeonic mentioned this pull request Oct 19, 2022

[release] v1.1.1 #780

Merged

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[refactor] 불필요하게 join을 활용하는 조회 로직을 개선한다. #738

[refactor] 불필요하게 join을 활용하는 조회 로직을 개선한다. #738

hyeonic commented Oct 8, 2022

gudonghee2000 left a comment

gudonghee2000 Oct 9, 2022

gudonghee2000 Oct 9, 2022

gudonghee2000 Oct 9, 2022

gudonghee2000 Oct 9, 2022

hyeonic Oct 10, 2022 •

edited

Loading

gudonghee2000 Oct 9, 2022

summerlunaa left a comment

summerlunaa Oct 10, 2022

summerlunaa Oct 10, 2022

		List<Subscription> findByMemberIdIn(final List<Long> memberIds);

[refactor] 불필요하게 join을 활용하는 조회 로직을 개선한다. #738

[refactor] 불필요하게 join을 활용하는 조회 로직을 개선한다. #738

Conversation

hyeonic commented Oct 8, 2022

작업 내용

예시

개선 전

개선 후

스크린샷

주의사항

gudonghee2000 left a comment

Choose a reason for hiding this comment

gudonghee2000 Oct 9, 2022

Choose a reason for hiding this comment

gudonghee2000 Oct 9, 2022

Choose a reason for hiding this comment

gudonghee2000 Oct 9, 2022

Choose a reason for hiding this comment

gudonghee2000 Oct 9, 2022

Choose a reason for hiding this comment

hyeonic Oct 10, 2022 • edited Loading

Choose a reason for hiding this comment

gudonghee2000 Oct 9, 2022

Choose a reason for hiding this comment

summerlunaa left a comment

Choose a reason for hiding this comment

summerlunaa Oct 10, 2022

Choose a reason for hiding this comment

summerlunaa Oct 10, 2022

Choose a reason for hiding this comment

hyeonic Oct 10, 2022 •

edited

Loading