Skip to content

Commit

Permalink
Convert null keys and values to default when parsing map entries (#536)
Browse files Browse the repository at this point in the history
  • Loading branch information
@sigurdm
sigurdm authored Dec 2, 2021
1 parent bb4cf0b commit 18346f5
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 57 deletions.
6 changes: 4 additions & 2 deletions protobuf/lib/src/protobuf/pb_map.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,10 @@ class PbMap<K, V> extends MapBase<K, V> {
_mergeFromCodedBufferReader(mapEntryMeta, entryFieldSet, input, registry!);
input.checkLastTagWas(0);
input._currentLimit = oldLimit;
var key = entryFieldSet._$get<K>(0, null);
var value = entryFieldSet._$get<V>(1, null);
var key =
entryFieldSet._values[0] ?? mapEntryMeta.byIndex[0].makeDefault!();
var value =
entryFieldSet._values[1] ?? mapEntryMeta.byIndex[1].makeDefault!();
_wrappedMap[key] = value;
}

Expand Down
91 changes: 36 additions & 55 deletions protoc_plugin/test/map_field_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@
// 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.

// @dart=2.11

import 'dart:convert';

import 'package:protobuf/protobuf.dart';
Expand Down Expand Up @@ -83,9 +81,9 @@ void main() {
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.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);
Expand Down Expand Up @@ -114,9 +112,9 @@ void main() {
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.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);
Expand All @@ -142,45 +140,6 @@ void main() {
_expectMapValuesUpdated(testMap);
});

test('null keys and value are not allowed', () {
var 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', () {
var testMap = TestMap();
_setValues(testMap);
Expand Down Expand Up @@ -255,9 +214,9 @@ void main() {
..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);
expect(testMap.int32ToMessageField[1]!.value, 0);
expect(testMap.int32ToMessageField[1]!.secondValue, 43);
expect(testMap.int32ToMessageField[2]!.value, 44);
});

test('parse duplicate keys', () {
Expand Down Expand Up @@ -286,12 +245,12 @@ void main() {

test('retain explicit default values of sub-messages', () {
var testMap = TestMap()..int32ToMessageField[1] = TestMap_MessageValue();
expect(testMap.int32ToMessageField[1].secondValue, 42);
expect(testMap.int32ToMessageField[1]!.secondValue, 42);

var testMap2 = TestMap()..int32ToMessageField[2] = TestMap_MessageValue();

testMap.mergeFromBuffer(testMap2.writeToBuffer());
expect(testMap.int32ToMessageField[2].secondValue, 42);
expect(testMap.int32ToMessageField[2]!.secondValue, 42);
});

test('Freeze message with map field', () {
Expand All @@ -301,7 +260,7 @@ void main() {

expect(() => _updateValues(testMap),
throwsA(const TypeMatcher<UnsupportedError>()));
expect(() => testMap.int32ToMessageField[1].value = 42,
expect(() => testMap.int32ToMessageField[1]!.value = 42,
throwsA(const TypeMatcher<UnsupportedError>()));
expect(() => testMap.int32ToStringField.remove(1),
throwsA(const TypeMatcher<UnsupportedError>()));
Expand All @@ -315,8 +274,8 @@ void main() {
testMap.int32ToMessageField[2] = (TestMap_MessageValue()..secondValue = 12);

void testValues(TestMap candidate) {
final message1 = candidate.int32ToMessageField[1];
final message2 = candidate.int32ToMessageField[2];
final message1 = candidate.int32ToMessageField[1]!;
final message2 = candidate.int32ToMessageField[2]!;

expect(message1.hasValue(), true);
expect(message1.value, 11);
Expand Down Expand Up @@ -365,4 +324,26 @@ void main() {
);
_expectMapValuesSet(testMap);
});

test('Parses null keys and values', () {
// Use a desugared version of the message to create missing
// values in the serialized form.
final d = Desugared(int32ToStringField: [
Desugared_Int32ToString(key: null, value: 'abc'),
Desugared_Int32ToString(key: 42, value: null),
Desugared_Int32ToString(key: 11, value: 'def'),
], stringToInt32Field: [
Desugared_StringToInt32(key: null, value: 11),
Desugared_StringToInt32(key: 'abc', value: null),
Desugared_StringToInt32(key: 'def', value: 42),
]);

final m = TestMap.fromBuffer(d.writeToBuffer());
expect(m.int32ToStringField[0], 'abc');
expect(m.int32ToStringField[42], '');
expect(m.int32ToStringField[11], 'def');
expect(m.stringToInt32Field['abc'], 0);
expect(m.stringToInt32Field[''], 11);
expect(m.stringToInt32Field['def'], 42);
});
}
13 changes: 13 additions & 0 deletions protoc_plugin/test/protos/map_field.proto
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,3 +37,16 @@ message Inner {
message Outer {
optional Inner i = 1;
}

message Desugared {
message Int32ToString {
optional int32 key = 1;
optional string value = 2;
}
message StringToInt32 {
optional string key = 1;
optional int32 value = 2;
}
repeated Int32ToString int32_to_string_field = 2;
repeated StringToInt32 string_to_int32_field = 6;
}

0 comments on commit 18346f5

Please sign in to comment.