You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@cxf.apache.org by "Ben Noordhuis (JIRA)" <ji...@apache.org> on 2011/03/04 22:29:45 UTC

[jira] Created: (CXF-3379) @Context fails to inject Application instance

@Context fails to inject Application instance
---------------------------------------------

                 Key: CXF-3379
                 URL: https://issues.apache.org/jira/browse/CXF-3379
             Project: CXF
          Issue Type: Bug
          Components: JAX-RS
    Affects Versions: 2.3.3
            Reporter: Ben Noordhuis


Quoting JSR 311:

"The instance of the application-supplied Application subclass can be injected into a class field or method parameter using the @Context annotation. Access to the Application subclass instance allows configuration information to be centralized in that class. Note that this cannot be injected into the Application subclass itself since this would create a circular dependency."

JAXRSUtils.createContextValue() doesn't handle this. This bug exists in 2.3.x and HEAD.

I'd submit a patch but I don't know where (or if) the Application class is registered after it's instantiated by CXFNonSpringJaxrsServlet.

-- 
This message is automatically generated by JIRA.
-
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] Commented: (CXF-3379) @Context fails to inject Application instance

Posted by "Ben Noordhuis (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/CXF-3379?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13003596#comment-13003596 ] 

Ben Noordhuis commented on CXF-3379:
------------------------------------

Sergey, can you mentor me a little?

> you might want to drop contextMessage.getExchange() it can only be null if message.seyExchange(new ExchangeImpl()) was not done in the test, never null in the real case

At the top of JAXRSUtils.createContextValue() this check is done:
{code}
Message contextMessage = m.getExchange() != null ? m.getExchange().getInMessage() : m;
if (contextMessage == null && Boolean.FALSE.equals(m.get(Message.INBOUND_MESSAGE))) {
    contextMessage = m;
}
{code}
So conceivably it could be true that contextMessage == m and m.getExchange() == null, right?

> The only missing bit is the related ThreadLocal implementation

The Application object is a singleton, its thread safety the responsibility of the developer - like servlets, really. How and why does a ThreadLocalProxy enter the mix? (There probably is a splendid reason, I just haven't caught on yet.)

> @Context fails to inject Application instance
> ---------------------------------------------
>
>                 Key: CXF-3379
>                 URL: https://issues.apache.org/jira/browse/CXF-3379
>             Project: CXF
>          Issue Type: Bug
>          Components: JAX-RS
>    Affects Versions: 2.3.3
>            Reporter: Ben Noordhuis
>
> Quoting JSR 311:
> "The instance of the application-supplied Application subclass can be injected into a class field or method parameter using the @Context annotation. Access to the Application subclass instance allows configuration information to be centralized in that class. Note that this cannot be injected into the Application subclass itself since this would create a circular dependency."
> JAXRSUtils.createContextValue() doesn't handle this. This bug exists in 2.3.x and HEAD.
> I'd submit a patch but I don't know where (or if) the Application class is registered after it's instantiated by CXFNonSpringJaxrsServlet.

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] Commented: (CXF-3379) @Context fails to inject Application instance

Posted by "Sergey Beryozkin (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/CXF-3379?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13003901#comment-13003901 ] 

Sergey Beryozkin commented on CXF-3379:
---------------------------------------

Hi Ben

m.getExchange() != null

is most likely (99%) a lame attempt to avoid initializing Messages in tests. It is most likely a dead code.
I can see now few tests failing with NPEs without this check because I just pass new MessageImpl().
But I'd keep it anyway, just to feel safer :-) - so yes, please keep the null guard in your patch too, sorry...

About Applications and singletons.

Do we have the proper visibility guarantee if we have a singleton root resource class injected the same Application reference on every request across multiple threads ? Is field injection an atomic operation at the low level ? So say the thread1 injects the Application reference and then just before it attempts to read this field from the application code another thread2 assigns the same reference to that field. Is there a chance the thread1 will see a corrupted reference or null, not sure - need to review the basics... 

Not an issue for per-request resource classes though...


> @Context fails to inject Application instance
> ---------------------------------------------
>
>                 Key: CXF-3379
>                 URL: https://issues.apache.org/jira/browse/CXF-3379
>             Project: CXF
>          Issue Type: Bug
>          Components: JAX-RS
>    Affects Versions: 2.3.3
>            Reporter: Ben Noordhuis
>
> Quoting JSR 311:
> "The instance of the application-supplied Application subclass can be injected into a class field or method parameter using the @Context annotation. Access to the Application subclass instance allows configuration information to be centralized in that class. Note that this cannot be injected into the Application subclass itself since this would create a circular dependency."
> JAXRSUtils.createContextValue() doesn't handle this. This bug exists in 2.3.x and HEAD.
> I'd submit a patch but I don't know where (or if) the Application class is registered after it's instantiated by CXFNonSpringJaxrsServlet.

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] Issue Comment Edited: (CXF-3379) @Context fails to inject Application instance

Posted by "Sergey Beryozkin (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/CXF-3379?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13003460#comment-13003460 ] 

Sergey Beryozkin edited comment on CXF-3379 at 3/7/11 5:39 PM:
---------------------------------------------------------------

Hi Ben

Please stay with with me for a moment while I'm trying to grasp this idea.
I can see why an Application can be injected in a plain servlet web app for the latter to dynamically get the service objects. So that makes sense.

Actually, MyApplication.getWidgetDao() kind of start making sense too. So we have a root resource and it delegates to that widgetDao... Definitely the non-Spring option :-) but I can see the picture a bit better, thanks.

So yes, the patch looks fine so far, you might want to drop contextMessage.getExchange() it can only be null if message.seyExchange(new ExchangeImpl()) was not done in the test, never null in the real case...Same for the endpoint representation, even on the client side. 

The only missing bit is the related ThreadLocal implementation, to be located in impl.tl subpackage and it will need to be initialized in the InjectionUtils, which is where the thread-safe proxy is initially injected. 

May be the time has come to remove most of those explicit thread local implementations and use generic proxies instead. I'd probably leave ThreadLocalMessageContext only, so that we can avoid NPEs in the JAX-WS/JAX-RS case. Please feel free to open a related JIRA, but probably we will keep explicit TL implementations in 2.4.0 and work on it later on...

thanks   



      was (Author: sergey_beryozkin):
    Hi Ben

Please stay with with me for a moment while I'm trying to grasp this idea.
I can see why an Application can be injected in a plain servlet web app for the latter to dynamically get the service objects. So that makes sense.

Actually, MyApplication.getWidgetDao() kind of start making sense too. So we have a root resource and it delegates to that widgetDao... Definitely the non-Spring option :-) but I can see the picture a bit better, thanks.

So yes, the patch looks fine so far, you might want to drop contextMessage.getExchange() it can only be null if message.seyExchange(new ExchangeImpl()) was not done in the test, never null in the real case...Same for the endpoint representation, even on the client side. 

The only missing bit is the related ThreadLocal implementation, to be located in impl.tl subpackage and it will need to be initialized in the InjectionUtils, which is where the thread-safe proxy is initially injected. 

May be the time has come to remove most of those explicit thread local implementations and use generic proxies instead. I'd probably leave ThreadLocalMessageContext only, so that we can avoid NPEs in the JAX-WS/JAX-RS case. Please feel free to open a related JIRA, but probably we won't do this refactoring before 2.4.0...

thanks   


  
> @Context fails to inject Application instance
> ---------------------------------------------
>
>                 Key: CXF-3379
>                 URL: https://issues.apache.org/jira/browse/CXF-3379
>             Project: CXF
>          Issue Type: Bug
>          Components: JAX-RS
>    Affects Versions: 2.3.3
>            Reporter: Ben Noordhuis
>
> Quoting JSR 311:
> "The instance of the application-supplied Application subclass can be injected into a class field or method parameter using the @Context annotation. Access to the Application subclass instance allows configuration information to be centralized in that class. Note that this cannot be injected into the Application subclass itself since this would create a circular dependency."
> JAXRSUtils.createContextValue() doesn't handle this. This bug exists in 2.3.x and HEAD.
> I'd submit a patch but I don't know where (or if) the Application class is registered after it's instantiated by CXFNonSpringJaxrsServlet.

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] Commented: (CXF-3379) @Context fails to inject Application instance

Posted by "Sergey Beryozkin (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/CXF-3379?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13005245#comment-13005245 ] 

Sergey Beryozkin commented on CXF-3379:
---------------------------------------

Ben - can you please add ThreadLocalApplication to the patch and update the relevant InjectionUtils code ? I'll add a system test myself

cheers, Sergey

> @Context fails to inject Application instance
> ---------------------------------------------
>
>                 Key: CXF-3379
>                 URL: https://issues.apache.org/jira/browse/CXF-3379
>             Project: CXF
>          Issue Type: Bug
>          Components: JAX-RS
>    Affects Versions: 2.3.3
>            Reporter: Ben Noordhuis
>
> Quoting JSR 311:
> "The instance of the application-supplied Application subclass can be injected into a class field or method parameter using the @Context annotation. Access to the Application subclass instance allows configuration information to be centralized in that class. Note that this cannot be injected into the Application subclass itself since this would create a circular dependency."
> JAXRSUtils.createContextValue() doesn't handle this. This bug exists in 2.3.x and HEAD.
> I'd submit a patch but I don't know where (or if) the Application class is registered after it's instantiated by CXFNonSpringJaxrsServlet.

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] Commented: (CXF-3379) @Context fails to inject Application instance

Posted by "Sergey Beryozkin (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/CXF-3379?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13003460#comment-13003460 ] 

Sergey Beryozkin commented on CXF-3379:
---------------------------------------

Hi Ben

Please stay with with me for a moment while I'm trying to grasp this idea.
I can see why an Application can be injected in a plain servlet web app for the latter to dynamically get the service objects. So that makes sense.

Actually, MyApplication.getWidgetDao() kind of start making sense too. So we have a root resource and it delegates to that widgetDao... Definitely the non-Spring option :-) but I can see the picture a bit better, thanks.

So yes, the patch looks fine so far, you might want to drop contextMessage.getExchange() it can only be null if message.seyExchange(new ExchangeImpl()) was not done in the test, never null in the real case...Same for the endpoint representation, even on the client side. 

The only missing bit is the related ThreadLocal implementation, to be located in impl.tl subpackage and it will need to be initialized in the InjectionUtils, which is where the thread-safe proxy is initially injected. 

May be the time has come to remove most of those explicit thread local implementations and use generic proxies instead. I'd probably leave ThreadLocalMessageContext only, so that we can avoid NPEs in the JAX-WS/JAX-RS case. Please feel free to open a related JIRA, but probably we won't do this refactoring before 2.4.0...

thanks   



> @Context fails to inject Application instance
> ---------------------------------------------
>
>                 Key: CXF-3379
>                 URL: https://issues.apache.org/jira/browse/CXF-3379
>             Project: CXF
>          Issue Type: Bug
>          Components: JAX-RS
>    Affects Versions: 2.3.3
>            Reporter: Ben Noordhuis
>
> Quoting JSR 311:
> "The instance of the application-supplied Application subclass can be injected into a class field or method parameter using the @Context annotation. Access to the Application subclass instance allows configuration information to be centralized in that class. Note that this cannot be injected into the Application subclass itself since this would create a circular dependency."
> JAXRSUtils.createContextValue() doesn't handle this. This bug exists in 2.3.x and HEAD.
> I'd submit a patch but I don't know where (or if) the Application class is registered after it's instantiated by CXFNonSpringJaxrsServlet.

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] Resolved: (CXF-3379) @Context fails to inject Application instance

Posted by "Sergey Beryozkin (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/CXF-3379?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Sergey Beryozkin resolved CXF-3379.
-----------------------------------

       Resolution: Fixed
    Fix Version/s: 2.3.4
                   2.4
         Assignee: Sergey Beryozkin

> @Context fails to inject Application instance
> ---------------------------------------------
>
>                 Key: CXF-3379
>                 URL: https://issues.apache.org/jira/browse/CXF-3379
>             Project: CXF
>          Issue Type: Bug
>          Components: JAX-RS
>    Affects Versions: 2.3.3
>            Reporter: Ben Noordhuis
>            Assignee: Sergey Beryozkin
>             Fix For: 2.4, 2.3.4
>
>
> Quoting JSR 311:
> "The instance of the application-supplied Application subclass can be injected into a class field or method parameter using the @Context annotation. Access to the Application subclass instance allows configuration information to be centralized in that class. Note that this cannot be injected into the Application subclass itself since this would create a circular dependency."
> JAXRSUtils.createContextValue() doesn't handle this. This bug exists in 2.3.x and HEAD.
> I'd submit a patch but I don't know where (or if) the Application class is registered after it's instantiated by CXFNonSpringJaxrsServlet.

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] Commented: (CXF-3379) @Context fails to inject Application instance

Posted by "Sergey Beryozkin (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/CXF-3379?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13006195#comment-13006195 ] 

Sergey Beryozkin commented on CXF-3379:
---------------------------------------

Hi Ben

I think it would work but the existing TL support won't help indeed and as you said it is actually not needed if it is injected only once.

You are right, injecting it into the singleton resource only once is exactly what is needed. 
But the patch will need to be modified because it affect the code which does the injection on every request for both singletons and per-request resources but we only need to do it once for singletons.

I have to think how to do it better, especially given that JAX-RS 1.1 also requires supporting the injection into Application instances, so we need to deal with injecting Applications and also with injecting contexts such as UriInfo, etc into the injected Applications, ouch :-)

thanks, Sergey  

> @Context fails to inject Application instance
> ---------------------------------------------
>
>                 Key: CXF-3379
>                 URL: https://issues.apache.org/jira/browse/CXF-3379
>             Project: CXF
>          Issue Type: Bug
>          Components: JAX-RS
>    Affects Versions: 2.3.3
>            Reporter: Ben Noordhuis
>
> Quoting JSR 311:
> "The instance of the application-supplied Application subclass can be injected into a class field or method parameter using the @Context annotation. Access to the Application subclass instance allows configuration information to be centralized in that class. Note that this cannot be injected into the Application subclass itself since this would create a circular dependency."
> JAXRSUtils.createContextValue() doesn't handle this. This bug exists in 2.3.x and HEAD.
> I'd submit a patch but I don't know where (or if) the Application class is registered after it's instantiated by CXFNonSpringJaxrsServlet.

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] Commented: (CXF-3379) @Context fails to inject Application instance

Posted by "Ben Noordhuis (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/CXF-3379?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13003432#comment-13003432 ] 

Ben Noordhuis commented on CXF-3379:
------------------------------------

Sergey, would this be an acceptable approach? Tests and style checks pass. Color diff and raw patch:

https://github.com/bnoordhuis/cxf/compare/trunk...CXF-3379
https://github.com/bnoordhuis/cxf/compare/trunk...CXF-3379.patch

I'll write some unit tests if you give the green light.

> However, I don't understand why would a root resource or provider class want to have Application instance injected ? Can you explain please what you'd use it for ?

For that configuration information the JSR talks about or as the source of service objects in a plain servlet web app (think MyApplication.getWidgetDao() and such). It's a good place for it, right?

> @Context fails to inject Application instance
> ---------------------------------------------
>
>                 Key: CXF-3379
>                 URL: https://issues.apache.org/jira/browse/CXF-3379
>             Project: CXF
>          Issue Type: Bug
>          Components: JAX-RS
>    Affects Versions: 2.3.3
>            Reporter: Ben Noordhuis
>
> Quoting JSR 311:
> "The instance of the application-supplied Application subclass can be injected into a class field or method parameter using the @Context annotation. Access to the Application subclass instance allows configuration information to be centralized in that class. Note that this cannot be injected into the Application subclass itself since this would create a circular dependency."
> JAXRSUtils.createContextValue() doesn't handle this. This bug exists in 2.3.x and HEAD.
> I'd submit a patch but I don't know where (or if) the Application class is registered after it's instantiated by CXFNonSpringJaxrsServlet.

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] Commented: (CXF-3379) @Context fails to inject Application instance

Posted by "Sergey Beryozkin (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/CXF-3379?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13003349#comment-13003349 ] 

Sergey Beryozkin commented on CXF-3379:
---------------------------------------

thanks for opening this JIRA.

CXFNonSpringJaxrsServlet.createServerFromApplication returns Server, so Application instance may be saved as the server.getEndpoint() property. Endpoint can later be retrieved from the current message:
m.getExchange().get(Endpoint.class) and then the property pointing to the Application instance can be checked.

However, I don't understand why would a root resource or provider class want to have Application instance injected ? Can you explain please what you'd use it for ?

thanks, Sergey


> @Context fails to inject Application instance
> ---------------------------------------------
>
>                 Key: CXF-3379
>                 URL: https://issues.apache.org/jira/browse/CXF-3379
>             Project: CXF
>          Issue Type: Bug
>          Components: JAX-RS
>    Affects Versions: 2.3.3
>            Reporter: Ben Noordhuis
>
> Quoting JSR 311:
> "The instance of the application-supplied Application subclass can be injected into a class field or method parameter using the @Context annotation. Access to the Application subclass instance allows configuration information to be centralized in that class. Note that this cannot be injected into the Application subclass itself since this would create a circular dependency."
> JAXRSUtils.createContextValue() doesn't handle this. This bug exists in 2.3.x and HEAD.
> I'd submit a patch but I don't know where (or if) the Application class is registered after it's instantiated by CXFNonSpringJaxrsServlet.

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (CXF-3379) @Context fails to inject Application instance

Posted by "Ben Noordhuis (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/CXF-3379?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13011441#comment-13011441 ] 

Ben Noordhuis commented on CXF-3379:
------------------------------------

Thanks for the heads up, Sergey!

> @Context fails to inject Application instance
> ---------------------------------------------
>
>                 Key: CXF-3379
>                 URL: https://issues.apache.org/jira/browse/CXF-3379
>             Project: CXF
>          Issue Type: Bug
>          Components: JAX-RS
>    Affects Versions: 2.3.3
>            Reporter: Ben Noordhuis
>            Assignee: Sergey Beryozkin
>             Fix For: 2.4, 2.3.4
>
>
> Quoting JSR 311:
> "The instance of the application-supplied Application subclass can be injected into a class field or method parameter using the @Context annotation. Access to the Application subclass instance allows configuration information to be centralized in that class. Note that this cannot be injected into the Application subclass itself since this would create a circular dependency."
> JAXRSUtils.createContextValue() doesn't handle this. This bug exists in 2.3.x and HEAD.
> I'd submit a patch but I don't know where (or if) the Application class is registered after it's instantiated by CXFNonSpringJaxrsServlet.

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (CXF-3379) @Context fails to inject Application instance

Posted by "Sergey Beryozkin (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/CXF-3379?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13010691#comment-13010691 ] 

Sergey Beryozkin commented on CXF-3379:
---------------------------------------

Hi Ben, just FYI, the injection of Contexts into injected Applications is also supported, see
https://issues.apache.org/jira/browse/CXF-3417

thanks, Sergey

> @Context fails to inject Application instance
> ---------------------------------------------
>
>                 Key: CXF-3379
>                 URL: https://issues.apache.org/jira/browse/CXF-3379
>             Project: CXF
>          Issue Type: Bug
>          Components: JAX-RS
>    Affects Versions: 2.3.3
>            Reporter: Ben Noordhuis
>            Assignee: Sergey Beryozkin
>             Fix For: 2.4, 2.3.4
>
>
> Quoting JSR 311:
> "The instance of the application-supplied Application subclass can be injected into a class field or method parameter using the @Context annotation. Access to the Application subclass instance allows configuration information to be centralized in that class. Note that this cannot be injected into the Application subclass itself since this would create a circular dependency."
> JAXRSUtils.createContextValue() doesn't handle this. This bug exists in 2.3.x and HEAD.
> I'd submit a patch but I don't know where (or if) the Application class is registered after it's instantiated by CXFNonSpringJaxrsServlet.

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] Commented: (CXF-3379) @Context fails to inject Application instance

Posted by "Ben Noordhuis (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/CXF-3379?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13005917#comment-13005917 ] 

Ben Noordhuis commented on CXF-3379:
------------------------------------

Sergey, would a ThreadLocalProxy work for Application instances? The other thread-local proxies implement an interface (e.g. Request) and delegate to a thread-local concrete implementation of that interface. But Application objects are already concrete (and derived) so that doesn't work.

The Application object is a singleton, right? Correct me if I'm wrong, but wouldn't it be enough to inject it once into the singleton root resource and be done with it?

> @Context fails to inject Application instance
> ---------------------------------------------
>
>                 Key: CXF-3379
>                 URL: https://issues.apache.org/jira/browse/CXF-3379
>             Project: CXF
>          Issue Type: Bug
>          Components: JAX-RS
>    Affects Versions: 2.3.3
>            Reporter: Ben Noordhuis
>
> Quoting JSR 311:
> "The instance of the application-supplied Application subclass can be injected into a class field or method parameter using the @Context annotation. Access to the Application subclass instance allows configuration information to be centralized in that class. Note that this cannot be injected into the Application subclass itself since this would create a circular dependency."
> JAXRSUtils.createContextValue() doesn't handle this. This bug exists in 2.3.x and HEAD.
> I'd submit a patch but I don't know where (or if) the Application class is registered after it's instantiated by CXFNonSpringJaxrsServlet.

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira