From 57409b0a223a59b4fc5cda6b26dfb9fa34e9c64d Mon Sep 17 00:00:00 2001 From: Jonathan Green Date: Fri, 20 Feb 2026 11:26:37 -0400 Subject: [PATCH 1/5] Add admin endpoint to queue collection import on demand Add a POST /admin/collection//import endpoint that allows admins to manually trigger a collection import task. The endpoint validates admin permissions, checks that the collection exists and is not marked for deletion, and queues the import task via Celery. Also adds a `supports_import` flag to protocol details so the admin UI can determine which collections support import, and an IMPORT_NOT_SUPPORTED problem detail for unsupported protocols. --- .../admin/controller/collection_settings.py | 35 ++++++ .../manager/api/admin/problem_details.py | 7 ++ src/palace/manager/api/admin/routes.py | 10 ++ src/palace/manager/api/circulation/base.py | 12 +- .../api/admin/controller/test_collections.py | 111 ++++++++++++++++++ 5 files changed, 174 insertions(+), 1 deletion(-) diff --git a/src/palace/manager/api/admin/controller/collection_settings.py b/src/palace/manager/api/admin/controller/collection_settings.py index 85c6e8dae2..751a3eea7e 100644 --- a/src/palace/manager/api/admin/controller/collection_settings.py +++ b/src/palace/manager/api/admin/controller/collection_settings.py @@ -12,6 +12,7 @@ from palace.manager.api.admin.form_data import ProcessFormData from palace.manager.api.admin.problem_details import ( CANNOT_DELETE_COLLECTION_WITH_CHILDREN, + IMPORT_NOT_SUPPORTED, MISSING_COLLECTION, MISSING_PARENT, MISSING_SERVICE, @@ -195,6 +196,40 @@ def process_delete(self, service_id: int) -> Response | ProblemDetail: collection_delete.delay(collection.id) return Response("Deleted", 200) + def process_import(self, collection_id: int) -> Response | ProblemDetail: + """Queue a collection import task on demand. + + :param collection_id: The integration configuration ID of the collection to import. + :return: A 200 response on success, or a ProblemDetail on error. + """ + self.require_system_admin() + + integration = get_one( + self._db, + IntegrationConfiguration, + id=collection_id, + goal=self.registry.goal, + ) + if not integration: + return MISSING_SERVICE + + collection = integration.collection + if not collection: + return MISSING_COLLECTION + + if collection.marked_for_deletion: + return MISSING_COLLECTION + + impl_cls = self.registry[integration.protocol] + force = flask.request.form.get("force", "false").lower() == "true" + + try: + impl_cls.import_task(collection.id, force=force).apply_async() + except NotImplementedError: + return IMPORT_NOT_SUPPORTED + + return Response("Import task queued.", 200) + def process_collection_self_tests( self, identifier: int | None ) -> Response | ProblemDetail: diff --git a/src/palace/manager/api/admin/problem_details.py b/src/palace/manager/api/admin/problem_details.py index ca3a825ab1..a8bd0aeabd 100644 --- a/src/palace/manager/api/admin/problem_details.py +++ b/src/palace/manager/api/admin/problem_details.py @@ -482,6 +482,13 @@ detail=_("You can only make a lane visible if its parent is already visible."), ) +IMPORT_NOT_SUPPORTED = pd( + "http://palaceproject.io/terms/problem/import-not-supported", + status_code=400, + title=_("Import not supported"), + detail=_("The collection's protocol does not support import."), +) + COLLECTION_DOES_NOT_SUPPORT_REGISTRATION = pd( "http://librarysimplified.org/terms/problem/collection-does-not-support-registration", status_code=400, diff --git a/src/palace/manager/api/admin/routes.py b/src/palace/manager/api/admin/routes.py index 817fa71e65..dd8c03c300 100644 --- a/src/palace/manager/api/admin/routes.py +++ b/src/palace/manager/api/admin/routes.py @@ -418,6 +418,16 @@ def collection(collection_id): ) +@app.route("/admin/collection//import", methods=["POST"]) +@returns_json_or_response_or_problem_detail +@requires_admin +@requires_csrf_token +def collection_import(collection_id): + return app.manager.admin_collection_settings_controller.process_import( + int(collection_id) + ) + + @app.route("/admin/collection_self_tests/", methods=["GET", "POST"]) @returns_json_or_response_or_problem_detail @requires_admin diff --git a/src/palace/manager/api/circulation/base.py b/src/palace/manager/api/circulation/base.py index 1efc9c4232..bcb92c7095 100644 --- a/src/palace/manager/api/circulation/base.py +++ b/src/palace/manager/api/circulation/base.py @@ -5,7 +5,7 @@ from abc import ABC, abstractmethod from collections.abc import Iterable, Mapping from functools import cached_property -from typing import TypedDict, Unpack +from typing import Any, TypedDict, Unpack from celery.canvas import Signature from flask_babel import lazy_gettext as _ @@ -207,6 +207,16 @@ class FulfillKwargs(TypedDict, total=False): exponent: str | None """The exponent part of the RSA public key used for DRM.""" + @classmethod + def protocol_details(cls, db: Session) -> dict[str, Any]: + """Include ``supports_import`` so the admin UI knows whether to + offer an import button for collections using this protocol.""" + details = super().protocol_details(db) + details["supports_import"] = ( + cls.import_task is not BaseCirculationAPI.import_task + ) + return details + @classmethod def import_task(cls, collection_id: int, force: bool = False) -> Signature: """ diff --git a/tests/manager/api/admin/controller/test_collections.py b/tests/manager/api/admin/controller/test_collections.py index b23b3f5ec5..00cf27b26b 100644 --- a/tests/manager/api/admin/controller/test_collections.py +++ b/tests/manager/api/admin/controller/test_collections.py @@ -15,8 +15,10 @@ CANNOT_CHANGE_PROTOCOL, CANNOT_DELETE_COLLECTION_WITH_CHILDREN, FAILED_TO_RUN_SELF_TESTS, + IMPORT_NOT_SUPPORTED, INCOMPLETE_CONFIGURATION, INTEGRATION_NAME_ALREADY_IN_USE, + MISSING_COLLECTION, MISSING_IDENTIFIER, MISSING_PARENT, MISSING_SERVICE, @@ -1004,3 +1006,112 @@ def collection(self) -> None: mock.assert_called_once_with(db.session, collection) mock()._run_self_tests.assert_called_once_with(db.session) assert mock().store_self_test_results.call_count == 1 + + def test_process_import_requires_system_admin( + self, + controller: CollectionSettingsController, + flask_app_fixture: FlaskAppFixture, + db: DatabaseTransactionFixture, + ): + collection = db.collection(protocol=BoundlessApi) + assert collection.integration_configuration.id is not None + with flask_app_fixture.test_request_context("/", method="POST"): + pytest.raises( + AdminNotAuthorized, + controller.process_import, + collection.integration_configuration.id, + ) + + def test_process_import_missing_service( + self, + controller: CollectionSettingsController, + flask_app_fixture: FlaskAppFixture, + ): + with flask_app_fixture.test_request_context_system_admin("/", method="POST"): + response = controller.process_import(999999) + assert response == MISSING_SERVICE + + def test_process_import_marked_for_deletion( + self, + controller: CollectionSettingsController, + flask_app_fixture: FlaskAppFixture, + db: DatabaseTransactionFixture, + ): + collection = db.collection(protocol=BoundlessApi) + collection.marked_for_deletion = True + assert collection.integration_configuration.id is not None + with flask_app_fixture.test_request_context_system_admin("/", method="POST"): + response = controller.process_import( + collection.integration_configuration.id + ) + assert response == MISSING_COLLECTION + + def test_process_import_unsupported_protocol( + self, + controller: CollectionSettingsController, + flask_app_fixture: FlaskAppFixture, + db: DatabaseTransactionFixture, + ): + collection = db.collection(protocol=OverdriveAPI) + assert collection.integration_configuration.id is not None + with flask_app_fixture.test_request_context_system_admin("/", method="POST"): + flask.request.form = ImmutableMultiDict({}) + response = controller.process_import( + collection.integration_configuration.id + ) + assert response == IMPORT_NOT_SUPPORTED + + def test_process_import_success( + self, + controller: CollectionSettingsController, + flask_app_fixture: FlaskAppFixture, + db: DatabaseTransactionFixture, + ): + collection = db.collection(protocol=BoundlessApi) + assert collection.integration_configuration.id is not None + with ( + flask_app_fixture.test_request_context_system_admin("/", method="POST"), + patch.object(boundless, "import_collection") as mock_import, + ): + flask.request.form = ImmutableMultiDict({}) + response = controller.process_import( + collection.integration_configuration.id + ) + assert isinstance(response, Response) + assert response.status_code == 200 + assert response.get_data(as_text=True) == "Import task queued." + mock_import.s.assert_called_once_with(collection.id, import_all=False) + mock_import.s.return_value.apply_async.assert_called_once() + + def test_process_import_with_force( + self, + controller: CollectionSettingsController, + flask_app_fixture: FlaskAppFixture, + db: DatabaseTransactionFixture, + ): + collection = db.collection(protocol=BoundlessApi) + assert collection.integration_configuration.id is not None + with ( + flask_app_fixture.test_request_context_system_admin("/", method="POST"), + patch.object(boundless, "import_collection") as mock_import, + ): + flask.request.form = ImmutableMultiDict({"force": "true"}) + response = controller.process_import( + collection.integration_configuration.id + ) + assert isinstance(response, Response) + assert response.status_code == 200 + mock_import.s.assert_called_once_with(collection.id, import_all=True) + mock_import.s.return_value.apply_async.assert_called_once() + + def test_protocol_details_supports_import( + self, + db: DatabaseTransactionFixture, + ): + # BoundlessApi overrides import_task, so supports_import should be True. + boundless_details = BoundlessApi.protocol_details(db.session) + assert boundless_details["supports_import"] is True + + # OverdriveAPI does not override import_task, so supports_import should be False. + overdrive_details = OverdriveAPI.protocol_details(db.session) + assert overdrive_details["supports_import"] is False From 046de01d77f2e1b411aef5efe0baa53b25ab89ca Mon Sep 17 00:00:00 2001 From: Jonathan Green Date: Fri, 20 Feb 2026 11:57:24 -0400 Subject: [PATCH 2/5] Fix collection import support detection and error handling --- .../admin/controller/collection_settings.py | 7 +++++- src/palace/manager/api/admin/routes.py | 8 ++++++- src/palace/manager/api/circulation/base.py | 5 ++-- .../api/admin/controller/test_collections.py | 14 +++++++++++ tests/manager/api/admin/test_routes.py | 23 +++++++++++++++++++ 5 files changed, 53 insertions(+), 4 deletions(-) diff --git a/src/palace/manager/api/admin/controller/collection_settings.py b/src/palace/manager/api/admin/controller/collection_settings.py index 751a3eea7e..8346f0b154 100644 --- a/src/palace/manager/api/admin/controller/collection_settings.py +++ b/src/palace/manager/api/admin/controller/collection_settings.py @@ -17,6 +17,7 @@ MISSING_PARENT, MISSING_SERVICE, PROTOCOL_DOES_NOT_SUPPORT_PARENTS, + UNKNOWN_PROTOCOL, ) from palace.manager.api.admin.util.flask import get_request_admin from palace.manager.api.circulation.base import CirculationApiType @@ -220,7 +221,11 @@ def process_import(self, collection_id: int) -> Response | ProblemDetail: if collection.marked_for_deletion: return MISSING_COLLECTION - impl_cls = self.registry[integration.protocol] + protocol = integration.protocol + if protocol not in self.registry: + return UNKNOWN_PROTOCOL + + impl_cls = self.registry[protocol] force = flask.request.form.get("force", "false").lower() == "true" try: diff --git a/src/palace/manager/api/admin/routes.py b/src/palace/manager/api/admin/routes.py index dd8c03c300..d8cf8d36d2 100644 --- a/src/palace/manager/api/admin/routes.py +++ b/src/palace/manager/api/admin/routes.py @@ -15,6 +15,7 @@ from palace.manager.api.admin.problem_details import ( ADMIN_NOT_AUTHORIZED, INVALID_ADMIN_CREDENTIALS, + MISSING_SERVICE, ) from palace.manager.api.app import app from palace.manager.api.controller.static_file import StaticFileController @@ -423,8 +424,13 @@ def collection(collection_id): @requires_admin @requires_csrf_token def collection_import(collection_id): + try: + integration_id = int(collection_id) + except ValueError: + return MISSING_SERVICE + return app.manager.admin_collection_settings_controller.process_import( - int(collection_id) + integration_id ) diff --git a/src/palace/manager/api/circulation/base.py b/src/palace/manager/api/circulation/base.py index bcb92c7095..be120fa0fb 100644 --- a/src/palace/manager/api/circulation/base.py +++ b/src/palace/manager/api/circulation/base.py @@ -212,9 +212,10 @@ def protocol_details(cls, db: Session) -> dict[str, Any]: """Include ``supports_import`` so the admin UI knows whether to offer an import button for collections using this protocol.""" details = super().protocol_details(db) - details["supports_import"] = ( - cls.import_task is not BaseCirculationAPI.import_task + import_task_owner = next( + base for base in cls.__mro__ if "import_task" in base.__dict__ ) + details["supports_import"] = import_task_owner is not BaseCirculationAPI return details @classmethod diff --git a/tests/manager/api/admin/controller/test_collections.py b/tests/manager/api/admin/controller/test_collections.py index 00cf27b26b..369eef965c 100644 --- a/tests/manager/api/admin/controller/test_collections.py +++ b/tests/manager/api/admin/controller/test_collections.py @@ -1046,6 +1046,20 @@ def test_process_import_marked_for_deletion( ) assert response == MISSING_COLLECTION + def test_process_import_unknown_protocol( + self, + controller: CollectionSettingsController, + flask_app_fixture: FlaskAppFixture, + db: DatabaseTransactionFixture, + ): + collection = db.collection(protocol=BoundlessApi) + integration = collection.integration_configuration + integration.protocol = "UnknownProtocol" + assert integration.id is not None + with flask_app_fixture.test_request_context_system_admin("/", method="POST"): + response = controller.process_import(integration.id) + assert response == UNKNOWN_PROTOCOL + def test_process_import_unsupported_protocol( self, controller: CollectionSettingsController, diff --git a/tests/manager/api/admin/test_routes.py b/tests/manager/api/admin/test_routes.py index 65ab57d73d..7b2b345773 100644 --- a/tests/manager/api/admin/test_routes.py +++ b/tests/manager/api/admin/test_routes.py @@ -21,6 +21,7 @@ ADMIN_NOT_AUTHORIZED, INVALID_ADMIN_CREDENTIALS, INVALID_CSRF_TOKEN, + MISSING_SERVICE, ) from palace.manager.api.controller.circulation_manager import ( CirculationManagerController, @@ -701,6 +702,28 @@ def test_process_post(self, fixture: AdminRouteFixture): ) fixture.assert_supported_methods(url, "DELETE") + def test_process_import(self, fixture: AdminRouteFixture): + url = "/admin/collection/123/import" + fixture.assert_authenticated_request_calls( + url, + fixture.controller.process_import, + 123, + http_method="POST", + ) + fixture.assert_supported_methods(url, "POST") + + def test_process_import_invalid_collection_id(self, fixture: AdminRouteFixture): + fixture.manager.admin_sign_in_controller.authenticated = True + try: + response = fixture.request("/admin/collection/not-a-number/import", "POST") + finally: + fixture.manager.admin_sign_in_controller.authenticated = False + + body, status_code, _ = response + assert status_code == MISSING_SERVICE.status_code + payload = json.loads(body) + assert payload["type"] == MISSING_SERVICE.uri + def test_process_collection_self_tests(self, fixture: AdminRouteFixture): url = "/admin/collection_self_tests/" fixture.assert_authenticated_request_calls( From c2ca8941a1b7b3c10bb6b163709065f0942513f9 Mon Sep 17 00:00:00 2001 From: Jonathan Green Date: Fri, 20 Feb 2026 13:19:09 -0400 Subject: [PATCH 3/5] Rename to service_id --- .../manager/api/admin/controller/collection_settings.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/palace/manager/api/admin/controller/collection_settings.py b/src/palace/manager/api/admin/controller/collection_settings.py index 8346f0b154..b4df5a4a62 100644 --- a/src/palace/manager/api/admin/controller/collection_settings.py +++ b/src/palace/manager/api/admin/controller/collection_settings.py @@ -197,10 +197,10 @@ def process_delete(self, service_id: int) -> Response | ProblemDetail: collection_delete.delay(collection.id) return Response("Deleted", 200) - def process_import(self, collection_id: int) -> Response | ProblemDetail: + def process_import(self, service_id: int) -> Response | ProblemDetail: """Queue a collection import task on demand. - :param collection_id: The integration configuration ID of the collection to import. + :param service_id: The integration configuration ID of the collection to import. :return: A 200 response on success, or a ProblemDetail on error. """ self.require_system_admin() @@ -208,7 +208,7 @@ def process_import(self, collection_id: int) -> Response | ProblemDetail: integration = get_one( self._db, IntegrationConfiguration, - id=collection_id, + id=service_id, goal=self.registry.goal, ) if not integration: From a3e1e539103c2822a74c1f5bed2cc52b86c15ce6 Mon Sep 17 00:00:00 2001 From: Jonathan Green Date: Fri, 20 Feb 2026 13:34:53 -0400 Subject: [PATCH 4/5] Replace MRO introspection with SupportsImport mixin The fragile __mro__ walk in protocol_details and try/except NotImplementedError pattern in the controller are replaced with a SupportsImport ABC mixin and issubclass checks, following the existing HasSelfTests mixin precedent. --- .../admin/controller/collection_settings.py | 13 ++++----- src/palace/manager/api/circulation/base.py | 29 ++++++++++--------- .../integration/license/boundless/api.py | 2 ++ .../license/opds/for_distributors/api.py | 3 +- .../integration/license/opds/odl/api.py | 3 +- .../integration/license/opds/opds1/api.py | 5 +++- .../integration/license/opds/opds2/api.py | 6 ++-- .../api/admin/controller/test_collections.py | 4 +-- 8 files changed, 36 insertions(+), 29 deletions(-) diff --git a/src/palace/manager/api/admin/controller/collection_settings.py b/src/palace/manager/api/admin/controller/collection_settings.py index b4df5a4a62..9af3eaa1b4 100644 --- a/src/palace/manager/api/admin/controller/collection_settings.py +++ b/src/palace/manager/api/admin/controller/collection_settings.py @@ -20,7 +20,7 @@ UNKNOWN_PROTOCOL, ) from palace.manager.api.admin.util.flask import get_request_admin -from palace.manager.api.circulation.base import CirculationApiType +from palace.manager.api.circulation.base import CirculationApiType, SupportsImport from palace.manager.celery.tasks.collection_delete import collection_delete from palace.manager.celery.tasks.reaper import ( reap_unassociated_holds, @@ -147,14 +147,11 @@ def process_post(self) -> Response | ProblemDetail: # If we have an importer task for this protocol, we start it # in the background, so that the collection is ready to go # as quickly as possible. - try: + if issubclass(impl_cls, SupportsImport): impl_cls.import_task(integration.collection.id).apply_async( # Delay the task to ensure the collection has been created by the time the task starts countdown=10 ) - except NotImplementedError: - # If the protocol does not support import tasks, we just skip it. - ... except ProblemDetailException as e: self._db.rollback() @@ -228,11 +225,11 @@ def process_import(self, service_id: int) -> Response | ProblemDetail: impl_cls = self.registry[protocol] force = flask.request.form.get("force", "false").lower() == "true" - try: - impl_cls.import_task(collection.id, force=force).apply_async() - except NotImplementedError: + if not issubclass(impl_cls, SupportsImport): return IMPORT_NOT_SUPPORTED + impl_cls.import_task(collection.id, force=force).apply_async() + return Response("Import task queued.", 200) def process_collection_self_tests( diff --git a/src/palace/manager/api/circulation/base.py b/src/palace/manager/api/circulation/base.py index be120fa0fb..fee8aa6132 100644 --- a/src/palace/manager/api/circulation/base.py +++ b/src/palace/manager/api/circulation/base.py @@ -64,6 +64,20 @@ def internal_format(self, delivery_mechanism: LicensePoolDeliveryMechanism) -> s return internal_format +class SupportsImport(ABC): + """Mixin for circulation APIs that support collection import.""" + + @classmethod + @abstractmethod + def import_task(cls, collection_id: int, force: bool = False) -> Signature: + """Return the Celery task signature for importing the collection. + + :param collection_id: The ID of the collection to import. + :param force: If True, the import will be forced even if it has already been done. + """ + ... + + class BaseCirculationAPI[ SettingsType: BaseCirculationApiSettings, LibrarySettingsType: BaseSettings ]( @@ -212,22 +226,9 @@ def protocol_details(cls, db: Session) -> dict[str, Any]: """Include ``supports_import`` so the admin UI knows whether to offer an import button for collections using this protocol.""" details = super().protocol_details(db) - import_task_owner = next( - base for base in cls.__mro__ if "import_task" in base.__dict__ - ) - details["supports_import"] = import_task_owner is not BaseCirculationAPI + details["supports_import"] = issubclass(cls, SupportsImport) return details - @classmethod - def import_task(cls, collection_id: int, force: bool = False) -> Signature: - """ - Return the signature for a Celery task that will import the collection. - - :param collection_id: The ID of the collection to import. - :param force: If True, the import will be forced even if it has already been done. - """ - raise NotImplementedError() - @property @abstractmethod def data_source(self) -> DataSource: diff --git a/src/palace/manager/integration/license/boundless/api.py b/src/palace/manager/integration/license/boundless/api.py index efe170cb43..83ffdc09ac 100644 --- a/src/palace/manager/integration/license/boundless/api.py +++ b/src/palace/manager/integration/license/boundless/api.py @@ -14,6 +14,7 @@ from palace.manager.api.circulation.base import ( BaseCirculationAPI, PatronActivityCirculationAPI, + SupportsImport, ) from palace.manager.api.circulation.data import HoldInfo, LoanInfo from palace.manager.api.circulation.exceptions import ( @@ -77,6 +78,7 @@ class BoundlessApi( PatronActivityCirculationAPI[BoundlessSettings, BoundlessLibrarySettings], HasCollectionSelfTests, + SupportsImport, ): SET_DELIVERY_MECHANISM_AT = BaseCirculationAPI.BORROW_STEP diff --git a/src/palace/manager/integration/license/opds/for_distributors/api.py b/src/palace/manager/integration/license/opds/for_distributors/api.py index 75415f2b74..1ad0a22b40 100644 --- a/src/palace/manager/integration/license/opds/for_distributors/api.py +++ b/src/palace/manager/integration/license/opds/for_distributors/api.py @@ -9,7 +9,7 @@ from celery.canvas import Signature from sqlalchemy.orm import Session -from palace.manager.api.circulation.base import BaseCirculationAPI +from palace.manager.api.circulation.base import BaseCirculationAPI, SupportsImport from palace.manager.api.circulation.data import HoldInfo, LoanInfo from palace.manager.api.circulation.exceptions import ( CannotFulfill, @@ -46,6 +46,7 @@ class OPDSForDistributorsAPI( BaseCirculationAPI[OPDSForDistributorsSettings, OPDSForDistributorsLibrarySettings], HasCollectionSelfTests, + SupportsImport, ): @classmethod def settings_class(cls) -> type[OPDSForDistributorsSettings]: diff --git a/src/palace/manager/integration/license/opds/odl/api.py b/src/palace/manager/integration/license/opds/odl/api.py index a50dc1b02a..6cd0bedb06 100644 --- a/src/palace/manager/integration/license/opds/odl/api.py +++ b/src/palace/manager/integration/license/opds/odl/api.py @@ -14,7 +14,7 @@ from sqlalchemy.orm import Session from uritemplate import URITemplate -from palace.manager.api.circulation.base import BaseCirculationAPI +from palace.manager.api.circulation.base import BaseCirculationAPI, SupportsImport from palace.manager.api.circulation.data import HoldInfo, LoanInfo from palace.manager.api.circulation.exceptions import ( AlreadyCheckedOut, @@ -84,6 +84,7 @@ class OPDS2WithODLApi( BaseCirculationAPI[OPDS2WithODLSettings, OPDS2WithODLLibrarySettings], + SupportsImport, ): """ODL (Open Distribution to Libraries) is a specification that allows libraries to manage their own loans and holds. It offers a deeper level diff --git a/src/palace/manager/integration/license/opds/opds1/api.py b/src/palace/manager/integration/license/opds/opds1/api.py index 1553ca8527..95adfa9928 100644 --- a/src/palace/manager/integration/license/opds/opds1/api.py +++ b/src/palace/manager/integration/license/opds/opds1/api.py @@ -2,6 +2,7 @@ from celery.canvas import Signature +from palace.manager.api.circulation.base import SupportsImport from palace.manager.integration.license.opds.base.api import BaseOPDSAPI from palace.manager.integration.license.opds.opds1.settings import ( OPDSImporterLibrarySettings, @@ -9,7 +10,9 @@ ) -class OPDSAPI(BaseOPDSAPI[OPDSImporterSettings, OPDSImporterLibrarySettings]): +class OPDSAPI( + BaseOPDSAPI[OPDSImporterSettings, OPDSImporterLibrarySettings], SupportsImport +): @classmethod def settings_class(cls) -> type[OPDSImporterSettings]: return OPDSImporterSettings diff --git a/src/palace/manager/integration/license/opds/opds2/api.py b/src/palace/manager/integration/license/opds/opds2/api.py index 8c2f0ec14b..1cf1e406ef 100644 --- a/src/palace/manager/integration/license/opds/opds2/api.py +++ b/src/palace/manager/integration/license/opds/opds2/api.py @@ -10,7 +10,7 @@ from sqlalchemy.orm import Session from uritemplate import URITemplate -from palace.manager.api.circulation.base import BaseCirculationAPI +from palace.manager.api.circulation.base import BaseCirculationAPI, SupportsImport from palace.manager.api.circulation.exceptions import CannotFulfill from palace.manager.api.circulation.fulfillment import RedirectFulfillment from palace.manager.integration.license.opds.base.api import BaseOPDSAPI @@ -48,7 +48,9 @@ class TemplateVariable(StrEnum): SUPPORTED_TEMPLATE_VARIABLES: frozenset[str] = frozenset(TemplateVariable) -class OPDS2API(BaseOPDSAPI[OPDS2ImporterSettings, OPDS2ImporterLibrarySettings]): +class OPDS2API( + BaseOPDSAPI[OPDS2ImporterSettings, OPDS2ImporterLibrarySettings], SupportsImport +): TOKEN_AUTH_CONFIG_KEY = "token_auth_endpoint" LAST_REAP_TIME_KEY = "last_reap_time" diff --git a/tests/manager/api/admin/controller/test_collections.py b/tests/manager/api/admin/controller/test_collections.py index 369eef965c..a43c07e1b8 100644 --- a/tests/manager/api/admin/controller/test_collections.py +++ b/tests/manager/api/admin/controller/test_collections.py @@ -1122,10 +1122,10 @@ def test_protocol_details_supports_import( self, db: DatabaseTransactionFixture, ): - # BoundlessApi overrides import_task, so supports_import should be True. + # BoundlessApi implements SupportsImport, so supports_import should be True. boundless_details = BoundlessApi.protocol_details(db.session) assert boundless_details["supports_import"] is True - # OverdriveAPI does not override import_task, so supports_import should be False. + # OverdriveAPI does not implement SupportsImport, so supports_import should be False. overdrive_details = OverdriveAPI.protocol_details(db.session) assert overdrive_details["supports_import"] is False From 67d4b291167d3c5d20efd7df17cd16ab99153e78 Mon Sep 17 00:00:00 2001 From: Jonathan Green Date: Fri, 20 Feb 2026 14:00:01 -0400 Subject: [PATCH 5/5] Fix tests that mocked import_task on OverdriveAPI OverdriveAPI does not implement SupportsImport, so process_post skips the import step entirely. Remove the now-unnecessary mocks and assertions. --- .../api/admin/controller/test_collections.py | 25 ++----------------- 1 file changed, 2 insertions(+), 23 deletions(-) diff --git a/tests/manager/api/admin/controller/test_collections.py b/tests/manager/api/admin/controller/test_collections.py index a43c07e1b8..2241bb572c 100644 --- a/tests/manager/api/admin/controller/test_collections.py +++ b/tests/manager/api/admin/controller/test_collections.py @@ -395,12 +395,7 @@ def test_collections_post_create( short_name="L3", ) - with ( - flask_app_fixture.test_request_context_system_admin("/", method="POST"), - patch.object( - OverdriveAPI, "import_task", autospec=True - ) as mock_import_task, - ): + with flask_app_fixture.test_request_context_system_admin("/", method="POST"): flask.request.form = ImmutableMultiDict( [ ("name", "New Collection"), @@ -446,10 +441,6 @@ def test_collections_post_create( ] ) - # We queued up an import task for the new collection. - mock_import_task.assert_called_once_with(collection.id) - mock_import_task.return_value.apply_async.assert_called_once() - # Two libraries now have access to the collection. assert [collection] == l1.associated_collections assert [collection] == l2.associated_collections @@ -525,15 +516,7 @@ def test_collections_post_edit( short_name="L1", ) - with ( - flask_app_fixture.test_request_context_system_admin("/", method="POST"), - patch.object( - OverdriveAPI, - "import_task", - autospec=True, - side_effect=NotImplementedError, - ) as mock_import_task, - ): + with flask_app_fixture.test_request_context_system_admin("/", method="POST"): flask.request.form = ImmutableMultiDict( [ ("id", str(collection.integration_configuration.id)), @@ -556,10 +539,6 @@ def test_collections_post_edit( assert collection.integration_configuration.id == int(response.get_data()) - # We tried to queue up an import task for the collection, but it failed - # with a NotImplementedError. This is expected and silently handled. - mock_import_task.assert_called_once_with(collection.id) - # The collection has been changed. assert "user2" == collection.integration_configuration.settings_dict.get( "overdrive_client_key"