You can not select more than 25 topics
Topics must start with a letter or number, can include dashes ('-') and can be up to 35 characters long.
299 lines
17 KiB
299 lines
17 KiB
3 weeks ago
|
<!doctype html>
|
||
|
|
<html lang="zh">
|
||
|
|
<head>
|
||
|
|
<meta charset="utf-8" />
|
||
|
|
<title>Architecture Review — crm-backend-matt (Round 3)</title>
|
||
|
|
<script src="https://cdn.tailwindcss.com"></script>
|
||
|
|
<script type="module">
|
||
|
|
import mermaid from "https://cdn.jsdelivr.net/npm/mermaid@11/dist/mermaid.esm.min.mjs";
|
||
|
|
mermaid.initialize({ startOnLoad: true, theme: "neutral", securityLevel: "loose" });
|
||
|
|
</script>
|
||
|
|
<style>
|
||
|
|
.seam { stroke-dasharray: 4 4; }
|
||
|
|
.leak { stroke: #dc2626; }
|
||
|
|
.deep { background: linear-gradient(135deg, #0f172a, #1e293b); }
|
||
|
|
.module-box { border: 2px solid #475569; border-radius: 6px; }
|
||
|
|
.deep-box { border: 3px solid #0f172a; border-radius: 6px; background: linear-gradient(135deg, #0f172a, #1e293b); color: #e2e8f0; }
|
||
|
|
.interface-bar { background: #cbd5e1; border-radius: 3px 3px 0 0; }
|
||
|
|
.lifecycle-a { background: #dbeafe; }
|
||
|
|
.lifecycle-b { background: #fef3c7; }
|
||
|
|
.lifecycle-c { background: #dcfce7; }
|
||
|
|
</style>
|
||
|
|
</head>
|
||
|
|
<body class="bg-stone-50 text-slate-900 font-sans">
|
||
|
|
<main class="max-w-5xl mx-auto px-6 py-12 space-y-12">
|
||
|
|
|
||
|
|
<!-- ========== Header ========== -->
|
||
|
|
<header class="space-y-3">
|
||
|
|
<div class="flex items-baseline gap-4">
|
||
|
|
<h1 class="text-3xl font-serif text-slate-900">Architecture Review</h1>
|
||
|
|
<span class="text-sm text-slate-500">crm-backend-matt · Round 3 · 2026-08-13</span>
|
||
|
|
</div>
|
||
|
|
<div class="flex flex-wrap gap-4 text-xs text-slate-500">
|
||
|
|
<span class="flex items-center gap-1"><span class="inline-block w-4 h-3 module-box bg-white"></span> module</span>
|
||
|
|
<span class="flex items-center gap-1"><span class="inline-block w-4 h-3 deep-box"></span> deep module</span>
|
||
|
|
<span class="flex items-center gap-1"><span class="inline-block w-4 h-1 bg-slate-400 rounded"></span> interface surface</span>
|
||
|
|
<span class="flex items-center gap-1"><span class="inline-block w-4 h-3 rounded border-2 border-dashed border-slate-400"></span> seam</span>
|
||
|
|
</div>
|
||
|
|
</header>
|
||
|
|
|
||
|
|
<!-- ========== Candidate 1 ========== -->
|
||
|
|
<article id="c1" class="space-y-4">
|
||
|
|
<div class="flex items-baseline gap-3 flex-wrap">
|
||
|
|
<h2 class="text-2xl font-serif">1. Extract MultipartUploader from FileApiImpl</h2>
|
||
|
|
<span class="text-xs font-bold uppercase tracking-wider px-2 py-0.5 rounded bg-emerald-100 text-emerald-700">Strong</span>
|
||
|
|
<span class="text-xs text-slate-400">in-process</span>
|
||
|
|
</div>
|
||
|
|
|
||
|
|
<!-- Files -->
|
||
|
|
<div class="text-sm font-mono text-slate-600">
|
||
|
|
crm-file/…/service/impl/FileApiImpl.java (508 lines)<br>
|
||
|
|
crm-file/…/api/FileApi.java (119 lines, 9 methods)<br>
|
||
|
|
crm-file/…/service/impl/FileApiImplTest.java (763 lines)
|
||
|
|
</div>
|
||
|
|
|
||
|
|
<!-- Before / After -->
|
||
|
|
<div class="grid grid-cols-2 gap-4">
|
||
|
|
<!-- Before -->
|
||
|
|
<div class="space-y-2">
|
||
|
|
<h3 class="text-xs uppercase tracking-wider text-slate-400">Before</h3>
|
||
|
|
<div class="module-box bg-white p-3 space-y-2" style="min-height: 280px;">
|
||
|
|
<div class="interface-bar h-5 flex items-center justify-center text-xs text-slate-600">FileApi · 9 methods</div>
|
||
|
|
<div class="text-xs font-mono text-slate-700 text-center">FileApiImpl · 508 lines</div>
|
||
|
|
<div class="space-y-1.5">
|
||
|
|
<div class="lifecycle-a rounded px-2 py-2 text-xs">
|
||
|
|
<div class="font-semibold text-blue-800">Simple File I/O · ~130 lines</div>
|
||
|
|
<div class="text-slate-500">upload · download · getInfo · delete · getPreviewUrl</div>
|
||
|
|
</div>
|
||
|
|
<div class="lifecycle-b rounded px-2 py-2 text-xs">
|
||
|
|
<div class="font-semibold text-amber-800">Multipart Upload · ~210 lines</div>
|
||
|
|
<div class="text-slate-500">init · uploadChunk · completeMultipart + 6 helpers</div>
|
||
|
|
<div class="text-slate-500">Redis session · chunk I/O · compose · cleanup</div>
|
||
|
|
</div>
|
||
|
|
<div class="lifecycle-c rounded px-2 py-2 text-xs">
|
||
|
|
<div class="font-semibold text-green-800">Thumbnail Retrieval · ~80 lines</div>
|
||
|
|
<div class="text-slate-500">getThumbnail + 4 helpers · sync fallback · poll</div>
|
||
|
|
</div>
|
||
|
|
</div>
|
||
|
|
</div>
|
||
|
|
<p class="text-xs text-slate-400">3 lifecycles mixed in one module. Tests need 8 mocks for any path.</p>
|
||
|
|
</div>
|
||
|
|
<!-- After -->
|
||
|
|
<div class="space-y-2">
|
||
|
|
<h3 class="text-xs uppercase tracking-wider text-slate-400">After</h3>
|
||
|
|
<div class="space-y-2" style="min-height: 280px;">
|
||
|
|
<div class="module-box bg-white p-3 space-y-1">
|
||
|
|
<div class="interface-bar h-5 flex items-center justify-center text-xs text-slate-600">FileApi · 6 methods (delegates 3)</div>
|
||
|
|
<div class="text-xs font-mono text-slate-700 text-center">FileApiImpl · ~310 lines</div>
|
||
|
|
<div class="space-y-1">
|
||
|
|
<div class="lifecycle-a rounded px-2 py-1 text-xs">
|
||
|
|
<span class="font-semibold text-blue-800">Simple File I/O</span>
|
||
|
|
</div>
|
||
|
|
<div class="lifecycle-c rounded px-2 py-1 text-xs">
|
||
|
|
<span class="font-semibold text-green-800">Thumbnail Retrieval</span>
|
||
|
|
</div>
|
||
|
|
</div>
|
||
|
|
</div>
|
||
|
|
<div class="border-2 border-dashed border-slate-300 rounded p-1 text-center text-xs text-slate-400">seam: 3 methods</div>
|
||
|
|
<div class="deep-box p-3 space-y-1">
|
||
|
|
<div class="bg-slate-700 h-5 flex items-center justify-center text-xs text-slate-300 rounded">MultipartUploader · 3 methods</div>
|
||
|
|
<div class="text-xs font-mono text-slate-300 text-center">~200 lines</div>
|
||
|
|
<div class="text-xs text-slate-400">session lifecycle · chunk validation</div>
|
||
|
|
<div class="text-xs text-slate-400">compose · cleanup</div>
|
||
|
|
</div>
|
||
|
|
</div>
|
||
|
|
<p class="text-xs text-slate-400">Multipart protocol concentrated in one deep module.</p>
|
||
|
|
</div>
|
||
|
|
</div>
|
||
|
|
|
||
|
|
<!-- Problem / Solution -->
|
||
|
|
<div class="grid grid-cols-2 gap-4 text-sm">
|
||
|
|
<div>
|
||
|
|
<div class="text-xs uppercase tracking-wider text-red-500 mb-1">Problem</div>
|
||
|
|
<p>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.</p>
|
||
|
|
</div>
|
||
|
|
<div>
|
||
|
|
<div class="text-xs uppercase tracking-wider text-emerald-600 mb-1">Solution</div>
|
||
|
|
<p>Extract a <code class="text-sm">MultipartUploader</code> 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.</p>
|
||
|
|
</div>
|
||
|
|
</div>
|
||
|
|
|
||
|
|
<!-- Wins -->
|
||
|
|
<div>
|
||
|
|
<div class="text-xs uppercase tracking-wider text-slate-400 mb-1">Wins</div>
|
||
|
|
<ul class="text-sm space-y-0.5 text-slate-700">
|
||
|
|
<li>• <b>Locality</b>: multipart protocol lives in one module, not interleaved with file I/O</li>
|
||
|
|
<li>• <b>Interface shrinks</b>: FileApi 9 → 6 visible methods, 3 delegated</li>
|
||
|
|
<li>• <b>Tests isolate</b>: multipart tests mock only MinIO + Redis, not 8 deps</li>
|
||
|
|
<li>• <b>Leverage</b>: 3-method interface controls 210 lines of protocol</li>
|
||
|
|
</ul>
|
||
|
|
</div>
|
||
|
|
</article>
|
||
|
|
|
||
|
|
<!-- ========== Candidate 2 ========== -->
|
||
|
|
<article id="c2" class="space-y-4">
|
||
|
|
<div class="flex items-baseline gap-3 flex-wrap">
|
||
|
|
<h2 class="text-2xl font-serif">2. Extract ThumbnailResolver from FileApiImpl</h2>
|
||
|
|
<span class="text-xs font-bold uppercase tracking-wider px-2 py-0.5 rounded bg-amber-100 text-amber-700">Worth exploring</span>
|
||
|
|
<span class="text-xs text-slate-400">in-process</span>
|
||
|
|
</div>
|
||
|
|
|
||
|
|
<!-- Note about previous assessment -->
|
||
|
|
<div class="text-xs bg-amber-50 border border-amber-200 rounded px-3 py-2 text-amber-800">
|
||
|
|
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.
|
||
|
|
</div>
|
||
|
|
|
||
|
|
<!-- Files -->
|
||
|
|
<div class="text-sm font-mono text-slate-600">
|
||
|
|
crm-file/…/service/impl/FileApiImpl.java (lines 198–294, ~80 lines)<br>
|
||
|
|
crm-file/…/service/impl/FileApiImplTest.java (thumbnail tests)
|
||
|
|
</div>
|
||
|
|
|
||
|
|
<!-- Before / After (mass diagram) -->
|
||
|
|
<div class="grid grid-cols-2 gap-4">
|
||
|
|
<!-- Before -->
|
||
|
|
<div class="space-y-2">
|
||
|
|
<h3 class="text-xs uppercase tracking-wider text-slate-400">Before (after Candidate 1)</h3>
|
||
|
|
<div class="module-box bg-white p-3 space-y-2" style="min-height: 200px;">
|
||
|
|
<div class="interface-bar h-5 flex items-center justify-center text-xs text-slate-600">FileApi · 6 methods</div>
|
||
|
|
<div class="text-xs font-mono text-slate-700 text-center">FileApiImpl · ~310 lines</div>
|
||
|
|
<div class="space-y-1">
|
||
|
|
<div class="lifecycle-a rounded px-2 py-2 text-xs">
|
||
|
|
<div class="font-semibold text-blue-800">Simple File I/O · ~130 lines</div>
|
||
|
|
</div>
|
||
|
|
<div class="lifecycle-c rounded px-2 py-2 text-xs">
|
||
|
|
<div class="font-semibold text-green-800">Thumbnail Retrieval · ~80 lines</div>
|
||
|
|
<div class="text-slate-500">handlePending · readThumbnail · placeholderOf · toThumbnailAfterGeneration</div>
|
||
|
|
</div>
|
||
|
|
</div>
|
||
|
|
</div>
|
||
|
|
<p class="text-xs text-slate-400">Thumbnail retrieval protocol still mixed with file I/O.</p>
|
||
|
|
</div>
|
||
|
|
<!-- After -->
|
||
|
|
<div class="space-y-2">
|
||
|
|
<h3 class="text-xs uppercase tracking-wider text-slate-400">After</h3>
|
||
|
|
<div class="space-y-2" style="min-height: 200px;">
|
||
|
|
<div class="module-box bg-white p-3 space-y-1">
|
||
|
|
<div class="interface-bar h-5 flex items-center justify-center text-xs text-slate-600">FileApi · 5 methods (delegates 1)</div>
|
||
|
|
<div class="text-xs font-mono text-slate-700 text-center">FileApiImpl · ~230 lines</div>
|
||
|
|
<div class="lifecycle-a rounded px-2 py-1 text-xs">
|
||
|
|
<span class="font-semibold text-blue-800">Simple File I/O only</span>
|
||
|
|
</div>
|
||
|
|
</div>
|
||
|
|
<div class="border-2 border-dashed border-slate-300 rounded p-1 text-center text-xs text-slate-400">seam: getThumbnail(fileId)</div>
|
||
|
|
<div class="deep-box p-3 space-y-1">
|
||
|
|
<div class="bg-slate-700 h-5 flex items-center justify-center text-xs text-slate-300 rounded">ThumbnailResolver · 1 method</div>
|
||
|
|
<div class="text-xs font-mono text-slate-300 text-center">~80 lines</div>
|
||
|
|
<div class="text-xs text-slate-400">status check → sync fallback</div>
|
||
|
|
<div class="text-xs text-slate-400">→ poll → placeholder</div>
|
||
|
|
</div>
|
||
|
|
</div>
|
||
|
|
<p class="text-xs text-slate-400">File gateway clean; thumbnail protocol isolated.</p>
|
||
|
|
</div>
|
||
|
|
</div>
|
||
|
|
|
||
|
|
<!-- Problem / Solution -->
|
||
|
|
<div class="grid grid-cols-2 gap-4 text-sm">
|
||
|
|
<div>
|
||
|
|
<div class="text-xs uppercase tracking-wider text-red-500 mb-1">Problem</div>
|
||
|
|
<p>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.</p>
|
||
|
|
</div>
|
||
|
|
<div>
|
||
|
|
<div class="text-xs uppercase tracking-wider text-emerald-600 mb-1">Solution</div>
|
||
|
|
<p>Extract a <code class="text-sm">ThumbnailResolver</code> 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.</p>
|
||
|
|
</div>
|
||
|
|
</div>
|
||
|
|
|
||
|
|
<!-- Wins -->
|
||
|
|
<div>
|
||
|
|
<div class="text-xs uppercase tracking-wider text-slate-400 mb-1">Wins</div>
|
||
|
|
<ul class="text-sm space-y-0.5 text-slate-700">
|
||
|
|
<li>• <b>Locality</b>: thumbnail resolution protocol in one module</li>
|
||
|
|
<li>• <b>Tests isolate</b>: thumbnail tests mock 3 deps, not 8</li>
|
||
|
|
<li>• <b>FileApiImpl becomes a clean gateway</b> — only file I/O remains</li>
|
||
|
|
<li>• <b>Deletion test passes</b>: protocol concentrates, not just moves</li>
|
||
|
|
</ul>
|
||
|
|
</div>
|
||
|
|
</article>
|
||
|
|
|
||
|
|
<!-- ========== Candidate 3 ========== -->
|
||
|
|
<article id="c3" class="space-y-4">
|
||
|
|
<div class="flex items-baseline gap-3 flex-wrap">
|
||
|
|
<h2 class="text-2xl font-serif">3. Fix N+1 region query in fillDisplayFields</h2>
|
||
|
|
<span class="text-xs font-bold uppercase tracking-wider px-2 py-0.5 rounded bg-slate-200 text-slate-600">Speculative</span>
|
||
|
|
<span class="text-xs text-slate-400">in-process</span>
|
||
|
|
</div>
|
||
|
|
|
||
|
|
<!-- Files -->
|
||
|
|
<div class="text-sm font-mono text-slate-600">
|
||
|
|
crm-lead/…/service/impl/LeadServiceImpl.java (lines 517–565, fillDisplayFields)<br>
|
||
|
|
crm-rule/…/service/ISysRegionService.java (getByCode — called in loop)
|
||
|
|
</div>
|
||
|
|
|
||
|
|
<!-- Before / After -->
|
||
|
|
<div class="grid grid-cols-2 gap-4">
|
||
|
|
<!-- Before -->
|
||
|
|
<div class="space-y-2">
|
||
|
|
<h3 class="text-xs uppercase tracking-wider text-slate-400">Before</h3>
|
||
|
|
<div class="rounded-lg border border-red-200 bg-red-50 p-4">
|
||
|
|
<pre class="text-xs font-mono text-red-800 leading-relaxed">for (String code : regionCodes) {
|
||
|
|
var region = sysRegionService
|
||
|
|
.getByCode(code); // N+1
|
||
|
|
regionNameMap.put(code,
|
||
|
|
region.getName());
|
||
|
|
}</pre>
|
||
|
|
<div class="mt-2 text-xs text-red-600">10 leads × 2 codes = 20 individual queries</div>
|
||
|
|
</div>
|
||
|
|
<p class="text-xs text-slate-400">fillDisplayFields: 50 lines of read-model assembly inside the lead service. Follow counts use batch queries; region names do not.</p>
|
||
|
|
</div>
|
||
|
|
<!-- After -->
|
||
|
|
<div class="space-y-2">
|
||
|
|
<h3 class="text-xs uppercase tracking-wider text-slate-400">After</h3>
|
||
|
|
<div class="rounded-lg border border-emerald-200 bg-emerald-50 p-4">
|
||
|
|
<pre class="text-xs font-mono text-emerald-800 leading-relaxed">Map<String,String> names =
|
||
|
|
sysRegionService
|
||
|
|
.batchGetNames(codes); // 1 query</pre>
|
||
|
|
<div class="mt-2 text-xs text-emerald-600">1 batch query replaces N loop calls</div>
|
||
|
|
</div>
|
||
|
|
<p class="text-xs text-slate-400">Fix: add batchGetByCodes to ISysRegionService. Optional: extract fillDisplayFields into a LeadDisplayEnricher module (~50 lines, 1-method interface).</p>
|
||
|
|
</div>
|
||
|
|
</div>
|
||
|
|
|
||
|
|
<!-- Problem / Solution -->
|
||
|
|
<div class="grid grid-cols-2 gap-4 text-sm">
|
||
|
|
<div>
|
||
|
|
<div class="text-xs uppercase tracking-wider text-red-500 mb-1">Problem</div>
|
||
|
|
<p><code class="text-sm">fillDisplayFields</code> resolves region names by calling <code class="text-sm">sysRegionService.getByCode(code)</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.</p>
|
||
|
|
</div>
|
||
|
|
<div>
|
||
|
|
<div class="text-xs uppercase tracking-wider text-emerald-600 mb-1">Solution</div>
|
||
|
|
<p><b>Fix</b>: add <code class="text-sm">batchGetByCodes(Set<String>)</code> to ISysRegionService, replace the loop with one call. <b>Optional</b>: extract <code class="text-sm">fillDisplayFields</code> into a <code class="text-sm">LeadDisplayEnricher</code> module (1-method interface, ~50 lines) to isolate read-model assembly from the lead write model.</p>
|
||
|
|
</div>
|
||
|
|
</div>
|
||
|
|
|
||
|
|
<!-- Wins -->
|
||
|
|
<div>
|
||
|
|
<div class="text-xs uppercase tracking-wider text-slate-400 mb-1">Wins</div>
|
||
|
|
<ul class="text-sm space-y-0.5 text-slate-700">
|
||
|
|
<li>• <b>Bug fix</b>: N+1 → batch query (concrete, regardless of extraction)</li>
|
||
|
|
<li>• <b>Locality (optional)</b>: read-model assembly isolated from write model</li>
|
||
|
|
<li>• <b>Leverage (optional)</b>: 1-method interface, ~50 lines behind seam</li>
|
||
|
|
</ul>
|
||
|
|
</div>
|
||
|
|
</article>
|
||
|
|
|
||
|
|
<!-- ========== Top recommendation ========== -->
|
||
|
|
<section id="top-recommendation" class="pt-4">
|
||
|
|
<div class="deep-box rounded-xl p-6 space-y-2">
|
||
|
|
<div class="flex items-baseline gap-3">
|
||
|
|
<span class="text-xs uppercase tracking-wider text-emerald-400">Top recommendation</span>
|
||
|
|
</div>
|
||
|
|
<h2 class="text-xl font-serif text-white">Candidate 1 — Extract MultipartUploader</h2>
|
||
|
|
<p class="text-sm text-slate-300">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. <a href="#c1" class="text-emerald-400 underline">↑ jump to card</a></p>
|
||
|
|
</div>
|
||
|
|
</section>
|
||
|
|
|
||
|
|
</main>
|
||
|
|
</body>
|
||
|
|
</html>
|