-
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 라이브러리 구현하기 - 1단계] 헙크(정현승) 미션 제출합니다. #294
Changes from 6 commits
4d32d5f
7bf90f8
b7063c3
875b5b4
9faa5c6
c89e572
db5f5fd
2cd6c77
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 |
---|---|---|
|
@@ -5,16 +5,22 @@ | |
import com.techcourse.support.jdbc.init.DatabasePopulatorUtils; | ||
import org.junit.jupiter.api.BeforeEach; | ||
import org.junit.jupiter.api.Test; | ||
import org.springframework.jdbc.core.JdbcTemplate; | ||
import org.springframework.jdbc.core.RowMapper; | ||
|
||
import java.util.List; | ||
|
||
import static org.assertj.core.api.Assertions.assertThat; | ||
|
||
class UserDaoTest { | ||
|
||
private UserDao userDao; | ||
private JdbcTemplate template; | ||
|
||
@BeforeEach | ||
void setup() { | ||
DatabasePopulatorUtils.execute(DataSourceConfig.getInstance()); | ||
template = new JdbcTemplate(DataSourceConfig.getInstance()); | ||
|
||
userDao = new UserDao(DataSourceConfig.getInstance()); | ||
final var user = new User("gugu", "password", "[email protected]"); | ||
|
@@ -23,22 +29,25 @@ void setup() { | |
|
||
@Test | ||
void findAll() { | ||
final var users = userDao.findAll(); | ||
final String sql = "SELECT id, account, password, email FROM users"; | ||
final List<User> users = template.query(sql, userRowMapper()); | ||
|
||
assertThat(users).isNotEmpty(); | ||
} | ||
|
||
@Test | ||
void findById() { | ||
final var user = userDao.findById(1L); | ||
final String sql = "SELECT id, account, password, email FROM users WHERE id = ?"; | ||
final User user = template.queryForObject(sql, userRowMapper(), 1L).orElseThrow(); | ||
|
||
assertThat(user.getAccount()).isEqualTo("gugu"); | ||
} | ||
|
||
@Test | ||
void findByAccount() { | ||
final var account = "gugu"; | ||
final var user = userDao.findByAccount(account); | ||
final String sql = "SELECT id, account, password, email FROM users WHERE account = ?"; | ||
final User user = template.queryForObject(sql, userRowMapper(), account).orElseThrow(); | ||
|
||
assertThat(user.getAccount()).isEqualTo(account); | ||
} | ||
|
@@ -47,7 +56,8 @@ void findByAccount() { | |
void insert() { | ||
final var account = "insert-gugu"; | ||
final var user = new User(account, "password", "[email protected]"); | ||
userDao.insert(user); | ||
final String sql1 = "INSERT INTO users(account, password, email) VALUES(?, ?, ?)"; | ||
final int updatedRow = template.update(sql1, user.getAccount(), user.getPassword(), user.getEmail()); | ||
|
||
final var actual = userDao.findById(2L); | ||
|
||
|
@@ -60,10 +70,18 @@ void update() { | |
final var user = userDao.findById(1L); | ||
user.changePassword(newPassword); | ||
|
||
userDao.update(user); | ||
|
||
final String sql2 = "UPDATE users SET password = ? WHERE id = ?"; | ||
template.update(sql2, user.getPassword(), user.getId()); | ||
final var actual = userDao.findById(1L); | ||
|
||
assertThat(actual.getPassword()).isEqualTo(newPassword); | ||
} | ||
|
||
private RowMapper<User> userRowMapper() { | ||
return rs -> new User( | ||
rs.getString("account"), | ||
rs.getString("password"), | ||
rs.getString("email") | ||
); | ||
} | ||
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. 이 부분도 UserDao 클래스에 들어있으면 더 좋을 것 같아요. 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,17 +1,65 @@ | ||
package org.springframework.jdbc.core; | ||
|
||
import org.slf4j.Logger; | ||
import org.slf4j.LoggerFactory; | ||
import org.springframework.dao.DataAccessException; | ||
|
||
import javax.sql.DataSource; | ||
import java.sql.Connection; | ||
import java.sql.PreparedStatement; | ||
import java.sql.ResultSet; | ||
import java.sql.SQLException; | ||
import java.util.ArrayList; | ||
import java.util.List; | ||
import java.util.Optional; | ||
|
||
public class JdbcTemplate { | ||
|
||
private static final Logger log = LoggerFactory.getLogger(JdbcTemplate.class); | ||
|
||
private final DataSource dataSource; | ||
|
||
public JdbcTemplate(final DataSource dataSource) { | ||
this.dataSource = dataSource; | ||
} | ||
|
||
public <T> Optional<T> queryForObject(final String sql, final RowMapper<T> rowMapper, Object... args) { | ||
try (final Connection conn = dataSource.getConnection(); | ||
final PreparedStatement pstmt = conn.prepareStatement(sql)) { | ||
for (int i = 0; i < args.length; i++) { | ||
pstmt.setObject(i + 1, args[i]); | ||
} | ||
try (final ResultSet rs = pstmt.executeQuery()) { | ||
if (rs.next()) { | ||
return Optional.ofNullable(rowMapper.mapRow(rs)); | ||
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. nullable 좋네요👍👍 |
||
} | ||
return Optional.empty(); | ||
} | ||
} catch (final SQLException e) { | ||
throw new DataAccessException(e); | ||
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. DataAccessException으로 바꿔 던지는거 좋습니다👍 |
||
} | ||
} | ||
|
||
public <T> List<T> query(String sql, RowMapper<T> rowMapper) throws DataAccessException { | ||
try (final Connection conn = dataSource.getConnection(); | ||
final PreparedStatement pstmt = conn.prepareStatement(sql)) { | ||
final List<T> result = new ArrayList<>(); | ||
try (final ResultSet rs = pstmt.executeQuery()) { | ||
if (rs.next()) { | ||
result.add(rowMapper.mapRow(rs)); | ||
} | ||
return result; | ||
} | ||
|
||
} catch (final SQLException e) { | ||
throw new DataAccessException(e); | ||
} | ||
} | ||
|
||
public int update(final String sql, final Object... args) { | ||
try (final Connection conn = dataSource.getConnection(); | ||
final PreparedStatement pstmt = conn.prepareStatement(sql)) { | ||
for (int i = 0; i < args.length; i++) { | ||
pstmt.setObject(i + 1, args[i]); | ||
} | ||
Comment on lines
+57
to
+59
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. 좋습니다! 확실히 가독성이 떨어지긴 하죠! |
||
return pstmt.executeUpdate(); | ||
} catch (final SQLException e) { | ||
throw new DataAccessException(e); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
package org.springframework.jdbc.core; | ||
|
||
import javax.annotation.Nullable; | ||
import java.sql.ResultSet; | ||
import java.sql.SQLException; | ||
|
||
@FunctionalInterface | ||
public interface RowMapper<T> { | ||
|
||
@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. 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. 실제 실제로 저도 어쨌든 저도 따라 붙이긴 했는데, 다른 개발자들에게 해당 메서드가 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. 반환값이 null 이 될 수도 있다는 뜻이었군요! 배워갑니다~😄 |
||
T mapRow(ResultSet rs) throws SQLException; | ||
|
||
} |
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.
전체적으로 테스트가 jdbcTemplate으로 진헹되었는데여요. UserDaoTest라는 클래스와는 맞지 않는거 같아요. userDao를 사용하는 방향으로 변경해보면 어떨까요?
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.
그렇네요! userDao의 존재를 전혀 알아차리지 못했네요.. ㅎㅎㅎ 바로 dao의 로직들을 JdbcTemplate으로 이동시켜야 한다고 생각했던 것 같습니다. 현재는 JdbcTemplate에서는 라이브러리처럼 CRUD에 대한 로직을 남겨두었고, Dao는 app 패키지에 있는 만큼, 데이터에 접근하기 위한 클래스이면서 비즈니스 로직을 포함하도록 sql 문이라든지, 상황에 맞는 파라미터 값을 넘겨주도록 하였습니다. UserDaoTest는 롤백시켜두었습니다! 감사합니다 :)