Skip to content

fix(ids): implement attribute selection and value presence checks #855

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

Draft
rihokirss wants to merge 3 commits into
base: main
Choose a base branch
from
+2,110 −202
Draft

fix(ids): implement attribute selection and value presence checks #855

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
240b1eb
fix(ids): apply optional and prohibited cardinality to the property f…
helenkwok Nov 7, 2026
c9f0a57
fix(ids): an optional property requirement never fails
helenkwok Nov 7, 2026
74cdc2c
fix(ids): implement attribute selection and value presence checks
rihokirss Nov 7, 2026
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
197 changes: 73 additions & 124 deletions packages/core/src/openbim/IDSSpecifications/src/facets/Attribute.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { DataMap } from "@thatopen/fragments";
import { IDSFacetParameter, IDSItemCheckResult } from "../types";
import { Components } from "../../../../core/Components";
import { IDSFacet } from "./Facet";
Expand Down Expand Up @@ -37,43 +38,24 @@ export class IDSAttribute extends IDSFacet {
</attribute>`;
}

// This can be very ineficcient as we do not have an easy way to get an entity based on an attribute
// Right now, all entities must be iterated.
// When the new IfcEntitiesFinder comes, this can become easier.
// This may be greatly increase in performance if the applicability has any of the other facets and this is applied the latest
async getEntities() {}
// async getEntities(
// model: FRAGS.FragmentsGroup,
// collector: FRAGS.IfcProperties = {},
// ) {
// return [];
// // for (const expressID in model) {
// // if (collector[expressID]) continue;
// // const entity = model[expressID];
// // // Check if the attribute exists
// // const attribute = entity[this.name];
// // const attributeExists = !!attribute;
// // // Check if the attribute value matches
// // let valueMatches = true;
// // if (attributeExists && this.value && this.value.value) {
// // if (this.value.type === "simpleValue") {
// // valueMatches = attribute.value === this.value.value;
// // }
// // if (this.value.type === "restriction") {
// // const regex = new RegExp(this.value.value);
// // valueMatches = regex.test(attribute.value);
// // }
// // }
// // if (attributeExists && valueMatches) {
// // collector[entity.expressID] = entity;
// // }
// // }
// }
async getEntities(modelIds: RegExp[], collector: ModelIdMap) {
const fragments = this._components.get(FragmentsManager);
for (const [modelId, model] of fragments.list) {
if (!modelIds.some((pattern) => pattern.test(modelId))) continue;
const ids = Object.values(
await model.getItemsOfCategories([/.*/]),
).flat();
const results: ModelIdDataMap<IDSItemCheckResult> = new DataMap();
await this.test({ [modelId]: new Set(ids) }, results, {
skipIfFails: false,
});
const selected = collector[modelId] ?? new Set<number>();
for (const [id, result] of results.get(modelId) ?? [])
if (result.pass) selected.add(id);
if (selected.size) collector[modelId] = selected;
}
}

// https://github.com/buildingSMART/IDS/tree/development/Documentation/ImplementersDocumentation/TestCases/attribute
// Test cases from buildingSMART repo have been tested and they all match with the expected result
// All invalid cases have been treated as failures
// FragmentsGroup do not hold some of the entities used in the tests
async test(
items: ModelIdMap,
collector: ModelIdDataMap<IDSItemCheckResult>,
Expand All @@ -83,7 +65,10 @@ export class IDSAttribute extends IDSFacet {
for (const [modelId, localIds] of Object.entries(items)) {
const model = fragments.list.get(modelId);
if (!model) continue;
const data = await model.getItemsData([...localIds]);
const data = await model.getItemsData([...localIds], {
attributesDefault: true,
relationsDefault: { attributes: true, relations: false },
});
for (const item of data) {
const checks = this.getItemChecks(
collector,
Expand All @@ -93,99 +78,63 @@ export class IDSAttribute extends IDSFacet {
);
if (!checks) continue;

const attrNames = Object.keys(item);
const matchingAttributes = attrNames.filter((name) => {
const nameMatches = this.evalRequirement(name, this.name, "Name");
if (!nameMatches) return false;

const attribute = item[name];
if (Array.isArray(attribute)) return true;

// IDSDocs: Attributes with null values always fail
if (attribute === null || attribute.value === null) {
if (
this.cardinality === "optional" ||
this.cardinality === "prohibited"
) {
return true;
// GlobalId is retained by Fragments as _guid. Internal metadata is not
// an IFC attribute and must never satisfy a name restriction.
const attributes = { ...item, GlobalId: item._guid };
const names = Object.keys(attributes).filter(
(name) =>
!name.startsWith("_") &&
this.evalRequirement(
name,
this.name,
"Name",
undefined,
"required",
),
);
let present = false;
let matches = false;
let actual: string | number | boolean | null = null;
for (const name of names) {
const attribute = attributes[name as keyof typeof attributes];
if (attribute == null) continue;
if (Array.isArray(attribute)) {
if (attribute.length) {
present = true;
if (!this.value) matches = true;
}
return false;
continue;
}

// IDSDocs: Attributes with a logical unknown always fail
// TODO: Can this be replicated with Fragments 2.0?
// if (nameMatches && attribute?.type === 3 && attribute.value === 2) {
// return false;
// }

// IDSDocs: Attributes with an empty list always fail
// TODO: How to know for sure if the attribute value is an array
if (Array.isArray(attribute.value) && attribute.value.length === 0) {
return false;
const value = attribute.value;
if (value === null || value === undefined) continue;
present = true;
if (typeof value === "string" && !value.trim()) continue;
if (Array.isArray(value)) {
if (value.length && !this.value) matches = true;
continue;
}

// IDSDocs: Attributes with empty strings always fail
actual = value;
if (
typeof attribute.value === "string" &&
attribute.value.trim() === ""
) {
return false;
}

return nameMatches;
});

const attributeMatches = matchingAttributes.length > 0;

!this.value ||
this.evalRequirement(
value,
this.value,
"Value",
undefined,
"required",
)
)
matches = true;
}
let pass = matches;
if (this.cardinality === "prohibited") pass = !matches;
if (this.cardinality === "optional" && !present) pass = true;
checks.push({
parameter: "Name",
currentValue: attributeMatches ? matchingAttributes[0] : null,
requiredValue: this.name,
pass:
this.cardinality === "prohibited"
? !attributeMatches
: attributeMatches,
parameter: this.value ? "Value" : "Name",
currentValue: actual,
requiredValue: this.value ?? this.name,
pass,
});

// Check if the attribute value matches
if (this.value) {
if (matchingAttributes[0]) {
const attribute = item[matchingAttributes[0]];
if (Array.isArray(attribute)) {
checks.push({
parameter: "Value",
currentValue: null,
requiredValue: this.value,
pass: this.cardinality === "prohibited",
});
} else {
// IDSDocs: Value checks always fail for objects
const isRef = Array.isArray(attribute.value);
if (isRef) {
checks.push({
parameter: "Value",
currentValue: null,
requiredValue: this.value,
pass: this.cardinality === "prohibited",
});
} else {
this.evalRequirement(
attribute.value,
this.value,
"Value",
checks,
);
}
}
} else {
checks.push({
parameter: "Value",
currentValue: null,
requiredValue: this.value,
pass: this.cardinality === "prohibited",
});
}
}
}
}
}
Expand Down
6 changes: 4 additions & 2 deletions packages/core/src/openbim/IDSSpecifications/src/facets/Facet.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ export abstract class IDSFacet {
facetParameter: IDSFacetParameter,
parameter: IDSFacetParameterName,
checks?: IDSCheck[],
cardinality: IDSSimpleCardinality | IDSConditionalCardinaltiy = this
.cardinality,
) => {
const checkLog: IDSCheck = {
parameter,
Expand Down Expand Up @@ -96,8 +98,8 @@ export abstract class IDSFacet {
pass = minPass && maxPass;
}

if (this.cardinality === "prohibited") pass = !pass;
if (this.cardinality === "optional") pass = true;
if (cardinality === "prohibited") pass = !pass;
if (cardinality === "optional") pass = true;

checkLog.pass = pass;
return checkLog.pass;
Expand Down
Loading
Loading