diff --git a/alembic/versions/20241127_8dde64eab209_update_database_nullable_constraints.py b/alembic/versions/20241127_8dde64eab209_update_database_nullable_constraints.py new file mode 100644 index 000000000..90d74d4b8 --- /dev/null +++ b/alembic/versions/20241127_8dde64eab209_update_database_nullable_constraints.py @@ -0,0 +1,326 @@ +"""Update database nullable constraints + +Revision ID: 8dde64eab209 +Revises: 272da5f400de +Create Date: 2024-11-27 19:04:42.562571+00:00 + +""" + +import sqlalchemy as sa +from alembic import op +from sqlalchemy.dialects import postgresql + +# revision identifiers, used by Alembic. +revision = "8dde64eab209" +down_revision = "272da5f400de" +branch_labels = None +depends_on = None + + +def upgrade() -> None: + # We add a nullable=False constraint to these columns because they previously had a + # non-null default value but still allowed null values. None of them actually contained + # null values in production, so setting the constraint ensures that nulls cannot be + # introduced in the future. + op.alter_column("annotations", "active", existing_type=sa.BOOLEAN(), nullable=False) + op.alter_column( + "collections", "marked_for_deletion", existing_type=sa.BOOLEAN(), nullable=False + ) + op.alter_column( + "contributors", + "extra", + existing_type=postgresql.JSON(astext_type=sa.Text()), + nullable=False, + ) + op.alter_column( + "customlists", "auto_update_enabled", existing_type=sa.BOOLEAN(), nullable=False + ) + op.alter_column( + "customlists", + "auto_update_status", + existing_type=postgresql.ENUM( + "init", "updated", "repopulate", name="auto_update_status" + ), + nullable=False, + ) + op.alter_column( + "datasources", "offers_licenses", existing_type=sa.BOOLEAN(), nullable=False + ) + op.alter_column( + "datasources", + "extra", + existing_type=postgresql.JSON(astext_type=sa.Text()), + nullable=False, + ) + op.alter_column( + "deliverymechanisms", + "default_client_can_fulfill", + existing_type=sa.BOOLEAN(), + nullable=False, + ) + op.alter_column( + "editions", + "extra", + existing_type=postgresql.JSON(astext_type=sa.Text()), + nullable=False, + ) + op.alter_column("equivalents", "votes", existing_type=sa.INTEGER(), nullable=False) + op.alter_column( + "equivalents", "enabled", existing_type=sa.BOOLEAN(), nullable=False + ) + op.alter_column( + "libraries", "is_default", existing_type=sa.BOOLEAN(), nullable=False + ) + op.alter_column( + "licensepools", "superceded", existing_type=sa.BOOLEAN(), nullable=False + ) + op.alter_column( + "licensepools", "suppressed", existing_type=sa.BOOLEAN(), nullable=False + ) + op.alter_column( + "measurements", + "weight", + existing_type=postgresql.DOUBLE_PRECISION(precision=53), + nullable=False, + ) + op.alter_column( + "playtime_entries", "processed", existing_type=sa.BOOLEAN(), nullable=False + ) + op.alter_column( + "playtime_summaries", + "total_seconds_played", + existing_type=sa.INTEGER(), + nullable=False, + ) + op.alter_column( + "resources", + "voted_quality", + existing_type=postgresql.DOUBLE_PRECISION(precision=53), + nullable=False, + ) + op.alter_column( + "resources", "votes_for_quality", existing_type=sa.INTEGER(), nullable=False + ) + op.alter_column( + "resourcetransformations", + "settings", + existing_type=postgresql.JSON(astext_type=sa.Text()), + nullable=False, + ) + op.alter_column("subjects", "locked", existing_type=sa.BOOLEAN(), nullable=False) + op.alter_column("subjects", "checked", existing_type=sa.BOOLEAN(), nullable=False) + op.alter_column( + "workgenres", + "affinity", + existing_type=postgresql.DOUBLE_PRECISION(precision=53), + nullable=False, + ) + op.alter_column( + "works", "presentation_ready", existing_type=sa.BOOLEAN(), nullable=False + ) + + # These columns previously had type hints indicating they were not nullable, but the database + # schema allowed null values. We set the nullable=False constraint to match the type hint. + op.alter_column("lanes", "display_name", existing_type=sa.VARCHAR(), nullable=False) + op.alter_column( + "licensepools", "licenses_owned", existing_type=sa.INTEGER(), nullable=False + ) + op.alter_column( + "licensepools", "licenses_available", existing_type=sa.INTEGER(), nullable=False + ) + op.alter_column( + "licensepools", "licenses_reserved", existing_type=sa.INTEGER(), nullable=False + ) + op.alter_column( + "licensepools", + "patrons_in_hold_queue", + existing_type=sa.INTEGER(), + nullable=False, + ) + + # These columns had foreign key constraints that allowed null values. After verifying that they + # are never null in production, we set the nullable=False constraint to ensure that SQLAlchemy + # relationships will always return an object, rather than None. + op.alter_column( + "editions", "data_source_id", existing_type=sa.INTEGER(), nullable=False + ) + op.alter_column( + "editions", "primary_identifier_id", existing_type=sa.INTEGER(), nullable=False + ) + op.alter_column( + "equivalents", "input_id", existing_type=sa.INTEGER(), nullable=False + ) + op.alter_column( + "equivalentscoveragerecords", + "equivalency_id", + existing_type=sa.INTEGER(), + nullable=False, + ) + op.alter_column("holds", "patron_id", existing_type=sa.INTEGER(), nullable=False) + op.alter_column( + "holds", "license_pool_id", existing_type=sa.INTEGER(), nullable=False + ) + op.alter_column( + "licensepools", "data_source_id", existing_type=sa.INTEGER(), nullable=False + ) + op.alter_column( + "licensepools", "identifier_id", existing_type=sa.INTEGER(), nullable=False + ) + op.alter_column( + "licenses", "license_pool_id", existing_type=sa.INTEGER(), nullable=False + ) + op.alter_column("loans", "patron_id", existing_type=sa.INTEGER(), nullable=False) + op.alter_column( + "loans", "license_pool_id", existing_type=sa.INTEGER(), nullable=False + ) + op.alter_column( + "workgenres", "genre_id", existing_type=sa.INTEGER(), nullable=False + ) + op.alter_column("workgenres", "work_id", existing_type=sa.INTEGER(), nullable=False) + + +def downgrade() -> None: + op.alter_column( + "works", "presentation_ready", existing_type=sa.BOOLEAN(), nullable=True + ) + op.alter_column( + "workgenres", + "affinity", + existing_type=postgresql.DOUBLE_PRECISION(precision=53), + nullable=True, + ) + op.alter_column("workgenres", "work_id", existing_type=sa.INTEGER(), nullable=True) + op.alter_column("workgenres", "genre_id", existing_type=sa.INTEGER(), nullable=True) + op.alter_column("subjects", "checked", existing_type=sa.BOOLEAN(), nullable=True) + op.alter_column("subjects", "locked", existing_type=sa.BOOLEAN(), nullable=True) + op.alter_column( + "resourcetransformations", + "settings", + existing_type=postgresql.JSON(astext_type=sa.Text()), + nullable=True, + ) + op.alter_column( + "resources", "votes_for_quality", existing_type=sa.INTEGER(), nullable=True + ) + op.alter_column( + "resources", + "voted_quality", + existing_type=postgresql.DOUBLE_PRECISION(precision=53), + nullable=True, + ) + op.alter_column( + "playtime_summaries", + "total_seconds_played", + existing_type=sa.INTEGER(), + nullable=True, + ) + op.alter_column( + "playtime_entries", "processed", existing_type=sa.BOOLEAN(), nullable=True + ) + op.alter_column( + "measurements", + "weight", + existing_type=postgresql.DOUBLE_PRECISION(precision=53), + nullable=True, + ) + op.alter_column( + "loans", "license_pool_id", existing_type=sa.INTEGER(), nullable=True + ) + op.alter_column("loans", "patron_id", existing_type=sa.INTEGER(), nullable=True) + op.alter_column( + "licenses", "license_pool_id", existing_type=sa.INTEGER(), nullable=True + ) + op.alter_column( + "licensepools", + "patrons_in_hold_queue", + existing_type=sa.INTEGER(), + nullable=True, + ) + op.alter_column( + "licensepools", "licenses_reserved", existing_type=sa.INTEGER(), nullable=True + ) + op.alter_column( + "licensepools", "licenses_available", existing_type=sa.INTEGER(), nullable=True + ) + op.alter_column( + "licensepools", "licenses_owned", existing_type=sa.INTEGER(), nullable=True + ) + op.alter_column( + "licensepools", "suppressed", existing_type=sa.BOOLEAN(), nullable=True + ) + op.alter_column( + "licensepools", "superceded", existing_type=sa.BOOLEAN(), nullable=True + ) + op.alter_column( + "licensepools", "identifier_id", existing_type=sa.INTEGER(), nullable=True + ) + op.alter_column( + "licensepools", "data_source_id", existing_type=sa.INTEGER(), nullable=True + ) + op.alter_column( + "libraries", "is_default", existing_type=sa.BOOLEAN(), nullable=True + ) + op.alter_column("lanes", "display_name", existing_type=sa.VARCHAR(), nullable=True) + op.alter_column( + "holds", "license_pool_id", existing_type=sa.INTEGER(), nullable=True + ) + op.alter_column("holds", "patron_id", existing_type=sa.INTEGER(), nullable=True) + op.alter_column( + "equivalentscoveragerecords", + "equivalency_id", + existing_type=sa.INTEGER(), + nullable=True, + ) + op.alter_column("equivalents", "enabled", existing_type=sa.BOOLEAN(), nullable=True) + op.alter_column("equivalents", "votes", existing_type=sa.INTEGER(), nullable=True) + op.alter_column( + "equivalents", "input_id", existing_type=sa.INTEGER(), nullable=True + ) + op.alter_column( + "editions", + "extra", + existing_type=postgresql.JSON(astext_type=sa.Text()), + nullable=True, + ) + op.alter_column( + "editions", "primary_identifier_id", existing_type=sa.INTEGER(), nullable=True + ) + op.alter_column( + "editions", "data_source_id", existing_type=sa.INTEGER(), nullable=True + ) + op.alter_column( + "deliverymechanisms", + "default_client_can_fulfill", + existing_type=sa.BOOLEAN(), + nullable=True, + ) + op.alter_column( + "datasources", + "extra", + existing_type=postgresql.JSON(astext_type=sa.Text()), + nullable=True, + ) + op.alter_column( + "datasources", "offers_licenses", existing_type=sa.BOOLEAN(), nullable=True + ) + op.alter_column( + "customlists", + "auto_update_status", + existing_type=postgresql.ENUM( + "init", "updated", "repopulate", name="auto_update_status" + ), + nullable=True, + ) + op.alter_column( + "customlists", "auto_update_enabled", existing_type=sa.BOOLEAN(), nullable=True + ) + op.alter_column( + "contributors", + "extra", + existing_type=postgresql.JSON(astext_type=sa.Text()), + nullable=True, + ) + op.alter_column( + "collections", "marked_for_deletion", existing_type=sa.BOOLEAN(), nullable=True + ) + op.alter_column("annotations", "active", existing_type=sa.BOOLEAN(), nullable=True) diff --git a/src/palace/manager/api/admin/controller/individual_admin_settings.py b/src/palace/manager/api/admin/controller/individual_admin_settings.py index 787e1dd02..5aa19ffb5 100644 --- a/src/palace/manager/api/admin/controller/individual_admin_settings.py +++ b/src/palace/manager/api/admin/controller/individual_admin_settings.py @@ -418,7 +418,10 @@ def handle_password(self, password, admin: Admin, is_new, settingUp): # libraries then they should not be able to change the password for role in admin.roles: if not user.is_library_manager(role.library): - message = f"User is part of '{role.library.name}', you are not authorized to change their password." + library_name = ( + role.library.name if role.library else "unknown" + ) + message = f"User is part of '{library_name}', you are not authorized to change their password." break else: can_change_pw = True diff --git a/src/palace/manager/api/admin/controller/integration_settings.py b/src/palace/manager/api/admin/controller/integration_settings.py index 0ed2b28c7..6cd9e51e8 100644 --- a/src/palace/manager/api/admin/controller/integration_settings.py +++ b/src/palace/manager/api/admin/controller/integration_settings.py @@ -98,15 +98,6 @@ def configured_service_info( """This is the default implementation for getting details about a configured integration. It can be overridden by implementations that need to add additional information to the service info dict that gets returned to the admin UI.""" - - if service.goal is None: - # We should never get here, since we only query for services with a goal, and goal - # is a required field, but for mypy and safety, we check for it anyway. - self.log.warning( - f"IntegrationConfiguration {service.name}({service.id}) has no goal set. Skipping." - ) - return None - if service.protocol is None or service.protocol not in self.registry: self.log.warning( f"Unknown protocol: {service.protocol} for goal {self.registry.goal}" diff --git a/src/palace/manager/api/admin/controller/patron_auth_services.py b/src/palace/manager/api/admin/controller/patron_auth_services.py index f855a9199..fc40ab47c 100644 --- a/src/palace/manager/api/admin/controller/patron_auth_services.py +++ b/src/palace/manager/api/admin/controller/patron_auth_services.py @@ -1,6 +1,6 @@ from __future__ import annotations -from typing import Any, cast +from typing import Any import flask from flask import Response @@ -193,7 +193,4 @@ def get_library_configuration( if not (library_configurations := integration.library_configurations): return None # We sort by library id to ensure that the result is predictable. - # We cast the library id to `int`, since mypy doesn't understand the relationship. - return sorted( - library_configurations, key=lambda config: cast(int, config.library_id) - )[0] + return sorted(library_configurations, key=lambda config: config.library_id)[0] diff --git a/src/palace/manager/api/admin/dashboard_stats.py b/src/palace/manager/api/admin/dashboard_stats.py index 575b767fa..222191ebc 100644 --- a/src/palace/manager/api/admin/dashboard_stats.py +++ b/src/palace/manager/api/admin/dashboard_stats.py @@ -122,7 +122,7 @@ def _authorized_collections( self, admin: Admin, authorized_libraries: list[Library], - ) -> tuple[list[_Collection], dict[str | None, list[_Collection]], bool]: + ) -> tuple[list[_Collection], dict[str, list[_Collection]], bool]: authorized_collections: set[_Collection] = set() authorized_collections_by_library = {} diff --git a/src/palace/manager/api/admin/password_admin_authentication_provider.py b/src/palace/manager/api/admin/password_admin_authentication_provider.py index f4e3d81c2..ca9f96b20 100644 --- a/src/palace/manager/api/admin/password_admin_authentication_provider.py +++ b/src/palace/manager/api/admin/password_admin_authentication_provider.py @@ -45,10 +45,7 @@ def __init__(self, send_email: SendEmailCallable): @staticmethod def get_secret_key(db: Session) -> str: - key = Key.get_key(db, KeyType.ADMIN_SECRET_KEY, raise_exception=True).value - # We know .value is a str because its a non-null column in the DB, so - # we use an ignore to tell mypy to trust us. - return key # type: ignore[return-value] + return Key.get_key(db, KeyType.ADMIN_SECRET_KEY, raise_exception=True).value def sign_in_template(self, redirect): password_sign_in_url = url_for("password_auth") @@ -108,13 +105,6 @@ def generate_reset_password_token(self, admin: Admin, _db: Session) -> str: def send_reset_password_email(self, admin: Admin, reset_password_url: str) -> None: subject = f"{AdminClientConfig.APP_NAME} - Reset password email" - if admin.email is None: - # This should never happen, but if it does, we should log it. - self.log.error( - "Admin has no email address, cannot send reset password email." - ) - return - receivers = [admin.email] mail_text = render_template_string( diff --git a/src/palace/manager/api/authentication/access_token.py b/src/palace/manager/api/authentication/access_token.py index 83a957d2c..d6156c359 100644 --- a/src/palace/manager/api/authentication/access_token.py +++ b/src/palace/manager/api/authentication/access_token.py @@ -3,7 +3,7 @@ import uuid from dataclasses import dataclass from datetime import timedelta -from typing import TYPE_CHECKING, cast +from typing import TYPE_CHECKING from jwcrypto import jwe, jwk @@ -80,7 +80,7 @@ def generate_token( plaintext=jwe.json_encode(dict(id=patron.id, pwd=password)), protected=dict( alg="dir", - kid=uuid_encode(cast(uuid.UUID, key.id)), + kid=uuid_encode(key.id), typ="JWE", enc="A128CBC-HS256", cty=cls.CTY, diff --git a/src/palace/manager/api/authentication/basic_token.py b/src/palace/manager/api/authentication/basic_token.py index 5106bbb01..61efa9e8c 100644 --- a/src/palace/manager/api/authentication/basic_token.py +++ b/src/palace/manager/api/authentication/basic_token.py @@ -1,7 +1,7 @@ from __future__ import annotations from collections.abc import Generator -from typing import TYPE_CHECKING, cast +from typing import TYPE_CHECKING from flask import url_for from sqlalchemy.orm import Session @@ -48,7 +48,7 @@ def __init__( basic_provider: BasicAuthenticationProvider, ): self._db = _db - self.library_id = cast(int, library.id) + self.library_id = library.id # An access token provider is a companion authentication to the basic providers self.basic_provider = basic_provider diff --git a/src/palace/manager/api/authenticator.py b/src/palace/manager/api/authenticator.py index 6737a4d38..ec7547ef2 100644 --- a/src/palace/manager/api/authenticator.py +++ b/src/palace/manager/api/authenticator.py @@ -6,7 +6,6 @@ import sys from abc import ABC from collections.abc import Iterable -from typing import cast import flask import jwt @@ -139,11 +138,6 @@ def populate_authenticators( log_method=self.log.debug, message_prefix="populate_authenticators" ): for library in libraries: - if library.short_name is None: - self.log.error( - f"Library {library.name} ({library.id}) has no short name." - ) - continue self.library_authenticators[library.short_name] = ( LibraryAuthenticator.from_config(_db, library, analytics) ) @@ -262,9 +256,11 @@ def __init__( ) self.saml_providers_by_name = {} - self.bearer_token_signing_secret = bearer_token_signing_secret or cast( - str, - Key.get_key(_db, KeyType.BEARER_TOKEN_SIGNING, raise_exception=True).value, + self.bearer_token_signing_secret = ( + bearer_token_signing_secret + or Key.get_key( + _db, KeyType.BEARER_TOKEN_SIGNING, raise_exception=True + ).value ) self.initialization_exceptions: dict[ tuple[int | None, int | None], Exception @@ -307,8 +303,6 @@ def identifies_individuals(self) -> bool: @property def library(self) -> Library | None: - if self.library_id is None: - return None return Library.by_id(self._db, self.library_id) def register_provider( @@ -350,8 +344,8 @@ def register_provider( settings = impl_cls.settings_load(integration.parent) library_settings = impl_cls.library_settings_load(integration) provider = impl_cls( - self.library_id, # type: ignore[arg-type] - integration.parent_id, # type: ignore[arg-type] + self.library_id, + integration.parent_id, settings, library_settings, analytics, diff --git a/src/palace/manager/api/circulation.py b/src/palace/manager/api/circulation.py index bd1a8a07a..11d020d49 100644 --- a/src/palace/manager/api/circulation.py +++ b/src/palace/manager/api/circulation.py @@ -603,8 +603,6 @@ def __init__(self, _db: Session, collection: Collection): @property def collection(self) -> Collection | None: - if self.collection_id is None: - return None return Collection.by_id(self._db, id=self.collection_id) @classmethod @@ -889,8 +887,6 @@ def __init__( @property def library(self) -> Library | None: - if self.library_id is None: - return None return Library.by_id(self._db, self.library_id) def api_for_license_pool( diff --git a/src/palace/manager/api/enki.py b/src/palace/manager/api/enki.py index 2c86861ac..c87cf6534 100644 --- a/src/palace/manager/api/enki.py +++ b/src/palace/manager/api/enki.py @@ -165,8 +165,6 @@ def _url(self, endpoint: str) -> str: def enki_library_id(self, library: Library) -> str | None: """Find the Enki library ID for the given library.""" - if library.id is None: - return None settings = self.library_settings(library.id) if settings is None: return None diff --git a/src/palace/manager/api/odl/api.py b/src/palace/manager/api/odl/api.py index 0b0e31b60..5556484f6 100644 --- a/src/palace/manager/api/odl/api.py +++ b/src/palace/manager/api/odl/api.py @@ -64,6 +64,7 @@ LicensePoolDeliveryMechanism, ) from palace.manager.sqlalchemy.model.patron import Hold, Loan, Patron +from palace.manager.sqlalchemy.model.resource import Resource from palace.manager.sqlalchemy.util import get_one from palace.manager.util import base64 from palace.manager.util.datetime_helpers import utc_now @@ -239,6 +240,11 @@ def _checkin(self, loan: Loan) -> None: # bookshelf forever. self.log.error(f"Loan {loan.id} has no external identifier.") return + if loan.license is None: + # We can't return a loan that doesn't have a license. This should never happen but if it does, + # we log an error and continue on, so it doesn't stay on the patrons bookshelf forever. + self.log.error(f"Loan {loan.id} has no license.") + return doc = self._request_loan_status("GET", loan.external_identifier) if not doc.active: @@ -496,29 +502,33 @@ def fulfill( return self._fulfill(loan, delivery_mechanism) @staticmethod - def _check_delivery_mechanism_available( + def _get_resource_for_delivery_mechanism( requested_delivery_mechanism: DeliveryMechanism, licensepool: LicensePool - ) -> None: - fulfillment = next( + ) -> Resource: + resource = next( ( - lpdm + lpdm.resource for lpdm in licensepool.delivery_mechanisms if lpdm.delivery_mechanism == requested_delivery_mechanism + and lpdm.resource is not None ), None, ) - if fulfillment is None: + if resource is None: raise FormatNotAvailable() + return resource def _unlimited_access_fulfill( self, loan: Loan, delivery_mechanism: LicensePoolDeliveryMechanism ) -> Fulfillment: licensepool = loan.license_pool - self._check_delivery_mechanism_available( + resource = self._get_resource_for_delivery_mechanism( delivery_mechanism.delivery_mechanism, licensepool ) - content_link = delivery_mechanism.resource.representation.public_url - content_type = delivery_mechanism.resource.representation.media_type + if resource.representation is None: + raise FormatNotAvailable() + content_link = resource.representation.public_url + content_type = resource.representation.media_type return RedirectFulfillment(content_link, content_type) def _license_fulfill( @@ -571,7 +581,7 @@ def _bearer_token_fulfill( self, loan: Loan, delivery_mechanism: LicensePoolDeliveryMechanism ) -> Fulfillment: licensepool = loan.license_pool - self._check_delivery_mechanism_available( + resource = self._get_resource_for_delivery_mechanism( delivery_mechanism.delivery_mechanism, licensepool ) @@ -592,7 +602,7 @@ def _bearer_token_fulfill( token_type="Bearer", access_token=self._session_token.token, expires_in=(int((self._session_token.expires - utc_now()).total_seconds())), - location=delivery_mechanism.resource.url, + location=resource.url, ) return DirectFulfillment( diff --git a/src/palace/manager/api/opds_for_distributors.py b/src/palace/manager/api/opds_for_distributors.py index a177ff3c2..1d30b3abb 100644 --- a/src/palace/manager/api/opds_for_distributors.py +++ b/src/palace/manager/api/opds_for_distributors.py @@ -306,6 +306,8 @@ def fulfill( # Find the acquisition link with the right media type. url = None for link in links: + if link.resource.representation is None: + continue media_type = link.resource.representation.media_type if ( link.rel == Hyperlink.GENERIC_OPDS_ACQUISITION diff --git a/src/palace/manager/celery/tasks/generate_inventory_and_hold_reports.py b/src/palace/manager/celery/tasks/generate_inventory_and_hold_reports.py index 8ad049551..d53731e86 100644 --- a/src/palace/manager/celery/tasks/generate_inventory_and_hold_reports.py +++ b/src/palace/manager/celery/tasks/generate_inventory_and_hold_reports.py @@ -34,9 +34,6 @@ def eligible_integrations( """Subset a list of integrations to only those that are eligible for the inventory report.""" def is_eligible(integration: IntegrationConfiguration) -> bool: - if integration.protocol is None: - return False - settings_cls = registry[integration.protocol].settings_class() return issubclass(settings_cls, OPDSImporterSettings) diff --git a/src/palace/manager/core/opds_import.py b/src/palace/manager/core/opds_import.py index 88e70bf4d..665456778 100644 --- a/src/palace/manager/core/opds_import.py +++ b/src/palace/manager/core/opds_import.py @@ -300,7 +300,7 @@ def fulfill( delivery_mechanism: LicensePoolDeliveryMechanism, ) -> RedirectFulfillment: requested_mechanism = delivery_mechanism.delivery_mechanism - fulfillment = None + rep = None for lpdm in licensepool.delivery_mechanisms: if ( lpdm.resource is None @@ -313,15 +313,14 @@ def fulfill( if lpdm.delivery_mechanism == requested_mechanism: # We found it! This is how the patron wants # the book to be delivered. - fulfillment = lpdm + rep = lpdm.resource.representation break - if not fulfillment: + if not rep: # There is just no way to fulfill this loan the way the # patron wants. raise FormatNotAvailable() - rep = fulfillment.resource.representation content_link = rep.public_url media_type = rep.media_type diff --git a/src/palace/manager/core/selftest.py b/src/palace/manager/core/selftest.py index 69ce87f26..3796b2090 100644 --- a/src/palace/manager/core/selftest.py +++ b/src/palace/manager/core/selftest.py @@ -279,20 +279,8 @@ def store_self_test_results( @classmethod def load_self_test_results( - cls, integration: IntegrationConfiguration | None - ) -> dict[str, Any] | str | None: - if integration is None: - cls.logger().error( - "No IntegrationConfiguration was found. Self-test results could not be loaded." - ) - return None - - if not isinstance(integration.self_test_results, dict): - cls.logger().error( - "Self-test results were not stored as a dict. Self-test results could not be loaded." - ) - return None - + cls, integration: IntegrationConfiguration + ) -> dict[str, Any] | str: if integration.self_test_results == {}: # No self-test results have been stored yet. return "No results yet" diff --git a/src/palace/manager/feed/acquisition.py b/src/palace/manager/feed/acquisition.py index 9dae74cb7..aa85a6db2 100644 --- a/src/palace/manager/feed/acquisition.py +++ b/src/palace/manager/feed/acquisition.py @@ -564,14 +564,11 @@ def single_entry_loans_feed( # Sometimes the pool or work may be None # In those cases we have to protect against the exceptions - try: - work = license_pool.work or license_pool.presentation_edition.work - except AttributeError as ex: - cls.logger().error(f"Error retrieving a Work Object {ex}") - cls.logger().error( - f"Error Data: {license_pool} | {license_pool and license_pool.presentation_edition}" - ) - return NOT_FOUND_ON_REMOTE + work: Work | None = None + if license_pool.work: + work = license_pool.work + elif license_pool.presentation_edition: + work = license_pool.presentation_edition.work if not work: return NOT_FOUND_ON_REMOTE @@ -615,11 +612,17 @@ def single_entry( """Turn a work into an annotated work entry for an acquisition feed.""" identifier = None _work: Work + active_edition: Edition | None if isinstance(work, Edition): active_edition = work identifier = active_edition.primary_identifier active_license_pool = None - _work = active_edition.work # We always need a work for an entry + if active_edition.work is None: + # We always need a work for an entry + raise PalaceValueError( + f"Edition has no associated work: {active_edition!r}" + ) + _work = active_edition.work else: _work = work active_license_pool = annotator.active_licensepool_for(work) diff --git a/src/palace/manager/feed/annotator/admin.py b/src/palace/manager/feed/annotator/admin.py index e0c27b515..a68419da9 100644 --- a/src/palace/manager/feed/annotator/admin.py +++ b/src/palace/manager/feed/annotator/admin.py @@ -46,7 +46,8 @@ def annotate_work_entry( # Find staff rating and add a tag for it. for measurement in identifier.measurements: if ( - measurement.data_source.name == DataSource.LIBRARY_STAFF + measurement.data_source + and measurement.data_source.name == DataSource.LIBRARY_STAFF and measurement.is_most_recent and measurement.value is not None ): diff --git a/src/palace/manager/feed/annotator/base.py b/src/palace/manager/feed/annotator/base.py index 8eb0ea9f2..4d6638b09 100644 --- a/src/palace/manager/feed/annotator/base.py +++ b/src/palace/manager/feed/annotator/base.py @@ -293,11 +293,13 @@ def annotate_work_entry( samples = self.samples(edition) for sample in samples: + representation = sample.resource.representation + media_type = representation.media_type if representation else None other_links.append( Link( rel=Hyperlink.CLIENT_SAMPLE, href=sample.resource.url, - type=sample.resource.representation.media_type, + type=media_type, ) ) diff --git a/src/palace/manager/feed/annotator/circulation.py b/src/palace/manager/feed/annotator/circulation.py index 59b4123f0..c46a1a902 100644 --- a/src/palace/manager/feed/annotator/circulation.py +++ b/src/palace/manager/feed/annotator/circulation.py @@ -626,9 +626,10 @@ def open_access_link( # LicensePoolDeliveryMechanism's Resource is the URL we should # send for download purposes. This will be the case unless we # previously mirrored that URL somewhere else. - href = lpdm.resource.url + resource = lpdm.resource + href = resource.url if resource else None - rep = lpdm.resource.representation + rep = resource.representation if resource else None if rep: if rep.media_type: kw["type"] = rep.media_type diff --git a/src/palace/manager/marc/annotator.py b/src/palace/manager/marc/annotator.py index ea1be3b8c..0b7da03af 100644 --- a/src/palace/manager/marc/annotator.py +++ b/src/palace/manager/marc/annotator.py @@ -57,12 +57,13 @@ def marc_record( # though they have different titles. We do not group editions of the same work in # different languages, so we can't use those yet. - cls.add_title(record, edition) - cls.add_contributors(record, edition) - cls.add_publisher(record, edition) - cls.add_physical_description(record, edition) + if edition is not None: + cls.add_title(record, edition) + cls.add_contributors(record, edition) + cls.add_publisher(record, edition) + cls.add_physical_description(record, edition) + cls.add_series(record, edition) cls.add_audience(record, work) - cls.add_series(record, edition) cls.add_system_details(record) cls.add_ebooks_subject(record) cls.add_distributor(record, license_pool) @@ -154,7 +155,11 @@ def leader(cls, revised: bool = False) -> str: @classmethod def add_control_fields( - cls, record: Record, identifier: Identifier, pool: LicensePool, edition: Edition + cls, + record: Record, + identifier: Identifier, + pool: LicensePool, + edition: Edition | None, ) -> None: # Unique identifier for this record. record.add_field(Field(tag="001", data=identifier.urn)) @@ -180,7 +185,7 @@ def add_control_fields( # Field 008 (fixed-length data elements): data = utc_now().strftime("%y%m%d") - publication_date = edition.issued or edition.published + publication_date = (edition.issued or edition.published) if edition else None if publication_date: date_type = "s" # single known date # Not using strftime because some years are pre-1900. @@ -195,7 +200,7 @@ def add_control_fields( data += "xxu" data += " " language = "eng" - if edition.language: + if edition and edition.language: language = LanguageCodes.string_to_alpha_3(edition.language) data += language data += " " diff --git a/src/palace/manager/marc/exporter.py b/src/palace/manager/marc/exporter.py index 3d6b79835..c948473fa 100644 --- a/src/palace/manager/marc/exporter.py +++ b/src/palace/manager/marc/exporter.py @@ -188,11 +188,6 @@ def enabled_libraries( library = library_integration.library library_id = library.id library_short_name = library.short_name - if library_id is None or library_short_name is None: - cls.logger().warning( - f"Library {library} is missing an ID or short name." - ) - continue last_updated_time = cls._last_updated(session, library, collection) update_frequency = cls.settings_load( library_integration.parent diff --git a/src/palace/manager/scripts/configuration.py b/src/palace/manager/scripts/configuration.py index f9dd92500..008299f87 100644 --- a/src/palace/manager/scripts/configuration.py +++ b/src/palace/manager/scripts/configuration.py @@ -278,13 +278,17 @@ def do_run(self, _db=None, cmd_args=None, output=sys.stdout): id = args.id lane = get_one(_db, Lane, id=id) if not lane: - if args.library_short_name: + if args.library_short_name and args.display_name: library = get_one(_db, Library, short_name=args.library_short_name) if not library: raise ValueError('No such library: "%s".' % args.library_short_name) - lane, is_new = create(_db, Lane, library=library) + lane, is_new = create( + _db, Lane, library=library, display_name=args.display_name + ) else: - raise ValueError("Library short name is required to create a new lane.") + raise ValueError( + "Library short name and lane display name are required to create a new lane." + ) if args.parent_id: lane.parent_id = args.parent_id diff --git a/src/palace/manager/search/external_search.py b/src/palace/manager/search/external_search.py index 7c326350f..13cc8f2cf 100644 --- a/src/palace/manager/search/external_search.py +++ b/src/palace/manager/search/external_search.py @@ -214,9 +214,6 @@ def count_works(self, filter): def remove_work(self, work: Work | int) -> None: """Remove the search document for `work` from the search index.""" if isinstance(work, Work): - if work.id is None: - self.log.warning("Work has no ID, unable to remove. %r", work) - return work = work.id self._search_service.index_remove_document(doc_id=work) diff --git a/src/palace/manager/sqlalchemy/model/admin.py b/src/palace/manager/sqlalchemy/model/admin.py index 32aa4b4ec..6080eee55 100644 --- a/src/palace/manager/sqlalchemy/model/admin.py +++ b/src/palace/manager/sqlalchemy/model/admin.py @@ -32,8 +32,8 @@ class Admin(Base, HasSessionCache): __tablename__ = "admins" - id = Column(Integer, primary_key=True) - email = Column(Unicode, unique=True, nullable=False) + id: Mapped[int] = Column(Integer, primary_key=True) + email: Mapped[str] = Column(Unicode, unique=True, nullable=False) # Admins can also log in with a local password. password_hashed = Column(Unicode, index=True) @@ -288,12 +288,16 @@ def __repr__(self): class AdminRole(Base, HasSessionCache): __tablename__ = "adminroles" - id = Column(Integer, primary_key=True) - admin_id = Column(Integer, ForeignKey("admins.id"), nullable=False, index=True) + id: Mapped[int] = Column(Integer, primary_key=True) + admin_id: Mapped[int] = Column( + Integer, ForeignKey("admins.id"), nullable=False, index=True + ) admin: Mapped[Admin] = relationship("Admin", back_populates="roles") library_id = Column(Integer, ForeignKey("libraries.id"), nullable=True, index=True) - library: Mapped[Library] = relationship("Library", back_populates="adminroles") - role = Column(Unicode, nullable=False, index=True) + library: Mapped[Library | None] = relationship( + "Library", back_populates="adminroles" + ) + role: Mapped[str] = Column(Unicode, nullable=False, index=True) __table_args__ = (UniqueConstraint("admin_id", "library_id", "role"),) diff --git a/src/palace/manager/sqlalchemy/model/announcements.py b/src/palace/manager/sqlalchemy/model/announcements.py index 9422bbf0e..ab200cfe5 100644 --- a/src/palace/manager/sqlalchemy/model/announcements.py +++ b/src/palace/manager/sqlalchemy/model/announcements.py @@ -25,10 +25,12 @@ class Announcement(Base): __tablename__ = "announcements" - id = Column(UUID(as_uuid=True), primary_key=True, default=uuid.uuid4) - content = Column(Unicode, nullable=False) - start = Column(Date, nullable=False) - finish = Column(Date, nullable=False) + id: Mapped[uuid.UUID] = Column( + UUID(as_uuid=True), primary_key=True, default=uuid.uuid4 + ) + content: Mapped[str] = Column(Unicode, nullable=False) + start: Mapped[datetime.date] = Column(Date, nullable=False) + finish: Mapped[datetime.date] = Column(Date, nullable=False) # The Library associated with the announcement, announcements that should be shown to # all libraries will have a null library_id. @@ -38,8 +40,7 @@ class Announcement(Base): index=True, nullable=True, ) - - library: Mapped[Library] = relationship( + library: Mapped[Library | None] = relationship( "Library", back_populates="library_announcements" ) @@ -107,7 +108,7 @@ def sync( db.delete(existing_announcements[id]) for id in announcements_to_update: - existing_announcements[id].update(new[id]) # type: ignore[index] + existing_announcements[id].update(new[id]) for id in announcements_to_create: Announcement.from_data(db, new[id], library=library) diff --git a/src/palace/manager/sqlalchemy/model/circulationevent.py b/src/palace/manager/sqlalchemy/model/circulationevent.py index 763409dac..91b9d5eed 100644 --- a/src/palace/manager/sqlalchemy/model/circulationevent.py +++ b/src/palace/manager/sqlalchemy/model/circulationevent.py @@ -27,7 +27,7 @@ class CirculationEvent(Base): # Used to explicitly tag an event as happening at an unknown time. NO_DATE = object() - id = Column(Integer, primary_key=True) + id: Mapped[int] = Column(Integer, primary_key=True) # One LicensePool can have many circulation events. license_pool_id = Column(Integer, ForeignKey("licensepools.id"), index=True) diff --git a/src/palace/manager/sqlalchemy/model/classification.py b/src/palace/manager/sqlalchemy/model/classification.py index d43e3c63b..1ce554518 100644 --- a/src/palace/manager/sqlalchemy/model/classification.py +++ b/src/palace/manager/sqlalchemy/model/classification.py @@ -95,7 +95,7 @@ class Subject(Base): uri_lookup[v] = k __tablename__ = "subjects" - id = Column(Integer, primary_key=True) + id: Mapped[int] = Column(Integer, primary_key=True) # Type should be one of the constants in this class. type = Column(Unicode, index=True) @@ -132,15 +132,15 @@ class Subject(Base): # Each Subject may claim affinity with one Genre. genre_id = Column(Integer, ForeignKey("genres.id"), index=True) - genre: Mapped[Genre] = relationship("Genre", back_populates="subjects") + genre: Mapped[Genre | None] = relationship("Genre", back_populates="subjects") # A locked Subject has been reviewed by a human and software will # not mess with it without permission. - locked = Column(Boolean, default=False, index=True) + locked: Mapped[bool] = Column(Boolean, default=False, index=True, nullable=False) # A checked Subject has been reviewed by software and will # not be checked again unless forced. - checked = Column(Boolean, default=False, index=True) + checked: Mapped[bool] = Column(Boolean, default=False, index=True, nullable=False) # One Subject may participate in many Classifications. classifications: Mapped[list[Classification]] = relationship( @@ -345,13 +345,15 @@ class Classification(Base): """The assignment of a Identifier to a Subject.""" __tablename__ = "classifications" - id = Column(Integer, primary_key=True) + id: Mapped[int] = Column(Integer, primary_key=True) identifier_id = Column(Integer, ForeignKey("identifiers.id"), index=True) identifier: Mapped[Identifier | None] = relationship( "Identifier", back_populates="classifications" ) subject_id = Column(Integer, ForeignKey("subjects.id"), index=True) - subject: Mapped[Subject] = relationship("Subject", back_populates="classifications") + subject: Mapped[Subject | None] = relationship( + "Subject", back_populates="classifications" + ) data_source_id = Column(Integer, ForeignKey("datasources.id"), index=True) data_source: Mapped[DataSource | None] = relationship( "DataSource", back_populates="classifications" @@ -482,7 +484,7 @@ class Genre(Base, HasSessionCache): """ __tablename__ = "genres" - id = Column(Integer, primary_key=True) + id: Mapped[int] = Column(Integer, primary_key=True) name = Column(Unicode, unique=True, index=True) # One Genre may have affinity with many Subjects. diff --git a/src/palace/manager/sqlalchemy/model/collection.py b/src/palace/manager/sqlalchemy/model/collection.py index ae0a35049..3ad6474f8 100644 --- a/src/palace/manager/sqlalchemy/model/collection.py +++ b/src/palace/manager/sqlalchemy/model/collection.py @@ -49,7 +49,7 @@ class Collection(Base, HasSessionCache, RedisKeyMixin): """A Collection is a set of LicensePools obtained through some mechanism.""" __tablename__ = "collections" - id = Column(Integer, primary_key=True, nullable=False) + id: Mapped[int] = Column(Integer, primary_key=True, nullable=False) DATA_SOURCE_NAME_SETTING = "data_source" DATA_SOURCE_FOR_LICENSE_PROTOCOL = [ @@ -66,7 +66,7 @@ class Collection(Base, HasSessionCache, RedisKeyMixin): # designates the integration technique we will use to actually get # the metadata and licenses. Each Collection has a distinct # integration configuration. - integration_configuration_id = Column( + integration_configuration_id: Mapped[int] = Column( Integer, ForeignKey("integration_configurations.id"), unique=True, @@ -87,7 +87,7 @@ class Collection(Base, HasSessionCache, RedisKeyMixin): # secret as the Overdrive collection, but it has a distinct # external_account_id. parent_id = Column(Integer, ForeignKey("collections.id"), index=True) - parent: Collection = relationship( + parent: Mapped[Collection | None] = relationship( "Collection", remote_side=[id], back_populates="children" ) @@ -101,7 +101,7 @@ class Collection(Base, HasSessionCache, RedisKeyMixin): # When deleting a collection, this flag is set to True so that the deletion # script can take care of deleting it in the background. This is # useful for deleting large collections which can timeout when deleting. - marked_for_deletion = Column(Boolean, default=False) + marked_for_deletion: Mapped[bool] = Column(Boolean, default=False, nullable=False) # A Collection can provide books to many Libraries. # https://docs.sqlalchemy.org/en/14/orm/extensions/associationproxy.html#composite-association-proxies @@ -147,7 +147,7 @@ class Collection(Base, HasSessionCache, RedisKeyMixin): "CustomList", secondary="collections_customlists", back_populates="collections" ) - export_marc_records = Column(Boolean, default=False, nullable=False) + export_marc_records: Mapped[bool] = Column(Boolean, default=False, nullable=False) # Most data sources offer different catalogs to different # libraries. Data sources in this list offer the same catalog to diff --git a/src/palace/manager/sqlalchemy/model/contributor.py b/src/palace/manager/sqlalchemy/model/contributor.py index b12554093..bc27ab2d3 100644 --- a/src/palace/manager/sqlalchemy/model/contributor.py +++ b/src/palace/manager/sqlalchemy/model/contributor.py @@ -32,7 +32,7 @@ class Contributor(Base): """Someone (usually human) who contributes to books.""" __tablename__ = "contributors" - id = Column(Integer, primary_key=True) + id: Mapped[int] = Column(Integer, primary_key=True) # Standard identifiers for this contributor. lc = Column(Unicode, index=True) @@ -60,7 +60,9 @@ class Contributor(Base): # provided by a publisher. biography = Column(Unicode) - extra: Mapped[dict[str, str]] = Column(MutableDict.as_mutable(JSON), default={}) + extra: Mapped[dict[str, str]] = Column( + MutableDict.as_mutable(JSON), default={}, nullable=False + ) contributions: Mapped[list[Contribution]] = relationship( "Contribution", back_populates="contributor", uselist=True @@ -483,12 +485,14 @@ class Contribution(Base): """A contribution made by a Contributor to a Edition.""" __tablename__ = "contributions" - id = Column(Integer, primary_key=True) + id: Mapped[int] = Column(Integer, primary_key=True) edition: Mapped[Edition] = relationship("Edition", back_populates="contributions") - edition_id = Column(Integer, ForeignKey("editions.id"), index=True, nullable=False) + edition_id: Mapped[int] = Column( + Integer, ForeignKey("editions.id"), index=True, nullable=False + ) - contributor_id = Column( + contributor_id: Mapped[int] = Column( Integer, ForeignKey("contributors.id"), index=True, nullable=False ) contributor: Mapped[Contributor] = relationship( diff --git a/src/palace/manager/sqlalchemy/model/coverage.py b/src/palace/manager/sqlalchemy/model/coverage.py index ddeeb2d1d..4d4f911d9 100644 --- a/src/palace/manager/sqlalchemy/model/coverage.py +++ b/src/palace/manager/sqlalchemy/model/coverage.py @@ -121,10 +121,10 @@ class Timestamp(Base): ) # Unique ID - id = Column(Integer, primary_key=True) + id: Mapped[int] = Column(Integer, primary_key=True) # Name of the service. - service = Column(String(255), index=True, nullable=False) + service: Mapped[str] = Column(String(255), index=True, nullable=False) # Type of the service -- monitor, coverage provider, or script. # If the service type does not fit into these categories, this field @@ -136,7 +136,7 @@ class Timestamp(Base): collection_id = Column( Integer, ForeignKey("collections.id"), index=True, nullable=True ) - collection: Mapped[Collection] = relationship( + collection: Mapped[Collection | None] = relationship( "Collection", back_populates="timestamps" ) @@ -324,16 +324,16 @@ class CoverageRecord(Base, BaseCoverageRecord): REPAIR_SORT_NAME_OPERATION = "repair-sort-name" METADATA_UPLOAD_OPERATION = "metadata-upload" - id = Column(Integer, primary_key=True) + id: Mapped[int] = Column(Integer, primary_key=True) identifier_id = Column(Integer, ForeignKey("identifiers.id"), index=True) - identifier: Mapped[Identifier] = relationship( + identifier: Mapped[Identifier | None] = relationship( "Identifier", back_populates="coverage_records" ) # If applicable, this is the ID of the data source that took the # Identifier as input. data_source_id = Column(Integer, ForeignKey("datasources.id")) - data_source: Mapped[DataSource] = relationship( + data_source: Mapped[DataSource | None] = relationship( "DataSource", back_populates="coverage_records" ) operation = Column(String(255), default=None) @@ -623,9 +623,9 @@ class WorkCoverageRecord(Base, BaseCoverageRecord): SUMMARY_OPERATION = "summary" QUALITY_OPERATION = "quality" - id = Column(Integer, primary_key=True) + id: Mapped[int] = Column(Integer, primary_key=True) work_id = Column(Integer, ForeignKey("works.id"), index=True) - work: Mapped[Work] = relationship("Work", back_populates="coverage_records") + work: Mapped[Work | None] = relationship("Work", back_populates="coverage_records") operation = Column(String(255), index=True, default=None) timestamp = Column(DateTime(timezone=True), index=True) @@ -775,10 +775,13 @@ class EquivalencyCoverageRecord(Base, BaseCoverageRecord): __tablename__ = "equivalentscoveragerecords" - id = Column(Integer, primary_key=True) + id: Mapped[int] = Column(Integer, primary_key=True) - equivalency_id = Column( - Integer, ForeignKey("equivalents.id", ondelete="CASCADE"), index=True + equivalency_id: Mapped[int] = Column( + Integer, + ForeignKey("equivalents.id", ondelete="CASCADE"), + index=True, + nullable=False, ) equivalency: Mapped[Equivalency] = relationship( "Equivalency", foreign_keys=equivalency_id diff --git a/src/palace/manager/sqlalchemy/model/credential.py b/src/palace/manager/sqlalchemy/model/credential.py index b33143b6f..c6ceaf3b6 100644 --- a/src/palace/manager/sqlalchemy/model/credential.py +++ b/src/palace/manager/sqlalchemy/model/credential.py @@ -25,15 +25,15 @@ class Credential(Base): """A place to store credentials for external services.""" __tablename__ = "credentials" - id = Column(Integer, primary_key=True) + id: Mapped[int] = Column(Integer, primary_key=True) data_source_id = Column(Integer, ForeignKey("datasources.id"), index=True) - data_source: Mapped[DataSource] = relationship( + data_source: Mapped[DataSource | None] = relationship( "DataSource", back_populates="credentials" ) patron_id = Column(Integer, ForeignKey("patrons.id"), index=True) - patron: Mapped[Patron] = relationship("Patron", back_populates="credentials") + patron: Mapped[Patron | None] = relationship("Patron", back_populates="credentials") collection_id = Column(Integer, ForeignKey("collections.id"), index=True) - collection: Mapped[Collection] = relationship( + collection: Mapped[Collection | None] = relationship( "Collection", back_populates="credentials" ) type = Column(String(255), index=True) diff --git a/src/palace/manager/sqlalchemy/model/customlist.py b/src/palace/manager/sqlalchemy/model/customlist.py index f6b5adb57..91933f5a9 100644 --- a/src/palace/manager/sqlalchemy/model/customlist.py +++ b/src/palace/manager/sqlalchemy/model/customlist.py @@ -47,10 +47,10 @@ class CustomList(Base): REPOPULATE = "repopulate" __tablename__ = "customlists" - id = Column(Integer, primary_key=True) + id: Mapped[int] = Column(Integer, primary_key=True) primary_language = Column(Unicode, index=True) data_source_id = Column(Integer, ForeignKey("datasources.id"), index=True) - data_source: Mapped[DataSource] = relationship( + data_source: Mapped[DataSource | None] = relationship( "DataSource", back_populates="custom_lists" ) foreign_identifier = Column(Unicode, index=True) @@ -60,11 +60,13 @@ class CustomList(Base): updated = Column(DateTime(timezone=True), index=True) responsible_party = Column(Unicode) library_id = Column(Integer, ForeignKey("libraries.id"), index=True, nullable=True) - library: Mapped[Library] = relationship("Library", back_populates="custom_lists") + library: Mapped[Library | None] = relationship( + "Library", back_populates="custom_lists" + ) # How many titles are in this list? This is calculated and # cached when the list contents change. - size = Column(Integer, nullable=False, default=0) + size: Mapped[int] = Column(Integer, nullable=False, default=0) entries: Mapped[list[CustomListEntry]] = relationship( "CustomListEntry", back_populates="customlist", uselist=True @@ -78,12 +80,14 @@ class CustomList(Base): uselist=True, ) - auto_update_enabled = Column(Boolean, default=False) + auto_update_enabled: Mapped[bool] = Column(Boolean, default=False, nullable=False) auto_update_query = Column(Unicode, nullable=True) # holds json data auto_update_facets = Column(Unicode, nullable=True) # holds json data auto_update_last_update = Column(DateTime, nullable=True) auto_update_status: Mapped[str] = Column( - Enum(INIT, UPDATED, REPOPULATE, name="auto_update_status"), default=INIT + Enum(INIT, UPDATED, REPOPULATE, name="auto_update_status"), + default=INIT, + nullable=False, ) collections: Mapped[list[Collection]] = relationship( @@ -370,9 +374,9 @@ def update_size(self, db: Session): class CustomListEntry(Base): __tablename__ = "customlistentries" - id = Column(Integer, primary_key=True) + id: Mapped[int] = Column(Integer, primary_key=True) list_id = Column(Integer, ForeignKey("customlists.id"), index=True) - customlist: Mapped[CustomList] = relationship( + customlist: Mapped[CustomList | None] = relationship( "CustomList", back_populates="entries" ) @@ -386,7 +390,7 @@ class CustomListEntry(Base): "Work", back_populates="custom_list_entries" ) - featured = Column(Boolean, nullable=False, default=False) + featured: Mapped[bool] = Column(Boolean, nullable=False, default=False) annotation = Column(Unicode) # These two fields are for best-seller lists. Even after a book diff --git a/src/palace/manager/sqlalchemy/model/datasource.py b/src/palace/manager/sqlalchemy/model/datasource.py index 8f19bdf9a..5735e7037 100644 --- a/src/palace/manager/sqlalchemy/model/datasource.py +++ b/src/palace/manager/sqlalchemy/model/datasource.py @@ -37,11 +37,13 @@ class DataSource(Base, HasSessionCache, DataSourceConstants): """A source for information about books, and possibly the books themselves.""" __tablename__ = "datasources" - id = Column(Integer, primary_key=True) + id: Mapped[int] = Column(Integer, primary_key=True) name = Column(String, unique=True, index=True) - offers_licenses = Column(Boolean, default=False) + offers_licenses: Mapped[bool] = Column(Boolean, default=False, nullable=False) primary_identifier_type = Column(String, index=True) - extra: Mapped[dict[str, str]] = Column(MutableDict.as_mutable(JSON), default={}) + extra: Mapped[dict[str, str]] = Column( + MutableDict.as_mutable(JSON), default={}, nullable=False + ) # One DataSource can generate many Editions. editions: Mapped[list[Edition]] = relationship( diff --git a/src/palace/manager/sqlalchemy/model/devicetokens.py b/src/palace/manager/sqlalchemy/model/devicetokens.py index 2b69ff759..06004093b 100644 --- a/src/palace/manager/sqlalchemy/model/devicetokens.py +++ b/src/palace/manager/sqlalchemy/model/devicetokens.py @@ -25,8 +25,8 @@ class DeviceToken(Base): __tablename__ = "devicetokens" - id = Column("id", Integer, primary_key=True) - patron_id = Column( + id: Mapped[int] = Column("id", Integer, primary_key=True) + patron_id: Mapped[int] = Column( Integer, ForeignKey("patrons.id", ondelete="CASCADE", name="devicetokens_patron_fkey"), index=True, @@ -34,12 +34,14 @@ class DeviceToken(Base): ) patron: Mapped[Patron] = relationship("Patron", back_populates="device_tokens") - token_type_enum = Enum( - DeviceTokenTypes.FCM_ANDROID, DeviceTokenTypes.FCM_IOS, name="token_types" + token_type: Mapped[str] = Column( + Enum( + DeviceTokenTypes.FCM_ANDROID, DeviceTokenTypes.FCM_IOS, name="token_types" + ), + nullable=False, ) - token_type = Column(token_type_enum, nullable=False) - device_token = Column(Unicode, nullable=False, index=True) + device_token: Mapped[str] = Column(Unicode, nullable=False, index=True) __table_args__ = ( Index( diff --git a/src/palace/manager/sqlalchemy/model/discovery_service_registration.py b/src/palace/manager/sqlalchemy/model/discovery_service_registration.py index f88c03d8d..609b96002 100644 --- a/src/palace/manager/sqlalchemy/model/discovery_service_registration.py +++ b/src/palace/manager/sqlalchemy/model/discovery_service_registration.py @@ -34,12 +34,12 @@ class DiscoveryServiceRegistration(Base): __tablename__ = "discovery_service_registrations" - status = Column( + status: Mapped[RegistrationStatus] = Column( AlchemyEnum(RegistrationStatus), default=RegistrationStatus.FAILURE, nullable=False, ) - stage = Column( + stage: Mapped[RegistrationStage] = Column( AlchemyEnum(RegistrationStage), default=RegistrationStage.TESTING, nullable=False, @@ -50,7 +50,7 @@ class DiscoveryServiceRegistration(Base): shared_secret = Column(Unicode) # The IntegrationConfiguration this registration is associated with. - integration_id = Column( + integration_id: Mapped[int] = Column( Integer, ForeignKey("integration_configurations.id", ondelete="CASCADE"), nullable=False, @@ -61,7 +61,7 @@ class DiscoveryServiceRegistration(Base): ) # The Library this registration is associated with. - library_id = Column( + library_id: Mapped[int] = Column( Integer, ForeignKey("libraries.id", ondelete="CASCADE"), nullable=False, diff --git a/src/palace/manager/sqlalchemy/model/edition.py b/src/palace/manager/sqlalchemy/model/edition.py index c071af001..6405e49ee 100644 --- a/src/palace/manager/sqlalchemy/model/edition.py +++ b/src/palace/manager/sqlalchemy/model/edition.py @@ -52,9 +52,11 @@ class Edition(Base, EditionConstants): """ __tablename__ = "editions" - id = Column(Integer, primary_key=True) + id: Mapped[int] = Column(Integer, primary_key=True) - data_source_id = Column(Integer, ForeignKey("datasources.id"), index=True) + data_source_id: Mapped[int] = Column( + Integer, ForeignKey("datasources.id"), index=True, nullable=False + ) data_source: Mapped[DataSource] = relationship( "DataSource", back_populates="editions" ) @@ -85,14 +87,16 @@ class Edition(Base, EditionConstants): # identifier--the one used by its data source to identify # it. Through the Equivalency class, it is associated with a # (probably huge) number of other identifiers. - primary_identifier_id = Column(Integer, ForeignKey("identifiers.id"), index=True) + primary_identifier_id = Column( + Integer, ForeignKey("identifiers.id"), index=True, nullable=False + ) primary_identifier: Mapped[Identifier] = relationship( "Identifier", back_populates="primarily_identifies" ) # An Edition may be the presentation edition for a single Work. If it's not # a presentation edition for a work, work will be None. - work: Mapped[Work] = relationship( + work: Mapped[Work | None] = relationship( "Work", uselist=False, back_populates="presentation_edition" ) @@ -158,7 +162,9 @@ class Edition(Base, EditionConstants): cover_thumbnail_url = Column(Unicode) # Information kept in here probably won't be used. - extra: Mapped[dict[str, str]] = Column(MutableDict.as_mutable(JSON), default={}) + extra: Mapped[dict[str, str]] = Column( + MutableDict.as_mutable(JSON), default={}, nullable=False + ) def __repr__(self): id_repr = repr(self.primary_identifier) diff --git a/src/palace/manager/sqlalchemy/model/identifier.py b/src/palace/manager/sqlalchemy/model/identifier.py index 6dfb0e7fc..aef3eb2bc 100644 --- a/src/palace/manager/sqlalchemy/model/identifier.py +++ b/src/palace/manager/sqlalchemy/model/identifier.py @@ -237,7 +237,7 @@ class Identifier(Base, IdentifierConstants): """A way of uniquely referring to a particular edition.""" __tablename__ = "identifiers" - id = Column(Integer, primary_key=True) + id: Mapped[int] = Column(Integer, primary_key=True) type = Column(String(64), index=True) identifier = Column(String, index=True) @@ -1124,26 +1124,28 @@ class Equivalency(Base): # 'input' is the ID that was used as input to the datasource. # 'output' is the output - id = Column(Integer, primary_key=True) - input_id = Column(Integer, ForeignKey("identifiers.id"), index=True) + id: Mapped[int] = Column(Integer, primary_key=True) + input_id: Mapped[int] = Column( + Integer, ForeignKey("identifiers.id"), index=True, nullable=False + ) input: Mapped[Identifier] = relationship( "Identifier", foreign_keys=input_id, back_populates="equivalencies" ) - output_id = Column(Integer, ForeignKey("identifiers.id"), index=True) + output_id: Mapped[int] = Column(Integer, ForeignKey("identifiers.id"), index=True) output: Mapped[Identifier] = relationship( "Identifier", foreign_keys=output_id, back_populates="inbound_equivalencies" ) # Who says? data_source_id = Column(Integer, ForeignKey("datasources.id"), index=True) - data_source: Mapped[DataSource] = relationship( + data_source: Mapped[DataSource | None] = relationship( "DataSource", back_populates="id_equivalencies" ) # How many distinct votes went into this assertion? This will let # us scale the change to the strength when additional votes come # in. - votes = Column(Integer, default=1) + votes: Mapped[int] = Column(Integer, default=1, nullable=False) # How strong is this assertion (-1..1)? A negative number is an # assertion that the two Identifiers do *not* identify the @@ -1154,7 +1156,7 @@ class Equivalency(Base): # is not manipulated directly, but it gives us the ability to use # manual intervention to defuse large chunks of problematic code # without actually deleting the data. - enabled = Column(Boolean, default=True, index=True) + enabled: Mapped[bool] = Column(Boolean, default=True, index=True, nullable=False) def __repr__(self): r = "[%s ->\n %s\n source=%s strength=%.2f votes=%d)]" % ( @@ -1199,19 +1201,19 @@ class RecursiveEquivalencyCache(Base): __tablename__ = "recursiveequivalentscache" - id = Column(Integer, primary_key=True) + id: Mapped[int] = Column(Integer, primary_key=True) # The "parent" or the start of the chain parent_identifier_id = Column( Integer, ForeignKey("identifiers.id", ondelete="CASCADE") ) - parent_identifier: Mapped[Identifier] = relationship( + parent_identifier: Mapped[Identifier | None] = relationship( "Identifier", foreign_keys=parent_identifier_id ) # The identifier chained to the parent identifier_id = Column(Integer, ForeignKey("identifiers.id", ondelete="CASCADE")) - identifier: Mapped[Identifier] = relationship( + identifier: Mapped[Identifier | None] = relationship( "Identifier", foreign_keys=identifier_id ) diff --git a/src/palace/manager/sqlalchemy/model/integration.py b/src/palace/manager/sqlalchemy/model/integration.py index 3a9ecf1d7..2742cc3ae 100644 --- a/src/palace/manager/sqlalchemy/model/integration.py +++ b/src/palace/manager/sqlalchemy/model/integration.py @@ -29,20 +29,20 @@ class IntegrationConfiguration(Base): """ __tablename__ = "integration_configurations" - id = Column(Integer, primary_key=True) + id: Mapped[int] = Column(Integer, primary_key=True) # The protocol is used to load the correct implementation class for # this integration. It is looked up in the IntegrationRegistry. - protocol = Column(Unicode, nullable=False) + protocol: Mapped[str] = Column(Unicode, nullable=False) # The goal of the integration is used to differentiate between the # different types of integrations. For example, a goal of "authentication" # would be used for an authentication provider. - goal = Column(SQLAlchemyEnum(Goals), nullable=False, index=True) + goal: Mapped[Goals] = Column(SQLAlchemyEnum(Goals), nullable=False, index=True) # A unique name for this integration. This is primarily # used to identify integrations from command-line scripts. - name = Column(Unicode, nullable=False, unique=True) + name: Mapped[str] = Column(Unicode, nullable=False, unique=True) # The configuration settings for this integration. Stored as json. settings_dict: Mapped[dict[str, Any]] = Column( @@ -68,7 +68,9 @@ def context_update(self, new_context: dict[str, Any]) -> None: flag_modified(self, "context") # Self test results, stored as json. - self_test_results = Column(JSONB, nullable=False, default=dict) + self_test_results: Mapped[dict[str, Any]] = Column( + JSONB, nullable=False, default=dict + ) library_configurations: Mapped[list[IntegrationLibraryConfiguration]] = ( relationship( @@ -102,8 +104,6 @@ def for_library( db = Session.object_session(self) if isinstance(library, Library): - if library.id is None: - return None library_id = library.id else: library_id = library @@ -187,7 +187,7 @@ class IntegrationLibraryConfiguration(Base): # The IntegrationConfiguration this library configuration is # associated with. - parent_id = Column( + parent_id: Mapped[int] = Column( Integer, ForeignKey("integration_configurations.id", ondelete="CASCADE"), primary_key=True, @@ -198,7 +198,7 @@ class IntegrationLibraryConfiguration(Base): ) # The library this integration is associated with. - library_id = Column( + library_id: Mapped[int] = Column( Integer, ForeignKey("libraries.id", ondelete="CASCADE"), primary_key=True, diff --git a/src/palace/manager/sqlalchemy/model/key.py b/src/palace/manager/sqlalchemy/model/key.py index 2645b427e..98c5f1888 100644 --- a/src/palace/manager/sqlalchemy/model/key.py +++ b/src/palace/manager/sqlalchemy/model/key.py @@ -8,7 +8,7 @@ from sqlalchemy import Enum as SaEnum from sqlalchemy import Unicode, delete, select from sqlalchemy.dialects.postgresql import UUID -from sqlalchemy.orm import Session +from sqlalchemy.orm import Mapped, Session from typing_extensions import Self from palace.manager.sqlalchemy.model.base import Base @@ -26,12 +26,14 @@ class KeyType(Enum): class Key(Base): __tablename__ = "keys" - id = Column(UUID(as_uuid=True), primary_key=True, default=uuid.uuid4) - created = Column( + id: Mapped[uuid.UUID] = Column( + UUID(as_uuid=True), primary_key=True, default=uuid.uuid4 + ) + created: Mapped[datetime.datetime] = Column( DateTime(timezone=True), index=True, nullable=False, default=utc_now ) - value = Column(Unicode, nullable=False) - type = Column(SaEnum(KeyType), nullable=False, index=True) + value: Mapped[str] = Column(Unicode, nullable=False) + type: Mapped[KeyType] = Column(SaEnum(KeyType), nullable=False, index=True) def __repr__(self) -> str: return f"" diff --git a/src/palace/manager/sqlalchemy/model/lane.py b/src/palace/manager/sqlalchemy/model/lane.py index 73a87897e..d50f5f06f 100644 --- a/src/palace/manager/sqlalchemy/model/lane.py +++ b/src/palace/manager/sqlalchemy/model/lane.py @@ -2598,22 +2598,26 @@ class LaneGenre(Base): """Relationship object between Lane and Genre.""" __tablename__ = "lanes_genres" - id = Column(Integer, primary_key=True) - lane_id = Column(Integer, ForeignKey("lanes.id"), index=True, nullable=False) + id: Mapped[int] = Column(Integer, primary_key=True) + lane_id: Mapped[int] = Column( + Integer, ForeignKey("lanes.id"), index=True, nullable=False + ) lane: Mapped[Lane] = relationship("Lane", back_populates="lane_genres") - genre_id = Column(Integer, ForeignKey("genres.id"), index=True, nullable=False) + genre_id: Mapped[int] = Column( + Integer, ForeignKey("genres.id"), index=True, nullable=False + ) genre: Mapped[Genre] = relationship(Genre, back_populates="lane_genres") # An inclusive relationship means that books classified under the # genre are included in the lane. An exclusive relationship means # that books classified under the genre are excluded, even if they # would otherwise be included. - inclusive = Column(Boolean, default=True, nullable=False) + inclusive: Mapped[bool] = Column(Boolean, default=True, nullable=False) # By default, this relationship applies not only to the genre # itself but to all of its subgenres. Setting recursive=false # means that only the genre itself is affected. - recursive = Column(Boolean, default=True, nullable=False) + recursive: Mapped[bool] = Column(Boolean, default=True, nullable=False) __table_args__ = (UniqueConstraint("lane_id", "genre_id"),) @@ -2640,8 +2644,10 @@ class Lane(Base, DatabaseBackedWorkList, HierarchyWorkList): MAX_CACHE_AGE = 20 * 60 __tablename__ = "lanes" - id = Column(Integer, primary_key=True) - library_id = Column(Integer, ForeignKey("libraries.id"), index=True, nullable=False) + id: Mapped[int] = Column(Integer, primary_key=True) + library_id: Mapped[int] = Column( + Integer, ForeignKey("libraries.id"), index=True, nullable=False + ) library: Mapped[Library] = relationship(Library, back_populates="lanes") parent_id = Column(Integer, ForeignKey("lanes.id"), index=True, nullable=True) @@ -2651,11 +2657,11 @@ class Lane(Base, DatabaseBackedWorkList, HierarchyWorkList): remote_side=[id], ) - priority = Column(Integer, index=True, nullable=False, default=0) + priority: Mapped[int] = Column(Integer, index=True, nullable=False, default=0) # How many titles are in this lane? This is periodically # calculated and cached. - size = Column(Integer, nullable=False, default=0) + size: Mapped[int] = Column(Integer, nullable=False, default=0) # How many titles are in this lane when viewed through a specific # entry point? This is periodically calculated and cached. @@ -2680,7 +2686,7 @@ class Lane(Base, DatabaseBackedWorkList, HierarchyWorkList): # okay for this to be duplicated within a library, but it's not # okay to have two lanes with the same parent and the same display # name -- that would be confusing. - display_name: str = Column(Unicode) + display_name: Mapped[str] = Column(Unicode, nullable=False) # True = Fiction only # False = Nonfiction only @@ -2712,7 +2718,7 @@ class Lane(Base, DatabaseBackedWorkList, HierarchyWorkList): license_datasource_id = Column( Integer, ForeignKey("datasources.id"), index=True, nullable=True ) - license_datasource: Mapped[DataSource] = relationship( + license_datasource: Mapped[DataSource | None] = relationship( "DataSource", back_populates="license_lanes", foreign_keys=[license_datasource_id], @@ -2723,7 +2729,7 @@ class Lane(Base, DatabaseBackedWorkList, HierarchyWorkList): _list_datasource_id = Column( Integer, ForeignKey("datasources.id"), index=True, nullable=True ) - _list_datasource: Mapped[DataSource] = relationship( + _list_datasource: Mapped[DataSource | None] = relationship( "DataSource", back_populates="list_lanes", foreign_keys=[_list_datasource_id] ) @@ -2742,7 +2748,9 @@ class Lane(Base, DatabaseBackedWorkList, HierarchyWorkList): # If this is set to True, then a book will show up in a lane only # if it would _also_ show up in its parent lane. - inherit_parent_restrictions = Column(Boolean, default=True, nullable=False) + inherit_parent_restrictions: Mapped[bool] = Column( + Boolean, default=True, nullable=False + ) # Patrons whose external type is in this list will be sent to this # lane when they ask for the root lane. @@ -2757,11 +2765,13 @@ class Lane(Base, DatabaseBackedWorkList, HierarchyWorkList): # one would want to see a big list containing everything, and b) # the sublanes are exhaustive of the Lane's content, so there's # nothing new to be seen by going into that big list. - include_self_in_grouped_feed = Column(Boolean, default=True, nullable=False) + include_self_in_grouped_feed: Mapped[bool] = Column( + Boolean, default=True, nullable=False + ) # Only a visible lane will show up in the user interface. The # admin interface can see all the lanes, visible or not. - _visible = Column("visible", Boolean, default=True, nullable=False) + _visible: Mapped[bool] = Column("visible", Boolean, default=True, nullable=False) __table_args__ = (UniqueConstraint("parent_id", "display_name"),) diff --git a/src/palace/manager/sqlalchemy/model/library.py b/src/palace/manager/sqlalchemy/model/library.py index 286a89f85..387613744 100644 --- a/src/palace/manager/sqlalchemy/model/library.py +++ b/src/palace/manager/sqlalchemy/model/library.py @@ -64,7 +64,7 @@ class Library(Base, HasSessionCache): __tablename__ = "libraries" - id = Column(Integer, primary_key=True) + id: Mapped[int] = Column(Integer, primary_key=True) # The human-readable name of this library. Used in the library's # Authentication for OPDS document. @@ -72,7 +72,7 @@ class Library(Base, HasSessionCache): # A short name of this library, to use when identifying it in # scripts. e.g. "NYPL" for NYPL. - short_name = Column(Unicode, unique=True, nullable=False) + short_name: Mapped[str] = Column(Unicode, unique=True, nullable=False) # A UUID that uniquely identifies the library among all libraries # in the world. This is used to serve the library's Authentication @@ -82,7 +82,9 @@ class Library(Base, HasSessionCache): # One, and only one, library may be the default. The default # library is the one chosen when an incoming request does not # designate a library. - _is_default = Column("is_default", Boolean, index=True, default=False) + _is_default: Mapped[bool] = Column( + "is_default", Boolean, index=True, default=False, nullable=False + ) # The name of this library to use when signing short client tokens # for consumption by the library registry. e.g. "NYNYPL" for NYPL. @@ -120,7 +122,7 @@ class Library(Base, HasSessionCache): ) # Any additional configuration information is stored as JSON on this column. - settings_dict: dict[str, Any] = Column(JSONB, nullable=False, default=dict) + settings_dict: Mapped[dict[str, Any]] = Column(JSONB, nullable=False, default=dict) # A Library may have many CirculationEvents circulation_events: Mapped[list[CirculationEvent]] = relationship( @@ -150,12 +152,12 @@ class Library(Base, HasSessionCache): # The library's public / private RSA key-pair. # The public key is stored in PEM format. - public_key = Column(Unicode, nullable=False) + public_key: Mapped[str] = Column(Unicode, nullable=False) # The private key is stored in DER binary format. - private_key = Column(LargeBinary, nullable=False) + private_key: Mapped[bytes] = Column(LargeBinary, nullable=False) # The libraries logo image, stored as a base64 encoded string. - logo: Mapped[LibraryLogo] = relationship( + logo: Mapped[LibraryLogo | None] = relationship( "LibraryLogo", back_populates="library", cascade="all, delete-orphan", @@ -500,13 +502,15 @@ class LibraryLogo(Base): """ __tablename__ = "libraries_logos" - library_id = Column(Integer, ForeignKey("libraries.id"), primary_key=True) + library_id: Mapped[int] = Column( + Integer, ForeignKey("libraries.id"), primary_key=True + ) library: Mapped[Library] = relationship( "Library", back_populates="logo", uselist=False ) # The logo stored as a base-64 encoded png. - content = Column(LargeBinary, nullable=False) + content: Mapped[bytes] = Column(LargeBinary, nullable=False) @property def data_url(self) -> str: diff --git a/src/palace/manager/sqlalchemy/model/licensing.py b/src/palace/manager/sqlalchemy/model/licensing.py index f5569d7b7..970567238 100644 --- a/src/palace/manager/sqlalchemy/model/licensing.py +++ b/src/palace/manager/sqlalchemy/model/licensing.py @@ -130,7 +130,7 @@ class License(Base, LicenseFunctions): """ __tablename__ = "licenses" - id = Column(Integer, primary_key=True) + id: Mapped[int] = Column(Integer, primary_key=True) identifier = Column(Unicode) checkout_url = Column(Unicode) @@ -149,7 +149,9 @@ class License(Base, LicenseFunctions): terms_concurrency = Column(Integer) # A License belongs to one LicensePool. - license_pool_id = Column(Integer, ForeignKey("licensepools.id"), index=True) + license_pool_id: Mapped[int] = Column( + Integer, ForeignKey("licensepools.id"), index=True, nullable=False + ) license_pool: Mapped[LicensePool] = relationship( "LicensePool", back_populates="licenses" ) @@ -215,27 +217,31 @@ class LicensePool(Base): UNLIMITED_ACCESS = -1 __tablename__ = "licensepools" - id = Column(Integer, primary_key=True) + id: Mapped[int] = Column(Integer, primary_key=True) # A LicensePool may be associated with a Work. (If it's not, no one # can check it out.) work_id = Column(Integer, ForeignKey("works.id"), index=True) - work: Mapped[Work] = relationship("Work", back_populates="license_pools") + work: Mapped[Work | None] = relationship("Work", back_populates="license_pools") # Each LicensePool is associated with one DataSource and one # Identifier. - data_source_id = Column(Integer, ForeignKey("datasources.id"), index=True) + data_source_id: Mapped[int] = Column( + Integer, ForeignKey("datasources.id"), index=True, nullable=False + ) data_source: Mapped[DataSource] = relationship( "DataSource", back_populates="license_pools", lazy="joined" ) - identifier_id = Column(Integer, ForeignKey("identifiers.id"), index=True) + identifier_id: Mapped[int] = Column( + Integer, ForeignKey("identifiers.id"), index=True, nullable=False + ) identifier: Mapped[Identifier] = relationship( "Identifier", back_populates="licensed_through", lazy="joined" ) # Each LicensePool belongs to one Collection. - collection_id = Column( + collection_id: Mapped[int] = Column( Integer, ForeignKey("collections.id"), index=True, nullable=False ) @@ -246,7 +252,7 @@ class LicensePool(Base): # Each LicensePool has an Edition which contains the metadata used # to describe this book. presentation_edition_id = Column(Integer, ForeignKey("editions.id"), index=True) - presentation_edition: Mapped[Edition] = relationship( + presentation_edition: Mapped[Edition | None] = relationship( "Edition", back_populates="is_presentation_for" ) @@ -282,11 +288,13 @@ class LicensePool(Base): # associated with the same Work. This may happen if it's an # open-access LicensePool and a better-quality version of the same # book is available from another Open-Access source. - superceded = Column(Boolean, default=False) + superceded: Mapped[bool] = Column(Boolean, default=False, nullable=False) # A LicensePool that seemingly looks fine may be manually suppressed # to be temporarily or permanently removed from the collection. - suppressed = Column(Boolean, default=False, index=True) + suppressed: Mapped[bool] = Column( + Boolean, default=False, index=True, nullable=False + ) # A textual description of a problem with this license pool # that caused us to suppress it. @@ -294,11 +302,13 @@ class LicensePool(Base): open_access = Column(Boolean, index=True) last_checked = Column(DateTime(timezone=True), index=True) - licenses_owned: int = Column(Integer, default=0, index=True) - licenses_available: int = Column(Integer, default=0, index=True) - licenses_reserved: int = Column(Integer, default=0) - patrons_in_hold_queue: int = Column(Integer, default=0) - should_track_playtime = Column(Boolean, default=False, nullable=False) + licenses_owned: Mapped[int] = Column(Integer, default=0, index=True, nullable=False) + licenses_available: Mapped[int] = Column( + Integer, default=0, index=True, nullable=False + ) + licenses_reserved: Mapped[int] = Column(Integer, default=0, nullable=False) + patrons_in_hold_queue: Mapped[int] = Column(Integer, default=0, nullable=False) + should_track_playtime: Mapped[bool] = Column(Boolean, default=False, nullable=False) # This lets us cache the work of figuring out the best open access # link for this LicensePool. @@ -1203,11 +1213,6 @@ def calculate_work( """ from palace.manager.sqlalchemy.model.work import Work - if not self.identifier: - # A LicensePool with no Identifier should never have a Work. - self.work = None - return None, False - if known_edition: presentation_edition = known_edition else: @@ -1503,23 +1508,23 @@ class LicensePoolDeliveryMechanism(Base): __tablename__ = "licensepooldeliveries" - id = Column(Integer, primary_key=True) + id: Mapped[int] = Column(Integer, primary_key=True) - data_source_id = Column( + data_source_id: Mapped[int] = Column( Integer, ForeignKey("datasources.id"), index=True, nullable=False ) data_source: Mapped[DataSource] = relationship( "DataSource", back_populates="delivery_mechanisms" ) - identifier_id = Column( + identifier_id: Mapped[int] = Column( Integer, ForeignKey("identifiers.id"), index=True, nullable=False ) identifier: Mapped[Identifier] = relationship( "Identifier", back_populates="delivery_mechanisms" ) - delivery_mechanism_id = Column( + delivery_mechanism_id: Mapped[int] = Column( Integer, ForeignKey("deliverymechanisms.id"), index=True, nullable=False ) delivery_mechanism: Mapped[DeliveryMechanism] = relationship( @@ -1528,7 +1533,7 @@ class LicensePoolDeliveryMechanism(Base): ) resource_id = Column(Integer, ForeignKey("resources.id"), nullable=True) - resource: Mapped[Resource] = relationship( + resource: Mapped[Resource | None] = relationship( "Resource", back_populates="licensepooldeliverymechanisms" ) @@ -1760,13 +1765,15 @@ class DeliveryMechanism(Base, HasSessionCache): } __tablename__ = "deliverymechanisms" - id = Column(Integer, primary_key=True) + id: Mapped[int] = Column(Integer, primary_key=True) content_type = Column(String) drm_scheme = Column(String) # Can the Library Simplified client fulfill a book with this # content type and this DRM scheme? - default_client_can_fulfill = Column(Boolean, default=False, index=True) + default_client_can_fulfill: Mapped[bool] = Column( + Boolean, default=False, index=True, nullable=False + ) # These are the media type/DRM scheme combos known to be supported # by the default Library Simplified client. @@ -2036,7 +2043,7 @@ class RightsStatus(Base): } __tablename__ = "rightsstatus" - id = Column(Integer, primary_key=True) + id: Mapped[int] = Column(Integer, primary_key=True) # A URI unique to the license. This may be a URL (e.g. Creative # Commons) diff --git a/src/palace/manager/sqlalchemy/model/marcfile.py b/src/palace/manager/sqlalchemy/model/marcfile.py index 8c3a555b8..28b43d7e8 100644 --- a/src/palace/manager/sqlalchemy/model/marcfile.py +++ b/src/palace/manager/sqlalchemy/model/marcfile.py @@ -1,5 +1,6 @@ from __future__ import annotations +import datetime import uuid from typing import TYPE_CHECKING @@ -18,7 +19,9 @@ class MarcFile(Base): """A record that a MARC file has been created and cached for a particular library and collection.""" __tablename__ = "marcfiles" - id = Column(UUID(as_uuid=True), primary_key=True, default=uuid.uuid4) + id: Mapped[uuid.UUID] = Column( + UUID(as_uuid=True), primary_key=True, default=uuid.uuid4 + ) # The library should never be null in normal operation, but if a library is deleted, we don't want to lose the # record of the MARC file, so we set the library to null. @@ -28,9 +31,7 @@ class MarcFile(Base): nullable=True, index=True, ) - library: Mapped[Library] = relationship( - "Library", - ) + library: Mapped[Library | None] = relationship("Library") # The collection should never be null in normal operation, but similar to the library, if a collection is deleted, # we don't want to lose the record of the MARC file, so we set the collection to null. @@ -40,15 +41,15 @@ class MarcFile(Base): nullable=True, index=True, ) - collection: Mapped[Collection] = relationship( - "Collection", - ) + collection: Mapped[Collection | None] = relationship("Collection") # The key in s3 used to store the file. - key = Column(Unicode, nullable=False) + key: Mapped[str] = Column(Unicode, nullable=False) # The creation date of the file. - created = Column(DateTime(timezone=True), nullable=False, index=True) + created: Mapped[datetime.datetime] = Column( + DateTime(timezone=True), nullable=False, index=True + ) # If the file is a delta, the date of the previous file. If the file is a full file, null. since = Column(DateTime(timezone=True), nullable=True) diff --git a/src/palace/manager/sqlalchemy/model/measurement.py b/src/palace/manager/sqlalchemy/model/measurement.py index 86b508562..c24b0b6a1 100644 --- a/src/palace/manager/sqlalchemy/model/measurement.py +++ b/src/palace/manager/sqlalchemy/model/measurement.py @@ -710,7 +710,7 @@ class Measurement(Base): DataSourceConstants.LIBRARY_STAFF: [1, 5], } - id = Column(Integer, primary_key=True) + id: Mapped[int] = Column(Integer, primary_key=True) # A Measurement is always associated with some Identifier. identifier_id = Column(Integer, ForeignKey("identifiers.id"), index=True) @@ -720,7 +720,7 @@ class Measurement(Base): # A Measurement always comes from some DataSource. data_source_id = Column(Integer, ForeignKey("datasources.id"), index=True) - data_source: Mapped[DataSource] = relationship( + data_source: Mapped[DataSource | None] = relationship( "DataSource", back_populates="measurements" ) @@ -735,7 +735,7 @@ class Measurement(Base): # How much weight should be assigned this measurement, relative to # other measurements of the same quantity from the same source. - weight = Column(Float, default=1) + weight: Mapped[float] = Column(Float, default=1, nullable=False) # When the measurement was taken taken_at = Column(DateTime(timezone=True), index=True) diff --git a/src/palace/manager/sqlalchemy/model/patron.py b/src/palace/manager/sqlalchemy/model/patron.py index 242b7f65e..88c7f5912 100644 --- a/src/palace/manager/sqlalchemy/model/patron.py +++ b/src/palace/manager/sqlalchemy/model/patron.py @@ -56,8 +56,6 @@ class LoanAndHoldMixin: def work(self) -> Work | None: """Try to find the corresponding work for this Loan/Hold.""" license_pool = self.license_pool - if not license_pool: - return None if license_pool.work: return license_pool.work if license_pool.presentation_edition and license_pool.presentation_edition.work: @@ -65,22 +63,21 @@ def work(self) -> Work | None: return None @property - def library(self) -> Library | None: - """Try to find the corresponding library for this Loan/Hold.""" - if self.patron: - return self.patron.library - # If this Loan/Hold belongs to an external patron, there may be no library. - return None + def library(self) -> Library: + """The corresponding library for this Loan/Hold.""" + return self.patron.library class Patron(Base, RedisKeyMixin): __tablename__ = "patrons" - id = Column(Integer, primary_key=True) + id: Mapped[int] = Column(Integer, primary_key=True) # Each patron is the patron _of_ one particular library. An # individual human being may patronize multiple libraries, but # they will have a different patron account at each one. - library_id = Column(Integer, ForeignKey("libraries.id"), index=True, nullable=False) + library_id: Mapped[int] = Column( + Integer, ForeignKey("libraries.id"), index=True, nullable=False + ) library: Mapped[Library] = relationship("Library", back_populates="patrons") # The patron's permanent unique identifier in an external library @@ -113,7 +110,9 @@ class Patron(Base, RedisKeyMixin): # in a way that allows users to disassociate their patron info # with account activity at any time. When this UUID is reset it effectively # dissociates any patron activity history with this patron. - uuid = Column(UUID(as_uuid=True), nullable=False, default=uuid.uuid4) + uuid: Mapped[uuid.UUID] = Column( + UUID(as_uuid=True), nullable=False, default=uuid.uuid4 + ) # The last time this record was synced up with an external library # system such as an ILS. @@ -517,13 +516,17 @@ def ensure_tuple( class Loan(Base, LoanAndHoldMixin): __tablename__ = "loans" - id = Column(Integer, primary_key=True) + id: Mapped[int] = Column(Integer, primary_key=True) - patron_id = Column(Integer, ForeignKey("patrons.id"), index=True) + patron_id: Mapped[int] = Column( + Integer, ForeignKey("patrons.id"), index=True, nullable=False + ) patron: Mapped[Patron] = relationship("Patron", back_populates="loans") # A Loan is always associated with a LicensePool. - license_pool_id = Column(Integer, ForeignKey("licensepools.id"), index=True) + license_pool_id: Mapped[int] = Column( + Integer, ForeignKey("licensepools.id"), index=True, nullable=False + ) license_pool: Mapped[LicensePool] = relationship( "LicensePool", back_populates="loans" ) @@ -531,7 +534,7 @@ class Loan(Base, LoanAndHoldMixin): # It may also be associated with an individual License if the source # provides information about individual licenses. license_id = Column(Integer, ForeignKey("licenses.id"), index=True, nullable=True) - license: Mapped[License] = relationship("License", back_populates="loans") + license: Mapped[License | None] = relationship("License", back_populates="loans") fulfillment_id = Column(Integer, ForeignKey("licensepooldeliveries.id")) fulfillment: Mapped[LicensePoolDeliveryMechanism | None] = relationship( @@ -568,9 +571,16 @@ class Hold(Base, LoanAndHoldMixin): """A patron is in line to check out a book.""" __tablename__ = "holds" - id = Column(Integer, primary_key=True) - patron_id = Column(Integer, ForeignKey("patrons.id"), index=True) - license_pool_id = Column(Integer, ForeignKey("licensepools.id"), index=True) + id: Mapped[int] = Column(Integer, primary_key=True) + patron_id: Mapped[int] = Column( + Integer, ForeignKey("patrons.id"), index=True, nullable=False + ) + patron: Mapped[Patron] = relationship( + "Patron", back_populates="holds", lazy="joined" + ) + license_pool_id: Mapped[int] = Column( + Integer, ForeignKey("licensepools.id"), index=True, nullable=False + ) license_pool: Mapped[LicensePool] = relationship( "LicensePool", back_populates="holds" ) @@ -579,10 +589,6 @@ class Hold(Base, LoanAndHoldMixin): position = Column(Integer, index=True) patron_last_notified = Column(DateTime, nullable=True) - patron: Mapped[Patron] = relationship( - "Patron", back_populates="holds", lazy="joined" - ) - def __lt__(self, other: Any) -> bool: if not isinstance(other, Hold) or self.id is None or other.id is None: return NotImplemented @@ -717,7 +723,7 @@ class Annotation(Base): ] __tablename__ = "annotations" - id = Column(Integer, primary_key=True) + id: Mapped[int] = Column(Integer, primary_key=True) patron_id = Column(Integer, ForeignKey("patrons.id"), index=True) patron: Mapped[Patron] = relationship("Patron", back_populates="annotations") @@ -728,7 +734,7 @@ class Annotation(Base): motivation = Column(Unicode, index=True) timestamp = Column(DateTime(timezone=True), index=True) - active = Column(Boolean, default=True) + active: Mapped[bool] = Column(Boolean, default=True, nullable=False) content = Column(Unicode) target = Column(Unicode) diff --git a/src/palace/manager/sqlalchemy/model/resource.py b/src/palace/manager/sqlalchemy/model/resource.py index 5bc439604..462522fba 100644 --- a/src/palace/manager/sqlalchemy/model/resource.py +++ b/src/palace/manager/sqlalchemy/model/resource.py @@ -67,7 +67,7 @@ class Resource(Base): # than a lousy cover we got from the Internet. MINIMUM_IMAGE_QUALITY = 0.25 - id = Column(Integer, primary_key=True) + id: Mapped[int] = Column(Integer, primary_key=True) # A URI that uniquely identifies this resource. Most of the time # this will be an HTTP URL, which is why we're calling it 'url', @@ -106,7 +106,7 @@ class Resource(Base): # An archived Representation of this Resource. representation_id = Column(Integer, ForeignKey("representations.id"), index=True) - representation: Mapped[Representation] = relationship( + representation: Mapped[Representation | None] = relationship( "Representation", back_populates="resource", uselist=False ) @@ -129,7 +129,7 @@ class Resource(Base): ) # A derivative resource may have one original. - derived_through: Mapped[ResourceTransformation] = relationship( + derived_through: Mapped[ResourceTransformation | None] = relationship( "ResourceTransformation", foreign_keys="ResourceTransformation.derivative_id", back_populates="derivative", @@ -143,12 +143,12 @@ class Resource(Base): # The average of human-entered values for the quality of this # resource. - voted_quality = Column(Float, default=float(0)) + voted_quality: Mapped[float] = Column(Float, default=float(0), nullable=False) # How many votes contributed to the voted_quality value. This lets # us scale new votes proportionately while keeping only two pieces # of information. - votes_for_quality = Column(Integer, default=0) + votes_for_quality: Mapped[int] = Column(Integer, default=0, nullable=False) # A combination of the calculated quality value and the # human-entered quality value. @@ -385,7 +385,7 @@ class ResourceTransformation(Base): __tablename__ = "resourcetransformations" # The derivative resource. A resource can only be derived from one other resource. - derivative_id = Column( + derivative_id: Mapped[int] = Column( Integer, ForeignKey("resources.id"), index=True, primary_key=True ) derivative: Mapped[Resource] = relationship( @@ -394,12 +394,14 @@ class ResourceTransformation(Base): # The original resource that was transformed into the derivative. original_id = Column(Integer, ForeignKey("resources.id"), index=True) - original: Mapped[Resource] = relationship( + original: Mapped[Resource | None] = relationship( "Resource", back_populates="transformations", foreign_keys=[original_id] ) # The settings used for the transformation. - settings: Mapped[dict[str, str]] = Column(MutableDict.as_mutable(JSON), default={}) + settings: Mapped[dict[str, str]] = Column( + MutableDict.as_mutable(JSON), default={}, nullable=False + ) class Hyperlink(Base, LinkRelations): @@ -407,25 +409,25 @@ class Hyperlink(Base, LinkRelations): __tablename__ = "hyperlinks" - id = Column(Integer, primary_key=True) + id: Mapped[int] = Column(Integer, primary_key=True) # A Hyperlink is always associated with some Identifier. - identifier_id = Column( + identifier_id: Mapped[int] = Column( Integer, ForeignKey("identifiers.id"), index=True, nullable=False ) identifier: Mapped[Identifier] = relationship("Identifier", back_populates="links") # The DataSource through which this link was discovered. - data_source_id = Column( + data_source_id: Mapped[int] = Column( Integer, ForeignKey("datasources.id"), index=True, nullable=False ) data_source: Mapped[DataSource] = relationship("DataSource", back_populates="links") # The link relation between the Identifier and the Resource. - rel = Column(Unicode, index=True, nullable=False) + rel: Mapped[str] = Column(Unicode, index=True, nullable=False) # The Resource on the other end of the link. - resource_id = Column( + resource_id: Mapped[int] = Column( Integer, ForeignKey("resources.id"), index=True, nullable=False ) resource: Mapped[Resource] = relationship("Resource", back_populates="links") @@ -477,7 +479,7 @@ class Representation(Base, MediaTypes): """ __tablename__ = "representations" - id = Column(Integer, primary_key=True) + id: Mapped[int] = Column(Integer, primary_key=True) # URL from which the representation was fetched. url = Column(Unicode, index=True) @@ -485,7 +487,7 @@ class Representation(Base, MediaTypes): # The media type of the representation. media_type = Column(Unicode) - resource: Mapped[Resource] = relationship( + resource: Mapped[Resource | None] = relationship( "Resource", back_populates="representation", uselist=False ) diff --git a/src/palace/manager/sqlalchemy/model/saml.py b/src/palace/manager/sqlalchemy/model/saml.py index ba27410e7..214af282e 100644 --- a/src/palace/manager/sqlalchemy/model/saml.py +++ b/src/palace/manager/sqlalchemy/model/saml.py @@ -11,9 +11,9 @@ class SAMLFederation(Base): __tablename__ = "samlfederations" - id = Column(Integer, primary_key=True) - type = Column(String(256), nullable=False, unique=True) - idp_metadata_service_url = Column(String(2048), nullable=False) + id: Mapped[int] = Column(Integer, primary_key=True) + type: Mapped[str] = Column(String(256), nullable=False, unique=True) + idp_metadata_service_url: Mapped[str] = Column(String(2048), nullable=False) last_updated_at = Column(DateTime(), nullable=True) certificate = Column(Text(), nullable=True) @@ -81,14 +81,14 @@ class SAMLFederatedIdentityProvider(Base): __tablename__ = "samlfederatedidps" - id = Column(Integer, primary_key=True) - entity_id = Column(String(256), nullable=False) - display_name = Column(String(256), nullable=False) + id: Mapped[int] = Column(Integer, primary_key=True) + entity_id: Mapped[str] = Column(String(256), nullable=False) + display_name: Mapped[str] = Column(String(256), nullable=False) - xml_metadata = Column(Text(), nullable=False) + xml_metadata: Mapped[str] = Column(Text(), nullable=False) federation_id = Column(Integer, ForeignKey("samlfederations.id"), index=True) - federation: Mapped[SAMLFederation] = relationship( + federation: Mapped[SAMLFederation | None] = relationship( "SAMLFederation", foreign_keys=federation_id, back_populates="identity_providers", diff --git a/src/palace/manager/sqlalchemy/model/time_tracking.py b/src/palace/manager/sqlalchemy/model/time_tracking.py index 4309bbcc8..0a9480dd0 100644 --- a/src/palace/manager/sqlalchemy/model/time_tracking.py +++ b/src/palace/manager/sqlalchemy/model/time_tracking.py @@ -31,7 +31,7 @@ class PlaytimeEntry(Base): __tablename__ = "playtime_entries" - id = Column(Integer, autoincrement=True, primary_key=True) + id: Mapped[int] = Column(Integer, autoincrement=True, primary_key=True) # Even if related objects are deleted, we keep our row. identifier_id = Column( @@ -39,39 +39,38 @@ class PlaytimeEntry(Base): ForeignKey("identifiers.id", onupdate="CASCADE", ondelete="SET NULL"), nullable=True, ) + identifier: Mapped[Identifier | None] = relationship("Identifier", uselist=False) collection_id = Column( Integer, ForeignKey("collections.id", onupdate="CASCADE", ondelete="SET NULL"), nullable=True, ) + collection: Mapped[Collection | None] = relationship("Collection", uselist=False) library_id = Column( Integer, ForeignKey("libraries.id", onupdate="CASCADE", ondelete="SET NULL"), nullable=True, ) + library: Mapped[Library | None] = relationship("Library", uselist=False) # Related objects can be deleted, so we keep string representation. - identifier_str = Column(String, nullable=False) - collection_name = Column(String, nullable=False) - library_name = Column(String, nullable=False) + identifier_str: Mapped[str] = Column(String, nullable=False) + collection_name: Mapped[str] = Column(String, nullable=False) + library_name: Mapped[str] = Column(String, nullable=False) timestamp: Mapped[datetime.datetime] = Column( DateTime(timezone=True), nullable=False ) - total_seconds_played = Column( + total_seconds_played: Mapped[int] = Column( Integer, CheckConstraint( "total_seconds_played <= 60", name="max_total_seconds_played_constraint" ), nullable=False, ) - tracking_id = Column(String(64), nullable=False) - processed = Column(Boolean, default=False) + tracking_id: Mapped[str] = Column(String(64), nullable=False) + processed: Mapped[bool] = Column(Boolean, default=False, nullable=False) - identifier: Mapped[Identifier] = relationship("Identifier", uselist=False) - collection: Mapped[Collection] = relationship("Collection", uselist=False) - library: Mapped[Library] = relationship("Library", uselist=False) - - loan_identifier = Column(String(40), nullable=False) + loan_identifier: Mapped[str] = Column(String(40), nullable=False) __table_args__ = ( UniqueConstraint( @@ -87,7 +86,7 @@ class PlaytimeEntry(Base): class PlaytimeSummary(Base): __tablename__ = "playtime_summaries" - id = Column(Integer, autoincrement=True, primary_key=True) + id: Mapped[int] = Column(Integer, autoincrement=True, primary_key=True) # Even if related objects are deleted, we keep our row. identifier_id = Column( @@ -96,22 +95,25 @@ class PlaytimeSummary(Base): nullable=True, index=True, ) + identifier: Mapped[Identifier | None] = relationship("Identifier", uselist=False) collection_id = Column( Integer, ForeignKey("collections.id", onupdate="CASCADE", ondelete="SET NULL"), nullable=True, index=True, ) + collection: Mapped[Collection | None] = relationship("Collection", uselist=False) library_id = Column( Integer, ForeignKey("libraries.id", onupdate="CASCADE", ondelete="SET NULL"), nullable=True, index=True, ) + library: Mapped[Library | None] = relationship("Library", uselist=False) # Related objects can be deleted, so we keep string representation. - identifier_str = Column(String, nullable=False) - collection_name = Column(String, nullable=False) - library_name = Column(String, nullable=False) + identifier_str: Mapped[str] = Column(String, nullable=False) + collection_name: Mapped[str] = Column(String, nullable=False) + library_name: Mapped[str] = Column(String, nullable=False) # This should be a per-minute datetime timestamp: Mapped[datetime.datetime] = Column( @@ -123,15 +125,11 @@ class PlaytimeSummary(Base): nullable=False, ) - total_seconds_played = Column(Integer, default=0) + total_seconds_played: Mapped[int] = Column(Integer, default=0, nullable=False) title = Column(String) isbn = Column(String) - loan_identifier = Column(String(40), nullable=False) - - identifier: Mapped[Identifier] = relationship("Identifier", uselist=False) - collection: Mapped[Collection] = relationship("Collection", uselist=False) - library: Mapped[Library] = relationship("Library", uselist=False) + loan_identifier: Mapped[str] = Column(String(40), nullable=False) __table_args__ = ( UniqueConstraint( diff --git a/src/palace/manager/sqlalchemy/model/work.py b/src/palace/manager/sqlalchemy/model/work.py index 9c0cc4263..71ed73843 100644 --- a/src/palace/manager/sqlalchemy/model/work.py +++ b/src/palace/manager/sqlalchemy/model/work.py @@ -82,14 +82,18 @@ class WorkGenre(Base): """An assignment of a genre to a work.""" __tablename__ = "workgenres" - id = Column(Integer, primary_key=True) - genre_id = Column(Integer, ForeignKey("genres.id"), index=True) + id: Mapped[int] = Column(Integer, primary_key=True) + genre_id: Mapped[int] = Column( + Integer, ForeignKey("genres.id"), index=True, nullable=False + ) genre: Mapped[Genre] = relationship("Genre", back_populates="work_genres") - work_id = Column(Integer, ForeignKey("works.id"), index=True) + work_id: Mapped[int] = Column( + Integer, ForeignKey("works.id"), index=True, nullable=False + ) work: Mapped[Work] = relationship("Work", back_populates="work_genres") - affinity = Column(Float, index=True, default=0) + affinity: Mapped[float] = Column(Float, index=True, default=0, nullable=False) @classmethod def from_genre(cls, genre): @@ -140,7 +144,7 @@ class Work(Base, LoggerMixin): } __tablename__ = "works" - id = Column(Integer, primary_key=True) + id: Mapped[int] = Column(Integer, primary_key=True) # One Work may have copies scattered across many LicensePools. license_pools: Mapped[list[LicensePool]] = relationship( @@ -175,7 +179,7 @@ class Work(Base, LoggerMixin): target_age = Column(INT4RANGE, index=True) fiction = Column(Boolean, index=True) - summary_id: Mapped[int] = Column( + summary_id = Column( Integer, ForeignKey("resources.id", use_alter=True, name="fk_works_summary_id"), index=True, @@ -223,7 +227,9 @@ class Work(Base, LoggerMixin): # This is set to True once all metadata and availability # information has been obtained for this Work. Until this is True, # the work will not show up in feeds. - presentation_ready = Column(Boolean, default=False, index=True) + presentation_ready: Mapped[bool] = Column( + Boolean, default=False, index=True, nullable=False + ) # This is the last time we tried to make this work presentation ready. presentation_ready_attempt = Column( @@ -1878,6 +1884,7 @@ def delete( def add_work_to_customlists_for_collection(pool_or_work: LicensePool | Work) -> None: + work: Work | None if isinstance(pool_or_work, Work): work = pool_or_work pools = work.license_pools diff --git a/src/palace/manager/util/notifications.py b/src/palace/manager/util/notifications.py index 7e731dc88..c78d6c75a 100644 --- a/src/palace/manager/util/notifications.py +++ b/src/palace/manager/util/notifications.py @@ -112,11 +112,15 @@ def send_loan_expiry_message( - Which patron and make the loans api request with the right authentication""" url = self.base_url edition = loan.license_pool.presentation_edition + if edition is None: + self.log.error( + f"Failed to send loan expiry notification because the edition is missing for " + f"loan '{loan.id}', patron '{loan.patron.authorization_identifier}', lp '{loan.license_pool.id}'" + ) + return [] + identifier = loan.license_pool.identifier library = loan.library - # It shouldn't be possible to get here for a loan without a library, but for mypy - # and safety we will assert it anyway - assert library is not None library_short_name = library.short_name library_name = library.name title = f"Only {days_to_expiry} {'days' if days_to_expiry != 1 else 'day'} left on your loan!" diff --git a/tests/fixtures/database.py b/tests/fixtures/database.py index 13933062d..d6b3037e9 100644 --- a/tests/fixtures/database.py +++ b/tests/fixtures/database.py @@ -725,7 +725,7 @@ def edition( unlimited_access=False, ): id = identifier_id or self.fresh_str() - source = DataSource.lookup(self.session, data_source_name) + source = DataSource.lookup(self.session, data_source_name, autocreate=True) wr = Edition.for_foreign_id(self.session, source, identifier_type, id)[0] if not title: title = self.fresh_str() diff --git a/tests/manager/api/admin/controller/test_custom_lists.py b/tests/manager/api/admin/controller/test_custom_lists.py index 37ea54842..8c5b54e82 100644 --- a/tests/manager/api/admin/controller/test_custom_lists.py +++ b/tests/manager/api/admin/controller/test_custom_lists.py @@ -1054,12 +1054,8 @@ def test_share_locally_delete(self, admin_librarian_fixture: AdminLibrarianFixtu # First, we are shared with a library which uses the list # so we cannot delete the share status - lane_with_shared, _ = create( - admin_librarian_fixture.ctrl.db.session, - Lane, - library_id=s.shared_with.id, - customlists=[s.list], - ) + lane_with_shared = admin_librarian_fixture.ctrl.db.lane(library=s.shared_with) + lane_with_shared.customlists = [s.list] with admin_librarian_fixture.request_context_with_library_and_admin( "/", method="DELETE", library=s.primary_library @@ -1090,12 +1086,10 @@ def test_share_locally_delete(self, admin_librarian_fixture: AdminLibrarianFixtu resp = self._share_locally(s.list, s.primary_library, admin_librarian_fixture) assert resp["successes"] == 1 - lane_with_primary, _ = create( - admin_librarian_fixture.ctrl.db.session, - Lane, - library_id=s.primary_library.id, - customlists=[s.list], + lane_with_primary = admin_librarian_fixture.ctrl.db.lane( + library=s.primary_library, ) + lane_with_primary.customlists = [s.list] with admin_librarian_fixture.request_context_with_library_and_admin( "/", method="DELETE", library=s.primary_library ): @@ -1113,6 +1107,7 @@ def test_auto_update_edit(self, admin_librarian_fixture: AdminLibrarianFixture): custom_list, _ = admin_librarian_fixture.ctrl.db.customlist( data_source_name=DataSource.LIBRARY_STAFF, num_entries=0 ) + custom_list.library = admin_librarian_fixture.ctrl.db.default_library() custom_list.add_entry(w1) custom_list.auto_update_enabled = True custom_list.auto_update_query = '{"query":"...."}' @@ -1120,6 +1115,7 @@ def test_auto_update_edit(self, admin_librarian_fixture: AdminLibrarianFixture): admin_librarian_fixture.ctrl.db.session.commit() assert isinstance(custom_list.name, str) + assert custom_list.library is not None response = admin_librarian_fixture.manager.admin_custom_lists_controller._create_or_update_list( custom_list.library, custom_list.name, diff --git a/tests/manager/api/admin/test_password_admin_authentication_provider.py b/tests/manager/api/admin/test_password_admin_authentication_provider.py index 2688afcab..a9e4f6633 100644 --- a/tests/manager/api/admin/test_password_admin_authentication_provider.py +++ b/tests/manager/api/admin/test_password_admin_authentication_provider.py @@ -1,7 +1,6 @@ -from unittest.mock import MagicMock, create_autospec +from unittest.mock import create_autospec import pytest -from pytest import LogCaptureFixture from palace.manager.api.admin.password_admin_authentication_provider import ( PasswordAdminAuthenticationProvider, @@ -130,20 +129,3 @@ def test_sign_in_case_insensitive( assert "ADMIN2@example.org" == admin_details.get("email") assert PasswordAdminAuthenticationProvider.NAME == admin_details.get("type") assert "foo" == redirect - - def test_send_reset_password_email( - self, - password_auth_provider: PasswordAdminAuthenticationProviderFixture, - caplog: LogCaptureFixture, - ): - password_auth = password_auth_provider.password_auth - mock_admin = MagicMock() - mock_admin.email = None - assert ( - password_auth.send_reset_password_email(mock_admin, "reset_password_url") - is None - ) - assert ( - "Admin has no email address, cannot send reset password email" - in caplog.text - ) diff --git a/tests/manager/api/controller/test_playtime_entries.py b/tests/manager/api/controller/test_playtime_entries.py index fb8341961..f94b662cf 100644 --- a/tests/manager/api/controller/test_playtime_entries.py +++ b/tests/manager/api/controller/test_playtime_entries.py @@ -201,7 +201,7 @@ def test_track_playtime_duplicate_id_ok( library_id=library.id, identifier_str=identifier.urn, collection_name=collection.name, - library_name=library.name, + library_name=library.name or "", loan_identifier=loan_identifier, ) ) diff --git a/tests/manager/api/controller/test_work.py b/tests/manager/api/controller/test_work.py index 4a92038de..5ca4b53c8 100644 --- a/tests/manager/api/controller/test_work.py +++ b/tests/manager/api/controller/test_work.py @@ -29,7 +29,6 @@ from palace.manager.feed.types import WorkEntry from palace.manager.search.external_search import SortKeyPagination from palace.manager.sqlalchemy.model.datasource import DataSource -from palace.manager.sqlalchemy.model.edition import Edition from palace.manager.sqlalchemy.model.identifier import Identifier from palace.manager.sqlalchemy.model.lane import Facets, FeaturedFacets from palace.manager.sqlalchemy.model.licensing import LicensePool @@ -46,18 +45,13 @@ class WorkFixture(CirculationControllerFixture): - lp: LicensePool - identifier: Identifier - datasource: DataSource - edition: Edition - def __init__( self, db: DatabaseTransactionFixture, services_fixture: ServicesFixture ): super().__init__(db, services_fixture) [self.lp] = self.english_1.license_pools self.edition = self.lp.presentation_edition - self.datasource = self.lp.data_source.name # type: ignore + self.datasource = self.lp.data_source.name self.identifier = self.lp.identifier diff --git a/tests/manager/api/test_circulationapi.py b/tests/manager/api/test_circulationapi.py index 7c367bf1b..27042a399 100644 --- a/tests/manager/api/test_circulationapi.py +++ b/tests/manager/api/test_circulationapi.py @@ -38,7 +38,7 @@ from palace.manager.sqlalchemy.model.datasource import DataSource from palace.manager.sqlalchemy.model.identifier import Identifier from palace.manager.sqlalchemy.model.licensing import DeliveryMechanism, RightsStatus -from palace.manager.sqlalchemy.model.patron import Hold, Loan +from palace.manager.sqlalchemy.model.patron import Loan from palace.manager.sqlalchemy.model.resource import Hyperlink, Representation from palace.manager.util.datetime_helpers import utc_now from tests.fixtures.database import DatabaseTransactionFixture @@ -1100,53 +1100,6 @@ def patron_activity_circulation_api( class TestPatronActivityCirculationAPI: - def test_sync_patron_activity_ignores_local_loan_with_no_identifier( - self, - db: DatabaseTransactionFixture, - patron_activity_circulation_api: PatronActivityCirculationAPIFixture, - ): - loan, _ = patron_activity_circulation_api.pool.loan_to( - patron_activity_circulation_api.patron - ) - loan.start = patron_activity_circulation_api.yesterday - patron_activity_circulation_api.pool.identifier = None - - # Verify that we can sync without crashing. - patron_activity_circulation_api.sync_patron_activity() - - # The invalid loan was ignored and is still there. - loans = db.session.query(Loan).all() - assert [loan] == loans - - # Even worse - the loan has no license pool! - loan.license_pool = None - - # But we can still sync without crashing. - patron_activity_circulation_api.sync_patron_activity() - - def test_sync_patron_activity_ignores_local_hold_with_no_identifier( - self, - db: DatabaseTransactionFixture, - patron_activity_circulation_api: PatronActivityCirculationAPIFixture, - ): - hold, _ = patron_activity_circulation_api.pool.on_hold_to( - patron_activity_circulation_api.patron - ) - patron_activity_circulation_api.pool.identifier = None - - # Verify that we can sync without crashing. - patron_activity_circulation_api.sync_patron_activity() - - # The invalid hold was ignored and is still there. - holds = db.session.query(Hold).all() - assert [hold] == holds - - # Even worse - the hold has no license pool! - hold.license_pool = None - - # But we can still sync without crashing. - patron_activity_circulation_api.sync_patron_activity() - def test_sync_patron_activity_with_old_local_loan_and_no_remote_loan_deletes_local_loan( self, db: DatabaseTransactionFixture, diff --git a/tests/manager/celery/tasks/test_opds_odl.py b/tests/manager/celery/tasks/test_opds_odl.py index 500448bda..213802ed1 100644 --- a/tests/manager/celery/tasks/test_opds_odl.py +++ b/tests/manager/celery/tasks/test_opds_odl.py @@ -1,5 +1,4 @@ from datetime import datetime, timedelta -from typing import cast from unittest.mock import call, patch import pytest @@ -96,9 +95,7 @@ def holds( for idx in range(10) } - return cast(set[int], expired_holds), cast( - set[int], ready_non_expired_holds | not_ready_non_expired_holds - ) + return expired_holds, ready_non_expired_holds | not_ready_non_expired_holds def pool_with_licenses( self, collection: Collection, num_licenses: int = 2, available: bool = False diff --git a/tests/manager/core/test_coverage.py b/tests/manager/core/test_coverage.py index b13281842..47a4a3d17 100644 --- a/tests/manager/core/test_coverage.py +++ b/tests/manager/core/test_coverage.py @@ -1715,6 +1715,7 @@ def test_work(self, db: DatabaseTransactionFixture): # that just had to create a LicensePool using an # INTERNAL_PROCESSING DataSource rather than the DataSource # associated with the CoverageProvider. + db.session.delete(pool) identifier2 = db.identifier() identifier.licensed_through = [] collection2 = db.collection() diff --git a/tests/manager/core/test_monitor.py b/tests/manager/core/test_monitor.py index bbadb504e..0122d5507 100644 --- a/tests/manager/core/test_monitor.py +++ b/tests/manager/core/test_monitor.py @@ -854,7 +854,7 @@ class Mock(WorkSweepMonitor): w4.presentation_ready = True w2.presentation_ready = False - w3.presentation_ready = None + w3.presentation_ready = False # Two Collections, each with one book. c1 = db.collection() diff --git a/tests/manager/feed/test_library_annotator.py b/tests/manager/feed/test_library_annotator.py index c0276960c..ee73cc65b 100644 --- a/tests/manager/feed/test_library_annotator.py +++ b/tests/manager/feed/test_library_annotator.py @@ -1089,28 +1089,6 @@ def test_loans_feed_includes_annotations_link( ] assert "/annotations" in annotations_link["href"] - def test_active_loan_feed_ignores_inconsistent_local_data( - self, annotator_fixture: LibraryAnnotatorFixture - ): - patron = annotator_fixture.db.patron() - - work1 = annotator_fixture.db.work(language="eng", with_license_pool=True) - loan, ignore = work1.license_pools[0].loan_to(patron) - work2 = annotator_fixture.db.work(language="eng", with_license_pool=True) - hold, ignore = work2.license_pools[0].on_hold_to(patron) - - # Uh-oh, our local loan data is bad. - loan.license_pool.identifier = None - - # Our local hold data is also bad. - hold.license_pool = None - - # We can still get a feed... - feed_obj = OPDSAcquisitionFeed.active_loans_for(None, patron).as_response() - - # ...but it's empty. - assert "" not in str(feed_obj) - def test_acquisition_feed_includes_license_information( self, annotator_fixture: LibraryAnnotatorFixture ): diff --git a/tests/manager/feed/test_loan_and_hold_annotator.py b/tests/manager/feed/test_loan_and_hold_annotator.py index 1ea3b987e..cc07dfde7 100644 --- a/tests/manager/feed/test_loan_and_hold_annotator.py +++ b/tests/manager/feed/test_loan_and_hold_annotator.py @@ -9,7 +9,6 @@ from palace.manager.sqlalchemy.constants import EditionConstants, LinkRelations from palace.manager.sqlalchemy.model.lane import WorkList from palace.manager.sqlalchemy.model.licensing import LicensePool -from palace.manager.sqlalchemy.model.patron import Loan from palace.manager.sqlalchemy.util import get_one from tests.fixtures.database import DatabaseTransactionFixture from tests.fixtures.search import ExternalSearchFixtureFake @@ -113,14 +112,6 @@ def test_single_item_feed_without_work(self, db: DatabaseTransactionFixture): mock = MagicMock() # A loan without a pool annotator = LibraryLoanAndHoldAnnotator(mock, None, db.default_library()) - loan = Loan() - loan.patron = db.patron() - not_found_result = OPDSAcquisitionFeed.single_entry_loans_feed( - mock, - loan, - annotator, - ) - assert not_found_result == NOT_FOUND_ON_REMOTE work = db.work(with_license_pool=True) pool = get_one(db.session, LicensePool, work_id=work.id) diff --git a/tests/manager/feed/test_opds_acquisition_feed.py b/tests/manager/feed/test_opds_acquisition_feed.py index 1948a46ac..687386650 100644 --- a/tests/manager/feed/test_opds_acquisition_feed.py +++ b/tests/manager/feed/test_opds_acquisition_feed.py @@ -41,6 +41,8 @@ ) from palace.manager.sqlalchemy.model.licensing import DeliveryMechanism from palace.manager.sqlalchemy.model.resource import Representation +from palace.manager.sqlalchemy.model.work import Work +from palace.manager.sqlalchemy.util import create from palace.manager.util.datetime_helpers import utc_now from palace.manager.util.flask_util import OPDSEntryResponse, OPDSFeedResponse from palace.manager.util.opds_writer import OPDSFeed, OPDSMessage @@ -504,9 +506,7 @@ def test_error_when_work_has_no_identifier(self, db: DatabaseTransactionFixture) # We cannot create an OPDS entry for a Work that cannot be associated # with an Identifier. - work = db.work(title="Hello, World!", with_license_pool=True) - work.license_pools[0].identifier = None - work.presentation_edition.primary_identifier = None + work, _ = create(db.session, Work) with pytest.raises(PalaceValueError, match="Work has no associated identifier"): OPDSAcquisitionFeed.single_entry(work, Annotator()) diff --git a/tests/manager/scripts/test_configuration.py b/tests/manager/scripts/test_configuration.py index fea973f63..afcca2c88 100644 --- a/tests/manager/scripts/test_configuration.py +++ b/tests/manager/scripts/test_configuration.py @@ -229,15 +229,18 @@ def test_bad_arguments(self, db: DatabaseTransactionFixture): script = ConfigureLaneScript() # No lane id but no library short name for creating it either. - with pytest.raises(ValueError) as excinfo: + with pytest.raises( + ValueError, + match="Library short name and lane display name are required to create a new lane", + ): script.do_run(db.session, []) - assert "Library short name is required to create a new lane" in str( - excinfo.value - ) # Try to create a lane for a nonexistent library. with pytest.raises(ValueError) as excinfo: - script.do_run(db.session, ["--library-short-name=nosuchlibrary"]) + script.do_run( + db.session, + ["--library-short-name=nosuchlibrary", "--display-name=name"], + ) assert 'No such library: "nosuchlibrary".' in str(excinfo.value) def test_create_lane(self, db: DatabaseTransactionFixture): diff --git a/tests/manager/scripts/test_playtime_entries.py b/tests/manager/scripts/test_playtime_entries.py index b137a641b..c15886a35 100644 --- a/tests/manager/scripts/test_playtime_entries.py +++ b/tests/manager/scripts/test_playtime_entries.py @@ -47,7 +47,7 @@ def create_playtime_entries( total_seconds_played=entry.seconds_played, identifier_str=identifier.urn, collection_name=collection.name, - library_name=library.name, + library_name=library.name or "", loan_identifier=loan_identifier, ) db.session.add(inserted) diff --git a/tests/manager/search/test_external_search.py b/tests/manager/search/test_external_search.py index 38398828d..efe61b0ed 100644 --- a/tests/manager/search/test_external_search.py +++ b/tests/manager/search/test_external_search.py @@ -147,11 +147,6 @@ def test_remove_work( client.indices.refresh() end_to_end_search_fixture.expect_results([], "Moby") - # If we try to remove a work with no id, we log a warning - work_no_id = Work() - index.remove_work(work_no_id) - assert "Work has no ID" in caplog.text - def test_add_document( self, end_to_end_search_fixture: EndToEndSearchFixture, diff --git a/tests/manager/sqlalchemy/model/test_customlist.py b/tests/manager/sqlalchemy/model/test_customlist.py index 36eda66d3..9c6f7d56c 100644 --- a/tests/manager/sqlalchemy/model/test_customlist.py +++ b/tests/manager/sqlalchemy/model/test_customlist.py @@ -283,6 +283,7 @@ def test_remove_entry( assert 2 == custom_list.size # The editon's work has been scheduled for reindexing. + assert first.work is not None assert work_queue_indexing.is_queued(first.work, clear=True) # An entry is also removed if any of its equivalent editions diff --git a/tests/manager/sqlalchemy/model/test_patron.py b/tests/manager/sqlalchemy/model/test_patron.py index 00e21ae80..449a3f10f 100644 --- a/tests/manager/sqlalchemy/model/test_patron.py +++ b/tests/manager/sqlalchemy/model/test_patron.py @@ -312,9 +312,6 @@ def test_work(self, db: DatabaseTransactionFixture): loan, is_new = pool.loan_to(patron) assert work == loan.work - loan.license_pool = None - assert None == loan.work - # If pool.work is None but pool.edition.work is valid, we use that. loan.license_pool = pool pool.work = None @@ -334,9 +331,6 @@ def test_library(self, db: DatabaseTransactionFixture): loan, is_new = pool.loan_to(patron) assert db.default_library() == loan.library - loan.patron = None - assert None == loan.library - patron.library = db.library() loan.patron = patron assert patron.library == loan.library @@ -479,7 +473,7 @@ def test_cascade_delete(self, db: DatabaseTransactionFixture): assert len(db.session.query(Annotation).all()) == 1 # Give the patron a credential and check that it has been created - credential, is_new = create(db.session, Credential, patron=patron) + credential = db.credential(patron=patron) assert [credential] == patron.credentials assert len(db.session.query(Credential).all()) == 1 diff --git a/tests/manager/sqlalchemy/model/test_work.py b/tests/manager/sqlalchemy/model/test_work.py index d9717091c..bd8f711b8 100644 --- a/tests/manager/sqlalchemy/model/test_work.py +++ b/tests/manager/sqlalchemy/model/test_work.py @@ -31,6 +31,7 @@ work_library_suppressions, ) from palace.manager.sqlalchemy.util import ( + create, get_one_or_create, numericrange_to_tuple, tuple_to_numericrange, @@ -3090,28 +3091,21 @@ def test_merge_into_raises_exception_if_pwids_differ( excinfo.value ) - def test_licensepool_without_identifier_gets_no_work( - self, db: DatabaseTransactionFixture - ): - work = db.work(with_license_pool=True) - [lp] = work.license_pools - lp.identifier = None - - # Even if the LicensePool had a work before, it gets removed. - assert (None, False) == lp.calculate_work() - assert None == lp.work - def test_licensepool_without_presentation_edition_gets_no_work( self, db: DatabaseTransactionFixture ): - work = db.work(with_license_pool=True) - [lp] = work.license_pools - - # This LicensePool has no presentation edition and no way of - # getting one. - lp.presentation_edition = None - lp.identifier.primarily_identifies = [] + data_source = DataSource.lookup(db.session, DataSource.OVERDRIVE) + identifier = db.identifier() + work, _ = create(db.session, Work) + lp, _ = create( + db.session, + LicensePool, + identifier=identifier, + collection=db.default_collection(), + data_source=data_source, + work=work, + ) # Even if the LicensePool had a work before, it gets removed. - assert (None, False) == lp.calculate_work() - assert None == lp.work + assert lp.calculate_work() == (None, False) + assert lp.work is None