-
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 3,4단계] 키아라 미션 제출합니다 #554
Changes from 12 commits
5c96f25
daef050
b80fa20
cc8cce4
2ccc69d
3acf179
e6278dd
47f248b
99f4543
63febec
4659c4a
92a5284
80bea25
4416994
f0d39b7
2ae1bc3
52f90a4
5f659c8
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 |
---|---|---|
@@ -1,62 +1,30 @@ | ||
package com.techcourse.dao; | ||
|
||
import com.techcourse.config.DataSourceConfig; | ||
import com.techcourse.domain.UserHistory; | ||
import org.springframework.jdbc.core.JdbcTemplate; | ||
import org.slf4j.Logger; | ||
import org.slf4j.LoggerFactory; | ||
import org.springframework.jdbc.core.JdbcTemplate; | ||
import org.springframework.jdbc.datasource.DataSourceUtils; | ||
|
||
import javax.sql.DataSource; | ||
import java.sql.Connection; | ||
import java.sql.PreparedStatement; | ||
import java.sql.SQLException; | ||
|
||
public class UserHistoryDao { | ||
|
||
private static final Logger log = LoggerFactory.getLogger(UserHistoryDao.class); | ||
|
||
private final DataSource dataSource; | ||
|
||
public UserHistoryDao(final DataSource dataSource) { | ||
this.dataSource = dataSource; | ||
} | ||
private final JdbcTemplate jdbcTemplate; | ||
private final DataSource dataSource = DataSourceConfig.getInstance(); | ||
|
||
public UserHistoryDao(final JdbcTemplate jdbcTemplate) { | ||
this.dataSource = null; | ||
this.jdbcTemplate = jdbcTemplate; | ||
} | ||
|
||
public void log(final UserHistory userHistory) { | ||
public void log(UserHistory userHistory) { | ||
Connection connection = DataSourceUtils.getConnection(dataSource); | ||
final var sql = "insert into user_history (user_id, account, password, email, created_at, created_by) values (?, ?, ?, ?, ?, ?)"; | ||
|
||
Connection conn = null; | ||
PreparedStatement pstmt = null; | ||
try { | ||
conn = dataSource.getConnection(); | ||
pstmt = conn.prepareStatement(sql); | ||
|
||
log.debug("query : {}", sql); | ||
|
||
pstmt.setLong(1, userHistory.getUserId()); | ||
pstmt.setString(2, userHistory.getAccount()); | ||
pstmt.setString(3, userHistory.getPassword()); | ||
pstmt.setString(4, userHistory.getEmail()); | ||
pstmt.setObject(5, userHistory.getCreatedAt()); | ||
pstmt.setString(6, userHistory.getCreateBy()); | ||
pstmt.executeUpdate(); | ||
} catch (SQLException e) { | ||
log.error(e.getMessage(), e); | ||
throw new RuntimeException(e); | ||
} finally { | ||
try { | ||
if (pstmt != null) { | ||
pstmt.close(); | ||
} | ||
} catch (SQLException ignored) {} | ||
|
||
try { | ||
if (conn != null) { | ||
conn.close(); | ||
} | ||
} catch (SQLException ignored) {} | ||
} | ||
log.info("[LOG] insert user into user_history"); | ||
jdbcTemplate.update(connection, sql, userHistory.getUserId(), userHistory.getAccount(), userHistory.getPassword(), userHistory.getEmail(), userHistory.getCreatedAt(), userHistory.getCreateBy()); | ||
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. 앗 넵! |
||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,41 @@ | ||
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; | ||
|
||
public class AppUserService implements UserService { | ||
|
||
private final UserDao userDao; | ||
private final UserHistoryDao userHistoryDao; | ||
private static final int QUERY_SINGLE_SIZE = 1; | ||
|
||
public AppUserService(final UserDao userDao, final UserHistoryDao userHistoryDao) { | ||
this.userDao = userDao; | ||
this.userHistoryDao = userHistoryDao; | ||
} | ||
|
||
public User findById(long id) { | ||
return userDao.findById(id); | ||
} | ||
|
||
public void insert(User user) { | ||
userDao.insert(user); | ||
} | ||
|
||
public void changePassword(long id, String newPassword, String createBy) { | ||
User user = findById(id); | ||
user.changePassword(newPassword); | ||
updateUser(user); | ||
userHistoryDao.log(new UserHistory(user, createBy)); | ||
} | ||
|
||
private void updateUser(User user) { | ||
int updateSize = userDao.update(user); | ||
if (updateSize != QUERY_SINGLE_SIZE) { | ||
throw new DataAccessException("갱신된 데이터의 개수가 올바르지 않습니다."); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,73 @@ | ||
package com.techcourse.service; | ||
|
||
import com.techcourse.config.DataSourceConfig; | ||
import com.techcourse.domain.User; | ||
import org.springframework.jdbc.datasource.DataSourceUtils; | ||
import org.springframework.transaction.support.TransactionSynchronizationManager; | ||
|
||
import javax.sql.DataSource; | ||
import java.sql.Connection; | ||
import java.sql.SQLException; | ||
import java.sql.SQLTransactionRollbackException; | ||
|
||
public class TxUserService implements UserService { | ||
|
||
private UserService userService; | ||
private final DataSource dataSource = DataSourceConfig.getInstance(); | ||
|
||
public TxUserService(UserService userService) { | ||
this.userService = userService; | ||
} | ||
|
||
@Override | ||
public User findById(long id) throws SQLException { | ||
Connection connection = null; | ||
try { | ||
if (TransactionSynchronizationManager.hasConnection(dataSource)) { | ||
return userService.findById(id); | ||
} | ||
connection = DataSourceUtils.getConnection(dataSource); | ||
connection.setAutoCommit(false); | ||
User user = userService.findById(id); | ||
connection.commit(); | ||
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. commit과 rollback에 대해 공부해보면 좋을 것 같아요 ! transaction이 필요하지 않은 로직은 JdbcTemplate에서 커넥션을 관리하고 있으므로 그리고 conncetion을 close하는 메서드는 반드시 finally 문 안에 있어야해요! |
||
DataSourceUtils.releaseConnection(connection, dataSource); | ||
return user; | ||
} catch (SQLException e) { | ||
connection.rollback(); | ||
throw new SQLTransactionRollbackException(e.getMessage()); | ||
} | ||
} | ||
|
||
@Override | ||
public void insert(User user) throws SQLException { | ||
Connection connection = null; | ||
try { | ||
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. try - catch 와 커넥션을 만들고 커밋/롤백하는 작업이 반복되고 있는데... 이 부분을 분리하여 다른 메서드로 정의하고 이 클래스의 메인 로직인 userService의 메서드를 파라미터로 넘기면 중복을 제거할 수 잇을 것 같아요! |
||
if (TransactionSynchronizationManager.hasConnection(dataSource)) { | ||
userService.insert(user); | ||
return; | ||
} | ||
connection = DataSourceUtils.getConnection(dataSource); | ||
connection.setAutoCommit(false); | ||
userService.insert(user); | ||
connection.commit(); | ||
} catch (SQLException e) { | ||
connection.rollback(); | ||
throw new SQLTransactionRollbackException(e.getMessage()); | ||
} | ||
} | ||
|
||
@Override | ||
public void changePassword(long id, String newPassword, String createBy) throws SQLException { | ||
Connection connection = DataSourceUtils.getConnection(dataSource); | ||
connection.setAutoCommit(false); | ||
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. setAutoCommit도 SQL Exception을 발생시킬 수 있는데 try문에 안 넣어도 될까요?? |
||
try { | ||
userService.changePassword(id, newPassword, createBy); | ||
connection.commit(); | ||
} catch (SQLException e) { | ||
connection.rollback(); | ||
throw new SQLTransactionRollbackException(e.getMessage()); | ||
} finally { | ||
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. transaction을 사용 완료한 connection의 autocommit을 다시 켜준다면 DataSourceUtil에서 해당 커넥션이 여러 작업을 수행하고 있는지 알 수 있을 것 같아요 ㅎㅅㅎ 그럼 realseConncetion에서 종료할 작업을 잘 분리할 수 있을 것 같아요 |
||
DataSourceUtils.releaseConnection(connection, dataSource); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,33 +1,14 @@ | ||
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 UserService { | ||
import java.sql.SQLException; | ||
|
||
private final UserDao userDao; | ||
private final UserHistoryDao userHistoryDao; | ||
public interface UserService { | ||
|
||
public UserService(final UserDao userDao, final UserHistoryDao userHistoryDao) { | ||
this.userDao = userDao; | ||
this.userHistoryDao = userHistoryDao; | ||
} | ||
User findById(long id) throws SQLException; | ||
|
||
public User findById(final long id) { | ||
return userDao.findById(id) | ||
.orElseThrow(() -> new IllegalArgumentException("해당하는 유저가 없습니다.")); | ||
} | ||
void insert(User user) throws SQLException; | ||
|
||
public void insert(final User user) { | ||
userDao.insert(user); | ||
} | ||
|
||
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)); | ||
} | ||
void changePassword(long id, String newPassword, String createBy) throws SQLException; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,63 +7,79 @@ | |
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; | ||
import static org.assertj.core.api.AssertionsForClassTypes.assertThatThrownBy; | ||
|
||
class UserDaoTest { | ||
|
||
private UserDao userDao; | ||
private Connection connection; | ||
|
||
@BeforeEach | ||
void setup() { | ||
DatabasePopulatorUtils.execute(DataSourceConfig.getInstance()); | ||
void setup() throws SQLException { | ||
DataSource dataSource = DataSourceConfig.getInstance(); | ||
DatabasePopulatorUtils.execute(dataSource); | ||
connection = dataSource.getConnection(); | ||
|
||
userDao = new UserDao(new JdbcTemplate(DataSourceConfig.getInstance())); | ||
final var user = new User("gugu", "password", "[email protected]"); | ||
userDao = new UserDao(new JdbcTemplate(dataSource)); | ||
userDao.insert(user); | ||
} | ||
|
||
@Test | ||
void findAll() { | ||
final var users = userDao.findAll(); | ||
final var users = userDao.findAll(connection); | ||
|
||
assertThat(users).isNotEmpty(); | ||
} | ||
|
||
@Test | ||
void findById() { | ||
final var user = userDao.findById(1L).get(); | ||
final var user = userDao.findById(1L); | ||
|
||
assertThat(user.getAccount()).isEqualTo("gugu"); | ||
} | ||
|
||
@Test | ||
void findByAccount() { | ||
final var account = "gugu"; | ||
final var user = userDao.findByAccount(account).get(); | ||
final var user = userDao.findByAccount(connection, account); | ||
|
||
assertThat(user.getAccount()).isEqualTo(account); | ||
} | ||
|
||
@Test | ||
void findByWrongAccount() { | ||
final var account = "gaga"; | ||
assertThatThrownBy(() -> userDao.findByAccount(connection, account)) | ||
.isInstanceOf(RuntimeException.class); | ||
} | ||
|
||
@Test | ||
void insert() { | ||
final var account = "insert-gugu"; | ||
final var user = new User(account, "password", "[email protected]"); | ||
userDao.insert(user); | ||
|
||
final var actual = userDao.findById(2L).get(); | ||
final var actual = userDao.findById(2L); | ||
|
||
assertThat(actual.getAccount()).isEqualTo(account); | ||
} | ||
|
||
@Test | ||
void update() { | ||
final var newPassword = "password99"; | ||
final var user = userDao.findById(1L).get(); | ||
final var user = userDao.findById(1L); | ||
user.changePassword(newPassword); | ||
|
||
userDao.update(user); | ||
userDao.update(connection, user); | ||
|
||
final var actual = userDao.findById(1L).get(); | ||
final var actual = userDao.findById(1L); | ||
|
||
assertThat(actual.getPassword()).isEqualTo(newPassword); | ||
} | ||
|
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도 DataSource를 가지고 있으니, dao에서 반복적으로 전달하기 보단
getConnection과정도 JdbcTemplate에서 해주는 것은 어떤가여??
dao에서 connection을 관리하는 것이 책임 분리가 되었다고 느껴지지 않아서영,,,
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.
기억속엔 connection 파라미터를 없앤걸로 알고 있는데 이렇게나 많이 남아있었다니.. 발견해주셔서 감사합니다