fix: unable to get model class when using arrays in schema (#207)

* fix: resolve NPE when attempting to use a null modelClass by trying to use the x-parser-schema-id property of the source schemafirst, then try the property name. * fix: fixed issues with keyword schema naming and writing inner classes that should have been their own class; the schema name is going to be correct more often from using x-schema-parser-id instead when appropriate; broke all args constructor for schemas that are arrays with this commit. * fix: created partial for all args constructor; all args constructor now correctly created for array types * chore: remove code smells; stripPackage function is more cohesive; removed need for a tentative class name. * chore: remove useless modelClass assignment in anonSchema allOf handling * chore: clean up todo; use ternary instead of if statement * update snapshot; unnecessary spacing removed due to removal of comment * chore: fix linting problems
asyncapi · Jan 5, 2022 · 582fced · 582fced
1 parent 0313e3f
commit 582fced
Show file tree

Hide file tree

Showing 11 changed files with 1,514 additions and 139 deletions.
diff --git a/filters/all.js b/filters/all.js
@@ -423,12 +423,16 @@ const getMethods = (obj) => {
   return [...properties.keys()].filter(item => typeof obj[item] === 'function');
 };
 
-function getModelClass(schemaName) {
-  return applicationModel.getModelClass(schemaName);
+function getModelClass(customSchemaObject) {
+  return applicationModel.getModelClass(customSchemaObject);
 }
-
 filter.getModelClass = getModelClass;
 
+function getAnonymousSchemaForRef(schemaName) {
+  return applicationModel.getAnonymousSchemaForRef(schemaName);
+}
+filter.getAnonymousSchemaForRef = getAnonymousSchemaForRef;
+
 function getRealPublisher([info, params, channel]) {
   return scsLib.getRealPublisher(info, params, channel);
 }

diff --git a/hooks/post-process.js b/hooks/post-process.js
@@ -2,6 +2,7 @@
 const fs = require('fs');
 const path = require('path');
 const ApplicationModel = require('../lib/applicationModel.js');
+const _ = require('lodash');
 const applicationModel = new ApplicationModel('post');
 // To enable debug logging, set the env var DEBUG="postProcess" with whatever things you want to see.
 const debugPostProcess = require('debug')('postProcess');
@@ -74,12 +75,14 @@ function processSchema(generator, schemaName, schema, sourcePath, defaultJavaPac
   const filePath = path.resolve(sourcePath, fileName);
   debugPostProcess(`processSchema ${schemaName}`);
   debugPostProcess(schema);
-  if (schema.type() !== 'object') {
+  const modelClass = applicationModel.getModelClass({schema, schemaName});
+  const javaName = modelClass.getClassName();
+  if ((schema.type() && schema.type() !== 'object') || _.startsWith(javaName, 'Anonymous')) {
     debugPostProcess(`deleting ${filePath}`);
-    fs.unlinkSync(filePath);
+    if (fs.existsSync(filePath)) {
+      fs.unlinkSync(filePath);
+    }
   } else {
-    const modelClass = applicationModel.getModelClass(schemaName);
-    const javaName = modelClass.getClassName();
     const packageDir = getPackageDir(generator, defaultJavaPackageDir, modelClass);
     debugPostProcess(`packageDir: ${packageDir}`);
 

diff --git a/lib/applicationModel.js b/lib/applicationModel.js
@@ -1,5 +1,6 @@
 const ModelClass = require('./modelClass.js');
 const debugApplicationModel = require('debug')('applicationModel');
+const _ = require('lodash');
 const instanceMap = new Map();
 
 class ApplicationModel {
@@ -10,12 +11,26 @@ class ApplicationModel {
     debugApplicationModel(instanceMap);
   }
 
-  getModelClass(schemaName) {
+  getModelClass({schema, schemaName}) {
     debugApplicationModel(`getModelClass for caller ${this.caller} schema ${schemaName}`);
     this.setupSuperClassMap();
     this.setupModelClassMap();
-    const modelClass = this.modelClassMap[schemaName];
-    debugApplicationModel(`returning modelClass for  caller ${this.caller} ${schemaName}`);
+    let modelClass;
+    if (schema) {
+      const parserSchemaName = schema.ext('x-parser-schema-id');
+      // Try to use x-parser-schema-id as key
+      modelClass = this.modelClassMap[parserSchemaName];
+      if (modelClass && _.startsWith(modelClass.getClassName(), 'Anonymous')) {
+        // If we translated this schema from the map using an anonymous schema key, we have no idea what the name should be, so we use the one provided directly from the source - not the generator.
+        // If we translated this schema from the map using a known schema (the name of the schema was picked out correctly by the generator), use that name.
+        modelClass.setClassName(_.upperFirst(this.isAnonymousSchema(parserSchemaName) ? schemaName : parserSchemaName));
+      }
+    }
+    // Using x-parser-schema-id didn't work for us, fall back to trying to get at least something using the provided name.
+    if (!modelClass) {
+      modelClass = this.modelClassMap[schemaName];
+    }
+    debugApplicationModel(`returning modelClass for caller ${this.caller} ${schemaName}`);
     debugApplicationModel(modelClass);
     return modelClass;
   }
@@ -66,68 +81,123 @@ class ApplicationModel {
     } else {
       this.superClassMap[anonymousSchema] = namedSchema;
       this.anonymousSchemaToSubClassMap[anonymousSchema] = schemaName;
+      this.superClassMap[schemaName] = namedSchema;
+      this.anonymousSchemaToSubClassMap[schemaName] = anonymousSchema;
     }
   }  
 
   setupModelClassMap() {
     if (!this.modelClassMap) {
       this.modelClassMap = new Map();
+      this.nameToSchemaMap = new Map();
+      // Register all schemas first, then check the anonymous schemas for duplicates
+      ApplicationModel.asyncapi.allSchemas().forEach((schema, name) => {
+        debugApplicationModel(`setupModelClassMap ${name} type ${schema.type()}`);
+        this.registerSchemaNameToModelClass(schema, name);
+        this.nameToSchemaMap[name] = schema;
+      });
+
       ApplicationModel.asyncapi.allSchemas().forEach((schema, schemaName) => {
-        debugApplicationModel(`setupModelClassMap ${schemaName} type ${schema.type()}`);
+        debugApplicationModel(`setupModelClassMap anonymous schemas ${schemaName} type ${schema.type()}`);
+        this.checkProperties(schema);
+
         const allOf = schema.allOf();
         debugApplicationModel('allOf:');
         debugApplicationModel(allOf);
         if (allOf) {
           allOf.forEach(innerSchema => {
-            const name = innerSchema._json['x-parser-schema-id'];
+            const name = innerSchema.ext('x-parser-schema-id');
             if (this.isAnonymousSchema(name) && innerSchema.type() === 'object') {
-              this.addSchemaToMap(innerSchema, schemaName);
+              this.registerSchemaNameToModelClass(innerSchema, schemaName);
             }
           });
-        } else {
-          this.addSchemaToMap(schema, schemaName);
         }
       });
       debugApplicationModel('modelClassMap:');
       debugApplicationModel(this.modelClassMap);
     }
   }
 
+  checkProperties(schema) {
+    if (!!Object.keys(schema.properties()).length) {
+      // Each property name is the name of a schema. It should also have an x-parser-schema-id name. We'll be adding duplicate mappings (two mappings to the same model class) since the anon schemas do have names
+      Object.keys(schema.properties()).forEach(property => {
+        const innerSchema = schema.properties()[property];
+        const innerSchemaParserId = innerSchema.ext('x-parser-schema-id');
+        const existingModelClass = this.modelClassMap[innerSchemaParserId];
+        if (existingModelClass) {
+          this.modelClassMap[property] = existingModelClass;
+        } else {
+          this.registerSchemaNameToModelClass(innerSchema, property);
+        }
+      });
+    }
+  }
+
   isAnonymousSchema(schemaName) {
     return schemaName.startsWith('<');
   }
 
-  addSchemaToMap(schema, schemaName) {
-    const modelClass = new ModelClass();
-    let tentativeClassName = schemaName;
+  registerSchemaNameToModelClass(schema, schemaName) {
+    let modelClass = this.modelClassMap[schemaName];
+    if (!modelClass) {
+      modelClass = new ModelClass();
+    }
+
     if (this.isAnonymousSchema(schemaName)) {
-      // It's an anonymous schema. It might be a subclass...
-      const subclassName = this.anonymousSchemaToSubClassMap[schemaName];
-      if (subclassName) {
-        tentativeClassName = subclassName;
-        modelClass.setSuperClassName(this.superClassMap[schemaName]);
-      }
+      this.handleAnonymousSchemaForAllOf(modelClass, schemaName);
     }
-    // If there is a dot in the schema name, it's probably an Avro schema with a fully qualified name (including the namespace.)
-    const indexOfDot = schemaName.lastIndexOf('.');
-    let javaPackage;
-    if (indexOfDot > 0) {
-      javaPackage = schemaName.substring(0, indexOfDot);
-      tentativeClassName = schemaName.substring(indexOfDot + 1);
-      modelClass.setJavaPackage(javaPackage);
+    const components = ApplicationModel.asyncapi._json.components;
+    const nonInnerClassSchemas = Object.keys(components? components.schemas || {} : {});
+    if (nonInnerClassSchemas.includes(schemaName)) {
+      modelClass.setCanBeInnerClass(false);
     }
-    modelClass.setClassName(tentativeClassName);
+
+    const { className, javaPackage } = this.stripPackageName(schemaName);
+    modelClass.setJavaPackage(javaPackage);
+    modelClass.setClassName(className);
     debugApplicationModel(`schemaName ${schemaName} className: ${modelClass.getClassName()} super: ${modelClass.getSuperClassName()} javaPackage: ${javaPackage}`);
     this.modelClassMap[schemaName] = modelClass;
     debugApplicationModel(`Added ${schemaName}`);
     debugApplicationModel(modelClass);
   }
 
+  getAnonymousSchemaForRef(realSchemaName) {
+    // During our allOf parsing, we found this real schema to anon-schema association
+    const anonSchema = this.anonymousSchemaToSubClassMap[realSchemaName];
+    return anonSchema ? this.nameToSchemaMap[anonSchema] : undefined;
+  }
+
+  handleAnonymousSchemaForAllOf(modelClass, schemaName) {
+    const subclassName = this.anonymousSchemaToSubClassMap[schemaName];
+    if (subclassName) {
+      modelClass.setSuperClassName(this.superClassMap[schemaName]);
+      // Be sure the anonymous modelClass and the named modelClass are updated with the superclass information
+      // We dont want the anonymous schema because the class name won't be correct if it's a $ref, so if the modelClass exists, update that one, if it doesn't we'll make it
+      const existingModelClass = this.modelClassMap[subclassName];
+      if (existingModelClass) {
+        existingModelClass.setSuperClassName(this.superClassMap[schemaName]);
+      }
+      return subclassName;
+    }
+    return schemaName;
+  }
+
+  stripPackageName(schemaName) {
+    // If there is a dot in the schema name, it's probably an Avro schema with a fully qualified name (including the namespace.)
+    const indexOfDot = schemaName.lastIndexOf('.');
+    if (indexOfDot > 0) {
+      return { className: schemaName.substring(indexOfDot + 1), javaPackage: schemaName.substring(0, indexOfDot) };
+    }
+    return { className: schemaName, javaPackage: undefined };
+  }
+
   reset() {
     instanceMap.forEach((val) => {
       val.superClassMap = null;
       val.anonymousSchemaToSubClassMap = null;
       val.modelClassMap = null;
+      val.nameToSchemaMap = null;
     });
   }
 }

diff --git a/lib/modelClass.js b/lib/modelClass.js
@@ -1,6 +1,10 @@
 const _ = require('lodash');
 
 class ModelClass {
+  constructor() {
+    this.innerClass = true;
+  }
+
   getClassName() {
     return this.className;
   }
@@ -32,6 +36,14 @@ class ModelClass {
   fixClassName(originalName) {
     return _.upperFirst(_.camelCase(originalName));
   }
+
+  setCanBeInnerClass(innerClass) {
+    this.innerClass = innerClass;
+  }
+
+  canBeInnerClass() {
+    return this.innerClass;
+  }
 }
 
 module.exports = ModelClass;
diff --git a/package.json b/package.json
@@ -11,7 +11,8 @@
     "test": "jest --maxWorkers=50% --detectOpenHandles",
     "test:watch": "npm run test -- --watch",
     "test:watchAll": "npm run test -- --watchAll",
-    "test:coverage": "npm run test -- --coverage"
+    "test:coverage": "npm run test -- --coverage",
+    "test:updateSnapshots": "npm run test -- -u"
   },
   "keywords": [
     "asyncapi",

diff --git a/partials/all-args-constructor b/partials/all-args-constructor
@@ -0,0 +1,30 @@
+{%- macro allArgsConstructor(className, properties, indentLevel) -%}
+{% set indent1 = indentLevel | indent1 -%}
+{% set indent2 = indentLevel | indent2 -%}
+{% set indent3 = indentLevel | indent3 -%}
+{% set first = true -%}
+{%- set hasNoProperties = properties | isEmpty -%}
+{%- if not hasNoProperties -%}
+{{ indent2 }}public {{ className }} (
+{%- for name, prop in properties -%}
+{%- set propModelClass = {schema: prop, schemaName: name} | getModelClass %}
+{%- set realClassName = propModelClass.getClassName() %}
+{%- set variableName = realClassName | identifierName %}
+{%- set typeInfo = [name, realClassName, prop] | fixType %}
+{%- set type = typeInfo[0] -%}
+{%- if first -%}
+{%- set first = false -%}
+{%- else -%}
+, {% endif %}
+{{ indent3 }}{{ type }} {{ variableName }}
+{%- endfor -%}
+) {
+{% for name, prop in properties -%}
+{%- set propModelClass = {schema: prop, schemaName: name} | getModelClass %}
+{%- set realClassName = propModelClass.getClassName() %}
+{%- set variableName = realClassName | identifierName -%}
+{{ indent3 }}this.{{ variableName }} = {{ variableName }};
+{% endfor -%}
+{{ indent2 }}}
+{%- endif -%}
+{% endmacro %}