From fcffa278b96e56309798328dedeac94055696e66 Mon Sep 17 00:00:00 2001 From: ohmayr Date: Wed, 16 Sep 2026 08:43:41 +0000 Subject: [PATCH 01/21] feat(firestore): add BSONRegex support --- .../firestore-integration.yaml | 4 + .../google/cloud/firestore/__init__.py | 2 + .../google/cloud/firestore_v1/__init__.py | 2 + .../google/cloud/firestore_v1/bson.py | 83 +++++++++++++++++++ .../tests/system/test_system.py | 8 ++ .../tests/system/test_system_async.py | 8 ++ .../tests/unit/v1/test_bson.py | 71 ++++++++++++++++ 7 files changed, 178 insertions(+) diff --git a/.librarian/generator-input/client-post-processing/firestore-integration.yaml b/.librarian/generator-input/client-post-processing/firestore-integration.yaml index 429d658d8e96..49fd20d5f427 100644 --- a/.librarian/generator-input/client-post-processing/firestore-integration.yaml +++ b/.librarian/generator-input/client-post-processing/firestore-integration.yaml @@ -75,6 +75,7 @@ replacements: BSONMaxKey, BSONMinKey, BSONObjectId, + BSONRegex, BSONTimestamp, ) from google.cloud.firestore_v1.client import Client @@ -183,6 +184,7 @@ replacements: "BSONMaxKey", "BSONMinKey", "BSONObjectId", + "BSONRegex", "BSONTimestamp", "Client", "CountAggregation", @@ -261,6 +263,7 @@ replacements: BSONMaxKey, BSONMinKey, BSONObjectId, + BSONRegex, BSONTimestamp, Client, CollectionGroup, @@ -324,6 +327,7 @@ replacements: "BSONMaxKey", "BSONMinKey", "BSONObjectId", + "BSONRegex", "BSONTimestamp", "Client", "CountAggregation", diff --git a/packages/google-cloud-firestore/google/cloud/firestore/__init__.py b/packages/google-cloud-firestore/google/cloud/firestore/__init__.py index e89628a94ac1..eaa2daacd0f1 100644 --- a/packages/google-cloud-firestore/google/cloud/firestore/__init__.py +++ b/packages/google-cloud-firestore/google/cloud/firestore/__init__.py @@ -40,6 +40,7 @@ BSONMaxKey, BSONMinKey, BSONObjectId, + BSONRegex, BSONTimestamp, Client, CollectionGroup, @@ -103,6 +104,7 @@ "BSONMaxKey", "BSONMinKey", "BSONObjectId", + "BSONRegex", "BSONTimestamp", "Client", "CountAggregation", diff --git a/packages/google-cloud-firestore/google/cloud/firestore_v1/__init__.py b/packages/google-cloud-firestore/google/cloud/firestore_v1/__init__.py index 9d629e864507..e7445eeedf3a 100644 --- a/packages/google-cloud-firestore/google/cloud/firestore_v1/__init__.py +++ b/packages/google-cloud-firestore/google/cloud/firestore_v1/__init__.py @@ -52,6 +52,7 @@ BSONMaxKey, BSONMinKey, BSONObjectId, + BSONRegex, BSONTimestamp, ) from google.cloud.firestore_v1.client import Client @@ -160,6 +161,7 @@ "BSONMaxKey", "BSONMinKey", "BSONObjectId", + "BSONRegex", "BSONTimestamp", "Client", "CountAggregation", diff --git a/packages/google-cloud-firestore/google/cloud/firestore_v1/bson.py b/packages/google-cloud-firestore/google/cloud/firestore_v1/bson.py index fecf9d2aea7c..40b7ef346566 100644 --- a/packages/google-cloud-firestore/google/cloud/firestore_v1/bson.py +++ b/packages/google-cloud-firestore/google/cloud/firestore_v1/bson.py @@ -35,6 +35,7 @@ "BSONInt32", "BSONBinary", "BSONTimestamp", + "BSONRegex", ] _OBJECT_ID_BYTES_LEN = 12 @@ -342,3 +343,85 @@ def __eq__(self, other: Any) -> bool: def __hash__(self) -> int: return hash((type(self), self._seconds, self._increment)) + + +class BSONRegex(_BSONType): + """Represents a BSON Regular Expression container for Firestore. + + Args: + pattern (str): The regular expression pattern string. + options (Union[str, re.RegexFlag, int], optional): BSON regex option flags + as a string (e.g. "i", "m", "s") or Python `re` flag integer (e.g. `re.I | re.M`). + Defaults to "". + + Raises: + TypeError: If pattern is not a string or options is invalid type. + + Example: + >>> regex = BSONRegex("^hello.*$", options="i") + >>> regex.pattern + '^hello.*$' + >>> regex.options + 'i' + """ + + __slots__ = ("_pattern", "_options") + + _FLAG_TO_OPTION: Dict[int, str] = { + re.IGNORECASE: "i", + re.LOCALE: "l", + re.MULTILINE: "m", + re.DOTALL: "s", + re.UNICODE: "u", + re.VERBOSE: "x", + } + + def __init__(self, pattern: str, options: Union[str, re.RegexFlag, int] = ""): + if not isinstance(pattern, str): + raise TypeError("BSONRegex pattern must be a str.") + + if isinstance(options, bool): + raise TypeError("BSONRegex options must be a str or re flag integer.") + + if isinstance(options, str): + self._options: str = "".join(sorted(set(options))) + elif isinstance(options, int): + opts = [] + for flag, char in self._FLAG_TO_OPTION.items(): + if options & flag: + opts.append(char) + self._options = "".join(sorted(opts)) + else: + raise TypeError("BSONRegex options must be a str or re flag integer.") + + self._pattern: str = pattern + + @property + def pattern(self) -> str: + """str: The regular expression pattern string.""" + return self._pattern + + @property + def options(self) -> str: + """str: The normalized BSON regex option flags sorted alphabetically.""" + return self._options + + def _to_map_value(self) -> Dict[str, Dict[str, str]]: + """Returns map dictionary representation for wire serialization.""" + return { + "__regex__": { + "pattern": self._pattern, + "options": self._options, + } + } + + def __repr__(self) -> str: + return f"BSONRegex({self._pattern!r}, options={self._options!r})" + + def __eq__(self, other: Any) -> bool: + if isinstance(other, BSONRegex): + return self._pattern == other._pattern and self._options == other._options + return NotImplemented + + def __hash__(self) -> int: + return hash((type(self), self._pattern, self._options)) diff --git a/packages/google-cloud-firestore/tests/system/test_system.py b/packages/google-cloud-firestore/tests/system/test_system.py index 7997c95265b3..bee648ace9b3 100644 --- a/packages/google-cloud-firestore/tests/system/test_system.py +++ b/packages/google-cloud-firestore/tests/system/test_system.py @@ -54,6 +54,7 @@ BSONMaxKey, BSONMinKey, BSONObjectId, + BSONRegex, BSONTimestamp, ) from google.cloud.firestore_v1.vector import Vector @@ -1296,6 +1297,7 @@ def test_bson_document_writes(client, cleanup, database): "int32_val": BSONInt32(42), "binary_val_sub128": BSONBinary(b"world", subtype=128), "timestamp_val": BSONTimestamp(1700000000, 1), + "regex_val": BSONRegex("^hello.*$", options="i"), } doc_ref.set(bson_payload) @@ -1314,6 +1316,12 @@ def test_bson_document_writes(client, cleanup, database): "increment": 1, } }, + "regex_val": { + "__regex__": { + "pattern": "^hello.*$", + "options": "i", + } + }, } diff --git a/packages/google-cloud-firestore/tests/system/test_system_async.py b/packages/google-cloud-firestore/tests/system/test_system_async.py index 24ebf2992ea0..479fa66ee860 100644 --- a/packages/google-cloud-firestore/tests/system/test_system_async.py +++ b/packages/google-cloud-firestore/tests/system/test_system_async.py @@ -57,6 +57,7 @@ BSONMaxKey, BSONMinKey, BSONObjectId, + BSONRegex, BSONTimestamp, ) from google.cloud.firestore_v1.query_profile import ( @@ -1269,6 +1270,7 @@ async def test_async_bson_document_writes(client, cleanup, database): "int32_val": BSONInt32(42), "binary_val_sub128": BSONBinary(b"world", subtype=128), "timestamp_val": BSONTimestamp(1700000000, 1), + "regex_val": BSONRegex("^hello.*$", options="i"), } await doc_ref.set(bson_payload) @@ -1287,6 +1289,12 @@ async def test_async_bson_document_writes(client, cleanup, database): "increment": 1, } }, + "regex_val": { + "__regex__": { + "pattern": "^hello.*$", + "options": "i", + } + }, } diff --git a/packages/google-cloud-firestore/tests/unit/v1/test_bson.py b/packages/google-cloud-firestore/tests/unit/v1/test_bson.py index d90328d4cc1c..b5be269f0061 100644 --- a/packages/google-cloud-firestore/tests/unit/v1/test_bson.py +++ b/packages/google-cloud-firestore/tests/unit/v1/test_bson.py @@ -17,6 +17,7 @@ import copy import pickle +import re import pytest @@ -26,6 +27,7 @@ BSONMaxKey, BSONMinKey, BSONObjectId, + BSONRegex, BSONTimestamp, _BSONType, ) @@ -411,3 +413,72 @@ def test_bson_timestamp_copy(): def test_bson_timestamp_pickle(): ts = BSONTimestamp(100, 1) assert pickle.loads(pickle.dumps(ts)) == ts + + +def test_bson_regex_valid(): + rx = BSONRegex("^hello.*$", options="i") + assert rx.pattern == "^hello.*$" + assert rx.options == "i" + assert rx._to_map_value() == { + "__regex__": { + "pattern": "^hello.*$", + "options": "i", + } + } + assert repr(rx) == "BSONRegex('^hello.*$', options='i')" + + +def test_bson_regex_options_sorting_and_deduplication(): + rx1 = BSONRegex("foo", options="msi") + assert rx1.options == "ims" + + rx2 = BSONRegex("foo", options="mmiis") + assert rx2.options == "ims" + + +def test_bson_regex_options_from_re_flags(): + rx = BSONRegex("foo", options=re.IGNORECASE | re.MULTILINE) + assert rx.options == "im" + + +@pytest.mark.parametrize( + "pattern_input, options_input, exc_type, match_msg", + [ + (123, "i", TypeError, "pattern must be a str"), + (None, "i", TypeError, "pattern must be a str"), + ("foo", True, TypeError, "options must be a str or re flag integer"), + ("foo", [1, 2], TypeError, "options must be a str or re flag integer"), + ], +) +def test_bson_regex_invalid_inputs(pattern_input, options_input, exc_type, match_msg): + with pytest.raises(exc_type, match=match_msg): + BSONRegex(pattern_input, options_input) + + +def test_bson_regex_equality(): + rx1 = BSONRegex("^abc", options="i") + rx2 = BSONRegex("^abc", options="i") + rx3 = BSONRegex("^abc", options="m") + rx4 = BSONRegex("^xyz", options="i") + assert rx1 == rx2 + assert rx1 != rx3 + assert rx1 != rx4 + assert rx1 != "^abc" + + +def test_bson_regex_hash_and_dict_key(): + rx1 = BSONRegex("^abc", options="i") + rx2 = BSONRegex("^abc", options="i") + assert hash(rx1) == hash(rx2) + assert len({rx1, rx2}) == 1 + + +def test_bson_regex_copy(): + rx = BSONRegex("^abc", options="i") + assert copy.copy(rx) == rx + assert copy.deepcopy(rx) == rx + + +def test_bson_regex_pickle(): + rx = BSONRegex("^abc", options="i") + assert pickle.loads(pickle.dumps(rx)) == rx From d133864b3e6ee55d57a9d40a4aef8d46d0e39049 Mon Sep 17 00:00:00 2001 From: ohmayr Date: Mon, 21 Sep 2026 08:13:16 +0000 Subject: [PATCH 02/21] refactor(firestore): address review feedback for BSONRegex options validation - Restrict BSONRegex options parameter to string only, removing re.RegexFlag and int support. - Add client-side validation for BSON regex option characters ("i", "m", "s", "x", "u", "a"), raising ValueError on invalid options. - Remove obsolete re-flag test and clean up unused import in test_bson.py. --- .../google/cloud/firestore_v1/bson.py | 44 +++++++------------ .../tests/unit/v1/test_bson.py | 13 +++--- 2 files changed, 22 insertions(+), 35 deletions(-) diff --git a/packages/google-cloud-firestore/google/cloud/firestore_v1/bson.py b/packages/google-cloud-firestore/google/cloud/firestore_v1/bson.py index 40b7ef346566..f887a6491d83 100644 --- a/packages/google-cloud-firestore/google/cloud/firestore_v1/bson.py +++ b/packages/google-cloud-firestore/google/cloud/firestore_v1/bson.py @@ -26,7 +26,7 @@ import abc import re -from typing import Any, Dict, Union +from typing import Any, Dict, FrozenSet, Union __all__ = [ "BSONObjectId", @@ -350,12 +350,12 @@ class BSONRegex(_BSONType): Args: pattern (str): The regular expression pattern string. - options (Union[str, re.RegexFlag, int], optional): BSON regex option flags - as a string (e.g. "i", "m", "s") or Python `re` flag integer (e.g. `re.I | re.M`). - Defaults to "". + options (str, optional): BSON regex option flags as a string + (e.g. "i", "m", "s", "x", "u", "a"). Defaults to "". Raises: - TypeError: If pattern is not a string or options is invalid type. + TypeError: If pattern is not a string or options is not a string. + ValueError: If options contains invalid BSON regex flag characters. Example: >>> regex = BSONRegex("^hello.*$", options="i") @@ -367,34 +367,24 @@ class BSONRegex(_BSONType): __slots__ = ("_pattern", "_options") - _FLAG_TO_OPTION: Dict[int, str] = { - re.IGNORECASE: "i", - re.LOCALE: "l", - re.MULTILINE: "m", - re.DOTALL: "s", - re.UNICODE: "u", - re.VERBOSE: "x", - } + _VALID_OPTIONS: FrozenSet[str] = frozenset({"i", "m", "s", "x", "u", "a"}) - def __init__(self, pattern: str, options: Union[str, re.RegexFlag, int] = ""): + def __init__(self, pattern: str, options: str = ""): if not isinstance(pattern, str): raise TypeError("BSONRegex pattern must be a str.") - if isinstance(options, bool): - raise TypeError("BSONRegex options must be a str or re flag integer.") - - if isinstance(options, str): - self._options: str = "".join(sorted(set(options))) - elif isinstance(options, int): - opts = [] - for flag, char in self._FLAG_TO_OPTION.items(): - if options & flag: - opts.append(char) - self._options = "".join(sorted(opts)) - else: - raise TypeError("BSONRegex options must be a str or re flag integer.") + if not isinstance(options, str): + raise TypeError("BSONRegex options must be a str.") + + invalid = set(options) - self._VALID_OPTIONS + if invalid: + raise ValueError( + f"Invalid BSON regex option(s): {sorted(invalid)}. " + f"Valid options are: {sorted(self._VALID_OPTIONS)}" + ) self._pattern: str = pattern + self._options: str = "".join(sorted(set(options))) @property def pattern(self) -> str: diff --git a/packages/google-cloud-firestore/tests/unit/v1/test_bson.py b/packages/google-cloud-firestore/tests/unit/v1/test_bson.py index b5be269f0061..4d7610892357 100644 --- a/packages/google-cloud-firestore/tests/unit/v1/test_bson.py +++ b/packages/google-cloud-firestore/tests/unit/v1/test_bson.py @@ -17,7 +17,6 @@ import copy import pickle -import re import pytest @@ -436,18 +435,16 @@ def test_bson_regex_options_sorting_and_deduplication(): assert rx2.options == "ims" -def test_bson_regex_options_from_re_flags(): - rx = BSONRegex("foo", options=re.IGNORECASE | re.MULTILINE) - assert rx.options == "im" - - @pytest.mark.parametrize( "pattern_input, options_input, exc_type, match_msg", [ (123, "i", TypeError, "pattern must be a str"), (None, "i", TypeError, "pattern must be a str"), - ("foo", True, TypeError, "options must be a str or re flag integer"), - ("foo", [1, 2], TypeError, "options must be a str or re flag integer"), + ("foo", 123, TypeError, "options must be a str"), + ("foo", True, TypeError, "options must be a str"), + ("foo", [1, 2], TypeError, "options must be a str"), + ("foo", "l", ValueError, "Invalid BSON regex option"), + ("foo", "invalid", ValueError, "Invalid BSON regex option"), ], ) def test_bson_regex_invalid_inputs(pattern_input, options_input, exc_type, match_msg): From a614d3f3e39384add57bb87f3135ce77c18ab534 Mon Sep 17 00:00:00 2001 From: ohmayr Date: Tue, 22 Sep 2026 22:58:54 +0000 Subject: [PATCH 03/21] refactor(firestore): defer BSONRegex options validation to backend Remove client-side regex options character validation and whitelist in BSONRegex, letting the Firestore backend validate regex option flags. Preserves alphabetical sorting and deduplication per BSON specification while avoiding unhandled ValueError exceptions during read deserialization when documents contain standard BSON options like 'l' (locale). Fixes: b/562163604 --- .../google/cloud/firestore_v1/bson.py | 14 ++------------ .../tests/unit/v1/test_bson.py | 5 +++-- 2 files changed, 5 insertions(+), 14 deletions(-) diff --git a/packages/google-cloud-firestore/google/cloud/firestore_v1/bson.py b/packages/google-cloud-firestore/google/cloud/firestore_v1/bson.py index f887a6491d83..0c4b8dd00b9d 100644 --- a/packages/google-cloud-firestore/google/cloud/firestore_v1/bson.py +++ b/packages/google-cloud-firestore/google/cloud/firestore_v1/bson.py @@ -26,7 +26,7 @@ import abc import re -from typing import Any, Dict, FrozenSet, Union +from typing import Any, Dict, Union __all__ = [ "BSONObjectId", @@ -351,11 +351,10 @@ class BSONRegex(_BSONType): Args: pattern (str): The regular expression pattern string. options (str, optional): BSON regex option flags as a string - (e.g. "i", "m", "s", "x", "u", "a"). Defaults to "". + (e.g. "i", "m", "s", "x", "u"). Defaults to "". Raises: TypeError: If pattern is not a string or options is not a string. - ValueError: If options contains invalid BSON regex flag characters. Example: >>> regex = BSONRegex("^hello.*$", options="i") @@ -367,8 +366,6 @@ class BSONRegex(_BSONType): __slots__ = ("_pattern", "_options") - _VALID_OPTIONS: FrozenSet[str] = frozenset({"i", "m", "s", "x", "u", "a"}) - def __init__(self, pattern: str, options: str = ""): if not isinstance(pattern, str): raise TypeError("BSONRegex pattern must be a str.") @@ -376,13 +373,6 @@ def __init__(self, pattern: str, options: str = ""): if not isinstance(options, str): raise TypeError("BSONRegex options must be a str.") - invalid = set(options) - self._VALID_OPTIONS - if invalid: - raise ValueError( - f"Invalid BSON regex option(s): {sorted(invalid)}. " - f"Valid options are: {sorted(self._VALID_OPTIONS)}" - ) - self._pattern: str = pattern self._options: str = "".join(sorted(set(options))) diff --git a/packages/google-cloud-firestore/tests/unit/v1/test_bson.py b/packages/google-cloud-firestore/tests/unit/v1/test_bson.py index 4d7610892357..79654e1669cb 100644 --- a/packages/google-cloud-firestore/tests/unit/v1/test_bson.py +++ b/packages/google-cloud-firestore/tests/unit/v1/test_bson.py @@ -434,6 +434,9 @@ def test_bson_regex_options_sorting_and_deduplication(): rx2 = BSONRegex("foo", options="mmiis") assert rx2.options == "ims" + rx3 = BSONRegex("foo", options="xl") + assert rx3.options == "lx" + @pytest.mark.parametrize( "pattern_input, options_input, exc_type, match_msg", @@ -443,8 +446,6 @@ def test_bson_regex_options_sorting_and_deduplication(): ("foo", 123, TypeError, "options must be a str"), ("foo", True, TypeError, "options must be a str"), ("foo", [1, 2], TypeError, "options must be a str"), - ("foo", "l", ValueError, "Invalid BSON regex option"), - ("foo", "invalid", ValueError, "Invalid BSON regex option"), ], ) def test_bson_regex_invalid_inputs(pattern_input, options_input, exc_type, match_msg): From 7dad0c38a52273535b69496fdc613ae5b5b64f72 Mon Sep 17 00:00:00 2001 From: ohmayr Date: Thu, 24 Sep 2026 00:03:37 +0000 Subject: [PATCH 04/21] test(firestore): add system tests for BSONRegex with invalid options Add sync and async system test cases verifying that writing a BSONRegex with unsupported options (such as 'l') fails backend validation with google.api_core.exceptions.InvalidArgument and an informative error message on an Enterprise database. Fixes: b/562163604 --- .../tests/system/test_system.py | 14 ++++++++++++++ .../tests/system/test_system_async.py | 15 +++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/packages/google-cloud-firestore/tests/system/test_system.py b/packages/google-cloud-firestore/tests/system/test_system.py index bee648ace9b3..ddc8c8a58529 100644 --- a/packages/google-cloud-firestore/tests/system/test_system.py +++ b/packages/google-cloud-firestore/tests/system/test_system.py @@ -1325,6 +1325,20 @@ def test_bson_document_writes(client, cleanup, database): } +@pytest.mark.parametrize("database", [FIRESTORE_ENTERPRISE_DB], indirect=True) +def test_bson_regex_invalid_options(client, cleanup, database): + """Test write operations for BSONRegex with invalid options against backend.""" + collection_id = "bson_regex_invalid_" + UNIQUE_RESOURCE_ID + doc_ref = client.collection(collection_id).document("invalid_regex") + cleanup(doc_ref.delete) + + # Backend enforces supported BSON regex flags ('i', 'm', 's', 'u', 'x') + # and rejects unsupported options (e.g. 'l') with InvalidArgument. + with pytest.raises(InvalidArgument) as exc_info: + doc_ref.set({"regex_val": BSONRegex("hello", options="l")}) + assert "Invalid regex option" in exc_info.value.message + + @pytest.fixture(scope="module") def query_docs(client, database): collection_id = "qs" + UNIQUE_RESOURCE_ID diff --git a/packages/google-cloud-firestore/tests/system/test_system_async.py b/packages/google-cloud-firestore/tests/system/test_system_async.py index 479fa66ee860..cca12f5eca61 100644 --- a/packages/google-cloud-firestore/tests/system/test_system_async.py +++ b/packages/google-cloud-firestore/tests/system/test_system_async.py @@ -1298,6 +1298,21 @@ async def test_async_bson_document_writes(client, cleanup, database): } +@pytest.mark.asyncio +@pytest.mark.parametrize("database", [FIRESTORE_ENTERPRISE_DB], indirect=True) +async def test_async_bson_regex_invalid_options(client, cleanup, database): + """Test async write operations for BSONRegex with invalid options against backend.""" + collection_id = "async_bson_regex_invalid_" + UNIQUE_RESOURCE_ID + doc_ref = client.collection(collection_id).document("invalid_regex") + cleanup(doc_ref.delete) + + # Backend enforces supported BSON regex flags ('i', 'm', 's', 'u', 'x') + # and rejects unsupported options (e.g. 'l') with InvalidArgument. + with pytest.raises(InvalidArgument) as exc_info: + await doc_ref.set({"regex_val": BSONRegex("hello", options="l")}) + assert "Invalid regex option" in exc_info.value.message + + @pytest_asyncio.fixture(scope="module") async def query_docs(client): collection_id = "qs" + UNIQUE_RESOURCE_ID From 9e7e5f5bc1567bb37634f5ec7c5db5ec4c4c22f1 Mon Sep 17 00:00:00 2001 From: ohmayr Date: Wed, 16 Sep 2026 08:49:01 +0000 Subject: [PATCH 05/21] feat(firestore): add BSONDecimal128 support --- .../firestore-integration.yaml | 4 + .../google/cloud/firestore/__init__.py | 2 + .../google/cloud/firestore_v1/__init__.py | 2 + .../google/cloud/firestore_v1/bson.py | 83 ++++++++++++++++ .../tests/system/test_system.py | 3 + .../tests/system/test_system_async.py | 3 + .../tests/unit/v1/test_bson.py | 97 +++++++++++++++++++ 7 files changed, 194 insertions(+) diff --git a/.librarian/generator-input/client-post-processing/firestore-integration.yaml b/.librarian/generator-input/client-post-processing/firestore-integration.yaml index 49fd20d5f427..f43581fb6130 100644 --- a/.librarian/generator-input/client-post-processing/firestore-integration.yaml +++ b/.librarian/generator-input/client-post-processing/firestore-integration.yaml @@ -71,6 +71,7 @@ replacements: from google.cloud.firestore_v1.batch import WriteBatch from google.cloud.firestore_v1.bson import ( BSONBinary, + BSONDecimal128, BSONInt32, BSONMaxKey, BSONMinKey, @@ -180,6 +181,7 @@ replacements: "AsyncTransaction", "AsyncWriteBatch", "BSONBinary", + "BSONDecimal128", "BSONInt32", "BSONMaxKey", "BSONMinKey", @@ -259,6 +261,7 @@ replacements: AsyncTransaction, AsyncWriteBatch, BSONBinary, + BSONDecimal128, BSONInt32, BSONMaxKey, BSONMinKey, @@ -323,6 +326,7 @@ replacements: "AsyncTransaction", "AsyncWriteBatch", "BSONBinary", + "BSONDecimal128", "BSONInt32", "BSONMaxKey", "BSONMinKey", diff --git a/packages/google-cloud-firestore/google/cloud/firestore/__init__.py b/packages/google-cloud-firestore/google/cloud/firestore/__init__.py index eaa2daacd0f1..f14aa807d509 100644 --- a/packages/google-cloud-firestore/google/cloud/firestore/__init__.py +++ b/packages/google-cloud-firestore/google/cloud/firestore/__init__.py @@ -36,6 +36,7 @@ AsyncTransaction, AsyncWriteBatch, BSONBinary, + BSONDecimal128, BSONInt32, BSONMaxKey, BSONMinKey, @@ -100,6 +101,7 @@ "AsyncTransaction", "AsyncWriteBatch", "BSONBinary", + "BSONDecimal128", "BSONInt32", "BSONMaxKey", "BSONMinKey", diff --git a/packages/google-cloud-firestore/google/cloud/firestore_v1/__init__.py b/packages/google-cloud-firestore/google/cloud/firestore_v1/__init__.py index e7445eeedf3a..f8a91acf9124 100644 --- a/packages/google-cloud-firestore/google/cloud/firestore_v1/__init__.py +++ b/packages/google-cloud-firestore/google/cloud/firestore_v1/__init__.py @@ -48,6 +48,7 @@ from google.cloud.firestore_v1.batch import WriteBatch from google.cloud.firestore_v1.bson import ( BSONBinary, + BSONDecimal128, BSONInt32, BSONMaxKey, BSONMinKey, @@ -157,6 +158,7 @@ "AsyncTransaction", "AsyncWriteBatch", "BSONBinary", + "BSONDecimal128", "BSONInt32", "BSONMaxKey", "BSONMinKey", diff --git a/packages/google-cloud-firestore/google/cloud/firestore_v1/bson.py b/packages/google-cloud-firestore/google/cloud/firestore_v1/bson.py index 0c4b8dd00b9d..7b3ef468f9d2 100644 --- a/packages/google-cloud-firestore/google/cloud/firestore_v1/bson.py +++ b/packages/google-cloud-firestore/google/cloud/firestore_v1/bson.py @@ -25,6 +25,7 @@ """ import abc +import decimal import re from typing import Any, Dict, Union @@ -36,6 +37,7 @@ "BSONBinary", "BSONTimestamp", "BSONRegex", + "BSONDecimal128", ] _OBJECT_ID_BYTES_LEN = 12 @@ -405,3 +407,84 @@ def __eq__(self, other: Any) -> bool: def __hash__(self) -> int: return hash((type(self), self._pattern, self._options)) + + +class BSONDecimal128(_BSONType): + """Represents a BSON 128-bit Decimal container for Firestore. + + Args: + value (Union[str, int, float, decimal.Decimal, BSONDecimal128]): + The decimal value as a string, integer, float, decimal.Decimal, + or BSONDecimal128 instance. + + Raises: + TypeError: If value is a boolean or unsupported type. + ValueError: If value cannot be parsed as a valid decimal number. + + Example: + >>> dec = BSONDecimal128("123.45") + >>> dec.value + '123.45' + >>> dec.to_decimal + Decimal('123.45') + """ + + __slots__ = ("_value",) + + def __init__( + self, + value: Union[str, int, float, decimal.Decimal, "BSONDecimal128"], + ): + if isinstance(value, BSONDecimal128): + self._value: str = value._value + elif isinstance(value, (str, int, float, decimal.Decimal)) and not isinstance( + value, bool + ): + self._value = str(value) + else: + raise TypeError( + "BSONDecimal128 value must be a Decimal, str, int, or float." + ) + + @property + def value(self) -> str: + """str: The string representation of the 128-bit decimal value.""" + return self._value + + @property + def to_decimal(self) -> decimal.Decimal: + """decimal.Decimal: Convert to Python standard library Decimal instance.""" + return decimal.Decimal(self._value) + + def _to_map_value(self) -> Dict[str, str]: + """Returns map dictionary representation for wire serialization.""" + return {"__decimal128__": self._value} + + def __repr__(self) -> str: + return f"BSONDecimal128({self._value!r})" + + def __str__(self) -> str: + return self._value + + def __eq__(self, other: Any) -> bool: + if isinstance(other, BSONDecimal128): + if self._value.upper() == "NAN" and other._value.upper() == "NAN": + return True + try: + return self.to_decimal == other.to_decimal + except decimal.InvalidOperation: + return self._value == other._value + if isinstance(other, decimal.Decimal): + try: + return self.to_decimal == other + except decimal.InvalidOperation: + return False + return NotImplemented + + def __hash__(self) -> int: + if self._value.upper() == "NAN": + return hash((type(self), "NAN")) + try: + return hash(self.to_decimal) + except decimal.InvalidOperation: + return hash((type(self), self._value)) diff --git a/packages/google-cloud-firestore/tests/system/test_system.py b/packages/google-cloud-firestore/tests/system/test_system.py index ddc8c8a58529..6608e077245a 100644 --- a/packages/google-cloud-firestore/tests/system/test_system.py +++ b/packages/google-cloud-firestore/tests/system/test_system.py @@ -50,6 +50,7 @@ from google.cloud.firestore_v1.base_vector_query import DistanceMeasure from google.cloud.firestore_v1.bson import ( BSONBinary, + BSONDecimal128, BSONInt32, BSONMaxKey, BSONMinKey, @@ -1298,6 +1299,7 @@ def test_bson_document_writes(client, cleanup, database): "binary_val_sub128": BSONBinary(b"world", subtype=128), "timestamp_val": BSONTimestamp(1700000000, 1), "regex_val": BSONRegex("^hello.*$", options="i"), + "decimal128_val": BSONDecimal128("123.45"), } doc_ref.set(bson_payload) @@ -1322,6 +1324,7 @@ def test_bson_document_writes(client, cleanup, database): "options": "i", } }, + "decimal128_val": {"__decimal128__": "123.45"}, } diff --git a/packages/google-cloud-firestore/tests/system/test_system_async.py b/packages/google-cloud-firestore/tests/system/test_system_async.py index cca12f5eca61..8adcc3b026ed 100644 --- a/packages/google-cloud-firestore/tests/system/test_system_async.py +++ b/packages/google-cloud-firestore/tests/system/test_system_async.py @@ -53,6 +53,7 @@ from google.cloud.firestore_v1.base_vector_query import DistanceMeasure from google.cloud.firestore_v1.bson import ( BSONBinary, + BSONDecimal128, BSONInt32, BSONMaxKey, BSONMinKey, @@ -1271,6 +1272,7 @@ async def test_async_bson_document_writes(client, cleanup, database): "binary_val_sub128": BSONBinary(b"world", subtype=128), "timestamp_val": BSONTimestamp(1700000000, 1), "regex_val": BSONRegex("^hello.*$", options="i"), + "decimal128_val": BSONDecimal128("123.45"), } await doc_ref.set(bson_payload) @@ -1295,6 +1297,7 @@ async def test_async_bson_document_writes(client, cleanup, database): "options": "i", } }, + "decimal128_val": {"__decimal128__": "123.45"}, } diff --git a/packages/google-cloud-firestore/tests/unit/v1/test_bson.py b/packages/google-cloud-firestore/tests/unit/v1/test_bson.py index 79654e1669cb..cc619ebec642 100644 --- a/packages/google-cloud-firestore/tests/unit/v1/test_bson.py +++ b/packages/google-cloud-firestore/tests/unit/v1/test_bson.py @@ -16,12 +16,14 @@ """Unit tests for google.cloud.firestore_v1.bson classes.""" import copy +import decimal import pickle import pytest from google.cloud.firestore_v1.bson import ( BSONBinary, + BSONDecimal128, BSONInt32, BSONMaxKey, BSONMinKey, @@ -480,3 +482,98 @@ def test_bson_regex_copy(): def test_bson_regex_pickle(): rx = BSONRegex("^abc", options="i") assert pickle.loads(pickle.dumps(rx)) == rx + + +def test_bson_decimal128_valid(): + dec1 = BSONDecimal128("123.45") + assert dec1.value == "123.45" + assert dec1.to_decimal == decimal.Decimal("123.45") + assert dec1._to_map_value() == {"__decimal128__": "123.45"} + assert repr(dec1) == "BSONDecimal128('123.45')" + assert str(dec1) == "123.45" + + dec2 = BSONDecimal128(42) + assert dec2.value == "42" + + dec3 = BSONDecimal128(1.5) + assert dec3.value == "1.5" + + dec4 = BSONDecimal128(decimal.Decimal("99.99")) + assert dec4.value == "99.99" + + dec5 = BSONDecimal128(dec1) + assert dec5.value == "123.45" + + +def test_bson_decimal128_special_values(): + nan_dec = BSONDecimal128("NaN") + assert nan_dec.value == "NaN" + assert nan_dec._to_map_value() == {"__decimal128__": "NaN"} + + inf_dec = BSONDecimal128("Infinity") + assert inf_dec.value == "Infinity" + + neg_inf_dec = BSONDecimal128("-Infinity") + assert neg_inf_dec.value == "-Infinity" + + +@pytest.mark.parametrize( + "val_input, exc_type, match_msg", + [ + (True, TypeError, "value must be a Decimal, str, int, or float"), + (False, TypeError, "value must be a Decimal, str, int, or float"), + ([1, 2], TypeError, "value must be a Decimal, str, int, or float"), + ], +) +def test_bson_decimal128_invalid_inputs(val_input, exc_type, match_msg): + with pytest.raises(exc_type, match=match_msg): + BSONDecimal128(val_input) + + +def test_bson_decimal128_equality(): + d1 = BSONDecimal128("123.45") + d2 = BSONDecimal128("123.45") + d3 = BSONDecimal128("678.90") + assert d1 == d2 + assert d1 != d3 + assert d1 == decimal.Decimal("123.45") + assert d1 != "123.45" + + # Transitivity test: BSONDecimal128("1.0") == Decimal("1") == BSONDecimal128("1") + d_trail = BSONDecimal128("1.0") + d_int = BSONDecimal128("1") + dec_int = decimal.Decimal("1") + assert d_trail == dec_int + assert d_int == dec_int + assert d_trail == d_int # Transitivity enforced! + + nan1 = BSONDecimal128("NaN") + nan2 = BSONDecimal128("NaN") + assert nan1 == nan2 + + +def test_bson_decimal128_hash_and_dict_key(): + d1 = BSONDecimal128("123.45") + d2 = BSONDecimal128("123.45") + dec_val = decimal.Decimal("123.45") + + # Hash invariant test: if a == b, then hash(a) == hash(b) + assert hash(d1) == hash(d2) + assert hash(d1) == hash(dec_val) + assert len({d1, d2, dec_val}) == 1 + + nan1 = BSONDecimal128("NaN") + nan2 = BSONDecimal128("NaN") + assert hash(nan1) == hash(nan2) + assert len({nan1, nan2}) == 1 + + +def test_bson_decimal128_copy(): + d = BSONDecimal128("123.45") + assert copy.copy(d) == d + assert copy.deepcopy(d) == d + + +def test_bson_decimal128_pickle(): + d = BSONDecimal128("123.45") + assert pickle.loads(pickle.dumps(d)) == d From a52bb0a559d8cb0393772fc10cd4ad116f4cd03d Mon Sep 17 00:00:00 2001 From: ohmayr Date: Mon, 21 Sep 2026 08:25:37 +0000 Subject: [PATCH 06/21] refactor(firestore): address review feedback for BSONDecimal128 - Convert to_decimal from property to method call to_decimal(). - Add __float__ and __int__ numeric protocol methods to BSONDecimal128. - Improve __eq__ and __hash__ for IEEE 754 special values (NaN, -NaN, sNaN, inf, -inf) using Decimal.is_nan(). - Add comprehensive test cases in test_bson.py for float/int conversions and special decimal values. --- .../google/cloud/firestore_v1/bson.py | 30 +++++++++---- .../tests/unit/v1/test_bson.py | 43 +++++++++++++++---- 2 files changed, 55 insertions(+), 18 deletions(-) diff --git a/packages/google-cloud-firestore/google/cloud/firestore_v1/bson.py b/packages/google-cloud-firestore/google/cloud/firestore_v1/bson.py index 7b3ef468f9d2..7bc3786949c8 100644 --- a/packages/google-cloud-firestore/google/cloud/firestore_v1/bson.py +++ b/packages/google-cloud-firestore/google/cloud/firestore_v1/bson.py @@ -425,7 +425,7 @@ class BSONDecimal128(_BSONType): >>> dec = BSONDecimal128("123.45") >>> dec.value '123.45' - >>> dec.to_decimal + >>> dec.to_decimal() Decimal('123.45') """ @@ -451,7 +451,6 @@ def value(self) -> str: """str: The string representation of the 128-bit decimal value.""" return self._value - @property def to_decimal(self) -> decimal.Decimal: """decimal.Decimal: Convert to Python standard library Decimal instance.""" return decimal.Decimal(self._value) @@ -466,25 +465,38 @@ def __repr__(self) -> str: def __str__(self) -> str: return self._value + def __float__(self) -> float: + """float: Convert decimal value to float.""" + return float(self._value) + + def __int__(self) -> int: + """int: Convert decimal value to integer.""" + return int(self.to_decimal()) + def __eq__(self, other: Any) -> bool: if isinstance(other, BSONDecimal128): - if self._value.upper() == "NAN" and other._value.upper() == "NAN": - return True try: - return self.to_decimal == other.to_decimal + d1, d2 = self.to_decimal(), other.to_decimal() + if d1.is_nan() and d2.is_nan(): + return True + return d1 == d2 except decimal.InvalidOperation: return self._value == other._value if isinstance(other, decimal.Decimal): try: - return self.to_decimal == other + d1 = self.to_decimal() + if d1.is_nan() and other.is_nan(): + return True + return d1 == other except decimal.InvalidOperation: return False return NotImplemented def __hash__(self) -> int: - if self._value.upper() == "NAN": - return hash((type(self), "NAN")) try: - return hash(self.to_decimal) + d = self.to_decimal() + if d.is_nan(): + return hash((type(self), "NAN")) + return hash(d) except decimal.InvalidOperation: return hash((type(self), self._value)) diff --git a/packages/google-cloud-firestore/tests/unit/v1/test_bson.py b/packages/google-cloud-firestore/tests/unit/v1/test_bson.py index cc619ebec642..8cd0401afc07 100644 --- a/packages/google-cloud-firestore/tests/unit/v1/test_bson.py +++ b/packages/google-cloud-firestore/tests/unit/v1/test_bson.py @@ -17,6 +17,7 @@ import copy import decimal +import math import pickle import pytest @@ -487,7 +488,7 @@ def test_bson_regex_pickle(): def test_bson_decimal128_valid(): dec1 = BSONDecimal128("123.45") assert dec1.value == "123.45" - assert dec1.to_decimal == decimal.Decimal("123.45") + assert dec1.to_decimal() == decimal.Decimal("123.45") assert dec1._to_map_value() == {"__decimal128__": "123.45"} assert repr(dec1) == "BSONDecimal128('123.45')" assert str(dec1) == "123.45" @@ -505,16 +506,40 @@ def test_bson_decimal128_valid(): assert dec5.value == "123.45" -def test_bson_decimal128_special_values(): - nan_dec = BSONDecimal128("NaN") - assert nan_dec.value == "NaN" - assert nan_dec._to_map_value() == {"__decimal128__": "NaN"} +def test_bson_decimal128_float_and_int(): + dec = BSONDecimal128("123.45") + assert float(dec) == 123.45 + assert int(dec) == 123 - inf_dec = BSONDecimal128("Infinity") - assert inf_dec.value == "Infinity" - neg_inf_dec = BSONDecimal128("-Infinity") - assert neg_inf_dec.value == "-Infinity" +@pytest.mark.parametrize( + "special_val", + [ + "inf", + "-inf", + "Infinity", + "-Infinity", + "NaN", + "-NaN", + "sNaN", + "-sNaN", + float("inf"), + float("-inf"), + float("nan"), + ], +) +def test_bson_decimal128_special_values(special_val): + dec1 = BSONDecimal128(special_val) + dec2 = BSONDecimal128(special_val) + assert dec1.value == str(special_val) + assert dec1._to_map_value() == {"__decimal128__": str(special_val)} + assert dec1 == dec2 + assert hash(dec1) == hash(dec2) + + if "inf" in str(special_val).lower(): + assert math.isinf(float(dec1)) + elif "nan" in str(special_val).lower(): + assert math.isnan(float(dec1)) @pytest.mark.parametrize( From 96bcd32c204b335c9e0c8e7049f320c7713c9984 Mon Sep 17 00:00:00 2001 From: ohmayr Date: Mon, 21 Sep 2026 08:31:22 +0000 Subject: [PATCH 07/21] fix(firestore): handle signaling NaN strings in BSONDecimal128.__float__ - Safely convert sNaN and -sNaN to float("nan") / float("-nan") in __float__ without raising ValueError. Towards #18395 --- .../google-cloud-firestore/google/cloud/firestore_v1/bson.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/google-cloud-firestore/google/cloud/firestore_v1/bson.py b/packages/google-cloud-firestore/google/cloud/firestore_v1/bson.py index 7bc3786949c8..18e54dadef45 100644 --- a/packages/google-cloud-firestore/google/cloud/firestore_v1/bson.py +++ b/packages/google-cloud-firestore/google/cloud/firestore_v1/bson.py @@ -467,7 +467,10 @@ def __str__(self) -> str: def __float__(self) -> float: """float: Convert decimal value to float.""" - return float(self._value) + d = self.to_decimal() + if d.is_nan(): + return float("-nan") if d.is_signed() else float("nan") + return float(d) def __int__(self) -> int: """int: Convert decimal value to integer.""" From f513edc41714e0c20640051f71638cbfa30e99ca Mon Sep 17 00:00:00 2001 From: ohmayr Date: Wed, 23 Sep 2026 18:26:55 +0000 Subject: [PATCH 08/21] fix(firestore): disallow float in BSONDecimal128 and validate input strings Disallow passing float to BSONDecimal128 to prevent binary floating-point precision loss and edge cases, aligning with PyMongo's Decimal128 behavior. Validate string and numeric inputs via decimal.Decimal in __init__ to reject invalid numeric strings. --- .../google/cloud/firestore_v1/bson.py | 22 ++++++++++------ .../tests/unit/v1/test_bson.py | 25 +++++++++---------- 2 files changed, 27 insertions(+), 20 deletions(-) diff --git a/packages/google-cloud-firestore/google/cloud/firestore_v1/bson.py b/packages/google-cloud-firestore/google/cloud/firestore_v1/bson.py index 18e54dadef45..5a6b602dca3d 100644 --- a/packages/google-cloud-firestore/google/cloud/firestore_v1/bson.py +++ b/packages/google-cloud-firestore/google/cloud/firestore_v1/bson.py @@ -413,12 +413,12 @@ class BSONDecimal128(_BSONType): """Represents a BSON 128-bit Decimal container for Firestore. Args: - value (Union[str, int, float, decimal.Decimal, BSONDecimal128]): - The decimal value as a string, integer, float, decimal.Decimal, + value (Union[str, int, decimal.Decimal, BSONDecimal128]): + The decimal value as a string, integer, decimal.Decimal, or BSONDecimal128 instance. Raises: - TypeError: If value is a boolean or unsupported type. + TypeError: If value is a boolean or an unsupported type. ValueError: If value cannot be parsed as a valid decimal number. Example: @@ -433,18 +433,26 @@ class BSONDecimal128(_BSONType): def __init__( self, - value: Union[str, int, float, decimal.Decimal, "BSONDecimal128"], + value: Union[str, int, decimal.Decimal, "BSONDecimal128"], ): if isinstance(value, BSONDecimal128): self._value: str = value._value - elif isinstance(value, (str, int, float, decimal.Decimal)) and not isinstance( + elif isinstance(value, (str, int, decimal.Decimal)) and not isinstance( value, bool ): + try: + # Validate the value parses as a valid decimal number + decimal.Decimal(value) + except decimal.InvalidOperation as exc: + raise ValueError(f"Cannot convert {value!r} to Decimal: {exc}") from exc self._value = str(value) - else: + elif isinstance(value, float): raise TypeError( - "BSONDecimal128 value must be a Decimal, str, int, or float." + "BSONDecimal128 does not accept float values due to potential precision loss. " + "Convert the float to a str or decimal.Decimal first." ) + else: + raise TypeError("BSONDecimal128 value must be a Decimal, str, or int.") @property def value(self) -> str: diff --git a/packages/google-cloud-firestore/tests/unit/v1/test_bson.py b/packages/google-cloud-firestore/tests/unit/v1/test_bson.py index 8cd0401afc07..92a3a1b128e3 100644 --- a/packages/google-cloud-firestore/tests/unit/v1/test_bson.py +++ b/packages/google-cloud-firestore/tests/unit/v1/test_bson.py @@ -496,14 +496,11 @@ def test_bson_decimal128_valid(): dec2 = BSONDecimal128(42) assert dec2.value == "42" - dec3 = BSONDecimal128(1.5) - assert dec3.value == "1.5" + dec3 = BSONDecimal128(decimal.Decimal("99.99")) + assert dec3.value == "99.99" - dec4 = BSONDecimal128(decimal.Decimal("99.99")) - assert dec4.value == "99.99" - - dec5 = BSONDecimal128(dec1) - assert dec5.value == "123.45" + dec4 = BSONDecimal128(dec1) + assert dec4.value == "123.45" def test_bson_decimal128_float_and_int(): @@ -523,9 +520,6 @@ def test_bson_decimal128_float_and_int(): "-NaN", "sNaN", "-sNaN", - float("inf"), - float("-inf"), - float("nan"), ], ) def test_bson_decimal128_special_values(special_val): @@ -545,9 +539,14 @@ def test_bson_decimal128_special_values(special_val): @pytest.mark.parametrize( "val_input, exc_type, match_msg", [ - (True, TypeError, "value must be a Decimal, str, int, or float"), - (False, TypeError, "value must be a Decimal, str, int, or float"), - ([1, 2], TypeError, "value must be a Decimal, str, int, or float"), + (True, TypeError, "value must be a Decimal, str, or int"), + (False, TypeError, "value must be a Decimal, str, or int"), + ([1, 2], TypeError, "value must be a Decimal, str, or int"), + (1.5, TypeError, "does not accept float values"), + (float("inf"), TypeError, "does not accept float values"), + (float("nan"), TypeError, "does not accept float values"), + ("not-a-number", ValueError, "Cannot convert 'not-a-number' to Decimal"), + ("12.34.56", ValueError, "Cannot convert '12.34.56' to Decimal"), ], ) def test_bson_decimal128_invalid_inputs(val_input, exc_type, match_msg): From b7ac973f4d663996bd0c0c3172400a9e30a095d2 Mon Sep 17 00:00:00 2001 From: ohmayr Date: Wed, 16 Sep 2026 20:34:31 +0000 Subject: [PATCH 09/21] feat(firestore): add BSON read deserialization support --- .../google/cloud/firestore_v1/_helpers.py | 75 ++++++++++++++----- .../google/cloud/firestore_v1/async_client.py | 2 + .../google/cloud/firestore_v1/base_client.py | 2 + .../cloud/firestore_v1/base_document.py | 18 ++++- .../google/cloud/firestore_v1/bson.py | 38 +++++++++- .../google/cloud/firestore_v1/client.py | 2 + .../cloud/firestore_v1/pipeline_result.py | 3 +- .../tests/system/test_system.py | 28 ++----- .../tests/system/test_system_async.py | 28 ++----- .../tests/unit/v1/test__helpers.py | 37 +++++++++ 10 files changed, 163 insertions(+), 70 deletions(-) diff --git a/packages/google-cloud-firestore/google/cloud/firestore_v1/_helpers.py b/packages/google-cloud-firestore/google/cloud/firestore_v1/_helpers.py index 9793c0685121..51f288f9a84c 100644 --- a/packages/google-cloud-firestore/google/cloud/firestore_v1/_helpers.py +++ b/packages/google-cloud-firestore/google/cloud/firestore_v1/_helpers.py @@ -44,7 +44,7 @@ import google from google.cloud import exceptions # type: ignore from google.cloud.firestore_v1 import transforms, types -from google.cloud.firestore_v1.bson import _BSONType +from google.cloud.firestore_v1.bson import _BSON_DECODERS, _BSONType from google.cloud.firestore_v1.field_path import FieldPath, parse_field_path from google.cloud.firestore_v1.types import common, document, write from google.cloud.firestore_v1.types.write import DocumentTransform @@ -347,11 +347,7 @@ def reference_value_to_document(reference_value, client) -> Any: return document -def decode_value( - value, client -) -> Union[ - None, bool, int, float, list, datetime.datetime, str, bytes, dict, GeoPoint, Vector -]: +def decode_value(value, client=None, decode_bson: Optional[bool] = None) -> Any: """Converts a Firestore protobuf ``Value`` to a native Python value. Args: @@ -359,15 +355,10 @@ def decode_value( Firestore protobuf to be decoded / parsed / converted. client (:class:`~google.cloud.firestore_v1.client.Client`): A client that has a document factory. + decode_bson (Optional[bool]): Whether to decode BSON extended types. Returns: - Union[NoneType, bool, int, float, datetime.datetime, \ - str, bytes, dict, ~google.cloud.Firestore.GeoPoint]: A native - Python value converted from the ``value``. - - Raises: - NotImplementedError: If the ``value_type`` is ``reference_value``. - ValueError: If the ``value_type`` is unknown. + Any: A native Python value converted from the ``value``. """ value_pb = getattr(value, "_pb", value) value_type = value_pb.WhichOneof("value_type") @@ -394,15 +385,45 @@ def decode_value( ) elif value_type == "array_value": return [ - decode_value(element, client) for element in value_pb.array_value.values + decode_value(element, client, decode_bson=decode_bson) + for element in value_pb.array_value.values ] elif value_type == "map_value": - return decode_dict(value_pb.map_value.fields, client) + return decode_dict(value_pb.map_value.fields, client, decode_bson=decode_bson) else: raise ValueError("Unknown ``value_type``", value_type) -def decode_dict(value_fields, client) -> Union[dict, Vector]: +def _decode_bson_dict(data: dict) -> Optional[_BSONType]: + """Decode a single-key wire map dictionary if registered.""" + if len(data) == 1: + key, val = next(iter(data.items())) + decoder = _BSON_DECODERS.get(key) + if decoder is not None: + try: + return decoder(val) + except Exception: + pass + return None + + +def _decode_bson_dict_recursive(data: Any) -> Any: + """Recursively decodes BSON wire map dictionaries.""" + if isinstance(data, dict): + decoded = _decode_bson_dict(data) + if decoded is not None: + return decoded + return {k: _decode_bson_dict_recursive(v) for k, v in data.items()} + elif isinstance(data, list): + return [_decode_bson_dict_recursive(item) for item in data] + return data + + +def decode_dict( + value_fields, + client=None, + decode_bson: Optional[bool] = None, +) -> Union[dict, Vector, _BSONType]: """Converts a protobuf map of Firestore ``Value``-s. Args: @@ -410,14 +431,18 @@ def decode_dict(value_fields, client) -> Union[dict, Vector]: protobuf map of Firestore ``Value``-s. client (:class:`~google.cloud.firestore_v1.client.Client`): A client that has a document factory. + decode_bson (Optional[bool]): Whether to decode BSON extended types. Returns: - Dict[str, Union[NoneType, bool, int, float, datetime.datetime, \ - str, bytes, dict, ~google.cloud.Firestore.GeoPoint]]: A dictionary - of native Python values converted from the ``value_fields``. + Union[dict, ~google.cloud.firestore_v1.vector.Vector, \ + ~google.cloud.firestore_v1.bson._BSONType]: A dictionary of native \ + Python values, Vector, or BSON object converted from ``value_fields``. """ value_fields_pb = getattr(value_fields, "_pb", value_fields) - res = {key: decode_value(value, client) for key, value in value_fields_pb.items()} + res = { + key: decode_value(value, client, decode_bson=decode_bson) + for key, value in value_fields_pb.items() + } if res.get("__type__", None) == "__vector__": # Vector data type is represented as mapping. @@ -425,6 +450,16 @@ def decode_dict(value_fields, client) -> Union[dict, Vector]: values = cast(Sequence[float], res["value"]) return Vector(values) + should_decode = ( + decode_bson + if decode_bson is not None + else getattr(client, "_decode_bson", False) + ) + if should_decode: + decoded = _decode_bson_dict(res) + if decoded is not None: + return decoded + return res diff --git a/packages/google-cloud-firestore/google/cloud/firestore_v1/async_client.py b/packages/google-cloud-firestore/google/cloud/firestore_v1/async_client.py index 3167335e0385..4cd625387048 100644 --- a/packages/google-cloud-firestore/google/cloud/firestore_v1/async_client.py +++ b/packages/google-cloud-firestore/google/cloud/firestore_v1/async_client.py @@ -105,6 +105,7 @@ def __init__( database=None, client_info=_CLIENT_INFO, client_options=None, + decode_bson: bool = False, ) -> None: super(AsyncClient, self).__init__( project=project, @@ -112,6 +113,7 @@ def __init__( database=database, client_info=client_info, client_options=client_options, + decode_bson=decode_bson, ) def _to_sync_copy(self): diff --git a/packages/google-cloud-firestore/google/cloud/firestore_v1/base_client.py b/packages/google-cloud-firestore/google/cloud/firestore_v1/base_client.py index 95166266bef2..5cbdf47c1b8d 100644 --- a/packages/google-cloud-firestore/google/cloud/firestore_v1/base_client.py +++ b/packages/google-cloud-firestore/google/cloud/firestore_v1/base_client.py @@ -132,6 +132,7 @@ def __init__( database=None, client_info=_CLIENT_INFO, client_options=None, + decode_bson: bool = False, ) -> None: database = database or DEFAULT_DATABASE # NOTE: This API has no use for the _http argument, but sending it @@ -165,6 +166,7 @@ def __init__( self._client_options = client_options self._database = database + self._decode_bson: bool = decode_bson def _firestore_api_helper(self, transport, client_class, client_module) -> Any: """Lazy-loading getter GAPIC Firestore API. diff --git a/packages/google-cloud-firestore/google/cloud/firestore_v1/base_document.py b/packages/google-cloud-firestore/google/cloud/firestore_v1/base_document.py index 92d8daa21fd6..a2a423d36de8 100644 --- a/packages/google-cloud-firestore/google/cloud/firestore_v1/base_document.py +++ b/packages/google-cloud-firestore/google/cloud/firestore_v1/base_document.py @@ -512,12 +512,17 @@ def get(self, field_path: str) -> Any: nested_data = field_path_module.get_nested_value(field_path, self._data) return copy.deepcopy(nested_data) - def to_dict(self) -> Union[Dict[str, Any], None]: + def to_dict( + self, decode_bson: Optional[bool] = None + ) -> Union[Dict[str, Any], None]: """Retrieve the data contained in this snapshot. A copy is returned since the data may contain mutable values, but the data stored in the snapshot must remain immutable. + Args: + decode_bson (Optional[bool]): Whether to decode BSON extended types. + Returns: Dict[str, Any] or None: The data in the snapshot. Returns None if reference @@ -525,7 +530,16 @@ def to_dict(self) -> Union[Dict[str, Any], None]: """ if not self._exists: return None - return copy.deepcopy(self._data) + data = copy.deepcopy(self._data) + client = self._reference._client if self._reference is not None else None + should_decode = ( + decode_bson + if decode_bson is not None + else getattr(client, "_decode_bson", False) + ) + if should_decode: + return _helpers._decode_bson_dict_recursive(data) + return data def _to_protobuf(self) -> Optional[Document]: return _helpers.document_snapshot_to_protobuf(self) diff --git a/packages/google-cloud-firestore/google/cloud/firestore_v1/bson.py b/packages/google-cloud-firestore/google/cloud/firestore_v1/bson.py index 5a6b602dca3d..955ebd50ec99 100644 --- a/packages/google-cloud-firestore/google/cloud/firestore_v1/bson.py +++ b/packages/google-cloud-firestore/google/cloud/firestore_v1/bson.py @@ -27,7 +27,7 @@ import abc import decimal import re -from typing import Any, Dict, Union +from typing import Any, Callable, Dict, Union __all__ = [ "BSONObjectId", @@ -65,6 +65,24 @@ def __eq__(self, other: Any) -> bool: def __hash__(self) -> int: """Hash representation contract for set and dictionary keys.""" + @classmethod + def _from_dict(cls, data: Any) -> Any: + """Deserializes a BSON wire map dictionary into a BSON instance or bytes. + + Args: + data (Any): Potential BSON wire map dictionary. + + Returns: + Any: Deserialized BSON container instance/bytes, or None if not a BSON wire map. + """ + if not isinstance(data, dict) or len(data) != 1: + return None + key, val = next(iter(data.items())) + decoder = _BSON_DECODERS.get(key) + if decoder is None: + return None + return decoder(val) + def __repr__(self) -> str: return f"{self.__class__.__name__}()" @@ -511,3 +529,21 @@ def __hash__(self) -> int: return hash(d) except decimal.InvalidOperation: return hash((type(self), self._value)) + + +_BSON_DECODERS: Dict[str, Callable[[Any], Any]] = { + "__oid__": BSONObjectId, + "__min__": lambda _: BSONMinKey(), + "__max__": lambda _: BSONMaxKey(), + "__int__": BSONInt32, + "__decimal128__": BSONDecimal128, + "__binary__": lambda v: (v[1:] if v[0] == 0 else BSONBinary(v[1:], subtype=v[0])) + if isinstance(v, (bytes, bytearray)) and len(v) >= 1 + else None, + "__request_timestamp__": lambda v: BSONTimestamp(v["seconds"], v["increment"]) + if isinstance(v, dict) and "seconds" in v and "increment" in v + else None, + "__regex__": lambda v: BSONRegex(v["pattern"], v.get("options", "")) + if isinstance(v, dict) and "pattern" in v + else None, +} diff --git a/packages/google-cloud-firestore/google/cloud/firestore_v1/client.py b/packages/google-cloud-firestore/google/cloud/firestore_v1/client.py index e29d07cb09ac..7b97f3ac13a5 100644 --- a/packages/google-cloud-firestore/google/cloud/firestore_v1/client.py +++ b/packages/google-cloud-firestore/google/cloud/firestore_v1/client.py @@ -94,6 +94,7 @@ def __init__( database=None, client_info=_CLIENT_INFO, client_options=None, + decode_bson: bool = False, ) -> None: super(Client, self).__init__( project=project, @@ -101,6 +102,7 @@ def __init__( database=database, client_info=client_info, client_options=client_options, + decode_bson=decode_bson, ) @property diff --git a/packages/google-cloud-firestore/google/cloud/firestore_v1/pipeline_result.py b/packages/google-cloud-firestore/google/cloud/firestore_v1/pipeline_result.py index e3fd74677a1e..7edb9808d292 100644 --- a/packages/google-cloud-firestore/google/cloud/firestore_v1/pipeline_result.py +++ b/packages/google-cloud-firestore/google/cloud/firestore_v1/pipeline_result.py @@ -44,6 +44,7 @@ from google.cloud.firestore_v1.async_transaction import AsyncTransaction from google.cloud.firestore_v1.base_client import BaseClient from google.cloud.firestore_v1.base_document import BaseDocumentReference + from google.cloud.firestore_v1.bson import _BSONType from google.cloud.firestore_v1.client import Client from google.cloud.firestore_v1.pipeline import Pipeline from google.cloud.firestore_v1.pipeline_expressions import Constant @@ -138,7 +139,7 @@ def __eq__(self, other: object) -> bool: return NotImplemented return (self._ref == other._ref) and (self._fields_pb == other._fields_pb) - def data(self) -> dict | "Vector" | None: + def data(self) -> dict | "Vector" | "_BSONType" | None: """ Retrieves all fields in the result. diff --git a/packages/google-cloud-firestore/tests/system/test_system.py b/packages/google-cloud-firestore/tests/system/test_system.py index 6608e077245a..91c80021e317 100644 --- a/packages/google-cloud-firestore/tests/system/test_system.py +++ b/packages/google-cloud-firestore/tests/system/test_system.py @@ -1285,9 +1285,9 @@ def test_unicode_doc(client, cleanup, database): @pytest.mark.parametrize("database", [FIRESTORE_ENTERPRISE_DB], indirect=True) -def test_bson_document_writes(client, cleanup, database): - """Test write operations for BSON types on Enterprise DB.""" - collection_id = "bson_type_writes_" + UNIQUE_RESOURCE_ID +def test_bson_document_read_and_write(client, cleanup, database): + """Test read and write operations for BSON types on Enterprise DB.""" + collection_id = "bson_type_read_write_" + UNIQUE_RESOURCE_ID doc_ref = client.collection(collection_id).document("bson_doc") cleanup(doc_ref.delete) @@ -1296,6 +1296,7 @@ def test_bson_document_writes(client, cleanup, database): "min_key": BSONMinKey(), "max_key": BSONMaxKey(), "int32_val": BSONInt32(42), + "binary_val_sub0": b"hello", "binary_val_sub128": BSONBinary(b"world", subtype=128), "timestamp_val": BSONTimestamp(1700000000, 1), "regex_val": BSONRegex("^hello.*$", options="i"), @@ -1306,26 +1307,7 @@ def test_bson_document_writes(client, cleanup, database): snapshot = doc_ref.get() assert snapshot.exists - assert snapshot.to_dict() == { - "user_id": {"__oid__": "507f191e810c19729de860ea"}, - "min_key": {"__min__": None}, - "max_key": {"__max__": None}, - "int32_val": {"__int__": 42}, - "binary_val_sub128": {"__binary__": b"\x80world"}, - "timestamp_val": { - "__request_timestamp__": { - "seconds": 1700000000, - "increment": 1, - } - }, - "regex_val": { - "__regex__": { - "pattern": "^hello.*$", - "options": "i", - } - }, - "decimal128_val": {"__decimal128__": "123.45"}, - } + assert snapshot.to_dict(decode_bson=True) == bson_payload @pytest.mark.parametrize("database", [FIRESTORE_ENTERPRISE_DB], indirect=True) diff --git a/packages/google-cloud-firestore/tests/system/test_system_async.py b/packages/google-cloud-firestore/tests/system/test_system_async.py index 8adcc3b026ed..d721f6ca203d 100644 --- a/packages/google-cloud-firestore/tests/system/test_system_async.py +++ b/packages/google-cloud-firestore/tests/system/test_system_async.py @@ -1258,9 +1258,9 @@ async def test_list_collections_with_read_time(client, cleanup, database): @pytest.mark.asyncio @pytest.mark.parametrize("database", [FIRESTORE_ENTERPRISE_DB], indirect=True) -async def test_async_bson_document_writes(client, cleanup, database): - """Test async write operations for BSON types on Enterprise DB.""" - collection_id = "async_bson_type_writes_" + UNIQUE_RESOURCE_ID +async def test_async_bson_document_read_and_write(client, cleanup, database): + """Test async read and write operations for BSON types on Enterprise DB.""" + collection_id = "async_bson_type_read_write_" + UNIQUE_RESOURCE_ID doc_ref = client.collection(collection_id).document("bson_doc") cleanup(doc_ref.delete) @@ -1269,6 +1269,7 @@ async def test_async_bson_document_writes(client, cleanup, database): "min_key": BSONMinKey(), "max_key": BSONMaxKey(), "int32_val": BSONInt32(42), + "binary_val_sub0": b"hello", "binary_val_sub128": BSONBinary(b"world", subtype=128), "timestamp_val": BSONTimestamp(1700000000, 1), "regex_val": BSONRegex("^hello.*$", options="i"), @@ -1279,26 +1280,7 @@ async def test_async_bson_document_writes(client, cleanup, database): snapshot = await doc_ref.get() assert snapshot.exists - assert snapshot.to_dict() == { - "user_id": {"__oid__": "507f191e810c19729de860ea"}, - "min_key": {"__min__": None}, - "max_key": {"__max__": None}, - "int32_val": {"__int__": 42}, - "binary_val_sub128": {"__binary__": b"\x80world"}, - "timestamp_val": { - "__request_timestamp__": { - "seconds": 1700000000, - "increment": 1, - } - }, - "regex_val": { - "__regex__": { - "pattern": "^hello.*$", - "options": "i", - } - }, - "decimal128_val": {"__decimal128__": "123.45"}, - } + assert snapshot.to_dict(decode_bson=True) == bson_payload @pytest.mark.asyncio diff --git a/packages/google-cloud-firestore/tests/unit/v1/test__helpers.py b/packages/google-cloud-firestore/tests/unit/v1/test__helpers.py index 4ce48424d3c4..b0b81eb98e51 100644 --- a/packages/google-cloud-firestore/tests/unit/v1/test__helpers.py +++ b/packages/google-cloud-firestore/tests/unit/v1/test__helpers.py @@ -706,6 +706,43 @@ def test_decode_dict_w_many_types(): assert decode_dict(value_fields, mock.sentinel.client) == expected +def test_decode_dict_w_bson_types(): + from google.cloud.firestore_v1._helpers import decode_dict, encode_dict + from google.cloud.firestore_v1.bson import ( + BSONBinary, + BSONDecimal128, + BSONInt32, + BSONMaxKey, + BSONMinKey, + BSONObjectId, + BSONRegex, + BSONTimestamp, + ) + + original_dict = { + "oid": BSONObjectId("507f191e810c19729de860ea"), + "min_k": BSONMinKey(), + "max_k": BSONMaxKey(), + "int32_v": BSONInt32(42), + "bin_sub0": b"hello", + "bin_sub0_empty": b"", + "bin_sub128": BSONBinary(b"world", subtype=128), + "ts_v": BSONTimestamp(1700000000, 1), + "regex_v": BSONRegex("^hello.*$", options="i"), + "dec_v": BSONDecimal128("123.45"), + } + + pb_fields = encode_dict(original_dict) + # Default (decode_bson=False) returns raw dict + raw_decoded = decode_dict(pb_fields, mock.sentinel.client) + assert raw_decoded != original_dict + assert raw_decoded["oid"] == {"__oid__": "507f191e810c19729de860ea"} + + # decode_bson=True returns deserialized BSON objects + decoded = decode_dict(pb_fields, mock.sentinel.client, decode_bson=True) + assert decoded == original_dict + + def _dummy_ref_string(collection_id): from google.cloud.firestore_v1.base_client import DEFAULT_DATABASE From 5bab625933a7eff4681f30acd86a5e36709644e5 Mon Sep 17 00:00:00 2001 From: ohmayr Date: Mon, 21 Sep 2026 09:06:30 +0000 Subject: [PATCH 10/21] refactor(firestore): address review feedback for BSON read deserialization - Perform automatic BSON deserialization in decode_dict and DocumentSnapshot.to_dict using _BSONType._from_dict. - Remove decode_bson configuration parameter across Client, AsyncClient, BaseClient, and DocumentSnapshot. - Preserve precise return type annotations in decode_dict and restore docstring Raises section. Towards #18402 --- .../google/cloud/firestore_v1/_helpers.py | 45 ++++++------------- .../google/cloud/firestore_v1/async_client.py | 2 - .../google/cloud/firestore_v1/base_client.py | 2 - .../cloud/firestore_v1/base_document.py | 17 +------ .../google/cloud/firestore_v1/client.py | 2 - .../tests/system/test_system.py | 2 +- .../tests/system/test_system_async.py | 2 +- .../tests/unit/v1/test__helpers.py | 8 +--- 8 files changed, 18 insertions(+), 62 deletions(-) diff --git a/packages/google-cloud-firestore/google/cloud/firestore_v1/_helpers.py b/packages/google-cloud-firestore/google/cloud/firestore_v1/_helpers.py index 51f288f9a84c..3280b2b2a2a7 100644 --- a/packages/google-cloud-firestore/google/cloud/firestore_v1/_helpers.py +++ b/packages/google-cloud-firestore/google/cloud/firestore_v1/_helpers.py @@ -347,7 +347,7 @@ def reference_value_to_document(reference_value, client) -> Any: return document -def decode_value(value, client=None, decode_bson: Optional[bool] = None) -> Any: +def decode_value(value, client=None) -> Any: """Converts a Firestore protobuf ``Value`` to a native Python value. Args: @@ -355,10 +355,12 @@ def decode_value(value, client=None, decode_bson: Optional[bool] = None) -> Any: Firestore protobuf to be decoded / parsed / converted. client (:class:`~google.cloud.firestore_v1.client.Client`): A client that has a document factory. - decode_bson (Optional[bool]): Whether to decode BSON extended types. Returns: Any: A native Python value converted from the ``value``. + + Raises: + ValueError: If ``value_type`` is unknown or unsupported. """ value_pb = getattr(value, "_pb", value) value_type = value_pb.WhichOneof("value_type") @@ -385,32 +387,19 @@ def decode_value(value, client=None, decode_bson: Optional[bool] = None) -> Any: ) elif value_type == "array_value": return [ - decode_value(element, client, decode_bson=decode_bson) + decode_value(element, client) for element in value_pb.array_value.values ] elif value_type == "map_value": - return decode_dict(value_pb.map_value.fields, client, decode_bson=decode_bson) + return decode_dict(value_pb.map_value.fields, client) else: raise ValueError("Unknown ``value_type``", value_type) -def _decode_bson_dict(data: dict) -> Optional[_BSONType]: - """Decode a single-key wire map dictionary if registered.""" - if len(data) == 1: - key, val = next(iter(data.items())) - decoder = _BSON_DECODERS.get(key) - if decoder is not None: - try: - return decoder(val) - except Exception: - pass - return None - - def _decode_bson_dict_recursive(data: Any) -> Any: """Recursively decodes BSON wire map dictionaries.""" if isinstance(data, dict): - decoded = _decode_bson_dict(data) + decoded = _BSONType._from_dict(data) if decoded is not None: return decoded return {k: _decode_bson_dict_recursive(v) for k, v in data.items()} @@ -422,8 +411,7 @@ def _decode_bson_dict_recursive(data: Any) -> Any: def decode_dict( value_fields, client=None, - decode_bson: Optional[bool] = None, -) -> Union[dict, Vector, _BSONType]: +) -> Union[dict, Vector, _BSONType, bytes]: """Converts a protobuf map of Firestore ``Value``-s. Args: @@ -431,16 +419,15 @@ def decode_dict( protobuf map of Firestore ``Value``-s. client (:class:`~google.cloud.firestore_v1.client.Client`): A client that has a document factory. - decode_bson (Optional[bool]): Whether to decode BSON extended types. Returns: Union[dict, ~google.cloud.firestore_v1.vector.Vector, \ - ~google.cloud.firestore_v1.bson._BSONType]: A dictionary of native \ + ~google.cloud.firestore_v1.bson._BSONType, bytes]: A dictionary of native \ Python values, Vector, or BSON object converted from ``value_fields``. """ value_fields_pb = getattr(value_fields, "_pb", value_fields) res = { - key: decode_value(value, client, decode_bson=decode_bson) + key: decode_value(value, client) for key, value in value_fields_pb.items() } @@ -450,15 +437,9 @@ def decode_dict( values = cast(Sequence[float], res["value"]) return Vector(values) - should_decode = ( - decode_bson - if decode_bson is not None - else getattr(client, "_decode_bson", False) - ) - if should_decode: - decoded = _decode_bson_dict(res) - if decoded is not None: - return decoded + decoded = _BSONType._from_dict(res) + if decoded is not None: + return decoded return res diff --git a/packages/google-cloud-firestore/google/cloud/firestore_v1/async_client.py b/packages/google-cloud-firestore/google/cloud/firestore_v1/async_client.py index 4cd625387048..3167335e0385 100644 --- a/packages/google-cloud-firestore/google/cloud/firestore_v1/async_client.py +++ b/packages/google-cloud-firestore/google/cloud/firestore_v1/async_client.py @@ -105,7 +105,6 @@ def __init__( database=None, client_info=_CLIENT_INFO, client_options=None, - decode_bson: bool = False, ) -> None: super(AsyncClient, self).__init__( project=project, @@ -113,7 +112,6 @@ def __init__( database=database, client_info=client_info, client_options=client_options, - decode_bson=decode_bson, ) def _to_sync_copy(self): diff --git a/packages/google-cloud-firestore/google/cloud/firestore_v1/base_client.py b/packages/google-cloud-firestore/google/cloud/firestore_v1/base_client.py index 5cbdf47c1b8d..95166266bef2 100644 --- a/packages/google-cloud-firestore/google/cloud/firestore_v1/base_client.py +++ b/packages/google-cloud-firestore/google/cloud/firestore_v1/base_client.py @@ -132,7 +132,6 @@ def __init__( database=None, client_info=_CLIENT_INFO, client_options=None, - decode_bson: bool = False, ) -> None: database = database or DEFAULT_DATABASE # NOTE: This API has no use for the _http argument, but sending it @@ -166,7 +165,6 @@ def __init__( self._client_options = client_options self._database = database - self._decode_bson: bool = decode_bson def _firestore_api_helper(self, transport, client_class, client_module) -> Any: """Lazy-loading getter GAPIC Firestore API. diff --git a/packages/google-cloud-firestore/google/cloud/firestore_v1/base_document.py b/packages/google-cloud-firestore/google/cloud/firestore_v1/base_document.py index a2a423d36de8..95f515659a56 100644 --- a/packages/google-cloud-firestore/google/cloud/firestore_v1/base_document.py +++ b/packages/google-cloud-firestore/google/cloud/firestore_v1/base_document.py @@ -512,17 +512,12 @@ def get(self, field_path: str) -> Any: nested_data = field_path_module.get_nested_value(field_path, self._data) return copy.deepcopy(nested_data) - def to_dict( - self, decode_bson: Optional[bool] = None - ) -> Union[Dict[str, Any], None]: + def to_dict(self) -> Union[Dict[str, Any], None]: """Retrieve the data contained in this snapshot. A copy is returned since the data may contain mutable values, but the data stored in the snapshot must remain immutable. - Args: - decode_bson (Optional[bool]): Whether to decode BSON extended types. - Returns: Dict[str, Any] or None: The data in the snapshot. Returns None if reference @@ -531,15 +526,7 @@ def to_dict( if not self._exists: return None data = copy.deepcopy(self._data) - client = self._reference._client if self._reference is not None else None - should_decode = ( - decode_bson - if decode_bson is not None - else getattr(client, "_decode_bson", False) - ) - if should_decode: - return _helpers._decode_bson_dict_recursive(data) - return data + return _helpers._decode_bson_dict_recursive(data) def _to_protobuf(self) -> Optional[Document]: return _helpers.document_snapshot_to_protobuf(self) diff --git a/packages/google-cloud-firestore/google/cloud/firestore_v1/client.py b/packages/google-cloud-firestore/google/cloud/firestore_v1/client.py index 7b97f3ac13a5..e29d07cb09ac 100644 --- a/packages/google-cloud-firestore/google/cloud/firestore_v1/client.py +++ b/packages/google-cloud-firestore/google/cloud/firestore_v1/client.py @@ -94,7 +94,6 @@ def __init__( database=None, client_info=_CLIENT_INFO, client_options=None, - decode_bson: bool = False, ) -> None: super(Client, self).__init__( project=project, @@ -102,7 +101,6 @@ def __init__( database=database, client_info=client_info, client_options=client_options, - decode_bson=decode_bson, ) @property diff --git a/packages/google-cloud-firestore/tests/system/test_system.py b/packages/google-cloud-firestore/tests/system/test_system.py index 91c80021e317..fc864e9d555d 100644 --- a/packages/google-cloud-firestore/tests/system/test_system.py +++ b/packages/google-cloud-firestore/tests/system/test_system.py @@ -1307,7 +1307,7 @@ def test_bson_document_read_and_write(client, cleanup, database): snapshot = doc_ref.get() assert snapshot.exists - assert snapshot.to_dict(decode_bson=True) == bson_payload + assert snapshot.to_dict() == bson_payload @pytest.mark.parametrize("database", [FIRESTORE_ENTERPRISE_DB], indirect=True) diff --git a/packages/google-cloud-firestore/tests/system/test_system_async.py b/packages/google-cloud-firestore/tests/system/test_system_async.py index d721f6ca203d..4b0c2c26defa 100644 --- a/packages/google-cloud-firestore/tests/system/test_system_async.py +++ b/packages/google-cloud-firestore/tests/system/test_system_async.py @@ -1280,7 +1280,7 @@ async def test_async_bson_document_read_and_write(client, cleanup, database): snapshot = await doc_ref.get() assert snapshot.exists - assert snapshot.to_dict(decode_bson=True) == bson_payload + assert snapshot.to_dict() == bson_payload @pytest.mark.asyncio diff --git a/packages/google-cloud-firestore/tests/unit/v1/test__helpers.py b/packages/google-cloud-firestore/tests/unit/v1/test__helpers.py index b0b81eb98e51..88fe361eee31 100644 --- a/packages/google-cloud-firestore/tests/unit/v1/test__helpers.py +++ b/packages/google-cloud-firestore/tests/unit/v1/test__helpers.py @@ -733,13 +733,7 @@ def test_decode_dict_w_bson_types(): } pb_fields = encode_dict(original_dict) - # Default (decode_bson=False) returns raw dict - raw_decoded = decode_dict(pb_fields, mock.sentinel.client) - assert raw_decoded != original_dict - assert raw_decoded["oid"] == {"__oid__": "507f191e810c19729de860ea"} - - # decode_bson=True returns deserialized BSON objects - decoded = decode_dict(pb_fields, mock.sentinel.client, decode_bson=True) + decoded = decode_dict(pb_fields, mock.sentinel.client) assert decoded == original_dict From b3443ff42901de75a5d0ac342e26a3981e054264 Mon Sep 17 00:00:00 2001 From: ohmayr Date: Mon, 21 Sep 2026 09:17:05 +0000 Subject: [PATCH 11/21] refactor(firestore): restore precise return types and docstrings on decode_value - Restore full Union return type with _BSONType on decode_value. - Restore Returns and Raises docstring sections in decode_value matching base branch. - Remove unused _BSON_DECODERS import from _helpers.py. - Revert extraneous changes to pipeline_result.py. Towards #18402 --- .../google/cloud/firestore_v1/_helpers.py | 28 ++++++++++++++++--- .../cloud/firestore_v1/pipeline_result.py | 3 +- 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/packages/google-cloud-firestore/google/cloud/firestore_v1/_helpers.py b/packages/google-cloud-firestore/google/cloud/firestore_v1/_helpers.py index 3280b2b2a2a7..92e033eee1ce 100644 --- a/packages/google-cloud-firestore/google/cloud/firestore_v1/_helpers.py +++ b/packages/google-cloud-firestore/google/cloud/firestore_v1/_helpers.py @@ -44,7 +44,7 @@ import google from google.cloud import exceptions # type: ignore from google.cloud.firestore_v1 import transforms, types -from google.cloud.firestore_v1.bson import _BSON_DECODERS, _BSONType +from google.cloud.firestore_v1.bson import _BSONType from google.cloud.firestore_v1.field_path import FieldPath, parse_field_path from google.cloud.firestore_v1.types import common, document, write from google.cloud.firestore_v1.types.write import DocumentTransform @@ -347,7 +347,22 @@ def reference_value_to_document(reference_value, client) -> Any: return document -def decode_value(value, client=None) -> Any: +def decode_value( + value, client=None +) -> Union[ + None, + bool, + int, + float, + list, + datetime.datetime, + str, + bytes, + dict, + GeoPoint, + Vector, + _BSONType, +]: """Converts a Firestore protobuf ``Value`` to a native Python value. Args: @@ -357,10 +372,15 @@ def decode_value(value, client=None) -> Any: A client that has a document factory. Returns: - Any: A native Python value converted from the ``value``. + Union[NoneType, bool, int, float, datetime.datetime, \ + str, bytes, dict, ~google.cloud.Firestore.GeoPoint, \ + ~google.cloud.firestore_v1.vector.Vector, \ + ~google.cloud.firestore_v1.bson._BSONType]: A native \ + Python value converted from the ``value``. Raises: - ValueError: If ``value_type`` is unknown or unsupported. + NotImplementedError: If the ``value_type`` is ``reference_value``. + ValueError: If the ``value_type`` is unknown. """ value_pb = getattr(value, "_pb", value) value_type = value_pb.WhichOneof("value_type") diff --git a/packages/google-cloud-firestore/google/cloud/firestore_v1/pipeline_result.py b/packages/google-cloud-firestore/google/cloud/firestore_v1/pipeline_result.py index 7edb9808d292..e3fd74677a1e 100644 --- a/packages/google-cloud-firestore/google/cloud/firestore_v1/pipeline_result.py +++ b/packages/google-cloud-firestore/google/cloud/firestore_v1/pipeline_result.py @@ -44,7 +44,6 @@ from google.cloud.firestore_v1.async_transaction import AsyncTransaction from google.cloud.firestore_v1.base_client import BaseClient from google.cloud.firestore_v1.base_document import BaseDocumentReference - from google.cloud.firestore_v1.bson import _BSONType from google.cloud.firestore_v1.client import Client from google.cloud.firestore_v1.pipeline import Pipeline from google.cloud.firestore_v1.pipeline_expressions import Constant @@ -139,7 +138,7 @@ def __eq__(self, other: object) -> bool: return NotImplemented return (self._ref == other._ref) and (self._fields_pb == other._fields_pb) - def data(self) -> dict | "Vector" | "_BSONType" | None: + def data(self) -> dict | "Vector" | None: """ Retrieves all fields in the result. From f4bba9badf12d569ea9a75d1320e946a3eec478d Mon Sep 17 00:00:00 2001 From: ohmayr Date: Mon, 21 Sep 2026 17:53:36 +0000 Subject: [PATCH 12/21] refactor(firestore): align decode_dict and PipelineResult.data return types with _BSONType - Annotate decode_dict with Union[dict, Vector, _BSONType]. - Update PipelineResult.data to return dict | Vector | _BSONType | None. - Import _BSONType under TYPE_CHECKING in pipeline_result.py. Towards #18402 --- .../google/cloud/firestore_v1/_helpers.py | 4 ++-- .../google/cloud/firestore_v1/pipeline_result.py | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/google-cloud-firestore/google/cloud/firestore_v1/_helpers.py b/packages/google-cloud-firestore/google/cloud/firestore_v1/_helpers.py index 92e033eee1ce..016679c3812b 100644 --- a/packages/google-cloud-firestore/google/cloud/firestore_v1/_helpers.py +++ b/packages/google-cloud-firestore/google/cloud/firestore_v1/_helpers.py @@ -431,7 +431,7 @@ def _decode_bson_dict_recursive(data: Any) -> Any: def decode_dict( value_fields, client=None, -) -> Union[dict, Vector, _BSONType, bytes]: +) -> Union[dict, Vector, _BSONType]: """Converts a protobuf map of Firestore ``Value``-s. Args: @@ -442,7 +442,7 @@ def decode_dict( Returns: Union[dict, ~google.cloud.firestore_v1.vector.Vector, \ - ~google.cloud.firestore_v1.bson._BSONType, bytes]: A dictionary of native \ + ~google.cloud.firestore_v1.bson._BSONType]: A dictionary of native \ Python values, Vector, or BSON object converted from ``value_fields``. """ value_fields_pb = getattr(value_fields, "_pb", value_fields) diff --git a/packages/google-cloud-firestore/google/cloud/firestore_v1/pipeline_result.py b/packages/google-cloud-firestore/google/cloud/firestore_v1/pipeline_result.py index e3fd74677a1e..7edb9808d292 100644 --- a/packages/google-cloud-firestore/google/cloud/firestore_v1/pipeline_result.py +++ b/packages/google-cloud-firestore/google/cloud/firestore_v1/pipeline_result.py @@ -44,6 +44,7 @@ from google.cloud.firestore_v1.async_transaction import AsyncTransaction from google.cloud.firestore_v1.base_client import BaseClient from google.cloud.firestore_v1.base_document import BaseDocumentReference + from google.cloud.firestore_v1.bson import _BSONType from google.cloud.firestore_v1.client import Client from google.cloud.firestore_v1.pipeline import Pipeline from google.cloud.firestore_v1.pipeline_expressions import Constant @@ -138,7 +139,7 @@ def __eq__(self, other: object) -> bool: return NotImplemented return (self._ref == other._ref) and (self._fields_pb == other._fields_pb) - def data(self) -> dict | "Vector" | None: + def data(self) -> dict | "Vector" | "_BSONType" | None: """ Retrieves all fields in the result. From f6327c104a92d5b7579e38dcb87c7ea9e57ac761 Mon Sep 17 00:00:00 2001 From: ohmayr Date: Tue, 22 Sep 2026 23:21:46 +0000 Subject: [PATCH 13/21] fix(firestore): restore required client parameter and format comprehensions for librarian - Make client a required positional parameter in decode_value and decode_dict. - Format comprehensions in _helpers.py as single lines to satisfy librarian generation check. Towards #18402 --- .../google/cloud/firestore_v1/_helpers.py | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/packages/google-cloud-firestore/google/cloud/firestore_v1/_helpers.py b/packages/google-cloud-firestore/google/cloud/firestore_v1/_helpers.py index 016679c3812b..9403dcfc999a 100644 --- a/packages/google-cloud-firestore/google/cloud/firestore_v1/_helpers.py +++ b/packages/google-cloud-firestore/google/cloud/firestore_v1/_helpers.py @@ -348,7 +348,7 @@ def reference_value_to_document(reference_value, client) -> Any: def decode_value( - value, client=None + value, client ) -> Union[ None, bool, @@ -407,8 +407,7 @@ def decode_value( ) elif value_type == "array_value": return [ - decode_value(element, client) - for element in value_pb.array_value.values + decode_value(element, client) for element in value_pb.array_value.values ] elif value_type == "map_value": return decode_dict(value_pb.map_value.fields, client) @@ -430,7 +429,7 @@ def _decode_bson_dict_recursive(data: Any) -> Any: def decode_dict( value_fields, - client=None, + client, ) -> Union[dict, Vector, _BSONType]: """Converts a protobuf map of Firestore ``Value``-s. @@ -446,10 +445,7 @@ def decode_dict( Python values, Vector, or BSON object converted from ``value_fields``. """ value_fields_pb = getattr(value_fields, "_pb", value_fields) - res = { - key: decode_value(value, client) - for key, value in value_fields_pb.items() - } + res = {key: decode_value(value, client) for key, value in value_fields_pb.items()} if res.get("__type__", None) == "__vector__": # Vector data type is represented as mapping. From b065d0f3c72160d434f7868961835c376ed1cd77 Mon Sep 17 00:00:00 2001 From: ohmayr Date: Wed, 23 Sep 2026 18:41:36 +0000 Subject: [PATCH 14/21] refactor(firestore): expose BSONType publicly Rename abstract base class _BSONType to BSONType and export it in google.cloud.firestore_v1 and __all__. Update return type annotations and docstrings on decode_value, decode_dict, and PipelineResult.data. --- .../firestore-integration.yaml | 4 ++++ .../google/cloud/firestore/__init__.py | 2 ++ .../google/cloud/firestore_v1/__init__.py | 2 ++ .../google/cloud/firestore_v1/_helpers.py | 16 ++++++++-------- .../google/cloud/firestore_v1/bson.py | 19 ++++++++++--------- .../cloud/firestore_v1/pipeline_result.py | 4 ++-- .../tests/unit/v1/test_bson.py | 6 +++--- 7 files changed, 31 insertions(+), 22 deletions(-) diff --git a/.librarian/generator-input/client-post-processing/firestore-integration.yaml b/.librarian/generator-input/client-post-processing/firestore-integration.yaml index f43581fb6130..dba36740817e 100644 --- a/.librarian/generator-input/client-post-processing/firestore-integration.yaml +++ b/.librarian/generator-input/client-post-processing/firestore-integration.yaml @@ -78,6 +78,7 @@ replacements: BSONObjectId, BSONRegex, BSONTimestamp, + BSONType, ) from google.cloud.firestore_v1.client import Client from google.cloud.firestore_v1.collection import CollectionReference @@ -188,6 +189,7 @@ replacements: "BSONObjectId", "BSONRegex", "BSONTimestamp", + "BSONType", "Client", "CountAggregation", "CollectionGroup", @@ -268,6 +270,7 @@ replacements: BSONObjectId, BSONRegex, BSONTimestamp, + BSONType, Client, CollectionGroup, CollectionReference, @@ -333,6 +336,7 @@ replacements: "BSONObjectId", "BSONRegex", "BSONTimestamp", + "BSONType", "Client", "CountAggregation", "CollectionGroup", diff --git a/packages/google-cloud-firestore/google/cloud/firestore/__init__.py b/packages/google-cloud-firestore/google/cloud/firestore/__init__.py index f14aa807d509..44a9651d6c02 100644 --- a/packages/google-cloud-firestore/google/cloud/firestore/__init__.py +++ b/packages/google-cloud-firestore/google/cloud/firestore/__init__.py @@ -43,6 +43,7 @@ BSONObjectId, BSONRegex, BSONTimestamp, + BSONType, Client, CollectionGroup, CollectionReference, @@ -108,6 +109,7 @@ "BSONObjectId", "BSONRegex", "BSONTimestamp", + "BSONType", "Client", "CountAggregation", "CollectionGroup", diff --git a/packages/google-cloud-firestore/google/cloud/firestore_v1/__init__.py b/packages/google-cloud-firestore/google/cloud/firestore_v1/__init__.py index f8a91acf9124..4656b5b5441f 100644 --- a/packages/google-cloud-firestore/google/cloud/firestore_v1/__init__.py +++ b/packages/google-cloud-firestore/google/cloud/firestore_v1/__init__.py @@ -55,6 +55,7 @@ BSONObjectId, BSONRegex, BSONTimestamp, + BSONType, ) from google.cloud.firestore_v1.client import Client from google.cloud.firestore_v1.collection import CollectionReference @@ -165,6 +166,7 @@ "BSONObjectId", "BSONRegex", "BSONTimestamp", + "BSONType", "Client", "CountAggregation", "CollectionGroup", diff --git a/packages/google-cloud-firestore/google/cloud/firestore_v1/_helpers.py b/packages/google-cloud-firestore/google/cloud/firestore_v1/_helpers.py index 9403dcfc999a..7334c788b993 100644 --- a/packages/google-cloud-firestore/google/cloud/firestore_v1/_helpers.py +++ b/packages/google-cloud-firestore/google/cloud/firestore_v1/_helpers.py @@ -44,7 +44,7 @@ import google from google.cloud import exceptions # type: ignore from google.cloud.firestore_v1 import transforms, types -from google.cloud.firestore_v1.bson import _BSONType +from google.cloud.firestore_v1.bson import BSONType from google.cloud.firestore_v1.field_path import FieldPath, parse_field_path from google.cloud.firestore_v1.types import common, document, write from google.cloud.firestore_v1.types.write import DocumentTransform @@ -211,7 +211,7 @@ def encode_value(value) -> types.document.Value: if document_path is not None: return document.Value(reference_value=document_path) - if isinstance(value, _BSONType): + if isinstance(value, BSONType): return encode_value(value._to_map_value()) if isinstance(value, GeoPoint): @@ -361,7 +361,7 @@ def decode_value( dict, GeoPoint, Vector, - _BSONType, + BSONType, ]: """Converts a Firestore protobuf ``Value`` to a native Python value. @@ -375,7 +375,7 @@ def decode_value( Union[NoneType, bool, int, float, datetime.datetime, \ str, bytes, dict, ~google.cloud.Firestore.GeoPoint, \ ~google.cloud.firestore_v1.vector.Vector, \ - ~google.cloud.firestore_v1.bson._BSONType]: A native \ + ~google.cloud.firestore_v1.bson.BSONType]: A native \ Python value converted from the ``value``. Raises: @@ -418,7 +418,7 @@ def decode_value( def _decode_bson_dict_recursive(data: Any) -> Any: """Recursively decodes BSON wire map dictionaries.""" if isinstance(data, dict): - decoded = _BSONType._from_dict(data) + decoded = BSONType._from_dict(data) if decoded is not None: return decoded return {k: _decode_bson_dict_recursive(v) for k, v in data.items()} @@ -430,7 +430,7 @@ def _decode_bson_dict_recursive(data: Any) -> Any: def decode_dict( value_fields, client, -) -> Union[dict, Vector, _BSONType]: +) -> Union[dict, Vector, BSONType]: """Converts a protobuf map of Firestore ``Value``-s. Args: @@ -441,7 +441,7 @@ def decode_dict( Returns: Union[dict, ~google.cloud.firestore_v1.vector.Vector, \ - ~google.cloud.firestore_v1.bson._BSONType]: A dictionary of native \ + ~google.cloud.firestore_v1.bson.BSONType]: A dictionary of native \ Python values, Vector, or BSON object converted from ``value_fields``. """ value_fields_pb = getattr(value_fields, "_pb", value_fields) @@ -453,7 +453,7 @@ def decode_dict( values = cast(Sequence[float], res["value"]) return Vector(values) - decoded = _BSONType._from_dict(res) + decoded = BSONType._from_dict(res) if decoded is not None: return decoded diff --git a/packages/google-cloud-firestore/google/cloud/firestore_v1/bson.py b/packages/google-cloud-firestore/google/cloud/firestore_v1/bson.py index 955ebd50ec99..fed3aa1d199a 100644 --- a/packages/google-cloud-firestore/google/cloud/firestore_v1/bson.py +++ b/packages/google-cloud-firestore/google/cloud/firestore_v1/bson.py @@ -30,6 +30,7 @@ from typing import Any, Callable, Dict, Union __all__ = [ + "BSONType", "BSONObjectId", "BSONMinKey", "BSONMaxKey", @@ -44,7 +45,7 @@ _HEX_24_REGEX = re.compile(r"^[0-9a-fA-F]{24}$") -class _BSONType(abc.ABC): +class BSONType(abc.ABC): """Abstract base class for all BSON type containers in Firestore.""" __slots__ = () @@ -87,7 +88,7 @@ def __repr__(self) -> str: return f"{self.__class__.__name__}()" -class BSONObjectId(_BSONType): +class BSONObjectId(BSONType): """Represents a 12-byte BSON ObjectId identifier. Args: @@ -146,7 +147,7 @@ def __hash__(self) -> int: return hash((type(self), self._value)) -class BSONMinKey(_BSONType): +class BSONMinKey(BSONType): """Represents the BSON MinKey sentinel value for query range boundaries.""" __slots__ = () @@ -164,7 +165,7 @@ def __hash__(self) -> int: return hash(type(self)) -class BSONMaxKey(_BSONType): +class BSONMaxKey(BSONType): """Represents the BSON MaxKey sentinel value for query range boundaries.""" __slots__ = () @@ -182,7 +183,7 @@ def __hash__(self) -> int: return hash(type(self)) -class BSONInt32(_BSONType): +class BSONInt32(BSONType): """Represents a 32-bit signed integer value container for Firestore BSON. Args: @@ -239,7 +240,7 @@ def __hash__(self) -> int: return hash((type(self), self._value)) -class BSONBinary(_BSONType): +class BSONBinary(BSONType): """Represents a BSON binary data container with a subtype for Firestore. Args: @@ -304,7 +305,7 @@ def __hash__(self) -> int: return hash((type(self), self._data, self._subtype)) -class BSONTimestamp(_BSONType): +class BSONTimestamp(BSONType): """Container for BSON Timestamp values. Args: @@ -365,7 +366,7 @@ def __hash__(self) -> int: return hash((type(self), self._seconds, self._increment)) -class BSONRegex(_BSONType): +class BSONRegex(BSONType): """Represents a BSON Regular Expression container for Firestore. Args: @@ -427,7 +428,7 @@ def __hash__(self) -> int: return hash((type(self), self._pattern, self._options)) -class BSONDecimal128(_BSONType): +class BSONDecimal128(BSONType): """Represents a BSON 128-bit Decimal container for Firestore. Args: diff --git a/packages/google-cloud-firestore/google/cloud/firestore_v1/pipeline_result.py b/packages/google-cloud-firestore/google/cloud/firestore_v1/pipeline_result.py index 7edb9808d292..80ddcf6d98ff 100644 --- a/packages/google-cloud-firestore/google/cloud/firestore_v1/pipeline_result.py +++ b/packages/google-cloud-firestore/google/cloud/firestore_v1/pipeline_result.py @@ -44,7 +44,7 @@ from google.cloud.firestore_v1.async_transaction import AsyncTransaction from google.cloud.firestore_v1.base_client import BaseClient from google.cloud.firestore_v1.base_document import BaseDocumentReference - from google.cloud.firestore_v1.bson import _BSONType + from google.cloud.firestore_v1.bson import BSONType from google.cloud.firestore_v1.client import Client from google.cloud.firestore_v1.pipeline import Pipeline from google.cloud.firestore_v1.pipeline_expressions import Constant @@ -139,7 +139,7 @@ def __eq__(self, other: object) -> bool: return NotImplemented return (self._ref == other._ref) and (self._fields_pb == other._fields_pb) - def data(self) -> dict | "Vector" | "_BSONType" | None: + def data(self) -> dict | "Vector" | "BSONType" | None: """ Retrieves all fields in the result. diff --git a/packages/google-cloud-firestore/tests/unit/v1/test_bson.py b/packages/google-cloud-firestore/tests/unit/v1/test_bson.py index 92a3a1b128e3..c544d6892245 100644 --- a/packages/google-cloud-firestore/tests/unit/v1/test_bson.py +++ b/packages/google-cloud-firestore/tests/unit/v1/test_bson.py @@ -31,18 +31,18 @@ BSONObjectId, BSONRegex, BSONTimestamp, - _BSONType, + BSONType, ) def test_bson_type_abc_cannot_be_instantiated(): with pytest.raises(TypeError): - _BSONType() # type: ignore + BSONType() # type: ignore def test_bson_type_inheritance(): oid = BSONObjectId("507f191e810c19729de860ea") - assert isinstance(oid, _BSONType) + assert isinstance(oid, BSONType) def test_bson_object_id_from_hex_string(): From 8217cd3db1851f91705005ceecd52837643818a6 Mon Sep 17 00:00:00 2001 From: ohmayr Date: Wed, 16 Sep 2026 22:48:11 +0000 Subject: [PATCH 15/21] feat(firestore): add BSON cross-type query ordering support --- .../google/cloud/firestore_v1/order.py | 140 +++++++++++++++--- .../tests/system/test_system.py | 20 +++ .../tests/unit/v1/test_order.py | 58 ++++++++ 3 files changed, 194 insertions(+), 24 deletions(-) diff --git a/packages/google-cloud-firestore/google/cloud/firestore_v1/order.py b/packages/google-cloud-firestore/google/cloud/firestore_v1/order.py index a3d65cc5000e..037f447fecca 100644 --- a/packages/google-cloud-firestore/google/cloud/firestore_v1/order.py +++ b/packages/google-cloud-firestore/google/cloud/firestore_v1/order.py @@ -36,10 +36,16 @@ class TypeOrder(Enum): ARRAY = 8 OBJECT = 9 VECTOR = 10 + BSON_MIN_KEY = 11 + BSON_MAX_KEY = 12 + BSON_OBJECT_ID = 13 + BSON_BINARY = 14 + BSON_REGEX = 15 @staticmethod def from_value(value) -> Any: - v = value._pb.WhichOneof("value_type") + value_pb = getattr(value, "_pb", value) + v = value_pb.WhichOneof("value_type") lut = { "null_value": TypeOrder.NULL, "boolean_value": TypeOrder.BOOLEAN, @@ -58,10 +64,24 @@ def from_value(value) -> Any: raise ValueError(f"Could not detect value type for {v}") if v == "map_value": - if ( - "__type__" in value.map_value.fields - and value.map_value.fields["__type__"].string_value == "__vector__" - ): + fields = value_pb.map_value.fields + if len(fields) == 1: + key = next(iter(fields)) + if key == "__min__": + return TypeOrder.BSON_MIN_KEY + if key == "__max__": + return TypeOrder.BSON_MAX_KEY + if key == "__oid__": + return TypeOrder.BSON_OBJECT_ID + if key in ("__int__", "__decimal128__"): + return TypeOrder.NUMBER + if key == "__binary__": + return TypeOrder.BSON_BINARY + if key == "__regex__": + return TypeOrder.BSON_REGEX + if key == "__request_timestamp__": + return TypeOrder.TIMESTAMP + if "__type__" in fields and fields["__type__"].string_value == "__vector__": return TypeOrder.VECTOR return lut[v] @@ -69,16 +89,21 @@ def from_value(value) -> Any: # NOTE: This order is defined by the backend and cannot be changed. _TYPE_ORDER_MAP = { TypeOrder.NULL: 0, - TypeOrder.BOOLEAN: 1, - TypeOrder.NUMBER: 2, - TypeOrder.TIMESTAMP: 3, - TypeOrder.STRING: 4, - TypeOrder.BLOB: 5, - TypeOrder.REF: 6, - TypeOrder.GEO_POINT: 7, - TypeOrder.ARRAY: 8, - TypeOrder.VECTOR: 9, - TypeOrder.OBJECT: 10, + TypeOrder.BSON_MIN_KEY: 1, + TypeOrder.BOOLEAN: 2, + TypeOrder.NUMBER: 3, + TypeOrder.TIMESTAMP: 4, + TypeOrder.STRING: 5, + TypeOrder.BLOB: 6, + TypeOrder.BSON_BINARY: 7, + TypeOrder.REF: 8, + TypeOrder.BSON_OBJECT_ID: 9, + TypeOrder.GEO_POINT: 10, + TypeOrder.BSON_REGEX: 11, + TypeOrder.ARRAY: 12, + TypeOrder.VECTOR: 13, + TypeOrder.OBJECT: 14, + TypeOrder.BSON_MAX_KEY: 15, } @@ -102,8 +127,12 @@ def compare(cls, left, right) -> int: else: return 1 - if leftType == TypeOrder.NULL: - return 0 # nulls are all equal + if ( + leftType == TypeOrder.NULL + or leftType == TypeOrder.BSON_MIN_KEY + or leftType == TypeOrder.BSON_MAX_KEY + ): + return 0 # sentinels are equal elif leftType == TypeOrder.BOOLEAN: return cls._compare_to(left.boolean_value, right.boolean_value) elif leftType == TypeOrder.NUMBER: @@ -114,10 +143,16 @@ def compare(cls, left, right) -> int: return cls._compare_to(left.string_value, right.string_value) elif leftType == TypeOrder.BLOB: return cls.compare_blobs(left, right) + elif leftType == TypeOrder.BSON_BINARY: + return cls.compare_bson_binaries(left, right) elif leftType == TypeOrder.REF: return cls.compare_resource_paths(left, right) + elif leftType == TypeOrder.BSON_OBJECT_ID: + return cls.compare_bson_object_ids(left, right) elif leftType == TypeOrder.GEO_POINT: return cls.compare_geo_points(left, right) + elif leftType == TypeOrder.BSON_REGEX: + return cls.compare_bson_regexes(left, right) elif leftType == TypeOrder.ARRAY: return cls.compare_arrays(left, right) elif leftType == TypeOrder.VECTOR: @@ -135,16 +170,69 @@ def compare_blobs(left, right) -> int: return Order._compare_to(left_bytes, right_bytes) + @staticmethod + def compare_bson_binaries(left, right) -> int: + l_bin = left.map_value.fields["__binary__"].bytes_value + r_bin = right.map_value.fields["__binary__"].bytes_value + + l_subtype = l_bin[0] if l_bin else 0 + r_subtype = r_bin[0] if r_bin else 0 + + cmp_subtype = Order._compare_to(l_subtype, r_subtype) + if cmp_subtype != 0: + return cmp_subtype + + return Order._compare_to( + l_bin[1:] if l_bin else b"", r_bin[1:] if r_bin else b"" + ) + + @staticmethod + def compare_bson_object_ids(left, right) -> int: + l_oid = left.map_value.fields["__oid__"].string_value + r_oid = right.map_value.fields["__oid__"].string_value + return Order._compare_to(l_oid, r_oid) + + @staticmethod + def compare_bson_regexes(left, right) -> int: + l_regex = left.map_value.fields["__regex__"].map_value.fields + r_regex = right.map_value.fields["__regex__"].map_value.fields + + l_pattern = l_regex["pattern"].string_value if "pattern" in l_regex else "" + r_pattern = r_regex["pattern"].string_value if "pattern" in r_regex else "" + cmp_pat = Order._compare_to(l_pattern, r_pattern) + if cmp_pat != 0: + return cmp_pat + + l_options = l_regex["options"].string_value if "options" in l_regex else "" + r_options = r_regex["options"].string_value if "options" in r_regex else "" + return Order._compare_to(l_options, r_options) + @staticmethod def compare_timestamps(left, right) -> Any: - left = left._pb.timestamp_value - right = right._pb.timestamp_value + left_pb = getattr(left, "_pb", left) + right_pb = getattr(right, "_pb", right) + + if left_pb.WhichOneof("value_type") == "map_value": + l_ts = left_pb.map_value.fields["__request_timestamp__"].map_value.fields + l_sec = l_ts["seconds"].integer_value if "seconds" in l_ts else 0 + l_inc = l_ts["increment"].integer_value if "increment" in l_ts else 0 + else: + l_sec = left_pb.timestamp_value.seconds + l_inc = left_pb.timestamp_value.nanos - seconds = Order._compare_to(left.seconds or 0, right.seconds or 0) + if right_pb.WhichOneof("value_type") == "map_value": + r_ts = right_pb.map_value.fields["__request_timestamp__"].map_value.fields + r_sec = r_ts["seconds"].integer_value if "seconds" in r_ts else 0 + r_inc = r_ts["increment"].integer_value if "increment" in r_ts else 0 + else: + r_sec = right_pb.timestamp_value.seconds + r_inc = right_pb.timestamp_value.nanos + + seconds = Order._compare_to(l_sec, r_sec) if seconds != 0: return seconds - return Order._compare_to(left.nanos or 0, right.nanos or 0) + return Order._compare_to(l_inc, r_inc) @staticmethod def compare_geo_points(left, right) -> Any: @@ -231,9 +319,13 @@ def compare_objects(left, right) -> int: @staticmethod def compare_numbers(left, right) -> int: - left_value = decode_value(left, None) - right_value = decode_value(right, None) - return Order.compare_doubles(left_value, right_value) + left_val = decode_value(left, None, decode_bson=True) + right_val = decode_value(right, None, decode_bson=True) + if hasattr(left_val, "value"): + left_val = left_val.value + if hasattr(right_val, "value"): + right_val = right_val.value + return Order.compare_doubles(float(left_val), float(right_val)) @staticmethod def compare_doubles(left, right) -> int: diff --git a/packages/google-cloud-firestore/tests/system/test_system.py b/packages/google-cloud-firestore/tests/system/test_system.py index fc864e9d555d..91f79f3431cd 100644 --- a/packages/google-cloud-firestore/tests/system/test_system.py +++ b/packages/google-cloud-firestore/tests/system/test_system.py @@ -1324,6 +1324,26 @@ def test_bson_regex_invalid_options(client, cleanup, database): assert "Invalid regex option" in exc_info.value.message +def test_bson_query_ordering(client, cleanup, database): + """Test server query ordering for BSON types.""" + collection_id = "bson_ordering_" + UNIQUE_RESOURCE_ID + coll_ref = client.collection(collection_id) + + doc1 = coll_ref.document("doc1") + doc2 = coll_ref.document("doc2") + doc3 = coll_ref.document("doc3") + cleanup.extend([doc1.delete, doc2.delete, doc3.delete]) + + doc1.set({"val": BSONMinKey()}) + doc2.set({"val": BSONInt32(10)}) + doc3.set({"val": BSONMaxKey()}) + + query = coll_ref.order_by("val") + results = [doc.to_dict(decode_bson=True)["val"] for doc in query.stream()] + assert results == [BSONMinKey(), BSONInt32(10), BSONMaxKey()] + + + @pytest.fixture(scope="module") def query_docs(client, database): collection_id = "qs" + UNIQUE_RESOURCE_ID diff --git a/packages/google-cloud-firestore/tests/unit/v1/test_order.py b/packages/google-cloud-firestore/tests/unit/v1/test_order.py index 1942a5298438..51659c307f00 100644 --- a/packages/google-cloud-firestore/tests/unit/v1/test_order.py +++ b/packages/google-cloud-firestore/tests/unit/v1/test_order.py @@ -199,6 +199,64 @@ def test_order_all_value_present(): assert type_order in _TYPE_ORDER_MAP +def test_order_bson_type_ordering(): + from google.cloud.firestore_v1._helpers import encode_value + from google.cloud.firestore_v1.bson import ( + BSONBinary, + BSONDecimal128, + BSONInt32, + BSONMaxKey, + BSONMinKey, + BSONObjectId, + BSONRegex, + BSONTimestamp, + ) + from google.cloud.firestore_v1.order import Order + + min_k = encode_value(BSONMinKey()) + max_k = encode_value(BSONMaxKey()) + null_v = nullValue() + int32_v = encode_value(BSONInt32(10)) + int64_v = _int_value(10) + dec_v = encode_value(BSONDecimal128("10.0")) + ts_bson = encode_value(BSONTimestamp(100, 1)) + ts_native = _timestamp_value(100, 0) + bin_b = encode_value(BSONBinary(b"xyz", subtype=1)) + bytes_native = _blob_value(b"xyz") + ref_v = _reference_value("projects/p1/databases/d1/documents/c1/doc1") + oid_v = encode_value(BSONObjectId("507f191e810c19729de860ea")) + geo_v = _geoPoint_value(0, 0) + regex_v = encode_value(BSONRegex("abc")) + arr_v = _array_value() + map_v = _object_value({"a": 1}) + + # Test 16-rank ordering bounds + target = Order() + assert target.compare(null_v, min_k) == -1 + assert target.compare(min_k, null_v) == 1 + + assert target.compare(max_k, map_v) == 1 + assert target.compare(map_v, max_k) == -1 + + # Test numbers comparison equality across int32, int64, decimal128 + assert target.compare(int32_v, int64_v) == 0 + assert target.compare(int32_v, dec_v) == 0 + + # Test timestamp comparison (native timestamp < BSON timestamp with increment) + assert target.compare(ts_native, ts_bson) == -1 + + # Test BSON binary > bytes + assert target.compare(bytes_native, bin_b) == -1 + + # Test ObjectId rank (REF < OID < GEO_POINT) + assert target.compare(ref_v, oid_v) == -1 + assert target.compare(oid_v, geo_v) == -1 + + # Test Regex rank (GEO_POINT < REGEX < ARRAY) + assert target.compare(geo_v, regex_v) == -1 + assert target.compare(regex_v, arr_v) == -1 + + def test_order_compare_w_objects_different_keys(): left = _object_value({"foo": 0}) right = _object_value({"bar": 0}) From fcbc4d1b46c3b4f94ee1a227aedb279d82904b91 Mon Sep 17 00:00:00 2001 From: ohmayr Date: Mon, 21 Sep 2026 19:30:16 +0000 Subject: [PATCH 16/21] fix(firestore): streamline BSON cross-type ordering and numeric comparisons Use _BSON_KEY_TO_TYPE_ORDER dictionary lookup in order.py for O(1) wire key resolution, decoupling bson.py from query ordering. Consolidate cross-type numeric comparisons in compare_numbers with safe Decimal handling and restore compare_doubles to standard float comparisons. --- .../google/cloud/firestore_v1/order.py | 74 +++++++++++++------ .../tests/system/test_system.py | 2 +- .../tests/unit/v1/test_order.py | 26 +++++++ 3 files changed, 80 insertions(+), 22 deletions(-) diff --git a/packages/google-cloud-firestore/google/cloud/firestore_v1/order.py b/packages/google-cloud-firestore/google/cloud/firestore_v1/order.py index 037f447fecca..e4d421a77eec 100644 --- a/packages/google-cloud-firestore/google/cloud/firestore_v1/order.py +++ b/packages/google-cloud-firestore/google/cloud/firestore_v1/order.py @@ -12,6 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. +import decimal import math from enum import Enum from typing import Any @@ -67,26 +68,33 @@ def from_value(value) -> Any: fields = value_pb.map_value.fields if len(fields) == 1: key = next(iter(fields)) - if key == "__min__": - return TypeOrder.BSON_MIN_KEY - if key == "__max__": - return TypeOrder.BSON_MAX_KEY - if key == "__oid__": - return TypeOrder.BSON_OBJECT_ID - if key in ("__int__", "__decimal128__"): - return TypeOrder.NUMBER - if key == "__binary__": - return TypeOrder.BSON_BINARY - if key == "__regex__": - return TypeOrder.BSON_REGEX - if key == "__request_timestamp__": - return TypeOrder.TIMESTAMP + bson_order = _BSON_KEY_TO_TYPE_ORDER.get(key) + if bson_order is not None: + return bson_order if "__type__" in fields and fields["__type__"].string_value == "__vector__": return TypeOrder.VECTOR return lut[v] +# Maps BSON wire map keys directly to their corresponding TypeOrder. +# BSONTimestamp maps to TypeOrder.TIMESTAMP, and BSONInt32 / BSONDecimal128 +# map to TypeOrder.NUMBER, enabling cross-type comparisons. +_BSON_KEY_TO_TYPE_ORDER = { + "__min__": TypeOrder.BSON_MIN_KEY, + "__max__": TypeOrder.BSON_MAX_KEY, + "__oid__": TypeOrder.BSON_OBJECT_ID, + "__int__": TypeOrder.NUMBER, + "__decimal128__": TypeOrder.NUMBER, + "__binary__": TypeOrder.BSON_BINARY, + "__request_timestamp__": TypeOrder.TIMESTAMP, + "__regex__": TypeOrder.BSON_REGEX, +} + + # NOTE: This order is defined by the backend and cannot be changed. +# BSONTimestamp shares TypeOrder.TIMESTAMP with native timestamps, and +# BSONInt32 / BSONDecimal128 share TypeOrder.NUMBER, enabling direct cross-type +# value comparison within those categories. _TYPE_ORDER_MAP = { TypeOrder.NULL: 0, TypeOrder.BSON_MIN_KEY: 1, @@ -136,8 +144,10 @@ def compare(cls, left, right) -> int: elif leftType == TypeOrder.BOOLEAN: return cls._compare_to(left.boolean_value, right.boolean_value) elif leftType == TypeOrder.NUMBER: + # Handles int64, double, BSONInt32, and BSONDecimal128. return cls.compare_numbers(left, right) elif leftType == TypeOrder.TIMESTAMP: + # Handles native Firestore timestamps and BSONTimestamp. return cls.compare_timestamps(left, right) elif leftType == TypeOrder.STRING: return cls._compare_to(left.string_value, right.string_value) @@ -209,6 +219,7 @@ def compare_bson_regexes(left, right) -> int: @staticmethod def compare_timestamps(left, right) -> Any: + """Compare native Firestore timestamps and BSON timestamps.""" left_pb = getattr(left, "_pb", left) right_pb = getattr(right, "_pb", right) @@ -319,13 +330,34 @@ def compare_objects(left, right) -> int: @staticmethod def compare_numbers(left, right) -> int: - left_val = decode_value(left, None, decode_bson=True) - right_val = decode_value(right, None, decode_bson=True) - if hasattr(left_val, "value"): - left_val = left_val.value - if hasattr(right_val, "value"): - right_val = right_val.value - return Order.compare_doubles(float(left_val), float(right_val)) + """Compare numeric values across int, float, BSONInt32, and BSONDecimal128.""" + + def _to_number(val): + num = decode_value(val, None) + to_decimal = getattr(num, "to_decimal", None) + return ( + to_decimal() + if callable(to_decimal) + else getattr(num, "value", num) + ) + + l = _to_number(left) + r = _to_number(right) + + l_nan = l.is_nan() if hasattr(l, "is_nan") else math.isnan(l) + r_nan = r.is_nan() if hasattr(r, "is_nan") else math.isnan(r) + if l_nan or r_nan: + return 0 if (l_nan and r_nan) else (-1 if l_nan else 1) + + # Python raises TypeError when comparing Decimal with float directly, + # but allows comparing Decimal with int. Convert float to Decimal + # to ensure safe cross-type comparison without float overflow. + if isinstance(l, decimal.Decimal) and isinstance(r, float): + r = decimal.Decimal(str(r)) + elif isinstance(r, decimal.Decimal) and isinstance(l, float): + l = decimal.Decimal(str(l)) + + return Order._compare_to(l, r) @staticmethod def compare_doubles(left, right) -> int: diff --git a/packages/google-cloud-firestore/tests/system/test_system.py b/packages/google-cloud-firestore/tests/system/test_system.py index 91f79f3431cd..1a71441a683c 100644 --- a/packages/google-cloud-firestore/tests/system/test_system.py +++ b/packages/google-cloud-firestore/tests/system/test_system.py @@ -1339,7 +1339,7 @@ def test_bson_query_ordering(client, cleanup, database): doc3.set({"val": BSONMaxKey()}) query = coll_ref.order_by("val") - results = [doc.to_dict(decode_bson=True)["val"] for doc in query.stream()] + results = [doc.to_dict()["val"] for doc in query.stream()] assert results == [BSONMinKey(), BSONInt32(10), BSONMaxKey()] diff --git a/packages/google-cloud-firestore/tests/unit/v1/test_order.py b/packages/google-cloud-firestore/tests/unit/v1/test_order.py index 51659c307f00..977efd6dd302 100644 --- a/packages/google-cloud-firestore/tests/unit/v1/test_order.py +++ b/packages/google-cloud-firestore/tests/unit/v1/test_order.py @@ -242,6 +242,16 @@ def test_order_bson_type_ordering(): assert target.compare(int32_v, int64_v) == 0 assert target.compare(int32_v, dec_v) == 0 + # Test large decimal comparison exceeding float limit + large_dec = encode_value(BSONDecimal128("1e1000")) + assert target.compare(large_dec, _double_value(1e300)) == 1 + assert target.compare(_double_value(1e300), large_dec) == -1 + + # Test decimal NaN comparison + nan_dec = encode_value(BSONDecimal128("NaN")) + assert target.compare(nan_dec, int32_v) == -1 + assert target.compare(int32_v, nan_dec) == 1 + # Test timestamp comparison (native timestamp < BSON timestamp with increment) assert target.compare(ts_native, ts_bson) == -1 @@ -256,6 +266,22 @@ def test_order_bson_type_ordering(): assert target.compare(geo_v, regex_v) == -1 assert target.compare(regex_v, arr_v) == -1 + # Verify _BSON_KEY_TO_TYPE_ORDER mapping directly + from google.cloud.firestore_v1.order import _BSON_KEY_TO_TYPE_ORDER, TypeOrder + + expected_orders = { + "__min__": TypeOrder.BSON_MIN_KEY, + "__max__": TypeOrder.BSON_MAX_KEY, + "__oid__": TypeOrder.BSON_OBJECT_ID, + "__int__": TypeOrder.NUMBER, + "__decimal128__": TypeOrder.NUMBER, + "__binary__": TypeOrder.BSON_BINARY, + "__regex__": TypeOrder.BSON_REGEX, + "__request_timestamp__": TypeOrder.TIMESTAMP, + } + for key, expected_order in expected_orders.items(): + assert _BSON_KEY_TO_TYPE_ORDER.get(key) == expected_order + def test_order_compare_w_objects_different_keys(): left = _object_value({"foo": 0}) From 9874bff5a8fc10c43a0055a3e9bf33d23c59fdb3 Mon Sep 17 00:00:00 2001 From: ohmayr Date: Mon, 21 Sep 2026 19:47:50 +0000 Subject: [PATCH 17/21] fix(firestore): resolve lint E741 and format in compare_numbers Rename ambiguous single-letter variable l to left_val to satisfy flake8 E741 and align expression formatting with ruff. --- .../google/cloud/firestore_v1/order.py | 34 ++++++++++--------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/packages/google-cloud-firestore/google/cloud/firestore_v1/order.py b/packages/google-cloud-firestore/google/cloud/firestore_v1/order.py index e4d421a77eec..d5e740c03118 100644 --- a/packages/google-cloud-firestore/google/cloud/firestore_v1/order.py +++ b/packages/google-cloud-firestore/google/cloud/firestore_v1/order.py @@ -335,29 +335,31 @@ def compare_numbers(left, right) -> int: def _to_number(val): num = decode_value(val, None) to_decimal = getattr(num, "to_decimal", None) - return ( - to_decimal() - if callable(to_decimal) - else getattr(num, "value", num) - ) + return to_decimal() if callable(to_decimal) else getattr(num, "value", num) - l = _to_number(left) - r = _to_number(right) + left_val = _to_number(left) + right_val = _to_number(right) - l_nan = l.is_nan() if hasattr(l, "is_nan") else math.isnan(l) - r_nan = r.is_nan() if hasattr(r, "is_nan") else math.isnan(r) - if l_nan or r_nan: - return 0 if (l_nan and r_nan) else (-1 if l_nan else 1) + left_nan = ( + left_val.is_nan() if hasattr(left_val, "is_nan") else math.isnan(left_val) + ) + right_nan = ( + right_val.is_nan() + if hasattr(right_val, "is_nan") + else math.isnan(right_val) + ) + if left_nan or right_nan: + return 0 if (left_nan and right_nan) else (-1 if left_nan else 1) # Python raises TypeError when comparing Decimal with float directly, # but allows comparing Decimal with int. Convert float to Decimal # to ensure safe cross-type comparison without float overflow. - if isinstance(l, decimal.Decimal) and isinstance(r, float): - r = decimal.Decimal(str(r)) - elif isinstance(r, decimal.Decimal) and isinstance(l, float): - l = decimal.Decimal(str(l)) + if isinstance(left_val, decimal.Decimal) and isinstance(right_val, float): + right_val = decimal.Decimal(str(right_val)) + elif isinstance(right_val, decimal.Decimal) and isinstance(left_val, float): + left_val = decimal.Decimal(str(left_val)) - return Order._compare_to(l, r) + return Order._compare_to(left_val, right_val) @staticmethod def compare_doubles(left, right) -> int: From 705274a907a341c049c65391ac78b60cc409cd8d Mon Sep 17 00:00:00 2001 From: ohmayr Date: Wed, 23 Sep 2026 19:33:43 +0000 Subject: [PATCH 18/21] fix(firestore): separate BSON_TIMESTAMP in TypeOrder and optimize compare_numbers Separate BSON_TIMESTAMP from native TIMESTAMP to conform to the cross-SDK 17-rank TypeOrder specification. Restore compare_timestamps to native Firestore timestamps and introduce compare_bson_timestamps. Extract module-level _to_number and _is_nan helpers, directly inspecting protobuf fields to optimize numeric comparisons. --- .../google/cloud/firestore_v1/order.py | 126 +++++++++++------- .../tests/system/test_system.py | 1 - .../tests/unit/v1/test_order.py | 12 +- 3 files changed, 90 insertions(+), 49 deletions(-) diff --git a/packages/google-cloud-firestore/google/cloud/firestore_v1/order.py b/packages/google-cloud-firestore/google/cloud/firestore_v1/order.py index d5e740c03118..dab281d82fcc 100644 --- a/packages/google-cloud-firestore/google/cloud/firestore_v1/order.py +++ b/packages/google-cloud-firestore/google/cloud/firestore_v1/order.py @@ -20,6 +20,45 @@ from google.cloud.firestore_v1._helpers import GeoPoint, decode_value +def _to_number(val: Any) -> Any: + """Extract a numeric value (int, float, Decimal) from a Value protobuf or Python value. + + Directly inspects the protobuf value_type without calling decode_value() + for optimal performance. + """ + value_pb = getattr(val, "_pb", val) + which = ( + value_pb.WhichOneof("value_type") if hasattr(value_pb, "WhichOneof") else None + ) + + if which == "integer_value": + return value_pb.integer_value + elif which == "double_value": + return value_pb.double_value + elif which == "map_value": + fields = value_pb.map_value.fields + if "__int__" in fields: + return fields["__int__"].integer_value + elif "__decimal128__" in fields: + return decimal.Decimal(fields["__decimal128__"].string_value) + + num = decode_value(val, None) + to_decimal = getattr(num, "to_decimal", None) + return to_decimal() if callable(to_decimal) else getattr(num, "value", num) + + +def _is_nan(val: Any) -> bool: + """Check if a numeric value is NaN, safely handling OverflowError and non-floats.""" + if hasattr(val, "is_nan"): + return val.is_nan() + if isinstance(val, (int, decimal.Decimal)): + return False + try: + return math.isnan(val) + except (TypeError, OverflowError): + return False + + class TypeOrder(Enum): """The supported Data Type. @@ -42,6 +81,7 @@ class TypeOrder(Enum): BSON_OBJECT_ID = 13 BSON_BINARY = 14 BSON_REGEX = 15 + BSON_TIMESTAMP = 16 @staticmethod def from_value(value) -> Any: @@ -77,8 +117,7 @@ def from_value(value) -> Any: # Maps BSON wire map keys directly to their corresponding TypeOrder. -# BSONTimestamp maps to TypeOrder.TIMESTAMP, and BSONInt32 / BSONDecimal128 -# map to TypeOrder.NUMBER, enabling cross-type comparisons. +# BSONInt32 and BSONDecimal128 map to TypeOrder.NUMBER, enabling cross-type comparisons. _BSON_KEY_TO_TYPE_ORDER = { "__min__": TypeOrder.BSON_MIN_KEY, "__max__": TypeOrder.BSON_MAX_KEY, @@ -86,32 +125,30 @@ def from_value(value) -> Any: "__int__": TypeOrder.NUMBER, "__decimal128__": TypeOrder.NUMBER, "__binary__": TypeOrder.BSON_BINARY, - "__request_timestamp__": TypeOrder.TIMESTAMP, + "__request_timestamp__": TypeOrder.BSON_TIMESTAMP, "__regex__": TypeOrder.BSON_REGEX, } # NOTE: This order is defined by the backend and cannot be changed. -# BSONTimestamp shares TypeOrder.TIMESTAMP with native timestamps, and -# BSONInt32 / BSONDecimal128 share TypeOrder.NUMBER, enabling direct cross-type -# value comparison within those categories. _TYPE_ORDER_MAP = { TypeOrder.NULL: 0, TypeOrder.BSON_MIN_KEY: 1, TypeOrder.BOOLEAN: 2, TypeOrder.NUMBER: 3, TypeOrder.TIMESTAMP: 4, - TypeOrder.STRING: 5, - TypeOrder.BLOB: 6, - TypeOrder.BSON_BINARY: 7, - TypeOrder.REF: 8, - TypeOrder.BSON_OBJECT_ID: 9, - TypeOrder.GEO_POINT: 10, - TypeOrder.BSON_REGEX: 11, - TypeOrder.ARRAY: 12, - TypeOrder.VECTOR: 13, - TypeOrder.OBJECT: 14, - TypeOrder.BSON_MAX_KEY: 15, + TypeOrder.BSON_TIMESTAMP: 5, + TypeOrder.STRING: 6, + TypeOrder.BLOB: 7, + TypeOrder.BSON_BINARY: 8, + TypeOrder.REF: 9, + TypeOrder.BSON_OBJECT_ID: 10, + TypeOrder.GEO_POINT: 11, + TypeOrder.BSON_REGEX: 12, + TypeOrder.ARRAY: 13, + TypeOrder.VECTOR: 14, + TypeOrder.OBJECT: 15, + TypeOrder.BSON_MAX_KEY: 16, } @@ -147,8 +184,9 @@ def compare(cls, left, right) -> int: # Handles int64, double, BSONInt32, and BSONDecimal128. return cls.compare_numbers(left, right) elif leftType == TypeOrder.TIMESTAMP: - # Handles native Firestore timestamps and BSONTimestamp. return cls.compare_timestamps(left, right) + elif leftType == TypeOrder.BSON_TIMESTAMP: + return cls.compare_bson_timestamps(left, right) elif leftType == TypeOrder.STRING: return cls._compare_to(left.string_value, right.string_value) elif leftType == TypeOrder.BLOB: @@ -219,25 +257,31 @@ def compare_bson_regexes(left, right) -> int: @staticmethod def compare_timestamps(left, right) -> Any: - """Compare native Firestore timestamps and BSON timestamps.""" left_pb = getattr(left, "_pb", left) right_pb = getattr(right, "_pb", right) - if left_pb.WhichOneof("value_type") == "map_value": - l_ts = left_pb.map_value.fields["__request_timestamp__"].map_value.fields - l_sec = l_ts["seconds"].integer_value if "seconds" in l_ts else 0 - l_inc = l_ts["increment"].integer_value if "increment" in l_ts else 0 - else: - l_sec = left_pb.timestamp_value.seconds - l_inc = left_pb.timestamp_value.nanos + seconds = Order._compare_to( + left_pb.timestamp_value.seconds, right_pb.timestamp_value.seconds + ) + if seconds != 0: + return seconds - if right_pb.WhichOneof("value_type") == "map_value": - r_ts = right_pb.map_value.fields["__request_timestamp__"].map_value.fields - r_sec = r_ts["seconds"].integer_value if "seconds" in r_ts else 0 - r_inc = r_ts["increment"].integer_value if "increment" in r_ts else 0 - else: - r_sec = right_pb.timestamp_value.seconds - r_inc = right_pb.timestamp_value.nanos + return Order._compare_to( + left_pb.timestamp_value.nanos, right_pb.timestamp_value.nanos + ) + + @staticmethod + def compare_bson_timestamps(left, right) -> Any: + left_pb = getattr(left, "_pb", left) + right_pb = getattr(right, "_pb", right) + + l_ts = left_pb.map_value.fields["__request_timestamp__"].map_value.fields + l_sec = l_ts["seconds"].integer_value if "seconds" in l_ts else 0 + l_inc = l_ts["increment"].integer_value if "increment" in l_ts else 0 + + r_ts = right_pb.map_value.fields["__request_timestamp__"].map_value.fields + r_sec = r_ts["seconds"].integer_value if "seconds" in r_ts else 0 + r_inc = r_ts["increment"].integer_value if "increment" in r_ts else 0 seconds = Order._compare_to(l_sec, r_sec) if seconds != 0: @@ -331,23 +375,11 @@ def compare_objects(left, right) -> int: @staticmethod def compare_numbers(left, right) -> int: """Compare numeric values across int, float, BSONInt32, and BSONDecimal128.""" - - def _to_number(val): - num = decode_value(val, None) - to_decimal = getattr(num, "to_decimal", None) - return to_decimal() if callable(to_decimal) else getattr(num, "value", num) - left_val = _to_number(left) right_val = _to_number(right) - left_nan = ( - left_val.is_nan() if hasattr(left_val, "is_nan") else math.isnan(left_val) - ) - right_nan = ( - right_val.is_nan() - if hasattr(right_val, "is_nan") - else math.isnan(right_val) - ) + left_nan = _is_nan(left_val) + right_nan = _is_nan(right_val) if left_nan or right_nan: return 0 if (left_nan and right_nan) else (-1 if left_nan else 1) diff --git a/packages/google-cloud-firestore/tests/system/test_system.py b/packages/google-cloud-firestore/tests/system/test_system.py index 1a71441a683c..5b1139cae804 100644 --- a/packages/google-cloud-firestore/tests/system/test_system.py +++ b/packages/google-cloud-firestore/tests/system/test_system.py @@ -1343,7 +1343,6 @@ def test_bson_query_ordering(client, cleanup, database): assert results == [BSONMinKey(), BSONInt32(10), BSONMaxKey()] - @pytest.fixture(scope="module") def query_docs(client, database): collection_id = "qs" + UNIQUE_RESOURCE_ID diff --git a/packages/google-cloud-firestore/tests/unit/v1/test_order.py b/packages/google-cloud-firestore/tests/unit/v1/test_order.py index 977efd6dd302..ff70eb441260 100644 --- a/packages/google-cloud-firestore/tests/unit/v1/test_order.py +++ b/packages/google-cloud-firestore/tests/unit/v1/test_order.py @@ -254,6 +254,16 @@ def test_order_bson_type_ordering(): # Test timestamp comparison (native timestamp < BSON timestamp with increment) assert target.compare(ts_native, ts_bson) == -1 + assert target.compare(ts_bson, ts_native) == 1 + + # Test BSON timestamp comparison + ts_bson2 = encode_value(BSONTimestamp(100, 2)) + ts_bson_later = encode_value(BSONTimestamp(101, 0)) + assert target.compare(ts_bson, ts_bson2) == -1 + assert target.compare(ts_bson2, ts_bson) == 1 + assert target.compare(ts_bson, ts_bson_later) == -1 + assert target.compare(ts_bson_later, ts_bson) == 1 + assert target.compare(ts_bson, ts_bson) == 0 # Test BSON binary > bytes assert target.compare(bytes_native, bin_b) == -1 @@ -277,7 +287,7 @@ def test_order_bson_type_ordering(): "__decimal128__": TypeOrder.NUMBER, "__binary__": TypeOrder.BSON_BINARY, "__regex__": TypeOrder.BSON_REGEX, - "__request_timestamp__": TypeOrder.TIMESTAMP, + "__request_timestamp__": TypeOrder.BSON_TIMESTAMP, } for key, expected_order in expected_orders.items(): assert _BSON_KEY_TO_TYPE_ORDER.get(key) == expected_order From d079139f202d4595a878162a7f1913844af15879 Mon Sep 17 00:00:00 2001 From: ohmayr Date: Wed, 16 Sep 2026 22:56:59 +0000 Subject: [PATCH 19/21] feat(firestore): add PyMongo duck-typing serialization support --- .../google/cloud/firestore_v1/_helpers.py | 26 ++++++++++++++++ .../tests/unit/v1/test__helpers.py | 30 +++++++++++++++++++ 2 files changed, 56 insertions(+) diff --git a/packages/google-cloud-firestore/google/cloud/firestore_v1/_helpers.py b/packages/google-cloud-firestore/google/cloud/firestore_v1/_helpers.py index 7334c788b993..c3191213a86c 100644 --- a/packages/google-cloud-firestore/google/cloud/firestore_v1/_helpers.py +++ b/packages/google-cloud-firestore/google/cloud/firestore_v1/_helpers.py @@ -214,6 +214,32 @@ def encode_value(value) -> types.document.Value: if isinstance(value, BSONType): return encode_value(value._to_map_value()) + # Duck-type native PyMongo / third-party BSON objects + if hasattr(value, "__class__"): + cls_name = value.__class__.__name__ + if cls_name == "ObjectId" and hasattr(value, "binary"): + return encode_value({"__oid__": str(value).lower()}) + if cls_name == "Decimal128" and hasattr(value, "to_decimal"): + return encode_value({"__decimal128__": str(value)}) + if cls_name == "Regex" and hasattr(value, "pattern"): + opts = getattr(value, "flags", "") or getattr(value, "options", "") + return encode_value( + {"__regex__": {"pattern": value.pattern, "options": str(opts)}} + ) + if cls_name == "Timestamp" and hasattr(value, "time") and hasattr(value, "inc"): + return encode_value( + { + "__request_timestamp__": { + "seconds": value.time, + "increment": value.inc, + } + } + ) + if cls_name == "MinKey": + return encode_value({"__min__": None}) + if cls_name == "MaxKey": + return encode_value({"__max__": None}) + if isinstance(value, GeoPoint): return document.Value(geo_point_value=value.to_protobuf()) diff --git a/packages/google-cloud-firestore/tests/unit/v1/test__helpers.py b/packages/google-cloud-firestore/tests/unit/v1/test__helpers.py index 88fe361eee31..1c755d5b4c6c 100644 --- a/packages/google-cloud-firestore/tests/unit/v1/test__helpers.py +++ b/packages/google-cloud-firestore/tests/unit/v1/test__helpers.py @@ -46,6 +46,36 @@ def test_geopoint_to_protobuf(): assert result == geo_pt_pb +def test_encode_value_pymongo_duck_typing(): + from google.cloud.firestore_v1._helpers import encode_value + + class ObjectId: + def __init__(self, val): + self.val = val + self.binary = b"12bytes_raw_" + + def __str__(self): + return self.val + + class Decimal128: + def __init__(self, val): + self.val = val + + def to_decimal(self): + return self.val + + def __str__(self): + return self.val + + oid_obj = ObjectId("507f191e810c19729de860ea") + oid_pb = encode_value(oid_obj) + assert oid_pb.map_value.fields["__oid__"].string_value == "507f191e810c19729de860ea" + + dec_obj = Decimal128("123.45") + dec_pb = encode_value(dec_obj) + assert dec_pb.map_value.fields["__decimal128__"].string_value == "123.45" + + def test_geopoint___eq__w_same_value(): lat = 0.015625 lng = 20.03125 From 0107a0ce0eee632b15bbe535b4964b69ac1f6efa Mon Sep 17 00:00:00 2001 From: ohmayr Date: Mon, 21 Sep 2026 20:00:24 +0000 Subject: [PATCH 20/21] feat(firestore): optimize PyMongo duck typing serialization and delegate to BSON types Move duck-typing checks in encode_value() to a fallback position before raising TypeError, preserving hot-path performance for standard types. Delegate conversion of duck-typed objects to Firestore BSON classes rather than manually creating wire dictionaries, and document supported BSON types in the encode_value docstring. --- .../google/cloud/firestore_v1/_helpers.py | 58 +++++++++---------- .../tests/unit/v1/test__helpers.py | 50 ++++++++++++++++ 2 files changed, 79 insertions(+), 29 deletions(-) diff --git a/packages/google-cloud-firestore/google/cloud/firestore_v1/_helpers.py b/packages/google-cloud-firestore/google/cloud/firestore_v1/_helpers.py index c3191213a86c..92ab9cef36a3 100644 --- a/packages/google-cloud-firestore/google/cloud/firestore_v1/_helpers.py +++ b/packages/google-cloud-firestore/google/cloud/firestore_v1/_helpers.py @@ -43,7 +43,7 @@ import google from google.cloud import exceptions # type: ignore -from google.cloud.firestore_v1 import transforms, types +from google.cloud.firestore_v1 import bson, transforms, types from google.cloud.firestore_v1.bson import BSONType from google.cloud.firestore_v1.field_path import FieldPath, parse_field_path from google.cloud.firestore_v1.types import common, document, write @@ -170,8 +170,10 @@ def encode_value(value) -> types.document.Value: Args: value (Union[NoneType, bool, int, float, datetime.datetime, \ str, bytes, dict, ~google.cloud.Firestore.GeoPoint, \ - ~google.cloud.firestore_v1.vector.Vector]): A native - Python value to convert to a protobuf field. + ~google.cloud.firestore_v1.vector.Vector, \ + ~google.cloud.firestore_v1.bson._BSONType]): A native \ + Python value or supported BSON / PyMongo-compatible value to \ + convert to a protobuf field. Returns: ~google.cloud.firestore_v1.types.Value: A @@ -214,32 +216,6 @@ def encode_value(value) -> types.document.Value: if isinstance(value, BSONType): return encode_value(value._to_map_value()) - # Duck-type native PyMongo / third-party BSON objects - if hasattr(value, "__class__"): - cls_name = value.__class__.__name__ - if cls_name == "ObjectId" and hasattr(value, "binary"): - return encode_value({"__oid__": str(value).lower()}) - if cls_name == "Decimal128" and hasattr(value, "to_decimal"): - return encode_value({"__decimal128__": str(value)}) - if cls_name == "Regex" and hasattr(value, "pattern"): - opts = getattr(value, "flags", "") or getattr(value, "options", "") - return encode_value( - {"__regex__": {"pattern": value.pattern, "options": str(opts)}} - ) - if cls_name == "Timestamp" and hasattr(value, "time") and hasattr(value, "inc"): - return encode_value( - { - "__request_timestamp__": { - "seconds": value.time, - "increment": value.inc, - } - } - ) - if cls_name == "MinKey": - return encode_value({"__min__": None}) - if cls_name == "MaxKey": - return encode_value({"__max__": None}) - if isinstance(value, GeoPoint): return document.Value(geo_point_value=value.to_protobuf()) @@ -256,11 +232,35 @@ def encode_value(value) -> types.document.Value: value_pb = document.MapValue(fields=value_dict) return document.Value(map_value=value_pb) + # Fallback: Coerce third-party BSON objects (e.g. PyMongo) to Firestore BSON types + bson_val = _try_duck_type_bson(value) + if bson_val is not None: + return encode_value(bson_val._to_map_value()) + raise TypeError( "Cannot convert to a Firestore Value", value, "Invalid type", type(value) ) +def _try_duck_type_bson(value) -> Optional[bson._BSONType]: + """Coerce third-party BSON objects (e.g. PyMongo) to Firestore BSON types.""" + cls_name = getattr(value.__class__, "__name__", "") + if cls_name == "ObjectId" and hasattr(value, "binary"): + return bson.BSONObjectId(str(value).lower()) + if cls_name == "Decimal128" and hasattr(value, "to_decimal"): + return bson.BSONDecimal128(value.to_decimal()) + if cls_name == "Regex" and hasattr(value, "pattern"): + opts = getattr(value, "flags", "") or getattr(value, "options", "") + return bson.BSONRegex(value.pattern, opts) + if cls_name == "Timestamp" and hasattr(value, "time") and hasattr(value, "inc"): + return bson.BSONTimestamp(value.time, value.inc) + if cls_name == "MinKey": + return bson.BSONMinKey() + if cls_name == "MaxKey": + return bson.BSONMaxKey() + return None + + def encode_dict(values_dict) -> dict: """Encode a dictionary into protobuf ``Value``-s. diff --git a/packages/google-cloud-firestore/tests/unit/v1/test__helpers.py b/packages/google-cloud-firestore/tests/unit/v1/test__helpers.py index 1c755d5b4c6c..012ea0b64829 100644 --- a/packages/google-cloud-firestore/tests/unit/v1/test__helpers.py +++ b/packages/google-cloud-firestore/tests/unit/v1/test__helpers.py @@ -67,6 +67,22 @@ def to_decimal(self): def __str__(self): return self.val + class Regex: + def __init__(self, pattern, flags="i"): + self.pattern = pattern + self.flags = flags + + class Timestamp: + def __init__(self, time, inc): + self.time = time + self.inc = inc + + class MinKey: + pass + + class MaxKey: + pass + oid_obj = ObjectId("507f191e810c19729de860ea") oid_pb = encode_value(oid_obj) assert oid_pb.map_value.fields["__oid__"].string_value == "507f191e810c19729de860ea" @@ -75,6 +91,40 @@ def __str__(self): dec_pb = encode_value(dec_obj) assert dec_pb.map_value.fields["__decimal128__"].string_value == "123.45" + regex_obj = Regex("^[a-z]+$", "i") + regex_pb = encode_value(regex_obj) + assert ( + regex_pb.map_value.fields["__regex__"].map_value.fields["pattern"].string_value + == "^[a-z]+$" + ) + assert ( + regex_pb.map_value.fields["__regex__"].map_value.fields["options"].string_value + == "i" + ) + + ts_obj = Timestamp(1700000000, 42) + ts_pb = encode_value(ts_obj) + assert ( + ts_pb.map_value.fields["__request_timestamp__"] + .map_value.fields["seconds"] + .integer_value + == 1700000000 + ) + assert ( + ts_pb.map_value.fields["__request_timestamp__"] + .map_value.fields["increment"] + .integer_value + == 42 + ) + + min_obj = MinKey() + min_pb = encode_value(min_obj) + assert "__min__" in min_pb.map_value.fields + + max_obj = MaxKey() + max_pb = encode_value(max_obj) + assert "__max__" in max_pb.map_value.fields + def test_geopoint___eq__w_same_value(): lat = 0.015625 From 977f9c00bcb58df9a5a474d779edd22f9a52e70e Mon Sep 17 00:00:00 2001 From: ohmayr Date: Wed, 23 Sep 2026 20:17:51 +0000 Subject: [PATCH 21/21] fix(firestore): preserve Binary subtype and support int flags in duck typing Support PyMongo Binary subclasses without losing subtype tags by inspecting subtype attribute in bytes check. Support integer regex bitmasks in duck-typed Regex by mapping standard re flags to BSON options characters. Document ValueError in encode_value. --- .../google/cloud/firestore_v1/_helpers.py | 48 ++++++++++++++++++- .../tests/unit/v1/test__helpers.py | 26 ++++++++++ 2 files changed, 72 insertions(+), 2 deletions(-) diff --git a/packages/google-cloud-firestore/google/cloud/firestore_v1/_helpers.py b/packages/google-cloud-firestore/google/cloud/firestore_v1/_helpers.py index 92ab9cef36a3..447a0acd4355 100644 --- a/packages/google-cloud-firestore/google/cloud/firestore_v1/_helpers.py +++ b/packages/google-cloud-firestore/google/cloud/firestore_v1/_helpers.py @@ -18,6 +18,7 @@ import datetime import json +import re from typing import ( TYPE_CHECKING, Any, @@ -181,6 +182,8 @@ def encode_value(value) -> types.document.Value: Raises: TypeError: If the ``value`` is not one of the accepted types. + ValueError: If a BSON or duck-typed BSON value has an invalid value + or representation (e.g. invalid ObjectId hex or binary subtype). """ if value is None: return document.Value(null_value=struct_pb2.NULL_VALUE) @@ -205,6 +208,9 @@ def encode_value(value) -> types.document.Value: return document.Value(string_value=value) if isinstance(value, bytes): + subtype = getattr(value, "subtype", None) + if subtype is not None: + return encode_value(bson.BSONBinary(value, subtype=subtype)._to_map_value()) return document.Value(bytes_value=value) # NOTE: We avoid doing an isinstance() check for a Document @@ -242,7 +248,40 @@ def encode_value(value) -> types.document.Value: ) -def _try_duck_type_bson(value) -> Optional[bson._BSONType]: +# Mapping of Python standard library regex flags to their canonical BSON regex +# option characters per the BSON specification (https://bsonspec.org/spec.html, type 0x0B). +# Stored in alphabetical order of option characters to produce normalized output. +_REGEX_FLAG_TO_BSON_CHAR: Tuple[Tuple[int, str], ...] = ( + (re.IGNORECASE, "i"), # Case-insensitive matching + (re.LOCALE, "l"), # Locale-dependent matching + (re.MULTILINE, "m"), # Multi-line matching + (re.DOTALL, "s"), # Dot matches all (including newline) + (re.UNICODE, "u"), # Unicode matching + (re.VERBOSE, "x"), # Verbose / whitespace-ignored matching +) + + +def _flags_to_options_string(flags: Any) -> str: + """Convert regex flags to a normalized BSON options string. + + Supports string options directly (e.g. ``"i"``), integer bitmasks from + the standard library ``re`` module (e.g. ``re.IGNORECASE | re.MULTILINE``), + or third-party driver types like PyMongo's ``Regex.flags``. + + Args: + flags (Any): A string of flag characters or an integer bitmask of regex flags. + + Returns: + str: The corresponding BSON regex options string. + """ + if isinstance(flags, str): + return flags + if isinstance(flags, int): + return "".join(char for flag, char in _REGEX_FLAG_TO_BSON_CHAR if flags & flag) + return str(flags) + + +def _try_duck_type_bson(value) -> Optional[BSONType]: """Coerce third-party BSON objects (e.g. PyMongo) to Firestore BSON types.""" cls_name = getattr(value.__class__, "__name__", "") if cls_name == "ObjectId" and hasattr(value, "binary"): @@ -250,7 +289,10 @@ def _try_duck_type_bson(value) -> Optional[bson._BSONType]: if cls_name == "Decimal128" and hasattr(value, "to_decimal"): return bson.BSONDecimal128(value.to_decimal()) if cls_name == "Regex" and hasattr(value, "pattern"): - opts = getattr(value, "flags", "") or getattr(value, "options", "") + raw_opts = getattr(value, "flags", None) + if raw_opts is None or raw_opts == "": + raw_opts = getattr(value, "options", "") + opts = _flags_to_options_string(raw_opts) return bson.BSONRegex(value.pattern, opts) if cls_name == "Timestamp" and hasattr(value, "time") and hasattr(value, "inc"): return bson.BSONTimestamp(value.time, value.inc) @@ -258,6 +300,8 @@ def _try_duck_type_bson(value) -> Optional[bson._BSONType]: return bson.BSONMinKey() if cls_name == "MaxKey": return bson.BSONMaxKey() + if cls_name == "Binary" and hasattr(value, "subtype"): + return bson.BSONBinary(value, subtype=value.subtype) return None diff --git a/packages/google-cloud-firestore/tests/unit/v1/test__helpers.py b/packages/google-cloud-firestore/tests/unit/v1/test__helpers.py index 012ea0b64829..fb2ac42a0b5d 100644 --- a/packages/google-cloud-firestore/tests/unit/v1/test__helpers.py +++ b/packages/google-cloud-firestore/tests/unit/v1/test__helpers.py @@ -83,6 +83,12 @@ class MinKey: class MaxKey: pass + class Binary(bytes): + def __new__(cls, data, subtype=0): + obj = super().__new__(cls, data) + obj.subtype = subtype + return obj + oid_obj = ObjectId("507f191e810c19729de860ea") oid_pb = encode_value(oid_obj) assert oid_pb.map_value.fields["__oid__"].string_value == "507f191e810c19729de860ea" @@ -102,6 +108,21 @@ class MaxKey: == "i" ) + import re + + regex_int_flags = Regex("^[a-z]+$", re.IGNORECASE | re.MULTILINE) + regex_int_pb = encode_value(regex_int_flags) + assert ( + regex_int_pb.map_value.fields["__regex__"] + .map_value.fields["options"] + .string_value + == "im" + ) + + bin_obj = Binary(b"\x01\x02\x03", subtype=128) + bin_pb = encode_value(bin_obj) + assert bin_pb.map_value.fields["__binary__"].bytes_value == b"\x80\x01\x02\x03" + ts_obj = Timestamp(1700000000, 42) ts_pb = encode_value(ts_obj) assert ( @@ -125,6 +146,11 @@ class MaxKey: max_pb = encode_value(max_obj) assert "__max__" in max_pb.map_value.fields + import pytest + + with pytest.raises(ValueError): + encode_value(ObjectId("invalid_hex")) + def test_geopoint___eq__w_same_value(): lat = 0.015625