Hi,
MR> The rationale behind this is:
> MR> "Adding it at the end is adding new behavior"
> MR> "Putting it up higher is changing existing behavior".
Correction to last point: " ... when a 3rd party component library is
included which changed existing behavior".
Cheers, B
On Fri, Jul 10, 2015 at 5:49 PM, arjan tijms <arjan.tijms_at_gmail.com> wrote:
> 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>
>> 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>
>>> 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
>>> >>>>
>>> >>>
>>> >>
>>> >
>>>
>>
>>