Skip to content

[3단계 - Transaction 적용하기] 링크(손준형) 미션 제출합니다. #1158

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.

Sign up for GitHub

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

Open
wants to merge 5 commits into
base: sonjh919
Choose a base branch
from
Open

[3단계 - Transaction 적용하기] 링크(손준형) 미션 제출합니다. #1158

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
3a3e429
feat: implement TransactionSynchronizationManager
sonjh919 Oct 17, 2025
982aba7
feat: integrate DataSourceUtils with JdbcTemplate
sonjh919 Oct 17, 2025
c65efea
refactor: migrate UserHistoryDao to JdbcTemplate
sonjh919 Oct 17, 2025
39be9d5
feat: apply transaction to UserService.changePassword()
sonjh919 Oct 17, 2025
a7b9f73
test: enable UserServiceTest and verify transaction
sonjh919 Oct 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 11 additions & 41 deletions app/src/main/java/com/techcourse/dao/UserHistoryDao.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,61 +2,31 @@

import com.techcourse.domain.UserHistory;
import com.interface21.jdbc.core.JdbcTemplate;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import javax.sql.DataSource;
import java.sql.Connection;
import java.sql.PreparedStatement;
import java.sql.SQLException;

public class UserHistoryDao {

private static final Logger log = LoggerFactory.getLogger(UserHistoryDao.class);

private final DataSource dataSource;
private final JdbcTemplate jdbcTemplate;

public UserHistoryDao(final DataSource dataSource) {
this.dataSource = dataSource;
this.jdbcTemplate = new JdbcTemplate(dataSource);
}

public UserHistoryDao(final JdbcTemplate jdbcTemplate) {
this.dataSource = null;
this.jdbcTemplate = jdbcTemplate;
}

public void log(final UserHistory userHistory) {
final var sql = "insert into user_history (user_id, account, password, email, created_at, created_by) values (?, ?, ?, ?, ?, ?)";

Connection conn = null;
PreparedStatement pstmt = null;
try {
conn = dataSource.getConnection();
pstmt = conn.prepareStatement(sql);

log.debug("query : {}", sql);

pstmt.setLong(1, userHistory.getUserId());
pstmt.setString(2, userHistory.getAccount());
pstmt.setString(3, userHistory.getPassword());
pstmt.setString(4, userHistory.getEmail());
pstmt.setObject(5, userHistory.getCreatedAt());
pstmt.setString(6, userHistory.getCreateBy());
pstmt.executeUpdate();
} catch (SQLException e) {
log.error(e.getMessage(), e);
throw new RuntimeException(e);
} finally {
try {
if (pstmt != null) {
pstmt.close();
}
} catch (SQLException ignored) {}

try {
if (conn != null) {
conn.close();
}
} catch (SQLException ignored) {}
}
jdbcTemplate.update(sql,
userHistory.getUserId(),
userHistory.getAccount(),
userHistory.getPassword(),
userHistory.getEmail(),
userHistory.getCreatedAt(),
userHistory.getCreateBy()
);
}
}
43 changes: 43 additions & 0 deletions app/src/main/java/com/techcourse/service/UserService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,30 @@
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 class UserService {

private final UserDao userDao;
private final UserHistoryDao userHistoryDao;
private final DataSource dataSource;

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) {
Expand All @@ -24,6 +39,34 @@ public void insert(final User 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);
Expand Down
6 changes: 2 additions & 4 deletions app/src/test/java/com/techcourse/service/UserServiceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,11 @@
import com.interface21.dao.DataAccessException;
import com.interface21.jdbc.core.JdbcTemplate;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;

import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.jupiter.api.Assertions.assertThrows;

@Disabled
class UserServiceTest {

private JdbcTemplate jdbcTemplate;
Expand All @@ -33,7 +31,7 @@ void setUp() {
@Test
void testChangePassword() {
final var userHistoryDao = new UserHistoryDao(jdbcTemplate);
final var userService = new UserService(userDao, userHistoryDao);
final var userService = new UserService(userDao, userHistoryDao, DataSourceConfig.getInstance());

final var newPassword = "qqqqq";
final var createBy = "gugu";
Expand All @@ -48,7 +46,7 @@ void testChangePassword() {
void testTransactionRollback() {
// 트랜잭션 롤백 테스트를 위해 mock으로 교체
final var userHistoryDao = new MockUserHistoryDao(jdbcTemplate);
final var userService = new UserService(userDao, userHistoryDao);
final var userService = new UserService(userDao, userHistoryDao, DataSourceConfig.getInstance());

final var newPassword = "newPassword";
final var createBy = "gugu";
Expand Down
22 changes: 10 additions & 12 deletions jdbc/src/main/java/com/interface21/jdbc/core/JdbcTemplate.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,24 +34,25 @@ public JdbcTemplate(final DataSource dataSource) {
/**
* SQL 실행의 핵심 메서드. 자원 관리 관련 중복 부분을 함수형 인터페이스를 이용하여 분리함.
* - Connection, PreparedStatement를 생성하고, 파라미터를 바인딩한 후 StatementExecutor를 통해 실행한다.
* - 자원 해제를 try-with-resources로 자동 처리한다.
* - Connection은 트랜잭션 컨텍스트를 고려하여 DataSourceUtils를 통해 관리
* - PreparedStatement는 자동으로 close
*
* @param sql 실행할 SQL문
* @param args PreparedStatement에 바인딩할 파라미터 배열
* @param executor PreparedStatement를 실행하는 로직을 가진 함수형 인터페이스
* @return 제네릭 타입 결과값 (쿼리 결과나 update 결과 등)
*/
public <R> R execute(String sql, Object[] args, StatementExecutor<R> executor) {
try (
Connection conn = getConnection();
PreparedStatement pstmt = conn.prepareStatement(sql)
) {
final Connection conn = getConnection();
try (PreparedStatement pstmt = conn.prepareStatement(sql)) {
setPreparedStatementParameter(args, pstmt);
log.info("query = {}", sql);

return executor.execute(pstmt);
} catch (SQLException e) {
throw new DataAccessException("sql 실행 과정에서 문제가 발생하였습니다.", e);
} finally {
com.interface21.jdbc.datasource.DataSourceUtils.releaseConnection(conn, dataSource);
}
Comment on lines +46 to 56
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

치명적: 비(非)트랜잭션 경로에서 커넥션 해제가 되지 않을 수 있음

execute()가 finally에서 releaseConnection을 호출하지만, DataSourceUtils가 “바인딩된 커넥션은 닫지 않음”으로 정의되어 있어 트랜잭션이 없는 호출에서도 커넥션이 닫히지 않을 수 있습니다. 현재 DataSourceUtils는 getConnection 시 무조건 바인딩하므로 둘의 결합으로 누수가 발생합니다.

해결 방향(설계 힌트):

  • “트랜잭션 활성” 개념을 도입하여 JdbcTemplate의 일반 호출은 바인딩 없이 획득/즉시 해제되도록 하고, 서비스 경계에서만 바인딩을 사용,
  • 또는 트랜잭션 템플릿을 통해 경계 내 execute 호출들이 바인딩된 커넥션을 재사용하도록 통제.

왜 중요한가: 커넥션 풀 고갈, 요청 지연/장애로 직결됩니다.

Also applies to: 143-145

🤖 Prompt for AI Agents 
jdbc/src/main/java/com/interface21/jdbc/core/JdbcTemplate.java around lines
46-56 (also applies to 143-145): the current code always obtains a connection in
a way that DataSourceUtils treats as "bound", so finally-releaseConnection may
not close it on non-transactional paths, causing pool leaks. Fix by
distinguishing transactional vs non-transactional acquisition: when no active
transaction, obtain a non-bound connection (or mark it non-transactional) and
ensure it is closed in finally; when in a transaction, obtain/keep a bound
connection so it can be reused and released by transaction management.
Concretely, add a transactionActive check (or use
DataSourceUtils.isConnectionTransactional) before obtaining/deciding how to
release the connection, and ensure releaseConnection actually closes non-bound
connections (or call conn.close() for non-bound) while leaving bound connections
to transaction manager; alternatively move connection-binding logic into a
TransactionTemplate so plain execute() uses short-lived connections.

}

Expand Down Expand Up @@ -133,16 +134,13 @@ private <T> List<T> getQueryResult(RowMapper<T> rowMapper, PreparedStatement pst
}

/**
* DataSource에서 새로운 데이터베이스 Connection 객체 획득
* DataSource에서 데이터베이스 Connection 객체 획득
* DataSourceUtils를 사용하여 트랜잭션 컨텍스트에서 관리되는 Connection을 재사용
*
* @return 데이터베이스에 연결된 Connection 객체
* @throws DataAccessException 커넥션 획득 과정에서 SQL 오류가 발생한 경우
* @throws DataAccessException 커넥션 획득 과정에서 오류가 발생한 경우
*/
private Connection getConnection() {
try {
return dataSource.getConnection();
} catch (SQLException e) {
throw new DataAccessException(e);
}
return com.interface21.jdbc.datasource.DataSourceUtils.getConnection(dataSource);
}
}
11 changes: 11 additions & 0 deletions jdbc/src/main/java/com/interface21/jdbc/datasource/DataSourceUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,17 @@ public static Connection getConnection(DataSource dataSource) throws CannotGetJd
}

public static void releaseConnection(Connection connection, DataSource dataSource) {
if (connection == null) {
return;
}

// 트랜잭션 컨텍스트에서 관리되는 Connection인 경우 닫지 않음
Connection boundConnection = TransactionSynchronizationManager.getResource(dataSource);
if (boundConnection == connection) {
return;
}

Comment on lines +31 to +40
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

치명적: 트랜잭션 외부에서 커넥션 누수 가능성

현재 동작 조합:

  • getConnection: 바인딩이 없으면 새 커넥션을 획득 “하고” TransactionSynchronizationManager에 바인딩합니다.
  • releaseConnection: 바인딩된 커넥션이면 닫지 않고 반환합니다.

트랜잭션이 없는 일반 JdbcTemplate 호출에서도 커넥션이 바인딩되고, JdbcTemplate의 finally에서 releaseConnection이 “바인딩됨”을 이유로 닫지 않아 스레드-로컬에 남을 수 있습니다. 서비스 계층이 unbind를 수행하지 않는 한 커넥션이 장시간 유지/고갈될 위험이 있습니다.

개선 방향(설계 힌트):

  • “트랜잭션 활성” 신호가 있을 때만 바인딩하도록 분기(예: TSM에 active 플래그/스택 도입)하거나,
  • 트랜잭션 경계에서만 명시적으로 bind/unbind하고, 일반 getConnection은 바인딩하지 않도록 역할을 분리,
  • release 시 “트랜잭션 활성”을 고려하여 닫을지/유지할지를 결정.

왜 중요한가: 리소스 누수는 장애로 직결되며, 테스트 환경에서는 드러나지 않고 운영에서만 표면화됩니다.

// 트랜잭션 컨텍스트에 없는 Connection만 닫음
try {
connection.close();
} catch (SQLException ex) {
Expand Down
23 changes: 21 additions & 2 deletions .../src/main/java/com/interface21/transaction/support/TransactionSynchronizationManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import javax.sql.DataSource;
import java.sql.Connection;
import java.util.HashMap;
import java.util.Map;

public abstract class TransactionSynchronizationManager {
Expand All @@ -11,13 +12,31 @@ public abstract class TransactionSynchronizationManager {
private TransactionSynchronizationManager() {}

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

public static void bindResource(DataSource key, Connection value) {
Map<DataSource, Connection> map = resources.get();
if (map == null) {
map = new HashMap<>();
resources.set(map);
}
map.put(key, value);
}

public static Connection unbindResource(DataSource key) {
return null;
final Map<DataSource, Connection> map = resources.get();
if (map == null) {
return null;
}
final Connection connection = map.remove(key);
if (map.isEmpty()) {
resources.remove();
}
return connection;
}
}