-
Notifications
You must be signed in to change notification settings - Fork 379
[4단계 - Transaction synchronization 적용하기] 링크(손준형) 미션 제출합니다. #1215
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 7 commits
f843ebb
d2841a4
c9b09d0
9dfe2cf
5a885cb
5a2fdd7
16b0c93
53a75d5
8641863
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| package com.techcourse.service; | ||
|
|
||
| import com.techcourse.dao.UserDao; | ||
| import com.techcourse.dao.UserHistoryDao; | ||
| import com.techcourse.domain.User; | ||
| import com.techcourse.domain.UserHistory; | ||
|
|
||
| public class AppUserService implements UserService { | ||
|
|
||
| private final UserDao userDao; | ||
| private final UserHistoryDao userHistoryDao; | ||
|
|
||
| public AppUserService(final UserDao userDao, final UserHistoryDao userHistoryDao) { | ||
| this.userDao = userDao; | ||
| this.userHistoryDao = userHistoryDao; | ||
| } | ||
|
|
||
| @Override | ||
| public User findById(final long id) { | ||
| return userDao.findById(id); | ||
| } | ||
|
|
||
| @Override | ||
| public void insert(final User user) { | ||
| userDao.insert(user); | ||
| } | ||
|
|
||
| @Override | ||
| public void changePassword(final long id, final String newPassword, final String createdBy) { | ||
| final var user = findById(id); | ||
| user.changePassword(newPassword); | ||
| userDao.update(user); | ||
| userHistoryDao.log(new UserHistory(user, createdBy)); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,62 @@ | ||
| package com.techcourse.service; | ||
|
|
||
| import com.techcourse.domain.User; | ||
| import com.interface21.dao.DataAccessException; | ||
| import com.interface21.jdbc.datasource.DataSourceUtils; | ||
| import com.interface21.transaction.support.TransactionSynchronizationManager; | ||
|
|
||
| import javax.sql.DataSource; | ||
| import java.sql.Connection; | ||
| import java.sql.SQLException; | ||
|
|
||
| public class TxUserService implements UserService { | ||
|
|
||
| private final UserService userService; | ||
| private final DataSource dataSource; | ||
|
|
||
| public TxUserService(final UserService userService, final DataSource dataSource) { | ||
| this.userService = userService; | ||
| this.dataSource = dataSource; | ||
| } | ||
|
|
||
| @Override | ||
| public User findById(final long id) { | ||
| return userService.findById(id); | ||
| } | ||
|
|
||
| @Override | ||
| public void insert(final User user) { | ||
| userService.insert(user); | ||
| } | ||
|
|
||
| @Override | ||
| public void changePassword(final long id, final String newPassword, final String createdBy) { | ||
| final Connection connection = DataSourceUtils.getConnection(dataSource); | ||
| TransactionSynchronizationManager.bindResource(dataSource, connection); | ||
|
|
||
| boolean originalAutoCommit = false; | ||
|
||
| try { | ||
| originalAutoCommit = connection.getAutoCommit(); | ||
| connection.setAutoCommit(false); | ||
|
|
||
| userService.changePassword(id, newPassword, createdBy); | ||
|
|
||
| connection.commit(); | ||
| } catch (Exception e) { | ||
| try { | ||
| connection.rollback(); | ||
| } catch (SQLException rollbackException) { | ||
| throw new DataAccessException("Failed to rollback transaction", rollbackException); | ||
| } | ||
| throw new DataAccessException(e); | ||
| } finally { | ||
| try { | ||
| connection.setAutoCommit(originalAutoCommit); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 만약 여기서 SQLException이 발생 시,
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. finally 블록 안에서 각각의 리소스 해제 작업을 개별 try-catch로 감싸서, 하나의 예외 발생이 나머지 해제 작업을 방해하지 않도록 리팩토링하였습니다!! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 확인했습니다~ |
||
| } catch (SQLException e) { | ||
| throw new DataAccessException("Failed to reset autoCommit", e); | ||
| } | ||
| TransactionSynchronizationManager.unbindResource(dataSource); | ||
| DataSourceUtils.releaseConnection(connection, dataSource); | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,75 +1,12 @@ | ||
| package com.techcourse.service; | ||
|
|
||
| import com.techcourse.dao.UserDao; | ||
| import com.techcourse.dao.UserHistoryDao; | ||
| import com.techcourse.domain.User; | ||
| import com.techcourse.domain.UserHistory; | ||
| import com.interface21.dao.DataAccessException; | ||
| import com.interface21.jdbc.datasource.DataSourceUtils; | ||
| import com.interface21.transaction.support.TransactionSynchronizationManager; | ||
|
|
||
| import javax.sql.DataSource; | ||
| import java.sql.Connection; | ||
| import java.sql.SQLException; | ||
| public interface UserService { | ||
|
|
||
| public class UserService { | ||
| User findById(final long id); | ||
|
|
||
| private final UserDao userDao; | ||
| private final UserHistoryDao userHistoryDao; | ||
| private final DataSource dataSource; | ||
| void insert(final User user); | ||
|
|
||
| public UserService(final UserDao userDao, final UserHistoryDao userHistoryDao) { | ||
| this.userDao = userDao; | ||
| this.userHistoryDao = userHistoryDao; | ||
| this.dataSource = null; | ||
| } | ||
|
|
||
| public UserService(final UserDao userDao, final UserHistoryDao userHistoryDao, final DataSource dataSource) { | ||
| this.userDao = userDao; | ||
| this.userHistoryDao = userHistoryDao; | ||
| this.dataSource = dataSource; | ||
| } | ||
|
|
||
| public User findById(final long id) { | ||
| return userDao.findById(id); | ||
| } | ||
|
|
||
| public void insert(final User user) { | ||
| userDao.insert(user); | ||
| } | ||
|
|
||
| public void changePassword(final long id, final String newPassword, final String createBy) { | ||
| if (dataSource == null) { | ||
| // 트랜잭션 없이 실행 | ||
| executeChangePassword(id, newPassword, createBy); | ||
| return; | ||
| } | ||
|
|
||
| // 트랜잭션과 함께 실행 | ||
| final Connection connection = DataSourceUtils.getConnection(dataSource); | ||
| try { | ||
| connection.setAutoCommit(false); | ||
|
|
||
| executeChangePassword(id, newPassword, createBy); | ||
|
|
||
| connection.commit(); | ||
| } catch (Exception e) { | ||
| try { | ||
| connection.rollback(); | ||
| } catch (SQLException rollbackException) { | ||
| throw new DataAccessException("Failed to rollback transaction", rollbackException); | ||
| } | ||
| throw new DataAccessException(e); | ||
| } finally { | ||
| TransactionSynchronizationManager.unbindResource(dataSource); | ||
| DataSourceUtils.releaseConnection(connection, dataSource); | ||
| } | ||
| } | ||
|
|
||
| private void executeChangePassword(final long id, final String newPassword, final String createBy) { | ||
| final var user = findById(id); | ||
| user.changePassword(newPassword); | ||
| userDao.update(user); | ||
| userHistoryDao.log(new UserHistory(user, createBy)); | ||
| } | ||
| void changePassword(final long id, final String newPassword, final String createdBy); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
커넥션 획득 단계에서 예외가 발생하면 어떻게 될까요?
DataSourceUtils.getConnection()에서 커넥션을 얻은 직후bindResource()를 호출하는데, 만약bindResource()과정에서 예외가 발생한다면 이미 획득한 커넥션이 정리되지 않을 수 있습니다.커넥션 획득과 바인딩 과정도 리소스 관리 측면에서 안전하게 처리할 방법을 고려해보시면 좋을 것 같아요.
트랜잭션 관리 및 커넥션 리소스 핸들링을 검증하기 위해 실제 코드를 살펴보겠습니다.
Length of output: 110
🏁 Script executed:
Length of output: 2408
커넥션 획득 및 바인딩 단계에서 리소스 누수 위험 있음
현재 코드를 살펴보니 33-35번 라인의 우려가 실제로 존재합니다.
DataSourceUtils.getConnection()또는TransactionSynchronizationManager.bindResource()에서 예외가 발생하면, 이미 획득한 커넥션이 37번 라인부터 시작하는 try-finally 블록에 도달하지 않아 정리되지 않습니다.예를 들어 bindResource()에서 예외가 발생한 경우:
개선 방향:
커넥션 획득과 바인딩 과정도 try-catch로 감싸거나, 이 로직을 기존 try 블록 내부로 이동시켜 반드시 finally 블록이 실행되도록 구조화하는 것을 고려해보세요.
🤖 Prompt for AI Agents