Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Fix multipleOf to work with decimals #436

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
97 changes: 23 additions & 74 deletions jsonschema/src/keywords/multiple_of.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,87 +9,33 @@ use crate::{
use fraction::{BigFraction, BigUint};
use serde_json::{Map, Value};

pub(crate) struct MultipleOfFloatValidator {
pub(crate) struct MultipleOfValidator {
multiple_of: f64,
schema_path: JSONPointer,
}

impl MultipleOfFloatValidator {
impl MultipleOfValidator {
#[inline]
pub(crate) fn compile<'a>(multiple_of: f64, schema_path: JSONPointer) -> CompilationResult<'a> {
Ok(Box::new(MultipleOfFloatValidator {
Ok(Box::new(MultipleOfValidator {
multiple_of,
schema_path,
}))
}
}

impl Validate for MultipleOfFloatValidator {
impl Validate for MultipleOfValidator {
fn is_valid(&self, instance: &Value) -> bool {
if let Value::Number(item) = instance {
let item = item.as_f64().expect("Always valid");
let remainder = (item / self.multiple_of) % 1.;
if remainder.is_nan() {
// Involves heap allocations via the underlying `BigUint` type
let fraction = BigFraction::from(item) / BigFraction::from(self.multiple_of);
if let Some(denom) = fraction.denom() {
denom == &BigUint::from(1_u8)
} else {
true
}
} else {
remainder < f64::EPSILON
}
} else {
true
}
}
let mut tmp_item = item.as_f64().expect("Always valid");
let mut tmp_multiple_of = self.multiple_of;

fn validate<'instance>(
&self,
instance: &'instance Value,
instance_path: &InstancePath,
) -> ErrorIterator<'instance> {
if !self.is_valid(instance) {
return error(ValidationError::multiple_of(
self.schema_path.clone(),
instance_path.into(),
instance,
self.multiple_of,
));
}
no_error()
}
}

impl core::fmt::Display for MultipleOfFloatValidator {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
write!(f, "multipleOf: {}", self.multiple_of)
}
}

pub(crate) struct MultipleOfIntegerValidator {
multiple_of: f64,
schema_path: JSONPointer,
}

impl MultipleOfIntegerValidator {
#[inline]
pub(crate) fn compile<'a>(multiple_of: f64, schema_path: JSONPointer) -> CompilationResult<'a> {
Ok(Box::new(MultipleOfIntegerValidator {
multiple_of,
schema_path,
}))
}
}
while tmp_item.fract() != 0. {
tmp_item *= 10.0;
tmp_multiple_of *= 10.0;
}

impl Validate for MultipleOfIntegerValidator {
fn is_valid(&self, instance: &Value) -> bool {
if let Value::Number(item) = instance {
let item = item.as_f64().expect("Always valid");
// As the divisor has its fractional part as zero, then any value with a non-zero
// fractional part can't be a multiple of this divisor, therefore it is short-circuited
item.fract() == 0. && (item % self.multiple_of) == 0.
tmp_item % tmp_multiple_of == 0.0
} else {
true
}
Expand All @@ -112,7 +58,7 @@ impl Validate for MultipleOfIntegerValidator {
}
}

impl core::fmt::Display for MultipleOfIntegerValidator {
impl core::fmt::Display for MultipleOfValidator {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
write!(f, "multipleOf: {}", self.multiple_of)
}
Expand All @@ -126,14 +72,10 @@ pub(crate) fn compile<'a>(
if let Value::Number(multiple_of) = schema {
let multiple_of = multiple_of.as_f64().expect("Always valid");
let schema_path = context.as_pointer_with("multipleOf");
if multiple_of.fract() == 0. {
Some(MultipleOfIntegerValidator::compile(
multiple_of,
schema_path,
))
} else {
Some(MultipleOfFloatValidator::compile(multiple_of, schema_path))
}
Some(MultipleOfValidator::compile(
multiple_of,
schema_path,
))
} else {
Some(Err(ValidationError::single_type_error(
JSONPointer::default(),
Expand All @@ -154,11 +96,18 @@ mod tests {
#[test_case(&json!({"multipleOf": 1.0}), &json!(4.0))]
#[test_case(&json!({"multipleOf": 1.5}), &json!(3.0))]
#[test_case(&json!({"multipleOf": 1.5}), &json!(4.5))]
#[test_case(&json!({"multipleOf": 0.1}), &json!(1.1))]
#[test_case(&json!({"multipleOf": 0.1}), &json!(1.2))]
#[test_case(&json!({"multipleOf": 0.1}), &json!(1.3))]
#[test_case(&json!({"multipleOf": 0.02}), &json!(1.02))]
fn multiple_of_is_valid(schema: &Value, instance: &Value) {
tests_util::is_valid(schema, instance)
}

#[test_case(&json!({"multipleOf": 1.0}), &json!(4.5))]
#[test_case(&json!({"multipleOf": 0.1}), &json!(4.55))]
#[test_case(&json!({"multipleOf": 0.2}), &json!(4.5))]
#[test_case(&json!({"multipleOf": 0.02}), &json!(1.01))]
fn multiple_of_is_not_valid(schema: &Value, instance: &Value) {
tests_util::is_not_valid(schema, instance)
}
Expand Down