From af5a175c1cfd26e31387c22676c435da89e3ac9d Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Mon, 4 Oct 2021 13:02:03 -0400 Subject: [PATCH 1/4] fix: hash snapshots using correct type for 'update_time' Document that 'DocumentSnapshot.create_time' and 'DocumentSnapshot.update_time' are instances of 'proto.datetime_helpers.DatetimeWithNanoseconds'. Closes #398. --- google/cloud/firestore_v1/base_document.py | 10 ++++------ tests/unit/v1/test_base_document.py | 11 ++++------- 2 files changed, 8 insertions(+), 13 deletions(-) diff --git a/google/cloud/firestore_v1/base_document.py b/google/cloud/firestore_v1/base_document.py index 9e15b108c2..15e539fd10 100644 --- a/google/cloud/firestore_v1/base_document.py +++ b/google/cloud/firestore_v1/base_document.py @@ -342,11 +342,11 @@ class DocumentSnapshot(object): exists (bool): Indicates if the document existed at the time the snapshot was retrieved. - read_time (:class:`google.protobuf.timestamp_pb2.Timestamp`): + read_time (:class:`proto.datetime_helpers.DatetimeWithNanoseconds`): The time that this snapshot was read from the server. - create_time (:class:`google.protobuf.timestamp_pb2.Timestamp`): + create_time (:class:`proto.datetime_helpers.DatetimeWithNanoseconds`): The time that this document was created. - update_time (:class:`google.protobuf.timestamp_pb2.Timestamp`): + update_time (:class:`proto.datetime_helpers.DatetimeWithNanoseconds`): The time that this document was last updated. """ @@ -368,9 +368,7 @@ def __eq__(self, other): return self._reference == other._reference and self._data == other._data def __hash__(self): - seconds = int(self.update_time.timestamp()) - nanos = self.update_time.nanosecond - return hash(self._reference) + hash(seconds) + hash(nanos) + return hash(self._reference) + hash(self.update_time) @property def _client(self): diff --git a/tests/unit/v1/test_base_document.py b/tests/unit/v1/test_base_document.py index bba47a9848..51111eb405 100644 --- a/tests/unit/v1/test_base_document.py +++ b/tests/unit/v1/test_base_document.py @@ -12,11 +12,11 @@ # See the License for the specific language governing permissions and # limitations under the License. +import datetime import unittest import mock from proto.datetime_helpers import DatetimeWithNanoseconds -from google.protobuf import timestamp_pb2 class TestBaseDocumentReference(unittest.TestCase): @@ -274,15 +274,12 @@ def test___hash__(self): client.__hash__.return_value = 234566789 reference = self._make_reference("hi", "bye", client=client) data = {"zoop": 83} - update_time = DatetimeWithNanoseconds.from_timestamp_pb( - timestamp_pb2.Timestamp(seconds=123456, nanos=123456789) - ) + now = DatetimeWithNanoseconds.utcnow() + update_time = now.replace(tzinfo=datetime.timezone.utc) snapshot = self._make_one( reference, data, True, None, mock.sentinel.create_time, update_time ) - self.assertEqual( - hash(snapshot), hash(reference) + hash(123456) + hash(123456789) - ) + self.assertEqual(hash(snapshot), hash(reference) + hash(update_time)) def test__client_property(self): reference = self._make_reference( From e1e5a9788fe428d91fbce292a5bd7ade38217d51 Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Mon, 4 Oct 2021 13:28:01 -0400 Subject: [PATCH 2/4] tests: add systest confirming hash fix Closes #391. --- tests/system/test_system.py | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/tests/system/test_system.py b/tests/system/test_system.py index 74c04db182..ac5a10eae7 100644 --- a/tests/system/test_system.py +++ b/tests/system/test_system.py @@ -1584,8 +1584,7 @@ def test_repro_429(client, cleanup): now = datetime.datetime.utcnow().replace(tzinfo=UTC) collection = client.collection("repro-429" + UNIQUE_RESOURCE_ID) - document_ids = [f"doc-{doc_id:02d}" for doc_id in range(30)] - for document_id in document_ids: + for document_id in [f"doc-{doc_id:02d}" for doc_id in range(30)]: data = {"now": now, "paymentId": None} _, document = collection.add(data, document_id) cleanup(document.delete) @@ -1601,3 +1600,17 @@ def test_repro_429(client, cleanup): for snapshot in query2.stream(): print(f"id: {snapshot.id}") + + +def test_repro_391(client, cleanup): + # See: https://github.com/googleapis/python-firestore/issues/391 + now = datetime.datetime.utcnow().replace(tzinfo=UTC) + collection = client.collection("repro-391" + UNIQUE_RESOURCE_ID) + + document_ids = [f"doc-{doc_id:02d}" for doc_id in range(30)] + + for document_id in [f"doc-{doc_id:02d}" for doc_id in range(30)]: + data = {"now": now} + _, document = collection.add(data, document_id) + + assert len(set(collection.stream())) == len(document_ids) From 2ab2a23c39aceaeefcdddbb189373605d53a877f Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Mon, 4 Oct 2021 13:45:47 -0400 Subject: [PATCH 3/4] fix: work around quirk of proto-plus DatetimeWithNanosecond Its 'utcnow' factory (derived from stdlib 'datetime') skips its '__new__'. --- tests/unit/v1/test_base_document.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/unit/v1/test_base_document.py b/tests/unit/v1/test_base_document.py index 51111eb405..21b3be7976 100644 --- a/tests/unit/v1/test_base_document.py +++ b/tests/unit/v1/test_base_document.py @@ -274,8 +274,8 @@ def test___hash__(self): client.__hash__.return_value = 234566789 reference = self._make_reference("hi", "bye", client=client) data = {"zoop": 83} - now = DatetimeWithNanoseconds.utcnow() - update_time = now.replace(tzinfo=datetime.timezone.utc) + update_time = DatetimeWithNanoseconds( + 2021, 10, 4, 17, 43, 27, nanosecond=123456789, tzinfo=datetime.timezone.utc) snapshot = self._make_one( reference, data, True, None, mock.sentinel.create_time, update_time ) From 00adb3cb38825256f88a9e65c336528336976b0f Mon Sep 17 00:00:00 2001 From: Owl Bot Date: Mon, 4 Oct 2021 17:47:56 +0000 Subject: [PATCH 4/4] =?UTF-8?q?=F0=9F=A6=89=20Updates=20from=20OwlBot?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md --- tests/unit/v1/test_base_document.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/unit/v1/test_base_document.py b/tests/unit/v1/test_base_document.py index 21b3be7976..2342f4485c 100644 --- a/tests/unit/v1/test_base_document.py +++ b/tests/unit/v1/test_base_document.py @@ -275,7 +275,8 @@ def test___hash__(self): reference = self._make_reference("hi", "bye", client=client) data = {"zoop": 83} update_time = DatetimeWithNanoseconds( - 2021, 10, 4, 17, 43, 27, nanosecond=123456789, tzinfo=datetime.timezone.utc) + 2021, 10, 4, 17, 43, 27, nanosecond=123456789, tzinfo=datetime.timezone.utc + ) snapshot = self._make_one( reference, data, True, None, mock.sentinel.create_time, update_time )