Skip to content
11 changes: 0 additions & 11 deletions app/src/main/java/com/techcourse/dao/UserDao.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
import com.interface21.jdbc.core.JdbcTemplate;
import com.interface21.jdbc.core.RowMapper;
import com.techcourse.domain.User;
import java.sql.Connection;
import java.util.List;
import java.util.Optional;
import javax.sql.DataSource;
Expand Down Expand Up @@ -41,11 +40,6 @@ public void update(final User user) {
jdbcTemplate.update(sql, user.getAccount(), user.getPassword(), user.getEmail(), user.getId());
}

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

public List<User> findAll() {
final var sql = "select id, account, password, email from users";
return jdbcTemplate.query(sql, ROW_MAPPER);
Expand All @@ -56,11 +50,6 @@ public Optional<User> findById(final Long id) {
return jdbcTemplate.queryForObject(sql, ROW_MAPPER, id);
}

public Optional<User> findById(Connection connection, final Long id) {
final var sql = "select id, account, password, email from users where id = ?";
return jdbcTemplate.queryForObject(connection, sql, ROW_MAPPER, id);
}

public Optional<User> findByAccount(final String account) {
final var sql = "select id, account, password, email from users where account = ?";
return jdbcTemplate.queryForObject(sql, ROW_MAPPER, account);
Expand Down
4 changes: 1 addition & 3 deletions app/src/main/java/com/techcourse/dao/UserHistoryDao.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

import com.interface21.jdbc.core.JdbcTemplate;
import com.techcourse.domain.UserHistory;
import java.sql.Connection;
import javax.sql.DataSource;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand All @@ -21,10 +20,9 @@ public UserHistoryDao(final JdbcTemplate jdbcTemplate) {
this.jdbcTemplate = jdbcTemplate;
}

public void log(Connection connection, final UserHistory userHistory) {
public void log(final UserHistory userHistory) {
final var sql = "insert into user_history (user_id, account, password, email, created_at, created_by) values (?, ?, ?, ?, ?, ?)";
jdbcTemplate.update(
connection,
sql,
userHistory.getUserId(),
userHistory.getAccount(),
Expand Down
37 changes: 37 additions & 0 deletions app/src/main/java/com/techcourse/service/AppUserService.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
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 UserServiceInterface {

private final UserDao userDao;
private final UserHistoryDao userHistoryDao;

public AppUserService(UserDao userDao, UserHistoryDao userHistoryDao) {
this.userDao = userDao;
this.userHistoryDao = userHistoryDao;
}

@Override
public User findById(long id) {
return userDao.findById(id)
.orElseThrow(() -> new IllegalArgumentException("사용자 정보가 존재하지 않습니다."));
}

@Override
public void save(User user) {
userDao.insert(user);
}

@Override
public void changePassword(long id, String newPassword, String createdBy) {
final var user = userDao.findById(id)
.orElseThrow(() -> new IllegalArgumentException("사용자 정보가 존재하지 않습니다."));
user.changePassword(newPassword);
userDao.update(user);
userHistoryDao.log(new UserHistory(user, createdBy));
}
}
86 changes: 86 additions & 0 deletions app/src/main/java/com/techcourse/service/TxUserService.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
package com.techcourse.service;

import com.interface21.dao.DataAccessException;
import com.interface21.jdbc.core.TransactionCallback;
import com.interface21.jdbc.datasource.DataSourceUtils;
import com.techcourse.config.DataSourceConfig;
import com.techcourse.domain.User;
import java.sql.Connection;
import java.sql.SQLException;
import javax.sql.DataSource;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

public class TxUserService implements UserServiceInterface {

private static final Logger log = LoggerFactory.getLogger(TxUserService.class);
private final UserServiceInterface userServiceInterface;

public TxUserService(UserServiceInterface userServiceInterface) {
this.userServiceInterface = userServiceInterface;
}

// override 대상인 메서드는 userService의 메서드를 그대로 위임(delegate)한다.
@Override
public void changePassword(final long id, final String newPassword, final String createdBy) {
executeTransaction(() -> userServiceInterface.changePassword(id, newPassword, createdBy));
Copy link

Choose a reason for hiding this comment

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

함수 인터페이스를 통해 트랜잭션 관련 코드의 중복을 없앤 점 좋습니다! 👍

}

@Override
public void save(User user) {
executeTransaction(() -> userServiceInterface.save(user));
}

@Override
public User findById(long id) {
return executeTransaction(() -> userServiceInterface.findById(id));
}

private void executeTransaction(Runnable runnable) {
executeTransaction(() -> {
runnable.run();
return null;
});
}

private <T> T executeTransaction(TransactionCallback<T> transactionCallback) {
DataSource dataSource = DataSourceConfig.getInstance();
Connection connection = null;
try {
connection = DataSourceUtils.getConnection(dataSource);
connection.setAutoCommit(false);
T result = transactionCallback.doInTransaction();
connection.commit();
return result;
} catch (SQLException e) {
if (connection != null) {
try {
connection.rollback();
} catch (SQLException ex) {
log.error("Rollback failed", ex);
throw new DataAccessException("Rollback failed");
}
}
throw new DataAccessException(e);
Copy link

Choose a reason for hiding this comment

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

확실히 AI 리뷰와 마찬가지로 데이터 접근 예외만 DataAccessException로 매핑하고, 도메인 관련 예외는 기존 예외 그대로 외부에 전파되면 좋을 것 같습니다!

Copy link
Author

Choose a reason for hiding this comment

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

동의해요! 반영해서 수정했습니다 ☺️

} catch (RuntimeException e) {
if (connection != null) {
try {
connection.rollback();
} catch (SQLException ex) {
log.error("Rollback failed", ex);
throw new DataAccessException("Rollback failed");
}
}
throw e;
} finally {
if (connection != null) {
try {
DataSourceUtils.releaseConnection(connection, dataSource);
} catch (Exception ex) {
log.error("Failed to release connection or unbind resource", ex);
}
}
Comment on lines +76 to +82
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

트랜잭션 종료 후 커넥션 해제가 이뤄지지 않는 문제를 확인해주세요

finally 블록에서 DataSourceUtils.releaseConnection을 호출하지만, 해당 유틸은 TransactionSynchronizationManager.hasResource(dataSource)true면 즉시 반환하도록 되어 있습니다. 현재 흐름에서는 getConnection에서 이미 리소스를 ThreadLocal에 바인딩했고, 커밋/롤백 이후에도 언바인딩을 하지 않아서 이 조건이 계속 true인 상태로 남습니다. 그 결과 커넥션이 닫히지도, ThreadLocal에서 해제되지도 않아 같은 스레드가 다음 요청을 처리할 때도 동일 커넥션이 autoCommit=false 상태로 남아 버립니다. 커넥션 풀이 반환되지 않아 고갈될 수도 있고, 데이터 정합성도 깨질 수 있겠지요. 트랜잭션 경계가 끝난 뒤에는 어떤 순서로 자원을 언바인딩해야 안전할지, 그리고 예외 경로에서도 동일하게 정리가 되는지 한 번 점검해 보실 수 있을까요? ThreadLocal에 남은 커넥션이 실제로 어떻게 재사용되는지 로그로 확인해 보는 것도 도움이 될 것 같습니다.

}
}
}

40 changes: 26 additions & 14 deletions app/src/main/java/com/techcourse/service/UserService.java
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
package com.techcourse.service;

import com.interface21.dao.DataAccessException;
import com.techcourse.config.DataSourceConfig;
import com.interface21.jdbc.datasource.DataSourceUtils;
import com.techcourse.dao.UserDao;
import com.techcourse.dao.UserHistoryDao;
import com.techcourse.domain.User;
import com.techcourse.domain.UserHistory;
import java.sql.Connection;
import java.sql.SQLException;
import javax.sql.DataSource;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand All @@ -16,10 +17,12 @@ public class UserService {
private static final Logger log = LoggerFactory.getLogger(UserService.class);
private final UserDao userDao;
private final UserHistoryDao userHistoryDao;
private final DataSource dataSource;

public UserService(final UserDao userDao, final UserHistoryDao userHistoryDao) {
public UserService(UserDao userDao, UserHistoryDao userHistoryDao, DataSource dataSource) {
this.userDao = userDao;
this.userHistoryDao = userHistoryDao;
this.dataSource = dataSource;
}

public User findById(final long id) {
Expand All @@ -32,26 +35,35 @@ public void insert(final User user) {
}

public void changePassword(final long id, final String newPassword, final String createBy) {
try (Connection connection = DataSourceConfig.getInstance().getConnection()) {
Connection connection = null;
try {
connection = DataSourceUtils.getConnection(dataSource);
connection.setAutoCommit(false);
try {
final var user = userDao.findById(connection, id)
.orElseThrow(() -> new IllegalArgumentException("사용자 정보가 존재하지 않습니다."));
user.changePassword(newPassword);
userDao.update(connection, user);
userHistoryDao.log(connection, new UserHistory(user, createBy));
connection.commit();
} catch (Exception e) {

final var user = userDao.findById(id)
.orElseThrow(() -> new IllegalArgumentException("사용자 정보가 존재하지 않습니다."));
user.changePassword(newPassword);
userDao.update(user);
userHistoryDao.log(new UserHistory(user, createBy));

connection.commit();
} catch (Exception e) {
if (connection != null) {
try {
connection.rollback();
} catch (SQLException ex) {
log.error("Rollback failed", ex);
}
throw e;
}
} catch (SQLException e) {
log.error(e.getMessage(), e);
throw new DataAccessException(e);
} finally {
if (connection != null) {
try {
DataSourceUtils.releaseConnection(connection, dataSource);
} catch (Exception ex) {
log.error("Failed to release connection or unbind resource", ex);
}
}
}
}
}
11 changes: 11 additions & 0 deletions app/src/main/java/com/techcourse/service/UserServiceInterface.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package com.techcourse.service;

import com.techcourse.domain.User;

public interface UserServiceInterface {
User findById(final long id);

void save(final User user);

void changePassword(final long id, final String newPassword, final String createdBy);
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
import com.interface21.jdbc.core.JdbcTemplate;
import com.techcourse.dao.UserHistoryDao;
import com.techcourse.domain.UserHistory;
import java.sql.Connection;

public class MockUserHistoryDao extends UserHistoryDao {

Expand All @@ -13,7 +12,7 @@ public MockUserHistoryDao(final JdbcTemplate jdbcTemplate) {
}

@Override
public void log(Connection connection, final UserHistory userHistory) {
public void log(final UserHistory userHistory) {
throw new DataAccessException();
}
}
12 changes: 8 additions & 4 deletions app/src/test/java/com/techcourse/service/UserServiceTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ void setUp() {
@Test
void testChangePassword() {
final var userHistoryDao = new UserHistoryDao(jdbcTemplate);
final var userService = new UserService(userDao, userHistoryDao);
final var appUserService = new AppUserService(userDao, userHistoryDao);
final var userService = new TxUserService(appUserService);

final var newPassword = "qqqqq";
final var createBy = "gugu";
Expand All @@ -46,13 +47,16 @@ void testChangePassword() {
void testTransactionRollback() {
// 트랜잭션 롤백 테스트를 위해 mock으로 교체
final var userHistoryDao = new MockUserHistoryDao(jdbcTemplate);
final var userService = new UserService(userDao, userHistoryDao);
// 애플리케이션 서비스
final var appUserService = new AppUserService(userDao, userHistoryDao);
// 트랜잭션 서비스 추상화
final var userService = new TxUserService(appUserService);

final var newPassword = "newPassword";
final var createBy = "gugu";
final var createdBy = "gugu";
// 트랜잭션이 정상 동작하는지 확인하기 위해 의도적으로 MockUserHistoryDao에서 예외를 발생시킨다.
assertThrows(DataAccessException.class,
() -> userService.changePassword(1L, newPassword, createBy));
() -> userService.changePassword(1L, newPassword, createdBy));

final var actual = userService.findById(1L);

Expand Down
43 changes: 13 additions & 30 deletions jdbc/src/main/java/com/interface21/jdbc/core/JdbcTemplate.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import com.interface21.dao.DataAccessException;
import com.interface21.dao.IncorrectResultSizeException;
import com.interface21.jdbc.datasource.DataSourceUtils;
import java.sql.Connection;
import java.sql.PreparedStatement;
import java.sql.ResultSet;
Expand All @@ -27,20 +28,10 @@ public void update(String sql, Object... parameters) {
execute(PreparedStatement::executeUpdate, sql, parameters);
}

public void update(Connection connection, String sql, Object... parameters) {
execute(connection, PreparedStatement::executeUpdate, sql, parameters);
}

public <T> List<T> query(String sql, RowMapper<T> rowMapper, Object... parameters) {
return execute((preparedStatement) -> executeQuery(preparedStatement, rowMapper), sql, parameters);
}

public <T> List<T> query(Connection connection, String sql, RowMapper<T> rowMapper, Object... parameters) {
return execute(connection,
(preparedStatement) -> executeQuery(preparedStatement, rowMapper), sql, parameters
);
}

private <T> List<T> executeQuery(PreparedStatement preparedStatement, RowMapper<T> rowMapper) throws SQLException {
List<T> results = new ArrayList<>();
try (ResultSet resultSet = preparedStatement.executeQuery()) {
Expand All @@ -56,30 +47,22 @@ public <T> Optional<T> queryForObject(String sql, RowMapper<T> rowMapper, Object
return extractSingleResult(results);
}

public <T> Optional<T> queryForObject(Connection connection, String sql, RowMapper<T> rowMapper,
Object... parameters) {
List<T> results = query(connection, sql, rowMapper, parameters);
return extractSingleResult(results);
}

private <T> T execute(PreparedStatementCallback<T> preparedStatementCallback, String sql, Object... parameters) {
try (Connection connection = dataSource.getConnection()) {
return execute(connection, preparedStatementCallback, sql, parameters);
} catch (SQLException e) {
log.error(e.getMessage(), e);
throw new DataAccessException(e);
}
}

private <T> T execute(Connection connection, PreparedStatementCallback<T> preparedStatementCallback, String sql,
Object... parameters) {
try (PreparedStatement preparedStatement = connection.prepareStatement(sql)) {
log.debug("query : {}", sql);
setParameters(preparedStatement, parameters);
return preparedStatementCallback.doInPreparedStatement(preparedStatement);
Connection connection = null;
try {
connection = DataSourceUtils.getConnection(dataSource);
try (PreparedStatement preparedStatement = connection.prepareStatement(sql)) {
log.debug("query : {}", sql);
setParameters(preparedStatement, parameters);
return preparedStatementCallback.doInPreparedStatement(preparedStatement);
}
} catch (SQLException e) {
log.error(e.getMessage(), e);
throw new DataAccessException(e);
} finally {
if (connection != null) {
DataSourceUtils.releaseConnection(connection, dataSource);
}
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package com.interface21.jdbc.core;

@FunctionalInterface
public interface TransactionCallback<T> {

T doInTransaction();
}
Loading