From 0065c5fba89c0186469f655371249a4eb8568e43 Mon Sep 17 00:00:00 2001
From: Dion Moult <dion@thinkmoult.com>
Date: Sat, 30 Mar 2024 00:13:02 +1100
Subject: [PATCH] Fix incorrect handling of prohibited specifications

---
 src/ifctester/ifctester/ids.py | 38 +++++++++++++++++-----------------
 src/ifctester/test/test_ids.py | 13 +++++++-----
 2 files changed, 27 insertions(+), 24 deletions(-)

diff --git a/src/ifctester/ifctester/ids.py b/src/ifctester/ifctester/ids.py
index b7bc0c9cf6..e92d6fdcbc 100644
--- a/src/ifctester/ifctester/ids.py
+++ b/src/ifctester/ifctester/ids.py
@@ -259,31 +259,31 @@ class Specification:
             self.applicable_entities.append(element)
             for facet in self.requirements:
                 result = facet(element)
-                if not bool(result):
-                    self.failed_entities.add(element)
-                    facet.failures.append({"element": element, "reason": str(result)})
-
-        for facet in self.requirements:
-            if facet.cardinality == "required":
-                facet.status = not bool(facet.failures)
-            elif facet.cardinality == "optional":
-                facet.status = True
-            elif facet.cardinality == "prohibited":
-                facet.status = bool(facet.failures)
+                is_pass = bool(result)
+                if facet.cardinality == "prohibited":
+                    is_pass = not is_pass
+                if self.maxOccurs != 0: # This is a required or optional specification
+                    if not is_pass:
+                        self.failed_entities.add(element)
+                        facet.failures.append({"element": element, "reason": str(result)})
+                else: # This is a prohibited specification
+                    if is_pass:
+                        self.failed_entities.add(element)
+                        facet.failures.append({"element": element, "reason": str(result)})
 
         self.status = True
-        if self.minOccurs != 0:
+        for facet in self.requirements:
+            facet.status = not bool(facet.failures)
+            if not facet.status:
+                self.status = False
+
+        if self.minOccurs != 0: # Required specification
             if not self.applicable_entities:
                 self.status = False
                 for facet in self.requirements:
                     facet.status = False
-            elif self.failed_entities:
-                self.status = False
-        elif self.minOccurs == 0 and self.maxOccurs != 0:
-            if self.failed_entities:
-                self.status = False
-        elif self.maxOccurs == 0:
-            if (len(self.applicable_entities)) > 0:
+        elif self.maxOccurs == 0: # Prohibited specification
+            if self.applicable_entities and not self.requirements:
                 self.status = False
 
     def get_usage(self):
diff --git a/src/ifctester/test/test_ids.py b/src/ifctester/test/test_ids.py
index 569e974deb..78e26b4512 100644
--- a/src/ifctester/test/test_ids.py
+++ b/src/ifctester/test/test_ids.py
@@ -36,7 +36,7 @@ def run(name, ids, ifc, expected, applicable_entities=None, failed_entities=None
         all_applicable.update(spec.applicable_entities)
         for requirement in spec.requirements:
             if requirement.status is False:
-                all_failures.update(requirement.failed_entities)
+                all_failures.update([f["element"] for f in requirement.failures])
     assert set(all_applicable) == set(applicable_entities)
     assert set(all_failures) == set(failed_entities)
 
@@ -162,9 +162,9 @@ class TestIds:
             "Prohibited specifications fail if at least one entity passes all requirements 2/3",
             specs,
             model,
-            False,
-            [wall],
+            True,
             [wall],
+            [],
         )
         model = ifcopenshell.file()
         wall = model.createIfcWall(Name="Waldo")
@@ -174,6 +174,7 @@ class TestIds:
             model,
             False,
             [wall],
+            [wall],
         )
 
         spec.minOccurs = 0
@@ -238,8 +239,10 @@ class TestIds:
 
         assert spec.status == False
         assert set(spec.applicable_entities) == {wall, waldo}
-        assert spec.requirements[0].failed_entities == [wall]
-        assert spec2.requirements[0].failed_entities == [wall]
+        assert len(spec.requirements[0].failures) == 1
+        assert len(spec2.requirements[0].failures) == 1
+        assert spec.requirements[0].failures[0]["element"] == wall
+        assert spec2.requirements[0].failures[0]["element"] == wall
 
 
 class TestSpecification: