로그인 만들기

[giibeom]

강의 내용이 좀 어려워서 하루 늦게 PR 요청드리네요!
이번 주는 FakeObject를 사용하는 대신 Mock을 사용해서 TDD 하는 연습을 해보려합니다!

이번주도 잘 부탁드립니다 :)

[hannut91]

에러 응답을 구조화할려고 하신 것 같아요. 비슷한 내용을 공부한 적이 있어서 참고로 남겨드립니다

https://codesoom.github.io/wiki/api-design/07/#%ED%94%BC%EB%93%9C%EB%B0%B1-%EA%B5%AC%EC%A1%B0%ED%99%94-%ED%95%98%EA%B8%B0

[ssisksl77]

exception 패키지에 존재하는 이유를 조금 설명해주시면 감사하겠습니다

[giibeom]

예외 객체를 구조화 하다보니 아래와 같이 도메인 별로 exceptionHandler를 묶어주는 구조로 잡아보았습니다!

따라서 각 도메인의 @ControllerAdvice도 exception을 핸들링하려는 용도여서 exception 패키지에 소속시키긴 했습니다만, 문득 controller 패키지에 있어야 하나..? 라는 생각이 들기도 하네요😂
어디 패키지에 있어야 좀 더 자연스러울까요?

[생각 1]

해당 커스텀 어노테이션의 이름을 ProductControllerAdvice -> ProductErrorAdvice로 변경하는 것이 더 자연스러울까요?

[생각 2]

패키지를 controller로 옮긴다면 아래와 같은 @ProductControllerAdvice가 선언되어 있는 ProductExceptionHandler도 같이 옮겨야 할까요?

@ProductErrorAdvice
public class ProductExceptionHandler {

    @ResponseStatus(HttpStatus.NOT_FOUND)
    @ExceptionHandler(ProductNotFoundException.class)
    public ErrorResponse handleProductNotFound() {
        return new ErrorResponse("Product not found");
    }
}

[ssisksl77]

음...!!! 커스텀 어노테이션이 controller를 의존하는 것이 걸립니다!

[ssisksl77]

서비스 레이어의 로직이나 ProductController 를 바라보고 있다는 점이요!

[giibeom]

잘 이해가 되지 않아 조금만 더 설명해주실 수 있으신가요??
어떻게 해야될지 감이 잘 안잡히는 거 같네요😂

일단 basePackageClasses를 사용하여 직접 관련 컨트롤러들을 지정해주기 된 계기는, 기존 basePackages로 패키지 경로를 문자열로 설정하기에는 문자열 하드코딩 자체가 별로 좋아보이지 않아서 선택한 결정이였습니다!
따라서 product 도메인의 관련된 모든 예외를 응집하여 핸들링하기 위해 product 관련 컨트롤러들을 basePackageClasses에 모아서 선언해봐야겠다는 생각이였습니다

@RestControllerAdvice(basePackageClasses = {
        ProductController.class // 상품 관련 컨트롤러 클래스들을 모두 모아놓는 곳
})
@Order(Ordered.HIGHEST_PRECEDENCE)
public @interface ProductErrorAdvice {
}

controller를 직접 의존하지 않고 도메인 별로 예외 핸들링을 할 수 있는 다른 좋은 방법이 있을까요?

[ssisksl77]

RestControllerAdvice를 다시 보면 ControllerAdvice와 ResponseBody 어노테이션이 있습니다. 도메인에서 벌써 Response가 어떻게 될지를 아는 것은 MVC 패턴과는 조금 다르다고 생각해요

https://docs.spring.io/spring-framework/docs/current/javadoc-api/org/springframework/web/bind/annotation/RestControllerAdvice.html

[ssisksl77]

기범님의 의중이 레이어에 상관없이 핸들링하고 싶다면 common 같은 곳을 두어 레이어에 상관없이 모두 아우른다는 의사표현을 하는 것이 가장 나은 방법 같네요!

[giibeom]

먼저 기존에 몰랐던 새로운 공부 주제들을 계속해서 추천해주셔서 정말 감사드립니다!
헥사고날도 그렇고 멘토님 덕분에 시야가 계속해서 넓어지는 것 같아요😊

(d16fb31) 일단 커밋은 마지막에 추천해주신 방식대로 @xxxxErrorAdvice 커스텀 어노테이션은 삭제하고 일단 ErrorAdvice 클래스는 common의 error 패키지에다 모아놓았습니다!
또한 각 도메인 별 Exception들은 기존대로 각 도메인 패키지 내부의 exception 패키지에 배치해놓았구요

확실히 드는 생각은 basePackageClasses에 컨트롤러가 직접 의존하는 것이 그렇게 이뻐 보이진 않은 것 같아서 추천해주신 AOP를 사용하여 의존을 끊어보려하고있습니다..! (이런식으로 특정 컨트롤러만 어드바이스 하도록 지정해준다면 관리포인트가 하나 더 늘어난다고 생각이 들었습니다... 안좋은 방법인거 같아욥...)

@RestControllerAdvice(basePackageClasses = {
        ProductController.class
})
@Order(Ordered.HIGHEST_PRECEDENCE)
public class ProductErrorAdvice {

    @ResponseStatus(HttpStatus.NOT_FOUND)
    @ExceptionHandler(ProductNotFoundException.class)
    public ErrorResponse handleProductNotFound() {
        return new ErrorResponse("Product not found");
    }
}

[giibeom]

어떤 어노테이션이 존재하는 경우 우리는 도메인 예외 A가 던져지면 B로 바꿔서 던질 수 있을 것입니다.

이 부분을 너무 해보고 싶은데, 예시에서 @Around("@annotation( 어노테이션 이름 )") 어노테이션이 @Target(ElementType.METHOD)인 어노테이션을 지정해주어야 AOP를 통해 잡히고, @Target(ElementType.TYPE)인 어노테이션은 잡히지 않는 것으로 확인되었습니다.

[기존 ProductErrorAdvice를 Around에 지정한 예시 코드]

import org.aspectj.lang.ProceedingJoinPoint;
import org.aspectj.lang.annotation.Around;
import org.aspectj.lang.annotation.Aspect;
import org.springframework.stereotype.Component;

@Aspect
@Component
public class ExampleAspect {
    @Around("@annotation(com.codesoom.assignment.domain.product.exception.ProductErrorAdvice)")
    public Object logExecutionTime(ProceedingJoinPoint joinPoint) throws Throwable {
        System.out.println(joinPoint);   // 이쪽으로 들어오지 않음 ㅠㅠ
        return joinPoint.proceed();
    }
}

하지만 시험 삼아 어노테이션의 ElementType이 Method인 @ExceptionHandler으로 @Around에 어노테이션을 지정해줬더니 잡히는 것을 확인하였습니다

[ExceptionHandler를 Around에 지정한 예시 코드]

@Aspect
@Component
public class ExampleAspect {
    @Around("@annotation(org.springframework.web.bind.annotation.ExceptionHandler)")
    public Object logExecutionTime(ProceedingJoinPoint joinPoint) throws Throwable {
        System.out.println(joinPoint);   // 모든 예외가 터질 때 ExceptionHandler로 가기전에 일로 들어옴!!!
        return joinPoint.proceed();
    }
}

그렇다면 ExceptionHandler로 모든 예외를 AOP로 잡는 방법을 추천해주신 것이 맞는건지요??
근데 한가지 궁금한게 모든 ExceptionHandler를 잡는다고 가정했을 때,
우리는 도메인 예외 A가 던져지면 B로 바꿔서 던질 수 있을 것입니다.
다른 예외로 던지는 방법을 말씀해주셨는데, 어떤 예외를 바꿔 던져야할지 모르겠습니다..!

[ssisksl77]

아 그것은 컨트롤러 레이어에 있는 예외(만들어야하는)를 말하는 것이었습니다.

AOP를 사용하면 컨트롤러에서 핸들링하는 예외를 따로 만들고 그것을 던진다. 그리고 컨트롤러에서 그것을 핸들링하는 코드는 따로 있을 것으로 기대된다.

입니다.

[ssisksl77]

error 라는 이름을 쓰셨군요!

제가 잠시 놓쳤던 것이 있습니다. 자바에서 Error는 Exception과 다릅니다!

https://docs.oracle.com/javase/7/docs/api/java/lang/Error.html

[giibeom]

제가 도메인 패키지쪽에 있는 exception과 좀 차별점을 두고 싶은 마음에 패키지 이름을 error라고 지정해주었는데, 생각해보니 말씀해주신대로 error와 exception은 다른 개념이네요
그렇다면 common 패키지에서의 exception 패키지와, 각 도메인 별 exception 패키지가 존재해도 별로 이상해보이지 않을까요?

[giibeom]

질문 드렸던 것에 대한 예시입니다
현재 구조에서 좀 더 개선되면 좋을만한 점이 있을 지 궁금합니다!

common

• exception
  • ProductExceptionHandler

product

• exception
  • ProductNotFoundException
• presentation
  • ProductController
• application
  • ProductService
• infra
  • JpaProductRepository
• domain
  • Product
  • ProductRepository

추가로 저번주까지 헥사고날을 연습했다가, 다양하게 이것저것 해보고싶어서 이번주에는 해당 아키텍처로 진행해보았습니다!
마지막 주차인 8주차때는 멘토님께서 알려주신 헥사고날로 진행하면 과정을 마무리 해보려고 합니다!

[ssisksl77]

흥미롭군요!

product안에 controller도 들어가는 걸까요?

[giibeom]

기존대로 도메인 안에 또 도메인이 있는게 약간 이상하게 생각이 들어서 변경해보았습니다..!
domain > product > domain이 있는게 좀 어색해보여가지구요

따라서 바꾼 구조에서 product 안에는 아래와 같이 5개의 패키지로 구성해보았습니다

• presentation
• application
• infra
• exception
• domain

혹시 최상단을 domain으로 한번 묶어주는 것이 좋을까요?

[ssisksl77]

음... 일단 application, presentation, domain은 어떻게 다른가요?

[ssisksl77]

또한 현재 기범님께서는 Product.java 객체에는 아마 비즈니스 로직을 넣지 않는 것을 선호하시는 것으로 보입니다
맞을까요?

[giibeom]

presentation계층은 외부환경과 유스케이스를 연결시켜주는 어댑터의 계층이라고 생각합니다!
application은 실제 비지니스 로직을 담당하는 계층이라고 생각합니다!
domain은 실제 비지니스 로직을 제외한 해당 도메인에 관련된 로직을 담당하는 계층이라고 생각합니다

• ex) User 객체에서 데이터 값에 변경을 담당, List<User>에서 특정 도메인의 이메일인 사람을 찾는 로직 등

[giibeom]

또한 현재 기범님께서는 Product.java 객체에는 아마 비즈니스 로직을 넣지 않는 것을 선호하시는 것으로 보입니다 맞을까요?

Product에는 비지니스 로직을 제외한 도메인 자체의 로직을 집어넣는 것이 좋다고 생각하고 있습니다..!
위에서 예시를 든 것처럼 만약 상품 정보를 수정하는 로직은 Product.java 객체에서 수행하는 것이 좋다고 생각합니다
그것이 Product 객체를 일급 컬렉션으로 관리하는데 도움이 된다고 알고 있습니다!

[giibeom]

update 같은 경우에는 DB I/O가 아닌, Product 객체의 멤버 값을 변경하는 기능이므로 Product 객체가 담당하는것이 좋을 것 같다는 관점입니다

[ssisksl77]

말씀하신 개념과 service / repository 와는 어떻게 다른가요?

[ssisksl77]

그것이 Product 객체를 일급 컬렉션으로 관리하는데 도움이 된다고 알고 있습니다! 이것에 대해서 좀 더 알고 싶습니다.

[giibeom]

말씀하신 개념과 service / repository 와는 어떻게 다른가요?

service는 유스케이스를 달성하기 위해 비지니스 로직을 수행하는 역할을 담당하고, repository는 DB의 커넥션을 맺으며 I/O 를 수행하는 역할을 담당한다고 생각합니다

---

그것이 Product 객체를 일급 컬렉션으로 관리하는데 도움이 된다고 알고 있습니다! 이것에 대해서 좀 더 알고 싶습니다.

Product 객체의 상태와 행위를 Product 클래스에 모두 응집시켜야 유지보수에 좋은 것으로 알고 있습니다
해당 자료를 참고하면서 생각해 본 내용이였습니다

[ssisksl77]

d16fb31 커밋이 단순히 이관만 한 것처럼 보이지는 않습니다! 뭔가 많은 것이 바뀐 것 처럼 보입니다.

[giibeom]

d16fb31 커밋이 단순히 이관만 한 것처럼 보이지는 않습니다! 뭔가 많은 것이 바뀐 것 처럼 보입니다.

패키지 구조에서 최상단의 domain을 삭제하여서 변경 파일이 많아진 것 같습니다
또한 기존 각 예외 핸들링을 위한 커스텀 어노테이션을 제거하였는데, 이 부분은 커밋을 분리하는 것이 좋았을 것 같다는 생각이 드네요
커밋 단위를 어떤식으로 관리해야할지... 솔직히 잘 감이 오지 않습니다ㅠ

[ssisksl77]

d16fb31 커밋이 단순히 이관만 한 것처럼 보이지는 않습니다! 뭔가 많은 것이 바뀐 것 처럼 보입니다.

패키지 구조에서 최상단의 domain을 삭제하여서 변경 파일이 많아진 것 같습니다 또한 기존 각 예외 핸들링을 위한 커스텀 어노테이션을 제거하였는데, 이 부분은 커밋을 분리하는 것이 좋았을 것 같다는 생각이 드네요 커밋 단위를 어떤식으로 관리해야할지... 솔직히 잘 감이 오지 않습니다ㅠ

커밋타이틀에 작업한 하나의 일을 적고 그것만 들어있도록 하는 것이 시작입니다. 어렵게 생각하지 않아도 될 것 같습니다.

[giibeom]

커밋타이틀에 작업한 하나의 일을 적고 그것만 들어있도록 하는 것이 시작입니다. 어렵게 생각하지 않아도 될 것 같습니다.

커밋을 너무 잘게 쪼개봤더니, 커밋이 너무 많으면 리뷰하기 힘들다는 단점이 있다고 해서... 약간 혼란스러웠습니다
커밋을 어떤 단위로 타협점을 찾아야 할까요?
회원 생성 기능을 예시로 들어보면 아래와 같은 방식들이 있을 것 같습니다

• 기능 단위: 회원 생성 관련 모든 C/S/R 구현 및 테스트
• 계층 단위: 회원 생성 Service 구현 및 테스트
• 테스트/구현 단위
  • 커밋 1. 회원 생성 Service 테스트
  • 커밋 2. 회원 생성 Service 구현

어떤 단위가 코드리뷰를 진행하는 리뷰어 입장에서 보기 편할 지 궁금합니다!
체계적인 환경에서의 경험이 없다보니 어려운 것 같아요 😂😂

[ssisksl77]

커밋타이틀에 작업한 하나의 일을 적고 그것만 들어있도록 하는 것이 시작입니다. 어렵게 생각하지 않아도 될 것 같습니다.

커밋을 너무 잘게 쪼개봤더니, 커밋이 너무 많으면 리뷰하기 힘들다는 단점이 있다고 해서... 약간 혼란스러웠습니다 커밋을 어떤 단위로 타협점을 찾아야 할까요? 회원 생성 기능을 예시로 들어보면 아래와 같은 방식들이 있을 것 같습니다

• 기능 단위: 회원 생성 관련 모든 C/S/R 구현 및 테스트

• 계층 단위: 회원 생성 Service 구현 및 테스트

• 테스트/구현 단위

  • 커밋 1. 회원 생성 Service 테스트
  • 커밋 2. 회원 생성 Service 구현

어떤 단위가 코드리뷰를 진행하는 리뷰어 입장에서 보기 편할 지 궁금합니다! 체계적인 환경에서의 경험이 없다보니 어려운 것 같아요 😂😂

커밋을 단순히 파일단위로 쪼개서 무조건 작은 단위로 만드는 것은 좋지 않습니다.
만들어진 기능단위에서 작은 단위로 커밋이 만들어지면 읽기 좋을 것 같습니다. (테스트 포함)

코드 리뷰가 쉽지 않은 이유는 대부분 커밋의 개수보다는 파일의 개수에 있습니다.

물론 커밋의 개수가 파일의 개수를 말하는 것이기도 하지만 파일의 개수가 많아지는 것 만으로 변경을 헤아리기가 쉽지 않습니다.

실제 업무에서는 이렇게 많은 파일량이 있도록 PR을 올리면 안됩니다.
아마도 동료들은 PR을 다시 올려달라고 하거나, 세세하게 리뷰를 하지 못하고 승인을 할 수도 있습니다.