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

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

From: arjan tijms <arjan.tijms_at_gmail.com>
Date: Thu, 16 Feb 2017 00:52:46 +0100

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 DataModels 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> wrote:

> Hi,
>
> On Wed, Feb 15, 2017 at 6:43 PM, Edward Burns <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> 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> 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> 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> 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> 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> 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> 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> 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 | office: +1 407 458 0017
>> | 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
>>
>
>