Skip to content

Commit

Permalink
refactor: Apply code review feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
mmelko committed Feb 18, 2025
1 parent 5f8bb32 commit 279cbdf
Show file tree
Hide file tree
Showing 20 changed files with 190 additions and 241 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
#serializer-selector {
width: 20rem;
}
Original file line number Diff line number Diff line change
@@ -1,23 +1,22 @@
import { FunctionComponent, RefObject, useContext, useState } from 'react';
import { MenuToggle, Select, SelectList, SelectOption } from '@patternfly/react-core';
import { XmlCamelResourceSerializer, YamlCamelResourceSerializer } from '../../../../serializers';
import { SerializerType } from '../../../../serializers';
import { EntitiesContext } from '../../../../providers';
import './SerializerSelector.scss';

export const SerializerSelector: FunctionComponent = () => {
const { camelResource, updateSourceCodeFromEntities } = useContext(EntitiesContext)!;
const [isOpen, setIsOpen] = useState(false);
const [selected, setSelected] = useState<'XML' | 'YAML'>(camelResource.getSerializer().getLabel() as 'XML' | 'YAML');
const [selected, setSelected] = useState<SerializerType>(camelResource.getSerializerType());

Check warning on line 10 in packages/ui/src/components/Visualization/ContextToolbar/SerializerSelector/SerializerSelector.tsx

View check run for this annotation

Codecov / codecov/patch

packages/ui/src/components/Visualization/ContextToolbar/SerializerSelector/SerializerSelector.tsx#L7-L10

Added lines #L7 - L10 were not covered by tests

function onSelect(serializer: string) {
setSelected(serializer as 'XML' | 'YAML');
const onSelect = (serializer: SerializerType) => {
setSelected(serializer);

Check warning on line 13 in packages/ui/src/components/Visualization/ContextToolbar/SerializerSelector/SerializerSelector.tsx

View check run for this annotation

Codecov / codecov/patch

packages/ui/src/components/Visualization/ContextToolbar/SerializerSelector/SerializerSelector.tsx#L12-L13

Added lines #L12 - L13 were not covered by tests
if (serializer !== selected) {
camelResource.setSerializer(
serializer === 'XML' ? new XmlCamelResourceSerializer() : new YamlCamelResourceSerializer(),
);
camelResource.setSerializer(serializer);
updateSourceCodeFromEntities();
setIsOpen(false);

Check warning on line 17 in packages/ui/src/components/Visualization/ContextToolbar/SerializerSelector/SerializerSelector.tsx

View check run for this annotation

Codecov / codecov/patch

packages/ui/src/components/Visualization/ContextToolbar/SerializerSelector/SerializerSelector.tsx#L15-L17

Added lines #L15 - L17 were not covered by tests
}
}
};

const toggle = (toggleRef: RefObject<HTMLButtonElement>) => (
<MenuToggle
Expand All @@ -36,27 +35,30 @@ export const SerializerSelector: FunctionComponent = () => {
<Select
id="serializer-list-select"
isOpen={isOpen}
onSelect={(_event, value) => onSelect(value as string)}
onOpenChange={(isOpen) => setIsOpen(isOpen)}
onSelect={(_event, value) => {
onSelect(value as SerializerType);

Check warning on line 39 in packages/ui/src/components/Visualization/ContextToolbar/SerializerSelector/SerializerSelector.tsx

View check run for this annotation

Codecov / codecov/patch

packages/ui/src/components/Visualization/ContextToolbar/SerializerSelector/SerializerSelector.tsx#L38-L39

Added lines #L38 - L39 were not covered by tests
}}
onOpenChange={(isOpen) => {
setIsOpen(isOpen);

Check warning on line 42 in packages/ui/src/components/Visualization/ContextToolbar/SerializerSelector/SerializerSelector.tsx

View check run for this annotation

Codecov / codecov/patch

packages/ui/src/components/Visualization/ContextToolbar/SerializerSelector/SerializerSelector.tsx#L41-L42

Added lines #L41 - L42 were not covered by tests
}}
toggle={toggle}
style={{ width: '20rem' }}
>
<SelectList>
<SelectOption
key={`serializer-yaml`}
data-testid={`serializer-yaml`}
itemId={'XML'}
value={'XML'}
isDisabled={selected === 'XML'}
key="serializer-yaml"
data-testid="serializer-yaml"
itemId="XML"
value={SerializerType.XML}
isDisabled={selected === SerializerType.XML}
>
XML
</SelectOption>
<SelectOption
key={`serializer-xml`}
data-testid={`serializer-yaml`}
itemId={'YAML'}
value={'YAML'}
isDisabled={selected === 'YAML'}
key="serializer-xml"
data-testid="serializer-yaml"
itemId="YAML"
value={SerializerType.YAML}
isDisabled={selected === SerializerType.YAML}
>
YAML
</SelectOption>
Expand Down
8 changes: 4 additions & 4 deletions packages/ui/src/models/camel/camel-k-resource.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { MetadataEntity } from '../visualization/metadata';
import { BaseVisualCamelEntityDefinition, CamelResource } from './camel-resource';
import { BaseCamelEntity } from './entities';
import { SourceSchemaType } from './source-schema-type';
import { CamelResourceSerializer, YamlCamelResourceSerializer } from '../../serializers';
import { CamelResourceSerializer, SerializerType, YamlCamelResourceSerializer } from '../../serializers';

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

Expand Down Expand Up @@ -97,11 +97,11 @@ export abstract class CamelKResource implements CamelResource {
getCompatibleComponents(_mode: AddStepMode, _visualEntityData: IVisualizationNodeData): TileFilter | undefined {
return undefined;
}
getSerializer() {
return this.serializer;
getSerializerType() {
return this.serializer.getType();

Check warning on line 101 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#L100-L101

Added lines #L100 - L101 were not covered by tests
}

setSerializer(_serializer: CamelResourceSerializer): void {
setSerializer(_serializer: SerializerType): void {

Check warning on line 104 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#L104

Added line #L104 was not covered by tests
/** Not supported by default */
}

Expand Down
6 changes: 3 additions & 3 deletions packages/ui/src/models/camel/camel-resource.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { BeansEntity } from '../visualization/metadata';
import { RouteTemplateBeansEntity } from '../visualization/metadata/routeTemplateBeansEntity';
import { BaseCamelEntity, EntityType } from './entities';
import { SourceSchemaType } from './source-schema-type';
import { CamelResourceSerializer } from '../../serializers';
import { SerializerType } from '../../serializers';

export interface CamelResource {
getVisualEntities(): BaseVisualCamelEntity[];
Expand All @@ -16,8 +16,8 @@ export interface CamelResource {
toString(): string;
getType(): SourceSchemaType;
getCanvasEntityList(): BaseVisualCamelEntityDefinition;
getSerializer(): CamelResourceSerializer;
setSerializer(serializer: CamelResourceSerializer): void;
getSerializerType(): SerializerType;
setSerializer(serializer: SerializerType): void;

/** Components Catalog related methods */
getCompatibleComponents(
Expand Down
16 changes: 9 additions & 7 deletions packages/ui/src/models/camel/camel-route-resource.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { CamelYamlDsl, RouteDefinition } from '@kaoto/camel-catalog/types';
import { TileFilter } from '../../components/Catalog';
import { YamlCamelResourceSerializer } from '../../serializers';
import { SerializerType, XmlCamelResourceSerializer, YamlCamelResourceSerializer } from '../../serializers';
import { CamelResourceSerializer } from '../../serializers/camel-resource-serializer';
import { createCamelPropertiesSorter, isDefined } from '../../utils';
import { CatalogKind } from '../catalog-kind';
Expand Down Expand Up @@ -50,10 +50,11 @@ export class CamelRouteResource implements CamelResource, BeansAwareResource {
) => number;
private entities: BaseCamelEntity[] = [];
private resolvedEntities: BaseVisualCamelEntityDefinition | undefined;
private serializer: CamelResourceSerializer;

constructor(rawEntities?: CamelYamlDsl, serializer?: CamelResourceSerializer) {
this.serializer = serializer ?? new YamlCamelResourceSerializer();
constructor(
rawEntities?: CamelYamlDsl,
private serializer: CamelResourceSerializer = new YamlCamelResourceSerializer(),
) {
if (!rawEntities) return;

const entities = Array.isArray(rawEntities) ? rawEntities : [rawEntities];
Expand Down Expand Up @@ -95,12 +96,13 @@ export class CamelRouteResource implements CamelResource, BeansAwareResource {
return this.resolvedEntities;
}

getSerializer(): CamelResourceSerializer {
return this.serializer;
getSerializerType() {
return this.serializer.getType();

Check warning on line 100 in packages/ui/src/models/camel/camel-route-resource.ts

View check run for this annotation

Codecov / codecov/patch

packages/ui/src/models/camel/camel-route-resource.ts#L99-L100

Added lines #L99 - L100 were not covered by tests
}

setSerializer(serializer: CamelResourceSerializer): void {
setSerializer(serializerType: SerializerType): void {

Check warning on line 103 in packages/ui/src/models/camel/camel-route-resource.ts

View check run for this annotation

Codecov / codecov/patch

packages/ui/src/models/camel/camel-route-resource.ts#L103

Added line #L103 was not covered by tests
// Preserve comments
const serializer = serializerType === 'XML' ? new XmlCamelResourceSerializer() : new YamlCamelResourceSerializer();
serializer.setComments(this.serializer.getComments());
this.serializer = serializer;
}
Expand Down
7 changes: 6 additions & 1 deletion packages/ui/src/serializers/camel-resource-serializer.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,15 @@
import { CamelResource } from '../models/camel';
import { CamelYamlDsl, Integration, Kamelet, KameletBinding, Pipe } from '@kaoto/camel-catalog/types';

export enum SerializerType {
XML = 'XML',
YAML = 'YAML',
}

export interface CamelResourceSerializer {
parse: (code: string) => CamelYamlDsl | Integration | Kamelet | KameletBinding | Pipe | undefined;
serialize: (resource: CamelResource) => string;
getComments: () => string[];
setComments: (comments: string[]) => void;
getLabel(): string;
getType(): SerializerType;
}
13 changes: 7 additions & 6 deletions packages/ui/src/serializers/xml-camel-resource-serializer.ts
Original file line number Diff line number Diff line change
@@ -1,16 +1,18 @@
import { isXML, KaotoXmlParser } from './xml/kaoto-xml-parser';
import { CamelResource } from '../models/camel/camel-resource';
import { EntityDefinition, KaotoXmlSerializer } from './xml/serializers/kaoto-xml-serializer';
import { KaotoXmlSerializer } from './xml/serializers/kaoto-xml-serializer';
import { formatXml } from './xml/xml-utils';
import { CamelResourceSerializer } from './camel-resource-serializer';
import { CamelResourceSerializer, SerializerType } from './camel-resource-serializer';
import { CamelYamlDsl, Integration, Kamelet, KameletBinding, Pipe } from '@kaoto/camel-catalog/types';
import { EntityType } from '../models/camel/entities';
import { EntityDefinition } from './xml/serializers/entitiy-definition';

export class XmlCamelResourceSerializer implements CamelResourceSerializer {
private comments: string[] = [];

Check warning on line 11 in packages/ui/src/serializers/xml-camel-resource-serializer.ts

View check run for this annotation

Codecov / codecov/patch

packages/ui/src/serializers/xml-camel-resource-serializer.ts#L11

Added line #L11 was not covered by tests
private static readonly COMMENT_REGEX = /<!--([\s\S]*?)-->/g;

getLabel(): string {
return 'XML';
getType(): SerializerType {
return SerializerType.XML;

Check warning on line 15 in packages/ui/src/serializers/xml-camel-resource-serializer.ts

View check run for this annotation

Codecov / codecov/patch

packages/ui/src/serializers/xml-camel-resource-serializer.ts#L14-L15

Added lines #L14 - L15 were not covered by tests
}

static isApplicable(code: unknown): boolean {
Expand Down Expand Up @@ -49,11 +51,10 @@ export class XmlCamelResourceSerializer implements CamelResourceSerializer {
}

private extractComments(xml: string): void {
const commentRegex = /<!--([\s\S]*?)-->/g;
this.comments = [];

Check warning on line 54 in packages/ui/src/serializers/xml-camel-resource-serializer.ts

View check run for this annotation

Codecov / codecov/patch

packages/ui/src/serializers/xml-camel-resource-serializer.ts#L53-L54

Added lines #L53 - L54 were not covered by tests
let match;

while ((match = commentRegex.exec(xml)) !== null) {
while ((match = XmlCamelResourceSerializer.COMMENT_REGEX.exec(xml)) !== null) {

Check warning on line 57 in packages/ui/src/serializers/xml-camel-resource-serializer.ts

View check run for this annotation

Codecov / codecov/patch

packages/ui/src/serializers/xml-camel-resource-serializer.ts#L57

Added line #L57 was not covered by tests
if (xml.slice(0, match.index).trim() === '') {
this.comments.push(match[1].trim());
} else {
Expand Down
20 changes: 6 additions & 14 deletions packages/ui/src/serializers/xml-to-yaml.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,34 +10,26 @@ import { KaotoXmlParser } from './xml/kaoto-xml-parser';
describe('XmlParser - XML to YAML comparison', () => {
let parser: KaotoXmlParser;
let yamlSerializer: YamlCamelResourceSerializer;
const xmlDir = path.join(__dirname, '../stubs/xml');
const yamlDir = path.join(__dirname, '../stubs/yaml');
const xmlFiles = fs.readdirSync(xmlDir).filter((file) => file.endsWith('.xml'));

beforeAll(async () => {
parser = new KaotoXmlParser();
yamlSerializer = new YamlCamelResourceSerializer(); // Initialize your YAML serializer
yamlSerializer = new YamlCamelResourceSerializer();
const catalogsMap = await getFirstCatalogMap(catalogLibrary as CatalogLibrary);
CamelCatalogService.setCatalogKey(CatalogKind.Processor, catalogsMap.modelCatalogMap);
});

const xmlDir = path.join(__dirname, '../stubs/xml');
const yamlDir = path.join(__dirname, '../stubs/yaml');

const xmlFiles = fs.readdirSync(xmlDir).filter((file) => file.endsWith('.xml'));

const tesXmlToYaml = (xmlFile: string) => {
it.each(xmlFiles)('parses and compares %s correctly', (xmlFile) => {
const xmlFilePath = path.join(xmlDir, xmlFile);
const yamlFilePath = path.join(yamlDir, xmlFile.replace('.xml', '.yaml'));

const xmlContent = fs.readFileSync(xmlFilePath, 'utf-8');
const expectedYamlContent = fs.readFileSync(yamlFilePath, 'utf-8');
const expectedYaml = yamlSerializer.parse(expectedYamlContent); // Use your YAML serializer
const expectedYaml = yamlSerializer.parse(expectedYamlContent);
const result = parser.parseXML(xmlContent);

expect(result).toEqual(expectedYaml);
};

xmlFiles.forEach((xmlFile) => {
it(`parses and compares ${xmlFile} correctly`, () => {
tesXmlToYaml(xmlFile);
});
});
});
39 changes: 15 additions & 24 deletions packages/ui/src/serializers/xml/kaoto-xml-parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ export class KaotoXmlParser {
'interceptSendToEndpoint',
'onCompletion',
];

static domParser = new DOMParser();
routeXmlParser: RouteXmlParser;
beanParser: BeansXmlParser;

Expand All @@ -49,23 +49,19 @@ export class KaotoXmlParser {
}

parseXML(xml: string): unknown {
const parser = new DOMParser();
const xmlDoc = parser.parseFromString(xml, 'application/xml');
const xmlDoc = KaotoXmlParser.domParser.parseFromString(xml, 'application/xml');

return this.parseFromXmlDocument(xmlDoc);
}

parseFromXmlDocument(xmlDoc: Document): unknown {
const rawEntities: unknown[] = [];
const rawEntities = [];

// Process route entities
const routes = Array.from(xmlDoc.getElementsByTagName('route')).map((routeElement) =>
RouteXmlParser.parse(routeElement),
);

if (routes.length > 0) {
routes.forEach((r) => rawEntities.push({ route: r }));
}
Array.from(xmlDoc.getElementsByTagName('route')).forEach((routeElement) => {
const route = RouteXmlParser.parse(routeElement);
rawEntities.push({ route });
});

// Process beans (bean factory)
const beansSection = xmlDoc.getElementsByTagName('beans')[0];
Expand All @@ -75,22 +71,17 @@ export class KaotoXmlParser {
}

// Process rest entities
const restEntities = Array.from(xmlDoc.getElementsByTagName('rest')).map((restElement) => ({
rest: RestXmlParser.parse(restElement),
}));

if (restEntities.length > 0) {
rawEntities.push(...restEntities);
}
Array.from(xmlDoc.getElementsByTagName('rest')).forEach((restElement) => {
const rest = RestXmlParser.parse(restElement);
rawEntities.push({ rest });
});

// Process route configurations
const routeConfigurations = Array.from(xmlDoc.getElementsByTagName('routeConfiguration')).map((routeConf) => ({
routeConfiguration: RouteXmlParser.parseRouteConfiguration(routeConf),
}));
Array.from(xmlDoc.getElementsByTagName('routeConfiguration')).forEach((routeConf) => {
const routeConfiguration = RouteXmlParser.parseRouteConfiguration(routeConf);
rawEntities.push({ routeConfiguration });
});

if (routeConfigurations.length > 0) {
rawEntities.push(...routeConfigurations);
}
// rest of the elements
const rootCamelElement = xmlDoc.getElementsByTagName('camel')[0];
const children = rootCamelElement ? rootCamelElement.children : xmlDoc.children;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (C) 2023 Red Hat, Inc.
* Copyright (C) 2025 Red Hat, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
* limitations under the License.
*/

import { describe } from 'node:test';
import { ExpressionParser } from './expression-parser';
import catalogLibrary from '@kaoto/camel-catalog/index.json';
import { getFirstCatalogMap } from '../../../stubs/test-load-catalog';
Expand Down
Loading

0 comments on commit 279cbdf

Please sign in to comment.