Skip to content

Can not extend RetryClient because it's final #991

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

Closed
techouse opened this issue Jul 15, 2023 · 2 comments
Closed

Can not extend RetryClient because it's final #991

techouse opened this issue Jul 15, 2023 · 2 comments
Labels
package:http type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)

Comments

@techouse
Copy link

techouse commented Jul 15, 2023

Hi,

In 0.13.6 I used to extend the RetryClient in order to add caching support inside the send method, for example

@override
Future<StreamedResponse> send(BaseRequest request) async {
  if (request.method == 'GET') {
    /// Check if there is a cached response with this URL
    final CachedResponse? cachedResponse = store.getCachedResponse(request);

    /// In case we have the old cached response
    if (cachedResponse != null) {
      return cachedResponse.streamedResponse;
    }

    /// In case there is no cache send the request
    final StreamedResponse response = await super.send(request);

    /// If the response is OK cache it
    if (response.statusCode >= HttpStatus.ok &&
        response.statusCode < HttpStatus.multipleChoices) {
      /// Grab the stream
      final Uint8List body = await response.stream.toBytes();

      /// Cache it
      store.cacheResponse(request, response, body);

      /// Return a copy with a fresh stream
      return response.copyWith(
        stream: Stream<List<int>>.value(
          List<int>.from(body),
        ),
      );
    }

    /// Return the response
    return response;
  }

  /// Non-GET requests are not cached
  return super.send(request);
}

In #920 the class got the final modifier and the only way for me to add caching now would be to completely copy the RetryClient and maintain it.

Could you explain why this decision was taken to add the final modifier to the RetryClient but not to the IOClient which I can still extend with this kind of caching strategy?

@techouse techouse added package:http type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) labels Jul 15, 2023
@SamJakob
Copy link

SamJakob commented Jul 17, 2023

Have you considered making a ‘CachingClient’ (as you’ve shown) and passing it into ReplyClient or vice versa? I would think that’s the intended usage.

That would probably give you more flexibility (to allow mixing and matching different clients together - e.g., to add caching to some other client) and be consistent with the composable principle behind the API. The use of final is possibly to encourage (force) this kind of design pattern.

(Edit: I’m not the maintainer or original designer by the way - but I would suspect — that this applies to ReplyClient and not IOClient or BrowserClient because the former is not a ‘base client’; it is intended to wrap another client as defined in this package, whilst the latter two wrap dart:io and dart:html - i.e., the behavior/implementation come from something outside of this package’s API surface that may need additional extensions or alteration by the consumer. That is, they’re not designed with composition in mind so it’s riskier to make them final as a general rule.)

@techouse
Copy link
Author

Have you considered making a ‘CachingClient’ (as you’ve shown) and passing it into ReplyClient or vice versa? I would think that’s the intended usage.

Yeah, you're probably right. 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package:http type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)
Projects
None yet
Development

No branches or pull requests

2 participants