Merge pull request #10840 from rouault/libkml_update_delete

LIBKML: report XML id attribute as 'id' field; implement SetFeature() and DeleteFeature() for regular layers

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.                                            */

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"

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' `<name>`.
 
+-  .. config:: LIBKML_ID_FIELD
+      :default: id
+      :since: 3.10
+
+      Name of the string field that maps to the kml attribute
+      `<id> <https://developers.google.com/kml/documentation/kmlreference#object>`__.
+
 -  .. config:: LIBKML_NAME_FIELD
       :default: name
 

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 <even dot rouault at spatialys.com>
+ *
+ * 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

ogr/ogrsf_frmts/libkml/ogr_libkml.h

@@ -33,6 +33,7 @@
 #include "ogrsf_frmts.h"
 
 #include "libkml_headers.h"
+#include "fieldconfig.h"
 
 #include <map>
 
@@ -47,18 +48,20 @@ CPLString OGRLIBKMLGetSanitizedNCName(const char *pszName);
 class OGRLIBKMLLayer final : public OGRLayer,
                              public OGRGetNextFeatureThroughRaw<OGRLIBKMLLayer>
 {
-    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<GIntBig, std::string> m_oMapOGRIdToKmlId{};
+    std::map<std::string, GIntBig> 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,

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();
@@ -838,13 +836,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);
@@ -877,14 +875,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)
 {
@@ -905,7 +904,7 @@ OGRFeature *kmlgroundoverlay2feat(GroundOverlayPtr poKmlOverlay,
     }
 
     /***** fields *****/
-    kml2field(poOgrFeat, AsFeature(poKmlOverlay));
+    kml2field(poOgrFeat, AsFeature(poKmlOverlay), poOgrLayer->GetFieldConfig());
 
     return poOgrFeat;
 }

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);