Closed GoogleCodeExporter closed 9 years ago
Fixed in r450
Original comment by chkalt
on 18 Oct 2010 at 3:49
hey Christian,
I'm not sure we can assume that folks will be operating in an environment where
FacesContext is available, and I see that you've taken that into account. I'm
going to add another method that requires an HttpServletResponse so that
non-faces users can also benefit from this method.
Lincoln
Original comment by lincolnb...@gmail.com
on 18 Oct 2010 at 4:06
Hey Lincoln,
yes, another method with an HttpServletResponse as an argument would be great!
Or could we store HttpServletRequest and HttpServletResponse in the
PrettyContext during its creation? The PrettyContext lifetime is restricted to
a single request, isn't it?
But perhaps that would be dirty and lead to problems if users don't use the API
correctly and store the PrettyContext somewhere...
Original comment by chkalt
on 18 Oct 2010 at 4:53
This causes big problems if people hold on to references of PrettyContext
across servlet forwards (threadlocal boom). I think we should just require that
they pass in the request objects :)
I think what we have is good, but good thought nonetheless.
Original comment by lincolnb...@gmail.com
on 18 Oct 2010 at 4:59
Ok, good argument! Holding a reference to the request/response would be bad.
But I'm note sure whether it makes sense to let the users pass the response
object. If the user has a reference to the response outside of the JSF
lifecycle, using the Servlet API would be much easier for him to use.
The user would have to write something like:
PrettyContext.getCurrentInstance().sendError(404, "Not found", response);
But using the Servlet API directly it would be even shorter:
response.sendError(404, "Not found");
I intended this method mainly for page actions that want to report an error
easily, because sending status codes manually from JSF is horrible:
1) Obtain FacesContext
2) Get response object and cast it to HttpServletResonse
3) Send status code + message via sendError()
4) Catch IOExpcetions
But I totally agree with you that the Javadocs of PrettyContext.sendError()
should mention that the current implementation can only be called from inside
the JSF lifecycle.
What do you think?
Original comment by chkalt
on 19 Oct 2010 at 6:06
Just saw that you have already added the new method. So lets keep it and ignore
my comment above. Perhaps the method will be useful for people as it doesn't
throw IOExceptions like the native Servlet API. :-)
I've added a note on the FacesContext requirement to the other two methods in
r457.
Original comment by chkalt
on 19 Oct 2010 at 6:18
Also good points. I don't think it hurts to have it :)
Original comment by lincolnb...@gmail.com
on 19 Oct 2010 at 1:43
Original issue reported on code.google.com by
chkalt
on 13 Oct 2010 at 6:19