diff --git a/pyproject.toml b/pyproject.toml index 3b6fa01daa..2bccae4748 100644 --- a/pyproject.toml +++ b/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", diff --git a/src/palace/manager/service/analytics/analytics.py b/src/palace/manager/service/analytics/analytics.py index 71e6df38d7..007bcce834 100644 --- a/src/palace/manager/service/analytics/analytics.py +++ b/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: diff --git a/src/palace/manager/core/local_analytics_provider.py b/src/palace/manager/service/analytics/local.py similarity index 81% rename from src/palace/manager/core/local_analytics_provider.py rename to src/palace/manager/service/analytics/local.py index c2cfc99ce8..b0780ad743 100644 --- a/src/palace/manager/core/local_analytics_provider.py +++ b/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,15 +16,15 @@ 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, @@ -33,4 +32,5 @@ def collect_event( new_value, start=time, library=library, + location=neighborhood, ) diff --git a/src/palace/manager/api/s3_analytics_provider.py b/src/palace/manager/service/analytics/s3.py similarity index 97% rename from src/palace/manager/api/s3_analytics_provider.py rename to src/palace/manager/service/analytics/s3.py index b1507d32ad..3f391ea67b 100644 --- a/src/palace/manager/api/s3_analytics_provider.py +++ b/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 diff --git a/src/palace/manager/sqlalchemy/model/circulationevent.py b/src/palace/manager/sqlalchemy/model/circulationevent.py index 114da50e8c..6284b7535b 100644 --- a/src/palace/manager/sqlalchemy/model/circulationevent.py +++ b/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. """ diff --git a/tests/manager/api/controller/test_analytics.py b/tests/manager/api/controller/test_analytics.py index 673c745bfd..f1cd2b966c 100644 --- a/tests/manager/api/controller/test_analytics.py +++ b/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) diff --git a/tests/manager/service/analytics/test_analytics.py b/tests/manager/service/analytics/test_analytics.py index e6701221d1..5853f4581b 100644 --- a/tests/manager/service/analytics/test_analytics.py +++ b/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 diff --git a/tests/manager/core/test_local_analytics_provider.py b/tests/manager/service/analytics/test_local.py similarity index 96% rename from tests/manager/core/test_local_analytics_provider.py rename to tests/manager/service/analytics/test_local.py index 2296190f23..1b08d4a235 100644 --- a/tests/manager/core/test_local_analytics_provider.py +++ b/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 diff --git a/tests/manager/core/test_s3_analytics_provider.py b/tests/manager/service/analytics/test_s3.py similarity index 99% rename from tests/manager/core/test_s3_analytics_provider.py rename to tests/manager/service/analytics/test_s3.py index 58c4c104df..e34045d789 100644 --- a/tests/manager/core/test_s3_analytics_provider.py +++ b/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 diff --git a/tests/manager/sqlalchemy/model/test_circulationevent.py b/tests/manager/sqlalchemy/model/test_circulationevent.py index b14961ac2d..a3da9b9645 100644 --- a/tests/manager/sqlalchemy/model/test_circulationevent.py +++ b/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