[JDBC 라이브러리 구현하기 - 4단계] 에단(김석호) 미션 제출합니다. #556
Conversation
| @Override | ||
| public User findById(final long id) { | ||
| return userDao.findById(id) | ||
| .orElseThrow(() -> new NoSuchElementException("id 값으로 해당하는 User 를 찾을 수 없습니다.")); |
There was a problem hiding this comment.
| .orElseThrow(() -> new NoSuchElementException("id 값으로 해당하는 User 를 찾을 수 없습니다.")); | |
| .orElseThrow(() -> new NoSuchElementException(String.format("id 값으로 해당하는 User 를 찾을 수 없습니다. 입력값: %s", id))); |
- 클라이언트가 어떤 입력값을 보냈을 때 찾을 수 없었는지 추적할 수 있도록 메세지에 입력값을 남기세요.
- NoSuchElementException 가 적절한가요? Element라는 용어는 자바 컬렉션에서 사용되는걸로 보이는데, 적절한 예외일까요?
There was a problem hiding this comment.
- 넵 디버깅하기가 쉬워지겠네요.
- 오.. 그 점은 고려하지 못했습니다. 앞으로 예외의 패키지도 의식적으로 보는 연습이 필요할것 같습니다
| public class TransactionTemplate { | ||
|
|
||
| public void executeWithTransaction(final TransactionLogicExecutor transactionLogicExecutor) { | ||
| final DataSource dataSource = DataSourceConfig.getInstance(); |
There was a problem hiding this comment.
DataSource는 주입 받도록 만드는게 좋지 않을까요? Connection과는 다른 결이에요.
싱글톤 패턴을 사용할 때 문제점을 찾아봅시다.
There was a problem hiding this comment.
이 부분에 대해서 고민을 많이 해봤는데요. 가장 간단하게는 연관관계를 가지고 있어서 테스트와 같이 DataSource를 변경할 수도 있는 상황에서 변경하기가 어렵습니다. 나아가서 일반적인 DataSource 구현체로 HikariCP를 사용하고 있지만, 더 좋은게 나와서 바꾸려면 어려울 수 있습니다. DataSource를 TransactionTemplate을 만들 때 주입받는 방식으로 변경했습니다.
|
|
||
| private void rollback(final Connection connection) { | ||
| if (connection != null) { | ||
| try { |
|
|
||
| @Override | ||
| public User findById(final long id) { | ||
| return userService.findById(id); |
There was a problem hiding this comment.
findById는 왜 트랜잭션을 적용하지 않나요?
There was a problem hiding this comment.
수업에서 들은대로 readonly로 트랜잭션 걸어주도록 리팩토링했습니다
| @@ -81,7 +80,7 @@ void dirtyReading() throws SQLException { | |||
| final var subConnection = dataSource.getConnection(); | |||
|
|
|||
| // 적절한 격리 레벨을 찾는다. | |||
| final int isolationLevel = Connection.TRANSACTION_NONE; | |||
| final int isolationLevel = Connection.TRANSACTION_SERIALIZABLE; | |||
There was a problem hiding this comment.
dirty read를 해결할 수 있는 최소한의 격리 레벨로 설정해보세요.
| @@ -130,7 +129,7 @@ void noneRepeatable() throws SQLException { | |||
| connection.setAutoCommit(false); | |||
|
|
|||
| // 적절한 격리 레벨을 찾는다. | |||
| final int isolationLevel = Connection.TRANSACTION_NONE; | |||
| final int isolationLevel = Connection.TRANSACTION_SERIALIZABLE; | |||
There was a problem hiding this comment.
여기도 마찬가지로 격리 레벨을 적절히 조절해보세요.
| .hasSize(0) | ||
| .containsExactly(""); | ||
| .hasSize(1) | ||
| .containsExactly("transaction.stage2.FirstUserService.saveFirstTransactionWithRequired"); | ||
| } | ||
|
|
||
| /** |
| @@ -0,0 +1,7 @@ | |||
| package com.techcourse.service.transaction; | |||
There was a problem hiding this comment.
패키지를 프레임워크로 옮기는게 낫지 않을까요?
| @@ -0,0 +1,51 @@ | |||
| package com.techcourse.service.transaction; | |||
There was a problem hiding this comment.
패키지를 프레임워크로 옮기는게 낫지 않을까요?
클라이언트가 인자로 다른 connection을 넘겨줄 수 있으니 안전하게 확인하는 장치를 둔 것 같네요. |
|
그런 이유가 있었네요. 알려주셔서 감사합니다! 히스토리를 찾아봤지면 처음 구현할 때부터 저렇게 구현되어있던 걸 확인했습니다. 구구가 말씀해주신 이유말고는 다른 이유는 없을것 같습니다 |
안녕하세요 준팍! 4단계 미션 구현을 완료해서 리뷰 요청 보냅니다😄
이번 단계에서 요구사항과 달라진 부분은 template callback 패턴을 사용해서 트랜잭션을 처리한 부분입니다.
트랜잭션의 서비스에서 트랜잭션을 하는 동작이 많아지면 중복된 로직이 발생할거라 예상되기 때문에 미리 분리했습니다.
추가적으로 이번 단계를 진행하면서 궁금한점이 있었는데요.
스프링은 위와 같이 datasource와 connection이 일치하는 커넥션인지 확인하고 release를 하고 있습니다. 저는 잘 몰라서 추가적인 동작을 구혀안해주었는데요. 왜 스프링은 저렇게 구현을 했을까요?🤔 저런 상황이 필요한 상황은 언제가 될까요? 궁금해서 여쭤봅니다.
그럼 이번에도 리뷰 잘 부탁드립니다.🙇♀️🙇♂️