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

[1단계 - 상품 관리 기능] 제나(위예나) 미션 제출합니다 #189

Merged
merged 37 commits into from
May 1, 2023

Conversation

yenawee
Copy link

@yenawee yenawee commented Apr 26, 2023

안녕하세요 빙봉!
우테코 첫 미션 리뷰어로 만나고 웹 미션에서도 만나게 되었습니다 !! 너무 반가워요 😊

장바구니 1단계 미션 구현 완료해서 리뷰 요청 드립니다.

구현 중에 의문이 생겨서 아래 정리해서 공유드려요.

  1. controller <-> service 간 DTO 를 만들어야 할 필요가 있는지
    현재 AdminController 에서 createProduct 시 view <-> controller 간 ProductRequest 로 매핑해서 받아오는데 해당 객체를 다시 Service 로 보내주고 있습니다. 이러면 view 의 객체가 Service 에도 영향을 준다고 생각해서 controller 에서 값을 꺼내와서 Service 에 전달해주는 방법으로 수정을 했었는데 그러면 컨트롤러에서 Service 에서 사용되는 값들을 알게 되고, 코드도 깔끔하지 않아져서 다시 ProductRequest 를 사용하도록 원복했습니다. 새로 controller <-> service 간 DTO 를 만들면 ProductRequest 랑 동일한 코드구현을 갖게 될텐데 그래도 따로 구현해주는게 좋은지, 아니면 지금 상태로 놔두는게 좋을지 궁금합니다!

  2. controller 분리 기준을 어떻게 해야 하는지
    현재는 url 기준으로 AdminController 와 RootController 를 구분하였는데 index.html, admin.html 을 반환하는 메소드들은 @responsebody 어노테이션이 필요가 없어서 둘 다 @controller 어노테이션만 사용했습니다. 이럴때는 @RestController@controller 로 분리하는게 좋을지 아니면 현재처럼 기능(url?)별로 묶는 것이 좋은지 기준점이 궁금합니다 !

리뷰 잘 부탁드립니다 🙌

jaehee329 and others added 21 commits April 25, 2023 14:09
Co-authored-by: yenawee <[email protected]>
기존: image blob 으로 저장
변경 후: image url varchar 로 저장

Co-authored-by: yenawee <[email protected]>
Copy link

@aegis1920 aegis1920 left a comment

Choose a reason for hiding this comment

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

안녕하세요 제나! 빙봉입니다 🙂

다시 만나서 반갑습니다!ㅎㅎ 질문에 대한 답변 및 코멘트 드렸으니 확인해주세요!

궁금한 점이 있다면 편하게 DM 주세요~

Choose a reason for hiding this comment

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

해당 객체를 다시 Service 로 보내주고 있습니다. 이러면 view 의 객체가 Service 에도 영향을 준다고 생각해서

view의 객체는 아마도 ProductRequest를 의미하신 거겠죠? 이게 Service에도 영향을 준다고 하셨는데 어떤 영향을 주나요?

controller 에서 값을 꺼내와서 Service 에 전달해주는 방법으로 수정을 했었는데 그러면 컨트롤러에서 Service 에서 사용되는 값들을 알게 되고 코드도 깔끔하지 않아져서 다시 ProductRequest 를 사용하도록 원복했습니다.

컨트롤러에서 Service 에서 사용되는 값들을 알게 되면 안 좋은 점이 있는 뉘앙스로 말씀해주신 것 같은데요. 만약 그렇다면 어떤 점이 안 좋나요?

Copy link
Author

Choose a reason for hiding this comment

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

  • 다시 코드를 보면서 생각해보니 ProductRequest 가 거의 DTO 같은 역할을 하고 있고 Service 에서는 ProductRequest 값을 get 만 해서 다시 Entity 객체로 변환해 사용하고 있어서 Service 에 영향을 줄 가능성은 극히 적다고 생각이 듭니다..! 너무 레이어 간 데이터 전달은 서로 다른 객체를 사용해야 한다는 생각에 매몰되어서 실제로 그런지 생각을 덜 했던 거 같습니다 🥲

  • 컨트롤러는 사용자 응답을 받아와서 비즈니스 로직을 처리하는 서비스에 그 값을 전달해주는 역할만을 해야 한다고 생각합니다.

    • 컨트롤러에서 서비스에서 사용되는 값들을 알게 되면 두 레이어 간의 결합도가 높아져서 유지보수성과 확장성이 나빠집니다
    • 서비스는 핵심 비지니스 로직을 처리하는 레이어인만큼 민감한 데이터를 다루는 경우도 많은데 컨트롤러에 해당 데이터들이 공개될 수 있습니다

Choose a reason for hiding this comment

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

다시 코드를 보면서 생각해보니 ProductRequest 가 거의 DTO 같은 역할을 하고 있고 Service 에서는 ProductRequest 값을 get 만 해서 다시 Entity 객체로 변환해 사용하고 있어서 Service 에 영향을 줄 가능성은 극히 적다고 생각이 듭니다..!

넵! 말씀대로 ProductRequest는 DTO 역할을 하고 있는 게 맞습니다. 그리고 레이어 간 데이터 전달은 서로 다른 객체를 사용해야 한다는 생각도 인자를 전달할 때 의존성을 줄인다는 측면에서 맞습니다. 잘 생각하셨어요! 다만 상황에 따라 적절한 방법을 택해야할텐데 말씀해주신대로 Service에 영향을 줄 게 딱히 없다고 생각들어서 어떤 영향을 주는지 여쭤봤었습니다ㅎㅎ

조금만 첨언드리자면, CartService에 필요한 필드를 전달해주기 위해서 아래와 같은 방법들이 있을 것 같은데요.

  1. ProductRequest 객체를 그대로 넘겨주는 것
  2. ProductRequest 객체의 필드를 꺼내서 넘겨주는 것
  3. ProductRequest 객체의 필드를 꺼내서 다른 클래스(Service용 DTO)로 감싸 넘겨주는 것
class CartService {

  // 1
  void someLogic(ProductRequest productRequest) {

  }

  // 2
  void someLogic(String productRequest.a, int productRequest.b) {

  }
  
  // 3
  void someLogic(ServiceDto serviceDto) {

  }
}

static class ServiceDto {
  String a,
  int b
}

이런 방법들로 정의할 수 있을 것 같습니다. 각 방법마다 트레이드 오프가 있으며 상황에 맞게 적절하게 사용하시면 됩니다. 지금 상황에서는 민감한 데이터도 없고, 필드 수도 적고, 도메인이 복잡하지 않아서 1번, 2번, 3번 방법 모두 가능할 것 같은데요. CartService에서 ProductRequest를 의존하는 게 신경쓰이신다면 2번 혹은 3번 방법을 택해도 좋을 것 같습니다.

컨트롤러에서 서비스에서 사용되는 값들을 알게 되면 두 레이어 간의 결합도가 높아져서 유지보수성과 확장성이 나빠집니다

맞습니다! 👍 👍
그러지 않기 위한 방법은 3번으로 아예 의존성을 제거하는 것입니다. 다만 매번 새로운 클래스를 만들게 되므로 그에 따른 단점도 존재한다는 걸 기억해주세요!

Choose a reason for hiding this comment

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

controller 분리 기준을 어떻게 해야 하는지
현재는 url 기준으로 AdminController 와 RootController 를 구분하였는데 index.html, admin.html 을 반환하는 메소드들은 @responsebody 어노테이션이 필요가 없어서 둘 다 @controller 어노테이션만 사용했습니다. 이럴때는 @RestController@controller 로 분리하는게 좋을지 아니면 현재처럼 기능(url?)별로 묶는 것이 좋은지 기준점이 궁금합니다 !

따로 기준이 있는 건 아닙니다. @RestController는 단순히 @Controller@ResponseBody가 있는 거라 필요한 상황에 따라 알맞게 쓰면 됩니다. 지금은 말씀대로 RespnoseBody 어노테이션이 쓸 상황이 아니라면 안 써도 됩니다. 잘 구현하셨어요! 👍

Copy link
Author

Choose a reason for hiding this comment

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

리팩토링을 하면서 다른 크루들 코드를 구경하다보니 View 를 반환하는 컨트롤러와 API 작업을 수행하는 컨트롤러를 구분하는 경우가 많이 보였습니다!
확실히 View 반환하는 컨트롤러만 반환값이 다르기도 하고 추후에 API 가 추가되었을때나 View 페이지 변경이 필요할 때 컨트롤러를 나눠서 관리하는게 수정에 용이할 거라고 생각이 들어서 저도 ViewController 와 AdminController 로 컨트롤러 분리를 해봤어요!

Choose a reason for hiding this comment

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

일관성과 유지보수 측면에서 더 낫겠군요 👍

this.cartService = cartService;
}

@PostMapping("/product")

Choose a reason for hiding this comment

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

개인적인 취향인데요. 저는 @RequestMapping을 없애고 중복이 되더라도 URL을 분리하지 않고 한 번에 쓰는 편입니다. (e.g. @PostMapping("/admin/product"))
그래야 찾기 편하더라구요ㅎㅎ (물론 분리해도 shift + shift로 뜨는 창에서 /url 검색할 URL 이런 식으로 찾을 수 있긴 합니다)

}

@GetMapping
public String getProductList(final Model model) {

Choose a reason for hiding this comment

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

Products가 아니라 ProductList인 이유가 있을까요?

Copy link
Author

@yenawee yenawee Apr 30, 2023

Choose a reason for hiding this comment

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

정말 단순히 상품목록 페이지라서 메소드명을 productList 라고 지었었습니다 😂
자바에서 배우는 것처럼 List 를 반환했으면 자료구조인 List 를 지우고 Products 라고 반환했을 텐데 String 으로 view 가 보여주는 것을 메소드명에 담으려다 보니 너무 직독직해를 해버렸어요 ㅎㅎ

그런데 지금 리팩토링을 하면서 보니 GetMapping 을 했을 때 반환되는 view 의 이름과 매칭을 시켜주려면 getIndex, getAdmin 이 더 메소드가 하는 일을 잘 나타낸다고 생각해서 해당 이름으로 변경을 해봤어요!! 빙봉은 해당 메소드명도 괜찮다구 보시는지 궁금합니다!
현재는 index.html이 상품목록 페이지를 보여주고 있지만 추후에 페이지에 다른 대표 화면이 생겨서 index.html 이 생기면 getProducts 로 바꾸면 좋을 거 같아요!

Choose a reason for hiding this comment

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

넵 좋습니다! 👍 근데 또 드는 생각이 getIndexView, getAdminView 처럼 View라는 걸 강조하는 것도 괜찮을 것 같네요?

}

@GetMapping
public String getProductList(final Model model) {

Choose a reason for hiding this comment

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

  • Model이 반환되는 과정을 말씀해주실 수 있을까요?
  • Model이 아닌 ModelAndView를 써보는 건 어떨까요?

Copy link
Author

Choose a reason for hiding this comment

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

  • Controller 에서 반환된 view name 이 RequestMappingHandlerAdaptor 의
public final ModelAndView handle(HttpServletRequest request, HttpServletResponse response, Object handler)
			throws Exception {};

메소드에서 호출되는

private ModelAndView getModelAndView(ModelAndViewContainer mavContainer,
			ModelFactory modelFactory, NativeWebRequest webRequest) throws Exception {};

를 통해 Model 과 View 가 ModelAndView 객체로 합쳐져
반환된 ModelAndView 객체가 DispatcherServlet 에 전달되고 viewResolver 를 통해 view 를 렌더링을 하게 됩니다.

  • ModelAndView 키워드를 학습해보고 장단점을 생각해보았습니다.
    • 장점

      • 반환되는 값이 Model 과 View 라는 것을 명확하게 보여줄 수 있다
    • 단점

      • 프레임워크에서 view 를 렌더링하는 과정에서 ModelAndView를 만들어주는데 컨트롤러에서 ModelAndView 객체를 생성해서 넘겨줄 필요가 있을까 싶습니다! (기존 코드와 ModelAndView 를 사용하는 코드를 스프링이 처리하는 과정을 디버깅으로 코드를 따라가봤는데 ModelAndView 를 반환해주더라도 스프링에서 새로 객체를 할당해서 Model 과 View 를 세팅해주는 것을 확인했습니다.)
    • 결론적으로 저는 ModelAndView를 사용했을 때 장점보다 단점이 더 크다고 생각해서 적용하지 않았는데 빙봉의 생각은 어떠신가요?? 새로운 키워드 알려주셔서 스프링 코드도 분석해볼 수 있어서 재밌었습니다 😊

Choose a reason for hiding this comment

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

  • Model이 반환되는 과정을 말씀해주실 수 있을까요? 이 질문은 뷰를 렌더링하는 과정에 대해 한 번 디버깅해보셨으면 좋겠다는 의미에서 질문드렸는데요. 정말 잘 알고계시네요! 잘 설명해주셨어요! 👍 👍
  • Model이 아닌 ModelAndView를 써보는 건 어떨까요? 이 질문은 반환하는 방법에는 다양한 방법이 있으니 한 번 작성해보면 어떨까? 하는 취지였습니다ㅎㅎ Controller에서 Model, ModelAndView 뿐만 아니라 다양한 형태로 반환시켜줄 수 있는데요. 시간되시면 org.springframework.web.servlet.View 인터페이스를 상속한 여러 View를 살펴보셔도 도움이 될 것 같습니다 🙂

결론적으로 저는 ModelAndView를 사용했을 때 장점보다 단점이 더 크다고 생각해서 적용하지 않았는데 빙봉의 생각은 어떠신가요??

저는 Model보단 ModelAndView가 좀 더 명시적인 코드라고 생각이 들어서 개인적으로 ModelAndView를 더 선호하긴 합니다ㅎㅎ (개인 취향 영역이니 바꾸지 않으셔도 됩니다!)

return productDao.save(productEntity);
}

public List<ProductResponse> read() {

Choose a reason for hiding this comment

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

리스트 조회는 보통 All을 많이 붙입니다. e.g. readAll

}

public void update(final long id, final ProductRequest productRequest) {
final ProductEntity productEntity =
Copy link

@aegis1920 aegis1920 Apr 26, 2023

Choose a reason for hiding this comment

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

  • 해당하는 id가 없으면 어떻게 되나요?
  • 예외가 발생했을 때 어떻게 되나요?
  • 예외가 발생했을 때 어떻게 처리해야할까요?

Copy link
Author

@yenawee yenawee May 1, 2023

Choose a reason for hiding this comment

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

  • 해당하는 id 가 없으면 예외가 발생합니다!!
  • 예외가 발생했을 때 적절히 처리가 되지 않으면 client 쪽에서는 페이지 로딩이 안되고 서버가 중지될 예외가 아닌데도 서버가 죽을 수 있으며 적절한 로깅이 되어있지 않으면 디버깅도 어려워질 수 있습니다.
  • ControllerAdvice 를 사용해서 ExceptionHandling 을 하였습니다

}

public void delete(final long id) {
productDao.deleteById(id);

Choose a reason for hiding this comment

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

  • 마찬가지로 해당하는 id가 없으면 어떻게 되나요?

@@ -0,0 +1,9 @@
CREATE TABLE product
(
product_id BIGINT NOT NULL AUTO_INCREMENT,

Choose a reason for hiding this comment

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

autoincrement의 경우, unsigned를 사용하면 범위를 더 넓힐 수 있습니다.

Choose a reason for hiding this comment

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

가장 중요한 서비스 레이어 테스트가 없군요! 추가하지 않은 이유가 있을까요?

Copy link
Author

Choose a reason for hiding this comment

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

Service 레이어에서 수행한 일이 Entity 객체를 만들어서 Dao 의 메소드를 호출한 일 밖에 없어서 Dao 테스트와 중복된다고 생각해서 추가하지 않았습니다!
근데 Mock 을 사용해서 Service 가 실제로 기대되는 행동을 하고 있는지 검증을 할 수 있다는 것을 학습했고 해당 방식으로 Service Test 를 추가해보았습니다 !!

Choose a reason for hiding this comment

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

  • 서비스 레이어는 비즈니스 로직이 존재하는 레이어로 다른 레이어보다 테스트 중요도가 훨씬 높습니다!
  • 도메인이 너무 간단해서 DAO만 호출하는 경우라도 DAO를 테스트하는 것이 아닌 서비스 레이어를 테스트하는 것이 유지보수 측면에서 테스트를 수정하기 더 나을 것 같습니다 🙂 (비즈니스 로직은 어차피 서비스 레이어에 들어가니)

@yenawee
Copy link
Author

yenawee commented May 1, 2023

안녕하세요 빙봉!!
1단계 리뷰를 빨리 받았는데 리팩토링 후 재제출이 늦었습니다 😭
제출한 코드에 수정할 부분이 많았고 스프링 강의를 듣던 걸 완료하고 수정하고 싶어서... 많이 늦어버렸네요 ㅠㅠ

피드백 주신거 중점으로 수정해보았고 아래는 리팩토링 진행하면서 궁금했던 사항들입니다!!

  1. 우선 지금 Production 과 Test 용 데이터베이스를 분리해봤습니다! 근데 Production DB 에는 초기 실행 시 더미 데이터를 집어넣어주고 싶어서 data.sql 에 정의해놓았는데요! 현재 sql.init.mode가 always 라서 data.sql 이 어플리케이션을 실행할때마다 더미데이터가 들어갑니다! 해당 설정을 하지 않으면 schema.sql 도 초기에 실행이 안되고 다른 설정 조건인 embedded, never 도 제가 원하는 설정값이 아닙니다.

그래서 궁금한 점은 현재 설정에서 data.sql 이 초기에만 실행되게 바꿀수 있을까요?
또 실제 업계에서 사용되는 Production DB 에는 더미데이터를 어떻게 집어넣어주는지 궁금합니다!

  1. 또 Test DB 관련 문제입니다! 여기서 해결하고 싶어서 시간이 좀 걸렸는데 결국 해결하지 못했습니다.. Test DB 를 분명히 분리해뒀는데 main/resources/data.sql 도 함께 실행이 되어서 문제입니다.. 데이터 테이블은 잘 분리가 된걸 확인했는데 왜 초기데이터가 들어가는지 의문입니다🤔 그래서 현재 Test code 중 findAll() 이 fail 이 되고 있습니다.

  2. 마지막으로 update 와 delete 를 할때 존재하지 않는 Id 일 때를 대비해서 ControllerAdvice 에서 예외 핸들링을 하게 처리했습니다!

  • 먼저 findById()를 사용해서 한번 확인을 해주고 없는 Id 면 에러를 날리고 존재하는 Id 일때만 delete, update 호출
  • 바로 update, delete 메소드를 호출해서 없는 Id 이면 바로 해당 메소드에서 예외 발생하게 처리
    요 두가지 중에 어떤 방식이 더 자주 사용되고 나은 방식일까요?? 저는 첫번째 방식은 쿼리를 두번 날릴 수 있다 생각해서 두번째 방식으로 처리했는데 첫번째 방식도 많이 사용하시는 거 같아 빙봉의 의견이 궁금해졌습니다!

꼼꼼한 리뷰 덕분에 많이 배웠습니다
이번에도 잘부탁드립니다 🌱

Copy link

@aegis1920 aegis1920 left a comment

Choose a reason for hiding this comment

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

안녕하세요 제나! 빙봉입니다 🙂

1단계 구현하시느라 고생 많으셨습니다! 👍 질문에 대한 답변 및 추가 코멘트 남겼습니다. 추가 코멘트는 2단계를 진행하면서 반영하셔도 될 것 같아요.

1단계는 여기서 머지하겠습니다! 수고하셨어요!

@@ -0,0 +1,4 @@
INSERT INTO product(name, price, image_url)

Choose a reason for hiding this comment

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

궁금한 점은 현재 설정에서 data.sql 이 초기에만 실행되게 바꿀수 있을까요?

이미 알고계실 수 있겠지만 문제가 되는 지점부터 살펴볼게요.

  • data.sql과 schema.sql은 스프링 부트가 시작되면 모두 자동으로 실행됩니다.
  • 현재 코드에서 main의 application.properties를 보면 spring.datasource.url=jdbc:h2:./db/shopping-cart;MODE=MySQL 로 되어있는데요. h2의 db를 로컬에 특정하고 있습니다.
  • 즉, 매번 같은 경로의 로컬 DB 봅니다.
  • 그래서 기존에 있던 데이터가 지워지지 않고 스프링 부트를 실행할 때마다 data.sql과 schema.sql이 실행되었습니다. schema.sql은 IF NOT EXISTS가 있어서 제외되었고 data.sql은 매번 실행되어 스프링 부트를 실행할 때마다 데이터가 추가되어 들어갑니다.
  • 스프링 부트를 몇 번 실행시켰는지 외부에서 데이터를 저장하지 않는 이상 data.sql이 초기에만 실행되게 바꿀 수 있는 방법은 없을 것 같습니다.

이를 수정하기 위한 방법으로 두 가지가 떠오릅니다.

  1. h2의 DB 경로를 로컬 DB로 바라보지 않게 하고 메모리에 올립니다.

특별한 이유가 없다면 메모리에 올려도 된다고 생각해서요. spring.datasource.url=jdbc:h2:mem:testdb;MODE=MySQL 로 가도 되지 않을까요?

  1. INSERT IGNORE 구문을 사용합니다.
INSERT IGNORE INTO product(product_id, name, price, image_url)  
VALUES (1, '스프링', 1000, 'https://previews.123rf.com/images/marucyan/marucyan1211/marucyan121100106/16114832-새싹.jpg'),  
(2, '빙봉', 10000,  
'https://mblogthumb-phinf.pstatic.net/MjAxNjEyMTNfMTk4/MDAxNDgxNjAwMDE1ODM1.mxPz7FBOV9DF5ryoeYEAcfZfBwMeFOYp-p03N7RAO68g.ICYGpmoaW0Q0wSenF5JQzQ4LZ3aqRWazx66DTqYgJFcg.JPEG.jeenyjin/Screenshot_2016-12-12-13-48-12-1.jpg?type=w800');

위와 같이 작성해서 data.sql이 반복 실행되더라도 무시해줄 수 있습니다.

또 실제 업계에서 사용되는 Production DB 에는 더미데이터를 어떻게 집어넣어주는지 궁금합니다!

똑같습니다. Production DB에 더미데이터를 넣을 때는 INSERT 구문을 통해 넣습니다. 특별히 좀 더 신경 쓸 부분이라면 insert를 반복하면 성능 상 문제가 있을 수 있으니 batch insert를 쓴다는 정도입니다.

@@ -0,0 +1,3 @@
spring.h2.console.enabled=true
spring.datasource.url=jdbc:h2:mem:testdb;MODE=MySQL
spring.datasource.driver-class-name=org.h2.Driver
Copy link

@aegis1920 aegis1920 May 1, 2023

Choose a reason for hiding this comment

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

또 Test DB 관련 문제입니다! 여기서 해결하고 싶어서 시간이 좀 걸렸는데 결국 해결하지 못했습니다.. Test DB 를 분명히 분리해뒀는데 main/resources/data.sql 도 함께 실행이 되어서 문제입니다.. 데이터 테이블은 잘 분리가 된걸 확인했는데 왜 초기데이터가 들어가는지 의문입니다🤔 그래서 현재 Test code 중 findAll() 이 fail 이 되고 있습니다.

org.springframework.boot.autoconfigure.sql.init.SettingsCreator의 scriptLocations 메서드에 브레이크 포인트를 걸고 디버깅해보시면 알 수 있습니다.

private static List<String> scriptLocations(List<String> locations, String fallback, String platform) {  
if (locations != null) {  
return locations;  
}  
List<String> fallbackLocations = new ArrayList<>();  
fallbackLocations.add("optional:classpath*:" + fallback + "-" + platform + ".sql");  
fallbackLocations.add("optional:classpath*:" + fallback + ".sql");  
return fallbackLocations;  
}

locations이 설정되어있으면 그 locations를 리턴하고, locations가 설정되어있지 않으면 기본적으로 data-all.sqldata.sql 이 들어가게 됩니다.

초기 데이터를 넣지 않으려면 application-test.properties에서 spring.sql.init.data-locations= 를 넣어보세요. 해당 값에 아무것도 넣지 않은채로 세팅해주면 size가 0인 locations을 내보내기 때문에 data.sql을 찾지 않아 테스트가 통과합니다.

import java.util.stream.Collectors;

@Service
public class CartService {

Choose a reason for hiding this comment

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

먼저 findById()를 사용해서 한번 확인을 해주고 없는 Id 면 에러를 날리고 존재하는 Id 일때만 delete, update 호출. 바로 update, delete 메소드를 호출해서 없는 Id 이면 바로 해당 메소드에서 예외 발생하게 처리. 요 두가지 중에 어떤 방식이 더 자주 사용되고 나은 방식일까요?? 저는 첫번째 방식은 쿼리를 두번 날릴 수 있다 생각해서 두번째 방식으로 처리했는데 첫번째 방식도 많이 사용하시는 거 같아 빙봉의 의견이 궁금해졌습니다!

  • 일단 현재 코드에서 DB에는 존재하지 않는 id의 update, delete 메서드를 호출하면 예외가 발생하는지 확인 한 번 부탁드려요.
    • jdbcTemplate.update()를 실행하면 어떤 게 return 될까요?
  • 특정 도메인에 대한 데이터가 존재하지 않아서 발생하는 에러는 DB에서 관리하는 게 맞을까요? 아니면 서비스 레이어에서 관리하는 게 맞을까요? 이것에 대한 고민을 해보셨으면 좋겠습니다.

Copy link
Author

Choose a reason for hiding this comment

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

  • 앗.. EmptyResultDataAccessException 은 jdbcTemplate.queryForObject() 에서만 발생하고 jdbcTemplate.update() 에서는 발생하지 않는 예외였습니다.. 코드 수정하고 테스트를 안해본게 여기서 티가 났네요 😂 jdbcTemplate.update() 실행시 반환되는 값은 해당 쿼리를 실행함으로써 영향을 받은 행의 수 입니다. 그래서 영향을 받은 행의 수가 1 이 아닐때 Service 에서 예외를 던지도록 수정했습니다!
  • 특정 도메인에 대한 데이터이므로 비지니스 로직을 처리하는 서비스 레이어에서 관리하는게 맞다고 생각합니다!

import static org.assertj.core.api.AssertionsForClassTypes.assertThatThrownBy;

@JdbcTest
@AutoConfigureTestDatabase(replace = AutoConfigureTestDatabase.Replace.NONE)

Choose a reason for hiding this comment

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

잘 사용해주셨군요ㅎㅎ 👍 전에 리뷰했던 크루 중에 관련 이슈로 분석을 한 적이 있었는데요. 궁금하시면 보시는 걸 추천드립니다. (링크)

@ExtendWith(MockitoExtension.class)
@SuppressWarnings("NonAsciiCharacters")
@DisplayNameGeneration(DisplayNameGenerator.ReplaceUnderscores.class)
class CartServiceTest {

Choose a reason for hiding this comment

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

Service 테스트에 대해서 Mocking하는 게 좋을 지 통합 테스트를 하는 게 좋을지 고민해보시는 걸 추천드립니다 🙂

Copy link
Author

Choose a reason for hiding this comment

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

서비스는 중요 비지니스 로직을 다루니 통합 테스트를 작성하는게 프로그램의 안정성을 높여줄거같습니다 2단계에는 Dao 테스트 대신 서비스 테스트를 작성해서 Dao 도 예상대로 동작하는지 함께 테스트해봤습니다 !

Comment on lines +42 to +43
@Override
public boolean equals(Object o) {

Choose a reason for hiding this comment

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

Response에 대해 equals and hashCode를 구현해주신 이유가 있을까요?

Copy link
Author

Choose a reason for hiding this comment

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

 @Test
    void 전체_상품을_조회한다() {
        //given
        List<ProductEntity> givenProducts = List.of(new ProductEntity("jena", 90, "http://naver.com"),
                new ProductEntity("modi", 50, "http://daum.com"));
        given(productDao.findAll()).willReturn(givenProducts);

        //when
        List<ProductResponse> products = cartService.readAll();

        //then
        assertThat(products).isEqualTo(givenProducts.stream()
                .map(ProductResponse::from)
                .collect(Collectors.toList()));
    }

이 테스트를 실행할 때 productDao.findAll() 은 ProductEntity 를 반환하고 cartService.readAll() 은 ProductResponse 를 반환해서 equal 을 검증해주려면 equals, hashCode 를 재구현해주었습니다! ProductResponse 도 약간 Entity 느낌이라서 필드의 값들이 같으면 같은 객체라고 판단해도 된다고 생각했습니다 !!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants