From 4b103681e08593794fe8551b6fea52da860addda Mon Sep 17 00:00:00 2001 From: Hemang Date: Thu, 13 Jun 2019 17:19:43 +0530 Subject: [PATCH 1/3] solve document update server_timestamp remove data --- firestore/google/cloud/firestore_v1/_helpers.py | 14 ++------------ firestore/tests/unit/v1/test__helpers.py | 15 +++++---------- .../unit/v1/testdata/set-st-merge-both.textproto | 1 + .../unit/v1/testdata/set-st-mergeall.textproto | 1 + 4 files changed, 9 insertions(+), 22 deletions(-) diff --git a/firestore/google/cloud/firestore_v1/_helpers.py b/firestore/google/cloud/firestore_v1/_helpers.py index 7f47e74bcf18..8bfb9e0e356a 100644 --- a/firestore/google/cloud/firestore_v1/_helpers.py +++ b/firestore/google/cloud/firestore_v1/_helpers.py @@ -760,14 +760,8 @@ def apply_merge(self, merge): def _get_update_mask(self, allow_empty_mask=False): # Mask uses dotted / quoted paths. - mask_paths = [ - field_path.to_api_repr() - for field_path in self.merge - if field_path not in self.transform_merge - ] - - if mask_paths or allow_empty_mask: - return common_pb2.DocumentMask(field_paths=mask_paths) + mask_paths = [field_path.to_api_repr() for field_path in self.merge] + return common_pb2.DocumentMask(field_paths=mask_paths) def pbs_for_set_with_merge(document_path, document_data, merge): @@ -836,10 +830,6 @@ def _get_update_mask(self, allow_empty_mask=False): for field_path in self.top_level_paths: if field_path not in self.transform_paths: mask_paths.append(field_path.to_api_repr()) - else: - prefix = FieldPath(*field_path.parts[:-1]) - if prefix.parts: - mask_paths.append(prefix.to_api_repr()) return common_pb2.DocumentMask(field_paths=mask_paths) diff --git a/firestore/tests/unit/v1/test__helpers.py b/firestore/tests/unit/v1/test__helpers.py index e33a2c9d0855..1101255ebef2 100644 --- a/firestore/tests/unit/v1/test__helpers.py +++ b/firestore/tests/unit/v1/test__helpers.py @@ -1951,10 +1951,11 @@ def test_with_merge_true_w_transform(self): document_data["butter"] = SERVER_TIMESTAMP write_pbs = self._call_fut(document_path, document_data, merge=True) - update_pb = self._make_write_w_document(document_path, **update_data) + update_data["butter"] = SERVER_TIMESTAMP self._update_document_mask(update_pb, field_paths=sorted(update_data)) transform_pb = self._make_write_w_transform(document_path, fields=["butter"]) + expected_pbs = [update_pb, transform_pb] self.assertEqual(write_pbs, expected_pbs) @@ -1973,7 +1974,7 @@ def test_with_merge_field_w_transform(self): update_pb = self._make_write_w_document( document_path, cheese=document_data["cheese"] ) - self._update_document_mask(update_pb, ["cheese"]) + self._update_document_mask(update_pb, ["cheese", "butter"]) transform_pb = self._make_write_w_transform(document_path, fields=["butter"]) expected_pbs = [update_pb, transform_pb] self.assertEqual(write_pbs, expected_pbs) @@ -1987,8 +1988,8 @@ def test_with_merge_field_w_transform_masking_simple(self): document_data["butter"] = {"pecan": SERVER_TIMESTAMP} write_pbs = self._call_fut(document_path, document_data, merge=["butter.pecan"]) - update_pb = self._make_write_w_document(document_path) + self._update_document_mask(update_pb, ["butter.pecan"]) transform_pb = self._make_write_w_transform( document_path, fields=["butter.pecan"] ) @@ -2107,14 +2108,8 @@ def _helper(self, option=None, do_transform=False, **write_kwargs): field_updates[field_path2] = SERVER_TIMESTAMP write_pbs = self._call_fut(document_path, field_updates, option) - map_pb = document_pb2.MapValue(fields={"yum": _value_pb(bytes_value=value)}) - - if do_transform: - field_paths = [field_path1, "blog"] - else: - field_paths = [field_path1] - + field_paths = [field_path1] expected_update_pb = write_pb2.Write( update=document_pb2.Document( name=document_path, fields={"bitez": _value_pb(map_value=map_pb)} diff --git a/firestore/tests/unit/v1/testdata/set-st-merge-both.textproto b/firestore/tests/unit/v1/testdata/set-st-merge-both.textproto index 5cc7bbc9efbf..e5a32729affe 100644 --- a/firestore/tests/unit/v1/testdata/set-st-merge-both.textproto +++ b/firestore/tests/unit/v1/testdata/set-st-merge-both.textproto @@ -30,6 +30,7 @@ set: < > update_mask: < field_paths: "a" + field_paths: "b" > > writes: < diff --git a/firestore/tests/unit/v1/testdata/set-st-mergeall.textproto b/firestore/tests/unit/v1/testdata/set-st-mergeall.textproto index b8c53a566fdd..f4a9013665a8 100644 --- a/firestore/tests/unit/v1/testdata/set-st-mergeall.textproto +++ b/firestore/tests/unit/v1/testdata/set-st-mergeall.textproto @@ -25,6 +25,7 @@ set: < > update_mask: < field_paths: "a" + field_paths: "b" > > writes: < From b57caa92cb807ad69e27fe1974e4a97fe72ae680 Mon Sep 17 00:00:00 2001 From: Hemang Date: Tue, 25 Jun 2019 18:54:07 +0530 Subject: [PATCH 2/3] add system test for server timestamp --- firestore/tests/system.py | 56 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/firestore/tests/system.py b/firestore/tests/system.py index a8e683629add..f64f2fb9c5b5 100644 --- a/firestore/tests/system.py +++ b/firestore/tests/system.py @@ -223,6 +223,38 @@ def test_document_set_merge(client, cleanup): assert snapshot2.update_time == write_result2.update_time +def test_document_set_merge_w_server_timestamp(client, cleanup): + document_id = "for-set" + unique_resource_id("-") + document = client.document("i-did-it", document_id) + # Add to clean-up before API request (in case ``set()`` fails). + cleanup(document) + + # 0. Make sure the document doesn't exist yet + snapshot = document.get() + assert not snapshot.exists + + # 1. Use ``create()`` to create the document. + data1 = {"name": "Sam", "address": {"city": "SF", "state": "CA"}} + write_result1 = document.create(data1) + snapshot1 = document.get() + assert snapshot1.update_time == write_result1.update_time + + # 2. Call ``set()`` to merge + data2 = {"address": {"city": "LA"}, "now": firestore.SERVER_TIMESTAMP} + write_result2 = document.set(data2, merge=True) + snapshot2 = document.get() + stored_data = snapshot2.to_dict() + server_now = stored_data["now"] + assert stored_data == { + "name": "Sam", + "address": {"city": "LA", "state": "CA"}, + "now": server_now, + } + # Make sure the create time hasn't changed. + assert snapshot2.create_time == snapshot1.create_time + assert snapshot2.update_time == write_result2.update_time + + def test_document_set_w_int_field(client, cleanup): document_id = "set-int-key" + unique_resource_id("-") document = client.document("i-did-it", document_id) @@ -333,6 +365,30 @@ def test_update_document(client, cleanup): document.update({"bad": "time-future"}, option=option6) +def test_update_document_w_server_timestamp(client, cleanup): + document_id = "for-update" + unique_resource_id("-") + document = client.document("made", document_id) + # Add to clean-up before API request (in case ``create()`` fails). + cleanup(document) + + # 1. Update and create the document (with an option). + data = {"foo": {"bar": "baz"}, "scoop": {"barn": 981}, "other": True} + write_result2 = document.create(data) + + # 2. Send an update without a field path (no option). + field_updates3 = {"foo.now": firestore.SERVER_TIMESTAMP} + write_result3 = document.update(field_updates3) + assert_timestamp_less(write_result2.update_time, write_result3.update_time) + snapshot3 = document.get() + stored_data = snapshot3.to_dict() + expected3 = { + "foo": stored_data["foo"], + "scoop": data["scoop"], + "other": data["other"], + } + assert stored_data == expected3 + + def check_snapshot(snapshot, document, data, write_result): assert snapshot.reference is document assert snapshot.to_dict() == data From f5079e08939f41f525d01c5c8967e3008d65421b Mon Sep 17 00:00:00 2001 From: Hemang Date: Tue, 16 Jul 2019 12:39:47 +0530 Subject: [PATCH 3/3] Change proto file as per comment and change code to resolve update SERVER_TIMESTAMP output --- firestore/google/cloud/firestore_v1/_helpers.py | 10 ++++++++-- firestore/tests/unit/v1/test__helpers.py | 5 +---- .../tests/unit/v1/testdata/set-st-merge-both.textproto | 1 - .../tests/unit/v1/testdata/set-st-mergeall.textproto | 1 - 4 files changed, 9 insertions(+), 8 deletions(-) diff --git a/firestore/google/cloud/firestore_v1/_helpers.py b/firestore/google/cloud/firestore_v1/_helpers.py index 8bfb9e0e356a..09f5d7f41c0e 100644 --- a/firestore/google/cloud/firestore_v1/_helpers.py +++ b/firestore/google/cloud/firestore_v1/_helpers.py @@ -760,8 +760,14 @@ def apply_merge(self, merge): def _get_update_mask(self, allow_empty_mask=False): # Mask uses dotted / quoted paths. - mask_paths = [field_path.to_api_repr() for field_path in self.merge] - return common_pb2.DocumentMask(field_paths=mask_paths) + mask_paths = [ + field_path.to_api_repr() + for field_path in self.merge + if field_path not in self.transform_merge + ] + + if mask_paths or allow_empty_mask: + return common_pb2.DocumentMask(field_paths=mask_paths) def pbs_for_set_with_merge(document_path, document_data, merge): diff --git a/firestore/tests/unit/v1/test__helpers.py b/firestore/tests/unit/v1/test__helpers.py index 1101255ebef2..81338a0c359f 100644 --- a/firestore/tests/unit/v1/test__helpers.py +++ b/firestore/tests/unit/v1/test__helpers.py @@ -1952,10 +1952,8 @@ def test_with_merge_true_w_transform(self): write_pbs = self._call_fut(document_path, document_data, merge=True) update_pb = self._make_write_w_document(document_path, **update_data) - update_data["butter"] = SERVER_TIMESTAMP self._update_document_mask(update_pb, field_paths=sorted(update_data)) transform_pb = self._make_write_w_transform(document_path, fields=["butter"]) - expected_pbs = [update_pb, transform_pb] self.assertEqual(write_pbs, expected_pbs) @@ -1974,7 +1972,7 @@ def test_with_merge_field_w_transform(self): update_pb = self._make_write_w_document( document_path, cheese=document_data["cheese"] ) - self._update_document_mask(update_pb, ["cheese", "butter"]) + self._update_document_mask(update_pb, ["cheese"]) transform_pb = self._make_write_w_transform(document_path, fields=["butter"]) expected_pbs = [update_pb, transform_pb] self.assertEqual(write_pbs, expected_pbs) @@ -1989,7 +1987,6 @@ def test_with_merge_field_w_transform_masking_simple(self): write_pbs = self._call_fut(document_path, document_data, merge=["butter.pecan"]) update_pb = self._make_write_w_document(document_path) - self._update_document_mask(update_pb, ["butter.pecan"]) transform_pb = self._make_write_w_transform( document_path, fields=["butter.pecan"] ) diff --git a/firestore/tests/unit/v1/testdata/set-st-merge-both.textproto b/firestore/tests/unit/v1/testdata/set-st-merge-both.textproto index e5a32729affe..5cc7bbc9efbf 100644 --- a/firestore/tests/unit/v1/testdata/set-st-merge-both.textproto +++ b/firestore/tests/unit/v1/testdata/set-st-merge-both.textproto @@ -30,7 +30,6 @@ set: < > update_mask: < field_paths: "a" - field_paths: "b" > > writes: < diff --git a/firestore/tests/unit/v1/testdata/set-st-mergeall.textproto b/firestore/tests/unit/v1/testdata/set-st-mergeall.textproto index f4a9013665a8..b8c53a566fdd 100644 --- a/firestore/tests/unit/v1/testdata/set-st-mergeall.textproto +++ b/firestore/tests/unit/v1/testdata/set-st-mergeall.textproto @@ -25,7 +25,6 @@ set: < > update_mask: < field_paths: "a" - field_paths: "b" > > writes: <