From 6fedf8fd5b751326e626221344ea9f7ae1843058 Mon Sep 17 00:00:00 2001 From: Nathan Gillett Date: Mon, 9 Dec 2019 11:33:32 -0500 Subject: [PATCH] Restrict searching repository content to Repository objects Client.search_repository_content was found to be unnecessary, so this commit removes it. A Repository object is now required to search content therein. A issue involving type_ids in criteria structures was also corrected. --- examples/search-repo-content | 14 +----- pubtools/pulplib/_impl/client/client.py | 39 +++------------- pubtools/pulplib/_impl/fake/client.py | 21 +-------- .../pulplib/_impl/model/repository/base.py | 10 ++++- tests/client/test_client.py | 39 ++-------------- tests/fake/test_fake_search.py | 44 ------------------- 6 files changed, 22 insertions(+), 145 deletions(-) diff --git a/examples/search-repo-content b/examples/search-repo-content index fb304d69..fc8ec30d 100644 --- a/examples/search-repo-content +++ b/examples/search-repo-content @@ -44,18 +44,8 @@ def main(): log.setLevel(logging.DEBUG) client = make_client(p) - units = client.search_repository_content(p.repo_id, type_ids=p.content_type) - - """ - Alternatively, one could use the following: - - .. code-block:: python - repository = client.get_repository(p.repo_id) - units = repository.search_content(type_id=p.content_type) - - This is functionally equivalent to client.search_repository_content - but is useful if further operations on the repository are required. - """ + repository = client.get_repository(p.repo_id) + units = repository.search_content(type_id=p.content_type) if units: log.info("Found %s %s units: \n\t%s" % ( diff --git a/pubtools/pulplib/_impl/client/client.py b/pubtools/pulplib/_impl/client/client.py index b5ce9244..60ba7597 100644 --- a/pubtools/pulplib/_impl/client/client.py +++ b/pubtools/pulplib/_impl/client/client.py @@ -14,7 +14,7 @@ from ..page import Page from ..criteria import Criteria from ..model import Repository, MaintenanceReport, Distributor, Unit -from .search import filters_for_criteria, validate_type_ids +from .search import filters_for_criteria from .errors import PulpException from .poller import TaskPoller from . import retry @@ -203,36 +203,6 @@ def search_repository(self, criteria=None): Repository, "repositories", criteria=criteria, search_options=search_options ) - def search_repository_content(self, repo_id, type_ids, criteria=None): - """Search the given repository for content matching the given criteria. - - Args: - repo_id (str) - The ID of the repository to search. - type_ids (str, list, tuple) - A list of content types to search. - criteria (:class:`~pubtools.pulplib.Criteria`) - A criteria object used for this search. - - Returns: - list[:class:`~pubtools.pulplib.Unit`] - A list of zero or more :class:`~pubtools.pulplib.Unit` - subclasses found by the search operation. - - Raises: - :class:`~pubtools.pulplib.InvalidContentTypeException` - If the criteria does not contain a valid _content_type_id. - - .. versionadded:: 2.4.0 - """ - resource_type = "repositories/%s" % repo_id - search_options = {"type_ids": validate_type_ids(type_ids)} - return list( - self._search( - Unit, resource_type, criteria=criteria, search_options=search_options - ) - ) - def search_distributor(self, criteria=None): """Search the distributors matching the given criteria. @@ -253,13 +223,16 @@ def search_distributor(self, criteria=None): return self._search(Distributor, "distributors", criteria=criteria) def _search(self, return_type, resource_type, criteria=None, search_options=None): - type_ids = search_options.pop("type_ids", None) if search_options else None pulp_crit = { "skip": 0, "limit": self._PAGE_SIZE, "filters": filters_for_criteria(criteria, return_type), - "type_ids": type_ids, } + + type_ids = search_options.pop("type_ids", None) if search_options else None + if type_ids: + pulp_crit["type_ids"] = type_ids + search = {"criteria": pulp_crit} search.update(search_options or {}) diff --git a/pubtools/pulplib/_impl/fake/client.py b/pubtools/pulplib/_impl/fake/client.py index 2b0712d5..8a706e21 100644 --- a/pubtools/pulplib/_impl/fake/client.py +++ b/pubtools/pulplib/_impl/fake/client.py @@ -19,10 +19,9 @@ Task, Repository, Distributor, - Unit, MaintenanceReport, ) -from pubtools.pulplib._impl.client.search import filters_for_criteria, validate_type_ids +from pubtools.pulplib._impl.client.search import filters_for_criteria from .. import compat_attr as attr from .match import match_object @@ -91,24 +90,6 @@ def search_repository(self, criteria=None): random.shuffle(repos) return self._prepare_pages(repos) - def search_repository_content(self, repo_id, type_ids, criteria=None): - criteria = criteria or Criteria.true() - units = [] - - filters_for_criteria(criteria, Unit) - - try: - for unit in self._repo_units[repo_id]: - if unit.content_type_id in validate_type_ids(type_ids) and match_object( - criteria, unit - ): - units.append(attr.evolve(unit)) - except Exception as ex: # pylint: disable=broad-except - return f_return_error(ex) - - random.shuffle(units) - return self._prepare_pages(units) - def search_distributor(self, criteria=None): criteria = criteria or Criteria.true() distributors = [] diff --git a/pubtools/pulplib/_impl/model/repository/base.py b/pubtools/pulplib/_impl/model/repository/base.py index 1dd0d004..4109caff 100644 --- a/pubtools/pulplib/_impl/model/repository/base.py +++ b/pubtools/pulplib/_impl/model/repository/base.py @@ -12,6 +12,8 @@ from ..attr import pulp_attrib from ..distributor import Distributor from ..frozenlist import FrozenList +from ..unit import Unit +from ...client.search import validate_type_ids from ...schema import load_schema from ... import compat_attr as attr @@ -266,7 +268,13 @@ def search_content(self, type_id, criteria=None): if not self._client: raise DetachedException() - return self._client.search_repository_content(self.id, type_id, criteria) + resource_type = "repositories/%s" % self.id + search_options = {"type_ids": validate_type_ids(type_id)} + return list( + self._client._search( + Unit, resource_type, criteria=criteria, search_options=search_options + ) + ) def delete(self): """Delete this repository from Pulp. diff --git a/tests/client/test_client.py b/tests/client/test_client.py index 36b18980..04036b57 100644 --- a/tests/client/test_client.py +++ b/tests/client/test_client.py @@ -58,37 +58,6 @@ def test_can_search(client, requests_mocker): assert requests_mocker.call_count == 1 -def test_can_search_repository_content(client, requests_mocker): - """search_repository_content issues /search/units/ POST requests as expected""" - requests_mocker.post( - "https://pulp.example.com/pulp/api/v2/repositories/some-repo/search/units/", - json=[ - { - "metadata": { - "_content_type_id": "modulemd_defaults", - "name": "mdd", - "repo_id": "mdd-repo", - "stream": "1.0", - "profiles": {"p1": ["something"]}, - } - } - ], - ) - - units = client.search_repository_content("some-repo", type_ids="modulemd_defaults") - - # It should have returned one ModulemdDefaultsUnit - assert units == [ - ModulemdDefaultsUnit( - content_type_id="modulemd_defaults", - name="mdd", - repo_id="mdd-repo", - stream="1.0", - profiles={"p1": ["something"]}, - ) - ] - - def test_can_search_distributor(client, requests_mocker): """search_distributor issues distributors/search POST request as expected.""" requests_mocker.post( @@ -248,10 +217,10 @@ def test_search_can_paginate(client, requests_mocker): # We'll just sample a few of the requests here. history = requests_mocker.request_history criteria = [h.json()["criteria"] for h in history] - assert criteria[0] == {"filters": {}, "skip": 0, "limit": 10, "type_ids": None} - assert criteria[1] == {"filters": {}, "skip": 10, "limit": 10, "type_ids": None} - assert criteria[2] == {"filters": {}, "skip": 20, "limit": 10, "type_ids": None} - assert criteria[-1] == {"filters": {}, "skip": 990, "limit": 10, "type_ids": None} + assert criteria[0] == {"filters": {}, "skip": 0, "limit": 10} + assert criteria[1] == {"filters": {}, "skip": 10, "limit": 10} + assert criteria[2] == {"filters": {}, "skip": 20, "limit": 10} + assert criteria[-1] == {"filters": {}, "skip": 990, "limit": 10} def test_can_get(client, requests_mocker): diff --git a/tests/fake/test_fake_search.py b/tests/fake/test_fake_search.py index daade83c..00877766 100644 --- a/tests/fake/test_fake_search.py +++ b/tests/fake/test_fake_search.py @@ -213,12 +213,8 @@ def test_search_bad_criteria(): with pytest.raises(Exception) as dist_exc: client.search_distributor("invalid criteria") - with pytest.raises(Exception) as repo_cont_exc: - client.search_repository_content("some-repo", "rpm", "criteria?") - assert "Not a criteria" in str(repo_exc.value) assert "Not a criteria" in str(dist_exc.value) - assert "Not a criteria" in str(repo_cont_exc.value) def test_search_created_timestamp(): @@ -357,46 +353,6 @@ def test_search_paginates(): assert sorted(found_repos) == sorted(repos) -@pytest.mark.parametrize("type_id", ["rpm", "quark"]) -def test_search_repository_content(type_id): - """Can return desired units when type ID is valid and raises otherwise""" - controller = FakeController() - - repo1 = Repository(id="repo1") - controller.insert_repository(repo1) - - units = [ - Unit.from_data( - { - "_content_type_id": "iso", - "name": "hello.txt", - "size": 23, - "checksum": "a" * 64, - } - ), - Unit.from_data( - { - "_content_type_id": "rpm", - "name": "bash", - "epoch": "0", - "filename": "bash-x86_64.rpm", - "version": "4.0", - "release": "1", - "arch": "x86_64", - } - ), - ] - controller.insert_units(repo1, units) - - if type_id is "quark": - with pytest.raises(Exception) as ex: - controller.client.search_repository_content("repo1", type_ids=type_id) - assert "Invalid content type ID(s)" in str(ex.value) - else: - found = controller.client.search_repository_content("repo1", type_ids="rpm") - assert sorted(found) == [units[1]] - - def test_search_distributor(): controller = FakeController()