fix: dynamic topic params out of order and non-bean methods added to application.yml (#286)

* feat: Added file names to generated test folders; only beans are included in the generated application.yml file; order of variables in topic are fixed

* chore: remove commented out code

* chore: linting

Author: CameronRushton

filters/all.js

@@ -691,8 +691,8 @@ function getBrokerSettings(asyncapi, params) {
 function getBindings(asyncapi, params) {
   const ret = {};
   const funcs = getFunctionSpecs(asyncapi, params);
-
-  funcs.forEach((spec, name, map) => {
+  const functionsThatAreBeans = filterOutNonBeanFunctionSpecs(funcs);
+  functionsThatAreBeans.forEach((spec, name, map) => {
     if (spec.isPublisher) {
       ret[spec.publishBindingName] = {};
       ret[spec.publishBindingName].destination = spec.publishChannel;
@@ -736,11 +736,31 @@ function getFunctionName(channelName, operation, isSubscriber) {
   return ret;
 }
 
+function filterOutNonBeanFunctionSpecs(funcs) {
+  const beanFunctionSpecs = new Map();
+  const entriesIterator = funcs.entries();
+
+  let iterValue = entriesIterator.next();
+  while (!iterValue.done) {
+    const funcSpec = iterValue.value[1];
+
+    // This is the inverse of the condition in the Application template file
+    // Add it if its not dynamic (hasParams = true, isPublisher = true) and type is supplier or streamBridge
+    if (!(funcSpec.isPublisher && funcSpec.channelInfo.hasParams &&
+      (funcSpec.type === 'supplier' || funcSpec.dynamicType === 'streamBridge'))) {
+      beanFunctionSpecs.set(iterValue.value[0], iterValue.value[1]);
+    }
+    iterValue = entriesIterator.next();
+  }
+  return beanFunctionSpecs;
+}
+
 // This returns the string that gets rendered in the function.definition part of application.yaml.
 function getFunctionDefinitions(asyncapi, params) {
   let ret = '';
   const funcs = getFunctionSpecs(asyncapi, params);
-  const names = funcs.keys();
+  const functionsThatAreBeans = filterOutNonBeanFunctionSpecs(funcs);
+  const names = functionsThatAreBeans.keys();
   ret = Array.from(names).join(';');
   return ret;
 }
@@ -953,6 +973,14 @@ function getMessagePayloadType(message) {
   return ret;
 }
 
+function sortParametersUsingChannelName(parameters, channelName) {
+  // This doesnt work if theres two of the same variables in the channel name (scenario unlikely)
+  parameters.forEach(param => {
+    param.indexInChannelName = channelName.indexOf(param.rawName);
+  });
+  return _.sortBy(parameters, ['indexInChannelName']);
+}
+
 // This returns the connection properties for a solace binder, for application.yaml.
 function getSolace(params) {
   const ret = {};
@@ -975,10 +1003,6 @@ function getChannelInfo(params, channelName, channel) {
   let publishChannel = String(channelName);
   let subscribeChannel = String(channelName);
   const parameters = [];
-  let functionParamList = '';
-  let functionArgList = '';
-  let sampleArgList = '';
-  let first = true;
 
   debugChannel('parameters:');
   debugChannel(channel.parameters());
@@ -987,39 +1011,35 @@ function getChannelInfo(params, channelName, channel) {
     const parameter = channel.parameter(name);
     const schema = parameter.schema();
     const type = getType(schema.type(), schema.format());
-    const param = { name: _.camelCase(name) };
+    const param = {
+      name: _.camelCase(name),
+      rawName: name
+    };
     debugChannel(`name: ${name} type:`);
     debugChannel(type);
     let sampleArg = 1;
 
-    // Figure out what position it's in. This is just for the parameterToHeader feature.
+    subscribeChannel = subscribeChannel.replace(nameWithBrackets, '*');
+
+    // Figure out what channel part it's in. This is just for the parameterToHeader feature.
     for (let i = 0; i < channelParts.length; i++) {
       if (channelParts[i] === nameWithBrackets) {
         param.position = i;
         break;
       }
     }
 
-    if (first) {
-      first = false;
-    } else {
-      functionParamList += ', ';
-      functionArgList += ', ';
-    }
-
-    sampleArgList += ', ';
     [publishChannel, sampleArg] = handleParameterType(name, param, type, publishChannel, schema, nameWithBrackets);
-    subscribeChannel = subscribeChannel.replace(nameWithBrackets, '*');
-    functionParamList += `${param.type} ${param.name}`;
-    functionArgList += param.name;
-    sampleArgList += sampleArg;
+    param.sampleArg = sampleArg;
     parameters.push(param);
   }
-  ret.functionArgList = functionArgList;
-  ret.functionParamList = functionParamList;
-  ret.sampleArgList = sampleArgList;
+  // The channel parameters aren't in any particular order when they come in.
+  // This means, to be safe, we need to order them like how it is in the channel name.
+  ret.parameters = sortParametersUsingChannelName(parameters, channelName);
+  ret.functionArgList = ret.parameters.map(param => param.name).join(', ');
+  ret.functionParamList = ret.parameters.map(param => `${param.type} ${param.name}`).join(', ');
+  ret.sampleArgList = ret.parameters.map(param => param.sampleArg).join(', ');
   ret.channelName = channelName;
-  ret.parameters = parameters;
   ret.publishChannel = publishChannel;
   ret.subscribeChannel = subscribeChannel;
   ret.hasParams = parameters.length > 0;

test/__snapshots__/integration.test.js.snap

@@ -481,11 +481,9 @@ exports[`template integration tests using the generator should generate applicat
 "spring:
   cloud:
     function:
-      definition: testLevel1MessageIdOperationSupplier;testLevel1MessageIdOperationConsumer
+      definition: testLevel1MessageIdOperationConsumer
     stream:
       bindings:
-        testLevel1MessageIdOperationSupplier-out-0:
-          destination: 'testLevel1/{messageId}/{operation}'
         testLevel1MessageIdOperationConsumer-in-0:
           destination: testLevel1/*/*
       binders:
@@ -985,11 +983,9 @@ exports[`template integration tests using the generator should generate extra co
           input-header-mapping-expression:
             messageId: 'headers.solace_destination.getName.split(\\"/\\")[1]'
             operation: 'headers.solace_destination.getName.split(\\"/\\")[2]'
-      definition: testLevel1MessageIdOperationSupplier;testLevel1MessageIdOperationConsumer
+      definition: testLevel1MessageIdOperationConsumer
     stream:
       bindings:
-        testLevel1MessageIdOperationSupplier-out-0:
-          destination: 'testLevel1/{messageId}/{operation}'
         testLevel1MessageIdOperationConsumer-in-0:
           destination: testLevel1/*/*
       binders:
@@ -1599,6 +1595,22 @@ public class Debtor {
 "
 `;
 
+exports[`template integration tests using the generator should not populate application yml with functions that are not beans 1`] = `
+"spring:
+  cloud:
+    function:
+      definition: ''
+    stream:
+      bindings: {}
+logging:
+  level:
+    root: info
+    org:
+      springframework: info
+
+"
+`;
+
 exports[`template integration tests using the generator should package and import schemas in another avro namespace 1`] = `
 "
 
@@ -1802,6 +1814,43 @@ public class JobAcknowledge {
 "
 `;
 
+exports[`template integration tests using the generator should place the topic variables in the correct order 1`] = `
+"
+
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.boot.SpringApplication;
+import org.springframework.boot.autoconfigure.SpringBootApplication;
+import org.springframework.cloud.stream.function.StreamBridge;
+import org.springframework.messaging.Message;
+import org.springframework.messaging.support.MessageBuilder;
+
+@SpringBootApplication
+public class Application {
+
+	private static final Logger logger = LoggerFactory.getLogger(Application.class);
+
+	@Autowired
+	private StreamBridge streamBridge;
+
+	public static void main(String[] args) {
+		SpringApplication.run(Application.class);
+	}
+
+
+	public void sendAcmeBillingReceiptsReceiptIdCreatedVersionRegionsRegionChargifyRideId(
+		RideReceipt payload, String receiptId, String version, String region, String rideId
+		) {
+		String topic = String.format(\\"acme/billing/receipts/%s/created/%s/regions/%s/chargify/%s\\",
+			receiptId, version, region, rideId);
+		streamBridge.send(topic, payload);
+	}
+}
+"
+`;
+
 exports[`template integration tests using the generator should return object when avro union type is used specifying many possible types 1`] = `
 "package com.example.api.jobOrder;
 

test/integration.test.js

@@ -3,10 +3,12 @@ const Generator = require('@asyncapi/generator');
 const { readFile } = require('fs').promises;
 const crypto = require('crypto');
 
+const TEST_SUITE_NAME = 'template integration tests using the generator';
 // Constants not overridden per test
 const TEST_FOLDER_NAME = 'test';
 const MAIN_TEST_RESULT_PATH = path.join(TEST_FOLDER_NAME, 'temp', 'integrationTestResult');
 
+// Unfortunately, the test suite name must be a hard coded string
 describe('template integration tests using the generator', () => {
   jest.setTimeout(30000);
 
@@ -18,7 +20,8 @@ describe('template integration tests using the generator', () => {
 
   const generateFolderName = () => {
     // we always want to generate to new directory to make sure test runs in clear environment
-    return path.resolve(MAIN_TEST_RESULT_PATH, crypto.randomBytes(4).toString('hex'));
+    const testName = expect.getState().currentTestName.substring(TEST_SUITE_NAME.length + 1);
+    return path.resolve(MAIN_TEST_RESULT_PATH, `${testName  } - ${  crypto.randomBytes(4).toString('hex')}`);
   };
 
   const generate = (asyncApiFilePath, params) => {
@@ -192,4 +195,24 @@ describe('template integration tests using the generator', () => {
     ];
     await assertExpectedFiles(validatedFiles);
   });
+
+  it('should place the topic variables in the correct order', async () => {
+    // For a topic of test/{var1}/{var2}, the listed params in the asyncapi document can be in any order
+    await generate('mocks/multivariable-topic.yaml');
+
+    const validatedFiles = [
+      'src/main/java/Application.java'
+    ];
+    await assertExpectedFiles(validatedFiles);
+  });
+
+  it('should not populate application yml with functions that are not beans', async () => {
+    // If the function is a supplier or using a stream bridge, the function isn't a bean and shouldnt be in application.yml
+    await generate('mocks/multivariable-topic.yaml');
+
+    const validatedFiles = [
+      'src/main/resources/application.yml'
+    ];
+    await assertExpectedFiles(validatedFiles);
+  });
 });

test/mocks/multivariable-topic.yaml

@@ -0,0 +1,45 @@
+components:
+  schemas:
+    RideReceipt:
+      $schema: 'http://json-schema.org/draft-07/schema#'
+      type: object
+      title: This schema is irrelevant
+      $id: 'http://example.com/root.json'
+  messages:
+    Billing Receipt Created:
+      payload:
+        $ref: '#/components/schemas/RideReceipt'
+      schemaFormat: application/vnd.aai.asyncapi+json;version=2.0.0
+      contentType: application/json
+channels:
+  'acme/billing/receipts/{receipt_id}/created/{version}/regions/{region}/chargify/{ride_id}':
+    subscribe:
+      bindings:
+        solace:
+          bindingVersion: 0.1.0
+          destinations:
+            - destinationType: topic
+      message:
+        $ref: '#/components/messages/Billing Receipt Created'
+    parameters:
+      version:
+        schema:
+          type: string
+      receipt_id:
+        schema:
+          type: string
+      ride_id:
+        schema:
+          type: string
+      region:
+        schema:
+          type: string
+          enum:
+            - US
+            - UK
+            - CA
+            - MX
+asyncapi: 2.0.0
+info:
+  title: ExpenseReportingIntegrationApplication
+  version: 0.0.1