From bda7c9f5ce30ab2f0761697a0de53944bb2988a7 Mon Sep 17 00:00:00 2001 From: James Kasten Date: Thu, 25 Jun 2015 18:57:10 -0700 Subject: [PATCH] refine vhost display --- letsencrypt_apache/configurator.py | 1 - letsencrypt_apache/display_ops.py | 30 ++++++++++++-------- letsencrypt_apache/dvsni.py | 1 - letsencrypt_apache/obj.py | 21 ++++++++------ letsencrypt_apache/tests/display_ops_test.py | 16 +++++++---- 5 files changed, 42 insertions(+), 27 deletions(-) diff --git a/letsencrypt_apache/configurator.py b/letsencrypt_apache/configurator.py index ae2ee261a..62064163d 100644 --- a/letsencrypt_apache/configurator.py +++ b/letsencrypt_apache/configurator.py @@ -929,7 +929,6 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): try: # Use check_output so the command will finish before reloading # TODO: a2enmod is debian specific... - self. subprocess.check_call([self.conf("enmod"), mod_name], stdout=open("/dev/null", "w"), stderr=open("/dev/null", "w")) diff --git a/letsencrypt_apache/display_ops.py b/letsencrypt_apache/display_ops.py index aab839d0c..39ad38cf2 100644 --- a/letsencrypt_apache/display_ops.py +++ b/letsencrypt_apache/display_ops.py @@ -1,3 +1,4 @@ +"""Contains UI methods for Apache operations.""" import os import zope.component @@ -38,26 +39,31 @@ def _vhost_menu(domain, vhosts): :rtype: `tuple` """ + filename_size = 24 + disp_name_size = 17 choices = [] for vhost in vhosts: - if vhost.names == 1: + if len(vhost.names) == 1: disp_name = next(iter(vhost.names)) - elif vhost.names == 0: + elif len(vhost.names) == 0: disp_name = "" else: disp_name = "Multiple Names" choices.append( - "%s | %s | %s | %s" % ( - os.path.basename(vhost.filep), - disp_name, - "HTTPS" if vhost.ssl, - "Enabled" if vhost.enabled) + "{0:{4}s} | {1:{5}s} | {2:5s} | {3:7s}".format( + os.path.basename(vhost.filep)[:filename_size], + disp_name[:disp_name_size], + "HTTPS" if vhost.ssl else "", + "Enabled" if vhost.enabled else "", + filename_size, + disp_name_size) ) code, tag = zope.component.getUtility(interfaces.IDisplay).menu( - "We were unable to find a vhost with a Servername or Address of %s." - "Which virtual host would you like to choose?" % domain, + "We were unable to find a vhost with a ServerName or Address of {0}.{1}" + "Which virtual host would you like to choose?".format( + domain, os.linesep), choices, help_label="More Info", ok_label="Select") return code, tag @@ -65,6 +71,6 @@ def _vhost_menu(domain, vhosts): def _more_info_vhost(vhost): zope.component.getUtility(interfaces.IDisplay).notification( - "Virtual Host Information:{0}{1}".format( - os.linesep, str(vhost)), - height=display_util.HEIGHT) \ No newline at end of file + "Virtual Host Information:{0}{1}{0}{2}".format( + os.linesep, "-" * (display_util.WIDTH - 4), str(vhost)), + height=display_util.HEIGHT) diff --git a/letsencrypt_apache/dvsni.py b/letsencrypt_apache/dvsni.py index bff317189..2542b242f 100644 --- a/letsencrypt_apache/dvsni.py +++ b/letsencrypt_apache/dvsni.py @@ -1,5 +1,4 @@ """ApacheDVSNI""" -import logging import os from letsencrypt.plugins import common diff --git a/letsencrypt_apache/obj.py b/letsencrypt_apache/obj.py index 8fa597ca6..b4cb6d960 100644 --- a/letsencrypt_apache/obj.py +++ b/letsencrypt_apache/obj.py @@ -31,14 +31,19 @@ class VirtualHost(object): # pylint: disable=too-few-public-methods self.names.add(name) def __str__(self): - addr_str = ", ".join(str(addr) for addr in self.addrs) - return ("File: %s\n" - "Vhost path: %s\n" - "Addresses: %s\n" - "Names: %s\n" - "TLS: %s\n" - "Enabled: %s" % (self.filep, self.path, addr_str, - self.names, self.ssl, self.enabled)) + return ( + "File: %s\n" + "Vhost path: %s\n" + "Addresses: %s\n" + "Names: %s\n" + "TLS Enabled: %s\n" + "Site Enabled: %s" % ( + self.filep, + self.path, + ", ".join(str(addr) for addr in self.addrs), + ", ".join(name for name in self.names), + "Yes" if self.ssl else "No", + "Yes" if self.enabled else "No")) def __eq__(self, other): if isinstance(other, self.__class__): diff --git a/letsencrypt_apache/tests/display_ops_test.py b/letsencrypt_apache/tests/display_ops_test.py index e08ba6198..bb9f1e136 100644 --- a/letsencrypt_apache/tests/display_ops_test.py +++ b/letsencrypt_apache/tests/display_ops_test.py @@ -1,7 +1,9 @@ """Test letsencrypt_apache.display_ops.""" +import sys import unittest import mock +import zope.component from letsencrypt_apache.tests import util @@ -12,6 +14,7 @@ class SelectVhostTest(unittest.TestCase): """Tests for letsencrypt_apache.display_ops.select_vhost.""" def setUp(self): + zope.component.provideUtility(display_util.FileDisplay(sys.stdout)) self.base_dir = "/example_path" self.vhosts = util.get_vh_truth( self.base_dir, "debian_apache_2_4/two_vhost_80") @@ -19,12 +22,12 @@ class SelectVhostTest(unittest.TestCase): @classmethod def _call(cls, vhosts): from letsencrypt_apache.display_ops import select_vhost - select_vhost("example.com", vhosts) + return select_vhost("example.com", vhosts) @mock.patch("letsencrypt_apache.display_ops.zope.component.getUtility") def test_successful_choice(self, mock_util): - mock_util().menu.return_value = (display_util.OK, 1) - self.assertEqual(self.vhosts[1], self._call(self.vhosts)) + mock_util().menu.return_value = (display_util.OK, 3) + self.assertEqual(self.vhosts[3], self._call(self.vhosts)) @mock.patch("letsencrypt_apache.display_ops.zope.component.getUtility") def test_more_info_cancel(self, mock_util): @@ -34,8 +37,11 @@ class SelectVhostTest(unittest.TestCase): (display_util.CANCEL, -1), ] - self.assertEqual(None, self._call()) + self.assertEqual(None, self._call(self.vhosts)) self.assertEqual(mock_util().notification.call_count, 2) def test_no_vhosts(self): - self.assertEqual(self._call([]), None) \ No newline at end of file + self.assertEqual(self._call([]), None) + +if __name__ == "__main__": + unittest.main() # pragma: no cover