Skip to content

Commit

Permalink
feat: make id of xAPI statements idempotent (#290)
Browse files Browse the repository at this point in the history
make id of xAPI statements idempotent
  • Loading branch information
ziafazal authored May 12, 2023
1 parent 7ec1424 commit 783033f
Show file tree
Hide file tree
Showing 6 changed files with 75 additions and 15 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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]
~~~~~~~
Expand Down
2 changes: 1 addition & 1 deletion event_routing_backends/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
27 changes: 23 additions & 4 deletions event_routing_backends/helpers.py
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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.
Expand All @@ -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)
Expand Down Expand Up @@ -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
))

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
18 changes: 12 additions & 6 deletions event_routing_backends/processors/xapi/transformer.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
xAPI Transformer Class
"""
import hashlib
import uuid

from django.conf import settings
from tincan import (
Expand All @@ -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

Expand Down Expand Up @@ -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):
"""
Expand Down Expand Up @@ -147,7 +152,8 @@ class XApiVerbTransformerMixin:
"""
verb_map = None

def get_verb(self):
@property
def verb(self):
"""
Get verb for xAPI transformed event.
Expand Down
31 changes: 30 additions & 1 deletion event_routing_backends/tests/test_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down Expand Up @@ -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)

0 comments on commit 783033f

Please sign in to comment.