From dbea7bdaba4b283d12ed4a6428573ac942bce875 Mon Sep 17 00:00:00 2001 From: Nathan Gillett Date: Mon, 11 Nov 2019 15:36:03 -0500 Subject: [PATCH 01/19] Introduce search_content method for repositories [DELIVERY-7839] This commit adds the search_content method for the Repository class which retrieves rpm, srpm, modulemd, and modulemd_default content with optional filtering by name, arch, file name, or modulemd stream. --- pubtools/pulplib/_impl/client/client.py | 25 +++--- .../pulplib/_impl/model/repository/base.py | 49 +++++++++++ pubtools/pulplib/_impl/schema/unit.yaml | 4 +- tests/repository/test_search_content.py | 84 +++++++++++++++++++ 4 files changed, 151 insertions(+), 11 deletions(-) create mode 100644 tests/repository/test_search_content.py diff --git a/pubtools/pulplib/_impl/client/client.py b/pubtools/pulplib/_impl/client/client.py index 29adb955..bc50b691 100644 --- a/pubtools/pulplib/_impl/client/client.py +++ b/pubtools/pulplib/_impl/client/client.py @@ -13,7 +13,7 @@ from ..page import Page from ..criteria import Criteria -from ..model import Repository, MaintenanceReport, Distributor +from ..model import Repository, MaintenanceReport, Distributor, Unit from .search import filters_for_criteria from .errors import PulpException from .poller import TaskPoller @@ -231,13 +231,18 @@ def _search(self, return_type, resource_type, criteria=None, search_options=None search = {"criteria": pulp_crit} search.update(search_options or {}) - response_f = self._do_search(resource_type, search) + url = os.path.join(self._url, "pulp/api/v2/%s/search/" % resource_type) + + if return_type is Unit and "repositories/" in resource_type: + url = os.path.join(url, "units/") + + response_f = self._do_search(url, search) # When this request is resolved, we'll have the first page of data. # We'll need to convert that into a page and also keep going with # the search if there's more to be done. return f_proxy( - f_map(response_f, lambda data: self._handle_page(return_type, search, data)) + f_map(response_f, lambda data: self._handle_page(url, return_type, search, data)) ) def get_maintenance_report(self): @@ -417,12 +422,14 @@ def _log_spawned_tasks(cls, taskdata): pass return taskdata - def _handle_page(self, object_class, search, raw_data): + def _handle_page(self, url, object_class, search, raw_data): LOG.debug("Got pulp response for %s, %s elems", search, len(raw_data)) page_data = [object_class.from_data(elem) for elem in raw_data] for obj in page_data: - obj._set_client(self) + # Set client is only applicable for repository and distributor objects + if hasattr(obj, "_set_client"): + obj._set_client(self) # Do we need a next page? next_page = None @@ -433,11 +440,11 @@ def _handle_page(self, object_class, search, raw_data): search = search.copy() search["criteria"] = search["criteria"].copy() search["criteria"]["skip"] = search["criteria"]["skip"] + limit - response_f = self._do_search("repositories", search) + response_f = self._do_search(url, search) next_page = f_proxy( f_map( response_f, - lambda data: self._handle_page(object_class, search, data), + lambda data: self._handle_page(url, object_class, search, data), ) ) @@ -458,9 +465,7 @@ def _new_session(self): def _do_request(self, **kwargs): return self._session.request(**kwargs) - def _do_search(self, resource_type, search): - url = os.path.join(self._url, "pulp/api/v2/{0}/search/".format(resource_type)) - + def _do_search(self, url, search): LOG.debug("Submitting %s search: %s", url, search) return self._request_executor.submit( self._do_request, method="POST", url=url, json=search diff --git a/pubtools/pulplib/_impl/model/repository/base.py b/pubtools/pulplib/_impl/model/repository/base.py index 8dd0d3b6..5bd5e0e6 100644 --- a/pubtools/pulplib/_impl/model/repository/base.py +++ b/pubtools/pulplib/_impl/model/repository/base.py @@ -10,6 +10,8 @@ from ..frozenlist import FrozenList from ...schema import load_schema from ... import compat_attr as attr +from ...criteria import Criteria, Matcher +from ..unit.base import Unit LOG = logging.getLogger("pubtools.pulplib") @@ -188,6 +190,53 @@ def distributor(self, distributor_id): """ return self._distributors_by_id.get(distributor_id) + def search_content(self, name=None, arch=None, filename=None, stream=None): + """Search this repository for content matching the given criteria. + + Args: + name (str) + Name of the desired unit. + arch (str) + Architecture of the desired unit. + filename (str) + File name of the desired unit. + stream (str) + Stream of the desired modulemd or modulemd_defaults unit. + + Returns: + Future[list[:class:`~pubtools.pulplib.Task`]] + A future which is resolved when search succeeds. + + The future contains a list of zero or more tasks triggered and awaited + during the search operation. + + Raises: + DetachedException + If this instance is not attached to a Pulp client. + + """ + if not self._client: + raise DetachedException() + + criteria = ( + Criteria.with_field("type_ids", Matcher.in_( + ["rpm", "srpm", "modulemd", "modulemd_defaults"] + )), + ) + + if name: + criteria = criteria + (Criteria.with_field("unit.name", name),) + if arch: + criteria = criteria + (Criteria.with_field("unit.arch", arch),) + if filename: + criteria = criteria + (Criteria.with_field("unit.filename", filename),) + if stream: + criteria = criteria + (Criteria.with_field("unit.stream", stream),) + + return f_proxy(self._client._search( + Unit, "repositories/%s" % self.id, criteria=Criteria.and_(*criteria) + )) + def delete(self): """Delete this repository from Pulp. diff --git a/pubtools/pulplib/_impl/schema/unit.yaml b/pubtools/pulplib/_impl/schema/unit.yaml index ecc75b34..506f67f3 100644 --- a/pubtools/pulplib/_impl/schema/unit.yaml +++ b/pubtools/pulplib/_impl/schema/unit.yaml @@ -95,7 +95,9 @@ definitions: properties: # Type of the unit _content_type_id: - const: modulemd + enum: + - modulemd + - modulemd_defaults # NSVCA components name: diff --git a/tests/repository/test_search_content.py b/tests/repository/test_search_content.py new file mode 100644 index 00000000..ce1c4bb6 --- /dev/null +++ b/tests/repository/test_search_content.py @@ -0,0 +1,84 @@ +import pytest +from pubtools.pulplib import Repository, DetachedException, RpmUnit, ModulemdUnit + + +def test_detached(): + """search_content raises if called on a detached repo""" + with pytest.raises(DetachedException): + Repository(id="some-repo").search_content() + + +class TestSearchContent(object): + def __init__(self, requests_mocker, client): + self.repo = Repository(id="some-repo") + self.repo.__dict__["_client"] = client + + requests_mocker.post( + "https://pulp.example.com/pulp/api/v2/repositories/some-repo/search/units/", + json=[ + { + "_content_type_id": "iso", + "name": "hello.txt", + "size": 23, + "checksum": "a" * 64, + }, + { + "_content_type_id": "srpm", + "name": "bash", + "epoch": "0", + "version": "4.0", + "release": "1", + "arch": "x86_64", + }, + { + "_content_type_id": "rpm", + "name": "bash", + "epoch": "0", + "version": "4.0", + "release": "1", + "arch": "x86_64", + }, + { + "_content_type_id": "modulemd", + "name": "m1", + "stream": "s1", + "version": 1234, + "context": "a1b2c3", + "arch": "s390x", + }, + { + "_content_type_id": "modulemd_defaults", + "name": "m2", + "stream": "s2", + "version": 1234, + "context": "a1b2c3", + "arch": "s390x", + }, + ] + ) + + def test_search_content(self): + """search_content gets all rpm and modulemd content from the repository""" + units_f = self.repo.search_content() + units = [unit for unit in units_f.result().as_iter()] + + assert len(units) == 4 + assert sorted(units)[0].content_type_id == "modulemd" + assert sorted(units)[1].content_type_id == "modulemd_defaults" + assert sorted(units)[2].content_type_id == "rpm" + assert sorted(units)[3].content_type_id == "srpm" + + def test_search_matched_content(self): + """search_content gets only matching content from the repository""" + units_f = self.repo.search_content(arch="s390x", stream="s1") + units = [unit for unit in units_f.result().as_iter()] + + assert len(units) == 1 + assert units[0].content_type_id == "modulemd" + + def test_search_no_matched_content(self): + """search_content gets no matching content from the repository""" + units_f = self.repo.search_content(name="hello.txt") + units = [unit for unit in units_f.result().as_iter()] + + assert len(units) == 0 From 3e34df703cf12bc69a07e218569a4f6ecb9d4ccf Mon Sep 17 00:00:00 2001 From: Nathan Gillett Date: Mon, 11 Nov 2019 15:53:49 -0500 Subject: [PATCH 02/19] Allow black to refactor and bump version to 2.4.0 per pidiff --- CHANGELOG.md | 4 ++++ pubtools/pulplib/_impl/client/client.py | 5 ++++- pubtools/pulplib/_impl/model/repository/base.py | 16 ++++++++++------ setup.py | 2 +- tests/repository/test_search_content.py | 2 +- 5 files changed, 20 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a045ca8a..7cde0c4e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added +- Introduced `Repository.search_content` API for retrieving content units + from Pulp repositories. + ### Fixed - Fixed a bug that export an empty maintenance report would crash. diff --git a/pubtools/pulplib/_impl/client/client.py b/pubtools/pulplib/_impl/client/client.py index bc50b691..a4bb0bde 100644 --- a/pubtools/pulplib/_impl/client/client.py +++ b/pubtools/pulplib/_impl/client/client.py @@ -242,7 +242,10 @@ def _search(self, return_type, resource_type, criteria=None, search_options=None # We'll need to convert that into a page and also keep going with # the search if there's more to be done. return f_proxy( - f_map(response_f, lambda data: self._handle_page(url, return_type, search, data)) + f_map( + response_f, + lambda data: self._handle_page(url, return_type, search, data), + ) ) def get_maintenance_report(self): diff --git a/pubtools/pulplib/_impl/model/repository/base.py b/pubtools/pulplib/_impl/model/repository/base.py index 5bd5e0e6..bfebc80f 100644 --- a/pubtools/pulplib/_impl/model/repository/base.py +++ b/pubtools/pulplib/_impl/model/repository/base.py @@ -214,14 +214,16 @@ def search_content(self, name=None, arch=None, filename=None, stream=None): DetachedException If this instance is not attached to a Pulp client. + .. versionadded:: 2.4.0 """ if not self._client: raise DetachedException() criteria = ( - Criteria.with_field("type_ids", Matcher.in_( - ["rpm", "srpm", "modulemd", "modulemd_defaults"] - )), + Criteria.with_field( + "type_ids", + Matcher.in_(["rpm", "srpm", "modulemd", "modulemd_defaults"]), + ), ) if name: @@ -233,9 +235,11 @@ def search_content(self, name=None, arch=None, filename=None, stream=None): if stream: criteria = criteria + (Criteria.with_field("unit.stream", stream),) - return f_proxy(self._client._search( - Unit, "repositories/%s" % self.id, criteria=Criteria.and_(*criteria) - )) + return f_proxy( + self._client._search( + Unit, "repositories/%s" % self.id, criteria=Criteria.and_(*criteria) + ) + ) def delete(self): """Delete this repository from Pulp. diff --git a/setup.py b/setup.py index d685d13b..5feb4896 100644 --- a/setup.py +++ b/setup.py @@ -21,7 +21,7 @@ def get_requirements(): setup( name="pubtools-pulplib", - version="2.3.2", + version="2.4.0", packages=find_packages(exclude=["tests"]), package_data={"pubtools.pulplib._impl.schema": ["*.yaml"]}, url="https://github.com/release-engineering/pubtools-pulplib", diff --git a/tests/repository/test_search_content.py b/tests/repository/test_search_content.py index ce1c4bb6..186a8ca4 100644 --- a/tests/repository/test_search_content.py +++ b/tests/repository/test_search_content.py @@ -54,7 +54,7 @@ def __init__(self, requests_mocker, client): "context": "a1b2c3", "arch": "s390x", }, - ] + ], ) def test_search_content(self): From 8ef4857d1732239270906e327d71cf209e592dc6 Mon Sep 17 00:00:00 2001 From: Nathan Gillett Date: Mon, 11 Nov 2019 16:14:40 -0500 Subject: [PATCH 03/19] Improve tests to cover all search filters --- tests/repository/test_search_content.py | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/tests/repository/test_search_content.py b/tests/repository/test_search_content.py index 186a8ca4..65924481 100644 --- a/tests/repository/test_search_content.py +++ b/tests/repository/test_search_content.py @@ -1,5 +1,5 @@ import pytest -from pubtools.pulplib import Repository, DetachedException, RpmUnit, ModulemdUnit +from pubtools.pulplib import Repository, DetachedException def test_detached(): @@ -26,6 +26,7 @@ def __init__(self, requests_mocker, client): "_content_type_id": "srpm", "name": "bash", "epoch": "0", + "filename": "bash-x86_64.srpm", "version": "4.0", "release": "1", "arch": "x86_64", @@ -34,6 +35,7 @@ def __init__(self, requests_mocker, client): "_content_type_id": "rpm", "name": "bash", "epoch": "0", + "filename": "bash-x86_64.rpm", "version": "4.0", "release": "1", "arch": "x86_64", @@ -68,7 +70,7 @@ def test_search_content(self): assert sorted(units)[2].content_type_id == "rpm" assert sorted(units)[3].content_type_id == "srpm" - def test_search_matched_content(self): + def test_search_modulemd_content(self): """search_content gets only matching content from the repository""" units_f = self.repo.search_content(arch="s390x", stream="s1") units = [unit for unit in units_f.result().as_iter()] @@ -76,6 +78,14 @@ def test_search_matched_content(self): assert len(units) == 1 assert units[0].content_type_id == "modulemd" + def test_search_rpm_content(self): + """search_content gets only matching content from the repository""" + units_f = self.repo.search_content(name="bash", filename="bash-x86_64.rpm") + units = [unit for unit in units_f.result().as_iter()] + + assert len(units) == 1 + assert units[0].content_type_id == "rpm" + def test_search_no_matched_content(self): """search_content gets no matching content from the repository""" units_f = self.repo.search_content(name="hello.txt") From 1a0dfccd2d908d5ccf75c8a56348a3228e745867 Mon Sep 17 00:00:00 2001 From: Nathan Gillett Date: Fri, 15 Nov 2019 11:25:12 -0500 Subject: [PATCH 04/19] Rework search_content and implement modulemd_defaults --- pubtools/pulplib/__init__.py | 1 + pubtools/pulplib/_impl/model/__init__.py | 2 +- pubtools/pulplib/_impl/model/common.py | 6 +++ .../pulplib/_impl/model/repository/base.py | 49 ++++++------------- .../_impl/model/unit/modulemd_defaults.py | 32 ++++++++++++ pubtools/pulplib/_impl/schema/unit.yaml | 33 +++++++++++-- tests/repository/test_search_content.py | 46 +++++++---------- 7 files changed, 103 insertions(+), 66 deletions(-) create mode 100644 pubtools/pulplib/_impl/model/unit/modulemd_defaults.py diff --git a/pubtools/pulplib/__init__.py b/pubtools/pulplib/__init__.py index c2e5daff..5fbf852c 100644 --- a/pubtools/pulplib/__init__.py +++ b/pubtools/pulplib/__init__.py @@ -4,6 +4,7 @@ from ._impl.model import ( PulpObject, DetachedException, + InvalidContentTypeException, InvalidDataException, Repository, YumRepository, diff --git a/pubtools/pulplib/_impl/model/__init__.py b/pubtools/pulplib/_impl/model/__init__.py index c80929e1..949a5328 100644 --- a/pubtools/pulplib/_impl/model/__init__.py +++ b/pubtools/pulplib/_impl/model/__init__.py @@ -1,4 +1,4 @@ -from .common import PulpObject, DetachedException, InvalidDataException +from .common import PulpObject, DetachedException, InvalidContentTypeException, InvalidDataException from .repository import ( Repository, YumRepository, diff --git a/pubtools/pulplib/_impl/model/common.py b/pubtools/pulplib/_impl/model/common.py index 03406a70..f33699a0 100644 --- a/pubtools/pulplib/_impl/model/common.py +++ b/pubtools/pulplib/_impl/model/common.py @@ -19,6 +19,12 @@ class DetachedException(Exception): """ +class InvalidContentTypeException(Exception): + """Raised if a content type id is provided which appears to be invalid + (i.e. not among :class:`pubtools.pulplib.Unit` subclasses). + """ + + class InvalidDataException(Exception): """Raised if raw Pulp data appears to be invalid (i.e. not matching expected schema). """ diff --git a/pubtools/pulplib/_impl/model/repository/base.py b/pubtools/pulplib/_impl/model/repository/base.py index bfebc80f..4b6c0bdf 100644 --- a/pubtools/pulplib/_impl/model/repository/base.py +++ b/pubtools/pulplib/_impl/model/repository/base.py @@ -4,7 +4,7 @@ from attr import validators from more_executors.futures import f_proxy -from ..common import PulpObject, Deletable, DetachedException +from ..common import PulpObject, Deletable, DetachedException, InvalidContentTypeException from ..attr import pulp_attrib from ..distributor import Distributor from ..frozenlist import FrozenList @@ -190,25 +190,20 @@ def distributor(self, distributor_id): """ return self._distributors_by_id.get(distributor_id) - def search_content(self, name=None, arch=None, filename=None, stream=None): + def search_content(self, type_id, criteria=None): """Search this repository for content matching the given criteria. Args: - name (str) - Name of the desired unit. - arch (str) - Architecture of the desired unit. - filename (str) - File name of the desired unit. - stream (str) - Stream of the desired modulemd or modulemd_defaults unit. + type_id (str) + Type of the desired unit. + criteria (:class:`~pubtools.pulplib.Criteria`) Returns: - Future[list[:class:`~pubtools.pulplib.Task`]] + Future[list[:class:`~pubtools.pulplib.Unit`]] A future which is resolved when search succeeds. - The future contains a list of zero or more tasks triggered and awaited - during the search operation. + The future contains a list of zero or more units returned + by the search operation. Raises: DetachedException @@ -219,27 +214,13 @@ def search_content(self, name=None, arch=None, filename=None, stream=None): if not self._client: raise DetachedException() - criteria = ( - Criteria.with_field( - "type_ids", - Matcher.in_(["rpm", "srpm", "modulemd", "modulemd_defaults"]), - ), - ) - - if name: - criteria = criteria + (Criteria.with_field("unit.name", name),) - if arch: - criteria = criteria + (Criteria.with_field("unit.arch", arch),) - if filename: - criteria = criteria + (Criteria.with_field("unit.filename", filename),) - if stream: - criteria = criteria + (Criteria.with_field("unit.stream", stream),) - - return f_proxy( - self._client._search( - Unit, "repositories/%s" % self.id, criteria=Criteria.and_(*criteria) - ) - ) + if type_id not in [cls.__name__ for cls in Unit.__subclasses__()]: + raise InvalidContentTypeException() + + type_crit = Criteria.with_field("_content_type_ids", Matcher.in_([type_id])) + criteria = (type_crit, criteria) if criteria else (type_crit,) + + return f_proxy(self._client._search(Unit, "repositories/%s" % self.id, criteria=Criteria.and_(*criteria))) def delete(self): """Delete this repository from Pulp. diff --git a/pubtools/pulplib/_impl/model/unit/modulemd_defaults.py b/pubtools/pulplib/_impl/model/unit/modulemd_defaults.py new file mode 100644 index 00000000..464353cb --- /dev/null +++ b/pubtools/pulplib/_impl/model/unit/modulemd_defaults.py @@ -0,0 +1,32 @@ +from .base import Unit, unit_type + +from ..attr import pulp_attrib +from ... import compat_attr as attr + + +@unit_type("modulemd") +@attr.s(kw_only=True, frozen=True) +class ModulemdDefaultsUnit(Unit): + """A :class:`~pubtools.pulplib.Unit` representing a modulemd_defaults document. + + .. versionadded:: 1.5.0 + """ + + name = pulp_attrib(type=str, pulp_field="name") + """The name of this modulemd defaults unit. + + Example: the name of ant:rhel-8-for-aarch64-appstream-htb-rpms is "ant". + """ + + stream = pulp_attrib(type=str, pulp_field="stream") + """The stream of this modulemd defaults unit. + + Example: the stream of ant:rhel-8-for-aarch64-appstream-htb-rpms is "1.10". + """ + + repository_id = pulp_attrib(type=str, pulp_field="repo_id") + """The repository ID bound to this modulemd defaults unit. + + Example: the repo_id of ant:rhel-8-for-aarch64-appstream-htb-rpms + is "rhel-8-for-aarch64-appstream-htb-rpms". + """ diff --git a/pubtools/pulplib/_impl/schema/unit.yaml b/pubtools/pulplib/_impl/schema/unit.yaml index 506f67f3..b1bb4e8a 100644 --- a/pubtools/pulplib/_impl/schema/unit.yaml +++ b/pubtools/pulplib/_impl/schema/unit.yaml @@ -95,9 +95,7 @@ definitions: properties: # Type of the unit _content_type_id: - enum: - - modulemd - - modulemd_defaults + const: modulemd # NSVCA components name: @@ -119,6 +117,33 @@ definitions: - context - arch + # modulemd_defaults units + modulemd_defaults: + type: object + + properties: + # Type of the unit + _content_type_id: + const: modulemd_defaults + + # Name of the unit + name: + type: string + + # Repository associated with the unit + repo_id: + type: string + + # Stream of the unit + stream: + type: string + + required: + - _content_type_id + - name + - stream + - repo_id + # Schema for any unknown type of unit unknown: type: object @@ -135,6 +160,7 @@ definitions: - rpm - srpm - modulemd + - modulemd_defaults required: - _content_type_id @@ -143,4 +169,5 @@ anyOf: - $ref: "#/definitions/iso" - $ref: "#/definitions/rpm" - $ref: "#/definitions/modulemd" +- $ref: "#/definitions/modulemd_defaults" - $ref: "#/definitions/unknown" diff --git a/tests/repository/test_search_content.py b/tests/repository/test_search_content.py index 65924481..88260baa 100644 --- a/tests/repository/test_search_content.py +++ b/tests/repository/test_search_content.py @@ -1,11 +1,11 @@ import pytest -from pubtools.pulplib import Repository, DetachedException +from pubtools.pulplib import Repository, DetachedException, InvalidContentTypeException, Criteria def test_detached(): """search_content raises if called on a detached repo""" with pytest.raises(DetachedException): - Repository(id="some-repo").search_content() + Repository(id="some-repo").search_content(type_id="rpm") class TestSearchContent(object): @@ -52,43 +52,33 @@ def __init__(self, requests_mocker, client): "_content_type_id": "modulemd_defaults", "name": "m2", "stream": "s2", - "version": 1234, - "context": "a1b2c3", - "arch": "s390x", + "repo_id": "some-repo", }, ], ) - def test_search_content(self): - """search_content gets all rpm and modulemd content from the repository""" - units_f = self.repo.search_content() - units = [unit for unit in units_f.result().as_iter()] - - assert len(units) == 4 - assert sorted(units)[0].content_type_id == "modulemd" - assert sorted(units)[1].content_type_id == "modulemd_defaults" - assert sorted(units)[2].content_type_id == "rpm" - assert sorted(units)[3].content_type_id == "srpm" - - def test_search_modulemd_content(self): - """search_content gets only matching content from the repository""" - units_f = self.repo.search_content(arch="s390x", stream="s1") + @pytest.mark.parametrize("type_id", ["rpm", "srpm", "iso", "modulemd", "modulemd_defaults"]) + def test_search_content(self, type_id): + """search_content gets all content from the repository""" + units_f = self.repo.search_content(type_id) units = [unit for unit in units_f.result().as_iter()] assert len(units) == 1 - assert units[0].content_type_id == "modulemd" + assert sorted(units)[0].content_type_id == type_id - def test_search_rpm_content(self): + def test_search_content_with_criteria(self): """search_content gets only matching content from the repository""" - units_f = self.repo.search_content(name="bash", filename="bash-x86_64.rpm") + crit = Criteria.and_( + Criteria.with_field("arch", "s390x"), + Criteria.with_field("stream", "s1") + ) + units_f = self.repo.search_content(type_id="modulemd", criteria=crit) units = [unit for unit in units_f.result().as_iter()] assert len(units) == 1 - assert units[0].content_type_id == "rpm" + assert units[0].name == "m1" - def test_search_no_matched_content(self): + def test_search_content_with_bad_type_id(self): """search_content gets no matching content from the repository""" - units_f = self.repo.search_content(name="hello.txt") - units = [unit for unit in units_f.result().as_iter()] - - assert len(units) == 0 + with pytest.raises(InvalidContentTypeException): + self.repo.search_content(type_id="foo") From 4985db2fe4ec8fc376338145378cc7f760bd7324 Mon Sep 17 00:00:00 2001 From: Nathan Gillett Date: Fri, 15 Nov 2019 11:53:42 -0500 Subject: [PATCH 05/19] Refactor with black --- pubtools/pulplib/_impl/model/__init__.py | 7 ++++++- pubtools/pulplib/_impl/model/repository/base.py | 13 +++++++++++-- tests/repository/test_search_content.py | 14 ++++++++++---- 3 files changed, 27 insertions(+), 7 deletions(-) diff --git a/pubtools/pulplib/_impl/model/__init__.py b/pubtools/pulplib/_impl/model/__init__.py index 949a5328..c1f3400d 100644 --- a/pubtools/pulplib/_impl/model/__init__.py +++ b/pubtools/pulplib/_impl/model/__init__.py @@ -1,4 +1,9 @@ -from .common import PulpObject, DetachedException, InvalidContentTypeException, InvalidDataException +from .common import ( + PulpObject, + DetachedException, + InvalidContentTypeException, + InvalidDataException, +) from .repository import ( Repository, YumRepository, diff --git a/pubtools/pulplib/_impl/model/repository/base.py b/pubtools/pulplib/_impl/model/repository/base.py index 4b6c0bdf..6e5d030e 100644 --- a/pubtools/pulplib/_impl/model/repository/base.py +++ b/pubtools/pulplib/_impl/model/repository/base.py @@ -4,7 +4,12 @@ from attr import validators from more_executors.futures import f_proxy -from ..common import PulpObject, Deletable, DetachedException, InvalidContentTypeException +from ..common import ( + PulpObject, + Deletable, + DetachedException, + InvalidContentTypeException, +) from ..attr import pulp_attrib from ..distributor import Distributor from ..frozenlist import FrozenList @@ -220,7 +225,11 @@ def search_content(self, type_id, criteria=None): type_crit = Criteria.with_field("_content_type_ids", Matcher.in_([type_id])) criteria = (type_crit, criteria) if criteria else (type_crit,) - return f_proxy(self._client._search(Unit, "repositories/%s" % self.id, criteria=Criteria.and_(*criteria))) + return f_proxy( + self._client._search( + Unit, "repositories/%s" % self.id, criteria=Criteria.and_(*criteria) + ) + ) def delete(self): """Delete this repository from Pulp. diff --git a/tests/repository/test_search_content.py b/tests/repository/test_search_content.py index 88260baa..e3716bbb 100644 --- a/tests/repository/test_search_content.py +++ b/tests/repository/test_search_content.py @@ -1,5 +1,10 @@ import pytest -from pubtools.pulplib import Repository, DetachedException, InvalidContentTypeException, Criteria +from pubtools.pulplib import ( + Repository, + DetachedException, + InvalidContentTypeException, + Criteria, +) def test_detached(): @@ -57,7 +62,9 @@ def __init__(self, requests_mocker, client): ], ) - @pytest.mark.parametrize("type_id", ["rpm", "srpm", "iso", "modulemd", "modulemd_defaults"]) + @pytest.mark.parametrize( + "type_id", ["rpm", "srpm", "iso", "modulemd", "modulemd_defaults"] + ) def test_search_content(self, type_id): """search_content gets all content from the repository""" units_f = self.repo.search_content(type_id) @@ -69,8 +76,7 @@ def test_search_content(self, type_id): def test_search_content_with_criteria(self): """search_content gets only matching content from the repository""" crit = Criteria.and_( - Criteria.with_field("arch", "s390x"), - Criteria.with_field("stream", "s1") + Criteria.with_field("arch", "s390x"), Criteria.with_field("stream", "s1") ) units_f = self.repo.search_content(type_id="modulemd", criteria=crit) units = [unit for unit in units_f.result().as_iter()] From 1e05708802774c7df180efa27e8c3c0bb5e75b58 Mon Sep 17 00:00:00 2001 From: Nathan Gillett Date: Tue, 19 Nov 2019 17:00:05 -0500 Subject: [PATCH 06/19] Refactoring, add test cases --- examples/repository-content | 51 +++++ pubtools/pulplib/__init__.py | 1 + pubtools/pulplib/_impl/client/client.py | 51 ++++- pubtools/pulplib/_impl/fake/client.py | 40 ++++ pubtools/pulplib/_impl/model/__init__.py | 2 +- .../pulplib/_impl/model/repository/base.py | 56 +++-- pubtools/pulplib/_impl/model/unit/__init__.py | 3 + .../_impl/model/unit/modulemd_defaults.py | 21 +- pubtools/pulplib/_impl/schema/unit.yaml | 7 +- tests/client/test_client.py | 49 ++++ tests/fake/test_fake_search.py | 46 +++- tests/repository/test_search_content.py | 214 +++++++++++------- 12 files changed, 411 insertions(+), 130 deletions(-) create mode 100644 examples/repository-content diff --git a/examples/repository-content b/examples/repository-content new file mode 100644 index 00000000..5a8744ce --- /dev/null +++ b/examples/repository-content @@ -0,0 +1,51 @@ +#!/usr/bin/env python +import os +import logging +from argparse import ArgumentParser + +from pubtools.pulplib import Client, Criteria + +log = logging.getLogger("repository-content") + + +def make_client(args): + auth = None + + if args.username: + password = args.password + if password is None: + password = os.environ.get("PULP_PASSWORD") + if not password: + log.warning("No password provided for %s", args.username) + auth = (args.username, args.password) + + return Client(args.url, auth=auth, verify=not args.insecure) + + +def main(): + log.setLevel(logging.INFO) + logging.basicConfig(format="%(message)s", level=logging.INFO) + + parser = ArgumentParser(description="Retrieve unit objects of the specified type from a repository") + parser.add_argument("--url", help="Pulp server URL", required=True) + parser.add_argument("--username", help="Pulp username") + parser.add_argument("--password", help="Pulp password (or set PULP_PASSWORD in env)") + parser.add_argument("--repo-id", action="store", required=True) + parser.add_argument("--content-type", action="store", required=True) + parser.add_argument("--debug", action="store_true") + parser.add_argument("--insecure", default=False, action="store_true") + + p = parser.parse_args() + + if p.debug: + logging.getLogger("pubtools.pulplib").setLevel(logging.DEBUG) + log.setLevel(logging.DEBUG) + + client = make_client(p) + repo = client.get_repository(p.repo_id) + criteria = Criteria.with_field("_content_type_id", p.content_type) + return repo.search_content(criteria) + + +if __name__ == "__main__": + main() diff --git a/pubtools/pulplib/__init__.py b/pubtools/pulplib/__init__.py index 5fbf852c..84959ba9 100644 --- a/pubtools/pulplib/__init__.py +++ b/pubtools/pulplib/__init__.py @@ -14,6 +14,7 @@ FileUnit, RpmUnit, ModulemdUnit, + ModulemdDefaultsUnit, Distributor, PublishOptions, Task, diff --git a/pubtools/pulplib/_impl/client/client.py b/pubtools/pulplib/_impl/client/client.py index a4bb0bde..c52973c6 100644 --- a/pubtools/pulplib/_impl/client/client.py +++ b/pubtools/pulplib/_impl/client/client.py @@ -12,8 +12,10 @@ from more_executors.futures import f_map, f_flat_map, f_return, f_proxy from ..page import Page -from ..criteria import Criteria +from ..criteria import Criteria, AndCriteria, FieldMatchCriteria from ..model import Repository, MaintenanceReport, Distributor, Unit +from ..model.common import InvalidContentTypeException +from ..model.unit import SUPPORTED_UNIT_TYPES from .search import filters_for_criteria from .errors import PulpException from .poller import TaskPoller @@ -203,6 +205,49 @@ def search_repository(self, criteria=None): Repository, "repositories", criteria=criteria, search_options=search_options ) + def search_repository_content(self, repository_id, criteria=None): + """Search the given repository for content matching the given criteria. + + Args: + repository_id + The ID of the repository in which 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 + """ + + def get_type_id(crit): + if ( + isinstance(crit, FieldMatchCriteria) + and crit._field is "_content_type_id" + ): + return crit._matcher._value + + type_id = None + if isinstance(criteria, AndCriteria): + for c in criteria._operands: + type_id = get_type_id(c) + if type_id: + break + else: + type_id = get_type_id(criteria) + + if type_id not in SUPPORTED_UNIT_TYPES: + raise InvalidContentTypeException() + + search_f = self._search(Unit, "repositories/%s" % repository_id, criteria) + return [unit for unit in search_f.result().as_iter()] + def search_distributor(self, criteria=None): """Search the distributors matching the given criteria. @@ -428,6 +473,10 @@ def _log_spawned_tasks(cls, taskdata): def _handle_page(self, url, object_class, search, raw_data): LOG.debug("Got pulp response for %s, %s elems", search, len(raw_data)) + # Extract metadata from Pulp units + if object_class is Unit and url.endswith("units/"): + raw_data = [elem["metadata"] for elem in raw_data] + page_data = [object_class.from_data(elem) for elem in raw_data] for obj in page_data: # Set client is only applicable for repository and distributor objects diff --git a/pubtools/pulplib/_impl/fake/client.py b/pubtools/pulplib/_impl/fake/client.py index 8a706e21..3243a294 100644 --- a/pubtools/pulplib/_impl/fake/client.py +++ b/pubtools/pulplib/_impl/fake/client.py @@ -15,13 +15,17 @@ from pubtools.pulplib import ( Page, PulpException, + InvalidContentTypeException, Criteria, Task, Repository, Distributor, + Unit, MaintenanceReport, ) +from pubtools.pulplib._impl.criteria import FieldMatchCriteria, AndCriteria from pubtools.pulplib._impl.client.search import filters_for_criteria +from ..model.unit import SUPPORTED_UNIT_TYPES from .. import compat_attr as attr from .match import match_object @@ -90,6 +94,42 @@ def search_repository(self, criteria=None): random.shuffle(repos) return self._prepare_pages(repos) + def search_repository_content(self, repository_id, criteria=None): + units = [] + + def get_type_id(crit): + if ( + isinstance(crit, FieldMatchCriteria) + and crit._field is "_content_type_id" + ): + return crit._matcher._value + + type_id = None + if isinstance(criteria, AndCriteria): + for c in criteria._operands: + type_id = get_type_id(c) + if type_id: + break + else: + type_id = get_type_id(criteria) + + if type_id not in SUPPORTED_UNIT_TYPES: + raise InvalidContentTypeException() + + print(criteria) + filters_for_criteria(criteria, Unit) + + try: + for unit in self._repo_units[repository_id]: + print(unit) + if 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/__init__.py b/pubtools/pulplib/_impl/model/__init__.py index c1f3400d..2a365ab3 100644 --- a/pubtools/pulplib/_impl/model/__init__.py +++ b/pubtools/pulplib/_impl/model/__init__.py @@ -11,7 +11,7 @@ ContainerImageRepository, PublishOptions, ) -from .unit import Unit, FileUnit, RpmUnit, ModulemdUnit +from .unit import Unit, FileUnit, RpmUnit, ModulemdUnit, ModulemdDefaultsUnit from .task import Task from .distributor import Distributor from .maintenance import MaintenanceReport, MaintenanceEntry diff --git a/pubtools/pulplib/_impl/model/repository/base.py b/pubtools/pulplib/_impl/model/repository/base.py index 6e5d030e..57c1f00b 100644 --- a/pubtools/pulplib/_impl/model/repository/base.py +++ b/pubtools/pulplib/_impl/model/repository/base.py @@ -15,8 +15,8 @@ from ..frozenlist import FrozenList from ...schema import load_schema from ... import compat_attr as attr -from ...criteria import Criteria, Matcher -from ..unit.base import Unit +from ...criteria import Criteria, FieldMatchCriteria, AndCriteria, Matcher +from ..unit import Unit, SUPPORTED_UNIT_TYPES LOG = logging.getLogger("pubtools.pulplib") @@ -195,41 +195,39 @@ def distributor(self, distributor_id): """ return self._distributors_by_id.get(distributor_id) - def search_content(self, type_id, criteria=None): - """Search this repository for content matching the given criteria. + @property + def rpm_content(self): + """A list of rpm units stored in this repository""" + return self._search_content(Criteria.with_field("_content_type_id", "rpm")) - Args: - type_id (str) - Type of the desired unit. - criteria (:class:`~pubtools.pulplib.Criteria`) + @property + def srpm_content(self): + """A list of srpm units stored in this repository""" + return self._search_content(Criteria.with_field("_content_type_id", "srpm")) - Returns: - Future[list[:class:`~pubtools.pulplib.Unit`]] - A future which is resolved when search succeeds. + @property + def iso_content(self): + """A list of iso units stored in this repository""" + return self._search_content(Criteria.with_field("_content_type_id", "iso")) - The future contains a list of zero or more units returned - by the search operation. + @property + def modulemd_content(self): + """A list of modulemd units stored in this repository""" + return self._search_content(Criteria.with_field("_content_type_id", "modulemd")) - Raises: - DetachedException - If this instance is not attached to a Pulp client. + @property + def modulemd_defaults_content(self): + """A list of modulemd_defaults units stored in this repository""" + return self._search_content( + Criteria.with_field("_content_type_id", "modulemd_defaults") + ) - .. versionadded:: 2.4.0 - """ + def _search_content(self, criteria=None): + # Search this repository for content matching the given criteria if not self._client: raise DetachedException() - if type_id not in [cls.__name__ for cls in Unit.__subclasses__()]: - raise InvalidContentTypeException() - - type_crit = Criteria.with_field("_content_type_ids", Matcher.in_([type_id])) - criteria = (type_crit, criteria) if criteria else (type_crit,) - - return f_proxy( - self._client._search( - Unit, "repositories/%s" % self.id, criteria=Criteria.and_(*criteria) - ) - ) + return self._client.search_repository_content(self.id, criteria) def delete(self): """Delete this repository from Pulp. diff --git a/pubtools/pulplib/_impl/model/unit/__init__.py b/pubtools/pulplib/_impl/model/unit/__init__.py index 2ce4fa26..adbf66b4 100644 --- a/pubtools/pulplib/_impl/model/unit/__init__.py +++ b/pubtools/pulplib/_impl/model/unit/__init__.py @@ -2,3 +2,6 @@ from .file import FileUnit from .rpm import RpmUnit from .modulemd import ModulemdUnit +from .modulemd_defaults import ModulemdDefaultsUnit + +SUPPORTED_UNIT_TYPES = ("iso", "rpm", "srpm", "modulemd", "modulemd_defaults") diff --git a/pubtools/pulplib/_impl/model/unit/modulemd_defaults.py b/pubtools/pulplib/_impl/model/unit/modulemd_defaults.py index 464353cb..df274241 100644 --- a/pubtools/pulplib/_impl/model/unit/modulemd_defaults.py +++ b/pubtools/pulplib/_impl/model/unit/modulemd_defaults.py @@ -4,29 +4,22 @@ from ... import compat_attr as attr -@unit_type("modulemd") +@unit_type("modulemd_defaults") @attr.s(kw_only=True, frozen=True) class ModulemdDefaultsUnit(Unit): """A :class:`~pubtools.pulplib.Unit` representing a modulemd_defaults document. - .. versionadded:: 1.5.0 + .. versionadded:: 2.4.0 """ name = pulp_attrib(type=str, pulp_field="name") - """The name of this modulemd defaults unit. - - Example: the name of ant:rhel-8-for-aarch64-appstream-htb-rpms is "ant". - """ + """The name of this modulemd defaults unit""" stream = pulp_attrib(type=str, pulp_field="stream") - """The stream of this modulemd defaults unit. - - Example: the stream of ant:rhel-8-for-aarch64-appstream-htb-rpms is "1.10". - """ + """The stream of this modulemd defaults unit""" repository_id = pulp_attrib(type=str, pulp_field="repo_id") - """The repository ID bound to this modulemd defaults unit. + """The repository ID bound to this modulemd defaults unit""" - Example: the repo_id of ant:rhel-8-for-aarch64-appstream-htb-rpms - is "rhel-8-for-aarch64-appstream-htb-rpms". - """ + profiles = pulp_attrib(pulp_field="profiles") + """The profiles of this modulemd defaults unit""" diff --git a/pubtools/pulplib/_impl/schema/unit.yaml b/pubtools/pulplib/_impl/schema/unit.yaml index b1bb4e8a..5e5bdce5 100644 --- a/pubtools/pulplib/_impl/schema/unit.yaml +++ b/pubtools/pulplib/_impl/schema/unit.yaml @@ -126,17 +126,14 @@ definitions: _content_type_id: const: modulemd_defaults - # Name of the unit name: type: string - - # Repository associated with the unit repo_id: type: string - - # Stream of the unit stream: type: string + profiles: + type: object required: - _content_type_id diff --git a/tests/client/test_client.py b/tests/client/test_client.py index c17444cc..c78da34b 100644 --- a/tests/client/test_client.py +++ b/tests/client/test_client.py @@ -13,7 +13,9 @@ Criteria, Matcher, Repository, + ModulemdDefaultsUnit, PulpException, + InvalidContentTypeException, MaintenanceReport, Task, Distributor, @@ -58,6 +60,53 @@ 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"]}, + } + }, + { + "metadata": { + "_content_type_id": "iso", + "name": "hello.txt", + "size": 23, + "checksum": "a" * 64, + } + }, + ], + ) + + crit = Criteria.with_field("_content_type_id", "modulemd_defaults") + units = client.search_repository_content("some-repo", crit) + + # It should have returned one ModulemdDefaultsUnit + assert units == [ + ModulemdDefaultsUnit( + content_type_id="modulemd_defaults", + name="mdd", + repository_id="mdd-repo", + stream="1.0", + profiles={"p1": ["something"]}, + ) + ] + + +def test_cannot_search_repository_content_without_content_type(client): + """search_content raises if called without criteria containing _content_type_id""" + + with pytest.raises(InvalidContentTypeException): + client.search_repository_content("some-repo") + + def test_can_search_distributor(client, requests_mocker): """search_distributor issues distributors/search POST request as expected.""" requests_mocker.post( diff --git a/tests/fake/test_fake_search.py b/tests/fake/test_fake_search.py index 55beeb99..c373023d 100644 --- a/tests/fake/test_fake_search.py +++ b/tests/fake/test_fake_search.py @@ -2,7 +2,14 @@ import pytest -from pubtools.pulplib import FakeController, Repository, Criteria, Matcher, Distributor +from pubtools.pulplib import ( + FakeController, + Repository, + Unit, + Criteria, + Matcher, + Distributor, +) def test_can_search_id(): @@ -346,6 +353,43 @@ def test_search_paginates(): assert sorted(found_repos) == sorted(repos) +def test_search_repository_content(): + 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) + + client = controller.client + crit = Criteria.with_field("_content_type_id", "rpm") + + found = client.search_repository_content("repo1", crit) + + assert sorted(found) == [units[1]] + + def test_search_distributor(): controller = FakeController() diff --git a/tests/repository/test_search_content.py b/tests/repository/test_search_content.py index e3716bbb..c9d687ba 100644 --- a/tests/repository/test_search_content.py +++ b/tests/repository/test_search_content.py @@ -1,90 +1,146 @@ import pytest from pubtools.pulplib import ( Repository, + FileUnit, + RpmUnit, + ModulemdUnit, + ModulemdDefaultsUnit, DetachedException, InvalidContentTypeException, - Criteria, ) +FAKE_UNIT_SEARCH = [ + { + "metadata": { + "_content_type_id": "iso", + "name": "hello.txt", + "size": 23, + "checksum": "a" * 64, + } + }, + { + "metadata": { + "_content_type_id": "srpm", + "name": "bash", + "epoch": "0", + "filename": "bash-x86_64.srpm", + "version": "4.0", + "release": "1", + "arch": "x86_64", + } + }, + { + "metadata": { + "_content_type_id": "rpm", + "name": "bash", + "epoch": "0", + "filename": "bash-x86_64.rpm", + "version": "4.0", + "release": "1", + "arch": "x86_64", + } + }, + { + "metadata": { + "_content_type_id": "modulemd", + "name": "md", + "stream": "s1", + "version": 1234, + "context": "a1b2c3", + "arch": "s390x", + } + }, + { + "metadata": { + "_content_type_id": "modulemd_defaults", + "name": "mdd", + "stream": "1.0", + "repo_id": "some-repo", + "profiles": {"p1": ["something"]}, + } + }, +] + def test_detached(): - """search_content raises if called on a detached repo""" + """content searches raise if called on a detached repository object""" with pytest.raises(DetachedException): - Repository(id="some-repo").search_content(type_id="rpm") - - -class TestSearchContent(object): - def __init__(self, requests_mocker, client): - self.repo = Repository(id="some-repo") - self.repo.__dict__["_client"] = client - - requests_mocker.post( - "https://pulp.example.com/pulp/api/v2/repositories/some-repo/search/units/", - json=[ - { - "_content_type_id": "iso", - "name": "hello.txt", - "size": 23, - "checksum": "a" * 64, - }, - { - "_content_type_id": "srpm", - "name": "bash", - "epoch": "0", - "filename": "bash-x86_64.srpm", - "version": "4.0", - "release": "1", - "arch": "x86_64", - }, - { - "_content_type_id": "rpm", - "name": "bash", - "epoch": "0", - "filename": "bash-x86_64.rpm", - "version": "4.0", - "release": "1", - "arch": "x86_64", - }, - { - "_content_type_id": "modulemd", - "name": "m1", - "stream": "s1", - "version": 1234, - "context": "a1b2c3", - "arch": "s390x", - }, - { - "_content_type_id": "modulemd_defaults", - "name": "m2", - "stream": "s2", - "repo_id": "some-repo", - }, - ], - ) - - @pytest.mark.parametrize( - "type_id", ["rpm", "srpm", "iso", "modulemd", "modulemd_defaults"] + assert not Repository(id="some-repo").iso_content + + +def test_search_content_without_content_type(client, requests_mocker): + """search_content raises if called without criteria containing _content_type_id""" + repo = Repository(id="some-repo") + repo.__dict__["_client"] = client + requests_mocker.post( + "https://pulp.example.com/pulp/api/v2/repositories/some-repo/search/units/", + json=FAKE_UNIT_SEARCH, + ) + + with pytest.raises(InvalidContentTypeException): + assert not repo._search_content() + + +def test_iso_content(client, requests_mocker): + """iso_content returns correct units from the repository""" + repo = Repository(id="some-repo") + repo.__dict__["_client"] = client + requests_mocker.post( + "https://pulp.example.com/pulp/api/v2/repositories/some-repo/search/units/", + json=FAKE_UNIT_SEARCH, + ) + + assert len(repo.iso_content) == 1 + assert repo.iso_content[0].content_type_id == "iso" + + +def test_rpm_content(client, requests_mocker): + """rpm_content returns correct units from the repository""" + repo = Repository(id="some-repo") + repo.__dict__["_client"] = client + requests_mocker.post( + "https://pulp.example.com/pulp/api/v2/repositories/some-repo/search/units/", + json=FAKE_UNIT_SEARCH, + ) + + assert len(repo.rpm_content) == 1 + assert repo.rpm_content[0].content_type_id == "rpm" + + +def test_srpm_content(client, requests_mocker): + """srpm_content returns correct units from the repository""" + repo = Repository(id="some-repo") + repo.__dict__["_client"] = client + requests_mocker.post( + "https://pulp.example.com/pulp/api/v2/repositories/some-repo/search/units/", + json=FAKE_UNIT_SEARCH, + ) + + assert len(repo.srpm_content) == 1 + assert repo.srpm_content[0].content_type_id == "srpm" + + +def test_modulemd_content(client, requests_mocker): + """modulemd_content returns correct units from the repository""" + repo = Repository(id="some-repo") + repo.__dict__["_client"] = client + requests_mocker.post( + "https://pulp.example.com/pulp/api/v2/repositories/some-repo/search/units/", + json=FAKE_UNIT_SEARCH, + ) + + assert len(repo.modulemd_content) == 1 + assert repo.modulemd_content[0].content_type_id == "modulemd" + + +def test_modulemd_defaults_content(client, requests_mocker): + """modulemd_defaults_content returns correct units from the repository""" + repo = Repository(id="some-repo") + repo.__dict__["_client"] = client + requests_mocker.post( + "https://pulp.example.com/pulp/api/v2/repositories/some-repo/search/units/", + json=FAKE_UNIT_SEARCH, ) - def test_search_content(self, type_id): - """search_content gets all content from the repository""" - units_f = self.repo.search_content(type_id) - units = [unit for unit in units_f.result().as_iter()] - - assert len(units) == 1 - assert sorted(units)[0].content_type_id == type_id - - def test_search_content_with_criteria(self): - """search_content gets only matching content from the repository""" - crit = Criteria.and_( - Criteria.with_field("arch", "s390x"), Criteria.with_field("stream", "s1") - ) - units_f = self.repo.search_content(type_id="modulemd", criteria=crit) - units = [unit for unit in units_f.result().as_iter()] - - assert len(units) == 1 - assert units[0].name == "m1" - - def test_search_content_with_bad_type_id(self): - """search_content gets no matching content from the repository""" - with pytest.raises(InvalidContentTypeException): - self.repo.search_content(type_id="foo") + + assert len(repo.modulemd_defaults_content) == 1 + assert repo.modulemd_defaults_content[0].content_type_id == "modulemd_defaults" From 5ea6424a7f06067287147492df68fc50394a7404 Mon Sep 17 00:00:00 2001 From: Nathan Gillett Date: Wed, 20 Nov 2019 10:49:32 -0500 Subject: [PATCH 07/19] Correct criteria for unit searches --- ...tory-content => repository-content-search} | 9 +-- pubtools/pulplib/_impl/client/client.py | 66 ++++++++++++------- .../pulplib/_impl/model/repository/base.py | 10 +-- tests/client/test_client.py | 3 +- tests/repository/test_search_content.py | 4 -- 5 files changed, 54 insertions(+), 38 deletions(-) rename examples/{repository-content => repository-content-search} (86%) diff --git a/examples/repository-content b/examples/repository-content-search similarity index 86% rename from examples/repository-content rename to examples/repository-content-search index 5a8744ce..fc127747 100644 --- a/examples/repository-content +++ b/examples/repository-content-search @@ -5,7 +5,7 @@ from argparse import ArgumentParser from pubtools.pulplib import Client, Criteria -log = logging.getLogger("repository-content") +log = logging.getLogger("repository-content-search") def make_client(args): @@ -42,9 +42,10 @@ def main(): log.setLevel(logging.DEBUG) client = make_client(p) - repo = client.get_repository(p.repo_id) - criteria = Criteria.with_field("_content_type_id", p.content_type) - return repo.search_content(criteria) + criteria = Criteria.with_field("type_ids", [p.content_type]) + units = client.search_repository_content(p.repo_id, criteria) + print(units) + return units if __name__ == "__main__": diff --git a/pubtools/pulplib/_impl/client/client.py b/pubtools/pulplib/_impl/client/client.py index c52973c6..3f716b54 100644 --- a/pubtools/pulplib/_impl/client/client.py +++ b/pubtools/pulplib/_impl/client/client.py @@ -12,7 +12,7 @@ from more_executors.futures import f_map, f_flat_map, f_return, f_proxy from ..page import Page -from ..criteria import Criteria, AndCriteria, FieldMatchCriteria +from ..criteria import Criteria, AndCriteria, OrCriteria, FieldMatchCriteria from ..model import Repository, MaintenanceReport, Distributor, Unit from ..model.common import InvalidContentTypeException from ..model.unit import SUPPORTED_UNIT_TYPES @@ -205,6 +205,39 @@ def search_repository(self, criteria=None): Repository, "repositories", criteria=criteria, search_options=search_options ) + @staticmethod + def _filter_type_id_criteria(criteria): + type_ids = [] + filtered_criteria = None + + def extract_type_ids(crit): + if isinstance(crit, FieldMatchCriteria) and crit._field is "type_ids": + for type_id in crit._matcher._value: + if type_id in SUPPORTED_UNIT_TYPES: + type_ids.append(type_id) + + def criteria_set(crit): + other_crit = () + for c in crit._operands: + extract_type_ids(c) + if type_ids: + continue + else: + other_crit += (c,) + return other_crit + + if isinstance(criteria, AndCriteria): + filtered_criteria = Criteria.and_(*criteria_set(criteria)) + elif isinstance(criteria, OrCriteria): + filtered_criteria = Criteria.or_(*criteria_set(criteria)) + else: + extract_type_ids(criteria) + + if not type_ids: + raise InvalidContentTypeException() + + return type_ids, filtered_criteria + def search_repository_content(self, repository_id, criteria=None): """Search the given repository for content matching the given criteria. @@ -225,27 +258,14 @@ def search_repository_content(self, repository_id, criteria=None): .. versionadded:: 2.4.0 """ + # We have to extract criteria with type_ids, as they are not + # placed in "filters" with other criteria. + type_ids, criteria = self._filter_type_id_criteria(criteria) + search_options = {"type_ids": type_ids} - def get_type_id(crit): - if ( - isinstance(crit, FieldMatchCriteria) - and crit._field is "_content_type_id" - ): - return crit._matcher._value - - type_id = None - if isinstance(criteria, AndCriteria): - for c in criteria._operands: - type_id = get_type_id(c) - if type_id: - break - else: - type_id = get_type_id(criteria) - - if type_id not in SUPPORTED_UNIT_TYPES: - raise InvalidContentTypeException() - - search_f = self._search(Unit, "repositories/%s" % repository_id, criteria) + search_f = self._search( + Unit, "repositories/%s" % repository_id, criteria=criteria, search_options=search_options + ) return [unit for unit in search_f.result().as_iter()] def search_distributor(self, criteria=None): @@ -278,7 +298,7 @@ def _search(self, return_type, resource_type, criteria=None, search_options=None url = os.path.join(self._url, "pulp/api/v2/%s/search/" % resource_type) - if return_type is Unit and "repositories/" in resource_type: + if "type_ids" in search: url = os.path.join(url, "units/") response_f = self._do_search(url, search) @@ -479,7 +499,7 @@ def _handle_page(self, url, object_class, search, raw_data): page_data = [object_class.from_data(elem) for elem in raw_data] for obj in page_data: - # Set client is only applicable for repository and distributor objects + # set_client is only applicable for repository and distributor objects if hasattr(obj, "_set_client"): obj._set_client(self) diff --git a/pubtools/pulplib/_impl/model/repository/base.py b/pubtools/pulplib/_impl/model/repository/base.py index 57c1f00b..2a50188a 100644 --- a/pubtools/pulplib/_impl/model/repository/base.py +++ b/pubtools/pulplib/_impl/model/repository/base.py @@ -198,28 +198,28 @@ def distributor(self, distributor_id): @property def rpm_content(self): """A list of rpm units stored in this repository""" - return self._search_content(Criteria.with_field("_content_type_id", "rpm")) + return self._search_content(Criteria.with_field("type_ids", ["rpm"])) @property def srpm_content(self): """A list of srpm units stored in this repository""" - return self._search_content(Criteria.with_field("_content_type_id", "srpm")) + return self._search_content(Criteria.with_field("type_ids", ["srpm"])) @property def iso_content(self): """A list of iso units stored in this repository""" - return self._search_content(Criteria.with_field("_content_type_id", "iso")) + return self._search_content(Criteria.with_field("type_ids", ["iso"])) @property def modulemd_content(self): """A list of modulemd units stored in this repository""" - return self._search_content(Criteria.with_field("_content_type_id", "modulemd")) + return self._search_content(Criteria.with_field("type_ids", ["modulemd"])) @property def modulemd_defaults_content(self): """A list of modulemd_defaults units stored in this repository""" return self._search_content( - Criteria.with_field("_content_type_id", "modulemd_defaults") + Criteria.with_field("type_ids", ["modulemd_defaults"]) ) def _search_content(self, criteria=None): diff --git a/tests/client/test_client.py b/tests/client/test_client.py index c78da34b..57f6f3ef 100644 --- a/tests/client/test_client.py +++ b/tests/client/test_client.py @@ -2,7 +2,6 @@ import datetime import json import pytest -import requests_mock from mock import patch @@ -85,7 +84,7 @@ def test_can_search_repository_content(client, requests_mocker): ], ) - crit = Criteria.with_field("_content_type_id", "modulemd_defaults") + crit = Criteria.with_field("type_ids", ["modulemd_defaults"]) units = client.search_repository_content("some-repo", crit) # It should have returned one ModulemdDefaultsUnit diff --git a/tests/repository/test_search_content.py b/tests/repository/test_search_content.py index c9d687ba..f309134c 100644 --- a/tests/repository/test_search_content.py +++ b/tests/repository/test_search_content.py @@ -1,10 +1,6 @@ import pytest from pubtools.pulplib import ( Repository, - FileUnit, - RpmUnit, - ModulemdUnit, - ModulemdDefaultsUnit, DetachedException, InvalidContentTypeException, ) From d3532b09c8cc24a5a313f4eeddad77ca752558b9 Mon Sep 17 00:00:00 2001 From: Nathan Gillett Date: Wed, 20 Nov 2019 14:13:34 -0500 Subject: [PATCH 08/19] Deprivitize repository's search_content --- examples/repository-content-search | 1 - .../pulplib/_impl/model/repository/base.py | 70 +++++++++++++++---- .../_impl/model/unit/modulemd_defaults.py | 4 +- pubtools/pulplib/_impl/schema/unit.yaml | 1 + tests/client/test_client.py | 1 - tests/repository/test_search_content.py | 2 +- 6 files changed, 61 insertions(+), 18 deletions(-) diff --git a/examples/repository-content-search b/examples/repository-content-search index fc127747..bf9d31c7 100644 --- a/examples/repository-content-search +++ b/examples/repository-content-search @@ -44,7 +44,6 @@ def main(): client = make_client(p) criteria = Criteria.with_field("type_ids", [p.content_type]) units = client.search_repository_content(p.repo_id, criteria) - print(units) return units diff --git a/pubtools/pulplib/_impl/model/repository/base.py b/pubtools/pulplib/_impl/model/repository/base.py index 2a50188a..5bf12e2b 100644 --- a/pubtools/pulplib/_impl/model/repository/base.py +++ b/pubtools/pulplib/_impl/model/repository/base.py @@ -195,35 +195,77 @@ def distributor(self, distributor_id): """ return self._distributors_by_id.get(distributor_id) + @property + def iso_content(self): + """A list of iso units stored in this repository. + + Returns: + list[:class:`~pubtools.pulplib.FileUnit`] + + .. versionadded:: 2.4.0 + """ + return self.search_content(Criteria.with_field("type_ids", ["iso"])) + @property def rpm_content(self): - """A list of rpm units stored in this repository""" - return self._search_content(Criteria.with_field("type_ids", ["rpm"])) + """A list of rpm units stored in this repository. + + Returns: + list[:class:`~pubtools.pulplib.RpmUnit`] + + .. versionadded:: 2.4.0 + """ + return self.search_content(Criteria.with_field("type_ids", ["rpm"])) @property def srpm_content(self): - """A list of srpm units stored in this repository""" - return self._search_content(Criteria.with_field("type_ids", ["srpm"])) + """A list of srpm units stored in this repository. - @property - def iso_content(self): - """A list of iso units stored in this repository""" - return self._search_content(Criteria.with_field("type_ids", ["iso"])) + Returns: + list[:class:`~pubtools.pulplib.Unit`] + + .. versionadded:: 2.4.0 + """ + return self.search_content(Criteria.with_field("type_ids", ["srpm"])) @property def modulemd_content(self): - """A list of modulemd units stored in this repository""" - return self._search_content(Criteria.with_field("type_ids", ["modulemd"])) + """A list of modulemd units stored in this repository. + + Returns: + list[:class:`~pubtools.pulplib.ModulemdUnit`] + + .. versionadded:: 2.4.0 + """ + return self.search_content(Criteria.with_field("type_ids", ["modulemd"])) @property def modulemd_defaults_content(self): - """A list of modulemd_defaults units stored in this repository""" - return self._search_content( + """A list of modulemd_defaults units stored in this repository. + + Returns: + list[:class:`~pubtools.pulplib.ModulemdDefaultsUnit`] + + .. versionadded:: 2.4.0 + """ + return self.search_content( Criteria.with_field("type_ids", ["modulemd_defaults"]) ) - def _search_content(self, criteria=None): - # Search this repository for content matching the given criteria + def search_content(self, criteria=None): + """Search this repository for content matching the given criteria. + + Args: + 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. + + .. versionadded:: 2.4.0 + """ if not self._client: raise DetachedException() diff --git a/pubtools/pulplib/_impl/model/unit/modulemd_defaults.py b/pubtools/pulplib/_impl/model/unit/modulemd_defaults.py index df274241..3177ec28 100644 --- a/pubtools/pulplib/_impl/model/unit/modulemd_defaults.py +++ b/pubtools/pulplib/_impl/model/unit/modulemd_defaults.py @@ -22,4 +22,6 @@ class ModulemdDefaultsUnit(Unit): """The repository ID bound to this modulemd defaults unit""" profiles = pulp_attrib(pulp_field="profiles") - """The profiles of this modulemd defaults unit""" + """The profiles of this modulemd defaults unit + The type for this attribute is omitted to allow for either dict or None. + """ diff --git a/pubtools/pulplib/_impl/schema/unit.yaml b/pubtools/pulplib/_impl/schema/unit.yaml index 5e5bdce5..638b7306 100644 --- a/pubtools/pulplib/_impl/schema/unit.yaml +++ b/pubtools/pulplib/_impl/schema/unit.yaml @@ -134,6 +134,7 @@ definitions: type: string profiles: type: object + nullable: true required: - _content_type_id diff --git a/tests/client/test_client.py b/tests/client/test_client.py index 57f6f3ef..1bd4b0c7 100644 --- a/tests/client/test_client.py +++ b/tests/client/test_client.py @@ -101,7 +101,6 @@ def test_can_search_repository_content(client, requests_mocker): def test_cannot_search_repository_content_without_content_type(client): """search_content raises if called without criteria containing _content_type_id""" - with pytest.raises(InvalidContentTypeException): client.search_repository_content("some-repo") diff --git a/tests/repository/test_search_content.py b/tests/repository/test_search_content.py index f309134c..93677fe4 100644 --- a/tests/repository/test_search_content.py +++ b/tests/repository/test_search_content.py @@ -74,7 +74,7 @@ def test_search_content_without_content_type(client, requests_mocker): ) with pytest.raises(InvalidContentTypeException): - assert not repo._search_content() + assert not repo.search_content() def test_iso_content(client, requests_mocker): From ce47032f7c088ab953b8a971fb53b924fdfe3c44 Mon Sep 17 00:00:00 2001 From: Nathan Gillett Date: Fri, 22 Nov 2019 13:29:09 -0500 Subject: [PATCH 09/19] Fix broken search criteria JSON and refactor as necessary --- docs/api/model.rst | 2 + ...ory-content-search => search-repo-content} | 20 +- pubtools/pulplib/__init__.py | 1 - pubtools/pulplib/_impl/client/client.py | 67 ++----- pubtools/pulplib/_impl/client/search.py | 35 ++++ pubtools/pulplib/_impl/fake/client.py | 33 +--- pubtools/pulplib/_impl/model/__init__.py | 1 - pubtools/pulplib/_impl/model/common.py | 6 - .../pulplib/_impl/model/repository/base.py | 21 +-- .../_impl/model/unit/modulemd_defaults.py | 3 +- pubtools/pulplib/_impl/schema/unit.yaml | 1 - tests/client/test_client.py | 28 +-- tests/client/test_search.py | 28 ++- tests/fake/test_fake_search.py | 3 +- tests/repository/test_search_content.py | 175 +++++++++--------- 15 files changed, 208 insertions(+), 216 deletions(-) rename examples/{repository-content-search => search-repo-content} (71%) diff --git a/docs/api/model.rst b/docs/api/model.rst index 09cfa87b..27782aea 100644 --- a/docs/api/model.rst +++ b/docs/api/model.rst @@ -49,6 +49,8 @@ Units .. autoclass:: pubtools.pulplib.ModulemdUnit() :members: +.. autoclass:: pubtools.pulplib.ModulemdDefaultUnit() + :members: Task ---- diff --git a/examples/repository-content-search b/examples/search-repo-content similarity index 71% rename from examples/repository-content-search rename to examples/search-repo-content index bf9d31c7..132157a9 100644 --- a/examples/repository-content-search +++ b/examples/search-repo-content @@ -3,9 +3,9 @@ import os import logging from argparse import ArgumentParser -from pubtools.pulplib import Client, Criteria +from pubtools.pulplib import Client -log = logging.getLogger("repository-content-search") +log = logging.getLogger("search-repo-content") def make_client(args): @@ -26,7 +26,9 @@ def main(): log.setLevel(logging.INFO) logging.basicConfig(format="%(message)s", level=logging.INFO) - parser = ArgumentParser(description="Retrieve unit objects of the specified type from a repository") + parser = ArgumentParser( + description="Retrieve unit objects of the given type from the given repository" + ) parser.add_argument("--url", help="Pulp server URL", required=True) parser.add_argument("--username", help="Pulp username") parser.add_argument("--password", help="Pulp password (or set PULP_PASSWORD in env)") @@ -42,8 +44,16 @@ def main(): log.setLevel(logging.DEBUG) client = make_client(p) - criteria = Criteria.with_field("type_ids", [p.content_type]) - units = client.search_repository_content(p.repo_id, criteria) + units = client.search_repository_content(p.repo_id, type_ids=p.content_type) + + if units: + log.info("Found %s %s units: \n\t%s" % ( + len(units), p.content_type, + "\n\t".join(str(unit) for unit in units) + )) + else: + log.info("No %s units found." % p.content_type) + return units diff --git a/pubtools/pulplib/__init__.py b/pubtools/pulplib/__init__.py index 84959ba9..2a95db6c 100644 --- a/pubtools/pulplib/__init__.py +++ b/pubtools/pulplib/__init__.py @@ -4,7 +4,6 @@ from ._impl.model import ( PulpObject, DetachedException, - InvalidContentTypeException, InvalidDataException, Repository, YumRepository, diff --git a/pubtools/pulplib/_impl/client/client.py b/pubtools/pulplib/_impl/client/client.py index 3f716b54..281fc4a6 100644 --- a/pubtools/pulplib/_impl/client/client.py +++ b/pubtools/pulplib/_impl/client/client.py @@ -12,11 +12,9 @@ from more_executors.futures import f_map, f_flat_map, f_return, f_proxy from ..page import Page -from ..criteria import Criteria, AndCriteria, OrCriteria, FieldMatchCriteria +from ..criteria import Criteria from ..model import Repository, MaintenanceReport, Distributor, Unit -from ..model.common import InvalidContentTypeException -from ..model.unit import SUPPORTED_UNIT_TYPES -from .search import filters_for_criteria +from .search import filters_for_criteria, validate_type_ids from .errors import PulpException from .poller import TaskPoller from . import retry @@ -205,45 +203,14 @@ def search_repository(self, criteria=None): Repository, "repositories", criteria=criteria, search_options=search_options ) - @staticmethod - def _filter_type_id_criteria(criteria): - type_ids = [] - filtered_criteria = None - - def extract_type_ids(crit): - if isinstance(crit, FieldMatchCriteria) and crit._field is "type_ids": - for type_id in crit._matcher._value: - if type_id in SUPPORTED_UNIT_TYPES: - type_ids.append(type_id) - - def criteria_set(crit): - other_crit = () - for c in crit._operands: - extract_type_ids(c) - if type_ids: - continue - else: - other_crit += (c,) - return other_crit - - if isinstance(criteria, AndCriteria): - filtered_criteria = Criteria.and_(*criteria_set(criteria)) - elif isinstance(criteria, OrCriteria): - filtered_criteria = Criteria.or_(*criteria_set(criteria)) - else: - extract_type_ids(criteria) - - if not type_ids: - raise InvalidContentTypeException() - - return type_ids, filtered_criteria - - def search_repository_content(self, repository_id, criteria=None): + def search_repository_content(self, repository_id, type_ids, criteria=None): """Search the given repository for content matching the given criteria. Args: - repository_id + repository_id (str) The ID of the repository in which 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. @@ -258,13 +225,11 @@ def search_repository_content(self, repository_id, criteria=None): .. versionadded:: 2.4.0 """ - # We have to extract criteria with type_ids, as they are not - # placed in "filters" with other criteria. - type_ids, criteria = self._filter_type_id_criteria(criteria) - search_options = {"type_ids": type_ids} - search_f = self._search( - Unit, "repositories/%s" % repository_id, criteria=criteria, search_options=search_options + Unit, + "repositories/%s" % repository_id, + criteria=criteria, + type_ids=validate_type_ids(type_ids), ) return [unit for unit in search_f.result().as_iter()] @@ -287,18 +252,26 @@ 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): + def _search( + self, + return_type, + resource_type, + criteria=None, + search_options=None, + type_ids=None, + ): pulp_crit = { "skip": 0, "limit": self._PAGE_SIZE, "filters": filters_for_criteria(criteria, return_type), + "type_ids": type_ids, } search = {"criteria": pulp_crit} search.update(search_options or {}) url = os.path.join(self._url, "pulp/api/v2/%s/search/" % resource_type) - if "type_ids" in search: + if return_type is Unit and search["criteria"]["type_ids"]: url = os.path.join(url, "units/") response_f = self._do_search(url, search) diff --git a/pubtools/pulplib/_impl/client/search.py b/pubtools/pulplib/_impl/client/search.py index 640fb5a4..3852b502 100644 --- a/pubtools/pulplib/_impl/client/search.py +++ b/pubtools/pulplib/_impl/client/search.py @@ -1,3 +1,4 @@ +import logging import datetime from pubtools.pulplib._impl.criteria import ( AndCriteria, @@ -13,6 +14,9 @@ from pubtools.pulplib._impl import compat_attr as attr from pubtools.pulplib._impl.model.attr import PULP2_FIELD, PY_PULP2_CONVERTER +from pubtools.pulplib._impl.model.unit import SUPPORTED_UNIT_TYPES + +LOG = logging.getLogger("pubtools.pulplib") def all_subclasses(klass): @@ -113,3 +117,34 @@ def field_match(to_match): return {"$lt": to_mongo_json(to_match._value)} raise TypeError("Not a matcher: %s" % repr(to_match)) + + +def validate_type_ids(type_ids): + valid_type_ids = [] + invalid_type_ids = [] + + if isinstance(type_ids, str): + type_ids = [type_ids] + + if not isinstance(type_ids, (list, tuple)): + raise TypeError("Expected str, list, or tuple, got %s" % type(type_ids)) + + for type_id in type_ids: + if type_id in SUPPORTED_UNIT_TYPES: + valid_type_ids.append(type_id) + else: + invalid_type_ids.append(type_id) + + if invalid_type_ids: + LOG.error( + "Invalid content type IDs: \n\t%s", + ", ".join(type_id for type_id in invalid_type_ids), + ) + + if valid_type_ids: + return valid_type_ids + + raise ValueError( + "Must provide valid content type ID(s):\n\t%s" + ", ".join(type_id for type_id in SUPPORTED_UNIT_TYPES) + ) diff --git a/pubtools/pulplib/_impl/fake/client.py b/pubtools/pulplib/_impl/fake/client.py index 3243a294..fa09a2c2 100644 --- a/pubtools/pulplib/_impl/fake/client.py +++ b/pubtools/pulplib/_impl/fake/client.py @@ -15,7 +15,6 @@ from pubtools.pulplib import ( Page, PulpException, - InvalidContentTypeException, Criteria, Task, Repository, @@ -23,9 +22,7 @@ Unit, MaintenanceReport, ) -from pubtools.pulplib._impl.criteria import FieldMatchCriteria, AndCriteria -from pubtools.pulplib._impl.client.search import filters_for_criteria -from ..model.unit import SUPPORTED_UNIT_TYPES +from pubtools.pulplib._impl.client.search import filters_for_criteria, validate_type_ids from .. import compat_attr as attr from .match import match_object @@ -94,35 +91,17 @@ def search_repository(self, criteria=None): random.shuffle(repos) return self._prepare_pages(repos) - def search_repository_content(self, repository_id, criteria=None): + def search_repository_content(self, repository_id, type_ids, criteria=None): + criteria = criteria or Criteria.true() units = [] - def get_type_id(crit): - if ( - isinstance(crit, FieldMatchCriteria) - and crit._field is "_content_type_id" - ): - return crit._matcher._value - - type_id = None - if isinstance(criteria, AndCriteria): - for c in criteria._operands: - type_id = get_type_id(c) - if type_id: - break - else: - type_id = get_type_id(criteria) - - if type_id not in SUPPORTED_UNIT_TYPES: - raise InvalidContentTypeException() - - print(criteria) filters_for_criteria(criteria, Unit) try: for unit in self._repo_units[repository_id]: - print(unit) - if match_object(criteria, unit): + 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) diff --git a/pubtools/pulplib/_impl/model/__init__.py b/pubtools/pulplib/_impl/model/__init__.py index 2a365ab3..8eeb7dc5 100644 --- a/pubtools/pulplib/_impl/model/__init__.py +++ b/pubtools/pulplib/_impl/model/__init__.py @@ -1,7 +1,6 @@ from .common import ( PulpObject, DetachedException, - InvalidContentTypeException, InvalidDataException, ) from .repository import ( diff --git a/pubtools/pulplib/_impl/model/common.py b/pubtools/pulplib/_impl/model/common.py index f33699a0..03406a70 100644 --- a/pubtools/pulplib/_impl/model/common.py +++ b/pubtools/pulplib/_impl/model/common.py @@ -19,12 +19,6 @@ class DetachedException(Exception): """ -class InvalidContentTypeException(Exception): - """Raised if a content type id is provided which appears to be invalid - (i.e. not among :class:`pubtools.pulplib.Unit` subclasses). - """ - - class InvalidDataException(Exception): """Raised if raw Pulp data appears to be invalid (i.e. not matching expected schema). """ diff --git a/pubtools/pulplib/_impl/model/repository/base.py b/pubtools/pulplib/_impl/model/repository/base.py index 5bf12e2b..1dd0d004 100644 --- a/pubtools/pulplib/_impl/model/repository/base.py +++ b/pubtools/pulplib/_impl/model/repository/base.py @@ -8,15 +8,12 @@ PulpObject, Deletable, DetachedException, - InvalidContentTypeException, ) from ..attr import pulp_attrib from ..distributor import Distributor from ..frozenlist import FrozenList from ...schema import load_schema from ... import compat_attr as attr -from ...criteria import Criteria, FieldMatchCriteria, AndCriteria, Matcher -from ..unit import Unit, SUPPORTED_UNIT_TYPES LOG = logging.getLogger("pubtools.pulplib") @@ -204,7 +201,7 @@ def iso_content(self): .. versionadded:: 2.4.0 """ - return self.search_content(Criteria.with_field("type_ids", ["iso"])) + return self.search_content("iso") @property def rpm_content(self): @@ -215,7 +212,7 @@ def rpm_content(self): .. versionadded:: 2.4.0 """ - return self.search_content(Criteria.with_field("type_ids", ["rpm"])) + return self.search_content("rpm") @property def srpm_content(self): @@ -226,7 +223,7 @@ def srpm_content(self): .. versionadded:: 2.4.0 """ - return self.search_content(Criteria.with_field("type_ids", ["srpm"])) + return self.search_content("srpm") @property def modulemd_content(self): @@ -237,7 +234,7 @@ def modulemd_content(self): .. versionadded:: 2.4.0 """ - return self.search_content(Criteria.with_field("type_ids", ["modulemd"])) + return self.search_content("modulemd") @property def modulemd_defaults_content(self): @@ -248,14 +245,14 @@ def modulemd_defaults_content(self): .. versionadded:: 2.4.0 """ - return self.search_content( - Criteria.with_field("type_ids", ["modulemd_defaults"]) - ) + return self.search_content("modulemd_defaults") - def search_content(self, criteria=None): + def search_content(self, type_id, criteria=None): """Search this repository for content matching the given criteria. Args: + type_id (str) + The content type to search criteria (:class:`~pubtools.pulplib.Criteria`) A criteria object used for this search. @@ -269,7 +266,7 @@ def search_content(self, criteria=None): if not self._client: raise DetachedException() - return self._client.search_repository_content(self.id, criteria) + return self._client.search_repository_content(self.id, type_id, criteria) def delete(self): """Delete this repository from Pulp. diff --git a/pubtools/pulplib/_impl/model/unit/modulemd_defaults.py b/pubtools/pulplib/_impl/model/unit/modulemd_defaults.py index 3177ec28..2d109ee8 100644 --- a/pubtools/pulplib/_impl/model/unit/modulemd_defaults.py +++ b/pubtools/pulplib/_impl/model/unit/modulemd_defaults.py @@ -22,6 +22,7 @@ class ModulemdDefaultsUnit(Unit): """The repository ID bound to this modulemd defaults unit""" profiles = pulp_attrib(pulp_field="profiles") - """The profiles of this modulemd defaults unit + """The profiles of this modulemd defaults unit. + The type for this attribute is omitted to allow for either dict or None. """ diff --git a/pubtools/pulplib/_impl/schema/unit.yaml b/pubtools/pulplib/_impl/schema/unit.yaml index 638b7306..5e5bdce5 100644 --- a/pubtools/pulplib/_impl/schema/unit.yaml +++ b/pubtools/pulplib/_impl/schema/unit.yaml @@ -134,7 +134,6 @@ definitions: type: string profiles: type: object - nullable: true required: - _content_type_id diff --git a/tests/client/test_client.py b/tests/client/test_client.py index 1bd4b0c7..712b1732 100644 --- a/tests/client/test_client.py +++ b/tests/client/test_client.py @@ -14,7 +14,6 @@ Repository, ModulemdDefaultsUnit, PulpException, - InvalidContentTypeException, MaintenanceReport, Task, Distributor, @@ -72,20 +71,11 @@ def test_can_search_repository_content(client, requests_mocker): "stream": "1.0", "profiles": {"p1": ["something"]}, } - }, - { - "metadata": { - "_content_type_id": "iso", - "name": "hello.txt", - "size": 23, - "checksum": "a" * 64, - } - }, + } ], ) - crit = Criteria.with_field("type_ids", ["modulemd_defaults"]) - units = client.search_repository_content("some-repo", crit) + units = client.search_repository_content("some-repo", type_ids="modulemd_defaults") # It should have returned one ModulemdDefaultsUnit assert units == [ @@ -99,12 +89,6 @@ def test_can_search_repository_content(client, requests_mocker): ] -def test_cannot_search_repository_content_without_content_type(client): - """search_content raises if called without criteria containing _content_type_id""" - with pytest.raises(InvalidContentTypeException): - client.search_repository_content("some-repo") - - def test_can_search_distributor(client, requests_mocker): """search_distributor issues distributors/search POST request as expected.""" requests_mocker.post( @@ -264,10 +248,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} - assert criteria[1] == {"filters": {}, "skip": 10, "limit": 10} - assert criteria[2] == {"filters": {}, "skip": 20, "limit": 10} - assert criteria[-1] == {"filters": {}, "skip": 990, "limit": 10} + 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} def test_can_get(client, requests_mocker): diff --git a/tests/client/test_search.py b/tests/client/test_search.py index b3cfaeb6..df2867d7 100644 --- a/tests/client/test_search.py +++ b/tests/client/test_search.py @@ -3,7 +3,11 @@ from pubtools.pulplib import Criteria, Matcher -from pubtools.pulplib._impl.client.search import filters_for_criteria, field_match +from pubtools.pulplib._impl.client.search import ( + filters_for_criteria, + field_match, + validate_type_ids, +) def test_null_criteria(): @@ -97,3 +101,25 @@ def test_dict_matcher_value(): assert filters_for_criteria(crit) == { "created": {"$lt": {"created_date": {"$date": "2019-09-04T00:00:00Z"}}} } + + +def test_valid_type_ids(caplog): + assert validate_type_ids(["srpm", "iso", "quark", "rpm"]) == ["srpm", "iso", "rpm"] + for m in ["Invalid content type IDs:", "quark"]: + assert m in caplog.text + + +def test_invalid_type_ids(): + """validate_type_ids raises if called without valid criteria""" + with pytest.raises(ValueError) as e: + validate_type_ids("quark") + + assert "Must provide valid content type ID(s)" in str(e) + + +def test_invalid_type_ids_type(): + """validate_type_ids raises if called without valid criteria""" + with pytest.raises(TypeError) as e: + validate_type_ids({"srpm": "some-srpm"}) + + assert "Expected str, list, or tuple, got %s" % type({}) in str(e) diff --git a/tests/fake/test_fake_search.py b/tests/fake/test_fake_search.py index c373023d..c67de79b 100644 --- a/tests/fake/test_fake_search.py +++ b/tests/fake/test_fake_search.py @@ -383,9 +383,8 @@ def test_search_repository_content(): controller.insert_units(repo1, units) client = controller.client - crit = Criteria.with_field("_content_type_id", "rpm") - found = client.search_repository_content("repo1", crit) + found = client.search_repository_content("repo1", type_ids="rpm") assert sorted(found) == [units[1]] diff --git a/tests/repository/test_search_content.py b/tests/repository/test_search_content.py index 93677fe4..0ce1a8d6 100644 --- a/tests/repository/test_search_content.py +++ b/tests/repository/test_search_content.py @@ -1,61 +1,5 @@ import pytest -from pubtools.pulplib import ( - Repository, - DetachedException, - InvalidContentTypeException, -) - -FAKE_UNIT_SEARCH = [ - { - "metadata": { - "_content_type_id": "iso", - "name": "hello.txt", - "size": 23, - "checksum": "a" * 64, - } - }, - { - "metadata": { - "_content_type_id": "srpm", - "name": "bash", - "epoch": "0", - "filename": "bash-x86_64.srpm", - "version": "4.0", - "release": "1", - "arch": "x86_64", - } - }, - { - "metadata": { - "_content_type_id": "rpm", - "name": "bash", - "epoch": "0", - "filename": "bash-x86_64.rpm", - "version": "4.0", - "release": "1", - "arch": "x86_64", - } - }, - { - "metadata": { - "_content_type_id": "modulemd", - "name": "md", - "stream": "s1", - "version": 1234, - "context": "a1b2c3", - "arch": "s390x", - } - }, - { - "metadata": { - "_content_type_id": "modulemd_defaults", - "name": "mdd", - "stream": "1.0", - "repo_id": "some-repo", - "profiles": {"p1": ["something"]}, - } - }, -] +from pubtools.pulplib import Repository, DetachedException def test_detached(): @@ -64,79 +8,130 @@ def test_detached(): assert not Repository(id="some-repo").iso_content -def test_search_content_without_content_type(client, requests_mocker): - """search_content raises if called without criteria containing _content_type_id""" - repo = Repository(id="some-repo") - repo.__dict__["_client"] = client - requests_mocker.post( - "https://pulp.example.com/pulp/api/v2/repositories/some-repo/search/units/", - json=FAKE_UNIT_SEARCH, - ) - - with pytest.raises(InvalidContentTypeException): - assert not repo.search_content() - - def test_iso_content(client, requests_mocker): - """iso_content returns correct units from the repository""" + """iso_content returns correct unit types""" repo = Repository(id="some-repo") repo.__dict__["_client"] = client requests_mocker.post( "https://pulp.example.com/pulp/api/v2/repositories/some-repo/search/units/", - json=FAKE_UNIT_SEARCH, + json=[ + { + "metadata": { + "_content_type_id": "iso", + "name": "hello.txt", + "size": 23, + "checksum": "a" * 64, + } + } + ], ) - assert len(repo.iso_content) == 1 - assert repo.iso_content[0].content_type_id == "iso" + isos = repo.iso_content + + assert len(isos) == 1 + assert isos[0].content_type_id == "iso" def test_rpm_content(client, requests_mocker): - """rpm_content returns correct units from the repository""" + """rpm_content returns correct unit types""" repo = Repository(id="some-repo") repo.__dict__["_client"] = client requests_mocker.post( "https://pulp.example.com/pulp/api/v2/repositories/some-repo/search/units/", - json=FAKE_UNIT_SEARCH, + json=[ + { + "metadata": { + "_content_type_id": "rpm", + "name": "bash", + "epoch": "0", + "filename": "bash-x86_64.rpm", + "version": "4.0", + "release": "1", + "arch": "x86_64", + } + } + ], ) - assert len(repo.rpm_content) == 1 - assert repo.rpm_content[0].content_type_id == "rpm" + rpms = repo.rpm_content + + assert len(rpms) == 1 + assert rpms[0].content_type_id == "rpm" def test_srpm_content(client, requests_mocker): - """srpm_content returns correct units from the repository""" + """srpm_content returns correct unit types""" repo = Repository(id="some-repo") repo.__dict__["_client"] = client requests_mocker.post( "https://pulp.example.com/pulp/api/v2/repositories/some-repo/search/units/", - json=FAKE_UNIT_SEARCH, + json=[ + { + "metadata": { + "_content_type_id": "srpm", + "name": "bash", + "epoch": "0", + "filename": "bash-x86_64.srpm", + "version": "4.0", + "release": "1", + "arch": "x86_64", + } + } + ], ) - assert len(repo.srpm_content) == 1 - assert repo.srpm_content[0].content_type_id == "srpm" + srpms = repo.srpm_content + + assert len(srpms) == 1 + assert srpms[0].content_type_id == "srpm" def test_modulemd_content(client, requests_mocker): - """modulemd_content returns correct units from the repository""" + """modulemd_content returns correct unit types""" repo = Repository(id="some-repo") repo.__dict__["_client"] = client requests_mocker.post( "https://pulp.example.com/pulp/api/v2/repositories/some-repo/search/units/", - json=FAKE_UNIT_SEARCH, + json=[ + { + "metadata": { + "_content_type_id": "modulemd", + "name": "md", + "stream": "s1", + "version": 1234, + "context": "a1b2c3", + "arch": "s390x", + } + } + ], ) - assert len(repo.modulemd_content) == 1 - assert repo.modulemd_content[0].content_type_id == "modulemd" + modulemds = repo.modulemd_content + + assert len(modulemds) == 1 + assert modulemds[0].content_type_id == "modulemd" def test_modulemd_defaults_content(client, requests_mocker): - """modulemd_defaults_content returns correct units from the repository""" + """modulemd_defaults_content returns correct unit types""" repo = Repository(id="some-repo") repo.__dict__["_client"] = client requests_mocker.post( "https://pulp.example.com/pulp/api/v2/repositories/some-repo/search/units/", - json=FAKE_UNIT_SEARCH, + json=[ + { + "metadata": { + "_content_type_id": "modulemd_defaults", + "name": "mdd", + "stream": "1.0", + "repo_id": "some-repo", + "profiles": {"p1": ["something"]}, + } + } + ], ) - assert len(repo.modulemd_defaults_content) == 1 - assert repo.modulemd_defaults_content[0].content_type_id == "modulemd_defaults" + modulemd_defaults = repo.modulemd_defaults_content + + assert len(modulemd_defaults) == 1 + assert modulemd_defaults[0].content_type_id == "modulemd_defaults" From 32edb12d0d670322b7d6435e3724b7c0dc39eb96 Mon Sep 17 00:00:00 2001 From: Nathan Gillett Date: Fri, 22 Nov 2019 13:39:21 -0500 Subject: [PATCH 10/19] Cover fake search_repository_content exception --- tests/fake/test_fake_search.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/fake/test_fake_search.py b/tests/fake/test_fake_search.py index c67de79b..46e9a08e 100644 --- a/tests/fake/test_fake_search.py +++ b/tests/fake/test_fake_search.py @@ -213,8 +213,12 @@ 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(): From b3f918437da8bf195a497896a71de2d9db425186 Mon Sep 17 00:00:00 2001 From: Nathan Gillett Date: Tue, 26 Nov 2019 15:33:38 -0500 Subject: [PATCH 11/19] Add test case for fake search_repository_content --- pubtools/pulplib/_impl/client/search.py | 2 +- tests/client/test_search.py | 2 +- tests/fake/test_fake_search.py | 16 ++++++++++------ 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/pubtools/pulplib/_impl/client/search.py b/pubtools/pulplib/_impl/client/search.py index 3852b502..e74b73c7 100644 --- a/pubtools/pulplib/_impl/client/search.py +++ b/pubtools/pulplib/_impl/client/search.py @@ -137,7 +137,7 @@ def validate_type_ids(type_ids): if invalid_type_ids: LOG.error( - "Invalid content type IDs: \n\t%s", + "Invalid content type ID(s): \n\t%s", ", ".join(type_id for type_id in invalid_type_ids), ) diff --git a/tests/client/test_search.py b/tests/client/test_search.py index df2867d7..fb980209 100644 --- a/tests/client/test_search.py +++ b/tests/client/test_search.py @@ -105,7 +105,7 @@ def test_dict_matcher_value(): def test_valid_type_ids(caplog): assert validate_type_ids(["srpm", "iso", "quark", "rpm"]) == ["srpm", "iso", "rpm"] - for m in ["Invalid content type IDs:", "quark"]: + for m in ["Invalid content type ID(s):", "quark"]: assert m in caplog.text diff --git a/tests/fake/test_fake_search.py b/tests/fake/test_fake_search.py index 46e9a08e..daade83c 100644 --- a/tests/fake/test_fake_search.py +++ b/tests/fake/test_fake_search.py @@ -357,7 +357,9 @@ def test_search_paginates(): assert sorted(found_repos) == sorted(repos) -def test_search_repository_content(): +@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") @@ -386,11 +388,13 @@ def test_search_repository_content(): ] controller.insert_units(repo1, units) - client = controller.client - - found = client.search_repository_content("repo1", type_ids="rpm") - - assert sorted(found) == [units[1]] + 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(): From b0bc322e7e380425aea4cf565b78acb032f5d94b Mon Sep 17 00:00:00 2001 From: Nathan Gillett Date: Wed, 27 Nov 2019 13:23:50 -0500 Subject: [PATCH 12/19] Remove unnecessary calls from search_repository_content result --- pubtools/pulplib/_impl/client/client.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pubtools/pulplib/_impl/client/client.py b/pubtools/pulplib/_impl/client/client.py index 281fc4a6..b31194fc 100644 --- a/pubtools/pulplib/_impl/client/client.py +++ b/pubtools/pulplib/_impl/client/client.py @@ -231,7 +231,7 @@ def search_repository_content(self, repository_id, type_ids, criteria=None): criteria=criteria, type_ids=validate_type_ids(type_ids), ) - return [unit for unit in search_f.result().as_iter()] + return [unit for unit in search_f] def search_distributor(self, criteria=None): """Search the distributors matching the given criteria. From d453337c8766046cdae4ead4bd2599f5c668fec7 Mon Sep 17 00:00:00 2001 From: Nathan Gillett Date: Wed, 27 Nov 2019 13:54:36 -0500 Subject: [PATCH 13/19] Fix validate_type_ids ValueError message and update CHANGELOG.md --- pubtools/pulplib/_impl/client/search.py | 2 +- tests/client/test_search.py | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/pubtools/pulplib/_impl/client/search.py b/pubtools/pulplib/_impl/client/search.py index e74b73c7..a86a64a3 100644 --- a/pubtools/pulplib/_impl/client/search.py +++ b/pubtools/pulplib/_impl/client/search.py @@ -145,6 +145,6 @@ def validate_type_ids(type_ids): return valid_type_ids raise ValueError( - "Must provide valid content type ID(s):\n\t%s" + "Must provide valid content type ID(s): \n\t%s", ", ".join(type_id for type_id in SUPPORTED_UNIT_TYPES) ) diff --git a/tests/client/test_search.py b/tests/client/test_search.py index fb980209..59c5f37d 100644 --- a/tests/client/test_search.py +++ b/tests/client/test_search.py @@ -114,7 +114,8 @@ def test_invalid_type_ids(): with pytest.raises(ValueError) as e: validate_type_ids("quark") - assert "Must provide valid content type ID(s)" in str(e) + assert "Must provide valid content type ID(s):" in str(e) + assert "iso, rpm, srpm, modulemd, modulemd_defaults" in str(e) def test_invalid_type_ids_type(): From 4ef7c0c6a663b1a5683cc21a94f451b679d126fb Mon Sep 17 00:00:00 2001 From: Nathan Gillett Date: Wed, 27 Nov 2019 13:58:25 -0500 Subject: [PATCH 14/19] Refactor with black --- pubtools/pulplib/_impl/client/search.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pubtools/pulplib/_impl/client/search.py b/pubtools/pulplib/_impl/client/search.py index a86a64a3..a0ad6f29 100644 --- a/pubtools/pulplib/_impl/client/search.py +++ b/pubtools/pulplib/_impl/client/search.py @@ -146,5 +146,5 @@ def validate_type_ids(type_ids): raise ValueError( "Must provide valid content type ID(s): \n\t%s", - ", ".join(type_id for type_id in SUPPORTED_UNIT_TYPES) + ", ".join(type_id for type_id in SUPPORTED_UNIT_TYPES), ) From 804a4272d53b3994fb7f183e9f93eed010ed9e20 Mon Sep 17 00:00:00 2001 From: Nathan Gillett Date: Wed, 27 Nov 2019 14:59:34 -0500 Subject: [PATCH 15/19] Clean up --- pubtools/pulplib/_impl/client/client.py | 26 ++++++++++--------------- pubtools/pulplib/_impl/client/search.py | 10 ++-------- tests/client/test_search.py | 3 +-- 3 files changed, 13 insertions(+), 26 deletions(-) diff --git a/pubtools/pulplib/_impl/client/client.py b/pubtools/pulplib/_impl/client/client.py index b31194fc..a963304d 100644 --- a/pubtools/pulplib/_impl/client/client.py +++ b/pubtools/pulplib/_impl/client/client.py @@ -208,9 +208,9 @@ def search_repository_content(self, repository_id, type_ids, criteria=None): Args: repository_id (str) - The ID of the repository in which to search + The ID of the repository to search. type_ids (str, list, tuple) - A list of content types to search + A list of content types to search. criteria (:class:`~pubtools.pulplib.Criteria`) A criteria object used for this search. @@ -225,13 +225,13 @@ def search_repository_content(self, repository_id, type_ids, criteria=None): .. versionadded:: 2.4.0 """ - search_f = self._search( - Unit, - "repositories/%s" % repository_id, - criteria=criteria, - type_ids=validate_type_ids(type_ids), + resource_type = "repositories/%s" % repository_id + search_options = {"type_ids": validate_type_ids(type_ids)} + return list( + self._search( + Unit, resource_type, criteria=criteria, search_options=search_options + ) ) - return [unit for unit in search_f] def search_distributor(self, criteria=None): """Search the distributors matching the given criteria. @@ -252,14 +252,8 @@ 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=None, - ): + 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, diff --git a/pubtools/pulplib/_impl/client/search.py b/pubtools/pulplib/_impl/client/search.py index a0ad6f29..1e3b9604 100644 --- a/pubtools/pulplib/_impl/client/search.py +++ b/pubtools/pulplib/_impl/client/search.py @@ -136,15 +136,9 @@ def validate_type_ids(type_ids): invalid_type_ids.append(type_id) if invalid_type_ids: - LOG.error( - "Invalid content type ID(s): \n\t%s", - ", ".join(type_id for type_id in invalid_type_ids), - ) + LOG.error("Invalid content type ID(s): \n\t%s", ", ".join(invalid_type_ids)) if valid_type_ids: return valid_type_ids - raise ValueError( - "Must provide valid content type ID(s): \n\t%s", - ", ".join(type_id for type_id in SUPPORTED_UNIT_TYPES), - ) + raise ValueError("Must provide valid content type ID(s)") diff --git a/tests/client/test_search.py b/tests/client/test_search.py index 59c5f37d..fb980209 100644 --- a/tests/client/test_search.py +++ b/tests/client/test_search.py @@ -114,8 +114,7 @@ def test_invalid_type_ids(): with pytest.raises(ValueError) as e: validate_type_ids("quark") - assert "Must provide valid content type ID(s):" in str(e) - assert "iso, rpm, srpm, modulemd, modulemd_defaults" in str(e) + assert "Must provide valid content type ID(s)" in str(e) def test_invalid_type_ids_type(): From 8ebeb3a160c345f8cf0bac7f4dfea4d1bbce9937 Mon Sep 17 00:00:00 2001 From: Nathan Gillett Date: Tue, 3 Dec 2019 16:49:55 -0500 Subject: [PATCH 16/19] Add alternative method to examples/search-repo-content --- examples/search-repo-content | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/examples/search-repo-content b/examples/search-repo-content index 132157a9..fb304d69 100644 --- a/examples/search-repo-content +++ b/examples/search-repo-content @@ -46,6 +46,17 @@ def main(): 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. + """ + if units: log.info("Found %s %s units: \n\t%s" % ( len(units), p.content_type, From af354da547a015b1bd9dfbecef2eccdccaab3c60 Mon Sep 17 00:00:00 2001 From: Nathan Gillett Date: Thu, 5 Dec 2019 08:52:10 -0500 Subject: [PATCH 17/19] Rename repository_id variable to repo_id --- pubtools/pulplib/_impl/client/client.py | 6 +++--- pubtools/pulplib/_impl/fake/client.py | 4 ++-- pubtools/pulplib/_impl/model/unit/modulemd_defaults.py | 2 +- tests/client/test_client.py | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/pubtools/pulplib/_impl/client/client.py b/pubtools/pulplib/_impl/client/client.py index a963304d..b5ce9244 100644 --- a/pubtools/pulplib/_impl/client/client.py +++ b/pubtools/pulplib/_impl/client/client.py @@ -203,11 +203,11 @@ def search_repository(self, criteria=None): Repository, "repositories", criteria=criteria, search_options=search_options ) - def search_repository_content(self, repository_id, type_ids, criteria=None): + def search_repository_content(self, repo_id, type_ids, criteria=None): """Search the given repository for content matching the given criteria. Args: - repository_id (str) + repo_id (str) The ID of the repository to search. type_ids (str, list, tuple) A list of content types to search. @@ -225,7 +225,7 @@ def search_repository_content(self, repository_id, type_ids, criteria=None): .. versionadded:: 2.4.0 """ - resource_type = "repositories/%s" % repository_id + resource_type = "repositories/%s" % repo_id search_options = {"type_ids": validate_type_ids(type_ids)} return list( self._search( diff --git a/pubtools/pulplib/_impl/fake/client.py b/pubtools/pulplib/_impl/fake/client.py index fa09a2c2..2b0712d5 100644 --- a/pubtools/pulplib/_impl/fake/client.py +++ b/pubtools/pulplib/_impl/fake/client.py @@ -91,14 +91,14 @@ def search_repository(self, criteria=None): random.shuffle(repos) return self._prepare_pages(repos) - def search_repository_content(self, repository_id, type_ids, criteria=None): + 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[repository_id]: + for unit in self._repo_units[repo_id]: if unit.content_type_id in validate_type_ids(type_ids) and match_object( criteria, unit ): diff --git a/pubtools/pulplib/_impl/model/unit/modulemd_defaults.py b/pubtools/pulplib/_impl/model/unit/modulemd_defaults.py index 2d109ee8..912750f0 100644 --- a/pubtools/pulplib/_impl/model/unit/modulemd_defaults.py +++ b/pubtools/pulplib/_impl/model/unit/modulemd_defaults.py @@ -18,7 +18,7 @@ class ModulemdDefaultsUnit(Unit): stream = pulp_attrib(type=str, pulp_field="stream") """The stream of this modulemd defaults unit""" - repository_id = pulp_attrib(type=str, pulp_field="repo_id") + repo_id = pulp_attrib(type=str, pulp_field="repo_id") """The repository ID bound to this modulemd defaults unit""" profiles = pulp_attrib(pulp_field="profiles") diff --git a/tests/client/test_client.py b/tests/client/test_client.py index 712b1732..36b18980 100644 --- a/tests/client/test_client.py +++ b/tests/client/test_client.py @@ -82,7 +82,7 @@ def test_can_search_repository_content(client, requests_mocker): ModulemdDefaultsUnit( content_type_id="modulemd_defaults", name="mdd", - repository_id="mdd-repo", + repo_id="mdd-repo", stream="1.0", profiles={"p1": ["something"]}, ) From 8d27f1d2588f702c2ccb20e17a7febc655c6c64f Mon Sep 17 00:00:00 2001 From: Nathan Gillett Date: Mon, 9 Dec 2019 11:33:32 -0500 Subject: [PATCH 18/19] 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() From 4f1d346e8fb5a19d0b6f130225ea1f259737f903 Mon Sep 17 00:00:00 2001 From: Nathan Gillett Date: Mon, 9 Dec 2019 11:48:10 -0500 Subject: [PATCH 19/19] Remove unused variables --- tests/client/test_client.py | 1 - tests/fake/test_fake_search.py | 9 +-------- 2 files changed, 1 insertion(+), 9 deletions(-) diff --git a/tests/client/test_client.py b/tests/client/test_client.py index 04036b57..9b5dc233 100644 --- a/tests/client/test_client.py +++ b/tests/client/test_client.py @@ -12,7 +12,6 @@ Criteria, Matcher, Repository, - ModulemdDefaultsUnit, PulpException, MaintenanceReport, Task, diff --git a/tests/fake/test_fake_search.py b/tests/fake/test_fake_search.py index 00877766..55beeb99 100644 --- a/tests/fake/test_fake_search.py +++ b/tests/fake/test_fake_search.py @@ -2,14 +2,7 @@ import pytest -from pubtools.pulplib import ( - FakeController, - Repository, - Unit, - Criteria, - Matcher, - Distributor, -) +from pubtools.pulplib import FakeController, Repository, Criteria, Matcher, Distributor def test_can_search_id():