Split up `MockServer` `reply` method into static and dynamic version

[sebastianbuechler]

Description

Currently, we only have one reply method on the MockServer class that takes data of type dynamic. I would propose to split this function into replyStatic and replyCallback (or something similar), so that instead of the dynamic type for the data we can have a proper signature.

In the example of replyCallback it would then look like this:

  @override
  void replyCallback(
    int statusCode,
    MockDataCallback data, {
    Map<String, List<String>> headers = const {
      Headers.contentTypeHeader: [Headers.jsonContentType],
    },
    String? statusMessage,
    bool isRedirect = false,
    Duration? delay,
  }) {
    final isJsonContentType = headers[Headers.contentTypeHeader]?.contains(
          Headers.jsonContentType,
        ) ??
        false;

    mockResponse = (requestOptions) {
      var rawData = data(requestOptions);

      return MockResponseBody.from(
        isJsonContentType ? jsonEncode(rawData) : rawData,
        statusCode,
        headers: headers,
        statusMessage: statusMessage,
        isRedirect: isRedirect,
        delay: delay,
      );
    };
  }

The advantage of this change is that when registering a route you'll get type-safety for the callbacks and it's actually easier for beginners with the package so see all the available information. It took me quite some time to realize that the callback parameter is actually of type RequestOptions from Dio.

[LukaGiorgadze]

Thanks @sebastianbuechler makes sense, currently I don't have to make this change, unfortunately, but you can make PR and I'll approve. Or I can address this in the future.

[sebastianbuechler]

@LukaGiorgadze Hi, I opened up #157 with two adjustments. I think it could be nice to even go a step further and deprecate the reply method in favor of dedicated methods for each response type. It's straight forward for callbacks and async callbacks I guess, but for static JSON responses we could think about introducing a new sealed class like this here: https://github.com/dart-lang/language/issues/83#issuecomment-1465846202

This would allow full typesafety for all cases of response variants. What do you think?

[LukaGiorgadze]

Hi @sebastianbuechler, your proposal makes sense, I agree with that. Thank you! Just make sure tests are passed since they are failing https://github.com/lomsa-dev/http-mock-adapter/actions/runs/5923116833/job/16058693248?pr=157

And will merge this.

I upgraded dependencies and upgraded the collection to 1.18.0 but since the stable version of Flutter still depends on an older version of the collection, I'll first downgrade the collection package and then release your changes with a major version since it might be a breaking change for some people.

btw, what version of Flutter and http-mock-adapter do you use?

[sebastianbuechler]

@LukaGiorgadze Okay, I will try another PR separately with the Json sealed class approach then.

For #157 I adjusted the unit tests (had to await the statusHandler call because it's now a future for all cases), added two new cases (one for each new method), and regrouped the existing tests for the request handler. Do those changes make sense for you?

I saw that collection was just upgraded to 1.18.0 a couple of days ago, so I had to manually pin the version on my test repo in order to use it. I'm currently using Flutter 3.10.5 and will soon upgrade to latest. I was creating the issue with http-mock-adapter v0.4.4 and built the PR with v0.5.0.

Btw, what's up with the matcher section of the project? How can I make use of it? I would also like to add some documentation for new users of the project as I think the project is awesome and can help a lot of people.

[LukaGiorgadze]

@sebastianbuechler Thank you Sebastian, updating the document makes really sense. I'm thinking to downgrade the collection back, my gut feeling tells me Flutter stable is far away from upgrading the collection, so does it make sense to keep 1.18? I'm curious how it works for you.

[LukaGiorgadze]

https://github.com/lomsa-dev/http-mock-adapter/pull/157 has been merged

[sebastianbuechler]

@LukaGiorgadze I have the same feeling that it still will take some time until will be useful. I don't see currently a reason to not downgrade, to be honest.

[LukaGiorgadze]

@LukaGiorgadze I have the same feeling that it still will take some time until will be useful. I don't see currently a reason to not downgrade, to be honest

@sebastianbuechler Look at this https://github.com/lomsa-dev/http-mock-adapter/issues/155 Did you face the same issue?

[LukaGiorgadze]

@sebastianbuechler btw, I've invited you as a collaborator with write access.

[sebastianbuechler]

@LukaGiorgadze I have the same feeling that it still will take some time until will be useful. I don't see currently a reason to not downgrade, to be honest

@sebastianbuechler Look at this #155 Did you face the same issue?

Yes, exactly. I just set an override in my pubspec.yaml:

dependency_overrides:
  collection: ^1.17.1 # Temporary fix for http_mock_adapter

[sebastianbuechler]

@sebastianbuechler btw, I've invited you as a collaborator with write access.

Thanks you!