Re: svn commit: r1789710 - /ofbiz/ofbiz-framework/trunk/applications/content/groovyScripts/content/GetContentLookupList.groovy

[Michael Brohl]

Hi Jacques,










This whole file must be refactored.

Please revert or change your changes at least for [1] or [2].

Thanks,

Michael


Am 31.03.17 um 18:56 schrieb jler...@apache.org:

I think this is a functional change because you not only print the exception to the error log but you also put it in the errorMsgList. In this way, the error is printed to the user interface. This might not be intended because the VIEW_SIZE controls the number of entries in the before it is paginated and is normally set to a default if missing or wrong. No need to bring this to the UI. [1] Additionally, it is not translated because the error message is hard coded which is not acceptable IMO. [2] I would change the implementation according to the viewIndex block at the beginning (catching the exception and setting a default value, which should be read from the configuration file as it is done in other places). Looking further, there are other weeknesses, e.g. the errorMsgList is filled after it is already put in the request attribute _ERROR_MESSAGE_LIST_.

Author: jleroux
Date: Fri Mar 31 16:56:04 2017
New Revision: 1789710

URL: http://svn.apache.org/viewvc?rev=1789710&view=rev
Log:
Fixed: Fix Default or Empty Catch block in Java and Groovy files
(OFBIZ-8341)

While working on OFBIZ-7759, I stumbled upon this by chance.
It's unlikely that this NumberFormatException is ever caught, but not a reason
to swallow it.

I checked there are no other swallowed exceptions in Groovy files.

Modified:
     
ofbiz/ofbiz-framework/trunk/applications/content/groovyScripts/content/GetContentLookupList.groovy

Modified: 
ofbiz/ofbiz-framework/trunk/applications/content/groovyScripts/content/GetContentLookupList.groovy
URL: 
http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/applications/content/groovyScripts/content/GetContentLookupList.groovy?rev=1789710&r1=1789709&r2=1789710&view=diff
==============================================================================
--- 
ofbiz/ofbiz-framework/trunk/applications/content/groovyScripts/content/GetContentLookupList.groovy
 (original)
+++ 
ofbiz/ofbiz-framework/trunk/applications/content/groovyScripts/content/GetContentLookupList.groovy
 Fri Mar 31 16:56:04 2017
@@ -69,7 +69,8 @@ context.curFindString = curFindString
  try {
      viewSize = Integer.valueOf((String)parameters.get("VIEW_SIZE")).intValue()
  } catch (NumberFormatException nfe) {
-
+    Debug.logError(nfe, "Caught an exception : " + nfe.toString(), 
"GetContentLookupList.groovy")
+    errMsgList.add("Entered value is non-numeric for numeric field: " + 
field.getName())
  }

context.viewSize = viewSize

smime.p7s
Description: S/MIME Cryptographic Signature