Skip to content

Commit

Permalink
Merge pull request #8392 from m-kuhn/mergeDuplicateFidBackport
Browse files Browse the repository at this point in the history
Processing: GeoPackage output, regenerate fid when there's a risk of duplicates
  • Loading branch information
m-kuhn committed Nov 1, 2018
2 parents 891c16a + 1ad4123 commit 7ac3d0e
Show file tree
Hide file tree
Showing 28 changed files with 236 additions and 51 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -611,7 +611,7 @@ Evaluates the parameter with matching ``name`` to a static boolean value.
%End

QgsFeatureSink *parameterAsSink( const QVariantMap &parameters, const QString &name, QgsProcessingContext &context, QString &destinationIdentifier /Out/,
const QgsFields &fields, QgsWkbTypes::Type geometryType = QgsWkbTypes::NoGeometry, const QgsCoordinateReferenceSystem &crs = QgsCoordinateReferenceSystem() ) const /Factory/;
const QgsFields &fields, QgsWkbTypes::Type geometryType = QgsWkbTypes::NoGeometry, const QgsCoordinateReferenceSystem &crs = QgsCoordinateReferenceSystem(), QgsFeatureSink::SinkFlags sinkFlags = 0 ) const /Factory/;
%Docstring
Evaluates the parameter with matching ``name`` to a feature sink.

Expand Down Expand Up @@ -922,6 +922,13 @@ this is possible to determine in advance.
virtual QgsProcessingFeatureSource::Flag sourceFlags() const;
%Docstring
Returns the processing feature source flags to be used in the algorithm.
%End

virtual QgsFeatureSink::SinkFlags sinkFlags() const;
%Docstring
Returns the feature sink flags to be used for the output.

.. versionadded:: 3.4.1
%End

virtual QgsWkbTypes::Type outputWkbType( QgsWkbTypes::Type inputWkbType ) const;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -588,7 +588,7 @@ Evaluates the parameter with matching ``definition`` and ``value`` to a static b

static QgsFeatureSink *parameterAsSink( const QgsProcessingParameterDefinition *definition, const QVariantMap &parameters,
const QgsFields &fields, QgsWkbTypes::Type geometryType, const QgsCoordinateReferenceSystem &crs,
QgsProcessingContext &context, QString &destinationIdentifier /Out/ ) /Factory/;
QgsProcessingContext &context, QString &destinationIdentifier /Out/, QgsFeatureSink::SinkFlags sinkFlags = 0 ) /Factory/;
%Docstring
Evaluates the parameter with matching ``definition`` to a feature sink.

Expand All @@ -605,7 +605,7 @@ This function creates a new object and the caller takes responsibility for delet

static QgsFeatureSink *parameterAsSink( const QgsProcessingParameterDefinition *definition, const QVariant &value,
const QgsFields &fields, QgsWkbTypes::Type geometryType, const QgsCoordinateReferenceSystem &crs,
QgsProcessingContext &context, QString &destinationIdentifier /Out/ ) /Factory/;
QgsProcessingContext &context, QString &destinationIdentifier /Out/, QgsFeatureSink::SinkFlags sinkFlags = 0 ) /Factory/;
%Docstring
Evaluates the parameter with matching ``definition`` and ``value`` to a feature sink.

Expand Down
8 changes: 8 additions & 0 deletions python/core/auto_generated/qgsfeaturesink.sip.in
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,14 @@ An interface for objects which accept features via addFeature(s) methods.
%End
public:

enum SinkFlag
{

RegeneratePrimaryKey,
};
typedef QFlags<QgsFeatureSink::SinkFlag> SinkFlags;


enum Flag
{

Expand Down
3 changes: 2 additions & 1 deletion python/core/auto_generated/qgsvectorfilewriter.sip.in
Original file line number Diff line number Diff line change
Expand Up @@ -408,7 +408,8 @@ Writes a layer out to a vector file.
const QStringList &datasourceOptions = QStringList(),
const QStringList &layerOptions = QStringList(),
QString *newFilename = 0,
QgsVectorFileWriter::SymbologyExport symbologyExport = QgsVectorFileWriter::NoSymbology
QgsVectorFileWriter::SymbologyExport symbologyExport = QgsVectorFileWriter::NoSymbology,
QgsFeatureSink::SinkFlags sinkFlags = 0
);


Expand Down
4 changes: 3 additions & 1 deletion python/core/auto_generated/qgsvectorlayerexporter.sip.in
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,8 @@ Writes the contents of vector layer to a different datasource.
QgsWkbTypes::Type geometryType,
const QgsCoordinateReferenceSystem &crs,
bool overwrite = false,
const QMap<QString, QVariant> &options = QMap<QString, QVariant>() );
const QMap<QString, QVariant> &options = QMap<QString, QVariant>(),
QgsFeatureSink::SinkFlags sinkFlags = 0 );
%Docstring
Constructor for QgsVectorLayerExporter.

Expand All @@ -89,6 +90,7 @@ Constructor for QgsVectorLayerExporter.
not available
:param overwrite: set to true to overwrite any existing data source
:param options: optional provider dataset options
:param sinkFlags: for how to add features
%End


Expand Down
2 changes: 1 addition & 1 deletion python/plugins/processing/algs/qgis/PointsAlongGeometry.py
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ def processAlgorithm(self, parameters, context, feedback):
fields.append(QgsField('angle', QVariant.Double))

(sink, dest_id) = self.parameterAsSink(parameters, self.OUTPUT, context,
fields, QgsWkbTypes.Point, source.sourceCrs())
fields, QgsWkbTypes.Point, source.sourceCrs(), QgsFeatureSink.RegeneratePrimaryKey)
if sink is None:
raise QgsProcessingException(self.invalidSinkError(parameters, self.OUTPUT))

Expand Down
4 changes: 2 additions & 2 deletions python/plugins/processing/algs/qgis/SpatialJoin.py
Original file line number Diff line number Diff line change
Expand Up @@ -184,12 +184,12 @@ def processAlgorithm(self, parameters, context, feedback):
out_fields = QgsProcessingUtils.combineFields(source_fields, fields_to_join)

(sink, dest_id) = self.parameterAsSink(parameters, self.OUTPUT, context,
out_fields, source.wkbType(), source.sourceCrs())
out_fields, source.wkbType(), source.sourceCrs(), QgsFeatureSink.RegeneratePrimaryKey)
if self.OUTPUT in parameters and parameters[self.OUTPUT] is not None and sink is None:
raise QgsProcessingException(self.invalidSinkError(parameters, self.OUTPUT))

(non_matching_sink, non_matching_dest_id) = self.parameterAsSink(parameters, self.NON_MATCHING, context,
source.fields(), source.wkbType(), source.sourceCrs())
source.fields(), source.wkbType(), source.sourceCrs(), QgsFeatureSink.RegeneratePrimaryKey)
if self.NON_MATCHING in parameters and parameters[self.NON_MATCHING] is not None and non_matching_sink is None:
raise QgsProcessingException(self.invalidSinkError(parameters, self.NON_MATCHING))

Expand Down
37 changes: 28 additions & 9 deletions python/plugins/processing/tests/AlgorithmsTestBase.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
import glob
import hashlib
import tempfile
import re

from osgeo.gdalconst import GA_ReadOnly
from numpy import nan_to_num
Expand Down Expand Up @@ -199,7 +200,8 @@ def load_result_param(self, param):
basename = os.path.basename(param['name'])
else:
basename = os.path.basename(param['name'][0])
filepath = os.path.join(outdir, basename)

filepath = self.uri_path_join(outdir, basename)
return filepath
elif param['type'] == 'rasterhash':
outdir = tempfile.mkdtemp()
Expand All @@ -215,13 +217,14 @@ def load_result_param(self, param):

def load_layers(self, id, param):
layers = []
if param['type'] in ('vector', 'table') and isinstance(param['name'], str):
layers.append(self.load_layer(id, param))
elif param['type'] in ('vector', 'table'):
for n in param['name']:
layer_param = deepcopy(param)
layer_param['name'] = n
layers.append(self.load_layer(id, layer_param))
if param['type'] in ('vector', 'table'):
if isinstance(param['name'], str) or 'uri' in param:
layers.append(self.load_layer(id, param))
else:
for n in param['name']:
layer_param = deepcopy(param)
layer_param['name'] = n
layers.append(self.load_layer(id, layer_param))
else:
layers.append(self.load_layer(id, param))
return layers
Expand All @@ -230,6 +233,7 @@ def load_layer(self, id, param):
"""
Loads a layer which was specified as parameter.
"""

filepath = self.filepath_from_param(param)

if 'in_place' in param and param['in_place']:
Expand Down Expand Up @@ -268,7 +272,22 @@ def filepath_from_param(self, param):
if 'location' in param and param['location'] == 'qgs':
prefix = unitTestDataPath()

return os.path.join(prefix, param['name'])
if 'uri' in param:
path = param['uri']
else:
path = param['name']

return self.uri_path_join(prefix, path)

def uri_path_join(self, prefix, filepath):
if filepath.startswith('ogr:'):
if not prefix[-1] == os.path.sep:
prefix += os.path.sep
filepath = re.sub(r"dbname='", "dbname='{}".format(prefix), filepath)
else:
filepath = os.path.join(prefix, filepath)

return filepath

def check_results(self, results, context, params, expected):
"""
Expand Down
Binary file not shown.
Binary file not shown.
23 changes: 23 additions & 0 deletions python/plugins/processing/tests/testdata/qgis_algorithm_tests.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -6232,5 +6232,28 @@ tests:
name: expected/interpolate_point_polys.gml
type: vector

- algorithm: native:mergevectorlayers
name: Merge vector layers with conflicting feature ids
params:
LAYERS:
params:
- name: custom/pol.gpkg|layername=pol1
type: vector
- name: custom/pol.gpkg|layername=pol2
type: vector
- name: custom/pol.gpkg|layername=pol3
type: vector
type: multi
results:
OUTPUT:
# If you ever run into this test producing Polygons instead of MultiPolygons as output
# that is totally expected and you are invited to replace the file merged_pol.gpkg with
# a single polygon version.
name: ogr:dbname='expected/merged_pol.gpkg' table="output" (geom) sql=
uri: expected/merged_pol.gpkg|layername=merged_pol
type: vector
compare:
fields:
path: skip

# See ../README.md for a description of the file format
4 changes: 2 additions & 2 deletions src/analysis/processing/qgsalgorithmjoinbyattribute.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -165,13 +165,13 @@ QVariantMap QgsJoinByAttributeAlgorithm::processAlgorithm( const QVariantMap &pa

QString dest;
std::unique_ptr< QgsFeatureSink > sink( parameterAsSink( parameters, QStringLiteral( "OUTPUT" ), context, dest, outFields,
input->wkbType(), input->sourceCrs() ) );
input->wkbType(), input->sourceCrs(), QgsFeatureSink::RegeneratePrimaryKey ) );
if ( parameters.value( QStringLiteral( "OUTPUT" ) ).isValid() && !sink )
throw QgsProcessingException( invalidSinkError( parameters, QStringLiteral( "OUTPUT" ) ) );

QString destNonMatching1;
std::unique_ptr< QgsFeatureSink > sinkNonMatching1( parameterAsSink( parameters, QStringLiteral( "NON_MATCHING" ), context, destNonMatching1, input->fields(),
input->wkbType(), input->sourceCrs() ) );
input->wkbType(), input->sourceCrs(), QgsFeatureSink::RegeneratePrimaryKey ) );
if ( parameters.value( QStringLiteral( "NON_MATCHING" ) ).isValid() && !sinkNonMatching1 )
throw QgsProcessingException( invalidSinkError( parameters, QStringLiteral( "NON_MATCHING" ) ) );

Expand Down
2 changes: 1 addition & 1 deletion src/analysis/processing/qgsalgorithmmergevector.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ QVariantMap QgsMergeVectorAlgorithm::processAlgorithm( const QVariantMap &parame
}

QString dest;
std::unique_ptr< QgsFeatureSink > sink( parameterAsSink( parameters, QStringLiteral( "OUTPUT" ), context, dest, outputFields, outputType, outputCrs ) );
std::unique_ptr< QgsFeatureSink > sink( parameterAsSink( parameters, QStringLiteral( "OUTPUT" ), context, dest, outputFields, outputType, outputCrs, QgsFeatureSink::RegeneratePrimaryKey ) );
if ( !sink )
throw QgsProcessingException( invalidSinkError( parameters, QStringLiteral( "OUTPUT" ) ) );

Expand Down
5 changes: 5 additions & 0 deletions src/analysis/processing/qgsalgorithmmultiparttosinglepart.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,11 @@ QgsProcessingFeatureSource::Flag QgsMultipartToSinglepartAlgorithm::sourceFlags(
return QgsProcessingFeatureSource::FlagSkipGeometryValidityChecks;
}

QgsFeatureSink::SinkFlags QgsMultipartToSinglepartAlgorithm::sinkFlags() const
{
return QgsFeatureSink::RegeneratePrimaryKey;
}

QgsFeatureList QgsMultipartToSinglepartAlgorithm::processFeature( const QgsFeature &feature, QgsProcessingContext &, QgsProcessingFeedback * )
{
if ( !feature.hasGeometry() )
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ class QgsMultipartToSinglepartAlgorithm : public QgsProcessingFeatureBasedAlgori
protected:

QgsProcessingFeatureSource::Flag sourceFlags() const override;
QgsFeatureSink::SinkFlags sinkFlags() const override;
QgsFeatureList processFeature( const QgsFeature &feature,
QgsProcessingContext &context, QgsProcessingFeedback *feedback ) override;

Expand Down
5 changes: 4 additions & 1 deletion src/app/qgsgeometryvalidationservice.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -348,6 +348,9 @@ void QgsGeometryValidationService::processFeature( QgsVectorLayer *layer, QgsFea
if ( !allErrors.empty() )
mLayerChecks[layer].singleFeatureCheckErrors.insert( fid, allErrors );

if ( !mLayerChecks[layer].singleFeatureCheckErrors.empty() )
layer->setAllowCommit( false );

emit geometryCheckCompleted( layer, fid, allErrors );
}

Expand Down Expand Up @@ -432,7 +435,7 @@ void QgsGeometryValidationService::triggerTopologyChecks( QgsVectorLayer *layer
connect( futureWatcher, &QFutureWatcherBase::finished, this, [&allErrors, layer, feedbacks, futureWatcher, this]()
{
QgsReadWriteLocker errorLocker( mTopologyCheckLock, QgsReadWriteLocker::Read );
layer->setAllowCommit( allErrors.empty() );
layer->setAllowCommit( allErrors.empty() && mLayerChecks[layer].singleFeatureCheckErrors.empty() );
errorLocker.unlock();
qDeleteAll( feedbacks.values() );
futureWatcher->deleteLater();
Expand Down
12 changes: 9 additions & 3 deletions src/core/processing/qgsprocessingalgorithm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -586,9 +586,9 @@ bool QgsProcessingAlgorithm::parameterAsBool( const QVariantMap &parameters, con
return QgsProcessingParameters::parameterAsBool( parameterDefinition( name ), parameters, context );
}

QgsFeatureSink *QgsProcessingAlgorithm::parameterAsSink( const QVariantMap &parameters, const QString &name, QgsProcessingContext &context, QString &destinationIdentifier, const QgsFields &fields, QgsWkbTypes::Type geometryType, const QgsCoordinateReferenceSystem &crs ) const
QgsFeatureSink *QgsProcessingAlgorithm::parameterAsSink( const QVariantMap &parameters, const QString &name, QgsProcessingContext &context, QString &destinationIdentifier, const QgsFields &fields, QgsWkbTypes::Type geometryType, const QgsCoordinateReferenceSystem &crs, QgsFeatureSink::SinkFlags sinkFlags ) const
{
return QgsProcessingParameters::parameterAsSink( parameterDefinition( name ), parameters, fields, geometryType, crs, context, destinationIdentifier );
return QgsProcessingParameters::parameterAsSink( parameterDefinition( name ), parameters, fields, geometryType, crs, context, destinationIdentifier, sinkFlags );
}

QgsProcessingFeatureSource *QgsProcessingAlgorithm::parameterAsSource( const QVariantMap &parameters, const QString &name, QgsProcessingContext &context ) const
Expand Down Expand Up @@ -824,6 +824,11 @@ QgsProcessingFeatureSource::Flag QgsProcessingFeatureBasedAlgorithm::sourceFlags
return static_cast<QgsProcessingFeatureSource::Flag>( 0 );
}

QgsFeatureSink::SinkFlags QgsProcessingFeatureBasedAlgorithm::sinkFlags() const
{
return nullptr;
}

QgsWkbTypes::Type QgsProcessingFeatureBasedAlgorithm::outputWkbType( QgsWkbTypes::Type inputWkbType ) const
{
return inputWkbType;
Expand Down Expand Up @@ -858,7 +863,8 @@ QVariantMap QgsProcessingFeatureBasedAlgorithm::processAlgorithm( const QVariant
std::unique_ptr< QgsFeatureSink > sink( parameterAsSink( parameters, QStringLiteral( "OUTPUT" ), context, dest,
outputFields( mSource->fields() ),
outputWkbType( mSource->wkbType() ),
outputCrs( mSource->sourceCrs() ) ) );
outputCrs( mSource->sourceCrs() ),
sinkFlags() ) );
if ( !sink )
throw QgsProcessingException( invalidSinkError( parameters, QStringLiteral( "OUTPUT" ) ) );

Expand Down
9 changes: 8 additions & 1 deletion src/core/processing/qgsprocessingalgorithm.h
Original file line number Diff line number Diff line change
Expand Up @@ -619,7 +619,7 @@ class CORE_EXPORT QgsProcessingAlgorithm
* This function creates a new object and the caller takes responsibility for deleting the returned object.
*/
QgsFeatureSink *parameterAsSink( const QVariantMap &parameters, const QString &name, QgsProcessingContext &context, QString &destinationIdentifier SIP_OUT,
const QgsFields &fields, QgsWkbTypes::Type geometryType = QgsWkbTypes::NoGeometry, const QgsCoordinateReferenceSystem &crs = QgsCoordinateReferenceSystem() ) const SIP_FACTORY;
const QgsFields &fields, QgsWkbTypes::Type geometryType = QgsWkbTypes::NoGeometry, const QgsCoordinateReferenceSystem &crs = QgsCoordinateReferenceSystem(), QgsFeatureSink::SinkFlags sinkFlags = nullptr ) const SIP_FACTORY;

/**
* Evaluates the parameter with matching \a name to a feature source.
Expand Down Expand Up @@ -929,6 +929,13 @@ class CORE_EXPORT QgsProcessingFeatureBasedAlgorithm : public QgsProcessingAlgor
*/
virtual QgsProcessingFeatureSource::Flag sourceFlags() const;

/**
* Returns the feature sink flags to be used for the output.
*
* \since QGIS 3.4.1
*/
virtual QgsFeatureSink::SinkFlags sinkFlags() const;

/**
* Maps the input WKB geometry type (\a inputWkbType) to the corresponding
* output WKB type generated by the algorithm. The default behavior is that the algorithm maintains
Expand Down
10 changes: 5 additions & 5 deletions src/core/processing/qgsprocessingparameters.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -350,18 +350,18 @@ bool QgsProcessingParameters::parameterAsBool( const QgsProcessingParameterDefin

QgsFeatureSink *QgsProcessingParameters::parameterAsSink( const QgsProcessingParameterDefinition *definition, const QVariantMap &parameters, const QgsFields &fields,
QgsWkbTypes::Type geometryType, const QgsCoordinateReferenceSystem &crs,
QgsProcessingContext &context, QString &destinationIdentifier )
QgsProcessingContext &context, QString &destinationIdentifier, QgsFeatureSink::SinkFlags sinkFlags )
{
QVariant val;
if ( definition )
{
val = parameters.value( definition->name() );
}

return parameterAsSink( definition, val, fields, geometryType, crs, context, destinationIdentifier );
return parameterAsSink( definition, val, fields, geometryType, crs, context, destinationIdentifier, sinkFlags );
}

QgsFeatureSink *QgsProcessingParameters::parameterAsSink( const QgsProcessingParameterDefinition *definition, const QVariant &value, const QgsFields &fields, QgsWkbTypes::Type geometryType, const QgsCoordinateReferenceSystem &crs, QgsProcessingContext &context, QString &destinationIdentifier )
QgsFeatureSink *QgsProcessingParameters::parameterAsSink( const QgsProcessingParameterDefinition *definition, const QVariant &value, const QgsFields &fields, QgsWkbTypes::Type geometryType, const QgsCoordinateReferenceSystem &crs, QgsProcessingContext &context, QString &destinationIdentifier, QgsFeatureSink::SinkFlags sinkFlags )
{
QVariant val = value;

Expand All @@ -374,6 +374,7 @@ QgsFeatureSink *QgsProcessingParameters::parameterAsSink( const QgsProcessingPar
QgsProcessingOutputLayerDefinition fromVar = qvariant_cast<QgsProcessingOutputLayerDefinition>( val );
destinationProject = fromVar.destinationProject;
createOptions = fromVar.createOptions;

val = fromVar.sink;
destName = fromVar.destinationName;
}
Expand Down Expand Up @@ -401,7 +402,7 @@ QgsFeatureSink *QgsProcessingParameters::parameterAsSink( const QgsProcessingPar
if ( dest.isEmpty() )
return nullptr;

std::unique_ptr< QgsFeatureSink > sink( QgsProcessingUtils::createFeatureSink( dest, context, fields, geometryType, crs, createOptions ) );
std::unique_ptr< QgsFeatureSink > sink( QgsProcessingUtils::createFeatureSink( dest, context, fields, geometryType, crs, createOptions, sinkFlags ) );
destinationIdentifier = dest;

if ( destinationProject )
Expand Down Expand Up @@ -4580,4 +4581,3 @@ bool QgsProcessingParameterDistance::fromVariantMap( const QVariantMap &map )
mParentParameterName = map.value( QStringLiteral( "parent" ) ).toString();
return true;
}

0 comments on commit 7ac3d0e

Please sign in to comment.