Commit Graph
1727 Commits
Author SHA1 Message Date
zhangwenjian 2a900c9876 fix🐛: drop the stray token from the Cache-Control header
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.
2026-09-18 14:47:28 +08:00
wenjianzhang 0008b943a3 Merge pull request #937 from Tuoxie423/test/header-middleware
test: add unit tests for NoCache/Options/Secure middleware
2026-09-18 14:41:31 +08:00
拖鞋423 50c74b1f96 test: add unit tests for NoCache/Options/Secure middleware 2026-09-17 23:41:21 +08:00
wenjianzhang ae1eef6d4f Merge pull request #936 from go-admin-team/fix/gen-import-accepts-json-body
fix🐛: accept the import table list from a JSON body as well
2026-09-16 20:53:03 +08:00
zhangwenjian d01cdc040f fix🐛: accept the import table list from a JSON body as well
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.
2026-09-16 17:57:51 +08:00
zhangwenjian 92b9af17b7 refactor🎨: name the empty-table-name message once
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.
2026-09-16 17:57:37 +08:00
zhangwenjian 898e1b023a refactor🎨: share the generator tests' engine and response decoding
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.
2026-09-16 17:57:28 +08:00
wenjianzhang a90c67473e Merge pull request #924 from jackwalkerlabs/fix/job-stop-timeout-890
fix🐛: report job stop timeouts as errors
2026-09-16 08:02:47 +08:00
wenjianzhang 1a84b8a892 Merge pull request #930 from Tuoxie423/docs/remove-dead-contributor-links
docs📝: 删除 README 中失效的贡献者链接
2026-09-16 08:02:43 +08:00
wenjianzhang 656d14cd54 Merge pull request #934 from go-admin-team/fix/928-929-seed-repair
Reseeding a menu tree repairs what it finds, and finds what it wrote
2026-09-14 14:52:33 +08:00
zhangwenjian 9dd271ecab fix🐛: claim the rows an application wrote before seed_code existed
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.
2026-09-13 20:34:04 +08:00
zhangwenjian 4973ee030d fix🐛: bring an existing seeded menu up to what the spec says
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.
2026-09-13 20:33:47 +08:00
wenjianzhang 137bb3ad33 Merge pull request #931 from go-admin-team/feat/008-wire-example
008 layer three: status reads sys_app, dependencies are checked, and the example gets installed for real
2026-09-12 21:17:33 +08:00
zhangwenjian 03d587db6a ci👷: give the end-to-end install a make target
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.
2026-09-11 08:23:42 +08:00
zhangwenjian 1f56b956d2 test: build the end-to-end binary once, and assert from one list
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.
2026-09-11 08:21:36 +08:00
zhangwenjian 37aece9791 test: share one sys_app fixture, and count through the helper that checks
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.
2026-09-11 08:21:35 +08:00
zhangwenjian 309b400bc0 refactor♻️: take one registry snapshot, and one route into sys_app
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.
2026-09-11 08:20:56 +08:00
zhangwenjian 1b9868b72b feat: refuse an install whose dependencies are not installed
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.
2026-09-10 21:30:29 +08:00
zhangwenjian 25344aa572 feat: let migrate status say what sys_app knows
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.
2026-09-10 21:30:03 +08:00
拖鞋423 ea9d27cf6d docs📝: 删除 README 中失效的贡献者链接 2026-09-09 20:28:35 +08:00
zhangwenjian 0bee8ec46c ci👷: run the end-to-end install on every push
The separate module is invisible to `go test ./...` in the main module, which
is the point, and also means nothing would ever run it. This step does.
2026-09-09 16:40:15 +08:00
zhangwenjian 3a5afeb518 test: install and uninstall the example application end to end
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.
2026-09-09 16:40:15 +08:00
zhangwenjian fc8ba4d615 feat: let the example application say what it is
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.
2026-09-09 16:40:14 +08:00
wenjianzhang dc20062e5b Merge pull request #927 from go-admin-team/fix/sqlserver-null-unique
The seed natural-key indexes cannot be built on SQL Server
2026-09-09 16:19:26 +08:00
zhangwenjian ff430c509b ci👷: run the migration tests against SQL Server too
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.
2026-09-09 13:49:20 +08:00
zhangwenjian d43d7a46dd fix🐛: make the seed natural-key indexes buildable on SQL Server
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.
2026-09-09 13:49:20 +08:00
wenjianzhang 72c496ab93 Merge pull request #926 from go-admin-team/feat/008-installer
008 layer two: the installer and the uninstaller
2026-09-09 13:43:08 +08:00
zhangwenjian b228152308 docs📝: drop a reference to a document this repository does not carry
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.
2026-09-09 13:28:45 +08:00
zhangwenjian 006756ea40 test: cover the uninstall's chunk boundary and its empty id lists
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.
2026-09-09 12:56:33 +08:00
zhangwenjian aa539c061f feat: uninstall one application's menus, apis and grants
`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.
2026-09-09 12:35:35 +08:00
zhangwenjian 74b0ee8776 fix🐛: stop sys_menu declaring an index stricter than the real one
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.
2026-09-09 12:35:17 +08:00
zhangwenjian 412413c12f feat: record which casbin policies an install created
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.
2026-09-09 12:27:38 +08:00
zhangwenjian 35d213f339 feat: install one application from its manifest
`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.
2026-09-09 12:22:16 +08:00
zhangwenjian 9c68bc25a5 refactor♻️: report a failed migration instead of ending the process
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.
2026-09-09 12:14:12 +08:00
wenjianzhang 9c5d9d16a7 Merge pull request #925 from go-admin-team/fix/deploy-image-bloat
Stop the demo host filling up with old images
2026-09-09 10:54:08 +08:00
zhangwenjian 46c10f999a ci👷: reclaim before the pull, not only after a healthy deploy
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.
2026-09-09 08:11:26 +08:00
zhangwenjian d12f40c9a0 ci👷: remove this repository's old images after a healthy deploy
Every deployment pulls an image tagged with its commit and nothing removed the
previous one, so they only accumulated. 68 had built up when a deployment failed
on a pull with no space left on the device. That is the harmless place to fail -
the site kept serving the image it already had - but no later run would have
recovered on its own.

Three are kept so a release can be re-run by tag by hand. Only this repository's
images are listed, because the host runs other services. The image the new
container is on is excluded by id rather than by position, and rmi is called
without -f so an image a container still holds is refused rather than taken from
it.

Verified against a real docker daemon: with five images newer than the running
one, so position alone no longer protects it, the pipeline leaves three and does
not select the live one. Removing the id exclusion from the same pipeline does
select it, so that guard is load-bearing rather than decorative.
2026-09-09 07:45:51 +08:00
zhangwenjian cd363fce3d perf👌: stop shipping a C toolchain in the runtime image
gcc and g++ were 273MB of a 381MB image, and nothing in the container ever
invoked them: the binary is compiled and statically linked before the image is
built and arrives as a COPY, and Go is not installed here either.

The layer cost more than its size. apk resolves against an index that moves, so
its digest differed on every build and no two images shared it - a host that
keeps one image per deployed commit paid the full 273MB each time rather than
storing it once.

libc6-compat is kept although nothing measured needs it: a container built
without it resolves a hostname and opens a database connection exactly as one
built with it, but it costs half a megabyte and covers a ./main that was linked
dynamically, which this Dockerfile cannot check.

Verified by building this Dockerfile and running the result under the check the
deploy script uses - captcha answering 200 and the log reporting the datastore
connected. A control built from the current recipe passes the same check and
carries a 273MB apk layer this one does not; a third build with a deliberately
truncated binary fails the check, so it distinguishes a serving process from a
dead one.
2026-09-09 07:45:40 +08:00
Jack Walker 709cebd4a7 fix🐛: return an error when stopping a job times out
Fixes #890
2026-09-08 19:26:49 -04:00
wenjianzhang ba5ef9f79c Merge pull request #923 from go-admin-team/feat/008-host-schema
008: the application registry, its natural keys, and an idempotent seed
2026-09-08 20:35:28 +08:00
zhangwenjian 2c50317a98 fix🐛: stop the duplicate check refusing what the index would accept
The check grouped by (app_code, path, action) and refused whatever
appeared more than once. GROUP BY treats two NULLs as the same value; a
unique index treats them as different ones and allows both. So a
database holding rows with a null path or action was refused for
duplicates the index it was blocking would have accepted - and the
migration stopped, on a database with nothing wrong with it.

Both columns are nullable: neither carries a not-null tag, so gorm built
them that way. Measured on MySQL 8.0, PostgreSQL 15 and SQLite: two rows
with both columns null are one group to GROUP BY, and the unique index
builds over them without complaint. Standard SQL, not a dialect quirk.

They are now excluded from the check rather than grouped. Each column
needs its own exclusion and has its own test: one null column is enough
to make the index accept the pair, so removing either condition alone
lets that half through - which is what the two subtests are for, and
each fails only for its own half.

The message could not name the rows either. MySQL's CONCAT returns NULL
when any argument is, and scanning that into a string fails with
"converting NULL to string is unsupported" - so the check reported a
driver error instead of the duplicates it exists to report. SQLite and
PostgreSQL treat a null argument as empty and say nothing, which is why
this never surfaced in the tests: they run on SQLite, and this
repository has no MySQL in CI. The comment says so, so that a
postgres-only test file is not mistaken for cover.

No COALESCE was added to paper over that. It would have had nothing left
to guard once the nulls are excluded, and it would make a future
regression quieter: someone dropping the exclusions would get a report
naming rows that are not duplicates, which reads as a real answer,
rather than a scan error that reads as a broken query.

Leaving path and action nullable is deliberate. Tightening them is a
migration of its own - existing null rows have to be given values, and
what those values should be belongs to whoever owns the data, not to a
migration whose job is adding an index.
2026-09-08 20:17:50 +08:00
zhangwenjian 28350a15bb fix🐛: repair the row a retry reuses instead of walking past it
The idempotency check added in the previous commit stopped a retry
inserting a second copy, and introduced a quieter failure in its place:
a retry that found an existing sys_menu row skipped everything after the
insert. Those are the sys_menu_api_rule bindings and the materialized
path, and neither is written by the statement that writes the menu -
paths is a separate UPDATE, and on MySQL an earlier DDL has already
committed the transaction that was supposed to hold them together.

So an install interrupted between those steps left a menu that exists,
sits outside the tree with an empty path, and is bound to no API. What
core's contract.md says about such a menu is that it is invisible to
every role and its apis are authorized for no one - while the installer
reports success.

Reusing now repairs. paths is compared before it is written, so a row
that is already right is not touched. Bindings are inserted with WHERE
NOT EXISTS rather than deleted and rebuilt: an administrator can bind an
api to a menu from the menu screen, and delete-then-rebuild would take
that with it on the next retry - the same accident as sys_role.go's
Association.Delete, pointing the other way.

Confirmed as a defect before it was fixed, by building the half-written
state and watching the assertions fail:

    dir.Paths = "", want "/0/1"
    binding count for list = 0, want 1

Four paths through the repair, each with a degradation that reds its own
test and leaves the others green: missing bindings only, missing paths
only, both, and neither. The fourth asserts no UPDATE is issued for a
row already correct.

A fifth covers what the repair must not do. Rebuilding bindings instead
of inserting them leaves every other test green while silently deleting
a binding this code did not create; that one now fails with "a retry
silently deleted a binding it does not own".

Bindings an older version of a manifest created and a newer one no
longer lists are left alone. Removing them is a delete, and a delete
needs the same certainty about ownership that uninstall does - this
function cannot tell a stale binding from one somebody added by hand.
2026-09-08 19:51:22 +08:00
zhangwenjian c6d3ea5f81 fix🐛: stop a retried seed from inserting a second copy
seedApis and seedMenuTree were bare tx.Create calls. A migration that
failed partway and was run again re-inserted everything it had already
written - which is not hypothetical: the demo site collected eighteen
duplicate sys_menu rows this way, and three duplicate menus were visible
in its sidebar.

Both now look for a live row already holding the natural key and reuse
it. Only live rows count: a row an earlier soft-delete retired does not
stand in the way of a fresh insert under the same key, which is also
what the unique indexes allow.

The app_code half of each key has a test of its own. Without it the
lookups still passed every existing test while quietly letting one
application adopt another's rows - and an uninstall would then delete
rows the other application believed were its own, on both sides without
an error. Removing app_code from either lookup now fails with
"has 1 row(s) ... want 2 - one per app".
2026-09-08 19:26:42 +08:00
zhangwenjian 7fadb4b585 feat: give the seeded rows a natural key to be found by
Seeding needs something to look for before it inserts, or a retry writes
a second copy of everything it already wrote. sys_api already had one in
(app_code, path, action). sys_menu had nothing usable: menu_name is
pascalCase(appCode) + pascalCase(code), which is not injective -
"list-all", "listAll" and "list_all" all become "ListAll" - so the
original code cannot be recovered from it. Hence a new column.

seed_code is nullable, against this repository's habit of NOT NULL
DEFAULT '' for a new column, and deliberately. Every row that predates
it has no meaningful value, and under a unique index an empty string
collides with every other empty string while NULL collides with nothing.
The convention exists because deleted_at's nullability broke a unique
index; here nullability is what makes one possible.

The unique index on sys_api cannot simply be created: a live database is
known to hold historical duplicates - the demo site had eighteen. The
migration looks first and refuses while naming the offending rows,
rather than letting CREATE UNIQUE INDEX fail with a constraint error
that names none. Same shape as 1786700003000's refuseOnDuplicates.

Run against SQLite, MySQL 8.0 and PostgreSQL 15, including the CONCAT
duplicate check, which had only ever been executed by SQLite's driver.
2026-09-08 19:26:42 +08:00
zhangwenjian 691df82016 feat: add the sys_app registry and the casbin grant ledger
sys_app is one row per installed application. It is physically deleted
on uninstall rather than following the millisecond soft-delete marker
the other sys_ tables use: an installed-app registry has no "deleted by
accident, needs recovering" case, and a physical delete is what lets the
same code be installed again afterwards.

status is installing/installed/failed rather than a boolean, because an
install spanning several migration files is not atomic on MySQL - DDL
commits implicitly, so a run can stop in the middle. failed_version and
last_error are diagnostic snapshots for a person to read; nothing may
decide anything from them, and the field comments say so. Where to
resume is answered by sys_migration, which cannot drift from what was
actually applied.

sys_app_casbin_grant records which casbin_rule rows an install created,
keyed by casbin_rule's own natural key. That table is not extended
instead: gorm-adapter's SavePolicy truncates and reloads it from an
in-memory model, which would drop any column added here without a word.

Built against SQLite, MySQL 8.0 and PostgreSQL 15.
2026-09-08 19:26:40 +08:00
zhangwenjian ea348fa9d1 build📦: pin go-admin-core v2.8.0
v2.8.0 adds sdk/contract/app - an application's manifest, and the one
comparator for its version - which the installer in this batch is built
on. Nothing here uses it yet; this is the dependency arriving.

Checked that the release is consumable rather than only tagged: a
program built against the published module registers a manifest, reads
it back, and gets -1 from Compare("1.9.0", "1.10.0"), which is the
multi-digit case a string comparison would order backwards.

25 packages pass and checksilent is clean on the new version.
2026-09-08 18:35:13 +08:00
wenjianzhang 925c6772a6 Merge pull request #921 from go-admin-team/fix/920-schema-readiness
Fail readiness while the database is behind the migrations
2026-09-08 17:18:34 +08:00
wenjianzhang 0c60e44aee Merge pull request #922 from go-admin-team/fix/919-drop-index-postgres
Drop the index with SQL this dialect can parse
2026-09-08 17:18:08 +08:00
zhangwenjian a716086295 test: fail these tests when the database does not answer
Every Create and Raw dropped its error. Most of them would have failed
an assertion further down anyway, with a message describing the wrong
problem - but one of them would not.

The last count in the index test reads how many indexes survived and
expects zero. An unchecked query that fails leaves the variable at zero,
and zero is what success looks like: a test that cannot reach the
database reports that the indexes were dropped.

Demonstrated rather than assumed, by breaking that one query both ways:

    with the check:    FAIL  counting the indexes after: relation
                             "pg_indexes_nope" does not exist
    without it:        PASS

Raised by Copilot on #922, as a consistency point with the SQLite tests
in this package. It is that as well, but the reason it is worth doing is
the row above.
2026-09-08 17:04:31 +08:00
zhangwenjian f8a5066a40 test: run the migration against PostgreSQL in CI
The rest of this package's tests run on SQLite, where dropping an index
through the migrator works. That is why a migration which failed on
every PostgreSQL database it was pointed at had a green suite: the
defect cannot occur on the backend being tested.

A test alone would not have helped either - without a service it skips,
and a test that never runs is the same as no test. So the workflow gains
a postgres service and the DSN, and the helper refuses to skip when CI
is set: a workflow that drops the service or renames the variable fails
rather than going quietly green, which is the shape of the original
defect.

Counter-proved by putting Migrator().DropIndex back, which reproduces
the statement verbatim:

    DROP INDEX CURRENT_SCHEMA()."idx_sd_pg_user_deleted_at"
    ERROR: syntax error at or near "CURRENT_SCHEMA" (SQLSTATE 42601)

The conversion test also checks the timestamp survives as a marker,
since a conversion that dropped it would bring deleted rows back live
while still passing a column-type assertion.
2026-09-08 16:57:17 +08:00