3625ce8 fixed the no-empty-object-type lint error by switching a
zero-IsQuery table's {ClassName}Query to `Record<string, never>`, the
same default useTable.ts's own `TQuery extends object = Record<string,
never>` uses. That default is safe there only because useTable.ts's one
internal `TQuery & PageQuery` goes through an `as` cast rather than a
structural check. Code that builds the object literal directly does not
get that protection - and vue.go.template's foreign-key dropdown fetch
does exactly that: `list{FkClass}({ pageIndex: 1, pageSize: 100 })`.
Record<string, never> is a mapped type over every string key, each
mapped to never, so `Record<string, never> & PageQuery` does not leave
PageQuery's own properties alone: pageIndex becomes `never & number`,
i.e. never, and no value can be passed for it - a straight type error,
not a lint warning, so the previous fix's pnpm lint pass did not catch
it. Trips on any query-less table referenced by a foreign key - a plain
lookup/dict table used as a dropdown source, an ordinary shape, not a
rare one.
Switched to Record<never, never>: a mapped type over the empty key
set, which behaves as the empty object type `{}` under intersection
(PageQuery's properties come through unchanged) while still satisfying
`TQuery extends object` and not tripping no-empty-object-type (it is a
generic instantiation, not a literal `{}` type annotation) - confirmed
all three separately before touching the template.
Verified with node 24.11.0: generated a real zero-IsQuery table, copied
its .ts into go-admin-ui alongside a throwaway file reproducing
vue.go.template's exact FK call site
(`await list{Class}({ pageIndex: 1, pageSize: 100 })`), and ran both
eslint and vue-tsc --noEmit - the type-check step lint alone cannot
cover, which is what let this through the first time. Confirmed red
first (swapped the generated file's Record<never, never> back to
Record<string, never>): vue-tsc reported the exact "Property 'pageIndex'
is incompatible with index signature" error. Restored the fix - both
clean.
jsonFieldPattern copied businessName's rule (^[a-z][A-Za-z]+$: at least
two letters, no digits) on the theory that jsonField should tighten to
the same identifier shape. That does not hold: businessName is typed
by a person on genInfoForm.vue, so a strict pattern is a reasonable
guardrail on human input. jsonField is computed by the importer from
the column name (sys_tables.go's namelist/JsonField loop) - nobody
types it, so the same pattern only rejected names the importer
legitimately produces: a single-letter column ("x") or one whose last
segment ends in a digit ("address2", "a1") both collapse to a single
camelCase word with nothing left to re-capitalize, and both failed the
old check.
The blast radius is wider than "this one column can't be edited":
validateAndSanitizeColumns runs over every column on every Update, so
a table that merely contains one such column could not save any
config change at all, including edits with nothing to do with that
column.
Relaxed to ^[a-z][A-Za-z0-9]*$ - any legal JS/TS identifier starting
with a lowercase letter. Still rejects what has to be rejected: empty,
whitespace/punctuation, and leading-digit names, since those cannot be
unquoted object keys in the generated interface/lang file at all.
Uniqueness and the expression-content check on defaultValue are
unchanged - defaultValue is genuinely user-typed (F6's config page),
so tightening it was the right call to begin with; this was the only
place a human-input rule had been copied onto machine-generated data.
Verified against the real import path, not hand-typed jsonField values:
built a table with columns id/x/address2/a1 in a fake information_schema,
ran it through the real SysTable.Insert, confirmed the importer computes
exactly jsonField x/address2/a1, then submitted an update through the
real SysTable.Update changing only tableComment (nothing about those
columns). Confirmed red first - 500, "jsonField 格式不合法：\"x\"" - a
change unrelated to any of the three columns was rejected solely because
they existed on the table. Restored the fix - 200, "修改成功".
A plain display table with no search form (zero columns marked
IsQuery=1) is a normal shape, not an edge case - but ts.go.template's
Query interface only had a body when the range over .Columns found a
match, so it rendered `export interface {ClassName}Query {}`, which
@typescript-eslint/no-empty-object-type flags and pnpm lint fails on.
Falls back to `export type {ClassName}Query = Record<string, never>`
when no column qualifies - the same answer go-admin-ui's own
useTable.ts already gives this shape (`TQuery extends object =
Record<string, never>`), so it intersects with PageQuery the same way
an empty interface would have and callers do not need to special-case
a query-less table.
Audited the rest of this template for the same "zero of some optional
feature" shape while in here, since this is the third time a query-less/
select-less/whatever-less table has slipped through (dictLabel, DateCell,
this one):
- a zero-column table: cannot occur - a table without at least a
primary key column cannot exist to be imported from information_schema
in the first place, so .Columns is never empty here.
- a table with only its primary key column: the row interface still
has one property (the pk); not empty, no lint issue.
- a table where every column has IsInsert=0: does not affect this
template - add{Class}/update{Class} both take the full row interface
unconditionally (every column, not just IsInsert ones), so there is
no column-count-dependent shape to go empty here.
Verified with node 24.11.0 (not the machine default): generated a
zero-IsQuery table and a with-IsQuery table, copied both into
go-admin-ui and ran eslint + vue-tsc --noEmit. Confirmed red first -
`export interface VerifyNoQueryQuery {}` failed eslint with exactly the
no-empty-object-type error. Restored the fix - clean on both, plus a
throwaway call site instantiating useTable<VerifyNoQuery,
VerifyNoQueryQuery> to prove the type satisfies useTable's `TQuery
extends object` constraint, not just that it parses in isolation.
MLTBName (table_name with underscores turned to dashes, e.g. "user_profile"
-> "user-profile") is a gorm:"-" field - table.Get never fills it in, the
caller has to. NOActionsGen has done so since it existed; Preview never
did, so every template's import path that reads it
(from '@/api/{PackageName}/{MLTBName}' in vue.go.template, present in
both the pre-Vue3 template and F4's rewrite) rendered with the module
segment missing - "from '@/api/admin/'" - in the preview dialog only.
The real generated file was always correct.
Pre-existing, not introduced by this PRD, but worth fixing now: F3/F9
added two more Preview panes (the language packs) on top of an assumption
- that Preview's output stands in for what NOActionsGen actually writes -
that was never true for this field. Checked the rest of gen.go for the
same "only set on the write path" shape; MLTBName is the only one -
every other field Preview's templates read comes straight off the
sys_tables/sys_columns rows table.Get already loads.
Verified with a throwaway harness driving Gen.Preview through a real gin
Context and sqlite-backed db, parsing the JSON response and reading back
template/vue.go.template's import line. Confirmed red first (temporarily
removed the added line): "from '@/api/verify010/'". Restored it: "from
'@/api/verify010/verify-widget'".
vue.go.template (F4) only had a flat fallback for an unconfigured
colWidth: 110 for datetime, 120 for everything else. R2 asks for a
precise per-type inference instead - varchar(n) tiered by n, tinyint(1)
narrower than a general integer, text/blob wide - sized so a typical
table's list columns land inside go-admin-ui's ~580px text-column
budget. text/template cannot parse "varchar(255)" itself, so this has
to run in gen.go before the template executes, not in the template.
Judgment is columnType, not goType, same as F1/F2's earlier reversal
(API契约.md §1.1): sys_tables.go:323-338 gives every non-primary-key
int/tinyint/bigint/decimal column goType "string", so goType alone
cannot tell a boolean flag from a bigint from a name column.
InferColumnWidth(columnType string) int is exported and pure per
测试用例.md §2.5's own request, so it can be pinned with an exact
input/output table rather than only asserting "the rendered page
happens not to overflow" - see column_width_test.go, including the
tinyint(1)-vs-tinyint(4) and the varchar tier-boundary cases.
applyInferredColumnWidths only touches a column already at the 0
sentinel, wired into both Preview and NOActionsGen ahead of the
template execute calls; a column already configured (by the user or F6)
is left untouched, also pinned by a test.
Verified against the real NOActionsGen call path (not just the unit
test): generated a table mixing every branch (tinyint(1), datetime,
decimal, varchar at three tiers, text, and one pre-configured column)
and printed the resulting tab.Columns[i].ColWidth after generation -
every value matched InferColumnWidth's own table, and the
pre-configured column's 333 was left untouched.
ts.go.template hardcoded the pk parameter as `: number`. vue.go.template
(F4, landed after this file) derives $pkType per table - "number" unless
the pk column's GoType is "string" (natural keys do exist; sys_tables.go
gives a pk column GoType "string" whenever its ColumnType is not
int-shaped), then types useForm on that same $pkType. A string-pk table
therefore generated a page calling get{Class}(id: string) against an api
module whose get{Class} only accepted number - a TS compile error, not
a runtime bug, and exactly the kind of gap each side's own template-only
verification could not see (F4 was checked against a hand-written .ts
stub before this template existed; this template was checked against a
numeric-pk fixture only).
Copies vue.go.template's $pkType derivation verbatim so both templates
agree by construction rather than by convention.
Verified: generated both a numeric-pk and a string-pk table's api module,
copied both into go-admin-ui and ran vue-tsc --noEmit - clean. Added a
throwaway call-site file exercising getVerifyStrPk with a string and (via
@ts-expect-error) confirming a number argument is now rejected, so the
type is actually enforced rather than having quietly widened to any.
That bind-and-save path had zero field-level validation (API契约.md
§1.2/§2.1, decision D6), which left four ways to silently corrupt the
generator's own metadata or the language packs it writes:
- jsonField had no format rule at all, unlike tableName/className/
businessName, which all have a pattern check. A jsonField with a
dot, a leading digit, or anything not matching lower camelCase
still saved, and would land verbatim as a key segment in
gen/{PackageName}/{BusinessName}.ts.
- jsonField had no per-table uniqueness check. Two columns retitled
to the same jsonField overwrite each other's generated i18n key
with no warning.
- businessName had no cross-table uniqueness check within a
packageName. Two tables sharing one write the same language pack
path and the second's generation silently clobbers the first's.
- defaultValue is spliced into the generated defaultModel() as a
literal, never evaluated (API契约.md §2.1) - so content shaped
like a function call or block does not do what it looks like it
does, and previously saved without complaint either way.
colWidth is handled differently on purpose: API契约.md §2.1 says an
out-of-range value should fall back to the inferred width rather than
be rejected, so it is reset to the 0 sentinel in place instead of
failing the request - same outcome as if it had never been set.
G10 (colliding with the built-in admin/* i18n namespace) is not one of
the four: D9 already moved generated keys into their own gen/
namespace, so that collision no longer exists.
All four checks and colWidth's sanitize-in-place path are covered by
sys_tables_validate_test.go. Confirmed red first: swapped in a no-op
stand-in for both validators and reran - 13 sub-tests that should now
be rejected or sanitized passed straight through instead (jsonField
format x5, jsonField uniqueness x1, colWidth range x2, defaultValue
expression x4, businessName uniqueness x1). Restored the real
implementation and reran - all green.
Updates(&e) uses GORM's struct form, which skips zero-value fields -
but 0/"" is exactly the sentinel PRD 010 F1/F2 chose for "unconfigured"
(数据库变更.md §1.1). A caller resetting colWidth or defaultValue back
to that sentinel was therefore silently ignored: the row kept its old
value, the API reported success, and reopening the edit form showed the
stale number/string again.
Not fixed by switching to Select("*") - that would also start writing
this struct's other zero-valued fields (Sort, the Pk/Required/... bools),
none of which are part of this change. A second, map-form Updates
scoped to just these two columns writes the sentinel without touching
anything else; map-form Updates does not skip zero values.
TestSysColumnsUpdateClearsSentinelFields pins the regression: seed
150/"active", update to 0/"", read back and assert it stuck. Confirmed
red before this change (got 150/"active" back) and green after.
Add lang-zh.go.template and lang-en.go.template, one key per column
keyed by JsonField, valued from ColumnComment (R3) with a fallback to
JsonField when ColumnComment is blank (G11) - both locales resolve to
the same zh-CN text today since D3 rules out machine translation and
go-admin-ui's parity test rejects blank values.
NOActionsGen writes both files to lang/{locale}/gen/{PackageName}/
{BusinessName}.ts, two levels deep so go-admin-ui's gen-namespace.ts
glob (./*/*.ts under each locale's gen/) picks them up; nesting under
PackageName (rather than a flat gen/{BusinessName}.ts) avoids two
tables in different packages silently overwriting each other's
translations, since BusinessName has no uniqueness check.
Output has to be single-quoted with no trailing comma to satisfy
go-admin-ui's eslint config, which text/template's builtin `printf "%q"`
does not produce, hence the small parseGenTemplate/singleQuote helper
shared by the two templates.
Verified end to end: generated a real table's output (including a
blank-ColumnComment column, to exercise the G11 fallback), copied the
two language packs into go-admin-ui and ran vue-tsc --noEmit and eslint
against them - both clean, comma-dangle/quotes included.
Rewrite template/v4/js.go.template (renamed to ts.go.template) to emit
TypeScript matching go-admin-ui's src/api/demo/product.ts: an interface
per row and per query params, typed CRUD functions, and a del{Class}
that takes Id[] like useRemove expects. GoType maps to number for
int/int64/float32/float64 and string otherwise, since go-admin-ui's
request() types the {code,data,msg} envelope rather than the payload.
Preview's response map key changes from template/js.go.template to
template/api.ts.template so its Tab label matches the new content;
NOActionsGen writes the file with a .ts extension.
Verified end to end: generated a real table's output, copied it into
go-admin-ui and ran vue-tsc --noEmit and eslint against it - both clean.
Back the code generator's Vue 3 template migration: col_width lets R2's
column-width inference be overridden per field, and default_value lets
R1/A6's "unconfigured rows still generate a usable page" guarantee hold
for generated forms. Both use a sentinel default (0 / "") rather than
NULL so "unconfigured" has exactly one representation - see
docs-prd/010-代码生成器前端模板迁移Vue3/数据库变更.md §1.1.
The migration and the model change land together: AddColumn reads the
column definition off tools.SysColumns's own gorm tag, so splitting them
across commits would leave one of them failing to compile.
The tree is at zero unformatted files for the first time. Nothing holds it
there: gofmt drift is invisible in review because the common form of it is a
missing newline at the end of a file, which no diff reader notices and no
existing check looks at. That is how the batch cleaned up just now reached 26
files.
`make fmt-check` reports rather than rewrites. CI cannot commit, and a target
that silently reformats hides what it touched; `gofmt -l` names the files and
says nothing when there are none, so its output is both the failure message and
the fix.
The step runs before the tests rather than beside checksilent at the end. A
formatting miss is a one-command fix, and learning about it after five minutes
of tests and an end-to-end install wastes five minutes for nothing.
The diagram at the top reads Router -> Api -> Service -> Model, the section is
headed "Api 层", and the directory is app/admin/apis/. Three sentences in the
body still called that layer Handler -- the name it went by before the rename,
and one nothing in the tree answers to now. A reader following the document has
no way to tell the two are the same layer.
Wording only. No rule changes, and no other file is touched.
`gofmt -l` listed 26 files. Seventeen of them were missing the newline at the
end of the file; the rest are indentation that used spaces where the file uses
tabs, a handful of call sites written `f(a,b)`, and the doc comment spacing
gofmt has rewritten since 1.19 (`//X` to `// X`).
Nothing here changes behaviour: `go build ./...` and `go vet ./...` are clean
and `go test ./common/...` passes, which is the half of the tree these files
are concentrated in.
Only the files gofmt named are touched, so the diff reads line by line rather
than as a reflow of the whole repository. `gofmt -l` is now empty, which is the
precondition for gating it in CI -- worth doing, but a separate change.
NoCache sent `no-cache, no-store, max-age=0, must-revalidate, value`. The
trailing `, value` is not a directive; it is a leftover token that has been on
every response this middleware touches since the file was written. Unknown
directives are ignored, so nothing misbehaved because of it, but it went out on
the wire and read as a mistake to anyone looking.
The assertion added in #937 pins the old value, so it moves with the source:
removing the token from the middleware alone turns TestNoCache red, which is
the whole point of that test and the reason both lines change together here.
The generator's import reads its comma-separated table list with
c.Request.FormValue("tables"), which on a request declaring itself as JSON reads
the URL query and nothing else. go-admin-ui v3.2.0 began sending that list in
the body, so the handler saw an empty string, asked information_schema for a
table named "", and every import failed with "table name cannot be empty！" —
on a fresh installation that is the first thing the generator is asked to do.
tablesToImport reads the query first and falls back to the body, so a front end
sending either works against this server. It also drops blank entries:
splitting "" yields one empty name rather than nothing at all, which is why the
old code reached a database query at all before failing.
The front end sends the list in the query again on its side; this half is what
lets an installation already running v3.2.0 recover without changing it.
The string was spelled out at each site that raises it, and once more in the
test file that asserts on it. A test holding its own copy cannot tell the
difference between the handler answering something else and the message having
been reworded: it goes on asserting a string the server no longer sends, and
goes on passing.
The three copies in app/other/models/tools are left alone; they are raised from
a different layer and nothing asserts on them.
newEngine takes the method, path and handler, so a second test file does not
have to restate the sqlite connection, the driver override and its cleanup, the
CustomError middleware and the two context keys. serveJSON does the same for
running one request and decoding the envelope.
Nothing about what is asserted changes; newColumnListEngine and columnListMsg
keep their names and their callers.
1786700008000 added sys_menu.seed_code and left it NULL on every row that was
already there. That is right for the host's own hand-placed menus: there is
nothing to derive one from.
An application's rows are in that population too, and for those it is
derivable - menu_name is what identified them before the column existed. The
natural-key lookup missed them, so a reseed inserted a second copy beside each
one, and the new unique index could not object, because NULL never collides on
MySQL, PostgreSQL or SQLite and is filtered out of the index on SQL Server.
Claimed when the application is seeded rather than by a backfill migration.
The value is only derivable where the spec's own Code is in hand: menuName
concatenates two pascalCase strings and does not reverse, so a migration
looking at menu_name alone would be guessing. For the same reason more than
one match is refused and named rather than picked from - attaching an
application's menu to whichever row the database returned first is the failure
this is meant to prevent, not a smaller version of it.
The match is scoped to the application's own app_code, so a row belonging to
another application, or to the host, is not claimed.
An adopted row then goes through the ordinary repair, so it comes out carrying
what the spec says rather than what it held from before.
Three degradations turn the new assertions red: not adopting at all, picking a
row when there is more than one, and dropping the app_code from the match.
repairExistingMenu reconciled a row's paths and its api bindings and left
every other column as an earlier run had written it. That cost three different
things, and none of them announced itself.
A menu whose parent was removed and seeded again kept parent_id pointing at
the dead row while its paths named the new one. The tree is built from
parent_id - SysMenu.GetPage walks down from ParentId == 0 - so the menu was
gone from the sidebar, with the migration reporting success.
A menu somebody added by hand under a seeded one kept the old prefix when its
ancestor moved. It is in no spec, so nothing else would ever rewrite it;
SysMenu.Update already does this cascade for the same column when a menu is
moved through the UI.
An application that renamed a menu, or moved its component, in a new version
had the change ignored: the row was found by its natural key and returned
untouched.
The row a spec describes now has one definition, and both the insert and the
repair use it - they cannot drift into disagreeing about what a spec decides.
The repair writes every column on that list.
Visible and IsFrame are deliberately not on it. They are seeding defaults the
application never expressed, so an administrator who hid a seeded menu keeps
it hidden; there is a test that hides one and reseeds.
The cascade matches the row itself or a row strictly underneath it, rather
than `paths LIKE old || '%'`, which also catches /0/1/20 when old is /0/1/2.
An empty old path takes the single-row branch instead: there is no subtree
under one, and the LIKE would have matched the whole table.
Five degradations turn the new assertions red: not writing the spec columns at
all, leaving ParentId off the list, putting Visible on it, not cascading, and
cascading on the loose prefix. The last one did not, at first - the decoy rows
were built against the path of the menu whose parent moved rather than the
path that actually gets rewritten, so the prefix they collided with was never
the one passed to the query.
It was an inlined `go test` in the workflow, and the only thing in the build
that runs it. Every other gate there goes through make - make test, make
build, make checksilent - and `make test` is `go test ./...` in this module,
which cannot reach test/e2e-apporder because that is a module of its own.
So the one check that exercises installing an application was the one check a
developer had no command for, and the only place it could turn red was after
pushing.
Three tests each called newEnv, and newEnv built the binary, so the same
binary was linked three times - about 17 seconds of the run, measured. A
binary is read-only and there is nothing to isolate between tests; each test
still gets its own directory and its own database. The package now builds it
on the first test that needs one and removes it in TestMain. The suite goes
from 39 seconds to 9.
The three assertion blocks listed the same six queries, two or three times
each, differing only in the counts expected - so renaming a table meant
finding three places. They now share one list, with a flag for the uninstall's
"all of them at zero". The reinstall check gets stronger on the way past: it
was three of the six and is now all six.
The per-call sql.Open in count and exec stays. It looks like waste and is not:
the binary under test writes the same file, and a connection held open across
a run of it is a second writer for nothing. The shared part is factored out;
the opening is still per call, and the comment now says why.
Two tests in this package each wrote out what an installed sys_app row looks
like, field for field, and a third inlined the same Create with a different
status. One appRow(t, db, code, status) now covers all three, so a new NOT
NULL column on SysApp is one edit rather than three.
One assertion counted with a bare db.Model(...).Count(&n) and dropped the
error that call returns. A failing query leaves n at zero, which is exactly
what that assertion wanted to see - so the test would have passed on a broken
query. The package already had a count helper that fails on the error, and
this now uses it.
Also a cycle reached from outside itself. The existing case walks straight
into its own cycle from the first code, so the path trimming had nothing to do
and replacing it with the untrimmed path left the test green - the trimming
was never covered. With a requiring b, b requiring c and c requiring b, the
untrimmed report names a as part of a cycle it is not in, and the test goes
red.
Cleanup from a review pass over this branch. No behaviour changes except the
two noted below.
runInstall took app.Snapshot() twice, once inside manifestFor and once for the
cycle check. Snapshot is a deep copy of the registry, and worse than the
copying, the two calls could in principle disagree - the set the cycle check
validated was not provably the set the manifest came from. One snapshot,
passed to both.
appSummary converted a display code back to a stored one with
NormalizeAppCode, which is not that inverse: it leaves "core" as "core", so
the framework needed a branch of its own to stay out of the listing. AppFilter
is the documented inverse and maps it to the empty string, which is not a code
any row is filed under - so the branch goes, and the function now matches
filterAppsByApp twenty lines below it, which was already using AppFilter.
That branch only half-covered what it guarded: a sys_app row carrying an empty
or reserved app_code was still merged into the framework's group by
groupByApp, with only its summary suppressed. loadApps now drops such rows,
which is the one place that settles it for every reader of the map.
requiresInstalled built two parallel slices with a tuple assignment repeated in
three branches; it now picks a reason and appends once. Its last arm was a
catch-all on "not installed", so a status constant added later would have been
described as "did not finish" - a sentence that would be wrong for whatever
reason the constant was added. Unrecognised values now say so. It also takes
the normalised code the caller already has rather than computing it a third
time.
refuseOnDependencyCycle sorted each manifest's Requires before walking them.
Requires is a slice and already has a fixed order, so the sort bought no
determinism - that comes from the sorted outer loop, which walks a map - and
only made a reported cycle harder to line up against the manifest that caused
it. The filtering pass that went with it is covered by the registration check
underneath. The cycle path is trimmed with slices.Index, which also removes a
fallback return that the grey/path invariant made unreachable.
An application's manifest can name others it needs. Until now the list was
stored and never read.
It is checked, not satisfied. Installing the dependencies too would make
"install this application" mean "and everything it happens to name, and
everything those name" - a blast radius the operator did not ask for and
cannot see beforehand. What they get is the list and the order to do it in.
A dependency whose own install failed, or never finished, is not a dependency
that is there. The message says which, because the two send you to different
places: one to install it, the other to look at why it did not take.
The check runs before anything is written, so a refusal cannot cost the
operator the row that told them what they had.
Separately, a cycle anywhere in the registered manifests is refused, whether
or not the application being installed is in it. A cycle between two others is
still an authoring mistake, and the day somebody installs into it - with an
error naming two applications they did not ask for - is the worse time to find
out. The error is the cycle rather than the walk that reached it, and the
walk's order is sorted, so the same set of manifests always reports the same
one. Requires naming an application that is not registered is not a cycle; it
is the database's answer to give, at the time it matters.
Six degradations turn the new assertions red: accepting any dependency,
accepting a row regardless of its status, returning no cycle, not trimming the
reported path to the cycle itself, and running the check after the row has
already been written - the last of which was rebuilt after the first attempt
at it deleted the check rather than moving it, and so went red on the wrong
assertion.
The migration rows answer "did this run". They cannot answer "is this
application installed", and the difference is not academic: an install that
stopped partway leaves every migration reading applied and a row saying the
install never finished. Until now nothing printed that row.
[order] 1.0.0 failed at order-1793800000000
applied order-1793800000000 2026-09-10 21:02:07
The application list is the union of the two sources rather than either one.
Reading it from sys_app alone would drop an application whose migrations ran
under plain `migrate`, which records no row; reading it from the migration
rows alone drops one whose code has been taken out of the binary, which is
when somebody most wants to see it named - that one now gets a group of its
own, empty, saying why.
A database from before sys_app existed prints exactly what it printed before.
`migrate status` has to keep working on a database that has not been migrated
at all, which is when it is most wanted.
Four degradations turn the new assertions red: dropping the sys_app-only
applications from the listing, printing no summary, not narrowing sys_app by
--app, and reporting an unfinished install as an installed one.
Everything under `migrate install` was covered with an injected engine and a
hand-built schema, which is where the shapes belong. What none of it could
catch is the wiring: whether an application's init() reaches both registries,
whether the installer finds a manifest through app.Snapshot, whether the
seeder writes what the uninstaller goes looking for, and whether the command
exits non-zero when a migration fails - which a deployment reads to decide
whether to start the new version.
This builds a go-admin binary with the example application linked in, migrates
a real database with it, and drives the whole sequence: framework migrations
only, install, install again, put a row in the application's own table,
uninstall, reinstall.
It lives in its own module. A tagged import in the main module would still be
resolved by `go mod tidy`, which considers every build tag and would go
looking for github.com/go-admin-team/example-app-order on the network - a
repository that does not exist, because the example is a directory inside this
one. That was checked rather than assumed: tidy fails there with "Repository
not found". A build tag of `ignore` is skipped by tidy but cannot be turned on
either, because the standard library uses it for files that are not meant to
build at all. A separate module with replace directives is invisible to the
main module's tidy, its build, its tests and checksilent, and needs no
go.work.
Three degradations turn it red: the example application not registering a
manifest, the uninstall not clearing sys_migration - where the reinstall then
seeds nothing and the assertion reads "menus = 0, want 4" - and the seeder not
recording its grants, where the uninstall then leaves every policy behind.
app-order registered its migrations and its menus and nothing else, so
`migrate install order` answered that no application in the binary registers a
manifest. Which was true, and made the installer untestable against the one
application this repository ships.
The manifest goes in the migration package rather than one of its own because
that is the package a host has to import for the application to exist at all -
its migrations register from there too. A second package would be a second
thing to remember to import, and forgetting it would leave an application
whose migrations run and which no installer can name.
Its Version is not the migration version and the two move independently:
adding a migration file without renaming the application is normal, and so is
a release that changes no schema. The migration versions decide what runs;
this decides what the installed row says.
go-admin-core moves to v2.8.0, which is where contract/app lives.
The fourth registered driver, and the one that disagrees with the other three
about NULL. A suite that never pointed at it reported success for a migration
no SQL Server database could apply - the same shape as the PostgreSQL gap that
put the postgres service here, one driver further along.
SQL Server has no equivalent of POSTGRES_DB, so the database the DSN names is
created in a step before the tests. The test helper fails rather than skips
when CI is set and the variable is not, so dropping the service or renaming
the variable cannot quietly go green.
1786700008000 could not be applied to any SQL Server database. Not an old one
with awkward data - any of them, including an empty one:
Msg 1505 ... duplicate key ... The duplicate key value is (, <NULL>, 0).
MySQL, PostgreSQL and SQLite treat two NULLs in a unique index as different
values, so any number of rows missing a seed_code coexist under
uk_sys_menu_app_seed_code_del. SQL Server treats them as equal and permits
exactly one. 1786700001000 seeds five menus and none of them has a seed_code,
so the second one already collides with the first. sys_api's index has the
same shape over two nullable columns, path and action.
On SQL Server the index is now filtered to the rows that carry a value, which
is what the other three engines do by not comparing their NULLs. The filter is
not added elsewhere: MySQL has no filtered index at all, and on PostgreSQL and
SQLite it would only restate what those engines already do.
Nothing that has applied this migration is affected, and no SQL Server
database can have.
Verified against SQL Server 2022. The migration completes; the filtered index
still rejects a second (order, dir) and still lets another app reuse "dir",
so filtering removed the NULL rows from the index rather than the index's
teeth. Two degradations turn that red: dropping the filter, and naming only
path in sys_api's - the second one needed a fixture row with a path and no
action, because rows missing both are excluded either way and the first
attempt at that degradation came out green.
There is also a control test asserting the unfiltered statement still fails on
this engine, so the first test is passing because of the fix rather than
because SQL Server turned out not to mind.
Two comments added in this branch cite docs-prd/008-.../数据库变更.md by path.
That directory is not tracked here, so the citation reads as a file the reader
can open and cannot. The reasoning it pointed at is short enough to state in
place.
Three comments from the previous batch cite the same path and are left alone;
they belong to a different change.
Copilot could not review this branch - the account is over its review quota -
so these are what a second pass over the uninstaller turned up. No defect: the
three cases were uncovered rather than wrong.
findOrphanPolicies batches its OR chain because a driver runs out of
placeholders long before an application runs out of endpoints, and nothing
exercised the boundary. 205 paths across three batches, the last one short,
plus one policy no key names as a control. Taking one fewer per batch,
advancing one too far, and stopping after the first batch each turn it red.
An application with apis and no menus, and one with menus and no apis, are
both normal - endpoints another service calls, or a section with no endpoints
of its own - and each leaves one of the two id lists the uninstall reads
empty.
That last pair also corrected a comment. The guard in front of the join-table
delete was described as being there because an empty IN list is a syntax
error. It is in raw SQL, but GORM renders IN with an empty slice as a
condition that matches nothing, and removing the guard leaves the new test
green. It stays as a statement of intent, and now says so.
`migrate uninstall <code>` removes what an application's install wrote and
leaves the application's own tables alone. Removing an order module is not the
same decision as destroying the orders, and nothing here can tell an operator
who is done with it from one who will reinstall tomorrow.
One transaction, and this one really is one: every statement is DML or a
SELECT, so unlike an install there is no DDL to commit it out from under
itself. Child rows go first, while the ids that identify them can still be
read from their parents, and the api paths are read before the rows carrying
them are deleted.
The two join tables need no ledger and get none. menu_id is a surrogate key,
so a sys_role_menu or sys_menu_api_rule row can only have come from a menu
this application wrote - there is no "looks like it but is not". A column on
sys_role_menu would have been worse than unnecessary: SysRole.Update deletes a
role's rows and writes them back through GORM's many2many, which does not
carry extra columns, so the column would be blanked the first time anybody
edited a role, silently. There is a test that performs that edit and then
uninstalls.
casbin_rule is the opposite case, because its key is business text somebody
else may have written for their own reasons. Policies are removed one at a
time, by exact tuple, and only the ones the ledger says this install created.
A tuple the ledger names that is no longer there is reported, not treated as a
failure - the uninstall wanted it gone and it is gone. Then, with everything
the ledger could speak for already dealt with, a read-only pass lists the
policies still naming this application's paths: those are grants somebody made
by hand, they are about to point at APIs that no longer exist, and they are
not this command's to delete. The two lists stay separate because they mean
different things - one is something of ours that had already gone, the other
is somebody else's grant now pointing at nothing - and merged into one "could
not remove" list neither would be actionable.
sys_migration's rows for the application go too. Without that a reinstall
finds every version applied, runs no migration, seeds nothing, and reports
success. It is the easiest step to leave out, because a migration record does
not look like the application's data.
A sys_app row is not required. `migrate` with no subcommand applies every
registered migration, an application's included, so an application can have
all of its rows without ever having gone through the installer - and that is
the case where nothing else can clean up after it.
Eleven degradations were applied one at a time, each red on the assertion it
was aimed at: skipping either join table, deleting sys_role_menu without its
filter, skipping sys_migration, deleting sys_migration without its filter,
matching policies by path instead of by ledger tuple, dropping the orphan
pass, treating a missing policy as a failure, leaving the ledger behind, soft
deleting sys_menu instead of removing it, and running the whole thing outside
a transaction.
uk_sys_menu_app_seed_code_del covers (app_code, seed_code, deleted_at) and is
created by 1786700008000 with explicit SQL. The struct tag named the same
index on SeedCode alone, and a named uniqueIndex tag collects only the fields
carrying that name - so AutoMigrate on this model would build a unique index
on seed_code by itself: stricter than the real one, and forbidding two
applications from both having a "dir" node, which the composite key exists to
allow.
Worse than being stricter, it would win. The migration only creates its index
when HasIndex says the name is free, so a schema built by AutoMigrate first
keeps the wrong index and the migration steps over it without a word.
The tag cannot express the real index: deleted_at comes from the ModelTime
embed shared by every table, which no single model can add a tag to. So the
tag goes and the migration is the only thing that creates it.
No database is affected. The initial table migration AutoMigrates a frozen
snapshot of this model that has neither app_code nor seed_code, and nothing
else in the repository AutoMigrates the live one - which is why this stayed
invisible until a test built the schema from the live model and seeded two
applications, and got a unique-constraint failure on a seed code they are
supposed to be able to share.
sys_app_casbin_grant has existed since the registry tables were added and
nothing ever wrote to it. An uninstaller reading it would have found it empty,
deleted no policy at all, and reported every one of them as an unattributable
leftover - which is what "report and skip" looks like when the ledger was
simply never written, and is indistinguishable from it working.
grantToAdminRole now writes an entry for each policy it creates. The entry
carries the tuple casbin_rule is unique on rather than a foreign key into it,
because casbin_rule is not this project's table: the gorm adapter's SavePolicy
truncates it and writes it back from memory, and SysRole.Update replaces a
role's policy rows wholesale. Both rebuild the same tuple from the same
sys_menu/sys_api data, so a match on the tuple survives what a row id does
not.
Only policies this install actually created are recorded - the insert is
conditional and its RowsAffected says which. A policy that was already there
was granted by somebody else and is not this app's to take away.
The two ways that can be wrong are not equally bad, which is what settles it.
Under-recording leaves a policy behind and the uninstall says so, because a
policy naming this app's own path with no ledger entry is exactly what it
reports as an orphan. Over-recording deletes somebody's authorization,
silently. Between a visible leftover and an invisible deletion, take the
leftover.
The ledger insert is itself conditional, for a case the obvious retry test
does not reach: on a plain re-run the policy still exists, so the insert is
skipped before the ledger is touched. It is reached when the policy row was
removed while its entry stayed, and a plain insert would then abort the whole
seed on the ledger's unique index. There is a test for that specific shape,
and replacing the insert with a plain one turns it red - which the plain retry
test does not.
Ordering: the ledger table is created by a framework migration, and version
strings sort bare digits ahead of any app-prefixed one, so it exists before
any application's seed runs. Nothing in the framework's own migrations calls
SeedMenus.
zhangwenjian