googleapis / python-firestore

Apache License 2.0
218 stars 75 forks source link

Cannot use field names with dashes when using update #959

Open plammens opened 2 months ago

plammens commented 2 months ago

Environment details

Steps to reproduce

  1. Do doc.update with some keys containing dashes. Either directly or wrapping in a FieldPath.

Code example

from firebase_admin import firestore, initialize_app

app = initialize_app()
firestore_instance = firestore.client()
doc = firestore_instance.collection("foo").document("bar")
doc.update({"spam-eggs": 123})

This gives an error because keys are interpreted as field paths with dots and whatnot.

But I can't explicitly specify the field path either to allow field names with special characters:

from google.cloud.firestore_v1.field_path import FieldPath

doc.update({FieldPath("spam-eggs"): 123})

So there is no way to update fields with dashes. The operation doc.set(..., merge=True) has different semantics, namely it creates the document if it doesn't exist.

Stack trace

``` > Traceback (most recent call last): > File "C:\Users\Paolo\Code\AndroidStudioProjects\commitments_app\functions\venv\Lib\site-packages\flask\app.py", line 1473, in wsgi_app > response = self.full_dispatch_request() > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > File "C:\Users\Paolo\Code\AndroidStudioProjects\commitments_app\functions\venv\Lib\site-packages\flask\app.py", line 882, in full_dispatch_request > rv = self.handle_user_exception(e) > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > File "C:\Users\Paolo\Code\AndroidStudioProjects\commitments_app\functions\venv\Lib\site-packages\flask\app.py", line 880, in full_dispatch_request > rv = self.dispatch_request() > ^^^^^^^^^^^^^^^^^^^^^^^ > File "C:\Users\Paolo\Code\AndroidStudioProjects\commitments_app\functions\venv\Lib\site-packages\flask\app.py", line 865, in dispatch_request > return self.ensure_sync(self.view_functions[rule.endpoint])(**view_args) # type: ignore[no-any-return] > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > File "C:\Users\Paolo\Code\AndroidStudioProjects\commitments_app\functions\venv\Lib\site-packages\functions_framework\execution_id.py", line 106, in wrapper > return view_function(*args, **kwargs) > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > File "C:\Users\Paolo\Code\AndroidStudioProjects\commitments_app\functions\venv\Lib\site-packages\functions_framework\__init__.py", line 142, in view_func > return function(request._get_current_object()) > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > File "C:\Users\Paolo\Code\AndroidStudioProjects\commitments_app\functions\venv\Lib\site-packages\firebase_functions\https_fn.py", line 447, in on_request_wrapped > return _core._with_init(func)(request) > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > File "C:\Users\Paolo\Code\AndroidStudioProjects\commitments_app\functions\venv\Lib\site-packages\firebase_functions\core.py", line 125, in wrapper > return fn(*args, **kwargs) > ^^^^^^^^^^^^^^^^^^^ > File "C:\Users\Paolo\Code\AndroidStudioProjects\commitments_app\functions\main.py", line 43, in google_calendar_webhook > doc_ref.update(updates) > File "C:\Users\Paolo\Code\AndroidStudioProjects\commitments_app\functions\venv\Lib\site-packages\google\cloud\firestore_v1\document.py", line 323, in update > batch, kwargs = self._prep_update(field_updates, option, retry, timeout) > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > File "C:\Users\Paolo\Code\AndroidStudioProjects\commitments_app\functions\venv\Lib\site-packages\google\cloud\firestore_v1\base_document.py", line 236, in _prep_update > batch.update(self, field_updates, option=option) > File "C:\Users\Paolo\Code\AndroidStudioProjects\commitments_app\functions\venv\Lib\site-packages\google\cloud\firestore_v1\base_batch.py", line 142, in update > write_pbs = _helpers.pbs_for_update( > ^^^^^^^^^^^^^^^^^^^^^^^^ > File "C:\Users\Paolo\Code\AndroidStudioProjects\commitments_app\functions\venv\Lib\site-packages\google\cloud\firestore_v1\_helpers.py", line 932, in pbs_for_update > extractor = DocumentExtractorForUpdate(field_updates) > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > File "C:\Users\Paolo\Code\AndroidStudioProjects\commitments_app\functions\venv\Lib\site-packages\google\cloud\firestore_v1\_helpers.py", line 883, in __init__ > super(DocumentExtractorForUpdate, self).__init__(document_data) > File "C:\Users\Paolo\Code\AndroidStudioProjects\commitments_app\functions\venv\Lib\site-packages\google\cloud\firestore_v1\_helpers.py", line 513, in __init__ > for field_path, value in iterator: > File "C:\Users\Paolo\Code\AndroidStudioProjects\commitments_app\functions\venv\Lib\site-packages\google\cloud\firestore_v1\_helpers.py", line 453, in extract_fields > sub_key = FieldPath.from_string(key) > ^^^^^^^^^^^^^^^^^^^^^^^^^^ > File "C:\Users\Paolo\Code\AndroidStudioProjects\commitments_app\functions\venv\Lib\site-packages\google\cloud\firestore_v1\field_path.py", line 306, in from_string > return cls.from_api_repr(path_string) > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > File "C:\Users\Paolo\Code\AndroidStudioProjects\commitments_app\functions\venv\Lib\site-packages\google\cloud\firestore_v1\field_path.py", line 285, in from_api_repr > api_repr = api_repr.strip() > ^^^^^^^^^^^^^^ > AttributeError: 'FieldPath' object has no attribute 'strip' ```
gkevinzheng commented 2 months ago

@plammens Looks like for update, you'll have to escape spam-eggs with backticks, like doc.update({"`spam-eggs`": 123}).

It definitely should be more consistent, because for set, you'd do doc.set({"spam-eggs": 123}) (and backticks get put into the field name) but for update you need to do the backtick escape. The rationale behind needing the backtick escape might be explained here in the proto file for documents: https://github.com/googleapis/googleapis/blob/9f96d27b2eb1db1943747c3a241b2f2443e4a75b/google/firestore/v1/document.proto#L41-L64