Skip to content

Commit

Permalink
Merge pull request #10197 from rouault/fix_geojson_IUpdateFeature
Browse files Browse the repository at this point in the history 
ODS, XLSX, GeoJSON: implement IUpdateFeature() that was broken up to now
  • Loading branch information
@rouault
rouault committed Jun 13, 2024
2 parents 064ea59 + 48eca31 commit f555f47
Show file tree
Hide file tree
Showing 15 changed files with 505 additions and 60 deletions.
188 changes: 170 additions & 18 deletions apps/test_ogrsf.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2001,7 +2001,7 @@ static int TestOGRLayerRandomWrite(OGRLayer *poLayer)

{
int bRet = TRUE;
OGRFeature *papoFeatures[5], *poFeature;
OGRFeature *papoFeatures[5];

memset(papoFeatures, 0, sizeof(papoFeatures));

Expand Down Expand Up @@ -2086,25 +2086,27 @@ static int TestOGRLayerRandomWrite(OGRLayer *poLayer)
/* -------------------------------------------------------------------- */
/* Now re-read feature 2 to verify the effect stuck. */
/* -------------------------------------------------------------------- */
poFeature = LOG_ACTION(poLayer->GetFeature(nFID5));
if (poFeature == nullptr)
{
bRet = FALSE;
printf("ERROR: Attempt to GetFeature( nFID5 ) failed.\n");
goto end;
}
if (!poFeature->Equal(papoFeatures[1]))
{
bRet = FALSE;
poFeature->DumpReadable(stderr);
papoFeatures[1]->DumpReadable(stderr);
printf("ERROR: Written feature didn't seem to retain value.\n");
}
else if (bVerbose)
{
printf("INFO: Random write test passed.\n");
auto poFeature =
std::unique_ptr<OGRFeature>(LOG_ACTION(poLayer->GetFeature(nFID5)));
if (poFeature == nullptr)
{
bRet = FALSE;
printf("ERROR: Attempt to GetFeature( nFID5 ) failed.\n");
goto end;
}
if (!poFeature->Equal(papoFeatures[1]))
{
bRet = FALSE;
poFeature->DumpReadable(stderr);
papoFeatures[1]->DumpReadable(stderr);
printf("ERROR: Written feature didn't seem to retain value.\n");
}
else if (bVerbose)
{
printf("INFO: Random write test passed.\n");
}
}
DestroyFeatureAndNullify(poFeature);

/* -------------------------------------------------------------------- */
/* Re-invert the features to restore to original state */
Expand All @@ -2130,6 +2132,156 @@ static int TestOGRLayerRandomWrite(OGRLayer *poLayer)
printf("ERROR: Attempt to restore SetFeature(4) failed.\n");
}

/* -------------------------------------------------------------------- */
/* Test UpdateFeature() */
/* -------------------------------------------------------------------- */

if (bRet)
{
int nOldVal = 0;
std::string osOldVal;
int iUpdatedFeature = -1;
int iUpdatedField = -1;
std::unique_ptr<OGRFeature> poUpdatedFeature;

for (int iFeature = 0; iUpdatedFeature < 0 && iFeature < 5; iFeature++)
{
for (int iField = 0;
iField < poLayer->GetLayerDefn()->GetFieldCount(); ++iField)
{
if (papoFeatures[iFeature]->IsFieldSetAndNotNull(iField))
{
if (poLayer->GetLayerDefn()
->GetFieldDefn(iField)
->GetType() == OFTInteger)
{
iUpdatedFeature = iFeature;
iUpdatedField = iField;
nOldVal =
papoFeatures[iFeature]->GetFieldAsInteger(iField);
poUpdatedFeature.reset(papoFeatures[iFeature]->Clone());
poUpdatedFeature->SetField(iField, 0xBEEF);
break;
}
else if (poLayer->GetLayerDefn()
->GetFieldDefn(iField)
->GetType() == OFTString)
{
iUpdatedFeature = iFeature;
iUpdatedField = iField;
osOldVal =
papoFeatures[iFeature]->GetFieldAsString(iField);
poUpdatedFeature.reset(papoFeatures[iFeature]->Clone());
poUpdatedFeature->SetField(iField, "0xBEEF");
break;
}
}
}
}

if (poUpdatedFeature)
{
if (LOG_ACTION(poLayer->UpdateFeature(poUpdatedFeature.get(), 1,
&iUpdatedField, 0, nullptr,
false)) != OGRERR_NONE)
{
bRet = FALSE;
printf("ERROR: UpdateFeature() failed.\n");
}
if (bRet)
{
LOG_ACTION(poLayer->ResetReading());
for (int iFeature = 0; iFeature < 5; iFeature++)
{
auto poFeature = std::unique_ptr<OGRFeature>(LOG_ACTION(
poLayer->GetFeature(papoFeatures[iFeature]->GetFID())));
if (iFeature != iUpdatedFeature)
{
if (!poFeature ||
!poFeature->Equal(papoFeatures[iFeature]))
{
bRet = false;
printf("ERROR: UpdateFeature() test: "
"!poFeature->Equals(papoFeatures[iFeature]) "
"unexpected.\n");
if (poFeature)
poFeature->DumpReadable(stdout);
papoFeatures[iFeature]->DumpReadable(stdout);
}
}
}

auto poFeature =
std::unique_ptr<OGRFeature>(LOG_ACTION(poLayer->GetFeature(
papoFeatures[iUpdatedFeature]->GetFID())));
if (!poFeature)
{
bRet = FALSE;
printf("ERROR: at line %d", __LINE__);
goto end;
}
if (poLayer->GetLayerDefn()
->GetFieldDefn(iUpdatedField)
->GetType() == OFTInteger)
{
if (poFeature->GetFieldAsInteger(iUpdatedField) != 0xBEEF)
{
bRet = FALSE;
printf("ERROR: Did not get expected field value after "
"UpdateFeature().\n");
}
poFeature->SetField(iUpdatedField, nOldVal);
}
else if (poLayer->GetLayerDefn()
->GetFieldDefn(iUpdatedField)
->GetType() == OFTString)
{
if (!EQUAL(poFeature->GetFieldAsString(iUpdatedField),
"0xBEEF"))
{
bRet = FALSE;
printf("ERROR: Did not get expected field value after "
"UpdateFeature().\n");
}
poFeature->SetField(iUpdatedField, osOldVal.c_str());
}
else
{
CPLAssert(false);
}

if (LOG_ACTION(poLayer->UpdateFeature(
poFeature.get(), 1, &iUpdatedField, 0, nullptr,
false)) != OGRERR_NONE)
{
bRet = FALSE;
printf("ERROR: UpdateFeature() failed.\n");
}

poFeature.reset(LOG_ACTION(poLayer->GetFeature(
papoFeatures[iUpdatedFeature]->GetFID())));
if (!poFeature)
{
bRet = FALSE;
printf("ERROR: at line %d", __LINE__);
goto end;
}
if (!poFeature->Equal(papoFeatures[iUpdatedFeature]))
{
bRet = false;
printf("ERROR: UpdateFeature() test: "
"!poFeature->Equals(papoFeatures[iUpdatedFeature]) "
"unexpected.\n");
}
}
}
else
{
if (bVerbose)
printf("INFO: Could not test UpdateFeature().\n");
}
}

end:
/* -------------------------------------------------------------------- */
/* Cleanup. */
Expand Down
54 changes: 52 additions & 2 deletions autotest/ogr/ogr_geojson.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1591,7 +1591,7 @@ def test_ogr_geojson_46(tmp_vsimem):


###############################################################################
# Test update support
# Test SetFeature() support


@gdaltest.disable_exceptions()
Expand Down Expand Up @@ -1764,7 +1764,7 @@ def test_ogr_geojson_47(tmp_vsimem):


###############################################################################
# Test update support with file that has a single feature not in a FeatureCollection
# Test SetFeature() support with file that has a single feature not in a FeatureCollection


def test_ogr_geojson_48(tmp_vsimem):
Expand Down Expand Up @@ -1802,6 +1802,34 @@ def test_ogr_geojson_48(tmp_vsimem):
)


###############################################################################
# Test UpdateFeature() support


def test_ogr_geojson_update_feature(tmp_vsimem):

filename = str(tmp_vsimem / "test.json")

with ogr.GetDriverByName("GeoJSON").CreateDataSource(filename) as ds:
lyr = ds.CreateLayer("test")
lyr.CreateField(ogr.FieldDefn("int64list", ogr.OFTInteger64List))
f = ogr.Feature(lyr.GetLayerDefn())
f["int64list"] = [123456790123, -123456790123]
lyr.CreateFeature(f)

with ogr.Open(filename, update=1) as ds:
lyr = ds.GetLayer(0)
f = ogr.Feature(lyr.GetLayerDefn())
f.SetFID(0)
f["int64list"] = [123456790123, -123456790123]
lyr.UpdateFeature(f, [0], [], False)

with ogr.Open(filename) as ds:
lyr = ds.GetLayer(0)
f = lyr.GetNextFeature()
assert f["int64list"] == [123456790123, -123456790123]


###############################################################################
# Test ARRAY_AS_STRING

Expand Down Expand Up @@ -4280,6 +4308,28 @@ def test_ogr_geojson_test_ogrsf():
assert "ERROR" not in ret


###############################################################################
# Run test_ogrsf


def test_ogr_geojson_test_ogrsf_update(tmp_path):

import test_cli_utilities

if test_cli_utilities.get_test_ogrsf_path() is None:
pytest.skip()

filename = str(tmp_path / "out.json")
gdal.VectorTranslate(filename, "data/poly.shp", format="GeoJSON")

ret = gdaltest.runexternal(
test_cli_utilities.get_test_ogrsf_path() + f" {filename}"
)

assert "INFO" in ret
assert "ERROR" not in ret


###############################################################################
# Test fix for https://github.com/OSGeo/gdal/issues/7313

Expand Down
22 changes: 22 additions & 0 deletions autotest/ogr/ogr_ods.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,28 @@ def test_ogr_ods_4():
assert ret.find("INFO") != -1 and ret.find("ERROR") == -1


###############################################################################
# Run test_ogrsf


def test_ogr_ods_test_ogrsf_update(tmp_path):

import test_cli_utilities

if test_cli_utilities.get_test_ogrsf_path() is None:
pytest.skip()

filename = str(tmp_path / "out.ods")
gdal.VectorTranslate(filename, "data/poly.shp", format="ODS")

ret = gdaltest.runexternal(
test_cli_utilities.get_test_ogrsf_path() + f" {filename}"
)

assert "INFO" in ret
assert "ERROR" not in ret


###############################################################################
# Test write support

Expand Down
22 changes: 22 additions & 0 deletions autotest/ogr/ogr_xlsx.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,28 @@ def test_ogr_xlsx_4():
assert ret.find("INFO") != -1 and ret.find("ERROR") == -1


###############################################################################
# Run test_ogrsf


def test_ogr_xlsx_test_ogrsf_update(tmp_path):

import test_cli_utilities

if test_cli_utilities.get_test_ogrsf_path() is None:
pytest.skip()

filename = str(tmp_path / "out.xlsx")
gdal.VectorTranslate(filename, "data/poly.shp", format="XLSX")

ret = gdaltest.runexternal(
test_cli_utilities.get_test_ogrsf_path() + f" {filename}"
)

assert "INFO" in ret
assert "ERROR" not in ret


###############################################################################
# Test write support

Expand Down
6 changes: 6 additions & 0 deletions ogr/ogrsf_frmts/geojson/ogr_geojson.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,11 @@ class OGRGeoJSONLayer final : public OGRMemLayer

OGRErr ISetFeature(OGRFeature *poFeature) override;
OGRErr ICreateFeature(OGRFeature *poFeature) override;
OGRErr IUpdateFeature(OGRFeature *poFeature, int nUpdatedFieldsCount,
const int *panUpdatedFieldsIdx,
int nUpdatedGeomFieldsCount,
const int *panUpdatedGeomFieldsIdx,
bool bUpdateStyleString) override;
virtual OGRErr DeleteFeature(GIntBig nFID) override;
virtual OGRErr CreateField(const OGRFieldDefn *poField,
int bApproxOK = TRUE) override;
Expand Down Expand Up @@ -140,6 +145,7 @@ class OGRGeoJSONLayer final : public OGRMemLayer

bool IngestAll();
void TerminateAppendSession();
bool SetOrUpdateFeaturePreparation();

CPL_DISALLOW_COPY_ASSIGN(OGRGeoJSONLayer)
};
Expand Down
3 changes: 3 additions & 0 deletions ogr/ogrsf_frmts/geojson/ogrgeojsondriver.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -688,6 +688,9 @@ void RegisterOGRGeoJSON()
poDriver->SetMetadataItem(GDAL_DCAP_VECTOR, "YES");
poDriver->SetMetadataItem(GDAL_DCAP_CREATE_LAYER, "YES");
poDriver->SetMetadataItem(GDAL_DCAP_CREATE_FIELD, "YES");
poDriver->SetMetadataItem(GDAL_DCAP_DELETE_FIELD, "YES");
poDriver->SetMetadataItem(GDAL_DCAP_REORDER_FIELDS, "YES");
poDriver->SetMetadataItem(GDAL_DMD_ALTER_FIELD_DEFN_FLAGS, "Name Type");
poDriver->SetMetadataItem(GDAL_DCAP_Z_GEOMETRIES, "YES");
poDriver->SetMetadataItem(GDAL_DMD_LONGNAME, "GeoJSON");
poDriver->SetMetadataItem(GDAL_DMD_EXTENSIONS, "json geojson");
Expand Down
Loading

0 comments on commit f555f47

Please sign in to comment.