[2단계 - 장바구니 기능] 히이로(문제웅) 미션 제출합니다 #329

MoonJeWoong · 2023-05-05T12:49:48Z

안녕하세요 영이, 히이로입니다.
step1 리뷰를 빠르게 해주셨는데 이것 저것 삽질하면서 고민하다보니 리뷰 요청까지 시간이 오래 걸렸습니다... 😭
이번 step2 리뷰도 잘 부탁드립니다!

이번 step2를 진행하며 궁금했던 부분은 다음과 같습니다.

인증 기능 구현
- 이번 미션을 진행하는 기간 중 학습하게 되었던 ArgumentResolver와 Interceptor를 Configuration 해서 요청 헤더의 응답 정보를 인증하는 기능을 구현하고자 했습니다.
- 한 가지 아쉬웠던 부분은 interceptor 내부에서 memberService를 이용해 실제로 DB에 등록된 사용자인지 확인도 해보고 싶었으나 interceptor 내부에 memberService 빈을 주입시키는 것이 불가능하다고 판단되어 지금의 로직으로 구현하였습니다.
- 지금의 로직은 cartController에서 인증이 필요한 요청이 들어오면 memberService를 이용해 인증을 처리하는 방식입니다.
- 혹시 제가 모르는 구현 방식으로 interceptor 내부에서 사용자 인증 기능이 구현 가능한지 질문드리고 싶습니다!

이번에 도메인을 설계하면서 많은 시간을 고민했던 것 같습니다.
- 현재 도메인을 보면 실질적인 비즈니스 로직이 거의 없는 객체들이 대부분입니다. 이 부분은 현재 미션의 요구사항이 아직 적어서 비즈니스 로직이라고 할만한 것이 아직 없어서라고 판단을 하긴 했습니다만 설계하면서도 도메인이 왜 필요한지에 대해서 많이 고민했던 것 같습니다.
- 그리고 id와 같은 DB(외부 기술)에 의해 생성되는 property들에 의해 순수 자바 객체로서 도메인이 오염되는 것이 괜찮은가에 대해서도 은연중에 계속 고민이 되었던 것 같습니다. 지난 자동차 경주 미션에서는 도메인이 먼저 POJO로 설계되어 있었고 개인적으로 진행하면서 DB테이블의 단일 행과 mapping되는 Entity 객체를 만들어서 최대한 도메인이 오염되는 것을 막아야 한다고 생각했었습니다.
- 하지만 이번 미션에서는 id 자체가 비즈니스 로직 측면에서 필요하다고 생각이 되었습니다. 예를 들어 장바구니에 중복된 품목을 담을 수 없게 하는 기능을 구현하기 위해서는 품목 간 id 비교가 불가피 했기 때문입니다. 결국 이번 미션에서는 id 값들을 도메인의 property로 가지게 할 수 밖에 없었습니다.
- 이 외에도 도메인 객체마다 id 값이 필요한 경우가 있고 없는 경우가 혼재했는데, 이럴 때 통일성을 위해서 사용하지 않는 id 값을 도메인에 부여해야 하는지에 대해서도 굉장히 고민이 되었던 것 같습니다.
- 정답이 없는 문제들을 가지고 너무 오랫동안 고민한게 아니었나 싶기도 하네요... 😭
- 영이는 이런 부분에 대해서 어떤 생각을 가지고 계신지 질문드리고 싶습니다.

세부사항 - cart 정보 응답 시 id값을 item이 아닌 product id로 응답하는 버그 수정

choijy1705

안녕하세요 히이로
전체적으로 엄청 깔끔하게 짜주신것 같아요!👍
이번 리뷰에서는 인증과 관련된 부분 위주로 남겼습니다

choijy1705 · 2023-05-06T04:39:42Z

src/main/java/cart/auth/AuthArgumentResolver.java

+            MethodParameter parameter,
+            ModelAndViewContainer mavContainer,
+            NativeWebRequest webRequest,
+            WebDataBinderFactory binderFactory) throws Exception {


throws Exception은 없어도 되겠네요

choijy1705 · 2023-05-06T04:46:22Z

src/main/java/cart/auth/AuthArgumentResolver.java

+    }
+
+    @Override
+    public Object resolveArgument(


Suggested change

public Object resolveArgument(

public AuthInfo resolveArgument(

AuthInfo로 바꿔도 되지 않나요?
체크해봐주세요!

밑에서 언급해주신 throws Exception 부분도 그렇고 인터페이스를 구현하는 과정에서 메서드 시그니처를 반드시 그대로 따를 필요는 없다는 것은 몰랐네요... 아직 스프링 프레임워크의 전체적인 구조를 머리속에 그리지 못하다보니 이런 세부적인 것들을 건드리기가 무섭더라고요... ㅎㅎ 감사합니다!

choijy1705 · 2023-05-06T04:55:14Z

src/main/java/cart/domain/cart/Item.java

+    private final Long memberId;
+    private final Long productId;


여기도 도메인에서는 Member, Product를 필드로 가지는게 적절하지 않을지 고민해봐주세요!

choijy1705 · 2023-05-06T04:58:14Z

src/main/java/cart/service/CartService.java

-        );
+    public ItemResponse createItem(Long memberId, Long productId) {
+        Cart memberCart = getCartByMemberId(memberId);
+        Item requestedItem = new Item(memberId, productId);


product와 member 를 기반으로 item이 만들어지는것 같은데
실제 product와 member 값이 아닌
id를 통해서만 로직이 처리되고 있는것 같네요

밑에서 productDao를 통해 실제 product를 조회하고 있긴한데 response dto를 만들기위해 조회를 하고 있어요
로직 흐름상 먼저 조회를 해서 완벽한 product 로 처리가 되어야하지 않을까요?

동의합니다. 도메인 설계를 미리 해둔 상태였던 자동차 경주 미션과 다르게 이번 미션에서는 웹 기반 서비스를 개발하면서 도메인 설계를 같이해야 했습니다. 이 과정에서 전체적인 계층형 구조와 API 설계 로직을 먼저 구상하고 거기에 도메인을 끼워 맞추는 방식으로 도메인 설계를 진행했습니다. 그러다 보니 도메인 로직이 최우선시 되지 않고 주변 영속성 로직과 API 설계에 도메인 설계가 영향을 많이 받아서 이런 결과를 낳게 되었습니다.

도메인의 다른 부분에서 리뷰해주신 부분들도 이 때문에 발생했던 문제라고 생각됩니다.

해당 부분들은 영속성 로직의 측면이 아니라 도메인 설계적 측면에서 가져야 하는 프로퍼티들을 가질 수 있게끔 일괄적으로 수정했습니다. 또한 BusinessLayer(Service)에서 최대한 도메인이 먼저 완성된 이후에 영속성 로직 혹은 비즈니스 로직이 수행될 수 있는 방향으로 수정했습니다.

앞으로는 반드시 도메인 설계를 최우선적으로 수행한 이후에 영속성 로직과 API 설계를 고려하는 순으로 개발을 진행해야 한다는 것을 배울 수 있었습니다! 😀

choijy1705 · 2023-05-06T05:00:28Z

src/main/java/cart/service/CartService.java

-                created.getPrice(),
-                created.getImageUrl());
+        Long registeredItemId = cartDao.createItem(requestedItem.getMemberId(), requestedItem.getProductId());
+        Item registeredItem = cartDao.findItemById(registeredItemId);


굳이 다시 조회를 안해도 될것같은데
어떤가요??

현재 API 설계시 step1에서 피드백 주신대로 생성 요청 시 생성된 항목을 반환하는 방식을 고수하고 있습니다. 이 부분을 조회를 한 번 더 하지 말고 이미 메서드에 있는 정보들을 바탕으로 반환 값을 만들어서 반환해주라는 의미로 이해하고 리팩토링을 진행했습니다.

혹시 제가 생각한 방향이 틀렸다면 알려주시면 감사하겠습니다!

choijy1705 · 2023-05-06T05:11:25Z

src/main/java/cart/auth/BasicAuthorizationExtractor.java

+            return new AuthInfo(email, password);
+        }
+
+        return null;


@AuthResolving이 붙은 곳에서 null이 반환되고 문제없이 진행되기보다(물론 controller에서 체크하지만)
여기서 throw를 하는게 더 안전하지 않나요?

현재 프로젝트에서는 controller에 request 메세지가 도달하기 전에 LoginInterceptor가 Authorization 헤더에 대해 null인지 한 번 검증하고 있습니다. 그래서 현재 BasicAuthorizationExtractor 클래스에서 헤더가 null인지 검증하는 부분에서는 LoginInterceptor에서 throw되는 것과 동일한 예외를 던지도록 수정했습니다.

그리고 리뷰해주신 마지막에 null을 반환하는 부분에 대해서는 Authorization 헤더가 정상적으로 존재하나 Basic 방식을 채택하지 않은 값이 들어있을때 수행되게 되므로 새로운 customException을 정의하여 반환하도록 수정하였습니다!

choijy1705 · 2023-05-06T05:39:06Z

src/main/java/cart/auth/LoginInterceptor.java

+public class LoginInterceptor implements HandlerInterceptor {
+
+    @Override
+    public boolean preHandle(HttpServletRequest request, HttpServletResponse response, Object handler) {


memberService를 사용하는게 불가능하다고 하셨는데 이유가 뭘까요?😭
interceptor는 spring의 영역이라서 충분히 가능할것 같은데요
단순히 member조회라면 memberDao만을 사용하는것도 방법이라고 생각합니다

혹시 인터셉터를 @bean 어노테이션으로 직접 빈으로 등록하여 꼬여서 안되는거라면 @component를 사용해보면 될것 같습니다!

앗 제가 질문을 드릴 때 어떻게 해서 불가능하다고 판단했는지를 말씀을 안드렸네요!
처음 LoginInterceptor를 @component로 선언하고 memberService를 @Autowired를 사용해 의존성을 주입하려고 했습니다. 그런데 @configuration 클래스에서 LoginInterceptor를 등록할 때 아래와 같이 Interceptor를 생성하면서 등록을 해주고 있었습니다.

// CartWebConfiguration... @Bean public LoginInterceptor loginInterceptor() { return new LoginInterceptor(); } @Override public void addInterceptors(InterceptorRegistry registry) { registry.addInterceptor(loginInterceptor()) .addPathPatterns("/api/cart/**"); }

그런데 LoginInterceptor에서 memberService를 생성자 주입을 통해 사용하게 하기 위해 생성자를 열어주면 다음과 같은 코드로 바뀌었습니다.

// CartWebConfiguration... @Bean public LoginInterceptor loginInterceptor() { return new LoginInterceptor(membersService); // membersService를 configuration 클래스에서 주입...? } @Override public void addInterceptors(InterceptorRegistry registry) { registry.addInterceptor(loginInterceptor()) .addPathPatterns("/api/cart/**"); }

여기에서 Configuration 클래스에서 memberService를 LoginInterceptor에게 직접 주입하는 방식에 대한 확신이 들지 않았기에 불가능하다고 판단했습니다. 하지만 리뷰를 받고 다시 알아보니 @configuration 클래스도 @component로 관리되기에 빈을 주입받을 수 있어서 memberService를 주입해서 등록해주는 방식으로 수정했습니다!

choijy1705 · 2023-05-06T05:46:32Z

src/main/java/cart/auth/LoginInterceptor.java

+            throw new MissingAuthorizationHeaderException();
+        }
+
+        return true;


그리고 인증과 관련된 작업들은 interceptor에서 처리하는게 적절하지 않을까요?
argumentResolver는 원하는 객체로 만들어서 반환하는 역할만 하는게 적절해보입니다!
interceptor에서 인증과 관련된 로직을 처리하고 request.setAttribute() 를 활용해 arugmentResolover에서 email, password를 꺼내서 사용만 하는 형태가 되면 어떨지 고민해봐주세요!
그렇게 된다면 extractor는 없어지고 extractor의 로직 대부분이 interceptor에서 처리될듯하네요!

동의합니다. 미션을 진행하면서 인증관련된 로직은 LoginInterceptor가 모두 처리하도록 하고 싶었는데 위에서 언급했던 것과 같이 membersService 의존성 문제를 해결하지 못했기에 이렇게 구현하게 되었습니다. 의존성 문제를 해결하고 LoginInterceptor에서 인증과 setAttribute를 통해 email과 password를 request 메세지의 속성으로 추가할 수 있도록 했습니다.

덕분에 인자로 넘어오는 request와 response 객체에 대해 필요에 따라 적절하게 작업을 수행해 줄 수 있다는 점을 배울 수 있었습니다. 감사합니다! 😄

choijy1705 · 2023-05-06T05:51:33Z

src/main/java/cart/domain/cart/Cart.java

+
+public class Cart {
+
+    private final Long memberId;


memberId가 아닌 실제 Member를 가지고 있어야하지 않을까요?

choijy1705 · 2023-05-06T05:52:51Z

src/main/java/cart/dto/auth/AuthInfo.java

@@ -0,0 +1,20 @@
+package cart.dto.auth;
+
+public class AuthInfo {


interceptor에서 인증이 제대로된다면
argument resolver에서 반환되는 객체는 Member가 되고
AuthInfo는 필요없겠네요!

영이가 리뷰해주신대로 따라가다 보니 다음과 같이 처음에 생각했던 인증 절차대로 구현할 수 있었습니다.

MembersService 기능을 사용해서 LoginInterceptor가 인증 절차를 진행하고 request 메세지의 attribute로 email과 password를 추가합니다.

이후 MembersDao 기능을 사용해서 ArgumentResolver가 request메세지의 email 속성 값을 받아와 member 객체를 완성해 반환합니다.

다만 AuthInfo dto는 loginInterceptor에서 membersService로 데이터를 넘겨 인증 과정을 처리할 때 필요하므로 삭제하지는 않았습니다. 😄

세부사항 - 어노테이션 명 변경 - BasicAuthorizationExtractor에서 null 대신 예외를 throw하도록 수정

세부사항 - 기존 BasicAuthorizationExtractor 기능 LoginInterceptor로 이관 - AuthArgumentResolver는 request 메세지의 attribute를 추출해서 넘겨주도록 수정

MoonJeWoong

안녕하세요 영이, 히이로입니다!

인증관련 부분과 도메인 부분에서 막연하게 느꼈던 부분들을 잘 짚어주셔서 크게 엇나가지 않고 원하는 방향으로 리팩토링할 수 있었던 것 같습니다. 혹시 제가 잘못된 방향으로 이해하고 수정한 부분이 있다면 말씀해주시면 감사하겠습니다!

영이께서 조목조목 리뷰로 잘 짚어주셨어서 이번 미션을 계기로 스프링 프레임워크에 대한 이해도나 관점을 많이 확장시킬 수 있었던 것 같습니다. 이번 미션동안 리뷰해주시느라 정말 감사했습니다! 🙇

MoonJeWoong · 2023-05-06T14:11:35Z

src/main/java/cart/auth/AuthArgumentResolver.java

+    }
+
+    @Override
+    public Object resolveArgument(


밑에서 언급해주신 throws Exception 부분도 그렇고 인터페이스를 구현하는 과정에서 메서드 시그니처를 반드시 그대로 따를 필요는 없다는 것은 몰랐네요... 아직 스프링 프레임워크의 전체적인 구조를 머리속에 그리지 못하다보니 이런 세부적인 것들을 건드리기가 무섭더라고요... ㅎㅎ 감사합니다!

MoonJeWoong · 2023-05-06T14:24:12Z

src/main/java/cart/auth/BasicAuthorizationExtractor.java

+    public static final int CREDENTIALS_PASSWORD_INDEX = 1;
+
+    public AuthInfo extract(HttpServletRequest request) {
+        String header = request.getHeader(AUTHORIZATION);


HTTP와 관련된 상수들은 기본 스프링 프레임워크 패키지에서 제공을 한다는 것을 처음 알았습니다. 감사합니다. 😄

그런데 알려주신 Basic 상수 값은 찾아보니 스프링 시큐리티 패키지에 선언되어 있는 것 말고는 없다고 나오더라구요... 상수 값을 사용하기 위해 패키지를 import 하는 것은 뭔가 배보다 배꼽이 커지는 것 같아 일단 Basic은 상수로 유지하는 것으로 결정했습니다!

MoonJeWoong · 2023-05-06T14:35:45Z

src/main/java/cart/auth/BasicAuthorizationExtractor.java

+            return new AuthInfo(email, password);
+        }
+
+        return null;


현재 프로젝트에서는 controller에 request 메세지가 도달하기 전에 LoginInterceptor가 Authorization 헤더에 대해 null인지 한 번 검증하고 있습니다. 그래서 현재 BasicAuthorizationExtractor 클래스에서 헤더가 null인지 검증하는 부분에서는 LoginInterceptor에서 throw되는 것과 동일한 예외를 던지도록 수정했습니다.

그리고 리뷰해주신 마지막에 null을 반환하는 부분에 대해서는 Authorization 헤더가 정상적으로 존재하나 Basic 방식을 채택하지 않은 값이 들어있을때 수행되게 되므로 새로운 customException을 정의하여 반환하도록 수정하였습니다!

MoonJeWoong · 2023-05-06T15:13:20Z

src/main/java/cart/auth/LoginInterceptor.java

+public class LoginInterceptor implements HandlerInterceptor {
+
+    @Override
+    public boolean preHandle(HttpServletRequest request, HttpServletResponse response, Object handler) {


앗 제가 질문을 드릴 때 어떻게 해서 불가능하다고 판단했는지를 말씀을 안드렸네요!
처음 LoginInterceptor를 @component로 선언하고 memberService를 @Autowired를 사용해 의존성을 주입하려고 했습니다. 그런데 @configuration 클래스에서 LoginInterceptor를 등록할 때 아래와 같이 Interceptor를 생성하면서 등록을 해주고 있었습니다.

// CartWebConfiguration... @Bean public LoginInterceptor loginInterceptor() { return new LoginInterceptor(); } @Override public void addInterceptors(InterceptorRegistry registry) { registry.addInterceptor(loginInterceptor()) .addPathPatterns("/api/cart/**"); }

그런데 LoginInterceptor에서 memberService를 생성자 주입을 통해 사용하게 하기 위해 생성자를 열어주면 다음과 같은 코드로 바뀌었습니다.

// CartWebConfiguration... @Bean public LoginInterceptor loginInterceptor() { return new LoginInterceptor(membersService); // membersService를 configuration 클래스에서 주입...? } @Override public void addInterceptors(InterceptorRegistry registry) { registry.addInterceptor(loginInterceptor()) .addPathPatterns("/api/cart/**"); }

여기에서 Configuration 클래스에서 memberService를 LoginInterceptor에게 직접 주입하는 방식에 대한 확신이 들지 않았기에 불가능하다고 판단했습니다. 하지만 리뷰를 받고 다시 알아보니 @configuration 클래스도 @component로 관리되기에 빈을 주입받을 수 있어서 memberService를 주입해서 등록해주는 방식으로 수정했습니다!

MoonJeWoong · 2023-05-06T16:37:36Z

src/main/java/cart/auth/LoginInterceptor.java

+            throw new MissingAuthorizationHeaderException();
+        }
+
+        return true;


동의합니다. 미션을 진행하면서 인증관련된 로직은 LoginInterceptor가 모두 처리하도록 하고 싶었는데 위에서 언급했던 것과 같이 membersService 의존성 문제를 해결하지 못했기에 이렇게 구현하게 되었습니다. 의존성 문제를 해결하고 LoginInterceptor에서 인증과 setAttribute를 통해 email과 password를 request 메세지의 속성으로 추가할 수 있도록 했습니다.

덕분에 인자로 넘어오는 request와 response 객체에 대해 필요에 따라 적절하게 작업을 수행해 줄 수 있다는 점을 배울 수 있었습니다. 감사합니다! 😄

MoonJeWoong · 2023-05-07T05:56:22Z

src/main/java/cart/service/CartService.java

-        );
+    public ItemResponse createItem(Long memberId, Long productId) {
+        Cart memberCart = getCartByMemberId(memberId);
+        Item requestedItem = new Item(memberId, productId);


동의합니다. 도메인 설계를 미리 해둔 상태였던 자동차 경주 미션과 다르게 이번 미션에서는 웹 기반 서비스를 개발하면서 도메인 설계를 같이해야 했습니다. 이 과정에서 전체적인 계층형 구조와 API 설계 로직을 먼저 구상하고 거기에 도메인을 끼워 맞추는 방식으로 도메인 설계를 진행했습니다. 그러다 보니 도메인 로직이 최우선시 되지 않고 주변 영속성 로직과 API 설계에 도메인 설계가 영향을 많이 받아서 이런 결과를 낳게 되었습니다.

도메인의 다른 부분에서 리뷰해주신 부분들도 이 때문에 발생했던 문제라고 생각됩니다.

해당 부분들은 영속성 로직의 측면이 아니라 도메인 설계적 측면에서 가져야 하는 프로퍼티들을 가질 수 있게끔 일괄적으로 수정했습니다. 또한 BusinessLayer(Service)에서 최대한 도메인이 먼저 완성된 이후에 영속성 로직 혹은 비즈니스 로직이 수행될 수 있는 방향으로 수정했습니다.

앞으로는 반드시 도메인 설계를 최우선적으로 수행한 이후에 영속성 로직과 API 설계를 고려하는 순으로 개발을 진행해야 한다는 것을 배울 수 있었습니다! 😀

MoonJeWoong · 2023-05-07T06:23:05Z

src/main/java/cart/service/CartService.java

-                created.getPrice(),
-                created.getImageUrl());
+        Long registeredItemId = cartDao.createItem(requestedItem.getMemberId(), requestedItem.getProductId());
+        Item registeredItem = cartDao.findItemById(registeredItemId);


현재 API 설계시 step1에서 피드백 주신대로 생성 요청 시 생성된 항목을 반환하는 방식을 고수하고 있습니다. 이 부분을 조회를 한 번 더 하지 말고 이미 메서드에 있는 정보들을 바탕으로 반환 값을 만들어서 반환해주라는 의미로 이해하고 리팩토링을 진행했습니다.

혹시 제가 생각한 방향이 틀렸다면 알려주시면 감사하겠습니다!

MoonJeWoong · 2023-05-07T06:49:38Z

src/main/java/cart/dto/auth/AuthInfo.java

@@ -0,0 +1,20 @@
+package cart.dto.auth;
+
+public class AuthInfo {


영이가 리뷰해주신대로 따라가다 보니 다음과 같이 처음에 생각했던 인증 절차대로 구현할 수 있었습니다.

MembersService 기능을 사용해서 LoginInterceptor가 인증 절차를 진행하고 request 메세지의 attribute로 email과 password를 추가합니다.

이후 MembersDao 기능을 사용해서 ArgumentResolver가 request메세지의 email 속성 값을 받아와 member 객체를 완성해 반환합니다.

다만 AuthInfo dto는 loginInterceptor에서 membersService로 데이터를 넘겨 인증 과정을 처리할 때 필요하므로 삭제하지는 않았습니다. 😄

choijy1705

안녕하세요 히이로
피드백 잘 반영해주셨네요👍
이번미션은 머지해도 될것같습니다!
몇가지 마이너한 코멘트 남겼는데 챙겨봐주세요

choijy1705 · 2023-05-07T15:16:22Z

src/main/java/cart/auth/AuthArgumentResolver.java

+        long memberId = membersDao.findIdByEmail(email);
+
+        return membersDao.findById(memberId);


email을 통해 id를 찾는것보다 emai을 통해 member를 찾는게 어떤가요?
한번에 찾을 수 있는걸 두번에 나눠서 하고 있는것 같아서요!
필요하다면 인덱스, 유니크 제약조건에 대해 공부하고 인덱스를 적용해보면 좋을것 같습니다

choijy1705 · 2023-05-07T15:22:19Z

src/main/java/cart/auth/LoginInterceptor.java

+    }
+
+    private void validateAuthorization(AuthInfo authInfo) {
+        if (!membersService.isMemberCertified(authInfo)) {


authInfo 객체를 만들어 넘기기보다는 그냥 email, password를 넘겨서 사용해도 되지않을까요?

choijy1705 · 2023-05-07T15:27:49Z

src/main/java/cart/auth/LoginInterceptor.java

+        request.setAttribute("email", email);
+        request.setAttribute("password", password);


"email", "password" 는 key값이고 argumentResolver에서도 공통적으로 쓰이는거라 public 상수로 관리되면 좋을것 같네요

choijy1705 · 2023-05-07T15:30:30Z

src/main/java/cart/controller/rest/CartsController.java

+            @Auth Member member,
+            @RequestParam("product-id") Long productId) {
+
+        ItemResponse itemResponse = cartService.createItem(member.getId(), productId);


argumentResolver에서 완성된 member를 조회했는데 여기서 다시 memberId만 넘겨주고 있어요!

choijy1705 · 2023-05-07T15:32:42Z

src/main/java/cart/controller/rest/CartsController.java

+
+    @GetMapping("/items")
+    public ResponseEntity<CartResponse> readItemsByMember(@Auth Member member) {
+        CartResponse cartResponse = cartService.readAllItemsByMemberId(member.getId());


여기는 id만 쓰여서 상관없긴한데
쓰임새를 생각해보면 argumentResolver에서 memberId를 넘기거나 email을 넘기고
dao를 통한 조회를 실제 로직에서 처리하는 방법도 있겠네요

choijy1705 · 2023-05-07T15:37:07Z

src/main/java/cart/domain/cart/Cart.java

+        this.items = new ArrayList<>();
+
+        if (items.size() > 0) {
+            this.items.addAll(items);
+        }


item이 없다면 에러가 날필요는 없나요?

비즈니스 로직 상 사용자의 장바구니가 초기에는 아무 상품도 담고 있지 않을 수 있기에, 또 cart 도메인 객체는 이를 나타낼 수 있어야 하기에 해당 부분을 예외처리하지는 않았습니다! 😄

solarsalt · 2023-08-01T10:36:52Z

src/main/java/cart/service/ProductsService.java

+        Product product = new Product(
+                productRequest.getName(),
+                productRequest.getPrice(),
+                productRequest.getImageUrl()
+        );


@MoonJeWoong
product에 더 많은 정보를 담게 된다면 생성자 파라미터가 늘어나서 힘들어 지므로 아래처럼 하는 방법도 있음

Product.of(productRquest)

solarsalt · 2023-08-01T10:42:34Z

src/main/java/cart/controller/exception/CartHasDuplicatedItemsException.java

+public class CartHasDuplicatedItemsException extends RuntimeException {
+    public CartHasDuplicatedItemsException() {
+        super("[ERROR] 장바구니에는 동일 상품을 중복으로 담을 수 없습니다.");
+    }


@MoonJeWoong 서비스가 복잡해져서, 에러케이스가 많아지면
custom Exception class가 아주 많이 늘어날 수 있음.
그런 경우에는 보통 에러케이스를 정리해서 에러코드로 변경하고,
에러코드와 에러메시지를 세트로 관리함 -> 보통 enum 클래스 사용.
custom Exception class는 해당 서비스에서 매니징 할수있는 제한된 갯수(1-2개?)만 생성

throw new CartWebException(CartWebErrorCode.DUPLICATED_ITEMS); public enum CartWebErrorCode { ..... DUPLICATED_ITEMS("장바구니에는 동일 상품을 중복으로 담을 수 없습니다."); .... }

MoonJeWoong added 25 commits May 1, 2023 18:00

refactor: ResponseStatus 대신 ResponseEntity를 반환하도록 변경

acc1dfc

refactor: Mapping URI 오타 수정

8ef2e61

refactor: 도메인 내 금액 상태 값 타입 int로 변경

913388e

test: 도메인 테스트 작성

90cc907

refactor: view, rest 컨트롤러 구분으로 인한 코드 및 테스트 수정

8c32058

docs: step2 요구사항 정리

82e37a2

feat: 사용자 설정 페이지 연동

0db6a8d

test: 뷰 컨트롤러 관련 기능 테스트 코드 통합

da5e488

refactor: component 클래스 명 변경

96c5583

feat: 사용자가 원하는 상품을 장바구니에 담는 기능 구현

3243267

fix: productsDao 내부 쿼리문 오류 수정

e7d40cd

feat: 사용자가 장바구니에 담은 상품들을 조회하는 기능 구현

983ad3c

feat: 중복된 상품은 장바구니에 담을 수 없도록 하는 기능 구현

5a8ddc7

feat: 장바구니 상품 삭제 기능 구현

7b0911a

feat: 헤더 인증 기능 구현

9b49436

refactor: product 관련 restAPI uri 변경

4a49f97

style: 패키지 명 대문자 수정

2a8a54c

test: 장바구니 관련 도메인 테스트 작성

1db1c64

refactor: 뷰 컨트롤러 통합

b0eec73

test: viewController 테스트 추가 작성

9f7c686

test: 장바구니 api관련 통합 테스트 작성

e95d2f9

style: 미사용 프로퍼티, import문 정리

9bccf78

docs: 요구사항 명세서 수정 및 API 명세 추가

5cf4ce9

refactor: delete 관련 버그 수정

1d8c971

세부사항 - cart 정보 응답 시 id값을 item이 아닌 product id로 응답하는 버그 수정

style: 불필요한 import 삭제

4771f26

choijy1705 requested changes May 6, 2023

View reviewed changes

MoonJeWoong added 4 commits May 6, 2023 23:51

refactor: 인증 관련 피드백 반영

989f34f

세부사항 - 어노테이션 명 변경 - BasicAuthorizationExtractor에서 null 대신 예외를 throw하도록 수정

refactor: Auth 패키지 reolver, interceptor 기능 리팩토링

21aec27

세부사항 - 기존 BasicAuthorizationExtractor 기능 LoginInterceptor로 이관 - AuthArgumentResolver는 request 메세지의 attribute를 추출해서 넘겨주도록 수정

refactor: 도메인 로직상 가져야할 프로퍼티들을 반드시 가지도록 수정

9107f40

refactor: ArgumentResolver에서 member 객체를 반환하도록 수정

61eef57

feat: LoginInterceptor에서 발생하는 예외 핸들링 기능 구현

bc7a30d

MoonJeWoong commented May 7, 2023

View reviewed changes

choijy1705 approved these changes May 7, 2023

View reviewed changes

choijy1705 merged commit 559aa5b into woowacourse:moonjewoong May 7, 2023

solarsalt reviewed Aug 1, 2023

View reviewed changes

	public Object resolveArgument(
	public AuthInfo resolveArgument(

		@@ -0,0 +1,20 @@
		package cart.dto.auth;

		public class AuthInfo {

		long memberId = membersDao.findIdByEmail(email);

		return membersDao.findById(memberId);

		request.setAttribute("email", email);
		request.setAttribute("password", password);

[2단계 - 장바구니 기능] 히이로(문제웅) 미션 제출합니다 #329

[2단계 - 장바구니 기능] 히이로(문제웅) 미션 제출합니다 #329

Uh oh!

Conversation

MoonJeWoong commented May 5, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

choijy1705 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MoonJeWoong left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

choijy1705 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MoonJeWoong commented May 5, 2023 •

edited

Loading