OSIV false #461

donghae-kim · 2023-09-20T06:09:38Z

#️⃣ 연관된 이슈

close #460

📝 작업 내용

osiv를 false 설정하였습니다.

그 과정에서 LoginUser 라는 어노테이션이 생겼는데요. argumentResolver에서 repository 조회를 하지 않고 service레이어에서 진행하고자 이렇게 하였습니다.

또한 cafe.getImages.geturls의 lazyloading 강제 초기화를 .size를 통해 진행하였습니다. 더 좋은 방식이 있을까요 ? stream을 이용하려다가 그것보다는 size가 훨씬 성능이 좋다고 생각이 들어서요!

💬 리뷰 요구사항

LoginUser 어노테이션 , lazyloading객체 강제 초기화 방식

nuyh99

고생하셨습니다. 전반적으로 좋네요!

저라면 CafeMapper 등의 유틸을 만들 것 같습니다.
Approve~

nuyh99 · 2023-09-20T12:11:08Z

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

@@ -17,11 +18,13 @@
 public class LikedCafeService {

    private final CafeRepository cafeRepository;
+    private final MemberRepository memberRepository;


쓰이고 있는 의존성인가요...?!

ㅎㅎ.. 제거하였습니다!

nuyh99 · 2023-09-20T12:18:57Z

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

+                .map(cafe -> {
+                    cafe.getImages().getUrls().size();
+                    return LikedCafeResponse.from(cafe);
+                })
                .toList();


화제의 부분이군요!

DTO 측에서 뭔가 할 수는 없을까요??
이미 Cafe와 강하게 결합된 DTO에서 파싱하는 것이 좋을 것 같아요!

dto측에서 조회를 할 때 url을 getUrls를 할때 lazy강제 초기화를 진행시키도록 수정하였습니다.

다만 점점 한 문제를 해결하기위해서 일부분만 수정되는 모습이 현재까지 저희가 구현해온 코드를 해치는게 아닐까 라는 의문과 걱정이 드네요..

nuyh99 · 2023-09-20T12:21:20Z

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

+        return foundCafes.stream().map(cafe -> {
+            cafe.getImages().getUrls().size();
+            return CafeResponse.fromUnLoggedInUser(cafe);
+        }).toList();
    }


Cafe에서 다른 여러 DTO로 매핑해주는 매퍼로 분리되면 좋을 것 같습니다.
대부분의 서비스에서 이렇게 직접 파싱한다면 Cafe가 바뀌었을 때 할 일이 많을 것 같아요!

음 저도 비슷하게 생각합니다!

이부분에 대해서는 새로운 이슈를 파서 진행해도 좋을 것 같네요

nuyh99 · 2023-09-20T12:27:14Z

server/src/main/resources/application.properties

@@ -29,3 +29,5 @@ 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
+spring.jpa.open-in-view=false


nuyh99 · 2023-09-20T12:28:46Z

server/src/test/java/com/project/yozmcafe/controller/CafeControllerTest.java

@@ -367,7 +367,6 @@ private ResponseFieldsSnippet getCafeResponseFields() {
        );
    }



오 두 줄 개행 삭제 패치인가요?? 좋아요

nuyh99 · 2023-09-20T12:29:20Z

server/src/test/java/com/project/yozmcafe/service/CafeServiceTest.java

        final Cafe cafe1 = cafeRepository.save(Fixture.getCafe("카페1", "주소1", 10));
        final Cafe cafe2 = cafeRepository.save(Fixture.getCafe("카페2", "주소2", 11));
        member.addUnViewedCafes(Arrays.asList(cafe1, cafe2));
        member.updateLikedCafesBy(cafe1, true);
+        memberRepository.save(member);


cascade를 사용해서 잘 개편했군요

…/460-osiv

green-kong

고생하셨습니다!
결과는 심플하지만 많은 고민이 담겨있다는 걸 잘 알고있어서,
오션의 부단한 노력에 감사합니다!

컨벤션 관련한 코멘트만 몇개 남겼습니다!

green-kong · 2023-09-20T16:07:42Z

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

-        return foundCafes.stream()
-                .map(cafe -> CafeRankResponse.of(cafeRankGenerator.makeRank(foundCafes.indexOf(cafe), pageable), cafe))
-                .toList();
+        return foundCafes.stream().map(cafe -> CafeRankResponse.of(cafeRankGenerator.makeRank(foundCafes.indexOf(cafe), pageable), cafe)).toList();


엥 개행이 사라졌네요??

헉 수정하였습니다 왜사라졌지 ;;;;

green-kong · 2023-09-20T16:08:06Z

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

    private final UnViewedCafeService unViewedCafeService;
    private final CafeRankGenerator cafeRankGenerator;

-    public CafeService(final CafeRepository cafeRepository, final UnViewedCafeService unViewedCafeService, final CafeRankGenerator cafeRankGenerator) {
+    public CafeService(final CafeRepository cafeRepository, final MemberService memberService, final UnViewedCafeService unViewedCafeService, final CafeRankGenerator cafeRankGenerator) {


여기도 파리미터 마다 개행 해주면 좋을것 같아요!

왜자꾸 달라지지 ............죄송합니다 수정했습니다

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

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

        final Cafe cafe = cafeRepository.findById(cafeId)
                .orElseThrow(() -> new IllegalArgumentException("해당하는 카페가 존재하지 않습니다."));



아니 이게 뭐야;; 이걸 이제 봤네요;;

Suggested change

final Cafe cafe = cafeRepository.findById(cafeId)

.orElseThrow(() -> new IllegalArgumentException("해당하는 카페가 존재하지 않습니다."));

final Cafe cafe = cafeRepository.findById(cafeId)

.orElseThrow(() -> new BadRequestException(NOT_EXISTED_CAFE));

요렇게 수정 부탁드려도 될까요~?

green-kong · 2023-09-20T16:21:43Z

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

@@ -24,7 +25,7 @@ public Images(List<String> urls) {
    }

    public List<String> getUrls() {
-        return urls;
+        return new ArrayList<>(urls);


나왔던 여러가지 방법 중 이게 가장 심플하고 좋네요!

더 좋은 방법이 있으면 좋을 것 같기는 해요.

점점 한 문제를 해결하기위해서 일부분만 수정되는 모습이 현재까지 저희가 구현해온 코드를 해치는게 아닐까 라는 의문과 걱정이 들어서요 !

green-kong · 2023-09-21T00:20:25Z

아니 이거 왜 코멘트 두번달리지;;

hum02

좋아요~~

hum02 · 2023-09-21T06:25:00Z

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

    private final UnViewedCafeService unViewedCafeService;
    private final CafeRankGenerator cafeRankGenerator;

-    public CafeService(final CafeRepository cafeRepository, final UnViewedCafeService unViewedCafeService, final CafeRankGenerator cafeRankGenerator) {
+    public CafeService(final CafeRepository cafeRepository, final MemberService memberService, final UnViewedCafeService unViewedCafeService, final CafeRankGenerator cafeRankGenerator) {


굿~ 혹시나 MemberService에서 다른 service를 참조해서 순환참조가 일어나지 않을까? 했는데 아니네요. MemberService는 여러 service에서 참조되고 있어서 다른 참조가 없도록 유지하는 게 좋은 것 같아요

donghae-kim added 5 commits September 19, 2023 22:08

refactor: argumentResolver 반환값 수정 및 커스텀 어노테이션 추가

d404a3a

test: test 수정

081941c

refactor: cafe urls 강제 초기화

32e0402

refactor: member조회 로직 수정

4190d07

refactor: 코드 리팩토링

39cf630

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

Merge branch 'dev' into refactor/460-osiv

adc5f68

nuyh99 approved these changes Sep 20, 2023

View reviewed changes

donghae-kim added 2 commits September 20, 2023 22:04

refactor: urls 프록시 객체 강제 초기화 수정

486f196

Merge remote-tracking branch 'origin/refactor/460-osiv' into refactor…

ddc82df

…/460-osiv

green-kong approved these changes Sep 20, 2023

View reviewed changes

hum02 approved these changes Sep 21, 2023

View reviewed changes

donghae-kim added 3 commits September 21, 2023 21:22

style: 코드 포멧팅 수정

a89e4cd

refactor: 예외 로직 수정

e0ea147

test: exception 변경에 따른 test 수정

dc2701d

donghae-kim added the BE 개발은 백이징 label Sep 25, 2023

Merge branch 'dev' into refactor/460-osiv

b6f77e7

donghae-kim merged commit 31a436a into dev Sep 26, 2023
1 check passed

donghae-kim deleted the refactor/460-osiv branch September 26, 2023 03:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

OSIV false #461

OSIV false #461

donghae-kim commented Sep 20, 2023

nuyh99 left a comment

nuyh99 Sep 20, 2023

donghae-kim Sep 21, 2023

nuyh99 Sep 20, 2023

donghae-kim Sep 21, 2023

nuyh99 Sep 20, 2023

donghae-kim Sep 21, 2023

nuyh99 Sep 20, 2023

nuyh99 Sep 20, 2023

nuyh99 Sep 20, 2023

green-kong left a comment

green-kong Sep 20, 2023

donghae-kim Sep 21, 2023

green-kong Sep 20, 2023

donghae-kim Sep 21, 2023

green-kong Sep 20, 2023

green-kong Sep 20, 2023

donghae-kim Sep 21, 2023

green-kong commented Sep 21, 2023

hum02 left a comment

hum02 Sep 21, 2023

		@@ -367,7 +367,6 @@ private ResponseFieldsSnippet getCafeResponseFields() {
		);
		}

		final Cafe cafe = cafeRepository.findById(cafeId)
		.orElseThrow(() -> new IllegalArgumentException("해당하는 카페가 존재하지 않습니다."));

OSIV false #461

OSIV false #461

Conversation

donghae-kim commented Sep 20, 2023

#️⃣ 연관된 이슈

📝 작업 내용

💬 리뷰 요구사항

nuyh99 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

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

green-kong commented Sep 21, 2023

hum02 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment