diff --git a/README.md b/README.md index 72dae44d..98c1b81e 100644 --- a/README.md +++ b/README.md @@ -75,7 +75,7 @@ graph TD # Development -We use [pandoc](https://pandoc.org) to convert markdown to the Jira syntax. Make sure the binary is found in path or [specify your custom location](https://github.com/JessicaTegner/pypandoc#specifying-the-location-of-pandoc-binaries). +We use [pandoc](https://pandoc.org) to convert markdown to the Jira syntax. Make sure the binary is found in path or [specify your custom location](https://github.com/JessicaTegner/pypandoc#specifying-the-location-of-pandoc-binaries) and its version is sufficiently recent to support Jira syntax (e.g. 3.6.3). - `make start`: run the application locally (http://localhost:8000) - `make test`: run the unit tests suites diff --git a/jbi/models.py b/jbi/models.py index 728d261e..71d32b8e 100644 --- a/jbi/models.py +++ b/jbi/models.py @@ -238,7 +238,7 @@ class RunnerContext(Context, extra="forbid"): operation: Operation event: WebhookEvent - action: Optional[Action] = None + actions: Optional[list[Action]] = None bug: BugId | Bug diff --git a/jbi/runner.py b/jbi/runner.py index 282b3389..82840955 100644 --- a/jbi/runner.py +++ b/jbi/runner.py @@ -58,20 +58,23 @@ def groups2operation(steps: ActionSteps): return by_operation -def lookup_action(bug: bugzilla_models.Bug, actions: Actions) -> Action: +def lookup_actions(bug: bugzilla_models.Bug, actions: Actions) -> list[Action]: """ - Find first matching action from bug's whiteboard field. + Find matching actions from bug's whiteboard field. Tags are strings between brackets and can have prefixes/suffixes using dashes (eg. ``[project]``, ``[project-moco]``, ``[project-moco-sprint1]``). """ if bug.whiteboard: + relevant_actions = [] for tag, action in actions.by_tag.items(): # [tag-word], [tag-], [tag], but not [word-tag] or [tagword] search_string = r"\[" + tag + r"(-[^\]]*)*\]" if re.search(search_string, bug.whiteboard, flags=re.IGNORECASE): - return action + relevant_actions.append(action) + if len(relevant_actions): + return relevant_actions raise ActionNotFoundError(", ".join(actions.by_tag.keys())) @@ -203,12 +206,17 @@ def execute_action( request: bugzilla_models.WebhookRequest, actions: Actions, ): - """Execute the configured action for the specified `request`. + """Execute the configured actions for the specified `request`. + + If multiple actions are configured for a given request, all of them + are executed. This will raise an `IgnoreInvalidRequestError` error if the request does not contain bug data or does not match any action. - The value returned by the action call is returned. + A dictionary containing the values returned by the actions calls + is returned. The action tag is used to index the responses in the + dictionary. """ bug, event = request.bug, request.event runner_context = RunnerContext( @@ -221,7 +229,7 @@ def execute_action( raise IgnoreInvalidRequestError("private bugs are not supported") try: - action = lookup_action(bug, actions) + relevant_actions = lookup_actions(bug, actions) except ActionNotFoundError as err: raise IgnoreInvalidRequestError( f"no bug whiteboard matching action tags: {err}" @@ -238,10 +246,42 @@ def execute_action( # is processed (eg. if it spent some time in the DL queue) raise IgnoreInvalidRequestError(str(err)) from err - runner_context = runner_context.update(bug=bug) + runner_context = runner_context.update(bug=bug, actions=relevant_actions) + + return do_execute_actions(runner_context, bug, relevant_actions) + except IgnoreInvalidRequestError as exception: + logger.info( + "Ignore incoming request: %s", + exception, + extra=runner_context.update(operation=Operation.IGNORE).model_dump(), + ) + statsd.incr("jbi.bugzilla.ignored.count") + raise - runner_context = runner_context.update(action=action) +@statsd.timer("jbi.action.execution.timer") +def do_execute_actions( + runner_context: RunnerContext, + bug: bugzilla_models.Bug, + actions: Actions, +): + """Execute the provided actions on the bug, within the provided context. + + This will raise an `IgnoreInvalidRequestError` error if the request + does not contain bug data or does not match any action. + + A dictionary containing the values returned by the actions calls + is returned. The action tag is used to index the responses in the + dictionary. + """ + runner_context = runner_context.update(bug=bug) + + runner_context = runner_context.update(actions=actions) + + event = runner_context.event + + details = {} + for action in actions: linked_issue_key: Optional[str] = bug.extract_from_see_also( project_key=action.jira_project_key ) @@ -275,6 +315,10 @@ def execute_action( if ( project_key := jira_issue["fields"]["project"]["key"] ) != action_context.jira.project: + # TODO: We're now executing multiple actions for a given bug, we + # should probably either not fail and instead report which actions + # failed to apply, or execute all the changes as a "transaction" and + # roll them back if one of them fails. raise IgnoreInvalidRequestError( f"ignore linked project {project_key!r} (!={action_context.jira.project!r})" ) @@ -306,7 +350,8 @@ def execute_action( extra=runner_context.update(operation=Operation.EXECUTE).model_dump(), ) executor = Executor(parameters=action.parameters) - handled, details = executor(context=action_context) + handled, action_details = executor(context=action_context) + details[action.whiteboard_tag] = action_details statsd.incr(f"jbi.operation.{action_context.operation.lower()}.count") logger.info( "Action %r executed successfully for Bug %s", @@ -317,12 +362,4 @@ def execute_action( ).model_dump(), ) statsd.incr("jbi.bugzilla.processed.count") - return details - except IgnoreInvalidRequestError as exception: - logger.info( - "Ignore incoming request: %s", - exception, - extra=runner_context.update(operation=Operation.IGNORE).model_dump(), - ) - statsd.incr("jbi.bugzilla.ignored.count") - raise + return details diff --git a/tests/conftest.py b/tests/conftest.py index 7c103fa5..a0d71bae 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -55,6 +55,7 @@ def mocked_statsd(): yield _mocked_statsd +register(factories.ActionStepsFactory) register(factories.ActionContextFactory) register(factories.ActionFactory) register(factories.ActionsFactory, "_actions") diff --git a/tests/fixtures/factories.py b/tests/fixtures/factories.py index e7db1e47..86a52227 100644 --- a/tests/fixtures/factories.py +++ b/tests/fixtures/factories.py @@ -23,10 +23,16 @@ def _build(cls, model_class, *args, **kwargs): return model_class.model_construct(**kwargs) +class ActionStepsFactory(PydanticFactory): + class Meta: + model = models.ActionSteps + + class ActionParamsFactory(PydanticFactory): class Meta: model = models.ActionParams + steps = factory.SubFactory(ActionStepsFactory) jira_project_key = "JBI" jira_components = {} labels_brackets = "no" diff --git a/tests/unit/test_runner.py b/tests/unit/test_runner.py index 5314bffb..cc867ee6 100644 --- a/tests/unit/test_runner.py +++ b/tests/unit/test_runner.py @@ -5,6 +5,7 @@ import requests import responses +import tests.fixtures.factories as factories from jbi import Operation from jbi.bugzilla.client import BugNotAccessibleError from jbi.environment import get_settings @@ -15,7 +16,7 @@ Executor, execute_action, execute_or_queue, - lookup_action, + lookup_actions, ) @@ -187,7 +188,7 @@ def test_action_is_logged_as_success_if_returns_true( ("Action 'devtest' executed successfully for Bug 654321", Operation.SUCCESS), ] assert capturelogs.records[-1].bug["id"] == 654321 - assert capturelogs.records[-1].action["whiteboard_tag"] == "devtest" + assert capturelogs.records[-1].actions[0]["whiteboard_tag"] == "devtest" def test_action_is_logged_as_ignore_if_returns_false( @@ -619,11 +620,42 @@ def test_counter_is_incremented_for_attachment( ) def test_lookup_action_found(whiteboard, actions, bug_factory): bug = bug_factory(id=1234, whiteboard=whiteboard) - action = lookup_action(bug, actions) + action = lookup_actions(bug, actions)[0] assert action.whiteboard_tag == "devtest" assert "test config" in action.description +@pytest.mark.parametrize( + "whiteboard,expected_tags", + [ + ("[example][DevTest]", ["devtest"]), + ("[DevTest][example]", ["devtest"]), + ("[example][DevTest][other]", ["devtest", "other"]), + ], +) +def test_multiple_lookup_actions_found(whiteboard, expected_tags, bug_factory): + actions = factories.ActionsFactory( + root=[ + factories.ActionFactory( + whiteboard_tag="devtest", + bugzilla_user_id="tbd", + description="test config", + ), + factories.ActionFactory( + whiteboard_tag="other", + bugzilla_user_id="tbd", + description="test config", + ), + ] + ) + bug = bug_factory(id=1234, whiteboard=whiteboard) + acts = lookup_actions(bug, actions) + assert len(acts) == len(expected_tags) + looked_up_tags = [a.whiteboard_tag for a in acts] + assert sorted(looked_up_tags) == sorted(expected_tags) + assert all(["test config" == a.description for a in acts]) + + @pytest.mark.parametrize( "whiteboard", [ @@ -649,5 +681,114 @@ def test_lookup_action_found(whiteboard, actions, bug_factory): def test_lookup_action_not_found(whiteboard, actions, bug_factory): bug = bug_factory(id=1234, whiteboard=whiteboard) with pytest.raises(ActionNotFoundError) as exc_info: - lookup_action(bug, actions) + lookup_actions(bug, actions)[0] assert str(exc_info.value) == "devtest" + + +def test_request_triggers_multiple_actions( + webhook_request_factory, + mocked_bugzilla, +): + actions = factories.ActionsFactory( + root=[ + factories.ActionFactory( + whiteboard_tag="devtest", + bugzilla_user_id="tbd", + description="test config", + ), + factories.ActionFactory( + whiteboard_tag="other", + bugzilla_user_id="tbd", + description="test config", + ), + ] + ) + + webhook = webhook_request_factory(bug__whiteboard="[devtest][other]") + mocked_bugzilla.get_bug.return_value = webhook.bug + + details = execute_action(request=webhook, actions=actions) + + # Details has the following shape: + # {'devtest': {'responses': [..]}, 'other': {'responses': [...]}} + assert len(actions) == len(details) + assert "devtest" in details + assert "other" in details + + +def test_request_triggers_multiple_update_actions( + webhook_request_factory, + mocked_bugzilla, + mocked_jira, + webhook_event_change_factory, +): + actions = factories.ActionsFactory( + root=[ + factories.ActionFactory( + whiteboard_tag="devtest", + bugzilla_user_id="tbd", + description="test config", + parameters__jira_project_key="JBI", + parameters__steps__existing=["maybe_update_issue_resolution"], + parameters__resolution_map={ + "FIXED": "Closed", + }, + ), + factories.ActionFactory( + whiteboard_tag="other", + bugzilla_user_id="tbd", + description="test config", + parameters__jira_project_key="DE", + parameters__steps__existing=["maybe_update_issue_resolution"], + parameters__resolution_map={ + "FIXED": "Done", + }, + ), + ] + ) + + webhook = webhook_request_factory( + bug__whiteboard="[devtest][other]", + bug__see_also=[ + "https://mozilla.atlassian.net/browse/JBI-234", + "https://mozilla.atlassian.net/browse/DE-567", + ], + bug__resolution="FIXED", + event__changes=[ + webhook_event_change_factory( + field="resolution", removed="OPEN", added="FIXED" + ) + ], + ) + mocked_bugzilla.get_bug.return_value = webhook.bug + + def side_effect_for_get_issue(issue_key): + if issue_key.startswith("JBI-"): + return {"fields": {"project": {"key": "JBI"}}} + elif issue_key.startswith("DE-"): + return {"fields": {"project": {"key": "DE"}}} + + return None + + mocked_jira.get_issue.side_effect = side_effect_for_get_issue + + details = execute_action(request=webhook, actions=actions) + + mocked_jira.update_issue_field.assert_any_call( + key="JBI-234", + fields={ + "resolution": {"name": "Closed"}, + }, + ) + mocked_jira.update_issue_field.assert_any_call( + key="DE-567", + fields={ + "resolution": {"name": "Done"}, + }, + ) + + # Details has the following shape: + # {'devtest': {'responses': [..]}, 'other': {'responses': [...]}} + assert len(actions) == len(details) + assert "devtest" in details + assert "other" in details