Compare commits

..
Author SHA1 Message Date
zhangwenjian 22716e90c1 fix🐛: /getinfo cannot be scoped by a data permission it never receives
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.
2026-09-05 21:24:25 +08:00
wenjianzhang 73cce7fc2f Merge pull request #903 from go-admin-team/feat/005-sigterm
fix🐛: SIGTERM 从未被处理,优雅关闭在容器里是死代码
2026-09-05 18:00:58 +08:00
zhangwenjian b59c7f0d46 test✅: wait for the accept, not just the dial
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
2026-09-05 16:44:53 +08:00
zhangwenjian d3a44a2a6b test✅: dial the stalling connection after the signal, not at start-up
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
2026-09-05 16:42:47 +08:00
zhangwenjian 5c3c3907d5 fix🐛: arm the stop signals before announcing readiness
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
2026-09-05 16:42:44 +08:00
zhangwenjian f2215e132e fix🐛: handle SIGTERM, and stop exiting on a failed Shutdown
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
2026-09-05 15:28:15 +08:00
wenjianzhang a69afab34f Merge pull request #902 from go-admin-team/feat/006-contract-docs
docs📝: 契约文档指向 core,而不是宿主
2026-09-05 11:29:01 +08:00
zhangwenjian e0132db1b9 docs📝(checksilent): retire a comment that predates the lowering
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
2026-09-05 11:23:31 +08:00
zhangwenjian 7c3f55a873 fix🔧(checksilent): suggest the fix with the qualifier the file uses
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
2026-09-05 11:23:31 +08:00
zhangwenjian f7c0247394 fix🔧(checksilent): stop the seeded-value checks reporting their own tests
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
2026-09-05 11:16:53 +08:00
wenjianzhang ac23556029 Merge pull request #900 from go-admin-team/feat/006-host-wiring
fix🐛: 契约注册面接上宿主的执行端
2026-09-05 11:14:01 +08:00
zhangwenjian f64115e03a docs📝: state what an off-convention migration file name does
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
2026-09-05 11:14:01 +08:00
zhangwenjian a2524c31bf docs📝(contract): state the two menu seed rules a caller cannot infer
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
2026-09-05 11:14:01 +08:00
zhangwenjian 71d6211c61 fix🔧(checksilent): see a menu written as a contract MenuSpec
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
2026-09-05 11:14:01 +08:00
zhangwenjian c67760bc39 docs📝(agents): match the contract rules the reference document now states
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
2026-09-05 11:14:01 +08:00
zhangwenjian d3e7f46a46 docs📝(contract): point applications at core, not at the host
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
2026-09-05 11:14:01 +08:00
zhangwenjian 55866682ae feat✨(checksilent): require a contract shim to be a type alias
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
2026-09-05 11:14:01 +08:00
11 changed files with 1394 additions and 97 deletions
+9 -4
View File
@@ -125,9 +125,12 @@ func (SysPost) TableName() string { return "sys_post" }
两条与主仓贡献者直接相关的:
- **`common/`、`core/` 不得 import `app/`** —— `make checksilent` 在 CI 里守着,违反即红。
- **从 core 契约包声明出来的类型必须写成别名**(`type X = pkg.Y`,不是 `type X pkg.Y`)
—— `contract-shim-alias` 检查守着。defined type 会丢掉整个方法集,
而且**不一定在本仓编译失败**,理由见 `docs/contract.md` 末节。
- **注册类 API(`AppRouters` / `sdk.Runtime.SetAppRouters` / `migration.ForApp`)
只允许在 `init()` 中调用** —— 注册期靠 Go 的包初始化顺序保证无并发写,
core 侧的 setter 没有加锁。
必须在 `runStartupHooks()` 之前调用完** —— `init()` 是最省事的位置,
但约束的是**顺序**,不是写在哪个函数里;晚到的注册会被丢弃并只记一条 ERROR。
## 路由注册
@@ -191,7 +194,8 @@ go run -tags sqlite3 . server -c config/settings.sqlite.yml
## 数据库迁移
文件名前 13 位为时间戳版本号。**已执行过的迁移文件不可修改** ——
文件名前 13 位为毫秒时间戳版本号,不合规的名字会在启动时 panic 并报出该文件名。
**已执行过的迁移文件不可修改** ——
`sys_migration` 表按版本号去重,改动不会重跑,只能新增一个迁移来修正。
放哪个目录取决于身份:
@@ -223,7 +227,7 @@ go run -tags sqlite3 . server -c config/settings.sqlite.yml
## 静默失败校验
`make checksilent` 检查六类**不报错、不记日志、行为悄悄变得不对**的问题,
`make checksilent` 检查七类**不报错、不记日志、行为悄悄变得不对**的问题,
CI 会跑,命中 ERROR 即失败:
| 检查 | 级别 | 静默后果 |
@@ -233,6 +237,7 @@ CI 会跑,命中 ERROR 即失败:
| `config-value-truncation` | ERROR | `sys_config.config_value` 超 255 字符被静默截断 |
| `menu-id-collision` | ERROR | 两个模块硬编码同一菜单 ID,互相覆盖 |
| `contract-import-boundary` | ERROR | 契约包 import `app/`,应用无法独立编译 |
| `contract-shim-alias` | ERROR | 契约薄壳写成 defined type 而非别名,方法集丢失,本仓可能照常编译、第三方应用编译不过 |
| `menu-name-mismatch` | WARN | 菜单名与前端组件 `name` 不一致,keep-alive 缓存静默失效 |
最后一条要跨仓库比对,只能做正则启发式,因此是 WARN,**不影响退出码**,
+8 -2
View File
@@ -444,7 +444,6 @@ func (e SysUser) GetInfo(c *gin.Context) {
e.Error(500, err, err.Error())
return
}
p := actions.GetPermissionFromContext(c)
var roles = make([]string, 1)
roles[0] = user.GetRoleName(c)
var permissions = make([]string, 1)
@@ -464,7 +463,14 @@ func (e SysUser) GetInfo(c *gin.Context) {
}
sysUser := models.SysUser{}
req.Id = user.GetUserId(c)
err = s.Get(&req, p, &sysUser)
// Unscoped on purpose: the id is the caller's own, taken from the token.
// This used to go through Get with whatever GetPermissionFromContext
// returned - and this route installs no PermissionAction, so that was the
// zero value. An unset scope is not a recognised one, so once unknown
// scopes started failing closed rather than silently matching everything,
// every login on a deployment with enabledp: true ended here with a 401
// and the browser went straight back to the login page.
err = s.GetSelf(&req, &sysUser)
if err != nil {
e.Error(http.StatusUnauthorized, err, "登录失败")
return
+5 -1
View File
@@ -5,6 +5,7 @@ import (
jwt "github.com/go-admin-team/go-admin-core/v2/jwtauth"
"go-admin/app/admin/apis"
"go-admin/common/actions"
"go-admin/common/middleware"
)
@@ -15,7 +16,10 @@ func init() {
// registerSysApiRouter
func registerSysApiRouter(v1 *gin.RouterGroup, authMiddleware *jwt.GinJWTMiddleware) {
api := apis.SysApi{}
r := v1.Group("/sys-api").Use(authMiddleware.MiddlewareFunc()).Use(middleware.AuthCheckRole())
// PermissionAction is not optional here: all three handlers below read the
// data permission out of the context, and without it they read the zero
// value - an unset scope, which Permission now fails closed on.
r := v1.Group("/sys-api").Use(authMiddleware.MiddlewareFunc()).Use(middleware.AuthCheckRole()).Use(actions.PermissionAction())
{
r.GET("", api.GetPage)
r.GET("/:id", api.Get)
+24
View File
@@ -38,6 +38,30 @@ func (e *SysUser) GetPage(c *dto.SysUserGetPageReq, p *actions.DataPermission, l
return nil
}
// GetSelf 获取调用者自己的 SysUser 对象,不套数据权限
//
// The data scope answers "whose rows may this user see"; the caller here is
// reading their own, and the id comes from the token, so there is nothing left
// for a scope to restrict. Applying one is not a stricter version of this
// query - it is a broken one. DataScopeSelf matches on create_by, and a user
// account is created by whoever added it, so a scoped self-read would fail for
// every user who did not create their own account.
//
// GetProfile has always read the same row this way, with no scope at all.
func (e *SysUser) GetSelf(d *dto.SysUserById, model *models.SysUser) error {
err := e.Orm.First(model, d.GetId()).Error
if err != nil && errors.Is(err, gorm.ErrRecordNotFound) {
err = errors.New("查看对象不存在或无权查看")
e.Log.Errorf("db error: %s", err)
return err
}
if err != nil {
e.Log.Errorf("db error: %s", err)
return err
}
return nil
}
// Get 获取SysUser对象
func (e *SysUser) Get(d *dto.SysUserById, p *actions.DataPermission, model *models.SysUser) error {
var data models.SysUser
+54 -10
View File
@@ -6,6 +6,7 @@ import (
"net/http"
"os"
"os/signal"
"syscall"
"time"
"github.com/gin-gonic/gin"
@@ -85,8 +86,8 @@ func run() error {
runStartupHooks()
srv := &http.Server{
Addr: fmt.Sprintf("%s:%d", config.ApplicationConfig.Host, config.ApplicationConfig.Port),
Handler: sdk.Runtime.GetEngine(),
Addr: fmt.Sprintf("%s:%d", config.ApplicationConfig.Host, config.ApplicationConfig.Port),
Handler: sdk.Runtime.GetEngine(),
ReadTimeout: time.Duration(config.ApplicationConfig.ReadTimeout) * time.Second,
WriteTimeout: time.Duration(config.ApplicationConfig.WriterTimeout) * time.Second,
}
@@ -114,6 +115,13 @@ func run() error {
}
}
// Armed before the server starts serving, and well before the readiness
// banner: a signal arriving between "the process is up" and "the process
// is listening for signals" reaches the default handler and kills it
// without any of the shutdown below. That window is the whole reason
// arming is separate from waiting.
quit, disarmStopSignals := armStopSignals()
go func() {
// 服务连接
if config.SslConfig.Enable {
@@ -135,23 +143,59 @@ func run() error {
fmt.Printf("- Local: http://localhost:%d/swagger/admin/index.html \r\n", config.ApplicationConfig.Port)
fmt.Printf("- Network: %s://%s:%d/swagger/admin/index.html \r\n", "http", pkg.GetLocalHost(), config.ApplicationConfig.Port)
fmt.Printf("%s Enter Control + C Shutdown Server \r\n", pkg.GetCurrentTimeStr())
// 等待中断信号以优雅地关闭服务器(设置 5 秒的超时时间)
quit := make(chan os.Signal, 1)
signal.Notify(quit, os.Interrupt)
<-quit
// Restored here, not deferred: from this point a second signal must reach
// the default handler, so a shutdown that hangs can still be interrupted.
disarmStopSignals()
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
defer cancel()
log.Info("Shutdown Server ... ")
if err := srv.Shutdown(ctx); err != nil {
log.Fatal("Server Shutdown:", err)
if err := shutdownServer(srv, shutdownTimeout); err != nil {
// Not log.Fatal: that is an unconditional os.Exit(1), and Shutdown
// reports an error exactly when connections were still in flight -
// which is when the cleanup that follows matters most.
log.Error("Server Shutdown: ", err)
}
log.Info("Server exiting")
return nil
}
// shutdownTimeout is how long Shutdown waits for in-flight requests. It plus
// whatever cleanup follows has to stay inside the orchestrator's grace period
// - `docker stop` allows 10s by default before it sends SIGKILL.
const shutdownTimeout = 5 * time.Second
// armStopSignals registers for the stop signals and returns the channel they
// arrive on together with the function that restores the default disposition.
//
// SIGTERM is what actually arrives in production: `docker stop`, a Kubernetes
// pod deletion and `systemctl stop` all send it, and Go terminates the process
// immediately for a signal nobody listens for. Registering only os.Interrupt
// meant every graceful shutdown below the wait was dead code outside a
// terminal.
//
// Registering is separate from waiting so a caller can arm before it announces
// that it is ready: a signal that arrives between the two is delivered to the
// default handler, which for both of these means the process dies without
// running any of this.
func armStopSignals() (<-chan os.Signal, func()) {
quit := make(chan os.Signal, 1)
signal.Notify(quit, os.Interrupt, syscall.SIGTERM)
return quit, func() { signal.Stop(quit) }
}
// shutdownServer stops srv, giving in-flight requests up to timeout to finish.
//
// It returns the error instead of exiting on it. A caller that exits here skips
// its own cleanup, and Shutdown fails precisely when there was something left
// to clean up after.
func shutdownServer(srv *http.Server, timeout time.Duration) error {
ctx, cancel := context.WithTimeout(context.Background(), timeout)
defer cancel()
return srv.Shutdown(ctx)
}
// runStartupHooks runs the router registries and then the before callbacks.
//
// The package-level slice runs first and in its existing order, so a fork that
+292
View File
@@ -0,0 +1,292 @@
package api
import (
"fmt"
"net"
"net/http"
"os"
"os/exec"
"strings"
"syscall"
"testing"
"time"
)
// The signal path cannot be exercised in-process: delivering a signal to the
// test binary would race with the test framework, and the disposition changes
// are global. So the test re-executes itself as a child, and the child runs the
// same armStopSignals / shutdownServer the server does.
//
// The child deliberately serves an empty http.Server rather than the real one:
// this repository's CI has no database (.github/workflows/go.yml runs neither
// MySQL nor a sqlite-tagged build), and none of what is under test needs one.
const (
childEnv = "GO_ADMIN_SIGNAL_CHILD"
childStuckEnv = "GO_ADMIN_SIGNAL_CHILD_STUCK"
childHangConn = "GO_ADMIN_SIGNAL_CHILD_HANGCONN"
markerReady = "CHILD-READY"
markerSignal = "CHILD-SIGNAL"
markerShutdown = "CHILD-SHUTDOWN-OK"
markerExiting = "CHILD-EXITING"
)
// TestSignalChild is the child process. It is skipped in a normal run.
func TestSignalChild(t *testing.T) {
if os.Getenv(childEnv) != "1" {
t.Skip("child process entry point")
}
ln, err := net.Listen("tcp", "127.0.0.1:0")
if err != nil {
fmt.Println("listen:", err)
os.Exit(3)
}
// accepted fires once the server has taken a connection off the listener.
// Dialling is not enough: Shutdown only waits for connections the server
// has already accepted, so calling it between the dial and the accept
// finds nothing to wait for and returns immediately.
accepted := make(chan struct{}, 1)
srv := &http.Server{
Handler: http.NewServeMux(),
ConnState: func(_ net.Conn, state http.ConnState) {
if state == http.StateNew {
select {
case accepted <- struct{}{}:
default:
}
}
},
}
go func() { _ = srv.Serve(ln) }()
// Arm before announcing readiness. Doing it the other way round leaves a
// window in which the parent's signal reaches the default handler and
// kills the child before any of this runs - which is exactly the failure
// this whole change is about, so the test must not reproduce it by
// accident.
quit, disarm := armStopSignals()
fmt.Println(markerReady)
os.Stdout.Sync()
sig := <-quit
disarm()
fmt.Println(markerSignal, sig)
os.Stdout.Sync()
if os.Getenv(childStuckEnv) == "1" {
// Stand in for a cleanup hook that never finishes. The point of
// restoring the signal disposition is that a second signal still
// reaches the default handler and kills this.
time.Sleep(2 * time.Minute)
}
timeout := shutdownTimeout
if os.Getenv(childHangConn) == "1" {
// Dialled here, not at start-up. net/http stops counting a StateNew
// connection against Shutdown once it is more than five seconds old,
// so a connection opened before the wait would age out on a slow CI
// run and Shutdown would succeed - leaving the test asserting nothing.
c, err := net.Dial("tcp", ln.Addr().String())
if err != nil {
fmt.Println("dial:", err)
os.Exit(5)
}
defer func() { _ = c.Close() }()
// And wait for the accept, for the opposite reason: an unaccepted
// connection is not one Shutdown waits for either.
select {
case <-accepted:
case <-time.After(10 * time.Second):
fmt.Println("the server never accepted the stalling connection")
os.Exit(6)
}
// A connection that has sent nothing keeps Shutdown busy: net/http
// only treats a StateNew connection as idle once it is more than five
// seconds old. A short budget makes the timeout deterministic without
// waiting out the real one.
timeout = 300 * time.Millisecond
}
if err := shutdownServer(srv, timeout); err != nil {
// Deliberately not fatal, and deliberately not a bare return: the
// point is that whatever follows still runs.
fmt.Println("shutdown error:", err)
} else {
fmt.Println(markerShutdown)
}
fmt.Println(markerExiting)
os.Stdout.Sync()
}
func startChild(t *testing.T, stuck bool, extraEnv ...string) (*exec.Cmd, *os.File, chan string) {
t.Helper()
r, w, err := os.Pipe()
if err != nil {
t.Fatalf("pipe: %v", err)
}
cmd := exec.Command(os.Args[0], "-test.run=TestSignalChild", "-test.v")
cmd.Env = append(os.Environ(), childEnv+"=1")
if stuck {
cmd.Env = append(cmd.Env, childStuckEnv+"=1")
}
cmd.Env = append(cmd.Env, extraEnv...)
cmd.Stdout = w
cmd.Stderr = w
if err := cmd.Start(); err != nil {
t.Fatalf("start child: %v", err)
}
_ = w.Close()
lines := make(chan string, 64)
go func() {
defer close(lines)
buf := make([]byte, 4096)
var acc strings.Builder
for {
n, err := r.Read(buf)
if n > 0 {
acc.Write(buf[:n])
for {
s := acc.String()
i := strings.IndexByte(s, '\n')
if i < 0 {
break
}
lines <- s[:i]
acc.Reset()
acc.WriteString(s[i+1:])
}
}
if err != nil {
if acc.Len() > 0 {
lines <- acc.String()
}
return
}
}
}()
t.Cleanup(func() {
_ = cmd.Process.Kill()
_, _ = cmd.Process.Wait()
_ = r.Close()
})
return cmd, r, lines
}
// await drains lines until one contains want, or the deadline passes. It
// returns everything it saw, so a failure says what the child actually did.
func await(t *testing.T, lines chan string, want string, d time.Duration) []string {
t.Helper()
var seen []string
deadline := time.After(d)
for {
select {
case l, ok := <-lines:
if !ok {
t.Fatalf("child output ended before %q; saw:\n%s", want, strings.Join(seen, "\n"))
}
seen = append(seen, l)
if strings.Contains(l, want) {
return seen
}
case <-deadline:
t.Fatalf("timed out waiting for %q; saw:\n%s", want, strings.Join(seen, "\n"))
}
}
}
// Acceptance 19. Registering only os.Interrupt meant SIGTERM - the signal
// `docker stop`, Kubernetes and systemd all send - terminated the process
// before any of the shutdown path ran. Both must now reach it.
func TestBothSignalsRunTheShutdownPath(t *testing.T) {
for _, tc := range []struct {
name string
sig syscall.Signal
}{
{"SIGINT", syscall.SIGINT},
{"SIGTERM", syscall.SIGTERM},
} {
t.Run(tc.name, func(t *testing.T) {
cmd, _, lines := startChild(t, false)
await(t, lines, markerReady, 30*time.Second)
if err := cmd.Process.Signal(tc.sig); err != nil {
t.Fatalf("signal: %v", err)
}
await(t, lines, markerSignal, 10*time.Second)
await(t, lines, markerShutdown, 10*time.Second)
await(t, lines, markerExiting, 10*time.Second)
if err := cmd.Wait(); err != nil {
t.Fatalf("child exited with %v, want a clean exit", err)
}
})
}
}
// Acceptance 20. quit is a buffered channel and signal.Notify stays armed, so
// without restoring the disposition a second signal only refills the buffer:
// once SIGTERM is registered, a shutdown that hangs could not be interrupted by
// anything short of SIGKILL.
func TestASecondSignalStillKillsAStuckShutdown(t *testing.T) {
cmd, _, lines := startChild(t, true)
await(t, lines, markerReady, 30*time.Second)
if err := cmd.Process.Signal(syscall.SIGTERM); err != nil {
t.Fatalf("first signal: %v", err)
}
await(t, lines, markerSignal, 10*time.Second)
// The child is now inside a cleanup that will not finish on its own.
if err := cmd.Process.Signal(syscall.SIGTERM); err != nil {
t.Fatalf("second signal: %v", err)
}
done := make(chan error, 1)
go func() { done <- cmd.Wait() }()
select {
case err := <-done:
if err == nil {
t.Fatal("child exited cleanly; it was supposed to be killed by the second signal")
}
case <-time.After(15 * time.Second):
t.Fatal("the second signal did not kill a stuck shutdown - the escape hatch is gone")
}
}
// Acceptance 21. srv.Shutdown reports an error exactly when connections were
// still in flight, and the old code answered that with log.Fatal - an
// unconditional os.Exit(1). Everything after it, which is where the cleanup
// hooks will hang, never ran. A failed Shutdown must not end the process.
func TestShutdownTimeoutDoesNotStopWhatFollows(t *testing.T) {
cmd, _, lines := startChild(t, false, childHangConn+"=1")
await(t, lines, markerReady, 30*time.Second)
if err := cmd.Process.Signal(syscall.SIGTERM); err != nil {
t.Fatalf("signal: %v", err)
}
await(t, lines, markerSignal, 10*time.Second)
seen := await(t, lines, markerExiting, 20*time.Second)
var timedOut bool
for _, l := range seen {
if strings.Contains(l, "shutdown error:") {
timedOut = true
}
}
if !timedOut {
t.Fatalf("Shutdown did not time out, so this test proves nothing; saw:\n%s",
strings.Join(seen, "\n"))
}
if err := cmd.Wait(); err != nil {
t.Fatalf("child exited with %v after a failed Shutdown, want a clean exit", err)
}
}
+599 -76
View File
@@ -1,64 +1,453 @@
# 公共契约面
> 本文写给**第三方应用作者**:你写一个装进 go-admin 的业务模块,可以依赖什么、
> 怎么注册进来、哪些东西随时可能变。
> 怎么接进来、哪些约定不遵守会**不报错地出错**。
>
> 主仓贡献者的编码约定见根目录 `AGENTS.md`,设计取舍见 `docs/architecture.md`。
---
## 承诺稳定的包
## 契约面在 core,不在 go-admin
| 包 | 用途 |
|---|---|
| `common/actions` | 通用 CRUD Action(Index / View / Create / Update / Delete / Permission) |
| `common/dto` | 分页、`search` tag 解析、`Control` / `Index` 接口 |
| `common/models` | `ActiveRecord`、`ControlBy`、`ModelTime`、`Model` |
| `common/middleware` | `AuthCheckRole`、`InitMiddleware` 等 |
这份文档以前列的是 go-admin 自己的四个包(`common/actions` 等),依据写的是
「把 `app/demo` 的 import 去重之后恰好就是这四个」。
**依据不是拍脑袋列的**:`app/demo` 是一个可编译、有测试、CI 会跑的标准 CRUD 模块,
把它的 `go-admin/` 前缀 import 全部去重之后,恰好就是这四个包 —— 它代表
"写一个标准模块所需要的最小依赖面"。你的模块如果需要第五个包,先在 issue 里说一声,
那多半意味着契约面缺了什么。
**那个依据是错的,而且错的方向是把人引向依赖宿主。**
"稳定"的含义:**在 `2.x` 内不做破坏性变更**。新增导出符号不算破坏;改签名、
改语义、删除导出符号算,会走 major 版本并在 release note 里单列。
go-admin 的使用方式是 clone / fork:每个使用者拿到的是一整份代码,然后**改它**。
应用如果依赖 `go-admin/common/actions`,它依赖的是一个**每个使用者都不一样、
而且随时在变**的东西——你没有办法测试自己的应用在别人改过的 fork 上能不能编译。
### 没有已知例外
还有一条更硬的:`go-admin` 这个 module path 没有点号,
按 Go 的规则**不是合法的可解析模块路径**:
这四个包**不 import `app/` 下的任何东西**,2026-08-31 起由 CI 强制
(见下方「边界由 CI 守着」)。在此之前有两处反向依赖,都已根治:
```
$ go get go-admin/common/models
go: malformed module path "go-admin/common/models": missing dot in first path element
```
| 原位置 | 反向依赖 | 处理 |
|---|---|---|
| `common/middleware/logger.go` | `app/admin/service/dto` 的两个操作日志状态常量 | 常量下沉到 `common/global`,`dto` 侧保留同名常量作为 deprecated 别名,fork 不受影响 |
| `common/middleware/handler/auth.go` | `app/admin/models` 的 `SysUser` / `SysRole` | 该段断言恒失败、设的是零值且开源版无人读取,属死代码,已删除 |
想 import 它就必须写 `replace`,而**非主模块的 `replace` 会被忽略**——
你在自己应用里写的 replace 对使用者不生效。所以「应用 require go-admin」
这条路不是不优雅,是走不通。
之所以不把它们记成"已知例外":这份文档的作用就是告诉你哪些包可以依赖,
如果第一条下面就挂着例外脚注,后来人会照着例外抄,边界从第一天起就是脏的。
契约面因此落在 **go-admin-core**:那是唯一一个大家都一样、有版本号、
不会被使用者随手改的东西。
---
## 其余包不保证稳定
## 承诺稳定的包
`common/` 下没有出现在上表里的包(`common/global`、`common/storage`、
`common/database`、`common/file_store`、`common/response`、`common/service`、
`common/apis`、`common/middleware/handler`、根 `common` 包……)以及
`app/admin` 的内部实现,**均不承诺稳定**。
全部在 `github.com/go-admin-team/go-admin-core/v2` 下:
其中 `common/global`、`common/middleware/handler`、根 `common` 包是
`common/middleware` 的编译期依赖 —— 它们会被一起拉进你的依赖图,但这不代表
它们的 API 稳定。**不要因为"都在 `common/` 目录下"就认为是契约面。**
| 包 | 用途 |
|---|---|
| `sdk/contract/models` | `Model` / `ControlBy` / `ModelTime` / `ActiveRecord` / `BaseUser` / `Migration`、`sys_menu.menu_type` 的三个枚举值 |
| `sdk/contract/dto` | `Pagination` / `MakeCondition` / `Paginate` / `OrderDest` / `ObjectById`、`Index` 与 `Control` 接口 |
| `sdk/contract/actions` | 数据权限设施:`DataPermission` / `Permission` / `PermissionAction` / `GetPermissionFromContext`、五个 `DataScope*` 常量与 `IsValidDataScope` |
| `sdk/contract/migration` | `Registry` / `AppRegistrar` / `ForApp` / `SetVersion` / `GetFilename` |
| `sdk/contract/seed` | `MenuSpec` / `ApiSpec` / `Seeder` / `SeedMenus`——往侧边栏和接口表里登记自己 |
| `sdk/pkg` | `GetOrm(c)`:从请求上下文取本租户的数据库连接 |
| `sdk/api`、`sdk/service` | 可选的 Api / Service 基类 |
| `response` | `OK` / `Error` / `PageOK`:响应格式 |
| `jwtauth/user` | 从 token 取当前用户身份 |
| `sdk/runtime` | 中间件 key 常量与 `GetHandlerFunc`:复用宿主已注册的鉴权链 |
规划中的 001(模块路径改名)会把非契约包移进 `internal/`,由编译器强制这条边界。
届时上表之外的包对外部模块直接不可见 —— 现在就照上表写,那次改动对你零成本。
`sdk/contract/` 这个前缀的含义就是「**承诺对应用稳定**的那一面」。core 里
`sdk/` 下的其他包是框架基础设施,语义不同——上表逐个列了名字,
**不要因为「都在 core 里」就认为是契约面**。
"稳定"的含义:**在 core 的 `v2.x` 内不做破坏性变更**。新增导出符号不算破坏;
改签名、改语义、删除导出符号算,会走 major 版本并在 release note 里单列。
准确的语义以 core 那份文档为准:
[go-admin-core `docs/contract.md`](https://github.com/go-admin-team/go-admin-core/blob/main/docs/contract.md)。
本文写的是宿主这一侧——它管不着的那些。
### go-admin 自己的包
`go-admin/common/models`、`common/dto`、`common/actions` 里的契约类型现在是
**指向 core 的类型别名**(`type X = corepkg.X`),主仓和所有 fork 的存量代码
一行不用改。别名在编译期就是同一个类型,不是"兼容层"。
但**新写的应用不要 import 它们**——那样就又依赖上宿主了。
---
## 契约面是三层,不是一层
划分依据不是"应用会 import 哪些包",而是**"哪一条不遵守会静默出错"**:
| 层 | 内容 | 判据 |
|---|---|---|
| **一 · 必须遵守** | 路由注册、从 context 取库、响应 shape、`ControlBy`/`ModelTime`、鉴权、数据权限、事务范式 | 不遵守 → **不报错,行为悄悄不对** |
| **二 · 可选便利** | `api.Api`、`service.Service`、CRUD Action、`MakeCondition` | 用不用都对 |
| **三 · 今天空白** | 应用间调用、领域事件、缓存租户隔离 | **没有。别自己发明** |
**框架不强制任何一层抽象。** 一个不用任何便利层的 handler 完全合法:
```go
func handler(c *gin.Context) {
db, err := pkg.GetOrm(c)
if err != nil {
response.Error(c, 500, err, "")
return
}
var list []MyModel
if err := db.Find(&list).Error; err != nil {
response.Error(c, 500, err, "")
return
}
response.OK(c, list, "")
}
```
第一层则是不管你用不用便利层都要遵守的,逐条写在下面,每条都附**不遵守会怎样**。
---
## 第一层:不遵守就静默出错
### 1. 路由注册
见下方「注册路由」一节。
**不遵守会怎样**:注册表在 `RunAppRouters()` 之后就封闭了,晚到的注册被丢弃,
只记一条 ERROR 日志。包级 `AppRouters` 连这个都没有——它就是一个普通 slice,
什么时候 append 都"成功",启动钩子之后 append 的那些永远不会执行,**且不出声**。
### 2. 数据库连接从 context 取,不用全局变量
```go
db, err := pkg.GetOrm(c) // 唯一正确的取法
```
`common/middleware/db.go` 在每个请求上按 `c.Request.Host` 挑出本租户的连接
放进 context:
```go
c.Set("db", sdk.Runtime.GetDbByTenant(c.Request.Host).WithContext(c))
```
**不遵守会怎样**:连接是**按租户注册**的(`SetDbByTenant(host, db)`),
`GetOrm(c)` 按 `c.Request.Host` 挑。你要是在启动时把某个连接存进包级变量再一直用,
多租户部署下所有租户的读写就都落到那一个库上——不报错、不告警,数据串了才发现。
这个坑在本仓库真踩过:`common/global.Driver` 取的是启动循环
**迭代到的第一个**库的驱动(`common/database/initialize.go`),
而 Go 的 map 迭代顺序是随机的——两个库用不同驱动时,那个值每次启动都可能不一样。
所以「一个进程一个库」这个假设不要写进任何一行代码。
### 3. 响应 shape
一律用 `response.OK` / `response.Error` / `response.PageOK`,不要自己
`c.JSON`。它们发出去的形状是:
```jsonc
// 成功
{"requestId": "...", "code": 200, "data": {...}}
// 分页:data 里再套一层
{"requestId": "...", "code": 200, "data": {"count": 42, "pageIndex": 1, "pageSize": 10, "list": [...]}}
// 失败
{"requestId": "...", "code": 500, "msg": "...", "status": "error"}
```
**HTTP 状态码永远是 200**,业务码在 body 的 `code` 里——这是既定行为,
`response.Error` 走的是 `c.AbortWithStatusJSON(http.StatusOK, res)`。
**不遵守会怎样**:前端 `src/utils/request.ts` 的响应拦截器只读 body 的 `code`,
`code !== 200` 就弹一条 `msg` 内容的 error toast 并 reject。你自己
`c.JSON(200, myThing)` 的话 `code` 是 `undefined`,界面上弹出来的是**一条空的
错误提示**,数据到不了页面。列表更安静:`useTable.ts` 读的是
`page?.list ?? []` 和 `page?.count ?? 0`,形状对不上就是**一张空表,零报错**。
### 4. `ControlBy` 与 `ModelTime`
每张业务表的 model 都嵌这三个:
```go
type Order struct {
models.Model // Id
// ... 你的字段 ...
models.ControlBy // CreateBy / UpdateBy
models.ModelTime // CreatedAt / UpdatedAt / DeletedAt
}
func (Order) TableName() string { return "app_order" } // 必须显式声明
```
`ControlBy` 提供 `create_by` 列,**数据权限的每一条 SQL 都 join 在它上面**。
`ModelTime` 的 `DeletedAt` 是 `soft_delete.DeletedAt`(毫秒时间戳,活行为 0,
永不为 NULL),不是 `gorm.DeletedAt`。
**不遵守会怎样**:
- 嵌了 `ControlBy` 但写入时忘了 `SetCreateBy(user.GetUserId(c))`,
`create_by` 就是 0。除「全部数据权限」外的每一档都**查不到任何数据**,
而且不报错——看起来像"这个用户还没建过数据"。
- 用错 `ModelTime` 版本(可空的 `gorm.DeletedAt`):gorm 按
`deleted_at IS NULL` 过滤,而活行里存的是 0,于是**整张表一行都查不出来**。
主仓的 `sys_columns` / `sys_tables` 真在这个状态下待过——代码生成器
一张表都列不出来,没有任何报错。`make checksilent` 的 `modeltime-mix`
就是为这条加的。
- `TableName()` 忘了写:GORM 配了 `SingularTable`,不会推导复数,表名会是
你没预料的那个。
### 5. 鉴权:用宿主已注册的中间件,不要自己造
```go
jwtCheck, ok := sdk.Runtime.GetHandlerFunc(runtime.JwtTokenCheck)
if !ok {
log.Fatal("JwtTokenCheck is not registered; is the host started via cmd/api?")
}
roleCheck, _ := sdk.Runtime.GetHandlerFunc(runtime.RoleCheck)
permCheck, _ := sdk.Runtime.GetHandlerFunc(runtime.PermissionCheck)
g := v1.Group("/order").Use(jwtCheck).Use(roleCheck).Use(permCheck)
```
三个 key 的常量在 `sdk/runtime`,宿主启动时把三个中间件注册进去。
**不遵守会怎样**:`GetHandlerFunc` 在"没注册"和"注册成了别的类型"两种情况下
都返回 `ok=false` 而不是 panic——**因为路由注册跑在 core 的 panic 护栏里面,
裸类型断言 panic 之后日志报的是"这个模块一条路由都没注册上",跟真实原因对不上**。
所以 `ok` 必须自己判,判出来要**大声失败**:一个跳过鉴权继续注册的路由,
就是一条静默的匿名可访问接口。
**宿主必须注册绑定过的闭包。** 三个 key 存的都得是 `gin.HandlerFunc`——
比如 `authMiddleware.MiddlewareFunc()`,**不是** `(*jwt.GinJWTMiddleware).MiddlewareFunc`。
后者是方法表达式,没有接收者绑在上面,取回来断言不成 `gin.HandlerFunc`,
怎么断言都做不成一个能用的 handler。
> **当前状态**:`common/middleware/init.go` 里 `RoleCheck` 与 `PermissionCheck`
> 注册的是 `AuthCheckRole()` 和 `actions.PermissionAction()`,都是绑定过的闭包,
> 取回来就能用;**`JwtTokenCheck` 注册的还是那个方法表达式**,所以今天对它
> `GetHandlerFunc` 拿到的是 `ok=false`。上面那段 `log.Fatal` 会在启动时打出来——
> 这是有意的,宁可起不来也不要一条没鉴权的路由。主仓这一处的修复见 F10,
> 修完之后本段可以删掉。
还有一条**不影响行为但影响理解**的:主仓今天四个模块各自调一次 `AuthInit()`
(`app/admin`、`app/jobs`、`app/other`、`app/demo`),也就是有四个 JWT 实例。
这不产生行为差异——配置同源(`config.JwtConfig`),JWT 校验是无状态的,
不看实例身份。但它意味着 `GetHandlerFunc(runtime.JwtTokenCheck)` 取回来的是
**最后注册进去的那一个**。要让应用拿到一个有意义的共享实例,宿主应当在注册路由
之前构造一次,而不是每个模块构造一次。
**测的时候别用 `admin` 账号。** `AuthCheckRole` 里 `rolekey == "admin"` 直接
`c.Next()`,**完全跳过 Casbin**。拿 admin 压任何鉴权路径都测不到东西。
### 6. 数据权限
两件事都要做:
```go
// 路由上挂中间件(上一节的 permCheck 就是它)
g := v1.Group("/order").Use(permCheck)
// 查询里组合 scope
p := actions.GetPermissionFromContext(c)
db.Scopes(actions.Permission(Order{}.TableName(), p)).Find(&list)
```
`sys_role.data_scope` 有五档,`Permission()` 按它拼 WHERE 条件:
| 值 | 常量 | 含义 | 条件 |
|---|---|---|---|
| `1` | `DataScopeAll` | 全部数据权限 | 不加条件 |
| `2` | `DataScopeCustom` | 自定义数据权限 | `create_by` 属于 `sys_role_dept` 关联到的部门 |
| `3` | `DataScopeDept` | 本部门 | `create_by` 属于本部门 |
| `4` | `DataScopeDeptTree` | 本部门及以下 | `create_by` 属于 `dept_path` 匹配的子树 |
| `5` | `DataScopeSelf` | 仅本人 | `create_by = 当前用户` |
自己往 `sys_role.data_scope` 写值的话先过一遍 `IsValidDataScope`——
写进去的非法值不会在写入时报错,只会在**每一次查询**里静默地什么都查不到。
**不遵守会怎样**,两种漏法的方向相反,值得分清:
- **查询里忘了组合 `Permission()`** —— 就是**全量可见**,每个角色都看得到所有人
的数据,不报错、不记日志。**这是本框架里最贵的一类静默失败**,所以那一行
`db.Scopes(...)` 不是"最佳实践",是契约。
- **组合了 `Permission()` 但路由上漏挂中间件** —— 上下文里没有 `PermissionKey`,
拿到的是零值,`DataScope` 是空串,落进下面那个 fail-closed 的 default,
结果是**一行都查不到**。方向反了,至少还看得见。
五档之外的值(空串、拼错的、还没迁移的老数据)落到 `default` 分支,
那里是 **fail closed**:加一条 `1 = 0`,什么都不返回。注意 `1`(全部数据权限)
是**显式列出的一个 case**,不是"落到 default"——两者曾经是同一条路,
于是"没配置"和"配置成看全部"产出的 SQL 一个字都不差。
`3` / `4` 两档在 `DeptId <= 0` 时同样 fail closed。原因是
`sys_dept.dept_path` 一律以 `/0/` 开头,`dept_id=0` 会把 LIKE 模式变成
`'%/0/%'`,**命中全表**——本来想表达"没有部门",实际表达的是"全部部门"。
数据权限还有一个**全局开关** `application.enabledp`,默认是 `false`。
关掉时 `Permission()` 原样返回查询、`PermissionAction()` 直接放行——
**你的应用在默认配置下测不出数据权限的任何行为**,要验证得先把它打开。
**不要自己重写这段 SQL。** 那 20 行里埋着 8 项内部知识:JWT claims 的私有键名
(`datascope` / `deptid`)、`sys_user`↔`sys_role` 的 join、`sys_role_dept`
关联表、`sys_dept.dept_path` 的 `/0/1/2/` 编码、`create_by` 的归属约定、
`enabledp` 开关、老 token 的回落逻辑。**而且写错的方向是越权。**
仓库里有过一份第二实现,`dept_path` 的匹配写成 `"%"+id+"%"` 少了两个斜杠,
`dept_id=1` 会匹配上 `/11/`、`/21/`、`/100/`——写它的人比第三方更懂这套约定,
仍然写错了。那份实现已经删掉了。
### 7. 事务范式
**业务层的事务一律用 `Transaction()` 闭包形式**:
```go
err := db.Transaction(func(tx *gorm.DB) error {
if err := tx.Create(&order).Error; err != nil {
return err // rolled back
}
return tx.Model(&stock).Where("qty >= ?", n).
UpdateColumn("qty", gorm.Expr("qty - ?", n)).Error
})
```
GORM 自己处理提交、回滚,以及 **panic 时的回滚**。
**不要照抄 `app/admin/service/sys_role.go`。** 那里有 5 处手写的
`Begin` / `defer` 写法,三个缺陷都是静默的:
```go
tx := e.Orm
if config.DatabaseConfig.Driver != "sqlite3" { // 缺陷 2
tx = e.Orm.Begin()
defer func() {
if err != nil { tx.Rollback() } else { tx.Commit() } // 缺陷 1
}()
}
```
1. **panic 时提交半截事务**——defer 只看 `err`,panic 时 `err` 仍是 nil,走的是
`Commit()`
2. **sqlite 下根本不开事务**——那一整个特判让 `tx` 就是 `e.Orm` 本身,
写一半失败留一半
3. **读 `config.DatabaseConfig.Driver`**——那是全局单库配置,多租户下不是
当前租户的驱动
缺陷 1 不止那一处:`app/admin/service/sys_dept.go`、`sys_menu.go`、
`app/other/models/tools/sys_tables.go` 用的是同一个 `defer` 写法
(没有 sqlite 特判,所以只有缺陷 1)。**整个 `Begin`/`defer` 家族都别照抄。**
同一个仓库里就有正确的参照:`cmd/migrate/migration/version/` 下 7 个迁移里
5 个用的是闭包形式(另外两个是纯 DDL 标记,DDL 在 MySQL 下本来就不进事务),
且这条路在 sqlite 下实测跑得通(`make build-sqlite`)。
主仓那些写法本批次不改,单独跟。
**并发保护用条件更新 + `RowsAffected`**,不要"先查后改":
```go
res := tx.Model(&Order{}).Where("id = ? AND status = ?", id, StatusPending).
Update("status", StatusPaid)
if res.Error != nil { return res.Error }
if res.RowsAffected == 0 { return ErrAlreadyPaid } // 别人先改了
```
---
## 第二层:可选便利
用不用都对,**不用不会出任何问题**:
| 东西 | 在哪 | 是什么 |
|---|---|---|
| `api.Api` | core `sdk/api` | 一条链式糖:`MakeContext` / `Bind` / `MakeOrm` / `OK` / `PageOK` / `Error` |
| `service.Service` | core `sdk/service` | 一个装 `Orm` / `Log` / `Cache` / `Error` 的结构体加一个 `AddError` |
| `MakeCondition` / `search` tag | core `sdk/contract/dto` | 把 DTO 上的 `search:"type:exact;column:name;table:xx"` 翻成 WHERE |
| 通用 CRUD Action | go-admin `common/actions` | `IndexAction` 等五个。**留在 go-admin,没有下沉** |
最后一行是有意的:CRUD Action 是最需要演进的一类东西(分页参数、批量操作、
软删语义、字段级权限),而 core 的每一个导出都是永久承诺——放进去容易,
拿出来不可能。想用就把那 294 行抄走,抄走的那份还能按你自己的需要改。
主仓唯一的真实业务模块 `app/admin` **一个 CRUD Action 都没用**,全是手写 Service。
`MakeCondition` 返回的是 `func(db *gorm.DB) *gorm.DB` 闭包,方言从闭包里那个
`db.Dialector.Name()` 读,**必然是本租户那个库的驱动**,不需要你设置任何东西。
---
## 第三层:今天没有的
**明说没有,别自己发明**:
| 能力 | 现状 |
|---|---|
| 应用间调用 | 零定义。A 应用要调 B 应用只能直接 import 对方的包,循环依赖就回来了 |
| 领域事件 / EventBus | 无 |
| 缓存的租户隔离 | `service.Service` 有 `Cache` 字段,**是否按租户隔离未验证**。当作没隔离来写 |
| 异步任务 | 有队列,但热更新后消费者会丢(issue #892) |
这几条留给后续批次,按真实需求补——现在凭空设计只会设计错。
如果你的应用卡在这里,在 issue 里说一声,那正是我们要的输入。
---
## 装一个应用要接两处线
后端**两处**,漏掉第二处是**静默失败**:
```go
// 1. 路由:cmd/api/<name>.go
import _ "github.com/acme/go-admin-app-order/router"
// 2. 迁移:cmd/migrate/server.go 的 import 块里
import _ "github.com/acme/go-admin-app-order/migration"
```
两个都是空导入,作用只是让那个包的 `init()` 跑起来。
**漏了第二处会怎样**:不报错。`migrate` 命令照常跑完、照常打印成功,
你的建表和种子数据**就是不执行**。等到第一个请求打过来才会看到
"表不存在",而那时排查方向已经跑偏了。
`migrate --dry-run` 是确认接线成功的最快方式——它只读,可以直接对生产库跑:
```bash
go-admin migrate --dry-run -c config/settings.yml # 你的迁移应该出现在列表里
```
带界面的应用还有第三处,在前端仓库,见下一节。
---
## 前端:菜单 `component` 必须以 `apps/` 开头
前端那一处接线是 `go-admin-ui` 的 `apps.config.mjs`——加一条
`{ code: 'order', source: '...' }`,`source` 指到你的页面目录
(兄弟目录的相对路径,或 `./node_modules/@scope/app-order/views/order`)。
`scripts/sync-apps.mjs` 会在 `pnpm dev` 与 `pnpm build` 之前把它复制进
`src/apps/<code>/`,不需要手工跑。
`src/stores/permission.ts` 的 `appPath()` **只认路径第一段是 `apps`**,
其余一律当成主仓内置视图去 `src/views/` 下找。
所以你的菜单种子里 `Component` 必须写成:
```
apps/<code>/<该应用内的相对路径>/index
```
比如 `code` 是 `order` 的应用写 `apps/order/index`(开头带不带 `/` 都行,
只看第一段)。**不能**写成 `/order/index`。
**写错会怎样**:第一段是 `order` 而不是 `apps`,前端会去找一个不存在的
`src/views/order/index.vue`,页面摔到 `AppNotInstalled` 占位组件。
但控制台打印的是 `no component at src/views/order/index.vue`——
**跟真实原因(漏了 `apps/` 前缀)对不上**,排查时很容易被这条日志带偏。
对应的前端约定写在 go-admin-ui 的 `AGENTS.md`。另外一条:`source` 目录的内容
**原样**搬进 `src/apps/<code>/`,不会在 `code` 之外再自动插一层——想要
`apps/order/index` 这种最短形式,`source` 就要直接指到该应用**这一个页面模块**
的目录,而不是应用仓库的 `views` 根目录。
---
## 注册路由
一个应用模块要注册自己的路由,写一个 `func()` 签名的 `InitRouter`
(照抄 `app/demo/router/router.go`),然后二选一接进来:
写一个 `func()` 签名的 `InitRouter`(照抄 `app/demo/router/router.go`),
然后二选一接进来:
```go
// 方式一(历史写法,仍然有效):在主仓 cmd/api/<name>.go 里
@@ -68,36 +457,30 @@ AppRouters = append(AppRouters, router.InitRouter)
sdk.Runtime.SetAppRouters(router.InitRouter)
```
方式二是本次新接上的。差别只有一个但很关键:方式一要求你的模块
`import "go-admin/cmd/api"` —— 那是主程序的命令包,让业务模块依赖它很别扭,
也正是"主仓要为每个模块加一个七行文件"的根源。
**第三方应用只能走方式二**——方式一要求 `import "go-admin/cmd/api"`,
那就又依赖上宿主了。
**执行顺序**:先跑完包级 `AppRouters`,再由 core 的 `sdk.Runtime.RunAppRouters()`
跑它自己的注册表,各自内部保持注册顺序。别依赖跨来源的相对顺序,各模块的
`RouterGroup` 前缀互不相同,本来就不该有顺序依赖。
走方式二还多拿到两样东西,都在 core 那边实现:
走方式二还多拿到两样东西,都在 core 那边实现(见
[core 的 `docs/contract.md`](https://github.com/go-admin-team/go-admin-core/blob/main/docs/contract.md)):
**panic 护栏**——你的 `InitRouter` panic 了,其余模块照常注册、进程不退出,日志里会写明
是哪一行注册的;**失败分级**——`sdk.Runtime.SetAppRoutersWith(f, runtime.WithFatal())`
声明「我起不来就别启动」。方式一(包级 `AppRouters`)没有护栏,panic 直接掀桌。
- **panic 护栏**——你的 `InitRouter` panic 了,其余模块照常注册、进程不退出,
日志里会写明是哪一行注册的
- **失败分级**——`sdk.Runtime.SetAppRoutersWith(f, runtime.WithFatal())`
声明「我起不来就别启动」
`InitRouter()` 内部的约定:自己拿 `sdk.Runtime.GetEngine()`,按需建
方式一(包级 `AppRouters`)没有护栏,panic 直接掀桌。
**执行顺序**:先跑完包级 `AppRouters`,再由 `sdk.Runtime.RunAppRouters()`
跑 core 自己的注册表,各自内部保持注册顺序。别依赖跨来源的相对顺序。
`InitRouter()` 内部:自己拿 `sdk.Runtime.GetEngine()`,按需建
`gin.RouterGroup`,通过 `init()` 自注册到你自己包内的
`routerCheckRole` / `routerNoCheckRole` 列表,不在任何中心文件手工列举
(与 `AGENTS.md`「路由注册」一节一致)。
`routerCheckRole` / `routerNoCheckRole` 列表,不在任何中心文件手工列举。
---
## 注册数据库迁移
框架自身的迁移不变:
```go
migration.Migrate.SetVersion(migration.GetFilename(fileName), _1786700001000DemoMenu)
```
应用的迁移走 `ForApp`:
框架自身的迁移用 `SetVersion`;应用的迁移走 `ForApp`:
```go
func init() {
@@ -108,12 +491,20 @@ func init() {
func initCrmTables(db *gorm.DB, version, appCode string) error {
return db.Transaction(func(tx *gorm.DB) error {
// ... schema / data changes ...
return tx.Create(&common.Migration{Version: version, AppCode: appCode}).Error
return tx.Create(&models.Migration{Version: version, AppCode: appCode}).Error
})
}
```
四条必须知道的规则:
注册面(`ForApp` / `SetVersion` / `GetFilename`)在 core 的
`sdk/contract/migration`,是一个**进程级的包级注册表**——`ForApp` 直接当包级函数
调,不需要从宿主手里接过什么句柄。**执行面**——读 `sys_migration`、排序、跑事务、
`migrate` 与 `migrate status` 两个命令——留在宿主,它通过 `Snapshot()` 读那张表。
仓库内的模块继续经 `go-admin/cmd/migrate/migration` 走,那个包现在是薄壳,
导入路径不变;外置应用直接 import core 的那个包,**两边写法一模一样**。
五条必须知道的规则:
1. **完成记录由迁移函数自己写**,而且要写在自己的事务里。框架的调度循环只做
"这个 version 在 `sys_migration` 里有没有" 的判断,从不代你插入 —— 这样
@@ -122,12 +513,17 @@ func initCrmTables(db *gorm.DB, version, appCode string) error {
schema 上那一列等于白加,你的迁移会被记成框架的。
3. **落库的 `version` 是加了前缀的**。`ForApp("crm")` 注册 `1786800001000`,
实际写进 `sys_migration.version` 的是 `crm-1786800001000`,函数收到的
`version` 参数已经是这个带前缀的值,照抄进 `common.Migration{Version: version}`
`version` 参数已经是这个带前缀的值,照抄进 `models.Migration{Version: version}`
即可。前缀的意义是:两个来源不同的应用哪怕碰巧生成同一个毫秒时间戳,也不会撞主键、
不会有一方被误判为"已应用"。
4. **应用 code 一律小写**,`ForApp` 会自己 `strings.ToLower` 一遍。`core` 是保留字
(`migrate status` 用它表示框架自身,`--app core` 选中框架),`ForApp("core")`
会 panic。
5. **文件名前 13 位必须是毫秒时间戳**,`GetFilename` 就是从这里取版本号的。
不合规的名字会 panic,并把违规文件名报出来 —— 这是**故意的**:调用点全在
`init()` 里,没有 error 可返回,而另一条路是把文件名本身注册成"版本号"
(`add_orders.go` 恰好 13 个字符,只查长度是拦不住的),那样这条迁移
永远不会被执行,且不会有任何提示。宁可启动失败。
顺序保证:**同一应用内按版本号严格有序**。跨应用顺序不做承诺 —— 由于前缀的存在,
今天的实际顺序是"先跑完全部框架迁移,再按 appCode 字母序逐个应用跑完",
@@ -146,6 +542,98 @@ go-admin migrate --app crm -c config/settings.yml # 只跑 crm 的迁移
---
## 菜单与接口种子
一个带界面的应用要在侧边栏里出现,需要往四类数据里写东西:`sys_api`、
`sys_menu`、`sys_menu_api_rule`(菜单与接口的关联)、以及角色授权与 Casbin
策略(`sys_role_menu` / `casbin_rule`)。
**你不需要知道这些表长什么样。** `sdk/contract/seed` 让你只描述"我要什么",
由宿主决定"怎么写进它自己的表":
```go
// 在你自己的迁移里,用它自己的那个事务
err := seed.SeedMenus(tx, "order", []seed.MenuSpec{
{Code: "root", Kind: models.Directory, Title: "订单"},
{Code: "list", Parent: "root", Kind: models.Menu, Title: "订单列表",
Path: "/order", Component: "apps/order/index", ApiCodes: []string{"list"}},
}, []seed.ApiSpec{
{Code: "list", Title: "订单列表", Path: "/api/v1/order", Method: "GET"},
})
```
`Kind` 用的就是 `sdk/contract/models` 里 `sys_menu.menu_type` 的那三个值
(`Directory` / `Menu` / `Button`),不是另一套同值的常量。
`Component` 的写法见上面「前端」一节——**这里是最容易写错的一个字段**。
core 里**没有** `SysMenu`、没有 `SysApi`、没有任何表名。这是刻意划的边界:
这个框架的宿主里本来就已经有两份 `SysMenu`(一份冻结在迁移期、一份运行期),
两者在软删语义上不一致,害过人,为此专门建了一个仓库内的工具来守。
往 core 里再放第三份表结构,就等于在**唯一没有工具守着**的地方重造同一类 bug。
### `Sort` 有上界,越界会中断整场迁移
`sys_menu.sort` 声明为 `gorm:"size:4"`,MySQL 据此建成 **tinyint,取值 -128..127**。
sqlite 忽略宽度,所以越界值在本地测试里一路绿灯,到真实安装时是 Error 1264 ——
而且发生在一次迁移的**中途**,后面的迁移全部不再执行。
`make checksilent` 的 `menu-sort-overflow` 会扫出仓库树里的越界字面量,
**但它扫不到 module cache 里的应用**。外置应用只有宿主 Seeder 的运行期校验兜底。
### `MenuSpec` 没有菜单名字段,名字由宿主合成
前端用菜单名做 keep-alive 的缓存键。两个应用如果都取 `Code: "list"`,
缓存键就会撞在一起 —— 后打开的那个页面会拿到前一个的缓存实例。
所以宿主的 Seeder 不直接用 `Code` 当菜单名,而是用
**PascalCase(appCode) + PascalCase(Code)** 合成(`order` + `list` → `OrderList`)。
你不需要做什么,但要知道两件事:
- 菜单名不是你能指定的,也不必与 `Title` 一致 —— `Title` 才是界面上显示的文字
- 前端组件的 `name` 若要与菜单名对齐(`checksilent` 的 `menu-name-mismatch` 会比对),
按合成后的名字写,不是按 `Code`
---
## 应用配置节
不要改宿主的源码去加配置。`sdk/config.RegisterExtend` 让你认领
`extend:` 下自己那一节:
```go
type orderConfig struct {
PaymentEndpoint string
Timeout int
}
// 在 init() 里调,与 SetAppRouters / ForApp 同一约定
var getOrderConfig = config.RegisterExtend[orderConfig]("order")
func handler(c *gin.Context) {
cfg := getOrderConfig()
_ = cfg.PaymentEndpoint
}
```
```yaml
extend:
order:
PaymentEndpoint: https://payment.internal
Timeout: 30
```
每个 key 各自解码,互不覆盖。**同一个 key 注册两次会立刻 panic**——
注册期没有"封闭时刻"可以用来拒绝迟到的注册,所以重复只能在注册的那一刻
大声报出来,而不是让第二个人静默顶掉第一个人的配置节。
配置文件是被监听的,改动会触发重载。`RegisterExtend` 每次重载解码进一个全新的
`T` 再原子换指针,所以访问器拿到的永远是一个自洽的快照,请求路径上读它不需要加锁。
唯一要注意的:**不要跨两次调用拼一个视图**——从同一个返回值上读两个字段是一致的,
调两次访问器各读一个字段,中间夹一次重载就不是了。
---
## 硬约束:注册要赶在启动钩子之前
三个注册入口——`AppRouters`、`sdk.Runtime.SetAppRouters`、`migration.ForApp`——
@@ -153,18 +641,16 @@ go-admin migrate --app crm -c config/settings.yml # 只跑 crm 的迁移
`init()` 是最省事的位置:Go 规范保证包级变量初始化与 `init()` 在 `main()` 之前
**单 goroutine 顺序执行**,注册期天然没有并发写。但它不是唯一合法位置——
在 `run()` 之类早于启动钩子的地方注册同样成立。这条规则约束的是**顺序**,
不是你写在哪个函数里。
在 `run()` 之类早于启动钩子的地方注册同样成立。**这条规则约束的是顺序,
不是你写在哪个函数里。**
`sdk.Runtime.SetAppRouters` 的准确语义以 core 为准:
想在代码里判断注册窗口是否还开着:
> [go-admin-core `docs/contract.md`](https://github.com/go-admin-team/go-admin-core/blob/main/docs/contract.md)
```go
if sdk.Runtime.AppRoutersSealed() { /* RunAppRouters 已经跑过了 */ }
```
那份文档写明了注册类与资源类的划分、封闭时刻、护栏边界(**只覆盖同步 panic,
你自己 `go func()` 出去的 panic 框架够不着**)、以及配置热更新会在运行期
重新执行 setup 回调这件事。
主仓这边只补三条它管不着的:
主仓这边补三条 core 那份文档管不着的:
1. **`RunAppRouters()` 跑过之后,core 的注册表就封闭了**,再调
`sdk.Runtime.SetAppRouters` 会被丢弃并记一条 ERROR 日志。包级 `AppRouters`
@@ -181,19 +667,56 @@ go-admin migrate --app crm -c config/settings.yml # 只跑 crm 的迁移
```
`cmd/api/server_test.go` 里的 `freshRuntime` 就是这个。
3. **`migration.ForApp` 是主仓的东西**,core 不认识它,上面那份文档不覆盖它。
它的约束仍然是"注册要在迁移调度循环跑起来之前",实践上就是 `init()`。
3. **迁移的调度循环是主仓的东西**,core 只有注册面。迁移注册的约束仍然是
"赶在调度循环跑起来之前",实践上就是 `init()`。
---
## 安全边界:装一个应用等于信任它
**这一层划不出安全边界,本文不假装划得出。**
第三方应用的代码在**宿主进程内**运行,与宿主**同权限**。它持有的是裸的
`*gorm.DB`——`seed.SeedMenus` 用的就是你自己迁移里那个 `tx`,绕开 `Seeder`
直写 `sys_menu`、`sys_api`、甚至 `casbin_rule` 一直都做得到,Go 的类型系统
拦不住,本框架的任何一层也拦不住。
还有一条**不碰 `casbin_rule` 也能走通**的间接路径:把自己的菜单通过
`ApiCodes` 关联到别人的接口,然后等管理员在后台把这个菜单授权给某个角色——
策略是后台自己生成的,记在管理员头上。
所以:
> **装一个应用,等于信任它。** 这和 `import _` 一个 Go 库是同一量级的信任。
> `Seeder` 这类设计的目的是让**守规矩的应用不必知道宿主的表结构**,
> 不是把不守规矩的应用关起来。
给使用者的实际建议只有一条:**按信任 Go 依赖的标准来审应用**——看源码、
钉版本、认作者。不要因为它叫"应用"就以为它跑在沙箱里。
---
## 边界由 CI 守着
`common/`、`core/` 不得 import `app/`,这条由 `tools/checksilent` 的
`contract-import-boundary` 检查固化,`make checksilent` 在 CI 里跑,违反即失败
(测试文件同样算 —— 一个删掉 `app/admin` 的 fork 也应该能跑 `go test ./...`)。
`tools/checksilent` 里有两条盯契约面的检查,`make checksilent` 在 CI 里跑,
命中 ERROR 即失败:
靠人工评审列契约面会漏。上面那两处反向依赖里,第二处就是评审没发现、
靠机器全量扫描才找出来的。
| 检查 | 盯的是 |
|---|---|
| `contract-import-boundary` | `common/`、`core/` 不得 import `app/`——否则一个删掉 `app/admin` 的 fork 就编译不了它被告知可以依赖的那一面 |
| `contract-shim-alias` | 从 core 契约包声明出来的类型必须是**别名**(`type X = pkg.Y`),不能是 defined type。判据是右手边,不是一份包名清单,所以谁在哪加的都算 |
`tools/checksilent` 还检查另外五类"不出声的失败",写模块时值得先看一眼
第二条守的是一条一个字符的差别。`type X = pkg.Y` 和 `type X pkg.Y`
看着几乎一样,但后者只拿走底层结构、**丢掉整个方法集**,于是嵌了它的 model
不再满足 `ActiveRecord`。麻烦在于这**不一定在本仓编译失败**——本仓只用接口
使唤其中一部分类型,没被使唤到的那些在这里编译得好好的,
**到第三方应用或某个 fork 里才炸**,而那里没人看着。
测试文件同样算——一个删掉 `app/admin` 的 fork 也应该能跑 `go test ./...`。
**这两条工具都只扫仓库树。** 装在 module cache 里的第三方应用,
`checksilent` 一个文件都看不到。所以它保的是**这个仓库和它的 fork**,
不是你的应用——你的应用要自己跑自己的检查。
`checksilent` 还检查另外五类"不出声的失败",写模块时值得先看一眼
`go run ./tools/checksilent -h`。
+160 -2
View File
@@ -2,6 +2,7 @@ package main
import (
"fmt"
"go/ast"
"go/token"
"path"
"sort"
@@ -18,6 +19,7 @@ const (
checkConfigValue = "config-value-truncation"
checkMenuIDConflict = "menu-id-collision"
checkImportBoundary = "contract-import-boundary"
checkShimAlias = "contract-shim-alias"
)
// Package paths, relative to the module. Spelled once so a module rename
@@ -44,6 +46,7 @@ func runChecks(s *snapshot, opt options) ([]Finding, error) {
out = append(out, checkConfigValueLength(s)...)
out = append(out, checkMenuIDCollisions(s)...)
out = append(out, checkContractImportBoundary(s)...)
out = append(out, checkContractShimAlias(s)...)
if opt.UIDir != "" {
fs, err := checkMenuNames(s, opt.UIDir)
@@ -109,6 +112,9 @@ func checkModelTimeMixing(s *snapshot) []Finding {
frozen := s.pkg(pkgFrozenModels)
for _, sf := range s.Files {
if sf.isTest() {
continue
}
if strings.HasPrefix(sf.Path, "app/") && sf.Imports(frozen) {
tables := tableNames(sf)
for name, st := range structTypes(sf) {
@@ -166,6 +172,9 @@ func checkMenuSortOverflow(s *snapshot) []Finding {
)
var out []Finding
for _, sf := range s.Files {
if sf.isTest() {
continue
}
forEachStructLiteral(sf, func(lit structLiteral) {
if !s.isMenuModel(lit) {
return
@@ -204,6 +213,9 @@ func checkConfigValueLength(s *snapshot) []Finding {
const limit = 255
var out []Finding
for _, sf := range s.Files {
if sf.isTest() {
continue
}
forEachStructLiteral(sf, func(lit structLiteral) {
if lit.Name != "SysConfig" || !s.isModelPackage(lit.PkgPath) {
return
@@ -251,6 +263,9 @@ func checkMenuIDCollisions(s *snapshot) []Finding {
sites := map[int64][]site{}
for _, sf := range s.Files {
if sf.isTest() {
continue
}
forEachStructLiteral(sf, func(lit structLiteral) {
if !s.isMenuModel(lit) {
return
@@ -379,6 +394,135 @@ func checkContractImportBoundary(s *snapshot) []Finding {
return out
}
// ---------------------------------------------------------------------------
// check 7: a shim of a core contract type must be an alias
// ---------------------------------------------------------------------------
// coreModulePrefix and coreContractSegment together identify a package under
// core's contract namespace. Matched as prefix plus segment rather than as one
// literal path so that a major-version bump of core - which rewrites the
// /v2 in every import - does not quietly turn this check off.
const (
coreModulePrefix = "github.com/go-admin-team/go-admin-core/"
coreContractSegment = "/sdk/contract/"
)
// isCoreContractPkg reports whether an import path names one of core's
// contract packages.
func isCoreContractPkg(path string) bool {
return strings.HasPrefix(path, coreModulePrefix) && strings.Contains(path, coreContractSegment)
}
// checkContractShimAlias reports a shim of a core contract type that was
// written as a defined type instead of an alias.
//
// type ControlBy = models.ControlBy // alias: same type, same method set
// type ControlBy models.ControlBy // defined type: methods are gone
//
// The two lines differ by one character and by everything else. A defined type
// takes the underlying struct and none of the methods declared on it, so a
// model embedding the second one no longer has SetCreateBy or SetUpdateBy and
// no longer satisfies ActiveRecord - which is not a warning, it is a compile
// error, but only in code that actually uses the method set.
//
// That is why the compiler is not enough on its own. This repository exercises
// some of the contract types through interfaces and some not at all; the ones
// it does not exercise compile perfectly well as defined types here and break
// in a third-party application, or in a fork's own module, which is where
// nobody is looking. The check costs one field of the AST - a type alias
// records the position of its '=' - and covers the surface uniformly rather
// than covering whatever app/demo happens to touch this month.
//
// The trigger is the right-hand side, not a list of names: any type declared
// from a core contract package is one of these, whoever wrote it and whenever
// it was added. A type declared from a local struct literal is not caught by
// this - see ScannedShimAliases, which is what stops a run over a tree with no
// shims in it from reading as a clean bill of health.
func checkContractShimAlias(s *snapshot) []Finding {
var out []Finding
for _, sf := range s.Files {
forEachTypeSpec(sf, func(ts *ast.TypeSpec) {
qualifier, pkg, name, ok := qualifiedType(sf, ts.Type)
if !ok || !isCoreContractPkg(pkg) {
return
}
if ts.Assign.IsValid() {
return // "type X = pkg.Y", which is what it must be
}
out = append(out, s.finding(Error, checkShimAlias, sf, ts,
"%s is declared from %s.%s as a defined type, not an alias;\n"+
" a defined type keeps the fields and drops the method set, so anything embedding it stops satisfying\n"+
" the interfaces it satisfied before - here it may still compile, in a fork or a third-party app it does not.\n"+
" Write it as: type %s = %s.%s",
ts.Name.Name, qualifier, name, ts.Name.Name, qualifier, name))
})
}
return out
}
// ScannedShimAliases counts the type aliases into core's contract packages the
// snapshot holds, so the summary can say whether checkContractShimAlias found
// anything to guard at all.
//
// Reported for the same reason ScannedContractRoots is: before the contract
// packages are lowered into core there are no shims here, the check has
// nothing to look at, and a run that printed nothing would look exactly like a
// run over a tree that passed.
func ScannedShimAliases(s *snapshot) int {
n := 0
for _, sf := range s.Files {
forEachTypeSpec(sf, func(ts *ast.TypeSpec) {
_, pkg, _, ok := qualifiedType(sf, ts.Type)
if ok && isCoreContractPkg(pkg) && ts.Assign.IsValid() {
n++
}
})
}
return n
}
// forEachTypeSpec visits every type declaration in the file, including the
// ones inside a parenthesised type block.
func forEachTypeSpec(sf *sourceFile, fn func(*ast.TypeSpec)) {
for _, decl := range sf.Syntax.Decls {
gen, ok := decl.(*ast.GenDecl)
if !ok || gen.Tok != token.TYPE {
continue
}
for _, spec := range gen.Specs {
if ts, ok := spec.(*ast.TypeSpec); ok {
fn(ts)
}
}
}
}
// qualifiedType resolves a type expression that names a type in another
// package, returning that package's import path and the type name. A bare
// identifier, a struct literal or anything else reports false: this asks
// specifically "is the right-hand side pkg.Name", which is the shape both a
// correct shim and the mistake it guards against are written in.
// The qualifier returned is the one written in this file, which is not
// path.Base of the import path whenever the import is aliased - and the shim
// files alias every one of them (contractmodels, contractdto). A message that
// suggests a fix has to spell it the way the file already does, or the line it
// tells the author to write does not compile.
func qualifiedType(sf *sourceFile, typ ast.Expr) (qualifier, pkgPath, name string, ok bool) {
sel, isSel := typ.(*ast.SelectorExpr)
if !isSel {
return "", "", "", false
}
ident, isIdent := sel.X.(*ast.Ident)
if !isIdent {
return "", "", "", false
}
p, found := sf.imports[ident.Name]
if !found {
return "", "", "", false
}
return ident.Name, p, sel.Sel.Name, true
}
// migrationVersion reads the 13-digit timestamp a migration file name starts
// with. Files outside the two migration directories are not migrations, however
// they are named.
@@ -398,9 +542,23 @@ func migrationVersion(rel string) (int64, bool) {
return v, true
}
// isMenuModel reports whether a literal is one of the SysMenu models rather
// than, say, the SysMenu service struct that shares the name.
// isMenuModel reports whether a literal describes a menu row, whichever of
// the two shapes it is written in.
//
// A host module seeds a menu by building the SysMenu model directly. An
// application installed from outside this repository cannot reach that type,
// so it describes the same row as a seed.MenuSpec and hands it to the host's
// Seeder. Both end up in sys_menu and both are subject to its column widths,
// so a check that knew only the first shape would go quiet exactly when the
// author is furthest from the schema it protects.
//
// That is not hypothetical: this repository's own reference application was
// written with a Sort of 200 - past the tinyint sys_menu.sort is built as -
// and this check passed it, because a MenuSpec is not a SysMenu.
func (s *snapshot) isMenuModel(lit structLiteral) bool {
if lit.Name == "MenuSpec" && isCoreContractPkg(lit.PkgPath) {
return true
}
return lit.Name == "SysMenu" && s.isModelPackage(lit.PkgPath)
}
+216
View File
@@ -452,3 +452,219 @@ func TestComponentNameParsesBothVueStyles(t *testing.T) {
t.Error("a component with no declared name must not be compared")
}
}
// ---------------------------------------------------------------------------
const coreContractModels = "github.com/go-admin-team/go-admin-core/v2/sdk/contract/models"
// shimFixture writes one shim file declaring ControlBy from core's contract
// package, in whichever of the two forms the caller asks for.
func shimFixture(t *testing.T, decl string) string {
t.Helper()
return fixture(t, map[string]string{
"common/models/by.go": "package models\n\nimport \"" + coreContractModels + "\"\n\n" + decl + "\n",
})
}
func TestShimAliasDetectsADefinedType(t *testing.T) {
root := shimFixture(t, "type ControlBy models.ControlBy")
f := requireOne(t, check(t, root, options{}), checkShimAlias)
if f.Severity != "ERROR" {
t.Errorf("severity = %s", f.Severity)
}
if !strings.Contains(f.Message, "type ControlBy = models.ControlBy") {
t.Errorf("the message must spell out the fix; got %s", f.Message)
}
if f.File != "common/models/by.go" || f.Line != 5 {
t.Errorf("position = %s:%d", f.File, f.Line)
}
}
// The counterproof for the check above: the same fixture with the one
// character that makes it correct must produce nothing. Without this the check
// could be reporting every type declaration it sees and the test above would
// still pass.
func TestShimAliasAcceptsAnAlias(t *testing.T) {
root := shimFixture(t, "type ControlBy = models.ControlBy")
if got := only(t, check(t, root, options{}), checkShimAlias); len(got) != 0 {
t.Errorf("reported %v", got)
}
}
// A parenthesised type block is how a shim package with more than one type
// tends to get written, and a walker that only looked at single-spec
// declarations would skip all but the first.
func TestShimAliasReadsAParenthesisedBlock(t *testing.T) {
root := shimFixture(t, `type (
Model = models.Model
ControlBy models.ControlBy
ModelTime = models.ModelTime
)`)
f := requireOne(t, check(t, root, options{}), checkShimAlias)
if !strings.Contains(f.Message, "ControlBy") {
t.Errorf("message = %s", f.Message)
}
}
// A defined type over a package that is not core's contract namespace is
// somebody's ordinary code. The check exists for the surface core promises to
// keep stable, and reporting anything else would make it a style rule.
func TestShimAliasIgnoresOtherPackages(t *testing.T) {
root := fixture(t, map[string]string{
"app/demo/models/product.go": `package models
import "go-admin/common/models"
type Product models.Model
`,
})
if got := only(t, check(t, root, options{}), checkShimAlias); len(got) != 0 {
t.Errorf("reported %v", got)
}
}
// The version is part of core's import path and changes on every major bump.
// Matching the whole path literally would turn the check off on that day and
// say nothing about it.
func TestShimAliasSurvivesACoreMajorVersionBump(t *testing.T) {
root := fixture(t, map[string]string{
"common/models/by.go": `package models
import "github.com/go-admin-team/go-admin-core/v9/sdk/contract/models"
type ControlBy models.ControlBy
`,
})
if got := only(t, check(t, root, options{}), checkShimAlias); len(got) != 1 {
t.Errorf("findings = %v", got)
}
}
// A tree with no shims in it is the state of this repository until the
// contract packages are lowered, and the check saying nothing there must not
// be reported as a boundary being guarded.
func TestShimAliasCoverageIsReportedAsZeroWhenThereAreNoShims(t *testing.T) {
root := fixture(t, map[string]string{
"common/models/by.go": "package models\n\ntype ControlBy struct{}\n",
})
s, err := load(root)
if err != nil {
t.Fatalf("load: %v", err)
}
if n := ScannedShimAliases(s); n != 0 {
t.Errorf("ScannedShimAliases = %d, want 0", n)
}
var buf strings.Builder
if _, err := run(&buf, root, options{}, false); err != nil {
t.Fatalf("run: %v", err)
}
if !strings.Contains(buf.String(), "guarded nothing") {
t.Errorf("the summary must say the check covered nothing; got:\n%s", buf.String())
}
}
func TestShimAliasCoverageCountsTheAliasesItGuards(t *testing.T) {
root := shimFixture(t, `type (
Model = models.Model
ControlBy = models.ControlBy
)`)
s, err := load(root)
if err != nil {
t.Fatalf("load: %v", err)
}
if n := ScannedShimAliases(s); n != 2 {
t.Errorf("ScannedShimAliases = %d, want 2", n)
}
}
// A menu written as a seed.MenuSpec lands in the same sys_menu.sort column as
// one written as a SysMenu, so the same tinyint bound applies. Until this was
// covered, an application - the one author furthest from the schema - was the
// one the check went quiet for.
func TestMenuSortOverflowIsDetectedInAContractMenuSpec(t *testing.T) {
root := fixture(t, map[string]string{
"example/app-order/migration/migration.go": `package migration
import "github.com/go-admin-team/go-admin-core/v2/sdk/contract/seed"
func menus() []seed.MenuSpec {
return []seed.MenuSpec{
{Code: "dir", Sort: 200},
{Code: "ok", Sort: 20},
}
}
`,
})
f := requireOne(t, check(t, root, options{}), checkMenuSort)
if !strings.Contains(f.Message, "200") {
t.Errorf("finding should name the offending value, got: %s", f.Message)
}
}
// Every guard against a bad seeded value needs a test that writes the very
// value it rejects. Scanning _test.go made each of those guards report its
// own test - the check firing on the proof that it works.
func TestSeededValueChecksSkipTestFiles(t *testing.T) {
root := fixture(t, map[string]string{
"cmd/migrate/migration/models/models.go": frozenModelsPkg,
"app/admin/service/seed_test.go": `package service
import "go-admin/cmd/migrate/migration/models"
func fixtureMenus() []models.SysMenu {
return []models.SysMenu{
{MenuId: 9000, Sort: 900},
}
}
`,
})
if got := only(t, check(t, root, options{}), checkMenuSort); len(got) != 0 {
t.Fatalf("%s fired on a test fixture: %v", checkMenuSort, got)
}
}
// The other direction: the exemption must not turn the check off. A real
// seed - the thing that actually reaches MySQL - is still reported.
func TestSeededValueChecksStillCoverNonTestFiles(t *testing.T) {
root := fixture(t, map[string]string{
"cmd/migrate/migration/models/models.go": frozenModelsPkg,
"cmd/migrate/migration/version/1786700001000_seed.go": `package version
import "go-admin/cmd/migrate/migration/models"
func seed() []models.SysMenu {
return []models.SysMenu{
{MenuId: 9000, Sort: 900},
}
}
`,
})
f := requireOne(t, check(t, root, options{}), checkMenuSort)
if !strings.Contains(f.Message, "900") {
t.Errorf("finding = %+v", f)
}
}
// The suggested fix has to use the qualifier the file actually writes. Every
// shim in this repository aliases its import (contractmodels, contractdto),
// so building the message from path.Base of the import path told the author
// to write a line that does not compile.
func TestShimAliasSuggestionUsesTheInSourceQualifier(t *testing.T) {
root := fixture(t, map[string]string{
"common/models/by.go": "package models\n\nimport contractmodels \"" + coreContractModels +
"\"\n\ntype ControlBy contractmodels.ControlBy\n",
})
f := requireOne(t, check(t, root, options{}), checkShimAlias)
if !strings.Contains(f.Message, "type ControlBy = contractmodels.ControlBy") {
t.Errorf("the fix must name the import as this file spells it; got %s", f.Message)
}
if strings.Contains(f.Message, "= models.ControlBy") {
t.Errorf("the fix names a qualifier this file does not bind; got %s", f.Message)
}
}
+14 -2
View File
@@ -1,9 +1,9 @@
// Command checksilent reports the failures in this repository that do not
// announce themselves: no error, no log line, behaviour quietly wrong.
//
// Six checks, five of them ERROR and one WARN. An ERROR fails the run; a WARN
// Seven checks, six of them ERROR and one WARN. An ERROR fails the run; a WARN
// prints and does not. The split is not about how bad the consequence is - all
// six are bad - but about how certain the detection is. Everything reported as
// seven are bad - but about how certain the detection is. Everything reported as
// an ERROR is decided from this repository's own syntax. The one WARN compares
// against a second repository through a regular expression, and a check that
// can be wrong must not be able to stop a build, or the first response to it
@@ -102,4 +102,16 @@ func printSummary(w io.Writer, findings []Finding, opt options, s *snapshot) {
fmt.Fprintf(w, "The %s check covered %s; %s does not exist here and was not scanned.\n",
checkImportBoundary, strings.Join(scanned, ", "), strings.Join(absent, ", "))
}
// Same reason: a tree with no alias into core's contract packages gives
// this check nothing to look at, and its silence must not be read as a
// pass. That is now the interesting case rather than the expected one -
// the shims exist, so a count of zero means they stopped being aliases,
// or stopped being here.
if n := ScannedShimAliases(s); n == 0 {
fmt.Fprintf(w, "The %s check found no type alias into core's contract packages and guarded nothing.\n",
checkShimAlias)
} else {
fmt.Fprintf(w, "The %s check covered %d type alias(es) into core's contract packages.\n",
checkShimAlias, n)
}
}
+13
View File
@@ -38,6 +38,19 @@ type sourceFile struct {
consts map[string]int64 // package-level integer constants, filled per package
}
// isTest reports whether this file is a _test.go.
//
// The checks about a seeded value - a menu sort, a config value, a menu id, a
// soft-delete shape - are all about what reaches a real database through a
// migration, and a test fixture reaches none. Worse, each of those guards
// needs a test that writes the very value it rejects, so scanning test files
// makes every such guard report its own test. The import and alias checks do
// not skip tests: those are about the dependency graph, where a test file's
// import is as real as any other.
func (f *sourceFile) isTest() bool {
return strings.HasSuffix(f.Path, "_test.go")
}
// snapshot is every Go file under the root, parsed once and shared by all the
// checks.
type snapshot struct {