fix inconsistent firing of events for extension fields

lib/src/protobuf/generated_message.dart

 // Copyright (c) 2011, 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.
 
 part of protobuf;
 
 typedef GeneratedMessage CreateBuilderFunc();
 typedef Object MakeDefaultFunc();
 typedef ProtobufEnum ValueOfFunc(int value);
 
@@ skipping 135 matching lines @@
 
   /// Clears all data that was set in this message.
   ///
   /// After calling [clear], [getField] will still return default values for
   /// unset fields.
   void clear() {
     unknownFields.clear();
 
     if (_hasObservers) {
       for (int key in _fieldValues.keys) {
-        eventPlugin.beforeClearField(key);
+        if (info_.containsTagNumber(key)) {
+          eventPlugin.beforeClearField(key);
+        }
       }
     }
     _fieldValues.clear();
   }
 
   // TODO(antonm): move to getters.
   int getTagNumber(String fieldName) => info_.tagNumber(fieldName);
 
   bool operator ==(other) {
     if (other is! GeneratedMessage) return false;
@@ skipping 150 matching lines @@
 
     while (true) {
       int tag = input.readTag();
       if (tag == 0) {
         return;
       }
       int wireType = tag & 0x7;
       int tagNumber = tag >> 3;
       int fieldType = -1;
 
+      Extension extension;
       if (info_.containsTagNumber(tagNumber)) {
         fieldType = info_.fieldType(tagNumber);
       } else {
-        Extension extension = extensionRegistry
+        extension = extensionRegistry
             .getExtension(info_.messageName, tagNumber);
         if (extension != null) {
           _addExtensionToMap(extension);
           fieldType = extension.type;
         }
       }
       if (fieldType == -1 || !_wireTypeMatches(fieldType, wireType)) {
         if (!unknownFields.mergeFieldFromBuffer(tag, input)) {
           return;
         } else {
           continue;
         }
       }
 
       // Ignore required/optional packed/unpacked.
       fieldType &= ~(FieldType._PACKED_BIT | FieldType._REQUIRED_BIT);
       switch (fieldType) {
         case FieldType._OPTIONAL_BOOL:
-          _setField(tagNumber, input.readBool());
+          _setField(tagNumber, input.readBool(), extension);
           break;
         case FieldType._OPTIONAL_BYTES:
-          _setField(tagNumber, input.readBytes());
+          _setField(tagNumber, input.readBytes(), extension);
           break;
         case FieldType._OPTIONAL_STRING:
-          _setField(tagNumber, input.readString());
+          _setField(tagNumber, input.readString(), extension);
           break;
         case FieldType._OPTIONAL_FLOAT:
-          _setField(tagNumber, input.readFloat());
+          _setField(tagNumber, input.readFloat(), extension);
           break;
         case FieldType._OPTIONAL_DOUBLE:
-          _setField(tagNumber, input.readDouble());
+          _setField(tagNumber, input.readDouble(), extension);
           break;
         case FieldType._OPTIONAL_ENUM:
           int rawValue = input.readEnum();
           var value = _getValueOfFunc(tagNumber, extensionRegistry)(rawValue);
           if (value == null) {
             unknownFields.mergeVarintField(tagNumber, new Int64(rawValue));
           } else {
-            _setField(tagNumber, value);
+            _setField(tagNumber, value, extension);
           }
           break;
         case FieldType._OPTIONAL_GROUP:
           GeneratedMessage subMessage =
               _getEmptyMessage(tagNumber, extensionRegistry);
           if (_fieldValues.containsKey(tagNumber)) {
             subMessage.mergeFromMessage(getField(tagNumber));
           }
           input.readGroup(tagNumber, subMessage, extensionRegistry);
-          _setField(tagNumber, subMessage);
+          _setField(tagNumber, subMessage, extension);
           break;
         case FieldType._OPTIONAL_INT32:
-          _setField(tagNumber, input.readInt32());
+          _setField(tagNumber, input.readInt32(), extension);
           break;
         case FieldType._OPTIONAL_INT64:
-          _setField(tagNumber, input.readInt64());
+          _setField(tagNumber, input.readInt64(), extension);
           break;
         case FieldType._OPTIONAL_SINT32:
-          _setField(tagNumber, input.readSint32());
+          _setField(tagNumber, input.readSint32(), extension);
           break;
         case FieldType._OPTIONAL_SINT64:
-          _setField(tagNumber, input.readSint64());
+          _setField(tagNumber, input.readSint64(), extension);
           break;
         case FieldType._OPTIONAL_UINT32:
-          _setField(tagNumber, input.readUint32());
+          _setField(tagNumber, input.readUint32(), extension);
           break;
         case FieldType._OPTIONAL_UINT64:
-          _setField(tagNumber, input.readUint64());
+          _setField(tagNumber, input.readUint64(), extension);
           break;
         case FieldType._OPTIONAL_FIXED32:
-          _setField(tagNumber, input.readFixed32());
+          _setField(tagNumber, input.readFixed32(), extension);
           break;
         case FieldType._OPTIONAL_FIXED64:
-          _setField(tagNumber, input.readFixed64());
+          _setField(tagNumber, input.readFixed64(), extension);
           break;
         case FieldType._OPTIONAL_SFIXED32:
-          _setField(tagNumber, input.readSfixed32());
+          _setField(tagNumber, input.readSfixed32(), extension);
           break;
         case FieldType._OPTIONAL_SFIXED64:
-          _setField(tagNumber, input.readSfixed64());
+          _setField(tagNumber, input.readSfixed64(), extension);
           break;
         case FieldType._OPTIONAL_MESSAGE:
           GeneratedMessage subMessage =
               _getEmptyMessage(tagNumber, extensionRegistry);
           if (_fieldValues.containsKey(tagNumber)) {
             subMessage.mergeFromMessage(getField(tagNumber));
           }
           input.readMessage(subMessage, extensionRegistry);
-          _setField(tagNumber, subMessage);
+          _setField(tagNumber, subMessage, extension);
           break;
         case FieldType._REPEATED_BOOL:
           readPackable(wireType, tagNumber, input.readBool);
           break;
         case FieldType._REPEATED_BYTES:
           appendToRepeated(tagNumber, input.readBytes());
           break;
         case FieldType._REPEATED_STRING:
           appendToRepeated(tagNumber, input.readString());
           break;
@@ skipping 179 matching lines @@
       }
       if (_isRepeated(fieldType)) {
         List thisList = getField(tagNumber);
         for (var value in fieldValue) {
           thisList.add(_convertJsonValue(value, tagNumber, fieldType,
                                          extensionRegistry));
         }
       } else {
         var value = _convertJsonValue(fieldValue, tagNumber, fieldType,
             extensionRegistry);
-        setField(tagNumber, value, fieldType);
+        _validate(tagNumber, fieldType, value);
+        _setField(tagNumber, value, extension);
       }
     }
   }
 
   _convertJsonValue(value, int tagNumber, int fieldType,
                    ExtensionRegistry extensionRegistry) {
     String expectedType; // for exception message
     switch (FieldType._baseType(fieldType)) {
     case FieldType._BOOL_BIT:
       if (value is bool) {
@@ skipping 117 matching lines @@
     if (!_isRepeated(extension.type)) {
       throw new ArgumentError(
           'Cannot add to a non-repeated field (use setExtension())');
     }
     // Validate type and range.
     _validate(extension.tagNumber, extension.type, value);
 
     var list = _fieldValues[extension.tagNumber];
     if (list == null) {
       list = extension.makeDefault();
-      _setExtension(extension, list);
+      _addExtensionToMap(extension);
+      _setField(extension.tagNumber, list, extension);
     }
 
     list.add(value);
   }
 
   void clearExtension(Extension extension) {
     _checkExtension(extension);
     _fieldValues.remove(extension.tagNumber);
     var value = extension.makeDefault();
     if (value is List) {
-      _setExtension(extension, value);
+      _addExtensionToMap(extension);
+      _setField(extension.tagNumber, value, extension);
     } else {
       _extensions.remove(extension.tagNumber);
     }
   }
 
   /// Clears the contents of a given field.
   void clearField(int tagNumber) {
     if (_hasObservers) {
-      eventPlugin.beforeClearField(tagNumber);
+      if (info_.containsTagNumber(tagNumber)) {
+        eventPlugin.beforeClearField(tagNumber);
+      }
     }
     _fieldValues.remove(tagNumber);
   }
 
   bool extensionsAreInitialized() {
     return _extensions.keys.every((int tagNumber) {
       return info_._isFieldInitialized(_fieldValues, tagNumber,
           _extensions[tagNumber].type);
     });
   }
 
   /// Returns the value of [extension].
   ///
   /// For repeated fields that have not been set previously, [:null:] is
   /// returned.
   getExtension(Extension extension) {
     _checkExtension(extension);
     _addExtensionToMap(extension);
     return getField(extension.tagNumber);
   }
 
   /// Returns the value of the field associated with [tagNumber], or the
   /// default value if it is not set.
   getField(int tagNumber) {
     var value = _fieldValues[tagNumber];
     if (value != null) return value;
 
+    // Find the default function
+    MakeDefaultFunc f = info_.makeDefault(tagNumber);
+
+    Extension extension;
+    if (f == null) {
+      var extension = _extensions[tagNumber];
+      if (extension == null) {
+        throw new ArgumentError(
+            "tag $tagNumber not defined in ${info_.messageName}");
+      }
+      f = extension.makeDefault;
+    }
+
     // Initialize the field.
-    value = _callDefaultFunction(tagNumber);
+    value = f();
     if (value is List) {
-      return _getDefaultRepeatedField(tagNumber, value);
+      return _getDefaultRepeatedField(tagNumber, value, extension);
     }
     return value;
   }
 
-  List _getDefaultRepeatedField(int tagNumber, List value) {
+  List _getDefaultRepeatedField(int tagNumber, List value, Extension ext) {
     // Automatically save the repeated field so that changes won't be lost.
     //
     // TODO(skybrian) we could avoid this by generating another
     // method for repeated fields:
     //
     //   msg.mutableFoo().add(123);
     //
     // Then msg.foo could return an immutable empty list by default.
     // But it doesn't seem urgent or worth the migration.
-    _setField(tagNumber, value);
+    _setField(tagNumber, value, ext);
     return value;
   }
 
   /// Returns the value of a field, ignoring any defaults.
   ///
   /// For unset or cleared fields, returns null.
   /// Also returns null for unknown tag numbers.
   getFieldOrNull(int tagNumber) => _fieldValues[tagNumber];
 
   /// Returns the default value for the given field.
@@ skipping 38 matching lines @@
 
   /// Merges the contents of the [other] into this message.
   ///
   /// Singular fields that are set in [other] overwrite the corresponding fields
   /// in this message. Repeated fields are appended. Singular sub-messages are
   /// recursively merged.
   void mergeFromMessage(GeneratedMessage other) {
     for (int tagNumber in other._fieldValues.keys) {
       var fieldValue = other._fieldValues[tagNumber];
 
-      if (other._extensions.containsKey(tagNumber)) {
-        _addExtensionToMap(other._extensions[tagNumber]);
+      var extension = other._extensions[tagNumber];
+      if (extension != null) {
+        _addExtensionToMap(extension);
       }
       int fieldType = other._getFieldType(tagNumber);
       var cloner = (x) => x;
-      if ((fieldType & (FieldType._GROUP_BIT | FieldType._MESSAGE_BIT)) != 0) {
+      if (_isGroupOrMessage(fieldType)) {
         cloner = (message) => message.clone();
       }
       if (_isRepeated(fieldType)) {
         getField(tagNumber).addAll(new List.from(fieldValue).map(cloner));
       } else {
-        setField(tagNumber, cloner(fieldValue), fieldType);
+        fieldValue = cloner(fieldValue);
+        _validate(tagNumber, fieldType, fieldValue);
+        _setField(tagNumber, fieldValue, extension);
       }
     }
 
     mergeUnknownFields(other.unknownFields);
   }
 
   void mergeUnknownFields(UnknownFieldSet unknownFieldSet) {
     unknownFields.mergeFromUnknownFieldSet(unknownFieldSet);
   }
 
   /// Sets the value of a non-repeated extension field to [value].
   void setExtension(Extension extension, value) {
+    if (value == null) {
+      throw new ArgumentError('value is null');
+    } else if (_isRepeated(extension.type)) {
+      throw new ArgumentError(_generateMessage(tagNumber, value,
+          'repeating field (use get + .add())'));
+    }
     _checkExtension(extension);
     _addExtensionToMap(extension);
-    setField(extension.tagNumber, value, extension.type);
+    _validate(extension.tagNumber, extension.type, value);
+    _setField(extension.tagNumber, value, extension);
   }
 
-  /// Sets the value of a given field by its [tagNumber].
+  /// Sets the value of a non-extension field by its [tagNumber].
   ///
   /// Throws an [:ArgumentError:] if [value] does not match the type
   /// associated with [tagNumber].
   ///
   /// Throws an [:ArgumentError:] if [value] is [:null:]. To clear a field of
   /// it's current value, use [clearField] instead.
-  void setField(int tagNumber, value, [int fieldType = null]) {
-    if (value == null) {
-      throw new ArgumentError('value is null');
+  void setField(int tagNumber, value) {

[Søren Gjesse, 2015/08/10 13:37:19]

This is a breaking change. Is the pubspec.yaml updated with a new version? Maybe
add information to the CHANGELOG.

[skybrian, 2015/08/10 18:26:12]

Updated CHANGELOG. I will bump the version number before release.

+    if (value == null) throw new ArgumentError('value is null');
+
+    if (!info_.containsTagNumber(tagNumber)) {
+      throw new ArgumentError('Unknown tag: $tagNumber');
     }
-    if (fieldType == null) {
-      if (!info_.containsTagNumber(tagNumber)) {
-        throw new ArgumentError('Unknown tag: $tagNumber');
-      }
-      fieldType = info_.fieldType(tagNumber);
-    }
+    var fieldType = info_.fieldType(tagNumber);
     if (_isRepeated(fieldType)) {
       throw new ArgumentError(_generateMessage(tagNumber, value,
           'repeating field (use get + .add())'));
     }
 
     // Validate type and range.
     _validate(tagNumber, fieldType, value);
-
-    _setField(tagNumber, value);
+    _setField(tagNumber, value, null);
   }
 
-  void _setField(int tagNumber, value) {
-    if (_hasObservers) {
+  void _setField(int tagNumber, value, Extension extension) {
+    if (_hasObservers && extension == null) {
       eventPlugin.beforeSetField(tagNumber, value);
     }
     _fieldValues[tagNumber] = value;
   }
 
   void _addExtensionToMap(Extension extension) {
     _extensions[extension.tagNumber] = extension;
   }
 
   void _checkExtension(Extension extension) {
@@ skipping 29 matching lines @@
   ValueOfFunc _getValueOfFunc(int tagNumber,
       ExtensionRegistry extensionRegistry) {
     ValueOfFunc valueOfFunc = info_.valueOfFunc(tagNumber);
     if (valueOfFunc == null && extensionRegistry != null) {
       valueOfFunc = extensionRegistry.getExtension(info_.messageName,
         tagNumber).valueOf;
     }
     return valueOfFunc;
   }
 
-  // Does not perform validation.
-  void _setExtension(Extension extension, var value) {
-    _addExtensionToMap(extension);
-    _fieldValues[extension.tagNumber] = value;
-  }
-
   String _generateMessage(int tagNumber, var value, String detail) {
     String fieldName;
     if (_extensions[tagNumber] != null) {
       fieldName = _extensions[tagNumber].name;
     } else {
       fieldName = info_.fieldName(tagNumber);
     }
     String messageType = info_.messageName;
     return 'Illegal to set field $fieldName ($tagNumber) of $messageType'
         ' to value ($value): $detail';
@@ skipping 148 matching lines @@
               _generateMessage(tagNumber, value, 'not a GeneratedMessage'));
         }
         break;
       default:
         throw new ArgumentError(
             _generateMessage(tagNumber, value, 'field has unknown type '
             '$fieldType'));
     }
   }
 }