Non experimental scope parsing sc 30765 (#966)

* Migrate existing scope tests to new path

* Non-experimental Scope Parsing

* Update changelog.d/20240322_122646_derek_non_experimental_scope_parsing_sc_30765.rst

Co-authored-by: Stephen Rosen <sirosen@globus.org>

---------

Co-authored-by: Stephen Rosen <sirosen@globus.org>

changelog.d/20240322_122646_derek_non_experimental_scope_parsing_sc_30765.rst

@@ -0,0 +1,6 @@
+
+Added
+~~~~~
+
+- Moved scope parsing out of experimental. The ``Scope`` construct is now importable from
+  the top level `globus_sdk` module. (:pr:`NUMBER`)

src/globus_sdk/__init__.py

@@ -133,6 +133,11 @@ def _force_eager_imports() -> None:
         "TransferClient",
         "TransferData",
     },
+    "scopes": {
+        "Scope",
+        "ScopeParseError",
+        "ScopeCycleError",
+    },
     "utils": {
         "MISSING",
         "MissingType",
@@ -229,6 +234,9 @@ def _force_eager_imports() -> None:
     from .services.transfer import TransferAPIError
     from .services.transfer import TransferClient
     from .services.transfer import TransferData
+    from .scopes import Scope
+    from .scopes import ScopeParseError
+    from .scopes import ScopeCycleError
     from .utils import MISSING
     from .utils import MissingType
 
@@ -338,6 +346,9 @@ def __getattr__(name: str) -> t.Any:
     "RefreshTokenAuthorizer",
     "RemovedInV4Warning",
     "S3StoragePolicies",
+    "Scope",
+    "ScopeCycleError",
+    "ScopeParseError",
     "SearchAPIError",
     "SearchClient",
     "SearchQuery",

src/globus_sdk/_generate_init.py

@@ -215,6 +215,14 @@ def __getattr__(name: str) -> t.Any:
             "TransferData",
         ),
     ),
+    (
+        "scopes",
+        (
+            "Scope",
+            "ScopeParseError",
+            "ScopeCycleError",
+        ),
+    ),
     (
         "utils",
         (

src/globus_sdk/experimental/scope_parser.py

@@ -0,0 +1,13 @@
+"""
+Scope parsing has been moved out of experimental into the main SDK.
+This module will be removed in a future release but is maintained in the interim for
+  backwards compatibility.
+"""
+
+from globus_sdk.scopes import Scope, ScopeCycleError, ScopeParseError
+
+__all__ = (
+    "Scope",
+    "ScopeParseError",
+    "ScopeCycleError",
+)

src/globus_sdk/scopes/__init__.py

@@ -10,11 +10,16 @@
     TimerScopes,
     TransferScopes,
 )
+from .errors import ScopeCycleError, ScopeParseError
+from .representation import Scope
 from .scope_definition import MutableScope
 
 __all__ = (
     "ScopeBuilder",
     "MutableScope",
+    "Scope",
+    "ScopeParseError",
+    "ScopeCycleError",
     "GCSCollectionScopeBuilder",
     "GCSEndpointScopeBuilder",
     "AuthScopes",

src/globus_sdk/experimental/scope_parser/scope_definition.py → src/globus_sdk/scopes/representation.py

@@ -1,6 +1,5 @@
 from __future__ import annotations
 
-import typing as t
 import warnings
 
 from ._parser import parse_scope_graph
@@ -143,81 +142,3 @@ def __repr__(self) -> str:
 
     def __str__(self) -> str:
         return self.serialize()
-
-    def __contains__(self, other: t.Any) -> bool:
-        """
-        .. warning::
-
-            The ``__contains__`` method is a non-authoritative convenience for comparing
-            parsed scopes. Although the essence and intent of the check is summarized
-            below, there is no guarantee that it correctly reflects the permissions of a
-            token or tokens. The structure of the data for a given consent in Globus
-            Auth is not perfectly reflected in the parse tree.
-
-        ``in`` and ``not in`` are defined as permission coverage checks
-
-        ``scope1 in scope2`` means that a token scoped for
-        ``scope2`` has all of the permissions of a token scoped for ``scope1``.
-
-        A scope is covered by another scope if
-
-        - the top level strings match
-        - the optional-ness matches OR only the covered scope is optional
-        - the dependencies of the covered scope are all covered by dependencies of
-          the covering scope
-
-        Therefore, the following are true:
-
-        .. code-block:: pycon
-
-            >>> s = lambda x: Scope.deserialize(x)  # define this for brevity below
-            # self inclusion works, including when optional
-            >>> s("foo") in s("foo")
-            >>> s("*foo") in s("*foo")
-            # an optional scope is covered by a non-optional one, but not the reverse
-            >>> s("*foo") in s("foo")
-            >>> s("foo") not in s("*foo")
-            # dependencies have the expected meanings
-            >>> s("foo") in s("foo[bar]")
-            >>> s("foo[bar]") not in s("foo")
-            >>> s("foo[bar]") in s("foo[bar[baz]]")
-            # dependencies are not transitive and obey "optionalness" matching
-            >>> s("foo[bar]") not in s("foo[fizz[bar]]")
-            >>> s("foo[bar]") not in s("foo[*bar]")
-        """
-        # scopes can only contain other scopes
-        if not isinstance(other, Scope):
-            return False
-
-        # top-level scope must match
-        if self.scope_string != other.scope_string:
-            return False
-
-        # between self.optional and other.optional, there are four possibilities,
-        # of which three are acceptable and one is not
-        # both optional and neither optional are okay,
-        # 'self' being non-optional and 'other' being optional is okay
-        # the failing case is 'other in self' when 'self' is optional and 'other' is not
-        #
-        #    self.optional | other.optional | (other in self) is possible
-        #    --------------|----------------|----------------------------
-        #        true      |    true        |    true
-        #        false     |    false       |    true
-        #        false     |    true        |    true
-        #        true      |    false       |    false
-        #
-        # so only check for that one case
-        if self.optional and not other.optional:
-            return False
-
-        # dependencies must all be contained -- search for a contrary example
-        for other_dep in other.dependencies:
-            for dep in self.dependencies:
-                if other_dep in dep:
-                    break
-            # reminder: the else branch of a for-else means that the break was never hit
-            else:
-                return False
-
-        # all criteria were met -- True!
-        return True

src/globus_sdk/scopes/scope_definition.py

@@ -1,7 +1,10 @@
 """
-This defines the Scope object and the scope parser.
-Because these components are mutually dependent, it's easiest if they're kept in a
-single module.
+THIS IS A LEGACY MODULE
+
+This module defines a legacy scope object and parser called `MutableScope`.
+It is maintained for backwards compatibility.
+
+For new code, use the `globus_sdk.Scope` object.
 """
 
 from __future__ import annotations
@@ -23,7 +26,6 @@ def _iter_scope_collection(obj: ScopeCollectionType) -> t.Iterator[str]:
             yield str(item)
 
 
-# TODO: rename MutableScope to Scope
 class MutableScope:
     """
     A scope object is a representation of a scope which allows modifications to be

tests/unit/experimental/test_legacy_support.py

@@ -0,0 +1,18 @@
+"""
+Constructs which are added to `experimental` ultimately (hopefully) get ported over to
+  the main `globus_sdk` namespace.
+
+The tests in this module verify that those constructs are still available from the
+  `globus_sdk.experimental` namespace (for backwards compatibility).
+
+Eventually these constructs do get deprecated at which point the tests in this module
+  can be deleted.
+"""
+
+
+def test_scope_importable_from_experimental():
+    from globus_sdk.experimental.scope_parser import (  # noqa: F401
+        Scope,
+        ScopeCycleError,
+        ScopeParseError,
+    )

tests/unit/scopes/test_mutable_scope.py

@@ -0,0 +1,48 @@
+import pytest
+
+from globus_sdk.scopes import MutableScope
+
+
+def test_scope_str_and_repr_simple():
+    s = MutableScope("simple")
+    assert str(s) == "simple"
+    assert repr(s) == "MutableScope('simple')"
+
+
+def test_scope_str_and_repr_optional():
+    s = MutableScope("simple", optional=True)
+    assert str(s) == "*simple"
+    assert repr(s) == "MutableScope('simple', optional=True)"
+
+
+def test_scope_str_and_repr_with_dependencies():
+    s = MutableScope("top")
+    s.add_dependency("foo")
+    assert str(s) == "top[foo]"
+    s.add_dependency("bar")
+    assert str(s) == "top[foo bar]"
+    assert (
+        repr(s) == "MutableScope('top', "
+        "dependencies=[MutableScope('foo'), MutableScope('bar')])"
+    )
+
+
+def test_add_dependency_warns_on_optional_but_still_has_good_str_and_repr():
+    s = MutableScope("top")
+    # this should warn, the use of `optional=...` rather than adding a Scope object
+    # when optional dependencies are wanted is deprecated
+    with pytest.warns(DeprecationWarning):
+        s.add_dependency("foo", optional=True)
+
+    # confirm the str representation and repr for good measure
+    assert str(s) == "top[*foo]"
+    assert (
+        repr(s)
+        == "MutableScope('top', dependencies=[MutableScope('foo', optional=True)])"
+    )
+
+
+@pytest.mark.parametrize("scope_str", ("*foo", "foo[bar]", "foo[", "foo]", "foo bar"))
+def test_scope_init_forbids_special_chars(scope_str):
+    with pytest.raises(ValueError):
+        MutableScope(scope_str)