E2e step2 #877

jaejeong1 · 2023-07-03T14:38:23Z

지하철 노선 인수 테스트 작성
지하철 노선 기능 구현
인수테스트 격리를 위해 EntityManager를 활용한 Truncate 기능 구현(AcceptanceTest.class)

인수 테스트 격리 방안 별 장단점

@transactional

트랜잭션 상태를 현재 스레드에 바인딩하게 되는데(ThreadLocal), 이 경우 해당 테스트 메서드의 모든 작업은 트랜잭션 내에서 호출되지 않고, 롤백도 되지않는다
결국 실제 DB에 커밋되게된다
공식 문서에서는 Timeout이 대표적인 새 스레드를 띄워 진행하는 작업이라 소개하고 있다
인수테스트 또한 @SpringBootTest 어노테이션에 port를 지정하여 서버를 띄우게 되는 데 이때, HTTP 클라이언트와 서버는 각각 다른 스레드에서 실행된다
따라서 아무리 테스트 코드에 @transactional 어노테이션이 있다고 하더라도 호출되는 쪽은 다른 스레드에서 새로운 트랜잭션으로 커밋하기 때문에 롤백이 정상적으로 되지 않는다

DirtiesConext

매 테스트마다 ApplicationContext를 초기화해 테스트 격리가 가능하다
하지만, Spring boot의 기본 전략은 ApplicationContext를 캐싱해 재활용하는 것, 무거운 ApplicationContext를 초기화 후 재시작하는 것은 방지하고자 한다(너무 느리기 때문에)
너무 느려서 테스트가 많아지면 사용하기 어렵다

@Sql

trancate를 사용해 직접 테이블 별 sql문을 작성해 DB 초기화
DDL이라 delete보다 빠르다
테이블이 추가될 때 마다 일일이 sql문을 고쳐줘야한다
sql문에 의존하게 되고, 관리 포인트가 많아서 조금 아쉽다

EntityManager

테이블 명을 불러와서 @Sql의 테이블 관리 단점을 상쇄
추상화해 모든 인수 테스트에 공통 적용 가능해 편리하고, 유지보수가 쉽고, 빠르다

hjhello423

안녕하세요. 재정님
일부 이번 미션에 요구사항에 맞지 않은 부분들이 있어서 request change 하도록 하겠습니다.
관련해서 코멘트를 달아두었으니 미션 내용을 다시 한번 확인해 보면 좋을 것 같아요!

hjhello423 · 2023-07-03T15:15:16Z

src/main/java/subway/line/LineController.java

+    @PostMapping("/lines")
+    public ResponseEntity<LineResponse> createLine(@RequestBody LineRequest lineRequest) {
+        LineResponse line = lineService.saveLine(lineRequest);
+        return ResponseEntity.created(URI.create("/lines/" + line.getId())).body(line);
+    }


미션 요구사항과 다른 스펙으로 구현되어 있어요. 😭
LineController의 다른 부분들도 미션에서 요구하는 스펙에 맞게 변경해 주세요!

가장 중요한 요구사항 구현에 미스가 있었군요.. 참고해서 수정해보겠습니다! 감사합니다 :)

hjhello423 · 2023-07-03T15:18:22Z

src/test/java/subway/DatabaseCleanup.java

+    @Transactional
+    public void execute() {
+        // 쓰기 지연 저장소에 남은 SQL을 마저 수행
+        entityManager.flush();
+        // 연관 관계 맵핑된 테이블이 있는 경우 참조 무결성을 해제해줘야 Trancate 가능
+        entityManager.createNativeQuery("SET REFERENTIAL_INTEGRITY FALSE").executeUpdate();


cleanup 잘 만들어 주셨네요. 👍

hjhello423 · 2023-07-03T15:19:38Z

src/main/java/subway/line/LineService.java

+    @Transactional
+    public LineResponse saveLine(LineRequest lineRequest) {
+        Line line = lineRepository.save(new Line(lineRequest.getName(), lineRequest.getColor()));
+        return createLineResponse(line);
+    }


API 스펙을 미션 요구사항에 맞게 변경하면 아마 LineService의 로직들이 조금 더 복잡해질 거예요.

Builder 패턴 적용해 Line 엔티티 생성 과정의 유연성을 확보했습니다!

hjhello423 · 2023-07-03T15:24:03Z

src/test/java/subway/LineAcceptanceTest.java

+    /**
+     * 지하철 노선 생성
+     * 노선 생성 시 상행종점역과 하행종점역을 등록
+     * When 지하철 노선을 생성하면
+     * Then 지하철 노선 목록 조회 시 생성한 노선을 찾을 수 있다
+     */
+    @DisplayName("지하철 노선을 생성한다.")
+    @Test
+    void 지하철_노선_생성_테스트() {
+        // when & then
+        지하철_노선_생성(LINE_SHINBUNDANG, COLOR_CODE_RED);
+
+        // then
+        List<String> lineNames = getJsonPathList(지하철_노선_목록_조회(), PARAM_NAME);
+        assertThat(lineNames).containsAnyOf(LINE_SHINBUNDANG);
+    }


시나리오와 테스트가 일치하지 않고 있어요. 😭

시나리오 요구사항에 맞게 테스트와 클라이언트 수정하였습니다!

hjhello423 · 2023-07-03T15:32:35Z

src/test/java/subway/LineAcceptanceTest.java

+    private List<String> getJsonPathList(ExtractableResponse<Response> response, String path) {
+        return response.jsonPath().getList(path, String.class);
+    }


응답을 조금 더 편하게 deserialize할 수도 있어요.
https://github.com/rest-assured/rest-assured/wiki/Usage#content-type-based-deserialization

더 나은 방안 제시해주셔서 감사합니다.
해당 건을 살펴보고 제네릭 메서드로 선언해 클래스 타입을 주입받고,
이 제네릭 메서드를 통해 LineResponse 클래스로 변환하는 방식으로 직접 적용해보았는데,

현재 케이스는 name 필드만 추출해 검사가 필요한 케이스로, 기존 대비 코드 양이 더 많아지는 결과가 나와 제시해주신 방안은 적용하지 않았습니다.

혹시 조언주신 의도가 제가 적용한 방법과 다르다면 관련해 피드백 주시면 감사하겠습니다!

jaejeong1

피드백 반영

hjhello423

안녕하세요 재정님 😀
다음 단계로 넘어가기 전에 확인해 보면 도움이 될 것 같은 코멘트를 몇 개 남겨두었습니다!
금방 해결 가능하실 것 같아요.
확인해 보시고 다시 한번 리뷰 요청 주세요!

hjhello423 · 2023-07-06T12:50:14Z

src/main/java/subway/line/LineService.java

+        Line line = lineRepository.save(Line.builder()
+                .name(lineRequest.getName())
+                .color(lineRequest.getColor())
+                .upStationId(lineRequest.getUpStationId())
+                .downStationId(lineRequest.getDownStationId())
+                .distance(lineRequest.getDistance())
+                .build());


request에서 받은 stationId를 그대로 Line에 주입하고 있는데요.
request에 등록되어 있지 않은 역이 들어오면 어떻게 될까요?
예를 들어 99999라는 id가 들어오면 어떻게 될까요?

현재는 upStation, downStation 관련 아무런 매핑이 없기 때문에, 등록되어 있지 않은 역이 들어와도 알 수 있는 방법이 없습니다.
request에서 전달받은 stationId를 기준으로 해당 Id를 가진 Station이 존재하는지 확인 후 존재하지 않는다면 Fail 처리하고,
존재한다면 해당 엔티티 객체를 Line 엔티티에 주입해준뒤 stationId를 기준으로 joinColumn 매핑을 해 외래키로 관리해야할 것 입니다.

hjhello423 · 2023-07-06T12:52:36Z

src/main/java/subway/line/dto/LineCreateRequest.java

+@Builder
+@RequiredArgsConstructor
+public class LineCreateRequest {


@Builder는 어떤 방식으로 사용하는 게 좋은지, 어떤 부분에서 조심해야 하는지 찾아보면 좋을 것 같아요.
찾아보신 내용을 공유해 주세요. 😀

@builder는 객체 생성을 캡슐화할때 사용하는 빌더 패턴을 lombok 어노테이션으로 쉽게 구현할 수 있게 해줍니다.

주의사항

클래스 위에 사용할 경우 모든 멤버 변수를 받는 기본 생성자를 만들게 되어 값을 지정하면 안되는 id 같은 변수까지 지정 범위에 포함하게 됩니다.

따라서, 지정 필요한 매개변수를 설정할 수 있는 별도 생성자를 만들고, 이 생성자 위에 @builder를 사용하여 위와 같은 문제를 방지해야합니다(의도한대로 객체가 생성될 수 있도록)

hjhello423 · 2023-07-06T12:53:57Z

src/main/java/subway/line/dto/LineResponse.java

+public class LineResponse {
+    private final Long id;
+    private final String name;
+    private final String color;
+    private final List<Station> stations;
+}


Station은 엔티티인데요.
response에 엔티티를 이용해 응답을 하면 어떤 문제들이 있을까요? 🤔

Entity는 실제 DB 테이블과 매핑되고, 따라서 이를 기준으로 테이블이 생성되고 스키마가 변경됩니다.
Entity 객체가 변경되면 DB 데이터 또한 변경되므로, Entity를 요청이나 응답으로 전달하면 안되고 이런 경우 DTO를 사용해야합니다.
또한, Entity를 그대로 응답으로 넘기게 되면 불필요하거나 노출되면 안되는 정보(id 등)까지 노출될 수 있고, 이를 막기 위한 로직을 따로 구현해야 합니다.
DTO는 레이어 간 데이터 전송용으로 사용되며 별도 비즈니스 로직을 포함하지 않은채, Getter와 (생성자 또는 Setter)를 제공해 데이터를 전달할 수 있도록 해줍니다.

hjhello423 · 2023-07-06T12:55:56Z

src/main/java/subway/station/Station.java

 @Entity
+@NoArgsConstructor
 public class Station {


엔티티는 왜 기본 생성자가 필요한가요?
사용되지 않는 생성자들은 어떻게 접근을 제어할 수 있을까요? 🤔

하이버네이트에서 런타임 프록시 객체 생성 시 필요합니다.
해당 케이스가 사용되는 대표적인 예시는 지연 로딩인데, 엔티티 생성 시 프록시 객체를 생성해 반환하고, 실제 엔티티에 접근이 필요할 때 영속성 컨텍스트에 DB 조회를 요청해 실제 엔티티 객체를 생성 후 프록시 객체에서 실제 엔티티 객체를 참조하도록 해 데이터를 가져오는 매커니즘입니다.

프록시 객체는 실제 엔티티 객체와 동일하게 사용할 수 있어야하기 때문에 실제 엔티티를 상속받아 만들어집니다.
이때 상속된 자식 클래스는 부모 클래스의 생성자를 super를 통해 호출해야하는데 기본 생성자가 없다면 실패합니다.
따라서, 기본 생성자는 필요하고, 접근제어자는 private이 아닌 public 또는 protected여야 합니다.

이 기본 생성자는 프록시 객체 생성을 위한 것이지, 실제 비즈니스 로직에서 사용할 용도가 아니니 public이 아닌 protected로 선언해 접근을 제어하면 될 것입니다.

hjhello423 · 2023-07-06T13:04:09Z

src/test/java/subway/LineAcceptanceTest.java

+    void 지하철_노선_생성_테스트() {
+        // When 지하철 노선을 생성하면
+        지하철_노선_생성(LINE_SHINBUNDANG, COLOR_CODE_RED, UPSTATION_ID, DOWNSTATION_ID, DISTANCE);


노선 생성을 하려면 역이 등록되어 있어야 하지 않을까요?

맞습니다. 역을 먼저 등록한 후 반환된 Id를 노선 생성 시 전달하도록 변경하겠습니다.

jaejeong1 added 4 commits July 3, 2023 14:54

인수 조건을 검증하는 인수 테스트 작성

7b248d8

기능 구현 및 인수 테스트 케이스 수정

0b0eaf9

e2e step2 구현

c82e0ab

지하철역 인수테스트에 있는 불필요한 지하철노선 관련 코드 삭제

464e7be

hjhello423 requested changes Jul 4, 2023

View reviewed changes

요구사항에 맞는 테스트 수정 및 클라이언트 코드 수정

519d446

jaejeong1 commented Jul 6, 2023

View reviewed changes

jaejeong1 requested a review from hjhello423 July 6, 2023 07:23

hjhello423 requested changes Jul 6, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

E2e step2 #877

E2e step2 #877

jaejeong1 commented Jul 3, 2023

hjhello423 left a comment

hjhello423 Jul 3, 2023

jaejeong1 Jul 6, 2023

hjhello423 Jul 3, 2023

hjhello423 Jul 3, 2023

jaejeong1 Jul 6, 2023

hjhello423 Jul 3, 2023

jaejeong1 Jul 6, 2023

hjhello423 Jul 3, 2023

jaejeong1 Jul 6, 2023

jaejeong1 left a comment

hjhello423 left a comment

hjhello423 Jul 6, 2023

jaejeong1 Jul 13, 2023

hjhello423 Jul 6, 2023

jaejeong1 Jul 28, 2023

hjhello423 Jul 6, 2023

jaejeong1 Jul 28, 2023

hjhello423 Jul 6, 2023

jaejeong1 Jul 13, 2023

hjhello423 Jul 6, 2023

jaejeong1 Jul 13, 2023

E2e step2 #877

Are you sure you want to change the base?

E2e step2 #877

Conversation

jaejeong1 commented Jul 3, 2023

hjhello423 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jaejeong1 left a comment

Choose a reason for hiding this comment

hjhello423 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment