Only lookup a bug once through the API and correctly handle errors (mozilla#83)

Currently we retrieve the full bug in router and then again in the
action. Here a property is added to BugzillaWebhookRequest to get the
full bug when needed and cache it.

Author: Mossop

src/jbi/bugzilla.py

@@ -6,13 +6,15 @@
 import datetime
 import json
 import logging
+from functools import cached_property
 from typing import Dict, List, Optional, Tuple
 from urllib.parse import ParseResult, urlparse
 
 from pydantic import BaseModel  # pylint: disable=no-name-in-module
 
 from src.jbi.errors import ActionNotFoundError
 from src.jbi.models import Action, Actions
+from src.jbi.services import get_bugzilla
 
 logger = logging.getLogger(__name__)
 
@@ -216,6 +218,13 @@ def lookup_action(self, actions: Actions) -> Tuple[str, Action]:
 class BugzillaWebhookRequest(BaseModel):
     """Bugzilla Webhook Request Object"""
 
+    class Config:
+        """pydantic model config"""
+
+        keep_untouched = (
+            cached_property,
+        )  # https://github.com/samuelcolvin/pydantic/issues/1241
+
     webhook_id: int
     webhook_name: str
     event: BugzillaWebhookEvent
@@ -264,6 +273,20 @@ def map_as_comments(
 
         return [json.dumps(comment, indent=4) for comment in comments]
 
+    def getbug_as_bugzilla_object(self) -> BugzillaBug:
+        """Helper method to get up to date bug data from Request.bug.id in BugzillaBug format"""
+        current_bug_info = get_bugzilla().getbug(self.bug.id)  # type: ignore
+        return BugzillaBug.parse_obj(current_bug_info.__dict__)
+
+    @cached_property
+    def bugzilla_object(self) -> BugzillaBug:
+        """Returns the bugzilla bug object, querying the API as needed for private bugs"""
+        if not self.bug:
+            raise ValueError("missing bug reference")
+        if not self.bug.is_private:
+            return self.bug
+        return self.getbug_as_bugzilla_object()
+
 
 class BugzillaApiResponse(BaseModel):
     """Bugzilla Response Object"""

src/jbi/runner.py

@@ -7,7 +7,6 @@
 from src.jbi.bugzilla import BugzillaBug, BugzillaWebhookRequest
 from src.jbi.errors import ActionNotFoundError, IgnoreInvalidRequestError
 from src.jbi.models import Actions
-from src.jbi.services import getbug_as_bugzilla_object
 
 logger = logging.getLogger(__name__)
 
@@ -47,7 +46,13 @@ def execute_action(
         if not request.bug:
             raise IgnoreInvalidRequestError("no bug data received")
 
-        bug_obj: BugzillaBug = getbug_as_bugzilla_object(request=request)
+        try:
+            bug_obj: BugzillaBug = request.bugzilla_object
+        except Exception as ex:
+            logger.exception("Failed to get bug: %s", ex, extra=log_context)
+            raise IgnoreInvalidRequestError(
+                "bug not accessible or bugzilla down"
+            ) from ex
         log_context["bug"] = bug_obj.json()
 
         try:

src/jbi/services.py

@@ -3,7 +3,6 @@
 from atlassian import Jira  # type: ignore
 
 from src.app import environment
-from src.jbi.bugzilla import BugzillaBug, BugzillaWebhookRequest
 
 settings = environment.get_settings()
 
@@ -25,12 +24,6 @@ def get_bugzilla():
     )
 
 
-def getbug_as_bugzilla_object(request: BugzillaWebhookRequest) -> BugzillaBug:
-    """Helper method to get up to date bug data from Request.bug.id in BugzillaBug format"""
-    current_bug_info = get_bugzilla().getbug(request.bug.id)  # type: ignore
-    return BugzillaBug.parse_obj(current_bug_info.__dict__)
-
-
 def bugzilla_check_health():
     """Check health for Bugzilla Service"""
     bugzilla = get_bugzilla()

src/jbi/whiteboard_actions/default.py

@@ -10,7 +10,7 @@
 from src.app.environment import get_settings
 from src.jbi.bugzilla import BugzillaBug, BugzillaWebhookRequest
 from src.jbi.errors import ActionError
-from src.jbi.services import get_bugzilla, get_jira, getbug_as_bugzilla_object
+from src.jbi.services import get_bugzilla, get_jira
 
 settings = get_settings()
 
@@ -41,11 +41,9 @@ def __call__(  # pylint: disable=inconsistent-return-statements
         """Called from BZ webhook when default action is used. All default-action webhook-events are processed here."""
         target = payload.event.target  # type: ignore
         if target == "comment":
-            bug_obj = payload.bug
-            return self.comment_create_or_noop(payload=payload, bug_obj=bug_obj)  # type: ignore
+            return self.comment_create_or_noop(payload=payload)  # type: ignore
         if target == "bug":
-            bug_obj = getbug_as_bugzilla_object(payload)
-            return self.bug_create_or_update(payload=payload, bug_obj=bug_obj)
+            return self.bug_create_or_update(payload=payload)
         logger.debug(
             "Ignore event target %r",
             target,
@@ -54,10 +52,9 @@ def __call__(  # pylint: disable=inconsistent-return-statements
             },
         )
 
-    def comment_create_or_noop(
-        self, payload: BugzillaWebhookRequest, bug_obj: BugzillaBug
-    ):
+    def comment_create_or_noop(self, payload: BugzillaWebhookRequest):
         """Confirm issue is already linked, then apply comments; otherwise noop"""
+        bug_obj = payload.bugzilla_object
         linked_issue_key = bug_obj.extract_from_see_also()
 
         log_context = {
@@ -100,9 +97,10 @@ def update_issue(
         """Allows sub-classes to modify the Jira issue in response to a bug event"""
 
     def bug_create_or_update(
-        self, payload: BugzillaWebhookRequest, bug_obj: BugzillaBug
+        self, payload: BugzillaWebhookRequest
     ):  # pylint: disable=too-many-locals
         """Create and link jira issue with bug, or update; rollback if multiple events fire"""
+        bug_obj = payload.bugzilla_object
         linked_issue_key = bug_obj.extract_from_see_also()  # type: ignore
         if not linked_issue_key:
             return self.create_and_link_issue(payload, bug_obj)
@@ -180,7 +178,10 @@ def create_and_link_issue(
                 raise ActionError(f"response contains error: {jira_response_create}")
 
         jira_key_in_response = jira_response_create.get("key")
-        bug_obj = getbug_as_bugzilla_object(request=payload)
+
+        # In the time taken to create the Jira issue the bug may have been updated so
+        # re-retrieve it to ensure we have the latest data.
+        bug_obj = payload.getbug_as_bugzilla_object()
         jira_key_in_bugzilla = bug_obj.extract_from_see_also()
         _duplicate_creation_event = (
             jira_key_in_bugzilla is not None

tests/unit/jbi/test_bugzilla.py

@@ -112,3 +112,23 @@ def test_payload_changes_list_in_routing_key(webhook_change_status_assignee):
         "assigned_to",
         "status",
     ]
+
+
+def test_payload_bugzilla_object_public(mocked_bugzilla, webhook_modify_example):
+    bug_obj = webhook_modify_example.bugzilla_object
+    mocked_bugzilla().getbug.assert_not_called()
+    assert bug_obj.product == "JBI"
+    assert bug_obj.status == "NEW"
+    assert webhook_modify_example.bug == bug_obj
+
+
+def test_bugzilla_object_private(mocked_bugzilla, webhook_modify_private_example):
+    bug_obj = webhook_modify_private_example.bugzilla_object
+    mocked_bugzilla().getbug.assert_called_once_with(
+        webhook_modify_private_example.bug.id
+    )
+    assert bug_obj.product == "JBI"
+    assert bug_obj.status == "NEW"
+
+    bug_obj = webhook_modify_private_example.bugzilla_object
+    mocked_bugzilla().getbug.assert_called_once()