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

✨ [STMT-179] 스터디 활동 생성 기능 구현 #121

Merged
merged 9 commits into from
May 15, 2024

Conversation

zxcv9203
Copy link
Collaborator

@zxcv9203 zxcv9203 commented May 6, 2024

💁 해결 하려는 문제를 적어주세요

  • 스터디 활동 기능 구현

🤔 어떤 방식으로 해결했는지 적어주세요

  • 스터디 활동 기능을 구현했습니다.

🙋 중점적으로 리뷰 했으면 하는 부분이 있다면 적어주세요

  • 스터디 활동 생성시 Validation 체크에서 Enum 타입의 경우 Bean validation이 아닌 Jackson에서 Enum 변환 실패로 인한 에러가 발생합니다. 그래서 기존 Bean Validation 예외들과 통합이 안되는 문제가 있습니다. 이를 처리하기 위한 좋은 아이디어 있으신가요 🤔 (ExceptionHandler에서 예외를 다시 던지면 되지만 뭔가 우아한 방법이 아닌거같아서.. 😅)

@zxcv9203 zxcv9203 requested a review from 05AM as a code owner May 6, 2024 11:12
@zxcv9203 zxcv9203 self-assigned this May 6, 2024
@zxcv9203 zxcv9203 added the D-1 1일 전 까지 리뷰해주세요 label May 6, 2024
@github-actions github-actions bot added D-0 바로 리뷰가 필요해요 and removed D-1 1일 전 까지 리뷰해주세요 labels May 6, 2024
@05AM
Copy link
Collaborator

05AM commented May 7, 2024

  • 스터디 활동 생성시 Validation 체크에서 Enum 타입의 경우 Bean validation이 아닌 Jackson에서 Enum 변환 실패로 인한 에러가 발생합니다. 그래서 기존 Bean Validation 예외들과 통합이 안되는 문제가 있습니다. 이를 처리하기 위한 좋은 아이디어 있으신가요 🤔 (ExceptionHandler에서 예외를 다시 던지면 되지만 뭔가 우아한 방법이 아닌거같아서.. 😅)

사실 이에 대한 논의는 이전에 이루어졌던 것 같은데 결론은 명확히 나지 않았던 것 같습니다.
이전에 @jsonCreator@Converter, 인터셉터로 바인딩, @InitBinder나 custom validator 사용 등 많은 방법을 고민해보았으나 이미 바인딩된 객체가 아니라서 같은 유효성 검증을 하기 어렵고, 공통된 부모 예외를 던지기에 상세적인 예외처리가 어렵다는 결론에 다다랐습니다.

또한 저희가 같이 스터디하고 있는 만들면서 배우는 헥사고날 아키텍처에서는 도메인이나 db를 조회하지 않아도 알 수 있는 정보는 표현계층에서 처리하고, 비즈니스 로직과 관련되어 관련 정보가 필요한 것은 응용계층에서 검증하는 것이 바람직하다고 했던 것 같습니다.

이를 미루어 볼때 정석적으로 생각했을 때 컨트롤러에서는 enum을 string 값 그대로 받고, 응용 계층으로 넘어가는 dto(command/query)에서 해당 값을 검증하는 것이 클라이언트와의 결합도 없애고, 계층 간의 결합도 최소화할 수 있는 방법이 아닐까 생각합니다. 사실 enum은 domain 계층의 클래스인데 의존성 역전이라지만 요청 dto에서부터 결합되어 있다면 영향을 미칠 것이라고 생각합니다.

편의성뿐만 아니라 정보 은닉을 위한 캡슐화도 생각한다면 해당 방식에 더 이점이 있지 않을까 싶은데 이에 대해서 의견이 궁금합니다!

[참고]
https://velog.io/@saintho/URL-%EB%8B%A8%EC%B6%95-02
https://jaehoney.tistory.com/366
https://velog.io/@devmizz/Spring-Request%EB%A1%9C-Enum%EC%9D%B4-%EB%93%A4%EC%96%B4%EC%98%AC-%EB%95%8C

Copy link
Collaborator

@05AM 05AM left a comment

Choose a reason for hiding this comment

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

고생하셨습니다! 고민 많이 하셨을 것 같습니다! 👍👍👍

@zxcv9203
Copy link
Collaborator Author

zxcv9203 commented May 7, 2024

편의성뿐만 아니라 정보 은닉을 위한 캡슐화도 생각한다면 해당 방식에 더 이점이 있지 않을까 싶은데 이에 대해서 의견이 궁금합니다!

아주 좋은 의견 감사합니다 🙇 (감격)

말씀하신대로 도메인이나 DB를 조회하지 않아도 알 수 있는 로직은 어댑터 계층에서 처리하는게 맞다고 생각합니다.
Enum의 경우에는 이에 부합하는 타입이라는 생각이 듭니다. (Enum은 애플리케이션 로직에서 정의되어 있기 때문에 요청시 유효성 검사를 할 수 있음)

저 같은 경우에는 Enum 같은 경우에는 특정한 동작에 대한 상수의 나열이라고 생각합니다.
어떤면에서는 VO와 유사한 성격을 가진다고 생각이 들기도 합니다. (어떤 상황에 대한 정의나 추가적인 속성을 가짐으로써 특정 값에 대한 표현이 가능함)
Enum의 이러한 역할을 봤을 때 처리를 할 수 있다면 어댑터 계층에서 유효한 Enum인지를 체크하는 것이 좋은 형태라는 생각이 듭니다.

하지만 현재 문제는 Jackson 라이브러리와 Spring Bean Validation 간의 통합이 쉽게 일어나지 않기 때문에 각각의 예외에 대한 처리가 어려운게 문제인 것 같습니다. (Jackson 라이브러리에서 파싱 후 Bean validation이 동작하기 때문에 MethodArgumentsException이 아닌 HttpMessageNotReadableException이 발생하여 우리가 원하는 처리를 할 수 없음, HttpMessageNotReadableException는 BindResult를 사용하지 않음)

이를 해결 하기 위한 제 생각은 3가지가 있다고 생각합니다.

  1. 찬미님 의견대로 어댑터에서 검증을 포기하고 애플리케이션 로직에서 검증하기 (이 경우 다른 Bean Validation의 메시지를 여전히 확인할 수 없음)
  2. Jackson Deserializer를 이용하여 해당 Enum 클래스에 대해 동작을 커스텀하기 (챗지피티는 이렇게 하지 말라고 하더라고요 이유는 각 라이브러리는 그렇게 설계된게 아니다?)
  3. ExceptionHandler에서 HttpMessageNotReadableException를 잡고 MethodArgumentsException로 변환해서 던져준다 (뭔가.. 뭔가 우아하지 않음 + Enum에 대한 예외만 확인할 수 있음)

뭔가 Enum과 기존의 Bean Validation에 예외를 같이 나오게 할 기가막힌 아이디어가 없을지 고민이됩니다..

@05AM
Copy link
Collaborator

05AM commented May 8, 2024

우선 긴 글이 부담되실 수 있을 것 같아 죄송합니다..! ㅋㅋ 😅😅 내용은 아마 그정도의 부담은 아닐 것 같습니다

말씀하신대로 도메인이나 DB를 조회하지 않아도 알 수 있는 로직은 어댑터 계층에서 처리하는게 맞다고 생각합니다. Enum의 경우에는 이에 부합하는 타입이라는 생각이 듭니다. (Enum은 애플리케이션 로직에서 정의되어 있기 때문에 요청시 유효성 검사를 할 수 있음)

저 같은 경우에는 Enum 같은 경우에는 특정한 동작에 대한 상수의 나열이라고 생각합니다. 어떤면에서는 VO와 유사한 성격을 가진다고 생각이 들기도 합니다. (어떤 상황에 대한 정의나 추가적인 속성을 가짐으로써 특정 값에 대한 표현이 가능함) Enum의 이러한 역할을 봤을 때 처리를 할 수 있다면 어댑터 계층에서 유효한 Enum인지를 체크하는 것이 좋은 형태라는 생각이 듭니다.

저는 Enum에 대해서 조금 다른 생각을 가지고 있는 것 같습니다. 현재 Activity Category 등의 Enum 타입의 경우 응용 계층이 아닌 도메인 계층에 정의되어 있는 도메인 계층의 클래스입니다.

또한 Enum은 저희 코드에서 단순 상수 이상의 역할을 하고 있습니다. 구현하신 것처럼 각 카테고리의 default 상태에 대한 정의를 하기도 하고, 심지어 Activity를 create하는 역할도 수행하고 있습니다.

저는 이 역할을 수행하는 것이 잘못되었다고 생각하지 않고, 오히려 잘 수행되고 있다고 생각합니다. 하지만 문제는 이를 controller의 request dto에서 그대로 받고 있다는 것이 문제인 것 같습니다.

클라이언트는 이게 enum이라서 잘못 된건지 알 필요가 없다고 생각이 듭니다. 클라이언트는 요청 규약을 잘 지켜서 string으로 이름을 보냈지만 그게 도메인에서 정의된 것과 다른 유효하지 않은 값이기 때문에 에러가 발생하여 해당 에러에 대한 메시지가 전달되는 것이 자연스러운 것 같습니다.

현재 용철님이 고민하시는 부분의 원인은 도메인 계층의 클래스 (도메인의 정보를 알아야지만 검증할 수 있는 클래스)를 컨트롤러에서 검증하려고 하기 때문이라고 생각합니다.

또한 bean validation이나 jackson library가 공통된 예외를 던지고, 상세한 예외처리가 지원되기 어려운 것도 해당 부분에 대한 필요성을 느끼지 못해서 그럴 수 있다는 생각도 들었습니다.
에러가 생기는 경우는 도메인 로직을 통한 검사 결과 enum을 변환하지 못해서가 아닌, 단순히 현재 계층에서 검증할 수 있는 간단한 규약을 클라이언트가 지키지 못한 결과로 발생한 예외에 대한 메시지를 반환하는 것이 해당 라이브러리의 목적이라는 생각이 듭니다.

또한 이는 굳이 bean validation으로 통합하여 하지 않아도 되는 일이라고 생각합니다. 클라이언트는 어쨋든 형식을 잘 지켜서 보냈지만 로직에 유효하지 않은 값이었을 뿐이라고 생각합니다.

마치 memberId를 보냈지만 해당 멤버가 db에 없는 멤버인 것과 같다고 생각합니다.

그래서 저는 사실 아래의 말씀에 대한 이해를 잘 하지 못했습니다.

하지만 현재 문제는 Jackson 라이브러리와 Spring Bean Validation 간의 통합이 쉽게 일어나지 않기 때문에 각각의 예외에 대한 처리가 어려운게 문제인 것 같습니다. (Jackson 라이브러리에서 파싱 후 Bean validation이 동작하기 때문에 MethodArgumentsException이 아닌 HttpMessageNotReadableException이 발생하여 우리가 원하는 처리를 할 수 없음, HttpMessageNotReadableException는 BindResult를 사용하지 않음)

저는 해당 값을 string으로 받고 command에서 enum으로 변환하거나 서비스 내부에서 enum으로 변환하는 것이 자연스러운 과정인 것 같습니다. 그 과정에서 해당 도메인 클래스인 enum에서 비즈니스 로직을 통해 유효성 검사를 하고 유효하지 않을 경우 예외를 던지는 것이 자연스러운 것 같습니다. 이 경우에는 원하는 예외처리도 가능합니다.

아래는 Enum 타입의 StudyField 클래스의 코드입니다.

    public static StudyField getByName(String name) {
        return Arrays.stream(StudyField.values())
                .filter(field -> field.name.equals(name))
                .findFirst()
                .orElseThrow(() -> new StudyFieldNotExistsException(name));
    }

제가 생각하기에는 command에서는 enum에 대한 String 값을 받고, command의 생성자 내부나 서비스의 코드에서 Enum 타입을 받아오는 것이 가장 걸리는 점 없이 좋아 보이는 것 같습니다.

그렇게 되면 위의 답글에서 말씀드린 command에서 createActivity 등의 메서드를 만들어서 호출하지 않아도 서비스 코드에서 enum 타입의 category를 받기에 해당 객체에서 직접 create 메서드를 호출해도 문제가 없을 것이라 생각합니다.

그리고 지금 막 든 생각인데, command나 dto 사이의 비즈니스 로직을 수행하는 다른 개념이 없는 이유가 이미 해당하는 개념이 존재하거나, 해당 개념이 존재할 필요가 없을 수도 있겠다는 생각이 들었습니다.

해당하는 개념이 존재한다는 가설로 생각나는 것은 vo나 domain인 것 같습니다. 하지만 아직 생각이 정리되지 않아 이에 대해서 더 찾아보고 논의가 이루어진다면 좋을 것 같습니다.

해당 방법은 프레임워크 라이브러리에 의존하지 않고 아키텍처의 이념에도 (개인적으로 생각했을 때) 부합한다고 생각합니다. 이에 대한 의견을 공유해주시면 감사하겠습니다!

@zxcv9203
Copy link
Collaborator Author

zxcv9203 commented May 8, 2024

찬미님이 말씀하시는 이야기의 의도는 이해했습니다.

제가 이해한 내용은 카테고리는 우리의 비즈니스 규칙이니 비즈니스 로직을 담당하는 애플리케이션 계층에서 카테고리의 변환 및 검증이 이루어져야 한다라고 이해했습니다.

하지만 이미 잘못 검증된 값임을 알고 있음에도 애플리케이션 계층으로 넘어가 로직을 수행하는 것은 불필요한 행동이라는 생각이 듭니다.
이러한 상황을 막기위해 비즈니스 규칙상 수행할 수 없는 값들에 대해 Command라는 입력 모델을 통해 최소한의 검증과정을 거치는 것이라고 생각합니다.

예를 들어 단순 숫자같은 경우에 음수던지, 0이던지, 양수이던지 모두 유효한 숫자라고 볼 수 있을 것 같습니다.
이때 구현하려는 기능이 계좌 송금이라는 기능이라면 그때는 음수와 0은 더 이상 유효한 값이 아니라는 게 비즈니스 규칙으로 세워질 수 있을 것 같습니다.

제가 생각하는 애플리케이션 계층 혹은 도메인 계층에서 검증해야 하는 경우는 현재 상태를 기준으로 유효하지 않은 상황이 발생할 수 있는 경우라고 생각합니다. (쉽게 말하면 영속화된 데이터를 가져와서 어떠한 검증을 해야하는 경우라고 할 수 있을 것 같습니다.)
예를 들어 내가 가진 금액 이상으로 송금을 시도시 예외를 발생시키는 식으로 검증을 진행할 수 있을 것 같습니다.

그리고 Enum을 애플리케이션 로직에서 변경을 진행하게 된다면 해당 Enum을 이용해야하는 경우 계속하여 유스케이스 별로 변환 작업을 수행해야 한다는 점 역시 아쉬운 것 같습니다.
우리가 유효하다고 정한 상태에 따라 변하지 않는 값이 있다면 (카테고리와 같은 상수) 입력 모델에서 바로 변환해서 전달해주는게 효율적으로 동작하는 애플리케이션을 만들 수 있다고 생각합니다.

또한 bean validation이나 jackson library가 공통된 예외를 던지고, 상세한 예외처리가 지원되기 어려운 것도 해당 부분에 대한 필요성을 느끼지 못해서 그럴 수 있다는 생각도 들었습니다.

제가 공통된 처리를 하고 싶은 이유는 클라이언트 측에서 잘못된 값을 던졌을때 하나하나 예외를 던진다면 클라이언트는 여러개의 데이터가 유효하지 못한 상태로 전달했음에도 서버에 여러번 요청에서 사실을 알 수 있다는 문제가 발생할 것 같습니다.
한번만 요청했어도 다 고칠 수 있던 상황이 API를 여러번 호출해서 불 필요하게 리소스를 낭비하게된다는 단점이 있을 것 같습니다.

Jackson과 BeanValidation의 통합을 말씀드린 이유는 Jackson이 우선순위로 동작함으로써 다른 Bean Validation으로 검증하는 필드들도 잘못되었지만 클라이언트에서는 이를 알 수 없다는 점이 아쉬운 것 같습니다.

더 자세한 이야기는 텍스트보다 직접 말로 이야기하면 더 좋겠다는 생각이 드네요.!

@zxcv9203 zxcv9203 changed the title ✨ [STMT-179] 스터디 활동 기능 구현 ✨ [STMT-179] 스터디 활동 생성 기능 구현 May 12, 2024
Copy link
Collaborator

@05AM 05AM left a comment

Choose a reason for hiding this comment

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

고생하셨습니다!

@zxcv9203 zxcv9203 merged commit 402d36c into dev May 15, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
D-0 바로 리뷰가 필요해요
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants