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);
}
}
}
33 changes: 33 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,33 @@
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;
}

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 var user = userDao.findById(id);
user.changePassword(newPassword);
userDao.update(user);
userHistoryDao.log(new UserHistory(user, createBy));
}
}
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);
}
}
}
86 changes: 4 additions & 82 deletions app/src/main/java/com/techcourse/service/UserService.java
Original file line number Diff line number Diff line change
@@ -1,88 +1,10 @@
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 org.springframework.dao.DataAccessException;

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

Choose a reason for hiding this comment

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

좋은 과제가 하나 떠올랐어요 하마드!
마침 UserService를 인터페이스로 만드셨으니,
JDK Dynamic Proxy라는 키워드로 학습하셔서
서비스 코드와 트랜잭션 처리 코드를 완벽하게 분리해보세요.

    public void changePassword(final long id, final String newPassword, final String createBy) {
        final var user = userDao.findById(id);
        user.changePassword(newPassword);
        userDao.update(user);
        userHistoryDao.log(new UserHistory(user, createBy));
    } <- 서비스 코드는 비즈니스 로직만 있게, 근데도 사용할 때는 트랜잭션 적용까지 되어 있게 해보기

만약 성공하시면 진짜 기분 끝내줄 거에요!

Copy link
Author

Choose a reason for hiding this comment

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

오오 알지 못하고 있던 키워드였는데, JDK Dynamic Proxy를 활용하면 트랜잭션 처리 구현체를 따로 만들고, 트랜잭션 구현체에서 호출하는 메소드마다 트랜잭션 처리를 일일이 적용할 필요가 없겠네요! 부끄럽지만 여우가 제출한 코드를 레퍼런스 삼아서 저도 적용해 봤어요^^

궁금한 점이 하나 있는데, 이전 방식은
서비스 인터페이스(UserService) 생성
-> 비즈니스 로직만 가진 서비스 구현체(AppUserService) 생성
-> 비즈니스 구현체(AppUserService)를 주입받는 트랜잭션 처리용 서비스 구현체(TxUserService) 생성
하는 방식으로 트랜잭션 처리 관심사와 비즈니스 관심사를 분리했는데,

제가 다이나믹 프록시를 적용하면서 느낀 점은 이전 트랜잭션 처리 구현체를 만든 것과 다르게,
“메소드를 일일이 @OverRide 할 필요가 없다” 라는 것이었는데요.

그렇다면 결국 관심사 분리는 이전 방식으로도 이루어질 수 있지만, 단순히 코드 경량화가 이루어진다는 것이 추가된 장점일까요? 다른 장점이 있는지 잘 와닿지가 않아 질문해요.

Choose a reason for hiding this comment

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

사실 하마드의 코드도 관심사의 분리가 잘 된 코드에요!
다만 TxUserService은 Override한 메서드 각각에 트랜잭션 처리 코드가 중복으로 구현된 반면
프록시를 사용한 코드는 invoke() 메서드만 Override하면 일괄적으로 적용이 된다는 점에서,
'같은 관심사를 가진 코드를 한 곳에 모아서 수정과 확장이 편리하게 한다'는 관심사 분리의 장점을 극대화할 수 있다고 할 수 있어요.
그걸 쫌 간략하게 얘기하면 하마드의 말처럼 코드 경량화라고 표현할 수도 있겠네요. 어차피 클린코드라는 것도 결국 코에 걸면 코걸이고 귀에 걸면 귀걸이고 .. 😁


public class UserService {

private final UserDao userDao;
private final UserHistoryDao userHistoryDao;
private final DataSource dataSource;

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

public User findById(final long id) {
try (Connection con = dataSource.getConnection()) {
try {
con.setAutoCommit(false);

User user = userDao.findById(con, id);

con.commit();
return user;
} catch (Exception e) {
con.rollback();
throw new DataAccessException(e);
} finally {
con.setAutoCommit(true);
con.close();
}
} catch (SQLException e) {
throw new DataAccessException(e);
}
}

public void insert(final User user) {
try (Connection con = dataSource.getConnection()) {
try {
con.setAutoCommit(false);

userDao.insert(con, user);

con.commit();
} catch (Exception e) {
con.rollback();
throw new DataAccessException(e);
} finally {
con.setAutoCommit(true);
con.close();
}
} catch (SQLException e) {
throw new DataAccessException(e);
}
}

public void changePassword(final long id, final String newPassword, final String createBy) {
try (Connection con = dataSource.getConnection()) {
try {
con.setAutoCommit(false);

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

con.commit();
} catch (Exception e) {
con.rollback();
throw new DataAccessException(e);
} finally {
con.setAutoCommit(true);
con.close();
}
} catch (SQLException e) {
throw new DataAccessException(e);
}
}
User findById(final long id);
void insert(final User user);
void changePassword(final long id, final String newPassword, final String createBy);
}
Loading
Loading