FELIX-1655: enhance the webconsole plugin + unit tests, patch provided by David Bosschaert with many thx

git-svn-id: https://svn.apache.org/repos/asf/felix/trunk@825982 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/karaf/shell/admin/src/main/java/org/apache/felix/karaf/shell/admin/InstanceSettings.java b/karaf/shell/admin/src/main/java/org/apache/felix/karaf/shell/admin/InstanceSettings.java
index 65e3197..b8a9c01 100644
--- a/karaf/shell/admin/src/main/java/org/apache/felix/karaf/shell/admin/InstanceSettings.java
+++ b/karaf/shell/admin/src/main/java/org/apache/felix/karaf/shell/admin/InstanceSettings.java
@@ -46,4 +46,37 @@
     public List<String> getFeatures() {
         return features;
     }
+
+    @Override
+    public boolean equals(Object o) {
+        if (o == this) {
+            return true;
+        }
+        if (!(o instanceof InstanceSettings)) {
+            return false;
+        }
+        InstanceSettings is = (InstanceSettings) o;
+        return is.port == port &&
+               (location == null ? is.location == null : location.equals(is.location)) &&
+               (featureURLs == null ? is.featureURLs == null : featureURLs.equals(is.featureURLs)) &&
+               (features == null ? is.features == null : features.equals(is.features));
+    }
+
+    @Override
+    public int hashCode() {
+        int rc = 17;
+        rc = 37 * port;
+        if (location != null) {
+            rc = 37 * location.hashCode();
+        }
+        if (featureURLs != null) {
+            rc = 37 * featureURLs.hashCode();
+        }
+        if (features != null) {
+            rc = 37 * features.hashCode();
+        }
+        return rc;
+    }
+    
+    
 }
diff --git a/karaf/shell/admin/src/main/java/org/apache/felix/karaf/shell/admin/internal/AdminServiceImpl.java b/karaf/shell/admin/src/main/java/org/apache/felix/karaf/shell/admin/internal/AdminServiceImpl.java
index 0f19ecd..efb4d1a 100644
--- a/karaf/shell/admin/src/main/java/org/apache/felix/karaf/shell/admin/internal/AdminServiceImpl.java
+++ b/karaf/shell/admin/src/main/java/org/apache/felix/karaf/shell/admin/internal/AdminServiceImpl.java
@@ -176,7 +176,7 @@
         return instance;
     }
 
-    private void handleFeatures(File featuresCfg, InstanceSettings settings) throws IOException {
+    void handleFeatures(File featuresCfg, InstanceSettings settings) throws IOException {
         Properties p = loadStorage(featuresCfg);
 
         appendToPropList(p, "featuresBoot", settings.getFeatures());
@@ -188,7 +188,7 @@
         if (elements == null) {
             return;
         }
-        StringBuilder sb = new StringBuilder(p.getProperty(key));
+        StringBuilder sb = new StringBuilder(p.getProperty(key).trim());
         for (String f : elements) {
             if (sb.length() > 0) {
                 sb.append(',');
diff --git a/karaf/shell/admin/src/main/java/org/apache/felix/karaf/shell/admin/internal/commands/CreateCommand.java b/karaf/shell/admin/src/main/java/org/apache/felix/karaf/shell/admin/internal/commands/CreateCommand.java
index a8d3a40..194be21 100644
--- a/karaf/shell/admin/src/main/java/org/apache/felix/karaf/shell/admin/internal/commands/CreateCommand.java
+++ b/karaf/shell/admin/src/main/java/org/apache/felix/karaf/shell/admin/internal/commands/CreateCommand.java
@@ -33,10 +33,10 @@
 public class CreateCommand extends AdminCommandSupport
 {
     @Option(name = "-p", aliases = {"--port"}, description = "Port number for remote shell connection", required = false, multiValued = false)
-    private int port = 0;
+    int port = 0;
 
     @Option(name = "-l", aliases = {"--location"}, description = "Location of the new container instance in the file system", required = false, multiValued = false)
-    private String location;
+    String location;
     
     @Option(name = "-f", aliases = {"--feature"}, 
             description = "Initial features. This option can be specified multiple times to enable multiple initial features", required = false, multiValued = true)
@@ -47,7 +47,7 @@
     List<String> featureURLs;
 
     @Argument(index = 0, name = "name", description="The name of the new container instance", required = true, multiValued = false)
-    private String instance = null;
+    String instance = null;
 
     protected Object doExecute() throws Exception {
         InstanceSettings settings = new InstanceSettings(port, location, featureURLs, features);
diff --git a/karaf/shell/admin/src/test/java/org/apache/felix/karaf/shell/admin/InstanceSettingsTest.java b/karaf/shell/admin/src/test/java/org/apache/felix/karaf/shell/admin/InstanceSettingsTest.java
new file mode 100644
index 0000000..499c8ff
--- /dev/null
+++ b/karaf/shell/admin/src/test/java/org/apache/felix/karaf/shell/admin/InstanceSettingsTest.java
@@ -0,0 +1,53 @@
+/*
+ * 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.admin;
+
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+
+import junit.framework.TestCase;
+
+public class InstanceSettingsTest extends TestCase {
+    public void testInstanceSettings() {
+        InstanceSettings is = 
+            new InstanceSettings(1, null, Collections.<String>emptyList(), Arrays.asList("hi"));
+        assertEquals(1, is.getPort());
+        assertNull(is.getLocation());
+        assertEquals(Arrays.asList("hi"), is.getFeatures());
+        assertEquals(0, is.getFeatureURLs().size());
+    }
+    
+    public void testEqualsHashCode() {
+        testEqualsHashCode(1, "top", Collections.<String>emptyList(), Arrays.asList("hi"));
+        testEqualsHashCode(0, null, null, null);
+    }
+
+    private void testEqualsHashCode(int port, String location, List<String> featureURLs, List<String> features) {
+        InstanceSettings is = new InstanceSettings(port, location, featureURLs, features);
+        InstanceSettings is2 = new InstanceSettings(port, location, featureURLs, features);
+        assertEquals(is, is2);
+        assertEquals(is.hashCode(), is2.hashCode());
+    }
+    
+    public void testEqualsHashCode2() {
+        InstanceSettings is = new InstanceSettings(1, "top", Collections.<String>emptyList(), Arrays.asList("hi"));
+        assertFalse(is.equals(null));
+        assertFalse(is.equals(new Object()));
+        assertEquals(is, is);
+    }
+}
diff --git a/karaf/shell/admin/src/test/java/org/apache/felix/karaf/shell/admin/internal/AdminServiceImplTest.java b/karaf/shell/admin/src/test/java/org/apache/felix/karaf/shell/admin/internal/AdminServiceImplTest.java
new file mode 100644
index 0000000..f4ba88f
--- /dev/null
+++ b/karaf/shell/admin/src/test/java/org/apache/felix/karaf/shell/admin/internal/AdminServiceImplTest.java
@@ -0,0 +1,64 @@
+/*
+ * 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.admin.internal;
+
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileOutputStream;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.util.Arrays;
+import java.util.Properties;
+
+import junit.framework.TestCase;
+
+import org.apache.felix.karaf.shell.admin.InstanceSettings;
+
+public class AdminServiceImplTest extends TestCase {
+    public void testHandleFeatures() throws Exception {
+        AdminServiceImpl as = new AdminServiceImpl();
+        
+        File f = File.createTempFile(getName(), ".test");
+        try {
+            Properties p = new Properties();
+            p.put("featuresBoot", "abc,def ");
+            p.put("featuresRepositories", "somescheme://xyz");
+            OutputStream os = new FileOutputStream(f);
+            try {
+                p.store(os, "Test comment");
+            } finally {
+                os.close();
+            }
+            
+            InstanceSettings s = new InstanceSettings(8122, null, null, Arrays.asList("test"));
+            as.handleFeatures(f, s);
+            
+            Properties p2 = new Properties();
+            InputStream is = new FileInputStream(f);
+            try {
+                p2.load(is);
+            } finally {
+                is.close();
+            }
+            assertEquals(2, p2.size());
+            assertEquals("abc,def,test", p2.get("featuresBoot"));
+            assertEquals("somescheme://xyz", p2.get("featuresRepositories"));
+        } finally {
+            f.delete();
+        }
+    }
+}
diff --git a/karaf/shell/admin/src/test/java/org/apache/felix/karaf/shell/admin/internal/commands/CreateCommandTest.java b/karaf/shell/admin/src/test/java/org/apache/felix/karaf/shell/admin/internal/commands/CreateCommandTest.java
new file mode 100644
index 0000000..53cffce
--- /dev/null
+++ b/karaf/shell/admin/src/test/java/org/apache/felix/karaf/shell/admin/internal/commands/CreateCommandTest.java
@@ -0,0 +1,51 @@
+/*
+ * 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.admin.internal.commands;
+
+import java.util.Arrays;
+import java.util.Collections;
+
+import junit.framework.TestCase;
+
+import org.apache.felix.karaf.shell.admin.AdminService;
+import org.apache.felix.karaf.shell.admin.InstanceSettings;
+import org.easymock.EasyMock;
+
+public class CreateCommandTest extends TestCase {
+    public void testCreateCommandExecute() throws Exception {
+        AdminService adminService = EasyMock.createMock(AdminService.class);
+        EasyMock.replay(adminService);
+        
+        CreateCommand cc = new CreateCommand();
+        cc.setAdminService(adminService);
+        cc.port = 9941;
+        cc.location = "top";
+        cc.features = Arrays.asList("abc", "def");
+        cc.featureURLs = Collections.singletonList("http://something");
+        cc.instance = "myInstance";
+        
+        EasyMock.verify(adminService); // check precondition
+        EasyMock.reset(adminService);
+        InstanceSettings expectedIS = 
+            new InstanceSettings(9941, "top", Collections.singletonList("http://something"), Arrays.asList("abc", "def"));
+        EasyMock.expect(adminService.createInstance("myInstance", expectedIS)).andReturn(null);
+        EasyMock.replay(adminService);
+        
+        cc.doExecute();
+        EasyMock.verify(adminService);
+    }
+}
diff --git a/karaf/webconsole/admin/pom.xml b/karaf/webconsole/admin/pom.xml
index b8314dd..e2583d1 100644
--- a/karaf/webconsole/admin/pom.xml
+++ b/karaf/webconsole/admin/pom.xml
@@ -76,6 +76,14 @@
             <scope>compile</scope>
             <optional>true</optional>
         </dependency>
+        
+        <!-- Only needed while running the unit tests -->
+        <dependency>
+            <groupId>commons-fileupload</groupId>
+            <artifactId>commons-fileupload</artifactId>
+            <version>${commons.logging.version}</version>
+            <scope>test</scope>
+        </dependency>
     </dependencies>
   
     <build>
diff --git a/karaf/webconsole/admin/src/main/java/org/apache/felix/karaf/webconsole/admin/AdminPlugin.java b/karaf/webconsole/admin/src/main/java/org/apache/felix/karaf/webconsole/admin/AdminPlugin.java
index cf2ddf1..33af430 100644
--- a/karaf/webconsole/admin/src/main/java/org/apache/felix/karaf/webconsole/admin/AdminPlugin.java
+++ b/karaf/webconsole/admin/src/main/java/org/apache/felix/karaf/webconsole/admin/AdminPlugin.java
@@ -20,7 +20,8 @@
 import java.io.InputStream;
 import java.io.PrintWriter;
 import java.net.URL;
-import java.util.Collections;
+import java.util.ArrayList;
+import java.util.List;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
@@ -112,7 +113,10 @@
         } else if ("create".equals(action)) {
             int port = parsePortNumber(req.getParameter("port"));
             String location = parseString(req.getParameter("location"));
-            success = createInstance(name, port, location);
+            List<String> featureURLs = parseStringList(req.getParameter("featureURLs"));
+            List<String> features = parseStringList(req.getParameter("features"));
+            InstanceSettings settings = new InstanceSettings(port, location, featureURLs, features); 
+            success = createInstance(name, settings);
         } else if ("destroy".equals(action)) {
             success = destroyInstance(name);
         } else if ("start".equals(action)) {
@@ -142,6 +146,20 @@
         }
         return value;
     }
+    
+    private List<String> parseStringList(String value) {
+        List<String> list = new ArrayList<String>();
+        if (value != null) {
+            for (String el : value.split(",")) {
+                String trimmed = el.trim();
+                if (trimmed.length() == 0) {
+                    continue;
+                }
+                list.add(trimmed);
+            }            
+        }
+        return list;
+    }
 
     /*
      * Parse the port number for the String given, returning 0 if the String does not represent an integer 
@@ -273,10 +291,8 @@
         return buffer.toString();
     }
 
-    private boolean createInstance(String name, int port, String location) {
+    private boolean createInstance(String name, InstanceSettings settings) {
         try {
-            InstanceSettings settings = new InstanceSettings(port, location, 
-                    Collections.<String>emptyList(), Collections.<String>emptyList());
             adminService.createInstance(name, settings);
             return true;
         } catch (Exception ex) {
diff --git a/karaf/webconsole/admin/src/main/resources/res/ui/admin.js b/karaf/webconsole/admin/src/main/resources/res/ui/admin.js
index caf62e0..cc48772 100644
--- a/karaf/webconsole/admin/src/main/resources/res/ui/admin.js
+++ b/karaf/webconsole/admin/src/main/resources/res/ui/admin.js
@@ -29,6 +29,11 @@
     "<td>Name: <input id='name' type='text' name='name' style='width:70%' colspan='2'/></td>" +
     "<td>Port: <input id='port' type='text' name='port' style='width:70%' colspan='2'/></td>" +
     "<td>Location: <input id='location' type='text' name='location' style='width:70%' colspan='2'/></td>" +
+    "<td />" +
+    "</tr><tr><td>Features: <input id='features' type='text' name='features' style='width:70%' colspan='2'" + 
+    " title='Specify initial features separated by commas.'/></td>" + 
+    "<td colspan='2'>Feature URLs: <input id='featureURLs' type='text' name='featureURLs' style='width:80%' colspan='2'" + 
+    " title='Specify additional feature URLs separate by commas.'/></td>" +
     "<td class='col_Actions'><input type='button' value='Create' onclick='createInstance()'/></td>" +
     "</tr></tbody></table></div></form><br/>";
     $("#plugin_content").append( txt );
@@ -40,11 +45,15 @@
     var name = document.getElementById( "name" ).value;
     var port = document.getElementById( "port" ).value;
     var location = document.getElementById( "location" ).value;
-    postCreateInstance( name, port, location );
+    var features = document.getElementById( "features" ).value;
+    var featureURLs = document.getElementById( "featureURLs" ).value;
+    postCreateInstance( name, port, location, features, featureURLs );
 }
 
-function postCreateInstance( /* String */ name, /* String */ port, /* String */ location ) {
-    $.post( pluginRoot, {"action": "create", "name": name, "port": port, "location": location}, function( data ) {
+function postCreateInstance( /* String */ name, /* String */ port, /* String */ location, 
+		/* String */ features, /* String */ featureURLs ) {
+    $.post( pluginRoot, {"action": "create", "name": name, "port": port, "location": location, 
+                             "features": features, "featureURLs": featureURLs }, function( data ) {
         renderData( data );
     }, "json" );
 }
diff --git a/karaf/webconsole/admin/src/test/java/org/apache/felix/karaf/webconsole/admin/AdminPluginTest.java b/karaf/webconsole/admin/src/test/java/org/apache/felix/karaf/webconsole/admin/AdminPluginTest.java
new file mode 100644
index 0000000..3690e6b
--- /dev/null
+++ b/karaf/webconsole/admin/src/test/java/org/apache/felix/karaf/webconsole/admin/AdminPluginTest.java
@@ -0,0 +1,95 @@
+/*
+ * 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.webconsole.admin;
+
+import java.io.ByteArrayOutputStream;
+import java.io.PrintWriter;
+import java.lang.reflect.Method;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+
+import junit.framework.TestCase;
+
+import org.apache.felix.karaf.shell.admin.AdminService;
+import org.apache.felix.karaf.shell.admin.Instance;
+import org.apache.felix.karaf.shell.admin.InstanceSettings;
+import org.easymock.EasyMock;
+import org.easymock.IAnswer;
+
+public class AdminPluginTest extends TestCase {
+    public void testParseStringList() throws Exception {
+        assertEquals(Arrays.asList("a", "b"), testParseStringList(" a ,b"));
+        assertEquals(Collections.emptyList(), testParseStringList(null));
+        assertEquals(Arrays.asList("hello"), testParseStringList("hello"));
+        assertEquals(Arrays.asList("b"), testParseStringList(",b,"));
+    }
+    
+    @SuppressWarnings("unchecked")
+    private List<String> testParseStringList(String s) throws Exception {
+        AdminPlugin ap = new AdminPlugin();
+        Method m = ap.getClass().getDeclaredMethod("parseStringList", new Class [] {String.class});
+        m.setAccessible(true);
+        return (List<String>) m.invoke(ap, s);
+    }
+    
+    public void testDoPostCreate() throws Exception {
+        InstanceSettings is = 
+            new InstanceSettings(1234, null, Collections.singletonList("http://someURL"), Arrays.asList("abc", "def"));
+        AdminService adminService = EasyMock.createMock(AdminService.class);
+        EasyMock.expect(adminService.createInstance("instance1", is)).andReturn(null);
+        EasyMock.expect(adminService.getInstances()).andReturn(new Instance[] {}).anyTimes();
+        EasyMock.replay(adminService);
+        
+        AdminPlugin ap = new AdminPlugin();
+        ap.setAdminService(adminService);
+
+        final Map<String, String> params = new HashMap<String, String>();
+        params.put("action", "create");
+        params.put("name", "instance1");
+        params.put("port", "1234");
+        params.put("featureURLs", "http://someURL");
+        params.put("features", "abc,def");
+        HttpServletRequest req = EasyMock.createMock(HttpServletRequest.class);
+        EasyMock.expect(req.getParameter((String) EasyMock.anyObject())).andAnswer(new IAnswer<String>() {
+            public String answer() throws Throwable {
+                return params.get(EasyMock.getCurrentArguments()[0]);
+            }
+        }).anyTimes();
+        
+        HttpServletResponse res = EasyMock.createNiceMock(HttpServletResponse.class);
+        ByteArrayOutputStream baos = new ByteArrayOutputStream();
+        PrintWriter pw = new PrintWriter(baos);
+        EasyMock.expect(res.getWriter()).andReturn(pw);
+        
+        EasyMock.replay(req);
+        EasyMock.replay(res);
+        ap.doPost(req, res);        
+        EasyMock.verify(adminService);
+        
+        // Check that the operation has succeeded. This will cause some information to be written to 
+        // the outputstream...
+        pw.flush();
+        String s = new String(baos.toByteArray());
+        assertTrue(s.contains("instance"));
+    }
+}