Remove ExternalIntegration and ConfigurationSetting tables (PP-893) (#…

…1668) * Remove * Add migration * Remove unused function. * Codereview feedback.
ThePalaceProject · Feb 13, 2024 · 2429ec0 · 2429ec0
1 parent 279cbd5
commit 2429ec0
Show file tree

Hide file tree

Showing 38 changed files with 275 additions and 1,500 deletions.
diff --git a/alembic/versions/20240208_1c9f519415b5_remove_external_integration.py b/alembic/versions/20240208_1c9f519415b5_remove_external_integration.py
@@ -0,0 +1,169 @@
+"""Remove external integration
+
+Revision ID: 1c9f519415b5
+Revises: fc3c9ccf0ad8
+Create Date: 2024-02-08 23:50:50.399968+00:00
+
+"""
+import sqlalchemy as sa
+
+from alembic import op
+
+# revision identifiers, used by Alembic.
+revision = "1c9f519415b5"
+down_revision = "fc3c9ccf0ad8"
+branch_labels = None
+depends_on = None
+
+
+def upgrade() -> None:
+    op.drop_index(
+        "ix_configurationsettings_external_integration_id",
+        table_name="configurationsettings",
+    )
+    op.drop_index(
+        "ix_configurationsettings_external_integration_id_key",
+        table_name="configurationsettings",
+        postgresql_where="(library_id IS NULL)",
+    )
+    op.drop_index(
+        "ix_configurationsettings_external_integration_id_library_id_key",
+        table_name="configurationsettings",
+    )
+    op.drop_index(
+        "ix_configurationsettings_key",
+        table_name="configurationsettings",
+        postgresql_where="((external_integration_id IS NULL) AND (library_id IS NULL))",
+    )
+    op.drop_index(
+        "ix_configurationsettings_library_id", table_name="configurationsettings"
+    )
+    op.drop_index(
+        "ix_configurationsettings_library_id_key",
+        table_name="configurationsettings",
+        postgresql_where="(external_integration_id IS NULL)",
+    )
+    op.drop_table("configurationsettings")
+    op.drop_index(
+        "ix_externalintegrations_libraries_externalintegration_id",
+        table_name="externalintegrations_libraries",
+    )
+    op.drop_index(
+        "ix_externalintegrations_libraries_library_id",
+        table_name="externalintegrations_libraries",
+    )
+    op.drop_table("externalintegrations_libraries")
+    op.drop_table("externalintegrations")
+
+
+def downgrade() -> None:
+    op.create_table(
+        "externalintegrations",
+        sa.Column(
+            "id",
+            sa.INTEGER(),
+            server_default=sa.text("nextval('externalintegrations_id_seq'::regclass)"),
+            autoincrement=True,
+            nullable=False,
+        ),
+        sa.Column("protocol", sa.VARCHAR(), autoincrement=False, nullable=False),
+        sa.Column("goal", sa.VARCHAR(), autoincrement=False, nullable=True),
+        sa.Column("name", sa.VARCHAR(), autoincrement=False, nullable=True),
+        sa.PrimaryKeyConstraint("id", name="externalintegrations_pkey"),
+        sa.UniqueConstraint("name", name="externalintegrations_name_key"),
+        postgresql_ignore_search_path=False,
+    )
+    op.create_table(
+        "externalintegrations_libraries",
+        sa.Column(
+            "externalintegration_id", sa.INTEGER(), autoincrement=False, nullable=False
+        ),
+        sa.Column("library_id", sa.INTEGER(), autoincrement=False, nullable=False),
+        sa.ForeignKeyConstraint(
+            ["externalintegration_id"],
+            ["externalintegrations.id"],
+            name="externalintegrations_libraries_externalintegration_id_fkey",
+        ),
+        sa.ForeignKeyConstraint(
+            ["library_id"],
+            ["libraries.id"],
+            name="externalintegrations_libraries_library_id_fkey",
+        ),
+        sa.UniqueConstraint(
+            "externalintegration_id",
+            "library_id",
+            name="externalintegrations_librarie_externalintegration_id_librar_key",
+        ),
+    )
+    op.create_index(
+        "ix_externalintegrations_libraries_library_id",
+        "externalintegrations_libraries",
+        ["library_id"],
+        unique=False,
+    )
+    op.create_index(
+        "ix_externalintegrations_libraries_externalintegration_id",
+        "externalintegrations_libraries",
+        ["externalintegration_id"],
+        unique=False,
+    )
+    op.create_table(
+        "configurationsettings",
+        sa.Column("id", sa.INTEGER(), autoincrement=True, nullable=False),
+        sa.Column(
+            "external_integration_id", sa.INTEGER(), autoincrement=False, nullable=True
+        ),
+        sa.Column("library_id", sa.INTEGER(), autoincrement=False, nullable=True),
+        sa.Column("key", sa.VARCHAR(), autoincrement=False, nullable=True),
+        sa.Column("value", sa.VARCHAR(), autoincrement=False, nullable=True),
+        sa.ForeignKeyConstraint(
+            ["external_integration_id"],
+            ["externalintegrations.id"],
+            name="configurationsettings_external_integration_id_fkey",
+        ),
+        sa.ForeignKeyConstraint(
+            ["library_id"],
+            ["libraries.id"],
+            name="configurationsettings_library_id_fkey",
+        ),
+        sa.PrimaryKeyConstraint("id", name="configurationsettings_pkey"),
+    )
+    op.create_index(
+        "ix_configurationsettings_library_id_key",
+        "configurationsettings",
+        ["library_id", "key"],
+        unique=True,
+        postgresql_where="(external_integration_id IS NULL)",
+    )
+    op.create_index(
+        "ix_configurationsettings_library_id",
+        "configurationsettings",
+        ["library_id"],
+        unique=False,
+    )
+    op.create_index(
+        "ix_configurationsettings_key",
+        "configurationsettings",
+        ["key"],
+        unique=True,
+        postgresql_where="((external_integration_id IS NULL) AND (library_id IS NULL))",
+    )
+    op.create_index(
+        "ix_configurationsettings_external_integration_id_library_id_key",
+        "configurationsettings",
+        ["external_integration_id", "library_id", "key"],
+        unique=True,
+    )
+    op.create_index(
+        "ix_configurationsettings_external_integration_id_key",
+        "configurationsettings",
+        ["external_integration_id", "key"],
+        unique=True,
+        postgresql_where="(library_id IS NULL)",
+    )
+    op.create_index(
+        "ix_configurationsettings_external_integration_id",
+        "configurationsettings",
+        ["external_integration_id"],
+        unique=False,
+    )
diff --git a/api/authenticator.py b/api/authenticator.py
@@ -160,21 +160,20 @@ def from_config(
         analytics: Analytics | None = None,
     ) -> Self:
         """Initialize an Authenticator for the given Library based on its
-        configured ExternalIntegrations.
+        configured Integration Configuration.
         """
         # Start with an empty list of authenticators.
         authenticator = cls(_db=_db, library=library)
 
-        # Find all of this library's ExternalIntegrations set up with
+        # Find all of this library's integrations with
         # the goal of authenticating patrons.
         integrations: list[
             IntegrationLibraryConfiguration
         ] = IntegrationLibraryConfiguration.for_library_and_goal(
             _db, library, Goals.PATRON_AUTH_GOAL
         ).all()
 
-        # Turn each such ExternalIntegration into an
-        # AuthenticationProvider.
+        # Turn each such integration into an AuthenticationProvider.
         for integration in integrations:
             try:
                 authenticator.register_provider(integration, analytics)
@@ -285,10 +284,10 @@ def register_provider(
         integration: IntegrationLibraryConfiguration,
         analytics: Analytics | None = None,
     ):
-        """Turn an ExternalIntegration object into an AuthenticationProvider
+        """Turn an IntegrationLibraryConfiguration object into an AuthenticationProvider
         object, and register it.
 
-        :param integration: An ExternalIntegration that configures
+        :param integration: An IntegrationLibraryConfiguration that configures
             a way of authenticating patrons.
         """
         if integration.parent.goal != Goals.PATRON_AUTH_GOAL:

diff --git a/api/bibliotheca.py b/api/bibliotheca.py
@@ -34,7 +34,7 @@
 from api.selftest import HasCollectionSelfTests, SelfTestResult
 from api.web_publication_manifest import FindawayManifest, SpineItem
 from core.analytics import Analytics
-from core.config import CannotLoadConfiguration
+from core.config import CannotLoadConfiguration, ConfigurationAttributeValue
 from core.coverage import BibliographicCoverageProvider
 from core.integration.settings import (
     ConfigurationFormItem,
@@ -73,7 +73,6 @@
     Timestamp,
     get_one,
 )
-from core.model.configuration import ConfigurationAttributeValue
 from core.monitor import CollectionMonitor, IdentifierSweepMonitor, TimelineMonitor
 from core.scripts import RunCollectionMonitorScript
 from core.service.container import Services

diff --git a/api/discovery/opds_registration.py b/api/discovery/opds_registration.py
@@ -136,9 +136,7 @@ def post_request(
     def for_protocol_goal_and_url(
         cls, _db: Session, protocol: str, goal: Goals, url: str
     ) -> Self | None:
-        """Get a LibraryRegistry for the given protocol, goal, and
-        URL. Create the corresponding ExternalIntegration if necessary.
-        """
+        """Get a LibraryRegistry for the given protocol, goal, and URL."""
         settings = cls.settings_class().construct(url=url)  # type: ignore[arg-type]
         query = select(IntegrationConfiguration).where(
             IntegrationConfiguration.goal == goal,
@@ -499,7 +497,7 @@ def _process_registration_result(
             registration.short_name = short_name
         if encrypted_shared_secret:
             # NOTE: we can only store Unicode data in the
-            # ConfigurationSetting.value, so this requires that the
+            # shared_secret column, so this requires that the
             # shared secret encoded as UTF-8. This works for the
             # library registry product, which uses a long string of
             # hex digits as its shared secret.
@@ -514,7 +512,7 @@ def _process_registration_result(
         # communicated to the registry.
         registration.stage = desired_stage
 
-        # Store the web client URL as a ConfigurationSetting.
+        # Store the web client URL
         registration.web_client = web_client_url
 
         return True
diff --git a/api/enki.py b/api/enki.py
@@ -24,6 +24,7 @@
 from api.circulation_exceptions import *
 from api.selftest import HasCollectionSelfTests, SelfTestResult
 from core.analytics import Analytics
+from core.config import ConfigurationAttributeValue
 from core.integration.settings import (
     BaseSettings,
     ConfigurationFormItem,
@@ -56,7 +57,6 @@
     Representation,
     Subject,
 )
-from core.model.configuration import ConfigurationAttributeValue
 from core.monitor import CollectionMonitor, IdentifierSweepMonitor, TimelineMonitor
 from core.service.container import Services
 from core.util.datetime_helpers import from_timestamp, strptime_utc, utc_now

diff --git a/bin/informational/axis-raw-bibliographic b/bin/informational/axis-raw-bibliographic
@@ -7,9 +7,9 @@ bin_dir = os.path.split(__file__)[0]
 package_dir = os.path.join(bin_dir, "..", "..")
 sys.path.append(os.path.abspath(package_dir))
 
-from api.axis import Axis360API  # noqa: E402
-from core.model import Collection, ExternalIntegration  # noqa: E402
-from core.scripts import IdentifierInputScript  # noqa: E402
+from api.axis import Axis360API
+from core.model import Collection
+from core.scripts import IdentifierInputScript
 
 
 class Axis360RawBibliographicScript(IdentifierInputScript):

diff --git a/bin/informational/axis-raw-patron-status b/bin/informational/axis-raw-patron-status
@@ -7,9 +7,9 @@ bin_dir = os.path.split(__file__)[0]
 package_dir = os.path.join(bin_dir, "..", "..")
 sys.path.append(os.path.abspath(package_dir))
 
-from api.axis import Axis360API  # noqa: E402
-from core.model import Collection, ExternalIntegration  # noqa: E402
-from core.scripts import Script  # noqa: E402
+from api.axis import Axis360API
+from core.model import Collection
+from core.scripts import Script
 
 
 class Axis360RawPatronActivityScript(Script):

diff --git a/bin/informational/bibliotheca-raw-bibliographic b/bin/informational/bibliotheca-raw-bibliographic
@@ -7,9 +7,9 @@ bin_dir = os.path.split(__file__)[0]
 package_dir = os.path.join(bin_dir, "..", "..")
 sys.path.append(os.path.abspath(package_dir))
 
-from api.bibliotheca import BibliothecaAPI  # noqa: E402
-from core.model import Collection, ExternalIntegration  # noqa: E402
-from core.scripts import IdentifierInputScript  # noqa: E402
+from api.bibliotheca import BibliothecaAPI
+from core.model import Collection
+from core.scripts import IdentifierInputScript
 
 
 class BibliothecaRawBibliographicScript(IdentifierInputScript):

diff --git a/bin/informational/bibliotheca-raw-patron-status b/bin/informational/bibliotheca-raw-patron-status
@@ -7,9 +7,9 @@ bin_dir = os.path.split(__file__)[0]
 package_dir = os.path.join(bin_dir, "..", "..")
 sys.path.append(os.path.abspath(package_dir))
 
-from api.bibliotheca import BibliothecaAPI  # noqa: E402
-from core.model import Collection, ExternalIntegration, Patron  # noqa: E402
-from core.scripts import Script  # noqa: E402
+from api.bibliotheca import BibliothecaAPI
+from core.model import Collection, Patron
+from core.scripts import Script
 
 
 class BibliothecaRawPatronStatusScript(Script):

diff --git a/bin/informational/overdrive-raw-bibliographic b/bin/informational/overdrive-raw-bibliographic
@@ -7,9 +7,9 @@ bin_dir = os.path.split(__file__)[0]
 package_dir = os.path.join(bin_dir, "..", "..")
 sys.path.append(os.path.abspath(package_dir))
 
-from api.overdrive import OverdriveAPI  # noqa: E402
-from core.model import Collection, ExternalIntegration  # noqa: E402
-from core.scripts import IdentifierInputScript  # noqa: E402
+from api.overdrive import OverdriveAPI
+from core.model import Collection
+from core.scripts import IdentifierInputScript
 
 
 class OverdriveRawBibliographicScript(IdentifierInputScript):

diff --git a/bin/informational/overdrive-raw-circulation b/bin/informational/overdrive-raw-circulation
@@ -7,9 +7,9 @@ bin_dir = os.path.split(__file__)[0]
 package_dir = os.path.join(bin_dir, "..", "..")
 sys.path.append(os.path.abspath(package_dir))
 
-from api.overdrive import OverdriveAPI  # noqa: E402
-from core.model import Collection, ExternalIntegration  # noqa: E402
-from core.scripts import IdentifierInputScript  # noqa: E402
+from api.overdrive import OverdriveAPI
+from core.model import Collection
+from core.scripts import IdentifierInputScript
 
 
 class OverdriveRawCirculationScript(IdentifierInputScript):

diff --git a/core/config.py b/core/config.py
@@ -1,6 +1,9 @@
+from __future__ import annotations
+
 import json
 import logging
 import os
+from enum import Enum
 
 from sqlalchemy.engine.url import make_url
 from sqlalchemy.exc import ArgumentError
@@ -28,6 +31,13 @@ class ConfigurationConstants:
     FALSE = "false"
 
 
+class ConfigurationAttributeValue(Enum):
+    """Enumeration of common configuration attribute values"""
+
+    YESVALUE = "yes"
+    NOVALUE = "no"
+
+
 class Configuration(ConfigurationConstants):
     log = logging.getLogger("Configuration file loader")
 

diff --git a/core/coverage.py b/core/coverage.py
@@ -1103,10 +1103,8 @@ class CollectionCoverageProvider(IdentifierCoverageProvider):
     In addition to defining the class variables defined by
     CoverageProvider, you must define the class variable PROTOCOL when
     subclassing this class. This is the entity that provides the
-    licenses for this Collection. It should be one of the
-    collection-type provider constants defined in the
-    `ExternalIntegration` class, such as
-    ExternalIntegration.OPDS_IMPORT or ExternalIntegration.OVERDRIVE.
+    licenses for this Collection. It should be a valid protocol
+    in the LICENSE_GOAL registry.
     """
 
     # By default, this type of CoverageProvider will provide coverage to