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

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

From: Leonardo Uribe <leonardo.uribe_at_irian.at>
Date: Wed, 15 Feb 2017 19:19:27 -0500

+1

As a side comment, In MyFaces I do not use the sorting algorithm, but the
important thing is aim to find the best match.

2017-02-15 19:11 GMT-05:00 manfred riem <manfred.riem_at_oracle.com>:

> +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 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
>>> <%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
>>>
>>
>>
>