You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ofbiz.apache.org by Bob Morley <rm...@emforium.com> on 2010/04/13 20:50:59 UTC
"Magically" converted types from simpleTypeConvert
We have just started to make use of ProductionRun in our project and we were
having a problem in the create because the ProductionRunServices (around
line 274) makes a call to "createWorkEffort" using a "quantityToProduce"
that is a BigDecimal. The field on the WorkEffort entity is actually a
Double, so in our solution it was causing a "bad type error" and failing.
I jumped to trunk to try to figure out why it was working in stock Ofbiz.
After some digging, it turns out that the conversion enhancements now
(appear to) silently do this conversion, resulting in a context that
contains that field as a Double.
This happens because the ServiceDispatcher's runSync method calls
"checkAuth" which (if the user can call the service) calls
ModelService.makeValid which ultimately calls ObjectType.simpleTypeConvert
that handles the BigDecimal -> Double conversion.
Is this the intended behavior? To be honest, it seems to me that the
complaint that the parameter was not the proper data type was more
desirable. I understand that when we are handling a postback, values need
to be converted from String to their desired types; but if I am making a
call in our back-end to another service I would think I should be forced to
adhere to that service definition's signature.
--
View this message in context: http://n4.nabble.com/Magically-converted-types-from-simpleTypeConvert-tp1838891p1838891.html
Sent from the OFBiz - Dev mailing list archive at Nabble.com.
Re: "Magically" converted types from simpleTypeConvert
Posted by Bob Morley <rm...@emforium.com>.
I have captures the pertinent bits from this thread and created two new JIRAs
...
OFBIZ-3699 - ServiceDispatcher.checkAuth modifies the context if the
invocation service has a permissionServiceName
OFBIZ-3700 - Convert WorkEffort (and related entities) quantities from
Double -> BigDecimal
Not sure when I will get a chance to work on these tickets, but we can move
the conversation to them if more discussion is warranted.
--
View this message in context: http://n4.nabble.com/Magically-converted-types-from-simpleTypeConvert-tp1838891p1839821.html
Sent from the OFBiz - Dev mailing list archive at Nabble.com.
Re: "Magically" converted types from simpleTypeConvert
Posted by Scott Gray <sc...@hotwaxmedia.com>.
On 14/04/2010, at 10:23 AM, Bob Morley wrote:
>
>
> Scott Gray-2 wrote:
>>
>> Firstly and without verifying, the automatic conversion sounds wrong and
>> yes I'm quite sure it intentional never used to be that way.
>>
>> Secondly, I'm not sure why quantityToProduce is a floating point instead
>> of a fixed point data type. I'm surprised and can't imagine why I skipped
>> these when I was doing the double -> BigDecimal conversion of everything.
>>
>
> Ahhhh HA! Ok I did more digging and it appears that the call to
> ModelService.makeValid would always do these type of conversions
> "magically". I suspect the main intent here was to take a context that may
> or may not exactly match a service definition (parms & parm types) and
> select the desirable parameters into a new context that can be used for
> invocation. There is no difference between the old and new in terms of this
> method being called.
>
> However, there appears to be some problem code, here it is from
> ServiceDispatcher ...
>
> if (UtilValidate.isNotEmpty(origService.permissionServiceName)) {
> ...
> if (hasPermission.booleanValue()) {
> context.putAll(permResp);
> context = origService.makeValid(context,
> ModelService.IN_PARAM);
>
> Effectively what happens is if a ModelService has a permissionServiceName,
> it will invoke that permission and if it is successful it adds the
> permission response to the context and converts it to match the service that
> will be invoked. I suspect this gives us the ability to have the service
> permission provide arguments to the underlying service.
>
> Someone on our team overrode the "createWorkEffort" and removed the
> permissionServiceName. So this code would not execute, we had the context
> with the Double in it, and service dispatcher produced the standard
> violation.
>
> My suggestion is we change this code to make use of
> ServiceUtil.setServiceFields, something like this ...
>
> Map<String, Object> permRespContext = ServiceUtil.setServiceFields(dctx,
> serviceName, permResp);
> context.putAll(permRespContext);
>
> Effectively, we would take the permission response and grab the things from
> it that match our service signature and then add those to the context (and
> ultimately return it). This would ensure that only replaces are added
> without impacting anything else.
That sounds fine, we definitely shouldn't be modifying context variables that have nothing to do with the permission call. My only concern would be that switching from makeValid to setServiceFields could alter some sort of behavior that a previous developer deemed necessary (although I can't imagine what the reason would be). But yes I think we should definitely change it from dealing with context to dealing with permResp instead and I think using setServiceFields should be fine.
> The second part to this would be either
> changing WorkEffort to use BigDecimal OR change the service implementation
> to pass in a Double when invoking those services.
I think we should switch it to BigDecimal.
Regards
Scott
Re: "Magically" converted types from simpleTypeConvert
Posted by Bob Morley <rm...@emforium.com>.
Scott Gray-2 wrote:
>
> Firstly and without verifying, the automatic conversion sounds wrong and
> yes I'm quite sure it intentional never used to be that way.
>
> Secondly, I'm not sure why quantityToProduce is a floating point instead
> of a fixed point data type. I'm surprised and can't imagine why I skipped
> these when I was doing the double -> BigDecimal conversion of everything.
>
Ahhhh HA! Ok I did more digging and it appears that the call to
ModelService.makeValid would always do these type of conversions
"magically". I suspect the main intent here was to take a context that may
or may not exactly match a service definition (parms & parm types) and
select the desirable parameters into a new context that can be used for
invocation. There is no difference between the old and new in terms of this
method being called.
However, there appears to be some problem code, here it is from
ServiceDispatcher ...
if (UtilValidate.isNotEmpty(origService.permissionServiceName)) {
...
if (hasPermission.booleanValue()) {
context.putAll(permResp);
context = origService.makeValid(context,
ModelService.IN_PARAM);
Effectively what happens is if a ModelService has a permissionServiceName,
it will invoke that permission and if it is successful it adds the
permission response to the context and converts it to match the service that
will be invoked. I suspect this gives us the ability to have the service
permission provide arguments to the underlying service.
Someone on our team overrode the "createWorkEffort" and removed the
permissionServiceName. So this code would not execute, we had the context
with the Double in it, and service dispatcher produced the standard
violation.
My suggestion is we change this code to make use of
ServiceUtil.setServiceFields, something like this ...
Map<String, Object> permRespContext = ServiceUtil.setServiceFields(dctx,
serviceName, permResp);
context.putAll(permRespContext);
Effectively, we would take the permission response and grab the things from
it that match our service signature and then add those to the context (and
ultimately return it). This would ensure that only replaces are added
without impacting anything else. The second part to this would be either
changing WorkEffort to use BigDecimal OR change the service implementation
to pass in a Double when invoking those services.
Make sense?
--
View this message in context: http://n4.nabble.com/Magically-converted-types-from-simpleTypeConvert-tp1838891p1839077.html
Sent from the OFBiz - Dev mailing list archive at Nabble.com.
Re: "Magically" converted types from simpleTypeConvert
Posted by Scott Gray <sc...@hotwaxmedia.com>.
Firstly and without verifying, the automatic conversion sounds wrong and yes I'm quite sure it intentional never used to be that way.
Secondly, I'm not sure why quantityToProduce is a floating point instead of a fixed point data type. I'm surprised and can't imagine why I skipped these when I was doing the double -> BigDecimal conversion of everything.
Regards
Scott
On 14/04/2010, at 6:50 AM, Bob Morley wrote:
> We have just started to make use of ProductionRun in our project and we were
> having a problem in the create because the ProductionRunServices (around
> line 274) makes a call to "createWorkEffort" using a "quantityToProduce"
> that is a BigDecimal. The field on the WorkEffort entity is actually a
> Double, so in our solution it was causing a "bad type error" and failing.
>
> I jumped to trunk to try to figure out why it was working in stock Ofbiz.
> After some digging, it turns out that the conversion enhancements now
> (appear to) silently do this conversion, resulting in a context that
> contains that field as a Double.
>
> This happens because the ServiceDispatcher's runSync method calls
> "checkAuth" which (if the user can call the service) calls
> ModelService.makeValid which ultimately calls ObjectType.simpleTypeConvert
> that handles the BigDecimal -> Double conversion.
>
> Is this the intended behavior? To be honest, it seems to me that the
> complaint that the parameter was not the proper data type was more
> desirable. I understand that when we are handling a postback, values need
> to be converted from String to their desired types; but if I am making a
> call in our back-end to another service I would think I should be forced to
> adhere to that service definition's signature.
> --
> View this message in context: http://n4.nabble.com/Magically-converted-types-from-simpleTypeConvert-tp1838891p1838891.html
> Sent from the OFBiz - Dev mailing list archive at Nabble.com.