Skip to content

[refactor/#130] 문항세트 조회 리팩토링 #131

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

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

[refactor/#130] 문항세트 조회 리팩토링 #131

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
44 changes: 21 additions & 23 deletions ...n/java/com/moplus/moplus_server/admin/problemset/dto/response/ProblemSummaryResponse.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,29 +3,27 @@
import com.moplus.moplus_server.domain.problem.domain.problem.Problem;
import jakarta.validation.constraints.NotNull;
import java.util.Set;
import lombok.Builder;
import lombok.Getter;
import lombok.NoArgsConstructor;

@Builder
public record ProblemSummaryResponse(
@NotNull(message = "문항 ID는 필수입니다")
Long problemId,
@NotNull(message = "문항 custom ID는 필수입니다")
String problemCustomId,
String problemTitle,
String memo,
String mainProblemImageUrl,
@NotNull(message = "컬렉션 값은 필수입니다.")
Set<String> tagNames
) {
public static ProblemSummaryResponse of(Problem problem, Set<String> tagNames) {

return ProblemSummaryResponse.builder()
.problemId(problem.getId())
.problemCustomId(problem.getProblemCustomId())
.memo(problem.getMemo())
.problemTitle(problem.getTitle())
.mainProblemImageUrl(problem.getMainProblemImageUrl())
.tagNames(tagNames)
.build();
@Getter
@NoArgsConstructor
public class ProblemSummaryResponse{
@NotNull(message = "문항 ID는 필수입니다")
private Long problemId;
@NotNull(message = "문항 custom ID는 필수입니다")
private String problemCustomId;
private String problemTitle;
private String memo;
private String mainProblemImageUrl;
@NotNull(message = "컬렉션 값은 필수입니다.")
private Set<String> tagNames;
public ProblemSummaryResponse (Problem problem, Set<String> tagNames) {
this.problemId = problem.getId();
this.problemCustomId = problem.getProblemCustomId();
this.problemTitle = problem.getTitle();
this.memo = problem.getMemo();
this.mainProblemImageUrl = getMainProblemImageUrl();
this.tagNames = tagNames;
}
}
4 changes: 2 additions & 2 deletions src/main/java/com/moplus/moplus_server/client/homefeed/dto/response/HomeFeedResponse.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,8 @@ public record ProblemHomeFeedResponse(
) {
public static ProblemHomeFeedResponse of(ProblemSummaryResponse problemSummaryResponse) {
return new ProblemHomeFeedResponse(
problemSummaryResponse.problemId(),
problemSummaryResponse.mainProblemImageUrl()
problemSummaryResponse.getProblemId(),
problemSummaryResponse.getMainProblemImageUrl()
);
}
}
Expand Down
37 changes: 37 additions & 0 deletions .../com/moplus/moplus_server/domain/problemset/repository/ProblemSetGetRepositoryCustom.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
package com.moplus.moplus_server.domain.problemset.repository;

import static com.moplus.moplus_server.domain.concept.domain.QConceptTag.conceptTag;
import static com.moplus.moplus_server.domain.problem.domain.childProblem.QChildProblem.childProblem;
import static com.moplus.moplus_server.domain.problem.domain.problem.QProblem.problem;

import com.moplus.moplus_server.admin.problemset.dto.response.ProblemSummaryResponse;
import com.querydsl.core.group.GroupBy;
import com.querydsl.core.types.Projections;
import com.querydsl.jpa.impl.JPAQueryFactory;
import java.util.List;
import lombok.RequiredArgsConstructor;
import org.springframework.stereotype.Repository;

@Repository
@RequiredArgsConstructor
public class ProblemSetGetRepositoryCustom {
private final JPAQueryFactory queryFactory;

public List<ProblemSummaryResponse> findProblemSummariesWithTags(List<Long> problemIds) {
return queryFactory
.from(problem)
.leftJoin(childProblem).on(childProblem.in(problem.childProblems))
.leftJoin(conceptTag).on(
conceptTag.id.in(problem.conceptTagIds)
.or(conceptTag.id.in(childProblem.conceptTagIds))
)
Copy link
Contributor

Choose a reason for hiding this comment

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

현재 문항과 자식 문항을 조인한 뒤에 개념 태그까지 조인하다 보니, 카티시안 곱처럼 중복 데이터가 많이 생길 것 같아요. 특히 problem의 전체 컬럼을 다 가져오고 있어서 불필요한 데이터가 반복돼 성능상 부담이 될 수 있어요.

그래서 한 번에 모든 걸 가져오는 방식보다는, 문항과 자식 문항까지만 조인한 후, 개념 태그는 별도의 쿼리로 분리해서 조회하는 게 더 나을 것 같아요. 이렇게 하면 개념 태그 쿼리도 재사용하기 쉽고, concept_tag에 PK 인덱스가 잘 잡혀 있으니 성능도 더 좋아질 수 있어요.

다만, 개념 태그 쿼리가 문항마다 따로 나가는 건 비효율적이라 IN 절을 이용해서 한 번에 가져오는 방식으로 처리해야 할 것 같아요. 그리고 결과를 문항에 맞게 잘 매핑해주는 로직이 필요할 텐데, 이 부분은 따로 매퍼를 만들어서 정리해보면 좋을 것 같아요.

.where(problem.id.in(problemIds))
.distinct()
.transform(GroupBy.groupBy(problem.id).list(
Projections.constructor(ProblemSummaryResponse.class,
problem,
GroupBy.set(conceptTag.name)
)
));
}
}
42 changes: 15 additions & 27 deletions src/main/java/com/moplus/moplus_server/domain/problemset/service/ProblemSetGetService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,21 +3,14 @@
import com.moplus.moplus_server.admin.problemset.dto.response.ProblemSetGetResponse;
import com.moplus.moplus_server.admin.problemset.dto.response.ProblemSummaryResponse;
import com.moplus.moplus_server.admin.publish.domain.Publish;
import com.moplus.moplus_server.domain.concept.domain.ConceptTag;
import com.moplus.moplus_server.domain.concept.repository.ConceptTagRepository;
import com.moplus.moplus_server.domain.problem.domain.problem.Problem;
import com.moplus.moplus_server.domain.problem.repository.ProblemRepository;
import com.moplus.moplus_server.domain.problemset.domain.ProblemSet;
import com.moplus.moplus_server.domain.problemset.repository.ProblemSetGetRepositoryCustom;
import com.moplus.moplus_server.domain.problemset.repository.ProblemSetRepository;
import com.moplus.moplus_server.domain.publish.repository.PublishRepository;
import java.time.LocalDate;
import java.util.ArrayList;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
import java.util.Map;
import lombok.RequiredArgsConstructor;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;

Expand All @@ -26,10 +19,8 @@
public class ProblemSetGetService {

private final ProblemSetRepository problemSetRepository;
private final ProblemRepository problemRepository;
private final ConceptTagRepository conceptTagRepository;
private final PublishRepository publishRepository;
Logger log = LoggerFactory.getLogger(ProblemSetRepository.class);
private final ProblemSetGetRepositoryCustom problemSetGetRepositoryCustom;

@Transactional(readOnly = true)
public ProblemSetGetResponse getProblemSet(Long problemSetId) {
Expand All @@ -38,26 +29,23 @@ public ProblemSetGetResponse getProblemSet(Long problemSetId) {
List<LocalDate> publishedDates = publishRepository.findByProblemSetId(problemSetId).stream()
.map(Publish::getPublishedDate)
.toList();
List<Long> problemIds = problemSet.getProblemIds();
List<ProblemSummaryResponse> fetched = problemSetGetRepositoryCustom.findProblemSummariesWithTags(problemIds);

List<ProblemSummaryResponse> problemSummaries = new ArrayList<>();
for (Long problemId : problemSet.getProblemIds()) {
Problem problem = problemRepository.findByIdElseThrow(problemId);
Set<String> tagNames = new HashSet<>(
conceptTagRepository.findAllByIdsElseThrow(problem.getConceptTagIds())
.stream()
.map(ConceptTag::getName)
.toList());
problem.getChildProblems().stream()
.map(childProblem -> conceptTagRepository.findAllByIdsElseThrow(childProblem.getConceptTagIds()))
.forEach(conceptTags -> tagNames.addAll(conceptTags.stream().map(ConceptTag::getName).toList()));
problemSummaries.add(ProblemSummaryResponse.of(problem, tagNames));
}
return ProblemSetGetResponse.of(problemSet, publishedDates, problemSummaries);
// 순서 재정렬
Map<Long, ProblemSummaryResponse> mapped = fetched.stream()
.collect(java.util.stream.Collectors.toMap(ProblemSummaryResponse::getProblemId, p -> p));

List<ProblemSummaryResponse> orderedSummaries = problemIds.stream()
.map(mapped::get)
.filter(java.util.Objects::nonNull)
.toList();
Copy link
Contributor

Choose a reason for hiding this comment

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

정렬하는 로직은 어플리케이션보다 db가 잘할 수 있어요.
조회되는 문항이 많지 않으니 db에서 problemId로 정렬해서 가져오는건 어떨까요?
현재 DB에서 가져온걸 groupBy를 어플리케이션에서 하고 있는데 db에서 groupby를 하는게 좋을지 어플리케이션에서 할지도 고민해보면 좋을 것 같아요


return ProblemSetGetResponse.of(problemSet, publishedDates, orderedSummaries);
}

@Transactional(readOnly = true)
public List<ProblemSetGetResponse> getProblemSets(List<Long> problemSetIds) {
problemSetIds.forEach((id) -> log.atInfo().log("set id: " + id + "을 조회합니다."));
return problemSetIds.stream()
.map(this::getProblemSet)
.toList();
Expand Down
12 changes: 6 additions & 6 deletions ...est/java/com/moplus/moplus_server/domain/problemset/service/ProblemSetGetServiceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,8 @@ void setUp() {
assertThat(response).isNotNull();
assertThat(response.title()).isEqualTo("테스트 문항세트");
assertThat(response.problemSummaries()).hasSize(1);
assertThat(response.problemSummaries().get(0).problemCustomId()).isEqualTo("1224052001");
assertThat(response.problemSummaries().get(0).tagNames()).contains("미분 개념", "적분 개념");
assertThat(response.problemSummaries().get(0).getProblemCustomId()).isEqualTo("1224052001");
assertThat(response.problemSummaries().get(0).getTagNames()).contains("미분 개념", "적분 개념");
}

@Test
Expand Down Expand Up @@ -87,11 +87,11 @@ void setUp() {
assertThat(response.problemSummaries()).hasSize(2);

// 첫 번째 문제 검증
assertThat(response.problemSummaries().get(0).problemCustomId()).isEqualTo("1224052001");
assertThat(response.problemSummaries().get(0).tagNames()).contains("미분 개념", "적분 개념");
assertThat(response.problemSummaries().get(0).getProblemCustomId()).isEqualTo("1224052001");
assertThat(response.problemSummaries().get(0).getTagNames()).contains("미분 개념", "적분 개념");

// 두 번째 문제 검증
assertThat(response.problemSummaries().get(1).problemCustomId()).isEqualTo("1224052002");
assertThat(response.problemSummaries().get(1).tagNames()).contains("미분 개념", "삼각함수 개념");
assertThat(response.problemSummaries().get(1).getProblemCustomId()).isEqualTo("1224052002");
assertThat(response.problemSummaries().get(1).getTagNames()).contains("미분 개념", "삼각함수 개념");
}
}