details: https://code.openbravo.com/erp/devel/pi/rev/0a461eef07c3 changeset: 32423:0a461eef07c3 user: Stefan Hühner <stefan.huehner <at> openbravo.com> date: Thu Jul 06 13:18:39 2017 +0200 summary: Fixed 36431. Cleanup logging: Removes uses of printStackTrace + System.*.print*
Cleanup some uses of bad logging + mention of same in comments. In case of CryptoUtiliy, delete main method having bad usages (api-change). details: https://code.openbravo.com/erp/devel/pi/rev/36b24dd5e2ff changeset: 32424:36b24dd5e2ff user: Asier Lostalé <asier.lostale <at> openbravo.com> date: Fri Jul 07 12:28:04 2017 +0200 summary: fixed bug 36444: SqlC generates code hides stack traces in case of error Show stack traces when error occurs in sqlc methods. By default it is still not shown, but it can be enabled by setting DEBUG log lovel. diffstat: modules/org.openbravo.client.application/src/org/openbravo/client/application/window/StandardWindowComponent.java | 1 - modules/org.openbravo.service.datasource/src/org/openbravo/service/datasource/DataSourceServlet.java | 1 - modules/org.openbravo.service.datasource/src/org/openbravo/service/datasource/ReadOnlyDataSourceService.java | 3 - src-core/src/org/openbravo/data/Sqlc.java | 48 ++++++++- src-core/src/org/openbravo/utils/CryptoUtility.java | 10 -- src-test/src/org/openbravo/test/base/TestLogAppender.java | 6 +- src-test/src/org/openbravo/test/system/ErrorTextParserTest.java | 40 ++++++++- src/org/openbravo/dal/security/EntityAccessChecker.java | 3 +- 8 files changed, 85 insertions(+), 27 deletions(-) diffs (262 lines): diff -r aa2d71ec3678 -r 36b24dd5e2ff modules/org.openbravo.client.application/src/org/openbravo/client/application/window/StandardWindowComponent.java --- a/modules/org.openbravo.client.application/src/org/openbravo/client/application/window/StandardWindowComponent.java Fri Jul 07 10:34:39 2017 +0200 +++ b/modules/org.openbravo.client.application/src/org/openbravo/client/application/window/StandardWindowComponent.java Fri Jul 07 12:28:04 2017 +0200 @@ -97,7 +97,6 @@ public String generate() { final String jsCode = super.generate(); - // System.err.println(jsCode); return jsCode; } diff -r aa2d71ec3678 -r 36b24dd5e2ff modules/org.openbravo.service.datasource/src/org/openbravo/service/datasource/DataSourceServlet.java --- a/modules/org.openbravo.service.datasource/src/org/openbravo/service/datasource/DataSourceServlet.java Fri Jul 07 10:34:39 2017 +0200 +++ b/modules/org.openbravo.service.datasource/src/org/openbravo/service/datasource/DataSourceServlet.java Fri Jul 07 12:28:04 2017 +0200 @@ -180,7 +180,6 @@ log.error(e.getMessage(), e); writeResult(response, JsonUtils.convertExceptionToJson(e)); } catch (final Throwable t) { - t.printStackTrace(System.err); if (SessionHandler.isSessionHandlerPresent()) { SessionHandler.getInstance().setDoRollback(true); } diff -r aa2d71ec3678 -r 36b24dd5e2ff modules/org.openbravo.service.datasource/src/org/openbravo/service/datasource/ReadOnlyDataSourceService.java --- a/modules/org.openbravo.service.datasource/src/org/openbravo/service/datasource/ReadOnlyDataSourceService.java Fri Jul 07 10:34:39 2017 +0200 +++ b/modules/org.openbravo.service.datasource/src/org/openbravo/service/datasource/ReadOnlyDataSourceService.java Fri Jul 07 12:28:04 2017 +0200 @@ -108,9 +108,6 @@ jsonResponse.put(JsonConstants.RESPONSE_DATA, new JSONArray(jsonObjects)); jsonResult.put(JsonConstants.RESPONSE_RESPONSE, jsonResponse); - // if (jsonObjects.size() > 0) { - // System.err.println(jsonObjects.get(0)); - // } return jsonResult.toString(); } catch (JSONException e) { throw new OBException(e); diff -r aa2d71ec3678 -r 36b24dd5e2ff src-core/src/org/openbravo/data/Sqlc.java --- a/src-core/src/org/openbravo/data/Sqlc.java Fri Jul 07 10:34:39 2017 +0200 +++ b/src-core/src/org/openbravo/data/Sqlc.java Fri Jul 07 12:28:04 2017 +0200 @@ -1285,12 +1285,20 @@ out2.append(" instance.hasData = instance.result.isBeforeFirst();\n"); out2.append(" instance.countRecord = 0;\n"); out2.append(" } catch (SQLException e) {\n"); - out2.append(" log4j.error(\"SQL error in query: \" + strSql + \"Exception:\" + e);\n"); + out2.append(" if (log4j.isDebugEnabled()) {\n"); + out2.append(" log4j.error(\"SQL error in query: \" + strSql, e);\n"); + out2.append(" } else {\n"); + out2.append(" log4j.error(\"SQL error in query: \" + strSql + \" :\" + e);\n"); + out2.append(" }\n"); out2.append(" instance.errorOcurred = true;\n"); out2.append(" throw new ServletException(\"@CODE=\" + Integer.toString(e.getErrorCode()) + \"@\"\n"); out2.append(" + e.getMessage());\n"); out2.append(" } catch (Exception ex) {\n"); - out2.append(" log4j.error(\"Exception in query: \" + strSql + \"Exception:\", ex);\n"); + out2.append(" if (log4j.isDebugEnabled()) {\n"); + out2.append(" log4j.error(\"Exception in query: \" + strSql, ex);\n"); + out2.append(" } else {\n"); + out2.append(" log4j.error(\"Exception in query: \" + strSql + \" :\" + ex);\n"); + out2.append(" }\n"); out2.append(" instance.errorOcurred = true;\n"); out2.append(" throw new ServletException(\"@CODE=@\" + ex.getMessage());\n"); out2.append(" }\n"); @@ -1374,10 +1382,18 @@ out2.append(" }\n"); out2.append(" resultKey.close();\n"); out2.append(" } catch(SQLException e){\n"); - out2.append(" log4j.error(\"SQL error in query: \" + strSql1 + \"Exception:\"+ e);\n"); + out2.append(" if (log4j.isDebugEnabled()) {\n"); + out2.append(" log4j.error(\"SQL error in query: \" + strSql, e);\n"); + out2.append(" } else {\n"); + out2.append(" log4j.error(\"SQL error in query: \" + strSql + \" :\" + e);\n"); + out2.append(" }\n"); out2.append(" throw new ServletException(\"@CODE=\" + Integer.toString(e.getErrorCode()) + \"@\" + e.getMessage());\n"); out2.append(" } catch(Exception ex){\n"); - out2.append(" log4j.error(\"Exception in query: \" + strSql1 + \"Exception:\"+ ex);\n"); + out2.append(" if (log4j.isDebugEnabled()) {\n"); + out2.append(" log4j.error(\"Exception in query: \" + strSql, ex);\n"); + out2.append(" } else {\n"); + out2.append(" log4j.error(\"Exception in query: \" + strSql + \" :\" + ex);\n"); + out2.append(" }\n"); out2.append(" throw new ServletException(\"@CODE=@\" + ex.getMessage());\n"); out2.append(" } finally {\n"); out2.append(" try {\n"); @@ -1494,10 +1510,18 @@ } } out2.append(" } catch(SQLException e){\n"); - out2.append(" log4j.error(\"SQL error in query: \" + strSql + \"Exception:\"+ e);\n"); + out2.append(" if (log4j.isDebugEnabled()) {\n"); + out2.append(" log4j.error(\"SQL error in query: \" + strSql, e);\n"); + out2.append(" } else {\n"); + out2.append(" log4j.error(\"SQL error in query: \" + strSql + \" :\" + e);\n"); + out2.append(" }\n"); out2.append(" throw new ServletException(\"@CODE=\" + Integer.toString(e.getErrorCode()) + \"@\" + e.getMessage());\n"); out2.append(" } catch(Exception ex){\n"); - out2.append(" log4j.error(\"Exception in query: \" + strSql + \"Exception:\"+ ex);\n"); + out2.append(" if (log4j.isDebugEnabled()) {\n"); + out2.append(" log4j.error(\"Exception in query: \" + strSql, ex);\n"); + out2.append(" } else {\n"); + out2.append(" log4j.error(\"Exception in query: \" + strSql + \" :\" + ex);\n"); + out2.append(" }\n"); out2.append(" throw new ServletException(\"@CODE=@\" + ex.getMessage());\n"); out2.append(" } finally {\n"); out2.append(" try {\n"); @@ -1553,7 +1577,11 @@ if (outParams > 0) out2.append(paramsReceipt.toString()); out2.append(" } catch(SQLException e){\n"); - out2.append(" log4j.error(\"SQL error in query: \" + strSql + \"Exception:\"+ e);\n"); + out2.append(" if (log4j.isDebugEnabled()) {\n"); + out2.append(" log4j.error(\"SQL error in query: \" + strSql, e);\n"); + out2.append(" } else {\n"); + out2.append(" log4j.error(\"SQL error in query: \" + strSql + \" :\" + e);\n"); + out2.append(" }\n"); out2.append(" throw new ServletException(\"@CODE=\" + Integer.toString(e.getErrorCode()) + \"@\" + e.getMessage());\n"); out2.append(" } catch(NoConnectionAvailableException ec){\n"); out2.append(" log4j.error(\"Connection error in query: \" + strSql + \"Exception:\"+ ec);\n"); @@ -1562,7 +1590,11 @@ out2.append(" log4j.error(\"Pool error in query: \" + strSql + \"Exception:\"+ ep);\n"); out2.append(" throw new ServletException(\"@CODE=NoConnectionAvailable\");\n"); out2.append(" } catch(Exception ex){\n"); - out2.append(" log4j.error(\"Exception in query: \" + strSql + \"Exception:\"+ ex);\n"); + out2.append(" if (log4j.isDebugEnabled()) {\n"); + out2.append(" log4j.error(\"Exception in query: \" + strSql, ex);\n"); + out2.append(" } else {\n"); + out2.append(" log4j.error(\"Exception in query: \" + strSql + \" :\" + ex);\n"); + out2.append(" }\n"); out2.append(" throw new ServletException(\"@CODE=@\" + ex.getMessage());\n"); out2.append(" }\n"); out2.append(" }\n"); diff -r aa2d71ec3678 -r 36b24dd5e2ff src-core/src/org/openbravo/utils/CryptoUtility.java --- a/src-core/src/org/openbravo/utils/CryptoUtility.java Fri Jul 07 10:34:39 2017 +0200 +++ b/src-core/src/org/openbravo/utils/CryptoUtility.java Fri Jul 07 12:28:04 2017 +0200 @@ -24,16 +24,6 @@ public CryptoUtility() { } - public static void main(String argv[]) throws Exception { - System.out.println("Enter encryption password: "); - System.out.flush(); - String clave = argv[0]; - System.out.println("************* " + clave); - String strEnc = CryptoUtility.encrypt(clave); - System.out.println("ENCRYPTED TEXT: " + strEnc); - System.out.println("DECRYPTED TEXT: " + CryptoUtility.decrypt(strEnc)); - } - private static void initCipher() { try { s_key = new SecretKeySpec(new byte[] { 100, 25, 28, -122, -26, 94, -3, -72 }, "DES"); diff -r aa2d71ec3678 -r 36b24dd5e2ff src-test/src/org/openbravo/test/base/TestLogAppender.java --- a/src-test/src/org/openbravo/test/base/TestLogAppender.java Fri Jul 07 10:34:39 2017 +0200 +++ b/src-test/src/org/openbravo/test/base/TestLogAppender.java Fri Jul 07 12:28:04 2017 +0200 @@ -11,7 +11,7 @@ * under the License. * The Original Code is Openbravo ERP. * The Initial Developer of the Original Code is Openbravo SLU - * All portions are Copyright (C) 2016 Openbravo SLU + * All portions are Copyright (C) 2016-2017 Openbravo SLU * All Rights Reserved. * Contributor(s): ______________________________________. ************************************************************************ @@ -19,6 +19,7 @@ package org.openbravo.test.base; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.HashMap; import java.util.List; @@ -47,6 +48,9 @@ messages.put(event.getLevel(), levelMsgs); } levelMsgs.add(event.getMessage().toString()); + if (event.getThrowableStrRep() != null) { + levelMsgs.addAll(Arrays.asList(event.getThrowableStrRep())); + } } @Override diff -r aa2d71ec3678 -r 36b24dd5e2ff src-test/src/org/openbravo/test/system/ErrorTextParserTest.java --- a/src-test/src/org/openbravo/test/system/ErrorTextParserTest.java Fri Jul 07 10:34:39 2017 +0200 +++ b/src-test/src/org/openbravo/test/system/ErrorTextParserTest.java Fri Jul 07 12:28:04 2017 +0200 @@ -11,7 +11,7 @@ * under the License. * The Original Code is Openbravo ERP. * The Initial Developer of the Original Code is Openbravo SLU - * All portions are Copyright (C) 2010-2014 Openbravo SLU + * All portions are Copyright (C) 2010-2017 Openbravo SLU * All Rights Reserved. * Contributor(s): ______________________________________. ************************************************************************ @@ -19,12 +19,18 @@ package org.openbravo.test.system; +import static org.hamcrest.CoreMatchers.containsString; +import static org.hamcrest.CoreMatchers.not; +import static org.hamcrest.Matchers.hasItem; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertThat; import java.sql.Connection; import javax.servlet.ServletException; +import org.apache.log4j.Level; +import org.apache.log4j.Logger; import org.junit.Test; import org.openbravo.base.secureApp.VariablesSecureApp; import org.openbravo.database.ConnectionProvider; @@ -91,6 +97,38 @@ } } + @Test + public void whenStatementFailsNoStackTraceIsLoggedByDefault() throws Exception { + setTestLogAppenderLevel(Level.INFO); + + doErrorTextParserTest(1); + + assertThat( + "Stack trace shouldn't be included in log", + getTestLogAppender().getMessages(Level.ERROR), + not(hasItem(containsString("at org.openbravo.test.system.ErrorTextParserTestData.insertUserPK")))); + } + + @Test + public void whenStatementFailsStackTraceIsLoggedHavingDebugLogLevel() throws Exception { + Logger sqlcLogger = Logger.getLogger(ErrorTextParserTestData.class); + Level originalLogLevel = sqlcLogger.getLevel(); + + try { + setTestLogAppenderLevel(Level.DEBUG); + sqlcLogger.setLevel(Level.DEBUG); + + doErrorTextParserTest(1); + + assertThat( + "Stack trace should be included in log", + getTestLogAppender().getMessages(Level.ERROR), + hasItem(containsString("at org.openbravo.test.system.ErrorTextParserTestData.insertUserPK"))); + } finally { + sqlcLogger.setLevel(originalLogLevel); + } + } + private void doErrorTextParserTest(int testCase) throws Exception { ConnectionProvider conn = getConnectionProvider(); VariablesSecureApp vars = new VariablesSecureApp("", "", ""); diff -r aa2d71ec3678 -r 36b24dd5e2ff src/org/openbravo/dal/security/EntityAccessChecker.java --- a/src/org/openbravo/dal/security/EntityAccessChecker.java Fri Jul 07 10:34:39 2017 +0200 +++ b/src/org/openbravo/dal/security/EntityAccessChecker.java Fri Jul 07 12:28:04 2017 +0200 @@ -372,8 +372,7 @@ } /** - * Dumps the readable, writable, derived readable entities to the System.err outputstream. For - * debugging purposes. + * Dumps the readable, writable, derived readable entities. For debugging purposes. */ public void dump() { log.info(""); ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot _______________________________________________ Openbravo-commits mailing list Openbravo-commits@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openbravo-commits
