FELIX-1745: Unable to alter/unset a log level after it has been set from the console

git-svn-id: https://svn.apache.org/repos/asf/felix/trunk@824488 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/karaf/shell/log/pom.xml b/karaf/shell/log/pom.xml
index 52e89ab..03c64ae 100644
--- a/karaf/shell/log/pom.xml
+++ b/karaf/shell/log/pom.xml
@@ -62,6 +62,12 @@
             <groupId>org.ops4j.pax.logging</groupId>
             <artifactId>pax-logging-service</artifactId>
         </dependency>
+                
+        <dependency>
+            <groupId>org.ops4j.pax.logging</groupId>
+            <artifactId>pax-logging-api</artifactId>
+            <scope>test</scope>
+        </dependency>
 
     </dependencies>
 
diff --git a/karaf/shell/log/src/main/java/org/apache/felix/karaf/shell/log/SetLogLevel.java b/karaf/shell/log/src/main/java/org/apache/felix/karaf/shell/log/SetLogLevel.java
index d8d683e..6227d60 100644
--- a/karaf/shell/log/src/main/java/org/apache/felix/karaf/shell/log/SetLogLevel.java
+++ b/karaf/shell/log/src/main/java/org/apache/felix/karaf/shell/log/SetLogLevel.java
@@ -16,6 +16,7 @@
  */
 package org.apache.felix.karaf.shell.log;
 
+import java.io.IOException;
 import java.util.Dictionary;
 
 import org.apache.felix.karaf.shell.console.OsgiCommandSupport;
@@ -30,8 +31,8 @@
  */
 @Command(scope = "log", name = "set", description = "Sets the log level.")
 public class SetLogLevel extends OsgiCommandSupport {
-
-    @Argument(index = 0, name = "level", description = "The log level to set (TRACE, DEBUG, INFO, WARN, ERROR or - to unset)", required = true, multiValued = false)
+    
+    @Argument(index = 0, name = "level", description = "The log level to set (TRACE, DEBUG, INFO, WARN, ERROR) or DEFAULT to unset", required = true, multiValued = false)
     String level;
 
     @Argument(index = 1, name = "logger", description = "Logger name or ROOT (default)", required = false, multiValued = false)
@@ -47,7 +48,7 @@
     static final String INFO = "INFO";
     static final String WARN = "WARN";
     static final String ERROR = "ERROR";
-    static final String INHERITED = "-";
+    static final String DEFAULT = "DEFAULT";
 
     protected Object doExecute() throws Exception {
         if (ROOT_LOGGER.equalsIgnoreCase(this.logger)) {
@@ -58,17 +59,16 @@
                 !INFO.equals(level) &&
                 !WARN.equals(level) &&
                 !ERROR.equals(level) &&
-                !INHERITED.equals(level)) {
-            System.err.println("level must be set to TRACE, DEBUG, INFO, WARN or ERROR (or - to unset it)");
+                !DEFAULT.equals(level)) {
+            System.err.println("level must be set to TRACE, DEBUG, INFO, WARN or ERROR (or DEFAULT to unset it)");
             return null;
         }
-        if (INHERITED.equals(level) && logger == null) {
+        if (DEFAULT.equals(level) && logger == null) {
             System.err.println("Can not unset the ROOT logger");
             return null;
         }
 
-        ConfigurationAdmin cfgAdmin = getConfigAdmin();
-        Configuration cfg = cfgAdmin.getConfiguration(CONFIGURATION_PID, null);
+        Configuration cfg = getConfiguration();
         Dictionary props = cfg.getProperties();
 
         String logger = this.logger;
@@ -80,7 +80,7 @@
             prop = LOGGER_PREFIX + logger;
         }
         val = (String) props.get(prop);
-        if (INHERITED.equals(level)) {
+        if (DEFAULT.equals(level)) {
             if (val != null) {
                 val = val.trim();
                 int idx = val.indexOf(",");
@@ -96,9 +96,9 @@
             } else {
                 val = val.trim();
                 int idx = val.indexOf(",");
-                if (idx == 0) {
-                    val = level + val;
-                } else if (idx > 0) {
+                if (idx < 0) {
+                    val = level;
+                } else {
                     val = level + val.substring(idx);
                 }
             }
@@ -112,6 +112,13 @@
 
         return null;
     }
+    
+    
+
+    protected Configuration getConfiguration() throws IOException {
+        Configuration cfg = getConfigAdmin().getConfiguration(CONFIGURATION_PID, null);
+        return cfg;
+    }
 
     protected ConfigurationAdmin getConfigAdmin() {
         ServiceReference ref = getBundleContext().getServiceReference(ConfigurationAdmin.class.getName());
diff --git a/karaf/shell/log/src/test/java/org/apache/felix/karaf/shell/log/SetLogLevelTest.java b/karaf/shell/log/src/test/java/org/apache/felix/karaf/shell/log/SetLogLevelTest.java
new file mode 100644
index 0000000..0f2058b
--- /dev/null
+++ b/karaf/shell/log/src/test/java/org/apache/felix/karaf/shell/log/SetLogLevelTest.java
@@ -0,0 +1,134 @@
+/*
+ * 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.felix.karaf.shell.log;
+
+import java.io.IOException;
+import java.util.Hashtable;
+
+import junit.framework.TestCase;
+
+import org.easymock.EasyMock;
+import org.osgi.service.cm.Configuration;
+
+/**
+ * Test cases for {@link SetLogLevel}
+ */
+@SuppressWarnings("unchecked")
+public class SetLogLevelTest extends TestCase {
+    
+    private static final String ROOT_LOGGER = "log4j.rootLogger";
+    private static final String PACKAGE_LOGGER = "log4j.logger.org.apache.karaf.test";
+    
+    private SetLogLevel command;
+    private Hashtable properties = new Hashtable();
+
+    @Override
+    protected void setUp() throws Exception {
+        super.setUp();
+        properties.clear();
+
+        final Configuration configuration = EasyMock.createMock(Configuration.class);
+        EasyMock.expect(configuration.getProperties()).andReturn(properties);
+        configuration.update(properties);
+        EasyMock.replay(configuration);
+        
+        command = new SetLogLevel() {
+            @Override
+            protected Configuration getConfiguration() throws IOException {
+                return configuration;
+            }
+        };
+    }
+    
+    public void testSetLogLevel() throws Exception {
+        runCommand("log:set INFO org.apache.karaf.test");
+        
+        assertEquals("INFO", properties.get(PACKAGE_LOGGER));
+    }
+    
+    public void testSetRootLogLevel() throws Exception {
+        runCommand("log:set INFO");
+        
+        assertEquals("INFO", properties.get(ROOT_LOGGER));
+    }
+    
+    public void testChangeLogLevel() throws Exception {
+        properties.put(PACKAGE_LOGGER, "DEBUG");
+        
+        runCommand("log:set INFO org.apache.karaf.test");
+        
+        assertEquals("INFO", properties.get(PACKAGE_LOGGER));
+    }
+    
+    public void testChangeRootLogLevel() throws Exception {
+        properties.put(ROOT_LOGGER, "DEBUG");
+        
+        runCommand("log:set INFO");
+        
+        assertEquals("INFO", properties.get(ROOT_LOGGER));
+    }
+    
+    public void testChangeLogLevelWithAppender() throws Exception {
+        properties.put(PACKAGE_LOGGER, "DEBUG, APPENDER1");
+        
+        runCommand("log:set INFO org.apache.karaf.test");
+        
+        assertEquals("INFO, APPENDER1", properties.get(PACKAGE_LOGGER));
+    }
+    
+    public void testChangeRootLogLevelWithAppender() throws Exception {
+        properties.put(ROOT_LOGGER, "DEBUG, APPENDER1");
+        
+        runCommand("log:set INFO");
+        
+        assertEquals("INFO, APPENDER1", properties.get(ROOT_LOGGER));
+    }
+    
+    
+    public void testUnsetLogLevel() throws Exception {
+        properties.put(PACKAGE_LOGGER, "DEBUG");
+
+        runCommand("log:set DEFAULT org.apache.karaf.test");
+        
+        assertFalse("Configuration for logger org.apache.karaf.test has been removed", 
+                    properties.containsKey(PACKAGE_LOGGER));
+    }
+    
+    
+    public void testUnsetRootLogLevel() throws Exception {
+        properties.put(ROOT_LOGGER, "INFO");
+        
+        runCommand("log:set DEFAULT");
+        
+        assertEquals("Configuration for root logger should not be removed",
+                     "INFO", properties.get(ROOT_LOGGER));
+    }
+    
+    /*
+     * Simulate running the log:set command
+     */
+    private void runCommand(String commandline) throws Exception {
+        String[] parts = commandline.split(" ");
+
+        command.level = parts[1];
+        if (parts.length == 3) {
+            command.logger = "org.apache.karaf.test";
+        }
+        
+        command.doExecute();
+    }
+}