I agree with Andy's comments.
I can envision two different styles for using an interface like this:
pull style such as the described by the current API where the JSF
implementation "pulls" metadata from the application as well as a "push"
style where the application pushes metadata into the JSF
implementation. It probably makes sense to support both styles.
I'm also not convinced an XML Document is the best way to represent the
metadata. Maybe the application has matedata stored in some other form
(e.g. JSON, a database table, etc.), translating that to an XML document
would be a real pain. I think it makes more sense to define Java
interfaces for the faces-config.xml content. We already have some of
these: NavigationCase, Flow, FlowNode, etc. The root type would be
something like ApplicationResource which would in turn have getter
methods for the various types of faces-config.xml content, e.g.
getNavigationRules(), getManagedBeanDefinitions(), etc.
Given where we're at in the current schedule I doubt we have time to
make these kinds of changes. As the original requester of the feature
I'd be fine with deferring the feature to our next revision of the spec
in order to give ourselves time to define an interface like I described
above.
Dave
On 03/13/2013 10:13 AM, Andy Schwartz wrote:
> Gang -
>
> Some comments on:
>
> Index: javax/faces/ApplicationConfigurationResourceDocumentPopulator.java
> ===================================================================
> --- javax/faces/ApplicationConfigurationResourceDocumentPopulator.java
> (revision 0)
> +++ javax/faces/ApplicationConfigurationResourceDocumentPopulator.java
> (revision 11719)
>
> Prior to the addition of this class, we've got three APIs in the
> top-level javax.faces package:
>
> - FacesWrapper
> - FactoryFinder
> - FacesException
>
> ApplicationConfigurationResourceDocumentPopulator looks very out of
> place here - almost like we forgot to assign it a package.
>
> Can we push this class down to javax.faces.application?
>
> The name ApplicationConfigurationResourceDocumentPopulator seems
> unnecessarily verbose. Why not just ApplicationConfigurationPopulator?
>
> Similar comment for this method name:
>
> + public abstract void
> populateApplicationConfigurationResource(Document toPopulate);
>
> Seems like "populateApplicationConfiguration" is plenty descriptive.
> This also avoids any association with javax.faces.application.Resource.
>
> One thing that strikes me as awkward is that the Document is passed in
> rather than returned. This means that an implementation that uses an
> XML parser to produce a Document needs to copy the contents over to
> the provided Document.
>
> Why impose this extra step on implementations of this contract?
> (Possible this might be necessary, but I am not clear on why.)
>
> Perhaps something like:
>
> abstract public class ApplicationConfigurationProvider
> {
> abstract public Document getApplicationConfiguration();
> }
>
> Would be easier for folks to implement.
>
> However, this does raise a question...
>
> Will implementations require access to some additional context in
> order to locate their custom application config content? For example,
> is it possible that this content might be stored in the web app's
> WEB-INF directory, and without some way of accessing this location,
> potential implementations would be hosed?
>
> I think we need to verify that this contract is sufficient to address
> the requirements for the use case identified by the original filer.
> Since the issue was filed by members of the ADF team, I am following
> up with them on this topic.
>
> Andy