make frankenphp directive optional, thanks @francislavoie

caddy/caddy.go

@@ -33,10 +33,6 @@ const (
 
 var iniError = errors.New("'php_ini' must be in the format: php_ini \"<key>\" \"<value>\"")
 
-// FrankenPHPModule instances register their workers, and FrankenPHPApp reads them at Start() time.
-// FrankenPHPApp.Workers may be set by JSON config, so keep them separate.
-var moduleWorkerConfigs []workerConfig
-
 func init() {
 	caddy.RegisterModule(FrankenPHPApp{})
 	caddy.RegisterModule(FrankenPHPModule{})
@@ -104,6 +100,48 @@ func (f *FrankenPHPApp) Provision(ctx caddy.Context) error {
 	return nil
 }
 
+func (f *FrankenPHPApp) generateUniqueModuleWorkerName(filepath string) string {
+	var i uint
+	filepath, _ = fastabs.FastAbs(filepath)
+	name := "m#" + filepath
+
+outer:
+	for {
+		for _, wc := range f.Workers {
+			if wc.Name == name {
+				name = fmt.Sprintf("m#%s_%d", filepath, i)
+				i++
+
+				continue outer
+			}
+		}
+
+		return name
+	}
+}
+
+func (f *FrankenPHPApp) AddModuleWorkers(workers ...workerConfig) ([]workerConfig, error) {
+	for i := range workers {
+		w := &workers[i]
+		if frankenphp.EmbeddedAppPath != "" && filepath.IsLocal(w.FileName) {
+			w.FileName = filepath.Join(frankenphp.EmbeddedAppPath, w.FileName)
+		}
+		if w.Name == "" {
+			w.Name = f.generateUniqueModuleWorkerName(w.FileName)
+		}
+		if !strings.HasPrefix(w.Name, "m#") {
+			w.Name = "m#" + w.Name
+		}
+		for _, existingWorker := range f.Workers {
+			if existingWorker.Name == w.Name {
+				return nil, fmt.Errorf("two workers cannot have the same name: %q", w.Name)
+			}
+		}
+		f.Workers = append(f.Workers, *w)
+	}
+	return workers, nil
+}
+
 func (f *FrankenPHPApp) Start() error {
 	repl := caddy.NewReplacer()
 
@@ -115,9 +153,7 @@ func (f *FrankenPHPApp) Start() error {
 		frankenphp.WithPhpIni(f.PhpIni),
 		frankenphp.WithMaxWaitTime(f.MaxWaitTime),
 	}
-	// Add workers from FrankenPHPApp and FrankenPHPModule configurations
-	// f.Workers may have been set by JSON config, so keep them separate
-	for _, w := range append(f.Workers, moduleWorkerConfigs...) {
+	for _, w := range append(f.Workers) {
 		opts = append(opts, frankenphp.WithWorkers(w.Name, repl.ReplaceKnown(w.FileName, ""), w.Num, w.Env, w.Watch))
 	}
 
@@ -143,7 +179,6 @@ func (f *FrankenPHPApp) Stop() error {
 	f.Workers = nil
 	f.NumThreads = 0
 	f.MaxWaitTime = 0
-	moduleWorkerConfigs = nil
 
 	return nil
 }
@@ -391,6 +426,20 @@ func (FrankenPHPModule) CaddyModule() caddy.ModuleInfo {
 // Provision sets up the module.
 func (f *FrankenPHPModule) Provision(ctx caddy.Context) error {
 	f.logger = ctx.Slogger()
+	app, err := ctx.App("frankenphp")
+	if err != nil {
+		return err
+	}
+	fapp, ok := app.(*FrankenPHPApp)
+	if !ok {
+		return fmt.Errorf("expected ctx.App(\"frankenphp\") to return FrankenPHPApp, got %T", app)
+	}
+
+	workers, err := fapp.AddModuleWorkers(f.Workers...)
+	if err != nil {
+		return err
+	}
+	f.Workers = workers
 
 	if f.Root == "" {
 		if frankenphp.EmbeddedAppPath == "" {
@@ -504,25 +553,6 @@ func (f *FrankenPHPModule) ServeHTTP(w http.ResponseWriter, r *http.Request, _ c
 	return nil
 }
 
-func generateUniqueModuleWorkerName(filepath string) string {
-	var i uint
-	name := "m#" + filepath
-
-outer:
-	for {
-		for _, wc := range moduleWorkerConfigs {
-			if wc.Name == name {
-				name = fmt.Sprintf("m#%s_%d", filepath, i)
-				i++
-
-				continue outer
-			}
-		}
-
-		return name
-	}
-}
-
 // UnmarshalCaddyfile implements caddyfile.Unmarshaler.
 func (f *FrankenPHPModule) UnmarshalCaddyfile(d *caddyfile.Dispenser) error {
 	// First pass: Parse all directives except "worker"
@@ -608,28 +638,14 @@ func (f *FrankenPHPModule) UnmarshalCaddyfile(d *caddyfile.Dispenser) error {
 					}
 				}
 
-				if wc.Name == "" {
-					wc.Name = generateUniqueModuleWorkerName(wc.FileName)
-				}
-				if !strings.HasPrefix(wc.Name, "m#") {
-					wc.Name = "m#" + wc.Name
-				}
-
 				// Check if a worker with this filename already exists in this module
 				for _, existingWorker := range f.Workers {
 					if existingWorker.FileName == wc.FileName {
 						return fmt.Errorf(`workers in a single "php_server" block must not have duplicate filenames: %q`, wc.FileName)
 					}
 				}
-				// Check if a worker with this name and a different environment or filename already exists
-				for _, existingWorker := range moduleWorkerConfigs {
-					if existingWorker.Name == wc.Name {
-						return fmt.Errorf("workers must not have duplicate names: %q", wc.Name)
-					}
-				}
 
 				f.Workers = append(f.Workers, wc)
-				moduleWorkerConfigs = append(moduleWorkerConfigs, wc)
 			}
 		}
 	}

caddy/config_test.go

@@ -7,11 +7,6 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-// resetModuleWorkers resets the moduleWorkerConfigs slice for testing
-func resetModuleWorkers() {
-	moduleWorkerConfigs = make([]workerConfig, 0)
-}
-
 func TestModuleWorkerDuplicateFilenamesFail(t *testing.T) {
 	// Create a test configuration with duplicate worker filenames
 	configWithDuplicateFilenames := `
@@ -38,7 +33,6 @@ func TestModuleWorkerDuplicateFilenamesFail(t *testing.T) {
 	// Verify that an error was returned
 	require.Error(t, err, "Expected an error when two workers in the same module have the same filename")
 	require.Contains(t, err.Error(), "must not have duplicate filenames", "Error message should mention duplicate filenames")
-	resetModuleWorkers()
 }
 
 func TestModuleWorkersDuplicateNameFail(t *testing.T) {
@@ -80,11 +74,17 @@ func TestModuleWorkersDuplicateNameFail(t *testing.T) {
 
 	// Unmarshal the second configuration
 	err = module2.UnmarshalCaddyfile(d2)
+	require.NoError(t, err, "Second module should be configured without errors")
+
+	// Create a FrankenPHPApp and add the workers from the first module
+	app := &FrankenPHPApp{}
+	_, err = app.AddModuleWorkers(module1.Workers...)
+	require.NoError(t, err, "First module workers should be added without errors")
+	_, err = app.AddModuleWorkers(module2.Workers...)
 
 	// Verify that an error was returned
-	require.Error(t, err, "Expected an error when two workers have the same name, but different environments")
-	require.Contains(t, err.Error(), "must not have duplicate names", "Error message should mention duplicate names")
-	resetModuleWorkers()
+	require.Error(t, err, "Expected an error when two workers have the same name")
+	require.Contains(t, err.Error(), "same name", "Error message should mention duplicate names")
 }
 
 func TestModuleWorkersWithDifferentFilenames(t *testing.T) {
@@ -111,8 +111,6 @@ func TestModuleWorkersWithDifferentFilenames(t *testing.T) {
 	require.Len(t, module.Workers, 2, "Expected two workers to be added to the module")
 	require.Equal(t, "../testdata/worker-with-env.php", module.Workers[0].FileName, "First worker should have the correct filename")
 	require.Equal(t, "../testdata/worker-with-counter.php", module.Workers[1].FileName, "Second worker should have the correct filename")
-
-	resetModuleWorkers()
 }
 
 func TestModuleWorkersDifferentNamesSucceed(t *testing.T) {
@@ -130,6 +128,7 @@ func TestModuleWorkersDifferentNamesSucceed(t *testing.T) {
 
 	// Parse the first configuration
 	d1 := caddyfile.NewTestDispenser(configWithWorkerName1)
+	app := &FrankenPHPApp{}
 	module1 := &FrankenPHPModule{}
 
 	// Unmarshal the first configuration
@@ -158,12 +157,15 @@ func TestModuleWorkersDifferentNamesSucceed(t *testing.T) {
 	// Verify that no error was returned
 	require.NoError(t, err, "Expected no error when two workers have different names")
 
-	// Verify that both workers were added to moduleWorkerConfigs
-	require.Len(t, moduleWorkerConfigs, 2, "Expected two workers to be added to moduleWorkerConfigs")
-	require.Equal(t, "m#test-worker-1", moduleWorkerConfigs[0].Name, "First worker should have the correct name")
-	require.Equal(t, "m#test-worker-2", moduleWorkerConfigs[1].Name, "Second worker should have the correct name")
+	_, err = app.AddModuleWorkers(module1.Workers...)
+	require.NoError(t, err, "Expected no error when adding the first module workers")
+	_, err = app.AddModuleWorkers(module2.Workers...)
+	require.NoError(t, err, "Expected no error when adding the second module workers")
 
-	resetModuleWorkers()
+	// Verify that both workers were added
+	require.Len(t, app.Workers, 2, "Expected two workers in the app")
+	require.Equal(t, "m#test-worker-1", app.Workers[0].Name, "First worker should have the correct name")
+	require.Equal(t, "m#test-worker-2", app.Workers[1].Name, "Second worker should have the correct name")
 }
 
 func TestModuleWorkerWithEnvironmentVariables(t *testing.T) {
@@ -198,8 +200,6 @@ func TestModuleWorkerWithEnvironmentVariables(t *testing.T) {
 	require.Len(t, module.Workers[0].Env, 2, "Expected two environment variables")
 	require.Equal(t, "production", module.Workers[0].Env["APP_ENV"], "APP_ENV should be set to production")
 	require.Equal(t, "true", module.Workers[0].Env["DEBUG"], "DEBUG should be set to true")
-
-	resetModuleWorkers()
 }
 
 func TestModuleWorkerWithWatchConfiguration(t *testing.T) {
@@ -236,8 +236,6 @@ func TestModuleWorkerWithWatchConfiguration(t *testing.T) {
 	require.Equal(t, "./**/*.{php,yaml,yml,twig,env}", module.Workers[0].Watch[0], "First watch pattern should be the default")
 	require.Equal(t, "./src/**/*.php", module.Workers[0].Watch[1], "Second watch pattern should match the configuration")
 	require.Equal(t, "./config/**/*.yaml", module.Workers[0].Watch[2], "Third watch pattern should match the configuration")
-
-	resetModuleWorkers()
 }
 
 func TestModuleWorkerWithCustomName(t *testing.T) {
@@ -256,6 +254,7 @@ func TestModuleWorkerWithCustomName(t *testing.T) {
 	// Parse the configuration
 	d := caddyfile.NewTestDispenser(configWithCustomName)
 	module := &FrankenPHPModule{}
+	app := &FrankenPHPApp{}
 
 	// Unmarshal the configuration
 	err := module.UnmarshalCaddyfile(d)
@@ -267,8 +266,9 @@ func TestModuleWorkerWithCustomName(t *testing.T) {
 	require.Len(t, module.Workers, 1, "Expected one worker to be added to the module")
 	require.Equal(t, "../testdata/worker-with-env.php", module.Workers[0].FileName, "Worker should have the correct filename")
 
-	// Verify that the worker was added to moduleWorkerConfigs with the m# prefix
-	require.Equal(t, "m#custom-worker-name", module.Workers[0].Name, "Worker should have the custom name")
-
-	resetModuleWorkers()
+	// Verify that the worker was added to app.Workers with the m# prefix
+	_, err = app.AddModuleWorkers(module.Workers...)
+	require.NoError(t, err, "Expected no error when adding the worker to the app")
+	require.Equal(t, "m#custom-worker-name", module.Workers[0].Name, "Worker should have the custom name, prefixed with m#")
+	require.Equal(t, "m#custom-worker-name", app.Workers[0].Name, "Worker should have the custom name, prefixed with m#")
 }