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.
 
 
 
 
 
 

329 lines
26 KiB

<!doctype html>
<html lang="zh">
<head>
<meta charset="utf-8" />
<title>Architecture review — crm-backend-matt</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); }
.badge-strong { background: #059669; }
.badge-worth { background: #d97706; }
.badge-spec { background: #64748b; }
.module-box { border: 2px solid #475569; border-radius: 8px; }
.shallow-box { border: 2px solid #f59e0b; border-radius: 8px; background: #fffbeb; }
.deep-box { border: 3px solid #0f172a; border-radius: 8px; background: #f8fafc; }
.internal { opacity: 0.35; }
.legend-box { width: 16px; height: 16px; border-radius: 3px; display: inline-block; vertical-align: middle; margin-right: 4px;}
</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 class="border-b border-slate-200 pb-6">
<div class="flex items-baseline justify-between">
<h1 class="text-3xl font-serif text-slate-900">Architecture Review</h1>
<span class="text-sm text-slate-500 font-mono">crm-backend-matt &middot; 2026-08-08</span>
</div>
<div class="mt-4 flex flex-wrap gap-4 text-xs text-slate-600">
<span><span class="legend-box bg-white border-2 border-slate-500"></span> module</span>
<span><span class="legend-box bg-amber-50 border-2 border-amber-400"></span> shallow</span>
<span><span class="legend-box bg-slate-100 border-2 border-slate-900" style="border-width:3px;"></span> deep</span>
<span>&dotsquare; dashed = seam</span>
<span style="color:#dc2626;">&mdash; red = leakage</span>
</div>
<p class="mt-2 text-sm text-slate-500">Three deepening opportunities across the crm-file, crm-dict, and crm-auth modules. Vocabulary from <code class="text-xs">/codebase-design</code>; domain terms from <code class="text-xs">CONTEXT.md</code>.</p>
</header>
<section id="candidates" class="space-y-12">
<!-- Candidate 1: Thumbnail lifecycle split -->
<article class="bg-white rounded-xl border border-slate-200 shadow-sm overflow-hidden">
<div class="p-6 space-y-5">
<div class="flex items-center justify-between">
<h2 class="text-xl font-serif text-slate-900">1. Thumbnail lifecycle split across two modules</h2>
<div class="flex gap-2">
<span class="badge-strong text-white text-xs font-bold px-2.5 py-1 rounded-full">Strong</span>
<span class="bg-slate-200 text-slate-700 text-xs font-bold px-2.5 py-1 rounded-full">in-process</span>
</div>
</div>
<div>
<h3 class="text-xs uppercase tracking-wider text-slate-500 font-semibold mb-1">Files</h3>
<ul class="font-mono text-sm text-slate-700 space-y-0.5">
<li>crm-file/service/api/<strong>FileApi.java</strong> &mdash; 11 methods, includes <code>getThumbnail()</code></li>
<li>crm-file/service/impl/<strong>FileApiImpl.java</strong> &mdash; 551 lines; ~100 lines are thumbnail read/fallback</li>
<li>crm-file/task/<strong>ThumbnailGenerationTask.java</strong> &mdash; generation + lock, but no retrieval</li>
<li>crm-file/service/<strong>ThumbnailPlaceholderService.java</strong>, <strong>ThumbnailRenderer.java</strong></li>
</ul>
</div>
<div class="grid grid-cols-2 gap-4">
<div class="rounded-lg border border-amber-300 bg-amber-50/50 p-4">
<div class="text-xs uppercase tracking-wider text-amber-700 font-semibold mb-3 text-center">Before</div>
<div class="space-y-3">
<div class="shallow-box p-3">
<div class="text-xs uppercase tracking-wider text-amber-600 font-semibold">FileApiImpl (551 lines)</div>
<div class="text-xs text-slate-500 mt-1">upload &middot; download &middot; preview &middot; multipart &middot; <strong class="text-amber-700">getThumbnail &middot; handlePending &middot; readThumbnail &middot; placeholderOf</strong></div>
<div class="text-xs text-red-600 mt-1">&uarr; thumbnail read path leaks here</div>
</div>
<div class="text-center text-slate-400 text-xs">&dotsquare;&dotsquare; seam (not a real one) &dotsquare;&dotsquare;</div>
<div class="module-box p-3 bg-white">
<div class="text-xs uppercase tracking-wider text-slate-600 font-semibold">ThumbnailGenerationTask</div>
<div class="text-xs text-slate-500 mt-1">assignInitialStatus &middot; generate &middot; tryGenerateSync &middot; generateSync &middot; findRenderer</div>
<div class="text-xs text-slate-400 mt-1">generation only &mdash; half the lifecycle</div>
</div>
</div>
<p class="text-xs text-red-600 mt-3 text-center font-semibold">No locality: thumbnail bugs span two modules</p>
</div>
<div class="rounded-lg border border-emerald-300 bg-emerald-50/50 p-4">
<div class="text-xs uppercase tracking-wider text-emerald-700 font-semibold mb-3 text-center">After</div>
<div class="space-y-3">
<div class="deep-box p-3">
<div class="text-xs uppercase tracking-wider text-slate-800 font-semibold">FileApi (8 methods)</div>
<div class="text-xs text-slate-500 mt-1">upload &middot; download &middot; preview &middot; multipart</div>
<div class="text-xs text-slate-400 mt-1">file ops only &mdash; interface narrows</div>
</div>
<div class="text-center text-slate-400 text-xs">&dotsquare;&dotsquare; seam &dotsquare;&dotsquare;</div>
<div class="deep-box p-3" style="border-width: 3px;">
<div class="text-xs uppercase tracking-wider text-slate-800 font-semibold">ThumbnailService (deep)</div>
<div class="text-xs text-slate-500 mt-1"><strong>interface</strong>: getThumbnail &middot; assignInitialStatus &middot; generate</div>
<div class="text-xs text-slate-400 mt-1 internal"><strong>implementation</strong>: handlePending &middot; poll &middot; readThumbnail &middot; generateSync &middot; findRenderer &middot; placeholderOf</div>
</div>
</div>
<p class="text-xs text-emerald-600 mt-3 text-center font-semibold">Locality: full lifecycle in one module</p>
</div>
</div>
<div class="space-y-2">
<p class="text-sm"><strong class="text-slate-700">Problem:</strong> The thumbnail capability has no single module &mdash; generation lives in <code>ThumbnailGenerationTask</code>, retrieval and sync-fallback (<code>handlePending</code>, <code>readThumbnail</code>, <code>placeholderOf</code>) lives in <code>FileApiImpl</code>. 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.</p>
<p class="text-sm"><strong class="text-slate-700">Solution:</strong> Extract a <code>ThumbnailService</code> module that owns the entire thumbnail lifecycle &mdash; generation, retrieval, sync-fallback, placeholder. <code>FileApi</code> narrows to file operations only. <code>FileApiImpl.upload()</code> and <code>completeMultipart()</code> call <code>ThumbnailService.assignInitialStatus()</code> + <code>generate()</code> instead of reaching into <code>ThumbnailGenerationTask</code> directly.</p>
</div>
<div>
<h3 class="text-xs uppercase tracking-wider text-slate-500 font-semibold mb-2">Wins</h3>
<ul class="text-sm space-y-1">
<li class="flex items-start gap-2"><span class="text-emerald-600">&blacktriangleright;</span> <strong>locality</strong>: thumbnail bugs concentrate in one module</li>
<li class="flex items-start gap-2"><span class="text-emerald-600">&blacktriangleright;</span> <strong>leverage</strong>: one ThumbnailService interface, FileApi + FileController both call it</li>
<li class="flex items-start gap-2"><span class="text-emerald-600">&blacktriangleright;</span> interface shrinks: FileApi drops <code>getThumbnail</code></li>
<li class="flex items-start gap-2"><span class="text-emerald-600">&blacktriangleright;</span> tests hit one interface for the full lifecycle</li>
<li class="flex items-start gap-2"><span class="text-emerald-600">&blacktriangleright;</span> FileApiImpl shrinks ~100 lines</li>
</ul>
</div>
</div>
</article>
<!-- Candidate 2: Permission seeding duplicated -->
<article class="bg-white rounded-xl border border-slate-200 shadow-sm overflow-hidden">
<div class="p-6 space-y-5">
<div class="flex items-center justify-between">
<h2 class="text-xl font-serif text-slate-900">2. Permission seeding: shadow entities + logic duplication</h2>
<div class="flex gap-2">
<span class="badge-strong text-white text-xs font-bold px-2.5 py-1 rounded-full">Strong</span>
<span class="bg-slate-200 text-slate-700 text-xs font-bold px-2.5 py-1 rounded-full">ports &amp; adapters</span>
</div>
</div>
<div>
<h3 class="text-xs uppercase tracking-wider text-slate-500 font-semibold mb-1">Files</h3>
<ul class="font-mono text-sm text-slate-700 space-y-0.5">
<li>crm-dict/config/<strong>DictPermissionInitializer.java</strong> &mdash; 157 lines, seeds dict:* permission points</li>
<li>crm-dict/domain/entity/<strong>SysMenuSeed.java</strong>, <strong>SysRoleSeed.java</strong>, <strong>SysRoleMenuSeed.java</strong> &mdash; shadow entities mirroring auth tables</li>
<li>crm-auth/config/<strong>DataInitializer.java</strong> &mdash; 165 lines, seeds crm:role:* permission points (same pattern)</li>
</ul>
</div>
<div class="grid grid-cols-2 gap-4">
<div class="rounded-lg border border-amber-300 bg-amber-50/50 p-4">
<div class="text-xs uppercase tracking-wider text-amber-700 font-semibold mb-3 text-center">Before</div>
<div class="space-y-2">
<div class="shallow-box p-2">
<div class="text-xs font-semibold text-amber-700">crm-dict / DictPermissionInitializer</div>
<div class="text-xs text-slate-500">findMenuByName &middot; insertButtonIfAbsent &middot; bindIfAbsent</div>
</div>
<div class="shallow-box p-2 internal">
<div class="text-xs font-semibold text-amber-600">SysMenuSeed, SysRoleSeed, SysRoleMenuSeed</div>
<div class="text-xs text-slate-400">@TableName("sys_menu") &mdash; schema copy</div>
</div>
<div class="flex justify-center text-red-500 text-lg">&orarr;</div>
<div class="shallow-box p-2">
<div class="text-xs font-semibold text-amber-700">crm-auth / DataInitializer</div>
<div class="text-xs text-slate-500">findMenu &middot; insertButtonIfAbsent &middot; bindIfAbsent</div>
</div>
<div class="module-box p-2 bg-white">
<div class="text-xs font-semibold text-slate-600">SysMenu, SysRole, SysRoleMenu</div>
<div class="text-xs text-slate-400">@TableName("sys_menu") &mdash; real schema</div>
</div>
</div>
<p class="text-xs text-red-600 mt-3 text-center font-semibold">Schema duplicated; logic duplicated; leak across seam</p>
</div>
<div class="rounded-lg border border-emerald-300 bg-emerald-50/50 p-4">
<div class="text-xs uppercase tracking-wider text-emerald-700 font-semibold mb-3 text-center">After</div>
<div class="space-y-2">
<div class="deep-box p-2" style="border-width: 2px;">
<div class="text-xs font-semibold text-slate-700">crm-base / PermissionSeeder (interface)</div>
<div class="text-xs text-slate-500">registerPoint(perms, apiUrl, name, parentPath)</div>
</div>
<div class="text-center text-slate-400 text-xs">&dotsquare;&dotsquare; seam &dotsquare;&dotsquare;</div>
<div class="flex gap-2">
<div class="module-box p-2 bg-white flex-1">
<div class="text-xs font-semibold text-slate-600">crm-dict initializer</div>
<div class="text-xs text-slate-400">thin caller</div>
</div>
<div class="module-box p-2 bg-white flex-1">
<div class="text-xs font-semibold text-slate-600">crm-auth initializer</div>
<div class="text-xs text-slate-400">thin caller</div>
</div>
</div>
<div class="deep-box p-2" style="border-width: 3px;">
<div class="text-xs font-semibold text-slate-800">PermissionSeederImpl (crm-auth)</div>
<div class="text-xs text-slate-400 internal">owns SysMenu, SysRole, SysRoleMenu schema</div>
</div>
</div>
<p class="text-xs text-emerald-600 mt-3 text-center font-semibold">Schema knowledge in one module; one interface for seeding</p>
</div>
</div>
<div class="space-y-2">
<p class="text-sm"><strong class="text-slate-700">Problem:</strong> Two initializers duplicate the same permission-seeding logic (<code>findMenuByName</code>, <code>insertButtonIfAbsent</code>, <code>bindIfAbsent</code>) with nearly identical structure. <code>crm-dict</code> creates shadow entities (<code>SysMenuSeed</code>, <code>SysRoleSeed</code>, <code>SysRoleMenuSeed</code>) that mirror <code>crm-auth</code>'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 <code>sys_menu</code> schema (column names, menu types, status values) into <code>crm-dict</code>. Schema changes in <code>crm-auth</code> silently break <code>crm-dict</code>'s seeding.</p>
<p class="text-sm"><strong class="text-slate-700">Solution:</strong> Extract a <code>PermissionSeeder</code> seam &mdash; an interface in <code>crm-base</code> that declares <code>registerPoint(perms, apiUrl, name, parentMenuPath)</code> without exposing <code>sys_menu</code>'s schema. <code>crm-auth</code> provides the implementation (owns <code>SysMenu</code>, <code>SysRole</code>, <code>SysRoleMenu</code>). Both initializers become thin callers of the same seam. Three shadow entities deleted.</p>
</div>
<div class="rounded-lg border border-amber-300 bg-amber-50 p-3">
<p class="text-sm text-amber-800">
<strong>ADR-0015 alignment:</strong> This <em>fulfills</em> 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 <code>PermissionSeeder</code> interface in <code>crm-base</code>, implemented by <code>crm-auth</code>, preserves both constraints. The current shadow-entity approach actually <em>violates</em> the ADR's intent by leaking physical schema &mdash; the contract should be code strings, not <code>@TableName("sys_menu")</code>.
</p>
</div>
<div>
<h3 class="text-xs uppercase tracking-wider text-slate-500 font-semibold mb-2">Wins</h3>
<ul class="text-sm space-y-1">
<li class="flex items-start gap-2"><span class="text-emerald-600">&blacktriangleright;</span> <strong>locality</strong>: <code>sys_menu</code> schema knowledge concentrates in one module</li>
<li class="flex items-start gap-2"><span class="text-emerald-600">&blacktriangleright;</span> <strong>leverage</strong>: one <code>PermissionSeeder</code> interface, N seeders call it</li>
<li class="flex items-start gap-2"><span class="text-emerald-600">&blacktriangleright;</span> delete 3 shadow entities + ~100 lines duplicated logic</li>
<li class="flex items-start gap-2"><span class="text-emerald-600">&blacktriangleright;</span> future modules (crm-rule, crm-audit) seed permissions through the same seam</li>
<li class="flex items-start gap-2"><span class="text-emerald-600">&blacktriangleright;</span> schema changes no longer break cross-module seeding</li>
</ul>
</div>
</div>
</article>
<!-- Candidate 3: DingTalkAuthClient testability seam -->
<article class="bg-white rounded-xl border border-slate-200 shadow-sm overflow-hidden">
<div class="p-6 space-y-5">
<div class="flex items-center justify-between">
<h2 class="text-xl font-serif text-slate-900">3. DingTalkAuthClient: test-by-subclass instead of adapter</h2>
<div class="flex gap-2">
<span class="badge-worth text-white text-xs font-bold px-2.5 py-1 rounded-full">Worth exploring</span>
<span class="bg-slate-200 text-slate-700 text-xs font-bold px-2.5 py-1 rounded-full">mock</span>
</div>
</div>
<div>
<h3 class="text-xs uppercase tracking-wider text-slate-500 font-semibold mb-1">Files</h3>
<ul class="font-mono text-sm text-slate-700 space-y-0.5">
<li>crm-auth/service/client/<strong>DingTalkAuthClient.java</strong> &mdash; 314 lines; 4 protected HTTP stubs (~120 lines)</li>
<li>crm-auth/service/client/<strong>ThirdPartyAuthClient.java</strong> &mdash; 38 lines interface (getType, getUserInfo, isOrgMember)</li>
<li>crm-auth/service/client/<strong>ThirdPartyAuthClientFactory.java</strong> &mdash; 37 lines</li>
</ul>
</div>
<div class="grid grid-cols-2 gap-4">
<div class="rounded-lg border border-amber-300 bg-amber-50/50 p-4">
<div class="text-xs uppercase tracking-wider text-amber-700 font-semibold mb-3 text-center">Before &mdash; mixed mass</div>
<div class="flex items-end gap-2 justify-center" style="height: 180px;">
<div class="flex flex-col items-center">
<div class="text-xs text-slate-500 mb-1">interface</div>
<div class="shallow-box w-16 bg-amber-100" style="height: 20px;"></div>
<div class="text-xs text-slate-400 mt-1">3 methods</div>
</div>
<div class="text-2xl text-slate-300">&rarr;</div>
<div class="flex flex-col items-center">
<div class="text-xs text-slate-500 mb-1">implementation</div>
<div class="shallow-box w-24 bg-amber-100" style="height: 150px; display: flex; flex-direction: column; justify-content: space-around; align-items: center; padding: 6px 0;">
<div class="text-[10px] text-amber-700">HTTP stubs</div>
<div class="text-[10px] text-amber-600">token cache</div>
<div class="text-[10px] text-amber-600">error parse</div>
<div class="text-[10px] text-amber-600">org verify</div>
<div class="text-[10px] text-amber-600">corp token</div>
</div>
<div class="text-xs text-slate-400 mt-1">314 lines</div>
</div>
</div>
<p class="text-xs text-red-600 mt-2 text-center">HTTP plumbing tangled with business logic; tests subclass to stub</p>
</div>
<div class="rounded-lg border border-emerald-300 bg-emerald-50/50 p-4">
<div class="text-xs uppercase tracking-wider text-emerald-700 font-semibold mb-3 text-center">After &mdash; split by concern</div>
<div class="flex items-end gap-4 justify-center" style="height: 180px;">
<div class="flex flex-col items-center">
<div class="text-xs text-slate-500 mb-1">client interface</div>
<div class="deep-box w-16 bg-slate-100" style="height: 20px; border-width: 2px;"></div>
<div class="text-xs text-slate-400 mt-1">3 methods</div>
</div>
<div class="text-2xl text-slate-300">&rarr;</div>
<div class="flex flex-col items-center">
<div class="text-xs text-slate-500 mb-1">DingTalkAuthClient</div>
<div class="deep-box w-20 bg-slate-100" style="height: 90px; border-width: 3px; display: flex; flex-direction: column; justify-content: space-around; align-items: center; padding: 6px 0;">
<div class="text-[10px] text-slate-700">token cache</div>
<div class="text-[10px] text-slate-600">error parse</div>
<div class="text-[10px] text-slate-600">org verify</div>
</div>
<div class="text-xs text-slate-400 mt-1">~150 lines</div>
</div>
<div class="flex flex-col items-center">
<div class="text-xs text-slate-500 mb-1">DingTalkHttpClient</div>
<div class="deep-box w-20 bg-slate-100" style="height: 60px; border-width: 2px; display: flex; flex-direction: column; justify-content: center; align-items: center; padding: 6px 0;">
<div class="text-[10px] text-slate-600">HTTP plumbing</div>
</div>
<div class="text-xs text-slate-400 mt-1">~120 lines</div>
</div>
</div>
<p class="text-xs text-emerald-600 mt-2 text-center">Business logic testable through adapter; two adapters = real seam</p>
</div>
</div>
<div class="space-y-2">
<p class="text-sm"><strong class="text-slate-700">Problem:</strong> Four <code>protected</code> HTTP methods (<code>requestTokenApi</code>, <code>requestUserInfoApi</code>, <code>requestCorpTokenApi</code>, <code>requestGetByUnionIdApi</code>) exist purely as test hooks &mdash; 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 <code>ThirdPartyAuthClient</code> interface. Bugs in error parsing or token refresh hide in private methods that call the stubs.</p>
<p class="text-sm"><strong class="text-slate-700">Solution:</strong> Extract a <code>DingTalkHttpClient</code> adapter (interface + real implementation) that owns all HTTP calls. <code>DingTalkAuthClient</code> injects the adapter and becomes pure business logic. Tests inject a stub <code>DingTalkHttpClient</code> through the constructor, testing business logic through the <code>ThirdPartyAuthClient</code> interface.</p>
</div>
<div>
<h3 class="text-xs uppercase tracking-wider text-slate-500 font-semibold mb-2">Wins</h3>
<ul class="text-sm space-y-1">
<li class="flex items-start gap-2"><span class="text-amber-600">&blacktriangleright;</span> <strong>locality</strong>: HTTP plumbing concentrates in the adapter</li>
<li class="flex items-start gap-2"><span class="text-amber-600">&blacktriangleright;</span> tests inject stub adapter, test through interface (no inheritance)</li>
<li class="flex items-start gap-2"><span class="text-amber-600">&blacktriangleright;</span> delete 4 protected methods + boilerplate</li>
<li class="flex items-start gap-2"><span class="text-amber-600">&blacktriangleright;</span> two adapters (real + test stub) justify the seam</li>
</ul>
</div>
</div>
</article>
</section>
<!-- Top recommendation -->
<section id="top-recommendation" class="bg-slate-900 text-white rounded-xl p-6">
<h2 class="text-xs uppercase tracking-wider text-slate-400 font-semibold mb-2">Top recommendation</h2>
<h3 class="text-2xl font-serif mb-2">
<a href="#candidates" class="hover:text-emerald-400 transition">#2 &mdash; Permission seeding seam</a>
</h3>
<p class="text-sm text-slate-300">
Start here. It's the highest-leverage deepening: one new interface in <code>crm-base</code> 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") &mdash; 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.
</p>
<div class="mt-4 flex gap-4 text-xs text-slate-400">
<span><strong class="text-emerald-400">#1 Thumbnail</strong> &mdash; highest friction but larger blast radius (FileApi is the most-used interface)</span>
<span><strong class="text-emerald-400">#3 DingTalk</strong> &mdash; testability gain, but only one client exists today</span>
</div>
</section>
</main>
</body>
</html>