Fix issue with not being able to read extensions after freeze (flutte…

…r#186)

protobuf/CHANGELOG.md

@@ -1,3 +1,7 @@
+## 0.13.0
+
+* Breaking change: Fix issue with not being able to read extensions after freezing.
+
 ## 0.12.0
 
 * Breaking change: Changed `BuilderInfo.m()` to take class and package name of the protobuf message representing the map

protobuf/lib/src/protobuf/extension_field_set.dart

@@ -8,16 +8,14 @@ class _ExtensionFieldSet {
   final _FieldSet _parent;
   final Map<int, Extension> _info = <int, Extension>{};
   final Map<int, dynamic> _values = <int, dynamic>{};
+  bool _isReadOnly = false;
 
-  _ExtensionFieldSet(this._parent) {
-    // Read-only fieldsets shouldn't have extension fields.
-    assert(!_parent._isReadOnly);
-  }
+  _ExtensionFieldSet(this._parent);
 
   Extension _getInfoOrNull(int tagNumber) => _info[tagNumber];
 
   _getFieldOrDefault(Extension fi) {
-    if (fi.isRepeated) return _ensureRepeatedField(fi);
+    if (fi.isRepeated) return _getList(fi);
     _validateInfo(fi);
     // TODO(skybrian) seems unnecessary to add info?
     // I think this was originally here for repeated extensions.
@@ -39,13 +37,24 @@ class _ExtensionFieldSet {
   /// If it doesn't exist, creates the list and saves the extension.
   /// Suitable for public API and decoders.
   List<T> _ensureRepeatedField<T>(Extension<T> fi) {
+    assert(!_isReadOnly);
     assert(fi.isRepeated);
     assert(fi.extendee == _parent._messageName);
 
     var list = _values[fi.tagNumber];
     if (list != null) return list as List<T>;
 
-    // Add info and create list.
+    return _addInfoAndCreateList(fi);
+  }
+
+  List<T> _getList<T>(Extension<T> fi) {
+    var value = _values[fi.tagNumber];
+    if (value != null) return value as List<T>;
+    if (_isReadOnly) return new List<T>.unmodifiable(const []);
+    return _addInfoAndCreateList(fi);
+  }
+
+  List _addInfoAndCreateList(Extension fi) {
     _validateInfo(fi);
     var newList = fi._createRepeatedField(_parent._message);
     _addInfoUnchecked(fi);
@@ -61,6 +70,7 @@ class _ExtensionFieldSet {
   }
 
   void _clearField(Extension fi) {
+    _ensureWritable();
     _validateInfo(fi);
     if (_parent._hasObservers) _parent._eventPlugin.beforeClearField(fi);
     _values.remove(fi.tagNumber);
@@ -78,23 +88,30 @@ class _ExtensionFieldSet {
       throw new ArgumentError(_parent._setFieldFailedMessage(
           fi, value, 'repeating field (use get + .add())'));
     }
+    _ensureWritable();
     _parent._validateField(fi, value);
     _setFieldUnchecked(fi, value);
   }
 
   /// Sets a non-repeated value and extension.
   /// Overwrites any existing extension.
   void _setFieldAndInfo(Extension fi, value) {
+    _ensureWritable();
     if (fi.isRepeated) {
       throw new ArgumentError(_parent._setFieldFailedMessage(
           fi, value, 'repeating field (use get + .add())'));
     }
+    _ensureWritable();
     _validateInfo(fi);
     _parent._validateField(fi, value);
     _addInfoUnchecked(fi);
     _setFieldUnchecked(fi, value);
   }
 
+  void _ensureWritable() {
+    if (_isReadOnly) frozenMessageModificationHandler(_parent._messageName);
+  }
+
   void _validateInfo(Extension fi) {
     if (fi.extendee != _parent._messageName) {
       throw new ArgumentError(
@@ -138,11 +155,33 @@ class _ExtensionFieldSet {
       final value = original._getFieldOrNull(extension);
       if (value == null) continue;
       if (extension.isRepeated) {
-        assert(value is PbList);
+        assert(value is PbListBase);
         _ensureRepeatedField(extension)..addAll(value);
       } else {
         _setFieldUnchecked(extension, value);
       }
     }
   }
+
+  void _markReadOnly() {
+    if (_isReadOnly) return;
+    _isReadOnly = true;
+    for (Extension field in _info.values) {
+      if (field.isRepeated) {
+        final entries = _values[field.tagNumber];
+        if (entries == null) continue;
+        if (field.isGroupOrMessage) {
+          for (var subMessage in entries as List<GeneratedMessage>) {
+            subMessage.freeze();
+          }
+        }
+        _values[field.tagNumber] = entries.toFrozenPbList();
+      } else if (field.isGroupOrMessage) {
+        final entry = _values[field.tagNumber];
+        if (entry != null) {
+          (entry as GeneratedMessage).freeze();
+        }
+      }
+    }
+  }
 }

protobuf/lib/src/protobuf/field_set.dart

@@ -83,7 +83,6 @@ class _FieldSet {
   bool get hasUnknownFields => _unknownFields != null;
 
   _ExtensionFieldSet _ensureExtensions() {
-    _ensureWritable();
     if (!_hasExtensions) _extensions = new _ExtensionFieldSet(this);
     return _extensions;
   }
@@ -142,6 +141,9 @@ class _FieldSet {
         }
       }
     }
+    if (_hasExtensions) {
+      _ensureExtensions()._markReadOnly();
+    }
   }
 
   void _ensureWritable() {

protobuf/lib/src/protobuf/generated_message.dart

@@ -250,7 +250,6 @@ abstract class GeneratedMessage {
   ///
   /// If not set, returns the extension's default value.
   getExtension(Extension extension) {
-    if (_fieldSet._isReadOnly) return extension.readonlyDefault;
     return _fieldSet._ensureExtensions()._getFieldOrDefault(extension);
   }
 

protobuf/pubspec.yaml

@@ -1,5 +1,5 @@
 name: protobuf
-version: 0.12.0
+version: 0.13.0
 author: Dart Team <[email protected]>
 description: >
   Runtime library for protocol buffers support.

protoc_plugin/test/to_builder_test.dart

@@ -13,18 +13,22 @@ main() {
       expect(original.getExtension(FooExt.inner).value, 'extension');
       expect(
           original.getExtension(FooExt.inners)[0].value, 'repeatedExtension');
-    }, skip: 'https://github.com/dart-lang/protobuf/issues/171');
+    });
 
     test('frozen message cannot be modified', () {
       expect(() => original.inner = (Inner()..value = 'bar'),
           throwsA(TypeMatcher<UnsupportedError>()));
       expect(() => original.inner..value = 'bar',
           throwsA(TypeMatcher<UnsupportedError>()));
+      expect(() => original.inners.add(Inner()..value = 'bar'),
+          throwsA(TypeMatcher<UnsupportedError>()));
     });
 
     test('extensions cannot be modified', () {
       expect(() => original.setExtension(FooExt.inner, Inner()..value = 'bar'),
           throwsA(TypeMatcher<UnsupportedError>()));
+      expect(() => original.getExtension(FooExt.inner).value = 'bar',
+          throwsA(TypeMatcher<UnsupportedError>()));
       expect(
           () =>
               original.getExtension(FooExt.inners).add(Inner()..value = 'bar'),
@@ -38,7 +42,7 @@ main() {
     test('builder extensions are also copied shallowly', () {
       expect(builder.getExtension(FooExt.inner),
           same(original.getExtension(FooExt.inner)));
-    }, skip: 'https://github.com/dart-lang/protobuf/issues/171');
+    });
 
     test('repeated fields are cloned', () {
       expect(builder.inners, isNot(same(original.inners)));
@@ -50,7 +54,7 @@ main() {
           isNot(same(original.getExtension(FooExt.inners))));
       expect(builder.getExtension(FooExt.inners)[0],
           same(original.getExtension(FooExt.inners)[0]));
-    }, skip: 'https://github.com/dart-lang/protobuf/issues/171');
+    });
 
     test(
         'the builder is only a shallow copy, the nested message is still frozen.',