diff --git a/caddy/admin_test.go b/caddy/admin_test.go index b5fe37daae..e929c9ae69 100644 --- a/caddy/admin_test.go +++ b/caddy/admin_test.go @@ -6,6 +6,7 @@ import ( "fmt" "io" "net/http" + "strconv" "strings" "sync" "testing" @@ -409,3 +410,141 @@ func TestRegisteredModuleWorkerPoolsMustBeCorrect(t *testing.T) { assert.Contains(t, receivedThreadNames, "Worker PHP Thread - "+worker2Path, "expected module worker with \"match\" directive to be present") assert.Contains(t, receivedThreadNames, "Worker PHP Thread - "+worker3Path, "expected module worker without \"match\" directive to be present") } + +// Validate() must reject incorrect configs before Start() calls frankenphp.Shutdown() +func TestRejectedReloadKeepsThePreviousSiteServing(t *testing.T) { + tester := caddytest.NewTester(t) + initServer(t, tester, ` + { + skip_install_trust + admin localhost:2999 + http_port `+testPort+` + + frankenphp { + worker ../testdata/worker-with-counter.php 1 + } + } + + localhost:`+testPort+` { + route { + root ../testdata + rewrite worker-with-counter.php + php + } + } + `, "caddyfile") + + workerURL := "http://localhost:" + testPort + "/worker-with-counter.php" + servedBefore := countedRequests(t, workerURL) + + // the worker file does not exist, which Init() only reports once the + // running configuration is gone + rejected := ` + { + skip_install_trust + admin localhost:2999 + http_port ` + testPort + ` + + frankenphp { + worker ../testdata/not-a-worker.php 1 + } + } + + localhost:` + testPort + ` { + route { + root ../testdata + rewrite worker-with-counter.php + php + } + } + ` + + r, err := http.NewRequest("POST", "http://localhost:2999/load", bytes.NewBufferString(rejected)) + require.NoError(t, err) + r.Header.Set("Content-Type", "text/caddyfile") + resp, err := http.DefaultClient.Do(r) + require.NoError(t, err) + body, err := io.ReadAll(resp.Body) + require.NoError(t, err) + require.NoError(t, resp.Body.Close()) + // /load writes the adaptation warnings before loading, so the status is + // already 200 when the error follows them in the body + require.Contains(t, string(body), "invalid configuration: worker filename is invalid") + + // the runtime that served before the rejected reload still serves, and + // it is the same one: its worker kept counting + require.Equal(t, servedBefore+1, countedRequests(t, workerURL)) +} + +// Validate() cannot see a worker declared in a php_server block: Caddy calls it +// before the modules provision, so Start() must reject those before shutting down +func TestRejectedReloadWithAModuleWorkerKeepsThePreviousSiteServing(t *testing.T) { + tester := caddytest.NewTester(t) + initServer(t, tester, ` + { + skip_install_trust + admin localhost:2999 + http_port `+testPort+` + } + + localhost:`+testPort+` { + route { + root ../testdata + rewrite worker-with-counter.php + php { + worker ../testdata/worker-with-counter.php 1 + } + } + } + `, "caddyfile") + + workerURL := "http://localhost:" + testPort + "/worker-with-counter.php" + servedBefore := countedRequests(t, workerURL) + + rejected := ` + { + skip_install_trust + admin localhost:2999 + http_port ` + testPort + ` + } + + localhost:` + testPort + ` { + route { + root ../testdata + rewrite worker-with-counter.php + php { + worker ../testdata/not-a-worker.php 1 + } + } + } + ` + + r, err := http.NewRequest("POST", "http://localhost:2999/load", bytes.NewBufferString(rejected)) + require.NoError(t, err) + r.Header.Set("Content-Type", "text/caddyfile") + resp, err := http.DefaultClient.Do(r) + require.NoError(t, err) + body, err := io.ReadAll(resp.Body) + require.NoError(t, err) + require.NoError(t, resp.Body.Close()) + require.Contains(t, string(body), "worker filename is invalid") + require.Equal(t, servedBefore+1, countedRequests(t, workerURL)) +} + +// the number of requests testdata/worker-with-counter.php has served +func countedRequests(t *testing.T, workerURL string) int { + t.Helper() + + resp, err := http.Get(workerURL) + require.NoError(t, err) + defer func() { require.NoError(t, resp.Body.Close()) }() + require.Equal(t, http.StatusOK, resp.StatusCode) + + body, err := io.ReadAll(resp.Body) + require.NoError(t, err) + + count, err := strconv.Atoi(strings.TrimPrefix(string(body), "requests:")) + require.NoError(t, err, "unexpected worker response %q", body) + + return count +} diff --git a/caddy/app.go b/caddy/app.go index fcee129180..74ea6961dd 100644 --- a/caddy/app.go +++ b/caddy/app.go @@ -60,15 +60,15 @@ type FrankenPHPApp struct { // EXPERIMENTAL: MaxRequests sets the maximum number of requests a PHP thread handles before restarting (0 = unlimited) MaxRequests int `json:"max_requests,omitempty"` - opts []frankenphp.Option - metrics frankenphp.Metrics - ctx context.Context - logger *slog.Logger - modules []*FrankenPHPModule - usedWorkerNames map[string]bool - httpApp *caddyhttp.App - hasStarted atomic.Bool - started chan any + opts []frankenphp.Option + provisionOpts []frankenphp.Option + metrics frankenphp.Metrics + ctx context.Context + logger *slog.Logger + modules []*FrankenPHPModule + httpApp *caddyhttp.App + hasStarted atomic.Bool + started chan any } var errIni = errors.New(`"php_ini" must be in the format: php_ini "" ""`) @@ -107,18 +107,29 @@ func (f *FrankenPHPApp) Provision(ctx caddy.Context) error { return nil } -func (f *FrankenPHPApp) Start() error { - defer func() { - close(f.started) - }() +// Validate is called by Caddy before Start() to validate before shutting down the currently running config +func (f *FrankenPHPApp) Validate() error { + opts, err := f.collectOptions(caddy.NewReplacer(), false) + if err != nil { + return err + } - repl := caddy.NewReplacer() + return frankenphp.Validate(opts...) +} +// collectOptions turns the configuration into the options Init() takes. +// keep is for the configuration that starts, whose servers the modules +// serve from; Validate() collects those of one that may never start. +func (f *FrankenPHPApp) collectOptions(repl *caddy.Replacer, keep bool) ([]frankenphp.Option, error) { optionsMU.RLock() - f.opts = append(f.opts, options...) + // We have at least 9 hardcoded options + opts := make([]frankenphp.Option, 0, 9+len(options)+len(f.provisionOpts)) + opts = append(opts, options...) optionsMU.RUnlock() - f.opts = append(f.opts, + opts = append(opts, f.provisionOpts...) + + opts = append(opts, frankenphp.WithContext(f.ctx), frankenphp.WithLogger(f.logger), frankenphp.WithNumThreads(f.NumThreads), @@ -130,18 +141,42 @@ func (f *FrankenPHPApp) Start() error { frankenphp.WithMaxRequests(f.MaxRequests), ) + usedWorkerNames := make(map[string]bool, len(f.Workers)) + // register global workers for _, w := range f.Workers { w.FileName = repl.ReplaceKnown(w.FileName, "") - w.Name = f.createUniqueWorkerName(w, "") - opts, err := w.toWorkerOptions() + w.Name = createUniqueWorkerName(usedWorkerNames, w, "") + workerOptions, err := w.toWorkerOptions() if err != nil { - return err + return nil, err } - f.opts = append(f.opts, frankenphp.WithWorkers(w.Name, w.FileName, w.Num, opts...)) + opts = append(opts, frankenphp.WithWorkers(w.Name, w.FileName, w.Num, workerOptions...)) + } + + moduleOpts, err := f.collectModuleOptions(repl, usedWorkerNames, keep) + if err != nil { + return nil, err + } + + return append(opts, moduleOpts...), nil +} + +func (f *FrankenPHPApp) Start() error { + defer func() { + close(f.started) + }() + + opts, err := f.collectOptions(caddy.NewReplacer(), true) + if err != nil { + return err } + f.opts = opts - if err := f.registerModules(repl); err != nil { + // Validate() ran before the modules provisioned, so it saw the global + // block alone: what a php_server declares is checked here, in time for + // the configuration in place to survive a refusal + if err := frankenphp.Validate(f.opts...); err != nil { return err } @@ -178,35 +213,42 @@ func (f *FrankenPHPApp) Stop() error { } // register workers and servers for "php" and "php_server" modules -func (f *FrankenPHPApp) registerModules(repl *caddy.Replacer) error { - modulesByIndex := make(map[int]*FrankenPHPModule, len(f.modules)) - for _, module := range f.modules { - if module.ServerIndex == 0 { - if err := f.registerModule(repl, module); err != nil { - return err - } - continue - } +func (f *FrankenPHPApp) collectModuleOptions(repl *caddy.Replacer, usedWorkerNames map[string]bool, keep bool) ([]frankenphp.Option, error) { + opts := make([]frankenphp.Option, 0, len(f.modules)) + serversByIndex := make(map[int]*frankenphp.Server, len(f.modules)) + for _, module := range f.modules { // modules with the same server_idx should share the same server instance // example: the worker { match * } rule adds 2 "php" subroutes to the caddy handler // the 2 handlers belong to the same "php_server" and must therefore share workers - if existingModule, ok := modulesByIndex[module.ServerIndex]; ok { - module.server = existingModule.server - continue + if module.ServerIndex != 0 { + if server, ok := serversByIndex[module.ServerIndex]; ok { + if keep { + module.server = server + } + + continue + } + } + + server, moduleOpts, err := f.collectModule(repl, module, usedWorkerNames) + if err != nil { + return nil, err } - modulesByIndex[module.ServerIndex] = module - if err := f.registerModule(repl, module); err != nil { - return err + if keep { + module.server = server + } + if module.ServerIndex != 0 { + serversByIndex[module.ServerIndex] = server } + opts = append(opts, moduleOpts...) } - return nil + return opts, nil } -// register a server instance and its workers for a single Caddy module -func (f *FrankenPHPApp) registerModule(repl *caddy.Replacer, module *FrankenPHPModule) error { +func (f *FrankenPHPApp) collectModule(repl *caddy.Replacer, module *FrankenPHPModule, usedWorkerNames map[string]bool) (*frankenphp.Server, []frankenphp.Option, error) { serverName := f.resolveServerName(module) server, err := frankenphp.NewServer( module.resolvedDocumentRoot, @@ -216,34 +258,29 @@ func (f *FrankenPHPApp) registerModule(repl *caddy.Replacer, module *FrankenPHPM frankenphp.WithServerLogger(module.logger), ) if err != nil { - return err + return nil, nil, err } - module.server = server - f.opts = append(f.opts, frankenphp.WithServer(server)) + opts := []frankenphp.Option{frankenphp.WithServer(server)} for _, w := range module.Workers { w.FileName = repl.ReplaceKnown(w.FileName, "") - w.Name = f.createUniqueWorkerName(w, serverName) + w.Name = createUniqueWorkerName(usedWorkerNames, w, serverName) workerOptions, err := w.toWorkerOptions() if err != nil { - return err + return nil, nil, err } workerOptions = append(workerOptions, frankenphp.WithWorkerServerScope(server)) - f.opts = append(f.opts, frankenphp.WithWorkers(w.Name, w.FileName, w.Num, workerOptions...)) + opts = append(opts, frankenphp.WithWorkers(w.Name, w.FileName, w.Num, workerOptions...)) } - return nil + return server, opts, nil } // avoid name collisions for workers // on collision, a name is first qualified with the server name // (":") before falling back to a numeric postfix -func (f *FrankenPHPApp) createUniqueWorkerName(wc workerConfig, serverName string) string { - if f.usedWorkerNames == nil { - f.usedWorkerNames = make(map[string]bool) - } - +func createUniqueWorkerName(usedWorkerNames map[string]bool, wc workerConfig, serverName string) string { if wc.Name == "" { wc.Name, _ = fastabs.FastAbs(wc.FileName) } @@ -251,8 +288,8 @@ func (f *FrankenPHPApp) createUniqueWorkerName(wc workerConfig, serverName strin name := wc.Name suffix := 0 for { - if _, ok := f.usedWorkerNames[name]; !ok { - f.usedWorkerNames[name] = true + if _, ok := usedWorkerNames[name]; !ok { + usedWorkerNames[name] = true break } if serverName != "" { diff --git a/caddy/config_test.go b/caddy/config_test.go index 607051cbd8..0598b42530 100644 --- a/caddy/config_test.go +++ b/caddy/config_test.go @@ -250,16 +250,16 @@ func TestModuleWorkerWithCustomName(t *testing.T) { } func TestCreateUniqueWorkerNames(t *testing.T) { - app := &FrankenPHPApp{} + usedWorkerNames := map[string]bool{} filename := "../testdata/worker-with-env.php" absFileName, _ := filepath.Abs(filename) names := make([]string, 6) for i := range 3 { - names[i] = app.createUniqueWorkerName(workerConfig{ + names[i] = createUniqueWorkerName(usedWorkerNames, workerConfig{ FileName: filename, Name: "custom-worker-name", }, "") - names[i+3] = app.createUniqueWorkerName(workerConfig{ + names[i+3] = createUniqueWorkerName(usedWorkerNames, workerConfig{ FileName: filename, }, "") } @@ -273,14 +273,14 @@ func TestCreateUniqueWorkerNames(t *testing.T) { } func TestCreateUniqueWorkerNamesQualifiedByServer(t *testing.T) { - app := &FrankenPHPApp{} + usedWorkerNames := map[string]bool{} wc := workerConfig{FileName: "../testdata/worker-with-env.php", Name: "queue"} - require.Equal(t, "queue", app.createUniqueWorkerName(wc, "one.example.com")) + require.Equal(t, "queue", createUniqueWorkerName(usedWorkerNames, wc, "one.example.com")) // on collision, the name is qualified with the server name - require.Equal(t, "two.example.com:queue", app.createUniqueWorkerName(wc, "two.example.com")) + require.Equal(t, "two.example.com:queue", createUniqueWorkerName(usedWorkerNames, wc, "two.example.com")) // when the qualified name is also taken, fall back to the numeric postfix - require.Equal(t, "queue_1", app.createUniqueWorkerName(wc, "two.example.com")) + require.Equal(t, "queue_1", createUniqueWorkerName(usedWorkerNames, wc, "two.example.com")) // workers without a server keep the numeric postfix behavior - require.Equal(t, "queue_2", app.createUniqueWorkerName(wc, "")) + require.Equal(t, "queue_2", createUniqueWorkerName(usedWorkerNames, wc, "")) } diff --git a/caddy/hotreload.go b/caddy/hotreload.go index bec8b16fbd..e882c8f147 100644 --- a/caddy/hotreload.go +++ b/caddy/hotreload.go @@ -48,7 +48,7 @@ func (f *FrankenPHPModule) configureHotReload(app *FrankenPHPApp) error { f.HotReload.Topic = "https://frankenphp.dev/hot-reload/" + uid } - app.opts = append(app.opts, frankenphp.WithHotReload(f.HotReload.Topic, f.mercureHub, f.HotReload.Watch)) + app.provisionOpts = append(app.provisionOpts, frankenphp.WithHotReload(f.HotReload.Topic, f.mercureHub, f.HotReload.Watch)) // add the hot reload to the env variables if f.Env == nil { diff --git a/caddy/serveridx_test.go b/caddy/serveridx_test.go index 7414c07169..1fd401429d 100644 --- a/caddy/serveridx_test.go +++ b/caddy/serveridx_test.go @@ -26,7 +26,8 @@ func TestRegisterModulesWithSameServerIndexShareOneServer(t *testing.T) { shared2 := &FrankenPHPModule{ServerIndex: 1, resolvedDocumentRoot: "../testdata"} app.modules = []*FrankenPHPModule{shared1, shared2} - require.NoError(t, app.registerModules(caddy.NewReplacer())) + _, err := app.collectModuleOptions(caddy.NewReplacer(), map[string]bool{}, true) + require.NoError(t, err) require.NotNil(t, shared1.server) require.Same(t, shared1.server, shared2.server, "modules with the same server_idx must share one server instance") @@ -39,7 +40,8 @@ func TestRegisterModulesWithoutServerIndexGetOwnServers(t *testing.T) { indexed := &FrankenPHPModule{ServerIndex: 1, resolvedDocumentRoot: "../testdata"} app.modules = []*FrankenPHPModule{auto1, indexed, auto2} - require.NoError(t, app.registerModules(caddy.NewReplacer())) + _, err := app.collectModuleOptions(caddy.NewReplacer(), map[string]bool{}, true) + require.NoError(t, err) require.NotNil(t, auto1.server) require.NotNil(t, auto2.server) @@ -59,8 +61,9 @@ func TestRegisterModulesFirstModuleWinsPerIdx(t *testing.T) { second := &FrankenPHPModule{ServerIndex: 2, resolvedDocumentRoot: "../testdata/env"} app.modules = []*FrankenPHPModule{first, second} - require.NoError(t, app.registerModules(caddy.NewReplacer())) + opts, err := app.collectModuleOptions(caddy.NewReplacer(), map[string]bool{}, true) + require.NoError(t, err) require.Same(t, first.server, second.server) - require.Len(t, app.opts, 1, "only one server must be registered for a shared index") + require.Len(t, opts, 1, "only one server must be registered for a shared index") } diff --git a/docs/library.md b/docs/library.md index 7782f03182..968d2bd971 100644 --- a/docs/library.md +++ b/docs/library.md @@ -17,7 +17,7 @@ For a minimal example see [https://pkg.go.dev](https://pkg.go.dev/github.com/dun `NewServer()` takes a human-readable name used to attribute workers, metrics and logs to the server (defaults to `server_` at registration when empty), the document root, the split path suffixes (defaults to `[".php"]`), environment variables made available to every request, and a `*slog.Logger` (defaults to the global logger). -`Init()` starts the PHP runtime and must be called exactly once before serving requests; `Shutdown()` stops it. Calling `Server.ServeHTTP()` before `Init()` or after `Shutdown()` returns `ErrNotRunning`. The same `*Server` may be passed to `Init()` again after a `Shutdown()`, for instance to reload the configuration. +`Init()` starts the PHP runtime and must be called exactly once before serving requests; `Shutdown()` stops it. `Validate()` takes the same options and reports whether `Init()` would accept them, without starting anything: a host replacing a running configuration should call it before stopping the one in place, since `Init()` only reports a declaration error once the previous runtime is gone. Calling `Server.ServeHTTP()` before `Init()` or after `Shutdown()` returns `ErrNotRunning`. The same `*Server` may be passed to `Init()` again after a `Shutdown()`, for instance to reload the configuration. ## Multiple servers diff --git a/frankenphp.go b/frankenphp.go index d8a8c2cf61..a0a13647e5 100644 --- a/frankenphp.go +++ b/frankenphp.go @@ -225,8 +225,6 @@ func calculateMaxThreads(opt *opt) (numWorkers int, _ error) { // https://github.com/php/frankenphp/issues/126 opt.workers[i].num = maxProcs } - metrics.TotalWorkers(w.name, w.num) - numWorkers += opt.workers[i].num if w.maxThreads > 0 { @@ -298,6 +296,49 @@ func calculateMaxThreads(opt *opt) (numWorkers int, _ error) { return numWorkers, nil } +// Validate reports whether Init() would accept a configuration, without +// starting anything: the thread budget, the worker files, and the names and +// scopes workers may take. A host replacing a running configuration should +// call it before stopping the one in place, since Init() only reports these +// errors once the previous runtime is gone. +func Validate(options ...Option) error { + opt := &opt{} + for _, o := range options { + if err := o(opt); err != nil { + return err + } + } + + if _, err := calculateMaxThreads(opt); err != nil { + return err + } + + takenNames := make(map[string]bool, len(opt.workers)) + takenPaths := make(map[*Server]map[string]bool, 1) + nameTaken := func(name string) bool { return takenNames[name] } + pathTaken := func(server *Server, path string) bool { return takenPaths[server][path] } + for _, w := range opt.workers { + w, err := resolveWorkerFile(w) + if err != nil { + return err + } + + if err := checkWorkerDeclaration(w, nameTaken, pathTaken); err != nil { + return err + } + + takenNames[w.name] = true + if w.matchRequest == nil { + if takenPaths[w.server] == nil { + takenPaths[w.server] = make(map[string]bool) + } + takenPaths[w.server][w.fileName] = true + } + } + + return nil +} + // Init starts the PHP runtime and the configured workers. func Init(options ...Option) error { if !isRunning.CompareAndSwap(false, true) { @@ -351,6 +392,9 @@ func Init(options ...Option) error { } metrics.TotalThreads(opt.numThreads) + for _, w := range opt.workers { + metrics.TotalWorkers(w.name, w.num) + } config := Config() diff --git a/frankenphp_test.go b/frankenphp_test.go index 6e1ae40cbd..520c9c1066 100644 --- a/frankenphp_test.go +++ b/frankenphp_test.go @@ -1504,3 +1504,46 @@ func testOpcachePreload(t *testing.T, opts *testOptions) { assert.Equal(t, "I am preloaded", body) }, opts) } + +// Validate reports what Init() would refuse, without starting anything +func TestValidateReportsDeclarationErrors(t *testing.T) { + assert.NoError(t, frankenphp.Validate( + frankenphp.WithNumThreads(2), + frankenphp.WithWorkers("worker", "testdata/worker.php", 1), + )) + + assert.ErrorContains(t, frankenphp.Validate( + frankenphp.WithWorkers("worker", "testdata/not-a-worker.php", 1), + ), "worker filename is invalid") + + assert.ErrorContains(t, frankenphp.Validate( + frankenphp.WithWorkers("same", "testdata/worker.php", 1), + frankenphp.WithWorkers("same", "testdata/index.php", 1), + ), "two workers cannot have the same name") + + assert.ErrorContains(t, frankenphp.Validate( + frankenphp.WithWorkers("one", "testdata/worker.php", 1), + frankenphp.WithWorkers("two", "testdata/worker.php", 1), + ), "two global workers cannot have the same filename") + + server, err := frankenphp.NewServer(testDataDir) + require.NoError(t, err) + matchAll := frankenphp.WithWorkerMatcher(func(*http.Request) bool { return true }) + + assert.ErrorContains(t, frankenphp.Validate( + frankenphp.WithServer(server), + frankenphp.WithWorkers("one", "testdata/worker.php", 1, frankenphp.WithWorkerServerScope(server)), + frankenphp.WithWorkers("two", "testdata/worker.php", 1, frankenphp.WithWorkerServerScope(server)), + ), "two workers in a server cannot have the same filename") + + assert.NoError(t, frankenphp.Validate( + frankenphp.WithServer(server), + frankenphp.WithWorkers("one", "testdata/worker.php", 1, frankenphp.WithWorkerServerScope(server)), + frankenphp.WithWorkers("two", "testdata/worker.php", 1, frankenphp.WithWorkerServerScope(server), matchAll), + )) + + assert.ErrorContains(t, frankenphp.Validate( + frankenphp.WithNumThreads(2), + frankenphp.WithMaxThreads(1), + ), "max_threads") +} diff --git a/worker.go b/worker.go index 388dfbd031..4a596c2fb6 100644 --- a/worker.go +++ b/worker.go @@ -101,43 +101,81 @@ func initWorkers(opts []workerOpt) error { return nil } -func newWorker(o workerOpt) (*worker, error) { - // Order is important! - // This order ensures that FrankenPHP started from inside a symlinked directory will properly resolve any paths. - // If it is started from outside a symlinked directory, it is resolved to the same path that we use in the Caddy module. +// resolveWorkerFile turns a declared filename into the path a worker runs, +// and names the worker after it when the declaration left the name out. +// +// Order is important! +// This order ensures that FrankenPHP started from inside a symlinked directory will properly resolve any paths. +// If it is started from outside a symlinked directory, it is resolved to the same path that we use in the Caddy module. +func resolveWorkerFile(o workerOpt) (workerOpt, error) { absFileName, err := filepath.EvalSymlinks(filepath.FromSlash(o.fileName)) if err != nil { - return nil, fmt.Errorf("worker filename is invalid %q: %w", o.fileName, err) + return o, fmt.Errorf("worker filename is invalid %q: %w", o.fileName, err) } absFileName, err = fastabs.FastAbs(absFileName) if err != nil { - return nil, fmt.Errorf("worker filename is invalid %q: %w", o.fileName, err) + return o, fmt.Errorf("worker filename is invalid %q: %w", o.fileName, err) } if _, err := os.Stat(absFileName); err != nil { - return nil, fmt.Errorf("worker file not found %q: %w", absFileName, err) + return o, fmt.Errorf("worker file not found %q: %w", absFileName, err) } + o.fileName = absFileName if o.name == "" { o.name = absFileName } + return o, nil +} + +// checkWorkerDeclaration holds the rules a set of workers must follow, so +// Validate() and newWorker() answer the same way; pathTaken is asked for the +// global workers with a nil server. +func checkWorkerDeclaration(o workerOpt, nameTaken func(string) bool, pathTaken func(*Server, string) bool) error { if o.server == nil { - if globalWorkersByPath[absFileName] != nil { - return nil, fmt.Errorf("two global workers cannot have the same filename: %q", absFileName) + if pathTaken(nil, o.fileName) { + return fmt.Errorf("two global workers cannot have the same filename: %q", o.fileName) } // no server means no set of requests to match against, the matcher would never run if o.matchRequest != nil { - return nil, fmt.Errorf("worker %q has a request matcher but no server scope, use WithWorkerServerScope()", o.name) + return fmt.Errorf("worker %q has a request matcher but no server scope, use WithWorkerServerScope()", o.name) } + } else if o.matchRequest == nil && pathTaken(o.server, o.fileName) { + // a matcher tells the workers of a server sharing a file apart + return fmt.Errorf("two workers in a server cannot have the same filename: %q", o.fileName) } - if workersByName[o.name] != nil { - return nil, fmt.Errorf("two workers cannot have the same name: %q", o.name) + if nameTaken(o.name) { + return fmt.Errorf("two workers cannot have the same name: %q", o.name) } + return nil +} + +func newWorker(o workerOpt) (*worker, error) { + o, err := resolveWorkerFile(o) + if err != nil { + return nil, err + } + + nameTaken := func(name string) bool { return workersByName[name] != nil } + pathTaken := func(server *Server, path string) bool { + if server == nil { + return globalWorkersByPath[path] != nil + } + + return server.workersByPath[path] != nil + } + + if err := checkWorkerDeclaration(o, nameTaken, pathTaken); err != nil { + return nil, err + } + + absFileName := o.fileName + // env should always contain FRANKENPHP_WORKER and the parent php_server env if o.env == nil { o.env = make(PreparedEnv, 1)