feat: GraphQL connector template #202

markfarkas-camunda · 2023-01-11T17:19:20Z

Description

Created the template and skeleton for GraphQL connector.

UPDATE

The code is ready for an early MVP use-case testing. Please test and provide feedback here under this ticket.

Steps to run it locally:

set up zeebe locally (e.g. with docker compose)
check out this branch
navigate to connectors-common-library folder(/connectors/connectors-common-library), and build this module by executing:

mvn clean install

navigate to graphql folder(/connectors/graphql), and build the graphql connector by executing:

mvn clean install

go back to the main folder and build the whole bundle by executing:

mvn clean package

and start the application with:

java -jar ./bundle/mvn/default-bundle/target/connector-runtime-bundle-0.16.0-SNAPSHOT-with-dependencies.jar  "-Dexec.mainClass=io.camunda.connector.bundle.LocalConnectorRuntime"

This should start the application

Add this element template to the Desktop modeler.
Test with the bpmns below (updated):
bpmns.zip
Validate the result in Operate
Visit the https://studio.apollographql.com/public/star-wars-swapi/home?variant=current site to see the GraphQL DB structure, and explore other properties for the queries.

Related issues

#153

https://github.com/camunda/product-hub/issues/784

bpmn-io/bpmn-js-properties-panel#866

bastiankoerber

please add variable input field and link to documentation how to use it :-)
Rest looks good for me

bastiankoerber · 2023-01-16T08:32:58Z

connectors/graphql/element-templates/template-connector.json

+    },
+    {
+      "label": "Query Parameters",
+      "description": "Map of query parameters to add to the request URL",


can we link here to a documentation, where we are showing how it will work?

i am missing the variable input view. Have i overseen it?

Thank you for noticing, I missed that. I added to the template rn. Please find the updated template in my PR in web modeler where you can instantly see it in the modeler by clicking one of the deployments. https://github.com/camunda/web-modeler/pull/3530

MaxTru · 2023-01-16T09:21:33Z

Note: the deploy branch can be found here (e.g., for testing)

MaxTru

Maybe I miss something, but:

Where would I put the GraphQL query ?
Where would I put the GraphQL variables?

Can you please add a usage example (e.g., a screenshot of a typical GraphQL examples using this connector)

MaxTru · 2023-01-16T09:25:42Z

connectors/graphql/element-templates/template-connector.json

+  "id": "io.camunda.connectors.GraphQL.v1",
+  "description": "Execute GraphQL query",
+  "version": 1,
+  "documentationRef": "https://docs.camunda.io/docs/components/modeler/web-modeler/connectors/available-connectors/template/",


This link is dead

I missed the variables, thank you for noticing. Fixed it, and the link also. See https://github.com/camunda/web-modeler/pull/3530

MaxTru · 2023-01-16T09:37:19Z

Consider example by @christian-konrad , @bastiankoerber :

MaxTru · 2023-01-17T13:28:10Z

Hey @markfarkas-camunda ,

is this ready for re-review? If so, please press the "request re-review" button:

This way @bastiankoerber and I know when we should look at this again.

markfarkas-camunda · 2023-01-17T14:18:58Z

@MaxTru I re-requested review here: https://github.com/camunda/web-modeler/pull/3530 now, thank you for reminding me, I had issues with the automatic deployment, I resolved that, so now you should be able to see the fixed template. I won't re-request the review here, my other PR should be the one to review.

nikku · 2023-01-27T10:56:06Z

connectors/graphql/connector.yml

@@ -0,0 +1,3 @@
+name: HTTPJSON


Should be GraphQL maybe?

Obviously. Thank you for noticing. 👍

chillleader

Tested the basic functionality - looks great!
Left a couple of generic comments for discussion.

chillleader · 2023-01-27T11:03:09Z

connectors/graphql/src/main/java/io/camunda/connector/graphql/GraphQLFunction.java

+    inputVariables = {
+      "url",
+      "method",
+      "authentication",
+      "query",
+      "variables",
+      "connectionTimeoutInSeconds"
+    },


A side comment: I think we might want to consider changing the naming pattern to connectorName.variableName, e.g. graphql.url here, because the current behavior of input mappings of variables with the same name is sometimes surprising. I have a feeling it may lead to a conflict between different connectors.

I didn't know about this, good catch. I will modify the input variables.

chillleader · 2023-01-27T11:06:03Z

connectors/graphql/src/main/java/io/camunda/connector/graphql/auth/Authentication.java

+import com.google.api.client.http.HttpHeaders;
+import com.google.common.base.Objects;
+
+public abstract class Authentication {


Many of the data classes are identical here and in REST connector. Should we move the common data classes into a separate module, e.g. connectors-common-library?

nikku · 2023-01-27T13:01:14Z

Looks good overall template UI wise 👍.

Nitty-gritty: It feels like we can further simplify our descriptions. I like the usage of links to external resources though.

I personally prefer call to actions like Learn how to <define variables> or <Configure variables> to send with the query over Variables to add to the query or mutation. See <Documentation>.

nikku · 2023-01-27T14:18:55Z

Nit-pick: Swap Method and URL (URL is important, method should be pre-filled with the most reasonable option (POST?):

markfarkas-camunda · 2023-01-27T15:26:41Z

Nit-pick: Swap Method and URL (URL is important, method should be pre-filled with the most reasonable option (POST?):

Make sense, but I would still choose GET as a pre-filled option. It is possible to execute a mutation with GET, although according to the official doc "by convention it's suggested that one doesn't use GET requests to modify data".
It is the customer's responsibility to use the right option here.
What we can do maybe is recognize that the query is a mutation(start with the keyword "mutation"), and automatically set the method to post, or make the field red if its value is GET. @nikku do you think something like this is possible on the frontend?

nikku · 2023-01-27T15:31:13Z

Make sense, but I would still choose GET as a pre-filled option.

Is it also a convention that queries are executed via GET? If that is the case I agree with you.

markfarkas-camunda · 2023-01-27T16:28:27Z

Is it also a convention that queries are executed via GET? If that is the case I agree with you.

If we follow the REST conventions GET should be the right choice for queries, although there are no rules regarding this in the docs. Theoretically there can be a limitation on the length of a url, hence on the query parameter, but that is very unlikely we will ever encounter this limitation. For example for a Windows IIS WebServer the default value for max length of query string is 2048 bytes according to the docs
I would still vote for GET as pre-filled option.

christian-konrad · 2023-01-27T18:23:21Z

@bastiankoerber do you want us building syntax highlighting for the GraphQL query/mutation part? I could at least open an issue for that in the core modeling team

nikku · 2023-01-27T21:20:10Z

~~@christian-konrad Please open an issue and link it.~~

I opened an issue: bpmn-io/bpmn-js-properties-panel#858.

We'd need to fix the GraphQL embedding one way or the other:

A proper GraphQL editor OR
A monospace textarea that resizes to a decent size (so contents are not hidden)

It leaves a bad impression to edit / view GraphQL queries in a non-monospace apperance:

bundle/mvn/default-bundle/src/main/resources/application.properties

igpetrov · 2023-02-10T09:20:04Z

...onnectors-common-library/src/main/java/io/camunda/connector/common/utils/ResponseParser.java

 /*
 * Copyright Camunda Services GmbH and/or licensed to Camunda Services GmbH
- * under one or more contributor license agreements. See the NOTICE file
- * distributed with this work for additional information regarding copyright
- * ownership. Camunda licenses this file to you under the Apache License,
- * Version 2.0; you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *     http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
+ * under one or more contributor license agreements. Licensed under a proprietary license.
+ * See the License.txt file for more information. You may not use this file
+ * except in compliance with the proprietary license.
 */


A question regarding a licensing. With a common library we apply Camunda OSS license, while HTTP REST connector was Apache 2.0. How is it resolved now? Is the HTTP REST still under Apache 2.0 despite few components being under under Camunda OSS?

@markfarkas-camunda @bastiankoerber @chillleader WDYT?

Yeah I also have the same question. I think at least the common-library should definitely be Apache 2.0.

igpetrov · 2023-02-10T09:21:11Z

connectors/graphql/LICENSE.txt

+Copyright Camunda Services GmbH and/or licensed to Camunda Services GmbH under one or more contributor license agreements and licensed to you under a proprietary license.
+You may not use this file except in compliance with the proprietary license.
+The proprietary license can be either the Camunda Platform Self-Managed Free Edition license (available on Camunda’s website) or the Camunda Platform Self-Managed Enterprise Edition license (a copy you obtain when you contact Camunda).
+The Camunda Platform Self-Managed Free Edition comes for free but only allows for usage of the software (file) in non-production environments.
+If you want to use the software (file) in production, you need to purchase the Camunda Platform Self-Managed Enterprise Edition.


Since we have REST connector under Apache 2.0, does it make sense to make GraphQL Apache 2.0 as well?

cc: @bastiankoerber

igpetrov · 2023-02-10T09:24:56Z

connectors/graphql/src/main/java/io/camunda/connector/graphql/model/GraphQLRequestWrapper.java

+
+public class GraphQLRequestWrapper {
+  @NotBlank private GraphQLRequest graphql;
+  private Authentication authentication;


Shouldn't it be marked as @Secret?

Not necessarily, we do not want to replace any secrets in these two attributes. This is just a helper class to handle the graphql namespace. (graphql.url, graphql.method, etc.)

igpetrov · 2023-02-10T09:28:58Z

pom.xml

@@ -6,7 +6,7 @@
  <parent>
    <groupId>io.camunda.connector</groupId>
    <artifactId>connector-parent</artifactId>
-    <version>0.6.0-SNAPSHOT</version>
+    <version>0.6.0-alpha3</version>


Should it be still SNAPSHOT in PR?

It needs to use the latest release with the relevant commit to handle secrets in inherited classes.

chillleader

I really like it - nicely done!
I just have a minor concern about licensing.

chillleader · 2023-02-10T09:13:05Z

bundle/mvn/default-bundle/src/main/resources/application.properties

+#camunda.connector.webhook.enabled=true
+##spring.main.web-application-type=none
+


nit: don't forget to remove these 😄

.../connectors-common-library/src/main/java/io/camunda/connector/common/model/CommonResult.java

chillleader · 2023-02-10T09:21:32Z

...connectors-common-library/src/main/java/io/camunda/connector/common/constants/Constants.java

 * Copyright Camunda Services GmbH and/or licensed to Camunda Services GmbH
- * under one or more contributor license agreements. See the NOTICE file
- * distributed with this work for additional information regarding copyright
- * ownership. Camunda licenses this file to you under the Apache License,
- * Version 2.0; you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *     http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
+ * under one or more contributor license agreements. Licensed under a proprietary license.
+ * See the License.txt file for more information. You may not use this file
+ * except in compliance with the proprietary license.


Do we want to license the GraphQL connector and the common library under proprietary license or Apache 2.0?

REST Connector is licensed under Apache 2.0, so I believe it makes sense to also have an open source license at least for common-library. And we can keep the GraphQL connector proprietary.

igpetrov · 2023-02-10T09:32:54Z

...common-library/src/main/java/io/camunda/connector/common/services/AuthenticationService.java

+    setTimeout(request, httpRequest);
+    HttpHeaders headers = new HttpHeaders();
+
+    if (authentication.getClientAuthentication().equals(Constants.BASIC_AUTH_HEADER)) {


It's still worse to maintain null-safety.

Suggested change

if (authentication.getClientAuthentication().equals(Constants.BASIC_AUTH_HEADER)) {

if (Constants.BASIC_AUTH_HEADER).equals(authentication.getClientAuthentication()) {

igpetrov · 2023-02-10T09:33:27Z

...common-library/src/main/java/io/camunda/connector/common/services/AuthenticationService.java

+    data.put(Constants.AUDIENCE, authentication.getAudience());
+    data.put(Constants.SCOPE, authentication.getScopes());
+
+    if (authentication.getClientAuthentication().equals(Constants.CREDENTIALS_BODY)) {


Same here regarding null-safety

igpetrov · 2023-02-10T09:35:40Z

...onnectors-common-library/src/main/java/io/camunda/connector/common/services/HTTPService.java

+          errorCode = errorContent.getErrorCode();
+          errorMessage = errorContent.getError();
+        } catch (Exception e) {
+          // cannot be loaded as JSON, ignore and use plain message


Can we put a log statement here?

WARN should be sufficient.

igpetrov · 2023-02-10T09:37:48Z

connectors/graphql/src/main/java/io/camunda/connector/graphql/GraphQLFunction.java

+    context.validate(connectorRequest);
+    context.replaceSecrets(connectorRequest);
+
+    return proxyFunctionUrl == null


If we have Apache Commons lib here, consider StringUtils.isBlank(proxyFunctionUrl)

Or String::isBlank since we're using Java 11 here. But this would still require a null check.

igpetrov

No critical issues found, thus approving.
Minor suggestions.

Please make sure you talk about licensing issues with @bastiankoerber before release.

Thank you.

markfarkas-camunda · 2023-02-10T12:23:40Z

I fixed the issues and suggestions, I am waiting for Bastian's decision about licensing.

igpetrov · 2023-02-10T14:20:04Z

connectors/connectors-common-library/pom.xml

+    <properties>
+    <license.inlineheader>Copyright Camunda Services GmbH and/or licensed to Camunda Services GmbH
+under one or more contributor license agreements. See the NOTICE file
+distributed with this work for additional information regarding copyright
+ownership. Camunda licenses this file to you under the Apache License,
+Version 2.0; you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.</license.inlineheader>
+    </properties>


I think, should be this one

Suggested change

<properties>

<license.inlineheader>Copyright Camunda Services GmbH and/or licensed to Camunda Services GmbH

under one or more contributor license agreements. See the NOTICE file

distributed with this work for additional information regarding copyright

ownership. Camunda licenses this file to you under the Apache License,

Version 2.0; you may not use this file except in compliance with the License.

You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software

distributed under the License is distributed on an "AS IS" BASIS,

WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.

See the License for the specific language governing permissions and

limitations under the License.</license.inlineheader>

</properties>

<licenses>

<license>

<name>Camunda Platform Self-Managed Free Edition license</name>

<url>https://camunda.com/legal/terms/cloud-terms-and-conditions/camunda-cloud-self-managed-free-edition-terms/</url>

</license>

<license>

<name>Camunda Platform Self-Managed Enterprise Edition license</name>

</license>

</licenses>

<properties>

<license.inlineheader>Copyright Camunda Services GmbH and/or licensed to Camunda Services GmbH

under one or more contributor license agreements. Licensed under a proprietary license.

See the License.txt file for more information. You may not use this file

except in compliance with the proprietary license.</license.inlineheader>

</properties>

Actually, disregard this comment. I think, it's okay to keep it as Apache 2.0 since it's a common lib.

igpetrov · 2023-02-10T14:23:47Z

connectors/graphql/pom.xml

+  <licenses>
+    <license>
+      <name>Camunda Platform Self-Managed Free Edition license</name>
+      <url>https://camunda.com/legal/terms/cloud-terms-and-conditions/camunda-cloud-self-managed-free-edition-terms/</url>
+    </license>
+    <license>
+      <name>Camunda Platform Self-Managed Enterprise Edition license</name>
+    </license>
+  </licenses>


This one is correct.

igpetrov

Looks good to me. Thank you!

feat: GraphQL connector template

d437444

markfarkas-camunda added the scope:graphql label Jan 11, 2023

markfarkas-camunda requested review from MaxTru and bastiankoerber January 11, 2023 17:19

fix(graphql): update license headers

856f06e

bastiankoerber requested changes Jan 16, 2023

View reviewed changes

MaxTru requested changes Jan 16, 2023

View reviewed changes

feat(graphql): implement GraphQL GET method without variables

4c4a942

markfarkas-camunda self-assigned this Jan 23, 2023

markfarkas-camunda added 3 commits January 23, 2023 19:06

feat(graphql): implement POST method and variable handling

949d43e

fix(graphql): additional character escape, and cleanup

d9afadf

feat(graphql): modify application.properties, code restructure

3400db7

markfarkas-camunda requested review from MaxTru, bastiankoerber and chillleader January 26, 2023 17:33

nikku reviewed Jan 27, 2023

View reviewed changes

chillleader reviewed Jan 27, 2023

View reviewed changes

fix(graphql): make input variable names graphql specific

6a111bd

fix(graphql): swap url and method in the element template

1ce9f74

markfarkas-camunda added 4 commits February 9, 2023 16:33

chore(graphql): add README

8768ae5

chore(graphql): fix README format

ae158c9

Merge branch 'main' into 153-create-template-for-graphql-deploy

7a853aa

chore(graphql): update links to point to internal documentation

5b8dbab

markfarkas-camunda marked this pull request as ready for review February 9, 2023 16:04

markfarkas-camunda requested review from nikku and chillleader February 9, 2023 16:04

igpetrov reviewed Feb 10, 2023

View reviewed changes

bundle/mvn/default-bundle/src/main/resources/application.properties Outdated Show resolved Hide resolved

igpetrov reviewed Feb 10, 2023

View reviewed changes

chillleader approved these changes Feb 10, 2023

View reviewed changes

igpetrov reviewed Feb 10, 2023

View reviewed changes

igpetrov approved these changes Feb 10, 2023

View reviewed changes

markfarkas-camunda added 2 commits February 10, 2023 11:36

fix(graphql): common library licensing, null safety

32238f2

fix(graphql): fix code style

518a5ba

igpetrov self-requested a review February 10, 2023 14:17

igpetrov reviewed Feb 10, 2023

View reviewed changes

igpetrov approved these changes Feb 10, 2023

View reviewed changes

chillleader approved these changes Feb 10, 2023

View reviewed changes

markfarkas-camunda merged commit 2e303e1 into main Feb 10, 2023

markfarkas-camunda deleted the 153-create-template-for-graphql-deploy branch February 10, 2023 14:45

markfarkas-camunda mentioned this pull request Feb 14, 2023

[GraphQL Connector] Implement backend functionality #226

Closed

chillleader pushed a commit that referenced this pull request Apr 21, 2023

renamed ProcessVariable class in docs to avoid confusion (closes #202)

df64475

		#camunda.connector.webhook.enabled=true
		##spring.main.web-application-type=none

	if (authentication.getClientAuthentication().equals(Constants.BASIC_AUTH_HEADER)) {
	if (Constants.BASIC_AUTH_HEADER).equals(authentication.getClientAuthentication()) {

feat: GraphQL connector template #202

feat: GraphQL connector template #202

Conversation

markfarkas-camunda commented Jan 11, 2023 • edited Loading

Description

UPDATE

Related issues

bastiankoerber left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MaxTru commented Jan 16, 2023

MaxTru left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MaxTru commented Jan 16, 2023

MaxTru commented Jan 17, 2023

markfarkas-camunda commented Jan 17, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chillleader left a comment

Choose a reason for hiding this comment

chillleader Jan 27, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nikku commented Jan 27, 2023 • edited Loading

nikku commented Jan 27, 2023

markfarkas-camunda commented Jan 27, 2023

nikku commented Jan 27, 2023

markfarkas-camunda commented Jan 27, 2023 • edited Loading

christian-konrad commented Jan 27, 2023

nikku commented Jan 27, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chillleader left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chillleader Feb 10, 2023 • edited Loading

Choose a reason for hiding this comment

igpetrov left a comment

Choose a reason for hiding this comment

markfarkas-camunda commented Feb 10, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

igpetrov left a comment

Choose a reason for hiding this comment

markfarkas-camunda commented Jan 11, 2023 •

edited

Loading

chillleader Jan 27, 2023 •

edited

Loading

nikku commented Jan 27, 2023 •

edited

Loading

markfarkas-camunda commented Jan 27, 2023 •

edited

Loading

nikku commented Jan 27, 2023 •

edited

Loading

chillleader Feb 10, 2023 •

edited

Loading