[refactor] 계층 의존 방향 어긋난 곳 수정 및 코드 일관화 - #208
Open
theminjunchoi wants to merge 10 commits into
Open
Conversation
Message·Conversation 이 BlindIndexer 를 알지 않게 한다. applySearchIndex 가 인덱서 대신 완성된 인덱스 값을 받으므로, 엔티티는 "검색 인덱스 값을 보관한다"만 알고 그것이 HMAC 인지 무엇인지는 모른다. domain -> global.crypto import 가 사라진다. 인덱스를 만드는 두 리스너는 JPA 라이프사이클 훅 + HMAC 계산이라 도메인이 아니므로 conversation/search 로 옮긴다. @EntityListeners 는 클래스 참조라 패키지가 달라도 그대로 동작한다.
ConversationSearchPort -> ConversationSearcher, BlindIndexConversationSearch -> BlindIndexConversationSearcher 로 바꾸고 주입 필드명 searchPort 와 "포트 구현" 표현도 함께 정리한다. 헥사고날 용어를 쓰지 않는다.
ErrorCode 는 도메인이 그대로 참조하는데(Nickname·ConversationTitle·FcmToken· ExcludedEmotionTypes·Card·Conversation) HttpStatus 를 필드로 들고 있어서, 닉네임 검증 규칙까지 스프링 웹 타입에 묶여 있었다. ErrorCode 는 에러의 성격만 ErrorKind 로 말하고, HTTP 상태 코드로의 번역은 웹 계층의 httpStatusOf 가 맡는다. when 이 exhaustive 라 ErrorKind 를 추가하면 상태 코드를 정하지 않고는 컴파일되지 않는다. 에러 코드 36개의 상태 코드는 변경 전과 전부 동일하다.
UsageStatsService·QualityStatsService 가 admin/controller/dto 의 Response 를 직접 조립해 반환하고 있었다. 서비스가 웹 응답 포맷을 알면 필드명·Swagger 스키마가 바뀔 때마다 집계 코드가 따라 바뀐다. 두 서비스는 UsageStats·QualityStats(및 DailyActivity·DailyGeneration·EmotionCount)를 반환하고, Response 로의 변환은 컨트롤러가 한다. ConversationUsage 가 이미 쓰던 방식과 같다. EmotionCountResponse 가 card/repository 의 EmotionCountProjection 을 알던 것도 없앤다. 컨트롤러 DTO 가 서비스를 건너뛰고 리포지토리 프로젝션을 직접 참조하고 있었다. TokenUsageResponse 는 tokenlimit 의 TokenUsage 대신 컨트롤러에서 값을 받아 만든다.
admin·monitoring·tokenlimit·notification·card 가 남의 모듈 리포지토리를 그대로 주입받아 쓰고 있었다. 이러면 쿼리 하나를 고칠 때 어느 모듈이 깨지는지 알 수 없고, 주인 모듈이 자기 저장 구조를 바꿀 자유를 잃는다. 읽기는 주인 모듈이 창구를 연다. - conversation/service/ConversationReadService - card/service/CardReadService - monitoring/service/GenerationLogReadService - member 는 이미 있는 MemberService 에 countSignupsBetween 을 더한다 카드 생성이 대화방 상태를 바꾸던 부분은 conversation/service/ConversationCardStateService 로 옮긴다. 어느 상태에서 어느 상태로 갈 수 있는지(SKIPPED 포함 CAS 대상)를 카드 모듈이 알고 있을 이유가 없다. 대화방 소유·종료 판정도 대화 모듈이 한 번만 정의한다. 인터페이스 프로젝션(EmotionCountProjection·ConversationSenderCountProjection)도 모듈 밖으로 내보내지 않는다. 프로젝션은 쿼리 모양에 딸린 것이라 밖으로 새면 쿼리를 못 바꾼다.
LayerDependencyTest 는 이번에 고친 규칙들을 코드로 남긴다. 도메인이 인프라를 모른다, 서비스가 컨트롤러를 모른다, 컨트롤러가 리포지토리를 건너뛰지 않는다, 모듈이 남의 리포지토리를 직접 잡지 않는다, global 이 기능 모듈을 모른다, 에러 코드가 HTTP 를 모른다, Port·Adapter 네이밍을 쓰지 않는다. 새 의존성 없이 소스의 import 만 본다. SearchIndexFillIntegrationTest 는 메시지를 저장하는 세 경로(사용자 메시지, 캐릭터 댓글· 티키타카, 유저 답글 재응답)와 제목 rename 에서 인덱스가 실제로 채워지는지 확인한다. 리스너를 도메인 밖으로 옮긴 뒤에도 그 보장이 그대로여야 한다. 두 테스트 모두 규칙을 일부러 되돌려 실제로 실패하는 것을 확인했다. @EntityListeners 참조는 규칙에서 예외로 둔다. @convert 와 같은 JPA 선언이고 엔티티가 리스너를 호출하는 것이 아니다(근거는 EncryptedStringConverter KDoc). global/exception 이 스프링 웹을 모르도록 GlobalExceptionHandler 와 httpStatusOf 를 global/web 으로 옮긴다.
D 에서 세운 규칙(다른 모듈이 읽어갈 때는 주인 모듈의 XxxReadService 를 거친다)을 member 만 지키지 않고 있었다. MemberService 에 쓰기와 밖에서 읽는 것이 섞여 있어, getById·search·getStats·countSignupsBetween 을 MemberReadService 로 옮긴다. MemberService 는 create·updateNickname·withdraw 만 남고 읽기는 창구에 위임한다. QualityStatsService 가 ConversationProperties.pendingGenerationTimeout 을 직접 읽어 '막힌 PENDING' 기준 시각을 계산하고 있었다. 그 기준은 대화 모듈이 정하는 값이라, 부르는 쪽이 계산하면 정리 스케줄러가 쓰는 기준과 조용히 어긋날 수 있다. countCommentsStuckBefore(before) 를 countStuckPendingComments() 로 바꿔 대화 모듈이 자기 기준으로 판단하게 한다.
theminjunchoi
changed the title
리팩터링으로 참조가 사라졌는데 남아 있던 것들이다. ktlint 1.8 의 standard 룰셋에는 no-unused-imports 가 없어서 빌드가 잡아주지 못한다.
|
Warning Review limit reachedNext included review available in 14 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (61)
Warning Your free Security trial is over. An organization admin can activate billing to continue. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
XxxReadService 는 기술 축이라, 새 기능마다 "이건 읽기인가"를 물어야 하는데 유스케이스
서비스에도 조회가 있어서 답이 갈렸다. 실제로 member 만 읽기·쓰기 축으로, 나머지는
모듈 안·밖 축으로 갈라져 있었다.
목적으로 가르면 이름이 곧 배치 규칙이 된다. "무엇을 위한 기능인가"만 물으면 된다.
- ConversationReadService(15) 를 셋으로 나눈다
- ConversationStatsService: 얼마나 있는지 센다(백오피스 집계·모니터링 게이지)
- ConversationCardGenerationService: 카드 생성이 대화방에 요구하는 것
(기존 ConversationCardStateService 를 개명하고 카드 생성용 조회 셋을 흡수)
- ConversationService: 대화방 삭제(카드 삭제에 딸린 것 포함)와 미종료 방 주인 조회
- CardReadService -> CardStatsService
- GenerationLogReadService -> GenerationLogStatsService
- MemberReadService 는 없앤다. getById·search 는 회원 유스케이스라 MemberService 로
돌리고, 집계 둘만 MemberStatsService 로 뺀다
getOwnedConversation 이 ConversationService 와 조회 창구 양쪽에 중복돼 있었다.
ConversationService 것을 공개해 하나로 만들고 카드 쪽은 위임한다.
LayerDependencyTest 에 이름 규칙을 더한다. Read·WriteService 로 되돌아가면 실패한다.
되돌려서 실제로 잡히는 것을 확인했다.
1. LayerDependencyTest 가 소스를 하나도 못 찾으면 실패시킨다. MAIN_SOURCE 가 상대경로라 작업 디렉터리가 프로젝트 루트가 아니면 빈 리스트가 되는데, 그때 규칙 여덟 개가 전부 "위반 0건"으로 통과해 버린다. 경로를 깨뜨려 확인했고, 고친 뒤에는 여덟 개가 모두 실패한다. 2. ErrorKind 와 ErrorCode 의 KDoc 링크가 옮기기 전 위치(global.response.httpStatusOf)를 가리키고 있었다. 실제 위치는 global.web 이다. KDoc 링크는 깨져도 컴파일이 통과한다. 3. 쓰이지 않는 import 세 개를 지운다. getOwnedConversation 을 위임으로 바꾸고 deleteForCardRemoval 을 옮기면서 생긴 것들이다.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🔗 연관 이슈
📌 개요
이슈가 지목한
Message·Conversation의BlindIndexer의존을 걷어내고, 같은 종류로 어긋나 있던 계층·모듈 의존 방향을 함께 바로잡았습니다. 다시 어긋나지 않도록 규칙을 테스트로 고정했습니다.🔧 주요 변경사항
검색 인덱스를 도메인 밖으로 (#191 본문)
applySearchIndex가BlindIndexer대신 완성된 인덱스 값을 받습니다. 엔티티는 "검색 인덱스 값을 보관한다"만 알고 그것이 HMAC 인지 모릅니다.domain -> global.cryptoimport 가 사라집니다MessageSearchIndexListener·ConversationSearchIndexListener)를conversation/domain에서conversation/search로 옮겼습니다헥사고날 용어 제거
ConversationSearchPort->ConversationSearcher,BlindIndexConversationSearch->BlindIndexConversationSearcher. 주입 필드명searchPort와 "포트 구현" 표현도 정리했습니다ErrorCode에서 HTTP 분리ErrorCode가HttpStatus를 필드로 들고 있어서, 이걸 참조하는 도메인(Nickname·ConversationTitle·FcmToken·ExcludedEmotionTypes·Card·Conversation)이 전부 스프링 웹 타입에 묶여 있었습니다ErrorCode는 성격만ErrorKind로 말하고, HTTP 상태 코드로의 번역은global/web/httpStatusOf가 맡습니다.when이 exhaustive 라ErrorKind를 추가하면 상태 코드를 정하지 않고는 컴파일되지 않습니다global/exception이 스프링 웹을 모르도록GlobalExceptionHandler를global/web으로 옮겼습니다서비스 -> 컨트롤러 역전 제거
UsageStatsService·QualityStatsService가admin/controller/dto의 Response 를 직접 조립해 반환하고 있었습니다. 이제UsageStats·QualityStats를 반환하고 Response 변환은 컨트롤러가 합니다(ConversationUsage가 이미 쓰던 방식)EmotionCountResponse가 서비스를 건너뛰고card/repository/EmotionCountProjection을 참조하던 것도 없앴습니다모듈 경계
ConversationCardGenerationService로 옮겼습니다. 어느 상태에서 어느 상태로 갈 수 있는지(SKIPPED 포함 CAS 대상)를 카드 모듈이 알 이유가 없습니다QualityStatsService가ConversationProperties.pendingGenerationTimeout을 읽어 '막힌 PENDING' 기준을 계산하던 것을countStuckPendingComments()로 바꿔 대화 모듈이 자기 기준으로 판단하게 했습니다서비스를 목적으로 가릅니다
처음에는 모듈 밖에서 읽어가는 창구를
XxxReadService로 뒀는데, 이건 기술 축이라 새 기능마다 "이건 읽기인가"를 물어야 하고 유스케이스 서비스에도 조회가 있어서 답이 갈립니다. 실제로 member 만 읽기·쓰기 축으로, 나머지는 모듈 안·밖 축으로 갈라져 있었습니다목적으로 가르면 이름이 곧 배치 규칙이 됩니다. "무엇을 위한 기능인가"만 물으면 됩니다
ConversationServiceConversationStatsServiceConversationCardGenerationServiceCardService/CardStatsServiceMemberService/MemberStatsServiceGenerationLogRecorder/GenerationLogStatsServicegetOwnedConversation이ConversationService와 조회 창구 양쪽에 중복돼 있던 것도 하나로 합쳤습니다테스트
LayerDependencyTest: 위 규칙 8개를 코드로 고정합니다(도메인이 인프라를 모른다 / 서비스가 컨트롤러를 모른다 / 컨트롤러가 리포지토리를 건너뛰지 않는다 / 모듈이 남의 리포지토리를 직접 잡지 않는다 / global 이 기능 모듈을 모른다 / 에러 코드가 HTTP 를 모른다 / 서비스를 읽기·쓰기로 가르지 않는다 / Port·Adapter 네이밍을 쓰지 않는다). 새 의존성 없이 소스의 import 만 봅니다SearchIndexFillIntegrationTest: 메시지를 저장하는 세 경로와 제목 rename 에서 인덱스가 실제로 채워지는지 확인합니다🌐 API · DB 영향
💬 리뷰 포인트
@Convert(converter = EncryptedStringConverter::class)는 일부러 남겼습니다.@Entity·@Column과 같은 JPA 매핑 선언이고, 이 레포는 엔티티를 JPA 매핑 모델로 쓰기로 이미 정했습니다.@Column은 받아들이면서@Convert만 위반으로 보는 기준은 성립하지 않는다고 봤습니다. 걷어낸 것은 행위 의존뿐입니다. 근거를EncryptedStringConverterKDoc 에 적었고, 같은 이유로@EntityListeners참조도LayerDependencyTest에서 예외로 뒀습니다~ReadService·~WriteService네이밍은 테스트로 막아뒀습니다이번 PR 에서 손대지 않은 것
훑어보다 찾았지만 판단이 갈려 그대로 둔 것들입니다. 필요하면 별도 이슈로 올리겠습니다.
admin/controller가member/controller/dto/MemberResponse를 재사용합니다. 레포에서 유일한 controller -> controller 크로스 모듈 참조인데, 고치면 DTO 가 중복됩니다llm모듈만controller/service/repository/domain규약을 쓰지 않습니다(generation·parsing·prompt·preview·selection·settings). 그래서LayerDependencyTest가 llm 모듈에는 사실상 적용되지 않습니다CardRepository의 JPQL 6개가Conversation을 조인해 삭제된 방을 거릅니다. 방향 위반은 아니지만 카드 쿼리가 대화 테이블 구조에 묶여 있습니다. 없애려면 데이터 모델 변경이 필요합니다admin/service가llm/config/GeminiPricing을 참조합니다.@ConfigurationProperties지만costUsd(...)계산 메서드를 가진 요금표라, 설정값 노출보다는 llm 모듈이 제공하는 계산 기능을 부르는 쪽에 가깝다고 봤습니다Summary by CodeRabbit
새로운 기능
버그 수정
테스트