You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@locus.apache.org on 2000/04/06 20:29:01 UTC
cvs commit: jakarta-tomcat/src/share/org/apache/jasper/compiler TagBeginGenerator.java
mandar 00/04/06 11:29:00
Modified: src/share/org/apache/jasper/compiler TagBeginGenerator.java
Log:
Fix for bug #109.
JspRuntimeLibrary breaks taglibs with multiple property setters.
Revision Changes Path
1.11 +17 -14 jakarta-tomcat/src/share/org/apache/jasper/compiler/TagBeginGenerator.java
Index: TagBeginGenerator.java
===================================================================
RCS file: /home/cvs/jakarta-tomcat/src/share/org/apache/jasper/compiler/TagBeginGenerator.java,v
retrieving revision 1.10
retrieving revision 1.11
diff -u -r1.10 -r1.11
--- TagBeginGenerator.java 2000/03/28 04:29:48 1.10
+++ TagBeginGenerator.java 2000/04/06 18:29:00 1.11
@@ -179,21 +179,24 @@
String attrValue = (String) attrs.get(attributes[i].getName());
if (attrValue != null) {
- String attrName = attributes[i].getName();
+ if (attributes[i].canBeRequestTime()) {
+ if (JspUtil.isExpression(attrValue))
+ attrValue = JspUtil.getExpr(attrValue);
+ else
+ attrValue = writer.quoteString(attrValue);
+ } else
+ attrValue = writer.quoteString(attrValue);
- if (!JspUtil.isExpression (attrValue)) {
- writer.println("JspRuntimeLibrary.introspecthelper(" +
- thVarName + ",\"" + attrName +
- "\",\"" + JspUtil.escapeQueryString(attrValue) +
- "\",null,null,false);");
- } else {
-
- // This requires some careful handling.
- // int, boolean, ... are not Object(s).
- writer.println("JspRuntimeLibrary.handleSetProperty(" +
- thVarName + ",\"" + attrName +
- "\"," + JspUtil.getExpr(attrValue) + ");");
- }
+ String attrName = attributes[i].getName();
+ Method m = tc.getSetterMethod(attrName);
+
+ if (m == null)
+ throw new JasperException
+ (Constants.getString
+ ("jsp.error.unable.to_find_method",
+ new Object[] { attrName }));
+
+ writer.println(thVarName+"."+m.getName()+"("+attrValue+");");
}
}
}
Re: cvs commit: jakarta-tomcat/src/share/org/apache/jasper/compiler
TagBeginGenerator.java
Posted by Costin Manolache <Co...@eng.sun.com>.
"Anil K. Vijendran" wrote:
> Hans Bergsten wrote:
>
> > I added a CC to Eduardo again. He's on a business trip right now, but I hope
> > he jumps in as soon as he's back.
>
> I have a feeling he's in Spain with poor or no connectivity.
>
> > I feel that we must resolve this before the release of Tomcat 3.1, since 3.1
> > will
> > likely be the benchmark for all containers that's currently in Beta, ready to
> > be released soon. How to handle custom action attributes is extremely important
> > to get right in all containers for true portability of tag libraries.
>
> I agree with you.
>
> Actually on a slightly different note there are a few other bugs (<10) that we should
> certainly fix before calling Tomcat 3.1 final. In addition to that I think we should
> have a 3.1 branch and make a final release and continue bugfixing it with 3.1.x
> releases. I doubt if we can abandon 3.1 like we did 3.0 (well almost except one
> customer -- the J2EE implementation).
What about setting a tag and going on with 3.2 ? It's almost a month of bug-fixing,
and it seems only few people are doing that - it would be nice to let the other
go ahead.
Tomcat 3.1 is still missing features ( like authentication ) - and we need to go on with
some performance tuning, more cleanup, etc - all of this is too dangerous for 3.1.
Tomcat 3.1 is better than 3.0, 3.2 will be even better - we spend too much time
fixing ( and many bugs will require more than a simple fix ).
I think all of the targets for 3.1 are hit, and we can't make it 100% bug-free or
implement all of the missing features as bug-fixes.
Costin
Re: cvs commit: jakarta-tomcat/src/share/org/apache/jasper/compiler
TagBeginGenerator.java
Posted by "Anil K. Vijendran" <An...@eng.sun.com>.
Hans Bergsten wrote:
> I added a CC to Eduardo again. He's on a business trip right now, but I hope
> he jumps in as soon as he's back.
I have a feeling he's in Spain with poor or no connectivity.
> I feel that we must resolve this before the release of Tomcat 3.1, since 3.1
> will
> likely be the benchmark for all containers that's currently in Beta, ready to
> be released soon. How to handle custom action attributes is extremely important
> to get right in all containers for true portability of tag libraries.
I agree with you.
Actually on a slightly different note there are a few other bugs (<10) that we should
certainly fix before calling Tomcat 3.1 final. In addition to that I think we should
have a 3.1 branch and make a final release and continue bugfixing it with 3.1.x
releases. I doubt if we can abandon 3.1 like we did 3.0 (well almost except one
customer -- the J2EE implementation).
--
Peace, Anil +<:-)
Re: cvs commit: jakarta-tomcat/src/share/org/apache/jasper/compiler
TagBeginGenerator.java
Posted by Hans Bergsten <ha...@gefionsoftware.com>.
I added a CC to Eduardo again. He's on a business trip right now, but I hope
he jumps in as soon as he's back.
I feel that we must resolve this before the release of Tomcat 3.1, since 3.1
will
likely be the benchmark for all containers that's currently in Beta, ready to
be released soon. How to handle custom action attributes is extremely important
to get right in all containers for true portability of tag libraries.
Mandar Raje wrote:
> [...]
> What I am little curious about is that if our interpretation is correct, then
> how are we going to ensure that a bean does not use two different setters.
>
> Consider the example mentioned above once again. We have two
> setter methos set(String) and set(int) and both will be used at different times
> (compile-time and request-time values resp.). Now in Tomcat how do
> we ensure that a call to one method succeeds and the other one fails?
> And which one should fail?
Yeah, it's tricky. I have made some tests to see what the Introspector tells
us in different scenarios.
First, I suggest that whatever the java.beans.Introspector regards as
properties is what we also regard as properties. That's the only way to
be sure we stick to the spec, even though the Introspector sometimes makes
some interesting decisions now and then ... It doesn't like multiple
setters at all, see more below.
So for a "correct" bean with one setIntVal(int val) method, the Inspector
correctly reports that there's one property of type int named intVal. During
the translation phase, the container can use the Introspector on all tag
handler classes to find out about their properties. If a page contains
a <foo:bar intVal="8" /> the container can throw an exception, saying that
there's no intVal property with the type String defined for this tag. Or
better IMHO, we convince Eduardo that the spec should be clarified to say
that static String attributes for custom actions should be converted the
same way as setProperty attributes are converted. That was how I had expected
things to work until I looked for proof in the spec ...
A tag like <foo:bar intVal="<%= 8 %>" /> would result in code like
barHandler.setIntVal(8). This must be done without hints from the Introspector,
since the container can not figure out the type of the expression. Or can it?
I'm confused by the code in this area and can't see how it could at the
momement, even though JspRuntimeLibrary.handleSetProperty() seems to be
called with the value as an Object. If it can, it could of course use the
Introspector to verify that there's really an int setter for this property.
Beans with multiple setters are handled in a weird way by the Introspector,
likely due to a combination of bugs and the fact that the JavaBeans spec
doesn't specify support for multiple setters. My experiments show that
the Introspector reports that a bean like this:
public class OneReadTwoWriteBean {
private String val;
private int intVal;
public String getVal() {
return val;
}
public void setVal(String val) {
this.val = val;
}
public void setVal(int val) {
this.intVal = val;
}
}
has one property named val with the type String, with one write method
called setVal but *no* read method! In other words, multiple setters cause
the reader to be ignored (a bug in my book).
This means it's hard to treat "incorrect" beans (with multiple setters)
in a nice way. As far as I can see, the container can only handle this type
of "incorrect" bean like this. During the translation phase it uses the
Introspector, and if it finds a <foo:bar intVal="8" />, it checks if there's
a corresponding property with the type String. In this example, the Introspector
will say yes, so the container generates code like barHandler.setIntVal("8").
If the page also contains <foo:bar intVal="<%= 8 %>" />, the container
has no other choice than generating barHandler.setIntVal(8), since it can't
say if the expression results in a String or not. Again, I may be wrong here.
If it can figure out the type of the expression, at least at runtime, it
could say that there's no int setter for this bean. Otherwise I'm afraid
it means that "incorrect" beans work, at least in some cases ...
This clearly need to be straightened out, and I'm pretty sure it requires
a clarification in the spec. Should we (Mandar, Eduardo and me) continue this
thread off-line until we have a solid proposal?
Hans
--
Hans Bergsten hans@gefionsoftware.com
Gefion Software http://www.gefionsoftware.com
Re: cvs commit: jakarta-tomcat/src/share/org/apache/jasper/compiler
TagBeginGenerator.java
Posted by Mandar Raje <ma...@pathfinder.eng.sun.com>.
>
> > comment on this.
>
> Okay, the way the spec is written, the piece I quoted applies to setProperty
> and there's nothing in the spec (AFAIK) that says that this type of conversion
> should be done for custom actions (even though it would probably be a Good Thing
> to add this in the next spec).
>
> So, my revised take on your example is:
>
> If the bean has a setIntVal(int value) method,
>
> <foo:bar intVal="8" />
> the assignment is generated as
> barHandler.setIntVal("8");
> which will fail with a compilation error.
>
> <foo:bar intVal="<%= 8 %>" />
> the assignment is generated as
> barHandler.setIntVal(8);
>
> That's similar to what you suggested, but since you didn't mention that the
> String
> assignment would fail, I assumed you suggested that the bean had two setters.
> And that's a no-no, based on the JavaBeans spec plus RI implementation.
>
What I am little curious about is that if our interpretation is correct, then
how are we going to ensure that a bean does not use two different setters.
Consider the example mentioned above once again. We have two
setter methos set(String) and set(int) and both will be used at different times
(compile-time and request-time values resp.). Now in Tomcat how do
we ensure that a call to one method succeeds and the other one fails?
And which one should fail?
Mandar.
Re: cvs commit: jakarta-tomcat/src/share/org/apache/jasper/compiler
TagBeginGenerator.java
Posted by Hans Bergsten <ha...@gefionsoftware.com>.
MANDAR RAJE wrote:
>
> >
> > The problem with the JavaBeans spec and the implementation of the
> > classes in the java.beans package is that they don't allow for
> > multiple setter methods with the same name but different types.
> > Your interpretation requires a bean with
> >
> > setIntVal(String value)
> > setIntVal(int value)
> >
> > But the java.beans classes don't even consider intVal to be a property
> > since there's more than one setIntVal() method.
> >
> > I stopped anaylyzing this after some discussion with the people
> > I mentioned, plus playing around with the BeanBox, so it may not
> > be consistently "unsupported". But the fact is that the JavaBeans
> > spec defines a property as something with one well defined type.
> >
> > The JSP spec says:
> >
> > "Properties in a Bean can be set from one or more parameters in the
> > request object, from a String constant, or from a computed request-time
> > expression. Simple and indexed properties can be set using setProperty.
> > The only types of properties that can be assigned to from String
> > constants and request parameter values are those listed in TABLE 2-4;
> > the conversion applied is that shown in the table. Request-time expressions
> > can be assigned to properties of any type; no automatic conversions will
> > be performed."
> >
> > So my take on your examples is:
> >
> > If the bean has a setIntVal(int value) method,
> >
> > <foo:bar intVal="8" />
> > is assigned as
> > barHandler.setIntVal(Integer.valueOf("8"));
> >
> > <foo:bar intVal="<%= 8 %>" />
> > is assigned as
> > barHandler.setIntVal(8);
> >
> > Hans
>
> I also talked to Eduardo sometime back and (my recollection of)
> what he said was (to my surprise):
>
> "properties that are set for taghandlers should be treated
> differently than those set using setProperty tag. If you are
> using a String constant then method setXXX(String) will be
> called no matter what"
>
> I am cc'ing eduardo on this. Hopefully he will be able to
> comment on this.
Okay, the way the spec is written, the piece I quoted applies to setProperty
and there's nothing in the spec (AFAIK) that says that this type of conversion
should be done for custom actions (even though it would probably be a Good Thing
to add this in the next spec).
So, my revised take on your example is:
If the bean has a setIntVal(int value) method,
<foo:bar intVal="8" />
the assignment is generated as
barHandler.setIntVal("8");
which will fail with a compilation error.
<foo:bar intVal="<%= 8 %>" />
the assignment is generated as
barHandler.setIntVal(8);
That's similar to what you suggested, but since you didn't mention that the
String
assignment would fail, I assumed you suggested that the bean had two setters.
And that's a no-no, based on the JavaBeans spec plus RI implementation.
Hans
--
Hans Bergsten hans@gefionsoftware.com
Gefion Software http://www.gefionsoftware.com
Re: cvs commit: jakarta-tomcat/src/share/org/apache/jasper/compiler
TagBeginGenerator.java
Posted by MANDAR RAJE <ma...@pathfinder.eng.sun.com>.
>
> The problem with the JavaBeans spec and the implementation of the
> classes in the java.beans package is that they don't allow for
> multiple setter methods with the same name but different types.
> Your interpretation requires a bean with
>
> setIntVal(String value)
> setIntVal(int value)
>
> But the java.beans classes don't even consider intVal to be a property
> since there's more than one setIntVal() method.
>
> I stopped anaylyzing this after some discussion with the people
> I mentioned, plus playing around with the BeanBox, so it may not
> be consistently "unsupported". But the fact is that the JavaBeans
> spec defines a property as something with one well defined type.
>
> The JSP spec says:
>
> "Properties in a Bean can be set from one or more parameters in the
> request object, from a String constant, or from a computed request-time
> expression. Simple and indexed properties can be set using setProperty.
> The only types of properties that can be assigned to from String
> constants and request parameter values are those listed in TABLE 2-4;
> the conversion applied is that shown in the table. Request-time expressions
> can be assigned to properties of any type; no automatic conversions will
> be performed."
>
> So my take on your examples is:
>
> If the bean has a setIntVal(int value) method,
>
> <foo:bar intVal="8" />
> is assigned as
> barHandler.setIntVal(Integer.valueOf("8"));
>
> <foo:bar intVal="<%= 8 %>" />
> is assigned as
> barHandler.setIntVal(8);
>
> Hans
I also talked to Eduardo sometime back and (my recollection of)
what he said was (to my surprise):
"properties that are set for taghandlers should be treated
differently than those set using setProperty tag. If you are
using a String constant then method setXXX(String) will be
called no matter what"
I am cc'ing eduardo on this. Hopefully he will be able to
comment on this.
Mandar.
Re: cvs commit: jakarta-tomcat/src/share/org/apache/jasper/compiler
TagBeginGenerator.java
Posted by Hans Bergsten <ha...@gefionsoftware.com>.
MANDAR RAJE wrote:
>
> Hans Bergsten wrote:
> >
> > > JspRuntimeLibrary breaks taglibs with multiple property setters.
> >
> > Actually, I had an offline discussion about this feature (multiple setters)
> > with Eduardo, Larry and Janet Koenig, and the conclusion was that support
> > for multiple setters is not mandated by the JavaBeans spec and not supported
> > by the java.beans.* classes. It's therefore better, IMHO, to keep the code
> > as it was before this patch. Adding support for this feature, with no proper
> > support in the spec, in the Reference Implementation doesn't seem right.
> >
> > Hans
> > PS.Sorry for not reporting back to the list about the discussion.
>
> This is my interpretation on the subject:
>
> If you have:
>
> <foo:bar intVal="8" />
>
> then you should invoke:
>
> barHandler.setIntVal("8"); // Note the String arg.
>
> and not invoke
>
> barHandler.setIntVal(8);
>
> And when you have:
>
> <foo:bar intVal="<%= 8 %>" />
>
> you should invoke:
>
> barHandler.setIntVal(8);
>
> Maybe there is a slight difference between my interpretation
> and the correct one. Here it is:
>
> 1) When you specify the value as a String constant there is no
> difference.
> 2) When you specify it as a runtime expression the correct
> interpretation is that we should delegate the
> responsibility of finding the setter method to the
> PropertyDescriptor for the bean (which will select it at
> random) and not call the method based on the TYPE of the
> value.
>
> If this is correct, I will change the code that handles the
> runtime expression part.
The problem with the JavaBeans spec and the implementation of the
classes in the java.beans package is that they don't allow for
multiple setter methods with the same name but different types.
Your interpretation requires a bean with
setIntVal(String value)
setIntVal(int value)
But the java.beans classes don't even consider intVal to be a property
since there's more than one setIntVal() method.
I stopped anaylyzing this after some discussion with the people
I mentioned, plus playing around with the BeanBox, so it may not
be consistently "unsupported". But the fact is that the JavaBeans
spec defines a property as something with one well defined type.
The JSP spec says:
"Properties in a Bean can be set from one or more parameters in the
request object, from a String constant, or from a computed request-time
expression. Simple and indexed properties can be set using setProperty.
The only types of properties that can be assigned to from String
constants and request parameter values are those listed in TABLE 2-4;
the conversion applied is that shown in the table. Request-time expressions
can be assigned to properties of any type; no automatic conversions will
be performed."
So my take on your examples is:
If the bean has a setIntVal(int value) method,
<foo:bar intVal="8" />
is assigned as
barHandler.setIntVal(Integer.valueOf("8"));
<foo:bar intVal="<%= 8 %>" />
is assigned as
barHandler.setIntVal(8);
Hans
--
Hans Bergsten hans@gefionsoftware.com
Gefion Software http://www.gefionsoftware.com
Re: cvs commit: jakarta-tomcat/src/share/org/apache/jasper/compiler
TagBeginGenerator.java
Posted by MANDAR RAJE <ma...@pathfinder.eng.sun.com>.
Hans Bergsten wrote:
>
> > JspRuntimeLibrary breaks taglibs with multiple property setters.
>
> Actually, I had an offline discussion about this feature (multiple setters)
> with Eduardo, Larry and Janet Koenig, and the conclusion was that support
> for multiple setters is not mandated by the JavaBeans spec and not supported
> by the java.beans.* classes. It's therefore better, IMHO, to keep the code
> as it was before this patch. Adding support for this feature, with no proper
> support in the spec, in the Reference Implementation doesn't seem right.
>
> Hans
> PS.Sorry for not reporting back to the list about the discussion.
This is my interpretation on the subject:
If you have:
<foo:bar intVal="8" />
then you should invoke:
barHandler.setIntVal("8"); // Note the String arg.
and not invoke
barHandler.setIntVal(8);
And when you have:
<foo:bar intVal="<%= 8 %>" />
you should invoke:
barHandler.setIntVal(8);
Maybe there is a slight difference between my interpretation
and the correct one. Here it is:
1) When you specify the value as a String constant there is no
difference.
2) When you specify it as a runtime expression the correct
interpretation is that we should delegate the
responsibility of finding the setter method to the
PropertyDescriptor for the bean (which will select it at
random) and not call the method based on the TYPE of the
value.
If this is correct, I will change the code that handles the
runtime expression part.
Thanks,
Mandar.
Re: cvs commit: jakarta-tomcat/src/share/org/apache/jasper/compiler
TagBeginGenerator.java
Posted by Hans Bergsten <ha...@gefionsoftware.com>.
mandar@locus.apache.org wrote:
>
> mandar 00/04/06 11:29:00
>
> Modified: src/share/org/apache/jasper/compiler TagBeginGenerator.java
> Log:
> Fix for bug #109.
>
> JspRuntimeLibrary breaks taglibs with multiple property setters.
Actually, I had an offline discussion about this feature (multiple setters)
with Eduardo, Larry and Janet Koenig, and the conclusion was that support
for multiple setters is not mandated by the JavaBeans spec and not supported
by the java.beans.* classes. It's therefore better, IMHO, to keep the code
as it was before this patch. Adding support for this feature, with no proper
support in the spec, in the Reference Implementation doesn't seem right.
Hans
PS.Sorry for not reporting back to the list about the discussion.
> Revision Changes Path
> 1.11 +17 -14 jakarta-tomcat/src/share/org/apache/jasper/compiler/TagBeginGenerator.java
>
> Index: TagBeginGenerator.java
> ===================================================================
> RCS file: /home/cvs/jakarta-tomcat/src/share/org/apache/jasper/compiler/TagBeginGenerator.java,v
> retrieving revision 1.10
> retrieving revision 1.11
> diff -u -r1.10 -r1.11
> --- TagBeginGenerator.java 2000/03/28 04:29:48 1.10
> +++ TagBeginGenerator.java 2000/04/06 18:29:00 1.11
> @@ -179,21 +179,24 @@
> String attrValue = (String) attrs.get(attributes[i].getName());
> if (attrValue != null) {
>
> - String attrName = attributes[i].getName();
> + if (attributes[i].canBeRequestTime()) {
> + if (JspUtil.isExpression(attrValue))
> + attrValue = JspUtil.getExpr(attrValue);
> + else
> + attrValue = writer.quoteString(attrValue);
> + } else
> + attrValue = writer.quoteString(attrValue);
>
> - if (!JspUtil.isExpression (attrValue)) {
> - writer.println("JspRuntimeLibrary.introspecthelper(" +
> - thVarName + ",\"" + attrName +
> - "\",\"" + JspUtil.escapeQueryString(attrValue) +
> - "\",null,null,false);");
> - } else {
> -
> - // This requires some careful handling.
> - // int, boolean, ... are not Object(s).
> - writer.println("JspRuntimeLibrary.handleSetProperty(" +
> - thVarName + ",\"" + attrName +
> - "\"," + JspUtil.getExpr(attrValue) + ");");
> - }
> + String attrName = attributes[i].getName();
> + Method m = tc.getSetterMethod(attrName);
> +
> + if (m == null)
> + throw new JasperException
> + (Constants.getString
> + ("jsp.error.unable.to_find_method",
> + new Object[] { attrName }));
> +
> + writer.println(thVarName+"."+m.getName()+"("+attrValue+");");
> }
> }
> }
>
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: tomcat-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: tomcat-dev-help@jakarta.apache.org
--
Hans Bergsten hans@gefionsoftware.com
Gefion Software http://www.gefionsoftware.com