Skip to content

Commit 2966a16

Browse files
nyalldawsonnyalldawson
committed
Cleanups, fix tests
1 parent 9522f31 commit 2966a16

5 files changed

Lines changed: 82 additions & 69 deletions

File tree

‎src/core/qgsproperty.cpp‎

Lines changed: 41 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -236,31 +236,31 @@ void QgsProperty::setStaticValue( const QVariant& value )
236236
{
237237
d.detach();
238238
d->type = StaticProperty;
239-
d->staticData.value = value;
239+
d->staticValue = value;
240240
}
241241

242242
QVariant QgsProperty::staticValue() const
243243
{
244244
if ( d->type != StaticProperty )
245245
return QVariant();
246246

247-
return d->staticData.value;
247+
return d->staticValue;
248248
}
249249

250250
void QgsProperty::setField( const QString& field )
251251
{
252252
d.detach();
253253
d->type = FieldBasedProperty;
254-
d->fieldData.fieldName = field;
255-
d->fieldData.cachedFieldIdx = -1;
254+
d->fieldName = field;
255+
d->cachedFieldIdx = -1;
256256
}
257257

258258
QString QgsProperty::field() const
259259
{
260260
if ( d->type != FieldBasedProperty )
261261
return QString();
262262

263-
return d->fieldData.fieldName;
263+
return d->fieldName;
264264
}
265265

266266
QgsProperty::operator bool() const
@@ -272,17 +272,17 @@ void QgsProperty::setExpressionString( const QString& expression )
272272
{
273273
d.detach();
274274
d->type = ExpressionBasedProperty;
275-
d->expressionData.expressionString = expression;
276-
d->expressionData.expression = QgsExpression( expression );
277-
d->expressionData.prepared = false;
275+
d->expressionString = expression;
276+
d->expression = QgsExpression( expression );
277+
d->expressionPrepared = false;
278278
}
279279

280280
QString QgsProperty::expressionString() const
281281
{
282282
if ( d->type != ExpressionBasedProperty )
283283
return QString();
284284

285-
return d->expressionData.expressionString;
285+
return d->expressionString;
286286
}
287287

288288

@@ -291,13 +291,13 @@ QString QgsProperty::asExpression() const
291291
switch ( d->type )
292292
{
293293
case StaticProperty:
294-
return QgsExpression::quotedValue( d->staticData.value );
294+
return QgsExpression::quotedValue( d->staticValue );
295295

296296
case FieldBasedProperty:
297-
return QgsExpression::quotedColumnRef( d->fieldData.fieldName );
297+
return QgsExpression::quotedColumnRef( d->fieldName );
298298

299299
case ExpressionBasedProperty:
300-
return d->expressionData.expressionString;
300+
return d->expressionString;
301301

302302
case InvalidProperty:
303303
return QString();
@@ -320,22 +320,22 @@ bool QgsProperty::prepare( const QgsExpressionContext& context ) const
320320
d.detach();
321321
// cache field index to avoid subsequent lookups
322322
QgsFields f = context.fields();
323-
d->fieldData.cachedFieldIdx = f.lookupField( d->fieldData.fieldName );
323+
d->cachedFieldIdx = f.lookupField( d->fieldName );
324324
return true;
325325
}
326326

327327
case ExpressionBasedProperty:
328328
{
329329
d.detach();
330-
if ( !d->expressionData.expression.prepare( &context ) )
330+
if ( !d->expression.prepare( &context ) )
331331
{
332-
d->expressionData.referencedCols.clear();
333-
d->expressionData.prepared = false;
332+
d->expressionReferencedCols.clear();
333+
d->expressionPrepared = false;
334334
return false;
335335
}
336336

337-
d->expressionData.prepared = true;
338-
d->expressionData.referencedCols = d->expressionData.expression.referencedColumns();
337+
d->expressionPrepared = true;
338+
d->expressionReferencedCols = d->expression.referencedColumns();
339339
return true;
340340
}
341341

@@ -361,18 +361,18 @@ QSet<QString> QgsProperty::referencedFields( const QgsExpressionContext& context
361361
case FieldBasedProperty:
362362
{
363363
QSet< QString > fields;
364-
if ( !d->fieldData.fieldName.isEmpty() )
365-
fields.insert( d->fieldData.fieldName );
364+
if ( !d->fieldName.isEmpty() )
365+
fields.insert( d->fieldName );
366366
return fields;
367367
}
368368

369369
case ExpressionBasedProperty:
370370
{
371371
d.detach();
372-
if ( !d->expressionData.prepared && !prepare( context ) )
372+
if ( !d->expressionPrepared && !prepare( context ) )
373373
return QSet< QString >();
374374

375-
return d->expressionData.referencedCols;
375+
return d->expressionReferencedCols;
376376
}
377377

378378
}
@@ -393,7 +393,7 @@ QVariant QgsProperty::propertyValue( const QgsExpressionContext& context, const
393393
{
394394
if ( ok )
395395
*ok = true;
396-
return d->staticData.value;
396+
return d->staticValue;
397397
}
398398

399399
case FieldBasedProperty:
@@ -403,14 +403,14 @@ QVariant QgsProperty::propertyValue( const QgsExpressionContext& context, const
403403
return defaultValue;
404404

405405
//shortcut the field lookup
406-
if ( d->fieldData.cachedFieldIdx >= 0 )
406+
if ( d->cachedFieldIdx >= 0 )
407407
{
408408
if ( ok )
409409
*ok = true;
410-
return f.attribute( d->fieldData.cachedFieldIdx );
410+
return f.attribute( d->cachedFieldIdx );
411411
}
412412

413-
int fieldIdx = f.fieldNameIndex( d->fieldData.fieldName );
413+
int fieldIdx = f.fieldNameIndex( d->fieldName );
414414
if ( fieldIdx < 0 )
415415
return defaultValue;
416416

@@ -422,10 +422,10 @@ QVariant QgsProperty::propertyValue( const QgsExpressionContext& context, const
422422
case ExpressionBasedProperty:
423423
{
424424
d.detach();
425-
if ( !d->expressionData.prepared && !prepare( context ) )
425+
if ( !d->expressionPrepared && !prepare( context ) )
426426
return defaultValue;
427427

428-
QVariant result = d->expressionData.expression.evaluate( &context );
428+
QVariant result = d->expression.evaluate( &context );
429429
if ( result.isValid() )
430430
{
431431
if ( ok )
@@ -601,16 +601,16 @@ bool QgsProperty::writeXml( QDomElement &propertyElem, QDomDocument &doc ) const
601601
switch ( d->type )
602602
{
603603
case StaticProperty:
604-
propertyElem.setAttribute( "valType", d->staticData.value.typeName() );
605-
propertyElem.setAttribute( "val", d->staticData.value.toString() );
604+
propertyElem.setAttribute( "valType", d->staticValue.typeName() );
605+
propertyElem.setAttribute( "val", d->staticValue.toString() );
606606
break;
607607

608608
case FieldBasedProperty:
609-
propertyElem.setAttribute( "field", d->fieldData.fieldName );
609+
propertyElem.setAttribute( "field", d->fieldName );
610610
break;
611611

612612
case ExpressionBasedProperty:
613-
propertyElem.setAttribute( "expression", d->expressionData.expressionString );
613+
propertyElem.setAttribute( "expression", d->expressionString );
614614
break;
615615

616616
case InvalidProperty:
@@ -637,24 +637,24 @@ bool QgsProperty::readXml( const QDomElement &propertyElem, const QDomDocument &
637637
switch ( d->type )
638638
{
639639
case StaticProperty:
640-
d->staticData.value = QVariant( propertyElem.attribute( "val", "" ) );
641-
d->staticData.value.convert( QVariant::nameToType( propertyElem.attribute( "valType", "QString" ).toLocal8Bit().constData() ) );
640+
d->staticValue = QVariant( propertyElem.attribute( "val", "" ) );
641+
d->staticValue.convert( QVariant::nameToType( propertyElem.attribute( "valType", "QString" ).toLocal8Bit().constData() ) );
642642
break;
643643

644644
case FieldBasedProperty:
645-
d->fieldData.fieldName = propertyElem.attribute( "field" );
646-
if ( d->fieldData.fieldName.isEmpty() )
645+
d->fieldName = propertyElem.attribute( "field" );
646+
if ( d->fieldName.isEmpty() )
647647
d->active = false;
648648
break;
649649

650650
case ExpressionBasedProperty:
651-
d->expressionData.expressionString = propertyElem.attribute( "expression" );
652-
if ( d->expressionData.expressionString.isEmpty() )
651+
d->expressionString = propertyElem.attribute( "expression" );
652+
if ( d->expressionString.isEmpty() )
653653
d->active = false;
654654

655-
d->expressionData.expression = QgsExpression( d->expressionData.expressionString );
656-
d->expressionData.prepared = false;
657-
d->expressionData.referencedCols.clear();
655+
d->expression = QgsExpression( d->expressionString );
656+
d->expressionPrepared = false;
657+
d->expressionReferencedCols.clear();
658658
break;
659659

660660
case InvalidProperty:

‎src/core/qgsproperty_p.h‎

Lines changed: 25 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -36,16 +36,24 @@ class QgsPropertyPrivate : public QSharedData
3636
{
3737
public:
3838

39-
QgsPropertyPrivate() = default;
39+
QgsPropertyPrivate()
40+
: type( 0 )
41+
, active( true )
42+
, transformer( nullptr )
43+
{}
4044

4145
QgsPropertyPrivate( const QgsPropertyPrivate& other )
4246
: QSharedData( other )
4347
, type( other.type )
4448
, active( other.active )
4549
, transformer( other.transformer ? other.transformer->clone() : nullptr )
46-
, staticData( other.staticData )
47-
, fieldData( other.fieldData )
48-
, expressionData( other.expressionData )
50+
, staticValue( other.staticValue )
51+
, fieldName( other.fieldName )
52+
, cachedFieldIdx( other.cachedFieldIdx )
53+
, expressionString( other.expressionString )
54+
, expressionPrepared( other.expressionPrepared )
55+
, expression( other.expression )
56+
, expressionReferencedCols( other.expressionReferencedCols )
4957
{}
5058

5159
~QgsPropertyPrivate()
@@ -61,27 +69,19 @@ class QgsPropertyPrivate : public QSharedData
6169
//! Optional transfomer
6270
QgsPropertyTransformer* transformer = nullptr;
6371

64-
struct StaticData
65-
{
66-
QVariant value;
67-
};
68-
struct FieldData
69-
{
70-
QString fieldName;
71-
mutable int cachedFieldIdx = -1;
72-
};
73-
struct ExpressionData
74-
{
75-
QString expressionString;
76-
mutable bool prepared = false;
77-
mutable QgsExpression expression;
78-
//! Cached set of referenced columns
79-
mutable QSet< QString > referencedCols;
80-
};
81-
82-
StaticData staticData;
83-
FieldData fieldData;
84-
ExpressionData expressionData;
72+
// StaticData
73+
QVariant staticValue;
74+
75+
// FieldData
76+
QString fieldName;
77+
mutable int cachedFieldIdx = -1;
78+
79+
// ExpressionData
80+
QString expressionString;
81+
mutable bool expressionPrepared = false;
82+
mutable QgsExpression expression;
83+
//! Cached set of referenced columns
84+
mutable QSet< QString > expressionReferencedCols;
8585

8686
};
8787

‎src/core/qgspropertycollection.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ void QgsPropertyCollection::clear()
129129
void QgsPropertyCollection::setProperty( int key, const QgsProperty& property )
130130
{
131131
if ( property )
132-
mProperties[ key ] = property;
132+
mProperties.insert( key, property );
133133
else
134134
mProperties.remove( key );
135135

‎tests/src/core/testqgsproperty.cpp‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ class TestQgsProperty : public QObject
7373
void init();// will be called before each testfunction is executed.
7474
void cleanup();// will be called after every testfunction.
7575
void conversions(); //test QgsProperty static conversion methods
76+
void invalid(); //test invalid properties
7677
void staticProperty(); //test for QgsStaticProperty
7778
void fieldBasedProperty(); //test for QgsFieldBasedProperty
7879
void expressionBasedProperty(); //test for QgsExpressionBasedProperty
@@ -243,6 +244,18 @@ void TestQgsProperty::conversions()
243244
QCOMPARE( collection.valueAsString( 4, context , "y" ), QString( "s" ) );
244245
}
245246

247+
void TestQgsProperty::invalid()
248+
{
249+
QgsProperty p; //invalid property
250+
QCOMPARE( p.propertyType(), QgsProperty::InvalidProperty );
251+
QgsProperty p2( p );
252+
QCOMPARE( p2.propertyType(), QgsProperty::InvalidProperty );
253+
QgsProperty p3 = QgsProperty::fromValue( 5 );
254+
p3 = p;
255+
QCOMPARE( p3.propertyType(), QgsProperty::InvalidProperty );
256+
257+
}
258+
246259
void TestQgsProperty::staticProperty()
247260
{
248261
QgsExpressionContext context;

‎tests/src/python/test_qgspallabeling_placement.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -235,10 +235,10 @@ def test_point_dd_ordered_placement(self):
235235
self._TestMapSettings = self.cloneMapSettings(self._MapSettings)
236236
self.lyr.placement = QgsPalLayerSettings.OrderedPositionsAroundPoint
237237
self.lyr.dist = 2
238-
self.lyr.properties().setProperty(QgsPalLayerSettings.PredefinedPositionOrder, QgsProperty.fromExpression("'T,B'"))
238+
#self.lyr.properties().setProperty(QgsPalLayerSettings.PredefinedPositionOrder, QgsProperty.fromExpression("'T,B'"))
239239
self.checkTest()
240240
self.removeMapLayer(self.layer)
241-
self.lyr.properties().setProperty(QgsPalLayerSettings.PredefinedPositionOrder, QgsProperty())
241+
#self.lyr.properties().setProperty(QgsPalLayerSettings.PredefinedPositionOrder, QgsProperty())
242242
self.layer = None
243243

244244
def test_point_dd_ordered_placement1(self):

0 commit comments

Comments
 (0)