Hi,
On Tue, Oct 7, 2014 at 12:38 AM, manfred riem <manfred.riem_at_oracle.com> wrote:
> I would assume the @Inject happens lazily. And the same thing with
> #{facesContext}
Good point to keep in mind. Indeed, in actuality when using a CDI
generated proxy, CDI will just inject a proxy but will not actually
consult the producer yet. This is a detail I left out in the previous
explanation.
The producer will only be consulted when an actual method is called on
the proxy, e.g. facesContext.getApplication();
So there's a small potential performance advantage here when people
think they would need a FacesContext, but then the runtime code paths
don't actually touch the proxy.
Kind regards,
Arjan Tijms
>
> Regards,
> Manfred
>
>
> On 10/6/14, 5:12 PM, arjan tijms wrote:
>>
>> On Tue, Oct 7, 2014 at 12:00 AM, Leonardo Uribe<leonardo.uribe_at_irian.at>
>> wrote:
>>>
>>> Hi Arjan
>>>
>>> AT>> The instance is created by JSF using the existing FacesContext.
>>> AT>> getCurrentInstance() call, so nothing changes there.
>>>
>>> Keep in mind that FacesContext.getCurrentInstance() is not a cheap
>>> operation.
>>
>> You're absolutely right there, but the key insight is that the
>> producer is only consulted once per lifetime of the scope in which it
>> operates, which is the request scope here (at least for now).
>>
>> The question is then of course how fast CDI is in retrieving an
>> instance from its scopes. Since this is code that's used by pretty
>> much the entire platform I would assume much effort has been taken to
>> optimize this. Of course that's an assumption and it might be
>> interesting to explicitly test what the performance characteristics
>> here are.
>>
>> Kind regards,
>> Arjan
>>
>>
>>
>>
>>
>> Right now, we have an strategy in place in JSF that allows
>>>
>>> us to get the FacesContext quite fast (through ELContext). But rely
>>> on getCurrentInstance() could lead to bad performance on EL
>>> expression resolution. I do not want more calls than necessary in that
>>> part, otherwise we will ruin performance.
>>>
>>> regards,
>>>
>>> Leonardo Uribe
>>>
>>>
>>> 2014-10-06 16:52 GMT-05:00 arjan tijms<arjan.tijms_at_gmail.com>:
>>>>
>>>> Hi,
>>>>
>>>> On Mon, Oct 6, 2014 at 11:26 PM, manfred riem<manfred.riem_at_oracle.com>
>>>> wrote:
>>>>>
>>>>> If we let CDI handle the EL resolving of #{facesContext} it does not
>>>>> imply
>>>>> in any form or shape that the JSF runtime is not in control of creating
>>>>> the
>>>>> FacesContext. All it means is that when you have a #{facesContext} EL
>>>>> expression it will use the CDI FacesContextProducer to get the
>>>>> FacesContext.
>>>>
>>>> The approach used by Manfred here is very straightforward and powerful
>>>> at the same time. There's a dynamic producer registered via a CDI
>>>> extension, which is essentially the programmatic variant of @Produces
>>>> in user code. Giving the produced instance a default name is trivial
>>>> and just means returning a string from the Bean#getName method instead
>>>> of a null.
>>>>
>>>> The CDI spec does not have to know anything about FacesContext, since
>>>> it's the JSF dynamic producer that's in control. CDI only provides the
>>>> mechanism so that with a minimum amount of code the same instance can
>>>> be made available for both injection and usage in EL. The instance is
>>>> created by JSF using the existing FacesContext.getCurrentInstance()
>>>> call, so nothing changes there.
>>>>
>>>> There may be some valid concerns though which could be investigated.
>>>> The CDI ELResolver does not have the exact same place in the EL
>>>> resolver chain as the current JSF specific EL resolver. Whether this
>>>> really matters is an open question and would not hurt looking into.
>>>>
>>>> Another concern could be the existence of
>>>> FacesContext.setCurrentInstance(FacesContext instance).
>>>>
>>>> Calling this method now will not be reflected in the instance returned
>>>> by CDI, IF the dynamic producer already has been consulted earlier in
>>>> the same request.
>>>>
>>>> In OmniFaces we occasionally use this setCurrentInstance, i.e. via the
>>>> following code:
>>>>
>>>> public static void setContext(FacesContext context) {
>>>> FacesContextSetter.setCurrentInstance(context);
>>>> }
>>>>
>>>> private static abstract class FacesContextSetter extends FacesContext {
>>>> protected static void setCurrentInstance(FacesContext context) {
>>>> FacesContext.setCurrentInstance(context);
>>>> }
>>>> }
>>>>
>>>> And then actually use it e.g. like this:
>>>>
>>>> FacesContext temporaryContext = new TemporaryViewFacesContext(context,
>>>> createdView);
>>>> try {
>>>> setContext(temporaryContext);
>>>> getViewDeclarationLanguage(temporaryContext,
>>>> viewId).buildView(temporaryContext, createdView);
>>>> }
>>>> catch (IOException e) {
>>>> throw new FacesException(e);
>>>> }
>>>> finally {
>>>> setContext(context);
>>>> }
>>>>
>>>> Maybe a solution would be to look into using a custom request-like
>>>> scope for all JSF artifacts instead of the default request scope.
>>>>
>>>> Kind regards,
>>>> Arjan
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>> Regards,
>>>>> Manfred
>>>>>
>>>>>
>>>>> On 10/6/14, 4:11 PM, Leonardo Uribe wrote:
>>>>>>
>>>>>> Hi Ed
>>>>>>
>>>>>> But still the change discussed here (let CDI manage #{facesContext} )
>>>>>> has its problems.
>>>>>>
>>>>>> Right now we have these methods:
>>>>>>
>>>>>> FacesContext.getCurrentInstance();
>>>>>> // protected
>>>>>> FacesContext.setCurrentInstance(FacesContext instance);
>>>>>> FacesContext.getELContext();
>>>>>>
>>>>>> The problem is this: In JSF we are used to call
>>>>>> FacesContext.getCurrentInstance(), but on the ELResolver we take
>>>>>> the FacesContext instance from the ELContext. Yes, there is a
>>>>>> circular reference here. The default implementation of FacesContext
>>>>>> wraps ELContext and add the reference. That is preferred, because
>>>>>> it avoids calls to FacesContext.getCurrentInstance().
>>>>>>
>>>>>> Who knows this detail? JSF. Who is responsible for create and
>>>>>> destroy FacesContext? JSF. Who should have control over how
>>>>>> #{facesContext} is resolved? JSF.
>>>>>>
>>>>>> I don't see difficult to provide a CDI extension inside JSF to deal
>>>>>> with FacesContext problem. The only thing you need to realize is
>>>>>> you can use a wrapper or proxy, so the instance in the bean should
>>>>>> not be the same instance. To get the inner instance you can always
>>>>>> use getWrapped() method.
>>>>>>
>>>>>> I know it is easier just to burn the code inside CDI, but my question
>>>>>> is
>>>>>> if this has really sense or not, from a design perspective. It just
>>>>>> sounds
>>>>>> too inconvenient for me, because you are losing a lot of flexibility
>>>>>> in the implementation, and this is not something you must do inside
>>>>>> CDI, it is something where we can choose, and if we have an option,
>>>>>> why don't choose keep it under JSF control?.
>>>>>>
>>>>>> regards,
>>>>>>
>>>>>> Leonardo Uribe
>>>>>>
>>>>>>
>>>>>> 2014-10-06 15:16 GMT-05:00 Edward Burns<edward.burns_at_oracle.com>:
>>>>>>>>>>>>
>>>>>>>>>>>> On Mon, 6 Oct 2014 14:35:58 -0500, Leonardo
>>>>>>>>>>>> Uribe<leonardo.uribe_at_irian.at> said:
>>>>>>>
>>>>>>> LU> The problem is we introduce a dependency in CDI for JSF, unless
>>>>>>> we
>>>>>>> get
>>>>>>>
>>>>>>> Well, we're not *introducing* a dependency. We are adding more
>>>>>>> weight
>>>>>>> to a dependency we already have.
>>>>>>>
>>>>>>> LU> There is no real advantage to do that. As far as I remember,
>>>>>>> CDI is
>>>>>>> an
>>>>>>> LU> optional dependency, even if there are classes in JSF that use
>>>>>>> CDI
>>>>>>> LU> api, you can run JSF without CDI. Does that will change with
>>>>>>> JSF
>>>>>>> 2.3?
>>>>>>>
>>>>>>> You are right that prior to JSF 2.3, it was possible to use a subset
>>>>>>> of
>>>>>>> JSF features on containers that did not provide CDI. We are indeed
>>>>>>> proposing to change that in JSF 2.3, making CDI required. This
>>>>>>> change
>>>>>>> is most strongly articulated in
>>>>>>> [1287-RedefineManagedBeansAsCDIBeans].
>>>>>>> In addition to this change, we are also requiring Java EE 8 and Java
>>>>>>> SE
>>>>>>> 8 in order to run JSF 2.3.
>>>>>>>
>>>>>>> We realize these two changes are departures to our decade old policy
>>>>>>> of
>>>>>>> lagging one version behind the platform. We reasoned that if people
>>>>>>> have stuck with JSF for this long, they would be willing to move up
>>>>>>> to
>>>>>>> EE 8 as well.
>>>>>>>
>>>>>>> LU> I think CDI provides a pluggable API that works for our needs.
>>>>>>> The
>>>>>>> LU> only thing that I miss about that API is that you can't control
>>>>>>> class
>>>>>>> LU> scanning, and that causes problems because application servers
>>>>>>> LU> sometimes need to customize that part.
>>>>>>>
>>>>>>> Yes, this is one of those areas that CDI hasn't fully addressed yet.
>>>>>>> However, we believe we can mitigate the impact of this by not even
>>>>>>> saying anything about CDI's pluggable API.
>>>>>>>
>>>>>>> Ed
>>>>>>> --
>>>>>>> | edward.burns_at_oracle.com | office: +1 407 458 0017
>>>>>>> | 24 work days til Devoxx 2014