Skip to content

[3단계 - Transaction 적용하기] 우가(민보욱) 미션 제출합니다. #1159

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

Merged
merged 4 commits into from
Oct 22, 2025
Merged

[3단계 - Transaction 적용하기] 우가(민보욱) 미션 제출합니다. #1159

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4c51ae7
refactor: UserHistoryDao jdbcTemplate 사용
bowook Oct 16, 2025
2e5cf3c
feat: Transaction 적용
bowook Oct 17, 2025
3a90260
test: 중복된 검증 제거
bowook Oct 17, 2025
a8de675
test: QUERY 수정
bowook 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
13 changes: 10 additions & 3 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 @@ -4,6 +4,7 @@
import com.interface21.jdbc.core.PreparedStatementSetter;
import com.interface21.jdbc.core.RowMapper;
import com.techcourse.domain.User;
import java.sql.Connection;
import java.util.List;
import java.util.Optional;

Expand All @@ -23,12 +24,18 @@ public UserDao(final JdbcTemplate jdbcTemplate) {

public void insert(final User user) {
final String sql = "insert into users (account, password, email) values (?, ?, ?)";
jdbcTemplate.update(sql, user.getAccount(), user.getPassword(), user.getEmail());
PreparedStatementSetter pss = ps -> {
ps.setString(1, user.getAccount());
ps.setObject(2, user.getPassword());
ps.setString(3, user.getEmail());
};

jdbcTemplate.update(sql, pss);
}

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

public List<User> findAll() {
Expand Down
61 changes: 16 additions & 45 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,62 +1,33 @@
package com.techcourse.dao;

import com.techcourse.domain.UserHistory;
import com.interface21.jdbc.core.JdbcTemplate;
import com.interface21.jdbc.core.PreparedStatementSetter;
import com.techcourse.domain.UserHistory;
import java.sql.Connection;
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;

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

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

public void log(final UserHistory userHistory) {
public void log(final UserHistory userHistory, final Connection connection) {
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) {}
}
PreparedStatementSetter pss = ps -> {
ps.setLong(1, userHistory.getUserId());
ps.setString(2, userHistory.getAccount());
ps.setString(3, userHistory.getPassword());
ps.setString(4, userHistory.getEmail());
ps.setObject(5, userHistory.getCreatedAt());
ps.setString(6, userHistory.getCreateBy());
};
Comment on lines +22 to +29
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

객체지향 생활 체조 원칙 9번 위반: Tell, Don't Ask

현재 코드는 UserHistory 객체에게 데이터를 요청(Ask)하여 직접 처리하고 있습니다. 이는 캡슐화를 약화시키고 UserHistory를 단순한 데이터 홀더로 만듭니다.

🤔 생각해볼 질문들:

  • UserHistory 객체 스스로가 PreparedStatement에 자신의 데이터를 설정하도록 할 수는 없을까요?
  • 만약 UserHistory가 "PreparedStatement에 내 정보를 바인딩해줘"라고 요청받는다면 어떻게 설계할 수 있을까요?
  • 데이터를 꺼내서(get) 사용하는 것과, 객체에게 행동을 요청하는(tell) 것의 차이는 무엇일까요?

💡 힌트:
"Tell, Don't Ask" 원칙에 따르면, 객체에게 데이터를 달라고 하지 말고, 객체에게 무언가를 하라고 말해야 합니다. UserHistory가 스스로 PreparedStatement 바인딩 책임을 가질 수 있는지 고민해보세요.

📖 이 원칙이 중요한 이유:

  • 진정한 캡슐화: 객체가 자신의 데이터를 스스로 관리
  • 객체의 자율성: 데이터와 행동이 함께 있어야 함
  • 변경에 강한 코드: UserHistory의 내부 구조가 변경되어도 이를 사용하는 코드는 영향받지 않음

As per coding guidelines


jdbcTemplate.update(connection, sql, pss);
}
}
37 changes: 32 additions & 5 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,16 +4,25 @@
import com.techcourse.dao.UserHistoryDao;
import com.techcourse.domain.User;
import com.techcourse.domain.UserHistory;
import java.sql.Connection;
import java.sql.SQLException;
import java.util.NoSuchElementException;
import javax.sql.DataSource;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

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(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 @@ -26,9 +35,27 @@ 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));
try (Connection connection = dataSource.getConnection()) {
try {
connection.setAutoCommit(false);
final var user = findById(id);
user.changePassword(newPassword);
userDao.update(user, connection);
userHistoryDao.log(new UserHistory(user, createBy), connection);

connection.commit();
} catch (Exception e) {
try {
connection.rollback();
log.warn("트랜잭션 롤백 성공", e);
} catch (SQLException ex) {
log.error("롤백 실패", ex);
}
throw new RuntimeException("비밀번호 변경 중 오류가 발생하여 롤백했습니다.", e);
}
} catch (SQLException e) {
log.error("데이터베이스 연결 실패", e);
throw new RuntimeException("데이터베이스 연결에 실패했습니다.", e);
}
}
}
11 changes: 7 additions & 4 deletions app/src/test/java/com/techcourse/dao/UserDaoTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
import com.techcourse.config.DataSourceConfig;
import com.techcourse.domain.User;
import com.techcourse.support.jdbc.init.DatabasePopulatorUtils;
import java.sql.Connection;
import java.sql.SQLException;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

Expand All @@ -13,13 +15,14 @@ class UserDaoTest {

private UserDao userDao;
private JdbcTemplate jdbcTemplate;
private Connection connection;
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

Connection 리소스 누수 문제

setup() 메서드에서 dataSource.getConnection()으로 Connection을 획득하고 있지만, 이를 닫는 코드가 보이지 않습니다. Connection은 데이터베이스와의 연결을 나타내는 중요한 리소스로, 사용 후 반드시 해제해야 합니다.

몇 가지 질문을 드려보겠습니다:

  • 테스트가 완료된 후 Connection을 어떻게 정리하고 계신가요?
  • 여러 테스트가 순차적으로 실행될 때 어떤 일이 발생할까요?
  • JDBC 리소스 관리에 대해 어떤 패턴들이 있는지 알아보셨나요?

💡 힌트: JUnit의 라이프사이클 애노테이션(@AfterEach 등)과 try-with-resources 구문을 고려해보세요.

Also applies to: 24-24

🤖 Prompt for AI Agents 
In app/src/test/java/com/techcourse/dao/UserDaoTest.java around lines 18 and 24,
the test acquires a Connection via dataSource.getConnection() but never closes
it, causing resource leaks; add proper cleanup by either (a) wrapping per-test
connection usage in try-with-resources so the Connection is closed
automatically, or (b) add an @AfterEach teardown method that checks if
connection is non-null and not closed and then calls connection.close(); ensure
any exceptions are handled or propagated as test failures and reset the
connection field to null after close.


@BeforeEach
void setup() {
void setup() throws SQLException {
var dataSource = DataSourceConfig.getInstance();
this.jdbcTemplate = new JdbcTemplate(dataSource);

jdbcTemplate.update("DROP TABLE users IF EXISTS");
this.connection = dataSource.getConnection();
jdbcTemplate.update(connection, "DROP TABLE users IF EXISTS");
DatabasePopulatorUtils.execute(dataSource);
userDao = new UserDao(jdbcTemplate);

Expand Down Expand Up @@ -72,7 +75,7 @@ void update() {
final var user = userDao.findByAccount("gugu").orElseThrow();
user.changePassword(newPassword);

userDao.update(user);
userDao.update(user, connection);

assertThat(userDao.findByAccount("gugu"))
.isPresent()
Expand Down
9 changes: 5 additions & 4 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 @@ -12,7 +13,7 @@ public MockUserHistoryDao(final JdbcTemplate jdbcTemplate) {
}

@Override
public void log(final UserHistory userHistory) {
throw new DataAccessException();
public void log(final UserHistory userHistory, final Connection connection) {
throw new DataAccessException("정상적인 롤백을 확인하기 위한 의도한 예외 발생");
}
}
41 changes: 26 additions & 15 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,29 +1,35 @@
package com.techcourse.service;

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 javax.sql.DataSource;
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.assertj.core.api.SoftAssertions.assertSoftly;
import static org.junit.jupiter.api.Assertions.assertThrows;

@Disabled
class UserServiceTest {

private DataSource dataSource;
private JdbcTemplate jdbcTemplate;
private UserDao userDao;
private UserHistoryDao userHistoryDao;
private UserService userService;

@BeforeEach
void setUp() {
this.jdbcTemplate = new JdbcTemplate(DataSourceConfig.getInstance());
this.dataSource = DataSourceConfig.getInstance();
this.jdbcTemplate = new JdbcTemplate(dataSource);
this.userDao = new UserDao(jdbcTemplate);
this.userHistoryDao = new UserHistoryDao(jdbcTemplate);

this.userService = new UserService(userDao, userHistoryDao, dataSource);

DatabasePopulatorUtils.execute(DataSourceConfig.getInstance());
final var user = new User("gugu", "password", "hkkang@woowahan.com");
Expand All @@ -32,32 +38,37 @@ void setUp() {

@Test
void testChangePassword() {
final var userHistoryDao = new UserHistoryDao(jdbcTemplate);
final var userService = new UserService(userDao, userHistoryDao);

// given
final var newPassword = "qqqqq";
final var createBy = "gugu";
userService.changePassword(1L, newPassword, createBy);

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

// then
assertThat(actual.getPassword()).isEqualTo(newPassword);
}
Comment on lines 49 to 50
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

비밀번호 getter 의존은 보안·설계 상 취약합니다

  • 테스트가 비밀번호를 직접 읽어 비교합니다. 비밀번호 노출은 캡슐화를 약화시키고, 실수로 로깅/출력을 유도할 수 있습니다.
  • 객체지향 생활 체조 규칙 9(게터/세터 금지), 규칙 3(원시값 포장) 관점에서도 도메인 행위로 검증하는 편이 좋습니다.

행위 기반 검증(예: 비밀번호 일치 여부를 도메인/서비스가 판단)으로 전환하고, Password 같은 값 객체 도입을 고민해보셨나요?

Also applies to: 67-69

🤖 Prompt for AI Agents 
In app/src/test/java/com/techcourse/service/UserServiceTest.java around lines
49-50 (and similarly lines 67-69), the test reads the user's password via a
getter which exposes sensitive state; change the assertions to behavior-based
verification instead: call the service/domain method that checks credentials
(e.g., authenticate(email, rawPassword) or user.matchesPassword(rawPassword)) or
introduce a Password value object with a match/verify method and assert that it
returns true for the expected password and false for old/invalid ones; remove
direct getPassword() usages and update tests accordingly to use the new
behavior-based API.


@Test
void testTransactionRollback() {
// 트랜잭션 롤백 테스트를 위해 mock으로 교체
final var userHistoryDao = new MockUserHistoryDao(jdbcTemplate);
final var userService = new UserService(userDao, userHistoryDao);
// given
final var mockUserHistoryDao = new MockUserHistoryDao(jdbcTemplate);
final var userServiceWithMock = new UserService(userDao, mockUserHistoryDao, dataSource);

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

assertThrows(RuntimeException.class,
() -> userServiceWithMock.changePassword(1L, newPassword, createBy));

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

assertThat(actual.getPassword()).isNotEqualTo(newPassword);
// then
assertSoftly(softAssertions -> {
softAssertions.assertThat(actual.getPassword()).isNotEqualTo(newPassword);
softAssertions.assertThat(actual.getPassword()).isNotEqualTo(newPassword);
});
}
}
21 changes: 17 additions & 4 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 @@ -32,8 +32,12 @@ public int update(final String sql, final PreparedStatementSetter pss) {
/**
* INSERT, UPDATE, DELETE 쿼리를 위한 메서드 (자동 파라미터 설정)
*/
public int update(final String sql, final Object... params) {
return executeUpdate(sql, createPss(params));
public int update(final Connection connection, final String sql, final Object... params) {
return executeUpdate(connection, sql, createPss(params));
}

public int update(final Connection connection, final String sql, final PreparedStatementSetter pss) {
return executeUpdate(connection, sql, pss);
}

/**
Expand Down Expand Up @@ -75,8 +79,17 @@ public <T> Optional<T> queryForObject(
}

private int executeUpdate(final String sql, final PreparedStatementSetter pss) {
try (final Connection conn = getConnection();
final PreparedStatement ps = createPreparedStatement(conn, sql, pss)) {
try (final Connection connection = dataSource.getConnection();
final PreparedStatement ps = createPreparedStatement(connection, sql, pss)) {
return ps.executeUpdate();
} catch (final SQLException e) {
log.error("SQL 실행 실패: ", e);
throw new DataAccessException("SQL 실행 실패", e);
}
}

private int executeUpdate(final Connection connection, final String sql, final PreparedStatementSetter pss) {
try (final PreparedStatement ps = createPreparedStatement(connection, sql, pss)) {
return ps.executeUpdate();
} catch (final SQLException e) {
log.error("SQL 실행 실패: ", e);
Expand Down