[Spring 장바구니 - 2단계] 조동현(후디) 미션 제출합니다. #130

devHudi · 2022-06-09T05:55:12Z

안녕하세요. 스티치!
API 명세를 자주 바꾸기도 했고, 프론트엔드 코드와 연결했을 때 CORS와 같은 이슈가 발생해서 다소 코드를 급하게 짠 것 같아 아쉽네요 🥲
질문은 코멘트를 남기도록 하겠습니다! 감사합니다.

API 문서 보기

프론트엔드 크루와의 회의 결과 반영

- 상품을 찾을 수 없는 경우 400 대신 404를 응답하도록 변경 - 인수 테스트 코드 추가

테스트코드 Display Name 오타 수정

…00 에러가 발생하도록 구현

…를 설정하도록 변경

…merNotFoundException으로 예외를 번역

인수 테스트에서 DirtiesContext 대신 Sql 어노테이션을 사용하여 테스트 격리

OrderDao를 OrdersDao로 이름 변경

메소드 추출하여 가독성 개선 OrdersNotFoundException 추가

RowMapper를 상수로 분리 SimpleJdbcInsert 사용하여 코드 개선

lxxjn0

후디, 2단계 미션 진행하시느라 수고하셨습니다 :)

몇 가지 부분에 대해서 간단하게 피드백 남겨놨으니 한번 확인하고 반영 부탁드릴게요. 그리고 추가적으로 1단계에서 남긴 피드백이 반영되지 않은 것 같아요. 해당 부분도 함께 확인하시고 반영해주시면 좋을 것 같습니다!!

1단계에 2번에 나눠서 피드백을 진행하였는데 두 번째 피드백을 남기면서 첫 번째 피드백에 대한 코멘트들도 남겨두었어요. 해당 부분들도 확인하고 진행해주면 좋을 것 같아요 👍🏼

lxxjn0 · 2022-06-09T15:53:56Z

README.md

@@ -35,3 +35,60 @@
 - [x] 회원 정보 제거 (회원 탈퇴)
    - [x] 토큰을 전달받아 유효성이 검증되면, 전달받은 customer id 에 해당되는 유저 정보를 제거한다.
    - [x] (예외) 토큰의 유효성이 검증되지 않을 경우(토큰의 customer id 와 전달받은 customer id가 불일치할 경우) 예외가 발생한다.
+
+## 2단계 기능 구현 목록


꼼꼼한 기능 구현 목록 작성이군요 👍🏼

src/main/java/woowacourse/shoppingcart/controller/CartItemController.java

lxxjn0 · 2022-06-09T16:04:44Z

src/main/java/woowacourse/shoppingcart/controller/CartItemController.java

+        return ResponseEntity.noContent().build();
+    }
+
+    @GetMapping("/{productId}")


해당 url도 함께 고민해보면 좋을 것 같아요!! path parameter를 쓰는게 적절한지 한번 고민해보면 좋을 것 같습니다 :)

말씀해주신 API는 특정 유저의 장바구니에 productId 에 해당하는 제품을 담고 있는지 알려주는 API 입니다! 저는 Path Parameter가 제외된 구현을 떠올리기 어려운데, 어떻게 하면 Path Parameter 를 사용하지 않고 구현할 수 있을까요??

제가 이 부분에 대한 피드백을 빠트렸군요.. ㅠㅠ query parameter를 사용해서 전달하면 좀 더 좋을 것 같아요!! path parameter는 주로 리소스의 위치를 나타내는 경향이 있는데 그런 파라미터로 적절하지 않을 경우는 query parameter를 사용하면 좋은 것 같습니다

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

lxxjn0 · 2022-06-09T16:07:39Z

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

    }

    @Override
    public void update(int id, CustomerEntity customerEntity) {
-        String sql = "UPDATE CUSTOMER SET password = ?, profile_image_url = ?, terms = ? WHERE id = ?";
+        String sql = "UPDATE customer SET password = ?, profile_image_url = ?, terms = ? WHERE id = ?";


NamedParameterJdbcTemplate을 어떤 dao에는 적용하고 어떤 dao에는 적용하지 않았는데 이렇게 작성된 이유가 있을까요?

lxxjn0 · 2022-06-09T16:08:36Z

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

-
-        return jdbcTemplate.query(sql, (rs, rowNum) -> rs.getLong("id"), customerId);
-    }
+public interface CartItemDao {


dao를 인터페이스로 분리한 이유가 있을까요?

나중에는 jdbc를 사용하지 않고 데이터베이스에 접근하는 구현체가 있을수도 있겠다는 생각에 만들어두었는데, 오버엔지니어링 같기도하고, DAO 계층에만 인터페이스를 만들어둔것이 일관성이 없어 보이기도 합니다!.. 스티치는 어떻게 생각하시나요?

저같은 경우에는 예상되어지는 부분에 대한 추상화는 진행하지만 그렇지 않은 부분에 대해서는 최대한 오버엔지니어링을 피하려고 하고 있어요!! 후디가 작성한 코드에서 추후 다른 DB 리소스를 붙일 상황이 예상된다거나 그러한 요구사항이 추후 존재한다는 걸 알고 있다면 지금의 추상화는 좋다고 생각해요!! 하지만 '혹시 모르니깐'의 개발은 오버엔지니어링의 시작점이라고 생각합니다!!

지금 다양한 방법으로 시도해보고 수정해보는 것은 좋다고 생각해요!! 제가 리뷰를 남긴 이유도 정말 어떠한 이유가 존재해서 인터페이스로 분리한 것인지 궁금해서 여쭤본 것이었습니다 :)

src/main/java/woowacourse/shoppingcart/service/CartService.java

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

Customer Id 타입을 Integer에서 Long으로 변경

Sha256 암호화를 위한 PasswordEncoder 구현

devHudi · 2022-06-11T08:14:43Z

안녕하세요 스티치! 리뷰이 후디입니다!

이번 미션 2단계 첫 피드백 요청 때 퀄리티가 다소 떨어지는 코드를 제출했었던 것 같아요 ㅠ0ㅠ 변명아닌 변명을 하자면, 미션 마감(데모데이 날)까지도 프론트엔드와 코드를 연결하여 이런 저런 버그와 문제점이 발생해서, 일단 동작하는 코드를 만드는데에만 시간을 전부 사용했던 것 같아요. 이제 방학이기도 하고, 시간적 여유가 생겨 그간 쌓아둔 기술부채(?)를 어느정도 리팩토링하게 되었습니다.

가장 큰 변화로는 DAO 계층과 Service 계층 사이에 Repository 계층이 하나 더 생겨 DAO에서 도메인 객체를 생성하는 복잡한 로직을 Repository 계층으로 위임하였습니다!.. 그리고 일관적이지 않은 final 키워드 사용 등 자잘한 부분을 리팩토링했습니다!

그리고 1단계 피드백 주신 내용들도 이번에 한번에 반영하여 커밋하였습니다! 이번 리뷰도 잘 부탁드립니다! 🙏🙏🙏

lxxjn0

후디, 장바구니 미션 진행하시느라 수고 많으셨어요!! 👍🏼

이번 리뷰에 개선된 코드는 정말 기존 코드에 비해서 잘 가다듬어지고 깔끔해진 것 같아요!! 💯 제가 몇 가지 부분에 대해서 코멘트 남겨두었는데 해당 부분은 참고만 해보면 될 것 같아요 :)

레벨 2 진행하시느라 수고 많으셨습니다!!

lxxjn0 · 2022-06-13T11:37:38Z

src/main/java/woowacourse/shoppingcart/repository/CartItemRepository.java

+        cartItemDao.delete(cartItemId);
+    }
+
+    private CartItem generateCartItem(CartItemEntity cartItemEntity) {


사용하는 코드 가까이에 둔다면 코드의 가독성을 더 좋게 만들어 줄 수 있을 것 같아요!!

lxxjn0 · 2022-06-13T11:39:43Z

src/main/java/woowacourse/shoppingcart/repository/ProductRepository.java

+    private Product convertEntityToDomain(ProductEntity productEntity) {
+        return new Product(
+                productEntity.getId(),
+                productEntity.getName(),
+                productEntity.getDescription(),
+                productEntity.getPrice(),
+                productEntity.getStock(),
+                productEntity.getImageUrl()
+        );
+    }


이건 취향에 따라 갈리긴 하는데 저는 이런 종류의 코드는 보통 (여기서의) entity 내부에 넣어두곤 합니다. 실제 생성에 필요한 정보를 entity가 모두 가지고 있어서요!!

lxxjn0 · 2022-06-13T11:43:41Z

src/main/java/woowacourse/shoppingcart/service/OrdersService.java

+    public List<OrdersResponse> findOrdersByCustomerId(Long customerId) {
+        List<Orders> orders = ordersRepository.findAllByCustomerId(customerId);
+
+        return orders.stream().map(this::convertOrderToResponse).collect(Collectors.toList());


Suggested change

return orders.stream().map(this::convertOrderToResponse).collect(Collectors.toList());

return orders.stream()

.map(this::convertOrderToResponse)

.collect(Collectors.toList());

체이닝 메서드는 보통 한 라인당 체이닝 로직을 둬서 가독성을 높이곤 합니다 👍🏼

lxxjn0 · 2022-06-13T11:46:01Z

src/main/java/woowacourse/shoppingcart/service/OrdersService.java

+    }
+
+    private void validateOrderIdByCustomerId(Orders orders, Long customerId) {
+        if (!orders.getCustomerId().equals(customerId)) {


혹시나 orders.getCustomerId()의 반환 값이 null일 경우 해당 라인에서는 NPE가 발생할 수 있어요!! 이럴 때는 NPE 발생을 방지해주는 Objects.equals 메서드를 활용하는 것도 좋습니다

lxxjn0 · 2022-06-13T11:49:14Z

src/main/java/woowacourse/shoppingcart/support/PasswordEncoder.java

+import java.security.MessageDigest;
+import java.security.NoSuchAlgorithmException;
+
+public class PasswordEncoder {


해당 객체를 정적 클래스 형식으로 사용하고 있군요. 그렇다면 생성자를 제한해서 객체로 활용되지 못하도록 하면 더 좋을 것 같아요!! 추가로 제가 리뷰에서 생각했었던 방법은 PasswordEncoder를 Password 객체가 가지고 있지 않으면서 어플리케이션에서 1개의 인스턴스만 생성해서 사용하는 방법이에요. 스프링의 빈으로 등록해서 사용한다면 각각의 Password 객체가 값을 가지고 있지 않으면서 모두 동일한 객체를 사용할 수 있을 것 같아요!!

lxxjn0 · 2022-06-13T11:49:31Z

src/main/java/woowacourse/shoppingcart/support/PasswordEncoder.java

+    private static String bytesToHex(byte[] bytes) {
+        StringBuilder builder = new StringBuilder();
+        for (byte b : bytes) {
+            builder.append(String.format("%02x", b));


매직넘버는 상수로 추출해주세요!!

devHudi added 30 commits June 7, 2022 13:30

docs: 2단계 기능 구현 목록 작성

6de48aa

feat: 상품 도메인에 설명, 재고수량 필드 추가

1069430

프론트엔드 크루와의 회의 결과 반영

refactor: 상품을 찾을 수 없을 경우에 대한 예외 추가

44f8494

- 상품을 찾을 수 없는 경우 400 대신 404를 응답하도록 변경 - 인수 테스트 코드 추가

docs: 기능 구현 목록 반영

52f4fae

feat: 액세스 토큰에서 유저 아이디를 추출하는 Argument Resolver 구현

dc98d78

refactor: Cart Item API가 유저를 식별할 때 customerName 대신 액세스 토큰으로 식별하도록 변경

a4ce601

refactor: Order API가 유저를 식별할 때 customerName 대신 액세스 토큰으로 식별하도록 변경

8124935

docs: 2단계 기능 구현 목록 추가

85a0ec9

test: 장바구니 및 주문 API에 인가 테스트 추가 (토큰 만료, 토큰 유효하지 않음)

b8f6767

docs: 2단계 기능 구현 목록 갱신

a2a0846

테스트코드 Display Name 오타 수정

feat: 변경된 명세에 따라 카트 아이템 추가 및 조회 기능 수정

aba2859

feat: 특정 상품이 특정 유저의 장바구니에 존재하는지 여부를 조회하는 API 구현

9ec5601

refactor: dto를 요청과 응답에 따라 패키지 분리

393d7d9

feat: 장바구니 수정 API 기능 구현

a7e5756

docs: 2단계 기능 구현 목록 변경

3333130

feat: 장바구니 추가 시 존재하지 않은 제품 ID를 전달받은 경우 404에러가 발생하도록 구현

0e807fd

feat: 장바구니 항목 삭제 시 존재하지 않은 장바구니 항목 ID가 전달되면 404 에러가 발생하도록 수정

62b4f6f

docs: 기능 개선 목록 갱신

7fa5bfb

feat: 주문 추가, 조회 기능을 변경된 기획에 맞게 수정

ba4b053

feat: 카트 아이템 수정 시 요청한 카트 아이템의 productId 와, Body 의 productId 가 다를 경우 4…

a0b80fe

…00 에러가 발생하도록 구현

refactor: Dao에 대한 인터페이스 추가

5a6fd00

refactor: ResponseEntity에 제네릭 타입 추가

9fb21e8

refactor: cartId를 cartItemId로 변경

7d74ce3

refactor: single_order 테이블을 orders로, order_product 테이블을 order_detail로 변경

6842fb2

refactor: SQL내의 대문자로 되어있는 테이블이름을 소문자로 변경

c6e0af3

refactor: schema.sql 에서 외래키를 설정할 때 alter table 대신 create table 에서 외래키…

3d22fc9

…를 설정하도록 변경

chore: mysql-connector-java 의존성 추가

b957caf

fix: 테이블명을 소문자로 변경

4e87602

refactor: 불필요한 출력 로직 제거

270d77d

feat: 회원 요청과 응답 DTO의 depth 제거

202aff9

devHudi added 12 commits June 8, 2022 22:14

refactor: API 문서에 따라 토큰 발급 응답값의 회원 ID필드를 customerId에서 userId로 변경

4f54ff4

fix: preflight 대응 위해 AuthInterceptor preHandle 메소드 수정

b835c77

refactor: JdbcCartItemDao에 SimpleJdbcInsert 사용하여 코드 개선

4e4cd9b

refactor: CustomerDao의 조회 메서드에서 EmptyResultDataAccessException을 Custo…

adef1f1

…merNotFoundException으로 예외를 번역

test: 인수 테스트 최적화

22a3c36

인수 테스트에서 DirtiesContext 대신 Sql 어노테이션을 사용하여 테스트 격리

refactor: OrdersDao와 OrderDetailDao 분리

9ccb697

OrderDao를 OrdersDao로 이름 변경

refactor: JdbcOrdersDao 리팩토링

0c4ab1b

메소드 추출하여 가독성 개선 OrdersNotFoundException 추가

refactor: JdbcProductDao 리팩토링

f7a2d0c

RowMapper를 상수로 분리 SimpleJdbcInsert 사용하여 코드 개선

refactor: auth 패키지와 shoppingcart 패키지의 Config를 병합

d5199a3

docs: 기능 구현 목록 개선

60e106a

feat: schema에 on delete 제약 조건 추가

38f6e7f

fix: 빈 BirthDay의 value를 null로 설정

d79ef38

lxxjn0 requested changes Jun 9, 2022

View reviewed changes

refactor: ProductRepository 구현

f7e3a5b

devHudi force-pushed the step2 branch from bc9db92 to f7e3a5b Compare June 10, 2022 19:24

devHudi added 12 commits June 11, 2022 12:16

refactor: OrderRepository 구현

7878409

refactor: CustomerRepository 구현

3d4b09d

refactor: CartItemRepository 구현

b75374c

refactor: 단건 조회시 DAO가 Optional을 반환하도록 개선

a1e45dc

refactor: 일관적이지 않게 사용된 final 키워드 제거 (파라미터 및 지역변수)

956ee2f

refactor: final 키워드 추가

946516f

refactor: 사용되지 않는 코드 제거

bc35f65

refactor: Id 도메인 제거

19c820a

Customer Id 타입을 Integer에서 Long으로 변경

refactor: Spring Security 의존성 제거

58bcd51

Sha256 암호화를 위한 PasswordEncoder 구현

refactor: RowMapper를 상수로 변경

81b1c5d

refactor: 컬럼이름을 상수로 분리

629133f

refactor: DTO의 생성자 파라미터로 도메인 객체를 전달하여 DTO를 생성할 수 있도록 개선

0f6eaaf

lxxjn0 approved these changes Jun 13, 2022

View reviewed changes

lxxjn0 merged commit 991237f into woowacourse:devhudi Jun 13, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Spring 장바구니 - 2단계] 조동현(후디) 미션 제출합니다. #130

[Spring 장바구니 - 2단계] 조동현(후디) 미션 제출합니다. #130

devHudi commented Jun 9, 2022 •

edited

Loading

lxxjn0 left a comment

lxxjn0 Jun 9, 2022

lxxjn0 Jun 9, 2022

devHudi Jun 11, 2022

lxxjn0 Jun 15, 2022

lxxjn0 Jun 9, 2022

lxxjn0 Jun 9, 2022

devHudi Jun 11, 2022

lxxjn0 Jun 13, 2022

devHudi commented Jun 11, 2022

lxxjn0 left a comment

lxxjn0 Jun 13, 2022

lxxjn0 Jun 13, 2022

lxxjn0 Jun 13, 2022

lxxjn0 Jun 13, 2022

lxxjn0 Jun 13, 2022

lxxjn0 Jun 13, 2022

[Spring 장바구니 - 2단계] 조동현(후디) 미션 제출합니다. #130

[Spring 장바구니 - 2단계] 조동현(후디) 미션 제출합니다. #130

Conversation

devHudi commented Jun 9, 2022 • edited Loading

lxxjn0 left a 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

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

devHudi commented Jun 11, 2022

lxxjn0 left a 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

devHudi commented Jun 9, 2022 •

edited

Loading