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

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

From: Edward Burns <edward.burns_at_oracle.com>
Date: Wed, 15 Feb 2017 09:43:05 -0800

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?

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.

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?

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.

>>>>> 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.

Thanks,

Ed

-- 
| edward.burns_at_oracle.com | office: +1 407 458 0017
| 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