Refactor how we load the analytics storage service for s3 analytics p…

…rovider.

api/app.py

@@ -17,7 +17,7 @@
     SessionManager,
     pg_advisory_lock,
 )
-from core.service.container import create_container
+from core.service.container import container_instance
 from core.util import LanguageCodes
 from core.util.cache import CachedData
 from scripts import InstanceInitializationScript
@@ -73,7 +73,7 @@ def initialize_circulation_manager():
         pass
     else:
         if getattr(app, "manager", None) is None:
-            container = create_container()
+            container = container_instance()
             try:
                 app.manager = CirculationManager(app._db, container)
             except Exception:

api/google_analytics_provider.py

@@ -6,6 +6,7 @@
 from flask_babel import lazy_gettext as _
 
 from core.model import ConfigurationSetting, ExternalIntegration, Session
+from core.service.container import Services
 from core.util.http import HTTP
 
 from .config import CannotLoadConfiguration
@@ -63,7 +64,7 @@ class GoogleAnalyticsProvider:
         {"key": TRACKING_ID, "label": _("Tracking ID"), "required": True},
     ]
 
-    def __init__(self, integration, library=None):
+    def __init__(self, integration, services: Services, library=None):
         _db = Session.object_session(integration)
         if not library:
             raise CannotLoadConfiguration(

api/s3_analytics_provider.py

@@ -4,15 +4,14 @@
 import string
 from typing import Dict, Optional
 
-from dependency_injector.wiring import Provide, inject
 from flask_babel import lazy_gettext as _
 from sqlalchemy.orm import Session
 
 from core.config import CannotLoadConfiguration
 from core.local_analytics_provider import LocalAnalyticsProvider
 from core.model import Library, LicensePool, MediaTypes
 from core.model.configuration import ConfigurationGrouping
-from core.service.storage.container import Storage
+from core.service.container import Services
 from core.service.storage.s3 import S3Service
 
 
@@ -30,15 +29,13 @@ class S3AnalyticsProvider(LocalAnalyticsProvider):
         LocalAnalyticsProvider.SETTINGS + S3AnalyticsProviderConfiguration.to_settings()
     )
 
-    @inject
     def __init__(
         self,
         integration,
+        services: Services,
         library=None,
-        s3_service: Optional[S3Service] = Provide[Storage.analytics],
     ):
-        self.s3_service = s3_service
-        super().__init__(integration, library)
+        super().__init__(integration, services, library)
 
     @staticmethod
     def _create_event_object(
@@ -252,12 +249,13 @@ def _get_storage(self) -> S3Service:
 
         :return: StorageServiceBase object
         """
-        if not self.s3_service:
+        s3_storage_service = self.services.storage.analytics()
+        if s3_storage_service is None:
             raise CannotLoadConfiguration(
                 "No storage service is configured with an analytics bucket."
             )
 
-        return self.s3_service
+        return s3_storage_service
 
 
 Provider = S3AnalyticsProvider

core/analytics.py

@@ -1,3 +1,5 @@
+from __future__ import annotations
+
 import importlib
 import logging
 from collections import defaultdict
@@ -7,6 +9,7 @@
 
 from .config import CannotLoadConfiguration
 from .model import ExternalIntegration
+from .service.container import container_instance
 from .util.datetime_helpers import utc_now
 from .util.log import log_elapsed_time
 
@@ -26,7 +29,7 @@ class Analytics:
     GLOBAL_ENABLED: Optional[bool] = None
     LIBRARY_ENABLED: Set[int] = set()
 
-    def __new__(cls, _db, refresh=False) -> "Analytics":
+    def __new__(cls, _db: Session, refresh: bool = False) -> Analytics:
         instance = cls._singleton_instance
         if instance is None:
             refresh = True
@@ -44,15 +47,16 @@ def _reset_singleton_instance(cls):
         cls._singleton_instance = None
 
     @log_elapsed_time(log_method=log.debug, message_prefix="Initializing instance")
-    def _initialize_instance(self, _db):
+    def _initialize_instance(self, _db: Session) -> None:
         """Initialize an instance (usually the singleton) of the class.
 
         We don't use __init__ because it would be run whether or not
         a new instance were instantiated.
         """
+        services = container_instance()
         sitewide_providers = []
         library_providers = defaultdict(list)
-        initialization_exceptions: Dict[int, Exception] = {}
+        initialization_exceptions: Dict[int, Exception | str] = {}
         global_enabled = False
         library_enabled = set()
         # Find a list of all the ExternalIntegrations set up with a
@@ -68,12 +72,12 @@ def _initialize_instance(self, _db):
                 provider_class = self._provider_class_from_module(module)
                 if provider_class:
                     if not libraries:
-                        provider = provider_class(integration)
+                        provider = provider_class(integration, services)
                         sitewide_providers.append(provider)
                         global_enabled = True
                     else:
                         for library in libraries:
-                            provider = provider_class(integration, library)
+                            provider = provider_class(integration, services, library)
                             library_providers[library.id].append(provider)
                             library_enabled.add(library.id)
                 else:

core/local_analytics_provider.py

@@ -2,6 +2,7 @@
 from sqlalchemy.orm.session import Session
 
 from .model import CirculationEvent, ExternalIntegration, create, get_one
+from .service.container import Services
 
 
 class LocalAnalyticsProvider:
@@ -41,12 +42,13 @@ class LocalAnalyticsProvider:
         },
     ]
 
-    def __init__(self, integration, library=None):
+    def __init__(self, integration, services: Services, library=None):
         self.integration_id = integration.id
         self.location_source = (
             integration.setting(self.LOCATION_SOURCE).value
             or self.LOCATION_SOURCE_DISABLED
         )
+        self.services = services
         if library:
             self.library_id = library.id
         else:

core/mock_analytics_provider.py

@@ -1,7 +1,7 @@
 class MockAnalyticsProvider:
     """A mock analytics provider that keeps track of how many times it's called."""
 
-    def __init__(self, integration=None, library=None):
+    def __init__(self, integration=None, services=None, library=None):
         self.count = 0
         self.event = None
         if integration:

core/scripts.py

@@ -58,7 +58,7 @@
 from .monitor import CollectionMonitor, ReaperMonitor
 from .opds_import import OPDSImporter, OPDSImportMonitor
 from .overdrive import OverdriveCoreAPI
-from .service.container import Services, create_container
+from .service.container import Services, container_instance
 from .util import fast_query_count
 from .util.datetime_helpers import strptime_utc, utc_now
 from .util.personal_names import contributor_name_match_ratio, display_name_to_sort_name
@@ -125,7 +125,7 @@ def __init__(self, _db=None, services: Optional[Services] = None, *args, **kwarg
             self._session = _db
 
         if services is None:
-            services = create_container()
+            services = container_instance()
         self._services = services
 
     def run(self):

core/service/container.py

@@ -19,3 +19,19 @@ def create_container() -> Services:
     container = Services()
     container.config.from_dict({"storage": StorageConfiguration().dict()})
     return container
+
+
+_container_instance = None
+
+
+def container_instance() -> Services:
+    # Create a singleton container instance, I'd like this to be used sparingly
+    # and eventually have it go away, but there are places in the code that
+    # are currently difficult to refactor to pass the container into the
+    # constructor.
+    # If at all possible please use the container that is stored in the CirculationManager
+    # or Scripts classes instead of using this function.
+    global _container_instance
+    if _container_instance is None:
+        _container_instance = create_container()
+    return _container_instance

core/service/storage/container.py

@@ -1,21 +1,11 @@
 import boto3
 from dependency_injector import providers
-from dependency_injector.containers import DeclarativeContainer, WiringConfiguration
+from dependency_injector.containers import DeclarativeContainer
 
 from core.service.storage.s3 import S3Service
 
 
 class Storage(DeclarativeContainer):
-
-    # See https://python-dependency-injector.ets-labs.org/wiring.html
-    # This lists modules that contain markers that will be used to wire up
-    # dependencies from this container automatically.
-    wiring_config = WiringConfiguration(
-        modules=[
-            "api.s3_analytics_provider",
-        ],
-    )
-
     config = providers.Configuration()
 
     s3_client = providers.Singleton(

tests/api/test_google_analytics_provider.py

@@ -1,5 +1,6 @@
 import unicodedata
 import urllib.parse
+from unittest.mock import MagicMock
 
 import pytest
 from psycopg2.extras import NumericRange
@@ -37,13 +38,13 @@ def test_init(self, db: DatabaseTransactionFixture):
         )
 
         with pytest.raises(CannotLoadConfiguration) as excinfo:
-            GoogleAnalyticsProvider(integration)
+            GoogleAnalyticsProvider(integration, MagicMock())
         assert "Google Analytics can't be configured without a library." in str(
             excinfo.value
         )
 
         with pytest.raises(CannotLoadConfiguration) as excinfo:
-            GoogleAnalyticsProvider(integration, db.default_library())
+            GoogleAnalyticsProvider(integration, MagicMock(), db.default_library())
         assert (
             "Missing tracking id for library %s" % db.default_library().short_name
             in str(excinfo.value)
@@ -55,12 +56,12 @@ def test_init(self, db: DatabaseTransactionFixture):
             db.default_library(),
             integration,
         ).value = "faketrackingid"
-        ga = GoogleAnalyticsProvider(integration, db.default_library())
+        ga = GoogleAnalyticsProvider(integration, MagicMock(), db.default_library())
         assert GoogleAnalyticsProvider.DEFAULT_URL == ga.url
         assert "faketrackingid" == ga.tracking_id
 
         integration.url = db.fresh_str()
-        ga = GoogleAnalyticsProvider(integration, db.default_library())
+        ga = GoogleAnalyticsProvider(integration, MagicMock(), db.default_library())
         assert integration.url == ga.url
         assert "faketrackingid" == ga.tracking_id
 
@@ -78,7 +79,7 @@ def test_collect_event_with_work(self, db: DatabaseTransactionFixture):
             db.default_library(),
             integration,
         ).value = "faketrackingid"
-        ga = MockGoogleAnalyticsProvider(integration, db.default_library())
+        ga = MockGoogleAnalyticsProvider(integration, MagicMock(), db.default_library())
 
         work = db.work(
             title="pi\u00F1ata",
@@ -146,7 +147,7 @@ def test_collect_event_without_work(self, db: DatabaseTransactionFixture):
             db.default_library(),
             integration,
         ).value = "faketrackingid"
-        ga = MockGoogleAnalyticsProvider(integration, db.default_library())
+        ga = MockGoogleAnalyticsProvider(integration, MagicMock(), db.default_library())
 
         identifier = db.identifier()
         source = DataSource.lookup(db.session, DataSource.GUTENBERG)
@@ -201,7 +202,7 @@ def test_collect_event_without_license_pool(self, db: DatabaseTransactionFixture
             db.default_library(),
             integration,
         ).value = "faketrackingid"
-        ga = MockGoogleAnalyticsProvider(integration, db.default_library())
+        ga = MockGoogleAnalyticsProvider(integration, MagicMock(), db.default_library())
 
         now = utc_now()
         ga.collect_event(db.default_library(), None, CirculationEvent.NEW_PATRON, now)

tests/core/test_local_analytics_provider.py

@@ -1,9 +1,14 @@
+from typing import TYPE_CHECKING
+
 import pytest
 
 from core.local_analytics_provider import LocalAnalyticsProvider
 from core.model import CirculationEvent, ExternalIntegration, create, get_one
 from core.util.datetime_helpers import utc_now
-from tests.fixtures.database import DatabaseTransactionFixture
+
+if TYPE_CHECKING:
+    from tests.fixtures.database import DatabaseTransactionFixture
+    from tests.fixtures.services import MockServicesFixture
 
 
 class TestInitializeLocalAnalyticsProvider:
@@ -49,24 +54,29 @@ class LocalAnalyticsProviderFixture:
     integration: ExternalIntegration
     la: LocalAnalyticsProvider
 
-    def __init__(self, transaction: DatabaseTransactionFixture):
+    def __init__(
+        self,
+        transaction: DatabaseTransactionFixture,
+        mock_services_fixture: MockServicesFixture,
+    ):
         self.transaction = transaction
         self.integration, ignore = create(
             transaction.session,
             ExternalIntegration,
             goal=ExternalIntegration.ANALYTICS_GOAL,
             protocol="core.local_analytics_provider",
         )
+        self.services = mock_services_fixture.services
         self.la = LocalAnalyticsProvider(
-            self.integration, transaction.default_library()
+            self.integration, self.services, transaction.default_library()
         )
 
 
 @pytest.fixture()
 def local_analytics_provider_fixture(
-    db,
+    db: DatabaseTransactionFixture, mock_services_fixture: MockServicesFixture
 ) -> LocalAnalyticsProviderFixture:
-    return LocalAnalyticsProviderFixture(db)
+    return LocalAnalyticsProviderFixture(db, mock_services_fixture)
 
 
 class TestLocalAnalyticsProvider:
@@ -123,7 +133,7 @@ def test_collect_event(
 
         # It's possible to instantiate the LocalAnalyticsProvider
         # without a library.
-        la = LocalAnalyticsProvider(data.integration)
+        la = LocalAnalyticsProvider(data.integration, data.services)
 
         # In that case, it will process events for any library.
         for library in [database.default_library(), library2]:
@@ -184,7 +194,7 @@ def test_neighborhood_is_location(
         data.integration.setting(
             p.LOCATION_SOURCE
         ).value = p.LOCATION_SOURCE_NEIGHBORHOOD
-        la = p(data.integration, database.default_library())
+        la = p(data.integration, data.services, database.default_library())
 
         event, is_new = la.collect_event(
             database.default_library(),

tests/core/test_s3_analytics_provider.py

@@ -37,7 +37,7 @@ def __init__(
         self.services = services_fixture.services
         self.analytics_storage = services_fixture.storage.analytics
         self.analytics_provider = S3AnalyticsProvider(
-            self.analytics_integration, db.default_library()
+            self.analytics_integration, self.services, db.default_library()
         )
 
 
@@ -65,11 +65,13 @@ def test_exception_is_raised_when_no_analytics_bucket_configured(
         self, s3_analytics_fixture: S3AnalyticsFixture
     ):
         # The services container returns None when there is no analytics storage service configured,
-        # we simulate this by directly passing None to the analytics provider.
+        # so we override the analytics storage service with None to simulate this situation.
+        s3_analytics_fixture.services.storage.analytics.override(None)
+
         provider = S3AnalyticsProvider(
             s3_analytics_fixture.analytics_integration,
+            s3_analytics_fixture.services,
             s3_analytics_fixture.db.default_library(),
-            s3_service=None,
         )
 
         # Act, Assert