+1 I completely agree
Jacopo On Aug 12, 2014, at 4:52 PM, Adrian Crum <adrian.c...@sandglass-software.com> wrote: > Inline... > > Adrian Crum > Sandglass Software > www.sandglass-software.com > > On 8/12/2014 2:38 PM, jaco...@apache.org wrote: >> Author: jacopoc >> Date: Tue Aug 12 13:38:55 2014 >> New Revision: 1617473 >> >> URL: http://svn.apache.org/r1617473 >> Log: >> Some optimization for service event handler to prevent unnecessary >> operations at every service invocation. >> >> Modified: >> >> ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/event/ServiceEventHandler.java >> >> Modified: >> ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/event/ServiceEventHandler.java >> URL: >> http://svn.apache.org/viewvc/ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/event/ServiceEventHandler.java?rev=1617473&r1=1617472&r2=1617473&view=diff >> ============================================================================== >> --- >> ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/event/ServiceEventHandler.java >> (original) >> +++ >> ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/event/ServiceEventHandler.java >> Tue Aug 12 13:38:55 2014 >> @@ -124,35 +124,38 @@ public class ServiceEventHandler impleme >> throw new EventHandlerException("Problems getting the service >> model"); >> } >> >> - if (Debug.verboseOn()) Debug.logVerbose("[Processing]: SERVICE >> Event", module); >> - if (Debug.verboseOn()) Debug.logVerbose("[Using delegator]: " + >> dispatcher.getDelegator().getDelegatorName(), module); >> + if (Debug.verboseOn()) { >> + Debug.logVerbose("[Processing]: SERVICE Event", module); >> + Debug.logVerbose("[Using delegator]: " + >> dispatcher.getDelegator().getDelegatorName(), module); >> + } >> >> - // get the http upload configuration >> - String maxSizeStr = >> EntityUtilProperties.getPropertyValue("general.properties", >> "http.upload.max.size", "-1", dctx.getDelegator()); >> - long maxUploadSize = -1; >> - try { >> - maxUploadSize = Long.parseLong(maxSizeStr); >> - } catch (NumberFormatException e) { >> - Debug.logError(e, "Unable to obtain the max upload size from >> general.properties; using default -1", module); >> - maxUploadSize = -1; >> - } >> - // get the http size threshold configuration - files bigger than >> this will be >> - // temporarly stored on disk during upload >> - String sizeThresholdStr = >> EntityUtilProperties.getPropertyValue("general.properties", >> "http.upload.max.sizethreshold", "10240", dctx.getDelegator()); >> - int sizeThreshold = 10240; // 10K >> - try { >> - sizeThreshold = Integer.parseInt(sizeThresholdStr); >> - } catch (NumberFormatException e) { >> - Debug.logError(e, "Unable to obtain the threshold size from >> general.properties; using default 10K", module); >> - sizeThreshold = -1; >> - } >> - // directory used to temporarily store files that are larger than >> the configured size threshold >> - String tmpUploadRepository = >> EntityUtilProperties.getPropertyValue("general.properties", >> "http.upload.tmprepository", "runtime/tmp", dctx.getDelegator()); >> - String encoding = request.getCharacterEncoding(); >> - // check for multipart content types which may have uploaded items >> boolean isMultiPart = ServletFileUpload.isMultipartContent(request); >> Map<String, Object> multiPartMap = FastMap.newInstance(); >> if (isMultiPart) { >> + // get the http upload configuration >> + String maxSizeStr = >> EntityUtilProperties.getPropertyValue("general.properties", >> "http.upload.max.size", "-1", dctx.getDelegator()); >> + long maxUploadSize = -1; >> + try { >> + maxUploadSize = Long.parseLong(maxSizeStr); >> + } catch (NumberFormatException e) { >> + Debug.logError(e, "Unable to obtain the max upload size >> from general.properties; using default -1", module); >> + maxUploadSize = -1; >> + } >> + // get the http size threshold configuration - files bigger >> than this will be >> + // temporarly stored on disk during upload >> + String sizeThresholdStr = >> EntityUtilProperties.getPropertyValue("general.properties", >> "http.upload.max.sizethreshold", "10240", dctx.getDelegator()); >> + int sizeThreshold = 10240; // 10K >> + try { >> + sizeThreshold = Integer.parseInt(sizeThresholdStr); >> + } catch (NumberFormatException e) { >> + Debug.logError(e, "Unable to obtain the threshold size from >> general.properties; using default 10K", module); >> + sizeThreshold = -1; >> + } > > > I understand this was a simple change to help optimization, but this code > points out something I really dislike: Hiding configuration errors. From my > perspective, if a value in a properties file is invalid, the code should blow > up. It should throw an exception and refuse to operate until someone fixes > the configuration. > > Hiding configuration errors gives you a system that appears to be running > normally - until an end user reports a problem. Then you are left with > parsing gigabytes of log files trying to figure out what's wrong.
