You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ofbiz.apache.org by Scott Gray <sc...@hotwaxmedia.com> on 2014/12/23 11:42:10 UTC
Re: svn commit: r1647522 - /ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/thirdparty/authorizedotnet/AIMPaymentServices.java
I know it looks like this makes sense at first glance but sometimes it's
useful to look at the history of changes. It should be clear that
something is amiss because of the useless try/catch block directly below
your change. It points out that perhaps this code previously did something
similar to what you're trying to do here.
So I have a look and we find this commit:
http://svn.apache.org/viewvc?view=revision&revision=1303329
Maybe these changes warrant further investigation instead of committers
switching it back and forth every few years.
Because neither you nor Hans have bothered to fully review the issue,
you've both missed that the problem is that ccRelease(...) is passing a
BigDecimal into the x_Amount map value while every other method is putting
a String on that value. The getXAmount method need to be restored properly
back to what it was before Hans' commit (i.e. actually use the try/catch
block when creating the BigDecimal) and ccRelease(...) needs to convert the
BigDecimal to a String before putting it in the map.
Regards
Scott
On Tue, Dec 23, 2014 at 10:51 PM, <jl...@apache.org> wrote:
> Author: jleroux
> Date: Tue Dec 23 09:51:56 2014
> New Revision: 1647522
>
> URL: http://svn.apache.org/r1647522
> Log:
> A patch from Prateek Ashtikar for "Issue reported while performing Refund
> & Void (java.lang.ClassCastException: java.lang.String cannot be cast to
> java.math.BigDecimal)" https://issues.apache.org/jira/browse/OFBIZ-5927
>
> Change committed in AIMPaymentServices.java.
>
> Modified:
>
> ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/thirdparty/authorizedotnet/AIMPaymentServices.java
>
> Modified:
> ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/thirdparty/authorizedotnet/AIMPaymentServices.java
> URL:
> http://svn.apache.org/viewvc/ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/thirdparty/authorizedotnet/AIMPaymentServices.java?rev=1647522&r1=1647521&r2=1647522&view=diff
>
> ==============================================================================
> ---
> ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/thirdparty/authorizedotnet/AIMPaymentServices.java
> (original)
> +++
> ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/thirdparty/authorizedotnet/AIMPaymentServices.java
> Tue Dec 23 09:51:56 2014
> @@ -824,7 +824,8 @@ public class AIMPaymentServices {
> private static BigDecimal getXAmount(Map<String, Object> request) {
> BigDecimal amt = BigDecimal.ZERO;
> if (request.get("x_Amount") != null) {
> - BigDecimal amount = (BigDecimal) request.get("x_Amount");
> + String newAmt = request.get("x_Amount").toString();
> + BigDecimal amount = new BigDecimal(newAmt);
> try {
> amt = amount;
> } catch (NumberFormatException e) {
>
>
>
Re: svn commit: r1647522 - /ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/thirdparty/au
thorizedotnet/AIMPaymentServices.java
Posted by Scott Gray <sc...@hotwaxmedia.com>.
I'm busy but I'm sure if you un-assign yourself someone else will fix it at
some point.
Regards
Scott
On Wed, Dec 24, 2014 at 12:03 AM, Jacques Le Roux <
jacques.le.roux@les7arts.com> wrote:
> OK no pb, I revert the commits and let yout take care of it
>
> Jacques
>
> Le 23/12/2014 11:44, Scott Gray a écrit :
>
> and as much as I want to have yet another rant about you not testing
>> changes before back porting them to the release branches.... I'm just
>> going to sigh and consider stable branches a failed experiment.
>>
>> On Tue, Dec 23, 2014 at 11:42 PM, Scott Gray <sc...@hotwaxmedia.com>
>> wrote:
>>
>> I know it looks like this makes sense at first glance but sometimes it's
>>> useful to look at the history of changes. It should be clear that
>>> something is amiss because of the useless try/catch block directly below
>>> your change. It points out that perhaps this code previously did
>>> something
>>> similar to what you're trying to do here.
>>>
>>> So I have a look and we find this commit:
>>> http://svn.apache.org/viewvc?view=revision&revision=1303329
>>>
>>> Maybe these changes warrant further investigation instead of committers
>>> switching it back and forth every few years.
>>>
>>> Because neither you nor Hans have bothered to fully review the issue,
>>> you've both missed that the problem is that ccRelease(...) is passing a
>>> BigDecimal into the x_Amount map value while every other method is
>>> putting
>>> a String on that value. The getXAmount method need to be restored
>>> properly
>>> back to what it was before Hans' commit (i.e. actually use the try/catch
>>> block when creating the BigDecimal) and ccRelease(...) needs to convert
>>> the
>>> BigDecimal to a String before putting it in the map.
>>>
>>> Regards
>>> Scott
>>>
>>> On Tue, Dec 23, 2014 at 10:51 PM, <jl...@apache.org> wrote:
>>>
>>> Author: jleroux
>>>> Date: Tue Dec 23 09:51:56 2014
>>>> New Revision: 1647522
>>>>
>>>> URL: http://svn.apache.org/r1647522
>>>> Log:
>>>> A patch from Prateek Ashtikar for "Issue reported while performing
>>>> Refund
>>>> & Void (java.lang.ClassCastException: java.lang.String cannot be cast to
>>>> java.math.BigDecimal)" https://issues.apache.org/jira/browse/OFBIZ-5927
>>>>
>>>> Change committed in AIMPaymentServices.java.
>>>>
>>>> Modified:
>>>>
>>>> ofbiz/trunk/applications/accounting/src/org/ofbiz/
>>>> accounting/thirdparty/authorizedotnet/AIMPaymentServices.java
>>>>
>>>> Modified:
>>>> ofbiz/trunk/applications/accounting/src/org/ofbiz/
>>>> accounting/thirdparty/authorizedotnet/AIMPaymentServices.java
>>>> URL:
>>>> http://svn.apache.org/viewvc/ofbiz/trunk/applications/
>>>> accounting/src/org/ofbiz/accounting/thirdparty/authorizedotnet/
>>>> AIMPaymentServices.java?rev=1647522&r1=1647521&r2=1647522&view=diff
>>>>
>>>> ============================================================
>>>> ==================
>>>> ---
>>>> ofbiz/trunk/applications/accounting/src/org/ofbiz/
>>>> accounting/thirdparty/authorizedotnet/AIMPaymentServices.java
>>>> (original)
>>>> +++
>>>> ofbiz/trunk/applications/accounting/src/org/ofbiz/
>>>> accounting/thirdparty/authorizedotnet/AIMPaymentServices.java
>>>> Tue Dec 23 09:51:56 2014
>>>> @@ -824,7 +824,8 @@ public class AIMPaymentServices {
>>>> private static BigDecimal getXAmount(Map<String, Object> request)
>>>> {
>>>> BigDecimal amt = BigDecimal.ZERO;
>>>> if (request.get("x_Amount") != null) {
>>>> - BigDecimal amount = (BigDecimal) request.get("x_Amount");
>>>> + String newAmt = request.get("x_Amount").toString();
>>>> + BigDecimal amount = new BigDecimal(newAmt);
>>>> try {
>>>> amt = amount;
>>>> } catch (NumberFormatException e) {
>>>>
>>>>
>>>>
>>>>
Re: svn commit: r1647522 -
/ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/thirdparty/au
thorizedotnet/AIMPaymentServices.java
Posted by Jacques Le Roux <ja...@les7arts.com>.
OK no pb, I revert the commits and let yout take care of it
Jacques
Le 23/12/2014 11:44, Scott Gray a écrit :
> and as much as I want to have yet another rant about you not testing
> changes before back porting them to the release branches.... I'm just
> going to sigh and consider stable branches a failed experiment.
>
> On Tue, Dec 23, 2014 at 11:42 PM, Scott Gray <sc...@hotwaxmedia.com>
> wrote:
>
>> I know it looks like this makes sense at first glance but sometimes it's
>> useful to look at the history of changes. It should be clear that
>> something is amiss because of the useless try/catch block directly below
>> your change. It points out that perhaps this code previously did something
>> similar to what you're trying to do here.
>>
>> So I have a look and we find this commit:
>> http://svn.apache.org/viewvc?view=revision&revision=1303329
>>
>> Maybe these changes warrant further investigation instead of committers
>> switching it back and forth every few years.
>>
>> Because neither you nor Hans have bothered to fully review the issue,
>> you've both missed that the problem is that ccRelease(...) is passing a
>> BigDecimal into the x_Amount map value while every other method is putting
>> a String on that value. The getXAmount method need to be restored properly
>> back to what it was before Hans' commit (i.e. actually use the try/catch
>> block when creating the BigDecimal) and ccRelease(...) needs to convert the
>> BigDecimal to a String before putting it in the map.
>>
>> Regards
>> Scott
>>
>> On Tue, Dec 23, 2014 at 10:51 PM, <jl...@apache.org> wrote:
>>
>>> Author: jleroux
>>> Date: Tue Dec 23 09:51:56 2014
>>> New Revision: 1647522
>>>
>>> URL: http://svn.apache.org/r1647522
>>> Log:
>>> A patch from Prateek Ashtikar for "Issue reported while performing Refund
>>> & Void (java.lang.ClassCastException: java.lang.String cannot be cast to
>>> java.math.BigDecimal)" https://issues.apache.org/jira/browse/OFBIZ-5927
>>>
>>> Change committed in AIMPaymentServices.java.
>>>
>>> Modified:
>>>
>>> ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/thirdparty/authorizedotnet/AIMPaymentServices.java
>>>
>>> Modified:
>>> ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/thirdparty/authorizedotnet/AIMPaymentServices.java
>>> URL:
>>> http://svn.apache.org/viewvc/ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/thirdparty/authorizedotnet/AIMPaymentServices.java?rev=1647522&r1=1647521&r2=1647522&view=diff
>>>
>>> ==============================================================================
>>> ---
>>> ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/thirdparty/authorizedotnet/AIMPaymentServices.java
>>> (original)
>>> +++
>>> ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/thirdparty/authorizedotnet/AIMPaymentServices.java
>>> Tue Dec 23 09:51:56 2014
>>> @@ -824,7 +824,8 @@ public class AIMPaymentServices {
>>> private static BigDecimal getXAmount(Map<String, Object> request) {
>>> BigDecimal amt = BigDecimal.ZERO;
>>> if (request.get("x_Amount") != null) {
>>> - BigDecimal amount = (BigDecimal) request.get("x_Amount");
>>> + String newAmt = request.get("x_Amount").toString();
>>> + BigDecimal amount = new BigDecimal(newAmt);
>>> try {
>>> amt = amount;
>>> } catch (NumberFormatException e) {
>>>
>>>
>>>
Re: svn commit: r1647522 - /ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/thirdparty/authorizedotnet/AIMPaymentServices.java
Posted by Scott Gray <sc...@hotwaxmedia.com>.
and as much as I want to have yet another rant about you not testing
changes before back porting them to the release branches.... I'm just
going to sigh and consider stable branches a failed experiment.
On Tue, Dec 23, 2014 at 11:42 PM, Scott Gray <sc...@hotwaxmedia.com>
wrote:
> I know it looks like this makes sense at first glance but sometimes it's
> useful to look at the history of changes. It should be clear that
> something is amiss because of the useless try/catch block directly below
> your change. It points out that perhaps this code previously did something
> similar to what you're trying to do here.
>
> So I have a look and we find this commit:
> http://svn.apache.org/viewvc?view=revision&revision=1303329
>
> Maybe these changes warrant further investigation instead of committers
> switching it back and forth every few years.
>
> Because neither you nor Hans have bothered to fully review the issue,
> you've both missed that the problem is that ccRelease(...) is passing a
> BigDecimal into the x_Amount map value while every other method is putting
> a String on that value. The getXAmount method need to be restored properly
> back to what it was before Hans' commit (i.e. actually use the try/catch
> block when creating the BigDecimal) and ccRelease(...) needs to convert the
> BigDecimal to a String before putting it in the map.
>
> Regards
> Scott
>
> On Tue, Dec 23, 2014 at 10:51 PM, <jl...@apache.org> wrote:
>
>> Author: jleroux
>> Date: Tue Dec 23 09:51:56 2014
>> New Revision: 1647522
>>
>> URL: http://svn.apache.org/r1647522
>> Log:
>> A patch from Prateek Ashtikar for "Issue reported while performing Refund
>> & Void (java.lang.ClassCastException: java.lang.String cannot be cast to
>> java.math.BigDecimal)" https://issues.apache.org/jira/browse/OFBIZ-5927
>>
>> Change committed in AIMPaymentServices.java.
>>
>> Modified:
>>
>> ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/thirdparty/authorizedotnet/AIMPaymentServices.java
>>
>> Modified:
>> ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/thirdparty/authorizedotnet/AIMPaymentServices.java
>> URL:
>> http://svn.apache.org/viewvc/ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/thirdparty/authorizedotnet/AIMPaymentServices.java?rev=1647522&r1=1647521&r2=1647522&view=diff
>>
>> ==============================================================================
>> ---
>> ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/thirdparty/authorizedotnet/AIMPaymentServices.java
>> (original)
>> +++
>> ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/thirdparty/authorizedotnet/AIMPaymentServices.java
>> Tue Dec 23 09:51:56 2014
>> @@ -824,7 +824,8 @@ public class AIMPaymentServices {
>> private static BigDecimal getXAmount(Map<String, Object> request) {
>> BigDecimal amt = BigDecimal.ZERO;
>> if (request.get("x_Amount") != null) {
>> - BigDecimal amount = (BigDecimal) request.get("x_Amount");
>> + String newAmt = request.get("x_Amount").toString();
>> + BigDecimal amount = new BigDecimal(newAmt);
>> try {
>> amt = amount;
>> } catch (NumberFormatException e) {
>>
>>
>>
>