Anonymous | Login
Project:
RSS
  
News | My View | View Issues | Roadmap | Summary

View Issue DetailsJump to Notes ] Issue History ] Print ]
ID
0036430
TypeCategorySeverityReproducibilityDate SubmittedLast Update
defect[Openbravo ERP] 09. Financial managementminorhave not tried2017-07-06 12:472017-09-21 16:49
ReportershuehnerView Statuspublic 
Assigned ToTriage Omni OMS 
PrioritynormalResolutionfixedFixed in Version3.0PR17Q4
StatusclosedFix in branchFixed in SCM revision47aab9353bb6
ProjectionnoneETAnoneTarget Version
OSAnyDatabaseAnyJava version
OS VersionDatabase versionAnt version
Product VersionSCM revision 
Review Assigned Tovmromanos
Web browser
ModulesCore
Regression level
Regression date
Regression introduced in release
Regression introduced by commit
Triggers an Emergency PackNo
Summary

0036430: 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
TagsNo tags attached.
Attached Filesdiff file icon bad-logging.diff [^] (10,572 bytes) 2017-07-06 12:49 [Show Content]

- Relationships Relation Graph ] Dependency Graph ]
related to design defect 0036162 acknowledgedTriage Platform Base clean up openbravo.log 

-  Notes
(0097906)
shuehner (administrator)
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 (administrator)
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 (administrator)
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 (developer)
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 (developer)
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 (developer)
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 (manager)
2017-07-07 14:03

Code review OK
(0099247)
hudsonbot (developer)
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 (developer)
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 (developer)
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

- Issue History
Date Modified Username Field Change
2017-07-06 12:47 shuehner New Issue
2017-07-06 12:47 shuehner Assigned To => Triage Finance
2017-07-06 12:47 shuehner Modules => Core
2017-07-06 12:47 shuehner Triggers an Emergency Pack => No
2017-07-06 12:49 shuehner File Added: bad-logging.diff
2017-07-06 12:50 shuehner Note Added: 0097906
2017-07-06 12:51 shuehner Note Added: 0097907
2017-07-06 12:52 shuehner Note Added: 0097908
2017-07-06 12:53 shuehner Relationship added related to 0036162
2017-07-07 08:44 vmromanos Status new => scheduled
2017-07-07 14:03 hgbot Checkin
2017-07-07 14:03 hgbot Note Added: 0097950
2017-07-07 14:03 hgbot Status scheduled => resolved
2017-07-07 14:03 hgbot Resolution open => fixed
2017-07-07 14:03 hgbot Fixed in SCM revision => http://code.openbravo.com/erp/devel/pi/rev/b8a0e333c8f24865ff1360fc6488bc4773ee649e [^]
2017-07-07 14:03 hgbot Checkin
2017-07-07 14:03 hgbot Note Added: 0097952
2017-07-07 14:03 hgbot Fixed in SCM revision http://code.openbravo.com/erp/devel/pi/rev/b8a0e333c8f24865ff1360fc6488bc4773ee649e [^] => http://code.openbravo.com/erp/devel/pi/rev/918cd718f0f314445614dd83207faa272d58ef94 [^]
2017-07-07 14:03 hgbot Checkin
2017-07-07 14:03 hgbot Note Added: 0097953
2017-07-07 14:03 hgbot Fixed in SCM revision http://code.openbravo.com/erp/devel/pi/rev/918cd718f0f314445614dd83207faa272d58ef94 [^] => http://code.openbravo.com/erp/devel/pi/rev/47aab9353bb688761b0ee35a27bc8e31a0e10d4b [^]
2017-07-07 14:03 vmromanos Review Assigned To => vmromanos
2017-07-07 14:03 vmromanos Note Added: 0097955
2017-07-07 14:03 vmromanos Status resolved => closed
2017-07-07 14:03 vmromanos Fixed in Version => 3.0PR17Q4
2017-09-21 16:49 hudsonbot Checkin
2017-09-21 16:49 hudsonbot Note Added: 0099247
2017-09-21 16:49 hudsonbot Checkin
2017-09-21 16:49 hudsonbot Note Added: 0099248
2017-09-21 16:49 hudsonbot Checkin
2017-09-21 16:49 hudsonbot Note Added: 0099249


Copyright © 2000 - 2009 MantisBT Group
Powered by Mantis Bugtracker