Remove frozen message modification handling (#643)

See #175 for the problems with this API.

Fixes #175

protobuf/CHANGELOG.md

@@ -1,9 +1,12 @@
 ## 3.0.0-dev
 
 * `ReadonlyMessageMixin` removed ([#183], [#644])
+* `frozenMessageModificationHandler` removed ([#175], [#643])
 
 [#183]: https://github.com/google/protobuf.dart/issues/183
 [#644]: https://github.com/google/protobuf.dart/pull/644
+[#175]: https://github.com/google/protobuf.dart/issues/175
+[#643]: https://github.com/google/protobuf.dart/pull/643
 
 ## 2.1.0
 

protobuf/lib/src/protobuf/extension_field_set.dart

@@ -113,7 +113,9 @@ class _ExtensionFieldSet {
   }
 
   void _ensureWritable() {
-    if (_isReadOnly) frozenMessageModificationHandler(_parent._messageName);
+    if (_isReadOnly) {
+      _throwFrozenMessageModificationError(_parent._messageName);
+    }
   }
 
   void _validateInfo(Extension fi) {

protobuf/lib/src/protobuf/field_set.dart

@@ -4,10 +4,7 @@
 
 part of protobuf;
 
-typedef FrozenMessageErrorHandler = void Function(String messageName,
-    [String? methodName]);
-
-void defaultFrozenMessageModificationHandler(String messageName,
+void _throwFrozenMessageModificationError(String messageName,
     [String? methodName]) {
   if (methodName != null) {
     throw UnsupportedError(
@@ -17,31 +14,6 @@ void defaultFrozenMessageModificationHandler(String messageName,
       'Attempted to change a read-only message ($messageName)');
 }
 
-/// Invoked when an attempt is made to modify a frozen message.
-///
-/// This handler can log the attempt, throw an exception, or ignore the attempt
-/// altogether.
-///
-/// If the handler returns normally, the modification is allowed, and execution
-/// proceeds as if the message was writable.
-FrozenMessageErrorHandler _frozenMessageModificationHandler =
-    defaultFrozenMessageModificationHandler;
-FrozenMessageErrorHandler get frozenMessageModificationHandler =>
-    _frozenMessageModificationHandler;
-set frozenMessageModificationHandler(FrozenMessageErrorHandler value) {
-  _hashCodesCanBeMemoized = false;
-  _frozenMessageModificationHandler = value;
-}
-
-/// Indicator for whether the FieldSet hashCodes can be memoized.
-///
-/// HashCode memoization relies on the [defaultFrozenMessageModificationHandler]
-/// behavior--that is, after freezing, field set values can't ever be changed.
-/// This keeps track of whether an application has ever modified the
-/// [FrozenMessageErrorHandler] used, not allowing hashCodes to be memoized if
-/// it ever changed.
-bool _hashCodesCanBeMemoized = true;
-
 /// All the data in a GeneratedMessage.
 ///
 /// These fields and methods are in a separate class to avoid
@@ -204,7 +176,9 @@ class _FieldSet {
   }
 
   void _ensureWritable() {
-    if (_isReadOnly) frozenMessageModificationHandler(_messageName);
+    if (_isReadOnly) {
+      _throwFrozenMessageModificationError(_messageName);
+    }
   }
 
   // Single-field operations
@@ -645,11 +619,10 @@ class _FieldSet {
   /// The hash may change when any field changes (recursively).
   /// Therefore, protobufs used as map keys shouldn't be changed.
   ///
-  /// If the protobuf contents have been frozen, and the
-  /// [FrozenMessageErrorHandler] has not been changed from the default
-  /// behavior, the hashCode can be memoized to speed up performance.
+  /// If the protobuf contents have been frozen the hashCode is memoized to
+  /// speed up performance.
   int get _hashCode {
-    if (_hashCodesCanBeMemoized && _memoizedHashCode != null) {
+    if (_memoizedHashCode != null) {
       return _memoizedHashCode!;
     }
 
@@ -677,7 +650,7 @@ class _FieldSet {
     // Hash with unknown fields.
     hash = _HashUtils._combine(hash, _unknownFields?.hashCode ?? 0);
 
-    if (_isReadOnly && _hashCodesCanBeMemoized) {
+    if (_isReadOnly) {
       _frozenState = hash;
     }
     return hash;

protobuf/lib/src/protobuf/unknown_field_set.dart

@@ -191,7 +191,7 @@ class UnknownFieldSet {
 
   void _ensureWritable(String methodName) {
     if (_isReadOnly) {
-      frozenMessageModificationHandler('UnknownFieldSet', methodName);
+      _throwFrozenMessageModificationError('UnknownFieldSet', methodName);
     }
   }
 }

protobuf/test/readonly_message_test.dart

@@ -6,13 +6,7 @@
 library readonly_message_test;
 
 import 'package:protobuf/protobuf.dart'
-    show
-        BuilderInfo,
-        GeneratedMessage,
-        PbFieldType,
-        UnknownFieldSetField,
-        frozenMessageModificationHandler,
-        defaultFrozenMessageModificationHandler;
+    show BuilderInfo, GeneratedMessage, PbFieldType, UnknownFieldSetField;
 import 'package:test/test.dart';
 
 Matcher throwsError(Type expectedType, Matcher expectedMessage) =>
@@ -76,42 +70,6 @@ void main() {
             equals('Attempted to change a read-only message (rec)')));
   });
 
-  test('can set a field on a read-only message with a custom read-only handler',
-      () {
-    try {
-      var called = 0;
-
-      frozenMessageModificationHandler =
-          (String messageName, [String? methodName]) {
-        expect(messageName, 'rec');
-        expect(methodName, isNull);
-        called++;
-      };
-
-      Rec.getDefault().setField(1, 456);
-      expect(called, 1);
-    } finally {
-      frozenMessageModificationHandler =
-          defaultFrozenMessageModificationHandler;
-    }
-  });
-
-  test('eagerly computes hashCode if a custom read-only handler is used', () {
-    try {
-      final message = Rec.getDefault();
-      final initialHashCode = message.hashCode;
-
-      frozenMessageModificationHandler =
-          (String messageName, [String? methodName]) {};
-      message.setField(1, 456);
-      final modifiedHashCode = message.hashCode;
-      expect(initialHashCode == modifiedHashCode, isFalse);
-    } finally {
-      frozenMessageModificationHandler =
-          defaultFrozenMessageModificationHandler;
-    }
-  });
-
   test("can't clear a read-only message", () {
     expect(
         () => Rec.getDefault().clear(),