You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ofbiz.apache.org by Adrian Crum <ad...@sandglass-software.com> on 2014/08/12 16:52:57 UTC
Re: svn commit: r1617473 - /ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/event/ServiceEventHandler.java
Inline...
Adrian Crum
Sandglass Software
www.sandglass-software.com
On 8/12/2014 2:38 PM, jacopoc@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.
Re: svn commit: r1617473 - /ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/event/ServiceEventHandler.java
Posted by Jacopo Cappellato <ja...@gmail.com>.
+1
I completely agree
Jacopo
On Aug 12, 2014, at 4:52 PM, Adrian Crum <ad...@sandglass-software.com> wrote:
> Inline...
>
> Adrian Crum
> Sandglass Software
> www.sandglass-software.com
>
> On 8/12/2014 2:38 PM, jacopoc@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.