figure out the worker to use in FrankenPHPModule::ServeHTTP

Author: henderkes

caddy/caddy.go

@@ -427,10 +427,6 @@ func (f *FrankenPHPModule) Provision(ctx caddy.Context) error {
 		}
 	}
 
-	if len(f.Workers) > 0 {
-		moduleWorkers = append(moduleWorkers, f.Workers...)
-	}
-
 	return nil
 }
 
@@ -460,20 +456,30 @@ func (f *FrankenPHPModule) ServeHTTP(w http.ResponseWriter, r *http.Request, _ c
 		}
 	}
 
-	workerNames := make([]string, len(f.Workers))
-	for i, w := range f.Workers {
-		workerNames[i] = w.Name
-	}
-
-	fr, err := frankenphp.NewRequestWithContext(
+	filename, fc, err := frankenphp.NewFrankenPHPContext(
 		r,
 		documentRootOption,
 		frankenphp.WithRequestSplitPath(f.SplitPath),
 		frankenphp.WithRequestPreparedEnv(env),
 		frankenphp.WithOriginalRequest(&origReq),
-		frankenphp.WithWorkerNames(workerNames),
 	)
+	if err != nil {
+		return caddyhttp.Error(http.StatusInternalServerError, err)
+	}
+
+	workerName := ""
+	for _, w := range f.Workers {
+		if p, _ := fastabs.FastAbs(w.FileName); p == filename {
+			workerName = w.Name
+		}
+	}
 
+	err = frankenphp.WithModuleWorker(workerName)(fc)
+	if err != nil {
+		return caddyhttp.Error(http.StatusInternalServerError, err)
+	}
+
+	fr, err := frankenphp.NewRequestWithExistingContext(r, fc)
 	if err != nil {
 		return caddyhttp.Error(http.StatusInternalServerError, err)
 	}
@@ -577,14 +583,28 @@ func (f *FrankenPHPModule) UnmarshalCaddyfile(d *caddyfile.Dispenser) error {
 							envString += k + "=" + v + ","
 						}
 						envString = strings.TrimSuffix(envString, ",")
-						wc.Name += "env:" + envString + "_" + wc.FileName
+						wc.Name += "env:" + envString
 					}
 				}
 				if wc.Name != "" && !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 must not have duplicate filenames: %s", wc.FileName)
+					}
+				}
+				// Check if a worker with this name already exists
+				for _, existingWorker := range moduleWorkers {
+					if existingWorker.Name == wc.Name {
+						return fmt.Errorf("workers must not have duplicate names: %s", wc.Name)
+					}
+				}
+
 				f.Workers = append(f.Workers, wc)
+				moduleWorkers = append(moduleWorkers, f.Workers...)
 			}
 		}
 	}

caddy/caddy_test.go

@@ -161,7 +161,7 @@ func TestGlobalAndModuleWorker(t *testing.T) {
 		}
 		`, "caddyfile")
 
-	for i := 0; i < 10; i++ {
+	for i := 0; i < 2; i++ {
 		wg.Add(1)
 
 		go func(i int) {
@@ -183,13 +183,7 @@ func TestNamedModuleWorkers(t *testing.T) {
 			skip_install_trust
 			admin localhost:2999
 
-			frankenphp {
-				worker {
-					file ../testdata/worker-with-env.php
-					num 1
-					env APP_ENV global
-				}
-			}
+			frankenphp
 		}
 
 		http://localhost:`+testPort+` {

context.go

@@ -23,7 +23,7 @@ type frankenPHPContext struct {
 	pathInfo       string
 	scriptName     string
 	scriptFilename string
-	workerNames    []string
+	workerName     string
 
 	// Whether the request is already closed by us
 	isDone bool
@@ -40,15 +40,15 @@ func fromContext(ctx context.Context) (fctx *frankenPHPContext, ok bool) {
 	return
 }
 
-func newFrankenPHPContext(r *http.Request, opts ...RequestOption) (*frankenPHPContext, error) {
+func NewFrankenPHPContext(r *http.Request, opts ...RequestOption) (string, *frankenPHPContext, error) {
 	fc := &frankenPHPContext{
 		done:      make(chan interface{}),
 		startedAt: time.Now(),
 		request:   r,
 	}
 	for _, o := range opts {
 		if err := o(fc); err != nil {
-			return nil, err
+			return "", nil, err
 		}
 	}
 
@@ -62,7 +62,7 @@ func newFrankenPHPContext(r *http.Request, opts ...RequestOption) (*frankenPHPCo
 		} else {
 			var err error
 			if fc.documentRoot, err = os.Getwd(); err != nil {
-				return nil, err
+				return "", nil, err
 			}
 		}
 	}
@@ -91,12 +91,12 @@ func newFrankenPHPContext(r *http.Request, opts ...RequestOption) (*frankenPHPCo
 
 	// SCRIPT_FILENAME is the absolute path of SCRIPT_NAME
 	fc.scriptFilename = sanitizedPathJoin(fc.documentRoot, fc.scriptName)
-	return fc, nil
+	return fc.scriptFilename, fc, nil
 }
 
 // NewRequestWithContext creates a new FrankenPHP request context.
 func NewRequestWithContext(r *http.Request, opts ...RequestOption) (*http.Request, error) {
-	fc, err2 := newFrankenPHPContext(r, opts...)
+	_, fc, err2 := NewFrankenPHPContext(r, opts...)
 	if err2 != nil {
 		return nil, err2
 	}
@@ -105,6 +105,13 @@ func NewRequestWithContext(r *http.Request, opts ...RequestOption) (*http.Reques
 	return r.WithContext(c), nil
 }
 
+// NewRequestWithExistingContext wraps an http request with an existing FrankenPHP request context.
+func NewRequestWithExistingContext(r *http.Request, fc *frankenPHPContext) (*http.Request, error) {
+	c := context.WithValue(r.Context(), contextKey, fc)
+
+	return r.WithContext(c), nil
+}
+
 func newDummyContext(requestPath string, opts ...RequestOption) (*frankenPHPContext, error) {
 	r, err := http.NewRequest(http.MethodGet, requestPath, nil)
 	if err != nil {

frankenphp.go

@@ -404,13 +404,14 @@ func ServeHTTP(responseWriter http.ResponseWriter, request *http.Request) error
 		return nil
 	}
 
-	workerNames := append(fc.workerNames, fc.scriptFilename)
-	for _, workerName := range workerNames {
-		// Detect if a worker is available to handle this request
-		if worker := getWorkerForName(workerName); worker != nil {
-			worker.handleRequest(fc)
-			return nil
-		}
+	workerName := fc.workerName
+	if workerName == "" {
+		workerName = fc.scriptFilename
+	}
+	// Detect if a worker is available to handle this request
+	if worker := getWorkerForName(workerName); worker != nil {
+		worker.handleRequest(fc)
+		return nil
 	}
 
 	// If no worker was available, send the request to non-worker threads

request_options.go

@@ -124,10 +124,10 @@ func WithRequestLogger(logger *zap.Logger) RequestOption {
 	}
 }
 
-// WithRequestLogger sets the logger associated with the current request
-func WithWorkerNames(names []string) RequestOption {
+// WithModuleWorker sets the worker that should handle the request
+func WithModuleWorker(name string) RequestOption {
 	return func(o *frankenPHPContext) error {
-		o.workerNames = names
+		o.workerName = name
 
 		return nil
 	}