users@javaserverfaces-spec-public.java.net

[jsr344-experts mirror] [jsr344-experts] ApplicationConfigurationResourceDocumentPopulator

From: Andy Schwartz <andy.schwartz_at_oracle.com>
Date: Wed, 13 Mar 2013 12:13:15 -0400

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