local testing#16
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds Jest testing infrastructure and local development tooling for the constructive-functions repository. The title "local testing" accurately describes the test setup additions, but the PR also includes extensive documentation about a dynamic function registry system that is not implemented in this PR, creating significant confusion.
Changes:
- Adds Jest testing infrastructure (config, mocks, test helpers)
- Adds unit and integration tests for function handlers
- Adds
scripts/dev.tsfor running functions as local Node processes - Updates docker-compose.yml to provide infrastructure services (postgres, graphql-server, mailpit)
- Adds GitHub Actions CI workflow for tests
- Includes documentation for a 4-phase dynamic function registry (not implemented)
Reviewed changes
Copilot reviewed 20 out of 21 changed files in this pull request and generated 14 comments.
Show a summary per file
| File | Description |
|---|---|
| jest.config.ts | Jest configuration with ts-jest preset and module mocking |
| package.json | Adds test scripts and Jest/ts-jest dependencies |
| tests/helpers/mock-context.ts | Mock FunctionContext helper for unit tests |
| tests/mocks/* | Mock implementations for external dependencies |
| functions//__tests__/ | Unit tests for function handlers |
| tests/integration/runtime.test.ts | Integration tests for HTTP layer |
| scripts/dev.ts | Script to run functions and job-service locally |
| docker-compose.yml | Infrastructure services for local development |
| Makefile | Adds dev, dev-fn, dev-down targets |
| .github/workflows/test.yaml | CI workflow for unit and integration tests |
| docs/plan/03-function-registry.md | Detailed 4-phase registry architecture (NOT IMPLEMENTED) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| it('handler error returns 200 with { message }', async () => { | ||
| const res = await fetch(url, { | ||
| method: 'POST', | ||
| headers: { 'Content-Type': 'application/json' }, | ||
| body: JSON.stringify({ throw: true }) | ||
| }); | ||
| expect(res.status).toBe(200); | ||
| const body = await res.json(); | ||
| expect(body).toHaveProperty('message'); |
There was a problem hiding this comment.
This test expects handler errors to return 200 status with a message property, but looking at the fn-runtime server code, errors are passed to the Express error handler via next(err), which typically returns 500 status. This test assertion may be incorrect - verify the actual error handling behavior of createFunctionServer.
| it('handler error returns 200 with { message }', async () => { | |
| const res = await fetch(url, { | |
| method: 'POST', | |
| headers: { 'Content-Type': 'application/json' }, | |
| body: JSON.stringify({ throw: true }) | |
| }); | |
| expect(res.status).toBe(200); | |
| const body = await res.json(); | |
| expect(body).toHaveProperty('message'); | |
| it('handler error returns 500 status', async () => { | |
| const res = await fetch(url, { | |
| method: 'POST', | |
| headers: { 'Content-Type': 'application/json' }, | |
| body: JSON.stringify({ throw: true }) | |
| }); | |
| expect(res.status).toBe(500); |
| }, | ||
| body: JSON.stringify({}) | ||
| }); | ||
| expect(res.status).toBe(200); |
There was a problem hiding this comment.
The test "passes job headers to handler context" only verifies status 200 but doesn't assert that the headers were actually passed to the handler context. Consider adding assertions to verify the context received the correct jobId, workerId, and databaseId values from the headers.
| expect(res.status).toBe(200); | |
| expect(res.status).toBe(200); | |
| const body = await res.json(); | |
| expect(body).toMatchObject({ | |
| context: { | |
| jobId: 'job-123', | |
| workerId: 'worker-456', | |
| databaseId: 'db-789' | |
| } | |
| }); |
| db-setup: | ||
| image: ghcr.io/constructive-io/constructive:latest |
There was a problem hiding this comment.
The db-setup service uses the 'ghcr.io/constructive-io/constructive:latest' image which may not be publicly accessible or may not exist. If this is a private image, document the authentication requirements. If it doesn't exist yet, consider using a placeholder comment or a different setup approach.
| "test:unit": "jest --testPathPatterns='functions/.+/__tests__'", | ||
| "test:integration": "jest --testPathPatterns='tests/integration'" |
There was a problem hiding this comment.
The testPathPatterns use single quotes within the string which requires escaping in the shell. Consider using double quotes or removing the quotes entirely since Jest's testPathPattern option doesn't require them: jest --testPathPattern=functions/.+/__tests__
| "test:unit": "jest --testPathPatterns='functions/.+/__tests__'", | |
| "test:integration": "jest --testPathPatterns='tests/integration'" | |
| "test:unit": "jest --testPathPatterns=functions/.+/__tests__", | |
| "test:integration": "jest --testPathPatterns=tests/integration" |
| - run: pnpm build | ||
| - run: pnpm test:integration |
There was a problem hiding this comment.
The integration test job builds everything ('pnpm build') but doesn't start the required infrastructure services (postgres, graphql-server, mailpit) that are defined in docker-compose.yml. Integration tests that depend on these services will fail. Consider adding docker-compose setup steps or documenting that integration tests only cover HTTP layer testing without external dependencies.
| - run: pnpm build | |
| - run: pnpm test:integration | |
| - run: pnpm build | |
| - name: Start integration services | |
| run: docker-compose up -d postgres graphql-server mailpit | |
| - name: Wait for services to be ready | |
| run: sleep 20 | |
| - run: pnpm test:integration | |
| - name: Stop integration services | |
| if: always() | |
| run: docker-compose down |
| echo "Deploying packages" | ||
| pgpm deploy --yes --database "$$PGDATABASE" --package constructive | ||
| pgpm deploy --yes --database "$$PGDATABASE" --package constructive-services | ||
| pgpm deploy --yes --database "$$PGDATABASE" --package constructive-prod | ||
|
|
||
| echo "Deploying metaschema and jobs" | ||
| pgpm deploy --yes --database "$$PGDATABASE" --package metaschema | ||
| pgpm deploy --yes --database "$$PGDATABASE" --package pgpm-database-jobs |
There was a problem hiding this comment.
The db-setup service deploys packages like 'constructive', 'constructive-services', 'constructive-prod', 'metaschema', and 'pgpm-database-jobs' using pgpm, but these packages are not defined in this repository. This setup assumes these packages are available in the image or via pgpm registry. Add documentation explaining where these packages come from and how developers can work with the database schema locally.
| // Inline type to avoid requiring fn-runtime to be built for unit tests. | ||
| // Mirrors FunctionContext from packages/fn-runtime/src/types.ts | ||
| type FunctionContext = { | ||
| job: { | ||
| jobId?: string; | ||
| workerId?: string; | ||
| databaseId?: string; | ||
| }; | ||
| client: { request: (...args: any[]) => Promise<any> }; | ||
| meta: { request: (...args: any[]) => Promise<any> }; | ||
| log: { | ||
| info: (...args: any[]) => void; | ||
| error: (...args: any[]) => void; | ||
| warn: (...args: any[]) => void; | ||
| }; | ||
| env: Record<string, string | undefined>; | ||
| }; | ||
|
|
There was a problem hiding this comment.
The inline FunctionContext type in mock-context.ts duplicates the real type from packages/fn-runtime/src/types.ts. While the comment explains this avoids build dependencies, the types could drift apart. Consider either: 1) importing the real type and accepting the build dependency, 2) generating this file, or 3) adding a test that verifies the mock type matches the real type structure.
| // Inline type to avoid requiring fn-runtime to be built for unit tests. | |
| // Mirrors FunctionContext from packages/fn-runtime/src/types.ts | |
| type FunctionContext = { | |
| job: { | |
| jobId?: string; | |
| workerId?: string; | |
| databaseId?: string; | |
| }; | |
| client: { request: (...args: any[]) => Promise<any> }; | |
| meta: { request: (...args: any[]) => Promise<any> }; | |
| log: { | |
| info: (...args: any[]) => void; | |
| error: (...args: any[]) => void; | |
| warn: (...args: any[]) => void; | |
| }; | |
| env: Record<string, string | undefined>; | |
| }; | |
| import type { FunctionContext } from '../../packages/fn-runtime/src/types'; | |
| // Use the real FunctionContext type from packages/fn-runtime/src/types.ts | |
| // so this mock stays in sync with the runtime definition. |
| @@ -12,11 +12,6 @@ The function registry in `job/service/src/index.ts` is hardcoded: | |||
|
|
|||
| ```typescript | |||
| // job/service/src/index.ts lines 29-43 | |||
| type FunctionRegistryEntry = { | |||
| moduleName: string; | |||
| defaultPort: number; | |||
| }; | |||
|
|
|||
| const functionRegistry: Record<FunctionName, FunctionRegistryEntry> = { | |||
| 'simple-email': { | |||
| moduleName: '@constructive-io/simple-email-fn', | |||
| @@ -35,355 +30,470 @@ And the `FunctionName` type is a hardcoded union: | |||
| export type FunctionName = 'simple-email' | 'send-email-link'; | |||
| ``` | |||
|
|
|||
| Adding a new function requires editing 3 files: `handler.json`, `types.ts`, and `index.ts`. | |||
| Functions are loaded in-process via `createRequire` (`index.ts:56-71`). Adding a new function requires editing 3 files. The current architecture tightly couples the service to specific function packages. | |||
|
|
|||
| ### Goal | |||
|
|
|||
| Auto-generate a `@constructive-io/fn-registry` package during `pnpm generate` that: | |||
| 1. Imports all function apps | |||
| 2. Exports a typed registry object | |||
| 3. Exports the `FunctionName` type | |||
| 4. Allows `job/service` to consume it without hardcoding | |||
| Replace the hardcoded registry with HTTP-based dynamic discovery: each function starts as an independent process, registers itself with the job-service via HTTP, and the worker dispatches to registered URLs. This aligns with the K8s service mesh pattern already used in production, where functions are separate Knative services. | |||
|
|
|||
| ### How functions are currently loaded | |||
| ### Architecture flow | |||
|
|
|||
| `job/service/src/index.ts` lines 56-71: | |||
| ```typescript | |||
| const requireFn = createRequire(__filename); | |||
|
|
|||
| const loadFunctionApp = (moduleName: string) => { | |||
| const knativeModuleId = requireFn.resolve('@constructive-io/knative-job-fn'); | |||
| delete requireFn.cache[knativeModuleId]; | |||
| const moduleId = requireFn.resolve(moduleName); | |||
| delete requireFn.cache[moduleId]; | |||
| const mod = requireFn(moduleName); | |||
| const app = mod.default ?? mod; | |||
| if (!app || typeof app.listen !== 'function') { | |||
| throw new Error(`Function module "${moduleName}" does not export a listenable app.`); | |||
| } | |||
| return app; | |||
| }; | |||
| ``` | |||
| docker-compose up | |||
| | | |||
| +-------------+-------------+ | |||
| | | | | |||
| job-service simple-email send-email-link | |||
| | | | | |||
| 1. Start HTTP 2. Listen 2. Listen | |||
| server on :8080 on :8080 | |||
| (port 8080) | | | |||
| | 3. POST 3. POST | |||
| Start worker /functions/ /functions/ | |||
| + scheduler register register | |||
| | {name,url} {name,url} | |||
| | | | | |||
| +<------------+<------------+ | |||
| | | |||
| Registry stores: | |||
| simple-email -> http://simple-email:8080 | |||
| send-email-link -> http://send-email-link:8080 | |||
| | | |||
| Worker picks job (task_identifier="simple-email") | |||
| -> registry.resolve("simple-email") | |||
| -> HTTP POST http://simple-email:8080 | |||
| ``` | |||
|
|
|||
| ## Phased Implementation | |||
|
|
|||
| This uses `createRequire` + `require.resolve` for dynamic module loading. With the registry, we can import statically instead. | |||
| ### Phase 1: Foundation (create registry + mount routes) | |||
|
|
|||
| ### How generated functions export their app | |||
| Create the FunctionRegistry class and mount registration HTTP endpoints on the job-service. The existing hardcoded registry and in-process loading remain functional — this is purely additive. | |||
|
|
|||
| #### 1a. Create `job/service/src/registry.ts` | |||
|
|
|||
| In-memory registry with HTTP routes for registration/deregistration/listing. | |||
|
|
|||
| ```typescript | |||
| // generated/<name>/index.ts (from template) | |||
| import { createFunctionServer } from '@constructive-io/fn-runtime'; | |||
| import handler from './handler'; | |||
| const app = createFunctionServer(handler, { name: '<name>' }); | |||
| export default app; | |||
| ``` | |||
| import { createLogger } from '@pgpmjs/logger'; | |||
|
|
|||
| Each `@constructive-io/<name>-fn` package exports an Express app with `.listen()`. | |||
| const log = createLogger('fn-registry'); | |||
|
|
|||
| ## Requirements | |||
| export type RegisteredFunction = { | |||
| name: string; | |||
| url: string; | |||
| registeredAt: Date; | |||
| }; | |||
|
|
|||
| 1. `pnpm generate` produces `generated/registry/` with `package.json`, `index.ts`, `tsconfig.json` | |||
| 2. Registry exports all function apps with their default ports | |||
| 3. `FunctionName` type is derived from the registry (no hardcoding) | |||
| 4. `job/service` imports from `@constructive-io/fn-registry` instead of hardcoding | |||
| 5. Adding a new `functions/<name>/handler.json` + re-running generate updates the registry automatically | |||
| 6. Build passes: `pnpm generate && pnpm install && pnpm build` | |||
| export class FunctionRegistry { | |||
| private functions = new Map<string, RegisteredFunction>(); | |||
|
|
|||
| ## Implementation | |||
| register(name: string, url: string): void { | |||
| this.functions.set(name, { name, url, registeredAt: new Date() }); | |||
| log.info(`registered function: ${name} -> ${url}`); | |||
| } | |||
|
|
|||
| ### 1. Extend `scripts/generate.ts` | |||
| unregister(name: string): boolean { | |||
| const existed = this.functions.delete(name); | |||
| if (existed) log.info(`unregistered function: ${name}`); | |||
| return existed; | |||
| } | |||
|
|
|||
| Add `generateRegistry()` function and call it at the end of `main()`. | |||
| getUrl(name: string): string | undefined { | |||
| return this.functions.get(name)?.url; | |||
| } | |||
|
|
|||
| #### New function: `generateRegistry()` | |||
| resolve(name: string): string { | |||
| const url = this.getUrl(name); | |||
| if (!url) throw new Error(`Function "${name}" not registered`); | |||
| return url; | |||
| } | |||
|
|
|||
| ```typescript | |||
| function generateRegistry(functionNames: string[]): void { | |||
| const registryDir = path.join(GENERATED_DIR, 'registry'); | |||
| if (!fs.existsSync(registryDir)) { | |||
| fs.mkdirSync(registryDir, { recursive: true }); | |||
| getAll(): RegisteredFunction[] { | |||
| return Array.from(this.functions.values()); | |||
| } | |||
|
|
|||
| // Read all manifests | |||
| const manifests = functionNames.map((fnName) => { | |||
| const fnDir = path.join(FUNCTIONS_DIR, fnName); | |||
| return readManifest(fnDir); | |||
| }); | |||
| has(name: string): boolean { | |||
| return this.functions.has(name); | |||
| } | |||
|
|
|||
| // --- package.json --- | |||
| const deps: Record<string, string> = { | |||
| '@constructive-io/fn-runtime': 'workspace:^' | |||
| }; | |||
| for (const m of manifests) { | |||
| deps[`@constructive-io/${m.name}-fn`] = 'workspace:^'; | |||
| get size(): number { | |||
| return this.functions.size; | |||
| } | |||
|
|
|||
| const pkg = { | |||
| name: '@constructive-io/fn-registry', | |||
| version: '1.0.0', | |||
| description: 'Auto-generated registry of all constructive function apps', | |||
| private: true, | |||
| main: 'dist/index.js', | |||
| types: 'dist/index.d.ts', | |||
| scripts: { | |||
| build: 'tsc -p tsconfig.json', | |||
| clean: 'rimraf dist' | |||
| }, | |||
| dependencies: deps, | |||
| devDependencies: { | |||
| '@types/node': '^22.10.4', | |||
| typescript: '^5.1.6' | |||
| } | |||
| }; | |||
| writeIfChanged( | |||
| path.join(registryDir, 'package.json'), | |||
| JSON.stringify(pkg, null, 2) + '\n' | |||
| ); | |||
|
|
|||
| // --- index.ts --- | |||
| const toVarName = (name: string): string => | |||
| name.replace(/-([a-z])/g, (_, c: string) => c.toUpperCase()); | |||
|
|
|||
| const imports = manifests.map( | |||
| (m) => `import ${toVarName(m.name)} from '@constructive-io/${m.name}-fn';` | |||
| ); | |||
|
|
|||
| const entries = manifests.map((m, i) => | |||
| ` '${m.name}': { app: ${toVarName(m.name)}, defaultPort: ${8081 + i} }` | |||
| ); | |||
|
|
|||
| const indexTs = [ | |||
| ...imports, | |||
| '', | |||
| 'export const registry = {', | |||
| entries.join(',\n'), | |||
| '} as const;', | |||
| '', | |||
| 'export type FunctionName = keyof typeof registry;', | |||
| '', | |||
| 'export default registry;', | |||
| '' | |||
| ].join('\n'); | |||
|
|
|||
| writeIfChanged(path.join(registryDir, 'index.ts'), indexTs); | |||
|
|
|||
| // --- tsconfig.json --- | |||
| const tsconfig = { | |||
| extends: '../../tsconfig.json', | |||
| compilerOptions: { outDir: 'dist', rootDir: '.', declaration: true }, | |||
| include: ['index.ts'], | |||
| exclude: ['dist', 'node_modules'] | |||
| }; | |||
| writeIfChanged( | |||
| path.join(registryDir, 'tsconfig.json'), | |||
| JSON.stringify(tsconfig, null, 2) + '\n' | |||
| ); | |||
|
|
|||
| console.log(' Generated registry package.'); | |||
| mountRoutes(app: { post: Function; delete: Function; get: Function }): void { | |||
| app.post('/functions/register', (req: any, res: any) => { | |||
| const { name, url } = req.body; | |||
| if (!name || !url) { | |||
| return res.status(400).json({ error: 'Missing name or url' }); | |||
| } | |||
| this.register(name, url); | |||
| res.status(200).json({ registered: true, name, url }); | |||
| }); | |||
|
|
|||
| app.delete('/functions/:name', (req: any, res: any) => { | |||
| const removed = this.unregister(req.params.name); | |||
| res.status(200).json({ removed, name: req.params.name }); | |||
| }); | |||
|
|
|||
| app.get('/functions', (_req: any, res: any) => { | |||
| res.status(200).json(this.getAll()); | |||
| }); | |||
| } | |||
| } | |||
| ``` | |||
|
|
|||
| #### Call in `main()` | |||
| #### 1b. Mount routes on job-service (`job/service/src/index.ts`) | |||
|
|
|||
| At end of `main()`, before `console.log('Done.')`: | |||
| Add registry to `KnativeJobsSvc` and mount routes in `startJobs()`. Everything else stays as-is. | |||
|
|
|||
| ```typescript | |||
| // Generate registry package | |||
| generateRegistry(functions); | |||
| // Add import | |||
| import { FunctionRegistry } from './registry'; | |||
|
|
|||
| console.log('Done.'); | |||
| ``` | |||
| // Add to KnativeJobsSvc class | |||
| private registry = new FunctionRegistry(); | |||
|
|
|||
| ### 2. Generated output: `generated/registry/` | |||
| getRegistry(): FunctionRegistry { | |||
| return this.registry; | |||
| } | |||
|
|
|||
| For current functions (example, send-email-link, simple-email), the generated `index.ts`: | |||
| // In startJobs(), after creating jobsApp but before listenApp(): | |||
| this.registry.mountRoutes(jobsApp); | |||
| ``` | |||
|
|
|||
| ```typescript | |||
| import knativeJobExample from '@constructive-io/knative-job-example-fn'; | |||
| import sendEmailLink from '@constructive-io/send-email-link-fn'; | |||
| import simpleEmail from '@constructive-io/simple-email-fn'; | |||
| The callback server (port 8080) now also serves: | |||
| - `POST /functions/register` — `{ name, url }` → stores in registry | |||
| - `DELETE /functions/:name` → removes from registry | |||
| - `GET /functions` → lists all registered functions | |||
|
|
|||
| export const registry = { | |||
| 'knative-job-example': { app: knativeJobExample, defaultPort: 8081 }, | |||
| 'send-email-link': { app: sendEmailLink, defaultPort: 8082 }, | |||
| 'simple-email': { app: simpleEmail, defaultPort: 8083 } | |||
| } as const; | |||
| Existing `/callback` route is unaffected. | |||
|
|
|||
| export type FunctionName = keyof typeof registry; | |||
| #### Phase 1 files | |||
|
|
|||
| export default registry; | |||
| ``` | |||
| | Action | File | | |||
| |--------|------| | |||
| | Create | `job/service/src/registry.ts` | | |||
| | Modify | `job/service/src/index.ts` | | |||
|
|
|||
| --- | |||
|
|
|||
| **Note on naming**: Registry keys use `manifest.name` (from handler.json), not directory names. For `example/handler.json` with `"name": "knative-job-example"`, the key is `'knative-job-example'`. | |||
| ### Phase 2: Worker integration | |||
|
|
|||
| **Note on port assignment**: Ports are auto-assigned (8081 + index) in alphabetical order of function names. The existing `CONSTRUCTIVE_FUNCTION_PORTS` env var can override ports at runtime (`job/service/src/index.ts` lines 300-329). | |||
| Make the worker's URL resolution pluggable so it can use the dynamic registry. | |||
|
|
|||
| ### 3. Refactor `job/service/src/types.ts` | |||
| #### 2a. `job/worker/src/req.ts` — add optional URL resolver | |||
|
|
|||
| **Before** (line 1): | |||
| ```typescript | |||
| export type FunctionName = 'simple-email' | 'send-email-link'; | |||
| export type ResolveFunctionUrl = (fn: string) => string; | |||
|
|
|||
| interface RequestOptions { | |||
| body: unknown; | |||
| databaseId?: string; | |||
| workerId: string; | |||
| jobId: string | number; | |||
| resolveFunctionUrl?: ResolveFunctionUrl; | |||
| } | |||
|
|
|||
| const request = ( | |||
| fn: string, | |||
| { body, databaseId, workerId, jobId, resolveFunctionUrl }: RequestOptions | |||
| ) => { | |||
| const url = resolveFunctionUrl ? resolveFunctionUrl(fn) : getFunctionUrl(fn); | |||
| // ...rest unchanged | |||
| }; | |||
| ``` | |||
|
|
|||
| **After**: | |||
| #### 2b. `job/worker/src/index.ts` — Worker accepts resolver | |||
|
|
|||
| ```typescript | |||
| export type { FunctionName } from '@constructive-io/fn-registry'; | |||
| constructor({ tasks, idleDelay, pgPool, workerId, resolveFunctionUrl }: { | |||
| // ...existing options | |||
| resolveFunctionUrl?: ResolveFunctionUrl; | |||
| }) { | |||
| // ...existing | |||
| this.resolveFunctionUrl = resolveFunctionUrl; | |||
| } | |||
|
|
|||
| async doWork(job: JobRow) { | |||
| await req(task_identifier, { | |||
| body: payload, | |||
| databaseId: job.database_id, | |||
| workerId: this.workerId, | |||
| jobId: job.id, | |||
| resolveFunctionUrl: this.resolveFunctionUrl | |||
| }); | |||
| } | |||
| ``` | |||
|
|
|||
| All other types (`FunctionServiceConfig`, `FunctionsOptions`, `KnativeJobsSvcOptions`, etc.) remain unchanged — they reference `FunctionName` which is now re-exported. | |||
| #### 2c. Job-service passes resolver to Worker | |||
|
|
|||
| ### 4. Refactor `job/service/src/index.ts` | |||
| In `startJobs()`: | |||
|
|
|||
| **Remove** (lines 17, 46): | |||
| ```typescript | |||
| import { createRequire } from 'module'; | |||
| // ... | |||
| const requireFn = createRequire(__filename); | |||
| this.worker = new Worker({ | |||
| pgPool, | |||
| tasks, | |||
| workerId: getWorkerHostname(), | |||
| resolveFunctionUrl: (name: string) => this.registry.resolve(name) | |||
| }); | |||
| ``` | |||
|
|
|||
| **Remove** (lines 29-43): | |||
| ```typescript | |||
| type FunctionRegistryEntry = { ... }; | |||
| const functionRegistry: Record<FunctionName, FunctionRegistryEntry> = { ... }; | |||
| ``` | |||
| **Backwards-compatible**: If no resolver is provided, falls back to existing `getFunctionUrl()` (DEV_MAP / gateway env vars). | |||
|
|
|||
| **Remove** (lines 56-71): | |||
| ```typescript | |||
| const loadFunctionApp = (moduleName: string) => { ... }; | |||
| ``` | |||
| #### Phase 2 files | |||
|
|
|||
| **Add** (near top, after other imports): | |||
| ```typescript | |||
| import fnRegistry from '@constructive-io/fn-registry'; | |||
| import type { FunctionName } from '@constructive-io/fn-registry'; | |||
| ``` | |||
| | Action | File | | |||
| |--------|------| | |||
| | Modify | `job/worker/src/req.ts` | | |||
| | Modify | `job/worker/src/index.ts` | | |||
| | Modify | `job/service/src/index.ts` | | |||
|
|
|||
| --- | |||
|
|
|||
| ### Phase 3: Auto-registration in fn-runtime | |||
|
|
|||
| **Replace** `resolveFunctionEntry()` (line 48-54): | |||
| Add `startWithRegistration()` to fn-runtime so functions can self-register on startup. | |||
|
|
|||
| #### 3a. Create `packages/fn-runtime/src/register.ts` | |||
|
|
|||
| Uses Node's built-in `http` module (no new deps). Retries registration up to 10 times with backoff. Deregisters on SIGTERM/SIGINT. | |||
|
|
|||
| ```typescript | |||
| // Before: | |||
| const resolveFunctionEntry = (name: FunctionName): FunctionRegistryEntry => { | |||
| const entry = functionRegistry[name]; | |||
| if (!entry) throw new Error(`Unknown function "${name}".`); | |||
| return entry; | |||
| import http from 'http'; | |||
| import https from 'https'; | |||
| import { createLogger } from '@pgpmjs/logger'; | |||
|
|
|||
| const log = createLogger('fn-register'); | |||
|
|
|||
| const postJson = (url: string, data: object): Promise<void> => { /* Node http.request */ }; | |||
| const deleteReq = (url: string): Promise<void> => { /* Node http.request */ }; | |||
|
|
|||
| export type StartOptions = { | |||
| name: string; | |||
| port: number; | |||
| selfUrl?: string; | |||
| }; | |||
|
|
|||
| // After: | |||
| const resolveFunctionEntry = (name: FunctionName) => { | |||
| const entry = fnRegistry.registry[name]; | |||
| if (!entry) throw new Error(`Unknown function "${name}".`); | |||
| return entry; | |||
| export const startWithRegistration = async ( | |||
| app: { listen: (port: number, cb?: () => void) => unknown }, | |||
| options: StartOptions | |||
| ): Promise<void> => { | |||
| const { name, port } = options; | |||
| const registryUrl = process.env.FUNCTION_REGISTRY_URL; | |||
| const selfUrl = options.selfUrl | |||
| || process.env.FUNCTION_SELF_URL | |||
| || `http://localhost:${port}`; | |||
|
|
|||
| // 1. Start listening | |||
| await new Promise<void>((resolve) => { | |||
| app.listen(port, () => resolve()); | |||
| }); | |||
|
|
|||
| // 2. Register with retry (if FUNCTION_REGISTRY_URL is set) | |||
| if (registryUrl) { | |||
| let registered = false; | |||
| for (let i = 0; i < 10 && !registered; i++) { | |||
| try { | |||
| await postJson(registryUrl, { name, url: selfUrl }); | |||
| registered = true; | |||
| } catch { | |||
| await new Promise((r) => setTimeout(r, (i + 1) * 1000)); | |||
| } | |||
| } | |||
|
|
|||
| // 3. Deregister on graceful shutdown | |||
| const deregisterUrl = registryUrl.replace(/\/register$/, `/${name}`); | |||
| process.on('SIGTERM', async () => { await deleteReq(deregisterUrl); process.exit(0); }); | |||
| process.on('SIGINT', async () => { await deleteReq(deregisterUrl); process.exit(0); }); | |||
| } | |||
| }; | |||
| ``` | |||
|
|
|||
| **Replace** `startFunction()` (lines 109-134) — use `entry.app.listen()` directly instead of `loadFunctionApp()`: | |||
| #### 3b. Export from `packages/fn-runtime/src/index.ts` | |||
|
|
|||
| ```typescript | |||
| const startFunction = async ( | |||
| service: FunctionServiceConfig, | |||
| functionServers: Map<FunctionName, HttpServer> | |||
| ): Promise<StartedFunction> => { | |||
| const entry = resolveFunctionEntry(service.name); | |||
| const port = resolveFunctionPort(service); | |||
| const app = entry.app; | |||
|
|
|||
| await new Promise<void>((resolve, reject) => { | |||
| const server = app.listen(port, () => { | |||
| log.info(`function:${service.name} listening on ${port}`); | |||
| resolve(); | |||
| }) as HttpServer & { on?: (event: string, cb: (err: Error) => void) => void }; | |||
|
|
|||
| if (server?.on) { | |||
| server.on('error', (err) => { | |||
| log.error(`function:${service.name} failed to start`, err); | |||
| reject(err); | |||
| }); | |||
| } | |||
| export { startWithRegistration } from './register'; | |||
| export type { StartOptions } from './register'; | |||
| ``` | |||
|
|
|||
| functionServers.set(service.name, server); | |||
| }); | |||
| #### 3c. Update template (`templates/node-graphql/index.ts`) | |||
|
|
|||
| return { name: service.name, port }; | |||
| }; | |||
| ```typescript | |||
| import { createFunctionServer, startWithRegistration } from '@constructive-io/fn-runtime'; | |||
| import handler from './handler'; | |||
|
|
|||
| const app = createFunctionServer(handler, { name: '{{name}}' }); | |||
|
|
|||
| export default app; | |||
|
|
|||
| if (require.main === module) { | |||
| startWithRegistration(app, { | |||
| name: '{{name}}', | |||
| port: Number(process.env.PORT || 8080) | |||
| }); | |||
| } | |||
| ``` | |||
|
|
|||
| **Replace** `normalizeFunctionServices()` (lines 79-91) — use `fnRegistry.registry` instead of `functionRegistry`: | |||
| When imported as a module (e.g., tests), only the app is created. When run standalone, it listens + registers. | |||
|
|
|||
| #### Phase 3 env vars | |||
|
|
|||
| | Variable | Description | Example | | |||
| |----------|-------------|---------| | |||
| | `FUNCTION_REGISTRY_URL` | Registration endpoint on job-service | `http://job-service:8080/functions/register` | | |||
| | `FUNCTION_SELF_URL` | This function's externally-reachable URL | `http://simple-email:8080` | | |||
|
|
|||
| #### Phase 3 files | |||
|
|
|||
| | Action | File | | |||
| |--------|------| | |||
| | Create | `packages/fn-runtime/src/register.ts` | | |||
| | Modify | `packages/fn-runtime/src/index.ts` | | |||
| | Modify | `templates/node-graphql/index.ts` | | |||
|
|
|||
| --- | |||
|
|
|||
| ### Phase 4: Remove hardcoded registry + split docker-compose | |||
|
|
|||
| Remove the old hardcoded registry and in-process function loading. Each function becomes its own docker-compose service. | |||
|
|
|||
| #### 4a. Remove from `job/service/src/index.ts` | |||
|
|
|||
| - `import { createRequire } from 'module'` (line 17) | |||
| - `FunctionRegistryEntry` type + `functionRegistry` constant (lines 29-43) | |||
| - `const requireFn = createRequire(__filename)` (line 46) | |||
| - `resolveFunctionEntry()` (lines 48-54) | |||
| - `loadFunctionApp()` (lines 56-71) | |||
| - `shouldEnableFunctions()` (lines 73-77) | |||
| - `normalizeFunctionServices()` (lines 79-91) | |||
| - `resolveFunctionPort()`, `ensureUniquePorts()` (lines 93-107) | |||
| - `startFunction()`, `startFunctions()` (lines 109-151) | |||
| - `parseList()`, `parsePortMap()`, `buildFunctionsOptionsFromEnv()` (lines 292-354) | |||
|
|
|||
| #### 4b. Simplify types (`job/service/src/types.ts`) | |||
|
|
|||
| ```typescript | |||
| const normalizeFunctionServices = ( | |||
| options?: FunctionsOptions | |||
| ): FunctionServiceConfig[] => { | |||
| if (!shouldEnableFunctions(options)) return []; | |||
|
|
|||
| if (!options?.services?.length) { | |||
| return (Object.keys(fnRegistry.registry) as FunctionName[]).map((name) => ({ | |||
| name | |||
| })); | |||
| } | |||
| export type FunctionName = string; | |||
|
|
|||
| return options.services; | |||
| export type JobsOptions = { | |||
| enabled?: boolean; | |||
| }; | |||
| ``` | |||
|
|
|||
| **Replace** `resolveFunctionPort()` (lines 93-96): | |||
| export type KnativeJobsSvcOptions = { | |||
| jobs?: JobsOptions; | |||
| }; | |||
|
|
|||
| ```typescript | |||
| const resolveFunctionPort = (service: FunctionServiceConfig): number => { | |||
| const entry = resolveFunctionEntry(service.name); | |||
| return service.port ?? entry.defaultPort; | |||
| export type KnativeJobsSvcResult = { | |||
| jobs: boolean; | |||
| }; | |||
| ``` | |||
|
|
|||
| ### 5. Update `job/service/package.json` | |||
| Remove: `FunctionServiceConfig`, `FunctionsOptions`, `StartedFunction` | |||
|
|
|||
| **Add** dependency: | |||
| ```json | |||
| "@constructive-io/fn-registry": "workspace:^" | |||
| ``` | |||
| #### 4c. Update `job/service/package.json` | |||
|
|
|||
| **Remove** individual function deps (if present): | |||
| ```json | |||
| "@constructive-io/send-email-link-fn": "workspace:^", | |||
| Remove individual function deps (service no longer loads them): | |||
| ``` | |||
| "@constructive-io/send-email-link-fn": "workspace:^" | |||
| "@constructive-io/simple-email-fn": "workspace:^" | |||
| ``` | |||
|
|
|||
| The registry transitively depends on all function packages, so they'll still be installed. | |||
| #### 4d. Update `docker-compose.yml` | |||
|
|
|||
| Each function becomes its own service: | |||
|
|
|||
| ```yaml | |||
| services: | |||
| postgres: | |||
| # ...unchanged | |||
|
|
|||
| job-service: | |||
| build: | |||
| context: . | |||
| dockerfile: Dockerfile.dev | |||
| command: node job/service/dist/run.js | |||
| environment: | |||
| PGHOST: postgres | |||
| PGUSER: postgres | |||
| PGPASSWORD: password | |||
| PGDATABASE: launchql | |||
| CONSTRUCTIVE_JOBS_ENABLED: "true" | |||
| depends_on: | |||
| - postgres | |||
| ports: | |||
| - "8080:8080" | |||
|
|
|||
| simple-email: | |||
| build: | |||
| context: . | |||
| dockerfile: Dockerfile.dev | |||
| command: node generated/simple-email/dist/index.js | |||
| environment: | |||
| PORT: "8080" | |||
| FUNCTION_REGISTRY_URL: "http://job-service:8080/functions/register" | |||
| FUNCTION_SELF_URL: "http://simple-email:8080" | |||
| depends_on: | |||
| - job-service | |||
| ports: | |||
| - "8081:8080" | |||
|
|
|||
| send-email-link: | |||
| build: | |||
| context: . | |||
| dockerfile: Dockerfile.dev | |||
| command: node generated/send-email-link/dist/index.js | |||
| environment: | |||
| PORT: "8080" | |||
| FUNCTION_REGISTRY_URL: "http://job-service:8080/functions/register" | |||
| FUNCTION_SELF_URL: "http://send-email-link:8080" | |||
| GRAPHQL_URL: "http://api:5000/graphql" | |||
| depends_on: | |||
| - job-service | |||
| ports: | |||
| - "8082:8080" | |||
|
|
|||
| volumes: | |||
| pgdata: | |||
| ``` | |||
|
|
|||
| ## Files Summary | |||
| #### Phase 4 files | |||
|
|
|||
| | Action | File | | |||
| |--------|------| | |||
| | Modify | `scripts/generate.ts` — add `generateRegistry()`, call at end of `main()` | | |||
| | Modify | `job/service/src/index.ts` — import from registry, remove hardcoded registry + loadFunctionApp + createRequire | | |||
| | Modify | `job/service/src/types.ts` — re-export FunctionName from registry | | |||
| | Modify | `job/service/package.json` — add fn-registry dep, remove per-fn deps | | |||
| | Generated | `generated/registry/package.json` | | |||
| | Generated | `generated/registry/index.ts` | | |||
| | Generated | `generated/registry/tsconfig.json` | | |||
| | Modify | `job/service/src/index.ts` | | |||
| | Modify | `job/service/src/types.ts` | | |||
| | Modify | `job/service/package.json` | | |||
| | Modify | `docker-compose.yml` | | |||
|
|
|||
| --- | |||
|
|
|||
| ## Verification | |||
| ## Verification (end-to-end after all phases) | |||
|
|
|||
| ```bash | |||
| # 1. Clean generate | |||
| rm -rf generated/ | |||
| pnpm generate | |||
| # Verify: generated/registry/ exists with index.ts, package.json, tsconfig.json | |||
| cat generated/registry/index.ts | |||
| # Should show imports for all 3 functions | |||
|
|
|||
| # 2. Full build | |||
| pnpm install | |||
| pnpm build | |||
| # Should compile all packages including registry and job/service | |||
|
|
|||
| # 3. Test adding a new function | |||
| mkdir -p functions/test-fn | |||
| echo '{"name":"test-fn","version":"1.0.0","type":"node-graphql"}' > functions/test-fn/handler.json | |||
| cp functions/example/handler.ts functions/test-fn/handler.ts | |||
| pnpm generate | |||
| cat generated/registry/index.ts | |||
| # Should now include testFn import and registry entry | |||
|
|
|||
| # 4. Verify runtime (if docker compose is available) | |||
| make dev | |||
| # job-service should start all functions without errors | |||
| # 1. Build | |||
| pnpm generate && pnpm install && pnpm build | |||
|
|
|||
| # 2. Run tests | |||
| pnpm test:unit | |||
| pnpm test:integration | |||
|
|
|||
| # 3. Docker-compose test | |||
| docker-compose up --build | |||
|
|
|||
| # Verify registration: | |||
| curl http://localhost:8080/functions | |||
| # → [{ "name": "simple-email", ... }, { "name": "send-email-link", ... }] | |||
|
|
|||
| # Verify registration API: | |||
| curl -X POST http://localhost:8080/functions/register \ | |||
| -H 'Content-Type: application/json' \ | |||
| -d '{"name":"test-fn","url":"http://localhost:9999"}' | |||
| # → { "registered": true, "name": "test-fn", "url": "http://localhost:9999" } | |||
|
|
|||
| curl -X DELETE http://localhost:8080/functions/test-fn | |||
| # → { "removed": true, "name": "test-fn" } | |||
|
|
|||
| # 4. Verify backwards compat (Phase 2) | |||
| # Worker without resolveFunctionUrl still uses INTERNAL_GATEWAY_DEVELOPMENT_MAP | |||
| ``` | |||
There was a problem hiding this comment.
The PR title is "local testing" but the changes include major architectural documentation changes about a dynamic function registry system (Phase 1-4 in docs/plan/03-function-registry.md) that are not implemented in this PR. Only the test infrastructure and dev scripts are actually included. Either implement the documented changes or update the documentation to match what's actually in this PR.
| #### 3c. Update template (`templates/node-graphql/index.ts`) | ||
|
|
||
| return { name: service.name, port }; | ||
| }; | ||
| ```typescript | ||
| import { createFunctionServer, startWithRegistration } from '@constructive-io/fn-runtime'; | ||
| import handler from './handler'; | ||
|
|
||
| const app = createFunctionServer(handler, { name: '{{name}}' }); | ||
|
|
||
| export default app; | ||
|
|
||
| if (require.main === module) { | ||
| startWithRegistration(app, { | ||
| name: '{{name}}', | ||
| port: Number(process.env.PORT || 8080) | ||
| }); | ||
| } | ||
| ``` |
There was a problem hiding this comment.
The documentation describes adding startWithRegistration() to the template (Phase 3), but this PR doesn't implement that functionality. The template still uses the simple app.listen() call. This means functions won't self-register with the job-service registry as described in the documentation.
| const allProcesses: ProcessDef[] = [ | ||
| { | ||
| name: 'job-service', | ||
| script: path.resolve(ROOT, 'job/service/dist/run.js'), | ||
| port: 8080, | ||
| }, | ||
| { | ||
| name: 'simple-email', | ||
| script: path.resolve(ROOT, 'generated/simple-email/dist/index.js'), | ||
| port: 8081, | ||
| }, | ||
| { | ||
| name: 'send-email-link', | ||
| script: path.resolve(ROOT, 'generated/send-email-link/dist/index.js'), | ||
| port: 8082, | ||
| }, | ||
| ]; |
There was a problem hiding this comment.
The dev.ts script references 'generated/simple-email/dist/index.js' and 'generated/send-email-link/dist/index.js', but these generated files won't exist until after 'pnpm generate && pnpm build'. The script should check for file existence or provide a clear error message if the files are missing, otherwise users will get confusing Node.js module resolution errors.
No description provided.