The note told the reader the scheduler stood in the way of raising the
replica count. It no longer does: one enabled job fires once however many
pods there are, a pod that loses the lease stops scheduling, and one that
exits hands the lease back rather than making its successor wait it out.
The shared log volume still does stand in the way, and that is now the only
thing the note asks for before the number goes up.
A second instance pointed at the same database did not divide the work, it
overwrote it. Every instance registered the whole enabled list into its own
cron.Cron, and startup ran `UPDATE sys_job SET entry_id = 0 WHERE entry_id
> 0` across the entire table, so the newest process erased the ids the
previous one wrote and put its own over the top. Neither symptom logged
anything: an enabled job fired once per instance, and stopping one from the
UI removed an entry from the wrong process and answered 200 either way.
A supervisor per tenant now keeps the scheduler in step with the lease.
Holding it is not decided once at startup, because both of the other
answers are wrong for longer than a moment:
- an instance that never got the lease keeps asking, so the death of the
holder does not stop the jobs until somebody restarts a process by hand;
- an instance that holds it stops scheduling as soon as its lease has
lapsed, because a holder still scheduling after the lease has gone
elsewhere is the two-schedulers defect reached from the other side.
A failed renewal is not a lost lease. The scheduler keeps running until the
lease could actually have expired: a database that is briefly unreachable
must not stop the jobs, and cannot have handed them to anyone else, because
nobody else can reach it to take the lease either.
Stopping is no longer arranged by startCrontab. A scheduler now stops for
two different reasons and only the supervisor knows which - and since a
start happens every time the lease is taken, registering a shutdown
callback there would add one per leadership change for the life of the
process, because SetShutdown appends.
sys_job.entry_id keeps its meaning. This does not distribute the jobs and is
not meant to: the HTTP side scales, the scheduler stays single-writer.
Fixes#915.
One row per database, taken and renewed by single UPDATE statements whose
RowsAffected the database decides. Nothing uses it yet; the scheduler is
wired to it next.
The two timestamps are epoch milliseconds in a BIGINT rather than timestamp
columns, which is the one decision here worth explaining. A timestamp does
not survive the trip through a driver unchanged: read over go-admin's own
`parseTime=True&loc=Local` DSN, MySQL's UTC_TIMESTAMP arrives relabelled as
local time, and on a UTC+8 host every lease is eight hours out. It is
invisible to any test that compares the lease against itself, because each
instance's own arithmetic stays self-consistent - only the comparison
between two instances is wrong, which is the only comparison that matters.
An integer has no timezone for a driver to apply.
The migration seeds the row free. There is no insert path at runtime, so two
instances starting together cannot race to create the row they are both
trying to claim, and neither has to tell a duplicate-key error apart from a
real one in whichever driver it is running against. A missing row is
therefore reported rather than recovered from: silently never scheduling
anywhere is the worse failure.
The current-time expression differs per dialect and all four are covered:
MySQL, PostgreSQL and SQL Server against real servers, SQLite by default.
MySQL is the dialect most installations run and the only registered driver
with no service here. The scheduler lease that follows reads the database's
clock, and the first implementation read it as a timestamp: over go-admin's
own `parseTime=True&loc=Local` DSN, MySQL's UTC_TIMESTAMP comes back
relabelled as local time, so on any host that is not UTC every lease was one
zone offset out. Every assertion that compared the lease only against itself
still passed, and the three dialects already here could not see it.
The DSN keeps loc=Local on purpose: it is what config/settings.yml ships. A
DSN here that quietly differed would test a configuration nobody runs.
value-format="YYYY-MM-DDTHH:mm:ssZ" works today only because dayjs does
not currently give T a format-token meaning, so it passes through as a
literal character -- an accident of the current token table, not
something this string declares. Escaped it to YYYY-MM-DD[T]HH:mm:ssZ,
dayjs's own syntax for "this character, verbatim, not a token": produces
byte-for-byte the same output today (confirmed below) and stops
depending on T staying meaningless in a future dayjs version.
Re-verified both directions against the escaped string, and did so
against the real el-date-picker component this time rather than dayjs
alone: mounted element-plus's actual ElDatePicker with
value-format="YYYY-MM-DD[T]HH:mm:ssZ" (copied from a real rendering of
this fixed template, not retyped) and confirmed it renders a real,
non-blank date -- not "Invalid Date" -- when its modelValue is set to
either shape Go's encoding/json actually sends (a numeric offset or a
literal Z for UTC), and that both render identically since they are the
same instant. Submission was re-checked through the same dayjs call the
component itself makes to format a picked value. A fourth check formats
the same instant with both the old and the escaped string and asserts
they are equal, so this suite would have caught the difference if the
escape had changed anything instead of just hardening it.
3625ce8 fixed the no-empty-object-type lint error by switching a
zero-IsQuery table's {ClassName}Query to `Record<string, never>`, the
same default useTable.ts's own `TQuery extends object = Record<string,
never>` uses. That default is safe there only because useTable.ts's one
internal `TQuery & PageQuery` goes through an `as` cast rather than a
structural check. Code that builds the object literal directly does not
get that protection - and vue.go.template's foreign-key dropdown fetch
does exactly that: `list{FkClass}({ pageIndex: 1, pageSize: 100 })`.
Record<string, never> is a mapped type over every string key, each
mapped to never, so `Record<string, never> & PageQuery` does not leave
PageQuery's own properties alone: pageIndex becomes `never & number`,
i.e. never, and no value can be passed for it - a straight type error,
not a lint warning, so the previous fix's pnpm lint pass did not catch
it. Trips on any query-less table referenced by a foreign key - a plain
lookup/dict table used as a dropdown source, an ordinary shape, not a
rare one.
Switched to Record<never, never>: a mapped type over the empty key
set, which behaves as the empty object type `{}` under intersection
(PageQuery's properties come through unchanged) while still satisfying
`TQuery extends object` and not tripping no-empty-object-type (it is a
generic instantiation, not a literal `{}` type annotation) - confirmed
all three separately before touching the template.
Verified with node 24.11.0: generated a real zero-IsQuery table, copied
its .ts into go-admin-ui alongside a throwaway file reproducing
vue.go.template's exact FK call site
(`await list{Class}({ pageIndex: 1, pageSize: 100 })`), and ran both
eslint and vue-tsc --noEmit - the type-check step lint alone cannot
cover, which is what let this through the first time. Confirmed red
first (swapped the generated file's Record<never, never> back to
Record<string, never>): vue-tsc reported the exact "Property 'pageIndex'
is incompatible with index signature" error. Restored the fix - both
clean.
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.
jsonFieldPattern copied businessName's rule (^[a-z][A-Za-z]+$: at least
two letters, no digits) on the theory that jsonField should tighten to
the same identifier shape. That does not hold: businessName is typed
by a person on genInfoForm.vue, so a strict pattern is a reasonable
guardrail on human input. jsonField is computed by the importer from
the column name (sys_tables.go's namelist/JsonField loop) - nobody
types it, so the same pattern only rejected names the importer
legitimately produces: a single-letter column ("x") or one whose last
segment ends in a digit ("address2", "a1") both collapse to a single
camelCase word with nothing left to re-capitalize, and both failed the
old check.
The blast radius is wider than "this one column can't be edited":
validateAndSanitizeColumns runs over every column on every Update, so
a table that merely contains one such column could not save any
config change at all, including edits with nothing to do with that
column.
Relaxed to ^[a-z][A-Za-z0-9]*$ - any legal JS/TS identifier starting
with a lowercase letter. Still rejects what has to be rejected: empty,
whitespace/punctuation, and leading-digit names, since those cannot be
unquoted object keys in the generated interface/lang file at all.
Uniqueness and the expression-content check on defaultValue are
unchanged - defaultValue is genuinely user-typed (F6's config page),
so tightening it was the right call to begin with; this was the only
place a human-input rule had been copied onto machine-generated data.
Verified against the real import path, not hand-typed jsonField values:
built a table with columns id/x/address2/a1 in a fake information_schema,
ran it through the real SysTable.Insert, confirmed the importer computes
exactly jsonField x/address2/a1, then submitted an update through the
real SysTable.Update changing only tableComment (nothing about those
columns). Confirmed red first - 500, "jsonField 格式不合法：\"x\"" - a
change unrelated to any of the three columns was rejected solely because
they existed on the table. Restored the fix - 200, "修改成功".
A plain display table with no search form (zero columns marked
IsQuery=1) is a normal shape, not an edge case - but ts.go.template's
Query interface only had a body when the range over .Columns found a
match, so it rendered `export interface {ClassName}Query {}`, which
@typescript-eslint/no-empty-object-type flags and pnpm lint fails on.
Falls back to `export type {ClassName}Query = Record<string, never>`
when no column qualifies - the same answer go-admin-ui's own
useTable.ts already gives this shape (`TQuery extends object =
Record<string, never>`), so it intersects with PageQuery the same way
an empty interface would have and callers do not need to special-case
a query-less table.
Audited the rest of this template for the same "zero of some optional
feature" shape while in here, since this is the third time a query-less/
select-less/whatever-less table has slipped through (dictLabel, DateCell,
this one):
- a zero-column table: cannot occur - a table without at least a
primary key column cannot exist to be imported from information_schema
in the first place, so .Columns is never empty here.
- a table with only its primary key column: the row interface still
has one property (the pk); not empty, no lint issue.
- a table where every column has IsInsert=0: does not affect this
template - add{Class}/update{Class} both take the full row interface
unconditionally (every column, not just IsInsert ones), so there is
no column-count-dependent shape to go empty here.
Verified with node 24.11.0 (not the machine default): generated a
zero-IsQuery table and a with-IsQuery table, copied both into
go-admin-ui and ran eslint + vue-tsc --noEmit. Confirmed red first -
`export interface VerifyNoQueryQuery {}` failed eslint with exactly the
no-empty-object-type error. Restored the fix - clean on both, plus a
throwaway call site instantiating useTable<VerifyNoQuery,
VerifyNoQueryQuery> to prove the type satisfies useTable's `TQuery
extends object` constraint, not just that it parses in isolation.
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).
MLTBName (table_name with underscores turned to dashes, e.g. "user_profile"
-> "user-profile") is a gorm:"-" field - table.Get never fills it in, the
caller has to. NOActionsGen has done so since it existed; Preview never
did, so every template's import path that reads it
(from '@/api/{PackageName}/{MLTBName}' in vue.go.template, present in
both the pre-Vue3 template and F4's rewrite) rendered with the module
segment missing - "from '@/api/admin/'" - in the preview dialog only.
The real generated file was always correct.
Pre-existing, not introduced by this PRD, but worth fixing now: F3/F9
added two more Preview panes (the language packs) on top of an assumption
- that Preview's output stands in for what NOActionsGen actually writes -
that was never true for this field. Checked the rest of gen.go for the
same "only set on the write path" shape; MLTBName is the only one -
every other field Preview's templates read comes straight off the
sys_tables/sys_columns rows table.Get already loads.
Verified with a throwaway harness driving Gen.Preview through a real gin
Context and sqlite-backed db, parsing the JSON response and reading back
template/vue.go.template's import line. Confirmed red first (temporarily
removed the added line): "from '@/api/verify010/'". Restored it: "from
'@/api/verify010/verify-widget'".
vue.go.template (F4) only had a flat fallback for an unconfigured
colWidth: 110 for datetime, 120 for everything else. R2 asks for a
precise per-type inference instead - varchar(n) tiered by n, tinyint(1)
narrower than a general integer, text/blob wide - sized so a typical
table's list columns land inside go-admin-ui's ~580px text-column
budget. text/template cannot parse "varchar(255)" itself, so this has
to run in gen.go before the template executes, not in the template.
Judgment is columnType, not goType, same as F1/F2's earlier reversal
(API契约.md §1.1): sys_tables.go:323-338 gives every non-primary-key
int/tinyint/bigint/decimal column goType "string", so goType alone
cannot tell a boolean flag from a bigint from a name column.
InferColumnWidth(columnType string) int is exported and pure per
测试用例.md §2.5's own request, so it can be pinned with an exact
input/output table rather than only asserting "the rendered page
happens not to overflow" - see column_width_test.go, including the
tinyint(1)-vs-tinyint(4) and the varchar tier-boundary cases.
applyInferredColumnWidths only touches a column already at the 0
sentinel, wired into both Preview and NOActionsGen ahead of the
template execute calls; a column already configured (by the user or F6)
is left untouched, also pinned by a test.
Verified against the real NOActionsGen call path (not just the unit
test): generated a table mixing every branch (tinyint(1), datetime,
decimal, varchar at three tiers, text, and one pre-configured column)
and printed the resulting tab.Columns[i].ColWidth after generation -
every value matched InferColumnWidth's own table, and the
pre-configured column's 333 was left untouched.
ts.go.template hardcoded the pk parameter as `: number`. vue.go.template
(F4, landed after this file) derives $pkType per table - "number" unless
the pk column's GoType is "string" (natural keys do exist; sys_tables.go
gives a pk column GoType "string" whenever its ColumnType is not
int-shaped), then types useForm on that same $pkType. A string-pk table
therefore generated a page calling get{Class}(id: string) against an api
module whose get{Class} only accepted number - a TS compile error, not
a runtime bug, and exactly the kind of gap each side's own template-only
verification could not see (F4 was checked against a hand-written .ts
stub before this template existed; this template was checked against a
numeric-pk fixture only).
Copies vue.go.template's $pkType derivation verbatim so both templates
agree by construction rather than by convention.
Verified: generated both a numeric-pk and a string-pk table's api module,
copied both into go-admin-ui and ran vue-tsc --noEmit - clean. Added a
throwaway call-site file exercising getVerifyStrPk with a string and (via
@ts-expect-error) confirming a number argument is now rejected, so the
type is actually enforced rather than having quietly widened to any.
That bind-and-save path had zero field-level validation (API契约.md
§1.2/§2.1, decision D6), which left four ways to silently corrupt the
generator's own metadata or the language packs it writes:
- jsonField had no format rule at all, unlike tableName/className/
businessName, which all have a pattern check. A jsonField with a
dot, a leading digit, or anything not matching lower camelCase
still saved, and would land verbatim as a key segment in
gen/{PackageName}/{BusinessName}.ts.
- jsonField had no per-table uniqueness check. Two columns retitled
to the same jsonField overwrite each other's generated i18n key
with no warning.
- businessName had no cross-table uniqueness check within a
packageName. Two tables sharing one write the same language pack
path and the second's generation silently clobbers the first's.
- defaultValue is spliced into the generated defaultModel() as a
literal, never evaluated (API契约.md §2.1) - so content shaped
like a function call or block does not do what it looks like it
does, and previously saved without complaint either way.
colWidth is handled differently on purpose: API契约.md §2.1 says an
out-of-range value should fall back to the inferred width rather than
be rejected, so it is reset to the 0 sentinel in place instead of
failing the request - same outcome as if it had never been set.
G10 (colliding with the built-in admin/* i18n namespace) is not one of
the four: D9 already moved generated keys into their own gen/
namespace, so that collision no longer exists.
All four checks and colWidth's sanitize-in-place path are covered by
sys_tables_validate_test.go. Confirmed red first: swapped in a no-op
stand-in for both validators and reran - 13 sub-tests that should now
be rejected or sanitized passed straight through instead (jsonField
format x5, jsonField uniqueness x1, colWidth range x2, defaultValue
expression x4, businessName uniqueness x1). Restored the real
implementation and reran - all green.
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.
Updates(&e) uses GORM's struct form, which skips zero-value fields -
but 0/"" is exactly the sentinel PRD 010 F1/F2 chose for "unconfigured"
(数据库变更.md §1.1). A caller resetting colWidth or defaultValue back
to that sentinel was therefore silently ignored: the row kept its old
value, the API reported success, and reopening the edit form showed the
stale number/string again.
Not fixed by switching to Select("*") - that would also start writing
this struct's other zero-valued fields (Sort, the Pk/Required/... bools),
none of which are part of this change. A second, map-form Updates
scoped to just these two columns writes the sentinel without touching
anything else; map-form Updates does not skip zero values.
TestSysColumnsUpdateClearsSentinelFields pins the regression: seed
150/"active", update to 0/"", read back and assert it stuck. Confirmed
red before this change (got 150/"active" back) and green after.
Add lang-zh.go.template and lang-en.go.template, one key per column
keyed by JsonField, valued from ColumnComment (R3) with a fallback to
JsonField when ColumnComment is blank (G11) - both locales resolve to
the same zh-CN text today since D3 rules out machine translation and
go-admin-ui's parity test rejects blank values.
NOActionsGen writes both files to lang/{locale}/gen/{PackageName}/
{BusinessName}.ts, two levels deep so go-admin-ui's gen-namespace.ts
glob (./*/*.ts under each locale's gen/) picks them up; nesting under
PackageName (rather than a flat gen/{BusinessName}.ts) avoids two
tables in different packages silently overwriting each other's
translations, since BusinessName has no uniqueness check.
Output has to be single-quoted with no trailing comma to satisfy
go-admin-ui's eslint config, which text/template's builtin `printf "%q"`
does not produce, hence the small parseGenTemplate/singleQuote helper
shared by the two templates.
Verified end to end: generated a real table's output (including a
blank-ColumnComment column, to exercise the G11 fallback), copied the
two language packs into go-admin-ui and ran vue-tsc --noEmit and eslint
against them - both clean, comma-dangle/quotes included.
Rewrite template/v4/js.go.template (renamed to ts.go.template) to emit
TypeScript matching go-admin-ui's src/api/demo/product.ts: an interface
per row and per query params, typed CRUD functions, and a del{Class}
that takes Id[] like useRemove expects. GoType maps to number for
int/int64/float32/float64 and string otherwise, since go-admin-ui's
request() types the {code,data,msg} envelope rather than the payload.
Preview's response map key changes from template/js.go.template to
template/api.ts.template so its Tab label matches the new content;
NOActionsGen writes the file with a .ts extension.
Verified end to end: generated a real table's output, copied it into
go-admin-ui and ran vue-tsc --noEmit and eslint against it - both clean.
Back the code generator's Vue 3 template migration: col_width lets R2's
column-width inference be overridden per field, and default_value lets
R1/A6's "unconfigured rows still generate a usable page" guarantee hold
for generated forms. Both use a sentinel default (0 / "") rather than
NULL so "unconfigured" has exactly one representation - see
docs-prd/010-代码生成器前端模板迁移Vue3/数据库变更.md §1.1.
The migration and the model change land together: AddColumn reads the
column definition off tools.SysColumns's own gorm tag, so splitting them
across commits would leave one of them failing to compile.
The tree is at zero unformatted files for the first time. Nothing holds it
there: gofmt drift is invisible in review because the common form of it is a
missing newline at the end of a file, which no diff reader notices and no
existing check looks at. That is how the batch cleaned up just now reached 26
files.
`make fmt-check` reports rather than rewrites. CI cannot commit, and a target
that silently reformats hides what it touched; `gofmt -l` names the files and
says nothing when there are none, so its output is both the failure message and
the fix.
The step runs before the tests rather than beside checksilent at the end. A
formatting miss is a one-command fix, and learning about it after five minutes
of tests and an end-to-end install wastes five minutes for nothing.
The diagram at the top reads Router -> Api -> Service -> Model, the section is
headed "Api 层", and the directory is app/admin/apis/. Three sentences in the
body still called that layer Handler -- the name it went by before the rename,
and one nothing in the tree answers to now. A reader following the document has
no way to tell the two are the same layer.
Wording only. No rule changes, and no other file is touched.
`gofmt -l` listed 26 files. Seventeen of them were missing the newline at the
end of the file; the rest are indentation that used spaces where the file uses
tabs, a handful of call sites written `f(a,b)`, and the doc comment spacing
gofmt has rewritten since 1.19 (`//X` to `// X`).
Nothing here changes behaviour: `go build ./...` and `go vet ./...` are clean
and `go test ./common/...` passes, which is the half of the tree these files
are concentrated in.
Only the files gofmt named are touched, so the diff reads line by line rather
than as a reflow of the whole repository. `gofmt -l` is now empty, which is the
precondition for gating it in CI -- worth doing, but a separate change.
NoCache sent `no-cache, no-store, max-age=0, must-revalidate, value`. The
trailing `, value` is not a directive; it is a leftover token that has been on
every response this middleware touches since the file was written. Unknown
directives are ignored, so nothing misbehaved because of it, but it went out on
the wire and read as a mistake to anyone looking.
The assertion added in #937 pins the old value, so it moves with the source:
removing the token from the middleware alone turns TestNoCache red, which is
the whole point of that test and the reason both lines change together here.
The generator's import reads its comma-separated table list with
c.Request.FormValue("tables"), which on a request declaring itself as JSON reads
the URL query and nothing else. go-admin-ui v3.2.0 began sending that list in
the body, so the handler saw an empty string, asked information_schema for a
table named "", and every import failed with "table name cannot be empty！" —
on a fresh installation that is the first thing the generator is asked to do.
tablesToImport reads the query first and falls back to the body, so a front end
sending either works against this server. It also drops blank entries:
splitting "" yields one empty name rather than nothing at all, which is why the
old code reached a database query at all before failing.
The front end sends the list in the query again on its side; this half is what
lets an installation already running v3.2.0 recover without changing it.
The string was spelled out at each site that raises it, and once more in the
test file that asserts on it. A test holding its own copy cannot tell the
difference between the handler answering something else and the message having
been reworded: it goes on asserting a string the server no longer sends, and
goes on passing.
The three copies in app/other/models/tools are left alone; they are raised from
a different layer and nothing asserts on them.
newEngine takes the method, path and handler, so a second test file does not
have to restate the sqlite connection, the driver override and its cleanup, the
CustomError middleware and the two context keys. serveJSON does the same for
running one request and decoding the envelope.
Nothing about what is asserted changes; newColumnListEngine and columnListMsg
keep their names and their callers.
#924 buffered the channel Remove returns, so the goroutine it starts can
finish when the caller has already timed out and walked away. That half of
the patch was not covered: reverting the buffer on its own left the whole
suite green, while the timeout assertion it shipped with stayed red only for
the error-return half.
This test abandons twenty channels and then requires every goroutine to be
gone. It waits for the scheduler to empty first, so a goroutine that has not
yet reached its send cannot make the test pass while proving nothing.
Without the buffer: 20 of 20 parked on `chan send` at jobbase.go:216.
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.
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.
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.
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.
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.
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".
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.
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.
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.
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.
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.
The soft-delete conversion dropped the indexes on deleted_at through
gorm's Migrator().DropIndex. Its PostgreSQL driver resolves a schema for
the statement and falls back to an expression when it cannot:
currentSchema, _ := m.CurrentSchema(stmt, stmt.Table)
m.DB.Exec("DROP INDEX ?.?", currentSchema, clause.Column{Name: name})
DROP INDEX takes an identifier in that position, so what reached the
server was
DROP INDEX CURRENT_SCHEMA()."idx_sys_api_deleted_at"
ERROR: syntax error at or near "CURRENT_SCHEMA" (SQLSTATE 42601)
The schema is unresolvable for every call this migration makes, because
it passes a table name as a string rather than a model. So it failed on
every PostgreSQL database rather than intermittently, and stopped the
whole conversion at the first table.
What that looked like from outside is go-admin#919: an upgrade that
could not complete, and a login rejecting a correct password, because
deleted_at was still a timestamptz while the current query compares it
to 0. Neither symptom names a migration.
Written per dialect, for the same reason addBigIntColumn and
renameColumn already are. MySQL and SQL Server name the table and have
no IF EXISTS for it; PostgreSQL and SQLite name the index alone.
Verified by running the shipped migration against PostgreSQL 15 and
MySQL 8.0 in containers, and SQLite through this package's tests. The
SQL Server form is from its documentation and has not been run - there
is no SQL Server here to run it against, and saying so is better than
implying four dialects were checked.
The process started, both probes passed, and the first sign that the
schema did not match was a login failing with a driver-level encoding
error - go-admin#919, where an operator upgraded the binary and
restarted the API without running migrate. Nothing between those two
events had an opinion about the schema.
Readiness is where this belongs. Liveness asks "restart me", and a
process whose database is on the wrong schema comes back to the same
schema. Readiness asks "send me requests", and the answer is no. A
rolling update then stalls at the deploy - new instances never become
ready, the old ones keep serving - rather than at somebody's login, and
running migrate clears it without a restart because the check is
evaluated per request.
Any tenant database being behind fails the check, not only the one being
served: migrations are applied to every database in one run, so one
behind means that run did not finish, and serving the rest would let a
half-applied deploy look like a partial success.
A missing sys_migration table is nothing applied rather than an error.
That is a first deploy, where every migration is pending and the
operator can act on being told so.
The one test that matters is the one that cannot be written normally.
The registry is filled by init() in packages cmd/api does not import, so
a test that imported them to look at it would pass whatever the real
binary links - and a binary that links none of them gives a check that
reports every database current, forever, with every other test here
still green. TestTheServingBinaryLinksTheMigrationRegistry asks the
build instead, with a negative control so that a query matching
everything fails rather than passes.
Closes#920.
Whether the schema matches what the binary expects is answered by the
migration registry, which lives under cmd/. common/ has never imported
cmd/, and starting with this would put the shared layer behind the
command layer for one check.
Register instead, from where both are already in scope. A duplicate name
panics rather than appending: two checks under one name make the failing
one impossible to identify from the response body, and registering the
same one twice is a wiring mistake better heard at start-up than never.
The registered check is run through the same guard as the built-in ones,
so one that panics fails its check rather than taking down the probe
that asked.
Status answers what is registered, what is applied, and what is applied
while nothing registers it - and needs a database to do it. A readiness
check needs only the first half, and it already holds the databases it
is asking about.
Without this it would have to call SetDb to reuse Status, writing this
package's shared state from a request path, for a question that does not
depend on any database at all.
The wait was one select over done and ctx.Done(). Both can be ready when
it runs, select picks at random among ready cases, and so a queue that
drained in the same instant the budget expired was reported as an
overrun about half the times it landed there - often enough to be read
as noise, and pointing at the wrong thing when it was not. core's own
RunShutdown re-checks for this reason; this did not.
The tie-break is now a function taking channels rather than a queue,
which is what lets a test hand it a closed done and an expired ctx
together. That state is the whole of the bug and cannot be arrived at
reliably from the outside; over 1000 iterations the single-select
version fails, and the second look does not.
The test for giving up on the deadline read the call counter straight
after shutdownQueue returned, while Shutdown runs on a goroutine nobody
joins. It passed because the goroutine is scheduled promptly, not
because anything ordered the two. The fake now signals that Shutdown has
been entered and the test waits for it.
Both raised by Copilot on #918.
The first attempt at the tie-break test was wrong and is not what
landed: it asserted that an immediately-returning Shutdown always counts
as drained under an already-expired context, which is not true and
should not be - if the goroutine has not run, nothing has drained. That
test failed, correctly. What is being claimed is narrower: when both are
ready, done wins.
Nothing stopped the queue when the process exited. core v2.7.0 made the
drain work - Memory.Shutdown closes the queue and waits for every
consumer to finish what it holds, and the legacy adapter cancels its
context and closes the underlying queue - but no call site ever reached
it. The only Shutdown() in this repository applies to the previous
adapter during a reload, so the installed one was simply left. The login
log, the operation log and the API sync all publish through it, so a
rolling restart dropped whatever had not been consumed, on the path
where the process exits 0 and reports "Server exiting".
Setup now registers a BeforeExit callback that shuts down the adapter
this package installed.
Three things it has to get right, each with a test.
It reads `installed` when it runs, not when it registers. A reload
replaces the adapter, and the one from start-up is a queue nobody has
published to since.
It never goes through sdk.Runtime.GetQueueAdapter. That accessor never
returns nil - with no queue section configured it wraps the runtime's
own fallback - so it would look like it worked while closing a queue
this package neither built nor started. That is the same trap setupQueue
already had to drop an `if q != nil` for.
It registers once. Setup is re-run on every configuration change, and a
callback per reload would leave the shutdown phase holding a row of
identical entries, each eligible to be named as the one that overran the
budget.
That last one needed a seam. shutdownQueue takes the adapter on its
first run, so the second and third callbacks find nothing and return -
three registrations produce exactly the same observable result as one,
and a test going through the effect passes either way. It did: the
counter-proof for "register on every reload" came back green until the
registration was counted at the seam instead.
The wait is bounded here rather than left to the phase. Shutdown takes
no context, so a consumer that never finishes would hold the process
until SIGKILL; the callback gives up and says what is being lost, which
the phase's generic overrun message cannot.
Ordering falls out of the phase rather than being arranged: callbacks
run in reverse registration order, this one registers during setup and
the job scheduler's registers on AfterListen, so the schedulers stop
before the queue drains. Verified against core v2.7.0 rather than read
off the source.
Closes#911.
The warning said to set application.mode and stopped there. Following
that on a running process does not close anything: buildRouter has one
call site, in run(), and route registration is on no phase and no reload
callback, so a configuration reload moves the mode and leaves the routes
exactly where they were.
The reader is then worse off than before they acted. The mode now says
prod, GenWriteRoutesEnabled agrees, and the endpoints are still served -
so the one thing they could check to confirm the fix reports success
while the exposure is untouched, until something restarts the process.
A test pins the gap rather than the prose: build under dev, move the
mode to prod, and the routes are still in the engine. It fails if
registration ever becomes dynamic, which is the change that would make
the new sentence wrong.
That test degrades differently from the others - making it fail means
rewriting registration, not weakening it - so what was checked instead
is that it cannot go vacuous. Both of its premises are guarded: with the
gate always refusing it reports building under dev without the writing
routes, and with the gate always allowing it reports the predicate still
allowing prod. Neither failure can be mistaken for the assertion passing.
Raised by Copilot on #917.
registeredRoutes set config.ApplicationConfig.Mode and gave it back with
t.Cleanup, which runs at the end of the test rather than at the end of
the helper. Everything the caller did after the call therefore ran under
the mode the helper had been asked about, not one the caller chose.
Nothing was wrong yet: the one caller that reads the mode afterwards
sets it itself, and the two cleanups happen to unwind in an order that
leaves the right value. Both of those are accidents, and neither is
visible at the call site.
A defer inside the helper makes the borrowing end where it starts. The
doc comment said the mode was put back before returning while the code
did not, so that is now true rather than aspirational.
The counter-proof is the reason this has a test of its own: with
t.Cleanup back in place TestRegisteredRoutesRestoresTheModeBeforeReturning
fails and nothing else does, which is what a leak this quiet looks like
when something is actually watching for it.
Raised by Copilot on #917.
The gate in the previous commit is decided by application.mode, and the
shipped configuration says dev. So the deployment most likely to be
serving the writing endpoints is the one that changed nothing, and that
is also the one least likely to go looking for them. A gate whose
default is open needs to say so.
Nothing is said in demo mode. The routes are registered there, but
DemoEvn refuses all three by name, so a warning would describe an
exposure that is not present.
The decision is split from the logging so it can be tested. Three
counter-proofs: warning in demo as well fails mode=demo; a warning that
never fires fails mode=dev, which is what shows the line can be reached
at all; and one that always fires fails every mode but dev.
Three of the code generator's endpoints do not read. /gen/toproject
writes seven Go and Vue source files onto the host, one of them under
the path gen.frontpath names; /gen/apitofile writes a migration;
/gen/todb inserts menus and APIs. All three are GET, and all three are
listed in CasbinExclude - which AuthCheckRole skips - so Enforce never
runs for them. Any account that can log in could call them, on every
deployment.
They are now registered only where application.mode is dev or demo. dev
is the shipped default and is where the generator is meant to be used.
demo keeps them because demo mode already has a better answer than a
404: DemoEvn refuses these three by name and explains itself, which is
what the demo is for. test and prod get nothing, and so does a process
whose mode was never set.
This does not make the endpoints safe where they exist; it stops them
existing where nobody should be calling them. A host left on the shipped
dev is still open, which is why the next commit says so at start-up.
CasbinExclude is left alone on purpose. Taking the three off that list
would make them require a permission no existing deployment has granted,
so every non-admin user would start getting 403 from a tool that worked
yesterday. That is a migration, not a guard, and it belongs with a
release that can carry one.
Four counter-proofs, each red on the test that names the behaviour and
green everywhere else: a gate that always allows fails test/prod/unset
only; a gate that always refuses fails dev/demo and takes
TestEveryRouteDemoModeRefusesStillExists with it; moving a read-only
route inside the gate fails the reading test; and spelling the condition
at the registration site instead of calling the predicate fails the
agreement test, which is what keeps that test from being a tautology.
Two pre-existing deviations: a space before the comma in
sysNoCheckRoleRouter's parameter list, and no newline at end of file.
Separated from the change that follows so its diff is only the change.
The comment at the top of this workflow says documentation-only changes
skip it, because a push to master pushes an image, runs the migrations
and restarts the demo container. The ignore list did not cover
scripts/k8s, so editing a manifest that the deploy never reads bought
the site an outage.
The pattern is scripts/k8s/** rather than scripts/** because
scripts/Dockerfile is a build input - go.yml builds the release image
from it on a tag.
This workflow file stays outside the list on purpose. paths-ignore skips
only when every changed path matches, so a change that edits the deploy
still runs it, which is the point.
The comment on replicas gave one obstacle to raising it, the shared log
volume, which reads as the only one. Someone who moves the log path off
that volume would conclude the way is clear.
The scheduler in app/jobs is the second, and it is the one that does not
announce itself. Its handle on a job lives in sys_job.entry_id, one
column shared by every process, and startup zeroes the whole column
before writing its own ids. A second pod therefore erases the first
pod's, and both pods run the full enabled list. Stopping a job from the
UI then removes an entry from whichever process is asked, by an id that
belongs to another one, and answers 200.
See #915.
DemoEvn decided by HTTP method: GET and OPTIONS through, everything else
refused. Three of the code generator's routes are registered as GET and write
anyway - two emit Go source files onto the server's filesystem, and the third
inserts menus, APIs and casbin rules into the database. They sit in a group
whose own name says it does no role check, and a demo deployment lets anybody
log in. So on the demo host any visitor could write to the machine and to the
database, and the one that writes menus had in fact been used: three generated
SysCasbinRule entries is how this was noticed.
The guard now also looks at the matched route. The method cannot answer the
question - whether a request changes anything is not something the verb reports
truthfully here - so the three are named, as gin route patterns, which is what
Context.FullPath returns and how CasbinExclude already spells them.
Changing them to POST would be the better shape and is not this change. sys_api
records an endpoint by method and path and the casbin policy follows it, so
flipping the verb needs a migration and a policy resync; until both land, every
existing deployment would start answering 403 to a role that could use the
generator the day before.
The read-only half stays reachable: preview, the table tree, and the two
database listings. A demo host that cannot demonstrate the generator is as
broken as one that lets visitors write to it - refusing too much is the same
defect facing the other way, and there is a test for that direction too.
Half of the general hole is closed and the other half is written down. The
closed half is a test beside the route registrations: it builds the generator's
routes, enumerates them, and fails if any entry in the guard has stopped being
a real route, so renaming one turns the list red instead of quietly making it
match nothing. It lives there because common/ may not import app/ - which is
also why the guard cannot check its own list from where it is. The open half is
that no static check can tell a handler that writes from one that reads, so the
next GET that writes has to be added by hand. The comment says that rather than
leaving the impression the class is covered.
application.demomsg was configuration nothing read. The message was hard-coded
in the middleware, and the demo host's configured string happened to be
identical, so the setting looked like it worked and never had. It is read now,
with the old string kept verbatim as the fallback, so a deployment that never
set it is answered exactly as before.
This covers demo mode only. On a deployment that is not a demo those three
routes remain in CasbinExclude and stay reachable by any authenticated user
whatever their role; that is a separate decision and is not touched here.
The budget is one number in config/settings.yml. The deadlines that have to
cover it are in four other files, none of which anybody edits while thinking
about shutdown - so raising the budget passes every test, deploys, and has the
cleanup callbacks killed on the next release.
Two checks share one arithmetic and one five-second margin.
shutdown-budget-overruns-grace compares preStop + drain + server + cleanup
against terminationGracePeriodSeconds in the shipped manifest. Those two files
are not merely adjacent examples: scripts/k8s/prerun.sh builds the
settings-admin ConfigMap out of config/settings.yml and the Deployment mounts
it, so the manifest deploys that file.
docker-stop-cuts-shutdown-short covers the three ways this container is
stopped: `docker stop` in the release workflow, the same in the Makefile, and
stop_grace_period on a compose service that runs this repository's own image. A
service running a database is not this process and is left alone. The duration
is parsed rather than scanned for digits - compose accepts 1m30s, and reading
the first number out of it would call ninety seconds one.
All three spellings of the deadline are read: --timeout, the deprecated --time,
and the short -t. A deadline the check cannot read is reported as no deadline at
all, so recognising only one of them would call a correct command broken and
send whoever fixed it towards the spelling docker is retiring. The message
quotes the flag back in the spelling it was written in, for the same reason:
suggesting a flag the line does not use is how a tool teaches people to
disbelieve it.
What neither covers is `docker rm -f`, which has no deadline to compare
against: it is SIGKILL by definition. That gap is deliberate, and it is why the
previous commit changed the one place that used it on a container that might
still be running.
Both report at two levels. A budget that already overruns is an ERROR; one that
fits with nothing to spare is a WARN, because it works today and failing the
build on a working configuration is how a project teaches people to ignore its
warnings. The two are exclusive: an overrun satisfies the headroom condition as
well, and an ERROR that always drags a duplicate WARN behind it teaches the same
lesson.
preStop is in the sum although the shipped manifest has no hook. That is the
point - a hook added later is spent before the process is told anything, and a
self-check that could not see it would understate the real budget by however
long somebody set it to, which is worse than not checking. A hook whose duration
cannot be read is reported rather than counted as zero.
The fallbacks for fields the settings file leaves out are read from the
constants in the scanned tree, not copied here; if they are renamed the run
stops instead of going quiet with the wrong numbers.
The wording differs by audience on purpose. At run time this is somebody else's
deployment under constraints the process cannot see, so the log states a
minimum. These checks read files this repository owns, where there is standing
to ask for headroom, so they name a target.
The table in AGENTS.md is relisted while it is being touched: the two new
checks, plus datascope-route-unguarded, which has been missing since it was
added. The hard-coded count is gone - it said seven and there were ten, which is
what a written-down count does. AGENTS.md and docs/contract.md both sent readers
to `go run ./tools/checksilent -h` for the list of checks; that prints
command-line flags and has never printed a check, so both now point at
runChecks.
The yaml parser moves from an indirect requirement to a direct one - it was
already in the module graph - and tidy drops four go.sum lines left over from
two older releases of core.
Stopping this process takes drain + server + cleanup seconds: eight out of the
box, and more for anyone who configures a drain window. Three places decide
whether it gets that long, and none of them was written with it in mind.
The release workflow stopped the previous container with the default deadline,
which docker sets at ten seconds. The compose file - which the Makefile calls
the first way to run this - set no stop_grace_period, so it took the same ten.
Under either, a drain window over two seconds would have been cut off by
SIGKILL part-way through the cleanup callbacks: this project's own deployments
could not have run the capability it ships.
The third was worse. `make run` removed the previous container with `docker rm
-f`, and the force flag kills a running container outright - "uses SIGKILL", in
docker's own words - with no grace at all. Restarting locally cut every
shutdown short, so the drain window would never once have been reached on a
developer's machine. It now stops with a deadline and then removes, which
leaves what gets removed unchanged: on a container that has already stopped,
stop is a no-op.
So: --timeout 30 in the workflow, stop_grace_period: 30s on the compose
service, and stop --timeout 30 before the removal in the Makefile. --timeout
rather than --time, which docker still honours but has deprecated - it prints a
warning on every use, and a deploy log that always carries a warning is one
nobody reads.
The three remaining `docker rm -f` calls in the workflow are left alone. Two
remove containers that have already been stopped and one is the rollback path,
and nothing static can tell those apart from a container that is still running -
which is also why the check added next does not look at `rm -f` at all: a forced
removal has no deadline to compare against. What keeps that path honest is the
line above it, not a check.
Thirty will drift the first time somebody raises a budget. The next commit is
what notices, which is also why these comments name a check that does not exist
yet.
The manifest in this repository had no probes at all. A pod was sent traffic as
soon as its container was running, whether or not the database it needs was
reachable, and it was stopped with whatever grace period Kubernetes defaults to
rather than one chosen against what this process actually spends shutting down.
It now mounts both probes, at the endpoint that answers each question:
readiness at /ready, which fails while a dependency is unreachable, and
liveness at /health, which is a bare 200 because restarting a process whose
database is down turns one outage into a crash loop. Both skip the rate
limiter, which is why that had to land first.
timeoutSeconds is 3, not the default 1. The handler allows its checks two
seconds, so at the default a database answering in 1.2s would be recorded as a
failed check while the handler was returning 200 - the probe would be failing on
the orchestrator's stopwatch, not on its own. The comment beside that constant
said the constraint was the polling period; the constraint is the per-check
timeout, and it is now written down correctly.
terminationGracePeriodSeconds is 30, against a shipped budget of 0 + 5 + 3.
Raising drain means raising this too, in the same commit; the check that
notices when somebody does not arrives two commits from here.
replicas stays at 1, and the comment says why that makes the drain window worth
nothing: there is nowhere to send the traffic this pod stops taking. Raising it
needs one more change than the number - the volume is shared by every replica
and the log path lives on it, so a second pod would append to the same rotating
file. The reason not to raise it is not the one the review assumed: the claim
was that the PVC is ReadWriteOnce, and it is not, it is ReadWriteMany on nfs-csi.
There is no preStop hook. How long one should sleep depends on how fast the
thing in front removes this instance, which the repository cannot know, and a
manifest carrying both a preStop sleep and a drain window is the double-counting
trap - the budget would be spent twice and the start-up line would report half
of it.
The three budgets are spent one after the other, so what has to fit inside the
orchestrator's grace period is their sum - and nothing said what that sum was.
Working it out meant reading a configuration file, remembering which fields
were absent, and knowing what each one falls back to.
Start-up now prints it: the three values and the total, taken from the resolved
budget rather than from the file. A field left out still costs its default, so
adding up what was written down understates the total by exactly the fields
nobody wrote - which is the arithmetic somebody doing this by hand gets wrong.
Whether the total fits is a separate question, and the framework cannot answer
it alone: `docker stop` allows ten seconds and Kubernetes thirty, three times
apart. A fixed threshold would have warned about the manifest this repository
is about to ship. So extend.shutdown.grace is optional, nothing reads it during
a shutdown, and when it is absent the line says so and prints both reference
values instead of judging.
When it is set and the budget does not fit, the warning names the shortfall:
how many more seconds are needed. A minimum, not a target - this is somebody
else's deployment under constraints this process cannot see, and asking them to
leave headroom on top is not this line's business. Equal does not fit either;
the grace period is when SIGKILL is sent, so a budget that ends exactly then
leaves the last callback no time to return.
/ready has failed from the moment shutdown begins since the readiness probe was
added, and the order it does that in is right: reversed, the state would be
reported after the connections were already cut. But order alone does not
produce a window. Nothing waited between the flip and Shutdown, so the two were
microseconds apart, and a poller on a multi-second interval never saw the 503 -
it saw a refused connection, which is the thing the probe was supposed to
avoid. Polling a container through a SIGTERM on the demo host recorded exactly
that: 200, then connection refused, and no 503 in between.
extend.shutdown.drain is that wait. The process keeps serving normally for it -
answering requests, not refusing them, because refusing them would move the
outage earlier rather than avoid it - and only then closes the listener.
It is zero by default, so nothing changes for a deployment that does not ask
for it. That is not timidity: the budgets are spent one after another, and a
non-zero default would push every existing shutdown closer to the orchestrator's
grace period, where being cut off part-way through the cleanup callbacks is
worse than never draining at all.
Keep-alive is switched off with the flip. The server keeps connections alive
until Shutdown sets shuttingDown() itself, so without this the pooled
connections a balancer holds would sit untouched for the whole window and be
cut at the end of it anyway - the cost of the window without its benefit. This
is the switch Shutdown flips, moved earlier by the window's length.
The signal disposition is restored after the window rather than on the first
signal. Before there was a window, the interval where a second signal killed
the process outright was only reachable while a cleanup callback hung; putting
a multi-second wait inside it would have made every ordinary shutdown
interruptible for the length of the drain. A second signal during the window is
taken by the channel and ends the window early instead - somebody sending
another kill wants this over with sooner - and the escape hatch comes back the
moment the window does.
What the window is worth depends on who removes this instance. A balancer that
polls /ready acts on the 503 and needs the window to cover its check interval
times its failure threshold; a Kubernetes Service withdraws the endpoint when
the Pod is deleted, concurrently with SIGTERM and regardless of what the probe
returns, and there the window covers the delay in that removal reaching every
node. The three comments that used to say a balancer "has a chance to" take the
instance out said it without either qualification, which is how a claim comes to
be repeated after a live test has refuted it.
The subprocess test polls the real probes on a connection it opens after the
signal - a reused one can be served after the listener is closed, which would
let this pass against a shutdown that had already broken it - and asserts on the
draining answer in the body, not on the status code. With no database the status
is 503 from start-up, so a status-code assertion would hold even with
BeginDraining deleted. Two window lengths, because one proves only that
something takes that long.
The limiter is installed on the engine and the probes are routes like any
other, so above the threshold they are answered with 429 too. Point a liveness
probe at one and the failure mode writes itself: traffic crosses the threshold,
the probe collects three 429s, the kubelet restarts the container, the capacity
that was already short gets shorter, and the instances that are left are pushed
further past the threshold. The limiter working exactly as designed is what
kills the pod.
It is the argument common/health already makes about restarting a process whose
database is unreachable, applied to load: turning one outage into a crash loop
is not an improvement on the outage.
Nothing points a liveness probe at these routes yet. The manifest that will is
two commits away, and this has to land first, because that manifest without
this change would be actively harmful.
The exemption wraps the middleware rather than teaching the limiter about these
paths. common/ may not import app/ - the contract check enforces it - so the
limiter cannot name routes that are registered over there. Wrapping it in the
command package, which imports both, is what keeps the boundary.
Naming those routes needs them exported, so the group prefix and the two paths
become constants and the router function becomes RegisterMonitorRouter. That
also gives a test something real to mount: a probe asserted against a
re-implementation of itself is a test of the copy.
The check that the middleware never runs is separate from the check that the
answer is not 429, because a probe can produce a 429 on its own. What has to be
true is that the request never reached the limiter.
How long a shutdown may spend waiting for in-flight requests, and how long the
cleanup callbacks get after that, were compile-time constants. The two together
have to fit inside whatever grace period the orchestrator allows before it
sends SIGKILL, and that number is not the same everywhere - `docker stop`
allows ten seconds, Kubernetes thirty by default - so the one deployment shape
these constants suited was the one they were written for.
They now come from extend.shutdown, beside rateLimit. Not from application:
that section is a fixed struct in core, and the decoder discards keys it has no
field for without an error, so a budget written there would be accepted and
never applied. That is the failure this whole change is about, and putting the
configuration where it cannot be read would have reproduced it.
Both fields are pointers, following RateLimit.InboundQPS: nil means "not
configured" and takes the default, and a number that was written down is spent
literally, zero included. Without that separation `server: 0` - do not wait for
in-flight requests at all, which is a reasonable thing to ask when the grace
period is very short - could not be expressed, and the section would need a
paragraph explaining which zeros mean what.
A negative is refused rather than clamped. Correcting a value quietly is the
same failure in a different costume, and Budget returns the error instead of
ending the process so that the rule can be tested without a subprocess.
The defaults live in config as seconds and in cmd/api as durations, both from
the same constants, and a test asserts the two agree - a deployment that
configures nothing is entitled to one answer about what it spends, not two.
The last test loads the two settings files this repository ships through the
real loader and asserts the section arrives with the documented values. Nothing
weaker can tell "the key is read" from "the key is discarded": the struct
compiles either way.
The child process built a server, restored its own signal disposition and
called shutdownServer and runShutdownHooks itself, in an order it chose. It
never called anything run() calls. So the assertions were about a copy of the
sequence: move a step in the real one, or drop it, and every test here stays
green. The acceptance criteria these back are worth exactly as much as that.
The child now calls gracefulShutdown and asserts on what comes out of it. The
budget it spends is defaultBudget with one field shortened where a test needs a
deterministic timeout, which is also how the two waits stop being wired by
hand.
The stuck-shutdown case changes shape as a result. It used to sleep inside the
child, between the steps it had copied; there is no "between" to sleep in any
more, so it registers a BeforeExit callback that never returns and gives it a
budget long enough to hang on. That is where a shutdown actually hangs, and it
now runs through the same function - which means this test also pins where the
signal disposition is restored, rather than just asserting that the child dies.
It signals repeatedly rather than once. The marker is printed immediately
before gracefulShutdown is entered, so a single signal sent on seeing it can
still arrive before the disposition is restored, land in the buffered channel
and be dropped. Which signal does the killing is not the assertion; that one of
them can is.
The steps between the stop signal and the last log line were written inline in
run(), which left nothing for a test to call. The signal tests reproduce that
sequence instead: they build their own server, restore their own disposition,
and call shutdownServer and runShutdownHooks in an order of their own. So they
assert against a copy - reorder the real sequence, or drop a step from it, and
they stay green.
The sequence now lives in gracefulShutdown, and the waits it spends are a
budget rather than two constants read at the point of use. Nothing changes
about what happens or in what order: the same disposition is restored first,
the same two announcements are made, the same waits are spent, and run() logs
the same two errors with the same messages.
Returning those errors instead of logging them inside is what lets a caller
other than run() react to them. That matters for the next commit, where the
tests stop reproducing this sequence and start running it.
Three comments said readiness failing before the server stops accepting gives
a load balancer the chance to withdraw the instance before its connections are
cut. Nothing between the two lines makes that possible: BeginDraining is
immediately followed by the shutdown, and a poller on a multi-second interval
never observes the flip.
On Kubernetes the endpoint is withdrawn when the Pod receives a
deletionTimestamp, concurrently with SIGTERM and independent of what the probe
returns, so the probe result is not the mechanism there either.
The order itself stands - reporting the state after the connections are cut is
worse - so the comments now say the order is necessary and not sufficient, and
that a window needs a configured delay that does not exist yet.
Only ErrQueueClosed was examined and every other error was discarded, so a
run where nothing reached a queue at all would leave the refusal count at
zero and the test green. The first unexpected error is now kept and fails
the test.
The pool is sized so a full queue cannot be one of them: it returns an error
of its own and is expected while the consumer is held, which would otherwise
make the new check fire on the normal path.
Two fixed sleeps decided when this test looked: one to let the consumer pick
a message up, one to let publishes accumulate inside the reload. Both are
guesses about how fast the runner is. The consumer now signals on its first
delivery, and the measurement waits until enough publishes have landed.
The "nothing was published" guard goes with them. It was the weaker form of
the same check, and it ran after the fact instead of holding the window open
until there was something to measure.
The assertion demanded zero refusals during a reload, and this ordering
cannot deliver that. GetQueuePrefix returns a wrapper that captured the
adapter, so a producer that fetched before the swap and appends after
Shutdown has begun is still holding the old queue. That window is one call
wide; closing it means resolving the adapter inside Append, which is core's
to change.
What the ordering removes is the sustained window - every producer that
fetches during the wait. Measured on a race-enabled run: 174 of 174 publishes
refused with the old order, 1 of 174 with the new one. The old assertion
therefore failed about half the time on a change that works.
The publishing goroutine was told to stop and never waited for. t.Cleanup
restores sdk.Runtime while a producer that has not yet noticed the stop is
still reading it, which -race reports as a write and a read on the same
package variable. Signalling is not joining.
Shutdown now waits for its consumers to deliver what the queue still holds, and
setupQueue called it first. For that whole wait sdk.Runtime still pointed at the
adapter that had stopped accepting, so every Append landing in the window came
back ErrQueueClosed - and both call sites in common/middleware log that at error
level while the row never reaches the database.
Measured with a held consumer: 177 of 178 publishes during one reload.
Installing first leaves no window. A producer fetches the adapter per call and
gets either the new queue or the old one, and both accept; the old one still
drains, because Shutdown is what waits for that.
The difference only exists during the wait - after Setup returns the two orders
look identical, which is why the test holds a consumer and publishes throughout
the reload rather than checking the state afterwards.
The counter-proof compiles and reports the 177.
It carries the queue shutdown fixes: a closed queue now delivers what it already
accepted and stops the goroutines consuming it. Both matter here, because this
host rebuilds its queue adapter on every configuration reload and shuts one down
on the way out.
Nothing in this commit uses the new behaviour. The one place that has to change
because of it follows.
One fixed key meant two probes overlapping - two /ready requests, or two
instances against the one redis, which is the normal deployment - overwrote each
other's value between the write and the read, and each concluded the cache was
broken. A readiness probe that reports false negatives under load pulls healthy
instances out of the pool, which is worse than not probing.
The key now carries eight random bytes. The written value is still read back,
because that is what tells a healthy cache apart from one that answers "miss"
for everything, and the key is deleted afterwards on a best-effort basis - its
error is dropped deliberately, since the verdict is already decided and a cache
that cannot delete what it just wrote is not a reason to refuse traffic.
The first version of the concurrency test had no teeth: the probes are short
enough that the scheduler ran them one after another, so the fixed-key
counter-proof passed three times out of three. The fake cache now holds every
writer until all of them have written, which makes the interleaving the test is
about actually happen. With one key that is a deterministic 15 failures out of
16 - only the last writer's value survives - and with a key per probe, none.
/health returned 200 without asking anything. Whatever it was meant to say, an
orchestrator reading it learned only that a process was accepting connections.
The two questions are not the same one, and the answers differ:
- /health stays a bare 200. It answers "should I restart you", and a process
whose database is unreachable does not want restarting - that turns one
outage into a crash loop and discards the connection pool, the cache and
every request in flight on the way.
- /ready is new. It answers "should I send you requests", fails while a
dependency is unreachable, and fails from the moment shutdown begins.
That last part is what the life-cycle phases bought. BeginDraining sits next to
BeginShutdown, before the server stops accepting, so a load balancer is told to
stop sending while this instance can still finish what it holds. Reversed - and
that is where it was - the connections are cut first and the probe reports it
afterwards.
The queue is deliberately not checked. Nothing on AdapterQueue answers "are you
reachable" without publishing something, the memory backend cannot fail, and a
queue that is down degrades logging rather than stopping requests: a reason to
alert, not a reason to leave the pool.
The cache probe writes and reads back rather than only reading. A cache that
answers "miss" for every key - a client pointed at the wrong server - is
indistinguishable from a healthy one on a read alone.
Every check runs behind a recover, and that is not defensive habit. The test for
"nothing configured" found the reason: GetCacheAdapter builds a wrapper around
whatever is configured and returns it even when nothing is, so the value is not
nil, the cache inside it is, and Set dereferences it. A nil check cannot see
that, and GetQueueAdapter behaves the same way. Whatever the cause, a probe is
the last thing that should be able to take the process down - the caller is
asking whether this instance is well, and killing it is the wrong reply.
The counter-proof compiles and fails: without the recover, the unconfigured case
panics rather than reporting two failed checks.
Setup installs the cache and queue adapters on sdk.Runtime, which is a
package-level singleton, and records what it installed in this package's own
variables. The test left all of it behind, so what a later test in this binary
saw depended on whether this one had run - the leak cmd/api's freshRuntime and
common/middleware's copy of it exist to prevent.
sdk.Runtime is swapped for a fresh one and everything is put back in Cleanup.
Issue #892 is that a reload replaces the queue adapter and the consumers
registered against the previous one are left attached to a queue nobody
publishes to any more.
The fix has two halves and only one of them was covered. attachQueueConsumers
gives a new queue its own consumers and the same queue none, which cmd/api
tests against a queue it controls by passing generation numbers in by hand. What
nothing asserted is that a reload actually produces a new generation for it to
notice - the half that lives in this package.
Setup is what config re-runs on every change, so calling it twice is what a
reload does here. The generation goes 0, 1, 2.
The counter-proof compiles and fails: dropping the increment in setupQueue
reports that the first Setup installed no queue at all, because a generation
that never moves is indistinguishable from never having been set.
This closes the loop rather than adding coverage for its own sake: with both
halves asserted, the claim that #892 is fixed rests on tests instead of on
reading the two functions and believing they meet.
AfterListen promises a hook that the port answers. The bind was moved onto the
caller's goroutine to keep that promise, but with ssl enabled there was a second
way to fail after the announcement: ServeTLS reads the certificate files itself,
on the serving goroutine, so a bad path or an unreadable key surfaced once the
hooks had already run.
tls.LoadX509KeyPair now runs before anything is announced, and its error is
returned from startServing the way a failed bind is. ServeTLS still does the
real work - handing it a tls.Listener built here instead would take over the
HTTP/2 negotiation it sets up, and quietly drop h2 for every TLS deployment. The
cost is one extra read of the certificate at startup.
The fatal in the serving goroutine said "listen:". Neither the bind nor the
certificate reaches it any more, so it says "serve:".
The test covers the certificate path alongside the bind: neither may announce
the phase, and neither may seal it.
The counter-proof is not clean, and saying so is the point. Removing the check
does turn the run red, but through log.Fatal killing the process from the
serving goroutine - "fatal serve: open no-such.pem: no such file or directory" -
rather than through the assertion. That still demonstrates the defect, because
the process could only get there after startServing had returned successfully
and the phase had been announced; it cannot be observed from inside the test,
because the fatal races the assertion that would report it.
Also: the redis-backed queue tests now fail instead of skipping when CI is set
and GO_ADMIN_TEST_REDIS_ADDR is not. A workflow that renamed the variable or
dropped the service would otherwise stay green while those two tests quietly did
nothing - the same shape as the defect they exist to cover. Locally, with no CI
in the environment, they still skip.
buildRouter is split out of run() so the order it establishes can be asserted:
the phase is announced, then initRouter builds the engine, then runStartupHooks
drains the registries.
The test covers both halves of the distinction the contract draws. A
BeforeRouter callback sees no engine - that is what the phase means, the last
point at which a module can still affect how routes are built. A callback in the
before registry, two lines later, sees one. The names invite treating them as
the same moment and they are not.
No database is involved. AuthInit reads ApplicationConfig.Mode and JwtConfig and
nothing else, and building a router registers handlers rather than calling them,
so the whole sequence runs in a package test with two package-level values set.
freshRuntime swaps the global runtime for the duration: runStartupHooks seals
the registries it drains, and a sealed registry silently drops everything
registered afterwards, which would leave every later test in this binary passing
while proving nothing.
The counter-proof compiles and fails - announcing the phase after initRouter
reports "BeforeRouter saw engine &{...}, want nil".
The rule that consumers are registered before the queue is started had no test
that could fail on the backend it exists for. Everything so far ran on the
memory queue, which is the default: queue.Memory's Register starts another
consumer goroutine whatever the state, so the wrong order passes there. A suite
that only exercises the default reports success for a queue that accepts no
consumers at all.
CI gets a redis service, and two tests build the queue the way setupQueue does -
through config.QueueConfig.Setup, so what is under test is the adapter this
repository actually gets, LegacyQueueAdapter included. They skip without
GO_ADMIN_TEST_REDIS_ADDR, so a developer with no server still gets a green run.
Registering first and starting second delivers the message. Starting first and
registering second is refused: no consumer group was created, so Append comes
back with storage.ErrNoHandler. Pinning that particular error rather than "some
error" is deliberate - the test is about the missing consumer, and a connection
failure that happened to error too would otherwise pass for it.
Running it corrected something written two commits ago. The claim there was
that a late registration loses consumers "with nothing said". Only half of that
holds: the registration is silent, because Register returns nothing, but every
publish afterwards fails loudly - ErrNoHandler, logged at error level by both
call sites in common/middleware - while the log rows are never written. The
symptom is missing rows plus a lot of noise, not a quiet nothing. That commit's
message and the contract doc both say so now.
core's contract says what the four phases promise. This says which line of
cmd/api/server.go each of them is, which is the part an application author
cannot read out of core.
Three things are recorded because getting them wrong is silent:
- BeforeRouter is not the before registry. Those callbacks run from
runStartupHooks, which is after initRouter has built the engine; the phase
is before it. The two are one line apart in the same function and describing
them as equivalent is the mistake this paragraph exists to prevent.
- AfterResource runs again after every configuration reload, so a callback
there is idempotent with respect to a resource rather than doing nothing
the second time. The queue consumers are the worked example, in both
directions: a new adapter must get consumers, the same adapter must not get
them twice. Identity has to come from where the resource is created -
GetQueueAdapter and GetQueuePrefix build a fresh wrapper per call, so two
of them never compare equal however often the adapter underneath changed.
- Consumers must be registered before the queue is started, and which
implementation is behind the interface decides whether that matters:
QueueConfig.Setup hands back queue.NewMemory when there is no redis
section, and that one does not care; a redis section reaches
LegacyQueueAdapter, whose Register cannot report a refusal. The table says
so rather than leaving "only on redis" as a claim. What is silent is the
registration alone - every publish afterwards is refused with ErrNoHandler
and logged at error level, so the symptom is missing log rows plus a lot of
noise, not a quiet nothing.
The third-layer table loses its "queue consumers are lost after a hot reload"
row, which is what AfterResource is for, and gains the honest replacement: the
four phases are the only mount points there are. There is no "after the routes
are installed, before the socket is listening".
AfterListen is described as the port being bound rather than Serve being in its
accept loop. Serve runs on another goroutine and may not have reached the first
Accept; what is true is that the bind returned, so the kernel is queueing
connections. A bind that fails produces no phase at all - the error returns
from run() and the banner never prints.
The per-tenant setup ended with `defer crontab.Stop()` on the line above
`select {}`. The select never returned, so the deferred call was unreachable
for the life of the process: the scheduler had never once been stopped. And
because setup never returned, the `for k, db := range dbs` loop in Setup never
reached its second iteration - with several tenant databases configured, only
whichever one came first out of the map ever got a scheduler at all.
Both fall out of deleting the select, which was blocking for nothing: cron.Start
is `go c.run()` and has never needed anything to hold the caller.
The stop becomes a BeforeExit callback. cron.Stop returns a context that closes
once the jobs already running have finished, so the shutdown budget has
something real to bound - and giving up on that wait leaves those jobs running
until the process exits, which is better than holding the whole shutdown open
for one job that will not end.
Startup moves from a bare goroutine in run() onto AfterListen. Two reasons: the
phase runs behind core's panic guard, which does not reach across a goroutine
boundary, so a panic while loading jobs used to take the process down; and the
jobs it starts can call the API, which is only true once the socket is
accepting. It can be synchronous now precisely because setup returns.
Tested where it can be: startCrontab is split out so a scheduler can be started
with no database in sight. The job runs every second; after RunShutdown, two
and a half seconds of silence is the assertion. The counter-proof - registering
no callback, which is what this commit replaces - compiles and reports "the job
fired 2 more times after shutdown".
There is one test, not several, because BeforeExit closes to further
registration once it has run; a second RunShutdown in the same binary would
find an empty registry and pass while proving nothing.
**The multi-tenant half has no test.** setup needs a *gorm.DB per tenant before
it reaches the line that was blocking, and this repository's CI has no database
- `make build` is CGO_ENABLED=0 with no sqlite tag. It is the same defect
though: the loop could not advance past a call that never returned.
setupQueue ended with `go queueAdapter.Run()`, and the three log consumers were
registered afterwards, from setup(). The contract implementations refuse a
registration once the queue is running - memqueue and the redis queue both
answer storage.ErrQueueAlreadyStarted - and Register cannot report it: it
returns nothing, which its own comment in core records as the reason the
interface is deprecated. Start first and register second, across two
goroutines, and the registration is dropped without the caller being able to
tell.
What follows is not quiet. No consumer group was created, so redis refuses
every later publish with storage.ErrNoHandler, and go-admin logs that at error
level from both call sites while the login and operation log rows are simply
never written. The silence is in the registration; the cost shows up on every
request after it.
Which implementation is behind the interface depends on the configuration.
config.QueueConfig.Setup returns queue.NewMemory directly when there is no
redis section - and that one does not care about the order, because its
Register just starts another consumer goroutine. Only a redis section reaches
storage.LegacyQueueAdapter, which wraps the contract implementation and
therefore refuses. So the defect is invisible in the default deployment and
shows up only where redis is configured, dropping the login log, the operation
log and the api check - the three things #892 was about.
The start therefore moves to the code that registers, and nothing starts the
queue but that.
The registration also moves onto AfterResource. It has to: a reload rebuilds
the adapter, and consumers attached to the one that existed at start-up are
attached to a queue nobody publishes to any more. Being on that phase means
running again on every reload, so the callback is idempotent with respect to a
given queue rather than "does nothing the second time" - registering twice on
the same queue would give every message two consumers and write every log row
twice.
Identity for that comes from common/storage, where the adapter is built, as a
generation counter. It cannot come from the accessors: GetQueueAdapter and
GetQueuePrefix build a fresh runtime.Queue wrapper on every call, so comparing
two of them compares two wrappers and never matches however many times the
adapter underneath has been replaced. A counter also keeps the comparison on a
uint64 rather than an `==` between two interface values, which would panic on
an adapter type that is not comparable.
Generation 0 means the configuration has no queue section, so nothing was
installed and the runtime hands back its own memory queue. That case still gets
consumers, because the registration this replaces was unconditional and
dropping it would stop the logs for anyone who commented the section out.
Two things fixed on the way past:
- `if q := sdk.Runtime.GetQueueAdapter(); q != nil { q.Shutdown() }` was
always true. GetQueueAdapter never returns nil - with nothing configured
the runtime falls back to its own memory queue and wraps that - so the
first start shut down the fallback queue before anything had used it. Only
an adapter this package installed is shut down now.
- config.Setup becomes bootstrap.SetupConfig, which is what announces
AfterResource, and announces it after the callbacks that build the
resources rather than before.
attachConsumersOnce is split out so the order and the once-ness can be checked
against a queue the test controls; neither can be read back out of a real
adapter. Four tests cover the ordering, both directions of the idempotency
rule, and the unconfigured case. Both counter-proofs compile and fail: calling
Run before the registrations reports each of the three as "came after Run", and
dropping the generation guard reports eight calls where four are wanted.
One honest limit: the counter-proof for the ordering makes Run synchronous.
The original arrangement started the queue on another goroutine, and a race
cannot be made to fail every time - which is the reason the order is enforced
by structure here instead of being left to be noticed in use.
A module can now register cleanup and have it happen. Until this commit the
process stopped serving and returned; anything a module had set up went down
with the process rather than being taken down.
BeginShutdown is said first, before anything is dismantled. Without it a
configuration reload arriving in this window re-runs AfterResource - rebuilding
the pool and the queue adapter and re-registering consumers - on top of cleanup
that has already run.
The cleanup runs whether or not Shutdown reported an error, which is the whole
reason that error stopped being fatal in the first place: Shutdown fails exactly
when connections were still in flight, and that is when there is most left to
take down.
The two budgets are spent one after the other, so what has to fit inside the
orchestrator's grace period is their sum. `docker stop` allows 10s by default
before SIGKILL; 5+3 leaves room to finish returning. Raising one without
lowering the other buys nothing.
Both halves are tested through the existing subprocess child, which now
registers a BeforeExit callback of its own:
- after a Shutdown that timed out, the callback still runs. Moving the call
into the success branch reports "the BeforeExit callback did not run after
a failed Shutdown".
- a callback that outlasts its budget is abandoned, not awaited. It sleeps
two seconds against a 300ms budget; RunShutdown reports the deadline, the
process exits cleanly inside one second, and the callback's own marker
never appears. Widening the budget to five seconds makes the test time out
waiting for the exit, which is what "awaited" looks like.
Both counter-proofs compile and fail.
Two phases are now announced from the command that serves traffic, so a module
can attach to them instead of being called by name from here.
The listener is opened by this goroutine rather than left to ListenAndServe,
which binds on the goroutine that serves. That mattered for the phase: a hook
on AfterListen is promised a reachable port, and with the bind happening out of
sight there was no way to keep that promise - "address already in use" surfaced
on a goroutine nobody read, after the banner had already announced the server
was up. It is now returned from run() and the process exits non-zero without
claiming anything.
AfterListen is announced synchronously. Moving it to a goroutine to save the
few milliseconds would let it overlap the shutdown, and on a fast SIGTERM the
cleanup callbacks could finish before the startup ones did.
What is left in the serving goroutine is still log.Fatal, deliberately: the
bind is no longer among the errors that reach it, so what remains is a serve
that failed after the port was taken, and carrying on would park the process on
<-quit with nothing serving. ServeTLS is the one case that can still fail
immediately, since it reads the certificate files - with ssl enabled a hook can
still run against a server on its way down. That is not a regression (the old
code printed the banner in the same situation) and it is not fixed here.
BeforeRouter is placed before initRouter, which is a different moment from the
before registry runStartupHooks drains: those callbacks run after the engine
has been built, not before it.
AfterListen is tested here, in one test rather than two because the phase seals
itself once it has run: a second test would find a closed registry and pass
while proving nothing. Both counter-proofs compile and fail - announcing on a
failed bind reports "AfterListen ran 1 times after a failed bind", and
`go RunPhase(...)` reports "ran 0 times, want 1" against the hook's own pause.
BeforeRouter's placement is not asserted in this commit. The test for it comes
with the buildRouter extraction later in this branch.
It carries the life-cycle phases and the shutdown registry. Nothing here uses
them yet - the wiring is the commits that follow, and keeping the bump on its
own means a bisect can tell "the dependency moved" apart from "the host
started calling into it".
The comment claimed bindings were collected as the body was walked so that a
registration only saw definitions above it. That is the single-pass design this
started as. The code does two passes - one to collect, one to report - and a
registration therefore sees every binding in the function.
That is the point rather than an accident: a `.Use` written below a route is
still part of the chain, because the chain is assembled before anything is
served. The price is that a name reused for two different things in one
function resolves to the last assignment, which the comment now says instead of
promising an ordering the code does not keep.
GetPermissionFromContext cannot fail. When no middleware put a *DataPermission
in the context it returns the zero value, and the zero value's DataScope is the
empty string - which is not one of the five scopes Permission recognises, so it
takes the default branch and fails closed. The query is handed `1 = 0` and
matches nothing.
The endpoint then reports "not found" or "no permission" for rows that plainly
exist, and only where enabledp is true. With data permissions off - the
repository default - Permission returns the query untouched and the missing
middleware costs nothing at all. A test suite and a CI that run on the default
cannot see it.
That is what happened to /api/v1/getinfo: it read the permission on a group
carrying only the JWT middleware, so every login on a deployment with data
permissions enabled ended in a 401 from the endpoint the browser calls
immediately after signing in, and went back to the login page. Three /sys-api
routes had the same shape.
The check matches a handler to the group it is registered on, through the AST
rather than through the text - a scratch grep for the same thing reported four
false positives from a comment that happened to contain the function's name,
and before that, a dozen from matching handler names across packages. Handlers
are keyed by package, type and method, so two SysUser types are two handlers.
Subgroups inherit their parent's chain, as gin does, and a `.Use` written below
a registration still counts, because the chain is assembled before anything is
served.
Either half is a fix and the message says both, because which one is right
depends on the route. A handler reading other people's rows wants the
middleware. A handler reading the caller's own row - id from the token - wants
no scope at all: DataScopeSelf matches on create_by, so scoping a self-read
rejects every user who did not create their own account. Reporting only "add
the middleware" would have turned /getinfo from broken into worse.
Five tests: the mistake, both fixes, subgroup inheritance, and a same-named
handler in another package. TestThisRepositoryIsClean covers the real tree, and
it is what fails on the commit before this one - four findings, all real.
Logging in on a deployment with enabledp: true ends on the login page. The
login itself succeeds - sys_login_log records it - and then /api/v1/getinfo
answers 401 "登录失败", which sends the browser straight back.
The query behind it reads:
SELECT * FROM sys_user WHERE sys_user.user_id = 1 AND 1 = 0 AND deleted_at = 0
The 1 = 0 comes from the data-permission scope. GetInfo asked for a permission
with GetPermissionFromContext, but the group this route sits in installs only
the JWT middleware - no PermissionAction - so nothing ever put one in the
context and what came back was the zero value. An unset scope is not one of the
five recognised ones, and since unknown scopes began failing closed rather than
silently matching every row, that zero value now means "match nothing".
The route was working by accident before, and only on deployments that enable
data permissions: the repository default is enabledp: false, where Permission
returns the query untouched. That is why the local suite and CI are both green
and the demo site is not.
Two different faults, so two different fixes:
/getinfo reads the caller's own row - the id comes from the token. A data
scope answers "whose rows may this user see", so there is nothing left for it
to restrict, and applying one is not a stricter version of the query but a
broken one: DataScopeSelf matches on create_by, and an account is created by
whoever added it, so a scoped self-read would 401 every user who did not create
their own account. It now goes through GetSelf, which does no scoping at all -
which is how GetProfile has always read the same row.
/sys-api is the opposite case. Its three handlers do read the permission, and
they are listing and updating other people's rows, so the middleware belongs
there and was simply missing. Added.
Those four endpoints were found by checking every handler that reads the
permission against the group it is registered on. The check reports four before
this commit and none after.
No test. Both paths need a *gorm.DB with sys_user and sys_role rows before they
reach the line that matters, and this repository's CI has no database - `make
build` is CGO_ENABLED=0 with no sqlite tag. What can be tested is the shape of
the mistake rather than its effect, and that belongs in tools/checksilent as a
rule of its own; it is not in this commit because a site that cannot be logged
into should not wait for it.
Moving the dial to just before the shutdown removed one flake and introduced
another: Shutdown only waits for connections the server has already accepted,
so calling it in the gap between the dial and the accept finds nothing to
wait for and returns cleanly. The test then fails on its own "this proves
nothing" guard - which it did, after passing once.
A ConnState hook closes both gaps deterministically. The connection is opened
late enough not to age past the five seconds net/http stops counting it at,
and the child does not proceed until the server has taken it off the
listener.
Ran five times in a row rather than once, because a single green run is what
made the previous version look fixed.
Claude-Session: https://claude.ai/code/session_01HPTAw8b8tAdFNFn8rKdPYx
net/http stops counting a StateNew connection against Shutdown once it is
more than five seconds old. The connection was opened when the child started
and the parent then waited for readiness before signalling, so on a slow run
the connection could age past that mark and Shutdown would succeed - and the
test would fail on its own "this proves nothing" guard rather than on the
behaviour it is there to pin.
Opening it immediately before the shutdown keeps the timeout deterministic.
Claude-Session: https://claude.ai/code/session_01HPTAw8b8tAdFNFn8rKdPYx
The previous commit split arming from waiting so a caller could arm first,
wrote a comment saying a signal landing in between reaches the default
handler and kills the process, used it that way in the subprocess test - and
then left run() calling the combined helper after the whole readiness banner.
The window it warned about was still there in the one place that ships.
The signals are now armed before the server starts serving, and the wait
happens where it did. The disposition is restored right after the first
signal rather than deferred, so a shutdown that hangs can still be
interrupted by a second one.
waitForStopSignal goes away: run() was its only caller, and what was worth
keeping from its comment is now on armStopSignals.
Note that no test covers this ordering. The subprocess test drives
armStopSignals directly, which is what makes it a test of the mechanism
rather than of run(); moving the call back below the banner leaves it green.
Verified by reading the sequence in run(), not by a failing test.
Claude-Session: https://claude.ai/code/session_01HPTAw8b8tAdFNFn8rKdPYx
Three defects on one path, none of which could be seen from the code alone.
**SIGTERM was never registered.** signal.Notify listened for os.Interrupt
only, and Go terminates the process outright for a signal nothing handles.
`docker stop`, a Kubernetes pod deletion and `systemctl stop` all send
SIGTERM, so every line of the graceful shutdown below the wait was dead code
outside a terminal: measured on a real binary, SIGINT printed "Shutdown
Server ..." and "Server exiting" and SIGTERM printed neither.
**A stuck shutdown could not be interrupted.** quit is buffered and
signal.Notify stays armed after the first delivery, so further signals only
refill the buffer. That was harmless while SIGTERM went to the default
handler - it was the escape hatch. Registering it removes the hatch, so the
disposition is now restored once the first signal is taken, and a second one
kills the process the default way. Arming is split from waiting so a caller
can arm before it announces readiness; a signal in between reaches the
default handler, which is the very failure being fixed.
**A failed Shutdown skipped everything after it.** log.Fatal is an
unconditional os.Exit(1), and Shutdown reports an error precisely when
connections were still in flight - the moment the cleanup that follows
matters most. It is an error now, and the process carries on.
That failure is closer than it looks. net/http only treats a StateNew
connection as idle once it is over five seconds old, so a connection opened
shortly before the signal that has sent nothing holds the whole budget: with
the shipped settings.yml (readtimeout 1) the server closes it first and
shutdown takes 5ms, but with settings.demo.yml (readtimeout 10000) the same
connection made shutdown take 5.04s and exit 1, printing no "Server
exiting". The default configuration is what has been hiding this.
The wait and the shutdown are extracted so the subprocess tests can drive the
real functions against an empty http.Server: CI has no database, and none of
this needs one.
Claude-Session: https://claude.ai/code/session_01HPTAw8b8tAdFNFn8rKdPYx
The note explained the empty-shim summary as the expected state "until the
contract packages are lowered into core". They are lowered, and the shims
exist - so a count of zero now means they stopped being aliases, or stopped
being here, which is the interesting case rather than the ordinary one.
Claude-Session: https://claude.ai/code/session_01HPTAw8b8tAdFNFn8rKdPYx
The contract-shim-alias message built its suggested line from path.Base of
the import path, so it told the author to write
type ControlBy = models.ControlBy
in a file whose import is `contractmodels "…/sdk/contract/models"`. Every
shim in this repository aliases that import, so the suggestion never
compiled as written - in the one message whose whole job is to be pasted in.
qualifiedType already read the in-source identifier to resolve the import;
it now returns it.
Claude-Session: https://claude.ai/code/session_01HPTAw8b8tAdFNFn8rKdPYx
Widening menu-sort-overflow to see a contract MenuSpec made it fire on
app/admin/service/seed_test.go, on the case that asserts SeedMenus rejects
a sort of 900. The check was reading the proof that it works as a defect.
That is not specific to this one guard: menu-sort-overflow,
config-value-truncation, menu-id-collision and modeltime-mix are all about
a value that reaches a real database through a migration, a test fixture
reaches none, and every one of those guards needs a test that writes the
value it rejects. Skip _test.go in all four.
The two import and alias checks keep scanning tests - those are about the
dependency graph, where a test file's import is as real as any other, and
TestContractImportBoundaryCoversTestFiles already pins that.
Both directions are covered: a fixture is ignored, a real seed is still
reported.
Claude-Session: https://claude.ai/code/session_01HPTAw8b8tAdFNFn8rKdPYx
The naming rule was documented; what happens when it is broken was not.
It now panics naming the offending file, which is worth saying out loud
because the alternative it replaced was silent: a name that is not a
timestamp used to register as its own version, and that migration would
never run and never report anything.
Claude-Session: https://claude.ai/code/session_01HPTAw8b8tAdFNFn8rKdPYx
Sort has an upper bound: sys_menu.sort is built as a tinyint, sqlite ignores
the width, and an overflow surfaces as Error 1264 partway through a migration
rather than as a rejected value.
MenuSpec carries no menu name, and the host synthesises one from the app code
and the spec code rather than using Code directly - two applications both
choosing "list" would otherwise share a keep-alive cache key on the frontend.
Nothing in the type says so, and every Seeder implementer would have to
rediscover it.
Claude-Session: https://claude.ai/code/session_01HPTAw8b8tAdFNFn8rKdPYx
The sort-overflow check recognised a SysMenu literal from this repository's
model packages and nothing else. An application installed from outside cannot
reach that type - it describes the same row as a seed.MenuSpec and hands it to
the host's Seeder - so the check went quiet for exactly the author furthest
from the schema it protects.
Not hypothetical: this repository's own reference application shipped a Sort
of 200, past the tinyint sys_menu.sort is built as, and this check passed it.
sqlite ignores the width, so it would have surfaced first on a real install,
as Error 1264 partway through a migration with everything after it unapplied.
The check still cannot see an application in the module cache; that half is
the Seeder's runtime validation. This closes the half that is in the tree.
Claude-Session: https://claude.ai/code/session_01HPTAw8b8tAdFNFn8rKdPYx
Two of the three bullets on the contract surface disagreed with
docs/contract.md and with core's own. Registration is constrained by
ordering - it must happen before the startup hooks run - not by being
written inside `init()`; and the claim that core's setters take no lock
is not true of them. The check table gains the new alias rule.
Claude-Session: https://claude.ai/code/session_01HPTAw8b8tAdFNFn8rKdPYx
The list of stable packages named four packages of this repository, on
the stated grounds that deduplicating app/demo's imports produces
exactly those four. That reasoning was wrong in the one direction that
matters: it sends a third-party author to depend on the host, and the
host is a fork that every user edits. `go-admin` is also not a
resolvable module path - it has no dot in its first element - so an
application cannot require it at all without a replace directive, which
is ignored outside the main module.
Rewritten around what core promises instead, and around a different
question: not "which packages does an application import" but "which
conventions fail without saying anything". Those are now spelled out
one by one, each with the mechanism that makes it silent - the response
envelope the frontend reads by `code`, the tenant-scoped connection,
`create_by` and the soft-delete marker, the data-scope middleware, the
transaction shape, and the `apps/` prefix a packaged application's menu
component must carry.
Also states two things the document was missing: installing an
application means trusting it with the host's database connection, at
the same level of trust as importing any other Go package - there is no
sandbox here and this does not pretend otherwise - and wiring an
application in touches two places, not one, where missing the second
means the migrations simply do not run, with no error.
Claude-Session: https://claude.ai/code/session_01HPTAw8b8tAdFNFn8rKdPYx
A shim of a core contract type written as `type X pkg.Y` instead of
`type X = pkg.Y` keeps the fields and drops the method set, so anything
embedding it stops satisfying the interfaces it satisfied before.
The compiler catches that only where the method set is actually
exercised. This repository exercises some of the contract types through
an interface and some not at all, so the ones it does not exercise
compile here and break in a fork or a third-party application - which
is the half nobody is watching.
The trigger is the right-hand side of the declaration rather than a
list of package names, so it covers whatever the lowering ends up
shaping without a list to keep in step. Until the contract packages
land in core there is nothing here to guard, and the summary says so
rather than letting the silence read as a pass.
Claude-Session: https://claude.ai/code/session_01HPTAw8b8tAdFNFn8rKdPYx
The comment called Visible "0" "hidden by default" and then said an
administrator should not have to unhide the menu - which cannot both be
true. "0" is shown; every menu this repository seeds, including the demo
product menu that is visible on the demo site, uses it.
Claude-Session: https://claude.ai/code/session_01HPTAw8b8tAdFNFn8rKdPYx
grantToAdminRole does two independent things - it grants the menus to the
admin role and writes a casbin rule per api - and SeedMenus skipped the
whole call whenever the menu list came back empty.
An application is free to register apis with no menus: endpoints another
service calls, a webhook, a UI mounted somewhere else. Those installs wrote
their sys_api rows and then no casbin rule for any of them, so every one of
those endpoints was denied to everyone, admin included - from a migration
that reported success and left rows in the table to prove it had run. There
is nothing to look at afterwards that says what went wrong.
Guard on both lists instead, so nothing registered stays a no-op and apis
alone still get granted.
Claude-Session: https://claude.ai/code/session_01HPTAw8b8tAdFNFn8rKdPYx
The host kept its own copy of the version-naming rule, byte-identical to
the one in contract/migration: slice the leading 13 characters, no check.
Two copies of a convention that applications also have to follow is two
things to keep in step, and the copies had already stopped matching - core
now rejects a name that carries no timestamp, and this one still accepted
"add_orders.go" and registered a migration under that string as its
version, which nothing would ever match and nothing would report.
Delegate instead, so there is one implementation of the rule and an app's
migration and a host migration derive their version the same way.
The test pins the reject case, not just the happy path: a re-divergence
that only sliced would still pass the happy path.
Claude-Session: https://claude.ai/code/session_01HPTAw8b8tAdFNFn8rKdPYx
core's seed package defines what an application may ask for and leaves the
writing to the host, which is the only side that knows its own tables. No
host implemented it, so SeedMenus returned ErrNoSeeder and an application's
menus never appeared.
adminSeeder writes all four kinds of row, not the two an obvious reading
would stop at: without sys_menu_api_rule and the sys_role_menu / casbin_rule
grants, the menu exists and no role can reach it.
Ids are always autoincrement, never caller-assigned - checksilent's
menu-id-collision check reads literals in this repository's tree and cannot
see an application in the module cache, so the collision is removed by
construction instead of guarded. The runtime validation covers what a static
scan cannot reach for a third-party spec: duplicate codes, unresolved parents
and api references, an unknown kind, and a sort outside sys_menu.sort's
tinyint range.
MenuSpec carries no menu name, so one is synthesised from the app code and
the spec code - two applications both choosing "list" would otherwise collide
on the frontend's keep-alive key.
It lives in app/admin/service because cmd links both subcommands into one
binary, so its init runs whichever one is invoked, and cmd/migrate never has
to import app/admin to reach it.
Claude-Session: https://claude.ai/code/session_01HPTAw8b8tAdFNFn8rKdPYx
sys_migration already carries app_code; sys_menu and sys_api did not, so
nothing said which application seeded a row - which is what an uninstall or
an audit would have to ask.
The migration adds the columns through the runtime models rather than
cmd/migrate/migration/models, whose frozen ModelTime is wrong for anything
ordered after the soft-delete conversion.
Claude-Session: https://claude.ai/code/session_01HPTAw8b8tAdFNFn8rKdPYx
core's sdk/contract/migration keeps its own process-wide registry, because
that is the only door open to an application that must not import the host.
Nothing here ever opened it: ForApp("crm").SetVersion(...) compiled,
registered, and then never ran - no error, no mention in status, nothing.
mergedEntries unions the host's own registry with contract/migration's
Snapshot(), and status, run and AppCodes all read through it, so migrate,
status, --dry-run and --app see an application's migrations exactly as they
see the host's. Version namespacing already keeps the two apart, so a key
collision should not be reachable; the host's own registration wins if one
ever is, rather than being silently replaced.
Claude-Session: https://claude.ai/code/session_01HPTAw8b8tAdFNFn8rKdPYx
The comment said the test had to be declared first because Go runs a
package's tests in source order. That is not a guarantee, and it is not what
makes this work: the test that registers RoleCheck puts it back in a
t.Cleanup, and the guard here turns a wrong order into a loud failure rather
than a silent pass. Verified with go test -shuffle on seeds that run the two
in either order.
Claude-Session: https://claude.ai/code/session_01HPTAw8b8tAdFNFn8rKdPYx
The Create handler's @Param named dto.OrderCreateReq, but this file imports
that package as orderdto. swag stops on it:
ParseComment error ... cannot find type definition: dto.OrderCreateReq
The @Success annotations name models.Response, which resolves - through
--parseDependency - to core's sdk/contract/models.Response rather than to
this package. That is the right envelope, and worth a note next to the
import, because the obvious "correction" is wrong: core's response.Response,
which the framework's own handlers name, carries no data field, so switching
to it would document these endpoints as returning no payload.
Claude-Session: https://claude.ai/code/session_01HPTAw8b8tAdFNFn8rKdPYx
sys_menu.sort is `gorm:"size:4"`, which MySQL builds as a tinyint holding
-128..127. Sort: 200 passes every sqlite test - sqlite ignores the width -
and fails on a real install with Error 1264, partway through a migration.
This is the exact incident class checksilent's menu-sort-overflow check
exists to prevent, and it reached a hand-written deliverable anyway: that
check only recognises a SysMenu literal from the host's model packages, so
a seed.MenuSpec is invisible to it. Widening the check is tracked
separately; this is the value it would have caught.
Claude-Session: https://claude.ai/code/session_01HPTAw8b8tAdFNFn8rKdPYx
Registration goes through core's package-level facades: SetAppRouters for
the routes, migration.ForApp for the schema, and seed.MenuSpec/ApiSpec
for the menu rows - none of which requires importing the host.
The menu component is spelled apps/order/order/index. The frontend tells
a packaged view from a built-in one by that first segment alone, and
getting it wrong is silent: the page falls back to the not-installed
placeholder while the console names a src/views path that was never going
to exist. The tests assert that prefix, that every Parent reference
closes, and that every ApiCode resolves - the three ways a menu graph is
wrong without anything saying so.
Claude-Session: https://claude.ai/code/session_01HPTAw8b8tAdFNFn8rKdPYx
No generic CRUD action anywhere: real business - a cross-table order
placement, a payment transition - is what the contract surface has to
carry, and the actions cover only the single-table case that a real
application outgrows immediately.
The transaction is Orm.Transaction(), not the Begin/defer shape that
app/admin/service/sys_role.go and three other files use. That shape
commits a half-written transaction when the body panics, because the
deferred check reads err, which a panic leaves nil.
The payment transition guards concurrency through the update itself -
WHERE status = 'pending' plus RowsAffected - rather than a read followed
by a write.
The tests cover both rollback paths, because they fail differently: a
mid-transaction error returns, a panic unwinds - and the second is what
tells Orm.Transaction() apart from the shape it replaces. The concurrency
test pins the pool to one writer so sqlite's own single-writer semantics
cannot stand in for the guard being tested. The data-scope tests assert
the fail-closed direction too: an unrecognised scope must return no rows
rather than every row.
Claude-Session: https://claude.ai/code/session_01HPTAw8b8tAdFNFn8rKdPYx
A reference application for a third-party author: its own module, and a
require list that names go-admin-core and nothing else. The point of the
example is that constraint - an application that reaches for the host
cannot be installed through a module proxy at all, because `go-admin` has
no dot in its first path element and a replace directive is ignored
outside the main module.
Two tables rather than one, because a single-table example proves only
what the generic CRUD actions already proved. The interesting question is
whether the contract surface holds up for business that spans tables.
The table names carry an app_ prefix: "order" is a reserved word, and
Permission() interpolates the table name into raw SQL without quoting.
Claude-Session: https://claude.ai/code/session_01HPTAw8b8tAdFNFn8rKdPYx
Two properties the previous shape broke silently: GetHandlerFunc must
report ok for the JwtToken key, and every module must read back the same
instance. Reverting the registration to the unbound method expression
still compiles and turns the first of these red, which is the failure
this pins.
Claude-Session: https://claude.ai/code/session_01HPTAw8b8tAdFNFn8rKdPYx
Four modules each called AuthInit and built their own instance, so which
one Runtime handed back was decided by whichever module initialised last.
The JwtToken key was also registered as an unbound method expression,
which GetHandlerFunc's type assertion can never match - the key was
registered and unusable at the same time.
The instance is now built once here and registered as a bound closure.
Modules read it back through GetAuthMiddleware, which is fatal rather
than nil when called before InitMiddleware has run: a process without a
JWT middleware should not reach the point of serving a request.
Only one call site needs the instance itself rather than the handler
(admin's /login, for LoginHandler); the thirty-odd MiddlewareFunc() call
sites are unchanged.
Claude-Session: https://claude.ai/code/session_01HPTAw8b8tAdFNFn8rKdPYx
IndexAction closes over one dto.Index and serves every request to the route
from it; Generate exists so each request gets its own instance, and every
implementation in this repository returns a copy for that reason. The probe
returned the receiver, which made it the one shape IndexAction is not
written against - and inconsistent with probeRow in the same file, which
already copied.
A single-request test cannot tell the two apart, so the assertion is on
Generate itself rather than on the action's behaviour.
Claude-Session: https://claude.ai/code/session_01HPTAw8b8tAdFNFn8rKdPYx
create.go/delete.go/index.go/update.go/view.go were not lowered to
core (PRD 006 F3) and still call actions.Permission directly, in this
repository, on a code path core's own test suite knows nothing about:
core pins down what Permission builds for a given scope, but nothing
covered whether this package's five Actions still remember to call it
at all. TestIndexActionAppliesDataPermission runs IndexAction exactly
as a real request would, against a real in-memory database, and
inspects the SQL GORM actually executed - not just that the handler
returned success, which it would just as happily do with the filter
missing entirely.
The SQL is captured through a gorm.io/gorm/logger.Interface wrapper
rather than read back from IndexAction's own *gorm.DB: IndexAction
builds and executes its query in one unbroken chain
(Model().Scopes().Find()...Count()) and never hands the built
statement back to its caller, so there is nothing else to inspect it
through.
Counterproof performed and reverted (not part of this commit): with
Permission(object.TableName(), p) removed from IndexAction's Scopes
call, the test failed with the captured SQL carrying no WHERE clause
at all (`SELECT * FROM action_probe_row LIMIT 10`); index.go was then
restored to its committed content (`git diff --exit-code` verified
clean).
Claude-Session: https://claude.ai/code/session_01HPTAw8b8tAdFNFn8rKdPYx
DataPermission, PermissionAction, Permission, GetPermissionFromContext,
IsValidDataScope, PermissionKey and the five DataScope* constants now
forward to go-admin-core's sdk/contract/actions, which carries the
already-fixed logic from feat/006-security-prereq (PRD 006 F14/H1-H3).
create.go/delete.go/index.go/update.go/view.go - the five generic CRUD
actions - are untouched: they call Permission and
GetPermissionFromContext by the same names, which now resolve to
forwards with identical behaviour, and stay in this package rather
than moving to core (PRD 006 F3: core's exports are a permanent
promise every fork inherits, and CRUD shape is this framework's most
volatile surface).
PermissionKey is declared as `const PermissionKey =
contractactions.PermissionKey`, a direct reference rather than a
restated literal, per PRD 006's hard constraint 4: PermissionAction
sets this gin context key and GetPermissionFromContext reads it back,
and an independently declared copy could silently drift from core's if
one were ever edited without the other. permission_test.go replaces
the detailed data-permission regression suite - which now lives in
core, next to the logic itself - with a test of this package's own
wiring: that PermissionAction and both of this package's own read
paths (GetPermissionFromContext, and c.Get(actions.PermissionKey)
directly) still meet on the same key.
Counterproof performed and reverted (not part of this commit): with
PermissionKey redeclared here as the literal "dataPermission" and
core's copy changed to a different value, TestPermissionKeyMatches-
WhatPermissionActionSets went red while GetPermissionFromContext's own
round-trip stayed green - confirming the exported constant, not the
GetPermissionFromContext wrapper, is what an independent literal would
put at risk.
PRD 006 F3/F5.
Claude-Session: https://claude.ai/code/session_01HPTAw8b8tAdFNFn8rKdPYx
AutoForm, ObjectById/ObjectGetReq/ObjectDeleteReq, Pagination,
GeneralDelDto/GeneralGetDto and Index/Control are now type aliases of
go-admin-core's sdk/contract/dto; OrderDest, MakeCondition and
Paginate forward to the same package (functions cannot be aliased the
way types can).
MakeCondition no longer reads common/global.Driver to choose which SQL
dialect to resolve search tags against. The lowered version reads
db.Dialector.Name() from inside the closure it returns instead, which
is always the driver the caller's own *gorm.DB is bound to - correct
even with more than one database open with different drivers, which a
single process-wide variable could never be. global.Driver is marked
Deprecated accordingly; it is still set and still readable for fork
code that reads it directly.
PRD 006 F2/F5.
Claude-Session: https://claude.ai/code/session_01HPTAw8b8tAdFNFn8rKdPYx
ControlBy, Model, ModelTime, ActiveRecord, BaseUser, Response, Page,
Migration and the menu type constants now read `type X = pkg.X` /
`const X = pkg.X` against go-admin-core's sdk/contract/models instead
of defining these shapes locally. Every embed, GORM tag and JSON tag
is unchanged - a type alias is the same type, not a new one - and
every existing import of go-admin/common/models keeps compiling with
no changes of its own (verified with `git diff --exit-code` over the
70 files that import common/models, common/dto or common/actions).
The menu type constants (Directory/Menu/Button) are declared as direct
references rather than restated literals: an independently written
copy of the same value can be edited out of step with go-admin-core's,
where a direct reference cannot (PRD 006 hard constraint 4).
PRD 006 F1/F5.
Claude-Session: https://claude.ai/code/session_01HPTAw8b8tAdFNFn8rKdPYx
The privilege-escalation tests skipped themselves when opening the in-memory
database or running AutoMigrate failed. Both depend on nothing outside the
process, so a failure there means the environment is genuinely broken - and a
security regression that quietly does not run is worse than one that is
missing, because CI stays green either way.
Claude-Session: https://claude.ai/code/session_01HPTAw8b8tAdFNFn8rKdPYx
`NULL NOT IN (...)` evaluates to NULL rather than true, so the previous
condition left a NULL data_scope exactly as it found it - and NULL is the one
value that most needs the repair: it scans into a Go string as "", which is
what the fail-closed default now refuses.
The column has no NOT NULL constraint, so the value is reachable from any
writer that is not the admin UI.
Claude-Session: https://claude.ai/code/session_01HPTAw8b8tAdFNFn8rKdPYx
app/admin/models/datascope.go carried a second copy of the scope logic with no
callers. Its department-tree pattern was written as "%" + id + "%" instead of
"%/" + id + "/%", so dept_id 1 also matched /11/, /21/ and /100/ - visibility
into unrelated subtrees.
It sat where someone looking for a data permission example would find it. The
copy that is actually wired up stays in common/actions.
Claude-Session: https://claude.ai/code/session_01HPTAw8b8tAdFNFn8rKdPYx
The seed fix only reaches new installations. An install that imported the old
db.sql already has an administrator with an empty data_scope, and after the
fail-closed change that account sees nothing.
The migration rewrites any value outside "1".."5" to "1", which is the
behaviour those rows had before. Plain SQL rather than the frozen migration
models, per the rule that migrations after 1786700003000 must not use them.
Claude-Session: https://claude.ai/code/session_01HPTAw8b8tAdFNFn8rKdPYx
The shipped seed data left data_scope empty for the built-in administrator.
That was harmless while an unrecognized scope meant "see everything"; with the
previous commits it means the opposite, so a fresh install with data permission
enabled would have blinded its own default account on every list endpoint.
The admin short-circuit does not help here: role_key == "admin" bypasses Casbin,
not the data permission scopes, which never look at role_key.
The value is "1" - all data - which is the behaviour the empty string used to
produce, so this restores the intent rather than tightening it. A test reads
both seed files back so the pair cannot drift apart again.
Claude-Session: https://claude.ai/code/session_01HPTAw8b8tAdFNFn8rKdPYx
Nothing checked what went into sys_role.data_scope, so creating a role without
a dataScope stored an empty string - the value that used to be indistinguishable
from "see everything".
All three DTOs that write the column are validated, not just the insert path:
they target the same column, and guarding one entrance while leaving two open
would not be a guard.
Claude-Session: https://claude.ai/code/session_01HPTAw8b8tAdFNFn8rKdPYx
A table over all five scopes plus the ones that are not scopes, asserting the
generated SQL rather than a boolean, because the defect was that two different
intentions produced the same query.
The rows that matter are the negative ones: an unrecognized value, a zero
value, and a department scope with a non-positive id. Each was verified to go
red with its own fix reverted and the others in place, so a regression names
the defect it belongs to.
Claude-Session: https://claude.ai/code/session_01HPTAw8b8tAdFNFn8rKdPYx
dept_path is built as "/0/" + id + "/..." for every department, so a DeptId of
0 turns the department-tree pattern into '%/0/%' - which matches every row in
sys_dept. The scope meant to narrow visibility to one subtree returned the
whole organisation instead.
Both department scopes now refuse a non-positive id rather than building a
pattern from it. The admin DTO validates deptId, but seed scripts, SSO and
third-party registration paths do not, and after the contract move the caller
is no longer ours to control.
Claude-Session: https://claude.ai/code/session_01HPTAw8b8tAdFNFn8rKdPYx
The switch ended in `default: return db`, which is the same answer as "this
role may see everything". That made a legitimate scope indistinguishable from a
broken one: "1" (all data) had no case of its own and fell into default too, so
"1", "", "6" and a zero value all produced byte-identical SQL.
Three changes, in this order, because reversing them would break "1":
- the five scope values become named constants, so a reader can tell which
string means what without consulting the seed data
- "1" gets an explicit case, which is what frees default to mean "not a
scope I recognise"
- default now matches nothing rather than everything
SysRole DTOs accept the scope unvalidated, so an empty string reaches this
switch from ordinary use, not just from a corrupted row.
Claude-Session: https://claude.ai/code/session_01HPTAw8b8tAdFNFn8rKdPYx
PermissionAction logged the error and returned. Gin treats a plain return as
"carry on", so the request reached the business handler with PermissionKey
never set - and a zero DataPermission means Permission() adds no WHERE clause
at all. A database hiccup turned into full visibility, silently.
The neighbouring newDataPermission branch already aborts. This one now does the
same.
Claude-Session: https://claude.ai/code/session_01HPTAw8b8tAdFNFn8rKdPYx
Two directions, because the fix has to hold both: an attacker with no policy on
this route cannot raise another user's role, and a self-edit cannot raise its
own. The second one is what keeps the fix from being "just remove the route
from CasbinExclude", which would break the profile page.
The tests drive the handler directly rather than through the router, because
the middleware is exactly what does not run for this route - the defence lives
in the handler, so that is where it has to be proven.
Claude-Session: https://claude.ai/code/session_01HPTAw8b8tAdFNFn8rKdPYx
The profile page posts the whole user object back, including roleId, deptId and
status, because it renders from a full SysUser it fetched earlier. A caller
editing their own record can therefore hand back a tampered roleId.
Self-edits now reload those three fields from the database and ignore whatever
the request carried. For an honest client this is a no-op - the values it sends
are already its own - so the profile page keeps working unchanged.
Claude-Session: https://claude.ai/code/session_01HPTAw8b8tAdFNFn8rKdPYx
PUT /api/v1/sys-user sits in CasbinExclude so the profile page can reach it,
which means AuthCheckRole never runs for this route. The handler took the
target user id from the request body, so any authenticated caller could edit
another user's record - including their roleId.
The route has to stay excluded: the profile page and the admin user list share
this one endpoint, so removing the exclusion would break self-service editing
for every non-admin role. The check therefore moves into the handler: when the
target is not the caller, the request is put through Casbin explicitly.
EnforceRoleFor carries the same admin short-circuit and enforcement AuthCheckRole
uses, so a route that opts out of the middleware can still ask the same question.
Claude-Session: https://claude.ai/code/session_01HPTAw8b8tAdFNFn8rKdPYx
The warning on Authorizator matters to anyone keeping a copy of that file, not
to one particular consumer, and it reads better addressed to all of them: check
what reads those context keys before taking this change.
The package-level AppRouters slice keeps working and keeps running first, so
a fork that only ever appended to it sees no change. What is new is that the
core registry runs too, and that before callbacks run at all - this server
never had a loop for them.
Both go through core RunAppRouters / RunBefore, which brings the panic guard
and the seal with them.
Six checks, five at ERROR and one - the cross-repository menu-name comparison -
at WARN, because it can only match by regular expression across two modules and
a false positive that fails CI teaches people to silence the tool.
The summary names which contract roots were actually scanned: core/ is a
separate module with no directory here, and a check that quietly covers less
than it claims is worse than no check.
--app rejects a code nothing was registered under, on all three paths. It used
to take a typo as "nothing matched" and report success: migrate said the app
was unknown and still exited 0, while --dry-run and status printed the same
words an up-to-date database produces.
The map Authorizator receives is built by IdentityHandler in the same file and
carries IdentityKey / UserName / RoleKey / UserId / RoleIds / DataScope - not
user and role. Both assertions failed on every request, and because the ok
result was discarded the five c.Set calls stored zero values and the function
returned true anyway. Nothing in this repository or in core reads those keys.
go-admin-pro has its own copy of this file and does read them; this change
must not be carried over there verbatim.
common/middleware imported app/admin/service/dto for two string constants,
which made a package apps are told to build on depend on one particular app.
go.work points this module at a local checkout of go-admin-core while the
two are developed together. It is a local tool and must never be committed:
CI resolves core from go.mod.
checksilent is where `go build ./tools/checksilent` drops its binary - four
megabytes beside the server one, which was already ignored by name.
The previous commit added a data-permission scope to SysApi.Update and
returned early on db.Error, which left the RowsAffected check below it
unreachable: First reports a row the scope excluded as ErrRecordNotFound,
so the caller got "record not found" where the code meant to say
"无权更新该数据".
Map that one error to the permission message and drop the check it made
dead. The two cases - the row does not exist, and the row exists but is
not yours - have to look the same from outside, and now do.
Found by Copilot's review of #889.
SysApi.Update took a DataPermission and never used it, so with data
permission enabled the update reached rows the caller could not read
through GetPage, Get or Remove, which all scope the query. It also
reported "无权更新该数据" for a row that simply did not exist, a message
that only becomes true once the scope is applied.
Drops the Debug() left on the query, which logged the statement for
every call.
LoggerToFile is registered on the engine, so every POST, PUT, GET and
DELETE had its body copied into memory - through a bytes.Buffer, a
ReadAll and a string conversion - before any handler ran. The only
consumer is operParam on the operation-log row, which is written when
logger.enableddb is on, and that is off in the shipped configuration.
There was no size limit either, and a file upload is a POST like any
other: a 1MB request allocated 4.3MB here and a 16MB upload allocated
about 67MB, to build a value nobody stored.
The body is now read only when the operation log will use it, and at
most 32KB of it. The handler still receives the whole request: it reads
the copied part from memory and the rest from the connection, so what
this holds is bounded however large the request is. 32KB also keeps the
value inside the TEXT column it is written to.
The bufio.Writer this replaces was never flushed. Nothing was truncated
only because bytes.Buffer implements io.ReaderFrom, so io.Copy bypassed
the buffer entirely - a different destination would have dropped the
tail of every request body.
BeforeCreate and BeforeUpdate run Encrypt on whatever is in the struct,
and a user read from the database carries the stored hash in Password.
Hashing it again produces a hash of a hash: the password that user knows
stops matching, they cannot log in, and nothing reports an error.
Only the Omit("password") on SysUser.Update stood between that and the
stored credential. Any other write to this model - a profile update
written the way every other model here is written - destroys the
password, permanently and silently.
Encrypt now returns early when Password already parses as a bcrypt hash.
That also removes the round SysUser.Update was paying and discarding:
306ns where it was 54.7ms, on a route reachable without the permission
check, since PUT /api/v1/sys-user is in CasbinExclude.
The cost of deciding from the value is that a password which is itself a
well-formed bcrypt hash would be stored unchanged. That is a
60-character string beginning "$2a$", and it grants whoever set it no
access they did not already have.
The repository has 19 test files and nothing was running any of them. Both
workflows build with go build, which does not compile _test.go, the Makefile's
test target was commented out, and there is no pre-commit hook. Every test in
the tree, including the schema guards that exist precisely to catch a silent
breakage, only ran when someone remembered to type go test.
Enables the commented-out target and calls it from go.yml, the one workflow
that fires on every push and pull request. build.yml is left alone: it skips
documentation-only changes and deploys on master, so it is the wrong place for
a gate that should never be skipped.
Runs with -race. common/actions reuses model instances across concurrent
requests, so a Generate() that returns in place rather than a copy leaks data
between them, which a single-threaded run cannot see.
Verified locally: the suite passes under CGO_ENABLED=0 and under -race, and
make test exits non-zero when a test fails, so the step actually gates.
Claude-Session: https://claude.ai/code/session_01DJhM6LvhkNPej35wy9F7Aq
The hazard had no signal at its point of contact. Someone adding a business
module is told to copy 1786700001000_demo_menu.go, which imports the frozen
seed models - correct for that file, wrong for anything ordered after the
soft-delete conversion. The frozen ModelTime carried no comment at all, so
opening it taught the reader nothing.
Documents the boundary in three places the author actually passes through:
the frozen type itself, the contributor guide, and the module-scaffolding
skill, which previously said to copy the reference file verbatim and now
says to copy its structure but not its imports.
Claude-Session: https://claude.ai/code/session_01DJhM6LvhkNPej35wy9F7Aq
Migrations ordered after 1786700003000 must not seed through
cmd/migrate/migration/models. That package's ModelTime declares a nullable
gorm.DeletedAt, which is the shape the columns had until that migration
converted deleted_at to a NOT NULL millisecond marker.
Afterwards it breaks in both directions. Writes put NULL into a NOT NULL
column and fail on the first insert. Reads are scoped "WHERE deleted_at IS
NULL" while live rows hold 0, so they match nothing - and 1786700001000
looks the admin role up that way and treats ErrRecordNotFound as "roles are
not seeded yet, skip authorisation", which would leave a module seeded with
no permissions and the migration still recorded as applied.
A fresh database does not surface either one: every migration using that
package today is ordered before the conversion, so it runs while the column
is still nullable. Only a migration added afterwards hits it.
Also pulls the import scan out of importsRuntimeModels so both checks share
one implementation, and derives the version through migration.GetFilename
rather than a second filename-parsing rule.
Verified by adding a violating migration and confirming the test fails with
an actionable message, then removing it and confirming the suite passes.
Claude-Session: https://claude.ai/code/session_01DJhM6LvhkNPej35wy9F7Aq
AuthCheckRole walks CasbinExclude for every non-admin request, and used
casbin's util.KeyMatch2 to test each entry. That delegates to
util.RegexMatch, which is regexp.MatchString - it compiles its pattern on
every call - so a 32-entry list cost about 2,566 allocations per request
before the request reached Enforce.
Test the method first, which rules out most entries with a string
compare, and take the path test from go-admin-core, whose KeyMatch2
answers the same thing without recompiling. The scan drops to 52ns and no
allocations.
The loop moves out of AuthCheckRole so the tests exercise the code a
request runs rather than a copy of it, and an allocation budget fails if
the uncached matcher comes back.
setupSimpleDatabase runs once per configured database - one per host in
the multi-tenant configuration - and passed the same empty key to
mycasbin.Setup every time. Setup caches per key, so every host after the
first was handed the enforcer built from the first host's database and
was authorized against a casbin_rule table that was not its own.
Takes effect with the go-admin-core release that keys the cache; before
it, Setup ignored the argument entirely.
A push to master here does not just build: it pushes an image, runs the
migrations and restarts the demo container, so the site takes a short outage
each time. The last two merges were markdown only and both paid for it.
Beyond the waste, a deploy can fail for reasons unrelated to the change that
triggered it - a container that will not come up, a database that is briefly
unreachable - and a README edit should not be able to turn the demo red.
Only build.yml is filtered. go.yml still builds on every push and pull request,
so nothing loses its compile check, and the badge keeps reporting the same
workflow it reports today.
`:::tip` and its closing `:::` are VitePress custom containers. GitHub has no
such syntax, so both markers rendered as literal text: a paragraph beginning
":::tip" and a stray ":::" sitting alone above the next heading.
The Chinese README carries the same warning as a plain paragraph, which GitHub
renders correctly, so the English one now matches it. Verified through GitHub's
own markdown API: the literal marker no longer appears in the output and the
warning survives as ordinary text.
Only README.md was affected; the other three never had it.
The project had English and Simplified Chinese. These two follow the same
structure - same sections, same code blocks, same contributor list - so a
reader in any of the four sees the same document.
The Traditional Chinese is a translation rather than a character conversion of
the Simplified: the terminology differs (設定檔, 資料庫, 選單, 程式碼產生,
排程任務, 相依套件), and a converted file would read as machine output to
anyone who actually uses it.
Language navigation across all four is unified in the same commit, since a link
to a file that does not exist yet would be worse than no link.
The build badge rendered "build - failing" on both READMEs while CI was green.
It referenced the workflow under the old personal repository path, where the
status has been stale for years - so the first thing anyone saw on opening the
project was a failed build. It now points at the current repository, names the
workflow file explicitly, and pins the branch, so it reports master rather than
whatever happens to be the default branch later.
The workflow it reports on is go.yml, which is what the old badge referenced
by workflow name and is the right one to show: build.yml also deploys the demo
site, so a server-side problem there would turn the badge red while the code
is fine.
The licence badge read from mashape/apistatus, the example repository from
shields.io's own documentation. It happened to show MIT, the same licence this
project uses, so nobody noticed - but it reports someone else's licence.
Documentation links were spread across three hosts: doc.go-admin.dev redirects
to www.go-admin.pro, www.go-admin.dev serves byte-identical content, and only
the Chinese README linked the canonical host at all. All of them now point at
www.go-admin.pro directly rather than relying on a redirect outliving the
domain that issues it.
Two smaller ones: the gorm link pointed at the archived v1 repository while the
project builds on gorm.io v2, and the English introduction listed two UI kits
where the Chinese listed three, with an Ant Design demo linked directly below.
Skipped unless GOADMIN_BENCH_ADDR points at a running server, so `go test
./...` is unaffected.
Reports latency percentiles rather than an average, which is what capacity
planning needs, and a status-code distribution - that last part is how the rate
limiter's 200-on-rejection was found, since throughput alone looked excellent
while nothing reached a handler.
Includes a routing-floor control case. When a business endpoint matches it, the
measurement has stopped describing the endpoint and started describing the
transport, or the load generator when both share a machine.
Two settings that decide whether a deployment survives load, neither of which
appeared in any template.
The connection pool. Left unset, Go's defaults apply, and MaxIdleConns is 2:
under load almost every request opens a TCP connection and closes it again,
local ports run out, and the process answers "can't assign requested address"
to everything. Not slower - unavailable. A sweep against MySQL collapsed to
zero successful responses at 64 concurrent requests without these, and served
13,846 req/s with no errors once they were set.
The queue buffer. poolSize is the point at which messages start being dropped,
not a tuning knob: a full queue discards the message and returns an error
rather than blocking, and each stream has one consumer goroutine writing to the
database. At the previous default of 100 a load test lost over 60% of them; at
1000, none. Login and operation logs travel this queue, so what gets lost is
audit data - though only when logger.enableddb is on.
Both carry the reasoning in the file, because the failure mode of each is
invisible until it happens in production.
A rejected request answered 200 with the failure only in the body, so every
layer that reads the status line counted it as served: load balancers, metrics,
client-side retry. A load test against this reported the limiter's own
rejections as successful traffic and overstated throughput more than tenfold.
The threshold was a constant in the middleware, which made 200 QPS the ceiling
of every deployment with nothing in the configuration to reveal it. It now
reads extend.rateLimit.inboundQPS; an absent value keeps 200, so an upgrade
changes nothing, and zero disables the limiter for a deployment behind its own
gateway.
Also drops Strategy: system.BBR. Reading sentinel's source, the adaptive
strategy is consulted only for Load and CpuUsage - for InboundQPS the trigger
count is compared directly - so it read as if the limit adapted to the machine
when it never did.
The answer was written at info level on every captcha request, so a currently
valid answer sat in the application log. Anyone able to read the log - an
operator, a log aggregator, anything that ships logs off the host - could
bypass the check the captcha exists to enforce.
The default log level records it, so this was not limited to debug builds.
Carries four concurrency fixes and a bounded in-memory cache. The two that
reach this repository are the search resolver, which no longer panics on an
unexported field in a DTO and skips tag parsing for zero-valued ones, and the
captcha driver, which is built once rather than per request.
The cache bound does not apply here: config.CacheConfig.Setup() returns the
older Memory implementation, which core leaves unbounded.
Closes#871.
The deploy did docker rm -f then docker run. Nothing ran migrations, so
new code met old tables, and nothing checked the result - a container
that exits immediately left the site down with a green deploy.
Now, in order: pull the image, run the migration with it, and only then
touch what is running. A failed migration stops there, leaving old code
with the old schema, which is at least self-consistent.
The running container is renamed rather than removed, so it can be
started again unchanged if the new one does not become healthy. Healthy
means both an HTTP response and a database connection in the log: the
captcha endpoint answers without touching the database, so it alone
would call a container healthy that cannot reach MySQL.
An applied migration printed its count - a bare '1' - so a database with
seven of them wrote seven lines of '1' at every start, and a failure said
only which error, never which migration.
It now names each one as it applies, reports the total, and says so when
there is nothing to do.
The startup line printed the DSN whole:
* => goadmin:<password>@tcp(host:3306)/go-admin?...
So every deployment wrote its own database credential into its own logs,
where a log shipper, a support bundle or a screenshot of a terminal
carries it onward. Found while reading deploy output, which is exactly
how it leaks.
The host and username stay - they are what makes the line worth printing
- and only the password is replaced. Both DSN shapes this project accepts
are covered, a sqlite path is left alone, and anything unparseable is
withheld rather than echoed, since it may hold a credential too.
The path is not a credential, and the file it points at is 600 and owned
by root, so this is not what protects it. But the repository is public
and there is no reason to publish the server's directory layout next to
the deploy that uses it.
DEMO_CONFIG_PATH holds it instead. It has to be set before this merges,
or the deploy stops at the guard - which is the intended failure: better
that than falling back to the sqlite in the image.
The demo ran on the sqlite file baked into the image, so every deploy
reset it and nothing there resembled how anyone actually runs this.
The config is mounted from the host rather than taken from the image.
config/settings.demo.yml ships in a public repository and is copied into
a public image, so the connection string cannot live there; that copy
stays on sqlite, which is what a fresh clone should get.
The deploy refuses to start if the host config is missing, rather than
falling back to the image's sqlite and looking like it worked.
sys_columns and sys_tables were left out of the soft-delete conversion in
1786700003000. Their runtime models embed common.ModelTime, which is the
millisecond marker, so GORM queries them with deleted_at = 0 - against a
nullable datetime column holding NULL. Every row was invisible.
The repository carries two ModelTime types: the one under
cmd/migrate/migration/models still has a nullable gorm.DeletedAt and is
what builds the tables, while common/models has the marker and is what
queries them. Nothing connected the two, so a table could be built one
way and read the other with no signal at all.
The test now walks app/ for models embedding the marker and requires a
migration to cover each. tb_demo is exempt and says why: nothing reads it
at runtime.
sort is gorm:"size:4", which MySQL builds as a tinyint holding -128..127.
The demo menu seeded Sort: 900, so on MySQL the run stopped at
1786700001000 with Error 1264, and every migration after it - including
the soft-delete conversion - never ran.
deleted_at therefore stayed NULL while the code queries deleted_at = 0,
and the login returned 'incorrect Username or Password' on a database
whose password hash was correct all along.
sqlite ignores the declared width, so a fresh install there passed and
the fault only appeared on MySQL.
The scope is decided by the user id, the role id, the department and the
data_scope string. Three of the four were already in the token; deptid
was not, though core's user.GetDeptId has always read that claim. Adding
it removes a sys_user join from every list, detail, update and delete.
This goes no more stale than rolekey does, which Casbin has read from
the token since the beginning: both settle on the next login.
A token minted before this still works. Its claims are incomplete, and
the lookup runs for it as before.
Permission() returns the query untouched when EnableDP is false, so the
lookup feeding it has nothing to feed. The lookup ran anyway: a sys_user
join against sys_role on every list, detail, update and delete, with the
result discarded.
enabledp is false in settings.full.yml, so this was the default.
thirdUpload dispatched on the source parameter and then built the same
zero-value ALiYunOSS in both branches, so source=3 could not have
reached qiniu even with credentials.
Neither branch had credentials to use. OXS.Setup is the initialisation
path and nothing in the repository called it, and no configuration field
existed to fill. The store is now taken from extend.fileStore, and a
provider that was not configured says so rather than producing the
provider's own complaint about an empty bucket name.
The two handlers passed errors.New("") to e.Error, discarding what
actually went wrong; they now pass the error.
Each implementation keeps its provider client in an interface{} field that
Setup assigns, so an unconfigured store holds nil - and asserting nil to
the provider's client type panics:
panic: interface conversion: interface {} is nil, not *oss.Client
The upload endpoint reaches that path for any request naming a provider
the deployment never configured.
Three more things were wrong in the same files. OXS.Setup printed a
failure and returned the store anyway, handing back exactly the broken
object that panics. HuaWeiOBS.UpLoad printed the provider's error and
returned nil, so a failed upload reported success. Both it and
QiNiuKODO.UpLoad asserted the local path was a string without checking.
The tests asked the reader to paste their own credentials, so they failed
for everyone who did not. They now cover the guards and skip the part
that needs a provider unless credentials are in the environment.
Two merges seconds apart raced. Both runs do docker rm -f then docker
run; the second removed the container the first had just created, and
the first's docker run failed on the name conflict:
Conflict. The container name "/go-admin-api" is already in use
The deploy went red and the demo stayed on the older image, which is the
worse half: a failure that leaves the wrong version running.
Grouping by ref serialises pushes to master while leaving pull request
runs independent, since those carry their own ref.
Added in 2022 and never touched since. Nothing references it - not the
workflows, not the Makefile, not a script - and it could not build
anyway: it copies config/settings.yml out of the builder, and that file
is gitignored.
It is a leftover from when the image was built inside the container,
before that was replaced by copying a binary built on the runner. Its
MAINTAINER line was the last one in the repository; Docker deprecated
the instruction in favour of LABEL maintainer years ago.
The code generator writes Go files, and its templates still spelled the
old import paths, so a module generated after this migration did not
compile: the router it emits declares InitBusinessRouter with the v1
*GinJWTMiddleware while common.AuthInit now returns the v2 type.
Two of the paths moved rather than gaining a /v2 segment - the jwtauth
and response shims under sdk/pkg are gone in v2 - so this is not the
same rewrite the Go files got.
Every import of the module changes, not only the seven packages that
moved out of sdk/pkg: Go requires the major version in the path from v2
on. Both happen in one pass —
go run github.com/go-admin-team/go-admin-core/tools/coreupgrade@v2.0.0 -w -v2 .
go mod tidy
— which is the command the release notes give, run here as a consumer
would run it. 210 imports across 95 files.
The compatibility shims this used are gone in v2, so the paths that
moved had to move: sdk/pkg/captcha, sdk/pkg/jwtauth and its user
package, sdk/pkg/response and sdk/pkg/casbin.
The count of unformatted files is unchanged at 34, none of them touched
by this: the tool reformats a file only if it was already gofmt clean,
so a migration cannot disappear into whitespace.
The skill walks a single-table CRUD module end to end: migration, the
Actions-mode model, dto and router, and the sys_menu / sys_api /
casbin_rule seed data without which the module builds but never appears.
.claude was ignored wholesale. Un-ignoring the skills directory would
have committed every skill put there, including personal ones, so the
skills that ship are re-included one directory at a time.
AGENTS.md now points at 1786700001000_demo_menu.go for the seed data,
which is the runnable version of what the skill describes.
go mod tidy moves glebarez/sqlite and gorm.io/plugin/soft_delete out of
the indirect block: the tests import the first and common/models the
second. CI runs tidy before building, so the tree was dirty from the
first command.
The exclusion list was a subquery against `$GenConfig.DBName`.sys_tables,
so it named the schema by hand. Generating from a schema that is not the
one holding sys_tables made the whole query fail, and because the
subquery read the table directly it also counted soft-deleted entries:
deleting a generator entry never handed its table back.
Read the registrations through the model on this connection instead. An
empty list skips the clause - NOT IN (NULL) is unknown for every row,
which would leave a fresh install with nothing to generate from.
opens is a map, so opens[c.Driver] on a driver this build does not carry
returns a nil function, and gorm.Open calls it. The operator saw a nil
dereference inside gorm with nothing naming the driver.
sqlite3 is the case that bites: it needs cgo and is only compiled in
under the sqlite3 build tag, so the same config file works on one binary
and dies on another. Resolve the driver first and say which ones this
build supports.
pkg.Assert panics when its condition is false, so pkg.Assert(true,
"目前只支持mysql数据库") is a no-op. On postgres or sqlserver the code
generator did not report that it needs MySQL: DBTables returned an empty
list with a nil error, and DBColumns ran its query on the zero-value
*gorm.DB left over from the branch that never assigned, which is a nil
dereference rather than a message.
Assert the driver up front instead of asserting a constant in an else,
which also removes the placeholder *gorm.DB the fall-through relied on.
DBColumns.GetPage had no guard at all and gets the same one.
pkg.Assert panics when its condition is false, so
Assert(TableName == "", "table name cannot be empty") rejected every
request that carried a table name and let the empty one through. The
model layer repeated the inversion with if TableName != "" { return
error }, so either one alone was enough to break the endpoint.
Flip both, and hoist the model guard out of the mysql branch so it
matches GetList ten lines below, which had it right all along.
go-admin-db.db ships in the repository and the Dockerfile copies it into
the image, which then runs only the server. Its last recorded migration
was from 2022, so every row still carried a null deleted_at while the
code queries deleted_at = 0. Nothing matched: not the login, not the
sixty-seven menus, not the five departments.
Anyone starting from the bundled sqlite database met the same wall, and
the failure reads as an incorrect username or password.
Two assumptions held on the test's table and on nothing else.
It dropped deleted_at while an index still referred to it. MySQL and
PostgreSQL drop dependent indexes along with the column; SQLite refuses,
and the migration stopped at the first table with such an index - which
is all thirteen of them.
It also read the rows through a column named id. sys_dept keys on
dept_id, sys_user on user_id, and only some tables on id, so the pass
that carries the deletion timestamps across never ran.
The test's table had an id key and no index on deleted_at, which is
exactly the shape that lets both through. It now matches sys_user.
Every push mirrored to Gitee and GitLab. Neither mirror is wanted any
more, so the workflow goes rather than half of it.
The GITEE_KEY and GITLAB_KEY secrets are left in place; restoring the
mirror is a revert of this commit.
The two tutorial links pointed at doc.zhangwj.com, which no longer
answers; the same paths serve from doc.go-admin.dev. golangroadmap.com
returns 503. The jwt-go credit pointed at dgrijalva/jwt-go, archived
years ago - this project builds on golang-jwt/jwt.
Also: the copyright years said 2022 and 2024, the English README asked
for a password in Chinese, and the Chinese README's link section lost
its only entry, so it gets the one the English side already had.
Review caught that this reissued getByRoleName's query instead of
calling it, so it passed whether or not the production line still said
what it was supposed to — a test named for a change it did not touch.
It calls getByRoleName now, and restoring the hand-written clause fails
it for exactly the reason this PR exists: with the marker non-null,
"deleted_at is null" matches nothing and the query returns an empty
list.
sys_user.username, sys_role.role_key and sys_dict_type.dict_type had no
unique index. Uniqueness was a SELECT COUNT followed by an INSERT, which
two concurrent requests both pass — and login resolves a username with
First, so which of the two accounts answers is whichever the database
returns.
The index cannot be on the key alone, because a soft-deleted row keeps
occupying the name and a deleted user's username could never be used
again. It has to include the delete marker, and the marker has to be
non-null: two live rows are (alice, NULL) and (alice, NULL), and NULL is
not equal to NULL, so an index over a nullable marker admits both. That
is the worst of the three states — a constraint that reads as protection
and binds nothing — and there is a test that demonstrates it rather than
asserting it.
ModelTime.DeletedAt is milliseconds since the epoch now, zero while the
row is live. Sixteen tables carry it; the migration converts each one,
preserving when each deleted row was deleted, then adds the three
indexes.
Written to be re-runnable rather than transactional, because DDL does not
roll back on MySQL and an operator whose first attempt failed halfway
should have nothing to do but run it again. It refuses before altering
anything if a table already holds duplicates, naming them, rather than
letting the index fail and leaving the operator to guess.
The timestamp conversion happens in Go: turning a timestamp into epoch
milliseconds is spelled differently by every dialect this supports, and
these row counts do not justify four versions of it.
Two things in front of the unique-index work, both safe on their own.
getSysMenuByRoleName carried "deleted_at is null" in its where clause.
GORM adds that condition itself for a model with a DeletedAt field, so
it was a duplicate — and one phrased as a column being null, which stops
being true the moment the column stops being nullable. A schema that
moves to a non-null delete marker would have turned this query into one
that matches nothing, silently, for admin users only.
SysDictType.Insert dropped the error from its duplicate check: a query
that failed left the count at zero and the insert went ahead as though
the name were free.
The test pins what the removed clause was there for. Its counter-proof
is Unscoped rather than deleting the field — taking ModelTime off the
model fails to compile, which proves nothing.
The sample had producer/consumer nested keys (streamMaxLength,
approximateMaxLength, visibilityTimeout, bufferSize, concurrency,
blockingTimeout, reclaimInterval) that don't exist on config.RedisQueue —
checked against sdk/config/queue.go, which only reads addr, password, and the
embedded RedisOptions fields, plus group, key_prefix and max_attempts. Filling
in the old sample as written would compile and start fine, since it's YAML
under a key the struct doesn't declare, and every one of those settings would
be silently ignored.
Replaced with the fields the struct actually has. Still commented out —
redis stays opt-in, this only fixes what filling it in would produce.
The pinned core dated from April, before sdk stopped being a separate module,
so the build resolved sdk packages from the old module and core packages from
the new one. Dropping the separate requirement is what makes the two agree
again.
Most of the diff is renames that came with that: the tenant accessors gained a
ByTenant suffix, GetDb now returns one database and GetAllDb the map, and
casbin moved to v3.
The change that matters is four call sites moving from GetMemoryQueue to
GetQueuePrefix. GetMemoryQueue returns a queue fixed at construction, so the
login log, the operate log and the api check ran in process no matter what the
settings file selected — a second instance saw none of it. GetQueuePrefix
returns whatever the configuration built, which is the point of being able to
configure a queue at all.
Verified against core at main: build and vet clean. The two file_store failures
are unchanged from before this branch; they need cloud credentials.
Replace version tags (@v1/@v2/@v3/@master) with pinned commit SHAs
across all workflow files to satisfy go-admin-team organization
security policy requiring immutable action references.
description:Scaffold a new single-table CRUD business module end to end — migration, Actions-mode model/dto/router, and the sys_menu/sys_api/casbin seed data that makes it show up in the UI with working permissions. Use when the user wants to add a new business table/module to go-admin, not for cross-table or non-CRUD business logic.
Gin + Vue + Element UI / Arco Design / Ant Design による、フロントエンドとバックエンドを分離した権限管理システムです。初期化は非常に簡単で、設定ファイルのデータベース接続情報を変更するだけで動作します。複数のコマンドに対応しており、マイグレーションコマンドでデータベースの初期化が容易になり、サーバーコマンドで API を手軽に起動できます。
English | [简体中文](https://github.com/go-admin-team/go-admin/blob/master/README.Zh-cn.md)
English | [简体中文](https://github.com/go-admin-team/go-admin/blob/master/README.Zh-cn.md) | [繁體中文](https://github.com/go-admin-team/go-admin/blob/master/README.zh-TW.md) | [日本語](https://github.com/go-admin-team/go-admin/blob/master/README.ja-JP.md)
The front-end and back-end separation authority management system based on Gin + Vue + Element UI is extremely simple to initialize the system. You only need to modify the database connection in the configuration file. The system supports multi-instruction operations. Migration instructions can make it easier to initialize database information. Service instructions It's easy to start the api service.
The front-end and back-end separation authority management system based on Gin + Vue + Element UI OR Arco Design OR Ant Design is extremely simple to initialize the system. You only need to modify the database connection in the configuration file. The system supports multi-instruction operations. Migration instructions can make it easier to initialize database information. Service instructions It's easy to start the api service.
// if err := orm.Eloquent.Table("sys_role_dept").Select("sys_role_dept.dept_id").Joins("LEFT JOIN sys_dept on sys_dept.dept_id=sys_role_dept.dept_id").Where("role_id = ? ", role.RoleId).Where(" sys_role_dept.dept_id not in(select sys_dept.parent_id from sys_role_dept LEFT JOIN sys_dept on sys_dept.dept_id=sys_role_dept.dept_id where role_id =? )", role.RoleId).Find(&deptList).Error; err != nil {
// FailedVersion and LastError are DIAGNOSTIC TEXT ONLY - what a human
// looking at this row is told about the last failure, nothing more. No
// code anywhere may read either one to decide what to do next.
//
// The question "where should a resume pick up" has exactly one
// authoritative answer, and it is not these two columns: subtract
// sys_migration's applied rows for this app_code from what the app's
// own compiled-in code has registered (migration.Snapshot()/ForApp -
// the same set F7's `migrate status` already walks). That answer can
// never go stale, because it is not stored anywhere to go stale - it is
// recomputed from sys_migration every time it is asked. FailedVersion
// is a snapshot of what that computation returned at the moment of
// failure, kept only so an operator does not have to go find the
// process's logs; if it and a fresh recomputation from sys_migration
// ever disagree, sys_migration is right and this column is stale, by
// definition, and nothing should ever notice or care except a human
// reading the row.
FailedVersionstring`json:"failedVersion" gorm:"size:64;not null;default:'';comment:diagnostic snapshot only, not a judgment basis; meaningful only when status=3"`
LastErrorstring`json:"lastError" gorm:"size:255;not null;default:'';comment:diagnostic text only, not a judgment basis; meaningful only when status=3"`
// InstalledAt is when this app first reached status=installed - set
// once, never moved by a later upgrade (see design doc §1.4). Nullable,
// unlike every other column here: a row can exist before it has a
// value (a fresh install starts at status=installing). This is not the
// deleted_at problem 1786700003000_soft_delete_marker.go fixed - that
// column sat inside a unique index, where NULL <> NULL let two live
// rows coexist under the same key. InstalledAt is in no index at all,
// so nullability here opens no such hole.
InstalledAt*time.Time`json:"installedAt" gorm:"comment:first successful install time; null until status first reaches installed"`
db=db.Where(tableName+".create_by in (select sys_user.user_id from sys_role_dept left join sys_user on sys_user.dept_id=sys_role_dept.dept_id where sys_role_dept.role_id = ?)",user.RoleId)
}
ifrole.DataScope=="3"{
db=db.Where(tableName+".create_by in (SELECT user_id from sys_user where dept_id = ? )",user.DeptId)
}
ifrole.DataScope=="4"{
db=db.Where(tableName+".create_by in (SELECT user_id from sys_user where sys_user.dept_id in(select dept_id from sys_dept where dept_path like ? ))","%"+pkg.IntToString(user.DeptId)+"%")
returndb.Where(tableName+".create_by in (select sys_user.user_id from sys_role_dept left join sys_user on sys_user.dept_id=sys_role_dept.dept_id where sys_role_dept.role_id = ?)",user.RoleId)
}
ifrole.DataScope=="3"{
returndb.Where(tableName+".create_by in (SELECT user_id from sys_user where dept_id = ? )",user.DeptId)
}
ifrole.DataScope=="4"{
returndb.Where(tableName+".create_by in (SELECT user_id from sys_user where sys_user.dept_id in(select dept_id from sys_dept where dept_path like ? ))","%"+pkg.IntToString(user.DeptId)+"%")
Some files were not shown because too many files have changed in this diff
Show More
Reference in New Issue
Block a user
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
wenjianzhang