Skip to content

Commit

Permalink
[UNDERTOW-2303] Renamed some classes and methods, moved some inner cl…
Browse files Browse the repository at this point in the history
…asses to their own files, improved JavaDoc and comments, other minor improvemets.
  • Loading branch information
dirkroets committed May 16, 2024
1 parent abf7d90 commit 9194944
Show file tree
Hide file tree
Showing 9 changed files with 3,093 additions and 2,726 deletions.
81 changes: 42 additions & 39 deletions core/src/main/java/io/undertow/server/RoutingHandler.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,10 @@
import io.undertow.util.HttpString;
import io.undertow.util.Methods;
import io.undertow.util.PathTemplateMatch;
import io.undertow.util.PathTemplatePatternEqualsAdapter;
import io.undertow.util.PathTemplateRouter;
import io.undertow.util.PathTemplaterRouteResult;
import io.undertow.util.PathTemplateRouterFactory;
import java.util.Collections;
import java.util.HashMap;
import java.util.LinkedList;
Expand All @@ -38,7 +41,7 @@
* A Handler that handles the common case of routing via path template and method name.
*
* @author Dirk Roets. This class was originally written by Stuart Douglas. After the introduction of
* {@link PathTemplateRouter}, it was rewritten against the original interface and tests.
* {@link PathTemplateRouterFactory}, it was rewritten against the original interface and tests.
*/
public class RoutingHandler implements HttpHandler {

Expand Down Expand Up @@ -100,12 +103,12 @@ public RoutingMatch get() {
//<editor-fold defaultstate="collapsed" desc="Routers inner class">
private static class Routers {

private final Map<HttpString, PathTemplateRouter.Router<RoutingMatch>> methodRouters;
private final PathTemplateRouter.Router<Object> allMethodsRouter;
private final Map<HttpString, PathTemplateRouter<RoutingMatch>> methodRouters;
private final PathTemplateRouter<Object> allMethodsRouter;

private Routers(
final Map<HttpString, PathTemplateRouter.Router<RoutingMatch>> methodRouters,
final PathTemplateRouter.Router<Object> allMethodsRouter
final Map<HttpString, PathTemplateRouter<RoutingMatch>> methodRouters,
final PathTemplateRouter<Object> allMethodsRouter
) {
this.methodRouters = Objects.requireNonNull(methodRouters);
this.allMethodsRouter = Objects.requireNonNull(allMethodsRouter);
Expand All @@ -121,7 +124,7 @@ public RoutingMatch get() {
return noRoutingMatch;
}
};
private final Map<HttpString, PathTemplateRouter.Builder<RoutingMatchBuilder, RoutingMatch>> methodRouterBuilders
private final Map<HttpString, PathTemplateRouterFactory.Builder<RoutingMatchBuilder, RoutingMatch>> methodRouterBuilders
= new HashMap<>();

// The routers to use.
Expand All @@ -142,7 +145,7 @@ public RoutingHandler(final boolean rewriteQueryParameters) {
this.rewriteQueryParameters = rewriteQueryParameters;
this.routers = new Routers(
Map.of(),
PathTemplateRouter.SimpleBuilder.newBuilder(new Object()).build()
PathTemplateRouterFactory.SimpleBuilder.newBuilder(new Object()).build()
);
}

Expand Down Expand Up @@ -172,8 +175,8 @@ private void handleNoMatch(
final Routers routers,
final HttpServerExchange exchange
) throws Exception {
final PathTemplateRouter.RouteResult<Object> routeResult = routers.allMethodsRouter
.apply(exchange.getRelativePath());
final PathTemplaterRouteResult<Object> routeResult = routers.allMethodsRouter
.route(exchange.getRelativePath());
if (routeResult.getPathTemplate().isPresent()) {
handlInvalidMethod(exchange);
} else {
Expand All @@ -185,14 +188,14 @@ private void handleNoMatch(
public void handleRequest(final HttpServerExchange exchange) throws Exception {
final Routers localRouters = this.routers;

final PathTemplateRouter.Router<RoutingMatch> methodRouter = localRouters.methodRouters
final PathTemplateRouter<RoutingMatch> methodRouter = localRouters.methodRouters
.get(exchange.getRequestMethod());
if (methodRouter == null) {
handleNoMatch(localRouters, exchange);
return;
}

final PathTemplateRouter.RouteResult<RoutingMatch> routeResult = methodRouter.apply(exchange.getRelativePath());
final PathTemplaterRouteResult<RoutingMatch> routeResult = methodRouter.route(exchange.getRelativePath());
if (routeResult.getPathTemplate().isEmpty()) {
handleNoMatch(localRouters, exchange);
return;
Expand Down Expand Up @@ -220,14 +223,14 @@ public void handleRequest(final HttpServerExchange exchange) throws Exception {
handleFallback(exchange);
}

private PathTemplateRouter.Builder<RoutingMatchBuilder, RoutingMatch> getOrAddMethodRouterBuiler(
private PathTemplateRouterFactory.Builder<RoutingMatchBuilder, RoutingMatch> getOrAddMethodRouterBuiler(
final HttpString method
) {
Objects.requireNonNull(method);

PathTemplateRouter.Builder<RoutingMatchBuilder, RoutingMatch> result = methodRouterBuilders.get(method);
PathTemplateRouterFactory.Builder<RoutingMatchBuilder, RoutingMatch> result = methodRouterBuilders.get(method);
if (result == null) {
result = PathTemplateRouter.Builder.newBuilder().updateDefaultTargetFactory(noRoutingMatchBuilder);
result = PathTemplateRouterFactory.Builder.newBuilder().updateDefaultTargetFactory(noRoutingMatchBuilder);
methodRouterBuilders.put(method, result);
}
return result;
Expand All @@ -239,9 +242,9 @@ private RoutingMatchBuilder getOrAddMethodRoutingMatchBuilder(
) {
Objects.requireNonNull(template);

final PathTemplateRouter.Builder<RoutingMatchBuilder, RoutingMatch> routeBuilder
final PathTemplateRouterFactory.Builder<RoutingMatchBuilder, RoutingMatch> routeBuilder
= getOrAddMethodRouterBuiler(method);
final PathTemplateRouter.Template<RoutingMatchBuilder> parsedTemplate = PathTemplateRouter.parseTemplate(
final PathTemplateRouterFactory.Template<RoutingMatchBuilder> parsedTemplate = PathTemplateRouterFactory.parseTemplate(
template, new RoutingMatchBuilder()
);

Expand All @@ -254,45 +257,45 @@ template, new RoutingMatchBuilder()
return parsedTemplate.getTarget();
}

private Map<HttpString, PathTemplateRouter.Router<RoutingMatch>> createMethodRouters() {
final Map<HttpString, PathTemplateRouter.Router<RoutingMatch>> result = new HashMap<>(
private Map<HttpString, PathTemplateRouter<RoutingMatch>> createMethodRouters() {
final Map<HttpString, PathTemplateRouter<RoutingMatch>> result = new HashMap<>(
(int) (methodRouterBuilders.size() / 0.75d) + 1
);
for (final Entry<HttpString, PathTemplateRouter.Builder<RoutingMatchBuilder, RoutingMatch>> entry
for (final Entry<HttpString, PathTemplateRouterFactory.Builder<RoutingMatchBuilder, RoutingMatch>> entry
: methodRouterBuilders.entrySet()) {
result.put(entry.getKey(), entry.getValue().build());
}
return Collections.unmodifiableMap(result);
}

private static <A> Consumer<PathTemplateRouter.Template<A>> createAddTemplateIfAbsentConsumer(
final PathTemplateRouter.SimpleBuilder<Object> builder,
private static <A> Consumer<PathTemplateRouterFactory.Template<A>> createAddTemplateIfAbsentConsumer(
final PathTemplateRouterFactory.SimpleBuilder<Object> builder,
final Supplier<Object> targetFactory
) {
Objects.requireNonNull(builder);
Objects.requireNonNull(targetFactory);

return (final PathTemplateRouter.Template<A> item) -> {
return (final PathTemplateRouterFactory.Template<A> item) -> {
final String template = item.getPathTemplate();
final PathTemplateRouter.Template<Supplier<Object>> parsedTemplate = PathTemplateRouter.parseTemplate(
final PathTemplateRouterFactory.Template<Supplier<Object>> parsedTemplate = PathTemplateRouterFactory.parseTemplate(
template, targetFactory
);
final PathTemplateRouter.PatternEqualsAdapter<PathTemplateRouter.Template<Supplier<Object>>> parsedTemplatePattern
= new PathTemplateRouter.PatternEqualsAdapter<>(parsedTemplate);
final PathTemplatePatternEqualsAdapter<PathTemplateRouterFactory.Template<Supplier<Object>>> parsedTemplatePattern
= new PathTemplatePatternEqualsAdapter<>(parsedTemplate);
if (!builder.getTemplates().containsKey(parsedTemplatePattern)) {
builder.getTemplates().put(parsedTemplatePattern, parsedTemplate.getTarget());
}
};
}

private PathTemplateRouter.Router<Object> createAllMethodsRouter() {
private PathTemplateRouter<Object> createAllMethodsRouter() {
final Object target = new Object();
final Supplier<Object> targetFactory = () -> target;
final PathTemplateRouter.SimpleBuilder<Object> builder = PathTemplateRouter.SimpleBuilder
final PathTemplateRouterFactory.SimpleBuilder<Object> builder = PathTemplateRouterFactory.SimpleBuilder
.newBuilder(target);
methodRouterBuilders.values().stream()
.flatMap(b -> b.getTemplates().keySet().stream())
.map(PathTemplateRouter.PatternEqualsAdapter::getElement)
.map(PathTemplatePatternEqualsAdapter::getPattern)
.forEach(createAddTemplateIfAbsentConsumer(builder, targetFactory));
return builder.build();
}
Expand Down Expand Up @@ -389,21 +392,21 @@ public synchronized RoutingHandler addAll(RoutingHandler routingHandler) {
(originally mutable) RoutingMatch class being held by both this RoutingHandler and the original
RoutingHandler. Since the original fields - specifically RoutingMatch.defaultHandler - were not marked
as volatile, that could result in the handle method of this handler using outdated / cached values for
the field. Since the new PathTemplateRouter is immutable, there is a requirement to rebuild it whenever
the field. Since the new PathTemplateRouterFactory is immutable, there is a requirement to rebuild it whenever
its configuration (templates etc) are mutated. Mutating via the original RoutingHandler would - after
having called this method - also result in the router being out of sync with the router builder.
For these reasons, this has been changed to a deep copy. Arguably, developers won't expect to end up with
two RoutingHandlers that are implicitely linked after having called this method anyway. */
synchronized (routingHandler) {
for (final Entry<HttpString, PathTemplateRouter.Builder<RoutingMatchBuilder, RoutingMatch>> outer
for (final Entry<HttpString, PathTemplateRouterFactory.Builder<RoutingMatchBuilder, RoutingMatch>> outer
: routingHandler.methodRouterBuilders.entrySet()) {
final PathTemplateRouter.Builder<RoutingMatchBuilder, RoutingMatch> builder
final PathTemplateRouterFactory.Builder<RoutingMatchBuilder, RoutingMatch> builder
= getOrAddMethodRouterBuiler(outer.getKey());
for (final Entry<PathTemplateRouter.PatternEqualsAdapter<PathTemplateRouter.Template<RoutingMatchBuilder>>, RoutingMatchBuilder> inner
for (final Entry<PathTemplatePatternEqualsAdapter<PathTemplateRouterFactory.Template<RoutingMatchBuilder>>, RoutingMatchBuilder> inner
: outer.getValue().getTemplates().entrySet()) {
builder.addTemplate(
inner.getKey().getElement().getPathTemplate(),
inner.getKey().getElement().getTarget().deepCopy()
inner.getKey().getPattern().getPathTemplate(),
inner.getKey().getPattern().getTarget().deepCopy()
);
}
}
Expand All @@ -415,16 +418,16 @@ private boolean removeIfPresent(final HttpString method, final String path) {
Objects.requireNonNull(method);
Objects.requireNonNull(path);

final PathTemplateRouter.Builder<RoutingMatchBuilder, RoutingMatch> builder = methodRouterBuilders.get(method);
final PathTemplateRouterFactory.Builder<RoutingMatchBuilder, RoutingMatch> builder = methodRouterBuilders.get(method);
if (builder == null) {
return false;
}

final PathTemplateRouter.Template<RoutingMatchBuilder> parsedTemplate = PathTemplateRouter.parseTemplate(
final PathTemplateRouterFactory.Template<RoutingMatchBuilder> parsedTemplate = PathTemplateRouterFactory.parseTemplate(
path, noRoutingMatchBuilder
);
final PathTemplateRouter.PatternEqualsAdapter<PathTemplateRouter.Template<RoutingMatchBuilder>> parsedTemplatePattern
= new PathTemplateRouter.PatternEqualsAdapter<>(parsedTemplate);
final PathTemplatePatternEqualsAdapter<PathTemplateRouterFactory.Template<RoutingMatchBuilder>> parsedTemplatePattern
= new PathTemplatePatternEqualsAdapter<>(parsedTemplate);

if (!builder.getTemplates().containsKey(parsedTemplatePattern)) {
return false;
Expand Down Expand Up @@ -463,7 +466,7 @@ public synchronized RoutingHandler remove(final String path) {
Objects.requireNonNull(path);

boolean removed = false;
for (final Entry<HttpString, PathTemplateRouter.Builder<RoutingMatchBuilder, RoutingMatch>> entry
for (final Entry<HttpString, PathTemplateRouterFactory.Builder<RoutingMatchBuilder, RoutingMatch>> entry
: methodRouterBuilders.entrySet()) {
removed = removeIfPresent(entry.getKey(), path) || removed;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,10 @@
import io.undertow.server.HttpHandler;
import io.undertow.server.HttpServerExchange;
import io.undertow.util.AttachmentKey;
import io.undertow.util.PathTemplatePatternEqualsAdapter;
import io.undertow.util.PathTemplateRouter;
import io.undertow.util.PathTemplaterRouteResult;
import io.undertow.util.PathTemplateRouterFactory;
import java.util.function.Supplier;
import java.util.ArrayList;
import java.util.List;
Expand All @@ -30,11 +33,11 @@

/**
* A drop-in substitute for the old PathTemplateHandler class. Ideally, one should use {@link PathTemplateRouterHandler} by
* instantiating it with a {@link PathTemplateRouter}. This class implements all of the methods from the original
* instantiating it with a {@link PathTemplateRouterFactory}. This class implements all of the methods from the original
* PathTemplateHandler to provide backwards compatibility.
*
* @author Dirk Roets. This class was originally written by Stuart Douglas. After the introduction of
* {@link PathTemplateRouter}, it was rewritten against the original interface and tests.
* {@link PathTemplateRouterFactory}, it was rewritten against the original interface and tests.
*/
public class PathTemplateHandler implements HttpHandler {

Expand All @@ -46,9 +49,9 @@ public class PathTemplateHandler implements HttpHandler {
create(PathTemplateHandler.PathTemplateMatch.class);

private final boolean rewriteQueryParameters;
private final PathTemplateRouter.SimpleBuilder<HttpHandler> builder;
private final PathTemplateRouterFactory.SimpleBuilder<HttpHandler> builder;
private final Object lock = new Object();
private volatile PathTemplateRouter.Router<HttpHandler> router;
private volatile PathTemplateRouter<HttpHandler> router;

/**
* Default constructor. Uses {@link ResponseCodeHandler#HANDLE_404} as the next (default) handler and sets
Expand Down Expand Up @@ -86,7 +89,7 @@ public PathTemplateHandler(final HttpHandler next, final boolean rewriteQueryPar
Objects.requireNonNull(next);

this.rewriteQueryParameters = rewriteQueryParameters;
builder = PathTemplateRouter.SimpleBuilder.newBuilder(next);
builder = PathTemplateRouterFactory.SimpleBuilder.newBuilder(next);
router = builder.build();
}

Expand All @@ -102,7 +105,7 @@ public PathTemplateHandler add(final String uriTemplate, final HttpHandler handl
Objects.requireNonNull(uriTemplate);
Objects.requireNonNull(handler);

// Router builders are not thread-safe, so we need to synchronize.
// PathTemplateRouter builders are not thread-safe, so we need to synchronize.
synchronized (lock) {
builder.addTemplate(uriTemplate, handler);
router = builder.build();
Expand All @@ -121,7 +124,7 @@ public PathTemplateHandler add(final String uriTemplate, final HttpHandler handl
public PathTemplateHandler remove(final String uriTemplate) {
Objects.requireNonNull(uriTemplate);

// Router builders are not thread-safe, so we need to synchronize.
// PathTemplateRouter builders are not thread-safe, so we need to synchronize.
synchronized (lock) {
builder.removeTemplate(uriTemplate);
router = builder.build();
Expand All @@ -132,24 +135,24 @@ public PathTemplateHandler remove(final String uriTemplate) {

@Override
public String toString() {
final List<PathTemplateRouter.PatternEqualsAdapter<PathTemplateRouter.Template<Supplier<HttpHandler>>>> templates
final List<PathTemplatePatternEqualsAdapter<PathTemplateRouterFactory.Template<Supplier<HttpHandler>>>> templates
= new ArrayList<>(builder.getBuilder().getTemplates().keySet());

final StringBuilder sb = new StringBuilder();
sb.append("path-template( ");
if (templates.size() == 1) {
sb.append(templates.get(0).getElement().getPathTemplate()).append(" )");
sb.append(templates.get(0).getPattern().getPathTemplate()).append(" )");
} else {
sb.append('{').append(
templates.stream().map(s -> s.getElement().getPathTemplate()).collect(Collectors.joining(", "))
templates.stream().map(s -> s.getPattern().getPathTemplate()).collect(Collectors.joining(", "))
).append("} )");
}
return sb.toString();
}

@Override
public void handleRequest(final HttpServerExchange exchange) throws Exception {
final PathTemplateRouter.RouteResult<HttpHandler> routeResult = router.apply(exchange.getRelativePath());
final PathTemplaterRouteResult<HttpHandler> routeResult = router.route(exchange.getRelativePath());
if (routeResult.getPathTemplate().isEmpty()) {
// This is the default handler, therefore it doesn't contain path parameters.
routeResult.getTarget().handleRequest(exchange);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
import io.undertow.server.HttpHandler;
import io.undertow.server.HttpServerExchange;
import io.undertow.util.PathTemplateRouter;
import io.undertow.util.PathTemplaterRouteResult;
import io.undertow.util.PathTemplateRouterFactory;
import java.util.Map;
import java.util.Objects;

Expand All @@ -29,11 +31,11 @@
* A handler that matches URI templates.
*
* @author Dirk Roets
* @see PathTemplateRouter
* @see PathTemplateRouterFactory
*/
public class PathTemplateRouterHandler implements HttpHandler {

private final PathTemplateRouter.Router<HttpHandler> router;
private final PathTemplateRouter<HttpHandler> router;
private final boolean rewriteQueryParameters;

/**
Expand All @@ -42,7 +44,7 @@ public class PathTemplateRouterHandler implements HttpHandler {
* to the exchange if this flag is 'true'.
*/
public PathTemplateRouterHandler(
final PathTemplateRouter.Router<HttpHandler> router,
final PathTemplateRouter<HttpHandler> router,
final boolean rewriteQueryParameters
) {
this.router = Objects.requireNonNull(router);
Expand All @@ -51,7 +53,7 @@ public PathTemplateRouterHandler(

@Override
public void handleRequest(final HttpServerExchange exchange) throws Exception {
final PathTemplateRouter.RouteResult<HttpHandler> routeResult = router.apply(exchange.getRelativePath());
final PathTemplaterRouteResult<HttpHandler> routeResult = router.route(exchange.getRelativePath());
if (routeResult.getPathTemplate().isEmpty()) {
// This is the default handler, therefore it doesn't contain path parameters.
routeResult.getTarget().handleRequest(exchange);
Expand Down
Loading

0 comments on commit 9194944

Please sign in to comment.