-
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단계] 주노(최준호) 미션 제출합니다. #532
Changes from 6 commits
e84424a
35985c2
943a0ae
14d4336
305a6b4
481b254
0355d74
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,35 @@ | ||
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(UserDao userDao, UserHistoryDao userHistoryDao) { | ||
this.userDao = userDao; | ||
this.userHistoryDao = userHistoryDao; | ||
} | ||
|
||
@Override | ||
public User findById(long id) { | ||
return userDao.findById(id); | ||
} | ||
|
||
@Override | ||
public void insert(User user) { | ||
userDao.insert(user); | ||
} | ||
|
||
@Override | ||
public void changePassword(long id, String newPassword, 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,30 @@ | ||
package com.techcourse.service; | ||
|
||
import com.techcourse.domain.User; | ||
import org.springframework.transaction.TransactionExecutor; | ||
|
||
public class TxUserService implements UserService { | ||
|
||
private final UserService userService; | ||
private final TransactionExecutor transactionExecutor; | ||
|
||
public TxUserService(UserService userService, TransactionExecutor transactionExecutor) { | ||
this.userService = userService; | ||
this.transactionExecutor = transactionExecutor; | ||
} | ||
|
||
@Override | ||
public User findById(long id) { | ||
return transactionExecutor.executeWithResult(() -> userService.findById(id)); | ||
} | ||
|
||
@Override | ||
public void insert(User user) { | ||
transactionExecutor.execute(() -> userService.insert(user)); | ||
} | ||
|
||
@Override | ||
public void changePassword(final long id, final String newPassword, final String createBy) { | ||
transactionExecutor.execute(() -> userService.changePassword(id, newPassword, createBy)); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,38 +1,10 @@ | ||
package com.techcourse.service; | ||
|
||
import com.techcourse.dao.UserDao; | ||
import com.techcourse.dao.UserHistoryDao; | ||
import com.techcourse.domain.User; | ||
import com.techcourse.domain.UserHistory; | ||
import org.springframework.transaction.TransactionExecutor; | ||
import org.springframework.transaction.TransactionManager; | ||
|
||
public class UserService { | ||
public interface UserService { | ||
|
||
private final UserDao userDao; | ||
private final UserHistoryDao userHistoryDao; | ||
private final TransactionManager transactionManager; | ||
|
||
public UserService(final TransactionManager transactionManager, final UserDao userDao, final UserHistoryDao userHistoryDao) { | ||
this.userDao = userDao; | ||
this.userHistoryDao = userHistoryDao; | ||
this.transactionManager = transactionManager; | ||
} | ||
|
||
public User findById(final long id) { | ||
return userDao.findById(id); | ||
} | ||
|
||
public void insert(final User user) { | ||
userDao.insert(user); | ||
} | ||
|
||
public void changePassword(final long id, final String newPassword, final String createBy) { | ||
TransactionExecutor.execute(transactionManager, () -> { | ||
final var user = findById(id); | ||
user.changePassword(newPassword); | ||
userDao.update(transactionManager.getConnection(), user); | ||
userHistoryDao.log(transactionManager.getConnection(), 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 |
---|---|---|
|
@@ -9,6 +9,7 @@ | |
import org.junit.jupiter.api.Test; | ||
import org.springframework.dao.DataAccessException; | ||
import org.springframework.jdbc.core.JdbcTemplate; | ||
import org.springframework.transaction.TransactionExecutor; | ||
import org.springframework.transaction.TransactionManager; | ||
|
||
import static org.assertj.core.api.Assertions.assertThat; | ||
|
@@ -34,7 +35,7 @@ void setUp() { | |
@Test | ||
void testChangePassword() { | ||
final var userHistoryDao = new UserHistoryDao(jdbcTemplate); | ||
final var userService = new UserService(transactionManager, userDao, userHistoryDao); | ||
final var userService = new AppUserService(userDao, userHistoryDao); | ||
|
||
final var newPassword = "qqqqq"; | ||
final var createBy = "gugu"; | ||
|
@@ -47,13 +48,12 @@ void testChangePassword() { | |
|
||
@Test | ||
void testTransactionRollback() { | ||
// 트랜잭션 롤백 테스트를 위해 mock으로 교체 | ||
final var userHistoryDao = new MockUserHistoryDao(jdbcTemplate); | ||
final var userService = new UserService(transactionManager, userDao, userHistoryDao); | ||
final var appUserService = new AppUserService(userDao, userHistoryDao); | ||
final var userService = new TxUserService(appUserService, new TransactionExecutor(transactionManager)); | ||
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. 이 테스트를 돌려보시면 예외가 발생할 거에요! 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. JdbcTemplate의 Connection을 DatasourceUtils를 이용하여 가져오도록 수정했습니다. 이 과정에서 try-with-resource의 대상에서 제외시켰습니다. 추가로 Transaction 수행 과정에서 TransactionManager가 Connection을 필드로 가짐으로써 close된 Connection이 계속 남아있던 부분을 확인했습니다. 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.
확인했습니다. 이제
고생하셨습니다~! 이제 닫힌 커넥션을 갱신할 수 있게 되었네요! 하지만 이 방법은 임시방편이라는 생각이 들어요. 멀티 쓰레드 환경으로 가면, 정말 그렇게 되어도 괜찮을까요? 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. 와 멀티스레드 환경 생각도 못하고있었네요...!!! 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. 저는 TxUserService로 AppUserService를 감싸서 사용하는게 엄청 부자연스럽게 느껴졌어요. 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.
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. 아무래도 그런 것 같죠? ㅎㅎ |
||
|
||
final var newPassword = "newPassword"; | ||
final var createBy = "gugu"; | ||
// 트랜잭션이 정상 동작하는지 확인하기 위해 의도적으로 MockUserHistoryDao에서 예외를 발생시킨다. | ||
assertThrows(DataAccessException.class, | ||
() -> userService.changePassword(1L, newPassword, createBy)); | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,42 +1,56 @@ | ||
package org.springframework.transaction; | ||
|
||
import org.springframework.dao.DataAccessException; | ||
import org.springframework.jdbc.datasource.DataSourceUtils; | ||
|
||
import javax.sql.DataSource; | ||
import java.sql.Connection; | ||
import java.sql.SQLException; | ||
|
||
public class TransactionManager { | ||
|
||
private final Connection connection; | ||
private final DataSource dataSource; | ||
private Connection connection; | ||
|
||
public TransactionManager(DataSource dataSource) { | ||
this.dataSource = dataSource; | ||
this.connection = DataSourceUtils.getConnection(dataSource); | ||
} | ||
|
||
public void begin() { | ||
try { | ||
this.connection = dataSource.getConnection(); | ||
if (connection.isClosed()) { | ||
connection = DataSourceUtils.getConnection(dataSource); | ||
} | ||
connection.setAutoCommit(false); | ||
} catch (SQLException e) { | ||
throw new DataAccessException(e); | ||
} | ||
} | ||
|
||
public TransactionManager(Connection connection) { | ||
this.connection = connection; | ||
} | ||
|
||
public void begin() throws SQLException { | ||
connection.setAutoCommit(false); | ||
} | ||
|
||
public void commit() throws SQLException { | ||
connection.commit(); | ||
connection.close(); | ||
public void commit() { | ||
try { | ||
connection.commit(); | ||
this.close(); | ||
} catch (SQLException e) { | ||
throw new DataAccessException(e); | ||
} | ||
} | ||
|
||
public void rollback() throws SQLException { | ||
connection.rollback(); | ||
connection.close(); | ||
public void rollback() { | ||
try { | ||
connection.rollback(); | ||
this.close(); | ||
} catch (SQLException e) { | ||
throw new DataAccessException(e); | ||
} | ||
Comment on lines
+37
to
+43
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. 커밋할 때, 그리고 예외로 인한 롤백 시에도 트랜잭션 종료 및 커넥션 제거를 잘 해주고 있네요👍 하나의 쓰레드에서 트랜잭션을 여러 번 실행했을 때, 하지만 주노 코드는 커넥션까지 잘 제거해서 이런 사고를 잘 막을 수 있겠네요! |
||
} | ||
|
||
public Connection getConnection() { | ||
return connection; | ||
} | ||
|
||
public void close() { | ||
DataSourceUtils.releaseConnection(connection, dataSource); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,22 +2,38 @@ | |
|
||
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 TransactionSynchronizationManager() {} | ||
private TransactionSynchronizationManager() { | ||
} | ||
|
||
public static Connection getResource(DataSource key) { | ||
return null; | ||
Map<DataSource, Connection> connectionWithDataSource = resources.get(); | ||
if (connectionWithDataSource == null) { | ||
return null; | ||
} | ||
return connectionWithDataSource.getOrDefault(key, null); | ||
} | ||
|
||
public static void bindResource(DataSource key, Connection value) { | ||
Map<DataSource, Connection> connectionWithDataSource = resources.get(); | ||
if (connectionWithDataSource == null) { | ||
connectionWithDataSource = new HashMap<>(); | ||
resources.set(connectionWithDataSource); | ||
} | ||
Comment on lines
+24
to
+28
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. 잘 해주셨습니다! 👍👍👍💯💯💯 |
||
connectionWithDataSource.put(key, value); | ||
} | ||
|
||
public static Connection unbindResource(DataSource key) { | ||
return null; | ||
Map<DataSource, Connection> connectionWithDataSource = resources.get(); | ||
if (connectionWithDataSource == null) { | ||
return null; | ||
} | ||
return connectionWithDataSource.remove(key); | ||
} | ||
} |
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.
UserService
인터페이스로 받기 vsAppUserService
구현체로 받기전자의 경우, 인자로 TxUserService가 들어오면 transactionExectuor의 메서드들이 여러 번 호출되는 문제가 생길 수 있을 것 같아요.
자주 발생할 경우 같지는 않아서 '굳이 그렇게 할 필요가 있나?'라는 고민도 드는데, 주노라면 어떤 선택을 하실건가요?
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.
저는
굳이 그렇게 할 필요가 있나?
라는 입장으로 이정도는 개발자의 책임의 영역이라고 생각해도 무관할 것 같습니다 😅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.
저도 동의합니다.
라이브러리를 개발하는 입장에서
타입을 특정 구현체로 제한시켜 유연성을 희생하기보다는, 이 정도는 사용자 책임으로 남기는 것이 좋을 것 같아요.
어떤 사용자는 정말
TxUserService
를 중첩해서 만들고 싶을 수도 있죠!좋은 답변 감사합니다!