From 402f18e039ff162afb2015ee3b56088596ba450b Mon Sep 17 00:00:00 2001 From: Mads Jensen Date: Thu, 2 Dec 2021 16:45:16 +0100 Subject: [PATCH] Apache augeas clean up (#9114) The `# self.comment = comment` caught my eye while working on #9071 as well as the intermediate variables, which aren't really needed. As a result, I reformatted the code slightly in those places. * Remove comment in AugeasCommentNode.__init__ * Replace some intermediate varibles with return-statements in apache augeas parser. * more clean-up --- .../certbot_apache/_internal/apache_util.py | 1 + .../certbot_apache/_internal/assertions.py | 10 +++++ .../certbot_apache/_internal/augeasparser.py | 45 ++++++++++--------- .../certbot_apache/_internal/configurator.py | 2 +- .../certbot_apache/_internal/display_ops.py | 2 +- .../certbot_apache/_internal/dualparser.py | 9 ++-- .../certbot_apache/_internal/obj.py | 1 - 7 files changed, 39 insertions(+), 31 deletions(-) diff --git a/certbot-apache/certbot_apache/_internal/apache_util.py b/certbot-apache/certbot_apache/_internal/apache_util.py index 3d06814b6..adbfc06bc 100644 --- a/certbot-apache/certbot_apache/_internal/apache_util.py +++ b/certbot-apache/certbot_apache/_internal/apache_util.py @@ -242,6 +242,7 @@ def _get_runtime_cfg(command): return stdout + def find_ssl_apache_conf(prefix): """ Find a TLS Apache config file in the dedicated storage. diff --git a/certbot-apache/certbot_apache/_internal/assertions.py b/certbot-apache/certbot_apache/_internal/assertions.py index 46bed3265..ea625d020 100644 --- a/certbot-apache/certbot_apache/_internal/assertions.py +++ b/certbot-apache/certbot_apache/_internal/assertions.py @@ -29,6 +29,7 @@ def assertEqual(first, second): # (but identical) directory structures. assert first.filepath == second.filepath + def assertEqualComment(first, second): # pragma: no cover """ Equality assertion for CommentNode """ @@ -38,6 +39,7 @@ def assertEqualComment(first, second): # pragma: no cover if not isPass(first.comment) and not isPass(second.comment): # type: ignore assert first.comment == second.comment # type: ignore + def _assertEqualDirectiveComponents(first, second): # pragma: no cover """ Handles assertion for instance variables for DirectiveNode and BlockNode""" @@ -50,6 +52,7 @@ def _assertEqualDirectiveComponents(first, second): # pragma: no cover if not isPass(first.parameters) and not isPass(second.parameters): assert first.parameters == second.parameters + def assertEqualDirective(first, second): """ Equality assertion for DirectiveNode """ @@ -57,12 +60,14 @@ def assertEqualDirective(first, second): assert isinstance(second, interfaces.DirectiveNode) _assertEqualDirectiveComponents(first, second) + def isPass(value): # pragma: no cover """Checks if the value is set to PASS""" if isinstance(value, bool): return True return PASS in value + def isPassDirective(block): """ Checks if BlockNode or DirectiveNode should pass the assertion """ @@ -74,6 +79,7 @@ def isPassDirective(block): return True return False + def isPassComment(comment): """ Checks if CommentNode should pass the assertion """ @@ -83,6 +89,7 @@ def isPassComment(comment): return True return False + def isPassNodeList(nodelist): # pragma: no cover """ Checks if a ParserNode in the nodelist should pass the assertion, this function is used for results of find_* methods. Unimplemented find_* @@ -101,11 +108,13 @@ def isPassNodeList(nodelist): # pragma: no cover return isPassDirective(node) return isPassComment(node) + def assertEqualSimple(first, second): """ Simple assertion """ if not isPass(first) and not isPass(second): assert first == second + def isEqualVirtualHost(first, second): """ Checks that two VirtualHost objects are similar. There are some built @@ -126,6 +135,7 @@ def isEqualVirtualHost(first, second): first.ancestor == second.ancestor ) + def assertEqualPathsList(first, second): # pragma: no cover """ Checks that the two lists of file paths match. This assertion allows for wildcard diff --git a/certbot-apache/certbot_apache/_internal/augeasparser.py b/certbot-apache/certbot_apache/_internal/augeasparser.py index 896e17cf8..e3b30c0a0 100644 --- a/certbot-apache/certbot_apache/_internal/augeasparser.py +++ b/certbot-apache/certbot_apache/_internal/augeasparser.py @@ -160,8 +160,7 @@ class AugeasParserNode(interfaces.ParserNode): # remove [...], it's not allowed in Apache configuration and is used # for indexing within Augeas - name = name.split("[")[0] - return name + return name.split("[")[0] class AugeasCommentNode(AugeasParserNode): @@ -170,7 +169,6 @@ class AugeasCommentNode(AugeasParserNode): def __init__(self, **kwargs): comment, kwargs = util.commentnode_kwargs(kwargs) # pylint: disable=unused-variable super().__init__(**kwargs) - # self.comment = comment self.comment = comment def __eq__(self, other): @@ -278,13 +276,14 @@ class AugeasBlockNode(AugeasDirectiveNode): ) # Parameters will be set at the initialization of the new object - new_block = AugeasBlockNode(name=name, - parameters=parameters, - enabled=enabled, - ancestor=assertions.PASS, - filepath=apache_util.get_file_path(realpath), - metadata=new_metadata) - return new_block + return AugeasBlockNode( + name=name, + parameters=parameters, + enabled=enabled, + ancestor=assertions.PASS, + filepath=apache_util.get_file_path(realpath), + metadata=new_metadata, + ) # pylint: disable=unused-argument def add_child_directive(self, name, parameters=None, position=None): # pragma: no cover @@ -308,13 +307,14 @@ class AugeasBlockNode(AugeasDirectiveNode): apache_util.get_file_path(realpath) ) - new_dir = AugeasDirectiveNode(name=name, - parameters=parameters, - enabled=enabled, - ancestor=assertions.PASS, - filepath=apache_util.get_file_path(realpath), - metadata=new_metadata) - return new_dir + return AugeasDirectiveNode( + name=name, + parameters=parameters, + enabled=enabled, + ancestor=assertions.PASS, + filepath=apache_util.get_file_path(realpath), + metadata=new_metadata, + ) def add_child_comment(self, comment="", position=None): """Adds a new CommentNode to the sequence of children""" @@ -330,11 +330,12 @@ class AugeasBlockNode(AugeasDirectiveNode): # Set the comment content self.parser.aug.set(realpath, comment) - new_comment = AugeasCommentNode(comment=comment, - ancestor=assertions.PASS, - filepath=apache_util.get_file_path(realpath), - metadata=new_metadata) - return new_comment + return AugeasCommentNode( + comment=comment, + ancestor=assertions.PASS, + filepath=apache_util.get_file_path(realpath), + metadata=new_metadata, + ) def find_blocks(self, name, exclude=True): """Recursive search of BlockNodes from the sequence of children""" diff --git a/certbot-apache/certbot_apache/_internal/configurator.py b/certbot-apache/certbot_apache/_internal/configurator.py index 35bc8609a..1decd976c 100644 --- a/certbot-apache/certbot_apache/_internal/configurator.py +++ b/certbot-apache/certbot_apache/_internal/configurator.py @@ -485,7 +485,7 @@ class ApacheConfigurator(common.Configurator): name=assertions.PASS, ancestor=None, filepath=self.parser.loc["root"], - metadata=metadata + metadata=metadata, ) def deploy_cert(self, domain, cert_path, key_path, diff --git a/certbot-apache/certbot_apache/_internal/display_ops.py b/certbot-apache/certbot_apache/_internal/display_ops.py index 86f696173..43de2f995 100644 --- a/certbot-apache/certbot_apache/_internal/display_ops.py +++ b/certbot-apache/certbot_apache/_internal/display_ops.py @@ -103,7 +103,7 @@ def _vhost_menu(domain, vhosts): https="HTTPS" if vhost.ssl else "", active="Enabled" if vhost.enabled else "", fn_size=filename_size, - name_size=disp_name_size) + name_size=disp_name_size), ) try: diff --git a/certbot-apache/certbot_apache/_internal/dualparser.py b/certbot-apache/certbot_apache/_internal/dualparser.py index c89ff95be..7559e3839 100644 --- a/certbot-apache/certbot_apache/_internal/dualparser.py +++ b/certbot-apache/certbot_apache/_internal/dualparser.py @@ -185,8 +185,7 @@ class DualBlockNode(DualNodeBase): primary_new = self.primary.add_child_block(name, parameters, position) secondary_new = self.secondary.add_child_block(name, parameters, position) assertions.assertEqual(primary_new, secondary_new) - new_block = DualBlockNode(primary=primary_new, secondary=secondary_new) - return new_block + return DualBlockNode(primary=primary_new, secondary=secondary_new) def add_child_directive(self, name, parameters=None, position=None): """ Creates a new child DirectiveNode, asserts that both implementations @@ -196,8 +195,7 @@ class DualBlockNode(DualNodeBase): primary_new = self.primary.add_child_directive(name, parameters, position) secondary_new = self.secondary.add_child_directive(name, parameters, position) assertions.assertEqual(primary_new, secondary_new) - new_dir = DualDirectiveNode(primary=primary_new, secondary=secondary_new) - return new_dir + return DualDirectiveNode(primary=primary_new, secondary=secondary_new) def add_child_comment(self, comment="", position=None): """ Creates a new child CommentNode, asserts that both implementations @@ -207,8 +205,7 @@ class DualBlockNode(DualNodeBase): primary_new = self.primary.add_child_comment(comment, position) secondary_new = self.secondary.add_child_comment(comment, position) assertions.assertEqual(primary_new, secondary_new) - new_comment = DualCommentNode(primary=primary_new, secondary=secondary_new) - return new_comment + return DualCommentNode(primary=primary_new, secondary=secondary_new) def _create_matching_list(self, primary_list, secondary_list): """ Matches the list of primary_list to a list of secondary_list and diff --git a/certbot-apache/certbot_apache/_internal/obj.py b/certbot-apache/certbot_apache/_internal/obj.py index 9001a860d..4905f971b 100644 --- a/certbot-apache/certbot_apache/_internal/obj.py +++ b/certbot-apache/certbot_apache/_internal/obj.py @@ -176,7 +176,6 @@ class VirtualHost: names=", ".join(self.get_names()), https="Yes" if self.ssl else "No")) - def __eq__(self, other): if isinstance(other, self.__class__): return (self.filep == other.filep and self.path == other.path and