Both date-pickers -- the search filter and the insert/edit form -- used
value-format="YYYY-MM-DD HH:mm:ss", which formats a picked instant as
e.g. "2026-09-19 12:30:00": no T separator, no offset. dto.go.template
declares every datetime column's InsertReq/UpdateReq field as
time.Time with a plain `json:"..."` tag (R6 leaves that file alone, so
there is no time_format tag to reach for instead), and encoding/json's
default (Un)MarshalJSON for time.Time only accepts RFC3339. The
generated form would submit new and edited datetime values in a shape
Go's JSON decoder cannot parse -- a runtime failure on every create and
update, with nothing in `pnpm type-check` or `pnpm lint` positioned to
see it: both check the request is well-typed TypeScript, not that the
string it produces is a string Go can read.
Changed both to value-format="YYYY-MM-DDTHH:mm:ssZ" -- dayjs's Z token
renders the picker's own local offset, which is what a zero-nanosecond
time.Time (anything without a database column storing sub-second
precision) round-trips to on either side of the wire; confirmed
separately against Go's actual json.Marshal/Unmarshal, not assumed from
the RFC.
The search filter needed the same fix, not just the form: GetPageReq
binds a `time.Time` query field via `form:"..."` (dto.go.template),
and gin's own default for an untagged time.Time binding is also
RFC3339 -- the same failure mode on the query side, one call the
report didn't name but the same root cause reaches.
This is a runtime behaviour change no compiler catches, so it was
verified as one: a Go program exercising encoding/json directly (not
assumed from reading the RFC) confirmed a zero-nanosecond time.Time
marshals to plain RFC3339 with no fractional seconds, and unmarshals
correctly from both a numeric offset and a literal Z. Separately, a
Node script loaded go-admin-ui's own installed dayjs 1.11.21 with the
customParseFormat plugin -- the same plugin element-plus's date-picker
extends dayjs with -- and called the same parseDate path date-picker
panel.mjs uses (time-picker/src/utils.ts, no strict flag passed, so
lenient parsing): formatting with this value-format produced a valid
submission string, and parsing either an offset or a literal-Z string
back with the same format produced a valid, correctly-valued date --
covering create, edit prefill, and update in the two directions that
matter (browser to Go, Go to browser) without needing a live backend.
A column configured with both FkTableName and DictType -- and reaching
a branch of each, e.g. required + IsQuery=1 + IsInsert=1 with
HtmlType=radio -- had both blocks declare `const {JsonField}Options`:
the dict branch from useDict, the FK branch from ref(). TS2451,
Cannot redeclare block-scoped variable, and the page does not compile.
This is reachable precisely because FkTableName and DictType do not
exclude each other consistently: search, list and the form's select
branch check FK first and fall back to dict, but the form's radio
branch never looks at FK at all -- it was already established (the
$dictUsed/$fkUsed audit two commits back) that a radio column's dict
options are used regardless of whatever FkTableName says. A column
that is both radio and query-or-insert-select can legitimately need
both sources at once, under one shared name.
Renamed to {JsonField}DictOptions and {JsonField}FkOptions and updated
every consuming branch to the name that matches what it was already
branching on: FK branches (search select, form select, the list
column's Label function) read FkOptions; dict branches (search select,
form select, form radio, the list column's dictLabel call) read
DictOptions. Mechanical rename, no new conditions -- each site already
knew which source it wanted from its own if/else-if.
Verified with team-lead's exact repro (FkTableName + DictType +
IsQuery=1 + IsInsert=1 + HtmlType=radio) rendered through the real
template.Execute and checked against a throwaway go-admin-ui worktree
(deleted afterwards): TS2451 fired twice before this change, zero
after -- and the rendered file confirms the search select actually
reads kindFkOptions (FK wins search's priority) while the insert radio
reads kindDictOptions (radio never checks FK), so both sources are
live, not just declared. Re-ran every fixture from every previous
round alongside it; all stayed green. pnpm type-check and pnpm lint
both zero error, on Node 24.11.0.
Two columns pointing at the same foreign table -- owner and approver
both selecting from the same users table, say -- each triggered their
own `import { listX } from ...` / `import type { X } from ...` line,
which is a duplicate ES module import once both fire: TS2300. Nothing
here ever asked whether a target table had already been imported by an
earlier column, because nothing tracked target tables at all -- only
source columns, and "one FK-configured column" was never the same
thing as "one distinct target table".
The same import was also gated on the column having FkTableName set,
not on $fkUsed -- the condition the previous fix already applies to the
const declarations that read the import. A column carrying FK metadata
but reaching no query, list or insert-select branch imported a module
nothing in the file references.
text/template has no set to check membership in, so the dedup is a
nested range: a column only imports its target if no earlier,
equally-used column already claimed the same FkTableNameClass. $fkUsed
is recomputed for both the outer and the inner column rather than
factored out, since text/template has no way to carry a per-column
value computed in one range into a second, later range over the same
data.
Verified with a fixture carrying three columns pointing at the same
target table -- one read only from search, one only from the list, one
from neither -- rendered through the real template.Execute and checked
against a throwaway go-admin-ui worktree (deleted afterwards): before
this change, TS2300 fired six times (the function and the type, three
times over); after, exactly one import of each, and the unused third
column contributes neither. Re-ran the previous rounds' fixtures
alongside it; all stayed green. pnpm type-check and pnpm lint both zero
error, on Node 24.11.0.
Every $hasX flag controlling an optional import matched "this column
carries the metadata" rather than "some rendered branch actually reads
it" -- necessary but not sufficient, since FkTableName/DictType lose to
each other by priority (FK wins search, list and the form's select
branch; the form's radio branch never checks FK at all) and a column
can carry either while being neither queryable, listed, nor an
insertable select/radio.
$hasDatetime was the reachable case integration testing found:
sys_tables.go assigns HtmlType "datetime" to any timestamp/datetime
column on import regardless of IsList, because GetList's audit-column
exclusion is a separate, later step editTable.vue never surfaces
created_at/updated_at through anyway. Nearly every real table has both,
so nearly every table imported DateCell without using it.
$hasFk and $hasDict had the identical shape one level down: the
per-column ref/onMounted/useDict declarations were gated on "this
column has FkTableName/DictType", not on whether the column reaches a
branch that reads the resulting Options ref -- an FK column used only
via search (no IsList, no insert-select) still declared a Label
function nothing calls, and a dict column used only in an insert radio
(no IsQuery, no IsList) still would have, had the two flags controlling
its import stayed as wide as the per-column check they were meant to
gate.
Rewrote both to a shared $dictUsed/$fkUsed condition, matching each
consuming branch's own guard term for term, and split the FK block's
Label function under its own IsList check -- Options can be needed for
search or the form's select without List ever being true. $hasDictList
already had this shape from the previous fix and needed no change.
Verified with two new fixtures, rendered through the real
template.Execute and checked against a throwaway go-admin-ui worktree
(deleted afterwards) with hand-written API-module stubs: a bare table
carrying only the standard created_at/updated_at pair -- confirmed red
on DateCell before this change, green after -- and a table exercising
every optional import through a path distinct from the ones the two
earlier verification rounds covered (a dict column read only from an
insert radio, an FK column read only from search, and a business
datetime column that IS listed, so DateCell still has to import when
the real thing needs it). Re-ran the three fixtures from the previous
two rounds alongside these two; all five stayed green. pnpm type-check
and pnpm lint both zero error, on Node 24.11.0.
The rules block joined entries with a $first-flag comma the same way
defaultQuery and defaultModel do, but on its own line rather than all
on one -- so the comma for every entry but the first sat at the start
of its line instead of the end of the one before it. @stylistic/comma-style
requires the opposite, and pnpm lint fails on any table with two or
more required insert fields (one comma is enough to trip it; a table
with 0-1 required fields never renders a second entry to get it wrong).
Reproduced first: rendering a four-required-field fixture reported
three comma-style errors, matching what integration testing found on
qa010_widget and qa010_natural.
Fixed by moving the separator so it trails the previous entry instead
of leading the next one: the else branch now emits ",\n " -- comma,
then the newline and indent -- rather than "\n , ", so the comma lands
on the line that already has content instead of opening a fresh one.
Also fixed while reproducing, found by the same fixture: dictLabel was
imported whenever any column had a DictType, but it is only called from
the list column's dictionary branch. A table using a dictionary solely
in its insert form (no such column in the list) imported dictLabel and
never called it, tripping no-unused-vars. $hasDict now only gates
useDict; a new $hasDictList gates dictLabel specifically.
Verified against three fixtures via a throwaway go-admin-ui worktree
(deleted afterwards) with hand-written API-module stubs standing in for
F5: the regression fixture (four required fields, confirmed red before
the fix, green after), and the two fixtures from the original F4
verification round (full branch coverage, and the all-flags-off edge
case), all of which stayed green. pnpm type-check and pnpm lint both
ran clean with zero errors, on Node 24.11.0 (this machine's default
node is 20.19.0; the project's engines field wants >=22).
template/v4/vue.go.template produced Vue 2 syntax -- slot-scope, .sync,
.native -- all removed outright in Vue 3, so every generated page
failed to render (PRD 010 G1). Rewritten from scratch to match
go-admin-ui's reference page (src/views/demo/product/index.vue):
PageContainer + ProTable + useTable/useForm/useRemove,
<script setup lang="ts">.
Four constraints from the phase-3 review, all load-bearing:
- An HtmlType this template doesn't recognise (the old "file" branch,
or anything future work adds) renders as a plain input rather than
nothing. sys_tables.go still assigns "datetime" automatically on
import, and a row can carry "file" from before F7 disabled it in the
UI, so "unknown" is a real, reachable state, not a hypothetical one.
- Every column width is min-width, never width -- a rigid width
repeats G5's overflow bug on any table with enough columns. colWidth
(F1) backs it when set, a flat per-column-kind number otherwise.
- Not one Chinese character anywhere in the output, comments included:
D10's CJK scan is a bare regex with no exception for "it's only a
comment". Every label goes through
$t('gen.{PackageName}.{BusinessName}.{JsonField}'), read from the
language pack F9 wired into src/lang/{locale}/gen/.
- defineOptions carries the ClassName + "Manage" suffix gen.go writes
into sys_menu.menu_name (R4) -- the old template's bare ClassName
was a latent mismatch masked by loadView()'s runtime rewrite (G7).
Verified by rendering two fixtures through the real template.Execute --
one exercising every optional import (dictionary, foreign key,
datetime, required-field rules, search filters) and one with none of
them, a string primary key, and a select/radio with neither a
dictionary nor a foreign key configured, plus an html_type this enum
has never had. Both scan clean for CJK, and both pass pnpm type-check
and pnpm lint with zero errors against a throwaway go-admin-ui
worktree (deleted afterwards) seeded with hand-written API-module
stubs standing in for F5, which lands in template/v4/js.go.template
separately.
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.
`migrate install <code>` brings one application up to the version its manifest
declares: it runs that application's outstanding migrations and records what
it did in sys_app.
It goes under migrate rather than under the existing `app` command, which
already means "generate the skeleton of a new application" - a directory that
does not exist yet, not an application already compiled into this binary.
Installing one is running its migrations, which is what this command is, so
--domain, resolveDB and the guard that refuses a mistyped code instead of
reporting a successful no-op are all already here.
Three phases, each committing on its own, and they are not one transaction.
An application's versions are separate migration files, and on MySQL a DDL
statement commits the transaction around it - destroying an outer transaction
and every savepoint taken from it. So this does not promise that a
half-installed application cannot happen. It promises one is visible when it
does: phase A writes "installing" before anything that can fail, phase B runs
the migrations, phase C turns that into "installed" or into "failed" with the
version it stopped on.
What is left to apply comes from sys_migration, never from sys_app. sys_app
is a derived view - a summary, and the answer to "which version does this app
think it is at". If it were the authority, an operator who deleted
sys_migration rows by hand would be told an application is installed while its
schema is not, which is worse than not knowing. So "already installed, nothing
to do" needs all three: nothing outstanding, recorded as installed, and the
same version. A row stuck at "installing" - what it reads as after the process
was killed partway - is not installed, and retrying is just running the
command again.
An upgrade is in place and keeps the first install's time; a downgrade is
refused, and refused before phase A writes anything, so a refusal cannot cost
the operator the row that told them what they had. An unparseable recorded
version is refused the same way, while it is still readable.
The report ends by saying the code is not running yet. That is not a
pleasantry: Go links at build time and Vite resolves its import globs at build
time, so installing an application writes its menus, its APIs and its
permissions and cannot make one line of its code run - and the menus appearing
is exactly what makes an operator believe otherwise.
Ten degradations were applied one at a time to check the tests name the
behaviour rather than the shape: deciding the no-op from sys_app alone,
always writing installed_at, allowing the downgrade, keeping the previous
attempt's diagnostics on a row that now says installed, treating "installing"
as installed, truncating last_error by bytes so a Chinese message is cut
mid-rune, not recording the failure at all, skipping code normalization, and
writing phase A before either the downgrade or the version-parse refusal.
Each went red on the assertion it was aimed at. An eleventh was discarded
rather than counted: it failed in the first install's setup, not on the claim.
run() called log.Fatalf on the first migration that failed, which ended the
process from inside the migration engine. Nothing above it could record what
happened - an installer needs to write down which version an attempt stopped
on - and no test could exercise a failing migration at all without taking the
test binary with it, which is why the one test that covers a failed migration
drove the registered function directly and left the scheduler uncovered.
run(), Migrate() and MigrateApp() now return an error, and the exit moved to
the command layer where the exit code is the command's business.
Two of those errors say more than "it failed". A migration that fails comes
back as a *VersionFailure naming the version, because an installer records
that as a diagnostic snapshot - the authoritative answer to where a retry
resumes is always recomputed from sys_migration, never read back, and asking
the database what is still pending answers a different question that merely
has the same answer most of the time. An app code nothing registered under is
now an error rather than a log line, so an installer asking for one app by
name cannot be told that installing an app that does not exist succeeded; the
command layer still rejects a typo before any database work.
exitOnError is what makes the command exit non-zero, and it covers more than
it replaces. Every path out of migrateModel used to return without an exit
code: an unreachable tenant database or a failed AutoMigrate printed a line
and exited 0, so a caller that migrates before starting a server - the deploy
workflow does exactly that - carried on onto a schema that had not been
brought forward. A failing migration function was the only failure reported,
and only as a side effect of the log.Fatalf this commit removes.
Each of these was checked by degrading it and watching the named assertion
go red: returning nil instead of the failure, naming the first version rather
than the one that failed, accepting an unregistered app code, and not exiting.
One gap is left open deliberately. Go allows a call whose only result is an
error to stand as a statement, so `migration.Migrate.Migrate()` still compiles
while dropping what it returns - `go build` passed while migrateModel was
doing exactly that during this change. Both call sites now return the value,
which the compiler does check, but nothing guards against the statement form
coming back. A checksilent rule was considered and dropped: that tool parses
without type information, so it could only match the method name, and a guard
that fires on any type with a Migrate method is noise.
Cleaning up after a successful deployment never runs on the host that needs it.
The pull is the first thing in this script that needs space and it is where a
full disk stops it, so the run ends before reaching any cleanup - and so does
the next run, and the one after that. That is not hypothetical: a deployment
failed on the pull with no space left on the device, and rerunning the workflow
unchanged failed at the same place. The disk had to be cleared by hand before a
deployment could go through.
The pipeline is now a function called twice, before the pull and after the
health check, so the window is bounded on both sides.
Verified against a real docker daemon, in the function form rather than the
inlined one: with five images newer than the running one, so position alone no
longer protects it, it leaves three and does not select the live one; removing
the id exclusion from the same function does select it. With NAME pointing at a
container that does not exist it selects nothing - as it also does without the
explicit guard, which is there because grep -v on an empty id reads like the
opposite of what it does, not because it changes the outcome.
zhangwenjian