Skip to content

Commit

Permalink
#9946 Stop passing Handler in constructor as a parent (#9948)
Browse files Browse the repository at this point in the history 
#9946 Stop passing Handler in constructor as a parent

Signed-off-by: Ludovic Orban <[email protected]>
  • Loading branch information
@lorban
lorban authored Jun 23, 2023
1 parent 840fd34 commit 8f4a15c
Show file tree
Hide file tree
Showing 59 changed files with 255 additions and 251 deletions.
2 changes: 1 addition & 1 deletion ...entation/src/main/java/org/eclipse/jetty/docs/programming/server/session/SessionDocs.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ public void servletContextWithSessionHandler()
//tag:schsession[]
Server server = new Server();

ServletContextHandler context = new ServletContextHandler(server, "/foo", ServletContextHandler.SESSIONS);
ServletContextHandler context = new ServletContextHandler("/foo", ServletContextHandler.SESSIONS);
SessionHandler sessions = context.getSessionHandler();
//make idle sessions valid for only 5mins
sessions.setMaxInactiveInterval(300);
Expand Down
17 changes: 9 additions & 8 deletions ...rc/main/java/org/eclipse/jetty/docs/programming/server/websocket/WebSocketServerDocs.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ public void standardContainerServletContextHandler() throws Exception
Server server = new Server(8080);

// Create a ServletContextHandler with the given context path.
ServletContextHandler handler = new ServletContextHandler(server, "/ctx");
ServletContextHandler handler = new ServletContextHandler("/ctx");
server.setHandler(handler);

// Ensure that JavaxWebSocketServletContainerInitializer is initialized,
Expand All @@ -82,7 +82,7 @@ public void standardEndpointsInitialization() throws Exception
Server server = new Server(8080);

// Create a ServletContextHandler with the given context path.
ServletContextHandler handler = new ServletContextHandler(server, "/ctx");
ServletContextHandler handler = new ServletContextHandler("/ctx");
server.setHandler(handler);

// Ensure that JavaxWebSocketServletContainerInitializer is initialized,
Expand Down Expand Up @@ -137,7 +137,7 @@ public void standardContainerAndEndpoints() throws Exception
Server server = new Server(8080);

// Create a ServletContextHandler with the given context path.
ServletContextHandler handler = new ServletContextHandler(server, "/ctx");
ServletContextHandler handler = new ServletContextHandler("/ctx");
server.setHandler(handler);

// Setup the ServerContainer and the WebSocket endpoints for this web application context.
Expand Down Expand Up @@ -169,7 +169,7 @@ public void jettyContainerServletContextHandler() throws Exception
Server server = new Server(8080);

// Create a ServletContextHandler with the given context path.
ServletContextHandler handler = new ServletContextHandler(server, "/ctx");
ServletContextHandler handler = new ServletContextHandler("/ctx");
server.setHandler(handler);

// Ensure that JettyWebSocketServletContainerInitializer is initialized,
Expand All @@ -188,7 +188,7 @@ public void jettyEndpointsInitialization() throws Exception
Server server = new Server(8080);

// Create a ServletContextHandler with the given context path.
ServletContextHandler handler = new ServletContextHandler(server, "/ctx");
ServletContextHandler handler = new ServletContextHandler("/ctx");
server.setHandler(handler);

// Ensure that JettyWebSocketServletContainerInitializer is initialized,
Expand Down Expand Up @@ -234,7 +234,7 @@ public void jettyContainerAndEndpoints() throws Exception
Server server = new Server(8080);

// Create a ServletContextHandler with the given context path.
ServletContextHandler handler = new ServletContextHandler(server, "/ctx");
ServletContextHandler handler = new ServletContextHandler("/ctx");
server.setHandler(handler);

// Setup the JettyWebSocketServerContainer and the WebSocket endpoints for this web application context.
Expand Down Expand Up @@ -301,7 +301,7 @@ public void jettyWebSocketServletMain() throws Exception
Server server = new Server(8080);

// Create a ServletContextHandler with the given context path.
ServletContextHandler handler = new ServletContextHandler(server, "/ctx");
ServletContextHandler handler = new ServletContextHandler("/ctx");
server.setHandler(handler);

// Setup the JettyWebSocketServerContainer to initialize WebSocket components.
Expand Down Expand Up @@ -357,7 +357,8 @@ public void uriTemplatePathSpec()
Server server = new Server(8080);

// tag::uriTemplatePathSpec[]
ServletContextHandler handler = new ServletContextHandler(server, "/ctx");
ServletContextHandler handler = new ServletContextHandler("/ctx");
server.setHandler(handler);

// Configure the JettyWebSocketServerContainer.
JettyWebSocketServletContainerInitializer.configure(handler, (servletContext, container) ->
Expand Down
15 changes: 0 additions & 15 deletions jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Handler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -216,21 +216,6 @@ default <T extends Handler.Container> T getContainer(Handler handler, Class<T> t
}
return null;
}

/**
* <p>Make a {@link Container} the parent of a {@link Handler}</p>
* @param parent The {@link Container} that will be the parent
* @param handler The {@link Handler} that will be the child
*/
static void setAsParent(Container parent, Handler handler)
{
if (parent instanceof Collection collection)
collection.addHandler(handler);
else if (parent instanceof Singleton wrapper)
wrapper.setHandler(handler);
else if (parent != null)
throw new IllegalArgumentException("Unknown parent type: " + parent);
}
}

/**
Expand Down
8 changes: 0 additions & 8 deletions jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -180,14 +180,6 @@ public ContextHandler(String contextPath)
_classLoader = classLoader;
}

@Deprecated
public ContextHandler(Handler.Container parent, String contextPath)
{
this(contextPath);
Container.setAsParent(parent, this);

}

@Override
public void setServer(Server server)
{
Expand Down
8 changes: 0 additions & 8 deletions ...core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/MovedContextHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
import org.eclipse.jetty.http.HttpStatus;
import org.eclipse.jetty.http.HttpURI;
import org.eclipse.jetty.http.PreEncodedHttpField;
import org.eclipse.jetty.server.Handler;
import org.eclipse.jetty.server.Request;
import org.eclipse.jetty.server.Response;
import org.eclipse.jetty.util.Callback;
Expand All @@ -42,13 +41,6 @@ public MovedContextHandler()
setAllowNullPathInContext(true);
}

public MovedContextHandler(Handler.Container parent, String contextPath, String redirectURI)
{
Handler.Container.setAsParent(parent, this);
setContextPath(contextPath);
setRedirectURI(redirectURI);
}

/**
* @return the redirect status code, by default 303
*/
Expand Down
172 changes: 83 additions & 89 deletions jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/StopTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import static org.awaitility.Awaitility.await;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.endsWith;
Expand Down Expand Up @@ -92,7 +93,7 @@ public void testWriteDuringShutdown() throws Exception
}
catch (Exception e)
{
e.printStackTrace();
throw new RuntimeException(e);
}
});
stopper.start();
Expand All @@ -107,10 +108,7 @@ public void testWriteDuringShutdown() throws Exception
).getBytes());
client.getOutputStream().flush();

while (!connector.isShutdown())
{
Thread.sleep(10);
}
await().atMost(10, TimeUnit.SECONDS).until(connector::isShutdown);

handler.latchB.countDown();

Expand Down Expand Up @@ -280,11 +278,9 @@ public void testCommittedResponsesAreClosed() throws Exception
LocalConnector connector = new LocalConnector(server);
server.addConnector(connector);

StatisticsHandler stats = new StatisticsHandler();
server.setHandler(stats);

ContextHandler context = new ContextHandler("/");
stats.setHandler(context);
StatisticsHandler stats = new StatisticsHandler(context);
server.setHandler(stats);

Exchanger<Void> exchanger0 = new Exchanger<>();
Exchanger<Void> exchanger1 = new Exchanger<>();
Expand Down Expand Up @@ -314,53 +310,51 @@ public boolean handle(Request request, Response response, Callback callback) thr
server.setStopTimeout(1000);
server.start();

LocalEndPoint endp = connector.executeRequest(
"GET / HTTP/1.1\r\n" +
"Host: localhost\r\n" +
"\r\n"
);

exchanger0.exchange(null);
exchanger1.exchange(null);
try (LocalEndPoint endp = connector.executeRequest(
"""
GET / HTTP/1.1\r
Host: localhost\r
\r
"""
))
{
exchanger0.exchange(null);
exchanger1.exchange(null);

String response = endp.getResponse();
assertThat(response, containsString("200 OK"));
assertThat(response, Matchers.not(containsString("Connection: close")));
String response = endp.getResponse();
assertThat(response, containsString("200 OK"));
assertThat(response, Matchers.not(containsString("Connection: close")));

endp.addInputAndExecute(BufferUtil.toBuffer("GET / HTTP/1.1\r\nHost:localhost\r\n\r\n"));
endp.addInputAndExecute(BufferUtil.toBuffer("GET / HTTP/1.1\r\nHost:localhost\r\n\r\n"));

exchanger0.exchange(null);
exchanger0.exchange(null);

FutureCallback stopped = new FutureCallback();
new Thread(() ->
{
try
FutureCallback stopped = new FutureCallback();
new Thread(() ->
{
server.stop();
stopped.succeeded();
}
catch (Throwable e)
{
stopped.failed(e);
}
}).start();
try
{
server.stop();
stopped.succeeded();
}
catch (Throwable e)
{
stopped.failed(e);
}
}).start();

long start = NanoTime.now();
while (!connector.isShutdown())
{
assertThat(NanoTime.secondsSince(start), lessThan(10L));
Thread.sleep(10);
}
await().atMost(10, TimeUnit.SECONDS).until(connector::isShutdown);

// Check new connections rejected!
assertThrows(IllegalStateException.class, () -> connector.getResponse("GET / HTTP/1.1\r\nHost:localhost\r\n\r\n"));
// Check new connections rejected!
assertThrows(IllegalStateException.class, () -> connector.getResponse("GET / HTTP/1.1\r\nHost:localhost\r\n\r\n"));

// Check completed 200 has close
exchanger1.exchange(null);
response = endp.getResponse();
assertThat(response, containsString("200 OK"));
assertThat(response, Matchers.containsString("Connection: close"));
stopped.get(10, TimeUnit.SECONDS);
// Check completed 200 has close
exchanger1.exchange(null);
response = endp.getResponse();
assertThat(response, containsString("200 OK"));
assertThat(response, Matchers.containsString("Connection: close"));
stopped.get(10, TimeUnit.SECONDS);
}
}

@Test
Expand All @@ -373,7 +367,6 @@ public void testContextStop() throws Exception

ContextHandler context = new ContextHandler("/");
server.setHandler(context);

StatisticsHandler stats = new StatisticsHandler();
context.setHandler(stats);

Expand Down Expand Up @@ -402,50 +395,51 @@ public boolean handle(Request request, Response response, Callback callback) thr

server.start();

LocalEndPoint endp = connector.executeRequest(
"GET / HTTP/1.1\r\n" +
"Host: localhost\r\n" +
"\r\n"
);

exchanger0.exchange(null);
exchanger1.exchange(null);
try (LocalEndPoint endp = connector.executeRequest(
"""
GET / HTTP/1.1\r
Host: localhost\r
\r
"""
))
{
exchanger0.exchange(null);
exchanger1.exchange(null);

String response = endp.getResponse();
assertThat(response, containsString("200 OK"));
assertThat(response, Matchers.not(containsString("Connection: close")));
String response = endp.getResponse();
assertThat(response, containsString("200 OK"));
assertThat(response, Matchers.not(containsString("Connection: close")));

endp.addInputAndExecute(BufferUtil.toBuffer("GET / HTTP/1.1\r\nHost:localhost\r\n\r\n"));
exchanger0.exchange(null);
endp.addInputAndExecute(BufferUtil.toBuffer("GET / HTTP/1.1\r\nHost:localhost\r\n\r\n"));
exchanger0.exchange(null);

CountDownLatch latch = new CountDownLatch(1);
new Thread(() ->
{
try
{
context.stop();
latch.countDown();
}
catch (Exception e)
CountDownLatch latch = new CountDownLatch(1);
new Thread(() ->
{
e.printStackTrace();
}
}).start();
while (context.isStarted())
{
Thread.sleep(10);
}
try
{
context.stop();
latch.countDown();
}
catch (Exception e)
{
e.printStackTrace();
}
}).start();

await().atMost(10, TimeUnit.SECONDS).until(context::isStopped);

// Check new connections accepted, but don't find context!
String unavailable = connector.getResponse("GET / HTTP/1.1\r\nHost:localhost\r\n\r\n");
assertThat(unavailable, containsString(" 404 Not Found"));
// Check new connections accepted, but don't find context!
String unavailable = connector.getResponse("GET / HTTP/1.1\r\nHost:localhost\r\n\r\n");
assertThat(unavailable, containsString(" 404 Not Found"));

// Check completed 200 does not have close
exchanger1.exchange(null);
response = endp.getResponse();
assertThat(response, containsString("200 OK"));
assertThat(response, Matchers.not(Matchers.containsString("Connection: close")));
assertTrue(latch.await(10, TimeUnit.SECONDS));
// Check completed 200 does not have close
exchanger1.exchange(null);
response = endp.getResponse();
assertThat(response, containsString("200 OK"));
assertThat(response, Matchers.not(Matchers.containsString("Connection: close")));
assertTrue(latch.await(10, TimeUnit.SECONDS));
}
}

@Test
Expand Down Expand Up @@ -479,12 +473,12 @@ protected void doStart() throws Exception
ContextHandler context2 = new ContextHandler("/two")
{
@Override
protected void doStart() throws Exception
protected void doStart()
{
context2Started.set(true);
}
};
contexts.setHandlers(new Handler[]{context0, context1, context2});
contexts.setHandlers(context0, context1, context2);

try
{
Expand Down
2 changes: 1 addition & 1 deletion ...0/jetty-ee10-apache-jsp/src/test/java/org/eclipse/jetty/ee10/jsp/TestJettyJspServlet.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ public void setUp() throws Exception
_server = new Server();
_connector = new LocalConnector(_server);
_server.addConnector(_connector);
ServletContextHandler context = new ServletContextHandler(_server, "/context", true, false);
ServletContextHandler context = new ServletContextHandler("/context", true, false);
_server.setHandler(context);
context.setClassLoader(new URLClassLoader(new URL[0], Thread.currentThread().getContextClassLoader()));
ServletHolder jspHolder = context.addServlet(JettyJspServlet.class, "/*");
Expand Down
Loading

0 comments on commit 8f4a15c

Please sign in to comment.