diff --git a/firestore/google/cloud/firestore.py b/firestore/google/cloud/firestore.py index 9e0efdcb576e..4e7fd4768f13 100644 --- a/firestore/google/cloud/firestore.py +++ b/firestore/google/cloud/firestore.py @@ -19,7 +19,6 @@ from google.cloud.firestore_v1beta1 import AdminClient from google.cloud.firestore_v1beta1 import Client from google.cloud.firestore_v1beta1 import CollectionReference -from google.cloud.firestore_v1beta1 import CreateIfMissingOption from google.cloud.firestore_v1beta1 import DELETE_FIELD from google.cloud.firestore_v1beta1 import DocumentReference from google.cloud.firestore_v1beta1 import DocumentSnapshot @@ -42,7 +41,6 @@ 'AdminClient', 'Client', 'CollectionReference', - 'CreateIfMissingOption', 'DELETE_FIELD', 'DocumentReference', 'DocumentSnapshot', diff --git a/firestore/google/cloud/firestore_v1beta1/__init__.py b/firestore/google/cloud/firestore_v1beta1/__init__.py index e0069d68dc0d..d8b992b1d661 100644 --- a/firestore/google/cloud/firestore_v1beta1/__init__.py +++ b/firestore/google/cloud/firestore_v1beta1/__init__.py @@ -22,7 +22,6 @@ from google.cloud.firestore_v1beta1._helpers import ReadAfterWriteError from google.cloud.firestore_v1beta1.batch import WriteBatch from google.cloud.firestore_v1beta1.client import Client -from google.cloud.firestore_v1beta1.client import CreateIfMissingOption from google.cloud.firestore_v1beta1.client import ExistsOption from google.cloud.firestore_v1beta1.client import LastUpdateOption from google.cloud.firestore_v1beta1.client import WriteOption @@ -46,7 +45,6 @@ 'AdminClient', 'Client', 'CollectionReference', - 'CreateIfMissingOption', 'DELETE_FIELD', 'DocumentReference', 'DocumentSnapshot', diff --git a/firestore/google/cloud/firestore_v1beta1/_helpers.py b/firestore/google/cloud/firestore_v1beta1/_helpers.py index 3acda674b7dc..8d07bbbf09e1 100644 --- a/firestore/google/cloud/firestore_v1beta1/_helpers.py +++ b/firestore/google/cloud/firestore_v1beta1/_helpers.py @@ -47,10 +47,6 @@ 'The data at {!r} is not a dictionary, so it cannot contain the key {!r}') FIELD_PATH_DELIMITER = '.' DOCUMENT_PATH_DELIMITER = '/' -_NO_CREATE_TEMPLATE = ( - 'The ``create_if_missing`` option cannot be used ' - 'on ``{}()`` requests.') -NO_CREATE_ON_DELETE = _NO_CREATE_TEMPLATE.format('delete') INACTIVE_TXN = ( 'Transaction not in progress, cannot be used in API requests.') READ_AFTER_WRITE_ERROR = 'Attempted read after write in a transaction.' @@ -856,7 +852,7 @@ def pbs_for_update(client, document_path, field_updates, option): """ if option is None: # Default uses ``exists=True``. - option = client.write_option(create_if_missing=False) + option = client.write_option(exists=True) transform_paths, actual_updates = remove_server_timestamp(field_updates) update_values, field_paths = FieldPathHelper.to_field_paths(actual_updates) @@ -897,7 +893,7 @@ def pb_for_delete(document_path, option): """ write_pb = write_pb2.Write(delete=document_path) if option is not None: - option.modify_write(write_pb, no_create_msg=NO_CREATE_ON_DELETE) + option.modify_write(write_pb) return write_pb diff --git a/firestore/google/cloud/firestore_v1beta1/client.py b/firestore/google/cloud/firestore_v1beta1/client.py index d19898bbb652..e2ccad950aa4 100644 --- a/firestore/google/cloud/firestore_v1beta1/client.py +++ b/firestore/google/cloud/firestore_v1beta1/client.py @@ -42,8 +42,7 @@ DEFAULT_DATABASE = '(default)' """str: The default database used in a :class:`~.firestore.client.Client`.""" _BAD_OPTION_ERR = ( - 'Exactly one of ``create_if_missing``, ``last_update_time`` ' - 'and ``exists`` must be provided.') + 'Exactly one of ``last_update_time`` or ``exists`` must be provided.') _BAD_DOC_TEMPLATE = ( 'Document {!r} appeared in response but was not present among references') _ACTIVE_TXN = 'There is already an active transaction.' @@ -252,10 +251,8 @@ def write_option(**kwargs): :meth:`~.DocumentReference.update` and :meth:`~.DocumentReference.delete`. - Exactly one of three keyword arguments must be provided: + One of the following keyword arguments must be provided: - * ``create_if_missing`` (:class:`bool`): Indicates if the document - should be created if it doesn't already exist. * ``last_update_time`` (:class:`google.protobuf.timestamp_pb2.\ Timestamp`): A timestamp. When set, the target document must exist and have been last updated at that time. Protobuf ``update_time`` @@ -268,9 +265,8 @@ def write_option(**kwargs): it is not allowed). Providing multiple would be an apparent contradiction, since ``last_update_time`` assumes that the document **was** updated (it can't have been updated if it - doesn't exist) and both ``create_if_missing`` and ``exists`` indicate - that it is unknown if the document exists or not (but in different - ways). + doesn't exist) and ``exists`` indicate that it is unknown if the + document exists or not. Args: kwargs (Dict[str, Any]): The keyword arguments described above. @@ -283,9 +279,7 @@ def write_option(**kwargs): raise TypeError(_BAD_OPTION_ERR) name, value = kwargs.popitem() - if name == 'create_if_missing': - return CreateIfMissingOption(value) - elif name == 'last_update_time': + if name == 'last_update_time': return LastUpdateOption(value) elif name == 'exists': return ExistsOption(value) @@ -424,73 +418,12 @@ def modify_write(self, write_pb, **unused_kwargs): write_pb.current_document.CopyFrom(current_doc) -class CreateIfMissingOption(WriteOption): - """Option used to assert "create if missing" on a write operation. - - This will typically be created by - :meth:`~.firestore_v1beta1.client.Client.write_option`. - - Args: - create_if_missing (bool): Indicates if the document should be created - if it doesn't already exist. - """ - - def __init__(self, create_if_missing): - self._create_if_missing = create_if_missing - - def modify_write(self, write_pb, no_create_msg=None): - """Modify a ``Write`` protobuf based on the state of this write option. - - If: - - * ``create_if_missing=False``, adds a precondition that requires - existence - * ``create_if_missing=True``, does not add any precondition - * ``no_create_msg`` is passed, raises an exception. For example, in a - :meth:`~.DocumentReference.delete`, no "create" can occur, so it - wouldn't make sense to "create if missing". - - Args: - write_pb (google.cloud.firestore_v1beta1.types.Write): A - ``Write`` protobuf instance to be modified with a precondition - determined by the state of this option. - no_create_msg (Optional[str]): A message to use to indicate that - a create operation is not allowed. - - Raises: - ValueError: If ``no_create_msg`` is passed. - """ - if no_create_msg is not None: - raise ValueError(no_create_msg) - elif not self._create_if_missing: - current_doc = types.Precondition(exists=True) - write_pb.current_document.CopyFrom(current_doc) - - class ExistsOption(WriteOption): """Option used to assert existence on a write operation. This will typically be created by :meth:`~.firestore_v1beta1.client.Client.write_option`. - This option is closely related to - :meth:`~.firestore_v1beta1.client.CreateIfMissingOption`, - but a "create if missing". In fact, - - .. code-block:: python - - >>> ExistsOption(exists=True) - - is (mostly) equivalent to - - .. code-block:: python - - >>> CreateIfMissingOption(create_if_missing=False) - - The only difference being that "create if missing" cannot be used - on some operations (e.g. :meth:`~.DocumentReference.delete`) - while "exists" can. - Args: exists (bool): Indicates if the document being modified should already exist. diff --git a/firestore/google/cloud/firestore_v1beta1/document.py b/firestore/google/cloud/firestore_v1beta1/document.py index 3ba6a4a82ca3..5bc4d332642d 100644 --- a/firestore/google/cloud/firestore_v1beta1/document.py +++ b/firestore/google/cloud/firestore_v1beta1/document.py @@ -377,9 +377,7 @@ def delete(self, option=None): Args: option (Optional[~.firestore_v1beta1.client.WriteOption]): A write option to make assertions / preconditions on the server - state of the document before applying changes. Note that - ``create_if_missing`` can't be used here since it does not - apply (i.e. a "delete" cannot "create"). + state of the document before applying changes. Returns: google.protobuf.timestamp_pb2.Timestamp: The time that the delete @@ -387,9 +385,6 @@ def delete(self, option=None): when the delete was sent (i.e. nothing was deleted), this method will still succeed and will still return the time that the request was received by the server. - - Raises: - ValueError: If the ``create_if_missing`` write option is used. """ write_pb = _helpers.pb_for_delete(self._document_path, option) with _helpers.remap_gax_error_on_commit(): diff --git a/firestore/tests/system.py b/firestore/tests/system.py index 2d8024a6c950..59083a109adb 100644 --- a/firestore/tests/system.py +++ b/firestore/tests/system.py @@ -139,7 +139,7 @@ def test_document_set(client, cleanup): cleanup(document) # 0. Make sure the document doesn't exist yet using an option. - option0 = client.write_option(create_if_missing=False) + option0 = client.write_option(exists=True) with pytest.raises(NotFound) as exc_info: document.set({'no': 'way'}, option=option0) @@ -148,7 +148,7 @@ def test_document_set(client, cleanup): # 1. Use ``set()`` to create the document (using an option). data1 = {'foo': 88} - option1 = client.write_option(create_if_missing=True) + option1 = client.write_option(exists=False) write_result1 = document.set(data1, option=option1) snapshot1 = document.get() assert snapshot1.to_dict() == data1 @@ -207,7 +207,7 @@ def test_update_document(client, cleanup): assert document_id in exc_info.value.message # 1. Try to update before the document exists (now with an option). - option1 = client.write_option(create_if_missing=False) + option1 = client.write_option(exists=True) with pytest.raises(NotFound) as exc_info: document.update({'still': 'not-there'}, option=option1) assert exc_info.value.message.startswith(MISSING_ENTITY) @@ -223,7 +223,7 @@ def test_update_document(client, cleanup): }, 'other': True, } - option2 = client.write_option(create_if_missing=True) + option2 = client.write_option(exists=False) write_result2 = document.update(data, option=option2) # 3. Send an update without a field path (no option). diff --git a/firestore/tests/unit/test__helpers.py b/firestore/tests/unit/test__helpers.py index 5669ba1da71f..246a35faf52f 100644 --- a/firestore/tests/unit/test__helpers.py +++ b/firestore/tests/unit/test__helpers.py @@ -1172,9 +1172,9 @@ def test_without_option(self): def test_with_option(self): from google.cloud.firestore_v1beta1.proto import common_pb2 - from google.cloud.firestore_v1beta1.client import CreateIfMissingOption + from google.cloud.firestore_v1beta1.client import ExistsOption - option = CreateIfMissingOption(False) + option = ExistsOption(True) precondition = common_pb2.Precondition(exists=True) self._helper(option=option, current_document=precondition) @@ -1191,11 +1191,12 @@ def _call_fut(client, document_path, field_updates, option): return pbs_for_update(client, document_path, field_updates, option) def _helper(self, option=None, do_transform=False, **write_kwargs): + from google.cloud.firestore_v1beta1.client import Client + from google.cloud.firestore_v1beta1.client import ExistsOption from google.cloud.firestore_v1beta1.gapic import enums from google.cloud.firestore_v1beta1.proto import common_pb2 from google.cloud.firestore_v1beta1.proto import document_pb2 from google.cloud.firestore_v1beta1.proto import write_pb2 - from google.cloud.firestore_v1beta1.client import Client from google.cloud.firestore_v1beta1.constants import SERVER_TIMESTAMP document_path = _make_ref_string( @@ -1216,6 +1217,9 @@ def _helper(self, option=None, do_transform=False, **write_kwargs): map_pb = document_pb2.MapValue(fields={ 'yum': _value_pb(bytes_value=value), }) + if isinstance(option, ExistsOption): + write_kwargs.update({'current_document' : {'exists': option._exists}}) + expected_update_pb = write_pb2.Write( update=document_pb2.Document( name=document_path, @@ -1250,9 +1254,9 @@ def test_without_option(self): def test_with_option(self): from google.cloud.firestore_v1beta1.proto import common_pb2 - from google.cloud.firestore_v1beta1.client import CreateIfMissingOption + from google.cloud.firestore_v1beta1.client import ExistsOption - option = CreateIfMissingOption(True) + option = ExistsOption(False) self._helper(option=option) def test_update_and_transform(self): @@ -1299,16 +1303,6 @@ def test_with_option(self): precondition = common_pb2.Precondition(update_time=update_time) self._helper(option=option, current_document=precondition) - def test_bad_option(self): - from google.cloud.firestore_v1beta1._helpers import NO_CREATE_ON_DELETE - from google.cloud.firestore_v1beta1.client import CreateIfMissingOption - - option = CreateIfMissingOption(True) - with self.assertRaises(ValueError) as exc_info: - self._helper(option=option) - - self.assertEqual(exc_info.exception.args, (NO_CREATE_ON_DELETE,)) - class Test_get_transaction_id(unittest.TestCase): diff --git a/firestore/tests/unit/test_client.py b/firestore/tests/unit/test_client.py index edb0bdfcdccf..314fc505ea6e 100644 --- a/firestore/tests/unit/test_client.py +++ b/firestore/tests/unit/test_client.py @@ -179,19 +179,6 @@ def test_field_path(self): klass = self._get_target_class() self.assertEqual(klass.field_path('a', 'b', 'c'), 'a.b.c') - def test_write_option_create(self): - from google.cloud.firestore_v1beta1.client import CreateIfMissingOption - - klass = self._get_target_class() - - option1 = klass.write_option(create_if_missing=False) - self.assertIsInstance(option1, CreateIfMissingOption) - self.assertFalse(option1._create_if_missing) - - option2 = klass.write_option(create_if_missing=True) - self.assertIsInstance(option2, CreateIfMissingOption) - self.assertTrue(option2._create_if_missing) - def test_write_option_last_update(self): from google.protobuf import timestamp_pb2 from google.cloud.firestore_v1beta1.client import LastUpdateOption @@ -234,7 +221,7 @@ def test_write_multiple_args(self): klass = self._get_target_class() with self.assertRaises(TypeError) as exc_info: klass.write_option( - create_if_missing=False, + exists=False, last_update_time=mock.sentinel.timestamp) self.assertEqual(exc_info.exception.args, (_BAD_OPTION_ERR,)) @@ -473,59 +460,6 @@ def test_modify_write_update_time(self): self.assertEqual(write_pb.current_document, expected_doc) -class TestCreateIfMissingOption(unittest.TestCase): - - @staticmethod - def _get_target_class(): - from google.cloud.firestore_v1beta1.client import CreateIfMissingOption - - return CreateIfMissingOption - - def _make_one(self, *args, **kwargs): - klass = self._get_target_class() - return klass(*args, **kwargs) - - def test_constructor(self): - option = self._make_one(mock.sentinel.totes_bool) - self.assertIs(option._create_if_missing, mock.sentinel.totes_bool) - - def test_modify_write_dont_create(self): - from google.cloud.firestore_v1beta1.proto import common_pb2 - from google.cloud.firestore_v1beta1.proto import write_pb2 - - option = self._make_one(False) - write_pb = write_pb2.Write() - ret_val = option.modify_write(write_pb) - - self.assertIsNone(ret_val) - expected_doc = common_pb2.Precondition(exists=True) - self.assertEqual(write_pb.current_document, expected_doc) - - def test_modify_write_do_create(self): - from google.cloud.firestore_v1beta1.proto import write_pb2 - - option = self._make_one(True) - write_pb = write_pb2.Write() - ret_val = option.modify_write(write_pb) - - self.assertIsNone(ret_val) - # No precondition is set here. - self.assertFalse(write_pb.HasField('current_document')) - - def test_modify_write_create_not_allowed(self): - no_create_msg = mock.sentinel.message - option1 = self._make_one(True) - option2 = self._make_one(False) - - with self.assertRaises(ValueError) as exc_info: - option1.modify_write(None, no_create_msg=no_create_msg) - self.assertEqual(exc_info.exception.args, (no_create_msg,)) - - with self.assertRaises(ValueError) as exc_info: - option2.modify_write(None, no_create_msg=no_create_msg) - self.assertEqual(exc_info.exception.args, (no_create_msg,)) - - class TestExistsOption(unittest.TestCase): @staticmethod diff --git a/firestore/tests/unit/test_document.py b/firestore/tests/unit/test_document.py index 8607c6fb6a6e..7e818694f08e 100644 --- a/firestore/tests/unit/test_document.py +++ b/firestore/tests/unit/test_document.py @@ -287,7 +287,7 @@ def test_set(self): self._set_helper() def test_set_with_option(self): - self._set_helper(create_if_missing=False) + self._set_helper(exists=True) @staticmethod def _write_pb_for_update(document_path, update_values, field_paths): @@ -355,7 +355,7 @@ def test_update(self): self._update_helper() def test_update_with_option(self): - self._update_helper(create_if_missing=False) + self._update_helper(exists=False) def _delete_helper(self, **option_kwargs): from google.cloud.firestore_v1beta1.proto import write_pb2 @@ -400,13 +400,6 @@ def test_delete_with_option(self): ) self._delete_helper(last_update_time=timestamp_pb) - def test_delete_with_bad_option(self): - from google.cloud.firestore_v1beta1._helpers import NO_CREATE_ON_DELETE - - with self.assertRaises(ValueError) as exc_info: - self._delete_helper(create_if_missing=True) - self.assertEqual(exc_info.exception.args, (NO_CREATE_ON_DELETE,)) - def test_get_success(self): # Create a minimal fake client with a dummy response. response_iterator = iter([mock.sentinel.snapshot])