Refactor to de-duplicate calls to extra_message_body

sync2jira/main.py

@@ -173,7 +173,8 @@ def listen(config):
 
         log.debug("Handling %r %r %r", suffix, topic, idx)
 
-        handle_msg(msg, suffix, config)
+        body = c.extract_message_body(msg)
+        handle_msg(body, suffix, config)
 
 
 def initialize_issues(config, testing=False, repo_name=None):
@@ -295,10 +296,10 @@ def initialize_recent(config):
         handle_msg({"body": body}, suffix, config)
 
 
-def handle_msg(msg, suffix, config):
+def handle_msg(body, suffix, config):
     """
     Function to handle incoming message from datagrepper
-    :param Dict msg: Incoming message
+    :param Dict body: Incoming message body
     :param String suffix: Incoming suffix
     :param Dict config: Config dict
     """
@@ -307,22 +308,21 @@ def handle_msg(msg, suffix, config):
     # GitHub '.issue*' is used for both PR and Issue
     # Check for that edge case
     if suffix.startswith("github.issue"):
-        body = c.extract_message_body(msg)
         if "pull_request" in body["issue"] and body["action"] != "deleted":
             # pr_filter turns on/off the filtering of PRs
-            pr = issue_handlers[suffix](msg, config, pr_filter=False)
+            pr = issue_handlers[suffix](body, config, pr_filter=False)
             if not pr:
                 return
             # Issues do not have suffix and reporter needs to be reformatted
             pr.suffix = suffix
             pr.reporter = pr.reporter.get("fullname")
             setattr(pr, "match", matcher(pr.content, pr.comments))
         else:
-            issue = issue_handlers[suffix](msg, config)
+            issue = issue_handlers[suffix](body, config)
     elif suffix in issue_handlers:
-        issue = issue_handlers[suffix](msg, config)
+        issue = issue_handlers[suffix](body, config)
     elif suffix in pr_handlers:
-        pr = pr_handlers[suffix](msg, config, suffix)
+        pr = pr_handlers[suffix](body, config, suffix)
 
     if not issue and not pr:
         return

sync2jira/upstream_issue.py

@@ -24,7 +24,6 @@
 from github import Github
 import requests
 
-import sync2jira.compat as c
 import sync2jira.intermediary as i
 
 log = logging.getLogger("sync2jira")
@@ -111,17 +110,16 @@
 """
 
 
-def handle_github_message(msg, config, pr_filter=True):
+def handle_github_message(body, config, pr_filter=True):
     """
     Handle GitHub message from FedMsg.
 
-    :param Dict msg: FedMsg Message
+    :param Dict body: FedMsg Message body
     :param Dict config: Config File
     :param Bool pr_filter: Switch to ignore pull_requests
     :returns: Issue object
     :rtype: sync2jira.intermediary.Issue
     """
-    body = c.extract_message_body(msg)
     owner = body["repository"]["owner"]["login"]
     repo = body["repository"]["name"]
     upstream = "{owner}/{repo}".format(owner=owner, repo=repo)

sync2jira/upstream_pr.py

@@ -21,24 +21,22 @@
 
 from github import Github
 
-import sync2jira.compat as c
 import sync2jira.intermediary as i
 import sync2jira.upstream_issue as u_issue
 
 log = logging.getLogger("sync2jira")
 
 
-def handle_github_message(msg, config, suffix):
+def handle_github_message(body, config, suffix):
     """
     Handle GitHub message from FedMsg.
 
-    :param Dict msg: FedMsg Message
+    :param Dict body: FedMsg Message body
     :param Dict config: Config File
     :param String suffix: FedMsg suffix
     :returns: Issue object
     :rtype: sync2jira.intermediary.PR
     """
-    body = c.extract_message_body(msg)
     owner = body["repository"]["owner"]["login"]
     repo = body["repository"]["name"]
     upstream = "{owner}/{repo}".format(owner=owner, repo=repo)

tests/test_main.py

@@ -30,7 +30,15 @@ def setUp(self):
         }
 
         # Mock Fedmsg Message
-        self.mock_message = {"msg_id": "mock_id", "msg": {"issue": "mock_issue"}}
+        self.mock_message_body = {"issue": "mock_issue"}
+        self.old_style_mock_message = {
+            "msg_id": "mock_id",
+            "msg": self.mock_message_body,
+        }
+        self.new_style_mock_message = {
+            "msg_id": "mock_id",
+            "msg": self.mock_message_body,
+        }
 
     def _check_for_exception(self, loader, target, exc=ValueError):
         try:
@@ -288,7 +296,7 @@ def test_listen_no_handlers(self, mock_fedmsg, mock_handle_msg):
         """
         # Set up return values
         mock_fedmsg.tail_messages.return_value = [
-            ("dummy", "dummy", "mock_topic", self.mock_message)
+            ("dummy", "dummy", "mock_topic", self.old_style_mock_message)
         ]
 
         # Call the function
@@ -307,7 +315,7 @@ def test_listen_no_issue(self, mock_fedmsg, mock_handlers_issue, mock_handle_msg
         # Set up return values
         mock_handlers_issue["github.issue.comment"].return_value = None
         mock_fedmsg.tail_messages.return_value = [
-            ("dummy", "dummy", "d.d.d.github.issue.drop", self.mock_message)
+            ("dummy", "dummy", "d.d.d.github.issue.drop", self.old_style_mock_message)
         ]
 
         # Call the function
@@ -326,15 +334,20 @@ def test_listen(self, mock_fedmsg, mock_handlers_issue, mock_handle_msg):
         # Set up return values
         mock_handlers_issue["github.issue.comment"].return_value = "dummy_issue"
         mock_fedmsg.tail_messages.return_value = [
-            ("dummy", "dummy", "d.d.d.github.issue.comment", self.mock_message)
+            (
+                "dummy",
+                "dummy",
+                "d.d.d.github.issue.comment",
+                self.old_style_mock_message,
+            )
         ]
 
         # Call the function
         m.listen(self.mock_config)
 
         # Assert everything was called correctly
         mock_handle_msg.assert_called_with(
-            self.mock_message, "github.issue.comment", self.mock_config
+            self.mock_message_body, "github.issue.comment", self.mock_config
         )
 
     @mock.patch(PATH + "send_mail")
@@ -370,7 +383,9 @@ def test_handle_msg_no_handlers(self, mock_d, mock_u):
         Tests 'handle_msg' function where there are no handlers
         """
         # Call the function
-        m.handle_msg(self.mock_message, "no_handler", self.mock_config)
+        m.handle_msg(
+            body=self.mock_message_body, suffix="no_handler", config=self.mock_config
+        )
 
         # Assert everything was called correctly
         mock_d.sync_with_jira.assert_not_called()
@@ -387,7 +402,11 @@ def test_handle_msg_no_issue(self, mock_d, mock_u, mock_handlers_issue):
         mock_handlers_issue["github.issue.comment"].return_value = None
 
         # Call the function
-        m.handle_msg(self.mock_message, "github.issue.comment", self.mock_config)
+        m.handle_msg(
+            body=self.mock_message_body,
+            suffix="github.issue.comment",
+            config=self.mock_config,
+        )
 
         # Assert everything was called correctly
         mock_d.sync_with_jira.assert_not_called()
@@ -405,7 +424,11 @@ def test_handle_msg(self, mock_d, mock_u, mock_handlers_issue):
         mock_u.handle_github_message.return_value = "dummy_issue"
 
         # Call the function
-        m.handle_msg(self.mock_message, "github.issue.comment", self.mock_config)
+        m.handle_msg(
+            body=self.mock_message_body,
+            suffix="github.issue.comment",
+            config=self.mock_config,
+        )
 
         # Assert everything was called correctly
         mock_d.sync_with_jira.assert_called_with("dummy_issue", self.mock_config)

tests/test_upstream_issue.py

@@ -39,33 +39,17 @@ def setUp(self):
         self.mock_github_comment.created_at = "mock_created_at"
 
         # Mock GitHub Message
-        self.old_style_mock_github_message = {
-            "msg": {
-                "repository": {"owner": {"login": "org"}, "name": "repo"},
-                "issue": {
-                    "filter1": "filter1",
-                    "labels": [{"name": "custom_tag"}],
-                    "comments": ["some_comments!"],
-                    "number": "mock_number",
-                    "user": {"login": "mock_login"},
-                    "assignees": [{"login": "mock_login"}],
-                    "milestone": {"title": "mock_milestone"},
-                },
-            }
-        }
-        self.new_style_mock_github_message = {
-            "body": {
-                "repository": {"owner": {"login": "org"}, "name": "repo"},
-                "issue": {
-                    "filter1": "filter1",
-                    "labels": [{"name": "custom_tag"}],
-                    "comments": ["some_comments!"],
-                    "number": "mock_number",
-                    "user": {"login": "mock_login"},
-                    "assignees": [{"login": "mock_login"}],
-                    "milestone": {"title": "mock_milestone"},
-                },
-            }
+        self.mock_github_message_body = {
+            "repository": {"owner": {"login": "org"}, "name": "repo"},
+            "issue": {
+                "filter1": "filter1",
+                "labels": [{"name": "custom_tag"}],
+                "comments": ["some_comments!"],
+                "number": "mock_number",
+                "user": {"login": "mock_login"},
+                "assignees": [{"login": "mock_login"}],
+                "milestone": {"title": "mock_milestone"},
+            },
         }
 
         # Mock GitHub issue
@@ -496,13 +480,11 @@ def test_handle_github_message_not_in_mapped(
         This function tests 'handle_github_message' where upstream is not in mapped repos
         """
         # Set up return values
-        self.old_style_mock_github_message["msg"]["repository"]["owner"][
-            "login"
-        ] = "bad_owner"
+        self.mock_github_message_body["repository"]["owner"]["login"] = "bad_owner"
 
         # Call the function
         response = u.handle_github_message(
-            msg=self.old_style_mock_github_message, config=self.mock_config
+            body=self.mock_github_message_body, config=self.mock_config
         )
 
         # Assert that all calls were made correctly
@@ -519,11 +501,11 @@ def test_handle_github_message_pull_request(
         This function tests 'handle_github_message' the issue is a pull request comment
         """
         # Set up return values
-        self.old_style_mock_github_message["msg"]["issue"] = {"pull_request": "test"}
+        self.mock_github_message_body["issue"] = {"pull_request": "test"}
 
         # Call the function
         response = u.handle_github_message(
-            msg=self.old_style_mock_github_message, config=self.mock_config
+            body=self.mock_github_message_body, config=self.mock_config
         )
 
         # Assert that all calls were made correctly
@@ -537,11 +519,11 @@ def test_handle_github_message_bad_filter(self, mock_issue_from_github):
         This function tests 'handle_github_message' where comparing the actual vs. filter does not equate
         """
         # Set up return values
-        self.old_style_mock_github_message["msg"]["issue"]["filter1"] = "filter2"
+        self.mock_github_message_body["issue"]["filter1"] = "filter2"
 
         # Call function
         response = u.handle_github_message(
-            msg=self.old_style_mock_github_message, config=self.mock_config
+            body=self.mock_github_message_body, config=self.mock_config
         )
         # Assert that calls were made correctly
         mock_issue_from_github.assert_not_called()
@@ -553,13 +535,11 @@ def test_handle_github_message_bad_label(self, mock_issue_from_github):
         This function tests 'handle_github_message' where comparing the actual vs. filter does not equate
         """
         # Set up return values
-        self.old_style_mock_github_message["msg"]["issue"]["labels"] = [
-            {"name": "bad_label"}
-        ]
+        self.mock_github_message_body["issue"]["labels"] = [{"name": "bad_label"}]
 
         # Call function
         response = u.handle_github_message(
-            msg=self.old_style_mock_github_message, config=self.mock_config
+            body=self.mock_github_message_body, config=self.mock_config
         )
         # Assert that calls were made correctly
         mock_issue_from_github.assert_not_called()
@@ -576,11 +556,11 @@ def test_handle_github_message_no_comments(
         # Set up return values
         mock_issue_from_github.return_value = "Successful Call!"
         mock_github.return_value = self.mock_github_client
-        self.old_style_mock_github_message["msg"]["issue"]["comments"] = 0
+        self.mock_github_message_body["issue"]["comments"] = 0
 
         # Call function
         response = u.handle_github_message(
-            msg=self.old_style_mock_github_message, config=self.mock_config
+            body=self.mock_github_message_body, config=self.mock_config
         )
         # Assert that calls were made correctly
         mock_issue_from_github.assert_called_with(
@@ -617,54 +597,7 @@ def test_handle_old_style_github_message_successful(
 
         # Call function
         response = u.handle_github_message(
-            msg=self.old_style_mock_github_message, config=self.mock_config
-        )
-
-        # Assert that calls were made correctly
-        mock_issue_from_github.assert_called_with(
-            "org/repo",
-            {
-                "labels": ["custom_tag"],
-                "number": "mock_number",
-                "comments": [
-                    {
-                        "body": "mock_body",
-                        "name": unittest.mock.ANY,
-                        "author": "mock_username",
-                        "changed": None,
-                        "date_created": "mock_created_at",
-                        "id": "mock_id",
-                    }
-                ],
-                "assignees": [{"fullname": "mock_name"}],
-                "filter1": "filter1",
-                "user": {"login": "mock_login", "fullname": "mock_name"},
-                "milestone": "mock_milestone",
-            },
-            self.mock_config,
-        )
-        mock_github.assert_called_with("mock_token", retry=5)
-        self.assertEqual("Successful Call!", response)
-        self.mock_github_client.get_repo.assert_called_with("org/repo")
-        self.mock_github_repo.get_issue.assert_called_with(number="mock_number")
-        self.mock_github_issue.get_comments.assert_any_call()
-        self.mock_github_client.get_user.assert_called_with("mock_login")
-
-    @mock.patch(PATH + "Github")
-    @mock.patch("sync2jira.intermediary.Issue.from_github")
-    def test_handle_new_style_github_message_successful(
-        self, mock_issue_from_github, mock_github
-    ):
-        """
-        This function tests 'handle_github_message' where everything goes smoothly!
-        """
-        # Set up return values
-        mock_issue_from_github.return_value = "Successful Call!"
-        mock_github.return_value = self.mock_github_client
-
-        # Call function
-        response = u.handle_github_message(
-            msg=self.new_style_mock_github_message, config=self.mock_config
+            body=self.mock_github_message_body, config=self.mock_config
         )
 
         # Assert that calls were made correctly