[JDBC 라이브러리 구현하기 - 3, 4단계] 제이미(임정수) 미션 제출합니다. #526
Conversation
JJ503
changed the title
| public class AppUserService implements UserService { | ||
|
|
||
| private final UserDao userDao; | ||
| final UserHistoryDao userHistoryDao; |
There was a problem hiding this comment.
접근 제어자가 빠진 것 같아요 :)
혹시 의도해주신 부분이라면 말씀해주세요 ㅎㅎ
|
|
||
| @Override | ||
| public User findById(final long id) { | ||
| return userDao.findById(id); |
There was a problem hiding this comment.
저도 이번 미션을 하면서 findById와 같이 조회가 발생하는 쿼리에 대해서 트랜잭션을 설정할지 말지 고민이 되었는데요 !!
제이미는 조회 쿼리에 트랜잭션을 걸어주신 이유가 궁금합니다 ㅎㅎ (정말 궁금증입니다 !)
There was a problem hiding this comment.
저는 TxUserService를 트랜잭션을 설정한 서비스라고 이해해서 해당 서비스 클래스 내의 메서드에는 모두 트랜잭션을 설정해 주었습니다..!
크게 고민 없이 트랜잭션이 없는 사용은 AppUserService를 통해 진행할 수 있기 때문에 현재와 같은 형태가 되었습니다.
그렇다면 그레이는 조회 쿼리의 경우 설정해 주셨을까요?
그레이가 최종적으로 내린 결론과 이유가 궁금하네요!
There was a problem hiding this comment.
저는 단건 조회 findById의 경우에는 해당 메서드에서 다른 메서드를 호출하지 않기 때문에 트랜잭션을 굳이 설정하지 않았지만, 트랜잭션을 걸지 않을 이유도 없다고 생각이 들더라구요 !
그래서 일관성을 위해서 트랜잭션을 달아주는 방법도 괜찮다고 느꼈습니다 :)
| return null; | ||
| } | ||
|
|
||
| return connectionHolder.remove(key); |
There was a problem hiding this comment.
key가 존재하지 않는 경우에는 예외를 발생시켜주는게 좋을지, null을 반환시키는게 좋을지 제이미는 어떻게 생각하시는지 궁금해요 🤓
There was a problem hiding this comment.
사실 큰 고민 없이 만들긴 했지만, 저는 굳이 해당 로직에서 예외를 발생시켜줘야 할까에 대한 의문이 있긴 합니다.
이 프레임워크를 사용할 사용자 입장에서 어떤 걸 원할지 감이 잘 안 오네요...
null을 통해 key가 없는 상황이라 그냥 remove 하지 않았다 정도의 의미가 전달되었지 않았을까 싶기도 하고요..!
이후 예외 처리 등에선 사용자가 하지 않을까 싶습니다.
그레이는 어떤 결론을 냈는지와 그 이유가 궁금합니다!
There was a problem hiding this comment.
개인적인 생각입니다 !!
TransactionSynchronizationManager의 필드를 강제로 초기화 시켜주는 경우에는(항상 null이 아님을 보장하므로) 예외를 발생시켜주는게 좋다는 생각이 들었고, 항상 null이 아님을 보장하지 않는 경우에는 제이미가 작성해주신 것 처럼 null을 반환하도록 할 것 같아요 !
| this.dataSource = dataSource; | ||
| } | ||
|
|
||
| public <T> T execute(final TransactionExecuteStrategy<T> executeStrategy) { |
There was a problem hiding this comment.
말씀해주신 org.h2.jdbc.JdbcSQLNonTransientException: The object is already closed [90007-220] 에 대한 코멘트는 여기에 남겨볼게요 ㅎㅎ
저도 미션을 진행하면서 이 예외를 마주쳤습니다 😂
원인을 찾아보니 connection이 필요한 순간에 close 되어 있는게 원인이었습니다 !
한 트랜잭션 내에서 다수의 메서드를 처리하는 과정에서
첫 번째 메서드가 connection 사용을 마친 후 커넥션을 종료하고 있는지 확인해보시면 좋을 것 같아요 !
트랜잭션이 활성화 된 상태이면 커넥션을 종료하지 않는 방법을 적용해볼 수도 있을 것 같네요!
There was a problem hiding this comment.
감사합니다 그레이..!
덕분에 좀 더 빨리 알아챌 수 있었습니다..!
이전 단계에서 구현했던 PreparedStatementExecutor에서 connection을 try-with-resource 방식을 통해 할당했기 때문이었습니다..!
이전 단계 코드라 고려하지 못한 점이 엄청난 삽질이 되었군요 😭
다만 이를 수정하는 과정에서 PreparedStatementExecutor와 TransactionExecutor가 모두 DataSourceUtil의 releaseConnection을 통해 커넥션을 할당을 해제하려고 한다면, 그레이가 말씀해 주신 대로 트랜잭션 활성화 여부를 신경 써야 되게 되었습니다.
이를 위해, releaseConnection에서 connection.getAutoCommit()을 통해 트랜잭션 활성화 여부를 확인해 주기로 했습니다.
또한, TransactionExecutor에서 커넥션 할당 해제를 하기 전 커넥션의 autoCommit을 true로 다시 변경시켜 주는 과정을 추가했습니다.
혹시 더 좋은 방법이 있다면 공유해 주시면 감사하겠습니다!
p.s. pr들을 구경하다 보니 connection holder를 만들어 진행해 주신 분들도 계시네요..!
그런데 아직 현제 코드보다의 장점에 대한 생각이 떠오르지 않아 일단은 쉬운 방식으로 진행했습니다.
There was a problem hiding this comment.
Connection의 autuCommit을 확인하는 방법을 사용할 수도 있군요 ㅎㅎ
저는 트랜잭션 매니저의 필드에 isTransactionActivce라는 필드를 두고 트랜잭션 시작 시 true, 종료 시 false로 설정하는 로직을 추가해 해당 에러를 해결했었습니다 ㅎㅎ
말씀해주신 부분과 일맥상통하는 부분인 것 같아요 !
커넥션 홀더는 저도 크게 필요성을 느끼지 못해서 구현하지 않았습니다...!!
필요하다고 느끼는 순간에 확장하는게 좋지 않을까 싶어요 😎
안녕하세요 그레이!
지난 2단계에서 좋은 리뷰들 감사합니다.
학습테스트와 함께 진행하다 보니 3단계 제출이 늦어졌네요...!
늦어진 제출이 그레이의 일정에 영향이 가지 않길 바랍니다 🥲
+) 연락드려 말씀드린대로 해당 pr에 4단계까지 함께 올립니다!
그런데 4단계를 수행하던 도중 아래와 같이 오류가 계속 발생하는 상황입니다.
그런데 어디서 문제가 발생하는 것인지 감이 오질 않네요... 😭
해당 문제는 계속 찾아볼 예정입니다..
혹시 찾게 된다면 코멘트로 달아둘 수 있도록 하겠습니다..!
이번 리뷰도 잘 부탁드립니다☺️