Openbravo Issue Tracking System - Openbravo ERP
View Issue Details
IDProjectCategoryView StatusDate SubmittedLast Update
0036430Openbravo ERP09. Financial managementpublic2017-07-06 12:472017-09-21 16:49
Reportershuehner 
Assigned ToTriage Omni OMS 
PrioritynormalSeverityminorReproducibilityhave not tried
StatusclosedResolutionfixed 
PlatformOS5OS Version
Product Version 
Target VersionFixed in Version3.0PR17Q4 
Merge Request Status
Review Assigned Tovmromanos
OBNetwork customer
Web browser
ModulesCore
Support ticket
Regression level
Regression date
Regression introduced in release
Regression introduced by commit
Triggers an Emergency PackNo
Summary0036430: Cleanup bad logging (modules/* functional code): don't use printStackTrace or System.out.print* or System.err.print*
DescriptionAs part of getting better logging all uses of i.e. printStackTrace or System.*.print* should be avoided whenever possible.

This issue tracks such bad usage in ERP functional code.
Steps To Reproducecd modules
grep -IrE '(System.(err|out)|printStackTrace)' . | grep -v src-test | less
Proposed SolutionConvert them to use proper log4j
Additional Information
TagsNo tags attached.
Relationships
related to design defect 0036162 acknowledged Triage Platform Base clean up openbravo.log 
Attached Filesdiff bad-logging.diff (10,572) 2017-07-06 12:49
https://issues.openbravo.com/file_download.php?file_id=10894&type=bug
Issue History
Date ModifiedUsernameFieldChange
2017-07-06 12:47shuehnerNew Issue
2017-07-06 12:47shuehnerAssigned To => Triage Finance
2017-07-06 12:47shuehnerModules => Core
2017-07-06 12:47shuehnerTriggers an Emergency Pack => No
2017-07-06 12:49shuehnerFile Added: bad-logging.diff
2017-07-06 12:50shuehnerNote Added: 0097906
2017-07-06 12:51shuehnerNote Added: 0097907
2017-07-06 12:52shuehnerNote Added: 0097908
2017-07-06 12:53shuehnerRelationship addedrelated to 0036162
2017-07-07 08:44vmromanosStatusnew => scheduled
2017-07-07 14:03hgbotCheckin
2017-07-07 14:03hgbotNote Added: 0097950
2017-07-07 14:03hgbotStatusscheduled => resolved
2017-07-07 14:03hgbotResolutionopen => fixed
2017-07-07 14:03hgbotFixed in SCM revision => http://code.openbravo.com/erp/devel/pi/rev/b8a0e333c8f24865ff1360fc6488bc4773ee649e [^]
2017-07-07 14:03hgbotCheckin
2017-07-07 14:03hgbotNote Added: 0097952
2017-07-07 14:03hgbotFixed in SCM revisionhttp://code.openbravo.com/erp/devel/pi/rev/b8a0e333c8f24865ff1360fc6488bc4773ee649e [^] => http://code.openbravo.com/erp/devel/pi/rev/918cd718f0f314445614dd83207faa272d58ef94 [^]
2017-07-07 14:03hgbotCheckin
2017-07-07 14:03hgbotNote Added: 0097953
2017-07-07 14:03hgbotFixed in SCM revisionhttp://code.openbravo.com/erp/devel/pi/rev/918cd718f0f314445614dd83207faa272d58ef94 [^] => http://code.openbravo.com/erp/devel/pi/rev/47aab9353bb688761b0ee35a27bc8e31a0e10d4b [^]
2017-07-07 14:03vmromanosReview Assigned To => vmromanos
2017-07-07 14:03vmromanosNote Added: 0097955
2017-07-07 14:03vmromanosStatusresolved => closed
2017-07-07 14:03vmromanosFixed in Version => 3.0PR17Q4
2017-09-21 16:49hudsonbotCheckin
2017-09-21 16:49hudsonbotNote Added: 0099247
2017-09-21 16:49hudsonbotCheckin
2017-09-21 16:49hudsonbotNote Added: 0099248
2017-09-21 16:49hudsonbotCheckin
2017-09-21 16:49hudsonbotNote Added: 0099249

Notes
(0097906)
shuehner   
2017-07-06 12:50   
Attached patch fixes up nearly add functional cases in modules/ folder + 1 in core (by just deleting dead code).
Note: In some cases also log4j.debug to log exception was changed to log4j.error also as that seems more correct.
(0097907)
shuehner   
2017-07-06 12:51   
The following files also use printStackTrace:
./org.openbravo.advpaymentmngt/src/org/openbravo/advpaymentmngt/process/FIN_AddPayment.java: e.printStackTrace(System.err);
./org.openbravo.advpaymentmngt/src/org/openbravo/advpaymentmngt/process/FIN_ExecutePayment.java: e.printStackTrace(System.err);
./org.openbravo.advpaymentmngt/src/org/openbravo/advpaymentmngt/process/FIN_TransactionModify.java: e.printStackTrace(System.err);
./org.openbravo.advpaymentmngt/src/org/openbravo/advpaymentmngt/process/FIN_BankStatementProcess.java: e.printStackTrace(System.err);
./org.openbravo.advpaymentmngt/src/org/openbravo/advpaymentmngt/process/FIN_ReconciliationProcess.java: e.printStackTrace(System.err);
./org.openbravo.advpaymentmngt/src/org/openbravo/advpaymentmngt/process/FIN_PaymentProposalProcess.java: e.printStackTrace(System.err);

When fixing please check if that printStacktrace is needed at all as OBException tyically are logged already or log themselves. So maybe the call is duplicate info anyway to be removed instead of just fixed.
(0097908)
shuehner   
2017-07-06 12:52   
This issue does not fix up all code in pi/src/ folder but 2 cases if possible should be also fixed.
./src/org/openbravo/common/hooks/InventoryStatusValidationHook.java:// System.out.println("somebody is calling me");
./src/org/openbravo/common/hooks/OrderLineQtyChangedHook.java:// System.out.println("somebody is calling me");

Those are especially bad examples as they are code-examples for implementers of the hook. And we should not show bad code we don't in examples at least.
(0097950)
hgbot   
2017-07-07 14:03   
Repository: erp/devel/pi
Changeset: b8a0e333c8f24865ff1360fc6488bc4773ee649e
Author: Víctor Martínez Romanos <victor.martinez <at> openbravo.com>
Date: Fri Jul 07 09:11:46 2017 +0200
URL: http://code.openbravo.com/erp/devel/pi/rev/b8a0e333c8f24865ff1360fc6488bc4773ee649e [^]

Fixed issue 36430: Cleanup bad logging (printStackTrace, System.*.print.*)

Pushed on Stefan's behalf

Removed usage of printStackTrace and System.*.print.* and tranform them to log4j.error

---
M modules/org.openbravo.advpaymentmngt/src/org/openbravo/advpaymentmngt/ad_actionbutton/AddTransaction.java
M modules/org.openbravo.advpaymentmngt/src/org/openbravo/advpaymentmngt/ad_actionbutton/MatchTransaction.java
M modules/org.openbravo.advpaymentmngt/src/org/openbravo/advpaymentmngt/ad_actionbutton/ProcessInvoice.java
M modules/org.openbravo.advpaymentmngt/src/org/openbravo/advpaymentmngt/ad_forms/BatchPaymentExecution.java
M modules/org.openbravo.advpaymentmngt/src/org/openbravo/advpaymentmngt/ad_forms/Transactions.java
M modules/org.openbravo.advpaymentmngt/src/org/openbravo/advpaymentmngt/ad_reports/ReportReconciliation.java
M src/org/openbravo/erpCommon/businessUtility/Tax.java
---
(0097952)
hgbot   
2017-07-07 14:03   
Repository: erp/devel/pi
Changeset: 918cd718f0f314445614dd83207faa272d58ef94
Author: Víctor Martínez Romanos <victor.martinez <at> openbravo.com>
Date: Fri Jul 07 09:36:56 2017 +0200
URL: http://code.openbravo.com/erp/devel/pi/rev/918cd718f0f314445614dd83207faa272d58ef94 [^]

Fixed issue 36430: Cleanup bad logging (printStackTrace, System.*.print.*)

Replaced to log.error() calls where necessary

---
M modules/org.openbravo.advpaymentmngt/src/org/openbravo/advpaymentmngt/process/FIN_AddPayment.java
M modules/org.openbravo.advpaymentmngt/src/org/openbravo/advpaymentmngt/process/FIN_BankStatementProcess.java
M modules/org.openbravo.advpaymentmngt/src/org/openbravo/advpaymentmngt/process/FIN_ExecutePayment.java
M modules/org.openbravo.advpaymentmngt/src/org/openbravo/advpaymentmngt/process/FIN_PaymentProposalProcess.java
M modules/org.openbravo.advpaymentmngt/src/org/openbravo/advpaymentmngt/process/FIN_ReconciliationProcess.java
M modules/org.openbravo.advpaymentmngt/src/org/openbravo/advpaymentmngt/process/FIN_TransactionModify.java
---
(0097953)
hgbot   
2017-07-07 14:03   
Repository: erp/devel/pi
Changeset: 47aab9353bb688761b0ee35a27bc8e31a0e10d4b
Author: Víctor Martínez Romanos <victor.martinez <at> openbravo.com>
Date: Fri Jul 07 09:39:49 2017 +0200
URL: http://code.openbravo.com/erp/devel/pi/rev/47aab9353bb688761b0ee35a27bc8e31a0e10d4b [^]

Fixed issue 36430: removed bad usage even in examples

---
M src/org/openbravo/common/hooks/InventoryStatusValidationHook.java
M src/org/openbravo/common/hooks/OrderLineQtyChangedHook.java
---
(0097955)
vmromanos   
2017-07-07 14:03   
Code review OK
(0099247)
hudsonbot   
2017-09-21 16:49   
A changeset related to this issue has been promoted main and to the
Central Repository, after passing a series of tests.

Promotion changeset: https://code.openbravo.com/erp/devel/main/rev/9750b78d3e5c [^]
Maturity status: Test
(0099248)
hudsonbot   
2017-09-21 16:49   
A changeset related to this issue has been promoted main and to the
Central Repository, after passing a series of tests.

Promotion changeset: https://code.openbravo.com/erp/devel/main/rev/9750b78d3e5c [^]
Maturity status: Test
(0099249)
hudsonbot   
2017-09-21 16:49   
A changeset related to this issue has been promoted main and to the
Central Repository, after passing a series of tests.

Promotion changeset: https://code.openbravo.com/erp/devel/main/rev/9750b78d3e5c [^]
Maturity status: Test