Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
20 changes: 10 additions & 10 deletions app/src/main/java/com/techcourse/dao/UserDao.java
Original file line number Diff line number Diff line change
Expand Up @@ -26,28 +26,28 @@ public UserDao(final JdbcTemplate jdbcTemplate) {
this.jdbcTemplate = jdbcTemplate;
}

public void insert(final Connection conn, final User user) {
public void insert(final User user) {
final var sql = "insert into users (account, password, email) values (?, ?, ?)";
jdbcTemplate.update(conn, sql, user.getAccount(), user.getPassword(), user.getEmail());
jdbcTemplate.update(sql, user.getAccount(), user.getPassword(), user.getEmail());
}
Comment on lines +29 to 32
Copy link

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: 게터/세터/프로퍼티 금지와 관련이 있습니다.

📖 왜 이 원칙이 중요한가요?

  • 객체의 내부 구조가 외부로 노출됩니다
  • 객체의 자율성이 떨어지고 진정한 캡슐화가 깨집니다
  • 변경에 취약한 코드가 됩니다 (User의 필드가 바뀌면 DAO도 수정해야 함)

💡 생각해볼 질문:

  • User 객체가 자신의 데이터를 직접 제공하는 메서드를 가지도록 하면 어떨까요?
  • "UserDao가 User에게 무엇을 달라고 요청하는 대신, User가 스스로 필요한 것을 제공하게" 하려면 어떻게 설계할 수 있을까요?


public void update(final Connection conn, final User user) {
public void update(final User user) {
final var sql = "update users set account = ?, password = ?, email = ? where id = ?";
jdbcTemplate.update(conn, sql, user.getAccount(), user.getPassword(), user.getEmail(), user.getId());
jdbcTemplate.update(sql, user.getAccount(), user.getPassword(), user.getEmail(), user.getId());
}

public List<User> findAll(final Connection conn) {
public List<User> findAll() {
final var sql = "select id, account, password, email from users";
return jdbcTemplate.query(conn, sql, userMapper);
return jdbcTemplate.query(sql, userMapper);
}

public User findById(final Connection conn, final Long id) {
public User findById(final Long id) {
final var sql = "select id, account, password, email from users where id = ?";
return jdbcTemplate.queryForObject(conn, sql, userMapper, id);
return jdbcTemplate.queryForObject(sql, userMapper, id);
}

public User findByAccount(final Connection conn, final String account) {
public User findByAccount(final String account) {
final var sql = "select id, account, password, email from users where account = ?";
return jdbcTemplate.queryForObject(conn, sql, userMapper, account);
return jdbcTemplate.queryForObject(sql, userMapper, account);
}
}
5 changes: 2 additions & 3 deletions app/src/main/java/com/techcourse/dao/UserHistoryDao.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

import com.interface21.jdbc.core.JdbcTemplate;
import com.techcourse.domain.UserHistory;
import java.sql.Connection;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand All @@ -16,11 +15,11 @@ public UserHistoryDao(final JdbcTemplate jdbcTemplate) {
this.jdbcTemplate = jdbcTemplate;
}

public void log(final Connection conn, final UserHistory userHistory) {
public void log(final UserHistory userHistory) {
final var sql = "insert into user_history (user_id, account, password, email, created_at, created_by) values (?, ?, ?, ?, ?, ?)";

log.debug("query : {}", sql);
jdbcTemplate.update(conn, sql, userHistory.getUserId(), userHistory.getAccount(), userHistory.getPassword(),
jdbcTemplate.update(sql, userHistory.getUserId(), userHistory.getAccount(), userHistory.getPassword(),
userHistory.getEmail(), userHistory.getCreatedAt(), userHistory.getCreateBy());

}
Expand Down
35 changes: 35 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,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 app/src/main/java/com/techcourse/service/TxUserService.java
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
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

else 예약어 사용이 객체지향 생활 체조 규칙을 위반하고 있습니다.

규칙 2에서는 else를 제거해 분기 흐름을 단순화하도록 요구합니다. 지금은 if (!existingTx) { ... } else { ... }로 분기가 깊어져 가독성이 떨어집니다. 가드 클로즈를 두거나, 이미 존재하는 트랜잭션을 먼저 처리해 조기 반환하는 식으로 재구성하면 어떨까요? 이런 방식이 책임을 더 명확하게 보여 주고, 복잡도를 줄여줍니다.

🤖 Prompt for AI Agents
In app/src/main/java/com/techcourse/service/TxUserService.java around lines 36
to 42, the current if(!existingTx){ ... } else { ... } uses an else that
violates the OOP gymnastics rule; refactor to remove the else by handling the
existing transaction case first (guard clause): if existingTx, obtain the
connection and proceed without binding or changing auto-commit (early path), and
then return or continue so the remaining code can assume no existing transaction
and perform connection = DataSourceUtils.getConnection(dataSource);
connection.setAutoCommit(false);
TransactionSynchronizationManager.bindResource(dataSource, connection); — ensure
the binding and setAutoCommit only occur in the non-existing-transaction path
and do not use else blocks.

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);
}
}
}
56 changes: 4 additions & 52 deletions app/src/main/java/com/techcourse/service/UserService.java
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);
}
22 changes: 11 additions & 11 deletions app/src/test/java/com/techcourse/dao/UserDaoTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand All @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ public MockUserHistoryDao(final JdbcTemplate jdbcTemplate) {
}

@Override
public void log(final Connection conn, final UserHistory userHistory) {
public void log(final UserHistory userHistory) {
throw new DataAccessException();
}
}
28 changes: 20 additions & 8 deletions app/src/test/java/com/techcourse/service/UserServiceTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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);
}
Expand All @@ -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);
}

}
Loading