Bugfix - fix handling of type_ids in remove_content

.gitignore

@@ -102,3 +102,6 @@ venv.bak/
 
 # mypy
 .mypy_cache/
+
+# JetBrains
+.idea/

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)
 

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)

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):

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):