[JDBC 라이브러리 구현하기 - 4단계] 가비 미션 제출합니다. #523

iamjooon2 · 2023-10-08T13:00:47Z

안녕하세요 홍고🍊

람다랑 함수형 인터페이스한테 또 혼나고 온 가비입니다

DataSourceUtils를 이용해서 Connection을 관리하도록 했고
TranasactionManager라는 친구를 만들어서, Service에 있던 트랜잭션 관리하는 넘을 안으로 밀어넣어줬습니다

날씨 추워졌는데 감기 조심하세요...!

4단계 미션 제출하신다음 리뷰해주셔도 됩니다...!
기간내 리뷰하신다고 스포당하진 않으셨으면🥲

…on을 사용하도록 변경

…호출 제거

hgo641

안녕하세요, 가비!! 😄
이번에도 역시 코드가 깔끔하군요 👍👍
TransactioManager로 트랜잭션을 관리하는 코드가 깔끔하게 분리되었네요! 👍
배우고 갑니다!
코드보면서 궁금한 거 질문으로 남겼습니다!
날씨가 많이 추워졌는데 감기 조심하세요!

hgo641 · 2023-10-10T15:18:56Z

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

+    @Override
+    public User findById(final long id) {
+        return userService.findById(id);
+    }


여기는 트랜잭션 처리를 안해주신 이유가 있나요? read-only여서인가요?

넵 단순 조회이기도 하고 반환값이 있는 함수형 인터페이스 새로 만들어줘야해서였는데
생각해보니 자바 기본 함수형 인터페이스가 있었네요🥲

hgo641 · 2023-10-10T15:19:38Z

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

+
+    public TxUserService(final UserService userService) {
+        this.userService = userService;
+        this.transactionManager = new TransactionManager(DataSourceConfig.getInstance());


Service에서 DataSource를 주입해주신 이유가 있으신가요?? 궁금합니다!

TransactionManager를 주입받도록 변경했습니다

hgo641 · 2023-10-10T15:51:22Z

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

+    private static Map<DataSource, Connection> getResources() {
+        return resources.get();
    }


resources.get()의 반복 사용이 없어졌네요 👍👍

hgo641 · 2023-10-10T16:42:51Z

app/src/test/java/com/techcourse/service/UserServiceTest.java

@@ -31,31 +31,35 @@ void setUp() {
    @Test


여기 setUp()에서 사용되는 DatabasePopulatorUtils의 connection은 DataSourceUtils로 connection을 가져오지 않으신 이유가 있나요? DatabasePopulatorUtils가 실행하는 쿼리에는 트랜잭션이 적용되지 않아도 될까요?

꼼꼼하게 봐주셔서 감사합니다! 반영했습니다~!

ㅎㅎㅎ... 진짜 궁금해서 여쭤본거였는데.... ☺

저는 setUp() 메서드에 DatabasePopulatorUtils라는 다른 종류의 클래스를 사용하는걸 미처 보지 못했어요
그리고 자연히 테스트에서도 동일하게 트랜잭션을 적용해야한다고 생각합니다.. 적용하지 않을 이유는 없다고 생각해요

놓친 것이 있다면 알려주심 감사하겠습니다..

hgo641 · 2023-10-10T16:52:39Z

jdbc/src/main/java/org/springframework/transaction/TransactionManager.java

+    private static void rollBackConnection(final Connection connection) {
+        try {
+            connection.rollback();
+        } catch (final SQLException e) {
+            throw new DataAccessException(e);
+        }
+    }


덕분에 try depth가 줄어들었군요👍

hgo641 · 2023-10-10T17:00:46Z

jdbc/src/main/java/org/springframework/transaction/TransactionManager.java

+    public void run(final TransactionExecutor transactionExecutor) {
+        final Connection connection = DataSourceUtils.getConnection(dataSource);
+        try {
+            connection.setAutoCommit(false);
+
+            transactionExecutor.action();
+
+            connection.commit();
+        } catch (final SQLException e) {
+            rollBackConnection(connection);
+            throw new DataAccessException(e);
+        } finally {
+            DataSourceUtils.releaseConnection(connection, dataSource);
+        }
+    }


여기도 템플릿 콜백패턴을 적용하면 로직이 더 깔끔해질 것 같습니다! 이미 알고 계실 것 같지만 리뷰가 넘 심심한 것 같아 쥐어짜내봅니다 😄 (반영은 안 해주셔도 됩니다!!!)

지금은 TransactionManager가 하나밖에 없지만 Spring을 보면 HibernateTransactionManager같이 TransactionManager가 여러 개 있더군요! 추상화를 위해 rollBack을 메서드로 분리하신 것처럼 commit도 메서드로 분리하셔도 좋을 것 같습니다. 이 부분도 콜백패턴을 적용하면 자연스럽게 반영되겠군여. 쥐어짜낸 리뷰222 (반영안해주셔도 됩니다222)

소중한 리뷰 감사합니다 😊
첫 리뷰 반영하면서 템플릿 콜백으로 변경했어요...!
connection 부분은 메서드만 분리했습니다..!

hgo641 · 2023-10-10T17:19:10Z

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

+    public static void unbindResource(DataSource key) {
+        getResources().remove(key);
    }


저도 가비랑 동일하게 코드를 짰었는데 소나가 "ThreadLocal" variables should be cleaned up when no longer used라고 경고를 띄워주더라고요!! Thread Pool을 사용할 경우, 사용한 ThreadLocal을 remove해주지 않으면 예상치못하게 동작할 수도 있다고 해요. (참고한 글)
물론 지금은 Thread Pool을 사용하지 않아 remove를 해주지 않아도 될 것 같지만, ThreadLocal을 사용할 땐 의식적으로 값이 비었을 경우 remove해줘도 괜찮을 것 같다... 는 생각이 들었지만 제 뇌피셜입니다

hgo641

안녕하세요, 가비!
4단계 고생하셨습니다!!👍
이번에는 질문없고 박수만 짝짝 치고갑니다.
다음 미션도 화이팅입니다 😄😄

hgo641 · 2023-10-11T04:48:17Z

jdbc/src/main/java/org/springframework/transaction/TransactionManager.java

+    public <T> T runForObject(final TransactionExecutor<T> transactionExecutor) {
+        return execute(transactionExecutor);
+    }
+
+    public void run(final Runnable runnable) {
+        execute(() -> {
+            runnable.run();
+            return null;
+        });
+    }


👍👍 짱깔끔해여

hgo641 · 2023-10-11T06:16:18Z

app/src/main/java/com/techcourse/support/jdbc/init/DatabasePopulatorUtils.java

@@ -39,8 +44,7 @@ public static void execute(final DataSource dataSource) {
                    connection.close();
                }
            } catch (SQLException ignored) {}
+            DataSourceUtils.releaseConnection(connection, dataSource);


릴리즈까지 꼼꼼!!! 👍👍

iamjooon2 added 12 commits October 8, 2023 14:12

refactor: 메서드명 수정

0b720af

fix: DataSourceUtil과 TransactionSynchronizationManager를 이용하여 Connecti…

e21617d

…on을 사용하도록 변경

refactor: findById의 반환 타입에서 Optional 제거

1ae2ea5

feat: DataSourceUtils 내 TransactionSynchronizationManger 자원 해제 추가

b05df24

feat: TransactionManager 구현

8637110

feat: UserService 추상화

9c51e1d

feat: AppUserService, TxUserService 구현

b072844

test: testTransactionRollback() 메서드를 LMS에 있는 내용으로 변경

87a8363

fix : jdbctemplate에서 connection 해제하는 부분 제거

f47ee27

refactor: Connection 선언부를 try-catch 밖으로 분리

06d7f7c

refactor: TranactionManager의 메서드 반환값을 void로 변경

c89797e

chore: final 키워드 통일

faa6f7d

iamjooon2 requested a review from hgo641 October 8, 2023 13:56

iamjooon2 force-pushed the step4 branch from 995ba8e to faa6f7d Compare October 9, 2023 15:17

fix: TransactionSynchronizationManager.unbindResource(dataSource) 중복 …

26a140a

…호출 제거

hgo641 requested changes Oct 10, 2023

View reviewed changes

iamjooon2 added 6 commits October 11, 2023 09:37

fix: 불필요한 static 제거

42d84c0

feat: DatabasePopulatorUtils도 DataSoureUtils를 이용해 connection을 가지고 오도록 변경

f0061ad

refactor: service에서 TransactionManager를 주입받도록 변경

a281e05

refactor: 템플릿 콜백 패턴 적용

7ad67af

refactor: 함수형 인터페이스 실행 메서드 분리

0077696

refactor: DatasourcePopulatorUtils connection 해제 추가

245d83e

hgo641 approved these changes Oct 11, 2023

View reviewed changes

hgo641 merged commit 4897a4f into woowacourse:iamjooon2 Oct 11, 2023
1 check failed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[JDBC 라이브러리 구현하기 - 4단계] 가비 미션 제출합니다. #523

[JDBC 라이브러리 구현하기 - 4단계] 가비 미션 제출합니다. #523

iamjooon2 commented Oct 8, 2023 •

edited

Loading

hgo641 left a comment

hgo641 Oct 10, 2023

iamjooon2 Oct 11, 2023

hgo641 Oct 10, 2023

iamjooon2 Oct 11, 2023

hgo641 Oct 10, 2023

hgo641 Oct 10, 2023

iamjooon2 Oct 11, 2023

hgo641 Oct 11, 2023

iamjooon2 Oct 11, 2023 •

edited

Loading

hgo641 Oct 10, 2023

hgo641 Oct 10, 2023

hgo641 Oct 10, 2023

iamjooon2 Oct 11, 2023

hgo641 Oct 10, 2023

hgo641 left a comment

hgo641 Oct 11, 2023

hgo641 Oct 11, 2023

[JDBC 라이브러리 구현하기 - 4단계] 가비 미션 제출합니다. #523

[JDBC 라이브러리 구현하기 - 4단계] 가비 미션 제출합니다. #523

Conversation

iamjooon2 commented Oct 8, 2023 • edited Loading

hgo641 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

iamjooon2 Oct 11, 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

Choose a reason for hiding this comment

hgo641 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

iamjooon2 commented Oct 8, 2023 •

edited

Loading

iamjooon2 Oct 11, 2023 •

edited

Loading