Eclipse
/
jetty.project
mirror of https://github.com/jetty/jetty.project.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

452503 - Start.jar --add-to-start=jstl results in GraphException: Unable to expand property in name: jsp-impl/${jsp-impl}-jstl 

+ Preserving [depend] declaration order by switching from Set to List
 + Switching from BFS based selection / resolve logic in node selection
   to DFS based, to allow parent nodes to resolve completely before
   attempting other parent nodes.
This commit is contained in:
Joakim Erdfelt 2014-11-20 08:32:55 -07:00
parent 91b8a889e3
commit 9bd0ce1938
10 changed files with 194 additions and 92 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
jetty-distribution/pom.xml
View File

@ -579,7 +579,6 @@
<arguments>
<argument>jetty.home=${assembly-directory}</argument>
<argument>jetty.base=${assembly-directory}</argument>
<argument>jsp-impl=apache</argument>
<argument>--add-to-start=deploy,websocket,ext,resources,jsp,jstl,http</argument>
</arguments>
</configuration>
@ -610,7 +609,6 @@
<arguments>
<argument>jetty.home=${assembly-directory}</argument>
<argument>jetty.base=${assembly-directory}/demo-base</argument>
<argument>jsp-impl=apache</argument>
<argument>--add-to-startd=jsp,jstl,http,https</argument>
</arguments>
</configuration>

2
jetty-distribution/src/main/resources/modules/jstl.mod
View File

@ -1,5 +1,5 @@
#
# Jetty JSP Module
# Jetty JSTL Module
#
[depend]

6
jetty-start/src/main/java/org/eclipse/jetty/start/BaseBuilder.java
View File

@ -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<String> startDNames = new HashSet<>();
List<String> startDNames = new ArrayList<>();
startDNames.addAll(startArgs.getAddToStartdIni());
Set<String> startIniNames = new HashSet<>();
List<String> startIniNames = new ArrayList<>();
startIniNames.addAll(startArgs.getAddToStartIni());
int count = 0;

4
jetty-start/src/main/java/org/eclipse/jetty/start/Module.java
View File

@ -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<Module>
public void expandProperties(Props props)
{
// Expand Parents
Set<String> parents = new HashSet<>();
List<String> parents = new ArrayList<>();
for (String parent : getParentNames())
{
parents.add(props.expand(parent));

3
jetty-start/src/main/java/org/eclipse/jetty/start/Modules.java
View File

@ -137,6 +137,7 @@ public class Modules extends Graph<Module>
@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<Module>
for (Module m : getNodes())
{
Set<String> resolvedParents = new HashSet<>();
List<String> resolvedParents = new ArrayList<>();
for (String parent : m.getParentNames())
{
if (parent.equals(module.getFilesystemRef()))

55
jetty-start/src/main/java/org/eclipse/jetty/start/graph/Graph.java
View File

@ -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<T extends Node<T>> implements Iterable<T>
{
private String selectionTerm = "select";
private String nodeTerm = "node";
private Map<String, T> nodes = new HashMap<>();
private Map<String, T> nodes = new LinkedHashMap<>();
private int maxDepth = -1;
protected Set<String> asNameSet(Set<T> nodeSet)
@ -321,27 +323,6 @@ public abstract class Graph<T extends Node<T>> implements Iterable<T>
*/
public abstract T resolveNode(String name);
private Set<T> resolveNodes(Set<String> parentNames)
{
Set<T> 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<String> resolveParentModulesOf(String nodeName)
{
Map<String, T> ret = new HashMap<>();
@ -361,7 +342,7 @@ public abstract class Graph<T extends Node<T>> implements Iterable<T>
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<T extends Node<T>> implements Iterable<T>
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<T extends Node<T>> implements Iterable<T>
// Walk transitive
Selection transitive = selection.asTransitive();
Set<String> parentNames = new HashSet<>();
List<String> parentNames = new ArrayList<>();
parentNames.addAll(node.getParentNames());
Set<T> parentNodes = resolveNodes(parentNames);
for (T parentNode : parentNodes)
{
count += selectNode(parentNode,transitive);
}
count += selectNodes(parentNames,transitive);
return count;
}
public int selectNodes(Collection<String> 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);
}

35
jetty-start/src/main/java/org/eclipse/jetty/start/graph/Node.java
View File

@ -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<T>
/** The depth of the Node in the tree */
private int depth = 0;
/** The set of selections for how this node was selected */
private Set<Selection> selections = new HashSet<>();
private Set<Selection> selections = new LinkedHashSet<>();
/** Set of Nodes, by name, that this Node depends on */
private Set<String> parentNames = new HashSet<>();
private List<String> parentNames = new ArrayList<>();
/** Set of Nodes, by name, that this Node optionally depend on */
private Set<String> optionalParentNames = new HashSet<>();
private List<String> optionalParentNames = new ArrayList<>();
/** The Edges to parent Nodes */
private Set<T> parentEdges = new HashSet<>();
private Set<T> parentEdges = new LinkedHashSet<>();
/** The Edges to child Nodes */
private Set<T> childEdges = new HashSet<>();
private Set<T> childEdges = new LinkedHashSet<>();
public void addChildEdge(T child)
{
@ -54,6 +57,11 @@ public abstract class Node<T>
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<T>
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<T>
return logicalName;
}
public Set<String> getOptionalParentNames()
public List<String> getOptionalParentNames()
{
return optionalParentNames;
}
@ -108,7 +121,7 @@ public abstract class Node<T>
return parentEdges;
}
public Set<String> getParentNames()
public List<String> getParentNames()
{
return parentNames;
}
@ -143,18 +156,12 @@ public abstract class Node<T>
this.depth = depth;
}
@Deprecated
public void setLogicalName(String logicalName)
{
this.logicalName = logicalName;
}
public void setName(String name)
{
this.logicalName = name;
}
public void setParentNames(Set<String> parents)
public void setParentNames(List<String> parents)
{
this.parentNames.clear();
this.parentEdges.clear();

73
jetty-start/src/test/java/org/eclipse/jetty/start/DistTest.java
View File

@ -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<String> cmds) throws Exception
{
int len = cmds.size();
@ -70,12 +53,12 @@ public class DistTest
main.start(startArgs);
}
public List<String> getBaseCommandLine(File basePath)
public List<String> getBaseCommandLine(Path basePath)
{
List<String> cmds = new ArrayList<String>();
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<String> 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<String> 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<String> 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<String> 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<String> 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<String> cmds = getBaseCommandLine(basePath);

104
jetty-start/src/test/java/org/eclipse/jetty/start/StartMatchers.java Normal file
View File

@ -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<Path> pathExists()
{
return new BaseMatcher<Path>()
{
@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<Path> notPathExists()
{
return new BaseMatcher<Path>()
{
@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<Path> fileExists()
{
return new BaseMatcher<Path>()
{
@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);
}
};
}
}

2
jetty-start/src/test/resources/dist-home/modules/jstl.mod
View File

@ -1,5 +1,5 @@
#
# Jetty JSP Module
# Jetty JSTL Module
#
[depend]