From 8b8233f735ac3420d1147a4e295fac32ea10b9a7 Mon Sep 17 00:00:00 2001 From: Mike Arbelaez <8730430+m1yag1@users.noreply.github.com> Date: Wed, 16 Oct 2024 16:14:32 -0500 Subject: [PATCH] Support `version` field in `SearchClient.post_search()` (#1079) * Support `version` field in `SearchClient.post_search()` * Updated SphinxDocs for `SearchQueryV1` * Added `SearchQueryV1` to the _generate_init.py script * Added `SearchQueryV1` class for new v1 search request version * Added deprecation warning to the `SearchQuery` class during init * Added unit and functional tests for `SearchQueryV1` * Updated unit tests with `@pytest.mark.filterwarnings` for deprecation warnings --------- Co-authored-by: MaxTueckeGlobus Co-authored-by: Stephen Rosen --- ...ag1_sc_35200_version_field_post_search.rst | 6 ++ docs/services/search.rst | 4 + src/globus_sdk/__init__.py | 3 + src/globus_sdk/_generate_init.py | 2 + src/globus_sdk/services/search/__init__.py | 10 ++- src/globus_sdk/services/search/data.py | 54 +++++++++++++- .../functional/services/search/test_search.py | 73 ++++++++++++++++--- tests/unit/helpers/test_search.py | 44 ++++++++++- 8 files changed, 178 insertions(+), 18 deletions(-) create mode 100644 changelog.d/20241011_113822_8730430+m1yag1_sc_35200_version_field_post_search.rst diff --git a/changelog.d/20241011_113822_8730430+m1yag1_sc_35200_version_field_post_search.rst b/changelog.d/20241011_113822_8730430+m1yag1_sc_35200_version_field_post_search.rst new file mode 100644 index 000000000..27f769f04 --- /dev/null +++ b/changelog.d/20241011_113822_8730430+m1yag1_sc_35200_version_field_post_search.rst @@ -0,0 +1,6 @@ +Added +~~~~~ + +- ``SearchQueryV1`` is a new class for submitting complex queries replacing + the legacy ``SearchQuery`` class. A deprecation warning has been added to the + ``SearchQuery`` class. (:pr:`1079`) diff --git a/docs/services/search.rst b/docs/services/search.rst index 2dce45328..38146ac9e 100644 --- a/docs/services/search.rst +++ b/docs/services/search.rst @@ -25,6 +25,10 @@ only to document the methods it provides to its subclasses. :members: :show-inheritance: +.. autoclass:: SearchQueryV1 + :members: + :show-inheritance: + .. autoclass:: SearchScrollQuery :members: :show-inheritance: diff --git a/src/globus_sdk/__init__.py b/src/globus_sdk/__init__.py index 548c1c79d..1b94b1dd1 100644 --- a/src/globus_sdk/__init__.py +++ b/src/globus_sdk/__init__.py @@ -128,6 +128,7 @@ def _force_eager_imports() -> None: "SearchAPIError", "SearchClient", "SearchQuery", + "SearchQueryV1", "SearchScrollQuery", }, "services.timers": { @@ -255,6 +256,7 @@ def _force_eager_imports() -> None: from .services.search import SearchAPIError from .services.search import SearchClient from .services.search import SearchQuery + from .services.search import SearchQueryV1 from .services.search import SearchScrollQuery from .services.timers import TimersAPIError from .services.timers import TimersClient @@ -405,6 +407,7 @@ def __getattr__(name: str) -> t.Any: "SearchAPIError", "SearchClient", "SearchQuery", + "SearchQueryV1", "SearchScrollQuery", "SpecificFlowClient", "StorageGatewayDocument", diff --git a/src/globus_sdk/_generate_init.py b/src/globus_sdk/_generate_init.py index 90ee5562f..e39824b61 100755 --- a/src/globus_sdk/_generate_init.py +++ b/src/globus_sdk/_generate_init.py @@ -204,7 +204,9 @@ def __getattr__(name: str) -> t.Any: ( "SearchAPIError", "SearchClient", + # legacy class (remove in the future) "SearchQuery", + "SearchQueryV1", "SearchScrollQuery", ), ), diff --git a/src/globus_sdk/services/search/__init__.py b/src/globus_sdk/services/search/__init__.py index 60edab284..1084a4948 100644 --- a/src/globus_sdk/services/search/__init__.py +++ b/src/globus_sdk/services/search/__init__.py @@ -1,5 +1,11 @@ from .client import SearchClient -from .data import SearchQuery, SearchScrollQuery +from .data import SearchQuery, SearchQueryV1, SearchScrollQuery from .errors import SearchAPIError -__all__ = ("SearchClient", "SearchQuery", "SearchScrollQuery", "SearchAPIError") +__all__ = ( + "SearchClient", + "SearchQuery", + "SearchQueryV1", + "SearchScrollQuery", + "SearchAPIError", +) diff --git a/src/globus_sdk/services/search/data.py b/src/globus_sdk/services/search/data.py index 56b001262..e438dd8e0 100644 --- a/src/globus_sdk/services/search/data.py +++ b/src/globus_sdk/services/search/data.py @@ -2,7 +2,7 @@ import typing as t -from globus_sdk import utils +from globus_sdk import exc, utils # workaround for absence of Self type # for the workaround and some background, see: @@ -115,6 +115,7 @@ def __init__( additional_fields: dict[str, t.Any] | None = None, ): super().__init__() + exc.warn_deprecated("'SearchQuery' is deprecated. Use 'SearchQueryV1' instead.") if q is not None: self["q"] = q if limit is not None: @@ -221,6 +222,57 @@ def add_sort( return self +class SearchQueryV1(utils.PayloadWrapper): + """ + A specialized dict which has helpers for creating and modifying a Search + Query document. Replaces the usage of ``SearchQuery``. + + :param q: The query string. Required unless filters are used. + :param limit: A limit on the number of results returned in a single page + :param offset: An offset into the set of all results for the query + :param advanced: Whether to enable (``True``) or not to enable (``False``) advanced + parsing of query strings. The default of ``False`` is robust and guarantees that + the query will not error with "bad query string" errors + :param filters: a list of filters to apply to the query + :param facets: a list of facets to apply to the query + :param post_facet_filters: a list of filters to apply after facet + results are returned + :param boosts: a list of boosts to apply to the query + :param sort: a list of fields to sort results + :param additional_fields: additional data to include in the query document + """ + + def __init__( + self, + *, + q: str | utils.MissingType = utils.MISSING, + limit: int | utils.MissingType = utils.MISSING, + offset: int | utils.MissingType = utils.MISSING, + advanced: bool | utils.MissingType = utils.MISSING, + filters: list[dict[str, t.Any]] | utils.MissingType = utils.MISSING, + facets: list[dict[str, t.Any]] | utils.MissingType = utils.MISSING, + post_facet_filters: list[dict[str, t.Any]] | utils.MissingType = utils.MISSING, + boosts: list[dict[str, t.Any]] | utils.MissingType = utils.MISSING, + sort: list[dict[str, t.Any]] | utils.MissingType = utils.MISSING, + additional_fields: dict[str, t.Any] | utils.MissingType = utils.MISSING, + ): + super().__init__() + self["@version"] = "query#1.0.0" + + self["q"] = q + self["limit"] = limit + self["offset"] = offset + self["advanced"] = advanced + self["filters"] = filters + self["facets"] = facets + self["post_facet_filters"] = post_facet_filters + self["boosts"] = boosts + self["sort"] = sort + + if not isinstance(additional_fields, utils.MissingType): + self.update(additional_fields) + + class SearchScrollQuery(SearchQueryBase): """ A scrolling query type, for scrolling the full result set for an index. diff --git a/tests/functional/services/search/test_search.py b/tests/functional/services/search/test_search.py index f8695cd3a..7a2d7b43e 100644 --- a/tests/functional/services/search/test_search.py +++ b/tests/functional/services/search/test_search.py @@ -39,14 +39,7 @@ def test_search_query_simple(search_client): } -@pytest.mark.parametrize( - "query_doc", - [ - {"q": "foo"}, - {"q": "foo", "limit": 10}, - globus_sdk.SearchQuery("foo"), - ], -) +@pytest.mark.parametrize("query_doc", [{"q": "foo"}, {"q": "foo", "limit": 10}]) def test_search_post_query_simple(search_client, query_doc): meta = load_response(search_client.post_search).metadata @@ -63,12 +56,44 @@ def test_search_post_query_simple(search_client, query_doc): assert req_body == dict(query_doc) +@pytest.mark.filterwarnings("ignore:'SearchQuery'*:DeprecationWarning") +def test_search_post_query_with_legacy_helper(search_client): + meta = load_response(search_client.post_search).metadata + query_doc = globus_sdk.SearchQuery("foo") + + res = search_client.post_search(meta["index_id"], query_doc) + assert res.http_status == 200 + + data = res.data + assert isinstance(data, dict) + assert data["gmeta"][0]["entries"][0]["content"]["foo"] == "bar" + + req = get_last_request() + assert req.body is not None + req_body = json.loads(req.body) + assert req_body == dict(query_doc) + + +def test_search_post_query_simple_with_v1_helper(search_client): + query_doc = globus_sdk.SearchQueryV1(q="foo") + meta = load_response(search_client.post_search).metadata + + res = search_client.post_search(meta["index_id"], query_doc) + assert res.http_status == 200 + + data = res.data + assert isinstance(data, dict) + assert data["gmeta"][0]["entries"][0]["content"]["foo"] == "bar" + + req = get_last_request() + assert req.body is not None + req_body = json.loads(req.body) + assert req_body == {"@version": "query#1.0.0", "q": "foo"} + + @pytest.mark.parametrize( "query_doc", - [ - {"q": "foo", "limit": 10, "offset": 0}, - globus_sdk.SearchQuery("foo", limit=10, offset=0), - ], + [{"q": "foo", "limit": 10, "offset": 0}], ) def test_search_post_query_arg_overrides(search_client, query_doc): meta = load_response(search_client.post_search).metadata @@ -92,6 +117,30 @@ def test_search_post_query_arg_overrides(search_client, query_doc): assert query_doc["offset"] == 0 +@pytest.mark.filterwarnings("ignore:'SearchQuery'*:DeprecationWarning") +def test_search_post_query_arg_overrides_with_legacy_helper(search_client): + meta = load_response(search_client.post_search).metadata + query_doc = globus_sdk.SearchQuery("foo", limit=10, offset=0) + + res = search_client.post_search(meta["index_id"], query_doc, limit=100, offset=150) + assert res.http_status == 200 + + data = res.data + assert isinstance(data, dict) + assert data["gmeta"][0]["entries"][0]["content"]["foo"] == "bar" + + req = get_last_request() + assert req.body is not None + req_body = json.loads(req.body) + assert req_body != dict(query_doc) + assert req_body["q"] == query_doc["q"] + assert req_body["limit"] == 100 + assert req_body["offset"] == 150 + # important! these should be unchanged (no side-effects) + assert query_doc["limit"] == 10 + assert query_doc["offset"] == 0 + + @pytest.mark.parametrize( "query_doc", [ diff --git a/tests/unit/helpers/test_search.py b/tests/unit/helpers/test_search.py index 9c7b431ae..db1552029 100644 --- a/tests/unit/helpers/test_search.py +++ b/tests/unit/helpers/test_search.py @@ -4,13 +4,15 @@ import pytest -from globus_sdk import SearchQuery +from globus_sdk import SearchQuery, SearchQueryV1, utils +from globus_sdk.exc.warnings import RemovedInV4Warning -def test_init(): +@pytest.mark.filterwarnings("ignore:'SearchQuery'*:DeprecationWarning") +def test_init_legacy(): """Creates SearchQuery and verifies results""" - # default init query = SearchQuery() + assert len(query) == 0 # init with supported fields @@ -26,7 +28,39 @@ def test_init(): assert param_query[par] == add_params[par] +def test_init_legacy_deprecation_warning(): + with pytest.warns( + RemovedInV4Warning, + match="'SearchQuery' is deprecated. Use 'SearchQueryV1' instead.", + ): + SearchQuery() + + +def test_init_v1(): + query = SearchQueryV1() + + # ensure the version is set to query#1.0.0 + assert query["@version"] == "query#1.0.0" + + # ensure key attributes initialize to empty lists + for attribute in ["facets", "filters", "post_facet_filters", "sort", "boosts"]: + assert query[attribute] == utils.MISSING + + # init with supported fields + params = {"q": "foo", "limit": 10, "offset": 0, "advanced": False} + param_query = SearchQueryV1(**params) + for par in params: + assert param_query[par] == params[par] + + # init with additional_fields + add_params = {"param1": "value1", "param2": "value2"} + param_query = SearchQueryV1(additional_fields=add_params) + for par in add_params: + assert param_query[par] == add_params[par] + + @pytest.mark.parametrize("attrname", ["q", "limit", "offset", "advanced"]) +@pytest.mark.filterwarnings("ignore:'SearchQuery'*:DeprecationWarning") def test_set_method(attrname): query = SearchQuery() method = getattr(query, "set_{}".format("query" if attrname == "q" else attrname)) @@ -38,6 +72,7 @@ def test_set_method(attrname): assert query[attrname] == "foo" +@pytest.mark.filterwarnings("ignore:'SearchQuery'*:DeprecationWarning") def test_add_facet(): query = SearchQuery() assert "facets" not in query @@ -93,6 +128,7 @@ def test_add_facet(): } +@pytest.mark.filterwarnings("ignore:'SearchQuery'*:DeprecationWarning") def test_add_filter(): query = SearchQuery() assert "filters" not in query @@ -133,6 +169,7 @@ def test_add_filter(): } +@pytest.mark.filterwarnings("ignore:'SearchQuery'*:DeprecationWarning") def test_add_boost(): query = SearchQuery() assert "boosts" not in query @@ -154,6 +191,7 @@ def test_add_boost(): } +@pytest.mark.filterwarnings("ignore:'SearchQuery'*:DeprecationWarning") def test_add_sort(): query = SearchQuery() assert "sort" not in query