From 6884894f87ab8cb99b5e147caccc9ee93ba1def1 Mon Sep 17 00:00:00 2001 From: Simon Beyzerov Date: Thu, 31 Jul 2025 14:40:20 -0400 Subject: [PATCH 1/9] Add unset field semantics (disc #536) Signed-off-by: Simon Beyzerov --- cpp/csp/adapters/kafka/KafkaInputAdapter.cpp | 4 + .../parquet/ParquetReaderColumnAdapter.cpp | 3 + .../utils/JSONMessageStructConverter.cpp | 4 + .../websocket/ClientAdapterManager.cpp | 11 +- .../adapters/websocket/ClientInputAdapter.cpp | 2 + cpp/csp/cppnodes/baselibimpl.cpp | 6 + cpp/csp/engine/BasketInfo.cpp | 4 + cpp/csp/engine/Struct.cpp | 36 +- cpp/csp/engine/Struct.h | 13 +- cpp/csp/python/PyStruct.cpp | 30 +- cpp/csp/python/PyStruct.h | 2 +- csp/impl/struct.py | 22 +- csp/impl/types/typing_utils.py | 7 + csp/impl/wiring/edge.py | 5 + csp/tests/test_strict_structs.py | 359 ++++++++++++++++++ 15 files changed, 492 insertions(+), 16 deletions(-) create mode 100644 csp/tests/test_strict_structs.py diff --git a/cpp/csp/adapters/kafka/KafkaInputAdapter.cpp b/cpp/csp/adapters/kafka/KafkaInputAdapter.cpp index 296df8daa..bbef00584 100644 --- a/cpp/csp/adapters/kafka/KafkaInputAdapter.cpp +++ b/cpp/csp/adapters/kafka/KafkaInputAdapter.cpp @@ -113,6 +113,10 @@ void KafkaInputAdapter::processMessage( RdKafka::Message* message, bool live, cs if( m_tickTimestampField ) msgTime = m_tickTimestampField->value(tick.get()); + if (!tick.get() -> validate()) + CSP_THROW( ValueError, "Struct validation failed for Kafka message, fields missing" ); + + bool pushLive = shouldPushLive(live, msgTime); if( shouldProcessMessage( pushLive, msgTime ) ) pushTick(pushLive, msgTime, std::move(tick), batch); diff --git a/cpp/csp/adapters/parquet/ParquetReaderColumnAdapter.cpp b/cpp/csp/adapters/parquet/ParquetReaderColumnAdapter.cpp index b8380ce53..f95ff6704 100644 --- a/cpp/csp/adapters/parquet/ParquetReaderColumnAdapter.cpp +++ b/cpp/csp/adapters/parquet/ParquetReaderColumnAdapter.cpp @@ -520,6 +520,9 @@ void ParquetStructAdapter::dispatchValue( const utils::Symbol *symbol, bool isNu { fieldSetter( s ); } + + CSP_TRUE_OR_THROW_RUNTIME( s -> validate(), "Struct validation failed for Parquet message, some fields are missing" ); + dispatchedValue = &s; } diff --git a/cpp/csp/adapters/utils/JSONMessageStructConverter.cpp b/cpp/csp/adapters/utils/JSONMessageStructConverter.cpp index 574f21084..585b3ee6d 100644 --- a/cpp/csp/adapters/utils/JSONMessageStructConverter.cpp +++ b/cpp/csp/adapters/utils/JSONMessageStructConverter.cpp @@ -145,6 +145,9 @@ StructPtr JSONMessageStructConverter::convertJSON( const char * fieldname, const } ); } + if( !struct_ -> validate() ) + CSP_THROW( ValueError, "JSON conversion of struct " << sType.meta() -> name() << " failed; some required fields were not set" ); + return struct_; } @@ -251,6 +254,7 @@ csp::StructPtr JSONMessageStructConverter::asStruct( void * bytes, size_t size ) } ); } + // root struct validation (validate()) deferred to adapter level return data; } diff --git a/cpp/csp/adapters/websocket/ClientAdapterManager.cpp b/cpp/csp/adapters/websocket/ClientAdapterManager.cpp index 423f2a234..e472357d8 100644 --- a/cpp/csp/adapters/websocket/ClientAdapterManager.cpp +++ b/cpp/csp/adapters/websocket/ClientAdapterManager.cpp @@ -52,8 +52,15 @@ void ClientAdapterManager::start( DateTime starttime, DateTime endtime ) if( m_inputAdapter ) { m_endpoint -> setOnMessage( [ this ]( void* c, size_t t ) { - PushBatch batch( m_engine -> rootEngine() ); - m_inputAdapter -> processMessage( c, t, &batch ); + try + { + PushBatch batch( m_engine -> rootEngine() ); + m_inputAdapter -> processMessage( c, t, &batch ); + } + catch( csp::Exception & err ) + { + pushStatus( StatusLevel::ERROR, ClientStatusType::GENERIC_ERROR, err.what() ); + } } ); } else { diff --git a/cpp/csp/adapters/websocket/ClientInputAdapter.cpp b/cpp/csp/adapters/websocket/ClientInputAdapter.cpp index e4b0b7ff7..b727341ed 100644 --- a/cpp/csp/adapters/websocket/ClientInputAdapter.cpp +++ b/cpp/csp/adapters/websocket/ClientInputAdapter.cpp @@ -31,6 +31,8 @@ void ClientInputAdapter::processMessage( void* c, size_t t, PushBatch* batch ) if( dataType() -> type() == CspType::Type::STRUCT ) { auto tick = m_converter -> asStruct( c, t ); + if (!tick.get() -> validate()) + CSP_THROW( ValueError, "Struct validation failed for WebSocket message, fields missing" ); pushTick( std::move(tick), batch ); } else if ( dataType() -> type() == CspType::Type::STRING ) { diff --git a/cpp/csp/cppnodes/baselibimpl.cpp b/cpp/csp/cppnodes/baselibimpl.cpp index 52a5537d9..5530bb446 100644 --- a/cpp/csp/cppnodes/baselibimpl.cpp +++ b/cpp/csp/cppnodes/baselibimpl.cpp @@ -705,6 +705,9 @@ DECLARE_CPPNODE( struct_fromts ) ); } + if( !out.get() -> validate( ) ) + CSP_THROW( ValueError, "Struct " << cls.value() -> name() << " is not valid; some required fields did not tick" ); + CSP_OUTPUT( std::move( out ) ); } @@ -758,6 +761,9 @@ DECLARE_CPPNODE( struct_collectts ) } ); } + + if( !out.get() -> validate( ) ) + CSP_THROW( ValueError, "Struct " << cls.value() -> name() << " is not valid; some required fields did not tick" ); CSP_OUTPUT( std::move( out ) ); } diff --git a/cpp/csp/engine/BasketInfo.cpp b/cpp/csp/engine/BasketInfo.cpp index f43e425e9..327ecec66 100644 --- a/cpp/csp/engine/BasketInfo.cpp +++ b/cpp/csp/engine/BasketInfo.cpp @@ -161,6 +161,8 @@ void DynamicOutputBasketInfo::addShapeChange( const DialectGenericType & key, bo { auto events = autogen::DynamicBasketEvents::create(); events -> set_events( {} ); + if( !events -> validate() ) + CSP_THROW( ValueError, "DynamicBasketEvents struct is not valid; some required fields were not set" ); m_shapeTs.outputTickTyped( m_parentNode -> rootEngine() -> cycleCount(), m_parentNode -> rootEngine() -> now(), events, false ); @@ -171,6 +173,8 @@ void DynamicOutputBasketInfo::addShapeChange( const DialectGenericType & key, bo auto event = autogen::DynamicBasketEvent::create(); event -> set_key( key ); event -> set_added( added ); + if( !event -> validate() ) + CSP_THROW( ValueError, "DynamicBasketEvent struct is not valid; some required fields were not set" ); const_cast &>( events ).emplace_back( event ); } diff --git a/cpp/csp/engine/Struct.cpp b/cpp/csp/engine/Struct.cpp index 42830357e..3b501036b 100644 --- a/cpp/csp/engine/Struct.cpp +++ b/cpp/csp/engine/Struct.cpp @@ -1,6 +1,8 @@ #include #include #include +#include +#include namespace csp { @@ -33,8 +35,8 @@ and adjustments required for the hidden fields */ -StructMeta::StructMeta( const std::string & name, const Fields & fields, - std::shared_ptr base ) : m_name( name ), m_base( base ), m_fields( fields ), +StructMeta::StructMeta( const std::string & name, const Fields & fields, bool isStrict, + std::shared_ptr base ) : m_name( name ), m_base( base ), m_isStrict( isStrict ), m_fields( fields ), m_size( 0 ), m_partialSize( 0 ), m_partialStart( 0 ), m_nativeStart( 0 ), m_basePadding( 0 ), m_maskLoc( 0 ), m_maskSize( 0 ), m_firstPartialField( 0 ), m_firstNativePartialField( 0 ), m_isPartialNative( true ), m_isFullyNative( true ) @@ -128,6 +130,18 @@ StructMeta::StructMeta( const std::string & name, const Fields & fields, 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() @@ -494,6 +508,24 @@ void StructMeta::destroy( Struct * s ) const m_base -> destroy( s ); } +[[nodiscard]] bool StructMeta::validate( const Struct * s ) const +{ + for ( const StructMeta * cur = this; cur; cur = cur -> m_base.get() ) + { + if ( !cur -> isStrict() ) + continue; + + // Note that we do not recursively validate nested struct. + // We assume after any creation on the C++ side, these structs + // are validated properly prior to being set as field values + if ( !cur -> allFieldsSet( s ) ) + return false; + } + return true; +} + + + Struct::Struct( const std::shared_ptr & meta ) { //Initialize meta shared_ptr diff --git a/cpp/csp/engine/Struct.h b/cpp/csp/engine/Struct.h index 64b51ecae..f3b0b63bf 100644 --- a/cpp/csp/engine/Struct.h +++ b/cpp/csp/engine/Struct.h @@ -587,7 +587,7 @@ class StructMeta : public std::enable_shared_from_this using FieldNames = std::vector; //Fields will be re-arranged and assigned their offsets in StructMeta for optimal performance - StructMeta( const std::string & name, const Fields & fields, std::shared_ptr base = nullptr ); + StructMeta( const std::string & name, const Fields & fields, bool isStrict, std::shared_ptr base = nullptr ); virtual ~StructMeta(); const std::string & name() const { return m_name; } @@ -595,6 +595,7 @@ class StructMeta : public std::enable_shared_from_this size_t partialSize() const { return m_partialSize; } bool isNative() const { return m_isFullyNative; } + bool isStrict() const { return m_isStrict; } const Fields & fields() const { return m_fields; } const FieldNames & fieldNames() const { return m_fieldnames; } @@ -602,6 +603,8 @@ class StructMeta : public std::enable_shared_from_this size_t maskLoc() const { return m_maskLoc; } size_t maskSize() const { return m_maskSize; } + [[nodiscard]] bool validate( const Struct * s ) const; + const StructFieldPtr & field( const char * name ) const { static StructFieldPtr s_empty; @@ -652,7 +655,8 @@ class StructMeta : public std::enable_shared_from_this std::shared_ptr m_base; StructPtr m_default; FieldMap m_fieldMap; - + bool m_isStrict; + //fields in order, memory owners of field objects which in turn own the key memory //m_fields includes all base fields as well. m_fieldnames maintains the proper iteration order of fields Fields m_fields; @@ -738,6 +742,11 @@ class Struct return meta() -> allFieldsSet( this ); } + [[nodiscard]] bool validate() const + { + return meta() -> validate( this ); + } + //used to cache dialect representations of this struct, if needed void * dialectPtr() const { return hidden() -> dialectPtr; } diff --git a/cpp/csp/python/PyStruct.cpp b/cpp/csp/python/PyStruct.cpp index 037e3f63a..9fedd486d 100644 --- a/cpp/csp/python/PyStruct.cpp +++ b/cpp/csp/python/PyStruct.cpp @@ -20,8 +20,8 @@ class PyObjectStructField final : public DialectGenericStructField public: using BASE = DialectGenericStructField; PyObjectStructField( const std::string & name, - PyTypeObjectPtr pytype ) : DialectGenericStructField( name, sizeof( PyObjectPtr ), alignof( PyObjectPtr ) ), - m_pytype( pytype ) + PyTypeObjectPtr pytype ) : BASE( name, sizeof( PyObjectPtr ), alignof( PyObjectPtr ) ), + m_pytype( pytype ) {} @@ -42,8 +42,8 @@ class PyObjectStructField final : public DialectGenericStructField }; DialectStructMeta::DialectStructMeta( PyTypeObject * pyType, const std::string & name, - const Fields & flds, std::shared_ptr base ) : - StructMeta( name, flds, base ), + const Fields & flds, bool isStrict, std::shared_ptr base ) : + StructMeta( name, flds, isStrict, base ), m_pyType( pyType ) { } @@ -110,12 +110,19 @@ static PyObject * PyStructMeta_new( PyTypeObject *subtype, PyObject *args, PyObj { PyObject *key, *type; Py_ssize_t pos = 0; + PyObject *optional_fields = PyDict_GetItemString( dict, "__optional_fields__" ); + + while( PyDict_Next( metadata, &pos, &key, &type ) ) { const char * keystr = PyUnicode_AsUTF8( key ); if( !keystr ) CSP_THROW( PythonPassthrough, "" ); + if (!PySet_Check(optional_fields)) + CSP_THROW( TypeError, "Struct metadata for key " << keystr << " expected a set, got " << PyObjectPtr::incref( optional_fields ) ); + + if( !PyType_Check( type ) && !PyList_Check( type ) ) CSP_THROW( TypeError, "Struct metadata for key " << keystr << " expected a type, got " << PyObjectPtr::incref( type ) ); @@ -151,7 +158,7 @@ static PyObject * PyStructMeta_new( PyTypeObject *subtype, PyObject *args, PyObj default: CSP_THROW( ValueError, "Unexpected csp type " << csptype -> type() << " on struct " << name ); } - + fields.emplace_back( field ); } } @@ -188,7 +195,12 @@ static PyObject * PyStructMeta_new( PyTypeObject *subtype, PyObject *args, PyObj | | PyStruct -------------------------- */ - auto structMeta = std::make_shared( ( PyTypeObject * ) pymeta, name, fields, metabase ); + + PyObject * strict_enabled = PyDict_GetItemString( dict, "__strict_enabled__" ); + if( !strict_enabled ) + CSP_THROW( KeyError, "StructMeta missing __strict_enabled__" ); + bool isStrict = strict_enabled == Py_True; + auto structMeta = std::make_shared( ( PyTypeObject * ) pymeta, name, fields, isStrict, metabase ); //Setup fast attr dict lookup pymeta -> attrDict = PyObjectPtr::own( PyDict_New() ); @@ -347,6 +359,7 @@ static PyObject * PyStructMeta_metadata_info( PyStructMeta * m ) return out.release(); } + static PyMethodDef PyStructMeta_methods[] = { {"_layout", (PyCFunction) PyStructMeta_layout, METH_NOARGS, "debug view of structs internal mem layout"}, {"_metadata_info", (PyCFunction) PyStructMeta_metadata_info, METH_NOARGS, "provide detailed information about struct layout"}, @@ -456,6 +469,9 @@ void PyStruct::setattr( Struct * s, PyObject * attr, PyObject * value ) if( !field ) CSP_THROW( AttributeError, "'" << s -> meta() -> name() << "' object has no attribute '" << PyUnicode_AsUTF8( attr ) << "'" ); + if ( s -> meta() -> isStrict() && value == nullptr ) + CSP_THROW( AttributeError, "Strict struct " << s -> meta() -> name() << " does not allow the deletion of field " << PyUnicode_AsUTF8( attr ) ); + try { switchCspType( field -> type(), [field,&struct_=s,value]( auto tag ) @@ -795,6 +811,8 @@ int PyStruct_init( PyStruct * self, PyObject * args, PyObject * kwargs ) CSP_BEGIN_METHOD; PyStruct_setattrs( self, args, kwargs, "__init__" ); + if( !self -> struct_ -> validate() ) + CSP_THROW( ValueError, "Struct " << self -> struct_ -> meta() -> name() << " is not valid; some required fields were not set on init" ); CSP_RETURN_INT; } diff --git a/cpp/csp/python/PyStruct.h b/cpp/csp/python/PyStruct.h index 2034268b8..2c794cdd9 100644 --- a/cpp/csp/python/PyStruct.h +++ b/cpp/csp/python/PyStruct.h @@ -25,7 +25,7 @@ class CSPTYPESIMPL_EXPORT DialectStructMeta : public StructMeta { public: DialectStructMeta( PyTypeObject * pyType, const std::string & name, - const Fields & fields, std::shared_ptr base = nullptr ); + const Fields & fields, bool isStrict, std::shared_ptr base = nullptr ); ~DialectStructMeta() {} PyTypeObject * pyType() const { return m_pyType; } diff --git a/csp/impl/struct.py b/csp/impl/struct.py index 30b201285..f1580434a 100644 --- a/csp/impl/struct.py +++ b/csp/impl/struct.py @@ -15,7 +15,7 @@ class StructMeta(_csptypesimpl.PyStructMeta): - def __new__(cls, name, bases, dct): + def __new__(cls, name, bases, dct, allow_unset=True): full_metadata = {} full_metadata_typed = {} metadata = {} @@ -29,12 +29,17 @@ def __new__(cls, name, bases, dct): defaults.update(base.__defaults__) annotations = dct.get("__annotations__", None) + optional_fields = set() if annotations: for k, v in annotations.items(): actual_type = v # Lists need to be normalized too as potentially we need to add a boolean flag to use FastList if v == FastList: raise TypeError(f"{v} annotation is not supported without args") + if CspTypingUtils.is_optional_type(v): + if (not allow_unset) and (k not in dct): + raise TypeError(f"Optional field {k} must have a default value") + optional_fields.add(k) if ( CspTypingUtils.is_generic_container(v) or CspTypingUtils.is_union_type(v) @@ -72,6 +77,8 @@ def __new__(cls, name, bases, dct): dct["__full_metadata_typed__"] = full_metadata_typed dct["__metadata__"] = metadata dct["__defaults__"] = defaults + dct["__optional_fields__"] = optional_fields + dct["__strict_enabled__"] = not allow_unset res = super().__new__(cls, name, bases, dct) # This is how we make sure we construct the pydantic schema from the new class @@ -174,6 +181,14 @@ def metadata(cls, typed=False): else: return cls.__full_metadata__ + @classmethod + def optional_fields(cls): + return cls.__optional_fields__ + + @classmethod + def is_strict(cls): + return cls.__strict_enabled__ + @classmethod def fromts(cls, trigger=None, /, **kwargs): """convert valid inputs into ts[ struct ] @@ -237,12 +252,13 @@ def _obj_from_python(cls, json, obj_type): elif issubclass(obj_type, Struct): if not isinstance(json, dict): raise TypeError("Representation of struct as json is expected to be of dict type") - res = obj_type() + obj_args = {} for k, v in json.items(): expected_type = obj_type.__full_metadata_typed__.get(k, None) if expected_type is None: raise KeyError(f"Unexpected key {k} for type {obj_type}") - setattr(res, k, cls._obj_from_python(v, expected_type)) + obj_args[k] = cls._obj_from_python(v, expected_type) + res = obj_type(**obj_args) return res else: if isinstance(json, obj_type): diff --git a/csp/impl/types/typing_utils.py b/csp/impl/types/typing_utils.py index f852168d2..3511888b2 100644 --- a/csp/impl/types/typing_utils.py +++ b/csp/impl/types/typing_utils.py @@ -83,6 +83,13 @@ def is_numpy_nd_array_type(cls, typ): def is_union_type(cls, typ): return isinstance(typ, typing._GenericAlias) and typ.__origin__ is typing.Union + @classmethod + def is_optional_type(cls, typ): + if cls.is_union_type(typ): + args = typing.get_args(typ) + return type(None) in args + return False + @classmethod def is_literal_type(cls, typ): return isinstance(typ, typing._GenericAlias) and typ.__origin__ is typing.Literal diff --git a/csp/impl/wiring/edge.py b/csp/impl/wiring/edge.py index 072b0a424..8ebf460e7 100644 --- a/csp/impl/wiring/edge.py +++ b/csp/impl/wiring/edge.py @@ -202,6 +202,11 @@ 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()) and (typ.is_strict()): + raise AttributeError( + "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) raise AttributeError("'Edge' object has no attribute '%s'" % (key)) diff --git a/csp/tests/test_strict_structs.py b/csp/tests/test_strict_structs.py new file mode 100644 index 000000000..817ec7d2b --- /dev/null +++ b/csp/tests/test_strict_structs.py @@ -0,0 +1,359 @@ +import unittest +from datetime import datetime, timedelta +from typing import Optional + +import csp +from csp import ts +from csp.impl.wiring.base_parser import CspParseError + + +class TestStrictStructs(unittest.TestCase): + def test_backwards_compatibility(self): + """quick test that existing struct behavior is unchanged""" + + class OldStruct(csp.Struct, allow_unset=True): + a: int + b: str + + s = OldStruct(a=5) + self.assertFalse(hasattr(s, "b")) + with self.assertRaisesRegex(AttributeError, "b"): + _ = s.b + del s.a + self.assertFalse(hasattr(s, "a")) + + def test_strict_struct_initialization(self): + """test initialization rules for strict structs. + + notably, + * Setting fields works as expected + * Initialize all non-default fields (including Optional) + * Missing required fields fail + """ + + class MyStrictStruct(csp.Struct, allow_unset=False): + req_int: int + opt_str: Optional[str] = None + def_int: int = 123 + opt_str_2: Optional[str] = None + + # Valid initialization + s1 = MyStrictStruct(req_int=10, opt_str="hello") + self.assertEqual(s1.req_int, 10) + self.assertEqual(s1.opt_str, "hello") + self.assertEqual(s1.def_int, 123) + self.assertIsNone(s1.opt_str_2) + + with self.assertRaisesRegex( + ValueError, "Struct MyStrictStruct is not valid; some required fields were not set on init" + ): + MyStrictStruct() + + def test_strict_struct_hasattr_delattr(self): + """test hasattr and delattr behavior for strict structs""" + + class MyStrictStruct1(csp.Struct, allow_unset=False): + req_int: int + opt_str: Optional[str] = None + + class MyStrictStruct2(csp.Struct, allow_unset=False): + req_int: int + opt_str: Optional[str] = None + + s = MyStrictStruct1(req_int=10, opt_str="hello") + r = MyStrictStruct2(req_int=5) + + # hasattr will always be True for all defined fields + self.assertTrue(hasattr(s, "req_int")) + self.assertTrue(hasattr(s, "opt_str")) + self.assertTrue(hasattr(r, "req_int")) + self.assertTrue(hasattr(r, "opt_str")) + + # delattr is forbidden + with self.assertRaisesRegex( + AttributeError, "Strict struct MyStrictStruct1 does not allow the deletion of field req_int" + ): + del s.req_int + + with self.assertRaisesRegex( + AttributeError, "Strict struct MyStrictStruct1 does not allow the deletion of field opt_str" + ): + del s.opt_str + + def test_strict_struct_serialization(self): + """test to_dict and from_dict behavior""" + + class MyStrictStruct(csp.Struct, allow_unset=False): + req_int: int + opt_str: Optional[str] = None + def_int: int = 100 + req_opt_str: Optional[str] = None + + s = MyStrictStruct(req_int=50, req_opt_str="NoneStr") + expected_dict = {"req_int": 50, "opt_str": None, "def_int": 100, "req_opt_str": "NoneStr"} + self.assertEqual(s.to_dict(), expected_dict) + + with self.assertRaisesRegex( + ValueError, "Struct MyStrictStruct is not valid; some required fields were not set on init" + ): + MyStrictStruct.from_dict({"opt_str": "hello", "def_int": 13}) + + MyStrictStruct.from_dict({"req_int": 60, "opt_str": None, "req_opt_str": None}) + s2 = MyStrictStruct.from_dict({"req_int": 60, "req_opt_str": None}) + self.assertEqual(s2.req_int, 60) + self.assertIsNone(s2.opt_str) + self.assertEqual(s2.def_int, 100) + + def test_strict_struct_wiring_access_1(self): + """test accessing fields on a time series at graph wiring time""" + + class MyStrictStruct(csp.Struct, allow_unset=False): + req_int: int + opt_str: Optional[str] = None + + # check that at graph and wire time we are able to access required fields just fine: + + @csp.node + def ok_node(x: csp.ts[MyStrictStruct]): + int_val = x.req_int + + @csp.graph + def g(): + s_ts = csp.const(MyStrictStruct(req_int=1)) + req_ts = s_ts.req_int + csp.add_graph_output("req_ts", req_ts) + + res = csp.run(g, starttime=datetime(2023, 1, 1)) + self.assertEqual(res["req_ts"][0][1], 1) + + # check that at graph time we cannot access optional fields: + + @csp.graph + def g_fail(): + s_ts = csp.const(MyStrictStruct(req_int=1)) + opt_ts = s_ts.opt_str + + with self.assertRaisesRegex( + AttributeError, + "Cannot access optional field 'opt_str' on strict struct object 'MyStrictStruct' at graph time", + ): + csp.run(g_fail, starttime=datetime(2023, 1, 1)) + + def test_strict_struct_fromts(self): + """fromts requires all non-defaulted fields to tick together""" + + class MyStrictStruct(csp.Struct, allow_unset=False): + req_int1: int + req_int2: int + opt_str: Optional[str] = None + req_default_str: str = "default" + + @csp.node + def make_ts(x: csp.ts[int]) -> csp.ts[int]: + if x % 2 == 0: + return x + + @csp.graph + def g(): + ts1 = make_ts(csp.const(2)) + ts2 = make_ts(csp.const(1)) + + # ts1 and ts2 don't tick together + s_ts = MyStrictStruct.fromts(req_int1=ts1, req_int2=ts2) + csp.add_graph_output("output", s_ts) + + with self.assertRaisesRegex( + ValueError, "Struct MyStrictStruct is not valid; some required fields did not tick" + ): + csp.run(g, starttime=datetime(2023, 1, 1)) + + @csp.graph + def g_ok(): + ts1 = csp.const(2) + ts2 = csp.const(4) + + # ts1 and ts2 tick together + s_ts = MyStrictStruct.fromts(req_int1=ts1, req_int2=ts2) + csp.add_graph_output("output", s_ts) + + csp.run(g_ok, starttime=datetime(2023, 1, 1)) + + @csp.graph + def g_ok_with_optional(): + beat = csp.timer(timedelta(days=1)) + even = csp.eq(csp.mod(csp.count(beat), csp.const(2)), csp.const(0)) + + int_ts1 = csp.sample(even, csp.const(1)) + int_ts2 = csp.sample(even, csp.const(2)) + str_ts = csp.sample(even, csp.const("Hello")) + + s_ts = MyStrictStruct.fromts(req_int1=int_ts1, req_int2=int_ts2, req_default_str=str_ts) + csp.add_graph_output("output", s_ts) + + csp.run(g_ok_with_optional, starttime=datetime(2025, 1, 1), endtime=datetime(2025, 1, 5)) + + def test_strict_struct_inheritance_and_nested(self): + class BaseStrict(csp.Struct, allow_unset=False): + base_req: int + + class DerivedStrict(BaseStrict, allow_unset=False): + derived_req: int + + d_ok = DerivedStrict(base_req=1, derived_req=2) + self.assertEqual(d_ok.base_req, 1) + self.assertEqual(d_ok.derived_req, 2) + + with self.assertRaisesRegex( + ValueError, "Struct DerivedStrict is not valid; some required fields were not set on init" + ): + DerivedStrict(base_req=10) + with self.assertRaisesRegex( + ValueError, "Struct DerivedStrict is not valid; some required fields were not set on init" + ): + DerivedStrict(derived_req=20) + + # loose base & strict child: + class LooseBase(csp.Struct, allow_unset=True): + loose_req: int + + class StrictChild(LooseBase, allow_unset=False): + child_req: int + + sc_ok = StrictChild(child_req=5, loose_req=10) + self.assertEqual(sc_ok.child_req, 5) + + with self.assertRaisesRegex( + ValueError, "Struct StrictChild is not valid; some required fields were not set on init" + ): + StrictChild() + with self.assertRaisesRegex( + ValueError, "Struct StrictChild is not valid; some required fields were not set on init" + ): + StrictChild(loose_req=10) + with self.assertRaisesRegex( + ValueError, "Struct StrictChild is not valid; some required fields were not set on init" + ): + StrictChild(child_req=5) + + # nested struct fields: + class InnerStrict(csp.Struct, allow_unset=False): + val: int + + class OuterStrict(csp.Struct, allow_unset=False): + inner: InnerStrict + + os_ok = OuterStrict(inner=InnerStrict(val=42)) + self.assertEqual(os_ok.inner.val, 42) + + with self.assertRaisesRegex( + ValueError, "Struct InnerStrict is not valid; some required fields were not set on init" + ): + OuterStrict(inner=InnerStrict()) + + # nested loose struct inside strict: + class InnerLoose(csp.Struct, allow_unset=True): + val: int + + class OuterStrict2(csp.Struct, allow_unset=False): + inner: InnerLoose + + ol_ok = OuterStrict2(inner=InnerLoose()) + self.assertIsInstance(ol_ok.inner, InnerLoose) + + with self.assertRaisesRegex( + ValueError, "Struct OuterStrict2 is not valid; some required fields were not set on init" + ): + OuterStrict2() + + def test_nonstrict_cannot_inherit_strict(self): + """non-strict structs inheriting from strict bases should raise""" + + class StrictBase(csp.Struct, allow_unset=False): + base_val: int + + with self.assertRaisesRegex(ValueError, "non-strict inheritance of strict base"): + + 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""" + + class NonStrictBase(csp.Struct, allow_unset=True): + base_val: int + + class StrictMiddle(NonStrictBase, allow_unset=False): + middle_val: int + + with self.assertRaisesRegex(ValueError, "non-strict inheritance of strict base"): + + 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""" + + class InnerStrict(csp.Struct, allow_unset=False): + x: int + + class InnerLoose(csp.Struct, allow_unset=True): + y: Optional[int] = None + + class OuterStruct(csp.Struct, allow_unset=False): + strict_inner: InnerStrict + loose_inner: InnerLoose + + o = OuterStruct(strict_inner=InnerStrict(x=5), loose_inner=InnerLoose()) + expected_dict = {"strict_inner": {"x": 5}, "loose_inner": {"y": None}} + self.assertEqual(o.to_dict(), expected_dict) + + o2 = OuterStruct.from_dict({"strict_inner": {"x": 10}, "loose_inner": {"y": 20}}) + self.assertEqual(o2.strict_inner.x, 10) + self.assertIsNotNone(o2.loose_inner) + self.assertEqual(o2.loose_inner.y, 20) + + with self.assertRaisesRegex( + ValueError, "Struct OuterStruct is not valid; some required fields were not set on init" + ): + OuterStruct.from_dict({"loose_inner": {"y": 1}}) + + with self.assertRaisesRegex( + ValueError, "Struct InnerStrict is not valid; some required fields were not set on init" + ): + OuterStruct.from_dict({"strict_inner": {}, "loose_inner": {"y": None}}) + + def test_strict_struct_wiring_access_2(self): + class Test(csp.Struct, allow_unset=False): + name: str + age: int + is_active: Optional[bool] = None + + def greet(self): + return f"Hello, my name is {self.name} and I am {self.age} years old." + + @csp.node + def test() -> csp.ts[Test]: + return Test(name="John", age=30, is_active=True) + + @csp.graph + def main_graph(): + res = test().is_active + csp.print("", res) + + with self.assertRaisesRegex( + AttributeError, "Cannot access optional field 'is_active' on strict struct object 'Test' at graph time" + ): + csp.build_graph(main_graph) + + def test_strict_struct_optional_field_validation_no_default(self): + """test that strict structs cannot have Optional fields without defaults""" + + with self.assertRaisesRegex(TypeError, "Optional field bad_field must have a default value"): + + class InvalidStrictStruct(csp.Struct, allow_unset=False): + req_field: int + bad_field: Optional[str] + + +if __name__ == "__main__": + unittest.main() From 7bd22fc8e90cc3d1f5743a59e758d7ecf482ee59 Mon Sep 17 00:00:00 2001 From: Adam Glustein Date: Thu, 18 Sep 2025 16:23:47 -0400 Subject: [PATCH 2/9] Fix up PR Signed-off-by: Adam Glustein --- cpp/csp/adapters/kafka/KafkaInputAdapter.cpp | 4 --- .../parquet/ParquetReaderColumnAdapter.cpp | 3 --- .../utils/JSONMessageStructConverter.cpp | 3 --- .../adapters/websocket/ClientInputAdapter.cpp | 2 -- cpp/csp/engine/InputAdapter.h | 7 +++++ csp/tests/test_strict_structs.py | 27 ++++++++++--------- 6 files changed, 21 insertions(+), 25 deletions(-) diff --git a/cpp/csp/adapters/kafka/KafkaInputAdapter.cpp b/cpp/csp/adapters/kafka/KafkaInputAdapter.cpp index bbef00584..296df8daa 100644 --- a/cpp/csp/adapters/kafka/KafkaInputAdapter.cpp +++ b/cpp/csp/adapters/kafka/KafkaInputAdapter.cpp @@ -113,10 +113,6 @@ void KafkaInputAdapter::processMessage( RdKafka::Message* message, bool live, cs if( m_tickTimestampField ) msgTime = m_tickTimestampField->value(tick.get()); - if (!tick.get() -> validate()) - CSP_THROW( ValueError, "Struct validation failed for Kafka message, fields missing" ); - - bool pushLive = shouldPushLive(live, msgTime); if( shouldProcessMessage( pushLive, msgTime ) ) pushTick(pushLive, msgTime, std::move(tick), batch); diff --git a/cpp/csp/adapters/parquet/ParquetReaderColumnAdapter.cpp b/cpp/csp/adapters/parquet/ParquetReaderColumnAdapter.cpp index f95ff6704..b8380ce53 100644 --- a/cpp/csp/adapters/parquet/ParquetReaderColumnAdapter.cpp +++ b/cpp/csp/adapters/parquet/ParquetReaderColumnAdapter.cpp @@ -520,9 +520,6 @@ void ParquetStructAdapter::dispatchValue( const utils::Symbol *symbol, bool isNu { fieldSetter( s ); } - - CSP_TRUE_OR_THROW_RUNTIME( s -> validate(), "Struct validation failed for Parquet message, some fields are missing" ); - dispatchedValue = &s; } diff --git a/cpp/csp/adapters/utils/JSONMessageStructConverter.cpp b/cpp/csp/adapters/utils/JSONMessageStructConverter.cpp index 585b3ee6d..405a02bb3 100644 --- a/cpp/csp/adapters/utils/JSONMessageStructConverter.cpp +++ b/cpp/csp/adapters/utils/JSONMessageStructConverter.cpp @@ -145,9 +145,6 @@ StructPtr JSONMessageStructConverter::convertJSON( const char * fieldname, const } ); } - if( !struct_ -> validate() ) - CSP_THROW( ValueError, "JSON conversion of struct " << sType.meta() -> name() << " failed; some required fields were not set" ); - return struct_; } diff --git a/cpp/csp/adapters/websocket/ClientInputAdapter.cpp b/cpp/csp/adapters/websocket/ClientInputAdapter.cpp index b727341ed..e4b0b7ff7 100644 --- a/cpp/csp/adapters/websocket/ClientInputAdapter.cpp +++ b/cpp/csp/adapters/websocket/ClientInputAdapter.cpp @@ -31,8 +31,6 @@ void ClientInputAdapter::processMessage( void* c, size_t t, PushBatch* batch ) if( dataType() -> type() == CspType::Type::STRUCT ) { auto tick = m_converter -> asStruct( c, t ); - if (!tick.get() -> validate()) - CSP_THROW( ValueError, "Struct validation failed for WebSocket message, fields missing" ); pushTick( std::move(tick), batch ); } else if ( dataType() -> type() == CspType::Type::STRING ) { diff --git a/cpp/csp/engine/InputAdapter.h b/cpp/csp/engine/InputAdapter.h index f992d6ff2..3f5b1b7d6 100644 --- a/cpp/csp/engine/InputAdapter.h +++ b/cpp/csp/engine/InputAdapter.h @@ -4,6 +4,7 @@ #include #include #include +#include #include namespace csp @@ -55,6 +56,12 @@ class InputAdapter : public TimeSeriesProvider, public EngineOwned template bool InputAdapter::consumeTick( const T & value ) { + if constexpr( CspType::Type::fromCType::type == CspType::TypeTraits::STRUCT ) + { + if( unlikely( !( value -> validate() ) ) ) + CSP_THROW( ValueError, "Struct " << value -> meta() -> name() << " is not valid; some required fields were not set on init" ); + } + switch( pushMode() ) { case PushMode::LAST_VALUE: diff --git a/csp/tests/test_strict_structs.py b/csp/tests/test_strict_structs.py index 817ec7d2b..e3fee1c00 100644 --- a/csp/tests/test_strict_structs.py +++ b/csp/tests/test_strict_structs.py @@ -52,16 +52,12 @@ class MyStrictStruct(csp.Struct, allow_unset=False): def test_strict_struct_hasattr_delattr(self): """test hasattr and delattr behavior for strict structs""" - class MyStrictStruct1(csp.Struct, allow_unset=False): - req_int: int - opt_str: Optional[str] = None - - class MyStrictStruct2(csp.Struct, allow_unset=False): + class MyStrictStruct(csp.Struct, allow_unset=False): req_int: int opt_str: Optional[str] = None - s = MyStrictStruct1(req_int=10, opt_str="hello") - r = MyStrictStruct2(req_int=5) + s = MyStrictStruct(req_int=10, opt_str="hello") + r = MyStrictStruct(req_int=5) # hasattr will always be True for all defined fields self.assertTrue(hasattr(s, "req_int")) @@ -71,12 +67,12 @@ class MyStrictStruct2(csp.Struct, allow_unset=False): # delattr is forbidden with self.assertRaisesRegex( - AttributeError, "Strict struct MyStrictStruct1 does not allow the deletion of field req_int" + AttributeError, "Strict struct MyStrictStruct does not allow the deletion of field req_int" ): del s.req_int with self.assertRaisesRegex( - AttributeError, "Strict struct MyStrictStruct1 does not allow the deletion of field opt_str" + AttributeError, "Strict struct MyStrictStruct does not allow the deletion of field opt_str" ): del s.opt_str @@ -348,11 +344,16 @@ def main_graph(): def test_strict_struct_optional_field_validation_no_default(self): """test that strict structs cannot have Optional fields without defaults""" - with self.assertRaisesRegex(TypeError, "Optional field bad_field must have a default value"): + class MyStruct(csp.Struct, allow_unset=False): + req_field: int + opt_field: Optional[str] + + with self.assertRaisesRegex(TypeError, "Struct MyStruct is not valid; some required fields were not set on init"): + MyStruct(req_field=42) - class InvalidStrictStruct(csp.Struct, allow_unset=False): - req_field: int - bad_field: Optional[str] + x = MyStruct(req_field=42, bad_field=None) + self.assertEqual(x.req_field, 42) + self.assertIsNone(x.opt_field) if __name__ == "__main__": From c4c14ed44e88a37d703265a7412fe47c0f398d18 Mon Sep 17 00:00:00 2001 From: Adam Glustein Date: Fri, 10 Oct 2025 13:53:45 -0400 Subject: [PATCH 3/9] Further clean up of PR; add docs, improve error messages, fix inheritance bug Signed-off-by: Adam Glustein --- .../utils/JSONMessageStructConverter.cpp | 1 - cpp/csp/cppnodes/baselibimpl.cpp | 6 +- cpp/csp/engine/BasketInfo.cpp | 4 -- cpp/csp/engine/InputAdapter.h | 2 +- cpp/csp/engine/Struct.cpp | 31 ++++++++- cpp/csp/engine/Struct.h | 8 +++ cpp/csp/python/PyStruct.cpp | 8 +-- csp/tests/test_strict_structs.py | 67 +++++++++---------- docs/wiki/api-references/csp.Struct-API.md | 36 +++++++++- 9 files changed, 110 insertions(+), 53 deletions(-) diff --git a/cpp/csp/adapters/utils/JSONMessageStructConverter.cpp b/cpp/csp/adapters/utils/JSONMessageStructConverter.cpp index 405a02bb3..574f21084 100644 --- a/cpp/csp/adapters/utils/JSONMessageStructConverter.cpp +++ b/cpp/csp/adapters/utils/JSONMessageStructConverter.cpp @@ -251,7 +251,6 @@ csp::StructPtr JSONMessageStructConverter::asStruct( void * bytes, size_t size ) } ); } - // root struct validation (validate()) deferred to adapter level return data; } diff --git a/cpp/csp/cppnodes/baselibimpl.cpp b/cpp/csp/cppnodes/baselibimpl.cpp index 5530bb446..c36393b98 100644 --- a/cpp/csp/cppnodes/baselibimpl.cpp +++ b/cpp/csp/cppnodes/baselibimpl.cpp @@ -705,8 +705,8 @@ DECLARE_CPPNODE( struct_fromts ) ); } - if( !out.get() -> validate( ) ) - CSP_THROW( ValueError, "Struct " << cls.value() -> name() << " is not valid; some required fields did not tick" ); + if( unlikely( !out.get() -> validate() ) ) + CSP_THROW( ValueError, "Struct " << cls.value() -> name() << " is not valid; required fields " << out -> formatAllUnsetStrictFields() << " did not tick" ); CSP_OUTPUT( std::move( out ) ); } @@ -762,7 +762,7 @@ DECLARE_CPPNODE( struct_collectts ) ); } - if( !out.get() -> validate( ) ) + if( unlikely( !out.get() -> validate( ) ) ) CSP_THROW( ValueError, "Struct " << cls.value() -> name() << " is not valid; some required fields did not tick" ); CSP_OUTPUT( std::move( out ) ); diff --git a/cpp/csp/engine/BasketInfo.cpp b/cpp/csp/engine/BasketInfo.cpp index 327ecec66..f43e425e9 100644 --- a/cpp/csp/engine/BasketInfo.cpp +++ b/cpp/csp/engine/BasketInfo.cpp @@ -161,8 +161,6 @@ void DynamicOutputBasketInfo::addShapeChange( const DialectGenericType & key, bo { auto events = autogen::DynamicBasketEvents::create(); events -> set_events( {} ); - if( !events -> validate() ) - CSP_THROW( ValueError, "DynamicBasketEvents struct is not valid; some required fields were not set" ); m_shapeTs.outputTickTyped( m_parentNode -> rootEngine() -> cycleCount(), m_parentNode -> rootEngine() -> now(), events, false ); @@ -173,8 +171,6 @@ void DynamicOutputBasketInfo::addShapeChange( const DialectGenericType & key, bo auto event = autogen::DynamicBasketEvent::create(); event -> set_key( key ); event -> set_added( added ); - if( !event -> validate() ) - CSP_THROW( ValueError, "DynamicBasketEvent struct is not valid; some required fields were not set" ); const_cast &>( events ).emplace_back( event ); } diff --git a/cpp/csp/engine/InputAdapter.h b/cpp/csp/engine/InputAdapter.h index 3f5b1b7d6..051ca83ea 100644 --- a/cpp/csp/engine/InputAdapter.h +++ b/cpp/csp/engine/InputAdapter.h @@ -59,7 +59,7 @@ bool InputAdapter::consumeTick( const T & value ) if constexpr( CspType::Type::fromCType::type == CspType::TypeTraits::STRUCT ) { if( unlikely( !( value -> validate() ) ) ) - CSP_THROW( ValueError, "Struct " << value -> meta() -> name() << " is not valid; some required fields were not set on init" ); + CSP_THROW( ValueError, "Struct " << value -> meta() -> name() << " is not valid; required fields " << value -> formatAllUnsetStrictFields() << " were not set on init" ); } switch( pushMode() ) diff --git a/cpp/csp/engine/Struct.cpp b/cpp/csp/engine/Struct.cpp index 3b501036b..17ea02ec4 100644 --- a/cpp/csp/engine/Struct.cpp +++ b/cpp/csp/engine/Struct.cpp @@ -3,6 +3,7 @@ #include #include #include +#include namespace csp { @@ -486,8 +487,34 @@ bool StructMeta::allFieldsSet( const Struct * s ) const if( ( *m & bitmask ) != bitmask ) return false; } + return true; +} - return m_base ? m_base -> allFieldsSet( s ) : true; +std::string StructMeta::formatAllUnsetStrictFields( const Struct * s ) const +{ + bool first = true; + std::stringstream ss; + ss << "["; + + for ( const StructMeta * cur = this; cur; cur = cur -> m_base.get() ) + { + if ( !cur -> isStrict() ) + continue; + + for( size_t i = cur -> m_firstPartialField; i < cur -> m_fields.size(); ++i ) + { + if( !cur -> m_fields[ i ] -> isSet( s ) ) + { + if( !first ) + ss << ", "; + else + first = false; + ss << cur -> m_fields[ i ] -> fieldname(); + } + } + } + ss << "]"; + return ss.str(); } void StructMeta::destroy( Struct * s ) const @@ -524,8 +551,6 @@ void StructMeta::destroy( Struct * s ) const return true; } - - Struct::Struct( const std::shared_ptr & meta ) { //Initialize meta shared_ptr diff --git a/cpp/csp/engine/Struct.h b/cpp/csp/engine/Struct.h index f3b0b63bf..74401034e 100644 --- a/cpp/csp/engine/Struct.h +++ b/cpp/csp/engine/Struct.h @@ -632,6 +632,9 @@ class StructMeta : public std::enable_shared_from_this void clear( Struct * s ) const; bool allFieldsSet( const Struct * s ) const; + // used for error messages / debugging + std::string formatAllUnsetStrictFields( const Struct * s ) const; + //for debugging layout of types std::string layout() const; @@ -747,6 +750,11 @@ class Struct return meta() -> validate( this ); } + std::string formatAllUnsetStrictFields() const + { + return meta() -> formatAllUnsetStrictFields( this ); + } + //used to cache dialect representations of this struct, if needed void * dialectPtr() const { return hidden() -> dialectPtr; } diff --git a/cpp/csp/python/PyStruct.cpp b/cpp/csp/python/PyStruct.cpp index b4e7fef0f..3bfc1fe10 100644 --- a/cpp/csp/python/PyStruct.cpp +++ b/cpp/csp/python/PyStruct.cpp @@ -112,7 +112,6 @@ static PyObject * PyStructMeta_new( PyTypeObject *subtype, PyObject *args, PyObj Py_ssize_t pos = 0; PyObject *optional_fields = PyDict_GetItemString( dict, "__optional_fields__" ); - while( PyDict_Next( metadata, &pos, &key, &type ) ) { const char * keystr = PyUnicode_AsUTF8( key ); @@ -158,7 +157,7 @@ static PyObject * PyStructMeta_new( PyTypeObject *subtype, PyObject *args, PyObj default: CSP_THROW( ValueError, "Unexpected csp type " << csptype -> type() << " on struct " << name ); } - + fields.emplace_back( field ); } } @@ -360,7 +359,6 @@ static PyObject * PyStructMeta_metadata_info( PyStructMeta * m ) return out.release(); } - static PyMethodDef PyStructMeta_methods[] = { {"_layout", (PyCFunction) PyStructMeta_layout, METH_NOARGS, "debug view of structs internal mem layout"}, {"_metadata_info", (PyCFunction) PyStructMeta_metadata_info, METH_NOARGS, "provide detailed information about struct layout"}, @@ -812,8 +810,8 @@ int PyStruct_init( PyStruct * self, PyObject * args, PyObject * kwargs ) CSP_BEGIN_METHOD; PyStruct_setattrs( self, args, kwargs, "__init__" ); - if( !self -> struct_ -> validate() ) - CSP_THROW( ValueError, "Struct " << self -> struct_ -> meta() -> name() << " is not valid; some required fields were not set on init" ); + if( unlikely( !self -> struct_ -> validate() ) ) + CSP_THROW( ValueError, "Struct " << self -> struct_ -> meta() -> name() << " is not valid; required fields " << self -> struct_ -> formatAllUnsetStrictFields() << " were not set on init" ); CSP_RETURN_INT; } diff --git a/csp/tests/test_strict_structs.py b/csp/tests/test_strict_structs.py index e3fee1c00..1174e5ce9 100644 --- a/csp/tests/test_strict_structs.py +++ b/csp/tests/test_strict_structs.py @@ -33,7 +33,7 @@ def test_strict_struct_initialization(self): class MyStrictStruct(csp.Struct, allow_unset=False): req_int: int - opt_str: Optional[str] = None + opt_str: str | None = None def_int: int = 123 opt_str_2: Optional[str] = None @@ -45,7 +45,7 @@ class MyStrictStruct(csp.Struct, allow_unset=False): self.assertIsNone(s1.opt_str_2) with self.assertRaisesRegex( - ValueError, "Struct MyStrictStruct is not valid; some required fields were not set on init" + ValueError, r"Struct MyStrictStruct is not valid; required fields \[req_int\] were not set on init" ): MyStrictStruct() @@ -83,22 +83,22 @@ class MyStrictStruct(csp.Struct, allow_unset=False): req_int: int opt_str: Optional[str] = None def_int: int = 100 - req_opt_str: Optional[str] = None + opt_str2: Optional[str] = None - s = MyStrictStruct(req_int=50, req_opt_str="NoneStr") - expected_dict = {"req_int": 50, "opt_str": None, "def_int": 100, "req_opt_str": "NoneStr"} + s = MyStrictStruct(req_int=50, opt_str2="NoneStr") + expected_dict = {"req_int": 50, "opt_str": None, "def_int": 100, "opt_str2": "NoneStr"} self.assertEqual(s.to_dict(), expected_dict) with self.assertRaisesRegex( - ValueError, "Struct MyStrictStruct is not valid; some required fields were not set on init" + ValueError, r"Struct MyStrictStruct is not valid; required fields \[req_int\] were not set on init" ): MyStrictStruct.from_dict({"opt_str": "hello", "def_int": 13}) - MyStrictStruct.from_dict({"req_int": 60, "opt_str": None, "req_opt_str": None}) - s2 = MyStrictStruct.from_dict({"req_int": 60, "req_opt_str": None}) + MyStrictStruct.from_dict({"req_int": 60, "opt_str": None, "opt_str2": None}) + s2 = MyStrictStruct.from_dict({"def_int": 72, "req_int": 60, "opt_str2": None}) self.assertEqual(s2.req_int, 60) self.assertIsNone(s2.opt_str) - self.assertEqual(s2.def_int, 100) + self.assertEqual(s2.def_int, 72) def test_strict_struct_wiring_access_1(self): """test accessing fields on a time series at graph wiring time""" @@ -159,7 +159,7 @@ def g(): csp.add_graph_output("output", s_ts) with self.assertRaisesRegex( - ValueError, "Struct MyStrictStruct is not valid; some required fields did not tick" + ValueError, r"Struct MyStrictStruct is not valid; required fields \[req_int2\] did not tick" ): csp.run(g, starttime=datetime(2023, 1, 1)) @@ -200,11 +200,11 @@ class DerivedStrict(BaseStrict, allow_unset=False): self.assertEqual(d_ok.derived_req, 2) with self.assertRaisesRegex( - ValueError, "Struct DerivedStrict is not valid; some required fields were not set on init" + ValueError, r"Struct DerivedStrict is not valid; required fields \[derived_req\] were not set on init" ): DerivedStrict(base_req=10) with self.assertRaisesRegex( - ValueError, "Struct DerivedStrict is not valid; some required fields were not set on init" + ValueError, r"Struct DerivedStrict is not valid; required fields \[base_req\] were not set on init" ): DerivedStrict(derived_req=20) @@ -219,32 +219,36 @@ class StrictChild(LooseBase, allow_unset=False): self.assertEqual(sc_ok.child_req, 5) with self.assertRaisesRegex( - ValueError, "Struct StrictChild is not valid; some required fields were not set on init" + ValueError, r"Struct StrictChild is not valid; required fields \[child_req\] were not set on init" ): StrictChild() with self.assertRaisesRegex( - ValueError, "Struct StrictChild is not valid; some required fields were not set on init" + ValueError, r"Struct StrictChild is not valid; required fields \[child_req\] were not set on init" ): StrictChild(loose_req=10) - with self.assertRaisesRegex( - ValueError, "Struct StrictChild is not valid; some required fields were not set on init" - ): - StrictChild(child_req=5) + + StrictChild(child_req=5) # nested struct fields: class InnerStrict(csp.Struct, allow_unset=False): val: int + val2: float class OuterStrict(csp.Struct, allow_unset=False): inner: InnerStrict - os_ok = OuterStrict(inner=InnerStrict(val=42)) + os_ok = OuterStrict(inner=InnerStrict(val=42, val2=43)) self.assertEqual(os_ok.inner.val, 42) + self.assertEqual(os_ok.inner.val2, 43) with self.assertRaisesRegex( - ValueError, "Struct InnerStrict is not valid; some required fields were not set on init" + ValueError, r"Struct InnerStrict is not valid; required fields \[val, val2\] were not set on init" ): OuterStrict(inner=InnerStrict()) + with self.assertRaisesRegex( + ValueError, r"Struct InnerStrict is not valid; required fields \[val2\] were not set on init" + ): + OuterStrict(inner=InnerStrict(val=42)) # nested loose struct inside strict: class InnerLoose(csp.Struct, allow_unset=True): @@ -257,7 +261,7 @@ class OuterStrict2(csp.Struct, allow_unset=False): self.assertIsInstance(ol_ok.inner, InnerLoose) with self.assertRaisesRegex( - ValueError, "Struct OuterStrict2 is not valid; some required fields were not set on init" + ValueError, r"Struct OuterStrict2 is not valid; required fields \[inner\] were not set on init" ): OuterStrict2() @@ -309,12 +313,12 @@ class OuterStruct(csp.Struct, allow_unset=False): self.assertEqual(o2.loose_inner.y, 20) with self.assertRaisesRegex( - ValueError, "Struct OuterStruct is not valid; some required fields were not set on init" + ValueError, r"Struct OuterStruct is not valid; required fields \[strict_inner\] were not set on init" ): OuterStruct.from_dict({"loose_inner": {"y": 1}}) with self.assertRaisesRegex( - ValueError, "Struct InnerStrict is not valid; some required fields were not set on init" + ValueError, r"Struct InnerStrict is not valid; required fields \[x\] were not set on init" ): OuterStruct.from_dict({"strict_inner": {}, "loose_inner": {"y": None}}) @@ -322,7 +326,7 @@ def test_strict_struct_wiring_access_2(self): class Test(csp.Struct, allow_unset=False): name: str age: int - is_active: Optional[bool] = None + is_active: bool | None = None def greet(self): return f"Hello, my name is {self.name} and I am {self.age} years old." @@ -342,18 +346,11 @@ def main_graph(): csp.build_graph(main_graph) def test_strict_struct_optional_field_validation_no_default(self): - """test that strict structs cannot have Optional fields without defaults""" - - class MyStruct(csp.Struct, allow_unset=False): - req_field: int - opt_field: Optional[str] - - with self.assertRaisesRegex(TypeError, "Struct MyStruct is not valid; some required fields were not set on init"): - MyStruct(req_field=42) + with self.assertRaisesRegex(TypeError, "Optional field bad_field must have a default value"): - x = MyStruct(req_field=42, bad_field=None) - self.assertEqual(x.req_field, 42) - self.assertIsNone(x.opt_field) + class InvalidStrictStruct(csp.Struct, allow_unset=False): + req_field: int + bad_field: Optional[str] if __name__ == "__main__": diff --git a/docs/wiki/api-references/csp.Struct-API.md b/docs/wiki/api-references/csp.Struct-API.md index 3df8970d7..50a3a1ede 100644 --- a/docs/wiki/api-references/csp.Struct-API.md +++ b/docs/wiki/api-references/csp.Struct-API.md @@ -147,6 +147,40 @@ print(f"Using FastList field: value {s.a}, type {type(s.a)}, is Python list: {is - **`to_json(self, callback=lambda x: x)`**: convert struct instance to a json string, callback is invoked for any values encountered when processing the struct that are not basic Python types, datetime types, tuples, lists, dicts, csp.Structs, or csp.Enums. The callback should convert the unhandled type to a combination of the known types. - **`all_fields_set(self)`**: returns `True` if all the fields on the struct are set. Note that this will not recursively check sub-struct fields -# Note on inheritance +## Note on inheritance `csp.Struct` types may inherit from each other, but **multiple inheritance is not supported**. Composition is usually a good choice in absence of multiple inheritance. + +## "Strict" Structs + +By default, CSP Struct objects allow any field to be unset. This means that accessing fields without checking `hasattr` first can cause an `AttributeError` at runtime. To improve safety, CSP Structs provide the `allow_unset` metaclass parameter that can be set to `False` to enable "strict struct" semantics. + +### `allow_unset` Parameter + +- **Default:** `True` (for backwards compatibility) +- **When set to `False`:** Enforces *strict struct semantics* — required fields must be set at initialization. + - Note that `del struct.field` is not allowed for stric structs — this will raise an error. As a result, for any strict struct, `hasattr(struct, field)` always returns True for any defined field. + - A non-strict struct may not inherit (directly or indirectly) from a strict base. + +### Semantics when `allow_unset=False` + +1. **Required Fields** + + - Any field with a non‑optional type (e.g., `x: int`) **must be set** upon struct creation. + - If no value is passed and no default is provided for a required field, a `ValueError` is raised with the missing fields. + +1. **Optional Fields** + + - Fields declared as `Optional[T] = None` or `T | None = None` are not required to be set. They must have a default value, which can be `None`, meaning they are unset. + +1. **Example** + + ```python + class MyStruct(csp.Struct, allow_unset=False): + x: int # required + y: Optional[str] = None # optional + z: str | None = None # optional (alternate syntax) + + MyStruct(x=3) # good: y and z default to None + MyStruct(y="hello") # bad: required field x is not set + ``` From e5889d240a0d5bb6134690f7937df24a8a2f4b55 Mon Sep 17 00:00:00 2001 From: Adam Glustein Date: Fri, 10 Oct 2025 14:16:33 -0400 Subject: [PATCH 4/9] Change lint to use 3.11 Signed-off-by: Adam Glustein --- .github/workflows/wiki-lint.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/wiki-lint.yml b/.github/workflows/wiki-lint.yml index a7bf39b1a..6b5126571 100644 --- a/.github/workflows/wiki-lint.yml +++ b/.github/workflows/wiki-lint.yml @@ -24,7 +24,7 @@ jobs: os: - ubuntu-24.04 python-version: - - 3.9 + - 3.11 runs-on: ${{ matrix.os }} From da7599d8089502da52c04bf4ec32e5323dee11b1 Mon Sep 17 00:00:00 2001 From: Adam Glustein Date: Fri, 10 Oct 2025 15:49:24 -0400 Subject: [PATCH 5/9] Fix 3.10+ Union annotations Signed-off-by: Adam Glustein --- csp/tests/test_strict_structs.py | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/csp/tests/test_strict_structs.py b/csp/tests/test_strict_structs.py index 1174e5ce9..f409c7c5c 100644 --- a/csp/tests/test_strict_structs.py +++ b/csp/tests/test_strict_structs.py @@ -1,3 +1,4 @@ +import sys import unittest from datetime import datetime, timedelta from typing import Optional @@ -33,7 +34,7 @@ def test_strict_struct_initialization(self): class MyStrictStruct(csp.Struct, allow_unset=False): req_int: int - opt_str: str | None = None + opt_str: Optional[str] = None def_int: int = 123 opt_str_2: Optional[str] = None @@ -49,6 +50,24 @@ class MyStrictStruct(csp.Struct, allow_unset=False): ): MyStrictStruct() + # test 3.10+ | operator + if sys.version_info >= (3, 10): + + class MyStrictStruct(csp.Struct, allow_unset=False): + req_int: int + opt_str: str | None = None + + s1 = MyStrictStruct(req_int=42, opt_str="hola") + self.assertEqual(s1.req_int, 42) + self.assertEqual(s1.opt_str, "hola") + with self.assertRaisesRegex( + ValueError, r"Struct MyStrictStruct is not valid; required fields \[req_int\] were not set on init" + ): + MyStrictStruct(opt_str="bonjour") + s2 = MyStrictStruct(req_int=43) + self.assertEqual(s2.req_int, 43) + self.assertIsNone(s2.opt_str) + def test_strict_struct_hasattr_delattr(self): """test hasattr and delattr behavior for strict structs""" @@ -326,7 +345,7 @@ def test_strict_struct_wiring_access_2(self): class Test(csp.Struct, allow_unset=False): name: str age: int - is_active: bool | None = None + is_active: Optional[bool] = None def greet(self): return f"Hello, my name is {self.name} and I am {self.age} years old." From 05d6ea71c62a2466b446fe627783d4fb9041c207 Mon Sep 17 00:00:00 2001 From: Adam Glustein Date: Mon, 10 Nov 2025 09:41:15 -0500 Subject: [PATCH 6/9] Fix typo in docs Signed-off-by: Adam Glustein --- docs/wiki/api-references/csp.Struct-API.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/wiki/api-references/csp.Struct-API.md b/docs/wiki/api-references/csp.Struct-API.md index 50a3a1ede..816f2a5af 100644 --- a/docs/wiki/api-references/csp.Struct-API.md +++ b/docs/wiki/api-references/csp.Struct-API.md @@ -159,7 +159,7 @@ By default, CSP Struct objects allow any field to be unset. This means that acce - **Default:** `True` (for backwards compatibility) - **When set to `False`:** Enforces *strict struct semantics* — required fields must be set at initialization. - - Note that `del struct.field` is not allowed for stric structs — this will raise an error. As a result, for any strict struct, `hasattr(struct, field)` always returns True for any defined field. + - Note that `del struct.field` is not allowed for strict structs — this will raise an error. As a result, for any strict struct, `hasattr(struct, field)` always returns True for any defined field. - A non-strict struct may not inherit (directly or indirectly) from a strict base. ### Semantics when `allow_unset=False` From c343b5a5c451b16b0b3ca42091d2be162dc16b1d Mon Sep 17 00:00:00 2001 From: Adam Glustein Date: Wed, 19 Nov 2025 18:09:24 -0500 Subject: [PATCH 7/9] Store optionality on the field object, and embed whether or not the field is set to None in the struct's bitmask Signed-off-by: Adam Glustein --- cpp/csp/engine/Struct.cpp | 187 ++++++++++++------ cpp/csp/engine/Struct.h | 81 ++++++-- cpp/csp/python/PyStruct.cpp | 59 +++--- cpp/csp/python/PyStructToDict.cpp | 24 ++- cpp/csp/python/PyStructToJson.cpp | 21 +- csp/impl/struct.py | 16 +- csp/tests/test_strict_structs.py | 213 ++++++++++----------- docs/wiki/api-references/csp.Struct-API.md | 22 ++- 8 files changed, 380 insertions(+), 243 deletions(-) diff --git a/cpp/csp/engine/Struct.cpp b/cpp/csp/engine/Struct.cpp index 17ea02ec4..3c327aee7 100644 --- a/cpp/csp/engine/Struct.cpp +++ b/cpp/csp/engine/Struct.cpp @@ -9,15 +9,19 @@ namespace csp { StructField::StructField( CspTypePtr type, const std::string & fieldname, - size_t size, size_t alignment ) : + size_t size, size_t alignment, bool isOptional ) : m_fieldname( fieldname ), m_offset( 0 ), m_size( size ), m_alignment( alignment ), m_maskOffset( 0 ), + m_noneMaskOffset( 0 ), m_maskBit( 0 ), + m_noneMaskBit( 0 ), m_maskBitMask( 0 ), - m_type( type ) + m_noneMaskBitMask( 0 ), + m_type( type ), + m_isOptional( isOptional ) { } @@ -37,10 +41,11 @@ and adjustments required for the hidden fields */ StructMeta::StructMeta( const std::string & name, const Fields & fields, bool isStrict, - std::shared_ptr base ) : m_name( name ), m_base( base ), m_isStrict( isStrict ), m_fields( fields ), + std::shared_ptr base ) : m_name( name ), m_base( base ), m_fields( fields ), m_size( 0 ), m_partialSize( 0 ), m_partialStart( 0 ), m_nativeStart( 0 ), m_basePadding( 0 ), m_maskLoc( 0 ), m_maskSize( 0 ), m_firstPartialField( 0 ), m_firstNativePartialField( 0 ), - m_isPartialNative( true ), m_isFullyNative( true ) + m_optionalFieldsSetBits( nullptr ), m_optionalFieldsNoneBits( nullptr ), m_isPartialNative( true ), + m_isFullyNative( true ), m_isStrict( isStrict ) { if( m_fields.empty() && !m_base) CSP_THROW( TypeError, "Struct types must define at least 1 field" ); @@ -98,20 +103,70 @@ StructMeta::StructMeta( const std::string & name, const Fields & fields, bool is //Setup masking bits for our fields //NOTE we can be more efficient by sticking masks into any potential alignment gaps, dont want to spend time on it //at this point - m_maskSize = !m_fields.empty() ? 1 + ( ( m_fields.size() - 1 ) / 8 ) : 0; + + size_t optionalFieldCount = std::count_if( m_fields.begin(), m_fields.end(), []( const auto & f ) { return f -> isOptional(); } ); + + m_maskSize = !m_fields.empty() ? 1 + ( ( m_fields.size() + optionalFieldCount - 1 ) / 8 ) : 0; m_size = offset + m_maskSize; m_partialSize = m_size - baseSize; m_maskLoc = m_size - m_maskSize; + + uint8_t numRemainingBits = ( m_fields.size() - m_firstPartialField + optionalFieldCount ) % 8; + m_lastByteMask = ( 1u << numRemainingBits ) - 1; size_t maskLoc = m_maskLoc; uint8_t maskBit = 0; - for( auto & f : m_fields ) + + // Set optional fields first so that their 2-bits never cross a byte boundary + m_optionalFieldsSetBits = new uint8_t[ m_maskSize ](); + m_optionalFieldsNoneBits = new uint8_t[ m_maskSize ](); + for( size_t i = 0; i < m_fields.size(); ++i ) { - f -> setMaskOffset( maskLoc, maskBit ); - if( ++maskBit == 8 ) + auto & f = m_fields[ i ]; + if( f -> isOptional() ) { - maskBit = 0; - ++maskLoc; + f -> setMaskOffset( maskLoc, maskBit ); + f -> setNoneMaskOffset( maskLoc, ++maskBit ); // use adjacent bit for None bit + if( ++maskBit == 8 ) + { + m_optionalFieldsSetBits[ maskLoc - m_maskLoc ] = 0xAA; + m_optionalFieldsNoneBits[ maskLoc - m_maskLoc ] = 0x55; + maskBit = 0; + ++maskLoc; + } + } + } + // deal with last (partial) byte filled with optional fields + auto lastOptIndex = maskLoc - m_maskLoc; + switch( maskBit / 2 ) + { + case 1: + m_optionalFieldsSetBits[ lastOptIndex ] = 0x1; + m_optionalFieldsNoneBits[ lastOptIndex ] = 0x2; + break; + case 2: + m_optionalFieldsSetBits[ lastOptIndex ] = 0x5; + m_optionalFieldsNoneBits[ lastOptIndex ] = 0xA; + break; + case 3: + m_optionalFieldsSetBits[ lastOptIndex ] = 0x15; + m_optionalFieldsNoneBits[ lastOptIndex ] = 0x2A; + break; + default: + break; // default initialized to 0 + } + + for( size_t i = 0; i < m_fields.size(); ++i ) + { + auto & f = m_fields[ i ]; + if( !f -> isOptional() ) + { + f -> setMaskOffset( maskLoc, maskBit ); + if( ++maskBit == 8 ) + { + maskBit = 0; + ++maskLoc; + } } } @@ -132,22 +187,21 @@ StructMeta::StructMeta( const std::string & name, const Fields & fields, bool is 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() ) + // The complete inheritance hierarchy must agree on strict/non-strict + if( m_base && m_isStrict != m_base -> isStrict() ) { - 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() << "'" ); + CSP_THROW( ValueError, + "Struct " << m_name << " was declared " << ( m_isStrict ? "strict" : "non-strict" ) << " but derives from " + << m_base -> name() << " which is " << ( m_base -> isStrict() ? "strict" : "non-strict" ) + ); } } StructMeta::~StructMeta() { m_default.reset(); + delete[] m_optionalFieldsSetBits; + delete[] m_optionalFieldsNoneBits; } Struct * StructMeta::createRaw() const @@ -468,53 +522,66 @@ void StructMeta::clear( Struct * s ) const m_base -> clear( s ); } +std::string StructMeta::formatAllUnsetStrictFields( const Struct * s ) const +{ + bool first = true; + std::stringstream ss; + ss << "["; + + for( size_t i = 0; i < m_fields.size(); ++i ) + { + const auto & f = m_fields[ i ]; + if( !f -> isSet( s ) && !f -> isNone( s ) ) + { + if( !first ) + ss << ", "; + else + first = false; + ss << f -> fieldname(); + } + } + ss << "]"; + return ss.str(); +} + bool StructMeta::allFieldsSet( const Struct * s ) const { - size_t numLocalFields = m_fields.size() - m_firstPartialField; - uint8_t numRemainingBits = numLocalFields % 8; + /* + We can use bit operations to validate the struct. + 1. Let M1 be the bitmask with 1s that align with the set bit of optional fields and + 2. Let M2 be the bitmask with 1s that align with the none bit of optional fields. + -- Both M1 and M2 are computed trivially when we create the meta. + 3. Let M1* = M1 & mask. M1* now is the bitmask of all set optional fields. + 4. Similarly, let M2* = M2 & mask, such that M2* is the bitmask of all none optional fields. + 5. Let M3 = mask | (M1* << 1) | (M2* >> 1). Since the shifted set/none bitsets will fill in that optional fields other bit, + the struct can validate iff M3 is all 1s. + + Notes: + 1) We do this on a byte by byte basis currently. If we add 32/64 bit padding to the struct mask, we could do this as one single step for most structs. + 2) There is an edge condition if a) the set bit of an optional field is the last bit of a byte or b) the none bit of an optional field is the first bit of a byte. + So, when we create the meta, we ensure this never happens by being smart about the ordering of fields in the mask. + */ const uint8_t * m = reinterpret_cast( s ) + m_maskLoc; - const uint8_t * e = m + m_maskSize - bool( numRemainingBits ); - for( ; m < e; ++m ) + const uint8_t * e = m + m_maskSize - bool( m_lastByteMask ); + + const uint8_t * M1 = m_optionalFieldsSetBits; + const uint8_t * M2 = m_optionalFieldsNoneBits; + + for( ; m < e; ++m, ++M1, ++M2 ) { - if( *m != 0xFF ) + if( ( *m | ( ( *m & *M1 ) << 1 ) | ( ( *m & *M2 ) >> 1 ) ) != 0xFF ) return false; } - if( numRemainingBits > 0 ) + if( m_lastByteMask ) { - uint8_t bitmask = ( 1u << numRemainingBits ) - 1; - if( ( *m & bitmask ) != bitmask ) + uint8_t masked = *m & m_lastByteMask; + if( ( masked | ( ( ( masked & *M1 ) << 1 ) & m_lastByteMask ) | ( ( masked & *M2 ) >> 1 ) ) != m_lastByteMask ) return false; } - return true; -} -std::string StructMeta::formatAllUnsetStrictFields( const Struct * s ) const -{ - bool first = true; - std::stringstream ss; - ss << "["; - - for ( const StructMeta * cur = this; cur; cur = cur -> m_base.get() ) - { - if ( !cur -> isStrict() ) - continue; - - for( size_t i = cur -> m_firstPartialField; i < cur -> m_fields.size(); ++i ) - { - if( !cur -> m_fields[ i ] -> isSet( s ) ) - { - if( !first ) - ss << ", "; - else - first = false; - ss << cur -> m_fields[ i ] -> fieldname(); - } - } - } - ss << "]"; - return ss.str(); + return m_base ? m_base -> allFieldsSet( s ) : true; } void StructMeta::destroy( Struct * s ) const @@ -536,18 +603,16 @@ void StructMeta::destroy( Struct * s ) const } [[nodiscard]] bool StructMeta::validate( const Struct * s ) const -{ +{ + if( !s -> meta() -> isStrict() ) + return true; + for ( const StructMeta * cur = this; cur; cur = cur -> m_base.get() ) { - if ( !cur -> isStrict() ) - continue; - - // Note that we do not recursively validate nested struct. - // We assume after any creation on the C++ side, these structs - // are validated properly prior to being set as field values if ( !cur -> allFieldsSet( s ) ) return false; } + return true; } diff --git a/cpp/csp/engine/Struct.h b/cpp/csp/engine/Struct.h index 74401034e..3bd312284 100644 --- a/cpp/csp/engine/Struct.h +++ b/cpp/csp/engine/Struct.h @@ -22,6 +22,8 @@ class StructField { public: + static constexpr uint8_t BITS_PER_BYTE = 8; + virtual ~StructField() {} const std::string & fieldname() const { return m_fieldname; } @@ -30,27 +32,55 @@ class StructField size_t size() const { return m_size; } //size of field in bytes size_t alignment() const { return m_alignment; } //alignment of the field size_t maskOffset() const { return m_maskOffset; } //offset to location of the mask byte fo this field from start of struct mem - uint8_t maskBit() const { return m_maskBit; } //bit within mask byte associated with this field + uint8_t maskBit() const { return m_maskBit; } //bit within mask byte associated with this field. Just used for debugging uint8_t maskBitMask() const { return m_maskBitMask; } //same as maskBit but as a mask ( 1 << bit + size_t noneMaskOffset() const { return m_noneMaskOffset; } //same fields as above but for the None bit, if this an optional field + uint8_t noneMaskBit() const { return m_noneMaskBit; } + uint8_t noneMaskBitMask() const { return m_noneMaskBitMask; } bool isNative() const { return m_type -> type() <= CspType::Type::MAX_NATIVE_TYPE; } + bool isOptional() const { return m_isOptional; } void setOffset( size_t off ) { m_offset = off; } void setMaskOffset( size_t off, uint8_t bit ) { - CSP_ASSERT( bit < 8 ); + CSP_ASSERT( bit < BITS_PER_BYTE ); m_maskOffset = off; m_maskBit = bit; m_maskBitMask = 1 << bit; } + void setNoneMaskOffset( size_t off, uint8_t bit ) + { + CSP_ASSERT( m_isOptional ); + CSP_ASSERT( bit < BITS_PER_BYTE ); + + m_noneMaskOffset = off; + m_noneMaskBit = bit; + m_noneMaskBitMask = 1 << bit; + } + bool isSet( const Struct * s ) const { const uint8_t * m = reinterpret_cast( s ) + m_maskOffset; return (*m ) & m_maskBitMask; } + bool isNone( const Struct * s ) const + { + const uint8_t * m = reinterpret_cast( s ) + m_noneMaskOffset; + return ( *m ) & m_noneMaskBitMask; + } + + void setNone( Struct * s ) const + { + CSP_ASSERT( m_isOptional ); + + uint8_t * m = reinterpret_cast( s ) + m_noneMaskOffset; + (*m) |= m_noneMaskBitMask; + } + //copy methods need not deal with mask set/unset, only copy values virtual void copyFrom( const Struct * src, Struct * dest ) const = 0; @@ -75,12 +105,14 @@ class StructField protected: StructField( CspTypePtr type, const std::string & fieldname, - size_t size, size_t alignment ); + size_t size, size_t alignment, bool optional ); void setIsSet( Struct * s ) const { uint8_t * m = reinterpret_cast( s ) + m_maskOffset; (*m) |= m_maskBitMask; + + clearIsNone( s ); // no-op if not an optional field } const void * valuePtr( const Struct * s ) const @@ -99,15 +131,25 @@ class StructField (*m) &= ~m_maskBitMask; } + void clearIsNone( Struct * s ) const + { + uint8_t * m = reinterpret_cast( s ) + m_noneMaskOffset; + (*m) &= ~m_noneMaskBitMask; + } + private: std::string m_fieldname; size_t m_offset; const size_t m_size; const size_t m_alignment; size_t m_maskOffset; + size_t m_noneMaskOffset; uint8_t m_maskBit; + uint8_t m_noneMaskBit; uint8_t m_maskBitMask; + uint8_t m_noneMaskBitMask; CspTypePtr m_type; + bool m_isOptional; }; using StructFieldPtr = std::shared_ptr; @@ -120,7 +162,7 @@ class NativeStructField : public StructField public: NativeStructField() {} - NativeStructField( const std::string & fieldname ) : NativeStructField( CspType::fromCType::type(), fieldname ) + NativeStructField( const std::string & fieldname, bool optional ) : NativeStructField( CspType::fromCType::type(), fieldname, optional ) { } @@ -157,7 +199,8 @@ class NativeStructField : public StructField } protected: - NativeStructField( CspTypePtr type, const std::string & fieldname ) : StructField( type, fieldname, sizeof( T ), alignof( T ) ) + NativeStructField( CspTypePtr type, const std::string & fieldname, bool optional ) + : StructField( type, fieldname, sizeof( T ), alignof( T ), optional ) {} }; @@ -179,7 +222,7 @@ using TimeStructField = NativeStructField