Move analytics provider into service.analytics package

pyproject.toml

@@ -120,6 +120,7 @@ module = [
     "palace.manager.service.*",
     "palace.manager.sqlalchemy.hassessioncache",
     "palace.manager.sqlalchemy.model.announcements",
+    "palace.manager.sqlalchemy.model.circulationevent",
     "palace.manager.sqlalchemy.model.classification",
     "palace.manager.sqlalchemy.model.collection",
     "palace.manager.sqlalchemy.model.datasource",

src/palace/manager/service/analytics/analytics.py

@@ -1,12 +1,12 @@
 from __future__ import annotations
 
 from datetime import datetime
-from typing import TYPE_CHECKING, Any
+from typing import TYPE_CHECKING
 
 import flask
 
-from palace.manager.api.s3_analytics_provider import S3AnalyticsProvider
-from palace.manager.core.local_analytics_provider import LocalAnalyticsProvider
+from palace.manager.service.analytics.local import LocalAnalyticsProvider
+from palace.manager.service.analytics.s3 import S3AnalyticsProvider
 from palace.manager.sqlalchemy.model.library import Library
 from palace.manager.sqlalchemy.model.licensing import LicensePool
 from palace.manager.sqlalchemy.model.patron import Patron
@@ -41,8 +41,10 @@ def collect_event(
         license_pool: LicensePool | None,
         event_type: str,
         time: datetime | None = None,
+        old_value: int | None = None,
+        new_value: int | None = None,
         patron: Patron | None = None,
-        **kwargs: Any,
+        neighborhood: str | None = None,
     ) -> None:
         if not time:
             time = utc_now()
@@ -61,9 +63,11 @@ def collect_event(
                 license_pool,
                 event_type,
                 time,
+                old_value=old_value,
+                new_value=new_value,
                 user_agent=user_agent,
                 patron=patron,
-                **kwargs,
+                neighborhood=neighborhood,
             )
 
     def is_configured(self) -> bool:

src/palace/manager/core/local_analytics_provider.py → src/palace/manager/service/analytics/local.py

@@ -1,5 +1,4 @@
 from datetime import datetime
-from typing import Any
 
 from sqlalchemy.orm.session import Session
 
@@ -17,20 +16,21 @@ def collect_event(
         license_pool: LicensePool | None,
         event_type: str,
         time: datetime,
-        old_value: Any = None,
-        new_value: Any = None,
+        old_value: int | None = None,
+        new_value: int | None = None,
         user_agent: str | None = None,
         patron: Patron | None = None,
-        **kwargs
-    ):
+        neighborhood: str | None = None,
+    ) -> None:
         _db = Session.object_session(library)
 
-        return CirculationEvent.log(
+        CirculationEvent.log(
             _db,
             license_pool,
             event_type,
             old_value,
             new_value,
             start=time,
             library=library,
+            location=neighborhood,
         )

src/palace/manager/api/s3_analytics_provider.py → src/palace/manager/service/analytics/s3.py

@@ -4,10 +4,10 @@
 import json
 import random
 import string
-from typing import TYPE_CHECKING
+from typing import TYPE_CHECKING, Any
 
 from palace.manager.core.config import CannotLoadConfiguration
-from palace.manager.core.local_analytics_provider import LocalAnalyticsProvider
+from palace.manager.service.analytics.local import LocalAnalyticsProvider
 from palace.manager.sqlalchemy.constants import MediaTypes
 from palace.manager.sqlalchemy.model.library import Library
 from palace.manager.sqlalchemy.model.licensing import LicensePool
@@ -34,7 +34,7 @@ def _create_event_object(
         neighborhood: str | None = None,
         user_agent: str | None = None,
         patron: Patron | None = None,
-    ) -> dict:
+    ) -> dict[str, Any]:
         """Create a Python dict containing required information about the event.
 
         :param library: Library associated with the event
@@ -149,8 +149,8 @@ def collect_event(
         new_value: int | None = None,
         user_agent: str | None = None,
         patron: Patron | None = None,
-        **kwargs,
-    ):
+        neighborhood: str | None = None,
+    ) -> None:
         """Log the event using the appropriate for the specific provider's mechanism.
 
         :param library: Library associated with the event
@@ -187,7 +187,7 @@ def collect_event(
             new_value,
             user_agent=user_agent,
             patron=patron,
-            **kwargs,
+            neighborhood=neighborhood,
         )
         content = json.dumps(
             event,
@@ -211,7 +211,7 @@ def _get_file_key(
         event_type: str,
         end_time: datetime.datetime,
         start_time: datetime.datetime | None = None,
-    ):
+    ) -> str:
         """The path to the analytics data file for the given library, license
         pool and date range."""
         root = library.short_name

src/palace/manager/sqlalchemy/model/circulationevent.py

@@ -1,11 +1,12 @@
 # CirculationEvent
 from __future__ import annotations
 
+import datetime
 import logging
 from typing import TYPE_CHECKING
 
 from sqlalchemy import Column, DateTime, ForeignKey, Index, Integer, String, Unicode
-from sqlalchemy.orm import Mapped, relationship
+from sqlalchemy.orm import Mapped, Session, relationship
 
 from palace.manager.sqlalchemy.model.base import Base
 from palace.manager.sqlalchemy.util import get_one_or_create
@@ -129,16 +130,16 @@ class CirculationEvent(Base):
     @classmethod
     def log(
         cls,
-        _db,
-        license_pool,
-        event_name,
-        old_value,
-        new_value,
-        start=None,
-        end=None,
-        library=None,
-        location=None,
-    ):
+        _db: Session,
+        license_pool: LicensePool | None,
+        event_name: str,
+        old_value: int | None,
+        new_value: int | None,
+        start: datetime.datetime | None = None,
+        end: datetime.datetime | None = None,
+        library: Library | None = None,
+        location: str | None = None,
+    ) -> tuple[CirculationEvent, bool]:
         """Log a CirculationEvent to the database, assuming it
         hasn't already been recorded.
         """

tests/manager/api/controller/test_analytics.py

@@ -57,7 +57,5 @@ def test_track_event(self, analytics_fixture: AnalyticsFixture):
                 license_pool=analytics_fixture.lp,
             )
             assert circulation_event is not None
-            assert (
-                circulation_event.location == None
-            )  # We no longer use the location source
+            assert circulation_event.location == "Mars Grid 4810579"
             db.session.delete(circulation_event)

tests/manager/service/analytics/test_analytics.py

@@ -4,9 +4,9 @@
 
 import pytest
 
-from palace.manager.api.s3_analytics_provider import S3AnalyticsProvider
-from palace.manager.core.local_analytics_provider import LocalAnalyticsProvider
 from palace.manager.service.analytics.analytics import Analytics
+from palace.manager.service.analytics.local import LocalAnalyticsProvider
+from palace.manager.service.analytics.s3 import S3AnalyticsProvider
 from palace.manager.sqlalchemy.model.circulationevent import CirculationEvent
 from tests.fixtures.api_controller import ControllerFixture
 from tests.fixtures.database import DatabaseTransactionFixture

tests/manager/core/test_local_analytics_provider.py → tests/manager/service/analytics/test_local.py

@@ -4,7 +4,7 @@
 
 import pytest
 
-from palace.manager.core.local_analytics_provider import LocalAnalyticsProvider
+from palace.manager.service.analytics.local import LocalAnalyticsProvider
 from palace.manager.sqlalchemy.model.circulationevent import CirculationEvent
 from palace.manager.util.datetime_helpers import utc_now
 

tests/manager/core/test_s3_analytics_provider.py → tests/manager/service/analytics/test_s3.py

@@ -7,9 +7,9 @@
 
 import pytest
 
-from palace.manager.api.s3_analytics_provider import S3AnalyticsProvider
 from palace.manager.core.classifier import Classifier
 from palace.manager.core.config import CannotLoadConfiguration
+from palace.manager.service.analytics.s3 import S3AnalyticsProvider
 from palace.manager.service.storage.s3 import S3Service
 from palace.manager.sqlalchemy.constants import MediaTypes
 from palace.manager.sqlalchemy.model.circulationevent import CirculationEvent

tests/manager/sqlalchemy/model/test_circulationevent.py

@@ -176,6 +176,7 @@ def test_log(self, db: DatabaseTransactionFixture):
             end=end,
             location=location,
         )
+        assert event.start is not None
         assert (utc_now() - event.start).total_seconds() < 2
         assert True == is_new
         assert pool == event.license_pool