Skip to content

Commit

Permalink
handle complex fieldnames when converting to rule based renderers
Browse files Browse the repository at this point in the history 
Fixes #46459
  • Loading branch information
@roya0045 @nyalldawson
roya0045 authored and nyalldawson committed Feb 21, 2022
1 parent b93d6b1 commit a006f83
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 15 deletions.
29 changes: 23 additions & 6 deletions src/core/symbology/qgsrulebasedrenderer.cpp
View file
Expand Up @@ -1327,9 +1327,17 @@ QgsRuleBasedRenderer *QgsRuleBasedRenderer::convertFromRenderer( const QgsFeatur

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( '\"' ) ) )
bool isField = false;
if ( layer )
{
isField = QgsExpression::expressionToLayerFieldIndex( attr, layer ) != -1;
}
else
{
QgsExpression testExpr( attr );
isField = testExpr.hasParserError() || testExpr.isField();
}
if ( isField && !attr.contains( '\"' ) )
{
//not an expression, so need to quote column name
attr = QgsExpression::quotedColumnRef( attr );
Expand Down Expand Up @@ -1418,13 +1426,22 @@ QgsRuleBasedRenderer *QgsRuleBasedRenderer::convertFromRenderer( const QgsFeatur
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( '\"' ) ) )
bool isField = false;
if ( layer )
{
isField = QgsExpression::expressionToLayerFieldIndex( attr, layer ) != -1;
}
else
{
QgsExpression testExpr( attr );
isField = testExpr.hasParserError() || testExpr.isField();
}
if ( isField && !attr.contains( '\"' ) )
{
//not an expression, so need to quote column name
attr = QgsExpression::quotedColumnRef( attr );
}
else if ( !testExpr.isField() )
else if ( !isField )
{
//otherwise wrap expression in brackets
attr = QStringLiteral( "(%1)" ).arg( attr );
Expand Down
41 changes: 32 additions & 9 deletions tests/src/python/test_qgsrulebasedrenderer.py
View file
Expand Up @@ -28,7 +28,7 @@

import os

from qgis.PyQt.QtCore import Qt, QSize
from qgis.PyQt.QtCore import Qt, QSize, QVariant
from qgis.PyQt.QtGui import QColor

from qgis.core import (QgsVectorLayer,
Expand All @@ -48,6 +48,7 @@
QgsSimpleMarkerSymbolLayer,
QgsProperty,
QgsFeature,
QgsField,
QgsGeometry,
QgsEmbeddedSymbolRenderer
)
Expand All @@ -71,6 +72,8 @@ def setUpClass(cls):
def setUp(self):
myShpFile = os.path.join(TEST_DATA_DIR, 'rectangles.shp')
layer = QgsVectorLayer(myShpFile, 'Rectangles', 'ogr')
vfield = QgsField('fa_cy-fie+ld', QVariant.Int)
layer.addExpressionField('"id"', vfield)
QgsProject.instance().addMapLayer(layer)

# Create rulebased style
Expand Down Expand Up @@ -272,7 +275,7 @@ def testRefineWithRanges(self):

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

vl = self.mapsettings.layers()[0]
# First, try with a field based category (id)
cats = []
cats.append(QgsRendererCategory(1, QgsMarkerSymbol(), "id 1"))
Expand All @@ -284,7 +287,7 @@ def testConvertFromCategorisedRenderer(self):
cats.append(QgsRendererCategory(['c', 'd'], QgsMarkerSymbol(), "c/d"))
c = QgsCategorizedSymbolRenderer("id", cats)

r = QgsRuleBasedRenderer.convertFromRenderer(c)
r = QgsRuleBasedRenderer.convertFromRenderer(c, vl)
self.assertEqual(len(r.rootRule().children()), 7)
self.assertEqual(r.rootRule().children()[0].filterExpression(), '"id" = 1')
self.assertEqual(r.rootRule().children()[1].filterExpression(), '"id" = 2')
Expand All @@ -301,7 +304,7 @@ def testConvertFromCategorisedRenderer(self):
cats.append(QgsRendererCategory([3, 4], QgsMarkerSymbol(), "result 3/4"))
c = QgsCategorizedSymbolRenderer("id + 1", cats)

r = QgsRuleBasedRenderer.convertFromRenderer(c)
r = QgsRuleBasedRenderer.convertFromRenderer(c, vl)
self.assertEqual(len(r.rootRule().children()), 3)
self.assertEqual(r.rootRule().children()[0].filterExpression(), 'id + 1 = 1')
self.assertEqual(r.rootRule().children()[1].filterExpression(), 'id + 1 = 2')
Expand All @@ -314,22 +317,32 @@ def testConvertFromCategorisedRenderer(self):
cats.append(QgsRendererCategory([3, 4], QgsMarkerSymbol(), "result 3/4"))
c = QgsCategorizedSymbolRenderer('"id"', cats)

r = QgsRuleBasedRenderer.convertFromRenderer(c)
r = QgsRuleBasedRenderer.convertFromRenderer(c, vl)
self.assertEqual(len(r.rootRule().children()), 3)
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" IN (3,4)')

# Next try with a complex name
cats = []
cats.append(QgsRendererCategory(1, QgsMarkerSymbol(), "fa_cy-fie+ld 1"))
cats.append(QgsRendererCategory(2, QgsMarkerSymbol(), "fa_cy-fie+ld 2"))
c = QgsCategorizedSymbolRenderer("fa_cy-fie+ld", cats)

r = QgsRuleBasedRenderer.convertFromRenderer(c, vl)
self.assertEqual(r.rootRule().children()[0].filterExpression(), '"fa_cy-fie+ld" = 1')
self.assertEqual(r.rootRule().children()[1].filterExpression(), '"fa_cy-fie+ld" = 2')

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

vl = self.mapsettings.layers()[0]
# First, try with a field based category (id)
ranges = []
ranges.append(QgsRendererRange(0, 1, QgsMarkerSymbol(), "0-1"))
ranges.append(QgsRendererRange(1, 2, QgsMarkerSymbol(), "1-2"))
g = QgsGraduatedSymbolRenderer("id", ranges)

r = QgsRuleBasedRenderer.convertFromRenderer(g)
r = QgsRuleBasedRenderer.convertFromRenderer(g, vl)
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')

Expand All @@ -339,7 +352,7 @@ def testConvertFromGraduatedRenderer(self):
ranges.append(QgsRendererRange(1, 2, QgsMarkerSymbol(), "1-2"))
g = QgsGraduatedSymbolRenderer("id / 2", ranges)

r = QgsRuleBasedRenderer.convertFromRenderer(g)
r = QgsRuleBasedRenderer.convertFromRenderer(g, vl)
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')

Expand All @@ -349,10 +362,20 @@ def testConvertFromGraduatedRenderer(self):
ranges.append(QgsRendererRange(1, 2, QgsMarkerSymbol(), "1-2"))
g = QgsGraduatedSymbolRenderer('"id"', ranges)

r = QgsRuleBasedRenderer.convertFromRenderer(g)
r = QgsRuleBasedRenderer.convertFromRenderer(g, vl)
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')

# Test with a complex field name
ranges = []
ranges.append(QgsRendererRange(0, 1, QgsMarkerSymbol(), "0-1"))
ranges.append(QgsRendererRange(1, 2, QgsMarkerSymbol(), "1-2"))
g = QgsGraduatedSymbolRenderer("fa_cy-fie+ld", ranges)

r = QgsRuleBasedRenderer.convertFromRenderer(g, vl)
self.assertEqual(r.rootRule().children()[0].filterExpression(), '"fa_cy-fie+ld" >= 0.000000 AND "fa_cy-fie+ld" <= 1.000000')
self.assertEqual(r.rootRule().children()[1].filterExpression(), '"fa_cy-fie+ld" > 1.000000 AND "fa_cy-fie+ld" <= 2.000000')

def testWillRenderFeatureTwoElse(self):
"""Regression #21287, also test rulesForFeature since there were no tests any where and I've found a couple of issues"""

Expand Down

0 comments on commit a006f83

Please sign in to comment.