[JDBC 라이브러리 구현하기 - 4단계] 홍실(홍혁준) 미션 제출합니다.#562
Conversation
JJ503
left a comment
JJ503
left a comment
There was a problem hiding this comment.
안녕하세요 홍실!
드디어 마지막 미션이네요.
이번 미션도 요구사항은 물론 코드도 깔끔하고, 테스트 코드까지 완벽하십니다 👍
홍실에게 배워가기만 하는 미션이네요 ㅎㅎ
그런데 홍실의 의견이 궁금한 사항들이 있어 request changes를 하게 되었습니다.
해당 내용들 한번 확인해 주시면 감사하겠습니다!
| ) { | ||
| final Connection connection = ConnectionManager.getInstance().getConnection(dataSource); | ||
| ) { | ||
| final Connection connection = DataSourceUtils.getConnection(dataSource); |
There was a problem hiding this comment.
현재로 로직에 의하면 TxUserService의 로직을 수행할 때는 해당 커넥션을 해제해 주지만, AppUserService로 수행하는 경우 해당 커넥션은 해제해주지 않는 것으로 보입니다.
혹시 제가 로직 중 놓친 부분이 있을까요?
현재 제가 고민 중인 상황이기도 해 질문드립니다!
There was a problem hiding this comment.
확실히 별도로 처리를 해주고 있진 않긴 하네요. JdbcTemplate이 transaction 범위 안에 있으면 괜찮지만, 그렇지 않은 경우엔 close 해주진 않는군요.
finally에서, transaction이 동작중인지 여부로 close가 가능할 것 같아요. 한 번 구현해보겠습니다.
| import org.springframework.jdbc.core.exception.DataNotFoundException; | ||
| import org.springframework.transaction.exception.ConnectionManagerException; | ||
|
|
||
| @Deprecated |
There was a problem hiding this comment.
오 실제로 사용하는 건 처음 보네요!
그리고 해당 클래스를 사용하지 않은 것도 굿 👍
| private final UserHistoryDao userHistoryDao; | ||
| public User findById(final long id); | ||
|
|
||
| public UserService(final UserDao userDao, final UserHistoryDao userHistoryDao) { | ||
| this.userDao = userDao; | ||
| this.userHistoryDao = userHistoryDao; | ||
| } | ||
| public void insert(final User user); | ||
|
|
||
| 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 ConnectionManager connectionManager = ConnectionManager.getInstance(); | ||
| connectionManager.beginTransaction(); | ||
| try { | ||
| final var user = findById(id); | ||
| user.changePassword(newPassword); | ||
|
|
||
| userDao.update(user); | ||
| userHistoryDao.log(new UserHistory(user, createBy)); | ||
|
|
||
| connectionManager.commit(); | ||
| } catch (final Exception e) { | ||
| connectionManager.rollback(); | ||
| throw e; | ||
| } finally { | ||
| connectionManager.close(); | ||
| } | ||
| } | ||
| public void changePassword(final long id, final String newPassword, final String createBy); |
There was a problem hiding this comment.
인터페이스이기에 접근제어자는 생략되어도 상관 없을 것 같네요!
There was a problem hiding this comment.
뺴도 될 것 같네요. 그냥 시그니처 다 복사하느라 들어온 것 같아요. 반영하겠습니다!!
| //의문 왜 여기서 datasource를 받는가? unbind도 안하는데 | ||
| //spring의 코드를 봤는데 너무 방대함, 한 번 학습해봐야 할듯? |
There was a problem hiding this comment.
현재로 로직에 의하면 TxUserService의 로직을 수행할 때는 해당 커넥션을 해제해 주지만, AppUserService로 수행하는 경우 해당 커넥션은 해제해주지 않는 것으로 보입니다.
아직은 생각뿐이긴 하지만, 위 제 의문을 해소하기 위한 로직이 여기 들어갈 수 있지 않을까 생각했습니다.
datasource를 통해 현재 트랜잭션 연결 상태에 따라 커넥션을 종료할지 말지에 대해 이 파트에서 진행할 수 있지 않을까 일단 고민 중에 있습니다..!
하지만, 당장의 구상일 뿐이라 이 이야기가 홍실에게 도움이 될지는 잘 모르겠네요... 😅
홍실은 어떻게 생각하시나요?
There was a problem hiding this comment.
제이미의 의견이 맞는 것 같아요. 실제 spring에서 이 부분에 transaction 여부에 따라 connection을 close하기도 하고, 해제하지 않기도 하더라고요.
(주석만 확인했는데 어떻게, 동작하는 지는 너무 복잡해서 다 학습하진 못했습니다.)
음ㅁ 조금 더 고민을 해봐야 할 것 같네요 한 번 노력해보겠습니다.
| private final UserService userService; | ||
| private final TransactionTemplate transactionTemplate; | ||
|
|
||
| public TxUserService(final UserService userService, |
There was a problem hiding this comment.
UserService 인터페이스를 파라미터로 받은 이유가 궁금합니다!
저는 UserService로 받게 된다면, TxUserService가 들어와 문제가 발생할 수도 있지 않을까라는 생각을 해 필드는 UserService로 하되 파라미터는 AppUserService타입을 설정해 주었습니다.
그런데 그냥 인터페이스를 받기로한 홍실의 결정 이유가 궁금합니다!
There was a problem hiding this comment.
항상 습관적으로, 결합도를 낮추려고 하다보니 인터페이스를 파라미터로 받고 있네요.
뭔가 service의 경우에는 확장을 할 필요도 없고, moking을 할 상황도 없어서, AppUserService가 좀 더 적절한 것 같아요.
좋은 의견 감사합니다!!
hong-sile
left a comment
hong-sile
left a comment
There was a problem hiding this comment.
트랜잭션일 때 Connection을 close하는 경우와 트랜잭션이 아닐 떄 Connection을 close하는 경우를 구현해봤습니다.
트랜잭션 활성화 여부를 통해서
JdbcTemplate에서 close하거나 close하지 않도록 했어요.
트랜잭션이 활성화 되어 있는 경우 conenction을 close하는 책임은 TransactionTemplate에
트랜잭션이 활성화 되어 있지 않는 경우 connection을 close하는 책임은 JdbcTemplate에
뒀습니다.
JJ503
left a comment
JJ503
left a comment
There was a problem hiding this comment.
홍실 수정하신 코드 잘 봤습니다!
그런데 한 가지 궁금한 점이 있어 코멘트 남겨두었으니 확인해 주시면 감사하겠습니다.
해당 코멘트는 단순한 궁금증이기에 이번 미션은 여기서 approve 하겠습니다.
4단계까지 구현하시는 동안 고생 많으셨습니다!
홍실 코드를 보면서 많이 배울 수 있었습니다.
그럼 다음 미션도 파이팅 하세요~
| public static boolean isTransactionActive(final DataSource dataSource) { | ||
| return resources.get().containsKey(dataSource); | ||
| try { | ||
| return !resources.get().get(dataSource).getAutoCommit(); |
There was a problem hiding this comment.
connection.getAutoCommit()이 아닌dataSource를 통해 authCommit 값을 가져온 이유가 궁금합니다!
2 participants
안녕하세요 제이미! 4단계 리뷰 요청 드립니다.
이전 PR에 대한 질문은 답을 해뒀어요!!
4단계를 진행하다가 의문인 점이 하나가 있는데요.
DataSourceUtils에서 releaseConnection의 파라미터에 DataSource가 있어요.
해당 DataSource를 이용해줘서 TxSyncronizationManager에서 unbind를 해줘야하는건가? 싶었지만
LMS에 있는 코드
에서는 unbind를 release랑 별도로 하더라고요...
그래서 releaseConnection에서 dataSource를 쓰는 것 같긴 한데, 어떨 때 쓰는 걸까요?
학습을 해보려고 했는데, 양이 많아서 차마 다 학습을 하진 못했어요.
혹시 해당 내용에 대해서 제이미가 알고 있는 내용이 있으면 공유해주시면 감사할 것 같아요.
(꼭 답을 해주셔야 되는 건 아닙니다!! 스스로도 학습해볼게요)
저 부분 말고는 크게 의문 가는 부분은 없고요.
코드에 대해서 설명을 잠시 하자면
graph LR TransactionTemplate --> DataSourceUtils DataSourceUtils --> TransactionSynchronizationManager TxUserService --> AppUserService TxUserService --> TransactionTemplate위 처럼 분류해서 transaction에 대한 책임과 비즈니스에 대한 책임을 분리 했습니다.
기존 ConnectionManager는 Deprecated 했습니다. 미션 요구사항에 맞춰서 작성하다보니 DataSourceUtils와 TransactionSyncrhonizationManager가 해당 역할을 대신하더라고요.
코드 보시면서 의문이 가는 부분 있으시면 편하게 이야기해주시면 감사하겠습니다.