Skip to content

Commit

Permalink
engine: missing resource behavior error by default, blank by config
Browse files Browse the repository at this point in the history
  • Loading branch information
eyelidlessness committed Nov 27, 2024
1 parent 6b1f145 commit 7222c30
Show file tree
Hide file tree
Showing 10 changed files with 248 additions and 53 deletions.
3 changes: 3 additions & 0 deletions packages/scenario/src/client/init.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import type {
import { initializeForm } from '@getodk/xforms-engine';
import type { Owner } from 'solid-js';
import { createRoot, getOwner, runWithOwner } from 'solid-js';
import type { MissingResourceBehavior } from '../../../xforms-engine/dist/client/constants';
import { FormDefinitionResource } from '../jr/resource/FormDefinitionResource.ts';

/**
Expand Down Expand Up @@ -55,6 +56,7 @@ const fetchResourceStub: typeof fetch = () => {

export interface InitializeTestFormOptions {
readonly resourceService: JRResourceService;
readonly missingResourceBehavior: MissingResourceBehavior;
readonly stateFactory: OpaqueReactiveObjectFactory;
}

Expand Down Expand Up @@ -85,6 +87,7 @@ export const initializeTestForm = async (
config: {
...defaultConfig,
fetchFormAttachment: options.resourceService.handleRequest,
missingResourceBehavior: options.missingResourceBehavior,
stateFactory: options.stateFactory,
},
});
Expand Down
4 changes: 4 additions & 0 deletions packages/scenario/src/jr/Scenario.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import type {
SubmissionOptions,
SubmissionResult,
} from '@getodk/xforms-engine';
import { constants as ENGINE_CONSTANTS } from '@getodk/xforms-engine';
import type { Accessor, Setter } from 'solid-js';
import { createMemo, createSignal, runWithOwner } from 'solid-js';
import { afterEach, expect } from 'vitest';
Expand Down Expand Up @@ -156,6 +157,9 @@ export class Scenario {
): InitializeTestFormOptions {
return {
resourceService: overrideOptions?.resourceService ?? SharedJRResourceService.init(),
missingResourceBehavior:
overrideOptions?.missingResourceBehavior ??
ENGINE_CONSTANTS.MISSING_RESOURCE_BEHAVIOR.DEFAULT,
stateFactory: overrideOptions?.stateFactory ?? nonReactiveIdentityStateFactory,
};
}
Expand Down
129 changes: 96 additions & 33 deletions packages/scenario/test/secondary-instances.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
title,
} from '@getodk/common/test/fixtures/xform-dsl/index.ts';
import type { PartiallyKnownString } from '@getodk/common/types/string/PartiallyKnownString.ts';
import { constants as ENGINE_CONSTANTS } from '@getodk/xforms-engine';
import { afterEach, beforeEach, describe, expect, it } from 'vitest';
import { stringAnswer } from '../src/answer/ExpectedStringAnswer.ts';
import { Scenario } from '../src/jr/Scenario.ts';
Expand Down Expand Up @@ -673,11 +674,64 @@ describe('Secondary instances', () => {
*/
it.todo('dummyNodesInExternalInstanceDeclaration_ShouldBeIgnored');

/**
* **PORTING NOTES**
*
* This sub-suite has been updated to reflect different semantics and expectations for missing external secondary instances between JavaRosa and Web Forms:
*
* - By default, Web Forms will fail to initialize a form when any of the external secondary instances are missing (i.e. with HTTP 404 semantics).
*
* - By optional configuration, Web Forms may ignore missing external secondary instances, treating them as blank.
*/
describe('//region Missing external file', () => {
it('[uses an] empty placeholder [~~]is used[~~] when [referenced] external instance [is] not found', async () => {
// JR: emptyPlaceholderInstanceIsUsed_whenExternalInstanceNotFound
it.fails(
'[uses an] empty placeholder [~~]is used[~~] when [referenced] external instance [is] not found',
async () => {
configureReferenceManagerIncorrectly();

const scenario = await Scenario.init('external-select-csv.xml');

expect(scenario.choicesOf('/data/first').size()).toBe(0);
}
);

/**
* **PORTING NOTES**
*
* Supplemental, exercises configured override of default missing resource
* behavior.
*/
it('uses an empty/blank placeholder when not found, and when overriding configuration is specified', async () => {
configureReferenceManagerIncorrectly();

const scenario = await Scenario.init('external-select-csv.xml');
const scenario = await Scenario.init(
'Missing resource treated as blank',
// prettier-ignore
html(
head(
title('Missing resource treated as blank'),
model(
mainInstance(
t('data id="missing-resource-treated-as-blank"',
t('first'))),

t('instance id="external-csv" src="jr://file-csv/missing.csv"'),

bind('/data/first').type('string')
)
),
body(
select1Dynamic(
'/data/first',
"instance('external-csv')/root/item"
)
)
),
{
missingResourceBehavior: ENGINE_CONSTANTS.MISSING_RESOURCE_BEHAVIOR.BLANK,
}
);

expect(scenario.choicesOf('/data/first').size()).toBe(0);
});
Expand Down Expand Up @@ -723,6 +777,42 @@ describe('Secondary instances', () => {
)
);

const activateFixtures = () => {
resourceService.activateFixtures(fixturesDirectory, ['file', 'file-csv']);
};

const activateInlineResources = () => {
resourceService.activateResource(
{
url: xmlAttachmentURL,
fileName: xmlAttachmentFileName,
mimeType: 'text/xml',
},
// prettier-ignore
t('instance-root',
t('instance-item',
t('item-label', 'A'),
t('item-value', 'a')),
t('instance-item',
t('item-label', 'B'),
t('item-value', 'b'))).asXml()
);
resourceService.activateResource(
{
url: csvAttachmentURL,
fileName: csvAttachmentFileName,
mimeType: 'text/csv',
},
// prettier-ignore
[
'item-label,item-value',
'Y,y',
'Z,z',
'\n\r\n'
].join('\n')
);
};

let fixturesDirectory: string;
let resourceService: JRResourceService;

Expand All @@ -747,7 +837,7 @@ describe('Secondary instances', () => {
});

it('supports external secondary instances (XML, file system fixture)', async () => {
resourceService.activateFixtures(fixturesDirectory, ['file']);
activateFixtures();

const scenario = await Scenario.init(formTitle, formDefinition, {
resourceService,
Expand All @@ -763,21 +853,7 @@ describe('Secondary instances', () => {
// worth considering if we want to expand external secondary instance
// support and/or test coverage.
it('supports external secondary instances (XML, inline fixture)', async () => {
resourceService.activateResource(
{
url: xmlAttachmentURL,
fileName: xmlAttachmentFileName,
mimeType: 'text/xml',
},
// prettier-ignore
t('instance-root',
t('instance-item',
t('item-label', 'A'),
t('item-value', 'a')),
t('instance-item',
t('item-label', 'B'),
t('item-value', 'b'))).asXml()
);
activateInlineResources();

const scenario = await Scenario.init(formTitle, formDefinition, {
resourceService,
Expand All @@ -789,7 +865,7 @@ describe('Secondary instances', () => {
});

it('supports external secondary instances (CSV, file system fixture)', async () => {
resourceService.activateFixtures(fixturesDirectory, ['file']);
activateFixtures();

const scenario = await Scenario.init(formTitle, formDefinition, {
resourceService,
Expand All @@ -801,20 +877,7 @@ describe('Secondary instances', () => {
});

it('supports external secondary instances (CSV, inline fixture)', async () => {
resourceService.activateResource(
{
url: csvAttachmentURL,
fileName: csvAttachmentFileName,
mimeType: 'text/csv',
},
// prettier-ignore
[
'item-label,item-value',
'Y,y',
'Z,z',
'\n\r\n'
].join('\n')
);
activateInlineResources();

const scenario = await Scenario.init(formTitle, formDefinition, {
resourceService,
Expand Down
12 changes: 9 additions & 3 deletions packages/xforms-engine/src/client/EngineConfig.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import type { initializeForm } from '../instance/index.ts';
import type { MissingResourceBehavior, MissingResourceBehaviorDefault } from './constants.ts';
import type { OpaqueReactiveObjectFactory } from './OpaqueReactiveObjectFactory.ts';
import type { FetchFormAttachment, FetchResource } from './resources.ts';

Expand Down Expand Up @@ -76,9 +77,14 @@ export interface EngineConfig {
* 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;

/**
* @see {@link MissingResourceBehavior}
* @see {@link MissingResourceBehaviorDefault}
*
* @default MissingResourceBehaviorDefault
*/
readonly missingResourceBehavior?: MissingResourceBehavior;
}
61 changes: 61 additions & 0 deletions packages/xforms-engine/src/client/constants.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,66 @@
import type { PrimaryInstance } from '../instance/PrimaryInstance.ts';
import type { InitializeForm } from './index.ts';
import type { ValidationTextRole } from './TextRange.ts';

export const MISSING_RESOURCE_BEHAVIOR = {
/**
* When this behavior is configured, {@link InitializeForm | initializing} a
* {@link PrimaryInstance} for a form which references any **missing**
* resources will fail, producing an error to the calling client.
*
* @see {@link MissingResourceBehavior}
*/
ERROR: 'ERROR',

/**
* When this behavior is configured, {@link InitializeForm | initializing} a
* {@link PrimaryInstance} for a form which references any **missing**
* resources will succeed (producing a warning).
*
* Such missing resources will be parsed as if they are blank, as appropriate
* for the resource's XForm semantic usage and/or format.
*
* @see {@link MissingResourceBehavior}
*/
BLANK: 'BLANK',

/**
* @see {@link MISSING_RESOURCE_BEHAVIOR.ERROR}
*/
get DEFAULT(): 'ERROR' {
return MISSING_RESOURCE_BEHAVIOR.ERROR;
},
} as const;

export type MissingResourceBehaviorError = typeof MISSING_RESOURCE_BEHAVIOR.ERROR;

export type MissingResourceBehaviorBlank = typeof MISSING_RESOURCE_BEHAVIOR.BLANK;

export type MissingResourceBehaviorDefault = typeof MISSING_RESOURCE_BEHAVIOR.DEFAULT;

/**
* Specifies behavior for {@link InitializeForm | initializing} a form's
* {@link PrimaryInstance} which references any **missing** resources.
*
* Here the term "missing" is consistent with
* {@link https://www.rfc-editor.org/rfc/rfc9110#status.404 | HTTP 404 status}
* semantics. Clients which provide access to form attachments by performing
* HTTP network requests (e.g. with {@link fetch}) can generally convey this
* semantic meaning with a standard {@link Response}.
*
* **IMPORTANT**
*
* The term "missing" is distinct from other network/IO failures, e.g. when
* network access itself is unavailable. In these cases, the engine will
* consider a resource's availability **ambiguous**, producing an error
* regardless of the configured behavior for **missing** resources.
*/
// prettier-ignore
export type MissingResourceBehavior =
// eslint-disable-next-line @typescript-eslint/sort-type-constituents
| MissingResourceBehaviorError
| MissingResourceBehaviorBlank;

export const VALIDATION_TEXT = {
constraintMsg: 'Condition not satisfied: constraint',
requiredMsg: 'Condition not satisfied: required',
Expand Down
3 changes: 3 additions & 0 deletions packages/xforms-engine/src/instance/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ 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 { MISSING_RESOURCE_BEHAVIOR } from '../client/constants.ts';
import type {
InitializeFormOptions as BaseInitializeFormOptions,
FormResource,
Expand All @@ -25,6 +26,7 @@ const buildInstanceConfig = (options: EngineConfig = {}): InstanceConfig => {
createUniqueId,
fetchFormDefinition: options.fetchFormDefinition ?? options.fetchResource ?? fetch,
fetchFormAttachment: options.fetchFormAttachment ?? fetch,
missingResourceBehavior: options.missingResourceBehavior ?? MISSING_RESOURCE_BEHAVIOR.DEFAULT,
stateFactory: options.stateFactory ?? identity,
};
};
Expand All @@ -42,6 +44,7 @@ export const initializeForm = async (
const xformDOM = XFormDOM.from(sourceXML);
const secondaryInstances = await SecondaryInstancesDefinition.load(xformDOM, {
fetchResource: engineConfig.fetchFormAttachment,
missingResourceBehavior: engineConfig.missingResourceBehavior,
});
const form = new XFormDefinition(xformDOM);
const primaryInstance = new PrimaryInstance(scope, form.model, secondaryInstances, engineConfig);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import type { MissingResourceBehavior } from '../../client/constants.ts';
import type { OpaqueReactiveObjectFactory } from '../../client/OpaqueReactiveObjectFactory.ts';
import type { FetchFormAttachment, FetchResource } from '../../client/resources.ts';
import type { CreateUniqueId } from '../../lib/unique-id.ts';
Expand All @@ -6,6 +7,7 @@ export interface InstanceConfig {
readonly stateFactory: OpaqueReactiveObjectFactory;
readonly fetchFormDefinition: FetchResource;
readonly fetchFormAttachment: FetchFormAttachment;
readonly missingResourceBehavior: MissingResourceBehavior;

/**
* Uniqueness per form instance session (so e.g. persistence isn't necessary).
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import type { XFormDOM } from '../../XFormDOM.ts';
import { SecondaryInstanceRootDefinition } from './SecondaryInstanceRootDefinition.ts';
import { BlankSecondaryInstanceSource } from './sources/BlankSecondaryInstanceSource.ts';
import { CSVExternalSecondaryInstanceSource } from './sources/CSVExternalSecondaryInstance.ts';
import type { ExternalSecondaryInstanceResourceOptions } from './sources/ExternalSecondaryInstanceResource.ts';
import type { ExternalSecondaryInstanceResourceLoadOptions } from './sources/ExternalSecondaryInstanceResource.ts';
import { ExternalSecondaryInstanceResource } from './sources/ExternalSecondaryInstanceResource.ts';
import { GeoJSONExternalSecondaryInstanceSource } from './sources/GeoJSONExternalSecondaryInstance.ts';
import { InternalSecondaryInstanceSource } from './sources/InternalSecondaryInstanceSource.ts';
Expand Down Expand Up @@ -41,7 +41,7 @@ export class SecondaryInstancesDefinition

static async load(
xformDOM: XFormDOM,
options: ExternalSecondaryInstanceResourceOptions
options: ExternalSecondaryInstanceResourceLoadOptions
): Promise<SecondaryInstancesDefinition> {
const { secondaryInstanceElements } = xformDOM;

Expand All @@ -67,7 +67,7 @@ export class SecondaryInstancesDefinition
options
);

if (resource == null) {
if (resource.isBlank) {
return new BlankSecondaryInstanceSource(instanceId, resourceURL, domElement);
}

Expand Down
Loading

0 comments on commit 7222c30

Please sign in to comment.