diff --git a/protobuf/CHANGELOG.md b/protobuf/CHANGELOG.md index f1cd835f3..1bae5efe8 100644 --- a/protobuf/CHANGELOG.md +++ b/protobuf/CHANGELOG.md @@ -1,3 +1,13 @@ +## 1.0.2 + +* Fixed generic handling of unknown fields. + * `GeneratedMessage.clearField(tagNumber)` will now also clear an unknown + field. + * `GeneratedMessage.hasField(tagNumber)` will now return true if there is an + unknown field. +* Handle cases where a field that is part of a oneof is removed by the + protobuf aware treeshaker. + ## 1.0.1 * Fix issue with `ExtensionRegistry.reparseMessage` not handling map fields with diff --git a/protobuf/lib/src/protobuf/coded_buffer.dart b/protobuf/lib/src/protobuf/coded_buffer.dart index b7cd01603..0788c5ce1 100644 --- a/protobuf/lib/src/protobuf/coded_buffer.dart +++ b/protobuf/lib/src/protobuf/coded_buffer.dart @@ -42,6 +42,7 @@ void _mergeFromCodedBufferReader( } if (fi == null || !_wireTypeMatches(fi.type, wireType)) { + fs._updateOneOfCase(tagNumber); if (!fs._ensureUnknownFields().mergeFieldFromBuffer(tag, input)) { return; } diff --git a/protobuf/lib/src/protobuf/field_set.dart b/protobuf/lib/src/protobuf/field_set.dart index 04256f124..3f7b50d16 100644 --- a/protobuf/lib/src/protobuf/field_set.dart +++ b/protobuf/lib/src/protobuf/field_set.dart @@ -236,24 +236,23 @@ class _FieldSet { bool _hasField(int tagNumber) { var fi = _nonExtensionInfo(tagNumber); if (fi != null) return _$has(fi.index); + if (_hasUnknownFields && _unknownFields.hasField(tagNumber)) { + return true; + } if (!_hasExtensions) return false; return _extensions._hasField(tagNumber); } void _clearField(int tagNumber) { _ensureWritable(); + int oneofIndex = _meta.oneofs[tagNumber]; + if (oneofIndex != null) _oneofCases.remove(oneofIndex); + var fi = _nonExtensionInfo(tagNumber); if (fi != null) { // clear a non-extension field if (_hasObservers) _eventPlugin.beforeClearField(fi); _values[fi.index] = null; - - if (_meta.oneofs.containsKey(fi.tagNumber)) { - _oneofCases.remove(_meta.oneofs[fi.tagNumber]); - } - - int oneofIndex = _meta.oneofs[fi.tagNumber]; - if (oneofIndex != null) _oneofCases[oneofIndex] = 0; return; } @@ -265,8 +264,9 @@ class _FieldSet { } } - // neither a regular field nor an extension. - // TODO(skybrian) throw? + if (_hasUnknownFields) { + _unknownFields._clearField(tagNumber); + } } /// Sets a non-repeated field with error-checking. @@ -342,14 +342,18 @@ class _FieldSet { return newValue; } - /// Sets a non-extended field and fires events. - void _setNonExtensionFieldUnchecked(FieldInfo fi, value) { - int tag = fi.tagNumber; - int oneofIndex = _meta.oneofs[tag]; + void _updateOneOfCase(int newTagnumber) { + int oneofIndex = _meta.oneofs[newTagnumber]; if (oneofIndex != null) { - _clearField(_oneofCases[oneofIndex]); - _oneofCases[oneofIndex] = tag; + final currentTagnumber = _oneofCases[oneofIndex]; + if (currentTagnumber != null) _clearField(currentTagnumber); + _oneofCases[oneofIndex] = newTagnumber; } + } + + /// Sets a non-extended field and fires events. + void _setNonExtensionFieldUnchecked(FieldInfo fi, value) { + _updateOneOfCase(fi.tagNumber); // It is important that the callback to the observers is not moved to the // beginning of this method but happens just before the value is set. @@ -498,13 +502,7 @@ class _FieldSet { if (_hasObservers) { _eventPlugin.beforeSetField(_nonExtensionInfoByIndex(index), value); } - int tag = _meta.byIndex[index].tagNumber; - int oneofIndex = _meta.oneofs[tag]; - - if (oneofIndex != null) { - _clearField(_oneofCases[oneofIndex]); - _oneofCases[oneofIndex] = tag; - } + _updateOneOfCase(_meta.byIndex[index].tagNumber); _values[index] = value; } @@ -713,7 +711,12 @@ class _FieldSet { } if (other._hasUnknownFields) { - _ensureUnknownFields().mergeFromUnknownFieldSet(other._unknownFields); + final unknownFieldSet = _ensureUnknownFields(); + final otherUnknownFieldSet = other._unknownFields; + otherUnknownFieldSet._fields.forEach((key, value) { + _updateOneOfCase(key); + unknownFieldSet.mergeField(key, value); + }); } } diff --git a/protobuf/lib/src/protobuf/generated_message.dart b/protobuf/lib/src/protobuf/generated_message.dart index 09445693c..9659cf235 100644 --- a/protobuf/lib/src/protobuf/generated_message.dart +++ b/protobuf/lib/src/protobuf/generated_message.dart @@ -300,8 +300,11 @@ abstract class GeneratedMessage { /// Clears the contents of a given field. /// - /// If it's an extension field, the Extension will be kept. - /// The tagNumber should be a valid tag or extension. + /// If it's an extension field, the value will be removed, but the Extension + /// will be kept. + /// + /// If the tagNumber is present as an unknown field, that field will be + /// removed. void clearField(int tagNumber) => _fieldSet._clearField(tagNumber); int $_whichOneof(int oneofIndex) => _fieldSet._oneofCases[oneofIndex] ?? 0; @@ -355,6 +358,8 @@ abstract class GeneratedMessage { _fieldSet._extensions._getFieldOrNull(extension) != null; /// Returns [:true:] if this message has a field associated with [tagNumber]. + /// + /// Will return [:true:] even if that field is an unknown field. bool hasField(int tagNumber) => _fieldSet._hasField(tagNumber); /// Merges the contents of the [other] into this message. diff --git a/protobuf/lib/src/protobuf/unknown_field_set.dart b/protobuf/lib/src/protobuf/unknown_field_set.dart index a136f36de..5488491d4 100644 --- a/protobuf/lib/src/protobuf/unknown_field_set.dart +++ b/protobuf/lib/src/protobuf/unknown_field_set.dart @@ -31,8 +31,15 @@ class UnknownFieldSet { UnknownFieldSetField getField(int tagNumber) => _fields[tagNumber]; + /// Returns `true` if [tagNumber] is set in [this]. bool hasField(int tagNumber) => _fields.containsKey(tagNumber); + /// Removes [tagNumber] from [this]. + /// + /// Does nothing if [tagNumber] is unset. + void _clearField(int tagNumber) => _fields.remove(tagNumber); + + // TODO(sigurdm): Does not update oneofs. Consider deprecating. void addField(int number, UnknownFieldSetField field) { _ensureWritable('addField'); _checkFieldNumber(number); @@ -49,6 +56,10 @@ class UnknownFieldSet { ..groups.addAll(field.groups); } + /// Merges the field following [tag] into [this]. + /// + /// Returns `false` if the tag is the end of a group. `true` otherwise. + // TODO(sigurdm): Does not update oneofs. Consider deprecating. bool mergeFieldFromBuffer(int tag, CodedBufferReader input) { _ensureWritable('mergeFieldFromBuffer'); int number = getTagFieldNumber(tag); @@ -76,6 +87,7 @@ class UnknownFieldSet { } } + // TODO(sigurdm): Does not update oneofs. Consider deprecating. void mergeFromCodedBufferReader(CodedBufferReader input) { _ensureWritable('mergeFromCodedBufferReader'); while (true) { @@ -86,6 +98,7 @@ class UnknownFieldSet { } } + // TODO(sigurdm): Does not update oneofs. Consider deprecating. void mergeFromUnknownFieldSet(UnknownFieldSet other) { _ensureWritable('mergeFromUnknownFieldSet'); for (int key in other._fields.keys) { @@ -99,26 +112,31 @@ class UnknownFieldSet { } } + // TODO(sigurdm): Does not update oneofs. Consider deprecating. void mergeFixed32Field(int number, int value) { _ensureWritable('mergeFixed32Field'); _getField(number).addFixed32(value); } + // TODO(sigurdm): Does not update oneofs. Consider deprecating. void mergeFixed64Field(int number, Int64 value) { _ensureWritable('mergeFixed64Field'); _getField(number).addFixed64(value); } + // TODO(sigurdm): Does not update oneofs. Consider deprecating. void mergeGroupField(int number, UnknownFieldSet value) { _ensureWritable('mergeGroupField'); _getField(number).addGroup(value); } + // TODO(sigurdm): Does not update oneofs. Consider deprecating. void mergeLengthDelimitedField(int number, List value) { _ensureWritable('mergeLengthDelimitedField'); _getField(number).addLengthDelimited(value); } + // TODO(sigurdm): Does not update oneofs. Consider deprecating. void mergeVarintField(int number, Int64 value) { _ensureWritable('mergeVarintField'); _getField(number).addVarint(value); diff --git a/protobuf/pubspec.yaml b/protobuf/pubspec.yaml index ad484aaf1..4ad688c2b 100644 --- a/protobuf/pubspec.yaml +++ b/protobuf/pubspec.yaml @@ -1,6 +1,6 @@ name: protobuf -version: 1.0.1 -description: > +version: 1.0.2 +description: Runtime library for protocol buffers support. Use https://pub.dartlang.org/packages/protoc_plugin to generate dart code for your '.proto' files. homepage: https://github.com/dart-lang/protobuf diff --git a/protoc_plugin/test/unknown_field_set_test.dart b/protoc_plugin/test/unknown_field_set_test.dart index 2971df5c4..324cc184d 100755 --- a/protoc_plugin/test/unknown_field_set_test.dart +++ b/protoc_plugin/test/unknown_field_set_test.dart @@ -330,4 +330,9 @@ void main() { _checkNotEqual(f1, f2); }); + + test('GeneratedMessage.hasField sees unknown fields', () { + expect(emptyMessage.hasField(testAllTypes.getTagNumber('optionalInt32')), + true); + }); }