In StandardJsonPlugin, deserialization fails when the value in a Map is NULL

[tamcy]

Consider the following model class:

abstract class Announcement implements Built<Announcement, AnnouncementBuilder> {
  static Serializer<Announcement> get serializer => _$announcementSerializer;

  BuiltMap<String, String> get remarks;

  Announcement._();

  factory Announcement([updates(AnnouncementBuilder b)]) = _$Announcement;
}

Here remarks is a Map that stores localized values. When deserializing with StandardJsonPlugin, the following data works fine:

    {
        "remarks": {
            "en_US": "Remarks EN",
            "zh_HK": "Remarks ZH"
        }
    }

But this example will fail:

    {
        "remarks": {
            "en_US": null,
            "zh_HK": null
        }
    }

And the error is as below:

I/flutter (29052): Deserializing '[remarks, {en_US: null, zh_HK: null}]' to 'Announcement' failed due to: Deserializing
I/flutter (29052): '[null, null, null, null]' to 'BuiltMap<String, String>' failed due to: Invalid argument(s): null
I/flutter (29052): key

Looks like this is caused by the function StandardJsonPlugin's _toListUsingDiscriminator, which converts the Map to a List:

built_value.dart/built_value/lib/standard_json_plugin.dart

Lines 159 to 202 in 2480cb4

	List _toListUsingDiscriminator(Map map) {
	var type = map[discriminator];
	if (type == null) {
	throw ArgumentError('Unknown type on deserialization. '
	'Need either specifiedType or discriminator field.');
	}
	if (type == 'list') {
	return [type]..addAll(map[valueKey] as Iterable);
	}
	if (map.containsKey(valueKey)) {
	// Just a type and a primitive value. Retrieve the value in the map.
	final result = List(2);
	result[0] = type;
	result[1] = map[valueKey];
	return result;
	}
	// A type name of `encoded_map` indicates that the map has non-String keys
	// that have been serialized and JSON-encoded; decode the keys when
	// converting back to a `List`.
	var needToDecodeKeys = type == 'encoded_map';
	if (needToDecodeKeys) {
	type = 'map';
	}
	var result = List(map.length * 2 - 1);
	result[0] = type;
	var i = 1;
	map.forEach((key, value) {
	if (key == discriminator) return;
	// Drop null values, they are represented by missing keys.
	if (value == null) return;
	result[i] = needToDecodeKeys ? _decodeKey(key as String) : key;
	result[i + 1] = value;
	i += 2;
	});
	return result;
	}

In this function, keys with NULL value are simply skipped (line 195). I don't know the rationale behind this, but since the length of the list is fixed to the sum of keys and values of the Map (line 187), this causes two NULL values at the end of the list whenever a NULL value in the Map is encountered and skipped. Which is why the second JSON sample data results in a List with four NULLs, and the code fails when attempting to use the NULL as the key in _toMap():

built_value.dart/built_value/lib/standard_json_plugin.dart

Lines 80 to 88 in 2480cb4

	Map _toMap(List list, bool needsEncodedKeys) {
	var result = <String, Object>{};
	for (int i = 0; i != list.length ~/ 2; ++i) {
	final key = list[i * 2];
	final value = list[i * 2 + 1];
	result[needsEncodedKeys ? _encodeKey(key) : key as String] = value;
	}
	return result;
	}

Seems the "lightest" fix would be to break the loop in _toMap() when key is NULL (this may affect other plugins though). What do you think?

[tamcy]

I am now working around the issue using the following plugin:

class RemoveNullInMapConvertedListPlugin implements SerializerPlugin {
  Object beforeSerialize(Object object, FullType specifiedType) => object;

  Object afterSerialize(Object object, FullType specifiedType) => object;

  Object beforeDeserialize(Object object, FullType specifiedType) {
    if (specifiedType.root == BuiltMap && object is List) {
      return object.where((v) => v != null).toList();
    }

    return object;
  }

  Object afterDeserialize(Object object, FullType specifiedType) => object;
}

But I'd prefer those keys with NULL values are preserved in the final BuiltMap, and still don't understand why it was decided that StandardJsonPlugin doesn't work this way.

[davidmorgan]

Thanks for the detailed report!

That code was written with objects in mind; when a field is null, built_value just omits the value, so nulls are not needed when deserializing. The intent was just to ignore them, but as you describe, it doesn't actually work.

BuiltMap can't contain null keys, so there's no way to actually keep the null. Given that, I think the correct fix is to make it actually ignore the nulls as originally intended.

[jffiorillo]

Thank you @tamcy for the workaround... @davidmorgan do you have plan to solve this issue anytime soon? In the meanwhile, @tamcy workaround should be used?

Thank you @davidmorgan

[davidmorgan]

No ETA I'm afraid; I'll try to include it with the next batch of fixes/improvements.

Yes, the workaround given above is the right way to go for now.

[davidmorgan]

#1049