From c9605b15940cb8ade289aba7779d697438f671ec Mon Sep 17 00:00:00 2001 From: Steven Borrelli Date: Wed, 30 Sep 2026 12:20:31 +0200 Subject: [PATCH] Make response helpers mutate and return nothing The response helpers were inconsistent about whether they mutate or return, and the signatures did not match the behaviour. Every helper that declared a return value returned the same object it was given -- none returned a new one -- so the type read like a transformation when it was a mutation. fatal declared RunFunctionResponse while its siblings normal and warning returned void, so it was not even a clean split between results helpers and setters. Settle it the way the Go and Python SDKs already have: to() builds a response, and every other helper that takes a response mutates it in place and returns void. A function author moving between the three SDKs now meets one contract rather than three. Nine helpers change signature: fatal, setDesiredComposedResources, setDesiredResources, setDesiredCompositeResource, setDesiredCompositeStatus, setContextKey, setOutput, requireSchema and requireResource. normal and warning already returned nothing; their signatures now say so explicitly rather than leaving it inferred. to(), update() and updateDesiredComposedResources() are unchanged. They build or transform resources rather than taking a response, so they keep their return values. This is breaking. TypeScript callers get a compile error on every `rsp = setX(...)` line, which makes the migration self-checking, but plain JavaScript callers get no error -- rsp silently becomes undefined. Bump to 0.8.0 and call that out in the release notes. Also fix the USAGE.md entry that described updateDesiredComposedResources as an alias for setDesiredComposedResources, when it takes a resource map rather than a response. Fixes #33 Co-Authored-By: Claude Opus 5 Signed-off-by: Steven Borrelli --- README.md | 26 ++-- USAGE.md | 14 ++- package.json | 2 +- src/response/response.test.ts | 227 ++++++++++++++++++++-------------- src/response/response.ts | 73 ++++------- 5 files changed, 181 insertions(+), 161 deletions(-) diff --git a/README.md b/README.md index 44e003e..84822f3 100644 --- a/README.md +++ b/README.md @@ -80,7 +80,7 @@ import { export class MyFunction implements FunctionHandler { async RunFunction(req: RunFunctionRequest, logger?: Logger): Promise { - let rsp = to(req); + const rsp = to(req); try { // Get observed composite resource and desired composed resources @@ -99,7 +99,7 @@ export class MyFunction implements FunctionHandler { } }); - rsp = setDesiredComposedResources(rsp, dcds); + setDesiredComposedResources(rsp, dcds); normal(rsp, "Function completed successfully"); return rsp; @@ -201,7 +201,7 @@ export class FunctionRunner { logger?: Logger, ): Promise { // Initialize response from request - let rsp = to(req); + const rsp = to(req); // Get desired composed resources from request let dcds = getDesiredComposedResources(req); @@ -223,7 +223,7 @@ export class FunctionRunner { }); // Set desired resources in response - rsp = setDesiredComposedResources(rsp, dcds); + setDesiredComposedResources(rsp, dcds); // Add a result message normal(rsp, "Resources created successfully"); @@ -355,22 +355,22 @@ import { } from "@crossplane-org/function-sdk-typescript"; // Initialize response from request (with optional TTL) -let rsp = to(req, DEFAULT_TTL); +const rsp = to(req, DEFAULT_TTL); // Set desired composed resources (merges with existing) -rsp = setDesiredComposedResources(rsp, dcds); +setDesiredComposedResources(rsp, dcds); // Set desired composite resource -rsp = setDesiredCompositeResource(rsp, dxr); +setDesiredCompositeResource(rsp, dxr); // Update composite resource status -rsp = setDesiredCompositeStatus({ rsp, status: { ready: true } }); +setDesiredCompositeStatus({ rsp, status: { ready: true } }); // Set context for next function -rsp = setContextKey(rsp, "my-key", "my-value"); +setContextKey(rsp, "my-key", "my-value"); // Set output (returned to user) -rsp = setOutput(rsp, { result: "success" }); +setOutput(rsp, { result: "success" }); // Add result messages normal(rsp, "Success message"); @@ -378,7 +378,7 @@ warning(rsp, "Warning message"); fatal(rsp, "Fatal error message"); // Request Crossplane fetch a resource (available in next invocation) -rsp = requireResource(rsp, "app-config", { +requireResource(rsp, "app-config", { apiVersion: "v1", kind: "ConfigMap", matchName: "my-config", @@ -386,7 +386,7 @@ rsp = requireResource(rsp, "app-config", { }); // Request Crossplane fetch a schema (available in next invocation) -rsp = requireSchema(rsp, "xr-schema", "example.org/v1", "MyResource"); +requireSchema(rsp, "xr-schema", "example.org/v1", "MyResource"); ``` #### Resource Helpers @@ -683,6 +683,8 @@ import { #### Response Functions +`to` builds a response. Every other helper below mutates the response it is given and returns nothing, matching the Go and Python SDKs -- call them as statements, not in an assignment. + - **`to(req, ttl?)`** - Initialize a response from a request - **`normal(rsp, message)`** - Add a normal (info) result - **`warning(rsp, message)`** - Add a warning result diff --git a/USAGE.md b/USAGE.md index 6dffcf0..3adc4e2 100644 --- a/USAGE.md +++ b/USAGE.md @@ -74,7 +74,7 @@ export class MyFunction implements FunctionHandler { logger?: Logger, ): Promise { // Initialize response from request - let rsp = to(req); + const rsp = to(req); try { // Get observed and desired state @@ -121,7 +121,7 @@ export class MyFunction implements FunctionHandler { }); // Update response with desired composed resources - rsp = setDesiredComposedResources(rsp, dcds); + setDesiredComposedResources(rsp, dcds); normal(rsp, "Function completed successfully"); return rsp; @@ -263,7 +263,7 @@ You can update the status of the composite resource: ```typescript import { setDesiredCompositeStatus } from "@crossplane-org/function-sdk-typescript"; -rsp = setDesiredCompositeStatus({ +setDesiredCompositeStatus({ rsp, status: { ready: true, @@ -286,8 +286,8 @@ if (exists) { } // Set context for next function -rsp = setContextKey(rsp, "resourceId", "my-resource-123"); -rsp = setContextKey(rsp, "status", { created: true, ready: false }); +setContextKey(rsp, "resourceId", "my-resource-123"); +setContextKey(rsp, "status", { created: true, ready: false }); ``` ### Working with Credentials @@ -351,9 +351,11 @@ normal(rsp, "Function completed successfully"); ### Response Helpers +`to` builds a response. Every other helper below mutates the response it is given and returns nothing, matching the Go and Python SDKs -- call them as statements, not in an assignment. + - `to(req, ttl?)` - Initialize response from request (optional TTL in seconds, defaults to 60) - `setDesiredComposedResources(rsp, resources)` - Set composed resources (merges with existing) -- `updateDesiredComposedResources(rsp, resources)` - Alias for `setDesiredComposedResources` +- `updateDesiredComposedResources(cds, namedResource)` - Add a named resource to a map of composed resources, returning the map. Operates on the map, not the response - `setDesiredCompositeResource(rsp, resource)` - Set the desired composite resource - `setDesiredCompositeStatus({ rsp, status })` - Update composite status - `setContextKey(rsp, key, value)` - Set context for next function in pipeline diff --git a/package.json b/package.json index 9a1c507..c8870ca 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@crossplane-org/function-sdk-typescript", - "version": "0.7.0", + "version": "0.8.0", "description": "A Crossplane Function SDK for Typescript", "keywords": [ "crossplane", diff --git a/src/response/response.test.ts b/src/response/response.test.ts index e39c21b..04c0761 100644 --- a/src/response/response.test.ts +++ b/src/response/response.test.ts @@ -1,12 +1,20 @@ import { describe, it, expect } from 'vitest'; import { + fatal, + normal, + setContextKey, + setDesiredComposedResources, + setDesiredCompositeResource, setDesiredCompositeStatus, setDesiredResources, + setOutput, requireSchema, requireResource, + to, + warning, } from './response.js'; import type { RunFunctionResponse } from '../proto/run_function.js'; -import { Ready } from '../proto/run_function.js'; +import { Ready, RunFunctionRequest } from '../proto/run_function.js'; describe('setDesiredCompositeStatus', () => { it('should set status when desired.composite.resource exists', () => { @@ -37,11 +45,11 @@ describe('setDesiredCompositeStatus', () => { conditions: [{ type: 'Synced', status: 'True' }], }; - const result = setDesiredCompositeStatus({ rsp, status }); + setDesiredCompositeStatus({ rsp, status }); - expect(result.desired?.composite?.resource?.status).toEqual(status); - expect(result.desired?.composite?.resource?.apiVersion).toBe('example.org/v1'); - expect(result.desired?.composite?.resource?.kind).toBe('XR'); + expect(rsp.desired?.composite?.resource?.status).toEqual(status); + expect(rsp.desired?.composite?.resource?.apiVersion).toBe('example.org/v1'); + expect(rsp.desired?.composite?.resource?.kind).toBe('XR'); }); it('should set status when desired.composite.resource is undefined', () => { @@ -66,10 +74,10 @@ describe('setDesiredCompositeStatus', () => { message: 'Waiting for resources', }; - const result = setDesiredCompositeStatus({ rsp, status }); + setDesiredCompositeStatus({ rsp, status }); - expect(result.desired?.composite?.resource?.status).toEqual(status); - expect(result.desired?.composite?.resource).toBeDefined(); + expect(rsp.desired?.composite?.resource?.status).toEqual(status); + expect(rsp.desired?.composite?.resource).toBeDefined(); }); it('should set status when desired.composite is undefined', () => { @@ -93,11 +101,11 @@ describe('setDesiredCompositeStatus', () => { ], }; - const result = setDesiredCompositeStatus({ rsp, status }); + setDesiredCompositeStatus({ rsp, status }); - expect(result.desired?.composite?.resource?.status).toEqual(status); - expect(result.desired?.composite).toBeDefined(); - expect(result.desired?.composite?.resource).toBeDefined(); + expect(rsp.desired?.composite?.resource?.status).toEqual(status); + expect(rsp.desired?.composite).toBeDefined(); + expect(rsp.desired?.composite?.resource).toBeDefined(); }); it('should set status when desired is undefined', () => { @@ -115,12 +123,12 @@ describe('setDesiredCompositeStatus', () => { ready: false, }; - const result = setDesiredCompositeStatus({ rsp, status }); + setDesiredCompositeStatus({ rsp, status }); - expect(result.desired).toBeDefined(); - expect(result.desired?.composite).toBeDefined(); - expect(result.desired?.composite?.resource).toBeDefined(); - expect(result.desired?.composite?.resource?.status).toEqual(status); + expect(rsp.desired).toBeDefined(); + expect(rsp.desired?.composite).toBeDefined(); + expect(rsp.desired?.composite?.resource).toBeDefined(); + expect(rsp.desired?.composite?.resource?.status).toEqual(status); }); it('should merge status with existing status fields', () => { @@ -155,12 +163,12 @@ describe('setDesiredCompositeStatus', () => { conditions: [{ type: 'NewCondition', status: 'True' }], }; - const result = setDesiredCompositeStatus({ rsp, status }); + setDesiredCompositeStatus({ rsp, status }); - expect(result.desired?.composite?.resource?.status?.existingField).toBe('preserved'); - expect(result.desired?.composite?.resource?.status?.phase).toBe('Ready'); + expect(rsp.desired?.composite?.resource?.status?.existingField).toBe('preserved'); + expect(rsp.desired?.composite?.resource?.status?.phase).toBe('Ready'); // Note: merge will combine the arrays - expect(result.desired?.composite?.resource?.status?.conditions).toHaveLength(2); + expect(rsp.desired?.composite?.resource?.status?.conditions).toHaveLength(2); }); it('should handle complex nested status objects', () => { @@ -196,11 +204,11 @@ describe('setDesiredCompositeStatus', () => { }, }; - const result = setDesiredCompositeStatus({ rsp, status }); + setDesiredCompositeStatus({ rsp, status }); - expect(result.desired?.composite?.resource?.status).toEqual(status); + expect(rsp.desired?.composite?.resource?.status).toEqual(status); expect( - result.desired?.composite?.resource?.status?.atProvider?.connectionPool?.maxConnections + rsp.desired?.composite?.resource?.status?.atProvider?.connectionPool?.maxConnections ).toBe(100); }); @@ -241,23 +249,23 @@ describe('setDesiredCompositeStatus', () => { phase: 'Ready', }; - const result = setDesiredCompositeStatus({ rsp, status }); + setDesiredCompositeStatus({ rsp, status }); // Verify status was set - expect(result.desired?.composite?.resource?.status).toEqual(status); + expect(rsp.desired?.composite?.resource?.status).toEqual(status); // Verify other fields preserved - expect(result.desired?.composite?.resource?.apiVersion).toBe('example.org/v1'); - expect(result.desired?.composite?.resource?.kind).toBe('XR'); - expect(result.desired?.composite?.resource?.metadata?.name).toBe('test-xr'); - expect(result.desired?.composite?.resource?.metadata?.namespace).toBe('production'); - expect(result.desired?.composite?.resource?.metadata?.labels?.app).toBe('myapp'); - expect(result.desired?.composite?.resource?.spec?.region).toBe('us-west-2'); - expect(result.desired?.composite?.resource?.spec?.replicas).toBe(3); + expect(rsp.desired?.composite?.resource?.apiVersion).toBe('example.org/v1'); + expect(rsp.desired?.composite?.resource?.kind).toBe('XR'); + expect(rsp.desired?.composite?.resource?.metadata?.name).toBe('test-xr'); + expect(rsp.desired?.composite?.resource?.metadata?.namespace).toBe('production'); + expect(rsp.desired?.composite?.resource?.metadata?.labels?.app).toBe('myapp'); + expect(rsp.desired?.composite?.resource?.spec?.region).toBe('us-west-2'); + expect(rsp.desired?.composite?.resource?.spec?.replicas).toBe(3); // Verify connection details and ready status preserved - expect(result.desired?.composite?.connectionDetails?.password).toEqual(Buffer.from('secret')); - expect(result.desired?.composite?.ready).toBe(Ready.READY_TRUE); + expect(rsp.desired?.composite?.connectionDetails?.password).toEqual(Buffer.from('secret')); + expect(rsp.desired?.composite?.ready).toBe(Ready.READY_TRUE); }); }); @@ -290,13 +298,13 @@ describe('setDesiredResources', () => { }, }; - const result = setDesiredResources(rsp, resources); + setDesiredResources(rsp, resources); - expect(result.desired?.resources).toBeDefined(); - expect(Object.keys(result.desired?.resources || {})).toHaveLength(2); - expect(result.desired?.resources?.['my-bucket']?.resource?.kind).toBe('Bucket'); - expect(result.desired?.resources?.['my-db']?.resource?.kind).toBe('Instance'); - expect(result.desired?.resources?.['my-bucket']?.resource?.spec?.forProvider?.region).toBe( + expect(rsp.desired?.resources).toBeDefined(); + expect(Object.keys(rsp.desired?.resources || {})).toHaveLength(2); + expect(rsp.desired?.resources?.['my-bucket']?.resource?.kind).toBe('Bucket'); + expect(rsp.desired?.resources?.['my-db']?.resource?.kind).toBe('Instance'); + expect(rsp.desired?.resources?.['my-bucket']?.resource?.spec?.forProvider?.region).toBe( 'us-west-2' ); }); @@ -320,12 +328,12 @@ describe('setDesiredResources', () => { }, }; - const result = setDesiredResources(rsp, resources); + setDesiredResources(rsp, resources); - expect(result.desired).toBeDefined(); - expect(result.desired?.resources).toBeDefined(); - expect(result.desired?.resources?.['my-resource']?.resource?.kind).toBe('ConfigMap'); - expect(result.desired?.resources?.['my-resource']?.resource?.data?.key).toBe('value'); + expect(rsp.desired).toBeDefined(); + expect(rsp.desired?.resources).toBeDefined(); + expect(rsp.desired?.resources?.['my-resource']?.resource?.kind).toBe('ConfigMap'); + expect(rsp.desired?.resources?.['my-resource']?.resource?.data?.key).toBe('value'); }); it('should merge with existing resources', () => { @@ -359,11 +367,11 @@ describe('setDesiredResources', () => { }, }; - const result = setDesiredResources(rsp, resources); + setDesiredResources(rsp, resources); - expect(Object.keys(result.desired?.resources || {})).toHaveLength(2); - expect(result.desired?.resources?.['existing-resource']?.resource?.kind).toBe('Secret'); - expect(result.desired?.resources?.['new-resource']?.resource?.kind).toBe('ConfigMap'); + expect(Object.keys(rsp.desired?.resources || {})).toHaveLength(2); + expect(rsp.desired?.resources?.['existing-resource']?.resource?.kind).toBe('Secret'); + expect(rsp.desired?.resources?.['new-resource']?.resource?.kind).toBe('ConfigMap'); }); it('should handle complex nested resource structures', () => { @@ -415,12 +423,12 @@ describe('setDesiredResources', () => { }, }; - const result = setDesiredResources(rsp, resources); + setDesiredResources(rsp, resources); - expect(result.desired?.resources?.['complex-resource']?.resource?.kind).toBe('Deployment'); - expect(result.desired?.resources?.['complex-resource']?.resource?.spec?.replicas).toBe(3); + expect(rsp.desired?.resources?.['complex-resource']?.resource?.kind).toBe('Deployment'); + expect(rsp.desired?.resources?.['complex-resource']?.resource?.spec?.replicas).toBe(3); expect( - result.desired?.resources?.['complex-resource']?.resource?.spec?.template?.spec?.containers[0] + rsp.desired?.resources?.['complex-resource']?.resource?.spec?.template?.spec?.containers[0] ?.name ).toBe('app'); }); @@ -438,10 +446,10 @@ describe('setDesiredResources', () => { results: [], }; - const result = setDesiredResources(rsp, {}); + setDesiredResources(rsp, {}); - expect(result.desired?.resources).toBeDefined(); - expect(Object.keys(result.desired?.resources || {})).toHaveLength(0); + expect(rsp.desired?.resources).toBeDefined(); + expect(Object.keys(rsp.desired?.resources || {})).toHaveLength(0); }); }); @@ -456,11 +464,11 @@ describe('requireSchema', () => { results: [], }; - const result = requireSchema(rsp, 'xr-schema', 'example.org/v1', 'MyResource'); + requireSchema(rsp, 'xr-schema', 'example.org/v1', 'MyResource'); - expect(result.requirements).toBeDefined(); - expect(result.requirements?.schemas).toBeDefined(); - expect(result.requirements?.schemas?.['xr-schema']).toEqual({ + expect(rsp.requirements).toBeDefined(); + expect(rsp.requirements?.schemas).toBeDefined(); + expect(rsp.requirements?.schemas?.['xr-schema']).toEqual({ apiVersion: 'example.org/v1', kind: 'MyResource', }); @@ -486,10 +494,10 @@ describe('requireSchema', () => { results: [], }; - const result = requireSchema(rsp, 'composite-schema', 'database.example.org/v1', 'Database'); + requireSchema(rsp, 'composite-schema', 'database.example.org/v1', 'Database'); - expect(result.requirements?.resources?.['existing-resource']).toBeDefined(); - expect(result.requirements?.schemas?.['composite-schema']).toEqual({ + expect(rsp.requirements?.resources?.['existing-resource']).toBeDefined(); + expect(rsp.requirements?.schemas?.['composite-schema']).toEqual({ apiVersion: 'database.example.org/v1', kind: 'Database', }); @@ -505,14 +513,14 @@ describe('requireSchema', () => { results: [], }; - let result = requireSchema(rsp, 'xr-schema', 'example.org/v1', 'XR'); - result = requireSchema(result, 'composed-schema', 'example.org/v1', 'ComposedResource'); - result = requireSchema(result, 'claim-schema', 'example.org/v1', 'Claim'); + requireSchema(rsp, 'xr-schema', 'example.org/v1', 'XR'); + requireSchema(rsp, 'composed-schema', 'example.org/v1', 'ComposedResource'); + requireSchema(rsp, 'claim-schema', 'example.org/v1', 'Claim'); - expect(Object.keys(result.requirements?.schemas || {})).toHaveLength(3); - expect(result.requirements?.schemas?.['xr-schema']?.kind).toBe('XR'); - expect(result.requirements?.schemas?.['composed-schema']?.kind).toBe('ComposedResource'); - expect(result.requirements?.schemas?.['claim-schema']?.kind).toBe('Claim'); + expect(Object.keys(rsp.requirements?.schemas || {})).toHaveLength(3); + expect(rsp.requirements?.schemas?.['xr-schema']?.kind).toBe('XR'); + expect(rsp.requirements?.schemas?.['composed-schema']?.kind).toBe('ComposedResource'); + expect(rsp.requirements?.schemas?.['claim-schema']?.kind).toBe('Claim'); }); it('should overwrite existing schema requirement with same name', () => { @@ -534,9 +542,9 @@ describe('requireSchema', () => { results: [], }; - const result = requireSchema(rsp, 'my-schema', 'new.example.org/v2', 'NewKind'); + requireSchema(rsp, 'my-schema', 'new.example.org/v2', 'NewKind'); - expect(result.requirements?.schemas?.['my-schema']).toEqual({ + expect(rsp.requirements?.schemas?.['my-schema']).toEqual({ apiVersion: 'new.example.org/v2', kind: 'NewKind', }); @@ -554,16 +562,16 @@ describe('requireResource', () => { results: [], }; - const result = requireResource(rsp, 'app-config', { + requireResource(rsp, 'app-config', { apiVersion: 'v1', kind: 'ConfigMap', matchName: 'my-app-config', namespace: 'production', }); - expect(result.requirements).toBeDefined(); - expect(result.requirements?.resources).toBeDefined(); - expect(result.requirements?.resources?.['app-config']).toEqual({ + expect(rsp.requirements).toBeDefined(); + expect(rsp.requirements?.resources).toBeDefined(); + expect(rsp.requirements?.resources?.['app-config']).toEqual({ apiVersion: 'v1', kind: 'ConfigMap', matchName: 'my-app-config', @@ -581,7 +589,7 @@ describe('requireResource', () => { results: [], }; - const result = requireResource(rsp, 'db-secrets', { + requireResource(rsp, 'db-secrets', { apiVersion: 'v1', kind: 'Secret', matchLabels: { @@ -593,7 +601,7 @@ describe('requireResource', () => { namespace: 'production', }); - expect(result.requirements?.resources?.['db-secrets']).toEqual({ + expect(rsp.requirements?.resources?.['db-secrets']).toEqual({ apiVersion: 'v1', kind: 'Secret', matchLabels: { @@ -616,21 +624,21 @@ describe('requireResource', () => { results: [], }; - let result = requireResource(rsp, 'config', { + requireResource(rsp, 'config', { apiVersion: 'v1', kind: 'ConfigMap', matchName: 'app-config', }); - result = requireResource(result, 'secret', { + requireResource(rsp, 'secret', { apiVersion: 'v1', kind: 'Secret', matchName: 'app-secret', }); - expect(Object.keys(result.requirements?.resources || {})).toHaveLength(2); - expect(result.requirements?.resources?.['config']?.kind).toBe('ConfigMap'); - expect(result.requirements?.resources?.['secret']?.kind).toBe('Secret'); + expect(Object.keys(rsp.requirements?.resources || {})).toHaveLength(2); + expect(rsp.requirements?.resources?.['config']?.kind).toBe('ConfigMap'); + expect(rsp.requirements?.resources?.['secret']?.kind).toBe('Secret'); }); it('should add resource requirement when schemas already exist', () => { @@ -652,7 +660,7 @@ describe('requireResource', () => { results: [], }; - const result = requireResource(rsp, 'namespaces', { + requireResource(rsp, 'namespaces', { apiVersion: 'v1', kind: 'Namespace', matchLabels: { @@ -662,8 +670,8 @@ describe('requireResource', () => { }, }); - expect(result.requirements?.schemas?.['existing-schema']).toBeDefined(); - expect(result.requirements?.resources?.['namespaces']).toEqual({ + expect(rsp.requirements?.schemas?.['existing-schema']).toBeDefined(); + expect(rsp.requirements?.resources?.['namespaces']).toEqual({ apiVersion: 'v1', kind: 'Namespace', matchLabels: { @@ -684,7 +692,7 @@ describe('requireResource', () => { results: [], }; - const result = requireResource(rsp, 'all-namespaces', { + requireResource(rsp, 'all-namespaces', { apiVersion: 'v1', kind: 'Namespace', matchLabels: { @@ -694,7 +702,7 @@ describe('requireResource', () => { }, }); - expect(result.requirements?.resources?.['all-namespaces']?.namespace).toBeUndefined(); + expect(rsp.requirements?.resources?.['all-namespaces']?.namespace).toBeUndefined(); }); it('should overwrite existing resource requirement with same name', () => { @@ -717,16 +725,53 @@ describe('requireResource', () => { results: [], }; - const result = requireResource(rsp, 'my-resource', { + requireResource(rsp, 'my-resource', { apiVersion: 'v1', kind: 'Secret', matchName: 'new-secret', }); - expect(result.requirements?.resources?.['my-resource']).toEqual({ + expect(rsp.requirements?.resources?.['my-resource']).toEqual({ apiVersion: 'v1', kind: 'Secret', matchName: 'new-secret', }); }); }); + +describe('response helper return contract', () => { + // Every helper that takes a response mutates it in place and returns nothing, + // matching the Go and Python SDKs. Pinned here so a helper cannot quietly go + // back to returning the response and reintroduce the ambiguity of issue #33. + it('returns undefined from every response mutator', () => { + const rsp = to(RunFunctionRequest.fromJSON({})); + + expect(fatal(rsp, 'm')).toBeUndefined(); + expect(normal(rsp, 'm')).toBeUndefined(); + expect(warning(rsp, 'm')).toBeUndefined(); + expect(setDesiredComposedResources(rsp, {})).toBeUndefined(); + expect(setDesiredResources(rsp, {})).toBeUndefined(); + expect( + setDesiredCompositeResource(rsp, { + resource: {}, + connectionDetails: {}, + ready: Ready.READY_UNSPECIFIED, + }) + ).toBeUndefined(); + expect(setDesiredCompositeStatus({ rsp, status: {} })).toBeUndefined(); + expect(setContextKey(rsp, 'k', 'v')).toBeUndefined(); + expect(setOutput(rsp, {})).toBeUndefined(); + expect(requireSchema(rsp, 'n', 'example.org/v1', 'Kind')).toBeUndefined(); + expect(requireResource(rsp, 'n', { apiVersion: 'v1', kind: 'ConfigMap' })).toBeUndefined(); + }); + + it('applies the mutations to the response it was given', () => { + const rsp = to(RunFunctionRequest.fromJSON({})); + + normal(rsp, 'created'); + setContextKey(rsp, 'endpoint', 'db.example.com:5432'); + + expect(rsp.results).toHaveLength(1); + expect(rsp.context?.['endpoint']).toBe('db.example.com:5432'); + }); +}); diff --git a/src/response/response.ts b/src/response/response.ts index a470236..cca1b60 100644 --- a/src/response/response.ts +++ b/src/response/response.ts @@ -98,7 +98,7 @@ type NamedResource = { * name: "my-bucket", * resource: Resource.fromJSON({ resource: bucketConfig }) * }); - * rsp = setDesiredComposedResources(rsp, dcds); + * setDesiredComposedResources(rsp, dcds); * ``` */ export function updateDesiredComposedResources( @@ -119,7 +119,6 @@ export function updateDesiredComposedResources( * * @param rsp - The RunFunctionResponse to add the result to * @param message - The error message describing the fatal condition - * @returns The updated response * * @example * ```typescript @@ -129,14 +128,13 @@ export function updateDesiredComposedResources( * } * ``` */ -export function fatal(rsp: RunFunctionResponse, message: string): RunFunctionResponse { +export function fatal(rsp: RunFunctionResponse, message: string): void { if (rsp && rsp.results) { rsp.results.push({ severity: Severity.SEVERITY_FATAL, message: message, }); } - return rsp; } /** @@ -154,7 +152,7 @@ export function fatal(rsp: RunFunctionResponse, message: string): RunFunctionRes * normal(rsp, "Successfully configured 3 database replicas"); * ``` */ -export function normal(rsp: RunFunctionResponse, message: string) { +export function normal(rsp: RunFunctionResponse, message: string): void { if (rsp && rsp.results) { rsp.results.push({ severity: Severity.SEVERITY_NORMAL, @@ -180,7 +178,7 @@ export function normal(rsp: RunFunctionResponse, message: string) { * } * ``` */ -export function warning(rsp: RunFunctionResponse, message: string) { +export function warning(rsp: RunFunctionResponse, message: string): void { if (rsp && rsp.results) { rsp.results.push({ severity: Severity.SEVERITY_WARNING, @@ -201,7 +199,6 @@ export function warning(rsp: RunFunctionResponse, message: string) { * * @param rsp - The RunFunctionResponse to update * @param dcds - A map of resource names to Resource objects to set as desired - * @returns The updated response * * @example * ```typescript @@ -209,13 +206,13 @@ export function warning(rsp: RunFunctionResponse, message: string) { * dcds["my-deployment"] = Resource.fromJSON({ * resource: { apiVersion: "apps/v1", kind: "Deployment", ... } * }); - * rsp = setDesiredComposedResources(rsp, dcds); + * setDesiredComposedResources(rsp, dcds); * ``` */ export function setDesiredComposedResources( rsp: RunFunctionResponse, dcds: { [key: string]: Resource } -): RunFunctionResponse { +): void { // Ensure desired state exists if (!rsp.desired) { rsp.desired = { composite: undefined, resources: {} }; @@ -225,8 +222,6 @@ export function setDesiredComposedResources( rsp.desired.resources = merge(rsp.desired.resources || {}, dcds) as { [key: string]: Resource; }; - - return rsp; } /** @@ -242,11 +237,10 @@ export function setDesiredComposedResources( * * @param rsp - The RunFunctionResponse to update * @param resources - A map of resource names to unstructured Kubernetes objects - * @returns The updated response * * @example * ```typescript - * rsp = setDesiredResources(rsp, { + * setDesiredResources(rsp, { * "my-bucket": { * apiVersion: "s3.aws.upbound.io/v1beta1", * kind: "Bucket", @@ -265,7 +259,7 @@ export function setDesiredComposedResources( export function setDesiredResources( rsp: RunFunctionResponse, resources: Record> -): RunFunctionResponse { +): void { // Ensure desired state exists if (!rsp.desired) { rsp.desired = { composite: undefined, resources: {} }; @@ -281,8 +275,6 @@ export function setDesiredResources( rsp.desired.resources = merge(rsp.desired.resources || {}, convertedResources) as { [key: string]: Resource; }; - - return rsp; } /** @@ -320,11 +312,10 @@ export function update(src: Resource, tgt: Resource): Resource { * @param params - Object containing the response and status to set * @param params.rsp - The RunFunctionResponse to update * @param params.status - The status object to merge into the composite resource - * @returns The updated response * * @example * ```typescript - * rsp = setDesiredCompositeStatus({ + * setDesiredCompositeStatus({ * rsp, * status: { * phase: "Ready", @@ -339,7 +330,7 @@ export function setDesiredCompositeStatus({ }: { rsp: RunFunctionResponse; status: Record; -}): RunFunctionResponse { +}): void { // Ensure desired state exists if (!rsp.desired) { rsp.desired = { composite: undefined, resources: {} }; @@ -359,8 +350,6 @@ export function setDesiredCompositeStatus({ rsp.desired.composite.resource = merge(rsp.desired.composite.resource, { status: status, }); - - return rsp; } /** @@ -373,25 +362,19 @@ export function setDesiredCompositeStatus({ * @param rsp - The RunFunctionResponse to update * @param key - The context key to set * @param value - The value to associate with the key (can be any JSON-serializable value) - * @returns The updated response * * @example * ```typescript * // Set context for next function in pipeline - * rsp = setContextKey(rsp, "database-endpoint", "db.example.com:5432"); - * rsp = setContextKey(rsp, "connection-config", { host: "db.example.com", port: 5432 }); + * setContextKey(rsp, "database-endpoint", "db.example.com:5432"); + * setContextKey(rsp, "connection-config", { host: "db.example.com", port: 5432 }); * ``` */ -export function setContextKey( - rsp: RunFunctionResponse, - key: string, - value: unknown -): RunFunctionResponse { +export function setContextKey(rsp: RunFunctionResponse, key: string, value: unknown): void { if (!rsp.context) { rsp.context = {}; } rsp.context[key] = value; - return rsp; } /** @@ -406,14 +389,13 @@ export function setContextKey( * @param rsp - The RunFunctionResponse to update * @param resource - The desired composite resource to set * @param ready - Optional ready status (READY_TRUE, READY_FALSE, or READY_UNSPECIFIED) - * @returns The updated response * * @example * ```typescript * const composite = getObservedCompositeResource(req); * if (composite) { * // Modify and set as desired with ready status - * rsp = setDesiredCompositeResource(rsp, composite, Ready.READY_TRUE); + * setDesiredCompositeResource(rsp, composite, Ready.READY_TRUE); * } * ``` */ @@ -421,7 +403,7 @@ export function setDesiredCompositeResource( rsp: RunFunctionResponse, resource: Resource, ready?: Ready -): RunFunctionResponse { +): void { if (!rsp.desired) { rsp.desired = { composite: undefined, resources: {} }; } @@ -433,8 +415,6 @@ export function setDesiredCompositeResource( connectionDetails: resource.connectionDetails, ready: ready !== undefined ? ready : Ready.READY_UNSPECIFIED, }); - - return rsp; } /** @@ -446,24 +426,19 @@ export function setDesiredCompositeResource( * * @param rsp - The RunFunctionResponse to update * @param output - The output object to set (must be JSON-serializable) - * @returns The updated response * * @example * ```typescript * // For operation functions - * rsp = setOutput(rsp, { + * setOutput(rsp, { * resourcesCreated: 5, * status: "success", * details: { timestamp: new Date().toISOString() } * }); * ``` */ -export function setOutput( - rsp: RunFunctionResponse, - output: Record -): RunFunctionResponse { +export function setOutput(rsp: RunFunctionResponse, output: Record): void { rsp.output = output; - return rsp; } /** @@ -481,12 +456,11 @@ export function setOutput( * @param name - A unique name to identify this schema requirement * @param apiVersion - API version of the resource kind (e.g., "example.org/v1") * @param kind - Kind of resource (e.g., "MyResource") - * @returns The updated response * * @example * ```typescript * // Request the OpenAPI schema for an XR type - * rsp = requireSchema(rsp, "xr-schema", "example.org/v1", "MyResource"); + * requireSchema(rsp, "xr-schema", "example.org/v1", "MyResource"); * * // In the next function invocation, retrieve the schema: * const [schema, ok] = getRequiredSchema(req, "xr-schema"); @@ -500,7 +474,7 @@ export function requireSchema( name: string, apiVersion: string, kind: string -): RunFunctionResponse { +): void { if (!rsp.requirements) { rsp.requirements = { extraResources: {}, @@ -515,7 +489,6 @@ export function requireSchema( apiVersion, kind, }; - return rsp; } /** @@ -532,12 +505,11 @@ export function requireSchema( * @param rsp - The RunFunctionResponse to update * @param name - A unique name to identify this resource requirement * @param selector - The resource selector specifying which resources to fetch - * @returns The updated response * * @example * ```typescript * // Match a specific ConfigMap by name - * rsp = requireResource(rsp, "app-config", { + * requireResource(rsp, "app-config", { * apiVersion: "v1", * kind: "ConfigMap", * matchName: "my-app-config", @@ -545,7 +517,7 @@ export function requireSchema( * }); * * // Match all Secrets with specific labels - * rsp = requireResource(rsp, "db-secrets", { + * requireResource(rsp, "db-secrets", { * apiVersion: "v1", * kind: "Secret", * matchLabels: { @@ -568,7 +540,7 @@ export function requireResource( rsp: RunFunctionResponse, name: string, selector: ResourceSelector -): RunFunctionResponse { +): void { if (!rsp.requirements) { rsp.requirements = { extraResources: {}, @@ -580,5 +552,4 @@ export function requireResource( rsp.requirements.resources = {}; } rsp.requirements.resources[name] = selector; - return rsp; }