[JDBC 라이브러리 구현하기 - 4단계] 조이(김성연) 미션 제출합니다. #573
Conversation
| private static final ThreadLocal<Map<DataSource, Connection>> resources = new ThreadLocal<>(); | ||
|
|
||
| private TransactionSynchronizationManager() {} | ||
| private TransactionSynchronizationManager() { | ||
| } | ||
|
|
||
| public static Connection getResource(final DataSource key) { | ||
| final Map<DataSource, Connection> resource = resources.get(); | ||
|
|
||
| if (resource == null) { | ||
| return null; | ||
| } |
There was a problem hiding this comment.
저는 ThreadLocal의 Map을 인스턴스 변수에 아예 초기화했습니다!
이유는 위의 명시된 null처리를 따로 하지 않으면서
resources.get() 에서 발생하는 npe를 방지하기 위함인데요.
현재 이 getResource 메소드는 DataSourceUtils.getConnection()에서 사용되고 있는데요.
이 때
Connection connection = TransactionSynchronizationManager.getResource(dataSource);
if (connection != null) {
return connection;
}
...해당 메소드는 위처럼 key(datasource)에 해당하는 커넥션이 있을 때 커넥션을 그대로 반환하고, 없다면 새로운 커넥션을 동기화 매니저에 bind 합니다.
할당된 MAP이 없다 == datasource에 할당된 커넥션이 없다
와 같은 의미일지 토론해봐도 좋을 것 같네요.
There was a problem hiding this comment.
추가로 MAP을 변수 선언에서 미리 초기화하는 것과 초기화하지 않는 것의 장점까지도 이야기해보면 좋을 것 같아요!
There was a problem hiding this comment.
- MAP을 변수 선언에서 미리 초기화하는 것과 초기화하지 않는 것의 장점까지도 이야기해보면 좋을 것 같아요!
하마드 말대로 미리 초기화 하면 null 처리를 위한 중복 코드들을 제거할 수 있겠네요.
하마드가 제안해준대로 초기화 하는게 좋은 것 같아요.
초기화하지 않았을 때의 장점은 초기화 여부에 따른 특정 로직이 추가된다면 있겠지만 현재로서는 잘 모르겠네요.
굳이 따져보자면 처음에 사용하지 않는 map을 생성하지 않아도 된다는 점? (초기화의 장점에 비해 미미하네요..^^)
할당된 MAP이 없다 == datasource에 할당된 커넥션이 없다와 같은 의미일지 토론해봐도 좋을 것 같네요.
저는 지금의 경우만 본다면 동일한 의미로 봐도 된다고 생각합니다.
map이 없다 == TransactionSynchronizationManager 가 초기화 되지 않았다 정도의 의미로 분리해볼수는 있겠지만, 현재 상황에서는 크게 필요성이 와닿지는 않는 것 같아요.
그리고 TransactionSynchronizationManager 의 역할이 DataSource 에 할당된 Connection들을 관리하는 것이고, 초기화 여부에 따른 로직이 있는 것도 아니라서 할당된 MAP이 없다 == datasource에 할당된 커넥션이 없다 로 보는 것이 저는 좋은 것 같아요!
하마드의 의견은 어떤가요?!
There was a problem hiding this comment.
사실 저도 질문하면서 느꼈는데, 귀에 걸면 귀걸이고 코에 걸면 코걸이인 해석이긴 하죠?ㅋㅋㅋㅋ
제 의견은 초기화를 미리 했다면 위와 같은 부수적인 처리가 들어갈 필요가 없지 않을까?! 라는 의미였습니다ㅎㅎ
| if (resource == null) { | ||
| resources.set(new HashMap<>()); | ||
| } |
There was a problem hiding this comment.
초기화를 하지 않으면, 커넥션 바인딩시 이와 같은 추가적인 null 처리가 들어거야 하는 점도 고려할 수 있겠네요.
| if (resource == null) { | ||
| return null; | ||
| } |
There was a problem hiding this comment.
초기화를 하지 않으면, 커넥션 바인딩시 이와 같은 추가적인 null 처리가 들어거야 하는 점도 고려할 수 있겠네요.
ditto
| public User findById(final long id) { | ||
| return transactionManager.execute( | ||
| () -> userService.findById(id) | ||
| ); | ||
| } |
There was a problem hiding this comment.
서비스 추상화까지 진행하고 나니 조이가 TransactionManager로 중복되는 트랜잭션 처리를 해결한 진가가 잘 드러나네요
| } | ||
| } | ||
|
|
||
| private <T> T executeAction(final ConnectionAction<T> action, final Connection connection) throws SQLException { | ||
| private <T> T executeAction(final TransactionAction<T> action, |
There was a problem hiding this comment.
3단계때 리뷰를 놓친 부분 같네요!
보통 예외 시 트랜잭션이 롤백되는 디폴트 경우는,
RunTimeException 이 발생하는 경우죠.
그렇기 때문에 우리가 통상적으로 jdbcTemplate에서 발생하는 checked 예외인 SQL Exception을 런타임으로 바꾸는 작업도 하게 되고요.
하지만 지금 예외 롤백은 checked 예외인 SQL Exception이 발생할 때만 처리되고 있네요!
조이가 BaseJdbcTemplate에서 열심히 잡아 처리한DataAccessException 발생 시 TransactionManager는 이 예외를 어떻게 처리 하고 있을까요? (디버깅을 찍어보시면 힌트가 될거에요)
There was a problem hiding this comment.
헉 완전 놓친 부분인데 감사해요 !! 저도 몰랐네요..;;
SQLException 이 아닌 예외가 발생하면 connection.rollback() 부분이 수행이 안 되네요..
RunTimeException으로 수정하겠습니다.
역시 👍 감사합니다!!
안녕하세요 하마드!
요구사항에 맞게 기능 추가 및 리팩터링 진행했습니다 ~~
4단계 리뷰도 잘 부탁드립니다 🚀