[JDBC 라이브러리 구현하기 - 4단계] 아코(안석환) 미션 제출합니다. #564
Conversation
| public static Connection getResource(DataSource key) { | ||
| final Map<DataSource, Connection> connections = resources.get(); | ||
| if (connections.containsKey(key)) { | ||
| return connections.get(key); | ||
| } | ||
| return null; | ||
| } |
There was a problem hiding this comment.
Map은 키 값이 없을 때 무슨 값을 반환하도록 설정되어 있을까요? 🤔
There was a problem hiding this comment.
Map에 존재하지 않는 키 값으로 조회를 하면 null을 반환하네요! 굳이 분기처리하지 않고 바로 connections.get(key)를 반환하겠습니다!
|
|
||
| @FunctionalInterface | ||
| public interface ServiceLogicExecutor { | ||
|
|
||
| void execute() throws SQLException; | ||
| } |
There was a problem hiding this comment.
Runnable을 사용하지 않고 따로 ServiceLogicExecutor를 사용해주신 이유가 있을까요?
There was a problem hiding this comment.
구현을 할 당시에 Runnable이 떠오르지 않았어서 구현을 했었는데 Runnable로 처리가 가능할것 같네요! 이 부분 수정하겠습니다!
| public User findById(final long id) { | ||
| return userService.findById(id); | ||
| } |
There was a problem hiding this comment.
조회는 트랜잭션이 없어도 괜찮을까요?!
스프링에서는 조회의 경우에도 @Transactional(readonly = true)처럼 설정을 해주는데요.
이런 경우 DB 레벨에서 최적화가 가능해지는 것으로 알고 있어요. (DB가 읽기 전용 최적화를 지원한다면)
적용은 안해주셔도 좋으나, 읽기 전용 트랜잭션으로 findById를 감싸는 방법도 고민해보셨으면 좋겠습니다~
There was a problem hiding this comment.
저도 리뷰를 받고 고민을 해보니 데이터를 조회하는 것도 결국에는 transaction의 격리수준을 보장 받아야 하기 때문에 데이터를 조회하는 것도 transsaction을 적용하겠습니다.
There was a problem hiding this comment.
connection.setReadOnly()설정이 있어서 이를 적용했습니다!
| import com.techcourse.domain.User; | ||
| import com.techcourse.support.transaction.TransactionExecutor; | ||
|
|
||
| public class TxUserService { |
There was a problem hiding this comment.
프록시의 역할을 하는 것 같은데 UserService 인터페이스를 구현하고 있지 않네요!
| serviceLogicExecutor.execute(); | ||
| connection.commit(); | ||
| } catch (SQLException e) { | ||
| rollback(connection); |
There was a problem hiding this comment.
롤백은 SQLException이 발생할 때만 수행되어야 할까요?
SQLException에 대해서만 rollback을 수행하면 어떤 문제가 생길 수 있을까요?
There was a problem hiding this comment.
Runtime 시점에 예외를 처리하지 못할 것 같습니다. Runtim에도 예외가 발생할 수 있게 수정하겠습니다.
꼼꼼한 리뷰 감사합니다!
| @@ -2,22 +2,34 @@ | |||
|
|
|||
| import javax.sql.DataSource; | |||
| import java.sql.Connection; | |||
| import java.util.HashMap; | |||
| import java.util.Map; | |||
|
|
|||
| public abstract class TransactionSynchronizationManager { | |||
|
|
|||
| private static final ThreadLocal<Map<DataSource, Connection>> resources = new ThreadLocal<>(); | |||
There was a problem hiding this comment.
이 부분 저도 고민했던 건데 생각해보시면 좋을 것 같아 질문드립니다! 😌
ThreadLocal을 Map으로 구현한 이유가 무엇일까요?
어차피 스레드마다 지역변수처럼 사용할텐데, ThreadLocal<Connection>처럼 사용하면 안되는걸까요?
There was a problem hiding this comment.
저도 생각을 해보았는데 현재 dataSource의 구현체도 여러개이고 Connection도 여러 개의 구현체가 존재합니다.
즉, 여러 dataSource를 통해서 여러개의 Connection을 생성이 가능하다는 것이고 이를 관리하기 위해서 Map으로 관리하는 것 같습니다.
이에 맞는 상황인지는 모르겠지만 하나의 프로그램에서 여러 DB를 이용하는 상황이 있다고 하겠습니다.
이 때에는 각 DB에 맞는 connection을 Map으로 관리하고 사용되는 DB에따라 알맞은 connection을 불러와서 이용할 것 같습니다.
그래서 저는 Map으로 관리를 한다고 생각했습니다.
There was a problem hiding this comment.
👍👍 저도 그렇게 생각했었습니다!
서비스가 하나의 DataSource만 사용한다는 보장도 없을 뿐더러 각각의 커넥션을 따로 관리해야하기 때문에 Map으로 관리한다는 생각이 들었었는데 아코도 똑같이 생각하고 계셨군요 ㅎㅎ
세부사항 - 불필요한 customException 제거 - TxUserService가 UserSerivce를 구현하도록 수정
안녕하세요 테오!
이제 어느덧 jdbc 미션도 마지막이 되었네요..
4단계를 요구사항을 진행하면서 추가적으로 반복되는 코드가 보여서 그 부분도 함께 리팩토링을 진행했습니다.
마지막 리뷰도 잘 부탁드립니다!🙇♂️🙇♂️