From 3eee332896d29d3bc6ec54e4ee29170b233f83d3 Mon Sep 17 00:00:00 2001 From: Randy Schott <1815175+schottra@users.noreply.github.com> Date: Fri, 23 Oct 2020 14:22:48 -0700 Subject: [PATCH] feat: support Struct as a Launch input type (#113) * test: checkpoint for adding test cases * test: fixing all the broken test cases * feat: add input to support entering structs * chore: cleanup --- src/common/utils.ts | 7 + .../Launch/LaunchForm/LaunchFormInputs.tsx | 3 + .../Launch/LaunchForm/StructInput.tsx | 38 +++++ .../LaunchForm/inputHelpers/constants.ts | 1 + .../inputHelpers/getHelperForInput.ts | 3 +- .../Launch/LaunchForm/inputHelpers/struct.ts | 149 ++++++++++++++++++ .../inputHelpers/test/inputHelpers.test.ts | 48 ++++-- .../inputHelpers/test/structTestCases.ts | 107 +++++++++++++ .../LaunchForm/inputHelpers/test/testCases.ts | 67 ++++++-- .../Launch/LaunchForm/inputHelpers/utils.ts | 8 +- src/components/common/DumpJSON.tsx | 5 +- 11 files changed, 402 insertions(+), 34 deletions(-) create mode 100644 src/components/Launch/LaunchForm/StructInput.tsx create mode 100644 src/components/Launch/LaunchForm/inputHelpers/struct.ts create mode 100644 src/components/Launch/LaunchForm/inputHelpers/test/structTestCases.ts diff --git a/src/common/utils.ts b/src/common/utils.ts index 8bf592490..3646f3a03 100644 --- a/src/common/utils.ts +++ b/src/common/utils.ts @@ -126,3 +126,10 @@ export function toBoolean(value?: string): boolean { } return ['true', 'True', 'TRUE', '1'].includes(value); } + +/** Simple shared stringify function to ensure consistency in formatting with + * respect to spacing. + */ +export function stringifyValue(value: any): string { + return JSON.stringify(value, null, 2); +} diff --git a/src/components/Launch/LaunchForm/LaunchFormInputs.tsx b/src/components/Launch/LaunchForm/LaunchFormInputs.tsx index 2dbf5cab8..ed9ee9636 100644 --- a/src/components/Launch/LaunchForm/LaunchFormInputs.tsx +++ b/src/components/Launch/LaunchForm/LaunchFormInputs.tsx @@ -4,6 +4,7 @@ import { CollectionInput } from './CollectionInput'; import { formStrings } from './constants'; import { LaunchState } from './launchMachine'; import { SimpleInput } from './SimpleInput'; +import { StructInput } from './StructInput'; import { useStyles } from './styles'; import { BaseInterpretedLaunchState, @@ -22,6 +23,8 @@ function getComponentForInput(input: InputProps, showErrors: boolean) { return ; case InputType.Collection: return ; + case InputType.Struct: + return ; case InputType.Map: case InputType.Unknown: case InputType.None: diff --git a/src/components/Launch/LaunchForm/StructInput.tsx b/src/components/Launch/LaunchForm/StructInput.tsx new file mode 100644 index 000000000..0f8327554 --- /dev/null +++ b/src/components/Launch/LaunchForm/StructInput.tsx @@ -0,0 +1,38 @@ +import { TextField } from '@material-ui/core'; +import * as React from 'react'; +import { InputChangeHandler, InputProps } from './types'; +import { getLaunchInputId } from './utils'; + +function stringChangeHandler(onChange: InputChangeHandler) { + return ({ target: { value } }: React.ChangeEvent) => { + onChange(value); + }; +} + +/** Handles rendering of the input component for a Struct */ +export const StructInput: React.FC = props => { + const { + error, + label, + name, + onChange, + typeDefinition: { subtype }, + value = '' + } = props; + const hasError = !!error; + const helperText = hasError ? error : props.helperText; + return ( + + ); +}; diff --git a/src/components/Launch/LaunchForm/inputHelpers/constants.ts b/src/components/Launch/LaunchForm/inputHelpers/constants.ts index 0951a3a89..a1a219af8 100644 --- a/src/components/Launch/LaunchForm/inputHelpers/constants.ts +++ b/src/components/Launch/LaunchForm/inputHelpers/constants.ts @@ -9,6 +9,7 @@ export const allowedDateFormats = [ISO_8601, RFC_2822]; const primitivePath = 'scalar.primitive'; export const schemaUriPath = 'scalar.schema.uri'; +export const structPath = 'scalar.generic'; /** Strings constants which can be used to perform a deep `get` on a scalar * literal type using a primitive value. diff --git a/src/components/Launch/LaunchForm/inputHelpers/getHelperForInput.ts b/src/components/Launch/LaunchForm/inputHelpers/getHelperForInput.ts index d58d8b191..76c06b203 100644 --- a/src/components/Launch/LaunchForm/inputHelpers/getHelperForInput.ts +++ b/src/components/Launch/LaunchForm/inputHelpers/getHelperForInput.ts @@ -9,6 +9,7 @@ import { integerHelper } from './integer'; import { noneHelper } from './none'; import { schemaHelper } from './schema'; import { stringHelper } from './string'; +import { structHelper } from './struct'; import { InputHelper } from './types'; const unsupportedHelper = noneHelper; @@ -28,7 +29,7 @@ const inputHelpers: Record = { [InputType.None]: noneHelper, [InputType.Schema]: schemaHelper, [InputType.String]: stringHelper, - [InputType.Struct]: unsupportedHelper, + [InputType.Struct]: structHelper, [InputType.Unknown]: unsupportedHelper }; diff --git a/src/components/Launch/LaunchForm/inputHelpers/struct.ts b/src/components/Launch/LaunchForm/inputHelpers/struct.ts new file mode 100644 index 000000000..f6ec9c02a --- /dev/null +++ b/src/components/Launch/LaunchForm/inputHelpers/struct.ts @@ -0,0 +1,149 @@ +import { stringifyValue } from 'common/utils'; +import { Core, Protobuf } from 'flyteidl'; +import { InputValue } from '../types'; +import { structPath } from './constants'; +import { ConverterInput, InputHelper, InputValidatorParams } from './types'; +import { extractLiteralWithCheck } from './utils'; + +type PrimitiveType = string | number | boolean | null | object; + +function asValueWithKind(value: Protobuf.IValue): Protobuf.Value { + return value instanceof Protobuf.Value + ? value + : Protobuf.Value.create(value); +} + +function protobufValueToPrimitive( + value: Protobuf.IValue +): PrimitiveType | PrimitiveType[] { + const valueWithKind = asValueWithKind(value); + const { kind } = valueWithKind; + switch (kind) { + case 'nullValue': + return null; + case 'structValue': + if (valueWithKind.structValue == null) { + throw new Error('Unexpected empty structValue field'); + } + return protobufStructToObject(valueWithKind.structValue); + case 'listValue': + if (valueWithKind.listValue == null) { + throw new Error('Unexpected empty listValue field'); + } + return protobufListToArray(valueWithKind.listValue); + case undefined: + throw new Error('Unexpected missing Value.kind'); + default: + return valueWithKind[kind]; + } +} + +function primitiveToProtobufValue(value: any): Protobuf.IValue { + if (value == null) { + return { nullValue: Protobuf.NullValue.NULL_VALUE }; + } + if (Array.isArray(value)) { + return { listValue: { values: value.map(primitiveToProtobufValue) } }; + } + switch (typeof value) { + case 'boolean': + return { boolValue: !!value }; + case 'number': + return { numberValue: value }; + case 'string': + return { stringValue: value }; + case 'object': + return { structValue: objectToProtobufStruct(value) }; + default: + throw new Error(`Unsupported value type: ${typeof value} `); + } +} + +function protobufListToArray(list: Protobuf.IListValue): PrimitiveType[] { + if (!list.values) { + return []; + } + + return list.values.map(protobufValueToPrimitive); +} + +function protobufStructToObject(struct: Protobuf.IStruct): Dictionary { + if (struct.fields == null) { + return {}; + } + + return Object.entries(struct.fields).reduce>( + (out, [key, value]) => { + out[key] = protobufValueToPrimitive(value); + return out; + }, + {} + ); +} + +function objectToProtobufStruct(obj: Dictionary): Protobuf.IStruct { + const fields = Object.entries(obj).reduce>( + (out, [key, value]) => { + try { + out[key] = primitiveToProtobufValue(value); + return out; + } catch (e) { + throw new Error( + `Failed to convert value ${key} to Protobuf.Value: ${e}` + ); + } + }, + {} + ); + + return { fields }; +} + +function fromLiteral(literal: Core.ILiteral): InputValue { + const structValue = extractLiteralWithCheck( + literal, + structPath + ); + + return stringifyValue(protobufStructToObject(structValue)); +} + +function toLiteral({ value }: ConverterInput): Core.ILiteral { + let parsedObject: Dictionary; + + if (typeof value === 'object') { + parsedObject = value; + } else { + const stringValue = + typeof value === 'string' ? value : value.toString(); + + try { + parsedObject = JSON.parse(stringValue); + if (typeof parsedObject !== 'object') { + throw new Error(`Result was of type: ${typeof parsedObject}`); + } + } catch (e) { + throw new Error(`Value did not parse to an object`); + } + } + + return { scalar: { generic: objectToProtobufStruct(parsedObject) } }; +} + +function validate({ value }: InputValidatorParams) { + if (typeof value !== 'string') { + throw new Error('Value is not a string'); + } + + try { + JSON.parse(value); + } catch (e) { + throw new Error(`Value did not parse to an object: ${e}`); + } +} + +export const structHelper: InputHelper = { + fromLiteral, + toLiteral, + validate +}; diff --git a/src/components/Launch/LaunchForm/inputHelpers/test/inputHelpers.test.ts b/src/components/Launch/LaunchForm/inputHelpers/test/inputHelpers.test.ts index 7f0ab1cde..4bed1617e 100644 --- a/src/components/Launch/LaunchForm/inputHelpers/test/inputHelpers.test.ts +++ b/src/components/Launch/LaunchForm/inputHelpers/test/inputHelpers.test.ts @@ -1,3 +1,4 @@ +import { stringifyValue } from 'common/utils'; import { Core } from 'flyteidl'; import * as Long from 'long'; import { BlobDimensionality } from 'models'; @@ -67,7 +68,7 @@ describe('literalToInputValue', () => { literalToInputTestCases.map(([typeDefinition, input, output]) => it(`should correctly convert ${ typeDefinition.type - }: ${JSON.stringify(input)}`, () => + }: ${stringifyValue(input)}`, () => expect(literalToInputValue(typeDefinition, input)).toEqual( output )) @@ -90,7 +91,7 @@ describe('literalToInputValue', () => { literalToInputTestCases.map(([typeDefinition, input, output]) => { it(`should correctly convert collection of ${ typeDefinition.type - }: ${JSON.stringify(input)}`, () => { + }: ${stringifyValue(input)}`, () => { const collection: Core.ILiteral = { collection: { // Duplicate it to test comma separation @@ -149,7 +150,7 @@ describe('inputToLiteral', () => { literalTestCases.map(([typeDefinition, input, output]) => { it(`should correctly convert ${ typeDefinition.type - }: ${JSON.stringify(input)} (${typeof input})`, () => + }: ${stringifyValue(input)} (${typeof input})`, () => expect( inputToLiteral(makeSimpleInput(typeDefinition, input)) ).toEqual(output)); @@ -158,33 +159,48 @@ describe('inputToLiteral', () => { describe('Collections', () => { literalTestCases.map(([typeDefinition, input, output]) => { - let value: any; - if (['boolean', 'number'].includes(typeof input)) { - value = input; + let singleCollectionValue: any; + let nestedCollectionValue: any; + if (typeDefinition.type === InputType.Struct) { + const objValue = JSON.parse(input); + singleCollectionValue = stringifyValue([objValue]); + nestedCollectionValue = stringifyValue([[objValue]]); + } else if (['boolean', 'number'].includes(typeof input)) { + singleCollectionValue = `[${input}]`; + nestedCollectionValue = `[[${input}]]`; } else if (input == null) { - value = 'null'; + singleCollectionValue = '[null]'; + nestedCollectionValue = '[[null]]'; } else if (typeof input === 'string' || Long.isLong(input)) { - value = `"${input}"`; + singleCollectionValue = `["${input}"]`; + nestedCollectionValue = `[["${input}"]]`; } else if (input instanceof Date) { - value = `"${input.toISOString()}"`; + const dateString = input.toISOString(); + singleCollectionValue = `["${dateString}"]`; + nestedCollectionValue = `[["${dateString}"]]`; } else { - value = JSON.stringify(input); + const stringValue = stringifyValue(input); + singleCollectionValue = `[${stringValue}]`; + nestedCollectionValue = `[[${stringValue}]]`; } it(`should correctly convert collection of type ${ typeDefinition.type - }: [${JSON.stringify(value)}] (${typeof input})`, () => { + }: ${singleCollectionValue} (${typeof input})`, () => { const result = inputToLiteral( - makeCollectionInput(typeDefinition, `[${value}]`) + makeCollectionInput(typeDefinition, singleCollectionValue) ); expect(result.collection!.literals![0]).toEqual(output); }); it(`should correctly convert nested collection of type ${ typeDefinition.type - }: [[${JSON.stringify(value)}]] (${typeof input})`, () => { + }: ${nestedCollectionValue} (${typeof input})`, () => { const result = inputToLiteral( - makeNestedCollectionInput(typeDefinition, `[[${value}]]`) + makeNestedCollectionInput( + typeDefinition, + nestedCollectionValue + ) ); expect( result.collection!.literals![0].collection!.literals![0] @@ -221,7 +237,7 @@ function generateValidityTests( { valid, invalid }: { valid: any[]; invalid: any[] } ) { valid.map(value => - it(`should treat ${JSON.stringify( + it(`should treat ${stringifyValue( value )} (${typeof value}) as valid`, () => { const input = makeSimpleInput(typeDefinition, value); @@ -229,7 +245,7 @@ function generateValidityTests( }) ); invalid.map(value => - it(`should treat ${JSON.stringify( + it(`should treat ${stringifyValue( value )} (${typeof value}) as invalid`, () => { const input = makeSimpleInput(typeDefinition, value); diff --git a/src/components/Launch/LaunchForm/inputHelpers/test/structTestCases.ts b/src/components/Launch/LaunchForm/inputHelpers/test/structTestCases.ts new file mode 100644 index 000000000..93bdb3f28 --- /dev/null +++ b/src/components/Launch/LaunchForm/inputHelpers/test/structTestCases.ts @@ -0,0 +1,107 @@ +import { stringifyValue } from 'common/utils'; +import { Core, Protobuf } from 'flyteidl'; + +export function structLiteral(generic: Protobuf.IStruct): Core.ILiteral { + return { scalar: { generic } }; +} + +const values = { + stringField: 'aString', + integerField: 123, + floatField: 123.456, + nullField: null, + booleanTrueField: true, + booleanFalseField: false +}; + +const structValues: { [k in keyof typeof values]: Protobuf.IValue } = { + stringField: { stringValue: 'aString' }, + integerField: { numberValue: 123 }, + floatField: { numberValue: 123.456 }, + nullField: { nullValue: Protobuf.NullValue.NULL_VALUE }, + booleanTrueField: { boolValue: true }, + booleanFalseField: { boolValue: false } +}; + +type StructTestCase = [string, Core.ILiteral]; +export const structTestCases: StructTestCase[] = [ + ['{}', structLiteral({ fields: {} })], + // simple case with no lists or nested structs + [ + stringifyValue({ ...values }), + structLiteral({ fields: { ...structValues } }) + ], + // Nested struct value + [ + stringifyValue({ nestedStruct: { ...values } }), + structLiteral({ + fields: { + nestedStruct: { structValue: { fields: { ...structValues } } } + } + }) + ], + // List + [ + stringifyValue({ listField: Object.values(values) }), + structLiteral({ + fields: { + listField: { + listValue: { values: Object.values(structValues) } + } + } + }) + ], + // Nested struct with list + [ + stringifyValue({ nestedStruct: { listField: Object.values(values) } }), + structLiteral({ + fields: { + nestedStruct: { + structValue: { + fields: { + listField: { + listValue: { + values: Object.values(structValues) + } + } + } + } + } + } + }) + ], + // List with nested struct + [ + stringifyValue({ listField: [{ ...values }] }), + structLiteral({ + fields: { + listField: { + listValue: { + values: [ + { structValue: { fields: { ...structValues } } } + ] + } + } + } + }) + ], + // List with nested list + [ + stringifyValue({ listField: [Object.values(values)] }), + structLiteral({ + fields: { + listField: { + listValue: { + values: [ + { + listValue: { + values: Object.values(structValues) + } + } + ] + } + } + } + }) + ] +]; diff --git a/src/components/Launch/LaunchForm/inputHelpers/test/testCases.ts b/src/components/Launch/LaunchForm/inputHelpers/test/testCases.ts index 7a14b9b90..22a059d91 100644 --- a/src/components/Launch/LaunchForm/inputHelpers/test/testCases.ts +++ b/src/components/Launch/LaunchForm/inputHelpers/test/testCases.ts @@ -1,13 +1,18 @@ -import { dateToTimestamp, millisecondsToDuration } from 'common/utils'; +import { + dateToTimestamp, + millisecondsToDuration, + stringifyValue +} from 'common/utils'; import { Core } from 'flyteidl'; import * as Long from 'long'; import { BlobDimensionality, SchemaColumnType } from 'models'; import { blobLiteral, primitiveLiteral } from '../../__mocks__/utils'; import { InputType, InputTypeDefinition, InputValue } from '../../types'; import { literalNone } from '../constants'; +import { structTestCases } from './structTestCases'; // Defines type of value, input, and expected value of a `Core.ILiteral` -type LiteralTestParams = [InputTypeDefinition, any, Core.ILiteral]; +type InputToLiteralTestParams = [InputTypeDefinition, any, Core.ILiteral]; type InputTypeKey = | 'binary' @@ -104,15 +109,15 @@ export const supportedPrimitives: InputTypeDefinition[] = [ inputTypes.duration, inputTypes.float, inputTypes.integer, - inputTypes.schema + inputTypes.schema, + inputTypes.struct ]; export const unsupportedTypes: InputTypeDefinition[] = [ inputTypes.binary, inputTypes.error, inputTypes.map, - inputTypes.none, - inputTypes.struct + inputTypes.none ]; export const validityTestCases = { @@ -195,10 +200,34 @@ export const validityTestCases = { }, // schema is just a specialized string input, so it has the same validity cases as string schema: { invalid: [123, true, new Date(), {}], valid: ['', 'abcdefg'] }, - string: { invalid: [123, true, new Date(), {}], valid: ['', 'abcdefg'] } + string: { invalid: [123, true, new Date(), {}], valid: ['', 'abcdefg'] }, + struct: { + invalid: [ + 123, + true, + new Date(), + {}, + 'nonObjectString', + '[]', + '{garbageobject}' + ], + valid: [ + // Valid use case is any stringified POJO + stringifyValue({ + someString: 'someValue', + someNumber: 123, + someBoolean: true, + nestedStruct: { someOtherString: 'someOtherValue' }, + nestedList: [123, 'stringListValue'], + nullValue: null + }) + ] + } }; -export const literalTestCases: LiteralTestParams[] = [ +/** Test cases for converting a *valid* input value to its corresponding ILiteral + * representation. */ +export const literalTestCases: InputToLiteralTestParams[] = [ [inputTypes.boolean, true, primitiveLiteral({ boolean: true })], [inputTypes.boolean, 'true', primitiveLiteral({ boolean: true })], [inputTypes.boolean, 't', primitiveLiteral({ boolean: true })], @@ -434,15 +463,24 @@ export const literalTestCases: LiteralTestParams[] = [ literalNone() ], // Blob which is not an object (results in None) - [inputTypes.blobMulti, undefined, literalNone()] + [inputTypes.blobMulti, undefined, literalNone()], + ...structTestCases.map( + ([stringValue, literalValue]) => [ + inputTypes.struct, + stringValue, + literalValue + ] + ) ]; -type InputToLiteralTestParams = [ +type LiteralToInputTestParams = [ InputTypeDefinition, Core.ILiteral, InputValue | undefined ]; -export const literalToInputTestCases: InputToLiteralTestParams[] = [ + +/** Test cases for converting a Core.ILiteral to a usable InputValue */ +export const literalToInputTestCases: LiteralToInputTestParams[] = [ [inputTypes.boolean, primitiveLiteral({ boolean: true }), true], [inputTypes.boolean, primitiveLiteral({ boolean: false }), false], [ @@ -575,5 +613,12 @@ export const literalToInputTestCases: InputToLiteralTestParams[] = [ format: 'csv', uri: 's3://somePath' } - ] + ], + ...structTestCases.map( + ([stringValue, literalValue]) => [ + inputTypes.struct, + literalValue, + stringValue + ] + ) ]; diff --git a/src/components/Launch/LaunchForm/inputHelpers/utils.ts b/src/components/Launch/LaunchForm/inputHelpers/utils.ts index 4018e0b4e..e53ab11b3 100644 --- a/src/components/Launch/LaunchForm/inputHelpers/utils.ts +++ b/src/components/Launch/LaunchForm/inputHelpers/utils.ts @@ -1,4 +1,4 @@ -import { assertNever } from 'common/utils'; +import { assertNever, stringifyValue } from 'common/utils'; import { Core } from 'flyteidl'; import { get } from 'lodash'; import { BlobDimensionality } from 'models'; @@ -25,7 +25,9 @@ export function collectionChildToString(type: InputType, value: any) { if (value === undefined) { return ''; } - return type === InputType.Integer ? `${value}` : JSON.stringify(value); + return type === (InputType.Integer || InputType.Struct) + ? `${value}` + : stringifyValue(value); } /** Determines if a given input type, including all levels of nested types, is @@ -38,7 +40,6 @@ export function typeIsSupported(typeDefinition: InputTypeDefinition): boolean { case InputType.Error: case InputType.Map: case InputType.None: - case InputType.Struct: case InputType.Unknown: return false; case InputType.Boolean: @@ -49,6 +50,7 @@ export function typeIsSupported(typeDefinition: InputTypeDefinition): boolean { case InputType.Integer: case InputType.Schema: case InputType.String: + case InputType.Struct: return true; case InputType.Collection: { if (!subtype) { diff --git a/src/components/common/DumpJSON.tsx b/src/components/common/DumpJSON.tsx index da2472caa..24e6ec441 100644 --- a/src/components/common/DumpJSON.tsx +++ b/src/components/common/DumpJSON.tsx @@ -1,11 +1,10 @@ +import { stringifyValue } from 'common/utils'; import { useCommonStyles } from 'components/common/styles'; import * as React from 'react'; export const DumpJSON: React.FC<{ value: any }> = ({ value }) => { const commonStyles = useCommonStyles(); return ( -
- {JSON.stringify(value, null, 2)} -
+
{stringifyValue(value)}
); };