-
Notifications
You must be signed in to change notification settings - Fork 379
[4단계 - Transaction synchronization 적용하기] 가콩(최가빈) 미션 제출합니다. #1217
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
Merged
Merged
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
f31b90b
refactor: Transaction synchronization 적용
gabean13 c083f02
refactor: 트랜잭션 서비스 추상화하기
gabean13 ba4a957
refactor: findById를 userDao.findById로 변경
gabean13 f6ddb9a
refactor: 기존 트랜잭션 처리 및 테스트 수정
gabean13 ab0d617
refactor: 파라미터 바인딩 순서 변경
gabean13 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or 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
This file contains hidden or 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
35 changes: 35 additions & 0 deletions
35
app/src/main/java/com/techcourse/service/AppUserService.java
This file contains hidden or 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
| 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 save(User user) { | ||
| userDao.insert(user); | ||
| } | ||
|
|
||
| @Override | ||
| public void changePassword(long id, String newPassword, String createdBy) { | ||
| final var user = userDao.findById(id); | ||
| user.changePassword(newPassword); | ||
| userDao.update(user); | ||
| userHistoryDao.log(new UserHistory(user, createdBy)); | ||
| } | ||
| } |
64 changes: 64 additions & 0 deletions
64
app/src/main/java/com/techcourse/service/TxUserService.java
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,64 @@ | ||
| package com.techcourse.service; | ||
|
|
||
| import com.interface21.dao.DataAccessException; | ||
| import com.interface21.jdbc.datasource.DataSourceUtils; | ||
| import com.interface21.transaction.support.TransactionSynchronizationManager; | ||
| import com.techcourse.config.DataSourceConfig; | ||
| import com.techcourse.domain.User; | ||
| import java.sql.Connection; | ||
| import java.sql.SQLException; | ||
| import javax.sql.DataSource; | ||
|
|
||
| public class TxUserService implements UserService { | ||
|
|
||
| private static final DataSource dataSource = DataSourceConfig.getInstance(); | ||
| private final UserService userService; | ||
|
|
||
| public TxUserService(UserService userService) { | ||
| this.userService = userService; | ||
| } | ||
|
|
||
| @Override | ||
| public User findById(long id) { | ||
| return userService.findById(id); | ||
| } | ||
|
|
||
| @Override | ||
| public void save(User user) { | ||
| userService.save(user); | ||
| } | ||
|
|
||
| @Override | ||
| public void changePassword(long id, String newPassword, String createdBy) { | ||
| Connection connection = null; | ||
| boolean existingTx = TransactionSynchronizationManager.hasResource(dataSource); | ||
| try { | ||
| if (!existingTx) { | ||
| connection = DataSourceUtils.getConnection(dataSource); | ||
| connection.setAutoCommit(false); | ||
| TransactionSynchronizationManager.bindResource(dataSource, connection); | ||
| } else { | ||
| connection = DataSourceUtils.getConnection(dataSource); | ||
| } | ||
|
Comment on lines
+36
to
+42
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. 🛠️ Refactor suggestion | 🟠 Major
규칙 2에서는 🤖 Prompt for AI Agents |
||
| try { | ||
| userService.changePassword(id, newPassword, createdBy); | ||
| } catch (Exception ex) { | ||
| if (!existingTx) { | ||
| connection.rollback(); | ||
| } | ||
| throw ex; | ||
| } | ||
| if (!existingTx) { | ||
| connection.commit(); | ||
| } | ||
| } catch (SQLException ex) { | ||
| throw new DataAccessException(ex); | ||
| } finally { | ||
| Connection bound = null; | ||
| if (!existingTx && TransactionSynchronizationManager.hasResource(dataSource)) { | ||
| bound = TransactionSynchronizationManager.unbindResource(dataSource); | ||
| } | ||
| DataSourceUtils.releaseConnection(bound != null ? bound : connection, dataSource); | ||
| } | ||
| } | ||
| } | ||
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,58 +1,10 @@ | ||
| package com.techcourse.service; | ||
|
|
||
| import com.interface21.dao.DataAccessException; | ||
| 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 java.sql.Connection; | ||
| import java.sql.SQLException; | ||
| import javax.sql.DataSource; | ||
|
|
||
| public class UserService { | ||
| public interface UserService { | ||
|
|
||
| private static final DataSource dataSource = DataSourceConfig.getInstance(); | ||
|
|
||
| private final UserDao userDao; | ||
| private final UserHistoryDao userHistoryDao; | ||
|
|
||
| public UserService(final UserDao userDao, final UserHistoryDao userHistoryDao) { | ||
| this.userDao = userDao; | ||
| this.userHistoryDao = userHistoryDao; | ||
| } | ||
|
|
||
| public User findById(final long id) { | ||
| try (final Connection connection = dataSource.getConnection()) { | ||
| return userDao.findById(connection, id); | ||
| } catch (SQLException ex) { | ||
| throw new DataAccessException(ex); | ||
| } | ||
| } | ||
|
|
||
| public void insert(final User user) { | ||
| try (final Connection connection = dataSource.getConnection()) { | ||
| userDao.insert(connection, user); | ||
| } catch (SQLException ex) { | ||
| throw new DataAccessException(ex); | ||
| } | ||
| } | ||
|
|
||
| public void changePassword(final long id, final String newPassword, final String createBy) { | ||
| try (final Connection connection = dataSource.getConnection()) { | ||
| connection.setAutoCommit(false); | ||
| try { | ||
| final var user = findById(id); | ||
| user.changePassword(newPassword); | ||
| userDao.update(connection, user); | ||
| userHistoryDao.log(connection, new UserHistory(user, createBy)); | ||
| } catch (Exception ex) { | ||
| connection.rollback(); | ||
| throw new DataAccessException("비밀번호를 변경하는데 실패했습니다.", ex); | ||
| } | ||
| connection.commit(); | ||
| } catch (SQLException ex) { | ||
| throw new DataAccessException(ex); | ||
| } | ||
| } | ||
| User findById(final long id); | ||
| void save(final User user); | ||
| void changePassword(final long id, final String newPassword, final String createdBy); | ||
| } |
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -23,34 +23,34 @@ void setup() throws SQLException { | |
|
|
||
| userDao = new UserDao(new JdbcTemplate(dataSource)); | ||
| final var user = new User("gugu", "password", "[email protected]"); | ||
| userDao.insert(dataSource.getConnection(), user); | ||
| userDao.insert(user); | ||
| } | ||
|
|
||
| @AfterEach | ||
| void after() throws SQLException { | ||
| JdbcTemplate jdbcTemplate = new JdbcTemplate(dataSource); | ||
| jdbcTemplate.update(dataSource.getConnection(), "delete from users"); | ||
| jdbcTemplate.update("delete from users"); | ||
| } | ||
|
|
||
| @Test | ||
| void findAll() throws SQLException { | ||
| final var users = userDao.findAll(dataSource.getConnection()); | ||
| final var users = userDao.findAll(); | ||
|
|
||
| assertThat(users).isNotEmpty(); | ||
| } | ||
|
|
||
| @Test | ||
| void findById() throws SQLException { | ||
| User gugu = userDao.findByAccount(dataSource.getConnection(), "gugu"); | ||
| final var user = userDao.findById(dataSource.getConnection(), gugu.getId()); | ||
| User gugu = userDao.findByAccount("gugu"); | ||
| final var user = userDao.findById(gugu.getId()); | ||
|
|
||
| assertThat(user.getAccount()).isEqualTo("gugu"); | ||
| } | ||
|
|
||
| @Test | ||
| void findByAccount() throws SQLException { | ||
| final var account = "gugu"; | ||
| final var user = userDao.findByAccount(dataSource.getConnection(), account); | ||
| final var user = userDao.findByAccount(account); | ||
|
|
||
| assertThat(user.getAccount()).isEqualTo(account); | ||
| } | ||
|
|
@@ -59,22 +59,22 @@ void findByAccount() throws SQLException { | |
| void insert() throws SQLException { | ||
| final var account = "insert-gugu"; | ||
| final var user = new User(account, "password", "[email protected]"); | ||
| userDao.insert(dataSource.getConnection(), user); | ||
| userDao.insert(user); | ||
|
|
||
| final var actual = userDao.findById(dataSource.getConnection(), 2L); | ||
| final var actual = userDao.findByAccount(account); | ||
|
|
||
| assertThat(actual.getAccount()).isEqualTo(account); | ||
| } | ||
|
|
||
| @Test | ||
| void update() throws SQLException { | ||
| final var newPassword = "password99"; | ||
| final var user = userDao.findByAccount(dataSource.getConnection(), "gugu"); | ||
| final var user = userDao.findByAccount("gugu"); | ||
| user.changePassword(newPassword); | ||
|
|
||
| userDao.update(dataSource.getConnection(), user); | ||
| userDao.update(user); | ||
|
|
||
| final var actual = userDao.findById(dataSource.getConnection(), 1L); | ||
| final var actual = userDao.findById(user.getId()); | ||
|
|
||
| assertThat(actual.getPassword()).isEqualTo(newPassword); | ||
| } | ||
|
|
||
This file contains hidden or 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
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,6 +12,7 @@ | |
| import com.techcourse.support.jdbc.init.DatabasePopulatorUtils; | ||
| import java.sql.SQLException; | ||
| import javax.sql.DataSource; | ||
| import org.junit.jupiter.api.AfterEach; | ||
| import org.junit.jupiter.api.BeforeEach; | ||
| import org.junit.jupiter.api.Test; | ||
|
|
||
|
|
@@ -28,19 +29,26 @@ void setUp() throws SQLException { | |
|
|
||
| DatabasePopulatorUtils.execute(DataSourceConfig.getInstance()); | ||
| final var user = new User("gugu", "password", "[email protected]"); | ||
| userDao.insert(dataSource.getConnection(), user); | ||
| userDao.insert(user); | ||
| } | ||
|
|
||
| @AfterEach | ||
| void after() throws SQLException { | ||
| JdbcTemplate jdbcTemplate = new JdbcTemplate(dataSource); | ||
| jdbcTemplate.update("delete from users"); | ||
| } | ||
|
|
||
| @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"; | ||
| userService.changePassword(1L, newPassword, createBy); | ||
| User user = userDao.findByAccount(createBy); | ||
| userService.changePassword(user.getId(), newPassword, createBy); | ||
|
|
||
| final var actual = userService.findById(1L); | ||
| final var actual = userService.findById(user.getId()); | ||
|
|
||
| assertThat(actual.getPassword()).isEqualTo(newPassword); | ||
| } | ||
|
|
@@ -49,16 +57,20 @@ 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 var newPassword = "newPassword"; | ||
| final var createBy = "gugu"; | ||
| final var createdBy = "gugu"; | ||
| // 트랜잭션이 정상 동작하는지 확인하기 위해 의도적으로 MockUserHistoryDao에서 예외를 발생시킨다. | ||
| assertThrows(DataAccessException.class, | ||
| () -> userService.changePassword(1L, newPassword, createBy)); | ||
| () -> userService.changePassword(1L, newPassword, createdBy)); | ||
|
|
||
| final var actual = userService.findById(1L); | ||
| final var actual = userDao.findByAccount("gugu"); | ||
|
|
||
| assertThat(actual.getPassword()).isNotEqualTo(newPassword); | ||
| } | ||
|
|
||
| } | ||
Oops, something went wrong.
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.
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.
🛠️ Refactor suggestion | 🟠 Major
객체에게 묻지 말고 시키기 (Tell, Don't Ask)
31번 라인에서
user객체에 여러 번 getter를 호출하여 데이터를 꺼내고 있습니다. 이는 규칙 9: 게터/세터/프로퍼티 금지와 관련이 있습니다.📖 왜 이 원칙이 중요한가요?
💡 생각해볼 질문: