Reorganize calls to Bugzilla into a dedicated class in services.py (#220)

* Remove bugzilla_object attribute from model

* Use model for Bugzilla comments

* Rename getbug() to get_bug()

* Fix metric name

* Use parse_obj_as

* Assert about comment presence only once

* Remove unused model method

* Remove useless model config

Author: leplatrem

jbi/actions/default.py

@@ -74,7 +74,7 @@ def __call__(  # pylint: disable=inconsistent-return-statements
 
     def comment_create_or_noop(self, payload: BugzillaWebhookRequest) -> ActionResult:
         """Confirm issue is already linked, then apply comments; otherwise noop"""
-        bug_obj = payload.bugzilla_object
+        bug_obj = payload.bug
         linked_issue_key = bug_obj.extract_from_see_also()
 
         log_context = ActionLogContext(
@@ -94,17 +94,17 @@ def comment_create_or_noop(self, payload: BugzillaWebhookRequest) -> ActionResul
             )
             return False, {}
 
-        comment = payload.map_as_jira_comment()
-        if comment is None:
+        if bug_obj.comment is None:
             logger.debug(
                 "No matching comment found in payload",
                 extra=log_context.dict(),
             )
             return False, {}
 
+        formatted_comment = payload.map_as_jira_comment()
         jira_response = self.jira_client.issue_add_comment(
             issue_key=linked_issue_key,
-            comment=payload.map_as_jira_comment(),
+            comment=formatted_comment,
         )
         logger.debug(
             "Comment added to Jira issue %s",
@@ -144,7 +144,7 @@ def bug_create_or_update(
         self, payload: BugzillaWebhookRequest
     ) -> ActionResult:  # pylint: disable=too-many-locals
         """Create and link jira issue with bug, or update; rollback if multiple events fire"""
-        bug_obj = payload.bugzilla_object
+        bug_obj = payload.bug
         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)
@@ -205,10 +205,8 @@ def create_and_link_issue(  # pylint: disable=too-many-locals
             bug_obj.id,
             extra=log_context.dict(),
         )
-        comment_list = self.bugzilla_client.get_comments(idlist=[bug_obj.id])
-        description = comment_list["bugs"][str(bug_obj.id)]["comments"][0]["text"][
-            :JIRA_DESCRIPTION_CHAR_LIMIT
-        ]
+        comment_list = self.bugzilla_client.get_comments(bug_obj.id)
+        description = comment_list[0].text[:JIRA_DESCRIPTION_CHAR_LIMIT]
 
         fields = {
             **self.jira_fields(bug_obj),  # type: ignore
@@ -238,7 +236,8 @@ def create_and_link_issue(  # pylint: disable=too-many-locals
 
         # 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()
+        bug_obj = self.bugzilla_client.get_bug(bug_obj.id)
+
         jira_key_in_bugzilla = bug_obj.extract_from_see_also()
         _duplicate_creation_event = (
             jira_key_in_bugzilla is not None
@@ -263,8 +262,9 @@ def create_and_link_issue(  # pylint: disable=too-many-locals
             bug_obj.id,
             extra=log_context.update(operation=Operation.LINK).dict(),
         )
-        update = self.bugzilla_client.build_update(see_also_add=jira_url)
-        bugzilla_response = self.bugzilla_client.update_bugs([bug_obj.id], update)
+        bugzilla_response = self.bugzilla_client.update_bug(
+            bug_obj, see_also_add=jira_url
+        )
 
         bugzilla_url = f"{settings.bugzilla_base_url}/show_bug.cgi?id={bug_obj.id}"
         logger.debug(

jbi/models.py

@@ -25,7 +25,6 @@
 
 from jbi import Operation
 from jbi.errors import ActionNotFoundError
-from jbi.services import get_bugzilla
 
 logger = logging.getLogger(__name__)
 
@@ -316,41 +315,16 @@ def lookup_action(self, actions: Actions) -> Action:
 class BugzillaWebhookRequest(BaseModel):
     """Bugzilla Webhook Request Object"""
 
-    class Config:
-        """pydantic model config"""
-
-        keep_untouched = (
-            functools.cached_property,
-        )  # https://github.com/samuelcolvin/pydantic/issues/1241
-
     webhook_id: int
     webhook_name: str
     event: BugzillaWebhookEvent
     bug: BugzillaBug
 
     def map_as_jira_comment(self):
         """Extract comment from Webhook Event"""
-        comment: BugzillaWebhookComment = self.bug.comment
         commenter: BugzillaWebhookUser = self.event.user
-        comment_body: str = comment.body
-
-        if comment.is_private:
-            bug_comments = get_bugzilla().get_comments([self.bug.id])
-            comment_list = bug_comments["bugs"][str(self.bug.id)]["comments"]
-            matching_comments = [c for c in comment_list if c["id"] == comment.id]
-            if len(matching_comments) != 1:
-                return None
-            comment_body = matching_comments[0]["text"]
-
-        body = f"*({commenter.login})* commented: \n{{quote}}{comment_body}{{quote}}"
-        return body
-
-    def map_as_jira_description(self):
-        """Extract description as comment from Webhook Event"""
         comment: BugzillaWebhookComment = self.bug.comment
-        comment_body: str = comment.body
-        body = f"*(description)*: \n{{quote}}{comment_body}{{quote}}"
-        return body
+        return f"*({commenter.login})* commented: \n{{quote}}{comment.body}{{quote}}"
 
     def map_as_comments(
         self,
@@ -380,17 +354,14 @@ 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__)
 
-    @functools.cached_property
-    def bugzilla_object(self) -> BugzillaBug:
-        """Returns the bugzilla bug object, querying the API as needed for private bugs"""
-        if not self.bug.is_private:
-            return self.bug
-        return self.getbug_as_bugzilla_object()
+class BugzillaComment(BaseModel):
+    """Bugzilla Comment"""
+
+    id: int
+    text: str
+    is_private: bool
+    creator: str
 
 
 class BugzillaApiResponse(BaseModel):

jbi/runner.py

@@ -8,7 +8,8 @@
 from jbi import Operation
 from jbi.environment import Settings
 from jbi.errors import ActionNotFoundError, IgnoreInvalidRequestError
-from jbi.models import Actions, BugzillaBug, BugzillaWebhookRequest, RunnerLogContext
+from jbi.models import Actions, BugzillaWebhookRequest, RunnerLogContext
+from jbi.services import get_bugzilla
 
 logger = logging.getLogger(__name__)
 
@@ -38,23 +39,26 @@ def execute_action(
         )
 
         try:
-            bug_obj: BugzillaBug = request.bugzilla_object
+            if request.bug.is_private:
+                request = request.copy(
+                    update={"bug": get_bugzilla().get_bug(request.bug.id)}
+                )
         except Exception as err:
             logger.exception("Failed to get bug: %s", err, extra=log_context.dict())
             raise IgnoreInvalidRequestError(
                 "bug not accessible or bugzilla down"
             ) from err
-        log_context = log_context.update(bug=bug_obj)
 
+        log_context = log_context.update(bug=request.bug)
         try:
-            action = bug_obj.lookup_action(actions)
+            action = request.bug.lookup_action(actions)
         except ActionNotFoundError as err:
             raise IgnoreInvalidRequestError(
                 f"no action matching bug whiteboard tags: {err}"
             ) from err
         log_context = log_context.update(action=action)
 
-        if bug_obj.is_private and not action.allow_private:
+        if request.bug.is_private and not action.allow_private:
             raise IgnoreInvalidRequestError(
                 f"private bugs are not valid for action {action.whiteboard_tag!r}"
             )
@@ -63,7 +67,7 @@ def execute_action(
             "Execute action '%s:%s' for Bug %s",
             action.whiteboard_tag,
             action.module,
-            bug_obj.id,
+            request.bug.id,
             extra=log_context.update(operation=Operation.EXECUTE).dict(),
         )
 
@@ -72,7 +76,7 @@ def execute_action(
         logger.info(
             "Action %r executed successfully for Bug %s",
             action.whiteboard_tag,
-            bug_obj.id,
+            request.bug.id,
             extra=log_context.update(
                 operation=Operation.SUCCESS if handled else Operation.IGNORE
             ).dict(),

jbi/services.py

@@ -8,9 +8,11 @@
 import backoff
 import bugzilla as rh_bugzilla
 from atlassian import Jira, errors
+from pydantic import parse_obj_as
 from statsd.defaults.env import statsd
 
 from jbi import environment
+from jbi.models import BugzillaBug, BugzillaComment
 
 if TYPE_CHECKING:
     from jbi.models import Actions
@@ -84,15 +86,54 @@ def jira_visible_projects(jira=None) -> list[dict]:
     return projects
 
 
+class BugzillaClient:
+    """
+    Wrapper around the Bugzilla client to turn responses into our models instances.
+    """
+
+    def __init__(self, base_url: str, api_key: str):
+        """Constructor"""
+        self._client = rh_bugzilla.Bugzilla(base_url, api_key=api_key)
+
+    @property
+    def logged_in(self):
+        """Return `true` if credentials are valid"""
+        return self._client.logged_in
+
+    def get_bug(self, bugid) -> BugzillaBug:
+        """Return the Bugzilla object with all attributes"""
+        response = self._client.getbug(bugid).__dict__
+        bug = BugzillaBug.parse_obj(response)
+        # If comment is private, then webhook does not have comment, fetch it from server
+        if bug.comment and bug.comment.is_private:
+            comment_list = self.get_comments(bugid)
+            matching_comments = [c for c in comment_list if c.id == bug.comment.id]
+            # If no matching entry is found, set `bug.comment` to `None`.
+            found = matching_comments[0] if matching_comments else None
+            bug = bug.copy(update={"comment": found})
+        return bug
+
+    def get_comments(self, bugid) -> list[BugzillaComment]:
+        """Return the list of comments for the specified bug ID"""
+        response = self._client.get_comments(idlist=[bugid])
+        comments = response["bugs"][str(bugid)]["comments"]
+        return parse_obj_as(list[BugzillaComment], comments)
+
+    def update_bug(self, bugid, **attrs):
+        """Update the specified bug with the specified attributes"""
+        update = self._client.build_update(**attrs)
+        return self._client.update_bugs([bugid], update)
+
+
 def get_bugzilla():
     """Get bugzilla service"""
-    bugzilla_client = rh_bugzilla.Bugzilla(
+    bugzilla_client = BugzillaClient(
         settings.bugzilla_base_url, api_key=str(settings.bugzilla_api_key)
     )
     instrumented_methods = (
-        "getbug",
+        "get_bug",
         "get_comments",
-        "update_bugs",
+        "update_bug",
     )
     return InstrumentedClient(
         wrapped=bugzilla_client,

tests/conftest.py

@@ -32,9 +32,12 @@ def settings():
 
 
 @pytest.fixture(autouse=True)
-def mocked_bugzilla():
-    with mock.patch("jbi.services.rh_bugzilla.Bugzilla") as mocked_bz:
-        yield mocked_bz()
+def mocked_bugzilla(request):
+    if "no_mocked_bugzilla" in request.keywords:
+        yield None
+    else:
+        with mock.patch("jbi.services.BugzillaClient") as mocked_bz:
+            yield mocked_bz()
 
 
 @pytest.fixture(autouse=True)

tests/fixtures/factories.py

@@ -1,6 +1,7 @@
 from jbi.models import (
     Action,
     BugzillaBug,
+    BugzillaComment,
     BugzillaWebhookEvent,
     BugzillaWebhookRequest,
     BugzillaWebhookUser,
@@ -77,12 +78,14 @@ def webhook_factory(**overrides):
 
 
 def comment_factory(**overrides):
-    return {
-        "id": 343,
-        "text": "comment text",
-        "bug_id": 654321,
-        "count": 1,
-        "is_private": True,
-        "creator": "[email protected]",
-        **overrides,
-    }
+    return BugzillaComment.parse_obj(
+        {
+            "id": 343,
+            "text": "comment text",
+            "bug_id": 654321,
+            "count": 1,
+            "is_private": True,
+            "creator": "[email protected]",
+            **overrides,
+        }
+    )