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단계] 민트(유재민) 미션 제출합니다. #458

Merged
merged 21 commits into from
Oct 12, 2023
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
23f8a48
패키지 위치 변경 및 코드 정리
kang-hyungu Sep 21, 2023
fd3b51e
docs: 3단계 3차 리뷰 내용 정리
yujamint Oct 4, 2023
161d0af
chore: EOF 해결
yujamint Oct 4, 2023
9ce3a83
test: 트랜잭션 학습 테스트
yujamint Oct 5, 2023
696a787
refactor: TransactionContext 삭제 및 TransactionSynchronizationManager 적용
yujamint Oct 5, 2023
93261f4
refactor: DataSourceUtils 적용
yujamint Oct 5, 2023
94d40f9
refactor: UserService 추상화
yujamint Oct 5, 2023
055eb1b
refactor: TransactionCallback의 반환 타입을 제네릭으로 수정
yujamint Oct 5, 2023
34ffba0
feat: Nullable 어노테이션 적용
yujamint Oct 5, 2023
3c484ec
docs: 4단계 리뷰 내용 정리
yujamint Oct 6, 2023
e205b8f
style: 불필요한 공백 제거
yujamint Oct 6, 2023
df7e776
refactor: TxUserService의 TransactionTemplate 생성자 주입받도록 수정
yujamint Oct 6, 2023
b33cfd1
feat(TransactionTemplate): 반환값이 없는 메서드 구현
yujamint Oct 6, 2023
36fbd06
refactor: 중복되는 resources.get() 제거
yujamint Oct 6, 2023
7e30cef
style: import 와일드카드 제거
yujamint Oct 6, 2023
3438d8d
refactor: ConnectionManager 제거 및 DataSourceUtils로 책임 이전
yujamint Oct 9, 2023
5ad575e
test: 학습 테스트 로깅 추가
yujamint Oct 9, 2023
13f7d03
refactor: 불필요한 어노테이션 제거
yujamint Oct 9, 2023
d0e93ca
docs: 4단계 2차 리뷰 내용 정리
yujamint Oct 10, 2023
c11f4f4
refactor: TransactionCallback 제거 후 표준 함수형 인터페이스 사용
yujamint Oct 10, 2023
c2efb84
fix: autoCommit 모드에서 Connection이 닫히지 않는 문제 해결
yujamint Oct 12, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -46,4 +46,7 @@
- [x] 공유자원 Singleton vs static
- [x] JdbcTemplate 테스트 작성
- [x] JdbcTemplate의 executeQuery 중복 제거
- [x] TransactionTemplate catch 절 final
- [x] TransactionTemplate catch 절 final

### 3차
- [x] EOF 해결(TransactionTemplateTest, PreparedStatementCreatorTest)
4 changes: 4 additions & 0 deletions app/src/main/java/com/techcourse/dao/UserDao.java
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package com.techcourse.dao;

import com.techcourse.domain.User;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.jdbc.core.JdbcTemplate;
import org.springframework.jdbc.core.RowMapper;

Expand All @@ -18,6 +20,8 @@ public class UserDao {
return new User(id, account, password, email);
};

private static final Logger log = LoggerFactory.getLogger(UserDao.class);

private final JdbcTemplate jdbcTemplate;

public UserDao(final DataSource dataSource) {
Expand Down
34 changes: 34 additions & 0 deletions app/src/main/java/com/techcourse/service/AppUserService.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
package com.techcourse.service;

import com.techcourse.dao.UserDao;
import com.techcourse.dao.UserHistoryDao;
import com.techcourse.domain.User;
import com.techcourse.domain.UserHistory;

public class AppUserService implements UserService {

private final UserDao userDao;
private final UserHistoryDao userHistoryDao;

public AppUserService(final UserDao userDao, final UserHistoryDao userHistoryDao) {
this.userDao = userDao;
this.userHistoryDao = userHistoryDao;

Copy link
Member

Choose a reason for hiding this comment

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

R: 빈칸은 없어도 될 것 같습니다!

}

public User findById(final long id) {
return userDao.findById(id)
.orElseThrow(() -> new IllegalArgumentException("유저가 존재하지 않습니다."));
}

public void insert(final User user) {
userDao.insert(user);
}

public void changePassword(final long id, final String newPassword, final String createBy) {
final var user = findById(id);
user.changePassword(newPassword);
userDao.update(user);
userHistoryDao.log(new UserHistory(user, createBy));
}
}
40 changes: 40 additions & 0 deletions app/src/main/java/com/techcourse/service/TxUserService.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
package com.techcourse.service;

import com.techcourse.config.DataSourceConfig;
import com.techcourse.domain.User;
import org.springframework.transaction.support.TransactionTemplate;

import javax.sql.DataSource;

public class TxUserService implements UserService {

private final UserService userService;
private final TransactionTemplate transactionTemplate;

public TxUserService(final UserService userService) {
this.userService = userService;
final DataSource dataSource = DataSourceConfig.getInstance();
this.transactionTemplate = new TransactionTemplate(dataSource);
}
Copy link
Member

Choose a reason for hiding this comment

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

C: UserService와 TransactionTemplate을 받는 생성자로 만들어보는건 어떨까요?

Copy link
Member Author

Choose a reason for hiding this comment

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

외부에서 주입한 TransactionTemplate(DataSource)를 사용하는 것이 더 좋은 방법임에 동의합니다!


@Override
public User findById(final long id) {
return transactionTemplate.executeWithTransaction(() -> userService.findById(id));
Copy link
Member

Choose a reason for hiding this comment

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

크.. 나머지 메서드들도 트랜잭션 다 깔끔하게 적용해주셨네요 👍

}

@Override
public void insert(final User user) {
transactionTemplate.executeWithTransaction(() -> {
userService.insert(user);
return null;
});
Copy link
Member

Choose a reason for hiding this comment

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

R: 반환값이 없는 메서드를 따로 만들어보는건 어떨까요!?

방금 생각난건데 표준 함수형 인터페이스를 사용해보는 것도 괜찮을 것 같네요!

Copy link
Member Author

Choose a reason for hiding this comment

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

좋은 의견 감사합니다 ㅎㅎ
Runnable을 사용하는 JdbcTemplate.executeWithoutResult() 따로 만들었습니다!

}

@Override
public void changePassword(final long id, final String newPassword, final String createBy) {
transactionTemplate.executeWithTransaction(() -> {
userService.changePassword(id, newPassword, createBy);
return null;
});
}
}
40 changes: 4 additions & 36 deletions app/src/main/java/com/techcourse/service/UserService.java
Original file line number Diff line number Diff line change
@@ -1,42 +1,10 @@
package com.techcourse.service;

import com.techcourse.config.DataSourceConfig;
import com.techcourse.dao.UserDao;
import com.techcourse.dao.UserHistoryDao;
import com.techcourse.domain.User;
import com.techcourse.domain.UserHistory;
import org.springframework.transaction.support.TransactionTemplate;

import javax.sql.DataSource;
public interface UserService {

public class UserService {

private final UserDao userDao;
private final UserHistoryDao userHistoryDao;
private final TransactionTemplate transactionTemplate;

public UserService(final UserDao userDao, final UserHistoryDao userHistoryDao) {
this.userDao = userDao;
this.userHistoryDao = userHistoryDao;
final DataSource dataSource = DataSourceConfig.getInstance();
this.transactionTemplate = new TransactionTemplate(dataSource);
}

public User findById(final long id) {
return userDao.findById(id)
.orElseThrow(() -> new IllegalArgumentException("유저가 존재하지 않습니다."));
}

public void insert(final User user) {
userDao.insert(user);
}

public void changePassword(final long id, final String newPassword, final String createBy) {
transactionTemplate.executeWithTransaction(() -> {
final var user = findById(id);
user.changePassword(newPassword);
userDao.update(user);
userHistoryDao.log(new UserHistory(user, createBy));
});
}
User findById(final long id);
void insert(final User user);
void changePassword(final long id, final String newPassword, final String createBy);
}
7 changes: 5 additions & 2 deletions app/src/test/java/com/techcourse/service/UserServiceTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ void setUp() {
@Test
void testChangePassword() {
final var userHistoryDao = new UserHistoryDao(jdbcTemplate);
final var userService = new UserService(userDao, userHistoryDao);
final var userService = new AppUserService(userDao, userHistoryDao);

final var newPassword = "qqqqq";
final var createBy = "gugu";
Expand All @@ -47,7 +47,10 @@ void testChangePassword() {
void testTransactionRollback() {
// 트랜잭션 롤백 테스트를 위해 mock으로 교체
final var userHistoryDao = new MockUserHistoryDao(jdbcTemplate);
final var userService = new UserService(userDao, userHistoryDao);
// 애플리케이션 서비스
final var appUserService = new AppUserService(userDao, userHistoryDao);
// 트랜잭션 서비스 추상화
final var userService = new TxUserService(appUserService);

final User user = userDao.findById(1L).get();
final var oldPassword = user.getPassword();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.transaction.support.TransactionContext;
import org.springframework.jdbc.datasource.DataSourceUtils;
import org.springframework.transaction.support.TransactionSynchronizationManager;

import javax.sql.DataSource;
import java.sql.Connection;
Expand All @@ -19,10 +20,10 @@ public ConnectionManager(final DataSource dataSource) {
}

public Connection getConnection() {
if (TransactionContext.isEmpty()) {
return getNewConnection();
if (TransactionSynchronizationManager.hasResource(dataSource)) {
return TransactionSynchronizationManager.getResource(dataSource);
Copy link
Member

Choose a reason for hiding this comment

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

R: ConnectionManager이 잘 동작하지만 제가 이 부분에 대해서 깊게 생각해봤는데, 3단계에서 사용했던 ConnectionManager를 제거하고 새로운 방식으로 해보면 어떨까 싶어요. TransactionTemplate, ConnectionManager, JdbcTemplate을 보면 Connection을 가져오고 반환하고 트랜잭션 동기화를 설정하는 것이 4단계에서 제공한 클래스를 사용하도록 코드를 리팩터링 하며 적절하게 추상화되어있지 않다고 느껴졌어요. (각각 클래스의 코드를 보시면 느껴지실껍니다!)

만약 좋은 아이디어가 떠오르지 않으신다면 해당 부분에 대해서 스프링의 TransactionTemplate, PlatformTransactionManager, DataSourceTransactionManager, ConnectionHolder, TransactionSynchronizationManager, DataSourceUtils 코드를 한 번 보시면 어떻게 추상화할지 힌트를 얻을 수도 있을 것 같아요. (해당 클래스 구현 권장이 아니라 각각 클래스의 책임을 보면 미션에 도움이 될 것 같다고 생각했어요.)

저는 다음 순서대로 대략적인 흐름만 파악했어요! 더 많은 정보를 얻으면 코멘트 알려주세요!
TransactionTemplate --> PlatformTransactionManager --> DataSourceTransactionManager --> ConnectionHolder(+ ResourceHolderSupport) + DataSourceUtils --> TransactionSynchronizationManager

Copy link
Member Author

Choose a reason for hiding this comment

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

저도 허브의 의견에 동의합니다!
ConnectionManager를 제거하고, DataSource 관련 책임은 DataSourceUtils를 통해서만 수행하도록 수정했습니다.
혹시나 현재 흐름도 어색하다고 느껴진다면 말씀해 주세요!!

모든 클래스를 다 확인해보진 못했지만, 말씀해주신 흐름에 따라 각 클래스의 역할에 대해 공부할 수 있었습니다. 감사합니다 ☺️

}
return TransactionContext.get();
return getNewConnection();
}

private Connection getNewConnection() {
Expand All @@ -38,22 +39,14 @@ public void closeNotTransactional(Connection connection) {
if (isTransactional(connection)) {
return;
}
close(connection);
DataSourceUtils.releaseConnection(connection);
}

private boolean isTransactional(final Connection connection) {
if (TransactionContext.isEmpty()) {
return false;
if (TransactionSynchronizationManager.hasResource(dataSource)) {
return TransactionSynchronizationManager.getResource(dataSource) == connection;
}
return TransactionContext.get() == (connection);
return false;
}

private void close(final Connection connection) {
try {
connection.close();
} catch (SQLException e) {
log.error(e.getMessage(), e);
throw new RuntimeException(e);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,21 +13,20 @@ public abstract class DataSourceUtils {
private DataSourceUtils() {}

public static Connection getConnection(DataSource dataSource) throws CannotGetJdbcConnectionException {
Connection connection = TransactionSynchronizationManager.getResource(dataSource);
if (connection != null) {
return connection;
if (TransactionSynchronizationManager.hasResource(dataSource)) {
return TransactionSynchronizationManager.getResource(dataSource);
}

try {
connection = dataSource.getConnection();
Connection connection = dataSource.getConnection();
TransactionSynchronizationManager.bindResource(dataSource, connection);
return connection;
} catch (SQLException ex) {
throw new CannotGetJdbcConnectionException("Failed to obtain JDBC Connection", ex);
}
}

public static void releaseConnection(Connection connection, DataSource dataSource) {
public static void releaseConnection(Connection connection) {
try {
connection.close();
} catch (SQLException ex) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package org.springframework.transaction.support;

import javax.annotation.Nonnull;
import javax.annotation.meta.TypeQualifierNickname;
import javax.annotation.meta.When;
import java.lang.annotation.*;

@Target(ElementType.METHOD)
@Retention(RetentionPolicy.RUNTIME)
@Nonnull(when = When.MAYBE)
@TypeQualifierNickname
public @interface Nullable {
Copy link
Member

Choose a reason for hiding this comment

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

C: 해당 애너테이션을 붙이면 어떤 효과가 있나용?

추가로 import 수정해주시면 일관성 있는 코드가 될 것 같습니다!

Copy link
Member Author

Choose a reason for hiding this comment

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

으음.. 해당 어노테이션을 붙이지 않으면 경고가 떴던 것으로 기억하는데 다시 확인해 보니 어노테이션이 없어도 경고가 안 뜨네요 ㅎㅎ;; 지웠습니다!

}
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
package org.springframework.transaction.support;

@FunctionalInterface
public interface TransactionCallback {
public interface TransactionCallback<T> {

void execute();
@Nullable
T execute();
}

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -2,22 +2,28 @@

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<>();
private static final ThreadLocal<Map<DataSource, Connection>> resources = ThreadLocal.withInitial(HashMap::new);
Copy link
Member

Choose a reason for hiding this comment

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

초기값 설정 잘 배워갑니다 👍👍👍


private TransactionSynchronizationManager() {}

public static Connection getResource(DataSource key) {
return null;
return resources.get().get(key);
}

public static void bindResource(DataSource key, Connection value) {
resources.get().put(key, value);
}

public static Connection unbindResource(DataSource key) {
return null;
public static void unbindResource(DataSource key) {
resources.get().remove(key);
}

public static boolean hasResource(DataSource dataSource) {
return resources.get().containsKey(dataSource);
Copy link
Member

Choose a reason for hiding this comment

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

C: .get() 의 반복을 제거하는 것에 대한 민트의 생각은 어떠신가요?

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.exception.UndeclaredThrowableException;
import org.springframework.jdbc.datasource.DataSourceUtils;

import javax.sql.DataSource;
import java.sql.Connection;
Expand All @@ -18,14 +19,15 @@ public TransactionTemplate(final DataSource dataSource) {
this.dataSource = dataSource;
}

public void executeWithTransaction(final TransactionCallback transactionCallback) {
@Nullable
public <T> T executeWithTransaction(final TransactionCallback<T> transactionCallback) {
Copy link
Member

@greeng00se greeng00se Oct 9, 2023

Choose a reason for hiding this comment

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

두개 잘 나누어 주셨네용 👍👍👍 일관성 있게 Runnable 처럼 표준 함수형 인터페이스인 Supplier를 사용하는 방법도 있을 것 같아요 👍

잘못적어놨네용 ㅋㅋ

Connection connection = null;
try {
connection = dataSource.getConnection();
TransactionContext.set(connection);
connection = DataSourceUtils.getConnection(dataSource);
connection.setAutoCommit(false);
transactionCallback.execute();
final T result = transactionCallback.execute();
connection.commit();
return result;
} catch (final RuntimeException e) {
if (connection != null) {
rollback(connection);
Expand All @@ -37,13 +39,9 @@ public void executeWithTransaction(final TransactionCallback transactionCallback
}
throw new UndeclaredThrowableException(e);
} finally {
TransactionContext.remove();
try {
if (connection != null) {
connection.close();
}
} catch (final SQLException e) {
throw new RuntimeException(e);
TransactionSynchronizationManager.unbindResource(dataSource);
if (connection != null) {
DataSourceUtils.releaseConnection(connection);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,4 +45,4 @@ void createPreparedStatement() throws SQLException {
() -> assertThat(parameterMetaData.getParameterClassName(2)).contains("Integer")
);
}
}
}
Loading