Skip to content

Commit

Permalink
Implement MergeOption as an option (#4851)
Browse files Browse the repository at this point in the history
Remove `CreateIfMissing` option

Closes #4111.
  • Loading branch information
chemelnucfin authored and tseaver committed Apr 10, 2018
1 parent 0b3a63f commit dd4b646
Show file tree
Hide file tree
Showing 12 changed files with 320 additions and 330 deletions.
2 changes: 0 additions & 2 deletions firestore/google/cloud/firestore.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
from google.cloud.firestore_v1beta1 import __version__
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 @@ -40,7 +39,6 @@
'__version__',
'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 @@ -41,7 +40,6 @@
'__version__',
'Client',
'CollectionReference',
'CreateIfMissingOption',
'DELETE_FIELD',
'DocumentReference',
'DocumentSnapshot',
Expand Down
107 changes: 68 additions & 39 deletions firestore/google/cloud/firestore_v1beta1/_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,9 @@
import grpc
import six

from google.cloud import exceptions
from google.cloud._helpers import _datetime_to_pb_timestamp
from google.cloud._helpers import _pb_timestamp_to_datetime
from google.cloud import exceptions

from google.cloud.firestore_v1beta1 import constants
from google.cloud.firestore_v1beta1.gapic import enums
from google.cloud.firestore_v1beta1.proto import common_pb2
Expand All @@ -43,10 +42,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 @@ -131,6 +126,13 @@ def __init__(self, *parts):
raise ValueError(error)
self.parts = tuple(parts)

def __repr__(self):
paths = ""
for part in self.parts:
paths += "'" + part + "',"
paths = paths[:-1]
return 'FieldPath({})'.format(paths)

@staticmethod
def from_string(string):
""" Creates a FieldPath from a unicode string representation.
Expand Down Expand Up @@ -768,7 +770,7 @@ def get_doc_id(document_pb, expected_prefix):
return document_id


def remove_server_timestamp(document_data):
def process_server_timestamp(document_data, split_on_dots=True):
"""Remove all server timestamp sentinel values from data.
If the data is nested, for example:
Expand All @@ -790,7 +792,7 @@ def remove_server_timestamp(document_data):
.. code-block:: python
>>> field_paths, actual_data = remove_server_timestamp(data)
>>> field_paths, actual_data = process_server_timestamp(data)
>>> field_paths
['top1.bottom2', 'top4']
>>> actual_data
Expand All @@ -802,37 +804,52 @@ def remove_server_timestamp(document_data):
}
Args:
document_data (dict): Property names and values to use for
sending a change to a document.
document_data (dict):
Property names and values to use for sending a change to
a document.
split_on_dots (bool):
Whether to split the property names on dots at the top level
(for updates only).
Returns:
Tuple[List[str, ...], Dict[str, Any]]: A two-tuple of
* A list of all field paths that use the server timestamp sentinel
* A list of all transform paths that use the server timestamp sentinel
* The remaining keys in ``document_data`` after removing the
server timestamp sentinels
"""
field_paths = []
transform_paths = []
actual_data = {}
for field_name, value in six.iteritems(document_data):
if split_on_dots:
top_level_path = FieldPath(*field_name.split("."))
else:
top_level_path = FieldPath.from_string(field_name)
if isinstance(value, dict):
sub_field_paths, sub_data = remove_server_timestamp(value)
field_paths.extend(
get_field_path([field_name, sub_path])
for sub_path in sub_field_paths
)
sub_transform_paths, sub_data, sub_field_paths = (
process_server_timestamp(value, False))
for sub_transform_path in sub_transform_paths:
transform_path = FieldPath.from_string(field_name)
transform_path.parts = (
transform_path.parts + sub_transform_path.parts)
transform_paths.extend([transform_path])
if sub_data:
# Only add a key to ``actual_data`` if there is data.
actual_data[field_name] = sub_data
for sub_field_path in sub_field_paths:
field_path = FieldPath.from_string(field_name)
field_path.parts = field_path.parts + sub_field_path.parts
field_paths.append(field_path)
elif value is constants.SERVER_TIMESTAMP:
field_paths.append(field_name)
transform_paths.append(top_level_path)
else:
actual_data[field_name] = value

if field_paths:
return field_paths, actual_data
else:
return field_paths, document_data
field_paths.append(top_level_path)
if not transform_paths:
actual_data = document_data
return transform_paths, actual_data, field_paths


def get_transform_pb(document_path, transform_paths):
Expand All @@ -849,6 +866,7 @@ def get_transform_pb(document_path, transform_paths):
google.cloud.firestore_v1beta1.types.Write: A
``Write`` protobuf instance for a document transform.
"""
transform_paths = canonicalize_field_paths(transform_paths)
return write_pb2.Write(
transform=write_pb2.DocumentTransform(
document=document_path,
Expand All @@ -857,44 +875,52 @@ def get_transform_pb(document_path, transform_paths):
field_path=field_path,
set_to_server_value=REQUEST_TIME_ENUM,
)
# Sort transform_paths so test comparision works.
for field_path in sorted(transform_paths)
for field_path in transform_paths
],
),
)


def pbs_for_set(document_path, document_data, option):
def pbs_for_set(document_path, document_data, merge=False, exists=None):
"""Make ``Write`` protobufs for ``set()`` methods.
Args:
document_path (str): A fully-qualified document path.
document_data (dict): Property names and values to use for
replacing a document.
option (optional[~.firestore_v1beta1.client.WriteOption]): A
write option to make assertions / preconditions on the server
state of the document before applying changes.
merge (bool): Whether to merge the fields or replace them
exists (bool): If set, a precondition to indicate whether the
document should exist or not. Used for create.
Returns:
List[google.cloud.firestore_v1beta1.types.Write]: One
or two ``Write`` protobuf instances for ``set()``.
"""
transform_paths, actual_data = remove_server_timestamp(document_data)

transform_paths, actual_data, field_paths = process_server_timestamp(
document_data, False)
update_pb = write_pb2.Write(
update=document_pb2.Document(
name=document_path,
fields=encode_dict(actual_data),
),
)
if option is not None:
option.modify_write(update_pb)
if exists is not None:
update_pb.current_document.CopyFrom(
common_pb2.Precondition(exists=exists))

if merge:
field_paths = canonicalize_field_paths(field_paths)
mask = common_pb2.DocumentMask(field_paths=sorted(field_paths))
update_pb.update_mask.CopyFrom(mask)

write_pbs = [update_pb]
if transform_paths:
# NOTE: We **explicitly** don't set any write option on
# the ``transform_pb``.
transform_pb = get_transform_pb(document_path, transform_paths)
if not actual_data:
write_pbs = [transform_pb]
return write_pbs
write_pbs.append(transform_pb)

return write_pbs
Expand All @@ -916,7 +942,8 @@ def canonicalize_field_paths(field_paths):
.. _Document: https://cloud.google.com/firestore/docs/reference/rpc/google.firestore.v1beta1#google.firestore.v1beta1.Document # NOQA
"""
return [path.to_api_repr() for path in field_paths]
field_paths = [path.to_api_repr() for path in field_paths]
return sorted(field_paths) # for testing purposes


def pbs_for_update(client, document_path, field_updates, option):
Expand All @@ -938,9 +965,12 @@ 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)
transform_paths, actual_updates, field_paths = (
process_server_timestamp(field_updates))
if not (transform_paths or actual_updates):
raise ValueError('There are only ServerTimeStamp objects or is empty.')
update_values, field_paths = FieldPathHelper.to_field_paths(actual_updates)
field_paths = canonicalize_field_paths(field_paths)

Expand All @@ -949,11 +979,10 @@ def pbs_for_update(client, document_path, field_updates, option):
name=document_path,
fields=encode_dict(update_values),
),
# Sort field_paths just for comparison in tests.
update_mask=common_pb2.DocumentMask(field_paths=sorted(field_paths)),
update_mask=common_pb2.DocumentMask(field_paths=field_paths),
)
# Due to the default, we don't have to check if ``None``.
option.modify_write(update_pb)
option.modify_write(update_pb, field_paths=field_paths)
write_pbs = [update_pb]

if transform_paths:
Expand All @@ -980,7 +1009,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
23 changes: 12 additions & 11 deletions firestore/google/cloud/firestore_v1beta1/batch.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,11 @@ def create(self, reference, document_data):
document_data (dict): Property names and values to use for
creating a document.
"""
option = self._client.write_option(exists=False)
self.set(reference, document_data, option=option)
write_pbs = _helpers.pbs_for_set(
reference._document_path, document_data, merge=False, exists=False)
self._add_write_pbs(write_pbs)

def set(self, reference, document_data, option=None):
def set(self, reference, document_data, merge=False):
"""Add a "change" to replace a document.
See
Expand All @@ -69,16 +70,16 @@ def set(self, reference, document_data, option=None):
applied.
Args:
reference (~.firestore_v1beta1.document.DocumentReference): A
document reference that will have values set in this batch.
document_data (dict): Property names and values to use for
replacing a document.
option (Optional[~.firestore_v1beta1.client.WriteOption]): A
write option to make assertions / preconditions on the server
state of the document before applying changes.
reference (~.firestore_v1beta1.document.DocumentReference):
A document reference that will have values set in this batch.
document_data (dict):
Property names and values to use for replacing a document.
merge (Optional[bool]):
If True, apply merging instead of overwriting the state
of the document.
"""
write_pbs = _helpers.pbs_for_set(
reference._document_path, document_data, option)
reference._document_path, document_data, merge=merge)
self._add_write_pbs(write_pbs)

def update(self, reference, field_updates, option=None):
Expand Down
Loading

0 comments on commit dd4b646

Please sign in to comment.