Author: kkolinko
Date: Sat Feb 13 17:55:11 2010
New Revision: 909860
URL: http://svn.apache.org/viewvc?rev=909860&view=rev
Log:
split EL fixes vote into individual patches, for clarity
veto the Enum EL patch backport
Modified:
tomcat/tc6.0.x/trunk/STATUS.txt
Modified: tomcat/tc6.0.x/trunk/STATUS.txt
URL:
http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/STATUS.txt?rev=909860&r1=909859&r2=909860&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/STATUS.txt (original)
+++ tomcat/tc6.0.x/trunk/STATUS.txt Sat Feb 13 17:55:11 2010
@@ -107,59 +107,64 @@
* Fix various EL TCK failures
http://svn.apache.org/viewvc?view=rev&rev=899653 (signatures)
+ +1: markt, kkolinko
+ -1:
+
http://svn.apache.org/viewvc?view=rev&rev=899769 (CCE expected)
http://svn.apache.org/viewvc?view=rev&rev=899770 (CCE expected)
+ +1: markt, kkolinko
+ -1:
+ kkolinko: Maybe better name for that message, because it says about
+ arrays, yet name is rather generic
+
http://svn.apache.org/viewvc?view=rev&rev=899783 (ELException expected)
+ +1: markt, kkolinko
+ -1:
+
http://svn.apache.org/viewvc?view=rev&rev=899788 (PNFE expected)
+ +1: markt, kkolinko
+ -1:
+ kkolinko: I think
o.a.jasper.el.ELResolverImpl#getType(ELContext,Object,Object)
+ should likewise throw a PropertyNotFoundException instead of returning
null.
+ I have no test, though.
+
http://svn.apache.org/viewvc?view=rev&rev=899792 (ELException rather than
IAE)
+ +1: markt, kkolinko
+ -1:
+
http://svn.apache.org/viewvc?view=rev&rev=899916 (ELException rather than
IAE)
+ +1: markt, kkolinko
+ -1:
+
http://svn.apache.org/viewvc?view=rev&rev=899918 (Enum coercion test cases)
http://svn.apache.org/viewvc?view=rev&rev=899919 (Enum coercion bug)
+ +1: markt
+ -1: kkolinko:
+ As far as I am reading, there is no provision in the EL
+ spec, that enum -> enum conversion can be performed by using its
+ string value.
+
+ The current TC 6.0 behaviour here will be a ClassCastException, trying
+ to assign the value, and that should be an ELException instead.
+
+ In EL 2.1 and 2.2 specifications chapter 1.18.6 'Coerce A to an Enum Type
T' says:
+ "If A is a String call Enum.valueOf(T.getClass(), A) and return the
result."
+ Thus, our
+ result = Enum.valueOf(type, obj.toString());
+ should only be applicable if obj is a String.
+
+ In ELSupport#coerceToType(Object, Class), that implements 1.18.7, we
+ already throw an ELException if A is not a String.
+
http://svn.apache.org/viewvc?view=rev&rev=899935 (ELException expected)
- http://svn.apache.org/viewvc?view=rev&rev=899949 (ignore whitespace on comp)
- +1: markt
- +1: kkolinko:
- 899653: OK. We do not have @Deprecated annotations in those classes,
- so the patch is about adding @SuppressWarnings("dep-ann")
- 899769: With 899770 that backports the message string used here.
- 899770: OK
- (Maybe better name for that message, because it says about arrays,
- yet name is rather generic).
- 899783: OK
- 899788: OK
- (Likewise,
o.a.jasper.el.ELResolverImpl#getType(ELContext,Object,Object)
- should probably throw a PropertyNotFoundException, instead of
returning null.
- I have no proof, though.)
- 899792: OK
- 899916: OK
-
- 899918, 899919: OK, but there is probably an omission in the EL spec:
- I do not see why we do conversion Enum->Enum via toString() call.
-
- The EL spec chapter 1.18.6 'Coerce A to an Enum Type T' says
- "If A is a String call Enum.valueOf(T.getClass(), A) and return
the result."
- It does not say what to do if A is not a String. (There is no
- explicit "Otherwise, error" statement below).
-
- In 1.18.7 (aka ELSupport#coerceToType(Object, Class)) we throw
- an error if A is not a String. Even if T has a PropertyEditor,
- we do not do editor.setAsText(obj.toString()), as the spec does
- not say to do so, but throw an exception.
-
- (In 1.18.7 the spec says "Otherwise, apply T's PropertyEditor",
- but PropertyEditor can be applied only is A is a String. Am I right?)
-
- Without 899919 patch we will throw a ClassCaseException when object
type
- is a different type of enum, but other values are still converted
- via toString() call. The patch makes that behaviour consistent, even
- if I do not understand why it is allowed.
-
- 899935: OK
- 899949: OK,
- but why ValueExpressionImpl.equals() is implemented as comparing
- the hash codes? What will happen with false positives?
+ +1: markt, kkolinko
+ -1:
- -1:
+ http://svn.apache.org/viewvc?view=rev&rev=899949 (ignore whitespace on comp)
+ +1: markt, kkolinko
+ -1:
+ kkolinko: Why ValueExpressionImpl.equals() is implemented as comparing
+ the hash codes? What will happen with false positives?
* Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=48627
Regression in re-working of EL parsing
@@ -176,7 +181,7 @@
Provide option to stop server if there is an error during init
Port of Filip's patch from trunk
http://svn.apache.org/viewvc?view=revision&revision=752323
- +1: markt
+ +1: markt, kkolinko
-1:
* Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=48645
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
