Skip to content

Commit

Permalink
Multi question xapi (#352)
Browse files Browse the repository at this point in the history
feat!: Allow xAPI processor to emit multiple statements per tracking log

Updates the event processor change to allow for the possibility that
some event transformers may generate multiple events from a single
event, and these events need to be carried down the processor chain.

This is necessary to handle multi-question problem check events, 
which previously only transformed the first question into xAPI and
dropped the rest. 

* refactor: applies functional programming principles to event transformation

This reduces the side-effects of event transformation, which will allow
us to re-use base event transformation methods when generating multiple
events from a single source event.

* Removes the BaseTransformerMixin.transformed_event instance variable
  in favor of passing an event through to base_transform() to be
  modified and returned.
* Adds BaseTransformerMixin.get_object() so that caliper events don't
  need to reference self.transformed_event when updating the object data.
* Adds BaseTransformerMixin.get_extensions() so that caliper events can
  don't have to hack in their transformerVersion during
  BaseTransformerMixin.transform()

* feat: multi-question problem_check events produce multiple xAPI events

Single-question problem_check events still only produce one xAPI event.

Changes to the top-level multi-question problem_check event data:

* object.type changed from Activity to GroupActivity
* object.id shows the base problem usage_key
* object.definition.interaction_type is "other"

New events emitted for each child problem are identical the top-level event,
except for:

* object.type is Activity
* object.id shows the base problem usage_key including the child usage string
* object.definition.interaction_type is determined by the child problem response_type
* result.score is omitted -- only relevant to the parent problem
* result.response is provided, pulled from the child question submission
* result.success is provided, pulled from the child question submission

Related fixes to all problem_check events:

* object.definition.name now shows the problem display_name
* object.id now uses shows the problem usage_key
* result.score max and raw are now always provided if present in the
  source event (bug fix)
* stops mocking uuid5 during tests
  uuid5 generates the same UUID when provided with the same namespace + name,
  and so we can rely on this remaining the same in the expected fixture files.
* updates test data to use the actual uuid5s generated for the given input events.

* fix: child events must use unique event IDs

* pulls event ID generation into get_event_id(),
  taking care not to modify how parent event IDs are generated.
* overrides get_event_id() for child events by using the child_id as
  part of the UUID namespace_key.
* updates tests to check that child events and their parent event use
  different event IDs, and the affected problem_check multiple-question
  test data

* fix: rebase, add newly added event types

---------

Co-authored-by: Jillian Vogel <[email protected]>
  • Loading branch information
bmtcril and pomegranited authored Sep 26, 2023
1 parent 9ccfee1 commit 99ede4e
Show file tree
Hide file tree
Showing 99 changed files with 1,861 additions and 212 deletions.
4 changes: 4 additions & 0 deletions .coveragerc
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,7 @@ omit =
*admin.py
*static*
*templates*
[report]
exclude_lines =
pragma: no cover
raise NotImplementedError
9 changes: 5 additions & 4 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,15 @@ help: ## display this help message
@awk -F ':.*?## ' '/^[a-zA-Z]/ && NF==2 {printf "\033[36m %-25s\033[0m %s\n", $$1, $$2}' $(MAKEFILE_LIST) | sort

clean: ## remove generated byte code, coverage reports, and build artifacts
find . -name '__pycache__' -exec rm -rf {} +
find . -name '*.pyc' -exec rm -f {} +
find . -name '*.pyo' -exec rm -f {} +
find . -name '*~' -exec rm -f {} +
coverage erase
rm -fr build/
rm -fr dist/
rm -fr *.egg-info
rm -rf test_output/*.json
find . -name '__pycache__' -exec rm -rf {} +
find . -name '*.pyc' -exec rm -f {} +
find . -name '*.pyo' -exec rm -f {} +
find . -name '*~' -exec rm -f {} +

coverage: clean ## generate and view HTML coverage report
pytest --cov-report html
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,4 +2,4 @@
Various backends for receiving edX LMS events..
"""

__version__ = '6.2.0'
__version__ = '7.0.0'
28 changes: 16 additions & 12 deletions event_routing_backends/backends/events_router.py
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ def prepare_to_send(self, events):
'Processing edx event "{}" for router with backend {}'.format(event_name, self.backend_name)
)

processed_event = self.process_event(event)
processed_events = self.process_event(event)
except (EventEmissionExit, ValueError):
logger.error(
'Could not process edx event "%s" for backend %s\'s router',
Expand All @@ -90,10 +90,10 @@ def prepare_to_send(self, events):
continue

logger.debug(
'Successfully processed edx event "%s" for router with backend %s. Processed event: %s',
'Successfully processed edx event "%s" for router with backend %s. Processed events: %s',
event_name,
self.backend_name,
processed_event
processed_events
)

for router in routers:
Expand All @@ -107,11 +107,13 @@ def prepare_to_send(self, events):
)
else:
host = self.configure_host(host, router)
updated_event = self.overwrite_event_data(processed_event, host, event_name)
is_business_critical = event_name in business_critical_events
if router_pk not in route_events:
route_events[router_pk] = [(event_name, updated_event, host, is_business_critical),]
else:

if processed_events and router_pk not in route_events:
route_events[router_pk] = []

for processed_event in processed_events:
updated_event = self.overwrite_event_data(processed_event, host, event_name)
is_business_critical = event_name in business_critical_events
route_events[router_pk].append((event_name, updated_event, host, is_business_critical))

return route_events
Expand Down Expand Up @@ -176,17 +178,19 @@ def process_event(self, event):
"""
Process the event through this router's processors.
This single event may produce multiple processed events, and so we return a list of events here.
Arguments:
event (dict): Event to be processed
Returns
dict
list of ANY
"""
event = event.copy()
events = [event.copy()]
for processor in self.processors:
event = processor(event)
events = processor(events)

return event
return events

def overwrite_event_data(self, event, host, event_name):
"""
Expand Down
19 changes: 13 additions & 6 deletions event_routing_backends/backends/tests/test_events_router.py
Original file line number Diff line number Diff line change
Expand Up @@ -153,9 +153,9 @@ def setUp(self):
@patch('event_routing_backends.models.RouterConfiguration.get_enabled_routers')
def test_with_processor_exception(self, mocked_get_enabled_routers, mocked_logger, mocked_post):
processors = [
MagicMock(return_value=self.transformed_event),
MagicMock(side_effect=EventEmissionExit, return_value=self.transformed_event),
MagicMock(return_value=self.transformed_event),
MagicMock(return_value=[self.transformed_event]),
MagicMock(side_effect=EventEmissionExit, return_value=[self.transformed_event]),
MagicMock(return_value=[self.transformed_event]),
]
processors[1].side_effect = EventEmissionExit

Expand All @@ -164,8 +164,8 @@ def test_with_processor_exception(self, mocked_get_enabled_routers, mocked_logge
router = EventsRouter(processors=processors, backend_name='test')
router.send(self.transformed_event)

processors[0].assert_called_once_with(self.transformed_event)
processors[1].assert_called_once_with(self.transformed_event)
processors[0].assert_called_once_with([self.transformed_event])
processors[1].assert_called_once_with([self.transformed_event])
processors[2].assert_not_called()

mocked_post.assert_not_called()
Expand Down Expand Up @@ -642,7 +642,14 @@ def test_duplicate_xapi_event_id(self, mocked_logger):
mocked_logger.info.mock_calls
)

def test_unsuccessful_routing_of_event_http(self):
@patch('event_routing_backends.utils.http_client.requests.post')
def test_unsuccessful_routing_of_event_http(self, mocked_post):
mock_response = MagicMock()
mock_response.status_code = 500
mock_response.request.method = "POST"
mock_response.text = "Fake Server Error"
mocked_post.return_value = mock_response

host_configurations = {
'url': 'http://test4.com',
'auth_scheme': 'bearer',
Expand Down
23 changes: 13 additions & 10 deletions event_routing_backends/processors/caliper/envelope_processor.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,19 +19,22 @@ def __init__(self, sensor_id):
"""
self.sensor_id = sensor_id

def __call__(self, event):
def __call__(self, events):
"""
Envelope the caliper transformed event.
Envelope the caliper transformed events.
Arguments:
event (dict): IMS Caliper compliant event dict
events (list of dicts): List of IMS Caliper compliant event dicts
Returns:
dict
list of dicts
"""
return {
'sensor': self.sensor_id,
'sendTime': convert_datetime_to_iso(datetime.now(UTC)),
'data': [event],
'dataVersion': CALIPER_EVENT_CONTEXT
}
enveloped_events = []
for event in events:
enveloped_events.append({
'sensor': self.sensor_id,
'sendTime': convert_datetime_to_iso(datetime.now(UTC)),
'data': [event],
'dataVersion': CALIPER_EVENT_CONTEXT
})
return enveloped_events
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,11 @@ def get_object(self):

# TODO: replace with anonymous enrollment id?
course_root_url = self.get_object_iri('course', self.get_data('data.course_id', True))
caliper_object = {
caliper_object = super().get_object()
caliper_object.update({
'id': course_root_url,
'type': 'CourseOffering',
'name': course['display_name'],
'extensions': {'mode': self.get_data('data.mode')} if self.get_data('data.mode') is not None else None,
}
})
return caliper_object
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ def get_object(self):
dict
"""
self.backend_name = 'caliper'
caliper_object = self.transformed_event['object']
caliper_object = super().get_object()

data = self.get_data('data')
extensions = {}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ def get_object(self):
else:
iri_url = object_id

caliper_object = self.transformed_event['object']
caliper_object = super().get_object()
caliper_object.update({
'id': self.get_object_iri('xblock', iri_url),
'type': OBJECT_TYPE_MAP.get(key, 'Attempt'),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ def get_object(self):
dict
"""
self.backend_name = 'caliper'
caliper_object = self.transformed_event['object']
caliper_object = super().get_object()
data = self.get_data('data')
course_id = self.get_data('context.course_id', True)
video_id = self.get_data('data.id', True)
Expand Down Expand Up @@ -176,10 +176,12 @@ class VideoSpeedChangedTransformer(BaseVideoTransformer):
"""
Transform the event fired when a video's speed is changed.
"""
additional_fields = ('target', 'extensions',)
additional_fields = ('target',)

def get_extensions(self):
return {
extensions = super().get_extensions()
extensions.update({
'oldSpeed': self.get_data('old_speed'),
'newSpeed': self.get_data('new_speed'),
}
})
return extensions
10 changes: 5 additions & 5 deletions event_routing_backends/processors/caliper/tests/test_caliper.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ def test_skip_event_when_disabled(self):
@patch('event_routing_backends.processors.mixins.base_transformer_processor.logger')
def test_send_method_with_no_transformer_implemented(self, mocked_logger):
with self.assertRaises(EventEmissionExit):
self.processor(self.sample_event)
self.processor([self.sample_event])

mocked_logger.error.assert_called_once_with(
'Could not get transformer for %s event.',
Expand All @@ -49,7 +49,7 @@ def test_send_method_with_no_transformer_implemented(self, mocked_logger):
@patch('event_routing_backends.processors.mixins.base_transformer_processor.logger')
def test_send_method_with_unknown_exception(self, mocked_logger, _):
with self.assertRaises(ValueError):
self.processor(self.sample_event)
self.processor([self.sample_event])

mocked_logger.exception.assert_called_once_with(
'There was an error while trying to transform event "sentinel.name" using CaliperProcessor'
Expand All @@ -73,7 +73,7 @@ def test_send_method_with_successfull_flow(
mocked_transformer.transform.return_value = transformed_event
mocked_get_transformer.return_value = mocked_transformer

self.processor(self.sample_event)
self.processor([self.sample_event])

self.assertIn(
call(
Expand Down Expand Up @@ -109,7 +109,7 @@ def test_send_method_with_successfull_flow_logging_disabled(
mocked_transformer.transform.return_value = transformed_event
mocked_get_transformer.return_value = mocked_transformer

self.processor(self.sample_event)
self.processor([self.sample_event])

self.assertIn(
call(
Expand All @@ -131,5 +131,5 @@ def test_with_no_registry(self, mocked_logger):
backend = CaliperProcessor()
backend.registry = None
with self.assertRaises(EventEmissionExit):
self.assertIsNone(backend(self.sample_event))
self.assertIsNone(backend([self.sample_event]))
mocked_logger.exception.assert_called_once()
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,10 @@ def setUp(self):
def test_caliper_envelope_processor(self, mocked_datetime):
mocked_datetime.now.return_value = FROZEN_TIME

result = CaliperEnvelopeProcessor(sensor_id=self.sensor_id)(self.sample_event)
self.assertEqual(result, {
result = CaliperEnvelopeProcessor(sensor_id=self.sensor_id)([self.sample_event])
self.assertEqual(result, [{
'sensor': self.sensor_id,
'sendTime': convert_datetime_to_iso(str(FROZEN_TIME)),
'data': [self.sample_event],
'dataVersion': CALIPER_EVENT_CONTEXT
})
}])
67 changes: 45 additions & 22 deletions event_routing_backends/processors/caliper/transformer.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,58 +19,81 @@ class CaliperTransformer(BaseTransformerMixin):
required_fields = (
'type',
'object',
'action'
'action',
'extensions',
)

def base_transform(self):
def base_transform(self, transformed_event):
"""
Transform common Caliper fields.
"""
self._add_generic_fields()
self._add_actor_info()
self._add_session_info()
transformed_event = super().base_transform(transformed_event)
self._add_generic_fields(transformed_event)
self._add_actor_info(transformed_event)
self._add_session_info(transformed_event)
return transformed_event

def _add_generic_fields(self):
def _add_generic_fields(self, transformed_event):
"""
Add all of the generic fields to the transformed_event object.
"""
self.transformed_event.update({
transformed_event.update({
'@context': CALIPER_EVENT_CONTEXT,
'id': uuid.uuid4().urn,
'eventTime': convert_datetime_to_iso(self.get_data('timestamp', True)),
'extensions': {}
})
self.transformed_event['object'] = {}
course_id = self.get_data('context.course_id')
if course_id is not None:
extensions = {"isPartOf": {}}
extensions['isPartOf']['id'] = self.get_object_iri('course', course_id)
extensions['isPartOf']['type'] = 'CourseOffering'
self.transformed_event['object']['extensions'] = {}
self.transformed_event['object']['extensions'].update(extensions)

def _add_actor_info(self):
def _add_actor_info(self, transformed_event):
"""
Add all generic information related to `actor`.
Add all generic information related to `actor` to the transformed_event.
"""
self.transformed_event['actor'] = {
transformed_event['actor'] = {
'id': self.get_object_iri(
'user',
get_anonymous_user_id(self.extract_username_or_userid(), 'CALIPER'),
),
'type': 'Person'
}

def _add_session_info(self):
def _add_session_info(self, transformed_event):
"""
Add session info related to the event
Add session info related to the transformed_event.
"""
sessionid = self.extract_sessionid()
if sessionid:
self.transformed_event['session'] = {
transformed_event['session'] = {
'id': self.get_object_iri(
'sessions',
sessionid,
),
'type': 'Session'
}

def get_object(self):
"""
Return object for the event.
Returns:
dict
"""
caliper_object = super().get_object()
course_id = self.get_data('context.course_id')
if course_id is not None:
extensions = {"isPartOf": {}}
extensions['isPartOf']['id'] = self.get_object_iri('course', course_id)
extensions['isPartOf']['type'] = 'CourseOffering'
caliper_object['extensions'] = {}
caliper_object['extensions'].update(extensions)

return caliper_object

def get_extensions(self):
"""
Return extensions for the event.
Returns:
dict
"""
return {
'transformerVersion': self.transformer_version,
}
Loading

0 comments on commit 99ede4e

Please sign in to comment.