Skip to content

[3단계 - Transaction 적용하기] 메이(김시원) 미션 제출합니다. #1131

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 4 commits into
base: seaniiio
Choose a base branch
from
Open

[3단계 - Transaction 적용하기] 메이(김시원) 미션 제출합니다. #1131

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e126048
refactor: UserServiceTest의 `@Disabled` 제거
seaniiio Oct 14, 2025
1c44a66
refactor: 비밀번호 변경 작업에 트랜잭션 적용
seaniiio Oct 14, 2025
fe81844
refactor: 서비스 코드에서 트랜잭션 관리 로직 분리
seaniiio Oct 14, 2025
5a1e12b
refactor: PreparedStatement에 파라미터 세팅 전에 null 여부 체크
seaniiio Oct 15, 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
6 changes: 6 additions & 0 deletions app/src/main/java/com/techcourse/dao/UserDao.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import com.interface21.jdbc.core.JdbcTemplate;
import com.techcourse.domain.User;
import java.sql.Connection;
import java.util.List;

public class UserDao {
Expand All @@ -17,6 +18,11 @@ public void insert(final User user) {
jdbcTemplate.update(sql, user.getAccount(), user.getPassword(), user.getEmail());
}

public void update(final 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 void update(final User user) {
final var sql = "update users set account = ?, password = ?, email = ? where id = ?";
jdbcTemplate.update(sql, user.getAccount(), user.getPassword(), user.getEmail(), user.getId());
Expand Down
44 changes: 37 additions & 7 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
@@ -1,14 +1,13 @@
package com.techcourse.dao;

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

import javax.sql.DataSource;
import com.techcourse.domain.UserHistory;
import java.sql.Connection;
import java.sql.PreparedStatement;
import java.sql.SQLException;
import javax.sql.DataSource;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

public class UserHistoryDao {

Expand Down Expand Up @@ -50,13 +49,44 @@ public void log(final UserHistory userHistory) {
if (pstmt != null) {
pstmt.close();
}
} catch (SQLException ignored) {}
} catch (SQLException ignored) {
}

try {
if (conn != null) {
conn.close();
}
} catch (SQLException ignored) {}
} catch (SQLException ignored) {
}
}
}

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

PreparedStatement pstmt = null;
try {
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) {
}
}
}
}
13 changes: 10 additions & 3 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
@@ -1,18 +1,22 @@
package com.techcourse.service;

import com.interface21.transaction.TransactionManager;
import com.techcourse.dao.UserDao;
import com.techcourse.dao.UserHistoryDao;
import com.techcourse.domain.User;
import com.techcourse.domain.UserHistory;
import javax.sql.DataSource;

public class UserService {

private final UserDao userDao;
private final UserHistoryDao userHistoryDao;
private final TransactionManager transactionManager;

public UserService(final UserDao userDao, final UserHistoryDao userHistoryDao) {
public UserService(final UserDao userDao, final UserHistoryDao userHistoryDao, final DataSource dataSource) {
this.userDao = userDao;
this.userHistoryDao = userHistoryDao;
this.transactionManager = new TransactionManager(dataSource);
}
Comment on lines +16 to 20
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

의존성 주입 패턴에 대해 다시 생각해볼까요?

현재 UserService는 TransactionManager를 직접 생성하고 있습니다(19줄). 하지만 UserDao와 UserHistoryDao는 생성자를 통해 주입받고 있습니다.

이런 차이가 발생한 이유는 무엇일까요? 만약 TransactionManager도 외부에서 주입받는다면 어떤 장점이 있을까요?

힌트: 테스트 작성 시나 다른 트랜잭션 구현체로 교체해야 할 때를 생각해보세요.

🤖 Prompt for AI Agents 
In app/src/main/java/com/techcourse/service/UserService.java around lines 16 to
20, UserService currently constructs a new
TransactionManager(this.transactionManager = new TransactionManager(dataSource))
instead of receiving it like the DAOs; change the constructor to accept a
TransactionManager parameter (inject it) and assign that field, remove direct
new TransactionManager(dataSource) construction, and update all call sites/DI
configuration and tests to provide a TransactionManager (or a mock) so swapping
implementations and unit testing becomes possible.


public User findById(final long id) {
Expand All @@ -26,7 +30,10 @@ public void insert(final User user) {
public void changePassword(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));

transactionManager.executeTransaction((connection) -> {
userDao.update(connection, user);
userHistoryDao.log(connection, new UserHistory(user, createBy));
});
}
}
10 changes: 8 additions & 2 deletions app/src/test/java/com/techcourse/service/MockUserHistoryDao.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
package com.techcourse.service;

import com.techcourse.dao.UserHistoryDao;
import com.techcourse.domain.UserHistory;
import com.interface21.dao.DataAccessException;
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 @@ -15,4 +16,9 @@ public MockUserHistoryDao(final JdbcTemplate jdbcTemplate) {
public void log(final UserHistory userHistory) {
throw new DataAccessException();
}

@Override
public void log(final Connection connection, final UserHistory userHistory) {
throw new DataAccessException();
}
}
16 changes: 7 additions & 9 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
@@ -1,20 +1,18 @@
package com.techcourse.service;

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

import com.interface21.dao.DataAccessException;
import com.interface21.jdbc.core.JdbcTemplate;
import com.techcourse.config.DataSourceConfig;
import com.techcourse.dao.UserDao;
import com.techcourse.dao.UserHistoryDao;
import com.techcourse.domain.User;
import com.techcourse.support.jdbc.init.DatabasePopulatorUtils;
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
19 changes: 19 additions & 0 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 @@ -25,6 +25,10 @@ public void update(final String sql, final Object... parameters) {
execute(sql, PreparedStatement::executeUpdate, parameters);
}

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

public <T> List<T> query(final String sql, final RowMapper<T> rowMapper, final Object... parameters) {
return execute(sql, (pstmt) -> {
try (final ResultSet rs = pstmt.executeQuery()) {
Expand Down Expand Up @@ -60,7 +64,22 @@ private <T> T execute(final String sql, final PreparedStatementExecutor<T> execu
}
}

private <T> T execute(final Connection conn, final String sql, final PreparedStatementExecutor<T> executor,
final Object... parameters) {
try (final PreparedStatement pstmt = conn.prepareStatement(sql)) {
setStatementParameters(pstmt, parameters);
log.debug("query : {}", sql);
return executor.execute(pstmt);
} catch (SQLException e) {
log.error(e.getMessage(), e);
throw new JdbcExecutionException(String.format("\"%s\" 쿼리 실행 중 오류가 발생했습니다.", sql), e);
}
}

private void setStatementParameters(final PreparedStatement pstmt, final Object[] parameters) throws SQLException {
if (parameters == null) {
throw new JdbcExecutionException(String.format("쿼리에 필요한 파라미터가 전달되지 않았습니다."));
}
for (int i = 0; i < parameters.length; i++) {
pstmt.setObject(i + 1, parameters[i]);
}
Expand Down
31 changes: 31 additions & 0 deletions jdbc/src/main/java/com/interface21/transaction/TransactionManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
package com.interface21.transaction;

import com.interface21.dao.DataAccessException;
import java.sql.SQLException;
import javax.sql.DataSource;

public class TransactionManager {

private final DataSource dataSource;

public TransactionManager(final DataSource dataSource) {
this.dataSource = dataSource;
}

public void executeTransaction(final TransactionalAction action) {
try (final var connection = dataSource.getConnection();) {
connection.setAutoCommit(false);

try {
action.execute(connection);
connection.commit();
} catch (Exception e) {
connection.rollback();
throw new DataAccessException("트랜잭션 내부 작업 수행 중 오류가 발생하여 롤백이 수행되었습니다.");
}
Comment on lines +15 to +25
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

메서드의 들여쓰기 깊이를 줄여볼 수 있을까요?

현재 executeTransaction 메서드는 try-with-resources(16줄) 안에 또 다른 try-catch 블록(19-25줄)이 중첩되어 있어 들여쓰기 깊이가 2입니다.

객체지향 생활 체조 규칙 1번은 "한 메서드에 오직 한 단계의 들여쓰기만"을 권장합니다. 이는 메서드의 복잡도를 낮추고 각 메서드가 하나의 책임만 갖도록 하기 위함입니다.

중첩된 트랜잭션 로직을 별도 메서드로 분리하면 어떨까요?

힌트: Connection을 받아서 트랜잭션을 실행하는 부분을 별도 메서드로 추출해보세요.

🤖 Prompt for AI Agents 
jdbc/src/main/java/com/interface21/transaction/TransactionManager.java around
lines 15-25: the executeTransaction method currently nests a try-with-resources
and an inner try-catch, increasing indentation depth; refactor by extracting the
inner transactional logic into a new private helper method that accepts a
Connection (e.g., private void executeWithinTransaction(Connection connection,
TransactionalAction action)), move the action.execute/commit/rollback and
exception-to-DataAccessException mapping into that helper, and have
executeTransaction simply obtain the connection in a try-with-resources and call
the new helper; ensure the helper performs rollback on exception and rethrows
the DataAccessException (preferably wrapping the original exception).


} catch (SQLException e) {
throw new RuntimeException("트랜잭션 로직을 실행하는 중 Connection 오류가 발생했습니다.");
}
}
}
9 changes: 9 additions & 0 deletions jdbc/src/main/java/com/interface21/transaction/TransactionalAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package com.interface21.transaction;

import java.sql.Connection;

@FunctionalInterface
public interface TransactionalAction {

void execute(Connection connection);
}