Skip to content

Commit

Permalink
fix: fixes performance issues in java when calling parent unions (#1501)
Browse files Browse the repository at this point in the history
  • Loading branch information
kennethaasan authored Sep 14, 2023
1 parent 4f09942 commit cf059d4
Show file tree
Hide file tree
Showing 10 changed files with 267 additions and 204 deletions.
36 changes: 18 additions & 18 deletions examples/rust-generate-crate/__snapshots__/index.spec.ts.snap
Original file line number Diff line number Diff line change
@@ -1,6 +1,24 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`Should be able to render Rust Models and should log expected output to console 1`] = `
Array [
"// Members represents a union of types: String, f64, bool
#[derive(Clone, Debug, Deserialize, PartialEq, PartialOrd, Serialize)]
#[serde(untagged)]
pub enum Members {
#[serde(rename=\\"MembersOneOf0\\")]
MembersOneOf0(String),
#[serde(rename=\\"MembersOneOf1\\")]
MembersOneOf1(f64),
#[serde(rename=\\"MembersOneOf2\\")]
MembersOneOf2(bool),
}
",
]
`;

exports[`Should be able to render Rust Models and should log expected output to console 2`] = `
Array [
"// Address represents a Address model.
#[derive(Clone, Debug, Deserialize, PartialEq, Serialize)]
Expand Down Expand Up @@ -47,24 +65,6 @@ impl Address {
]
`;
exports[`Should be able to render Rust Models and should log expected output to console 2`] = `
Array [
"// Members represents a union of types: String, f64, bool
#[derive(Clone, Debug, Deserialize, PartialEq, PartialOrd, Serialize)]
#[serde(untagged)]
pub enum Members {
#[serde(rename=\\"MembersOneOf0\\")]
MembersOneOf0(String),
#[serde(rename=\\"MembersOneOf1\\")]
MembersOneOf1(f64),
#[serde(rename=\\"MembersOneOf2\\")]
MembersOneOf2(bool),
}
",
]
`;
exports[`Should be able to render Rust Models and should log expected output to console 3`] = `
Array [
"// TupleType represents a TupleType model.
Expand Down
154 changes: 116 additions & 38 deletions src/generators/AbstractGenerator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ import {
RenderOutput,
ProcessorOptions,
MetaModel,
ConstrainedMetaModel
ConstrainedMetaModel,
UnionModel,
ConstrainedUnionModel
} from '../models';
import { InputProcessor } from '../processors';
import { IndentationTypes } from '../helpers';
Expand Down Expand Up @@ -97,6 +99,78 @@ export abstract class AbstractGenerator<
return options.dependencyManager;
}

/**
* Generates an array of ConstrainedMetaModel with its dependency manager from an InputMetaModel.
* It also adds parents to the ConstrainedMetaModel's which can be used in renderers which needs to know what parents they belong to.
*/
private getConstrainedModels(inputModel: InputMetaModel): Array<{
constrainedModel: ConstrainedMetaModel;
dependencyManager: AbstractDependencyManager;
}> {
interface ConstrainedMetaModelWithDepManager {
constrainedModel: ConstrainedMetaModel;
dependencyManager: AbstractDependencyManager;
}

const getConstrainedMetaModelWithDepManager = (
model: MetaModel
): ConstrainedMetaModelWithDepManager => {
const dependencyManager = this.getDependencyManager(this.options);
const constrainedModel = this.constrainToMetaModel(model, {
dependencyManager
} as DeepPartial<Options>);
return {
constrainedModel,
dependencyManager
};
};

const unionConstrainedModelsWithDepManager: ConstrainedMetaModelWithDepManager[] =
[];
const constrainedModelsWithDepManager: ConstrainedMetaModelWithDepManager[] =
[];

for (const model of Object.values(inputModel.models)) {
if (model instanceof UnionModel) {
unionConstrainedModelsWithDepManager.push(
getConstrainedMetaModelWithDepManager(model)
);
continue;
}

constrainedModelsWithDepManager.push(
getConstrainedMetaModelWithDepManager(model)
);
}

for (const { constrainedModel } of constrainedModelsWithDepManager) {
for (const unionConstrainedModel of unionConstrainedModelsWithDepManager) {
if (
unionConstrainedModel.constrainedModel instanceof
ConstrainedUnionModel &&
unionConstrainedModel.constrainedModel.union.some(
(m) =>
m.name === constrainedModel.name &&
m.type === constrainedModel.type
)
) {
if (!constrainedModel.options.parents) {
constrainedModel.options.parents = [];
}

constrainedModel.options.parents.push(
unionConstrainedModel.constrainedModel
);
}
}
}

return [
...unionConstrainedModelsWithDepManager,
...constrainedModelsWithDepManager
];
}

/**
* Generates the full output of a model, instead of a scattered model.
*
Expand All @@ -108,50 +182,54 @@ export abstract class AbstractGenerator<
completeOptions: Partial<RenderCompleteModelOptions>
): Promise<OutputModel[]> {
const inputModel = await this.processInput(input);
const renders = Object.values(inputModel.models).map(async (model) => {
const dependencyManager = this.getDependencyManager(this.options);
const constrainedModel = this.constrainToMetaModel(model, {
dependencyManager
} as DeepPartial<Options>);
const renderedOutput = await this.renderCompleteModel(
constrainedModel,
inputModel,
completeOptions,
{ dependencyManager } as DeepPartial<Options>
);
return OutputModel.toOutputModel({
result: renderedOutput.result,
modelName: renderedOutput.renderedName,
dependencies: renderedOutput.dependencies,
model: constrainedModel,
inputModel
});
});
return Promise.all(renders);

return Promise.all(
this.getConstrainedModels(inputModel).map(
async ({ constrainedModel, dependencyManager }) => {
const renderedOutput = await this.renderCompleteModel(
constrainedModel,
inputModel,
completeOptions,
{ dependencyManager } as DeepPartial<Options>
);
return OutputModel.toOutputModel({
result: renderedOutput.result,
modelName: renderedOutput.renderedName,
dependencies: renderedOutput.dependencies,
model: constrainedModel,
inputModel
});
}
)
);
}

/**
* Generates a scattered model where dependencies and rendered results are separated.
*/
public async generate(input: any | InputMetaModel): Promise<OutputModel[]> {
const inputModel = await this.processInput(input);
const renders = Object.values(inputModel.models).map(async (model) => {
const dependencyManager = this.getDependencyManager(this.options);
const constrainedModel = this.constrainToMetaModel(model, {
dependencyManager
} as DeepPartial<Options>);
const renderedOutput = await this.render(constrainedModel, inputModel, {
dependencyManager
} as DeepPartial<Options>);
return OutputModel.toOutputModel({
result: renderedOutput.result,
modelName: renderedOutput.renderedName,
dependencies: renderedOutput.dependencies,
model: constrainedModel,
inputModel
});
});
return Promise.all(renders);

return Promise.all(
this.getConstrainedModels(inputModel).map(
async ({ constrainedModel, dependencyManager }) => {
const renderedOutput = await this.render(
constrainedModel,
inputModel,
{
dependencyManager
} as DeepPartial<Options>
);
return OutputModel.toOutputModel({
result: renderedOutput.result,
modelName: renderedOutput.renderedName,
dependencies: renderedOutput.dependencies,
model: constrainedModel,
inputModel
});
}
)
);
}

/**
Expand Down
30 changes: 11 additions & 19 deletions src/generators/java/renderers/ClassRenderer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,7 @@ import {
ConstrainedDictionaryModel,
ConstrainedObjectModel,
ConstrainedObjectPropertyModel,
ConstrainedUnionModel,
UnionModel
ConstrainedUnionModel
} from '../../../models';
import { FormatHelpers } from '../../../helpers';
import { JavaOptions } from '../JavaGenerator';
Expand Down Expand Up @@ -101,23 +100,16 @@ ${this.indent(this.renderBlock(content, 2))}
private getParentUnions(): ConstrainedUnionModel[] | undefined {
const parentUnions: ConstrainedUnionModel[] = [];

for (const model of Object.values(this.inputModel.models)) {
if (model instanceof UnionModel) {
// Create a ConstrainedUnionModel of all Union Models
const unionModel = this.generator.constrainToMetaModel(
model,
this.options
) as ConstrainedUnionModel;

// Check if the current model is a child model of any of the union interfaces
if (
!unionIncludesBuiltInTypes(unionModel) &&
unionModel.union.some(
(m) => m.name === this.model.name && m.type === this.model.type
)
) {
parentUnions.push(unionModel);
}
if (!this.model.options.parents) {
return undefined;
}

for (const model of this.model.options.parents) {
if (
model instanceof ConstrainedUnionModel &&
!unionIncludesBuiltInTypes(model)
) {
parentUnions.push(model);
}
}

Expand Down
1 change: 1 addition & 0 deletions src/models/ConstrainedMetaModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ export interface ConstrainedMetaModelOptionsDiscriminator
export class ConstrainedMetaModelOptions extends MetaModelOptions {
const?: ConstrainedMetaModelOptionsConst;
discriminator?: ConstrainedMetaModelOptionsDiscriminator;
parents?: ConstrainedMetaModel[];
}

export abstract class ConstrainedMetaModel extends MetaModel {
Expand Down
11 changes: 8 additions & 3 deletions test/TestUtils/TestGenerator.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
/* eslint-disable @typescript-eslint/no-unused-vars */
import {
AbstractGenerator,
InputMetaModel,
IndentationTypes,
RenderOutput,
ConstrainedMetaModel,
MetaModel,
ConstrainedAnyModel,
Preset
Preset,
InputMetaModel
} from '../../src';
import { AbstractDependencyManager } from '../../src/generators/AbstractDependencyManager';

Expand All @@ -23,11 +23,13 @@ export class TestGenerator extends AbstractGenerator<any, any> {
}

public constrainToMetaModel(model: MetaModel): ConstrainedMetaModel {
return new ConstrainedAnyModel(model.name, undefined, '');
return new ConstrainedAnyModel(model.name, undefined, {}, '');
}

public splitMetaModel(model: MetaModel): MetaModel[] {
return [model];
}

public render(
model: MetaModel,
inputModel: InputMetaModel
Expand All @@ -39,6 +41,7 @@ export class TestGenerator extends AbstractGenerator<any, any> {
})
);
}

public renderCompleteModel(
model: MetaModel,
inputModel: InputMetaModel,
Expand All @@ -51,9 +54,11 @@ export class TestGenerator extends AbstractGenerator<any, any> {
})
);
}

public testGetPresets(string: string): Array<[Preset, unknown]> {
return this.getPresets(string);
}

public getDependencyManager(options: any): AbstractDependencyManager {
return new AbstractDependencyManager();
}
Expand Down
11 changes: 6 additions & 5 deletions test/generators/AbstractGenerator.spec.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { InputMetaModel, CommonModel, AnyModel } from '../../src/models';
import { InputMetaModel, AnyModel } from '../../src/models';
import { TestGenerator } from '../TestUtils/TestGenerator';

describe('AbstractGenerator', () => {
let generator: TestGenerator;
beforeEach(() => {
Expand All @@ -8,7 +9,7 @@ describe('AbstractGenerator', () => {

test('generate() should return OutputModels', async () => {
const cim = new InputMetaModel();
const model = new AnyModel('test', undefined);
const model = new AnyModel('test', undefined, {});
cim.models[model.name] = model;
const outputModels = await generator.generate(cim);

Expand All @@ -18,7 +19,7 @@ describe('AbstractGenerator', () => {

test('generateCompleteModels() should return OutputModels', async () => {
const cim = new InputMetaModel();
const model = new AnyModel('test', undefined);
const model = new AnyModel('test', undefined, {});
cim.models[model.name] = model;
const outputModels = await generator.generateCompleteModels(cim, {});

Expand All @@ -28,7 +29,7 @@ describe('AbstractGenerator', () => {

test('generate() should process InputMetaModel instance', async () => {
const cim = new InputMetaModel();
const model = new AnyModel('test', undefined);
const model = new AnyModel('test', undefined, {});
cim.models[model.name] = model;
const outputModels = await generator.generate(cim);
expect(outputModels[0].result).toEqual('test');
Expand All @@ -37,7 +38,7 @@ describe('AbstractGenerator', () => {

test('generateCompleteModels() should process InputMetaModel instance', async () => {
const cim = new InputMetaModel();
const model = new AnyModel('test', undefined);
const model = new AnyModel('test', undefined, {});
cim.models[model.name] = model;
const outputModels = await generator.generateCompleteModels(cim, {});

Expand Down
2 changes: 1 addition & 1 deletion test/generators/java/JavaGenerator.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ describe('JavaGenerator', () => {
const models = await generator.generate(doc);
expect(models).toHaveLength(4);
expect(models.map((model) => model.result)).toMatchSnapshot();
expect(models[0].dependencies).toEqual(expectedDependencies);
expect(models[3].dependencies).toEqual(expectedDependencies);
});

test('should work custom preset for `class` type', async () => {
Expand Down
Loading

0 comments on commit cf059d4

Please sign in to comment.