Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

added console client #604

Closed
wants to merge 5 commits into from
Closed

added console client #604

wants to merge 5 commits into from

Conversation

jonathanlukas
Copy link
Collaborator

This PR will add a generated console client, wrap it with a handsome api and also add it to the spring boot starter.

It is based on https://github.com/camunda-community-hub/camunda-console-client-java which will be archived after this PR is merged.

@jonathanlukas jonathanlukas requested a review from 1nb0und January 19, 2024 09:07
@jonathanlukas jonathanlukas self-assigned this Jan 19, 2024
<goal>generate</goal>
</goals>
<configuration>
<inputSpec>https://console.cloud.camunda.io/customer-api/openapi/swagger.json</inputSpec>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is some cool stuff. I never used a generator before. I need to see if shall we go the generator route, or write out the API models ourselves. Wondering what are the advantages and disadvantages.

The route I've been going is to manually add the classes and use sensible names (since TaskList, Operate and others from what I've seen are using different convention). I understand that going thru the generated route reduces time to market, and maintenance I assume would be easier since whenever they update their version we can just generate again and modify the wrapper classes calling it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can go the generator route for all APIs that offer some kind of spec.

I see that this kind of refactoring would probably cost some effort, but also reduce the amount of code to be maintained.

Copy link
Contributor

@1nb0und 1nb0und left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is looking awesome. I think I want to improve a couple of things by

  1. see if we still want to use the generator approach (the other modules are hand-crafted models). We can go the route of this being generator, and the remaining are hand-crafted and see how will versioning and maintenance will look like whenever there is a change in the API
  2. I'd like to reuse the existing http module. Theres some refactoring involved.

I'm more than happy to do those changes, but wanted to get your opinion. Overall this speeds things up and this is greatly valued!

Comment on lines +26 to +42
public DefaultApi get() {
ensureApiClient();
return api;
}

private void ensureApiClient() {
if (api == null) {
HttpRequestInterceptor interceptor =
new AuthenticationInterceptor(Product.CONSOLE, authentication);
CloseableHttpClient httpClient =
HttpClients.custom().addRequestInterceptorFirst(interceptor).build();
ApiClient apiClient = new ApiClient(httpClient);
apiClient.setBasePath(consoleUrl);
api = new DefaultApi(apiClient);
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to reuse the existing java-sdk-common.http package for this rather than creating another HttpClient

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The HttpClient you created is not compatible with the generated client as this one relies on the selected http client framework (apache in this case).

This is why I added the AuthenticationInterceptor.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand that your own http client helps us being independent of any http client framework, however the amount of code required to bind our implementation against a framework is REALLY limited (like 1 class with 10 loc)

@jonathanlukas
Copy link
Collaborator Author

jonathanlukas commented Jan 22, 2024

@1nb0und (and maybe @berndruecker ):

Maybe my 2 cents for generated client vs. implemented client (and forget what I wrote above, I wrapped my head around this for a while):

As an API is implemented API-first (define open api spec, generate server stub and implement it), there is a revisioned resource openapi.json available to work with. Here, a generated client makes sense.

If however, the API is implemented impl-first (write server stub, generate open api spec at runtime), there is no real resource available to implement against. Here, we could rely on an implemented client.

wdyt?

@jonathanlukas jonathanlukas requested a review from 1nb0und January 22, 2024 12:06
@jonathanlukas jonathanlukas requested review from sbuettner and removed request for 1nb0und February 5, 2024 13:29
@jonathanlukas jonathanlukas deleted the feature/console-client branch March 22, 2024 05:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants