diff --git a/lms/events/event.py b/lms/events/event.py index 435c2b47b3..78128e3008 100644 --- a/lms/events/event.py +++ b/lms/events/event.py @@ -68,6 +68,16 @@ def from_request( ): assignment_id = assignment.id + if type_ in {EventType.Type.CONFIGURED_LAUNCH, EventType.Type.DEEP_LINKING}: + # Store the raw LTI parameters for launches + data = {} if data is None else data + if request.lti_jwt: + # For LTI1.3 include the decoded JWT as a dict + data["lti_params"] = request.lti_jwt + else: + # For LTI1.1 include the request form parameters + data["lti_params"] = request.lti_params.serialize() + return cls( request=request, type=type_, diff --git a/lms/tasks/event.py b/lms/tasks/event.py index abc0f575ae..4cd678353e 100644 --- a/lms/tasks/event.py +++ b/lms/tasks/event.py @@ -1,6 +1,18 @@ +import logging +from datetime import UTC, datetime, timedelta + +from sqlalchemy import select, update + from lms.events.event import BaseEvent +from lms.models import Event, EventData from lms.tasks.celery import app +LOG = logging.getLogger(__name__) + + +PURGE_LAUNCH_DATA_BATCH_SIZE = 1000 +"How many rows to remove per call to purge_launch_data" + @app.task def insert_event(event: dict) -> None: @@ -11,3 +23,28 @@ def insert_event(event: dict) -> None: request.find_service(EventService).insert_event( BaseEvent(request=request, **event) ) + + +@app.task +def purge_launch_data(*, max_age_days=30) -> None: + with app.request_context() as request: + with request.tm: + events_with_old_lti_params = ( + select(Event.id) + .join(EventData) + .where( + # Find data that's is at least max_age_days old + Event.timestamp <= datetime.now(UTC) - timedelta(days=max_age_days), + # Limit the search for only twice as old as we'd expect, limiting the data set significally + Event.timestamp + >= datetime.now(UTC) - timedelta(days=max_age_days * 2), + EventData.data["lti_params"].is_not(None), + ) + .limit(PURGE_LAUNCH_DATA_BATCH_SIZE) + ) + results = request.db.execute( + update(EventData) + .where(EventData.event_id.in_(events_with_old_lti_params)) + .values(data=EventData.data - "lti_params") + ) + LOG.info("Removed lti_params from events for %d rows", results.rowcount) diff --git a/tests/factories/__init__.py b/tests/factories/__init__.py index 486e1eabb3..0dd8fe6fd2 100644 --- a/tests/factories/__init__.py +++ b/tests/factories/__init__.py @@ -22,7 +22,7 @@ USER_ID, ) from tests.factories.dashboard_admin import DashboardAdmin -from tests.factories.event import Event +from tests.factories.event import Event, EventData from tests.factories.file import File from tests.factories.grading_info import GradingInfo from tests.factories.grading_sync import GradingSync, GradingSyncGrade diff --git a/tests/factories/event.py b/tests/factories/event.py index d8745acc14..f34560fc6a 100644 --- a/tests/factories/event.py +++ b/tests/factories/event.py @@ -1,6 +1,13 @@ -from factory import make_factory +from factory import SubFactory, make_factory from factory.alchemy import SQLAlchemyModelFactory from lms import models Event = make_factory(models.Event, FACTORY_CLASS=SQLAlchemyModelFactory) + + +EventData = make_factory( + models.EventData, + FACTORY_CLASS=SQLAlchemyModelFactory, + event=SubFactory(Event), +) diff --git a/tests/unit/conftest.py b/tests/unit/conftest.py index 8b1d5c4e11..07f0ea7991 100644 --- a/tests/unit/conftest.py +++ b/tests/unit/conftest.py @@ -147,6 +147,27 @@ def pyramid_request(db_session, application_instance, lti_v11_params): return pyramid_request +@pytest.fixture +def lti_v13_pyramid_request( + pyramid_request, lti_v13_application_instance, lti_v11_params, lti_v13_params +): + pyramid_request.lti_user = factories.LTIUser( + application_instance_id=lti_v13_application_instance.id, + application_instance=lti_v13_application_instance, + user_id=lti_v11_params["user_id"], + roles=lti_v11_params["roles"], + ) + pyramid_request.user = factories.User( + application_instance_id=lti_v13_application_instance.id, + user_id=lti_v11_params["user_id"], + ) + + pyramid_request.lti_jwt = lti_v13_params + pyramid_request.lti_params = LTIParams.from_request(pyramid_request) + + return pyramid_request + + @pytest.fixture def product(pyramid_request): return pyramid_request.product diff --git a/tests/unit/lms/events/event_test.py b/tests/unit/lms/events/event_test.py index 5f5eaf81a7..c738e97a9c 100644 --- a/tests/unit/lms/events/event_test.py +++ b/tests/unit/lms/events/event_test.py @@ -5,6 +5,7 @@ from lms.events import AuditTrailEvent, BaseEvent, LTIEvent, ModelChange from lms.events.event import _serialize_change +from lms.models import EventType from tests import factories @@ -32,6 +33,12 @@ def test_serialize(self, pyramid_request): } +@pytest.mark.usefixtures( + "lti_role_service", + "application_instance_service", + "assignment_service", + "course_service", +) class TestLTIEvent: def test_lti_event_no_lti_user(self, pyramid_request): pyramid_request.lti_user = None @@ -70,24 +77,36 @@ def test_lti_event( assert event.assignment_id == assignment_service.get_assignment.return_value.id assert event.data == sentinel.data - @pytest.mark.usefixtures( - "lti_role_service", "application_instance_service", "assignment_service" - ) def test_lti_event_when_no_course(self, pyramid_request, course_service): course_service.get_by_context_id.return_value = None event = LTIEvent.from_request(request=pyramid_request, type_=sentinel.type) assert not event.course_id - @pytest.mark.usefixtures( - "lti_role_service", "application_instance_service", "course_service" - ) def test_lti_event_when_no_assignment(self, pyramid_request, assignment_service): assignment_service.get_assignment.return_value = None event = LTIEvent.from_request(request=pyramid_request, type_=sentinel.type) assert not event.assignment_id + @pytest.mark.parametrize( + "type_", [EventType.Type.CONFIGURED_LAUNCH, EventType.Type.DEEP_LINKING] + ) + def test_lti_event_includes_launch_data_for_lti_v13( + self, lti_v13_pyramid_request, type_ + ): + event = LTIEvent.from_request(request=lti_v13_pyramid_request, type_=type_) + + assert event.data["lti_params"] == lti_v13_pyramid_request.lti_jwt + + @pytest.mark.parametrize( + "type_", [EventType.Type.CONFIGURED_LAUNCH, EventType.Type.DEEP_LINKING] + ) + def test_lti_event_includes_launch_data_for_lti_v11(self, pyramid_request, type_): + event = LTIEvent.from_request(request=pyramid_request, type_=type_) + + assert event.data["lti_params"] == pyramid_request.lti_params.serialize() + @pytest.fixture def pyramid_request(self, pyramid_request): pyramid_request.lti_params.update( diff --git a/tests/unit/lms/tasks/event_test.py b/tests/unit/lms/tasks/event_test.py index 39c8e2896a..999bd6c4d6 100644 --- a/tests/unit/lms/tasks/event_test.py +++ b/tests/unit/lms/tasks/event_test.py @@ -1,8 +1,11 @@ from contextlib import contextmanager +from datetime import datetime import pytest +from freezegun import freeze_time -from lms.tasks.event import insert_event +from lms.tasks.event import insert_event, purge_launch_data +from tests import factories def test_insert_event(event_service, BaseEvent, pyramid_request): @@ -12,6 +15,31 @@ def test_insert_event(event_service, BaseEvent, pyramid_request): event_service.insert_event.assert_called_once_with(BaseEvent.return_value) +@freeze_time("2024-1-25") +def test_purge_launch_data(): + recent_data = factories.EventData( + event=factories.Event(timestamp=datetime(2024, 1, 20)), + data={"lti_params": {"some": "data"}}, + ) + old_data = factories.EventData( + event=factories.Event(timestamp=datetime(2024, 1, 10)), + data={"lti_params": {"some": "data"}}, + ) + old_data_no_launch = factories.EventData( + event=factories.Event(timestamp=datetime(2024, 1, 10)), + data={"some_other_data": {"some": "data"}}, + ) + + purge_launch_data(max_age_days=10) + + # Kept data for recent launches + assert "lti_params" in recent_data.data + # Removed for events in the time window + assert "lti_params" not in old_data.data + # Other keys are not removed + assert "some_other_data" in old_data_no_launch.data + + @pytest.fixture(autouse=True) def BaseEvent(patch): return patch("lms.tasks.event.BaseEvent")