MockDocumentReference.snapshots should return a continuous stream

[amatveiakin]

MockDocumentReference.snapshots() should return a continuous stream of updates that yields a new value every time the document is updated. Instead it returns a stream that produces only one value.

Sample code:

    MockDocumentReference documentReference = ...;
    documentReference.snapshots().listen(
      (snapshot) => debugPrint('onData'),
      onError: (error) => debugPrint('onError'),
      onDone: () => debugPrint('onDone'),
    );

Expected output:

onData
onData
onData
...

Actual output:

onData
onDone

[amatveiakin]

I think the problem is that MockDocumentReference.snapshots returns a Stream.value:

https://github.com/atn832/cloud_firestore_mocks/blob/38c30d477399648551a589ffe2fc6fd239b21a77/lib/src/mock_document_reference.dart#L134-L138

Compare it to MockCollectionReference.snapshots, which uses a StreamController:

https://github.com/atn832/cloud_firestore_mocks/blob/1375edbcda74f0e91880687be32fd8461513f4a2/lib/src/mock_collection_reference.dart#L128-L134

I haven't tested it, but I think the fix would be to replace MockDocumentReference.snapshots implementation with something like:

    @override
    Stream<DocumentSnapshot> snapshots({bool includeMetadataChanges = false}) {
      return parent()
          .snapshots(includeMetadataChanges: includeMetadataChanges)
          .where((s) =>
              s.documents.where((d) => d.documentID == _documentId).isNotEmpty)
          .map((s) =>
              s.documents.singleWhere((d) => d.documentID == _documentId));
    }

[amatveiakin]

After digging into the problem I realized that the it's more complicated. Snapshot are broken due to several different issues:

1. MockDocumentReference.snapshots doesn't return a dynamic stream, as noted above. Fix: https://github.com/amatveyakin/cloud_firestore_mocks/commit/2bdfd4e5722e33695e5de9f49d8b3ec2c892eb8a

2. Snapshots are not issued when documents are changed, only when documents are added. Fix: https://github.com/amatveyakin/cloud_firestore_mocks/commit/a2b32644466c7d915d3dbc443c7b6ec43bdc580a. I'm not sure if the fix is fully correct though, as it issues the snapshot only on parent collection level. Grandparent collections should probably report the update too (not sure what Firestore semantics is here).

3. MockDocumentSnapshot doesn't do a deep copy of the data, so it isn't really a snapshot. Fix: https://github.com/amatveyakin/cloud_firestore_mocks/commit/e588fc50c20986e1e5b22b99ddea917ac335986f

The combination of these three fixes made a simple test pass (added in the last commit). This is something, but my real world use-case still doesn't work, so there must be more issues at play.

[amatveiakin]

Good news: the problems mentioned above were on my side. When I solved them, it turned out that those three fixes resolve the issue with snapshots, at least for me. I can sent pull requests if you'd like.

[atn832]

Thanks for investigating and finding a fix for it!

For 1., would it be better to keep a snapshotStreamController in MockDocumentReference itself like it's done in MockCollectionReference? If you rely on MockCollectionReference, a change in one document would trigger an event on its sibling document's snapshot stream.

For 2., you are right. The library doesn't support firing a snapshot at the collection level when one of its documents is changed. Thank you for the fix. Could you make this into its own PR so that it can be unit/integration tested properly? Integration tests run against the real Firestore, the Firestore emulator and the library. I've filed #86 to keep track of it. As for grandparent collections, I've filed #87 to track this. It'd take extra time to write additional tests and run them, so it's up to you whether you want to tackle as well.

For 3., thanks for proposing a fix to #83. @anuragbhd actually already proposed a PR at #80, which he hasn't finalized yet. Perhaps we can wait for him to submit his PR.

[atn832]

After taking a closer look at #80, I realized @anuragbhd is actually fixing a separate issue where a call to the snapshot's data getter should return a deep copy as well. So it would be awesome if you could also send the fix you proposed at 3. a in PR!

[erabti]

Hi @amatveyakin , any updates?

[amatveiakin]

Sent a pull request for 3. Will send the other two afterwards.

[amatveiakin]

For 1., would it be better to keep a snapshotStreamController in MockDocumentReference itself like it's done in MockCollectionReference?

Could you elaborate on this, @atn832? How will this work when the document is updated by other means? If I understand correctly, for this approach to work two things need to be true:

• The document can be updated only through a DocumentReference.
• There can be only one DocumentReference for every document.

The first one might be true. But the second one is not as far as I can tell. There is no de-duplication logic in

https://github.com/atn832/cloud_firestore_mocks/blob/1375edbcda74f0e91880687be32fd8461513f4a2/lib/src/mock_collection_reference.dart#L94-L108

If we want to keep a dedicated stream controller for each document, then we need a cache for either the stream controllers or document references themselves. Similarly to snapshotStreamControllerRoot, I guess.

[atn832]

You're right, we'd need to use a shared controller for a given path, like it's done in snapshotStreamControllerRoot. This way all the references to the same document will fire events when it gets updated.

Right now, we use the '_snapshots' key to store a collection's stream controller. Maybe we could use '${documentId}_snapshots' as a key to store this collection's document's stream controller?