[LIME-61] bucket - bucketItem - item 양방향 연관관계 제거 #29

HandmadeCloud · 2024-01-26T13:32:19Z

📌 PR 종류

어떤 종류의 PR인지 아래 항목 중에 체크 해주세요.

🐛 버그 수정
✨ 기능 추가
✅ 테스트 추가
🎨 코드 스타일 변경 (formatting, local variables)
🔨 리팩토링 (기능 변경 X)
💚 빌드 관련 수정
📝 문서 내용 수정
그 외, 어떤 종류인지 기입 바람:

📌 어떤 기능이 추가 되었나요?

bucket - bucketItem - item간 연관관계를 제거함

Issue Number

lime - 61

기능 설명

연관관계가 불필요한 상황이라고 생각해 양방향 연관관계를 제거했습니다.

이유1 : 객체의 생명주기가 item과 bucket이 일치하지 않는다.

bucket이 삭제될 때 item이 삭제되지않고, 저희 프로젝트 요구사항상 item은 삭제하지 않는 로직을 짜여있습니다.

이유2 : 강결합

jpa관점에서는 데이터를 들고오기 쉽고 조회가 쉽다는 장점이 있지만, 그만큼 강결합의 형태를 유지하고 있습니다.

여러 관점에서 살펴보았을 때 불편함을 가져가더라도 기술적인 분리를 하면서 유의미한 코드를 작성하는게 좋다고 생각합니다.

트러블 슈팅

querydsl이 영향을 받습니다. 일전에는 bucketItem -> bucket, bucket -> bucketItem이 연결된 형태여서 join을 bucketItem-item만 해주면 됩니다. 하지만, 이제는 3중 테이블 join해서 정보를 조회해와야만 합니다.

org.hibernate.query.semanticexception: could not interpret path expression in querydsl transform
매핑테이블 객체를 만들게 되면서, insert쿼리가 매핑테이블 개수만큼 나가게 됩니다. (예전에도 비슷하긴 함) 다음 과제로 쿼리 최적화를 시켜야할듯 합니다.(batch사이즈만큼 데이터를 한번에 쿼리할 수 있도록 개선하기 using hibernate, spring batch)

사이드이펙트

bucket수정을 위한 memberItem 커서 조회에 문제가 발생합니다. 현재 이 문제는 차후에 수정될 요구사항이므로(삭제될 수도 있는 메소드)
아직 수정하지는 않겠습니다.

📌 기존에 있던 기능에 영향을 주나요?

네
아니요

Yiseull

연관 관계 제거하는 거 복잡했을텐데 고생 많으셨습니다! 궁금한 점 코멘트로 달아두었어요😊

Yiseull · 2024-01-30T05:23:39Z

lime-domain/src/main/java/com/programmers/lime/domains/bucket/domain/Bucket.java

-	public void modifyBucket(
-		final BucketInfo bucketInfo,
-		final Long memberId
-	) {
-		this.memberId = Objects.requireNonNull(memberId);
+	public void modifyBucket(final BucketInfo bucketInfo) {
 		this.bucketInfo = bucketInfo;
 	}


p5;
bucket- bucketItem 연관 관계를 제거하는데 modifyBucket 메서드가 수정 된 이유를 모르겠습니다. 설명 부탁드립니당!

연관관계와 관련 없이 memberId가 수정될 가능성이 없어서 제거했습니다.
지난 코드에서 불필요한 인자였더라구요

Yiseull · 2024-01-30T05:28:57Z

...-domain/src/main/java/com/programmers/lime/domains/bucket/implementation/BucketAppender.java

+					BucketItem bucketItem = new BucketItem(itemId, bucketId);
+
+					return bucketItem;


p5;

return new BucketItem(itemId, bucketId);

이렇게 작성하는 것은 어떤가요??

d827982 반영 완!

Yiseull · 2024-01-30T05:36:24Z

...omain/src/test/java/com/programmers/lime/domains/bucket/implementation/BucketReaderTest.java

+		given(bucketItemRepository.findAllByBucketId(anyLong()))
+			.willReturn(BucketBuilder.buildBucketItems(bucketId,new ItemIdRegistry(Arrays.asList(1L,2L,3L,4L))));


p2;
willReturn 부분은 변수로 따로 빼면 가독성이 더 좋아질 것 같아요!

d827982 반영 완!

Yiseull · 2024-01-30T05:39:24Z

.../main/java/com/programmers/lime/domains/bucket/repository/BucketRepositoryForCursorImpl.java

-			.join(item).on(bucketItem.item.id.eq(item.id))
-			.where(bucketItem.bucket.id.in(bucketIds))
+			.join(item).on(bucketItem.itemId.eq(item.id))
+			.join(bucket).on(bucketItem.bucketId.eq(bucket.id))
+			.where(bucketItem.itemId.in(bucketIds))


p5;
새로 추가된 where 부분이 이해가 잘 가지 않아요! bucketItem의 itemId가 bucketIds 안에 존재하면 되는 건가요..? 혹시 변수명을 잘못 적은 건가요??

리팩토링 과정에서 변수를 잘못 적었던 것 같아요 결과가 정상적으로 나와서 캐치하지 못했네요 고마워요!
80e4f57 반영 완!

Curry4182

고생하셨습니다 저도 연관 관계 리팩토링 할 때 참고하겠습니다

Yiseull

고생하셨습니다👍 저도 연관관계 제거할 때 많이 참고하겠습니다!

HandmadeCloud added 7 commits January 26, 2024 22:18

refactor : bucket-item간 양방향 연관관계 제거

b0ef7c9

refactor : 양방향 연관관계 제거로 인한 repository 메소드 수정

60facd9

refactor : 양방향 연관관계 제거로 인한 구현 계층 수정

2d0b53b

test : 양방향 연관관계 제거로 인한 테스트코드 수정

53d1d7f

repository : 양방향 연관관계 제거로 인한 querydsl join 메소드 수정

1e70324

chore : 사용하지 않는 메소드 제거

2b1a1df

test : 연관관계 bucket쪽 변경사항으로 인해 실패하는테스트 임시 비활성화 처리

fac9e8f

HandmadeCloud added the refactor label Jan 26, 2024

HandmadeCloud requested review from Curry4182 and Yiseull January 26, 2024 13:32

HandmadeCloud self-assigned this Jan 26, 2024

Yiseull reviewed Jan 30, 2024

View reviewed changes

Curry4182 approved these changes Jan 30, 2024

View reviewed changes

HandmadeCloud and others added 3 commits January 30, 2024 19:16

chore : Appender 반환값, readerTest 변수 코드 정리

d827982

refactor : querydsl 변수 수정

80e4f57

Merge branch 'main' into LIME-61-bucket-remove-relation

d4f1516

Yiseull approved these changes Jan 31, 2024

View reviewed changes

HandmadeCloud merged commit 5e90869 into main Jan 31, 2024

Yiseull deleted the LIME-61-bucket-remove-relation branch February 28, 2024 11:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[LIME-61] bucket - bucketItem - item 양방향 연관관계 제거 #29

[LIME-61] bucket - bucketItem - item 양방향 연관관계 제거 #29

HandmadeCloud commented Jan 26, 2024

Yiseull left a comment

Yiseull Jan 30, 2024

HandmadeCloud Jan 30, 2024

Yiseull Jan 30, 2024

HandmadeCloud Jan 30, 2024

Yiseull Jan 30, 2024

HandmadeCloud Jan 30, 2024

Yiseull Jan 30, 2024

HandmadeCloud Jan 30, 2024 •

edited

Loading

Curry4182 left a comment

Yiseull left a comment

		BucketItem bucketItem = new BucketItem(itemId, bucketId);

		return bucketItem;

		given(bucketItemRepository.findAllByBucketId(anyLong()))
		.willReturn(BucketBuilder.buildBucketItems(bucketId,new ItemIdRegistry(Arrays.asList(1L,2L,3L,4L))));

[LIME-61] bucket - bucketItem - item 양방향 연관관계 제거 #29

[LIME-61] bucket - bucketItem - item 양방향 연관관계 제거 #29

Conversation

HandmadeCloud commented Jan 26, 2024

📌 PR 종류

📌 어떤 기능이 추가 되었나요?

Issue Number

기능 설명

이유1 : 객체의 생명주기가 item과 bucket이 일치하지 않는다.

이유2 : 강결합

트러블 슈팅

사이드이펙트

📌 기존에 있던 기능에 영향을 주나요?

Yiseull left a comment

Choose a reason for hiding this comment

Yiseull Jan 30, 2024

Choose a reason for hiding this comment

HandmadeCloud Jan 30, 2024

Choose a reason for hiding this comment

Yiseull Jan 30, 2024

Choose a reason for hiding this comment

HandmadeCloud Jan 30, 2024

Choose a reason for hiding this comment

Yiseull Jan 30, 2024

Choose a reason for hiding this comment

HandmadeCloud Jan 30, 2024

Choose a reason for hiding this comment

Yiseull Jan 30, 2024

Choose a reason for hiding this comment

HandmadeCloud Jan 30, 2024 • edited Loading

Choose a reason for hiding this comment

Curry4182 left a comment

Choose a reason for hiding this comment

Yiseull left a comment

Choose a reason for hiding this comment

HandmadeCloud Jan 30, 2024 •

edited

Loading