-
Notifications
You must be signed in to change notification settings - Fork 1.8k
feat(firestore): add BSON read deserialization support #18402
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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,27 +347,18 @@ 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: | ||
| value (google.cloud.firestore_v1.types.Value): A | ||
| 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. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we have this opted in by default? It looks like other languages decode to BSON by default. And we do similar decoding to custom objects for GeoPoint, Vector, etc. Making this opt-in would really lower the value of for the feature BSON is a new feature, so we wouldn't expect workarounds in existing code. But let me know if you have any specific breaking change concerns, and maybe we can find solutions |
||
|
|
||
| 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``. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why did you drop the Raises section? |
||
| """ | ||
| value_pb = getattr(value, "_pb", value) | ||
| value_type = value_pb.WhichOneof("value_type") | ||
|
|
@@ -394,37 +385,81 @@ 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) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This seems like it should be implemented as a class method:
or
|
||
| 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, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why does this need 3 states? Can't it just be a bool that defaults to False? |
||
| ) -> Union[dict, Vector, _BSONType]: | ||
| """Converts a protobuf map of Firestore ``Value``-s. | ||
|
|
||
| Args: | ||
| value_fields (google.protobuf.pyext._message.MessageMapContainer): A | ||
| 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. | ||
| # {"__type__":"__vector__", "value": [1.0, 2.0, 3.0]}. | ||
| 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 | ||
|
|
||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we do this without changing this to Any? Can't we just add BSONType to the output list?
This would remove a lot of the value of the type annotations