Request extends http.BaseRequest

[techouse]

I think that Request should inherit from BaseRequest.

The added benefit of this is that Request ceases to be a Chopper-specific class and now behaves like a BaseRequest.

The only downside is that Request can no longer be immutable and have a const constructor as BaseClass and all its children also aren't immutable and don't have const constructors.

class Request extends http.BaseRequest {
  /// I renamed this to [path] because it is essentially the same as [Uri.path]
  final String path;
  /// I renamed this to [origin] because it is essentially the same as [Uri.origin]
  final String origin;
  final dynamic body;
  final Map<String, dynamic> parameters;
  final bool multipart;
  final List<PartValue> parts;
  final bool useBrackets;

  Request(
    String method,
    this.path,
    this.origin, {
    this.body,
    this.parameters = const {},
    Map<String, String> headers = const {},
    this.multipart = false,
    this.parts = const [],
    this.useBrackets = false,
  }) : super(
          method,
          /// here I used [buildUrl] to construct a [Uri] and use it as the [url] parameter 
          /// of the parent [BaseRequest] constructor
          buildUri(origin, path, parameters, useBrackets: useBrackets),
        ) {
    /// Updating [BaseRequest.headers] is just nasty 🤪
    this.headers.addAll(headers);
  }

  /// Build the Chopper [Request] using a [Uri] instead of a [path] and [origin].
  /// Both the query parameters in the [Uri] and those provided explicitly in
  /// the [parameters] are merged together.
  Request.uri(
    String method,
    Uri url, {
    this.body,
    Map<String, dynamic>? parameters,
    Map<String, String> headers = const {},
    this.multipart = false,
    this.parts = const [],
    this.useBrackets = false,
  })  : origin = url.origin,
        path = url.path,
        parameters = {...url.queryParametersAll, ...?parameters},
        super(
          method,
          buildUri(
            url.origin,
            url.path,
            {...url.queryParametersAll, ...?parameters},
            useBrackets: useBrackets,
          ),
        ) {
    this.headers.addAll(headers);
  }

  /// These methods that were before sitting outside are now class methods
  /// Their API if therefore now vastly simpler.

  /// Convert this [Request] to a [http.Request]
  @visibleForTesting
  http.Request toHttpRequest() {
    // ... stuff
  }
  
  /// Convert this [Request] to a [http.MultipartRequest]
  @visibleForTesting
  Future<http.MultipartRequest> toMultipartRequest() async {
    // ... stuff
  }
  
  /// Convert this [Request] to a [http.StreamedRequest]
  @visibleForTesting
  http.StreamedRequest toStreamedRequest(Stream<List<int>> bodyStream) {
    // ... stuff
  }

  // ... stuff
}

I'll work on this PR over the coming days to add some more tests and fix some broken ones.

Lemme know if you even agree with this. 🙈

[techouse]

I noticed that for some reason it was expected for buildUri to do this:

buildUri('foo/', '/bar'); // --> 'foo//bar'

I believe that to be an error. This should be either

• foo/bar (merging both so a single /)
• /bar (the / in the 2nd part denotes a root which discards the 1st part)

I went with he 1st option: a single slash

buildUri('foo/', '/bar'); // --> 'foo/bar'

[JEuler]

Hm, why absolute paths btw? :) Personal preference?

[techouse]

Because of the lint always_use_package_imports and the fact that I hate mixing the 2. Even in tests, it makes me cringe.

Check this Google Sheet https://docs.google.com/spreadsheets/d/1Nc1gFjmCOMubWZD7f2E4fLhWN7LYaOE__tsA7bf2NjA/edit#gid=0

[JEuler]

Heh, I was always using https://dart-lang.github.io/linter/lints/prefer_relative_imports.html but anyway, okay! пн, 10 окт. 2022 г. в 19:32, Klemen Tusar ***@***.***>:

…

***@***.**** commented on this pull request. ------------------------------ In chopper/lib/src/annotations.dart <#370 (comment)>: > @@ -1,11 +1,10 @@ import 'dart:async'; +import 'package:chopper/src/constants.dart'; Because of this https://dart-lang.github.io/linter/lints/always_use_package_imports.html Check this Google Sheet https://docs.google.com/spreadsheets/d/1Nc1gFjmCOMubWZD7f2E4fLhWN7LYaOE__tsA7bf2NjA/edit#gid=0 — Reply to this email directly, view it on GitHub <#370 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABYUDSXXJI7XSFHPSNWFZDWCRAITANCNFSM6AAAAAARAW5GIM> . You are receiving this because you were assigned.Message ID: ***@***.***>

-- Terekhin Ivan

[JEuler]

Thank you so much for hard work!

[JEuler]

I think the 'immutability' in this case is not 'important' 👍

[techouse]

Thank you so much for hard work!

My pleasure. 😇