diff --git a/pluginkit/CHANGELOG.md b/pluginkit/CHANGELOG.md index ba2177e..d5caa64 100644 --- a/pluginkit/CHANGELOG.md +++ b/pluginkit/CHANGELOG.md @@ -7,6 +7,13 @@ Releases of this module are tagged `pluginkit/vX.Y.Z`. Releases up to - The module moved to `github.com/gopherium/framework/pluginkit`. - The module needs Go 1.27.1. +- `Host.Migrate` applies every `Migrator` in order without starting any plugin. +- `Host.Start` takes a stop grace and refuses one that is not above zero, a breaking change. +- A failed `Host.Start` stops the started plugins within the stop grace, even after its context ends. +- The generated wiring registers every plugin it can and returns one error naming each failure. +- The generated wiring imports the SDK as `sdk`, so an SDK package with another name compiles. +- `wire.Config` gains an optional `Reserved` list of ids no plugin may take. +- `wire.Run` refuses a plugin id the generated Go or TypeScript wiring cannot use as an import name. ## 0.5.0 - 2026-08-14 diff --git a/pluginkit/graphwire/CHANGELOG.md b/pluginkit/graphwire/CHANGELOG.md index c00c257..e80ad22 100644 --- a/pluginkit/graphwire/CHANGELOG.md +++ b/pluginkit/graphwire/CHANGELOG.md @@ -19,6 +19,10 @@ stdlib-only `pluginkit` module so its gqlparser dependency never enters - The module moved to `github.com/gopherium/framework/pluginkit/graphwire`. - The module needs Go 1.27.1. +### Fixed + +- `Run` refuses a graphql plugin whose Go name collides with a name of the generated wiring. + ## [0.3.0] - 2026-08-14 ### Added diff --git a/pluginkit/graphwire/generate.go b/pluginkit/graphwire/generate.go index 6ad1fdc..37a45b3 100644 --- a/pluginkit/graphwire/generate.go +++ b/pluginkit/graphwire/generate.go @@ -77,7 +77,7 @@ type imported struct{ alias, path string } func wiringImports(cfg Config, plugins []contributor, n naming) []imported { imports := []imported{{"graph", cfg.ExecImport}} if len(plugins) > 0 { - imports = append(imports, imported{goName(pathBase(cfg.CoreImport)), cfg.CoreImport}) + imports = append(imports, imported{coreImportName(cfg), cfg.CoreImport}) } if n.packageMode { imports = append(imports, imported{"sdk", cfg.SDKImport}) @@ -105,14 +105,6 @@ func writeImports(b *strings.Builder, cfg Config, plugins []contributor, n namin b.WriteString(")\n\n") } -// pathBase returns the last segment of an import path. -func pathBase(path string) string { - if i := strings.LastIndex(path, "/"); i >= 0 { - return path[i+1:] - } - return path -} - // writePassthrough renders the zero plugin root. func writePassthrough(b *strings.Builder, n naming) { fmt.Fprintf(b, "// %s returns the core resolver root, no plugin extends the graph.\n", n.rootFunc()) diff --git a/pluginkit/graphwire/graphwire.go b/pluginkit/graphwire/graphwire.go index 037f953..3859e12 100644 --- a/pluginkit/graphwire/graphwire.go +++ b/pluginkit/graphwire/graphwire.go @@ -9,7 +9,9 @@ import ( "errors" "fmt" "go/format" + "go/token" "os" + "path" "path/filepath" "regexp" "sort" @@ -263,7 +265,7 @@ func coreContributor(root string, cfg Config) (contributor, error) { return contributor{}, err } return contributor{ - alias: goName(filepath.Base(cfg.CoreImport)), + alias: coreImportName(cfg), path: cfg.CoreImport, field: "core", param: "core", @@ -271,13 +273,36 @@ func coreContributor(root string, cfg Config) (contributor, error) { }, nil } +// coreImportName returns the Go name the generated wiring imports the core package under. +func coreImportName(cfg Config) string { + return goName(path.Base(cfg.CoreImport)) +} + +// reservedNames are the Go names the generated wiring owns, keyed by whether it writes a named package. +var reservedNames = map[bool]map[string]bool{ + false: {"core": true, "graph": true, "init": true, "main": true}, + true: {"core": true, "graph": true, "init": true, "sdk": true, "errors": true, "error": true, "nil": true}, +} + +// refuseCollision rejects a graphql plugin id whose Go name collides with a name of the generated wiring. +func refuseCollision(cfg Config, id string) error { + name := goName(id) + if token.IsKeyword(name) || reservedNames[namingFor(cfg).packageMode][name] || name == coreImportName(cfg) { + return fmt.Errorf("graphwire: plugin %s: its Go name %s collides with the generated wiring", id, name) + } + return nil +} + // pluginContributors scans every graphql flagged plugin into contributor entries. -func pluginContributors(root string, manifests []manifest) ([]contributor, error) { +func pluginContributors(root string, cfg Config, manifests []manifest) ([]contributor, error) { var contributors []contributor for _, m := range manifests { if !m.GraphQL { continue } + if err := refuseCollision(cfg, m.ID); err != nil { + return nil, err + } scanned, err := scanPlugin(root, m) if err != nil { return nil, err @@ -326,7 +351,7 @@ func Run(root string, cfg Config) error { if err != nil { return err } - plugins, err := pluginContributors(root, manifests) + plugins, err := pluginContributors(root, cfg, manifests) if err != nil { return err } diff --git a/pluginkit/graphwire/graphwire_test.go b/pluginkit/graphwire/graphwire_test.go index 8f84179..3b7f13d 100644 --- a/pluginkit/graphwire/graphwire_test.go +++ b/pluginkit/graphwire/graphwire_test.go @@ -5,6 +5,7 @@ package graphwire import ( "os" "path/filepath" + "slices" "strings" "testing" ) @@ -550,6 +551,134 @@ func TestFlaggedPluginWithoutSchemaFails(t *testing.T) { } } +func TestRunRefusesAPluginIDCollidingWithTheWiring(t *testing.T) { + t.Parallel() + + modes := []struct { + name string + cfg Config + output string + ids []string + }{ + {"main mode", testConfig, filepath.Join("cmd", "myapp"), + []string{"core", "graph", "graphres", "init", "main", "type"}}, + {"package mode", packageConfig, filepath.Join("internal", "graphroot"), + []string{"core", "graph", "graphres", "init", "sdk", "errors", "error", "nil", "type"}}, + } + for _, mode := range modes { + for _, id := range mode.ids { + t.Run(mode.name+" "+id, func(t *testing.T) { + t.Parallel() + + root := t.TempDir() + writeCore(t, root) + writePlugin(t, root, id, + `{"id": "`+id+`", "name": "Colliding", "backend": "example.com/myapp/plugins/`+id+`", "graphql": true}`, + betaSchema) + if err := os.MkdirAll(filepath.Join(root, mode.output), 0o755); err != nil { + t.Fatalf("creating the output directory: %v", err) + } + wiring := filepath.Join(root, filepath.FromSlash(mode.cfg.WiringPath)) + if err := os.WriteFile(wiring, []byte("earlier wiring\n"), 0o644); err != nil { + t.Fatalf("writing the earlier wiring: %v", err) + } + + err := Run(root, mode.cfg) + + want := "graphwire: plugin " + id + ": its Go name " + id + " collides with the generated wiring" + if err == nil || err.Error() != want { + t.Errorf("Run() error = %v, want %q", err, want) + } + if kept, _ := os.ReadFile(wiring); string(kept) != "earlier wiring\n" { + t.Errorf("wiring after the refusal = %q, want the earlier file untouched", kept) + } + }) + } + } +} + +func TestRunAcceptsAnIDOnlyTheOtherModeOwns(t *testing.T) { + t.Parallel() + + modes := []struct { + name string + cfg Config + output string + ids []string + }{ + {"main mode", testConfig, filepath.Join("cmd", "myapp"), []string{"sdk", "errors", "error", "nil"}}, + {"package mode", packageConfig, filepath.Join("internal", "graphroot"), []string{"main"}}, + } + for _, mode := range modes { + for _, id := range mode.ids { + t.Run(mode.name+" "+id, func(t *testing.T) { + t.Parallel() + + root := t.TempDir() + writeCore(t, root) + writePlugin(t, root, id, + `{"id": "`+id+`", "name": "Usable", "backend": "example.com/myapp/plugins/`+id+`", "graphql": true}`, + betaSchema) + if err := os.MkdirAll(filepath.Join(root, mode.output), 0o755); err != nil { + t.Fatalf("creating the output directory: %v", err) + } + + if err := Run(root, mode.cfg); err != nil { + t.Errorf("Run() error = %v, want nil", err) + } + }) + } + } +} + +func TestRunRefusesAHyphenatedIDWhoseGoNameIsTheCoreImport(t *testing.T) { + t.Parallel() + + for _, core := range []string{"example.com/myapp/internal/graph_res", "example.com/myapp/internal/graph-res"} { + t.Run(core, func(t *testing.T) { + t.Parallel() + + root := t.TempDir() + writeCore(t, root) + writePlugin(t, root, "graph-res", + `{"id": "graph-res", "name": "Colliding", "backend": "example.com/myapp/plugins/graph-res", "graphql": true}`, + betaSchema) + cfg := packageConfig + cfg.CoreImport = core + + err := Run(root, cfg) + + want := "graphwire: plugin graph-res: its Go name graph_res collides with the generated wiring" + if err == nil || err.Error() != want { + t.Errorf("Run() error = %v, want %q", err, want) + } + }) + } +} + +func TestACoreImportWithATrailingSlashKeepsOneName(t *testing.T) { + t.Parallel() + + root := t.TempDir() + writeCore(t, root) + writePlugin(t, root, "graphres", + `{"id": "graphres", "name": "Colliding", "backend": "example.com/myapp/plugins/graphres", "graphql": true}`, + betaSchema) + cfg := packageConfig + cfg.CoreImport = "example.com/myapp/internal/graphres/" + + err := Run(root, cfg) + + want := "graphwire: plugin graphres: its Go name graphres collides with the generated wiring" + if err == nil || err.Error() != want { + t.Errorf("Run() error = %v, want %q", err, want) + } + imports := wiringImports(cfg, []contributor{{alias: "beta", path: "example.com/myapp/plugins/beta"}}, namingFor(cfg)) + if !slices.Contains(imports, imported{"graphres", cfg.CoreImport}) { + t.Errorf("wiringImports() = %v, want the core imported as graphres", imports) + } +} + func TestGraphQLPluginsRequireABackend(t *testing.T) { t.Parallel() diff --git a/pluginkit/host.go b/pluginkit/host.go index 79a6b64..4fed594 100644 --- a/pluginkit/host.go +++ b/pluginkit/host.go @@ -7,6 +7,7 @@ import ( "errors" "fmt" "net/http" + "time" ) // Host starts and stops a fixed set of plugins. @@ -27,9 +28,31 @@ func NewHost(plugins ...Plugin) *Host { return &Host{plugins: plugins} } -// Start migrates every [Migrator] plugin, then starts every plugin in registration -// order, stopping the already-started ones in reverse order when a start fails. -func (h *Host) Start(ctx context.Context) error { +// Start migrates and starts every plugin in order, stopping the started ones within stopGrace when one fails. +func (h *Host) Start(ctx context.Context, stopGrace time.Duration) error { + if stopGrace <= 0 { + return fmt.Errorf("pluginkit: the stop grace must stand above zero, got %v", stopGrace) + } + if err := h.Migrate(ctx); err != nil { + return err + } + for i, p := range h.plugins { + if err := safeCall(ctx, p.ID(), "start", p.Start); err != nil { + return errors.Join(err, h.rollBack(ctx, stopGrace, i-1)) + } + } + return nil +} + +// rollBack stops the plugins from index down under a context stopGrace bounds and the end of ctx cannot cancel. +func (h *Host) rollBack(ctx context.Context, stopGrace time.Duration, index int) error { + stopping, cancel := context.WithTimeout(context.WithoutCancel(ctx), stopGrace) + defer cancel() + return h.stopDownFrom(stopping, index) +} + +// Migrate applies the schema of every [Migrator] plugin in registration order, stopping at the first failure. +func (h *Host) Migrate(ctx context.Context) error { for _, p := range h.plugins { migrator, ok := p.(Migrator) if !ok { @@ -39,11 +62,6 @@ func (h *Host) Start(ctx context.Context) error { return err } } - for i, p := range h.plugins { - if err := safeCall(ctx, p.ID(), "start", p.Start); err != nil { - return errors.Join(err, h.stopDownFrom(ctx, i-1)) - } - } return nil } diff --git a/pluginkit/host_test.go b/pluginkit/host_test.go index 62674b8..edfd999 100644 --- a/pluginkit/host_test.go +++ b/pluginkit/host_test.go @@ -9,10 +9,15 @@ import ( "reflect" "slices" "testing" + "testing/synctest" + "time" "github.com/gopherium/framework/pluginkit" ) +// stopGrace is the stop budget the tests hand Start. +const stopGrace = time.Minute + var ( _ pluginkit.Plugin = (*fakePlugin)(nil) _ pluginkit.Migrator = (*migratingPlugin)(nil) @@ -107,7 +112,7 @@ func TestHostStartsInOrderAndStopsInReverse(t *testing.T) { &fakePlugin{id: "beta", calls: &calls}, ) - if err := host.Start(t.Context()); err != nil { + if err := host.Start(t.Context(), stopGrace); err != nil { t.Fatalf("Start() error = %v, want nil", err) } if err := host.Stop(t.Context()); err != nil { @@ -165,7 +170,7 @@ func TestHostMigratesBeforeStarting(t *testing.T) { &migratingPlugin{fakePlugin: fakePlugin{id: "beta", calls: &calls}}, ) - if err := host.Start(t.Context()); err != nil { + if err := host.Start(t.Context(), stopGrace); err != nil { t.Fatalf("Start() error = %v, want nil", err) } @@ -175,6 +180,101 @@ func TestHostMigratesBeforeStarting(t *testing.T) { } } +func TestHostMigrateRunsEveryMigratorWithoutStarting(t *testing.T) { + t.Parallel() + + var calls []string + host := pluginkit.NewHost( + &migratingPlugin{fakePlugin: fakePlugin{id: "alpha", calls: &calls}}, + &fakePlugin{id: "beta", calls: &calls}, + &migratingPlugin{fakePlugin: fakePlugin{id: "gamma", calls: &calls}}, + ) + + if err := host.Migrate(t.Context()); err != nil { + t.Fatalf("Migrate() error = %v, want nil", err) + } + + want := []string{"alpha migrate", "gamma migrate"} + if !slices.Equal(want, calls) { + t.Errorf("migrate calls = %v, want %v", calls, want) + } +} + +// callerKey keys the context value a test hands the host. +type callerKey struct{} + +// contextMigrator is a migrator that records the caller value its context carries. +type contextMigrator struct { + fakePlugin + seen *[]any +} + +// Migrate records the caller value of ctx. +func (c *contextMigrator) Migrate(ctx context.Context) error { + *c.seen = append(*c.seen, ctx.Value(callerKey{})) + return nil +} + +func TestHostMigrateHandsTheCallerContextToEveryMigrator(t *testing.T) { + t.Parallel() + + var calls []string + var seen []any + host := pluginkit.NewHost(&contextMigrator{fakePlugin: fakePlugin{id: "alpha", calls: &calls}, seen: &seen}) + ctx := context.WithValue(t.Context(), callerKey{}, "caller") + + if err := host.Migrate(ctx); err != nil { + t.Fatalf("Migrate() error = %v, want nil", err) + } + if err := host.Start(ctx, stopGrace); err != nil { + t.Fatalf("Start() error = %v, want nil", err) + } + + if want := []any{"caller", "caller"}; !slices.Equal(want, seen) { + t.Errorf("context values seen = %v, want %v from Migrate and from Start", seen, want) + } +} + +func TestHostMigrateWithNoPluginsDoesNothing(t *testing.T) { + t.Parallel() + + if err := pluginkit.NewHost().Migrate(t.Context()); err != nil { + t.Errorf("Migrate() error = %v, want nil", err) + } +} + +func TestHostMigrateStopsAtTheFirstFailure(t *testing.T) { + t.Parallel() + + errSchema := errors.New("schema exploded") + cases := []struct { + name string + lead *migratingPlugin + err string + }{ + {"a failing migration", &migratingPlugin{migrateErr: errSchema}, "pluginkit: alpha migrate: schema exploded"}, + {"a panicking migration", &migratingPlugin{migratePanic: true}, "pluginkit: alpha migrate panicked: boom"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + var calls []string + tc.lead.fakePlugin = fakePlugin{id: "alpha", calls: &calls} + host := pluginkit.NewHost(tc.lead, &migratingPlugin{fakePlugin: fakePlugin{id: "beta", calls: &calls}}) + + err := host.Migrate(t.Context()) + + if err == nil || err.Error() != tc.err { + t.Fatalf("Migrate() error = %v, want %q", err, tc.err) + } + if want := []string{"alpha migrate"}; !slices.Equal(want, calls) { + t.Errorf("migrate calls = %v, want %v", calls, want) + } + }) + } +} + func TestHostAbortsWhenMigrationFails(t *testing.T) { t.Parallel() @@ -185,7 +285,7 @@ func TestHostAbortsWhenMigrationFails(t *testing.T) { &migratingPlugin{fakePlugin: fakePlugin{id: "beta", calls: &calls}, migrateErr: errSchema}, ) - startErr := host.Start(t.Context()) + startErr := host.Start(t.Context(), stopGrace) if !errors.Is(startErr, errSchema) { t.Fatalf("Start() error = %v, want %v in its chain", startErr, errSchema) @@ -204,8 +304,10 @@ func TestHostRecoversMigrationPanic(t *testing.T) { &migratingPlugin{fakePlugin: fakePlugin{id: "beta", calls: &calls}, migratePanic: true}, ) - if err := host.Start(t.Context()); err == nil { - t.Fatal("Start() error = nil, want a recovered panic error") + err := host.Start(t.Context(), stopGrace) + + if want := "pluginkit: beta migrate panicked: boom"; err == nil || err.Error() != want { + t.Fatalf("Start() error = %v, want the migration's own %q", err, want) } } @@ -220,7 +322,7 @@ func TestHostRollsBackWhenStartFails(t *testing.T) { &fakePlugin{id: "gamma", calls: &calls}, ) - startErr := host.Start(t.Context()) + startErr := host.Start(t.Context(), stopGrace) if !errors.Is(startErr, errBoot) { t.Fatalf("Start() error = %v, want %v in its chain", startErr, errBoot) @@ -231,6 +333,163 @@ func TestHostRollsBackWhenStartFails(t *testing.T) { } } +// stopContext is what a Stop call saw of its context while it ran. +type stopContext struct { + err error + remaining time.Duration + bounded bool + caller any +} + +// stopRecorder is a plugin that records what its Stop saw of its context. +type stopRecorder struct { + fakePlugin + stopped *stopContext +} + +// Stop records the state of ctx while the call runs. +func (s *stopRecorder) Stop(ctx context.Context) error { + deadline, bounded := ctx.Deadline() + s.stopped = &stopContext{ctx.Err(), time.Until(deadline), bounded, ctx.Value(callerKey{})} + return s.fakePlugin.Stop(ctx) +} + +// hangingStopper is a plugin whose Stop waits for its context to end. +type hangingStopper struct { + fakePlugin +} + +// Stop waits for ctx to end and answers its error. +func (h *hangingStopper) Stop(ctx context.Context) error { + *h.calls = append(*h.calls, h.id+" stop") + <-ctx.Done() + return ctx.Err() +} + +// cancellingStarter is a plugin whose Start ends the startup context and fails with its error. +type cancellingStarter struct { + fakePlugin + cancel context.CancelFunc +} + +// Start ends the startup context and answers its error. +func (c *cancellingStarter) Start(ctx context.Context) error { + *c.calls = append(*c.calls, c.id+" start") + c.cancel() + <-ctx.Done() + return ctx.Err() +} + +func TestHostRollsBackUnderItsOwnStopGrace(t *testing.T) { + t.Parallel() + + var calls []string + ctx, cancel := context.WithCancel(context.WithValue(t.Context(), callerKey{}, "caller")) + defer cancel() + started := &stopRecorder{fakePlugin: fakePlugin{id: "alpha", calls: &calls}} + failing := &cancellingStarter{fakePlugin: fakePlugin{id: "beta", calls: &calls}, cancel: cancel} + host := pluginkit.NewHost(started, failing) + + startErr := host.Start(ctx, stopGrace) + + if !errors.Is(startErr, context.Canceled) { + t.Fatalf("Start() error = %v, want the cancelled start in its chain", startErr) + } + seen := started.stopped + if seen == nil { + t.Fatalf("rollback calls = %v, want alpha stopped", calls) + } + if seen.err != nil || !seen.bounded || seen.remaining <= 0 || seen.remaining > stopGrace { + t.Errorf("rollback Stop context = err %v, deadline in %v bounded %t, want live and within the stop grace", + seen.err, seen.remaining, seen.bounded) + } + if seen.caller != "caller" { + t.Errorf("rollback Stop context value = %v, want the caller's", seen.caller) + } + if want := []string{"alpha start", "beta start", "alpha stop"}; !slices.Equal(want, calls) { + t.Errorf("rollback calls = %v, want %v", calls, want) + } +} + +func TestHostStartRefusesAStopGraceThatIsNotAboveZero(t *testing.T) { + t.Parallel() + + for _, grace := range []time.Duration{0, -time.Second} { + t.Run(grace.String(), func(t *testing.T) { + t.Parallel() + + var calls []string + host := pluginkit.NewHost(&migratingPlugin{fakePlugin: fakePlugin{id: "alpha", calls: &calls}}) + + err := host.Start(t.Context(), grace) + + want := "pluginkit: the stop grace must stand above zero, got " + grace.String() + if err == nil || err.Error() != want || len(calls) != 0 { + t.Errorf("Start() error = %v, calls %v, want %q before anything migrates or starts", err, calls, want) + } + }) + } +} + +func TestHostStartAcceptsTheSmallestStopGrace(t *testing.T) { + t.Parallel() + + var calls []string + host := pluginkit.NewHost(&fakePlugin{id: "alpha", calls: &calls}) + + if err := host.Start(t.Context(), time.Nanosecond); err != nil { + t.Fatalf("Start() error = %v, want a one nanosecond grace accepted", err) + } + + if want := []string{"alpha start"}; !slices.Equal(want, calls) { + t.Errorf("start calls = %v, want %v", calls, want) + } +} + +func TestHostStartReportsAFailedRollbackStop(t *testing.T) { + t.Parallel() + + errBoot := errors.New("boot failed") + errHalt := errors.New("halt failed") + var calls []string + host := pluginkit.NewHost( + &fakePlugin{id: "alpha", stopErr: errHalt, calls: &calls}, + &fakePlugin{id: "beta", startErr: errBoot, calls: &calls}, + ) + + err := host.Start(t.Context(), stopGrace) + + want := "pluginkit: beta start: boot failed\npluginkit: alpha stop: halt failed" + if !errors.Is(err, errBoot) || !errors.Is(err, errHalt) || err.Error() != want { + t.Errorf("Start() error = %v, want %q with both failures in its chain", err, want) + } +} + +func TestHostStartEndsAHungRollbackAtTheStopGrace(t *testing.T) { + t.Parallel() + + synctest.Test(t, func(t *testing.T) { + errBoot := errors.New("boot failed") + grace := 97 * time.Minute + var calls []string + host := pluginkit.NewHost( + &hangingStopper{fakePlugin: fakePlugin{id: "alpha", calls: &calls}}, + &fakePlugin{id: "beta", startErr: errBoot, calls: &calls}, + ) + began := time.Now() + + err := host.Start(t.Context(), grace) + + want := "pluginkit: beta start: boot failed\npluginkit: alpha stop: context deadline exceeded" + if !errors.Is(err, errBoot) || !errors.Is(err, context.DeadlineExceeded) || err.Error() != want { + t.Errorf("Start() error = %v, want %q with both failures in its chain", err, want) + } + if waited := time.Since(began); waited != grace { + t.Errorf("rollback waited %v, want exactly the stop grace %v", waited, grace) + } + }) +} + func TestHostRecoversStartPanic(t *testing.T) { t.Parallel() @@ -240,7 +499,7 @@ func TestHostRecoversStartPanic(t *testing.T) { &fakePlugin{id: "beta", startPanic: true, calls: &calls}, ) - startErr := host.Start(t.Context()) + startErr := host.Start(t.Context(), stopGrace) if startErr == nil { t.Fatal("Start() error = nil, want a recovered panic error") @@ -260,7 +519,7 @@ func TestHostStopCollectsAllFailures(t *testing.T) { &fakePlugin{id: "alpha", stopErr: errAlpha, calls: &calls}, &fakePlugin{id: "beta", stopPanic: true, calls: &calls}, ) - if err := host.Start(t.Context()); err != nil { + if err := host.Start(t.Context(), stopGrace); err != nil { t.Fatalf("Start() error = %v, want nil", err) } diff --git a/pluginkit/pluginkit.go b/pluginkit/pluginkit.go index 322b1b3..55c3d0a 100644 --- a/pluginkit/pluginkit.go +++ b/pluginkit/pluginkit.go @@ -14,6 +14,7 @@ import ( type Plugin interface { ID() string Start(ctx context.Context) error + // Stop releases what the plugin holds, returning by the time ctx ends, whether or not Start ran. Stop(ctx context.Context) error } diff --git a/pluginkit/stop_test.go b/pluginkit/stop_test.go new file mode 100644 index 0000000..0bdfa87 --- /dev/null +++ b/pluginkit/stop_test.go @@ -0,0 +1,28 @@ +// SPDX-License-Identifier: Apache-2.0 + +package pluginkit_test + +import ( + "slices" + "testing" + + "github.com/gopherium/framework/pluginkit" +) + +func TestHostStopReachesPluginsThatNeverStarted(t *testing.T) { + t.Parallel() + + var calls []string + host := pluginkit.NewHost( + &fakePlugin{id: "alpha", calls: &calls}, + &fakePlugin{id: "beta", calls: &calls}, + ) + + if err := host.Stop(t.Context()); err != nil { + t.Fatalf("Stop() error = %v, want nil", err) + } + + if want := []string{"beta stop", "alpha stop"}; !slices.Equal(want, calls) { + t.Errorf("stop calls = %v, want every plugin stopped in reverse order without a start", calls) + } +} diff --git a/pluginkit/wire/wire.go b/pluginkit/wire/wire.go index 3a1c3b8..5d54514 100644 --- a/pluginkit/wire/wire.go +++ b/pluginkit/wire/wire.go @@ -8,9 +8,11 @@ import ( "encoding/json" "errors" "fmt" + "go/token" "os" "path/filepath" "regexp" + "slices" "strings" ) @@ -26,6 +28,9 @@ type Config struct { GoRegistryPath string GoRegistryPackage string + + // Reserved lists ids no plugin may take. + Reserved []string } // roots returns the plugin root directories scanned in order, defaulting to plugins. @@ -93,8 +98,39 @@ func loadManifests(dir string) ([]manifest, error) { return manifests, nil } -// loadRoots loads the manifests under every plugin root in order, rejecting an id present in more than one root. -func loadRoots(dir string, roots []string) ([]manifest, error) { +// goOwned are the names an import alias of the generated Go wiring cannot take, beside the Go keywords. +var goOwned = map[string]bool{ + "errors": true, "fmt": true, "sdk": true, "deps": true, "plugins": true, "failed": true, "err": true, + "make": true, "append": true, "nil": true, "error": true, "init": true, "main": true, +} + +// tsOwned are the names an import alias of the generated TypeScript wiring cannot take. +var tsOwned = map[string]bool{ + "await": true, "break": true, "case": true, "catch": true, "class": true, "const": true, "continue": true, + "debugger": true, "default": true, "delete": true, "do": true, "else": true, "enum": true, "export": true, + "extends": true, "false": true, "finally": true, "for": true, "function": true, "if": true, "import": true, + "in": true, "instanceof": true, "new": true, "null": true, "return": true, "super": true, "switch": true, + "this": true, "throw": true, "true": true, "try": true, "typeof": true, "var": true, "void": true, + "while": true, "with": true, "yield": true, "implements": true, "interface": true, "let": true, + "package": true, "private": true, "protected": true, "public": true, "static": true, "eval": true, + "arguments": true, "plugins": true, +} + +// refuseReserved rejects an id the application reserves or an import alias of the generated wiring cannot take. +func refuseReserved(m manifest, reserved []string) error { + switch { + case slices.Contains(reserved, m.ID): + return fmt.Errorf("id %q is reserved", m.ID) + case m.Backend != "" && (token.IsKeyword(m.ID) || goOwned[m.ID]): + return fmt.Errorf("id %q collides with the generated Go wiring", m.ID) + case m.Frontend != "" && tsOwned[m.ID]: + return fmt.Errorf("id %q collides with the generated TypeScript wiring", m.ID) + } + return nil +} + +// loadRoots loads the manifests under every root in order, rejecting a reserved id or one present in two roots. +func loadRoots(dir string, roots, reserved []string) ([]manifest, error) { var manifests []manifest seen := make(map[string]string, len(roots)) for _, pluginRoot := range roots { @@ -106,6 +142,9 @@ func loadRoots(dir string, roots []string) ([]manifest, error) { if previous, ok := seen[m.ID]; ok { return nil, fmt.Errorf("pluginwire: plugin %s appears under %s and %s", m.ID, previous, pluginRoot) } + if err := refuseReserved(m, reserved); err != nil { + return nil, fmt.Errorf("pluginwire: %s: %w", filepath.Join(dir, pluginRoot, m.ID, "plugin.json"), err) + } seen[m.ID] = pluginRoot } manifests = append(manifests, loaded...) @@ -143,12 +182,14 @@ func generatedHeader(license string) string { // generateGo renders the generated Go plugin-wiring file. func generateGo(cfg Config, manifests []manifest) []byte { - return renderRegistration(cfg, manifests, "main", "", "registerPlugins") + doc := "// registerPlugins registers every compiled plugin, answering the ones that registered and an error naming " + + "each failure.\n" + return renderRegistration(cfg, manifests, "main", doc, "registerPlugins") } // generateRegistry renders the generated importable plugin registry file. func generateRegistry(cfg Config, manifests []manifest) []byte { - doc := "// All registers every plugin and returns them in registration order.\n" + doc := "// All registers every plugin, answering the ones that registered and an error naming each failure.\n" return renderRegistration(cfg, manifests, cfg.GoRegistryPackage, doc, "All") } @@ -164,13 +205,16 @@ func renderRegistration(cfg Config, manifests []manifest, pkg, doc, funcName str var b strings.Builder b.WriteString(generatedHeader(cfg.License)) fmt.Fprintf(&b, "package %s\n\nimport (\n", pkg) + if len(backends) > 0 { + b.WriteString("\t\"errors\"\n\t\"fmt\"\n\n") + } for _, m := range backends { fmt.Fprintf(&b, "\t%s %q\n", goName(m.ID), m.Backend) } if len(backends) > 0 { b.WriteString("\n") } - fmt.Fprintf(&b, "\t%q\n)\n\n", cfg.SDKImport) + fmt.Fprintf(&b, "\tsdk %q\n)\n\n", cfg.SDKImport) b.WriteString(doc) if len(backends) == 0 { fmt.Fprintf(&b, "func %s(_ sdk.Deps) ([]sdk.Plugin, error) {\n\treturn []sdk.Plugin{}, nil\n}\n", funcName) @@ -178,7 +222,7 @@ func renderRegistration(cfg Config, manifests []manifest, pkg, doc, funcName str } fmt.Fprintf( &b, - "func %s(deps sdk.Deps) ([]sdk.Plugin, error) {\n\tplugins := make([]sdk.Plugin, 0, %d)\n", + "func %s(deps sdk.Deps) ([]sdk.Plugin, error) {\n\tplugins := make([]sdk.Plugin, 0, %d)\n\tvar failed []error\n", funcName, len(backends), ) @@ -186,14 +230,16 @@ func renderRegistration(cfg Config, manifests []manifest, pkg, doc, funcName str name := goName(m.ID) fmt.Fprintf( &b, - "\t%sPlugin, err := %s.Register(deps)\n\tif err != nil {\n\t\treturn nil, err\n\t}\n"+ - "\tplugins = append(plugins, %sPlugin)\n", + "\t%sPlugin, err := %s.Register(deps)\n\tif err != nil {\n"+ + "\t\tfailed = append(failed, fmt.Errorf(\"plugin %s: %%w\", err))\n\t} else {\n"+ + "\t\tplugins = append(plugins, %sPlugin)\n\t}\n", name, name, + m.ID, name, ) } - b.WriteString("\treturn plugins, nil\n}\n") + b.WriteString("\treturn plugins, errors.Join(failed...)\n}\n") return []byte(b.String()) } @@ -225,7 +271,7 @@ func Run(root string, cfg Config) error { if err := validateConfig(cfg); err != nil { return err } - manifests, err := loadRoots(root, cfg.roots()) + manifests, err := loadRoots(root, cfg.roots(), cfg.Reserved) if err != nil { return err } diff --git a/pluginkit/wire/wire_test.go b/pluginkit/wire/wire_test.go index c17817c..d5beb27 100644 --- a/pluginkit/wire/wire_test.go +++ b/pluginkit/wire/wire_test.go @@ -130,25 +130,72 @@ func TestGenerateGoWiresBackendPlugins(t *testing.T) { package main import ( + "errors" + "fmt" + photo_gallery "example.com/gallery" feed "example.com/myapp/plugins/feed" - "example.com/myapp/sdk" + sdk "example.com/myapp/sdk" ) +// registerPlugins registers every compiled plugin, answering the ones that registered and an error naming each failure. func registerPlugins(deps sdk.Deps) ([]sdk.Plugin, error) { plugins := make([]sdk.Plugin, 0, 2) + var failed []error photo_galleryPlugin, err := photo_gallery.Register(deps) if err != nil { - return nil, err + failed = append(failed, fmt.Errorf("plugin photo-gallery: %w", err)) + } else { + plugins = append(plugins, photo_galleryPlugin) + } + feedPlugin, err := feed.Register(deps) + if err != nil { + failed = append(failed, fmt.Errorf("plugin feed: %w", err)) + } else { + plugins = append(plugins, feedPlugin) } - plugins = append(plugins, photo_galleryPlugin) + return plugins, errors.Join(failed...) +} +` + if got != want { + t.Errorf("generateGo() = %q, want %q", got, want) + } +} + +func TestGenerateGoWiresASingleBackendPlugin(t *testing.T) { + t.Parallel() + + got := string(generateGo(testConfig, []manifest{ + {ID: "feed", Name: "Feed", Backend: "example.com/myapp/plugins/feed"}, + })) + + want := `// SPDX-License-Identifier: Apache-2.0 + +// Code generated by pluginwire. DO NOT EDIT. + +package main + +import ( + "errors" + "fmt" + + feed "example.com/myapp/plugins/feed" + + sdk "example.com/myapp/sdk" +) + +// registerPlugins registers every compiled plugin, answering the ones that registered and an error naming each failure. +func registerPlugins(deps sdk.Deps) ([]sdk.Plugin, error) { + plugins := make([]sdk.Plugin, 0, 1) + var failed []error feedPlugin, err := feed.Register(deps) if err != nil { - return nil, err + failed = append(failed, fmt.Errorf("plugin feed: %w", err)) + } else { + plugins = append(plugins, feedPlugin) } - plugins = append(plugins, feedPlugin) - return plugins, nil + return plugins, errors.Join(failed...) } ` if got != want { @@ -170,9 +217,38 @@ func TestGenerateGoWithoutBackendPlugins(t *testing.T) { package main import ( - "example.com/myapp/sdk" + sdk "example.com/myapp/sdk" ) +// registerPlugins registers every compiled plugin, answering the ones that registered and an error naming each failure. +func registerPlugins(_ sdk.Deps) ([]sdk.Plugin, error) { + return []sdk.Plugin{}, nil +} +` + if got != want { + t.Errorf("generateGo() = %q, want %q", got, want) + } +} + +func TestGenerateGoAliasesTheSDKAsSdkWhateverItsPackageName(t *testing.T) { + t.Parallel() + + cfg := testConfig + cfg.SDKImport = "example.com/myapp/pluginapi" + + got := string(generateGo(cfg, nil)) + + want := `// SPDX-License-Identifier: Apache-2.0 + +// Code generated by pluginwire. DO NOT EDIT. + +package main + +import ( + sdk "example.com/myapp/pluginapi" +) + +// registerPlugins registers every compiled plugin, answering the ones that registered and an error naming each failure. func registerPlugins(_ sdk.Deps) ([]sdk.Plugin, error) { return []sdk.Plugin{}, nil } @@ -266,26 +342,32 @@ func TestGenerateRegistryRendersEveryBackend(t *testing.T) { package graphroot import ( + "errors" + "fmt" + photo_gallery "example.com/gallery" feed "example.com/myapp/plugins/feed" - "example.com/myapp/sdk" + sdk "example.com/myapp/sdk" ) -// All registers every plugin and returns them in registration order. +// All registers every plugin, answering the ones that registered and an error naming each failure. func All(deps sdk.Deps) ([]sdk.Plugin, error) { plugins := make([]sdk.Plugin, 0, 2) + var failed []error photo_galleryPlugin, err := photo_gallery.Register(deps) if err != nil { - return nil, err + failed = append(failed, fmt.Errorf("plugin photo-gallery: %w", err)) + } else { + plugins = append(plugins, photo_galleryPlugin) } - plugins = append(plugins, photo_galleryPlugin) feedPlugin, err := feed.Register(deps) if err != nil { - return nil, err + failed = append(failed, fmt.Errorf("plugin feed: %w", err)) + } else { + plugins = append(plugins, feedPlugin) } - plugins = append(plugins, feedPlugin) - return plugins, nil + return plugins, errors.Join(failed...) } ` if got != want { @@ -311,10 +393,10 @@ func TestGenerateRegistryWithoutBackendPlugins(t *testing.T) { package graphroot import ( - "example.com/myapp/sdk" + sdk "example.com/myapp/sdk" ) -// All registers every plugin and returns them in registration order. +// All registers every plugin, answering the ones that registered and an error naming each failure. func All(_ sdk.Deps) ([]sdk.Plugin, error) { return []sdk.Plugin{}, nil } @@ -540,6 +622,146 @@ func TestRunRejectsDuplicateIDAcrossRoots(t *testing.T) { } } +// earlierWiring is what the wiring files hold before a refused run. +const earlierWiring = "earlier wiring\n" + +// runBeside writes an alpha plugin, the manifest of id and earlier wiring files, then runs the generator. +func runBeside(t *testing.T, cfg Config, id, manifestJSON string) (string, error) { + t.Helper() + root := t.TempDir() + writePlugin(t, root, "alpha", + `{"id": "alpha", "name": "Alpha", "backend": "example.com/myapp/plugins/alpha", "frontend": "@myapp/plugin-alpha"}`) + writePlugin(t, root, id, manifestJSON) + for _, path := range []string{cfg.GoWiringPath, cfg.TSWiringPath} { + wiring := filepath.Join(root, filepath.FromSlash(path)) + if err := os.MkdirAll(filepath.Dir(wiring), 0o755); err != nil { + t.Fatalf("creating the wiring directory: %v", err) + } + if err := os.WriteFile(wiring, []byte(earlierWiring), 0o644); err != nil { + t.Fatalf("writing the earlier wiring: %v", err) + } + } + return root, Run(root, cfg) +} + +// assertRefused checks that err is the refusal of id under root and that no wiring file changed. +func assertRefused(t *testing.T, cfg Config, root, id, reason string, err error) { + t.Helper() + want := "pluginwire: " + filepath.Join(root, "plugins", id, "plugin.json") + `: id "` + id + `" ` + reason + if err == nil || err.Error() != want { + t.Errorf("Run() error = %v, want %q", err, want) + } + for _, path := range []string{cfg.GoWiringPath, cfg.TSWiringPath} { + if kept, _ := os.ReadFile(filepath.Join(root, filepath.FromSlash(path))); string(kept) != earlierWiring { + t.Errorf("%s after the refusal = %q, want the earlier wiring untouched", path, kept) + } + } +} + +func TestRunRefusesABackendIDTheGoWiringOwns(t *testing.T) { + t.Parallel() + + ids := []string{"errors", "fmt", "sdk", "deps", "plugins", "failed", "err", "make", "append", "nil", "error", + "init", "main", "type", "func"} + for _, id := range ids { + t.Run(id, func(t *testing.T) { + t.Parallel() + + root, err := runBeside(t, testConfig, id, + `{"id": "`+id+`", "name": "Colliding", "backend": "example.com/myapp/plugins/`+id+`"}`) + + assertRefused(t, testConfig, root, id, "collides with the generated Go wiring", err) + }) + } +} + +func TestRunRefusesAFrontendIDTheTypeScriptWiringOwns(t *testing.T) { + t.Parallel() + + ids := []string{"plugins", "class", "default", "let", "static", "await", "yield", "eval", "arguments"} + for _, id := range ids { + t.Run(id, func(t *testing.T) { + t.Parallel() + + root, err := runBeside(t, testConfig, id, + `{"id": "`+id+`", "name": "Colliding", "frontend": "@myapp/plugin-`+id+`"}`) + + assertRefused(t, testConfig, root, id, "collides with the generated TypeScript wiring", err) + }) + } +} + +func TestRunRefusesAnIDTheApplicationReserves(t *testing.T) { + t.Parallel() + + cfg := testConfig + cfg.Reserved = []string{"serve", "token"} + manifests := map[string]string{ + "token": `{"id": "token", "name": "Token", "backend": "example.com/myapp/plugins/token"}`, + "serve": `{"id": "serve", "name": "Serve", "frontend": "@myapp/plugin-serve"}`, + } + for id, manifestJSON := range manifests { + t.Run(id, func(t *testing.T) { + t.Parallel() + + root, err := runBeside(t, cfg, id, manifestJSON) + + assertRefused(t, cfg, root, id, "is reserved", err) + }) + } +} + +func TestRefuseReservedCoversEveryNameAnAliasCannotTake(t *testing.T) { + t.Parallel() + + goNames := []string{ + "break", "case", "chan", "const", "continue", "default", "defer", "else", "fallthrough", "for", "func", + "go", "goto", "if", "import", "interface", "map", "package", "range", "return", "select", "struct", + "switch", "type", "var", + "errors", "fmt", "sdk", "deps", "plugins", "failed", "err", "make", "append", "nil", "error", "init", "main", + } + tsNames := []string{ + "await", "break", "case", "catch", "class", "const", "continue", "debugger", "default", "delete", "do", + "else", "enum", "export", "extends", "false", "finally", "for", "function", "if", "import", "in", + "instanceof", "new", "null", "return", "super", "switch", "this", "throw", "true", "try", "typeof", "var", + "void", "while", "with", "yield", + "implements", "interface", "let", "package", "private", "protected", "public", "static", + "eval", "arguments", "plugins", + } + for _, id := range goNames { + want := `id "` + id + `" collides with the generated Go wiring` + if err := refuseReserved(manifest{ID: id, Backend: "example.com/myapp/plugins/" + id}, nil); err == nil || + err.Error() != want { + t.Errorf("refuseReserved(backend %s) = %v, want %q", id, err, want) + } + } + for _, id := range tsNames { + want := `id "` + id + `" collides with the generated TypeScript wiring` + if err := refuseReserved(manifest{ID: id, Frontend: "@myapp/plugin-" + id}, nil); err == nil || + err.Error() != want { + t.Errorf("refuseReserved(frontend %s) = %v, want %q", id, err, want) + } + } +} + +func TestRunAcceptsAnIDOnlyTheOtherWiringOwns(t *testing.T) { + t.Parallel() + + manifests := map[string]string{ + "err": `{"id": "err", "name": "Err", "frontend": "@myapp/plugin-err"}`, + "class": `{"id": "class", "name": "Class", "backend": "example.com/myapp/plugins/class"}`, + } + for id, manifestJSON := range manifests { + t.Run(id, func(t *testing.T) { + t.Parallel() + + if _, err := runBeside(t, testConfig, id, manifestJSON); err != nil { + t.Errorf("Run() error = %v, want nil", err) + } + }) + } +} + func TestRunRejectsIncompleteConfig(t *testing.T) { t.Parallel()