[Commits] (grant) Fix Bug 2520: Parcel XML copies change order of items in ref collection

commits at osafoundation.org commits at osafoundation.org
Mon Apr 25 14:54:47 PDT 2005


Commit by: grant
Modified files:
chandler/application/Parcel.py 1.55 1.56
chandler/application/tests/TestCopying.py 1.4 1.5
chandler/application/tests/testparcels/copying/data/parcel.xml 1.3 1.4

Log message:
Fix Bug 2520: Parcel XML copies change order of items in ref collection

- Add a case to the TestCopying.py (and its associated parcel) to check
  that sibling items aren't reordered when one of them is a copy.

- In Parcel.py, delay post-copy assignments for an item to the end of
  parcel loading.



Bugzilla links:
http://bugzilla.osafoundation.org/show_bug.cgi?id=2520

ViewCVS links:
http://cvs.osafoundation.org/index.cgi/chandler/application/Parcel.py.diff?r1=text&tr1=1.55&r2=text&tr2=1.56
http://cvs.osafoundation.org/index.cgi/chandler/application/tests/TestCopying.py.diff?r1=text&tr1=1.4&r2=text&tr2=1.5
http://cvs.osafoundation.org/index.cgi/chandler/application/tests/testparcels/copying/data/parcel.xml.diff?r1=text&tr1=1.3&r2=text&tr2=1.4

Index: chandler/application/Parcel.py
diff -u chandler/application/Parcel.py:1.55 chandler/application/Parcel.py:1.56
--- chandler/application/Parcel.py:1.55	Mon Apr 25 13:56:19 2005
+++ chandler/application/Parcel.py	Mon Apr 25 14:54:45 2005
@@ -569,7 +569,7 @@
             self.log.info("...done")
             
             self.__parcelsWithData = []
-            self.__copyOperations = []
+            self.__delayedOperations = []
 
             if not namespaces and self.__parcelsToLoad:
                 namespaces = self.__parcelsToLoad
@@ -597,35 +597,19 @@
 
             self.__parcelsWithData = None
             
-            for (reference, copyName, item, attributeName, file, line) in self.__copyOperations:
-                # Make sure that any ParcelException we raise here reports
-                # the correct parcel.xml line / file
-                self.saveState(file, line)
-                
-                # We may be reloading, so if the copy is already there,
-                # remove it and re-copy
-                existingCopy = item.findPath(copyName)
-                if existingCopy is not None:
-                    existingCopy.delete(recursive=True)
-        
-                # (either) Copy the item using cloud-copy:
-                copy = reference.copy(name=copyName, parent=item, 
-                 cloudAlias="default")
-                 
-                if copy == None:
-                    explanation = \
-                        ("Unable to make copy named '%s' for attribute '%s'. " + 
-                        "Maybe the original was moved/deleted?") % \
-                        (copyName, attributeName)
+            for (item, file, line, call, arguments, keywords) in self.__delayedOperations:
+                try:
+                    if keywords == None:
+                        call(*arguments)
+                    else:
+                        call(*arguments, **keywords)
+                except Exception, e:
+                    self.saveState(file, line)
+                    explanation = "Unable to perform assignment: %s" % e
                     self.saveExplanation(explanation)
                     raise ParcelException(explanation)
 
-                # (or) Copy the item using attribute-copy:
-                # copy = reference.copy(name=copyName, parent=item)
-        
-                item.addValue(attributeName, copy)
-        
-            self.__copyOperations = None
+            self.__delayedOperations = None
             
             self.resetState()
             self.log.info("Starting parcels...")
@@ -665,9 +649,32 @@
             self.__parcelsWithData.append(handler.namespace)
             
         return result
+        
+    def addDelayedCall(self, item, file, line, call, args, keywords):
+        self.__delayedOperations.append((item, file, line, call, args, keywords))
+
+    def performCopyOperation(self, reference, copyName, item, attributeName):
+        # We may be reloading, so if the copy is already there,
+        # remove it and re-copy
+        existingCopy = item.findPath(copyName)
+        if existingCopy is not None:
+            existingCopy.delete(recursive=True)
+
+        # (either) Copy the item using cloud-copy:
+
+        copy = reference.copy(name=copyName, parent=item, cloudAlias="default")
+
+        # (or) Copy the item using attribute-copy:
+        # copy = reference.copy(name=copyName, parent=item)
+        if copy == None:
+            explanation = \
+                ("Unable to make copy named '%s' for attribute '%s'. " + 
+                "Maybe the original was moved/deleted?") % \
+                (copyName, attributeName)
+            self.saveExplanation(explanation)
+            raise ParcelException(explanation)
 
-    def addCopyOperation(self, reference, copyName, item, attributeName, file, line):
-        self.__copyOperations.append((reference, copyName, item, attributeName, file, line))
+        item.addValue(attributeName, copy)
         
     def resetState(self):
         self.currentXMLFile = None
@@ -1393,8 +1400,12 @@
         """ Perform all the delayed attribute assignments for an item """
 
         (old, new) = ValueSet.getValueSets(item)
+        copiedAnAssignment = False
 
         for assignment in assignments:
+            assignmentCallable = None
+            assignmentArgs = None
+            assignmentKeywords = None
 
             attributeName = str(assignment["attrName"])
             reloading = assignment["reloading"]
@@ -1449,23 +1460,24 @@
 
                     # @@@ Special cases to resolve
                     if copyName:
-                        self.manager.addCopyOperation(reference, copyName,
-                                                      item, attributeName,
-                                                      file, line)
-
+                        copiedAnAssignment = True
+                        assignmentCallable = self.manager.performCopyOperation
+                        assignmentArgs = (reference, copyName, item, attributeName,)
                     elif attributeName == 'inverseAttribute':
-                        item.addValue('otherName', reference.itsName)
+                        assignmentCallable = item.addValue
+                        assignmentArgs = ('otherName', reference.itsName, )
                     elif attributeName == 'displayAttribute':
-                        item.addValue('displayAttribute', reference.itsName)
+                        assignmentCallable = item.addValue
+                        assignmentArgs = ('displayAttribute', reference.itsName, )
                     elif attributeName == 'attributes':
-                        item.addValue('attributes', reference,
-                                      alias=reference.itsName)
+                        assignmentCallable = item.addValue
+                        assignmentArgs = ('attributes', reference, )
+                        assignmentKeywords = { 'alias' : reference.itsName }
                     else:
+                        assignmentCallable = item.addValue
+                        assignmentArgs = (attributeName, reference, )
                         if aliasName:
-                            item.addValue(attributeName, reference,
-                             alias=aliasName)
-                        else:
-                            item.addValue(attributeName, reference)
+                             assignmentKeywords = { 'alias': aliasName }
 
                     if reloading:
                         if reference is None:
@@ -1502,8 +1514,9 @@
                 else:
                     # This assignment doesn't appear in the previous version
                     # of XML, so let's apply it to the item.
-
-                    item.addValue(attributeName, reference.itsUUID)
+                    
+                    assignmentCallable = item.addValue
+                    assignmentArgs = (attributeName, reference.itsUUID, )
 
                     if reloading:
                         print "Reload: item %s, assigning %s = UUID of %s" % \
@@ -1559,27 +1572,37 @@
                     # This assignment doesn't appear in the previous version
                     # of XML, so let's apply it to the item.
 
+                    if key is not None:
+                        assignmentCallable = item.setValue
+                        assignmentArgs = (attributeName, value, key, )
+                        if reloading:
+                            print "Reload: item %s, assigning %s[%s] = " \
+                             "'%s'" % \
+                             (item.itsPath, attributeName, key, value)
+                    else:
+                        assignmentCallable = item.addValue
+                        assignmentArgs = (attributeName, value, )
+                        if reloading:
+                            print "Reload: item %s, assigning %s = '%s'" % \
+                             (item.itsPath, attributeName, value)
+
+                # Record this assignment in the new set of assignments
+                new.addAssignment(assignmentTuple)
+                
+            if assignmentCallable is not None:
+                if copiedAnAssignment:
+                    self.manager.addDelayedCall(item, file, line, assignmentCallable, assignmentArgs, assignmentKeywords)
+                else:
                     try:
-                        if key is not None:
-                            item.setValue(attributeName, value, key)
-                            if reloading:
-                                print "Reload: item %s, assigning %s[%s] = " \
-                                 "'%s'" % \
-                                 (item.itsPath, attributeName, key, value)
+                        if assignmentKeywords is not None:
+                            assignmentCallable(*assignmentArgs, **assignmentKeywords)
                         else:
-                            item.addValue(attributeName, value)
-                            if reloading:
-                                print "Reload: item %s, assigning %s = '%s'" % \
-                                 (item.itsPath, attributeName, value)
-
+                            assignmentCallable(*assignmentArgs)
                     except Exception, e:
                         explanation = "Couldn't add value to item (%s)" % e
                         self.saveExplanation(explanation)
                         raise ParcelException(explanation)
 
-                # Record this assignment in the new set of assignments
-                new.addAssignment(assignmentTuple)
-
             #@@@Temporary testing tool written by Morgen -- DJA
             if timing: tools.timing.end("Attribute assignments")
 

Index: chandler/application/tests/TestCopying.py
diff -u chandler/application/tests/TestCopying.py:1.4 chandler/application/tests/TestCopying.py:1.5
--- chandler/application/tests/TestCopying.py:1.4	Mon Sep 27 11:37:05 2004
+++ chandler/application/tests/TestCopying.py	Mon Apr 25 14:54:45 2005
@@ -1,8 +1,8 @@
 """
 Item copying tests
 """
-__revision__  = "$Revision: 1.4 $"
-__date__      = "$Date: 2004/09/27 18:37:05 $"
+__revision__  = "$Revision: 1.5 $"
+__date__      = "$Date: 2005/04/25 21:54:45 $"
 __copyright__ = "Copyright (c) 2003-2004 Open Source Applications Foundation"
 __license__   = "http://osafoundation.org/Chandler_0.1_license_terms.htm"
 
@@ -10,25 +10,42 @@
 from application.Parcel import PrintItem
 
 class CopyingTestCase(ParcelLoaderTestCase.ParcelLoaderTestCase):
+    COPYING = "http://testparcels.org/copying"
+    DATA = "%s/data" % COPYING
 
-    def testCopying(self):
-
-        COPYING = "http://testparcels.org/copying"
-        DATA = "%s/data" % COPYING
+    def setUp(self):
+        super(CopyingTestCase, self).setUp()
         sys.path.append(os.path.join(self.testdir, 'testparcels'))
         self.manager.path.append(os.path.join(self.testdir, 'testparcels'))
-        self.loadParcels([COPYING, DATA])
+        self.loadParcels([self.COPYING, self.DATA])
+        self.dataParcel = self.manager.lookup(self.DATA)
+
+        
+
+    def testCopying(self):
 
         # PrintItem("//parcels/copying/data", self.rep, recursive=True)
 
-        data = self.manager.lookup(DATA)
-        parent = data.lookup("realParent")
+        parent = self.dataParcel.lookup("realParent")
         self.assert_(parent is not None)
         self.assert_(len(parent.myChildren) == 1)
         child = parent.myChildren.first()
         self.assertEquals(parent, child.myParent.first())
         grandChild = child.myChildren.first()
         self.assertEquals(child, grandChild.myParent.first())
+        
+        
+    def testCopyOrder(self):
+        orderTestItem = self.dataParcel.lookup("copyOrderTest")
+        self.assert_(orderTestItem != None)
+        orderedChildren = orderTestItem.myChildren
+        self.assert_(len(orderedChildren) == 2)
+        self.assertEquals(orderedChildren.first().itsName, "anotherCopiedChild")
+        self.assertEquals(
+                orderTestItem.myChildren.last(),
+                self.dataParcel.lookup("templateChild0")
+        )
+
 
 if __name__ == "__main__":
     unittest.main()

Index: chandler/application/tests/testparcels/copying/data/parcel.xml
diff -u chandler/application/tests/testparcels/copying/data/parcel.xml:1.3 chandler/application/tests/testparcels/copying/data/parcel.xml:1.4
--- chandler/application/tests/testparcels/copying/data/parcel.xml:1.3	Mon Sep 27 11:37:14 2004
+++ chandler/application/tests/testparcels/copying/data/parcel.xml	Mon Apr 25 14:54:45 2005
@@ -1,7 +1,7 @@
 <?xml version="1.0" encoding="iso-8859-1"?>
 
-<!-- $Revision: 1.3 $ -->
-<!-- $Date: 2004/09/27 18:37:14 $ -->
+<!-- $Revision: 1.4 $ -->
+<!-- $Date: 2005/04/25 21:54:45 $ -->
 <!-- Copyright (c) 2003-2004 Open Source Applications Foundation -->
 <!-- License: http://osafoundation.org/Chandler_0.1_license_terms.htm -->
 
@@ -30,12 +30,17 @@
     <cp:TestKind itsName="templateChild2">
     </cp:TestKind>
 
-
     <!-- An item which will make use of the template:  -->
 
     <cp:TestKind itsName="realParent">
         <myChildren itemref="me:templateParent" copy="copiedChild"/>
     </cp:TestKind>
 
+    <!-- An item to test Bug:2520 (order of copied items...) -->
+    <cp:TestKind itsName="copyOrderTest">
+        <myChildren itemref="me:templateChild1" copy="anotherCopiedChild"/>
+        <myChildren itemref="me:templateChild0"/>
+    </cp:TestKind>
+
 
 </Parcel>



More information about the Commits mailing list