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 23:49:56 +0100

Hi,

Thanks a lot for the review Ed!

I applied all your suggested changes and committed to the rolling 2.3
branch, with the good old "r=edburns".

Kind regards,
Arjan Tijms






On Thu, Feb 16, 2017 at 8:41 PM, Edward Burns <edward.burns_at_oracle.com>
wrote:

> >>>>> On Thu, 16 Feb 2017 00:52:46 +0100, arjan tijms <
> arjan.tijms_at_gmail.com> said:
>
> AT> Hi,
> AT> Hereby the JavaDoc as requested.
>
> AT> The main and most important Javadoc is on @FacesDataModel and in text
> form
> AT> is as follows (JavaDoc tagged version follows):
>
> This is great work. Comments inline, mostly editorial. Please read to
> the end.
>
> >>>>> On Wed, 15 Feb 2017 19:19:27 -0500, Leonardo Uribe <
> leonardo.uribe_at_irian.at> said:
>
> LU> +1
>
> >>>>> On Wed, 15 Feb 2017 17:11:30 -0700, manfred riem <
> manfred.riem_at_oracle.com> said:
>
> MR> +1
>
>
>
> AT> -------
>
> AT> The presence of this annotation on a class automatically registers the
> AT> class with the runtime as a DataModel that's capable of wrapping a type
> AT> indicated by the FacesDataModel.forClass() attribute.
>
> In this case, say "that is" instead of "that's".
>
> AT> The runtime must maintain a collection of these DataModels such that
> AT> UIData and other components defined by the JSF specification can
> AT> query the runtime for a suitable DataModel wrapper (adapter) for the
> AT> type of their value as the one but last option in the list as given
> AT> by UIData.getValue().
>
> What do you mean by "one but last"?
>
> AT> This query has to work as follows:
>
> Say "must" instead of "has to" here.
>
> AT> For an instance of type Z that's being bound to a UIData component or
> other
> AT> component defined by the JSF specification that utilizes DataModel, the
> AT> query for that type has to return the *most specific* DataModel that
> can
> AT> wrap Z.
>
> Again, "that is" instead of "that's", and "must" instead of "has to".
>
> AT> This *most specific* DataModel is defined as the DataModel that is
> obtained
> AT> by first sorting the collection in which the registered DataModels are
> AT> stored *(for details on this sorting see below)* and then iterating
> through
> AT> the sorted collection from beginning to end and stopping this
> iteration at
> AT> the first match where for the class ZZ wrapped by the DataModel (as
> AT> indicated by the FacesDataModel.forClass() attribute) it holds that
> AT> ZZ.isAssignableFrom(Z). This match is then taken as the *most specific*
> AT> DataModel.
>
> Excellent.
>
> AT> The sorting has to be done as follows:
>
> has to->must
>
> AT> Sort on the class wrapped by a DataModel that is stored in the above
> AT> mentioned collection such that for any 2 classes X and Y from this
> AT> collection, if an object of X is an instanceof an object of Y, X
> AT> appears in the collection *before* Y. The collection's sorting is
> AT> otherwise arbitrary.
>
> I'd throw in, "In other words, subclasses come before their
> superclasses".
>
> AT> E.g.
>
> Say "for example".
>
> AT> Given class B, class A extends B and class Q, two possible orders are;
>
> AT> 1. {A, B, Q}
> AT> 2. {Q, A, B}
>
> AT> The only requirement here is that A appears before B, since A is a
> subclass
> AT> of B.
>
> AT> This specification does not define a dedicated public method to
> directly
> AT> obtain an instance of the above mentioned most specific DataModel for a
> AT> given type, but instead defines that for that effect the following code
> AT> *MUST* work:
>
> Let me reword that as:
>
> The specification does not define a public method to obtain an instance
> of the "most specific DataModel for a given type". Such an instance can
> be obtained using code similar to the following.
>
> AT> @SuppressWarnings("unchecked")
> AT> public <T> DataModel<T> createDataModel(Class<T> forClass, Object
> 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> E.g. Given:
>
> For example:
>
> AT> public class Child1 {
>
> AT> }
>
> AT> and
>
> AT> package test.jsf23;
>
> AT> @FacesDataModel(forClass = Child1.class)
> AT> public class Child1Model<E> extends DataModel<E> {
>
> AT> @Override
> AT> public int getRowCount() {
> AT> return 0;
> AT> }
>
> AT> @Override
> AT> public E getRowData() {
> AT> return null;
> AT> }
>
> AT> @Override
> AT> public int getRowIndex() {
> AT> return 0;
> AT> }
>
> AT> @Override
> AT> public Object getWrappedData() {
> AT> return null;
> AT> }
>
> AT> @Override
> AT> public boolean isRowAvailable() {
> AT> return false;
> AT> }
>
> AT> @Override
> AT> public void setRowIndex(int arg0) {
>
> AT> }
>
> AT> @Override
> AT> public void setWrappedData(Object arg0) {
>
> AT> }
> AT> }
>
>
> AT> Then the following must work:
>
> AT> DataModel<Child1> myModel = createDataModel(Child1.class, new
> Child1());
> AT> assert myModel instanceof Child1Model;
> AT> System.out.println(myModel.getClass());
>
>
> AT> The result printed should be e.g.: "class test.jsf23.Child1Model"
>
>
> AT> ---------
>
> All good.
>
> AT> JavaDoc version:
>
> The only javadoc specific change I request is to put multi-line code
> within <pre><code><code></pre>.
>
> With the above changes, you have my +1 to commit, or as we used to say
> back in the old days: r=edburns.
>
> Thanks,
>
> Ed
>
> --
> | edward.burns_at_oracle.com | office: +1 407 458 0017
> | 13 business days until planned start of JSF 2.3 Final Approval Ballot
> | 3 business days until DevNexus 2017
> | 28 business days until JavaLand 2017
>