-
Notifications
You must be signed in to change notification settings - Fork 17
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Merge pull request #1 from eduNEXT/MJG/filter_tooling
feat: add Hooks Extension Framework tooling for filters #2
- Loading branch information
Showing
16 changed files
with
740 additions
and
20 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,5 @@ | ||
""" | ||
Events of the openedx platform. | ||
Filters of the Open edX platform. | ||
""" | ||
|
||
__version__ = "0.1.0" |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,38 @@ | ||
""" | ||
Exceptions thrown by filters. | ||
""" | ||
|
||
|
||
class HookFilterException(Exception): | ||
""" | ||
Base exception for filters. | ||
It is re-raised by the Pipeline Runner if any filter that is | ||
executing raises it. | ||
Arguments: | ||
message (str): message describing why the exception was raised. | ||
redirect_to (str): redirect URL. | ||
status_code (int): HTTP status code. | ||
keyword arguments (kwargs): extra arguments used to customize | ||
exception. | ||
""" | ||
|
||
def __init__(self, message="", redirect_to=None, status_code=None, **kwargs): | ||
""" | ||
Init method for HookFilterException. | ||
It's designed to allow flexible instantiation through **kwargs. | ||
""" | ||
super().__init__() | ||
self.message = message | ||
self.redirect_to = redirect_to | ||
self.status_code = status_code | ||
for key, value in kwargs.items(): | ||
setattr(self, key, value) | ||
|
||
def __str__(self): | ||
""" | ||
Show string representation of HookFilterException using its message. | ||
""" | ||
return "HookFilterException: {}".format(self.message) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,89 @@ | ||
""" | ||
Pipeline runner used to execute list of functions (actions or filters). | ||
""" | ||
from logging import getLogger | ||
|
||
from .exceptions import HookFilterException | ||
from .utils import get_functions_for_pipeline, get_pipeline_configuration | ||
|
||
log = getLogger(__name__) | ||
|
||
|
||
def run_pipeline(hook_name, *args, **kwargs): | ||
""" | ||
Execute filters in order. | ||
Given a list of functions paths, this function will execute | ||
them using the Accumulative Pipeline pattern defined in | ||
docs/decisions/0003-hooks-filter-tooling-pipeline.rst | ||
Example usage: | ||
result = run_pipeline( | ||
'org.openedx.service.subject.filter.action.major_version', | ||
raise_exception=True, | ||
request=request, | ||
user=user, | ||
) | ||
>>> result | ||
{ | ||
'result_test_1st_function': 1st_object, | ||
'result_test_2nd_function': 2nd_object, | ||
} | ||
Arguments: | ||
hook_name (str): determines which trigger we are listening to. | ||
It also specifies which hook configuration defined through settings. | ||
Returns: | ||
out (dict): accumulated outputs of the functions defined in pipeline. | ||
result (obj): return object of one of the pipeline functions. This will | ||
be the returned by the pipeline if one of the functions returns | ||
an object different than Dict or None. | ||
Exceptions raised: | ||
HookFilterException: custom exception re-raised when a function raises | ||
an exception of this type and raise_exception is set to True. This | ||
behavior is common when using filters. | ||
This pipeline implementation was inspired by: Social auth core. For more | ||
information check their Github repository: | ||
https://github.com/python-social-auth/social-core | ||
""" | ||
pipeline, raise_exception = get_pipeline_configuration(hook_name) | ||
|
||
if not pipeline: | ||
return kwargs | ||
|
||
functions = get_functions_for_pipeline(pipeline) | ||
|
||
out = kwargs.copy() | ||
for function in functions: | ||
try: | ||
result = function(*args, **out) or {} | ||
if not isinstance(result, dict): | ||
log.info( | ||
"Pipeline stopped by '%s' for returning an object.", | ||
function.__name__, | ||
) | ||
return result | ||
out.update(result) | ||
except HookFilterException as exc: | ||
if raise_exception: | ||
log.exception( | ||
"Exception raised while running '%s':\n %s", function.__name__, exc, | ||
) | ||
raise | ||
except Exception as exc: # pylint: disable=broad-except | ||
# We're catching this because we don't want the core to blow up | ||
# when a hook is broken. This exception will probably need some | ||
# sort of monitoring hooked up to it to make sure that these | ||
# errors don't go unseen. | ||
log.exception( | ||
"Exception raised while running '%s': %s\n%s", | ||
function.__name__, | ||
exc, | ||
"Continuing execution.", | ||
) | ||
continue | ||
|
||
return out |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
""" | ||
Test package for filters from the Hooks Extension Framework. | ||
""" |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
""" | ||
Tests for custom Hooks Exceptions. | ||
""" | ||
from django.test import TestCase | ||
|
||
from ..exceptions import HookFilterException | ||
|
||
|
||
class TestCustomHookFilterException(TestCase): | ||
""" | ||
Test class used to check flexibility when using HookFilterException. | ||
""" | ||
|
||
def test_exception_extra_arguments(self): | ||
""" | ||
This method raises HookFilterException with custom dynamic arguments. | ||
Expected behavior: | ||
Custom parameters can be accessed as instance arguments. | ||
""" | ||
hook_exception = HookFilterException(custom_arg="custom_argument") | ||
|
||
self.assertEqual( | ||
hook_exception.custom_arg, "custom_argument", # pylint: disable=no-member | ||
) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,214 @@ | ||
""" | ||
Tests for pipeline runner used by filters. | ||
""" | ||
from unittest.mock import Mock, patch | ||
|
||
from django.test import TestCase | ||
|
||
from ..exceptions import HookFilterException | ||
from ..pipeline import run_pipeline | ||
|
||
|
||
class TestRunningPipeline(TestCase): | ||
""" | ||
Test class to verify standard behavior of the Pipeline runner. | ||
""" | ||
|
||
def setUp(self): | ||
""" | ||
Setup common conditions for every test case. | ||
""" | ||
super().setUp() | ||
self.kwargs = { | ||
"request": Mock(), | ||
} | ||
self.pipeline = Mock() | ||
self.hook_name = "openedx.service.context.location.type.vi" | ||
|
||
@patch("openedx_filters.pipeline.get_pipeline_configuration") | ||
@patch("openedx_filters.pipeline.get_functions_for_pipeline") | ||
def test_run_empty_pipeline(self, get_functions_mock, get_configuration_mock): | ||
""" | ||
This method runs an empty pipeline, i.e, a pipeline without | ||
defined functions. | ||
Expected behavior: | ||
Returns the same input arguments. | ||
""" | ||
get_configuration_mock.return_value = ( | ||
[], | ||
True, | ||
) | ||
get_functions_mock.return_value = [] | ||
|
||
result = run_pipeline(self.hook_name, **self.kwargs) | ||
|
||
get_configuration_mock.assert_called_once_with( | ||
"openedx.service.context.location.type.vi", | ||
) | ||
get_functions_mock.assert_not_called() | ||
self.assertDictEqual(result, self.kwargs) | ||
|
||
@patch("openedx_filters.pipeline.get_pipeline_configuration") | ||
@patch("openedx_filters.pipeline.get_functions_for_pipeline") | ||
def test_raise_hook_exception(self, get_functions_mock, get_configuration_mock): | ||
""" | ||
This method runs a pipeline with a function that raises | ||
HookFilterException. This means that fail_silently must be set to | ||
False. | ||
Expected behavior: | ||
The pipeline re-raises the exception caught. | ||
""" | ||
get_configuration_mock.return_value = { | ||
"pipeline": self.pipeline, | ||
"fail_silently": False, | ||
} | ||
exception_message = "There was an error executing filter X." | ||
function = Mock(side_effect=HookFilterException(message=exception_message)) | ||
function.__name__ = "function_name" | ||
get_functions_mock.return_value = [function] | ||
log_message = "Exception raised while running '{func_name}':\n HookFilterException: {exc_msg}".format( | ||
func_name="function_name", exc_msg=exception_message, | ||
) | ||
|
||
with self.assertRaises(HookFilterException), self.assertLogs() as captured: | ||
run_pipeline(self.hook_name, **self.kwargs) | ||
self.assertEqual( | ||
captured.records[0].getMessage(), log_message, | ||
) | ||
|
||
@patch("openedx_filters.pipeline.get_pipeline_configuration") | ||
@patch("openedx_filters.pipeline.get_functions_for_pipeline") | ||
def test_not_raise_hook_exception(self, get_functions_mock, get_hook_config_mock): | ||
""" | ||
This method runs a pipeline with a function that raises | ||
HookFilterException but raise_exception is set to False. This means | ||
fail_silently must be set to True or not defined. | ||
Expected behavior: | ||
The pipeline does not re-raise the exception caught. | ||
""" | ||
get_hook_config_mock.return_value = ( | ||
Mock(), | ||
False, | ||
) | ||
return_value = { | ||
"request": Mock(), | ||
} | ||
function_with_exception = Mock(side_effect=HookFilterException) | ||
function_without_exception = Mock(return_value=return_value) | ||
get_functions_mock.return_value = [ | ||
function_with_exception, | ||
function_without_exception, | ||
] | ||
|
||
result = run_pipeline(self.hook_name, **self.kwargs) | ||
|
||
self.assertDictEqual(result, return_value) | ||
function_without_exception.assert_called_once_with(**self.kwargs) | ||
|
||
@patch("openedx_filters.pipeline.get_pipeline_configuration") | ||
@patch("openedx_filters.pipeline.get_functions_for_pipeline") | ||
def test_not_raise_common_exception(self, get_functions_mock, get_hook_config_mock): | ||
""" | ||
This method runs a pipeline with a function that raises a | ||
common Exception. | ||
Expected behavior: | ||
The pipeline continues execution after caughting Exception. | ||
""" | ||
get_hook_config_mock.return_value = ( | ||
self.pipeline, | ||
True, | ||
) | ||
return_value = { | ||
"request": Mock(), | ||
} | ||
function_with_exception = Mock(side_effect=ValueError("Value error exception")) | ||
function_with_exception.__name__ = "function_with_exception" | ||
function_without_exception = Mock(return_value=return_value) | ||
get_functions_mock.return_value = [ | ||
function_with_exception, | ||
function_without_exception, | ||
] | ||
log_message = ( | ||
"Exception raised while running 'function_with_exception': " | ||
"Value error exception\nContinuing execution." | ||
) | ||
|
||
with self.assertLogs() as captured: | ||
result = run_pipeline(self.hook_name, **self.kwargs) | ||
|
||
self.assertEqual( | ||
captured.records[0].getMessage(), log_message, | ||
) | ||
self.assertDictEqual(result, return_value) | ||
function_without_exception.assert_called_once_with(**self.kwargs) | ||
|
||
@patch("openedx_filters.pipeline.get_pipeline_configuration") | ||
@patch("openedx_filters.pipeline.get_functions_for_pipeline") | ||
def test_getting_pipeline_result(self, get_functions_mock, get_hook_config_mock): | ||
""" | ||
This method runs a pipeline with functions defined via configuration. | ||
Expected behavior: | ||
Returns the processed dictionary. | ||
""" | ||
get_hook_config_mock.return_value = ( | ||
self.pipeline, | ||
True, | ||
) | ||
return_value_1st = { | ||
"request": Mock(), | ||
} | ||
return_value_2nd = { | ||
"user": Mock(), | ||
} | ||
return_overall_value = {**return_value_1st, **return_value_2nd} | ||
first_function = Mock(return_value=return_value_1st) | ||
second_function = Mock(return_value=return_value_2nd) | ||
get_functions_mock.return_value = [ | ||
first_function, | ||
second_function, | ||
] | ||
|
||
result = run_pipeline(self.hook_name, **self.kwargs) | ||
|
||
first_function.assert_called_once_with(**self.kwargs) | ||
second_function.assert_called_once_with(**return_value_1st) | ||
self.assertDictEqual(result, return_overall_value) | ||
|
||
@patch("openedx_filters.pipeline.get_pipeline_configuration") | ||
@patch("openedx_filters.pipeline.get_functions_for_pipeline") | ||
def test_partial_pipeline(self, get_functions_mock, get_hook_config_mock): | ||
""" | ||
This method runs a pipeline with functions defined via configuration. | ||
At some point, returns an object to stop execution. | ||
Expected behavior: | ||
Returns the object used to stop execution. | ||
""" | ||
get_hook_config_mock.return_value = ( | ||
self.pipeline, | ||
True, | ||
) | ||
return_value_1st = Mock() | ||
first_function = Mock(return_value=return_value_1st) | ||
first_function.__name__ = "first_function" | ||
second_function = Mock() | ||
get_functions_mock.return_value = [ | ||
first_function, | ||
second_function, | ||
] | ||
log_message = "Pipeline stopped by 'first_function' for returning an object." | ||
|
||
with self.assertLogs() as captured: | ||
result = run_pipeline(self.hook_name, **self.kwargs) | ||
|
||
self.assertEqual( | ||
captured.records[0].getMessage(), log_message, | ||
) | ||
first_function.assert_called_once_with(**self.kwargs) | ||
second_function.assert_not_called() | ||
self.assertEqual(result, return_value_1st) |
Oops, something went wrong.