Merge pull request #505 from VineetReynolds/command-cache-perf-fix

Command cache performance fix

shell/impl/src/main/java/org/jboss/forge/addon/shell/aesh/ForgeCommandRegistry.java

@@ -132,7 +132,7 @@ private AbstractShellInteraction findCommand(ShellContext shellContext, String c
    {
       AbstractShellInteraction result = null;
       CommandLineUtil cmdLineUtil = getCommandLineUtil();
-      UICommand cmd = commandFactory.getCommandByName(shellContext, commandName);
+      UICommand cmd = commandFactory.getNewCommandByName(shellContext, commandName);
       if (cmd != null)
       {
          CommandController controller = commandControllerFactory.createController(shellContext, shell, cmd);

ui/api/src/main/java/org/jboss/forge/addon/ui/command/CommandFactory.java

@@ -19,6 +19,13 @@ public interface CommandFactory
 {
    /**
     * Get all {@link UICommand} instances from all available {@link CommandProvider} services.
+    * The underlying collection of UICommands is very likely be cached, and thus invoking operations
+    * directly on them is not recommended for safety.
+    * 
+    * Consecutive invocations may be performed on the same UICommand instance, instead of different instances.
+    * 
+    * It is recommended to create a new UICommand when required. This can be done through the
+    * {@link #getNewCommandByName(UIContext, String)} method.
     */
    Iterable<UICommand> getCommands();
 
@@ -41,4 +48,9 @@ public interface CommandFactory
     * Return the {@link UICommand} name for a given {@link UIContext}
     */
    String getCommandName(UIContext context, UICommand cmd);
+
+   /**
+    * Get an un-cached {@link UICommand} instance given its name in the given {@link UIContext}
+    */
+   UICommand getNewCommandByName(UIContext context, String name);
 }

ui/impl/src/main/java/org/jboss/forge/addon/ui/impl/command/CommandFactoryImpl.java

@@ -6,7 +6,6 @@
  */
 package org.jboss.forge.addon.ui.impl.command;
 
-import java.util.HashSet;
 import java.util.Iterator;
 import java.util.Set;
 import java.util.TreeSet;
@@ -25,6 +24,7 @@
 import org.jboss.forge.addon.ui.wizard.UIWizardStep;
 import org.jboss.forge.furnace.addons.AddonRegistry;
 import org.jboss.forge.furnace.services.Imported;
+import org.jboss.forge.furnace.util.Sets;
 import org.jboss.forge.furnace.util.Strings;
 
 /**
@@ -38,39 +38,17 @@ public class CommandFactoryImpl implements CommandFactory
 {
    @Inject
    private AddonRegistry registry;
+
+   private Set<UICommand> cache = Sets.getConcurrentSet();
+
+   private long version = -1;
 
    private static final Logger log = Logger.getLogger(CommandFactoryImpl.class.getName());
 
    @Override
    public Iterable<UICommand> getCommands()
    {
-      Set<UICommand> result = new HashSet<>();
-      synchronized (this)
-      {
-         Imported<CommandProvider> instances = registry.getServices(CommandProvider.class);
-         for (CommandProvider provider : instances)
-         {
-            Iterable<UICommand> commands = provider.getCommands();
-            Iterator<UICommand> iterator = commands.iterator();
-            while (iterator.hasNext())
-            {
-               try
-               {
-                  UICommand command = iterator.next();
-                  if (!(command instanceof UIWizardStep))
-                  {
-                     result.add(command);
-                  }
-               }
-               catch (Exception e)
-               {
-                  log.log(Level.SEVERE, "Error while retrieving command instance", e);
-               }
-            }
-            instances.release(provider);
-         }
-      }
-      return result;
+      return getCachedCommands();
    }
 
    @Override
@@ -143,6 +121,78 @@ public UICommand getCommandByName(UIContext context, String name)
       }
       return null;
    }
+
+   @Override
+   public UICommand getNewCommandByName(UIContext context, String name)
+   {
+      for (UICommand cmd : getCommandsFromSource())
+      {
+         String commandName = getCommandName(context, cmd);
+         if (Strings.compare(name, commandName) || Strings.compare(name, shellifyName(commandName)))
+         {
+            return cmd;
+         }
+      }
+      return null;
+   }
+
+   private Iterable<UICommand> getCachedCommands()
+   {
+      if (registry.getVersion() != version)
+      {
+         version = registry.getVersion();
+         cache.clear();
+         getCommands(new Operation()
+         {
+            @Override
+            public void execute(UICommand command)
+            {
+               cache.add(command);
+            }
+         });
+      }
+      return cache;
+   }
+
+   private Iterable<UICommand> getCommandsFromSource()
+   {
+      final Set<UICommand> result = Sets.getConcurrentSet();
+      getCommands(new Operation()
+      {
+         @Override
+         public void execute(UICommand command)
+         {
+            result.add(command);
+         }
+      });
+      return result;
+   }
+
+   private void getCommands(Operation operation)
+   {
+      Imported<CommandProvider> instances = registry.getServices(CommandProvider.class);
+      for (CommandProvider provider : instances)
+      {
+         Iterable<UICommand> commands = provider.getCommands();
+         Iterator<UICommand> iterator = commands.iterator();
+         while (iterator.hasNext())
+         {
+            try
+            {
+               UICommand command = iterator.next();
+               if (!(command instanceof UIWizardStep))
+               {
+                  operation.execute(command);
+               }
+            }
+            catch (Exception e)
+            {
+               log.log(Level.SEVERE, "Error while retrieving command instance", e);
+            }
+         }
+         instances.release(provider);
+      }
+   }
 
    /**
     * "Shellifies" a name (that is, makes the name shell-friendly) by replacing spaces with "-" and removing colons
@@ -151,5 +201,15 @@ private static String shellifyName(String name)
    {
       return name.trim().toLowerCase().replaceAll("\\W+", "-").replaceAll("\\:", "");
    }
+
+   /**
+    * 
+    * Strategy for operation to be performed when iterating through Commands 
+    *
+    */
+   interface Operation
+   {
+      void execute(UICommand command);
+   }
 
 }