dev@javaserverfaces.java.net

Re: Code review request for https://java.net/jira/browse/JAVASERVERFACES-3582

From: arjan tijms <arjan.tijms_at_gmail.com>
Date: Wed, 11 Feb 2015 13:02:14 +0100

Hi Zhijun,

If I understand correctly you'd like to add the public Object
getValue() method to UIInput in the Mojarra 2.1 (JSF 2.1) branch?

> So what's your concern? I can't take any action before you make me
> understand what you mean.

If my understanding of the issue is correct, then the problem is that
you'll be making a change to the public API of UIInput.

From an implementer and user perspective this is sometimes hard to
understand, as indeed the exact same method is already present in the
base class. For callers (users), nothing would change, so the change
would seem "safe".

Yet, from a spec perspective it IS a change to the public API (Manfred
uses the term "signature" loosely here, to refer to the entire API,
not just a single method). Even JavaDoc corrections constitute such a
change, and these too will not have any effect on the caller.

Again loosely speaking, the public API is basically what you see here:
http://docs.oracle.com/javaee/6/api/javax/faces/component/UIInput.html

As you can see, public Object getValue() is not in the list of methods
beneath "Method Summary". If you would add it, that list would change,
and http://docs.oracle.com/javaee/6/api/javax/faces/component/UIInput.html
has to be republished (as it would not be correct otherwise). On top
of that, the TCK would have to be updated (which can not only check
for the existence of methods that should be there, but if I'm not
mistaken also for the existence of methods that should NOT be there).

Then, you would have to convince Leonardo from MyFaces that this
method needs to be added, and he has to implement it in MyFaces as
well, exactly like the JavaDoc says it should (since it's a public
method on a spec class). If you wouldn't do that and
http://docs.oracle.com/javaee/6/api/javax/faces/component/UIInput.html
was updated for Mojarra, then it would only apply to Mojarra and not
to MyFaces anymore. The spec explicitly prohibits this.

There is a process though for all of this work, and that's the JCP MR
process. I think you could *theoretically* start that, but I'm not
sure how feasible it would be to start a MR process for JSF 2.1 at
this moment, seeing how JSF 2.2 is the current version and JSF 2.3 is
being developed now. Maybe it's not even possible (but you could ask
Ed or Heather to be sure).

At any length, an MR process should not be taken lightly, hence I
guess Manfred's request to see if the bug can be fixed in any other
way.

Kind regards,
Arjan





On Wed, Feb 11, 2015 at 3:35 AM, zhijun Ren <ren.zhijun_at_oracle.com> wrote:
> Hi Manfred,
>
> Let me clarify the concept things:
>
> Signature means the way how the caller do the invocation: method name, input
> parameters and the expected return values;
>
> Override changes what the method do, means the behavior changes.
>
> The key point here is:
> 1. The feature UIRepeat is also in 2.1.20 and the complained issue is also
> there;
> 2. I have done test locally which covered the tests set which is same as
> that in Hudson and didn't find any of the Spec tests was broken.
>
> Briefly, the issue exists and the fixing works and no legacy tests broken.
>
> So what's your concern? I can't take any action before you make me
> understand what you mean.
>
> BR,
> Zhijun
>
>
>
> On 2/10/15, 21:56, manfred riem wrote:
>
> Hi Zhijun,
>
> An override is a signature change as well. Ed, can you please weigh in as
> well?
>
> Thanks!
> Manfred
>
> On 2/9/15, 7:29 PM, zhijun Ren wrote:
>
> Hi Manfred,
>
> Why do you think it is a signature change? it is overriding, the signature
> is the same as it's parent class UIOutput:
>
> public Object getValue()
>
> and the code is now the same in 2.3.
>
> The issue complained by #2717 also exists in 2.1.20 which also have the
> UIRepeat feature.
>
> Maybe I am not catching your real meaning, can you make it more clear?
>
> BR,
> Zhijun
>
> On 2/9/15, 22:44, manfred riem wrote:
>
> Hi Zhijun,
>
> Unfortunately you cannot commit this as you are changing an API signature.
>
> As we discussed before for this issue you need to look to see if the test
> can be made to pass without changing the API signature.
>
> If that is not possible then we need to make sure the test itself is
> adjusted.
>
> Please verify the behavior against an older 2.1 release.
>
> Kind regards,
> Manfred
>
>
> On 2/9/15, 2:12 AM, zhijun Ren wrote:
>
> Hi Manfred and Ed,
>
> Please help to review the code attached on 20150209, please note that I have
> all the Hudson test cases locally against WLS and didn't found any failed
> test cases.
>
> attached the change here for your convenience.
>
> BR,
> Zhijun
>
>