Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cleanup of TypeUtil and ContextHandler stop/start #8998

Merged
merged 5 commits into from
Dec 6, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,44 @@ public void testSimpleGET() throws Exception
assertThat(BufferUtil.toString(stream.getResponseContent()), equalTo(helloHandler.getMessage()));
}

@Test
public void testNullPath() throws Exception
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand why we need processMovedPermanently and processByContextHandler both in ContextHandler one sends a 301 and other sends 302 response.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we do. I've removed processByContextHandler in #8978

{
HelloHandler helloHandler = new HelloHandler();
_contextHandler.setHandler(helloHandler);
_server.start();

ConnectionMetaData connectionMetaData = new MockConnectionMetaData(new MockConnector(_server));
HttpChannel channel = new HttpChannelState(connectionMetaData);
MockHttpStream stream = new MockHttpStream(channel);
HttpFields fields = HttpFields.build().add(HttpHeader.HOST, "localhost").asImmutable();
MetaData.Request request = new MetaData.Request("GET", HttpURI.from("http://localhost/ctx"), HttpVersion.HTTP_1_1, fields, 0);
Runnable task = channel.onRequest(request);
task.run();

assertThat(stream.isComplete(), is(true));
assertThat(stream.getFailure(), nullValue());
assertThat(stream.getResponse(), notNullValue());
assertThat(stream.getResponse().getStatus(), equalTo(301));
assertThat(stream.getResponseHeaders().get(HttpHeader.LOCATION), equalTo("/ctx/"));

_contextHandler.stop();
_contextHandler.setAllowNullPathInContext(true);
_contextHandler.start();

stream = new MockHttpStream(channel);
fields = HttpFields.build().add(HttpHeader.HOST, "localhost").asImmutable();
request = new MetaData.Request("GET", HttpURI.from("http://localhost/ctx"), HttpVersion.HTTP_1_1, fields, 0);
task = channel.onRequest(request);
task.run();

assertThat(stream.getResponse().getStatus(), equalTo(200));
assertThat(stream.getResponseHeaders().get(HttpHeader.CONTENT_TYPE), equalTo(MimeTypes.Type.TEXT_PLAIN_UTF_8.asString()));
// The original fields have been recycled.
assertThat(stream.getResponse().getFields().size(), equalTo(0));
assertThat(BufferUtil.toString(stream.getResponseContent()), equalTo(helloHandler.getMessage()));
}

@Test
public void testSetAvailable() throws Exception
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@
import java.util.ServiceConfigurationError;
import java.util.ServiceLoader;
import java.util.function.Function;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import java.util.stream.Stream;
import java.util.stream.StreamSupport;

Expand All @@ -61,6 +63,7 @@ public class TypeUtil
public static final Class<?>[] NO_ARGS = new Class[]{};
public static final int CR = '\r';
public static final int LF = '\n';
private static final Pattern TRAILING_DIGITS = Pattern.compile("^\\D*(\\d+)$");

private static final HashMap<String, Class<?>> name2Class = new HashMap<>();

Expand Down Expand Up @@ -253,7 +256,12 @@ public static String toShortName(Class<?> type)
{
String[] ss = p.split("\\.");
for (String s : ss)
{
b.append(s.charAt(0));
Matcher matcher = TRAILING_DIGITS.matcher(s);
if (matcher.matches())
b.append(matcher.group(1));
}
}
b.append('.');
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

import org.eclipse.jetty.util.resource.Resource;
import org.eclipse.jetty.util.resource.ResourceFactory;
import org.eclipse.jetty.util.test10.TestClass;
import org.hamcrest.Matchers;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
Expand All @@ -27,6 +28,7 @@

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.is;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;
Expand Down Expand Up @@ -236,4 +238,20 @@ public void testGetLocationJavaLangThreadDeathJPMS()
String expectedJavaBase = "/java.base";
assertThat(TypeUtil.getLocationOfClass(java.lang.ThreadDeath.class).toASCIIString(), containsString(expectedJavaBase));
}

public static Stream<Arguments> shortNames()
{
return Stream.of(
Arguments.of(TypeUtilTest.class, "oeju.TypeUtilTest"),
Arguments.of(TestClass.class, "oejut10.TestClass")
);
}

@ParameterizedTest
@MethodSource("shortNames")
public void testToShortName(Class<?> clazz, String shortName)
{
assertThat(TypeUtil.toShortName(clazz), is(shortName));
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
//
// ========================================================================
// Copyright (c) 1995-2022 Mort Bay Consulting Pty Ltd and others.
//
// This program and the accompanying materials are made available under the
// terms of the Eclipse Public License v. 2.0 which is available at
// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0
// which is available at https://www.apache.org/licenses/LICENSE-2.0.
//
// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0
// ========================================================================
//

package org.eclipse.jetty.util.test10;

public class TestClass
{
}
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,6 @@ public enum ContextStatus
private String[] _welcomeFiles;
private ErrorHandler _errorHandler;
private Logger _logger;
private boolean _allowNullPathInfo;
private int _maxFormKeys = Integer.getInteger(MAX_FORM_KEYS_KEY, DEFAULT_MAX_FORM_KEYS);
private int _maxFormContentSize = Integer.getInteger(MAX_FORM_CONTENT_SIZE_KEY, DEFAULT_MAX_FORM_CONTENT_SIZE);
private boolean _compactPath = false;
Expand Down Expand Up @@ -238,6 +237,7 @@ protected ContextHandler(APIContext context,
String contextPath)
{
_coreContextHandler = new CoreContextHandler();
addBean(_coreContextHandler, false);
_apiContext = context == null ? new APIContext() : context;
_initParams = new HashMap<>();
if (contextPath != null)
Expand Down Expand Up @@ -274,7 +274,7 @@ public APIContext getServletContext()
@ManagedAttribute("Checks if the /context is not redirected to /context/")
public boolean getAllowNullPathInfo()
{
return _allowNullPathInfo;
return _coreContextHandler.getAllowNullPathInContext();
}

// TODO this is a thought bubble
Expand All @@ -294,7 +294,7 @@ public boolean isCanonicalEncodingURIs()
*/
public void setAllowNullPathInfo(boolean allowNullPathInfo)
{
_allowNullPathInfo = allowNullPathInfo;
_coreContextHandler.setAllowNullPathInContext(allowNullPathInfo);
}

@Override
Expand Down Expand Up @@ -612,10 +612,16 @@ protected void doStart() throws Exception
// we need to run ourselves in the core context
if (org.eclipse.jetty.server.handler.ContextHandler.getCurrentContext() != _coreContextHandler.getContext())
{
_coreContextHandler.getContext().call(this::doStart, null);
return;
_coreContextHandler.unmanage(this);
_coreContextHandler.start();
_coreContextHandler.manage(this);
}

_coreContextHandler.getContext().call(this::doStartInContext, null);
}

protected void doStartInContext() throws Exception
{
if (_logger == null)
_logger = LoggerFactory.getLogger(ContextHandler.class.getName() + getLogNameSuffix());

Expand All @@ -631,6 +637,57 @@ protected void doStart() throws Exception
contextInitialized();
}

@Override
gregw marked this conversation as resolved.
Show resolved Hide resolved
protected void doStop() throws Exception
{
// If we are being stopped directly (rather than via a start of the CoreContextHandler), then
// we need to stop ourselves in the core context
if (org.eclipse.jetty.server.handler.ContextHandler.getCurrentContext() != _coreContextHandler.getContext())
{
_coreContextHandler.unmanage(this);
_coreContextHandler.stop();
_coreContextHandler.manage(this);
}
_coreContextHandler.getContext().call(this::doStopInContext, null);
}

protected void doStopInContext() throws Exception
{
try
{
stopContext();
contextDestroyed();

// retain only durable listeners
setEventListeners(_durableListeners);
_durableListeners.clear();

if (_errorHandler != null)
_errorHandler.stop();

for (EventListener l : _programmaticListeners)
{
removeEventListener(l);
if (l instanceof ContextScopeListener)
{
try
{
((ContextScopeListener)l).exitScope(_apiContext, null);
}
catch (Throwable e)
{
LOG.warn("Unable to exit scope", e);
}
}
}
_programmaticListeners.clear();
}
finally
{
_contextStatus = ContextStatus.NOTSET;
}
}

private String getLogNameSuffix()
{
// Use display name first
Expand Down Expand Up @@ -770,44 +827,6 @@ protected void callContextDestroyed(ServletContextListener l, ServletContextEven
l.contextDestroyed(e);
}

@Override
protected void doStop() throws Exception
{
try
{
stopContext();
contextDestroyed();

// retain only durable listeners
setEventListeners(_durableListeners);
_durableListeners.clear();

if (_errorHandler != null)
_errorHandler.stop();

for (EventListener l : _programmaticListeners)
{
removeEventListener(l);
if (l instanceof ContextScopeListener)
{
try
{
((ContextScopeListener)l).exitScope(_apiContext, null);
}
catch (Throwable e)
{
LOG.warn("Unable to exit scope", e);
}
}
}
_programmaticListeners.clear();
}
finally
{
_contextStatus = ContextStatus.NOTSET;
}
}

@Override
public void doScope(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException
{
Expand Down Expand Up @@ -1279,20 +1298,7 @@ public String toString()

StringBuilder b = new StringBuilder();

Package pkg = getClass().getPackage();
if (pkg != null)
{
String p = pkg.getName();
if (p.length() > 0)
{
String[] ss = p.split("\\.");
for (String s : ss)
{
b.append(s.charAt(0)).append('.');
}
}
}
b.append(getClass().getSimpleName()).append('@').append(Integer.toString(hashCode(), 16));
b.append(TypeUtil.toShortName(getClass())).append('@').append(Integer.toString(hashCode(), 16));
b.append('{');
if (getDisplayName() != null)
b.append(getDisplayName()).append(',');
Expand Down
Loading