refactor(firestore): streamline BSON classes with strict target constructors in PR 1A

Author: ohmayr

packages/google-cloud-firestore/google/cloud/firestore_v1/bson.py

@@ -38,23 +38,17 @@ class BSONObjectID:
 
     __slots__ = ("_value",)
 
-    def __init__(self, value: Any):
-        if hasattr(value, "binary"):
-            value = getattr(value, "binary")
-        elif hasattr(value, "value"):
-            value = getattr(value, "value")
-
+    def __init__(self, value: Union[str, bytes, bytearray]):
         if isinstance(value, str):
             if not _HEX_24_REGEX.match(value):
                 raise ValueError("BSONObjectID string must be a 24-character hex string.")
             self._value: str = value.lower()
-        elif isinstance(value, (bytes, bytearray, memoryview)):
-            raw_bytes = bytes(value)
-            if len(raw_bytes) != 12:
+        elif isinstance(value, (bytes, bytearray)):
+            if len(value) != 12:
                 raise ValueError("BSONObjectID bytes input must be 12 raw bytes.")
-            self._value = raw_bytes.hex()
+            self._value: str = bytes(value).hex()
         else:
-            raise TypeError("BSONObjectID requires str, bytes, bytearray, memoryview, or BSONObjectID instance.")
+            raise TypeError("BSONObjectID requires str or bytes.")
 
     @property
     def value(self) -> str:
@@ -81,11 +75,9 @@ class BSONDecimal128:
 
     __slots__ = ("_value",)
 
-    def __init__(self, value: Any):
+    def __init__(self, value: Union[str, decimal.Decimal]):
         if isinstance(value, bool):
             raise TypeError("BSONDecimal128 value cannot be bool.")
-        if hasattr(value, "to_decimal") and callable(getattr(value, "to_decimal")):
-            value = getattr(value, "to_decimal")()
 
         if isinstance(value, decimal.Decimal):
             self._value: str = str(value)
@@ -96,13 +88,14 @@ def __init__(self, value: Any):
                 raise ValueError(f"Invalid Decimal128 string format: {value!r}") from exc
             self._value = value
         else:
-            raise TypeError("BSONDecimal128 requires str, decimal.Decimal, or Decimal128-like object.")
+            raise TypeError("BSONDecimal128 requires str or decimal.Decimal.")
 
     @property
     def value(self) -> str:
         return self._value
 
     def to_decimal(self) -> decimal.Decimal:
+        """Converts to a native Python decimal.Decimal object."""
         return decimal.Decimal(self._value)
 
     def to_map_value(self) -> Dict[str, str]:
@@ -140,7 +133,12 @@ class BSONTimestamp:
     __slots__ = ("_seconds", "_increment")
 
     def __init__(self, seconds: int, increment: int):
-        if type(seconds) is not int or type(increment) is not int:
+        if (
+            type(seconds) is not int
+            or type(increment) is not int
+            or isinstance(seconds, bool)
+            or isinstance(increment, bool)
+        ):
             raise TypeError("seconds and increment must be ints.")
         if not (0 <= seconds <= 4294967295) or not (0 <= increment <= 4294967295):
             raise ValueError("seconds and increment must be uint32 (0 to 4294967295).")
@@ -176,11 +174,7 @@ class BSONRegex:
 
     __slots__ = ("_pattern", "_options")
 
-    def __init__(self, pattern: Any, options: str = ""):
-        if hasattr(pattern, "pattern") and not isinstance(pattern, str):
-            flags = getattr(pattern, "flags", "")
-            options = flags if isinstance(flags, str) else options
-            pattern = getattr(pattern, "pattern")
+    def __init__(self, pattern: str, options: str = ""):
         if not isinstance(pattern, str) or not isinstance(options, str):
             raise TypeError("pattern and options must be strings.")
         self._pattern: str = pattern
@@ -215,15 +209,14 @@ class BSONBinary:
 
     __slots__ = ("_subtype", "_data")
 
-    def __init__(self, data: Union[bytes, bytearray, memoryview], subtype: int = 0):
+    def __init__(self, data: Union[bytes, bytearray], subtype: int = 0):
         if isinstance(subtype, bool) or type(subtype) is not int:
             raise TypeError("subtype must be an integer.")
         if not (0 <= subtype <= 255):
             raise ValueError("subtype must be in range 0..255.")
-        try:
-            self._data: bytes = bytes(data)
-        except TypeError as exc:
-            raise TypeError("data must be bytes-like.") from exc
+        if not isinstance(data, (bytes, bytearray)):
+            raise TypeError("data must be bytes or bytearray.")
+        self._data: bytes = bytes(data)
         self._subtype: int = subtype
 
     @property
@@ -255,17 +248,12 @@ class BSONInt32:
 
     __slots__ = ("_value",)
 
-    def __init__(self, value: Any):
-        if isinstance(value, bool):
-            raise TypeError("BSONInt32 value cannot be bool.")
-        if hasattr(value, "value") and type(getattr(value, "value")) is int:
-            value = getattr(value, "value")
-        if type(value) is not int and not isinstance(value, (int, BSONInt32)):
+    def __init__(self, value: int):
+        if isinstance(value, bool) or type(value) is not int:
             raise TypeError("BSONInt32 value must be an integer.")
-        int_val = int(value)
-        if not (-2147483648 <= int_val <= 2147483647):
+        if not (-2147483648 <= value <= 2147483647):
             raise ValueError("BSONInt32 out of range [-2147483648, 2147483647].")
-        self._value: int = int_val
+        self._value: int = value
 
     @property
     def value(self) -> int:

packages/google-cloud-firestore/tests/system/test_bson.py

@@ -14,7 +14,6 @@
 # limitations under the License.
 
 import pytest
-from google.cloud import firestore
 from google.cloud.firestore import (
     ArrayUnion,
     BSONBinary,

packages/google-cloud-firestore/tests/unit/v1/test_bson.py

@@ -49,39 +49,11 @@ def test_bson_object_id_constructors_and_properties():
         BSONObjectID(b"invalid_bytes_len")
 
 
-def test_bson_constructors_duck_typed_objects():
-    class DummyPyMongoObjectId:
-        def __init__(self, raw: bytes):
-            self.binary = raw
-
-    class DummyPyMongoDecimal128:
-        def __init__(self, d: decimal.Decimal):
-            self._d = d
-        def to_decimal(self):
-            return self._d
-
-    class DummyPyMongoRegex:
-        def __init__(self, pat: str, flags: str):
-            self.pattern = pat
-            self.flags = flags
-
-    dummy_oid = DummyPyMongoObjectId(bytes.fromhex("507f1f77bcf86cd799439011"))
-    oid = BSONObjectID(dummy_oid)
-    assert oid.value == "507f1f77bcf86cd799439011"
-
-    dummy_dec = DummyPyMongoDecimal128(decimal.Decimal("123.45"))
-    dec = BSONDecimal128(dummy_dec)
-    assert dec.value == "123.45"
-
-    dummy_reg = DummyPyMongoRegex("^[a-z]+$", "i")
-    reg = BSONRegex(dummy_reg)
-    assert reg.pattern == "^[a-z]+$"
-    assert reg.options == "i"
-
-    # Copy constructor
-    target_oid = BSONObjectID("507f191e810c19729de860ea")
-    oid4 = BSONObjectID(target_oid)
-    assert oid4.value == "507f191e810c19729de860ea"
+def test_bson_constructors_validation():
+    # bytearray
+    raw_bytes = bytes.fromhex("507f1f77bcf86cd799439011")
+    oid_ba = BSONObjectID(bytearray(raw_bytes))
+    assert oid_ba.value == "507f1f77bcf86cd799439011"
 
     # Invalid constructors
     with pytest.raises(ValueError):