Skip to content

Commit

Permalink
Merge pull request #11796 from rouault/libkml_id_integer
Browse files Browse the repository at this point in the history 
LIBKML: fix error when creating a Id field of type integer
  • Loading branch information
@rouault
rouault authored Feb 10, 2025
2 parents 6332083 + 4c2e68a commit e1c8701
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 25 deletions.
22 changes: 22 additions & 0 deletions autotest/ogr/ogr_libkml.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2341,3 +2341,25 @@ def ogr_libkml_non_editable():
lyr = ds.GetLayer(0)
assert lyr.TestCapability(ogr.OLCRandomWrite) == 0
assert lyr.TestCapability(ogr.OLCDeleteFeature) == 0


###############################################################################
#


def test_ogr_libkml_create_field_id_integer(tmp_vsimem):

filename = str(tmp_vsimem / "test.kml")
with ogr.GetDriverByName("LIBKML").CreateDataSource(filename) as ds:
lyr = ds.CreateLayer("test")
lyr.CreateField(ogr.FieldDefn("id", ogr.OFTInteger))
lyr.CreateField(ogr.FieldDefn("name"))
f = ogr.Feature(lyr.GetLayerDefn())
f["id"] = 1
f.SetGeometry(ogr.CreateGeometryFromWkt("POINT (1 2)"))
lyr.CreateFeature(f)

with gdal.OpenEx(filename, gdal.OF_VECTOR | gdal.OF_UPDATE) as ds:
lyr = ds.GetLayer(0)
f = lyr.GetNextFeature()
assert f["id"] == "1"
9 changes: 5 additions & 4 deletions ogr/ogrsf_frmts/libkml/ogr_libkml.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@ CPLString OGRLIBKMLGetSanitizedNCName(const char *pszName);
class OGRLIBKMLLayer final : public OGRLayer,
public OGRGetNextFeatureThroughRaw<OGRLIBKMLLayer>
{
int bUpdate = false;
bool m_bNew = false;
bool bUpdate = false;

int nFeatures = 0;
int iFeature = 0;
Expand Down Expand Up @@ -84,7 +85,7 @@ class OGRLIBKMLLayer final : public OGRLayer,
OGRLIBKMLDataSource *poOgrDS, kmldom::ElementPtr poKmlRoot,
kmldom::ContainerPtr poKmlContainer,
kmldom::UpdatePtr poKmlUpdate, const char *pszFileName,
int bNew, int bUpdate);
bool bNew, bool bUpdate);
virtual ~OGRLIBKMLLayer();

void ResetReading() override
Expand Down Expand Up @@ -265,7 +266,7 @@ class OGRLIBKMLDataSource final : public GDALDataset
void SetStyleTableDirectly(OGRStyleTable *poStyleTable) override;
void SetStyleTable(OGRStyleTable *poStyleTable) override;

int Open(const char *pszFilename, int bUpdate);
int Open(const char *pszFilename, bool bUpdate);
int Create(const char *pszFilename, char **papszOptions);

CPLErr FlushCache(bool bAtClosing) override;
Expand Down Expand Up @@ -357,7 +358,7 @@ class OGRLIBKMLDataSource final : public GDALDataset
AddLayer(const char *pszLayerName, OGRwkbGeometryType eGType,
const OGRSpatialReference *poSRS, OGRLIBKMLDataSource *poOgrDS,
kmldom::ElementPtr poKmlRoot, kmldom::ContainerPtr poKmlContainer,
const char *pszFileName, int bNew, int bUpdate, int nGuess);
const char *pszFileName, bool bNew, bool bUpdate, int nGuess);
};

#endif
16 changes: 8 additions & 8 deletions ogr/ogrsf_frmts/libkml/ogrlibkmldatasource.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -846,7 +846,7 @@ OGRLIBKMLLayer *OGRLIBKMLDataSource::AddLayer(
const char *pszLayerName, OGRwkbGeometryType eGType,
const OGRSpatialReference *poSRS, OGRLIBKMLDataSource *poOgrDS,
ElementPtr poKmlRoot, ContainerPtr poKmlContainer, const char *pszFileName,
int bNew, int bUpdateIn, int nGuess)
bool bNew, bool bUpdateIn, int nGuess)
{
// Build unique layer name
CPLString osUniqueLayername(pszLayerName);
Expand Down Expand Up @@ -947,7 +947,7 @@ int OGRLIBKMLDataSource::ParseLayers(ContainerPtr poKmlContainer, bool bRecurse)
/***** create the layer *****/

AddLayer(oKmlFeatName.c_str(), wkbUnknown, nullptr, this,
nullptr, AsContainer(poKmlFeat), "", FALSE, bUpdate,
nullptr, AsContainer(poKmlFeat), "", false, bUpdate,
static_cast<int>(nKmlFeatures));

/***** check if any features are another layer *****/
Expand Down Expand Up @@ -1293,7 +1293,7 @@ int OGRLIBKMLDataSource::OpenKmz(const char *pszFilename, int bUpdateIn)

AddLayer(osLayerName.c_str(), wkbUnknown, nullptr, this,
std::move(poKmlLyrRoot), poKmlLyrContainer,
oKmlHref.get_path().c_str(), FALSE, bUpdateIn,
oKmlHref.get_path().c_str(), false, bUpdateIn,
static_cast<int>(nKmlFeatures));

/***** check if any features are another layer *****/
Expand Down Expand Up @@ -1335,7 +1335,7 @@ int OGRLIBKMLDataSource::OpenKmz(const char *pszFilename, int bUpdateIn)

AddLayer(layername_default.c_str(), wkbUnknown, nullptr, this,
std::move(poKmlDocKmlRoot), poKmlContainer, pszFilename,
FALSE, bUpdateIn, 1);
false, bUpdateIn, 1);
}

ParseLayers(std::move(poKmlContainer), true);
Expand Down Expand Up @@ -1458,7 +1458,7 @@ int OGRLIBKMLDataSource::OpenDir(const char *pszFilename, int bUpdateIn)
/***** create the layer *****/
AddLayer(osLayerName.c_str(), wkbUnknown, nullptr, this,
std::move(poKmlRoot), poKmlContainer, osFilePath.c_str(),
FALSE, bUpdateIn, nFiles);
false, bUpdateIn, nFiles);

/***** check if any features are another layer *****/
ParseLayers(std::move(poKmlContainer), true);
Expand Down Expand Up @@ -1514,7 +1514,7 @@ static bool CheckIsKMZ(const char *pszFilename)
return bFoundKML;
}

int OGRLIBKMLDataSource::Open(const char *pszFilename, int bUpdateIn)
int OGRLIBKMLDataSource::Open(const char *pszFilename, bool bUpdateIn)
{
bUpdate = CPL_TO_BOOL(bUpdateIn);

Expand Down Expand Up @@ -2171,7 +2171,7 @@ OGRLIBKMLLayer *OGRLIBKMLDataSource::CreateLayerKml(
/***** create the layer *****/
OGRLIBKMLLayer *poOgrLayer =
AddLayer(pszLayerName, eGType, poSRS, this, nullptr,
poKmlLayerContainer, "", TRUE, bUpdate, 1);
poKmlLayerContainer, "", true, bUpdate, 1);

/***** add the layer name as a <Name> *****/
if (poKmlLayerContainer)
Expand Down Expand Up @@ -2239,7 +2239,7 @@ OGRLIBKMLLayer *OGRLIBKMLDataSource::CreateLayerKmz(
OGRLIBKMLLayer *poOgrLayer =
AddLayer(pszLayerName, eGType, poSRS, this, nullptr, poKmlDocument,
CPLFormFilenameSafe(nullptr, pszLayerName, ".kml").c_str(),
TRUE, bUpdate, 1);
true, bUpdate, 1);

/***** add the layer name as a <Name> *****/
if (!m_poKmlUpdate)
Expand Down
29 changes: 16 additions & 13 deletions ogr/ogrsf_frmts/libkml/ogrlibkmllayer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,8 +105,8 @@ OGRLIBKMLLayer::OGRLIBKMLLayer(
const char *pszLayerName, OGRwkbGeometryType eGType,
const OGRSpatialReference *poSRSIn, OGRLIBKMLDataSource *poOgrDS,
ElementPtr poKmlRoot, ContainerPtr poKmlContainer, UpdatePtr poKmlUpdate,
const char *pszFileName, int bNew, int bUpdateIn)
: bUpdate(CPL_TO_BOOL(bUpdateIn)), m_pszName(CPLStrdup(pszLayerName)),
const char *pszFileName, bool bNew, bool bUpdateIn)
: m_bNew(bNew), bUpdate(bUpdateIn), m_pszName(CPLStrdup(pszLayerName)),
m_pszFileName(CPLStrdup(pszFileName)),
m_poKmlLayer(std::move(poKmlContainer)), // Store the layers container.
m_poKmlLayerRoot(
Expand Down Expand Up @@ -536,19 +536,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))
if (!m_bNew)
{
ScanAllFeatures();

if (cpl::contains(m_oMapKmlIdToOGRId,
poOgrFeat->GetFieldAsString(idxIdField)))
const int idxIdField =
m_poOgrFeatureDefn->GetFieldIndex(m_oFieldConfig.idfield);
if (idxIdField >= 0 && poOgrFeat->IsFieldSet(idxIdField))
{
CPLError(CE_Failure, CPLE_AppDefined,
"A feature with id %s already exists",
poOgrFeat->GetFieldAsString(idxIdField));
return OGRERR_FAILURE;
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;
}
}
}

Expand Down

0 comments on commit e1c8701

Please sign in to comment.