[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.
This commit is contained in:
John Firebaugh
2017-05-05 16:31:03 -07:00
parent e34d8b5c80
commit baaae99a03
19 changed files with 130 additions and 186 deletions

View File

@@ -59,6 +59,7 @@ set(MBGL_TEST_FILES
test/src/mbgl/test/stub_file_source.hpp
test/src/mbgl/test/stub_geometry_tile_feature.hpp
test/src/mbgl/test/stub_layer_observer.hpp
test/src/mbgl/test/stub_render_source_observer.hpp
test/src/mbgl/test/stub_style_observer.hpp
test/src/mbgl/test/stub_tile_observer.hpp
test/src/mbgl/test/test.cpp
@@ -100,7 +101,6 @@ set(MBGL_TEST_FILES
test/style/style_image.test.cpp
test/style/style_layer.test.cpp
test/style/style_parser.test.cpp
test/style/tile_source.test.cpp
# text
test/text/glyph_atlas.test.cpp

View File

@@ -1,14 +1,22 @@
#pragma once
#include <mbgl/style/conversion.hpp>
#include <mbgl/style/sources/geojson_source.hpp>
#include <mbgl/util/geojson.hpp>
namespace mbgl {
namespace style {
namespace conversion {
template <class V>
optional<GeoJSON> convertGeoJSON(const V& value, Error& error);
template <>
struct Converter<GeoJSON> {
public:
optional<GeoJSON> operator()(const std::string&, Error&) const;
// This is explicitly specialized in the .cpp file for JSValue. It may also be explicitly
// specialized for SDK-specific types (e.g. mbgl::android::Value).
template <class V>
optional<GeoJSON> operator()(const V&, Error&) const;
};
} // namespace conversion
} // namespace style

View File

@@ -122,7 +122,7 @@ private:
auto result = std::make_unique<GeoJSONSource>(id, *options);
if (isObject(*dataValue)) {
optional<GeoJSON> geoJSON = convertGeoJSON(*dataValue, error);
optional<GeoJSON> geoJSON = convert<GeoJSON>(*dataValue, error);
if (!geoJSON) {
return {};
}

View File

@@ -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

View File

@@ -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));
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;
requiredSourceURLs.erase(url);
if (requiredSourceURLs.empty()) {
status.requiredResourceCountIsPrecise = true;
}
}
});
}

View File

@@ -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 {};
}

View File

@@ -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);
}

View File

@@ -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

View File

@@ -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 {
optional<GeoJSON> Converter<GeoJSON>::operator()(const std::string& value, Error& error) const {
return convertJSON<GeoJSON>(value, error);
}
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() };
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 {};
}
optional<GeoJSON> geoJSON = conversion::convertGeoJSON<JSValue>(d, error);
if (!geoJSON) {
error = { error.message };
return {};
}
return geoJSON;
}
} // namespace conversion

View File

@@ -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

View File

@@ -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());

View File

@@ -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);

View File

@@ -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);

View File

@@ -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

View File

@@ -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

View File

@@ -1,8 +0,0 @@
{
"minzoom": 0,
"maxzoom": 15,
"center": [ 1, 2, 3 ],
"bounds": [ 4, 5, 6, 7 ],
"attribution": "attribution",
"tiles": [ "http://a.tiles.mapbox.com/v4/mapbox.satellite/{z}/{x}/{y}.png?access_token=key" ]
}

View File

@@ -1,8 +0,0 @@
{
"minzoom": 0,
"maxzoom": 15,
"center": [ 1, 2, 3 ],
"bounds": [ 4, 5, 6, 7 ],
"attribution": "attribution",
"tiles": [ "http://a.tiles.mapbox.com/v4/mapbox.streets/{z}/{x}/{y}.vector.pbf?access_token=key" ]
}

View File

@@ -1,37 +0,0 @@
#include <mbgl/test/util.hpp>
#include <mbgl/style/tile_source_impl.hpp>
#include <mbgl/util/io.hpp>
using namespace mbgl;
using namespace mbgl::style;
TEST(TileSourceImpl, ParseTileJSONRaster) {
auto result = TileSourceImpl::parseTileJSON(
util::read_file("test/fixtures/style_parser/tilejson.raster.json"),
"mapbox://mapbox.satellite",
SourceType::Raster,
256);
EXPECT_EQ(0, result.zoomRange.min);
EXPECT_EQ(15, result.zoomRange.max);
EXPECT_EQ("attribution", result.attribution);
#if !defined(__ANDROID__) && !defined(__APPLE__)
EXPECT_EQ("mapbox://tiles/mapbox.satellite/{z}/{x}/{y}{ratio}.webp", result.tiles[0]);
#else
EXPECT_EQ("mapbox://tiles/mapbox.satellite/{z}/{x}/{y}{ratio}.png", result.tiles[0]);
#endif
}
TEST(TileSourceImpl, ParseTileJSONVector) {
auto result = TileSourceImpl::parseTileJSON(
util::read_file("test/fixtures/style_parser/tilejson.vector.json"),
"mapbox://mapbox.streets",
SourceType::Vector,
256);
EXPECT_EQ(0, result.zoomRange.min);
EXPECT_EQ(15, result.zoomRange.max);
EXPECT_EQ("attribution", result.attribution);
EXPECT_EQ("mapbox://tiles/mapbox.streets/{z}/{x}/{y}.vector.pbf", result.tiles[0]);
}

View File

@@ -3,6 +3,7 @@
#include <mbgl/util/logging.hpp>
#include <mbgl/util/mapbox.hpp>
#include <mbgl/util/constants.hpp>
#include <mbgl/util/tileset.hpp>
#include <stdexcept>
using namespace mbgl;
@@ -210,3 +211,29 @@ TEST(Mapbox, CanonicalURL) {
"http://api.mapbox.com/v4/a.b/{z}/{x}/{y}/.",
mbgl::util::mapbox::canonicalizeTileURL("http://api.mapbox.com/v4/a.b/{z}/{x}/{y}/.", SourceType::Raster, 256));
}
TEST(Mapbox, CanonicalizeRasterTileset) {
mbgl::Tileset tileset;
tileset.tiles = {
"http://a.tiles.mapbox.com/v4/mapbox.satellite/{z}/{x}/{y}.png?access_token=key"
};
mbgl::util::mapbox::canonicalizeTileset(tileset, "mapbox://mapbox.satellite", SourceType::Raster, 256);
#if !defined(__ANDROID__) && !defined(__APPLE__)
EXPECT_EQ("mapbox://tiles/mapbox.satellite/{z}/{x}/{y}{ratio}.webp", tileset.tiles[0]);
#else
EXPECT_EQ("mapbox://tiles/mapbox.satellite/{z}/{x}/{y}{ratio}.png", tileset.tiles[0]);
#endif
}
TEST(Mapbox, CanonicalizeVectorTileset) {
mbgl::Tileset tileset;
tileset.tiles = {
"http://a.tiles.mapbox.com/v4/mapbox.streets/{z}/{x}/{y}.vector.pbf?access_token=key"
};
mbgl::util::mapbox::canonicalizeTileset(tileset, "mapbox://mapbox.streets", SourceType::Vector, 512);
EXPECT_EQ("mapbox://tiles/mapbox.streets/{z}/{x}/{y}.vector.pbf", tileset.tiles[0]);
}