diff --git a/.gitignore b/.gitignore index 894a44cc..8f1cd5bd 100644 --- a/.gitignore +++ b/.gitignore @@ -102,3 +102,6 @@ venv.bak/ # mypy .mypy_cache/ + +# JetBrains +.idea/ \ No newline at end of file diff --git a/pubtools/pulplib/_impl/client/client.py b/pubtools/pulplib/_impl/client/client.py index 72e63070..57f9b4a1 100644 --- a/pubtools/pulplib/_impl/client/client.py +++ b/pubtools/pulplib/_impl/client/client.py @@ -1,15 +1,15 @@ -import os -import logging -import threading import hashlib import json +import logging +import os +import threading from functools import partial -import six -from six.moves import StringIO import requests +import six from more_executors import Executors from more_executors.futures import f_map, f_flat_map, f_return, f_proxy, f_sequence +from six.moves import StringIO from ..page import Page from ..criteria import Criteria @@ -470,14 +470,17 @@ def _publish_distributor(self, repo_id, distributor_id, override_config): self._do_request, method="POST", url=url, json=body ) - def _do_unassociate(self, repo_id, type_ids): + def _do_unassociate(self, repo_id, criteria=None): url = os.path.join( self._url, "pulp/api/v2/repositories/%s/actions/unassociate/" % repo_id ) - body = {} - if type_ids is not None: - body["type_ids"] = type_ids + # use type hint=Unit so that if type_ids are the goal here + # then we will get back a properly prepared PulpSearch with + # a populated type_ids field + pulp_search = search_for_criteria(criteria, type_hint=Unit, type_ids_accum=None) + + body = {"criteria": {"type_ids": pulp_search.type_ids}} LOG.debug("Submitting %s unassociate: %s", url, body) diff --git a/pubtools/pulplib/_impl/fake/client.py b/pubtools/pulplib/_impl/fake/client.py index 739a4fd2..4c53fa0a 100644 --- a/pubtools/pulplib/_impl/fake/client.py +++ b/pubtools/pulplib/_impl/fake/client.py @@ -277,7 +277,7 @@ def do_next_upload(checksum, size): return out - def _do_unassociate(self, repo_id, type_ids): + def _do_unassociate(self, repo_id, criteria=None): repo_f = self.get_repository(repo_id) if repo_f.exception(): return repo_f @@ -286,8 +286,16 @@ def _do_unassociate(self, repo_id, type_ids): removed = set() kept = set() + # use type hint=Unit so that if type_ids are the goal here + # then we will get back a properly prepared PulpSearch with + # a populated type_ids field + pulp_search = search_for_criteria(criteria, type_hint=Unit, type_ids_accum=None) + for unit in current: - if type_ids is None or unit.content_type_id in type_ids: + if ( + pulp_search.type_ids is None + or unit.content_type_id in pulp_search.type_ids + ): removed.add(unit) else: kept.add(unit) diff --git a/pubtools/pulplib/_impl/model/repository/base.py b/pubtools/pulplib/_impl/model/repository/base.py index 3db36583..5938d1db 100644 --- a/pubtools/pulplib/_impl/model/repository/base.py +++ b/pubtools/pulplib/_impl/model/repository/base.py @@ -8,7 +8,7 @@ from ..attr import pulp_attrib from ..distributor import Distributor from ..frozenlist import FrozenList -from ...criteria import Criteria +from ...criteria import Criteria, Matcher from ...schema import load_schema from ... import compat_attr as attr from ...hooks import pm @@ -471,8 +471,31 @@ def remove_content(self, **kwargs): # version of this method will support some "criteria". Let's not fix the # argument order at least until then. + # start down the path of using Criteria per this issue: + # https://github.com/release-engineering/pubtools-pulplib/issues/62 + # by using Criteria internally. + # there seems to be pretty complex handling of Criteria + # for serialization in the search API, and it is unclear which parts + # might also be necessary to use for content removal. + # If any or all of the same handling is needed, it would be beneficial + # to encapsulate the preparation of a criteria JSON object in some + # (more generically named) functions or a class to avoid duplicating code. + # for reference see search_content, _impl.client.Client._search_repo_units, + # _impl.client.Client._search, and _impl.client.search.search_for_criteria + criteria = None type_ids = kwargs.get("type_ids") - return f_proxy(self._client._do_unassociate(self.id, type_ids)) + # use _content_type_id field name to coerce + # search_for_criteria to fill out the PulpSearch#type_ids field. + # passing a criteria with an empty type_ids list rather than + # None results in failing tests due to the implementation of + # FakeClient#_do_unassociate + if type_ids is not None: + criteria = Criteria.with_field( + "_content_type_id", + Matcher.in_(type_ids), # Criteria.with_field_in is deprecated + ) + + return f_proxy(self._client._do_unassociate(self.id, criteria=criteria)) @classmethod def from_data(cls, data): diff --git a/tests/repository/test_remove_content.py b/tests/repository/test_remove_content.py index 326c421b..78c14e3f 100644 --- a/tests/repository/test_remove_content.py +++ b/tests/repository/test_remove_content.py @@ -77,7 +77,7 @@ def test_remove_with_type_ids(fast_poller, requests_mocker, client): req[0].url == "https://pulp.example.com/pulp/api/v2/repositories/some-repo/actions/unassociate/" ) - assert req[0].json() == {"type_ids": ["type1", "type2"]} + assert req[0].json() == {"criteria": {"type_ids": ["type1", "type2"]}} def test_remove_loads_units(fast_poller, requests_mocker, client):