Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FieldPath.from_string does not accept UUIDs #1012

Closed
zaphod72 opened this issue Jan 29, 2025 · 4 comments · Fixed by #1021
Closed

FieldPath.from_string does not accept UUIDs #1012

zaphod72 opened this issue Jan 29, 2025 · 4 comments · Fixed by #1021
Assignees
Labels
type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@zaphod72
Copy link

The docs for FieldPath say

This method splits on the character . and disallows the characters ``*/[]. To create a FieldPath whose components have those characters, call the constructor.

When I try using a UUID string (ex: a344a6ea-9911-40a8-bd99-00f0b8d61155) it fails with:
"Non-alphanum char in element with leading alpha: {}"

The regex used is at

_LEADING_ALPHA_INVALID = re.compile("^[_a-zA-Z][_a-zA-Z0-9]*[^_a-zA-Z0-9]")

@product-auto-label product-auto-label bot added the api: firestore Issues related to the googleapis/python-firestore API. label Jan 29, 2025
@zaphod72
Copy link
Author

zaphod72 commented Jan 29, 2025

My repro hits this code path in a Transaction. I haven't found a different way to repro.

client = firestore.Client()
doc_ref = client.collection("test").document("uuid")

key = str(uuid.uuid4())
doc_ref.set({key: "I'm a UUID!"})
print(doc_ref.get().to_dict())


@firestore.transactional
def update_doc(tx: Transaction, doc_ref: DocumentReference, key: str, value: str) -> None:
    tx.update(doc_ref, {key: value})  # barfs here


update_doc(client.transaction(), doc_ref, key, "Uh oh!")
print(doc_ref.get().to_dict())

Stack trace:

  File "test.py", line 89, in update_doc
    tx.update(doc_ref, {key: value})
  File ".venv/lib/python3.12/site-packages/google/cloud/firestore_v1/base_batch.py", line 142, in update
    write_pbs = _helpers.pbs_for_update(
                ^^^^^^^^^^^^^^^^^^^^^^^^
  File ".venv/lib/python3.12/site-packages/google/cloud/firestore_v1/_helpers.py", line 932, in pbs_for_update
    extractor = DocumentExtractorForUpdate(field_updates)
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File ".venv/lib/python3.12/site-packages/google/cloud/firestore_v1/_helpers.py", line 883, in __init__
    super(DocumentExtractorForUpdate, self).__init__(document_data)
  File ".venv/lib/python3.12/site-packages/google/cloud/firestore_v1/_helpers.py", line 513, in __init__
    for field_path, value in iterator:
                             ^^^^^^^^
  File ".venv/lib/python3.12/site-packages/google/cloud/firestore_v1/_helpers.py", line 453, in extract_fields
    sub_key = FieldPath.from_string(key)
              ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File ".venv/lib/python3.12/site-packages/google/cloud/firestore_v1/field_path.py", line 313, in from_string
    raise ValueError(
ValueError: Non-alphanum char in element with leading alpha: ca5530b5-41b8-4bf8-8386-10a815ae545f

Really odd that this succeeds:
FieldPath.from_string(str(uuid.uuid4()))

@zaphod72
Copy link
Author

zaphod72 commented Jan 29, 2025

Sanity check:

from google.cloud.firestore_v1.field_path import _LEADING_ALPHA_INVALID

test_string = "c70d4541-37f6-4594-afbf-9f35af7afa84"

if _LEADING_ALPHA_INVALID.match(test_string):
    raise ValueError("The string matches the invalid pattern!")
else:
    print("The string is valid.")

@daniel-sanche
Copy link
Contributor

Thanks for the report

It looks like part of this error makes sense: dashes aren't valid in FieldPaths, unless surrouned by backticks:

Must separate field names with a single period (.)
May be passed as a dot-delimited (.) string of segments where each segment is either a simple field name or a quoted field name (defined below).
A simple field name is one where all of the following are true:
- Contains only the characters a-z, A-Z, 0-9, and underscore (_)
- Does not start with 0-9

So it makes sense why FieldPath.from_string("a344a6ea-9911-40a8-bd99-00f0b8d61155") would fail

But in your transaction code, you aren't trying to use FieldPaths directly, it seems like an internal call is building them. And I don't see any constraints on having field dashes in field names, only in path references. So it looks like the library is treating the field as a path, when it should be treated as a name directly. I'll take a look

As a workaround, you could try adding backticks to your key: key = f"`{uuid.uuid4()}`". I'm not sure if that will work, but based on how FieldPaths are implemented, it might worth trying

@daniel-sanche daniel-sanche added type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. priority: p2 Moderately-important priority. Fix may not be included in next release. and removed api: firestore Issues related to the googleapis/python-firestore API. priority: p2 Moderately-important priority. Fix may not be included in next release. labels Feb 19, 2025
@daniel-sanche
Copy link
Contributor

After looking at a number of places in the stack to make a fix, it seems like the simplest fix is to loosen the _LEADING_ALPHA_INVALID regex. I opened a PR, and added your code as a new test case

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants