lejard-h / chopper

Chopper is an http client generator using source_gen and inspired from Retrofit.
https://hadrien-lejard.gitbook.io/chopper
Other
714 stars 123 forks source link

[chopper_built_value] @Query does not use serializers to serialize query params #332

Open danielgomezrico opened 2 years ago

danielgomezrico commented 2 years ago

Current

I use BuiltValue with EnumClass to have nice enums, but Im trying to use an enum as a @Query param and it uses the toString value in the request instead of using the serializer value.

I have an Enum that is declared like:

class TurnVisitType extends EnumClass {
  const TurnVisitType._(String name) : super(name);

  static Serializer<TurnVisitType> get serializer => _$turnVisitTypeSerializer;
...

  @BuiltValueEnumConst(wireName: 'face_to_face')
  static const TurnVisitType faceToFace = _$faceToFace;

  static const TurnVisitType virtual = _$virtual;

The generated serializer:

class _$TurnVisitTypeSerializer implements PrimitiveSerializer<TurnVisitType> {
  static const Map<String, Object> _toWire = const <String, Object>{
    'faceToFace': 'face_to_face',
  };
  static const Map<Object, String> _fromWire = const <Object, String>{
    'face_to_face': 'faceToFace',
  };

  @override
  final Iterable<Type> types = const <Type>[TurnVisitType];
  @override
  final String wireName = 'TurnVisitType';

  @override
  Object serialize(Serializers serializers, TurnVisitType object,
          {FullType specifiedType = FullType.unspecified}) {
    return _toWire[object.name] ?? object.name;
  }

And my service like this:

  @Get(path: 'api/v1/available_turn_schedules/{id}')
  Future<Response<List<ScheduleResponse>>> getSchedules(
      @Path('id') int realEstateId,
      @Query('visity_type') TurnVisitType visitType);

But when I see how the request is done, it sends faceToFace (visitType.toString() value) instead of the serializer TurnVisitType.serializer and the wiredValue and send face_to_face.

Some places on chopper

It does use toString on every query param, and it does not try to use the serializes.

Here: https://github.com/lejard-h/chopper/blob/f558bc7aabc36e819611cc7001d8f37f59dae309/chopper/lib/src/request.dart#L126

Here: https://github.com/lejard-h/chopper/blob/82b951fe278fcd783de5285b4ef2b7cbbede61c9/chopper/lib/src/utils.dart#L76-L82

Expected

The url is built using the serializer if the type have it declared, and

danielgomezrico commented 2 years ago

👀

JEuler commented 2 years ago

Sorry, just dropped out of life a bit. Will have a look later.

techouse commented 1 year ago

@danielgomezrico is this still an issue?

danielgomezrico commented 1 year ago

@techouse yeah

techouse commented 1 year ago

@danielgomezrico Getting to the bottom of this slowly.

From some initial tests I've written, the support for these built_value serializers only extends to encoding and decoding the payload, not the query params, as per definition

/// An interface for implementing request and response converters.
///
/// [Converter]s convert objects to and from their representation in HTTP.
///
/// [convertRequest] is called before [RequestInterceptor]s
/// and [convertResponse] is called just after the HTTP response,
/// before [ResponseInterceptor]s.
///
/// See [JsonConverter] and [FormUrlEncodedConverter] for example implementations.
@immutable
abstract class Converter {
  /// Converts the received [Request] to a [Request] which has a body with the
  /// HTTP representation of the original body.
  FutureOr<Request> convertRequest(Request request);

  /// Converts the received [Response] to a [Response] which has a body of the
  /// type [BodyType].
  ///
  /// `BodyType` is the expected type of the resulting `Response`'s body
  /// \(e.g., `String` or `CustomObject)`.
  ///
  /// If `BodyType` is a `List` or a `BuiltList`, `InnerType` is the type of the
  /// generic parameter (e.g., `convertResponse<List<CustomObject>, CustomObject>(response)` ).
  FutureOr<Response<BodyType>> convertResponse<BodyType, InnerType>(
    Response response,
  );
}

To encode this in the Query will require quite some work.

radzish commented 1 year ago

I am missing this too. What is even worse is that I can not write my own converter to handle query params as I need. Request converter will be only applied when body is not empty.

techouse commented 1 year ago

I am missing this too. What is even worse is that I can not write my own converter to handle query params as I need.

@radzish you can always submit a PR 🤗

dustin-graham commented 1 year ago

Ran into this problem with regular enums. One way to work around this is to write custom toString() method:

@override
String toString() {
  return this.name;
}
techouse commented 1 year ago

@dustin-graham yep, that's how I always work with it.

I'll take another look at this over the Xmas holidays unless someone beats me to it 🤞

techouse commented 7 months ago

Not sure if this resolves this particular custom serialization issue, but last week I replaced the old query serialization with my own port of qs in https://github.com/lejard-h/chopper/pull/592. qs has the ability to use custom Encoders, however, that's not exposed in Chopper. Have a play with it and see if it works for your particular built_value issue, otherwise do as suggested above and always add a toString() method.

CC / @dustin-graham @radzish @danielgomezrico