🚀 4단계 - 트랜잭션 동기화 구현하기 #41

sm9171 · 2024-08-30T05:37:13Z

안녕하세요 정완님

미션을 구현했는데 UserServiceTest에서 testChangePassword 테스트에서 실패를 하고 있습니다.

디버깅을 했을 때 AppUserService에서 userHistoryDao.log(new UserHistory(user, createBy)); 를 실행할 때 jdbcTemplate클래스의 update메소드에서 `PreparedStatement를 할당할 때 에러가 발생하더라구요.

혼자 원인 파악이 어려워서 해당 내용 공유드립니다.

그리고 3번째 미션때 주신 질문이 저한테는 어려워서 혹시 정완님의 생각은 어떠신지 궁금합니다.
#39 (comment)

# Conflicts: # README.md # app/src/main/java/camp/nextstep/dao/UserDao.java # app/src/main/java/camp/nextstep/dao/UserRowMapper.java

# Conflicts: # README.md

- update 메소드에서 connection을 DataSourceUtils에서 가져오는걸로 변경

# Conflicts: # README.md # app/src/main/java/camp/nextstep/dao/UserHistoryDao.java # app/src/main/java/camp/nextstep/service/UserService.java # app/src/test/java/camp/nextstep/service/UserServiceTest.java

ghojeong · 2024-08-30T10:55:59Z

그리고 3번째 미션때 주신 질문이 저한테는 어려워서 혹시 정완님의 생각은 어떠신지 궁금합니다.
#39 (comment)

나상민 님!
저에게도 학습해야하는 내용인지라, 제가 공부했고 경험했던 내용들을 댓글로 남겨보았습니다.
저도 어려운 주제인데 답변 달아주셔서 감사합니다.

ghojeong

나상민님!
디버깅 도움 요청주신 것에 대해 의견 남겨보았습니다.
제 추측은 DataSourceUtils.releaseConnection 과는 별개로
try-with-resources 문법에서 한번 더 connection 을 close 하려 해서 발생한 버그가 아닌가 싶습니다.

현재 Transaction 관련 객체들에 관한 테스트가 전무한데,
테스트 작성 부탁드립니다!

ghojeong · 2024-08-30T11:16:18Z

jdbc/src/main/java/camp/nextstep/jdbc/core/JdbcTemplate.java

+        try (final Connection connection = DataSourceUtils.getConnection(dataSource);
             final PreparedStatement pstmt = connection.prepareStatement(sql)) {


Suggested change

try (final Connection connection = DataSourceUtils.getConnection(dataSource);

final PreparedStatement pstmt = connection.prepareStatement(sql)) {

try {

final Connection connection = DataSourceUtils.getConnection(dataSource);

final PreparedStatement pstmt = connection.prepareStatement(sql);

미션을 구현했는데 UserServiceTest에서 testChangePassword 테스트에서 실패를 하고 있습니다.
디버깅을 했을 때 AppUserService에서 userHistoryDao.log(new UserHistory(user, createBy)); 를 실행할 때 jdbcTemplate클래스의 update메소드에서 `PreparedStatement를 할당할 때 에러가 발생하더라구요.

제가 로그상 확인한 에러는 다음과 같습니다.
org.h2.jdbc.JdbcSQLNonTransientException: The object is already closed

이미 closed 된 자원을 사용하고 있다고 로그가 말합니다.
그리고 미션에서 closed 된 자원이란 곧 connection 을 뜻합니다.

Try-With-Resources 문법에 대해서 더 깊게 공부해보면 좋을 것 같습니다.
지금과 같은 상황에서는 update 메서드를 호출할 때마다, connection 이 바로 closed 되게 되어,
같은 트랜잭션 안에 있는 다른 update 에서 connection 을 재활용하지 못합니다.

같은 트랜잭션이라면 connection 이 closed 되지 않고, 같은 connection 을 재사용할 수 있고,
트랜잭션이 완료되었을 때 connection 이 closed 되도록 해주세요.

참고로 맨 위의 suggestion code 는 이렇게 하면 테스트를 통과할 수 있다이지,
이렇게 고쳐야 한다는 뜻이 절대 아닙니다.
위의 코드에서는 그래서 connection 을 어디서 어떻게 close 할 것인지에 대한 부분이 빠져있는데,
이 부분을 구현해주시기 바랍니다.

ghojeong · 2024-08-30T11:20:18Z

app/src/main/java/camp/nextstep/service/TxUserService.java

+public class TxUserService implements UserService {
+    private final UserService userService;
+    private final TransactionTemplate transactionTemplate;
+
+    @Autowired
+    public TxUserService(final UserService userService, DataSource dataSource) {
+        this.userService = userService;


DI 를 하는 상황을 가정하고 파라미터 타입을 고려하면 좋지 않을까 싶습니다.
가령 현재 UserService 를 implements 하는 클래스는 TxUserService 와 AppUserService 2개라서,

나중에 프레임워크가 DI 를 하려할 때 어떤 클래스를 의존성 주입을 해야할지 헷갈릴 것 같습니다.
내부 맴버의 타입이 UserService 이더라도,
DI 프레임워크가 확인하는 생성자 파라미터의 타입은 AppUserService 로 명시하는게 더 안전하지 않을까 제안해봅니다.

ghojeong · 2024-08-30T11:21:00Z

app/src/main/java/camp/nextstep/service/TxUserService.java

+    @Override
+    public void changePassword(final long id, final String newPassword, final String createdBy) {
+        transactionTemplate.execute(ignored -> {
+            userService.changePassword(id, newPassword, createdBy);
+            return null;
+        });
+    }


미션 의도와 일치하게 transactionTemplate 잘 활용해주셨습니다. 👍

ghojeong · 2024-08-30T11:23:04Z

jdbc/src/main/java/camp/nextstep/transaction/support/TransactionTemplate.java

+    public <T> T execute(TransactionCallback<T> action) {
+        TransactionStatus status = transactionManager.getTransaction();
+
+        try {
+            T result = action.doInTransaction(status);
+            transactionManager.commit(status);
+            return result;


TransactionCallback 을 별도 인터페이스로 무척 잘 선언해주셨습니다. 👍
의외로 Callback 인터페이스를 중요하게 생각지 않게 넘기는 경우가 종종 있더라고요.

ghojeong · 2024-08-30T11:24:59Z

jdbc/src/main/java/camp/nextstep/transaction/support/TransactionManager.java

+    public void commit(TransactionStatus status) {
+        Connection connection = status.connection();
+
+        status.checkConnectionActive();
+        try {
+            connection.commit();
+        } catch (SQLException e) {
+            throw new DataAccessException(e);
+        }
+        DataSourceUtils.releaseConnection(connection, dataSource);
+    }
+
+    public void rollback(TransactionStatus status) {
+        Connection connection = status.connection();
+
+        status.checkConnectionActive();
+        try {
+            connection.rollback();
+        } catch (SQLException e) {
+            throw new DataAccessException(e);
+        } finally {
+            DataSourceUtils.releaseConnection(connection, dataSource);
+        }
+    }


commit 혹은 rollback 이 완료된 이후에는 DataSourceUtils.releaseConnection 를 통해 connection 을 해방하는 군요

ghojeong · 2024-08-30T11:26:11Z

jdbc/src/main/java/camp/nextstep/jdbc/datasource/DataSourceUtils.java

            connection.close();
        } catch (SQLException ex) {
            throw new CannotGetJdbcConnectionException("Failed to close JDBC Connection");
+        } finally {
+            TransactionSynchronizationManager.unbindResource(dataSource);
        }


DataSourceUtils 에서 connection 의 close 를 담당하고 있는 것 확인했습니다.

ghojeong · 2024-08-30T11:28:03Z

jdbc/src/main/java/camp/nextstep/transaction/support/TransactionSynchronizationManager.java

    public static Connection getResource(DataSource key) {
-        return null;
+        final Map<DataSource, Connection> dataSourceConnectionMap = resources.get();
+
+        return dataSourceConnectionMap.get(key);
    }

    public static void bindResource(DataSource key, Connection value) {
+        final Map<DataSource, Connection> dataSourceConnectionMap = resources.get();
+
+        if (dataSourceConnectionMap.containsKey(key)) {
+            throw new IllegalStateException("Resource is already bound to key " + key);
+        }
+
+        dataSourceConnectionMap.put(key, value);
    }

    public static Connection unbindResource(DataSource key) {
-        return null;
+        return resources.get().remove(key);
    }


안타까운 점은 이번 미션의 핵심인 transaction/support 패키지 내부의 객체에 대한 테스트가 전혀 존재하지 않는 군요.
같은 트랜잭션이라면 같은 connection 인지,
트랜잭션이 달라진다면 connection 이 달라지는지,
한번 unbind 된 resource 는 다시 조회가 불가능하는지 등등
유의미한 테스트들을 작성해주시기 부탁드립니다.

sm9171 added 18 commits August 19, 2024 15:58

docs: 1단계 요구사항 정리

63be96d

feat: findById, insert 메서드 구현

8014f59

feat: findAll 구현

d4f3ca6

feat: update 구현

e28a7f9

feat: findByAccount 구현

f7f5d6b

refactor: UserRowMapper 싱글톤 객체로 전환

4a363cb

docs: 2단계 미션 요구사항 정리

1a95004

Merge remote-tracking branch 'nextstep/sm9171' into step2

02de0fe

# Conflicts: # README.md # app/src/main/java/camp/nextstep/dao/UserDao.java # app/src/main/java/camp/nextstep/dao/UserRowMapper.java

docs: 3단계 미션 요구사항 정리

9abc36c

feat: userDao와 userHistoryDao를 한 트랜잭션으로 묶기

b241737

feat: UserHistoryDao에서 jdbctemplate의 update 메소드 사용

7e6c781

Merge remote-tracking branch 'refs/remotes/nextstep/sm9171' into step3

8e5734e

# Conflicts: # README.md

docs: 4단계 미션 요구사항 정리

838d764

feat: Transaction synchronization 적용하기

c39d2ad

feat: UserService 추상화

f97d2b2

feat: TransactionManager 구현

baf33f7

fix: update실행시 rollback이 안되는 오류 수정

13244a6

- update 메소드에서 connection을 DataSourceUtils에서 가져오는걸로 변경

Merge remote-tracking branch 'nextstep/sm9171' into step4

5c90c17

# Conflicts: # README.md # app/src/main/java/camp/nextstep/dao/UserHistoryDao.java # app/src/main/java/camp/nextstep/service/UserService.java # app/src/test/java/camp/nextstep/service/UserServiceTest.java

ghojeong requested changes Aug 30, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

🚀 4단계 - 트랜잭션 동기화 구현하기 #41

🚀 4단계 - 트랜잭션 동기화 구현하기 #41

sm9171 commented Aug 30, 2024

ghojeong commented Aug 30, 2024

ghojeong left a comment

ghojeong Aug 30, 2024

ghojeong Aug 30, 2024

ghojeong Aug 30, 2024

ghojeong Aug 30, 2024

ghojeong Aug 30, 2024

ghojeong Aug 30, 2024

ghojeong Aug 30, 2024

		try (final Connection connection = DataSourceUtils.getConnection(dataSource);
		final PreparedStatement pstmt = connection.prepareStatement(sql)) {

🚀 4단계 - 트랜잭션 동기화 구현하기 #41

Are you sure you want to change the base?

🚀 4단계 - 트랜잭션 동기화 구현하기 #41

Conversation

sm9171 commented Aug 30, 2024

ghojeong commented Aug 30, 2024

ghojeong left a comment

Choose a reason for hiding this comment

ghojeong Aug 30, 2024

Choose a reason for hiding this comment

ghojeong Aug 30, 2024

Choose a reason for hiding this comment

ghojeong Aug 30, 2024

Choose a reason for hiding this comment

ghojeong Aug 30, 2024

Choose a reason for hiding this comment

ghojeong Aug 30, 2024

Choose a reason for hiding this comment

ghojeong Aug 30, 2024

Choose a reason for hiding this comment

ghojeong Aug 30, 2024

Choose a reason for hiding this comment