[JDBC 라이브러리 구현하기 - 4단계] 썬샷(오진택) 미션 제출합니다. #601

Ohjintaek · 2023-10-10T07:39:20Z

이번 미션도 벌써 마지막 단계네요 (저는 하루 늦게 제출하지만ㅠㅠ)
다시 한 번 죄송합니다

특정 스레드마다 dataSource에 대한 Connection을 등록해두고 같은 트랜잭션 내에서 진행되어야 한다면 같은 Connection을 쓰도록 하는 게 감탄이 나오네요.
그런데 4단계 미션을 위해 리팩토링 하면서 또 다른 심각한 의문점이 생겨서 질문 하나만 드릴게요.
현재 DAO에 있는 메서드들도 public으로 열려 있어서 다른 객체에서 사용할텐데 획득한 Connection을 닫아주지 못하고 있는 상태가 되었습니다. 그렇다고 메서드가 호출될 때마다 닫히면 여러 개의 쿼리를 한 트랜잭션으로 묶는 게 되질 않습니다.
결국 Connection을 닫고 싶으면 TxUserService와 같은 구현체를 만들어주어야 하는데 이런 행위가 스프링에서 @transactional을 달아주는 것과 비슷하다고 생각하면 되는 걸까요?

p.s. 질문드리고 나서 해결책을 몇 가지 배워서 DAO에서 호출된 메서드도 Connection을 닫아주도록 개선해주었습니다!!

Songusika

안녕하세요 썬샷!
리뷰가 많이 늦었네요 🥲
JDBC 미션 구현 잘 해주셨고 주신 질문도 스스로 해결해주셔서 너무 좋았습니다!
간단한 커멘트 및 질문 남겼는데 같이 이야기 해보면 좋을 것 같습니다!
추가적으로 Approve / RC 관련해서 DM 남겼으니 썬샷이 원하는 방향으로 가시면 좋을 것 같아요!

Songusika · 2023-10-11T06:10:12Z

README.md

+ - Dao의 메서드에서 connection 객체를 파라미터로 전달받지 않기
+ - Connection을 관리하는 기능을 가지는 객체 구현
+ - `UserService`에서 비즈니스 로직과 데이터 액세스 로직 분리하기 


Readme 작성 저는 항상 까먹는데 챙겨주시는 센스가 돋보입니다 👍

Songusika · 2023-10-11T06:11:31Z

app/src/main/java/com/techcourse/dao/UserDao.java

 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.jdbc.core.JdbcTemplate;
 import org.springframework.jdbc.core.RowMapper;

+import javax.sql.DataSource;
+import java.util.List;


org 패키지와 javax, java 패키지 간 공백은 자동완성 때문일까요? 아니면 서로 다른 패키지이므로 별도로 띄워주신건가요??

제 설정으로는 자동완성하면 저렇게 공백을 두더라구요
프로젝트 때 사용한 설정입니다

Songusika · 2023-10-11T06:12:00Z

app/src/main/java/com/techcourse/dao/UserDao.java

-    public void update(final Connection conn, final User user) {
-        final var sql = "update users set account = ?, password = ?, email = ? where id = ?";
-        jdbcTemplate.execute(conn, sql, user.getAccount(), user.getPassword(), user.getEmail(), user.getId());
-    }
-


더 이상 Connection을 서비스로 부터 받지 않아도 되네요 👍👍

Songusika · 2023-10-11T06:14:02Z

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

+
+    @Override
+    public User findById(long id) {
+        return transactionManager.execute(DataSourceConfig.getInstance(), () -> userService.findById(id));


DataSourceConfig.getInstance() 는 생성자에서 넣어주는게 어떨까요?
별로의 파라미터로 빼신 이유가 있을까요?

AppUserService가 Dao에게 dataSource나 jdbcTemplate과 같은 DB에 관련한 처리를 맡기고 아무것도 모르는 것처럼 TransactionService도 내부 필드로 dataSource를 안 가지도록 하고 싶어서 그랬었습니다. 근데 생각해보니 어떤 특정한 db에 저장된 User와 관련된 트랜잭션을 처리해주는 객체니까 dataSource를 내부 필드로 가져도 된다는 생각이 드네요. 중복도 줄일 수 있구요
수정해주었습니다!!

Songusika · 2023-10-11T06:25:33Z

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

+        transactionManager.execute(DataSourceConfig.getInstance(), () -> {
+            userService.insert(user);
+            return null;
+        });


null 리턴 대신, transactionManager 에서 아무 인자를 받지 않고, 아무것도 리턴을 하지 않는 callback 함수를 받을 수 있도록 메서드를 오버로딩 할 수 있을 것 같아요! (feat @FuntionalInterface)

혹은 Runnable 인터페이스를 받을 수 도 있을 것 같습니다. (단 저라면 이건 안쓸것 같습니다 🤔)

자바에서 제공하는 아무 인자도 받지 않고, 아무것도 리턴을 하지 않는 함수형 인터페이스가 없길래 Supplier로 함쳐서 써 버렸는데 들켜버렸군요ㅎㅎㅎ
하나 만들어주는 김에 기존에 Supplier도 Transaction용으로 사용한다고 명시적으로 하나 추가해주었습니다. 근데 왜 Java에서 이건 안 만들어주었을까요?

사실 아무 인자도 받지 않고, 아무것도 리턴을 하지 않는 함수형 인터페이스가 Runnable 인터페이스로 존재하긴 합니다!
그런데 개인적으로 Runnable 인터페이스는 Thread 로 분기하고 싶은 클래스가 구현을 해야해서
느낌상 쓰레드와 관련된 작업이라는 뉘양스로 느껴지더라구요 (지극히 개인적입니다😅)

실제 Runnable 인터페이스 주석을 확인해도 Thread 와 관련된 작업만 적어 놓았더라구요!
그래서 다른 목적으로 사용할 때는 다른 개발자에게 다소 혼동을 줄 수 있을 것 같아요.
그래서 저도 별도로 함수형 인터페이스를 구현합니다...
자바에서 왜 별도로 만들지 않았는지는 저도 궁금하네요...🤔

Songusika · 2023-10-12T17:56:52Z

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

+
+    private void closeConnectionIfAutoCommitted(Connection conn) {
+        try {
+            if (conn.getAutoCommit()) {


저는 해당 메서드가 실제 데이터베이스와의 통신 비용이 발생할 줄 알았는데
어느정도 찾아보니getAutoCommit 메서드는 캐싱된 정보를 반환한다고 하네요! (by Oracle)

단, MySql 한에서 XServerSession 의 구현체를 보면 해당 메서드에서는 뮤텍스락을 얻어서 캐싱된 autoCommit 을 반환하는 것을 볼 수 있는데요 이로 인해서 성능적 하락은 없을까요? 궁금하네요..

@Override public boolean getAutoCommit() throws SQLException { synchronized (getConnectionMutex()) { return this.session.getServerSession().isAutoCommit(); } }

Connection 객체에서 autoCommit을 따로 boolean 변수로 저장하는 걸 강제하고 있지 않았네요... 구현체에 따라 직접 데이터베이스에 접근해야 하는 상황이 된다면 성능적 하락이 존재할 것이라는 생각이 드네요

Songusika · 2023-10-12T17:59:48Z

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

+        if (Objects.isNull(connectionMap)) {
+            connectionMap = new HashMap<>();
+        }


ThreadLocal.withInitial() 메서드를 활용하면 제네릭 타입에 대한 인자로 선언과 동시 더 쉽게 초기화할 수 있어요!

오 좋은 방법 감사합니다!!!
TransactionSychronizationManager에서 정적 필드로 Map이 있는 ThreadLocal을 가지는 것이 맞는 것 같아서 거기서 초기화해주었습니다. 그에 따라 메서드들 로직도 좀 바꿨구요

Songusika

안녕하세요 썬샷 👋
마지막까지 고생많으셨습니다!

Songusika · 2023-10-16T16:31:13Z

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

+import java.sql.Connection;
+import java.sql.SQLException;
+
+public class JdbcTransactionManager implements TransactionManager {


구현체를 분리하셨군요 ! 👍

Songusika · 2023-10-16T16:32:38Z

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

+    @Override
+    public void execute(DataSource dataSource, TransactionExecutor method) {
+        final var connection = TransactionManager.getConnection(dataSource);
+        try {
+            connection.setAutoCommit(false);
+            method.execute();
+            connection.commit();
+        } catch (RuntimeException | SQLException e) {
+            handleTransactionException(connection, e);
+        } finally {
+            cleanUpTransaction(dataSource, connection);
+        }
+    }


위와 같은 구조를 적용하시면 TxService에서 DataSource 를 필드로 들고 있을 필요가 없을 것 같아요!

Songusika · 2023-10-16T16:39:39Z

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

+        transactionManager.execute(DataSourceConfig.getInstance(), () -> {
+            userService.insert(user);
+            return null;
+        });


사실 아무 인자도 받지 않고, 아무것도 리턴을 하지 않는 함수형 인터페이스가 Runnable 인터페이스로 존재하긴 합니다!
그런데 개인적으로 Runnable 인터페이스는 Thread 로 분기하고 싶은 클래스가 구현을 해야해서
느낌상 쓰레드와 관련된 작업이라는 뉘양스로 느껴지더라구요 (지극히 개인적입니다😅)

실제 Runnable 인터페이스 주석을 확인해도 Thread 와 관련된 작업만 적어 놓았더라구요!
그래서 다른 목적으로 사용할 때는 다른 개발자에게 다소 혼동을 줄 수 있을 것 같아요.
그래서 저도 별도로 함수형 인터페이스를 구현합니다...
자바에서 왜 별도로 만들지 않았는지는 저도 궁금하네요...🤔

Ohjintaek added 4 commits October 10, 2023 13:34

docs: 4단계 요구사항 정리

b4069e4

refactor: Dao에서 Connection 객체를 파라미터로 받지 않도록 리팩토링

bca50a4

feat: UserService에서 데이터 액세스와 트랜잭션 분리

6dfbebf

feat: UserService에서 데이터 액세스와 트랜잭션 분리

8850eec

Ohjintaek requested a review from Songusika October 10, 2023 07:39

Ohjintaek self-assigned this Oct 10, 2023

Ohjintaek added 2 commits October 10, 2023 17:08

feat: connection close 시 connectionMap에서 dataSource 등록 해제

4e5883e

feat: 단건 쿼리에서도 connection.close 호출되도록 수정

8827209

Songusika requested changes Oct 12, 2023

View reviewed changes

Ohjintaek added 3 commits October 16, 2023 01:26

refactor: TransactionService 객체가 DataSource를 갖도록 수정

e531b69

refactor: TransactionManager에서 전달받을 함수형 인터페이스를 명시적으로 정의

d0f339d

refactor: ThreadLocal 초기화

7aaacbd

Songusika approved these changes Oct 17, 2023

View reviewed changes

Songusika merged commit 5084210 into woowacourse:ohjintaek Oct 18, 2023
1 check failed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[JDBC 라이브러리 구현하기 - 4단계] 썬샷(오진택) 미션 제출합니다. #601

[JDBC 라이브러리 구현하기 - 4단계] 썬샷(오진택) 미션 제출합니다. #601

Ohjintaek commented Oct 10, 2023 •

edited

Loading

Songusika left a comment

Songusika Oct 11, 2023

Songusika Oct 11, 2023

Ohjintaek Oct 15, 2023

Songusika Oct 11, 2023

Songusika Oct 11, 2023

Ohjintaek Oct 15, 2023 •

edited

Loading

Songusika Oct 11, 2023

Ohjintaek Oct 15, 2023

Songusika Oct 16, 2023

Songusika Oct 12, 2023

Ohjintaek Oct 15, 2023

Songusika Oct 12, 2023

Ohjintaek Oct 15, 2023

Songusika left a comment

Songusika Oct 16, 2023

Songusika Oct 16, 2023

Songusika Oct 16, 2023

[JDBC 라이브러리 구현하기 - 4단계] 썬샷(오진택) 미션 제출합니다. #601

[JDBC 라이브러리 구현하기 - 4단계] 썬샷(오진택) 미션 제출합니다. #601

Conversation

Ohjintaek commented Oct 10, 2023 • edited Loading

Songusika 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

Ohjintaek Oct 15, 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Songusika 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

Ohjintaek commented Oct 10, 2023 •

edited

Loading

Ohjintaek Oct 15, 2023 •

edited

Loading