[RESTEASY-1865] SynchronousDispatcher #getResource not returning Resource Instance #1520
Conversation
| obj = factory.createResource(request, response, this); | ||
| } | ||
| else | ||
| { |
There was a problem hiding this comment.
I've not indented the else branch as it would have caused very confusing diff
|
Would this be OK for other implementations of |
|
@dansiviter This solution works and was tested for SingletonResource only. To have more generic solution, I advice to create it as a complex enhancement since we don't have any more relevant test cases.provided by the bug reporter. |
| */ | ||
| public <T> T injectedInstance(Class<? extends T> clazz, HttpRequest request, HttpResponse response) | ||
| { | ||
| Constructor<?> constructor = PickConstructor.pickSingletonConstructor(clazz); |
There was a problem hiding this comment.
The creation of the Constructor object should be moved within else part of the if you added below, shouldn't it?
rnetuka
force-pushed
the
register-singleton
branch
from
6dd43f9 to
1287f43
Compare
|
@asoldano Done |
rnetuka
force-pushed
the
register-singleton
branch
3 times, most recently
from
66adf1c to
78f4e7e
Compare
rnetuka
force-pushed
the
register-singleton
branch
from
78f4e7e to
d4024f5
Compare
|
Fixed. Please, review again. Particulary this line: (within changed files) |
|
@rnetuka thanks, sorry for the late reply, I've been on PTO and then catching up. I don't see problems in the line that you mentioned. Now that I think more about this PR, though, I'm thinking that the hashmap you're adding should be made a thread safe... WDYT? |
|
Hi @rnetuka, I don't see anything in the JAX-RS spec (2.0 or 2.1) that suggests that the resource returned by ResourceContext.getResource() should be any particular iinstance. The javadoc says
[My bold emphasis.] Could you elaborate? -Ron |
|
Hi @rnetuka, there are a couple of outstanding questions here. Any thoughts? |
| else root.addInvoker(classExpression, fullpath, locator); | ||
| } | ||
|
|
||
| if (rf instanceof SingletonResource) { |
There was a problem hiding this comment.
Hi @rnetuka,
I'm questioning whether this should be in
protected void register(ResourceFactory rf, String base, ResourceClass resourceClass)
{
for (ResourceMethod method : resourceClass.getResourceMethods())
{
processMethod(rf, base, method);
}
for (ResourceLocator method : resourceClass.getResourceLocators())
{
processMethod(rf, base, method);
}
}
Otherwise, you'll be calling registerSingletonResource() for each method, no?
-Ron
|
Thank you for the contribution. We are cleaning up older PR's that seem to be stale. If you feel this is still an issue please feel free to re-open. |
6 participants
JIRA: https://issues.jboss.org/browse/RESTEASY-1865
dispatcher.getRegistry().addSingletonResource(...) works for ResourceMethodInvoker (returned object is the proper singleton), but not for resourceContext.getResource(...). Here, the object being returned is simply constructed one by ResteasyProviderFactory using a constructor.