Skip to content

Commit

Permalink
Fix issues with conversion of renderers to rule based renderer
Browse files Browse the repository at this point in the history 
- expressions were not correctly translated and always treated
as a single field
- string values were not correctly escaped, resulting in a broken
renderer
  • Loading branch information
@nyalldawson
nyalldawson committed Jan 25, 2016
1 parent 39e1f68 commit 7400ca9
Show file tree
Hide file tree
Showing 2 changed files with 105 additions and 7 deletions.
38 changes: 31 additions & 7 deletions src/core/symbology-ng/qgsrulebasedrendererv2.cpp
View file
Expand Up @@ -1236,6 +1236,16 @@ QgsRuleBasedRendererV2* QgsRuleBasedRendererV2::convertFromRenderer( const QgsFe
if ( !categorizedRenderer )
return nullptr;

QString attr = categorizedRenderer->classAttribute();
// categorizedAttr could be either an attribute name or an expression.
// the only way to differentiate is to test it as an expression...
QgsExpression testExpr( attr );
if ( testExpr.hasParserError() || ( testExpr.isField() && !attr.startsWith( '\"' ) ) )
{
//not an expression, so need to quote column name
attr = QgsExpression::quotedColumnRef( attr );
}

QgsRuleBasedRendererV2::Rule* rootrule = new QgsRuleBasedRendererV2::Rule( nullptr );

QString expression;
Expand All @@ -1256,7 +1266,7 @@ QgsRuleBasedRendererV2* QgsRuleBasedRendererV2::convertFromRenderer( const QgsFe
}
else
{
value = '\'' + category.value().toString() + '\'';
value = QgsExpression::quotedString( category.value().toString() );
}

//An empty category is equivalent to the ELSE keyword
Expand All @@ -1266,7 +1276,7 @@ QgsRuleBasedRendererV2* QgsRuleBasedRendererV2::convertFromRenderer( const QgsFe
}
else
{
expression = QString( "%1 = %2" ).arg( QgsExpression::quotedColumnRef( categorizedRenderer->classAttribute() ), value );
expression = QString( "%1 = %2" ).arg( attr, value );
}
rule->setFilterExpression( expression );

Expand All @@ -1286,11 +1296,25 @@ QgsRuleBasedRendererV2* QgsRuleBasedRendererV2::convertFromRenderer( const QgsFe

if ( renderer->type() == "graduatedSymbol" )
{

const QgsGraduatedSymbolRendererV2* graduatedRenderer = dynamic_cast<const QgsGraduatedSymbolRendererV2*>( renderer );
if ( !graduatedRenderer )
return nullptr;

QString attr = graduatedRenderer->classAttribute();
// categorizedAttr could be either an attribute name or an expression.
// the only way to differentiate is to test it as an expression...
QgsExpression testExpr( attr );
if ( testExpr.hasParserError() || ( testExpr.isField() && !attr.startsWith( '\"' ) ) )
{
//not an expression, so need to quote column name
attr = QgsExpression::quotedColumnRef( attr );
}
else if ( !testExpr.isField() )
{
//otherwise wrap expression in brackets
attr = QString( "(%1)" ).arg( attr );
}

QgsRuleBasedRendererV2::Rule* rootrule = new QgsRuleBasedRendererV2::Rule( nullptr );

QString expression;
Expand All @@ -1302,13 +1326,13 @@ QgsRuleBasedRendererV2* QgsRuleBasedRendererV2::convertFromRenderer( const QgsFe
rule->setLabel( range.label() );
if ( i == 0 )//The lower boundary of the first range is included, while it is excluded for the others
{
expression = graduatedRenderer->classAttribute() + " >= " + QString::number( range.lowerValue(), 'f' ) + " AND " + \
graduatedRenderer->classAttribute() + " <= " + QString::number( range.upperValue(), 'f' );
expression = attr + " >= " + QString::number( range.lowerValue(), 'f' ) + " AND " + \
attr + " <= " + QString::number( range.upperValue(), 'f' );
}
else
{
expression = graduatedRenderer->classAttribute() + " > " + QString::number( range.lowerValue(), 'f' ) + " AND " + \
graduatedRenderer->classAttribute() + " <= " + QString::number( range.upperValue(), 'f' );
expression = attr + " > " + QString::number( range.lowerValue(), 'f' ) + " AND " + \
attr + " <= " + QString::number( range.upperValue(), 'f' );
}
rule->setFilterExpression( expression );

Expand Down
74 changes: 74 additions & 0 deletions tests/src/python/test_qgsrulebasedrenderer.py
View file
Expand Up @@ -171,5 +171,79 @@ def testRefineWithRanges(self):
assert self.r3.children()[0].filterExpression() == '"id" >= 0.0000 AND "id" <= 1.0000'
assert self.r3.children()[1].filterExpression() == '"id" > 1.0000 AND "id" <= 2.0000'

def testConvertFromCategorisedRenderer(self):
# Test converting categorised renderer to rule based

# First, try with a field based category (id)
cats = []
cats.append(QgsRendererCategoryV2(1, QgsMarkerSymbolV2(), "id 1"))
cats.append(QgsRendererCategoryV2(2, QgsMarkerSymbolV2(), "id 2"))
cats.append(QgsRendererCategoryV2('a\'b', QgsMarkerSymbolV2(), "id a'b"))
cats.append(QgsRendererCategoryV2('a\nb', QgsMarkerSymbolV2(), "id a\\nb"))
cats.append(QgsRendererCategoryV2('a\\b', QgsMarkerSymbolV2(), "id a\\\\b"))
cats.append(QgsRendererCategoryV2('a\tb', QgsMarkerSymbolV2(), "id a\\tb"))
c = QgsCategorizedSymbolRendererV2("id", cats)

r = QgsRuleBasedRendererV2.convertFromRenderer(c)
self.assertEqual(r.rootRule().children()[0].filterExpression(), '"id" = 1')
self.assertEqual(r.rootRule().children()[1].filterExpression(), '"id" = 2')
self.assertEqual(r.rootRule().children()[2].filterExpression(), '"id" = \'a\'\'b\'')
self.assertEqual(r.rootRule().children()[3].filterExpression(), '"id" = \'a\\nb\'')
self.assertEqual(r.rootRule().children()[4].filterExpression(), '"id" = \'a\\\\b\'')
self.assertEqual(r.rootRule().children()[5].filterExpression(), '"id" = \'a\\tb\'')

# Next try with an expression based category
cats = []
cats.append(QgsRendererCategoryV2(1, QgsMarkerSymbolV2(), "result 1"))
cats.append(QgsRendererCategoryV2(2, QgsMarkerSymbolV2(), "result 2"))
c = QgsCategorizedSymbolRendererV2("id + 1", cats)

r = QgsRuleBasedRendererV2.convertFromRenderer(c)
self.assertEqual(r.rootRule().children()[0].filterExpression(), 'id + 1 = 1')
self.assertEqual(r.rootRule().children()[1].filterExpression(), 'id + 1 = 2')

# Last try with an expression which is just a quoted field name
cats = []
cats.append(QgsRendererCategoryV2(1, QgsMarkerSymbolV2(), "result 1"))
cats.append(QgsRendererCategoryV2(2, QgsMarkerSymbolV2(), "result 2"))
c = QgsCategorizedSymbolRendererV2('"id"', cats)

r = QgsRuleBasedRendererV2.convertFromRenderer(c)
self.assertEqual(r.rootRule().children()[0].filterExpression(), '"id" = 1')
self.assertEqual(r.rootRule().children()[1].filterExpression(), '"id" = 2')

def testConvertFromGraduatedRenderer(self):
# Test converting graduated renderer to rule based

# First, try with a field based category (id)
ranges = []
ranges.append(QgsRendererRangeV2(0, 1, QgsMarkerSymbolV2(), "0-1"))
ranges.append(QgsRendererRangeV2(1, 2, QgsMarkerSymbolV2(), "1-2"))
g = QgsGraduatedSymbolRendererV2("id", ranges)

r = QgsRuleBasedRendererV2.convertFromRenderer(g)
self.assertEqual(r.rootRule().children()[0].filterExpression(), '"id" >= 0.000000 AND "id" <= 1.000000')
self.assertEqual(r.rootRule().children()[1].filterExpression(), '"id" > 1.000000 AND "id" <= 2.000000')

# Next try with an expression based range
ranges = []
ranges.append(QgsRendererRangeV2(0, 1, QgsMarkerSymbolV2(), "0-1"))
ranges.append(QgsRendererRangeV2(1, 2, QgsMarkerSymbolV2(), "1-2"))
g = QgsGraduatedSymbolRendererV2("id / 2", ranges)

r = QgsRuleBasedRendererV2.convertFromRenderer(g)
self.assertEqual(r.rootRule().children()[0].filterExpression(), '(id / 2) >= 0.000000 AND (id / 2) <= 1.000000')
self.assertEqual(r.rootRule().children()[1].filterExpression(), '(id / 2) > 1.000000 AND (id / 2) <= 2.000000')

# Last try with an expression which is just a quoted field name
ranges = []
ranges.append(QgsRendererRangeV2(0, 1, QgsMarkerSymbolV2(), "0-1"))
ranges.append(QgsRendererRangeV2(1, 2, QgsMarkerSymbolV2(), "1-2"))
g = QgsGraduatedSymbolRendererV2('"id"', ranges)

r = QgsRuleBasedRendererV2.convertFromRenderer(g)
self.assertEqual(r.rootRule().children()[0].filterExpression(), '"id" >= 0.000000 AND "id" <= 1.000000')
self.assertEqual(r.rootRule().children()[1].filterExpression(), '"id" > 1.000000 AND "id" <= 2.000000')

if __name__ == '__main__':
unittest.main()

0 comments on commit 7400ca9

Please sign in to comment.