[KARAF-2628] Fix synchronization issues in the commands completer
[KARAF-2307] Fix some backward compatibility issues with commands
Fix completes so that they only complete with the correct command (i.e. do not 
show options for other commands with the same name but a different scope)
Do not register the subshell service multiple times


Project: http://git-wip-us.apache.org/repos/asf/karaf/repo
Commit: http://git-wip-us.apache.org/repos/asf/karaf/commit/d75f6f0d
Tree: http://git-wip-us.apache.org/repos/asf/karaf/tree/d75f6f0d
Diff: http://git-wip-us.apache.org/repos/asf/karaf/diff/d75f6f0d

Branch: refs/heads/master
Commit: d75f6f0d5ecd94bcc4d0a8e2544c0837157be389
Parents: 8028465
Author: Guillaume Nodet <gno...@gmail.com>
Authored: Tue Dec 17 17:37:22 2013 +0100
Committer: Guillaume Nodet <gno...@gmail.com>
Committed: Tue Dec 17 17:49:27 2013 +0100

----------------------------------------------------------------------
 .../apache/karaf/shell/console/NameScoping.java |   2 +-
 .../console/commands/NamespaceHandler.java      |  75 ++---
 .../console/completer/ArgumentCompleter.java    |  15 +
 .../completer/CommandNamesCompleter.java        |   3 +
 .../console/completer/CommandsCompleter.java    | 313 +++++++++++++------
 .../console/completer/OldArgumentCompleter.java |   9 +-
 .../apache/karaf/shell/commands/Context.java    |   9 +-
 .../karaf/shell/commands/SimpleSubShell.java    |  38 +++
 .../karaf/shell/commands/TestCommands.java      |  26 +-
 .../shell/console/completer/CompletionTest.java | 172 ++++++++++
 10 files changed, 502 insertions(+), 160 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/karaf/blob/d75f6f0d/shell/console/src/main/java/org/apache/karaf/shell/console/NameScoping.java
----------------------------------------------------------------------
diff --git 
a/shell/console/src/main/java/org/apache/karaf/shell/console/NameScoping.java 
b/shell/console/src/main/java/org/apache/karaf/shell/console/NameScoping.java
index e50866f..355464d 100644
--- 
a/shell/console/src/main/java/org/apache/karaf/shell/console/NameScoping.java
+++ 
b/shell/console/src/main/java/org/apache/karaf/shell/console/NameScoping.java
@@ -33,7 +33,7 @@ public class NameScoping {
      */
     public static String getCommandNameWithoutGlobalPrefix(CommandSession 
session, String key) {
         if (!isMultiScopeMode(session)) {
-            String globalScope = (String) session.get("APPLICATION");
+            String globalScope = (String) (session != null ? 
session.get("APPLICATION") : null);
             if (globalScope != null) {
                 String prefix = globalScope + ":";
                 if (key.startsWith(prefix)) {

http://git-wip-us.apache.org/repos/asf/karaf/blob/d75f6f0d/shell/console/src/main/java/org/apache/karaf/shell/console/commands/NamespaceHandler.java
----------------------------------------------------------------------
diff --git 
a/shell/console/src/main/java/org/apache/karaf/shell/console/commands/NamespaceHandler.java
 
b/shell/console/src/main/java/org/apache/karaf/shell/console/commands/NamespaceHandler.java
index d1992f3..8330872 100644
--- 
a/shell/console/src/main/java/org/apache/karaf/shell/console/commands/NamespaceHandler.java
+++ 
b/shell/console/src/main/java/org/apache/karaf/shell/console/commands/NamespaceHandler.java
@@ -185,43 +185,46 @@ public class NamespaceHandler implements 
org.apache.aries.blueprint.NamespaceHan
         
         
context.getComponentDefinitionRegistry().registerComponentDefinition(commandService);
 
-        // create the sub-shell action
-        MutableBeanMetadata subShellAction = 
context.createMetadata(MutableBeanMetadata.class);
-        subShellAction.setRuntimeClass(SubShellAction.class);
-        subShellAction.setActivation(MutableBeanMetadata.ACTIVATION_LAZY);
-        subShellAction.setScope(MutableBeanMetadata.SCOPE_PROTOTYPE);
-        subShellAction.setId(getName());
-        if (scope != null && !scope.isEmpty()) {
-            // it's shell 1.0.0 schema, scope is contained in the descriptor 
itself
-            subShellAction.addProperty("subShell", createStringValue(context, 
scope));
-        } else {
-            // it's shell 1.1.0 schema, we inject the scope from the command
-            subShellAction.addProperty("subShell", getInvocationValue(context, 
"getScope", action.getClassName()));
-        }
-        
context.getComponentDefinitionRegistry().registerComponentDefinition(subShellAction);
-        // generate the sub-shell command
-        MutableBeanMetadata subShellCommand = 
context.createMetadata(MutableBeanMetadata.class);
-        subShellCommand.setId(getName());
-        subShellCommand.setRuntimeClass(BlueprintCommand.class);
-        subShellCommand.addProperty(BLUEPRINT_CONTAINER, createRef(context, 
BLUEPRINT_CONTAINER));
-        subShellCommand.addProperty(BLUEPRINT_CONVERTER, createRef(context, 
BLUEPRINT_CONVERTER));
-        subShellCommand.addProperty(ACTION_ID, createIdRef(context, 
subShellAction.getId()));
-        
context.getComponentDefinitionRegistry().registerComponentDefinition(subShellCommand);
-        // generate the sub-shell OSGi service
-        MutableServiceMetadata subShellCommandService = 
context.createMetadata(MutableServiceMetadata.class);
-        
subShellCommandService.setActivation(MutableServiceMetadata.ACTIVATION_LAZY);
-        subShellCommandService.setId(getName());
-        
subShellCommandService.setAutoExport(ServiceMetadata.AUTO_EXPORT_ALL_CLASSES);
-        subShellCommandService.setServiceComponent(subShellCommand);
-        subShellCommandService.addServiceProperty(createStringValue(context, 
"osgi.command.scope"), createStringValue(context, "*"));
-        if (scope != null && !scope.isEmpty()) {
-            // it's shell 1.0.0 schema, scope is contained in the descriptor 
itself
-            
subShellCommandService.addServiceProperty(createStringValue(context, 
"osgi.command.function"), createStringValue(context, scope));
-        } else {
-            // it's shell 1.1.0 schema, we inject the scope from the command
-            
subShellCommandService.addServiceProperty(createStringValue(context, 
"osgi.command.function"), getInvocationValue(context, "getScope", 
action.getClassName()));
+        String subShellName = ".subshell." + scope;
+        if 
(!context.getComponentDefinitionRegistry().containsComponentDefinition(subShellName))
 {
+            // create the sub-shell action
+            MutableBeanMetadata subShellAction = 
context.createMetadata(MutableBeanMetadata.class);
+            subShellAction.setRuntimeClass(SubShellAction.class);
+            subShellAction.setActivation(MutableBeanMetadata.ACTIVATION_LAZY);
+            subShellAction.setScope(MutableBeanMetadata.SCOPE_PROTOTYPE);
+            subShellAction.setId(getName());
+            if (scope != null && !scope.isEmpty()) {
+                // it's shell 1.0.0 schema, scope is contained in the 
descriptor itself
+                subShellAction.addProperty("subShell", 
createStringValue(context, scope));
+            } else {
+                // it's shell 1.1.0 schema, we inject the scope from the 
command
+                subShellAction.addProperty("subShell", 
getInvocationValue(context, "getScope", action.getClassName()));
+            }
+            
context.getComponentDefinitionRegistry().registerComponentDefinition(subShellAction);
+            // generate the sub-shell command
+            MutableBeanMetadata subShellCommand = 
context.createMetadata(MutableBeanMetadata.class);
+            subShellCommand.setId(getName());
+            subShellCommand.setRuntimeClass(BlueprintCommand.class);
+            subShellCommand.addProperty(BLUEPRINT_CONTAINER, 
createRef(context, BLUEPRINT_CONTAINER));
+            subShellCommand.addProperty(BLUEPRINT_CONVERTER, 
createRef(context, BLUEPRINT_CONVERTER));
+            subShellCommand.addProperty(ACTION_ID, createIdRef(context, 
subShellAction.getId()));
+            
context.getComponentDefinitionRegistry().registerComponentDefinition(subShellCommand);
+            // generate the sub-shell OSGi service
+            MutableServiceMetadata subShellCommandService = 
context.createMetadata(MutableServiceMetadata.class);
+            
subShellCommandService.setActivation(MutableServiceMetadata.ACTIVATION_LAZY);
+            subShellCommandService.setId(subShellName);
+            
subShellCommandService.setAutoExport(ServiceMetadata.AUTO_EXPORT_ALL_CLASSES);
+            subShellCommandService.setServiceComponent(subShellCommand);
+            
subShellCommandService.addServiceProperty(createStringValue(context, 
"osgi.command.scope"), createStringValue(context, "*"));
+            if (scope != null && !scope.isEmpty()) {
+                // it's shell 1.0.0 schema, scope is contained in the 
descriptor itself
+                
subShellCommandService.addServiceProperty(createStringValue(context, 
"osgi.command.function"), createStringValue(context, scope));
+            } else {
+                // it's shell 1.1.0 schema, we inject the scope from the 
command
+                
subShellCommandService.addServiceProperty(createStringValue(context, 
"osgi.command.function"), getInvocationValue(context, "getScope", 
action.getClassName()));
+            }
+            
context.getComponentDefinitionRegistry().registerComponentDefinition(subShellCommandService);
         }
-        
context.getComponentDefinitionRegistry().registerComponentDefinition(subShellCommandService);
     }
 
     private MutableBeanMetadata getInvocationValue(ParserContext context, 
String method, String className) {

http://git-wip-us.apache.org/repos/asf/karaf/blob/d75f6f0d/shell/console/src/main/java/org/apache/karaf/shell/console/completer/ArgumentCompleter.java
----------------------------------------------------------------------
diff --git 
a/shell/console/src/main/java/org/apache/karaf/shell/console/completer/ArgumentCompleter.java
 
b/shell/console/src/main/java/org/apache/karaf/shell/console/completer/ArgumentCompleter.java
index 1e26d66..dc874d7 100644
--- 
a/shell/console/src/main/java/org/apache/karaf/shell/console/completer/ArgumentCompleter.java
+++ 
b/shell/console/src/main/java/org/apache/karaf/shell/console/completer/ArgumentCompleter.java
@@ -31,6 +31,7 @@ import java.util.List;
 import java.util.Map;
 import java.util.Set;
 
+import org.apache.felix.service.command.Function;
 import org.apache.karaf.shell.commands.Action;
 import org.apache.karaf.shell.commands.Argument;
 import org.apache.karaf.shell.commands.CommandWithAction;
@@ -45,12 +46,16 @@ import org.apache.karaf.shell.console.NameScoping;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import static 
org.apache.karaf.shell.console.completer.CommandsCompleter.unProxy;
+
 public class ArgumentCompleter implements Completer {
 
     private static final Logger LOGGER = 
LoggerFactory.getLogger(ArgumentCompleter.class);
 
     public static final String ARGUMENTS_LIST = "ARGUMENTS_LIST";
 
+    public static final String COMMANDS = ".commands";
+
     final Completer commandCompleter;
     final Completer optionsCompleter;
     final List<Completer> argsCompleters;
@@ -225,6 +230,16 @@ public class ArgumentCompleter implements Completer {
             if (!verifyCompleter(commandCompleter, args[index])) {
                 return -1;
             }
+            // Verify scope if
+            // - the command name has no scope
+            // - we have a session
+            if (!args[index].contains(":") && commandSession != null) {
+                Function f1 = unProxy((Function) commandSession.get("*:" + 
args[index]));
+                Function f2 = unProxy(this.function);
+                if (f1 != null && f1 != f2) {
+                    return -1;
+                }
+            }
             index++;
         } else {
             comp = commandCompleter;

http://git-wip-us.apache.org/repos/asf/karaf/blob/d75f6f0d/shell/console/src/main/java/org/apache/karaf/shell/console/completer/CommandNamesCompleter.java
----------------------------------------------------------------------
diff --git 
a/shell/console/src/main/java/org/apache/karaf/shell/console/completer/CommandNamesCompleter.java
 
b/shell/console/src/main/java/org/apache/karaf/shell/console/completer/CommandNamesCompleter.java
index b188ad7..23f5447 100644
--- 
a/shell/console/src/main/java/org/apache/karaf/shell/console/completer/CommandNamesCompleter.java
+++ 
b/shell/console/src/main/java/org/apache/karaf/shell/console/completer/CommandNamesCompleter.java
@@ -84,6 +84,9 @@ public class CommandNamesCompleter implements Completer {
     private class CommandTracker {
         public CommandTracker() throws Exception {
             BundleContext context = 
FrameworkUtil.getBundle(getClass()).getBundleContext();
+            if (context == null) {
+                throw new IllegalStateException("Bundle is stopped");
+            }
             ServiceListener listener = new ServiceListener() {
                 public void serviceChanged(ServiceEvent event) {
                     commands.clear();

http://git-wip-us.apache.org/repos/asf/karaf/blob/d75f6f0d/shell/console/src/main/java/org/apache/karaf/shell/console/completer/CommandsCompleter.java
----------------------------------------------------------------------
diff --git 
a/shell/console/src/main/java/org/apache/karaf/shell/console/completer/CommandsCompleter.java
 
b/shell/console/src/main/java/org/apache/karaf/shell/console/completer/CommandsCompleter.java
index a8f96db..c2f868a 100644
--- 
a/shell/console/src/main/java/org/apache/karaf/shell/console/completer/CommandsCompleter.java
+++ 
b/shell/console/src/main/java/org/apache/karaf/shell/console/completer/CommandsCompleter.java
@@ -20,18 +20,30 @@ package org.apache.karaf.shell.console.completer;
 
 import java.lang.reflect.Field;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collections;
+import java.util.Comparator;
+import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
+import java.util.Map;
 import java.util.Set;
+import java.util.TreeMap;
+import java.util.concurrent.Callable;
 
+import org.apache.aries.proxy.ProxyManager;
+import org.apache.felix.service.command.CommandProcessor;
 import org.apache.karaf.shell.commands.CommandWithAction;
 import org.apache.felix.service.command.CommandSession;
 import org.apache.felix.service.command.Function;
 import org.apache.karaf.shell.console.CommandSessionHolder;
 import org.apache.karaf.shell.console.Completer;
 import org.apache.karaf.shell.console.SessionProperties;
+import org.osgi.framework.Bundle;
 import org.osgi.framework.BundleContext;
+import org.osgi.framework.FrameworkUtil;
+import org.osgi.framework.ServiceEvent;
+import org.osgi.framework.ServiceListener;
 import org.osgi.framework.ServiceReference;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -47,7 +59,8 @@ public class CommandsCompleter implements Completer {
     private static final Logger LOGGER = 
LoggerFactory.getLogger(CommandsCompleter.class);
 
     private CommandSession session;
-    private final List<Completer> completers = new ArrayList<Completer>();
+    private final Map<String, Completer> globalCompleters = new 
HashMap<String, Completer>();
+    private final Map<String, Completer> localCompleters = new HashMap<String, 
Completer>();
     private final Set<String> commands = new HashSet<String>();
 
     public CommandsCompleter() {
@@ -56,6 +69,11 @@ public class CommandsCompleter implements Completer {
 
     public CommandsCompleter(CommandSession session) {
         this.session = session;
+        try {
+            new CommandTracker();
+        } catch (Throwable t) {
+            // Ignore in case we're not in OSGi
+        }
     }
 
 
@@ -63,118 +81,190 @@ public class CommandsCompleter implements Completer {
         if (session == null) {
             session = CommandSessionHolder.getSession();
         }
-        checkData();
-        int res = new AggregateCompleter(completers).complete(buffer, cursor, 
candidates);
+
+        List<String> scopes = getCurrentScopes();
+        Map<String, Completer>[] allCompleters = checkData();
+        sort(allCompleters, scopes);
+
+        String subShell = getCurrentSubShell();
+        String completion = getCompletionType();
+
+        // SUBSHELL mode
+        if ("SUBSHELL".equalsIgnoreCase(completion)) {
+            if (subShell.isEmpty()) {
+                subShell = "*";
+            }
+            List<Completer> completers = new ArrayList<Completer>();
+            for (String name : allCompleters[1].keySet()) {
+                if (name.startsWith(subShell)) {
+                    completers.add(allCompleters[1].get(name));
+                }
+            }
+            if (!subShell.equals("*")) {
+                completers.add(new StringsCompleter(new String[] { "exit" }));
+            }
+            int res = new AggregateCompleter(completers).complete(buffer, 
cursor, candidates);
+            Collections.sort(candidates);
+            return res;
+        }
+
+        if ("FIRST".equalsIgnoreCase(completion)) {
+            if (!subShell.isEmpty()) {
+                List<Completer> completers = new ArrayList<Completer>();
+                for (String name : allCompleters[1].keySet()) {
+                    if (name.startsWith(subShell)) {
+                        completers.add(allCompleters[1].get(name));
+                    }
+                }
+                int res = new AggregateCompleter(completers).complete(buffer, 
cursor, candidates);
+                if (!candidates.isEmpty()) {
+                    Collections.sort(candidates);
+                    return res;
+                }
+            }
+            List<Completer> compl = new ArrayList<Completer>();
+            compl.add(new StringsCompleter(getAliases()));
+            compl.addAll(allCompleters[0].values());
+            int res = new AggregateCompleter(compl).complete(buffer, cursor, 
candidates);
+            Collections.sort(candidates);
+            return res;
+        }
+
+        List<Completer> compl = new ArrayList<Completer>();
+        compl.add(new StringsCompleter(getAliases()));
+        compl.addAll(allCompleters[0].values());
+        int res = new AggregateCompleter(compl).complete(buffer, cursor, 
candidates);
         Collections.sort(candidates);
         return res;
     }
 
-    @SuppressWarnings("unchecked")
-    protected synchronized void checkData() {
-        // Copy the set to avoid concurrent modification exceptions
-        // TODO: fix that in gogo instead
-        // get the current sub-shell
-        String subshell = (String) session.get("SUBSHELL");
-        String completion = (String) 
session.get(SessionProperties.COMPLETION_MODE);
-        if (completion == null)
-            completion = "GLOBAL";
-
-        Set<String> names = new HashSet<String>((Set<String>) 
session.get(COMMANDS));
-        Set<String> filteredNames = new HashSet<String>();
-        for (String command : names) {
-            if (subshell == null || command.startsWith(subshell)) {
-                filteredNames.add(command);
-            }
+    protected void sort(Map<String, Completer>[] completers, List<String> 
scopes) {
+        ScopeComparator comparator = new ScopeComparator(scopes);
+        for (int i = 0; i < completers.length; i++) {
+            Map<String, Completer> map = new TreeMap<String, 
Completer>(comparator);
+            map.putAll(completers[i]);
+            completers[i] = map;
         }
+    }
 
-        if (!filteredNames.equals(commands)) {
-            commands.clear();
-            completers.clear();
-
-            if (completion.equalsIgnoreCase("GLOBAL")) {
-                Set<String> aliases = this.getAliases();
-                completers.add(new StringsCompleter(aliases));
-            } else {
-                if (subshell == null || subshell.length() == 0) {
-                    // add aliases if we are not in a subshell
-                    Set<String> aliases = this.getAliases();
-                    completers.add(new StringsCompleter(aliases));
+    protected static class ScopeComparator implements Comparator<String> {
+        private final List<String> scopes;
+        public ScopeComparator(List<String> scopes) {
+            this.scopes = scopes;
+        }
+        @Override
+        public int compare(String o1, String o2) {
+            String[] p1 = o1.split(":");
+            String[] p2 = o2.split(":");
+            int p = 0;
+            while (p < p1.length && p < p2.length) {
+                int i1 = scopes.indexOf(p1[p]);
+                int i2 = scopes.indexOf(p2[p]);
+                if (i1 < 0) {
+                    if (i2 < 0) {
+                        int c = p1[p].compareTo(p2[p]);
+                        if (c != 0) {
+                            return c;
+                        } else {
+                            p++;
+                        }
+                    } else {
+                        return +1;
+                    }
+                } else if (i2 < 0) {
+                    return -1;
+                } else if (i1 < i2) {
+                    return -1;
+                } else if (i1 > i2) {
+                    return +1;
+                } else {
+                    p++;
                 }
             }
+            return 0;
+        }
+    }
+
+    protected List<String> getCurrentScopes() {
+        String scopes = (String) session.get("SCOPE");
+        return Arrays.asList(scopes.split(":"));
+    }
+
+    protected String getCurrentSubShell() {
+        String s = (String) session.get("SUBSHELL");
+        if (s == null) {
+            s = "";
+        }
+        return s;
+    }
+
+    protected String getCompletionType() {
+        String completion = (String) 
session.get(SessionProperties.COMPLETION_MODE);
+        if (completion == null) {
+            completion = "GLOBAL";
+        }
+        return completion;
+    }
+
+    protected String stripScope(String name) {
+        int index = name.indexOf(":");
+        return index > 0 ? name.substring(index + 1) : name;
+    }
+
+    @SuppressWarnings("unchecked")
+    protected Map<String, Completer>[] checkData() {
+        // Copy the set to avoid concurrent modification exceptions
+        // TODO: fix that in gogo instead
+        Set<String> names;
+        boolean update;
+        synchronized (this) {
+            names = new HashSet<String>((Set<String>) session.get(COMMANDS));
+            update = !names.equals(commands);
+        }
+        if (update) {
+            // get command aliases
+            Set<String> commands = new HashSet<String>();
+            Map<String, Completer> global = new HashMap<String, Completer>();
+            Map<String, Completer> local = new HashMap<String, Completer>();
 
             // add argument completers for each command
             for (String command : names) {
+                String rawCommand = stripScope(command);
                 Function function = (Function) session.get(command);
-
                 function = unProxy(function);
                 if (function instanceof CommandWithAction) {
-                    if (completion.equalsIgnoreCase("GLOBAL") ||
-                            (completion.equalsIgnoreCase("FIRST") && (subshell 
== null || subshell.length() == 0))) {
-                        try {
-                            completers.add(new ArgumentCompleter(session, 
(CommandWithAction) function, command));
-                        } catch (Throwable t) {
-                            LOGGER.debug("Unable to create completers for 
command '{}'", command, t);
-                        }
-                    } else {
-                        if (command.startsWith(subshell)) {
-
-                            if (subshell.length() > 1 && command.length() > 
subshell.length()) {
-                                command = command.substring(subshell.length() 
+ 1);
-                            }
-
-                            if (completion.equalsIgnoreCase("SUBSHELL")) {
-                                // filter on subshell
-                                // as the completion mode is set to SUBSHELL, 
we complete only with the commands local
-                                // to the current subshell
-                                if (command.contains(":")) {
-                                    int index = command.indexOf(':');
-                                    command = command.substring(0, index);
-                                }
-                                command.trim();
-                            }
-                            try {
-                                completers.add(new ArgumentCompleter(session, 
(CommandWithAction) function, command));
-                            } catch (Throwable t) {
-                                LOGGER.debug("Unable to create completers for 
command '{}'", command, t);
-                            }
-                        }
+                    try {
+                        global.put(command, new ArgumentCompleter(session, 
(CommandWithAction) function, command));
+                        local.put(command, new ArgumentCompleter(session, 
(CommandWithAction) function, rawCommand));
+                    } catch (Throwable t) {
+                        LOGGER.debug("Unable to create completers for command 
'" + command + "'", t);
                     }
                 }
-                if (function instanceof 
org.apache.felix.gogo.commands.CommandWithAction) {
-                    if (completion.equalsIgnoreCase("GLOBAL") ||
-                            (completion.equalsIgnoreCase("FIRST") && (subshell 
== null || subshell.length() == 0))) {
-                        try {
-                            completers.add(new OldArgumentCompleter(session, 
(org.apache.felix.gogo.commands.CommandWithAction) function, command));
-                        } catch (Throwable t) {
-                            LOGGER.debug("Unable to create completers for 
command '{}'", command, t);
-                        }
-                    } else {
-                        if (command.startsWith(subshell)) {
-
-                            if (subshell.length() > 1 && command.length() > 
subshell.length()) {
-                                command = command.substring(subshell.length() 
+ 1);
-                            }
-
-                            if (completion.equalsIgnoreCase("SUBSHELL")) {
-                                // filter on subshell
-                                // as the completion mode is set to SUBSHELL, 
we complete only with the commands local
-                                // to the current subshell
-                                if (command.contains(":")) {
-                                    int index = command.indexOf(':');
-                                    command = command.substring(0, index);
-                                }
-                                command.trim();
-                            }
-                            try {
-                                completers.add(new 
OldArgumentCompleter(session, 
(org.apache.felix.gogo.commands.CommandWithAction) function, command));
-                            } catch (Throwable t) {
-                                LOGGER.debug("Unable to create completers for 
command '{}'", command, t);
-                            }
-                        }
+                else if (function instanceof 
org.apache.felix.gogo.commands.CommandWithAction) {
+                    try {
+                        global.put(command, new OldArgumentCompleter(session, 
(org.apache.felix.gogo.commands.CommandWithAction) function, command));
+                        local.put(command, new OldArgumentCompleter(session, 
(org.apache.felix.gogo.commands.CommandWithAction) function, rawCommand));
+                    } catch (Throwable t) {
+                        LOGGER.debug("Unable to create completers for command 
'" + command + "'", t);
                     }
                 }
                 commands.add(command);
             }
+
+            synchronized (this) {
+                this.commands.clear();
+                this.globalCompleters.clear();
+                this.localCompleters.clear();
+                this.commands.addAll(commands);
+                this.globalCompleters.putAll(global);
+                this.localCompleters.putAll(local);
+            }
+        }
+        synchronized (this) {
+            return new Map[] {
+                    new HashMap<String, Completer>(this.globalCompleters),
+                    new HashMap<String, Completer>(this.localCompleters)
+            };
         }
     }
 
@@ -189,14 +279,14 @@ public class CommandsCompleter implements Completer {
         Set<String> aliases = new HashSet<String>();
         for (String var : vars) {
             Object content = session.get(var);
-            if 
("org.apache.felix.gogo.runtime.Closure".equals(content.getClass().getName())) {
+            if (content != null && 
"org.apache.felix.gogo.runtime.Closure".equals(content.getClass().getName())) {
                 aliases.add(var);
             }
         }
         return aliases;
     }
 
-    protected Function unProxy(Function function) {
+    public static Function unProxy(Function function) {
         try {
             if 
("org.apache.felix.gogo.runtime.CommandProxy".equals(function.getClass().getName()))
 {
                 Field contextField = 
function.getClass().getDeclaredField("context");
@@ -208,7 +298,7 @@ public class CommandsCompleter implements Completer {
                 Object target = context != null ? 
context.getService(reference) : null;
                 try {
                     if (target instanceof Function) {
-                        function = (Function) target;
+                        return unProxy((Function) target);
                     }
                 } finally {
                     if (context != null) {
@@ -216,10 +306,41 @@ public class CommandsCompleter implements Completer {
                     }
                 }
             }
+            Bundle bundle = FrameworkUtil.getBundle(CommandsCompleter.class);
+            BundleContext bc = bundle.getBundleContext();
+            ServiceReference<ProxyManager> ref = 
bc.getServiceReference(ProxyManager.class);
+            ProxyManager pm = ref != null ? bc.getService(ref) : null;
+            if (pm != null) {
+                Callable call = pm.unwrap(function);
+                if (call != null) {
+                    return unProxy((Function) call.call());
+                }
+                bc.ungetService(ref);
+            }
         } catch (Throwable t) {
         }
         return function;
     }
 
+    private class CommandTracker {
+        public CommandTracker() throws Exception {
+            BundleContext context = 
FrameworkUtil.getBundle(getClass()).getBundleContext();
+            if (context == null) {
+                throw new IllegalStateException("Bundle is stopped");
+            }
+            ServiceListener listener = new ServiceListener() {
+                public void serviceChanged(ServiceEvent event) {
+                    synchronized (CommandsCompleter.this) {
+                        commands.clear();
+                    }
+                }
+            };
+            context.addServiceListener(listener,
+                    String.format("(&(%s=*)(%s=*))",
+                            CommandProcessor.COMMAND_SCOPE,
+                            CommandProcessor.COMMAND_FUNCTION));
+        }
+    }
+
 }
 

http://git-wip-us.apache.org/repos/asf/karaf/blob/d75f6f0d/shell/console/src/main/java/org/apache/karaf/shell/console/completer/OldArgumentCompleter.java
----------------------------------------------------------------------
diff --git 
a/shell/console/src/main/java/org/apache/karaf/shell/console/completer/OldArgumentCompleter.java
 
b/shell/console/src/main/java/org/apache/karaf/shell/console/completer/OldArgumentCompleter.java
index 5587243..583d6f8 100644
--- 
a/shell/console/src/main/java/org/apache/karaf/shell/console/completer/OldArgumentCompleter.java
+++ 
b/shell/console/src/main/java/org/apache/karaf/shell/console/completer/OldArgumentCompleter.java
@@ -96,7 +96,14 @@ public class OldArgumentCompleter implements Completer {
         argsCompleters = new ArrayList<Completer>();
 
         if (function instanceof CompletableFunction) {
-            optionalCompleters = ((CompletableFunction) 
function).getOptionalCompleters();
+            Map<String, Completer> opt;
+            try {
+                //
+                opt = ((CompletableFunction) function).getOptionalCompleters();
+            } catch (Throwable t) {
+                opt = new HashMap<String, Completer>();
+            }
+            optionalCompleters = opt;
             List<Completer> fcl = ((CompletableFunction) 
function).getCompleters();
             if (fcl != null) {
                 for (Completer c : fcl) {

http://git-wip-us.apache.org/repos/asf/karaf/blob/d75f6f0d/shell/console/src/test/java/org/apache/karaf/shell/commands/Context.java
----------------------------------------------------------------------
diff --git 
a/shell/console/src/test/java/org/apache/karaf/shell/commands/Context.java 
b/shell/console/src/test/java/org/apache/karaf/shell/commands/Context.java
index 2ddee05..7737c11 100644
--- a/shell/console/src/test/java/org/apache/karaf/shell/commands/Context.java
+++ b/shell/console/src/test/java/org/apache/karaf/shell/commands/Context.java
@@ -21,6 +21,7 @@ package org.apache.karaf.shell.commands;
 import org.apache.felix.gogo.runtime.CommandProcessorImpl;
 import org.apache.felix.gogo.runtime.CommandSessionImpl;
 import org.apache.felix.gogo.runtime.threadio.ThreadIOImpl;
+import org.apache.felix.service.command.CommandSession;
 
 public class Context extends CommandProcessorImpl
 {
@@ -49,11 +50,6 @@ public class Context extends CommandProcessorImpl
         return session.execute(source);
     }
 
-    public void addCommand(String name, Object target)
-    {
-        put("test:" + name, target);
-    }
-
     public void set(String name, Object value)
     {
         session.put(name, value);
@@ -64,5 +60,8 @@ public class Context extends CommandProcessorImpl
         return session.get(name);
     }
 
+    public CommandSession getSession() {
+        return session;
+    }
 
 }

http://git-wip-us.apache.org/repos/asf/karaf/blob/d75f6f0d/shell/console/src/test/java/org/apache/karaf/shell/commands/SimpleSubShell.java
----------------------------------------------------------------------
diff --git 
a/shell/console/src/test/java/org/apache/karaf/shell/commands/SimpleSubShell.java
 
b/shell/console/src/test/java/org/apache/karaf/shell/commands/SimpleSubShell.java
new file mode 100644
index 0000000..59fe9a2
--- /dev/null
+++ 
b/shell/console/src/test/java/org/apache/karaf/shell/commands/SimpleSubShell.java
@@ -0,0 +1,38 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.karaf.shell.commands;
+
+import org.apache.karaf.shell.commands.basic.SimpleCommand;
+import org.apache.karaf.shell.console.SubShellAction;
+
+public class SimpleSubShell extends SimpleCommand {
+
+    private final String subshell;
+
+    public SimpleSubShell(String subshell) {
+        super(SubShellAction.class);
+        this.subshell = subshell;
+    }
+
+    @Override
+    public Action createNewAction() {
+        SubShellAction action = (SubShellAction) super.createNewAction();
+        action.setSubShell(subshell);
+        return action;
+    }
+}

http://git-wip-us.apache.org/repos/asf/karaf/blob/d75f6f0d/shell/console/src/test/java/org/apache/karaf/shell/commands/TestCommands.java
----------------------------------------------------------------------
diff --git 
a/shell/console/src/test/java/org/apache/karaf/shell/commands/TestCommands.java 
b/shell/console/src/test/java/org/apache/karaf/shell/commands/TestCommands.java
index e43595f..7ad5b75 100644
--- 
a/shell/console/src/test/java/org/apache/karaf/shell/commands/TestCommands.java
+++ 
b/shell/console/src/test/java/org/apache/karaf/shell/commands/TestCommands.java
@@ -29,15 +29,14 @@ import junit.framework.TestCase;
 import org.apache.karaf.shell.commands.basic.SimpleCommand;
 import org.apache.felix.service.command.CommandSession;
 import org.apache.karaf.shell.console.ExitAction;
-import org.apache.karaf.shell.console.SubShellAction;
 
 public class TestCommands extends TestCase {
 
     public void testSubShellScope() throws Exception {
         Context c = new Context();
         c.set("SCOPE", "*");
-        c.addCommand("foo", new SimpleSubShell("foo"));
-        c.addCommand("exit", new SimpleCommand(ExitAction.class));
+        c.addCommand("*", new SimpleSubShell("foo"), "foo");
+        c.addCommand("*", new SimpleCommand(ExitAction.class), "exit");
 
         String scope = (String) c.get("SCOPE");
         c.execute("foo");
@@ -58,8 +57,8 @@ public class TestCommands extends TestCase {
 
     public void testCommand() throws Exception {
         Context c = new Context();
-        c.addCommand("capture", this);
-        c.addCommand("my-action", new SimpleCommand(MyAction.class));
+        c.addCommand("*", this, "capture");
+        c.addCommand("*", new SimpleCommand(MyAction.class), "my-action");
 
         // Test help
         Object help = c.execute("my-action --help | capture");
@@ -94,7 +93,7 @@ public class TestCommands extends TestCase {
 
     public void testCommandTwoArguments() throws Exception {
         Context c = new Context();
-        c.addCommand("my-action-two-arguments", new 
SimpleCommand(MyActionTwoArguments.class));
+        c.addCommand("*", new SimpleCommand(MyActionTwoArguments.class), 
"my-action-two-arguments");
 
         // test required arguments
         try {
@@ -176,19 +175,4 @@ public class TestCommands extends TestCase {
 
     }
 
-    public static class SimpleSubShell extends SimpleCommand {
-        private final String subshell;
-        public SimpleSubShell(String subshell) {
-            super(SubShellAction.class);
-            this.subshell = subshell;
-        }
-
-        @Override
-        public Action createNewAction() {
-            SubShellAction action = (SubShellAction) super.createNewAction();
-            action.setSubShell(subshell);
-            return action;
-        }
-    }
-
 }

http://git-wip-us.apache.org/repos/asf/karaf/blob/d75f6f0d/shell/console/src/test/java/org/apache/karaf/shell/console/completer/CompletionTest.java
----------------------------------------------------------------------
diff --git 
a/shell/console/src/test/java/org/apache/karaf/shell/console/completer/CompletionTest.java
 
b/shell/console/src/test/java/org/apache/karaf/shell/console/completer/CompletionTest.java
new file mode 100644
index 0000000..5fa1c8e
--- /dev/null
+++ 
b/shell/console/src/test/java/org/apache/karaf/shell/console/completer/CompletionTest.java
@@ -0,0 +1,172 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.karaf.shell.console.completer;
+
+import java.util.Arrays;
+
+import org.apache.felix.service.command.CommandSession;
+import org.apache.karaf.shell.commands.Action;
+import org.apache.karaf.shell.commands.Argument;
+import org.apache.karaf.shell.commands.Context;
+import org.apache.karaf.shell.commands.Option;
+import org.apache.karaf.shell.commands.SimpleSubShell;
+import org.apache.karaf.shell.commands.basic.SimpleCommand;
+import org.apache.karaf.shell.console.CommandSessionHolder;
+import org.apache.karaf.shell.console.Completer;
+import org.apache.karaf.shell.console.ExitAction;
+import org.apache.karaf.shell.console.SessionProperties;
+import org.junit.Test;
+
+import static org.junit.Assert.assertEquals;
+
+public class CompletionTest extends CompleterTestSupport {
+
+    @Test
+    public void testSubShellCompletion() throws Exception {
+        Context context = new Context();
+        context.set("SCOPE", "*");
+        context.set(SessionProperties.COMPLETION_MODE, "subshell");
+        CommandSessionHolder.setSession(context.getSession());
+
+        context.addCommand("*", new SimpleSubShell("foo"), "foo");
+        context.addCommand("*", new SimpleCommand(ExitAction.class), "exit");
+        context.addCommand("foo", new SimpleCommand(MyAction.class), 
"my-action");
+        context.addCommand("foo", new 
SimpleCommand(MyActionTwoArguments.class), "one-action");
+        context.addCommand("bar", new SimpleCommand(MyAction.class), 
"one-action");
+        context.addCommand("bar", new 
SimpleCommand(MyActionTwoArguments.class), "another");
+
+        Completer comp = new CommandsCompleter(context.getSession());
+
+        context.execute("foo");
+        assertEquals(Arrays.asList("my-action "), complete(comp, "my"));
+        assertEquals(Arrays.asList("exit ", "my-action ", "one-action "), 
complete(comp, ""));
+        assertEquals(Arrays.asList(), complete(comp, "an"));
+        assertEquals(Arrays.asList("--check", "--foo", "--help", "--integer", 
"--string"),
+                     complete(comp, "my-action --"));
+        assertEquals(Arrays.asList("--dummy", "--help"), complete(comp, 
"one-action --"));
+
+        context.execute("exit");
+        assertEquals(Arrays.asList(), complete(comp, "my"));
+        assertEquals(Arrays.asList("exit ", "foo "), complete(comp, ""));
+        assertEquals(Arrays.asList(), complete(comp, "an"));
+    }
+
+    @Test
+    public void testFirstCompletion() throws Exception {
+        Context context = new Context();
+        context.set("SCOPE", "*");
+        context.set(SessionProperties.COMPLETION_MODE, "first");
+        CommandSessionHolder.setSession(context.getSession());
+
+        context.addCommand("*", new SimpleSubShell("foo"), "foo");
+        context.addCommand("*", new SimpleCommand(ExitAction.class), "exit");
+        context.addCommand("foo", new SimpleCommand(MyAction.class), 
"my-action");
+        context.addCommand("foo", new 
SimpleCommand(MyActionTwoArguments.class), "one-action");
+        context.addCommand("bar", new SimpleCommand(MyAction.class), 
"one-action");
+        context.addCommand("bar", new 
SimpleCommand(MyActionTwoArguments.class), "another");
+
+        Completer comp = new CommandsCompleter(context.getSession());
+
+        context.execute("foo");
+        assertEquals(Arrays.asList("my-action "), complete(comp, "my"));
+        assertEquals(Arrays.asList("my-action ", "one-action "), 
complete(comp, ""));
+        assertEquals(Arrays.asList("another "), complete(comp, "an"));
+        assertEquals(Arrays.asList("--check", "--foo", "--help", "--integer", 
"--string"),
+                     complete(comp, "my-action --"));
+        assertEquals(Arrays.asList("--dummy", "--help"), complete(comp, 
"one-action --"));
+
+        context.execute("exit");
+        assertEquals(Arrays.asList("my-action "), complete(comp, "my"));
+        assertEquals(Arrays.asList("*:exit", "*:foo", "another", "bar:another",
+                                   "bar:one-action", "exit", "foo",
+                                   "foo:my-action", "foo:one-action", 
"my-action",
+                                   "one-action", "one-action"), complete(comp, 
""));
+        assertEquals(Arrays.asList("another "), complete(comp, "an"));
+    }
+
+    @Test
+    public void testGlobalCompletion() throws Exception {
+        Context context = new Context();
+        context.set("SCOPE", "*");
+        context.set(SessionProperties.COMPLETION_MODE, "global");
+        CommandSessionHolder.setSession(context.getSession());
+
+        context.addCommand("*", new SimpleSubShell("foo"), "foo");
+        context.addCommand("*", new SimpleCommand(ExitAction.class), "exit");
+        context.addCommand("foo", new SimpleCommand(MyAction.class), 
"my-action");
+        context.addCommand("foo", new 
SimpleCommand(MyActionTwoArguments.class), "one-action");
+        context.addCommand("bar", new SimpleCommand(MyAction.class), 
"one-action");
+        context.addCommand("bar", new 
SimpleCommand(MyActionTwoArguments.class), "another");
+
+        Completer comp = new CommandsCompleter(context.getSession());
+
+        context.execute("foo");
+        assertEquals(Arrays.asList("my-action "), complete(comp, "my"));
+        assertEquals(Arrays.asList("*:exit", "*:foo", "another", "bar:another",
+                                    "bar:one-action", "exit", "foo",
+                                   "foo:my-action", "foo:one-action", 
"my-action",
+                                   "one-action", "one-action"), complete(comp, 
""));
+        assertEquals(Arrays.asList("another "), complete(comp, "an"));
+        assertEquals(Arrays.asList("--check", "--foo", "--help", "--integer", 
"--string"),
+                     complete(comp, "my-action --"));
+        assertEquals(Arrays.asList("--dummy", "--help"), complete(comp, 
"one-action --"));
+
+        context.execute("exit");
+        assertEquals(Arrays.asList("my-action "), complete(comp, "my"));
+        assertEquals(Arrays.asList("*:exit", "*:foo", "another", "bar:another",
+                                   "bar:one-action", "exit", "foo",
+                                   "foo:my-action", "foo:one-action", 
"my-action",
+                                   "one-action", "one-action"), complete(comp, 
""));
+        assertEquals(Arrays.asList("another "), complete(comp, "an"));
+    }
+
+    public static class MyAction implements Action {
+        @Option(name = "-f", aliases = { "--foo" })
+        int f;
+
+        @Option(name = "-c", aliases = "--check")
+        boolean check;
+
+        @Option(name = "-s", aliases = "--string")
+        String string;
+
+        @Option(name = "-i", aliases = "--integer")
+        String integer;
+
+        public Object execute(CommandSession session) throws Exception {
+            return null;
+        }
+    }
+
+    public static class MyActionTwoArguments implements Action {
+
+        @Option(name = "--dummy")
+        boolean dummy;
+
+        @Argument(index = 0, name = "one", description = "one description", 
required = true, multiValued = false)
+        private String one;
+
+        @Argument(index = 1, name = "two", description = "two description", 
required = true, multiValued = false)
+        private String two;
+
+        public Object execute(CommandSession session) throws Exception {
+            return null;
+        }
+
+    }
+}

Reply via email to