From 1801ba2a0e990c533865fef200bbcc3818b3b486 Mon Sep 17 00:00:00 2001 From: Christopher Wilcox Date: Fri, 24 Jul 2020 07:49:17 -0700 Subject: [PATCH] feat: use `DatetimeWithNanoseconds` throughout library (#116) * chore: update minimum version of protoplus to ensure DatetimeWithNanoseconds availability * feat: Incorporate nanoseconds back into components, such as hashing * blacken * remove unused imports --- google/cloud/firestore_v1/base_document.py | 6 +----- google/cloud/firestore_v1/watch.py | 8 +------- setup.py | 2 +- tests/system/test_system.py | 1 - tests/unit/v1/test_async_batch.py | 6 ++---- tests/unit/v1/test_base_client.py | 7 +++---- tests/unit/v1/test_base_document.py | 12 ++++++++---- tests/unit/v1/test_batch.py | 6 ++---- 8 files changed, 18 insertions(+), 30 deletions(-) diff --git a/google/cloud/firestore_v1/base_document.py b/google/cloud/firestore_v1/base_document.py index a69470f80..196e3cb5e 100644 --- a/google/cloud/firestore_v1/base_document.py +++ b/google/cloud/firestore_v1/base_document.py @@ -243,12 +243,8 @@ def __eq__(self, other): return self._reference == other._reference and self._data == other._data def __hash__(self): - # TODO(microgen, https://github.com/googleapis/proto-plus-python/issues/38): - # maybe add datetime_with_nanos to protoplus, revisit - # seconds = self.update_time.seconds - # nanos = self.update_time.nanos seconds = int(self.update_time.timestamp()) - nanos = 0 + nanos = self.update_time.nanosecond return hash(self._reference) + hash(seconds) + hash(nanos) @property diff --git a/google/cloud/firestore_v1/watch.py b/google/cloud/firestore_v1/watch.py index 9d13fa791..d3499e649 100644 --- a/google/cloud/firestore_v1/watch.py +++ b/google/cloud/firestore_v1/watch.py @@ -565,13 +565,7 @@ def push(self, read_time, next_resume_token): key = functools.cmp_to_key(self._comparator) keys = sorted(updated_tree.keys(), key=key) - self._snapshot_callback( - keys, - appliedChanges, - read_time - # TODO(microgen): now a datetime - # datetime.datetime.fromtimestamp(read_time.seconds, pytz.utc), - ) + self._snapshot_callback(keys, appliedChanges, read_time) self.has_pushed = True self.doc_tree = updated_tree diff --git a/setup.py b/setup.py index ef4c23071..a565fb27a 100644 --- a/setup.py +++ b/setup.py @@ -29,7 +29,7 @@ "google-cloud-core >= 1.0.3, < 2.0dev", "pytz", "libcst >= 0.2.5", - "proto-plus >= 0.4.0", + "proto-plus >= 1.3.0", ] extras = {} diff --git a/tests/system/test_system.py b/tests/system/test_system.py index f0a807f6f..4800014da 100644 --- a/tests/system/test_system.py +++ b/tests/system/test_system.py @@ -340,7 +340,6 @@ def test_update_document(client, cleanup): document.update({"bad": "time-past"}, option=option4) # 6. Call ``update()`` with invalid (in future) "last timestamp" option. - # TODO(microgen): start using custom datetime with nanos in protoplus? timestamp_pb = _datetime_to_pb_timestamp(snapshot4.update_time) timestamp_pb.seconds += 3600 diff --git a/tests/unit/v1/test_async_batch.py b/tests/unit/v1/test_async_batch.py index 7a5504dc4..59852fd88 100644 --- a/tests/unit/v1/test_async_batch.py +++ b/tests/unit/v1/test_async_batch.py @@ -67,8 +67,7 @@ async def test_commit(self): write_results = await batch.commit() self.assertEqual(write_results, list(commit_response.write_results)) self.assertEqual(batch.write_results, write_results) - # TODO(microgen): v2: commit time is already a datetime, though not with nano - # self.assertEqual(batch.commit_time, timestamp) + self.assertEqual(batch.commit_time.timestamp_pb(), timestamp) # Make sure batch has no more "changes". self.assertEqual(batch._write_pbs, []) @@ -108,8 +107,7 @@ async def test_as_context_mgr_wo_error(self): write_pbs = batch._write_pbs[::] self.assertEqual(batch.write_results, list(commit_response.write_results)) - # TODO(microgen): v2: commit time is already a datetime, though not with nano - # self.assertEqual(batch.commit_time, timestamp) + self.assertEqual(batch.commit_time.timestamp_pb(), timestamp) # Make sure batch has no more "changes". self.assertEqual(batch._write_pbs, []) diff --git a/tests/unit/v1/test_base_client.py b/tests/unit/v1/test_base_client.py index cc3a7f06b..631733e07 100644 --- a/tests/unit/v1/test_base_client.py +++ b/tests/unit/v1/test_base_client.py @@ -300,10 +300,9 @@ def test_found(self): self.assertIs(snapshot._reference, mock.sentinel.reference) self.assertEqual(snapshot._data, {"foo": 1.5, "bar": u"skillz"}) self.assertTrue(snapshot._exists) - # TODO(microgen): v2: datetime with nanos implementation needed. - # self.assertEqual(snapshot.read_time, read_time) - # self.assertEqual(snapshot.create_time, create_time) - # self.assertEqual(snapshot.update_time, update_time) + self.assertEqual(snapshot.read_time.timestamp_pb(), read_time) + self.assertEqual(snapshot.create_time.timestamp_pb(), create_time) + self.assertEqual(snapshot.update_time.timestamp_pb(), update_time) def test_missing(self): from google.cloud.firestore_v1.document import DocumentReference diff --git a/tests/unit/v1/test_base_document.py b/tests/unit/v1/test_base_document.py index 0f4556cf9..bba47a984 100644 --- a/tests/unit/v1/test_base_document.py +++ b/tests/unit/v1/test_base_document.py @@ -15,8 +15,8 @@ import unittest import mock -import datetime -import pytz +from proto.datetime_helpers import DatetimeWithNanoseconds +from google.protobuf import timestamp_pb2 class TestBaseDocumentReference(unittest.TestCase): @@ -274,11 +274,15 @@ def test___hash__(self): client.__hash__.return_value = 234566789 reference = self._make_reference("hi", "bye", client=client) data = {"zoop": 83} - update_time = datetime.datetime.fromtimestamp(123456, pytz.utc) + update_time = DatetimeWithNanoseconds.from_timestamp_pb( + timestamp_pb2.Timestamp(seconds=123456, nanos=123456789) + ) snapshot = self._make_one( reference, data, True, None, mock.sentinel.create_time, update_time ) - self.assertEqual(hash(snapshot), hash(reference) + hash(123456) + hash(0)) + self.assertEqual( + hash(snapshot), hash(reference) + hash(123456) + hash(123456789) + ) def test__client_property(self): reference = self._make_reference( diff --git a/tests/unit/v1/test_batch.py b/tests/unit/v1/test_batch.py index 5396540c6..f21dee622 100644 --- a/tests/unit/v1/test_batch.py +++ b/tests/unit/v1/test_batch.py @@ -64,8 +64,7 @@ def test_commit(self): write_results = batch.commit() self.assertEqual(write_results, list(commit_response.write_results)) self.assertEqual(batch.write_results, write_results) - # TODO(microgen): v2: commit time is already a datetime, though not with nano - # self.assertEqual(batch.commit_time, timestamp) + self.assertEqual(batch.commit_time.timestamp_pb(), timestamp) # Make sure batch has no more "changes". self.assertEqual(batch._write_pbs, []) @@ -104,8 +103,7 @@ def test_as_context_mgr_wo_error(self): write_pbs = batch._write_pbs[::] self.assertEqual(batch.write_results, list(commit_response.write_results)) - # TODO(microgen): v2: commit time is already a datetime, though not with nano - # self.assertEqual(batch.commit_time, timestamp) + self.assertEqual(batch.commit_time.timestamp_pb(), timestamp) # Make sure batch has no more "changes". self.assertEqual(batch._write_pbs, [])