Architecture Review

crm-backend-matt · 2026-08-08
module shallow deep ⊡ dashed = seam — red = leakage

Three deepening opportunities across the crm-file, crm-dict, and crm-auth modules. Vocabulary from /codebase-design; domain terms from CONTEXT.md.

1. Thumbnail lifecycle split across two modules

Strong in-process

Files

  • crm-file/service/api/FileApi.java — 11 methods, includes getThumbnail()
  • crm-file/service/impl/FileApiImpl.java — 551 lines; ~100 lines are thumbnail read/fallback
  • crm-file/task/ThumbnailGenerationTask.java — generation + lock, but no retrieval
  • crm-file/service/ThumbnailPlaceholderService.java, ThumbnailRenderer.java
Before
FileApiImpl (551 lines)
upload · download · preview · multipart · getThumbnail · handlePending · readThumbnail · placeholderOf
↑ thumbnail read path leaks here
⊡⊡ seam (not a real one) ⊡⊡
ThumbnailGenerationTask
assignInitialStatus · generate · tryGenerateSync · generateSync · findRenderer
generation only — half the lifecycle

No locality: thumbnail bugs span two modules

After
FileApi (8 methods)
upload · download · preview · multipart
file ops only — interface narrows
⊡⊡ seam ⊡⊡
ThumbnailService (deep)
interface: getThumbnail · assignInitialStatus · generate
implementation: handlePending · poll · readThumbnail · generateSync · findRenderer · placeholderOf

Locality: full lifecycle in one module

Problem: The thumbnail capability has no single module — generation lives in ThumbnailGenerationTask, retrieval and sync-fallback (handlePending, readThumbnail, placeholderOf) lives in FileApiImpl. Understanding thumbnail behavior requires bouncing between two modules. FileApi's interface is 11 methods wide because it carries thumbnail concerns that belong to a separate concern.

Solution: Extract a ThumbnailService module that owns the entire thumbnail lifecycle — generation, retrieval, sync-fallback, placeholder. FileApi narrows to file operations only. FileApiImpl.upload() and completeMultipart() call ThumbnailService.assignInitialStatus() + generate() instead of reaching into ThumbnailGenerationTask directly.

Wins

  • ▸ locality: thumbnail bugs concentrate in one module
  • ▸ leverage: one ThumbnailService interface, FileApi + FileController both call it
  • ▸ interface shrinks: FileApi drops getThumbnail
  • ▸ tests hit one interface for the full lifecycle
  • ▸ FileApiImpl shrinks ~100 lines

2. Permission seeding: shadow entities + logic duplication

Strong ports & adapters

Files

  • crm-dict/config/DictPermissionInitializer.java — 157 lines, seeds dict:* permission points
  • crm-dict/domain/entity/SysMenuSeed.java, SysRoleSeed.java, SysRoleMenuSeed.java — shadow entities mirroring auth tables
  • crm-auth/config/DataInitializer.java — 165 lines, seeds crm:role:* permission points (same pattern)
Before
crm-dict / DictPermissionInitializer
findMenuByName · insertButtonIfAbsent · bindIfAbsent
SysMenuSeed, SysRoleSeed, SysRoleMenuSeed
@TableName("sys_menu") — schema copy
↻
crm-auth / DataInitializer
findMenu · insertButtonIfAbsent · bindIfAbsent
SysMenu, SysRole, SysRoleMenu
@TableName("sys_menu") — real schema

Schema duplicated; logic duplicated; leak across seam

After
crm-base / PermissionSeeder (interface)
registerPoint(perms, apiUrl, name, parentPath)
⊡⊡ seam ⊡⊡
crm-dict initializer
thin caller
crm-auth initializer
thin caller
PermissionSeederImpl (crm-auth)
owns SysMenu, SysRole, SysRoleMenu schema

Schema knowledge in one module; one interface for seeding

Problem: Two initializers duplicate the same permission-seeding logic (findMenuByName, insertButtonIfAbsent, bindIfAbsent) with nearly identical structure. crm-dict creates shadow entities (SysMenuSeed, SysRoleSeed, SysRoleMenuSeed) that mirror crm-auth's physical schema. ADR-0015 says "the permission code string is the only contract with the auth system," but the shadow entities leak the entire sys_menu schema (column names, menu types, status values) into crm-dict. Schema changes in crm-auth silently break crm-dict's seeding.

Solution: Extract a PermissionSeeder seam — an interface in crm-base that declares registerPoint(perms, apiUrl, name, parentMenuPath) without exposing sys_menu's schema. crm-auth provides the implementation (owns SysMenu, SysRole, SysRoleMenu). Both initializers become thin callers of the same seam. Three shadow entities deleted.

ADR-0015 alignment: This fulfills the ADR, not contradicts it. The ADR says "crm-dict depends only on crm-base" and "the permission code string is the only contract." A PermissionSeeder interface in crm-base, implemented by crm-auth, preserves both constraints. The current shadow-entity approach actually violates the ADR's intent by leaking physical schema — the contract should be code strings, not @TableName("sys_menu").

Wins

  • ▸ locality: sys_menu schema knowledge concentrates in one module
  • ▸ leverage: one PermissionSeeder interface, N seeders call it
  • ▸ delete 3 shadow entities + ~100 lines duplicated logic
  • ▸ future modules (crm-rule, crm-audit) seed permissions through the same seam
  • ▸ schema changes no longer break cross-module seeding

3. DingTalkAuthClient: test-by-subclass instead of adapter

Worth exploring mock

Files

  • crm-auth/service/client/DingTalkAuthClient.java — 314 lines; 4 protected HTTP stubs (~120 lines)
  • crm-auth/service/client/ThirdPartyAuthClient.java — 38 lines interface (getType, getUserInfo, isOrgMember)
  • crm-auth/service/client/ThirdPartyAuthClientFactory.java — 37 lines
Before — mixed mass
interface
3 methods
→
implementation
HTTP stubs
token cache
error parse
org verify
corp token
314 lines

HTTP plumbing tangled with business logic; tests subclass to stub

After — split by concern
client interface
3 methods
→
DingTalkAuthClient
token cache
error parse
org verify
~150 lines
DingTalkHttpClient
HTTP plumbing
~120 lines

Business logic testable through adapter; two adapters = real seam

Problem: Four protected HTTP methods (requestTokenApi, requestUserInfoApi, requestCorpTokenApi, requestGetByUnionIdApi) exist purely as test hooks — tests subclass and override them to inject canned responses. The business logic (token caching, error-code parsing, org membership verification) is tangled with HTTP plumbing. Tests exercise the protected stubs, not the real business logic through the ThirdPartyAuthClient interface. Bugs in error parsing or token refresh hide in private methods that call the stubs.

Solution: Extract a DingTalkHttpClient adapter (interface + real implementation) that owns all HTTP calls. DingTalkAuthClient injects the adapter and becomes pure business logic. Tests inject a stub DingTalkHttpClient through the constructor, testing business logic through the ThirdPartyAuthClient interface.

Wins

  • ▸ locality: HTTP plumbing concentrates in the adapter
  • ▸ tests inject stub adapter, test through interface (no inheritance)
  • ▸ delete 4 protected methods + boilerplate
  • ▸ two adapters (real + test stub) justify the seam

Top recommendation

#2 — Permission seeding seam

Start here. It's the highest-leverage deepening: one new interface in crm-base deletes 3 shadow entities, eliminates ~100 lines of duplicated seeding logic, and future-proofs every new module that needs to seed permission points. It also aligns the codebase with ADR-0015's stated contract ("the permission code string is the only contract with the auth system") — the current shadow-entity approach leaks physical schema across the seam, which the ADR explicitly intended to prevent. The refactor is low-risk: the initializers already run at startup with no runtime callers, so the interface change has no downstream impact.

#1 Thumbnail — highest friction but larger blast radius (FileApi is the most-used interface) #3 DingTalk — testability gain, but only one client exists today