Gang -
More API review comments...
Index: javax/faces/context/FacesContext.java
===================================================================
--- javax/faces/context/FacesContext.java (revision 8845)
+++ javax/faces/context/FacesContext.java (revision 11719)
+ /**
+ * <p class="changed_added_2_2">Return the list of resource library
+ * contracts that have been calculated
+ * to be appropriate for use with this view, or {_at_code null} if
there are
+ * no such resource library contracts. The list returned by this
method
+ * must be immutable. For backward compatibility with implementations
+ * of the specification prior to when this method was introduced, an
+ * implementation is provided that returns {_at_code null}.
Implementations
+ * compliant with the version in which this method was introduced must
+ * implement this method as specified.</p>
+ *
+ * @throws IllegalStateException if this method is called after
+ * this instance has been released
+ *
+ * @since 2.2
+ */
+ public List<String> getResourceLibraryContracts() {
When introducing new methods that return collections, we should:
- Spec that the returned collection is immutable (if possible).
- Spec that a non-null return value is guaranteed.
- Either mark the new method as abstract, or, if we're not able/willing
to do that, provide a default implementation that returns the empty
collection.
Code that wants to distinguish between empty and non-empty cases can
easily do so. Code that does not care is not polluted by the null check.
+ /**
+ * <p class="changed_added_2_2">Set the resource library contracts
+ * calculated as valid to use with this view. The implementation must
+ * copy the contents of the incoming {_at_code List} into an immutable
+ * {_at_code List} for return from {_at_link #getResourceLibraryContracts}.
+ * If the argument is {_at_code null} or empty, the action taken is
the same as if
+ * the argument is {_at_code null}: a subsequent call to {_at_code
getResourceLibraryContracts}
+ * returns {_at_code null}. This method may only be called during the
+ * processing of {_at_link
javax.faces.view.ViewDeclarationLanguage#createView}
+ * and during the VDL tag handler for the tag corresponding to
+ * an instance of {_at_code UIViewRoot}. For backward compatibility
with implementations
+ * of the specification prior to when this method was introduced, an
+ * implementation is provided that takes no action. Implementations
+ * compliant with the version in which this method was introduced must
+ * implement this method as specified.
+ *
+ * </p>
+ *
+ * @param contracts The new contracts to be returned, as an immutable
+ * {_at_code List}. from a subsequent call to {_at_link
#getResourceLibraryContracts}.
+ *
+ * @throws IllegalStateException if this method is called after
+ * this instance has been released
+ *
+ */
+
+ public void setResourceLibraryContracts(List<String> contracts) {
I really don't like the approach of exposing a public setter with such
restrictions, particularly because we need to keep multiple pieces of
state coordinated.
Based on ViewDeclarationLanguage.calculateResourceLibraryContracts, it
seems that resource library contracts are dependent on the viewId. (I
don't fully understand this. Still need to review the documentation on
this from the spec.) The viewId can change via calls to
FacesContext.setViewRoot() or UIViewRoot.setViewId(). Isn't the best
way to ensure that getResourceLibraryContracts() returns the appropriate
contracts for the current viewId to have the FacesContext implementation
take care of this. For example,
FacesContextImpl.getResourceLibraryContracts() could:
- Get the current viewId.
- Check to see whether the resource library contracts for this viewId
has already been calculated and cached.
- If not, calculate the resource library contracts
- Cache the resource library contracts, noting the associated viewId.
- Return the cached resource library contracts.
With this approach, there is no need for a public setter, and we are
guaranteed that the resource library contracts will never get out of
sync with the current viewId.
+ /**
+ * <p class="changed_added_2_2"> Return the {_at_link
+ * javax.faces.component.visit.ComponentModificationManager} for
+ * this single run through the JSF request processing lifecycle.</p>
+ *
+ * <p class="changed_added_2_2">The default implementation throws
+ * <code>UnsupportedOperationException</code> and is provided
+ * for the sole purpose of not breaking existing applications that
extend
+ * this class.</p>
+ *
+ * @throws IllegalStateException if this method is called after
+ * this instance has been released
+ *
+ * @since 2.2
+ */
+ public ComponentModificationManager getComponentModificationManager() {
As mentioned in my ComponentModificationManager email, I would strongly
prefer that we kill off this API and revisit in 2.3.
Andy