marcglasberg / fast_immutable_collections

Dart Package: Immutable lists, sets, maps, and multimaps, which are as fast as their native mutable counterparts. Extension methods and comparators for native Dart collections.
BSD 2-Clause "Simplified" License
217 stars 30 forks source link

Json Serialization Support #6

Closed TimWhiting closed 3 years ago

TimWhiting commented 3 years ago

Looks great, just glanced through it and am interested in using it in some projects!

Just a question / feature request. Is there json serialization support?

See for example: json_serializable_immutable_collections which adds support for serializing other immutable collections to json and back.

Since the collections are iterables I'm sure they probably serialize fine without any issue. The problem is to deserialize them back to an immutable form.

marcglasberg commented 3 years ago

Yes, they are iterables, except the IMap. A map is not an iterable, even if at first glace it would seem to be an iterable of MapEntrys (but it isn't).

If you are doing your own serialization you can deserialize to regular collections and then simply use the unsafe constructors like IList.unsafe() (or lockUnsafe getter) to create the immutable collections with no performance losses at all.

However, if you actually want it to be compatible with json_serializable the collections must be included in json_serializable_immutable_collections instead of being a feature of FIC itself.

I've opened an issue there: https://github.com/knaeckeKami/json_serializable_immutable_collections/issues/14

knaeckeKami commented 3 years ago

Yes, I will add support for these collections soon :)

marcglasberg commented 3 years ago

So, this is done, but I'll leave it open for the moment, until I update the documentation to point to that package.

knaeckeKami commented 3 years ago

so, there is another way to make this work without an external package. There's no additional dependency needed at all actually. After a discussion with @rrousselGit he pointed out that classes with from/toJson methods/factories like this:

class MyCustomContainer<T> {
  final List<T> baseList;

  MyCustomContainer(this.baseList);

  factory MyCustomContainer.fromJson(
      dynamic json,T Function(Object?) fromJsonT) {
    return MyCustomContainer((json as Iterable).map(fromJsonT).toList());
  }

  Object toJson(Object Function(T) toJsonT) {
    return this.baseList.map(toJsonT).toList();
  }
}

Can be used like that:

@JsonSerializable()
class ObjectWithCustomContainer {
  final MyCustomContainer<String> myList;

  ObjectWithCustomContainer(this.myList);

  factory ObjectWithCustomContainer.fromJson(Map<String, dynamic> json) =>
      _$ObjectWithCustomContainerFromJson(json);

  Map<String, dynamic> toJson() => _$ObjectWithCustomContainerToJson(this);
}

Note that the custom collection does not have any dependency on json_serializable or json_annotation. Just these two methods/factories are required. For example. this tests succeeds:

  test("serialization works", (){

    expect(ObjectWithCustomContainer(MyCustomContainer<String>(["1", "2"])).toJson(), {"myList" : ["1", "2"]});

  });

  test("deserialization works", (){

    expect(ObjectWithCustomContainer.fromJson({"myList" : ["1", "2"]}).myList, isA<MyCustomContainer<String>>());

  });

So if you are willing to add this to fast_immutable_collections, it would just work without custom build_runner builders.

This of course has the disadvantage of a slighter bigger API surface and no clean separation between the serialization layer and the rest of the functionality, but the user experience is better.

marcglasberg commented 3 years ago

I don't mind the bigger API surface. I will implement this.

marcglasberg commented 3 years ago

@knaeckeKami

Do you know what I should actually use for the IMap?

Just published version [3.0.0] where I added these to the IList:

  /// Converts from JSon. Json serialization support for json_serializable with @JsonSerializable.
  factory IList.fromJson(dynamic json, T Function(Object?) fromJsonT) =>
      IList((json as Iterable).map(fromJsonT));

  /// Converts to JSon. Json serialization support for json_serializable with @JsonSerializable.
  Object toJson(Object Function(T) toJsonT) => map(toJsonT).toList();

And these to the ISet:

  /// Converts from JSon. Json serialization support for json_serializable with @JsonSerializable.
  factory ISet.fromJson(dynamic json, T Function(Object?) fromJsonT) =>
      ISet((json as Iterable).map(fromJsonT));

  /// Converts to JSon. Json serialization support for json_serializable with @JsonSerializable.
  Object toJson(Object Function(T) toJsonT) => map(toJsonT).toList();
knaeckeKami commented 3 years ago

Since Map has 2 generic parameters, you need two extra parameters, like this:

@JsonSerializable(genericArgumentFactories: true)
class Either<First, Second> {
  Either(this.first, this.second);
  factory Either.fromJson(
    Map<String, Object?> json,
    First Function(Object?) fromJsonFirst,
    Second Function(Object?) fromJsonSecond,
  ) {
    throw UnimplementedError();
  }
  First first;
  Second second;
}

@JsonSerializable()
class Another {
  Another(this.value, this.foo);
  factory Another.fromJson(Map<String, Object?> json) =>
      _$AnotherFromJson(json);
  final Either<int, String> foo;
}

Note that the naming of the function parameters has to match the names of the generic parameters, so for a generic parameter First the function parameter name has to be fromJsonFirst.

For the map, I would implement it somewhat like this:

class MyMap<K,V> {

  factory MyMap.fromJson(
    Map<String, Object?> json,
    K Function(Object?) fromJsonK,
    V Function(Object?) fromJsonV,
  ) {
     return json.map((key,value) => MapEntry(fromJsonK(key), fromJsonValue(value)).toImap();
  }

 Object toJson(Object Function(Key) toJsonKey, Object Function(Value) toJsonValue) {
    return this.unlock.map((key, value) => MapEntry(toJsonKey(key), toJsonValue(value));
  }

}

I didn't actually run or compile it, but the approach should work I think. If you want to we can have a call and look at it together.

marcglasberg commented 3 years ago

Ok, I have added this to the IMap:

  /// Converts from JSon. Json serialization support for json_serializable with @JsonSerializable.
  factory IMap.fromJson(
    Map<String, Object?> json,
    K Function(Object?) fromJsonK,
    V Function(Object?) fromJsonV,
  ) =>
      json.map<K, V>((key, value) => MapEntry(fromJsonK(key), fromJsonV(value))).lock;

  /// Converts to JSon. Json serialization support for json_serializable with @JsonSerializable.
  Object toJson(Object Function(K) toJsonK, Object Function(V) toJsonV) =>
      unlock.map((key, value) => MapEntry(toJsonK(key), toJsonV(value)));
marcglasberg commented 3 years ago

@knaeckeKami

Question: Is https://pub.dev/packages/json_serializable_fic still relevant? If so, I'll link to it from the docs.

rrousselGit commented 3 years ago
  /// Converts from JSon. Json serialization support for json_serializable with @JsonSerializable.
  factory IList.fromJson(dynamic json, T Function(Object?) fromJsonT) =>
      IList((json as Iterable).map(fromJsonT));

Why accept dynamic and not Iterable?

marcglasberg commented 3 years ago

@rrousselGit You mean it should be this?

factory IList.fromJson(Iterable json, T Function(Object?) fromJsonT) =>
      IList<T>json.map(fromJsonT));

I have no experience with json_serializable, never used it. I'll just put it there what you guys tell me to use.

knaeckeKami commented 3 years ago

Question: Is https://pub.dev/packages/json_serializable_fic still relevant? If so, I'll link to it from the docs.

No, if you support IList, IMap, and ISet, then my package is irrelevant and I'll deprecate it.

I have no experience with json_serializable, never used it. I'll just put it there what you guys tell me to use.

If Iterable works, then of course Iterable is better, I was not sure if json_serializable would support it.

We should probably add a test project for end to end tests with fast_immutable_collection and json_serializable to make sure everything works correctly. I'll make a PR when I have time.

rrousselGit commented 3 years ago

An e2e test is definitely important

From my tests a few weeks ago, it appeared that json_serializable supported fromJson(List<Object?> json)/List<Object?> toJson(). But it's worth double checking.

marcglasberg commented 3 years ago

Added this to the docs:

11. Json support
With some help from Martin Kamleithner and Remi Rousselet, now most FIC collections convert to and from Json, through .fromJson() and .toJson().

This means those FIC collections can be used with json_serializable in classes annotated with @JsonSerializable.

For example:

@JsonSerializable()
class MyClass {
  final IList<String> myList;
  MyClass(this.myList);
  factory MyClass.fromJson(Map<String, dynamic> json) => _$MyClassFromJson(json);
  Map<String, dynamic> toJson() => _$MyClassToJson(this);
}

Maybe I should add a warning that it's still experimental.

TimWhiting commented 3 years ago

Just tried to update to use the latest version:

I have a list with nulls having a meaning, and the toJson doesn't allow null

  Object toJson(Object Function(T) toJsonT) => map(toJsonT).toList();
   //                ^ this

  Object toJson(Object? Function(T) toJsonT) => map(toJsonT).toList();
   //                 ^ should be changed to this

I can always use some other value in the meantime, but maybe add an e2e test for this specific case? Probably a similar thing for the values of a map. I don't imagine either of them will have null being used excessively, but there are exceptions.

marcglasberg commented 3 years ago

@TimWhiting Could you please try version [5.0.0-dev.9] ?

TimWhiting commented 3 years ago

Perfect, that fixed it for me!