From 81efa9d39792aaf4338592df8aef78330dfd4a6e Mon Sep 17 00:00:00 2001 From: Jonathan Green Date: Thu, 3 Oct 2024 14:12:49 -0300 Subject: [PATCH] Update collection db fixture --- src/palace/manager/api/enki.py | 1 - .../manager/sqlalchemy/model/integration.py | 7 +- tests/fixtures/database.py | 127 ++++++++++------- tests/fixtures/odl.py | 26 ++-- .../api/admin/controller/test_collections.py | 55 ++++---- .../api/admin/controller/test_report.py | 20 +-- tests/manager/api/controller/test_loan.py | 40 +++--- tests/manager/api/odl/test_api.py | 27 +--- tests/manager/api/odl/test_importer.py | 32 ++--- tests/manager/api/odl/test_reaper.py | 7 - tests/manager/api/test_circulationapi.py | 5 +- tests/manager/api/test_enki.py | 39 +++--- tests/manager/api/test_lanes.py | 6 +- .../manager/api/test_opds_for_distributors.py | 98 ++++--------- tests/manager/api/test_overdrive.py | 72 +++++----- ...est_generate_inventory_and_hold_reports.py | 13 +- tests/manager/core/test_coverage.py | 15 +- tests/manager/core/test_monitor.py | 6 +- tests/manager/core/test_opds2_import.py | 23 ++-- tests/manager/core/test_opds_import.py | 129 +++++++----------- tests/manager/core/test_opds_validate.py | 15 +- tests/manager/feed/test_annotators.py | 30 ++-- .../feed/test_loan_and_hold_annotator.py | 2 +- tests/manager/scripts/test_configuration.py | 2 +- tests/manager/scripts/test_informational.py | 4 +- tests/manager/scripts/test_monitor.py | 2 +- tests/manager/scripts/test_opds_import.py | 9 +- .../sqlalchemy/model/test_collection.py | 111 +++++++-------- tests/mocks/enki.py | 24 +--- tests/mocks/opds_for_distributors.py | 29 ---- 30 files changed, 412 insertions(+), 564 deletions(-) diff --git a/src/palace/manager/api/enki.py b/src/palace/manager/api/enki.py index fb13bc6c88..25ac8b0176 100644 --- a/src/palace/manager/api/enki.py +++ b/src/palace/manager/api/enki.py @@ -112,7 +112,6 @@ class EnkiAPI( HasCollectionSelfTests, EnkiConstants, ): - ENKI_LIBRARY_ID_KEY = "enki_library_id" DESCRIPTION = _("Integrate an Enki collection.") list_endpoint = "ListAPI" diff --git a/src/palace/manager/sqlalchemy/model/integration.py b/src/palace/manager/sqlalchemy/model/integration.py index 40c5552fe0..3a9ecf1d70 100644 --- a/src/palace/manager/sqlalchemy/model/integration.py +++ b/src/palace/manager/sqlalchemy/model/integration.py @@ -130,9 +130,12 @@ def explain(self, include_secrets: bool = False) -> list[str]: def process_settings_dict( settings_dict: dict[str, Any], indent: int = 0 ) -> None: - secret_keys = ["key", "password", "token"] + secret_keys = ["key", "password", "token", "secret"] for setting_key, setting_value in sorted(settings_dict.items()): - if setting_key in secret_keys and not include_secrets: + if ( + any(secret_key in setting_key for secret_key in secret_keys) + and not include_secrets + ): setting_value = "********" lines.append(" " * indent + f"{setting_key}: {setting_value}") diff --git a/tests/fixtures/database.py b/tests/fixtures/database.py index a249e3c04f..f7405d88cf 100644 --- a/tests/fixtures/database.py +++ b/tests/fixtures/database.py @@ -1,5 +1,6 @@ from __future__ import annotations +import functools import importlib import logging import shutil @@ -20,14 +21,23 @@ from sqlalchemy import MetaData, create_engine, text from sqlalchemy.engine import Connection, Engine, Transaction, make_url from sqlalchemy.orm import Session, sessionmaker +from sqlalchemy.orm.attributes import flag_modified from typing_extensions import Self from palace.manager.api.authentication.base import AuthenticationProvider from palace.manager.api.authentication.base import SettingsType as TAuthProviderSettings +from palace.manager.api.circulation import ( + BaseCirculationAPI, + BaseCirculationApiSettings, +) +from palace.manager.api.circulation import SettingsType as TCirculationSettings from palace.manager.api.discovery.opds_registration import ( OpdsRegistrationService, OpdsRegistrationServiceSettings, ) +from palace.manager.api.odl.api import OPDS2WithODLApi +from palace.manager.api.odl.settings import OPDS2WithODLSettings +from palace.manager.api.overdrive import OverdriveAPI, OverdriveSettings from palace.manager.api.simple_authentication import ( SimpleAuthenticationProvider, SimpleAuthSettings, @@ -35,7 +45,8 @@ from palace.manager.core.classifier import Classifier from palace.manager.core.config import Configuration from palace.manager.core.exceptions import BasePalaceException, PalaceValueError -from palace.manager.core.opds_import import OPDSAPI +from palace.manager.core.opds2_import import OPDS2API +from palace.manager.core.opds_import import OPDSAPI, OPDSImporterSettings from palace.manager.integration.base import ( HasIntegrationConfiguration, HasLibraryIntegrationConfiguration, @@ -408,9 +419,8 @@ def _make_default_library(self) -> Library: library = self.library("default", "default") collection = self.collection( "Default Collection", - protocol=OPDSAPI.label(), - data_source_name="OPDS", - external_account_id="http://opds.example.com/feed", + protocol=OPDSAPI, + settings=self.opds_settings(data_source="OPDS"), ) collection.libraries.append(library) return library @@ -530,34 +540,73 @@ def library( ) return library + opds_settings = functools.partial( + OPDSImporterSettings, + external_account_id="http://opds.example.com/feed", + data_source="OPDS", + ) + + overdrive_settings = functools.partial( + OverdriveSettings, + external_account_id="library_id", + overdrive_website_id="website_id", + overdrive_client_key="client_key", + overdrive_client_secret="client_secret", + overdrive_server_nickname="production", + ) + + opds2_odl_settings = functools.partial( + OPDS2WithODLSettings, + username="username", + password="password", + external_account_id="http://example.com/feed", + data_source=DataSource.FEEDBOOKS, + ) + + def collection_settings( + self, protocol: type[BaseCirculationAPI[TCirculationSettings, Any]] + ) -> TCirculationSettings | None: + if protocol in [OPDSAPI, OPDS2API]: + return self.opds_settings() # type: ignore[return-value] + elif protocol == OverdriveAPI: + return self.overdrive_settings() # type: ignore[return-value] + elif protocol == OPDS2WithODLApi: + return self.opds2_odl_settings() # type: ignore[return-value] + return None + def collection( self, - name=None, - protocol=OPDSAPI.label(), - external_account_id=None, - url=None, - username=None, - password=None, - data_source_name=None, - settings: dict[str, Any] | None = None, + name: str | None = None, + *, + protocol: type[BaseCirculationAPI[Any, Any]] | str = OPDSAPI, + settings: BaseCirculationApiSettings | dict[str, Any] | None = None, library: Library | None = None, ) -> Collection: name = name or self.fresh_str() - collection, _ = Collection.by_name_and_protocol(self.session, name, protocol) - settings = settings or {} - if url: - settings["url"] = url - if username: - settings["username"] = username - if password: - settings["password"] = password - if external_account_id: - settings["external_account_id"] = external_account_id - collection.integration_configuration.settings_dict = settings - - if data_source_name: - collection.data_source = data_source_name - if library: + protocol_str = ( + protocol + if isinstance(protocol, str) + else self._goal_registry_mapping[Goals.LICENSE_GOAL].get_protocol(protocol) + ) + assert protocol_str is not None + collection, _ = Collection.by_name_and_protocol( + self.session, name, protocol_str + ) + + if settings is None and not isinstance(protocol, str): + settings = self.collection_settings(protocol) + + if isinstance(settings, BaseCirculationApiSettings): + if isinstance(protocol, str): + raise PalaceValueError( + "protocol must be a subclass of BaseCirculationAPI to set settings" + ) + protocol.settings_update(collection.integration_configuration, settings) + elif isinstance(settings, dict): + collection.integration_configuration.settings_dict = settings + flag_modified(collection.integration_configuration, "settings_dict") + + if library and library not in collection.libraries: collection.libraries.append(library) return collection @@ -933,6 +982,11 @@ def _goal_registry_mapping(self) -> Mapping[Goals, IntegrationRegistry[Any]]: Goals.PATRON_AUTH_GOAL: self._services.services.integration_registry.patron_auth(), } + def protocol_string( + self, goal: Goals, protocol: type[BaseCirculationAPI[Any, Any]] + ) -> str: + return self._goal_registry_mapping[goal].get_protocol(protocol, False) + def integration_configuration( self, protocol: type[HasIntegrationConfiguration[TIntegrationSettings]] | str, @@ -1046,25 +1100,6 @@ def simple_auth_integration( ), ) - @classmethod - def set_settings( - cls, - config: IntegrationConfiguration | IntegrationLibraryConfiguration, - *keyvalues, - **kwargs, - ): - settings = config.settings_dict.copy() - - # Alternating key: value in the args - for ix, item in enumerate(keyvalues): - if ix % 2 == 0: - key = item - else: - settings[key] = item - - settings.update(kwargs) - config.settings_dict = settings - def work_coverage_record( self, work, operation=None, status=CoverageRecord.SUCCESS ) -> WorkCoverageRecord: diff --git a/tests/fixtures/odl.py b/tests/fixtures/odl.py index de9591c283..dd860d4ba3 100644 --- a/tests/fixtures/odl.py +++ b/tests/fixtures/odl.py @@ -52,26 +52,18 @@ def __init__( def create_work(self, collection: Collection) -> Work: return self.db.work(with_license_pool=True, collection=collection) - @staticmethod - def default_collection_settings() -> dict[str, Any]: - return { - "username": "a", - "password": "b", - "external_account_id": "http://odl", - Collection.DATA_SOURCE_NAME_SETTING: "Feedbooks", - } - def create_collection(self, library: Library) -> Collection: - collection, _ = Collection.by_name_and_protocol( - self.db.session, + return self.db.collection( f"Test {OPDS2WithODLApi.__name__} Collection", - OPDS2WithODLApi.label(), - ) - collection.integration_configuration.settings_dict = ( - self.default_collection_settings() + protocol=OPDS2WithODLApi, + library=library, + settings=self.db.opds2_odl_settings( + username="a", + password="b", + external_account_id="http://odl", + data_source="Feedbooks", + ), ) - collection.libraries.append(library) - return collection def setup_license( self, diff --git a/tests/manager/api/admin/controller/test_collections.py b/tests/manager/api/admin/controller/test_collections.py index 5998c0cb07..a3586c4d22 100644 --- a/tests/manager/api/admin/controller/test_collections.py +++ b/tests/manager/api/admin/controller/test_collections.py @@ -28,9 +28,10 @@ ) from palace.manager.api.axis import Axis360API from palace.manager.api.odl.api import OPDS2WithODLApi -from palace.manager.api.overdrive import OverdriveAPI +from palace.manager.api.overdrive import OverdriveAPI, OverdriveLibrarySettings from palace.manager.api.selftest import HasCollectionSelfTests from palace.manager.core.selftest import HasSelfTests +from palace.manager.integration.goals import Goals from palace.manager.service.integration_registry.license_providers import ( LicenseProvidersRegistry, ) @@ -117,28 +118,31 @@ def test_collections_get_collections_with_multiple_collections( c2 = db.collection( name="Collection 2", - protocol=OverdriveAPI.label(), - external_account_id="1234", - settings=dict( + protocol=OverdriveAPI, + settings=db.overdrive_settings( overdrive_client_secret="b", overdrive_client_key="user", overdrive_website_id="100", + external_account_id="1234", ), ) c3 = db.collection( name="Collection 3", - protocol=OverdriveAPI.label(), - external_account_id="5678", + protocol=OverdriveAPI, + settings=db.overdrive_settings( + external_account_id="5678", + ), ) c3.parent = c2 l1 = db.library(short_name="L1") c3.libraries += [l1, db.default_library()] - assert isinstance(l1.id, int) - l1_config = c3.integration_configuration.for_library(l1.id) - assert l1_config is not None - DatabaseTransactionFixture.set_settings(l1_config, ebook_loan_duration="14") + db.integration_library_configuration( + c3.integration_configuration, + l1, + OverdriveLibrarySettings(ebook_loan_duration=14), + ) admin = flask_app_fixture.admin_user() l1_librarian = flask_app_fixture.admin_user( @@ -187,7 +191,7 @@ def test_collections_get_collections_with_multiple_collections( coll3_libraries, key=lambda x: x.get("short_name") ) assert "L1" == coll3_l1.get("short_name") - assert "14" == coll3_l1.get("ebook_loan_duration") + assert 14 == coll3_l1.get("ebook_loan_duration") assert db.default_library().short_name == coll3_default.get("short_name") with flask_app_fixture.test_request_context("/", admin=l1_librarian): @@ -203,7 +207,7 @@ def test_collections_get_collections_with_multiple_collections( coll3_libraries = coll3.get("libraries") assert 1 == len(coll3_libraries) assert "L1" == coll3_libraries[0].get("short_name") - assert "14" == coll3_libraries[0].get("ebook_loan_duration") + assert 14 == coll3_libraries[0].get("ebook_loan_duration") @pytest.mark.parametrize( "post_data,expected,detailed", @@ -323,7 +327,7 @@ def test_collections_post_errors( expected: ProblemDetail, detailed: bool, ): - collection = db.collection(name="Collection 1", protocol=OverdriveAPI.label()) + collection = db.collection(name="Collection 1", protocol=OverdriveAPI) if "id" in post_data and post_data["id"] == "": post_data["id"] = str(collection.integration_configuration.id) @@ -489,7 +493,7 @@ def test_collections_post_edit( db: DatabaseTransactionFixture, ): # The collection exists. - collection = db.collection(name="Collection 1", protocol=OverdriveAPI.label()) + collection = db.collection(name="Collection 1", protocol=OverdriveAPI) l1 = db.library( name="Library 1", @@ -501,7 +505,7 @@ def test_collections_post_edit( [ ("id", str(collection.integration_configuration.id)), ("name", "Collection 1"), - ("protocol", OverdriveAPI.label()), + ("protocol", db.protocol_string(Goals.LICENSE_GOAL, OverdriveAPI)), ("external_account_id", "1234"), ("overdrive_client_key", "user2"), ("overdrive_client_secret", "password"), @@ -572,14 +576,14 @@ def test_collections_post_edit( # All settings for that library and collection have been deleted. assert collection.integration_configuration.library_configurations == [] - parent = db.collection(name="Parent", protocol=OverdriveAPI.label()) + parent = db.collection(name="Parent", protocol=OverdriveAPI) with flask_app_fixture.test_request_context_system_admin("/", method="POST"): flask.request.form = ImmutableMultiDict( [ ("id", str(collection.integration_configuration.id)), ("name", "Collection 1"), - ("protocol", OverdriveAPI.label()), + ("protocol", db.protocol_string(Goals.LICENSE_GOAL, OverdriveAPI)), ("parent_id", str(parent.integration_configuration.id)), ("external_account_id", "1234"), ("libraries", json.dumps([])), @@ -595,15 +599,16 @@ def test_collections_post_edit( assert parent == collection.parent library = db.default_library() - collection2 = db.collection( - name="Collection 2", protocol=OPDS2WithODLApi.label() - ) + collection2 = db.collection(name="Collection 2", protocol=OPDS2WithODLApi) with flask_app_fixture.test_request_context_system_admin("/", method="POST"): flask.request.form = ImmutableMultiDict( [ ("id", str(collection2.integration_configuration.id)), ("name", "Collection 2"), - ("protocol", OPDS2WithODLApi.label()), + ( + "protocol", + db.protocol_string(Goals.LICENSE_GOAL, OPDS2WithODLApi), + ), ("external_account_id", "http://test.com/feed"), ("username", "user"), ("password", "password"), @@ -643,7 +648,7 @@ def test_collections_post_edit_library_specific_configuration( db: DatabaseTransactionFixture, ): # The collection exists. - collection = db.collection(name="Collection 1", protocol=Axis360API.label()) + collection = db.collection(name="Collection 1", protocol=Axis360API) l1 = db.library( name="Library 1", @@ -655,7 +660,7 @@ def test_collections_post_edit_library_specific_configuration( [ ("id", str(collection.integration_configuration.id)), ("name", "Collection 1"), - ("protocol", Axis360API.label()), + ("protocol", db.protocol_string(Goals.LICENSE_GOAL, Axis360API)), ("external_account_id", "1234"), ("username", "user2"), ("password", "password"), @@ -747,8 +752,8 @@ def test_collection_delete_cant_delete_parent( flask_app_fixture: FlaskAppFixture, db: DatabaseTransactionFixture, ): - parent = db.collection(protocol=OverdriveAPI.label()) - child = db.collection(protocol=OverdriveAPI.label()) + parent = db.collection(protocol=OverdriveAPI) + child = db.collection(protocol=OverdriveAPI) child.parent = parent with flask_app_fixture.test_request_context_system_admin("/", method="DELETE"): diff --git a/tests/manager/api/admin/controller/test_report.py b/tests/manager/api/admin/controller/test_report.py index a017ab1f07..92998a2898 100644 --- a/tests/manager/api/admin/controller/test_report.py +++ b/tests/manager/api/admin/controller/test_report.py @@ -1,6 +1,7 @@ import logging from http import HTTPStatus from types import SimpleNamespace +from typing import Any from unittest.mock import MagicMock, patch import pytest @@ -12,6 +13,7 @@ InventoryReportInfo, ) from palace.manager.api.admin.problem_details import ADMIN_NOT_AUTHORIZED +from palace.manager.api.circulation import BaseCirculationAPI from palace.manager.api.overdrive import OverdriveAPI from palace.manager.api.problem_details import LIBRARY_NOT_FOUND from palace.manager.core.opds_import import OPDSAPI @@ -110,8 +112,7 @@ def test_generate_report_authorization( ) collection = db.collection( - protocol=OPDSAPI.label(), - settings={"data_source": "test", "external_account_id": "http://url"}, + protocol=OPDSAPI, ) collection.libraries = [library1, library2] @@ -176,8 +177,7 @@ def test_inventory_report_info( ) collection = db.collection( - protocol=OPDSAPI.label(), - settings={"data_source": "test", "external_account_id": "http://url"}, + protocol=OPDSAPI, ) collection.libraries = [library1, library2] @@ -229,13 +229,13 @@ def test_inventory_report_info( "protocol, settings, parent_settings, expect_collection", ( ( - OPDSAPI.label(), + OPDSAPI, {"data_source": "test", "external_account_id": "http://url"}, None, True, ), ( - OPDSAPI.label(), + OPDSAPI, { "include_in_inventory_report": False, "data_source": "test", @@ -245,7 +245,7 @@ def test_inventory_report_info( False, ), ( - OPDSAPI.label(), + OPDSAPI, { "include_in_inventory_report": True, "data_source": "test", @@ -255,7 +255,7 @@ def test_inventory_report_info( True, ), ( - OverdriveAPI.label(), + OverdriveAPI, { "overdrive_website_id": "test", "overdrive_client_key": "test", @@ -265,7 +265,7 @@ def test_inventory_report_info( False, ), ( - OverdriveAPI.label(), + OverdriveAPI, {"external_account_id": "test"}, { "overdrive_website_id": "test", @@ -281,7 +281,7 @@ def test_inventory_report_info_reportable_collections( report_fixture: ReportControllerFixture, db: DatabaseTransactionFixture, flask_app_fixture: FlaskAppFixture, - protocol: str, + protocol: type[BaseCirculationAPI[Any, Any]], settings: dict, parent_settings: dict, expect_collection: bool, diff --git a/tests/manager/api/controller/test_loan.py b/tests/manager/api/controller/test_loan.py index 5e0d892f83..7ac1e726cf 100644 --- a/tests/manager/api/controller/test_loan.py +++ b/tests/manager/api/controller/test_loan.py @@ -1,6 +1,7 @@ import datetime from collections.abc import Generator from decimal import Decimal +from typing import Any from unittest.mock import MagicMock, create_autospec, patch from urllib.parse import quote @@ -1565,7 +1566,7 @@ def test_loans_refresh( None, None, Collection.STANDARD_DEFAULT_LOAN_PERIOD, - OPDSAPI.label(), + OPDSAPI, None, None, ], # DB and OPDS response loan duration mismatch @@ -1574,7 +1575,7 @@ def test_loans_refresh( Collection.STANDARD_DEFAULT_LOAN_PERIOD - 1, Collection.STANDARD_DEFAULT_LOAN_PERIOD - 1, Collection.STANDARD_DEFAULT_LOAN_PERIOD - 1, - OPDSAPI.label(), + OPDSAPI, None, None, ], @@ -1583,7 +1584,7 @@ def test_loans_refresh( Collection.STANDARD_DEFAULT_LOAN_PERIOD + 1, Collection.STANDARD_DEFAULT_LOAN_PERIOD + 1, Collection.STANDARD_DEFAULT_LOAN_PERIOD + 1, - OPDSAPI.label(), + OPDSAPI, None, None, ], @@ -1592,7 +1593,7 @@ def test_loans_refresh( None, None, Collection.STANDARD_DEFAULT_LOAN_PERIOD - 2, - BibliothecaAPI.label(), + BibliothecaAPI, DataSource.BIBLIOTHECA, Collection.STANDARD_DEFAULT_LOAN_PERIOD - 2, ], # DB and OPDS response loan duration mismatch @@ -1601,7 +1602,7 @@ def test_loans_refresh( Collection.STANDARD_DEFAULT_LOAN_PERIOD - 3, Collection.STANDARD_DEFAULT_LOAN_PERIOD - 3, Collection.STANDARD_DEFAULT_LOAN_PERIOD - 3, - BibliothecaAPI.label(), + BibliothecaAPI, DataSource.BIBLIOTHECA, Collection.STANDARD_DEFAULT_LOAN_PERIOD - 2, ], @@ -1610,7 +1611,7 @@ def test_loans_refresh( Collection.STANDARD_DEFAULT_LOAN_PERIOD - 1, Collection.STANDARD_DEFAULT_LOAN_PERIOD - 1, Collection.STANDARD_DEFAULT_LOAN_PERIOD - 1, - BibliothecaAPI.label(), + BibliothecaAPI, DataSource.BIBLIOTHECA, Collection.STANDARD_DEFAULT_LOAN_PERIOD - 2, ], @@ -1619,7 +1620,7 @@ def test_loans_refresh( Collection.STANDARD_DEFAULT_LOAN_PERIOD + 1, Collection.STANDARD_DEFAULT_LOAN_PERIOD + 1, Collection.STANDARD_DEFAULT_LOAN_PERIOD + 1, - BibliothecaAPI.label(), + BibliothecaAPI, DataSource.BIBLIOTHECA, Collection.STANDARD_DEFAULT_LOAN_PERIOD - 2, ], @@ -1628,7 +1629,7 @@ def test_loans_refresh( None, None, Collection.STANDARD_DEFAULT_LOAN_PERIOD + 2, - BibliothecaAPI.label(), + BibliothecaAPI, DataSource.BIBLIOTHECA, Collection.STANDARD_DEFAULT_LOAN_PERIOD + 2, ], # DB and OPDS response loan duration mismatch @@ -1637,7 +1638,7 @@ def test_loans_refresh( Collection.STANDARD_DEFAULT_LOAN_PERIOD - 1, Collection.STANDARD_DEFAULT_LOAN_PERIOD - 1, Collection.STANDARD_DEFAULT_LOAN_PERIOD - 1, - BibliothecaAPI.label(), + BibliothecaAPI, DataSource.BIBLIOTHECA, Collection.STANDARD_DEFAULT_LOAN_PERIOD + 2, ], @@ -1646,7 +1647,7 @@ def test_loans_refresh( Collection.STANDARD_DEFAULT_LOAN_PERIOD + 1, Collection.STANDARD_DEFAULT_LOAN_PERIOD + 1, Collection.STANDARD_DEFAULT_LOAN_PERIOD + 1, - BibliothecaAPI.label(), + BibliothecaAPI, DataSource.BIBLIOTHECA, Collection.STANDARD_DEFAULT_LOAN_PERIOD + 2, ], @@ -1655,7 +1656,7 @@ def test_loans_refresh( Collection.STANDARD_DEFAULT_LOAN_PERIOD + 3, Collection.STANDARD_DEFAULT_LOAN_PERIOD + 3, Collection.STANDARD_DEFAULT_LOAN_PERIOD + 3, - BibliothecaAPI.label(), + BibliothecaAPI, DataSource.BIBLIOTHECA, Collection.STANDARD_DEFAULT_LOAN_PERIOD + 2, ], @@ -1667,7 +1668,7 @@ def test_loan_duration_settings_impact_on_loans_and_borrow_response( target_loan_duration: int, db_loan_duration: int, opds_response_loan_duration: int, - collection_protocol: str, + collection_protocol: type[BaseCirculationAPI[Any, Any]], collection_data_source_name: str, collection_default_loan_period: int, ): @@ -1684,19 +1685,16 @@ def test_loan_duration_settings_impact_on_loans_and_borrow_response( collection = loan_fixture.db.collection( protocol=collection_protocol, - data_source_name=collection_data_source_name, ) collection.libraries.append(loan_fixture.db.default_library()) if collection_default_loan_period: - lib_config = collection.integration_configuration.for_library( - loan_fixture.db.default_library() - ) - assert lib_config is not None - DatabaseTransactionFixture.set_settings( - lib_config, - collection.loan_period_key(), - collection_default_loan_period, + loan_fixture.db.integration_library_configuration( + collection.integration_configuration, + library=loan_fixture.db.default_library(), + settings=collection_protocol.library_settings_class()( + ebook_loan_duration=collection_default_loan_period + ), ) def create_work_and_return_license_pool_and_loan_info(**kwargs): diff --git a/tests/manager/api/odl/test_api.py b/tests/manager/api/odl/test_api.py index 9b558f7d5f..117329cf56 100644 --- a/tests/manager/api/odl/test_api.py +++ b/tests/manager/api/odl/test_api.py @@ -37,9 +37,8 @@ ) from palace.manager.api.odl.api import OPDS2WithODLApi from palace.manager.api.odl.constants import FEEDBOOKS_AUDIO -from palace.manager.api.odl.settings import OPDS2AuthType +from palace.manager.api.odl.settings import OPDS2AuthType, OPDS2WithODLLibrarySettings from palace.manager.sqlalchemy.constants import MediaTypes -from palace.manager.sqlalchemy.model.collection import Collection from palace.manager.sqlalchemy.model.licensing import ( DeliveryMechanism, LicensePool, @@ -1197,20 +1196,13 @@ def test_update_hold_end_date( library = hold.patron.library # Set the reservation period and loan period. - config = ( - opds2_with_odl_api_fixture.collection.integration_configuration.for_library( - library.id - ) - ) - assert config is not None - DatabaseTransactionFixture.set_settings( - config, - **{ - Collection.DEFAULT_RESERVATION_PERIOD_KEY: 3, - Collection.EBOOK_LOAN_DURATION_KEY: 6, - }, + db.integration_library_configuration( + opds2_with_odl_api_fixture.collection.integration_configuration, + library=library, + settings=OPDS2WithODLLibrarySettings( + default_reservation_period=3, ebook_loan_duration=6 + ), ) - opds2_with_odl_api_fixture.db.session.commit() # A hold that's already reserved and has an end date doesn't change. info.end_date = tomorrow @@ -1448,11 +1440,6 @@ def test_update_hold_queue( ) -> None: licenses = [opds2_with_odl_api_fixture.license] - DatabaseTransactionFixture.set_settings( - opds2_with_odl_api_fixture.collection.integration_configuration, - **{Collection.DEFAULT_RESERVATION_PERIOD_KEY: 3}, - ) - # If there's no holds queue when we try to update the queue, it # will remove a reserved license and make it available instead. opds2_with_odl_api_fixture.pool.licenses_owned = 1 diff --git a/tests/manager/api/odl/test_importer.py b/tests/manager/api/odl/test_importer.py index 46a7565b30..3bec21ea04 100644 --- a/tests/manager/api/odl/test_importer.py +++ b/tests/manager/api/odl/test_importer.py @@ -19,7 +19,7 @@ OPDS2WithODLImporter, OPDS2WithODLImportMonitor, ) -from palace.manager.api.odl.settings import OPDS2AuthType +from palace.manager.api.odl.settings import OPDS2AuthType, OPDS2WithODLSettings from palace.manager.core.coverage import CoverageFailure from palace.manager.core.metadata_layer import LicenseData from palace.manager.sqlalchemy.constants import ( @@ -71,14 +71,9 @@ def test_import( ) opds2_with_odl_importer_fixture.queue_response(moby_dick_license) - - config = opds2_with_odl_importer_fixture.collection.integration_configuration opds2_with_odl_importer_fixture.importer.ignored_identifier_types = [ IdentifierConstants.URI ] - DatabaseTransactionFixture.set_settings( - config, skipped_license_formats=["text/html"] - ) # Act ( @@ -212,11 +207,6 @@ def test_import_audiobook_with_streaming( """Ensure that OPDSWithODLImporter correctly processes and imports a feed with an audiobook.""" opds2_with_odl_importer_fixture.queue_fixture_file("license-audiobook.json") - db.set_settings( - opds2_with_odl_importer_fixture.collection.integration_configuration, - skipped_license_formats=["text/html"], - ) - ( imported_editions, pools, @@ -318,6 +308,7 @@ def test_import_audiobook_no_streaming( ) def test_import_open_access( self, + db: DatabaseTransactionFixture, opds2_with_odl_importer_fixture: OPDS2WithODLImporterFixture, auth_type: OPDS2AuthType, ) -> None: @@ -326,8 +317,7 @@ def test_import_open_access( open access book. """ importer = opds2_with_odl_importer_fixture.importer - importer.settings = importer.settings_class()( - **opds2_with_odl_importer_fixture.api_fixture.default_collection_settings(), + importer.settings = db.opds2_odl_settings( auth_type=auth_type, ) ( @@ -377,6 +367,7 @@ def test_import_open_access( ) def test_import_unlimited_access( self, + db: DatabaseTransactionFixture, opds2_with_odl_importer_fixture: OPDS2WithODLImporterFixture, auth_type: OPDS2AuthType, ) -> None: @@ -385,8 +376,7 @@ def test_import_unlimited_access( unlimited access book. """ importer = opds2_with_odl_importer_fixture.importer - importer.settings = importer.settings_class()( - **opds2_with_odl_importer_fixture.api_fixture.default_collection_settings(), + importer.settings = db.opds2_odl_settings( auth_type=auth_type, ) ( @@ -1126,11 +1116,13 @@ def __init__(self, db: DatabaseTransactionFixture): self.username = "username" self.password = "password" self.collection = db.collection( - external_account_id=self.feed_url, - username=self.username, - password=self.password, - data_source_name="OPDS", - protocol=OPDS2WithODLApi.label(), + protocol=OPDS2WithODLApi, + settings=OPDS2WithODLSettings( + external_account_id=self.feed_url, + username=self.username, + password=self.password, + data_source="OPDS", + ), ) self.monitor = OPDS2WithODLImportMonitor( db.session, self.collection, OPDS2WithODLImporter diff --git a/tests/manager/api/odl/test_reaper.py b/tests/manager/api/odl/test_reaper.py index d1f6d02536..865ae6ad98 100644 --- a/tests/manager/api/odl/test_reaper.py +++ b/tests/manager/api/odl/test_reaper.py @@ -3,8 +3,6 @@ import datetime from palace.manager.api.odl.reaper import OPDS2WithODLHoldReaper -from palace.manager.sqlalchemy.model.collection import Collection -from palace.manager.sqlalchemy.model.datasource import DataSource from palace.manager.sqlalchemy.model.patron import Hold from palace.manager.util.datetime_helpers import utc_now from tests.fixtures.database import DatabaseTransactionFixture @@ -25,11 +23,6 @@ def test_run_once( api = opds2_with_odl_api_fixture.api pool = license.license_pool - data_source = DataSource.lookup(db.session, "Feedbooks", autocreate=True) - DatabaseTransactionFixture.set_settings( - collection.integration_configuration, - **{Collection.DATA_SOURCE_NAME_SETTING: data_source.name}, - ) reaper = OPDS2WithODLHoldReaper(db.session, collection, api=api) now = utc_now() diff --git a/tests/manager/api/test_circulationapi.py b/tests/manager/api/test_circulationapi.py index 9a3db75839..8437b1822a 100644 --- a/tests/manager/api/test_circulationapi.py +++ b/tests/manager/api/test_circulationapi.py @@ -802,10 +802,7 @@ def test_borrow_hold_limit_reached( def test_fulfill_errors(self, circulation_api: CirculationAPIFixture): # Here's an open-access title. - collection = circulation_api.db.collection( - data_source_name="OPDS", - external_account_id="http://url/", - ) + collection = circulation_api.db.collection() circulation_api.pool.open_access = True circulation_api.pool.collection = collection diff --git a/tests/manager/api/test_enki.py b/tests/manager/api/test_enki.py index e604f6efe4..ce768126b6 100644 --- a/tests/manager/api/test_enki.py +++ b/tests/manager/api/test_enki.py @@ -22,7 +22,9 @@ EnkiAPI, EnkiCollectionReaper, EnkiImport, + EnkiLibrarySettings, ) +from palace.manager.api.overdrive import OverdriveAPI from palace.manager.core.metadata_layer import CirculationData, Metadata, TimestampData from palace.manager.service.analytics.analytics import Analytics from palace.manager.sqlalchemy.model.classification import Subject @@ -58,10 +60,17 @@ class EnkiTestFixure: def __init__(self, db: DatabaseTransactionFixture, files: EnkiFilesFixture): self.db = db self.files = files - self.api = MockEnkiAPI(db.session, db.default_library()) - collection = self.api.collection - assert collection is not None - self.collection = collection + self.library = db.default_library() + self.collection = db.collection( + name="Test Enki Collection", protocol=EnkiAPI, library=self.library + ) + self.api = MockEnkiAPI(db.session, self.collection) + + db.integration_library_configuration( + self.collection.integration_configuration, + self.library, + EnkiLibrarySettings(enki_library_id="c"), + ) @pytest.fixture(scope="function") @@ -75,7 +84,7 @@ class TestEnkiAPI: def test_constructor(self, enki_test_fixture: EnkiTestFixure): db = enki_test_fixture.db # The constructor must be given an Enki collection. - collection = db.collection(protocol="Overdrive", url="http://test.enki.url") + collection = db.collection(protocol=OverdriveAPI) with pytest.raises(ValueError) as excinfo: EnkiAPI(db.session, collection) assert "Collection protocol is Overdrive, but passed into EnkiAPI!" in str( @@ -95,18 +104,11 @@ def test_enki_library_id(self, enki_test_fixture: EnkiTestFixure): # Associate another library with the mock Enki collection # and set its Enki library ID. other_library = db.library() - assert other_library.id is not None - config = enki_test_fixture.api.integration_configuration() - assert config is not None - - config.libraries.append(other_library) - lib_config = config.for_library(other_library) - assert lib_config is not None - DatabaseTransactionFixture.set_settings( - lib_config, - **{enki_test_fixture.api.ENKI_LIBRARY_ID_KEY: "other library id"}, + db.integration_library_configuration( + enki_test_fixture.collection.integration_configuration, + other_library, + EnkiLibrarySettings(enki_library_id="other library id"), ) - db.session.commit() assert "other library id" == m(other_library) def test_collection(self, enki_test_fixture: EnkiTestFixure): @@ -137,7 +139,7 @@ def patron_activity(self, patron, pin): self.patron_activity_called_with.append((patron, pin)) yield 1 - api = Mock(db.session, db.default_library()) + api = Mock(db.session, enki_test_fixture.collection) # Now let's make sure two Libraries have access to the # Collection used in the API -- one library with a default @@ -917,7 +919,6 @@ def _update_circulation(self, start, end): three_hours_ago = now - datetime.timedelta(hours=3) mock_api = MockEnkiAPI( db.session, - enki_test_fixture.db.default_library(), enki_test_fixture.collection, ) monitor = Mock(db.session, enki_test_fixture.collection, api_class=mock_api) @@ -979,7 +980,7 @@ def test__update_circulation(self, enki_test_fixture: EnkiTestFixure): } } - api = MockEnkiAPI(db.session, db.default_library()) + api = MockEnkiAPI(db.session, enki_test_fixture.collection) api.queue_response(200, content=json.dumps(circ_data)) api.queue_response(200, content=json.dumps(bib_data)) diff --git a/tests/manager/api/test_lanes.py b/tests/manager/api/test_lanes.py index ac48f5fe67..5211547487 100644 --- a/tests/manager/api/test_lanes.py +++ b/tests/manager/api/test_lanes.py @@ -1172,8 +1172,7 @@ def test_constructor(self, db: DatabaseTransactionFixture): library = db.default_library() overdrive_collection = db.collection( "Test Overdrive Collection", - protocol=OverdriveAPI.label(), - data_source_name=DataSource.OVERDRIVE, + protocol=OverdriveAPI, ) overdrive_collection.libraries.append(library) @@ -1181,8 +1180,7 @@ def test_constructor(self, db: DatabaseTransactionFixture): # library. It will not be used at all. ignored_collection = db.collection( "Ignored Collection", - protocol=BibliothecaAPI.label(), - data_source_name=DataSource.BIBLIOTHECA, + protocol=BibliothecaAPI, ) # Pass in a JackpotFacets object diff --git a/tests/manager/api/test_opds_for_distributors.py b/tests/manager/api/test_opds_for_distributors.py index 80bd546add..56be5cfec5 100644 --- a/tests/manager/api/test_opds_for_distributors.py +++ b/tests/manager/api/test_opds_for_distributors.py @@ -13,6 +13,7 @@ OPDSForDistributorsImporter, OPDSForDistributorsImportMonitor, OPDSForDistributorsReaperMonitor, + OPDSForDistributorsSettings, ) from palace.manager.api.overdrive import OverdriveAPI from palace.manager.core.metadata_layer import CirculationData, LinkData @@ -22,6 +23,7 @@ from palace.manager.sqlalchemy.model.credential import Credential from palace.manager.sqlalchemy.model.datasource import DataSource from palace.manager.sqlalchemy.model.identifier import Identifier +from palace.manager.sqlalchemy.model.library import Library from palace.manager.sqlalchemy.model.licensing import ( DeliveryMechanism, LicensePool, @@ -90,12 +92,29 @@ def __init__( self, db: DatabaseTransactionFixture, files: OPDSForDistributorsFilesFixture ): self.db = db - self.collection = MockOPDSForDistributorsAPI.mock_collection( - db.session, db.default_library() - ) + self.collection = self.mock_collection(db.default_library()) self.api = MockOPDSForDistributorsAPI(db.session, self.collection) self.files = files + def mock_collection( + self, + library: Library | None = None, + name: str = "Test OPDS For Distributors Collection", + ) -> Collection: + """Create a mock OPDS For Distributors collection to use in tests.""" + library = library or self.db.default_library() + return self.db.collection( + name, + protocol=OPDSForDistributorsAPI, + settings=OPDSForDistributorsSettings( + username="a", + password="b", + data_source="data_source", + external_account_id="http://opds", + ), + library=library, + ) + @pytest.fixture(scope="function") def opds_dist_api_fixture( @@ -260,9 +279,7 @@ def test_credentials_for_multiple_collections( feed = '' # Getting a token for a collection should result in a cached credential. - collection1 = MockOPDSForDistributorsAPI.mock_collection( - opds_dist_api_fixture.db.session, - opds_dist_api_fixture.db.default_library(), + collection1 = opds_dist_api_fixture.mock_collection( name="Collection 1", ) api1 = MockOPDSForDistributorsAPI(opds_dist_api_fixture.db.session, collection1) @@ -280,9 +297,7 @@ def test_credentials_for_multiple_collections( # Getting a token for a second collection should result in an # additional cached credential. - collection2 = MockOPDSForDistributorsAPI.mock_collection( - opds_dist_api_fixture.db.session, - opds_dist_api_fixture.db.default_library(), + collection2 = opds_dist_api_fixture.mock_collection( name="Collection 2", ) api2 = MockOPDSForDistributorsAPI(opds_dist_api_fixture.db.session, collection2) @@ -370,9 +385,7 @@ def test_checkin(self, opds_dist_api_fixture: OPDSForDistributorsAPIFixture): ) pool.loan_to(patron) - other_collection = opds_dist_api_fixture.db.collection( - protocol=OverdriveAPI.label() - ) + other_collection = opds_dist_api_fixture.db.collection(protocol=OverdriveAPI) other_edition, other_pool = opds_dist_api_fixture.db.edition( identifier_type=Identifier.OVERDRIVE_ID, data_source_name=DataSource.OVERDRIVE, @@ -496,16 +509,7 @@ class TestOPDSForDistributorsImporter: def test_import(self, opds_dist_api_fixture: OPDSForDistributorsAPIFixture): feed = opds_dist_api_fixture.files.sample_data("biblioboard_mini_feed.opds") - data_source = DataSource.lookup( - opds_dist_api_fixture.db.session, "Biblioboard", autocreate=True - ) - collection = MockOPDSForDistributorsAPI.mock_collection( - opds_dist_api_fixture.db.session, opds_dist_api_fixture.db.default_library() - ) - DatabaseTransactionFixture.set_settings( - collection.integration_configuration, - **{Collection.DATA_SOURCE_NAME_SETTING: data_source.name} - ) + collection = opds_dist_api_fixture.mock_collection() importer = OPDSForDistributorsImporter( opds_dist_api_fixture.db.session, @@ -638,32 +642,12 @@ def test__add_format_data( def test_update_work_for_edition_returns_correct_license_pool( self, opds_dist_api_fixture: OPDSForDistributorsAPIFixture ): - # If there are two or more collections, `update_work_for_edition` - # should return the license pool for the right one. - data_source_name = "BiblioBoard" - data_source = DataSource.lookup( - opds_dist_api_fixture.db.session, data_source_name, autocreate=True - ) - - def setup_collection(*, name: str, datasource: DataSource) -> Collection: - collection = MockOPDSForDistributorsAPI.mock_collection( - opds_dist_api_fixture.db.session, - opds_dist_api_fixture.db.default_library(), - name=name, - ) - DatabaseTransactionFixture.set_settings( - collection.integration_configuration, - **{Collection.DATA_SOURCE_NAME_SETTING: data_source.name} - ) - return collection - - collection1 = setup_collection(name="Test Collection 1", datasource=data_source) - collection2 = setup_collection(name="Test Collection 2", datasource=data_source) + collection1 = opds_dist_api_fixture.mock_collection(name="Test Collection 1") + collection2 = opds_dist_api_fixture.mock_collection(name="Test Collection 2") work = opds_dist_api_fixture.db.work( with_license_pool=False, collection=collection1, - data_source_name=data_source_name, ) edition = work.presentation_edition @@ -717,17 +701,7 @@ def _get(self, url, headers): 200, {"content-type": OPDSFeed.ACQUISITION_FEED_TYPE}, feed ) - data_source = DataSource.lookup( - opds_dist_api_fixture.db.session, "Biblioboard", autocreate=True - ) - collection = MockOPDSForDistributorsAPI.mock_collection( - opds_dist_api_fixture.db.session, - opds_dist_api_fixture.db.default_library(), - ) - DatabaseTransactionFixture.set_settings( - collection.integration_configuration, - **{Collection.DATA_SOURCE_NAME_SETTING: data_source.name} - ) + collection = opds_dist_api_fixture.mock_collection() monitor = MockOPDSForDistributorsReaperMonitor( opds_dist_api_fixture.db.session, collection, @@ -737,7 +711,6 @@ def _get(self, url, headers): # There's a license pool in the database that isn't in the feed anymore. edition, now_gone = opds_dist_api_fixture.db.edition( identifier_type=Identifier.URI, - data_source_name=data_source.name, with_license_pool=True, collection=collection, ) @@ -747,7 +720,6 @@ def _get(self, url, headers): edition, still_there = opds_dist_api_fixture.db.edition( identifier_type=Identifier.URI, identifier_id="urn:uuid:04377e87-ab69-41c8-a2a4-812d55dc0952", - data_source_name=data_source.name, with_license_pool=True, collection=collection, ) @@ -788,17 +760,7 @@ def _get(self, url, headers): ts = create(self._db, Timestamp) return (200, {"content-type": OPDSFeed.ACQUISITION_FEED_TYPE}, feed) - data_source = DataSource.lookup( - opds_dist_api_fixture.db.session, "Biblioboard", autocreate=True - ) - collection = MockOPDSForDistributorsAPI.mock_collection( - opds_dist_api_fixture.db.session, - opds_dist_api_fixture.db.default_library(), - ) - DatabaseTransactionFixture.set_settings( - collection.integration_configuration, - **{Collection.DATA_SOURCE_NAME_SETTING: data_source.name} - ) + collection = opds_dist_api_fixture.mock_collection() monitor = MockOPDSForDistributorsImportMonitor( opds_dist_api_fixture.db.session, collection, diff --git a/tests/manager/api/test_overdrive.py b/tests/manager/api/test_overdrive.py index ba92e3c53f..c237939010 100644 --- a/tests/manager/api/test_overdrive.py +++ b/tests/manager/api/test_overdrive.py @@ -275,27 +275,31 @@ def test_make_link_safe(self): def test_hosts(self, overdrive_api_fixture: OverdriveAPIFixture): fixture = overdrive_api_fixture - session = overdrive_api_fixture.db.session - c = OverdriveAPI - + db = fixture.db # By default, OverdriveAPI is initialized with the production # set of hostnames. - assert fixture.api.hosts() == c.HOSTS[OverdriveConstants.PRODUCTION_SERVERS] - - # You can instead initialize it to use the testing set of - # hostnames. - def api_with_setting(x): - config = fixture.collection.integration_configuration - DatabaseTransactionFixture.set_settings(config, overdrive_server_nickname=x) - return c(session, fixture.collection) + assert ( + fixture.api.hosts() + == OverdriveAPI.HOSTS[OverdriveConstants.PRODUCTION_SERVERS] + ) - testing = api_with_setting(OverdriveConstants.TESTING_SERVERS) - assert testing.hosts() == c.HOSTS[OverdriveConstants.TESTING_SERVERS] + collection = db.collection( + protocol=OverdriveAPI, + settings=db.overdrive_settings( + overdrive_server_nickname=OverdriveConstants.TESTING_SERVERS + ), + ) + testing = OverdriveAPI(db.session, collection) + assert testing.hosts() == OverdriveAPI.HOSTS[OverdriveConstants.TESTING_SERVERS] # If the setting doesn't make sense, we default to production # hostnames. - bad = api_with_setting("nonsensical") - assert bad.hosts() == c.HOSTS[OverdriveConstants.PRODUCTION_SERVERS] + collection = db.collection( + protocol=OverdriveAPI, + settings=db.overdrive_settings(overdrive_server_nickname="nonsensical"), + ) + bad = OverdriveAPI(db.session, collection) + assert bad.hosts() == OverdriveAPI.HOSTS[OverdriveConstants.PRODUCTION_SERVERS] def test_endpoint(self, overdrive_api_fixture: OverdriveAPIFixture): fixture = overdrive_api_fixture @@ -498,28 +502,22 @@ def test_401_during__refresh_token_raises_error( fixture.api._refresh_token() def test_advantage_differences(self, overdrive_api_fixture: OverdriveAPIFixture): - transaction = overdrive_api_fixture.db - session = transaction.session + db = overdrive_api_fixture.db + session = db.session # Test the differences between Advantage collections and # regular Overdrive collections. # Here's a regular Overdrive collection. - main = transaction.collection( - protocol=OverdriveAPI.label(), - external_account_id="1", - ) - DatabaseTransactionFixture.set_settings( - main.integration_configuration, "overdrive_client_key", "user" - ) - DatabaseTransactionFixture.set_settings( - main.integration_configuration, "overdrive_client_secret", "password" - ) - DatabaseTransactionFixture.set_settings( - main.integration_configuration, "overdrive_website_id", "100" - ) - DatabaseTransactionFixture.set_settings( - main.integration_configuration, "ils_name", "default" + main = db.collection( + protocol=OverdriveAPI, + settings=db.overdrive_settings( + external_account_id="1", + overdrive_client_key="user", + overdrive_client_secret="password", + overdrive_website_id="100", + ils_name="default", + ), ) # Here's an Overdrive API client for that collection. @@ -538,9 +536,9 @@ def test_advantage_differences(self, overdrive_api_fixture: OverdriveAPIFixture) # Here's an Overdrive Advantage collection associated with the # main Overdrive collection. - child = transaction.collection( - protocol=OverdriveAPI.label(), - external_account_id="2", + child = db.collection( + protocol=OverdriveAPI, + settings=db.overdrive_settings(external_account_id="2"), ) child.parent = main overdrive_child = MockOverdriveAPI(session, child) @@ -3908,8 +3906,8 @@ def test_to_collection(self, overdrive_api_fixture: OverdriveAPIFixture): # So, create a Collection to be the parent. parent = transaction.collection( name="Parent", - protocol=OverdriveAPI.label(), - external_account_id="parent_id", + protocol=OverdriveAPI, + settings=transaction.overdrive_settings(external_account_id="parent_id"), ) # Now it works. diff --git a/tests/manager/celery/tasks/test_generate_inventory_and_hold_reports.py b/tests/manager/celery/tasks/test_generate_inventory_and_hold_reports.py index b8cc641a4d..ae58808cf2 100644 --- a/tests/manager/celery/tasks/test_generate_inventory_and_hold_reports.py +++ b/tests/manager/celery/tasks/test_generate_inventory_and_hold_reports.py @@ -9,7 +9,7 @@ from pytest import LogCaptureFixture from sqlalchemy.orm import sessionmaker -from palace.manager.api.overdrive import OverdriveSettings +from palace.manager.api.overdrive import OverdriveAPI from palace.manager.celery.tasks.generate_inventory_and_hold_reports import ( GenerateInventoryAndHoldsReportsJob, generate_inventory_and_hold_reports, @@ -66,16 +66,9 @@ def test_job_run( "Another Test Collection", "AnotherOpdsDataSource", db, library, False ) - od_settings = OverdriveSettings( - overdrive_website_id="overdrive_id", - overdrive_client_key="client_key", - overdrive_client_secret="secret", - external_account_id="http://www.overdrive/id", - ) od_collection_not_to_include = db.collection( + protocol=OverdriveAPI, name="Overdrive Test Collection", - data_source_name="Overdrive", - settings=od_settings.model_dump(), ) od_collection_not_to_include.libraries = [library] @@ -287,7 +280,7 @@ def create_test_opds_collection( external_account_id="http://opds.com", data_source=data_source, ) - collection = db.collection(name=collection_name, settings=settings.model_dump()) + collection = db.collection(name=collection_name, settings=settings) collection.libraries = [library] return collection diff --git a/tests/manager/core/test_coverage.py b/tests/manager/core/test_coverage.py index fabe00c560..b132818423 100644 --- a/tests/manager/core/test_coverage.py +++ b/tests/manager/core/test_coverage.py @@ -2,6 +2,7 @@ import pytest +from palace.manager.api.axis import Axis360API from palace.manager.api.overdrive import OverdriveAPI from palace.manager.core.coverage import ( BaseCoverageProvider, @@ -823,7 +824,7 @@ def test_bulk_register_can_overwrite_existing_record_status( def test_bulk_register_with_collection(self, db: DatabaseTransactionFixture): identifier = db.identifier() provider = AlwaysSuccessfulCoverageProvider - collection = db.collection(data_source_name=DataSource.AXIS_360) + collection = db.collection(protocol=Axis360API) try: # If a DataSource or data source name is provided and @@ -1352,7 +1353,7 @@ def test_class_variables(self, db: DatabaseTransactionFixture): """Verify that class variables become appropriate instance variables. """ - collection = db.collection(protocol=OPDSAPI.label()) + collection = db.collection(protocol=OPDSAPI) provider = AlwaysSuccessfulCollectionCoverageProvider(collection) assert provider.DATA_SOURCE_NAME == provider.data_source.name @@ -1367,7 +1368,7 @@ def test_must_have_collection(self): def test_collection_protocol_must_match_class_protocol( self, db: DatabaseTransactionFixture ): - collection = db.collection(protocol=OverdriveAPI.label()) + collection = db.collection(protocol=OverdriveAPI) with pytest.raises(ValueError) as excinfo: AlwaysSuccessfulCollectionCoverageProvider(collection) assert ( @@ -1509,9 +1510,9 @@ def test_all(self, db: DatabaseTransactionFixture): objects, one for each Collection that implements the appropriate protocol. """ - opds1 = db.collection(protocol=OPDSAPI.label()) - opds2 = db.collection(protocol=OPDSAPI.label()) - overdrive = db.collection(protocol=OverdriveAPI.label()) + opds1 = db.collection(protocol=OPDSAPI) + opds2 = db.collection(protocol=OPDSAPI) + overdrive = db.collection(protocol=OverdriveAPI) providers = list( AlwaysSuccessfulCollectionCoverageProvider.all(db.session, batch_size=34) ) @@ -1783,7 +1784,7 @@ class OverdriveProvider(AlwaysSuccessfulCollectionCoverageProvider): PROTOCOL = OverdriveAPI.label() IDENTIFIER_TYPES = Identifier.OVERDRIVE_ID - collection = db.collection(protocol=OverdriveAPI.label()) + collection = db.collection(protocol=OverdriveAPI) provider = OverdriveProvider(collection) # We get a CoverageFailure if we don't pass in any data at all. diff --git a/tests/manager/core/test_monitor.py b/tests/manager/core/test_monitor.py index 165c34ca2a..bbadb504ea 100644 --- a/tests/manager/core/test_monitor.py +++ b/tests/manager/core/test_monitor.py @@ -290,8 +290,8 @@ class OverdriveMonitor(CollectionMonitor): PROTOCOL = OverdriveAPI.label() # Two collections. - c1 = db.collection(protocol=OverdriveAPI.label()) - c2 = db.collection(protocol=BibliothecaAPI.label()) + c1 = db.collection(protocol=OverdriveAPI) + c2 = db.collection(protocol=BibliothecaAPI) # The NoProtocolMonitor can be instantiated with either one, # or with no Collection at all. @@ -323,7 +323,7 @@ class OPDSCollectionMonitor(CollectionMonitor): o3 = db.collection("o3") # ...and a Bibliotheca collection. - b1 = db.collection(protocol=BibliothecaAPI.label()) + b1 = db.collection(protocol=BibliothecaAPI) # o1 just had its Monitor run. Timestamp.stamp( diff --git a/tests/manager/core/test_opds2_import.py b/tests/manager/core/test_opds2_import.py index 016da6edd4..887bebee6b 100644 --- a/tests/manager/core/test_opds2_import.py +++ b/tests/manager/core/test_opds2_import.py @@ -99,9 +99,11 @@ class OPDS2ImporterFixture: def __init__(self, db: DatabaseTransactionFixture) -> None: self.transaction = db self.collection = db.collection( - protocol=OPDS2API.label(), - data_source_name="OPDS 2.0 Data Source", - external_account_id="http://opds2.example.org/feed", + protocol=OPDS2API, + settings=db.opds_settings( + external_account_id="http://opds2.example.org/feed", + data_source="OPDS 2.0 Data Source", + ), ) self.library = db.default_library() self.collection.libraries.append(self.library) @@ -711,9 +713,11 @@ class Opds2ApiFixture: def __init__(self, db: DatabaseTransactionFixture, mock_http: MagicMock): self.patron = db.patron() self.collection: Collection = db.collection( - protocol=OPDS2API.label(), - data_source_name="test", - external_account_id="http://opds2.example.org/feed", + protocol=OPDS2API, + settings=db.opds_settings( + external_account_id="http://opds2.example.org/feed", + data_source="test", + ), ) self.collection.integration_configuration.context = { OPDS2API.TOKEN_AUTH_CONFIG_KEY: "http://example.org/token?userName={patron_id}" @@ -911,9 +915,10 @@ def test__verify_media_type( self, db: DatabaseTransactionFixture, content_type: str, exception: bool ) -> None: collection = db.collection( - protocol=OPDS2API.label(), - data_source_name="test", - external_account_id="http://test.com", + protocol=OPDS2API, + settings=db.opds_settings( + external_account_id="http://opds2.example.org/feed", + ), ) monitor = OPDS2ImportMonitor( db.session, diff --git a/tests/manager/core/test_opds_import.py b/tests/manager/core/test_opds_import.py index 7e670d7e27..41362dcf21 100644 --- a/tests/manager/core/test_opds_import.py +++ b/tests/manager/core/test_opds_import.py @@ -101,11 +101,6 @@ def __init__( self.importer = partial( OPDSImporter, _db=self.db.session, collection=self.db.default_collection() ) - db.set_settings( - db.default_collection().integration_configuration, - "data_source", - DataSource.OA_CONTENT_SERVER, - ) @pytest.fixture() @@ -236,11 +231,10 @@ def test_use_dcterm_identifier_as_id_with_id_and_dcterms_identifier( ) collection_to_test = db.collection( - settings={ - "primary_identifier_source": IdentifierSource.DCTERMS_IDENTIFIER, - }, - data_source_name="OPDS", - external_account_id="http://root.uri", + settings=db.opds_settings( + external_account_id="http://root.uri", + primary_identifier_source=IdentifierSource.DCTERMS_IDENTIFIER, + ) ) importer = opds_importer_fixture.importer(collection=collection_to_test) @@ -939,9 +933,8 @@ def test_import_with_lendability(self, opds_importer_fixture: OPDSImporterFixtur # that the licensing authority is Project Gutenberg, so that's used # as the DataSource for the first LicensePool. The information used # to create the second LicensePool didn't include a data source, - # so the source of the OPDS feed (the open-access content server) - # was used. - assert {DataSource.GUTENBERG, DataSource.OA_CONTENT_SERVER} == { + # so the source of the OPDS feed "OPDS" was used. + assert {DataSource.GUTENBERG, "OPDS"} == { pool.data_source.name for pool in pools } @@ -960,12 +953,11 @@ def test_import_with_unrecognized_distributor_creates_distributor( book is imported and both DataSources are created. """ feed = opds_files_fixture.sample_data("unrecognized_distributor.opds") - DatabaseTransactionFixture.set_settings( - db.default_collection().integration_configuration, - "data_source", - "some new source", + + collection = db.collection( + protocol=OPDSAPI, settings=db.opds_settings(data_source="some new source") ) - importer = opds_importer_fixture.importer() + importer = opds_importer_fixture.importer(collection=collection) imported_editions, pools, works, failures = importer.import_from_feed(feed) assert {} == failures @@ -1005,17 +997,16 @@ def test_import_updates_metadata( feed = feed.replace("{OVERDRIVE ID}", edition.primary_identifier.identifier) - DatabaseTransactionFixture.set_settings( - db.default_collection().integration_configuration, - "data_source", - DataSource.OVERDRIVE, + collection = db.collection( + protocol=OPDSAPI, + settings=db.opds_settings(data_source=DataSource.OVERDRIVE), ) ( imported_editions, imported_pools, imported_works, failures, - ) = opds_importer_fixture.importer().import_from_feed(feed) + ) = opds_importer_fixture.importer(collection=collection).import_from_feed(feed) # The edition we created has had its metadata updated. [new_edition] = imported_editions @@ -1062,12 +1053,8 @@ def test_import_from_license_source( assert DataSource.GUTENBERG == mouse_pool.data_source.name # But the license pool's presentation edition has a data - # source associated with the Library Simplified open-access - # content server, since that's where the metadata comes from. - assert ( - DataSource.OA_CONTENT_SERVER - == mouse_pool.presentation_edition.data_source.name - ) + # source associated with the collection. + assert "OPDS" == mouse_pool.presentation_edition.data_source.name # Since the 'mouse' book came with an open-access link, the license # pool delivery mechanism has been marked as open access. @@ -1083,9 +1070,8 @@ def test_import_from_license_source( # The OPDS feed didn't actually say where the 'crow' book # comes from, but we did tell the importer to use the open access # content server as the data source, so both a Work and a LicensePool - # were created, and their data source is the open access content server, - # not Project Gutenberg. - assert DataSource.OA_CONTENT_SERVER == crow_pool.data_source.name + # were created, and their data source is the datasoruce of the collection + assert "OPDS" == crow_pool.data_source.name def test_import_from_feed_treats_message_as_failure( self, @@ -1148,11 +1134,6 @@ def test_import_work_failure_becomes_coverage_failure( # imported edition generates a meaningful error message. feed = data.content_server_mini_feed - DatabaseTransactionFixture.set_settings( - db.default_collection().integration_configuration, - "data_source", - DataSource.OA_CONTENT_SERVER, - ) importer = DoomedWorkOPDSImporter(session, collection=db.default_collection()) imported_editions, pools, works, failures = importer.import_from_feed(feed) @@ -1441,18 +1422,14 @@ def _wayfless_circulation_api( collection = db.collection( "OPDS collection with a WAYFless acquisition link", - OPDSAPI.label(), - data_source_name="test", - external_account_id="http://wayfless.example.com/feed", + protocol=OPDSAPI, + settings=db.opds_settings( + external_account_id="http://wayfless.example.com/feed", + saml_wayfless_url_template="https://fsso.springer.com/saml/login?idp={idp}&targetUrl={targetUrl}", + ), ) collection.libraries.append(library) - DatabaseTransactionFixture.set_settings( - collection.integration_configuration, - "saml_wayfless_url_template", - "https://fsso.springer.com/saml/login?idp={idp}&targetUrl={targetUrl}", - ) - imported_editions, pools, works, failures = OPDSImporter( session, collection=collection ).import_from_feed(feed) @@ -1630,7 +1607,7 @@ def test_constructor(self, db: DatabaseTransactionFixture): "OPDSImportMonitor can only be run in the context of a Collection." in str(excinfo.value) ) - c1 = db.collection(protocol=OverdriveAPI.label()) + c1 = db.collection(protocol=OverdriveAPI) with pytest.raises(ValueError) as excinfo: OPDSImportMonitor(session, c1, OPDSImporter) assert ( @@ -1638,7 +1615,8 @@ def test_constructor(self, db: DatabaseTransactionFixture): in str(excinfo.value) ) - c2 = db.collection(protocol=OPDSAPI.label(), settings={"data_source": None}) + c2 = db.collection(protocol=OPDSAPI) + c2.integration_configuration.settings_dict = {} with pytest.raises(ValueError) as excinfo: OPDSImportMonitor(session, c2, OPDSImporter) assert f"Collection {c2.name} has no associated data source." in str( @@ -1646,11 +1624,10 @@ def test_constructor(self, db: DatabaseTransactionFixture): ) c3 = db.collection( - protocol=OPDSAPI.label(), - settings={ - "data_source": "OPDS", - "external_account_id": "https://opds.import.com/feed?size=100", - }, + protocol=OPDSAPI, + settings=db.opds_settings( + external_account_id="https://opds.import.com/feed?size=100", + ), ) monitor = OPDSImportMonitor(session, c3, OPDSImporter) assert monitor._feed_base_url == "https://opds.import.com/" @@ -1663,8 +1640,9 @@ def test_get( ## Test whether relative urls work collection = db.collection( - external_account_id="https://opds.import.com:9999/feed", - data_source_name="OPDS", + settings=db.opds_settings( + external_account_id="https://opds.import.com:9999/feed", + ), ) monitor = OPDSImportMonitor(session, collection, OPDSImporter) @@ -1684,9 +1662,7 @@ def test_hook_methods(self, db: DatabaseTransactionFixture): """By default, the OPDS URL and data source used by the importer come from the collection configuration. """ - collection = db.collection( - external_account_id="http://url/", data_source_name="OPDS" - ) + collection = db.collection() monitor = OPDSImportMonitor( db.session, collection, @@ -1715,7 +1691,8 @@ def _get(self, url, headers): data_source_name = "OPDS" collection = db.collection( - external_account_id="http://url/", data_source_name=data_source_name + protocol=OPDSAPI, + settings=db.opds_settings(data_source=data_source_name), ) monitor = OPDSImportMonitor( session, @@ -1802,7 +1779,8 @@ def test_follow_one_link(self, opds_importer_fixture: OPDSImporterFixture): ) data_source_name = "OPDS" collection = db.collection( - external_account_id="http://url/", data_source_name=data_source_name + protocol=OPDSAPI, + settings=db.opds_settings(data_source=data_source_name), ) monitor = OPDSImportMonitor( session, @@ -1877,8 +1855,10 @@ def test_import_one_feed(self, opds_importer_fixture: OPDSImporterFixture): # Check coverage records are created. data_source_name = "OPDS" collection = db.collection( - external_account_id="http://root-url/index.xml", - data_source_name=data_source_name, + settings=db.opds_settings( + external_account_id="http://root-url/index.xml", + data_source=data_source_name, + ), ) monitor = OPDSImportMonitor( session, @@ -1968,10 +1948,7 @@ def import_one_feed(self, feed): self.imports.append(feed) return [object(), object()], {"identifier": "Failure"} - data_source_name = "OPDS" - collection = db.collection( - external_account_id="http://url/", data_source_name=data_source_name - ) + collection = db.collection() monitor = MockOPDSImportMonitor( db.session, @@ -1997,10 +1974,7 @@ def import_one_feed(self, feed): assert progress.finish is None def test_update_headers(self, db: DatabaseTransactionFixture): - data_source_name = "OPDS" - collection = db.collection( - external_account_id="http://url/", data_source_name=data_source_name - ) + collection = db.collection() # Test the _update_headers helper method. monitor = OPDSImportMonitor( @@ -2053,11 +2027,11 @@ def test_retry(self, opds_importer_fixture: OPDSImporterFixture): feed = data.content_server_mini_feed feed_url = "https://example.com/feed.opds" - data_source_name = "OPDS" collection = db.collection( - external_account_id="http://url/", - data_source_name=data_source_name, - settings={"max_retry_count": retry_count}, + settings=db.opds_settings( + external_account_id=feed_url, + max_retry_count=retry_count, + ), ) # The importer takes its retry count from the collection settings. @@ -2092,9 +2066,10 @@ def __init__(self, db: DatabaseTransactionFixture): self.db = db self.session = db.session self.collection = db.collection( - protocol=OPDSAPI.label(), - data_source_name="OPDS", - external_account_id="http://opds.example.com/feed", + protocol=OPDSAPI, + settings=db.opds_settings( + external_account_id="http://opds2.example.org/feed", + ), ) self.api = OPDSAPI(self.session, self.collection) diff --git a/tests/manager/core/test_opds_validate.py b/tests/manager/core/test_opds_validate.py index 9e09de6b98..77abebc48f 100644 --- a/tests/manager/core/test_opds_validate.py +++ b/tests/manager/core/test_opds_validate.py @@ -13,7 +13,6 @@ OPDS2SchemaValidation, OPDS2WithODLSchemaValidation, ) -from palace.manager.sqlalchemy.model.datasource import DataSource from tests.fixtures.database import DatabaseTransactionFixture from tests.fixtures.files import OPDS2FilesFixture, OPDS2WithODLFilesFixture @@ -36,11 +35,7 @@ def test_opds2_schema( opds2_files_fixture: OPDS2FilesFixture, ): collection = db.collection( - protocol=OPDS2API.label(), - data_source_name=DataSource.FEEDBOOKS, - settings={ - "external_account_id": "http://example.com/feed", - }, + protocol=OPDS2API, ) validator = OPDS2SchemaValidation( db.session, @@ -72,13 +67,7 @@ def test_opds2_with_odl_schema( opds2_with_odl_files_fixture: OPDS2WithODLFilesFixture, ): collection = db.collection( - protocol=OPDS2WithODLApi.label(), - data_source_name=DataSource.FEEDBOOKS, - settings={ - "username": "username", - "password": "password", - "external_account_id": "http://example.com/feed", - }, + protocol=OPDS2WithODLApi, ) validator = OPDS2WithODLSchemaValidation( db.session, diff --git a/tests/manager/feed/test_annotators.py b/tests/manager/feed/test_annotators.py index 378489a264..db1395972f 100644 --- a/tests/manager/feed/test_annotators.py +++ b/tests/manager/feed/test_annotators.py @@ -11,14 +11,12 @@ from palace.manager.feed.annotator.verbose import VerboseAnnotator from palace.manager.feed.types import FeedEntryType, Link, WorkEntry from palace.manager.feed.util import strftime -from palace.manager.integration.configuration.formats import FormatPriorities from palace.manager.search.external_search import WorkSearchResult from palace.manager.sqlalchemy.constants import MediaTypes from palace.manager.sqlalchemy.model.classification import Subject from palace.manager.sqlalchemy.model.contributor import Contributor from palace.manager.sqlalchemy.model.datasource import DataSource from palace.manager.sqlalchemy.model.edition import Edition -from palace.manager.sqlalchemy.model.integration import IntegrationConfiguration from palace.manager.sqlalchemy.model.lane import WorkList from palace.manager.sqlalchemy.model.licensing import ( DeliveryMechanism, @@ -597,14 +595,22 @@ def test_visible_delivery_mechanisms( assert [] == list(no_epub.visible_delivery_mechanisms(pool)) def test_visible_delivery_mechanisms_configured_0( - self, circulation_fixture: CirculationManagerAnnotatorFixture + self, + circulation_fixture: CirculationManagerAnnotatorFixture, + db: DatabaseTransactionFixture, ): """Test that configuration options do affect OPDS feeds. Exhaustive testing of different configuration values isn't necessary here: See the tests for FormatProperties to see the actual semantics of the configuration values.""" - edition = circulation_fixture.db.edition() - pool: LicensePool = circulation_fixture.db.licensepool(edition) + collection = db.collection( + settings=db.opds_settings( + prioritized_drm_schemes=[DeliveryMechanism.LCP_DRM], + prioritized_content_types=[MediaTypes.PDF_MEDIA_TYPE], + ) + ) + edition = db.edition(collection=collection) + pool: LicensePool = db.licensepool(edition, collection=collection) pool.set_delivery_mechanism( MediaTypes.EPUB_MEDIA_TYPE, @@ -625,20 +631,6 @@ def test_visible_delivery_mechanisms_configured_0( None, ) - config: IntegrationConfiguration = pool.collection.integration_configuration - DatabaseTransactionFixture.set_settings( - config, - **{ - FormatPriorities.PRIORITIZED_DRM_SCHEMES_KEY: [ - f"{DeliveryMechanism.LCP_DRM}", - ], - FormatPriorities.PRIORITIZED_CONTENT_TYPES_KEY: [ - f"{MediaTypes.PDF_MEDIA_TYPE}" - ], - }, - ) - circulation_fixture.db.session.commit() - annotator = CirculationManagerAnnotator( circulation_fixture.lane, hidden_content_types=[], diff --git a/tests/manager/feed/test_loan_and_hold_annotator.py b/tests/manager/feed/test_loan_and_hold_annotator.py index 0a87f8e515..662b31fffe 100644 --- a/tests/manager/feed/test_loan_and_hold_annotator.py +++ b/tests/manager/feed/test_loan_and_hold_annotator.py @@ -213,7 +213,7 @@ def test_annotate_work_entry( feed = OPDSAcquisitionFeed("title", "url", [], annotator) # Annotate time tracking - opds_for_distributors = db.collection(protocol=OPDSForDistributorsAPI.label()) + opds_for_distributors = db.collection(protocol=OPDSForDistributorsAPI) opds_for_distributors.libraries.append(library) work = db.work(with_license_pool=True, collection=opds_for_distributors) work.active_license_pool().should_track_playtime = True diff --git a/tests/manager/scripts/test_configuration.py b/tests/manager/scripts/test_configuration.py index 6e47457189..68200c7972 100644 --- a/tests/manager/scripts/test_configuration.py +++ b/tests/manager/scripts/test_configuration.py @@ -197,7 +197,7 @@ def test_success(self, db: DatabaseTransactionFixture): def test_reconfigure_collection(self, db: DatabaseTransactionFixture): # The collection exists. - collection = db.collection(name="Collection 1", protocol=OverdriveAPI.label()) + collection = db.collection(name="Collection 1", protocol=OverdriveAPI) script = ConfigureCollectionScript() output = StringIO() diff --git a/tests/manager/scripts/test_informational.py b/tests/manager/scripts/test_informational.py index 89feff9240..a8400fcdd5 100644 --- a/tests/manager/scripts/test_informational.py +++ b/tests/manager/scripts/test_informational.py @@ -71,8 +71,8 @@ def test_with_no_collections(self, db: DatabaseTransactionFixture): assert "No collections found.\n" == output.getvalue() def test_with_multiple_collections(self, db: DatabaseTransactionFixture): - c1 = db.collection(name="Collection 1", protocol=OverdriveAPI.label()) - c2 = db.collection(name="Collection 2", protocol=BibliothecaAPI.label()) + c1 = db.collection(name="Collection 1", protocol=OverdriveAPI) + c2 = db.collection(name="Collection 2", protocol=BibliothecaAPI) # The output of this script is the result of running explain() # on both collections. diff --git a/tests/manager/scripts/test_monitor.py b/tests/manager/scripts/test_monitor.py index be6ee375b8..5ecf7ea93d 100644 --- a/tests/manager/scripts/test_monitor.py +++ b/tests/manager/scripts/test_monitor.py @@ -172,7 +172,7 @@ def test_monitors(self, db: DatabaseTransactionFixture): o3 = db.collection() # ...and a Bibliotheca collection. - b1 = db.collection(protocol=BibliothecaAPI.label()) + b1 = db.collection(protocol=BibliothecaAPI) script = RunCollectionMonitorScript( OPDSCollectionMonitor, db.session, cmd_args=[] diff --git a/tests/manager/scripts/test_opds_import.py b/tests/manager/scripts/test_opds_import.py index 74f00c796b..77bd861f72 100644 --- a/tests/manager/scripts/test_opds_import.py +++ b/tests/manager/scripts/test_opds_import.py @@ -2,8 +2,6 @@ from palace.manager.core.opds_import import OPDSImportMonitor from palace.manager.scripts.opds_import import OPDSImportScript -from palace.manager.sqlalchemy.model.collection import Collection -from palace.manager.sqlalchemy.model.datasource import DataSource from tests.fixtures.database import DatabaseTransactionFixture @@ -37,11 +35,8 @@ class MockOPDSImportScript(OPDSImportScript): class TestOPDSImportScript: def test_do_run(self, db: DatabaseTransactionFixture): - DatabaseTransactionFixture.set_settings( - db.default_collection().integration_configuration, - Collection.DATA_SOURCE_NAME_SETTING, - DataSource.OA_CONTENT_SERVER, - ) + # Create a collection to use as the default + db.default_collection() script = MockOPDSImportScript(db.session) script.do_run([]) diff --git a/tests/manager/sqlalchemy/model/test_collection.py b/tests/manager/sqlalchemy/model/test_collection.py index 77470d32c4..bcd66806df 100644 --- a/tests/manager/sqlalchemy/model/test_collection.py +++ b/tests/manager/sqlalchemy/model/test_collection.py @@ -4,7 +4,7 @@ from sqlalchemy import select from palace.manager.api.bibliotheca import BibliothecaAPI -from palace.manager.api.overdrive import OverdriveAPI +from palace.manager.api.overdrive import OverdriveAPI, OverdriveLibrarySettings from palace.manager.integration.goals import Goals from palace.manager.search.external_search import ExternalSearchIndex from palace.manager.sqlalchemy.model.circulationevent import CirculationEvent @@ -22,30 +22,18 @@ class ExampleCollectionFixture: - collection: Collection - database_fixture: DatabaseTransactionFixture - - def __init__( - self, collection: Collection, database_transaction: DatabaseTransactionFixture - ): - self.collection = collection - self.database_fixture = database_transaction - - def set_default_loan_period(self, medium, value, library=None): - config = self.collection.integration_configuration - if library is not None: - config = config.for_library(library.id) - DatabaseTransactionFixture.set_settings( - config, **{self.collection.loan_period_key(medium): value} + def __init__(self, database_transaction: DatabaseTransactionFixture): + self.collection = database_transaction.collection( + name="test collection", protocol=OverdriveAPI ) + self.database_fixture = database_transaction @pytest.fixture() def example_collection_fixture( db: DatabaseTransactionFixture, ) -> ExampleCollectionFixture: - c = db.collection(name="test collection", protocol=OverdriveAPI.label()) - return ExampleCollectionFixture(c, db) + return ExampleCollectionFixture(db) class TestCollection: @@ -113,11 +101,11 @@ def test_by_protocol(self, example_collection_fixture: ExampleCollectionFixture) db = example_collection_fixture.database_fixture test_collection = example_collection_fixture.collection - overdrive = OverdriveAPI.label() - bibliotheca = BibliothecaAPI.label() - c1 = db.collection(db.fresh_str(), protocol=overdrive) + overdrive = db.protocol_string(Goals.LICENSE_GOAL, OverdriveAPI) + bibliotheca = db.protocol_string(Goals.LICENSE_GOAL, BibliothecaAPI) + c1 = db.collection(db.fresh_str(), protocol=OverdriveAPI) c1.parent = test_collection - c2 = db.collection(db.fresh_str(), protocol=bibliotheca) + c2 = db.collection(db.fresh_str(), protocol=BibliothecaAPI) assert {test_collection, c1} == set( Collection.by_protocol(db.session, overdrive).all() ) @@ -177,17 +165,16 @@ def test_change_protocol( db = example_collection_fixture.database_fixture test_collection = example_collection_fixture.collection - overdrive = OverdriveAPI.label() - bibliotheca = BibliothecaAPI.label() + bibliotheca = db.protocol_string(Goals.LICENSE_GOAL, BibliothecaAPI) # Create a parent and a child collection, both with # protocol=Overdrive. - child = db.collection(db.fresh_str(), protocol=overdrive) + child = db.collection(db.fresh_str(), protocol=OverdriveAPI) child.parent = test_collection # We can't change the child's protocol to a value that contradicts # the parent's protocol. - child.protocol = overdrive + child.protocol = db.protocol_string(Goals.LICENSE_GOAL, OverdriveAPI) def set_child_protocol(): child.protocol = bibliotheca @@ -202,20 +189,21 @@ def set_child_protocol(): # If we change the parent's protocol, the children are # automatically updated. test_collection.protocol = bibliotheca - assert bibliotheca == child.protocol + assert child.protocol == bibliotheca def test_data_source(self, example_collection_fixture: ExampleCollectionFixture): db = example_collection_fixture.database_fixture - opds = db.collection() - bibliotheca = db.collection(protocol=BibliothecaAPI.label()) + opds = db.collection(settings=db.opds_settings(data_source="Foo")) + bibliotheca = db.collection(protocol=BibliothecaAPI) # The rote data_source is returned for the obvious collection. assert bibliotheca.data_source is not None assert DataSource.BIBLIOTHECA == bibliotheca.data_source.name - # The less obvious OPDS collection doesn't have a DataSource. - assert None == opds.data_source + # The OPDS collection has a data source derived from its settings. + assert opds.data_source is not None + assert "Foo" == opds.data_source.name # Trying to change the Bibliotheca collection's data_source does nothing. bibliotheca.data_source = DataSource.AXIS_360 # type: ignore[assignment] @@ -237,11 +225,12 @@ def test_data_source(self, example_collection_fixture: ExampleCollectionFixture) assert None == opds.data_source def test_default_loan_period( - self, example_collection_fixture: ExampleCollectionFixture + self, + example_collection_fixture: ExampleCollectionFixture, + db: DatabaseTransactionFixture, ): db = example_collection_fixture.database_fixture test_collection = example_collection_fixture.collection - library = db.default_library() test_collection.libraries.append(library) @@ -260,14 +249,14 @@ def test_default_loan_period( ) # Set a value, and it's used. - example_collection_fixture.set_default_loan_period(ebook, 604, library=library) - assert 604 == test_collection.default_loan_period(library) - assert ( - Collection.STANDARD_DEFAULT_LOAN_PERIOD - == test_collection.default_loan_period(library, audio) + db.integration_library_configuration( + test_collection.integration_configuration, + library=library, + settings=OverdriveLibrarySettings( + audio_loan_duration=606, ebook_loan_duration=604 + ), ) - - example_collection_fixture.set_default_loan_period(audio, 606, library=library) + assert 604 == test_collection.default_loan_period(library) assert 606 == test_collection.default_loan_period(library, audio) def test_default_reservation_period( @@ -287,11 +276,9 @@ def test_default_reservation_period( test_collection.default_reservation_period = 601 assert 601 == test_collection.default_reservation_period - # The underlying value is controlled by a integration setting. - DatabaseTransactionFixture.set_settings( - test_collection.integration_configuration, - Collection.DEFAULT_RESERVATION_PERIOD_KEY, - 954, + # The underlying value is controlled by an integration setting. + test_collection.integration_configuration.settings_dict.update( + {Collection.DEFAULT_RESERVATION_PERIOD_KEY: 954} ) assert 954 == test_collection.default_reservation_period @@ -337,38 +324,40 @@ def test_explain(self, example_collection_fixture: ExampleCollectionFixture): test_collection = example_collection_fixture.collection test_collection.libraries.append(library) - test_collection.integration_configuration.settings_dict = { - "url": "url", - "username": "username", - "password": "password", - "setting": "value", - "external_account_id": "id", - } - data = test_collection.explain() assert [ f"ID: {test_collection.integration_configuration.id}", "Name: test collection", "Protocol/Goal: Overdrive/Goals.LICENSE_GOAL", "Settings:", - " external_account_id: id", - " password: ********", - " setting: value", - " url: url", - " username: username", + " external_account_id: library_id", + " overdrive_client_key: ********", + " overdrive_client_secret: ********", + " overdrive_website_id: website_id", "Configured libraries:", " only one - The only library", ] == data with_password = test_collection.explain(include_secrets=True) - assert " password: password" in with_password + assert [ + f"ID: {test_collection.integration_configuration.id}", + "Name: test collection", + "Protocol/Goal: Overdrive/Goals.LICENSE_GOAL", + "Settings:", + " external_account_id: library_id", + " overdrive_client_key: client_key", + " overdrive_client_secret: client_secret", + " overdrive_website_id: website_id", + "Configured libraries:", + " only one - The only library", + ] == with_password # If the collection is the child of another collection, # its parent is mentioned. child = db.collection( name="Child", - external_account_id="id2", - protocol=OverdriveAPI.label(), + settings=dict(external_account_id="id2"), + protocol=OverdriveAPI, ) child.parent = test_collection data = child.explain() diff --git a/tests/mocks/enki.py b/tests/mocks/enki.py index 0754ce2bc6..8d1ce9fda4 100644 --- a/tests/mocks/enki.py +++ b/tests/mocks/enki.py @@ -4,37 +4,15 @@ from palace.manager.api.enki import EnkiAPI from palace.manager.sqlalchemy.model.collection import Collection -from palace.manager.sqlalchemy.model.library import Library from palace.manager.util.http import HTTP -from tests.fixtures.database import DatabaseTransactionFixture from tests.mocks.mock import MockRequestsResponse class MockEnkiAPI(EnkiAPI): - def __init__( - self, _db: Session, library: Library, collection: Collection | None = None - ) -> None: + def __init__(self, _db: Session, collection: Collection) -> None: self.responses: list[MockRequestsResponse] = [] self.requests: list[list[Any]] = [] - if not collection: - collection, ignore = Collection.by_name_and_protocol( - _db, name="Test Enki Collection", protocol=EnkiAPI.ENKI - ) - assert collection is not None - collection.protocol = EnkiAPI.ENKI - if collection not in library.collections: - collection.libraries.append(library) - - # Set the "Enki library ID" variable between the default library - # and this Enki collection. - library_config = collection.integration_configuration.for_library(library) - assert library_config is not None - DatabaseTransactionFixture.set_settings( - library_config, **{self.ENKI_LIBRARY_ID_KEY: "c"} - ) - _db.commit() - super().__init__(_db, collection) def queue_response(self, status_code, headers={}, content=None): diff --git a/tests/mocks/opds_for_distributors.py b/tests/mocks/opds_for_distributors.py index 3c2b5c72ed..02efb67d8d 100644 --- a/tests/mocks/opds_for_distributors.py +++ b/tests/mocks/opds_for_distributors.py @@ -1,38 +1,9 @@ -from sqlalchemy.orm import Session - from palace.manager.api.opds_for_distributors import OPDSForDistributorsAPI -from palace.manager.sqlalchemy.model.collection import Collection -from palace.manager.sqlalchemy.model.library import Library from palace.manager.util.http import HTTP from tests.mocks.mock import MockRequestsResponse class MockOPDSForDistributorsAPI(OPDSForDistributorsAPI): - @classmethod - def mock_collection( - self, - _db: Session, - library: Library, - name: str = "Test OPDS For Distributors Collection", - ) -> Collection: - """Create a mock OPDS For Distributors collection to use in tests. - - :param _db: Database session. - :param name: A name for the collection. - """ - collection, _ = Collection.by_name_and_protocol( - _db, name=name, protocol=OPDSForDistributorsAPI.label() - ) - collection.integration_configuration.settings_dict = dict( - username="a", - password="b", - data_source="data_source", - external_account_id="http://opds", - ) - if library not in collection.libraries: - collection.libraries.append(library) - return collection - def __init__(self, _db, collection, *args, **kwargs): self.responses = [] self.requests = []