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

[jsr344-experts] Re: ComponentSystemEvent API review

From: Edward Burns <edward.burns_at_oracle.com>
Date: Thu, 14 Mar 2013 09:27:47 -0700

>>>>> On Tue, 12 Mar 2013 22:01:34 -0500, Leonardo Uribe <lu4242_at_gmail.com> said:

AS> Which was added to address this spec issue:

AS> JAVASERVERFACES_SPEC_PUBLIC-1043 When publishing a ComponentSystemEvent,
AS> ensure the EL current component is pushed correctly
AS> http://java.net/jira/browse/JAVASERVERFACES_SPEC_PUBLIC-1043

AS> In my mind, it should be the responsibility of whatever code calls
AS> publishEvent() to ensure that proper context is set up before
AS> publishing the event. This should already be happening implicitly
AS> for most events, eg. the current component should already be set up
AS> for any events that are published during JSF lifecycle processing.
AS> Failure to do this would indicate a bug in the calling code that we
AS> don't want to mask with some bonus ComponentSystemEvent magic.

LU> +1. I supposed the documentation of that method was just to specify
LU> that the calling code should ensure that condition.

LU> I have to say that those lines are really bad for performance
LU> perspective, because call FacesContext.getCurrentInstance() is very
LU> expensive in that context.

AS> If there are some cases where the JSF implementations are failing to
AS> take care of this (eg. when delivering PreRenderViewEvents?), I
AS> would prefer that we clean up the implementations to handle this
AS> correctly rather than complicate
AS> ComponentSystemEvent.processListener().
AS>

LU> I remember this problem. I fixed it some time ago, but I only considered
LU> #{cc} case. The solution implemented was done on f:event tag handler. See

[...]

LU> I would strongly prefer do not do nothing, because any code in that
LU> part is really
LU> bad for performance.

We have product completion criterion that states we cannot release with
failing or ignored testcases. I feel that if I took this out, we'd see
some failing testcases and I don't think we have time to fix them on a
per-case basis as you suggest we should.

How about we leave it for 2.2 and then we can take it out and replace
the ComponentSystemEvent.processListener() with:

public void processListener(FacesListener listener) {
  super.processListener(listener);
}

after the release? We can do this without breaking binary compatibility
or even a signature change.

Is this ok?

Ed