Fixes #19836

troopa81 · nyalldawson · commit ee4afe2db1fd · 2018-10-01T14:17:04.000+10:00
Keep layer order when using different way for passing layer (layer id or layer pointer for instance)
diff --git a/src/core/processing/qgsprocessingparameters.cpp b/src/core/processing/qgsprocessingparameters.cpp
@@ -1280,19 +1280,19 @@ QList<QgsMapLayer *> QgsProcessingParameters::parameterAsLayerList( const QgsPro
     {
       Q_FOREACH ( const QString &s, var.toStringList() )
       {
-        resultStringList << s;
+        processVariant( s );
       }
     }
     else if ( var.canConvert<QgsProperty>() )
-      resultStringList << var.value< QgsProperty >().valueAsString( context.expressionContext(), definition->defaultValue().toString() );
+      processVariant( var.value< QgsProperty >().valueAsString( context.expressionContext(), definition->defaultValue().toString() ) );
     else if ( var.canConvert<QgsProcessingOutputLayerDefinition>() )
     {
       // input is a QgsProcessingOutputLayerDefinition - get extra properties from it
       QgsProcessingOutputLayerDefinition fromVar = qvariant_cast<QgsProcessingOutputLayerDefinition>( var );
       QVariant sink = fromVar.sink;
       if ( sink.canConvert<QgsProperty>() )
       {
-        resultStringList << sink.value< QgsProperty >().valueAsString( context.expressionContext(), definition->defaultValue().toString() );
+        processVariant( sink.value< QgsProperty >().valueAsString( context.expressionContext(), definition->defaultValue().toString() ) );
       }
     }
     else if ( QgsMapLayer *layer = qobject_cast< QgsMapLayer * >( qvariant_cast<QObject *>( var ) ) )
@@ -1301,15 +1301,16 @@ QList<QgsMapLayer *> QgsProcessingParameters::parameterAsLayerList( const QgsPro
     }
     else
     {
-      resultStringList << var.toString();
+      QgsMapLayer *alayer = QgsProcessingUtils::mapLayerFromString( var.toString(), context );
+      if ( alayer )
+        layers << alayer;
     }
   };
 
   processVariant( val );
 
-  if ( layers.isEmpty() && ( resultStringList.isEmpty() || resultStringList.at( 0 ).isEmpty() ) )
+  if ( layers.isEmpty() )
   {
-    resultStringList.clear();
     // check default
     if ( QgsMapLayer *layer = qobject_cast< QgsMapLayer * >( qvariant_cast<QObject *>( definition->defaultValue() ) ) )
     {
@@ -1325,19 +1326,12 @@ QList<QgsMapLayer *> QgsProcessingParameters::parameterAsLayerList( const QgsPro
         }
         else
         {
-          resultStringList << var.toString();
+          processVariant( var );
         }
       }
     }
     else
-      resultStringList << definition->defaultValue().toString();
-  }
-
-  Q_FOREACH ( const QString &s, resultStringList )
-  {
-    QgsMapLayer *layer = QgsProcessingUtils::mapLayerFromString( s, context );
-    if ( layer )
-      layers << layer;
+      processVariant( definition->defaultValue() );
   }
 
   return layers;
diff --git a/tests/src/analysis/testqgsprocessing.cpp b/tests/src/analysis/testqgsprocessing.cpp
@@ -3066,6 +3066,13 @@ void TestQgsProcessing::parameterLayerList()
   QCOMPARE( layers.at( 0 ), v1 );
   QCOMPARE( layers.at( 1 )->publicSource(), raster2 );
 
+  // mix of existing layer and ID (and check we keep order)
+  params.insert( "non_optional",  QVariantList() << QVariant::fromValue( v1 ) << v2->id() );
+  QCOMPARE( QgsProcessingParameters::parameterAsLayerList( def.get(), params, context ), QList< QgsMapLayer *>() << v1 << v2 );
+
+  params.insert( "non_optional",  QVariantList() << v1->id() << QVariant::fromValue( v2 ) );
+  QCOMPARE( QgsProcessingParameters::parameterAsLayerList( def.get(), params, context ), QList< QgsMapLayer *>() << v1 << v2 );
+
   // empty string
   params.insert( "non_optional",  QString( "" ) );
   QVERIFY( QgsProcessingParameters::parameterAsLayerList( def.get(), params, context ).isEmpty() );

Original file line number	Diff line number	Diff line change
`@@ -1280,19 +1280,19 @@ QList<QgsMapLayer *> QgsProcessingParameters::parameterAsLayerList( const QgsPro`
`1280`	`1280`	`{`
`1281`	`1281`	`Q_FOREACH ( const QString &s, var.toStringList() )`
`1282`	`1282`	`{`
`1283`		`- resultStringList << s;`
	`1283`	`+ processVariant( s );`
`1284`	`1284`	`}`
`1285`	`1285`	`}`
`1286`	`1286`	`else if ( var.canConvert<QgsProperty>() )`
`1287`		`- resultStringList << var.value< QgsProperty >().valueAsString( context.expressionContext(), definition->defaultValue().toString() );`
	`1287`	`+ processVariant( var.value< QgsProperty >().valueAsString( context.expressionContext(), definition->defaultValue().toString() ) );`
`1288`	`1288`	`else if ( var.canConvert<QgsProcessingOutputLayerDefinition>() )`
`1289`	`1289`	`{`
`1290`	`1290`	`// input is a QgsProcessingOutputLayerDefinition - get extra properties from it`
`1291`	`1291`	`QgsProcessingOutputLayerDefinition fromVar = qvariant_cast<QgsProcessingOutputLayerDefinition>( var );`
`1292`	`1292`	`QVariant sink = fromVar.sink;`
`1293`	`1293`	`if ( sink.canConvert<QgsProperty>() )`
`1294`	`1294`	`{`
`1295`		`- resultStringList << sink.value< QgsProperty >().valueAsString( context.expressionContext(), definition->defaultValue().toString() );`
	`1295`	`+ processVariant( sink.value< QgsProperty >().valueAsString( context.expressionContext(), definition->defaultValue().toString() ) );`
`1296`	`1296`	`}`
`1297`	`1297`	`}`
`1298`	`1298`	`else if ( QgsMapLayer layer = qobject_cast< QgsMapLayer >( qvariant_cast<QObject *>( var ) ) )`
`@@ -1301,15 +1301,16 @@ QList<QgsMapLayer *> QgsProcessingParameters::parameterAsLayerList( const QgsPro`
`1301`	`1301`	`}`
`1302`	`1302`	`else`
`1303`	`1303`	`{`
`1304`		`- resultStringList << var.toString();`
	`1304`	`+ QgsMapLayer *alayer = QgsProcessingUtils::mapLayerFromString( var.toString(), context );`
	`1305`	`+ if ( alayer )`
	`1306`	`+ layers << alayer;`
`1305`	`1307`	`}`
`1306`	`1308`	`};`
`1307`	`1309`
`1308`	`1310`	`processVariant( val );`
`1309`	`1311`
`1310`		`- if ( layers.isEmpty() && ( resultStringList.isEmpty() \|\| resultStringList.at( 0 ).isEmpty() ) )`
	`1312`	`+ if ( layers.isEmpty() )`
`1311`	`1313`	`{`
`1312`		`- resultStringList.clear();`
`1313`	`1314`	`// check default`
`1314`	`1315`	`if ( QgsMapLayer layer = qobject_cast< QgsMapLayer >( qvariant_cast<QObject *>( definition->defaultValue() ) ) )`
`1315`	`1316`	`{`
`@@ -1325,19 +1326,12 @@ QList<QgsMapLayer *> QgsProcessingParameters::parameterAsLayerList( const QgsPro`
`1325`	`1326`	`}`
`1326`	`1327`	`else`
`1327`	`1328`	`{`
`1328`		`- resultStringList << var.toString();`
	`1329`	`+ processVariant( var );`
`1329`	`1330`	`}`
`1330`	`1331`	`}`
`1331`	`1332`	`}`
`1332`	`1333`	`else`
`1333`		`- resultStringList << definition->defaultValue().toString();`
`1334`		`- }`
`1335`		`-`
`1336`		`- Q_FOREACH ( const QString &s, resultStringList )`
`1337`		`- {`
`1338`		`- QgsMapLayer *layer = QgsProcessingUtils::mapLayerFromString( s, context );`
`1339`		`- if ( layer )`
`1340`		`- layers << layer;`
	`1334`	`+ processVariant( definition->defaultValue() );`
`1341`	`1335`	`}`
`1342`	`1336`
`1343`	`1337`	`return layers;`