cli: miscellaneous IReporter removals (certbot#8436)

* certbot delete: use undecorated print

* certbot revoke: use undecorated print

* certbot revoke: remove ireporter usages

* eff: remove IReporter usages

* certbot unregister: remove IReporter usage

* certbot update_account: remove IReporter usages

* certbot run: remove IReporter in duplicate prompt

* fix test_revoke_multiple_lineages

certbot-ci/certbot_integration_tests/certbot_tests/test_main.py

@@ -544,7 +544,8 @@ def test_revoke_multiple_lineages(context):
         'revoke', '--cert-path', join(context.config_dir, 'live', cert1, 'cert.pem')
     ])
 
-    assert 'Not deleting revoked certs due to overlapping archive dirs' in output
+    with open(join(context.workspace, 'logs', 'letsencrypt.log'), 'r') as f:
+        assert 'Not deleting revoked certs due to overlapping archive dirs' in f.read()
 
 
 def test_wildcard_certificates(context):

certbot/certbot/_internal/cert_manager.py

@@ -100,8 +100,8 @@ def delete(config):
         return
     for certname in certnames:
         storage.delete_files(config, certname)
-        disp.notification("Deleted all files relating to certificate {0}."
-            .format(certname), pause=False)
+        display_util.notify("Deleted all files relating to certificate {0}."
+                            .format(certname))
 
 ###################
 # Public Helpers

certbot/certbot/_internal/eff.py

@@ -7,6 +7,7 @@
 from acme.magic_typing import Optional  # pylint: disable=unused-import
 
 from certbot import interfaces
+from certbot.display import util as display_util
 from certbot._internal import constants
 from certbot._internal.account import Account  # pylint: disable=unused-import
 from certbot._internal.account import AccountFileStorage
@@ -133,5 +134,4 @@ def _report_failure(reason=None):
         msg.append(' because ')
         msg.append(reason)
     msg.append('. You can try again later by visiting https://act.eff.org.')
-    reporter = zope.component.getUtility(interfaces.IReporter)
-    reporter.add_message(''.join(msg), reporter.LOW_PRIORITY)
+    display_util.notify(''.join(msg))

certbot/certbot/_internal/main.py

@@ -163,17 +163,15 @@ def _handle_subset_cert_request(config, domains, cert):
                                        cli_flag="--expand",
                                        force_interactive=True):
         return "renew", cert
-    reporter_util = zope.component.getUtility(interfaces.IReporter)
-    reporter_util.add_message(
+    display_util.notify(
         "To obtain a new certificate that contains these names without "
         "replacing your existing certificate for {0}, you must use the "
         "--duplicate option.{br}{br}"
         "For example:{br}{br}{1} --duplicate {2}".format(
             existing,
             sys.argv[0], " ".join(sys.argv[1:]),
             br=os.linesep
-        ),
-        reporter_util.HIGH_PRIORITY)
+        ))
     raise errors.Error(USER_CANCELLED)
 
 
@@ -542,7 +540,6 @@ def _delete_if_appropriate(config):
         archive dir is found for the specified lineage, etc ...
     """
     display = zope.component.getUtility(interfaces.IDisplay)
-    reporter_util = zope.component.getUtility(interfaces.IReporter)
 
     attempt_deletion = config.delete_after_revoke
     if attempt_deletion is None:
@@ -552,7 +549,6 @@ def _delete_if_appropriate(config):
                 force_interactive=True, default=True)
 
     if not attempt_deletion:
-        reporter_util.add_message("Not deleting revoked certs.", reporter_util.LOW_PRIORITY)
         return
 
     # config.cert_path must have been set
@@ -570,9 +566,8 @@ def _delete_if_appropriate(config):
         cert_manager.match_and_check_overlaps(config, [lambda x: archive_dir],
             lambda x: x.archive_dir, lambda x: x)
     except errors.OverlappingMatchFound:
-        msg = ('Not deleting revoked certs due to overlapping archive dirs. More than '
-                'one lineage is using {0}'.format(archive_dir))
-        reporter_util.add_message(''.join(msg), reporter_util.MEDIUM_PRIORITY)
+        logger.warning("Not deleting revoked certs due to overlapping archive dirs. More than "
+                       "one certificate is using %s", archive_dir)
         return
     except Exception as e:
         msg = ('config.default_archive_dir: {0}, config.live_dir: {1}, archive_dir: {2},'
@@ -625,7 +620,6 @@ def unregister(config, unused_plugins):
     """
     account_storage = account.AccountFileStorage(config)
     accounts = account_storage.find_all()
-    reporter_util = zope.component.getUtility(interfaces.IReporter)
 
     if not accounts:
         return "Could not find existing account to deactivate."
@@ -647,7 +641,7 @@ def unregister(config, unused_plugins):
     # delete local account files
     account_files.delete(config.account)
 
-    reporter_util.add_message("Account deactivated.", reporter_util.MEDIUM_PRIORITY)
+    display_util.notify("Account deactivated.")
     return None
 
 
@@ -698,8 +692,6 @@ def update_account(config, unused_plugins):
     # exist or not.
     account_storage = account.AccountFileStorage(config)
     accounts = account_storage.find_all()
-    reporter_util = zope.component.getUtility(interfaces.IReporter)
-    add_msg = lambda m: reporter_util.add_message(m, reporter_util.MEDIUM_PRIORITY)
 
     if not accounts:
         return "Could not find an existing account to update."
@@ -724,10 +716,11 @@ def update_account(config, unused_plugins):
     account_storage.update_regr(acc, cb_client.acme)
 
     if config.email is None:
-        add_msg("Any contact information associated with this account has been removed.")
+        display_util.notify("Any contact information associated "
+                            "with this account has been removed.")
     else:
         eff.prepare_subscription(config, acc)
-        add_msg("Your e-mail address was updated to {0}.".format(config.email))
+        display_util.notify("Your e-mail address was updated to {0}.".format(config.email))
 
     return None
 

certbot/certbot/display/ops.py

@@ -262,17 +262,15 @@ def success_renewal(domains):
 
 
 def success_revocation(cert_path):
-    """Display a box confirming a certificate has been revoked.
+    """Display a message confirming a certificate has been revoked.
 
     :param list cert_path: path to certificate which was revoked.
 
     """
-    z_util(interfaces.IDisplay).notification(
+    display_util.notify(
         "Congratulations! You have successfully revoked the certificate "
-        "that was located at {0}{1}{1}".format(
-            cert_path,
-            os.linesep),
-        pause=False)
+        "that was located at {0}.".format(cert_path)
+    )
 
 
 def _gen_https_names(domains):

certbot/tests/cert_manager_test.py

@@ -114,16 +114,20 @@ def _call(self):
         cert_manager.delete(self.config)
 
     @test_util.patch_get_utility()
+    @mock.patch('certbot.display.util.notify')
     @mock.patch('certbot._internal.cert_manager.lineage_for_certname')
     @mock.patch('certbot._internal.storage.delete_files')
     def test_delete_from_config_yes(self, mock_delete_files, mock_lineage_for_certname,
-        mock_util):
+        mock_notify, mock_util):
         """Test delete"""
         mock_lineage_for_certname.return_value = self.test_rc
         mock_util().yesno.return_value = True
         self.config.certname = "example.org"
         self._call()
         mock_delete_files.assert_called_once_with(self.config, "example.org")
+        mock_notify.assert_called_once_with(
+            "Deleted all files relating to certificate example.org."
+        )
 
     @test_util.patch_get_utility()
     @mock.patch('certbot._internal.cert_manager.lineage_for_certname')

certbot/tests/display/ops_test.py

@@ -384,16 +384,14 @@ def _call(cls, path):
         success_revocation(path)
 
     @test_util.patch_get_utility("certbot.display.ops.z_util")
-    def test_success_revocation(self, mock_util):
-        mock_util().notification.return_value = None
+    @mock.patch("certbot.display.util.notify")
+    def test_success_revocation(self, mock_notify, unused_mock_util):
         path = "/path/to/cert.pem"
         self._call(path)
-        mock_util().notification.assert_called_once_with(
+        mock_notify.assert_called_once_with(
             "Congratulations! You have successfully revoked the certificate "
-            "that was located at {0}{1}{1}".format(
-                path,
-                os.linesep), pause=False)
-        self.assertTrue(path in mock_util().notification.call_args[0][0])
+            "that was located at {0}.".format(path)
+        )
 
 
 class ValidatorTests(unittest.TestCase):

certbot/tests/eff_test.py

@@ -43,11 +43,12 @@ def _call(self):
         prepare_subscription(self.config, self.account)
 
     @test_util.patch_get_utility()
-    def test_failure(self, mock_get_utility):
+    @mock.patch("certbot._internal.eff.display_util.notify")
+    def test_failure(self, mock_notify, mock_get_utility):
         self.config.email = None
         self.config.eff_email = True
         self._call()
-        actual = mock_get_utility().add_message.call_args[0][0]
+        actual = mock_notify.call_args[0][0]
         expected_part = "because you didn't provide an e-mail address"
         self.assertTrue(expected_part in actual)
         self.assertIsNone(self.account.meta.register_to_eff)
@@ -121,6 +122,10 @@ def setUp(self):
         self.json = {'status': True}
         self.response = mock.Mock(ok=True)
         self.response.json.return_value = self.json
+        self.mock_notify = mock.patch("certbot._internal.eff.display_util.notify").start()
+
+    def tearDown(self):
+        self.mock_notify.stop()
 
     @mock.patch('certbot._internal.eff.requests.post')
     def _call(self, mock_post):
@@ -139,42 +144,38 @@ def _check_post_call(self, mock_post):
         self.assertFalse(data is None)
         self.assertEqual(data.get('email'), self.email)
 
-    @test_util.patch_get_utility()
-    def test_bad_status(self, mock_get_utility):
+    def test_bad_status(self):
         self.json['status'] = False
         self._call()
-        actual = self._get_reported_message(mock_get_utility)
+        actual = self._get_reported_message()
         expected_part = 'because your e-mail address appears to be invalid.'
         self.assertTrue(expected_part in actual)
 
-    @test_util.patch_get_utility()
-    def test_not_ok(self, mock_get_utility):
+    def test_not_ok(self):
         self.response.ok = False
         self.response.raise_for_status.side_effect = requests.exceptions.HTTPError
         self._call()
-        actual = self._get_reported_message(mock_get_utility)
+        actual = self._get_reported_message()
         unexpected_part = 'because'
         self.assertFalse(unexpected_part in actual)
 
-    @test_util.patch_get_utility()
-    def test_response_not_json(self, mock_get_utility):
+    def test_response_not_json(self):
         self.response.json.side_effect = ValueError()
         self._call()
-        actual = self._get_reported_message(mock_get_utility)
+        actual = self._get_reported_message()
         expected_part = 'problem'
         self.assertTrue(expected_part in actual)
 
-    @test_util.patch_get_utility()
-    def test_response_json_missing_status_element(self, mock_get_utility):
+    def test_response_json_missing_status_element(self):
         self.json.clear()
         self._call()
-        actual = self._get_reported_message(mock_get_utility)
+        actual = self._get_reported_message()
         expected_part = 'problem'
         self.assertTrue(expected_part in actual)
 
-    def _get_reported_message(self, mock_get_utility):
-        self.assertTrue(mock_get_utility().add_message.called)
-        return mock_get_utility().add_message.call_args[0][0]
+    def _get_reported_message(self):
+        self.assertTrue(self.mock_notify.called)
+        return self.mock_notify.call_args[0][0]
 
     @test_util.patch_get_utility()
     def test_subscribe(self, mock_get_utility):

certbot/tests/main_test.py

@@ -339,23 +339,23 @@ def _call(self, mock_config):
         from certbot._internal.main import _delete_if_appropriate
         _delete_if_appropriate(mock_config)
 
-    def _test_delete_opt_out_common(self, mock_get_utility):
+    def _test_delete_opt_out_common(self):
         with mock.patch('certbot._internal.cert_manager.delete') as mock_delete:
             self._call(self.config)
         mock_delete.assert_not_called()
-        self.assertTrue(mock_get_utility().add_message.called)
 
     @test_util.patch_get_utility()
-    def test_delete_flag_opt_out(self, mock_get_utility):
+    def test_delete_flag_opt_out(self, unused_mock_get_utility):
         self.config.delete_after_revoke = False
-        self._test_delete_opt_out_common(mock_get_utility)
+        self._test_delete_opt_out_common()
 
     @test_util.patch_get_utility()
     def test_delete_prompt_opt_out(self, mock_get_utility):
         util_mock = mock_get_utility()
         util_mock.yesno.return_value = False
-        self._test_delete_opt_out_common(mock_get_utility)
+        self._test_delete_opt_out_common()
 
+    @mock.patch("certbot._internal.main.logger.warning")
     @mock.patch('certbot._internal.storage.renewal_file_for_certname')
     @mock.patch('certbot._internal.cert_manager.delete')
     @mock.patch('certbot._internal.cert_manager.match_and_check_overlaps')
@@ -365,7 +365,7 @@ def test_delete_prompt_opt_out(self, mock_get_utility):
     def test_overlapping_archive_dirs(self, mock_get_utility,
             mock_cert_path_to_lineage, mock_archive,
             mock_match_and_check_overlaps, mock_delete,
-            mock_renewal_file_for_certname):
+            mock_renewal_file_for_certname, mock_warning):
         # pylint: disable = unused-argument
         config = self.config
         config.cert_path = "/some/reasonable/path"
@@ -374,6 +374,7 @@ def test_overlapping_archive_dirs(self, mock_get_utility,
         mock_match_and_check_overlaps.side_effect = errors.OverlappingMatchFound()
         self._call(config)
         mock_delete.assert_not_called()
+        self.assertEqual(mock_warning.call_count, 1)
 
     @mock.patch('certbot._internal.storage.renewal_file_for_certname')
     @mock.patch('certbot._internal.cert_manager.match_and_check_overlaps')
@@ -1448,9 +1449,10 @@ def test_update_account_remove_email(self, mocked_account_module, mock_prepare,
         # ensure we didn't try to subscribe (no email to subscribe with)
         self.assertFalse(mock_prepare.called)
 
+    @mock.patch("certbot._internal.main.display_util.notify")
     @mock.patch('certbot._internal.main.display_ops.get_email')
     @test_util.patch_get_utility()
-    def test_update_account_with_email(self, mock_utility, mock_email):
+    def test_update_account_with_email(self, mock_utility, mock_email, mock_notify):
         email = "[email protected]"
         mock_email.return_value = email
         with mock.patch('certbot._internal.eff.prepare_subscription') as mock_prepare:
@@ -1475,7 +1477,7 @@ def test_update_account_with_email(self, mock_utility, mock_email):
                         # and we saved the updated registration on disk
                         self.assertTrue(mocked_storage.update_regr.called)
                         self.assertTrue(
-                            email in mock_utility().add_message.call_args[0][0])
+                            email in mock_notify.call_args[0][0])
                         self.assertTrue(mock_prepare.called)
 
     @mock.patch('certbot._internal.plugins.selection.choose_configurator_plugins')
@@ -1517,7 +1519,8 @@ def test_abort_unregister(self):
         res = main.unregister(config, unused_plugins)
         self.assertEqual(res, "Deactivation aborted.")
 
-    def test_unregister(self):
+    @mock.patch("certbot._internal.main.display_util.notify")
+    def test_unregister(self, mock_notify):
         mocked_storage = mock.MagicMock()
         mocked_storage.find_all.return_value = ["an account"]
 
@@ -1533,9 +1536,7 @@ def test_unregister(self):
         res = main.unregister(config, unused_plugins)
 
         self.assertTrue(res is None)
-        self.assertTrue(cb_client.acme.deactivate_registration.called)
-        m = "Account deactivated."
-        self.assertTrue(m in self.mocks['get_utility']().add_message.call_args[0][0])
+        mock_notify.assert_called_once_with("Account deactivated.")
 
     def test_unregister_no_account(self):
         mocked_storage = mock.MagicMock()