-
Notifications
You must be signed in to change notification settings - Fork 0
[REFACTOR] Support 모듈 분리 #403
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
Conversation
- 공통 모듈 생성 (Eception, BaseEntity) - 로직 변경 없이 파일만을 옮김
- URL과 Method, Parameter의 경우, OncePerRequestFilter를 사용하여 필터 가장 앞 단에서 잡도록 함 - MDC를 사용하여, TraceId를 요청당 특정 짓도록 함 - Request Body의 경우, RequestBodyAdviceAdapter를 사용하여 로직 수행 이전 가로 채도록 함
- ASYNC를 사용하여, 콘솔 로그 I/O 작업이 API 호출에 영향이 없도록 함
- Filter에서 Body 값을 캐싱하지 않기 때문에 삭제
- group, version 등의 설정값을 외부 프로퍼티로 분리 - bootJar 파일 설정을 루트 Gradle로 이전 - plugins 버전을 settings.gradle로 일괄 적용
sansan20535
left a comment
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.
정말 고생하셨습니다!
| } | ||
| } | ||
|
|
||
| dependencies { |
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.
p5; 의존성을 모듈마다 분리하는 형식에 대해 여쭤보고 싶습니다!
현재는 DB, Starters, Security, Util과 관련한 의존성 root 모듈에 모두 모여있는 것 같습니다. 만약 이후 DB 모듈 등 모듈을 분리하고 관련된 의존성을 옮기면 해당 모듈의 역할이 분명해지고 유지보수가 쉬워질 것 같다는 생각이 들었습니다. 혹시 이러한 방식에 대한 트레이드오프가 있을까요?!
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.
- 제가 아래에서 리뷰를 하고 왔는데 중복되는 의존성이 있는 것 같았습니다! 혹시 이에 대한 특별한 의도가 있으신지가 궁금합니다!
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.
우선 초기 모듈 분리를 MSA 변경을 고려하여 각각의 독립적인 서버로 구성할 수 있는 단위로 구분 지었습니다.
이후에 각 모듈 내부에서 API (Spring Boot 의존성), 구현체 (DB, 외부 인프라 의존성), 도메인 (POJO) 처럼 또 나누게 된다면 의존성을 확실하게 분리할 수 있는 장점이 있습니다.
하지만 이 경우에는 패키지가 많아지다 보니 유지보수 관점에서 편할 수도 있지만, 규모 대비 너무 많은 폴더로 약간의 불편함을 느낄 수도 있습니다.
의존성 분리는 해당 프로젝트의 API, Admin 아키텍처를 확인해보시면 됩니다.
subproject와 루트 project에서의 의존성이 같이 들어있는거는 중복이 아닙니다! 해당 내용은 Gradle 노션 문서를 참고해주세요!
만약 이 내용이 아닌 중복은 어떤 의존성이 중복됐는지 말씀해주시면 확인해보겠습니다 :)
...vice-support/src/main/java/org/websoso/support/logging/request/RequestBodyLoggingAdvice.java
Outdated
Show resolved
Hide resolved
|
|
||
| @Slf4j | ||
| @Component | ||
| @Order(Ordered.HIGHEST_PRECEDENCE) |
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.
p5; 명시적으로 security filter에서 순서를 설정하는 방법과 비교하 해당 방법의 장점(?)이 궁금합니다!
.addFilterAfter(jwtVerificationFilter, JwtAuthenticationFilter.class);
저는 이렇게 순서를 지정하는 방법을 알고 있었는데 @order 어노테이션을 쓰니까 너무 편하고 직관적이어서 좋다는 것을 알게 되었습니다 : ) 한 가지 궁금한 점은 해당 방법이 가장 우선적으로 적용된다는 것인데 이로 인해 의도치 않은 오류?도 발생할 가능성이 있는지가 궁금합니다!
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.
wss-service-support/src/main/java/org/websoso/support/logging/sql/P6SpySqlFormatter.java
Show resolved
Hide resolved
| public class Version { | ||
|
|
||
| @Column(name = "minimum_version", columnDefinition = "varchar(20)", nullable = false) | ||
| private String value; |
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.
p4; 단일 필드 값을 가진 엔티티를 Embeddable로 분리하신 이유가 궁금합니다!
Embeddable은 유사한 여러 개의 필드를 가진 정보를 다룰 때 관심사를 명확하게 분리할 수 있다는 장점이 있다고 생각합니다! (ex : 생년월일을 Birthday로 Embed하고 year, month, day로 Embeddable)
Version을 검증하는 로직의 길이가 길어 MinimumVersion내에 위치하기 조금 부담이라는 생각이 들었지만, 단일 값으로 빼는 것과 비교했들 때의 트레이드오프가 궁금합니다!
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.
- 명확한 도메인을 표현할 수 있습니다.
단순 String 값의 경우, 해당 코드가 무엇을 의미하고 어떻게 사용되는지에 대해 파악하는데 시간이 걸릴 수 있지만, Value Object의 경우 의도와 역할을 바로 파악할 수 있는 장점이 있습니다. - 단일 책임 원칙을 준수합니다.
String 필드를 그냥 사용하게 된다면 Entity의 역할이 너무 비대해지기 때문에, 유지보수 관점에서 약간의 변경으로 다양한 사이드 이펙트가 발생할 가능성이 있습니다. - 타입의 안전성을 강화할 수 있습니다.
String 필드로 파라미터를 받게 된다면 순서가 변동되어도 컴파일 에러가 발생하지 않습니다. 이를 방지하기 위해 빌더 패턴등이 존재하지만, 타입을 지정함으로써 원천적으로 문제를 예방할 수 있습니다.
| @Service | ||
| @RequiredArgsConstructor | ||
| @Transactional(readOnly = true) | ||
| public class GetAppVersionService implements GetAppVersionQuery { |
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.
p2; 인터페이스로 구별하신 이유가 궁금합니다!
다양한 로직을 오버라이딩할 수 있다는 점에서 인터페이스는 매력적인 점이 맞는 것 같습니다! (ex : 고정 할인률, 가변 할인률 등) 하지만 현재와 같이 최소 버전을 반환하는 로직은 비즈니스로직이 잘 바뀌거나 다양하게 나올 수 없다고 생각이 들어서 인터페이스 분리가 오버 프로그래밍이지 않은가? 라는 생각이 들었습니다! 이에 대한 의도하신 바가 궁금합니다 : )
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.
현 상황에서는 오버 엔지니어링이 맞습니다.
버전 관리의 경우, 규모가 작다보니 어떻게 코드 컨벤션을 보여주는데에 한계가 있었습니다.
그래서 오버 엔지니어링 된 부분이 조금씩 존재하는데, 그 중 하나인 부분입니다.
이는 상위 코드인 Application 테스트 코드 작성을 더 편리하게 할 수 있으며, 관심사 분리, CQRS 패턴으로 쉽게 전환 가능 등의 장점이 있습니다.
Related Issue
Key Changes
멀티모듈
Version - Service 로직 관심사 분리
Version - Entity에서 Value Object 분리
Logging
로깅 시스템 정리 문서
Logging - SQL
Logging - API
@Order와OncePerRequestFilter를 사용하여, 1차적으로 요청이 들어오는 API의 URI, Method, Parameter를 핸들링하도록 함MDC를 사용하여 연속되는 로그를 추적할 수 있도록 함@RestControllerAdvice,RequestBodyAdviceAdapter를 사용하여, RequestBody 내용을 가로채도록 함Logging - Logback
To Reviewers
service 모듈과 api 모듈 간의 DTO
support 모듈을 작업하면서 로직이 복잡하지 않기 때문에, api 모듈에서 사용하는 DTO를 그대로 support 모듈에 가져왔습니다.
하지만, API의 입력값 검증, 응답 형태 관리는 Controller의 책임이라고 생각합니다.
따라서, Command와 Result와 같은 api 모듈에서 service 모듈로 접근하고 응답받는 DTO가 있는게 좋다고 생각합니다.
이는 오버 엔지니어링(관리해야하는 포인트가 늘어남)일 수 있기 때문에, 어떻게 생각하는지 의견을 받고 싶습니다.
로그 작성 컨벤션
개발 단계에서 로그는 중요한 지표라고 생각합니다.
따라서, 개발 단계에서 확인이 필요한 부분의 로그 작성이 필요할 것 같다고 생각합니다.
하지만 이러한 로그는 운영 서버에서는 불필요한 로그일 가능성이 크기 때문에, 로그 레벨에 대해서 얘기해볼 시간이 필요할 것 같다고 생각합니다.
이부분은 다음 회의때 얘기해보면 좋을것 같습니다.
로그 저장에서의 문제 발견
PR 적으면서 로그 저장 부분에서 문제 발생할 것을 알아챘습니다.
Docker를 사용해서 Server Image를 띄우기 때문에, 로그 파일이 Server Image를 띄운 컨테이너 내부에 저장됩니다.
따라서, 로그 파일을 확인하기 위해서 컨테이너 내부에 진입해서 해당 파일을 꺼내와야 합니다.
이부분은, 이슈를 생성해서 해결해보도록 하겠습니다.
References