Skip to content
Open
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/wiki-lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ jobs:
os:
- ubuntu-24.04
python-version:
- 3.9
- 3.11

runs-on: ${{ matrix.os }}

Expand Down
11 changes: 9 additions & 2 deletions cpp/csp/adapters/websocket/ClientAdapterManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
6 changes: 6 additions & 0 deletions cpp/csp/cppnodes/baselibimpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -705,6 +705,9 @@ DECLARE_CPPNODE( struct_fromts )
);
}

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 ) );
}

Expand Down Expand Up @@ -758,6 +761,9 @@ DECLARE_CPPNODE( struct_collectts )
}
);
}

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 ) );
}
Expand Down
7 changes: 7 additions & 0 deletions cpp/csp/engine/InputAdapter.h
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#include <csp/core/Time.h>
#include <csp/engine/Enums.h>
#include <csp/engine/RootEngine.h>
#include <csp/engine/Struct.h>
#include <csp/engine/TimeSeriesProvider.h>

namespace csp
Expand Down Expand Up @@ -55,6 +56,12 @@ class InputAdapter : public TimeSeriesProvider, public EngineOwned
template<typename T>
bool InputAdapter::consumeTick( const T & value )
{
if constexpr( CspType::Type::fromCType<T>::type == CspType::TypeTraits::STRUCT )
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there a central place we can do this check on push rather than consume? Will be much harder to track down on consumption

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did not see such a place - we could add it to each adapter type (push, pull, pushpull and managers) separately in their pushTick impls, but I think it's cleaner to have the validation logic in one spot.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, but my point stands that it would be a lot harder to tell where the error is coming from
Also unfortunate to vall validate on all input adapters, even python based ones which were already validated due to PyStruct creation in python

{
if( unlikely( !( value -> validate() ) ) )
CSP_THROW( ValueError, "Struct " << value -> meta() -> name() << " is not valid; required fields " << value -> formatAllUnsetStrictFields() << " were not set on init" );
}

switch( pushMode() )
{
case PushMode::LAST_VALUE:
Expand Down
63 changes: 60 additions & 3 deletions cpp/csp/engine/Struct.cpp
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
#include <csp/core/System.h>
#include <csp/engine/Struct.h>
#include <algorithm>
#include <ranges>
#include <string>
#include <sstream>

namespace csp
{
Expand Down Expand Up @@ -33,8 +36,8 @@ and adjustments required for the hidden fields

*/

StructMeta::StructMeta( const std::string & name, const Fields & fields,
std::shared_ptr<StructMeta> base ) : m_name( name ), m_base( base ), m_fields( fields ),
StructMeta::StructMeta( const std::string & name, const Fields & fields, bool isStrict,
std::shared_ptr<StructMeta> 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 )
Expand Down Expand Up @@ -128,6 +131,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()
Expand Down Expand Up @@ -472,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
Expand All @@ -494,6 +535,22 @@ 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<const StructMeta> & meta )
{
//Initialize meta shared_ptr
Expand Down
21 changes: 19 additions & 2 deletions cpp/csp/engine/Struct.h
Original file line number Diff line number Diff line change
Expand Up @@ -587,21 +587,24 @@ class StructMeta : public std::enable_shared_from_this<StructMeta>
using FieldNames = std::vector<std::string>;

//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<StructMeta> base = nullptr );
StructMeta( const std::string & name, const Fields & fields, bool isStrict, std::shared_ptr<StructMeta> base = nullptr );
virtual ~StructMeta();

const std::string & name() const { return m_name; }
size_t size() const { return m_size; }
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; }

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;
Expand Down Expand Up @@ -629,6 +632,9 @@ class StructMeta : public std::enable_shared_from_this<StructMeta>
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;

Expand All @@ -652,7 +658,8 @@ class StructMeta : public std::enable_shared_from_this<StructMeta>
std::shared_ptr<StructMeta> 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;
Expand Down Expand Up @@ -738,6 +745,16 @@ class Struct
return meta() -> allFieldsSet( this );
}

[[nodiscard]] bool validate() const
{
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; }
Expand Down
26 changes: 21 additions & 5 deletions cpp/csp/python/PyStruct.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 )
{}


Expand All @@ -42,8 +42,8 @@ class PyObjectStructField final : public DialectGenericStructField
};

DialectStructMeta::DialectStructMeta( PyTypeObject * pyType, const std::string & name,
const Fields & flds, std::shared_ptr<StructMeta> base ) :
StructMeta( name, flds, base ),
const Fields & flds, bool isStrict, std::shared_ptr<StructMeta> base ) :
StructMeta( name, flds, isStrict, base ),
m_pyType( PyTypeObjectPtr::incref( pyType ) )
{
}
Expand Down Expand Up @@ -110,12 +110,18 @@ 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 ) );

Expand Down Expand Up @@ -189,7 +195,12 @@ static PyObject * PyStructMeta_new( PyTypeObject *subtype, PyObject *args, PyObj
| |
PyStruct --------------------------
*/
auto structMeta = std::make_shared<DialectStructMeta>( ( 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<DialectStructMeta>( ( PyTypeObject * ) pymeta, name, fields, isStrict, metabase );

//Setup fast attr dict lookup
pymeta -> attrDict = PyObjectPtr::own( PyDict_New() );
Expand Down Expand Up @@ -457,6 +468,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 )
Expand Down Expand Up @@ -796,6 +810,8 @@ int PyStruct_init( PyStruct * self, PyObject * args, PyObject * kwargs )
CSP_BEGIN_METHOD;

PyStruct_setattrs( self, args, kwargs, "__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;
}
Expand Down
2 changes: 1 addition & 1 deletion cpp/csp/python/PyStruct.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ class CSPTYPESIMPL_EXPORT DialectStructMeta : public StructMeta
{
public:
DialectStructMeta( PyTypeObject * pyType, const std::string & name,
const Fields & fields, std::shared_ptr<StructMeta> base = nullptr );
const Fields & fields, bool isStrict, std::shared_ptr<StructMeta> base = nullptr );
~DialectStructMeta() {}

PyTypeObject * pyType() const { return m_pyType.get(); }
Expand Down
22 changes: 19 additions & 3 deletions csp/impl/struct.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {}
Expand All @@ -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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 ]
Expand Down Expand Up @@ -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):
Expand Down
7 changes: 7 additions & 0 deletions csp/impl/types/typing_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,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
Expand Down
Loading
Loading