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.

Sign up for GitHub

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

Jump to bottom

[refactor] N + 1 문제를 개선한다. #726

Merged
merged 5 commits into from
Oct 7, 2022
Merged

[refactor] N + 1 문제를 개선한다. #726

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
34cdf89
feat: N + 1 해결을 위한 애노테이션 추가
hyeonic Oct 6, 2022
27de2d8
feat: categoryRole 조회 로직 이동
hyeonic Oct 6, 2022
f9032ab
feat: 불필요 메서드 및 테스트 제거
hyeonic Oct 6, 2022
7c945d7
refactor: 불필요 로직 제거
hyeonic Oct 6, 2022
6cddc52
refactor: SubscriptionRepository N + 1 개선
hyeonic Oct 6, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 12 additions & 4 deletions backend/src/main/java/com/allog/dallog/domain/category/application/CategoryService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import com.allog.dallog.domain.subscription.domain.SubscriptionRepository;
import java.util.List;
import java.util.Set;
import java.util.stream.Collectors;
import org.springframework.data.domain.Pageable;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;
Expand Down Expand Up @@ -113,14 +114,21 @@ public CategoriesWithPageResponse findMyCategories(final Long memberId, final St

// 멤버가 ADMIN이 아니어도 일정 추가/제거/수정이 가능하므로, findAdminCategories와 별도의 메소드로 분리해야함
public CategoriesResponse findScheduleEditableCategories(final Long memberId) {
List<CategoryRole> categoryRoles = categoryRoleRepository.findByMemberId(memberId);
Copy link
Collaborator

Choose a reason for hiding this comment

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

CategoryRepository에 있던 CategoryRole을 통한 데이터 조회 로직이 삭제되었네요!!

Set<CategoryRoleType> roleTypes = CategoryRoleType.getHavingAuthorities(Set.of(ADD_SCHEDULE, UPDATE_SCHEDULE));
List<Category> categories = categoryRepository.findByMemberIdAndCategoryRoleTypes(memberId, roleTypes);
return new CategoriesResponse(categories);
return new CategoriesResponse(toCategories(categoryRoles, roleTypes));
}

public CategoriesResponse findAdminCategories(final Long memberId) {
List<Category> categories = categoryRepository.findByMemberIdAndCategoryRoleTypes(memberId, Set.of(ADMIN));
return new CategoriesResponse(categories);
List<CategoryRole> categoryRoles = categoryRoleRepository.findByMemberId(memberId);
return new CategoriesResponse(toCategories(categoryRoles, Set.of(ADMIN)));
}

private List<Category> toCategories(final List<CategoryRole> categoryRoles, final Set<CategoryRoleType> roleTypes) {
return categoryRoles.stream()
.filter(categoryRole -> roleTypes.contains(categoryRole.getCategoryRoleType()))
.map(CategoryRole::getCategory)
.collect(Collectors.toList());
Comment on lines +126 to +131
Copy link
Collaborator

Choose a reason for hiding this comment

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

a: 저는 변환 로직을 DTO로 옮기는 편이여서 CategoriesResponse에서 해당 변환로직을 가지는건 어떨까해요!
한번 의견만 제시해보았어요😊😊

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

이 부분은 항상 고민이네요... 저는 dto가 가벼운 걸 선호해서 일단 유지하겠습니다!

}

public CategoryDetailResponse findDetailCategoryById(final Long id) {
Expand Down
10 changes: 0 additions & 10 deletions backend/src/main/java/com/allog/dallog/domain/category/domain/CategoryRepository.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
package com.allog.dallog.domain.category.domain;

import com.allog.dallog.domain.category.exception.NoSuchCategoryException;
import com.allog.dallog.domain.categoryrole.domain.CategoryRoleType;
import java.util.List;
import java.util.Set;
import org.springframework.data.domain.Pageable;
import org.springframework.data.domain.Slice;
import org.springframework.data.jpa.repository.JpaRepository;
Expand All @@ -27,14 +25,6 @@ Slice<Category> findByCategoryTypeAndNameContaining(final CategoryType categoryT

List<Category> findByMemberIdAndCategoryType(final Long memberId, final CategoryType categoryType);

@Query("SELECT c "
+ "FROM CategoryRole r "
+ "JOIN r.category c "
+ "WHERE r.member.id = :memberId "
+ "AND r.categoryRoleType IN :categoryRoleTypes")
List<Category> findByMemberIdAndCategoryRoleTypes(final Long memberId,
final Set<CategoryRoleType> categoryRoleTypes);

List<Category> findByMemberId(final Long memberId);

boolean existsByIdAndMemberId(final Long id, final Long memberId);
Expand Down
3 changes: 3 additions & 0 deletions ...end/src/main/java/com/allog/dallog/domain/categoryrole/domain/CategoryRoleRepository.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@ public interface CategoryRoleRepository extends JpaRepository<CategoryRole, Long
@EntityGraph(attributePaths = {"member"})
List<CategoryRole> findByCategoryId(final Long categoryId);

@EntityGraph(attributePaths = {"category", "category.member"})
List<CategoryRole> findByMemberId(final Long memberId);

Comment on lines +16 to +18
Copy link
Collaborator

Choose a reason for hiding this comment

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

한번에 fetch join 해오는군요!!

int countByCategoryIdAndCategoryRoleType(final Long categoryId, final CategoryRoleType categoryRoleType);

void deleteByCategoryId(final Long categoryId);
Expand Down
10 changes: 2 additions & 8 deletions ...in/java/com/allog/dallog/domain/externalcalendar/application/ExternalCalendarService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,7 @@
import com.allog.dallog.domain.auth.application.OAuthClient;
import com.allog.dallog.domain.auth.domain.OAuthToken;
import com.allog.dallog.domain.auth.domain.OAuthTokenRepository;
import com.allog.dallog.domain.auth.exception.NoSuchOAuthTokenException;
import com.allog.dallog.domain.externalcalendar.dto.ExternalCalendar;
import com.allog.dallog.domain.externalcalendar.dto.ExternalCalendarsResponse;
import java.util.List;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;

Expand All @@ -26,14 +23,11 @@ public ExternalCalendarService(final OAuthClient oAuthClient, final ExternalCale
}

public ExternalCalendarsResponse findByMemberId(final Long memberId) {
OAuthToken oAuthToken = oAuthTokenRepository.findByMemberId(memberId)
.orElseThrow(NoSuchOAuthTokenException::new);
OAuthToken oAuthToken = oAuthTokenRepository.getByMemberId(memberId);
Copy link
Collaborator

Choose a reason for hiding this comment

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

레거시 했던 코드가 삭제됐군요!👍👍


String oAuthAccessToken = oAuthClient.getAccessToken(oAuthToken.getRefreshToken())
.getAccessToken();

List<ExternalCalendar> externalCalendars = externalCalendarClient.getExternalCalendars(oAuthAccessToken);

return new ExternalCalendarsResponse(externalCalendars);
return new ExternalCalendarsResponse(externalCalendarClient.getExternalCalendars(oAuthAccessToken));
}
}
5 changes: 3 additions & 2 deletions ...end/src/main/java/com/allog/dallog/domain/subscription/domain/SubscriptionRepository.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import com.allog.dallog.domain.subscription.exception.ExistSubscriptionException;
import com.allog.dallog.domain.subscription.exception.NoSuchSubscriptionException;
import java.util.List;
import org.springframework.data.jpa.repository.EntityGraph;
import org.springframework.data.jpa.repository.JpaRepository;

public interface SubscriptionRepository extends JpaRepository<Subscription, Long> {
Expand All @@ -12,10 +13,10 @@ public interface SubscriptionRepository extends JpaRepository<Subscription, Long

boolean existsByIdAndMemberId(final Long id, final Long memberId);

// TODO: N + 1 개선 예정
// TODO: @EntityGraph(attributePaths = {"category", "category.member"})
@EntityGraph(attributePaths = {"category", "category.member"})
Copy link
Collaborator

Choose a reason for hiding this comment

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

N+1이 개선 되었군요!!👍👍

List<Subscription> findByMemberId(final Long memberId);

@EntityGraph(attributePaths = {"category", "category.member"})
List<Subscription> findByCategoryId(final Long categoryId);

List<Subscription> findByMemberIdIn(final List<Long> memberIds);
Expand Down
1 change: 1 addition & 0 deletions backend/src/test/java/com/allog/dallog/domain/category/application/CategoryServiceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ class CategoryServiceTest extends ServiceTest {

SubscriptionsResponse subscriptions = subscriptionService.findByMemberId(파랑_id);
List<SubscriptionResponse> actual = subscriptions.getSubscriptions();

// then
assertThat(actual).hasSize(2);
}
Expand Down
35 changes: 0 additions & 35 deletions backend/src/test/java/com/allog/dallog/domain/category/domain/CategoryRepositoryTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,28 +10,23 @@
import static com.allog.dallog.common.fixtures.CategoryFixtures.매트_아고라;
import static com.allog.dallog.common.fixtures.CategoryFixtures.우아한테크코스_일정;
import static com.allog.dallog.common.fixtures.CategoryFixtures.후디_JPA_스터디;
import static com.allog.dallog.common.fixtures.CategoryFixtures.후디_JPA_스터디_이름;
import static com.allog.dallog.common.fixtures.MemberFixtures.관리자;
import static com.allog.dallog.common.fixtures.MemberFixtures.리버;
import static com.allog.dallog.common.fixtures.MemberFixtures.매트;
import static com.allog.dallog.common.fixtures.MemberFixtures.파랑;
import static com.allog.dallog.common.fixtures.MemberFixtures.후디;
import static com.allog.dallog.common.fixtures.SubscriptionFixtures.색상1_구독;
import static com.allog.dallog.domain.category.domain.CategoryType.NORMAL;
import static com.allog.dallog.domain.categoryrole.domain.CategoryRoleType.ADMIN;
import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.jupiter.api.Assertions.assertAll;

import com.allog.dallog.common.annotation.RepositoryTest;
import com.allog.dallog.domain.categoryrole.domain.CategoryRole;
import com.allog.dallog.domain.categoryrole.domain.CategoryRoleRepository;
import com.allog.dallog.domain.member.domain.Member;
import com.allog.dallog.domain.member.domain.MemberRepository;
import com.allog.dallog.domain.subscription.domain.SubscriptionRepository;
import java.util.List;
import java.util.Objects;
import java.util.Set;
import java.util.stream.Collectors;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;
Expand Down Expand Up @@ -204,36 +199,6 @@ class CategoryRepositoryTest extends RepositoryTest {
});
}

@DisplayName("멤버와 카테고리 역할 목록으로 카테고리를 조회한다.")
@Test
void 멤버와_카테고리_역할_목록으로_카테고리를_조회한다() {
// given
Member 관리자 = memberRepository.save(관리자());
Member 후디 = memberRepository.save(후디());

Category 카테고리1 = categoryRepository.save(공통_일정(관리자));
Category 카테고리2 = categoryRepository.save(BE_일정(관리자));
Category 카테고리3 = categoryRepository.save(FE_일정(관리자));
Category 카테고리4 = categoryRepository.save(매트_아고라(관리자));
Category 카테고리5 = categoryRepository.save(후디_JPA_스터디(관리자));
Comment on lines -207 to -218
Copy link
Collaborator

Choose a reason for hiding this comment

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

불필요한 테스트 코드가 삭제되었군요🚀🚀


categoryRoleRepository.save(new CategoryRole(카테고리2, 관리자, ADMIN));
categoryRoleRepository.save(new CategoryRole(카테고리4, 관리자, ADMIN));

categoryRoleRepository.save(new CategoryRole(카테고리1, 후디, ADMIN));
categoryRoleRepository.save(new CategoryRole(카테고리3, 후디, ADMIN));
categoryRoleRepository.save(new CategoryRole(카테고리5, 후디, ADMIN));

// when
List<Category> categories = categoryRepository.findByMemberIdAndCategoryRoleTypes(후디.getId(), Set.of(ADMIN));
List<String> actual = categories.stream()
.map(Category::getName)
.collect(Collectors.toList());

// then
assertThat(actual).containsExactly(공통_일정_이름, FE_일정_이름, 후디_JPA_스터디_이름);
}

@DisplayName("카테고리 id와 회원의 id가 모두 일치하는 카테고리가 있으면 true를 반환한다.")
@Test
void 카테고리_id와_회원의_id가_모두_일치하는_카테고리가_있으면_true를_반환한다() {
Expand Down