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

쿼리 추가 발생 문제 해결 #798

Merged
merged 9 commits into from
Nov 25, 2024
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
package mouda.backend.darakbang.infrastructure;

import java.util.List;
import java.util.Optional;

import org.springframework.data.jpa.repository.JpaRepository;
import org.springframework.data.jpa.repository.Query;
import org.springframework.stereotype.Repository;

import mouda.backend.darakbang.domain.Darakbang;
Expand All @@ -15,4 +17,11 @@ public interface DarakbangRepository extends JpaRepository<Darakbang, Long> {
boolean existsByCode(String code);

Optional<Darakbang> findByCode(String code);

@Query("""
SELECT d
FROM Darakbang d
WHERE d.id IN :darakbangIds
""")
List<Darakbang> findAllByIds(List<Long> darakbangIds);
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

import lombok.RequiredArgsConstructor;
import mouda.backend.darakbang.domain.Darakbang;
import mouda.backend.darakbang.infrastructure.DarakbangRepository;
import mouda.backend.darakbangmember.domain.DarakBangMemberRole;
import mouda.backend.darakbangmember.domain.DarakbangMember;
import mouda.backend.darakbangmember.domain.DarakbangMembers;
Expand All @@ -21,6 +22,7 @@
public class DarakbangMemberFinder {

private final DarakbangMemberRepository darakbangMemberRepository;
private final DarakbangRepository darakbangRepository;

public DarakbangMember find(Darakbang darakbang, Member member) {
return darakbangMemberRepository.findByDarakbangIdAndMemberId(darakbang.getId(), member.getId())
Expand All @@ -29,10 +31,8 @@ public DarakbangMember find(Darakbang darakbang, Member member) {
}

public List<Darakbang> findAllByMember(Member member) {
return darakbangMemberRepository.findAllByMemberId(member.getId())
.stream()
.map(DarakbangMember::getDarakbang)
.toList();
List<Long> darakbangIds = darakbangMemberRepository.findAllDarakbangIdsByMemberId(member.getId());
return darakbangRepository.findAllByIds(darakbangIds);
}

public DarakbangMembers findAllDarakbangMembers(Long darakbangId) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,20 @@
import java.util.Optional;

import org.springframework.data.jpa.repository.JpaRepository;
import org.springframework.data.jpa.repository.Query;
import org.springframework.stereotype.Repository;

import mouda.backend.darakbangmember.domain.DarakbangMember;

@Repository
public interface DarakbangMemberRepository extends JpaRepository<DarakbangMember, Long> {

List<DarakbangMember> findAllByMemberId(long memberId);
@Query("""
SELECT dm.darakbang.id
FROM DarakbangMember dm
WHERE dm.memberId = :memberId
""")
List<Long> findAllDarakbangIdsByMemberId(long memberId);

Optional<DarakbangMember> findByDarakbangIdAndMemberId(Long darakbangId, Long id);

Expand Down
3 changes: 2 additions & 1 deletion backend/src/main/java/mouda/backend/moim/domain/Moim.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import jakarta.persistence.Id;
import jakarta.persistence.Table;
import lombok.Builder;
import lombok.EqualsAndHashCode;
import lombok.Getter;
import lombok.NoArgsConstructor;
import mouda.backend.moim.exception.MoimErrorMessage;
Expand All @@ -25,6 +26,7 @@
@Table(name = "moim")
@Getter
@NoArgsConstructor
@EqualsAndHashCode
Copy link
Contributor

Choose a reason for hiding this comment

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

요건 id를 기준으로 동등성 비교를 하나요?
아님 모든 값으로 동등성 비교하나요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

모든 값으로 비교합니당 제외하고 싶은 필드나 지정하고 싶은 필드가 있으면 exclude, of 에 설정하면 됩니닷

public class Moim {

private static final int TITLE_MAX_LENGTH = 30;
Expand Down Expand Up @@ -215,7 +217,6 @@ public boolean isCompleted() {
return moimStatus == MoimStatus.COMPLETED;
}


public boolean isMoiming() {
return moimStatus == MoimStatus.MOIMING;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@ public class MoimOverview {
private final int currentPeople;
private final boolean isZzimed;

public MoimOverview(Moim moim, int currentPeople, boolean isZzimed) {
public MoimOverview(Moim moim, long currentPeople, boolean isZzimed) {
this.moim = moim;
this.currentPeople = currentPeople;
this.currentPeople = (int)currentPeople;
this.isZzimed = isZzimed;
}
}
5 changes: 3 additions & 2 deletions backend/src/main/java/mouda/backend/moim/domain/Zzim.java
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package mouda.backend.moim.domain;

import jakarta.persistence.Entity;
import jakarta.persistence.FetchType;
import jakarta.persistence.GeneratedValue;
import jakarta.persistence.GenerationType;
import jakarta.persistence.Id;
Expand All @@ -22,11 +23,11 @@ public class Zzim {
@GeneratedValue(strategy = GenerationType.IDENTITY)
private Long id;

@ManyToOne
@ManyToOne(fetch = FetchType.LAZY)
@JoinColumn(nullable = false)
private Moim moim;

@ManyToOne
@ManyToOne(fetch = FetchType.LAZY)
@JoinColumn(nullable = false)
private DarakbangMember darakbangMember;

Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
package mouda.backend.moim.implement.finder;

import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.function.Predicate;
import java.util.stream.Collectors;

import org.springframework.http.HttpStatus;
import org.springframework.stereotype.Component;
Expand All @@ -19,6 +22,7 @@
import mouda.backend.moim.infrastructure.ChamyoRepository;
import mouda.backend.moim.infrastructure.MoimRepository;
import mouda.backend.moim.infrastructure.ZzimRepository;
import mouda.backend.moim.infrastructure.dto.ChamyoMoim;

@Component
@RequiredArgsConstructor
Expand All @@ -38,17 +42,25 @@ public Moim read(long moimId, long currentDarakbangId) {
}

public List<MoimOverview> readAll(long darakbangId, DarakbangMember darakbangMember) {
return moimRepository.findAllByDarakbangIdOrderByIdDesc(darakbangId).stream()
.map(moim -> createMoimOverview(moim, darakbangMember))
.toList();
List<Moim> moims = moimRepository.findAllByDarakbangIdOrderByIdDesc(darakbangId);
List<ChamyoMoim> chamyoMoims = chamyoRepository.findAllByMoims(moims);
Set<Long> zzimedMoimIds = zzimRepository.findAllByDarakbangMemberId(darakbangMember.getId());
Copy link
Contributor

Choose a reason for hiding this comment

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

이게 노션에 남겨주신 방법 1인 것 같네요 ~ 개인적으로는 모임이 '찜'을 아는게 크게 문제가 되는 일인가 싶고, 이 과정에서 메서드 흐름이 더 복잡하게 느껴지는 것 같은데요 ! 저는 join을 쓰는 2번이 더 좋은 방법 같습니닷

Copy link
Contributor Author

Choose a reason for hiding this comment

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

흑흑 하지만 이건 재사용할 수 있어요 흑흑

Copy link
Contributor

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.

ㅋㅋㅋㅋㅋㅋ 더 열심히 설득하면 반영할 의향 있어요 😋


return createMoimOverview(moims, chamyoMoims, zzimedMoimIds);
Copy link
Contributor

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.

맞아용 ...ㅠㅅㅠ 이 부분은 할까말까 하다가 찜한모임목록, 나의모임목록 조회 시 동일한 메서드(findAll..., createMoimOverview)를 쓸 수 있길래 일단 수정한 상태입니닷

안나가 해결한 방식과 거의 동일해서 안나거 먼저 머지하고 충돌해결해도 괜찮을 거 같은데 어떠신가여 ?

}

public List<MoimOverview> readAllMyMoim(DarakbangMember darakbangMember, FilterType filterType) {
return chamyoRepository.findAllByDarakbangMemberIdOrderByIdDesc(darakbangMember.getId()).stream()
.map(Chamyo::getMoim)
List<ChamyoMoim> chamyoMoims = chamyoRepository.findAllByDarakbangMemberId(darakbangMember.getId());
Set<Long> moimIds = chamyoMoims.stream()
.map(ChamyoMoim::getMoimId)
.collect(Collectors.toSet());
Copy link
Contributor

Choose a reason for hiding this comment

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

🤔 내 모임을 조회할 때도 순서대로 정렬하는게 있었던 것 같은데요 ~! Set으로 했을 때 순서가 보장이 될까요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

여기는 순서 보장이 필요없어서 Set으로 했습니당! response는 아래에 List 순서에 따라 매핑합니닷!

List<Moim> moims = moimRepository.findAllByIds(moimIds)
.stream()
.filter(getFilter(filterType))
.map(moim -> createMoimOverview(moim, darakbangMember))
.toList();
Set<Long> zzimedMoimIds = zzimRepository.findAllByDarakbangMemberId(darakbangMember.getId());

return createMoimOverview(moims, chamyoMoims, zzimedMoimIds);
}

private Predicate<Moim> getFilter(FilterType filterType) {
Expand All @@ -62,16 +74,25 @@ private Predicate<Moim> getFilter(FilterType filterType) {
}

public List<MoimOverview> readAllZzimedMoim(DarakbangMember darakbangMember) {
return zzimRepository.findAllByDarakbangMemberIdOrderByIdDesc(darakbangMember.getId()).stream()
.map(zzim -> createMoimOverview(zzim.getMoim(), darakbangMember))
.toList();
Set<Long> zzimMoimIds = zzimRepository.findAllByDarakbangMemberId(darakbangMember.getId());
List<Moim> moims = moimRepository.findAllByIds(zzimMoimIds);
List<ChamyoMoim> chamyoMoims = chamyoRepository.findAllByMoims(moims);

return createMoimOverview(moims, chamyoMoims, zzimMoimIds);
}

private MoimOverview createMoimOverview(Moim moim, DarakbangMember darakbangMember) {
int currentPeople = countCurrentPeople(moim);
boolean isZzimed = zzimFinder.isMoimZzimedByMember(moim.getId(), darakbangMember);
private List<MoimOverview> createMoimOverview(List<Moim> moims, List<ChamyoMoim> chamyoMoims,
Set<Long> zzimedMoimIds) {
Map<Long, Long> chamyoMap = chamyoMoims.stream()
.collect(Collectors.toMap(ChamyoMoim::getMoimId, ChamyoMoim::getChamyoCount));
Copy link
Contributor

Choose a reason for hiding this comment

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

바로 아래에서는 '현재 참여중인 인원'을 currentPeople로 표현했는데 여기서는 chamyoCount로 표현하고 있네요 ~ ㅎㅎ 둘중 하나로 통일하면 좋겠습니다.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

헤헤 넹 !!


return new MoimOverview(moim, currentPeople, isZzimed);
return moims.stream()
.map(moim -> new MoimOverview(
moim,
chamyoMap.getOrDefault(moim.getId(), 0L),
Copy link
Contributor

Choose a reason for hiding this comment

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

여기까지의 흐름을 보면,

  1. 찜으로 모임을 조회
  2. 모임으로 참여를 조회(List -> List

의 흐름인데, 그러면 여기서의 moim에 대한 chamyoMap의 count는 반드시 존재해야 하는 값인 것 같아요 ~ 이게 없으면 0으로 넣을게 아니라 오류가 나는게 조금 더 타당한 것 같습니다.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

어랍쇼 ?? 그렇네요 !! 왜 저는 그 생각을 못했을까요 ㅋㅋ 감사합니다!!


그런데 목록이다 보니 하나 오류났다고 전체 조회를 못하는 건 곤란할 거 같아요. 참여인원이 없다면 제외하고 예외는 던지지 않도록 하겠습니다!

Copy link
Contributor

Choose a reason for hiding this comment

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

어랍쇼 ?? 그렇네요 !! 왜 저는 그 생각을 못했을까요 ㅋㅋ 감사합니다!!

그런데 목록이다 보니 하나 오류났다고 전체 조회를 못하는 건 곤란할 거 같아요. 참여인원이 없다면 제외하고 예외는 던지지 않도록 하겠습니다!

좋은 방법이네요~~ 😄

zzimedMoimIds.contains(moim.getId())
))
.toList();
}

public int countCurrentPeople(Moim moim) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

import mouda.backend.moim.domain.Chamyo;
import mouda.backend.moim.domain.Moim;
import mouda.backend.moim.infrastructure.dto.ChamyoMoim;

public interface ChamyoRepository extends JpaRepository<Chamyo, Long> {

Expand Down Expand Up @@ -36,4 +37,32 @@ public interface ChamyoRepository extends JpaRepository<Chamyo, Long> {

@Query("SELECT c FROM Chamyo c WHERE c.moim.id = :moimId AND c.moimRole = 'MOIMER'")
Optional<Chamyo> findMoimerByMoimId(@Param("moimId") Long moimId);

@Query("""
SELECT new mouda.backend.moim.infrastructure.dto.ChamyoMoim(
c.moim.id,
(SELECT COUNT(c2)
FROM Chamyo c2
WHERE c2.moim = c.moim)
)
FROM Chamyo c
WHERE c.darakbangMember.id = :darakbangMemberId
GROUP BY c.moim
ORDER BY c.moim.id DESC
""")
List<ChamyoMoim> findAllByDarakbangMemberId(long darakbangMemberId);
Copy link
Contributor

Choose a reason for hiding this comment

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

ChamyoMoim을 보면 dto 패키지에 정의했던데, 레포에서 DTO를 꺼내는건 기존 컨벤션과 어긋나지 않나요? 🙃

Copy link
Contributor Author

Choose a reason for hiding this comment

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

음 이 dto는 insfrastructure <-> service 범위에서 사용되길래 괜찮지 않을까 했습니다 🤔 그런데 상돌말도 일리가 있네요
중복코드를 제거하고 불필요한 쿼리를 줄이다가 여기서 moimId와 currentPeople을 담은 클래스를 반환해야 가장 깔끔해서 해당 방법을 선택했어요 . 다른 방법은 잘 .. 모르겠네요 ㅠ

혹시 domain 패키지에 ChamyoMoim을 넣으면 좀 컨벤션이 맞춰질까요 ?ㅎ

Copy link
Contributor

Choose a reason for hiding this comment

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

음 이 dto는 insfrastructure <-> service 범위에서 사용되길래 괜찮지 않을까 했습니다 🤔 그런데 상돌말도 일리가 있네요 중복코드를 제거하고 불필요한 쿼리를 줄이다가 여기서 moimId와 currentPeople을 담은 클래스를 반환해야 가장 깔끔해서 해당 방법을 선택했어요 . 다른 방법은 잘 .. 모르겠네요 ㅠ

혹시 domain 패키지에 ChamyoMoim을 넣으면 좀 컨벤션이 맞춰질까요 ?ㅎ

기존 코드를 보면 레이어와 상관없이 DTO를 반환하는게 없었어서요~ 한번 고민해 보시죠!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ㅎㅎ 일단 눈속임으로 domain에 넣었어요 ^^,,


@Query("""
SELECT new mouda.backend.moim.infrastructure.dto.ChamyoMoim(
c.moim.id,
(SELECT COUNT(c2)
FROM Chamyo c2
WHERE c2.moim = c.moim)
)
FROM Chamyo c
WHERE c.moim IN :moims
GROUP BY c.moim
ORDER BY c.moim.id DESC
""")
List<ChamyoMoim> findAllByMoims(List<Moim> moims);
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,13 @@

import java.util.List;
import java.util.Optional;
import java.util.Set;

import org.springframework.data.jpa.repository.JpaRepository;
import org.springframework.data.jpa.repository.Modifying;
import org.springframework.data.jpa.repository.Query;
import org.springframework.data.repository.query.Param;

import mouda.backend.darakbang.domain.Darakbang;
import mouda.backend.moim.domain.Moim;
import mouda.backend.moim.domain.MoimStatus;

Expand All @@ -33,4 +33,11 @@ public interface MoimRepository extends JpaRepository<Moim, Long> {

boolean existsByIdAndDarakbangId(Long moimId, Long darakbangId);

@Query("""
SELECT m
FROM Moim m
WHERE m.id IN :moimIds
ORDER BY m.id DESC
""")
List<Moim> findAllByIds(Set<Long> moimIds);
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@

import java.util.List;
import java.util.Optional;
import java.util.Set;

import org.springframework.data.jpa.repository.JpaRepository;
import org.springframework.data.jpa.repository.Query;

import mouda.backend.moim.domain.Zzim;

Expand All @@ -14,4 +16,13 @@ public interface ZzimRepository extends JpaRepository<Zzim, Long> {
Optional<Zzim> findByMoimIdAndDarakbangMemberId(Long moimId, Long darakbangMemberId);

List<Zzim> findAllByDarakbangMemberIdOrderByIdDesc(Long darakbangMemberId);

@Query("""
SELECT
z.moim.id
FROM Zzim z
WHERE z.darakbangMember.id = :darakbangMemberId
ORDER BY z.id DESC
""")
Set<Long> findAllByDarakbangMemberId(long darakbangMemberId);
Copy link
Contributor

Choose a reason for hiding this comment

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

이건 헷갈릴 수 있는 여지가 있겠는데요 ~ ZzimRepository에서의 findAl이면 일반적으로는 Collection을 기대할 것 같아요 !

Copy link
Contributor Author

Choose a reason for hiding this comment

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

나이스 눈썰미 👀

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package mouda.backend.moim.infrastructure.dto;

import lombok.AllArgsConstructor;
import lombok.EqualsAndHashCode;
import lombok.Getter;

@Getter
@EqualsAndHashCode
@AllArgsConstructor
public class ChamyoMoim {
Copy link
Contributor

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.

private final long moimId;
private final long chamyoCount;
}