Step1 지하철역 인수 테스트 작성
미션은 총 4주차까지 있고 지하철 시스템이란 도메인을 주제로 점진적으로 발전시켜 나가면서 ATDD와 TDD를 배운다.
1주차 Step1은 RestAssured를 사용하여 지하철역 목록 조회와 삭제 API 총 2개의 테스트 코드를 작성하는 미션이다.
구현하는데 크게 어려움은 없었다.
assertThat(stationsNames).hasSize(1)
.containsOnly("강남역");지하철역의 리스트를 검증하는 부분에서
containsOnly를 사용했다.
아래와 같은 피드백을 받았다.
💡 containsExactly() 메서드를 활용해 보셔도 좋을 것 같아요
containsOnly는 순서와 중복을 고려하지 않고 명시된 값이 반드시 포함하고 있는지를 검사한다.
containsExactly와의 차이는 순서와 중복을 고려하느냐이다.
검증은 정확하게 하는게 좋다. 요구사항이 언제 바뀔지 모르기 때문이다
요구사항에 정렬조건이 추가 되었을때 containsOnly로 검증했다고 쳐보자.
테스트 코드가 성공 하여 배포를 했는데 프로덕션에서 정렬 조건 구현을 놓쳤다면 곧 이슈로 연결된다.
containsExactly로 검증했다면 테스트코드가 깨지기 때문에 배포전 이슈를 캐치할 수 있다.
지하철역 생성 API를 총 두번 날렸고 그에 따른 response의 변수 네이밍을 생각없이 작성하였다.
// given
Map<String, String> params2 = new HashMap<>();
params2.put("name", "선릉역");
ExtractableResponse<Response> createResponse2 = createStation(params2);
assertThat(createResponse2.statusCode()).isEqualTo(HttpStatus.CREATED.value());
아래와 같은 피드백을 받았다.
💡 변수명 var2, var3 등 넘버링은 의미를 파악하기 어려워 권장하지 않는 방법인데요
넘버링 변수명을 사용하지 않도록 리팩토링을 하면 어떨까요?
가독성 좋은 코드를 작성하려고 항상 노력해야 한다.
넘버링 변수명을 작성했다는 건 아직까지 습관화 되어 있는것으로 생각되어 조금 부끄러웠다... 😅
변수명이 길더라도 의미있는 네이밍으로 지어야 겠다는 다짐을 했다.
위의 같은 경우는 seollEungStationCreateResponse 같이 앞부분에 어떤 지하철역을 생성하고 받은 response인지 구분하여 네이밍을 하였다.
Step2 지하철 노선 관리
Step2 미션은 노선을 CRUD하는 기능을 구현하는 거였다.
이번 단계에선 추가로 프로그래밍 요구사항이 있었고 다음과 같다.
1. 아래의 순서로 기능 구현
1. 인수 조건을 검증하는 인수 테스트 작성
2. 인수 테스트를 충족하는 기능 구현
2. 테스트 격리
- 인수 테스트의 결과가 다른 인수 테스트에 영향을 끼치지 않도록 인수 테스트를 서로 격리
3. 테스트 리팩터링
- 인수 테스트의 재사용성과 가독성, 그리고 빠른 테스트 의도 파악을 위해 인수 테스트를 리팩터링
인수 조건의 예시는 다음과 같다.
지하철 노선 조회
Given 지하철 노선을 생성하고
When 지하철 노선을 생성하면
Then 지하철 노선 목록 조회 시 생성한 노선을 찾을 수 있다
위와 같이 Given, When, Then으로 인수 조건을 작성한 다음, 이를 기반으로 테스트 코드를 작성한다.
/**
* 지하철역: 강남역, 선릉역, 양재역
* <p>
* When 지하철 노선을 생성하면
* Then 지하철 노선 목록 조회 시 생성한 노선을 찾을 수 있다
*/
@DisplayName("지하철 노선을 생성한다.")
@Test
void createLine() {
//..
}
테스트 격리를 하려면 테스트 마다 생성된 찌꺼기 데이터를 삭제해야만 했다.
@DirtiesContext, @Sql 와 같은 방법도 있었지만 테이블을 조회해서 각 테이블을 truncate 하는 방법을 선택했다.
※ @DirtiesContext와 @Sql을 선택하지 않은 이유
- @DirtiesContext: 오래 걸리는 단점
- @Sql: 스크립트를 매번 관리해줘야 하는 단점
// Guava
implementation("com.google.guava:guava:33.0.0-jre")
대문자를 소문자로 바꿔주는 CaseFormat을 사용하기 위해 guava를 의존성에 추가하였다.
@Entity
public class Line {
@Id
@GeneratedValue(strategy = GenerationType.IDENTITY)
private Long id;엔티티는 위와 같이 정의한 다음
@Component
public class DatabaseCleaner implements InitializingBean {
@PersistenceContext
private EntityManager entityManager;
private List<String> tableNames;
@Override
public void afterPropertiesSet() throws Exception {
this.tableNames = entityManager.getMetamodel()
.getEntities().stream()
.filter(e -> e.getJavaType().getAnnotation(Entity.class) != null)
.map(e -> CaseFormat.UPPER_CAMEL.to(CaseFormat.LOWER_UNDERSCORE, e.getName()))
.collect(Collectors.toList());
}
@Transactional
public void clear() {
entityManager.flush();
entityManager.createNativeQuery("SET REFERENTIAL_INTEGRITY FALSE").executeUpdate();
for (String tableName : tableNames) {
entityManager.createNativeQuery("TRUNCATE TABLE " + tableName).executeUpdate();
entityManager
.createNativeQuery("ALTER TABLE " + tableName + " ALTER COLUMN " + "id RESTART WITH 1")
.executeUpdate();
}
entityManager.createNativeQuery("SET REFERENTIAL_INTEGRITY TRUE").executeUpdate();
}
}
DatabaseCleaner는 위와 같이 구현 하였다.
과정은 아래와 같다.
1. DatabaseCleaner 빈 생성 후 afterPropertiesSet를 통해 @Entity가 달린 클래스의 엔티티 명을 조회
2. clear() 메서드 호출
3. 외래키 제약조건 비활성화
4. 모든 테이블 삭제
5. 각 테이블의 Auto Increment 1로 초기화
6. 외래키 제약조건 활성화
개인 프로젝트에서는 테이블명과 필드명을 마음대로 지어도 상관없기 때문에 쉽게 적용할 수 있었다.
만약 테이블명이 엔티티 이름과 다르고 PK 필드명도 단순 id가 아니면 어떻게 할 수 있을까?
@Entity
@Table(name = "kwc_team")
@NoArgsConstructor(access = AccessLevel.PROTECTED)
public class Team extends BaseEntity {
@Id
@GeneratedValue(strategy = GenerationType.IDENTITY)
@Column(name = "team_idx")
private Long id;
디비의 테이블명과 PK 값을 맞추기 위해 @Table과 @Column으로 각각 명시하였다.
똑같은 동작을 기대하려면 DatabaseCleaner를 아래와 같이 구현해야 한다.
@Component
public class DatabaseCleaner implements InitializingBean {
@PersistenceContext
private EntityManager entityManager;
private List<String> tableNames;
private Map<String, String> tablePrimaryKey;
private final static List<String> INIT_KEY_EXCLUDE_TABLE = List.of("exclude_table");
@Override
public void afterPropertiesSet() {
this.tableNames = entityManager.getMetamodel().getEntities().stream()
.filter(e -> e.getJavaType().getAnnotation(Entity.class) != null)
.map(e -> getTableName(e.getJavaType()))
.collect(Collectors.toList());
this.tablePrimaryKey = new HashMap<>();
entityManager.getMetamodel().getEntities().forEach(entityType -> {
String tableName = getTableName(entityType.getJavaType());
Stream.of(entityType.getJavaType().getDeclaredFields())
.filter(field -> field.isAnnotationPresent(Id.class))
.findFirst()
.ifPresent(field -> {
Column columnAnnotation = field.getAnnotation(Column.class);
String columnName = columnAnnotation != null ? columnAnnotation.name() : field.getName();
tablePrimaryKey.put(tableName, columnName);
});
});
}
private String getTableName(Class<?> entityClass) {
Table table = entityClass.getAnnotation(Table.class);
if (table != null && !table.name().isEmpty()) {
return table.name();
} else {
return CaseFormat.UPPER_CAMEL.to(CaseFormat.LOWER_UNDERSCORE, entityClass.getName());
}
}
@Transactional
public void clear() {
entityManager.flush();
entityManager.createNativeQuery("SET REFERENTIAL_INTEGRITY FALSE").executeUpdate();
clearTable();
initPublicKey();
entityManager.createNativeQuery("SET REFERENTIAL_INTEGRITY TRUE").executeUpdate();
}
private void clearTable() {
tableNames.forEach(tableName ->
entityManager.createNativeQuery("TRUNCATE TABLE " + tableName).executeUpdate());
}
private void initPublicKey() {
tablePrimaryKey.entrySet().stream()
.filter((entry) -> !INIT_KEY_EXCLUDE_TABLE.contains(entry.getKey()))
.forEach(entry -> entityManager.createNativeQuery("ALTER TABLE " + entry.getKey() + " ALTER COLUMN " + entry.getValue() + " RESTART WITH 1").executeUpdate());
}
}
@Table 어노테이션에 명시된 테이블명을 가져온다.
테이블을 truncate하는데 사용된다.
PK값을 얻기 위해 @Entity가 달린 클래스마다 돌면서 @Table에 명시된 테이블명과 @Id가 붙은 필드의 @Column에 명시된 이름을 가져와 Map에 넣어준다.
Map을 이용한 이유는 PK 초기화시 테이블명과 PK의 필드값을 같이 적어줘야 했기 때문이다.
PK가 Auto Increment가 아닌 테이블은 상수로 명시하여 PK 초기화 로직에서 필터링을 걸어 제외시켜 주면 된다.
3번째의 테스트 리팩토링은 http method 단위로 공통 메서드를 분리하였다.
protected ExtractableResponse<Response> get(String path, int statusCode, Object... pathParams) {
return RestAssured.given().log().all()
.contentType(MediaType.APPLICATION_JSON_VALUE)
.when().get(path, pathParams)
.then().log().all()
.statusCode(statusCode)
.extract();
}
protected ExtractableResponse<Response> post(String path, Object body, int statusCode, Object... pathParams) {
return RestAssured.given().log().all()
.body(body)
.contentType(MediaType.APPLICATION_JSON_VALUE)
.when().post(path, pathParams)
.then().log().all()
.statusCode(statusCode)
.extract();
}
protected ExtractableResponse<Response> put(String path, Object body, int statusCode, Object... pathParams) {
//...
}
protected ExtractableResponse<Response> delete(String path, int statusCode, Map<String, ?> queryParams, Object... pathParams) {
//...
}이 글을 적는 지금에야 모든 미션이 끝나서 하는 소리지만.. 중간부터 위와 같은 방법을 사용하지 않았다.
API에 path값, parameter값, header값 등 필요한 값이 추가될때 마다 해당 메서드에 추가를 해줘야 했다. Path가 필요없는 곳에서는 디폴트 값을 명시해줘야 해서 모든 코드에 영향이 갔다.
중간부턴 Rest Assured 문법이 중복이더라도 API 마다 분리하는게 가독성과 변경 여파를 최소화 할 수 있었다.
public class MemberSteps {
public static ExtractableResponse<Response> 회원_생성_요청(String email, String password, Integer age) {
Map<String, String> params = new HashMap<>();
params.put("email", email);
params.put("password", password);
params.put("age", age + "");
return RestAssured
.given().log().all()
.contentType(MediaType.APPLICATION_JSON_VALUE)
.body(params)
.when().post("/members")
.then().log().all().extract();
}
public static ExtractableResponse<Response> 회원_정보_조회_요청(ExtractableResponse<Response> response) {
String uri = response.header("Location");
return RestAssured.given().log().all()
.accept(MediaType.APPLICATION_JSON_VALUE)
.when().get(uri)
.then().log().all()
.extract();
}
//...
ReponseDto에 정적 팩토리 메서드를 적극 활용하는 편이다.
public static List<LineResponse> listOf(List<Line> lines, List<Station> stations) {
//...
}
public static LineResponse of(Line line, List<Station> stations) {
//...
}
private static List<Station> containStationWithLine(Line line, List<Station> stations) {
//...
}
아래와 같은 피드백을 받았다.
💡 정적 팩토리 메서드를 적극 활용하시네요 👍
위 메서드들은 생성자 다음에 위치하면 어떨까요?
static 필드는 클래스의 구성요소 중 가장 위에 선언하는 것은 알고 있었다.
하지만 static 메서드는 신경쓰지 않고 있었다..!!
리뷰 받은대로 생성자 위로 위치를 옮겼다.
매 테스트 코드마다 statusCode를 확인하는 검증문이 들어가 있었다.
// when
ExtractableResponse<Response> shinbundangDeleteResponse = deleteLine(createLineResponse.getId());
assertThat(shinbundangDeleteResponse.statusCode()).isEqualTo(HttpStatus.NO_CONTENT.value());
아래와 같은 피드백을 받았다.
💡 given, when 에서 status code를 검증하는 코드가 반복되고 있는데요
인수 테스트는 인수 조건에 집중하기 위해 status code 는 restassured 에서 검증하면 어떨까요?
확실히 statusCode는 부차적인 검증이라 테스트 코드의 검증문엔 인수 조건을 검증하는게 가독성에 좋을 것 같다는 생각이 들었다. statusCode 검증은 아래와 같이 Rest Assured에서 검증을 하였다.
return RestAssured.given().log().all()
.body(body)
.contentType(MediaType.APPLICATION_JSON_VALUE)
.when().put(path, pathParams)
.then().log().all()
.statusCode(statusCode)
.extract();
참고로 리뷰어님께 한가지 여쭤본게 있다.
질문은 아래와 같다.
리뷰어님은 ResponseDto 생성 및 반환을 서비스에서 하시나요? 아니면 컨트롤러까지는 도메인을 반환 후 해당 도메인을 가지고 ResponseDto를 생성하시나요?
ResponseDto는 화면이랑 관련있는 데이터다보니 프레젠테이션 영역에서 부터 만드는게 역할상 맞을까? 라는 고민이 순간 들었습니다. 그래서 라인 목록 조회를 아래와 같이 작성해보려 했습니다.
아래 2번에서와 같이 프레젠테이션 영역에 비즈니스 로직이 들어가는게 역할상 맞지 않다고 들어서 접긴 했습니다.. 아래와 같은 예시로는 제가 로직을 구현하지 못해서 그런걸까요? ㅠ
결론은 Service에서 ResponseDto를 생성해서 건네주는것보단 컨트롤러에 도메인을 반환해줘서 프레제테이션 영역에서 Dto를 만들자 vs 서비스에서 ResponseDto를 생성해서 바로 반환해줘도 아무 문제 없다둘중 어떤걸 선호하시는지 궁금합니다...!
// 라인 목록 조회 기능
@GetMapping("/lines")
public ResponseEntity<List<LineResponse>> showLines() {
List<Line> lines = lineService.findLines(); // 1. 라인 조회
List<Station> stations = stationService.findStationsBy(lines.stream().map(line -> line.getId()).collect(Collectors.toList()));
// 2. 라인이 가지고 있는 stationId 추출 후 역 정보 조회
return ResponseEntity.ok().body(lineResponses.listOf(lines, stations)); // 3. 조회한 라인과 라인이 가지고 있는 역 정보를 가지고 Response 생성
}
답변은 아래와 같다.
💡 StationService는 질문 주신 내용과 관계가 있어 보여 여기 답변 드리겠습니다
저는 서비스에서 ResponseDto를 생성해서 바로 반환해줘도 괜찮다 쪽입니다
우철님이 고민하신 것처럼 프레젠테이션 영역에서 비즈니스 로직을 처리하는 것도 역할에 맞지 않기도 하고
편의상(?) 컨트롤러에서 도메인을 사용하길 허용하다보면 코드 관리가 쉽지 않게 되는 것이 현실적으로 더 힘들더라구요 😅
위와 같은 게시글을 포스팅한 적이 있는데 리뷰어님도 Service에서 ResponseDto를 생성해서 반환하는 식으로 처리를 하시는 것 같다.
RequestDto와 ResponseDto의 패키지 위치와 관련해서 최근에 코딩 스타일을 변경한 적이 있다.
평소와 같았으면 controller 패키지 밑에 dto 패키지를 생성하여 하위에 Request와 Response를 두었다.
그리고 Request를 바로 Service에 메서드에 넘겨준다.
@Service
public class LineService {
@Transactional
public LineResponse saveLine(LineCreateRequest request) {
결론부터 말하면 Service에서 Controller를 의존하고 있는 문제가 있다.
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;
import nextstep.subway.controller.dto.LineCreateRequest;
import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;
@Service
public class LineService {
@Transactional
public LineResponse saveLine(LineCreateRequest request) {
패키지 의존성은 단뱡항이여야 한다.
해당 클래스의 import문을 보면 패키지간의 의존 관계가 Service가 Controller 쪽을 바라보고 있다.
Service 계층에 dto를 별도로 정의하고 RequestDto -> ServiceRequestDto로 변환해서 Service에 넘겨줘도 된다.
하지만 너무 번거롭고 배보다 배꼽이 커지는 상황이 된다.
controller에서 받는 request를 service에 즉시 넘기는 상황이 많다면 service 패키지 하위에 두면 의존성 문제가 해결된다.
import nextstep.subway.service.dto.LineCreateRequest;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;
@Service
public class LineService {
@Transactional
public LineResponse saveLine(LineCreateRequest request) {
Step3 지하철역 인수 테스트 작성
이번 미션부터는 노선에 구간을 생성하고 제거하는 요구사항이 추가되었다.
이전 미션에 비해서는 난이도가 올라갔다.
지하철을 보면 하나의 라인이 있고 3개의 지하철 역이 있으면
A - B 구간
B - C 구간
이렇게 지하철 사이의 공간을 구간이라고 한다.
구간과 노선을 별도의 개념으로 생각해 SectionController, LineController를 생성하였다.
아래와 같은 피드백을 받았다.
💡 노선관리와 동일하게 /lines/~로 시작하는데요
구간 관리의 기능과 노선 관리의 기능의 관계를 생각해보면 LineController를 활용해도 좋을 것 같은데 어떻게 생각하시나요?
SectionController의 메서드들을 LineController로 옮기라는 조언이셨다.
왜냐하면 구간은 노선에 포함되는 관계로 볼 수 있기 때문이다.
LineController로 옮겼다. 하지만 구간과 관련된 로직의 시작점인 SectionService는 Fat Service를 지양하기 위해 그대로 유지 하였다.
public boolean isSameId(Long id) {
return this.id.equals(id);
}
도메인에 정의한 메서드와 관련되서도 아래와 같은 피드백을 받았다.
💡 getter 메서드들 보다 상단에 위치해도 좋을 것 같아요 😃
getter나 setter같은 메서드들은 중요하지 않기 때문에 제일 하단에 위치 시켰다.
리뷰어님한테 OneToMany와 일급 컬렉션에 관련해서 질문을 남겼다.
💡OneToMany과 일급 컬렉션 관계
실무에서 OneToMany 필드들과 일급 컬렉션을 엮어서 사용 하시나요? 보통 OneToMany가 달린 필드들을 일급 컬렉션으로 래핑해서 쓰는 것 같더라구요.
개인적으론 OneToMany 사용하다가 몇번 당한적이 있어서(알수 없는 쿼리가 나간다던지, 또 알 수 없는 쿼리가 나간다 던지...) 사용하지 않고 있습니다.
그래서 별도로 조회 후 일급 컬렉션을 생성하고 있습니다.위 방법의 차이는 JPA의 연관 관계를 적극 활용할 것이냐 vs 아니냐 에 따라 다른 것 같긴 한데.. 리뷰어님은 어떻게 사용하시는지 궁금합니다 ㅎㅎ
나는 OneToMany를 사용할법한 상황에서도 사용하지 않는다. 신경써야 할께 많고 로직을 잘못 짜면 알수 없는 쿼리가 생성되어 디버깅 하기도 쉽지 않았다.
물론, 연관관계를 사용하지 않으면 반쪽자리 JPA라고 말할 수도 있다. 연관 관계가 맺어져 있으면 연결된 객체 그래프로
개발을 편하게 할 수 있기 때문이다.
하지만 편의성 메서드나 N + 1과 같이 고려해야할 상황이 많다. JPA는 쿼리만 생성해주는 것 그 이상 그 이하도 아니라고 생각한다. 하지만 A 객체 조회시 연관된 B 객체의 정보가 자주 필요하거나 하나의 도메인으로 볼 수 있을때 종종 @ManyToOne을 사용한다.
리뷰어님한테는 아래와 같은 답변을 받았다.
💡 JPA의 연관 관계를 적극 활용할 것이냐 vs 아니냐에 대한 경험이 있으셔서 현재 구조로 설계해주셨다고 생각이 드는데요!
적절한 근거가 있고 설득이 가능하다면 어느 방법이든 괜찮은 것 같아요. 이론적으로 권장하는 방향과 팀의 사정상 이론과 맞지 않는 부분은 얼마든지 변경해서 적용할 수 있다 생각합니다 😃
개인적으로 구간이 노선에 종속된 관계라면 노선에 메시지를 보내는 방식을 선호합니다 😃
final Line = repository.findById(...);
final Section section = new Section(...);
line.addSection(section);
일급 컬렉션 역할을 하는 객체들은 별도의 스프링 빈으로 만들지 않았다.
Line line = findBy(lineId);
Sections sections = findBy(line);
Stations stations = new Stations(stationRepository.findByIdIn(request.stationIds()));그 이유가 궁금해서 리뷰어님한테 여쭤 봤다.
💡 도메인 객체 혹은 일급 컬렉션 같은 녀석들은 스프링 빈으로 등록 하지 않고 new 키워드로 생성해서 해도 괜찮을까요?
저는 스프링 프레임워크에 엮이지 않는 순수 객체로 사용하는게 테스트 코드 작성하기도 편할 것 같은 생각이 듭니다.
하지만, 싱글톤으로 관리하지 않으면 메모리를 잡아먹을 수도 있겠다? 란 생각이 조금 들어서 어떻게 사용하시는지 궁금합니다!
아래와 같은 답변을 받았다.
💡 스프링 빈은 쓰레드 세이프하지 않기 때문에 도메인 객체를 스프링 빈으로 등록하는 것은 안티패턴이라고 생각해요
역 목록을 조회가 빈번해서 성능에 이슈가 발생한다면 스프링 빈이 아닌 캐싱을 고려하는 것은 어떨까요?
'외부활동 > ATDD, 클린 코드 with Spring 8기' 카테고리의 다른 글
| ATDD, 클린 코드 with Spring 8기 종료 (모든 미션 수료) (0) | 2024.03.06 |
|---|---|
| 인수 테스트와 TDD 회고 (0) | 2024.03.05 |
| ATDD, 클린 코드 with Spring 8기 시작! (2) | 2024.01.23 |
댓글