KNL-1284 Allow paths inside WEB-INF for tool registration (sakaiproje…

…ct#2174)

* KNL-1284 Allow paths inside WEB-INF for tool registration

* KNL-1284 tool reg from /tools/ /WEB-INF/tools/

We allow registrations from multiple locations so that contrib tools continue to work without changes but all the core tools can move their files to a better location.

* KNL-1284 Move tool registration files to WEB-INF

This means that the tool registration files are no longer accessible through requests to the webapps directly. This change was done with:

find . -type d -path '*/webapp/tools' | while read dir; do pushd $dir; cd ..; if [ -d WEB-INF ]; then git mv tools WEB-INF; fi; popd; done

kernel/api/src/main/java/org/sakaiproject/util/ToolListener.java

@@ -21,24 +21,25 @@
 
 package org.sakaiproject.util;
 
-import java.io.File;
-import java.util.Iterator;
-import java.util.Set;
+import org.sakaiproject.component.api.ServerConfigurationService;
+import org.sakaiproject.component.cover.ComponentManager;
+import org.sakaiproject.tool.api.ActiveToolManager;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
+import javax.servlet.ServletContext;
 import javax.servlet.ServletContextEvent;
 import javax.servlet.ServletContextListener;
-
-import org.sakaiproject.component.cover.ServerConfigurationService;
-import org.sakaiproject.tool.cover.ActiveToolManager;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
+import java.io.File;
+import java.util.*;
+import java.util.stream.Collectors;
 
 /**
  * <p>
  * Webapp listener to detect webapp-housed tool registration.<br/>
  * SAK-8908:<br/>
  * Re-wrote the contextInitialized() method to add tool localization files to the
- * newly registered tool(s).  These files are required to be in the /tool/ directory
+ * newly registered tool(s).  These files are required to be in the tools directory
  * have have a name of the form [toolId][_][localCode].properties.  This format allows
  * tool litles (etc) to be localized even when multiple tool registrations are included.
  * </p>
@@ -50,42 +51,73 @@
  * &lt;/listener&gt;<br/>
  * </code>
  * </p>
+ * <p>
+ * By default the tools directories looked at in the webapp are /tools/ and /WEB-INF/tools/ . It is
+ * recommended that all tool registration files are put in /WEB-INF/tools/ as the files don't need to
+ * served up by the container, however /tools/ is supported for backwards compatibility. If you wish to
+ * use a custom location set an parameter on the tool listener:
+ * </p>
+ * <p>
+ * <code>
+ * &lt;context-param&gt;<br>
+ * &nbsp;&lt;param-name&gt;org.sakaiproject.util.ToolListener.PATH&lt;/param-name&gt;<br>
+ * &nbsp;&lt;param-value&gt;/mypath/&lt;/param-value&gt;<br>
+ * &lt;/context-param&gt;<br>
+ * </code>
+ * </p>
  */
 public class ToolListener implements ServletContextListener
 {
 	private static final Logger M_log = LoggerFactory.getLogger(ToolListener.class);
 
+	/**
+	 * The content parameter in your web.xml specifying the webapp root relative
+	 * path to look in for the tool registration files.
+	 */
+	public final static String PATH = ToolListener.class.getName()+".PATH";
+
+	private final ActiveToolManager activeToolManager;
+	private final ServerConfigurationService serverConfigurationService;
+
+	public ToolListener()
+	{
+		activeToolManager = ComponentManager.get(ActiveToolManager.class);
+		serverConfigurationService = ComponentManager.get(ServerConfigurationService.class);
+	}
+
+	public ToolListener(ActiveToolManager activeToolManager, ServerConfigurationService serverConfigurationService)
+	{
+		this.activeToolManager = activeToolManager;
+		this.serverConfigurationService = serverConfigurationService;
+	}
+
 	/**
 	 * Initialize.
 	 */
 	public void contextInitialized(ServletContextEvent event)
 	{
+		final String sakaiHomePath = serverConfigurationService.getSakaiHomePath();
 		// The the location of resource and registration files.
-		Set paths = event.getServletContext().getResourcePaths("/tools/");
-		final String sakaiHomePath = ServerConfigurationService.getSakaiHomePath();
-
-		//	First Pass:  Search for tool registration file.
-		if (paths == null) {
-			return;
-		}
+		ServletContext context = event.getServletContext();
+		Set<String> paths = getToolsPaths(context);
+		if (paths == null) return;
 		int registered = 0;
-		for (Iterator i = paths.iterator(); i.hasNext();)
-		{
-			final String path = (String) i.next();
-
+		// First Pass: Search for tool registration files
+		for (final String path : paths) {
 			// skip directories
 			if (path.endsWith("/")) continue;
 
 			// If an XML file, use it as the tool registration file.
-			if (path.endsWith(".xml"))
-			{
-				final File f = new File(sakaiHomePath + path);
-				if(f.exists()) {
-					ActiveToolManager.register(f, event.getServletContext());
+			if (path.endsWith(".xml")) {
+				String file = path.substring(path.lastIndexOf("/") + 1);
+				// overrides are always in a folder called /tools/
+				final File f = new File(sakaiHomePath + "/tools/" + file);
+				if (f.exists()) {
+					activeToolManager.register(f, context);
 					M_log.info("overriding tools configuration: registering tools from resource: " + sakaiHomePath + path);
 				} else {
 					M_log.info("registering tools from resource: " + path);
-					ActiveToolManager.register(event.getServletContext().getResourceAsStream(path), event.getServletContext());
+					activeToolManager.register(context.getResourceAsStream(path), context);
 				}
 				registered++;
 			}
@@ -98,39 +130,82 @@ public void contextInitialized(ServletContextEvent event)
 		}
 
 		//	Second pass, search for message bundles.  Two passes are necessary to make sure the tool is registered first.
-		for (Iterator j = paths.iterator(); j.hasNext();)
-		{
-			String path = (String) j.next();
-
+		for (String path : paths) {
 			// skip directories
 			if (path.endsWith("/")) continue;
 
 			//	Check for a message properties file.
-			if (path.endsWith(".properties"))
-			{
+			if (path.endsWith(".properties")) {
 				//	Extract the tool id from the resource file name.
-				File reg = new File (path);
+				File reg = new File(path);
 				String tn = reg.getName();
-				String tid = null;
+				String tid;
 				if (tn.indexOf('_') == -1)
-					tid = tn.substring (0, tn.lastIndexOf('.'));	//	Default file.
+					tid = tn.substring(0, tn.lastIndexOf('.'));    //	Default file.
 				else
-					tid = tn.substring (0, tn.indexOf('_'));		//	Locale-based file.
+					tid = tn.substring(0, tn.indexOf('_'));        //	Locale-based file.
 
-				String msg = event.getServletContext().getRealPath(path.substring (0, path.lastIndexOf('.'))+".properties");
-				if (tid != null)
-				{
-					ActiveToolManager.setResourceBundle (tid, msg);
-					M_log.info("Added localization resources for " + tid);
-				}
+				String msg = context.getRealPath(path.substring(0, path.lastIndexOf('.')) + ".properties");
+				activeToolManager.setResourceBundle(tid, msg);
+				M_log.info("Added localization " + tn + "resources for " + tid);
 			}
 		}
 	}
 
+	/**
+	 * This looks for the tools folders that shoudl be used.
+	 * @param context The ServletContext
+	 * @return A list of all possible tool registration files or <code>null</code> if the containing folders weren't found.
+	 */
+	private Set<String> getToolsPaths(ServletContext context) {
+		Collection<String> toolFolders = getToolsFolders(context);
+		Set<String> paths = new HashSet<>();
+		toolFolders.stream().map(context::getResourcePaths).filter(files -> files != null).forEach(paths::addAll);
+		if (paths.isEmpty())
+		{
+			// Warn if the listener is setup but no tools found.
+			M_log.warn("No tools folder found: "+
+				toolFolders.stream().map(context::getRealPath).collect(Collectors.joining(", ")));
+			return null;
+		}
+		return paths;
+	}
+
 	/**
 	 * Destroy.
 	 */
 	public void contextDestroyed(ServletContextEvent event)
 	{
 	}
+
+	/**
+	 * This locates the tool registration folder inside the webapp.
+	 * @param context The servlet context.
+	 * @return The standard tool registration folder location or the configured value.
+	 */
+	protected Collection<String> getToolsFolders(ServletContext context)
+	{
+		String path = context.getInitParameter(PATH);
+		Collection<String> paths;
+		if (path == null)
+		{
+			Collection<String> defaultPaths = new LinkedList<>();
+			defaultPaths.add("/WEB-INF/tools/");
+			defaultPaths.add("/tools/");
+			paths = defaultPaths;
+		}
+		else
+		{
+			if (!path.startsWith("/"))
+			{
+				path = "/"+ path;
+			}
+			if (!path.endsWith("/"))
+			{
+				path = path+ "/";
+			}
+			paths = Collections.singleton(path);
+		}
+		return paths;
+	}
 }

kernel/api/src/test/java/org/sakaiproject/util/ToolListenerTest.java

@@ -0,0 +1,172 @@
+/* *********************************************************************************
+ * $URL$
+ * $Id$
+ * *********************************************************************************
+ *
+ * Copyright (c) 2016 Sakai Foundation
+ *
+ * Licensed under the Educational Community 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.opensource.org/licenses/ECL-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.sakaiproject.util;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.runners.MockitoJUnitRunner;
+import org.sakaiproject.component.api.ServerConfigurationService;
+import org.sakaiproject.tool.api.ActiveToolManager;
+
+import javax.servlet.ServletContext;
+import javax.servlet.ServletContextEvent;
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
+import static org.mockito.Mockito.*;
+
+/**
+ * Tests the basic functions of the tool listener
+ */
+@RunWith(MockitoJUnitRunner.class)
+public class ToolListenerTest {
+
+    private ToolListener listener;
+
+    @Mock
+    private ServletContextEvent event;
+
+    @Mock
+    private ServletContext context;
+
+    @Mock
+    private ServerConfigurationService serverConfigurationService;
+
+    @Mock
+    private ActiveToolManager activeToolManager;
+
+    private Path sakaiHome;
+    private Path toolsFolder;
+
+    @Before
+    public void setUp() throws IOException {
+        listener = new ToolListener(activeToolManager, serverConfigurationService);
+        when(event.getServletContext()).thenReturn(context);
+        // Create the tools folder inside our pretend sakai-home
+        sakaiHome = Files.createTempDirectory("ToolListenerTest");
+        toolsFolder = Files.createDirectories(sakaiHome.resolve("tools"));
+        when(serverConfigurationService.getSakaiHomePath()).thenReturn(sakaiHome.toString());
+        doAnswer(invocation -> "/webapp"+ invocation.getArgumentAt(0, String.class))
+                .when(context).getRealPath(anyString());
+    }
+
+    // Testing that it loads inside the /tools folder
+    @Test
+    public void testToolsRegistration() {
+        when(context.getResourcePaths("/tools/")).thenReturn(Stream.of("/tools/", "/tools/sakai-tool.xml").collect(Collectors.toSet()));
+        InputStream inputStream = mock(InputStream.class);
+        when(context.getResourceAsStream("/tools/sakai-tool.xml")).thenReturn(inputStream);
+        listener.contextInitialized(event);
+        verify(activeToolManager).register(inputStream, context);
+    }
+
+    // Testing that it loads inside the /WEB-INF/tools folder
+    @Test
+    public void testWebInfRegistration() {
+        when(context.getResourcePaths("/WEB-INF/tools/")).thenReturn(Stream.of("/WEB-INF/tools/", "/WEB-INF/tools/sakai-tool.xml").collect(Collectors.toSet()));
+        InputStream inputStream = mock(InputStream.class);
+        when(context.getResourceAsStream("/WEB-INF/tools/sakai-tool.xml")).thenReturn(inputStream);
+        listener.contextInitialized(event);
+        verify(activeToolManager).register(inputStream, context);
+    }
+
+    // Testing is doesn't fall over when no registrations found
+    @Test
+    public void testNoRegistration() {
+        when(context.getResourcePaths("/tools/")).thenReturn(Stream.of("/tools/" ).collect(Collectors.toSet()));
+        when(context.getResourcePaths("/WEB-INF/tools/")).thenReturn(Stream.of("/WEB-INF/tools/" ).collect(Collectors.toSet()));
+        InputStream inputStream = mock(InputStream.class);
+        listener.contextInitialized(event);
+        verify(activeToolManager, never()).register(inputStream, context);
+    }
+
+    // Check multiple registrations work across multiple locations.
+    @Test
+    public void testMultipleRegistrations() {
+        when(context.getResourcePaths("/tools/")).thenReturn(Stream.of("/tools/", "/tools/sakai-tool.xml").collect(Collectors.toSet()));
+        when(context.getResourcePaths("/WEB-INF/tools/")).thenReturn(Stream.of("/WEB-INF/tools/", "/WEB-INF/tools/sakai-tool.xml", "/WEB-INF/tools/another-tool.xml").collect(Collectors.toSet()));
+        InputStream inputStream = mock(InputStream.class);
+        when(context.getResourceAsStream(anyString())).thenReturn(inputStream);
+        listener.contextInitialized(event);
+        verify(activeToolManager, times(3)).register(inputStream, context);
+    }
+
+    // Check the folder and bad extension are ignored.
+    @Test
+    public void testIgnoreFiles() {
+        when(context.getResourcePaths("/WEB-INF/tools/")).thenReturn(Stream.of("/WEB-INF/tools/", "/WEB-INF/tools/sakai-tool.ignored", "/WEB-INF/tools/folder/").collect(Collectors.toSet()));
+        InputStream inputStream = mock(InputStream.class);
+        when(context.getResourceAsStream(anyString())).thenReturn(inputStream);
+        listener.contextInitialized(event);
+        verify(activeToolManager, never()).register(inputStream, context);
+    }
+
+    // Check that a custom location is set it doesn't use the standard ones.
+    @Test
+    public void testCustomLocation() {
+        when(context.getInitParameter(ToolListener.PATH)).thenReturn("/custom");
+        when(context.getResourcePaths("/custom/")).thenReturn(Stream.of("/custom/", "/custom/sakai-tool.xml").collect(Collectors.toSet()));
+        InputStream inputStream = mock(InputStream.class);
+        when(context.getResourceAsStream("/custom/sakai-tool.xml")).thenReturn(inputStream);
+        listener.contextInitialized(event);
+        verify(activeToolManager).register(inputStream, context);
+        verify(context, never()).getResourcePaths("/tools/");
+        verify(context, never()).getResourcePaths("/WEB-INF/tools/");
+    }
+
+    // Check that a tool override is loaded from sakai.home ok.
+    @Test
+    public void testToolOverride() throws IOException {
+        Path sakaiTool = Files.createFile(toolsFolder.resolve("sakai-tool.xml"));
+        Path otherTool = Files.createFile(toolsFolder.resolve("other-tool.xml"));
+        when(context.getResourcePaths("/tools/")).thenReturn(Stream.of("/tools/", "/tools/sakai-tool.xml").collect(Collectors.toSet()));
+        when(context.getResourcePaths("/WEB-INF/tools/")).thenReturn(Stream.of("/WEB-INF/tools/", "/WEB-INF/tools/other-tool.xml").collect(Collectors.toSet()));
+        listener.contextInitialized(event);
+        verify(activeToolManager, never()).register(any(InputStream.class), eq(context));
+        verify(activeToolManager).register(sakaiTool.toFile(), context);
+        verify(activeToolManager).register(otherTool.toFile(), context);
+    }
+
+    // Check that locale files are loaded out of the old tools folder.
+    @Test
+    public void testMessageBundle() {
+        when(context.getResourcePaths("/tools/")).thenReturn(Stream.of("/tools/", "/tools/sakai-tool.xml", "/tools/sakai-tool.properties", "/tools/sakai-tool_fr.properties").collect(Collectors.toSet()));
+        listener.contextInitialized(event);
+        verify(activeToolManager).setResourceBundle("sakai-tool", "/webapp/tools/sakai-tool.properties");
+        verify(activeToolManager).setResourceBundle("sakai-tool", "/webapp/tools/sakai-tool_fr.properties");
+    }
+
+    // Check that locale files are loaded out of the new tools folder.
+    @Test
+    public void testMessageBundleWebInf() {
+        when(context.getResourcePaths("/WEB-INF/tools/")).thenReturn(Stream.of("/WEB-INF/tools/", "/WEB-INF/tools/sakai-tool.xml", "/WEB-INF/tools/sakai-tool.properties", "/WEB-INF/tools/sakai-tool_fr.properties").collect(Collectors.toSet()));
+        listener.contextInitialized(event);
+        verify(activeToolManager).setResourceBundle("sakai-tool", "/webapp/WEB-INF/tools/sakai-tool.properties");
+        verify(activeToolManager).setResourceBundle("sakai-tool", "/webapp/WEB-INF/tools/sakai-tool_fr.properties");
+    }
+
+}