fix minor bugs in validation logic

Author: Simon Beyzerov

cpp/csp/engine/Struct.cpp

@@ -131,6 +131,18 @@ StructMeta::StructMeta( const std::string & name, const Fields & fields, bool is
         if( !rv.second )
             CSP_THROW( ValueError, "csp Struct " << name << " attempted to add existing field " << m_fields[ idx ] -> fieldname() );
     }
+    
+    // A non-strict struct may not inherit (directly or indirectly) from a strict base
+    bool encountered_non_strict = false;
+    for ( const StructMeta * cur = this; cur; cur = cur -> m_base.get() )
+    {
+        encountered_non_strict |= !cur -> isStrict();
+        if ( encountered_non_strict && cur -> isStrict() )
+            CSP_THROW( ValueError, 
+                    "Strict '" << m_name 
+                    << "' has non-strict inheritance of strict base '" 
+                    << cur -> name() << "'" );
+    }
 }
 
 StructMeta::~StructMeta()
@@ -498,40 +510,29 @@ void StructMeta::destroy( Struct * s ) const
 }
 
 void StructMeta::validate( const Struct * s ) const
-{
-    bool encountered_non_strict = false;
-    
+{    
     for ( const StructMeta * cur = this; cur; cur = cur -> m_base.get() )
     {
-        encountered_non_strict |= !cur -> isStrict();
         if ( !cur -> isStrict() )
             continue;
-        
-        // Rule 1: A non-strict struct may not inherit (directly or indirectly) from a strict base
-        if ( encountered_non_strict )
-            CSP_THROW( ValueError, 
-                    "Struct '" << s -> meta() -> name() 
-                    << "' has non-strict inheritance of strict base '" 
-                    << cur -> name() << "'" );
 
-        // Rule 2: All local fields are set
-        std::string missing_fields;
-        for ( const auto & field : cur -> m_fields )
+        if ( !cur -> allFieldsSet( s ) )
         {
-            if ( !field -> isSet( s ) )
+            std::string missing_fields;
+            for ( const auto & field : cur -> m_fields )
             {
-                if ( missing_fields.empty() )
-                    missing_fields = field -> fieldname();
-                else
-                    missing_fields += ", " + field -> fieldname();
+                if ( !field -> isSet( s ) )
+                {
+                    if ( missing_fields.empty() )
+                        missing_fields = field -> fieldname();
+                    else
+                        missing_fields += ", " + field -> fieldname();
+                }
             }
-        }
-        
-        // Raise error if any fields are missing
-        if ( !missing_fields.empty() )
             CSP_THROW( ValueError, 
-                    "Strict struct '" << cur -> name() 
+                    "Strict struct '" << s -> meta() -> name() 
                     << "' missing required fields: " << missing_fields );
+        }
     }
 }
 

csp/impl/wiring/edge.py

@@ -202,9 +202,10 @@ def __getattr__(self, key):
             elemtype = typ.metadata(typed=True).get(key)
             if elemtype is None:
                 raise AttributeError("'%s' object has no attribute '%s'" % (self.tstype.typ.__name__, key))
-            if key in typ.optional_fields():
+            if (key in typ.optional_fields()) and (typ.is_strict()):
                 raise AttributeError(
-                    "Cannot access optional field '%s' on object '%s' at graph time" % (key, self.tstype.typ.__name__)
+                    "Cannot access optional field '%s' on strict struct object '%s' at graph time"
+                    % (key, self.tstype.typ.__name__)
                 )
             return csp.struct_field(self, key, elemtype)
 

csp/tests/test_strict_structs.py

@@ -142,7 +142,7 @@ def g_fail():
 
         with self.assertRaisesRegex(
             AttributeError,
-            "Cannot access optional field 'opt_str' on object 'MyStrictStruct' at graph time",
+            "Cannot access optional field 'opt_str' on strict struct object 'MyStrictStruct' at graph time",
         ):
             csp.run(g_fail, starttime=datetime(2023, 1, 1))
 
@@ -264,23 +264,10 @@ def test_nonstrict_cannot_inherit_strict(self):
         class StrictBase(csp.Struct, allow_unset=False):
             base_val: int
 
-        class NonStrictChild1(StrictBase, allow_unset=True):
-            child_val1: Optional[int] = None
-
-        class NonStrictChild2(NonStrictChild1, allow_unset=True):
-            child_val2: Optional[int] = None
-
-        class NonStrictChild3(NonStrictChild2, allow_unset=True):
-            child_val3: Optional[int] = None
-
-        with self.assertRaisesRegex(ValueError, "non-strict inheritance of strict base"):
-            NonStrictChild1(base_val=1, child_val1=2)
-
         with self.assertRaisesRegex(ValueError, "non-strict inheritance of strict base"):
-            NonStrictChild2(base_val=3, child_val1=4, child_val2=5)
 
-        with self.assertRaisesRegex(ValueError, "non-strict inheritance of strict base"):
-            NonStrictChild3(base_val=6, child_val1=7, child_val2=8, child_val3=9)
+            class NonStrictChild1(StrictBase, allow_unset=True):
+                child_val1: Optional[int] = None
 
     def test_nonstrict_strict_nonstrict_inheritance_order(self):
         """inheritance order NonStrict -> Strict -> NonStrict raises an error"""
@@ -291,11 +278,10 @@ class NonStrictBase(csp.Struct, allow_unset=True):
         class StrictMiddle(NonStrictBase, allow_unset=False):
             middle_val: int
 
-        class NonStrictChild(StrictMiddle, allow_unset=True):
-            child_val: Optional[int] = None
-
         with self.assertRaisesRegex(ValueError, "non-strict inheritance of strict base"):
-            NonStrictChild(base_val=1, middle_val=2, child_val=3)
+
+            class NonStrictChild(StrictMiddle, allow_unset=True):
+                child_val: Optional[int] = None
 
     def test_nested_struct_serialization(self):
         """to_dict / from_dict work with nested strict & non-strict structs"""
@@ -344,7 +330,7 @@ def main_graph():
             csp.print("", res)
 
         with self.assertRaisesRegex(
-            AttributeError, "Cannot access optional field 'is_active' on object 'Test' at graph time"
+            AttributeError, "Cannot access optional field 'is_active' on strict struct object 'Test' at graph time"
         ):
             csp.build_graph(main_graph)