Fixes #1158: mofcomp rollback of test.mof with mof_compiler fails

Fixes issue in mof_compiler where the remove option fails to remove
mof created by an earlier compile of the same file. This was
primarily an issue of getting the instance paths correct on the
CreateInstance.

Adds a test for rollbacik to test_mofcompiler.py based on using the
mocker and test.mof

Adds a second manual test against a server at http://localhost
that executes mof_compiler to create and rollback multiple times
in two namespaces.

Adds documentation on the rollback limitations to the rollback
method documentation.

docs/changes.rst

@@ -73,6 +73,12 @@ Released: not yet
   the new classes and instances to the remote repository defined with the -s
   parameter. (see issue #1956 )
 
+* Fixed issue with mof_compiler and mof rollback where instances were
+  not removed when rollback was executed.  This was caused by MOFWBEMConnection
+  code that did not put correct paths on the instances when they were
+  inserted into the local repository so the rollback delete of the instances
+  could not identify the instances. (see issue #1158)
+
 **Enhancements:**
 
 * Removed the use of the 'pbr' package because it caused too many undesirable

pywbem/mof_compiler.py

@@ -1915,7 +1915,7 @@ class MOFWBEMConnection(BaseRepositoryConnection):
     This class implements the
     :class:`~pywbem.BaseRepositoryConnection` interface.
 
-    This implementation sets the path component of instances including
+    This implementation sets the path component of instances created including
     keybindings using property values in the instance. It does NOT confirm that
     all key properties are included in the path.
 
@@ -2029,6 +2029,16 @@ def CreateInstance(self, *args, **kwargs):
         """
 
         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, namespace=self.default_namespace)
+
         try:
             self.instances[self.default_namespace].append(inst)
         except KeyError:  # default_namespace does not exist. Create it
@@ -2116,8 +2126,8 @@ def CreateClass(self, *args, **kwargs):
         self.compile_ordered_classnames.append(cc.classname)
 
         # Class created in local repo before tests because that allows
-        # tests that may actually include this class to succeed in succeeding
-        # code.
+        # tests that may actually include this class to succeed in
+        # the test code below.
         this_ns = self.default_namespace
         try:
             # The following generates an exception for each new ns
@@ -2251,8 +2261,19 @@ def rollback(self, verbose=False):
         Remove classes and instances from the underlying repository, that have
         been created in the local repository of this class.
 
-        Limitation: At this point, only classes and instances will be removed,
+        Limitations:
+
+        1. At this point, only classes and instances will be removed,
         but not qualifiers.
+
+        2. This may not work with instances created by other means, ex.
+        other mof_compilers because of amgiguity in the definition of
+        instance names for associations (ex. The namespace component
+        in reference properties)
+
+        3. This only removes the classes specifically defined in the compiled
+        mof and not any classes installed as prerquisits as part of the
+        compile (ex. superclasses, classes defined by references)
         """
         for ns, insts in self.instances.items():
             insts.reverse()

tests/manualtest/run_mof_compiler_script.sh

@@ -0,0 +1,30 @@
+# 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
+# To get verbose output, set VERBOSE=-v
+VERBOSE=""
+NAMESPACE=root/SampleProvider
+
+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
+
+mof_compiler -s $SERVER_URL $VERBOSE -I $SCHEMA $MOF_FILE -n $NAMESPACE
+mof_compiler -s $SERVER_URL $VERBOSE -r -I $SCHEMA $MOF_FILE -n $NAMESPACE
+mof_compiler -s $SERVER_URL $VERBOSE -I $SCHEMA $MOF_FILE -n $NAMESPACE
+mof_compiler -s $SERVER_URL $VERBOSE -r -I $SCHEMA $MOF_FILE -n $NAMESPACE

tests/unittest/pywbem/test_mof_compiler.py

@@ -308,6 +308,75 @@ def test_invalid_flavor2(self):
             pass
 
 
+class TestRemove(MOFTest):
+    """Test rollback using test.mof"""
+
+    def test_remove(self):
+        """
+        Test create mof and rollback using mocker as remote repository
+        """
+
+        def moflog(msg):
+            """Display message to moflog"""
+            print(msg, file=self.logfile)
+
+        skip_if_moftab_regenerated()
+
+        # compile the mof. The DMTF schema mof is installed by the setup
+
+        ns = 'root/test'
+
+        conn = FakedWBEMConnection(default_namespace=ns)
+
+        # compile the file against the mocker
+        moflog_file = os.path.join(TEST_DIR, 'moflog.txt')
+        self.logfile = open(moflog_file, 'w')
+
+        # Create compiler using FakedWBEMConnection as conn
+        # This compiler writes new classes and instances to conn
+        self.mofcomp = MOFCompiler(
+            conn,
+            search_paths=[TEST_DMTF_CIMSCHEMA_MOF_DIR],
+            verbose=False,
+            log_func=moflog)
+
+        # Classes in the test class file
+        clns = ['PyWBEM_AllTypes', 'PyWBEM_MemberOfPersonCollection',
+                'PyWBEM_PersonCollection', 'PyWBEM_Person']
+
+        # Compile the file to the conn destination
+        self.mofcomp.compile_file(
+            os.path.join(TEST_DIR, 'test.mof'), ns)
+
+        for cln in clns:
+            conn.GetClass(cln)
+            assert conn.EnumerateInstances(cln) is not None
+
+        # Remove the mof defined by the testfile
+        # create rollback definition
+        conn_mof = MOFWBEMConnection(conn=conn)
+        conn_mof.default_namespace = conn.default_namespace
+        conn_mof.url = conn.url
+
+        conn_mof_mofcomp = MOFCompiler(
+            conn_mof,
+            search_paths=[TEST_DMTF_CIMSCHEMA_MOF_DIR],
+            verbose=False,
+            log_func=moflog)
+
+        # recompile for to MOFWBEMConnection for remove
+        conn_mof_mofcomp.compile_file(
+            os.path.join(TEST_DIR, 'test.mof'), ns)
+
+        conn_mof_mofcomp.rollback(verbose=False)
+
+        for cln in clns:
+            try:
+                conn.GetClass(cln)
+            except CIMError as ce:
+                assert ce.status_code == CIM_ERR_NOT_FOUND
+
+
 class TestAliases(MOFTest):
     """Test of a mof file that contains aliases"""