From 5cc824a7f402f1083a737916e9119399e51d002c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Kone=C4=8Dn=C3=BD?= Date: Fri, 25 Mar 2022 11:02:50 +0100 Subject: [PATCH] Send json the correct way MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit During the manual testing was found out that the json wasn't sent the way the pagure is expecting it (as string). So this commit adds json.dumps before sending the payload itself. It also fixes formatting of one message. Signed-off-by: Michal Konečný --- tests/plugins/test_scm_request_processor.py | 4 +- tests/utils/test_pagure.py | 187 +++++++++++--------- toddlers/plugins/scm_request_processor.py | 12 +- toddlers/utils/pagure.py | 19 +- 4 files changed, 128 insertions(+), 94 deletions(-) diff --git a/tests/plugins/test_scm_request_processor.py b/tests/plugins/test_scm_request_processor.py index 4aa8d11..7f1273e 100644 --- a/tests/plugins/test_scm_request_processor.py +++ b/tests/plugins/test_scm_request_processor.py @@ -1499,9 +1499,9 @@ class TestCreateNewBranch: message = ( "The branch in PDC already exists, you can now create it yourself as follows:\n" - "Check in the project's settings if you have activated the git hook preventing" + "Check in the project's settings if you have activated the git hook preventing " "new git branches from being created and if you did, de-activate it.\n" - "Then simply run in cloned repository: " + "Then simply run in cloned repository:\n" "``git checkout -b && git push -u origin ``.\n" "```` is the name of the branch you requested. \n" "You only need to do this once and you can then use fedpkg as you normally do." diff --git a/tests/utils/test_pagure.py b/tests/utils/test_pagure.py index 806d139..20e330b 100644 --- a/tests/utils/test_pagure.py +++ b/tests/utils/test_pagure.py @@ -1,6 +1,7 @@ """ Unit tests for `toddlers.utils.pagure`. """ +import json from unittest.mock import call, Mock, patch import pytest @@ -177,8 +178,8 @@ class TestPagureCloseIssue: self.pagure.close_issue(issue_id, namespace, message, reason) self.pagure._requests_session.post.assert_called_with( - "https://pagure.io/test/issue/100/status", - data={"status": reason}, + "https://pagure.io/api/0/test/issue/100/status", + data=json.dumps({"status": reason}), headers=self.pagure.get_auth_header(), ) @@ -206,8 +207,8 @@ class TestPagureCloseIssue: self.pagure.close_issue(issue_id, namespace, message, reason) self.pagure._requests_session.post.assert_called_with( - "https://pagure.io/test/issue/100/status", - data={"status": reason}, + "https://pagure.io/api/0/test/issue/100/status", + data=json.dumps({"status": reason}), headers=self.pagure.get_auth_header(), ) @@ -245,7 +246,7 @@ class TestPagureAddCommentToIssue: self.pagure._requests_session.post.assert_called_with( "https://pagure.io/api/0/test/issue/100/comment", - data={"comment": message}, + data=json.dumps({"comment": message}), headers=self.pagure.get_auth_header(), ) @@ -269,7 +270,7 @@ class TestPagureAddCommentToIssue: self.pagure._requests_session.post.assert_called_with( "https://pagure.io/api/0/test/issue/100/comment", - data={"comment": message}, + data=json.dumps({"comment": message}), headers=self.pagure.get_auth_header(), ) @@ -388,14 +389,16 @@ class TestPagureNewProject: self.pagure._requests_session.post.assert_called_with( "https://pagure.io/api/0/new", - data={ - "namespace": namespace, - "name": repo, - "default_branch": default_branch, - "description": description, - "url": upstream_url, - "wait": True, - }, + data=json.dumps( + { + "namespace": namespace, + "name": repo, + "default_branch": default_branch, + "description": description, + "url": upstream_url, + "wait": True, + } + ), headers=self.pagure.get_auth_header(), ) @@ -425,15 +428,17 @@ class TestPagureNewProject: self.pagure._requests_session.post.assert_called_with( "https://pagure.io/api/0/new", - data={ - "namespace": namespace, - "name": repo, - "default_branch": default_branch, - "description": description, - "url": upstream_url, - "wait": True, - "create_readme": True, - }, + data=json.dumps( + { + "namespace": namespace, + "name": repo, + "default_branch": default_branch, + "description": description, + "url": upstream_url, + "wait": True, + "create_readme": True, + } + ), headers=self.pagure.get_auth_header(), ) @@ -467,25 +472,29 @@ class TestPagureNewProject: [ call( "https://pagure.io/api/0/new", - data={ - "namespace": namespace, - "name": repo, - "default_branch": default_branch, - "description": description, - "url": upstream_url, - "wait": True, - "create_readme": True, - }, + data=json.dumps( + { + "namespace": namespace, + "name": repo, + "default_branch": default_branch, + "description": description, + "url": upstream_url, + "wait": True, + "create_readme": True, + } + ), headers=self.pagure.get_auth_header(), ), call( "https://pagure.io/api/0/{0}/{1}/git/alias/new".format( namespace, repo ), - data={ - "alias_from": "main", - "alias_to": "rawhide", - }, + data=json.dumps( + { + "alias_from": "main", + "alias_to": "rawhide", + } + ), headers=self.pagure.get_auth_header(), ), ] @@ -520,15 +529,17 @@ class TestPagureNewProject: self.pagure._requests_session.post.assert_called_with( "https://pagure.io/api/0/new", - data={ - "namespace": namespace, - "name": repo, - "default_branch": default_branch, - "description": description, - "url": upstream_url, - "wait": True, - "create_readme": True, - }, + data=json.dumps( + { + "namespace": namespace, + "name": repo, + "default_branch": default_branch, + "description": description, + "url": upstream_url, + "wait": True, + "create_readme": True, + } + ), headers=self.pagure.get_auth_header(), ) @@ -556,14 +567,16 @@ class TestPagureNewProject: self.pagure._requests_session.post.assert_called_with( "https://pagure.io/api/0/new", - data={ - "namespace": namespace, - "name": repo, - "default_branch": default_branch, - "description": description, - "url": upstream_url, - "wait": True, - }, + data=json.dumps( + { + "namespace": namespace, + "name": repo, + "default_branch": default_branch, + "description": description, + "url": upstream_url, + "wait": True, + } + ), headers=self.pagure.get_auth_header(), ) @@ -608,25 +621,29 @@ class TestPagureNewProject: [ call( "https://pagure.io/api/0/new", - data={ - "namespace": namespace, - "name": repo, - "default_branch": default_branch, - "description": description, - "url": upstream_url, - "wait": True, - "create_readme": True, - }, + data=json.dumps( + { + "namespace": namespace, + "name": repo, + "default_branch": default_branch, + "description": description, + "url": upstream_url, + "wait": True, + "create_readme": True, + } + ), headers=self.pagure.get_auth_header(), ), call( "https://pagure.io/api/0/{0}/{1}/git/alias/new".format( namespace, repo ), - data={ - "alias_from": "main", - "alias_to": "rawhide", - }, + data=json.dumps( + { + "alias_from": "main", + "alias_to": "rawhide", + } + ), headers=self.pagure.get_auth_header(), ), ] @@ -667,10 +684,12 @@ class TestPagureNewBranch: self.pagure._requests_session.post.assert_called_with( "https://pagure.io/api/0/{0}/{1}/git/branch".format(namespace, repo), - data={ - "branch": branch, - "from_commit": from_commit, - }, + data=json.dumps( + { + "branch": branch, + "from_commit": from_commit, + } + ), headers=self.pagure.get_auth_header(), ) @@ -692,10 +711,12 @@ class TestPagureNewBranch: self.pagure._requests_session.post.assert_called_with( "https://pagure.io/api/0/{0}/{1}/git/branch".format(namespace, repo), - data={ - "branch": branch, - "from_branch": from_branch, - }, + data=json.dumps( + { + "branch": branch, + "from_branch": from_branch, + } + ), headers=self.pagure.get_auth_header(), ) @@ -768,10 +789,12 @@ class TestPagureNewBranch: self.pagure._requests_session.post.assert_called_with( "https://pagure.io/api/0/{0}/{1}/git/branch".format(namespace, repo), - data={ - "branch": branch, - "from_branch": from_branch, - }, + data=json.dumps( + { + "branch": branch, + "from_branch": from_branch, + } + ), headers=self.pagure.get_auth_header(), ) @@ -809,7 +832,7 @@ class TestPagureSetMonitoringStatus: self.pagure._requests_session.post.assert_called_with( "https://pagure.io/_dg/anitya/{0}/{1}".format(namespace, repo), - data={"anitya_status": monitoring_level}, + data=json.dumps({"anitya_status": monitoring_level}), headers=self.pagure.get_auth_header(), ) @@ -835,7 +858,7 @@ class TestPagureSetMonitoringStatus: self.pagure._requests_session.post.assert_called_with( "https://pagure.io/_dg/anitya/{0}/{1}".format(namespace, repo), - data={"anitya_status": monitoring_level}, + data=json.dumps({"anitya_status": monitoring_level}), headers=self.pagure.get_auth_header(), ) @@ -873,7 +896,7 @@ class TestPagureChangeProjectMainAdmin: self.pagure._requests_session.post.assert_called_with( "https://pagure.io/api/0/{0}/{1}".format(namespace, repo), - data={"main_admin": main_admin}, + data=json.dumps({"main_admin": main_admin}), headers=self.pagure.get_auth_header(), ) @@ -899,7 +922,7 @@ class TestPagureChangeProjectMainAdmin: self.pagure._requests_session.post.assert_called_with( "https://pagure.io/api/0/{0}/{1}".format(namespace, repo), - data={"main_admin": main_admin}, + data=json.dumps({"main_admin": main_admin}), headers=self.pagure.get_auth_header(), ) diff --git a/toddlers/plugins/scm_request_processor.py b/toddlers/plugins/scm_request_processor.py index c82c2cd..b211f40 100644 --- a/toddlers/plugins/scm_request_processor.py +++ b/toddlers/plugins/scm_request_processor.py @@ -44,6 +44,14 @@ INVALID_EPEL_ERROR = ( _log = logging.getLogger(__name__) +# Currently there is no way to generate PDC API token on either +# pdc.stg.fedoraproject.org or pdc.fedoraproject.org +# So let's keep this here to be able to at least mock pdc calls +# when testing. +# When using this comment out the pdc.set_pdc line in process method +# from unittest.mock import Mock +# pdc = Mock() + class SCMRequestProcessor(ToddlerBase): """ @@ -686,9 +694,9 @@ class SCMRequestProcessor(ToddlerBase): if pdc_branch: ticket_text = ( "The branch in PDC already exists, you can now create it yourself as follows:\n" - "Check in the project's settings if you have activated the git hook preventing" + "Check in the project's settings if you have activated the git hook preventing " "new git branches from being created and if you did, de-activate it.\n" - "Then simply run in cloned repository: " + "Then simply run in cloned repository:\n" "``git checkout -b && git push -u origin ``.\n" "```` is the name of the branch you requested. \n" "You only need to do this once and you can then use fedpkg as you normally do." diff --git a/toddlers/utils/pagure.py b/toddlers/utils/pagure.py index 1aecbd5..fe2d655 100644 --- a/toddlers/utils/pagure.py +++ b/toddlers/utils/pagure.py @@ -15,6 +15,7 @@ Examples: pagure_io.close_issue("", ) """ +import json import logging from typing import Any, Optional @@ -137,7 +138,9 @@ class Pagure: toddlers.utils.exceptions.PagureError when the issue couldn't be closed. """ issue_url = "{0}/{1}/issue/{2}".format(self._pagure_url, namespace, issue_id) - api_url = "{0}/status".format(issue_url) + api_url = "{0}/api/0/{1}/issue/{2}/status".format( + self._pagure_url, namespace, issue_id + ) status_payload = {"status": reason} headers = self.get_auth_header() @@ -150,7 +153,7 @@ class Pagure: ) ) response = self._requests_session.post( - api_url, data=status_payload, headers=headers + api_url, data=json.dumps(status_payload), headers=headers ) if response.status_code == 200: @@ -187,7 +190,7 @@ class Pagure: "Commenting on issue '{0}' with message '{1}'".format(issue_url, comment) ) response = self._requests_session.post( - api_url, data=comment_payload, headers=headers + api_url, data=json.dumps(comment_payload), headers=headers ) if response.status_code == 200: @@ -271,7 +274,7 @@ class Pagure: log.debug("Creating project '{0}/{1}'".format(namespace, repo)) response = self._requests_session.post( - pagure_new_project_url, data=payload, headers=headers + pagure_new_project_url, data=json.dumps(payload), headers=headers ) if response.status_code != 200: @@ -300,7 +303,7 @@ class Pagure: "Creating alias for project '{0}/{1}'".format(namespace, repo) ) response = self._requests_session.post( - pagure_new_git_alias_url, data=payload, headers=headers + pagure_new_git_alias_url, data=json.dumps(payload), headers=headers ) if response.status_code != 200: @@ -360,7 +363,7 @@ class Pagure: log.debug("Creating branch for project '{0}/{1}'".format(namespace, repo)) response = self._requests_session.post( - branch_api_url, data=payload, headers=headers + branch_api_url, data=json.dumps(payload), headers=headers ) if response.status_code != 200: @@ -402,7 +405,7 @@ class Pagure: ) ) response = self._requests_session.post( - monitoring_api_url, data=payload, headers=headers + monitoring_api_url, data=json.dumps(payload), headers=headers ) if response.status_code != 200: @@ -439,7 +442,7 @@ class Pagure: ) ) response = self._requests_session.post( - admin_api_url, data=payload, headers=headers + admin_api_url, data=json.dumps(payload), headers=headers ) if response.status_code != 200: