[1단계 - 상품 관리 기능] 성하(김성훈) 미션 제출합니다. #226
Conversation
| this.price = price; | ||
| } | ||
|
|
||
| public static ProductResponse fromEntity(final ProductEntity productEntity) { |
There was a problem hiding this comment.
Q : DTO에서 fromEntity, toEntity등의 변환 로직이 있어도 되는가?
원래는 DTO <-> Entity의 변환을 서비스 계층에서 진행해주려고 하다가,
서비스 계층은 '비즈니스 로직'을 담당하는데 변환 로직은 비즈니스 로직이라고 생각하지 않아서
DTO 내부에서 변환을 수행하도록 했습니다!
DTO는 계층의 정의대로 데이터 전달만 담당해야하는지,
편의를 위해 변환 로직을 추가해도 되는지 터틀의 생각이 궁금합니다!!
There was a problem hiding this comment.
저도 이러한 형태를 선호합니다. Service에는 비즈니스 로직의 흐름과 인프라와의 상호작용이 관심사이지 매핑으로 뒤덮여버리면 가독성이 떨어진다고 생각해요.
| } | ||
|
|
||
| @PostMapping("/product") | ||
| public ResponseEntity<Void> registerProduct(@RequestBody @Valid ProductRequest productRequest) { |
There was a problem hiding this comment.
Q : void 같이 반환을 줄 필요가 없을 때 Void VS String
현재 상품 등록, 수정, 삭제 API에서 반환을 Void로 하고 있는데,
받는 프론트 입장에서는 상태 코드만 보고 요청이 성공했는지 판단을 해야할 것 같았습니다.
그래서 String으로 "상품 등록이 성공했습니다!"와 같은 메시지를 주는 방법도 생각해보았는데
이러한 상황에서 현업에서는 어떻게 처리하시는지 궁금합니다!!
There was a problem hiding this comment.
성하가 말해주신 것 처럼 200번대 응답은 정상적인 응답을 나타내서 그 자체로 요청 성공에 대한 의미를 담고있어요. 문자열로 표현하려고 해주신 이유는 무엇인가요? 정상적으로 요청을 처리했는데 상품 등록에 성공하지 못한 경우가 어떤게 있을까요?
There was a problem hiding this comment.
처음에는 단순히 프론트단에서 받는 응답에 구체적인 내용이 없이 200 코드만 담기는게 찝찝했던 것 같아요!
그래서 1차적으로는 프론트단에서 상태코드만 보고 '상품 등록이 잘 성공했을까?'를 믿을 수 있을까? 라는 생각이 들었던 것 같아요.
2차적으로는, 200 상태 코드는 �해당 API 요청시에만 나타나는 코드가 아니라 다른 API 요청 시에도 나타날 수 있기 때문에 위험하다고 생각했어요!
지금 상황이나 이런 경우는 흔치 않을 것 같지만 예를 들어서 2개의 API가 있고,
URI가 같고 둘 다 200 상태 코드를 반환하는데 HTTP METHOD만 PUT, DLELTE 등으로 다른 경우를 생각해보았습니다!
이럴 경우에 프론트 단에서 PUT을 호출하는 것이 아니라 실수로 DELETE로 호출했을 때도 상태코드는 똑같이 200이기 때문에
PUT이 성공했다고 착각할 것 같다고 생각했습니다!
이럴 경우는 너무 발생하지 않을 일일까요?
| .build(); | ||
| } | ||
|
|
||
| public ProductEntity toEntityBy(long id) { |
There was a problem hiding this comment.
Q : DTO를 분리해야할까?
현재는 저장이랑 업데이트 시 DTO를 하나로 쓰고 있습니다!
저장 시에는 해당 DTO에서 toEntity를 사용해서 Entity로 변환하여 서비스에 전달합니다.
이때는 DTO의 필드로 id가 필요 없습니다.
그러나 업데이트 시에는 Entity로 변환하기 위해 DTO의 필드로 id가 필요하기 때문에 고민을 해봤습니다.
그래서 위와 같은 toEntityBy 메소드를 만들어서 id를 받아서 Entity를 만들게 되었습니다.
방안으로 저장 DTO와 업데이트 DTO를 나누는 방법도 생각해보았는데, 중복되는 부분이 많을 것 같아 적용하지 않았습니다!
이렇게 해도 괜찮을지 궁금합니다!!
There was a problem hiding this comment.
지금은 Id 필드 하나만 차이나지만 요구사항이 늘어나면 어떻게 될까요? 저는 점점 달라질 수 밖에 없다고 생각합니다. 그렇다면 지금은 우연한 중복이지 않을까요? SRP에 대해서도 학습해보면 좋을것같아요!
There was a problem hiding this comment.
DTO를 단순히 '데이터 전달 계층'으로 생각만 해봐서 SRP를 적용해야겠다는 생각을 못해본 것 같아요!!
좋은 인사이트 주셔서 감사합니다!
확실히 단일 책임을 가지고 있지 않다고 느껴지고, 단일 책임이 아니라서 추후 확장 시에도 변경 범위가 많아질 것 같아요!
더 크리티컬 한 것은 등록과 수정 DTO가 같은 경우가 많이 없을 것 같아요!
| @Test | ||
| void findAll() { | ||
| //given | ||
| productDao.insert(productEntity); |
There was a problem hiding this comment.
Q : Dao 테스트 시 다른 테스트에 의존해도 괜찮은건지?
현재 위의 테스트는 전체 상품을 조회하는 findAll()의 단위 테스트입니다!
그런데 조회 기능을 테스트하려고 하다보니 DB에 상품이 저장되어 있는 상황이 좋을 것 같았습니다.
그래서 given 절에 save를 호출해서 상품을 저장하고 테스트를 진행했습니다.
이렇게 하니 테스트하는 기능이 '조회' 기능인데 '저장' 기능이 성공한다는 전제 하에 테스트를 하기 때문에 완전한 '조회' 기능 테스트라고 하기가 조금 찝찝합니다 😭
이렇게 DB 조회 테스트 같이 다른 기능을 호출하여 테스트해야 하는 테스트는 어떻게 작성하시는지 궁금합니다!!
There was a problem hiding this comment.
저는 편의를 위해 이정도는 타협하는 편이긴 하지만, 이 부분이 마음에 들지 않으신다면 테스트용 데이터를 셋업해서 해결할 수도 있을 것 같아요. - https://www.baeldung.com/spring-boot-data-sql-and-schema-sql
하지만 테스트를 읽는 팀원이나 미래의 성하가 테스트의 컨텍스트를 파악할때 given에 대한 부분이 별도 파일로 분리되어있으니 한눈에 보기 힘들다는 단점도 있을 것 같네요.
There was a problem hiding this comment.
테스트용 데이터를 셋업하는 방법은 생각하지 못 했었는데 터틀 덕분에 알게 되었어요!! 감사합니다!! 😃
확실히 테스트용 데이터를 셋업하는 방법은 테스트용 데이터 셋업 비용과 다른 개발자의 이해 비용이 많이 들어간다고 느껴지네요!
저도 편의를 위해 타협을 하는 방법을 선택할 것 같아요!
|
|
||
| public class ProductRequest { | ||
|
|
||
| @Length(min = 1, max = 50, message = "이름은 1글자 이상 50글자 이하여야합니다.") |
There was a problem hiding this comment.
Q : 검증 책임 Controller(DTO) VS 도메인(VO)
이번 미션에서 검증 책임을 DTO와 VO 누가 가져야할지 상당히 고민을 많이 했었습니다!
처음에 나온 결론은 사용자 입력단에 가까운 검증(빈 값 입력, 공백 등)은 Controller(DTO)에서 처리하고,
이외의 검증들은 도메인(VO)에서 처리하도록 결론을 냈었습니다.
그러나 현재 미션에서 도메인 로직이 없기 때문에 VO를 검증만 하는 용도로 생성해야 했고,
단순히 VO를 만들고 다시 값을 꺼내서 사용하는 것이 너무 불필요하다고 생각이 들어서
모든 검증 로직을 Controller(DTO)에 옮기게 되었습니다.
터틀은 검증을 생각할 때 어떠한 기준으로 DTO와 VO에 각각 책임으로 분리하는지 궁금합니다!
There was a problem hiding this comment.
자동차경주 미션과 반대로 웹 미션으로 시작해서 콘솔 미션으로 변경하는 미션이라면 어떻게 될까요?
굳이 한 곳에서 검증이 이뤄져야한다면 도메인에서 이뤄지는게 맞다고 생각합니다. 그것이 비즈니스 로직이기도 하고요. VO를 만들고 꺼내기만 했다고 하시는걸 보니 ProductName과 같은 형태로 작게 분리하신것 같은데 검증 로직이 객체를 분리할 정도로 많지 않다면 Product 안에서 모든 검증이 이루어져도 괜찮다고 생각해요.
저도 사용자 입력에 대한 가벼운 검증을 애너테이션 기반으로 사용하긴 하지만 도메인에 있는 검증을 보완하기 위한 수단이지 대체하는 수단으로 사용하진 않습니다!
There was a problem hiding this comment.
저도 사용자 입력에 대한 가벼운 검증을 애너테이션 기반으로 사용하긴 하지만 도메인에 있는 검증을 보완하기 위한 수단이지 대체하는 수단으로 사용하진 않습니다!
이 부분 말씀이 정말 와닿았던 것 같아요!
도메인을 추가해서 해당 로직 안에 도메인 검증 로직이라고 판단되는 부분을 추가하겠습니다!!
| } | ||
|
|
||
| @PostMapping("/product") | ||
| public ResponseEntity<Void> registerProduct(@RequestBody @Valid ProductRequest productRequest) { |
There was a problem hiding this comment.
성하가 말해주신 것 처럼 200번대 응답은 정상적인 응답을 나타내서 그 자체로 요청 성공에 대한 의미를 담고있어요. 문자열로 표현하려고 해주신 이유는 무엇인가요? 정상적으로 요청을 처리했는데 상품 등록에 성공하지 못한 경우가 어떤게 있을까요?
| public ResponseEntity<Void> modifyProduct(@RequestBody @Valid ProductRequest productRequest, | ||
| @PathVariable long id) { | ||
| cartService.modifyById(productRequest, id); | ||
| return ResponseEntity.ok().build(); |
There was a problem hiding this comment.
Put 요청의 경우 200 응답도 사용하지만 204 응답을 사용하기도 한답니다. 지금같이 Void를 사용하는 경우엔 204가 더 적절할것같아요.
There was a problem hiding this comment.
204 상태 코드를 사용하긴 했지만, 제대로 모르고 있어서 이번 기회에 200과 함께 공부하게 되었습니다!
RFC 7231 공식문서를 보고 공부해봤습니다!
200 상태 코드 - PUT
PUT의 경우에 200 상태코드는 행위의 상태에 대한 resource 표현을 반환한다고 되어 있었습니다!
그래서 200을 사용하면 void가 아닌 수정이 되었다는 resource 표현을 반환해야 할 것 같아요!
204 상태 코드
204 상태코드는 다음과 같이 요약해봤습니다!
- 204 코드는 서버가 성공적으로 요청을 수행했고, 응답 payload에 담을 추가적인 내용이 없다는 의미이다.
- 204 Status를 사용하면 요청이 대상 resource에 성공적으로 적용되었음을 나타낸다.
- 또, 클라이언트가 현재 페이지에서 벗어날 필요가 없음을 의미한다.
- 서버는 클라이언트가 새 데이터나 업데이트된 데이터를 사용자에게 제공할 것을 기대한다.
그래서 응답 payload에 담을 추가적인 내용이 없는 void로 반환하므로, 204가 적당할 것 같습니다!!
공부할 키워드를 알려주셔서 감사합니다!! 😃
| @ExceptionHandler(MethodArgumentNotValidException.class) | ||
| public ResponseEntity<String> handleMethodValidException(MethodArgumentNotValidException exception) { | ||
| String errorMessage = exception.getBindingResult().getAllErrors().get(0).getDefaultMessage(); | ||
| log.error(errorMessage); |
There was a problem hiding this comment.
사용자의 입력 오류는 시스템의 에러는 아니지 않을까요? 적절한 로그 레벨을 지정해보면 어떨까요?
https://www.tutorialspoint.com/log4j/log4j_logging_levels.htm
There was a problem hiding this comment.
로그 레벨은 생각지도 못하고 단순하게 error로 처리했었는데, 관련 링크와
이동욱님이 최근에 쓰신 글을 발견해서 더 공부해보게 되었습니다!!
사용자의 입력 오류는 WARN이 맞는 것 같아서 WARN으로 리팩토링하겠습니다!!
| this.price = price; | ||
| } | ||
|
|
||
| public static class Builder { |
There was a problem hiding this comment.
엔티티는 DB 컬럼과 필드가 1:1으로 맵핑되어서 필드 수가 많아지는 편이어서 빌더 패턴을 이용했던 것 같습니다!
빌더 패턴을 사용하지 않고 생성자의 매개변수로 필드의 값들을 받아서 Entity를 생성하게 된다면
외부에서 Entity를 생성할 때 매개변수의 순서를 잘못 지정하여 생성할 수 있는 위험이 있을 것 같았습니다.
필드의 타입이 다 다르면 컴파일 시에 에러가 발생하지만,
지금과 같은 경우 name과 imgUrl이 String으로 같은 타입이므로 생성자에서 순서가 다르게 생성될 시
컴파일 에러가 생기지 않고 Entity가 생성되게 됩니다!
이러한 문제를 방지하고자 빌더 패턴을 사용한 것 같아요!
또, 생성자를 사용해서 Entity를 생성할 때 보다 빌더를 사용해서 Entity를 생성하는 것이
어떤 필드에 어떤 값이 담기는지 가독성 측면에서도 좋다고 느껴서 사용한 것 같습니다!
|
|
||
| public class ProductRequest { | ||
|
|
||
| @Length(min = 1, max = 50, message = "이름은 1글자 이상 50글자 이하여야합니다.") |
There was a problem hiding this comment.
자동차경주 미션과 반대로 웹 미션으로 시작해서 콘솔 미션으로 변경하는 미션이라면 어떻게 될까요?
굳이 한 곳에서 검증이 이뤄져야한다면 도메인에서 이뤄지는게 맞다고 생각합니다. 그것이 비즈니스 로직이기도 하고요. VO를 만들고 꺼내기만 했다고 하시는걸 보니 ProductName과 같은 형태로 작게 분리하신것 같은데 검증 로직이 객체를 분리할 정도로 많지 않다면 Product 안에서 모든 검증이 이루어져도 괜찮다고 생각해요.
저도 사용자 입력에 대한 가벼운 검증을 애너테이션 기반으로 사용하긴 하지만 도메인에 있는 검증을 보완하기 위한 수단이지 대체하는 수단으로 사용하진 않습니다!
| .build(); | ||
| } | ||
|
|
||
| public ProductEntity toEntityBy(long id) { |
There was a problem hiding this comment.
지금은 Id 필드 하나만 차이나지만 요구사항이 늘어나면 어떻게 될까요? 저는 점점 달라질 수 밖에 없다고 생각합니다. 그렇다면 지금은 우연한 중복이지 않을까요? SRP에 대해서도 학습해보면 좋을것같아요!
| this.price = price; | ||
| } | ||
|
|
||
| public static ProductResponse fromEntity(final ProductEntity productEntity) { |
There was a problem hiding this comment.
저도 이러한 형태를 선호합니다. Service에는 비즈니스 로직의 흐름과 인프라와의 상호작용이 관심사이지 매핑으로 뒤덮여버리면 가독성이 떨어진다고 생각해요.
|
|
||
| @BeforeEach | ||
| void setUp() { | ||
| this.mockMvc = MockMvcBuilders.webAppContextSetup(context) |
There was a problem hiding this comment.
standaloneSetup을 사용해보면 어떨까요? 어떤 차이가 있을까요?
There was a problem hiding this comment.
이 부분은 예외 테스트 시 예외 메시지가 깨지는 문제 때문에 설정해주게 되었습니다!
그래서 단순히 해결만 하고 자세히 알아보지는 못 했었던 것 같아요!!
터틀 덕분에 어떤 역할을 하는지 알게 되었습니다!! 😃
webAppContextSetup과 standaloneSetup은
모두 MockMvc의 테스트 전 셋업 코드를 작성할 때 사용합니다!
webAppContextSetup은 스프링에서 가져온 WebApplicationContext를 파라미터로 받아서 작동하기 때문에
다른 스프링 빈까지 등록된 상태로 테스트를 진행해서 통합 테스트에 적절하다고 공부했습니다!!
반대로 standaloneSetup은 사용할 컨트롤러 클래스를 파라미터로 받아서 작동하기 때문에
지정한 컨트롤러 클래스만 빈으로 등록된 상태로 테스트를 진행해서 단위 테스트에 적절하다고 공부했습니다!!
그런데 제 테스트에 standaloneSetup을 적용해보니, 다른 API 호출 테스트는 정상적으로 통과하지만
검증 예외 테스트에서 response body에 예외 메시지가 아니라 빈 값이 담겨서 검증 테스트가 실패했고,
ViewResolver도 동작하지 않는 것 같아서 View를 조회하는 테스트도 실패하더라구요 ㅠㅠ
구글링해도 해결 방법이 나오질 않아서 다시 webAppcContextSetup으로 놔두게 되었습니다... 😭
| @Test | ||
| void findAll() { | ||
| //given | ||
| productDao.insert(productEntity); |
There was a problem hiding this comment.
저는 편의를 위해 이정도는 타협하는 편이긴 하지만, 이 부분이 마음에 들지 않으신다면 테스트용 데이터를 셋업해서 해결할 수도 있을 것 같아요. - https://www.baeldung.com/spring-boot-data-sql-and-schema-sql
하지만 테스트를 읽는 팀원이나 미래의 성하가 테스트의 컨텍스트를 파악할때 given에 대한 부분이 별도 파일로 분리되어있으니 한눈에 보기 힘들다는 단점도 있을 것 같네요.
|
#226 (comment) 에 대해서는 전반적으로 잘 정리해주신것 같아요 👍🏻 하지만 저는 단위테스트의 경우 메서드 처럼 작은 단위의 테스트라고 생각해서 E2E테스트와 성하가 정리해준 인수테스트와는 어떻게 다를지도 한번 고민해보시면 좋을것같네요! 하지만 브라운의 조언처럼 매몰되지 않으셨으면 좋겠습니다 😊 |
안녕하세요 터틀!! 지난 자동차 경주 미션에 이어서 만나게 되어서 너무 반갑습니다!! 😃
첫 미션이었는데 터틀 덕분에 그 후 미션에서도 편하게 리뷰어분과 소통하게 되어서 감사하다고 말하고 싶었어요!!
먼저 이번 애플리케이션에서 페어와 정했던 선택 분기점에 대해서 말씀드리고,
왜 해당 구조를 선택했는지 이유를 적어보도록 하겠습니다!
이런 부분이 더 나을 것 같다, 틀리다 하는 부분을 집어서 답변 주시면 감사하겠습니다!!! 😃
🎯 선택 분기점
1. 상품 수정 HTTP method :
PUTVSPATCH에서 PUT으로 한 이유현재 구조의 상품 수정 Form을 봤을 때, 모든 기존 데이터를 요청보내기 때문에 PUT을 사용하게 되었습니다!
2.
@ResponseBodyVSResponseEntity: ResponseEntity를 사용한 이유가장 큰 이유는 POST(상품 등록) 시에 201 상태코드를 반환하는데,
그때 URI Location에서 저장된 Product의 SavedId를 PathVariable로 헤더에 담아서 테스트에서 사용하기 위해
ResponseEntity를 사용해서 Created의 URI를 지정해줬습니다.
@responsebody를 사용하면, @ResponseStatus를 사용해야하는데, 해당 어노테이션에서는 Created의 URI를 지정할 수 없었기 때문에
ResponseEntity를 사용했습니다.
이후에는 일관성을 위해서 ResponseEntity로 사용하게 되었습니다!
그리고 추후 확장이 됐을 때에도 ResponsEntity가 더 많은 옵션(헤더, 상태코드 등)을 담을 수 있기 때문에
기본적으로는 ResponseEntity를 사용하려고 합니다!
3. Domain을 만들지 않은 이유
지금은 도메인의 로직이 필요하지 않은 것 같아서 만들지 않았습니다.
Entity와 도메인을 동일시 한 것이 아니라, 도메인의 로직이 필요하지 않은 것 같아서 도메인을 따로 추가하지 않았습니다.
(검증 관련 부분은 이후 질문에 남기도록 하겠습니다!!)
4. 테스트 단에 applictaion.properties로 TestDB의 DataSource를 따로 만든 이유?
지금은 실제 프로덕션 DB가 인메모리라서 상관이 없지만,
추후에 외부 DB를 사용하게 되면 테스트 DB에 프로덕션 DB가 영향을 받을 것 같아서 따로 만들었습니다!
다음은 제가 이번 미션을 경험하고, 크루들과 토론하면서 제 나름대로 정립된 테스트 용어의 개념을 정리해보도록 하겠습니다!
브라운이 테스트 용어에 매몰되는 건 좋지 않다고 하셨지만, 다른 크루들과 의사소통 하기 위해 용어가 필요하더라구요!!
그래서 조금이나마 간략하게 정리해보게 되었습니다!
이런 부분은 아닌 것 같다, 틀린 것 같다 하는 부분을 집어서 답변 주시면 감사하겠습니다!!! 😃
🎯 테스트 용어 개념
✅ 1. 통합 테스트
✅ 2. 슬라이스 테스트
✅ 3. 단위 테스트
✅ 4. 인수 테스트
테스트 용어를 정리할 때 헷갈렸던 점이 크루들이 통합 테스트의 반대 개념으로 단위 테스트를 많이 언급하더라고요!
근데 저는 레벨 1때 자바에서 했던 단위 테스트 개념인 '하나의 기능 테스트'로 본다면 통합 테스트와 묶이는 것이 아니라고 생각했습니다.
그래서 통합 테스트의 반대 개념이 슬라이스 테스트라고 생각하고 있습니다!!
이 생각이 맞는지 궁금합니다!!
🎯 질문
나머지 질문할 것들은 코멘트 단에 달도록 하겠습니다!!
너무 질문이 많고 길어진 것 같아 죄송합니다 ㅠㅠ 😂
이번 장바구니 미션도 잘부탁드립니다 터틀!! 🙇🏻♂️