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단계] 하마드(이건회) 미션 제출합니다. #538

Merged
merged 6 commits into from
Oct 12, 2023
48 changes: 35 additions & 13 deletions app/src/main/java/com/techcourse/dao/UserDao.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,11 @@
import com.techcourse.domain.User;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.dao.DataAccessException;
import org.springframework.jdbc.core.JdbcTemplate;
import org.springframework.jdbc.core.RowMapper;

import java.sql.Connection;
import java.sql.SQLException;
import java.util.List;
import java.util.NoSuchElementException;

Expand All @@ -20,33 +21,54 @@ public UserDao(final JdbcTemplate jdbcTemplate) {
this.jdbcTemplate = jdbcTemplate;
}

public void insert(Connection con, final User user) {
public void insert(final User user) {
final var sql = "insert into users (account, password, email) values (?, ?, ?)";
jdbcTemplate.update(con,sql, user.getAccount(), user.getPassword(), user.getEmail());
try {
jdbcTemplate.update(sql, user.getAccount(), user.getPassword(), user.getEmail());
} catch (SQLException e) {
throw new DataAccessException(e);
}
}

public void update(Connection con,final User user) {
public void update(final User user) {
String sql = "UPDATE users SET account = ?, password = ?, email = ? WHERE id = ?";
jdbcTemplate.update(con,sql, user.getAccount(), user.getPassword(), user.getEmail(), user.getId());
try {
jdbcTemplate.update(sql, user.getAccount(), user.getPassword(), user.getEmail(), user.getId());
} catch (SQLException e) {
throw new DataAccessException(e) {

Choose a reason for hiding this comment

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

실제 JdbcTemplate에서 SQLException을 잡은 후 자기네 DataAccessException으로 바꾸어 던지는 이유가 무엇일까요?

마드짱의 insert sql에 아무 글자나 넣어서 오타를 낸 후 실행해보면 JdbcSQLSyntaxErrorException이 나오는데,
image
예외의 패키지를 보면 org.h2.jdbc로 h2 라이브러리에서 제공하는 예외임을 알 수가 있죠.

이건 우리가 h2를 사용하기 때문에 h2 예외가 발생한 것이고, mysql을 쓰면 mysql 예외가, mariaDB를 쓰면 mariaDB 예외가 발생하겠죠.

지금의 UserDao는 catch문에서 SQLException 전체를 잡고 있는데.

    public void update(final User user) {
        String sql = "UPDATE users SET account = ?, password = ?, email = ? WHERE id = ?";
        try {
            jdbcTemplate.update(sql, user.getAccount(), user.getPassword(), user.getEmail(), user.getId());
        } catch (SQLException e) {
            throw new DataAccessException(e) {
            };
        }
    }

만약 SyntaxError 하나만 잡아서 특별한 처리를 하는 상황이 왔다고 가정해 보아요.

    public void update(final User user) {
        String sql = "UPDATE users SET account = ?, password = ?, email = ? WHERE id = ?";
        try {
            jdbcTemplate.update(sql, user.getAccount(), user.getPassword(), user.getEmail(), user.getId());
        } catch (JdbcSQLSyntaxErrorException e) {
            throw new DataAccessException(e) {
            };
        }
    }

h2를 쓰고 있는 지금은 syntax에러가 발생했을 때 catch문에서 딱 잡아주지만
만약 데이터베이스를 MySQL로 바꾸었다고 하면? 안 잡히겠죠! (두둥)
왜냐면 위 catch문에서 잡는 SyntaxError는 h2 예외이기 때문에
mysql에서 나온 SyntaxError는 당연히 안 잡겠죠?

이를 방지하려면 SQLException을 잡는 모든 try-catch문을 찾아가 h2 예외를 잡는 대신 mysql 예외를 잡도록 바꿔야겠네요.

public void update(final User user) {
        String sql = "UPDATE users SET account = ?, password = ?, email = ? WHERE id = ?";
        try {
            jdbcTemplate.update(sql, user.getAccount(), user.getPassword(), user.getEmail(), user.getId());
        } catch (org.mysql.jdbc.JdbcSQLSyntaxErrorException e) {
            throw new DataAccessException(e) {
            };
        }
    }

그런데 운영 환경에서는 mysql을 쓰지만 테스트 환경에서는 h2를 쓰겠다! 라고 하면
또다시 모든 try-catch문을 찾아가 h2 예외를 잡는 catch문을 밑에 더 붙여줘야 할 거에요.

public void update(final User user) {
        String sql = "UPDATE users SET account = ?, password = ?, email = ? WHERE id = ?";
        try {
            jdbcTemplate.update(sql, user.getAccount(), user.getPassword(), user.getEmail(), user.getId());
        } catch (org.mysql.jdbc.JdbcSQLSyntaxErrorException e) {
            throw new DataAccessException(e) {
            };
        } catch (org.h2.jdbc.JdbcSQLSyntaxErrorException e) {
            throw new DataAccessException(e) {
            };
        }
    }

'jdbcTemplate에서는 예외만 발생시키고 이걸 어떻게 처리할지는 dao에서 개발자가 결정하는게 옳다' 라고 하마드는 말씀해 주셨지만,
현재 사용중인 DBMS와 미래에 사용하게 될 DBMS의 종류까지 신경써가며 코드를 작성해야 하는 것이
과연 개발자에게 건강한 결정권을 주었다고 볼 수 있는지 모르겠어요.
하마드의 생각이 궁금하군요!

Copy link
Author

Choose a reason for hiding this comment

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

오 여우의 상세한 리뷰를 보고 나니 제가 착각한 부분이 있었네요...!
JdbcTemplate이 SQL Exception을 던진다고 착각했는데, Checked 예외를 DataAccessException으로 바꿔 던지는 것을 간과하고 있었군요.
수정하도록 할게요 좋은 리뷰 고마워요ㅎㅎ

};
}

}

public List<User> findAll(Connection con) {
public List<User> findAll() {
String sql = "SELECT id, account, password, email FROM users";
return jdbcTemplate.query(con,sql, getUserRowMapper());
try {
return jdbcTemplate.query(sql, getUserRowMapper());
} catch (SQLException e) {
throw new DataAccessException(e);
}
}


public User findById(Connection con,final Long id) {
public User findById(final Long id) {
final var sql = "select id, account, password, email from users where id = ?";
return jdbcTemplate.queryForObject(con,sql, getUserRowMapper(), id)
.orElseThrow(() -> new NoSuchElementException("결과가 존재하지 않습니다"));
try {
return jdbcTemplate.queryForObject(sql, getUserRowMapper(), id)
.orElseThrow(() -> new NoSuchElementException("결과가 존재하지 않습니다"));
} catch (SQLException e) {
throw new DataAccessException(e);
}
}

public User findByAccount(Connection con,final String account) {
public User findByAccount(final String account) {
String sql = "SELECT id, account, password, email FROM users WHERE account = ?";
return jdbcTemplate.queryForObject(con,sql, getUserRowMapper(), account)
.orElseThrow(() -> new NoSuchElementException("결과가 존재하지 않습니다"));
try {
return jdbcTemplate.queryForObject(sql, getUserRowMapper(), account)
.orElseThrow(() -> new NoSuchElementException("결과가 존재하지 않습니다"));
} catch (SQLException e) {
throw new DataAccessException(e);
}
}

private RowMapper<User> getUserRowMapper() {
Expand Down
25 changes: 15 additions & 10 deletions app/src/main/java/com/techcourse/dao/UserHistoryDao.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,10 @@
import com.techcourse.domain.UserHistory;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.dao.DataAccessException;
import org.springframework.jdbc.core.JdbcTemplate;

import java.sql.Connection;
import java.sql.SQLException;

public class UserHistoryDao {

Expand All @@ -17,15 +18,19 @@ public UserHistoryDao(final JdbcTemplate jdbcTemplate) {
this.jdbcTemplate = jdbcTemplate;
}

public void log(Connection con, 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(con, sql,
userHistory.getUserId(),
userHistory.getAccount(),
userHistory.getPassword(),
userHistory.getEmail(),
userHistory.getCreatedAt(),
userHistory.getCreateBy()
);
try {
jdbcTemplate.update(sql,
userHistory.getUserId(),
userHistory.getAccount(),
userHistory.getPassword(),
userHistory.getEmail(),
userHistory.getCreatedAt(),
userHistory.getCreateBy()
);
} catch (SQLException e) {
throw new DataAccessException(e);
}
}
}
79 changes: 79 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,79 @@
package com.techcourse.service;

import com.techcourse.domain.User;
import org.springframework.dao.DataAccessException;
import org.springframework.jdbc.datasource.DataSourceUtils;
import org.springframework.transaction.support.TransactionSynchronizationManager;

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

public class TxUserService implements UserService {

private final UserService userService;
private final DataSource dataSource;

public TxUserService(UserService userService, DataSource dataSource) {
this.userService = userService;
this.dataSource = dataSource;
}

public User findById(final long id) {
Connection connection = DataSourceUtils.getConnection(dataSource);
TransactionSynchronizationManager.bindResource(dataSource, connection);
try {
connection.setAutoCommit(false);
User user = userService.findById(id);
connection.commit();
return user;
} catch (Exception e) {
rollback(connection);
throw new DataAccessException(e);
} finally {
DataSourceUtils.releaseConnection(connection, dataSource);
TransactionSynchronizationManager.unbindResource(dataSource);
}
}


public void insert(final User user) {
Connection connection = DataSourceUtils.getConnection(dataSource);
TransactionSynchronizationManager.bindResource(dataSource, connection);
try {
connection.setAutoCommit(false);
userService.insert(user);
connection.commit();
} catch (Exception e) {
rollback(connection);
throw new DataAccessException(e);
} finally {
DataSourceUtils.releaseConnection(connection, dataSource);
TransactionSynchronizationManager.unbindResource(dataSource);
}
}

public void changePassword(final long id, final String newPassword, final String createBy) {

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.

순수 비즈니스 로직만 가진 서비스 클래스 (AppUserService
)와, 트랜잭션 처리 관심사를 담당하면서 위 AppUserService를 인스턴스로 받아 비즈니스 로직을 호출하는 클래스로 구분해서 둘의 관심사를 끊어내기 위함이었습니다.(요구사항을 지키기 위해서요)

그런데 테스트 코드에서

AppUserService appUserService = new AppUserService(userDao, userHistoryDao);
final var userService = new TxUserService(appUserService,dataSource);

다음과 같이 직접 구현체를 주입시켜야 하는 어색함이 있네요...

Choose a reason for hiding this comment

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

오홍 이해했습니다!
테스트 코드에서는 테스트하려는 구현체를 직접 넣는 경우가 흔하기 때문에 저는 어색하다고 느껴지지 않았어요 😊

Connection connection = DataSourceUtils.getConnection(dataSource);
TransactionSynchronizationManager.bindResource(dataSource, connection);
try {
connection.setAutoCommit(false);
userService.changePassword(id, newPassword, createBy);
connection.commit();
} catch (Exception e) {
rollback(connection);
throw new DataAccessException(e);
} finally {
DataSourceUtils.releaseConnection(connection, dataSource);
TransactionSynchronizationManager.unbindResource(dataSource);

Choose a reason for hiding this comment

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

releaseConnection() 메서드 안에 unbindResource() 작업을 포함해도 되지 않을까라는 생각이 들어요!

Copy link
Author

Choose a reason for hiding this comment

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

좋은 생각입니다 👍

}
}

private void rollback(Connection connection) {
try {
connection.rollback();
} catch (SQLException ex) {
throw new DataAccessException(ex);
}
}
}
50 changes: 16 additions & 34 deletions app/src/test/java/com/techcourse/dao/UserDaoTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,11 @@
import com.techcourse.config.DataSourceConfig;
import com.techcourse.domain.User;
import com.techcourse.support.jdbc.init.DatabasePopulatorUtils;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.springframework.jdbc.core.JdbcTemplate;

import javax.sql.DataSource;

import java.sql.Connection;
import java.sql.SQLException;

import static org.assertj.core.api.Assertions.assertThat;
Expand All @@ -19,77 +16,62 @@ class UserDaoTest {

private UserDao userDao;
private DataSource dataSource;
private Connection con;

@BeforeEach
void setup() throws SQLException {
void setup() {
DatabasePopulatorUtils.execute(DataSourceConfig.getInstance());
dataSource = DataSourceConfig.getInstance();
con = dataSource.getConnection();
userDao = new UserDao(new JdbcTemplate());
userDao = new UserDao(new JdbcTemplate(dataSource));
final var user = new User("gugu", "password", "[email protected]");
userDao.insert(con, user);
}

@AfterEach
void close() throws SQLException {
con.close();
userDao.insert(user);
}

@Test
void findAll() throws SQLException {
Connection con = dataSource.getConnection();
final var users = userDao.findAll(con);
void findAll() {
final var users = userDao.findAll();

assertThat(users).isNotEmpty();
}

@Test
void findById() throws SQLException {
Connection con = dataSource.getConnection();
void findById() {

final var user = userDao.findById(con, 1L);
final var user = userDao.findById(1L);

assertThat(user.getAccount()).isEqualTo("gugu");
}

@Test
void findByAccount() throws SQLException {
Connection con = dataSource.getConnection();
void findByAccount() {

final var account = "gugu";
final var user = userDao.findByAccount(con, account);
final var user = userDao.findByAccount(account);

assertThat(user.getAccount()).isEqualTo(account);
}

@Test
void insert() throws SQLException {
Connection con1 = dataSource.getConnection();
Connection con2 = dataSource.getConnection();
void insert() {

final var account = "insert-gugu";
final var user = new User(account, "password", "[email protected]");
userDao.insert(con1, user);
userDao.insert(user);

final var actual = userDao.findById(con2, 2L);
final var actual = userDao.findById(2L);

assertThat(actual.getAccount()).isEqualTo(account);
}

@Test
void update() throws SQLException {
Connection con1 = dataSource.getConnection();
Connection con2 = dataSource.getConnection();
Connection con3 = dataSource.getConnection();
void update() {

final var newPassword = "password99";
final var user = userDao.findById(con1, 1L);
final var user = userDao.findById(1L);
user.changePassword(newPassword);

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

final var actual = userDao.findById(con3, 1L);
final var actual = userDao.findById(1L);

assertThat(actual.getPassword()).isEqualTo(newPassword);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,15 @@
import org.springframework.dao.DataAccessException;
import org.springframework.jdbc.core.JdbcTemplate;

import java.sql.Connection;

public class MockUserHistoryDao extends UserHistoryDao {

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

@Override
public void log(Connection con, UserHistory userHistory) {
super.log(con, userHistory);
public void log(UserHistory userHistory) {
super.log(userHistory);
throw new DataAccessException();
}
}
9 changes: 3 additions & 6 deletions app/src/test/java/com/techcourse/service/UserServiceTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
import org.springframework.jdbc.core.JdbcTemplate;

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

import static org.assertj.core.api.Assertions.assertThat;
Expand All @@ -23,17 +22,15 @@ class UserServiceTest {
private JdbcTemplate jdbcTemplate;
private UserDao userDao;
private DataSource dataSource;
private Connection con;

@BeforeEach
void setUp() throws SQLException {
this.jdbcTemplate = new JdbcTemplate();
this.userDao = new UserDao(jdbcTemplate);
DatabasePopulatorUtils.execute(DataSourceConfig.getInstance());
dataSource = DataSourceConfig.getInstance();
con = dataSource.getConnection();
this.jdbcTemplate = new JdbcTemplate(dataSource);
this.userDao = new UserDao(jdbcTemplate);
final var user = new User("gugu", "password", "[email protected]");
userDao.insert(con, user);
userDao.insert(user);
}

@Test
Expand Down
Loading