Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expect same message types in mergeFromMessage #661

Open
wants to merge 16 commits into
base: master
Choose a base branch
from
Open
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 53 additions & 10 deletions protobuf/lib/src/protobuf/field_set.dart
Original file line number Diff line number Diff line change
Expand Up @@ -768,14 +768,16 @@ class _FieldSet {
/// in this message. Repeated fields are appended. Singular sub-messages are
/// recursively merged.
void _mergeFromMessage(_FieldSet other) {
// TODO(https://github.com/google/protobuf.dart/issues/60): Recognize
// when `this` and [other] are the same protobuf (e.g. from cloning). In
// this case, we can merge the non-extension fields without field lookups or
// validation checks.

final sameMessage = identical(_meta, other._meta);
osa1 marked this conversation as resolved.
Show resolved Hide resolved
for (var fi in other._infosSortedByTag) {
var value = other._values[fi.index!];
if (value != null) _mergeField(fi, value, isExtension: false);
if (value != null) {
if (sameMessage) {
_mergeNonExtensionFieldUnchecked(fi, value);
} else {
_mergeField(fi, value, isExtension: false);
}
}
}
if (other._hasExtensions) {
var others = other._extensions!;
Expand All @@ -791,14 +793,55 @@ class _FieldSet {
}
}

void _mergeField(FieldInfo otherFi, fieldValue, {bool? isExtension}) {
void _mergeNonExtensionFieldUnchecked(FieldInfo fi, dynamic fieldValue) {
if (fi.isMapField) {
final mapInfo = fi as MapFieldInfo<dynamic, dynamic>;
final map =
mapInfo._ensureMapField(_meta, this) as PbMap<dynamic, dynamic>;
if (_isGroupOrMessage(mapInfo.valueFieldType)) {
for (MapEntry entry in fieldValue.entries) {
map[entry.key] = (entry.value as GeneratedMessage).deepCopy();
}
} else {
map.addAll(fieldValue);
}
return;
}

if (fi.isRepeated) {
if (_isGroupOrMessage(fi.type)) {
PbListBase<GeneratedMessage> pbList = fieldValue;
var repeatedFields = fi._ensureRepeatedField(_meta, this);
for (var i = 0; i < pbList.length; ++i) {
repeatedFields.add(pbList[i].deepCopy());
}
} else {
PbListBase pbList = fieldValue;
fi._ensureRepeatedField(_meta, this).addAll(pbList);
}
return;
}

if (fi.isGroupOrMessage) {
final currentFieldValue = _values[fi.index!];
if (currentFieldValue == null) {
fieldValue = (fieldValue as GeneratedMessage).deepCopy();
} else {
fieldValue = currentFieldValue..mergeFromMessage(fieldValue);
}
}

_setNonExtensionFieldUnchecked(_meta, fi, fieldValue);
}

void _mergeField(FieldInfo otherFi, fieldValue, {required bool isExtension}) {
final tagNumber = otherFi.tagNumber;

// Determine the FieldInfo to use.
// Don't allow regular fields to be overwritten by extensions.
final meta = _meta;
var fi = _nonExtensionInfo(meta, tagNumber);
if (fi == null && isExtension!) {
if (fi == null && isExtension) {
// This will overwrite any existing extension field info.
fi = otherFi;
}
Expand Down Expand Up @@ -836,7 +879,7 @@ class _FieldSet {
}

if (otherFi.isGroupOrMessage) {
final currentFi = isExtension!
final currentFi = isExtension
? _ensureExtensions()._getFieldOrNull(fi as Extension<dynamic>)
: _values[fi.index!];

Expand All @@ -847,7 +890,7 @@ class _FieldSet {
}
}

if (isExtension!) {
if (isExtension) {
_ensureExtensions()
._setFieldAndInfo(fi as Extension<dynamic>, fieldValue);
} else {
Expand Down