Double significant digits

Issue #67 closed
Former user created an issue

First, it took me some time to understand what was the cause of my problem because the error message is not clear: it talks about doubleFormat whereas the error is about significant digits. Hopefully, you have a UT about it.

So I would suggest to modify the error message.

Then about the error itself: why do you limit to 15 digits? The default json serializer in .NET uses 17 and I couldn't see any reason about your choice.

Thank you

Comments (8)

  1. Ben Sayers

    Thanks for the bug report. To help us understand the problem you are seeing can you please share an example spec and mock that demonstrates the issue, what the actual output is and what you would expect the output to be.

  2. xavier le galles

    You have this UT about it:

    'should return the error when the pact response body contains a too large double value'
    

    And you can see that the error message returned is

    'Response body is incompatible with the response body schema in the swagger file: should pass "formatDouble" keyword validation'
    

    This is not the cause of the error as mentioned in the test case description. You should say that the double is too large.

    Also don't forget to explain why you limit double to 15 digits?

  3. Ben Sayers

    So from what I can gather there are two queries here:

    Can we improve the error messages to be more specific when format validation fails?

    The team will investigate what options there are to address this next week when we get back from the Easter break and get back to you.

    What is the thinking behind the double validation using a precision of 15?

    An IEEE 754 double has a precision of 15 to 17 significant decimal digits. Source: https://en.m.wikipedia.org/wiki/Double-precision_floating-point_format

    The thinking is that if the precision of a number is 15 or less it is safe to say this number is within the range that doubles can represent. I accept that there are valid double values with precision’s of 16 or 17 that will be incorrectly flagged as incompatible. We chose to be conservative in this case and only allow things that are certain to be doubles.

    I’m happy to accept your input on a more robust way for us to test if a number is a double that does not contain false positives or false negatives. I’ll brainstorm with my team on this topic as well. Alternatively if you feel that our decision to be conservative at the expensive of false positives is a mistake please share your perspective.

  4. xavier le galles

    So, to be precise, we use the default .NET Core Json serializer which is Json.Net: it uses exactly 16 significant digits which is still compliant with the IEEE 754 format. It means that you could maybe relax a bit your validation and support 16 instead of 15 digits and still be compliant. It would avoid us (and .NET developers) some customization. By the way, this tool is really really interesting and you did a great job.

  5. Ben Sayers

    @xlegalles we've looked into this and here is how we'd like to address the two issues you have raised:

    Can we improve the error messages to be more specific when format validation fails?

    We agree the error message returned here is too cryptic. The way we have implemented the format checking is via custom keywords in a json schema library called ajv and it seems ajv does not expose a way for us to change the output of this error message. So it is not trivial for us to improve these messages as we would have to submit a pull request to ajv or change how we have implemented the validation completely. We consider this request an enhancement rather then a bug so we have created a separate issue for this request:

    https://bitbucket.org/atlassian/swagger-mock-validator/issues/68/improve-error-messages-when-issues-are

    We will address this when we can, however just to set expectations we are in the middle of a big project right now and don't expect to get to this any time soon. Of course if you are interested in submitting a pull request we are happy to collaborate with you on this, just be sure to run any ideas past us first before you get too committed to an approach.

    Why is my valid double value being rejected by the validator?

    We have had another look at the way our double validation works (as well as int32, int64 and float) and we think we can implement all of them so there are no false positives at all. However this will have a caveat.

    We are going to assume that any json files we are given are valid json files. This means we are going to assume that any numeric values inside json files are doubles as per the json specification. If a json file contains a number that is too large to fit in a double the tool will automatically adjust the number so it fits in a double as this is the behavior of the json parser we are using. Because of this the double format check will never fail for values inside request and response bodies. We consider it the job of a json serializer or parser to detect when json files are not valid, not this tool.

    Path parameters can also have formats such as double and these will be handled differently by this tool because they are encoded as strings. For path parameters we will parse the string value as a numeric value then decide if that numeric value fits into the declared format.

    We are working on these changes now and should have a fix released in the next week or so.

    Please let me know if you have any concerns with the approach described above.

  6. Ben Sayers

    fix: format validators for int32, int64, float and double no longer accept spaces and now test for the full range of accepted values

    Close #67

    BREAKING CHANGE:

    Prior to this change the tool used to accept blank spaces in path parameters for int32, int64, float and double formatted numbers. A bug has been fixed to disallow blank spaces. This means some mock responses that were previously considered valid will now be considered invalid. This change has also updated the int64 and double format validators to now test for the full range of values these types support. This means some mock responses that were previously considered invalid will now be considered valid.

    It is recommended that consumers and providers coordinate upgrading to this release so that both sides agree on what is considered valid vs invalid.

    → <<cset a38b08534534>>

  7. Ben Sayers

    fix: format validators for int32, int64, float and double no longer accept spaces and now test for the full range values

    Close #67

    BREAKING CHANGE:

    Prior to this change the tool used to accept blank spaces in path parameters for int32, int64, float and double formatted numbers. A bug has been fixed to disallow blank spaces. This means some mock responses that were previously considered valid will now be considered invalid. This change has also updated the int64 and double format validators to now test for the full range of values these types support. This means some mock responses that were previously considered invalid will now be considered valid.

    It is recommended that consumers and providers coordinate upgrading to this release so that both sides agree on what is considered valid vs invalid.

    → <<cset 4197cb66c746>>

  8. Log in to comment