Ideas on how to handle supporting multiple versions of the API (0.18.x, 0.19.x)

[hjiangsu]

There's been some more changes to the API since the last major refactor (see #10). The changes are listed here: LemmyNet/lemmy-js-client@0.19.0-rc.14...0.19.0-alpha.16.

While most of the changes are fairly minor, there is one change that I'd like to have a discussion on. This is with regards to changes to the output of the API responses for specific endpoints. I wanted to start a discussion here to see if anyone has any ideas on how best we can support the changes to the API responses.

I do want to clarify that most of the changes resulting in different API responses are endpoints that we don't currently use (with the exception of a few). Depending on the result of this discussion, we might end up only partially supporting 0.18.x (and dropping 0.18.x support for endpoints whose API responses have changed that we don't currently use).

For example:

  markPostAsRead(form: MarkPostAsRead) {
    return this.#wrapper<MarkPostAsRead, PostResponse>(
      HttpType.Post,
      "/post/mark_as_read",
      form,
    );
  }

has changed to

  markPostAsRead(form: MarkPostAsRead) {
    return this.#wrapper<MarkPostAsRead, SuccessResponse>(
      HttpType.Post,
      "/post/mark_as_read",
      form,
    );
  }

This means that MarkPostAsRead no longer returns back PostResponse but instead returns SuccessResponse in the latest 0.19.x API.

Currently, the class that handles this API call looks like this:

@freezed
class MarkPostAsRead with _$MarkPostAsRead implements LemmyApiQuery<PostResponse>, LemmyApiAuthenticatedQuery {
  @apiSerde
  const factory MarkPostAsRead({
    int? postId,
    List<int>? postIds, // Available in lemmy v0.19.0 and above
    required bool read,
    String? auth,
  }) = _MarkPostAsRead;

  const MarkPostAsRead._();
  factory MarkPostAsRead.fromJson(Map<String, dynamic> json) => _$MarkPostAsReadFromJson(json);

  final path = '/post/mark_as_read';

  final httpMethod = HttpMethod.post;

  @override
  PostResponse responseFactory(Map<String, dynamic> json) => PostResponse.fromJson(json);
}

---

There's a couple of options I can think of at the moment. Feel free to suggest any other ideas you may have!

1. Create two MarkPostAsRead classes, each one targeting a specific version of the API. We would then call the respective class within Thunder depending on the instance version. If we want to fully support 0.18.x, then we would have to do this for each API call whose response has changed
2. Find a way to make the same class return different responses back to the client (Thunder). Feel free to suggest ideas on how to do this

CC: @micahmo

[shortwavesurfer2009]

how difficult would it be to create multiple classes to support different API versions? If it's just a matter of copy and paste, and a few changes, then it might be worth doing that, at least until 0.18.x. Drops to a low threshold of instances (say 2% for example) and then remove the 0.18.x classes in favor of the 0.19.x classes. This may only be a few months depending on instance administration.

[hjiangsu]

how difficult would it be to create multiple classes to support different API versions?

It wouldn't be too difficult to do that since it's more or less duplicating the class and changing the result of the response to the one that matches the associated version.

The main difficulty comes from Thunder's end, where we would need to add in logic to all the API calls which have multiple "versions" and call the correct one based on the instance. It's not impossible to do, but could get tedious if there's a lot of changes between API versions.

Generally speaking, I'd like to try to keep 2 fully supported versions at most at a given time (e.g., 0.18.x and 0.19.x). Once a newer version comes, then we would drop support for the oldest version (e.g., drop official support for 0.18.x once 0.20.x starts rolling out)

[micahmo]

Our approach thus far has been to try to make the API changes transparent to the calling code. While a caller might check compatibility for a particular new call using supportsFeature, we were kind of hoping that any existing calls (that haven't been removed) could stay untouched. In the case where the return type has changed, that obviously becomes more difficult. I can see adding code to handle these changes either at the app level (check version before calling and have two endpoint definitions), in a wrapper class (all callers use the wrapper, which checks version and makes appropriate call), or even in the API (so that the changes are relatively transparent to the caller, and the API can do direct deserialization, maybe even without checking the version first).

But the problem really comes down to the response type and how to handle multiple potential types in the code that uses it. My inclination would be to see if we can map any of the old types to the new type. For example, it seems like the Lemmy devs determined that putting the whole post in PostResponse was overkill when we just need to know whether it succeeded. Therefore, I wonder if we could translate all PostResponses to SuccessResponses (for example, just by checking that PostResponse.postView is not null). In other words, although the type has changed, we can essentially ascertain the same information either way. I'm not sure how or where this mapping would occur, but it might make things slightly easier on us.

[micahmo]

I thought of another option which would be to make a response model which contains both properties as optional. Then the caller can check if one or the other is set to determine success. To make this easier, we could put an extension method on the response object so that callers would still only have to check one thing.

I like this option because it doesn't require any version checking or fancy deserialization, and it has minimal impact on callers.

[hjiangsu]

First, thanks for the detailed response and providing ideas and general feedback!

I thought of another option which would be to make a response model which contains both properties as optional. Then the caller can check if one or the other is set to determine success.

Hmm, this is an interesting idea. I don't mind doing it this way since it somewhat mimics how we are deprecate 0.18.x attributes (making the attribute optional and adding @deprecated)!

To make this easier, we could put an extension method on the response object so that callers would still only have to check one thing.

Maybe you could give an example of how we can make the extension method so that I can visualize it better?

[micahmo]

Maybe you could give an example of how we can make the extension method so that I can visualize it better?

Sure! The beauty of extensions is that you can essentially add instance methods/properties to a class without modifying the class. It's super useful when you're dealing with third-party classes. In this case, we do control the models, but it still might make sense to have the success checker as an extension in Thunder code.

I mocked this out in DartPad. Hopefully it makes sense!

https://dartpad.dev/32b3bc309d89877baa2fa17cc04f437e

[hjiangsu]

Thanks for the example, I'll probably go with this suggestion of merging attributes of both responses into one for the API. Then, we can add the extension as you suggested within Thunder!