Skip to content
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

[JDBC 라이브러리 구현하기 - 4단계] 이리내(성채연) 미션 제출합니다. #588

Merged
merged 8 commits into from
Oct 12, 2023
5 changes: 0 additions & 5 deletions app/src/main/java/com/techcourse/dao/UserDao.java
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,6 @@ public void update(final User user) {
jdbcTemplate.update(sql, user.getAccount(), user.getPassword(), user.getEmail(), user.getId());
}

public void update(final Connection connection, final User user) {
final String 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 String sql = "select id, account, password, email from users";
return jdbcTemplate.queryForList(sql, ROW_MAPPER);
Expand Down
15 changes: 0 additions & 15 deletions app/src/main/java/com/techcourse/dao/UserHistoryDao.java
Original file line number Diff line number Diff line change
Expand Up @@ -30,19 +30,4 @@ public void log(final UserHistory userHistory) {
userHistory.getCreateBy()
);
}

public void log(final Connection connection, 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(),
userHistory.getPassword(),
userHistory.getEmail(),
userHistory.getCreatedAt(),
userHistory.getCreateBy()
);
}
}
35 changes: 35 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,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 createBy) {
final User user = findById(id);
user.changePassword(newPassword);
userDao.update(user);
userHistoryDao.log(new UserHistory(user, createBy));
}
}
30 changes: 30 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,30 @@
package com.techcourse.service;

import com.techcourse.domain.User;
import org.springframework.transaction.TransactionExecutor;

public class TxUserService implements UserService {

private final UserService userService;
private final TransactionExecutor transactionExecutor;

public TxUserService(final UserService userService, final TransactionExecutor transactionExecutor) {
this.userService = userService;
this.transactionExecutor = transactionExecutor;
}

@Override
public User findById(final long id) {
return transactionExecutor.execute(() -> userService.findById(id));
}

@Override
public void insert(final User user) {
transactionExecutor.execute(() -> userService.insert(user));
}
Comment on lines +21 to +24

Choose a reason for hiding this comment

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

캬 나머니 코드도 모두 트렌젝션을 적용해주셨군요 굳입니다


@Override
public void changePassword(final long id, final String newPassword, final String createBy) {
transactionExecutor.execute(() -> userService.changePassword(id, newPassword, createBy));
}
}
62 changes: 4 additions & 58 deletions app/src/main/java/com/techcourse/service/UserService.java
Original file line number Diff line number Diff line change
@@ -1,64 +1,10 @@
package com.techcourse.service;

import com.techcourse.config.DataSourceConfig;
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.springframework.dao.DataAccessException;

public class UserService {
public interface UserService {

private final UserDao userDao;
private final UserHistoryDao userHistoryDao;

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

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) {
final DataSource dataSource = DataSourceConfig.getInstance();
final Connection connection = getConnection(dataSource);
try (connection) {
connection.setAutoCommit(false);

final User user = findById(id);
user.changePassword(newPassword);
userDao.update(connection, user);
userHistoryDao.log(connection, new UserHistory(user, createBy));

connection.commit();
} catch (SQLException | DataAccessException e) {
rollback(connection);
throw new DataAccessException(e);
}
}

private Connection getConnection(final DataSource dataSource) {
try {
return dataSource.getConnection();
} catch (SQLException e) {
throw new DataAccessException();
}
}

private void rollback(final Connection connection) {
try {
connection.rollback();
} catch (SQLException e) {
throw new DataAccessException();
}
}
User findById(final long id);
void insert(final User user);
void changePassword(final long id, final String newPassword, final String createBy);
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,4 @@ 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();
}
}
22 changes: 13 additions & 9 deletions app/src/test/java/com/techcourse/service/UserServiceTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@
import org.springframework.dao.DataAccessException;
import org.springframework.jdbc.core.JdbcTemplate;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;
import org.springframework.transaction.TransactionExecutor;

import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.jupiter.api.Assertions.assertThrows;
Expand All @@ -31,31 +31,35 @@ void setUp() {

@Test
void testChangePassword() {
final var userHistoryDao = new UserHistoryDao(jdbcTemplate);
final var userService = new UserService(userDao, userHistoryDao);
final UserHistoryDao userHistoryDao = new UserHistoryDao(jdbcTemplate);
final AppUserService appUserService = new AppUserService(userDao, userHistoryDao);
final TxUserService txUserService = new TxUserService(appUserService, new TransactionExecutor(DataSourceConfig.getInstance()));

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

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

assertThat(actual.getPassword()).isEqualTo(newPassword);
}

@Test
void testTransactionRollback() {
// 트랜잭션 롤백 테스트를 위해 mock으로 교체
final var userHistoryDao = new MockUserHistoryDao(jdbcTemplate);
final var userService = new UserService(userDao, userHistoryDao);
final MockUserHistoryDao userHistoryDao = new MockUserHistoryDao(jdbcTemplate);
// 애플리케이션 서비스
final AppUserService appUserService = new AppUserService(userDao, userHistoryDao);
// 트랜잭션 서비스 추상화
final TxUserService txUserService = new TxUserService(appUserService, new TransactionExecutor(DataSourceConfig.getInstance()));

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

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

assertThat(actual.getPassword()).isNotEqualTo(newPassword);
}
Expand Down
1 change: 1 addition & 0 deletions jdbc/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ dependencies {
implementation "org.apache.commons:commons-lang3:3.13.0"
implementation "ch.qos.logback:logback-classic:1.2.12"

testImplementation "com.h2database:h2:2.2.220"
testImplementation "org.assertj:assertj-core:3.24.2"
testImplementation "org.junit.jupiter:junit-jupiter-api:5.7.2"
testImplementation "org.mockito:mockito-core:5.4.0"
Expand Down
31 changes: 31 additions & 0 deletions jdbc/src/main/java/org/springframework/jdbc/ConnectionHolder.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
package org.springframework.jdbc;

import static java.util.Objects.isNull;

import java.sql.Connection;

public class ConnectionHolder {

private Connection connection;
private boolean transactionActive = false;

public ConnectionHolder(final Connection connection) {
this.connection = connection;
Comment on lines +6 to +13

Choose a reason for hiding this comment

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

코멘트에 남겨주신것처럼 커넥션 홀더를 적용해주셨네요! 감동1

}

public void setTransactionActive(final boolean transactionActive) {
this.transactionActive = transactionActive;
}

public boolean isTransactionActive() {
return transactionActive;
}

public Connection getConnection() {
return connection;
}

public boolean has(final Connection connection) {
return this.connection == connection;
}
}
18 changes: 4 additions & 14 deletions jdbc/src/main/java/org/springframework/jdbc/core/JdbcTemplate.java
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import java.util.List;
import javax.sql.DataSource;
import org.springframework.dao.DataAccessException;
import org.springframework.jdbc.datasource.DataSourceUtils;

public class JdbcTemplate {

Expand All @@ -23,24 +24,13 @@ public int update(final String sql, final Object... args) {

private <T> T execute(final PreparedStatementCallback preparedStatementCallback,
final ExecutionCallback<T> executionCallback) {
try (final Connection connection = dataSource.getConnection()) {
return executeWithConnection(connection, preparedStatementCallback, executionCallback);
} catch (SQLException e) {
throw new DataAccessException(e);
}
}

public int update(final Connection connection, final String sql, final Object... args) {
return executeWithConnection(connection, conn -> prepareStatement(sql, conn, args), PreparedStatement::executeUpdate);
}

private <T> T executeWithConnection(final Connection connection,
final PreparedStatementCallback preparedStatementCallback,
final ExecutionCallback<T> executionCallback) {
final Connection connection = DataSourceUtils.getConnection(dataSource);

Choose a reason for hiding this comment

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

만약 트렌젝션 밖에서 해당 함수가 호출이 되었다면 커넥션을 어떻게 닫을 수 있을까요!? 저도 이부분에 대해서 많이 고민해 보았는데요! 만약 트렌젝션이 실행중이라면 닫지 않고, 실행중이 아니라면 커넥션을 닫는 로직에 대해서 이리내의 의견이 궁금합니다!

Copy link
Author

Choose a reason for hiding this comment

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

저도 코드리뷰를 하다가 힌트를 좀 얻었는데요
실제로 ConnectionHolder라는 클래스가 존재하더라구요
Connection 객체를 한번 더 감싸준 클래스인데요 필드에 transactionActive가 있어서 이걸 토대로 트랜잭션이 실행중인지 아닌지 판단해서 커넥션을 닫아줄지 말지를 결정할 수 있을 것 같아요
결론은 Connection을 ConnectionHolder로 한번 더 감싸주고 필드에 transactionActive를 추가하는 것!! 입니다.
image

try (final PreparedStatement pstmt = preparedStatementCallback.prepareStatement(connection)) {
return executionCallback.execute(pstmt);
} catch (SQLException e) {
throw new DataAccessException(e);
} finally {
DataSourceUtils.releaseConnection(connection, dataSource);
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
package org.springframework.jdbc.datasource;

import static java.util.Objects.isNull;

import org.springframework.dao.DataAccessException;
import org.springframework.jdbc.CannotGetJdbcConnectionException;
import org.springframework.jdbc.ConnectionHolder;
import org.springframework.transaction.support.TransactionSynchronizationManager;

import javax.sql.DataSource;
Expand All @@ -12,23 +16,59 @@ public abstract class DataSourceUtils {

private DataSourceUtils() {}

public static Connection getConnection(DataSource dataSource) throws CannotGetJdbcConnectionException {
Connection connection = TransactionSynchronizationManager.getResource(dataSource);
if (connection != null) {
return connection;
public static Connection getConnection(final DataSource dataSource) throws CannotGetJdbcConnectionException {
final ConnectionHolder connectionHolder = TransactionSynchronizationManager.getResource(dataSource);
if (connectionHolder != null) {
return connectionHolder.getConnection();
}

try {
connection = dataSource.getConnection();
final Connection connection = dataSource.getConnection();
TransactionSynchronizationManager.bindResource(dataSource, connection);
return connection;
} catch (SQLException ex) {
throw new CannotGetJdbcConnectionException("Failed to obtain JDBC Connection", ex);
}
}

public static void releaseConnection(Connection connection, DataSource dataSource) {
public static void startTransaction(final Connection connection, final DataSource dataSource) {
final ConnectionHolder connectionHolder = getConnectionHolder(connection, dataSource);
try{
connectionHolder.setTransactionActive(true);
connection.setAutoCommit(false);
}catch(SQLException e) {
throw new DataAccessException();
}
}

private static ConnectionHolder getConnectionHolder(final Connection connection, final DataSource dataSource) {
final ConnectionHolder connectionHolder = TransactionSynchronizationManager.getResource(dataSource);
if(isNull(connectionHolder)) {
throw new IllegalStateException();
}
if(!connectionHolder.has(connection)) {
throw new IllegalStateException();
}
return connectionHolder;
}

public static void finishTransaction(final Connection connection, final DataSource dataSource) {
final ConnectionHolder connectionHolder = getConnectionHolder(connection, dataSource);
try{
connection.commit();
connectionHolder.setTransactionActive(false);
}catch(SQLException e) {
throw new DataAccessException();
}
}

public static void releaseConnection(final Connection connection, final DataSource dataSource) {
try {
final ConnectionHolder connectionHolder = getConnectionHolder(connection, dataSource);
if(connectionHolder.isTransactionActive()) {
return;
}
TransactionSynchronizationManager.unbindResource(dataSource);

Choose a reason for hiding this comment

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

connection.close(); 에서 예외가 발생하더라도
TransactionSynchronizationManager.unbindResource(dataSource);가 일어날 수 있도록 하는것이 좋을까요? 예외가 발생하면 일어나지 않도록 하는것이 좋을까요? 저도 고민이 되었었는데 이리내와 얘기를 나눠보고싶네요!

Copy link
Author

@hectick hectick Oct 10, 2023

Choose a reason for hiding this comment

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

그러게요 생각해보지 못했는데 고민할만한 내용인 것 같습니다
생각해보니 저라면 unbind 해줄 것 같아요
쓰레드가 재사용되는 상황이라면, 이전 Connection이 비워지지 않고 남아있으면 위험하지 않을까요??

Choose a reason for hiding this comment

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

저또한 예외가 발생하더라도 비워주는것이 현재코드로서는 안전하다는 생각이 드네요
저같은 경우는 final로 처리를 하였는데 여기 순서를 변경하는 방법으로 구현하셨군요~!이런 방법도 있네요!?

connection.close();
} catch (SQLException ex) {
throw new CannotGetJdbcConnectionException("Failed to close JDBC Connection");
Expand Down
Loading
Loading