Update database constraints

alembic/versions/20241127_b1b08faa9811_update_database_nullable_constraints.py

@@ -0,0 +1,96 @@
+"""Update database nullable constraints
+
+Revision ID: b1b08faa9811
+Revises: 272da5f400de
+Create Date: 2024-11-27 17:05:00.839954+00:00
+
+"""
+
+import sqlalchemy as sa
+from alembic import op
+
+# revision identifiers, used by Alembic.
+revision = "b1b08faa9811"
+down_revision = "272da5f400de"
+branch_labels = None
+depends_on = None
+
+
+def upgrade() -> None:
+    # ### commands auto generated by Alembic - please adjust! ###
+    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)
+    # ### end Alembic commands ###
+
+
+def downgrade() -> None:
+    # ### commands auto generated by Alembic - please adjust! ###
+    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(
+        "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", "identifier_id", existing_type=sa.INTEGER(), nullable=True
+    )
+    op.alter_column(
+        "licensepools", "data_source_id", existing_type=sa.INTEGER(), 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", "input_id", existing_type=sa.INTEGER(), 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
+    )
+    # ### end Alembic commands ###

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

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}"

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]

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 = {}
 

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(

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,

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
 

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,

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(

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

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(

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

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)