diff --git a/jetty-distribution/pom.xml b/jetty-distribution/pom.xml index 5bfa8f7ac3e..b6e74215843 100644 --- a/jetty-distribution/pom.xml +++ b/jetty-distribution/pom.xml @@ -579,7 +579,6 @@ jetty.home=${assembly-directory} jetty.base=${assembly-directory} - jsp-impl=apache --add-to-start=deploy,websocket,ext,resources,jsp,jstl,http @@ -610,7 +609,6 @@ jetty.home=${assembly-directory} jetty.base=${assembly-directory}/demo-base - jsp-impl=apache --add-to-startd=jsp,jstl,http,https diff --git a/jetty-distribution/src/main/resources/modules/jstl.mod b/jetty-distribution/src/main/resources/modules/jstl.mod index cb06244f5e0..4694389b995 100644 --- a/jetty-distribution/src/main/resources/modules/jstl.mod +++ b/jetty-distribution/src/main/resources/modules/jstl.mod @@ -1,5 +1,5 @@ # -# Jetty JSP Module +# Jetty JSTL Module # [depend] diff --git a/jetty-start/src/main/java/org/eclipse/jetty/start/BaseBuilder.java b/jetty-start/src/main/java/org/eclipse/jetty/start/BaseBuilder.java index c2db5eee9bd..eaf4011e7d7 100644 --- a/jetty-start/src/main/java/org/eclipse/jetty/start/BaseBuilder.java +++ b/jetty-start/src/main/java/org/eclipse/jetty/start/BaseBuilder.java @@ -23,9 +23,7 @@ import java.net.URI; import java.nio.file.Files; import java.nio.file.Path; import java.util.ArrayList; -import java.util.HashSet; import java.util.List; -import java.util.Set; import org.eclipse.jetty.start.builders.StartDirBuilder; import org.eclipse.jetty.start.builders.StartIniBuilder; @@ -141,9 +139,9 @@ public class BaseBuilder Selection startDirSelection = new Selection(dirSource); Selection startIniSelection = new Selection(iniSource); - Set startDNames = new HashSet<>(); + List startDNames = new ArrayList<>(); startDNames.addAll(startArgs.getAddToStartdIni()); - Set startIniNames = new HashSet<>(); + List startIniNames = new ArrayList<>(); startIniNames.addAll(startArgs.getAddToStartIni()); int count = 0; diff --git a/jetty-start/src/main/java/org/eclipse/jetty/start/Module.java b/jetty-start/src/main/java/org/eclipse/jetty/start/Module.java index e8576791935..e2ae1ff4927 100644 --- a/jetty-start/src/main/java/org/eclipse/jetty/start/Module.java +++ b/jetty-start/src/main/java/org/eclipse/jetty/start/Module.java @@ -28,10 +28,8 @@ import java.text.CollationKey; import java.text.Collator; import java.util.ArrayList; import java.util.Comparator; -import java.util.HashSet; import java.util.List; import java.util.Locale; -import java.util.Set; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -121,7 +119,7 @@ public class Module extends Node public void expandProperties(Props props) { // Expand Parents - Set parents = new HashSet<>(); + List parents = new ArrayList<>(); for (String parent : getParentNames()) { parents.add(props.expand(parent)); diff --git a/jetty-start/src/main/java/org/eclipse/jetty/start/Modules.java b/jetty-start/src/main/java/org/eclipse/jetty/start/Modules.java index ee6666b2539..a4af0192923 100644 --- a/jetty-start/src/main/java/org/eclipse/jetty/start/Modules.java +++ b/jetty-start/src/main/java/org/eclipse/jetty/start/Modules.java @@ -137,6 +137,7 @@ public class Modules extends Graph @Override public void onNodeSelected(Module module) { + StartLog.debug("on node selected: %s [%s]",module.getName(),baseHome.toShortForm(module.getFilesystemRef())); args.parseModule(module); module.expandProperties(args.getProperties()); } @@ -273,7 +274,7 @@ public class Modules extends Graph for (Module m : getNodes()) { - Set resolvedParents = new HashSet<>(); + List resolvedParents = new ArrayList<>(); for (String parent : m.getParentNames()) { if (parent.equals(module.getFilesystemRef())) diff --git a/jetty-start/src/main/java/org/eclipse/jetty/start/graph/Graph.java b/jetty-start/src/main/java/org/eclipse/jetty/start/graph/Graph.java index c77280e981d..cfe2ba265fe 100644 --- a/jetty-start/src/main/java/org/eclipse/jetty/start/graph/Graph.java +++ b/jetty-start/src/main/java/org/eclipse/jetty/start/graph/Graph.java @@ -27,6 +27,7 @@ import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.Iterator; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Set; @@ -34,6 +35,7 @@ import java.util.Stack; import org.eclipse.jetty.start.Props; import org.eclipse.jetty.start.StartLog; +import org.eclipse.jetty.start.Utils; /** * Basic Graph @@ -42,7 +44,7 @@ public abstract class Graph> implements Iterable { private String selectionTerm = "select"; private String nodeTerm = "node"; - private Map nodes = new HashMap<>(); + private Map nodes = new LinkedHashMap<>(); private int maxDepth = -1; protected Set asNameSet(Set nodeSet) @@ -321,27 +323,6 @@ public abstract class Graph> implements Iterable */ public abstract T resolveNode(String name); - private Set resolveNodes(Set parentNames) - { - Set ret = new HashSet<>(); - for (String name : parentNames) - { - T node = get(name); - // Node doesn't exist yet (try to resolve it it just-in-time) - if (node == null) - { - node = resolveNode(name); - } - // Node still doesn't exist? this is now an invalid graph. - if (node == null) - { - throw new GraphException("Missing referenced dependency: " + name); - } - ret.add(node); - } - return ret; - } - public Set resolveParentModulesOf(String nodeName) { Map ret = new HashMap<>(); @@ -361,7 +342,7 @@ public abstract class Graph> implements Iterable err.append(" requested ").append(nodeTerm); err.append("s. ").append(nodePredicate); err.append(" returned no matches."); - System.err.println(err); + StartLog.warn(err.toString()); return count; } @@ -381,11 +362,11 @@ public abstract class Graph> implements Iterable if (node == null) { StringBuilder err = new StringBuilder(); - err.append("WARNING: Cannot ").append(selectionTerm); + err.append("Cannot ").append(selectionTerm); err.append(" requested ").append(nodeTerm); err.append(" [").append(name).append("]: not a valid "); err.append(nodeTerm).append(" name."); - System.err.println(err); + StartLog.warn(err.toString()); return count; } @@ -418,23 +399,35 @@ public abstract class Graph> implements Iterable // Walk transitive Selection transitive = selection.asTransitive(); - Set parentNames = new HashSet<>(); + List parentNames = new ArrayList<>(); parentNames.addAll(node.getParentNames()); - Set parentNodes = resolveNodes(parentNames); - for (T parentNode : parentNodes) - { - count += selectNode(parentNode,transitive); - } + + count += selectNodes(parentNames,transitive); return count; } public int selectNodes(Collection names, Selection selection) { + StartLog.debug("%s [%s] (via %s)",toCap(selectionTerm),Utils.join(names,", "),selection); + int count = 0; for (String name : names) { + T node = get(name); + // Node doesn't exist yet (try to resolve it it just-in-time) + if (node == null) + { + StartLog.debug("resolving node [%s]",name); + node = resolveNode(name); + } + // Node still doesn't exist? this is now an invalid graph. + if (node == null) + { + throw new GraphException("Missing referenced dependency: " + name); + } + count += selectNode(name,selection); } diff --git a/jetty-start/src/main/java/org/eclipse/jetty/start/graph/Node.java b/jetty-start/src/main/java/org/eclipse/jetty/start/graph/Node.java index 6086c1fc777..cae519ef002 100644 --- a/jetty-start/src/main/java/org/eclipse/jetty/start/graph/Node.java +++ b/jetty-start/src/main/java/org/eclipse/jetty/start/graph/Node.java @@ -18,7 +18,10 @@ package org.eclipse.jetty.start.graph; +import java.util.ArrayList; import java.util.HashSet; +import java.util.LinkedHashSet; +import java.util.List; import java.util.Set; /** @@ -31,16 +34,16 @@ public abstract class Node /** The depth of the Node in the tree */ private int depth = 0; /** The set of selections for how this node was selected */ - private Set selections = new HashSet<>(); + private Set selections = new LinkedHashSet<>(); /** Set of Nodes, by name, that this Node depends on */ - private Set parentNames = new HashSet<>(); + private List parentNames = new ArrayList<>(); /** Set of Nodes, by name, that this Node optionally depend on */ - private Set optionalParentNames = new HashSet<>(); + private List optionalParentNames = new ArrayList<>(); /** The Edges to parent Nodes */ - private Set parentEdges = new HashSet<>(); + private Set parentEdges = new LinkedHashSet<>(); /** The Edges to child Nodes */ - private Set childEdges = new HashSet<>(); + private Set childEdges = new LinkedHashSet<>(); public void addChildEdge(T child) { @@ -54,6 +57,11 @@ public abstract class Node public void addOptionalParentName(String name) { + if (this.optionalParentNames.contains(name)) + { + // skip, name already exists + return; + } this.optionalParentNames.add(name); } @@ -69,6 +77,11 @@ public abstract class Node public void addParentName(String name) { + if (this.parentNames.contains(name)) + { + // skip, name already exists + return; + } this.parentNames.add(name); } @@ -98,7 +111,7 @@ public abstract class Node return logicalName; } - public Set getOptionalParentNames() + public List getOptionalParentNames() { return optionalParentNames; } @@ -108,7 +121,7 @@ public abstract class Node return parentEdges; } - public Set getParentNames() + public List getParentNames() { return parentNames; } @@ -143,18 +156,12 @@ public abstract class Node this.depth = depth; } - @Deprecated - public void setLogicalName(String logicalName) - { - this.logicalName = logicalName; - } - public void setName(String name) { this.logicalName = name; } - public void setParentNames(Set parents) + public void setParentNames(List parents) { this.parentNames.clear(); this.parentEdges.clear(); diff --git a/jetty-start/src/test/java/org/eclipse/jetty/start/DistTest.java b/jetty-start/src/test/java/org/eclipse/jetty/start/DistTest.java index c9f17cacb9a..ec330467d5b 100644 --- a/jetty-start/src/test/java/org/eclipse/jetty/start/DistTest.java +++ b/jetty-start/src/test/java/org/eclipse/jetty/start/DistTest.java @@ -18,19 +18,15 @@ package org.eclipse.jetty.start; -import static org.hamcrest.Matchers.*; +import static org.eclipse.jetty.start.StartMatchers.*; import static org.junit.Assert.*; -import java.io.File; -import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; import java.util.ArrayList; import java.util.List; -import org.eclipse.jetty.toolchain.test.IO; import org.eclipse.jetty.toolchain.test.MavenTestingUtils; -import org.eclipse.jetty.toolchain.test.OS; import org.eclipse.jetty.toolchain.test.TestingDir; import org.junit.Ignore; import org.junit.Rule; @@ -47,19 +43,6 @@ public class DistTest @Rule public SystemExitAsException exitrule = new SystemExitAsException(); - protected String assertFileExists(File basePath, String name) throws IOException - { - File file = new File(basePath, OS.separators(name)); - FS.exists(file.toPath()); - return IO.readToString(file); - } - - protected void assertNotFileExists(File basePath, String name) throws IOException - { - File file = new File(basePath, OS.separators(name)); - assertThat("File should not exist: " + file, file.exists(), is(false)); - } - private void execMain(List cmds) throws Exception { int len = cmds.size(); @@ -70,12 +53,12 @@ public class DistTest main.start(startArgs); } - public List getBaseCommandLine(File basePath) + public List getBaseCommandLine(Path basePath) { List cmds = new ArrayList(); cmds.add("-Djava.io.tmpdir=" + MavenTestingUtils.getTargetDir().getAbsolutePath()); cmds.add("-Djetty.home=" + MavenTestingUtils.getTestResourceDir("dist-home").getAbsolutePath()); - cmds.add("-Djetty.base=" + basePath.getAbsolutePath()); + cmds.add("-Djetty.base=" + basePath.normalize().toAbsolutePath().toString()); cmds.add("--testing-mode"); return cmds; @@ -84,7 +67,7 @@ public class DistTest @Test public void testLikeDistro_SetupHome() throws Exception { - File basePath = testdir.getEmptyDir(); + Path basePath = testdir.getEmptyDir().toPath(); List cmds = getBaseCommandLine(basePath); @@ -93,23 +76,38 @@ public class DistTest execMain(cmds); } + @Test + public void testAddJstl() throws Exception + { + StartLog.enableDebug(); + Path basePath = testdir.getEmptyDir().toPath(); + + List cmds = getBaseCommandLine(basePath); + + cmds.add("--add-to-start=jstl"); + + execMain(cmds); + } + /** * Test for https://bugs.eclipse.org/452329 */ @Test public void testReAddServerModule() throws Exception { - File basePath = testdir.getEmptyDir(); + Path basePath = testdir.getEmptyDir().toPath(); List cmds = getBaseCommandLine(basePath); cmds.add("--add-to-startd=http"); execMain(cmds); - assertFileExists(basePath,"start.d/http.ini"); - assertFileExists(basePath,"start.d/server.ini"); + Path httpIni = basePath.resolve("start.d/http.ini"); + Path serverIni = basePath.resolve("start.d/server.ini"); + + assertThat("start.d/http.ini", httpIni, fileExists()); + assertThat("start.d/server.ini", serverIni, fileExists()); // Delete server.ini - Path serverIni = basePath.toPath().resolve("start.d/server.ini"); Files.deleteIfExists(serverIni); // Attempt to re-add via 'server' module reference @@ -117,7 +115,7 @@ public class DistTest cmds.add("--add-to-startd=server"); execMain(cmds); - assertFileExists(basePath,"start.d/server.ini"); + assertThat("start.d/server.ini", serverIni, fileExists()); } /** @@ -126,17 +124,19 @@ public class DistTest @Test public void testReAddServerViaHttpModule() throws Exception { - File basePath = testdir.getEmptyDir(); + Path basePath = testdir.getEmptyDir().toPath(); List cmds = getBaseCommandLine(basePath); cmds.add("--add-to-startd=http"); execMain(cmds); - assertFileExists(basePath,"start.d/http.ini"); - assertFileExists(basePath,"start.d/server.ini"); + Path httpIni = basePath.resolve("start.d/http.ini"); + Path serverIni = basePath.resolve("start.d/server.ini"); + + assertThat("start.d/http.ini", httpIni, fileExists()); + assertThat("start.d/server.ini", serverIni, fileExists()); // Delete server.ini - Path serverIni = basePath.toPath().resolve("start.d/server.ini"); Files.deleteIfExists(serverIni); // Attempt to re-add via 'http' module reference @@ -144,7 +144,7 @@ public class DistTest cmds.add("--add-to-startd=http"); execMain(cmds); - assertFileExists(basePath,"start.d/server.ini"); + assertThat("start.d/server.ini", serverIni, fileExists()); } /** @@ -153,28 +153,31 @@ public class DistTest @Test public void testReAddHttpThenDeployViaStartD() throws Exception { - File basePath = testdir.getEmptyDir(); + Path basePath = testdir.getEmptyDir().toPath(); List cmds = getBaseCommandLine(basePath); cmds.add("--add-to-start=http"); execMain(cmds); - assertFileExists(basePath,"start.ini"); + Path startIni = basePath.resolve("start.ini"); + assertThat("start.ini", startIni, fileExists()); + // Now add 'deploy' module. cmds = getBaseCommandLine(basePath); cmds.add("--add-to-startd=deploy"); execMain(cmds); // The following files should not exist (as its already defined in /start.ini) - assertNotFileExists(basePath,"start.d/server.ini"); + Path serverIni = basePath.resolve("start.d/server.ini"); + assertThat("start.d/server.ini", serverIni, notPathExists()); } @Test @Ignore("See https://bugs.eclipse.org/451973") public void testLikeDistro_SetupDemoBase() throws Exception { - File basePath = testdir.getEmptyDir(); + Path basePath = testdir.getEmptyDir().toPath(); List cmds = getBaseCommandLine(basePath); diff --git a/jetty-start/src/test/java/org/eclipse/jetty/start/StartMatchers.java b/jetty-start/src/test/java/org/eclipse/jetty/start/StartMatchers.java new file mode 100644 index 00000000000..ee9c2122186 --- /dev/null +++ b/jetty-start/src/test/java/org/eclipse/jetty/start/StartMatchers.java @@ -0,0 +1,104 @@ +// +// ======================================================================== +// Copyright (c) 1995-2014 Mort Bay Consulting Pty. Ltd. +// ------------------------------------------------------------------------ +// All rights reserved. This program and the accompanying materials +// are made available under the terms of the Eclipse Public License v1.0 +// and Apache License v2.0 which accompanies this distribution. +// +// The Eclipse Public License is available at +// http://www.eclipse.org/legal/epl-v10.html +// +// The Apache License v2.0 is available at +// http://www.opensource.org/licenses/apache2.0.php +// +// You may elect to redistribute this code under either of these licenses. +// ======================================================================== +// + +package org.eclipse.jetty.start; + +import java.nio.file.Files; +import java.nio.file.Path; + +import org.hamcrest.BaseMatcher; +import org.hamcrest.Description; +import org.hamcrest.Matcher; + +public final class StartMatchers +{ + public static Matcher pathExists() + { + return new BaseMatcher() + { + @Override + public boolean matches(Object item) + { + final Path path = (Path)item; + return Files.exists(path); + } + + @Override + public void describeTo(Description description) + { + description.appendText("Path should exist"); + } + + @Override + public void describeMismatch(Object item, Description description) + { + description.appendText("Path did not exist ").appendValue(item); + } + }; + } + + public static Matcher notPathExists() + { + return new BaseMatcher() + { + @Override + public boolean matches(Object item) + { + final Path path = (Path)item; + return !Files.exists(path); + } + + @Override + public void describeTo(Description description) + { + description.appendText("Path should not exist"); + } + + @Override + public void describeMismatch(Object item, Description description) + { + description.appendText("Path exists ").appendValue(item); + } + }; + } + + public static Matcher fileExists() + { + return new BaseMatcher() + { + @Override + public boolean matches(Object item) + { + final Path path = (Path)item; + return Files.exists(path) && Files.isRegularFile(path); + } + + @Override + public void describeTo(Description description) + { + description.appendText("File should exist"); + } + + @Override + public void describeMismatch(Object item, Description description) + { + description.appendText("File did not exist ").appendValue(item); + } + }; + } +} diff --git a/jetty-start/src/test/resources/dist-home/modules/jstl.mod b/jetty-start/src/test/resources/dist-home/modules/jstl.mod index cb06244f5e0..4694389b995 100644 --- a/jetty-start/src/test/resources/dist-home/modules/jstl.mod +++ b/jetty-start/src/test/resources/dist-home/modules/jstl.mod @@ -1,5 +1,5 @@ # -# Jetty JSP Module +# Jetty JSTL Module # [depend]