Skip to content

Commit

Permalink
H-3960: Make exclusiveMinimum/ Maximum number instead of boolean (#…
Browse files Browse the repository at this point in the history
…6611)

Co-authored-by: Ciaran Morinan <[email protected]>
  • Loading branch information
TimDiekmann and CiaranMn authored Mar 5, 2025
1 parent 74da7cb commit 12d23c3
Show file tree
Hide file tree
Showing 12 changed files with 394 additions and 240 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,8 @@ export const stripIdsFromDereferencedProperties = (params: {
} => {
const { properties } = params;

// @ts-expect-error - we need to update exclusiveMinimum and exclusiveMaximum to be numbers rather than booleans
return Object.entries(properties).reduce<{
[key: string]: JSONSchemaDefinition;
// @ts-expect-error - we need to update exclusiveMinimum and exclusiveMaximum to be numbers rather than booleans
}>((acc, [propertyKey, value]) => {
if ("items" in value) {
const { $id: _id, ...itemsWithoutId } = value.items;
Expand Down Expand Up @@ -57,7 +55,6 @@ export const stripIdsFromDereferencedProperties = (params: {

acc[propertyKey] = {
...valueWithoutId,
// @ts-expect-error - we need to update exclusiveMinimum and exclusiveMaximum to be numbers rather than booleans
oneOf: valueWithoutId.oneOf.map((oneOfValue) => {
if (typeof oneOfValue === "object" && "properties" in oneOfValue) {
return {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,11 +51,13 @@ const NumberRangeEditor = ({

const exclusiveMinimumDisabled =
hasEnum ||
ownMinimum === null ||
(ownMinimum === inheritedMinimum?.value.value &&
inheritedConstraints.minimum?.value.exclusive === true);

const exclusiveMaximumDisabled =
hasEnum ||
ownMaximum === null ||
(ownMaximum === inheritedMaximum?.value.value &&
inheritedConstraints.maximum?.value.exclusive === true);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -437,35 +437,44 @@ export const EnumEditor = ({
);
}

const mergedSchema: MergedValueSchema = useMemo(
() => ({
const mergedSchema: MergedValueSchema = useMemo(() => {
/**
* For tracking the data type form state, we use exclusiveMinimum and exclusiveMaximum as booleans,
* but externally (and in current JSON Schema drafts), they are numbers.
*/
const shouldBeExclusiveMinimum =
ownExclusiveMinimum ?? !!inheritedConstraints.minimum?.value.exclusive;
const shouldBeExclusiveMaximum =
ownExclusiveMaximum ?? !!inheritedConstraints.maximum?.value.exclusive;

const minimum = ownMinimum ?? inheritedConstraints.minimum?.value.value;
const maximum = ownMaximum ?? inheritedConstraints.maximum?.value.value;

return {
format: ownFormat ?? inheritedConstraints.format?.value,
minimum: ownMinimum ?? inheritedConstraints.minimum?.value.value,
maximum: ownMaximum ?? inheritedConstraints.maximum?.value.value,
exclusiveMinimum:
ownExclusiveMinimum ?? inheritedConstraints.minimum?.value.exclusive,
exclusiveMaximum:
ownExclusiveMaximum ?? inheritedConstraints.maximum?.value.exclusive,
minimum: shouldBeExclusiveMinimum ? undefined : minimum,
maximum: shouldBeExclusiveMaximum ? undefined : maximum,
exclusiveMinimum: shouldBeExclusiveMinimum ? minimum : undefined,
exclusiveMaximum: shouldBeExclusiveMaximum ? maximum : undefined,
multipleOf: ownMultipleOf
? [ownMultipleOf]
: inheritedConstraints.multipleOf?.map(({ value }) => value),
minLength: ownMinLength ?? inheritedConstraints.minLength?.value,
maxLength: ownMaxLength ?? inheritedConstraints.maxLength?.value,
type,
}),
[
ownFormat,
ownMinLength,
ownMaxLength,
ownMinimum,
ownMaximum,
ownExclusiveMinimum,
ownExclusiveMaximum,
ownMultipleOf,
inheritedConstraints,
type,
],
);
};
}, [
ownFormat,
ownMinLength,
ownMaxLength,
ownMinimum,
ownMaximum,
ownExclusiveMinimum,
ownExclusiveMaximum,
ownMultipleOf,
inheritedConstraints,
type,
]);

useEffect(() => {
for (const [index, value] of (items ?? []).entries()) {
Expand Down
46 changes: 38 additions & 8 deletions apps/hash-frontend/src/pages/shared/data-type/data-type-form.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -96,10 +96,24 @@ export const getDataTypeFromFormData = ({
} else {
unNulledConstraints = {
type: "number",
minimum: constraints.minimum ?? undefined,
maximum: constraints.maximum ?? undefined,
exclusiveMinimum: constraints.exclusiveMinimum ?? undefined,
exclusiveMaximum: constraints.exclusiveMaximum ?? undefined,
/**
* In JSON schema, exclusiveMinimum and exclusiveMaximum are numbers (after Draft 4),
* but in the form it is simpler to deal with them as booleans,
* so we need to convert them back to numbers here.
* We don't want both minimum and exclusiveMinimum to be set.
*/
minimum: !constraints.exclusiveMinimum
? (constraints.minimum ?? undefined)
: undefined,
maximum: !constraints.exclusiveMaximum
? (constraints.maximum ?? undefined)
: undefined,
exclusiveMinimum: constraints.exclusiveMinimum
? (constraints.minimum ?? undefined)
: undefined,
exclusiveMaximum: constraints.exclusiveMaximum
? (constraints.maximum ?? undefined)
: undefined,
multipleOf: constraints.multipleOf ?? undefined,
};
}
Expand Down Expand Up @@ -173,12 +187,28 @@ export const getFormDataFromDataType = (dataTypeWithMetadata: {
if ("enum" in constraints) {
nulledConstraints = { type: "number", enum: constraints.enum };
} else {
const applicableExclusiveMinimum =
constraints.exclusiveMinimum !== undefined
? constraints.minimum === undefined ||
constraints.minimum < constraints.exclusiveMinimum
: null;

const applicableExclusiveMaximum =
constraints.exclusiveMaximum !== undefined
? constraints.maximum === undefined ||
constraints.maximum > constraints.exclusiveMaximum
: null;

nulledConstraints = {
type: "number",
minimum: constraints.minimum ?? null,
maximum: constraints.maximum ?? null,
exclusiveMinimum: constraints.exclusiveMinimum ?? null,
exclusiveMaximum: constraints.exclusiveMaximum ?? null,
minimum: applicableExclusiveMinimum
? constraints.exclusiveMinimum!
: (constraints.minimum ?? null),
maximum: applicableExclusiveMaximum
? constraints.exclusiveMaximum!
: (constraints.maximum ?? null),
exclusiveMinimum: applicableExclusiveMinimum ?? null,
exclusiveMaximum: applicableExclusiveMaximum ?? null,
multipleOf: constraints.multipleOf ?? null,
};
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -191,29 +191,51 @@ const useInheritedConstraintsValue = (): InheritedConstraints => {
}
case "number": {
if (
typeof parentSchema.minimum !== "undefined" &&
!narrowedConstraints.minimum
typeof parentSchema.minimum !== "undefined" ||
typeof parentSchema.exclusiveMinimum !== "undefined"
) {
narrowedConstraints.minimum = {
value: {
value: parentSchema.minimum,
exclusive: !!parentSchema.exclusiveMinimum,
},
from: parentSchema,
};
const applicableExclusiveMinimum =
parentSchema.exclusiveMinimum !== undefined &&
(parentSchema.minimum === undefined ||
parentSchema.minimum < parentSchema.exclusiveMinimum);

const minimumValue = applicableExclusiveMinimum
? parentSchema.exclusiveMinimum
: parentSchema.minimum;

if (minimumValue !== undefined && !narrowedConstraints.minimum) {
narrowedConstraints.minimum = {
value: {
value: minimumValue,
exclusive: !!applicableExclusiveMinimum,
},
from: parentSchema,
};
}
}

if (
typeof parentSchema.maximum !== "undefined" &&
!narrowedConstraints.maximum
typeof parentSchema.maximum !== "undefined" ||
typeof parentSchema.exclusiveMaximum !== "undefined"
) {
narrowedConstraints.maximum = {
value: {
value: parentSchema.maximum,
exclusive: !!parentSchema.exclusiveMaximum,
},
from: parentSchema,
};
const applicableExclusiveMaximum =
parentSchema.exclusiveMaximum !== undefined &&
(parentSchema.maximum === undefined ||
parentSchema.maximum > parentSchema.exclusiveMaximum);

const maximumValue = applicableExclusiveMaximum
? parentSchema.exclusiveMaximum
: parentSchema.maximum;

if (maximumValue !== undefined && !narrowedConstraints.maximum) {
narrowedConstraints.maximum = {
value: {
value: maximumValue,
exclusive: !!applicableExclusiveMaximum,
},
from: parentSchema,
};
}
}

if (
Expand Down
33 changes: 21 additions & 12 deletions apps/hash-frontend/src/pages/shared/number-or-text-input.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -66,25 +66,34 @@ export const NumberOrTextInput = ({
? schema.multipleOf[0]
: 0.001;

// Get the effective minimum value, considering both minimum and exclusiveMinimum
const minimum =
"minimum" in schema && typeof schema.minimum === "number"
? schema.minimum
: undefined;

const exclusiveMinimum =
"exclusiveMinimum" in schema && typeof schema.exclusiveMinimum === "boolean"
"exclusiveMinimum" in schema && typeof schema.exclusiveMinimum === "number"
? schema.exclusiveMinimum
: false;
: undefined;

const minimum =
"minimum" in schema && typeof schema.minimum === "number"
? schema.minimum + (exclusiveMinimum ? step : 0)
const effectiveMinimum =
typeof exclusiveMinimum === "number" ? exclusiveMinimum + step : minimum;

// Get the effective maximum value, considering both maximum and exclusiveMaximum
const maximum =
"maximum" in schema && typeof schema.maximum === "number"
? schema.maximum
: undefined;

const exclusiveMaximum =
"exclusiveMaximum" in schema && typeof schema.exclusiveMaximum === "boolean"
"exclusiveMaximum" in schema && typeof schema.exclusiveMaximum === "number"
? schema.exclusiveMaximum
: false;
const maximum =
"maximum" in schema && typeof schema.maximum === "number"
? schema.maximum - (exclusiveMaximum ? step : 0)
: undefined;

const effectiveMaximum =
typeof exclusiveMaximum === "number" ? exclusiveMaximum - step : maximum;

const jsonStringFormat = "format" in schema ? schema.format : undefined;

let inputType: TextFieldProps["type"] = isNumber ? "number" : "text";
Expand Down Expand Up @@ -131,8 +140,8 @@ export const NumberOrTextInput = ({
inputProps: {
minLength,
maxLength,
min: minimum,
max: maximum,
min: effectiveMinimum,
max: effectiveMaximum,
step,
},
inputRef,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ impl From<AnyOfConstraints> for ValueConstraints {
&& constraints.any_of[0].description.is_none()
&& constraints.any_of[0].label.is_empty()
{
Self::Typed(constraints.any_of.remove(0).constraints)
Self::Typed(Box::new(constraints.any_of.remove(0).constraints))
} else {
Self::AnyOf(constraints)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ mod wasm {
#[cfg_attr(target_arch = "wasm32", derive(tsify::Tsify))]
#[serde(untagged, rename_all = "camelCase")]
pub enum ArraySchema {
Constrained(ArrayConstraints),
Constrained(Box<ArrayConstraints>),
Tuple(TupleConstraints),
}

Expand All @@ -164,10 +164,10 @@ impl Constraint for ArraySchema {
) -> Result<(Self, Option<Self>), Report<ResolveClosedDataTypeError>> {
Ok(match (self, other) {
(Self::Constrained(lhs), Self::Constrained(rhs)) => {
let (combined, remainder) = lhs.intersection(rhs)?;
let (combined, remainder) = lhs.intersection(*rhs)?;
(
Self::Constrained(combined),
remainder.map(Self::Constrained),
Self::Constrained(Box::new(combined)),
remainder.map(|remainder| Self::Constrained(Box::new(remainder))),
)
}
(Self::Tuple(lhs), Self::Constrained(rhs)) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ pub trait ConstraintValidator<V: ?Sized>: Constraint {
#[cfg_attr(target_arch = "wasm32", derive(tsify::Tsify))]
#[serde(untagged, rename_all = "camelCase")]
pub enum ValueConstraints {
Typed(SingleValueConstraints),
Typed(Box<SingleValueConstraints>),
AnyOf(AnyOfConstraints),
}

Expand Down Expand Up @@ -130,13 +130,16 @@ impl Constraint for ValueConstraints {
other: Self,
) -> Result<(Self, Option<Self>), Report<ResolveClosedDataTypeError>> {
match (self, other) {
(Self::Typed(lhs), Self::Typed(rhs)) => lhs
.intersection(rhs)
.map(|(lhs, rhs)| (Self::Typed(lhs), rhs.map(Self::Typed))),
(Self::Typed(lhs), Self::Typed(rhs)) => lhs.intersection(*rhs).map(|(lhs, rhs)| {
(
Self::Typed(Box::new(lhs)),
rhs.map(|rhs| Self::Typed(Box::new(rhs))),
)
}),
(Self::AnyOf(lhs), Self::Typed(rhs)) => {
let rhs = AnyOfConstraints {
any_of: vec![SingleValueSchema {
constraints: rhs,
constraints: *rhs,
description: None,
label: ValueLabel::default(),
}],
Expand All @@ -147,7 +150,7 @@ impl Constraint for ValueConstraints {
(Self::Typed(lhs), Self::AnyOf(rhs)) => {
let lhs = AnyOfConstraints {
any_of: vec![SingleValueSchema {
constraints: lhs,
constraints: *lhs,
description: None,
label: ValueLabel::default(),
}],
Expand Down
Loading

0 comments on commit 12d23c3

Please sign in to comment.