From f0bb19d48def42e6b48fba8ef240b86382cf9481 Mon Sep 17 00:00:00 2001 From: Thomas Krijnen Date: Thu, 13 Apr 2023 13:16:48 +0200 Subject: [PATCH] Fixes to validation of selected simple type / enum --- .../ifcopenshell/validate.py | 24 ++++++++++++------- .../fail-invalid-selected-enumeration.ifc | 10 ++++++++ .../fail-invalid-selected-simple-type.ifc | 10 ++++++++ .../pass-valid-selected-enumeration.ifc | 10 ++++++++ .../test/{validate.py => test_validate.py} | 6 ++++- 5 files changed, 51 insertions(+), 9 deletions(-) create mode 100644 src/ifcopenshell-python/test/fixtures/validate/fail-invalid-selected-enumeration.ifc create mode 100644 src/ifcopenshell-python/test/fixtures/validate/fail-invalid-selected-simple-type.ifc create mode 100644 src/ifcopenshell-python/test/fixtures/validate/pass-valid-selected-enumeration.ifc rename src/ifcopenshell-python/test/{validate.py => test_validate.py} (88%) diff --git a/src/ifcopenshell-python/ifcopenshell/validate.py b/src/ifcopenshell-python/ifcopenshell/validate.py index 4a83f6191a..dc2e67d4db 100644 --- a/src/ifcopenshell-python/ifcopenshell/validate.py +++ b/src/ifcopenshell-python/ifcopenshell/validate.py @@ -154,7 +154,13 @@ def get_select_members(schema, ty): for st in ty.subtypes(): yield from inner(st) elif isinstance(ty, type_declaration): + # @todo shouldn't we list subtypes (e.g IfcPositiveLengthMeasure -> IfcLengthMeasure) here as well? yield ty.name() + elif isinstance(ty, enumeration_type): + yield ty.name() + else: + # @todo raise exception? + pass v = select_members_cache[cache_key] = set(inner(ty)) return v @@ -182,20 +188,22 @@ def assert_valid(attr_type, val, schema, no_throw=False, attr=None): elif isinstance(attr_type, (entity_type, type_declaration)): invalid = not isinstance(val, ifcopenshell.entity_instance) or not val.is_a(attr_type.name()) elif isinstance(attr_type, select_type): - val_to_use = val - if isinstance(schema.declaration_by_name(val.is_a()), enumeration_type): - if isinstance(val, ifcopenshell.entity_instance): - val_to_use = val.wrappedValue - else: - invalid = True - if not invalid: + if not isinstance(val, ifcopenshell.entity_instance): + invalid = True + else: + value_type = schema.declaration_by_name(val.is_a()) + if not isinstance(value_type, entity_type): + # we need to check two things: is (enumeration) literal/value valid + # for this type and is enumeration/value type valid for this select. + assert_valid(value_type, val.wrappedValue, schema, no_throw=no_throw) + # Previously we relied on `is_a(x) for x in attr_type.select_items()` # this was linear in the number of select leafs, which is very large # for e.g IfcValue, which is an often used select. Therefore, we now # calculate (and cache) the select leafs (including entity subtypes) # for the select definition and simply check for membership in this # set. - invalid = val_to_use.is_a() not in get_select_members(schema, attr_type) + invalid = val.is_a() not in get_select_members(schema, attr_type) elif isinstance(attr_type, enumeration_type): invalid = val not in attr_type.enumeration_items() elif isinstance(attr_type, aggregation_type): diff --git a/src/ifcopenshell-python/test/fixtures/validate/fail-invalid-selected-enumeration.ifc b/src/ifcopenshell-python/test/fixtures/validate/fail-invalid-selected-enumeration.ifc new file mode 100644 index 0000000000..b6366b8002 --- /dev/null +++ b/src/ifcopenshell-python/test/fixtures/validate/fail-invalid-selected-enumeration.ifc @@ -0,0 +1,10 @@ +ISO-10303-21; +HEADER; +FILE_DESCRIPTION(('ViewDefinition [CoordinationView]'),'2;1'); +FILE_NAME('','2023-04-13T10:27:43',(),(),'IfcOpenShell v0.7.0-198fa67cc','IfcOpenShell v0.7.0-198fa67cc',''); +FILE_SCHEMA(('IFC4X3_RC4')); +ENDSEC; +DATA; +#1=IFCFACILITYPART('0DYKeUG9993PtW$2icoR4v',$,$,$,$,$,$,$,$,IFCBRIDGEPARTTYPEENUM('NOT_EXISTING_ENUM'),.LATERAL.); +ENDSEC; +END-ISO-10303-21; diff --git a/src/ifcopenshell-python/test/fixtures/validate/fail-invalid-selected-simple-type.ifc b/src/ifcopenshell-python/test/fixtures/validate/fail-invalid-selected-simple-type.ifc new file mode 100644 index 0000000000..0dba7e4479 --- /dev/null +++ b/src/ifcopenshell-python/test/fixtures/validate/fail-invalid-selected-simple-type.ifc @@ -0,0 +1,10 @@ +ISO-10303-21; +HEADER; +FILE_DESCRIPTION(('ViewDefinition [CoordinationView]'),'2;1'); +FILE_NAME('','2023-04-13T10:24:46',(),(),'IfcOpenShell v0.7.0-198fa67cc','IfcOpenShell v0.7.0-198fa67cc',''); +FILE_SCHEMA(('IFC4')); +ENDSEC; +DATA; +#1=IFCPROPERTYSINGLEVALUE('x',$,IFCPOSITIVELENGTHMEASURE('1'),$); +ENDSEC; +END-ISO-10303-21; diff --git a/src/ifcopenshell-python/test/fixtures/validate/pass-valid-selected-enumeration.ifc b/src/ifcopenshell-python/test/fixtures/validate/pass-valid-selected-enumeration.ifc new file mode 100644 index 0000000000..805b175784 --- /dev/null +++ b/src/ifcopenshell-python/test/fixtures/validate/pass-valid-selected-enumeration.ifc @@ -0,0 +1,10 @@ +ISO-10303-21; +HEADER; +FILE_DESCRIPTION(('ViewDefinition [CoordinationView]'),'2;1'); +FILE_NAME('','2023-04-13T10:27:43',(),(),'IfcOpenShell v0.7.0-198fa67cc','IfcOpenShell v0.7.0-198fa67cc',''); +FILE_SCHEMA(('IFC4X3_RC4')); +ENDSEC; +DATA; +#1=IFCFACILITYPART('0DYKeUG9993PtW$2icoR4v',$,$,$,$,$,$,$,$,IFCBRIDGEPARTTYPEENUM('DECK'),.LATERAL.); +ENDSEC; +END-ISO-10303-21; diff --git a/src/ifcopenshell-python/test/validate.py b/src/ifcopenshell-python/test/test_validate.py similarity index 88% rename from src/ifcopenshell-python/test/validate.py rename to src/ifcopenshell-python/test/test_validate.py index 5ad4a83f45..16b12f223d 100644 --- a/src/ifcopenshell-python/test/validate.py +++ b/src/ifcopenshell-python/test/test_validate.py @@ -30,7 +30,11 @@ import ifcopenshell.validate ) def test_file(file): logger = ifcopenshell.validate.json_logger() - ifcopenshell.validate.validate(file, logger) + try: + ifcopenshell.validate.validate(file, logger) + except ifcopenshell.SchemaError as e: + pytest.skip() + file = os.path.basename(file) if file.startswith("fail-"): assert len(logger.statements) > 0 if file.startswith("pass-"):