From 518c2e222e037ef5cc8cd2874ef04dbd6320d4d0 Mon Sep 17 00:00:00 2001 From: Even Rouault Date: Wed, 18 Sep 2024 23:04:07 +0200 Subject: [PATCH] LIBKML: report XML id attribute as 'id' field; implement SetFeature() and DeleteFeature() for regular layers Fixes qgis/QGIS#58780 --- apps/test_ogrsf.cpp | 3 +- autotest/ogr/ogr_libkml.py | 90 ++++++- doc/source/drivers/vector/libkml.rst | 7 + ogr/ogrsf_frmts/libkml/fieldconfig.h | 88 +++++++ ogr/ogrsf_frmts/libkml/ogr_libkml.h | 21 +- ogr/ogrsf_frmts/libkml/ogrlibkmlfeature.cpp | 15 +- ogr/ogrsf_frmts/libkml/ogrlibkmlfeature.h | 4 +- ogr/ogrsf_frmts/libkml/ogrlibkmlfield.cpp | 38 +-- ogr/ogrsf_frmts/libkml/ogrlibkmlfield.h | 64 +---- ogr/ogrsf_frmts/libkml/ogrlibkmlgeometry.cpp | 6 +- ogr/ogrsf_frmts/libkml/ogrlibkmllayer.cpp | 254 ++++++++++++++----- 11 files changed, 433 insertions(+), 157 deletions(-) create mode 100644 ogr/ogrsf_frmts/libkml/fieldconfig.h diff --git a/apps/test_ogrsf.cpp b/apps/test_ogrsf.cpp index ddb13d4dc22b..5f5e4a6a7134 100644 --- a/apps/test_ogrsf.cpp +++ b/apps/test_ogrsf.cpp @@ -2041,7 +2041,8 @@ static int TestOGRLayerRandomWrite(OGRLayer *poLayer) CPLString os_Id2; CPLString os_Id5; - const bool bHas_Id = poLayer->GetLayerDefn()->GetFieldIndex("_id") == 0; + const bool bHas_Id = poLayer->GetLayerDefn()->GetFieldIndex("_id") == 0 || + poLayer->GetLayerDefn()->GetFieldIndex("id") == 0; /* -------------------------------------------------------------------- */ /* Fetch five features. */ diff --git a/autotest/ogr/ogr_libkml.py b/autotest/ogr/ogr_libkml.py index 777058abd284..faa2ca2be036 100755 --- a/autotest/ogr/ogr_libkml.py +++ b/autotest/ogr/ogr_libkml.py @@ -29,7 +29,6 @@ # DEALINGS IN THE SOFTWARE. ############################################################################### - import gdaltest import ogrtest import pytest @@ -289,7 +288,7 @@ def ogr_libkml_write(filename): lyr = ds.CreateLayer("test_wgs72", srs=srs) assert lyr.TestCapability(ogr.OLCSequentialWrite) == 1 - assert lyr.TestCapability(ogr.OLCRandomWrite) == 0 + assert lyr.TestCapability(ogr.OLCRandomWrite) == 1 dst_feat = ogr.Feature(lyr.GetLayerDefn()) dst_feat.SetGeometry(ogr.CreateGeometryFromWkt("POINT (2 49)")) @@ -530,6 +529,32 @@ def test_ogr_libkml_test_ogrsf(): ) +############################################################################### +# Run test_ogrsf + + +def test_ogr_libkml_test_ogrsf_write(tmp_path): + + test_filename = str(tmp_path / "test.kml") + gdal.VectorTranslate( + test_filename, "data/poly.shp", options="-s_srs EPSG:32631 -t_srs EPSG:4326" + ) + + import test_cli_utilities + + if test_cli_utilities.get_test_ogrsf_path() is None: + pytest.skip() + + ret = gdaltest.runexternal( + test_cli_utilities.get_test_ogrsf_path() + + f" --config OGR_SKIP KML {test_filename}" + ) + + assert "using driver `LIBKML'" in ret + assert "INFO" in ret + assert "ERROR" not in ret + + ############################################################################### # Test reading KML with only Placemark @@ -2253,3 +2278,64 @@ def test_ogr_libkml_write_geometries(input_wkt, expected_wkt, tmp_vsimem): assert f.GetGeometryRef().ExportToIsoWkt() == expected_wkt else: assert f is None + + +############################################################################### +# Test update of existing file + + +@pytest.mark.parametrize("custom_id", [False, True]) +def test_ogr_libkml_update_delete_existing_kml(tmp_vsimem, custom_id): + + filename = str(tmp_vsimem / "test.kml") + with ogr.GetDriverByName("LIBKML").CreateDataSource(filename) as ds: + lyr = ds.CreateLayer("test") + lyr.CreateField(ogr.FieldDefn("id")) + lyr.CreateField(ogr.FieldDefn("name")) + f = ogr.Feature(lyr.GetLayerDefn()) + if custom_id: + f["id"] = "feat1" + f["name"] = "name1" + f.SetGeometry(ogr.CreateGeometryFromWkt("POINT (1 2)")) + lyr.CreateFeature(f) + f = ogr.Feature(lyr.GetLayerDefn()) + if custom_id: + f["id"] = "feat2" + f["name"] = "name2" + f.SetGeometry(ogr.CreateGeometryFromWkt("POINT (3 4)")) + lyr.CreateFeature(f) + + with gdal.OpenEx(filename, gdal.OF_VECTOR | gdal.OF_UPDATE) as ds: + lyr = ds.GetLayer(0) + with pytest.raises(Exception, match="Non existing feature"): + lyr.DeleteFeature(0) + + with gdal.OpenEx(filename, gdal.OF_VECTOR | gdal.OF_UPDATE) as ds: + lyr = ds.GetLayer(0) + with pytest.raises(Exception, match="Non existing feature"): + lyr.DeleteFeature(3) + + with gdal.OpenEx(filename, gdal.OF_VECTOR | gdal.OF_UPDATE) as ds: + lyr = ds.GetLayer(0) + lyr.DeleteFeature(1) + assert lyr.GetFeatureCount() == 1 + lyr.ResetReading() + f = lyr.GetNextFeature() + assert f.GetFID() == 2 + if custom_id: + assert f["id"] == "feat2" + assert f["name"] == "name2" + f["name"] = "name2_updated" + lyr.SetFeature(f) + + with gdal.OpenEx(filename, gdal.OF_VECTOR | gdal.OF_UPDATE) as ds: + lyr = ds.GetLayer(0) + f = lyr.GetNextFeature() + if custom_id: + # FIDs are renumbered after feature update/deletion´if using + # custom KML ids + assert f.GetFID() == 1 + assert f["id"] == "feat2" + else: + assert f.GetFID() == 2 + assert f["name"] == "name2_updated" diff --git a/doc/source/drivers/vector/libkml.rst b/doc/source/drivers/vector/libkml.rst index ee7ac9114270..b1d9eb9e8d9c 100644 --- a/doc/source/drivers/vector/libkml.rst +++ b/doc/source/drivers/vector/libkml.rst @@ -587,6 +587,13 @@ For example, if you want a field called 'Cities' to map to the tag in KML, you can set a configuration option. Note these are independent of layer creation and dataset creation options' ``. +- .. config:: LIBKML_ID_FIELD + :default: id + :since: 3.10 + + Name of the string field that maps to the kml attribute + ` `__. + - .. config:: LIBKML_NAME_FIELD :default: name diff --git a/ogr/ogrsf_frmts/libkml/fieldconfig.h b/ogr/ogrsf_frmts/libkml/fieldconfig.h new file mode 100644 index 000000000000..ab641388e0d6 --- /dev/null +++ b/ogr/ogrsf_frmts/libkml/fieldconfig.h @@ -0,0 +1,88 @@ +/****************************************************************************** + * + * Project: KML Translator + * Purpose: Implements OGRLIBKMLDriver + * Author: Brian Case, rush at winkey dot org + * + ****************************************************************************** + * Copyright (c) 2010, Brian Case + * Copyright (c) 2014, Even Rouault + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included + * in all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS + * OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER + * DEALINGS IN THE SOFTWARE. + *****************************************************************************/ + +#ifndef OGRLIBKMLFIELDCONFIG_H_INCLUDED +#define OGRLIBKMLFIELDCONFIG_H_INCLUDED + +/******************************************************************************* + Function to fetch the field config options. +*******************************************************************************/ + +struct fieldconfig +{ + const char *idfield; + const char *namefield; + const char *descfield; + const char *tsfield; + const char *beginfield; + const char *endfield; + const char *altitudeModefield; + const char *tessellatefield; + const char *extrudefield; + const char *visibilityfield; + const char *drawOrderfield; + const char *iconfield; + const char *headingfield; + const char *tiltfield; + const char *rollfield; + const char *snippetfield; + const char *modelfield; + const char *scalexfield; + const char *scaleyfield; + const char *scalezfield; + const char *networklinkfield; + const char *networklink_refreshvisibility_field; + const char *networklink_flytoview_field; + const char *networklink_refreshMode_field; + const char *networklink_refreshInterval_field; + const char *networklink_viewRefreshMode_field; + const char *networklink_viewRefreshTime_field; + const char *networklink_viewBoundScale_field; + const char *networklink_viewFormat_field; + const char *networklink_httpQuery_field; + const char *camera_longitude_field; + const char *camera_latitude_field; + const char *camera_altitude_field; + const char *camera_altitudemode_field; + const char *photooverlayfield; + const char *leftfovfield; + const char *rightfovfield; + const char *bottomfovfield; + const char *topfovfield; + const char *nearfield; + const char *photooverlay_shape_field; + const char *imagepyramid_tilesize_field; + const char *imagepyramid_maxwidth_field; + const char *imagepyramid_maxheight_field; + const char *imagepyramid_gridorigin_field; +}; + +void get_fieldconfig(struct fieldconfig *oFC); + +#endif diff --git a/ogr/ogrsf_frmts/libkml/ogr_libkml.h b/ogr/ogrsf_frmts/libkml/ogr_libkml.h index c1c51a1b209f..4c3da1186d24 100644 --- a/ogr/ogrsf_frmts/libkml/ogr_libkml.h +++ b/ogr/ogrsf_frmts/libkml/ogr_libkml.h @@ -33,6 +33,7 @@ #include "ogrsf_frmts.h" #include "libkml_headers.h" +#include "fieldconfig.h" #include @@ -47,18 +48,20 @@ CPLString OGRLIBKMLGetSanitizedNCName(const char *pszName); class OGRLIBKMLLayer final : public OGRLayer, public OGRGetNextFeatureThroughRaw { - int bUpdate; + int bUpdate = false; - int nFeatures; - int iFeature; - long nFID; + int nFeatures = 0; + int iFeature = 0; + GIntBig nFID = 1; const char *m_pszName; const char *m_pszFileName; + std::string m_osSanitizedNCName; kmldom::ContainerPtr m_poKmlLayer; kmldom::ElementPtr m_poKmlLayerRoot; kmldom::UpdatePtr m_poKmlUpdate; + fieldconfig m_oFieldConfig; OGRLIBKMLDataSource *m_poOgrDS; OGRFeatureDefn *m_poOgrFeatureDefn; kmldom::SchemaPtr m_poKmlSchema; @@ -84,6 +87,11 @@ class OGRLIBKMLLayer final : public OGRLayer, bool m_bUpdateIsFolder; + bool m_bAllReadAtLeastOnce = false; + std::map m_oMapOGRIdToKmlId{}; + std::map m_oMapKmlIdToOGRId{}; + + void ScanAllFeatures(); OGRFeature *GetNextRawFeature(); public: @@ -161,6 +169,11 @@ class OGRLIBKMLLayer final : public OGRLayer, return m_pszFileName; } + const fieldconfig &GetFieldConfig() const + { + return m_oFieldConfig; + } + void SetLookAt(const char *pszLookatLongitude, const char *pszLookatLatitude, const char *pszLookatAltitude, const char *pszLookatHeading, const char *pszLookatTilt, diff --git a/ogr/ogrsf_frmts/libkml/ogrlibkmlfeature.cpp b/ogr/ogrsf_frmts/libkml/ogrlibkmlfeature.cpp index 17c0ff7c96e5..ad1c6a6d20a6 100644 --- a/ogr/ogrsf_frmts/libkml/ogrlibkmlfeature.cpp +++ b/ogr/ogrsf_frmts/libkml/ogrlibkmlfeature.cpp @@ -225,9 +225,7 @@ FeaturePtr feat2kml(OGRLIBKMLDataSource *poOgrDS, OGRLIBKMLLayer *poOgrLayer, int bUseSimpleField) { FeaturePtr poKmlFeature = nullptr; - - struct fieldconfig oFC; - get_fieldconfig(&oFC); + const auto &oFC = poOgrLayer->GetFieldConfig(); /***** geometry *****/ OGRGeometry *poOgrGeom = poOgrFeat->GetGeometryRef(); @@ -833,13 +831,13 @@ FeaturePtr feat2kml(OGRLIBKMLDataSource *poOgrDS, OGRLIBKMLLayer *poOgrLayer, /***** fields *****/ field2kml(poOgrFeat, poOgrLayer, poKmlFactory, poKmlFeature, - bUseSimpleField); + bUseSimpleField, oFC); return poKmlFeature; } OGRFeature *kml2feat(PlacemarkPtr poKmlPlacemark, OGRLIBKMLDataSource *poOgrDS, - OGRLayer *poOgrLayer, OGRFeatureDefn *poOgrFeatDefn, + OGRLIBKMLLayer *poOgrLayer, OGRFeatureDefn *poOgrFeatDefn, OGRSpatialReference *poOgrSRS) { OGRFeature *poOgrFeat = new OGRFeature(poOgrFeatDefn); @@ -872,14 +870,15 @@ OGRFeature *kml2feat(PlacemarkPtr poKmlPlacemark, OGRLIBKMLDataSource *poOgrDS, } /***** fields *****/ - kml2field(poOgrFeat, AsFeature(poKmlPlacemark)); + kml2field(poOgrFeat, AsFeature(poKmlPlacemark), + poOgrLayer->GetFieldConfig()); return poOgrFeat; } OGRFeature *kmlgroundoverlay2feat(GroundOverlayPtr poKmlOverlay, OGRLIBKMLDataSource * /* poOgrDS */, - OGRLayer * /* poOgrLayer */, + OGRLIBKMLLayer *poOgrLayer, OGRFeatureDefn *poOgrFeatDefn, OGRSpatialReference *poOgrSRS) { @@ -900,7 +899,7 @@ OGRFeature *kmlgroundoverlay2feat(GroundOverlayPtr poKmlOverlay, } /***** fields *****/ - kml2field(poOgrFeat, AsFeature(poKmlOverlay)); + kml2field(poOgrFeat, AsFeature(poKmlOverlay), poOgrLayer->GetFieldConfig()); return poOgrFeat; } diff --git a/ogr/ogrsf_frmts/libkml/ogrlibkmlfeature.h b/ogr/ogrsf_frmts/libkml/ogrlibkmlfeature.h index d470c00746fb..a0a9ba0d6a22 100644 --- a/ogr/ogrsf_frmts/libkml/ogrlibkmlfeature.h +++ b/ogr/ogrsf_frmts/libkml/ogrlibkmlfeature.h @@ -45,13 +45,13 @@ kmldom::FeaturePtr feat2kml(OGRLIBKMLDataSource *poOgrDS, ******************************************************************************/ OGRFeature *kml2feat(kmldom::PlacemarkPtr poKmlPlacemark, - OGRLIBKMLDataSource *poOgrDS, OGRLayer *poOgrLayer, + OGRLIBKMLDataSource *poOgrDS, OGRLIBKMLLayer *poOgrLayer, OGRFeatureDefn *poOgrFeatDefn, OGRSpatialReference *poOgrSRS); OGRFeature *kmlgroundoverlay2feat(kmldom::GroundOverlayPtr poKmlOverlay, OGRLIBKMLDataSource *poOgrDS, - OGRLayer *poOgrLayer, + OGRLIBKMLLayer *poOgrLayer, OGRFeatureDefn *poOgrFeatDefn, OGRSpatialReference *poOgrSRS); diff --git a/ogr/ogrsf_frmts/libkml/ogrlibkmlfield.cpp b/ogr/ogrsf_frmts/libkml/ogrlibkmlfield.cpp index ad0161f1393e..a525524a616d 100644 --- a/ogr/ogrsf_frmts/libkml/ogrlibkmlfield.cpp +++ b/ogr/ogrsf_frmts/libkml/ogrlibkmlfield.cpp @@ -273,7 +273,7 @@ static char *OGRLIBKMLSanitizeUTF8String(const char *pszString) void field2kml(OGRFeature *poOgrFeat, OGRLIBKMLLayer *poOgrLayer, KmlFactory *poKmlFactory, FeaturePtr poKmlFeature, - int bUseSimpleFieldIn) + int bUseSimpleFieldIn, const fieldconfig &oFC) { const bool bUseSimpleField = CPL_TO_BOOL(bUseSimpleFieldIn); SchemaDataPtr poKmlSchemaData = nullptr; @@ -293,10 +293,6 @@ void field2kml(OGRFeature *poOgrFeat, OGRLIBKMLLayer *poOgrLayer, } } - /***** Get the field config *****/ - struct fieldconfig oFC; - get_fieldconfig(&oFC); - TimeSpanPtr poKmlTimeSpan = nullptr; const int nFields = poOgrFeat->GetFieldCount(); @@ -338,6 +334,13 @@ void field2kml(OGRFeature *poOgrFeat, OGRLIBKMLLayer *poOgrLayer, continue; } + /***** id *****/ + if (EQUAL(name, oFC.idfield)) + { + poKmlFeature->set_id(pszUTF8String); + CPLFree(pszUTF8String); + continue; + } /***** name *****/ if (EQUAL(name, oFC.namefield)) { @@ -1157,13 +1160,19 @@ static void kmldatetime2ogr(OGRFeature *poOgrFeat, const char *pszOGRField, function to read kml into ogr fields ******************************************************************************/ -void kml2field(OGRFeature *poOgrFeat, FeaturePtr poKmlFeature) +void kml2field(OGRFeature *poOgrFeat, FeaturePtr poKmlFeature, + const fieldconfig &oFC) { - /***** get the field config *****/ + /***** id *****/ - struct fieldconfig oFC; - get_fieldconfig(&oFC); + if (poKmlFeature->has_id()) + { + const std::string oKmlId = poKmlFeature->get_id(); + int iField = poOgrFeat->GetFieldIndex(oFC.idfield); + if (iField > -1) + poOgrFeat->SetField(iField, oKmlId.c_str()); + } /***** name *****/ if (poKmlFeature->has_name()) @@ -1552,15 +1561,13 @@ void kml2field(OGRFeature *poOgrFeat, FeaturePtr poKmlFeature) ******************************************************************************/ SimpleFieldPtr FieldDef2kml(const OGRFieldDefn *poOgrFieldDef, - KmlFactory *poKmlFactory, bool bApproxOK) + KmlFactory *poKmlFactory, bool bApproxOK, + const fieldconfig &oFC) { - /***** Get the field config. *****/ - struct fieldconfig oFC; - get_fieldconfig(&oFC); - const char *pszFieldName = poOgrFieldDef->GetNameRef(); - if (EQUAL(pszFieldName, oFC.namefield) || + if (EQUAL(pszFieldName, oFC.idfield) || + EQUAL(pszFieldName, oFC.namefield) || EQUAL(pszFieldName, oFC.descfield) || EQUAL(pszFieldName, oFC.tsfield) || EQUAL(pszFieldName, oFC.beginfield) || @@ -1724,6 +1731,7 @@ void kml2FeatureDef(SchemaPtr poKmlSchema, OGRFeatureDefn *poOgrFeatureDefn) void get_fieldconfig(struct fieldconfig *oFC) { + oFC->idfield = CPLGetConfigOption("LIBKML_ID_FIELD", "id"); oFC->namefield = CPLGetConfigOption("LIBKML_NAME_FIELD", "Name"); oFC->descfield = CPLGetConfigOption("LIBKML_DESCRIPTION_FIELD", "description"); diff --git a/ogr/ogrsf_frmts/libkml/ogrlibkmlfield.h b/ogr/ogrsf_frmts/libkml/ogrlibkmlfield.h index 2fa81e36fa77..010337c6d9e8 100644 --- a/ogr/ogrsf_frmts/libkml/ogrlibkmlfield.h +++ b/ogr/ogrsf_frmts/libkml/ogrlibkmlfield.h @@ -30,6 +30,8 @@ #ifndef OGRLIBKMLFIELD_H_INCLUDED #define OGRLIBKMLFIELD_H_INCLUDED +#include "fieldconfig.h" + /****************************************************************************** Function to output ogr fields in kml. @@ -55,13 +57,15 @@ void field2kml(OGRFeature *poOgrFeat, OGRLIBKMLLayer *poOgrLayer, kmldom::KmlFactory *poKmlFactory, - kmldom::FeaturePtr poKmlPlacemark, int bUseSimpleField); + kmldom::FeaturePtr poKmlPlacemark, int bUseSimpleField, + const fieldconfig &oFC); /****************************************************************************** Function to read kml into ogr fields. ******************************************************************************/ -void kml2field(OGRFeature *poOgrFeat, kmldom::FeaturePtr poKmlFeature); +void kml2field(OGRFeature *poOgrFeat, kmldom::FeaturePtr poKmlFeature, + const fieldconfig &oFC); /****************************************************************************** Function create a simplefield from a FieldDefn. @@ -69,7 +73,7 @@ void kml2field(OGRFeature *poOgrFeat, kmldom::FeaturePtr poKmlFeature); kmldom::SimpleFieldPtr FieldDef2kml(const OGRFieldDefn *poOgrFieldDef, kmldom::KmlFactory *poKmlFactory, - bool bApproxOK); + bool bApproxOK, const fieldconfig &oFC); /****************************************************************************** Function to add the simpleFields in a schema to a featuredefn. @@ -78,60 +82,6 @@ kmldom::SimpleFieldPtr FieldDef2kml(const OGRFieldDefn *poOgrFieldDef, void kml2FeatureDef(kmldom::SchemaPtr poKmlSchema, OGRFeatureDefn *poOgrFeatureDefn); -/******************************************************************************* - Function to fetch the field config options. -*******************************************************************************/ - -struct fieldconfig -{ - const char *namefield; - const char *descfield; - const char *tsfield; - const char *beginfield; - const char *endfield; - const char *altitudeModefield; - const char *tessellatefield; - const char *extrudefield; - const char *visibilityfield; - const char *drawOrderfield; - const char *iconfield; - const char *headingfield; - const char *tiltfield; - const char *rollfield; - const char *snippetfield; - const char *modelfield; - const char *scalexfield; - const char *scaleyfield; - const char *scalezfield; - const char *networklinkfield; - const char *networklink_refreshvisibility_field; - const char *networklink_flytoview_field; - const char *networklink_refreshMode_field; - const char *networklink_refreshInterval_field; - const char *networklink_viewRefreshMode_field; - const char *networklink_viewRefreshTime_field; - const char *networklink_viewBoundScale_field; - const char *networklink_viewFormat_field; - const char *networklink_httpQuery_field; - const char *camera_longitude_field; - const char *camera_latitude_field; - const char *camera_altitude_field; - const char *camera_altitudemode_field; - const char *photooverlayfield; - const char *leftfovfield; - const char *rightfovfield; - const char *bottomfovfield; - const char *topfovfield; - const char *nearfield; - const char *photooverlay_shape_field; - const char *imagepyramid_tilesize_field; - const char *imagepyramid_maxwidth_field; - const char *imagepyramid_maxheight_field; - const char *imagepyramid_gridorigin_field; -}; - -void get_fieldconfig(struct fieldconfig *oFC); - int kmlAltitudeModeFromString(const char *pszAltitudeMode, int &isGX); #endif /* OGRLIBKMLFIELD_H_INCLUDED */ diff --git a/ogr/ogrsf_frmts/libkml/ogrlibkmlgeometry.cpp b/ogr/ogrsf_frmts/libkml/ogrlibkmlgeometry.cpp index 7e083ea0dadc..ee55619eb41c 100644 --- a/ogr/ogrsf_frmts/libkml/ogrlibkmlgeometry.cpp +++ b/ogr/ogrsf_frmts/libkml/ogrlibkmlgeometry.cpp @@ -347,8 +347,7 @@ ElementPtr geom2kml(OGRGeometry *poOgrGeom, int extra, KmlFactory *poKmlFactory) bool bError; { CPLErrorStateBackuper oErrorStateBackuper; - bError = !poOgrGeom->IsValid() || - CPLGetLastErrorType() != CE_None; + bError = !poOgrGeom->IsValid(); } if (bError) { @@ -390,8 +389,7 @@ ElementPtr geom2kml(OGRGeometry *poOgrGeom, int extra, KmlFactory *poKmlFactory) bool bError; { CPLErrorStateBackuper oErrorStateBackuper; - bError = !poOgrGeom->IsValid() || - CPLGetLastErrorType() != CE_None; + bError = !poOgrGeom->IsValid(); } if (bError) { diff --git a/ogr/ogrsf_frmts/libkml/ogrlibkmllayer.cpp b/ogr/ogrsf_frmts/libkml/ogrlibkmllayer.cpp index 0e455726e11b..1d8141f0ac42 100644 --- a/ogr/ogrsf_frmts/libkml/ogrlibkmllayer.cpp +++ b/ogr/ogrsf_frmts/libkml/ogrlibkmllayer.cpp @@ -122,8 +122,8 @@ OGRLIBKMLLayer::OGRLIBKMLLayer( const OGRSpatialReference *poSRSIn, OGRLIBKMLDataSource *poOgrDS, ElementPtr poKmlRoot, ContainerPtr poKmlContainer, UpdatePtr poKmlUpdate, const char *pszFileName, int bNew, int bUpdateIn) - : bUpdate(CPL_TO_BOOL(bUpdateIn)), nFeatures(0), iFeature(0), nFID(1), - m_pszName(CPLStrdup(pszLayerName)), m_pszFileName(CPLStrdup(pszFileName)), + : bUpdate(CPL_TO_BOOL(bUpdateIn)), m_pszName(CPLStrdup(pszLayerName)), + m_pszFileName(CPLStrdup(pszFileName)), m_poKmlLayer(std::move(poKmlContainer)), // Store the layers container. m_poKmlLayerRoot( std::move(poKmlRoot)), // Store the root element pointer. @@ -140,6 +140,8 @@ OGRLIBKMLLayer::OGRLIBKMLLayer( m_dfRegionMinX(200), m_dfRegionMinY(200), m_dfRegionMaxX(-200), m_dfRegionMaxY(-200), m_bUpdateIsFolder(false) { + get_fieldconfig(&m_oFieldConfig); + m_poStyleTable = nullptr; m_poOgrSRS->SetWellKnownGeogCS("WGS84"); @@ -176,6 +178,9 @@ OGRLIBKMLLayer::OGRLIBKMLLayer( } } + m_osSanitizedNCName = + OGRLIBKMLGetSanitizedNCName(m_poOgrFeatureDefn->GetName()); + SetDescription(m_poOgrFeatureDefn->GetName()); m_poOgrFeatureDefn->Reference(); m_poOgrFeatureDefn->SetGeomType(eGType); @@ -188,52 +193,56 @@ OGRLIBKMLLayer::OGRLIBKMLLayer( /***** get the number of features on the layer *****/ nFeatures = static_cast(m_poKmlLayer->get_feature_array_size()); - /***** get the field config *****/ - struct fieldconfig oFC; - get_fieldconfig(&oFC); + /***** id field *****/ + OGRFieldDefn oOgrFieldId(m_oFieldConfig.idfield, OFTString); + m_poOgrFeatureDefn->AddFieldDefn(&oOgrFieldId); /***** name field *****/ - OGRFieldDefn oOgrFieldName(oFC.namefield, OFTString); + OGRFieldDefn oOgrFieldName(m_oFieldConfig.namefield, OFTString); m_poOgrFeatureDefn->AddFieldDefn(&oOgrFieldName); /***** description field *****/ - OGRFieldDefn oOgrFieldDesc(oFC.descfield, OFTString); + OGRFieldDefn oOgrFieldDesc(m_oFieldConfig.descfield, OFTString); m_poOgrFeatureDefn->AddFieldDefn(&oOgrFieldDesc); /***** timestamp field *****/ - OGRFieldDefn oOgrFieldTs(oFC.tsfield, OFTDateTime); + OGRFieldDefn oOgrFieldTs(m_oFieldConfig.tsfield, OFTDateTime); m_poOgrFeatureDefn->AddFieldDefn(&oOgrFieldTs); /***** timespan begin field *****/ - OGRFieldDefn oOgrFieldBegin(oFC.beginfield, OFTDateTime); + OGRFieldDefn oOgrFieldBegin(m_oFieldConfig.beginfield, OFTDateTime); m_poOgrFeatureDefn->AddFieldDefn(&oOgrFieldBegin); /***** timespan end field *****/ - OGRFieldDefn oOgrFieldEnd(oFC.endfield, OFTDateTime); + OGRFieldDefn oOgrFieldEnd(m_oFieldConfig.endfield, OFTDateTime); m_poOgrFeatureDefn->AddFieldDefn(&oOgrFieldEnd); /***** altitudeMode field *****/ - OGRFieldDefn oOgrFieldAltitudeMode(oFC.altitudeModefield, OFTString); + OGRFieldDefn oOgrFieldAltitudeMode(m_oFieldConfig.altitudeModefield, + OFTString); m_poOgrFeatureDefn->AddFieldDefn(&oOgrFieldAltitudeMode); /***** tessellate field *****/ - OGRFieldDefn oOgrFieldTessellate(oFC.tessellatefield, OFTInteger); + OGRFieldDefn oOgrFieldTessellate(m_oFieldConfig.tessellatefield, + OFTInteger); m_poOgrFeatureDefn->AddFieldDefn(&oOgrFieldTessellate); /***** extrude field *****/ - OGRFieldDefn oOgrFieldExtrude(oFC.extrudefield, OFTInteger); + OGRFieldDefn oOgrFieldExtrude(m_oFieldConfig.extrudefield, OFTInteger); m_poOgrFeatureDefn->AddFieldDefn(&oOgrFieldExtrude); /***** visibility field *****/ - OGRFieldDefn oOgrFieldVisibility(oFC.visibilityfield, OFTInteger); + OGRFieldDefn oOgrFieldVisibility(m_oFieldConfig.visibilityfield, + OFTInteger); m_poOgrFeatureDefn->AddFieldDefn(&oOgrFieldVisibility); /***** draw order field *****/ - OGRFieldDefn oOgrFieldDrawOrder(oFC.drawOrderfield, OFTInteger); + OGRFieldDefn oOgrFieldDrawOrder(m_oFieldConfig.drawOrderfield, + OFTInteger); m_poOgrFeatureDefn->AddFieldDefn(&oOgrFieldDrawOrder); /***** icon field *****/ - OGRFieldDefn oOgrFieldIcon(oFC.iconfield, OFTString); + OGRFieldDefn oOgrFieldIcon(m_oFieldConfig.iconfield, OFTString); m_poOgrFeatureDefn->AddFieldDefn(&oOgrFieldIcon); /***** get the styles *****/ @@ -298,19 +307,22 @@ OGRLIBKMLLayer::OGRLIBKMLLayer( if (camera->has_heading() && !bHasHeading) { bHasHeading = true; - OGRFieldDefn oOgrField(oFC.headingfield, OFTReal); + OGRFieldDefn oOgrField(m_oFieldConfig.headingfield, + OFTReal); m_poOgrFeatureDefn->AddFieldDefn(&oOgrField); } if (camera->has_tilt() && !bHasTilt) { bHasTilt = true; - OGRFieldDefn oOgrField(oFC.tiltfield, OFTReal); + OGRFieldDefn oOgrField(m_oFieldConfig.tiltfield, + OFTReal); m_poOgrFeatureDefn->AddFieldDefn(&oOgrField); } if (camera->has_roll() && !bHasRoll) { bHasRoll = true; - OGRFieldDefn oOgrField(oFC.rollfield, OFTReal); + OGRFieldDefn oOgrField(m_oFieldConfig.rollfield, + OFTReal); m_poOgrFeatureDefn->AddFieldDefn(&oOgrField); } } @@ -384,7 +396,8 @@ OGRLIBKMLLayer::OGRLIBKMLLayer( if (!bHasSnippet && poKmlFeature->has_snippet()) { bHasSnippet = true; - OGRFieldDefn oOgrField(oFC.snippetfield, OFTString); + OGRFieldDefn oOgrField(m_oFieldConfig.snippetfield, + OFTString); m_poOgrFeatureDefn->AddFieldDefn(&oOgrField); } } @@ -430,14 +443,20 @@ OGRFeature *OGRLIBKMLLayer::GetNextRawFeature() /***** loop over the kml features to find the next placemark *****/ + std::string id; do { if (iFeature >= nFeatures) + { + m_bAllReadAtLeastOnce = true; break; + } /***** get the next kml feature in the container *****/ const FeaturePtr poKmlFeature = m_poKmlLayer->get_feature_array_at(iFeature++); + if (poKmlFeature->has_id()) + id = poKmlFeature->get_id(); /***** what type of kml feature in the container? *****/ switch (poKmlFeature->Type()) @@ -463,11 +482,61 @@ OGRFeature *OGRLIBKMLLayer::GetNextRawFeature() /***** set the FID on the ogr feature *****/ if (poOgrFeature) - poOgrFeature->SetFID(nFID++); + { + // If KML id is of the form "layername.number", use number as the FID + if (!id.empty() && id.size() > m_osSanitizedNCName.size() && + id[m_osSanitizedNCName.size()] == '.' && + STARTS_WITH(id.c_str(), m_osSanitizedNCName.c_str())) + { + auto iFID = + CPLAtoGIntBig(id.c_str() + m_osSanitizedNCName.size() + 1); + if (iFID > 0) + { + poOgrFeature->SetFID(iFID); + nFID = std::max(iFID + 1, nFID); + } + } + if (poOgrFeature->GetFID() < 0) + poOgrFeature->SetFID(nFID++); + + if (bUpdate && !id.empty()) + { + auto oIter = m_oMapKmlIdToOGRId.find(id); + if (oIter != m_oMapKmlIdToOGRId.end()) + { + poOgrFeature->SetFID(oIter->second); + } + else + { + m_oMapOGRIdToKmlId[poOgrFeature->GetFID()] = id; + m_oMapKmlIdToOGRId[id] = poOgrFeature->GetFID(); + } + } + } return poOgrFeature; } +/******************************************************************************/ +/* ScanAllFeatures() */ +/******************************************************************************/ + +void OGRLIBKMLLayer::ScanAllFeatures() +{ + if (!m_bAllReadAtLeastOnce) + { + const auto iFeatureBackup = iFeature; + const auto nFIDBackup = nFID; + while (iFeature < nFeatures && + std::unique_ptr(GetNextRawFeature())) + { + // do nothing + } + iFeature = iFeatureBackup; + nFID = nFIDBackup; + } +} + /****************************************************************************** Method to add a feature to a layer. @@ -483,6 +552,22 @@ OGRErr OGRLIBKMLLayer::ICreateFeature(OGRFeature *poOgrFeat) if (!bUpdate) return OGRERR_UNSUPPORTED_OPERATION; + const int idxIdField = + m_poOgrFeatureDefn->GetFieldIndex(m_oFieldConfig.idfield); + if (idxIdField >= 0 && poOgrFeat->IsFieldSet(idxIdField)) + { + ScanAllFeatures(); + + if (cpl::contains(m_oMapKmlIdToOGRId, + poOgrFeat->GetFieldAsString(idxIdField))) + { + CPLError(CE_Failure, CPLE_AppDefined, + "A feature with id %s already exists", + poOgrFeat->GetFieldAsString(idxIdField)); + return OGRERR_FAILURE; + } + } + OGRGeometry *poGeomBackup = nullptr; if (nullptr != m_poCT) { @@ -529,7 +614,7 @@ OGRErr OGRLIBKMLLayer::ICreateFeature(OGRFeature *poOgrFeat) poContainer = poKmlFactory->CreateFolder(); else poContainer = poKmlFactory->CreateDocument(); - poContainer->set_targetid(OGRLIBKMLGetSanitizedNCName(GetName())); + poContainer->set_targetid(m_osSanitizedNCName); poContainer->add_feature(poKmlFeature); poCreate->add_container(poContainer); m_poKmlUpdate->add_updateoperation(poCreate); @@ -540,10 +625,19 @@ OGRErr OGRLIBKMLLayer::ICreateFeature(OGRFeature *poOgrFeat) { nFeatures++; - const char *pszId = CPLSPrintf( - "%s.%d", OGRLIBKMLGetSanitizedNCName(GetName()).c_str(), nFeatures); - poOgrFeat->SetFID(nFeatures); - poKmlFeature->set_id(pszId); + if (poOgrFeat->GetFID() < 0) + { + poOgrFeat->SetFID(nFeatures); + } + if (!poKmlFeature->has_id()) + { + const char *pszId = + CPLSPrintf("%s." CPL_FRMT_GIB, m_osSanitizedNCName.c_str(), + poOgrFeat->GetFID()); + poKmlFeature->set_id(pszId); + } + m_oMapOGRIdToKmlId[poOgrFeat->GetFID()] = poKmlFeature->get_id(); + m_oMapKmlIdToOGRId[poKmlFeature->get_id()] = poOgrFeat->GetFID(); } else { @@ -560,12 +654,13 @@ OGRErr OGRLIBKMLLayer::ICreateFeature(OGRFeature *poOgrFeat) } else { - const char *pszId = - CPLSPrintf("%s." CPL_FRMT_GIB, - OGRLIBKMLGetSanitizedNCName(GetName()).c_str(), - poOgrFeat->GetFID()); - poOgrFeat->SetFID(nFeatures); - poKmlFeature->set_id(pszId); + if (!poKmlFeature->has_id()) + { + const char *pszId = + CPLSPrintf("%s." CPL_FRMT_GIB, m_osSanitizedNCName.c_str(), + poOgrFeat->GetFID()); + poKmlFeature->set_id(pszId); + } } } @@ -589,24 +684,33 @@ OGRErr OGRLIBKMLLayer::ICreateFeature(OGRFeature *poOgrFeat) OGRErr OGRLIBKMLLayer::ISetFeature(OGRFeature *poOgrFeat) { - if (!bUpdate || !m_poKmlUpdate) + if (!bUpdate) return OGRERR_UNSUPPORTED_OPERATION; if (poOgrFeat->GetFID() == OGRNullFID) - return OGRERR_FAILURE; - - FeaturePtr poKmlFeature = - feat2kml(m_poOgrDS, this, poOgrFeat, m_poOgrDS->GetKmlFactory(), - m_bUseSimpleField); + return OGRERR_NON_EXISTING_FEATURE; - const KmlFactory *poKmlFactory = m_poOgrDS->GetKmlFactory(); - const ChangePtr poChange = poKmlFactory->CreateChange(); - poChange->add_object(poKmlFeature); - m_poKmlUpdate->add_updateoperation(poChange); - - const char *pszId = CPLSPrintf( - "%s." CPL_FRMT_GIB, OGRLIBKMLGetSanitizedNCName(GetName()).c_str(), - poOgrFeat->GetFID()); - poKmlFeature->set_targetid(pszId); + if (m_poKmlUpdate) + { + FeaturePtr poKmlFeature = + feat2kml(m_poOgrDS, this, poOgrFeat, m_poOgrDS->GetKmlFactory(), + m_bUseSimpleField); + + const KmlFactory *poKmlFactory = m_poOgrDS->GetKmlFactory(); + const ChangePtr poChange = poKmlFactory->CreateChange(); + poChange->add_object(poKmlFeature); + m_poKmlUpdate->add_updateoperation(poChange); + + const char *pszId = + CPLSPrintf("%s." CPL_FRMT_GIB, m_osSanitizedNCName.c_str(), + poOgrFeat->GetFID()); + poKmlFeature->set_targetid(pszId); + } + else if (m_poKmlLayer) + { + if (DeleteFeature(poOgrFeat->GetFID()) != OGRERR_NONE) + return OGRERR_NON_EXISTING_FEATURE; + return ICreateFeature(poOgrFeat); + } /***** mark as updated *****/ m_poOgrDS->Updated(); @@ -628,19 +732,41 @@ OGRErr OGRLIBKMLLayer::ISetFeature(OGRFeature *poOgrFeat) OGRErr OGRLIBKMLLayer::DeleteFeature(GIntBig nFIDIn) { - if (!bUpdate || !m_poKmlUpdate) + if (!bUpdate) return OGRERR_UNSUPPORTED_OPERATION; - KmlFactory *poKmlFactory = m_poOgrDS->GetKmlFactory(); - DeletePtr poDelete = poKmlFactory->CreateDelete(); - m_poKmlUpdate->add_updateoperation(poDelete); - PlacemarkPtr poKmlPlacemark = poKmlFactory->CreatePlacemark(); - poDelete->add_feature(poKmlPlacemark); + if (m_poKmlUpdate) + { + KmlFactory *poKmlFactory = m_poOgrDS->GetKmlFactory(); + DeletePtr poDelete = poKmlFactory->CreateDelete(); + m_poKmlUpdate->add_updateoperation(poDelete); + PlacemarkPtr poKmlPlacemark = poKmlFactory->CreatePlacemark(); + poDelete->add_feature(poKmlPlacemark); + + const char *pszId = + CPLSPrintf("%s." CPL_FRMT_GIB, m_osSanitizedNCName.c_str(), nFIDIn); + poKmlPlacemark->set_targetid(pszId); + } + else if (m_poKmlLayer) + { + auto oIter = m_oMapOGRIdToKmlId.find(nFIDIn); + if (oIter == m_oMapOGRIdToKmlId.end()) + { + ScanAllFeatures(); - const char *pszId = - CPLSPrintf("%s." CPL_FRMT_GIB, - OGRLIBKMLGetSanitizedNCName(GetName()).c_str(), nFIDIn); - poKmlPlacemark->set_targetid(pszId); + oIter = m_oMapOGRIdToKmlId.find(nFIDIn); + if (oIter == m_oMapOGRIdToKmlId.end()) + return OGRERR_NON_EXISTING_FEATURE; + } + const auto &osKmlId = oIter->second; + if (!m_poKmlLayer->DeleteFeatureById(osKmlId)) + { + return OGRERR_NON_EXISTING_FEATURE; + } + nFeatures = static_cast(m_poKmlLayer->get_feature_array_size()); + m_oMapKmlIdToOGRId.erase(osKmlId); + m_oMapOGRIdToKmlId.erase(oIter); + } /***** mark as updated *****/ m_poOgrDS->Updated(); @@ -748,8 +874,9 @@ OGRErr OGRLIBKMLLayer::CreateField(const OGRFieldDefn *poField, int bApproxOK) { SimpleFieldPtr poKmlSimpleField = nullptr; - if ((poKmlSimpleField = FieldDef2kml( - poField, m_poOgrDS->GetKmlFactory(), CPL_TO_BOOL(bApproxOK)))) + if ((poKmlSimpleField = + FieldDef2kml(poField, m_poOgrDS->GetKmlFactory(), + CPL_TO_BOOL(bApproxOK), m_oFieldConfig))) { if (!m_poKmlSchema) { @@ -759,8 +886,7 @@ OGRErr OGRLIBKMLLayer::CreateField(const OGRFieldDefn *poField, int bApproxOK) m_poKmlSchema = poKmlFactory->CreateSchema(); /***** Set the id on the new schema *****/ - std::string oKmlSchemaID = - OGRLIBKMLGetSanitizedNCName(m_pszName); + std::string oKmlSchemaID = m_osSanitizedNCName; oKmlSchemaID.append(".schema"); m_poKmlSchema->set_id(oKmlSchemaID); } @@ -886,11 +1012,11 @@ int OGRLIBKMLLayer::TestCapability(const char *pszCap) // TODO(schwehr): The false statements are weird. if (EQUAL(pszCap, OLCRandomRead)) - result = FALSE; + result = TRUE; else if (EQUAL(pszCap, OLCSequentialWrite)) result = bUpdate; else if (EQUAL(pszCap, OLCRandomWrite)) - result = bUpdate && m_poKmlUpdate; + result = bUpdate; else if (EQUAL(pszCap, OLCFastFeatureCount)) result = FALSE; else if (EQUAL(pszCap, OLCFastSetNextByIndex)) @@ -898,7 +1024,7 @@ int OGRLIBKMLLayer::TestCapability(const char *pszCap) else if (EQUAL(pszCap, OLCCreateField)) result = bUpdate; else if (EQUAL(pszCap, OLCDeleteFeature)) - result = bUpdate && m_poKmlUpdate; + result = bUpdate; else if (EQUAL(pszCap, OLCStringsAsUTF8)) result = TRUE; else if (EQUAL(pszCap, OLCZGeometries))