geronimo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Bill Dudney (JIRA)" <...@geronimo.apache.org>
Subject [jira] Updated: (GERONIMO-1686) Servlet 2.5 and JSP 2.1 api jars for JavaEE 5 work
Date Thu, 16 Mar 2006 13:06:08 GMT
     [ http://issues.apache.org/jira/browse/GERONIMO-1686?page=all ]

Bill Dudney updated GERONIMO-1686:
----------------------------------

    Attachment: servlet_fixes.patch

Hi Greg,

Thanks for testing the stuff out so quickly! I've put some comments into the issues below;

To apply this patch you have to use the -E option to patch i.e.

jee5_exp $ patch -E -p0 ./servlet_fixes.patch

Once the patch is fixed the renaming of SingleThreadedModel.java to SingleThreadModel.java
is done but the patch does not do the svn del and svn add so that will have to be done by
hand.

Thanks again Greg. My comments are inlined here...

* Geronimo classes do not use resource bundles for messages.

The G classes do use resource bundles but not in javax.servlet only in javax.servlet.http.
The <= 2.4 code used resource bundles to internationalize 'true' and 'false' and a ISO
character problem. I did not think we needed resource bundles to manage 'true' and 'false'
and the ISO character problem was related to the 'old' way to do conversion. I updated to
use JDK 1.4 classes and code so I'd expect that the internationalization is managed by JDK
classes so we don't need that either.

In javax.servlet.http I missed Cookie.java and fixed it with this patch.

* GenericServlet calls super() when it does not need to.

More specifics would be good here. I've changed the code though to look more like the <=
2.4 code which could clear things up.

* In GenericServlet, the initParameter, getServletContext and getServletName
methods are now all protected with an IllegalStateException if the ServletConfig is null (ie
if init(ServletConfig) has been overriden and does not call super).
This is mandated by the spec now.

I could not find anywhere in the 2.5 spec that spelled this out. However I totally agree that
IllegalStateException is much better than NullPointerException so I updated the code to throw
ISE instead of NPE. If you could point me to the spec bits that specify this I'd be glad to
make sure the spec is being followed.

* Cookie in geronimo does not default maxAge to -1

Fixed in this patch.

* Cookie.clone() in geronimo does not call super.clone()... which I think is bad???

Changed it anyway to call super.clone();

* geronimo is using @override and @deprecated  which I think is needless java5 featurism (I
know java5 is required for compliance.... but there is nothing in the rest of the javax classes
that needs it).

Does it matter one way or the other? I'm fine either way. Unchanged in this patch.

* HttpServletRequestWrapper getRequest()  returns HttpServletRequest.  I think this should
be ServletRequest.

Good point - fixed 

* ServletResponse.getWriter   should return PrintWriter not Writer.

Another fix

* ServletRequestListener does not implements EventListener

Another fix

* ServletContext.getServlet(String) does not throw  ServletException

Another fix

javax/servlet/SingleThreadedModel.java   should be javax/servlet/SingleThreadModel.java

Good point, fixed

> Servlet 2.5 and JSP 2.1 api jars for JavaEE 5 work
> --------------------------------------------------
>
>          Key: GERONIMO-1686
>          URL: http://issues.apache.org/jira/browse/GERONIMO-1686
>      Project: Geronimo
>         Type: New Feature
>     Reporter: Bill Dudney
>     Assignee: Jeff Genender
>     Priority: Minor
>  Attachments: jee5_exp.patch, jee5_exp_servlet.patch, servlet_fixes.patch
>
> I'm typing in the Servlet 2.5 and JSP 2.1 api's and will post a patch the week of 3/6/06.

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
   http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
   http://www.atlassian.com/software/jira


Mime
View raw message