Skip to content

Commit

Permalink
engine: separate configuration for retrieval of form definition, atta…
Browse files Browse the repository at this point in the history
…chments

**IMPORTANT NOTES!**

1. This change deprecates the former, more general, `fetchResource` config. While conceptually suitable for the purpose it was originally designed (and for this API expansion), it seems likely to be a footgun in real world use.

2. This change **intentionally deviates** from the design direction which we previously settled in #201. It is, instead, intended to be directly responsive to the discussion which followed that previously settled design.

- - -

**Why another `fetch`-like configuration?**

Taking the above important notes in reverse order:

_Intentional deviation_

The discussion in #201 ultimately convinced me that it would likely be a mistake to favor any Map-like interface for the purpose of resolving form attachments. There is too much ambiguity and baggage around the current mechanisms for _satisfying_ the interface.

Ultimately, @brontolosone’s axiom—that a complete `jr:` URL is a particular form attachment’s identifier—**is correct**! Any truncation of them is a convenient shorthand which would align with known data sources.

Paradoxically, in typical usage, clients ultimately **MUST** extrapolate those identifiers from more limited data: the source data itself will _only_ contain truncated shorthand values.

We’d discussed supplying a parse-stage summary of the `jr:` URLs referenced by a form, allowing clients to populate a mapping before proceding with post-parse initialization. But the logic a client would implement to satisfy that mapping would be **identical** to the logic it would implement to satisfy a `fetch`-like function: given an expected `jr://foo/bar.ext`, and given a URL to an attachment named `bar.ext`, provide access to that URL.

It seems more sensible to specify this API as a function rather than as a Map-like data structure, if for no other reason than to avoid introducing or further entrenching a particular shorthand interpretation. We will call a client’s `fetchFormAttachment` with each attachment’s complete identifier (the full `jr:` URL), and the client can satisfy that with whatever data it has available and whatever mapping logic it sees fit _at call time_, without codifying that shorthand into the engine’s logic.

_Distinct `fetch`-like configurations_

A single `fetchResource` configuration would meet the minimum API requisite API surface area to retrieve form definitions _and their attachments_, but it would be considerably more error prone for clients to integrate:

- `fetchResource` has up to this point been optional, and is typically ignored in existing client usage
- a single `fetchResource` config would need to serve dual purposes: resolve a form definition by URL (which is likely to be a real, network-accessible URL) and resolve form attachments by identifier (which happens to be a valid URL, but will never resolve to a real network resource except if clients implement the same resolution logic with a Service Worker or other network IO-intercepting mechanism)
- a client would need to determine the purpose of a given call _by inference_ (by inspecting the URL, and probably branching on a `jr:` prefix); the `fetch`-like API simply wouldn’t accommodate a more explicit distinction without **also overloading** HTTP semantics (e.g. by callling `fetchResource` with some special header)

- - -

It is also important to note that `fetchFormAttachment` is also optional, at least for now. The main reasons for this are detailed on the interface property’s JSDoc, but it also bears mentioning that this (along with keeping `fetchResource` as an alias to `fetchFormDefinition`) allows all currently supported functionality to keep working without any breaking changes.
  • Loading branch information
eyelidlessness committed Nov 25, 2024
1 parent 4fa77ba commit 9b5c195
Show file tree
Hide file tree
Showing 4 changed files with 75 additions and 27 deletions.
78 changes: 58 additions & 20 deletions packages/xforms-engine/src/client/EngineConfig.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import type { JRResourceURL } from '@getodk/common/jr-resources/JRResourceURL.ts';
import type { initializeForm } from '../instance/index.ts';
import type { OpaqueReactiveObjectFactory } from './OpaqueReactiveObjectFactory.ts';

/**
Expand Down Expand Up @@ -29,7 +31,13 @@ export interface FetchResourceResponse {
* actual resources (likely, but not necessarily, accessed at a corresponding
* HTTP URL).
*/
export type FetchResource = (resource: URL) => Promise<FetchResourceResponse>;
export type FetchResource<Resource extends URL = URL> = (
resource: Resource
) => Promise<FetchResourceResponse>;

export type FormAttachmentURL = JRResourceURL;

export type FetchFormAttachment = FetchResource<FormAttachmentURL>;

/**
* Options provided by a client to specify certain aspects of engine runtime
Expand All @@ -55,29 +63,59 @@ export interface EngineConfig {
readonly stateFactory?: OpaqueReactiveObjectFactory;

/**
* A client may specify a generic function for retrieving resources referenced
* by a form, such as:
*
* - Form definitions themselves (if not provided directly to the engine by
* the client)
* - External secondary instances
* - Media (images, audio, video, etc.)
* A client may specify an arbitrary {@link fetch}-like function for retrieving an XML XForm form
* definition.
*
* The function is expected to be a subset of the
* {@link https://developer.mozilla.org/en-US/docs/Web/API/Fetch_API | Fetch API},
* performing `GET` requests for:
* Notes:
*
* - Text resources (e.g. XML, CSV, JSON/GeoJSON)
* - Binary resources (e.g. media)
* - Optionally streamed binary data of either (e.g. for optimized
* presentation of audio/video)
* - This configuration will only be consuled for calls to
* {@link initializeForm} with a URL referencing an XML XForm definition. It
* will be ignored for calls passing an XML XForm form definition directly.
*
* If provided by a client, this function will be used by the engine to
* retrieve any such resource, as required for engine functionality. If
* absent, the engine will use the native `fetch` function (if available, a
* polyfill otherwise). Clients may use this function to provide resources
* from sources other than the network, (or even in a test client to provide
* e.g. resources from test fixtures).
* - For calls to {@link initializeForm} with a URL, if this configuration is
* not specified it will default to the global {@link fetch} function (if
* one is defined).
*/
readonly fetchFormDefinition?: FetchResource;

/**
* @deprecated
* @alias fetchFormDefinition
*/
readonly fetchResource?: FetchResource;

/**
* A client may specify an arbitrary {@link fetch}-like function to retrieve a
* form's attachments, i.e. any `jr:` URL referenced by the form (as specified
* by {@link https://getodk.github.io/xforms-spec/ | ODK XForms}).
*
* Notes:
*
* - This configuration will be consulted for all supported form attachments,
* as a part of {@link initializeForm | form initialization}.
*
* - If this configuration is not specified it will default to the global
* {@link fetch} function (if one is defined).
*
* This default behavior will typically result in failure to load form
* attachments—and in most cases this will also cause
* {@link initializeForm | form initialization} to fail overall—with the
* following exceptions:
*
* - **CLIENT-SPECIFIC:** Usage in coordination with a client-implemented
* {@link https://developer.mozilla.org/en-US/docs/Web/API/Service_Worker_API | Serivce Worker},
* which can intercept network requests **generally**. Clients already using
* a Service Worker may opt for the convenience of handling network requests
* for `jr:` URLs along with any other network interception logic. Client
* implementors should be warned, however, that such `jr:` URLs are not
* namespaced or otherwise scoped to a particular form; such a client would
* therefore inherently need to coordinate state between the Service Worker
* and the main thread (or whatever other realm calls
* {@link initializeForm}).
*
* - **PENDING:** Any usage where the engine does not require access to a
* form's attachments.
*/
readonly fetchFormAttachment?: FetchFormAttachment;
}
12 changes: 8 additions & 4 deletions packages/xforms-engine/src/instance/index.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { identity } from '@getodk/common/lib/identity.ts';
import { getOwner } from 'solid-js';
import type { EngineConfig } from '../client/EngineConfig.ts';
import type { RootNode } from '../client/RootNode.ts';
import type {
InitializeFormOptions as BaseInitializeFormOptions,
Expand All @@ -17,10 +18,11 @@ interface InitializeFormOptions extends BaseInitializeFormOptions {
readonly config: Partial<InstanceConfig>;
}

const buildInstanceConfig = (options: Partial<InstanceConfig> = {}): InstanceConfig => {
const buildInstanceConfig = (options: EngineConfig = {}): InstanceConfig => {
return {
createUniqueId: options.createUniqueId ?? createUniqueId,
fetchResource: options.fetchResource ?? fetch,
createUniqueId,
fetchFormDefinition: options.fetchFormDefinition ?? options.fetchResource ?? fetch,
fetchFormAttachment: options.fetchFormAttachment ?? fetch,
stateFactory: options.stateFactory ?? identity,
};
};
Expand All @@ -32,7 +34,9 @@ export const initializeForm = async (
const owner = getOwner();
const scope = createReactiveScope({ owner });
const engineConfig = buildInstanceConfig(options.config);
const sourceXML = await retrieveSourceXMLResource(input, engineConfig);
const sourceXML = await retrieveSourceXMLResource(input, {
fetchResource: engineConfig.fetchFormDefinition,
});
const form = new XFormDefinition(sourceXML);
const primaryInstance = new PrimaryInstance(scope, form.model, engineConfig);

Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
import type { EngineConfig } from '../../client/EngineConfig.ts';
import type { FetchFormAttachment, FetchResource } from '../../client/EngineConfig.ts';
import type { OpaqueReactiveObjectFactory } from '../../client/OpaqueReactiveObjectFactory.ts';
import type { CreateUniqueId } from '../../lib/unique-id.ts';

export interface InstanceConfig extends Required<EngineConfig> {
export interface InstanceConfig {
readonly stateFactory: OpaqueReactiveObjectFactory;
readonly fetchFormDefinition: FetchResource;
readonly fetchFormAttachment: FetchFormAttachment;

/**
* Uniqueness per form instance session (so e.g. persistence isn't necessary).
*/
Expand Down
3 changes: 2 additions & 1 deletion packages/xforms-engine/test/instance/PrimaryInstance.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,8 @@ describe('PrimaryInstance engine representation of instance state', () => {
return scope.runTask(() => {
return new PrimaryInstance(scope, xformDefinition.model, {
createUniqueId,
fetchResource,
fetchFormAttachment: fetchResource,
fetchFormDefinition: fetchResource,
stateFactory,
});
});
Expand Down

0 comments on commit 9b5c195

Please sign in to comment.