From 009056dc376633cca5b35918564c1733510eab22 Mon Sep 17 00:00:00 2001 From: Simon Binder Date: Thu, 1 Sep 2022 18:11:41 +0200 Subject: [PATCH] Handle equality and hashes for blobs --- drift/lib/drift.dart | 6 ++ drift/test/generated/todos.g.dart | 7 +- drift_dev/lib/src/model/types.dart | 3 + .../src/writer/queries/result_set_writer.dart | 20 +++--- .../src/writer/tables/data_class_writer.dart | 13 +++- .../lib/src/writer/utils/hash_and_equals.dart | 72 +++++++++++++++++++ drift_dev/lib/src/writer/utils/hash_code.dart | 28 -------- .../lib/src/writer/utils/override_equals.dart | 18 ----- drift_dev/lib/writer.dart | 3 +- .../test/writer/utils/hash_code_test.dart | 28 +++++--- .../writer/utils/override_equals_test.dart | 11 ++- 11 files changed, 134 insertions(+), 75 deletions(-) create mode 100644 drift_dev/lib/src/writer/utils/hash_and_equals.dart delete mode 100644 drift_dev/lib/src/writer/utils/hash_code.dart delete mode 100644 drift_dev/lib/src/writer/utils/override_equals.dart diff --git a/drift/lib/drift.dart b/drift/lib/drift.dart index 9a94fe16..c1d4584c 100644 --- a/drift/lib/drift.dart +++ b/drift/lib/drift.dart @@ -1,5 +1,7 @@ library drift; +import 'package:collection/collection.dart'; + // needed for the generated code that generates data classes with an Uint8List // field. export 'dart:typed_data' show Uint8List; @@ -17,3 +19,7 @@ export 'src/runtime/query_builder/query_builder.dart'; export 'src/runtime/types/converters.dart'; export 'src/runtime/types/mapping.dart'; export 'src/utils/lazy_database.dart'; + +/// A [ListEquality] instance used by generated drift code for the `==` and +/// [Object.hashCode] implementation of generated classes if they contain lists. +const ListEquality $driftBlobEquality = ListEquality(); diff --git a/drift/test/generated/todos.g.dart b/drift/test/generated/todos.g.dart index d3c11ebc..05b88841 100644 --- a/drift/test/generated/todos.g.dart +++ b/drift/test/generated/todos.g.dart @@ -650,8 +650,8 @@ class User extends DataClass implements Insertable { } @override - int get hashCode => - Object.hash(id, name, isAwesome, profilePicture, creationTime); + int get hashCode => Object.hash(id, name, isAwesome, + $driftBlobEquality.hash(profilePicture), creationTime); @override bool operator ==(Object other) => identical(this, other) || @@ -659,7 +659,8 @@ class User extends DataClass implements Insertable { other.id == this.id && other.name == this.name && other.isAwesome == this.isAwesome && - other.profilePicture == this.profilePicture && + $driftBlobEquality.equals( + other.profilePicture, this.profilePicture) && other.creationTime == this.creationTime); } diff --git a/drift_dev/lib/src/model/types.dart b/drift_dev/lib/src/model/types.dart index ff2227ef..b9672154 100644 --- a/drift_dev/lib/src/model/types.dart +++ b/drift_dev/lib/src/model/types.dart @@ -63,6 +63,9 @@ class DriftDartType { } extension OperationOnTypes on HasType { + bool get isUint8ListInDart => + type == DriftSqlType.blob && typeConverter == null; + /// Whether this type is nullable in Dart bool get nullableInDart { if (isArray) return false; // Is a List in Dart, not nullable diff --git a/drift_dev/lib/src/writer/queries/result_set_writer.dart b/drift_dev/lib/src/writer/queries/result_set_writer.dart index 800e7c26..d09ae5f4 100644 --- a/drift_dev/lib/src/writer/queries/result_set_writer.dart +++ b/drift_dev/lib/src/writer/queries/result_set_writer.dart @@ -12,7 +12,7 @@ class ResultSetWriter { void write() { final className = query.resultClassName; - final fieldNames = []; + final fields = []; final nonNullableFields = {}; final into = scope.leaf(); @@ -34,7 +34,7 @@ class ResultSetWriter { into.write('$modifier $runtimeType $name\n;'); - fieldNames.add(name); + fields.add(EqualityField(name, isList: column.isUint8ListInDart)); if (!column.nullable) nonNullableFields.add(name); } @@ -49,7 +49,7 @@ class ResultSetWriter { into.write('$modifier $typeName $fieldName;\n'); - fieldNames.add(fieldName); + fields.add(EqualityField(fieldName)); if (!nested.isNullable) nonNullableFields.add(fieldName); } else if (nested is NestedResultQuery) { final fieldName = nested.filedName(); @@ -61,7 +61,7 @@ class ResultSetWriter { into.write('$modifier List<$typeName> $fieldName;\n'); - fieldNames.add(fieldName); + fields.add(EqualityField(fieldName)); nonNullableFields.add(fieldName); } } @@ -73,11 +73,11 @@ class ResultSetWriter { into.write('$className({'); } - for (final column in fieldNames) { - if (nonNullableFields.contains(column)) { + for (final column in fields) { + if (nonNullableFields.contains(column.lexeme)) { into.write('required '); } - into.write('this.$column,'); + into.write('this.${column.lexeme},'); } if (scope.options.rawResultSetData) { @@ -89,11 +89,11 @@ class ResultSetWriter { // if requested, override hashCode and equals if (scope.writer.options.overrideHashAndEqualsInResultSets) { into.write('@override int get hashCode => '); - const HashCodeWriter().writeHashCode(fieldNames, into); + writeHashCode(fields, into); into.write(';\n'); - overrideEquals(fieldNames, className, into); - overrideToString(className, fieldNames, into); + overrideEquals(fields, className, into); + overrideToString(className, fields.map((f) => f.lexeme).toList(), into); } into.write('}\n'); diff --git a/drift_dev/lib/src/writer/tables/data_class_writer.dart b/drift_dev/lib/src/writer/tables/data_class_writer.dart index 99b2234f..c0e40cfd 100644 --- a/drift_dev/lib/src/writer/tables/data_class_writer.dart +++ b/drift_dev/lib/src/writer/tables/data_class_writer.dart @@ -87,7 +87,11 @@ class DataClassWriter { _writeHashCode(); overrideEquals( - columns.map((c) => c.dartGetterName), table.dartTypeCode(), _buffer); + columns.map( + (c) => EqualityField(c.dartGetterName, isList: c.isUint8ListInDart)), + table.dartTypeCode(), + _buffer, + ); // finish class declaration _buffer.write('}'); @@ -312,8 +316,11 @@ class DataClassWriter { void _writeHashCode() { _buffer.write('@override\n int get hashCode => '); - final fields = columns.map((c) => c.dartGetterName).toList(); - const HashCodeWriter().writeHashCode(fields, _buffer); + final fields = columns + .map( + (c) => EqualityField(c.dartGetterName, isList: c.isUint8ListInDart)) + .toList(); + writeHashCode(fields, _buffer); _buffer.write(';'); } } diff --git a/drift_dev/lib/src/writer/utils/hash_and_equals.dart b/drift_dev/lib/src/writer/utils/hash_and_equals.dart new file mode 100644 index 00000000..855ecc96 --- /dev/null +++ b/drift_dev/lib/src/writer/utils/hash_and_equals.dart @@ -0,0 +1,72 @@ +const int _maxArgsToObjectHash = 20; + +class EqualityField { + /// The Dart expression evaluating the field to include in the hash / equals + /// check. + final String lexeme; + + /// Whether the field is a list that can't be compared with `==` directly. + final bool isList; + + EqualityField(this.lexeme, {this.isList = false}); +} + +/// Writes an expression to calculate a hash code of an object that consists +/// of the [fields]. +void writeHashCode(List fields, StringBuffer into) { + if (fields.isEmpty) { + into.write('identityHashCode(this)'); + } else if (fields.length == 1) { + final field = fields[0]; + + if (field.isList) { + into.write('\$driftBlobEquality.hash(${field.lexeme})'); + } else { + into.write('${field.lexeme}.hashCode'); + } + } else { + final needsHashAll = fields.length > _maxArgsToObjectHash; + + into.write(needsHashAll ? 'Object.hashAll([' : 'Object.hash('); + var first = true; + for (final field in fields) { + if (!first) into.write(', '); + + if (field.isList) { + into.write('\$driftBlobEquality.hash(${field.lexeme})'); + } else { + into.write(field.lexeme); + } + + first = false; + } + + into.write(needsHashAll ? '])' : ')'); + } +} + +/// Writes a operator == override for a class consisting of the [fields] into +/// the buffer provided by [into]. +void overrideEquals( + Iterable fields, String className, StringBuffer into) { + into + ..writeln('@override') + ..write('bool operator ==(Object other) => ') + ..write('identical(this, other) || (other is $className'); + + if (fields.isNotEmpty) { + into + ..write(' && ') + ..write(fields.map((field) { + final lexeme = field.lexeme; + + if (field.isList) { + return '\$driftBlobEquality.equals(other.$lexeme, this.$lexeme)'; + } else { + return 'other.$lexeme == this.$lexeme'; + } + }).join(' && ')); + } + + into.writeln(');'); +} diff --git a/drift_dev/lib/src/writer/utils/hash_code.dart b/drift_dev/lib/src/writer/utils/hash_code.dart deleted file mode 100644 index f56efe18..00000000 --- a/drift_dev/lib/src/writer/utils/hash_code.dart +++ /dev/null @@ -1,28 +0,0 @@ -class HashCodeWriter { - static const int _maxArgsToObjectHash = 20; - - const HashCodeWriter(); - - /// Writes an expression to calculate a hash code of an object that consists - /// of the [fields]. - void writeHashCode(List fields, StringBuffer into) { - if (fields.isEmpty) { - into.write('identityHashCode(this)'); - } else if (fields.length == 1) { - into.write('${fields[0]}.hashCode'); - } else { - final needsHashAll = fields.length > _maxArgsToObjectHash; - - into.write(needsHashAll ? 'Object.hashAll([' : 'Object.hash('); - var first = true; - for (final field in fields) { - if (!first) into.write(', '); - - into.write(field); - first = false; - } - - into.write(needsHashAll ? '])' : ')'); - } - } -} diff --git a/drift_dev/lib/src/writer/utils/override_equals.dart b/drift_dev/lib/src/writer/utils/override_equals.dart deleted file mode 100644 index ac17efbf..00000000 --- a/drift_dev/lib/src/writer/utils/override_equals.dart +++ /dev/null @@ -1,18 +0,0 @@ -/// Writes a operator == override for a class consisting of the [fields] into -/// the buffer provided by [into]. -void overrideEquals( - Iterable fields, String className, StringBuffer into) { - into - ..write('@override\nbool operator ==(Object other) => ') - ..write('identical(this, other) || (other is $className'); - - if (fields.isNotEmpty) { - into - ..write(' && ') - ..write(fields.map((field) { - return 'other.$field == this.$field'; - }).join(' && ')); - } - - into.write(');\n'); -} diff --git a/drift_dev/lib/writer.dart b/drift_dev/lib/writer.dart index 8765fd01..f67ff50a 100644 --- a/drift_dev/lib/writer.dart +++ b/drift_dev/lib/writer.dart @@ -10,7 +10,6 @@ export 'src/writer/queries/result_set_writer.dart'; export 'src/writer/tables/data_class_writer.dart'; export 'src/writer/tables/table_writer.dart'; export 'src/writer/tables/update_companion_writer.dart'; -export 'src/writer/utils/hash_code.dart'; export 'src/writer/utils/memoized_getter.dart'; -export 'src/writer/utils/override_equals.dart'; +export 'src/writer/utils/hash_and_equals.dart'; export 'src/writer/writer.dart'; diff --git a/drift_dev/test/writer/utils/hash_code_test.dart b/drift_dev/test/writer/utils/hash_code_test.dart index a6f43712..733a7289 100644 --- a/drift_dev/test/writer/utils/hash_code_test.dart +++ b/drift_dev/test/writer/utils/hash_code_test.dart @@ -1,30 +1,42 @@ import 'package:charcode/ascii.dart'; -import 'package:drift_dev/src/writer/utils/hash_code.dart'; +import 'package:drift_dev/src/writer/utils/hash_and_equals.dart'; import 'package:test/test.dart'; void main() { test('hash code for no fields', () { final buffer = StringBuffer(); - const HashCodeWriter().writeHashCode([], buffer); + writeHashCode([], buffer); expect(buffer.toString(), r'identityHashCode(this)'); }); - test('hash code for a single field', () { + test('hash code for a single field - not a list', () { final buffer = StringBuffer(); - const HashCodeWriter().writeHashCode(['a'], buffer); + writeHashCode([EqualityField('a')], buffer); expect(buffer.toString(), r'a.hashCode'); }); + test('hash code for a single field - list', () { + final buffer = StringBuffer(); + writeHashCode([EqualityField('a', isList: true)], buffer); + expect(buffer.toString(), r'$driftBlobEquality.hash(a)'); + }); + test('hash code for multiple fields', () { final buffer = StringBuffer(); - const HashCodeWriter().writeHashCode(['a', 'b', 'c'], buffer); - expect(buffer.toString(), r'Object.hash(a, b, c)'); + writeHashCode([ + EqualityField('a'), + EqualityField('b', isList: true), + EqualityField('c'), + ], buffer); + expect(buffer.toString(), r'Object.hash(a, $driftBlobEquality.hash(b), c)'); }); test('hash code for lots of fields', () { final buffer = StringBuffer(); - const HashCodeWriter().writeHashCode( - List.generate(26, (index) => String.fromCharCode($a + index)), buffer); + writeHashCode( + List.generate( + 26, (index) => EqualityField(String.fromCharCode($a + index))), + buffer); expect( buffer.toString(), r'Object.hashAll([a, b, c, d, e, f, g, h, i, j, k, l, m, n, o, p, q, r, ' diff --git a/drift_dev/test/writer/utils/override_equals_test.dart b/drift_dev/test/writer/utils/override_equals_test.dart index de30f7d5..34ea94c1 100644 --- a/drift_dev/test/writer/utils/override_equals_test.dart +++ b/drift_dev/test/writer/utils/override_equals_test.dart @@ -1,4 +1,4 @@ -import 'package:drift_dev/src/writer/utils/override_equals.dart'; +import 'package:drift_dev/src/writer/utils/hash_and_equals.dart'; import 'package:test/test.dart'; void main() { @@ -14,12 +14,17 @@ void main() { test('overrides equals on class with fields', () { final buffer = StringBuffer(); - overrideEquals(['a', 'b', 'c'], 'Foo', buffer); + overrideEquals([ + EqualityField('a'), + EqualityField('b', isList: true), + EqualityField('c'), + ], 'Foo', buffer); expect( buffer.toString(), '@override\nbool operator ==(Object other) => ' 'identical(this, other) || (other is Foo && ' - 'other.a == this.a && other.b == this.b && other.c == this.c);\n'); + r'other.a == this.a && $driftBlobEquality.equals(other.b, this.b) && ' + 'other.c == this.c);\n'); }); }