You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by ro...@apache.org on 2015/09/30 19:56:00 UTC

svn commit: r1706098 - in /sling/trunk/tooling/ide: api/src/org/apache/sling/ide/transport/ impl-vlt-test/src/test/java/org/apache/sling/ide/impl/vlt/transport/ impl-vlt/src/org/apache/sling/ide/impl/vlt/ impl-vlt/src/org/apache/sling/ide/impl/vlt/tran...

Author: rombert
Date: Wed Sep 30 17:55:59 2015
New Revision: 1706098

URL: http://svn.apache.org/viewvc?rev=1706098&view=rev
Log:
SLING-4438 - Don't execute duplicate or out-of-order commands 

Add support for compacting identical add or update commands

Modified:
    sling/trunk/tooling/ide/api/src/org/apache/sling/ide/transport/Command.java
    sling/trunk/tooling/ide/impl-vlt-test/src/test/java/org/apache/sling/ide/impl/vlt/transport/VltBatcherTest.java
    sling/trunk/tooling/ide/impl-vlt/src/org/apache/sling/ide/impl/vlt/AddOrUpdateNodeCommand.java
    sling/trunk/tooling/ide/impl-vlt/src/org/apache/sling/ide/impl/vlt/transport/VltBatcher.java

Modified: sling/trunk/tooling/ide/api/src/org/apache/sling/ide/transport/Command.java
URL: http://svn.apache.org/viewvc/sling/trunk/tooling/ide/api/src/org/apache/sling/ide/transport/Command.java?rev=1706098&r1=1706097&r2=1706098&view=diff
==============================================================================
--- sling/trunk/tooling/ide/api/src/org/apache/sling/ide/transport/Command.java (original)
+++ sling/trunk/tooling/ide/api/src/org/apache/sling/ide/transport/Command.java Wed Sep 30 17:55:59 2015
@@ -27,7 +27,7 @@ public interface Command<T> {
      *
      */
     enum Kind {
-        DELETE
+        DELETE, ADD_OR_UPDATE
     }
 
 	Result<T> execute();

Modified: sling/trunk/tooling/ide/impl-vlt-test/src/test/java/org/apache/sling/ide/impl/vlt/transport/VltBatcherTest.java
URL: http://svn.apache.org/viewvc/sling/trunk/tooling/ide/impl-vlt-test/src/test/java/org/apache/sling/ide/impl/vlt/transport/VltBatcherTest.java?rev=1706098&r1=1706097&r2=1706098&view=diff
==============================================================================
--- sling/trunk/tooling/ide/impl-vlt-test/src/test/java/org/apache/sling/ide/impl/vlt/transport/VltBatcherTest.java (original)
+++ sling/trunk/tooling/ide/impl-vlt-test/src/test/java/org/apache/sling/ide/impl/vlt/transport/VltBatcherTest.java Wed Sep 30 17:55:59 2015
@@ -27,8 +27,10 @@ import java.util.List;
 import javax.jcr.Credentials;
 import javax.jcr.Repository;
 
+import org.apache.sling.ide.impl.vlt.AddOrUpdateNodeCommand;
 import org.apache.sling.ide.impl.vlt.DeleteNodeCommand;
 import org.apache.sling.ide.transport.Command;
+import org.apache.sling.ide.transport.ResourceProxy;
 import org.hamcrest.Matchers;
 import org.junit.Before;
 import org.junit.Test;
@@ -96,4 +98,37 @@ public class VltBatcherTest {
         batcher.get();
         assertThat(batcher.get(), hasSize(0));
     }
+    
+    @Test
+    public void identicalAddOrUpdatesAreCompacted() {
+        
+        AddOrUpdateNodeCommand first = new AddOrUpdateNodeCommand(mockRepo, credentials, null, null, new ResourceProxy("/content"), null);
+        AddOrUpdateNodeCommand second = new AddOrUpdateNodeCommand(mockRepo, credentials, null, null, new ResourceProxy("/content"), null);
+        
+        batcher.add(first);
+        batcher.add(second);
+        
+        List<Command<?>> batched = batcher.get();
+        
+        assertThat(batched, hasSize(1));
+        Command<?> command = batched.get(0);
+        assertThat(command, instanceOf(AddOrUpdateNodeCommand.class));
+        assertThat(command.getPath(), equalTo("/content"));
+        
+    }
+
+    @Test
+    public void unrelatedAddOrUpdatesAreNotCompacted() {
+        AddOrUpdateNodeCommand first = new AddOrUpdateNodeCommand(mockRepo, credentials, null, null, new ResourceProxy("/content/a"), null);
+        AddOrUpdateNodeCommand second = new AddOrUpdateNodeCommand(mockRepo, credentials, null, null, new ResourceProxy("/content/b"), null);
+        
+        batcher.add(first);
+        batcher.add(second);
+        
+        List<Command<?>> batched = batcher.get();
+        
+        assertThat(batched, hasSize(2));
+        assertThat(batched.get(0), Matchers.<Command<?>> sameInstance(first));
+        assertThat(batched.get(1), Matchers.<Command<?>> sameInstance(second));
+    }
 }

Modified: sling/trunk/tooling/ide/impl-vlt/src/org/apache/sling/ide/impl/vlt/AddOrUpdateNodeCommand.java
URL: http://svn.apache.org/viewvc/sling/trunk/tooling/ide/impl-vlt/src/org/apache/sling/ide/impl/vlt/AddOrUpdateNodeCommand.java?rev=1706098&r1=1706097&r2=1706098&view=diff
==============================================================================
--- sling/trunk/tooling/ide/impl-vlt/src/org/apache/sling/ide/impl/vlt/AddOrUpdateNodeCommand.java (original)
+++ sling/trunk/tooling/ide/impl-vlt/src/org/apache/sling/ide/impl/vlt/AddOrUpdateNodeCommand.java Wed Sep 30 17:55:59 2015
@@ -507,4 +507,8 @@ public class AddOrUpdateNodeCommand exte
         return values;
     }
 
+    @Override
+    public Kind getKind() {
+        return Kind.ADD_OR_UPDATE;
+    }
 }

Modified: sling/trunk/tooling/ide/impl-vlt/src/org/apache/sling/ide/impl/vlt/transport/VltBatcher.java
URL: http://svn.apache.org/viewvc/sling/trunk/tooling/ide/impl-vlt/src/org/apache/sling/ide/impl/vlt/transport/VltBatcher.java?rev=1706098&r1=1706097&r2=1706098&view=diff
==============================================================================
--- sling/trunk/tooling/ide/impl-vlt/src/org/apache/sling/ide/impl/vlt/transport/VltBatcher.java (original)
+++ sling/trunk/tooling/ide/impl-vlt/src/org/apache/sling/ide/impl/vlt/transport/VltBatcher.java Wed Sep 30 17:55:59 2015
@@ -18,10 +18,10 @@ package org.apache.sling.ide.impl.vlt.tr
 
 import java.util.ArrayList;
 import java.util.List;
+import java.util.ListIterator;
 
 import org.apache.sling.ide.transport.Batcher;
 import org.apache.sling.ide.transport.Command;
-import org.apache.sling.ide.transport.Command.Kind;
 import org.apache.sling.ide.util.PathUtil;
 
 public class VltBatcher implements Batcher {
@@ -36,18 +36,23 @@ public class VltBatcher implements Batch
     @Override
     public List<Command<?>> get() {
 
-        LinkedCommands deletes = new LinkedCommands();
+        LinkedCommands batched = new LinkedCommands();
         List<Command<?>> result = new ArrayList<Command<?>>();
         
         for ( Command<?> cmd : queue) {
-            if ( cmd.getKind() == Kind.DELETE ) {
-                deletes.addLinked(cmd);        
-            } else {
+            boolean accepted = batched.addLinked(cmd);
+            if ( !accepted) {
                 result.add(cmd);
             }
         }
         
-        result.addAll(0, deletes.getLinkHeads());
+        result.addAll(0, batched.getUpdates());
+        result.addAll(0, batched.getDeletes());
+        
+        // Expected order is:
+        // - delete
+        // - add-or-update
+        // - everything else, in the order it was specified
         
         queue.clear();
         
@@ -56,41 +61,70 @@ public class VltBatcher implements Batch
     
     private static class LinkedCommands {
         
-        private List<CommandWrapper> wrappers = new ArrayList<CommandWrapper>();
+        private List<Command<?>> deletes = new ArrayList<Command<?>>();
+        private List<Command<?>> updates = new ArrayList<Command<?>>();
         
-        public void addLinked(Command<?> cmd) {
+        public boolean addLinked(Command<?> newCmd) {
+            
+            if ( newCmd.getKind() == null ) {
+                return false;
+            }
             
-            String path = cmd.getPath();
-            for ( CommandWrapper wrapper : wrappers ) {
-                if ( PathUtil.isAncestor(wrapper.main.getPath(), path ) ) {
+            switch ( newCmd.getKind() ) {
+                case DELETE:
+                    processDelete(newCmd);
+                    return true;
+                    
+                case ADD_OR_UPDATE:
+                    processAddOrUpdate(newCmd);
+                    return true;
+                
+                default:
+                    return false;
+            
+            }
+        }
+
+        private void processDelete(Command<?> newCmd) {
+            String path = newCmd.getPath();
+            for ( ListIterator<Command<?>> iterator = deletes.listIterator(); iterator.hasNext(); ) {
+                
+                // if we already have an ancestor deleted, skip this one
+                Command<?> oldCmd = iterator.next();
+                if ( PathUtil.isAncestor(oldCmd.getPath(), path ) ) {
                     return;
                 }
                 
-                if ( PathUtil.isAncestor(path, wrapper.main.getPath())) {
-                    wrapper.main = cmd;
+                // if we are delete an ancestor of another resource which gets deleted, replace it
+                if ( PathUtil.isAncestor(path, oldCmd.getPath())) {
+                    iterator.set(newCmd);
                     return;
                 }
             }
             
-            wrappers.add(new CommandWrapper(cmd));
+            // no matching deletions, add it as-is
+            deletes.add(newCmd);
         }
         
-        public List<Command<?>> getLinkHeads() {
-            List<Command<?>> heads = new ArrayList<Command<?>>(wrappers.size());
-            for ( CommandWrapper wrapper : wrappers ) {
-                heads.add(wrapper.main);
+        private void processAddOrUpdate(Command<?> newCmd) {
+            String path = newCmd.getPath();
+            for (Command<?> oldCmd : updates) {
+                // if we already have an add-or-update for this path, skip it    
+                if ( path.equals(oldCmd.getPath()) ) {
+                    return;
+                }
             }
             
-            return heads;
+            // no adds or updates, add it as-is
+            updates.add(newCmd);            
         }
-    }
-    
-    private static class CommandWrapper {
         
-        public Command<?> main;
+        public List<Command<?>> getDeletes() {
+            return deletes;
+        }
         
-        private CommandWrapper(Command<?> main) {
-            this.main = main;
+        public List<Command<?>> getUpdates() {
+            return updates;
         }
     }
 }