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

[김대겸] Step2 PR #854

Merged
merged 11 commits into from
Nov 10, 2022
19 changes: 19 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,22 @@
-[X] 지하철역 관련 인수 테스트를 완성하세요.
-[X] 지하철역 목록 조회 인수 테스트 작성하기
-[X] 지하철역 삭제 인수 테스트 작성하기

## 🚀 2단계 - 지하철 노선 기능

### 기능 요구사항
- [X] 지하철 노선 생성
- [X] 인수 테스트 작성
- [X] 인수 테스트를 충족하는 기능 구현
- [X] 지하철 노선 목록 조회
- [X] 인수 테스트 작성
- [X] 인수 테스트를 충족하는 기능 구현
- [X] 지하철 노선 조회
- [X] 인수 테스트 작성
- [X] 인수 테스트를 충족하는 기능 구현
- [X] 지하철 노선 수정
- [X] 인수 테스트 작성
- [X] 인수 테스트를 충족하는 기능 구현
- [X] 지하철 노선 삭제
- [X] 인수 테스트 작성
- [X] 인수 테스트를 충족하는 기능 구현
Comment on lines +10 to +27

Choose a reason for hiding this comment

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

요구사항을 잘 정리해주신 것 같아요 👍👍

54 changes: 54 additions & 0 deletions src/main/java/nextstep/subway/application/LineService.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
package nextstep.subway.application;

import nextstep.subway.domain.Line;
import nextstep.subway.domain.LineRepository;
import nextstep.subway.dto.LineRequest;
import nextstep.subway.dto.LineResponse;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;

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

@Service
@Transactional(readOnly = true)
public class LineService {
private final LineRepository lineRepository;

public LineService(LineRepository lineRepository) {
this.lineRepository = lineRepository;
}

@Transactional
public LineResponse saveLine(LineRequest lineRequest) {
Line persistLine = lineRepository.save(lineRequest.toLine());
return LineResponse.of(persistLine);
}

public List<LineResponse> findAllLines() {
List<Line> lines = lineRepository.findAll();

return lines.stream()
.map(LineResponse::of)
.collect(Collectors.toList());
}

public LineResponse findLine(final Long id) {
Line persistLine = lineRepository.findById(id)
.orElseThrow(() -> new IllegalArgumentException(id + "번 노선을 찾을 수 없습니다."));

Choose a reason for hiding this comment

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

예외 메세지를 상세화 해주신 점 좋은 것 같아요 👍👍

return LineResponse.of(persistLine);
}

@Transactional
public void deleteLineById(Long id) {
lineRepository.deleteById(id);
}

@Transactional
public LineResponse modify(final Long id, final LineRequest lineRequest) {
Line persistLine = lineRepository.findById(id)
.orElseThrow(() -> new IllegalArgumentException(id + "번 노선을 찾을 수 없습니다."));
persistLine.modify(lineRequest);
return LineResponse.of(lineRepository.save(persistLine));
}
Comment on lines +52 to +53

Choose a reason for hiding this comment

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

영속화된 엔티티는 트랜잭션이 종료되기전에 더티 체킹이 일어나면서 데이터가 업데이트가 이뤄지게 되는데요.
save 호출해도 업데이트가 이뤄지기 때문에 문제되진 않겠지만
불필요한 로직이 수행될 수 있기 때문에 비효율적이지 않을까 라고 생각되는데
혹시 이부분에 대해서 어떻게 생각하시나요??🤔🤔

}
41 changes: 41 additions & 0 deletions src/main/java/nextstep/subway/domain/Line.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
package nextstep.subway.domain;

import nextstep.subway.dto.LineRequest;

import javax.persistence.*;

@Entity
public class Line extends BaseEntity {
@Id
@GeneratedValue(strategy = GenerationType.IDENTITY)
private Long id;
@Column(unique = true)
private String name;
@Column
private String color;

protected Line() {
}

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

public Long getId() {
return id;
}

public String getName() {
return name;
}

public String getColor() {
return color;
}

public void modify(final LineRequest lineRequest) {
this.name = lineRequest.getName();
this.color = lineRequest.getColor();
}
Comment on lines +37 to +40

Choose a reason for hiding this comment

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

도메인이 controller 에서 사용하고 있는 dto 까지 의존성이 생긴 것 같아요 🤔
결국 도메인과 dto가 양방향 의존성을 가진 상태 같은데요.
개인적으로 코드 유지보수를 위해 의존성의 방향은 안쪽으로만 향해야 한다고 생각합니다.
혹시 이 의존성을 제거하는 방향에 대해서 어떻게 생각하시나요??🤔🤔
제가 고민해보고 참고한 자료 링크 남겨드리도록 하겠습니다!
클린 아키텍처: 의존성 역전하기

Copy link
Author

Choose a reason for hiding this comment

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

실무에서 많이 고민하는 부분이기도 한데요,, 변경되는 필드들이 너무 많아져서 이렇게 자주사용하고 있었는데 말씀해주신것처럼 양방향 의존성을 가지게되네요...

링크 주신 것 참고해서 고민해보고 수정하도록 하겠습니다 ㅎㅎㅎ
감사합니다!!

}
10 changes: 10 additions & 0 deletions src/main/java/nextstep/subway/domain/LineRepository.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package nextstep.subway.domain;

import org.springframework.data.jpa.repository.JpaRepository;

import java.util.List;

public interface LineRepository extends JpaRepository<Line, Long> {
@Override
List<Line> findAll();
Comment on lines +8 to +9

Choose a reason for hiding this comment

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

혹시 기존에 존재하는 메서드를 오버라이드 하신 이유가 있으실까요?

}
20 changes: 20 additions & 0 deletions src/main/java/nextstep/subway/dto/LineRequest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
package nextstep.subway.dto;

import nextstep.subway.domain.Line;

public class LineRequest {
private String name;
private String color;

public String getColor() {
return color;
}

public String getName() {
return name;
}

public Line toLine() {
return new Line(name, color);
}
}
45 changes: 45 additions & 0 deletions src/main/java/nextstep/subway/dto/LineResponse.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
package nextstep.subway.dto;

import nextstep.subway.domain.Line;

import java.time.LocalDateTime;

public class LineResponse {
private final Long id;
private final String name;
private final String color;
private final LocalDateTime createdDate;
private final LocalDateTime modifiedDate;

public static LineResponse of(Line line) {
return new LineResponse(line.getId(), line.getName(), line.getColor(), line.getCreatedDate(), line.getModifiedDate());
}

public LineResponse(final Long id, final String name, final String color, final LocalDateTime createdDate, final LocalDateTime modifiedDate) {
this.id = id;
this.name = name;
this.color = color;
this.createdDate = createdDate;
this.modifiedDate = modifiedDate;
}

public Long getId() {
return id;
}

public String getName() {
return name;
}

public String getColor() {
return color;
}

public LocalDateTime getCreatedDate() {
return createdDate;
}

public LocalDateTime getModifiedDate() {
return modifiedDate;
}
}
53 changes: 53 additions & 0 deletions src/main/java/nextstep/subway/ui/LineController.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
package nextstep.subway.ui;

import nextstep.subway.application.LineService;
import nextstep.subway.dto.LineRequest;
import nextstep.subway.dto.LineResponse;
import org.springframework.dao.DataIntegrityViolationException;
import org.springframework.http.MediaType;
import org.springframework.http.ResponseEntity;
import org.springframework.web.bind.annotation.*;

import java.net.URI;
import java.util.List;

@RestController
public class LineController {
private final LineService lineService;

public LineController(LineService lineService) {
this.lineService = lineService;
}

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

@GetMapping("/lines/{id}")
public ResponseEntity<LineResponse> showLines(@PathVariable Long id) {
return ResponseEntity.ok().body(lineService.findLine(id));
}

@PutMapping("/lines/{id}")
public ResponseEntity<LineResponse> modifyLine(@PathVariable Long id, @RequestBody LineRequest lineRequest) {
return ResponseEntity.ok().body(lineService.modify(id, lineRequest));
}

@DeleteMapping("/lines/{id}")
public ResponseEntity<Void> deleteLines(@PathVariable Long id) {
lineService.deleteLineById(id);
return ResponseEntity.noContent().build();
}

@GetMapping(value = "/lines", produces = MediaType.APPLICATION_JSON_VALUE)
public ResponseEntity<List<LineResponse>> showLines() {
return ResponseEntity.ok().body(lineService.findAllLines());
}

@ExceptionHandler(DataIntegrityViolationException.class)
public ResponseEntity<Void> handleIllegalArgsException() {
return ResponseEntity.badRequest().build();
}
}
23 changes: 23 additions & 0 deletions src/test/java/nextstep/subway/AcceptanceTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
package nextstep.subway;

import io.restassured.RestAssured;
import org.junit.jupiter.api.BeforeEach;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.boot.web.server.LocalServerPort;
import org.springframework.test.context.ActiveProfiles;

@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT)
public class AcceptanceTest {
@LocalServerPort
int port;

@Autowired
private DatabaseCleanup databaseCleanup;

@BeforeEach
public void setUp() {
RestAssured.port = port;
databaseCleanup.execute();
}
}
49 changes: 49 additions & 0 deletions src/test/java/nextstep/subway/DatabaseCleanup.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
package nextstep.subway;

import org.springframework.beans.factory.InitializingBean;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.context.annotation.Profile;
import org.springframework.jdbc.core.JdbcTemplate;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;

import javax.sql.DataSource;
import java.sql.DatabaseMetaData;
import java.sql.ResultSet;
import java.util.ArrayList;
import java.util.List;

@Service
public class DatabaseCleanup implements InitializingBean {
@Autowired
Comment on lines +16 to +18

Choose a reason for hiding this comment

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

데이터를 초기화하기 위해 database cleanup 을 직접 구현해주셨네요 👍👍👍

private DataSource dataSource;

@Autowired
private JdbcTemplate jdbcTemplate;

private List<String> tableNames;

@Override
public void afterPropertiesSet() {
tableNames = new ArrayList<>();
try {
DatabaseMetaData metaData = dataSource.getConnection().getMetaData();
ResultSet tables = metaData.getTables(null, null, null, new String[]{"TABLE"});
while (tables.next()) {
String tableName = tables.getString("TABLE_NAME");
tableNames.add(tableName);
}
} catch (Exception e) {
throw new RuntimeException();
}
}

@Transactional
public void execute() {
jdbcTemplate.execute("SET REFERENTIAL_INTEGRITY FALSE");
for (String tableName : tableNames) {
jdbcTemplate.execute("TRUNCATE TABLE " + tableName);
}
jdbcTemplate.execute("SET REFERENTIAL_INTEGRITY TRUE");
}
}