Skip to content

fix(ids): don't crash on properties without data; match any value of list, bounded, enumerated and table properties #828

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.

Sign up for GitHub

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

Merged
agviegas merged 1 commit into from
Oct 11, 2026
Merged

fix(ids): don't crash on properties without data; match any value of list, bounded, enumerated and table properties #828

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
189 changes: 80 additions & 109 deletions packages/core/src/openbim/IDSSpecifications/src/facets/Property.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ export class IDSProperty extends IDSFacet {
!(
"value" in set._localId &&
"value" in set._category &&
set.Name &&
"value" in set.Name
)
) {
Expand Down Expand Up @@ -303,7 +304,7 @@ export class IDSProperty extends IDSFacet {

const sets = await this.getPsets(item);
const matchingSets = sets.filter((set) => {
if (!("value" in set.Name)) return false;
if (!(set.Name && "value" in set.Name)) return false;
const result = this.evalRequirement(
set.Name.value,
this.propertySet,
Expand Down Expand Up @@ -345,6 +346,7 @@ export class IDSProperty extends IDSFacet {
}

const matchingProps = props.filter((item) => {
if (!(item._category && item.Name)) return false;
if (!("value" in item._category && "value" in item.Name))
return false;
if (this._unsupportedTypes.includes(item._category.value)) {
Expand Down Expand Up @@ -389,7 +391,7 @@ export class IDSProperty extends IDSFacet {

private getPropertyListName(item: FRAGS.ItemData) {
let propsListName: "HasProperties" | "Quantities" | undefined;
if (!("value" in item._category)) return propsListName;
if (!(item._category && "value" in item._category)) return propsListName;

if (item._category.value === "IFCPROPERTYSET")
propsListName = "HasProperties";
Expand All @@ -399,10 +401,33 @@ export class IDSProperty extends IDSFacet {
return propsListName;
}

private getValueKey(item: FRAGS.ItemData) {
return Object.keys(item).find(
(name) => /Value/.test(name) || /Values/.test(name),
);
// Every value attribute of a property: NominalValue, ListValues,
// LowerBoundValue / UpperBoundValue / SetPointValue, EnumerationValues,
// DefiningValues / DefinedValues, and the quantities' *Value. Relations
// (arrays of items) and unset attributes are left out.
private getValueAttrs(item: FRAGS.ItemData) {
const attrs: { value: unknown; type?: string }[] = [];
for (const [name, attr] of Object.entries(item)) {
if (!/Values?$/.test(name) || !attr || Array.isArray(attr)) continue;
if (!("value" in attr) || attr.value === null || attr.value === undefined)
continue;
attrs.push(attr as { value: unknown; type?: string });
}
return attrs;
}

private describeValues(values: (string | number | boolean)[]) {
if (values.length === 0) return null;
if (values.length === 1) return values[0];
return values.join(", ");
}

// IDS: a property with several values (list, bounded, enumerated, table)
// matches when any one of them does.
private getValues(item: FRAGS.ItemData) {
return this.getValueAttrs(item).flatMap((attr) =>
Array.isArray(attr.value) ? attr.value : [attr.value],
) as (string | number | boolean)[];
}

private getTypePsets(item: FRAGS.ItemData) {
Expand Down Expand Up @@ -435,13 +460,13 @@ export class IDSProperty extends IDSFacet {
const mergedTypeSets = new Set<FRAGS.ItemData>();

for (const definition of item.IsDefinedBy) {
if (!("value" in definition.Name)) continue;
if (!(definition.Name && "value" in definition.Name)) continue;
const definitionName = definition.Name.value;
const propsListName = this.getPropertyListName(definition);
if (!(definitionName && propsListName)) continue;

const typeSet = typeSets.find((set) => {
if (!("value" in set.Name)) return false;
if (!(set.Name && "value" in set.Name)) return false;
return set.Name.value === definitionName;
});

Expand All @@ -452,10 +477,10 @@ export class IDSProperty extends IDSFacet {
Array.isArray(definition.HasProperties)
) {
for (const prop of typeSet.HasProperties) {
if (!("value" in prop.Name)) continue;
if (!(prop.Name && "value" in prop.Name)) continue;
const name = prop.Name.value;
const existingProp = definition.HasProperties.find((p) => {
if (!("value" in p.Name)) return false;
if (!(p.Name && "value" in p.Name)) return false;
return p.Name.value === name;
});
if (!existingProp) definition.HasProperties.push(prop);
Expand All @@ -476,67 +501,40 @@ export class IDSProperty extends IDSFacet {
return sets;
}

// IFCPROPERTYBOUNDEDVALUE are not supported yet
// IFCPROPERTYTABLEVALUE are not supported yet
// TODO: Work must to be done to convert numerical value units to IDS-nominated standard units https://github.com/buildingSMART/IDS/blob/development/Documentation/UserManual/units.md
private evalValue(item: FRAGS.ItemData, checks?: IDSCheck[]) {
const valueKey = this.getValueKey(item) as any;
const valueAttr = item[valueKey];
if (!("value" in valueAttr)) return false;
const values = this.getValues(item);

if (this.value) {
if (!valueKey) {
checks?.push({
parameter: "Value",
currentValue: null,
pass: false,
requiredValue: this.value,
});
return false;
}

const facetValue = structuredClone(this.value);
// The IDS XML parser coerces numeric-looking simpleValues to JS numbers
// (e.g. "410.20257" becomes 410.20257). When the actual IFC value is a
// string (IFCTEXT, IFCIDENTIFIER, IFCLABEL, ...), a strict comparison
// against that number always fails, so align the facet value to a string
// whenever the property value is a string (issue #731).
if (typeof valueAttr.value === "string" && facetValue.type === "simple") {
facetValue.parameter = String(facetValue.parameter);
const facetValue = this.value;
const match = values.find((value) => {
const parameter = structuredClone(facetValue);
// The IDS XML parser coerces numeric-looking simpleValues to JS numbers
// (e.g. "410.20257" becomes 410.20257). When the actual IFC value is a
// string (IFCTEXT, IFCIDENTIFIER, IFCLABEL, ...), a strict comparison
// against that number always fails, so align the facet value to a
// string whenever the property value is a string (issue #731).
if (typeof value === "string" && parameter.type === "simple") {
parameter.parameter = String(parameter.parameter);
}
return this.evalRequirement(value, parameter, "Value");
});
const pass = match !== undefined;
if (checks) {
this.addCheckResult(
{
parameter: "Value",
currentValue: pass ? match : this.describeValues(values),
pass,
requiredValue: facetValue,
},
checks,
);
}

// if (
// (item.type === WEBIFC.IFCPROPERTYLISTVALUE ||
// item.type === WEBIFC.IFCPROPERTYENUMERATEDVALUE) &&
// Array.isArray(valueAttr)
// ) {
// const values = valueAttr.map((value) => value.value);
// const matchingValue = valueAttr.find((value) => {
// if (!facetValue) return false;
// return this.evalRequirement(value.value, facetValue, "Value");
// });

// checks?.push({
// currentValue: values as any,
// pass: !!matchingValue,
// parameter: "Value",
// requiredValue: facetValue.parameter,
// });

// return !!matchingValue;
// }

const result = this.evalRequirement(
valueAttr.value,
facetValue,
"Value",
checks,
);

return result;
return pass;
}

if (!valueKey) return true;
if (values.length === 0) return true;

// IDSDocs: Values with a logical unknown always fail
// if (valueAttr.type === 3 && valueAttr.value === 2) {
Expand All @@ -550,7 +548,7 @@ export class IDSProperty extends IDSFacet {
// }

// IDSDocs: An empty string is considered false
if (typeof valueAttr.value === "string" && valueAttr.value.trim() === "") {
if (values.every((v) => typeof v === "string" && v.trim() === "")) {
checks?.push({
parameter: "Value",
currentValue: "",
Expand All @@ -565,51 +563,24 @@ export class IDSProperty extends IDSFacet {

private evalDataType(item: FRAGS.ItemData, checks?: IDSCheck[]) {
if (!this.dataType) return true;
const valueKey = this.getValueKey(item);

if (!(valueKey && "value" in item[valueKey])) {
checks?.push({
parameter: "DataType",
currentValue: null,
pass: false,
requiredValue: this.dataType,
});
return false;
}

const valueAttr = item[valueKey];

// TODO: Redo this
// if (
// (item.type === WEBIFC.IFCPROPERTYLISTVALUE ||
// item.type === WEBIFC.IFCPROPERTYENUMERATEDVALUE) &&
// Array.isArray(valueAttr) &&
// valueAttr[0]
// ) {
// const valueType = valueAttr[0].name;
// const result = this.evalRequirement(
// valueType,
// {
// type: "simple",
// parameter: this.dataType,
// },
// "DataType",
// checks,
// );
// return result;
// }

const result = this.evalRequirement(
(valueAttr as any).type ?? null,
{
type: "simple",
parameter: this.dataType,
},
"DataType",
checks,
const types = this.getValueAttrs(item).map((attr) => attr.type ?? null);
const dataType = { type: "simple" as const, parameter: this.dataType };
const match = types.find((type) =>
this.evalRequirement(type, dataType, "DataType"),
);

return result;
const pass = match !== undefined;
if (checks) {
this.addCheckResult(
{
parameter: "DataType",
currentValue: pass ? match : types[0] ?? null,
pass,
requiredValue: dataType,
},
checks,
);
}
return pass;
}

private evalURI() {
Expand Down
116 changes: 116 additions & 0 deletions packages/core/src/openbim/IDSSpecifications/src/facets/property-values.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
import * as FRAGS from "@thatopen/fragments";
import * as WEBIFC from "web-ifc";
import * as fs from "fs";
import * as path from "path";
import { afterAll, beforeAll, describe, expect, it, vi } from "vitest";
import type { Components } from "../../../../index";
import { loadSourceBundle } from "../../../../../tests/source-bundle";

// Needs the real `Components`: see `loadSourceBundle`.
const OBC = await loadSourceBundle();

// `tests/fixtures/property-values.ifc`: wall 10 with a list (A, B), a bounded
// (1 to 5, set point 3), an enumerated (EXISTING, DEMOLISH, of NEW, EXISTING,
// DEMOLISH) and a table (K -> 2) property in Test_Values.
const WALL = 10;
const MULTI_VALUE_CLASSES = [
WEBIFC.IFCPROPERTYLISTVALUE,
WEBIFC.IFCPROPERTYBOUNDEDVALUE,
WEBIFC.IFCPROPERTYENUMERATEDVALUE,
WEBIFC.IFCPROPERTYENUMERATION,
WEBIFC.IFCPROPERTYTABLEVALUE,
];

const convert = async (id: string, multiValue: boolean) => {
const importer = new FRAGS.IfcImporter();
if (multiValue) {
for (const c of MULTI_VALUE_CLASSES) importer.classes.abstract.add(c);
}
const root = path.resolve(__dirname, "../../../../..");
importer.wasm = {
path: `${path.resolve(root, "../../node_modules/web-ifc")}/`,
absolute: true,
};
const ifc = fs.readFileSync(
path.join(root, "tests", "fixtures", "property-values.ifc"),
);
const bytes = await importer.process({
bytes: new Uint8Array(ifc),
raw: true,
});
return new FRAGS.SingleThreadedFragmentsModel(id, bytes, true);
};

describe("IDSProperty on list, bounded, enumerated and table values", () => {
let components: Components;
const models = new Map<string, FRAGS.SingleThreadedFragmentsModel>();

beforeAll(async () => {
// "plain": the importer's default classes, which leave these properties
// out, so the pset refers to items without data.
models.set("plain", await convert("plain", false));
models.set("multi", await convert("multi", true));
components = new OBC.Components();
vi.spyOn(
components.get(OBC.FragmentsManager),
"list",
"get",
).mockReturnValue(models as never);
}, 60_000);

afterAll(() => {
vi.restoreAllMocks();
components?.dispose();
for (const model of models.values()) model.dispose();
});

const passes = async (
modelId: string,
baseName: string,
value?: string | number,
dataType?: string,
) => {
const facet = new OBC.IDSProperty(
components,
{ type: "simple", parameter: "Test_Values" },
{ type: "simple", parameter: baseName },
);
if (value !== undefined) facet.value = { type: "simple", parameter: value };
if (dataType) facet.dataType = dataType;
const results = new FRAGS.DataMap<
string,
FRAGS.DataMap<number, { pass: boolean }>
>();
await facet.test({ [modelId]: new Set([WALL]) }, results as never, {
skipIfFails: false,
});
return results.get(modelId)!.get(WALL)!.pass;
};

it("does not throw on properties the importer left out; they fail as missing", async () => {
for (const name of ["Codes", "Range", "Status", "Lookup"]) {
expect(await passes("plain", name)).toBe(false);
}
});

it("passes when any value of the property matches", async () => {
expect(await passes("multi", "Codes", "B")).toBe(true);
expect(await passes("multi", "Codes", "C")).toBe(false);
expect(await passes("multi", "Range", 3)).toBe(true);
expect(await passes("multi", "Range", 5)).toBe(true);
expect(await passes("multi", "Range", 4)).toBe(false);
expect(await passes("multi", "Status", "DEMOLISH")).toBe(true);
// In the enumeration, but not a value of the property.
expect(await passes("multi", "Status", "NEW")).toBe(false);
expect(await passes("multi", "Lookup", "K")).toBe(true);
expect(await passes("multi", "Lookup", 2)).toBe(true);
});

it("checks the data type of the values", async () => {
expect(await passes("multi", "Range", undefined, "IFCLENGTHMEASURE")).toBe(
true,
);
expect(await passes("multi", "Codes", undefined, "IFCLABEL")).toBe(true);
expect(await passes("multi", "Codes", undefined, "IFCTEXT")).toBe(false);
});
});
Loading
Loading