From cc4e3681af1d63d5f08c895044e7f1501c47193c Mon Sep 17 00:00:00 2001 From: Kenneth Aasan Date: Thu, 5 Sep 2024 09:17:20 +0200 Subject: [PATCH 1/3] fix: when allowInheritance is true, java now renders setters in interfaces for enums if const is not set by the classes that implements it --- docs/migrations/version-3-to-4.md | 4 +- src/generators/AbstractGenerator.ts | 22 ++++++++ .../java/renderers/ClassRenderer.ts | 36 +++++++++++-- src/models/ConstrainedMetaModel.ts | 1 + test/generators/java/JavaGenerator.spec.ts | 10 ++-- .../__snapshots__/JavaGenerator.spec.ts.snap | 51 ++++++++++--------- 6 files changed, 89 insertions(+), 35 deletions(-) diff --git a/docs/migrations/version-3-to-4.md b/docs/migrations/version-3-to-4.md index fc27a2a0d8..ef21efe370 100644 --- a/docs/migrations/version-3-to-4.md +++ b/docs/migrations/version-3-to-4.md @@ -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 enum is implemented by a class and has 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 enum is implemented by a class and has a constant value. diff --git a/src/generators/AbstractGenerator.ts b/src/generators/AbstractGenerator.ts index e3d238f73c..532f5d1339 100644 --- a/src/generators/AbstractGenerator.ts +++ b/src/generators/AbstractGenerator.ts @@ -162,6 +162,28 @@ export abstract class AbstractGenerator< } for (const { constrainedModel } of constrainedModelsWithDepManager) { + if (constrainedModel.options.extend) { + for (const extend of constrainedModel.options.extend) { + const extendModel = constrainedModelsWithDepManager.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 + ); + } + } + for (const unionConstrainedModel of unionConstrainedModelsWithDepManager) { if ( unionConstrainedModel.constrainedModel instanceof diff --git a/src/generators/java/renderers/ClassRenderer.ts b/src/generators/java/renderers/ClassRenderer.ts index a604528148..8fc07b90ba 100644 --- a/src/generators/java/renderers/ClassRenderer.ts +++ b/src/generators/java/renderers/ClassRenderer.ts @@ -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 = { self({ renderer }) { return renderer.defaultSelf(); @@ -205,16 +228,21 @@ export const JAVA_DEFAULT_CLASS_PRESET: ClassPresetType = { 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 not 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 not 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}; }`; } diff --git a/src/models/ConstrainedMetaModel.ts b/src/models/ConstrainedMetaModel.ts index e52a92ee3e..c6efc909fd 100644 --- a/src/models/ConstrainedMetaModel.ts +++ b/src/models/ConstrainedMetaModel.ts @@ -23,6 +23,7 @@ export class ConstrainedMetaModelOptions extends MetaModelOptions { discriminator?: ConstrainedMetaModelOptionsDiscriminator; parents?: ConstrainedMetaModel[]; extend?: ConstrainedMetaModel[]; + implementedBy?: ConstrainedMetaModel[]; } export interface GetNearestDependenciesArgument { diff --git a/test/generators/java/JavaGenerator.spec.ts b/test/generators/java/JavaGenerator.spec.ts index 3b5b2a3b45..493aa0cf35 100644 --- a/test/generators/java/JavaGenerator.spec.ts +++ b/test/generators/java/JavaGenerator.spec.ts @@ -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'] @@ -376,7 +376,7 @@ describe('JavaGenerator', () => { title: 'Test', type: 'object', properties: { - testEnum: { + test_enum: { title: 'TestEnum', type: 'string', enum: ['FOO', 'BAR'] @@ -390,10 +390,10 @@ describe('JavaGenerator', () => { { type: 'object', properties: { - testEnum: { + test_enum: { const: 'FOO' }, - testProp2: { + test_string: { type: 'string' } } diff --git a/test/generators/java/__snapshots__/JavaGenerator.spec.ts.snap b/test/generators/java/__snapshots__/JavaGenerator.spec.ts.snap index 26725d7cd8..d0c04f0606 100644 --- a/test/generators/java/__snapshots__/JavaGenerator.spec.ts.snap +++ b/test/generators/java/__snapshots__/JavaGenerator.spec.ts.snap @@ -20,9 +20,9 @@ public interface Pet { @NotNull @JsonProperty(\\"type\\") private final CloudEventType type = CloudEventType.DOG; - @JsonProperty(\\"sequencetype\\") + @JsonProperty(\\"sequence_type\\") @JsonInclude(JsonInclude.Include.NON_NULL) - private CloudEventDotSequenceType sequencetype; + private CloudEventDotSequenceType sequenceType; @JsonProperty(\\"data\\") @JsonInclude(JsonInclude.Include.NON_NULL) private String data; @@ -40,8 +40,9 @@ public interface Pet { public CloudEventType getType() { return this.type; } @Override - public CloudEventDotSequenceType getSequencetype() { return this.sequencetype; } - public void setSequencetype(CloudEventDotSequenceType sequencetype) { this.sequencetype = sequencetype; } + public CloudEventDotSequenceType getSequenceType() { return this.sequenceType; } + @Override + public void setSequenceType(CloudEventDotSequenceType sequenceType) { this.sequenceType = sequenceType; } public String getData() { return this.data; } public void setData(String data) { this.data = data; } @@ -64,7 +65,7 @@ public interface Pet { return Objects.equals(this.id, self.id) && Objects.equals(this.type, self.type) && - Objects.equals(this.sequencetype, self.sequencetype) && + Objects.equals(this.sequenceType, self.sequenceType) && Objects.equals(this.data, self.data) && Objects.equals(this.test, self.test) && Objects.equals(this.additionalProperties, self.additionalProperties); @@ -72,7 +73,7 @@ public interface Pet { @Override public int hashCode() { - return Objects.hash((Object)id, (Object)type, (Object)sequencetype, (Object)data, (Object)test, (Object)additionalProperties); + return Objects.hash((Object)id, (Object)type, (Object)sequenceType, (Object)data, (Object)test, (Object)additionalProperties); } @Override @@ -80,7 +81,7 @@ public interface Pet { return \\"class Dog {\\\\n\\" + \\" id: \\" + toIndentedString(id) + \\"\\\\n\\" + \\" type: \\" + toIndentedString(type) + \\"\\\\n\\" + - \\" sequencetype: \\" + toIndentedString(sequencetype) + \\"\\\\n\\" + + \\" sequenceType: \\" + toIndentedString(sequenceType) + \\"\\\\n\\" + \\" data: \\" + toIndentedString(data) + \\"\\\\n\\" + \\" test: \\" + toIndentedString(test) + \\"\\\\n\\" + \\" additionalProperties: \\" + toIndentedString(additionalProperties) + \\"\\\\n\\" + @@ -128,7 +129,7 @@ public interface Pet { } }", "public enum CloudEventDotSequenceType { - INTEGER((String)\\"Integer\\"); + INTEGER((String)\\"Integer\\"), STRING_TEST((String)\\"StringTest\\"); private String value; @@ -157,20 +158,20 @@ public interface Pet { } }", "public class TestAllOf implements Test { - @JsonProperty(\\"testEnum\\") + @JsonProperty(\\"test_enum\\") @JsonInclude(JsonInclude.Include.NON_NULL) private final TestEnum testEnum = TestEnum.FOO; - @JsonProperty(\\"testProp2\\") + @JsonProperty(\\"test_string\\") @JsonInclude(JsonInclude.Include.NON_NULL) - private String testProp2; + private String testString; @JsonInclude(JsonInclude.Include.NON_NULL) private Map additionalProperties; @Override public TestEnum getTestEnum() { return this.testEnum; } - public String getTestProp2() { return this.testProp2; } - public void setTestProp2(String testProp2) { this.testProp2 = testProp2; } + public String getTestString() { return this.testString; } + public void setTestString(String testString) { this.testString = testString; } public Map getAdditionalProperties() { return this.additionalProperties; } public void setAdditionalProperties(Map additionalProperties) { this.additionalProperties = additionalProperties; } @@ -186,20 +187,20 @@ public interface Pet { TestAllOf self = (TestAllOf) o; return Objects.equals(this.testEnum, self.testEnum) && - Objects.equals(this.testProp2, self.testProp2) && + Objects.equals(this.testString, self.testString) && Objects.equals(this.additionalProperties, self.additionalProperties); } @Override public int hashCode() { - return Objects.hash((Object)testEnum, (Object)testProp2, (Object)additionalProperties); + return Objects.hash((Object)testEnum, (Object)testString, (Object)additionalProperties); } @Override public String toString() { return \\"class TestAllOf {\\\\n\\" + \\" testEnum: \\" + toIndentedString(testEnum) + \\"\\\\n\\" + - \\" testProp2: \\" + toIndentedString(testProp2) + \\"\\\\n\\" + + \\" testString: \\" + toIndentedString(testString) + \\"\\\\n\\" + \\" additionalProperties: \\" + toIndentedString(additionalProperties) + \\"\\\\n\\" + \\"}\\"; } @@ -251,7 +252,8 @@ public interface Pet { public String getId(); public void setId(String id); - public CloudEventDotSequenceType getSequencetype(); + public CloudEventDotSequenceType getSequenceType(); + public void setSequenceType(CloudEventDotSequenceType sequenceType); }", "public class Cat implements Pet, CloudEvent { @NotNull @@ -260,9 +262,9 @@ public interface Pet { @NotNull @JsonProperty(\\"type\\") private final CloudEventType type = CloudEventType.CAT; - @JsonProperty(\\"sequencetype\\") + @JsonProperty(\\"sequence_type\\") @JsonInclude(JsonInclude.Include.NON_NULL) - private CloudEventDotSequenceType sequencetype; + private CloudEventDotSequenceType sequenceType; @JsonInclude(JsonInclude.Include.NON_NULL) private Map additionalProperties; @@ -274,8 +276,9 @@ public interface Pet { public CloudEventType getType() { return this.type; } @Override - public CloudEventDotSequenceType getSequencetype() { return this.sequencetype; } - public void setSequencetype(CloudEventDotSequenceType sequencetype) { this.sequencetype = sequencetype; } + public CloudEventDotSequenceType getSequenceType() { return this.sequenceType; } + @Override + public void setSequenceType(CloudEventDotSequenceType sequenceType) { this.sequenceType = sequenceType; } public Map getAdditionalProperties() { return this.additionalProperties; } public void setAdditionalProperties(Map additionalProperties) { this.additionalProperties = additionalProperties; } @@ -292,13 +295,13 @@ public interface Pet { return Objects.equals(this.id, self.id) && Objects.equals(this.type, self.type) && - Objects.equals(this.sequencetype, self.sequencetype) && + Objects.equals(this.sequenceType, self.sequenceType) && Objects.equals(this.additionalProperties, self.additionalProperties); } @Override public int hashCode() { - return Objects.hash((Object)id, (Object)type, (Object)sequencetype, (Object)additionalProperties); + return Objects.hash((Object)id, (Object)type, (Object)sequenceType, (Object)additionalProperties); } @Override @@ -306,7 +309,7 @@ public interface Pet { return \\"class Cat {\\\\n\\" + \\" id: \\" + toIndentedString(id) + \\"\\\\n\\" + \\" type: \\" + toIndentedString(type) + \\"\\\\n\\" + - \\" sequencetype: \\" + toIndentedString(sequencetype) + \\"\\\\n\\" + + \\" sequenceType: \\" + toIndentedString(sequenceType) + \\"\\\\n\\" + \\" additionalProperties: \\" + toIndentedString(additionalProperties) + \\"\\\\n\\" + \\"}\\"; } From 5fe09767dde7eb015752760c3bfbdc94392101d4 Mon Sep 17 00:00:00 2001 From: Kenneth Aasan Date: Thu, 5 Sep 2024 09:46:36 +0200 Subject: [PATCH 2/3] fix: when allowInheritance is true, java now renders setters in interfaces for enums if const is not set by the classes that implements it --- docs/migrations/version-3-to-4.md | 4 +- src/generators/AbstractGenerator.ts | 114 ++++++++++-------- .../java/renderers/ClassRenderer.ts | 4 +- 3 files changed, 68 insertions(+), 54 deletions(-) diff --git a/docs/migrations/version-3-to-4.md b/docs/migrations/version-3-to-4.md index ef21efe370..32fa966193 100644 --- a/docs/migrations/version-3-to-4.md +++ b/docs/migrations/version-3-to-4.md @@ -321,6 +321,6 @@ type info struct { ## Java -### When `allowInheritance` is true, Modelina no longer renders the setter for enums in interfaces if the enum is implemented by a class and has 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. Therefore, if `allowInheritance` is set to true, Modelina will no longer render the setter for enums in interfaces if the enum is implemented by a class and has 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. diff --git a/src/generators/AbstractGenerator.ts b/src/generators/AbstractGenerator.ts index 532f5d1339..c61475751c 100644 --- a/src/generators/AbstractGenerator.ts +++ b/src/generators/AbstractGenerator.ts @@ -63,6 +63,11 @@ export interface AbstractGeneratorRenderCompleteModelArgs< options?: DeepPartial; } +interface ConstrainedMetaModelWithDepManager { + constrainedModel: ConstrainedMetaModel; + dependencyManager: AbstractDependencyManager; +} + /** * Abstract generator which must be implemented by each language */ @@ -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( + constrainedModels: ConstrainedMetaModelWithDepManager[], + constrainedModel: ConstrainedMetaModel + ) { + for (const unionConstrainedModel of constrainedModels) { + 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. @@ -125,11 +181,6 @@ export abstract class AbstractGenerator< constrainedModel: ConstrainedMetaModel; dependencyManager: AbstractDependencyManager; }> { - interface ConstrainedMetaModelWithDepManager { - constrainedModel: ConstrainedMetaModel; - dependencyManager: AbstractDependencyManager; - } - const getConstrainedMetaModelWithDepManager = ( model: MetaModel ): ConstrainedMetaModelWithDepManager => { @@ -161,54 +212,17 @@ export abstract class AbstractGenerator< ); } - for (const { constrainedModel } of constrainedModelsWithDepManager) { - if (constrainedModel.options.extend) { - for (const extend of constrainedModel.options.extend) { - const extendModel = constrainedModelsWithDepManager.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 - ); - } - } - - 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(constrainedModels, constrainedModel); + } + + return constrainedModels; } /** diff --git a/src/generators/java/renderers/ClassRenderer.ts b/src/generators/java/renderers/ClassRenderer.ts index 8fc07b90ba..ab345fcd58 100644 --- a/src/generators/java/renderers/ClassRenderer.ts +++ b/src/generators/java/renderers/ClassRenderer.ts @@ -228,7 +228,7 @@ export const JAVA_DEFAULT_CLASS_PRESET: ClassPresetType = { const setterName = FormatHelpers.toPascalCase(property.propertyName); if (model.options.isExtended) { - // don't render setters for discriminator, dictionary properties, or enums that are not set with a const value + // don't render setters for discriminator, dictionary properties, or enums that are set with a const value if ( isDiscriminatorOrDictionary(model, property) || isEnumImplementedByConstValue(model, property) @@ -239,7 +239,7 @@ export const JAVA_DEFAULT_CLASS_PRESET: ClassPresetType = { return `public void set${setterName}(${property.property.type} ${property.propertyName});`; } - // don't render override for enums that are not set with a const value + // don't render override for enums that are set with a const value const override = !isEnumImplementedByConstValue(model, property) ? getOverride(model, property) : ''; From 2ab0861b2a33ce5e5535d0736004a588907e914c Mon Sep 17 00:00:00 2001 From: Kenneth Aasan Date: Thu, 5 Sep 2024 09:52:24 +0200 Subject: [PATCH 3/3] fix: when allowInheritance is true, java now renders setters in interfaces for enums if const is not set by the classes that implements it --- src/generators/AbstractGenerator.ts | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/generators/AbstractGenerator.ts b/src/generators/AbstractGenerator.ts index c61475751c..74769e3023 100644 --- a/src/generators/AbstractGenerator.ts +++ b/src/generators/AbstractGenerator.ts @@ -150,10 +150,10 @@ export abstract class AbstractGenerator< } private setParentsForModels( - constrainedModels: ConstrainedMetaModelWithDepManager[], + unionConstrainedModelsWithDepManager: ConstrainedMetaModelWithDepManager[], constrainedModel: ConstrainedMetaModel ) { - for (const unionConstrainedModel of constrainedModels) { + for (const unionConstrainedModel of unionConstrainedModelsWithDepManager) { if ( unionConstrainedModel.constrainedModel instanceof ConstrainedUnionModel && @@ -219,7 +219,10 @@ export abstract class AbstractGenerator< for (const { constrainedModel } of constrainedModels) { this.setImplementedByForModels(constrainedModels, constrainedModel); - this.setParentsForModels(constrainedModels, constrainedModel); + this.setParentsForModels( + unionConstrainedModelsWithDepManager, + constrainedModel + ); } return constrainedModels;