You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by Violeta Georgieva <mi...@gmail.com> on 2013/07/11 14:49:40 UTC
Couple of question about javax.el.BeanELResolver
Hi,
I have the following question about:
1. Method setValue
There is some mismatch in the javadoc. In the method description there is
the following statement:
"If the property exists but does not have a setter, then a
PropertyNotFoundException is thrown."
But the throws clause states:
"PropertyNotWritableException - if this resolver was constructed in
read-only mode, or if there is no setter for the property."
Our implementation currently throws PropertyNotFoundException.
I'm going to change it to throw PropertyNotWritableException.
What do you think?
Also I'm going to report a bug for this against the spec.
2. Method isReadOnly
There is not explicit statement what should be returned in case
base/property are null.
However there is statement that:
"If this resolver was constructed in read-only mode, this method will
always return true."
What do you think about this change:
@@ -195,7 +195,7 @@
throw new NullPointerException();
}
if (base == null || property == null) {
- return false;
+ return readOnly;
}
context.setPropertyResolved(base, property);
Thanks
Violeta
Re: Couple of question about javax.el.BeanELResolver
Posted by Violeta Georgieva <mi...@gmail.com>.
2013/7/11 Mark Thomas wrote:
>
> On 11/07/2013 13:49, Violeta Georgieva wrote:
> > Hi,
> >
> > I have the following question about:
> >
> > 1. Method setValue
> > There is some mismatch in the javadoc. In the method description there
is
> > the following statement:
> >
> > "If the property exists but does not have a setter, then a
> > PropertyNotFoundException is thrown."
> >
> > But the throws clause states:
> >
> > "PropertyNotWritableException - if this resolver was constructed in
> > read-only mode, or if there is no setter for the property."
> >
> > Our implementation currently throws PropertyNotFoundException.
> > I'm going to change it to throw PropertyNotWritableException.
> > What do you think?
>
> On balance, PropertyNotWritableException seems the better option in this
> case.
>
> > Also I'm going to report a bug for this against the spec.
>
> +1.
thanks
>
> > 2. Method isReadOnly
> > There is not explicit statement what should be returned in case
> > base/property are null.
> > However there is statement that:
> >
> > "If this resolver was constructed in read-only mode, this method will
> > always return true."
> >
> > What do you think about this change:
> >
> > @@ -195,7 +195,7 @@
> > throw new NullPointerException();
> > }
> > if (base == null || property == null) {
> > - return false;
> > + return readOnly;
> > }
>
> If base != null and property == null my reading of the javadoc is a
> PropertyNotFoundException should be thrown.
you are right on a second reading this should throw exception
>
> Generally, I think there needs to be a tightening up of language
> throughout the EL Javadoc. I'm not sure how best to raise this with the
> EG. A bug per ambiguity could be a lot of bugs but I suspect a single
> bug won't result in the issue being fully addressed. Maybe one bug per
> class is the way to go and list all the ambiguities in the Javadoc for
> that class.
>
> Mark
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
Re: Couple of question about javax.el.BeanELResolver
Posted by Mark Thomas <ma...@apache.org>.
On 11/07/2013 13:49, Violeta Georgieva wrote:
> Hi,
>
> I have the following question about:
>
> 1. Method setValue
> There is some mismatch in the javadoc. In the method description there is
> the following statement:
>
> "If the property exists but does not have a setter, then a
> PropertyNotFoundException is thrown."
>
> But the throws clause states:
>
> "PropertyNotWritableException - if this resolver was constructed in
> read-only mode, or if there is no setter for the property."
>
> Our implementation currently throws PropertyNotFoundException.
> I'm going to change it to throw PropertyNotWritableException.
> What do you think?
On balance, PropertyNotWritableException seems the better option in this
case.
> Also I'm going to report a bug for this against the spec.
+1.
> 2. Method isReadOnly
> There is not explicit statement what should be returned in case
> base/property are null.
> However there is statement that:
>
> "If this resolver was constructed in read-only mode, this method will
> always return true."
>
> What do you think about this change:
>
> @@ -195,7 +195,7 @@
> throw new NullPointerException();
> }
> if (base == null || property == null) {
> - return false;
> + return readOnly;
> }
If base != null and property == null my reading of the javadoc is a
PropertyNotFoundException should be thrown.
Generally, I think there needs to be a tightening up of language
throughout the EL Javadoc. I'm not sure how best to raise this with the
EG. A bug per ambiguity could be a lot of bugs but I suspect a single
bug won't result in the issue being fully addressed. Maybe one bug per
class is the way to go and list all the ambiguities in the Javadoc for
that class.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: Couple of question about javax.el.BeanELResolver
Posted by Violeta Georgieva <mi...@gmail.com>.
2013/7/11 Konstantin Kolinko wrote:
>
> 2013/7/11 Violeta Georgieva <mi...@gmail.com>:
> > Hi,
> >
> > I have the following question about:
> >
> > 1. Method setValue
> > There is some mismatch in the javadoc. In the method description there
is
> > the following statement:
> >
> > "If the property exists but does not have a setter, then a
> > PropertyNotFoundException is thrown."
> >
> > But the throws clause states:
> >
> > "PropertyNotWritableException - if this resolver was constructed in
> > read-only mode, or if there is no setter for the property."
> >
> > Our implementation currently throws PropertyNotFoundException.
> > I'm going to change it to throw PropertyNotWritableException.
> > What do you think?
> >
> > Also I'm going to report a bug for this against the spec.
> >
>
> The javadoc at
http://docs.oracle.com/javaee/7/api/javax/el/BeanELResolver.html
> says:
>
> "If this resolver was constructed in read-only mode, this method will
> always throw PropertyNotWritableException."
>
> I think it is what should actually happen. It is also easier to check
> a boolean property either than use reflection to lookup whether a
> property exists. I see no specification bug here.
There is no bug in the specification, but in the javadoc.
I meant to open a bug to ask for a consistent javadoc.
> > 2. Method isReadOnly
> > There is not explicit statement what should be returned in case
> > base/property are null.
> > However there is statement that:
> >
> > "If this resolver was constructed in read-only mode, this method will
> > always return true."
> >
>
> The same as for ResourceBundleELResolver. (That resolver is inherently
> read-only but you recently changed it to return false in a similar
> method).
>
> The return value does not matter if the property has not been marked
> as resolved. The docs require the caller to ignore the return value in
> such a case.
I agree.
Re: Couple of question about javax.el.BeanELResolver
Posted by Konstantin Kolinko <kn...@gmail.com>.
2013/7/11 Violeta Georgieva <mi...@gmail.com>:
> Hi,
>
> I have the following question about:
>
> 1. Method setValue
> There is some mismatch in the javadoc. In the method description there is
> the following statement:
>
> "If the property exists but does not have a setter, then a
> PropertyNotFoundException is thrown."
>
> But the throws clause states:
>
> "PropertyNotWritableException - if this resolver was constructed in
> read-only mode, or if there is no setter for the property."
>
> Our implementation currently throws PropertyNotFoundException.
> I'm going to change it to throw PropertyNotWritableException.
> What do you think?
>
> Also I'm going to report a bug for this against the spec.
>
The javadoc at http://docs.oracle.com/javaee/7/api/javax/el/BeanELResolver.html
says:
"If this resolver was constructed in read-only mode, this method will
always throw PropertyNotWritableException."
I think it is what should actually happen. It is also easier to check
a boolean property either than use reflection to lookup whether a
property exists. I see no specification bug here.
> 2. Method isReadOnly
> There is not explicit statement what should be returned in case
> base/property are null.
> However there is statement that:
>
> "If this resolver was constructed in read-only mode, this method will
> always return true."
>
The same as for ResourceBundleELResolver. (That resolver is inherently
read-only but you recently changed it to return false in a similar
method).
The return value does not matter if the property has not been marked
as resolved. The docs require the caller to ignore the return value in
such a case.
> What do you think about this change:
>
> @@ -195,7 +195,7 @@
> throw new NullPointerException();
> }
> if (base == null || property == null) {
> - return false;
> + return readOnly;
> }
>
> context.setPropertyResolved(base, property);
>
Best regards,
Konstantin Kolinko
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org