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

[jsr372-experts] Re: [URGENT] _at_FacesDataModel inconsistencies with the spec (Re: Get your remaining items in)

From: manfred riem <manfred.riem_at_oracle.com>
Date: Wed, 15 Feb 2017 17:11:30 -0700

+1

On 2/15/17, 4:52 PM, arjan tijms wrote:
> Hi,
>
> Hereby the JavaDoc as requested.
>
> The main and most important Javadoc is on @FacesDataModel and in text
> form is as follows (JavaDoc tagged version follows):
>
>
> -------
>
> The presence of this annotation on a class automatically registers the
> class with the runtime as a |DataModel| that's capable of wrapping a
> type indicated by the |FacesDataModel.forClass()| attribute.
>
> The runtime must maintain a collection of these |DataModel|s such that
> |UIData| and other components defined by the JSF specification can
> query the runtime for a suitable |DataModel| wrapper (adapter) for the
> type of their |value| as the one but last option in the list as given
> by |UIData.getValue()|.
>
> This query has to work as follows:
>
> For an instance of type |Z| that's being bound to a |UIData| component
> or other component defined by the JSF specification that utilizes
> |DataModel|, the query for that type has to return the /most
> specific/ DataModel that can wrap |Z|.
>
> This /most specific/ DataModel is defined as the DataModel that is
> obtained by first sorting the collection in which the registered
> |DataModels| are stored /(for details on this sorting see below)/ and
> then iterating through the sorted collection from beginning to end and
> stopping this iteration at the first match where for the class
> |ZZ| wrapped by the DataModel (as indicated by the
> |FacesDataModel.forClass()| attribute) it holds that
> |ZZ.isAssignableFrom(Z)|. This match is then taken as the /most
> specific/ DataModel.
>
> The sorting has to be done as follows:
>
> Sort on the class wrapped by a DataModel that is stored in the above
> mentioned collection such that for any 2 classes |X| and |Y| from this
> collection, if an object of |X| is an |instanceof| an object of |Y|,
> |X| appears in the collection /before/ |Y|. The collection's sorting
> is otherwise arbitrary.
>
> E.g.
>
> Given |class B|, |class A extends B| and |class Q|, two possible
> orders are;
>
> 1. |{A, B, Q}|
> 2. |{Q, A, B}|
>
> The only requirement here is that |A| appears before |B|, since |A| is
> a subclass of |B|.
>
> This specification does not define a dedicated public method to
> directly obtain an instance of the above mentioned most specific
> DataModel for a given type, but instead defines that for that effect
> the following code /MUST/ work:
>
> @SuppressWarnings("unchecked")
> public<T> DataModel<T> createDataModel(Class<T> forClass, Object value) {
> class LocalUIData extends UIData {
> @Override
> public DataModel<?> getDataModel() {
> return super.getDataModel();
> }
> }
> LocalUIData localUIData = new LocalUIData();
> localUIData.setValue(value);
>
> return (DataModel<T>) localUIData.getDataModel();
> }
>
>
> E.g. Given:
>
> public class Child1 {
>
> }
>
> and
> package test.jsf23;
>
> @FacesDataModel(forClass = Child1.class)
> public class Child1Model<E> extends DataModel<E> {
>
> @Override
> public int getRowCount() {
> return 0;
> }
>
> @Override
> public E getRowData() {
> return null;
> }
>
> @Override
> public int getRowIndex() {
> return 0;
> }
>
> @Override
> public Object getWrappedData() {
> return null;
> }
>
> @Override
> public boolean isRowAvailable() {
> return false;
> }
>
> @Override
> public void setRowIndex(int arg0) {
>
> }
>
> @Override
> public void setWrappedData(Object arg0) {
>
> }
> }
>
>
> Then the following must work:
>
> DataModel<Child1> myModel = createDataModel(Child1.class, new Child1());
> assert myModel instanceof Child1Model;
> System.out.println(myModel.getClass());
>
>
> The result printed should be e.g.: |"class test.jsf23.Child1Model"|
>
> |
> |
>
> |---------|
>
> |
> |
>
> |JavaDoc version:|
>
> |
> |
>
> /**
>
> * <div class="changed_added_2_3">
>
> * <p>The presence of this annotation on a class automatically
> registers the class with the runtime as a
>
> * {_at_link DataModel} that's capable of wrapping a type indicated by
> the {_at_link FacesDataModel#forClass()} attribute.
>
> *
>
> * <p>
>
> * The runtime must maintain a collection of these {_at_link DataModel}s
> such that
>
> * {_at_link UIData} and other components defined by the JSF
> specification can query the runtime for a
>
> * suitable {_at_link DataModel} wrapper (adapter) for the type of their
> <code>value</code> as the one but
>
> * last option in the list as given by {_at_link UIData#getValue()}.
>
> *
>
> * <p>
>
> * This query has to work as follows:
>
> *
>
> * <p>
>
> * For an instance of type <code>Z</code> that's being bound to a
> <code>UIData</code> component or other component
>
> * defined by the JSF specification that utilizes
> <code>DataModel</code>, the query for that type has to return the
>
> * <em>most specific</em> DataModel that can wrap <code>Z</code>.
>
> *
>
> * <p>
>
> * This <em>most specific</em> DataModel is defined as the DataModel
> that is obtained by first sorting the collection in
>
> * which the registered <code>DataModels</code> are stored <i>(for
> details on this sorting see below)</i> and then
>
> * iterating through the sorted collection from beginning to end and
> stopping this iteration at the first match where
>
> * for the class <code>ZZ</code> wrapped by the DataModel (as
> indicated by the {_at_link FacesDataModel#forClass()} attribute)
>
> * it holds that <code>ZZ.isAssignableFrom(Z)</code>. This match is
> then taken as the <em>most specific</em> DataModel.
>
> *
>
> * <p>
>
> * The sorting has to be done as follows:
>
> *
>
> * <p>
>
> * Sort on the class wrapped by a DataModel that is stored in the
> above mentioned collection such that for any 2 classes
>
> * <code>X</code> and <code>Y</code> from this collection, if an
> object of <code>X</code> is an <code>instanceof</code>
>
> * an object of <code>Y</code>, <code>X</code> appears in the
> collection <em>before</em> <code>Y</code>.
>
> * The collection's sorting is otherwise arbitrary.
>
> *
>
> * <p>
>
> * E.g.
>
> *
>
> *<p>
>
> * Given <code>class B</code>, <code>class A extends B</code> and
> <code>class Q</code>, two possible orders are;
>
> * <ol>
>
> * <li><code>{A, B, Q}</code>
>
> * <li><code>{Q, A, B}</code>
>
> * </ol>
>
> *
>
> * <p>
>
> * The only requirement here is that <code>A</code> appears before
> <code>B</code>, since <code>A</code> is a subclass of
>
> * <code>B</code>.
>
> *
>
> * <p>
>
> * This specification does not define a dedicated public method to
> directly obtain an instance of the above mentioned
>
> * most specific DataModel for a given type, but instead defines that
> for that effect the following code <em>MUST</em> work:
>
> *
>
> * <p>
>
> * <pre>
>
> * &#64;SuppressWarnings("unchecked")
>
> * public &lt;T&gt; DataModel&lt;T&gt;
> createDataModel(Class&lt;T&gt; forClass, Object value) {
>
> * class LocalUIData extends UIData {
>
> * &#64;Override
>
> * public DataModel&lt;?&gt; getDataModel() {
>
> * return super.getDataModel();
>
> * }
>
> * }
>
> * LocalUIData localUIData = new LocalUIData();
>
> * localUIData.setValue(value);
>
> *
>
> * return (DataModel&lt;T&gt;) localUIData.getDataModel();
>
> * }
>
> * </pre>
>
> *
>
> * <p>
>
> * E.g.
>
> *
>
> * Given:
>
> * <pre>
>
> * public class Child1 {
>
> *
>
> * }
>
> * </pre>
>
> *
>
> * and
>
> *
>
> * <pre>
>
> * package test.jsf23;
>
> *
>
> * &#64;FacesDataModel(forClass = Child1.class)
>
> * public class Child1Model&lt;E&gt; extends DataModel&lt;E&gt; {
>
> *
>
> * &#64;Override
>
> * public int getRowCount() {
>
> * return 0;
>
> * }
>
> *
>
> * &#64;Override
>
> * public E getRowData() {
>
> * return null;
>
> * }
>
> *
>
> * &#64;Override
>
> * public int getRowIndex() {
>
> * return 0;
>
> * }
>
> *
>
> * &#64;Override
>
> * public Object getWrappedData() {
>
> * return null;
>
> * }
>
> *
>
> * &#64;Override
>
> * public boolean isRowAvailable() {
>
> * return false;
>
> * }
>
> *
>
> * &#64;Override
>
> * public void setRowIndex(int arg0) {
>
> *
>
> * }
>
> *
>
> * &#64;Override
>
> * public void setWrappedData(Object arg0) {
>
> *
>
> * }
>
> * }
>
> * </pre>
>
> *
>
> * <p>
>
> * Then the following must work:
>
> *
>
> * <pre>
>
> * DataModel&lt;Child1&gt; myModel = createDataModel(Child1.class, new
> Child1());
>
> * assert myModel instanceof Child1Model;
>
> * System.out.println(myModel.getClass());
>
> * </pre>
>
> *
>
> * <p>
>
> * The result printed should be e.g.:
>
> *
>
> * <code>"class test.jsf23.Child1Model"</code>
>
> *
>
> * </div>
>
> *
>
> ||
>
> */
>
>
> --------
>
>
>
> |To be done is the updated JavaDoc for UIData#getValue and a similar
> update for the UIRepeat component (its tag then, since the component
> itself is impl).
> |
>
> |
> |
>
> |Kind regards,|
>
> |Arjan Tijms|
>
> |
> |
>
> |
> |
>
> |
> |
>
>
>
>
>
>
>
>
>
>
>
>
> On Wed, Feb 15, 2017 at 6:53 PM, arjan tijms <arjan.tijms_at_gmail.com
> <mailto:arjan.tijms_at_gmail.com>> wrote:
>
> Hi,
>
> On Wed, Feb 15, 2017 at 6:43 PM, Edward Burns
> <edward.burns_at_oracle.com <mailto:edward.burns_at_oracle.com>> wrote:
>
> Executive Summary: Ed Agrees with Manfred's ruling to not
> accept this
> particular change but insists Arjan updates the javadoc, with
> Ed and
> Manfred's review.
>
> Details:
>
> comments inline.
>
> LU> no new description in UIData.getDataModel(), nothing.
>
> >>>>> On Tue, 14 Feb 2017 00:31:16 +0100, arjan tijms
> <arjan.tijms_at_gmail.com <mailto:arjan.tijms_at_gmail.com>> said:
>
> AT> That's a good point, I'll look at providing something of a
> description
> AT> there right away.
>
> >>>>> On Mon, 13 Feb 2017 20:59:02 -0500, Leonardo Uribe
> <leonardo.uribe_at_irian.at <mailto:leonardo.uribe_at_irian.at>> said:
>
> LU> This issue is critical, because the presence of
> @FacesDataModel
> LU> force the spec to describe how this "create-and-wrap"
> algorithm
> LU> works, otherwise third party components will not have the
> "recipe"
> LU> to build DataModel instances.
>
> I agree this is important to at least have this recipe spelled out
> somewhere in the spec. Arjan, is that what you intend to add?
>
>
> Yes, I'll add the recipe/algorithm itself, as well as an example
> of using the code I outlined in the previous email.
>
>
> LU> And the two methods proposed in Application class?
>
> >>>>> On Tue, 14 Feb 2017 17:41:07 +0100, arjan tijms
> <arjan.tijms_at_gmail.com <mailto:arjan.tijms_at_gmail.com>> said:
>
> LU> public void addDataModel(Class<?> forClass, String
> dataModelClass)
> LU> public DataModel createDataModel(java.lang.Class<?>
> forClass, Object
> LU> value)
>
> >>>>> On Tue, 14 Feb 2017 13:08:33 -0700, manfred riem
> <manfred.riem_at_oracle.com <mailto:manfred.riem_at_oracle.com>> said:
>
> MR> Looking at what you are asking for and since it is NOT
> critical to
> MR> the usage of @FacesDataModel I am going to side with Arjan
> and make
> MR> the decision to not accept your proposed change as I am not
> MR> convinced folks will use it.
>
> I agree with this ruling, but insist on a javadoc change that
> includes
> the recipe, including a description of why it is needed.
>
> AT> The functionality for addDataModel at least is handled by
> CDI. That can't
> AT> be done by such a method, since it has to be done in the
> CDI extension and
> AT> the JSF Application class is not necessarily available by
> then. The other
> AT> way around, when the Application class is available it's
> too late for CDI
> AT> to accept new Bean<T> registrations, let alone annotated
> types.
>
> Yes, this "too early or too late" thing is a continued source
> of challenge
> when integrating JSF with CDI.
>
>
> It seems so indeed.
>
>
> AT> The createDataModel method is practically there already,
> it's in Mojarra:
>
> AT> private DataModel<?> createDataModel(final Class<?> forClass)
>
> AT> I would love to have added this method somewhere publicly,
> but I'm afraid
> AT> it's now too late for that :(
>
> I'm sorry about it too. Thanks Leo for bringing it up. The
> spec is
> better for it.
>
> AT> What you CAN do however, is subclass UIData, then
> instantiate that. Then do
>
> AT> myUIData.setValue(someClassInstance);
> AT> UIDataModel<?> myModel = myUIData.getDataModel();
>
> Is this the recipe?
>
>
> That's not the recipe/algorithm but a way to obtain the DataModel
> without needing knowledge of it.
>
> In general, Leo (or other MyFaces implementor) are the only ones
> who would really need that algorithm, Cagatay, Thomas, etc could
> do with the above code sample.
>
>
>
> AT> I humbly apologise for my oversight of adding that method
> at a public place
> AT> before. This is indeed my mistake. I will update the
> JavaDoc as you
> AT> requested to clarify this usage. For the next spec cycle
> we could add a
> AT> more convenient mechanism at the earliest opportunity.
>
> No worries, this happens. Yes, ok, thanks.
>
>
> Okay, thanks for understanding.
>
>
> >>>>> On Tue, 14 Feb 2017 13:56:53 -0500, Leonardo Uribe
> <leonardo.uribe_at_irian.at <mailto:leonardo.uribe_at_irian.at>> said:
>
> LU> The problem here is there are other components that does
> not extend from
> LU> UIData and uses DataModel. The closest example is
> UIRepeat, but there are
> LU> others too.
>
> Right, but since @FacesDataModel is new in 2.3, there will be no
> backward compatibility issue. It is just an additional hoop
> to jump
> through when using the feature, though.
>
> LU> The important thing is provide something somewhere that
> can be commonly
> LU> accessed. For example, we could use a map or some generic
> interface stored
> LU> in externalContext.getApplicationMap(). It that way, we
> can avoid add
> LU> methods, but we provide what we need just agreeing with
> the name and what
> LU> does it return. It is not perfect but it is something at
> least. Is this
> LU> possible alternative feasible?
>
> >>>>> On Tue, 14 Feb 2017 14:59:26 -0500, Leonardo Uribe
> <leonardo.uribe_at_irian.at <mailto:leonardo.uribe_at_irian.at>> said:
>
> AT> But I'm afraid it's, unfortunately, simply too late now.
>
> LU> @Ed, Manfred: WDYT? a public review where you send
> feedback before 30 days
> LU> and is it too late? why bother to send a public review if
> changes will not
> LU> be accepted?
>
> I prefer to look at it this way. The changes are accepted,
> but the
> change is manifesting in a clearer spec rather than in code
> changes.
>
> >>>>> On Tue, 14 Feb 2017 23:25:32 +0100, arjan tijms
> <arjan.tijms_at_gmail.com <mailto:arjan.tijms_at_gmail.com>> said:
>
> AT> p.s. the utility/trick method I mentioned earlier is this one:
> AT> @SuppressWarnings("unchecked")
> AT> public <T> DataModel<T> createDataModel(Class<T>
> forClass, Object
> AT> value) {
> AT> class LocalUIData extends UIData {
> AT> @Override
> AT> public DataModel<?> getDataModel() {
> AT> return super.getDataModel();
> AT> }
> AT> }
> AT> LocalUIData localUIData = new LocalUIData();
> AT> localUIData.setValue(value);
>
> AT> return (DataModel<T>) localUIData.getDataModel();
> AT> }
>
> AT> That one implements the required lookup without having to
> know the details
> AT> of the Bean discovery and type resolution algorithms.
>
> >>>>> On Tue, 14 Feb 2017 18:03:40 -0500, Leonardo Uribe
> <leonardo.uribe_at_irian.at <mailto:leonardo.uribe_at_irian.at>> said:
>
> LU> Yes, the trick clarifies my doubts about it. It is not
> pretty, but it will
> LU> work. Thanks for mention it.
>
> Ok: ACTION: Arjan: Please produce a patch with the requested
> javadoc
> change. I want this change reviewed by Manfred and I. Other
> reviewers
> are of course welcome, but at this point Manfred and I have
> the final
> word. The text should include the complete recepie, a code
> example, and
> an explanation of why the recipe is needed. Please try to
> have it by
> COB Friday, COB Thursday is even better.
>
>
> Okay, should be doable. I aim to have it done tonight.
>
> Kind regards,
> Arjan Tijms
>
>
>
>
>
> Thanks,
>
> Ed
>
> --
> | edward.burns_at_oracle.com <mailto:edward.burns_at_oracle.com> |
> office: +1 407 458 0017 <tel:%2B1%20407%20458%200017>
> | 14 business days until planned start of JSF 2.3 Final
> Approval Ballot
> | 4 business days until DevNexus 2017
> | 29 business days until JavaLand 2017
>
>
>