From 772ee3e9fbc4dcdab223ae5d69e88168194d62d0 Mon Sep 17 00:00:00 2001 From: Jeff Martin Date: Wed, 23 Mar 2022 10:06:20 -0600 Subject: [PATCH 01/12] JSON serialization emitDefuaults option --- protobuf/lib/protobuf.dart | 1 + .../lib/src/protobuf/generated_message.dart | 5 +- .../protobuf/json_serialization_context.dart | 9 +++ protobuf/lib/src/protobuf/proto3_json.dart | 42 ++++++++++++-- protobuf/test/json_test.dart | 58 +++++++++++++++++++ 5 files changed, 108 insertions(+), 7 deletions(-) create mode 100644 protobuf/lib/src/protobuf/json_serialization_context.dart diff --git a/protobuf/lib/protobuf.dart b/protobuf/lib/protobuf.dart index e2d5cdcb2..bed6b135e 100644 --- a/protobuf/lib/protobuf.dart +++ b/protobuf/lib/protobuf.dart @@ -13,6 +13,7 @@ import 'dart:typed_data' show TypedData, Uint8List, ByteData, Endian; import 'package:fixnum/fixnum.dart' show Int64; import 'src/protobuf/json_parsing_context.dart'; +import 'src/protobuf/json_serialization_context.dart'; import 'src/protobuf/permissive_compare.dart'; import 'src/protobuf/type_registry.dart'; export 'src/protobuf/type_registry.dart' show TypeRegistry; diff --git a/protobuf/lib/src/protobuf/generated_message.dart b/protobuf/lib/src/protobuf/generated_message.dart index 1c7359774..504b07ea3 100644 --- a/protobuf/lib/src/protobuf/generated_message.dart +++ b/protobuf/lib/src/protobuf/generated_message.dart @@ -233,8 +233,9 @@ abstract class GeneratedMessage { /// message encoding a type not in [typeRegistry] is encountered, an /// error is thrown. Object? toProto3Json( - {TypeRegistry typeRegistry = const TypeRegistry.empty()}) => - _writeToProto3Json(_fieldSet, typeRegistry); + {TypeRegistry typeRegistry = const TypeRegistry.empty(), + bool emitDefaults = false}) => + _writeToProto3Json(_fieldSet, typeRegistry, emitDefaults); /// Merges field values from [json], a JSON object using proto3 encoding. /// diff --git a/protobuf/lib/src/protobuf/json_serialization_context.dart b/protobuf/lib/src/protobuf/json_serialization_context.dart new file mode 100644 index 000000000..502e23e88 --- /dev/null +++ b/protobuf/lib/src/protobuf/json_serialization_context.dart @@ -0,0 +1,9 @@ +// Copyright (c) 2022, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +class JsonSerializationContext { + final bool emitDefaults; + + JsonSerializationContext(this.emitDefaults); +} diff --git a/protobuf/lib/src/protobuf/proto3_json.dart b/protobuf/lib/src/protobuf/proto3_json.dart index 6d35db920..5fc1ab24d 100644 --- a/protobuf/lib/src/protobuf/proto3_json.dart +++ b/protobuf/lib/src/protobuf/proto3_json.dart @@ -4,7 +4,10 @@ part of protobuf; -Object? _writeToProto3Json(_FieldSet fs, TypeRegistry typeRegistry) { +Object? _writeToProto3Json( + _FieldSet fs, TypeRegistry typeRegistry, bool emitDefaults) { + var context = JsonSerializationContext(emitDefaults); + String? convertToMapKey(dynamic key, int keyType) { var baseType = PbFieldType._baseType(keyType); @@ -36,8 +39,8 @@ Object? _writeToProto3Json(_FieldSet fs, TypeRegistry typeRegistry) { if (fieldValue == null) return null; if (_isGroupOrMessage(fieldType!)) { - return _writeToProto3Json( - (fieldValue as GeneratedMessage)._fieldSet, typeRegistry); + return _writeToProto3Json((fieldValue as GeneratedMessage)._fieldSet, + typeRegistry, context.emitDefaults); } else if (_isEnum(fieldType)) { return (fieldValue as ProtobufEnum).name; } else { @@ -81,6 +84,14 @@ Object? _writeToProto3Json(_FieldSet fs, TypeRegistry typeRegistry) { } } + bool isNullOrEmptyList(dynamic value) { + return value == null || (value is List && value.isEmpty); + } + + bool isNullOrEmptyMap(dynamic value) { + return value == null || (value is Map && value.isEmpty); + } + final meta = fs._meta; if (meta.toProto3Json != null) { return meta.toProto3Json!(fs._message!, typeRegistry); @@ -89,11 +100,32 @@ Object? _writeToProto3Json(_FieldSet fs, TypeRegistry typeRegistry) { var result = {}; for (var fieldInfo in fs._infosSortedByTag) { var value = fs._values[fieldInfo.index!]; - if (value == null || (value is List && value.isEmpty)) { + var overrideForEmitsDefaults = false; + dynamic overrideForEmitsDefaultsValue; + if (context.emitDefaults) { + if (fieldInfo.isRepeated && isNullOrEmptyList(value)) { + overrideForEmitsDefaults = true; + overrideForEmitsDefaultsValue = []; + } else if (fieldInfo.isMapField && isNullOrEmptyMap(value)) { + overrideForEmitsDefaults = true; + overrideForEmitsDefaultsValue = {}; + } else if (_isBytes(fieldInfo.type) && isNullOrEmptyList(value)) { + overrideForEmitsDefaults = true; + overrideForEmitsDefaultsValue = null; + } else if (_isGroupOrMessage(fieldInfo.type) && value == null) { + overrideForEmitsDefaults = true; + overrideForEmitsDefaultsValue = null; + } else { + value ??= fieldInfo.makeDefault!(); + } + } + if (isNullOrEmptyList(value) && !overrideForEmitsDefaults) { continue; // It's missing, repeated, or an empty byte array. } dynamic jsonValue; - if (fieldInfo.isMapField) { + if (overrideForEmitsDefaults) { + jsonValue = overrideForEmitsDefaultsValue; + } else if (fieldInfo.isMapField) { jsonValue = (value as PbMap).map((key, entryValue) { var mapEntryInfo = fieldInfo as MapFieldInfo; return MapEntry(convertToMapKey(key, mapEntryInfo.keyFieldType!), diff --git a/protobuf/test/json_test.dart b/protobuf/test/json_test.dart index 6291ad8c7..382777ffb 100644 --- a/protobuf/test/json_test.dart +++ b/protobuf/test/json_test.dart @@ -115,6 +115,64 @@ void main() { final decoded = T()..mergeFromJsonMap(encoded); expect(decoded.int64, value); }); + + test('testToProto3Json', () { + var json = jsonEncode(example.toProto3Json()); + checkProto3JsonMap(jsonDecode(json), 3); + }); + + test('testToProto3JsonEmitDefaults', () { + var json = jsonEncode(example.toProto3Json(emitDefaults: true)); + checkProto3JsonMap(jsonDecode(json), 6); + expect(json.contains('"child":null'), isTrue); + }); + + test('testToProto3JsonEmitDefaultsNoValues', () { + final exampleAllDefaults = T(); + var json = jsonEncode(exampleAllDefaults.toProto3Json(emitDefaults: true)); + Map m = jsonDecode(json); + expect(m.length, 6); + }); + + test('testToProto3JsonEmitDefaultsWithChild', () { + var child = example; + + var parent = T() + ..val = 123 + ..str = 'hello' + ..int32s.addAll([1, 2, 3]) + ..child = example; + var parentJson = jsonEncode(parent.toProto3Json(emitDefaults: true)); + var childJson = jsonEncode(child.toProto3Json(emitDefaults: true)); + checkProto3JsonMap(jsonDecode(parentJson), 6); + expect(parentJson.contains(childJson), isTrue); + }); + + test('testToProto3JsonEmitDefaultsWithNullList', () { + var exampleEmptyList = T() + ..val = example.val + ..str = example.str; + + var json = jsonEncode(exampleEmptyList.toProto3Json(emitDefaults: true)); + expect(json.contains('"int32s":[]'), isTrue); + }); + + test('testToProto3JsonEmitDefaultsWithEmptyList', () { + var exampleEmptyList = T() + ..val = example.val + ..str = example.str + ..int32s.addAll([]); + + var json = jsonEncode(exampleEmptyList.toProto3Json(emitDefaults: true)); + expect(json.contains('"int32s":[]'), isTrue); + }); +} + +void checkProto3JsonMap(Map m, int expectedLength) { + expect(m.length, expectedLength); + expect(m['val'], 123); + expect(m['str'], 'hello'); + expect(m['int32s'], [1, 2, 3]); } void checkJsonMap(Map m) { From 5c50a9512f894839a801bb3d3a12d5c0766d808a Mon Sep 17 00:00:00 2001 From: Jeff Martin Date: Fri, 1 Apr 2022 10:44:27 -0600 Subject: [PATCH 02/12] Better function names and removing temp variables --- protobuf/lib/src/protobuf/proto3_json.dart | 34 ++++++++-------------- 1 file changed, 12 insertions(+), 22 deletions(-) diff --git a/protobuf/lib/src/protobuf/proto3_json.dart b/protobuf/lib/src/protobuf/proto3_json.dart index 5fc1ab24d..b9b9ab844 100644 --- a/protobuf/lib/src/protobuf/proto3_json.dart +++ b/protobuf/lib/src/protobuf/proto3_json.dart @@ -84,11 +84,11 @@ Object? _writeToProto3Json( } } - bool isNullOrEmptyList(dynamic value) { + bool isDefaultListField(dynamic value) { return value == null || (value is List && value.isEmpty); } - bool isNullOrEmptyMap(dynamic value) { + bool isDefaultMapField(dynamic value) { return value == null || (value is Map && value.isEmpty); } @@ -100,31 +100,21 @@ Object? _writeToProto3Json( var result = {}; for (var fieldInfo in fs._infosSortedByTag) { var value = fs._values[fieldInfo.index!]; - var overrideForEmitsDefaults = false; - dynamic overrideForEmitsDefaultsValue; + dynamic jsonValue; if (context.emitDefaults) { - if (fieldInfo.isRepeated && isNullOrEmptyList(value)) { - overrideForEmitsDefaults = true; - overrideForEmitsDefaultsValue = []; - } else if (fieldInfo.isMapField && isNullOrEmptyMap(value)) { - overrideForEmitsDefaults = true; - overrideForEmitsDefaultsValue = {}; - } else if (_isBytes(fieldInfo.type) && isNullOrEmptyList(value)) { - overrideForEmitsDefaults = true; - overrideForEmitsDefaultsValue = null; + if (fieldInfo.isRepeated && isDefaultListField(value)) { + jsonValue = []; + } else if (fieldInfo.isMapField && isDefaultMapField(value)) { + jsonValue = {}; + } else if (_isBytes(fieldInfo.type) && isDefaultListField(value)) { + jsonValue = null; } else if (_isGroupOrMessage(fieldInfo.type) && value == null) { - overrideForEmitsDefaults = true; - overrideForEmitsDefaultsValue = null; + jsonValue = null; } else { - value ??= fieldInfo.makeDefault!(); + jsonValue = valueToProto3Json(value, fieldInfo.type); } - } - if (isNullOrEmptyList(value) && !overrideForEmitsDefaults) { + } else if (isDefaultListField(value)) { continue; // It's missing, repeated, or an empty byte array. - } - dynamic jsonValue; - if (overrideForEmitsDefaults) { - jsonValue = overrideForEmitsDefaultsValue; } else if (fieldInfo.isMapField) { jsonValue = (value as PbMap).map((key, entryValue) { var mapEntryInfo = fieldInfo as MapFieldInfo; From db5637f61d1c69e16bf0397a0b0f2c55ee1e591a Mon Sep 17 00:00:00 2001 From: Jeff Martin Date: Fri, 1 Apr 2022 10:59:15 -0600 Subject: [PATCH 03/12] Leverating existing readonlyDefault function for emitDefaults --- protobuf/lib/src/protobuf/proto3_json.dart | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/protobuf/lib/src/protobuf/proto3_json.dart b/protobuf/lib/src/protobuf/proto3_json.dart index b9b9ab844..1308ad875 100644 --- a/protobuf/lib/src/protobuf/proto3_json.dart +++ b/protobuf/lib/src/protobuf/proto3_json.dart @@ -102,12 +102,10 @@ Object? _writeToProto3Json( var value = fs._values[fieldInfo.index!]; dynamic jsonValue; if (context.emitDefaults) { - if (fieldInfo.isRepeated && isDefaultListField(value)) { - jsonValue = []; - } else if (fieldInfo.isMapField && isDefaultMapField(value)) { - jsonValue = {}; - } else if (_isBytes(fieldInfo.type) && isDefaultListField(value)) { - jsonValue = null; + if ((fieldInfo.isRepeated && isDefaultListField(value)) || + (fieldInfo.isMapField && isDefaultMapField(value)) || + (_isBytes(fieldInfo.type) && isDefaultListField(value))) { + jsonValue = fieldInfo.readonlyDefault; } else if (_isGroupOrMessage(fieldInfo.type) && value == null) { jsonValue = null; } else { From 7121dc74ff35ab94e405b2c2551b72d73388cea4 Mon Sep 17 00:00:00 2001 From: Jeff Martin Date: Fri, 1 Apr 2022 15:04:36 -0600 Subject: [PATCH 04/12] Ensuring bytes are handled for emitDefaults --- protobuf/lib/src/protobuf/proto3_json.dart | 5 ++--- protobuf/test/json_test.dart | 13 ++++++++++++- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/protobuf/lib/src/protobuf/proto3_json.dart b/protobuf/lib/src/protobuf/proto3_json.dart index 1308ad875..617fbad01 100644 --- a/protobuf/lib/src/protobuf/proto3_json.dart +++ b/protobuf/lib/src/protobuf/proto3_json.dart @@ -103,10 +103,9 @@ Object? _writeToProto3Json( dynamic jsonValue; if (context.emitDefaults) { if ((fieldInfo.isRepeated && isDefaultListField(value)) || - (fieldInfo.isMapField && isDefaultMapField(value)) || - (_isBytes(fieldInfo.type) && isDefaultListField(value))) { + (fieldInfo.isMapField && isDefaultMapField(value))) { jsonValue = fieldInfo.readonlyDefault; - } else if (_isGroupOrMessage(fieldInfo.type) && value == null) { + } else if (_isBytes(fieldInfo.type) && isDefaultListField(value)) { jsonValue = null; } else { jsonValue = valueToProto3Json(value, fieldInfo.type); diff --git a/protobuf/test/json_test.dart b/protobuf/test/json_test.dart index 382777ffb..ec729450f 100644 --- a/protobuf/test/json_test.dart +++ b/protobuf/test/json_test.dart @@ -141,13 +141,24 @@ void main() { ..val = 123 ..str = 'hello' ..int32s.addAll([1, 2, 3]) - ..child = example; + ..child = child; var parentJson = jsonEncode(parent.toProto3Json(emitDefaults: true)); var childJson = jsonEncode(child.toProto3Json(emitDefaults: true)); checkProto3JsonMap(jsonDecode(parentJson), 6); expect(parentJson.contains(childJson), isTrue); }); + test('testToProto3JsonEmitDefaultsWithDefaultChild', () { + var parent = T() + ..val = 123 + ..str = 'hello' + ..int32s.addAll([1, 2, 3]) + ..child = T(); + var parentJson = jsonEncode(parent.toProto3Json(emitDefaults: true)); + checkProto3JsonMap(jsonDecode(parentJson), 6); + expect(parentJson.contains('"child":null'), isTrue); + }); + test('testToProto3JsonEmitDefaultsWithNullList', () { var exampleEmptyList = T() ..val = example.val From d714dbc3a8ce8358f60c7dab29e7182462a05128 Mon Sep 17 00:00:00 2001 From: Jeff Martin Date: Fri, 1 Apr 2022 22:46:14 -0600 Subject: [PATCH 05/12] Additional unit tests for emitDefaults JSON serialization --- protobuf/lib/src/protobuf/proto3_json.dart | 19 +-- protobuf/test/json_test.dart | 147 +++++++++++++++------ protobuf/test/map_mixin_test.dart | 15 ++- protobuf/test/mock_util.dart | 20 ++- 4 files changed, 147 insertions(+), 54 deletions(-) diff --git a/protobuf/lib/src/protobuf/proto3_json.dart b/protobuf/lib/src/protobuf/proto3_json.dart index 617fbad01..81f4b7291 100644 --- a/protobuf/lib/src/protobuf/proto3_json.dart +++ b/protobuf/lib/src/protobuf/proto3_json.dart @@ -101,15 +101,16 @@ Object? _writeToProto3Json( for (var fieldInfo in fs._infosSortedByTag) { var value = fs._values[fieldInfo.index!]; dynamic jsonValue; - if (context.emitDefaults) { - if ((fieldInfo.isRepeated && isDefaultListField(value)) || - (fieldInfo.isMapField && isDefaultMapField(value))) { - jsonValue = fieldInfo.readonlyDefault; - } else if (_isBytes(fieldInfo.type) && isDefaultListField(value)) { - jsonValue = null; - } else { - jsonValue = valueToProto3Json(value, fieldInfo.type); - } + if (context.emitDefaults && + ((fieldInfo.isRepeated && isDefaultListField(value)) || + (fieldInfo.isMapField && isDefaultMapField(value)))) { + jsonValue = fieldInfo.readonlyDefault; + } else if (context.emitDefaults && + ((_isBytes(fieldInfo.type) && isDefaultListField(value)) || + (_isGroupOrMessage(fieldInfo.type) && value == null))) { + jsonValue = null; + } else if (context.emitDefaults && value == null) { + jsonValue = valueToProto3Json(fieldInfo.readonlyDefault, fieldInfo.type); } else if (isDefaultListField(value)) { continue; // It's missing, repeated, or an empty byte array. } else if (fieldInfo.isMapField) { diff --git a/protobuf/test/json_test.dart b/protobuf/test/json_test.dart index ec729450f..42ec49e9f 100644 --- a/protobuf/test/json_test.dart +++ b/protobuf/test/json_test.dart @@ -6,6 +6,7 @@ library json_test; import 'dart:convert'; import 'package:fixnum/fixnum.dart' show Int64; +import 'package:protobuf/protobuf.dart'; import 'package:test/test.dart'; import 'mock_util.dart' show T, mockEnumValues; @@ -16,6 +17,32 @@ void main() { ..str = 'hello' ..int32s.addAll([1, 2, 3]); + final double doubleZeroVal = 0; + final exampleAllDefaults = T() + ..val = 0 + ..str = '' + ..child = T() + ..int64 = Int64(0) + ..enm = ProtobufEnum(1, 'a') + ..dbl = doubleZeroVal + ..bl = false + ..byts = [] + ..int32s.addAll([]); + exampleAllDefaults.mp.addAll({}); + + final double doubleSetVal = 1.34; + final exampleAllSet = T() + ..val = 32 + ..str = 'the-string' + ..child = example + ..int64 = Int64(78) + ..enm = ProtobufEnum(1, 'b') + ..dbl = doubleSetVal + ..bl = true + ..byts = [46, 28] + ..int32s.addAll([3, 4, 6]); + exampleAllSet.mp.addAll({'k1': 'v2'}); + test('testProto3JsonEnum', () { // No enum value specified. expect(example.hasEnm, isFalse); @@ -121,64 +148,100 @@ void main() { checkProto3JsonMap(jsonDecode(json), 3); }); - test('testToProto3JsonEmitDefaults', () { - var json = jsonEncode(example.toProto3Json(emitDefaults: true)); - checkProto3JsonMap(jsonDecode(json), 6); - expect(json.contains('"child":null'), isTrue); + test('testToProto3JsonNoFieldsSet', () { + final json = jsonEncode(T().toProto3Json()); + expect(json.contains('{}'), isTrue); + final Map m = jsonDecode(json); + expect(m.isEmpty, isTrue); }); - test('testToProto3JsonEmitDefaultsNoValues', () { - final exampleAllDefaults = T(); - var json = jsonEncode(exampleAllDefaults.toProto3Json(emitDefaults: true)); - Map m = jsonDecode(json); - expect(m.length, 6); + test('testToProto3JsonFieldsSetToDefaults', () { + final json = jsonEncode(exampleAllDefaults.toProto3Json()); + expect(json.contains('"child":{}'), isTrue); + final Map m = jsonDecode(json); + expect(m.isEmpty, isFalse); }); - test('testToProto3JsonEmitDefaultsWithChild', () { - var child = example; + test('testToProto3JsonFieldsSetToValues', () { + // verify the expected child is present + final json = jsonEncode(exampleAllSet.toProto3Json()); + final expectedChild = '{"val":123,"str":"hello","int32s":[1,2,3]}'; + expect(json.contains('"child":$expectedChild'), isTrue); + final jsonNoChild = json.replaceAll(expectedChild, ''); + + // verify the remaining parent object is accurate + checkExampleAllParentSetValues(jsonNoChild); + }); - var parent = T() - ..val = 123 - ..str = 'hello' - ..int32s.addAll([1, 2, 3]) - ..child = child; - var parentJson = jsonEncode(parent.toProto3Json(emitDefaults: true)); - var childJson = jsonEncode(child.toProto3Json(emitDefaults: true)); - checkProto3JsonMap(jsonDecode(parentJson), 6); - expect(parentJson.contains(childJson), isTrue); + test('testToProto3JsonEmitDefaults', () { + final json = jsonEncode(example.toProto3Json(emitDefaults: true)); + checkProto3JsonMap(jsonDecode(json), 10); }); - test('testToProto3JsonEmitDefaultsWithDefaultChild', () { - var parent = T() - ..val = 123 - ..str = 'hello' - ..int32s.addAll([1, 2, 3]) - ..child = T(); - var parentJson = jsonEncode(parent.toProto3Json(emitDefaults: true)); - checkProto3JsonMap(jsonDecode(parentJson), 6); - expect(parentJson.contains('"child":null'), isTrue); + test('testToProto3JsonEmitDefaultsNoFieldsSet', () { + final json = jsonEncode(T().toProto3Json(emitDefaults: true)); + expect(json.contains('"val":42,'), isTrue); + expect(json.contains('"str":"",'), isTrue); + expect(json.contains('"child":null,'), isTrue); + expect(json.contains('"int32s":[],'), isTrue); + expect(json.contains('"int64":"0",'), isTrue); + expect(json.contains('"enm":"a",'), isTrue); + expect(json.contains('"dbl":0.0,'), isTrue); + expect(json.contains('"bl":false,'), isTrue); + expect(json.contains('"byts":null'), isTrue); + expect(json.contains('"mp":{}'), isTrue); }); - test('testToProto3JsonEmitDefaultsWithNullList', () { - var exampleEmptyList = T() - ..val = example.val - ..str = example.str; + test('testToProto3JsonEmitDefaultsFieldsSetToDefaults', () { + // verify the default child is present + final json = + jsonEncode(exampleAllDefaults.toProto3Json(emitDefaults: true)); + final defaultChild = + '{"val":42,"str":"","child":null,"int32s":[],"int64":"0","enm":"a","dbl":0.0,"bl":false,"byts":null,"mp":{}}'; + expect(json.contains('"child":$defaultChild'), isTrue); + final jsonNoChild = json.replaceAll(defaultChild, ''); - var json = jsonEncode(exampleEmptyList.toProto3Json(emitDefaults: true)); - expect(json.contains('"int32s":[]'), isTrue); + // verify the remaining parent object is accurate + checkExampleAllParentSetDefaults(jsonNoChild); }); - test('testToProto3JsonEmitDefaultsWithEmptyList', () { - var exampleEmptyList = T() - ..val = example.val - ..str = example.str - ..int32s.addAll([]); + test('testToProto3JsonEmitDefaultsFieldsSetToValues', () { + // verify the expected child is present + final json = jsonEncode(exampleAllSet.toProto3Json(emitDefaults: true)); + final expectedChild = + '{"val":123,"str":"hello","child":null,"int32s":[1,2,3],"int64":"0","enm":"a","dbl":0.0,"bl":false,"byts":null,"mp":{}}'; + expect(json.contains('"child":$expectedChild'), isTrue); + final jsonNoChild = json.replaceAll(expectedChild, ''); - var json = jsonEncode(exampleEmptyList.toProto3Json(emitDefaults: true)); - expect(json.contains('"int32s":[]'), isTrue); + // verify the remaining parent object is accurate + checkExampleAllParentSetValues(jsonNoChild); }); } +void checkExampleAllParentSetDefaults(String json) { + expect(json.contains('"val":0,'), isTrue); + expect(json.contains('"str":"",'), isTrue); + expect(json.contains('"int32s":[],'), isTrue); + expect(json.contains('"int64":"0",'), isTrue); + expect(json.contains('"enm":"a",'), isTrue); + expect(json.contains('"dbl":0.0,'), isTrue); + expect(json.contains('"bl":false,'), isTrue); + expect(json.contains('"byts":null'), isTrue); + expect(json.contains('"mp":{}'), isTrue); +} + +void checkExampleAllParentSetValues(String json) { + expect(json.contains('"val":32,'), isTrue); + expect(json.contains('"str":"the-string",'), isTrue); + expect(json.contains('"int32s":[3,4,6],'), isTrue); + expect(json.contains('"int64":"78",'), isTrue); + expect(json.contains('"enm":"b",'), isTrue); + expect(json.contains('"dbl":1.34,'), isTrue); + expect(json.contains('"bl":true,'), isTrue); + expect(json.contains('"byts":"Lhw="'), isTrue); + expect(json.contains('"mp":{"k1":"v2"}'), isTrue); +} + void checkProto3JsonMap(Map m, int expectedLength) { expect(m.length, expectedLength); expect(m['val'], 123); diff --git a/protobuf/test/map_mixin_test.dart b/protobuf/test/map_mixin_test.dart index 2e40c5bcb..22a027d53 100644 --- a/protobuf/test/map_mixin_test.dart +++ b/protobuf/test/map_mixin_test.dart @@ -33,7 +33,18 @@ void main() { expect(r.isEmpty, false); expect(r.isNotEmpty, true); - expect(r.keys, ['val', 'str', 'child', 'int32s', 'int64', 'enm']); + expect(r.keys, [ + 'val', + 'str', + 'child', + 'int32s', + 'int64', + 'enm', + 'dbl', + 'bl', + 'byts', + 'mp' + ]); expect(r['val'], 42); expect(r['str'], ''); @@ -42,7 +53,7 @@ void main() { expect(r['int32s'], []); var v = r.values; - expect(v.length, 6); + expect(v.length, 10); expect(v.first, 42); expect(v.toList()[1], ''); expect(v.toList()[3].toString(), '[]'); diff --git a/protobuf/test/mock_util.dart b/protobuf/test/mock_util.dart index f586f2d0d..236b26991 100644 --- a/protobuf/test/mock_util.dart +++ b/protobuf/test/mock_util.dart @@ -24,7 +24,12 @@ BuilderInfo mockInfo(String className, CreateBuilderFunc create) { ..e(7, 'enm', PbFieldType.OE, defaultOrMaker: mockEnumValues.first, valueOf: (i) => mockEnumValues.firstWhereOrNull((e) => e.value == i), - enumValues: mockEnumValues); + enumValues: mockEnumValues) + ..a(8, 'dbl', PbFieldType.OD) + ..aOB(9, 'bl') + ..a(10, 'byts', PbFieldType.OY) + ..m(11, 'mp', + keyFieldType: PbFieldType.OS, valueFieldType: PbFieldType.OS); } /// A minimal protobuf implementation for testing. @@ -43,12 +48,25 @@ abstract class MockMessage extends GeneratedMessage { set child(x) => setField(3, x); List get int32s => $_getList(3); + //set int32s(x) => setField(4, x); Int64 get int64 => $_get(4, Int64(0)); set int64(x) => setField(5, x); ProtobufEnum get enm => $_getN(5); bool get hasEnm => $_has(5); + set enm(x) => setField(7, x); + + double get dbl => $_get(6, 0.0); + set dbl(x) => setField(8, x); + + bool get bl => $_get(7, false); + set bl(x) => setField(9, x); + + List get byts => $_get(8, []); + set byts(x) => setField(10, x); + + Map get mp => $_getMap(9); @override GeneratedMessage clone() { From c253b823ee94c2effe529583e1bfaef33687c5c5 Mon Sep 17 00:00:00 2001 From: Jeff Martin Date: Fri, 1 Apr 2022 22:59:17 -0600 Subject: [PATCH 06/12] Removing sray comment for emitDefaults --- protobuf/test/mock_util.dart | 1 - 1 file changed, 1 deletion(-) diff --git a/protobuf/test/mock_util.dart b/protobuf/test/mock_util.dart index 236b26991..b7cd92ca0 100644 --- a/protobuf/test/mock_util.dart +++ b/protobuf/test/mock_util.dart @@ -48,7 +48,6 @@ abstract class MockMessage extends GeneratedMessage { set child(x) => setField(3, x); List get int32s => $_getList(3); - //set int32s(x) => setField(4, x); Int64 get int64 => $_get(4, Int64(0)); set int64(x) => setField(5, x); From d3bd5aec8ac3208daf8b3df1798ec5c70d4b7f6c Mon Sep 17 00:00:00 2001 From: Jeff Martin Date: Tue, 5 Apr 2022 21:03:41 -0600 Subject: [PATCH 07/12] First iteration of new method strategy for JSON serialization --- protobuf/lib/src/protobuf/proto3_json.dart | 116 ++++++++++++++++----- protobuf/test/json_test.dart | 6 +- 2 files changed, 93 insertions(+), 29 deletions(-) diff --git a/protobuf/lib/src/protobuf/proto3_json.dart b/protobuf/lib/src/protobuf/proto3_json.dart index 81f4b7291..cd6c7ff03 100644 --- a/protobuf/lib/src/protobuf/proto3_json.dart +++ b/protobuf/lib/src/protobuf/proto3_json.dart @@ -99,34 +99,98 @@ Object? _writeToProto3Json( var result = {}; for (var fieldInfo in fs._infosSortedByTag) { - var value = fs._values[fieldInfo.index!]; - dynamic jsonValue; - if (context.emitDefaults && - ((fieldInfo.isRepeated && isDefaultListField(value)) || - (fieldInfo.isMapField && isDefaultMapField(value)))) { - jsonValue = fieldInfo.readonlyDefault; - } else if (context.emitDefaults && - ((_isBytes(fieldInfo.type) && isDefaultListField(value)) || - (_isGroupOrMessage(fieldInfo.type) && value == null))) { - jsonValue = null; - } else if (context.emitDefaults && value == null) { - jsonValue = valueToProto3Json(fieldInfo.readonlyDefault, fieldInfo.type); - } else if (isDefaultListField(value)) { - continue; // It's missing, repeated, or an empty byte array. + // if the value for this field is null, this function will return the + // default value, which simplifies the need to handle null for most cases + // below + var value = fs._getField(fieldInfo.tagNumber); + + bool skipField = true; + Object? jsonValue; + if (fieldInfo.isRepeated) { + if (context.emitDefaults || (value as List).isNotEmpty) { + skipField = false; + jsonValue = (value as PbListBase) + .map((element) => valueToProto3Json(element, fieldInfo.type)) + .toList(); + } } else if (fieldInfo.isMapField) { - jsonValue = (value as PbMap).map((key, entryValue) { - var mapEntryInfo = fieldInfo as MapFieldInfo; - return MapEntry(convertToMapKey(key, mapEntryInfo.keyFieldType!), - valueToProto3Json(entryValue, mapEntryInfo.valueFieldType)); - }); - } else if (fieldInfo.isRepeated) { - jsonValue = (value as PbListBase) - .map((element) => valueToProto3Json(element, fieldInfo.type)) - .toList(); - } else { - jsonValue = valueToProto3Json(value, fieldInfo.type); + if (context.emitDefaults || (value as Map).isNotEmpty) { + skipField = false; + jsonValue = (value as PbMap).map((key, entryValue) { + var mapEntryInfo = fieldInfo as MapFieldInfo; + return MapEntry(convertToMapKey(key, mapEntryInfo.keyFieldType!), + valueToProto3Json(entryValue, mapEntryInfo.valueFieldType)); + }); + } + } else if (_isBytes(fieldInfo.type)) { + if (context.emitDefaults || (value as List).isNotEmpty) { + skipField = false; + if ((value as List).isEmpty) { + jsonValue = null; + } else { + jsonValue = valueToProto3Json(value, fieldInfo.type); + } + } + } else if (fieldInfo.isEnum) { + // For enums, the default value is the first value listed in the enum's type definition + final defaultEnum = fieldInfo.enumValues!.first; + if (context.emitDefaults || + (value as ProtobufEnum).name != defaultEnum.name) { + skipField = false; + jsonValue = valueToProto3Json(value, fieldInfo.type); + } + } else if (fieldInfo.isGroupOrMessage) { + // TODO see if there is a better way to check for the default message + if (context.emitDefaults || fs._values[fieldInfo.index!] != null) { + skipField = false; + if (fs._values[fieldInfo.index!] == null) { + jsonValue = null; + } else { + jsonValue = valueToProto3Json(value, fieldInfo.type); + } + } + } else if (PbFieldType._baseType(fieldInfo.type) == + PbFieldType._STRING_BIT) { + if (context.emitDefaults || (value as String).isNotEmpty) { + skipField = false; + jsonValue = valueToProto3Json(value, fieldInfo.type); + } + } else if (PbFieldType._baseType(fieldInfo.type) == + PbFieldType._DOUBLE_BIT) { + if (context.emitDefaults || (value as double) != 0.0) { + skipField = false; + jsonValue = valueToProto3Json(value, fieldInfo.type); + } + } else if (PbFieldType._baseType(fieldInfo.type) == PbFieldType._BOOL_BIT) { + if (context.emitDefaults || value != false) { + skipField = false; + jsonValue = valueToProto3Json(value, fieldInfo.type); + } + } else if (PbFieldType._baseType(fieldInfo.type) == + PbFieldType._INT32_BIT) { + if (context.emitDefaults || (value as int) != 0) { + skipField = false; + jsonValue = valueToProto3Json(value, fieldInfo.type); + } + } else if (PbFieldType._baseType(fieldInfo.type) == + PbFieldType._INT64_BIT) { + if (value is Int64) { + if (context.emitDefaults || !value.isZero) { + skipField = false; + jsonValue = valueToProto3Json(value, fieldInfo.type); + } + } else if (value is int) { + if (context.emitDefaults || value != 0) { + skipField = false; + jsonValue = valueToProto3Json(value, fieldInfo.type); + } + } + // TODO handling of additional types + } + + if (!skipField) { + result[fieldInfo.name] = jsonValue; } - result[fieldInfo.name] = jsonValue; } // Extensions and unknown fields are not encoded by proto3 JSON. return result; diff --git a/protobuf/test/json_test.dart b/protobuf/test/json_test.dart index 42ec49e9f..a19e0c3dc 100644 --- a/protobuf/test/json_test.dart +++ b/protobuf/test/json_test.dart @@ -150,14 +150,14 @@ void main() { test('testToProto3JsonNoFieldsSet', () { final json = jsonEncode(T().toProto3Json()); - expect(json.contains('{}'), isTrue); + expect(json.contains('{"val":42}'), isTrue); final Map m = jsonDecode(json); - expect(m.isEmpty, isTrue); + expect(m.length, 1); }); test('testToProto3JsonFieldsSetToDefaults', () { final json = jsonEncode(exampleAllDefaults.toProto3Json()); - expect(json.contains('"child":{}'), isTrue); + expect(json.contains('"child":{"val":42}'), isTrue); final Map m = jsonDecode(json); expect(m.isEmpty, isFalse); }); From 86af2f2d418cd6d9fbecc6839bf143a46934872d Mon Sep 17 00:00:00 2001 From: Jeff Martin Date: Tue, 5 Apr 2022 21:18:58 -0600 Subject: [PATCH 08/12] Ensuring remaining cases are covered --- protobuf/lib/src/protobuf/proto3_json.dart | 40 +++++++++------------- 1 file changed, 16 insertions(+), 24 deletions(-) diff --git a/protobuf/lib/src/protobuf/proto3_json.dart b/protobuf/lib/src/protobuf/proto3_json.dart index cd6c7ff03..a052d49a3 100644 --- a/protobuf/lib/src/protobuf/proto3_json.dart +++ b/protobuf/lib/src/protobuf/proto3_json.dart @@ -140,10 +140,10 @@ Object? _writeToProto3Json( jsonValue = valueToProto3Json(value, fieldInfo.type); } } else if (fieldInfo.isGroupOrMessage) { - // TODO see if there is a better way to check for the default message - if (context.emitDefaults || fs._values[fieldInfo.index!] != null) { + final originalValue = fs._values[fieldInfo.index!]; + if (context.emitDefaults || originalValue != null) { skipField = false; - if (fs._values[fieldInfo.index!] == null) { + if (originalValue == null) { jsonValue = null; } else { jsonValue = valueToProto3Json(value, fieldInfo.type); @@ -155,37 +155,29 @@ Object? _writeToProto3Json( skipField = false; jsonValue = valueToProto3Json(value, fieldInfo.type); } - } else if (PbFieldType._baseType(fieldInfo.type) == - PbFieldType._DOUBLE_BIT) { - if (context.emitDefaults || (value as double) != 0.0) { + } else if (PbFieldType._baseType(fieldInfo.type) == PbFieldType._BOOL_BIT) { + if (context.emitDefaults || value != false) { skipField = false; jsonValue = valueToProto3Json(value, fieldInfo.type); } - } else if (PbFieldType._baseType(fieldInfo.type) == PbFieldType._BOOL_BIT) { - if (context.emitDefaults || value != false) { + } else if (value is Int64) { + if (context.emitDefaults || !value.isZero) { skipField = false; jsonValue = valueToProto3Json(value, fieldInfo.type); } - } else if (PbFieldType._baseType(fieldInfo.type) == - PbFieldType._INT32_BIT) { - if (context.emitDefaults || (value as int) != 0) { + } else if (value is int) { + if (context.emitDefaults || value != 0) { skipField = false; jsonValue = valueToProto3Json(value, fieldInfo.type); } - } else if (PbFieldType._baseType(fieldInfo.type) == - PbFieldType._INT64_BIT) { - if (value is Int64) { - if (context.emitDefaults || !value.isZero) { - skipField = false; - jsonValue = valueToProto3Json(value, fieldInfo.type); - } - } else if (value is int) { - if (context.emitDefaults || value != 0) { - skipField = false; - jsonValue = valueToProto3Json(value, fieldInfo.type); - } + } else if (value is double) { + if (context.emitDefaults || value != 0.0) { + skipField = false; + jsonValue = valueToProto3Json(value, fieldInfo.type); } - // TODO handling of additional types + } else { + skipField = false; + jsonValue = valueToProto3Json(value, fieldInfo.type); } if (!skipField) { From b622504ddf23680fc292ebaeb01240f2e2cd7b43 Mon Sep 17 00:00:00 2001 From: Jeff Martin Date: Wed, 6 Apr 2022 23:53:04 -0600 Subject: [PATCH 09/12] Fixing failing unit tests in proto3_json_test.dart --- protoc_plugin/test/proto3_json_test.dart | 85 ++++++++++++++++++++---- 1 file changed, 71 insertions(+), 14 deletions(-) diff --git a/protoc_plugin/test/proto3_json_test.dart b/protoc_plugin/test/proto3_json_test.dart index 02fb71ae2..6c2fb2d0a 100644 --- a/protoc_plugin/test/proto3_json_test.dart +++ b/protoc_plugin/test/proto3_json_test.dart @@ -107,14 +107,26 @@ final testAllTypesJson = { 'defaultCord': '425' }; +// these fields have default values set for them within {@code setAllFields} and +// thus will not be serialized to JSON adhering to the proto3 specification +final keysWithDefaultValues = [ + 'defaultBool', + 'defaultNestedEnum', + 'defaultForeignEnum', + 'defaultImportEnum' +]; + +final testAllTypesJsonNoDefaults = Map.from(testAllTypesJson) + ..removeWhere((k, v) => (keysWithDefaultValues.contains(k))); + void main() { group('encode', () { test('testOutput', () { - expect(getAllSet().toProto3Json(), testAllTypesJson); + expect(getAllSet().toProto3Json(), testAllTypesJsonNoDefaults); }); test('testFrozenOutput', () { - expect(getAllSet().freeze().toProto3Json(), testAllTypesJson); + expect(getAllSet().freeze().toProto3Json(), testAllTypesJsonNoDefaults); }); test('testUnsignedOutput', () { @@ -125,26 +137,51 @@ void main() { expect(message.toProto3Json(), { 'optionalUint64': '17293822573397606400', - 'optionalFixed64': '-1152921500311945215' + 'optionalFixed64': '-1152921500311945215', + 'defaultInt32': 41, + 'defaultInt64': '42', + 'defaultUint32': 43, + 'defaultUint64': '44', + 'defaultSint32': -45, + 'defaultSint64': '46', + 'defaultFixed32': 47, + 'defaultFixed64': '48', + 'defaultSfixed32': 49, + 'defaultSfixed64': '-50', + 'defaultFloat': 51.5, + 'defaultDouble': 52000.0, + 'defaultBool': true, + 'defaultString': 'hello', + 'defaultBytes': 'd29ybGQ=', + 'defaultNestedEnum': 'BAR', + 'defaultForeignEnum': 'FOREIGN_BAR', + 'defaultImportEnum': 'IMPORT_BAR', + 'defaultStringPiece': 'abc', + 'defaultCord': '123' }); }); test('doubles', () { - void testValue(double value, Object expected) { + void testValue(double value, bool expectedInJson, Object expected) { var message = TestAllTypes() ..defaultFloat = value ..defaultDouble = value; - expect( - (message.toProto3Json() as Map)['defaultDouble'], equals(expected)); + + if (expectedInJson) { + expect((message.toProto3Json() as Map)['defaultDouble'], + equals(expected)); + } else { + expect((message.toProto3Json() as Map)['defaultDouble'], isNull); + } } - testValue(-0.0, -0.0); - testValue(0.0, 0); - testValue(1.0, 1); - testValue(-1.0, -1); - testValue(double.nan, 'NaN'); - testValue(double.infinity, 'Infinity'); - testValue(double.negativeInfinity, '-Infinity'); + testValue(-0.0, false, 0); + testValue(0.0, false, 0); + testValue(1.0, true, 1); + testValue(-1.0, true, -1); + testValue(double.nan, true, 'NaN'); + testValue(double.infinity, true, 'Infinity'); + testValue(double.negativeInfinity, true, '-Infinity'); }); test('map value', () { @@ -255,7 +292,27 @@ void main() { .toProto3Json(typeRegistry: TypeRegistry([TestAllTypes()])), { '@type': 'type.googleapis.com/protobuf_unittest.TestAllTypes', - 'optionalFixed64': '100' + 'optionalFixed64': '100', + 'defaultInt32': 41, + 'defaultInt64': '42', + 'defaultUint32': 43, + 'defaultUint64': '44', + 'defaultSint32': -45, + 'defaultSint64': '46', + 'defaultFixed32': 47, + 'defaultFixed64': '48', + 'defaultSfixed32': 49, + 'defaultSfixed64': '-50', + 'defaultFloat': 51.5, + 'defaultDouble': 52000.0, + 'defaultBool': true, + 'defaultString': 'hello', + 'defaultBytes': 'd29ybGQ=', + 'defaultNestedEnum': 'BAR', + 'defaultForeignEnum': 'FOREIGN_BAR', + 'defaultImportEnum': 'IMPORT_BAR', + 'defaultStringPiece': 'abc', + 'defaultCord': '123', }); expect( Any.pack(Timestamp.fromDateTime(DateTime.utc(1969, 7, 20, 20, 17))) From 3cd3d15378ed12c96349d3bcacefc88f78a2c01a Mon Sep 17 00:00:00 2001 From: Jeff Martin Date: Thu, 7 Apr 2022 00:13:23 -0600 Subject: [PATCH 10/12] Throwing exception when proto3 json serialization reaches unexpected type --- .../lib/src/protobuf/json_serialization_context.dart | 9 +++++++++ protobuf/lib/src/protobuf/proto3_json.dart | 4 ++-- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/protobuf/lib/src/protobuf/json_serialization_context.dart b/protobuf/lib/src/protobuf/json_serialization_context.dart index 502e23e88..1fdab29c5 100644 --- a/protobuf/lib/src/protobuf/json_serialization_context.dart +++ b/protobuf/lib/src/protobuf/json_serialization_context.dart @@ -7,3 +7,12 @@ class JsonSerializationContext { JsonSerializationContext(this.emitDefaults); } + +class JsonSerializationException implements Exception { + final String message; + + JsonSerializationException(this.message); + + @override + String toString() => message; +} diff --git a/protobuf/lib/src/protobuf/proto3_json.dart b/protobuf/lib/src/protobuf/proto3_json.dart index a052d49a3..4eaeb5cea 100644 --- a/protobuf/lib/src/protobuf/proto3_json.dart +++ b/protobuf/lib/src/protobuf/proto3_json.dart @@ -176,8 +176,8 @@ Object? _writeToProto3Json( jsonValue = valueToProto3Json(value, fieldInfo.type); } } else { - skipField = false; - jsonValue = valueToProto3Json(value, fieldInfo.type); + throw JsonSerializationException( + 'Unexpected field type for proto3 JSON serialization ${fieldInfo.type}'); } if (!skipField) { From ed9527e83072e2e39cc2f285171595f9ef0f83a4 Mon Sep 17 00:00:00 2001 From: Jeff Martin Date: Thu, 7 Apr 2022 00:28:11 -0600 Subject: [PATCH 11/12] Removing unused functions from proto3 json serialization --- protobuf/lib/src/protobuf/proto3_json.dart | 8 -------- 1 file changed, 8 deletions(-) diff --git a/protobuf/lib/src/protobuf/proto3_json.dart b/protobuf/lib/src/protobuf/proto3_json.dart index 4eaeb5cea..0ab824e72 100644 --- a/protobuf/lib/src/protobuf/proto3_json.dart +++ b/protobuf/lib/src/protobuf/proto3_json.dart @@ -84,14 +84,6 @@ Object? _writeToProto3Json( } } - bool isDefaultListField(dynamic value) { - return value == null || (value is List && value.isEmpty); - } - - bool isDefaultMapField(dynamic value) { - return value == null || (value is Map && value.isEmpty); - } - final meta = fs._meta; if (meta.toProto3Json != null) { return meta.toProto3Json!(fs._message!, typeRegistry); From 89ba6cf96f65aba25df8f622cb253af3c3de126f Mon Sep 17 00:00:00 2001 From: Jeff Martin Date: Thu, 7 Apr 2022 00:30:25 -0600 Subject: [PATCH 12/12] Standardizing setting of list/map fields --- protobuf/test/json_test.dart | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/protobuf/test/json_test.dart b/protobuf/test/json_test.dart index a19e0c3dc..093ab8b66 100644 --- a/protobuf/test/json_test.dart +++ b/protobuf/test/json_test.dart @@ -26,8 +26,8 @@ void main() { ..enm = ProtobufEnum(1, 'a') ..dbl = doubleZeroVal ..bl = false - ..byts = [] - ..int32s.addAll([]); + ..byts = []; + exampleAllDefaults.int32s.addAll([]); exampleAllDefaults.mp.addAll({}); final double doubleSetVal = 1.34; @@ -39,8 +39,8 @@ void main() { ..enm = ProtobufEnum(1, 'b') ..dbl = doubleSetVal ..bl = true - ..byts = [46, 28] - ..int32s.addAll([3, 4, 6]); + ..byts = [46, 28]; + exampleAllSet.int32s.addAll([3, 4, 6]); exampleAllSet.mp.addAll({'k1': 'v2'}); test('testProto3JsonEnum', () {