AVMSyncService checkpoint. Compare() substantially works but is only

lightly tested.  This uncovered a bug in LayeredDirectoryNodeImpl of 
relatively ancient etiology. I thought I had a test that would have 
caught it. I was wrong.


git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/alfresco/BRANCHES/WCM-DEV2/root@3796 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261

source/java/org/alfresco/repo/avm/AVMServiceTest.java

@@ -58,6 +58,34 @@ import org.alfresco.service.transaction.TransactionService;
  */
 public class AVMServiceTest extends AVMServiceTestBase
 {
+    /**
+     * Test of Descriptor indirection field.
+     */
+    public void testDescriptorIndirection()
+    {
+        try
+        {
+            setupBasicTree();
+            fService.createLayeredDirectory("main:/a", "main:/", "layer");
+            fService.createFile("main:/layer/b/c", "bambino").close();
+            AVMNodeDescriptor desc = fService.lookup(-1, "main:/layer");
+            assertEquals("main:/a", desc.getIndirection());
+            Map<String, AVMNodeDescriptor> list = fService.getDirectoryListing(-1, "main:/");
+            assertEquals("main:/a", list.get("layer").getIndirection());
+            desc = fService.lookup(-1, "main:/layer/b");
+            assertEquals("main:/a/b", desc.getIndirection());
+            list = fService.getDirectoryListing(-1, "main:/layer");
+            assertEquals("main:/a/b", list.get("b").getIndirection());
+            list = fService.getDirectoryListingDirect(-1, "main:/layer");
+            assertEquals("main:/a/b", list.get("b").getIndirection());
+        }
+        catch (Exception e)
+        {
+            e.printStackTrace(System.err);
+            fail();
+        }
+    }
+    
     /**
      * Test AVMSyncService update.
      */
@@ -71,6 +99,13 @@ public class AVMServiceTest extends AVMServiceTestBase
             fService.createFile("main:/abranch", "monkey").close();
             fService.getFileOutputStream("main:/abranch/b/c/foo").close();
             System.out.println(recursiveList("main", -1, true));
+            List<AVMDifference> cmp =
+                fSyncService.compare(-1, "main:/abranch", -1, "main:/a");
+            for (AVMDifference diff : cmp)
+            {
+                System.out.println(diff);
+            }
+            assertEquals(2, cmp.size());
             List<AVMDifference> diffs = new ArrayList<AVMDifference>();
             diffs.add(new AVMDifference(-1, "main:/abranch/monkey",
                                         -1, "main:/a/monkey",
@@ -88,17 +123,32 @@ public class AVMServiceTest extends AVMServiceTestBase
             // Try updating a deletion.
             fService.removeNode("main:/abranch", "monkey");
             System.out.println(recursiveList("main", -1, true));
+            cmp =
+                fSyncService.compare(-1, "main:/abranch", -1, "main:/a");
+            for (AVMDifference diff : cmp)
+            {
+                System.out.println(diff);
+            }
+            assertEquals(1, cmp.size());
             diffs.clear();
             diffs.add(new AVMDifference(-1, "main:/abranch/monkey",
                                         -1, "main:/a/monkey",
                                         AVMDifference.NEWER));
             fSyncService.update(diffs, false, false, false, false);
+            assertEquals(0, fSyncService.compare(-1, "main:/abranch", -1, "main:/a").size());
             fService.createSnapshot("main");
             System.out.println(recursiveList("main", -1, true));
             assertEquals(fService.lookup(-1, "main:/abranch/monkey", true).getId(),
                          fService.lookup(-1, "main:/a/monkey", true).getId());
             // Try one that should fail.
             fService.createFile("main:/abranch", "monkey").close();
+            cmp =
+                fSyncService.compare(-1, "main:/abranch", -1, "main:/a");
+            for (AVMDifference diff : cmp)
+            {
+                System.out.println(diff);
+            }
+            assertEquals(1, cmp.size());
             diffs.clear();
             diffs.add(new AVMDifference(-1, "main:/a/monkey",
                                         -1, "main:/abranch/monkey",
@@ -119,10 +169,95 @@ public class AVMServiceTest extends AVMServiceTestBase
                       -1, "main:/abranch/monkey",
                       AVMDifference.NEWER));
             fSyncService.update(diffs, false, false, true, false);
+            assertEquals(0, fSyncService.compare(-1, "main:/abranch", -1, "main:/a").size());
             fService.createSnapshot("main");
             System.out.println(recursiveList("main", -1, true));
             assertEquals(fService.lookup(-1, "main:/a/monkey", true).getId(),
                          fService.lookup(-1, "main:/abranch/monkey", true).getId());
+            // Cleanup for layered tests.
+            fService.purgeAVMStore("main");
+            fService.createAVMStore("main");
+            setupBasicTree();
+            fService.createLayeredDirectory("main:/a", "main:/", "layer");
+            fService.createFile("main:/layer", "monkey").close();
+            fService.getFileOutputStream("main:/layer/b/c/foo").close();
+            cmp =
+                fSyncService.compare(-1, "main:/layer", -1, "main:/a");
+            for (AVMDifference diff : cmp)
+            {
+                System.out.println(diff);
+            }
+            assertEquals(2, cmp.size());
+            System.out.println(recursiveList("main", -1, true));
+            diffs.clear();
+            diffs.add(new AVMDifference(-1, "main:/layer/monkey",
+                                        -1, "main:/a/monkey",
+                                        AVMDifference.NEWER));
+            diffs.add(new AVMDifference(-1, "main:/layer/b/c/foo",
+                                        -1, "main:/a/b/c/foo",
+                                        AVMDifference.NEWER));
+            fSyncService.update(diffs, false, false, false, false);
+            assertEquals(0, fSyncService.compare(-1, "main:/layer", -1, "main:/a").size());
+            fService.createSnapshot("main");
+            System.out.println(recursiveList("main", -1, true));
+            assertEquals(fService.lookup(-1, "main:/layer/monkey").getId(),
+                         fService.lookup(-1, "main:/a/monkey").getId());
+            assertEquals(fService.lookup(-1, "main:/layer/b/c/foo").getId(),
+                         fService.lookup(-1, "main:/a/b/c/foo").getId());
+            // Try updating a deletion.
+            fService.removeNode("main:/layer", "monkey");
+            System.out.println(recursiveList("main", -1, true));
+            cmp =
+                fSyncService.compare(-1, "main:/layer", -1, "main:/a");
+            for (AVMDifference diff : cmp)
+            {
+                System.out.println(diff);
+            }
+            assertEquals(1, cmp.size());
+            diffs.clear();
+            diffs.add(new AVMDifference(-1, "main:/layer/monkey",
+                                        -1, "main:/a/monkey",
+                                        AVMDifference.NEWER));
+            fSyncService.update(diffs, false, false, false, false);
+            assertEquals(0, fSyncService.compare(-1, "main:/layer", -1, "main:/a").size());
+            fService.createSnapshot("main");
+            System.out.println(recursiveList("main", -1, true));
+            assertEquals(fService.lookup(-1, "main:/layer/monkey", true).getId(),
+                         fService.lookup(-1, "main:/a/monkey", true).getId());
+            // Try one that should fail.
+            fService.createFile("main:/layer", "monkey").close();
+            cmp =
+                fSyncService.compare(-1, "main:/layer", -1, "main:/a");
+            for (AVMDifference diff : cmp)
+            {
+                System.out.println(diff);
+            }
+            assertEquals(1, cmp.size());
+            diffs.clear();
+            diffs.add(new AVMDifference(-1, "main:/a/monkey",
+                                        -1, "main:/layer/monkey",
+                                        AVMDifference.NEWER));
+            try
+            {
+                fSyncService.update(diffs, false, false, false, false);
+                fail();
+            }
+            catch (AVMSyncException se)
+            {
+                // Do nothing.
+            }
+            // Get synced again by doing an override conflict.
+            System.out.println(recursiveList("main", -1, true));
+            diffs.clear();
+            diffs.add(new AVMDifference(-1, "main:/a/monkey",
+                                        -1, "main:/layer/monkey",
+                                        AVMDifference.NEWER));
+            fSyncService.update(diffs, false, false, true, false);
+            assertEquals(0, fSyncService.compare(-1, "main:/layer", -1, "main:/a").size());
+            fService.createSnapshot("main");
+            System.out.println(recursiveList("main", -1, true));
+            assertEquals(fService.lookup(-1, "main:/a/monkey", true).getId(),
+                         fService.lookup(-1, "main:/layer/monkey", true).getId());
         }
         catch (Exception e)
         {

source/java/org/alfresco/repo/avm/AVMSyncServiceImpl.java

@@ -74,8 +74,182 @@ public class AVMSyncServiceImpl implements AVMSyncService
     public List<AVMDifference> compare(int srcVersion, String srcPath, 
                                        int dstVersion, String dstPath)
     {
-        // TODO Implement.
+        if (srcPath == null || dstPath == null)
-        return new ArrayList<AVMDifference>();
+        {
+            throw new AVMBadArgumentException("Illegal null path.");
+        }
+        List<AVMDifference> result = new ArrayList<AVMDifference>();
+        AVMNodeDescriptor srcDesc = fAVMService.lookup(srcVersion, srcPath, true);
+        if (srcDesc == null)
+        {
+            throw new AVMSyncException("Source not found: " + srcPath);
+        }
+        AVMNodeDescriptor dstDesc = fAVMService.lookup(dstVersion, dstPath, true);
+        if (dstDesc == null)
+        {
+            // Special case: no pre-existing version in the destination.
+            result.add(new AVMDifference(srcVersion, srcPath,
+                                         dstVersion, dstPath,
+                                         AVMDifference.NEWER));
+        }
+        else
+        {
+            // Invoke the recursive implementation.
+            compare(srcVersion, srcDesc, dstVersion, dstDesc, result);
+        }
+        return result;
+    }
+    
+    // TODO We need getDirectlyListingDirect(descriptor, includeDeleted)
+    /**
+     * Internal recursive implementation of compare.
+     * @param srcVersion The version of the source tree.
+     * @param srcDesc The current source descriptor.
+     * @param dstVersion The version of the destination tree.
+     * @param dstDesc The current dstDesc
+     */
+    private void compare(int srcVersion, AVMNodeDescriptor srcDesc,
+                         int dstVersion, AVMNodeDescriptor dstDesc,
+                         List<AVMDifference> result)
+    {
+        // Determine how the source and destination nodes differ.
+        int diffCode = compareOne(srcDesc, dstDesc);
+        switch (diffCode)
+        {
+            case AVMDifference.SAME :
+            {
+                // A big short circuit.
+                return;
+            }
+            // The trivial to handle cases.
+            case AVMDifference.NEWER :
+            case AVMDifference.OLDER :
+            case AVMDifference.CONFLICT :
+            {
+                result.add(new AVMDifference(srcVersion, srcDesc.getPath(),
+                                             dstVersion, dstDesc.getPath(),
+                                             diffCode));
+                return;
+            }
+            case AVMDifference.DIRECTORY :
+            {
+                // First special case: source is a layered directory which points to 
+                // the destinations path, and we are comparing 'head' versions.
+                if (srcDesc.isLayeredDirectory() && 
+                    srcDesc.getIndirection().equals(dstDesc.getPath()) && srcVersion < 0 && dstVersion < 0)
+                {
+                    // Get only a direct listing, since that's all that can be different.
+                    Map<String, AVMNodeDescriptor> srcList =
+                        fAVMService.getDirectoryListingDirect(-1, srcDesc.getPath(), true);
+                    // The biggest shortcut: if the source directory is directly empty
+                    // then we're done.
+                    if (srcList.size() == 0)
+                    {
+                        return;
+                    }
+                    // We grab a complete listing of the destination.
+                    Map<String, AVMNodeDescriptor> dstList =
+                        fAVMService.getDirectoryListing(dstDesc, true);
+                    for (String name : srcList.keySet())
+                    {
+                        AVMNodeDescriptor srcChild = srcList.get(name);
+                        AVMNodeDescriptor dstChild = dstList.get(name);
+                        if (dstChild == null)
+                        {
+                            // A missing destination child means the source is NEWER.
+                            result.add(new AVMDifference(srcVersion, srcChild.getPath(),
+                                    dstVersion, 
+                                    AVMNodeConverter.ExtendAVMPath(dstDesc.getPath(), name),
+                                    AVMDifference.NEWER));
+                            continue;
+                        }
+                        // Otherwise recursively invoke.
+                        compare(srcVersion, srcChild,
+                                dstVersion, dstChild,
+                                result);
+                    }
+                    return;
+                }
+                // Second special case.  Just as above but reversed.
+                if (dstDesc.isLayeredDirectory() &&
+                    dstDesc.getIndirection().equals(srcDesc.getPath()) && srcVersion < 0 && dstVersion < 0)
+                {
+                    // Get direct content of destination.
+                    Map<String, AVMNodeDescriptor> dstList =
+                        fAVMService.getDirectoryListingDirect(dstVersion, dstDesc.getPath(), true);
+                    // Big short circuit.
+                    if (dstList.size() == 0)
+                    {
+                        return;
+                    }
+                    // Get the source listing.
+                    Map<String, AVMNodeDescriptor> srcList =
+                        fAVMService.getDirectoryListing(srcDesc, true);
+                    for (String name : dstList.keySet())
+                    {
+                        AVMNodeDescriptor dstChild = dstList.get(name);
+                        AVMNodeDescriptor srcChild = srcList.get(name);
+                        if (srcChild == null)
+                        {
+                            // Missing means the source is older.
+                            result.add(new AVMDifference(srcVersion, 
+                                                         AVMNodeConverter.ExtendAVMPath(srcDesc.getPath(), name),
+                                                         dstVersion, dstChild.getPath(),
+                                                         AVMDifference.OLDER));
+                            continue;
+                        }
+                        // Otherwise, recursively invoke.
+                        compare(srcVersion, srcChild, 
+                                dstVersion, dstChild,
+                                result);
+                    }
+                    return;
+                }
+                // Neither of the special cases apply, so brute force is the only answer.
+                Map<String, AVMNodeDescriptor> srcList =
+                    fAVMService.getDirectoryListing(srcDesc, true);
+                Map<String, AVMNodeDescriptor> dstList =
+                    fAVMService.getDirectoryListing(dstDesc, true);
+                // Iterate over the source.
+                for (String name : srcList.keySet())
+                {
+                    AVMNodeDescriptor srcChild = srcList.get(name);
+                    AVMNodeDescriptor dstChild = dstList.get(name);
+                    if (dstChild == null)
+                    {
+                        // Not found in the destination means NEWER.
+                        result.add(new AVMDifference(srcVersion, srcChild.getPath(),
+                                                     dstVersion,
+                                                     AVMNodeConverter.ExtendAVMPath(dstDesc.getPath(), name),
+                                                     AVMDifference.NEWER));
+                        continue;
+                    }
+                    // Otherwise recursive invocation.
+                    compare(srcVersion, srcChild,
+                            dstVersion, dstChild,
+                            result);
+                }
+                // Iterate over the destination.
+                for (String name : dstList.keySet())
+                {
+                    if (srcList.containsKey(name))
+                    {
+                        continue;
+                    }
+                    AVMNodeDescriptor dstChild = dstList.get(name);
+                    // An entry not found in the source is OLDER.
+                    result.add(new AVMDifference(srcVersion,
+                                                 AVMNodeConverter.ExtendAVMPath(srcDesc.getPath(), name),
+                                                 dstVersion, dstChild.getPath(),
+                                                 AVMDifference.OLDER));
+                }
+                break;
+            }
+            default :
+            {
+                throw new AVMSyncException("Invalid Difference Code, Internal Error.");
+            }
+        }
     }
     
     /**
@@ -112,22 +286,28 @@ public class AVMSyncServiceImpl implements AVMSyncService
             String [] dstParts = AVMNodeConverter.SplitBase(diff.getDestinationPath());
             if (dstParts[0] == null || diff.getDestinationVersion() >= 0)
             {
+                // You can't have a root node as a destination.
                 throw new AVMSyncException("Invalid destination node: " + diff.getDestinationPath());
             }
             AVMNodeDescriptor dstDesc = fAVMService.lookup(-1, diff.getDestinationPath(), true);
+            // The default is that the source is newer in the case where
+            // the destination doesn't exist.
             int diffCode = AVMDifference.NEWER;
             if (dstDesc != null) 
             {
                 diffCode = compareOne(srcDesc, dstDesc);
             }
+            // Dispatch.
             switch (diffCode)
             {
                 case AVMDifference.SAME :
                 {
+                    // Nada to do.
                     continue;
                 }
                 case AVMDifference.NEWER :
                 {
+                    // You can't delete what isn't there.
                     if (dstDesc != null)
                     {
                         fAVMService.removeNode(dstParts[0], dstParts[1]);
@@ -137,38 +317,46 @@ public class AVMSyncServiceImpl implements AVMSyncService
                 }
                 case AVMDifference.OLDER :
                 {
+                    // You can force it.
                     if (overrideOlder)
                     {
                         fAVMService.removeNode(dstParts[0], dstParts[1]);
                         fAVMService.link(dstParts[0], dstParts[1], srcDesc);
                         continue;
                     }
+                    // You can ignore it.
                     if (ignoreOlder)
                     {
                         continue;
                     }
+                    // Or it's an error.
                     throw new AVMSyncException("Older version prevents update.");
                 }
                 case AVMDifference.CONFLICT :
                 {
+                    // You can force it.
                     if (overrideConflicts)
                     {
                         fAVMService.removeNode(dstParts[0], dstParts[1]);
                         fAVMService.link(dstParts[0], dstParts[1], srcDesc);
                         continue;
                     }
+                    // You can ignore it.
                     if (ignoreConflicts)
                     {
                         continue;
                     }
+                    // Or it's an error.
                     throw new AVMSyncException("Conflict prevents update.");
                 }
                 case AVMDifference.DIRECTORY :
                 {
+                    // You can only ignore this.
                     if (ignoreConflicts)
                     {
                         continue;
                     }
+                    // Otherwise it's an error.
                     throw new AVMSyncException("Directory conflict prevents update.");
                 }
                 default :
@@ -192,8 +380,9 @@ public class AVMSyncServiceImpl implements AVMSyncService
         {
             return AVMDifference.SAME;
         }
-        // Matched directories that are not identical are operationally in conflict
+        // Matched directories that are not identical are nominally in conflict
-        // but get their own special difference code.
+        // but get their own special difference code for comparison logic. The DIRECTORY
+        // difference code never gets returned to callers of compare.
         if (srcDesc.isDirectory() && dstDesc.isDirectory())
         {
             return AVMDifference.DIRECTORY;
@@ -204,6 +393,8 @@ public class AVMSyncServiceImpl implements AVMSyncService
         {
             return AVMDifference.CONFLICT;
         }
+        // A deleted node on either side means uniform handling because
+        // a deleted node can be the descendent of any other type of node.
         if (srcDesc.isDeleted() || dstDesc.isDeleted())
         {
             AVMNodeDescriptor common = fAVMService.getCommonAncestor(srcDesc, dstDesc);
@@ -225,8 +416,11 @@ public class AVMSyncServiceImpl implements AVMSyncService
         // At this point both source and destination are both some kind of file.
         if (srcDesc.isLayeredFile())
         {
+            // Handle the layered file source case.
             if (dstDesc.isPlainFile())
             {
+                // We consider a layered source file that points at the destination
+                // file SAME.
                 if (srcDesc.getIndirection().equals(dstDesc.getPath()))
                 {
                     return AVMDifference.SAME;
@@ -254,6 +448,8 @@ public class AVMSyncServiceImpl implements AVMSyncService
         // Source is a plain file.
         if (dstDesc.isLayeredFile())
         {
+            // We consider a source file that is the target of a layered destination file to be
+            // SAME.
             if (dstDesc.getIndirection().equals(srcDesc.getPath()))
             {
                 return AVMDifference.SAME;

source/java/org/alfresco/repo/avm/LayeredDirectoryNodeImpl.java

@@ -637,13 +637,15 @@ class LayeredDirectoryNodeImpl extends DirectoryNodeImpl implements LayeredDirec
     {
         BasicAttributes attrs = getBasicAttributes();
         String path = lPath.getRepresentedPath();
-        if (path.endsWith("/"))
+        path = AVMNodeConverter.ExtendAVMPath(path, name);
+        String indirect = null;
+        if (fPrimaryIndirection)
         {
-            path = path + name;
+            indirect = fIndirection;
         }
         else
         {
-            path = path + "/" + name;
+            indirect = AVMNodeConverter.ExtendAVMPath(lPath.getCurrentIndirection(), name);
         }
         return new AVMNodeDescriptor(path,
                                      name,
@@ -656,7 +658,7 @@ class LayeredDirectoryNodeImpl extends DirectoryNodeImpl implements LayeredDirec
                                      attrs.getAccessDate(),
                                      getId(),
                                      getVersionID(),
-                                     getUnderlying(lPath),
+                                     indirect,
                                      fPrimaryIndirection,
                                      fLayerID,
                                      fOpacity,

source/java/org/alfresco/service/cmr/avmsync/AVMDifference.java

@@ -131,4 +131,43 @@ public class AVMDifference implements Serializable
     {
         return fSourcePath != null && fDestPath != null;
     }
+    
+    /**
+     * Get as String.
+     * @return A String representation of this.
+     */
+    @Override
+    public String toString()
+    {
+        StringBuilder builder = new StringBuilder();
+        builder.append(fSourcePath);
+        builder.append("[");
+        builder.append(fSourceVersion);
+        builder.append("] ");
+        switch (fDiffCode)
+        {
+            case SAME :
+                builder.append("= ");
+                break;
+            case NEWER :
+                builder.append("> ");
+                break;
+            case OLDER :
+                builder.append("< ");
+                break;
+            case CONFLICT :
+                builder.append("<> ");
+                break;
+            case DIRECTORY :
+                builder.append("| ");
+                break;
+            default :
+                builder.append("? ");
+        }
+        builder.append(fDestPath);
+        builder.append("[");
+        builder.append(fDestVersion);
+        builder.append("]");
+        return builder.toString();
+    }
 }