Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DRAFT: refactor(entities,resources): Decouple parsing from entities hook into a separate serializer class #1671

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ describe('routeIdValidator', () => {
});

it('should return sucess if the name is unique', () => {
const resource = new CamelRouteResource(camelRouteJson);
const resource = new CamelRouteResource([camelRouteJson]);
const visualEntities = resource.getVisualEntities();
jest.spyOn(visualEntities[0], 'getId').mockReturnValue('flow-1234');

Expand All @@ -28,7 +28,7 @@ describe('routeIdValidator', () => {
});

it('should return an error if the name is not unique', () => {
const resource = new CamelRouteResource(camelRouteJson);
const resource = new CamelRouteResource([camelRouteJson]);
const visualEntities = resource.getVisualEntities();
jest.spyOn(visualEntities[0], 'getId').mockReturnValue('flow-1234');

Expand All @@ -39,7 +39,7 @@ describe('routeIdValidator', () => {
});

it('should return an error if the name is not a valid URI', () => {
const resource = new CamelRouteResource(camelRouteJson);
const resource = new CamelRouteResource([camelRouteJson]);
const visualEntities = resource.getVisualEntities();
jest.spyOn(visualEntities[0], 'getId').mockReturnValue('flow-1234');

Expand All @@ -50,7 +50,7 @@ describe('routeIdValidator', () => {
});

it('should return an error if the name is not unique neither a valid URI', () => {
const resource = new CamelRouteResource(camelRouteJson);
const resource = new CamelRouteResource([camelRouteJson]);
const visualEntities = resource.getVisualEntities();
jest.spyOn(visualEntities[0], 'getId').mockReturnValue('The amazing Route');

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ describe('Canvas', () => {
});

it('should be able to delete the routes', async () => {
const camelResource = new CamelRouteResource(camelRouteJson);
const camelResource = new CamelRouteResource([camelRouteJson]);
const routeEntities = camelResource.getVisualEntities();
const removeSpy = jest.spyOn(camelResource, 'removeEntity');

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ describe('ItemEnableAllSteps', () => {
});

it('should NOT render an ItemEnableAllSteps if there are not at least 2 or more disabled steps', () => {
const camelResource = new CamelRouteResource(camelRouteJson);
const camelResource = new CamelRouteResource([camelRouteJson]);
const visualEntity = camelResource.getVisualEntities()[0];
const { nodes, edges } = FlowService.getFlowDiagram(visualEntity.toVizNode());

Expand Down Expand Up @@ -50,7 +50,7 @@ describe('ItemEnableAllSteps', () => {
});

it('should call updateModel and updateEntitiesFromCamelResource on click', async () => {
const camelResource = new CamelRouteResource(camelRouteWithDisabledSteps);
const camelResource = new CamelRouteResource([camelRouteWithDisabledSteps]);
const visualEntity = camelResource.getVisualEntities()[0];
const { nodes, edges } = FlowService.getFlowDiagram(visualEntity.toVizNode());

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ describe('NodeContextMenu', () => {
});

it('should render an ItemEnableAllSteps', () => {
const camelResource = new CamelRouteResource(camelRouteWithDisabledSteps);
const camelResource = new CamelRouteResource([camelRouteWithDisabledSteps]);
const visualEntity = camelResource.getVisualEntities()[0];
const { nodes, edges } = FlowService.getFlowDiagram(visualEntity.toVizNode());

Expand Down
69 changes: 3 additions & 66 deletions packages/ui/src/hooks/entities.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -59,12 +59,14 @@ describe('useEntities', () => {
expect(notifierSpy).toHaveBeenCalledWith('entities:updated', camelRouteYaml_1_1_updated);
});

it('should notifiy subscribers when the entities are updated', () => {
it('should notify subscribers when the entities are updated', () => {
const notifierSpy = jest.spyOn(eventNotifier, 'next');
const { result } = renderHook(() => useEntities());

act(() => {
console.log('result.current.camelResource:', result.current.camelResource);
result.current.camelResource.addNewEntity();
console.log('result.current.camelResource:', result.current.camelResource);
result.current.updateSourceCodeFromEntities();
});

Expand Down Expand Up @@ -162,69 +164,4 @@ describe('useEntities', () => {
`,
);
});

describe('comments', () => {
it(`should store code's comments`, () => {
const code = `# This is a comment
# An indented comment

- route:
id: route-1234
from:
id: from-1234
uri: timer
parameters:
period: "1000"
timerName: template
# This comment won't be stored
steps:
- log:
id: log-1234
message: \${body}
`;

const { result } = renderHook(() => useEntities());

act(() => {
eventNotifier.next('code:updated', code);
});

expect(result.current.camelResource.getComments()).toEqual([
'# This is a comment',
' # An indented comment',
'',
]);
});

it('should add comments to the source code', () => {
const notifierSpy = jest.spyOn(eventNotifier, 'next');
const { result } = renderHook(() => useEntities());

act(() => {
result.current.camelResource.setComments(['# This is a comment', ' # An indented comment', '']);
result.current.camelResource.addNewEntity();
result.current.updateSourceCodeFromEntities();
});

expect(notifierSpy).toHaveBeenCalledWith(
'entities:updated',
`# This is a comment
# An indented comment

- route:
id: route-1234
from:
id: from-1234
uri: timer
parameters:
period: "1000"
timerName: template
steps:
- log:
id: log-1234
message: \${body}
`,
);
});
});
});
45 changes: 8 additions & 37 deletions packages/ui/src/hooks/entities.ts
Original file line number Diff line number Diff line change
@@ -1,21 +1,10 @@
import { useCallback, useLayoutEffect, useMemo, useState } from 'react';
import { parse, stringify } from 'yaml';
import { CamelResource, SourceSchemaType, createCamelResource } from '../models/camel';

import { CamelResource, SourceSchemaType } from '../models/camel';
import { BaseCamelEntity } from '../models/camel/entities';
import { BaseVisualCamelEntity } from '../models/visualization/base-visual-entity';
import { EventNotifier } from '../utils';

/**
* Regular expression to match commented lines, regardless of indentation
* Given the following examples, the regular expression should match the comments:
* ```
* # This is a comment
* # This is an indented comment
*# This is an indented comment
* ```
* The regular expression should match the first three lines
*/
const COMMENTED_LINES_REGEXP = /^\s*#.*$/;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[minor] Please let's move this comment as well

import { CamelResourceFactory } from '../models/camel/camel-resource-factory';

export interface EntitiesContextResult {
entities: BaseCamelEntity[];
Expand Down Expand Up @@ -49,7 +38,7 @@ export interface EntitiesContextResult {

export const useEntities = (): EntitiesContextResult => {
const eventNotifier = EventNotifier.getInstance();
const [camelResource, setCamelResource] = useState<CamelResource>(createCamelResource());
const [camelResource, setCamelResource] = useState<CamelResource>(CamelResourceFactory.createCamelResource());
const [entities, setEntities] = useState<BaseCamelEntity[]>([]);
const [visualEntities, setVisualEntities] = useState<BaseVisualCamelEntity[]>([]);

Expand All @@ -58,36 +47,18 @@ export const useEntities = (): EntitiesContextResult => {
*/
useLayoutEffect(() => {
return eventNotifier.subscribe('code:updated', (code) => {
/** Extract comments from the source code */
const lines = code.split('\n');
const comments: string[] = [];
for (const line of lines) {
if (line.trim() === '' || COMMENTED_LINES_REGEXP.test(line)) {
comments.push(line);
} else {
break;
}
}

const rawEntities = parse(code);
const camelResource = createCamelResource(rawEntities);
camelResource.setComments(comments);
const camelResource = CamelResourceFactory.createCamelResource(code);
const entities = camelResource.getEntities();
const visualEntities = camelResource.getVisualEntities();

setCamelResource(camelResource);
setEntities(entities);
setVisualEntities(visualEntities);
});
}, [eventNotifier]);

const updateSourceCodeFromEntities = useCallback(() => {
let code = stringify(camelResource, { sortMapEntries: camelResource.sortFn, schema: 'yaml-1.1' }) || '';

if (camelResource.getComments().length > 0) {
const comments = camelResource.getComments().join('\n');
code = comments + '\n' + code;
}

const code = camelResource.toString();
eventNotifier.next('entities:updated', code);
}, [camelResource, eventNotifier]);

Expand All @@ -105,7 +76,7 @@ export const useEntities = (): EntitiesContextResult => {

const setCurrentSchemaType = useCallback(
(type: SourceSchemaType) => {
setCamelResource(createCamelResource(type));
setCamelResource(CamelResourceFactory.createCamelResource(type));
updateEntitiesFromCamelResource();
},
[updateEntitiesFromCamelResource],
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`createCamelResource should create an empty KameletResource if no args is specified 1`] = `
exports[`CamelResourceFactory.createCamelResource should create an empty KameletResource if no args is specified 1`] = `
[
{
"from": {
Expand Down
35 changes: 35 additions & 0 deletions packages/ui/src/models/camel/camel-k-resource-factory.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import { SourceSchemaType } from './source-schema-type';
import { CamelResource } from './camel-resource';
import { IntegrationResource } from './integration-resource';
import { KameletResource } from './kamelet-resource';
import { KameletBindingResource } from './kamelet-binding-resource';
import { PipeResource } from './pipe-resource';
import { IKameletDefinition } from '../kamelets-catalog';
import {
Integration as IntegrationType,
KameletBinding as KameletBindingType,
Pipe as PipeType,
} from '@kaoto/camel-catalog/types';

export class CamelKResourceFactory {
static getCamelKResource(
json?: IntegrationType | IKameletDefinition | KameletBindingType | PipeType,
type?: SourceSchemaType,
): CamelResource | undefined {
const jsonRecord = json ? (json as Record<string, unknown>) : {};

if ((jsonRecord && typeof json === 'object' && 'kind' in jsonRecord) || type) {
switch (jsonRecord['kind'] || type) {
case SourceSchemaType.Integration:
return new IntegrationResource(json as IntegrationType);
case SourceSchemaType.Kamelet:
return new KameletResource(json as IKameletDefinition);
case SourceSchemaType.KameletBinding:
return new KameletBindingResource(json as KameletBindingType);
case SourceSchemaType.Pipe:
return new PipeResource(json as PipeType);
}
}
return undefined;
}
}
23 changes: 15 additions & 8 deletions packages/ui/src/models/camel/camel-k-resource.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import { BaseVisualCamelEntityDefinition, CamelResource } from './camel-resource';
import { BaseCamelEntity } from './entities';
import { SourceSchemaType } from './source-schema-type';
import { CamelResourceSerializer, YamlCamelResourceSerializer } from '../../serializers';

export type CamelKType = IntegrationType | IKameletDefinition | KameletBindingType | PipeType;

Expand All @@ -25,14 +26,17 @@

export abstract class CamelKResource implements CamelResource {
static readonly PARAMETERS_ORDER = ['apiVersion', 'kind', 'metadata', 'spec', 'source', 'steps', 'sink'];
// static serializer = new YamlResourceSerializer();
readonly sortFn = createCamelPropertiesSorter(CamelKResource.PARAMETERS_ORDER) as (a: unknown, b: unknown) => number;
protected resource: CamelKType;
private metadata?: MetadataEntity;
private comments: string[] = [];

constructor(resource?: CamelKType) {
if (resource) {
this.resource = resource;
constructor(
parsedResource: unknown,
private readonly serializer: CamelResourceSerializer = new YamlCamelResourceSerializer(),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps we could set it as protected to perform some tests if we want to.

Suggested change
private readonly serializer: CamelResourceSerializer = new YamlCamelResourceSerializer(),
protected readonly serializer: CamelResourceSerializer = new YamlCamelResourceSerializer(),

) {
if (parsedResource) {
this.resource = parsedResource as CamelKType;
} else {
this.resource = {
apiVersion: CAMEL_K_K8S_API_VERSION_V1,
Expand Down Expand Up @@ -94,12 +98,15 @@
getCompatibleComponents(_mode: AddStepMode, _visualEntityData: IVisualizationNodeData): TileFilter | undefined {
return undefined;
}
getSerializer() {
return this.serializer;

Check warning on line 102 in packages/ui/src/models/camel/camel-k-resource.ts

View check run for this annotation

Codecov / codecov/patch

packages/ui/src/models/camel/camel-k-resource.ts#L101-L102

Added lines #L101 - L102 were not covered by tests
}
lordrip marked this conversation as resolved.
Show resolved Hide resolved

setComments(comments: string[]) {
this.comments = comments;
setSerializer(_serializer: CamelResourceSerializer): void {

Check warning on line 105 in packages/ui/src/models/camel/camel-k-resource.ts

View check run for this annotation

Codecov / codecov/patch

packages/ui/src/models/camel/camel-k-resource.ts#L105

Added line #L105 was not covered by tests
/** Not supported by default */
}
Comment on lines +105 to 107
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably same here

Suggested change
setSerializer(_serializer: CamelResourceSerializer): void {
/** Not supported by default */
}


getComments(): string[] {
return this.comments;
toString(): string {
return this.serializer.serialize(this);

Check warning on line 110 in packages/ui/src/models/camel/camel-k-resource.ts

View check run for this annotation

Codecov / codecov/patch

packages/ui/src/models/camel/camel-k-resource.ts#L109-L110

Added lines #L109 - L110 were not covered by tests
}
}
32 changes: 32 additions & 0 deletions packages/ui/src/models/camel/camel-resource-factory.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import { SourceSchemaType } from './source-schema-type';
import { CamelResource } from './camel-resource';
import { CamelResourceSerializer, XmlCamelResourceSerializer, YamlCamelResourceSerializer } from '../../serializers';
import { CamelRouteResource } from './camel-route-resource';
import { CamelKResourceFactory } from './camel-k-resource-factory';
import { CamelYamlDsl, Integration, KameletBinding, Pipe } from '@kaoto/camel-catalog/types';
import { IKameletDefinition } from '../kamelets-catalog';

export class CamelResourceFactory {
/**
* Creates a CamelResource based on the given {@link type} and {@link source}. If
* both are not specified, a default empty {@link CamelRouteResource} is created.
* If only {@link type} is specified, an empty {@link CamelResource} of the given
* {@link type} is created.
* @param type
* @param source
*/
static createCamelResource(source?: string, type?: SourceSchemaType): CamelResource {
const serializer: CamelResourceSerializer = XmlCamelResourceSerializer.isApplicable(source)
? new XmlCamelResourceSerializer()

Check warning on line 20 in packages/ui/src/models/camel/camel-resource-factory.ts

View check run for this annotation

Codecov / codecov/patch

packages/ui/src/models/camel/camel-resource-factory.ts#L20

Added line #L20 was not covered by tests
: new YamlCamelResourceSerializer();

const parsedCode = typeof source === 'string' ? serializer.parse(source) : source;
const resource = CamelKResourceFactory.getCamelKResource(
parsedCode as Integration | KameletBinding | Pipe | IKameletDefinition,
type,
);

if (resource) return resource;
return new CamelRouteResource(parsedCode as CamelYamlDsl, serializer);
}
}
Loading
Loading