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

fix!: when allowInheritance is true, java now renders setters in interfaces for enums if const is not set by the classes that implements it #2096

Merged
merged 3 commits into from
Sep 5, 2024
Merged
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
4 changes: 2 additions & 2 deletions docs/migrations/version-3-to-4.md
Original file line number Diff line number Diff line change
Expand Up @@ -321,6 +321,6 @@ type info struct {

## Java

### when allowInheritance is true, Modelina now don't render the setter for enums in interfaces because the classes that implement the interface might use a constant value
### When `allowInheritance` is true, Modelina no longer renders the setter for enums in interfaces if the property exist in the implemented by class with a constant value

In Java, when a class implements an interface, it must implement all the methods of that interface. Because of that, Modelina now doesn't render the setter for enums in interfaces when allowInheritance is true because the classes that implement the interface might use a constant value.
In Java, when a class implements an interface, it must implement all the methods of that interface. Therefore, if `allowInheritance` is set to true, Modelina will no longer render the setter for enums in interfaces if the property exists in the implemented by class with a constant value.
95 changes: 67 additions & 28 deletions src/generators/AbstractGenerator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,11 @@ export interface AbstractGeneratorRenderCompleteModelArgs<
options?: DeepPartial<Options>;
}

interface ConstrainedMetaModelWithDepManager {
constrainedModel: ConstrainedMetaModel;
dependencyManager: AbstractDependencyManager;
}

/**
* Abstract generator which must be implemented by each language
*/
Expand Down Expand Up @@ -117,6 +122,57 @@ export abstract class AbstractGenerator<
return options.dependencyManager;
}

private setImplementedByForModels(
constrainedModels: ConstrainedMetaModelWithDepManager[],
constrainedModel: ConstrainedMetaModel
) {
if (!constrainedModel.options.extend) {
return;
}

for (const extend of constrainedModel.options.extend) {
const extendModel = constrainedModels.find(
(m) => m.constrainedModel.name === extend.name
);

if (!extendModel) {
throw new Error(
`Could not find the model ${extend.name} to extend in the constrained models`
);
}

if (!extendModel.constrainedModel.options.implementedBy) {
extendModel.constrainedModel.options.implementedBy = [];
}

extendModel.constrainedModel.options.implementedBy.push(constrainedModel);
}
}

private setParentsForModels(
unionConstrainedModelsWithDepManager: ConstrainedMetaModelWithDepManager[],
constrainedModel: ConstrainedMetaModel
) {
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
);
}
}
}

/**
* 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.
Expand All @@ -125,11 +181,6 @@ export abstract class AbstractGenerator<
constrainedModel: ConstrainedMetaModel;
dependencyManager: AbstractDependencyManager;
}> {
interface ConstrainedMetaModelWithDepManager {
constrainedModel: ConstrainedMetaModel;
dependencyManager: AbstractDependencyManager;
}

const getConstrainedMetaModelWithDepManager = (
model: MetaModel
): ConstrainedMetaModelWithDepManager => {
Expand Down Expand Up @@ -161,32 +212,20 @@ export abstract class AbstractGenerator<
);
}

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 [
const constrainedModels = [
...unionConstrainedModelsWithDepManager,
...constrainedModelsWithDepManager
];

for (const { constrainedModel } of constrainedModels) {
this.setImplementedByForModels(constrainedModels, constrainedModel);
this.setParentsForModels(
unionConstrainedModelsWithDepManager,
constrainedModel
);
}

return constrainedModels;
}

/**
Expand Down
36 changes: 32 additions & 4 deletions src/generators/java/renderers/ClassRenderer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,29 @@ export const isDiscriminatorOrDictionary = (
property.unconstrainedPropertyName ||
property.property instanceof ConstrainedDictionaryModel;

const isEnumImplementedByConstValue = (
model: ConstrainedObjectModel,
property: ConstrainedObjectPropertyModel
): boolean => {
if (!isEnum(property)) {
return false;
}

if (!model.options.implementedBy) {
return false;
}

// if the implementedBy property exist in the model options, check if the property exists in the implementedBy model and check if the property is set with a const value
return model.options.implementedBy.some((implementedBy) => {
return (
implementedBy instanceof ConstrainedObjectModel &&
implementedBy.properties[property.propertyName] &&
implementedBy.properties[property.propertyName].property.options.const
?.value
);
});
};

export const JAVA_DEFAULT_CLASS_PRESET: ClassPresetType<JavaOptions> = {
self({ renderer }) {
return renderer.defaultSelf();
Expand Down Expand Up @@ -205,16 +228,21 @@ export const JAVA_DEFAULT_CLASS_PRESET: ClassPresetType<JavaOptions> = {
const setterName = FormatHelpers.toPascalCase(property.propertyName);

if (model.options.isExtended) {
// don't render setters for discriminator, dictionary properties, or enums (because they can be set to constants in the models that extend them)
if (isDiscriminatorOrDictionary(model, property) || isEnum(property)) {
// don't render setters for discriminator, dictionary properties, or enums that are set with a const value
if (
isDiscriminatorOrDictionary(model, property) ||
isEnumImplementedByConstValue(model, property)
) {
return '';
}

return `public void set${setterName}(${property.property.type} ${property.propertyName});`;
}

// don't render override for enums because enum setters in the interfaces are not rendered
const override = !isEnum(property) ? getOverride(model, property) : '';
// don't render override for enums that are set with a const value
const override = !isEnumImplementedByConstValue(model, property)
? getOverride(model, property)
: '';

return `${override}public void set${setterName}(${property.property.type} ${property.propertyName}) { this.${property.propertyName} = ${property.propertyName}; }`;
}
Expand Down
1 change: 1 addition & 0 deletions src/models/ConstrainedMetaModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ export class ConstrainedMetaModelOptions extends MetaModelOptions {
discriminator?: ConstrainedMetaModelOptionsDiscriminator;
parents?: ConstrainedMetaModel[];
extend?: ConstrainedMetaModel[];
implementedBy?: ConstrainedMetaModel[];
}

export interface GetNearestDependenciesArgument {
Expand Down
10 changes: 5 additions & 5 deletions test/generators/java/JavaGenerator.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -364,10 +364,10 @@ describe('JavaGenerator', () => {
type: 'string',
description: 'test'
},
sequencetype: {
sequence_type: {
title: 'CloudEvent.SequenceType',
type: 'string',
enum: ['Integer']
enum: ['Integer', 'StringTest']
}
},
required: ['id', 'type']
Expand All @@ -376,7 +376,7 @@ describe('JavaGenerator', () => {
title: 'Test',
type: 'object',
properties: {
testEnum: {
test_enum: {
title: 'TestEnum',
type: 'string',
enum: ['FOO', 'BAR']
Expand All @@ -390,10 +390,10 @@ describe('JavaGenerator', () => {
{
type: 'object',
properties: {
testEnum: {
test_enum: {
const: 'FOO'
},
testProp2: {
test_string: {
type: 'string'
}
}
Expand Down
Loading
Loading