Skip to content

Commit

Permalink
Currently, we're removing parameters set by the user that match the d…
Browse files Browse the repository at this point in the history
…efault value intending to have less verbose routes.

There's an issue with the `file` component, detailed as follows:
```
from: file:someFile.txt
parameters:
  noop: true
```

In the previous route, `noop=true` implicitly set `idempotent=true` **IF THE VALUE HAS NOT BEEN SET TO EITHER TRUE OR FALSE**, usually this is OK and doesn't represent a problem.

In case the user wants to set `noop=true` and `idempotent=false` to force the `file` component to re-read the files when the scheduler kicks in, like the following:

```
from: file:someFile.txt
parameters:
  noop: true
  idempotent: false
```

Then we will remove the `idempotent=false` as it is the default value for the `idempotent` parameter, effectively blocking the user from setting a different behavior for the `file` component.

This commit aims to keep values set by the user, even in case they are the default ones.

fixes: #633
  • Loading branch information
lordrip committed Jan 15, 2024
1 parent 091655c commit 66ef2cf
Show file tree
Hide file tree
Showing 6 changed files with 177 additions and 184 deletions.
132 changes: 1 addition & 131 deletions packages/ui/src/components/Visualization/Canvas/CanvasForm.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { IVisualizationNode, VisualComponentSchema } from '../../../models/visua
import { EntitiesContext } from '../../../providers/entities.provider';
import { SchemaService } from '../../Form';
import { CustomAutoFieldDetector } from '../../Form/CustomAutoField';
import { CanvasForm, getNonDefaultProperties, getNonEmptyProperties } from './CanvasForm';
import { CanvasForm } from './CanvasForm';
import { CanvasNode } from './canvas.models';

describe('CanvasForm', () => {
Expand Down Expand Up @@ -191,133 +191,3 @@ describe('CanvasForm', () => {
});
});
});

describe('CanvasForm getNonDefaultProperties()', () => {
const schema = {
type: 'object',
properties: {
parameters: {
properties: {
events: {
type: 'string',
default: 'CREATE,MODIFY,DELETE',
title: 'Events',
},
concurrentConsumers: {
type: 'integer',
default: 1,
title: 'Concurrent Consumers',
},
bridgeErrorHandler: {
type: 'boolean',
default: false,
title: 'Bridge Error Handler',
},
},
},
},
} as unknown as JSONSchemaType<unknown>;

const newModel: Record<string, unknown> = {
id: 'from-7126',
description: 'test',
steps: [],
uri: 'file-watch',
parameters: {
events: 'CREATE',
concurrentConsumers: '1',
bridgeErrorHandler: false,
},
};

const newModelExpected: Record<string, unknown> = {
id: 'from-7126',
description: 'test',
steps: [],
uri: 'file-watch',
parameters: {
events: 'CREATE',
},
};

it('should return only the properties which are different from default', () => {
const newModelClean = getNonDefaultProperties(schema?.properties.parameters.properties, newModel);
expect(newModelClean).toMatchObject(newModelExpected);
});
});

describe('CanvasForm getNonEmptyProperties()', () => {
const schema = {
type: 'object',
properties: {
parameters: {
properties: {
events: {
type: 'string',
default: 'CREATE,MODIFY,DELETE',
title: 'Events',
},
concurrentConsumers: {
type: 'integer',
default: 1,
title: 'Concurrent Consumers',
},
bridgeErrorHandler: {
type: 'boolean',
default: false,
title: 'Bridge Error Handler',
},
exchangePattern: {
type: 'object',
title: 'Exchange Pattern',
},
},
},
},
} as unknown as JSONSchemaType<unknown>;

const newModel: Record<string, unknown> = {
id: 'from-7126',
description: 'test',
steps: [],
uri: 'file-watch',
parameters: {
events: 'CREATE',
concurrentConsumers: '',
bridgeErrorHandler: false,
exchangePattern: {},
},
};

const newModelIntermediate: Record<string, unknown> = {
id: 'from-7126',
description: 'test',
steps: [],
uri: 'file-watch',
parameters: {
events: 'CREATE',
concurrentConsumers: '',
exchangePattern: {},
},
};

const newModelExpected: Record<string, unknown> = {
id: 'from-7126',
description: 'test',
steps: [],
uri: 'file-watch',
parameters: {
events: 'CREATE',
},
};

it('should return only the properties which are different from default', () => {
const newModelClean = getNonDefaultProperties(schema?.properties.parameters.properties, newModel);
expect(newModelClean).toMatchObject(newModelIntermediate);
});

it('should return only the non-empty properties', () => {
const newModelClean = getNonEmptyProperties(newModel);
expect(newModelClean).toMatchObject(newModelExpected);
});
});
54 changes: 1 addition & 53 deletions packages/ui/src/components/Visualization/Canvas/CanvasForm.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { AutoField, AutoFields, AutoForm, ErrorsField } from '@kaoto-next/uniforms-patternfly';
import { Title } from '@patternfly/react-core';
import type { JSONSchemaType } from 'ajv';
import { FunctionComponent, useCallback, useContext, useEffect, useMemo, useRef } from 'react';
import { EntitiesContext } from '../../../providers/entities.provider';
import { ErrorBoundary } from '../../ErrorBoundary';
Expand All @@ -16,46 +15,6 @@ interface CanvasFormProps {
selectedNode: CanvasNode;
}

export function getNonDefaultProperties(
obj1: JSONSchemaType<unknown>,
obj2: Record<string, unknown>,
): Record<string, unknown> {
const newModelUpdated = Object.entries(obj2.parameters as object).reduce(
(acc: [string, unknown][], currentValue: [string, unknown]) => {
if (!(obj1[currentValue[0]]['default'] == currentValue[1])) {
acc.push(currentValue);
}
return acc;
},
[],
);
return { ...obj2, parameters: Object.fromEntries(newModelUpdated) };
}

export function getNonEmptyProperties(obj: Record<string, unknown>): Record<string, unknown> {
const result = Object.entries(obj.parameters as object).reduce(
(acc: [string, unknown][], currentValue: [string, unknown]) => {
switch (typeof currentValue[1]) {
case 'string':
if (currentValue[1].trim().length !== 0) {
acc.push(currentValue);
}
break;
case 'object':
if (Object.keys(currentValue[1] as object).length !== 0) {
acc.push(currentValue);
}
break;
default:
acc.push(currentValue);
}
return acc;
},
[],
);
return { ...obj, parameters: Object.fromEntries(result) };
}

const omitFields = [
'expression',
'dataFormatType',
Expand Down Expand Up @@ -93,18 +52,7 @@ export const CanvasForm: FunctionComponent<CanvasFormProps> = (props) => {

const handleOnChange = useCallback(
(newModel: Record<string, unknown>) => {
if (newModel.parameters === undefined) {
props.selectedNode.data?.vizNode?.updateModel(newModel);
} else {
// newModelNonDefault will contain only those properties that has different value than default.
const newModelNonDefault = getNonDefaultProperties(
props.selectedNode.data?.vizNode?.getComponentSchema()?.schema?.properties.parameters.properties,
newModel,
);
// newModelClean will contain only non empty propertis that has different value than default.
const newModelClean = getNonEmptyProperties(newModelNonDefault);
props.selectedNode.data?.vizNode?.updateModel(newModelClean);
}
props.selectedNode.data?.vizNode?.updateModel(newModel);
entitiesContext?.updateSourceCodeFromEntities();
},
[entitiesContext, props.selectedNode.data?.vizNode, props.selectedNode.id],
Expand Down
56 changes: 56 additions & 0 deletions packages/ui/src/utils/get-non-default-properties.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
import { JSONSchemaType } from 'ajv';
import { getNonDefaultProperties } from './get-non-default-properties';

describe('getNonDefaultProperties()', () => {
const schema = {
type: 'object',
properties: {
parameters: {
properties: {
events: {
type: 'string',
default: 'CREATE,MODIFY,DELETE',
title: 'Events',
},
concurrentConsumers: {
type: 'integer',
default: 1,
title: 'Concurrent Consumers',
},
bridgeErrorHandler: {
type: 'boolean',
default: false,
title: 'Bridge Error Handler',
},
},
},
},
} as unknown as JSONSchemaType<unknown>;

const newModel: Record<string, unknown> = {
id: 'from-7126',
description: 'test',
steps: [],
uri: 'file-watch',
parameters: {
events: 'CREATE',
concurrentConsumers: '1',
bridgeErrorHandler: false,
},
};

const newModelExpected: Record<string, unknown> = {
id: 'from-7126',
description: 'test',
steps: [],
uri: 'file-watch',
parameters: {
events: 'CREATE',
},
};

it('should return only the properties which are different from default', () => {
const newModelClean = getNonDefaultProperties(schema?.properties.parameters.properties, newModel);
expect(newModelClean).toMatchObject(newModelExpected);
});
});
17 changes: 17 additions & 0 deletions packages/ui/src/utils/get-non-default-properties.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import type { JSONSchemaType } from 'ajv';

export function getNonDefaultProperties(
obj1: JSONSchemaType<unknown>,
obj2: Record<string, unknown>,
): Record<string, unknown> {
const newModelUpdated = Object.entries(obj2.parameters as object).reduce(
(acc: [string, unknown][], currentValue: [string, unknown]) => {
if (!(obj1[currentValue[0]]['default'] == currentValue[1])) {
acc.push(currentValue);
}
return acc;
},
[],
);
return { ...obj2, parameters: Object.fromEntries(newModelUpdated) };
}
79 changes: 79 additions & 0 deletions packages/ui/src/utils/get-non-empty-properties.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
import { JSONSchemaType } from 'ajv';
import { getNonDefaultProperties } from './get-non-default-properties';
import { getNonEmptyProperties } from './get-non-empty-properties';

describe('CanvasForm getNonEmptyProperties()', () => {
const schema = {
type: 'object',
properties: {
parameters: {
properties: {
events: {
type: 'string',
default: 'CREATE,MODIFY,DELETE',
title: 'Events',
},
concurrentConsumers: {
type: 'integer',
default: 1,
title: 'Concurrent Consumers',
},
bridgeErrorHandler: {
type: 'boolean',
default: false,
title: 'Bridge Error Handler',
},
exchangePattern: {
type: 'object',
title: 'Exchange Pattern',
},
},
},
},
} as unknown as JSONSchemaType<unknown>;

const newModel: Record<string, unknown> = {
id: 'from-7126',
description: 'test',
steps: [],
uri: 'file-watch',
parameters: {
events: 'CREATE',
concurrentConsumers: '',
bridgeErrorHandler: false,
exchangePattern: {},
},
};

const newModelIntermediate: Record<string, unknown> = {
id: 'from-7126',
description: 'test',
steps: [],
uri: 'file-watch',
parameters: {
events: 'CREATE',
concurrentConsumers: '',
exchangePattern: {},
},
};

const newModelExpected: Record<string, unknown> = {
id: 'from-7126',
description: 'test',
steps: [],
uri: 'file-watch',
parameters: {
events: 'CREATE',
},
};

it('should return only the properties which are different from default', () => {
const newModelClean = getNonDefaultProperties(schema?.properties.parameters.properties, newModel);
expect(newModelClean).toMatchObject(newModelIntermediate);
});

it('should return only the non-empty properties', () => {
const newModelClean = getNonEmptyProperties(newModel);
expect(newModelClean).toMatchObject(newModelExpected);
});
});
23 changes: 23 additions & 0 deletions packages/ui/src/utils/get-non-empty-properties.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
export function getNonEmptyProperties(obj: Record<string, unknown>): Record<string, unknown> {
const result = Object.entries(obj.parameters as object).reduce(
(acc: [string, unknown][], currentValue: [string, unknown]) => {
switch (typeof currentValue[1]) {
case 'string':
if (currentValue[1].trim().length !== 0) {
acc.push(currentValue);
}
break;
case 'object':
if (Object.keys(currentValue[1] as object).length !== 0) {
acc.push(currentValue);
}
break;
default:
acc.push(currentValue);
}
return acc;
},
[],
);
return { ...obj, parameters: Object.fromEntries(result) };
}

0 comments on commit 66ef2cf

Please sign in to comment.