refactor: Remove options used solely by the connector

[zregvart]

Options not recognized recognized by the base Camel component cannot be specified on the endpoint URI, this removes those options that are used only by the connector.

[KurtStam]

@zregvart I'm assuming they are removed /after/ being used to potentially use them to contract another parameter like the connection url:

https://github.com/syndesisio/connectors/blob/master/connectors/sql-stored-connector/src/main/java/io/syndesis/connector/springboot/SqlStoredConnectorConnectorAutoConfiguration.java#L105

Also I'm a bit puzzled why this did not come up in our test.

[zregvart]

Yeah, so this is a Camel only thing, the *AutoConfiguration thing is Spring's and one does not affect the other. And mind you, this is just a quick & dirty fix, I think proper fix might be to use those properties in the Connector and create the DataSource there, and leave the *AutoConfiguration classes to Camel Maven plugin to manage.

[zregvart]

Yeah, and the tests passed as we don't set those options on the endpoint parameters (https://github.com/syndesisio/connectors/blob/master/connectors/sql-stored-connector/src/test/java/io/syndesis/connector/SqlStoredConnectorComponentTest.java#L78), only the template.

[davsclaus]

The connector generates the following meta-data about the endpoint options (sql-stored-connector.json) file

  "properties": {
    "procedureName": { "kind": "path", "displayName": "Template", "group": "producer", "required": true, "type": "string", "javaType": "java.lang.String", "deprecated": false, "secret": false, "description": "Sets the StoredProcedure name to perform" },
    "template": { "kind": "path", "displayName": "Template", "group": "producer", "required": true, "type": "string", "javaType": "java.lang.String", "deprecated": false, "secret": false, "description": "Sets the StoredProcedure template to perform" },
    "batch": { "kind": "parameter", "displayName": "Batch", "group": "producer", "required": false, "type": "boolean", "javaType": "boolean", "deprecated": false, "secret": false, "defaultValue": false, "description": "Enables or disables batch mode" },
    "noop": { "kind": "parameter", "displayName": "Noop", "group": "producer", "required": false, "type": "boolean", "javaType": "boolean", "deprecated": false, "secret": false, "defaultValue": false, "description": "If set will ignore the results of the template and use the existing IN message as the OUT message for the continuation of processing" }
  }

The properties are the endpoint options (they are named properties when it was first created and hence we are stuck with that name, instead of endpointProperties or endpointOptions. As you can see those options you attempt to remove are not listed there.

However in the same file, you have the component options listed as

  "componentProperties": {
    "url": { "kind": "property", "displayName": "Connection URL", "group": "producer", "label": "common", "required": true, "type": "string", "javaType": "java.lang.String", "deprecated": false, "secret": false, "defaultValue": "jdbc:postgresql:database", "description": "JDBC URL of the database" },
    "user": { "kind": "property", "displayName": "Username", "group": "security", "label": "common,security", "required": true, "type": "string", "javaType": "java.lang.String", "deprecated": false, "secret": false, "defaultValue": "", "description": "Username for the database connection" },
    "password": { "kind": "property", "displayName": "Password", "group": "security", "label": "common,security", "required": true, "type": "string", "javaType": "java.lang.String", "deprecated": false, "secret": true, "description": "Password for the database connection" },
    "schema": { "kind": "property", "displayName": "Schema", "group": "producer", "label": "common", "required": false, "type": "string", "javaType": "java.lang.String", "deprecated": false, "secret": false, "description": "Database schema" },
    "catalog": { "kind": "property", "displayName": "Catalog", "group": "producer", "label": "common", "required": false, "type": "string", "javaType": "java.lang.String", "deprecated": false, "secret": false, "description": "Database catalog" }
  },

[KurtStam]

@davsclaus what are you trying to say? Did we mislabel the properties?

[davsclaus]

Okay I look at the source code, and there is a problem that the component class does not have getter/setter for the 4 options it has defined in the camel-connector.json

   "componentOptions" : [ "user", "password", "url", "schema", "catalog" ],

But instead a hack has been added to the SqlStoredConnectorConnectorAutoConfiguration class where you read those configurations and create the datasource in some post constructor code.

So you added code to a file that says Generated by camel-connector-maven-plugin - do not edit this file!

Down the road I dont think this is advised.

[davsclaus]

@KurtStam @zregvart look in the target folder what the JAR includes.

Look at

• target/classes/camel-connector.json
• target/classes/camel-connector-scham.json
• target/classes/io/syndesis/connector/sql-stored-connector.json

[davsclaus]

I assume camel-connector-schema.json is what Sydesis UI is using to know what a connector can do

{
  "component": {
    "kind": "component",
    "baseScheme": "sql-stored",
    "scheme": "sql-stored-connector",
    "syntax": "sql-stored-connector:template",
    "title": "SqlStoredConnector",
    "description": "SQL Stored Procedure Connector to invoke a SQL Stored Procedure",
    "label": "sql-stored",
    "deprecated": false,
    "async": false,
    "producerOnly": true,
    "lenientProperties": false,
    "javaType": "io.syndesis.connector.SqlStoredConnectorComponent",
    "groupId": "io.syndesis",
    "artifactId": "sql-stored-connector",
    "version": "0.4-SNAPSHOT"
  },
  "componentProperties": {
    "url": { "kind": "property", "displayName": "Connection URL", "group": "producer", "label": "common", "required": true, "type": "string", "javaType": "java.lang.String", "deprecated": false, "secret": false, "defaultValue": "jdbc:postgresql:database", "description": "JDBC URL of the database" },
    "user": { "kind": "property", "displayName": "Username", "group": "security", "label": "common,security", "required": true, "type": "string", "javaType": "java.lang.String", "deprecated": false, "secret": false, "defaultValue": "", "description": "Username for the database connection" },
    "password": { "kind": "property", "displayName": "Password", "group": "security", "label": "common,security", "required": true, "type": "string", "javaType": "java.lang.String", "deprecated": false, "secret": true, "description": "Password for the database connection" },
    "schema": { "kind": "property", "displayName": "Schema", "group": "producer", "label": "common", "required": false, "type": "string", "javaType": "java.lang.String", "deprecated": false, "secret": false, "description": "Database schema" },
    "catalog": { "kind": "property", "displayName": "Catalog", "group": "producer", "label": "common", "required": false, "type": "string", "javaType": "java.lang.String", "deprecated": false, "secret": false, "description": "Database catalog" }
  },
  "properties": {
    "procedureName": { "kind": "path", "displayName": "Template", "group": "producer", "required": true, "type": "string", "javaType": "java.lang.String", "deprecated": false, "secret": false, "description": "Sets the StoredProcedure name to perform" },
    "template": { "kind": "path", "displayName": "Template", "group": "producer", "required": true, "type": "string", "javaType": "java.lang.String", "deprecated": false, "secret": false, "description": "Sets the StoredProcedure template to perform" },
    "batch": { "kind": "parameter", "displayName": "Batch", "group": "producer", "required": false, "type": "boolean", "javaType": "boolean", "deprecated": false, "secret": false, "defaultValue": false, "description": "Enables or disables batch mode" },
    "noop": { "kind": "parameter", "displayName": "Noop", "group": "producer", "required": false, "type": "boolean", "javaType": "boolean", "deprecated": false, "secret": false, "defaultValue": false, "description": "If set will ignore the results of the template and use the existing IN message as the OUT message for the continuation of processing" }
  }
}

• componentProperties = options you can configure on the component level, or via spring boot auto configuration, eg application.properties
• properties = options you can configure in endpoint uri (yeah it should have been named endpointProperties, but we are stuck with this name).

Sometimes properties can be configured on both component and endpoint level and hence why they may appear in both of them. Other times you have exclusive properties in either of them.

For example this connector is such a case, where properties are either only on the component level or the endpoint level.

[davsclaus]

I a Camel connector you can specify which properties you want included, to filter out unwanted properties from the base Camel component (eg sql-component).

You do this in the camel-connector.json where you have done

  "componentOptions" : [ "user", "password", "url", "schema", "catalog" ],
  "endpointOptions" : [ "procedureName", "template", "batch", "noop" ],

PS: I do not know what catalog is used for, it does not look like its in use.

[davsclaus]

This PR does not make sense as the options are NOT on the endpoint level at all from the beginning.

If they are in use somehow, then its a bug in Syndesis where it provides those invalid options to Camel. The only endpoint options allowed are the ones in the properties section in the camel-connector-schema.json file, which are these:

 "properties": {
    "procedureName": { "kind": "path", "displayName": "Template", "group": "producer", "required": true, "type": "string", "javaType": "java.lang.String", "deprecated": false, "secret": false, "description": "Sets the StoredProcedure name to perform" },
    "template": { "kind": "path", "displayName": "Template", "group": "producer", "required": true, "type": "string", "javaType": "java.lang.String", "deprecated": false, "secret": false, "description": "Sets the StoredProcedure template to perform" },
    "batch": { "kind": "parameter", "displayName": "Batch", "group": "producer", "required": false, "type": "boolean", "javaType": "boolean", "deprecated": false, "secret": false, "defaultValue": false, "description": "Enables or disables batch mode" },
    "noop": { "kind": "parameter", "displayName": "Noop", "group": "producer", "required": false, "type": "boolean", "javaType": "boolean", "deprecated": false, "secret": false, "defaultValue": false, "description": "If set will ignore the results of the template and use the existing IN message as the OUT message for the continuation of processing" }
  }

So I dont understand the need for this PR in the first place, unless its to workaround a bug in Syndesis, such as if it provides invalid endpoint options

[zregvart]

So I dont understand the need for this PR in the first place, unless its to workaround a bug in Syndesis, such as if it provides invalid endpoint options

Yeah, we workaround to get the UX we need, we can't expect the user to provide a DataSource reference or the template parameter, which is something that the base SQL stored procedure component expects.

I think we might be able to work this out by adding additional logic to the connector component implementation by creating a DataSource and template parameters there.

The issue this PR is trying to address is that those properties that we use to synthesize DataSource and template parameters get added by the runtime to endpoint URI as parameters, and rightfully Camel complains about that.

Perhaps we can merge this and log an issue to see if we could add the properties needed for the creation of DataSource and template parameters in the connector component?

[davsclaus]

@zregvart yeah sure the PR does no harm.

And +1 to log a new ticket about adding those options to create the DataSource/template stuff more correctly.

[KurtStam]

@lburgazzoli has been working on a feature so we can add 'custom' properties to a syndesis connector. This work was done before this feature was ready. We maybe able to upgrade so we are using it. In the meantime the maven plugins in the pom where commented out.

[pure-bot[bot]]

Pull request approved by @KurtStam - applying approved label