이번 미션에서는 지난 스프링 지하철 노선도 미션에 이어서 경로 조회 요구사항이 추가됐다. 최단경로를 찾아주는 dijkstra 알고리즘을 기반으로 제공되는 외부 라이브러리(jgrapht)를 이용하여 요구사항을 구현하면 됐기 때문에, 알고리즘 상으로는 크게 고민할 필요가 없었다. 그렇지만 아키텍처 설계 면에서도 배울 점이 없었던 건 아니다. 외부 라이브러리를 사용하는 미션이 처음이었던 덕분에, 오히려 이번 미션에서 굉장히 많은 것들을 배울 수 있었다.
이번 미션에서의 내 코드 및 PR 링크를 바탕으로 간단하게 내가 배운 점들 중 인상깊었던 점들을 포스팅해보도록 하겠다.
인터페이스를 활용한 외부 라이브러리 의존성 제거
이번 미션의 핵심 포인트인 듯하다. 나는 도메인 영역에서 jgrapht 외부 라이브러리 간선에 대한 객체를 도메인으로 가지고 있었다. 그리고, 외부 라이브러리에서 제공해주는 graphPath를 서비스 영역에서 의존하고 있었다. 리뷰어 앨런은 이 점을 지적해주셨다.
도메인이나 서비스에서 외부 라이브러리를 의존할 경우, 외부 라이브러리 버전/기능 변경 및 삭제가 이루어질 때에 많은 변경을 해주어야 한다. 따라서 변경을 최소화해주기 위해 의존성을 제거해주어야 한다.
어떻게 하면 좋을까?
나는 인터페이스 분리 및 최단경로 관련 도메인을 생성해주어 위 문제를 해결했다.
// 최단 경로 정보를 제공하는 인터페이스
public interface PathFinder {
Path findShortestPathByGraph(Station source, Station target);
void addVertex(List<Station> stations);
void addEdge(Station upStation, Station downStation, Section section);
}
최단 경로를 제공해주는 인터페이스이다.
여기서는 외부 라이브러리에 의존하지 않아야 한다. 따라서, 최단 경로 관련 정보를 담고 있는 `Path` 도메인을 새로 생성해 제공해준다. 실제 사용되는 service layer에서는 구현체가 아닌, 추상화 객체를 의존하고(주입받고) 있기 때문에 DIP 원칙을 지켜주어 변경에 용이하게 해주었다.
// 외부 라이브러리 (jgrapht)를 사용하는 구현체
public class ShortestPathFinder implements PathFinder {
private final WeightedMultigraph<Station, ShortestPathEdge> graph;
public ShortestPathFinder() {
this.graph = new WeightedMultigraph<>(ShortestPathEdge.class);
}
@Override
public Path findShortestPathByGraph(Station source, Station target) {
// 현재 정점 및 간선 바탕으로 최단경로 도메인 제공
}
@Override
public void addVertex(List<Station> stations) {
// 정점 초기화
}
@Override
public void addEdge(Station upStation, Station downStation, Section section) {
// 간선 초기화;
}
}
PathFinder 인터페이스를 상속받는 구현체이다.
이 구현체에서 외부 라이브러리를 사용하여 최단경로 정보를 구해준다.
만약 더 이상 jgrapht 라이브러리가 아닌, 다른 라이브러리를 활용하여 최단경로를 구해준다고 해보자.
이 때, 우리는 DIP 원칙을 지켰기 때문에 주입받는 구현체만 바꾸어주면 된다. 스프링을 활용하고 있다면 @Component 어노테이션 위치만 변경해주면 되겠다.
@Valid를 이용한 Request DTO 검증
이번 미션에서는 입력값을 검증해주기 위한 @Valid 어노테이션을 써보았다.
@Valid 어노테이션을 사용하면 request dto에서 의도하지 않은 값이 들어올 때 500 에러를 던져준다. 물론, exception handler를 이용하여 자신이 원하는 에러로 보내줄 수도 있다. 나의 경우는 bad request라 판단했기 때문에 400 에러를 던져주도록 하였다.
LineRequest를 예시로 살펴보자.
public class LineRequest {
@NotBlank(message = "노선의 이름을 입력해주세요.")
@Length(max = 255)
private String name;
@NotBlank(message = "노선의 색깔을 정해주세요.")
@Length(max = 20)
private String color;
@NotNull(message = "노선의 상행역을 입력해주세요.")
private Long upStationId;
@NotNull(message = "노선의 하행역을 입력해주세요.")
private Long downStationId;
@Min(value = 1, message = "거리는 자연수여야 합니다.")
private int distance;
private int extraFare;
// ...
}
위와 같이 요구하는 값의 내용 및 범위를 설정해줄 수 있다. 만족하지 않을 경우에 발생하는 에러 메시지를 지정해줄 수도 있다.
주의할 점은, @NotBlank는 String에서만 쓸 수 있으며, @NotNull은 String과 Collection, wrapper class에서만 사용할 수 있다.
의문사하지 않도록 조심하자.
public class LineController {
// ...
@PostMapping
public ResponseEntity<LineResponse> createLine(@Valid @RequestBody LineRequest lineRequest) {
final LineResponse lineResponse = lineService.saveLine(lineRequest);
return ResponseEntity.created(URI.create("/lines/" + lineResponse.getId())).body(lineResponse);
}
// ...
}
컨트롤러에서 Request Body로 들어오는 dto를 검증하고 싶은 것이므로 앞에 @Valid를 붙여주면 된다.
만약 query parameter를 검증해주고 싶다면? request dto를 따로 생성해주어 @ModelAttribute를 붙여주도록 변경해주고, @Valid 어노테이션을 붙여주면 된다.
@ControllerAdvice
public class SubwayControllerAdvice {
@ExceptionHandler(MethodArgumentNotValidException.class)
public ResponseEntity<ErrorResponse> requestBodyInvalidError(MethodArgumentNotValidException e) {
final String message = e.getFieldErrors().stream()
.map(FieldError::getDefaultMessage)
.collect(Collectors.joining(" "));
return ResponseEntity.badRequest().body(new ErrorResponse(message));
}
// ...
}
주의할 점은, ControllerAdviced에서 따로 FieldError 작업 처리를 해주지 않으면 우리가 원하는 형태의 에러메시지가 뜨지 않고 복잡한 문구의 에러메시지가 발생하게 된다. 이는 @Valid 자체에서 에러가 발생하면 bindingResult의 모든 에러들을 포함하여 아래와 같은 꼴로 메시지를 던져주기 때문이다.
위 코드에서 우리가 원하는 메시지는 defaultMessage 하나 뿐이다.
final String message = e.getFieldErrors().stream()
.map(FieldError::getDefaultMessage)
.collect(Collectors.joining(" "));
따라서 getFieldError에서의 defaultMessage를 얻어준 후에, 공백을 join해주면 된다. (공백 join을 안해주면 우리가 의도한 메시지가 아닌, 띄어쓰기가 하나도 없는 에러메시지가 발생한다.) 그러므로 ControllerAdvice에서 위와 같이 추가작업을 해준 후에 에러 dto를 보내주었다.
domain에서, 그리고 front-end의 입력 폼에서 검증을 해주는데 @Valid, 혹은 @Validated로 또 검증을 해주어야 하는지 의문이 들 수도 있다. 나도 실제로 이 의문이 들어 @Valid를 쓸지 말지 고민을 많이 했다.
실제로 @Valid를 사용하지 않는 곳도 좀 존재한다는 한 크루의 피셜도 있었다. 그렇지만 대부분은 @Valid를 사용해주어 철저하게 검증을 해주는 모습을 볼 수 있었다. 크루들과 토론을 해보면서 이유를 정리해보았다.
- 백엔드는 '프론트에서 입력값 예외처리 해주겠지~' 하는 안일한 마음을 가지지 말고, 충분한 예외처리를 해주어야 한다.
- 도메인 레이어까지 내려갈 필요 없이 검증이 가능한 부분은 검증해주는 것이 좋다.
- 도메인 영역과 dto 영역에서 요구되는 값의 범위나 내용이 다를 수 있다.
첫 번째, 두 번째 내용이 주된 이유였다. 세 번째 내용은 어떤 크루에게 들은 의견인데, 꽤나 그럴싸하여 적어보았다.
만약 어떠한 도메인 규칙이 있었는데, 임시적으로 사정이 생겨 특정 요청값을 제한해야된다면? 도메인 규칙에는 만족하지만 현재 입력값으로는 받고 싶지 않은 경우가 생긴다면? 그러한 경우에 @Valid로 검증 처리를 해주면 좋지 않을까 싶다.
실제로 아직 경험이 적기 때문에 앞으로 더 많은 경험을 하면서 생각해봐야할 듯하다.
어떤 것을 주생성자로 삼을까?
기존에 나는 딱히 주생성자, 부생성자를 정하는 기준이 없었다. 굳이 정하자면, 가장 빨리 만들어진 생성자를 주생성자로 했었다. 늦게 만들어진 생성자에서는 아래와 같이 this를 사용하고 있었다.
public Section(Long upStationId, Long downStationId, int distance) {
validateSection(upStationId, downStationId, distance);
this.upStationId = upStationId;
this.downStationId = downStationId;
this.distance = distance;
}
public Section(Long lineId, Long upStationId, Long downStationId, int distance) {
this(upStationId, downStationId, distance);
this.lineId = lineId;
}
public Section(Long id, Long lineId, Long upStationId, Long downStationId, int distance) {
this(lineId, upStationId, downStationId, distance);
this.id = id;
}
기존에 주생성자, 부생성자를 정해두었던 방식이다.
이 부분에서 페어 코드를 담당하는 리뷰어 스티치의 피드백이 왔다.
리뷰어 스티치 (페어의 리뷰어님)
이런 경우에는 가장 많은 매개변수를 받는 생성자를 main으로 사용하고 그 외의 생성자에서 this를 호출하면 코드가 더 알아보기 쉽고 가독성 측면에서도 좋습니다!! 유효성 검증 또한 메인이 되는 생성자에서 가지도록 하구요!!
나는 이 부분에서 많은 고민을 했다. 가독성은 크게 차이가 나지 않는 것 같아 더 고민됐던 듯하다.
가장 많은 매개변수를 받는 생성자를 주생성자로 변경해줄 경우에 변경되는 점은 아래와 같다.
- 프로퍼티 값들을 모두 final로 설정해줄 수 있다.
- 매개변수를 적게 받는 생성자에서 null 혹은 우리가 원하는 default value를 설정하여 넣어줄 수 있다.
- 요구사항이 추가될 때마다 매번 주생성자를 변경해주어야 하며, 부생성자에도 변경이 발생한다.
1, 2번은 장점이고, 3번은 단점이다. 실제로 3번 때문에 많은 고민을 했었다.
1번의 경우를 부연 설명하자면, 가장 적은 매개변수를 받는 생성자를 주생성자로 사용할 경우, 가장 적은 파라미터를 매개변수로 받는 프로퍼티들만 final로 지정해줄 수 있다는 단점을 보완해준다. final이 붙지 않을 경우, 속성값이 변경될 수 있다는 위험에 노출되게 된다.
큰 차이는 없다고 생각하지만, 1번의 장점이 비교적 크게 다가와 주생성자를 변경해보았다.
변경한 주생성자, 부생성자 코드는 아래와 같다.
private final Long id;
private final String name;
private final String color;
private final int extraFare;
private final Sections sections;
public Line(Long id, String name, String color, int extraFare, List<Section> sections) {
validateLine(name, color);
this.id = id;
this.name = name;
this.color = color;
this.extraFare = extraFare;
this.sections = new Sections(sections);
}
public Line(Long id, String name, String color, int extraFare, Section section) {
this(id, name, color, extraFare, List.of(section));
}
public Line(Long id, String name, String color, int extraFare) {
this(id, name, color, extraFare, Collections.emptyList());
}
public Line(String name, String color, int extraFare) {
this(null, name, color, extraFare, Collections.emptyList());
}
public Line(String name, String color) {
this(null, name, color, DEFAULT_EXTRA_FARE, Collections.emptyList());
}
Service에서 서비스 의존? dao(repository) 의존?
이번 미션에서 핫한 주제 중 하나였다. 크루들마다 의견도 굉장히 달랐다.
나는 일단 이번 미션에서 repository를 쓰지 않고 dao만 작성했다. 단순 지하철 구간, 경로, 역 CRUD 정도만 존재하기 때문에 layer 계층을 추가로 늘리지 않고 서비스에서 dao를 이용하여 처리할 수 있다고 판단했기 때문이다.
그렇다면 서비스에서 다른 도메인의 로직이 필요할 때엔 서비스를 의존하는 것이 좋을까, dao(repository)를 의존하는 것이 좋을까?
난 일단 이번 미션에선 dao를 의존하도록 했다. 사실 깊게 생각하진 않았고, 현재 규모에서 dao를 의존하더라도 크게 불편하지 않고 코드량도 많지 않았기 때문이다. dao layer에 비해 service layer가 더 크기도 하므로, 의존성 관련에 크게 문제가 되지 않는다 판단해서 dao를 의존하도록 했다.
그렇지만 서비스에서 서비스를 의존해도 좋다고 생각한다. 아니, 애플리케이션 규모가 커진다면 오히려 그게 더 좋을 수도 있다고 판단한다. 다른 서비스에서 트랜잭션 단위로 작업을 해둔 것을 이용할 수 있기 때문에 dao를 의존할 때보다 책임이 명확해질 수 있고, 하나의 서비스에서 코드량도 줄어들 수 있다고 판단했기 때문이다. 또한, 서비스 쪽에선 transaction 단위의 작업을 해주기 때문에 @Transactional 어노테이션을 이용하여 동시성 문제 방지를 해줄 수 있다는 점도 좋아보인다. 순환참조가 발생할 수 있다는 단점도 있다고 하는데, 이는 리팩터링 및 구조 설계 변경으로 방지하는 것이 좋아보인다.
크루들이 토론한 내용은 아래 링크에서 확인할 수 있다.
https://github.com/woowacourse/retrospective/discussions/15
전반적으로 작업하면서 느낀 점 (PR에 작성한 내용)
- Section 도메인에서 station이 아닌 station의 Id값을 가지고 있어서 객체를 찾기 위해 find 쿼리를 한번 더 사용하여 N+1 쿼리문제가 발생한다는 점을 느낄 수 있었습니다.
- 도메인에서 외부 라이브러리를 의존하지 않도록 인터페이스를 활용할 수 있다는 점을 느꼈습니다.
- fakeDao로 서비스 테스트를 진행하다보니, service 생성자 의존성이 변경될 때, 변경에 용이하지 못하다는 것을 느꼈습니다. 실제로 변경 과정에서 PathService의 생성자 부분을 PathFinder 인터페이스를 주입받도록 변경하려 할 때, test에서 ShortestPathFinder의 fake 버전이 요구돼서 의존성 변경이 힘듦을 느꼈습니다.
특히 이번 미션에선 다른 크루들과 많이 대화해보고, 다른 크루들의 관점에서 접근해보면서 많이 배운 듯하다. 우테코 크루들과 많이 대화하는 시간을 가져봐야겠다는 생각이 드는 미션이었다~
'JAVA > 우아한테크코스 4기' 카테고리의 다른 글
[220614] 우아한테크코스 레벨2 후기 (0) | 2022.06.15 |
---|---|
[220609] 장바구니 협업 미션 회고 (0) | 2022.06.14 |
[220512] Spring - 지하철 노선도 미션을 통해 배운 점 (4) | 2022.05.12 |
[220507] 우아한테크코스 4기 13주차 후기 (9) | 2022.05.08 |
[220419] 우아한테크코스 레벨2 개학 후기 (6) | 2022.04.20 |