아이템 생성&수정, DayLog수정&ordinal 수정 쿼리 개선 #693

hgo641 · 2023-10-10T17:38:54Z

📄 Summary

크흠흠... DayLog수정&ordinal 수정 쿼리 개선 코드가 별로 안나옴 + 둘이 겹치는 코드 존재라는 이유로 같은 PR로 요청드리옵니다...^^ 양해부탁드립니다...^^

전반적으로 Image, Place, Expense의 CASCADE 옵션(Deleted만 남겨둠), orphanRemoval 삭제
- CASCADE 옵션 사용시 쿼리가 하나씩 나가기 때문에 전부 지우고 batch update를 하게 변경함
- CASCADE 옵션을 지움으로 인해, placeRepository.save(place)와 같이 Repository에서 직접 Image, Place, Expense를 저장하는 로직 추가됨

아이템 생성

쿼리 최대12개 -> 최대8개

DayLogRepository에서 findWithItemById 메서드 구현 - 아이템 생성시 ordinal을 알아오기 위해 DayLog의 아이템 리스트 사이즈가 필요함. DayLog와 Item테이블에 각각 쿼리가 날아가는 것을 막기 위해 Item과 fetch join을 해서 가져오는 메서드 생성
CustomImageRepositoryImpl의 saveAll메서드로 batch insert

아이템 수정

쿼리 최대 17개 -> 최대 10개

DayLogRepository에서 findWithItemDetailsById 메서드 구현 - DayLog를 조회해올때 Item, Place, Expense, Category, Image 전부 fetch join해서 함께 조회
ImageRepository에서 deleteAll 메서드로 이미지 리스트를 한 번에 삭제

DayLog 수정

쿼리 4개 -> 3개

DayLog와 Item을 각각 불러오던 것을 DayLogRepository의 findWithItemById로 한 번에 조회하게 변경

DayLog의 아이템 ordinal 변경

쿼리 최대 23개 -> 3개

DayLog와 Item을 각각 불러오던 것을 DayLogRepository의 findWithItemById로 한 번에 조회하게 변경 (3 -> 2)
CustomItemRepository에서 아이템 ordinal batch update하게 변경 (20 -> 1)

🙋🏻 More

개선 쿼리 상세 내용

아이템 생성 최대 쿼리

tripService.validateTripByMember로 인한 trip 조회 한 번
dayLog 조회
PlaceRequest의 apiCategory 조회
ExpenseRequest의 category 조회
insert place
insert expense
insert item
batch insert image

select t1_0.id from trip t1_0 where (t1_0.status = 'USABLE') and t1_0.member_id=1 and t1_0.id=3 limit 1;

select d1_0.id,d1_0.created_at,i1_0.day_log_id,i1_0.id,i1_0.created_at,i1_0.expense_id,i1_0.item_type,i1_0.memo,i1_0.modified_at,i1_0.ordinal,i1_0.place_id,i1_0.rating,i1_0.status,i1_0.title,d1_0.modified_at,d1_0.ordinal,d1_0.status,d1_0.title,d1_0.trip_id from day_log d1_0 left join item i1_0 on d1_0.id=i1_0.day_log_id and (i1_0.status = 'USABLE')  where (d1_0.status = 'USABLE') and d1_0.id=1 order by i1_0.ordinal;

select c1_0.id,c1_0.created_at,c1_0.eng_name,c1_0.kor_name,c1_0.modified_at,c1_0.status from category c1_0 where (c1_0.status = 'USABLE') and c1_0.eng_name in ('food','cafe');

select c1_0.id,c1_0.created_at,c1_0.eng_name,c1_0.kor_name,c1_0.modified_at,c1_0.status from category c1_0 where c1_0.id=200 and (c1_0.status = 'USABLE');

insert into expense (amount,category_id,created_at,currency,modified_at,status) values (10000.0,200,'2023-10-10T17:20:29.768+0900','krw','2023-10-10T17:20:29.768+0900','USABLE');

insert into place (category_id,created_at,latitude,longitude,modified_at,name,status) values (100,'2023-10-10T17:20:29.870+0900',38.1111111111111,38.1111111111111,'2023-10-10T17:20:29.870+0900','힐튼 호텔','USABLE');

insert into item (created_at,day_log_id,expense_id,item_type,memo,modified_at,ordinal,place_id,rating,status,title) values ('2023-10-10T17:20:29.749+0900',1,33,'SPOT','힐튼 호텔에서 숙박을 했습니다.','2023-10-10T17:20:29.749+0900',12,31,3.5,'USABLE','힐튼 호텔');

h.global.detector.QueryCounterAop        : LoggingForm(apiUrl=/trips/3/items, apiMethod=POST, queryCounts=7, queryTime=49)

아이템 수정 최대 쿼리

tripService.validateTripByMember로 인한 trip 조회 한 번
dayLog 조회
PlaceRequest의 apiCategory조회
insert place
insert expense
더 이상 사용하지 않는 이미지가 존재하면 delete where in
새로운 이미지가 존재하면 batch insert
아이템 update
사용하지 않는 Place가 존재하면 delete
사용하지 않는 Expense가 존재하면 delete

select t1_0.id from trip t1_0 where (t1_0.status = 'USABLE') and t1_0.member_id=1 and t1_0.id=3 limit 1;

select d1_0.id,d1_0.created_at,i1_0.day_log_id,i1_0.id,i1_0.created_at,e1_0.id,e1_0.amount,e1_0.category_id,c1_0.id,c1_0.created_at,c1_0.eng_name,c1_0.kor_name,c1_0.modified_at,c1_0.status,e1_0.created_at,e1_0.currency,e1_0.modified_at,e1_0.status,i2_0.item_id,i2_0.id,i2_0.created_at,i2_0.modified_at,i2_0.name,i2_0.status,i1_0.item_type,i1_0.memo,i1_0.modified_at,i1_0.ordinal,p1_0.id,p1_0.category_id,c2_0.id,c2_0.created_at,c2_0.eng_name,c2_0.kor_name,c2_0.modified_at,c2_0.status,p1_0.created_at,p1_0.latitude,p1_0.longitude,p1_0.modified_at,p1_0.name,p1_0.status,i1_0.rating,i1_0.status,i1_0.title,d1_0.modified_at,d1_0.ordinal,d1_0.status,d1_0.title,d1_0.trip_id from day_log d1_0 left join item i1_0 on d1_0.id=i1_0.day_log_id and (i1_0.status = 'USABLE')  left join image i2_0 on i1_0.id=i2_0.item_id and (i2_0.status = 'USABLE')  left join expense e1_0 on e1_0.id=i1_0.expense_id and (e1_0.status = 'USABLE') left join category c1_0 on c1_0.id=e1_0.category_id and (c1_0.status = 'USABLE') left join place p1_0 on p1_0.id=i1_0.place_id and (p1_0.status = 'USABLE') left join category c2_0 on c2_0.id=p1_0.category_id and (c2_0.status = 'USABLE') where (d1_0.status = 'USABLE') and d1_0.id=1 order by i1_0.ordinal;

select c1_0.id,c1_0.created_at,c1_0.eng_name,c1_0.kor_name,c1_0.modified_at,c1_0.status from category c1_0 where (c1_0.status = 'USABLE') and c1_0.eng_name in ('cafe','none');

insert into place (category_id,created_at,latitude,longitude,modified_at,name,status) values (122,'2023-10-11T03:48:07.294+0900',36.1111111111111,37.1111111111111,'2023-10-11T03:48:07.294+0900','변경dddd찐진찐마지막ㅇㅇㅇ이ㅡ아아악ㅇ!!!','USABLE');

insert into expense (amount,category_id,created_at,currency,modified_at,status) values (120.0,200,'2023-10-11T03:48:07.433+0900','krw','2023-10-11T03:48:07.433+0900','USABLE');

delete from image where id in (95,96);

update item set day_log_id=1,expense_id=54,item_type='SPOT',memo='맛있다!',modified_at='2023-10-11T03:48:07.572+0900',ordinal=13,place_id=49,rating=4.5,status='USABLE',title='이야아아dd아아아' where id=24;

UPDATE place SET status = 'DELETED' WHERE id = 48;

UPDATE expense SET status = 'DELETED' WHERE id = 50;

2023-10-11T03:48:07.668+09:00  INFO 4648 --- [nio-8080-exec-1] h.global.detector.QueryCounterAop        : LoggingForm(apiUrl=/trips/3/items/24, apiMethod=PUT, queryCounts=9, queryTime=28)

DayLog 수정

tripService.validateTripByMember로 인한 trip 조회 한 번
DayLog 조회
DayLog 제목 update

select t1_0.id from trip t1_0 where (t1_0.status = 'USABLE') and t1_0.member_id=1 and t1_0.id=3 limit 1;

select d1_0.id,d1_0.created_at,i1_0.day_log_id,i1_0.id,i1_0.created_at,i1_0.expense_id,i1_0.item_type,i1_0.memo,i1_0.modified_at,i1_0.ordinal,i1_0.place_id,i1_0.rating,i1_0.status,i1_0.title,d1_0.modified_at,d1_0.ordinal,d1_0.status,d1_0.title,d1_0.trip_id from day_log d1_0 left join item i1_0 on d1_0.id=i1_0.day_log_id and (i1_0.status = 'USABLE')  where (d1_0.status = 'USABLE') and d1_0.id=1 order by i1_0.ordinal;

update day_log set modified_at='2023-10-11T16:36:49.899+0900',ordinal=1,status='USABLE',title='내가진짜데이로그다!!!',trip_id=3 where id=1;

LoggingForm(apiUrl=/trips/3/daylogs/1, apiMethod=PATCH, queryCounts=3, queryTime=24)

DayLog의 아이템 ordinal 변경

tripService.validateTripByMember로 인한 trip 조회 한 번
DayLog 조회
아이템 ordinal batch update

select t1_0.id from trip t1_0 where (t1_0.status = 'USABLE') and t1_0.member_id=1 and t1_0.id=3 limit 1;

select d1_0.id,d1_0.created_at,i1_0.day_log_id,i1_0.id,i1_0.created_at,i1_0.expense_id,i1_0.item_type,i1_0.memo,i1_0.modified_at,i1_0.ordinal,i1_0.place_id,i1_0.rating,i1_0.status,i1_0.title,d1_0.modified_at,d1_0.ordinal,d1_0.status,d1_0.title,d1_0.trip_id from day_log d1_0 left join item i1_0 on d1_0.id=i1_0.day_log_id and (i1_0.status = 'USABLE')  where (d1_0.status = 'USABLE') and d1_0.id=2 order by i1_0.ordinal;

LoggingForm(apiUrl=/trips/3/daylogs/2/order, apiMethod=PATCH, queryCounts=2, queryTime=3)

더 개선할 부분 보이시면 코멘트plz
close #690
close #695

* feat: N+1 detector 적용 * feat: AOP 스프링 빈 등록 * chore: rebase develop * chore: p6spy 재적용 * refactor: info-appender logging 패턴 수정 * refactor: info-appender logging 패턴 개행 추가 * refactor: 패키지 위치 및 네이밍 변경 --------- Co-authored-by: mcodnjs <[email protected]>

github-actions · 2023-10-10T17:41:58Z

📝 Jacoco Test Coverage

Total Project Coverage	83.43%	🍏

File	Coverage [95.6%]
CustomImageRepositoryImpl.java	100%	🍏
Item.java	98.62%	🍏
Expense.java	96.83%	🍏
ItemService.java	93.96%	🍏
Image.java	90%	🍏

jjongwa

오우 고생하셨습니다 홍고
바뀐게 꽤 많네요..
일단 논의하고 싶은 사항 몇개 있어서 남겼습니다..

jjongwa · 2023-10-11T04:29:09Z

backend/src/main/java/hanglog/expense/domain/Expense.java

+    public boolean isDifferent(final Expense expense) {
+        if (expense == null) {
+            return true;
+        }
+        return !(Objects.equals(currency, expense.currency) && Objects.equals(amount, expense.amount)
+                && Objects.equals(category, expense.category));
+    }


여기서 매개변수로 Expense을 받지 말고,
curreny와 amount와 category를 받아서 비교하면 어떨까요?
인자의 null check는 서비스 단에서 하구요.!

Expense가 Expense을 받아서 비교하는게 살짝 어색하게 느껴집니다..

+그리고 getter 안쓰고 expense.currency 하면 인자로 받은 expense의 currency가 나오는건가요? 뭔가 당연한거 같으면서도 되게 어색하네요 왤케 낯설지....?

어떻게 개선할 지 고민해봤는데, 인자를 전부 받는 것보다 Expense자체를 받아오는 게 더 깔끔해서... 고민이 되는군요
차라리 isDifferent 메서드 대신 equals를 오버라이딩해서 사용하면 덜 어색하게 느껴질까요?
서비스에서 !equals(expense) 이렇게 !붙는 게 싫어서 isDifferent 메서드를 만들었는데, 어색하게 느껴진다면 차라리 equals를 그대로 사용하면 될까... 싶네요
물론 이 경우엔 Expense가 값객체처럼 되겠지만, Expense는 값객체로 사용해도 된다고 생각하기 때문에 . . .

아 그러게요..
전 equals 오버라이딩을 하는 편이 조금 더 익숙하게 느껴지는데,
이건 다른 분들의 의견도 들어보는게 좋을 것 같네요.!

expense..가계부가 아니라 item의 expense라면..값객체..Ok..

jjongwa · 2023-10-11T04:30:06Z

backend/src/main/java/hanglog/image/domain/repository/CustomImageRepositoryImpl.java

+        final String sql = """
+                    INSERT INTO image (created_at, modified_at, item_id, name, status) 
+                    VALUES (:createdAt, :modifiedAt, :itemId, :name, :status)
+                """;


jjongwa · 2023-10-11T04:39:34Z

backend/src/main/java/hanglog/image/domain/repository/ImageRepository.java

+    @Modifying
+    @Query("DELETE FROM Image image WHERE image IN :images")
+    void deleteAll(@Param("images") final List<Image> images);


지금 해당 로직에 대한 쿼리가
delete from image where id in (95,96) and (image.status = 'USABLE');
이렇게 나오는거죠?

image 객체를 넣어도 알아서 where절에 id로만 비교하는군요.. 신기하네요

그리고 image.status = 'USABLE' 조건이 추가되는건 custom이 아닌 JPA 레포라서 그런거 같은데,
굳이 where절에 index인 id로 검색하고 추가적으로 status를 확인하는게 불필요하다고 느껴지기도 합니다..

방법은 이걸 custom으로 바꾸는 건데 지금 보니까 imagRepository를 쓰는데가 ItemService밖에 없네요..?
그러면 그냥 custom으로 바꾸고 imageRepository를 삭제하는게 조금 더 깔끔할지도..?

jjongwa · 2023-10-11T04:41:08Z

backend/src/main/java/hanglog/trip/domain/repository/DayLogRepository.java

+    @Query("""
+            SELECT dayLog
+            FROM DayLog dayLog
+            LEFT JOIN FETCH dayLog.items items
+            WHERE dayLog.id = :dayLogId
+            """)
+    Optional<DayLog> findWithItemsById(@Param("dayLogId") final Long dayLogId);
+
+    @Query("""
+            SELECT dayLog
+            FROM DayLog dayLog
+            LEFT JOIN FETCH dayLog.items items
+            LEFT JOIN FETCH items.images images
+            LEFT JOIN FETCH items.expense expense
+            LEFT JOIN FETCH items.place place
+            LEFT JOIN FETCH expense.category expense_category
+            LEFT JOIN FETCH place.category place_category
+            WHERE dayLog.id = :dayLogId
+            """)
+    Optional<DayLog> findWithItemDetailsById(@Param("dayLogId") final Long dayLogId);


굉장합니다 정말

LJW25

궁금한게있어서 어프루브는 안하고 코멘트

LJW25 · 2023-10-12T01:35:48Z

backend/src/main/java/hanglog/image/domain/repository/CustomImageRepositoryImpl.java

+    @Override
+    public void deleteAll(final List<Image> images) {
+        final String sql = """
+                    DELETE FROM image WHERE id IN (:imageIds)
+                """;
+        namedParameterJdbcTemplate.update(sql, getDeletedImageIdsSqlParameterSources(images));
+    }


저 이거 제커밋에서 만들었는데 히히
근데 JPA 안쓰고 Custom으로 만든 이유가 있나요?
저는 JPA에서 쿼리짜서 구현했어서 궁금합니당

원래 JPA로 했었는데 위에서 디노가 말한 걸 듣고 설득당했습니다 😄
#693 (comment)

3자대면 하시죠. 지금 이대로면 제거랑 중복코드 됨 헤헤
일단 제 코드에서는 ImageRepository가 더 자연스럽기 때문에..전 ImageRepository 주장 예정..

mcodnjs

홍고홍고생하셨습니다 🍊🍊🍊🍊🍊🍊
ImageRepository은 조금 더 얘기해보는걸로!

mcodnjs · 2023-10-12T03:04:26Z

backend/src/main/java/hanglog/trip/domain/Item.java

필수는 아니고 .. 전부터 Item 생성자들 정리(==정팩메 or 삭제)하고 싶었는데 .. 🫠
빠른 정리가 가능하면 정리해주시면 감사하고 그렇지 않다면 다음 태스크로 넘깁시다 ㅎㅎ

mcodnjs · 2023-10-12T04:37:08Z

backend/src/main/java/hanglog/trip/service/ItemService.java

-        Place updatedPlace = item.getPlace();
-        if (itemUpdateRequest.getIsPlaceUpdated() || isChangedToSpot(itemUpdateRequest, item)) {
-            updatedPlace = makePlace(itemUpdateRequest.getPlace());
-        }
-
+        Place updatedPlace = makeUpdatedPlace(itemUpdateRequest, item);
        if (item.getItemType() == ItemType.SPOT && !itemUpdateRequest.getItemType()) {
            updatedPlace = null;
+            placeRepository.delete(item.getPlace());
        }


별건 아니고 .. ! 이런 로직은 어떨지 제안합니다 ..!
어렵네요 .. 한참생각함 (틀릴수도있음)

final Place updatedPlace = makeUpdatedPlace(itemUpdateRequest, item); private Place makeUpdatedPlace(final ItemUpdateRequest itemUpdateRequest, final Item item) { if (item.getItemType() == ItemType.NON_SPOT) { if (itemUpdateRequest.getItemType() && itemUpdateRequest.getIsPlaceUpdated()) { // non_spot -> spot return makePlace(itemUpdateRequest.getPlace()); } return null; // non_spot -> nothing } final Place originalPlace = item.getPlace(); if (item.getItemType() == ItemType.SPOT) { if (itemUpdateRequest.getItemType() && !itemUpdateRequest.getIsPlaceUpdated()) { // spot -> no update return originalPlace; } placeRepository.delete(originalPlace); if (itemUpdateRequest.getItemType() && itemUpdateRequest.getIsPlaceUpdated()) { return makePlace(itemUpdateRequest.getPlace()); // spot -> new spot } return null; // spot -> non_spot } }

jjongwa and others added 5 commits October 10, 2023 16:28

refactor: 여행 생성 쿼리 개선

93692d2

test: 메서드 이름 변경

b896f52

refactor: 아이템 수정 쿼리 개선

ba6d11b

fix: ItemUpdateRequest 값 변경

9a5bb10

hgo641 added 🔨 Refactor BE labels Oct 10, 2023

hgo641 requested review from LJW25, mcodnjs, jjongwa and waterricecake October 10, 2023 17:38

hgo641 self-assigned this Oct 10, 2023

hgo641 added 2 commits October 11, 2023 03:17

refactor: deleteAll 메서드 생성

58db0a9

refactor: 로직 분리 및 deleteNotUsedImage 분기 처리

b7664a3

jjongwa requested changes Oct 11, 2023

View reviewed changes

refactor: DayLog 수정, ordinal 수정 쿼리 개선

1019a0e

hgo641 changed the title ~~아이템 생성&수정 쿼리 개선~~ 아이템 생성&수정, DayLog수정&ordinal 수정 쿼리 개선 Oct 11, 2023

refactor: 이미지 deleteAll시 JdbcTemplate을 사용하게 변경

ae47c08

LJW25 reviewed Oct 12, 2023

View reviewed changes

jjongwa force-pushed the develop branch from 6b1addc to 384e078 Compare October 12, 2023 04:32

mcodnjs approved these changes Oct 12, 2023

View reviewed changes

hgo641 closed this Oct 13, 2023

hgo641 mentioned this pull request Oct 13, 2023

아이템 수정 & 데이로그 수정 & 아이템 oridinal 수정 재업로드 #710

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

아이템 생성&수정, DayLog수정&ordinal 수정 쿼리 개선 #693

아이템 생성&수정, DayLog수정&ordinal 수정 쿼리 개선 #693

hgo641 commented Oct 10, 2023 •

edited

Loading

github-actions bot commented Oct 10, 2023 •

edited

Loading

jjongwa left a comment

jjongwa Oct 11, 2023

hgo641 Oct 11, 2023

jjongwa Oct 12, 2023

LJW25 Oct 12, 2023

jjongwa Oct 11, 2023

jjongwa Oct 11, 2023

hgo641 Oct 11, 2023

jjongwa Oct 11, 2023

LJW25 left a comment

LJW25 Oct 12, 2023

hgo641 Oct 12, 2023

LJW25 Oct 12, 2023

mcodnjs left a comment

mcodnjs Oct 12, 2023

mcodnjs Oct 12, 2023

아이템 생성&수정, DayLog수정&ordinal 수정 쿼리 개선 #693

아이템 생성&수정, DayLog수정&ordinal 수정 쿼리 개선 #693

Conversation

hgo641 commented Oct 10, 2023 • edited Loading

📄 Summary

아이템 생성

아이템 수정

DayLog 수정

DayLog의 아이템 ordinal 변경

🙋🏻 More

개선 쿼리 상세 내용

아이템 생성 최대 쿼리

아이템 수정 최대 쿼리

DayLog 수정

DayLog의 아이템 ordinal 변경

github-actions bot commented Oct 10, 2023 • edited Loading

📝 Jacoco Test Coverage

jjongwa 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

LJW25 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

mcodnjs left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hgo641 commented Oct 10, 2023 •

edited

Loading

github-actions bot commented Oct 10, 2023 •

edited

Loading