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
smime.p7s
Description: S/MIME cryptographic signature
