Firestore: Fix sorting 'delete_changes' in 'Watch._compute_snapshot'.

[liamuk]

delete_changes on the changed line is a list of string, not a list of DocumentSnapshots, so it should not be sorted with the query comparator function (see https://github.com/googleapis/google-cloud-python/blob/master/firestore/google/cloud/firestore_v1/watch.py#L575)

For context, I'm getting this error in one of my snapshot listeners in google-cloud-firestore==1.3.0

Thread-ConsumeBidirectionalStream caught unexpected exception 'str' object has no attribute 'reference' and will exit.
Traceback (most recent call last):
  File ".../venv/lib/python3.5/site-packages/google/api_core/bidi.py", line 635, in _thread_main
    self._on_response(response)
  File ".../venv/lib/python3.5/site-packages/google/cloud/firestore_v1/watch.py", line 443, in on
_snapshot
    meth(proto)
  File ".../venv/lib/python3.5/site-packages/google/cloud/firestore_v1/watch.py", line 378, in _o
n_snapshot_target_change_no_change
    self.push(change.read_time, change.resume_token)
  File ".../venv/lib/python3.5/site-packages/google/cloud/firestore_v1/watch.py", line 540, in pu
sh
    self.doc_tree, self.doc_map, deletes, adds, updates
  File ".../venv/lib/python3.5/site-packages/google/cloud/firestore_v1/watch.py", line 669, in _c
ompute_snapshot
    delete_changes = sorted(delete_changes, key=key)
  File ".../venv/lib/python3.5/site-packages/google/cloud/firestore_v1/query.py", line 819, in _c
omparator
    comp = Order()._compare_to(doc1.reference._path, doc2.reference._path)
AttributeError: 'str' object has no attribute 'reference'

[googlebot]

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it!) and we'll verify it.

---

What to do if you already signed the CLA

Individual signers

• It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

• Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
• The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
• The email used to register you as an authorized contributor must also be attached to your GitHub account.

ℹ️ Googlers: Go here for more info.

[liamuk]

@googlebot I signed it!

[googlebot]

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

[tseaver]

@liamuk Thanks for the patch! Our tests are passing here because they do not exercise Watch._compute_snapshot on an instance with a non-dummy comparator. Can you please add a test to the TestWatch class tests/unit/v1/test_watch.py which exercises that case? E.g.:

    def test__compute_snapshot_deletes_w_real_comparator(self):
        from google.cloud.firestore_v1.watch import WatchDocTree

        doc_tree = WatchDocTree()

        class DummyDoc(object):
            update_time = mock.sentinel

        def _comparator(doc1, doc2):
            if doc1.field > doc2.field:
                return 1
            if doc1.field < doc2.field:
                return -1
            return 0

        deleted_doc_1 = DummyDoc()
        deleted_doc_2 = DummyDoc()
        doc_tree = doc_tree.insert(deleted_doc_1, None)
        doc_tree = doc_tree.insert(deleted_doc_2, None)
        doc_map = {
            "/deleted_1": deleted_doc_1,
            "/deleted_2": deleted_doc_2,
        }
        delete_changes = ["/deleted_1", "/deleted_2"]
        add_changes = []
        update_changes = []
        inst = self._makeOne(comparator=_comparator)
        updated_tree, updated_map, applied_changes = inst._compute_snapshot(
            doc_tree, doc_map, delete_changes, add_changes, update_changes
        )
        self.assertEqual(updated_map, {})

[liamuk]

Added that test verbatim

[tseaver]

Sorry to have to ask for changes in the code I suggested: I checked that the test ran and failed without your patch, but not that it appeased coverage / lint checkers.

[liamuk]

Changed those two things, and also added you as a collaborator to the patch's fork in case anything else comes up.

You writing code for me to copy into the patch branch isn't the most efficient process haha.

[tseaver]

The Video Intelligence failure is due to a Kokoro glitch: "Cannot check out non-mergeable pull request as if merged."

[tseaver]

@liamuk Thanks again for the patch!