sagold / json-schema-library

Customizable and hackable json-validator and json-schema utilities for traversal, data generation and validation
MIT License
164 stars 19 forks source link

multipleOf running into floating point issue #43

Closed xjamundx closed 10 months ago

xjamundx commented 10 months ago

We have the use case of wanting to enforce numbers having no more than 2 decimal places so we're using the multipleOf keyword with .01 to try to achieve this. Unfortunately it fails in unexpected ways.

There is a well known issue with multipleOf keyword when used with floating point numbers:

Do you think this is something that this parser would like to fix or should we just use strings with regexes to achieve a similar result?

sagold commented 10 months ago

Can you describe the error you get? Basically multipleOf with floats does work as expected. For example the following tests are successful: https://github.com/sagold/json-schema-library/blob/main/test/unit/issues/issue43.multipleOf.float.test.ts.

In general, the number type of javascript does have issues for small decimals and division on decimals. So if you need a more robust solution to restrict decimal length, while still maintaining the number type, I recommend a custom keyword, for example maxDigits: 2.

import { Draft07, JsonValidator } from "json-schema-library";

const jsonSchema = new Draft07(mySchema, {
    validateKeyword: {
        // register validator for maxDigits
        maxDigits: maxDigitsValidator as JsonValidator
    },
    // register keyword to numbers (adding maxDigits to the list)
    typeKeywords: {
        number: draft07Config.typeKeywords.number.concat("maxDigits")
    }
});

with the validator something like

const maxDigitsValidator: JsonValidator =   (draft, schema, value: number, pointer) => {
   if (isNaN(value) || isNaN(schema.maxDigits)) {
     return undefined;
   }

   const [_, digits] = `${value}`.split('.');
   if (digits && digits.length > schema.maxDigits) {
     return {
       type: 'error',
       code: 'max-digits-errors',
       name: 'MaxDigitsError',
       message: `number should not have mor than ${schema.maxDigits} digits`,
       data: { pointer, value, schema }
     }
   }
}

As for the error generatiion it is recommended to follow the custom error introduction.

xjamundx commented 10 months ago

That's great. Thanks for the detailed list of options. I'll consider them!

In our case it's tricky as we're using a non-JS backend and JS front-end and finding JSON Schema has a lot of peculiarities that make it tough to keep them working exactly the same.So custom stuff we're trying to avoid, because the implementation will likely be difficult in Kotlin.

An example where the lib doesn't work as expected is 1.36:

> var draft =  new Draft07({ type: "number", multipleOf: 0.01 })
undefined
> draft.validate(1.36)
[
  {
    type: 'error',
    name: 'MultipleOfError',
    code: 'multiple-of-error',
    message: 'Expected `1.36` in `#` to be multiple of `0.01`',
    data: { multipleOf: 0.01, value: 1.36, pointer: '#' }
  }
]
>
xjamundx commented 10 months ago

You can also run something like this and get so many random ones:

for (let i = 1; i < 100; i++) { let num = `2.${i}`; console.log(num, draft.validate(parseFloat(num))); }

2.73 []
2.74 [
  {
    type: 'error',
    name: 'MultipleOfError',
    code: 'multiple-of-error',
    message: 'Expected `2.74` in `#` to be multiple of `0.01`',
    data: { multipleOf: 0.01, value: 2.74, pointer: '#' }
  }
]
2.75 []
2.76 [
  {
    type: 'error',
    name: 'MultipleOfError',
    code: 'multiple-of-error',
    message: 'Expected `2.76` in `#` to be multiple of `0.01`',
    data: { multipleOf: 0.01, value: 2.76, pointer: '#' }
  }
]
sagold commented 10 months ago

Yes, you can see this also in you browser console

1.36 % 0.01
> 6.938893903907228e-17

floating point arithmetic is a known issue and, to my knowledge, unsolvable. For example: https://github.com/ajv-validator/ajv/issues/84#issuecomment-364795655.

To work around most issues json-schema-library (like other validators) shifts decimals by multiplying it with a constant, but in this case javascript results to the following

1.36 * 10000
> 13600.000000000002

Changing the number to

1.36 * 100000
> 136000

results in a correct value, which you can configure in the settings

import { settings } from "json-schema-library";
settings.floatingPointPrecision = 100000;

but I would not rely on it and use the regex-pattern approach you mentioned. This would align the validation between the programming languages.

I am very open to other suggestions

sagold commented 10 months ago

Update I replaced the multipleOfPrecision proposal with the following solution:

export function getPrecision(value: number): number {
    const string = `${value}`;
    const index = string.indexOf(".");
    return index === -1 ? 0 : string.length - (index + 1);
}

function validateMultipleOf(draft, schema, value, pointer) {
    if (isNaN(schema.multipleOf) || typeof value !== "number") {
        return undefined;
    }

    const valuePrecision = getPrecision(value);
    const multiplePrecision = getPrecision(schema.multipleOf);
    if (valuePrecision > multiplePrecision) {
        // higher precision of value can never be multiple of lower precision
        return draft.errors.multipleOfError({
            multipleOf: schema.multipleOf,
            value,
            pointer,
            schema
        });
    }

    const precision = Math.pow(10, multiplePrecision);
    const val = Math.round(value * precision);
    const multiple = Math.round(schema.multipleOf * precision);
    if ((val % multiple) / precision !== 0) {
        return draft.errors.multipleOfError({
            multipleOf: schema.multipleOf,
            value,
            pointer,
            schema
        });
    }

    return undefined;
}

new Draft07(
  {
    type: "number",
    multipleOf: 0.01,
    multipleOfPrecision: 2
  },
  {
    validateKeyword: {
      multipleOf: validateMultipleOf
    }
  }
);

You should be able to add this configuration in your code or install with npm install sagold/json-schema-library#fix-multipleOf-floating-point-issue. Unit tests and json-schema test library validates this solution. I would feel comfortable with this change. Please verify if this solution works for you.

Cheers, sagold

xjamundx commented 10 months ago

This is so great. Thanks for your thorough explanation and showing us all of this magic, not to mention fixing the "unsolvable" :D

xjamundx commented 10 months ago

From my colleague Jemma about your fix

so smooth it’s ridiculous

πŸ†

sagold commented 10 months ago

πŸ˜€ Yes, the unsolvable label was set too quickly. Thank you, you are welcome!

So does this solve your issue? Then we could merge this.

xjamundx commented 10 months ago

It does yes!

On Tue, 15 Aug 2023 at 4:07 pm, Sascha Goldhofer @.***> wrote:

πŸ˜€ Yes, the unsolvable label was set too quickly. Thank you, you are welcome!

So does this solve your issue? Then we could merge this.

β€” Reply to this email directly, view it on GitHub https://github.com/sagold/json-schema-library/issues/43#issuecomment-1679096170, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAB4C76ADPHFF7MA6ZVLJ73XVOGBJANCNFSM6AAAAAA3Q3JZQM . You are receiving this because you authored the thread.Message ID: @.***>

sagold commented 10 months ago

This issue is fixed with json-schema-library@9.0.1.

Thank you for your feedback!