Add api_quirk decorator to mark api spec patches

fixes #658

CHANGES/pulp-glue/658.feature

@@ -0,0 +1 @@
+Added decorator `api_quirk` to declare version dependent fixes to the api spec.

pulp-glue/pulp_glue/common/context.py

@@ -3,6 +3,7 @@
 import re
 import sys
 import time
+import typing as t
 from typing import IO, Any, ClassVar, Dict, List, Mapping, Optional, Set, Type, Union, cast
 
 from packaging.specifiers import SpecifierSet
@@ -75,6 +76,11 @@ def __contains__(self, version: Optional[str]) -> bool:
             return self.inverted
         return (version in self.specifier) != self.inverted
 
+    def __str__(self) -> str:
+        return f"{self.__class__.__name__}({self.name}{self.specifier})"
+
+    __repr__ = __str__
+
 
 class PulpException(Exception):
     pass
@@ -103,6 +109,78 @@ def preprocess_payload(payload: EntityDefinition) -> EntityDefinition:
     )
 
 
+_REGISTERED_API_QUIRKS: t.List[t.Tuple[PluginRequirement, t.Callable[[OpenAPI], None]]] = []
+
+
+def api_quirk(
+    req: PluginRequirement,
+) -> t.Callable[[t.Callable[[OpenAPI], None]], None]:
+    def _decorator(patch: t.Callable[[OpenAPI], None]) -> None:
+        _REGISTERED_API_QUIRKS.append((req, patch))
+
+    return _decorator
+
+
+@api_quirk(PluginRequirement("core", specifier="<3.20.0"))
+def patch_ordering_filters(api: OpenAPI) -> None:
+    for method, path in api.operations.values():
+        operation = api.api_spec["paths"][path][method]
+        if method == "get" and "parameters" in operation:
+            for parameter in operation["parameters"]:
+                if (
+                    parameter["name"] == "ordering"
+                    and parameter["in"] == "query"
+                    and "schema" in parameter
+                    and parameter["schema"]["type"] == "string"
+                ):
+                    parameter["schema"] = {"type": "array", "items": {"type": "string"}}
+                    parameter["explode"] = False
+                    parameter["style"] = "form"
+
+
+@api_quirk(PluginRequirement("core", specifier="<3.22.0"))
+def patch_field_select_filters(api: OpenAPI) -> None:
+    for method, path in api.operations.values():
+        operation = api.api_spec["paths"][path][method]
+        if method == "get" and "parameters" in operation:
+            for parameter in operation["parameters"]:
+                if (
+                    parameter["name"] in ["fields", "exclude_fields"]
+                    and parameter["in"] == "query"
+                    and "schema" in parameter
+                    and parameter["schema"]["type"] == "string"
+                ):
+                    parameter["schema"] = {"type": "array", "items": {"type": "string"}}
+
+
+@api_quirk(PluginRequirement("core", specifier="<99.99.0"))
+def patch_content_in_query_filters(api: OpenAPI) -> None:
+    # https://github.com/pulp/pulpcore/issues/3634
+    for operation_id, (method, path) in api.operations.items():
+        if (
+            operation_id == "repository_versions_list"
+            or (
+                operation_id.startswith("repositories_") and operation_id.endswith("_versions_list")
+            )
+            or (operation_id.startswith("publications_") and operation_id.endswith("_list"))
+        ):
+            operation = api.api_spec["paths"][path][method]
+            for parameter in operation["parameters"]:
+                if (
+                    parameter["name"] == "content__in"
+                    and parameter["in"] == "query"
+                    and "schema" in parameter
+                    and parameter["schema"]["type"] == "string"
+                ):
+                    parameter["schema"] = {"type": "array", "items": {"type": "string"}}
+
+
+@api_quirk(PluginRequirement("core", specifier=">=3.23,<3.30.0"))
+def patch_upstream_pulp_replicate_request_body(api: OpenAPI) -> None:
+    operation = api.api_spec["paths"]["{upstream_pulp_href}replicate/"]["post"]
+    operation.pop("requestBody", None)
+
+
 class PulpContext:
     """
     Abstract class for the global PulpContext object.
@@ -138,75 +216,13 @@ def __init__(
             timeout = datetime.timedelta(seconds=timeout)
         self.timeout: datetime.timedelta = timeout
 
-    def _patch_api_spec(self) -> None:
+    def _patch_api_spec(self, api: OpenAPI) -> None:
         # A place for last minute fixes to the api_spec.
         # WARNING: Operations are already indexed at this point.
-        api_spec = self.api.api_spec
-        if self.has_plugin(PluginRequirement("core", specifier="<3.20.0")):
-            for method, path in self.api.operations.values():
-                operation = api_spec["paths"][path][method]
-                if method == "get" and "parameters" in operation:
-                    for parameter in operation["parameters"]:
-                        if (
-                            parameter["name"] == "ordering"
-                            and parameter["in"] == "query"
-                            and "schema" in parameter
-                            and parameter["schema"]["type"] == "string"
-                        ):
-                            parameter["schema"] = {"type": "array", "items": {"type": "string"}}
-                            parameter["explode"] = False
-                            parameter["style"] = "form"
-        if self.has_plugin(PluginRequirement("core", specifier="<3.22.0")):
-            for method, path in self.api.operations.values():
-                operation = api_spec["paths"][path][method]
-                if method == "get" and "parameters" in operation:
-                    for parameter in operation["parameters"]:
-                        if (
-                            parameter["name"] in ["fields", "exclude_fields"]
-                            and parameter["in"] == "query"
-                            and "schema" in parameter
-                            and parameter["schema"]["type"] == "string"
-                        ):
-                            parameter["schema"] = {"type": "array", "items": {"type": "string"}}
-        if self.has_plugin(PluginRequirement("core", specifier="<99.99.0")):
-            # https://github.com/pulp/pulpcore/issues/3634
-            for operation_id, (method, path) in self.api.operations.items():
-                if (
-                    operation_id == "repository_versions_list"
-                    or (
-                        operation_id.startswith("repositories_")
-                        and operation_id.endswith("_versions_list")
-                    )
-                    or (operation_id.startswith("publications_") and operation_id.endswith("_list"))
-                ):
-                    operation = api_spec["paths"][path][method]
-                    for parameter in operation["parameters"]:
-                        if (
-                            parameter["name"] == "content__in"
-                            and parameter["in"] == "query"
-                            and "schema" in parameter
-                            and parameter["schema"]["type"] == "string"
-                        ):
-                            parameter["schema"] = {"type": "array", "items": {"type": "string"}}
-        if self.has_plugin(PluginRequirement("core", specifier=">=3.23,<3.30.0")):
-            operation = api_spec["paths"]["{upstream_pulp_href}replicate/"]["post"]
-            operation.pop("requestBody", None)
-        if self.has_plugin(PluginRequirement("file", specifier=">=1.10.0,<1.11.0")):
-            operation = api_spec["paths"]["{file_file_alternate_content_source_href}refresh/"][
-                "post"
-            ]
-            operation.pop("requestBody", None)
-        if self.has_plugin(PluginRequirement("python", specifier="<99.99.0.dev")):
-            # TODO Add version bounds
-            python_remote_serializer = api_spec["components"]["schemas"]["python.PythonRemote"]
-            patched_python_remote_serializer = api_spec["components"]["schemas"][
-                "Patchedpython.PythonRemote"
-            ]
-            for prop in ("includes", "excludes"):
-                python_remote_serializer["properties"][prop]["type"] = "array"
-                python_remote_serializer["properties"][prop]["items"] = {"type": "string"}
-                patched_python_remote_serializer["properties"][prop]["type"] = "array"
-                patched_python_remote_serializer["properties"][prop]["items"] = {"type": "string"}
+        assert self._api is not None
+        for req, patch in _REGISTERED_API_QUIRKS:
+            if self.has_plugin(req):
+                patch(self._api)
 
     @property
     def domain_enabled(self) -> bool:
@@ -232,7 +248,7 @@ def api(self) -> OpenAPI:
             # Rerun scheduled version checks
             for plugin_requirement in self._needed_plugins:
                 self.needs_plugin(plugin_requirement)
-            self._patch_api_spec()
+            self._patch_api_spec(self._api)
         return self._api
 
     @property

pulp-glue/pulp_glue/file/context.py

@@ -1,4 +1,4 @@
-from typing import Any, Mapping, Optional
+import typing as t
 
 from pulp_glue.common.context import (
     EntityDefinition,
@@ -10,14 +10,22 @@
     PulpRemoteContext,
     PulpRepositoryContext,
     PulpRepositoryVersionContext,
+    api_quirk,
 )
 from pulp_glue.common.i18n import get_translation
+from pulp_glue.common.openapi import OpenAPI
 from pulp_glue.core.context import PulpArtifactContext
 
 translation = get_translation(__name__)
 _ = translation.gettext
 
 
+@api_quirk(PluginRequirement("file", specifier=">=1.10.0,<1.11.0"))
+def patch_file_acs_refresh_request_body(api: OpenAPI) -> None:
+    operation = api.api_spec["paths"]["{file_file_alternate_content_source_href}refresh/"]["post"]
+    operation.pop("requestBody", None)
+
+
 class PulpFileACSContext(PulpACSContext):
     PLUGIN = "file"
     RESOURCE_TYPE = "file"
@@ -42,9 +50,9 @@ class PulpFileContentContext(PulpContentContext):
     def create(
         self,
         body: EntityDefinition,
-        parameters: Optional[Mapping[str, Any]] = None,
+        parameters: t.Optional[t.Mapping[str, t.Any]] = None,
         non_blocking: bool = False,
-    ) -> Any:
+    ) -> t.Any:
         if "sha256" in body:
             body = body.copy()
             body["artifact"] = PulpArtifactContext(

pulp-glue/pulp_glue/python/context.py

@@ -7,13 +7,29 @@
     PulpRemoteContext,
     PulpRepositoryContext,
     PulpRepositoryVersionContext,
+    api_quirk,
 )
 from pulp_glue.common.i18n import get_translation
+from pulp_glue.common.openapi import OpenAPI
 
 translation = get_translation(__name__)
 _ = translation.gettext
 
 
+# TODO Add version bounds
+@api_quirk(PluginRequirement("python", specifier="<99.99.0.dev"))
+def patch_python_remote_includes_excludes(api: OpenAPI) -> None:
+    python_remote_serializer = api.api_spec["components"]["schemas"]["python.PythonRemote"]
+    patched_python_remote_serializer = api.api_spec["components"]["schemas"][
+        "Patchedpython.PythonRemote"
+    ]
+    for prop in ("includes", "excludes"):
+        python_remote_serializer["properties"][prop]["type"] = "array"
+        python_remote_serializer["properties"][prop]["items"] = {"type": "string"}
+        patched_python_remote_serializer["properties"][prop]["type"] = "array"
+        patched_python_remote_serializer["properties"][prop]["items"] = {"type": "string"}
+
+
 class PulpPythonContentContext(PulpContentContext):
     PLUGIN = "python"
     RESOURCE_TYPE = "package"

pulpcore/cli/rpm/remote.py

@@ -26,7 +26,7 @@
 
 
 def _uln_url_callback(ctx: click.Context, param: click.Parameter, value: str) -> str:
-    if type(ctx.obj) == PulpUlnRemoteContext and (value and not value.startswith("uln://")):
+    if type(ctx.obj) is PulpUlnRemoteContext and (value and not value.startswith("uln://")):
         raise click.ClickException("Invalid url format. Please enter correct uln channel.")
 
     return value

tests/test_api_quirks.py

@@ -2,7 +2,7 @@
 from copy import deepcopy
 
 import pytest
-from pulp_glue.common.context import PulpContext
+from pulp_glue.common.context import PulpContext, _REGISTERED_API_QUIRKS
 
 
 @pytest.mark.glue
@@ -21,7 +21,12 @@ def test_api_quirks_idempotent(
         background_tasks=False,
         timeout=settings.get("timeout", 120),
     )
+
+    # TODO: Find a better way to ensure all modules are loaded
+    assert len(_REGISTERED_API_QUIRKS) == 6
+
     patched_once_api = deepcopy(pulp_ctx.api.api_spec)
     # Patch a second time
-    pulp_ctx._patch_api_spec()
+    assert pulp_ctx._api is not None
+    pulp_ctx._patch_api_spec(pulp_ctx._api)
     assert pulp_ctx.api.api_spec == patched_once_api