From baaae99a03202641ae5b8024e57b691fe61a6688 Mon Sep 17 00:00:00 2001
From: John Firebaugh <john.firebaugh@gmail.com>
Date: Fri, 5 May 2017 16:31:03 -0700
Subject: [core, android] Factor JSON string conversions

This adds a `convertJSON` template, to be used like:

   Error error
   optional<Foo> foo = convertJSON<Foo>(string, error);

Internally, it parses the string with RapidJSON and then calls `convert<Foo>(parsed, error)`.

While here, rationalize GeoJSON converters and fix error handling for Tileset conversion in OfflineDownload.
---
 platform/android/src/style/conversion/geojson.hpp  | 14 ++---------
 platform/default/mbgl/storage/offline_download.cpp | 27 ++++++++++++++--------
 platform/node/src/node_geojson.hpp                 |  2 +-
 platform/qt/src/qmapboxgl.cpp                      |  2 +-
 platform/qt/src/qt_geojson.hpp                     | 11 +++------
 5 files changed, 25 insertions(+), 31 deletions(-)

(limited to 'platform')

diff --git a/platform/android/src/style/conversion/geojson.hpp b/platform/android/src/style/conversion/geojson.hpp
index 32a473b092..748fe7361e 100644
--- a/platform/android/src/style/conversion/geojson.hpp
+++ b/platform/android/src/style/conversion/geojson.hpp
@@ -10,25 +10,15 @@ namespace style {
 namespace conversion {
 
 template <>
-optional<GeoJSON> convertGeoJSON(const mbgl::android::Value& value, Error& error) {
-
+optional<GeoJSON> Converter<GeoJSON>::operator()(const mbgl::android::Value& value, Error& error) const {
     if(value.isNull() || !value.isString()) {
         error = { "no json data found" };
         return {};
     }
 
-    return convertGeoJSON(value.toString(), error);
+    return convert<GeoJSON>(value.toString(), error);
 }
 
-template <>
-struct Converter<GeoJSON> {
-
-    optional<GeoJSON> operator()(const mbgl::android::Value& value, Error& error) const {
-        return convertGeoJSON(value, error);
-    }
-
-};
-
 } // namespace conversion
 } // namespace style
 } // namespace mbgl
diff --git a/platform/default/mbgl/storage/offline_download.cpp b/platform/default/mbgl/storage/offline_download.cpp
index cba2f70df8..901f996a4f 100644
--- a/platform/default/mbgl/storage/offline_download.cpp
+++ b/platform/default/mbgl/storage/offline_download.cpp
@@ -7,6 +7,8 @@
 #include <mbgl/style/parser.hpp>
 #include <mbgl/style/sources/geojson_source_impl.hpp>
 #include <mbgl/style/tile_source_impl.hpp>
+#include <mbgl/style/conversion/json.hpp>
+#include <mbgl/style/conversion/tileset.hpp>
 #include <mbgl/text/glyph.hpp>
 #include <mbgl/util/mapbox.hpp>
 #include <mbgl/util/run_loop.hpp>
@@ -87,9 +89,12 @@ OfflineRegionStatus OfflineDownload::getStatus() const {
                 const std::string& url = urlOrTileset.get<std::string>();
                 optional<Response> sourceResponse = offlineDatabase.get(Resource::source(url));
                 if (sourceResponse) {
-                    result.requiredResourceCount +=
-                        definition.tileCover(type, tileSize, style::TileSourceImpl::parseTileJSON(
-                            *sourceResponse->data, url, type, tileSize).zoomRange).size();
+                    style::conversion::Error error;
+                    optional<Tileset> tileset = style::conversion::convertJSON<Tileset>(*sourceResponse->data, error);
+                    if (tileset) {
+                        result.requiredResourceCount +=
+                            definition.tileCover(type, tileSize, (*tileset).zoomRange).size();
+                    }
                 } else {
                     result.requiredResourceCountIsPrecise = false;
                 }
@@ -152,12 +157,16 @@ void OfflineDownload::activateDownload() {
                     requiredSourceURLs.insert(url);
 
                     ensureResource(Resource::source(url), [=](Response sourceResponse) {
-                        queueTiles(type, tileSize, style::TileSourceImpl::parseTileJSON(
-                            *sourceResponse.data, url, type, tileSize));
-
-                        requiredSourceURLs.erase(url);
-                        if (requiredSourceURLs.empty()) {
-                            status.requiredResourceCountIsPrecise = true;
+                        style::conversion::Error error;
+                        optional<Tileset> tileset = style::conversion::convertJSON<Tileset>(*sourceResponse.data, error);
+                        if (tileset) {
+                            util::mapbox::canonicalizeTileset(*tileset, url, type, tileSize);
+                            queueTiles(type, tileSize, *tileset);
+
+                            requiredSourceURLs.erase(url);
+                            if (requiredSourceURLs.empty()) {
+                                status.requiredResourceCountIsPrecise = true;
+                            }
                         }
                     });
                 }
diff --git a/platform/node/src/node_geojson.hpp b/platform/node/src/node_geojson.hpp
index e4b2ca47e7..9bae86b76a 100644
--- a/platform/node/src/node_geojson.hpp
+++ b/platform/node/src/node_geojson.hpp
@@ -5,7 +5,7 @@ namespace style {
 namespace conversion {
 
 template <>
-optional<GeoJSON> convertGeoJSON(const v8::Local<v8::Value>&, Error& error) {
+optional<GeoJSON> Converter<GeoJSON>::operator()(const v8::Local<v8::Value>&, Error& error) const {
     error = { "not implemented" };
     return {};
 }
diff --git a/platform/qt/src/qmapboxgl.cpp b/platform/qt/src/qmapboxgl.cpp
index edb8afa28a..ce7e237afb 100644
--- a/platform/qt/src/qmapboxgl.cpp
+++ b/platform/qt/src/qmapboxgl.cpp
@@ -1303,7 +1303,7 @@ void QMapboxGL::updateSource(const QString &id, const QVariantMap &params)
 
     if (params.contains("data")) {
         Error error;
-        auto result = convertGeoJSON(params["data"], error);
+        auto result = convert<mbgl::GeoJSON>(params["data"], error);
         if (result) {
             sourceGeoJSON->setGeoJSON(*result);
         }
diff --git a/platform/qt/src/qt_geojson.hpp b/platform/qt/src/qt_geojson.hpp
index 7d12660aec..a6958b7edc 100644
--- a/platform/qt/src/qt_geojson.hpp
+++ b/platform/qt/src/qt_geojson.hpp
@@ -178,25 +178,20 @@ namespace style {
 namespace conversion {
 
 template <>
-optional<GeoJSON> convertGeoJSON(const QMapbox::Feature& feature, Error&) {
-    return GeoJSON { asMapboxGLFeature(feature) };
-}
-
-template <>
-optional<GeoJSON> convertGeoJSON(const QVariant& value, Error& error) {
+optional<GeoJSON> Converter<GeoJSON>::operator()(const QVariant& value, Error& error) const {
 #if QT_VERSION >= 0x050000
     if (value.typeName() == QStringLiteral("QMapbox::Feature")) {
 #else
     if (value.typeName() == QString("QMapbox::Feature")) {
 #endif
-        return convertGeoJSON(value.value<QMapbox::Feature>(), error);
+        return GeoJSON { asMapboxGLFeature(value.value<QMapbox::Feature>()) };
     } else if (value.type() != QVariant::ByteArray) {
         error = { "JSON data must be in QByteArray" };
         return {};
     }
 
     QByteArray data = value.toByteArray();
-    return convertGeoJSON(std::string(data.constData(), data.size()), error);
+    return convert<GeoJSON>(std::string(data.constData(), data.size()), error);
 }
 
 } // namespace conversion
-- 
cgit v1.2.1