rest
rest copied to clipboard
jakartaee
[spec] Matching Requests to Resource Methods – Fails to match some straightforward case
I've initially opened eclipse-ee4j/jersey#4557 but it was suggested I put this up for discussion here.
Using @Path configuration as following:
@Path("/rest")
public class FooResource {
@GET
@Path("/foo")
@Produces(MediaType.TEXT_PLAIN)
public String foo() {
return "foo";
}
}
@Path("/rest/foo")
public class BarResource {
@GET
@Path("/bar")
@Produces(MediaType.TEXT_PLAIN)
public String bar() {
return "bar";
}
@GET
@Path("/baz")
@Produces(MediaType.TEXT_PLAIN)
public String baz() {
return "baz";
}
}
causes GET /rest/foo to return 404 Not Found, while GET /rest/foo/bar and GET /rest/foo/baz return 200 OK as expected. FWIW, I'm seeing GET /application.wadl:
<resources base="http://localhost:8080/resource-paths/">
<resource path="/rest/foo">
<resource path="/bar">
<method id="bar" name="GET">...</method>
</resource>
<resource path="/baz">
<method id="baz" name="GET">...</method>
</resource>
</resource>
<resource path="/rest">
<resource path="/foo">
<method id="foo" name="GET">...</method>
</resource>
</resource>
</resources>
which doesn't suggest any wrongdoing to me. Changing the @Path declarations as following:
@Path("/rest")
public class BarResource {
@GET
@Path("/foo/bar")
@Produces(MediaType.TEXT_PLAIN)
public String bar() {
return "bar";
}
@GET
@Path("/foo/baz")
@Produces(MediaType.TEXT_PLAIN)
public String baz() {
return "baz";
}
}
appears to work around the problem. Try out resource-paths.zip – a minimal WAR project demonstrating the problem. By default, it packages Jersey but one could use:
mvn package -P !jersey,resteasy
to package RESTEasy, instead.
I've generally traced it down to the JAX-RS specification – § 3.7.2 Request Matching:
Identify a set of candidate root resource classes matching the request: ...
Obtain a set of candidate resource methods for the request:
Input U = final capturing group not yet matched, C′ = {root resouce classes matched so far} Output M = {candidate resource methods}
(a) If U is null or ‘/’... (b) Set E = {}. (c) For each class Z′ in C′ add regular expressions to E for each sub-resource method and locator... (d) Filter E by matching each member against U...
The result of (1) would be an ordererd set:
/rest/foo() BarResource
/rest(/foo) FooResource
The parentheses above visualize U defined in (2). I think it is kinda obvious U used to match in (d) could not/should not be the same for all classes but the spec is written as such it is likely behaving like that. It is either just bad wording of the spec or a general omission that should be revisited so each entry in E should be matched against U for the corresponding class, in my opinion.
resource-paths-2.zip – Includes a cxf profile:
mvn package -P !jersey,cxf
The three implementations (Jersey, RESTEasy, and CXF) appear to behave the same – as per current spec, though it is suboptimal in my opinion.
IIUC what you actually want is that we shall change the spec so matching /rest/foo would work without the workaround? The problem is that this would break all existing applications for the sake of just some new ones that now can omit the workaround. Does not sound like a good enough reason for really touching it.
As far as I can tell the spec is currently broken in a way it makes just a specific case work (I would say inadvertently). I think the spec could be adjusted so that specific case would continue to work but would make other non-conflicting cases work as well. I'll try to revisit the algorithm in the spec and suggest an appropriate change.
At the very least I think this should be considered for future JAX-RS version but I think it could be revisited for JAX-RS 2.1 as well (possibly producing JAX-RS 2.2?).
Note that the algorithm needs to be fast. For that, it has been designed to match-or-miss, no backtracking. Potentially, when there would be many resources, with subresources, the backtracking could be very slow. So when you already match /rest/foo, you find no resource method that applies to you, the algorithm does no backtracking to other root-resource classes. That's why 404. So it works as designed, I would say.
The problem I see is not related to backtracking but using the wrong value to match against:
request URI path = /rest/foo
U = final capturing group not yet matched (step 2)
@Path("/rest/foo") BarResource, U =""@Path("/rest") FooResource, U ="/foo"
Note, this is not explicitly stated in the spec but it appears to use just the first U for matching resource methods in all other classes, thus misses @Path("/foo") FooResource.foo().
Step 1e:
Sort E using the number of literal characters in each member as the primary key (descending order)
Tells you to use /rest/foo.
The backtracking is in your example, it does not matter which resource method is matched first:
@Path("/rest")
public class FooResource {
@Path("/foo")
public Foo foo() {
return new Foo();
}
}
class Foo {
@GET
@Path("/bar")
@Produces(MediaType.TEXT_PLAIN)
public String bar() {
return "bar";
}
}
@Path("/rest/foo")
public class BarResource {
@GET
@Path("/bar")
@Produces(MediaType.TEXT_PLAIN)
public String bar() {
return "bar";
}
Whatever you match, /rest/foo, or just /rest, for /rest/foo/bar you would need to have backtracking, if the bar() method is in the other-than-matched resource only.
Not sure what you consider backtracking here. My problem is the resource method is not matched, although there's no ambiguity. Here are both examples:
Not working
@Path("/rest/foo") BarResource@Path("/bar") bar()@Path("/baz") baz()
@Path("/rest") FooResource@Path("/foo") foo()
Working
@Path("/rest") BarResource@Path("/foo/bar") bar()@Path("/foo/baz") baz()
@Path("/rest") FooResource@Path("/foo") foo()
In both examples, both root resource classes are picked up to analyze further. It is that in the first example the FooResource methods are matched against an improper path (U).
Step 1e:
Sort E using the number of literal characters in each member as the primary key (descending order)
Tells you to use
/rest/foo.
As far as I can tell it just sorts the classes which are then used in step 2:
Input U = final capturing group not yet matched, C′ = {root resouce classes matched so far}
Ah, yes. I must have missed that. Very unfortunate. I still think the given original case should just work, and I don't think the matching would suffer a significant performance penalty – with the workaround I've used here, there's no difference.
IMO it doesn't seem worth adding any more complexity (no matter how much that is) to the algorithm to handle this use case. You may be able to get around this limitation using resource locators.
O.k. So the suggestion is to use:
@Path("/rest")
public class FooResource {
@GET
@Path("/foo")
@Produces(MediaType.TEXT_PLAIN)
public String foo() {
return "foo";
}
@Path("/foo")
public BarResource fooMore() {
return new BarResource();
}
}
public class BarResource {
@GET
@Path("/bar")
@Produces(MediaType.TEXT_PLAIN)
public String bar() {
return "bar";
}
@GET
@Path("/baz")
@Produces(MediaType.TEXT_PLAIN)
public String baz() {
return "baz";
}
}
I don't think the spec would necessarily become more complex, while it would need some change, but implementations would become more user friendly allowing for a wider usage variety and less user frustration. Please close this at your discretion.
So the suggestion is to use:
I may have misunderstood it as with Jersey I'm getting the following:
The following warnings have been detected: WARNING: The resource (or sub resource) Resource{"/foo", 0 child resources, 4 resource methods, 1 sub-resource locator, 4 method handler classes, 0 method handler instances} with path "/foo" contains (sub) resource method(s) and sub resource locator. The resource cannot have both, methods and locator, defined on same path. The locator will be ignored.
The application.wadl contains only:
<resources base="http://localhost:8080/resource-paths/">
<resource path="/rest">
<resource path="/foo">
<resource path="/bar">
<method id="bar" name="GET">...</method>
</resource>
<resource path="/baz">
<method id="baz" name="GET">...</method>
</resource>
</resource>
</resource>
</resources>
although all resources appear to be accessible, incl. GET /rest/foo.
If I understand correctly, for this specific part of the matching algorithm in the specification, it is optimized for performance, not for accuracy. This means that, by design, it will have some false negatives, i.e. not finding a match when there actually is one, as in the example. I think that makes sense though.
The warning in Jersey is because you can't have foo() (resource method) and fooMore() (sub-resource locator) with the same path. So Jersey ignores both. My recommended structure for these paths is to have them all in a single root resource class.
@Path("/rest/foo")
public class FooResource {
@GET
@Produces(MediaType.TEXT_PLAIN)
public String foo() {
return "foo";
}
@GET
@Path("/bar")
@Produces(MediaType.TEXT_PLAIN)
public String bar() {
return "bar";
}
@GET
@Path("/baz")
@Produces(MediaType.TEXT_PLAIN)
public String baz() {
return "baz";
}
}
If you really need to have them in separate classes, you could still use a sub-resource locator method as in the latest example, except the other way around.
public class FooResource {
@GET
@Produces(MediaType.TEXT_PLAIN)
public String foo() {
return "foo";
}
}
@Path("/rest/foo")
public class BarResource {
public FooResource foo() {
return new FooResource();
}
@GET
@Path("/bar")
@Produces(MediaType.TEXT_PLAIN)
public String bar() {
return "bar";
}
@GET
@Path("/baz")
@Produces(MediaType.TEXT_PLAIN)
public String baz() {
return "baz";
}
}
I have not tried this myself but I think this should also work.
These examples are probably not useful to @stanio anymore but I thought it might be useful to anyone else that may come across this issue.
I have not tried this myself but I think this should also work.
As far as I'm aware the sub-resource locator needs a @Path annotation in order to be recognized as such ("3.4.1 Sub Resources"). Whether @Path("") or @Path("/") is o.k. for sub-resource locator – I can't tell from reading the spec, currently.
Yesterday, I started an exercise to implement a standalone Request Matching: one according to the spec, and one as I anticipate it should behave. The purpose is to compare the possible performance differences and assess whether they are really that significant. Going through the spec I encounter unclear stuff, and I find it written the wrong way overall – it should specify rules that guarantee (where possible) unambiguous results and let implementation worry about performance. For example, it seems to me, an implementation could use just a flat mapping to all possible resource methods, rather than delving into resource trees top-to-bottom. But then the specification just demands a specific algorithm and not expected outcomes.
So, I couldn't find the spec mentioning how to handle leading /, or more precisely the lack of it, in @Path templates. The example with WidgetsResource (at the end of "3.7.2 Request Matching") states:
Step 2 Obtain a set of candidate resource methods for the request. Let R({id}) = ([ˆ/]+?)(/.*)?, Input U = /1 and C′ = {WidgetsResource} Output M = {findWidget}
Note the regular expression ([^/]+?)(/.*)? and the input to match /1 – these do not actually match. That is the regular expression doesn't match the complete input. I'm just guessing implementations work using a side-effect of Pattern.find() matching a subsequence of the actual input.
Here's my experiment (small Maven project):
It implements simplified Request Matching – just the path part. It could be extended a bit to filter on request method at the end for the sake of coming up with more complete examples.
All in all, I don't see any performance benefit from the overly restrictive spec algorithm. In the current experiment version, I'm getting ~5% difference that is however inconclusive because the standard error from the sample measurements I'm getting is always bigger than that. Less often, but some of the runs show my algorithm performing better. Given the number of request matching operations generally achieved is very high (~70m/min here) I would not worry about its performance alone being even twice slower. I'm not saying it is not good to be as fast as possible but I think this is best left to the implementations. For example, I've seen the path string matching is probably the slowest part so I've tried to optimize just that (for both reference implementations).
The virtually non-existent performance benefit is not worth crippling the usability of the framework. It is very important to have enough flexibility to match requests to existing and new classes and methods, in my opinion. JAX-RS implementations could still issue warnings if they detect resource path setups that might result in slower matching.
TL;DR: I'm matching all possible methods in depth. (I'm still confused by the earlier backtracking remarks if that's considered backtracking.)
Here are my algorithm changes to the "3.7.2 Request Matching":
-
Skip sorting in (1.e). Sorting is performed just after (3.a);
-
Eliminate the filtering done in (1.f)
By definition, all root resource classes in C′ must be annotated with the same URI path template modulo variable names.
Then don't use a single U input to (2) but let all matches in C' use their own previously matched final group. I don't see any practical performance benefit from this rule – just taking a look at my earlier example to this issue:
Spec-compatible
@Path("/rest") FooResource@Path("/foo") foo()
@Path("/rest") BarResource@Path("/foo/bar") bar()@Path("/foo/baz") baz()
Spec-incompatible
@Path("/rest") FooResource@Path("/foo") foo()
@Path("/rest/foo") BarResource@Path("/bar") bar()@Path("/baz") baz()
In both cases, matching
/rest/fooneeds to pick the two of the root classes then match resource methods from both. -
I've further merged (2.a) and (2.h) into (2.c-d), modulo the shortcut to step (3);
-
Skip sorting in (2.f). Done just after (3.a);
-
Eliminate the (2.g), (2.i) rule for traversing just a single sub-locator, and make (2.j) process all matched locators using their own final capturing group.
Given all of the above, I think the spec could be much simpler, while much more operational. It could still include non-normative suggestions addressing performance concerns.
The attached example includes convenience benchmark shell scripts (win, bash) for running the sample benchmark I'm using. One may compare RequestMatchingSpec.java and RequestMatchingEnhanced.java side-by-side to spot the differences easily.
- Fixed two obvious defects (affecting resource method but not sub-resource method matching;
RequestMatchingSpec.findStep2x(),ResourceInfo()) - Added HTTP method input and filtering on.
Here's a sample:
Sample duration: 180 sec
.
[A] 229011128
[B] 225102304
[A] 220423368
[B] 220146014
[A] 219682546
[B] 234420498
[A] 232207288
[B] 227875578
[A] 241445506
[B] 224556724
[A] – RequestMatchingSpec [B] – RequestMatchingEnhanced
[A] – 10% standard error [B] – 6% standard error ([B] almost always gives a smaller standard error than [A])
1% difference [A] > [B] ([A] better than [B]) – inconclusive
Using the "enhanced" implementation it should be possible to match pretty much whatever combination one could think of, for example:
@Path("/rest") FooResource
@Path("/foo") @GET foo()
@Path("/foo") fooMore() : Foo2Resource
Foo2Resource
@POST foo2()
@Path("") fooMore() : Foo3Resource
@Path("/bar") @POST bar1()
Foo3Resource
@DELETE foo3()
@Path("/bar") @GET bar2()
GET /rest/foo : FooResource.foo()
POST /rest/foo : Foo2Resource.foo2()
DELETE /rest/foo : Foo3Resource.foo3()
GET /rest/foo/bar : Foo3Resource.bar2()
POST /rest/foo/bar : Foo2Resource.bar1()
So, I've thought my comments and analysis are directly related to the original issue I've opened here. I'm commenting and sharing my findings as I haven't yet seen a definitive resolution on it (such as "won't fix"), but I don't want to spam anyone either. I've archived my experiment here, in case anyone's interested: stanio/jaxrs-request-matching.
Just landed here after having experienced this issue in our application. It's the second time I've experienced this issue in my career and could vaguely remember that it might be intentional.
IMHO as a user of JAX-RS it's not clear why this is happening at all and it is very surprising. The fix is also not easy in our case since we need to have different resource classes. Just because they share a common prefix doesn't mean that they need to be related at all.
Thanks for your research @stanio, hope it can be fixed some day! I'll now write a comment in our application why we need to split up paths in an unintuitive way and hope no one will change the code without this issue in mind.
