Skip to content
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

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

Open
danielgomezrico opened this issue Feb 23, 2022 · 10 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@danielgomezrico
Copy link
Contributor

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:

Uri buildUri(String baseUrl, String url, Map<String, dynamic> parameters) {

Here:

if (value is Iterable) {
pairs.addAll(_iterableToQuery(name, value));
} else if (value is Map<String, dynamic>) {
pairs.addAll(_mapToQuery(value, prefix: name));
} else if (value.toString().isNotEmpty == true) {
pairs.add(_Pair<String, String>(name, _normalizeValue(value)));
}

Expected

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

@danielgomezrico

This comment was marked as spam.

@JEuler
Copy link
Collaborator

JEuler commented Aug 27, 2022

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

@techouse
Copy link
Collaborator

@danielgomezrico is this still an issue?

@techouse techouse added question Further information is requested no-issue-activity investigation The issue needs further investigation labels Feb 18, 2023
@danielgomezrico
Copy link
Contributor Author

@techouse yeah

@techouse techouse removed question Further information is requested no-issue-activity labels Feb 20, 2023
@techouse techouse changed the title @Query does not use serializers to serialize query params @Query does not use built_value serializers to serialize query params Mar 12, 2023
@techouse
Copy link
Collaborator

techouse commented Mar 12, 2023

@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.

@techouse techouse added enhancement New feature or request and removed investigation The issue needs further investigation labels Mar 12, 2023
@techouse techouse self-assigned this Mar 12, 2023
@techouse techouse changed the title @Query does not use built_value serializers to serialize query params [chopper_built_value] @Query does not use serializers to serialize query params Mar 12, 2023
@techouse techouse added the help wanted Extra attention is needed label Mar 12, 2023
@radzish
Copy link

radzish commented Apr 5, 2023

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
Copy link
Collaborator

techouse commented Aug 20, 2023

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
Copy link

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
Copy link
Collaborator

@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
Copy link
Collaborator

techouse commented Apr 11, 2024

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 #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

@techouse techouse removed their assignment Oct 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

5 participants