Skip to content

Commit

Permalink
Fix failing test
Browse files Browse the repository at this point in the history
  • Loading branch information
nyalldawson committed Jan 23, 2017
1 parent 2c24175 commit f0f0170
Show file tree
Hide file tree
Showing 5 changed files with 59 additions and 20 deletions.
2 changes: 1 addition & 1 deletion python/core/qgsproperty.sip
Expand Up @@ -108,7 +108,7 @@ class QgsProperty

virtual QSet< QString > referencedFields( const QgsExpressionContext& context = QgsExpressionContext() ) const;

QVariant value( const QgsExpressionContext& context, const QVariant& defaultValue = QVariant() ) const;
QVariant value( const QgsExpressionContext& context, const QVariant& defaultValue = QVariant(), bool* ok /Out/ = nullptr ) const;

QString valueAsString( const QgsExpressionContext& context, const QString& defaultString = QString(), bool* ok /Out/ = nullptr ) const;

Expand Down
64 changes: 50 additions & 14 deletions src/core/qgsproperty.cpp
Expand Up @@ -379,15 +379,22 @@ QSet<QString> QgsProperty::referencedFields( const QgsExpressionContext& context
return QSet<QString>();
}

QVariant QgsProperty::propertyValue( const QgsExpressionContext& context, const QVariant& defaultValue ) const
QVariant QgsProperty::propertyValue( const QgsExpressionContext& context, const QVariant& defaultValue, bool* ok ) const
{
if ( ok )
*ok = false;

if ( !d->active )
return defaultValue;

switch ( d->type )
{
case StaticProperty:
{
if ( ok )
*ok = true;
return d->staticData.value;
}

case FieldBasedProperty:
{
Expand All @@ -397,12 +404,18 @@ QVariant QgsProperty::propertyValue( const QgsExpressionContext& context, const

//shortcut the field lookup
if ( d->fieldData.cachedFieldIdx >= 0 )
{
if ( ok )
*ok = true;
return f.attribute( d->fieldData.cachedFieldIdx );
}

int fieldIdx = f.fieldNameIndex( d->fieldData.fieldName );
if ( fieldIdx < 0 )
return defaultValue;

if ( ok )
*ok = true;
return f.attribute( fieldIdx );
}

Expand All @@ -413,7 +426,16 @@ QVariant QgsProperty::propertyValue( const QgsExpressionContext& context, const
return defaultValue;

QVariant result = d->expressionData.expression.evaluate( &context );
return result.isValid() ? result : defaultValue;
if ( result.isValid() )
{
if ( ok )
*ok = true;
return result;
}
else
{
return defaultValue;
}
}

case InvalidProperty:
Expand All @@ -425,23 +447,33 @@ QVariant QgsProperty::propertyValue( const QgsExpressionContext& context, const
}


QVariant QgsProperty::value( const QgsExpressionContext& context, const QVariant& defaultValue ) const
QVariant QgsProperty::value( const QgsExpressionContext& context, const QVariant& defaultValue, bool* ok ) const
{
QVariant val = propertyValue( context, defaultValue );
if ( ok )
*ok = false;

bool valOk = false;
QVariant val = propertyValue( context, defaultValue, &valOk );
if ( !valOk )
return defaultValue;

if ( d->transformer )
{
val = d->transformer->transform( context, val );
}

if ( ok )
*ok = true;

return val;
}

QString QgsProperty::valueAsString( const QgsExpressionContext& context, const QString& defaultString, bool* ok ) const
{
QVariant val = value( context, defaultString );
bool valOk = false;
QVariant val = value( context, defaultString, &valOk );

if ( !val.isValid() )
if ( !valOk || !val.isValid() )
{
if ( ok )
*ok = false;
Expand All @@ -460,9 +492,10 @@ QColor QgsProperty::valueAsColor( const QgsExpressionContext &context, const QCo
if ( ok )
*ok = false;

QVariant val = value( context, defaultColor );
bool valOk = false;
QVariant val = value( context, defaultColor, &valOk );

if ( !val.isValid() )
if ( !valOk || !val.isValid() )
return defaultColor;

QColor color;
Expand Down Expand Up @@ -490,9 +523,10 @@ double QgsProperty::valueAsDouble( const QgsExpressionContext &context, double d
if ( ok )
*ok = false;

QVariant val = value( context, defaultValue );
bool valOk = false;
QVariant val = value( context, defaultValue, &valOk );

if ( !val.isValid() )
if ( !valOk || !val.isValid() )
return defaultValue;

bool convertOk = false;
Expand All @@ -512,9 +546,10 @@ int QgsProperty::valueAsInt( const QgsExpressionContext &context, int defaultVal
if ( ok )
*ok = false;

QVariant val = value( context, defaultValue );
bool valOk = false;
QVariant val = value( context, defaultValue, &valOk );

if ( !val.isValid() )
if ( !valOk || !val.isValid() )
return defaultValue;

bool convertOk = false;
Expand Down Expand Up @@ -547,9 +582,10 @@ bool QgsProperty::valueAsBool( const QgsExpressionContext& context, bool default
if ( ok )
*ok = false;

QVariant val = value( context, defaultValue );
bool valOk = false;
QVariant val = value( context, defaultValue, &valOk );

if ( !val.isValid() )
if ( !valOk || !val.isValid() )
return defaultValue;

if ( ok )
Expand Down
5 changes: 3 additions & 2 deletions src/core/qgsproperty.h
Expand Up @@ -300,14 +300,15 @@ class CORE_EXPORT QgsProperty
* in the expression context can be used to alter the calculated value for the property, so that a property
* is able to respond to the current environment, layers and features within QGIS.
* @param defaultValue default value to return if the property is not active or cannot be calculated
* @param ok if specified, will be set to true if conversion was successful
* @returns calculated value for property
* @see valueAsString()
* @see valueAsColor()
* @see valueAsDouble()
* @see valueAsInt()
* @see valueAsBool()
*/
QVariant value( const QgsExpressionContext& context, const QVariant& defaultValue = QVariant() ) const;
QVariant value( const QgsExpressionContext& context, const QVariant& defaultValue = QVariant(), bool* ok = nullptr ) const;

/**
* Calculates the current value of the property and interprets it as a string.
Expand Down Expand Up @@ -417,7 +418,7 @@ class CORE_EXPORT QgsProperty
* Calculates the current value of the property, before any transformations or
* conversions are applied.
*/
QVariant propertyValue( const QgsExpressionContext& context, const QVariant& defaultValue = QVariant() ) const;
QVariant propertyValue( const QgsExpressionContext& context, const QVariant& defaultValue = QVariant(), bool* ok = nullptr ) const;

};

Expand Down
4 changes: 3 additions & 1 deletion tests/src/core/testqgscomposerscalebar.cpp
Expand Up @@ -161,6 +161,7 @@ void TestQgsComposerScaleBar::singleBoxAlpha()
mComposerScaleBar->setFillColor2( QColor( 0, 255, 0, 50 ) );
mComposerScaleBar->setLineColor( QColor( 0, 0, 255, 150 ) );
mComposerScaleBar->setFontColor( QColor( 255, 0, 255, 100 ) );
mComposerScaleBar->setLineWidth( 1.0 );
QgsCompositionChecker checker( QStringLiteral( "composerscalebar_singlebox_alpha" ), mComposition );
checker.setControlPathPrefix( QStringLiteral( "composer_scalebar" ) );
QVERIFY( checker.testComposition( mReport, 0, 0 ) );
Expand All @@ -172,7 +173,7 @@ void TestQgsComposerScaleBar::doubleBox()
mComposerScaleBar->setFillColor( Qt::black );
mComposerScaleBar->setFillColor2( Qt::white );
mComposerScaleBar->setLineColor( Qt::black );
mComposerScaleBar->setLineWidth( 0.3 );
mComposerScaleBar->setLineWidth( 1.0 );
mComposerScaleBar->setFontColor( Qt::black );
mComposerScaleBar->setStyle( QStringLiteral( "Double Box" ) );

Expand All @@ -193,6 +194,7 @@ void TestQgsComposerScaleBar::numeric()
void TestQgsComposerScaleBar::tick()
{
mComposerScaleBar->setStyle( QStringLiteral( "Line Ticks Up" ) );
mComposerScaleBar->setLineWidth( 1.0 );
QgsCompositionChecker checker( QStringLiteral( "composerscalebar_tick" ), mComposition );
checker.setControlPathPrefix( QStringLiteral( "composer_scalebar" ) );
QVERIFY( checker.testComposition( mReport, 0, 0 ) );
Expand Down
4 changes: 2 additions & 2 deletions tests/src/python/test_qgspallabeling_placement.py
Expand Up @@ -238,7 +238,7 @@ def test_point_dd_ordered_placement(self):
self.lyr.properties().setProperty(QgsPalLayerSettings.PredefinedPositionOrder, QgsProperty.fromExpression("'T,B'"))
self.checkTest()
self.removeMapLayer(self.layer)
self.lyr.removeDataDefinedProperty(QgsPalLayerSettings.PredefinedPositionOrder)
self.lyr.properties().setProperty(QgsPalLayerSettings.PredefinedPositionOrder, QgsProperty())
self.layer = None

def test_point_dd_ordered_placement1(self):
Expand All @@ -252,7 +252,7 @@ def test_point_dd_ordered_placement1(self):
self.checkTest()
self.removeMapLayer(obstacleLayer)
self.removeMapLayer(self.layer)
self.lyr.removeDataDefinedProperty(QgsPalLayerSettings.PredefinedPositionOrder)
self.lyr.properties().setProperty(QgsPalLayerSettings.PredefinedPositionOrder, QgsProperty())
self.layer = None

def test_point_ordered_symbol_bound_offset(self):
Expand Down

0 comments on commit f0f0170

Please sign in to comment.