Rename and deprecate Zeebe classes

[nicpuppa]

Description

With this PR all the Zeebe.... classes are replaced or deprecated in favor of more general classes.

Related issues

closes #19485
closes #19488

[nicpuppa]

Sorry for the big amount of files @tmetzke, let me know if you want to review it in a pairing session. I tried to divide the changes into separate commits (was hard 🤣)

[tmetzke]

❓ This will mean a lot of code changes for users, I guess? Do we really need this?

[nicpuppa]

I added this method to not make users change their code. Because I can't change/override the return type of the send method

[tmetzke]

Oh, yeah, I see. Well, that's a bit tricky 🤔
I'll try to find some time to think about it again.

[tmetzke]

Thinking about this again, I would still like to avoid introducing this new method.
What happens if we return a CamundaFuture from the send method here and the CamundaFuture extends the ZeebeFuture?
If I currently have a variable final ZeebeFuture result, the send method should still work, shouldn't it? I now receive a CamundaFuture but that is just a specialized ZeebeFuture, it still fits into the ZeebeFuture and my code should still be fine. I'll still only have access to the ZeebeFuture-part of the return CamundaFuture but that should be OK.

[nicpuppa]

It's not doable. CamundaFuture extends ZeebeFuture, if we change the return type also users might need to do that

[nicpuppa]

Let me try

[nicpuppa]

Seems not working as expected

[nicpuppa]

Oh because tasklist is using mockito on the return type. So if users have implemented some tests and they stub the return type they need to adjust them. This is not something that we want now @tmetzke

[tmetzke]

OK, I see the issue. It is a breaking change for sure since we're adjusting the return type of the method.
Well, I guess, if we want to stay backward-compatible, we'll have to introduce a new method 😞
Sorry for the noise and additional work. We can also represent it on Monday again in the team and ask for feedback but I think there aren't many ways around it...

[nicpuppa]

I will rollback the changes, and raise it to the team

[tmetzke]

💭 I am wondering: should we tackle the "extends" mechanism the other way around, i.e. HttpZeebeFuture extends HttpCamundaFuture?

More details:

• All implementation code sits in the new Camunda... classes and moves from the io.camunda.zeebe.client package to io.camunda.client.
• All Zeebe... classes would extend the Camunda classes so they still exist for easier migration. If Zeebe classes are not needed externally, we don't extend the respective Camunda classes.
• Under the hood, all code already works with Camunda classes.
• With 8.8, we can remove the io.camunda.zeebe.client package in one swoop, no further adjustments should be necessary.
• Methods like those in the HttpClient could be called with a HttpCamundaFuture or HttpZeebeFuture. The method signature changes to use the HttpCamundaFuture. A deprecated HttpZeebeFuture could be handed in since it fulfills the contract, i.e. it is a HttpCamundaFuture.

I might overlook something that makes the transition for people harder, of course 🙂

[nicpuppa]

I will try to follow this approach and let you know 👍

[nicpuppa]

It seems more complex then I thought. I got stuck migrating/renaming the clients implementation and all related interfaces

[tmetzke]

I was fearing that would happen, yeah 🙈

[tmetzke]

Let's keep the "extends" definitions as they are.
For the client here, to avoid duplicating methods, you could also write them as follows, I believe:

  public <HttpT, RespT, FutureT extends HttpZeebeFuture<RespT>> void get(
      final String path,
      final RequestConfig requestConfig,
      final Class<HttpT> responseType,
      final JsonResponseTransformer<HttpT, RespT> transformer,
      final FutureT result) {
    sendRequest(Method.GET, path, null, requestConfig, responseType, transformer, result);
  }

The crucial part would be the FutureT extends HttpZeebeFuture<RespT> type definition and using that as the type for the result parameter.
That way, you can hand in HttpCamundaFuture and HttpZeebeFuture.

[tmetzke]

Keeping the deprecated methods and adding the ones for the new type work just as well, of course. However, since this is an internal class, I was wondering if we actually need to keep the deprecated methods at all or if we can simply adjust all calling methods and use the new HttpCamundaFuture there.

[nicpuppa]

Yes, it make more sense to just use the old methods with HttpCamundaFuture

[tmetzke]

Sounds like the cleanest solution, if possible.

[tmetzke]

❓ At some point, we need to move this class to the new package name as well. Better we do that now, I guess? Can this be considered a breaking change? 🙈

[nicpuppa]

This is going to be a breaking change, so it is not doable now

[tmetzke]

So, we're introducing the breaking change with 8.8 then?

[nicpuppa]

This is the plan I think

[tmetzke]

❓ Just wondering: is this less of a breaking change for users than returning a CamundaFuture instead of a ZeebeFuture?

[nicpuppa]

You are right, I think we need to take the same approach used for the command here. Creating a new method with a different return type

[tmetzke]

We could also let the Zeebe... classes simply still return their Zeebe... counterparts wherever possible to be as minimally invasive as possible. Then we should only have very few points where it "collides" (e.g. FinalCommandStep).

[nicpuppa]

Or, just duplicate the code here

[nicpuppa]

Something like:

public final class CamundaClientBuilderImpl implements
    CamundaClientBuilder, CamundaClientConfiguration 

and

@Deprecated
public class ZeebeClientBuilderImpl implements ZeebeClientBuilder, ZeebeClientConfiguration 

[tmetzke]

Yeah, I was also just thinking:
Can we simply duplicate the code completely into the io.camunda.client package, rename everything there and at the critical conversion points take care of having both and handle it accordingly. With 8.8, we simply remove the io.camunda.zeebe.client package, rename the conversion point classes like FinalCommandStep, and remove their deprecated methods like send().

[tmetzke]

It's a bit ugly for now and for patching later though, as the code is duplicated completely.

[nicpuppa]

Let's leave this as an open point for Monday's team meeting. Deeper we go, deeper I'm thinking about just duplicating everythin 🤣