Fix refining rule based renderer using expression (fix #10815)

qgis · Oct 12, 2015 · ae85376 · ae85376
1 parent c7b9fa6
commit ae85376
Show file tree

Hide file tree

Showing 2 changed files with 101 additions and 4 deletions.
diff --git a/src/core/symbology-ng/qgsrulebasedrendererv2.cpp b/src/core/symbology-ng/qgsrulebasedrendererv2.cpp
@@ -1055,9 +1055,18 @@ QgsFeatureRendererV2* QgsRuleBasedRendererV2::createFromSld( QDomElement& elemen
 
 void QgsRuleBasedRendererV2::refineRuleCategories( QgsRuleBasedRendererV2::Rule* initialRule, QgsCategorizedSymbolRendererV2* r )
 {
+  QString attr = r->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 );
+  }
+
   Q_FOREACH ( const QgsRendererCategoryV2& cat, r->categories() )
   {
-    QString attr = QgsExpression::quotedColumnRef( r->classAttribute() );
     QString value;
     // not quoting numbers saves a type cast
     if ( cat.value().type() == QVariant::Int )
@@ -1076,14 +1085,31 @@ void QgsRuleBasedRendererV2::refineRuleCategories( QgsRuleBasedRendererV2::Rule*
 
 void QgsRuleBasedRendererV2::refineRuleRanges( QgsRuleBasedRendererV2::Rule* initialRule, QgsGraduatedSymbolRendererV2* r )
 {
+  QString attr = r->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 );
+  }
+
+  bool firstRange = true;
   Q_FOREACH ( const QgsRendererRangeV2& rng, r->ranges() )
   {
     // due to the loss of precision in double->string conversion we may miss out values at the limit of the range
     // TODO: have a possibility to construct expressions directly as a parse tree to avoid loss of precision
-    QString attr = QgsExpression::quotedColumnRef( r->classAttribute() );
-    QString filter = QString( "%1 >= %2 AND %1 <= %3" ).arg( attr )
+    QString filter = QString( "%1 %2 %3 AND %1 <= %4" ).arg( attr )
+                     .arg( firstRange ? ">=" : ">" )
                      .arg( QString::number( rng.lowerValue(), 'f', 4 ) )
                      .arg( QString::number( rng.upperValue(), 'f', 4 ) );
+    firstRange = false;
     QString label = filter;
     initialRule->appendChild( new Rule( rng.symbol()->clone(), 0, 0, filter, label ) );
   }

diff --git a/tests/src/python/test_qgsrulebasedrenderer.py b/tests/src/python/test_qgsrulebasedrenderer.py
@@ -33,7 +33,12 @@
                        QgsRectangle,
                        QgsMultiRenderChecker,
                        QgsRuleBasedRendererV2,
-                       QgsFillSymbolV2
+                       QgsFillSymbolV2,
+                       QgsMarkerSymbolV2,
+                       QgsRendererCategoryV2,
+                       QgsCategorizedSymbolRendererV2,
+                       QgsGraduatedSymbolRendererV2,
+                       QgsRendererRangeV2
                        )
 from utilities import (unitTestDataPath,
                        getQgisTestApp,
@@ -100,5 +105,71 @@ def testDisabledElse(self):
 
         assert result
 
+    def testRefineWithCategories(self):
+        #Test refining rule with categories (refs #10815)
+
+        #First, try with a field based category (id)
+        cats = []
+        cats.append(QgsRendererCategoryV2(1, QgsMarkerSymbolV2(), "id 1"))
+        cats.append(QgsRendererCategoryV2(2, QgsMarkerSymbolV2(), "id 2"))
+        c = QgsCategorizedSymbolRendererV2("id", cats)
+
+        QgsRuleBasedRendererV2.refineRuleCategories(self.r2, c)
+        assert self.r2.children()[0].filterExpression() == '"id" = 1'
+        assert self.r2.children()[1].filterExpression() == '"id" = 2'
+
+        #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)
+
+        QgsRuleBasedRendererV2.refineRuleCategories(self.r1, c)
+        assert self.r1.children()[0].filterExpression() == 'id + 1 = 1'
+        assert self.r1.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)
+
+        QgsRuleBasedRendererV2.refineRuleCategories(self.r3, c)
+        assert self.r3.children()[0].filterExpression() == '"id" = 1'
+        assert self.r3.children()[1].filterExpression() == '"id" = 2'
+
+    def testRefineWithRanges(self):
+        #Test refining rule with ranges (refs #10815)
+
+        #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)
+
+        QgsRuleBasedRendererV2.refineRuleRanges(self.r2, g)
+        assert self.r2.children()[0].filterExpression() == '"id" >= 0.0000 AND "id" <= 1.0000'
+        assert self.r2.children()[1].filterExpression() == '"id" > 1.0000 AND "id" <= 2.0000'
+
+        #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)
+
+        QgsRuleBasedRendererV2.refineRuleRanges(self.r1, g)
+        assert self.r1.children()[0].filterExpression() == '(id / 2) >= 0.0000 AND (id / 2) <= 1.0000'
+        assert self.r1.children()[1].filterExpression() == '(id / 2) > 1.0000 AND (id / 2) <= 2.0000'
+
+        #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)
+
+        QgsRuleBasedRendererV2.refineRuleRanges(self.r3, g)
+        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'
+
 if __name__ == '__main__':
     unittest.main()