You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ofbiz.apache.org by le...@apache.org on 2009/10/18 02:34:30 UTC
svn commit: r826322 -
/ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/entityops/StoreValue.java
Author: lektran
Date: Sun Oct 18 00:34:29 2009
New Revision: 826322
URL: http://svn.apache.org/viewvc?rev=826322&view=rev
Log:
Catch ClassCastExceptions in <store-value/> operations, these will occur if you have a null GenericValue (perhaps from a lookup that returned no results), set some fields on it and then try to store it. What you actually have at that point is a FastMap created when you set the fields.
Also added use of the utility MethodContext.setErrorReturn(String, SimpleMethod) rather than manually adding errors to the environment context.
Modified:
ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/entityops/StoreValue.java
Modified: ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/entityops/StoreValue.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/entityops/StoreValue.java?rev=826322&r1=826321&r2=826322&view=diff
==============================================================================
--- ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/entityops/StoreValue.java (original)
+++ ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/entityops/StoreValue.java Sun Oct 18 00:34:29 2009
@@ -56,18 +56,19 @@
public boolean exec(MethodContext methodContext) {
boolean doCacheClear = !"false".equals(methodContext.expandString(doCacheClearStr));
- GenericValue value = valueAcsr.get(methodContext);
+ GenericValue value = null;
+ try {
+ value = valueAcsr.get(methodContext);
+ } catch (ClassCastException e) {
+ String errMsg = "In store-value the value specified by valueAcsr [" + valueAcsr + "] was not an instance of GenericValue, not storing";
+ Debug.logError(errMsg, module);
+ methodContext.setErrorReturn(errMsg, simpleMethod);
+ return false;
+ }
if (value == null) {
String errMsg = "In store-value a value was not found with the specified valueAcsr: " + valueAcsr + ", not storing";
-
Debug.logWarning(errMsg, module);
- if (methodContext.getMethodType() == MethodContext.EVENT) {
- methodContext.putEnv(simpleMethod.getEventErrorMessageName(), errMsg);
- methodContext.putEnv(simpleMethod.getEventResponseCodeName(), simpleMethod.getDefaultErrorCode());
- } else if (methodContext.getMethodType() == MethodContext.SERVICE) {
- methodContext.putEnv(simpleMethod.getServiceErrorMessageName(), errMsg);
- methodContext.putEnv(simpleMethod.getServiceResponseMessageName(), simpleMethod.getDefaultErrorCode());
- }
+ methodContext.setErrorReturn(errMsg, simpleMethod);
return false;
}
@@ -76,14 +77,7 @@
} catch (GenericEntityException e) {
Debug.logError(e, module);
String errMsg = "ERROR: Could not complete the " + simpleMethod.getShortDescription() + " process [problem storing the " + valueAcsr + " value: " + e.getMessage() + "]";
-
- if (methodContext.getMethodType() == MethodContext.EVENT) {
- methodContext.putEnv(simpleMethod.getEventErrorMessageName(), errMsg);
- methodContext.putEnv(simpleMethod.getEventResponseCodeName(), simpleMethod.getDefaultErrorCode());
- } else if (methodContext.getMethodType() == MethodContext.SERVICE) {
- methodContext.putEnv(simpleMethod.getServiceErrorMessageName(), errMsg);
- methodContext.putEnv(simpleMethod.getServiceResponseMessageName(), simpleMethod.getDefaultErrorCode());
- }
+ methodContext.setErrorReturn(errMsg, simpleMethod);
return false;
}
return true;
Re: svn commit: r826322 - /ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/entityops/StoreValue.java
Posted by Adam Heath <do...@brainfood.com>.
Tim Ruppert wrote:
> Maybe you could leave out the "bad commit" part and just recommend the
> two separate commits :) - I like to see both features go in and bugs get
> fixed, so not so much bad as maybe another, even better way of handling
> it Scott!
Yeah, should have said bad commit message.
Re: svn commit: r826322 - /ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/entityops/StoreValue.java
Posted by Tim Ruppert <ti...@hotwaxmedia.com>.
Maybe you could leave out the "bad commit" part and just recommend the
two separate commits :) - I like to see both features go in and bugs
get fixed, so not so much bad as maybe another, even better way of
handling it Scott!
Cheers,
Ruppert
--
Tim Ruppert
HotWax Media
http://www.hotwaxmedia.com
o:801.649.6594
f:801.649.6595
On Oct 20, 2009, at 11:29 AM, Adam Heath wrote:
> lektran@apache.org wrote:
>> Author: lektran
>> Date: Sun Oct 18 00:34:29 2009
>> New Revision: 826322
>>
>> URL: http://svn.apache.org/viewvc?rev=826322&view=rev
>> Log:
>> Catch ClassCastExceptions in <store-value/> operations, these will
>> occur if you have a null GenericValue (perhaps from a lookup that
>> returned no results), set some fields on it and then try to store
>> it. What you actually have at that point is a FastMap created when
>> you set the fields.
>> Also added use of the utility MethodContext.setErrorReturn(String,
>> SimpleMethod) rather than manually adding errors to the environment
>> context.
>
> Bad commit, this is 2 separate changes, should have been done as 2
> separate commits. One is a bug fix, the other is just a feature.
>
Re: svn commit: r826322 - /ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/entityops/StoreValue.java
Posted by Scott Gray <sc...@hotwaxmedia.com>.
Aren't we getting a bit carried away with this?
The first was not a bug fix, catching the exception still results in
an error, it's just that now you can actually understand what went
wrong by looking at the logs.
As part of that improvement I wanted to return an error message and
noticed that to do so required another 7 lines of code. Must be an
easier way I thought, that's when I came across the utility method in
MethodContext which did exactly what I needed. I chose to use that
method for my change and also clean up the class by using it in the
other two places that errors were being returned.
I've understood your reasoning in the past but if you expect me to
split up super minor commits into tiny little digestible micro pieces,
then no I won't, I'll just leave the code cleanups for someone who has
the time to build, test and commit every 30 seconds.
Regards
Scott
HotWax Media
http://www.hotwaxmedia.com
On 21/10/2009, at 6:29 AM, Adam Heath wrote:
> lektran@apache.org wrote:
>> Author: lektran
>> Date: Sun Oct 18 00:34:29 2009
>> New Revision: 826322
>>
>> URL: http://svn.apache.org/viewvc?rev=826322&view=rev
>> Log:
>> Catch ClassCastExceptions in <store-value/> operations, these will
>> occur if you have a null GenericValue (perhaps from a lookup that
>> returned no results), set some fields on it and then try to store
>> it. What you actually have at that point is a FastMap created when
>> you set the fields.
>> Also added use of the utility MethodContext.setErrorReturn(String,
>> SimpleMethod) rather than manually adding errors to the environment
>> context.
>
> Bad commit, this is 2 separate changes, should have been done as 2
> separate commits. One is a bug fix, the other is just a feature.
>
Re: svn commit: r826322 - /ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/entityops/StoreValue.java
Posted by Adam Heath <do...@brainfood.com>.
lektran@apache.org wrote:
> Author: lektran
> Date: Sun Oct 18 00:34:29 2009
> New Revision: 826322
>
> URL: http://svn.apache.org/viewvc?rev=826322&view=rev
> Log:
> Catch ClassCastExceptions in <store-value/> operations, these will occur if you have a null GenericValue (perhaps from a lookup that returned no results), set some fields on it and then try to store it. What you actually have at that point is a FastMap created when you set the fields.
> Also added use of the utility MethodContext.setErrorReturn(String, SimpleMethod) rather than manually adding errors to the environment context.
Bad commit, this is 2 separate changes, should have been done as 2
separate commits. One is a bug fix, the other is just a feature.