[jsr-314-open] [ADMIN] JSF 2.0 Spec Snapshot

[Simon Lessard]

Hi all,

I'm terribly sorry for the late review, but here are some minor API/JavaDoc issues I found while locally integrating those change to MyFaces :

________________________________________________________________________

javax.faces.application.Application

public abstract void addBehavior(String behaviorId, String behaviorClass);

public abstract Behavior createBehavior(String behaviorId) throws FacesException;

public abstract Iterator<String> getBehaviorIds();

All those methods should not be abstract and throw UnsupportedOperationException to be consistent with other  methods added after JSF 1.1. 

On that UnsupportedOperationException, would it be possible to revisit that strategy for 2.1? I know we provide that for binary backward compatibility, but in real life usage it's only dust thrown in the eyes of users imho. In fact, it only delays the error until the application is executed and it's then harder to fix issues as you have to redeploy the application after every fixes, as opposed to compilation errors that all show up at the same time. For example, 2.0 API have UIViewRoot depend on PartialViewContext accessed through a new getPartialViewContext on the FacesContext. By default that method now throws an UnsupportedOperationException so if the user application use a FacesContext decorator, the application WILL crash at runtime while a real abstract method would have warned the user about the missing method that he could have redirected to the wrapped instance. Also, since 2.0 provides a lot of new Wrapper classes, assuming users will start using them, then adding new abstract methods should no longer be much of an issue since by default the provided Wrapper classes will already take care of redirecting those.

_____________________________________________________________________________

javax.faces.component.behavior.AjaxBehavior

public Set<BehaviorHint> getHints()

public String getRendererType()

Those methods are either not needed as they only call super (copied doc), or the JavaDoc is incorrect.

_____________________________________________________________________________

javax.faces.component.UIComponent

public void unsubscribeFromEvent(Class<? extends SystemEvent> eventClass, ComponentSystemEventListener componentListener)

The JavaDoc says 'When doing the comparison to determine if an existing listener is equal to the argument componentListener (and thus must be removed), the equals() method on the existing listener must be invoked, passing the argument componentListener, rather than the other way around.' However equals should always be commutative so I don't really know if such really low and strange implementation detail should be part of the JavaDoc

_____________________________________________________________________________

javax.faces.component.UIComponentBase

public FacesListener[] getFacesListeners(Class clazz)

Method is not generic enabled, it should be 

public FacesListener[] getFacesListeners(Class<? extends FacesListener> clazz)

Also, on a slightly different but related note, BehaviorBase class also has the listener handling methods, but does not provide the getBehaviorsListeners method, should it be added for consistency purposes?

_____________________________________________________________________________

javax.faces.model.DataModel

public Iterator<E> iterator()

The only way for the returned Iterator to get row data is to set the model's rowIndex (because there's no getRowData(int index) method on DataModel). Therefore, the model become inherently not thread safe. I believe such limitation should be mentioned in the class' documentation. Of course, DataModel were already not thread safe in nearly all implementation so it's only an extra documentation needed here for new users imho so they don't try to place those in application scoped beans.

Also since Java 5.0 allows return type covariance for overridden methods I think we could change the various getWrapped() methods to return relevant type (E[]m List<E>, etc.)

_____________________________________________________________________________

javax.faces.webapp.pdl.facelets.FaceletContext

public static final String FACELET_CONTEXT_KEY = "com.sun.faces.facelets.FACELET_CONTEXT";

Can we get a "javax.faces.XYZ" constant value for that please?

_____________________________________________________________________________

javax.faces.event.ExceptionQueuedEventContext

public static final String IN_AFTER_PHASE_KEY;

public static final String IN_BEFORE_PHASE_KEY;

The spec does not define any value at all for those two constants.

_____________________________________________________________________________

That's about it!

Regards,

~ Simon

 

________________________________

From: JSR 314 Open Mailing list on behalf of Ed Burns
Sent: Wed 3/11/2009 10:51 AM
To: JSR-314-OPEN at JCP.ORG
Subject: Re: [jsr-314-open] [ADMIN] JSF 2.0 Spec Snapshot



>>>>> On Thu, 05 Mar 2009 11:00:20 +0000, Pete Muir <pmuir at REDHAT.COM> said:

PM> Ed,
PM> Here are some questions:

PM> * is delta state saving in this revision? The state saving stuff is 
PM> still in an appendix, and I find it hard to check.

It's not in the revision.  The stuff in the appendix is now quite stale
due to the lifecycle changes introduced to accomodate Andy Schwartz and
Dan Allen's concerns over the performance of view metada gathering.
Ryan is currently reworking the state saving branch, including Martin's
recent patches, but it's tricky.  The state management revision may have
to be slipped to 2.1.

PM> * Issue 450 (https://javaserverfaces-spec-public.dev.java.net/issues/show_bug.cgi?id=450
PM> ) still hasn't been merged into the spec - I've asked about this one 
PM> multiple times. Can we get this merged in?

THANKS for your persistence.  This email will include an action plan for
Roger and I to follow, and this will be on it.

PM> * the big changes due to view parameters don't appear to be merged
PM> in?

See action plan below.

PM> Otherwise, the areas I have been involved in look good:

PM> * exception handling
PM> * bean validation

Thanks for the oversight.

>>>>> On Thu, 05 Mar 2009 16:32:18 +0100, Martin Marinschek <mmarinschek at APACHE.ORG> said:

MM> Hi Ed,
MM> my feedback below. I already (wrongly) sent these using my Gmail-address,
MM> but I guess (hope) these mails didn't make it...

I've put these into the action plan.

MM> also, should I send out the patch-proposal that I made and the
MM> follow up discussions I had with Leonardo to the EG? Pete and Andy
MM> seemed to be interested...

AS> Is this regarding delta state saving?  I am definitely curious to hear
AS> where we ended up with this.

As mentioned above, late breaking view parameter changes have impacted
our ability to include the state saving changes into 2.0.

>>>>> On Thu, 05 Mar 2009 17:26:44 +0000, Pete Muir <pmuir at REDHAT.COM> said:

PM> Another couple more after talking to Dan:

In action plan

PM> RegexValidator

ACTION PLAN for Roger and Ed.

These need to be done to the spec.

rogerk * 10.3.1.1: in table 10.3, the f:ajax attribute is called event -
  in several paragraphs around this, events - which one is true?

  It is now "event".  Roger should be taking care of this soon.

edburns * Merge spec issue [SelectItemsCollection-450] into spec.

edburns * Merge all view parameter changes into spec.

edburns * 3.1.8: should the tree-visitor be mentioned here?

edburns * 4.1.18.4: shouldn't this reference AfterAddToViewEvent?

edburns * 5.4.1: seems to have old spec language (language on error
  handler says that only predestroy does not call through to it)

edburns * 6.1.14: managed-bean-scope can now be view on top of none,
  request, session, application - can it also be flash? I don't see this
  mentioned anywhere.

edburns * the spec language for the BeanValidator is light compared to
  other validators

  (see 3.5.5 - for example RegexValidator's description vs. BeanValidator

Thanks for your feedback!

Ed

--
| ed.burns at sun.com  | office: 408 884 9519 OR x31640
| homepage:         | http://ridingthecrest.com/