Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[JDBC 라이브러리 구현하기 - 4단계] 콩하나(최한빈) 미션 제출합니다. #560

Merged
merged 8 commits into from
Oct 11, 2023

Conversation

kong-hana01
Copy link

@kong-hana01 kong-hana01 commented Oct 9, 2023

안녕하세요 주드! 콩하나입니다.
마지막 미션이네용

원래 어제 다 마무리해서 제출까지 하려고 했으나,
어제 갑작스러운 인텔리제이 라이센스 만료 이슈로 인해...
갖가지 방법을 찾아보다가 결국 트라이얼 모드로 30일 무료 이용해서 하고 있네요.
아직 학교 졸업 안했는데 인증이 안된대요.. ㅠㅠ
깃허브로 인증받는 것도 위와 동일한 이슈로 안되고... 이클립스를 하려하니까 생산성도 떨어지고 빌드가 안돼서
돌고 돌아서 인텔리제이네요 ㅋㅋㅋㅋ

아무튼 그래도 오늘 무사히 미션 완료해서 제출합니다.
이번에도 특별히 집중했던 부분을 공유드릴게요.

집중했던 부분

~~ jdbcTemplate보단 Service의 트랜잭션 추상화~~...를 하려 했으나 어쩔 수 없이 JdbcTemplate의 수정... ㅠㅠ

원래는 다음과 같이 생각했었습니다.

커넥션을 직접 생성하는 대신, `DataSourceUtils`를 사용해 대신 커넥션을 생성해주는 기능을 만들다보니
JdbcTemplate 클래스도 수정을 할지 말지 고민이 많았습니다.
JdbcTemplate에서 `커넥션을 전달받는 메소드는 트랜잭션 관리 책임을 외부`에 맡기고 있지만,
`커넥션을 전달받지 않는 메소드는 트랜잭션 관리를 내부`에서 하고 있기 때문이죠.

그래서 이 부분을 통일해주는게 좋을까말까 고민하다가 일단 한번 해보자!라는 마음으로 JdbcTemplate을 수정했었는데요.
결국 다시 롤백하고, JdbcTemplate은 건들지 않기로 결정했습니다. 그 이유는
1. 4단계 미션은 트랜잭션 추상화에 목표를 둔다. 현재 JdbcTemplate을 관리하는 것은 미션의 목표에는 다소 동떨어져있다
2. 라이브러리는 ocp원칙을 잘 지켜야한다. 그리고 저는 이 원칙을 지키려고 현재의 코드 구조가 나왔다고 판단했습니다.
3. 외부에서 connection을 받지 않고, JdbcTemplate 내부에서 `DataSourceUtils`을 사용해 커넥션을 받는 구조가 좋은 구조인지 잘 모르겠다. 커넥션을 받는 해당 정적 유틸리티가 너무 다양한 곳에서 사용돼서 문제가 있다고 판단했습니다.

결국 이러한 이유들로 JdbcTemplate은 건들지 않고 Service의 트랜잭션 추상화에만 집중하기로 결정했습니다.
이 때문에 `service -> dao -> jdbcTemplate으로 connection이 연달아 전달`되고 있지만,
이렇게 전달되는 것을 끊기 위해 정적 유틸리티에 의존하는 것이 더 위험하다고 생각해 나쁘지 않다고 생각했어요.
이에 대해 주드의 의견도 궁금하네요 😄

하지만 결국 JdbcTemplate에서 connection을 전달받는 기능을 모두 제거했습니다.

  1. 2단계 미션을 진행하면서 dao로 connection을 전달해주는 것이 불가능해졌고,
  2. 다시 생각해보니 DataSourceUtils라는 헬퍼 클래스가 라이브러리에서 제공해주는 클래스인만큼 JdbcTemplate을 사용하는 방법으로 숙지해두면 괜찮겠다는 생각이 들었어요. 이를 보강하기 위해 스프링에서는 프록시를 만들어서 트랜잭션을 따로 관리해주고 있기도 하니까요.

그래도 외부에서 connection을 직접 해제하지 않으면 jdbcTemplate에서는 이를 해제할 수 있는 방법이 없다는게 아쉽네요.

참고하면 좋을 사항

DataSourceUtils.releaseConnection() 메소드 내에 TransactionSynchronizationManager.unbindResource() 추가

예시에서는 둘을 분리해서 사용해주고 있는데,
아무리 생각해도 둘을 하나로 묶어서 이를 사용하는 측에서는 DataSourceUtils만 사용하도록 좋아보여서 이렇게 구현했습니다.

왜 둘을 분리했을까요?
그냥 예시일 뿐인 걸까요?
주드의 생각이 궁금합니다.

Copy link

@kevstevie kevstevie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

안녕하세요 하나콩
굉장히 훌륭하게 구현해주셨네요
현재 file changed에서는 마땅히 달 코멘트가 없었어요
근데 하나 생각해볼만한 지점이 있기는 합니다

TxUserService에서 탬플릿 콜백 패턴을 사용한 부분을 @Transactional 어노테이션이라고 생각해볼게요.
커넥션을 release하는 로직이 해당 패턴에 있어서 @Transactional을 사용한다면 정상적으로 커넥션을 해제합니다.
하지만 만약 사용자가 @Transactional을 달지 않고 사용한다면 jdbcTemplate에서 커넥션을 생성하지만 닫거나 제거하는 로직이 없어서 커넥션이 계속 유지될 것 같아요.
해당 질문에 대한 답변만 해결되면 아마 다음이 마지막 리뷰가 될 것 같습니다!

Comment on lines 10 to 15
private static final ThreadLocal<Map<DataSource, Connection>> resources;

static {
resources = new ThreadLocal<>();
resources.set(new HashMap<>());
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
private static final ThreadLocal<Map<DataSource, Connection>> resources;
static {
resources = new ThreadLocal<>();
resources.set(new HashMap<>());
}
private static final ThreadLocal<Map<DataSource, Connection>> resources = ThreadLocal.withInitial(HashMap::new);

이런 메서드가 있더군요

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

와... 이거 static으로 초기화하면 ThreadLocal을 제대로 사용하지 못하는 거였더라고요.
이 방법을 사용해야지 제대로 ThreadLocal을 사용하는거네요!
감사합니다 👍

@kong-hana01
Copy link
Author

안녕하세요 주드! 콩하나입니다.
리뷰확인하고 보니 close를 제대로 하지 못하는 문제가 있긴하더라고요.
그래서 해당 문제 해결해서 다시 리뷰요청보냅니다!

Connection을 가져오는 작업PreparedStatement을 가져오는 작업 모두 ConnectionManager에게 위임했습니다.
ConnectionManager는 inner 클래스로 만들었고, AutoClosable을 구현해서 connection을 닫아주는 작업까지 진행했습니다.
테스트로 이를 확인했으니 테스트도 참고해주세요~

감사합니다!

Copy link

@kevstevie kevstevie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

안녕하세요 하나콩
jdbc 라이브러리 미션 수고하셨습니다
auto closeable 멋지네요 배워갑니다
더 리뷰하고 싶은데 레거시도 제가 리뷰해도 될까요?

@kevstevie kevstevie merged commit 21c7042 into woowacourse:kong-hana01 Oct 11, 2023
1 check failed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants