#279 async service client

[hohwille]

Implements #279

[hohwille]

I fixed the tests, but now travis is failing with:

The job exceeded the maximum log length, and has been terminated.

I will see if I can raise the log threshold for some frameworks like CXF or kafka to workaround this problem but already maven downloads seem 20% of the log.

[hohwille]

Working now, but still some open issues:

• when doing a REST GET request with PathParam containing a whitespace, the service client performs URL encoding of the value transforming the whitespace to become a plus sign (+). However, Apache CXF on the server side does not URL decode such PathParams leading to a received value with a plus sign instead of the whitespace. What is the correct behaviour here? Is CXF correct and do path params only need to encode chars such as questionmark (?) ? Or did I find a bug in CXF?
• the documentation needs to be updated
• I need feedback on the API. I wanted to name callVoid just call but due to erasure for some reason Oracle compiler pervents this even though Eclipse compiler did accept it. Any better ideas how to deal with this?
• Should we also have support returning CompletableFuture? IMHO the Consumer callback is sufficient for almost all async cases. If you need to wait for the result, you could even call synchronously. However, in some cases you may want to call multiple services in parallel async but then aggregate the different results and process when all of them are complete.
  While this may still be archived by chaining the callbacks it might be a little bit more tricky to do. WDYT? --> Created issue Support for CompletableFuture in async service client #283 for this feature.

[hohwille]

For the first point, I found the answer here:
https://stackoverflow.com/questions/13626990/jax-rs-automatic-decode-pathparam

You need to encode space as %20 (or leave it as space). Encoding it as + is only valid for query params.
So this nees to be fixed...

[hohwille]

Any feedback on the two open issues about the API in my comment above from anyone?

[maybeec]

I need feedback on the API. I wanted to name callVoid just call but due to erasure for some reason Oracle compiler pervents this even though Eclipse compiler did accept it. Any better ideas how to deal with this?

I think this might be a bug of the JDK. Did you even tried it with oracle or just openjdk or even adoptJdk?
Anyhow, in any case, I think it might be valuable to raise a bug for it as it it truly overloading rather than type erasure. Given type erasure you would have a method signature Object Object whereas the other function is Runnable Object. So I don't see what's the issue here. Even without Type erasure there is no conflict even. In any case of substitution of R, you cannot get the method signature of callVoid.

Should we also have support returning CompletableFuture? IMHO the Consumer callback is sufficient for almost all async cases. If you need to wait for the result, you could even call synchronously. However, in some cases you may want to call multiple services in parallel async but then aggregate the different results and process when all of them are complete. While this may still be archived by chaining the callbacks it might be a little bit more tricky to do. WDYT?

I think CompletableFuture is very valuable for example for uploads. You cannot go for a synchronous call as of timeout issues during communication, but you actually want to wait until the upload is completed to process further on the client side.

[hohwille]

@maybeec Thank you for your feedback.

I think CompletableFuture is very valuable ...

Thanks. So we will consider it for some future release after 2020.08.001. I created issue #283 for this. It is not hard to do and can be easily added to the existing API. However, for the current release it is getting too late and we have learned about last minute features.

I think this might be a bug of the JDK ... it might be valuable to raise a bug

Sorry, but I completely gave up with this for Eclipse or Oracle compiler. It is ultra complex, they have very few and very limited experts on this. They do not care much about your feedback unless you provide exact, minimal,automated tests and including a PhD thesis proving this is a bug ;) See here for all my experience and time I already wasted:
m-m-m/util#166

Further, you might got mistaken. I stated that I named both methods call before. Because of the bug I had to rename one to callVoid instead. The JDK compiler is not wrong here as for the generic R you could in an odd edge-case fill in Runnable and then the method call void would indeed be ambiguous.
My actual question is more if there is a better name for callVoid or a better signature to prevent the ambiguity?

Futher, as nobody dares to approve... Should I use admin permission to merge this PR?
We are getting out of time and still need some time for other assets like devonfw-ide to do changes that require devon4j being released.

[maybeec]

Further, you might got mistaken. I stated that I named both methods call before. Because of the bug I had to rename one to callVoid instead. The JDK compiler is not wrong here as for the generic R you could in an odd edge-case fill in Runnable and then the method call void would indeed be ambiguous.
My actual question is more if there is a better name for callVoid or a better signature to prevent the ambiguity?

Got the point. I think callVoid is fine. I don't fine any better name either.

[hohwille]

Got the point. I think callVoid is fine. I don't fine any better name either.

Just to open options. Instead of this API:

Consumer<Void> resultHandler = r -> { System.out.println("Response received")};
client.callVoid(() -> { client.get().deleteEntity(id);}, resultHandler);

It could work like this:

Consumer<Void> resultHandler = r -> { System.out.println("Response received")};
client.get().deleteEntity(id);
client.call(resultHandler);

I desinged the current approach to avoid that users invoke the call[Void] method without the corresponding call of the according service operation method (here in this example deleteEntity). In customer projects I already collected experience with proprietary components for something like this and it turned out that developers easily get mistaken and wonder why it is not working. Surely, you can still provdide a runnable that is not calling the client (() --> {}) but the API taking a Runnable is forcing the developer to think "whats this? I better read the documentation how to use this.".

Another alternative is the general "switch parameters" poor-man solution:

Consumer<Void> resultHandler = r -> { System.out.println("Response received")};
client.call(resultHandler, () -> { client.get().deleteEntity(id);});

As callVoid is making something actually more explicit by the name, it is IMHO actually not so bad and these switching args is often causing more confusion to API users.

[hohwille]

@maybeec thanks for approval. As we are approaching our release, I will merge this tomorrow. Feedback is still very welcome but only if you can give it quickly there are high chances we can respect them before our 2020.08 release.

[hohwille]

For the record: I changed the name of the starter so we can later implement #284 without breaking changes and sane naming. So with #284 I plan to introduce devon4j-starter-http-client-rest (name was taken and has now been renamed to devon4j-starter-http-client-rest-async that only supports async REST client) that then will support both async and sync REST client via HTTP. Users can use this and get rid of CXF for clients if they like. All working smooth with the same API with no code changes and config happens by adding/removing starters.