Skip to content

Commit

Permalink
Merge pull request #25489 from Postremus/issues/25462
Browse files Browse the repository at this point in the history 
Refine how resource classes with same components are merged
  • Loading branch information
@geoand
geoand authored Feb 2, 2023
2 parents 22d0448 + e828d27 commit a9070e8
Show file tree
Hide file tree
Showing 2 changed files with 178 additions and 6 deletions.
81 changes: 75 additions & 6 deletions ...c/main/java/org/jboss/resteasy/reactive/server/core/startup/RuntimeDeploymentManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -131,17 +131,20 @@ public BeanFactory.BeanInstance<?> apply(Class<?> aClass) {

//it is possible that multiple resource classes use the same path
//we use this map to merge them
Map<URITemplate, Map<String, TreeMap<URITemplate, List<RequestMapper.RequestPath<RuntimeResource>>>>> mappers = new TreeMap<>();
Map<MappersKey, Map<String, TreeMap<URITemplate, List<RequestMapper.RequestPath<RuntimeResource>>>>> mappers = new TreeMap<>();

for (int i = 0; i < resourceClasses.size(); i++) {
ResourceClass clazz = resourceClasses.get(i);
if ((clazz.getIsDisabled() != null) && clazz.getIsDisabled().get()) {
continue;
}
URITemplate classTemplate = new URITemplate(clazz.getPath(), true);
var perClassMappers = mappers.get(classTemplate);

MappersKey key = new MappersKey(classTemplate);

var perClassMappers = mappers.get(key);
if (perClassMappers == null) {
mappers.put(classTemplate, perClassMappers = new HashMap<>());
mappers.put(key, perClassMappers = new HashMap<>());
}
for (int j = 0; j < clazz.getMethods().size(); j++) {
ResourceMethod method = clazz.getMethods().get(j);
Expand All @@ -153,6 +156,7 @@ public BeanFactory.BeanInstance<?> apply(Class<?> aClass) {
}

}

classMappers = new ArrayList<>(mappers.size());
mappers.forEach(this::forEachMapperEntry);

Expand Down Expand Up @@ -208,14 +212,14 @@ public BeanFactory.BeanInstance<?> apply(Class<?> aClass) {
runtimeConfigurableServerRestHandlers, exceptionMapper, info.isResumeOn404(), info.getResteasyReactiveConfig());
}

private void forEachMapperEntry(URITemplate path,
private void forEachMapperEntry(MappersKey key,
Map<String, TreeMap<URITemplate, List<RequestMapper.RequestPath<RuntimeResource>>>> classTemplates) {
int classTemplateNameCount = path.countPathParamNames();
int classTemplateNameCount = key.path.countPathParamNames();
RuntimeMappingDeployment runtimeMappingDeployment = new RuntimeMappingDeployment(classTemplates);
ClassRoutingHandler classRoutingHandler = new ClassRoutingHandler(runtimeMappingDeployment.buildClassMapper(),
classTemplateNameCount,
info.isResumeOn404());
classMappers.add(new RequestMapper.RequestPath<>(true, path,
classMappers.add(new RequestMapper.RequestPath<>(true, key.path,
new RestInitialHandler.InitialMatch(new ServerRestHandler[] { classRoutingHandler },
runtimeMappingDeployment.getMaxMethodTemplateNameCount() + classTemplateNameCount)));
}
Expand Down Expand Up @@ -267,4 +271,69 @@ private String sanitizePathPrefix(String prefix) {
return prefix;
}

private static class MappersKey implements Comparable<MappersKey> {
private final String key;
private final URITemplate path;

public MappersKey(URITemplate path) {
this.path = path;

if (path.components.length == 0) {
this.key = "";
} else {
// create a key without any names. Names of e.g. default regex components can differ, but the component still has the same meaning.
StringBuilder keyBuilder = new StringBuilder();
for (URITemplate.TemplateComponent component : path.components) {
int standardLength = component.type.name().length() + 1
+ (component.literalText != null ? component.literalText.length() : 0) + 1 + 1;
int additionalLength = 0;
if (component.pattern != null) {
additionalLength = component.pattern.pattern().length();
}
StringBuilder kb = new StringBuilder(standardLength + additionalLength);
kb.append(component.type);
kb.append(";");
kb.append(component.literalText);
kb.append(";");
if (component.pattern != null) {
// (?<id1>[a-zA-Z]+) -> [a-zA-Z]+
String pattern = component.pattern.pattern();
kb.append(component.pattern.pattern(), pattern.indexOf('>') + 1, pattern.length() - 1);
}
kb.append("|");
keyBuilder.append(kb);
}

this.key = keyBuilder.toString();
}

}

@Override
public boolean equals(Object o) {
if (this == o) {
return true;
}
if (o == null) {
return false;
}

return key.equals(((MappersKey) o).key);
}

@Override
public int hashCode() {
return key.hashCode();
}

@Override
public int compareTo(MappersKey o) {
if (key.equals(o.key)) {
return 0;
}

return path.compareTo(o.path);
}
}

}
103 changes: 103 additions & 0 deletions ...t/java/org/jboss/resteasy/reactive/server/vertx/test/matching/ResourceClassMergeTest.java
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
package org.jboss.resteasy.reactive.server.vertx.test.matching;

import static io.restassured.RestAssured.given;
import static org.hamcrest.Matchers.is;

import java.util.function.Supplier;

import javax.ws.rs.GET;
import javax.ws.rs.Path;
import javax.ws.rs.PathParam;
import javax.ws.rs.Produces;
import javax.ws.rs.core.MediaType;
import javax.ws.rs.core.Response;

import org.jboss.resteasy.reactive.server.vertx.test.framework.ResteasyReactiveUnitTest;
import org.jboss.shrinkwrap.api.ShrinkWrap;
import org.jboss.shrinkwrap.api.spec.JavaArchive;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;

public class ResourceClassMergeTest {
@RegisterExtension
static ResteasyReactiveUnitTest test = new ResteasyReactiveUnitTest()
.setArchiveProducer(new Supplier<>() {
@Override
public JavaArchive get() {
return ShrinkWrap.create(JavaArchive.class)
.addClasses(MatchDefaultRegexDifferentNameResourceA.class,
MatchDefaultRegexDifferentNameResourceB.class, MatchCustomRegexDifferentNameResourceA.class,
MatchCustomRegexDifferentNameResourceB.class);
}
});

@Test
public void testCallMatchDefaultRegexDifferentNameResource() {
given()
.when().get("routing-broken/abc/some/other/path")
.then()
.statusCode(200)
.body(is("abc"));

given()
.when().get("routing-broken/efg/some/path")
.then()
.statusCode(200)
.body(is("efg"));
}

@Test
public void testCallMatchCustomRegexDifferentNameResource() {
given()
.when().get("routing-broken-custom-regex/abc/some/other/path")
.then()
.statusCode(200)
.body(is("abc"));

given()
.when().get("routing-broken-custom-regex/efg/some/path")
.then()
.statusCode(200)
.body(is("efg"));
}

@Path("/routing-broken/{id1}")
public static class MatchDefaultRegexDifferentNameResourceA {
@GET
@Path("/some/other/path")
@Produces(MediaType.TEXT_PLAIN)
public Response doSomething(@PathParam("id1") String id) {
return Response.ok(id).build();
}
}

@Path("/routing-broken/{id}")
public static class MatchDefaultRegexDifferentNameResourceB {
@GET
@Path("/some/path")
@Produces(MediaType.TEXT_PLAIN)
public Response doSomething(@PathParam("id") String id) {
return Response.ok(id).build();
}
}

@Path("/routing-broken-custom-regex/{id1: [a-zA-Z]+}")
public static class MatchCustomRegexDifferentNameResourceA {
@GET
@Path("/some/other/path")
@Produces(MediaType.TEXT_PLAIN)
public Response doSomething(@PathParam("id1") String id) {
return Response.ok(id).build();
}
}

@Path("/routing-broken-custom-regex/{id: [a-zA-Z]+}")
public static class MatchCustomRegexDifferentNameResourceB {
@GET
@Path("/some/path")
@Produces(MediaType.TEXT_PLAIN)
public Response doSomething(@PathParam("id") String id) {
return Response.ok(id).build();
}
}
}

0 comments on commit a9070e8

Please sign in to comment.