Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[processing] Gracefully handle FID constraints when writing layers to temporary provider-compatible formats #58028

Merged
merged 1 commit into from
Jul 9, 2024
Merged

[processing] Gracefully handle FID constraints when writing layers to temporary provider-compatible formats #58028

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 21 additions & 5 deletions src/core/processing/qgsprocessingutils.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1350,7 +1350,7 @@ QString QgsProcessingUtils::formatHelpMapAsHtml( const QVariantMap &map, const Q
}

QString convertToCompatibleFormatInternal( const QgsVectorLayer *vl, bool selectedFeaturesOnly, const QString &baseName, const QStringList &compatibleFormats, const QString &preferredFormat, QgsProcessingContext &context, QgsProcessingFeedback *feedback, QString *layerName,
long long featureLimit, const QString &filterExpression )
long long featureLimit, const QString &filterExpression, bool renameFid )
{
bool requiresTranslation = false;

Expand Down Expand Up @@ -1412,7 +1412,14 @@ QString convertToCompatibleFormatInternal( const QgsVectorLayer *vl, bool select
QgsVectorFileWriter::SaveVectorOptions saveOptions;
saveOptions.fileEncoding = context.defaultEncoding();
saveOptions.driverName = QgsVectorFileWriter::driverForExtension( preferredFormat );
std::unique_ptr< QgsVectorFileWriter > writer( QgsVectorFileWriter::create( temp, vl->fields(), vl->wkbType(), vl->crs(), context.transformContext(), saveOptions ) );
QgsFields fields = vl->fields();
if ( renameFid )
{
const int fidIndex = fields.lookupField( QStringLiteral( "fid" ) );
if ( fidIndex >= 0 )
fields.rename( fidIndex, QStringLiteral( "OLD_FID" ) );
}
std::unique_ptr< QgsVectorFileWriter > writer( QgsVectorFileWriter::create( temp, fields, vl->wkbType(), vl->crs(), context.transformContext(), saveOptions ) );
QgsFeature f;
QgsFeatureIterator it;
QgsFeatureRequest request;
Expand All @@ -1439,10 +1446,19 @@ QString convertToCompatibleFormatInternal( const QgsVectorLayer *vl, bool select

if ( !writer->addFeature( f, QgsFeatureSink::FastInsert ) && feedback )
{
const QString errorMessage = writer->errorMessage();
if ( !renameFid && saveOptions.driverName == QLatin1String( "GPKG" ) && errorMessage.contains( "fid", Qt::CaseInsensitive ) )
{
// try again, dropping the FID field
feedback->reportError( QObject::tr( "Cannot store existing FID values in temporary GeoPackage layer, these will be moved to \"OLD_FID\" instead." ), false );
return convertToCompatibleFormatInternal( vl, selectedFeaturesOnly, baseName, compatibleFormats, preferredFormat, context, feedback, layerName,
featureLimit, filterExpression, true );
}

QString errorText;
if ( errorCounter++ < maxErrors )
{
errorText = QObject::tr( "Error writing feature # %1 to output layer: %2" ).arg( QString::number( f.id() ), writer->errorMessage() );
errorText = QObject::tr( "Error writing feature # %1 to output layer: %2" ).arg( QString::number( f.id() ), errorMessage );

feedback->reportError( errorText );
}
Expand All @@ -1462,13 +1478,13 @@ QString convertToCompatibleFormatInternal( const QgsVectorLayer *vl, bool select

QString QgsProcessingUtils::convertToCompatibleFormat( const QgsVectorLayer *vl, bool selectedFeaturesOnly, const QString &baseName, const QStringList &compatibleFormats, const QString &preferredFormat, QgsProcessingContext &context, QgsProcessingFeedback *feedback, long long featureLimit, const QString &filterExpression )
{
return convertToCompatibleFormatInternal( vl, selectedFeaturesOnly, baseName, compatibleFormats, preferredFormat, context, feedback, nullptr, featureLimit, filterExpression );
return convertToCompatibleFormatInternal( vl, selectedFeaturesOnly, baseName, compatibleFormats, preferredFormat, context, feedback, nullptr, featureLimit, filterExpression, false );
}

QString QgsProcessingUtils::convertToCompatibleFormatAndLayerName( const QgsVectorLayer *layer, bool selectedFeaturesOnly, const QString &baseName, const QStringList &compatibleFormats, const QString &preferredFormat, QgsProcessingContext &context, QgsProcessingFeedback *feedback, QString &layerName, long long featureLimit, const QString &filterExpression )
{
layerName.clear();
return convertToCompatibleFormatInternal( layer, selectedFeaturesOnly, baseName, compatibleFormats, preferredFormat, context, feedback, &layerName, featureLimit, filterExpression );
return convertToCompatibleFormatInternal( layer, selectedFeaturesOnly, baseName, compatibleFormats, preferredFormat, context, feedback, &layerName, featureLimit, filterExpression, false );
}

QgsFields QgsProcessingUtils::combineFields( const QgsFields &fieldsA, const QgsFields &fieldsB, const QString &fieldsBPrefix )
Expand Down
15 changes: 11 additions & 4 deletions tests/src/analysis/testqgsprocessing.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12672,11 +12672,18 @@ void TestQgsProcessing::convertCompatibleDuplicateFids()
QVariantMap params;
params.insert( QStringLiteral( "source" ), QgsProcessingFeatureSourceDefinition( layer->id(), false ) );

QgsProcessingParameters::parameterAsCompatibleSourceLayerPath( def.get(), params, context, QStringList() << "gpkg", QString( "gpkg" ), &feedback );
const QString temporaryFile = QgsProcessingParameters::parameterAsCompatibleSourceLayerPath( def.get(), params, context, QStringList() << "gpkg", QString( "gpkg" ), &feedback );
const QStringList logs( feedback.textLog().split( '\n', Qt::SplitBehaviorFlags::SkipEmptyParts ) );
QCOMPARE( logs.size(), 11 );
QVERIFY( logs.first().startsWith( QLatin1String( "Error writing feature # 2" ) ) );
QVERIFY( logs.last().startsWith( QLatin1String( "There were 11 errors writing features" ) ) );
QCOMPARE( logs.size(), 1 );
QCOMPARE( logs.at( 0 ), QStringLiteral( "Cannot store existing FID values in temporary GeoPackage layer, these will be moved to \"OLD_FID\" instead." ) );

QgsVectorLayer vl( temporaryFile );
QVERIFY( vl.isValid() );

QCOMPARE( vl.featureCount(), 12 );
QCOMPARE( vl.fields().at( 0 ).name(), QStringLiteral( "fid" ) );
QCOMPARE( vl.fields().at( 1 ).name(), QStringLiteral( "OLD_FID" ) );
QCOMPARE( vl.fields().at( 2 ).name(), QStringLiteral( "name" ) );
}

void TestQgsProcessing::create()
Expand Down
Loading