fix!: when allowInheritance is true, java now renders setters in inte…

…rfaces for enums if const is not set by the classes that implements it (#2096) * fix: when allowInheritance is true, java now renders setters in interfaces for enums if const is not set by the classes that implements it * fix: when allowInheritance is true, java now renders setters in interfaces for enums if const is not set by the classes that implements it * fix: when allowInheritance is true, java now renders setters in interfaces for enums if const is not set by the classes that implements it
asyncapi · Sep 5, 2024 · f6cd360 · f6cd360
1 parent 1b561f9
commit f6cd360
Show file tree

Hide file tree

Showing 6 changed files with 134 additions and 63 deletions.
diff --git 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 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.
diff --git a/src/generators/AbstractGenerator.ts b/src/generators/AbstractGenerator.ts
@@ -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
  */
@@ -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.
@@ -125,11 +181,6 @@ export abstract class AbstractGenerator<
     constrainedModel: ConstrainedMetaModel;
     dependencyManager: AbstractDependencyManager;
   }> {
-    interface ConstrainedMetaModelWithDepManager {
-      constrainedModel: ConstrainedMetaModel;
-      dependencyManager: AbstractDependencyManager;
-    }
-
     const getConstrainedMetaModelWithDepManager = (
       model: MetaModel
     ): ConstrainedMetaModelWithDepManager => {
@@ -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;
   }
 
   /**

diff --git 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<JavaOptions> = {
   self({ renderer }) {
     return renderer.defaultSelf();
@@ -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}; }`;
   }

diff --git 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
@@ -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'
                     }
                   }