From da714e9f8435b71180ad6aed17167247f91ef0ce Mon Sep 17 00:00:00 2001 From: kschopmeyer Date: Thu, 5 Mar 2020 13:46:34 -0600 Subject: [PATCH] Refactor tree display code Refactor the code in the _displaytree.py module to make it more comprehensible, faster, and correct an outstanding issue where we were not always displaying the tree in the same order. This also adds tests for boundary conditions. This pr does not have a defined issue since it does not change external behavior. --- docs/changes.rst | 4 + pywbemtools/pywbemcli/_cmd_class.py | 110 +++++++++++++++++--------- pywbemtools/pywbemcli/_displaytree.py | 62 +++++++++------ tests/unit/cli_test_extensions.py | 4 - tests/unit/test_class_cmds.py | 15 ++++ 5 files changed, 130 insertions(+), 65 deletions(-) diff --git a/docs/changes.rst b/docs/changes.rst index 1cbcf9c67..1fc0697e5 100644 --- a/docs/changes.rst +++ b/docs/changes.rst @@ -63,6 +63,10 @@ Released: not yet * Adds command to test connection for existence of the pull operations (connection test-pull) +* Refactored display_class_tree() and other functions in _displaytree.py and + _cmd_class.py cmd_class_tree function to eliminate boundary conditions, and + clarify code. + **Known issues:** * See `list of open issues`_. diff --git a/pywbemtools/pywbemcli/_cmd_class.py b/pywbemtools/pywbemcli/_cmd_class.py index 6bba39218..d38a08646 100644 --- a/pywbemtools/pywbemcli/_cmd_class.py +++ b/pywbemtools/pywbemcli/_cmd_class.py @@ -823,57 +823,89 @@ def cmd_class_find(context, classname_glob, options): raise_pywbem_error_exception(er) -def cmd_class_tree(context, classname, options): +def get_class_hierarchy(conn, classname, namespace, superclasses=None): """ - Execute the command to enumerate classes from the top or starting at the - classname argument. Then format the results to be displayed as a - left-justified tree using the asciitree library. - The --superclasses option determines if the superclass tree or the - subclass tree is displayed. + Get the class hierarchy from the server, either the superclasses + associated or subclasseswith classname . If getting subclasses + the classname parameter may be None which requests that the complete + class hiearchy be retrieved. + + Parameters: + conn (:class:`~pywbem.WBEMConnection` or subclass): + Current connection to a WBEM server. + + classname(:term:`string): + classname if the tree is to be initiated from + within the class hiearchy. May be None + + namespace(:term:`string): + Namespace to use to acquire classes from WBEM server + + superclasses (:class:`py:bool`): + If `True` display the superclasses of classname. If not True + display the subclasses. The default is None (display subclasses). + + Returns: + tuple of classes, classnames where + classes (list of :class:`~pywbem.CIMClass`): that are either the + superclasses or the subclasses of classname. + + Raises: + CIM_Error: """ - # TODO FUTURE: Sort out how we handle output format with tree output. try: - if options['superclasses']: + if superclasses: if classname is None: raise click.ClickException('CLASSNAME argument required for ' '--superclasses option') - # Get the superclasses into a list - class_ = context.conn.GetClass(classname, - namespace=options['namespace']) - - # Include target class in display in list - classes = [class_] - # Get all superclasses to class_ - while class_.superclass: - class_ = context.conn.GetClass(class_.superclass, - namespace=options['namespace']) - classes.append(class_) - - # classname not used when displaying superclasses. - # display_class_tree sets it to root - classname = None - + # get the superclasses into a list + klass = conn.GetClass(classname, namespace=namespace) + classes = [] + classes.append(klass) + while klass.superclass: + klass = conn.GetClass(klass.superclass, namespace=namespace) + classes.append(klass) else: - # Get the subclass hierarchy either complete or starting at the - # optional CLASSNAME. NOTE: We do not include target_classname - # in lists of classes sent to display_class_tree. That function - # attaches it. - classes = context.conn.EnumerateClasses( - ClassName=classname, - namespace=options['namespace'], - DeepInheritance=True) - - # Get correct case sensitive classname for target class if - # it exists. Simplifies display_class_tree - if classname: - tclass = context.conn.GetClass(classname, - namespace=options['namespace']) - classname = tclass.classname + # get the subclass hierarchy either complete or starting at the + # optional CLASSNAME. Gets minimum data from server to define + # class, superclass data + classes = conn.EnumerateClasses(ClassName=classname, + namespace=namespace, + LocalOnly=True, + IncludeQualifiers=False, + DeepInheritance=True) + except Error as er: raise_pywbem_error_exception(er) + return classes + + +def cmd_class_tree(context, classname, options): + """ + Execute the command to display graphical tree of either superclasses or + subclasses of classname. If classname is None, display tree starting from + the class hierarchy root. + Tree is displayed on the console as a left-justified tree using the + asciitree library. + The --superclasses option determines if the superclass tree or the + subclass tree is displayed. + """ + superclasses = options['superclasses'] + if superclasses and classname is None: + raise click.ClickException('CLASSNAME argument required for ' + '--superclasses option') + + classes = get_class_hierarchy(context.conn, classname, options['namespace'], + superclasses) + + # If showing superclasses, set classname to None for the display + # to add the 'root' as top class. + if superclasses: + classname = None + # Display the list of classes as a tree. The classname is the top # of the tree. context.spinner_stop() diff --git a/pywbemtools/pywbemcli/_displaytree.py b/pywbemtools/pywbemcli/_displaytree.py index d57884e13..72ae213b4 100644 --- a/pywbemtools/pywbemcli/_displaytree.py +++ b/pywbemtools/pywbemcli/_displaytree.py @@ -14,8 +14,8 @@ # See the License for the specific language governing permissions and # limitations under the License. """ -Click Command definition for the qualifer command group which includes -cmds for get and enumerate for CIM qualifier types. +Functions to build a tree dictionary and also a displayable tree in ascii +from a list of CIM classes. """ from __future__ import absolute_import, print_function @@ -42,7 +42,7 @@ def _tree_node(class_to_subclass_dict, cln): Build dictionary of the class/subclass relationships for class cn in dictionary of class_subclass names. This maps the input dictionary that is a flat class_name: [subclass_name] dictionary to a tree - dictionary for the hiearchy of classes. + of dictionaries for the hiearchy of classes. Returns dictionary of dictionaries in form suitable for asciitree @@ -50,6 +50,13 @@ def _tree_node(class_to_subclass_dict, cln): class_to_subclass_dict: Dictionary with all classes to be in as keys and the corresponding subclasses for each class as a list of classnames + + cln (:term:`string`): + Class for which a tree node will be generated. An empty node + is generated if class not in class_to_subclass_dict. + + Returns: + Structure of nested dictionaries defining the class/subclass structure """ node_dict = odicti() # If there is no subclass, the class will not exist in this dictionary @@ -59,46 +66,43 @@ def _tree_node(class_to_subclass_dict, cln): if cln_list: for key in cln_list: node_dict[key] = _tree_node(class_to_subclass_dict, key) - else: - node_dict = odicti() - else: - return odicti() - return node_dict rtn_dict = odicti() # _tree_node generates dictionary node for elements in class-subclass - # dictionary and returns complete node structure + # dictionary and returns complete node structure. This is recursive, + # with _tree_node recursively calling until there are no subclasses. rtn_dict[top_class] = _tree_node(class_subclass_dict, top_class) return rtn_dict -def display_class_tree(classes, top_class=None): +def build_class_tree_dict(classes, top_class=None): """ - Display the list of classes as a left justified tree in ascii to the - click.echo output. - + Build a hierarchical dictionary of classes (each dictionary entry + defines a single classname: list of subclasses Parameters: - classes (list of :class:`~pywbem.CIMClass`): - This is the classes that will be included in the display - including the top_class. If the top_class is None an - artificial top_class named 'root' will be added by this - function. + classes (list of :class:`~pywbem.CIMClass`) + List of classes to be included in the tree including at least + the classname and superclass name. All other parameters are + ignored - top_class (:term:`string`): - The top level class to display or None. + top_class (:term: `string`) + The top level class to display or None if the display is + from root. In that case, the tree builder attaches a top node + named "root" """ # Build dictionary of classname : superclassname from list of CIM classes cln_to_supercln = {cln.classname: cln.superclass for cln in classes} + # Sort so there is a fixed order to the resulting tree. cln_supercln_sorted = odicti() for key in sorted(cln_to_supercln.keys()): cln_supercln_sorted[key] = cln_to_supercln[key] cln_to_supercln = cln_supercln_sorted - # if top_class is none, create artifical root + # If top_class is none, create artifical root if top_class is None: for cln in cln_to_supercln: if not cln_to_supercln[cln]: @@ -113,7 +117,21 @@ def display_class_tree(classes, top_class=None): [subcln_in_cln.setdefault(v, []).append(k) for (k, v) in six.iteritems(cln_to_supercln)] # noqa: F841 - tree = build_tree(subcln_in_cln, top_class) + return build_tree(subcln_in_cln, top_class) + +def display_class_tree(classes, top_class=None): + """ + Build and display in ASCII output a tree of classes. + + Parameters: + classes (list of :class:`~pywbem.CIMClass`) + + top_class (:term: `string`) + The top level class to display or None if the display is + from root. + + """ + tree = build_class_tree_dict(classes, top_class=top_class) tr = LeftAligned() click.echo(tr(tree)) diff --git a/tests/unit/cli_test_extensions.py b/tests/unit/cli_test_extensions.py index 09d23f7d7..650932e56 100644 --- a/tests/unit/cli_test_extensions.py +++ b/tests/unit/cli_test_extensions.py @@ -32,10 +32,6 @@ # create it for the tests depending on pywbem version FAKEURL_STR = '//FakedUrl:5988' if PYWBEM_1 else '//FakedUrl' -# TODO remove this. Temp flag for condition that allows skipping tests that -# are failing with pywbem 1.0.0 -ISSUE_100 = False - class CLITestsBase(object): # pylint: disable=too-few-public-methods, useless-object-inheritance diff --git a/tests/unit/test_class_cmds.py b/tests/unit/test_class_cmds.py index f730f27c7..7536c00a4 100644 --- a/tests/unit/test_class_cmds.py +++ b/tests/unit/test_class_cmds.py @@ -1114,6 +1114,13 @@ class CIM_Foo_sub_sub : CIM_Foo_sub { ['tree', 'CIM_Foo_sub'], {'stdout': """CIM_Foo_sub +-- CIM_Foo_sub_sub +""", + 'test': 'innows'}, + SIMPLE_MOCK_FILE, OK], + + ['Verify class command tree top down starting at leaf class', + ['tree', 'CIM_Foo_sub'], + {'stdout': """CIM_Foo_sub_sub """, 'test': 'innows'}, SIMPLE_MOCK_FILE, OK], @@ -1128,6 +1135,14 @@ class CIM_Foo_sub_sub : CIM_Foo_sub { 'test': 'innows'}, SIMPLE_MOCK_FILE, OK], + ['Verify class command tree -s from top class', + ['tree', '-s', 'CIM_Foo'], + {'stdout': """root + +-- CIM_Foo +""", + 'test': 'innows'}, + SIMPLE_MOCK_FILE, OK], + ['Verify class command tree bottom up. --superclasses', ['tree', '--superclasses', 'CIM_Foo_sub_sub'], {'stdout': """root