Change ExtensionRegistry.reparseMessage to also reparse extensions on… (#277)
diff --git a/protobuf/CHANGELOG.md b/protobuf/CHANGELOG.md
index fb1c577..fba6f37 100644
--- a/protobuf/CHANGELOG.md
+++ b/protobuf/CHANGELOG.md
@@ -5,6 +5,9 @@
* **Breaking**: The field-adder methods on `BuilderInfo` now takes only named optional arguments.
To migrate, update `protoc_plugin` to version 18.0.0 or higher.
* The field-adder methods on `BuilderInfo` all take a new argument `protoName`.
+* **Breaking**: Changed `ExtensionRegistry.reparseMessage` to reparse extensions deeply, that is it looks at every
+nested message and tries to reparse extensions from its unknown fields.
+
## 0.13.16
diff --git a/protobuf/lib/src/protobuf/extension_registry.dart b/protobuf/lib/src/protobuf/extension_registry.dart
index 0892c2d..44ca3b7 100644
--- a/protobuf/lib/src/protobuf/extension_registry.dart
+++ b/protobuf/lib/src/protobuf/extension_registry.dart
@@ -35,12 +35,14 @@
}
/// Returns a shallow copy of [message], with all extensions in [this] parsed
- /// from the unknown fields of [message].
+ /// from the unknown fields of [message] and of every nested submessage.
///
/// Extensions already present in [message] will be preserved.
///
/// If [message] is frozen, the result will be as well.
///
+ /// Returns the original message if no new extensions are parsed.
+ ///
/// Throws an [InvalidProtocolBufferException] if the parsed extensions are
/// malformed.
///
@@ -50,7 +52,7 @@
///
/// Example:
///
- /// `foo.proto`
+ /// `sample.proto`
/// ```proto
/// syntax = "proto2";
///
@@ -90,28 +92,89 @@
T _reparseMessage<T extends GeneratedMessage>(
T message, ExtensionRegistry extensionRegistry) {
- T result = message.createEmptyInstance();
+ T result;
+ T ensureResult() {
+ if (result == null) {
+ result ??= message.createEmptyInstance();
+ result._fieldSet._shallowCopyValues(message._fieldSet);
+ }
+ return result;
+ }
- result._fieldSet._shallowCopyValues(message._fieldSet);
- UnknownFieldSet resultUnknownFields = result._fieldSet._unknownFields;
- if (resultUnknownFields != null) {
+ UnknownFieldSet resultUnknownFields;
+ UnknownFieldSet ensureUnknownFields() =>
+ resultUnknownFields ??= ensureResult()._fieldSet._unknownFields;
+
+ UnknownFieldSet messageUnknownFields = message._fieldSet._unknownFields;
+ if (messageUnknownFields != null) {
CodedBufferWriter codedBufferWriter = CodedBufferWriter();
extensionRegistry._extensions[message.info_.qualifiedMessageName]
?.forEach((tagNumber, extension) {
final UnknownFieldSetField unknownField =
- resultUnknownFields._fields[tagNumber];
+ messageUnknownFields._fields[tagNumber];
if (unknownField != null) {
unknownField.writeTo(tagNumber, codedBufferWriter);
+ ensureUnknownFields()._fields.remove(tagNumber);
}
- resultUnknownFields._fields.remove(tagNumber);
});
- result.mergeFromBuffer(codedBufferWriter.toBuffer(), extensionRegistry);
+ if (codedBufferWriter.toBuffer().length > 0) {
+ ensureResult()
+ .mergeFromBuffer(codedBufferWriter.toBuffer(), extensionRegistry);
+ }
}
- if (message._fieldSet._isReadOnly) {
+
+ message._fieldSet._meta.byIndex.forEach((FieldInfo field) {
+ PbList resultEntries;
+ PbList ensureEntries() =>
+ resultEntries ??= ensureResult()._fieldSet._values[field.index];
+
+ PbMap resultMap;
+ PbMap ensureMap() =>
+ resultMap ??= ensureResult()._fieldSet._values[field.index];
+
+ if (field.isRepeated) {
+ final messageEntries = message._fieldSet._values[field.index];
+ if (messageEntries == null) return;
+ if (field.isGroupOrMessage) {
+ for (int i = 0; i < messageEntries.length; i++) {
+ final GeneratedMessage entry = messageEntries[i];
+ final GeneratedMessage reparsedEntry =
+ _reparseMessage(entry, extensionRegistry);
+ if (!identical(entry, reparsedEntry)) {
+ ensureEntries()[i] = reparsedEntry;
+ }
+ }
+ }
+ } else if (field.isMapField) {
+ final messageMap = message._fieldSet._values[field.index];
+ if (messageMap == null) return;
+ if (field.isGroupOrMessage) {
+ for (var key in messageMap.keys) {
+ final GeneratedMessage value = messageMap[key];
+ final GeneratedMessage reparsedValue =
+ _reparseMessage(value, extensionRegistry);
+ if (!identical(value, reparsedValue)) {
+ ensureMap()[key] = reparsedValue;
+ }
+ }
+ }
+ } else if (field.isGroupOrMessage) {
+ final messageSubField = message._fieldSet._values[field.index];
+ if (messageSubField == null) return;
+ final GeneratedMessage reparsedSubField =
+ _reparseMessage(messageSubField, extensionRegistry);
+ if (!identical(messageSubField, reparsedSubField)) {
+ ensureResult()._fieldSet._values[field.index] = reparsedSubField;
+ }
+ }
+ });
+
+ if (result != null && message.isFrozen) {
result.freeze();
}
- return result;
+
+ return result ?? message;
}
class _EmptyExtensionRegistry implements ExtensionRegistry {
diff --git a/protoc_plugin/lib/src/dart_options.pb.dart b/protoc_plugin/lib/src/dart_options.pb.dart
index 74f381a..b3aca8c 100644
--- a/protoc_plugin/lib/src/dart_options.pb.dart
+++ b/protoc_plugin/lib/src/dart_options.pb.dart
@@ -5,8 +5,7 @@
// @dart = 2.3
// ignore_for_file: camel_case_types,non_constant_identifier_names,library_prefixes,unused_import,unused_shown_name,return_of_invalid_type
-import 'dart:core' as $core
- show bool, Deprecated, double, int, List, Map, override, pragma, String;
+import 'dart:core' as $core;
import 'package:protobuf/protobuf.dart' as $pb;
diff --git a/protoc_plugin/lib/src/descriptor.pb.dart b/protoc_plugin/lib/src/descriptor.pb.dart
index 4697c1a..a0d6b2b 100644
--- a/protoc_plugin/lib/src/descriptor.pb.dart
+++ b/protoc_plugin/lib/src/descriptor.pb.dart
@@ -5,8 +5,7 @@
// @dart = 2.3
// ignore_for_file: camel_case_types,non_constant_identifier_names,library_prefixes,unused_import,unused_shown_name,return_of_invalid_type
-import 'dart:core' as $core
- show bool, Deprecated, double, int, List, Map, override, pragma, String;
+import 'dart:core' as $core;
import 'package:fixnum/fixnum.dart';
import 'package:protobuf/protobuf.dart' as $pb;
diff --git a/protoc_plugin/lib/src/descriptor.pbenum.dart b/protoc_plugin/lib/src/descriptor.pbenum.dart
index 8b3e18b..b31a733 100644
--- a/protoc_plugin/lib/src/descriptor.pbenum.dart
+++ b/protoc_plugin/lib/src/descriptor.pbenum.dart
@@ -6,7 +6,7 @@
// ignore_for_file: camel_case_types,non_constant_identifier_names,library_prefixes,unused_import,unused_shown_name,return_of_invalid_type
// ignore_for_file: UNDEFINED_SHOWN_NAME,UNUSED_SHOWN_NAME
-import 'dart:core' as $core show int, dynamic, String, List, Map;
+import 'dart:core' as $core;
import 'package:protobuf/protobuf.dart' as $pb;
class FieldDescriptorProto_Type extends $pb.ProtobufEnum {
diff --git a/protoc_plugin/lib/src/plugin.pb.dart b/protoc_plugin/lib/src/plugin.pb.dart
index e7e57f3..2056f9d 100644
--- a/protoc_plugin/lib/src/plugin.pb.dart
+++ b/protoc_plugin/lib/src/plugin.pb.dart
@@ -5,8 +5,7 @@
// @dart = 2.3
// ignore_for_file: camel_case_types,non_constant_identifier_names,library_prefixes,unused_import,unused_shown_name,return_of_invalid_type
-import 'dart:core' as $core
- show bool, Deprecated, double, int, List, Map, override, pragma, String;
+import 'dart:core' as $core;
import 'package:protobuf/protobuf.dart' as $pb;
diff --git a/protoc_plugin/test/extension_test.dart b/protoc_plugin/test/extension_test.dart
index a5c14bb..8a37ea7 100644
--- a/protoc_plugin/test/extension_test.dart
+++ b/protoc_plugin/test/extension_test.dart
@@ -336,7 +336,7 @@
final reparsedWithEmpty =
ExtensionRegistry().reparseMessage(withUnknownFields);
- expect(reparsedWithEmpty, isNot(same(withUnknownFields)));
+ expect(reparsedWithEmpty, same(withUnknownFields));
final reparsed = r.reparseMessage(withUnknownFields);
@@ -386,4 +386,165 @@
.setExtension(Unittest.defaultStringExtension, 'blah'),
throwsA(TypeMatcher<UnsupportedError>()));
});
+
+ test(
+ 'ExtensionRegistry.reparseMessage returns the same object when no new extensions are parsed',
+ () {
+ final original = Outer()
+ ..inner = (Inner()
+ ..innerMost = (InnerMost()
+ ..setExtension(Extend_unittest.innerMostExtensionString, 'a')))
+ ..inners.addAll([
+ (Inner()..setExtension(Extend_unittest.innerExtensionString, "a")),
+ Inner()..value = 'value'
+ ])
+ ..innerMap[0] =
+ (Inner()..setExtension(Extend_unittest.innerExtensionString, "a"));
+
+ final reparsed = ExtensionRegistry().reparseMessage(original);
+ expect(identical(reparsed, original), isTrue);
+ });
+
+ test(
+ 'ExtensionRegistry.reparseMessage reparses extension in deeply nested subfield',
+ () {
+ final original = Outer()
+ ..inner = (Inner()
+ ..innerMost = (InnerMost()
+ ..setExtension(Extend_unittest.innerMostExtensionString, 'a')));
+
+ final withUnknownFields = Outer.fromBuffer(original.writeToBuffer());
+
+ expect(
+ withUnknownFields.inner.innerMost
+ .hasExtension(Extend_unittest.innerMostExtensionString),
+ isFalse);
+
+ final r = ExtensionRegistry()
+ ..add(Extend_unittest.innerMostExtensionString);
+ final reparsed = r.reparseMessage(withUnknownFields);
+
+ expect(identical(reparsed.inner, withUnknownFields.inner), isFalse);
+ expect(
+ identical(reparsed.inner.innerMost, withUnknownFields.inner.innerMost),
+ isFalse);
+ expect(
+ reparsed.inner.innerMost
+ .hasExtension(Extend_unittest.innerMostExtensionString),
+ isTrue);
+ });
+
+ test(
+ 'ExtensionRegistry.reparseMessage reparses extensions in all nested subfields',
+ () {
+ final original = Outer()
+ ..inner = (Inner()
+ ..setExtension(Extend_unittest.innerExtensionString, "a")
+ ..innerMost = (InnerMost()
+ ..setExtension(Extend_unittest.innerMostExtensionString, 'a')));
+
+ final withUnknownFields = Outer.fromBuffer(original.writeToBuffer());
+
+ expect(
+ withUnknownFields.inner
+ .hasExtension(Extend_unittest.innerExtensionString),
+ isFalse);
+ expect(
+ withUnknownFields.inner.innerMost
+ .hasExtension(Extend_unittest.innerMostExtensionString),
+ isFalse);
+
+ final r = ExtensionRegistry()
+ ..addAll([
+ Extend_unittest.innerExtensionString,
+ Extend_unittest.innerMostExtensionString
+ ]);
+ final reparsed = r.reparseMessage(withUnknownFields);
+
+ expect(identical(reparsed.inner, withUnknownFields.inner), isFalse);
+ expect(
+ identical(reparsed.inner.innerMost, withUnknownFields.inner.innerMost),
+ isFalse);
+ expect(reparsed.inner.hasExtension(Extend_unittest.innerExtensionString),
+ isTrue);
+ expect(
+ reparsed.inner.innerMost
+ .hasExtension(Extend_unittest.innerMostExtensionString),
+ isTrue);
+ });
+
+ test(
+ 'ExtensionRegistry.reparseMessage doesn\'t copy deepest subfield without extensions',
+ () {
+ final original = Outer()
+ ..inner = (Inner()
+ ..setExtension(Extend_unittest.innerExtensionString, "a")
+ ..innerMost = InnerMost());
+
+ final withUnknownFields = Outer.fromBuffer(original.writeToBuffer());
+
+ expect(
+ withUnknownFields.inner
+ .hasExtension(Extend_unittest.innerExtensionString),
+ false);
+
+ final r = ExtensionRegistry()..add(Extend_unittest.innerExtensionString);
+ final reparsed = r.reparseMessage(withUnknownFields);
+
+ expect(identical(reparsed.inner, withUnknownFields.inner), isFalse);
+ expect(
+ identical(reparsed.inner.innerMost, withUnknownFields.inner.innerMost),
+ isTrue);
+ expect(reparsed.inner.hasExtension(Extend_unittest.innerExtensionString),
+ isTrue);
+ });
+
+ test(
+ 'ExtensionRegistry.reparseMessage reparses extensions in repeated fields',
+ () {
+ final original = Outer()
+ ..inners.addAll([
+ (Inner()..setExtension(Extend_unittest.innerExtensionString, "a")),
+ Inner()..value = 'value'
+ ]);
+
+ final withUnknownFields = Outer.fromBuffer(original.writeToBuffer());
+
+ expect(
+ withUnknownFields.inners.first
+ .hasExtension(Extend_unittest.innerExtensionString),
+ isFalse);
+
+ final r = ExtensionRegistry()..add(Extend_unittest.innerExtensionString);
+ final reparsed = r.reparseMessage(withUnknownFields);
+
+ expect(
+ reparsed.inners[0].hasExtension(Extend_unittest.innerExtensionString),
+ isTrue);
+ expect(identical(withUnknownFields.inners[1], reparsed.inners[1]), isTrue);
+ });
+
+ test('ExtensionRegistry.reparseMessage reparses extensions in map fields',
+ () {
+ final original = Outer()
+ ..innerMap[0] =
+ (Inner()..setExtension(Extend_unittest.innerExtensionString, "a"))
+ ..innerMap[1] = (Inner());
+
+ final withUnknownFields = Outer.fromBuffer(original.writeToBuffer());
+
+ expect(
+ withUnknownFields.innerMap[0]
+ .hasExtension(Extend_unittest.innerExtensionString),
+ isFalse);
+
+ final r = ExtensionRegistry()..add(Extend_unittest.innerExtensionString);
+ final reparsed = r.reparseMessage(withUnknownFields);
+
+ expect(
+ reparsed.innerMap[0].hasExtension(Extend_unittest.innerExtensionString),
+ isTrue);
+ expect(
+ identical(withUnknownFields.innerMap[1], reparsed.innerMap[1]), isTrue);
+ });
}
diff --git a/protoc_plugin/test/protos/extend_unittest.proto b/protoc_plugin/test/protos/extend_unittest.proto
index cdbdaee..93084e8 100644
--- a/protoc_plugin/test/protos/extend_unittest.proto
+++ b/protoc_plugin/test/protos/extend_unittest.proto
@@ -8,15 +8,32 @@
message Outer {
optional Inner inner = 1;
- extensions 2 to max;
+ repeated Inner inners = 2;
+ map<int32, Inner> inner_map = 3;
+ extensions 4 to max;
}
extend Outer {
- optional Inner extension_inner = 2;
+ optional Inner extension_inner = 4;
}
message Inner {
optional string value = 1;
+ optional InnerMost inner_most = 2;
+ extensions 3 to max;
+}
+
+extend Inner {
+ optional string inner_extension_string = 3;
+}
+
+message InnerMost {
+ optional string value = 1;
+ extensions 2 to max;
+}
+
+extend InnerMost {
+ optional string inner_most_extension_string = 2;
}
extend protobuf_unittest.TestAllExtensions {
