Skip to content

Commit e740688

Browse files
elpasoelpaso
authoredJun 11, 2019
Merge pull request #30151 from elpaso/bugfix-gh30115-value-relation-widget-match-name-if-id-fails
When loading a QLR, try to match the layer name if id does not
·
ltr-3_40final-3_8_0
2 parents 502e18f + 2d45ba8 commit e740688

13 files changed

+230
-38
lines changed
 

‎python/core/auto_generated/fieldformatter/qgsvaluerelationfieldformatter.sip.in

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,15 @@ Check whether the ``feature`` has all values required by the ``expression``
121121
.. versionadded:: 3.2
122122
%End
123123

124+
static QgsVectorLayer *resolveLayer( const QVariantMap &config, const QgsProject *project );
125+
%Docstring
126+
Returns the (possibly NULL) layer from the widget's ``config`` and ``project``
127+
128+
.. versionadded:: 3.8
129+
%End
130+
131+
132+
124133
};
125134

126135

‎python/core/auto_generated/qgsmaplayer.sip.in

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1260,6 +1260,14 @@ The storage format for the XML is qlr
12601260
.. versionadded:: 3.6
12611261
%End
12621262

1263+
static QString generateId( const QString &layerName );
1264+
%Docstring
1265+
Generates an unique identifier for this layer, the generate ID is prefixed by ``layerName``
1266+
1267+
.. versionadded:: 3.8
1268+
%End
1269+
1270+
12631271
public slots:
12641272

12651273
void setMinimumScale( double scale );

‎src/core/fieldformatter/qgsvaluerelationfieldformatter.cpp

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121
#include "qgsexpressionnodeimpl.h"
2222
#include "qgsapplication.h"
2323
#include "qgsexpressioncontextutils.h"
24-
24+
#include "qgsvectorlayerref.h"
2525

2626
#include <nlohmann/json.hpp>
2727
using json = nlohmann::json;
@@ -118,7 +118,7 @@ QgsValueRelationFieldFormatter::ValueRelationCache QgsValueRelationFieldFormatte
118118
{
119119
ValueRelationCache cache;
120120

121-
QgsVectorLayer *layer = QgsProject::instance()->mapLayer<QgsVectorLayer *>( config.value( QStringLiteral( "Layer" ) ).toString() );
121+
const QgsVectorLayer *layer = resolveLayer( config, QgsProject::instance() );
122122

123123
if ( !layer )
124124
return cache;
@@ -273,3 +273,13 @@ bool QgsValueRelationFieldFormatter::expressionIsUsable( const QString &expressi
273273
return false;
274274
return true;
275275
}
276+
277+
QgsVectorLayer *QgsValueRelationFieldFormatter::resolveLayer( const QVariantMap &config, const QgsProject *project )
278+
{
279+
QgsVectorLayerRef ref { config.value( QStringLiteral( "Layer" ) ).toString(),
280+
config.value( QStringLiteral( "LayerName" ) ).toString(),
281+
config.value( QStringLiteral( "LayerSource" ) ).toString(),
282+
config.value( QStringLiteral( "LayerProviderName" ) ).toString() };
283+
return ref.resolveByIdOrNameOnly( project );
284+
}
285+

‎src/core/fieldformatter/qgsvaluerelationfieldformatter.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,14 @@ class CORE_EXPORT QgsValueRelationFieldFormatter : public QgsFieldFormatter
119119
*/
120120
static bool expressionIsUsable( const QString &expression, const QgsFeature &feature );
121121

122+
/**
123+
* Returns the (possibly NULL) layer from the widget's \a config and \a project
124+
* \since QGIS 3.8
125+
*/
126+
static QgsVectorLayer *resolveLayer( const QVariantMap &config, const QgsProject *project );
127+
128+
129+
122130
};
123131

124132
Q_DECLARE_METATYPE( QgsValueRelationFieldFormatter::ValueRelationCache )

‎src/core/layout/qgslayoutitemattributetable.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -303,7 +303,7 @@ class CORE_EXPORT QgsLayoutItemAttributeTable: public QgsLayoutTable
303303
//! Attribute source
304304
ContentSource mSource = LayerAttributes;
305305
//! Associated vector layer
306-
QgsVectorLayerRef mVectorLayer;
306+
QgsVectorLayerRef mVectorLayer = nullptr;
307307

308308
//! Data defined vector layer - only
309309
QPointer< QgsVectorLayer > mDataDefinedVectorLayer;

‎src/core/qgslayerdefinition.cpp

Lines changed: 38 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -87,26 +87,25 @@ bool QgsLayerDefinition::loadLayerDefinition( QDomDocument doc, QgsProject *proj
8787

8888
// IDs of layers should be changed otherwise we may have more then one layer with the same id
8989
// We have to replace the IDs before we load them because it's too late once they are loaded
90-
QDomNodeList ids = doc.elementsByTagName( QStringLiteral( "id" ) );
91-
for ( int i = 0; i < ids.size(); ++i )
90+
QDomNodeList treeLayerNodes = doc.elementsByTagName( QStringLiteral( "layer-tree-layer" ) );
91+
for ( int i = 0; i < treeLayerNodes.size(); ++i )
9292
{
93-
QDomNode idnode = ids.at( i );
94-
QDomElement idElem = idnode.toElement();
95-
QString oldid = idElem.text();
96-
// Strip the date part because we will replace it.
97-
QString layername = oldid.left( oldid.length() - 17 );
98-
QDateTime dt = QDateTime::currentDateTime();
99-
QString newid = layername + dt.toString( QStringLiteral( "yyyyMMddhhmmsszzz" ) ) + QString::number( qrand() );
100-
idElem.firstChild().setNodeValue( newid );
101-
QDomNodeList treeLayerNodes = doc.elementsByTagName( QStringLiteral( "layer-tree-layer" ) );
102-
103-
for ( int i = 0; i < treeLayerNodes.count(); ++i )
93+
QDomNode treeLayerNode = treeLayerNodes.at( i );
94+
QDomElement treeLayerElem = treeLayerNode.toElement();
95+
QString oldid = treeLayerElem.attribute( QStringLiteral( "id" ) );
96+
QString layername = treeLayerElem.attribute( QStringLiteral( "name" ) );
97+
QString newid = QgsMapLayer::generateId( layername );
98+
treeLayerElem.setAttribute( QStringLiteral( "id" ), newid );
99+
100+
// Replace IDs for map layers
101+
QDomNodeList ids = doc.elementsByTagName( QStringLiteral( "id" ) );
102+
for ( int i = 0; i < ids.size(); ++i )
104103
{
105-
QDomNode layerNode = treeLayerNodes.at( i );
106-
QDomElement layerElem = layerNode.toElement();
107-
if ( layerElem.attribute( QStringLiteral( "id" ) ) == oldid )
104+
QDomNode idnode = ids.at( i );
105+
QDomElement idElem = idnode.toElement();
106+
if ( idElem.text() == oldid )
108107
{
109-
layerNode.toElement().setAttribute( QStringLiteral( "id" ), newid );
108+
idElem.firstChild().setNodeValue( newid );
110109
}
111110
}
112111

@@ -137,6 +136,28 @@ bool QgsLayerDefinition::loadLayerDefinition( QDomDocument doc, QgsProject *proj
137136
}
138137
}
139138

139+
// Change IDs of widget config values
140+
QDomNodeList widgetConfig = doc.elementsByTagName( QStringLiteral( "editWidget" ) );
141+
for ( int i = 0; i < widgetConfig.size(); i++ )
142+
{
143+
QDomNodeList config = widgetConfig.at( i ).childNodes();
144+
for ( int j = 0; j < config.size(); j++ )
145+
{
146+
QDomNodeList optMap = config.at( j ).childNodes();
147+
for ( int z = 0; z < optMap.size(); z++ )
148+
{
149+
QDomNodeList opts = optMap.at( z ).childNodes();
150+
for ( int k = 0; k < opts.size(); k++ )
151+
{
152+
QDomElement opt = opts.at( k ).toElement();
153+
if ( opt.attribute( QStringLiteral( "value" ) ) == oldid )
154+
{
155+
opt.setAttribute( QStringLiteral( "value" ), newid );
156+
}
157+
}
158+
}
159+
}
160+
}
140161
}
141162

142163
QDomElement layerTreeElem = doc.documentElement().firstChildElement( QStringLiteral( "layer-tree-group" ) );

‎src/core/qgsmaplayer.cpp

Lines changed: 17 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -78,24 +78,9 @@ QgsMapLayer::QgsMapLayer( QgsMapLayerType type,
7878
, mStyleManager( new QgsMapLayerStyleManager( this ) )
7979
, mRefreshTimer( new QTimer( this ) )
8080
{
81-
//mShortName.replace( QRegExp( "[\\W]" ), "_" );
82-
83-
// Generate the unique ID of this layer
84-
QString uuid = QUuid::createUuid().toString();
85-
// trim { } from uuid
86-
mID = lyrname + '_' + uuid.mid( 1, uuid.length() - 2 );
87-
88-
// Tidy the ID up to avoid characters that may cause problems
89-
// elsewhere (e.g in some parts of XML). Replaces every non-word
90-
// character (word characters are the alphabet, numbers and
91-
// underscore) with an underscore.
92-
// Note that the first backslashe in the regular expression is
93-
// there for the compiler, so the pattern is actually \W
94-
mID.replace( QRegExp( "[\\W]" ), QStringLiteral( "_" ) );
95-
81+
mID = generateId( lyrname );
9682
connect( this, &QgsMapLayer::crsChanged, this, &QgsMapLayer::configChanged );
9783
connect( this, &QgsMapLayer::nameChanged, this, &QgsMapLayer::configChanged );
98-
9984
connect( mRefreshTimer, &QTimer::timeout, this, [ = ] { triggerRepaint( true ); } );
10085
}
10186

@@ -1876,6 +1861,22 @@ void QgsMapLayer::setOriginalXmlProperties( const QString &originalXmlProperties
18761861
mOriginalXmlProperties = originalXmlProperties;
18771862
}
18781863

1864+
QString QgsMapLayer::generateId( const QString &layerName )
1865+
{
1866+
// Generate the unique ID of this layer
1867+
QString uuid = QUuid::createUuid().toString();
1868+
// trim { } from uuid
1869+
QString id = layerName + '_' + uuid.mid( 1, uuid.length() - 2 );
1870+
// Tidy the ID up to avoid characters that may cause problems
1871+
// elsewhere (e.g in some parts of XML). Replaces every non-word
1872+
// character (word characters are the alphabet, numbers and
1873+
// underscore) with an underscore.
1874+
// Note that the first backslash in the regular expression is
1875+
// there for the compiler, so the pattern is actually \W
1876+
id.replace( QRegExp( "[\\W]" ), QStringLiteral( "_" ) );
1877+
return id;
1878+
}
1879+
18791880
void QgsMapLayer::setProviderType( const QString &providerType )
18801881
{
18811882
mProviderKey = providerType;

‎src/core/qgsmaplayer.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1133,6 +1133,13 @@ class CORE_EXPORT QgsMapLayer : public QObject
11331133
*/
11341134
void setOriginalXmlProperties( const QString &originalXmlProperties );
11351135

1136+
/**
1137+
* Generates an unique identifier for this layer, the generate ID is prefixed by \a layerName
1138+
* \since QGIS 3.8
1139+
*/
1140+
static QString generateId( const QString &layerName );
1141+
1142+
11361143
public slots:
11371144

11381145
/**

‎src/core/qgsmaplayerref.h

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,7 @@ struct _LayerRef
162162
* found, NULLPTR is returned.
163163
* \see resolve()
164164
* \see layerMatchesSource()
165+
* \see resolveByIdOrNameOnly()
165166
*/
166167
TYPE *resolveWeakly( const QgsProject *project )
167168
{
@@ -186,6 +187,52 @@ struct _LayerRef
186187
}
187188
return nullptr;
188189
}
190+
191+
/**
192+
* Resolves the map layer by attempting to find a matching layer
193+
* in a \a project using a weak match.
194+
*
195+
* First, the layer is attempted to match to project layers using the
196+
* layer's ID (calling this method implicitly calls resolve()).
197+
*
198+
* Failing a match by layer ID, the layer will be matched by using
199+
* the weak references to layer public source, layer name and data
200+
* provider contained in this layer reference.
201+
*
202+
* Failing a match by weak reference, the layer will be matched by using
203+
* the name only.
204+
*
205+
* If a matching layer is found, this reference will be updated to match
206+
* the found layer and the layer will be returned. If no matching layer is
207+
* found, NULLPTR is returned.
208+
* \see resolve()
209+
* \see layerMatchesSource()
210+
* \see resolveWeakly()
211+
* \since QGIS 3.8
212+
*/
213+
TYPE *resolveByIdOrNameOnly( const QgsProject *project )
214+
{
215+
// first try by matching by layer ID, or weakly by source, name and provider
216+
if ( resolveWeakly( project ) )
217+
return layer;
218+
219+
// fallback to checking by name only
220+
if ( project && !name.isEmpty() )
221+
{
222+
const QList<QgsMapLayer *> layers = project->mapLayersByName( name );
223+
for ( QgsMapLayer *l : layers )
224+
{
225+
if ( TYPE *tl = qobject_cast< TYPE *>( l ) )
226+
{
227+
setLayer( tl );
228+
return tl;
229+
}
230+
}
231+
}
232+
return nullptr;
233+
}
234+
235+
189236
};
190237

191238
typedef _LayerRef<QgsMapLayer> QgsMapLayerRef;

‎src/gui/editorwidgets/qgsvaluerelationconfigdlg.cpp

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
#include "qgsvectorlayer.h"
1919
#include "qgsexpressionbuilderdialog.h"
2020
#include "qgsexpressioncontextutils.h"
21+
#include "qgsvaluerelationfieldformatter.h"
2122

2223
QgsValueRelationConfigDlg::QgsValueRelationConfigDlg( QgsVectorLayer *vl, int fieldIdx, QWidget *parent )
2324
: QgsEditorConfigWidget( vl, fieldIdx, parent )
@@ -55,6 +56,11 @@ QVariantMap QgsValueRelationConfigDlg::config()
5556
QVariantMap cfg;
5657

5758
cfg.insert( QStringLiteral( "Layer" ), mLayerName->currentLayer() ? mLayerName->currentLayer()->id() : QString() );
59+
cfg.insert( QStringLiteral( "LayerName" ), mLayerName->currentLayer() ? mLayerName->currentLayer()->name() : QString() );
60+
cfg.insert( QStringLiteral( "LayerSource" ), mLayerName->currentLayer() ? mLayerName->currentLayer()->publicSource() : QString() );
61+
cfg.insert( QStringLiteral( "LayerProviderName" ), ( mLayerName->currentLayer() && mLayerName->currentLayer()->dataProvider() ) ?
62+
mLayerName->currentLayer()->dataProvider()->name() :
63+
QString() );
5864
cfg.insert( QStringLiteral( "Key" ), mKeyColumn->currentField() );
5965
cfg.insert( QStringLiteral( "Value" ), mValueColumn->currentField() );
6066
cfg.insert( QStringLiteral( "AllowMulti" ), mAllowMulti->isChecked() );
@@ -69,7 +75,7 @@ QVariantMap QgsValueRelationConfigDlg::config()
6975

7076
void QgsValueRelationConfigDlg::setConfig( const QVariantMap &config )
7177
{
72-
QgsVectorLayer *lyr = QgsProject::instance()->mapLayer<QgsVectorLayer *>( config.value( QStringLiteral( "Layer" ) ).toString() );
78+
QgsVectorLayer *lyr = QgsValueRelationFieldFormatter::resolveLayer( config, QgsProject::instance() );
7379
mLayerName->setLayer( lyr );
7480
mKeyColumn->setField( config.value( QStringLiteral( "Key" ) ).toString() );
7581
mValueColumn->setField( config.value( QStringLiteral( "Value" ) ).toString() );

‎src/gui/editorwidgets/qgsvaluerelationwidgetwrapper.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -299,9 +299,10 @@ int QgsValueRelationWidgetWrapper::columnCount() const
299299
return std::max( 1, config( QStringLiteral( "NofColumns" ) ).toInt() );
300300
}
301301

302+
302303
QVariant::Type QgsValueRelationWidgetWrapper::fkType() const
303304
{
304-
QgsVectorLayer *layer = QgsProject::instance()->mapLayer<QgsVectorLayer *>( config().value( QStringLiteral( "Layer" ) ).toString() );
305+
const QgsVectorLayer *layer = QgsValueRelationFieldFormatter::resolveLayer( config(), QgsProject::instance() );
305306
if ( layer )
306307
{
307308
QgsFields fields = layer->fields();

‎tests/src/core/testqgsmaplayer.cpp

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@ class TestQgsMapLayer : public QObject
7272

7373
void layerRef();
7474
void layerRefListUtils();
75+
void layerRefResolveByIdOrNameOnly();
7576

7677
void styleCategories();
7778

@@ -287,6 +288,25 @@ void TestQgsMapLayer::layerRefListUtils()
287288
QCOMPARE( refs.at( 1 ).get(), vlC );
288289
}
289290

291+
void TestQgsMapLayer::layerRefResolveByIdOrNameOnly()
292+
{
293+
QgsVectorLayer *vlA = new QgsVectorLayer( QStringLiteral( "Point" ), QStringLiteral( "name" ), QStringLiteral( "memory" ) );
294+
QgsVectorLayerRef ref;
295+
QgsProject::instance()->addMapLayer( vlA );
296+
ref.name = vlA->name();
297+
QCOMPARE( ref.resolveByIdOrNameOnly( QgsProject::instance() ), vlA );
298+
ref.layerId = vlA->id();
299+
// Same name, different id
300+
QgsVectorLayer *vlB = new QgsVectorLayer( QStringLiteral( "Point" ), QStringLiteral( "name" ), QStringLiteral( "memory" ) );
301+
QgsProject::instance()->addMapLayer( vlB );
302+
QCOMPARE( ref.resolveByIdOrNameOnly( QgsProject::instance() ), vlA );
303+
// Remove layer A and check if B is returned (because they have the same name)
304+
QgsProject::instance()->removeMapLayer( vlA );
305+
QCOMPARE( ref.resolveByIdOrNameOnly( QgsProject::instance() ), vlB );
306+
// Cleanup
307+
QgsProject::instance()->removeAllMapLayers();
308+
}
309+
290310
void TestQgsMapLayer::styleCategories()
291311
{
292312
// control that AllStyleCategories is actually complete

‎tests/src/gui/testqgsvaluerelationwidgetwrapper.cpp

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ class TestQgsValueRelationWidgetWrapper : public QObject
5353
void testWithJsonInGPKG();
5454
void testWithJsonInSpatialite();
5555
void testWithJsonInSpatialiteTextFk();
56+
void testMatchLayerName();
5657
};
5758

5859
void TestQgsValueRelationWidgetWrapper::initTestCase()
@@ -921,5 +922,58 @@ void TestQgsValueRelationWidgetWrapper::testWithJsonInSpatialiteTextFk()
921922

922923
}
923924

925+
void TestQgsValueRelationWidgetWrapper::testMatchLayerName()
926+
{
927+
// create a vector layer
928+
QgsVectorLayer vl1( QStringLiteral( "Polygon?crs=epsg:4326&field=pk:int&field=province:int&field=municipality:string" ), QStringLiteral( "vl1" ), QStringLiteral( "memory" ) );
929+
QgsVectorLayer vl2( QStringLiteral( "Point?crs=epsg:4326&field=pk:int&field=fk_province:int&field=fk_municipality:int" ), QStringLiteral( "vl2" ), QStringLiteral( "memory" ) );
930+
QgsProject::instance()->addMapLayer( &vl1, false, false );
931+
QgsProject::instance()->addMapLayer( &vl2, false, false );
932+
933+
// insert some features
934+
QgsFeature f1( vl1.fields() );
935+
f1.setAttribute( QStringLiteral( "pk" ), 0 ); // !!! Notice: pk 0
936+
f1.setAttribute( QStringLiteral( "province" ), 123 );
937+
f1.setAttribute( QStringLiteral( "municipality" ), QStringLiteral( "Some Place By The River" ) );
938+
f1.setGeometry( QgsGeometry::fromWkt( QStringLiteral( "POLYGON(( 0 0, 0 1, 1 1, 1 0, 0 0 ))" ) ) );
939+
QVERIFY( f1.isValid() );
940+
QgsFeature f2( vl1.fields() );
941+
f2.setAttribute( QStringLiteral( "pk" ), 2 );
942+
f2.setAttribute( QStringLiteral( "province" ), 245 );
943+
f2.setAttribute( QStringLiteral( "municipality" ), QStringLiteral( "Dreamland By The Clouds" ) );
944+
f2.setGeometry( QgsGeometry::fromWkt( QStringLiteral( "POLYGON(( 1 0, 1 1, 2 1, 2 0, 1 0 ))" ) ) );
945+
QVERIFY( f2.isValid() );
946+
QVERIFY( vl1.dataProvider()->addFeatures( QgsFeatureList() << f1 << f2 ) );
947+
948+
QgsFeature f3( vl2.fields() );
949+
f3.setAttribute( QStringLiteral( "fk_province" ), 123 );
950+
f3.setAttribute( QStringLiteral( "fk_municipality" ), QStringLiteral( "0" ) );
951+
f3.setGeometry( QgsGeometry::fromWkt( QStringLiteral( "POINT( 0.5 0.5)" ) ) );
952+
QVERIFY( f3.isValid() );
953+
QVERIFY( f3.geometry().isGeosValid() );
954+
QVERIFY( vl2.dataProvider()->addFeature( f3 ) );
955+
956+
// build a value relation widget wrapper for municipality
957+
QgsValueRelationWidgetWrapper w_municipality( &vl2, vl2.fields().indexOf( QStringLiteral( "fk_municipality" ) ), nullptr, nullptr );
958+
QVariantMap cfg_municipality;
959+
cfg_municipality.insert( QStringLiteral( "Layer" ), QStringLiteral( "wrong_id_here_hope_name_is_good" ) );
960+
cfg_municipality.insert( QStringLiteral( "LayerName" ), vl1.name() );
961+
cfg_municipality.insert( QStringLiteral( "Key" ), QStringLiteral( "pk" ) );
962+
cfg_municipality.insert( QStringLiteral( "Value" ), QStringLiteral( "municipality" ) );
963+
cfg_municipality.insert( QStringLiteral( "AllowMulti" ), false );
964+
cfg_municipality.insert( QStringLiteral( "NofColumns" ), 1 );
965+
cfg_municipality.insert( QStringLiteral( "AllowNull" ), true );
966+
cfg_municipality.insert( QStringLiteral( "OrderByValue" ), false );
967+
cfg_municipality.insert( QStringLiteral( "UseCompleter" ), false );
968+
w_municipality.setConfig( cfg_municipality );
969+
w_municipality.widget();
970+
w_municipality.setEnabled( true );
971+
972+
w_municipality.setValue( 0 );
973+
QCOMPARE( w_municipality.mComboBox->currentIndex(), 1 );
974+
QCOMPARE( w_municipality.mComboBox->currentText(), QStringLiteral( "Some Place By The River" ) );
975+
976+
}
977+
924978
QGSTEST_MAIN( TestQgsValueRelationWidgetWrapper )
925979
#include "testqgsvaluerelationwidgetwrapper.moc"

0 commit comments

Comments
 (0)
Please sign in to comment.