Skip to content

Commit 38f008e

Browse files
feat: batch ensure + FRANKENPHP_WORKER_BACKGROUND server flag
Two small, related polish steps on the bg-worker surface, landing together: - frankenphp_ensure_background_worker now accepts string|array. The array form lazy-starts every named worker fire-and-forget, with the same semantics as the single-string call repeated N times. Input is validated up-front: empty arrays raise ValueError, non-string elements raise TypeError, empty-string and duplicate names raise ValueError. Validation happens before any worker is started so a bad input never leaves a half-spawned batch behind. - $_SERVER['FRANKENPHP_WORKER_BACKGROUND'] = true in background worker scripts, alongside the existing FRANKENPHP_WORKER_NAME wiring. Gives scripts a single-key branch for "am I a bg worker?" without having to probe other frankenphp_* helpers. Set unconditionally for bg workers (catch-all instances with no declared name still see the flag, just no name). - TestEnsureBackgroundWorkerBatch: ensure(['a','b','c']) starts three catch-all-resolved instances; assert three per-name sentinels appear. - TestEnsureBackgroundWorkerBatchEmpty: [] raises ValueError. Driven through a PHP fixture that catches the throwable since the validation lives in the Zend parameter-parsing path. - TestEnsureBackgroundWorkerBatchNonString: ['ok-name', 42] raises TypeError, same fixture pattern. - TestEnsureBackgroundWorkerBatchDuplicate: ['dup','dup'] raises ValueError (duplicate names rejected, not silently deduped). - TestBackgroundWorkerBgFlag: bg worker writes var_export() of $_SERVER['FRANKENPHP_WORKER_BACKGROUND'] to a sentinel; assert the exact value is the bool true.
1 parent c3c8e29 commit 38f008e

11 files changed

Lines changed: 396 additions & 50 deletions

background_worker_batch_test.go

Lines changed: 165 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,165 @@
1+
package frankenphp_test
2+
3+
import (
4+
"io"
5+
"net/http/httptest"
6+
"os"
7+
"path/filepath"
8+
"strings"
9+
"testing"
10+
"time"
11+
12+
"github.com/dunglas/frankenphp"
13+
"github.com/stretchr/testify/assert"
14+
"github.com/stretchr/testify/require"
15+
)
16+
17+
// serveTestRequest issues an HTTP request through frankenphp against the
18+
// given script under testdata/ and returns the response body. ErrRejected
19+
// is treated as a non-fatal outcome so worker-mode quirks don't fail tests
20+
// that only care about the script's stdout.
21+
func serveTestRequest(t *testing.T, testDataDir, script, query string) string {
22+
t.Helper()
23+
url := "http://example.com/" + script
24+
if query != "" {
25+
url += "?" + query
26+
}
27+
req := httptest.NewRequest("GET", url, nil)
28+
fr, err := frankenphp.NewRequestWithContext(req, frankenphp.WithRequestDocumentRoot(testDataDir, false))
29+
require.NoError(t, err)
30+
31+
w := httptest.NewRecorder()
32+
err = frankenphp.ServeHTTP(w, fr)
33+
if err != nil {
34+
require.ErrorAs(t, err, &frankenphp.ErrRejected{})
35+
}
36+
body, err := io.ReadAll(w.Result().Body)
37+
require.NoError(t, err)
38+
return string(body)
39+
}
40+
41+
// TestEnsureBackgroundWorkerBatch declares a single catch-all bg worker
42+
// and ensures three distinct names from a single ensure() call. Each
43+
// catch-all instance touches a per-name sentinel; the test asserts that
44+
// all three appear, proving the array form started one worker per name.
45+
func TestEnsureBackgroundWorkerBatch(t *testing.T) {
46+
cwd, _ := os.Getwd()
47+
testDataDir := cwd + "/testdata/"
48+
tmp := t.TempDir()
49+
50+
require.NoError(t, frankenphp.Init(
51+
frankenphp.WithWorkers("", testDataDir+"background-worker-named.php", 0,
52+
frankenphp.WithWorkerBackground(),
53+
frankenphp.WithWorkerMaxThreads(8),
54+
frankenphp.WithWorkerEnv(map[string]string{"BG_SENTINEL_DIR": tmp}),
55+
),
56+
frankenphp.WithNumThreads(8),
57+
))
58+
t.Cleanup(frankenphp.Shutdown)
59+
60+
body := serveTestRequest(t, testDataDir, "background-worker-batch-ensure.php", "")
61+
assert.Contains(t, body, "ok", "batch ensure script should echo ok, got: %q", body)
62+
63+
for _, name := range []string{"batch-a", "batch-b", "batch-c"} {
64+
path := filepath.Join(tmp, name)
65+
assert.Eventually(t, func() bool {
66+
_, err := os.Stat(path)
67+
return err == nil
68+
}, 5*time.Second, 10*time.Millisecond,
69+
"catch-all instance %q should have written its sentinel", name)
70+
}
71+
}
72+
73+
// TestEnsureBackgroundWorkerBatchEmpty exercises the C-side validation
74+
// that an empty array raises a ValueError before any worker is started.
75+
// The fixture catches the throwable and echoes its class.
76+
func TestEnsureBackgroundWorkerBatchEmpty(t *testing.T) {
77+
cwd, _ := os.Getwd()
78+
testDataDir := cwd + "/testdata/"
79+
80+
require.NoError(t, frankenphp.Init(
81+
frankenphp.WithWorkers("", testDataDir+"background-worker-named.php", 0,
82+
frankenphp.WithWorkerBackground(),
83+
),
84+
frankenphp.WithNumThreads(2),
85+
))
86+
t.Cleanup(frankenphp.Shutdown)
87+
88+
body := serveTestRequest(t, testDataDir, "background-worker-batch-errors.php", "mode=empty")
89+
assert.True(t, strings.Contains(body, "ValueError"),
90+
"empty array should raise ValueError, got: %q", body)
91+
assert.Contains(t, body, "must not be empty")
92+
}
93+
94+
// TestEnsureBackgroundWorkerBatchNonString verifies a non-string element
95+
// raises a TypeError (PHP's standard for argument-type mismatches inside
96+
// our parsed array).
97+
func TestEnsureBackgroundWorkerBatchNonString(t *testing.T) {
98+
cwd, _ := os.Getwd()
99+
testDataDir := cwd + "/testdata/"
100+
101+
require.NoError(t, frankenphp.Init(
102+
frankenphp.WithWorkers("", testDataDir+"background-worker-named.php", 0,
103+
frankenphp.WithWorkerBackground(),
104+
),
105+
frankenphp.WithNumThreads(2),
106+
))
107+
t.Cleanup(frankenphp.Shutdown)
108+
109+
body := serveTestRequest(t, testDataDir, "background-worker-batch-errors.php", "mode=nonstring")
110+
assert.True(t, strings.Contains(body, "TypeError"),
111+
"non-string element should raise TypeError, got: %q", body)
112+
}
113+
114+
// TestEnsureBackgroundWorkerBatchDuplicate verifies that duplicate names
115+
// in the same batch are rejected as a ValueError, matching the e17577e
116+
// reference behavior (no silent dedup).
117+
func TestEnsureBackgroundWorkerBatchDuplicate(t *testing.T) {
118+
cwd, _ := os.Getwd()
119+
testDataDir := cwd + "/testdata/"
120+
121+
require.NoError(t, frankenphp.Init(
122+
frankenphp.WithWorkers("", testDataDir+"background-worker-named.php", 0,
123+
frankenphp.WithWorkerBackground(),
124+
),
125+
frankenphp.WithNumThreads(2),
126+
))
127+
t.Cleanup(frankenphp.Shutdown)
128+
129+
body := serveTestRequest(t, testDataDir, "background-worker-batch-errors.php", "mode=duplicate")
130+
assert.True(t, strings.Contains(body, "ValueError"),
131+
"duplicate name should raise ValueError, got: %q", body)
132+
assert.Contains(t, body, "duplicate")
133+
}
134+
135+
// TestBackgroundWorkerBgFlag asserts that a bg worker script sees
136+
// $_SERVER['FRANKENPHP_WORKER_BACKGROUND'] === true. The fixture writes
137+
// var_export() of the value to a sentinel so the test can read the exact
138+
// PHP-level representation.
139+
func TestBackgroundWorkerBgFlag(t *testing.T) {
140+
cwd, _ := os.Getwd()
141+
testDataDir := cwd + "/testdata/"
142+
143+
tmp := t.TempDir()
144+
sentinel := filepath.Join(tmp, "bg-flag.sentinel")
145+
146+
require.NoError(t, frankenphp.Init(
147+
frankenphp.WithWorkers("bg-flag", testDataDir+"background-worker-bg-flag.php", 1,
148+
frankenphp.WithWorkerBackground(),
149+
frankenphp.WithWorkerEnv(map[string]string{"BG_SENTINEL": sentinel}),
150+
),
151+
frankenphp.WithNumThreads(2),
152+
))
153+
t.Cleanup(frankenphp.Shutdown)
154+
155+
require.Eventually(t, func() bool {
156+
_, err := os.Stat(sentinel)
157+
return err == nil
158+
}, 5*time.Second, 10*time.Millisecond,
159+
"bg worker should have written the FRANKENPHP_WORKER_BACKGROUND sentinel")
160+
161+
contents, err := os.ReadFile(sentinel)
162+
require.NoError(t, err)
163+
assert.Equal(t, "true", string(contents),
164+
"$_SERVER['FRANKENPHP_WORKER_BACKGROUND'] should be the bool true")
165+
}

background_worker_ensure_test.go

Lines changed: 11 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -11,18 +11,13 @@ import (
1111
"github.com/stretchr/testify/require"
1212
)
1313

14-
// waitForSentinel polls for a file to appear; returns true if it does
15-
// before the deadline.
16-
func waitForSentinel(t *testing.T, path string, within time.Duration) bool {
17-
t.Helper()
18-
deadline := time.Now().Add(within)
19-
for time.Now().Before(deadline) {
20-
if _, err := os.Stat(path); err == nil {
21-
return true
22-
}
23-
time.Sleep(10 * time.Millisecond)
14+
// sentinelExists is a predicate suitable for assert.Eventually that returns
15+
// true once the named file appears on disk.
16+
func sentinelExists(path string) func() bool {
17+
return func() bool {
18+
_, err := os.Stat(path)
19+
return err == nil
2420
}
25-
return false
2621
}
2722

2823
// TestEnsureBackgroundWorkerNamedLazy declares a num=0 named worker, then
@@ -47,8 +42,9 @@ func TestEnsureBackgroundWorkerNamedLazy(t *testing.T) {
4742
_, err := os.Stat(filepath.Join(tmp, "bg-lazy"))
4843
require.True(t, os.IsNotExist(err), "lazy worker should not have started yet")
4944

50-
require.NoError(t, ensureBackgroundWorker(nil,"bg-lazy"))
51-
assert.True(t, waitForSentinel(t, filepath.Join(tmp, "bg-lazy"), 5*time.Second),
45+
require.NoError(t, ensureBackgroundWorker(nil, "bg-lazy"))
46+
assert.Eventually(t, sentinelExists(filepath.Join(tmp, "bg-lazy")),
47+
5*time.Second, 10*time.Millisecond,
5248
"ensure() should have lazy-started the named bg worker")
5349
}
5450

@@ -76,7 +72,8 @@ func TestEnsureBackgroundWorkerCatchAll(t *testing.T) {
7672
}
7773

7874
for _, name := range []string{"job-a", "job-b"} {
79-
assert.True(t, waitForSentinel(t, filepath.Join(tmp, name), 5*time.Second),
75+
assert.Eventually(t, sentinelExists(filepath.Join(tmp, name)),
76+
5*time.Second, 10*time.Millisecond,
8077
"catch-all instance %q should have written its sentinel", name)
8178
}
8279
}

background_worker_scope_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,8 @@ func TestBackgroundWorkerCatchAllPerScope(t *testing.T) {
136136

137137
// Wait for the worker to write its sentinel so we know the right
138138
// fixture ran (sanity check that env was inherited from scope A).
139-
assert.True(t, waitForSentinel(t, filepath.Join(dirA, "job-a"), 3*time.Second),
139+
assert.Eventually(t, sentinelExists(filepath.Join(dirA, "job-a")),
140+
3*time.Second, 10*time.Millisecond,
140141
"scope A catch-all instance must write its sentinel under scope A's BG_SENTINEL_DIR")
141142
}
142143

background_worker_test.go

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package frankenphp_test
22

33
import (
4-
"errors"
54
"io"
65
"net/http/httptest"
76
"os"
@@ -109,11 +108,13 @@ func TestBackgroundWorkerWithoutHTTP(t *testing.T) {
109108
require.NoError(t, err)
110109

111110
w := httptest.NewRecorder()
112-
if err := frankenphp.ServeHTTP(w, fr); err != nil && !errors.As(err, &frankenphp.ErrRejected{}) {
113-
t.Fatalf("serve: %v", err)
111+
err = frankenphp.ServeHTTP(w, fr)
112+
if err != nil {
113+
require.ErrorAs(t, err, &frankenphp.ErrRejected{})
114114
}
115115

116-
body, _ := io.ReadAll(w.Result().Body)
116+
body, err := io.ReadAll(w.Result().Body)
117+
require.NoError(t, err)
117118
assert.True(t, strings.Contains(string(body), "I am by birth a Genevese") || strings.Contains(string(body), "<?php") || len(body) > 0,
118119
"expected non-empty body from index.php, got %q", string(body))
119120
}
Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"strings"
99
"sync"
1010
"sync/atomic"
11+
"unsafe"
1112
)
1213

1314
// defaultMaxBackgroundWorkers is the default safety cap for catch-all
@@ -20,7 +21,7 @@ const defaultMaxBackgroundWorkers = 16
2021
// declare workers with the same name without conflict. The zero value is
2122
// the global/embed scope (used when no per-block scope was assigned).
2223
// Representation is opaque; obtain values via NextBackgroundWorkerScope.
23-
type BackgroundScope int
24+
type BackgroundScope uint64
2425

2526
var backgroundScopeCounter atomic.Uint64
2627

@@ -314,17 +315,27 @@ func ensureBackgroundWorker(thread *phpThread, bgWorkerName string) error {
314315
return nil
315316
}
316317

317-
// go_frankenphp_ensure_background_worker declares a dependency on a
318-
// background worker by name. Lazy-starts it if not already running and
319-
// returns immediately. Fire-and-forget: there is no readiness signal in
320-
// this step, so callers cannot block on the worker reaching any state.
318+
// go_frankenphp_ensure_background_worker declares a dependency on one or
319+
// more background workers by name. Each named worker is lazy-started if
320+
// not already running. Fire-and-forget: there is no readiness signal in
321+
// this step, so callers cannot block on the workers reaching any state.
322+
// The C side has already validated that names is non-empty and that every
323+
// element is a non-empty unique string.
321324
//
322325
//export go_frankenphp_ensure_background_worker
323-
func go_frankenphp_ensure_background_worker(threadIndex C.uintptr_t, name *C.char, nameLen C.size_t) *C.char {
326+
func go_frankenphp_ensure_background_worker(threadIndex C.uintptr_t, names **C.char, nameLens *C.size_t, nameCount C.int) *C.char {
324327
thread := phpThreads[threadIndex]
325-
goName := C.GoStringN(name, C.int(nameLen))
326-
if err := ensureBackgroundWorker(thread, goName); err != nil {
327-
return C.CString(err.Error())
328+
n := int(nameCount)
329+
if n <= 0 {
330+
return nil
331+
}
332+
nameSlice := unsafe.Slice(names, n)
333+
nameLenSlice := unsafe.Slice(nameLens, n)
334+
for i := 0; i < n; i++ {
335+
goName := C.GoStringN(nameSlice[i], C.int(nameLenSlice[i]))
336+
if err := ensureBackgroundWorker(thread, goName); err != nil {
337+
return C.CString(err.Error())
338+
}
328339
}
329340
return nil
330341
}

0 commit comments

Comments
 (0)