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

Record list serialization unexpectedly converted to a map by StandardJsonPlugin. #1295

Closed
Leptopoda opened this issue Jan 1, 2024 · 5 comments · Fixed by #1296
Closed

Record list serialization unexpectedly converted to a map by StandardJsonPlugin. #1295

Leptopoda opened this issue Jan 1, 2024 · 5 comments · Fixed by #1296
Assignees
Labels

Comments

@Leptopoda
Copy link
Contributor

Leptopoda commented Jan 1, 2024

Hi there,
I hope y'all had calm holidays and a good start into 2024.

Problem:

I'm trying to serialize a record with a list into a json list. Unfortunately the StandardJsonPlugin is converting this record into a json map.

I have the following scenario:

typedef MyRecord = ({
  BuiltList<String>? list,
  int? number,
});

extension $MyRecordExtension on MyRecord {
  static Serializer<MyRecord> get _serializer => const MyRecordSerializer();
  static MyRecord fromJson(Object? json) => _$jsonSerializers.deserializeWith(_serializer, json)!;
  Object? toJson() => _$jsonSerializers.serializeWith(_serializer, this);
}

  test('MyRecord serialization', () {
    MyRecord object = (
      list: BuiltList<String>(),
      number: null,
    );

    Object? json = [];

    expect(object.toJson(), equals(json)); // results in {}
    expect($MyRecordExtension.fromJson(json), equals(object));

    object = (
      list: BuiltList<String>(['value1', 'value2', 'value3']),
      number: null,
    );

    json = ['value1', 'value2', 'value3'];

    expect(object.toJson(), equals(json)); // results in {"value1", "value2"}
    expect($MyRecordExtension.fromJson(json), equals(object));
  });

The registered serializer looks like this:

class MyRecordSerializer implements PrimitiveSerializer<MyRecord> {
  const MyRecordSerializer();

  @override
  Iterable<Type> get types => const [MyRecord];

  @override
  String get wireName => 'MyRecord';

  @override
  Object serialize(
    Serializers serializers,
    MyRecord object, {
    FullType specifiedType = FullType.unspecified,
  }) {
    dynamic value;
    value = object.list;
    if (value != null) {
      return serializers.serialize(value, specifiedType: const FullType(BuiltList, [FullType(String)]))!;
    }
    value = object.number;
    if (value != null) {
      return serializers.serialize(value, specifiedType: const FullType(int))!;
    }

	// Should not be possible after validation.
    throw StateError('Tried to serialize without any value.');
  }

  @override
  MyRecord deserialize(
    Serializers serializers,
    Object data, {
    FullType specifiedType = FullType.unspecified,
  }) {
    BuiltList<String>? list;
    try {
      list = serializers.deserialize(
        data,
        specifiedType: const FullType(BuiltList, [FullType(String)]),
      )! as BuiltList<String>;
    } catch (_) {}
    int? number;
    try {
      number = serializers.deserialize(data, specifiedType: const FullType(int))!
          as int;
    } catch (_) {}
    
    return (number: number, list: list);
  }
}

Cause:

The issue lies in this part of the StandardJsonPlugin. Is there any easy way to mitigate this?

  @override
  Object? afterSerialize(Object? object, FullType specifiedType) {
    if (object is List &&
        specifiedType.root != BuiltList &&
        specifiedType.root != BuiltSet &&
        specifiedType.root != JsonObject) {
      if (specifiedType.isUnspecified) {
        return _toMapWithDiscriminator(object);
      } else {
        return _toMap(object, _needsEncodedKeys(specifiedType));
      }
    } else {
      return object;
    }
  }
@Leptopoda Leptopoda changed the title Record list deserialization unexpectedly converted to a map by StandardJsonPlugin. Record list serialization unexpectedly converted to a map by StandardJsonPlugin. Jan 1, 2024
@davidmorgan
Copy link
Collaborator

A PrimitiveSerializer is only supposed to return simple primitives, per the class docs:

/// Returns a value that can be represented as a JSON primitive: a boolean,
/// an integer, a double, or a String.

I think you will need to use StructuredSerializer, which must always return an Iterable, and then pack either the list or the int inside that; I don't think there is a way to have data that is sometimes primitive (int) and sometimes structured (list).

@davidmorgan davidmorgan self-assigned this Jan 8, 2024
@Leptopoda
Copy link
Contributor Author

Leptopoda commented Jan 9, 2024

This is not going to work. Returning

['wireName', ['element1', 'element2']]

from the serializer will result in

{
  "wireName": [
    "element1", 
    "element2"
  ]
}

which is not the desired plain list

[
  "element1", 
  "element2"
]

One would need to provide a list of root types that are ignored by the StandartJsonPligin (similar to how it already ignores BuiltList, BuiltSet and JsonObject).

Are you willing to support such a use case or do we need to write our own JsonPlugin handling this functionality?

@davidmorgan
Copy link
Collaborator

Does it work for you to do something in StandardJsonPlugin like

final BuiltSet<Type> typesToLeaveAsList;

defaults to BuiltList, BuiltSet, JsonObject and can be passed in via the constructor? ... i.e. have you tried it? If it works for your use case then that is a good sign :)

It's not obvious to me that this will work 100%, since it was never intended to allow a serializer that sometimes returns an int and sometimes returns a list. That said I notice now that this is already how JsonObjectSerializer works, so I guess it probably does work :)

@Leptopoda
Copy link
Contributor Author

Yes this does work (with the above PrimitiveSerializer),
Should I open a PR?

@davidmorgan
Copy link
Collaborator

Yes please :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants