-
Notifications
You must be signed in to change notification settings - Fork 0
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
Conversation
사실 이에 대한 논의는 이전에 이루어졌던 것 같은데 결론은 명확히 나지 않았던 것 같습니다. 또한 저희가 같이 스터디하고 있는 이를 미루어 볼때 정석적으로 생각했을 때 컨트롤러에서는 enum을 string 값 그대로 받고, 응용 계층으로 넘어가는 dto(command/query)에서 해당 값을 검증하는 것이 클라이언트와의 결합도 없애고, 계층 간의 결합도 최소화할 수 있는 방법이 아닐까 생각합니다. 사실 enum은 domain 계층의 클래스인데 의존성 역전이라지만 요청 dto에서부터 결합되어 있다면 영향을 미칠 것이라고 생각합니다. 편의성뿐만 아니라 정보 은닉을 위한 캡슐화도 생각한다면 해당 방식에 더 이점이 있지 않을까 싶은데 이에 대해서 의견이 궁금합니다! [참고] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
고생하셨습니다! 고민 많이 하셨을 것 같습니다! 👍👍👍
src/main/java/com/stumeet/server/activity/adapter/in/ActivityCreateApi.java
Outdated
Show resolved
Hide resolved
src/main/java/com/stumeet/server/activity/adapter/out/model/ActivityImageJpaEntity.java
Show resolved
Hide resolved
...main/java/com/stumeet/server/activity/application/port/in/command/ActivityCreateCommand.java
Show resolved
Hide resolved
src/main/java/com/stumeet/server/activity/application/port/in/mapper/ActivityUseCaseMapper.java
Outdated
Show resolved
Hide resolved
src/main/java/com/stumeet/server/activity/application/port/in/ActivityCreateUseCase.java
Outdated
Show resolved
Hide resolved
src/main/java/com/stumeet/server/activity/application/service/ActivityCreateService.java
Outdated
Show resolved
Hide resolved
src/main/java/com/stumeet/server/activity/application/service/ActivityCreateService.java
Outdated
Show resolved
Hide resolved
src/main/java/com/stumeet/server/activity/domain/model/ActivityCategory.java
Show resolved
Hide resolved
src/test/java/com/stumeet/server/activity/application/service/ActivityCreateServiceTest.java
Show resolved
Hide resolved
아주 좋은 의견 감사합니다 🙇 (감격) 말씀하신대로 도메인이나 DB를 조회하지 않아도 알 수 있는 로직은 어댑터 계층에서 처리하는게 맞다고 생각합니다. 저 같은 경우에는 Enum 같은 경우에는 특정한 동작에 대한 상수의 나열이라고 생각합니다. 하지만 현재 문제는 Jackson 라이브러리와 Spring Bean Validation 간의 통합이 쉽게 일어나지 않기 때문에 각각의 예외에 대한 처리가 어려운게 문제인 것 같습니다. (Jackson 라이브러리에서 파싱 후 Bean validation이 동작하기 때문에 MethodArgumentsException이 아닌 HttpMessageNotReadableException이 발생하여 우리가 원하는 처리를 할 수 없음, HttpMessageNotReadableException는 BindResult를 사용하지 않음) 이를 해결 하기 위한 제 생각은 3가지가 있다고 생각합니다.
뭔가 Enum과 기존의 Bean Validation에 예외를 같이 나오게 할 기가막힌 아이디어가 없을지 고민이됩니다.. |
우선 긴 글이 부담되실 수 있을 것 같아 죄송합니다..! ㅋㅋ 😅😅 내용은 아마 그정도의 부담은 아닐 것 같습니다
저는 Enum에 대해서 조금 다른 생각을 가지고 있는 것 같습니다. 현재 Activity Category 등의 Enum 타입의 경우 응용 계층이 아닌 도메인 계층에 정의되어 있는 도메인 계층의 클래스입니다. 또한 Enum은 저희 코드에서 단순 상수 이상의 역할을 하고 있습니다. 구현하신 것처럼 각 카테고리의 default 상태에 대한 정의를 하기도 하고, 심지어 Activity를 create하는 역할도 수행하고 있습니다. 저는 이 역할을 수행하는 것이 잘못되었다고 생각하지 않고, 오히려 잘 수행되고 있다고 생각합니다. 하지만 문제는 이를 controller의 request dto에서 그대로 받고 있다는 것이 문제인 것 같습니다. 클라이언트는 이게 enum이라서 잘못 된건지 알 필요가 없다고 생각이 듭니다. 클라이언트는 요청 규약을 잘 지켜서 string으로 이름을 보냈지만 그게 도메인에서 정의된 것과 다른 유효하지 않은 값이기 때문에 에러가 발생하여 해당 에러에 대한 메시지가 전달되는 것이 자연스러운 것 같습니다. 현재 용철님이 고민하시는 부분의 원인은 도메인 계층의 클래스 (도메인의 정보를 알아야지만 검증할 수 있는 클래스)를 컨트롤러에서 검증하려고 하기 때문이라고 생각합니다. 또한 bean validation이나 jackson library가 공통된 예외를 던지고, 상세한 예외처리가 지원되기 어려운 것도 해당 부분에 대한 필요성을 느끼지 못해서 그럴 수 있다는 생각도 들었습니다. 또한 이는 굳이 bean validation으로 통합하여 하지 않아도 되는 일이라고 생각합니다. 클라이언트는 어쨋든 형식을 잘 지켜서 보냈지만 로직에 유효하지 않은 값이었을 뿐이라고 생각합니다. 마치 memberId를 보냈지만 해당 멤버가 db에 없는 멤버인 것과 같다고 생각합니다. 그래서 저는 사실 아래의 말씀에 대한 이해를 잘 하지 못했습니다.
저는 해당 값을 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인 것 같습니다. 하지만 아직 생각이 정리되지 않아 이에 대해서 더 찾아보고 논의가 이루어진다면 좋을 것 같습니다. 해당 방법은 프레임워크 라이브러리에 의존하지 않고 아키텍처의 이념에도 (개인적으로 생각했을 때) 부합한다고 생각합니다. 이에 대한 의견을 공유해주시면 감사하겠습니다! |
찬미님이 말씀하시는 이야기의 의도는 이해했습니다. 제가 이해한 내용은 하지만 이미 잘못 검증된 값임을 알고 있음에도 애플리케이션 계층으로 넘어가 로직을 수행하는 것은 불필요한 행동이라는 생각이 듭니다. 예를 들어 단순 숫자같은 경우에 음수던지, 0이던지, 양수이던지 모두 유효한 숫자라고 볼 수 있을 것 같습니다. 제가 생각하는 애플리케이션 계층 혹은 도메인 계층에서 검증해야 하는 경우는 현재 상태를 기준으로 유효하지 않은 상황이 발생할 수 있는 경우라고 생각합니다. (쉽게 말하면 영속화된 데이터를 가져와서 어떠한 검증을 해야하는 경우라고 할 수 있을 것 같습니다.) 그리고 Enum을 애플리케이션 로직에서 변경을 진행하게 된다면 해당 Enum을 이용해야하는 경우 계속하여 유스케이스 별로 변환 작업을 수행해야 한다는 점 역시 아쉬운 것 같습니다.
제가 공통된 처리를 하고 싶은 이유는 클라이언트 측에서 잘못된 값을 던졌을때 하나하나 예외를 던진다면 클라이언트는 여러개의 데이터가 유효하지 못한 상태로 전달했음에도 서버에 여러번 요청에서 사실을 알 수 있다는 문제가 발생할 것 같습니다. Jackson과 BeanValidation의 통합을 말씀드린 이유는 Jackson이 우선순위로 동작함으로써 다른 Bean Validation으로 검증하는 필드들도 잘못되었지만 클라이언트에서는 이를 알 수 없다는 점이 아쉬운 것 같습니다. 더 자세한 이야기는 텍스트보다 직접 말로 이야기하면 더 좋겠다는 생각이 드네요.! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
고생하셨습니다!
💁 해결 하려는 문제를 적어주세요
🤔 어떤 방식으로 해결했는지 적어주세요
🙋 중점적으로 리뷰 했으면 하는 부분이 있다면 적어주세요