Override == and hashCode in PbMap (#224)

Author: szakarias

protobuf/CHANGELOG.md

@@ -1,3 +1,7 @@
+## 0.13.7
+
+* Override `operator ==` and `hashCode` in `PbMap` so that two `PbMap`s are equal if they have equal key/value pairs.
+
 ## 0.13.6
 
 * Fixed equality check between messages with and without extensions.

protobuf/lib/src/protobuf/coded_buffer.dart

@@ -16,7 +16,7 @@ void _writeToCodedBufferWriter(_FieldSet fs, CodedBufferWriter out) {
   }
 
   if (fs._hasExtensions) {
-    for (var tagNumber in sorted(fs._extensions._tagNumbers)) {
+    for (var tagNumber in _sorted(fs._extensions._tagNumbers)) {
       var fi = fs._extensions._getInfoOrNull(tagNumber);
       out.writeField(tagNumber, fi.type, fs._extensions._getFieldOrNull(fi));
     }

protobuf/lib/src/protobuf/field_set.dart

@@ -523,51 +523,48 @@ class _FieldSet {
   /// The hash may change when any field changes (recursively).
   /// Therefore, protobufs used as map keys shouldn't be changed.
   int get _hashCode {
-    int hash;
-
-    void hashEnumList(PbListBase enums) {
-      for (ProtobufEnum enm in enums) {
-        hash = 0x1fffffff & ((31 * hash) + enm.value);
-      }
-    }
-
     // Hashes the value of one field (recursively).
-    void hashField(FieldInfo fi, value) {
+    int hashField(int hash, FieldInfo fi, value) {
       if (value is List && value.isEmpty) {
-        return; // It's either repeated or an empty byte array.
+        return hash; // It's either repeated or an empty byte array.
       }
-      hash = 0x1fffffff & ((37 * hash) + fi.tagNumber);
+
+      hash = _HashUtils._combine(hash, fi.tagNumber);
       if (!_isEnum(fi.type)) {
-        hash = 0x1fffffff & ((53 * hash) + value.hashCode);
+        hash = _HashUtils._combine(hash, value.hashCode);
       } else if (fi.isRepeated) {
-        hashEnumList(value);
+        hash = _HashUtils._hashObjects(value.map((enm) => enm.value));
       } else {
         ProtobufEnum enm = value;
-        hash = 0x1fffffff & ((53 * hash) + enm.value);
+        hash = _HashUtils._combine(hash, enm.value);
       }
+
+      return hash;
     }
 
-    void hashEachField() {
-      for (var fi in _infosSortedByTag) {
-        var v = _values[fi.index];
-        if (v != null) hashField(fi, v);
-      }
-      if (!_hasExtensions) return;
-      for (int tagNumber in sorted(_extensions._tagNumbers)) {
+    int hashEachField(int hash) {
+      //non-extension fields
+      hash = _infosSortedByTag.where((fi) => _values[fi.index] != null).fold(
+          hash, (int h, FieldInfo fi) => hashField(h, fi, _values[fi.index]));
+
+      if (!_hasExtensions) return hash;
+
+      hash =
+          _sorted(_extensions._tagNumbers).fold(hash, (int h, int tagNumber) {
         var fi = _extensions._getInfoOrNull(tagNumber);
-        hashField(fi, _extensions._getFieldOrNull(fi));
-      }
+        return hashField(h, fi, _extensions._getFieldOrNull(fi));
+      });
+
+      return hash;
     }
 
-    // Generate hash.
-    hash = 41;
     // Hash with descriptor.
-    hash = 0x1fffffff & ((19 * hash) + _meta.hashCode);
+    int hash = _HashUtils._combine(0, _meta.hashCode);
     // Hash with fields.
-    hashEachField();
+    hash = hashEachField(hash);
     // Hash with unknown fields.
     if (_hasUnknownFields) {
-      hash = 0x1fffffff & ((29 * hash) + _unknownFields.hashCode);
+      hash = _HashUtils._combine(hash, _unknownFields.hashCode);
     }
     return hash;
   }

protobuf/lib/src/protobuf/json.dart

@@ -62,7 +62,7 @@ Map<String, dynamic> _writeToJsonMap(_FieldSet fs) {
     result['${fi.tagNumber}'] = convertToMap(value, fi.type);
   }
   if (fs._hasExtensions) {
-    for (int tagNumber in sorted(fs._extensions._tagNumbers)) {
+    for (int tagNumber in _sorted(fs._extensions._tagNumbers)) {
       var value = fs._extensions._values[tagNumber];
       if (value is List && value.isEmpty) {
         continue; // It's repeated or an empty byte array.

protobuf/lib/src/protobuf/pb_list.dart

@@ -171,17 +171,8 @@ abstract class PbListBase<E> extends ListBase<E> {
   bool operator ==(other) =>
       (other is PbListBase) && _areListsEqual(other, this);
 
-  int get hashCode {
-    int hash = 0;
-    for (final value in _wrappedList) {
-      hash = 0x1fffffff & (hash + value.hashCode);
-      hash = 0x1fffffff & (hash + ((0x0007ffff & hash) << 10));
-      hash = hash ^ (hash >> 6);
-    }
-    hash = 0x1fffffff & (hash + ((0x03ffffff & hash) << 3));
-    hash = hash ^ (hash >> 11);
-    return 0x1fffffff & (hash + ((0x00003fff & hash) << 15));
-  }
+  @override
+  int get hashCode => _HashUtils._hashObjects(_wrappedList);
 
   /// Returns an [Iterator] for the list.
   Iterator<E> get iterator => _wrappedList.iterator;

protobuf/lib/src/protobuf/pb_map.dart

@@ -39,6 +39,40 @@ class PbMap<K, V> extends MapBase<K, V> {
     _wrappedMap[key] = value;
   }
 
+  /// A [PbMap] is equal to another [PbMap] with equal key/value
+  /// pairs in any order.
+  @override
+  bool operator ==(other) {
+    if (identical(other, this)) {
+      return true;
+    }
+    if (other is! PbMap) {
+      return false;
+    }
+    if (other.length != length) {
+      return false;
+    }
+    for (final key in keys) {
+      if (!other.containsKey(key)) {
+        return false;
+      }
+    }
+    for (final key in keys) {
+      if (other[key] != this[key]) {
+        return false;
+      }
+    }
+    return true;
+  }
+
+  /// A [PbMap] is equal to another [PbMap] with equal key/value
+  /// pairs in any order. Then, the `hashCode` is guaranteed to be the same.
+  @override
+  int get hashCode {
+    return _wrappedMap.entries
+        .fold(0, (h, entry) => h ^ _HashUtils._hash2(entry.key, entry.value));
+  }
+
   @override
   void clear() {
     if (_isReadonly)

protobuf/lib/src/protobuf/unknown_field_set.dart

@@ -151,7 +151,7 @@ class UnknownFieldSet {
   String _toString(String indent) {
     var stringBuffer = StringBuffer();
 
-    for (int tag in sorted(_fields.keys)) {
+    for (int tag in _sorted(_fields.keys)) {
       var field = _fields[tag];
       for (var value in field.values) {
         if (value is UnknownFieldSet) {

protobuf/lib/src/protobuf/utils.dart

@@ -35,4 +35,32 @@ bool _areByteDataEqual(ByteData lhs, ByteData rhs) {
   return _areListsEqual(asBytes(lhs), asBytes(rhs));
 }
 
-List<T> sorted<T>(Iterable<T> list) => List.from(list)..sort();
+@Deprecated("This function was not intended to be public. "
+    "It will be removed from the public api in next major version. ")
+List<T> sorted<T>(Iterable<T> list) => new List.from(list)..sort();
+
+List<T> _sorted<T>(Iterable<T> list) => new List.from(list)..sort();
+
+class _HashUtils {
+// Jenkins hash functions copied from https://github.com/google/quiver-dart/blob/master/lib/src/core/hash.dart.
+
+  static int _combine(int hash, int value) {
+    hash = 0x1fffffff & (hash + value);
+    hash = 0x1fffffff & (hash + ((0x0007ffff & hash) << 10));
+    return hash ^ (hash >> 6);
+  }
+
+  static int _finish(int hash) {
+    hash = 0x1fffffff & (hash + ((0x03ffffff & hash) << 3));
+    hash = hash ^ (hash >> 11);
+    return 0x1fffffff & (hash + ((0x00003fff & hash) << 15));
+  }
+
+  /// Generates a hash code for multiple [objects].
+  static int _hashObjects(Iterable objects) =>
+      _finish(objects.fold(0, (h, i) => _combine(h, i.hashCode)));
+
+  /// Generates a hash code for two objects.
+  static int _hash2(a, b) =>
+      _finish(_combine(_combine(0, a.hashCode), b.hashCode));
+}

protobuf/pubspec.yaml

@@ -1,5 +1,5 @@
 name: protobuf
-version: 0.13.6
+version: 0.13.7
 author: Dart Team <misc@dartlang.org>
 description: >
   Runtime library for protocol buffers support.

protoc_plugin/CHANGELOG.md

@@ -1,4 +1,5 @@
 ## 16.0.3
+
 * Sync Kythe metadata updates from internal repo:
 * Remove the extra 1 (field name) from generated field paths,
   so they refer to the whole field rather than the name.

protoc_plugin/pubspec.yaml

@@ -1,5 +1,5 @@
 name: protoc_plugin
-version: 16.0.3-dev-1
+version: 16.0.3-dev-2
 author: Dart Team <misc@dartlang.org>
 description: Protoc compiler plugin to generate Dart code
 homepage: https://github.com/dart-lang/protobuf
@@ -10,7 +10,7 @@ environment:
 dependencies:
   fixnum: ^0.10.5
   path: ^1.0.0
-  protobuf: ^0.13.6
+  protobuf: ^0.13.7
   dart_style: ^1.0.6
 
 dev_dependencies:

protoc_plugin/test/map_field_test.dart

@@ -7,6 +7,7 @@ library map_field_test;
 
 import 'dart:convert';
 
+import 'package:protobuf/protobuf.dart';
 import 'package:test/test.dart';
 
 import '../out/protos/map_field.pb.dart';
@@ -212,6 +213,33 @@ void main() {
     _expectEmpty(testMap);
   });
 
+  test(
+      'PbMap` is equal to another PbMap with equal key/value pairs in any order',
+      () {
+    TestMap t = TestMap()
+      ..int32ToStringField[2] = 'test2'
+      ..int32ToStringField[1] = 'test';
+    TestMap t2 = TestMap()
+      ..int32ToStringField[1] = 'test'
+      ..int32ToStringField[2] = 'test2';
+    TestMap t3 = TestMap()..int32ToStringField[1] = 'test';
+
+    PbMap<int, String> m = t.int32ToStringField;
+    PbMap<int, String> m2 = t2.int32ToStringField;
+    PbMap<int, String> m3 = t3.int32ToStringField;
+
+    expect(t, t2);
+    expect(t.hashCode, t2.hashCode);
+
+    expect(m, m2);
+    expect(m == m2, isTrue);
+    expect(m.hashCode, m2.hashCode);
+
+    expect(m, isNot(m3));
+    expect(m == m3, isFalse);
+    expect(m.hashCode, isNot(m3.hashCode));
+  });
+
   test('merge from other message', () {
     TestMap testMap = TestMap();
     _setValues(testMap);