try to unify POST processing in servlets#2402
Conversation
merks
left a comment
merks
left a comment
There was a problem hiding this comment.
Mostly just a few small changes to suggest, but the exception handling looks like it will be easily to miss problem unless one watches the console.
| Iterator it = request.getParameterMap().keySet().iterator(); | ||
| while (it.hasNext()) { |
There was a problem hiding this comment.
Why not use an for-each loop directly on the keySet?
There was a problem hiding this comment.
It’s maybe my iPhone, but I don’t see the change. It’s been force pushed? I’ll check on my computer, but that will be much later or tomorrow.
There was a problem hiding this comment.
My fault Ed, now it is force pushed.
| * @param response | ||
| */ | ||
| public void soapService(HttpServletRequest request, HttpServletResponse response) { | ||
|
|
There was a problem hiding this comment.
Kind of pointless whites pace.
| GetUpdatedObjectsResponse responseObj = binding.getUpdatedObjects(requestObj); | ||
| BirtSoapMarshaller.marshalResponse(responseObj, response); | ||
| } catch (BirtSoapException e) { | ||
| // TODO Auto-generated catch block |
There was a problem hiding this comment.
There are quite a few printStackTraces that should be logged in some more visible way.
| * @param context | ||
| */ | ||
| public void __doPost(IContext context) throws BirtException { | ||
| // TODO Auto-generated method stub |
There was a problem hiding this comment.
Yes its complete and added comment here
|
|
||
| } | ||
|
|
||
| private static String escape(String s) { |
There was a problem hiding this comment.
Surely a utility must exist that does this...
There was a problem hiding this comment.
Exists but not between BIRT dependencies - one function here is simpler than add next dependency
| StringWriter sw = new StringWriter(); | ||
| PrintWriter pw = new PrintWriter(sw); | ||
| current.printStackTrace(pw); | ||
| pw.flush(); |
There was a problem hiding this comment.
I know it's not necessary to close these, but one might use a try-with anyway.
There was a problem hiding this comment.
Added close to PrintWriter
ischindl
force-pushed
the
fix_post_in_engineservlet
branch
from
32a9bc9 to
86a0fd9
Compare
ischindl
force-pushed
the
fix_post_in_engineservlet
branch
from
86a0fd9 to
d720f08
Compare
2 participants
#2393 fixing missing POST processing in EngineServlet by moving common parts into BaseReportEngineServlet