[Apache v2] Modifications to ParserNode interfaces (#7330)

This PR contains the changes requested in initial pre-review comments of #7308

Move properties to class pydocs in interfaces.py
Prefer class ABC register() functionality instead of class inheritance for interface classes
Add apache implementation specific functions to interfaces

* Move class argument definitions to class pydoc

* Add apache specific functionality to the interface

* Bring inheritance back

* Define initialization for different ParserNode classes

* Add parsernode utils to check keyword arguments and document the defaults in pydoc

* Fix pydocs and make BlockNode a child of DirectiveNode

* Refine docs, and remove unused __init__ from BlockNode

* Split parsernode util tests to their own respective file

* Skip cover for dummy calls to super

* Add types to method documentation

* Add documentation for children
This commit is contained in:
Joona Hoikkala
2019-08-30 13:42:18 -07:00
committed by Brad Warren
parent 270754deff
commit af1c66b28f
4 changed files with 360 additions and 161 deletions
+119 -129
View File
@@ -86,6 +86,8 @@ For this reason the internal representation of data should not ignore the case.
import abc
import six
from acme.magic_typing import Optional, Tuple # pylint: disable=unused-import, no-name-in-module
@six.add_metaclass(abc.ABCMeta)
class ParserNode(object):
@@ -110,35 +112,42 @@ class ParserNode(object):
it's responsibility of each ParserNode object itself to store its prepending
whitespaces in order to be able to reconstruct the complete configuration file
as it was when originally read from the disk.
ParserNode objects should have the following attributes:
# Reference to ancestor node, or None if the node is the root node of the
# configuration tree.
ancestor: Optional[ParserNode]
# True if this node has been modified since last save.
dirty: bool
# Filepath of the file where the configuration element for this ParserNode
# object resides. For root node, the value for filepath is the httpd root
# configuration file. Filepath can be None if a configuration directive is
# defined in for example the httpd command line.
filepath: Optional[str]
"""
@property
@abc.abstractmethod
def ancestor(self): # pragma: no cover
def __init__(self, **kwargs):
"""
This property contains a reference to ancestor node, or None if the node
is the root node of the configuration tree.
Initializes the ParserNode instance, and sets the ParserNode specific
instance variables. This is not meant to be used directly, but through
specific classes implementing ParserNode interface.
:returns: The ancestor BlockNode object, or None for root node.
:rtype: ParserNode
:param ancestor: BlockNode ancestor for this CommentNode. Required.
:type ancestor: BlockNode or None
:param filepath: Filesystem path for the file where this CommentNode
does or should exist in the filesystem. Required.
:type filepath: str or None
:param dirty: Boolean flag for denoting if this CommentNode has been
created or changed after the last save. Default: False.
:type dirty: bool
"""
raise NotImplementedError
@property
@abc.abstractmethod
def dirty(self): # pragma: no cover
"""
This property contains a boolean value of the information if this node has
been modified since last save (or after the initial parse).
:returns: True if this node has had changes that have not yet been written
to disk.
:rtype: bool
"""
raise NotImplementedError
@abc.abstractmethod
def save(self, msg):
"""
@@ -154,10 +163,13 @@ class ParserNode(object):
file writes properly, the file specific temporary trees should be extracted
from the full ParserNode tree where necessary when writing to disk.
:param str msg: Message describing the reason for the save.
"""
@six.add_metaclass(abc.ABCMeta)
# Linter rule exclusion done because of https://github.com/PyCQA/pylint/issues/179
@six.add_metaclass(abc.ABCMeta) # pylint: disable=abstract-method
class CommentNode(ParserNode):
"""
CommentNode class is used for representation of comments within the parsed
@@ -167,20 +179,38 @@ class CommentNode(ParserNode):
CommentNode stores its contents in class variable 'comment' and does not
have a specific name.
CommentNode objects should have the following attributes in addition to
the ones described in ParserNode:
# Contains the contents of the comment without the directive notation
# (typically # or /* ... */).
comment: str
"""
@property
@abc.abstractmethod
def comment(self): # pragma: no cover
def __init__(self, **kwargs):
"""
Comment property contains the contents of the comment without the comment
directives (typically # or /* ... */).
Initializes the CommentNode instance and sets its instance variables.
:returns: A string containing the comment
:rtype: str
:param comment: Contents of the comment. Required.
:type comment: str
:param ancestor: BlockNode ancestor for this CommentNode. Required.
:type ancestor: BlockNode or None
:param filepath: Filesystem path for the file where this CommentNode
does or should exist in the filesystem. Required.
:type filepath: str or None
:param dirty: Boolean flag for denoting if this CommentNode has been
created or changed after the last save. Default: False.
:type dirty: bool
"""
super(CommentNode, self).__init__(ancestor=kwargs['ancestor'],
dirty=kwargs.get('dirty', False),
filepath=kwargs['filepath']) # pragma: no cover
raise NotImplementedError
@six.add_metaclass(abc.ABCMeta)
class DirectiveNode(ParserNode):
@@ -189,45 +219,61 @@ class DirectiveNode(ParserNode):
It can have zero or more parameters attached to it. Because of the nature of
single directives, it is not able to have child nodes and hence it is always
treated as a leaf node.
If a this directive was defined on the httpd command line, the ancestor instance
variable for this DirectiveNode should be None, and it should be inserted to the
beginning of root BlockNode children sequence.
DirectiveNode objects should have the following attributes in addition to
the ones described in ParserNode:
# True if this DirectiveNode is enabled and False if it is inside of an
# inactive conditional block.
enabled: bool
# Name, or key of the configuration directive. If BlockNode subclass of
# DirectiveNode is the root configuration node, the name should be None.
name: Optional[str]
# Tuple of parameters of this ParserNode object, excluding whitespaces.
parameters: Tuple[str, ...]
"""
@property
@abc.abstractmethod
def enabled(self): # pragma: no cover
def __init__(self, **kwargs):
"""
Configuration blocks may have conditional statements enabling or disabling
their contents. This property returns the state of this DirectiveNode.
Initializes the DirectiveNode instance and sets its instance variables.
:param name: Name or key of the DirectiveNode object. Required.
:type name: str or None
:param tuple parameters: Tuple of str parameters for this DirectiveNode.
Default: ().
:type parameters: tuple
:param ancestor: BlockNode ancestor for this DirectiveNode, or None for
root configuration node. Required.
:type ancestor: BlockNode or None
:param filepath: Filesystem path for the file where this DirectiveNode
does or should exist in the filesystem, or None for directives introduced
in the httpd command line. Required.
:type filepath: str or None
:param dirty: Boolean flag for denoting if this DirectiveNode has been
created or changed after the last save. Default: False.
:type dirty: bool
:param enabled: True if this DirectiveNode object is parsed in the active
configuration of the httpd. False if the DirectiveNode exists within a
unmatched conditional configuration block. Default: True.
:type enabled: bool
:returns: True if the DirectiveNode is parsed and enabled in the configuration.
:rtype: bool
"""
raise NotImplementedError
@property
@abc.abstractmethod
def name(self): # pragma: no cover
"""
Name property contains the name of the directive.
:returns: Name of this node
:rtype: str
"""
raise NotImplementedError
@property
@abc.abstractmethod
def parameters(self): # pragma: no cover
"""
This property contains a tuple of parameters of this ParserNode object
excluding whitespaces.
:returns: A tuple of parameters for this node
:rtype: tuple
"""
raise NotImplementedError
super(DirectiveNode, self).__init__(ancestor=kwargs['ancestor'],
dirty=kwargs.get('dirty', False),
filepath=kwargs['filepath']) # pragma: no cover
@abc.abstractmethod
def set_parameters(self, parameters):
@@ -242,7 +288,7 @@ class DirectiveNode(ParserNode):
@six.add_metaclass(abc.ABCMeta)
class BlockNode(ParserNode):
class BlockNode(DirectiveNode):
"""
BlockNode class represents a block of nested configuration directives, comments
and other blocks as its children. A BlockNode can have zero or more parameters
@@ -272,6 +318,15 @@ class BlockNode(ParserNode):
The applicable parameters are dependent on the underlying configuration language
and its grammar.
BlockNode objects should have the following attributes in addition to
the ones described in DirectiveNode:
# Tuple of direct children of this BlockNode object. The order of children
# in this tuple retain the order of elements in the parsed configuration
# block.
children: Tuple[ParserNode, ...]
"""
@abc.abstractmethod
@@ -335,33 +390,6 @@ class BlockNode(ParserNode):
"""
@property
@abc.abstractmethod
def children(self): # pragma: no cover
"""
This property contains a list ParserNode objects that are the children
for this node. The order of children is the same as that of the parsed
configuration block.
:returns: A tuple of this block's children
:rtype: tuple
"""
raise NotImplementedError
@property
@abc.abstractmethod
def enabled(self):
"""
Configuration blocks may have conditional statements enabling or disabling
their contents. This property returns the state of this configuration block.
In case of unmatched conditional statement in block, this block itself should
be set enabled while its children should be set disabled.
:returns: True if the BlockNode is parsed and enabled in the configuration.
:rtype: bool
"""
@abc.abstractmethod
def find_blocks(self, name, exclude=True):
"""
@@ -424,44 +452,6 @@ class BlockNode(ParserNode):
of children of the callee.
"""
@property
@abc.abstractmethod
def name(self): # pragma: no cover
"""
Name property contains the name of the block. As an example for config:
<VirtualHost *:80> ... </VirtualHost>
the name would be "VirtualHost".
:returns: Name of this node
:rtype: str
"""
raise NotImplementedError
@property
@abc.abstractmethod
def parameters(self): # pragma: no cover
"""
This property contains a tuple of parameters of this ParserNode object
excluding whitespaces.
:returns: A tuple of parameters for this node
:rtype: tuple
"""
raise NotImplementedError
@abc.abstractmethod
def set_parameters(self, parameters):
"""
Sets the sequence of parameters for this ParserNode object without
whitespaces. While the whitespaces for parameters are discarded when using
this method, the whitespacing preceeding the ParserNode itself should be
kept intact.
:param list parameters: sequence of parameters
"""
@abc.abstractmethod
def unsaved_files(self):
"""
@@ -0,0 +1,85 @@
"""ParserNode utils"""
def validate_kwargs(kwargs, required_names):
"""
Ensures that the kwargs dict has all the expected values. This function modifies
the kwargs dictionary, and hence the returned dictionary should be used instead
in the caller function instead of the original kwargs.
:param dict kwargs: Dictionary of keyword arguments to validate.
:param list required_names: List of required parameter names.
"""
validated_kwargs = dict()
for name in required_names:
try:
validated_kwargs[name] = kwargs.pop(name)
except KeyError:
raise TypeError("Required keyword argument: {} undefined.".format(name))
# Raise exception if unknown key word arguments are found.
if kwargs:
unknown = ", ".join(kwargs.keys())
raise TypeError("Unknown keyword argument(s): {}".format(unknown))
return validated_kwargs
def parsernode_kwargs(kwargs):
"""
Validates keyword arguments for ParserNode. This function modifies the kwargs
dictionary, and hence the returned dictionary should be used instead in the
caller function instead of the original kwargs.
:param dict kwargs: Keyword argument dictionary to validate.
:returns: Tuple of validated and prepared arguments.
"""
kwargs.setdefault("dirty", False)
kwargs = validate_kwargs(kwargs, ["ancestor", "dirty", "filepath"])
return kwargs["ancestor"], kwargs["dirty"], kwargs["filepath"]
def commentnode_kwargs(kwargs):
"""
Validates keyword arguments for CommentNode and sets the default values for
optional kwargs. This function modifies the kwargs dictionary, and hence the
returned dictionary should be used instead in the caller function instead of
the original kwargs.
:param dict kwargs: Keyword argument dictionary to validate.
:returns: Tuple of validated and prepared arguments and ParserNode kwargs.
"""
kwargs.setdefault("dirty", False)
kwargs = validate_kwargs(kwargs, ["ancestor", "dirty", "filepath", "comment"])
comment = kwargs.pop("comment")
return comment, kwargs
def directivenode_kwargs(kwargs):
"""
Validates keyword arguments for DirectiveNode and BlockNode and sets the
default values for optional kwargs. This function modifies the kwargs
dictionary, and hence the returned dictionary should be used instead in the
caller function instead of the original kwargs.
:param dict kwargs: Keyword argument dictionary to validate.
:returns: Tuple of validated and prepared arguments and ParserNode kwargs.
"""
kwargs.setdefault("dirty", False)
kwargs.setdefault("enabled", True)
kwargs.setdefault("parameters", ())
kwargs = validate_kwargs(kwargs, ["ancestor", "dirty", "filepath", "name",
"parameters", "enabled"])
name = kwargs.pop("name")
parameters = kwargs.pop("parameters")
enabled = kwargs.pop("enabled")
return name, parameters, enabled, kwargs
@@ -2,84 +2,121 @@
import unittest
from acme.magic_typing import Optional, Tuple # pylint: disable=unused-import, no-name-in-module
from certbot_apache import interfaces
from certbot_apache import parsernode_util as util
class DummyParserNode(interfaces.ParserNode):
""" A dummy class implementing ParserNode interface """
class DummyCommentNode(interfaces.CommentNode):
def __init__(self, **kwargs):
"""
Initializes the ParserNode instance.
"""
ancestor, dirty, filepath = util.parsernode_kwargs(kwargs)
self.ancestor = ancestor
self.dirty = dirty
self.filepath = filepath
super(DummyParserNode, self).__init__(**kwargs)
def save(self, msg): # pragma: no cover
"""Save"""
pass
class DummyCommentNode(DummyParserNode):
""" A dummy class implementing CommentNode interface """
ancestor = None
comment = ""
dirty = False
def save(self, msg): # pragma: no cover
pass
def __init__(self, **kwargs):
"""
Initializes the CommentNode instance and sets its instance variables.
"""
comment, kwargs = util.commentnode_kwargs(kwargs)
self.comment = comment
super(DummyCommentNode, self).__init__(**kwargs)
class DummyDirectiveNode(interfaces.DirectiveNode):
class DummyDirectiveNode(DummyParserNode):
""" A dummy class implementing DirectiveNode interface """
ancestor = None
parameters = tuple() # type: Tuple[str, ...]
dirty = False
enabled = True
name = ""
def save(self, msg): # pragma: no cover
pass
# pylint: disable=too-many-arguments
def __init__(self, **kwargs):
"""
Initializes the DirectiveNode instance and sets its instance variables.
"""
name, parameters, enabled, kwargs = util.directivenode_kwargs(kwargs)
self.name = name
self.parameters = parameters
self.enabled = enabled
super(DummyDirectiveNode, self).__init__(**kwargs)
def set_parameters(self, parameters): # pragma: no cover
"""Set parameters"""
pass
class DummyBlockNode(interfaces.BlockNode):
class DummyBlockNode(DummyDirectiveNode):
""" A dummy class implementing BlockNode interface """
ancestor = None
parameters = tuple() # type: Tuple[str, ...]
children = tuple() # type: Tuple[interfaces.ParserNode, ...]
dirty = False
enabled = True
name = ""
def save(self, msg): # pragma: no cover
pass
def add_child_block(self, name, parameters=None, position=None): # pragma: no cover
"""Add child block"""
pass
def add_child_directive(self, name, parameters=None, position=None): # pragma: no cover
"""Add child directive"""
pass
def add_child_comment(self, comment="", position=None): # pragma: no cover
"""Add child comment"""
pass
def find_blocks(self, name, exclude=True): # pragma: no cover
"""Find blocks"""
pass
def find_directives(self, name, exclude=True): # pragma: no cover
"""Find directives"""
pass
def find_comments(self, comment, exact=False): # pragma: no cover
"""Find comments"""
pass
def delete_child(self, child): # pragma: no cover
pass
def set_parameters(self, parameters): # pragma: no cover
"""Delete child"""
pass
def unsaved_files(self): # pragma: no cover
"""Unsaved files"""
pass
interfaces.CommentNode.register(DummyCommentNode)
interfaces.DirectiveNode.register(DummyDirectiveNode)
interfaces.BlockNode.register(DummyBlockNode)
class ParserNodeTest(unittest.TestCase):
"""Dummy placeholder test case for ParserNode interfaces"""
def test_dummy(self):
dummyblock = DummyBlockNode()
dummydirective = DummyDirectiveNode()
dummycomment = DummyCommentNode()
dummyblock = DummyBlockNode(
name="None",
parameters=(),
ancestor=None,
dirty=False,
filepath="/some/random/path"
)
dummydirective = DummyDirectiveNode(
name="Name",
ancestor=None,
filepath="/another/path"
)
dummycomment = DummyCommentNode(
comment="Comment",
ancestor=dummyblock,
filepath="/some/file"
)
if __name__ == "__main__":
@@ -0,0 +1,87 @@
""" Tests for ParserNode utils """
import unittest
from certbot_apache import parsernode_util as util
class ParserNodeUtilTest(unittest.TestCase):
"""Tests for ParserNode utils"""
def _setup_parsernode(self):
""" Sets up kwargs dict for ParserNode """
return {
"ancestor": None,
"dirty": False,
"filepath": "/tmp",
}
def _setup_commentnode(self):
""" Sets up kwargs dict for CommentNode """
pn = self._setup_parsernode()
pn["comment"] = "x"
return pn
def _setup_directivenode(self):
""" Sets up kwargs dict for DirectiveNode """
pn = self._setup_parsernode()
pn["name"] = "Name"
pn["parameters"] = ("first",)
pn["enabled"] = True
return pn
def test_unknown_parameter(self):
params = self._setup_parsernode()
params["unknown"] = "unknown"
self.assertRaises(TypeError, util.parsernode_kwargs, params)
params = self._setup_commentnode()
params["unknown"] = "unknown"
self.assertRaises(TypeError, util.commentnode_kwargs, params)
params = self._setup_directivenode()
params["unknown"] = "unknown"
self.assertRaises(TypeError, util.directivenode_kwargs, params)
def test_parsernode(self):
params = self._setup_parsernode()
ctrl = self._setup_parsernode()
ancestor, dirty, filepath = util.parsernode_kwargs(params)
self.assertEqual(ancestor, ctrl["ancestor"])
self.assertEqual(dirty, ctrl["dirty"])
self.assertEqual(filepath, ctrl["filepath"])
def test_commentnode(self):
params = self._setup_commentnode()
ctrl = self._setup_commentnode()
comment, _ = util.commentnode_kwargs(params)
self.assertEqual(comment, ctrl["comment"])
def test_directivenode(self):
params = self._setup_directivenode()
ctrl = self._setup_directivenode()
name, parameters, enabled, _ = util.directivenode_kwargs(params)
self.assertEqual(name, ctrl["name"])
self.assertEqual(parameters, ctrl["parameters"])
self.assertEqual(enabled, ctrl["enabled"])
def test_missing_required(self):
c_params = self._setup_commentnode()
c_params.pop("comment")
self.assertRaises(TypeError, util.commentnode_kwargs, c_params)
d_params = self._setup_directivenode()
d_params.pop("ancestor")
self.assertRaises(TypeError, util.directivenode_kwargs, d_params)
p_params = self._setup_parsernode()
p_params.pop("filepath")
self.assertRaises(TypeError, util.parsernode_kwargs, p_params)
if __name__ == "__main__":
unittest.main() # pragma: no cover