Architecture Review

crm-backend-matt · Round 3 · 2026-08-13
module deep module interface surface seam

1. Extract MultipartUploader from FileApiImpl

Strong in-process
crm-file/…/service/impl/FileApiImpl.java (508 lines)
crm-file/…/api/FileApi.java (119 lines, 9 methods)
crm-file/…/service/impl/FileApiImplTest.java (763 lines)

Before

FileApi · 9 methods
FileApiImpl · 508 lines
Simple File I/O · ~130 lines
upload · download · getInfo · delete · getPreviewUrl
Multipart Upload · ~210 lines
init · uploadChunk · completeMultipart + 6 helpers
Redis session · chunk I/O · compose · cleanup
Thumbnail Retrieval · ~80 lines
getThumbnail + 4 helpers · sync fallback · poll

3 lifecycles mixed in one module. Tests need 8 mocks for any path.

After

FileApi · 6 methods (delegates 3)
FileApiImpl · ~310 lines
Simple File I/O
Thumbnail Retrieval
seam: 3 methods
MultipartUploader · 3 methods
~200 lines
session lifecycle · chunk validation
compose · cleanup

Multipart protocol concentrated in one deep module.

Problem

FileApiImpl mixes three distinct lifecycles. The multipart upload protocol — Redis session management, chunk I/O, composeObject, cleanup — is self-contained but shares the same class as simple file put/get and thumbnail retrieval. FileApiImplTest is 763 lines because multipart tests, thumbnail tests, and simple upload tests all share the same 8-mock setup.

Solution

Extract a MultipartUploader module: 3-method interface (init, uploadChunk, completeMultipart) backed by ~200 lines of implementation. FileApiImpl delegates the 3 multipart methods; the multipart helpers (requireSession, listUploadedChunks, chunkKey, cleanupChunks, etc.) move behind the seam.

Wins
  • • Locality: multipart protocol lives in one module, not interleaved with file I/O
  • • Interface shrinks: FileApi 9 → 6 visible methods, 3 delegated
  • • Tests isolate: multipart tests mock only MinIO + Redis, not 8 deps
  • • Leverage: 3-method interface controls 210 lines of protocol

2. Extract ThumbnailResolver from FileApiImpl

Worth exploring in-process
Previously assessed as marginal in isolation (round 1). Stronger when combined with Candidate 1 — together they leave FileApiImpl as a clean ~230-line file gateway.
crm-file/…/service/impl/FileApiImpl.java (lines 198–294, ~80 lines)
crm-file/…/service/impl/FileApiImplTest.java (thumbnail tests)

Before (after Candidate 1)

FileApi · 6 methods
FileApiImpl · ~310 lines
Simple File I/O · ~130 lines
Thumbnail Retrieval · ~80 lines
handlePending · readThumbnail · placeholderOf · toThumbnailAfterGeneration

Thumbnail retrieval protocol still mixed with file I/O.

After

FileApi · 5 methods (delegates 1)
FileApiImpl · ~230 lines
Simple File I/O only
seam: getThumbnail(fileId)
ThumbnailResolver · 1 method
~80 lines
status check → sync fallback
→ poll → placeholder

File gateway clean; thumbnail protocol isolated.

Problem

The thumbnail retrieval protocol (status → sync fallback → polling → placeholder) is a distinct concern from file put/get. It depends on ThumbnailGenerationTask, ThumbnailPlaceholderService, and MinioClient for reading thumbnails — different dependencies from simple file I/O. In isolation this is 80 lines; after Candidate 1, it's the remaining non-file-I/O concern in FileApiImpl.

Solution

Extract a ThumbnailResolver module: 1-method interface (getThumbnail) backed by ~80 lines. The 4 private helpers (handlePending, readThumbnail, placeholderOf, toThumbnailAfterGeneration) move behind the seam. FileApi delegates getThumbnail to it.

Wins
  • • Locality: thumbnail resolution protocol in one module
  • • Tests isolate: thumbnail tests mock 3 deps, not 8
  • • FileApiImpl becomes a clean gateway — only file I/O remains
  • • Deletion test passes: protocol concentrates, not just moves

3. Fix N+1 region query in fillDisplayFields

Speculative in-process
crm-lead/…/service/impl/LeadServiceImpl.java (lines 517–565, fillDisplayFields)
crm-rule/…/service/ISysRegionService.java (getByCode — called in loop)

Before

for (String code : regionCodes) {
    var region = sysRegionService
        .getByCode(code);   // N+1
    regionNameMap.put(code,
        region.getName());
}
10 leads × 2 codes = 20 individual queries

fillDisplayFields: 50 lines of read-model assembly inside the lead service. Follow counts use batch queries; region names do not.

After

Map<String,String> names =
    sysRegionService
      .batchGetNames(codes);  // 1 query
1 batch query replaces N loop calls

Fix: add batchGetByCodes to ISysRegionService. Optional: extract fillDisplayFields into a LeadDisplayEnricher module (~50 lines, 1-method interface).

Problem

fillDisplayFields resolves region names by calling sysRegionService.getByCode(code) in a loop — an N+1 query pattern. The method also does batch queries for follow counts and followed status, making the mixed pattern harder to spot. This is a performance bug; the optional extraction is an architecture improvement.

Solution

Fix: add batchGetByCodes(Set<String>) to ISysRegionService, replace the loop with one call. Optional: extract fillDisplayFields into a LeadDisplayEnricher module (1-method interface, ~50 lines) to isolate read-model assembly from the lead write model.

Wins
  • • Bug fix: N+1 → batch query (concrete, regardless of extraction)
  • • Locality (optional): read-model assembly isolated from write model
  • • Leverage (optional): 1-method interface, ~50 lines behind seam
Top recommendation

Candidate 1 — Extract MultipartUploader

The multipart upload protocol is the largest self-contained lifecycle inside FileApiImpl (~210 lines). It has its own state management (Redis sessions), its own MinIO interaction pattern (chunks + composeObject), and its own cleanup — all distinct from simple file I/O. Extracting it creates a deep module (3-method interface, ~200-line implementation) and shrinks FileApiImpl by 40%. The test file (763 lines) can split cleanly along the seam. ↑ jump to card