diff --git a/changelog.d/20240322_122646_derek_non_experimental_scope_parsing_sc_30765.rst b/changelog.d/20240322_122646_derek_non_experimental_scope_parsing_sc_30765.rst new file mode 100644 index 000000000..e254b321f --- /dev/null +++ b/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`) diff --git a/src/globus_sdk/__init__.py b/src/globus_sdk/__init__.py index 63b52e004..f91ad60f4 100644 --- a/src/globus_sdk/__init__.py +++ b/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", diff --git a/src/globus_sdk/_generate_init.py b/src/globus_sdk/_generate_init.py index 7053544a3..e33c97cf9 100755 --- a/src/globus_sdk/_generate_init.py +++ b/src/globus_sdk/_generate_init.py @@ -215,6 +215,14 @@ def __getattr__(name: str) -> t.Any: "TransferData", ), ), + ( + "scopes", + ( + "Scope", + "ScopeParseError", + "ScopeCycleError", + ), + ), ( "utils", ( diff --git a/src/globus_sdk/experimental/scope_parser.py b/src/globus_sdk/experimental/scope_parser.py new file mode 100644 index 000000000..8bfdd1402 --- /dev/null +++ b/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", +) diff --git a/src/globus_sdk/experimental/scope_parser/__init__.py b/src/globus_sdk/experimental/scope_parser/__init__.py deleted file mode 100644 index b9fd8ca6f..000000000 --- a/src/globus_sdk/experimental/scope_parser/__init__.py +++ /dev/null @@ -1,8 +0,0 @@ -from .errors import ScopeCycleError, ScopeParseError -from .scope_definition import Scope - -__all__ = ( - "Scope", - "ScopeParseError", - "ScopeCycleError", -) diff --git a/src/globus_sdk/experimental/scope_parser/__main__.py b/src/globus_sdk/experimental/scope_parser/__main__.py deleted file mode 100644 index a21740a81..000000000 --- a/src/globus_sdk/experimental/scope_parser/__main__.py +++ /dev/null @@ -1,99 +0,0 @@ -from __future__ import annotations - -import sys -import timeit -import warnings - -from ._parser import parse_scope_graph - -warnings.warn( - "This is a test module for scope parsing. It is not intended for public use.", - stacklevel=1, -) - - -def timeit_test() -> None: - for size, num_iterations, style in ( - (10, 1000, "deep"), - (100, 1000, "deep"), - (1000, 1000, "deep"), - (2000, 100, "deep"), - (3000, 100, "deep"), - (4000, 100, "deep"), - (5000, 100, "deep"), - (5000, 1000, "wide"), - (10000, 1000, "wide"), - ): - if style == "deep": - setup = f"""\ -from globus_sdk.experimental.scope_parser import Scope -big_scope = "" -for i in range({size}): - big_scope += f"foo{{i}}[" -big_scope += "bar" -for _ in range({size}): - big_scope += "]" -""" - elif style == "wide": - setup = f"""\ -from globus_sdk.experimental.scope_parser import Scope -big_scope = "" -for i in range({size}): - big_scope += f"foo{{i}} " -""" - else: - raise NotImplementedError - - timer = timeit.Timer("Scope.parse(big_scope)", setup=setup) - - raw_timings = timer.repeat(repeat=5, number=num_iterations) - best, worst, average, variance = _stats(raw_timings) - if style == "deep": - print(f"{num_iterations} runs on a deep scope, depth={size}") - elif style == "wide": - print(f"{num_iterations} runs on a wide scope, width={size}") - else: - raise NotImplementedError - print(f" best={best} worst={worst} average={average} variance={variance}") - print(f" normalized best={best / num_iterations}") - print() - print("The most informative stat over these timings is the min timing (best).") - print("Normed best is best/iterations.") - print( - "Max timing (worst) and dispersion (variance vis-a-vis average) indicate " - "how consistent the results are, but are not a report of speed." - ) - - -def _stats(timing_data: list[float]) -> tuple[float, float, float, float]: - best = min(timing_data) - worst = max(timing_data) - average = sum(timing_data) / len(timing_data) - variance = sum((x - average) ** 2 for x in timing_data) / len(timing_data) - return best, worst, average, variance - - -def parse_test(scope_string: str) -> None: - parsed_graph = parse_scope_graph(scope_string) - print( - "top level scopes:", - ", ".join([name for name, _optional in parsed_graph.top_level_scopes]), - ) - print(parsed_graph) - - -def main() -> None: - if len(sys.argv) < 2 or sys.argv[1] in ("-h", "--help"): - print("This script supports two usage patterns:") - print(" python -m globus_sdk.experimental.scope_parser SCOPE_STRING") - print(" python -m globus_sdk.experimental.scope_parser --timeit") - sys.exit(0) - - print() - if sys.argv[1] == "--timeit": - timeit_test() - else: - parse_test(sys.argv[1]) - - -main() diff --git a/src/globus_sdk/scopes/__init__.py b/src/globus_sdk/scopes/__init__.py index a21616274..4fda17da3 100644 --- a/src/globus_sdk/scopes/__init__.py +++ b/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", diff --git a/src/globus_sdk/experimental/scope_parser/_parser.py b/src/globus_sdk/scopes/_parser.py similarity index 100% rename from src/globus_sdk/experimental/scope_parser/_parser.py rename to src/globus_sdk/scopes/_parser.py diff --git a/src/globus_sdk/experimental/scope_parser/errors.py b/src/globus_sdk/scopes/errors.py similarity index 100% rename from src/globus_sdk/experimental/scope_parser/errors.py rename to src/globus_sdk/scopes/errors.py diff --git a/src/globus_sdk/experimental/scope_parser/scope_definition.py b/src/globus_sdk/scopes/representation.py similarity index 61% rename from src/globus_sdk/experimental/scope_parser/scope_definition.py rename to src/globus_sdk/scopes/representation.py index f4cafbca2..c4c47b09a 100644 --- a/src/globus_sdk/experimental/scope_parser/scope_definition.py +++ b/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 diff --git a/src/globus_sdk/scopes/scope_definition.py b/src/globus_sdk/scopes/scope_definition.py index 8888345ee..f370c32bc 100644 --- a/src/globus_sdk/scopes/scope_definition.py +++ b/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 diff --git a/tests/unit/experimental/test_legacy_support.py b/tests/unit/experimental/test_legacy_support.py new file mode 100644 index 000000000..9cde41677 --- /dev/null +++ b/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, + ) diff --git a/tests/unit/scopes/test_mutable_scope.py b/tests/unit/scopes/test_mutable_scope.py new file mode 100644 index 000000000..4e4e640bc --- /dev/null +++ b/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) diff --git a/tests/unit/test_scopes.py b/tests/unit/scopes/test_scope_builder.py similarity index 67% rename from tests/unit/test_scopes.py rename to tests/unit/scopes/test_scope_builder.py index ce2d0ce11..c354bef12 100644 --- a/tests/unit/test_scopes.py +++ b/tests/unit/scopes/test_scope_builder.py @@ -1,8 +1,6 @@ import uuid -import pytest - -from globus_sdk.scopes import FlowsScopes, MutableScope, ScopeBuilder +from globus_sdk.scopes import FlowsScopes, ScopeBuilder def test_url_scope_string(): @@ -79,45 +77,6 @@ def test_sb_allowed_inputs_types(): assert list_sb.do_a_thing == scope_1_urn -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)])" - ) - - def test_scopebuilder_make_mutable_produces_same_strings(): sb = ScopeBuilder(str(uuid.UUID(int=0)), known_scopes="foo", known_url_scopes="bar") assert str(sb.make_mutable("foo")) == sb.foo @@ -135,9 +94,3 @@ def test_flows_scopes_creation(): FlowsScopes.run == "https://auth.globus.org/scopes/eec9b274-0c81-4334-bdc2-54e90e689b9a/run" ) - - -@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) diff --git a/tests/unit/experimental/test_scope_parser.py b/tests/unit/scopes/test_scope_parser.py similarity index 65% rename from tests/unit/experimental/test_scope_parser.py rename to tests/unit/scopes/test_scope_parser.py index 184ec4246..6cb8b9bcd 100644 --- a/tests/unit/experimental/test_scope_parser.py +++ b/tests/unit/scopes/test_scope_parser.py @@ -2,7 +2,7 @@ import pytest -from globus_sdk.experimental.scope_parser import Scope, ScopeCycleError, ScopeParseError +from globus_sdk import Scope, ScopeCycleError, ScopeParseError def test_scope_str_and_repr_simple(): @@ -210,105 +210,6 @@ def test_scope_init_forbids_special_chars(scope_str): Scope(scope_str) -def test_scope_contains_requires_scope_objects(): - s = Scope("foo") - assert "foo" not in s - - -@pytest.mark.parametrize( - "contained, containing, expect", - [ - # string matching, including optional on both sides - ("foo", "foo", True), # identity - ("*foo", "*foo", True), # identity - ("foo", "bar", False), - ("foo", "*bar", False), - ("*foo", "bar", False), - # optional-ness is one-way when mismatched - ("foo", "*foo", False), - ("*foo", "foo", True), - # dependency matching is also one-way when mismatched - ("foo[bar]", "foo[bar]", True), # identity - ("foo[bar]", "foo", False), - ("foo", "foo[bar]", True), - ("foo", "foo[bar[baz]]", True), - ("foo[bar]", "foo[bar[baz]]", True), - ("foo[bar[baz]]", "foo[bar[baz]]", True), # identity - # and the combination of dependencies with optional also works - ("foo[*bar]", "foo[bar[baz]]", True), - ("foo[*bar]", "foo[*bar[baz]]", True), - ("foo[bar]", "foo[bar[*baz]]", True), - ("foo[*bar]", "foo[bar[*baz]]", True), - ], -) -def test_scope_contains_simple_cases(contained, containing, expect): - outer_s = Scope.deserialize(containing) - inner_s = Scope.deserialize(contained) - assert (inner_s in outer_s) == expect - - -@pytest.mark.parametrize( - "contained, containing, expect", - [ - # "simple" cases for multiple dependencies - ("foo[bar baz]", "foo[bar[baz] baz]", True), - ("foo[bar baz]", "foo[bar[baz]]", False), - ("foo[baz bar]", "foo[bar[baz] baz]", True), - ("foo[bar baz]", "foo[bar[baz] baz buzz]", True), - # these scenarios will mirror some "realistic" usage - ( - "timer[transfer_ap[transfer]]", - "timer[transfer_ap[transfer[*foo/data_access]]]", - True, - ), - ( - "timer[transfer_ap[transfer[*foo/data_access]]]", - "timer[transfer_ap[transfer[*foo/data_access *bar/data_access]]]", - True, - ), - ( - "timer[transfer_ap[transfer[*bar *foo]]]", - "timer[transfer_ap[transfer[*foo *bar *baz]]]", - True, - ), - ( - "timer[transfer_ap[transfer[*foo/data_access]]]", - "timer[transfer_ap[transfer]]", - False, - ), - ( - "timer[transfer_ap[transfer[*foo/data_access *bar/data_access]]]", - "timer[transfer_ap[transfer[*foo/data_access]]]", - False, - ), - ( - "timer[transfer_ap[transfer[foo/data_access]]]", - "timer[transfer_ap[transfer[*foo/data_access]]]", - False, - ), - ( - "timer[transfer_ap[transfer[*foo/data_access]]]", - "timer[transfer_ap[transfer[foo/data_access]]]", - True, - ), - ( - "timer[transfer_ap[*transfer[*foo/data_access]]]", - "timer[transfer_ap[transfer[foo/data_access]]]", - True, - ), - ( - "timer[transfer_ap[transfer[*foo/data_access]]]", - "timer[transfer_ap[*transfer[foo/data_access]]]", - False, - ), - ], -) -def test_scope_contains_complex_usages(contained, containing, expect): - outer_s = Scope.deserialize(containing) - inner_s = Scope.deserialize(contained) - assert (inner_s in outer_s) == expect - - @pytest.mark.parametrize( "original, reserialized", [ diff --git a/tests/unit/experimental/test_scope_parser_intermediate_representations.py b/tests/unit/scopes/test_scope_parser_intermediate_representations.py similarity index 93% rename from tests/unit/experimental/test_scope_parser_intermediate_representations.py rename to tests/unit/scopes/test_scope_parser_intermediate_representations.py index bac7dce65..06a3d89d9 100644 --- a/tests/unit/experimental/test_scope_parser_intermediate_representations.py +++ b/tests/unit/scopes/test_scope_parser_intermediate_representations.py @@ -1,7 +1,4 @@ -from globus_sdk.experimental.scope_parser._parser import ( - ScopeTreeNode, - parse_scope_graph, -) +from globus_sdk.scopes._parser import ScopeTreeNode, parse_scope_graph def test_graph_str_single_node():