Skip to content

Commit

Permalink
Firestore: Remove CreateIfMissing option
Browse files Browse the repository at this point in the history
  • Loading branch information
chemelnucfin committed Dec 22, 2017
1 parent 465a888 commit a5a2f5a
Show file tree
Hide file tree
Showing 9 changed files with 25 additions and 183 deletions.
2 changes: 0 additions & 2 deletions firestore/google/cloud/firestore.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -42,7 +41,6 @@
'AdminClient',
'Client',
'CollectionReference',
'CreateIfMissingOption',
'DELETE_FIELD',
'DocumentReference',
'DocumentSnapshot',
Expand Down
2 changes: 0 additions & 2 deletions firestore/google/cloud/firestore_v1beta1/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -46,7 +45,6 @@
'AdminClient',
'Client',
'CollectionReference',
'CreateIfMissingOption',
'DELETE_FIELD',
'DocumentReference',
'DocumentSnapshot',
Expand Down
8 changes: 2 additions & 6 deletions firestore/google/cloud/firestore_v1beta1/_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.'
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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

Expand Down
77 changes: 5 additions & 72 deletions firestore/google/cloud/firestore_v1beta1/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.'
Expand Down Expand Up @@ -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 two 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``
Expand All @@ -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.
Expand All @@ -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)
Expand Down Expand Up @@ -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.
Expand Down
7 changes: 1 addition & 6 deletions firestore/google/cloud/firestore_v1beta1/document.py
Original file line number Diff line number Diff line change
Expand Up @@ -377,19 +377,14 @@ 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
request was received by the server. If the document did not exist
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():
Expand Down
9 changes: 5 additions & 4 deletions firestore/tests/system.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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
Expand Down Expand Up @@ -207,7 +207,8 @@ 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)
Expand All @@ -223,7 +224,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).
Expand Down
24 changes: 9 additions & 15 deletions firestore/tests/unit/test__helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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(
Expand All @@ -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 = {'current_document' : {'exists': option._exists}}

expected_update_pb = write_pb2.Write(
update=document_pb2.Document(
name=document_path,
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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):

Expand Down
68 changes: 1 addition & 67 deletions firestore/tests/unit/test_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,))
Expand Down Expand Up @@ -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
Expand Down
Loading

0 comments on commit a5a2f5a

Please sign in to comment.