Skip to content

Commit

Permalink
Optimized path parameter maps to save allocating it if not necessary
Browse files Browse the repository at this point in the history
  • Loading branch information
franz1981 committed Sep 5, 2024
1 parent de06d19 commit fca143e
Show file tree
Hide file tree
Showing 8 changed files with 63 additions and 48 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ private Map<String, RequestParameter> validatePathParams(RoutingContext routingC

for (ParameterValidationRule rule : pathParamsRules.values()) {
String name = rule.getName();
if (routingContext.hasPathParam(name)) {
if (name != null) {
String pathParam = routingContext.pathParam(name);
if (pathParam != null || !rule.isOptional() ) {
RequestParameter parsedParam = rule.validateSingleParam(pathParam);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -187,8 +187,12 @@ private void runPredicates(RoutingContext context) throws BadRequestException {

private Future<Map<String, RequestParameter>> validatePathParams(RoutingContext routingContext) {
// Validation process validate only params that are registered in the validation -> extra params are allowed
Map<String, List<String>> pathParams = new HashMap<>();
routingContext.forEachPathParam((name, params) -> pathParams.put(name, Collections.singletonList(params)));
Map<String, List<String>> pathParams = routingContext
.pathParams()
.entrySet()
.stream()
.map(e -> new SimpleImmutableEntry<>(e.getKey(), Collections.singletonList(e.getValue())))
.collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));

Map<String, RequestParameter> parsedParams = new HashMap<>();

Expand Down
19 changes: 6 additions & 13 deletions vertx-web/src/main/java/io/vertx/ext/web/RoutingContext.java
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@
import java.time.Instant;
import java.util.List;
import java.util.Map;
import java.util.function.BiConsumer;

import static io.vertx.codegen.annotations.GenIgnore.PERMITTED_TYPE;

Expand Down Expand Up @@ -614,11 +613,10 @@ default LanguageHeader preferredLanguage() {
}

/**
* Returns a map of named parameters as defined in path declaration with their actual values
*
* @return the map of named parameters
* Add a new one or replace an existing path parameter
* @throws NullPointerException when the name or value is null
*/
Map<String, String> pathParams();
void addOrReplacePathParam(String name, String value);

/**
* Remove a path parameter
Expand All @@ -627,16 +625,11 @@ default LanguageHeader preferredLanguage() {
boolean removePathParam(String s);

/**
* Iterate over all the path parameters
* Returns an unmodifiable map of named parameters as defined in path declaration with their actual values
*
* @param pathParamsConsumer the consumer for each path parameter
*/
void forEachPathParam(BiConsumer<String, String> pathParamsConsumer);

/**
* @return {@code true} if the context has a path parameter with the specified {@code name}, {@code false} otherwise.
* @return the map of named parameters
*/
boolean hasPathParam(String name);
Map<String, String> pathParams();

/**
* Gets the value of a single path parameter
Expand Down
15 changes: 6 additions & 9 deletions vertx-web/src/main/java/io/vertx/ext/web/impl/RouteState.java
Original file line number Diff line number Diff line change
Expand Up @@ -1030,8 +1030,7 @@ public int matches(RoutingContextImplBase context, String mountPoint, boolean fa
if (!exactPath) {
context.matchRest = m.start("rest");
// always replace
context.pathParams()
.put("*", path.substring(context.matchRest));
context.addOrReplacePathParam("*", path.substring(context.matchRest));
}

if (!isEmpty(groups)) {
Expand Down Expand Up @@ -1162,8 +1161,7 @@ private boolean pathMatches(String mountPoint, RoutingContext ctx) {

if (exactPath) {
// exact path has no "rest"
ctx.pathParams()
.remove("*");
ctx.removePathParam("*");

return pathMatchesExact(thePath, requestPath, pathEndsWithSlash);
} else {
Expand All @@ -1184,17 +1182,16 @@ private boolean pathMatches(String mountPoint, RoutingContext ctx) {
// because the mount path ended with a wildcard we are relaxed in the check
if (thePath.regionMatches(0, requestPath, 0, pathLen - 1)) {
// handle the "rest" as path param *, always known to be empty
ctx.pathParams()
.put("*", "/");
ctx.addOrReplacePathParam("*", "/");
return true;
}
}
}

if (requestPath.startsWith(thePath)) {
// handle the "rest" as path param *
ctx.pathParams()
.put("*", URIDecoder.decodeURIComponent(requestPath.substring(thePath.length()), false));
ctx.addOrReplacePathParam("*",
URIDecoder.decodeURIComponent(requestPath.substring(thePath.length()), false));
return true;
}
return false;
Expand Down Expand Up @@ -1267,7 +1264,7 @@ private void addPathParam(RoutingContext context, String name, String value) {
if (!request.params().contains(name)) {
request.params().add(name, decodedValue);
}
context.pathParams().put(name, decodedValue);
context.addOrReplacePathParam(name, decodedValue);
}

boolean hasNextContextHandler(RoutingContextImplBase context) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,16 @@ public void reroute(HttpMethod method, String path) {
decoratedContext.reroute(method, path);
}

@Override
public void addOrReplacePathParam(String name, String value) {
decoratedContext.addOrReplacePathParam(name, value);
}

@Override
public boolean removePathParam(String s) {
return decoratedContext.removePathParam(s);
}

@Override
public Map<String, String> pathParams() {
return decoratedContext.pathParams();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -441,28 +441,40 @@ public void reroute(HttpMethod method, String path) {
}

@Override
public void forEachPathParam(final BiConsumer<String, String> pathParamsConsumer) {
if (pathParams != null) {
pathParams.forEach(pathParamsConsumer);
}
public void addOrReplacePathParam(final String name, final String value) {
Objects.requireNonNull(name, "name");
Objects.requireNonNull(value, "value");
getOrCreatePathParams().put(name, value);
}

@Override
public boolean hasPathParam(final String name) {
if (pathParams != null) {
return pathParams.containsKey(name);
public Map<String, String> pathParams() {
if (pathParams == null || pathParams.isEmpty()) {
return Collections.emptyMap();
}
return false;
return Collections.unmodifiableMap(pathParams);
}

@Override
public Map<String, String> pathParams() {
return getPathParams();
public boolean removePathParam(String s) {
if (s == null) {
return false;
}
if (pathParams != null) {
return pathParams.remove(s) != null;
}
return false;
}

@Override
public @Nullable String pathParam(String name) {
return getPathParams().get(name);
if (name == null) {
return null;
}
if (pathParams != null) {
return pathParams.get(name);
}
return null;
}

@Override
Expand Down Expand Up @@ -506,9 +518,10 @@ private MultiMap getQueryParams(Charset charset) {
return queryParams;
}

private Map<String, String> getPathParams() {
private Map<String, String> getOrCreatePathParams() {
if (pathParams == null) {
pathParams = new HashMap<>();
// let's start small
pathParams = new HashMap<>(1);
}
return pathParams;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -310,18 +310,18 @@ public void reroute(HttpMethod method, String path) {
}

@Override
public Map<String, String> pathParams() {
return inner.pathParams();
public void addOrReplacePathParam(final String name, final String value) {
inner.addOrReplacePathParam(name, value);
}

@Override
public boolean hasPathParam(final String name) {
return inner.hasPathParam(name);
public boolean removePathParam(final String s) {
return inner.removePathParam(s);
}

@Override
public void forEachPathParam(final BiConsumer<String, String> pathParamsConsumer) {
inner.forEachPathParam(pathParamsConsumer);
public Map<String, String> pathParams() {
return inner.pathParams();
}

@Override
Expand Down
6 changes: 2 additions & 4 deletions vertx-web/src/test/java/io/vertx/ext/web/RouterTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -863,8 +863,7 @@ public void testPercentEncoding() throws Exception {
@Test
public void testPathParamsAreFulfilled() throws Exception {
router.route("/blah/:abc/quux/:def/eep/:ghi").handler(rc -> {
Map<String, String> params = rc.pathParams();
rc.response().setStatusMessage(params.get("abc") + params.get("def") + params.get("ghi")).end();
rc.response().setStatusMessage(rc.pathParam("abc") + rc.pathParam("def") + rc.pathParam("ghi")).end();
});
testPattern("/blah/tim/quux/julien/eep/nick", "timjuliennick");
}
Expand All @@ -877,11 +876,10 @@ public void testPathParamsDoesNotOverrideQueryParam() throws Exception {
final String queryParamValue2 = "queryParamValue2";
final String sep = ",";
router.route("/blah/:" + paramName + "/test").handler(rc -> {
Map<String, String> params = rc.pathParams();
MultiMap queryParams = rc.request().params();
List<String> values = queryParams.getAll(paramName);
String qValue = values.stream().collect(Collectors.joining(sep));
rc.response().setStatusMessage(params.get(paramName) + "|" + qValue).end();
rc.response().setStatusMessage(rc.pathParam(paramName) + "|" + qValue).end();
});
testRequest(HttpMethod.GET,
"/blah/" + pathParamValue + "/test?" + paramName + "=" + queryParamValue1 + "&" + paramName + "=" + queryParamValue2,
Expand Down

0 comments on commit fca143e

Please sign in to comment.