-
Notifications
You must be signed in to change notification settings - Fork 300
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
Changes from 1 commit
f4e8aa3
940e754
a208a88
2c18f84
5d0eb39
ccdfaff
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 트랜잭션이 적용된 서비스와 안 적용된 서비스 이렇게 두 개를 만드신 건가요? 😮 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 순수 비즈니스 로직만 가진 서비스 클래스 (AppUserService 그런데 테스트 코드에서 AppUserService appUserService = new AppUserService(userDao, userHistoryDao);
final var userService = new TxUserService(appUserService,dataSource); 다음과 같이 직접 구현체를 주입시켜야 하는 어색함이 있네요... There was a problem hiding this comment. Choose a reason for hiding this commentThe 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); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. releaseConnection() 메서드 안에 unbindResource() 작업을 포함해도 되지 않을까라는 생각이 들어요! There was a problem hiding this comment. Choose a reason for hiding this commentThe 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); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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; | ||
|
@@ -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); | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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; | ||
|
@@ -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 | ||
|
There was a problem hiding this comment.
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이 나오는데,
예외의 패키지를 보면 org.h2.jdbc로 h2 라이브러리에서 제공하는 예외임을 알 수가 있죠.
이건 우리가 h2를 사용하기 때문에 h2 예외가 발생한 것이고, mysql을 쓰면 mysql 예외가, mariaDB를 쓰면 mariaDB 예외가 발생하겠죠.
지금의 UserDao는 catch문에서 SQLException 전체를 잡고 있는데.
만약 SyntaxError 하나만 잡아서 특별한 처리를 하는 상황이 왔다고 가정해 보아요.
h2를 쓰고 있는 지금은 syntax에러가 발생했을 때 catch문에서 딱 잡아주지만
만약 데이터베이스를 MySQL로 바꾸었다고 하면? 안 잡히겠죠! (두둥)
왜냐면 위 catch문에서 잡는 SyntaxError는 h2 예외이기 때문에
mysql에서 나온 SyntaxError는 당연히 안 잡겠죠?
이를 방지하려면 SQLException을 잡는 모든 try-catch문을 찾아가 h2 예외를 잡는 대신 mysql 예외를 잡도록 바꿔야겠네요.
그런데 운영 환경에서는 mysql을 쓰지만 테스트 환경에서는 h2를 쓰겠다! 라고 하면
또다시 모든 try-catch문을 찾아가 h2 예외를 잡는 catch문을 밑에 더 붙여줘야 할 거에요.
'jdbcTemplate에서는 예외만 발생시키고 이걸 어떻게 처리할지는 dao에서 개발자가 결정하는게 옳다' 라고 하마드는 말씀해 주셨지만,
현재 사용중인 DBMS와 미래에 사용하게 될 DBMS의 종류까지 신경써가며 코드를 작성해야 하는 것이
과연 개발자에게 건강한 결정권을 주었다고 볼 수 있는지 모르겠어요.
하마드의 생각이 궁금하군요!
There was a problem hiding this comment.
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으로 바꿔 던지는 것을 간과하고 있었군요.
수정하도록 할게요 좋은 리뷰 고마워요ㅎㅎ