[JDBC 라이브러리 구현하기 - 3,4단계] 도치(김동흠) 미션 제출합니다. #529
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
깊이 고민하고 리뷰해 주셔서 감사합니다! |
Jaeyoung22
reviewed
Oct 11, 2023
| } | ||
|
|
||
| @Override | ||
| public User findById(long id) { |
There was a problem hiding this comment.
findById랑 insert에는 트랜잭션을 적용하지 않은 이유가 있을까요?
| @@ -1,6 +1,6 @@ | |||
| create table if not exists users ( | |||
| id bigint auto_increment, | |||
| account varchar(100) not null, | |||
| account varchar(100) not null unique, | |||
| if (resources.get() == null) { | ||
| return null; | ||
| } | ||
| return resources.get().get(key); |
There was a problem hiding this comment.
key에 맞는 값이 없을 경우 null을 반환하는 데요, 명시적으로 getOrdefault로 수정하는 게 좋을 것 같습니다.
| public static void close(DataSource dataSource, Connection connection) { | ||
| try { | ||
| DataSourceUtils.releaseConnection(connection, dataSource); | ||
| TransactionSynchronizationManager.unbindResource(dataSource); |
There was a problem hiding this comment.
그럴리는 없겠지만... unbind된 connection이 파라미터로 들어온 connection이랑 다른 경우는 어떻게 되나요?
There was a problem hiding this comment.
unbind할 때는 스레드로컬로부터 같은 dataSource를 통해 connection을 가져오기에 그럴 일이 없을 것 같기는 하네요! 흠.. 그래도 안전장치를 마련하자면 connection을 가져온 후 닫혔는지 확인 후 unbind하는 게 더 좋을 것 같아 그렇게 수정하겠습니다
Jaeyoung22
approved these changes
Oct 12, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
안녕하세요 리오!
3,4단계 같이 리뷰요청 드립니다.
말했던 account관련한 것은 unique제약을 걸고, 테스트에서는 deleteAll로 초기화를 해주고, insert후 해당 user 조회를 위해 updateAndReturnKey를 구현해 id를 반환받는 식으로 해결해보았어요