diff --git a/gerrit_util.py b/gerrit_util.py index 603a12dd28..6fc66fb813 100644 --- a/gerrit_util.py +++ b/gerrit_util.py @@ -1766,15 +1766,10 @@ def SetReview(host, labels=None, notify=None, ready=None, - automatic_attention_set_update: Optional[bool] = None, - project: Optional[str] = None): + automatic_attention_set_update: Optional[bool] = None): """Sets labels and/or adds a message to a code review. https://gerrit-review.googlesource.com/Documentation/rest-api-changes.html#set-review - - We need to check if this change's `project` needs to satisfy ReAuth - requirements, and attach an ReAuth credential to the request. `project` - can be optionally be provided to save one Gerrit server round-trip. """ if not msg and not labels: return @@ -1791,19 +1786,7 @@ def SetReview(host, if automatic_attention_set_update is not None: body[ 'ignore_automatic_attention_set_rules'] = not automatic_attention_set_update - - # ReAuth needed if we set labels (notably Code-Review). - reauth_context = None - if bool(labels): - reauth_context = auth.ReAuthContext( - host=host, - project=project or GetChangeDetail(host, change)["project"]) - - conn = CreateHttpConn(host, - path, - reqtype='POST', - body=body, - reauth_context=reauth_context) + conn = CreateHttpConn(host, path, reqtype='POST', body=body) response = ReadHttpJsonResponse(conn) if labels: for key, val in labels.items(): diff --git a/git_cl.py b/git_cl.py index e465596914..8e45c1161c 100755 --- a/git_cl.py +++ b/git_cl.py @@ -2270,8 +2270,7 @@ class Changelist(object): gerrit_util.SetReview(self.GetGerritHost(), self._GerritChangeIdentifier(), labels=labels, - notify=notify, - project=self.GetGerritProject()) + notify=notify) return 0 except KeyboardInterrupt: raise @@ -2589,8 +2588,7 @@ class Changelist(object): gerrit_util.SetReview(self.GetGerritHost(), self._GerritChangeIdentifier(), msg=message, - ready=publish, - project=self.GetGerritProject()) + ready=publish) def GetCommentsSummary(self, readable=True): # DETAILED_ACCOUNTS is to get emails in accounts. diff --git a/tests/gerrit_util_test.py b/tests/gerrit_util_test.py index 874e96e695..512386b181 100755 --- a/tests/gerrit_util_test.py +++ b/tests/gerrit_util_test.py @@ -656,59 +656,6 @@ class GerritUtilTest(unittest.TestCase): mockJsonResponse.return_value = {'status': {}} self.assertTrue(gerrit_util.IsCodeOwnersEnabledOnRepo('host', 'repo')) - @mock.patch('gerrit_util.CreateHttpConn') - @mock.patch('gerrit_util.ReadHttpJsonResponse') - @mock.patch('gerrit_util.GetChangeDetail') - def testSetReview_ReAuth(self, mockGetChangeDetail, mockJsonResponse, - mockCreateHttpConn): - mockJsonResponse.return_value = {"labels": {"Code-Review": 1}} - mockGetChangeDetail.return_value = { - "project": "infra/infra", - } - - gerrit_util.SetReview('chromium', 123456, labels={'Code-Review': 1}) - - # Check `project` in reauth_context is backfilled. - mockGetChangeDetail.assert_called_with('chromium', 123456) - httpConnKwargs = mockCreateHttpConn.call_args[1] - self.assertIn('reauth_context', httpConnKwargs) - self.assertEqual( - auth.ReAuthContext(host='chromium', project='infra/infra'), - httpConnKwargs['reauth_context']) - - @mock.patch('gerrit_util.CreateHttpConn') - @mock.patch('gerrit_util.ReadHttpJsonResponse') - @mock.patch('gerrit_util.GetChangeDetail') - def testSetReview_ReAuth_WithProject(self, mockGetChangeDetail, - mockJsonResponse, mockCreateHttpConn): - mockJsonResponse.return_value = {"labels": {"Code-Review": 1}} - - gerrit_util.SetReview('chromium', - 123456, - labels={'Code-Review': 1}, - project="infra/experimental") - - # Check reauth_context uses the given project. - mockGetChangeDetail.assert_not_called() - httpConnKwargs = mockCreateHttpConn.call_args[1] - self.assertIn('reauth_context', httpConnKwargs) - self.assertEqual( - auth.ReAuthContext(host='chromium', project='infra/experimental'), - httpConnKwargs['reauth_context']) - - @mock.patch('gerrit_util.CreateHttpConn') - @mock.patch('gerrit_util.ReadHttpJsonResponse') - @mock.patch('gerrit_util.GetChangeDetail') - def testSetReview_ReAuthNotNeeded(self, mockGetChangeDetail, - mockJsonResponse, mockCreateHttpConn): - mockJsonResponse.return_value = {"ready": True} - - gerrit_util.SetReview('chromium', 123456, msg="test") - - # ReAuth not needed. - mockGetChangeDetail.assert_not_called() - httpConnKwargs = mockCreateHttpConn.call_args[1] - self.assertIsNone(httpConnKwargs.get('reauth_context', None)) class SSOAuthenticatorTest(unittest.TestCase): diff --git a/tests/git_cl_test.py b/tests/git_cl_test.py index bb706ffa6c..149f37840a 100755 --- a/tests/git_cl_test.py +++ b/tests/git_cl_test.py @@ -638,11 +638,9 @@ class TestGitCl(unittest.TestCase): mock.patch('git_cl.gerrit_util.AddReviewers', lambda *a: self._mocked_call('AddReviewers', *a)).start() mock.patch('git_cl.gerrit_util.SetReview', - lambda h, i, msg=None, labels=None, notify=None, ready=None, - automatic_attention_set_update=None, project=None: + lambda h, i, msg=None, labels=None, notify=None, ready=None: (self._mocked_call('SetReview', h, i, msg, labels, notify, - ready, automatic_attention_set_update, - project))).start() + ready))).start() mock.patch('git_cl.gerrit_util.LuciContextAuthenticator.is_applicable', return_value=False).start() mock.patch('git_cl.gerrit_util.GceAuthenticator.is_applicable', @@ -2553,7 +2551,7 @@ class TestGitCl(unittest.TestCase): (('SetReview', 'chromium-review.googlesource.com', 'infra%2Finfra~123', None, { 'Commit-Queue': vote - }, notify, None, None, 'infra/infra'), ''), + }, notify, None), ''), ] @unittest.skipIf(gclient_utils.IsEnvCog(), @@ -3207,8 +3205,7 @@ class TestGitCl(unittest.TestCase): 'https://chromium.googlesource.com/infra/infra') self.calls = [ (('SetReview', 'chromium-review.googlesource.com', - 'infra%2Finfra~10', 'msg', None, None, None, None, 'infra/infra'), - None), + 'infra%2Finfra~10', 'msg', None, None, None), None), ] self.assertEqual(0, git_cl.main(['comment', '-i', '10', '-a', 'msg']))