Hi
AT> A CDI extension provided by JSF installs a dynamic producer
That's what I wanted to hear. Thanks for the clarification.
I would like that the spec should be given in those terms without
mention that the instance is somehow bound to request scope.
I think at the end the producer will be consulted most of the time.
I mean, you usually use facesContext in a EL to build an url
(facesContext.externalContext...), but if JSF provides the extension,
we can make something to overcome that situation. We did that
already in MyFaces to override FactoryFinder, so in this case I
suppose the best is use reflection over setCurrentInstance(...),
move it to the implementation and do it right.
Really this issue is relate to a more deep discusssion about how
can we use @Inject on objects like Application, PhaseListener,
Converter, Validator and others. In the past, JSF has been in
control of the instantiation, but with CDI there are moments when
you want to use @Inject. We did that part partially
on JSF 2.2, see:
https://java.net/jira/browse/JAVASERVERFACES_SPEC_PUBLIC-763
Maybe it is a good time to reopen this issue or create a new one?
regards,
Leonardo Uribe
2014-10-06 17:56 GMT-05:00 arjan tijms <arjan.tijms_at_gmail.com>:
> 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