Fix Mapbox plugin mixing tiles for specified map_ids

This patch fixes the handling of map_ids, preventing
the plugin from picking the wrong ids when reading
or writing tiles.

This changes also makes the generation of map types
consistent with the other plugin, with the first type
having mapId of 1.

Change-Id: I1562a2bbccacf11958ee22a60e9eee74faa15a2e
Reviewed-by: Thiago Marcos P. Santos <tmpsantos@gmail.com>
Reviewed-by: Alex Blasche <alexander.blasche@qt.io>

4 files changed, 24 insertions, 21 deletions

diff --git a/src/location/doc/src/plugins/mapbox.qdoc b/src/location/doc/src/plugins/mapbox.qdoc
index e962bead..8f83ae55 100644
--- a/src/location/doc/src/plugins/mapbox.qdoc
+++ b/src/location/doc/src/plugins/mapbox.qdoc
@@ -70,6 +70,9 @@ The following table lists optional parameters that can be passed to the Mapbox p
     \li \l{https://www.mapbox.com/help/define-map-id/}{ID} of the Mapbox map to show. An example ID is "examples.map-zr0njcqy".
     If this parameter is present, the specified map type will be used by default, unless another is selected.
     If not present, the default Mapbox map ID is "mapbox.streets".
+    \b{Note:} neither in this parameter nor in \b{mapbox.mapping.additional_map_ids} it is allowed to use repeated map_ids.
+    This includes the map_ids bundled in the plugin by default (documented \l{https://www.mapbox.com/api-documentation/#maps}{here} under
+    \b{Mapbox classic map IDs}). Failing to do so will cause tile cache corruption.
 \row
     \li mapbox.mapping.additional_map_ids
     \li Additional, comma separated, Mapbox map IDs to be added to the available map types.
diff --git a/src/plugins/geoservices/mapbox/qgeofiletilecachemapbox.cpp b/src/plugins/geoservices/mapbox/qgeofiletilecachemapbox.cpp
index 8cc3622b..2792ee78 100644
--- a/src/plugins/geoservices/mapbox/qgeofiletilecachemapbox.cpp
+++ b/src/plugins/geoservices/mapbox/qgeofiletilecachemapbox.cpp
@@ -45,7 +45,7 @@ QGeoFileTileCacheMapbox::QGeoFileTileCacheMapbox(const QList<QGeoMapType> &mapTy
 {
     m_scaleFactor = qBound(1, scaleFactor, 2);
     for (int i=0; i < mapTypes.size(); i++)
-        m_mapNameToId.insert(mapTypes[i].name(), i);
+        m_mapNameToId.insert(mapTypes[i].name(), i+1);
 }
 
 QGeoFileTileCacheMapbox::~QGeoFileTileCacheMapbox()
@@ -57,7 +57,7 @@ QString QGeoFileTileCacheMapbox::tileSpecToFilename(const QGeoTileSpec &spec, co
 {
     QString filename = spec.plugin();
     filename += QLatin1String("-");
-    filename += m_mapTypes[spec.mapId()].name();
+    filename += m_mapTypes[spec.mapId()-1].name();
     filename += QLatin1String("-");
     filename += QString::number(spec.zoom());
     filename += QLatin1String("-");
@@ -87,7 +87,7 @@ QGeoTileSpec QGeoFileTileCacheMapbox::filenameToTileSpec(const QString &filename
 {
     QStringList parts = filename.split('.');
 
-    if (parts.length() != 3)
+    if (parts.length() != 3) // 3 because the map name has always a dot in it.
         return QGeoTileSpec();
 
     QString name = parts.at(0) + parts.at(1);
diff --git a/src/plugins/geoservices/mapbox/qgeotiledmappingmanagerenginemapbox.cpp b/src/plugins/geoservices/mapbox/qgeotiledmappingmanagerenginemapbox.cpp
index 3177b6c4..03a574c3 100644
--- a/src/plugins/geoservices/mapbox/qgeotiledmappingmanagerenginemapbox.cpp
+++ b/src/plugins/geoservices/mapbox/qgeotiledmappingmanagerenginemapbox.cpp
@@ -66,41 +66,41 @@ QGeoTiledMappingManagerEngineMapbox::QGeoTiledMappingManagerEngineMapbox(const Q
     // as index 0 to retain compatibility with the current API, that expects the passed map_id to be on by default.
     if (parameters.contains(QStringLiteral("mapbox.mapping.map_id"))) {
         const QString name = parameters.value(QStringLiteral("mapbox.mapping.map_id")).toString();
-        mapTypes << QGeoMapType(QGeoMapType::CustomMap, name, name, false, false, mapTypes.size(), pluginName);
+        mapTypes << QGeoMapType(QGeoMapType::CustomMap, name, name, false, false, mapTypes.size() + 1, pluginName);
     } else if (parameters.contains(QStringLiteral("mapbox.map_id"))) { //deprecated
         const QString name = parameters.value(QStringLiteral("mapbox.map_id")).toString();
-        mapTypes << QGeoMapType(QGeoMapType::CustomMap, name, name, false, false, mapTypes.size(), pluginName);
+        mapTypes << QGeoMapType(QGeoMapType::CustomMap, name, name, false, false, mapTypes.size() + 1, pluginName);
     }
 
     // As of 2016.06.15, valid mapbox map_ids are documented at https://www.mapbox.com/api-documentation/#maps
     //: Noun describing map type 'Street map'
-    mapTypes << QGeoMapType(QGeoMapType::StreetMap, QStringLiteral("mapbox.streets"), tr("Street"), false, false, mapTypes.size(), pluginName);
+    mapTypes << QGeoMapType(QGeoMapType::StreetMap, QStringLiteral("mapbox.streets"), tr("Street"), false, false, mapTypes.size() + 1, pluginName);
     //: Noun describing type of a map using light colors (weak contrast)
-    mapTypes << QGeoMapType(QGeoMapType::StreetMap, QStringLiteral("mapbox.light"), tr("Light"), false, false, mapTypes.size(), pluginName);
+    mapTypes << QGeoMapType(QGeoMapType::StreetMap, QStringLiteral("mapbox.light"), tr("Light"), false, false, mapTypes.size() + 1, pluginName);
     //: Noun describing type of a map using dark colors
-    mapTypes << QGeoMapType(QGeoMapType::StreetMap, QStringLiteral("mapbox.dark"), tr("Dark"), false, true, mapTypes.size(), pluginName);
+    mapTypes << QGeoMapType(QGeoMapType::StreetMap, QStringLiteral("mapbox.dark"), tr("Dark"), false, true, mapTypes.size() + 1, pluginName);
     //: Noun describing type of a map created by satellite
-    mapTypes << QGeoMapType(QGeoMapType::SatelliteMapDay, QStringLiteral("mapbox.satellite"), tr("Satellite"), false, false, mapTypes.size(), pluginName);
+    mapTypes << QGeoMapType(QGeoMapType::SatelliteMapDay, QStringLiteral("mapbox.satellite"), tr("Satellite"), false, false, mapTypes.size() + 1, pluginName);
     //: Noun describing type of a street map created by satellite
-    mapTypes << QGeoMapType(QGeoMapType::HybridMap, QStringLiteral("mapbox.streets-satellite"), tr("Streets Satellite"), false, false, mapTypes.size(), pluginName);
+    mapTypes << QGeoMapType(QGeoMapType::HybridMap, QStringLiteral("mapbox.streets-satellite"), tr("Streets Satellite"), false, false, mapTypes.size() + 1, pluginName);
     //: Noun describing type of a map using wheat paste colors
-    mapTypes << QGeoMapType(QGeoMapType::CustomMap, QStringLiteral("mapbox.wheatpaste"), tr("Wheatpaste"), false, false, mapTypes.size(), pluginName);
+    mapTypes << QGeoMapType(QGeoMapType::CustomMap, QStringLiteral("mapbox.wheatpaste"), tr("Wheatpaste"), false, false, mapTypes.size() + 1, pluginName);
     //: Noun describing type of a basic street map
-    mapTypes << QGeoMapType(QGeoMapType::StreetMap, QStringLiteral("mapbox.streets-basic"), tr("Streets Basic"), false, false, mapTypes.size(), pluginName);
+    mapTypes << QGeoMapType(QGeoMapType::StreetMap, QStringLiteral("mapbox.streets-basic"), tr("Streets Basic"), false, false, mapTypes.size() + 1, pluginName);
     //: Noun describing type of a map using cartoon-style fonts
-    mapTypes << QGeoMapType(QGeoMapType::CustomMap, QStringLiteral("mapbox.comic"), tr("Comic"), false, false, mapTypes.size(), pluginName);
+    mapTypes << QGeoMapType(QGeoMapType::CustomMap, QStringLiteral("mapbox.comic"), tr("Comic"), false, false, mapTypes.size() + 1, pluginName);
     //: Noun describing type of a map for outdoor activities
-    mapTypes << QGeoMapType(QGeoMapType::PedestrianMap, QStringLiteral("mapbox.outdoors"), tr("Outdoors"), false, false, mapTypes.size(), pluginName);
+    mapTypes << QGeoMapType(QGeoMapType::PedestrianMap, QStringLiteral("mapbox.outdoors"), tr("Outdoors"), false, false, mapTypes.size() + 1, pluginName);
     //: Noun describing type of a map for sports
-    mapTypes << QGeoMapType(QGeoMapType::CycleMap, QStringLiteral("mapbox.run-bike-hike"), tr("Run Bike Hike"), false, false, mapTypes.size(), pluginName);
+    mapTypes << QGeoMapType(QGeoMapType::CycleMap, QStringLiteral("mapbox.run-bike-hike"), tr("Run Bike Hike"), false, false, mapTypes.size() + 1, pluginName);
     //: Noun describing type of a map drawn by pencil
-    mapTypes << QGeoMapType(QGeoMapType::CustomMap, QStringLiteral("mapbox.pencil"), tr("Pencil"), false, false, mapTypes.size(), pluginName);
+    mapTypes << QGeoMapType(QGeoMapType::CustomMap, QStringLiteral("mapbox.pencil"), tr("Pencil"), false, false, mapTypes.size() + 1, pluginName);
     //: Noun describing type of a treasure map with pirate boat watermark
-    mapTypes << QGeoMapType(QGeoMapType::CustomMap, QStringLiteral("mapbox.pirates"), tr("Pirates"), false, false, mapTypes.size(), pluginName);
+    mapTypes << QGeoMapType(QGeoMapType::CustomMap, QStringLiteral("mapbox.pirates"), tr("Pirates"), false, false, mapTypes.size() + 1, pluginName);
     //: Noun describing type of a map using emerald colors
-    mapTypes << QGeoMapType(QGeoMapType::CustomMap, QStringLiteral("mapbox.emerald"), tr("Emerald"), false, false, mapTypes.size(), pluginName);
+    mapTypes << QGeoMapType(QGeoMapType::CustomMap, QStringLiteral("mapbox.emerald"), tr("Emerald"), false, false, mapTypes.size() + 1, pluginName);
     //: Noun describing type of a map with high contrast
-    mapTypes << QGeoMapType(QGeoMapType::CustomMap, QStringLiteral("mapbox.high-contrast"), tr("High Contrast"), false, false, mapTypes.size(), pluginName);
+    mapTypes << QGeoMapType(QGeoMapType::CustomMap, QStringLiteral("mapbox.high-contrast"), tr("High Contrast"), false, false, mapTypes.size() + 1, pluginName);
 
     // New way to specify multiple customized map_ids via additional_map_ids
     if (parameters.contains(QStringLiteral("mapbox.mapping.additional_map_ids"))) {
@@ -109,7 +109,7 @@ QGeoTiledMappingManagerEngineMapbox::QGeoTiledMappingManagerEngineMapbox(const Q
 
         for (const QString &name: idList) {
             if (!name.isEmpty())
-                mapTypes << QGeoMapType(QGeoMapType::CustomMap, name, name, false, false, mapTypes.size(), pluginName);
+                mapTypes << QGeoMapType(QGeoMapType::CustomMap, name, name, false, false, mapTypes.size() + 1, pluginName);
         }
     }
 
diff --git a/src/plugins/geoservices/mapbox/qgeotilefetchermapbox.cpp b/src/plugins/geoservices/mapbox/qgeotilefetchermapbox.cpp
index 732d04e9..74971692 100644
--- a/src/plugins/geoservices/mapbox/qgeotilefetchermapbox.cpp
+++ b/src/plugins/geoservices/mapbox/qgeotilefetchermapbox.cpp
@@ -87,7 +87,7 @@ QGeoTiledMapReply *QGeoTileFetcherMapbox::getTileImage(const QGeoTileSpec &spec)
     request.setRawHeader("User-Agent", m_userAgent);
 
     request.setUrl(QUrl(QStringLiteral("http://api.tiles.mapbox.com/v4/") +
-                        ((spec.mapId() >= m_mapIds.size()) ? QStringLiteral("mapbox.streets") : m_mapIds[spec.mapId()]) + QLatin1Char('/') +
+                        ((spec.mapId() >= m_mapIds.size()) ? QStringLiteral("mapbox.streets") : m_mapIds[spec.mapId() - 1]) + QLatin1Char('/') +
                         QString::number(spec.zoom()) + QLatin1Char('/') +
                         QString::number(spec.x()) + QLatin1Char('/') +
                         QString::number(spec.y()) +