[JDBC 4단계] 민트(유재민) 미션 제출합니다. #458

yujamint · 2023-10-05T18:01:11Z

안녕하세요 허브🌿~ 오랜만에 돌아온 민트입니다!
추석 연휴 때 시간이 널널했어서 자주 티키타카하다가 연휴가 끝나니 오랜만에 요청하는 느낌이네요 ㅋㅋㅋ
이번 단계에서 중점적으로 구현한 것은 다음과 같습니다.

요구사항에 맞춰, 기존의 TransactionContext를 제거하고 TransactionSynchronizationManager 적용하기
DataSourceUtils를 활용하여 Connection 생성/제거 및 예외처리의 책임을 분리
트랜잭션 서비스 추상화
TransactionCallback.execute() 제네릭 타입 반환하도록 리팩토링

1,2,3단계 전부 허브의 도움을 많이 받은 거 같아요. 혼자서라면 생각하지 못했을 부분에 대해서 생각할 기회를 주셔서 감사합니다 ㅎㅎ
이번 단계에서도 많이 배우겠습니다. 잘 부탁드립니다!

greeng00se

안녕하세요 민트🌿 오랜만입니다 😄

코드 꼼꼼하게 잘 작성해주신 부분 잘 봤습니다!
저도 민트의 코드를 리뷰하며 너무 많이 배우는 것 같습니다 🙇
꼼꼼히 코드를 봐야하는데 약간 새벽이라 비몽사몽하네요~~

미션 종료까지 시간이 조금 남은 것 같아, 깊게 고민해볼 수 있는 + 변경사항이 많을 수도 있는 + 같이 이야기 나눠볼 수 있는 코멘트를 ConnectionManager 클래스에 하나 남겨봤어요.
해당 코멘트와 함께 다음 요구사항을 보니 JdbcTemplate에서는 DataSourceUtils를 이용해서 Connection을 받아야할 것 같아요.

서비스와 DAO에서 Connection 객체를 가져오는 부분은 DataSourceUtils를 사용하도록 수정하자.

이야기를 더 나누고 싶은 부분은 코멘트나 DM으로 언제든지 남겨주세요.

greeng00se · 2023-10-05T18:06:03Z

app/src/main/java/com/techcourse/service/AppUserService.java

+    public AppUserService(final UserDao userDao, final UserHistoryDao userHistoryDao) {
+        this.userDao = userDao;
+        this.userHistoryDao = userHistoryDao;
+


R: 빈칸은 없어도 될 것 같습니다!

greeng00se · 2023-10-05T18:07:00Z

app/src/main/java/com/techcourse/service/TxUserService.java

+    public TxUserService(final UserService userService) {
+        this.userService = userService;
+        final DataSource dataSource = DataSourceConfig.getInstance();
+        this.transactionTemplate = new TransactionTemplate(dataSource);
+    }


C: UserService와 TransactionTemplate을 받는 생성자로 만들어보는건 어떨까요?

외부에서 주입한 TransactionTemplate(DataSource)를 사용하는 것이 더 좋은 방법임에 동의합니다!

greeng00se · 2023-10-05T18:07:47Z

app/src/main/java/com/techcourse/service/TxUserService.java

+
+    @Override
+    public User findById(final long id) {
+        return transactionTemplate.executeWithTransaction(() -> userService.findById(id));


크.. 나머지 메서드들도 트랜잭션 다 깔끔하게 적용해주셨네요 👍

greeng00se · 2023-10-05T18:08:11Z

app/src/main/java/com/techcourse/service/TxUserService.java

+        transactionTemplate.executeWithTransaction(() -> {
+            userService.insert(user);
+            return null;
+        });


R: 반환값이 없는 메서드를 따로 만들어보는건 어떨까요!?

방금 생각난건데 표준 함수형 인터페이스를 사용해보는 것도 괜찮을 것 같네요!

좋은 의견 감사합니다 ㅎㅎ
Runnable을 사용하는 JdbcTemplate.executeWithoutResult() 따로 만들었습니다!

greeng00se · 2023-10-05T18:18:52Z

study/src/test/java/transaction/stage2/Stage2Test.java


        log.info("transactions : {}", actual);
        assertThat(actual)
-                .hasSize(0)
-                .containsExactly("");
+                .hasSize(1)
+                .containsExactly("transaction.stage2.SecondUserService.saveSecondTransactionWithNotSupported");


학습 테스트도 꼼꼼히 해주셨네요 👍 👍
저도 이거에 대한 확실한 답은 몰라서 민트의 의견을 듣고싶어요(공식문서 찾아봐도 NotSupported에 관한 부분을 잘 못찾겠더라고요ㅜㅜ) 물리적 트랜잭션은 각각 몇개 동작할까요?

으음 .. 저도 결론을 내지 못하겠는 상황이네요 ㅜ

일단 저도 궁금증이 생겨 내부 코드를 살펴본 결과를 공유합니다.
일단 학습테스트의 Service에서 로그를 찍기 위해 사용하는 TransactionSynchronizationManager.isActualTransactionActive()의 자바독을 보니, Transaction Synchronization(TS)과 Actual Transaction(AT)이 구분된다는 것을 알 수 있었습니다.
같은 클래스 내에 isSynchronizationActive()라는 메서드가 따로 존재하기도 하더라고요.

그래서 Service의 로깅 메서드에 isSynchronizationActive() 여부에 따른 로그도 추가하고 NOT_SUPPORTED 케이스에 대한 테스트를 실행해봤습니다.
트랜잭션 전파 속성은 다음과 같습니다.

FirstUserService: 트랜잭션 X

SecondUserService: NOT_SUPPORTED

제가 예상한 테스트 결과는 두 메서드 모두 물리적 트랜잭션이 존재하지 않아야 한다는 것입니다. (NOT_SUPPORTED는 트랜잭션을 사용하지 않는다고 알고 있기 때문입니다.)
하지만 getCurrentTransactionName()을 통해 검증을 진행하는 테스트는 통과하기 때문에 의문이 생겼습니다.
과연 물리적 트랜잭션이 존재하는 걸까요??..

테스트 결과입니다.

우선 @Transactional을 선언하지 않은 첫 번째 메서드는 AT, TS 모두 활성화돼 있지 않은 상태입니다.
그리고 NOT_SUPPORTED 전파 속성을 가진 두 번째 메서드는 AT는 비활성화, TS는 활성화돼 있습니다.
여기서 알아낼 수 있는 점은 TS만 활성화 되어 있어도 트랜잭션 이름을 가져올 수 있기 때문에, 서비스가 반환하는 Set<String>에 SecondUserService의 트랜잭션의 이름이 담기고, 테스트가 통과한다는 것입니다.

그렇다면 이제 생기는 궁금점은, TS는 과연 물리적 트랜잭션일까?입니다.
일단 제가 찾아본 것은 여기까지로, 궁금증을 끝까지 풀어내지 못했습니다 ㅠㅠ

제가 적절한 의문을 가지고 있는 것인지도 모르겠고, AT와 TS를 나누는 기준도 당장에 모르겠고, 우가가 올린 슬랙 스레드를 보니 왜 물리적 트랜잭션이 2개인지도 모르겠고 ... 너무 모르겠는 것 투성이어서 일단 당장은 기술부채에 쌓아두려고 합니다 하하..
이에 대해 나중에 알아보고 새로운 결론에 도달하게 된다면 다시 공유하도록 하겠습니다!! 혹시 허브도 의견 주실 수 있다면 남겨주시면 감사하겠습니다 ㅎㅎ

말씀해주신 Transaction Synchronization과 Actual Transaction에 대해 저는 정확히 잘 모르지만 일단 트랜잭션명은 Synchronization 과정에서 할당되어 실제 물리적 트랜잭션 여부와 상관없이 이름이 생성됩니다.

우가가 슬랙 스레드에 올린 내용은 정리하자면 다음과 같습니다.

Required -> NotSupported

X -> NotSupported

1번의 경우에는 Required에서 물리적 트랜잭션을 시작하고 두 번째 서비스에서 트랜잭션을 사용하지 않지만 JpaRepository의 구현체인 SimpleJpaRepository의 save에 Transaction이 달려있어서 물리적 트랜잭션이 2개입니다.

해당 부분이 정확하지 않을 수 있지만 간단하게 주석으로 트랜잭션 범위를 명시해볼게용

// 물리적 트랜잭션 시작 @Transactional(propagation = Propagation.REQUIRED) public Set<String> saveFirstTransactionWithNotSupported() { final var firstTransactionName = TransactionSynchronizationManager.getCurrentTransactionName(); userRepository.save(User.createTest()); logActualTransactionActive(); final var secondTransactionName = secondUserService.saveSecondTransactionWithNotSupported(); return of(firstTransactionName, secondTransactionName); } // 물리적 트랜잭션 종료 @Transactional(propagation = Propagation.NOT_SUPPORTED) public String saveSecondTransactionWithNotSupported() { // 물리적 트랜잭션 시작 userRepository.save(User.createTest()); // 물리적 트랜잭션 종료 logActualTransactionActive(); return TransactionSynchronizationManager.getCurrentTransactionName(); }

2번의 경우에는 메서드단에 트랜잭션이 달려있지 않지만 마찬가지로 Jpa를 사용하기 때문에 물리적 트랜잭션이 2개입니다.

public Set<String> saveFirstTransactionWithNotSupported() { final var firstTransactionName = TransactionSynchronizationManager.getCurrentTransactionName(); // 물리적 트랜잭션 시작 userRepository.save(User.createTest()); // 물리적 트랜잭션 종료 logActualTransactionActive(); final var secondTransactionName = secondUserService.saveSecondTransactionWithNotSupported(); return of(firstTransactionName, secondTransactionName); } @Transactional(propagation = Propagation.NOT_SUPPORTED) public String saveSecondTransactionWithNotSupported() { // 물리적 트랜잭션 시작 userRepository.save(User.createTest()); // 물리적 트랜잭션 종료 logActualTransactionActive(); return TransactionSynchronizationManager.getCurrentTransactionName(); }

제가 본 spring-managed transactions에 관한 참고 자료입니다.
https://docs.spring.io/spring-framework/reference/data-access/transaction/declarative/tx-propagation.html

greeng00se · 2023-10-05T18:34:50Z

jdbc/src/main/java/org/springframework/transaction/support/Nullable.java

+@Nonnull(when = When.MAYBE)
+@TypeQualifierNickname
+public @interface Nullable {


C: 해당 애너테이션을 붙이면 어떤 효과가 있나용?

추가로 import 수정해주시면 일관성 있는 코드가 될 것 같습니다!

으음.. 해당 어노테이션을 붙이지 않으면 경고가 떴던 것으로 기억하는데 다시 확인해 보니 어노테이션이 없어도 경고가 안 뜨네요 ㅎㅎ;; 지웠습니다!

greeng00se · 2023-10-05T18:39:11Z

...src/main/java/org/springframework/transaction/support/TransactionSynchronizationManager.java

 import java.util.Map;

 public abstract class TransactionSynchronizationManager {

-    private static final ThreadLocal<Map<DataSource, Connection>> resources = new ThreadLocal<>();
+    private static final ThreadLocal<Map<DataSource, Connection>> resources = ThreadLocal.withInitial(HashMap::new);


초기값 설정 잘 배워갑니다 👍👍👍

greeng00se · 2023-10-05T18:41:58Z

...src/main/java/org/springframework/transaction/support/TransactionSynchronizationManager.java

+    public static void unbindResource(DataSource key) {
+        resources.get().remove(key);
+    }
+
+    public static boolean hasResource(DataSource dataSource) {
+        return resources.get().containsKey(dataSource);


C: .get() 의 반복을 제거하는 것에 대한 민트의 생각은 어떠신가요?

greeng00se · 2023-10-05T19:34:11Z

jdbc/src/main/java/org/springframework/jdbc/core/ConnectionManager.java

+        if (TransactionSynchronizationManager.hasResource(dataSource)) {
+            return TransactionSynchronizationManager.getResource(dataSource);


R: ConnectionManager이 잘 동작하지만 제가 이 부분에 대해서 깊게 생각해봤는데, 3단계에서 사용했던 ConnectionManager를 제거하고 새로운 방식으로 해보면 어떨까 싶어요. TransactionTemplate, ConnectionManager, JdbcTemplate을 보면 Connection을 가져오고 반환하고 트랜잭션 동기화를 설정하는 것이 4단계에서 제공한 클래스를 사용하도록 코드를 리팩터링 하며 적절하게 추상화되어있지 않다고 느껴졌어요. (각각 클래스의 코드를 보시면 느껴지실껍니다!)

만약 좋은 아이디어가 떠오르지 않으신다면 해당 부분에 대해서 스프링의 TransactionTemplate, PlatformTransactionManager, DataSourceTransactionManager, ConnectionHolder, TransactionSynchronizationManager, DataSourceUtils 코드를 한 번 보시면 어떻게 추상화할지 힌트를 얻을 수도 있을 것 같아요. (해당 클래스 구현 권장이 아니라 각각 클래스의 책임을 보면 미션에 도움이 될 것 같다고 생각했어요.)

저는 다음 순서대로 대략적인 흐름만 파악했어요! 더 많은 정보를 얻으면 코멘트 알려주세요!
TransactionTemplate --> PlatformTransactionManager --> DataSourceTransactionManager --> ConnectionHolder(+ ResourceHolderSupport) + DataSourceUtils --> TransactionSynchronizationManager

저도 허브의 의견에 동의합니다!
ConnectionManager를 제거하고, DataSource 관련 책임은 DataSourceUtils를 통해서만 수행하도록 수정했습니다.
혹시나 현재 흐름도 어색하다고 느껴진다면 말씀해 주세요!!

모든 클래스를 다 확인해보진 못했지만, 말씀해주신 흐름에 따라 각 클래스의 역할에 대해 공부할 수 있었습니다. 감사합니다 ☺️

shin-mallang · 2023-10-05T23:41:46Z

아니 이사람은 무슨 벌써 4단계를..

greeng00se

안녕하세요 민트 🌿
한글날 잘 보내고 계신가요?

코드도 꼼꼼하게 작성해주시고 너무 잘하셔서 머지하고 싶지만 👍👍👍👍👍👍
리팩터링 과정에서 기존의 Connection이 종료되지 않는 문제가 발생할 것 같아서 마지막으로 RC 남겨봅니다! 😄

greeng00se · 2023-10-09T08:58:07Z

jdbc/src/main/java/org/springframework/transaction/support/TransactionTemplate.java

@@ -18,14 +19,15 @@ public TransactionTemplate(final DataSource dataSource) {
        this.dataSource = dataSource;
    }

-    public void executeWithTransaction(final TransactionCallback transactionCallback) {
+    @Nullable
+    public <T> T executeWithTransaction(final TransactionCallback<T> transactionCallback) {


두개 잘 나누어 주셨네용 👍👍👍 일관성 있게 Runnable 처럼 표준 함수형 인터페이스인 Supplier를 사용하는 방법도 있을 것 같아요 👍

잘못적어놨네용 ㅋㅋ

greeng00se · 2023-10-09T08:58:40Z

jdbc/src/main/java/org/springframework/transaction/support/TransactionTemplate.java

+    public void executeWithoutResult(final Runnable action) {
+        executeWithTransaction(() -> {
+            action.run();
+            return null;
+        });
+    }


재사용 센스 ㄷㄷ 👍

greeng00se · 2023-10-09T09:22:55Z

study/src/test/java/transaction/stage2/Stage2Test.java


        log.info("transactions : {}", actual);
        assertThat(actual)
-                .hasSize(0)
-                .containsExactly("");
+                .hasSize(1)
+                .containsExactly("transaction.stage2.SecondUserService.saveSecondTransactionWithNotSupported");


말씀해주신 Transaction Synchronization과 Actual Transaction에 대해 저는 정확히 잘 모르지만 일단 트랜잭션명은 Synchronization 과정에서 할당되어 실제 물리적 트랜잭션 여부와 상관없이 이름이 생성됩니다.

우가가 슬랙 스레드에 올린 내용은 정리하자면 다음과 같습니다.

Required -> NotSupported

X -> NotSupported

1번의 경우에는 Required에서 물리적 트랜잭션을 시작하고 두 번째 서비스에서 트랜잭션을 사용하지 않지만 JpaRepository의 구현체인 SimpleJpaRepository의 save에 Transaction이 달려있어서 물리적 트랜잭션이 2개입니다.

해당 부분이 정확하지 않을 수 있지만 간단하게 주석으로 트랜잭션 범위를 명시해볼게용

// 물리적 트랜잭션 시작 @Transactional(propagation = Propagation.REQUIRED) public Set<String> saveFirstTransactionWithNotSupported() { final var firstTransactionName = TransactionSynchronizationManager.getCurrentTransactionName(); userRepository.save(User.createTest()); logActualTransactionActive(); final var secondTransactionName = secondUserService.saveSecondTransactionWithNotSupported(); return of(firstTransactionName, secondTransactionName); } // 물리적 트랜잭션 종료 @Transactional(propagation = Propagation.NOT_SUPPORTED) public String saveSecondTransactionWithNotSupported() { // 물리적 트랜잭션 시작 userRepository.save(User.createTest()); // 물리적 트랜잭션 종료 logActualTransactionActive(); return TransactionSynchronizationManager.getCurrentTransactionName(); }

2번의 경우에는 메서드단에 트랜잭션이 달려있지 않지만 마찬가지로 Jpa를 사용하기 때문에 물리적 트랜잭션이 2개입니다.

public Set<String> saveFirstTransactionWithNotSupported() { final var firstTransactionName = TransactionSynchronizationManager.getCurrentTransactionName(); // 물리적 트랜잭션 시작 userRepository.save(User.createTest()); // 물리적 트랜잭션 종료 logActualTransactionActive(); final var secondTransactionName = secondUserService.saveSecondTransactionWithNotSupported(); return of(firstTransactionName, secondTransactionName); } @Transactional(propagation = Propagation.NOT_SUPPORTED) public String saveSecondTransactionWithNotSupported() { // 물리적 트랜잭션 시작 userRepository.save(User.createTest()); // 물리적 트랜잭션 종료 logActualTransactionActive(); return TransactionSynchronizationManager.getCurrentTransactionName(); }

제가 본 spring-managed transactions에 관한 참고 자료입니다.
https://docs.spring.io/spring-framework/reference/data-access/transaction/declarative/tx-propagation.html

greeng00se · 2023-10-09T09:37:16Z

jdbc/src/main/java/org/springframework/jdbc/datasource/DataSourceUtils.java

+    private static boolean isTransactional(final DataSource dataSource, final Connection connection) {
+        if (TransactionSynchronizationManager.hasResource(dataSource)) {
+            return TransactionSynchronizationManager.getResource(dataSource) == connection;
+        }
+        return false;
+    }


R: 매번 true를 반환하는 것 같아요! TransactionTemplate을 사용하지 않는다면 커넥션을 영원히 종료하지 않는 문제가 생길 것 같아요!

Connection.getAutoCommit()을 통해 확인 후, close 하도록 수정했습니다!

greeng00se

안녕하세요 민트~
추석에 미션 진행하던게 어제 같은데 벌써 레벨 4가 끝나가고 있네요..
수정된 부분 확인했습니다! 미션 진행하느라 고생하셨어요 👍👍👍
저도 민트 덕분에 많이 배웠습니다! 🙇

greeng00se · 2023-10-12T23:24:00Z

jdbc/src/main/java/org/springframework/jdbc/datasource/DataSourceUtils.java

+            if (connection.getAutoCommit()) {
+                TransactionSynchronizationManager.unbindResource(dataSource);
+                releaseConnection(connection);


auto commit으로 구분해주셨네요 👍

kang-hyungu and others added 9 commits October 4, 2023 15:07

패키지 위치 변경 및 코드 정리

23f8a48

docs: 3단계 3차 리뷰 내용 정리

fd3b51e

chore: EOF 해결

161d0af

test: 트랜잭션 학습 테스트

9ce3a83

refactor: TransactionContext 삭제 및 TransactionSynchronizationManager 적용

696a787

refactor: DataSourceUtils 적용

93261f4

refactor: UserService 추상화

94d40f9

refactor: TransactionCallback의 반환 타입을 제네릭으로 수정

055eb1b

feat: Nullable 어노테이션 적용

34ffba0

yujamint requested a review from greeng00se October 5, 2023 18:01

yujamint self-assigned this Oct 5, 2023

greeng00se requested changes Oct 5, 2023

View reviewed changes

yujamint added 9 commits October 6, 2023 21:54

docs: 4단계 리뷰 내용 정리

3c484ec

style: 불필요한 공백 제거

e205b8f

refactor: TxUserService의 TransactionTemplate 생성자 주입받도록 수정

df7e776

feat(TransactionTemplate): 반환값이 없는 메서드 구현

b33cfd1

refactor: 중복되는 resources.get() 제거

36fbd06

style: import 와일드카드 제거

7e30cef

refactor: ConnectionManager 제거 및 DataSourceUtils로 책임 이전

3438d8d

test: 학습 테스트 로깅 추가

5ad575e

refactor: 불필요한 어노테이션 제거

13f7d03

greeng00se requested changes Oct 9, 2023

View reviewed changes

yujamint added 3 commits October 10, 2023 09:40

docs: 4단계 2차 리뷰 내용 정리

d0e93ca

refactor: TransactionCallback 제거 후 표준 함수형 인터페이스 사용

c11f4f4

fix: autoCommit 모드에서 Connection이 닫히지 않는 문제 해결

c2efb84

greeng00se approved these changes Oct 12, 2023

View reviewed changes

greeng00se merged commit cf099d0 into woowacourse:yujamint Oct 12, 2023
1 check failed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[JDBC 4단계] 민트(유재민) 미션 제출합니다. #458

[JDBC 4단계] 민트(유재민) 미션 제출합니다. #458

yujamint commented Oct 5, 2023 •

edited

Loading

greeng00se left a comment

greeng00se Oct 5, 2023

greeng00se Oct 5, 2023

yujamint Oct 9, 2023

greeng00se Oct 5, 2023

greeng00se Oct 5, 2023

yujamint Oct 9, 2023

greeng00se Oct 5, 2023

yujamint Oct 9, 2023

greeng00se Oct 9, 2023

greeng00se Oct 5, 2023

yujamint Oct 9, 2023

greeng00se Oct 5, 2023

greeng00se Oct 5, 2023

greeng00se Oct 5, 2023

yujamint Oct 9, 2023

shin-mallang commented Oct 5, 2023

greeng00se left a comment

greeng00se Oct 9, 2023 •

edited

Loading

greeng00se Oct 9, 2023

greeng00se Oct 9, 2023

greeng00se Oct 9, 2023

yujamint Oct 12, 2023

greeng00se left a comment

greeng00se Oct 12, 2023

		if (TransactionSynchronizationManager.hasResource(dataSource)) {
		return TransactionSynchronizationManager.getResource(dataSource);

[JDBC 4단계] 민트(유재민) 미션 제출합니다. #458

[JDBC 4단계] 민트(유재민) 미션 제출합니다. #458

Conversation

yujamint commented Oct 5, 2023 • edited Loading

greeng00se 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

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

shin-mallang commented Oct 5, 2023

greeng00se left a comment

Choose a reason for hiding this comment

greeng00se Oct 9, 2023 • edited Loading

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

greeng00se left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yujamint commented Oct 5, 2023 •

edited

Loading

greeng00se Oct 9, 2023 •

edited

Loading