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단계] 에단(김석호) 미션 제출합니다. #556

Merged
merged 16 commits into from
Oct 13, 2023
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
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
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,5 @@
- [x] update()
- [x] JdbcTemplate 으로 중복 제거
- [x] 트랜잭션 경계 설정하기
- [x] Transaction synchronization 적용하기
- [x] 트랜잭션 서비스 추상화하기
7 changes: 0 additions & 7 deletions app/src/main/java/com/techcourse/dao/UserDao.java
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
import org.springframework.jdbc.core.JdbcTemplate;
import org.springframework.jdbc.core.RowMapper;

import java.sql.Connection;
import java.util.List;
import java.util.Optional;

Expand Down Expand Up @@ -33,12 +32,6 @@ public void update(final User user) {
jdbcTemplate.execute(sql, user.getAccount(), user.getPassword(), user.getEmail(), user.getId());
}

public void update(final Connection connection, final User user) {
final var sql = "update users set (account, password, email) = (?, ?, ?) where id = ?";

jdbcTemplate.executeWithConnection(connection, sql, user.getAccount(), user.getPassword(), user.getEmail(), user.getId());
}

List<User> findAll() {
final var sql = "select id, account, password, email from users";

Expand Down
16 changes: 0 additions & 16 deletions app/src/main/java/com/techcourse/dao/UserHistoryDao.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@
import com.techcourse.domain.UserHistory;
import org.springframework.jdbc.core.JdbcTemplate;

import java.sql.Connection;

public class UserHistoryDao {

private final JdbcTemplate jdbcTemplate;
Expand All @@ -25,18 +23,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.executeWithConnection(connection,
sql,
userHistory.getUserId(),
userHistory.getAccount(),
userHistory.getPassword(),
userHistory.getEmail(),
userHistory.getCreatedAt(),
userHistory.getCreateBy()
);
}
}
39 changes: 39 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,39 @@
package com.techcourse.service;

import com.techcourse.dao.UserDao;
import com.techcourse.dao.UserHistoryDao;
import com.techcourse.domain.User;
import com.techcourse.domain.UserHistory;

import java.util.NoSuchElementException;

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)
.orElseThrow(() -> new NoSuchElementException("id 값으로 해당하는 User 를 찾을 수 없습니다."));
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
.orElseThrow(() -> new NoSuchElementException("id 값으로 해당하는 User 를 찾을 수 없습니다."));
.orElseThrow(() -> new NoSuchElementException(String.format("id 값으로 해당하는 User 를 찾을 수 없습니다. 입력값: %s", id)));
  1. 클라이언트가 어떤 입력값을 보냈을 때 찾을 수 없었는지 추적할 수 있도록 메세지에 입력값을 남기세요.
  2. NoSuchElementException 가 적절한가요? Element라는 용어는 자바 컬렉션에서 사용되는걸로 보이는데, 적절한 예외일까요?

Copy link
Author

Choose a reason for hiding this comment

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

  1. 넵 디버깅하기가 쉬워지겠네요.
  2. 오.. 그 점은 고려하지 못했습니다. 앞으로 예외의 패키지도 의식적으로 보는 연습이 필요할것 같습니다

}

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

@Override
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));
}
}
79 changes: 4 additions & 75 deletions app/src/main/java/com/techcourse/service/UserService.java
Original file line number Diff line number Diff line change
@@ -1,83 +1,12 @@
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 org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.dao.DataAccessException;

import javax.sql.DataSource;
import java.sql.Connection;
import java.sql.SQLException;
import java.util.NoSuchElementException;
public interface UserService {

public class UserService {
User findById(final long id);

private static final Logger log = LoggerFactory.getLogger(UserService.class);
void insert(final User user);

private final UserDao userDao;
private final UserHistoryDao userHistoryDao;

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

User findById(final long id) {
return userDao.findById(id)
.orElseThrow(() -> new NoSuchElementException("id 값으로 해당하는 User 를 찾을 수 없습니다."));
}

public void insert(final User user) {
userDao.insert(user);
}

void changePassword(final long id, final String newPassword, final String createBy) {
final var user = findById(id);
user.changePassword(newPassword);

Connection connection = null;
try {
final DataSource dataSource = DataSourceConfig.getInstance();
connection = dataSource.getConnection();
connection.setAutoCommit(false);

userDao.update(connection, user);
userHistoryDao.log(connection, new UserHistory(user, createBy));

connection.commit();
} catch (final SQLException | DataAccessException e) {
rollback(connection);
log.error("SQLException occurred");
throw new DataAccessException(e);
} finally {
release(connection);
}
}

private void rollback(final Connection connection) {
if (connection != null) {
try {
connection.rollback();
} catch (final SQLException ex) {
log.error("rollback callback");
throw new DataAccessException(ex);
}
}
}

private void release(final Connection connection) {
if (connection != null) {
try {
connection.setAutoCommit(true);
connection.close();
} catch (final SQLException e) {
log.error("Cannot close Connection");
throw new DataAccessException(e);
}
}
}
void changePassword(final long id, final String newPassword, final String createBy);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package com.techcourse.service.transaction;
Copy link
Contributor

Choose a reason for hiding this comment

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

패키지를 프레임워크로 옮기는게 낫지 않을까요?


@FunctionalInterface
public interface TransactionLogicExecutor {

void run();
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
package com.techcourse.service.transaction;
Copy link
Contributor

Choose a reason for hiding this comment

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

패키지를 프레임워크로 옮기는게 낫지 않을까요?


import com.techcourse.config.DataSourceConfig;
import org.springframework.dao.DataAccessException;
import org.springframework.jdbc.datasource.DataSourceUtils;
import org.springframework.transaction.TransactionException;
import org.springframework.transaction.support.TransactionSynchronizationManager;

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

public class TransactionTemplate {

public void executeWithTransaction(final TransactionLogicExecutor transactionLogicExecutor) {
final DataSource dataSource = DataSourceConfig.getInstance();
Copy link
Contributor

Choose a reason for hiding this comment

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

DataSource는 주입 받도록 만드는게 좋지 않을까요? Connection과는 다른 결이에요.
싱글톤 패턴을 사용할 때 문제점을 찾아봅시다.

Copy link
Author

Choose a reason for hiding this comment

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

이 부분에 대해서 고민을 많이 해봤는데요. 가장 간단하게는 연관관계를 가지고 있어서 테스트와 같이 DataSource를 변경할 수도 있는 상황에서 변경하기가 어렵습니다. 나아가서 일반적인 DataSource 구현체로 HikariCP를 사용하고 있지만, 더 좋은게 나와서 바꾸려면 어려울 수 있습니다. DataSource를 TransactionTemplate을 만들 때 주입받는 방식으로 변경했습니다.

final Connection connection = DataSourceUtils.getConnection(dataSource);
try {
connection.setAutoCommit(false);

transactionLogicExecutor.run();

connection.commit();
} catch (final SQLException | DataAccessException e) {
rollback(connection);
throw new DataAccessException(e);
} finally {
release(dataSource, connection);
}
}

private void rollback(final Connection connection) {
if (connection != null) {
try {
Copy link
Contributor

Choose a reason for hiding this comment

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

2뎁스를 1뎁스로 줄이시기 바랍니다.

connection.rollback();
} catch (final SQLException ex) {
throw new DataAccessException(ex);
}
}
}

private static void release(final DataSource dataSource, final Connection connection) {
try {
connection.setAutoCommit(true);
TransactionSynchronizationManager.unbindResource(dataSource);
DataSourceUtils.releaseConnection(connection, dataSource);
} catch (final SQLException e) {
throw new TransactionException("Failed to change auto commit to true");
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
package com.techcourse.service.transaction;

import com.techcourse.domain.User;
import com.techcourse.service.UserService;

public class TxUserService implements UserService {

private final UserService userService;
private final TransactionTemplate transactionTemplate;

public TxUserService(final UserService userService, final TransactionTemplate transactionTemplate) {
this.userService = userService;
this.transactionTemplate = transactionTemplate;
}

@Override
public User findById(final long id) {
return userService.findById(id);
Copy link
Contributor

Choose a reason for hiding this comment

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

findById는 왜 트랜잭션을 적용하지 않나요?

Copy link
Author

Choose a reason for hiding this comment

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

수업에서 들은대로 readonly로 트랜잭션 걸어주도록 리팩토링했습니다

}

@Override
public void insert(final User user) {
transactionTemplate.executeWithTransaction(() -> userService.insert(user));
}

@Override
public void changePassword(final long id, final String newPassword, final String createBy) {
transactionTemplate.executeWithTransaction(() -> userService.changePassword(id, newPassword, createBy));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,14 @@
import org.springframework.dao.DataAccessException;
import org.springframework.jdbc.core.JdbcTemplate;

import java.sql.Connection;

public class MockUserHistoryDao extends UserHistoryDao {

MockUserHistoryDao(final JdbcTemplate jdbcTemplate) {
super(jdbcTemplate);
}

@Override
public void log(final Connection connection, final UserHistory userHistory) {
public void log(final UserHistory userHistory) {
throw new DataAccessException();
}
}
9 changes: 7 additions & 2 deletions app/src/test/java/com/techcourse/service/UserServiceTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
import com.techcourse.dao.UserDao;
import com.techcourse.dao.UserHistoryDao;
import com.techcourse.domain.User;
import com.techcourse.service.transaction.TransactionTemplate;
import com.techcourse.service.transaction.TxUserService;
import com.techcourse.support.jdbc.init.DatabasePopulatorUtils;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
Expand Down Expand Up @@ -33,7 +35,7 @@ void setUp() {

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

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

final var newPassword = "newPassword";
final var createBy = "gugu";
Expand Down
23 changes: 4 additions & 19 deletions jdbc/src/main/java/org/springframework/jdbc/core/JdbcTemplate.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.dao.DataAccessException;
import org.springframework.jdbc.datasource.DataSourceUtils;

import javax.sql.DataSource;
import java.sql.Connection;
Expand All @@ -24,8 +25,8 @@ public JdbcTemplate(final DataSource dataSource) {
}

public void execute(final String sql, final Object... params) {
final Connection conn = DataSourceUtils.getConnection(dataSource);
try (
final Connection conn = dataSource.getConnection();
final PreparedStatement pstmt = conn.prepareStatement(sql)
) {
log.debug("query : {}", sql);
Expand All @@ -34,27 +35,13 @@ public void execute(final String sql, final Object... params) {

pstmt.executeUpdate();
} catch (final SQLException e) {
log.error(e.getMessage(), e);
throw new DataAccessException(e);
}
}

public void executeWithConnection(final Connection conn, final String sql, final Object... params) {
try (final PreparedStatement pstmt = conn.prepareStatement(sql)) {
log.debug("query : {}", sql);

setCondition(params, pstmt);

pstmt.executeUpdate();
} catch (final SQLException e) {
log.error(e.getMessage(), e);
throw new DataAccessException(e);
}
}

public <T> List<T> query(final String sql, final RowMapper<T> rowMapper) {
final Connection conn = DataSourceUtils.getConnection(dataSource);
try (
final Connection conn = dataSource.getConnection();
final PreparedStatement pstmt = conn.prepareStatement(sql);
final ResultSet rs = pstmt.executeQuery()
) {
Expand All @@ -69,14 +56,13 @@ public <T> List<T> query(final String sql, final RowMapper<T> rowMapper) {

return results;
} catch (final SQLException e) {
log.error(e.getMessage(), e);
throw new DataAccessException(e);
}
}

public <T> Optional<T> queryForObject(final String sql, final RowMapper<T> rowMapper, final Object... params) {
final Connection conn = DataSourceUtils.getConnection(dataSource);
try (
final Connection conn = dataSource.getConnection();
final PreparedStatement pstmt = getPreparedStatement(conn, sql, params);
final ResultSet rs = pstmt.executeQuery()
) {
Expand All @@ -89,7 +75,6 @@ public <T> Optional<T> queryForObject(final String sql, final RowMapper<T> rowMa

return Optional.empty();
} catch (final SQLException e) {
log.error(e.getMessage(), e);
throw new DataAccessException(e);
}
}
Expand Down
Loading
Loading