Skip to content

[cloud_firestore, cloud_firestore_web] Fix encoding issues in DocumentSnapshotPlatform and FieldValueFactoryWeb #2105

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Mar 6, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions packages/cloud_firestore/cloud_firestore/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
## 0.13.4+1

* Fix crash with pagination with `DocumentReference` (#2044)
* Minor tweaks to integ tests.

## 0.13.4

* Support equality comparison on `FieldValue` instances.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
#!/usr/bin/sh
echo "Moving tests to lib/main.dart..."
cp test_driver/cloud_firestore.dart lib/main.dart

flutter run -d chrome

echo "Restoring lib/main.dart..."
git restore lib/main.dart

Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ void main() {
snapshot.metadata.hasPendingWrites || snapshot.metadata.isFromCache) {
snapshot = await snapshotsWithMetadataChanges.take(1).first;
}
// At this point, the test is a flake...
expect(snapshot.data['hello'], 'world');

await ref.delete();
Expand Down Expand Up @@ -294,6 +295,49 @@ void main() {
await doc2.delete();
});

test('pagination with DocumentReference (#2044)', () async {
// Populate the database with two test documents
final CollectionReference messages = firestore.collection('messages');
final DocumentReference doc1 = messages.document();
// Use document ID as a unique identifier to ensure that we don't
// collide with other tests running against this database.
final String testRun = doc1.documentID;
await doc1.setData(<String, dynamic>{
'message': 'pagination testing1',
'test_run': testRun,
'some_order': 1,
});
final DocumentReference doc2 = messages.document();
await doc2.setData(<String, dynamic>{
'message': 'pagination testing2',
'test_run': testRun,
'some_order': 2,
'reference': doc1,
});
final DocumentSnapshot snapshot2 = await doc2.get();

QuerySnapshot snapshot;
List<DocumentSnapshot> results;

// startAtDocument with the Snapshot of a doc that contains a DocumentReference.
snapshot = await messages
.orderBy('some_order', descending: true)
.where('test_run', isEqualTo: testRun)
.startAtDocument(snapshot2)
.getDocuments();

results = snapshot.documents;
expect(results.length, 2);

// Results are expected in descending order.
expect(results[0].data['message'], 'pagination testing2');
expect(results[1].data['message'], 'pagination testing1');

// Clean up
await doc2.delete();
await doc1.delete();
});

test('FieldPath.documentId', () async {
// Populate the database with two test documents.
final CollectionReference messages = firestore.collection('messages');
Expand Down Expand Up @@ -339,16 +383,14 @@ void main() {
await ref.document('maputo').setData(<String, dynamic>{
'country': 'Mozambique',
});
final QuerySnapshot snapshot = await ref
.where('country', whereIn: <String>['USA', 'Mozambique'])
.orderBy('country')
.getDocuments();
final QuerySnapshot snapshot = await ref.where('country',
whereIn: <String>['USA', 'Mozambique']).getDocuments();
final List<DocumentSnapshot> results = snapshot.documents;
expect(results.length, 2);
final DocumentSnapshot snapshot1 = results[0];
final DocumentSnapshot snapshot2 = results[1];
expect(snapshot1.documentID, 'maputo');
expect(snapshot2.documentID, 'la');
expect(snapshot1.documentID, 'la');
expect(snapshot2.documentID, 'maputo');
});

test('Query.whereArrayContainsAny', () async {
Expand All @@ -369,8 +411,8 @@ void main() {
expect(results.length, 2);
final DocumentSnapshot snapshot1 = results[0];
final DocumentSnapshot snapshot2 = results[1];
expect(snapshot1.documentID, 'la');
expect(snapshot2.documentID, 'tokyo');
expect(snapshot1.documentID, 'tokyo');
expect(snapshot2.documentID, 'la');
});

test('Query.whereArrayContainsAny using DocumentReference', () async {
Expand Down Expand Up @@ -454,7 +496,7 @@ void main() {
expect(snapshot.data['children'].length, equals(2));
});

test('FieldValue.arrayRemove with DocumentRefrence', () async {
test('FieldValue.arrayRemove with DocumentReference', () async {
final CollectionReference ref = firestore.collection('messages');
await ref.document('test-docRef-1').setData({"message": "1"});
await ref.document('test-docRef-2').setData({"message": "2"});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,10 @@ class _PlatformUtils {
DocumentSnapshot documentSnapshot) =>
platform.DocumentSnapshotPlatform(
documentSnapshot.reference.path,
documentSnapshot.data,
// We could access `documentSnapshot._delegate.data` directly instead
// of going through the getter that `replaceDelegatesWithValuesInMap`
// on the data, but this way, the code is not tied to a part-ed lib implementation.
_CodecUtility.replaceValueWithDelegatesInMap(documentSnapshot.data),
platform.SnapshotMetadataPlatform(
documentSnapshot.metadata.hasPendingWrites,
documentSnapshot.metadata.isFromCache,
Expand Down
2 changes: 1 addition & 1 deletion packages/cloud_firestore/cloud_firestore/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ description:
Flutter plugin for Cloud Firestore, a cloud-hosted, noSQL database with
live synchronization and offline support on Android and iOS.
homepage: https://github.com/FirebaseExtended/flutterfire/tree/master/packages/cloud_firestore/cloud_firestore
version: 0.13.4
version: 0.13.4+1

flutter:
plugin:
Expand Down
4 changes: 4 additions & 0 deletions packages/cloud_firestore/cloud_firestore_web/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 0.1.1+1

* Ensure FieldValueFactoryWeb correctly encodes parameters for arrayRemove/arrayUnion FieldValues.

## 0.1.1

* Support equality comparison of field values.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,17 +7,18 @@ import 'package:firebase/firestore.dart' as web;
import 'package:js/js_util.dart';

import 'package:cloud_firestore_web/src/field_value_web.dart';
import 'package:cloud_firestore_web/src/utils/codec_utility.dart';

/// An implementation of [FieldValueFactoryPlatform] which builds [FieldValuePlatform]
/// instance that is [jsify] friendly
class FieldValueFactoryWeb extends FieldValueFactoryPlatform {
@override
FieldValueWeb arrayRemove(List elements) =>
FieldValueWeb(web.FieldValue.arrayRemove(elements));
FieldValueWeb arrayRemove(List elements) => FieldValueWeb(
web.FieldValue.arrayRemove(CodecUtility.valueEncode(elements)));

@override
FieldValueWeb arrayUnion(List elements) =>
FieldValueWeb(web.FieldValue.arrayUnion(elements));
FieldValueWeb arrayUnion(List elements) => FieldValueWeb(
web.FieldValue.arrayUnion(CodecUtility.valueEncode(elements)));

@override
FieldValueWeb delete() => FieldValueWeb(web.FieldValue.delete());
Expand Down
2 changes: 1 addition & 1 deletion packages/cloud_firestore/cloud_firestore_web/pubspec.yaml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
name: cloud_firestore_web
description: The web implementation of cloud_firestore
homepage: https://github.com/FirebaseExtended/flutterfire/tree/master/packages/cloud_firestore/cloud_firestore_web
version: 0.1.1
version: 0.1.1+1

flutter:
plugin:
Expand Down