Fixed #650: Sorted and added missing properties in table output of in…

…stances

Details:

* When the set of instances to be displayed in a table format had different
  sets of properties, there were cases when not all properties were
  displayed. The algorithm for determining the set of properties to be
  displayed simply determined the instance with the largest number of
  properties and used their names. This change fixes that by determining
  the set of properties to be displayed by adding all differently named
  properties in all instances into the set to be displayed.

* In addition, this change sorts the determined properties such that
  first the key properties are displayed in sorted order, and then
  the non-key properties in sorted order. Any property name comparisons
  and the sorting are done case insenitively. The key properties are
  determined fro the path of the instances, without causing any additional
  class retrieval operations.

* Added testcases for the missing property case.

* Corrected comments on the testcase definition structures, and made
  the function and structure names more consistent, for
  _format_instances_as_rows() and _print_instances_as_table().

Signed-off-by: Andreas Maier <andreas.r.maier@gmx.de>

docs/changes.rst

@@ -41,6 +41,8 @@ Released: not yet
   outputs the compile error message and exception to stderr whereas before
   the compile error text was routed to stdout. (See issue #637)
 
+* Fixed an issue where displaying instances in a table format missed properties
+  if the list of instances had different sets of properties. (See issue #650)
 
 **Enhancements:**
 
@@ -62,6 +64,10 @@ Released: not yet
   the output of instances in table format now includes the value of the Values
   qualifier in parenthesis, in addition to the integer value. (See issue #634)
 
+* The order of properties when displaying instances in a table format is now
+  predictable: First the sorted key properties, then the sorted non-key
+  properties. (Part of fix for issue #650)
+
 **Cleanup**
 
 * Adds command to test connection for existence of  the pull operations

pywbemtools/pywbemcli/_common.py

@@ -29,6 +29,7 @@
 except ImportError:
     from ordereddict import OrderedDict  # pylint: disable=import-error
 
+from pydicti import odicti
 import six
 import click
 import tabulate
@@ -1404,12 +1405,19 @@ def _print_qual_decls_as_table(qual_decls, table_width, table_format):
 
 
 def _format_instances_as_rows(insts, max_cell_width=DEFAULT_MAX_CELL_WIDTH,
-                              include_classes=False, context=None):
+                              include_classes=False, context=None,
+                              prop_names=None):
     """
     Format the list of instances properties into as a list of the property
     values for each instance( a row of the table) gathered into a list of
     the rows.
 
+    The prop_names parameter is the list of (originally cased) property names
+    to be output, in the desired output order. It could be determined from
+    the instances, but since it is determined already by the caller, it
+    is passed in as an optimization. For test convenience, None is permitted
+    and causes the properties to again be determined from the instances.
+
     Include_classes for each instance if True. Sets the classname as the first
     column.
 
@@ -1432,18 +1440,9 @@ def _format_instances_as_rows(insts, max_cell_width=DEFAULT_MAX_CELL_WIDTH,
     if max_cell_width is None:
         max_cell_width = DEFAULT_MAX_CELL_WIDTH
     lines = []
-    prop_names = []
 
-    # find instance with max number of properties
-
-    # TODO: The following code misses properties if there are instances with
-    # different sets of properties where no instance has all of them.
-    # See issue #650.
-
-    for inst in insts:
-        pn = inst.keys()
-        if len(pn) > len(prop_names):
-            prop_names = pn
+    if prop_names is None:
+        prop_names = sorted_prop_names(insts)
 
     # Cache of ValueMapping objects for integer-typed properties.
     # Key: classname.propertyname, both in lower case.
@@ -1523,13 +1522,7 @@ def _print_instances_as_table(insts, table_width, table_format,
     if table_width is None:
         table_width = DEFAULT_TABLE_WIDTH
 
-    # Find instance with max number of prop names to determine number
-    # of columns
-    prop_names = []
-    for inst in insts:
-        pn = inst.keys()
-        if len(pn) > len(prop_names):
-            prop_names = pn
+    prop_names = sorted_prop_names(insts)
 
     # Try to estimate max cell width from number of cols
     # This allows folding long data.  However it is incomplete in
@@ -1561,13 +1554,49 @@ def _print_instances_as_table(insts, table_width, table_format,
 
     rows = _format_instances_as_rows(insts, max_cell_width=max_cell_width,
                                      include_classes=include_classes,
-                                     context=context)
+                                     context=context, prop_names=prop_names)
 
     title = 'Instances: {}'.format(insts[0].classname)
     click.echo(format_table(rows, new_header_line, title=title,
                             table_format=table_format))
 
 
+def sorted_prop_names(insts):
+    """
+    Return the list of (originally cased) property names that is the superset
+    of all properties in the input instances.
+
+    The returned list has the key properties first, followed by the non-key
+    properties. Each group is sorted case insensitively.
+
+    The key properties are determined from the instance paths, if present.
+    The function tolerates it if only some of the instances have a path,
+    and if instances of subclasses have additional keys.
+    """
+
+    all_props = odicti()  # key: org prop name, value: lower cased prop name
+    key_props = odicti()  # key: org prop name, value: lower cased prop name
+    for inst in insts:
+        inst_props = inst.keys()
+        for pn in inst_props:
+            all_props[pn] = pn.lower()
+        if inst.path:
+            key_prop_names = inst.path.keys()
+            for pn in inst_props:
+                if pn in key_prop_names:
+                    key_props[pn] = pn.lower()
+
+    nonkey_props = odicti()  # key: org prop name, value: lower cased prop name
+    for pn in all_props:
+        if pn not in key_props:
+            nonkey_props[pn] = all_props[pn]
+
+    key_prop_list = sorted(key_props.keys(), key=lambda p: p.lower())
+    nonkey_prop_list = sorted(nonkey_props.keys(), key=lambda p: p.lower())
+    key_prop_list.extend(nonkey_prop_list)
+    return key_prop_list
+
+
 def _print_objects_as_table(objects, output_format, context=None):
     """
     Call the method for each type of object to print that object type

tests/unit/simple_assoc_mock_model.mof

@@ -70,41 +70,37 @@ class TST_FamilyCollection {
 
 instance of TST_Person as $Mike {
     name = "Mike";
-    gender = 2;
     likes = {1, 2};
+    gender = 2;
 };
 instance of TST_Person as $Saara {
     name = "Saara";
-    gender = 1;
     likes = {1};
+    gender = 1;
 };
 instance of TST_Person as $Sofi {
     name = "Sofi";
     gender = 1;
-    likes = NULL;
 };
 instance of TST_Person as $Gabi{
     name = "Gabi";
+    likes = {2};
     gender = 1;
-    likes = {};
 };
 
 // Define instances of the TST_PersonSub
 instance of TST_PersonSub as $Mikesub{ name = "Mikesub";
                             gender = 2;
-                            likes = {1, 2};
                             secondProperty = "one" ;
                             counter = 1; };
 
 instance of TST_PersonSub as $Saarasub { name = "Saarasub";
                             gender = 1;
-                            likes = {2};
                             secondProperty = "two" ;
                             counter = 2; };
 
 instance of TST_PersonSub as $Sofisub{ name = "Sofisub";
                             gender = 1;
-                            likes = NULL;
                             secondProperty = "three" ;
                             counter = 3; };