Skip to content
Permalink
Browse files

Merge pull request #7207 from rouault/geojson_add

[OGR provider] Fix adding a new GeoJSON field, starting a new edit session and filling it (fixes #7205)
  • Loading branch information
rouault committed Jun 8, 2018
2 parents b4cbfed + 82c2aaa commit 289032bd25a2b5baa7e9bf3511badeb78bebba86
Showing with 122 additions and 57 deletions.
  1. +85 −57 src/providers/ogr/qgsogrprovider.cpp
  2. +2 −0 src/providers/ogr/qgsogrprovider.h
  3. +35 −0 tests/src/python/test_provider_ogr.py
@@ -1548,6 +1548,73 @@ bool QgsOgrProvider::addFeatures( QgsFeatureList &flist, Flags flags )
return returnvalue;
}

bool QgsOgrProvider::addAttributeOGRLevel( const QgsField &field, bool &ignoreErrorOut )
{
ignoreErrorOut = false;

OGRFieldType type;

switch ( field.type() )
{
case QVariant::Int:
case QVariant::Bool:
type = OFTInteger;
break;
case QVariant::LongLong:
{
const char *pszDataTypes = GDALGetMetadataItem( mOgrLayer->driver(), GDAL_DMD_CREATIONFIELDDATATYPES, nullptr );
if ( pszDataTypes && strstr( pszDataTypes, "Integer64" ) )
type = OFTInteger64;
else
{
type = OFTReal;
}
break;
}
case QVariant::Double:
type = OFTReal;
break;
case QVariant::Date:
type = OFTDate;
break;
case QVariant::Time:
type = OFTTime;
break;
case QVariant::DateTime:
type = OFTDateTime;
break;
case QVariant::String:
type = OFTString;
break;
default:
pushError( tr( "type %1 for field %2 not found" ).arg( field.typeName(), field.name() ) );
ignoreErrorOut = true;
return false;
}

gdal::ogr_field_def_unique_ptr fielddefn( OGR_Fld_Create( textEncoding()->fromUnicode( field.name() ).constData(), type ) );
int width = field.length();
if ( field.precision() )
width += 1;
OGR_Fld_SetWidth( fielddefn.get(), width );
OGR_Fld_SetPrecision( fielddefn.get(), field.precision() );

switch ( field.type() )
{
case QVariant::Bool:
OGR_Fld_SetSubType( fielddefn.get(), OFSTBoolean );
default:
break;
}

if ( mOgrLayer->CreateField( fielddefn.get(), true ) != OGRERR_NONE )
{
pushError( tr( "OGR error creating field %1: %2" ).arg( field.name(), CPLGetLastErrorMsg() ) );
return false;
}
return true;
}

bool QgsOgrProvider::addAttributes( const QList<QgsField> &attributes )
{
if ( !doInitialActionsForEdition() )
@@ -1564,69 +1631,18 @@ bool QgsOgrProvider::addAttributes( const QList<QgsField> &attributes )

QMap< QString, QgsField > mapFieldNameToOriginalField;

for ( QList<QgsField>::const_iterator iter = attributes.constBegin(); iter != attributes.constEnd(); ++iter )
for ( const auto &field : attributes )
{
mapFieldNameToOriginalField[ iter->name()] = *iter;
mapFieldNameToOriginalField[ field.name()] = field;

OGRFieldType type;

switch ( iter->type() )
bool ignoreErrorOut = false;
if ( !addAttributeOGRLevel( field, ignoreErrorOut ) )
{
case QVariant::Int:
case QVariant::Bool:
type = OFTInteger;
break;
case QVariant::LongLong:
returnvalue = false;
if ( !ignoreErrorOut )
{
const char *pszDataTypes = GDALGetMetadataItem( mOgrLayer->driver(), GDAL_DMD_CREATIONFIELDDATATYPES, nullptr );
if ( pszDataTypes && strstr( pszDataTypes, "Integer64" ) )
type = OFTInteger64;
else
{
type = OFTReal;
}
break;
}
case QVariant::Double:
type = OFTReal;
break;
case QVariant::Date:
type = OFTDate;
break;
case QVariant::Time:
type = OFTTime;
break;
case QVariant::DateTime:
type = OFTDateTime;
break;
case QVariant::String:
type = OFTString;
break;
default:
pushError( tr( "type %1 for field %2 not found" ).arg( iter->typeName(), iter->name() ) );
returnvalue = false;
continue;
}

gdal::ogr_field_def_unique_ptr fielddefn( OGR_Fld_Create( textEncoding()->fromUnicode( iter->name() ).constData(), type ) );
int width = iter->length();
if ( iter->precision() )
width += 1;
OGR_Fld_SetWidth( fielddefn.get(), width );
OGR_Fld_SetPrecision( fielddefn.get(), iter->precision() );

switch ( iter->type() )
{
case QVariant::Bool:
OGR_Fld_SetSubType( fielddefn.get(), OFSTBoolean );
default:
break;
}

if ( mOgrLayer->CreateField( fielddefn.get(), true ) != OGRERR_NONE )
{
pushError( tr( "OGR error creating field %1: %2" ).arg( iter->name(), CPLGetLastErrorMsg() ) );
returnvalue = false;
}
}

@@ -4270,6 +4286,18 @@ bool QgsOgrProvider::leaveUpdateMode()
reloadData();
if ( mValid )
{
// Make sure that new fields we added, but didn't populate yet, are
// recreated at the OGR level, otherwise we won't be able to populate
// them.
for ( const auto &field : oldFields )
{
int idx = mAttributeFields.lookupField( field.name() );
if ( idx < 0 )
{
bool ignoreErrorOut = false;
addAttributeOGRLevel( field, ignoreErrorOut );
}
}
mAttributeFields = oldFields;
}
}
@@ -313,6 +313,8 @@ class QgsOgrProvider : public QgsVectorDataProvider

bool doInitialActionsForEdition();

bool addAttributeOGRLevel( const QgsField &field, bool &ignoreErrorOut );

#ifndef QT_NO_NETWORKPROXY
void setupProxy();
#endif
@@ -369,6 +369,41 @@ def testEditGeoJsonAddField(self):
# all in the GeoJSON file, so it has disappeared
self.assertEqual(len(vl.fields()), 1)

def testEditGeoJsonAddFieldAndThenAddFeatures(self):
""" Test bugfix of https://issues.qgis.org/issues/18596 (adding a new field)"""

datasource = os.path.join(self.basetestpath, 'testEditGeoJsonAddField.json')
with open(datasource, 'wt') as f:
f.write("""{
"type": "FeatureCollection",
"features": [
{ "type": "Feature", "properties": { "x": 1 }, "geometry": { "type": "Point", "coordinates": [ 0, 0 ] } } ] }""")

vl = QgsVectorLayer(datasource, 'test', 'ogr')
self.assertTrue(vl.isValid())
self.assertTrue(vl.startEditing())
self.assertTrue(vl.addAttribute(QgsField('strfield', QVariant.String)))
self.assertTrue(vl.commitChanges())
self.assertEqual(len(vl.dataProvider().fields()), 1 + 1)
self.assertEqual([f.name() for f in vl.dataProvider().fields()], ['x', 'strfield'])

f = QgsFeature()
self.assertTrue(vl.getFeatures(QgsFeatureRequest()).nextFeature(f))
self.assertIsNone(f['strfield'])
self.assertEqual([field.name() for field in f.fields()], ['x', 'strfield'])

self.assertTrue(vl.startEditing())
vl.changeAttributeValue(f.id(), 1, 'x')
self.assertTrue(vl.commitChanges())
f = QgsFeature()
self.assertTrue(vl.getFeatures(QgsFeatureRequest()).nextFeature(f))
self.assertEqual(f['strfield'], 'x')
self.assertEqual([field.name() for field in f.fields()], ['x', 'strfield'])

# Completely reload file
vl = QgsVectorLayer(datasource, 'test', 'ogr')
self.assertEqual(len(vl.fields()), 2)


if __name__ == '__main__':
unittest.main()

0 comments on commit 289032b

Please sign in to comment.
You can’t perform that action at this time.