Fixes issue #1158 mofcomp remove of test.mof fails

The first error encountered was that in the rollback logic the instance
paths are not set into instances when they are created locally. This is
because the normal compile with the pyebem mof compiler does not force
this.  We cannot do this by forcing the Alias on every instance created
in the mof to be able to remove it so the solution is to create a new
MOFWBEMConnection adaption that set the path into the mof as it is
created as part of the remove.

The second issue found was that the property values for reference
properties in associations where the value is defined by the mof
compiler alias were being inserted into the instance with keybindings
with no keys.

Corrects this issue in mof_compiler.py; incorrect parameter on GetClass
was being passed to recursive GetClass call that could result in
compiler processing classes for key qualifier but the getclass with
IncludeQualifiers=False meant no keys were found. Thus, reference
properties in instances where value defined by alais were
built with empty keybindings.

Added a test to test_mof_compiler.py to assure that we are propagating
key property qualifiers correctly.

Adds a bash script to test mof_compiler

As a temporary measure, this adds a script that executes mof_compiler
multiple time with the same MOF to add the MOF to a defined server,
remove it and do that a second time.

This is a very limited test using a local WBEM Server and is in the
manualtests group.

mof_compiler

@@ -36,7 +36,8 @@ from getpass import getpass
 from pywbem._cliutils import SmartFormatter
 from pywbem import WBEMConnection, Error, CIMError, CIM_ERR_NOT_FOUND, \
     CIM_ERR_INVALID_SUPERCLASS, CIM_ERR_INVALID_PARAMETER
-from pywbem.mof_compiler import MOFWBEMConnection, MOFCompiler
+from pywbem.mof_compiler import MOFWBEMConnection, MOFWBEMConnectionRollback, \
+    MOFCompiler
 from pywbem.cim_http import get_default_ca_cert_paths
 from pywbem import __version__
 from pywbem._nocasedict import NocaseDict
@@ -402,15 +403,17 @@ Example:
 
     if args.remove or args.dry_run:
         # Write classes/instances to local repo
-        conn_mof = MOFWBEMConnection(conn=conn)
+        # The MOFWBEMConnectionRollback connection changes CreateInstance
+        # to set the path in the NewInstance
+        conn_mof = MOFWBEMConnectionRollback(conn=conn)
     else:
-        # sent classes/instance to conn
-        conn_mof = MyMOFWBEMConnection(conn=conn)
+        # This sends classes/instance to conn
+        conn_mof = MOFWBEMConnection(conn=conn, write_remot=True)
+
 
     # Make it simple for this script to use these attributes:
     conn_mof.default_namespace = conn.default_namespace
     conn_mof.url = conn.url
-
     conn = conn_mof
 
     # conn.debug = True

pywbem/mof_compiler.py

@@ -1896,17 +1896,19 @@ def DeleteQualifier(self, *args, **kwargs):
 
 class MOFWBEMConnection(BaseRepositoryConnection):
     """
-    A CIM repository connection to an in-memory repository on top of an
-    underlying repository, that is used by the MOF compiler to provide rollback
-    support.
+    A CIM repository connection to either in-memory repository on top of an
+    underlying repository or an external repository defined by the
+    conn constructor parameter.  The constructor parameters determine whether
+    this class gets CIM objects from the in-memory or remote repository and/or
+    writes new CIM objects to the in-memory or remote repository
 
     This class implements the
     :class:`~pywbem.BaseRepositoryConnection` interface.
 
     This adaption does not force paths on instances and just appends
     each new instance to the instance repository
 
-    Also this adapt does not send the results of CreateClass, CreateInstance,
+    Also this adaptation does not send the results of CreateClass, CreateInstance,
     or setQualifier to the defined conn.  It always writes the results
     to the local repository.  Other operations that access data from the
     repository access the repository defined by conn.
@@ -1917,15 +1919,27 @@ class MOFWBEMConnection(BaseRepositoryConnection):
         class :class:`~pywbem.WBEMConnection`.
     """
 
-    def __init__(self, conn=None):
+    def __init__(self, conn=None, write_remote=None):
         """
         Parameters:
 
           conn (BaseRepositoryConnection):
-            The underlying repository connection.
+            The underlying repository connection. This is the source for
+            getting CIM objects when it is defined and the destination for
+            creating CIM objects when it is defined and the write_remote
+            parameter is True
 
             `None` means that there is no underlying repository and all
             operations performed through this object will fail.
+
+          write_remote (:class:`py:bool`):
+            If False, the CreateClass, CreateInstance, and SetQualifier
+            methods write the objects directly to the local in-memory
+            repository.
+
+            If True and conn is defined, CreateClass, CreateInstance, and
+            SetQualifier write to the implementation defined by conn.
+)
         """
 
         self.conn = conn
@@ -1935,6 +1949,7 @@ def __init__(self, conn=None):
         self.instances = {}
         self.classes = {}
         self.compile_ordered_classnames = []
+        self.write_remote = write_remote
         if conn is None:
             # This attribute is used only to make get/set
             # of 'default_namespace' behave as it should, in the case
@@ -2021,6 +2036,10 @@ def CreateInstance(self, *args, **kwargs):
         """
 
         inst = args[0] if args else kwargs['NewInstance']
+
+        if self.write_remote and self.conn:
+            rtn_path = self.conn.CreateInstance(*args, **kwargs)
+            return rtn_path
         try:
             self.instances[self.default_namespace].append(inst)
         except KeyError:  # default_namespace does not exist. Create it
@@ -2050,6 +2069,7 @@ def GetClass(self, *args, **kwargs):
             if self.conn is None:
                 ce = CIMError(CIM_ERR_NOT_FOUND, cname)
                 raise ce
+            # Otherwise get the class from conn and insert into self.classes
             cc = self.conn.GetClass(*args, **kwargs)
             try:
                 self.classes[self.default_namespace][cc.classname] = cc
@@ -2094,8 +2114,11 @@ def CreateClass(self, *args, **kwargs):
         cc = args[0] if args else kwargs['NewClass']
         if cc.superclass:
             try:
-                _ = self.GetClass(cc.superclass, LocalOnly=True,  # noqa: F841
-                                  IncludeQualifiers=False)
+                # Since this may cause recursive GetClass calls
+                # IncludeQualifiers = True to insure reference properties on
+                # instances with aliases get built correctly.
+                self.GetClass(cc.superclass, LocalOnly=True,
+                              IncludeQualifiers=True)
             except CIMError as ce:
                 if ce.status_code == CIM_ERR_NOT_FOUND:
                     ce.args = (CIM_ERR_INVALID_SUPERCLASS, cc.superclass)
@@ -2106,14 +2129,14 @@ def CreateClass(self, *args, **kwargs):
         # again at the end. Note that it executes tests on the class
         # before again inserting it into the repo and these can cause
         # exceptions.
-        try:
-            self.compile_ordered_classnames.append(cc.classname)
-
-            # The following generates an exception for each new ns
-            self.classes[self.default_namespace][cc.classname] = cc
-        except KeyError:
-            self.classes[self.default_namespace] = \
-                NocaseDict({cc.classname: cc})
+        self.compile_ordered_classnames.append(cc.classname)
+        if not self.conn:
+            try:
+                # The following generates an exception for each new ns
+                self.classes[self.default_namespace][cc.classname] = cc
+            except KeyError:
+                self.classes[self.default_namespace] = \
+                    NocaseDict({cc.classname: cc})
 
         objects = list(cc.properties.values())
         for meth in cc.methods.values():
@@ -2156,6 +2179,10 @@ def CreateClass(self, *args, **kwargs):
                                 conn_id=self.conn_id)
                         raise
 
+        if self.write_remote and self.conn:
+            self.conn.CreateClass(cc)
+            return
+
         # Issue #991: CreateClass should reject if the class already exists
         try:
             self.class_names[self.default_namespace].append(cc.classname)
@@ -2207,13 +2234,18 @@ def GetQualifier(self, *args, **kwargs):
 
     def SetQualifier(self, *args, **kwargs):
         """Create or modify a qualifier type in the local repository of this
-        class.
+        class or the implementation defined by conn.
 
         For a description of the parameters, see
         :meth:`pywbem.WBEMConnection.SetQualifier`.
         """
 
         qual = args[0] if args else kwargs['QualifierDeclaration']
+
+        if self.write_remote and self.conn:
+            self.conn.SetQualifier(qual)
+            return
+
         try:
             self.qualifiers[self.default_namespace][qual.name] = qual
         except KeyError:
@@ -2265,6 +2297,49 @@ def rollback(self, verbose=False):
         # Issue #990: Also roll back changes to qualifier declarations
 
 
+class MOFWBEMConnectionRollback(MOFWBEMConnection):
+    """
+    A CIM repository connection to an in-memory repository on top of an
+    underlying repository, adapts MOFWBEMConnection by modifying
+    CreateInstance to set CIMInstanceName on each created instance from
+    the properties in the instance. This is required for rollback
+    because the rollback uses the instance path to tell the server
+    delete each instance.
+
+    This class implements the
+    :class:`~pywbem.BaseRepositoryConnection` interface.
+
+    Raises:
+
+      : The methods of this class may raise any exceptions described for
+        class :class:`~pywbem.WBEMConnection`.
+    """
+    def CreateInstance(self, *args, **kwargs):
+        """
+        Extend CreateInstance to set instancePath on the instance so that
+        there is a path to be used in rollback.
+
+        For a description of the parameters, see
+        :meth:`pywbem.WBEMConnection.CreateInstance`.
+        """
+
+        inst = args[0] if args else kwargs['NewInstance']
+
+        # If the path or keybindings do not exist, create the path from
+        # the class and instance and set it into NewInstance
+        if not inst.path or not inst.path.keybindings:
+            cls = self.GetClass(inst.classname,
+                                LocalOnly=False,
+                                IncludeQualifiers=True)
+            inst.path = CIMInstanceName.from_instance(cls, inst)
+        try:
+            self.instances[self.default_namespace].append(inst)
+        except KeyError:  # default_namespace does not exist. Create it
+            self.instances[self.default_namespace] = [inst]
+
+        return inst.path
+
+
 def _print_logger(msg):
     """Print the `msg` parameter to stdout."""
     print(msg)
@@ -2461,6 +2536,8 @@ def compile_string(self, mof, ns, filename=None):
 
         except CIMError as ce:
             if hasattr(ce, 'file_line'):
+                # pylint: disable=no-member
+                # file_line, attribute dynamically added by error code
                 self.parser.log(
                     _format("Fatal Error: {0}:{1}",
                             ce.file_line[0], ce.file_line[1]))

tests/manualtest/run_mof_compiler_script.sh

@@ -0,0 +1,24 @@
+# Test mof_compiler creation and removal of a collection of MOF in
+# a single file
+# This is a very limited simplistic test.  It simply runs the script
+# mof_compiler twice, once to build the test mof into defined server and
+# a second time to remove it.  The only validation is that the scripts
+# run without error.
+# Further, it assumes that the WBEMServer is running, uses http, and is
+# at localhost.. It does this twice to confirm that everything was removed
+# on the first remove.
+
+DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )"
+echo DIR = $DIR
+PYWBEM_ROOT_DIR=$DIR/../../
+
+MOF_FILE=$PYWBEM_ROOT_DIR/tests/unittest/pywbem/test.mof
+SCHEMA=$PYWBEM_ROOT_DIR/tests/schema/mofFinal2.51.0/
+SERVER_URL=http://localhost
+VERBOSE=""
+# TO get verbose output, set VERBOSE="-v"
+
+mof_compiler -s $SERVER_URL $VERBOSE -I $SCHEMA $MOF_FILE
+mof_compiler -s $SERVER_URL $VERBOSE -r -I $SCHEMA $MOF_FILE
+mof_compiler -s $SERVER_URL $VERBOSE -I $SCHEMA $MOF_FILE
+mof_compiler -s $SERVER_URL $VERBOSE -r -I $SCHEMA $MOF_FILE

tests/unittest/pywbem/test.mof

@@ -37,17 +37,17 @@ instance of PyWBEM_PersonCollection as $Collection {
     InstanceID = "PersonCollection";
 };
 
-instance of PyWBEM_MemberOfPersonCollection {
+instance of PyWBEM_MemberOfPersonCollection as $MemOf1 {
     Collection = $Collection;
     Member = $Alice;
 };
 
-instance of PyWBEM_MemberOfPersonCollection {
+instance of PyWBEM_MemberOfPersonCollection as $MemOf2 {
     Collection = $Collection;
     Member = $Bob;
 };
 
-instance of PyWBEM_MemberOfPersonCollection {
+instance of PyWBEM_MemberOfPersonCollection as $MemOf3 {
     Collection = $Collection;
     Member = $Charlie;
 };
@@ -160,11 +160,8 @@ instance of PyWBEM_AllTypes {
    arraySint32 = {0, -9999};
    arrayUint64 = {0, 99999};
    arraySint64 = {-99999, 0, 99999};
-   arrayReal32 = {0, 1.9};
-   arrayReal64 = {0, 1.9};
+   arrayReal32 = {1.1, 1.9};
+   arrayReal64 = {1.2345, 1.9};
    arrayString = {"This is a test string", "Second String"};
    arrayDateTime = {"19991224120000.000000+360", "19991224120000.000000+360"};
 };
-
-
-

tests/unittest/pywbem/test_mof_compiler.py

@@ -338,6 +338,12 @@ def test_testmof(self):
             LocalOnly=False, IncludeQualifiers=True)
         self.assertEqual(ccs.properties['Member'].type, 'reference')
 
+        ccs = self.mofcomp.handle.GetClass(
+            'PyWBEM_PersonCollection',
+            LocalOnly=False, IncludeQualifiers=True)
+        self.assertEqual(ccs.properties['InstanceID'].type, 'string')
+        self.assertTrue('Key' in ccs.properties['InstanceID'].qualifiers)
+
         # get the instances
         insts = self.mofcomp.handle.instances[NAME_SPACE]