카페 이미지 Fetch Join 시 Inner Join 하도록 변경 #468

nuyh99 · 2023-09-20T14:24:41Z

#️⃣ 연관된 이슈

카페가 이미지를 최소 1개 이상 포함하도록 한다. #467

📝 작업 내용

테스트 환경 설정에서 빠진 batch size 추가(팀시티에도 없더라구요...)
CafeRankGenerator 제거(Pageable의 offset 사용)
cafe.images.url 조인할 때 outer join -> inner join으로 변경 (패치 조인 가능)
- 이미지 최소 1개 이상 포함하도록 검증 로직 추가

green-kong

연어덕에 전반적으로 코드가 많이 좋아졌네요!
감사합니다!

green-kong · 2023-09-20T16:28:22Z

server/src/main/java/com/project/yozmcafe/domain/cafe/Cafe.java

+import jakarta.persistence.Column;
+import jakarta.persistence.Embedded;
+import jakarta.persistence.Entity;
+import jakarta.persistence.GeneratedValue;
+import jakarta.persistence.GenerationType;
+import jakarta.persistence.Id;


감사합니다!

green-kong · 2023-09-20T16:29:09Z

server/src/main/java/com/project/yozmcafe/domain/cafe/Cafe.java

+    public Cafe(final String name, final String address, final Images images, final Detail detail) {
        this(null, name, address, images, detail, 0);
    }


감사합니다2

green-kong · 2023-09-20T16:36:49Z

server/src/main/java/com/project/yozmcafe/domain/member/MemberRepository.java

    @Query("""
-            select m
-            from Member m
-            left join fetch m.unViewedCafes uvc
-            left join fetch uvc.cafe
-            where m.id = :id
+            SELECT m
+            FROM Member m
+            LEFT JOIN FETCH m.unViewedCafes uvc
+            LEFT JOIN FETCH uvc.cafe
+            WHERE m.id = :id
            """)
    Optional<Member> findById(@Param("id") String id);


흐음. 여기도 inner join 해도 되지 않나요???'
문제가 없는 부분이라 대문자 패치만 하신걸까요??

아 대문자 패치는 감사합니다!3

UnViewedCafe와 Cafe는 1:1 not null 관계이기 때문에 inner join이 좋을 것 같아요!

hum02 · 2023-09-21T06:47:33Z

server/src/main/java/com/project/yozmcafe/exception/ErrorCode.java

@@ -35,8 +32,6 @@ public enum ErrorCode {
    NOT_EXISTED_OAUTH_PROVIDER("O2", "잘못된 Provider Name 입니다."),
    NOT_EXISTED_OAUTH_CLIENT("O3", "일치하는 OAuthClient가 존재하지 않습니다."),

-    RANK_OUT_OF_BOUNDS("R1", format("좋아요 개수 랭킹 범위를 벗어나는 요청입니다. 좋아요 랭킹은 %d위까지 가능합니다. 요청의 페이지 확인이 필요합니다.", MAX_RANK)),
-


랭킹 범위 수정 감사합니다~

hum02 · 2023-09-21T06:50:29Z

server/src/main/resources/application-test.properties

@@ -22,3 +22,4 @@ spring.flyway.enabled=true
 spring.flyway.baseline-version=20230901153300
 spring.flyway.baseline-on-migrate=true
 spring.flyway.out-of-order=true
+spring.jpa.properties.hibernate.default_batch_fetch_size=1000


굿~ 테스트 쪽을 신경 못 썼네요..

hum02 · 2023-09-21T07:11:28Z

server/src/main/java/com/project/yozmcafe/domain/member/MemberRepository.java

+            SELECT m
+            FROM Member m
+            LEFT JOIN FETCH m.unViewedCafes uvc
+            JOIN FETCH uvc.cafe


헉 이 부분 때문에 테스트가 와장창 깨지는 것 같아요!
테스트에서 member를 찾지 못하고 있네요

이렇게 진행될 경우에 uvc가 없는 멤버는 조회되지 않을 것 같네요 ...

음 . . findbyid 메소드 자체를 오버로딩해서 사용해야 될 수도 있겠는데요 ?!

메서드를 따로 분리했습니다

donghae-kim

고생하셨어요 연어~

도치 폴로가 이미 좋은 코멘트를 남겨줬었네요!

현재 테스트 코드가 깨지고있는 fetch join 시 inner join에 대해서 같이 이야기하면서 해결하면 좋을 것 같아요! 일단 inner조인 처리를 하려는 이유가 궁금합니다! 또한 코멘트에 남겼듯이 지금과 같은 방식대로 진행하려면 메소드를 오버로딩 해서 처리해도 될 것 같은데 연어의 의견도 궁금하네용

donghae-kim · 2023-09-21T11:31:27Z

server/src/main/java/com/project/yozmcafe/domain/cafe/CafeRepository.java

+            SELECT c
+            FROM Cafe c
+            JOIN FETCH c.images.urls
+            WHERE c.id = :cafeId


👍

코멘트 달아주신

cafe.images.url 조인할 때 outer join -> inner join으로 변경 (패치 조인 가능)

에서 패치 조인 가능 이라는게 어떤 건가요? outer join -> inner join으로 변경해서 패치조인이 가능해졌다 ?

outer join -> inner join으로 변경해서 패치조인이 가능해졌다 ? 말이 맞습니다!

donghae-kim · 2023-09-21T11:33:31Z

server/src/main/java/com/project/yozmcafe/domain/cafe/Detail.java

@@ -22,7 +23,7 @@ public class Detail {
    public static final int PHONE_MAX_LENGTH = 20;

    @ElementCollection(fetch = FetchType.LAZY)


여기도 images 처럼 fetchtype 생략해도 되겠네요

donghae-kim · 2023-09-21T11:57:31Z

server/src/main/java/com/project/yozmcafe/domain/member/MemberRepository.java

+            SELECT m
+            FROM Member m
+            LEFT JOIN FETCH m.unViewedCafes uvc
+            JOIN FETCH uvc.cafe


donghae-kim · 2023-09-21T11:58:49Z

server/src/main/java/com/project/yozmcafe/service/CafeService.java

+            response.add(CafeRankResponse.of(++rank, cafe));
+        }
+
+        return response;


RankGenerator를 없애주셨군요 👍

donghae-kim · 2023-09-21T12:07:32Z

server/src/main/java/com/project/yozmcafe/domain/member/MemberRepository.java

+            SELECT m
+            FROM Member m
+            LEFT JOIN FETCH m.unViewedCafes uvc
+            JOIN FETCH uvc.cafe


이렇게 진행될 경우에 uvc가 없는 멤버는 조회되지 않을 것 같네요 ...

음 . . findbyid 메소드 자체를 오버로딩해서 사용해야 될 수도 있겠는데요 ?!

지도 기능 main merge

osiv main merge

…-log 불필요한 `console.log` 삭제

donghae-kim

고생했어요 연어

nuyh99 requested review from donghae-kim, green-kong and hum02 as code owners September 20, 2023 14:24

nuyh99 self-assigned this Sep 20, 2023

nuyh99 added the BE 개발은 백이징 label Sep 20, 2023

nuyh99 force-pushed the feat/467-inner-join branch from 515f684 to 7d5fbde Compare September 20, 2023 14:26

nuyh99 requested review from jeongwusi, geuntaek1013 and solo5star as code owners September 20, 2023 14:26

nuyh99 changed the title ~~Feat/467 inner join~~ 카페 이미지 Fetch Join 시 Inner Join 하도록 변경 Sep 20, 2023

green-kong approved these changes Sep 20, 2023

View reviewed changes

hum02 reviewed Sep 21, 2023

View reviewed changes

donghae-kim requested changes Sep 21, 2023

View reviewed changes

solo5star linked an issue Sep 22, 2023 that may be closed by this pull request

카페가 이미지를 최소 1개 이상 포함하도록 한다. #467

Closed

green-kong and others added 14 commits September 25, 2023 21:38

Merge pull request #484 from woowacourse-teams/feat/451-map

41b7486

지도 기능 main merge

refactor: import 정리

0e13f43

Merge branch 'main' into refactor/460-osiv

0a3b5ad

Merge pull request #487 from woowacourse-teams/refactor/460-osiv

6f1e6d5

osiv main merge

Merge pull request #483 from woowacourse-teams/fix/477-remove-console…

4836720

…-log 불필요한 `console.log` 삭제

feat: 테스트 때도 batch size 추가

0cbc612

feat: inner join으로 변경(nullable 객체 fetch join하기 위함)

cbbd7a4

feat: 컬렉션 타입 필드 null 방지

53d9b09

refactor: CafeRankGenerator -> Pageable의 offset 사용

072f435

feat: 카페 이미지 생성자 검증 기능 추가

fe8ef7f

feat: UnviewedCafe와 Cafe outer 조인 -> inner 조인으로 변경

33332c7

chore: 디폴트 설정 명시 제거

c4635fe

chore: 필요 없는 의존 제거

c8a063b

refactor: 필요한 부분만 fetch join 하도록 변경

8642078

nuyh99 force-pushed the feat/467-inner-join branch from f9bd513 to 8642078 Compare September 26, 2023 07:47

chore: resolve merge conflict

3198e3a

refactor: Cafe가 없는 Member는 어차피 left join해도 조회가 불가능하므로 inner join으로 변경

6dcdd30

donghae-kim approved these changes Sep 26, 2023

View reviewed changes

nuyh99 merged commit f4b6dad into dev Sep 26, 2023

nuyh99 deleted the feat/467-inner-join branch September 26, 2023 08:25

nuyh99 restored the feat/467-inner-join branch September 26, 2023 09:18

nuyh99 mentioned this pull request Sep 26, 2023

Offset 관련 에러 해결 #491

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

카페 이미지 Fetch Join 시 Inner Join 하도록 변경 #468

카페 이미지 Fetch Join 시 Inner Join 하도록 변경 #468

nuyh99 commented Sep 20, 2023

green-kong left a comment

green-kong Sep 20, 2023

green-kong Sep 20, 2023

green-kong Sep 20, 2023

nuyh99 Sep 26, 2023

hum02 Sep 21, 2023

hum02 Sep 21, 2023

hum02 Sep 21, 2023

donghae-kim Sep 21, 2023

donghae-kim Sep 21, 2023

nuyh99 Sep 26, 2023

donghae-kim left a comment

donghae-kim Sep 21, 2023

nuyh99 Sep 26, 2023

donghae-kim Sep 21, 2023

donghae-kim Sep 21, 2023

donghae-kim Sep 21, 2023

donghae-kim Sep 21, 2023

donghae-kim left a comment

		@@ -22,7 +23,7 @@ public class Detail {
		public static final int PHONE_MAX_LENGTH = 20;

		@ElementCollection(fetch = FetchType.LAZY)

카페 이미지 Fetch Join 시 Inner Join 하도록 변경 #468

카페 이미지 Fetch Join 시 Inner Join 하도록 변경 #468

Conversation

nuyh99 commented Sep 20, 2023

#️⃣ 연관된 이슈

📝 작업 내용

green-kong left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

donghae-kim left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

donghae-kim left a comment

Choose a reason for hiding this comment