[Spring 지하철 노선도 - 1,2단계] 연로그(권시연) 미션 제출합니다. #190

yeon-06 · 2022-05-04T05:48:08Z

안녕하세요 또링! 이번 미션 잘 부탁드립니다😆

1단계 기능 목록

지하철 역 관리 API 기능 완성
- 지하철 역 생성 시 이미 등록된 이름으로 요청한다면 에러 응답
지하철 노선 관리 API 구현
- 지하철 노선 등록
  - 등록된 이름으로 요청하면 에러 응답
- 지하철 노선 목록
- 지하철 노선 조회
  - 조회된 노선이 없는 경우 에러 응답
- 지하철 노선 수정
  - 중복된 이름으로 수정 시 에러 응답
- 지하철 노선 삭제
End to End 테스트 작성

2단계 기능 목록

데이터를 H2 DB에 저장하기
- H2 DB 설정하기
- 테이블 생성하기
Spring Bean & Spring JDBC 적용하기
- @Controller, @Service, @Repository 적용
- 예외 @ExceptionHandler로 처리
- JdbcTemplate는 NamedJdbcTemplate 사용

고민했던 포인트

1단계 요구사항 - 컴포넌트/빈 없이 의존 관계를 주입하기
- 첫번째 시도: dao의 데이터가 공유되어야 하므로 필드에 static 키워드 사용
  - 테스트 코드에서 BeforeEach를 통해 초기 데이터를 주입할 때 Exception 발생 (중복 값 생성이 불가능하므로)
  - 데이터를 비워주기 위해 테스트를 위한 메서드가 생겨버림
- 두번째 시도: @BeforeAll을 이용해 초기 데이터 주입
  - 포트 번호 불일치로 인한 에러 발생
  - @BeforeAll는 static 키워드가 필수
  - RestAssured의 포트 번호를 가져올 수 없음
entity vs domain
- 이번 단계까지는 entity와 domain을 분리할 필요가 없어서 분리하지 않았습니다! 다만 entity와 domain의 차이점에 대해서 잘 인지하지 못해서 나름 찾아보려고 노력했으나... DDD를 알아야한다 어쩌구 복잡한 설명들을 보고 더 헷갈리게 되었네요😵 또링이 추천하는 관련 책/자료 같은게 있으실까요...?🥺
produces
- Station에 전체 조회 시 produces = MediaType.APPLICATION_JSON_VALUE라는 옵션이 있었는데요 (직접 작성한 로직이 아닌 이미 만들어져있던 API임) 추가하나 하지않으나 JSON 형태로 return하는 것 같아 불필요한 옵션이라고 판단, 해당 옵션을 삭제했습니다.

- 중복되는 이름으로 노선 생성 - 존재하지 않는 노선 id 검색

jnsorn

안녕하세요, 연로그! 리뷰어 또링입니다. 🤗 만나서 반가워요!
1, 2단계 잘 구현해주신 것 같아요. 코드 보면서 깔끔하다는 느낌을 많이 받았습니다. 😃
README와 PR에 남겨주신 내용 덕분에 어떤 학습을 하셨는지 쉽게 파악할 수 있어서 좋았네요.
코멘트 몇 개 남겼는데 확인부탁드려요. 궁금하거나 얘기 나누고 싶은 부분은 언제든 편히 DM주시구요!

jnsorn · 2022-05-06T03:15:30Z

src/main/java/wooteco/subway/domain/Line.java

@@ -0,0 +1,40 @@
+package wooteco.subway.domain;


entity vs domain
이번 단계까지는 entity와 domain을 분리할 필요가 없어서 분리하지 않았습니다! 다만 entity와 domain의 차이점에 대해서 잘 인지하지 못해서 나름 찾아보려고 노력했으나... DDD를 알아야한다 어쩌구 복잡한 설명들을 보고 더 헷갈리게 되었네요😵 또링이 추천하는 관련 책/자료 같은게 있으실까요...?🥺

entity, domain, vo.. 다양한 용어들의 등장으로 혼란스러우실 것 같아요. 🤯 저는 entity와 domain이 명확하게 분리되는 개념이라고 생각하지 않아요. https://doing7.tistory.com/79 이 글을 읽어보시면 조금 더 감이 잡히실거예요. DDD라는 단어는 블러처리하고 봐주셔도 좋아요 :)

jnsorn · 2022-05-06T03:16:38Z

src/main/java/wooteco/subway/ui/StationController.java

        return ResponseEntity.created(URI.create("/stations/" + newStation.getId())).body(stationResponse);
    }

-    @GetMapping(value = "/stations", produces = MediaType.APPLICATION_JSON_VALUE)


Station에 전체 조회 시 produces = MediaType.APPLICATION_JSON_VALUE라는 옵션이 있었는데요 (직접 작성한 로직이 아닌 이미 만들어져있던 API임) 추가하나 하지않으나 JSON 형태로 return하는 것 같아 불필요한 옵션이라고 판단, 해당 옵션을 삭제했습니다.

넵 좋습니다!😃 produces, consumes를 추가하게되면 주고 받는 데이터 형식을 명시적으로 확인 및 필터링 할 수 있어서 이점이 있지만, 말씀하신대로 여기서 굳이 가지고 있을 필요는 없어보이네요! (참고로 전 파일과 같이 특이한 형태의 데이터를 주고 받을 때 붙여주고 있어요.)

jnsorn · 2022-05-06T05:13:37Z

src/main/java/wooteco/subway/domain/Line.java

+    public Line(String name, String color) {
+        this.name = name;
+        this.color = color;
+    }
+
+    public Line(Long id, String name, String color) {
+        this.id = id;
+        this.name = name;
+        this.color = color;
+    }


생성자에 중복된 코드가 보여요! this를 이용해 중복을 제거해볼까요?

jnsorn · 2022-05-06T05:14:07Z

src/main/java/wooteco/subway/domain/Station.java

+    public boolean matchId(Long id) {
+        return this.id.equals(id);
+    }


사용하지 않는 메서드는 제거해주세요.

jnsorn · 2022-05-06T05:21:25Z

src/main/java/wooteco/subway/ui/LineController.java

+    @PutMapping("/{id}")
+    public void updateLine(@PathVariable Long id, @RequestBody LineRequest lineRequest) {
+        lineService.update(id, lineRequest);
+    }
+
+    @DeleteMapping("/{id}")
+    @ResponseStatus(HttpStatus.NO_CONTENT)
+    public void deleteById(@PathVariable Long id) {
+        lineService.deleteById(id);
+    }


다양한 상태코드에 대해 학습하신것 같아요! 👍
수정에 대한 응답은 어떻게 내려줘야 좋을지 고민해볼까요?

저라면 수정된 결과를 응답으로 받아야한다면 200, 응답이 불필요하다면 204 no content로 보낼 것 같아요
또는 화면 새로 고침을 요구하기 위해 205 까지도 사용할 수 있을 것 같습니다!

그렇군요! 좋아요😃 보통 PUT에 대한 응답은 200이나 201로 많이 내려요. 리소스가 생성(혹은 변경)되었다는 의미로 200보다는 조금 더 구체적인 201을 사용하기도해서, 연로그의 생각을 물어봤어요. 200도 좋습니다!

jnsorn · 2022-05-06T05:25:04Z

src/main/java/wooteco/subway/ui/LineController.java

+    @PostMapping
+    public ResponseEntity<LineDto> createLine(@RequestBody LineRequest lineRequest) {
+        Line newLine = lineService.save(lineRequest);
+        LineDto lineResponse = LineDto.from(newLine);
+        return ResponseEntity.created(URI.create("/lines/" + newLine.getId())).body(lineResponse);
+    }


자원 생성에 대해 201 응답 + URI까지 잘 담아주셨네요! 💯
혹시 body는 왜 넣어주고 있나요~?

요번 과제에서는 API 문서가 존재하는데요
해당 API 문서를 따른 내용입니다😄

jnsorn · 2022-05-06T05:30:09Z

src/main/java/wooteco/subway/ui/SubwayControllerAdvice.java

+@ControllerAdvice
+public class SubwayControllerAdvice {
+
+    @ExceptionHandler
+    @ResponseStatus(HttpStatus.BAD_REQUEST)
+    public void handleException(IllegalArgumentException e) {
+        e.printStackTrace();
+    }
+}


여러 controller에서 발생하는 예외 처리를 위해 ControllerAdvice를 따로 분리하셨군요 👍
하지만 예외가 발생하면 stackTrace만 찍을 뿐 어떤 처리도 하지 않고 있네요. 이런 상황에서 사용자는 어떤 경험을 하게 될까요?
또, SQL 예외나 NPE와 같은 예외는 어떻게 처리할 수 있을까요?

예외가 발생하는 경우 프런트 쪽에서 별도로 처리해둔 코드를 알지 못해 Http 상태 코드만 설정해두었습니다😂
전에 프런트까지 직접 구현했을 때는 에러의 메시지를 보내주었던 것 같아요

SQL이나 NPE 같은 경우에는 리팩터링하며 수정하겠습니다!

jnsorn · 2022-05-06T05:32:38Z

src/test/java/wooteco/subway/acceptance/LineAcceptanceTest.java

+    @DisplayName("id 를 이용하여 노선을 삭제한다.")
+    @Test
+    void deleteLineById() {
+        //given
+        String id = String.valueOf(savedId1);
+
+        //when
+        ExtractableResponse<Response> response = RestAssured.given().log().all()
+                .when()
+                .delete("/lines/" + id)
+                .then().log().all()
+                .extract();
+
+        //then
+        assertThat(response.statusCode()).isEqualTo(HttpStatus.NO_CONTENT.value());
+    }


삭제 API에 대한 응답을 확인하는 것도 좋지만, 다시 조회 했을 때 해당 id를 가진 노선이 없다는 걸 확인해보면 더 정확한 테스트가 될 것 같아요!

jnsorn · 2022-05-06T05:34:39Z

docs/README.md

+2. Spring Bean & Spring JDBC 적용하기
+    - `@Controller`, `@Service`, `@Repository` 적용
+    - 예외 `@ExceptionHandler`로 처리
+    - JdbcTemplate는 `NamedJdbcTemplate` 사용


이 과정에서 연로그가 어떤 걸 느꼈는지 궁금해요! 혹시 공유해주실 수 있나요? ☺️

@Controller, @Service, @Repository 적용

의존 관계를 직접 주입하며 크고 작은 에러를 많이 만나다 테스트 코드를 위한 메서드를 생성하는 상황(..)까지 이르렀는데 컴포넌트를 활용할 수 있게 되니 훨씬 편리함을 느꼈습니다
컴포넌트 어노테이션을 왜 사용하느냐 라고 누가 묻는다면 그 이유 중 하나로 편리한 의존 관계 주입이라고 답할 것 같아요!👍

예외 @ExceptionHandler로 처리

예외를 위해 controller의 모든 메서드에서 try - catch를 사용해야했습니다
반환 값도 모든 것을 ResponseEntity를 활용해야했고요 (개인적으로는 더 가독성이 좋다고 생각해 @ResponseStatus를 사용하고 dto를 반환하는 것을 선호합니다)
예외 발생하는 부분을 공통적으로 처리할 수 있는게 편리했어요😄

JdbcTemplate는 NamedJdbcTemplate 사용

여러가지 JdbcTemplate 중에서 NamedJdbcTemplate을 사용한 이유는 파라미터의 이름을 지정할 수 있었기 때문입니다
JdbcTemplate은 ?를 통해 순서에 맞춰서 파라미터를 주입시키는데 누가 잘못 눌러서 명령어 순서만 바뀌어도 오류가 나는 상황이 마음에 들지 않았어요

크 공유 고마워요! 다양한 걸 느끼셨군요.😎
앞으로 더 편리하고 신기한 기술들을 마주치게 될텐데, 그때도 지금처럼 이 기술이 무엇을 해결하기 위해 등장했고 어떤 점이 편리한지 등을 생각하면서 사용하면 기술들을 더 잘 사용하실 수 있을거예요 👍

- 상태 코드가 아닌 id 목록의 일치하는지 검사

- 불필요/필요한 dto 정리 - 테스트 전체 실행 시 일부 테스트 실패하는 현상 개선

jnsorn

연로그! 리뷰 잘 반영해주셨네요. 🚀
몇 가지 코멘트 남겼는데 확인해보시고 다음 단계 구현하면서 반영해주세요.
이번 단계는 여기서 마무리할게요. 수고 많으셨어요 :)
다음 단계도 화이팅입니다!

jnsorn · 2022-05-08T14:03:49Z

docs/README.md

+### 1단계 기능 목록
+> 데이터 형식은 [API 문서](https://techcourse-storage.s3.ap-northeast-2.amazonaws.com/d5c93e187919493da3280be44de0f17f#Line) 에 따른다.


🙇🏻‍♀️

jnsorn · 2022-05-08T14:04:31Z

src/main/java/wooteco/subway/ui/SubwayControllerAdvice.java

+import org.springframework.web.bind.annotation.ResponseStatus;
+import org.springframework.web.bind.annotation.RestControllerAdvice;
+
+@RestControllerAdvice


jnsorn · 2022-05-08T14:09:46Z

src/main/java/wooteco/subway/ui/SubwayControllerAdvice.java

+    @ExceptionHandler
+    @ResponseStatus(HttpStatus.INTERNAL_SERVER_ERROR)
+    public void handleException(Exception e) {
+        logger.error(EMPTY_STRING, e);
+    }


사용자에게 메세지는 내려주지 않아도 괜찮을까요?

코드상 의미있는 메시지를 던져준건 IllegalArgumentException 뿐입니다!
만약 의미있는 메시지를 던지는 Exception을 던지면 그때 가서 별도로 처리해줄 것 같아요~

Exception은 그 외에 예상치 못한 예외 상황일텐데 이 메시지들까지 사용자에게 던져도 괜찮은가?에 대한 의문이 찜찜해서 따로 넘겨주지는 않았습니다!

jnsorn · 2022-05-08T14:24:15Z

docs/README.md

+2. Spring Bean & Spring JDBC 적용하기
+    - `@Controller`, `@Service`, `@Repository` 적용
+    - 예외 `@ExceptionHandler`로 처리
+    - JdbcTemplate는 `NamedJdbcTemplate` 사용


크 공유 고마워요! 다양한 걸 느끼셨군요.😎
앞으로 더 편리하고 신기한 기술들을 마주치게 될텐데, 그때도 지금처럼 이 기술이 무엇을 해결하기 위해 등장했고 어떤 점이 편리한지 등을 생각하면서 사용하면 기술들을 더 잘 사용하실 수 있을거예요 👍

jnsorn · 2022-05-08T14:25:53Z

src/main/java/wooteco/subway/service/LineService.java

+    private void validDuplicatedName(String name) {
+        if (lineDao.countByName(name) > 0) {
+            throw new IllegalArgumentException(DUPLICATED_NAME_ERROR_MESSAGE);
+        }
+    }


중복 확인을 위해 count 쿼리를 사용하고 있는데, 더 좋은 방법은 없을까요? https://jojoldu.tistory.com/516 참고하면 좋을 것 같아요.

jnsorn · 2022-05-08T14:29:02Z

src/main/java/wooteco/subway/ui/LineController.java

+    @PutMapping("/{id}")
+    public void updateLine(@PathVariable Long id, @RequestBody LineRequest lineRequest) {
+        lineService.update(id, lineRequest);
+    }
+
+    @DeleteMapping("/{id}")
+    @ResponseStatus(HttpStatus.NO_CONTENT)
+    public void deleteById(@PathVariable Long id) {
+        lineService.deleteById(id);
+    }


그렇군요! 좋아요😃 보통 PUT에 대한 응답은 200이나 201로 많이 내려요. 리소스가 생성(혹은 변경)되었다는 의미로 200보다는 조금 더 구체적인 201을 사용하기도해서, 연로그의 생각을 물어봤어요. 200도 좋습니다!

jnsorn · 2022-05-08T14:30:32Z

src/test/java/wooteco/subway/dao/StationDaoTest.java

+import wooteco.subway.domain.Station;
+
+@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT)
+@Transactional


@Transactional 어노테이션이 등장했네요. ㅎㅎ 연로그는 이 어노테이션을 어떤 문제를 해결하기 위해 사용하셨나요? 또, 문제를 해결할 수 있는 다른 방법은 무엇이 있을까요?

테스트 코드에서 생성된 데이터는 DB에 반영되지 않도록 하기 위함입니다!
테스트 전용 테이블을 따로 만들거나 테스트가 끝날때마다 데이터를 수동으로 지워주는 방법이 있을 것 같아요 :)

jnsorn · 2022-05-08T14:31:13Z

src/test/java/wooteco/subway/dao/StationDaoTest.java

+        // then
+        assertThat(before.size() - 1).isEqualTo(after.size());
+    }
+}


해당 경고에 대해 알아볼까요?

jnsorn · 2022-05-08T14:35:43Z

src/test/java/wooteco/subway/acceptance/LineAcceptanceTest.java

+    @DisplayName("id 를 이용하여 노선을 삭제한다.")
+    @Test
+    void deleteLineById() {
+        //given
+        String id = String.valueOf(savedId1);
+        List<Long> expectedIds = selectLines();
+        expectedIds.remove(Long.parseLong(id));
+
+        //when
+        ExtractableResponse<Response> response = RestAssured.given().log().all()
+                .when()
+                .delete("/lines/" + id)
+                .then().log().all()
+                .extract();
+
+        //then
+        assertThat(expectedIds).isEqualTo(selectLines());
+    }
+
+    private List<Long> selectLines() {
+        ExtractableResponse<Response> response = RestAssured.given().log().all()
+                .when()
+                .get("/lines")
+                .then().log().all()
+                .extract();
+
+        return response.jsonPath().getList(".", LineResponse.class).stream()
+                .map(LineResponse::getId)
+                .collect(Collectors.toList());
+    }


정확한 검증 👍
selectLines()라는 이름보다는 getAllLines()`` findAllLines()와 같은 이름이 명확할 것 같다는 생각이 드네요 ㅎ_ㅎ 네이밍은 연로그의 취향에 맞춰 선택해주세요~

jnsorn · 2022-05-08T14:36:28Z

src/test/java/wooteco/subway/acceptance/LineAcceptanceTest.java

+        Map<String, String> parameter = new HashMap<>();
+        String name = "분당선";
+        parameter.put("name", name);
+        parameter.put("color", "bg-red-600");


Map이 아닌 객체를 만들어 바로 넣을 수 있어요 :) Request객체 이용하면 편하겠네요 ㅎㅎ

yeon-06 added 28 commits May 3, 2022 14:28

docs: 기능 요구사항 정리

76560a1

feat: 중복 이름으로 지하철 역 등록 시 예외 발생 기능 구현

5e0d05b

feat: 지하철 역 삭제 기능 구현

fd61628

refactor: controller와 service 분리

a874566

feat: 지하철 노선 등록 기능 구현

249e855

feat: 지하철 노선 목록 조회 기능 구현

5fc8c12

feat: 지하철 노선 검색 기능 구현

a935ac2

test: 예외 테스트 케이스 추가

852228e

- 중복되는 이름으로 노선 생성 - 존재하지 않는 노선 id 검색

feat: 지하철 노선 수정 기능 구현

61f1fcb

feat: 지하철 노선 삭제 기능 구현

594b0ac

docs: 1단계 요구사항 수정

725da71

fix: 노선은 중복되는 이름으로 수정 불가능

5312509

docs: 2단계 기능 목록 정리

5e2f038

feat: Spring bean 적용

4170996

refactor: 예외는 @ExceptionHandler로 처리

83aa4e3

chore: DB 관련 설정 파일 생성

e46329e

refactor: StationDao에 Spring JDBC 적용

67d6741

refactor: LineDao에 Spring JDBC 적용

faddc64

refactor: 중복 확인 로직 변경

49ead28

test: 테스트 중복 코드 제거

e27b167

refactor: 빈 생성자 private화

426d26c

refactor: 불필요한 옵션 제거

2adb9eb

refactor: 중복 URI 제거

36e7b77

refactor: 반환값 ResponseEntity -> DTO 변환

200abf4

refactor: 에러 메시지 상수화

422a1d3

refactor: dto 변환 방식 변경

89d16a9

refactor: dto 변환 방식 변경

c5036ab

Merge remote-tracking branch 'origin/step1' into step1

3579369

jnsorn requested changes May 6, 2022

View reviewed changes

refactor: 생성자 체이닝 활용

6d8fde4

yeon-06 added 9 commits May 6, 2022 16:47

refactor: 컨트롤러의 예외 처리

7ef06ed

refactor: 미사용 메서드 제거

9784d0a

docs: API 문서 링크 첨부

375fef9

test: 노선 삭제하는 테스트 코드 수정

f0788db

- 상태 코드가 아닌 id 목록의 일치하는지 검사

refactor: API에 적절한 dto 사용

6820fe9

refactor: equals & hashCode 재정의

1542648

test: dao 테스트 코드 추가

22732a1

refactor: 불필요한 import 제거

1de0d86

fix: 테스트 케이스 깨지는 현상 수정

8ff3fd8

- 불필요/필요한 dto 정리 - 테스트 전체 실행 시 일부 테스트 실패하는 현상 개선

jnsorn approved these changes May 8, 2022

View reviewed changes

jnsorn merged commit 44055ce into woowacourse:yeon-06 May 8, 2022

jnsorn mentioned this pull request May 13, 2022

[Spring 지하철 노선도 - 3단계] 연로그(권시연) 미션 제출합니다. #274

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Spring 지하철 노선도 - 1,2단계] 연로그(권시연) 미션 제출합니다. #190

[Spring 지하철 노선도 - 1,2단계] 연로그(권시연) 미션 제출합니다. #190

yeon-06 commented May 4, 2022 •

edited

Loading

jnsorn left a comment

jnsorn May 6, 2022

jnsorn May 6, 2022

jnsorn May 6, 2022

jnsorn May 6, 2022

jnsorn May 6, 2022

yeon-06 May 6, 2022

jnsorn May 8, 2022

jnsorn May 6, 2022

yeon-06 May 6, 2022

jnsorn May 6, 2022

yeon-06 May 6, 2022

jnsorn May 6, 2022

jnsorn May 6, 2022

yeon-06 May 6, 2022

jnsorn May 8, 2022

jnsorn left a comment

jnsorn May 8, 2022

jnsorn May 8, 2022

jnsorn May 8, 2022

yeon-06 May 9, 2022

jnsorn May 8, 2022

jnsorn May 8, 2022

jnsorn May 8, 2022

jnsorn May 8, 2022

yeon-06 May 9, 2022

jnsorn May 8, 2022

jnsorn May 8, 2022

jnsorn May 8, 2022

		### 1단계 기능 목록
		> 데이터 형식은 [API 문서](https://techcourse-storage.s3.ap-northeast-2.amazonaws.com/d5c93e187919493da3280be44de0f17f#Line) 에 따른다.

[Spring 지하철 노선도 - 1,2단계] 연로그(권시연) 미션 제출합니다. #190

[Spring 지하철 노선도 - 1,2단계] 연로그(권시연) 미션 제출합니다. #190

Conversation

yeon-06 commented May 4, 2022 • edited Loading

1단계 기능 목록

2단계 기능 목록

고민했던 포인트

jnsorn 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

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

@Controller, @Service, @Repository 적용

예외 @ExceptionHandler로 처리

JdbcTemplate는 NamedJdbcTemplate 사용

Choose a reason for hiding this comment

jnsorn 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yeon-06 commented May 4, 2022 •

edited

Loading

`@Controller`, `@Service`, `@Repository` 적용

예외 `@ExceptionHandler`로 처리

JdbcTemplate는 `NamedJdbcTemplate` 사용