fix(ids): apply optional and prohibited cardinality to the property facet as a whole

packages/core/src/openbim/IDSSpecifications/src/facets/Facet.ts

@@ -41,6 +41,8 @@ export abstract class IDSFacet {
     facetParameter: IDSFacetParameter,
     parameter: IDSFacetParameterName,
     checks?: IDSCheck[],
+    cardinality: IDSSimpleCardinality | IDSConditionalCardinaltiy = this
+      .cardinality,
   ) => {
     const checkLog: IDSCheck = {
       parameter,
@@ -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;

packages/core/src/openbim/IDSSpecifications/src/facets/Property.ts

@@ -84,7 +84,10 @@ export class IDSProperty extends IDSFacet {
       // Matching sets, by localId: their name and the property names that
       // matched. A set attached to a type (IfcTypeObject.HasPropertySets)
       // has no DefinesOccurrence, so its elements are found from the type.
-      const matchedSets = new Map<number, { name: string; props: Set<string> }>();
+      const matchedSets = new Map<
+        number,
+        { name: string; props: Set<string> }
+      >();
 
       for (const set of data) {
         const definesOccurrence = set.DefinesOccurrence ?? set.DefinesOcurrence;
@@ -220,7 +223,8 @@ export class IDSProperty extends IDSFacet {
     for (const type of types) {
       if (!Array.isArray(type.HasPropertySets)) continue;
       const sets = type.HasPropertySets.flatMap((set) => {
-        const id = set._localId && "value" in set._localId ? set._localId.value : null;
+        const id =
+          set._localId && "value" in set._localId ? set._localId.value : null;
         const matched = matchedSets.get(id as number);
         return matched ? [matched] : [];
       });
@@ -260,12 +264,15 @@ export class IDSProperty extends IDSFacet {
           const props = list ? set[list] : undefined;
           if (!Array.isArray(props)) return false;
           return props.some(
-            (prop) => prop.Name && "value" in prop.Name && prop.Name.value === propName,
+            (prop) =>
+              prop.Name && "value" in prop.Name && prop.Name.value === propName,
           );
         });
       const inherits = inherited
         .get(id)!
-        .some((set) => [...set.props].some((prop) => !overrides(set.name, prop)));
+        .some((set) =>
+          [...set.props].some((prop) => !overrides(set.name, prop)),
+        );
       if (inherits) applies.push(id);
     }
     ModelIdMapUtils.append(collector, modelId, ...applies);
@@ -301,90 +308,132 @@ export class IDSProperty extends IDSFacet {
         );
         if (!checks) continue;
 
-        const sets = await this.getPsets(item);
-        const matchingSets = sets.filter((set) => {
-          if (!("value" in set.Name)) return false;
-          const result = this.evalRequirement(
-            set.Name.value,
-            this.propertySet,
-            "PropertySet",
-          );
-          if (!result) return false;
+        // Evaluate the facet itself (as if required), then apply the
+        // cardinality once. Prohibited: passes when the facet does not match.
+        // Optional: never fails ("If the requirement is optional but would fail
+        // if it were required instead, the checker tool must not log an
+        // error", IDS developer guide).
+        const facetChecks: IDSCheck[] = [];
+        const present = await this.evalFacet(item, facetChecks);
+        const matches =
+          facetChecks.length > 0 && facetChecks.every(({ pass }) => pass);
+
+        if (this.cardinality === "prohibited") {
+          checks.push({
+            parameter: "BaseName",
+            currentValue: present ? this.baseName.parameter : null,
+            pass: !matches,
+            requiredValue: this.baseName,
+          } as IDSCheck);
+        } else if (this.cardinality === "optional") {
           checks.push({
-            currentValue: set.Name.value,
-            parameter: "PropertySet",
+            parameter: "BaseName",
+            currentValue: present ? this.baseName.parameter : null,
             pass: true,
-            requiredValue: this.propertySet,
-          });
-          return true;
+            requiredValue: this.baseName,
+          } as IDSCheck);
+        } else {
+          checks.push(...facetChecks);
+        }
+      }
+    }
+  }
+
+  /**
+   * Checks the property set, name, value and data type of one item, as for a
+   * required facet, and returns whether a property with the required set and
+   * name is present at all.
+   */
+  private async evalFacet(item: FRAGS.ItemData, checks: IDSCheck[]) {
+    const sets = await this.getPsets(item);
+    const matchingSets = sets.filter((set) => {
+      if (!("value" in set.Name)) return false;
+      const result = this.evalRequirement(
+        set.Name.value,
+        this.propertySet,
+        "PropertySet",
+        undefined,
+        "required",
+      );
+      if (!result) return false;
+      checks.push({
+        currentValue: set.Name.value,
+        parameter: "PropertySet",
+        pass: true,
+        requiredValue: this.propertySet,
+      });
+      return true;
+    });
+
+    if (matchingSets.length === 0) {
+      checks.push({
+        currentValue: null,
+        parameter: "PropertySet",
+        pass: false,
+        requiredValue: this.propertySet,
+      });
+      return false;
+    }
+
+    let present = false;
+    for (const set of matchingSets) {
+      const propsListName = this.getPropertyListName(set);
+      if (!propsListName) continue;
+
+      const props = set[propsListName];
+      if (!Array.isArray(props)) {
+        checks.push({
+          currentValue: null,
+          parameter: "BaseName",
+          pass: false,
+          requiredValue: this.baseName,
         });
+        continue;
+      }
 
-        if (matchingSets.length === 0) {
-          checks.push({
-            currentValue: null,
-            parameter: "PropertySet",
-            pass: false,
-            requiredValue: this.propertySet,
-          });
-          continue;
+      const matchingProps = props.filter((prop) => {
+        if (!("value" in prop._category && "value" in prop.Name)) return false;
+        if (this._unsupportedTypes.includes(prop._category.value)) {
+          return false;
         }
 
-        for (const set of matchingSets) {
-          const propsListName = this.getPropertyListName(set);
-          if (!propsListName) continue;
-
-          const props = set[propsListName];
-          if (!Array.isArray(props)) {
-            checks.push({
-              currentValue: null,
-              parameter: "BaseName",
-              pass: false,
-              requiredValue: this.baseName,
-            });
-            continue;
-          }
+        const result = this.evalRequirement(
+          prop.Name.value,
+          this.baseName,
+          "BaseName",
+          undefined,
+          "required",
+        );
+        if (!result) return false;
 
-          const matchingProps = props.filter((item) => {
-            if (!("value" in item._category && "value" in item.Name))
-              return false;
-            if (this._unsupportedTypes.includes(item._category.value)) {
-              return false;
-            }
-
-            const result = this.evalRequirement(
-              item.Name.value,
-              this.baseName,
-              "BaseName",
-            );
-            if (!result) return false;
+        checks.push({
+          currentValue: prop.Name.value,
+          parameter: "BaseName",
+          pass: true,
+          requiredValue: this.baseName,
+        });
+        return true;
+      });
 
-            checks.push({
-              currentValue: item.Name.value,
-              parameter: "BaseName",
-              pass: true,
-              requiredValue: this.baseName,
-            });
-            return true;
-          });
-
-          if (matchingProps.length === 0) {
-            checks.push({
-              currentValue: null,
-              parameter: "BaseName",
-              pass: false,
-              requiredValue: this.baseName,
-            });
-            continue;
-          }
+      if (matchingProps.length === 0) {
+        checks.push({
+          currentValue: null,
+          parameter: "BaseName",
+          pass: false,
+          requiredValue: this.baseName,
+        });
+        continue;
+      }
 
-          for (const item of matchingProps) {
-            this.evalValue(item, checks);
-            this.evalDataType(item, checks);
-            this.evalURI();
-          }
-        }
+      present = true;
+      for (const prop of matchingProps) {
+        this.evalValue(prop, checks);
+        this.evalDataType(prop, checks);
+        this.evalURI();
       }
     }
+
+    return present;
   }
 
   private getPropertyListName(item: FRAGS.ItemData) {
@@ -531,6 +580,7 @@ export class IDSProperty extends IDSFacet {
         facetValue,
         "Value",
         checks,
+        "required",
       );
 
       return result;
@@ -607,6 +657,7 @@ export class IDSProperty extends IDSFacet {
       },
       "DataType",
       checks,
+      "required",
     );
 
     return result;

packages/core/src/openbim/IDSSpecifications/src/facets/property-cardinality.test.ts

@@ -0,0 +1,91 @@
+import * as FRAGS from "@thatopen/fragments";
+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();
+
+const MODEL_ID = "type-psets";
+
+// `tests/fixtures/type-psets.ifc` (see property.test.ts): IsExternal is TRUE
+// on walls 40, 41, 42 and 46, FALSE on 45, and absent on 43.
+const WALLS = [40, 41, 42, 43, 45, 46];
+
+const convert = async () => {
+  const importer = new FRAGS.IfcImporter();
+  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", "type-psets.ifc"),
+  );
+  const bytes = await importer.process({
+    bytes: new Uint8Array(ifc),
+    raw: true,
+  });
+  return new FRAGS.SingleThreadedFragmentsModel(MODEL_ID, bytes, true);
+};
+
+describe("IDSProperty cardinality as a requirement", () => {
+  let components: Components;
+  let model: FRAGS.SingleThreadedFragmentsModel;
+
+  beforeAll(async () => {
+    model = await convert();
+    components = new OBC.Components();
+    const fragments = components.get(OBC.FragmentsManager);
+    vi.spyOn(fragments, "list", "get").mockReturnValue(
+      new Map([[MODEL_ID, model]]) as never,
+    );
+  }, 60_000);
+
+  afterAll(() => {
+    vi.restoreAllMocks();
+    components?.dispose();
+    model?.dispose();
+  });
+
+  const passing = async (
+    cardinality: "required" | "optional" | "prohibited",
+    value?: boolean,
+  ) => {
+    const facet = new OBC.IDSProperty(
+      components,
+      { type: "simple", parameter: "Pset_WallCommon" },
+      { type: "simple", parameter: "IsExternal" },
+    );
+    if (value !== undefined) facet.value = { type: "simple", parameter: value };
+    facet.cardinality = cardinality;
+    const results = new FRAGS.DataMap<
+      string,
+      FRAGS.DataMap<number, { pass: boolean }>
+    >();
+    await facet.test({ [MODEL_ID]: new Set(WALLS) }, results as never, {
+      skipIfFails: false,
+    });
+    return [...results.get(MODEL_ID)!.entries()]
+      .filter(([, result]) => result.pass)
+      .map(([id]) => id)
+      .sort((a, b) => a - b);
+  };
+
+  it("required: passes only where the property has the value", async () => {
+    expect(await passing("required", true)).toEqual([40, 41, 42, 46]);
+  });
+
+  it("prohibited without a value: passes only where the property is absent", async () => {
+    expect(await passing("prohibited")).toEqual([43]);
+  });
+
+  it("optional: never fails, whether the property is absent, matches or differs", async () => {
+    // buildingSMART IDS test cases "an optional facet always passes regardless
+    // of outcome", and the developer guide: an optional requirement that would
+    // fail if required must not be logged as an error.
+    expect(await passing("optional", true)).toEqual(WALLS);
+  });
+});