revert and simplify optional field interface

Simon Beyzerov · Simon Beyzerov · commit 4acbb08a9c14 · 2025-08-13T15:18:02.000-04:00
diff --git a/cpp/csp/engine/Struct.cpp b/cpp/csp/engine/Struct.cpp
@@ -8,7 +8,7 @@ namespace csp
 {
 
 StructField::StructField( CspTypePtr type, const std::string & fieldname, 
-                          size_t size, size_t alignment, bool isOptional ) :
+                          size_t size, size_t alignment ) :
     m_fieldname( fieldname ),
     m_offset( 0 ),
     m_size( size ),
@@ -17,7 +17,7 @@ StructField::StructField( CspTypePtr type, const std::string & fieldname,
     m_maskBit( 0 ),
     m_maskBitMask( 0 ),
     m_type( type ),
-    m_isOptional( isOptional )
+    m_isOptional( 1 )
 {
 }
 
diff --git a/cpp/csp/engine/Struct.h b/cpp/csp/engine/Struct.h
@@ -47,6 +47,11 @@ class StructField
         m_maskBitMask = 1 << bit;
     }
 
+    void setRequired()
+    {
+        m_isOptional = false;
+    }
+
     bool isSet( const Struct * s ) const
     {
         const uint8_t * m = reinterpret_cast<const uint8_t *>( s ) + m_maskOffset;
@@ -77,7 +82,7 @@ class StructField
 protected:
 
     StructField( CspTypePtr type, const std::string & fieldname,
-                 size_t size, size_t alignment, bool isOptional );
+                 size_t size, size_t alignment );
 
     void setIsSet( Struct * s ) const
     {
@@ -110,7 +115,7 @@ class StructField
     uint8_t      m_maskBit;
     uint8_t      m_maskBitMask;
     CspTypePtr   m_type;
-    const bool   m_isOptional;
+    bool         m_isOptional;
 };
 
 using StructFieldPtr = std::shared_ptr<StructField>;
@@ -123,7 +128,7 @@ class NativeStructField : public StructField
 
 public:
     NativeStructField() {}
-    NativeStructField( const std::string & fieldname, bool isOptional ) : NativeStructField( CspType::fromCType<T>::type(), fieldname, isOptional )
+    NativeStructField( const std::string & fieldname ) : NativeStructField( CspType::fromCType<T>::type(), fieldname )
     {
     }
 
@@ -160,7 +165,7 @@ class NativeStructField : public StructField
     }
 
 protected:
-    NativeStructField( CspTypePtr type, const std::string & fieldname, bool isOptional ) : StructField( type, fieldname, sizeof( T ), alignof( T ), isOptional )
+    NativeStructField( CspTypePtr type, const std::string & fieldname ) : StructField( type, fieldname, sizeof( T ), alignof( T ) )
     {}
 };
 
@@ -182,7 +187,7 @@ using TimeStructField      = NativeStructField<Time>;
 class CspEnumStructField final : public NativeStructField<CspEnum>
 {
 public:
-    CspEnumStructField( CspTypePtr type, const std::string & fieldname, bool isOptional ) : NativeStructField( type, fieldname, isOptional )
+    CspEnumStructField( CspTypePtr type, const std::string & fieldname ) : NativeStructField( type, fieldname )
     {}
 };
 
@@ -221,8 +226,8 @@ class NotImplementedStructField : public StructField
 class NonNativeStructField : public StructField
 {
 public:
-    NonNativeStructField( CspTypePtr type, const std::string &fieldname, size_t size, size_t alignment, bool isOptional ) :
-        StructField( type, fieldname, size, alignment, isOptional )
+    NonNativeStructField( CspTypePtr type, const std::string &fieldname, size_t size, size_t alignment ) :
+        StructField( type, fieldname, size, alignment )
     {}
 
     virtual void initialize( Struct * s ) const = 0;
@@ -247,8 +252,8 @@ class StringStructField final : public NonNativeStructField
 public:
     using CType = csp::CspType::StringCType;
 
-    StringStructField( CspTypePtr type, const std::string & fieldname, bool isOptional ) :
-        NonNativeStructField( type, fieldname, sizeof( CType ), alignof( CType ), isOptional )
+    StringStructField( CspTypePtr type, const std::string & fieldname ) :
+        NonNativeStructField( type, fieldname, sizeof( CType ), alignof( CType ) )
     {}
 
     void initialize( Struct * s ) const override
@@ -346,8 +351,8 @@ class ArrayStructField : public NonNativeStructField
     }
 
 public:
-    ArrayStructField( CspTypePtr arrayType, const std::string & fieldname, bool isOptional ) :
-        NonNativeStructField( arrayType, fieldname, sizeof( CType ), alignof( CType ), isOptional )
+    ArrayStructField( CspTypePtr arrayType, const std::string & fieldname ) :
+        NonNativeStructField( arrayType, fieldname, sizeof( CType ), alignof( CType ) )
     {}
 
     const CType & value( const Struct * s ) const
@@ -424,8 +429,8 @@ class ArrayStructField : public NonNativeStructField
 class DialectGenericStructField : public NonNativeStructField
 {
 public:
-    DialectGenericStructField( const std::string & fieldname, size_t size, size_t alignment, bool isOptional ) :
-        NonNativeStructField( CspType::DIALECT_GENERIC(), fieldname, size, alignment, isOptional )
+    DialectGenericStructField( const std::string & fieldname, size_t size, size_t alignment ) :
+        NonNativeStructField( CspType::DIALECT_GENERIC(), fieldname, size, alignment )
     {}
 
     const DialectGenericType & value( const Struct * s ) const
@@ -834,8 +839,8 @@ bool TypedStructPtr<T>::operator==( const TypedStructPtr<T> & rhs ) const
 class StructStructField final : public NonNativeStructField
 {
 public:
-    StructStructField( CspTypePtr cspType, const std::string &fieldname, bool isOptional ) :
-        NonNativeStructField( cspType, fieldname, sizeof( StructPtr ), alignof( StructPtr ), isOptional )
+    StructStructField( CspTypePtr cspType, const std::string &fieldname ) :
+        NonNativeStructField( cspType, fieldname, sizeof( StructPtr ), alignof( StructPtr ) )
     {
         CSP_ASSERT( cspType -> type() == CspType::Type::STRUCT );
         m_meta = std::static_pointer_cast<const CspStructType>( cspType ) -> meta();
diff --git a/cpp/csp/python/PyStruct.cpp b/cpp/csp/python/PyStruct.cpp
@@ -20,8 +20,7 @@ class PyObjectStructField final : public DialectGenericStructField
 public:
     using BASE = DialectGenericStructField;
     PyObjectStructField( const std::string & name,
-                         PyTypeObjectPtr pytype,
-                         bool isOptional ) : BASE( name, sizeof( PyObjectPtr ), alignof( PyObjectPtr ), isOptional ),
+                         PyTypeObjectPtr pytype ) : BASE( name, sizeof( PyObjectPtr ), alignof( PyObjectPtr ) ),
                                              m_pytype( pytype )
     {}
 
@@ -120,9 +119,8 @@ static PyObject * PyStructMeta_new( PyTypeObject *subtype, PyObject *args, PyObj
             if( !keystr )
                 CSP_THROW( PythonPassthrough, "" );
 
-            if (!PySet_Check(optional_fields)) {
+            if (!PySet_Check(optional_fields)) 
                 CSP_THROW( TypeError, "Struct metadata for key " << keystr << " expected a set, got " << PyObjectPtr::incref( optional_fields ) );
-            }
 
             bool isOptional = PySet_Contains( optional_fields, key ) == 1;
 
@@ -134,33 +132,36 @@ static PyObject * PyStructMeta_new( PyTypeObject *subtype, PyObject *args, PyObj
 
             switch( csptype -> type() )
             {
-                case csp::CspType::Type::BOOL:      field = std::make_shared<BoolStructField>( keystr, isOptional ); break;
-                case csp::CspType::Type::INT64:     field = std::make_shared<Int64StructField>( keystr, isOptional ); break;
-                case csp::CspType::Type::DOUBLE:    field = std::make_shared<DoubleStructField>( keystr, isOptional ); break;
-                case csp::CspType::Type::DATETIME:  field = std::make_shared<DateTimeStructField>( keystr, isOptional ); break;
-                case csp::CspType::Type::TIMEDELTA: field = std::make_shared<TimeDeltaStructField>( keystr, isOptional ); break;
-                case csp::CspType::Type::DATE:      field = std::make_shared<DateStructField>( keystr, isOptional ); break;
-                case csp::CspType::Type::TIME:      field = std::make_shared<TimeStructField>( keystr, isOptional ); break;
-                case csp::CspType::Type::STRING:    field = std::make_shared<StringStructField>( csptype, keystr, isOptional ); break;
-                case csp::CspType::Type::ENUM:      field = std::make_shared<CspEnumStructField>( csptype, keystr, isOptional ); break;
-                case csp::CspType::Type::STRUCT:    field = std::make_shared<StructStructField>( csptype, keystr, isOptional ); break;
+                case csp::CspType::Type::BOOL:      field = std::make_shared<BoolStructField>( keystr ); break;
+                case csp::CspType::Type::INT64:     field = std::make_shared<Int64StructField>( keystr ); break;
+                case csp::CspType::Type::DOUBLE:    field = std::make_shared<DoubleStructField>( keystr ); break;
+                case csp::CspType::Type::DATETIME:  field = std::make_shared<DateTimeStructField>( keystr ); break;
+                case csp::CspType::Type::TIMEDELTA: field = std::make_shared<TimeDeltaStructField>( keystr ); break;
+                case csp::CspType::Type::DATE:      field = std::make_shared<DateStructField>( keystr ); break;
+                case csp::CspType::Type::TIME:      field = std::make_shared<TimeStructField>( keystr ); break;
+                case csp::CspType::Type::STRING:    field = std::make_shared<StringStructField>( csptype, keystr ); break;
+                case csp::CspType::Type::ENUM:      field = std::make_shared<CspEnumStructField>( csptype, keystr ); break;
+                case csp::CspType::Type::STRUCT:    field = std::make_shared<StructStructField>( csptype, keystr ); break;
                 case csp::CspType::Type::ARRAY:
                 {
                     const CspArrayType & arrayType = static_cast<const CspArrayType&>( *csptype );
-                    field = ArraySubTypeSwitch::invoke( arrayType.elemType(), [csptype,keystr,isOptional]( auto tag ) -> std::shared_ptr<StructField>
+                    field = ArraySubTypeSwitch::invoke( arrayType.elemType(), [csptype,keystr]( auto tag ) -> std::shared_ptr<StructField>
                     {
                         using CElemType = typename decltype(tag)::type;
                         using CType = typename CspType::Type::toCArrayType<CElemType>::type;
-                        return std::make_shared<ArrayStructField<CType>>( csptype, keystr, isOptional );
+                        return std::make_shared<ArrayStructField<CType>>( csptype, keystr );
                     } );
 
                     break;
                 }
 
-                case csp::CspType::Type::DIALECT_GENERIC: field = std::make_shared<PyObjectStructField>( keystr, PyTypeObjectPtr::incref( ( PyTypeObject * ) type ), isOptional ); break;
+                case csp::CspType::Type::DIALECT_GENERIC: field = std::make_shared<PyObjectStructField>( keystr, PyTypeObjectPtr::incref( ( PyTypeObject * ) type ) ); break;
                 default:
                     CSP_THROW( ValueError, "Unexpected csp type " << csptype -> type() << " on struct " << name );
             }
+            
+            if ( !isOptional )
+                field -> setRequired( );
 
             fields.emplace_back( field );
         }
@@ -362,31 +363,10 @@ static PyObject * PyStructMeta_metadata_info( PyStructMeta * m )
     return out.release();
 }
 
-int PyStruct_init( PyStruct * self, PyObject * args, PyObject * kwargs, bool validate );
-
-static PyObject * PyStructMeta_unvalidated__call__( PyObject * self, PyObject * args, PyObject * kwargs )
-{
-    CSP_BEGIN_METHOD;
-    PyTypeObject * type = (PyTypeObject*)self;
-
-    PyObject* instance = type->tp_new( type, args, kwargs );
-    if( !instance )
-        CSP_THROW( PythonPassthrough, "" );
-
-    if( PyStruct_init( (PyStruct*) instance, args, kwargs, false ) < 0 )
-    {
-        Py_DECREF( instance );
-        CSP_THROW( PythonPassthrough, "" );
-    }
-
-    return instance;
-    CSP_RETURN_NULL;
-}
 
 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"},
-    {"_unvalidated__call__", (PyCFunction) PyStructMeta_unvalidated__call__, METH_VARARGS | METH_KEYWORDS, "create and initialize a struct without underlying validation"},
     {NULL}
 };
 
@@ -836,27 +816,16 @@ PyObject * PyStruct_validate( PyStruct * self ) {
     CSP_RETURN_NONE;
 }
 
-int PyStruct_init( PyStruct * self, PyObject * args, PyObject * kwargs, bool validate )
+int PyStruct_init( PyStruct * self, PyObject * args, PyObject * kwargs )
 {
     CSP_BEGIN_METHOD;
 
     PyStruct_setattrs( self, args, kwargs, "__init__" );
-    if( validate )
-    {
-        PyObject * rv = PyStruct_validate( self );
-        if( !rv )
-            return -1;
-        Py_DECREF( rv );
-    }
+    self -> struct_ -> validate();
 
     CSP_RETURN_INT;
 }
 
-static int PyStruct_init_validated( PyStruct * self, PyObject * args, PyObject * kwargs )
-{
-    return PyStruct_init( self, args, kwargs, true );
-}
-
 PyObject * PyStruct_update( PyStruct * self, PyObject * args, PyObject * kwargs )
 {
     CSP_BEGIN_METHOD;
@@ -1060,7 +1029,6 @@ static PyMethodDef PyStruct_methods[] = {
     { "update_from",    (PyCFunction) PyStruct_update_from,    METH_O,      "update from struct. struct must be same type or a derived type. unset fields will be not be copied" },
     { "update",         (PyCFunction) PyStruct_update,         METH_VARARGS | METH_KEYWORDS, "update from key=val.  given fields will be set on struct.  other fields will remain as is in struct" },
     { "all_fields_set", (PyCFunction) PyStruct_all_fields_set, METH_NOARGS, "return true if all fields on the struct are set" },
-    { "validate",       (PyCFunction) PyStruct_validate,       METH_NOARGS, "validate the struct (strict struct fields set, etc...)" },
     { "to_dict",        (PyCFunction) PyStruct_to_dict,        METH_VARARGS | METH_KEYWORDS, "return a python dict of the struct by recursively converting struct members into python dicts" },
     { "to_json",        (PyCFunction) PyStruct_to_json,        METH_VARARGS | METH_KEYWORDS, "return a json string of the struct by recursively converting struct members into json format" },
     { NULL}
@@ -1103,7 +1071,7 @@ PyTypeObject PyStruct::PyType = {
     0,                         /* tp_descr_get */
     0,                         /* tp_descr_set */
     0,                         /* tp_dictoffset */
-    ( initproc ) PyStruct_init_validated,  /* tp_init */
+    ( initproc ) PyStruct_init,  /* tp_init */
     PyType_GenericAlloc,       /* tp_alloc */
     ( newfunc ) PyStruct_new,    /* tp_new */
     PyObject_GC_Del,           /* tp_free */
diff --git a/csp/impl/struct.py b/csp/impl/struct.py
@@ -250,13 +250,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._unvalidated__call__()
+            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))
-            res.validate()
+                obj_args[k] = cls._obj_from_python(v, expected_type)
+            res = obj_type(**obj_args)
             return res
         else:
             if isinstance(json, obj_type):

Original file line number	Diff line number	Diff line change
`@@ -8,7 +8,7 @@ namespace csp`
`8`	`8`	`{`
`9`	`9`
`10`	`10`	`StructField::StructField( CspTypePtr type, const std::string & fieldname,`
`11`		`- size_t size, size_t alignment, bool isOptional ) :`
	`11`	`+ size_t size, size_t alignment ) :`
`12`	`12`	`m_fieldname( fieldname ),`
`13`	`13`	`m_offset( 0 ),`
`14`	`14`	`m_size( size ),`
`@@ -17,7 +17,7 @@ StructField::StructField( CspTypePtr type, const std::string & fieldname,`
`17`	`17`	`m_maskBit( 0 ),`
`18`	`18`	`m_maskBitMask( 0 ),`
`19`	`19`	`m_type( type ),`
`20`		`- m_isOptional( isOptional )`
	`20`	`+ m_isOptional( 1 )`
`21`	`21`	`{`
`22`	`22`	`}`
`23`	`23`