Skip to content

Commit e6c2ecd

Browse files
committedFeb 11, 2014
Fix #9290 (crashes in symbology on fields with special characters)
Fixes several crashes and problems with backwards compatibility. Before expressions were allowed, field names were loaded and saved unquoted, e.g. x.y - with introduction of expressions it was necessary to use properly quoted "x.y" name so that expression is correctly parsed. With these changes both options are possible and when possible, the unquoted field name is used for compatibility
1 parent c13b6a4 commit e6c2ecd

12 files changed

+159
-53
lines changed
 

‎python/core/symbology-ng/qgscategorizedsymbolrendererv2.sip

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -110,12 +110,12 @@ class QgsCategorizedSymbolRendererV2 : QgsFeatureRendererV2
110110
void setInvertedColorRamp( bool inverted );
111111

112112
//! @note added in 1.6
113-
void setRotationField( QString expression );
113+
void setRotationField( QString fieldOrExpression );
114114
//! @note added in 1.6
115115
QString rotationField() const;
116116

117117
//! @note added in 1.6
118-
void setSizeScaleField( QString expression );
118+
void setSizeScaleField( QString fieldOrExpression );
119119
//! @note added in 1.6
120120
QString sizeScaleField() const;
121121

‎python/core/symbology-ng/qgsgraduatedsymbolrendererv2.sip

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -136,12 +136,12 @@ class QgsGraduatedSymbolRendererV2 : QgsFeatureRendererV2
136136
void updateSymbols( QgsSymbolV2* sym /Transfer/ );
137137

138138
//! @note added in 1.6
139-
void setRotationField( QString expression );
139+
void setRotationField( QString fieldOrExpression );
140140
//! @note added in 1.6
141141
QString rotationField() const;
142142

143143
//! @note added in 1.6
144-
void setSizeScaleField( QString expression );
144+
void setSizeScaleField( QString fieldOrExpression );
145145
//! @note added in 1.6
146146
QString sizeScaleField() const;
147147

‎python/core/symbology-ng/qgssinglesymbolrendererv2.sip

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,12 +21,12 @@ class QgsSingleSymbolRendererV2 : QgsFeatureRendererV2
2121
void setSymbol( QgsSymbolV2* s /Transfer/ );
2222

2323
//! @note added in 1.5
24-
void setRotationField( QString expression );
24+
void setRotationField( QString fieldOrExpression );
2525
//! @note added in 1.5
2626
QString rotationField() const;
2727

2828
//! @note added in 1.5
29-
void setSizeScaleField( QString expression );
29+
void setSizeScaleField( QString fieldOrExpression );
3030
//! @note added in 1.5
3131
QString sizeScaleField() const;
3232

‎python/core/symbology-ng/qgssymbollayerv2utils.sip

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -226,4 +226,20 @@ class QgsSymbolLayerV2Utils
226226

227227
//! Calculate the centroid point of a QPolygonF
228228
static QPointF polygonCentroid( const QPolygonF& points );
229+
230+
/** Return a new valid expression instance for given field or expression string.
231+
* If the input is not a valid expression, it is assumed that it is a field name and gets properly quoted.
232+
* If the string is empty, returns null pointer.
233+
* This is useful when accepting input which could be either a non-quoted field name or expression.
234+
* @note added in 2.2
235+
*/
236+
static QgsExpression* fieldOrExpressionToExpression( const QString& fieldOrExpression ) /Factory/;
237+
238+
/** Return a field name if the whole expression is just a name of the field .
239+
* Returns full expression string if the expression is more complex than just one field.
240+
* Using just expression->expression() method may return quoted field name, but that is not
241+
* wanted for saving (due to backward compatibility) or display in GUI.
242+
* @note added in 2.2
243+
*/
244+
static QString fieldOrExpressionFromExpression( QgsExpression* expression );
229245
};

‎src/core/symbology-ng/qgscategorizedsymbolrendererv2.cpp

Lines changed: 31 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -211,7 +211,7 @@ QgsSymbolV2* QgsCategorizedSymbolRendererV2::symbolForFeature( QgsFeature& featu
211211
const double sizeScale = mSizeScale.data() ? mSizeScale->evaluate( feature ).toDouble() : 1.;
212212

213213
// take a temporary symbol (or create it if doesn't exist)
214-
QgsSymbolV2* tempSymbol = mTempSymbols[attrs[mAttrNum].toString()];
214+
QgsSymbolV2* tempSymbol = mTempSymbols[value.toString()];
215215

216216
// modify the temporary symbol and return it
217217
if ( tempSymbol->type() == QgsSymbolV2::Marker )
@@ -388,8 +388,14 @@ void QgsCategorizedSymbolRendererV2::stopRender( QgsRenderContext& context )
388388

389389
QList<QString> QgsCategorizedSymbolRendererV2::usedAttributes()
390390
{
391-
QgsExpression exp( mAttrName );
392-
QSet<QString> attributes( exp.referencedColumns().toSet() );
391+
QSet<QString> attributes;
392+
393+
if ( QgsExpression* exp = QgsSymbolLayerV2Utils::fieldOrExpressionToExpression( mAttrName ) )
394+
{
395+
attributes.unite( exp->referencedColumns().toSet() );
396+
delete exp;
397+
}
398+
393399
if ( mRotation.data() ) attributes.unite( mRotation->referencedColumns().toSet() );
394400
if ( mSizeScale.data() ) attributes.unite( mSizeScale->referencedColumns().toSet() );
395401

@@ -582,12 +588,12 @@ QDomElement QgsCategorizedSymbolRendererV2::save( QDomDocument& doc )
582588

583589
QDomElement rotationElem = doc.createElement( "rotation" );
584590
if ( mRotation.data() )
585-
rotationElem.setAttribute( "field", mRotation->expression() );
591+
rotationElem.setAttribute( "field", QgsSymbolLayerV2Utils::fieldOrExpressionFromExpression( mRotation.data() ) );
586592
rendererElem.appendChild( rotationElem );
587593

588594
QDomElement sizeScaleElem = doc.createElement( "sizescale" );
589595
if ( mSizeScale.data() )
590-
sizeScaleElem.setAttribute( "field", mSizeScale->expression() );
596+
sizeScaleElem.setAttribute( "field", QgsSymbolLayerV2Utils::fieldOrExpressionFromExpression( mSizeScale.data() ) );
591597
sizeScaleElem.setAttribute( "scalemethod", QgsSymbolLayerV2Utils::encodeScaleMethod( mScaleMethod ) );
592598
rendererElem.appendChild( sizeScaleElem );
593599

@@ -656,6 +662,26 @@ void QgsCategorizedSymbolRendererV2::setSourceColorRamp( QgsVectorColorRampV2* r
656662
mSourceColorRamp.reset( ramp );
657663
}
658664

665+
void QgsCategorizedSymbolRendererV2::setRotationField( QString fieldOrExpression )
666+
{
667+
mRotation.reset( QgsSymbolLayerV2Utils::fieldOrExpressionToExpression( fieldOrExpression ) );
668+
}
669+
670+
QString QgsCategorizedSymbolRendererV2::rotationField() const
671+
{
672+
return mRotation.data() ? QgsSymbolLayerV2Utils::fieldOrExpressionFromExpression( mRotation.data() ) : QString();
673+
}
674+
675+
void QgsCategorizedSymbolRendererV2::setSizeScaleField( QString fieldOrExpression )
676+
{
677+
mSizeScale.reset( QgsSymbolLayerV2Utils::fieldOrExpressionToExpression( fieldOrExpression ) );
678+
}
679+
680+
QString QgsCategorizedSymbolRendererV2::sizeScaleField() const
681+
{
682+
return mSizeScale.data() ? QgsSymbolLayerV2Utils::fieldOrExpressionFromExpression( mSizeScale.data() ) : QString();
683+
}
684+
659685
void QgsCategorizedSymbolRendererV2::updateSymbols( QgsSymbolV2 * sym )
660686
{
661687
int i = 0;

‎src/core/symbology-ng/qgscategorizedsymbolrendererv2.h

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -139,22 +139,14 @@ class CORE_EXPORT QgsCategorizedSymbolRendererV2 : public QgsFeatureRendererV2
139139
void setInvertedColorRamp( bool inverted ) { mInvertedColorRamp = inverted; }
140140

141141
//! @note added in 1.6
142-
void setRotationField( QString expression )
143-
{
144-
mRotation.reset( expression.isEmpty() ? NULL : new QgsExpression( expression ) );
145-
Q_ASSERT( !mRotation.data() || !mRotation->hasParserError() );
146-
}
142+
void setRotationField( QString fieldOrExpression );
147143
//! @note added in 1.6
148-
QString rotationField() const { return mRotation.data() ? mRotation->expression() : QString();}
144+
QString rotationField() const;
149145

150146
//! @note added in 1.6
151-
void setSizeScaleField( QString expression )
152-
{
153-
mSizeScale.reset( expression.isEmpty() ? NULL : new QgsExpression( expression ) );
154-
Q_ASSERT( !mSizeScale.data() || !mSizeScale->hasParserError() );
155-
}
147+
void setSizeScaleField( QString fieldOrExpression );
156148
//! @note added in 1.6
157-
QString sizeScaleField() const { return mSizeScale.data() ? mSizeScale->expression() : QString(); }
149+
QString sizeScaleField() const;
158150

159151
//! @note added in 2.0
160152
void setScaleMethod( QgsSymbolV2::ScaleMethod scaleMethod );

‎src/core/symbology-ng/qgsgraduatedsymbolrendererv2.cpp

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -263,8 +263,14 @@ void QgsGraduatedSymbolRendererV2::stopRender( QgsRenderContext& context )
263263

264264
QList<QString> QgsGraduatedSymbolRendererV2::usedAttributes()
265265
{
266-
QgsExpression exp( mAttrName );
267-
QSet<QString> attributes( exp.referencedColumns().toSet() );
266+
QSet<QString> attributes;
267+
268+
if ( QgsExpression* exp = QgsSymbolLayerV2Utils::fieldOrExpressionToExpression( mAttrName ) )
269+
{
270+
attributes.unite( exp->referencedColumns().toSet() );
271+
delete exp;
272+
}
273+
268274
if ( mRotation.data() ) attributes.unite( mRotation->referencedColumns().toSet() );
269275
if ( mSizeScale.data() ) attributes.unite( mSizeScale->referencedColumns().toSet() );
270276

@@ -1041,12 +1047,12 @@ QDomElement QgsGraduatedSymbolRendererV2::save( QDomDocument& doc )
10411047

10421048
QDomElement rotationElem = doc.createElement( "rotation" );
10431049
if ( mRotation.data() )
1044-
rotationElem.setAttribute( "field", mRotation->expression() );
1050+
rotationElem.setAttribute( "field", QgsSymbolLayerV2Utils::fieldOrExpressionFromExpression( mRotation.data() ) );
10451051
rendererElem.appendChild( rotationElem );
10461052

10471053
QDomElement sizeScaleElem = doc.createElement( "sizescale" );
10481054
if ( mSizeScale.data() )
1049-
sizeScaleElem.setAttribute( "field", mSizeScale->expression() );
1055+
sizeScaleElem.setAttribute( "field", QgsSymbolLayerV2Utils::fieldOrExpressionFromExpression( mSizeScale.data() ) );
10501056
sizeScaleElem.setAttribute( "scalemethod", QgsSymbolLayerV2Utils::encodeScaleMethod( mScaleMethod ) );
10511057
rendererElem.appendChild( sizeScaleElem );
10521058

@@ -1154,6 +1160,26 @@ void QgsGraduatedSymbolRendererV2::updateSymbols( QgsSymbolV2 *sym )
11541160
this->setSourceSymbol( sym->clone() );
11551161
}
11561162

1163+
void QgsGraduatedSymbolRendererV2::setRotationField( QString fieldOrExpression )
1164+
{
1165+
mRotation.reset( QgsSymbolLayerV2Utils::fieldOrExpressionToExpression( fieldOrExpression ) );
1166+
}
1167+
1168+
QString QgsGraduatedSymbolRendererV2::rotationField() const
1169+
{
1170+
return mRotation.data() ? QgsSymbolLayerV2Utils::fieldOrExpressionFromExpression( mRotation.data() ) : QString();
1171+
}
1172+
1173+
void QgsGraduatedSymbolRendererV2::setSizeScaleField( QString fieldOrExpression )
1174+
{
1175+
mSizeScale.reset( QgsSymbolLayerV2Utils::fieldOrExpressionToExpression( fieldOrExpression ) );
1176+
}
1177+
1178+
QString QgsGraduatedSymbolRendererV2::sizeScaleField() const
1179+
{
1180+
return mSizeScale.data() ? QgsSymbolLayerV2Utils::fieldOrExpressionFromExpression( mSizeScale.data() ) : QString();
1181+
}
1182+
11571183
void QgsGraduatedSymbolRendererV2::setScaleMethod( QgsSymbolV2::ScaleMethod scaleMethod )
11581184
{
11591185
mScaleMethod = scaleMethod;

‎src/core/symbology-ng/qgsgraduatedsymbolrendererv2.h

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -165,22 +165,14 @@ class CORE_EXPORT QgsGraduatedSymbolRendererV2 : public QgsFeatureRendererV2
165165
void updateSymbols( QgsSymbolV2* sym );
166166

167167
//! @note added in 1.6
168-
void setRotationField( QString expression )
169-
{
170-
mRotation.reset( expression.isEmpty() ? NULL : new QgsExpression( expression ) );
171-
Q_ASSERT( !mRotation.data() || !mRotation->hasParserError() );
172-
}
168+
void setRotationField( QString fieldOrExpression );
173169
//! @note added in 1.6
174-
QString rotationField() const { return mRotation.data() ? mRotation->expression() : QString();}
170+
QString rotationField() const;
175171

176172
//! @note added in 1.6
177-
void setSizeScaleField( QString expression )
178-
{
179-
mSizeScale.reset( expression.isEmpty() ? NULL : new QgsExpression( expression ) );
180-
Q_ASSERT( !mSizeScale.data() || !mSizeScale->hasParserError() );
181-
}
173+
void setSizeScaleField( QString fieldOrExpression );
182174
//! @note added in 1.6
183-
QString sizeScaleField() const { return mSizeScale.data() ? mSizeScale->expression() : QString(); }
175+
QString sizeScaleField() const;
184176

185177
//! @note added in 2.0
186178
void setScaleMethod( QgsSymbolV2::ScaleMethod scaleMethod );

‎src/core/symbology-ng/qgssinglesymbolrendererv2.cpp

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,26 @@ void QgsSingleSymbolRendererV2::setSymbol( QgsSymbolV2* s )
136136
mSymbol.reset( s );
137137
}
138138

139+
void QgsSingleSymbolRendererV2::setRotationField( QString fieldOrExpression )
140+
{
141+
mRotation.reset( QgsSymbolLayerV2Utils::fieldOrExpressionToExpression( fieldOrExpression ) );
142+
}
143+
144+
QString QgsSingleSymbolRendererV2::rotationField() const
145+
{
146+
return mRotation.data() ? QgsSymbolLayerV2Utils::fieldOrExpressionFromExpression( mRotation.data() ) : QString();
147+
}
148+
149+
void QgsSingleSymbolRendererV2::setSizeScaleField( QString fieldOrExpression )
150+
{
151+
mSizeScale.reset( QgsSymbolLayerV2Utils::fieldOrExpressionToExpression( fieldOrExpression ) );
152+
}
153+
154+
QString QgsSingleSymbolRendererV2::sizeScaleField() const
155+
{
156+
return mSizeScale.data() ? QgsSymbolLayerV2Utils::fieldOrExpressionFromExpression( mSizeScale.data() ) : QString();
157+
}
158+
139159
void QgsSingleSymbolRendererV2::setScaleMethod( QgsSymbolV2::ScaleMethod scaleMethod )
140160
{
141161
mScaleMethod = scaleMethod;
@@ -315,12 +335,12 @@ QDomElement QgsSingleSymbolRendererV2::save( QDomDocument& doc )
315335

316336
QDomElement rotationElem = doc.createElement( "rotation" );
317337
if ( mRotation.data() )
318-
rotationElem.setAttribute( "field", mRotation->expression() );
338+
rotationElem.setAttribute( "field", QgsSymbolLayerV2Utils::fieldOrExpressionFromExpression( mRotation.data() ) );
319339
rendererElem.appendChild( rotationElem );
320340

321341
QDomElement sizeScaleElem = doc.createElement( "sizescale" );
322342
if ( mSizeScale.data() )
323-
sizeScaleElem.setAttribute( "field", mSizeScale->expression() );
343+
sizeScaleElem.setAttribute( "field", QgsSymbolLayerV2Utils::fieldOrExpressionFromExpression( mSizeScale.data() ) );
324344
sizeScaleElem.setAttribute( "scalemethod", QgsSymbolLayerV2Utils::encodeScaleMethod( mScaleMethod ) );
325345
rendererElem.appendChild( sizeScaleElem );
326346

‎src/core/symbology-ng/qgssinglesymbolrendererv2.h

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -41,22 +41,14 @@ class CORE_EXPORT QgsSingleSymbolRendererV2 : public QgsFeatureRendererV2
4141
void setSymbol( QgsSymbolV2* s );
4242

4343
//! @note added in 1.5
44-
void setRotationField( QString expression )
45-
{
46-
mRotation.reset( expression.isEmpty() ? NULL : new QgsExpression( expression ) );
47-
Q_ASSERT( !mRotation.data() || !mRotation->hasParserError() );
48-
}
44+
void setRotationField( QString fieldOrExpression );
4945
//! @note added in 1.5
50-
QString rotationField() const { return mRotation.data() ? mRotation->expression() : QString(); }
46+
QString rotationField() const;
5147

5248
//! @note added in 1.5
53-
void setSizeScaleField( QString expression )
54-
{
55-
mSizeScale.reset( expression.isEmpty() ? NULL : new QgsExpression( expression ) );
56-
Q_ASSERT( !mSizeScale.data() || !mSizeScale->hasParserError() );
57-
}
49+
void setSizeScaleField( QString fieldOrExpression );
5850
//! @note added in 1.5
59-
QString sizeScaleField() const { return mSizeScale.data() ? mSizeScale->expression() : QString(); }
51+
QString sizeScaleField() const;
6052

6153
//! @note added in 2.0
6254
void setScaleMethod( QgsSymbolV2::ScaleMethod scaleMethod );

‎src/core/symbology-ng/qgssymbollayerv2utils.cpp

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2972,3 +2972,28 @@ QPointF QgsSymbolLayerV2Utils::polygonCentroid( const QPolygonF& points )
29722972
}
29732973

29742974

2975+
QgsExpression* QgsSymbolLayerV2Utils::fieldOrExpressionToExpression( const QString& fieldOrExpression )
2976+
{
2977+
if ( fieldOrExpression.isEmpty() )
2978+
return 0;
2979+
2980+
QgsExpression* expr = new QgsExpression( fieldOrExpression );
2981+
if ( !expr->hasParserError() )
2982+
return expr;
2983+
2984+
// now try with quoted field name
2985+
delete expr;
2986+
QgsExpression* expr2 = new QgsExpression( QgsExpression::quotedColumnRef( fieldOrExpression ) );
2987+
Q_ASSERT( !expr2->hasParserError() );
2988+
return expr2;
2989+
}
2990+
2991+
QString QgsSymbolLayerV2Utils::fieldOrExpressionFromExpression( QgsExpression* expression )
2992+
{
2993+
const QgsExpression::Node* n = expression->rootNode();
2994+
2995+
if ( n && n->nodeType() == QgsExpression::ntColumnRef )
2996+
return static_cast<const QgsExpression::NodeColumnRef*>( n )->name();
2997+
2998+
return expression->expression();
2999+
}

‎src/core/symbology-ng/qgssymbollayerv2utils.h

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
#include "qgssymbolv2.h"
2626
#include "qgis.h"
2727

28+
class QgsExpression;
2829
class QgsSymbolLayerV2;
2930
class QgsVectorColorRampV2;
3031

@@ -260,6 +261,22 @@ class CORE_EXPORT QgsSymbolLayerV2Utils
260261

261262
//! Calculate the centroid point of a QPolygonF
262263
static QPointF polygonCentroid( const QPolygonF& points );
264+
265+
/** Return a new valid expression instance for given field or expression string.
266+
* If the input is not a valid expression, it is assumed that it is a field name and gets properly quoted.
267+
* If the string is empty, returns null pointer.
268+
* This is useful when accepting input which could be either a non-quoted field name or expression.
269+
* @note added in 2.2
270+
*/
271+
static QgsExpression* fieldOrExpressionToExpression( const QString& fieldOrExpression );
272+
273+
/** Return a field name if the whole expression is just a name of the field .
274+
* Returns full expression string if the expression is more complex than just one field.
275+
* Using just expression->expression() method may return quoted field name, but that is not
276+
* wanted for saving (due to backward compatibility) or display in GUI.
277+
* @note added in 2.2
278+
*/
279+
static QString fieldOrExpressionFromExpression( QgsExpression* expression );
263280
};
264281

265282
class QPolygonF;

0 commit comments

Comments
 (0)
Please sign in to comment.