[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.

7 files changed, 66 insertions, 98 deletions

diff --git a/src/mbgl/style/conversion/geojson.cpp b/src/mbgl/style/conversion/geojson.cpp
index 0fba6e5b4d..8103e9014a 100644
--- a/src/mbgl/style/conversion/geojson.cpp
+++ b/src/mbgl/style/conversion/geojson.cpp
@@ -1,33 +1,26 @@
 #include <mbgl/style/conversion/geojson.hpp>
-
+#include <mbgl/style/conversion/json.hpp>
 #include <mbgl/util/rapidjson.hpp>
 
-#include <string>
-#include <sstream>
+#include <mapbox/geojson.hpp>
+#include <mapbox/geojson/rapidjson.hpp>
 
 namespace mbgl {
 namespace style {
 namespace conversion {
 
-template <>
-optional<GeoJSON> convertGeoJSON(const std::string& string, Error& error) {
-    rapidjson::GenericDocument<rapidjson::UTF8<>, rapidjson::CrtAllocator> d;
-    d.Parse(string.c_str());
-
-    if (d.HasParseError()) {
-        std::stringstream message;
-        message << d.GetErrorOffset() << " - " << rapidjson::GetParseError_En(d.GetParseError());
-        error = { message.str() };
-        return {};
-    }
+optional<GeoJSON> Converter<GeoJSON>::operator()(const std::string& value, Error& error) const {
+    return convertJSON<GeoJSON>(value, error);
+}
 
-    optional<GeoJSON> geoJSON = conversion::convertGeoJSON<JSValue>(d, error);
-    if (!geoJSON) {
-        error = { error.message };
+template <>
+optional<GeoJSON> Converter<GeoJSON>::operator()(const JSValue& value, Error& error) const {
+    try {
+        return mapbox::geojson::convert(value);
+    } catch (const std::exception& ex) {
+        error = { ex.what() };
         return {};
     }
-
-    return geoJSON;
 }
 
 } // namespace conversion
diff --git a/src/mbgl/style/conversion/json.hpp b/src/mbgl/style/conversion/json.hpp
new file mode 100644
index 0000000000..0817ac09df
--- /dev/null
+++ b/src/mbgl/style/conversion/json.hpp
@@ -0,0 +1,28 @@
+#include <mbgl/style/conversion.hpp>
+#include <mbgl/style/rapidjson_conversion.hpp>
+
+#include <string>
+#include <sstream>
+
+namespace mbgl {
+namespace style {
+namespace conversion {
+
+template <class T, class...Args>
+optional<T> convertJSON(const std::string& json, Error& error, Args&&...args) {
+    JSDocument document;
+    document.Parse<0>(json.c_str());
+
+    if (document.HasParseError()) {
+        std::stringstream message;
+        message << document.GetErrorOffset() << " - " << rapidjson::GetParseError_En(document.GetParseError());
+        error = { message.str() };
+        return {};
+    }
+
+    return convert<T, JSValue>(document, error, std::forward<Args>(args)...);
+}
+
+} // namespace conversion
+} // namespace style
+} // namespace mbgl
diff --git a/src/mbgl/style/sources/geojson_source_impl.cpp b/src/mbgl/style/sources/geojson_source_impl.cpp
index 08ed95ea3f..1a686ff9bc 100644
--- a/src/mbgl/style/sources/geojson_source_impl.cpp
+++ b/src/mbgl/style/sources/geojson_source_impl.cpp
@@ -1,37 +1,18 @@
 #include <mbgl/style/sources/geojson_source_impl.hpp>
+#include <mbgl/style/conversion/json.hpp>
 #include <mbgl/style/conversion/geojson.hpp>
 #include <mbgl/style/source_observer.hpp>
 #include <mbgl/tile/tile_id.hpp>
 #include <mbgl/storage/file_source.hpp>
 #include <mbgl/renderer/sources/render_geojson_source.hpp>
-#include <mbgl/util/rapidjson.hpp>
 #include <mbgl/util/constants.cpp>
 #include <mbgl/util/logging.hpp>
 
-#include <mapbox/geojson.hpp>
-#include <mapbox/geojson/rapidjson.hpp>
 #include <mapbox/geojsonvt.hpp>
-#include <mapbox/geojsonvt/convert.hpp>
 #include <supercluster.hpp>
 
-#include <rapidjson/error/en.h>
-
-#include <sstream>
-
 namespace mbgl {
 namespace style {
-namespace conversion {
-
-template <>
-optional<GeoJSON> convertGeoJSON(const JSValue& value, Error& error) {
-    try {
-        return mapbox::geojson::convert(value);
-    } catch (const std::exception& ex) {
-        error = { ex.what() };
-        return {};
-    }
-}
-} // namespace conversion
 
 class GeoJSONVTData : public GeoJSONData {
 public:
@@ -129,20 +110,8 @@ void GeoJSONSource::Impl::loadDescription(FileSource& fileSource) {
             observer->onSourceError(
                 base, std::make_exception_ptr(std::runtime_error("unexpectedly empty GeoJSON")));
         } else {
-            rapidjson::GenericDocument<rapidjson::UTF8<>, rapidjson::CrtAllocator> d;
-            d.Parse<0>(res.data->c_str());
-
-            if (d.HasParseError()) {
-                std::stringstream message;
-                message << d.GetErrorOffset() << " - "
-                        << rapidjson::GetParseError_En(d.GetParseError());
-                observer->onSourceError(base,
-                                        std::make_exception_ptr(std::runtime_error(message.str())));
-                return;
-            }
-
             conversion::Error error;
-            optional<GeoJSON> geoJSON = conversion::convertGeoJSON<JSValue>(d, error);
+            optional<GeoJSON> geoJSON = conversion::convertJSON<GeoJSON>(*res.data, error);
             if (!geoJSON) {
                 Log::Error(Event::ParseStyle, "Failed to parse GeoJSON data: %s",
                            error.message.c_str());
diff --git a/src/mbgl/style/tile_source_impl.cpp b/src/mbgl/style/tile_source_impl.cpp
index 4623714794..d2ce3def9f 100644
--- a/src/mbgl/style/tile_source_impl.cpp
+++ b/src/mbgl/style/tile_source_impl.cpp
@@ -1,45 +1,13 @@
 #include <mbgl/style/tile_source_impl.hpp>
 #include <mbgl/style/source_observer.hpp>
-#include <mbgl/style/rapidjson_conversion.hpp>
+#include <mbgl/style/conversion/json.hpp>
 #include <mbgl/style/conversion/tileset.hpp>
-#include <mbgl/util/tileset.hpp>
 #include <mbgl/util/mapbox.hpp>
 #include <mbgl/storage/file_source.hpp>
 
-#include <rapidjson/document.h>
-#include <rapidjson/error/en.h>
-
-#include <sstream>
-
 namespace mbgl {
 namespace style {
 
-Tileset TileSourceImpl::parseTileJSON(const std::string& json, const std::string& sourceURL, SourceType type, uint16_t tileSize) {
-    rapidjson::GenericDocument<rapidjson::UTF8<>, rapidjson::CrtAllocator> document;
-    document.Parse<0>(json.c_str());
-
-    if (document.HasParseError()) {
-        std::stringstream message;
-        message << document.GetErrorOffset() << " - " << rapidjson::GetParseError_En(document.GetParseError());
-        throw std::runtime_error(message.str());
-    }
-
-    conversion::Error error;
-    optional<Tileset> result = conversion::convert<Tileset, JSValue>(document, error);
-    if (!result) {
-        throw std::runtime_error(error.message);
-    }
-
-    // TODO: Remove this hack by delivering proper URLs in the TileJSON to begin with.
-    if (util::mapbox::isMapboxURL(sourceURL)) {
-        for (auto& url : (*result).tiles) {
-            url = util::mapbox::canonicalizeTileURL(url, type, tileSize);
-        }
-    }
-
-    return *result;
-}
-
 TileSourceImpl::TileSourceImpl(SourceType type_, std::string id_, Source& base_,
                                variant<std::string, Tileset> urlOrTileset_,
                                uint16_t tileSize_)
@@ -70,21 +38,17 @@ void TileSourceImpl::loadDescription(FileSource& fileSource) {
         } else if (res.noContent) {
             observer->onSourceError(base, std::make_exception_ptr(std::runtime_error("unexpectedly empty TileJSON")));
         } else {
-            Tileset newTileset;
-
-            // Create a new copy of the Tileset object that holds the base values we've parsed
-            // from the stylesheet. Then merge in the values parsed from the TileJSON we retrieved
-            // via the URL.
-            try {
-                newTileset = parseTileJSON(*res.data, url, type, tileSize);
-            } catch (...) {
-                observer->onSourceError(base, std::current_exception());
+            conversion::Error error;
+            optional<Tileset> newTileset = conversion::convertJSON<Tileset>(*res.data, error);
+            if (!newTileset) {
+                observer->onSourceError(base, std::make_exception_ptr(std::runtime_error(error.message)));
                 return;
             }
 
-            bool attributionChanged = tileset.attribution != newTileset.attribution;
+            util::mapbox::canonicalizeTileset(*newTileset, url, type, tileSize);
+            bool attributionChanged = tileset.attribution != (*newTileset).attribution;
 
-            tileset = newTileset;
+            tileset = *newTileset;
             loaded = true;
 
             observer->onSourceLoaded(base);
diff --git a/src/mbgl/style/tile_source_impl.hpp b/src/mbgl/style/tile_source_impl.hpp
index 405408ff54..0e5a53add7 100644
--- a/src/mbgl/style/tile_source_impl.hpp
+++ b/src/mbgl/style/tile_source_impl.hpp
@@ -17,8 +17,6 @@ namespace style {
 */
 class TileSourceImpl : public Source::Impl {
 public:
-    static Tileset parseTileJSON(const std::string& json, const std::string& sourceURL, SourceType, uint16_t tileSize);
-
     TileSourceImpl(SourceType, std::string id, Source&,
                    variant<std::string, Tileset> urlOrTileset,
                    uint16_t tileSize);
diff --git a/src/mbgl/util/mapbox.cpp b/src/mbgl/util/mapbox.cpp
index c03cc2a8e7..8cbc85d492 100644
--- a/src/mbgl/util/mapbox.cpp
+++ b/src/mbgl/util/mapbox.cpp
@@ -2,6 +2,7 @@
 #include <mbgl/util/constants.hpp>
 #include <mbgl/util/logging.hpp>
 #include <mbgl/util/url.hpp>
+#include <mbgl/util/tileset.hpp>
 
 #include <stdexcept>
 #include <vector>
@@ -170,6 +171,15 @@ canonicalizeTileURL(const std::string& str, const SourceType type, const uint16_
     return result;
 }
 
+void canonicalizeTileset(Tileset& tileset, const std::string& sourceURL, SourceType type, uint16_t tileSize) {
+    // TODO: Remove this hack by delivering proper URLs in the TileJSON to begin with.
+    if (isMapboxURL(sourceURL)) {
+        for (auto& url : tileset.tiles) {
+            url = canonicalizeTileURL(url, type, tileSize);
+        }
+    }
+}
+
 const uint64_t DEFAULT_OFFLINE_TILE_COUNT_LIMIT = 6000;
 
 } // end namespace mapbox
diff --git a/src/mbgl/util/mapbox.hpp b/src/mbgl/util/mapbox.hpp
index 72f4e0a567..f3dfdd0b01 100644
--- a/src/mbgl/util/mapbox.hpp
+++ b/src/mbgl/util/mapbox.hpp
@@ -4,6 +4,9 @@
 #include <mbgl/style/types.hpp>
 
 namespace mbgl {
+
+class Tileset;
+
 namespace util {
 namespace mapbox {
 
@@ -18,6 +21,9 @@ std::string normalizeTileURL(const std::string& baseURL, const std::string& url,
 // Return a "mapbox://tiles/..." URL (suitable for normalizeTileURL) for the given Mapbox tile URL.
 std::string canonicalizeTileURL(const std::string& url, SourceType, uint16_t tileSize);
 
+// Replace URL templates with "mapbox://tiles/..." URLs (suitable for normalizeTileURL).
+void canonicalizeTileset(Tileset&, const std::string& url, SourceType, uint16_t tileSize);
+
 extern const uint64_t DEFAULT_OFFLINE_TILE_COUNT_LIMIT;
 
 } // namespace mapbox