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

[Step1] 경로 찾기 - 지하철역 관리 #34

Merged
merged 8 commits into from
Feb 10, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 5 additions & 0 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,16 @@ dependencies {
implementation 'org.springframework.boot:spring-boot-starter-jdbc'
implementation 'org.jgrapht:jgrapht-core:1.0.1'
implementation 'net.rakugakibox.spring.boot:logback-access-spring-boot-starter:2.7.1'
implementation 'org.springframework.boot:spring-boot-starter-data-jpa'
runtimeOnly 'com.h2database:h2'
testImplementation 'org.springframework.boot:spring-boot-starter-webflux'
testImplementation('org.springframework.boot:spring-boot-starter-test') {
exclude group: 'org.junit.vintage', module: 'junit-vintage-engine'
}
compileOnly("org.projectlombok:lombok")
testCompileOnly("org.projectlombok:lombok")
Comment on lines +28 to +29

Choose a reason for hiding this comment

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

👍

annotationProcessor("org.projectlombok:lombok")
testAnnotationProcessor("org.projectlombok:lombok")
}

test {
Expand Down
38 changes: 38 additions & 0 deletions src/main/java/atdd/station/Service/StationService.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
package atdd.station.Service;

Choose a reason for hiding this comment

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

Suggested change
package atdd.station.Service;
package atdd.station.service;


import atdd.station.domain.Station;
import atdd.station.domain.dto.StationDto;
import atdd.station.repository.StationRepository;
import org.springframework.stereotype.Service;

import javax.annotation.Resource;
import java.util.List;
import java.util.Optional;

@Service("stationService")
public class StationService
{
@Resource(name = "stationRepository")
private StationRepository stationRepository;

public Station create(StationDto stationDto)
{
return stationRepository.save(stationDto.toEntity());
}

public List<Station> list()
{
List<Station> returnList = stationRepository.findAll();

Choose a reason for hiding this comment

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

List, Set 등 자료구조를 지칭하는 단어를 클래스나 변수이름으로 사용하는 것을 지양하고 있습니다.
사용하는 자료구조가 변경되면 변경의 범위가 외부로 전파될 수 있기 때문입니다.
List와 같은 복수의 데이터의 경우 복수형을 쓰는 것이 더 적합하다고 생각합니다.

https://github.com/Yooii-Studios/Clean-Code/blob/master/Chapter%2002%20-%20의미%20있는%20이름.md#그릇된-정보를-피하라

return returnList;
}

public Optional<Station> findById(long id)
{
return stationRepository.findById(id);
}

public void deleteStationById(long id)
{
stationRepository.deleteById(id);
}
}
8 changes: 0 additions & 8 deletions src/main/java/atdd/station/StationController.java

This file was deleted.

65 changes: 65 additions & 0 deletions src/main/java/atdd/station/controller/StationController.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
package atdd.station.controller;

import atdd.station.Service.StationService;
import atdd.station.domain.Station;
import atdd.station.domain.dto.StationDto;
import org.springframework.http.HttpHeaders;
import org.springframework.http.HttpStatus;
import org.springframework.http.ResponseEntity;
import org.springframework.web.bind.annotation.*;

import javax.annotation.Resource;
import java.net.URI;
import java.util.List;
import java.util.Optional;

/*
step1 : 지하철역 관리 API 만들기
*/
@RestController
@RequestMapping(value = "/stations", produces = "application/json")
public class StationController
{
@Resource(name = "stationService")
private StationService stationService;
/*
1. 지하철역 등록
*/
Comment on lines +25 to +27

Choose a reason for hiding this comment

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

저는 주석보다는 코드로 의도를 드러내는 것을 선호하는 편입니다 ㅎ;

@PostMapping("/create")

Choose a reason for hiding this comment

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

REST API 설계시에,

  1. URI는 정보의 자원을 표현해야 한다.(리소스명은 동사보다는 명사를 사용)
  2. 자원에 대한 행위는 HTTP Method(GET, POST, PUT, DELETE 등)으로 표현

stations URI에 POST 요청을 보내는 것만으로도 생성의 의미를 표현할 수 있습니다.


POST와 PUT 사용에 대해서는 해석의 여지가 있습니다.
보통 PUT은 수정, POST는 생성으로 많이 사용합니다.
다만, HTTP 완벽가이드 등에서는 아래와 같이 제시하기도 합니다.

  • PUT은 URI가 존재하면 update 없으면 insert를 한다.
  • POST는 URI가 collection일 경우 collection에 추가하고 element일 경우 해당 URI를 collection으로 간주하여 그 하위에 들어간다.

이 경우엔, PUT을 사용하여 리소스를 생성할 경우, 요청하는 쪽에서 ID를 알고 있어야 합니다.
반면, POST로 생성을 하려면 ID를 모르는 상태에서 URI에 collection을 지정해야 한다.
ex) "PUT /articles/1" 하면 1번 글이 생성되고, "POST /articles" 하면 서버에서 할당한 번호의 글이 생성된다.

public ResponseEntity<Station> createStations(@RequestBody StationDto stationDto)
{
Station createdStation = stationService.create(stationDto);
HttpHeaders headers = new HttpHeaders();
headers.setLocation(URI.create("/stations/list" + createdStation.getId()));
ResponseEntity<Station> returnEntity = new ResponseEntity<>(createdStation, headers, HttpStatus.CREATED);
return returnEntity;

Choose a reason for hiding this comment

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

Suggested change
return returnEntity;
return ResponseEntity
.created(URI.create(BASE_STATIONS_URL + createdStation.getId()))
.contentType(MediaType.APPLICATION_JSON)
.body(createdStation);
	public static BodyBuilder created(URI location) {
		BodyBuilder builder = status(HttpStatus.CREATED);
		return builder.location(location);
	}

}
/*
2. 지하철역 목록 조회
*/
@GetMapping("/list")
public ResponseEntity list()
{
List<Station> stationList = stationService.list();
ResponseEntity returnEntity = new ResponseEntity(stationList, HttpStatus.OK);
return returnEntity;
}
/*
3. 지하철역 조회
*/
@GetMapping("/detail/{id}")
public ResponseEntity detailById(@PathVariable long id)
{
Optional<Station> detailStation = stationService.findById(id);
ResponseEntity returnEntity = new ResponseEntity(detailStation, HttpStatus.OK);
return returnEntity;
}
/*
4. 지하철역 삭제
*/
@DeleteMapping("/delete/{id}")
public void deleteStation(@PathVariable long id)
{
stationService.deleteStationById(id);
}
}
25 changes: 25 additions & 0 deletions src/main/java/atdd/station/domain/Station.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
package atdd.station.domain;

import lombok.*;

import javax.persistence.*;

@NoArgsConstructor
@Getter
@Entity
public class Station
{
@Id
@GeneratedValue(strategy = GenerationType.AUTO)
private long id;

@Column(length = 20, nullable = false)
private String name;

@Builder

Choose a reason for hiding this comment

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

👍

public Station(String name)
{
this.name = name;
}

}
19 changes: 19 additions & 0 deletions src/main/java/atdd/station/domain/dto/StationDto.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
package atdd.station.domain.dto;

Choose a reason for hiding this comment

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

dto는 presentation layer에 해당하니 controller 하위 패키지에 있는 것은 어떨까요


import atdd.station.domain.Station;
import lombok.Getter;
import lombok.NoArgsConstructor;
import lombok.Setter;

@Getter
@Setter
@NoArgsConstructor
public class StationDto
{
private String name;

public Station toEntity()

Choose a reason for hiding this comment

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

👍
변경이 잦은 쪽에서 변경이 잦지 않은 쪽을 참조하도록 잘 구성하셨습니다.
그 외에도 DTO assembler pattern 등의 방법도 있으니 참고해보세요.
http://redutan.github.io/2016/04/15/poeaa-distribution-pattern

추가적으로 아래 링크들에서 DTO 관련 논의들이 있으니 참고해보세요.
https://www.slipp.net/questions/22
https://www.slipp.net/questions/23
https://www.slipp.net/questions/93

{
return Station.builder().name(name).build();
}
}
8 changes: 8 additions & 0 deletions src/main/java/atdd/station/repository/StationRepository.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
package atdd.station.repository;

import atdd.station.domain.Station;
import org.springframework.data.jpa.repository.JpaRepository;
import org.springframework.stereotype.Repository;

@Repository
public interface StationRepository extends JpaRepository<Station, Long> {}
3 changes: 3 additions & 0 deletions src/main/resources/application.yml
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
spring:
h2:
console:
enabled: true
datasource:
driver-class-name: org.h2.Driver
url: jdbc:h2:mem:testdb
Expand Down
29 changes: 0 additions & 29 deletions src/test/java/atdd/station/StationAcceptanceTest.java

This file was deleted.

92 changes: 92 additions & 0 deletions src/test/java/atdd/station/controller/StationAcceptanceTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
package atdd.station.controller;

import org.junit.jupiter.api.Test;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.autoconfigure.web.reactive.AutoConfigureWebTestClient;
import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.http.MediaType;
import org.springframework.test.annotation.DirtiesContext;
import org.springframework.test.web.reactive.server.WebTestClient;
import reactor.core.publisher.Mono;

@DirtiesContext(classMode = DirtiesContext.ClassMode.BEFORE_EACH_TEST_METHOD)
@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT)
@AutoConfigureWebTestClient
public class StationAcceptanceTest
{
private static final Logger logger = LoggerFactory.getLogger(StationAcceptanceTest.class);

private final String STATIOIN_NAME = "강남역";
private final String BASE_URL = "/stations";
private final int TEST_ID = 1;

@Autowired
private WebTestClient webTestClient;

@Test

Choose a reason for hiding this comment

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

junit5의 경우 @DisplayName 애노테이션을 활용하여 의도를 드러낼 수도 있습니다.
https://www.journaldev.com/21674/junit-display-name-displayname

public void createStation()
{
String inputJson = "{\"name\":\""+ this.STATIOIN_NAME +"\"}";

webTestClient.post().uri(BASE_URL + "/create")
.contentType(MediaType.APPLICATION_JSON)
.body(Mono.just(inputJson), String.class)
.exchange()
.expectStatus().isCreated()
.expectHeader().contentType(MediaType.APPLICATION_JSON)
.expectHeader().exists("Location")
.expectBody().jsonPath("$.name").isEqualTo(this.STATIOIN_NAME);
}

@Test
public void list()
{
testCreateStation(STATIOIN_NAME);

webTestClient.get().uri(BASE_URL + "/list")
.exchange()
.expectStatus().isOk()
.expectHeader().contentType(MediaType.APPLICATION_JSON)
.expectBody().jsonPath("$.[0].name").isEqualTo(STATIOIN_NAME);
}

@Test
public void detailById()
{
testCreateStation(STATIOIN_NAME);

webTestClient.get().uri(BASE_URL + "/detail/" + TEST_ID)
.exchange()
.expectStatus().isOk()
.expectHeader().contentType(MediaType.APPLICATION_JSON)
.expectBody().jsonPath("$.name").isEqualTo(STATIOIN_NAME);

}

@Test
public void deleteStation()
{
testCreateStation(STATIOIN_NAME);

webTestClient.delete().uri(BASE_URL + "/delete/" + TEST_ID)

Choose a reason for hiding this comment

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

ID가 반드시 1임을 어떻게 보장할 수 있을까요
현재 DirtiesContext를 사용하기에 전체 테스트가 통과하나, 이는 ApplicationContext를 매번 재생성하기에 비용이 큰 테스트 방식입니다. F.I.R.S.T를 보장하기 위해서는 어떻게 작성하는 것이 좋을지 고민해보는 것도 좋을거 같아요.
https://pivotal.io/application-modernization-recipes/testing/spring-boot-testing-best-practices

.exchange()
.expectStatus().isOk();
}


public void testCreateStation(String stationName)
{
String inputJson = "{\"name\":\""+ this.STATIOIN_NAME +"\"}";
Comment on lines +79 to +81

Choose a reason for hiding this comment

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

메서드를 통해 인자로 받는다면 inputJson에서 이 값을 활용하여야 하지 않을까요
상수를 사용하는 부분과 인자로 받는 부분이 혼재되어 있어 actual과 expected가 분명히 드러나지 않는거 같아요.


webTestClient.post().uri(BASE_URL + "/create")
.contentType(MediaType.APPLICATION_JSON)
.body(Mono.just(inputJson), String.class)
.exchange()
.expectStatus().isCreated()
.expectHeader().contentType(MediaType.APPLICATION_JSON)
.expectHeader().exists("Location")
.expectBody().jsonPath("$.name").isEqualTo(stationName);
}
}