Skip to content
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

Step3리뷰 요청드립니다. #501

Open
wants to merge 25 commits into
base: sang-eun
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
8ba6868
first commit
sang-eun Jul 7, 2022
2c883bb
[add] api 테스트 추가
sang-eun Jul 10, 2022
6e12f68
Merge branch 'sang-eun' into step1
sang-eun Jul 10, 2022
b4e2282
[add] 테스트 리스트
sang-eun Jul 10, 2022
5aed214
[add] create api 및 테스트 추가
sang-eun Jul 10, 2022
599824c
[modify] line 테스트 beforeAll 추가해서 자원공유하도록 수정
sang-eun Jul 10, 2022
325e5ab
[modify] step1 코드리뷰 반영
sang-eun Jul 12, 2022
2594684
[add] get api 테스트
sang-eun Jul 12, 2022
489e81a
[add] get lines api
sang-eun Jul 12, 2022
8beeb22
[add] get line api
sang-eun Jul 12, 2022
e094936
[add] update line api
sang-eun Jul 12, 2022
15ae19d
[add] delete line api
sang-eun Jul 12, 2022
3ecd092
[add] controllerAdvice 추가하려고 했으나 제대로 동작하지 않음. 캐치하고 204에러를 리턴해야 하는데, 계…
sang-eun Jul 12, 2022
3007872
Merge branch 'step1' of github.com:sang-eun/atdd-subway-map into step2
sang-eun Jul 12, 2022
47b5e09
[modify] 불필요한 부분 수정
sang-eun Jul 12, 2022
112e8c0
Merge remote-tracking branch 'upstream/sang-eun' into step2
sang-eun Jul 12, 2022
1d49dc3
코드리뷰 반영
sang-eun Jul 24, 2022
6181976
[add] 섹션 기반 코드 작성
sang-eun Jul 30, 2022
b992dd1
[add] 섹션 추가 api
sang-eun Jul 30, 2022
bcc0eb3
[modify] Line과 section 관계
sang-eun Jul 31, 2022
b5bb1d9
[add] 구간 삭제 테스트 추가
sang-eun Jul 31, 2022
4b11cea
[add] 구간 삭제 api 추가
sang-eun Jul 31, 2022
44a6707
[modify] section 저장 로직 수정
sang-eun Aug 1, 2022
b74b646
코드리뷰 반영
sang-eun Aug 6, 2022
33c993d
Merge branch 'sang-eun' of https://github.com/next-step/atdd-subway-m…
sang-eun Aug 6, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ dependencies {

// https://mvnrepository.com/artifact/org.projectlombok/lombok
compileOnly 'org.projectlombok:lombok:1.18.24'
annotationProcessor 'org.projectlombok:lombok:1.18.24'
implementation 'org.apache.commons:commons-text:1.9'

// log
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package nextstep.subway.applicaion.common;

import com.sun.jdi.request.DuplicateRequestException;

import java.util.NoSuchElementException;

public class DuplicatedDownStationException extends DuplicateRequestException {

public DuplicatedDownStationException() {
super("하행역이 이미 존재하는 역입니다.");
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package nextstep.subway.applicaion.common;

import com.sun.jdi.request.DuplicateRequestException;
import org.springframework.http.HttpStatus;
import org.springframework.http.ResponseEntity;
import org.springframework.web.bind.annotation.ExceptionHandler;
Expand All @@ -11,8 +12,13 @@
@RestControllerAdvice
public class ErrorController {

@ExceptionHandler(value = NoSuchElementException.class)
public ResponseEntity<ErrorResponse> noElementAdviser(NoSuchElementException e) {
return new ResponseEntity<>(new ErrorResponse(e.getMessage(), HttpStatus.NOT_FOUND), HttpStatus.NOT_FOUND);
}
@ExceptionHandler(value = NoSuchElementException.class)
public ResponseEntity<ErrorResponse> noElementAdviser(NoSuchElementException e) {
return new ResponseEntity<>(new ErrorResponse(e.getMessage(), HttpStatus.NOT_FOUND), HttpStatus.NOT_FOUND);
}

@ExceptionHandler(value = {IllegalArgumentException.class, DuplicateRequestException.class})
public ResponseEntity<ErrorResponse> noElementAdviser(RuntimeException e) {
return new ResponseEntity<>(new ErrorResponse(e.getMessage(), HttpStatus.BAD_REQUEST), HttpStatus.BAD_REQUEST);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package nextstep.subway.applicaion.common;

public class MinimumSectionException extends IllegalArgumentException {
public MinimumSectionException() {
super("해당 라인의 구간 갯수가 최소 갯수이므로 삭제가 불가능합니다.");
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package nextstep.subway.applicaion.common;

import java.util.NoSuchElementException;

public class SectionNotFoundException extends NoSuchElementException {

public SectionNotFoundException() {
super("해당 구간이 존재하지 않습니다.");
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
package nextstep.subway.applicaion.common;

public class UnappropriateStationException extends IllegalArgumentException {

public UnappropriateStationException() {
super("역이 해당 라인의 하행종점역이 아닙니다.");
}
}
29 changes: 12 additions & 17 deletions src/main/java/nextstep/subway/applicaion/line/LineService.java
Original file line number Diff line number Diff line change
Expand Up @@ -6,36 +6,35 @@
import nextstep.subway.applicaion.line.dto.LineRequest;
import nextstep.subway.applicaion.line.dto.LineResponse;
import nextstep.subway.applicaion.line.dto.LineUpdateRequest;
import nextstep.subway.applicaion.section.SectionService;
import nextstep.subway.applicaion.station.StationService;
import nextstep.subway.applicaion.station.domain.Station;
import nextstep.subway.applicaion.station.dto.StationResponse;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;

import java.util.List;
import java.util.NoSuchElementException;
import java.util.stream.Collectors;

@Service
@Transactional(readOnly = true)
public class LineService {

private LineRepository lineRepository;
private StationService stationService;

public LineService(LineRepository lineRepository, StationService stationService) {
private final LineRepository lineRepository;
private final StationService stationService;
private final SectionService sectionService;
public LineService(LineRepository lineRepository, StationService stationService, SectionService sectionService) {
this.lineRepository = lineRepository;
this.stationService = stationService;
this.sectionService = sectionService;
}

@Transactional
public LineResponse saveLine(LineRequest lineRequest) {
Station upStation = stationService.getStationById(lineRequest.getUpStationId());
Station downStation = stationService.getStationById(lineRequest.getDownStationId());
Station upStation = stationService.getStationThrowExceptionIfNotExists(lineRequest.getUpStationId());
Station downStation = stationService.getStationThrowExceptionIfNotExists(lineRequest.getDownStationId());

Line line = lineRepository.save(lineRequest.toLine());
line.getSections().add(line, upStation, downStation, lineRequest.getDistance());

return new LineResponse(line.getId(), line.getName(), line.getColor(), List.of(new StationResponse(upStation.getId(), upStation.getName()), new StationResponse(downStation.getId(), downStation.getName())));
return createLineResponse(line);
}

public List<LineResponse> findAllLines() {
Expand All @@ -53,7 +52,7 @@ public LineResponse findLineById(Long id) {
@Transactional
public void updateLineById(Long id, LineUpdateRequest lineRequest) {
Line line = lineRepository.findById(id).orElseThrow(LineNotFoundException::new);
line.updateLine(lineRequest.getName(), lineRequest.getColor());
line.updateLineInfo(lineRequest.getName(), lineRequest.getColor());
}

@Transactional
Expand All @@ -62,14 +61,10 @@ public void deleteLineById(Long id) {
}

private LineResponse createLineResponse(Line line) {
Station upStation = stationService.getStationById(line.getUpStationId());
Station downStation = stationService.getStationById(line.getDownStationId());

return new LineResponse(
line.getId(),
line.getName(),
line.getColor(),
List.of(new StationResponse(upStation.getId(), upStation.getName()), new StationResponse(downStation.getId(), downStation.getName()))
);
line.getSections().stations().stream().map(stationService::createStationResponse).collect(Collectors.toList()));
}
}
68 changes: 23 additions & 45 deletions src/main/java/nextstep/subway/applicaion/line/domain/Line.java
Original file line number Diff line number Diff line change
@@ -1,52 +1,30 @@
package nextstep.subway.applicaion.line.domain;

import lombok.Getter;
import lombok.NoArgsConstructor;

import javax.persistence.*;

@Getter
@Entity
@NoArgsConstructor
public class Line {
@Id
@GeneratedValue(strategy = GenerationType.IDENTITY)
private Long id;
@Column(unique = true)
private String name;
private String color;
private Long upStationId;
private Long downStationId;
private Integer distance;

public Line() {
}

public Long getId() {
return id;
}

public String getName() {
return name;
}

public String getColor() {
return color;
}

public Long getUpStationId() {
return upStationId;
}

public Long getDownStationId() {
return downStationId;
}

public Line(String name, String color, Long upStationId, Long downStationId, Integer distance) {
this.name = name;
this.color = color;
this.upStationId = upStationId;
this.downStationId = downStationId;
this.distance = distance;
}

public void updateLine(String name, String color) {
this.name = name;
this.color = color;
}
@Id
@GeneratedValue(strategy = GenerationType.IDENTITY)
private Long id;
@Column(unique = true)
private String name;
private String color;
@Embedded
private Sections sections = new Sections();

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

public void updateLineInfo(String name, String color) {
this.name = name;
this.color = color;
}
}
65 changes: 65 additions & 0 deletions src/main/java/nextstep/subway/applicaion/line/domain/Sections.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
package nextstep.subway.applicaion.line.domain;

import nextstep.subway.applicaion.common.DuplicatedDownStationException;
import nextstep.subway.applicaion.common.MinimumSectionException;
import nextstep.subway.applicaion.common.UnappropriateStationException;
import nextstep.subway.applicaion.section.domain.Section;
import nextstep.subway.applicaion.station.domain.Station;

import javax.persistence.*;
import java.util.ArrayList;
import java.util.List;
import java.util.Objects;
import java.util.stream.Collectors;

@Embeddable
public class Sections {
private final Integer MINIMUM_SECTION_SIZE = 1;
@OneToMany(fetch = FetchType.LAZY, cascade = {CascadeType.MERGE, CascadeType.PERSIST}, orphanRemoval = true)
@JoinColumn(name = "line_id")
private List<Section> sections = new ArrayList<>();

public void add(Line line, Station upStation, Station downStation, Integer distance) {
Section section = new Section(upStation, downStation, distance, line);
sections.add(section);
}

public List<Station> stations() {
List<Station> stations = sections.stream().map(Section::getUpStation).collect(Collectors.toList());
stations.add(getLastStation());
return stations;
}

public void removeStation(Long stationId) {
if (sections.size() <= MINIMUM_SECTION_SIZE) {
throw new MinimumSectionException();
}

if (!Objects.equals(getLastStation().getId(), stationId)) {
throw new UnappropriateStationException();
}
sections.remove(getLastSection());
}

public void checkIsLastStation(Station station) {
if (!Objects.equals(getLastStation().getId(), station.getId())) {
throw new UnappropriateStationException();
}
}

public void checkIsNewStation(Station station) {
stations().forEach(it -> {
if (Objects.equals(it.getId(), station.getId())) {
throw new DuplicatedDownStationException();
}
});
}

private Section getLastSection() {
return sections.get(sections.size() - 1);
}

private Station getLastStation() {
return getLastSection().getDownStation();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,6 @@ public LineRequest(String name, String color, Long upStationId, Long downStation
}

public Line toLine() {
return new Line(name, color, upStationId, downStationId, distance);
return new Line(name, color);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
package nextstep.subway.applicaion.section;

import nextstep.subway.applicaion.common.LineNotFoundException;
import nextstep.subway.applicaion.line.domain.Line;
import nextstep.subway.applicaion.line.domain.LineRepository;
import nextstep.subway.applicaion.section.dto.SectionRequest;
import nextstep.subway.applicaion.station.StationService;
import nextstep.subway.applicaion.station.domain.Station;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;

@Service
@Transactional
public class SectionService {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

해당 Section 은 Line 에 속해 있는 도메인으로 보이는데요
SectionService 를 선언하지 않아도 해당 로직을 Sections 와 Line 에서 할 수 있을 것으로 보이는데 어떻게 생각하시나요? 😊

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

클린아키텍쳐에서처럼 서비스가 비대해지는 것을 방지하기 위해 한 컨트롤러마다 GetSectionUseCase 와 같이 나누는 것까지는 아니더라도 최소한 중심이 되는 도메인(Section)별로 나누는 것이 좋다고 생각했습니다
응집시켜두는 것이 좋다고 생각하시는 이유가 있으실까요?

private final StationService stationService;
private final LineRepository lineRepository;

public SectionService(StationService stationService, LineRepository lineRepository) {
this.stationService = stationService;
this.lineRepository = lineRepository;
}

public void saveSection(Long lineId, SectionRequest sectionRequest) {
Line line = lineRepository.findById(lineId).orElseThrow(LineNotFoundException::new);

Station upStation = stationService.getStationThrowExceptionIfNotExists(sectionRequest.getUpStationId());
Station downStation = stationService.getStationThrowExceptionIfNotExists(sectionRequest.getDownStationId());

line.getSections().checkIsLastStation(upStation);
line.getSections().checkIsNewStation(downStation);

line.getSections().add(line, upStation, downStation, sectionRequest.getDistance());
}

public void deleteSection(Long lineId, Long stationId) {
Line line = lineRepository.findById(lineId).orElseThrow(LineNotFoundException::new);

line.getSections().removeStation(stationId);

}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
package nextstep.subway.applicaion.section.domain;

import lombok.Getter;
import lombok.NoArgsConstructor;
import nextstep.subway.applicaion.line.domain.Line;
import nextstep.subway.applicaion.station.domain.Station;

import javax.persistence.*;

@Getter
@Entity
@NoArgsConstructor
public class Section {

@Id
@GeneratedValue(strategy = GenerationType.IDENTITY)
private Long id;
@ManyToOne(cascade = CascadeType.PERSIST)
@JoinColumn(name = "up_station_id")
private Station upStation;
@ManyToOne(cascade = CascadeType.PERSIST)
@JoinColumn(name = "down_station_id")
private Station downStation;
private Integer distance;
@ManyToOne(cascade = CascadeType.PERSIST)
@JoinColumn(name = "line_id")
private Line line;

public Section(Station upStation, Station downStation, Integer distance, Line line) {
this.upStation = upStation;
this.downStation = downStation;
this.distance = distance;
this.line = line;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package nextstep.subway.applicaion.section.domain;

import nextstep.subway.applicaion.line.domain.Line;
import org.springframework.data.jpa.repository.JpaRepository;

import java.util.List;

public interface SectionRepository extends JpaRepository<Section, Long> {
List<Section> findAllByLineId(Long lineId);
}
Loading