[Spring 장바구니 - 2단계] 연로그(권시연) 미션 제출합니다. #95

yeon-06 · 2022-06-08T06:08:18Z

안녕하세요 범블비~!
리팩터링할 API가 많기도 했고 서버에 배포해 테스트까지 해보느라 PR이 많이 늦어졌습니다😅
이번 단계에도 잘 부탁드립니다!

2단계

이번 단계에서는 아래와 같은 일들을 했습니다

기존 레거시 API 리팩터링
- 최대한 기존 코드를 주석 처리하지 않고 리팩터링하려 노력했습니다.
- 팀과 합의한 API 명세서에 맞춰 url, request, response을 변경했습니다.
EC2 배포
MySQL으로 전환
CORS 에러 제거

- JdbcTemplate -> NamedParameterJdbcTemplate 전환 - Domain을 return 하는 대신 Entity return - 공용으로 사용할 수 있는 RowMapper 생성 - Controller의 반환값 dto로 변경 - Dao에 파라미터로 전달하는 데이터 Domain 또는 DTO 대신 Entity로 변경

- 메서드명 한글 허용 - 정확한 값 검증

@transactional

- 파라미터 Entity가 아닌 DTO로 변경 - 메서드명 변경 - @transactional 설정 변경 - 불필요한 ResponseEntity 반환 제거 - HTTP 응답 코드 설정

- 테스트 코드 리팩터링

@transactional

- @transactional 옵션 변경 - findByAccount 로직 개선 - delete 시 불필요한 검증 제거 - ProductDao id 목록으로 정보 조회하는 메서드 추가

- API 명세서와 request/response 형식 일치 - order test 주석

- 생성 시 존재하는 Product 인지 확인 - 조회 시 존재하는 Customer 인지 확인 - 테스트 케이스 추가 - CartItem 잘못 매칭되는 현상 개선

- 타 DAO 제거 - 메서드명 변경 - @beforeeach 제거

- 테스트 케이스 추가 - @beforeeach 제거 - 메서드 분리 - 타 DAO 제거

- 메서드 분리 - 불필요한 메서드 호출 제거

@transactional

- @transactional 옵션 변경 - 메서드명 변경 - 불필요한 sql 호출 제거

- 사용하지 않는 예외 제거 - 테스트 코드 수정

ddaaac

안녕하세요 연로그 👋

잘 변경해주셔서 크게 리뷰할 부분이 많지는 않네요.
커밋 단위를 작게 쪼개주셔서 파일 변화가 많음에도 불구하고 리뷰하기 편했습니다 👍

몇 가지 코멘트와 질문 남겼으니 확인 부탁드려요!

ddaaac · 2022-06-08T14:58:40Z

src/main/java/woowacourse/shoppingcart/application/OrderService.java

+@Transactional
 public class OrderService {


조금 번거롭고 시간이 부족할 수 있지만 기존 클래스를 리팩토링할 때는 테스트 코드가 없다면 먼저 작성하고 코드를 변경하는 순서대로 리팩토링을 하면 안정적으로 리팩토링을 할 수 있어요.
기존 코드에 대한 테스트 코드를 작성하며 기존 코드의 동작을 파악하고, 코드를 변경해도 테스트가 통과하는 것을 확인하며 동작이 제대로 되는지 확인할 수 있는 장점이 있습니다.
커밋 내역을 따라가다보니 테스트 코드를 작성하지 않은 부분이 보이는데요, 이런 부분을 한 번 미션을 통해서 느껴보시면 좋을 것 같습니다 😄

기존 코드를 얼른 바꾸고 싶은 마음에 TDD를 못지킨거 같아서 뜨끔하네요😅💦
후반에는 의식적으로 테스트 코드 작성하고 기존 코드를 수정하는 방향으로 진행하려고 노력은 했는데 마감이 촉박한 상황이라 성공하는 테스트만 짜고 리팩터링한거 같아요
배포 후에야 발견한 오류들도 있었어서 좀 더 다양한 상황을 고려해서 테스트를 짤걸 하는 아쉬움이 있네요ㅠ_ㅠ
의견 감사합니다 ㅎㅎ

ddaaac · 2022-06-08T15:06:13Z

src/main/java/woowacourse/shoppingcart/dao/entity/ProductEntity.java

+
+import woowacourse.shoppingcart.domain.product.Product;
+
+public class ProductEntity {


이 부분은 궁금한 점인데, 현재 도메인이랑 엔티티들이 거의 일치하는 것처럼 보이는데 분리해주신 이유가 있을까요?

새로운 상품을 생성할 때 Product 생성자를 통해 값을 검증해준다 (문자열 길이, 형식 등)

Product에 로직이 추가되는 경우 기존 코드를 많이 바꿀 필요가 없다.

다른 개념들과 형식을 통일한다.

위와 같은 사유로 분리했습니다😊

CartItem을 보면 엔티티만 존재하고 도메인은 존재하지 않는 거 같아요. 통일의 의미를 살리려면 CartItem도 형식을 맞춰줘야하지 않을까요?

Cart를 활용하도록 수정했습니다!
다만 원래 기존에는 dto에서 domain을 생성하도록 로직을 짰었는데요
이번에는 일부 값은 토큰에서 꺼내오고 일부 값은 DB에서 꺼내오는 현상이 발생했습니다
어쩔 수 없이 Cart는 서비스에서 생성하도록 만들었습니다!

ddaaac · 2022-06-08T15:08:28Z

src/test/java/woowacourse/shoppingcart/dao/ProductDaoTest.java


 @JdbcTest
 @AutoConfigureTestDatabase(replace = Replace.NONE)
 @Sql("classpath:schema.sql")
 @TestConstructor(autowireMode = TestConstructor.AutowireMode.ALL)
-public class ProductDaoTest {
+@DisplayNameGeneration(DisplayNameGenerator.ReplaceUnderscores.class)
+@SuppressWarnings({"NonAsciiCharacters"})


Suppress도 사용해주셨군요 👍
개발자가 의도하고 무시하는 경고에 대해서는 suppress 해주는 습관이 필요한 것 같습니다.

ddaaac · 2022-06-08T15:13:33Z

src/main/java/woowacourse/shoppingcart/domain/product/vo/Price.java

+ }
+
+ private void validate(int value) {
+ if (value <= 0) {


여기도 정의해놓은 MIN_VALUE 상수를 통해서 표현할 수 있지 않을까요?

ddaaac · 2022-06-08T15:15:56Z

src/main/java/woowacourse/shoppingcart/domain/customer/Customer.java

@@ -15,13 +15,13 @@ public class Customer {
 private Address address;
 private PhoneNumber phoneNumber;

- public Customer(Long id, Account account, Nickname nickname, Password password, Address address,
+ public Customer(Long id, String account, String nickname, Password password, String address,


값 객체를 사용해서 얻을 수 있는 이점 중에는 파라미터의 혼동이 없어진다는 점이 있습니다.
연속된 String으로 받는다면 account와 nickname의 순서가 바뀌어도 컴파일러로 통과할 것 같아요.
그래서 전 값 객체를 파라미터로 받는 방식을 유지하면 좋을 것 같은데 연로그의 생각은 어떠신가요?

일단 값 객체에서 String으로 바꾼 이유는 다른 객체에서 도메인으로 전환해야하는 로직 때문입니다
저는 현재 dto나 entity에서 도메인으로 전환하는 로직을 갖고 있는데 해당 객체들이 도메인의 구성 요소는 무엇인지 하나하나 알고 있는게 마음에 걸렸어요

다만 범블비가 말씀해주신 것처럼 순서가 바뀌어서 오류를 실제로 겪은 적이 있긴 해요ㅠ_ㅠ
이런 경우에는 빌더 패턴을 적용한다면 파라미터의 혼동이 줄어들지 않을까 싶은데 빌더 패턴에 대해서는 어떻게 생각하시나요?

값 객체가 아닌 원시 타입을 사용하면 외부에서 도메인의 구성 요소가 무엇인지 모르는걸까요? 생성자 파라미터에 타입을 두는 것이 외부에 구성 요소를 노출하는건지도 한 번 생각해보시면 좋을 것 같아요.

저는 값 객체를 사용할 거라면 값 객체에서 값이 꺼내지는 시점은 양 끝 단이어야 한다고 생각해요. 요청이 들어올 때 가장 앞단(dto나 컨트롤러)에서 포장해주고, 어플리케이션에서는 최대한 감싸진 값 객체를 사용하고, 그 값을 꺼내쓰는 시점은 DB에 저장할 때 같은 가장 끝에서 이뤄져야한다고 생각합니다. 연로그의 생각은 어떠신가요?

빌더 패턴을 사용할 수도 있지만 값 객체를 이미 도입하고 사용하고 있으니 값 객체를 사용해도 괜찮을 거 같아요.

범블비 말씀을 계속 곱씹어보니 도메인의 구성 요소를 알게 되는 것과 생성자의 파라미터는 상관이 없을 것 같아요. 애초에 어떤 타입이 존재하는지 알면 안되는 이유가 없고요 (불필요한 getter 남발 등에서 내부 데이터를 노출하지 말자는 말이 이런 곳에서 쓰이는 말이 아닌데 순간 헷갈렸던 것 같아요😅) 범블비의 의견에 동의합니다!
👉 값 객체를 사용하도록 다시 롤백했습니다!

ddaaac · 2022-06-08T15:24:55Z

src/main/java/woowacourse/auth/ui/AuthenticationInterceptor.java

+ if (HttpMethod.OPTIONS.matches(request.getMethod())) {
+ return true;
+ }
+


이 부분으로 CORS 에러를 처리하셨군요.
이 부분은 궁금한 점인데 preflight 요청에는 AUTHORIZATION 헤더가 담기지 않나요? OPTIONS일 경우 인터셉터 통과를 시키는 처리를 했을 때 CORS 에러가 해결되는 이유가 궁금합니다 :)

글 정리해왔습니다! 😊
https://yeonyeon.tistory.com/236

기다렸던 글이네요! 잘 정리해주셨습니다 👍
조금만 더 깊게 다뤄보면

For a CORS-preflight request, request’s credentials mode is always "same-origin", i.e., it excludes credentials, but for any subsequent CORS requests it might not be. Support therefore needs to be indicated as part of the HTTP response to the CORS-preflight request as well. 출처

preflight 요청에는 Auth 헤더, 쿠키, tls 인증 등등 각종 credential 들이 빠집니다. 나중에 CORS 관련 에러를 마주치면 꼭 지금을 떠올리시길 바랍니다 :)

+) 스프링은 CorsUtils.isPreflightRequest() 라는 유틸을 제공합니다. 활용해보시면 좋을 것 같아요.

ddaaac · 2022-06-08T15:28:26Z

src/main/java/woowacourse/shoppingcart/dao/CartItemDao.java

- public void deleteCartItem(final Long id) {
- final String sql = "DELETE FROM cart_item WHERE id = ?";
-
- final int rowCount = jdbcTemplate.update(sql, id);
- if (rowCount == 0) {
- throw new InvalidCartItemException();
- }
+ public void delete(CartItemEntity cartItemEntity) {
+ String sql = "DELETE FROM cart_item WHERE customer_id = :customerId AND product_id = :productId";
+ SqlParameterSource source = new BeanPropertySqlParameterSource(cartItemEntity);
+ jdbcTemplate.update(sql, source);


식별자인 id 대신에 다른 필드들로 삭제하는 이유가 있을까요?

cart를 조회할 때 cart id를 보내주지 않고 있기 때문에 삭제할때 cart id를 받아내지도 못했습니다ㅠㅡㅠ
뒤늦게 이 부분을 발견했으나 이미 프론트/백 대부분이 개발 완료하거나 막바지인 상태였고 마감 기한이 코앞인 상태였어서 수정하지 못했습니다!
시간적 여유가 있었다면 cart id를 받는게 가장 확실하다는 생각이 들어요

- DAO 테스트에서 불필요한 기본 데이터 세팅 삭제 - 불필요한 메서드 호출 제거 - 존재하지 않는 ID 값 수정

ddaaac

안녕하세요 연로그 👋

리뷰 반영 잘 해주셨어요. 레벨 2의 마지막 미션은 여기서 마무리해도 좋을 것 같습니다.

고생 많으셨고 다음 레벨의 미션들도 화이팅하세요 💪

ddaaac · 2022-06-14T16:11:59Z

src/main/java/woowacourse/auth/ui/AuthenticationInterceptor.java

@@ -16,6 +17,10 @@ public AuthenticationInterceptor(JwtTokenProvider jwtTokenProvider) {

 @Override
 public boolean preHandle(HttpServletRequest request, HttpServletResponse response, Object handler) {
+ if (HttpMethod.OPTIONS.matches(request.getMethod())) {


Suggested change

if (HttpMethod.OPTIONS.matches(request.getMethod())) {

if (CorsUtils.isPreFlightRequest(request)) {

Options 의 모든 메서드에 대해서 로직을 수행하기보다는 Preflight로 특정하면 좀 더 CORS 관련 처리라는 걸 보여줄 수 있을 것 같네요. 혹시 모를 Cors 요청이 아닌 Options 메서드에 대해서도 인증을 수행할 수 있구요.

yeon-06 added 30 commits June 5, 2022 21:41

refactor: Exception 발생 시 로그 찍기

7e41f2c

refactor: OrderDetail 내부에 Product 추가

f9fffec

refactor: Cart 내부에 Product 추가

5041fbc

docs: 2단계 요구 사항 추가

514940b

docs: 누락된 이미지 추가

c8797a3

refactor: dto 패키지 이동

f21b719

test: Product 테스트 코드 리팩터링

633523b

- 메서드명 한글 허용 - 정확한 값 검증

refactor: sql에서 * 제거 및 메서드 순서 변경

ede9537

refactor: dao에서 불필요한 메서드 제거

6e92033

refactor: 도메인 폴더 분리

a73c0e7

refactor: Product Service 및 Controller 리팩터링

34c9441

- 파라미터 Entity가 아닌 DTO로 변경 - 메서드명 변경 - @transactional 설정 변경 - 불필요한 ResponseEntity 반환 제거 - HTTP 응답 코드 설정

refactor: Product API URL 변경

396f6b2

- 테스트 코드 리팩터링

refactor: http status로 검증

d7eb54a

test: 테스트 케이스 추가

92f6a5f

refactor: 반환값 API 명세서에 맞추기

d3ace08

refactor: Price 래퍼 클래스 생성

31926b7

refactor: Name 래퍼 클래스 생성

308d2b8

refactor: ImageUrl 래퍼 클래스 생성

db51630

refactor: Customer 생성자 변경

7e94e22

fix: Product 역직렬화 실패하는 현상 개선

98ae883

style: ProductRequest 위치 이동

7231b9e

fix: CORS 에러 해결

19ae60f

refactor: 글자수 제한 변경

48d62e4

chore: 로그 파일명 형식 변경

5c57657

chore: MySQL 연동 설정 파일 추가

84f4cf3

refactor: CartItemDao 에서 JdbcTemplate을 NamedJdbcTemplate으로 변환

11e5309

refactor: findById()의 결과 Optional로 반환

aed4057

refactor: CartService 리팩터링

fda5cee

- @transactional 옵션 변경 - findByAccount 로직 개선 - delete 시 불필요한 검증 제거 - ProductDao id 목록으로 정보 조회하는 메서드 추가

refactor: CartController 리팩터링

418c971

- API 명세서와 request/response 형식 일치 - order test 주석

yeon-06 added 17 commits June 7, 2022 21:33

refactor: Cart 파라미터 유효성 검증

0b9cc21

- 생성 시 존재하는 Product 인지 확인 - 조회 시 존재하는 Customer 인지 확인 - 테스트 케이스 추가 - CartItem 잘못 매칭되는 현상 개선

feat: 총 합계 구하는 로직 추가

405293c

refactor: request 값 및 URI 형식 API 명세서와 일치시킴

ca61c8a

refactor: request값 cartId -> id 변경

fb621f5

refactor: OrdersDetailDao NamedParameterJdbcTemplate으로 변경

40a1fd3

test: OrdersDetailDaoTest 리팩터링

0c7894c

- 타 DAO 제거 - 메서드명 변경 - @beforeeach 제거

test: OrdersDaoTest 리팩터링

9dcd29f

- 테스트 케이스 추가 - @beforeeach 제거 - 메서드 분리 - 타 DAO 제거

refactor: Order 저장 로직 개선

173c2db

- 메서드 분리 - 불필요한 메서드 호출 제거

refactor: Order Servcie 리팩터링

84afa42

- @transactional 옵션 변경 - 메서드명 변경 - 불필요한 sql 호출 제거

refactor: 반환값 API 명세서와 일치

c852b11

refactor: 존재하지 않는 객체를 찾을 때 반환하는 에러 Not Found로 변경

20a28cc

- 사용하지 않는 예외 제거 - 테스트 코드 수정

refactor: 도메인을 이용해 DTO 변환하도록 로직 개선

cbc5b50

refactor: cors 테스트 메서드 변경

744ca94

refactor: sql 중복 호출 제거

038d78c

docs: 리드미 요구 사항 체크

4aec974

docs: 기본 데이터 추가

293d163

fix: 빈 리스트로 조회하는 경우 BadSqlGrammarException 발생하는 현상 제거

c64cc1b

ddaaac requested changes Jun 8, 2022

View reviewed changes

yeon-06 added 4 commits June 13, 2022 21:00

refactor: Cart 도메인 활용

305c47f

test: 기본 데이터 추가로 인한 테스트 케이스 수정

1ca9339

- DAO 테스트에서 불필요한 기본 데이터 세팅 삭제 - 불필요한 메서드 호출 제거 - 존재하지 않는 ID 값 수정

refactor: 하드코딩 제거

6f24554

refactor: 생성자에서 값 객체 이용

39c2d48

ddaaac approved these changes Jun 14, 2022

View reviewed changes

ddaaac merged commit fab3bac into woowacourse:yeon-06 Jun 14, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Spring 장바구니 - 2단계] 연로그(권시연) 미션 제출합니다. #95

[Spring 장바구니 - 2단계] 연로그(권시연) 미션 제출합니다. #95

yeon-06 commented Jun 8, 2022 •

edited

Loading

ddaaac left a comment

ddaaac Jun 8, 2022

yeon-06 Jun 9, 2022 •

edited

Loading

ddaaac Jun 8, 2022

yeon-06 Jun 9, 2022 •

edited

Loading

ddaaac Jun 10, 2022

yeon-06 Jun 13, 2022

ddaaac Jun 8, 2022

ddaaac Jun 8, 2022

ddaaac Jun 8, 2022

yeon-06 Jun 9, 2022 •

edited

Loading

ddaaac Jun 10, 2022

yeon-06 Jun 13, 2022 •

edited

Loading

ddaaac Jun 8, 2022

yeon-06 Jun 13, 2022 •

edited

Loading

ddaaac Jun 13, 2022 •

edited

Loading

ddaaac Jun 8, 2022

yeon-06 Jun 9, 2022

ddaaac left a comment

ddaaac Jun 14, 2022


		import woowacourse.shoppingcart.domain.product.Product;

		public class ProductEntity {

	if (HttpMethod.OPTIONS.matches(request.getMethod())) {
	if (CorsUtils.isPreFlightRequest(request)) {

[Spring 장바구니 - 2단계] 연로그(권시연) 미션 제출합니다. #95

[Spring 장바구니 - 2단계] 연로그(권시연) 미션 제출합니다. #95

Conversation

yeon-06 commented Jun 8, 2022 • edited Loading

2단계

ddaaac left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yeon-06 Jun 9, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yeon-06 Jun 9, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yeon-06 Jun 9, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yeon-06 Jun 13, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yeon-06 Jun 13, 2022 • edited Loading

Choose a reason for hiding this comment

ddaaac Jun 13, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ddaaac left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yeon-06 commented Jun 8, 2022 •

edited

Loading

yeon-06 Jun 9, 2022 •

edited

Loading

yeon-06 Jun 9, 2022 •

edited

Loading

yeon-06 Jun 9, 2022 •

edited

Loading

yeon-06 Jun 13, 2022 •

edited

Loading

yeon-06 Jun 13, 2022 •

edited

Loading

ddaaac Jun 13, 2022 •

edited

Loading