-
Notifications
You must be signed in to change notification settings - Fork 300
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
Changes from 9 commits
23f8a48
fd3b51e
161d0af
9ce3a83
696a787
93261f4
94d40f9
055eb1b
34ffba0
3c484ec
e205b8f
df7e776
b33cfd1
36fbd06
7e30cef
3438d8d
5ad575e
13f7d03
d0e93ca
c11f4f4
c2efb84
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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; | ||
|
||
} | ||
|
||
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)); | ||
} | ||
} |
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); | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. C: UserService와 TransactionTemplate을 받는 생성자로 만들어보는건 어떨까요? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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; | ||
}); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. R: 반환값이 없는 메서드를 따로 만들어보는건 어떨까요!? 방금 생각난건데 표준 함수형 인터페이스를 사용해보는 것도 괜찮을 것 같네요! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 좋은 의견 감사합니다 ㅎㅎ |
||
} | ||
|
||
@Override | ||
public void changePassword(final long id, final String newPassword, final String createBy) { | ||
transactionTemplate.executeWithTransaction(() -> { | ||
userService.changePassword(id, newPassword, createBy); | ||
return null; | ||
}); | ||
} | ||
} |
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); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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; | ||
|
@@ -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); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 코드를 한 번 보시면 어떻게 추상화할지 힌트를 얻을 수도 있을 것 같아요. (해당 클래스 구현 권장이 아니라 각각 클래스의 책임을 보면 미션에 도움이 될 것 같다고 생각했어요.) 저는 다음 순서대로 대략적인 흐름만 파악했어요! 더 많은 정보를 얻으면 코멘트 알려주세요! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 저도 허브의 의견에 동의합니다! 모든 클래스를 다 확인해보진 못했지만, 말씀해주신 흐름에 따라 각 클래스의 역할에 대해 공부할 수 있었습니다. 감사합니다 |
||
} | ||
return TransactionContext.get(); | ||
return getNewConnection(); | ||
} | ||
|
||
private Connection getNewConnection() { | ||
|
@@ -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 |
---|---|---|
@@ -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 { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. C: 해당 애너테이션을 붙이면 어떤 효과가 있나용? 추가로 import 수정해주시면 일관성 있는 코드가 될 것 같습니다! There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
---|---|---|
|
@@ -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); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. C: .get() 의 반복을 제거하는 것에 대한 민트의 생각은 어떠신가요? |
||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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; | ||
|
@@ -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) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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); | ||
|
@@ -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); | ||
} | ||
} | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
R: 빈칸은 없어도 될 것 같습니다!