jsr372-experts@javaserverfaces-spec-public.java.net

[jsr372-experts] Re: [jsr372-experts mirror] Re: Re: Re: Re: [1311-FacesContextCDI] Let CDI handle #{facesContext}

From: manfred riem <manfred.riem_at_oracle.com>
Date: Mon, 06 Oct 2014 17:38:31 -0500

Hi Arjan,

I would assume the @Inject happens lazily. And the same thing with
#{facesContext}

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