Skip to content

Commit

Permalink
Fix comparison of empty lists from frozen messages. (google#492)
Browse files Browse the repository at this point in the history
The existing implementation returns an unmodifiable list, which does not
obey the value-comparison semantics expected for protobuf lists.
Returning a FrozenPbList instead resolves this.

Co-authored-by: Loren Van Spronsen <[email protected]>
  • Loading branch information
LorenVS and Loren Van Spronsen authored Mar 23, 2021
1 parent deeacd7 commit 85716c4
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 1 deletion.
2 changes: 1 addition & 1 deletion protobuf/lib/src/protobuf/field_set.dart
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ class _FieldSet {

List<T> _getDefaultList<T>(FieldInfo<T> fi) {
assert(fi.isRepeated);
if (_isReadOnly) return List.unmodifiable(const []);
if (_isReadOnly) return FrozenPbList._(const []);

// TODO(skybrian) we could avoid this by generating another
// method for repeated fields:
Expand Down
52 changes: 52 additions & 0 deletions protobuf/test/list_equality_test.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
// Test for ensuring that protobuf lists compare using value semantics.
library list_equality_test;

import 'package:test/test.dart';

import 'mock_util.dart' show T;

void main() {
test('empty lists compare as equal', () {
final first = T();
final second = T();
expect(first.int32s == second.int32s, isTrue);
});

test('empty frozen lists compare as equal', () {
final first = T()..freeze();
final second = T()..freeze();
expect(first.int32s == second.int32s, isTrue);
});

test('non-empty lists compare as equal', () {
final first = T()..int32s.add(1);
final second = T()..int32s.add(1);
expect(first.int32s == second.int32s, isTrue);
});

test('non-empty frozen lists compare as equal', () {
final first = T()
..int32s.add(1)
..freeze();
final second = T()
..int32s.add(1)
..freeze();
expect(first.int32s == second.int32s, isTrue);
});

test('different lists do not compare as equal', () {
final first = T()..int32s.add(1);
final second = T()..int32s.add(2);
expect(first.int32s == second.int32s, isFalse);
});

test('different frozen lists do not compare as equal', () {
final first = T()
..int32s.add(1)
..freeze();
final second = T()
..int32s.add(2)
..freeze();
expect(first.int32s == second.int32s, isFalse);
});
}

0 comments on commit 85716c4

Please sign in to comment.