Removed location_source and adjoining logic

api/app.py

@@ -18,7 +18,6 @@
 )
 from core.app_server import ErrorHandler
 from core.flask_sqlalchemy_session import flask_scoped_session
-from core.local_analytics_provider import LocalAnalyticsProvider
 from core.model import (
     LOCK_ID_APP_INIT,
     ConfigurationSetting,
@@ -68,9 +67,6 @@ def initialize_admin(_db=None):
     _db = _db or app._db
     # The secret key is used for signing cookies for admin login
     app.secret_key = ConfigurationSetting.sitewide_secret(_db, Configuration.SECRET_KEY)
-    # Create a default Local Analytics service if one does not
-    # already exist.
-    LocalAnalyticsProvider.initialize(_db)
 
 
 def initialize_circulation_manager(container: Services):

api/s3_analytics_provider.py

@@ -11,7 +11,6 @@
 from core.config import CannotLoadConfiguration
 from core.local_analytics_provider import LocalAnalyticsProvider
 from core.model import Library, LicensePool, MediaTypes
-from core.service.analytics.configuration import AnalyticsConfiguration
 
 if TYPE_CHECKING:
     from core.service.storage.s3 import S3Service
@@ -23,9 +22,8 @@ class S3AnalyticsProvider(LocalAnalyticsProvider):
     NAME = _("S3 Analytics")
     DESCRIPTION = _("Store analytics events in a S3 bucket.")
 
-    def __init__(self, s3_service: Optional[S3Service], config: AnalyticsConfiguration):
+    def __init__(self, s3_service: Optional[S3Service]):
         self.s3_service = s3_service
-        super().__init__(config)
 
     @staticmethod
     def _create_event_object(
@@ -35,7 +33,7 @@ def _create_event_object(
         time: datetime.datetime,
         old_value,
         new_value,
-        neighborhood: str,
+        neighborhood: Optional[str] = None,
     ) -> Dict:
         """Create a Python dict containing required information about the event.
 
@@ -175,12 +173,8 @@ def collect_event(
         if not library and not license_pool:
             raise ValueError("Either library or license_pool must be provided.")
 
-        neighborhood = None
-        if self.location_source == self.LOCATION_SOURCE_NEIGHBORHOOD:
-            neighborhood = kwargs.pop("neighborhood", None)
-
         event = self._create_event_object(
-            library, license_pool, event_type, time, old_value, new_value, neighborhood
+            library, license_pool, event_type, time, old_value, new_value
         )
         content = json.dumps(
             event,

core/analytics.py

@@ -4,7 +4,6 @@
 
 from api.s3_analytics_provider import S3AnalyticsProvider
 from core.local_analytics_provider import LocalAnalyticsProvider
-from core.service.analytics.configuration import AnalyticsConfiguration
 from core.util.datetime_helpers import utc_now
 from core.util.log import LoggerMixin
 
@@ -17,19 +16,14 @@ class Analytics(LoggerMixin):
 
     def __init__(
         self,
-        config: Optional[dict] = None,
+        s3_analytics_enabled: bool = False,
         s3_service: Optional[S3Service] = None,
     ) -> None:
-        self.providers = []
-        self.config = AnalyticsConfiguration.from_values(
-            **(config if config is not None else {})
-        )
-        if self.config.local_analytics_enabled:
-            self.providers.append(LocalAnalyticsProvider(self.config))
-
-        if self.config.s3_analytics_enabled:
+        self.providers = [LocalAnalyticsProvider()]
+
+        if s3_analytics_enabled:
             if s3_service is not None:
-                self.providers.append(S3AnalyticsProvider(s3_service, self.config))
+                self.providers.append(S3AnalyticsProvider(s3_service))
             else:
                 self.log.info(
                     "S3 analytics is not configured: No analytics bucket was specified."
@@ -43,4 +37,4 @@ def collect_event(self, library, license_pool, event_type, time=None, **kwargs):
             provider.collect_event(library, license_pool, event_type, time, **kwargs)
 
     def is_configured(self):
-        return self.config.local_analytics_enabled or self.config.s3_analytics_enabled
+        return len(self.providers) > 0

core/local_analytics_provider.py

@@ -1,7 +1,7 @@
 from flask_babel import lazy_gettext as _
 from sqlalchemy.orm.session import Session
 
-from core.model import CirculationEvent, ExternalIntegration, create, get_one
+from core.model import CirculationEvent
 from core.util.log import LoggerMixin
 
 
@@ -13,19 +13,6 @@ class LocalAnalyticsProvider(LoggerMixin):
     # A given site can only have one analytics provider.
     CARDINALITY = 1
 
-    # Where to get the 'location' of an analytics event.
-    LOCATION_SOURCE = "location_source"
-
-    # The 'location' of an analytics event is the 'neighborhood' of
-    # the request's authenticated patron.
-    LOCATION_SOURCE_NEIGHBORHOOD = "neighborhood"
-
-    # Analytics events have no 'location'.
-    LOCATION_SOURCE_DISABLED = None
-
-    def __init__(self, config):
-        self.location_source = config.location_source
-
     def collect_event(
         self,
         library,
@@ -43,10 +30,6 @@ def collect_event(
         else:
             _db = Session.object_session(license_pool)
 
-        neighborhood = None
-        if self.location_source == self.LOCATION_SOURCE_NEIGHBORHOOD:
-            neighborhood = kwargs.pop("neighborhood", None)
-
         return CirculationEvent.log(
             _db,
             license_pool,
@@ -55,34 +38,8 @@ def collect_event(
             new_value,
             start=time,
             library=library,
-            location=neighborhood,
         )
 
-    @classmethod
-    def initialize(cls, _db):
-        """Find or create a local analytics service."""
-
-        # If a local analytics service already exists, return it.
-        local_analytics = get_one(
-            _db,
-            ExternalIntegration,
-            protocol=cls.__module__,
-            goal=ExternalIntegration.ANALYTICS_GOAL,
-        )
-
-        # If a local analytics service already exists, don't create a
-        # default one. Otherwise, create it with default name of
-        # "Local Analytics".
-        if not local_analytics:
-            local_analytics, ignore = create(
-                _db,
-                ExternalIntegration,
-                protocol=cls.__module__,
-                goal=ExternalIntegration.ANALYTICS_GOAL,
-                name=str(cls.NAME),
-            )
-        return local_analytics
-
 
 # The Analytics class looks for the name "Provider".
 Provider = LocalAnalyticsProvider

core/service/analytics/configuration.py

@@ -1,9 +1,5 @@
-from typing import Optional
-
 from core.service.configuration import ServiceConfiguration
 
 
 class AnalyticsConfiguration(ServiceConfiguration):
-    local_analytics_enabled: bool = False
     s3_analytics_enabled: bool = False
-    location_source: Optional[str] = None

core/service/analytics/container.py

@@ -8,8 +8,8 @@ class AnalyticsContainer(DeclarativeContainer):
     config = providers.Configuration()
     storage = providers.DependenciesContainer()
 
-    analytics = providers.Singleton(
+    analytics: providers.Provider[Analytics] = providers.Singleton(
         Analytics,
-        config=config,
-        storage_client=storage.analytics,
+        s3_analytics_enabled=config.s3_analytics_enabled,
+        s3_service=storage.analytics,
     )

tests/api/feed/test_library_annotator.py

@@ -40,7 +40,6 @@
     Work,
 )
 from core.opds_import import OPDSXMLParser
-from core.service.analytics.configuration import AnalyticsConfiguration
 from core.service.container import container_instance
 from core.util.datetime_helpers import utc_now
 from core.util.flask_util import OPDSFeedResponse
@@ -580,10 +579,8 @@ def test_annotate_work_entry(self, annotator_fixture: LibraryAnnotatorFixture):
 
         with patch.object(
             container_instance().analytics.analytics(),
-            "config",
-            AnalyticsConfiguration(
-                local_analytics_enabled=False, s3_analytics_enabled=False
-            ),
+            "is_configured",
+            lambda: False,
         ):
             annotator.annotate_work_entry(work_entry)
         assert work_entry.computed is not None
@@ -612,15 +609,10 @@ def test_annotate_work_entry(self, annotator_fixture: LibraryAnnotatorFixture):
 
         # If analytics are configured, a link is added to
         # create an 'open_book' analytics event for this title.
-        with patch.object(
-            container_instance().analytics.analytics(),
-            "config",
-            AnalyticsConfiguration(local_analytics_enabled=True),
-        ):
-            work_entry = WorkEntry(
-                work=work, license_pool=None, edition=edition, identifier=identifier
-            )
-            annotator.annotate_work_entry(work_entry)
+        work_entry = WorkEntry(
+            work=work, license_pool=None, edition=edition, identifier=identifier
+        )
+        annotator.annotate_work_entry(work_entry)
 
         assert work_entry.computed is not None
         [analytics_link] = [

tests/api/test_controller_analytics.py

@@ -2,8 +2,6 @@
 import pytest
 
 from api.problem_details import INVALID_ANALYTICS_EVENT_TYPE
-from core.analytics import Analytics
-from core.local_analytics_provider import LocalAnalyticsProvider
 from core.model import CirculationEvent, get_one
 from tests.fixtures.api_controller import CirculationControllerFixture
 from tests.fixtures.database import DatabaseTransactionFixture
@@ -24,14 +22,6 @@ def analytics_fixture(db: DatabaseTransactionFixture):
 class TestAnalyticsController:
     def test_track_event(self, analytics_fixture: AnalyticsFixture):
         db = analytics_fixture.db
-        # The Analytics singleton will have already been instantiated,
-        # so here we simulate a reload of its configuration with `refresh`.
-        analytics_fixture.manager.analytics = Analytics(
-            config=dict(
-                local_analytics_enabled=True,
-                location_source=LocalAnalyticsProvider.LOCATION_SOURCE_NEIGHBORHOOD,
-            )
-        )
 
         with analytics_fixture.request_context_with_library("/"):
             response = analytics_fixture.manager.analytics_controller.track_event(
@@ -46,25 +36,6 @@ def test_track_event(self, analytics_fixture: AnalyticsFixture):
         # associated neighborhood, the CirculationEvent is created
         # with no location.
         patron = db.patron()
-        for request_patron in (None, patron):
-            with analytics_fixture.request_context_with_library("/"):
-                flask.request.patron = request_patron  # type: ignore
-                response = analytics_fixture.manager.analytics_controller.track_event(
-                    analytics_fixture.identifier.type,
-                    analytics_fixture.identifier.identifier,
-                    "open_book",
-                )
-                assert 200 == response.status_code
-
-                circulation_event = get_one(
-                    db.session,
-                    CirculationEvent,
-                    type="open_book",
-                    license_pool=analytics_fixture.lp,
-                )
-                assert None is not circulation_event
-                assert None == circulation_event.location
-                db.session.delete(circulation_event)
 
         # If the patron has an associated neighborhood, and the
         # analytics controller is set up to use patron neighborhood as

tests/core/test_analytics.py

@@ -1,7 +1,8 @@
+from unittest.mock import MagicMock
+
 from api.s3_analytics_provider import S3AnalyticsProvider
 from core.analytics import Analytics
 from core.local_analytics_provider import LocalAnalyticsProvider
-from core.service.storage.container import Storage
 
 # We can't import mock_analytics_provider from within a test,
 # and we can't tell Analytics to do so either. We need to tell
@@ -13,40 +14,25 @@
 
 class TestAnalytics:
     def test_is_configured(self):
-        analytics = Analytics(config={})
-        print(analytics.config)
-        assert analytics.is_configured() == False
-
-        analytics = Analytics(config=dict(local_analytics_enabled=True))
+        analytics = Analytics()
         assert analytics.is_configured() == True
 
-        analytics = Analytics(config=dict(s3_analytics_enabled=True))
+        analytics = Analytics(s3_analytics_enabled=True)
         assert analytics.is_configured() == True
 
+        # If somehow we don't have providers, we don't have analytics
+        analytics.providers = []
+        assert analytics.is_configured() == False
+
     def test_init_analytics(self):
-        analytics = Analytics(
-            config=dict(local_analytics_enabled=True, location_source=None),
-            storage=None,
-        )
+        analytics = Analytics()
 
         assert len(analytics.providers) == 1
         assert type(analytics.providers[0]) == LocalAnalyticsProvider
 
         analytics = Analytics(
-            config=dict(
-                local_analytics_enabled=True,
-                s3_analytics_enabled=True,
-                location_source=None,
-            ),
-            storage=Storage(
-                config=dict(
-                    aws_access_key="key",
-                    aws_secret_key="secret",
-                    region="us-west-2",
-                    analytics_bucket="bucket",
-                    url_template="http://template{key}",
-                )
-            ),
+            s3_analytics_enabled=True,
+            s3_service=MagicMock(),
         )
 
         assert len(analytics.providers) == 2