From b5cfc2a793f22da60606a4dd7387fcf3d3163843 Mon Sep 17 00:00:00 2001 From: "(quasar) nebula" Date: Mon, 25 Sep 2023 14:23:23 -0300 Subject: data: misc. improvements for input validation & infrastructure --- src/data/things/composite.js | 254 ++++++++++++++++++++++++++++-------------- src/data/things/thing.js | 72 +++++++----- src/data/things/track.js | 17 +-- src/data/things/validators.js | 58 +++++++++- src/util/sugar.js | 7 +- 5 files changed, 279 insertions(+), 129 deletions(-) (limited to 'src') diff --git a/src/data/things/composite.js b/src/data/things/composite.js index 1148687c..0f943ec3 100644 --- a/src/data/things/composite.js +++ b/src/data/things/composite.js @@ -5,7 +5,6 @@ import {TupleMap} from '#wiki-data'; import { is, - isArray, isString, isWholeNumber, validateArrayItems, @@ -18,6 +17,7 @@ import { openAggregate, stitchArrays, unique, + withAggregate, } from '#sugar'; // Composes multiple compositional "steps" and a "base" to form a property @@ -443,13 +443,53 @@ function getStaticInputMetadata(inputOptions) { return metadata; } -export function templateCompositeFrom(description) { - const compositeName = +function getCompositionName(description) { + return ( (description.annotation ? description.annotation - : `unnamed composite`); + : `unnamed composite`)); +} + +function validateInputValue(value, description) { + const tokenValue = getInputTokenValue(description); + + const {acceptsNull, defaultValue, type, validate} = tokenValue || {}; - const descriptionAggregate = openAggregate({message: `Errors in description for ${compositeName}`}); + if (value === null || value === undefined) { + if (acceptsNull || defaultValue === null) { + return true; + } else { + throw new TypeError( + (type + ? `Expected ${type}, got ${value}` + : `Expected value, got ${value}`)); + } + } + + if (type) { + // Note: null is already handled earlier in this function, so it won't + // cause any trouble here. + const typeofValue = + (typeof value === 'object' + ? Array.isArray(value) ? 'array' : 'object' + : typeof value); + + if (typeofValue !== type) { + throw new TypeError(`Expected ${type}, got ${typeofValue}`); + } + } + + if (validate) { + validate(value); + } + + return true; +} + +export function templateCompositeFrom(description) { + const compositionName = getCompositionName(description); + + const descriptionAggregate = openAggregate({message: `Errors in description for ${compositionName}`}); if ('steps' in description) { if (Array.isArray(description.steps)) { @@ -469,7 +509,7 @@ export function templateCompositeFrom(description) { break validateInputs; } - descriptionAggregate.nest({message: `Errors in input descriptions for ${compositeName}`}, ({push}) => { + descriptionAggregate.nest({message: `Errors in static input descriptions for ${compositionName}`}, ({push}) => { const missingCallsToInput = []; const wrongCallsToInput = []; @@ -515,7 +555,7 @@ export function templateCompositeFrom(description) { throw new Error(`${value}: Expected "#" at start`); } }), - {message: `Errors in output descriptions for ${compositeName}`}); + {message: `Errors in output descriptions for ${compositionName}`}); } } @@ -527,7 +567,7 @@ export function templateCompositeFrom(description) { : []); const instantiate = (inputOptions = {}) => { - const inputOptionsAggregate = openAggregate({message: `Errors in input options passed to ${compositeName}`}); + const inputOptionsAggregate = openAggregate({message: `Errors in input options passed to ${compositionName}`}); const providedInputNames = Object.keys(inputOptions); @@ -543,7 +583,6 @@ export function templateCompositeFrom(description) { if (!inputDescription) return true; if ('defaultValue' in inputDescription) return false; if ('defaultDependency' in inputDescription) return false; - if (inputDescription.null === true) return false; return true; }); @@ -655,7 +694,7 @@ export function templateCompositeFrom(description) { symbol: templateCompositeFrom.symbol, outputs(providedOptions) { - const outputOptionsAggregate = openAggregate({message: `Errors in output options passed to ${compositeName}`}); + const outputOptionsAggregate = openAggregate({message: `Errors in output options passed to ${compositionName}`}); const misplacedOutputNames = []; const wrongTypeOutputNames = []; @@ -718,28 +757,27 @@ export function templateCompositeFrom(description) { } if ('inputs' in description) { - const finalInputs = {}; - - for (const [name, description_] of Object.entries(description.inputs)) { - const description = getInputTokenValue(description_); - const tokenShape = getInputTokenShape(description_); + const inputMapping = {}; + for (const [name, token] of Object.entries(description.inputs)) { + const tokenValue = getInputTokenValue(token); if (name in inputOptions) { if (typeof inputOptions[name] === 'string') { - finalInputs[name] = input.dependency(inputOptions[name]); + inputMapping[name] = input.dependency(inputOptions[name]); } else { - finalInputs[name] = inputOptions[name]; + inputMapping[name] = inputOptions[name]; } - } else if (description.defaultValue) { - finalInputs[name] = input.value(description.defaultValue); - } else if (description.defaultDependency) { - finalInputs[name] = input.dependency(description.defaultDependency); + } else if (tokenValue.defaultValue) { + inputMapping[name] = input.value(tokenValue.defaultValue); + } else if (tokenValue.defaultDependency) { + inputMapping[name] = input.dependency(tokenValue.defaultDependency); } else { - finalInputs[name] = input.value(null); + inputMapping[name] = input.value(null); } } - finalDescription.inputs = finalInputs; + finalDescription.inputMapping = inputMapping; + finalDescription.inputDescriptions = description.inputs; } if ('outputs' in description) { @@ -768,7 +806,7 @@ export function templateCompositeFrom(description) { const finalDescription = {...ownDescription}; - const aggregate = openAggregate({message: `Errors resolving ${compositeName}`}); + const aggregate = openAggregate({message: `Errors resolving ${compositionName}`}); const steps = ownDescription.steps(); @@ -804,6 +842,7 @@ export const noTransformSymbol = Symbol.for('compositeFrom: no-transform symbol' export function compositeFrom(description) { const {annotation} = description; + const compositionName = getCompositionName(description); const debug = fn => { if (compositeFrom.debug === true) { @@ -835,7 +874,7 @@ export function compositeFrom(description) { ? compositeFrom(step.toResolvedComposition()) : step)); - const inputMetadata = getStaticInputMetadata(description.inputs ?? {}); + const inputMetadata = getStaticInputMetadata(description.inputMapping ?? {}); function _mapDependenciesToOutputs(providedDependencies) { if (!description.outputs) { @@ -861,7 +900,7 @@ export function compositeFrom(description) { // nested inside, so input('name')-shaped tokens are going to be evaluated // in the context of the containing composition. const dependenciesFromInputs = - Object.values(description.inputs ?? {}) + Object.values(description.inputMapping ?? {}) .map(token => { const tokenShape = getInputTokenShape(token); const tokenValue = getInputTokenValue(token); @@ -884,10 +923,41 @@ export function compositeFrom(description) { .filter(dependency => isInputToken(dependency)) .some(token => getInputTokenShape(token) === 'input.updateValue'); + const inputNames = + Object.keys(description.inputMapping ?? {}); + + const inputSymbols = + inputNames.map(name => input(name)); + + const inputsMayBeDynamicValue = + stitchArrays({ + mappingToken: Object.values(description.inputMapping ?? {}), + descriptionToken: Object.values(description.inputDescriptions ?? {}), + }).map(({mappingToken, descriptionToken}) => { + if (getInputTokenShape(descriptionToken) === 'input.staticValue') return false; + if (getInputTokenShape(mappingToken) === 'input.value') return false; + return true; + }); + + const inputDescriptions = + Object.values(description.inputDescriptions ?? {}); + + /* + const inputsAcceptNull = + Object.values(description.inputDescriptions ?? {}) + .map(token => { + const tokenValue = getInputTokenValue(token); + if (!tokenValue) return false; + if ('acceptsNull' in tokenValue) return tokenValue.acceptsNull; + if ('defaultValue' in tokenValue) return tokenValue.defaultValue === null; + return false; + }); + */ + // Update descriptions passed as the value in an input.updateValue() token, // as provided as inputs for this composition. const inputUpdateDescriptions = - Object.values(description.inputs ?? {}) + Object.values(description.inputMapping ?? {}) .map(token => (getInputTokenShape(token) === 'input.updateValue' ? getInputTokenValue(token) @@ -903,7 +973,6 @@ export function compositeFrom(description) { (annotation ? ` (${annotation})` : ''), }); - // TODO: Check description.compose ?? true instead. const compositionNests = description.compose ?? true; const exposeDependencies = new Set(); @@ -1141,30 +1210,44 @@ export function compositeFrom(description) { const availableDependencies = {...initialDependencies}; const inputValues = - ('inputs' in description - ? Object.fromEntries(Object.entries(description.inputs) - .map(([name, token]) => { - const tokenShape = getInputTokenShape(token); - const tokenValue = getInputTokenValue(token); - switch (tokenShape) { - case 'input.dependency': - return [input(name), initialDependencies[tokenValue]]; - case 'input.value': - return [input(name), tokenValue]; - case 'input.updateValue': - if (!expectingTransform) { - throw new Error(`Unexpected input.updateValue() accessed on non-transform call`); - } - return [input(name), valueSoFar]; - case 'input.myself': - return [input(name), initialDependencies['this']]; - case 'input': - return [input(name), initialDependencies[token]]; - default: - throw new TypeError(`Unexpected input shape ${tokenShape}`); - } - })) - : {}); + Object.values(description.inputMapping ?? {}) + .map(token => { + const tokenShape = getInputTokenShape(token); + const tokenValue = getInputTokenValue(token); + switch (tokenShape) { + case 'input.dependency': + return initialDependencies[tokenValue]; + case 'input.value': + return tokenValue; + case 'input.updateValue': + if (!expectingTransform) + throw new Error(`Unexpected input.updateValue() accessed on non-transform call`); + return valueSoFar; + case 'input.myself': + return initialDependencies['this']; + case 'input': + return initialDependencies[token]; + default: + throw new TypeError(`Unexpected input shape ${tokenShape}`); + } + }); + + withAggregate({message: `Errors in dynamic input values provided to ${compositionName}`}, ({push}) => { + for (const {dynamic, name, value, description} of stitchArrays({ + dynamic: inputsMayBeDynamicValue, + name: inputNames, + value: inputValues, + description: inputDescriptions, + })) { + if (!dynamic) continue; + try { + validateInputValue(value, description); + } catch (error) { + error.message = `${name}: ${error.message}`; + throw error; + } + } + }); if (expectingTransform) { debug(() => [colors.bright(`begin composition - transforming from:`), initialValue]); @@ -1220,10 +1303,15 @@ export function compositeFrom(description) { let continuationStorage; + const inputDictionary = + Object.fromEntries( + stitchArrays({symbol: inputSymbols, value: inputValues}) + .map(({symbol, value}) => [symbol, value])); + const filterableDependencies = { ...availableDependencies, ...inputMetadata, - ...inputValues, + ...inputDictionary, ... (expectingTransform ? {[input.updateValue()]: valueSoFar} @@ -1568,7 +1656,7 @@ export const exposeDependency = templateCompositeFrom({ compose: false, inputs: { - dependency: input.staticDependency(), + dependency: input.staticDependency({acceptsNull: true}), }, steps: () => [ @@ -1618,17 +1706,17 @@ export const exposeConstant = templateCompositeFrom({ // for values like zero and the empty string! // -const availabilityCheckModeInput = { +const inputAvailabilityCheckMode = () => input({ validate: is('null', 'empty', 'falsy'), defaultValue: 'null', -}; +}); export const withResultOfAvailabilityCheck = templateCompositeFrom({ annotation: `withResultOfAvailabilityCheck`, inputs: { - from: input(), - mode: input(availabilityCheckModeInput), + from: input({acceptsNull: true}), + mode: inputAvailabilityCheckMode(), }, outputs: ['#availability'], @@ -1669,8 +1757,8 @@ export const exposeDependencyOrContinue = templateCompositeFrom({ annotation: `exposeDependencyOrContinue`, inputs: { - dependency: input(), - mode: input(availabilityCheckModeInput), + dependency: input({acceptsNull: true}), + mode: inputAvailabilityCheckMode(), }, steps: () => [ @@ -1700,8 +1788,12 @@ export const exposeUpdateValueOrContinue = templateCompositeFrom({ annotation: `exposeUpdateValueOrContinue`, inputs: { - mode: input(availabilityCheckModeInput), - validate: input({type: 'function', null: true}), + mode: inputAvailabilityCheckMode(), + + validate: input({ + type: 'function', + defaultValue: null, + }), }, update: ({ @@ -1725,9 +1817,9 @@ export const exitWithoutDependency = templateCompositeFrom({ annotation: `exitWithoutDependency`, inputs: { - dependency: input(), - mode: input(availabilityCheckModeInput), - value: input({null: true}), + dependency: input({acceptsNull: true}), + mode: inputAvailabilityCheckMode(), + value: input({defaultValue: null}), }, steps: () => [ @@ -1755,7 +1847,7 @@ export const exitWithoutUpdateValue = templateCompositeFrom({ annotation: `exitWithoutUpdateValue`, inputs: { - mode: input(availabilityCheckModeInput), + mode: inputAvailabilityCheckMode(), value: input({defaultValue: null}), }, @@ -1763,6 +1855,7 @@ export const exitWithoutUpdateValue = templateCompositeFrom({ exitWithoutDependency({ dependency: input.updateValue(), mode: input('mode'), + value: input('value'), }), ], }); @@ -1773,8 +1866,8 @@ export const raiseOutputWithoutDependency = templateCompositeFrom({ annotation: `raiseOutputWithoutDependency`, inputs: { - dependency: input(), - mode: input(availabilityCheckModeInput), + dependency: input({acceptsNull: true}), + mode: inputAvailabilityCheckMode(), output: input.staticValue({defaultValue: {}}), }, @@ -1807,7 +1900,7 @@ export const raiseOutputWithoutUpdateValue = templateCompositeFrom({ annotation: `raiseOutputWithoutUpdateValue`, inputs: { - mode: input(availabilityCheckModeInput), + mode: inputAvailabilityCheckMode(), output: input.staticValue({defaultValue: {}}), }, @@ -1841,7 +1934,7 @@ export const withPropertyFromObject = templateCompositeFrom({ annotation: `withPropertyFromObject`, inputs: { - object: input({type: 'object', null: true}), + object: input({type: 'object', acceptsNull: true}), property: input({type: 'string'}), }, @@ -1907,19 +2000,13 @@ export const withPropertiesFromObject = templateCompositeFrom({ annotation: `withPropertiesFromObject`, inputs: { - object: input({ - type: 'object', - null: true, - }), + object: input({type: 'object', acceptsNull: true}), properties: input({ validate: validateArrayItems(isString), }), - prefix: input.staticValue({ - type: 'string', - null: true, - }), + prefix: input.staticValue({type: 'string', defaultValue: null}), }, outputs: ({ @@ -2036,10 +2123,7 @@ export const withPropertiesFromList = templateCompositeFrom({ validate: validateArrayItems(isString), }), - prefix: input.staticValue({ - type: 'string', - null: true, - }), + prefix: input.staticValue({type: 'string', defaultValue: null}), }, outputs: ({ @@ -2109,7 +2193,7 @@ export const fillMissingListItems = templateCompositeFrom({ inputs: { list: input({type: 'array'}), - fill: input(), + fill: input({acceptsNull: true}), }, outputs: ({ @@ -2150,8 +2234,8 @@ export const excludeFromList = templateCompositeFrom({ inputs: { list: input(), - item: input({null: true}), - items: input({validate: isArray, null: true}), + item: input({defaultValue: null}), + items: input({type: 'array', defaultValue: null}), }, outputs: ({ diff --git a/src/data/things/thing.js b/src/data/things/thing.js index ef547f74..290be59b 100644 --- a/src/data/things/thing.js +++ b/src/data/things/thing.js @@ -25,8 +25,8 @@ import { import { isAdditionalFileList, isBoolean, - isCommentary, isColor, + isCommentary, isContributionList, isDate, isDimensions, @@ -41,12 +41,13 @@ import { validateInstanceOf, validateReference, validateReferenceList, + validateWikiData, } from '#validators'; import CacheableObject from './cacheable-object.js'; export default class Thing extends CacheableObject { - static referenceType = Symbol('Thing.referenceType'); + static referenceType = Symbol.for('Thing.referenceType'); static getPropertyDescriptors = Symbol('Thing.getPropertyDescriptors'); static getSerializeDescriptors = Symbol('Thing.getSerializeDescriptors'); @@ -283,10 +284,8 @@ export const referenceList = templateCompositeFrom({ inputs: { class: input.staticValue(thingClassInput), + data: inputWikiData({allowMixedTypes: false}), find: input({type: 'function'}), - - // todo: validate - data: input(), }, update: ({ @@ -316,9 +315,7 @@ export const singleReference = templateCompositeFrom({ inputs: { class: input(thingClassInput), find: input({type: 'function'}), - - // todo: validate - data: input(), + data: inputWikiData({allowMixedTypes: false}), }, update: ({ @@ -347,7 +344,10 @@ export const contribsPresent = templateCompositeFrom({ compose: false, inputs: { - contribs: input({type: 'string'}), + contribs: input.staticDependency({ + validate: isContributionList, + acceptsNull: true, + }), }, steps: () => [ @@ -371,9 +371,7 @@ export const reverseReferenceList = templateCompositeFrom({ compose: false, inputs: { - // todo: validate - data: input(), - + data: inputWikiData({allowMixedTypes: false}), list: input({type: 'string'}), }, @@ -448,6 +446,21 @@ export const commentatorArtists = templateCompositeFrom({ // Compositional utilities +// TODO: This doesn't access a class's own ThingSubclass[Thing.referenceType] +// value because classes aren't initialized by when templateCompositeFrom gets +// called (see: circular imports). So the reference types have to be hard-coded, +// which somewhat defeats the point of storing them on the class in the first +// place... +export function inputWikiData({ + referenceType = '', + allowMixedTypes = false, +} = {}) { + return input({ + validate: validateWikiData(referenceType), + acceptsNull: true, + }); +} + // Resolves the contribsByRef contained in the provided dependency, // providing (named by the second argument) the result. "Resolving" // means mapping the "who" reference of each contribution to an artist @@ -456,8 +469,10 @@ export const withResolvedContribs = templateCompositeFrom({ annotation: `withResolvedContribs`, inputs: { - // todo: validate - from: input(), + from: input({ + validate: isContributionList, + acceptsNull: true, + }), notFoundMode: input({ validate: is('exit', 'filter', 'null'), @@ -514,10 +529,12 @@ export const exitWithoutContribs = templateCompositeFrom({ annotation: `exitWithoutContribs`, inputs: { - // todo: validate - contribs: input(), + contribs: input({ + validate: isContributionList, + acceptsNull: true, + }), - value: input({null: true}), + value: input({defaultValue: null}), }, steps: () => [ @@ -553,12 +570,9 @@ export const withResolvedReference = templateCompositeFrom({ annotation: `withResolvedReference`, inputs: { - // todo: validate - ref: input(), - - // todo: validate - data: input(), + ref: input({type: 'string', acceptsNull: true}), + data: inputWikiData({allowMixedTypes: false}), find: input({type: 'function'}), notFoundMode: input({ @@ -618,12 +632,12 @@ export const withResolvedReferenceList = templateCompositeFrom({ annotation: `withResolvedReferenceList`, inputs: { - // todo: validate - list: input(), - - // todo: validate - data: input(), + list: input({ + validate: validateArrayItems(isString), + acceptsNull: true, + }), + data: inputWikiData({allowMixedTypes: false}), find: input({type: 'function'}), notFoundMode: input({ @@ -706,9 +720,7 @@ export const withReverseReferenceList = templateCompositeFrom({ annotation: `withReverseReferenceList`, inputs: { - // todo: validate - data: input(), - + data: inputWikiData({allowMixedTypes: false}), list: input({type: 'string'}), }, diff --git a/src/data/things/track.js b/src/data/things/track.js index 3e0d95bf..c77bf889 100644 --- a/src/data/things/track.js +++ b/src/data/things/track.js @@ -18,12 +18,13 @@ import { } from '#composite'; import { + is, isBoolean, isColor, isContributionList, isDate, isFileExtension, - oneOf, + validateWikiData, } from '#validators'; import CacheableObject from './cacheable-object.js'; @@ -434,7 +435,7 @@ export const withAlbum = templateCompositeFrom({ inputs: { notFoundMode: input({ - validate: oneOf('exit', 'null'), + validate: is('exit', 'null'), defaultValue: 'null', }), }, @@ -488,7 +489,7 @@ export const withPropertyFromAlbum = templateCompositeFrom({ property: input.staticValue({type: 'string'}), notFoundMode: input({ - validate: oneOf('exit', 'null'), + validate: is('exit', 'null'), defaultValue: 'null', }), }, @@ -527,7 +528,7 @@ export const withContainingTrackSection = templateCompositeFrom({ inputs: { notFoundMode: input({ - validate: oneOf('exit', 'null'), + validate: is('exit', 'null'), defaultValue: 'null', }), }, @@ -589,8 +590,10 @@ export const withOriginalRelease = templateCompositeFrom({ inputs: { selfIfOriginal: input({type: 'boolean', defaultValue: false}), - // todo: validate - data: input({defaultDependency: 'trackData'}), + data: input({ + validate: validateWikiData({referenceType: 'track'}), + defaultDependency: 'trackData', + }), }, outputs: ['#originalRelease'], @@ -683,7 +686,7 @@ export const exitWithoutUniqueCoverArt = templateCompositeFrom({ annotation: `exitWithoutUniqueCoverArt`, inputs: { - value: input({null: true}), + value: input({defaultValue: null}), }, steps: () => [ diff --git a/src/data/things/validators.js b/src/data/things/validators.js index cd4c2b46..048f7ebb 100644 --- a/src/data/things/validators.js +++ b/src/data/things/validators.js @@ -1,7 +1,7 @@ import {inspect as nodeInspect} from 'node:util'; import {colors, ENABLE_COLOR} from '#cli'; -import {withAggregate} from '#sugar'; +import {empty, withAggregate} from '#sugar'; function inspect(value) { return nodeInspect(value, {colors: ENABLE_COLOR}); @@ -404,6 +404,62 @@ export function validateReferenceList(type = '') { return validateArrayItems(validateReference(type)); } +export function validateWikiData({ + referenceType = '', + allowMixedTypes = false, +}) { + if (referenceType && allowMixedTypes) { + throw new TypeError(`Don't specify both referenceType and allowMixedTypes`); + } + + const isArrayOfObjects = validateArrayItems(isObject); + + return (array) => { + isArrayOfObjects(array); + + if (empty(array)) { + return true; + } + + const allRefTypes = + new Set(array.map(object => + object.constructor[Symbol.for('Thing.referenceType')])); + + if (allRefTypes.has(undefined)) { + if (allRefTypes.size === 1) { + throw new TypeError(`Expected array of wiki data objects, got array of other objects`); + } else { + throw new TypeError(`Expected array of wiki data objects, got mixed items`); + } + } + + if (allRefTypes.size > 1) { + if (allowMixedTypes) { + return true; + } + + const types = () => Array.from(allRefTypes).join(', '); + + if (referenceType) { + if (allRefTypes.has(referenceType)) { + allRefTypes.remove(referenceType); + throw new TypeError(`Expected array of only ${referenceType}, also got other types: ${types()}`) + } else { + throw new TypeError(`Expected array of only ${referenceType}, got other types: ${types()}`); + } + } + + throw new TypeError(`Expected array of unmixed reference types, got multiple: ${types()}`); + } + + if (referenceType && !allRefTypes.has(referenceType)) { + throw new TypeError(`Expected array of ${referenceType}, got array of ${allRefTypes[0]}`) + } + + return true; + }; +} + // Compositional utilities export function oneOf(...checks) { diff --git a/src/util/sugar.js b/src/util/sugar.js index 14fb250e..24d409fb 100644 --- a/src/util/sugar.js +++ b/src/util/sugar.js @@ -599,12 +599,7 @@ export function showAggregate(topError, { return [headerPart, causePart, aggregatePart].filter(Boolean).join('\n'); }; - const message = - (topError instanceof AggregateError - ? recursive(topError, {level: 0}) - : (showTraces - ? topError.stack - : topError.toString())); + const message = recursive(topError, {level: 0}); if (print) { console.error(message); -- cgit 1.3.0-6-gf8a5