mikuso / ocpp-rpc

A Node.js client & server implementation of the WAMP-like RPC-over-websocket system defined in the OCPP-J protocols.
MIT License
97 stars 29 forks source link

Validation issue with strictMode and the multipleOf keyword in Ajv when using decimal numbers #50

Closed Nextdrive-IanChiu closed 1 year ago

Nextdrive-IanChiu commented 1 year ago

Hi,

I noticed that there is a validation issue with the strictMode and multipleOf keyword in Ajv when using decimal numbers. When I receive a certain PDU of GetCompositeSchedule.conf, I encounter an error message stating "data/chargingSchedule/chargingSchedulePeriod/0/limit must be a multiple of 0.1". This occurs because the value of the limit (9.1) is not considered a multiple of 0.1 due to a precision issue in JavaScript where 9.1/0.1 is equal to 90.99999999999999.

// example PDU of GetCompositeSchedule.conf
{
  "status":"Accepted",
  "connectorId":0,
  "scheduleStart":"2023-07-13T07:36:10Z",
  "chargingSchedule":{
    "duration":300,
    "startSchedule":"2023-07-13T07:36:10Z",
    "chargingRateUnit":"A",
    "chargingSchedulePeriod":[{
      "startPeriod":0,
      "limit":9.1,
      "numberPhases":3
    }]
  }
}

I understand that this is an issue with Ajv. However, I was wondering if we could implement a temporary workaround for this problem until Ajv addresses it. One possible solution could be to modify the Ajv configuration by removing the default "multipleOf" keyword and adding a custom implementation that handles decimal numbers using the "number-precision" library.

Here's an example of how we could modify the Ajv configuration:

import Ajv from "ajv";
import NP from "number-precision";

const ajv = new Ajv();
// remove default keyword: multipleOf
ajv.removeKeyword("multipleOf");

ajv.addKeyword({
    keyword: "multipleOf",
    type: "number",
    compile(schema) {
        return (data) => Number.isInteger(NP.divide(data, schema));
    },
    errors: false,
    metaSchema: {
        type: "number",
    },
});

const schema = {
    type: "object",
    properties: {
        price: { type: "number", minimum: 0, multipleOf: 0.01 },
    },
    required: ["price"],
    additionalProperties: false,
};

const data = {price: 2.22}

https://github.com/ajv-validator/ajv/issues/652#issuecomment-944202626

By implementing this workaround, we can handle decimal numbers more accurately in validation process.

Let me know if you have any further questions or if there's anything else I can assist you with.

Nextdrive-IanChiu commented 1 year ago

Update:

Perhaps we could use multipleOfPercision instead. https://ajv.js.org/options.html#multipleofprecision

mikuso commented 1 year ago

Hi @Nextdrive-IanChiu

Thanks for the report.

I will put together some test cases and see if either of these options fix the issue.

Leave it with me 👍

mikuso commented 1 year ago

Thank you for the detailed description of the problem and the suggested fixes.

I settled on a solution that was a combination of both of these ideas.

Rather than add multipleOfPrecision to the schemas, I thought it best to simply re-implement the ajv multipleOf keyword to use a presumed default multipleOfPrecision value of 6 (borrowing the implementation of this from the ajv libarary). Perhaps this might need to be revisited later, but it seems like it should be sane enough for most use cases for the time being.

Fixed in 55a85b90b4e44d4773482eafe8cdab9e98f0d4ef. Version 2.0.1 published to npm.

Nextdrive-IanChiu commented 1 year ago

Hi @mikuso

Thanks a ton for getting back to me so quickly and fixing the issue. I gave version 2.0.1 a try, and I'm happy to report that the problem with Ajv's precision has been sorted out. Awesome!

I just wanted to drop a quick note to express my gratitude for not only resolving the issue but also for creating such an amazing library that makes implementing OCPP-related features a piece of cake.

mikuso commented 1 year ago

Thanks for the kind words. Much appreciated.