Hi,
On Friday, July 10, 2015, Bauke Scholtz <balusc_at_gmail.com> wrote:
> Hi,
>
> Indeed, in corner case as described in #3 the user is itself the
> responsible for consequences, not JSF.
>
> Go ahead with checking it early.
>
Okay, +1 noted ;)
> Only those ArrayDataModel checks can be simplified as if(current
> instanceof Object[]) or if(current.getClass().isArray()).
>
There are a number of things in the method that can be simplified. I'll
create an issue to do these seperately. Thanks!
Kind regards,
Arjan
>
> Cheers, B
>
>
> On Thu, Jul 9, 2015 at 9:35 PM, arjan tijms <arjan.tijms_at_gmail.com
> <javascript:_e(%7B%7D,'cvml','arjan.tijms_at_gmail.com');>> wrote:
>
>> 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
>> <javascript:_e(%7B%7D,'cvml','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
>> <javascript:_e(%7B%7D,'cvml','arjan.tijms_at_gmail.com');>> wrote:
>> >
>> > Hi,
>> >
>> > On Monday, June 15, 2015, Josh Juneau <juneau001_at_gmail.com
>> <javascript:_e(%7B%7D,'cvml','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
>> <javascript:_e(%7B%7D,'cvml','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
>> <javascript:_e(%7B%7D,'cvml','arjan.tijms_at_gmail.com');>>
>> >> wrote:
>> >>>
>> >>> Hi,
>> >>>
>> >>> On Wed, Jun 10, 2015 at 3:20 PM, Kito Mann <kito.mann_at_virtua.com
>> <javascript:_e(%7B%7D,'cvml','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
>> <javascript:_e(%7B%7D,'cvml','arjan.tijms_at_gmail.com');>> wrote:
>> >>>>>
>> >>>>> Hi,
>> >>>>>
>> >>>>> On Wed, Jun 10, 2015 at 2:42 PM, Kito Mann <kito.mann_at_virtua.com
>> <javascript:_e(%7B%7D,'cvml','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
>> <javascript:_e(%7B%7D,'cvml','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
>> <javascript:_e(%7B%7D,'cvml','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
>> <javascript:_e(%7B%7D,'cvml','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
>> <javascript:_e(%7B%7D,'cvml','hampidu_at_gmail.com');>>
>> >>>>>>>>> wrote:
>> >>>>>>>>>>
>> >>>>>>>>>> +1
>> >>>>>>>>>>
>> >>>>>>>>>> Hanspeter
>> >>>>>>>>>>
>> >>>>>>>>>> Am 09.03.2015 20:41 schrieb "Frank Caputo" <
>> frank_at_frankcaputo.de
>> <javascript:_e(%7B%7D,'cvml','frank_at_frankcaputo.de');>>:
>> >>>>>>>>>>>
>> >>>>>>>>>>> +1
>> >>>>>>>>>>>
>> >>>>>>>>>>> Ciao Frank
>> >>>>>>>>>>>
>> >>>>>>>>>>> Am 09.03.2015 um 17:31 schrieb Bauke Scholtz <
>> balusc_at_gmail.com <javascript:_e(%7B%7D,'cvml','balusc_at_gmail.com');>>:
>> >>>>>>>>>>>
>> >>>>>>>>>>> +1
>> >>>>>>>>>>>
>> >>>>>>>>>>> Cheers, B
>> >>>>>>>>>>>
>> >>>>>>>>>>> On Sun, Mar 8, 2015 at 11:22 PM, arjan tijms
>> >>>>>>>>>>> <arjan.tijms_at_gmail.com
>> <javascript:_e(%7B%7D,'cvml','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
>> >>>>
>> >>>
>> >>
>> >
>>
>
>