From 8031d968da89e6ecf0764c1d47f72f5d018f521d Mon Sep 17 00:00:00 2001 From: Ben Marchant <13385275+bmarchant@users.noreply.github.com> Date: Wed, 2 Nov 2022 15:50:19 -0500 Subject: [PATCH] Fix duplicate relation member insertion failure (#5485) --- hoot-core/src/main/cpp/hoot/core/io/ApiDb.cpp | 2 +- .../src/main/cpp/hoot/core/io/HootApiDb.cpp | 28 +++++++++---------- .../core/io/OsmApiDbSqlChangesetApplier.cpp | 4 +-- .../main/cpp/hoot/core/io/OsmJsonReader.cpp | 14 ++++++++-- 4 files changed, 28 insertions(+), 20 deletions(-) diff --git a/hoot-core/src/main/cpp/hoot/core/io/ApiDb.cpp b/hoot-core/src/main/cpp/hoot/core/io/ApiDb.cpp index 84d2b3501c..76a6e0bb0e 100644 --- a/hoot-core/src/main/cpp/hoot/core/io/ApiDb.cpp +++ b/hoot-core/src/main/cpp/hoot/core/io/ApiDb.cpp @@ -554,7 +554,7 @@ std::shared_ptr ApiDb::selectWayIdsByWayNodeIds(const QSet& } //this has to be prepared every time due to the varying number of IDs passed in QString sql = QString("SELECT DISTINCT way_id FROM %1 WHERE node_id IN (%2)") - .arg(tableTypeToTableName(TableType::WayNode), QStringList(nodeIds.toList()).join(",")); + .arg(tableTypeToTableName(TableType::WayNode), nodeIds.toList().join(",")); _selectWayIdsByWayNodeIds->prepare(sql); LOG_VART(_selectWayIdsByWayNodeIds->lastQuery().right(100)); diff --git a/hoot-core/src/main/cpp/hoot/core/io/HootApiDb.cpp b/hoot-core/src/main/cpp/hoot/core/io/HootApiDb.cpp index b8d110df91..70c52b5940 100644 --- a/hoot-core/src/main/cpp/hoot/core/io/HootApiDb.cpp +++ b/hoot-core/src/main/cpp/hoot/core/io/HootApiDb.cpp @@ -709,9 +709,7 @@ bool HootApiDb::insertNode(const long id, const double lat, const double lon, co { QStringList columns({"id", "latitude", "longitude", "changeset_id", "timestamp", "tile", "version", "tags"}); - _nodeBulkInsert = - std::make_shared( - _db, getCurrentNodesTableName(mapId), columns, _ignoreInsertConflicts); + _nodeBulkInsert = std::make_shared(_db, getCurrentNodesTableName(mapId), columns, _ignoreInsertConflicts); } QList v; @@ -769,8 +767,7 @@ bool HootApiDb::insertRelation(const long relationId, const Tags &tags, long ver QStringList columns({"id", "changeset_id", "timestamp", "version", "tags"}); _relationBulkInsert = - std::make_shared( - _db, getCurrentRelationsTableName(mapId), columns, _ignoreInsertConflicts); + std::make_shared(_db, getCurrentRelationsTableName(mapId), columns, _ignoreInsertConflicts); } QList v; @@ -808,11 +805,16 @@ bool HootApiDb::insertRelationMember(const long relationId, const ElementType& t if (_insertRelationMembers == nullptr) { + QString sql = QString("INSERT INTO %1 (relation_id, member_type, member_id, member_role, sequence_id) " + "VALUES (:relation_id, :member_type, :member_id, :member_role, :sequence_id)") + .arg(getCurrentRelationMembersTableName(mapId)); + // ON CONFLICT DO NOTHING simply ignores each inserted row that violates an + // arbiter constraint or index + if (_ignoreInsertConflicts) + sql.append(" ON CONFLICT DO NOTHING"); + _insertRelationMembers = std::make_shared(_db); - _insertRelationMembers->prepare( - QString("INSERT INTO %1 (relation_id, member_type, member_id, member_role, sequence_id) " - "VALUES (:relation_id, :member_type, :member_id, :member_role, :sequence_id)") - .arg(getCurrentRelationMembersTableName(mapId))); + _insertRelationMembers->prepare(sql); } _insertRelationMembers->bindValue(":relation_id", (qlonglong)relationId); @@ -1427,8 +1429,7 @@ bool HootApiDb::mapExists(const long id) if (_mapExistsById == nullptr) { _mapExistsById = std::make_shared(_db); - _mapExistsById->prepare( - QString("SELECT display_name FROM %1 WHERE id = :mapId").arg(getMapsTableName())); + _mapExistsById->prepare(QString("SELECT display_name FROM %1 WHERE id = :mapId").arg(getMapsTableName())); } _mapExistsById->bindValue(":mapId", (qlonglong)id); if (_mapExistsById->exec() == false) @@ -1670,8 +1671,7 @@ void HootApiDb::insertUserSession(const long userId, const QString& sessionId) } } -void HootApiDb::updateUserAccessTokens(const long userId, const QString& accessToken, - const QString& accessTokenSecret) +void HootApiDb::updateUserAccessTokens(const long userId, const QString& accessToken, const QString& accessTokenSecret) { if (_updateUserAccessTokens == nullptr) { @@ -1958,7 +1958,7 @@ long HootApiDb::reserveElementId(const ElementType::Type type) { long retVal = -1; - switch ( type ) + switch (type) { case ElementType::Node: retVal = _getNextNodeId(); diff --git a/hoot-core/src/main/cpp/hoot/core/io/OsmApiDbSqlChangesetApplier.cpp b/hoot-core/src/main/cpp/hoot/core/io/OsmApiDbSqlChangesetApplier.cpp index 0340a43a1f..2caeccb8a6 100644 --- a/hoot-core/src/main/cpp/hoot/core/io/OsmApiDbSqlChangesetApplier.cpp +++ b/hoot-core/src/main/cpp/hoot/core/io/OsmApiDbSqlChangesetApplier.cpp @@ -87,14 +87,14 @@ void OsmApiDbSqlChangesetApplier::write(const QString& sql) const QStringList sqlParts = sql.split(";"); LOG_VARD(sqlParts.length()); - if (!sqlParts[0].toUpper().startsWith("INSERT INTO CHANGESETS")) + if (!sqlParts[0].startsWith("INSERT INTO CHANGESETS", Qt::CaseInsensitive)) throw HootException(QString("The first SQL statement in a changeset SQL file must create a changeset. Found: %1").arg(sqlParts[0].left(25))); for (const auto& sqlStatement : qAsConst(sqlParts)) { LOG_VART(sqlStatement); QString changesetStatType; - if (sqlStatement.toUpper().startsWith("INSERT INTO CHANGESETS")) + if (sqlStatement.startsWith("INSERT INTO CHANGESETS", Qt::CaseInsensitive)) { // flush if (!changesetInsertStatement.isEmpty()) diff --git a/hoot-core/src/main/cpp/hoot/core/io/OsmJsonReader.cpp b/hoot-core/src/main/cpp/hoot/core/io/OsmJsonReader.cpp index 9e7844d2fe..81d14c72a7 100644 --- a/hoot-core/src/main/cpp/hoot/core/io/OsmJsonReader.cpp +++ b/hoot-core/src/main/cpp/hoot/core/io/OsmJsonReader.cpp @@ -196,9 +196,10 @@ void OsmJsonReader::read(const OsmMapPtr& map) LOG_VARD(_map->getElementCount()); - // See related note in OsmXmlReader::read. - if (_bounds.get()) + // Bounded network queries are already cropped + if (_bounds.get() && !_isWeb) { + // See related note in OsmXmlReader::read. IoUtils::cropToBounds(_map, _bounds, _keepImmediatelyConnectedWaysOutsideBounds); LOG_VARD(StringUtils::formatLargeNumber(_map->getElementCount())); } @@ -209,7 +210,8 @@ void OsmJsonReader::_readToMap() _parseOverpassJson(); LOG_VARD(_map->getElementCount()); - if (_bounds.get()) + // Bounded network queries are already cropped + if (_bounds.get() && _isWeb) { IoUtils::cropToBounds(_map, _bounds, _keepImmediatelyConnectedWaysOutsideBounds); LOG_VARD(StringUtils::formatLargeNumber(_map->getElementCount())); @@ -268,6 +270,8 @@ bool OsmJsonReader::isValidJson(const QString& jsonStr) void OsmJsonReader::loadFromString(const QString& jsonStr, const OsmMapPtr& map) { + _isFile = false; + _isWeb = false; _map = map; _loadJSON(jsonStr); _readToMap(); @@ -276,6 +280,8 @@ void OsmJsonReader::loadFromString(const QString& jsonStr, const OsmMapPtr& map) OsmMapPtr OsmJsonReader::loadFromPtree(const boost::property_tree::ptree& tree) { + _isFile = false; + _isWeb = false; _propTree = tree; _map = std::make_shared(); _readToMap(); @@ -288,6 +294,8 @@ OsmMapPtr OsmJsonReader::loadFromFile(const QString& path) if (!infile.is_open()) throw HootException("Unable to read JSON file: " + path); + _isFile = true; + _isWeb = false; _loadJSON(infile); _map = std::make_shared(); _readToMap();