diff --git a/CHANGELOG.md b/CHANGELOG.md index 731e10d0bfab6..650143aee5c32 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,16 @@ +## 0.11.0 + +* Breaking change: Support for [map fields](https://developers.google.com/protocol-buffers/docs/proto3#maps) + Generated files require package:protobuf version 0.10.5 or newer. + Protobuf map fields such as: + + message Foo { + map map_field = 1; + } + are now no longer represented as List but as Map. + + All code handling these fields needs to be updated. + ## 0.10.5 * Generated files now import `dart:async` with a prefix to prevent name diff --git a/Makefile b/Makefile index c6c338df1b6eb..e75116cace9e4 100644 --- a/Makefile +++ b/Makefile @@ -24,6 +24,7 @@ TEST_PROTO_LIST = \ import_clash \ map_api \ map_api2 \ + map_field \ mixins \ multiple_files_test \ nested_extension \ diff --git a/lib/file_generator.dart b/lib/file_generator.dart index 9dfaf63f981fc..0b9d2a3dbf315 100644 --- a/lib/file_generator.dart +++ b/lib/file_generator.dart @@ -245,7 +245,7 @@ class FileGenerator extends ProtobufContainer { // Make sure any other symbols in dart:core don't cause name conflicts with // protobuf classes that have the same name. out.println("// ignore: UNUSED_SHOWN_NAME\n" - "import 'dart:core' show int, bool, double, String, List, override;\n"); + "import 'dart:core' show int, bool, double, String, List, Map, override;\n"); if (_needsFixnumImport) { out.println("import 'package:fixnum/fixnum.dart';"); diff --git a/lib/message_generator.dart b/lib/message_generator.dart index 7a82ebb6f221d..365953a07e983 100644 --- a/lib/message_generator.dart +++ b/lib/message_generator.dart @@ -213,6 +213,9 @@ class MessageGenerator extends ProtobufContainer { checkResolved(); for (MessageGenerator m in _messageGenerators) { + // Don't output the generated map entry type. Instead, the `PbMap` type + // from the protobuf library is used to hold the keys and values. + if (m._descriptor.options.hasMapEntry()) continue; m.generate(out); } @@ -381,8 +384,8 @@ class MessageGenerator extends ProtobufContainer { var names = field.memberNames; _emitOverrideIf(field.overridesGetter, out); - var getterExpr = _getterExpression( - fieldTypeString, field.index, defaultExpr, field.isRepeated); + final getterExpr = _getterExpression(fieldTypeString, field.index, + defaultExpr, field.isRepeated, field.isMapField); out.println('${fieldTypeString} get ${names.fieldName} => ${getterExpr};'); if (field.isRepeated) { @@ -421,8 +424,11 @@ class MessageGenerator extends ProtobufContainer { } } - String _getterExpression( - String fieldType, int index, String defaultExpr, bool isRepeated) { + String _getterExpression(String fieldType, int index, String defaultExpr, + bool isRepeated, bool isMapField) { + if (isMapField) { + return '\$_getMap($index)'; + } if (fieldType == 'String') { return '\$_getS($index, $defaultExpr)'; } diff --git a/lib/protobuf_field.dart b/lib/protobuf_field.dart index 13764cafdf886..ce53c7d3334dd 100644 --- a/lib/protobuf_field.dart +++ b/lib/protobuf_field.dart @@ -72,11 +72,24 @@ class ProtobufField { /// True if this field uses the Int64 from the fixnum package. bool get needsFixnumImport => baseType.unprefixed == "Int64"; + /// True if this field is a map field definition: `map map_field = N`. + bool get isMapField { + if (!isRepeated || !baseType.isMessage) return false; + MessageGenerator generator = baseType.generator; + return generator._descriptor.options.hasMapEntry(); + } + /// Returns the expression to use for the Dart type. /// /// This will be a List for repeated types. /// [fileGen] represents the .proto file where we are generating code. String getDartType(FileGenerator fileGen) { + if (isMapField) { + MessageGenerator d = baseType.generator; + String keyType = d._fieldList[0].baseType.getDartType(fileGen); + String valueType = d._fieldList[1].baseType.getDartType(fileGen); + return 'Map<$keyType, $valueType>'; + } if (isRepeated) return baseType.getRepeatedDartType(fileGen); return baseType.getDartType(fileGen); } @@ -106,18 +119,41 @@ class ProtobufField { String quotedName = "'$dartFieldName'"; String type = baseType.getDartType(fileGen); + if (isMapField) { + MessageGenerator generator = baseType.generator; + ProtobufField key = generator._fieldList[0]; + ProtobufField value = generator._fieldList[1]; + String keyType = key.baseType.getDartType(fileGen); + String valueType = value.baseType.getDartType(fileGen); + String keyTypeConstant = key.typeConstant; + String valTypeConstant = value.typeConstant; + + if (value.baseType.isMessage || value.baseType.isGroup) { + return '..m<$keyType, $valueType>($number, $quotedName, ' + '$keyTypeConstant, $valTypeConstant, $valueType.create)'; + } + if (value.baseType.isEnum) { + return '..m<$keyType, $valueType>($number, $quotedName, ' + '$keyTypeConstant, $valTypeConstant, null, $valueType.valueOf, ' + '$valueType.values)'; + } + return '..m<$keyType, $valueType>($number, $quotedName, ' + '$keyTypeConstant, $valTypeConstant)'; + } + if (isRepeated) { if (baseType.isMessage || baseType.isGroup) { return '..pp<$type>($number, $quotedName, $typeConstant,' ' $type.$checkItem, $type.create)'; - } else if (baseType.isEnum) { + } + if (baseType.isEnum) { return '..pp<$type>($number, $quotedName, $typeConstant,' ' $type.$checkItem, null, $type.valueOf, $type.values)'; - } else if (typeConstant == '$_protobufImportPrefix.PbFieldType.PS') { + } + if (typeConstant == '$_protobufImportPrefix.PbFieldType.PS') { return '..pPS($number, $quotedName)'; - } else { - return '..p<$type>($number, $quotedName, $typeConstant)'; } + return '..p<$type>($number, $quotedName, $typeConstant)'; } String makeDefault = generateDefaultFunction(fileGen); diff --git a/lib/src/dart_options.pb.dart b/lib/src/dart_options.pb.dart index 25816e1fd6512..058f2205b4558 100644 --- a/lib/src/dart_options.pb.dart +++ b/lib/src/dart_options.pb.dart @@ -5,7 +5,7 @@ // ignore_for_file: non_constant_identifier_names,library_prefixes,unused_import // ignore: UNUSED_SHOWN_NAME -import 'dart:core' show int, bool, double, String, List, override; +import 'dart:core' show int, bool, double, String, List, Map, override; import 'package:protobuf/protobuf.dart' as $pb; diff --git a/lib/src/descriptor.pb.dart b/lib/src/descriptor.pb.dart index 925d8ab41fc8a..09c44e09c037f 100644 --- a/lib/src/descriptor.pb.dart +++ b/lib/src/descriptor.pb.dart @@ -5,7 +5,7 @@ // ignore_for_file: non_constant_identifier_names,library_prefixes,unused_import // ignore: UNUSED_SHOWN_NAME -import 'dart:core' show int, bool, double, String, List, override; +import 'dart:core' show int, bool, double, String, List, Map, override; import 'package:fixnum/fixnum.dart'; import 'package:protobuf/protobuf.dart' as $pb; diff --git a/lib/src/plugin.pb.dart b/lib/src/plugin.pb.dart index cb34f633d7a47..f8a6ba2ee37ad 100644 --- a/lib/src/plugin.pb.dart +++ b/lib/src/plugin.pb.dart @@ -5,7 +5,7 @@ // ignore_for_file: non_constant_identifier_names,library_prefixes,unused_import // ignore: UNUSED_SHOWN_NAME -import 'dart:core' show int, bool, double, String, List, override; +import 'dart:core' show int, bool, double, String, List, Map, override; import 'package:protobuf/protobuf.dart' as $pb; diff --git a/pubspec.yaml b/pubspec.yaml index d01ed268365e6..5d447640a3fe8 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -10,7 +10,7 @@ environment: dependencies: fixnum: ^0.10.5 path: ^1.0.0 - protobuf: ^0.10.4 + protobuf: ^0.10.5 dart_style: ^1.0.6 dev_dependencies: diff --git a/test/goldens/grpc_service.pb b/test/goldens/grpc_service.pb index 9bf1ff929c889..2515d8758923d 100644 --- a/test/goldens/grpc_service.pb +++ b/test/goldens/grpc_service.pb @@ -5,7 +5,7 @@ // ignore_for_file: non_constant_identifier_names,library_prefixes,unused_import // ignore: UNUSED_SHOWN_NAME -import 'dart:core' show int, bool, double, String, List, override; +import 'dart:core' show int, bool, double, String, List, Map, override; import 'package:protobuf/protobuf.dart' as $pb; diff --git a/test/goldens/header_in_package.pb b/test/goldens/header_in_package.pb index 7b7407783ce47..0399a53ae68d1 100644 --- a/test/goldens/header_in_package.pb +++ b/test/goldens/header_in_package.pb @@ -5,7 +5,7 @@ // ignore_for_file: non_constant_identifier_names,library_prefixes,unused_import // ignore: UNUSED_SHOWN_NAME -import 'dart:core' show int, bool, double, String, List, override; +import 'dart:core' show int, bool, double, String, List, Map, override; import 'package:protobuf/protobuf.dart' as $pb; diff --git a/test/goldens/header_with_fixnum.pb b/test/goldens/header_with_fixnum.pb index 4d353b18d47a9..9df6c14d5b08d 100644 --- a/test/goldens/header_with_fixnum.pb +++ b/test/goldens/header_with_fixnum.pb @@ -5,7 +5,7 @@ // ignore_for_file: non_constant_identifier_names,library_prefixes,unused_import // ignore: UNUSED_SHOWN_NAME -import 'dart:core' show int, bool, double, String, List, override; +import 'dart:core' show int, bool, double, String, List, Map, override; import 'package:fixnum/fixnum.dart'; import 'package:protobuf/protobuf.dart' as $pb; diff --git a/test/goldens/imports.pb b/test/goldens/imports.pb index f75afa72e32f2..2c384a9696677 100644 --- a/test/goldens/imports.pb +++ b/test/goldens/imports.pb @@ -5,7 +5,7 @@ // ignore_for_file: non_constant_identifier_names,library_prefixes,unused_import // ignore: UNUSED_SHOWN_NAME -import 'dart:core' show int, bool, double, String, List, override; +import 'dart:core' show int, bool, double, String, List, Map, override; import 'package:protobuf/protobuf.dart' as $pb; diff --git a/test/goldens/oneMessage.pb b/test/goldens/oneMessage.pb index 0278d17dbea24..7ca300654e9cf 100644 --- a/test/goldens/oneMessage.pb +++ b/test/goldens/oneMessage.pb @@ -5,7 +5,7 @@ // ignore_for_file: non_constant_identifier_names,library_prefixes,unused_import // ignore: UNUSED_SHOWN_NAME -import 'dart:core' show int, bool, double, String, List, override; +import 'dart:core' show int, bool, double, String, List, Map, override; import 'package:protobuf/protobuf.dart' as $pb; diff --git a/test/goldens/service.pb b/test/goldens/service.pb index ba27e9b904df7..507b6bf14f3f4 100644 --- a/test/goldens/service.pb +++ b/test/goldens/service.pb @@ -6,7 +6,7 @@ import 'dart:async' as $async; // ignore: UNUSED_SHOWN_NAME -import 'dart:core' show int, bool, double, String, List, override; +import 'dart:core' show int, bool, double, String, List, Map, override; import 'package:protobuf/protobuf.dart' as $pb; diff --git a/test/goldens/topLevelEnum.pb b/test/goldens/topLevelEnum.pb index af9ca99acd463..bb968a8f439a7 100644 --- a/test/goldens/topLevelEnum.pb +++ b/test/goldens/topLevelEnum.pb @@ -5,7 +5,7 @@ // ignore_for_file: non_constant_identifier_names,library_prefixes,unused_import // ignore: UNUSED_SHOWN_NAME -import 'dart:core' show int, bool, double, String, List, override; +import 'dart:core' show int, bool, double, String, List, Map, override; export 'test.pbenum.dart'; diff --git a/test/map_field_test.dart b/test/map_field_test.dart new file mode 100644 index 0000000000000..594250fe67912 --- /dev/null +++ b/test/map_field_test.dart @@ -0,0 +1,271 @@ +#!/usr/bin/env dart +// Copyright (c) 2018, 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. + +library map_field_test; + +import 'dart:convert'; + +import 'package:test/test.dart'; + +import '../out/protos/map_field.pb.dart'; + +void main() { + void _setValues(TestMap testMap) { + testMap + ..int32ToInt32Field[1] = 11 + ..int32ToInt32Field[2] = 22 + ..int32ToInt32Field[3] = 33 + ..int32ToStringField[1] = '11' + ..int32ToStringField[2] = '22' + ..int32ToStringField[3] = '33' + ..int32ToBytesField[1] = utf8.encode('11') + ..int32ToBytesField[2] = utf8.encode('22') + ..int32ToBytesField[3] = utf8.encode('33') + ..int32ToEnumField[1] = TestMap_EnumValue.FOO + ..int32ToEnumField[2] = TestMap_EnumValue.BAR + ..int32ToEnumField[3] = TestMap_EnumValue.BAZ + ..int32ToMessageField[1] = (TestMap_MessageValue()..value = 11) + ..int32ToMessageField[2] = (TestMap_MessageValue()..value = 22) + ..int32ToMessageField[3] = (TestMap_MessageValue()..value = 33) + ..stringToInt32Field['1'] = 11 + ..stringToInt32Field['2'] = 22 + ..stringToInt32Field['3'] = 33; + } + + void _updateValues(TestMap testMap) { + testMap + ..int32ToInt32Field[1] = 111 + ..int32ToInt32Field.remove(2) + ..int32ToInt32Field[4] = 44 + ..int32ToStringField[1] = '111' + ..int32ToStringField.remove(2) + ..int32ToStringField[4] = '44' + ..int32ToBytesField[1] = utf8.encode('111') + ..int32ToBytesField.remove(2) + ..int32ToBytesField[4] = utf8.encode('44') + ..int32ToEnumField[1] = TestMap_EnumValue.BAR + ..int32ToEnumField.remove(2) + ..int32ToEnumField[4] = TestMap_EnumValue.ZOP + ..int32ToMessageField[1] = (TestMap_MessageValue()..value = 111) + ..int32ToMessageField.remove(2) + ..int32ToMessageField[4] = (TestMap_MessageValue()..value = 44) + ..stringToInt32Field['1'] = 111 + ..stringToInt32Field.remove('2') + ..stringToInt32Field['4'] = 44; + } + + void _expectEmpty(TestMap testMap) { + expect(testMap.int32ToInt32Field, isEmpty); + expect(testMap.int32ToStringField, isEmpty); + expect(testMap.int32ToBytesField, isEmpty); + expect(testMap.int32ToEnumField, isEmpty); + expect(testMap.int32ToMessageField, isEmpty); + expect(testMap.stringToInt32Field, isEmpty); + } + + void _expectMapValuesSet(TestMap testMap) { + expect(testMap.int32ToInt32Field[1], 11); + expect(testMap.int32ToInt32Field[2], 22); + expect(testMap.int32ToInt32Field[3], 33); + + expect(testMap.int32ToStringField[1], '11'); + expect(testMap.int32ToStringField[2], '22'); + expect(testMap.int32ToStringField[3], '33'); + + expect(testMap.int32ToBytesField[1], utf8.encode('11')); + expect(testMap.int32ToBytesField[2], utf8.encode('22')); + expect(testMap.int32ToBytesField[3], utf8.encode('33')); + + expect(testMap.int32ToEnumField[1], TestMap_EnumValue.FOO); + expect(testMap.int32ToEnumField[2], TestMap_EnumValue.BAR); + expect(testMap.int32ToEnumField[3], TestMap_EnumValue.BAZ); + + expect(testMap.int32ToMessageField[1].value, 11); + expect(testMap.int32ToMessageField[2].value, 22); + expect(testMap.int32ToMessageField[3].value, 33); + + expect(testMap.stringToInt32Field['1'], 11); + expect(testMap.stringToInt32Field['2'], 22); + expect(testMap.stringToInt32Field['3'], 33); + } + + void _expectMapValuesUpdated(TestMap testMap) { + expect(testMap.int32ToInt32Field.length, 3); + expect(testMap.int32ToInt32Field[1], 111); + expect(testMap.int32ToInt32Field[3], 33); + expect(testMap.int32ToInt32Field[4], 44); + + expect(testMap.int32ToStringField.length, 3); + expect(testMap.int32ToStringField[1], '111'); + expect(testMap.int32ToStringField[3], '33'); + expect(testMap.int32ToStringField[4], '44'); + + expect(testMap.int32ToBytesField.length, 3); + expect(testMap.int32ToBytesField[1], utf8.encode('111')); + expect(testMap.int32ToBytesField[3], utf8.encode('33')); + expect(testMap.int32ToBytesField[4], utf8.encode('44')); + + expect(testMap.int32ToEnumField.length, 3); + expect(testMap.int32ToEnumField[1], TestMap_EnumValue.BAR); + expect(testMap.int32ToEnumField[3], TestMap_EnumValue.BAZ); + expect(testMap.int32ToEnumField[4], TestMap_EnumValue.ZOP); + + expect(testMap.int32ToMessageField.length, 3); + expect(testMap.int32ToMessageField[1].value, 111); + expect(testMap.int32ToMessageField[3].value, 33); + expect(testMap.int32ToMessageField[4].value, 44); + + expect(testMap.stringToInt32Field.length, 3); + expect(testMap.stringToInt32Field['1'], 111); + expect(testMap.stringToInt32Field['3'], 33); + expect(testMap.stringToInt32Field['4'], 44); + } + + test('set and clear values', () { + TestMap testMap = TestMap(); + _expectEmpty(testMap); + + _setValues(testMap); + _expectMapValuesSet(testMap); + + testMap.clear(); + _expectEmpty(testMap); + }); + + test('update map values', () { + TestMap testMap = TestMap(); + _setValues(testMap); + _updateValues(testMap); + _expectMapValuesUpdated(testMap); + }); + + test('null keys and value are not allowed', () { + TestMap testMap = TestMap(); + + try { + testMap.stringToInt32Field[null] = 1; + fail('Should have thrown an exception.'); + } on ArgumentError catch (e) { + expect(e.message, "Can't add a null to a map field"); + } + + try { + testMap.int32ToBytesField[1] = null; + fail('Should have thrown an exception.'); + } on ArgumentError catch (e) { + expect(e.message, "Can't add a null to a map field"); + } + + try { + testMap.int32ToStringField[1] = null; + fail('Should have thrown an exception.'); + } on ArgumentError catch (e) { + expect(e.message, "Can't add a null to a map field"); + } + + try { + testMap.int32ToEnumField[1] = null; + fail('Should have thrown an exception.'); + } on ArgumentError catch (e) { + expect(e.message, "Can't add a null to a map field"); + } + + try { + testMap.int32ToMessageField[1] = null; + fail('Should have thrown an exception.'); + } on ArgumentError catch (e) { + expect(e.message, "Can't add a null to a map field"); + } + }); + + test('Serialize and parse map', () { + TestMap testMap = TestMap(); + _setValues(testMap); + + testMap = TestMap.fromBuffer(testMap.writeToBuffer()); + _expectMapValuesSet(testMap); + + _updateValues(testMap); + testMap = TestMap.fromBuffer(testMap.writeToBuffer()); + _expectMapValuesUpdated(testMap); + + testMap.clear(); + testMap = TestMap.fromBuffer(testMap.writeToBuffer()); + _expectEmpty(testMap); + }); + + test('json serialize map', () { + TestMap testMap = TestMap(); + _setValues(testMap); + + testMap = TestMap.fromJson(testMap.writeToJson()); + _expectMapValuesSet(testMap); + + _updateValues(testMap); + testMap = TestMap.fromJson(testMap.writeToJson()); + _expectMapValuesUpdated(testMap); + + testMap.clear(); + testMap = TestMap.fromJson(testMap.writeToJson()); + _expectEmpty(testMap); + }); + + test('merge from other message', () { + TestMap testMap = TestMap(); + _setValues(testMap); + + TestMap other = TestMap(); + other.mergeFromMessage(testMap); + _expectMapValuesSet(other); + + testMap = TestMap() + ..int32ToMessageField[1] = (TestMap_MessageValue()..value = 42) + ..int32ToMessageField[2] = (TestMap_MessageValue()..value = 44); + other = TestMap() + ..int32ToMessageField[1] = (TestMap_MessageValue()..secondValue = 43); + testMap.mergeFromMessage(other); + + expect(testMap.int32ToMessageField[1].value, 0); + expect(testMap.int32ToMessageField[1].secondValue, 43); + expect(testMap.int32ToMessageField[2].value, 44); + }); + + test('parse duplicate keys', () { + TestMap testMap = TestMap()..int32ToStringField[1] = 'foo'; + TestMap testMap2 = TestMap()..int32ToStringField[1] = 'bar'; + + TestMap merge = TestMap.fromBuffer( + []..addAll(testMap.writeToBuffer())..addAll(testMap2.writeToBuffer())); + + // When parsing from the wire, if there are duplicate map keys the last key + // seen should be used. + expect(merge.int32ToStringField.length, 1); + expect(merge.int32ToStringField[1], 'bar'); + }); + + // TODO(zarah): remove skip once https://github.com/dart-lang/protobuf/issues/139 is fixed. + test('Deep merge from other message', () { + Inner i1 = Inner()..innerMap['a'] = 'a'; + Inner i2 = Inner()..innerMap['b'] = 'b'; + + Outer o1 = Outer()..i = i1; + Outer o2 = Outer()..i = i2; + + o1.mergeFromMessage(o2); + expect(o1.i.innerMap.length, 2); + }, skip: true); + + test('retain explicit default values of sub-messages', () { + TestMap testMap = TestMap() + ..int32ToMessageField[1] = TestMap_MessageValue(); + expect(testMap.int32ToMessageField[1].secondValue, 42); + + TestMap testMap2 = TestMap() + ..int32ToMessageField[2] = TestMap_MessageValue(); + + testMap.mergeFromBuffer(testMap2.writeToBuffer()); + expect(testMap.int32ToMessageField[2].secondValue, 42); + }); +} diff --git a/test/protos/map_field.proto b/test/protos/map_field.proto new file mode 100644 index 0000000000000..d5af64f640cc2 --- /dev/null +++ b/test/protos/map_field.proto @@ -0,0 +1,38 @@ +// Copyright (c) 2018, 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. + +syntax = "proto2"; + +package protobuf_unittest; + +message TestMap { + message MessageValue { + optional int32 value = 1; + optional int32 secondValue = 2 [default = 42]; + } + + enum EnumValue { + FOO = 0; + BAR = 1; + BAZ = 2; + ZOP = 3; + } + + map int32_to_int32_field = 1; + map int32_to_string_field = 2; + map int32_to_bytes_field = 3; + map int32_to_enum_field = 4; + map int32_to_message_field = 5; + map string_to_int32_field = 6; + map uint32_to_int32_field = 7; + map int64_to_int32_field = 8; +} + +message Inner { + map inner_map = 1; +} + +message Outer { + optional Inner i = 1; +}