diff --git a/src/common/utils.ts b/src/common/utils.ts index e727f243b..cdf07cb5b 100644 --- a/src/common/utils.ts +++ b/src/common/utils.ts @@ -87,9 +87,9 @@ export function createCorsProxyURL(path: string) { } /** Returns entires for an object, sorted lexicographically */ -export function sortedObjectEntries( - object: Object -): ReturnType { +export function sortedObjectEntries(object: { + [s: string]: T; +}): [string, T][] { return Object.entries(object).sort((a, b) => a[0].localeCompare(b[0])); } diff --git a/src/components/Launch/LaunchWorkflowForm/__mocks__/mockInputs.ts b/src/components/Launch/LaunchWorkflowForm/__mocks__/mockInputs.ts index 13f54400e..8f1a67116 100644 --- a/src/components/Launch/LaunchWorkflowForm/__mocks__/mockInputs.ts +++ b/src/components/Launch/LaunchWorkflowForm/__mocks__/mockInputs.ts @@ -1,10 +1,10 @@ +import { dateToTimestamp, millisecondsToDuration } from 'common/utils'; +import { Core } from 'flyteidl'; import { mapValues } from 'lodash'; -import { - ParameterMap, - SimpleType, - TypedInterface, - Variable -} from 'models/Common'; +import * as Long from 'long'; +import { SimpleType, TypedInterface, Variable } from 'models/Common'; +import { literalNone } from '../inputHelpers/constants'; +import { primitiveLiteral } from './utils'; function simpleType(primitiveType: SimpleType, description?: string): Variable { return { @@ -15,7 +15,21 @@ function simpleType(primitiveType: SimpleType, description?: string): Variable { }; } -export const mockSimpleVariables: Record = { +const validDateString = '2019-01-10T00:00:00.000Z'; // Dec 1, 2019 + +export type SimpleVariableKey = + | 'simpleString' + | 'stringNoLabel' + | 'simpleInteger' + | 'simpleFloat' + | 'simpleBoolean' + | 'simpleDuration' + | 'simpleDatetime' + | 'simpleBinary' + | 'simpleError' + | 'simpleStruct'; + +export const mockSimpleVariables: Record = { simpleString: simpleType(SimpleType.STRING, 'a simple string value'), stringNoLabel: simpleType(SimpleType.STRING), simpleInteger: simpleType(SimpleType.INTEGER, 'a simple integer value'), @@ -32,6 +46,26 @@ export const mockSimpleVariables: Record = { // blob: {} }; +export const simpleVariableDefaults: Record< + SimpleVariableKey, + Core.ILiteral +> = { + simpleString: primitiveLiteral({ stringValue: 'abcdefg' }), + stringNoLabel: primitiveLiteral({ stringValue: 'abcdefg' }), + simpleBinary: literalNone(), + simpleBoolean: primitiveLiteral({ boolean: false }), + simpleDatetime: primitiveLiteral({ + datetime: dateToTimestamp(new Date(validDateString)) + }), + simpleDuration: primitiveLiteral({ + duration: millisecondsToDuration(10000) + }), + simpleError: literalNone(), + simpleFloat: primitiveLiteral({ floatValue: 1.5 }), + simpleInteger: primitiveLiteral({ integer: Long.fromNumber(12345) }), + simpleStruct: literalNone() +}; + export const mockCollectionVariables: Record = mapValues( mockSimpleVariables, v => ({ diff --git a/src/components/Launch/LaunchWorkflowForm/__mocks__/utils.ts b/src/components/Launch/LaunchWorkflowForm/__mocks__/utils.ts new file mode 100644 index 000000000..6584ea8a0 --- /dev/null +++ b/src/components/Launch/LaunchWorkflowForm/__mocks__/utils.ts @@ -0,0 +1,5 @@ +import { Core } from 'flyteidl'; + +export function primitiveLiteral(primitive: Core.IPrimitive): Core.ILiteral { + return { scalar: { primitive } }; +} diff --git a/src/components/Launch/LaunchWorkflowForm/__stories__/LaunchWorkflowForm.stories.tsx b/src/components/Launch/LaunchWorkflowForm/__stories__/LaunchWorkflowForm.stories.tsx index cc70310aa..a6e64c383 100644 --- a/src/components/Launch/LaunchWorkflowForm/__stories__/LaunchWorkflowForm.stories.tsx +++ b/src/components/Launch/LaunchWorkflowForm/__stories__/LaunchWorkflowForm.stories.tsx @@ -4,7 +4,7 @@ import { resolveAfter } from 'common/promiseUtils'; import { mockAPIContextValue } from 'components/data/__mocks__/apiContext'; import { APIContext } from 'components/data/apiContext'; import { mapValues } from 'lodash'; -import { Variable, Workflow } from 'models'; +import { Literal, Variable, Workflow } from 'models'; import { createMockLaunchPlan } from 'models/__mocks__/launchPlanData'; import { createMockWorkflow, @@ -16,13 +16,18 @@ import { createMockWorkflowInputsInterface, mockCollectionVariables, mockNestedCollectionVariables, - mockSimpleVariables + mockSimpleVariables, + simpleVariableDefaults, + SimpleVariableKey } from '../__mocks__/mockInputs'; import { LaunchWorkflowForm } from '../LaunchWorkflowForm'; +const booleanInputName = 'simpleBoolean'; +const stringInputName = 'simpleString'; +const integerInputName = 'simpleInteger'; const submitAction = action('createWorkflowExecution'); -const renderForm = (variables: Record) => { +const generateMocks = (variables: Record) => { const mockWorkflow = createMockWorkflow('MyWorkflow'); const mockLaunchPlan = createMockLaunchPlan( mockWorkflow.id.name, @@ -63,6 +68,13 @@ const renderForm = (variables: Record) => { listLaunchPlans: () => resolveAfter(500, { entities: [mockLaunchPlan] }) }); + return { mockWorkflow, mockLaunchPlan, mockWorkflowVersions, mockApi }; +}; + +const renderForm = ({ + mockApi, + mockWorkflow +}: ReturnType) => { const onClose = () => console.log('Close'); return ( @@ -79,8 +91,28 @@ const renderForm = (variables: Record) => { const stories = storiesOf('Launch/LaunchWorkflowForm', module); -stories.add('Simple', () => renderForm(mockSimpleVariables)); -stories.add('Collections', () => renderForm(mockCollectionVariables)); +stories.add('Simple', () => renderForm(generateMocks(mockSimpleVariables))); +stories.add('Required Inputs', () => { + const mocks = generateMocks(mockSimpleVariables); + const parameters = mocks.mockLaunchPlan.closure!.expectedInputs.parameters; + parameters[stringInputName].required = true; + parameters[integerInputName].required = true; + parameters[booleanInputName].required = true; + return renderForm(mocks); +}); +stories.add('Default Values', () => { + const mocks = generateMocks(mockSimpleVariables); + const parameters = mocks.mockLaunchPlan.closure!.expectedInputs.parameters; + Object.keys(parameters).forEach(paramName => { + const defaultValue = + simpleVariableDefaults[paramName as SimpleVariableKey]; + parameters[paramName].default = defaultValue as Literal; + }); + return renderForm(mocks); +}); +stories.add('Collections', () => + renderForm(generateMocks(mockCollectionVariables)) +); stories.add('Nested Collections', () => - renderForm(mockNestedCollectionVariables) + renderForm(generateMocks(mockNestedCollectionVariables)) ); diff --git a/src/components/Launch/LaunchWorkflowForm/inputHelpers/boolean.ts b/src/components/Launch/LaunchWorkflowForm/inputHelpers/boolean.ts index 380accf95..5de989e65 100644 --- a/src/components/Launch/LaunchWorkflowForm/inputHelpers/boolean.ts +++ b/src/components/Launch/LaunchWorkflowForm/inputHelpers/boolean.ts @@ -1,6 +1,8 @@ import { Core } from 'flyteidl'; import { InputValue } from '../types'; +import { primitiveLiteralPaths } from './constants'; import { ConverterInput, InputHelper } from './types'; +import { extractLiteralWithCheck } from './utils'; /** Checks that a value is an acceptable boolean value. These can be * an actual Boolean instance, any variant of ('T', 'F', 'TRUE', 'FALSE') or @@ -38,6 +40,13 @@ function toLiteral({ value }: ConverterInput): Core.ILiteral { return { scalar: { primitive: { boolean: parseBoolean(value) } } }; } +function fromLiteral(literal: Core.ILiteral): InputValue { + return extractLiteralWithCheck( + literal, + primitiveLiteralPaths.scalarBoolean + ); +} + function validate({ value }: ConverterInput) { if (!isValidBoolean(value)) { throw new Error('Value is not a valid boolean'); @@ -45,6 +54,8 @@ function validate({ value }: ConverterInput) { } export const booleanHelper: InputHelper = { + fromLiteral, toLiteral, - validate + validate, + defaultValue: false }; diff --git a/src/components/Launch/LaunchWorkflowForm/inputHelpers/collection.ts b/src/components/Launch/LaunchWorkflowForm/inputHelpers/collection.ts index 6cc171911..afd8717c0 100644 --- a/src/components/Launch/LaunchWorkflowForm/inputHelpers/collection.ts +++ b/src/components/Launch/LaunchWorkflowForm/inputHelpers/collection.ts @@ -1,9 +1,12 @@ import { Core } from 'flyteidl'; +import { InputTypeDefinition, InputValue } from '../types'; import { literalNone } from './constants'; import { getHelperForInput } from './getHelperForInput'; import { parseJSON } from './parseJson'; import { ConverterInput, InputHelper } from './types'; +const missingSubTypeError = 'Unexpected missing subtype for collection'; + function parseCollection(list: string) { const parsed = parseJSON(list); if (!Array.isArray(parsed)) { @@ -12,12 +15,42 @@ function parseCollection(list: string) { return parsed; } -export function toLiteral({ +function fromLiteral( + literal: Core.ILiteral, + { subtype }: InputTypeDefinition +): InputValue { + if (!subtype) { + throw new Error(missingSubTypeError); + } + if (!literal.collection) { + throw new Error('Collection literal missing `collection` property'); + } + if (!literal.collection.literals) { + throw new Error( + 'Collection literal missing `colleciton.literals` property' + ); + } + + const subTypeHelper = getHelperForInput(subtype.type); + const values = literal.collection.literals.reduce( + (out, literal) => { + const value = subTypeHelper.fromLiteral(literal, subtype); + if (value !== undefined) { + out.push(value.toString()); + } + return out; + }, + [] + ); + return `[${values.join(',')}]`; +} + +function toLiteral({ value, typeDefinition: { subtype } }: ConverterInput): Core.ILiteral { if (!subtype) { - throw new Error('Unexpected missing subtype for collection'); + throw new Error(missingSubTypeError); } let parsed: any[]; // If we're processing a nested collection, it may already have been parsed @@ -63,6 +96,7 @@ function validate({ value }: ConverterInput) { } export const collectionHelper: InputHelper = { + fromLiteral, toLiteral, validate }; diff --git a/src/components/Launch/LaunchWorkflowForm/inputHelpers/constants.ts b/src/components/Launch/LaunchWorkflowForm/inputHelpers/constants.ts index b7e8c5b8e..15aad09e3 100644 --- a/src/components/Launch/LaunchWorkflowForm/inputHelpers/constants.ts +++ b/src/components/Launch/LaunchWorkflowForm/inputHelpers/constants.ts @@ -6,3 +6,17 @@ export function literalNone(): Core.ILiteral { } export const allowedDateFormats = [ISO_8601, RFC_2822]; + +const primitivePath = 'scalar.primitive'; + +/** Strings constants which can be used to perform a deep `get` on a scalar + * literal type using a primitive value. + */ +export const primitiveLiteralPaths = { + scalarBoolean: `${primitivePath}.boolean`, + scalarDatetime: `${primitivePath}.datetime`, + scalarDuration: `${primitivePath}.duration`, + scalarFloat: `${primitivePath}.floatValue`, + scalarInteger: `${primitivePath}.integer`, + scalarString: `${primitivePath}.stringValue` +}; diff --git a/src/components/Launch/LaunchWorkflowForm/inputHelpers/datetime.ts b/src/components/Launch/LaunchWorkflowForm/inputHelpers/datetime.ts index 4c9b3cc68..f1c5a1998 100644 --- a/src/components/Launch/LaunchWorkflowForm/inputHelpers/datetime.ts +++ b/src/components/Launch/LaunchWorkflowForm/inputHelpers/datetime.ts @@ -1,9 +1,10 @@ -import { dateToTimestamp } from 'common/utils'; -import { Core } from 'flyteidl'; +import { dateToTimestamp, timestampToDate } from 'common/utils'; +import { Core, Protobuf } from 'flyteidl'; import { utc as moment } from 'moment'; import { InputValue } from '../types'; -import { allowedDateFormats } from './constants'; +import { allowedDateFormats, primitiveLiteralPaths } from './constants'; import { ConverterInput, InputHelper } from './types'; +import { extractLiteralWithCheck } from './utils'; function parseDate(value: InputValue) { return value instanceof Date @@ -11,6 +12,14 @@ function parseDate(value: InputValue) { : moment(value.toString(), allowedDateFormats).toDate(); } +function fromLiteral(literal: Core.ILiteral): InputValue { + const value = extractLiteralWithCheck( + literal, + primitiveLiteralPaths.scalarDatetime + ); + return timestampToDate(value).toISOString(); +} + function toLiteral({ value }: ConverterInput): Core.ILiteral { const datetime = dateToTimestamp(parseDate(value)); return { @@ -26,6 +35,7 @@ function validate({ value }: ConverterInput) { } export const datetimeHelper: InputHelper = { + fromLiteral, toLiteral, validate }; diff --git a/src/components/Launch/LaunchWorkflowForm/inputHelpers/duration.ts b/src/components/Launch/LaunchWorkflowForm/inputHelpers/duration.ts index 24fa4af73..8af578678 100644 --- a/src/components/Launch/LaunchWorkflowForm/inputHelpers/duration.ts +++ b/src/components/Launch/LaunchWorkflowForm/inputHelpers/duration.ts @@ -1,7 +1,18 @@ -import { millisecondsToDuration } from 'common/utils'; -import { Core } from 'flyteidl'; +import { durationToMilliseconds, millisecondsToDuration } from 'common/utils'; +import { Core, Protobuf } from 'flyteidl'; +import { InputValue } from '../types'; +import { primitiveLiteralPaths } from './constants'; import { isValidFloat } from './float'; import { ConverterInput, InputHelper } from './types'; +import { extractLiteralWithCheck } from './utils'; + +function fromLiteral(literal: Core.ILiteral): InputValue { + const value = extractLiteralWithCheck( + literal, + primitiveLiteralPaths.scalarDuration + ); + return durationToMilliseconds(value); +} function toLiteral({ value }: ConverterInput): Core.ILiteral { const parsed = @@ -19,6 +30,7 @@ function validate({ value }: ConverterInput) { } export const durationHelper: InputHelper = { + fromLiteral, toLiteral, validate }; diff --git a/src/components/Launch/LaunchWorkflowForm/inputHelpers/float.ts b/src/components/Launch/LaunchWorkflowForm/inputHelpers/float.ts index cd3d09726..7a3676767 100644 --- a/src/components/Launch/LaunchWorkflowForm/inputHelpers/float.ts +++ b/src/components/Launch/LaunchWorkflowForm/inputHelpers/float.ts @@ -1,6 +1,15 @@ import { Core } from 'flyteidl'; import { InputValue } from '../types'; +import { primitiveLiteralPaths } from './constants'; import { ConverterInput, InputHelper } from './types'; +import { extractLiteralWithCheck } from './utils'; + +function fromLiteral(literal: Core.ILiteral): InputValue { + return extractLiteralWithCheck( + literal, + primitiveLiteralPaths.scalarFloat + ); +} function toLiteral({ value }: ConverterInput): Core.ILiteral { const floatValue = @@ -27,6 +36,7 @@ function validate({ value }: ConverterInput) { } export const floatHelper: InputHelper = { + fromLiteral, toLiteral, validate }; diff --git a/src/components/Launch/LaunchWorkflowForm/inputHelpers/inputHelpers.ts b/src/components/Launch/LaunchWorkflowForm/inputHelpers/inputHelpers.ts index 82f1c4232..2c9a9531f 100644 --- a/src/components/Launch/LaunchWorkflowForm/inputHelpers/inputHelpers.ts +++ b/src/components/Launch/LaunchWorkflowForm/inputHelpers/inputHelpers.ts @@ -1,10 +1,14 @@ import { ValueError } from 'errors'; import { Core } from 'flyteidl'; -import { InputProps } from '../types'; +import { Literal } from 'models'; +import { InputProps, InputTypeDefinition, InputValue } from '../types'; import { literalNone } from './constants'; import { getHelperForInput } from './getHelperForInput'; type ToLiteralParams = Pick; +/** Converts a type/InputValue combination to a `Core.ILiteral` which can be + * submitted to Admin for creating an execution. + */ export function inputToLiteral(input: ToLiteralParams): Core.ILiteral { if (input.value == null) { return literalNone(); @@ -15,9 +19,50 @@ export function inputToLiteral(input: ToLiteralParams): Core.ILiteral { return toLiteral({ value, typeDefinition }); } -type ValidationParams = Pick; +/** Generates the default value (if any) for a given type. */ +export function defaultValueForInputType( + typeDefinition: InputTypeDefinition +): InputValue | undefined { + return getHelperForInput(typeDefinition.type).defaultValue; +} + +/** Converts a Core.ILiteral to an InputValue which can be used to populate + * a form control. + */ +export function literalToInputValue( + typeDefinition: InputTypeDefinition, + literal: Core.ILiteral +): InputValue | undefined { + const { defaultValue, fromLiteral } = getHelperForInput( + typeDefinition.type + ); + + if (literal.scalar && literal.scalar.noneType) { + return undefined; + } + + try { + return fromLiteral(literal, typeDefinition); + } catch (e) { + // If something goes wrong (most likely malformed default value input), + // we'll return the system default value. + console.debug((e as Error).message); + return defaultValue; + } +} + +type ValidationParams = Pick< + InputProps, + 'name' | 'required' | 'typeDefinition' | 'value' +>; +/** Validates a given InputValue based on rules for the provided type. Returns + * void if no errors, throws an error otherwise. + */ export function validateInput(input: ValidationParams) { if (input.value == null) { + if (input.required) { + throw new ValueError(input.name, 'Value is required'); + } return; } diff --git a/src/components/Launch/LaunchWorkflowForm/inputHelpers/integer.ts b/src/components/Launch/LaunchWorkflowForm/inputHelpers/integer.ts index 624203294..2a962c167 100644 --- a/src/components/Launch/LaunchWorkflowForm/inputHelpers/integer.ts +++ b/src/components/Launch/LaunchWorkflowForm/inputHelpers/integer.ts @@ -1,10 +1,19 @@ import { Core } from 'flyteidl'; import * as Long from 'long'; import { InputValue } from '../types'; +import { primitiveLiteralPaths } from './constants'; import { ConverterInput, InputHelper } from './types'; +import { extractLiteralWithCheck } from './utils'; const integerRegexPattern = /^-?[0-9]+$/; +function fromLiteral(literal: Core.ILiteral): InputValue { + return extractLiteralWithCheck( + literal, + primitiveLiteralPaths.scalarInteger + ).toString(); +} + function toLiteral({ value }: ConverterInput): Core.ILiteral { const integer = value instanceof Long ? value : Long.fromString(value.toString()); @@ -33,6 +42,7 @@ function validate({ value }: ConverterInput) { } export const integerHelper: InputHelper = { + fromLiteral, toLiteral, validate }; diff --git a/src/components/Launch/LaunchWorkflowForm/inputHelpers/none.ts b/src/components/Launch/LaunchWorkflowForm/inputHelpers/none.ts index 4c9230ef1..273679c21 100644 --- a/src/components/Launch/LaunchWorkflowForm/inputHelpers/none.ts +++ b/src/components/Launch/LaunchWorkflowForm/inputHelpers/none.ts @@ -2,6 +2,7 @@ import { literalNone } from './constants'; import { InputHelper } from './types'; export const noneHelper: InputHelper = { + fromLiteral: () => undefined, toLiteral: literalNone, validate: () => {} }; diff --git a/src/components/Launch/LaunchWorkflowForm/inputHelpers/string.ts b/src/components/Launch/LaunchWorkflowForm/inputHelpers/string.ts index d98c5acf3..46364e8bf 100644 --- a/src/components/Launch/LaunchWorkflowForm/inputHelpers/string.ts +++ b/src/components/Launch/LaunchWorkflowForm/inputHelpers/string.ts @@ -1,5 +1,15 @@ import { Core } from 'flyteidl'; +import { InputValue } from '../types'; +import { primitiveLiteralPaths } from './constants'; import { ConverterInput, InputHelper } from './types'; +import { extractLiteralWithCheck } from './utils'; + +function fromLiteral(literal: Core.ILiteral): InputValue { + return extractLiteralWithCheck( + literal, + primitiveLiteralPaths.scalarString + ); +} function toLiteral({ value }: ConverterInput): Core.ILiteral { const stringValue = typeof value === 'string' ? value : value.toString(); @@ -13,6 +23,7 @@ function validate({ value }: ConverterInput) { } export const stringHelper: InputHelper = { + fromLiteral, toLiteral, validate }; diff --git a/src/components/Launch/LaunchWorkflowForm/inputHelpers/test/inputHelpers.test.ts b/src/components/Launch/LaunchWorkflowForm/inputHelpers/test/inputHelpers.test.ts index eb65630e7..2fe8bcb98 100644 --- a/src/components/Launch/LaunchWorkflowForm/inputHelpers/test/inputHelpers.test.ts +++ b/src/components/Launch/LaunchWorkflowForm/inputHelpers/test/inputHelpers.test.ts @@ -1,6 +1,17 @@ +import { Core } from 'flyteidl'; import { InputProps, InputType } from '../../types'; -import { inputToLiteral, validateInput } from '../inputHelpers'; -import { literalTestCases, validityTestCases } from './testCases'; +import { literalNone } from '../constants'; +import { getHelperForInput } from '../getHelperForInput'; +import { + inputToLiteral, + literalToInputValue, + validateInput +} from '../inputHelpers'; +import { + literalTestCases, + literalToInputTestCases, + validityTestCases +} from './testCases'; const baseInputProps: InputProps = { description: 'test', @@ -34,10 +45,97 @@ function makeNestedCollectionInput(type: InputType, value: string): InputProps { }; } +describe('literalToInputValue', () => { + describe('Primitives', () => { + literalToInputTestCases.map(([type, input, output]) => + it(`should correctly convert ${type}: ${JSON.stringify( + input.scalar!.primitive + )}`, () => { + const result = literalToInputValue({ type }, input); + expect(result).toEqual(output); + }) + ); + + [ + InputType.Collection, + InputType.Datetime, + InputType.Duration, + InputType.Float, + InputType.Integer, + InputType.String + ].map(type => + it(`should convert None value for ${type} to undefined`, () => { + expect( + literalToInputValue({ type }, literalNone()) + ).toBeUndefined(); + }) + ); + + it('should correctly convert noneType to undefined', () => { + expect( + literalToInputValue({ type: InputType.None }, literalNone()) + ).toEqual(undefined); + }); + }); + + describe('Collections', () => { + literalToInputTestCases.map(([type, input, output]) => + it(`should correctly convert collection of ${type}: ${JSON.stringify( + input.scalar!.primitive + )}`, () => { + const collection: Core.ILiteral = { + collection: { + // Duplicate it to test comma separation + literals: [input, input] + } + }; + const stringifiedValue = + output === undefined ? '' : output.toString(); + const expectedString = `[${stringifiedValue},${stringifiedValue}]`; + const result = literalToInputValue( + { type: InputType.Collection, subtype: { type } }, + collection + ); + expect(result).toEqual(expectedString); + }) + ); + + it('should return empty for noneType literals', () => { + const collection: Core.ILiteral = { + collection: { + // Duplicate it to test comma separation + literals: [literalNone(), literalNone()] + } + }; + + expect( + literalToInputValue( + { + type: InputType.Collection, + subtype: { type: InputType.None } + }, + collection + ) + ).toEqual('[]'); + }); + }); + + it('should return system default if parsing literal fails', () => { + const { defaultValue } = getHelperForInput(InputType.Boolean); + expect( + literalToInputValue( + { type: InputType.Boolean }, + // Invalid boolean input value because it uses the string field + { scalar: { primitive: { stringValue: 'whoops' } } } + ) + ).toEqual(defaultValue); + }); +}); + describe('inputToLiteral', () => { describe('Primitives', () => { literalTestCases.map(([type, input, output]) => - it(`Should correctly convert ${type}: ${input} (${typeof input})`, () => { + it(`should correctly convert ${type}: ${input} (${typeof input})`, () => { const result = inputToLiteral(makeSimpleInput(type, input)); expect(result.scalar!.primitive).toEqual(output); }) @@ -55,7 +153,7 @@ describe('inputToLiteral', () => { value = `"${input}"`; } - it(`Should correctly convert collection of type ${type}: [${value}] (${typeof input})`, () => { + it(`should correctly convert collection of type ${type}: [${value}] (${typeof input})`, () => { const result = inputToLiteral( makeCollectionInput(type, `[${value}]`) ); @@ -64,7 +162,7 @@ describe('inputToLiteral', () => { ).toEqual(output); }); - it(`Should correctly convert nested collection of type ${type}: [[${value}]] (${typeof input})`, () => { + it(`should correctly convert nested collection of type ${type}: [[${value}]] (${typeof input})`, () => { const result = inputToLiteral( makeNestedCollectionInput(type, `[[${value}]]`) ); @@ -87,7 +185,7 @@ describe('inputToLiteral', () => { InputType.Struct, InputType.Unknown ].map(type => - it(`Should return empty value for type: ${type}`, () => { + it(`should return empty value for type: ${type}`, () => { expect( inputToLiteral(makeSimpleInput(type, '')).scalar ).toEqual({ noneType: {} }); @@ -137,4 +235,12 @@ describe('validateInput', () => { describe('string', () => { generateValidityTests(InputType.String, validityTestCases.string); }); + + it('should throw errors for missing required values', () => { + const [type, input] = literalTestCases[0]; + const simpleInput = makeSimpleInput(type, input); + simpleInput.required = true; + delete simpleInput.value; + expect(() => validateInput(simpleInput)).toThrowError(); + }); }); diff --git a/src/components/Launch/LaunchWorkflowForm/inputHelpers/test/testCases.ts b/src/components/Launch/LaunchWorkflowForm/inputHelpers/test/testCases.ts index 081b40bff..8524c21c3 100644 --- a/src/components/Launch/LaunchWorkflowForm/inputHelpers/test/testCases.ts +++ b/src/components/Launch/LaunchWorkflowForm/inputHelpers/test/testCases.ts @@ -1,7 +1,8 @@ import { dateToTimestamp, millisecondsToDuration } from 'common/utils'; import { Core } from 'flyteidl'; import * as Long from 'long'; -import { InputType } from '../../types'; +import { primitiveLiteral } from '../../__mocks__/utils'; +import { InputType, InputValue } from '../../types'; // Defines type of value, input, and expected value of innermost `IScalar` type PrimitiveTestParams = [InputType, any, Core.IPrimitive]; @@ -97,3 +98,59 @@ export const literalTestCases: PrimitiveTestParams[] = [ [InputType.String, '', { stringValue: '' }], [InputType.String, 'abcdefg', { stringValue: 'abcdefg' }] ]; + +type InputToLiteralTestParams = [ + InputType, + Core.ILiteral, + InputValue | undefined +]; +export const literalToInputTestCases: InputToLiteralTestParams[] = [ + [InputType.Boolean, primitiveLiteral({ boolean: true }), true], + [InputType.Boolean, primitiveLiteral({ boolean: false }), false], + [ + InputType.Datetime, + primitiveLiteral({ + datetime: dateToTimestamp(new Date(validDateString)) + }), + validDateString + ], + [ + InputType.Duration, + primitiveLiteral({ duration: millisecondsToDuration(0) }), + 0 + ], + [ + InputType.Duration, + primitiveLiteral({ duration: millisecondsToDuration(10000) }), + 10000 + ], + [ + InputType.Duration, + primitiveLiteral({ duration: millisecondsToDuration(1.5) }), + 1.5 + ], + [InputType.Float, primitiveLiteral({ floatValue: 0 }), 0], + [InputType.Float, primitiveLiteral({ floatValue: -1.5 }), -1.5], + [InputType.Float, primitiveLiteral({ floatValue: 1.5 }), 1.5], + [InputType.Float, primitiveLiteral({ floatValue: 1.25e10 }), 1.25e10], + // Integers will be returned as strings because they may overflow numbers + [InputType.Integer, primitiveLiteral({ integer: Long.fromNumber(0) }), '0'], + [InputType.Integer, primitiveLiteral({ integer: Long.fromNumber(1) }), '1'], + [ + InputType.Integer, + primitiveLiteral({ integer: Long.fromNumber(-1) }), + '-1' + ], + [ + InputType.Integer, + primitiveLiteral({ integer: Long.MAX_VALUE }), + Long.MAX_VALUE.toString() + ], + [ + InputType.Integer, + primitiveLiteral({ integer: Long.MIN_VALUE }), + Long.MIN_VALUE.toString() + ], + [InputType.String, primitiveLiteral({ stringValue: '' }), ''], + [InputType.String, primitiveLiteral({ stringValue: 'abcdefg' }), 'abcdefg'] +]; diff --git a/src/components/Launch/LaunchWorkflowForm/inputHelpers/types.ts b/src/components/Launch/LaunchWorkflowForm/inputHelpers/types.ts index e7b28f517..fb69991f9 100644 --- a/src/components/Launch/LaunchWorkflowForm/inputHelpers/types.ts +++ b/src/components/Launch/LaunchWorkflowForm/inputHelpers/types.ts @@ -6,9 +6,17 @@ export interface ConverterInput { typeDefinition: InputTypeDefinition; } -export type LiteralConverterFn = (input: ConverterInput) => Core.ILiteral; +export type InputToLiteralConverterFn = ( + input: ConverterInput +) => Core.ILiteral; +export type LiteralToInputConterterFn = ( + literal: Core.ILiteral, + typeDefinition: InputTypeDefinition +) => InputValue | undefined; export interface InputHelper { - toLiteral: LiteralConverterFn; + defaultValue?: InputValue; + toLiteral: InputToLiteralConverterFn; + fromLiteral: LiteralToInputConterterFn; /** Will throw in the case of a failed validation */ validate: (input: ConverterInput) => void; } diff --git a/src/components/Launch/LaunchWorkflowForm/inputHelpers/utils.ts b/src/components/Launch/LaunchWorkflowForm/inputHelpers/utils.ts new file mode 100644 index 000000000..ace48caa9 --- /dev/null +++ b/src/components/Launch/LaunchWorkflowForm/inputHelpers/utils.ts @@ -0,0 +1,16 @@ +import { Core } from 'flyteidl'; +import { get } from 'lodash'; + +/** Performs a deep get of `path` on the given `Core.ILiteral`. Will throw + * if the given property doesn't exist. + */ +export function extractLiteralWithCheck( + literal: Core.ILiteral, + path: string +): T { + const value = get(literal, path); + if (value === undefined) { + throw new Error(`Failed to extract literal value with path ${path}`); + } + return value as T; +} diff --git a/src/components/Launch/LaunchWorkflowForm/test/LaunchWorkflowForm.test.tsx b/src/components/Launch/LaunchWorkflowForm/test/LaunchWorkflowForm.test.tsx index 9f07640cd..86bc00eb2 100644 --- a/src/components/Launch/LaunchWorkflowForm/test/LaunchWorkflowForm.test.tsx +++ b/src/components/Launch/LaunchWorkflowForm/test/LaunchWorkflowForm.test.tsx @@ -14,6 +14,7 @@ import { APIContext } from 'components/data/apiContext'; import { muiTheme } from 'components/Theme'; import { Core } from 'flyteidl'; import { get, mapValues } from 'lodash'; +import * as Long from 'long'; import { createWorkflowExecution, CreateWorkflowExecutionArguments, @@ -23,6 +24,7 @@ import { LaunchPlan, listLaunchPlans, listWorkflows, + Literal, NamedEntityIdentifier, Variable, Workflow @@ -375,10 +377,7 @@ describe('LaunchWorkflowForm', () => { }); describe('Input Values', () => { - /* TODO: Un-skip this when https://github.com/lyft/flyte/issues/18 - * is fixed. - */ - it.skip('Should send false for untouched toggles', async () => { + it('Should send false for untouched toggles', async () => { let inputs: Core.ILiteralMap = {}; mockCreateWorkflowExecution.mockImplementation( ({ @@ -403,6 +402,45 @@ describe('LaunchWorkflowForm', () => { ); expect(value).toBe(false); }); + + it('should use default values when provided', async () => { + // Add defaults for the string/integer inputs and check that they are + // correctly populated + const parameters = mockLaunchPlans[0].closure!.expectedInputs + .parameters; + parameters[stringInputName].default = { + scalar: { primitive: { stringValue: 'abc' } } + } as Literal; + parameters[integerInputName].default = { + scalar: { primitive: { integer: Long.fromNumber(10000) } } + } as Literal; + mockGetLaunchPlan.mockResolvedValue(mockLaunchPlans[0]); + + const { getByLabelText } = renderForm(); + await wait(); + + expect( + getByLabelText(stringInputName, { exact: false }) + ).toHaveValue('abc'); + expect( + getByLabelText(integerInputName, { exact: false }) + ).toHaveValue('10000'); + }); + + it('should decorate labels for required inputs', async () => { + // Add defaults for the string/integer inputs and check that they are + // correctly populated + const parameters = mockLaunchPlans[0].closure!.expectedInputs + .parameters; + parameters[stringInputName].required = true; + mockGetLaunchPlan.mockResolvedValue(mockLaunchPlans[0]); + + const { getByText } = renderForm(); + await wait(); + expect( + getByText(stringInputName, { exact: false }).textContent + ).toContain('*'); + }); }); }); }); diff --git a/src/components/Launch/LaunchWorkflowForm/types.ts b/src/components/Launch/LaunchWorkflowForm/types.ts index 046d76604..53a9d93b5 100644 --- a/src/components/Launch/LaunchWorkflowForm/types.ts +++ b/src/components/Launch/LaunchWorkflowForm/types.ts @@ -77,7 +77,10 @@ export interface InputProps { onChange: InputChangeHandler; } -export type ParsedInput = Pick< - InputProps, - 'description' | 'label' | 'name' | 'required' | 'typeDefinition' ->; +export interface ParsedInput + extends Pick< + InputProps, + 'description' | 'label' | 'name' | 'required' | 'typeDefinition' + > { + defaultValue?: InputValue; +} diff --git a/src/components/Launch/LaunchWorkflowForm/useFormInputsState.ts b/src/components/Launch/LaunchWorkflowForm/useFormInputsState.ts index 87ca216ba..8646374fb 100644 --- a/src/components/Launch/LaunchWorkflowForm/useFormInputsState.ts +++ b/src/components/Launch/LaunchWorkflowForm/useFormInputsState.ts @@ -1,5 +1,4 @@ import { useDebouncedValue } from 'components/hooks/useDebouncedValue'; -import { ValidationError, ValueError } from 'errors'; import { Core } from 'flyteidl'; import { useEffect, useState } from 'react'; import { validateInput } from './inputHelpers/inputHelpers'; @@ -19,15 +18,17 @@ interface FormInputsState { } function useFormInputState(parsedInput: ParsedInput): FormInputState { - const [value, setValue] = useState(); + const [value, setValue] = useState( + parsedInput.defaultValue + ); const [error, setError] = useState(); const validationValue = useDebouncedValue(value, debounceDelay); const validate = () => { - const { name, typeDefinition } = parsedInput; + const { name, required, typeDefinition } = parsedInput; try { - validateInput({ name, typeDefinition, value }); + validateInput({ name, required, typeDefinition, value }); setError(undefined); return true; } catch (e) { diff --git a/src/components/Launch/LaunchWorkflowForm/useLaunchWorkflowFormState.ts b/src/components/Launch/LaunchWorkflowForm/useLaunchWorkflowFormState.ts index 7067a4eac..b5a16bcaf 100644 --- a/src/components/Launch/LaunchWorkflowForm/useLaunchWorkflowFormState.ts +++ b/src/components/Launch/LaunchWorkflowForm/useLaunchWorkflowFormState.ts @@ -18,6 +18,10 @@ import { } from 'models'; import { useEffect, useMemo, useRef, useState } from 'react'; import { history, Routes } from 'routes'; +import { + defaultValueForInputType, + literalToInputValue +} from './inputHelpers/inputHelpers'; import { SearchableSelectorOption } from './SearchableSelector'; import { LaunchWorkflowFormInputsRef, @@ -47,7 +51,7 @@ function getInputs(workflow: Workflow, launchPlan: LaunchPlan): ParsedInput[] { const launchPlanInputs = launchPlan.closure.expectedInputs.parameters; return sortedObjectEntries(launchPlanInputs).map(value => { const [name, parameter] = value; - const required = !!(parameter.default || parameter.required); + const required = !!parameter.required; const workflowInput = workflowInputs[name]; const description = workflowInput && workflowInput.description @@ -57,11 +61,16 @@ function getInputs(workflow: Workflow, launchPlan: LaunchPlan): ParsedInput[] { const typeDefinition = getInputDefintionForLiteralType( parameter.var.type ); - const label = formatLabelWithType(name, typeDefinition); + const typeLabel = formatLabelWithType(name, typeDefinition); + const label = required ? `${typeLabel}*` : typeLabel; + + const defaultValue = + parameter.default !== undefined + ? literalToInputValue(typeDefinition, parameter.default) + : defaultValueForInputType(typeDefinition); - // TODO: - // Extract default value for more specific type (maybe just for simple) return { + defaultValue, description, label, name, diff --git a/src/models/Common/types.ts b/src/models/Common/types.ts index 534b1d21e..895c130a8 100644 --- a/src/models/Common/types.ts +++ b/src/models/Common/types.ts @@ -1,4 +1,5 @@ import { Admin, Core, Protobuf } from 'flyteidl'; +import { Collection } from 'react-virtualized'; /* --- BEGIN flyteidl type aliases --- */ /** These are types shared across multiple sections of the data model. Most of @@ -59,6 +60,8 @@ export interface Error extends RequiredNonNullable {} export interface Literal extends Core.Literal { value: keyof Core.ILiteral; + collection?: Core.ILiteralCollection; + map?: Core.ILiteralMap; scalar?: Scalar; } @@ -72,6 +75,7 @@ export interface LiteralMapBlob extends Admin.ILiteralMapBlob { } export interface Scalar extends Core.IScalar { + primitive?: Primitive; value: keyof Core.IScalar; }