Skip to content

Commit 9c84f30

Browse files
author
Simon Beyzerov
committed
revert and simplify optional field interface
1 parent 7c9fcff commit 9c84f30

4 files changed

Lines changed: 46 additions & 71 deletions

File tree

‎cpp/csp/engine/Struct.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ namespace csp
88
{
99

1010
StructField::StructField( CspTypePtr type, const std::string & fieldname,
11-
size_t size, size_t alignment, bool isOptional ) :
11+
size_t size, size_t alignment ) :
1212
m_fieldname( fieldname ),
1313
m_offset( 0 ),
1414
m_size( size ),
@@ -17,7 +17,7 @@ StructField::StructField( CspTypePtr type, const std::string & fieldname,
1717
m_maskBit( 0 ),
1818
m_maskBitMask( 0 ),
1919
m_type( type ),
20-
m_isOptional( isOptional )
20+
m_isOptional( 1 )
2121
{
2222
}
2323

‎cpp/csp/engine/Struct.h‎

Lines changed: 20 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,11 @@ class StructField
4747
m_maskBitMask = 1 << bit;
4848
}
4949

50+
void setRequired()
51+
{
52+
m_isOptional = false;
53+
}
54+
5055
bool isSet( const Struct * s ) const
5156
{
5257
const uint8_t * m = reinterpret_cast<const uint8_t *>( s ) + m_maskOffset;
@@ -77,7 +82,7 @@ class StructField
7782
protected:
7883

7984
StructField( CspTypePtr type, const std::string & fieldname,
80-
size_t size, size_t alignment, bool isOptional );
85+
size_t size, size_t alignment );
8186

8287
void setIsSet( Struct * s ) const
8388
{
@@ -110,7 +115,7 @@ class StructField
110115
uint8_t m_maskBit;
111116
uint8_t m_maskBitMask;
112117
CspTypePtr m_type;
113-
const bool m_isOptional;
118+
bool m_isOptional;
114119
};
115120

116121
using StructFieldPtr = std::shared_ptr<StructField>;
@@ -123,7 +128,7 @@ class NativeStructField : public StructField
123128

124129
public:
125130
NativeStructField() {}
126-
NativeStructField( const std::string & fieldname, bool isOptional ) : NativeStructField( CspType::fromCType<T>::type(), fieldname, isOptional )
131+
NativeStructField( const std::string & fieldname ) : NativeStructField( CspType::fromCType<T>::type(), fieldname )
127132
{
128133
}
129134

@@ -160,7 +165,7 @@ class NativeStructField : public StructField
160165
}
161166

162167
protected:
163-
NativeStructField( CspTypePtr type, const std::string & fieldname, bool isOptional ) : StructField( type, fieldname, sizeof( T ), alignof( T ), isOptional )
168+
NativeStructField( CspTypePtr type, const std::string & fieldname ) : StructField( type, fieldname, sizeof( T ), alignof( T ) )
164169
{}
165170
};
166171

@@ -182,7 +187,7 @@ using TimeStructField = NativeStructField<Time>;
182187
class CspEnumStructField final : public NativeStructField<CspEnum>
183188
{
184189
public:
185-
CspEnumStructField( CspTypePtr type, const std::string & fieldname, bool isOptional ) : NativeStructField( type, fieldname, isOptional )
190+
CspEnumStructField( CspTypePtr type, const std::string & fieldname ) : NativeStructField( type, fieldname )
186191
{}
187192
};
188193

@@ -221,8 +226,8 @@ class NotImplementedStructField : public StructField
221226
class NonNativeStructField : public StructField
222227
{
223228
public:
224-
NonNativeStructField( CspTypePtr type, const std::string &fieldname, size_t size, size_t alignment, bool isOptional ) :
225-
StructField( type, fieldname, size, alignment, isOptional )
229+
NonNativeStructField( CspTypePtr type, const std::string &fieldname, size_t size, size_t alignment ) :
230+
StructField( type, fieldname, size, alignment )
226231
{}
227232

228233
virtual void initialize( Struct * s ) const = 0;
@@ -247,8 +252,8 @@ class StringStructField final : public NonNativeStructField
247252
public:
248253
using CType = csp::CspType::StringCType;
249254

250-
StringStructField( CspTypePtr type, const std::string & fieldname, bool isOptional ) :
251-
NonNativeStructField( type, fieldname, sizeof( CType ), alignof( CType ), isOptional )
255+
StringStructField( CspTypePtr type, const std::string & fieldname ) :
256+
NonNativeStructField( type, fieldname, sizeof( CType ), alignof( CType ) )
252257
{}
253258

254259
void initialize( Struct * s ) const override
@@ -346,8 +351,8 @@ class ArrayStructField : public NonNativeStructField
346351
}
347352

348353
public:
349-
ArrayStructField( CspTypePtr arrayType, const std::string & fieldname, bool isOptional ) :
350-
NonNativeStructField( arrayType, fieldname, sizeof( CType ), alignof( CType ), isOptional )
354+
ArrayStructField( CspTypePtr arrayType, const std::string & fieldname ) :
355+
NonNativeStructField( arrayType, fieldname, sizeof( CType ), alignof( CType ) )
351356
{}
352357

353358
const CType & value( const Struct * s ) const
@@ -424,8 +429,8 @@ class ArrayStructField : public NonNativeStructField
424429
class DialectGenericStructField : public NonNativeStructField
425430
{
426431
public:
427-
DialectGenericStructField( const std::string & fieldname, size_t size, size_t alignment, bool isOptional ) :
428-
NonNativeStructField( CspType::DIALECT_GENERIC(), fieldname, size, alignment, isOptional )
432+
DialectGenericStructField( const std::string & fieldname, size_t size, size_t alignment ) :
433+
NonNativeStructField( CspType::DIALECT_GENERIC(), fieldname, size, alignment )
429434
{}
430435

431436
const DialectGenericType & value( const Struct * s ) const
@@ -834,8 +839,8 @@ bool TypedStructPtr<T>::operator==( const TypedStructPtr<T> & rhs ) const
834839
class StructStructField final : public NonNativeStructField
835840
{
836841
public:
837-
StructStructField( CspTypePtr cspType, const std::string &fieldname, bool isOptional ) :
838-
NonNativeStructField( cspType, fieldname, sizeof( StructPtr ), alignof( StructPtr ), isOptional )
842+
StructStructField( CspTypePtr cspType, const std::string &fieldname ) :
843+
NonNativeStructField( cspType, fieldname, sizeof( StructPtr ), alignof( StructPtr ) )
839844
{
840845
CSP_ASSERT( cspType -> type() == CspType::Type::STRUCT );
841846
m_meta = std::static_pointer_cast<const CspStructType>( cspType ) -> meta();

‎cpp/csp/python/PyStruct.cpp‎

Lines changed: 21 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,7 @@ class PyObjectStructField final : public DialectGenericStructField
2020
public:
2121
using BASE = DialectGenericStructField;
2222
PyObjectStructField( const std::string & name,
23-
PyTypeObjectPtr pytype,
24-
bool isOptional ) : BASE( name, sizeof( PyObjectPtr ), alignof( PyObjectPtr ), isOptional ),
23+
PyTypeObjectPtr pytype ) : BASE( name, sizeof( PyObjectPtr ), alignof( PyObjectPtr ) ),
2524
m_pytype( pytype )
2625
{}
2726

@@ -120,9 +119,8 @@ static PyObject * PyStructMeta_new( PyTypeObject *subtype, PyObject *args, PyObj
120119
if( !keystr )
121120
CSP_THROW( PythonPassthrough, "" );
122121

123-
if (!PySet_Check(optional_fields)) {
122+
if (!PySet_Check(optional_fields))
124123
CSP_THROW( TypeError, "Struct metadata for key " << keystr << " expected a set, got " << PyObjectPtr::incref( optional_fields ) );
125-
}
126124

127125
bool isOptional = PySet_Contains( optional_fields, key ) == 1;
128126

@@ -134,33 +132,36 @@ static PyObject * PyStructMeta_new( PyTypeObject *subtype, PyObject *args, PyObj
134132

135133
switch( csptype -> type() )
136134
{
137-
case csp::CspType::Type::BOOL: field = std::make_shared<BoolStructField>( keystr, isOptional ); break;
138-
case csp::CspType::Type::INT64: field = std::make_shared<Int64StructField>( keystr, isOptional ); break;
139-
case csp::CspType::Type::DOUBLE: field = std::make_shared<DoubleStructField>( keystr, isOptional ); break;
140-
case csp::CspType::Type::DATETIME: field = std::make_shared<DateTimeStructField>( keystr, isOptional ); break;
141-
case csp::CspType::Type::TIMEDELTA: field = std::make_shared<TimeDeltaStructField>( keystr, isOptional ); break;
142-
case csp::CspType::Type::DATE: field = std::make_shared<DateStructField>( keystr, isOptional ); break;
143-
case csp::CspType::Type::TIME: field = std::make_shared<TimeStructField>( keystr, isOptional ); break;
144-
case csp::CspType::Type::STRING: field = std::make_shared<StringStructField>( csptype, keystr, isOptional ); break;
145-
case csp::CspType::Type::ENUM: field = std::make_shared<CspEnumStructField>( csptype, keystr, isOptional ); break;
146-
case csp::CspType::Type::STRUCT: field = std::make_shared<StructStructField>( csptype, keystr, isOptional ); break;
135+
case csp::CspType::Type::BOOL: field = std::make_shared<BoolStructField>( keystr ); break;
136+
case csp::CspType::Type::INT64: field = std::make_shared<Int64StructField>( keystr ); break;
137+
case csp::CspType::Type::DOUBLE: field = std::make_shared<DoubleStructField>( keystr ); break;
138+
case csp::CspType::Type::DATETIME: field = std::make_shared<DateTimeStructField>( keystr ); break;
139+
case csp::CspType::Type::TIMEDELTA: field = std::make_shared<TimeDeltaStructField>( keystr ); break;
140+
case csp::CspType::Type::DATE: field = std::make_shared<DateStructField>( keystr ); break;
141+
case csp::CspType::Type::TIME: field = std::make_shared<TimeStructField>( keystr ); break;
142+
case csp::CspType::Type::STRING: field = std::make_shared<StringStructField>( csptype, keystr ); break;
143+
case csp::CspType::Type::ENUM: field = std::make_shared<CspEnumStructField>( csptype, keystr ); break;
144+
case csp::CspType::Type::STRUCT: field = std::make_shared<StructStructField>( csptype, keystr ); break;
147145
case csp::CspType::Type::ARRAY:
148146
{
149147
const CspArrayType & arrayType = static_cast<const CspArrayType&>( *csptype );
150-
field = ArraySubTypeSwitch::invoke( arrayType.elemType(), [csptype,keystr,isOptional]( auto tag ) -> std::shared_ptr<StructField>
148+
field = ArraySubTypeSwitch::invoke( arrayType.elemType(), [csptype,keystr]( auto tag ) -> std::shared_ptr<StructField>
151149
{
152150
using CElemType = typename decltype(tag)::type;
153151
using CType = typename CspType::Type::toCArrayType<CElemType>::type;
154-
return std::make_shared<ArrayStructField<CType>>( csptype, keystr, isOptional );
152+
return std::make_shared<ArrayStructField<CType>>( csptype, keystr );
155153
} );
156154

157155
break;
158156
}
159157

160-
case csp::CspType::Type::DIALECT_GENERIC: field = std::make_shared<PyObjectStructField>( keystr, PyTypeObjectPtr::incref( ( PyTypeObject * ) type ), isOptional ); break;
158+
case csp::CspType::Type::DIALECT_GENERIC: field = std::make_shared<PyObjectStructField>( keystr, PyTypeObjectPtr::incref( ( PyTypeObject * ) type ) ); break;
161159
default:
162160
CSP_THROW( ValueError, "Unexpected csp type " << csptype -> type() << " on struct " << name );
163161
}
162+
163+
if ( !isOptional )
164+
field -> setRequired( );
164165

165166
fields.emplace_back( field );
166167
}
@@ -364,29 +365,10 @@ static PyObject * PyStructMeta_metadata_info( PyStructMeta * m )
364365

365366
int PyStruct_init( PyStruct * self, PyObject * args, PyObject * kwargs, bool validate );
366367

367-
static PyObject * PyStructMeta_unvalidated__call__( PyObject * self, PyObject * args, PyObject * kwargs )
368-
{
369-
CSP_BEGIN_METHOD;
370-
PyTypeObject * type = (PyTypeObject*)self;
371-
372-
PyObject* instance = type->tp_new( type, args, kwargs );
373-
if( !instance )
374-
CSP_THROW( PythonPassthrough, "" );
375-
376-
if( PyStruct_init( (PyStruct*) instance, args, kwargs, false ) < 0 )
377-
{
378-
Py_DECREF( instance );
379-
CSP_THROW( PythonPassthrough, "" );
380-
}
381-
382-
return instance;
383-
CSP_RETURN_NULL;
384-
}
385368

386369
static PyMethodDef PyStructMeta_methods[] = {
387370
{"_layout", (PyCFunction) PyStructMeta_layout, METH_NOARGS, "debug view of structs internal mem layout"},
388371
{"_metadata_info", (PyCFunction) PyStructMeta_metadata_info, METH_NOARGS, "provide detailed information about struct layout"},
389-
{"_unvalidated__call__", (PyCFunction) PyStructMeta_unvalidated__call__, METH_VARARGS | METH_KEYWORDS, "create and initialize a struct without underlying validation"},
390372
{NULL}
391373
};
392374

@@ -836,27 +818,16 @@ PyObject * PyStruct_validate( PyStruct * self ) {
836818
CSP_RETURN_NONE;
837819
}
838820

839-
int PyStruct_init( PyStruct * self, PyObject * args, PyObject * kwargs, bool validate )
821+
int PyStruct_init( PyStruct * self, PyObject * args, PyObject * kwargs )
840822
{
841823
CSP_BEGIN_METHOD;
842824

843825
PyStruct_setattrs( self, args, kwargs, "__init__" );
844-
if( validate )
845-
{
846-
PyObject * rv = PyStruct_validate( self );
847-
if( !rv )
848-
return -1;
849-
Py_DECREF( rv );
850-
}
826+
self -> struct_ -> validate();
851827

852828
CSP_RETURN_INT;
853829
}
854830

855-
static int PyStruct_init_validated( PyStruct * self, PyObject * args, PyObject * kwargs )
856-
{
857-
return PyStruct_init( self, args, kwargs, true );
858-
}
859-
860831
PyObject * PyStruct_update( PyStruct * self, PyObject * args, PyObject * kwargs )
861832
{
862833
CSP_BEGIN_METHOD;
@@ -1060,7 +1031,6 @@ static PyMethodDef PyStruct_methods[] = {
10601031
{ "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" },
10611032
{ "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" },
10621033
{ "all_fields_set", (PyCFunction) PyStruct_all_fields_set, METH_NOARGS, "return true if all fields on the struct are set" },
1063-
{ "validate", (PyCFunction) PyStruct_validate, METH_NOARGS, "validate the struct (strict struct fields set, etc...)" },
10641034
{ "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" },
10651035
{ "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" },
10661036
{ NULL}
@@ -1103,7 +1073,7 @@ PyTypeObject PyStruct::PyType = {
11031073
0, /* tp_descr_get */
11041074
0, /* tp_descr_set */
11051075
0, /* tp_dictoffset */
1106-
( initproc ) PyStruct_init_validated, /* tp_init */
1076+
( initproc ) PyStruct_init, /* tp_init */
11071077
PyType_GenericAlloc, /* tp_alloc */
11081078
( newfunc ) PyStruct_new, /* tp_new */
11091079
PyObject_GC_Del, /* tp_free */

‎csp/impl/struct.py‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -250,13 +250,13 @@ def _obj_from_python(cls, json, obj_type):
250250
elif issubclass(obj_type, Struct):
251251
if not isinstance(json, dict):
252252
raise TypeError("Representation of struct as json is expected to be of dict type")
253-
res = obj_type._unvalidated__call__()
253+
obj_args = {}
254254
for k, v in json.items():
255255
expected_type = obj_type.__full_metadata_typed__.get(k, None)
256256
if expected_type is None:
257257
raise KeyError(f"Unexpected key {k} for type {obj_type}")
258-
setattr(res, k, cls._obj_from_python(v, expected_type))
259-
res.validate()
258+
obj_args[k] = cls._obj_from_python(v, expected_type)
259+
res = obj_type(**obj_args)
260260
return res
261261
else:
262262
if isinstance(json, obj_type):

0 commit comments

Comments
 (0)