Merge pull request #885 from sirosen/introduce-missingtype

Introduce the MISSING sentinel and use it for HA timeout on GroupPolicies

changelog.d/20231025_122826_sirosen_introduce_missingtype.rst

@@ -0,0 +1,9 @@
+Added
+~~~~~
+
+- A new sentinel value, ``globus_sdk.MISSING``, has been introduced.
+  It is used for method calls which need to distinguish missing parameters from
+  an explicit ``None`` used to signify ``null`` (:pr:`NUMBER`)
+
+  - ``globus_sdk.MISSING`` is now supported in payload data for all methods, and
+    will be automatically removed from the payload before sending to the server

changelog.d/20231025_125551_sirosen_introduce_missingtype.rst

@@ -0,0 +1,5 @@
+Changed
+~~~~~~~
+
+- ``GroupPolicies`` objects now treat an explicit instantiation with
+  ``high_assurance_timeout=None`` as setting the timeout to ``null`` (:pr:`NUMBER`)

src/globus_sdk/__init__.py

@@ -126,6 +126,10 @@ def _force_eager_imports() -> None:
         "TransferClient",
         "TransferData",
     },
+    "utils": {
+        "MISSING",
+        "MissingType",
+    },
 }
 
 if t.TYPE_CHECKING:
@@ -211,6 +215,8 @@ def _force_eager_imports() -> None:
     from .services.transfer import TransferAPIError
     from .services.transfer import TransferClient
     from .services.transfer import TransferData
+    from .utils import MISSING
+    from .utils import MissingType
 
 
 def __dir__() -> t.List[str]:
@@ -297,7 +303,9 @@ def __getattr__(name: str) -> t.Any:
     "IterableTransferResponse",
     "LocalGlobusConnectPersonal",
     "LocalGlobusConnectServer",
+    "MISSING",
     "MappedCollectionDocument",
+    "MissingType",
     "NativeAppAuthClient",
     "NetworkError",
     "NullAuthorizer",

src/globus_sdk/_generate_init.py

@@ -200,6 +200,13 @@ def __getattr__(name: str) -> t.Any:
             "TransferData",
         ),
     ),
+    (
+        "utils",
+        (
+            "MISSING",
+            "MissingType",
+        ),
+    ),
 ]
 
 

src/globus_sdk/client.py

@@ -303,9 +303,9 @@ def request(
         r = self.transport.request(
             method=method,
             url=url,
-            data=data.data if isinstance(data, utils.PayloadWrapper) else data,
-            query_params=query_params,
-            headers=rheaders,
+            data=utils.PayloadWrapper._prepare(data),
+            query_params=utils.filter_missing(query_params),
+            headers=utils.filter_missing(rheaders),
             encoding=encoding,
             authorizer=self.authorizer,
             allow_redirects=allow_redirects,

src/globus_sdk/services/groups/data.py

@@ -263,7 +263,9 @@ def __init__(
         group_members_visibility: _GROUP_MEMBER_VISIBILITY_T,
         join_requests: bool,
         signup_fields: t.Iterable[_GROUP_REQUIRED_SIGNUP_FIELDS_T],
-        authentication_assurance_timeout: int | None = None,
+        authentication_assurance_timeout: (
+            int | None | utils.MissingType
+        ) = utils.MISSING,
     ):
         super().__init__()
         self["is_high_assurance"] = is_high_assurance
@@ -273,5 +275,4 @@ def __init__(
         )
         self["join_requests"] = join_requests
         self["signup_fields"] = utils.render_enums_for_api(signup_fields)
-        if authentication_assurance_timeout is not None:
-            self["authentication_assurance_timeout"] = authentication_assurance_timeout
+        self["authentication_assurance_timeout"] = authentication_assurance_timeout

src/globus_sdk/utils.py

@@ -22,6 +22,47 @@
     PayloadWrapperBase = collections.UserDict
 
 
+class MissingType:
+    def __init__(self) -> None:
+        # disable instantiation, but gated to be able to run once
+        # when this module is imported
+        if "MISSING" in globals():
+            raise TypeError("MissingType should not be instantiated")
+
+    def __bool__(self) -> bool:
+        return False
+
+    def __copy__(self) -> MissingType:
+        return self
+
+    def __deepcopy__(self, memo: dict[int, t.Any]) -> MissingType:
+        return self
+
+    # unpickling a MissingType should always return the "MISSING" sentinel
+    def __reduce__(self) -> str:
+        return "MISSING"
+
+    def __repr__(self) -> str:
+        return "<globus_sdk.MISSING>"
+
+
+# a sentinel value for "missing" values which are distinguished from `None` (null)
+# this is the default used to indicate that a parameter was not passed, so that
+# method calls passing `None` can be distinguished from those which did not pass any
+# value
+# users should typically not use this value directly, but it is part of the public SDK
+# interfaces along with its type for annotation purposes
+#
+# *new after v3.29.0*
+MISSING = MissingType()
+
+
+def filter_missing(data: dict[str, t.Any] | None) -> dict[str, t.Any] | None:
+    if data is None:
+        return None
+    return {k: v for k, v in data.items() if v is not MISSING}
+
+
 def sha256_string(s: str) -> str:
     return hashlib.sha256(s.encode("utf-8")).hexdigest()
 
@@ -154,6 +195,38 @@ def _set_optints(self, **kwargs: t.Any) -> None:
         for k, v in kwargs.items():
             self._set_value(k, v, callback=int)
 
+    @classmethod
+    def _prepare(
+        cls, data: str | None | dict[str, t.Any] | PayloadWrapper
+    ) -> str | None | dict[str, t.Any]:
+        """
+        Prepare a payload for serialization.
+
+        It may already be of one of the acceptable input types (str, None, dict),
+        but if it is a dict it will be recursively converted to
+
+        - remove instances of `MISSING`
+        - convert any `PayloadWrapper` instances to dicts
+        """
+        if data is None:
+            return None
+        if isinstance(data, str):
+            return data
+        return t.cast("dict[str, t.Any]", _recursively_prepare_payload(data))
+
+
+def _recursively_prepare_payload(data: t.Any) -> t.Any:
+    if isinstance(data, (dict, PayloadWrapper)):
+        return {
+            k: _recursively_prepare_payload(v)
+            for k, v in data.items()
+            if v is not MISSING
+        }
+    elif isinstance(data, (list, tuple)):
+        return [_recursively_prepare_payload(x) for x in data if x is not MISSING]
+    else:
+        return data
+
 
 def in_sphinx_build() -> bool:  # pragma: no cover
     # check if `sphinx-build` was used to invoke

tests/functional/base_client/test_filter_missing.py

@@ -0,0 +1,57 @@
+import json
+import urllib.parse
+
+import pytest
+
+from globus_sdk import utils
+from globus_sdk._testing import RegisteredResponse, get_last_request, load_response
+
+
+@pytest.fixture(autouse=True)
+def setup_mock_responses():
+    load_response(
+        RegisteredResponse(
+            path="https://foo.api.globus.org/bar",
+            json={"foo": "bar"},
+        )
+    )
+    load_response(
+        RegisteredResponse(
+            path="https://foo.api.globus.org/bar",
+            method="POST",
+            json={"foo": "bar"},
+        )
+    )
+
+
+def test_query_params_can_filter_missing(client):
+    res = client.get("/bar", query_params={"foo": "bar", "baz": utils.MISSING})
+    assert res.http_status == 200
+    req = get_last_request()
+    assert req.params == {"foo": "bar"}
+
+
+def test_headers_can_filter_missing(client):
+    res = client.get("/bar", headers={"foo": "bar", "baz": utils.MISSING})
+    assert res.http_status == 200
+    req = get_last_request()
+    assert req.headers["foo"] == "bar"
+    assert "baz" not in req.headers
+
+
+def test_json_body_can_filter_missing(client):
+    res = client.post("/bar", data={"foo": "bar", "baz": utils.MISSING})
+    assert res.http_status == 200
+    req = get_last_request()
+    sent = json.loads(req.body)
+    assert sent == {"foo": "bar"}
+
+
+def test_form_body_can_filter_missing(client):
+    res = client.post(
+        "/bar", data={"foo": "bar", "baz": utils.MISSING}, encoding="form"
+    )
+    assert res.http_status == 200
+    req = get_last_request()
+    sent = urllib.parse.parse_qs(req.body)
+    assert sent == {"foo": ["bar"]}

tests/functional/services/groups/test_set_group_policies.py

@@ -7,6 +7,7 @@
     GroupPolicies,
     GroupRequiredSignupFields,
     GroupVisibility,
+    utils,
 )
 from globus_sdk._testing import get_last_request, load_response
 
@@ -72,27 +73,30 @@ def test_set_group_policies(
 
 
 @pytest.mark.parametrize(
-    "group_vis, group_member_vis, signup_fields, signup_fields_str",
+    "group_vis, group_member_vis, signup_fields, signup_fields_str, auth_timeout",
     (
         (
             GroupVisibility.private,
             GroupMemberVisibility.members,
             [GroupRequiredSignupFields.address1],
             ["address1"],
+            28800,
         ),
         (
             GroupVisibility.authenticated,
             GroupMemberVisibility.managers,
             ["address1"],
             ["address1"],
+            utils.MISSING,
         ),
         (
             "private",
             "members",
             [GroupRequiredSignupFields.address1, "address2"],
             ["address1", "address2"],
+            0,
         ),
-        ("authenticated", "managers", ["address1"], ["address1"]),
+        ("authenticated", "managers", ["address1"], ["address1"], None),
     ),
 )
 @pytest.mark.parametrize("setter_usage", (False, "enum", "str"))
@@ -102,6 +106,7 @@ def test_set_group_policies_explicit_payload(
     group_member_vis,
     signup_fields,
     signup_fields_str,
+    auth_timeout,
     setter_usage,
 ):
     group_vis_str = group_vis if isinstance(group_vis, str) else group_vis.value
@@ -120,7 +125,7 @@ def test_set_group_policies_explicit_payload(
         group_members_visibility=group_member_vis,
         join_requests=False,
         signup_fields=signup_fields,
-        authentication_assurance_timeout=28800,
+        authentication_assurance_timeout=auth_timeout,
     )
     if setter_usage:
         # set a string in the payload directly
@@ -139,3 +144,10 @@ def test_set_group_policies_explicit_payload(
     assert req_body["group_visibility"] == group_vis_str
     assert req_body["group_members_visibility"] == group_member_vis_str
     assert req_body["signup_fields"] == signup_fields_str
+
+    # check the authentication_assurance_timeout
+    # it should be omitted if it's MISSING
+    if auth_timeout is utils.MISSING:
+        assert "authentication_assurance_timeout" not in req_body
+    else:
+        assert req_body["authentication_assurance_timeout"] == auth_timeout

tests/unit/helpers/test_payload_wrapper.py

@@ -0,0 +1,53 @@
+import pytest
+
+from globus_sdk import utils
+
+
+def test_payload_preparation_strips_missing_dict():
+    original = {"foo": None, "bar": utils.MISSING}
+    prepared = utils.PayloadWrapper._prepare(original)
+    assert prepared == {"foo": None}
+
+
+# this is a weird case (not really recommended usage), but we have well defined behavior
+# for it, so exercise it here
+@pytest.mark.parametrize("type_", (list, tuple))
+def test_payload_preparation_strips_missing_list_or_tuple(type_):
+    original = type_([None, 1, utils.MISSING, 0])
+    prepared = utils.PayloadWrapper._prepare(original)
+    assert prepared == [None, 1, 0]
+
+
+@pytest.mark.parametrize("original", (None, 1, 0, True, False, "foo", object()))
+def test_payload_preparation_retains_simple_datatype_identity(original):
+    prepared = utils.PayloadWrapper._prepare(original)
+    # check not only that the values are equal, but that they pass the identity test
+    assert prepared is original
+
+
+# this test makes sense in the context of the identity test above:
+# check that the values are equal, although the type may be reconstructed
+@pytest.mark.parametrize("original", (["foo", "bar"], {"foo": "bar"}))
+def test_payload_preparation_retains_complex_datatype_equality(original):
+    prepared = utils.PayloadWrapper._prepare(original)
+    assert prepared == original
+
+
+def test_payload_preparation_dictifies_wrappers():
+    x = utils.PayloadWrapper()
+    x["foo"] = 1
+    prepared = utils.PayloadWrapper._prepare(x)
+    assert prepared == {"foo": 1}
+    assert isinstance(prepared, dict)
+    assert prepared is not x
+    assert not isinstance(prepared, utils.PayloadWrapper)
+
+
+def test_payload_preparation_recursively_dictifies_wrappers():
+    x = utils.PayloadWrapper()
+    x["foo"] = 1
+    y = utils.PayloadWrapper()
+    y["bar"] = x
+    y["baz"] = [2, x]
+    prepared = utils.PayloadWrapper._prepare(y)
+    assert prepared == {"bar": {"foo": 1}, "baz": [2, {"foo": 1}]}

tests/unit/test_missing_type.py

@@ -0,0 +1,28 @@
+import copy
+import pickle
+
+import pytest
+
+from globus_sdk import utils
+
+
+def test_missing_type_cannot_be_instantiated():
+    with pytest.raises(TypeError, match="MissingType should not be instantiated"):
+        utils.MissingType()
+
+
+def test_missing_sentinel_bools_as_false():
+    assert bool(utils.MISSING) is False
+
+
+def test_str_of_missing():
+    assert str(utils.MISSING) == "<globus_sdk.MISSING>"
+
+
+def test_copy_of_missing_is_self():
+    assert copy.copy(utils.MISSING) is utils.MISSING
+    assert copy.deepcopy(utils.MISSING) is utils.MISSING
+
+
+def test_pickle_of_missing_is_self():
+    assert pickle.loads(pickle.dumps(utils.MISSING)) is utils.MISSING