multipleOf validation fails when the value is fractional

[craignicol]

Describe the bug

Given an openAPI spec that contains validation of multipleOf: 0.1 for a double type, there are several valid values for which the validation fails, and therefor the API request is not sent.

To Reproduce

Steps to reproduce the behavior:

1. Use autorest to generate a typescript client with the following type contained
2. Attempt to call the API with a value of 2.0
3. Observe that the validation fails and the command is not sent

       fractionalValue:
          type: number
          format: double
          minimum: 0
          maximum: 100
          multipleOf: 0.1
          example: 0.1

Expected behavior

For a multipleOf value of 0.1, all of the following values (and others) should be valid:

• 0.1
• 0.2
• 1
• 2

Additional context

I believe this may be resolved by changing line 57 of https://github.com/Azure/ms-rest-js/blob/45f89c90414db812e4950185ffe7f9f6becb5664/lib/serializer.ts to use an epsilon value of MultipleOf when the type is double.

This may include false positives that I have not considered. However, for client-side validation false positives are expected, since the client does not have full context, and will rely on server validation in other scenarios.

[jeremymeng]

@craignicol what version of autorest are you using? We've made many improvements in recent versions, and now the generated code are using @azure/core-client and @azure/core-rest-pipeline instead of @azure/ms-rest-js. Would it be possible for you to try the latest version and open issue in the https://github.com/Azure/azure-sdk-for-js repository?

[craignicol]

craignicol@SM-LAPTOP-0244:~$ autorest --version
AutoRest code generation utility [cli version: 3.6.1; node: v14.16.1]
(C) 2018 Microsoft Corporation.
https://aka.ms/autorest

Showing All Installed Extensions

 Type       Extension Name                           Version      Location
 core       @autorest/core                           3.8.3        /home/craignicol/.autorest/@autorest_core@3.8.3
 extension  @autorest/modelerfour                    4.21.4       /home/craignicol/.autorest/@autorest_modelerfour@4.21.4
 extension  @autorest/typescript                     6.0.0-beta.19 /home/craignicol/.autorest/@autorest_typescript@6.0.0-beta.19
 core       @microsoft.azure/autorest-core           2.0.4421     /home/craignicol/.autorest/@microsoft.azure_autorest-core@2.0.4421
 extension  @microsoft.azure/autorest.modeler        2.3.51       /home/craignicol/.autorest/@microsoft.azure_autorest.modeler@2.3.51
 extension  @microsoft.azure/autorest.typescript     4.2.4        /home/craignicol/.autorest/@microsoft.azure_autorest.typescript@4.2.4

I can see the same logic on line 83 of https://github.com/Azure/azure-sdk-for-js/blob/5762893591c904be4880a47a56041c7298dc959a/sdk/core/core-http/src/serializer.ts which would fail under the same circumstances.

[azure-sdk]

Label prediction was below confidence level 0.6 for Model:ServiceLabels: 'Cosmos:0.086542636,Service Bus:0.07491286,Azure.Core:0.05635434'

[jeremymeng]

@joheredi @sarangan12 could you please have a look?

[sarangan12]

Taking a look at it. Will update soon with my findings

[sarangan12]

@xirzec @jeremymeng Per our SDK Guidelines, we should not be doing any validations. Any reasons why we should still keep these validations? Why can't we remove them altogether?

[sarangan12]

I have sent a note to the Autorest crew abount these validations and based on their input, I will decide on removing the constraints.

[xirzec]

@xirzec @jeremymeng Per our SDK Guidelines, we should not be doing any validations. Any reasons why we should still keep these validations? Why can't we remove them altogether?

Per our offline discussion, I agree we can remove this check on the client side

[sarangan12]

Removed the constraints check. Closing this issue