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

[jsr372-experts] Re: 1078-DataModelRegistrable

From: arjan tijms <arjan.tijms_at_gmail.com>
Date: Thu, 9 Jul 2015 21:35:26 +0200

Hi,

It's now over a month since I posted the change bundle, and I wonder
how to continue.

With the code as posted the feature works for UIData. It still has to
be repeated for UIRepeat. There were some remarks about changing the
ordering. Currently the code checks for a user provided DataModel
wrapper as the LAST thing in the chain, so after the existing code
checks if the data is a List, Collection, etc.

The requested change was to have it as one of the first checks, right
after the null check and the check to see if the data is not already
of type DataModel.

The pro of doing the check early is that users or libraries can
provide alternative wrappers for List, Collection, etc (in essence,
overriding the JSF default behaviour).

The con is that there is a (likely) rare, but possible case where
behaviour might change.

See these 3 cases I identified:

1. When an existing JSF 2.2 or earlier application that normally runs
on a JSF 2.2/EE7 implementation now runs on the new JSF 2.3/EE8
implementation NOTHING changes. This application nor its libraries can
provide an @FacesDataModel annotated wrapper, since this was new in
2.3.

2. When an existing JSF 2.2 or earlier application that now runs on
the new JSF 2.3/EE8 implementation upgrades one of its libraries, and
this library happens to provide an @FacesDataModel annotated wrapper
that happens to wrap List, and the application happens to rely on an
obscure behaviour of the build-in List wrapper, something may
potentially break. This is however not a silent breakage, since the
upgrade of the library was an explicit user action, and the library
explicitly overrides the build-in List. This is not different from any
other library that automatically overrides some JSF default behaviour.

3. When an existing JSF 2.2 or earlier application that still runs on
the old JSF 2.2/EE7 implementation upgrades one of its libraries, and
this library happens to provide an @FacesDataModel annotated wrapper
that happens to wrap List, and the application happens to rely on an
obscure behaviour of the build-in List wrapper, then initially nothing
happens (JSF 2.2 will not do anything with the provided
@FacesDataModel). However if the application after having upgraded one
of its libraries upgrades to JSF 2.3, it will now "suddenly" face
different behaviour. Namely, the behaviour that was dormant before
will now "suddenly" be activated.

It's a question how likely 3) is to occur in practice, and how much of
a problem this really is. After all, the user did 2 explicit upgrade
actions and by using a library that overrides standard behaviour the
user or library could be seen as responsible, not JSF.

As for the code, checking for the DataModel wrapper EARLY looks like this:

protected DataModel getDataModel() {

        // Return any previously cached DataModel instance
        if (this.model != null) {
            return (model);
        }

        // Synthesize a DataModel around our current value if possible
        Object current = getValue();
        if (current == null) {
            setDataModel(new ListDataModel(Collections.EMPTY_LIST));
        } else if (current instanceof DataModel) {
            setDataModel((DataModel) current);
        } else {

            // A type other than DataModel was provided. Check if there's
            // a DataModel wrapper registered for this type.
            DataModel<?> dataModel = createDataModel(current.getClass());
            if (dataModel != null) {
                dataModel.setWrappedData(current);
                setDataModel(dataModel);
            } else {

                // No registered DataModel wrapper available. Try our list
                // of build in wrappers
                if (current instanceof List) {
                    setDataModel(new ListDataModel((List) current));
                } else if
(Object[].class.isAssignableFrom(current.getClass())) {
                    setDataModel(new ArrayDataModel((Object[]) current));
                } else if (current instanceof ResultSet) {
                    setDataModel(new ResultSetDataModel((ResultSet) current));
                } else if (current instanceof Result) {
                    setDataModel(new ResultDataModel((Result) current));
                } else if (current instanceof Collection) {
                    setDataModel(new CollectionDataModel((Collection) current));
                } else if (current instanceof Iterable) {
                    setDataModel(new IterableDataModel<>((Iterable<?>)
current));
                } else if (current instanceof Map) {
                    setDataModel(new IterableDataModel<>(((Map<?, ?>)
current).entrySet()));
                } else {
                    setDataModel(new ScalarDataModel(current));
                }
            }
        }

        return (model);

    }

Checking for the model LAST looks like this:

    protected DataModel getDataModel() {

        // Return any previously cached DataModel instance
        if (this.model != null) {
            return (model);
        }

        // Synthesize a DataModel around our current value if possible
        Object current = getValue();
        if (current == null) {
            setDataModel(new ListDataModel(Collections.EMPTY_LIST));
        } else if (current instanceof DataModel) {
            setDataModel((DataModel) current);
        } else if (current instanceof List) {
            setDataModel(new ListDataModel((List) current));
        } else if (Object[].class.isAssignableFrom(current.getClass())) {
            setDataModel(new ArrayDataModel((Object[]) current));
        } else if (current instanceof ResultSet) {
            setDataModel(new ResultSetDataModel((ResultSet) current));
        } else if (current instanceof Result) {
            setDataModel(new ResultDataModel((Result) current));
        } else if (current instanceof Collection) {
            setDataModel(new CollectionDataModel((Collection) current));
        } else if (current instanceof Iterable) {
            setDataModel(new IterableDataModel<>((Iterable<?>) current));
        } else if (current instanceof Map) {
            setDataModel(new IterableDataModel<>(((Map<?, ?>)
current).entrySet()));
        } else {
            DataModel<?> dataModel = createDataModel(current.getClass());
            if (dataModel != null) {
                dataModel.setWrappedData(current);
                setDataModel(dataModel);
            } else {
                setDataModel(new ScalarDataModel(current));
            }
        }
        return (model);

    }

Kind regards,
Arjan Tijms

On Mon, Jun 15, 2015 at 8:04 PM, Josh Juneau <juneau001_at_gmail.com> wrote:
> I think it will be fine for beginners to set the parameter. I am going to
> guess that most IDEs can automatically apply the parameter to new (or
> upgraded) JSF apps anyways.
>
> Thanks for all if your excellent work on making JSF even better!
>
> Josh Juneau
> http://jj-blogger.blogspot.com
> https://www.apress.com/index.php/author/author/view/id/1866
>
> On Jun 15, 2015, at 12:16 PM, arjan tijms <arjan.tijms_at_gmail.com> wrote:
>
> Hi,
>
> On Monday, June 15, 2015, Josh Juneau <juneau001_at_gmail.com> wrote:
>>
>> +1 to checking custom first and making it the default behavior....less
>> configuration is better.
>
>
> Definitely agree with that, although every JSF 2.3 application will need to
> set the javax.faces.ENABLE_CDI_RESOLVER_CHAIN parameter already.
>
> So hooking into that particular parameter should not be a big issue, since
> for 2.3 applications it's already there.
>
> For people starting with JSF (and therefore have no 2.2 or earlier
> compatibility concern) the parameter is a bit unfortunate perhaps. It's
> likely hard to explain why they need to put something that will probably
> look like gibberish to them in their web.xml to fully "activate" JSF.
>
> But I don't quite see how to do it differently.
>
> Kind regards,
> Arjan Tijms
>
>
>
>
>>
>>
>> Josh Juneau
>> juneau001_at_gmail.com
>> http://jj-blogger.blogspot.com
>> https://www.apress.com/index.php/author/author/view/id/1866
>>
>>
>> On Wed, Jun 10, 2015 at 10:15 AM, arjan tijms <arjan.tijms_at_gmail.com>
>> wrote:
>>>
>>> Hi,
>>>
>>> On Wed, Jun 10, 2015 at 3:20 PM, Kito Mann <kito.mann_at_virtua.com> wrote:
>>>>
>>>> Only people who are using JSF 2.3 will be using this annotation, right?
>>>
>>>
>>> Basically yes. But I was half thinking about people running their JSF 2.2
>>> application on a Java EE 8 server and using a library that may contain this
>>> new annotation as a default for some type.
>>>
>>>
>>>>
>>>> So I say we check custom ones first and make that the default behavior.
>>>
>>>
>>> If this was an OmniFaces feature there would be no doubt in my mind to
>>> check custom ones first, so functionality wise I agree with this ;)
>>>
>>> I was just being extra careful with backwards compatibility, as from
>>> previous discussions here I more or less picked up the signal that backwards
>>> compatibility is the single most important consideration. But maybe there
>>> isn't really an issue to begin with. So if we all, and specifically the spec
>>> leads, agree to do custom checks first I'd be more than happy to change
>>> this.
>>>
>>>
>>>>
>>>> Maybe provide a switch, but don't tie it to the version number.
>>>
>>>
>>> Well, I was contemplating if it's wise or not to tie in to the existing
>>> (in JSF 2.3) javax.faces.ENABLE_CDI_RESOLVER_CHAIN switch.
>>>
>>> See
>>> https://github.com/omnifaces/mojarra/commit/6dd5dd1b4095cc8dbec62021ba948379aa4b51bd
>>>
>>> I specifically wonder what Manfred thinks about this, as he introduced
>>> this switch.
>>>
>>> Kind regards,
>>> Arjan Tijms
>>>
>>>
>>>>
>>>>
>>>>
>>>> On Wednesday, June 10, 2015, arjan tijms <arjan.tijms_at_gmail.com> wrote:
>>>>>
>>>>> Hi,
>>>>>
>>>>> On Wed, Jun 10, 2015 at 2:42 PM, Kito Mann <kito.mann_at_virtua.com>
>>>>> wrote:
>>>>>>
>>>>>> I didn't review the code, but one thing I wonder about is the
>>>>>> inability to register a custom DataModel for types like List. That is a
>>>>>> pretty common use case.
>>>>>
>>>>>
>>>>> Yes, this is a design decision we as an EG have to make. Maybe there's
>>>>> even some compromise possible. E.g. using the same 2.3 switch that's used
>>>>> for CDI resolving to activate this feature, and then when activated in "2.3
>>>>> mode" do give it the ability to override List and such. Implementation of
>>>>> that is trivial (just move the check up in the list)
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> On Wednesday, June 10, 2015, arjan tijms <arjan.tijms_at_gmail.com>
>>>>>> wrote:
>>>>>>>
>>>>>>> Hi,
>>>>>>>
>>>>>>> Just wondering, anyone has any feedback on the changebundle?
>>>>>>>
>>>>>>> Kind regards,
>>>>>>> Arjan
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On Sun, Jun 7, 2015 at 11:45 PM, arjan tijms <arjan.tijms_at_gmail.com>
>>>>>>> wrote:
>>>>>>>>
>>>>>>>> Hi there,
>>>>>>>>
>>>>>>>> I just attached the changebundle for this at:
>>>>>>>> https://java.net/jira/secure/attachment/54975/changebundle.txt
>>>>>>>>
>>>>>>>> The main and only addition to the public API is the annotation
>>>>>>>> @FacesDataModel that when applied to a user provided DataModel registers it
>>>>>>>> with the runtime. Via an attribute it declares the class it's able to wrap.
>>>>>>>>
>>>>>>>> As for the Implementation, those classes are collected in
>>>>>>>> CdiExtension#processBean
>>>>>>>>
>>>>>>>> After the collecting phase the collected classes are sorted in
>>>>>>>> CdiExtension#processBean such that if an instance of any class from the
>>>>>>>> collection would be a subtype of any other class in that collection, it has
>>>>>>>> to appear before it. Apart from this constraint the order does not matter.
>>>>>>>>
>>>>>>>> This sorting allows a sequential scan through the collection to find
>>>>>>>> the best matching type. E.g. suppose the collection would contain List.class
>>>>>>>> and Collection.class, then List.class would appear before Collection.class.
>>>>>>>> Given an instance of ArrayList, List would match before Collection.
>>>>>>>>
>>>>>>>> The resulting collection is then made available via CDI in
>>>>>>>> application scope, albeit via an implementation specific name. For Mojarra I
>>>>>>>> choose "comSunFacesDataModelClassesMap" for this name.
>>>>>>>>
>>>>>>>> Finally UIData obtains said collection via CDI and does a
>>>>>>>> filter/findFirst on it checking if the object for which a DataModel wrapper
>>>>>>>> needs to be found is assignable to the type that each DataModel declared to
>>>>>>>> be able to handle. As explained above, the first match is the best batch.
>>>>>>>>
>>>>>>>> One design decision was to have the custom DataModel check to be
>>>>>>>> done *after* all existing checks for wrappers are done (e.g. the hardcoded
>>>>>>>> checks for List, Collection, Iterable, etc). This means that a custom
>>>>>>>> DataModel cannot override those types. This was done to ensure maximum
>>>>>>>> backwards compatibility, although one could argue it's more powerful to
>>>>>>>> check for the custom DataModels first.
>>>>>>>>
>>>>>>>> Thoughts?
>>>>>>>>
>>>>>>>> Kind regards,
>>>>>>>> Arjan Tijms
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> On Wed, Jun 3, 2015 at 1:04 AM, arjan tijms <arjan.tijms_at_gmail.com>
>>>>>>>> wrote:
>>>>>>>>>
>>>>>>>>> Hi y'all,
>>>>>>>>>
>>>>>>>>> Just some quick update on this. After a small hiatus I returned to
>>>>>>>>> this issue and started implementing it. There were a few small challenges
>>>>>>>>> that made this issue a bit more difficult than initially anticipated.
>>>>>>>>>
>>>>>>>>> Specifically the fact that registering a DataModel for say
>>>>>>>>> java.util.List requires some clever matching of class types, as the runtime
>>>>>>>>> object encountered can be e.g. an ArrayList or whatever sub type, and the
>>>>>>>>> "whatever sub type" can have a chain of super classes and a tree of
>>>>>>>>> interfaces and their super interfaces, where each super class can also have
>>>>>>>>> its own tree of interfaces and their super interfaces.
>>>>>>>>>
>>>>>>>>> A small hindrance is also that UIData (of course) contains
>>>>>>>>> implementation code, but it's in the API project of Mojarra which
>>>>>>>>> can't/doesn't have utility classes. So UIData and UIRepeat can't share
>>>>>>>>> between each other directly, nor can either of them use a utility class from
>>>>>>>>> the impl. project.
>>>>>>>>>
>>>>>>>>> Nevertheless I'm making good progress and hope to present a
>>>>>>>>> changebundle for evaluation soon.
>>>>>>>>>
>>>>>>>>> Kind regards,
>>>>>>>>> Arjan Tijms
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On Tue, Mar 10, 2015 at 9:17 AM, Hanspeter <hampidu_at_gmail.com>
>>>>>>>>> wrote:
>>>>>>>>>>
>>>>>>>>>> +1
>>>>>>>>>>
>>>>>>>>>> Hanspeter
>>>>>>>>>>
>>>>>>>>>> Am 09.03.2015 20:41 schrieb "Frank Caputo" <frank_at_frankcaputo.de>:
>>>>>>>>>>>
>>>>>>>>>>> +1
>>>>>>>>>>>
>>>>>>>>>>> Ciao Frank
>>>>>>>>>>>
>>>>>>>>>>> Am 09.03.2015 um 17:31 schrieb Bauke Scholtz <balusc_at_gmail.com>:
>>>>>>>>>>>
>>>>>>>>>>> +1
>>>>>>>>>>>
>>>>>>>>>>> Cheers, B
>>>>>>>>>>>
>>>>>>>>>>> On Sun, Mar 8, 2015 at 11:22 PM, arjan tijms
>>>>>>>>>>> <arjan.tijms_at_gmail.com> wrote:
>>>>>>>>>>>>
>>>>>>>>>>>> Hi,
>>>>>>>>>>>>
>>>>>>>>>>>> As the third part of extending the data model wrappers for
>>>>>>>>>>>> UIData and
>>>>>>>>>>>> UIRepeat, it was suggested to make data model wrappers
>>>>>>>>>>>> registrable by
>>>>>>>>>>>> the user. Issue JAVASERVERFACES_SPEC_PUBLIC-1078 was created for
>>>>>>>>>>>> this,
>>>>>>>>>>>> but it was also mentioned as part of
>>>>>>>>>>>> JAVASERVERFACES_SPEC_PUBLIC-1103.
>>>>>>>>>>>>
>>>>>>>>>>>> I took a quick look, and now with CDI available it seems almost
>>>>>>>>>>>> trivial to implement this.
>>>>>>>>>>>>
>>>>>>>>>>>> JSF would provide a CDI qualifier annotation like:
>>>>>>>>>>>>
>>>>>>>>>>>> @Retention(RUNTIME)
>>>>>>>>>>>> @Target(TYPE)
>>>>>>>>>>>> @Inherited
>>>>>>>>>>>> @Qualifier
>>>>>>>>>>>> public @interface FacesDataModel {
>>>>>>>>>>>> Class<?> forClass();
>>>>>>>>>>>> }
>>>>>>>>>>>>
>>>>>>>>>>>> The user then provides something like the following:
>>>>>>>>>>>>
>>>>>>>>>>>> @FacesDataModel(forClass=MyObject.class)
>>>>>>>>>>>> public class MyDataModel<E> extends DataModel<E> {
>>>>>>>>>>>> // datamodel methods
>>>>>>>>>>>> }
>>>>>>>>>>>>
>>>>>>>>>>>> The runtime then looks up an instance of that via CDI (just like
>>>>>>>>>>>> happens now for CD Converters and Validators, which is almost a
>>>>>>>>>>>> one-liner in Manfred's CDIUtils).
>>>>>>>>>>>>
>>>>>>>>>>>> Thoughts?
>>>>>>>>>>>>
>>>>>>>>>>>> Kind regards,
>>>>>>>>>>>> Arjan Tijms
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>>
>>>>>> --
>>>>>> ___
>>>>>>
>>>>>> Kito D. Mann | @kito99 | Author, JSF in Action
>>>>>> Virtua, Inc. | http://www.virtua.com | JSF/Java EE training and
>>>>>> consulting
>>>>>> http://www.JSFCentral.com | @jsfcentral
>>>>>> +1 203-998-0403
>>>>>>
>>>>>> * Listen to the Enterprise Java Newscast:
>>>>>> http://enterprisejavanews.com
>>>>>> * JSFCentral Interviews Podcast:
>>>>>> http://www.jsfcentral.com/resources/jsfcentralpodcasts/
>>>>>> * Sign up for the JSFCentral Newsletter:
>>>>>> http://oi.vresp.com/?fid=ac048d0e17
>>>>>>
>>>>>
>>>>
>>>>
>>>> --
>>>> ___
>>>>
>>>> Kito D. Mann | @kito99 | Author, JSF in Action
>>>> Virtua, Inc. | http://www.virtua.com | JSF/Java EE training and
>>>> consulting
>>>> http://www.JSFCentral.com | @jsfcentral
>>>> +1 203-998-0403
>>>>
>>>> * Listen to the Enterprise Java Newscast: http://enterprisejavanews.com
>>>> * JSFCentral Interviews Podcast:
>>>> http://www.jsfcentral.com/resources/jsfcentralpodcasts/
>>>> * Sign up for the JSFCentral Newsletter:
>>>> http://oi.vresp.com/?fid=ac048d0e17
>>>>
>>>
>>
>