diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 2967b928..270597ab 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -14,10 +14,15 @@ Change Log Unreleased ~~~~~~~~~~ + +[5.3.0] +~~~~~~~ + * Use proper externalId types XAPI and CALIPER instead of LTI * Make user identifier in xAPI events configurable * Switch from ``edx-sphinx-theme`` to ``sphinx-book-theme`` since the former is deprecated +* Make id of xAPI statements idempotent [5.2.2] ~~~~~~~ diff --git a/event_routing_backends/__init__.py b/event_routing_backends/__init__.py index d2816803..dec9ba74 100644 --- a/event_routing_backends/__init__.py +++ b/event_routing_backends/__init__.py @@ -2,6 +2,6 @@ Various backends for receiving edX LMS events.. """ -__version__ = '5.2.2' +__version__ = '5.3.0' default_app_config = 'event_routing_backends.apps.EventRoutingBackendsConfig' # pylint: disable=invalid-name diff --git a/event_routing_backends/helpers.py b/event_routing_backends/helpers.py index 8314e25a..140beb51 100644 --- a/event_routing_backends/helpers.py +++ b/event_routing_backends/helpers.py @@ -1,10 +1,10 @@ """ Helper utilities for event transformers. """ +import datetime import logging -from datetime import timedelta +import uuid from urllib.parse import parse_qs, urlparse -from uuid import uuid4 # Imported from edx-platform # pylint: disable=import-error @@ -23,6 +23,25 @@ UTC_DATETIME_FORMAT = '%Y-%m-%dT%H:%M:%S.%f' +def get_uuid5(namespace_key, name): + """ + Create a UUID5 string based on custom namesapce and name. + + Arguments: + namespace_key (str): key to be used to create a custom namespace + name (str): string for which we need to creat a UUID + + Returns: + str + + """ + # We are not pulling base uuid from settings to avoid + # data discrepancies incase setting is changed inadvertently + base_uuid = uuid.UUID('6ba7b810-9dad-11d1-80b4-00c04fd430c8') + base_namespace = uuid.uuid5(base_uuid, namespace_key) + return uuid.uuid5(base_namespace, name) + + def get_anonymous_user_id(username_or_id, external_type): """ Generate anonymous user id. @@ -42,7 +61,7 @@ def get_anonymous_user_id(username_or_id, external_type): logger.info('User with username "%s" does not exist. ' 'Cannot generate anonymous ID', username_or_id) - anonymous_id = str(uuid4()) + anonymous_id = str(uuid.uuid4()) else: type_name = getattr(ExternalIdType, external_type) external_id, _ = ExternalId.add_new_user_id(user, type_name) @@ -132,7 +151,7 @@ def convert_seconds_to_iso(seconds): """ if seconds is None: return None - return duration_isoformat(timedelta( + return duration_isoformat(datetime.timedelta( seconds=seconds )) diff --git a/event_routing_backends/processors/tests/transformers_test_mixin.py b/event_routing_backends/processors/tests/transformers_test_mixin.py index 456e47ca..eeb0320a 100644 --- a/event_routing_backends/processors/tests/transformers_test_mixin.py +++ b/event_routing_backends/processors/tests/transformers_test_mixin.py @@ -89,10 +89,11 @@ def compare_events(self, transformed_event, expected_event): Every transformer's test case will implement its own logic to test events transformation """ - @patch('event_routing_backends.helpers.uuid4') + @patch('event_routing_backends.helpers.uuid') @ddt.data(*EVENT_FIXTURE_FILENAMES) - def test_event_transformer(self, event_filename, mocked_uuid4): - mocked_uuid4.return_value = '32e08e30-f8ae-4ce2-94a8-c2bfe38a70cb' + def test_event_transformer(self, event_filename, mocked_uuid): + mocked_uuid.uuid4.return_value = '32e08e30-f8ae-4ce2-94a8-c2bfe38a70cb' + mocked_uuid.uuid5.return_value = '32e08e30-f8ae-4ce2-94a8-c2bfe38a70cb' # if an event's expected fixture doesn't exist, the test shouldn't fail. # evaluate transformation of only supported event fixtures. diff --git a/event_routing_backends/processors/xapi/transformer.py b/event_routing_backends/processors/xapi/transformer.py index 1fdd4c7f..482fab15 100644 --- a/event_routing_backends/processors/xapi/transformer.py +++ b/event_routing_backends/processors/xapi/transformer.py @@ -2,7 +2,6 @@ xAPI Transformer Class """ import hashlib -import uuid from django.conf import settings from tincan import ( @@ -18,7 +17,7 @@ Verb, ) -from event_routing_backends.helpers import get_anonymous_user_id, get_course_from_id, get_user_email +from event_routing_backends.helpers import get_anonymous_user_id, get_course_from_id, get_user_email, get_uuid5 from event_routing_backends.processors.mixins.base_transformer import BaseTransformerMixin from event_routing_backends.processors.xapi import constants @@ -51,12 +50,18 @@ def base_transform(self): """ Transform the fields that are common for all events. """ + actor = self.get_actor() + event_timestamp = self.get_timestamp() self.transformed_event = { - 'actor': self.get_actor(), + 'actor': actor, 'context': self.get_context(), - 'timestamp': self.get_timestamp(), - 'id': uuid.uuid4() + 'timestamp': event_timestamp, } + # Warning! changing anything in these 2 lines or changing the "base_uuid" can invalidate + # billions of rows in the database. Please have a community discussion first before introducing + # any change in generation of UUID. + uuid_str = f'{actor.to_json()}-{event_timestamp}' + self.transformed_event['id'] = get_uuid5(self.verb.to_json(), uuid_str) # pylint: disable=no-member def get_actor(self): """ @@ -147,7 +152,8 @@ class XApiVerbTransformerMixin: """ verb_map = None - def get_verb(self): + @property + def verb(self): """ Get verb for xAPI transformed event. diff --git a/event_routing_backends/tests/test_helpers.py b/event_routing_backends/tests/test_helpers.py index 1e16ff12..5008c3cf 100644 --- a/event_routing_backends/tests/test_helpers.py +++ b/event_routing_backends/tests/test_helpers.py @@ -5,7 +5,12 @@ from django.test import TestCase -from event_routing_backends.helpers import get_anonymous_user_id, get_block_id_from_event_referrer, get_user_email +from event_routing_backends.helpers import ( + get_anonymous_user_id, + get_block_id_from_event_referrer, + get_user_email, + get_uuid5, +) from event_routing_backends.tests.factories import UserFactory @@ -44,3 +49,27 @@ def test_get_anonymous_user_id_with_error(self, mocked_external_id): get_anonymous_user_id('edx', 'XAPI') self.assertIsNotNone(get_anonymous_user_id('12345678', 'XAPI')) + + def test_get_uuid5(self): + actor = '''{ + "objectType": "Agent", + "mbox": "mailto:edx@example.com" + }''' + verb = '''{ + "id": "http://id.tincanapi.com/verb/unregistered", + "display": { + "en": "unregistered" + }''' + timestamp = '2023-05-09T06:36:11.256Z' + name = f'{actor}-{timestamp}' + uuid_1 = get_uuid5(verb, name) + uuid_2 = get_uuid5(verb, name) + self.assertEqual(uuid_1, uuid_2) + + another_actor = '''{ + "objectType": "Agent", + "mbox": "mailto:test@example.com" + }''' + name = f'{another_actor}-{timestamp}' + uuid_3 = get_uuid5(verb, name) + self.assertNotEqual(uuid_1, uuid_3)