[all] Replace Result<T> with optional<T> plus out Error parameter

7 files changed, 56 insertions, 37 deletions

diff --git a/platform/android/src/style/conversion/filter.hpp b/platform/android/src/style/conversion/filter.hpp
index fc36d3a044..1f0abcf3a4 100644
--- a/platform/android/src/style/conversion/filter.hpp
+++ b/platform/android/src/style/conversion/filter.hpp
@@ -17,9 +17,10 @@ inline optional<mbgl::style::Filter> toFilter(jni::JNIEnv& env, jni::Array<jni::
     mbgl::optional<mbgl::style::Filter> filter;
     if (jfilter) {
       Value filterValue(env, jfilter);
-      auto converted = mbgl::style::conversion::convert<mbgl::style::Filter>(filterValue);
+      mbgl::style::conversion::Error error;
+      auto converted = mbgl::style::conversion::convert<mbgl::style::Filter>(filterValue, error);
       if (!converted) {
-          mbgl::Log::Error(mbgl::Event::JNI, "Error converting filter: " + converted.error().message);
+          mbgl::Log::Error(mbgl::Event::JNI, "Error converting filter: " + error.message);
       }
       filter = std::move(*converted);
     }
diff --git a/platform/android/src/style/conversion/geojson.hpp b/platform/android/src/style/conversion/geojson.hpp
index 415d96f467..6ac6abcd7c 100644
--- a/platform/android/src/style/conversion/geojson.hpp
+++ b/platform/android/src/style/conversion/geojson.hpp
@@ -17,12 +17,13 @@ namespace style {
 namespace conversion {
 
 template <>
-Result<GeoJSON> convertGeoJSON(const mbgl::android::Value& value) {
+optional<GeoJSON> convertGeoJSON(const mbgl::android::Value& value, Error& error) {
 
     // Value should be a string wrapped in an object
     mbgl::android::Value jsonValue = value.get("data");
     if(value.isNull()) {
-        return Error { "no json data found" };
+        error = { "no json data found" };
+        return {};
     }
     std::string jsonString = value.get("data").toString();
 
@@ -32,12 +33,14 @@ Result<GeoJSON> convertGeoJSON(const mbgl::android::Value& value) {
     if (d.HasParseError()) {
         std::stringstream message;
         message << d.GetErrorOffset() << " - " << rapidjson::GetParseError_En(d.GetParseError());
-        return Error { message.str() };
+        error = { message.str() };
+        return {};
     }
 
-    conversion::Result<GeoJSON> geoJSON = conversion::convertGeoJSON<JSValue>(d);
+    optional<GeoJSON> geoJSON = conversion::convertGeoJSON<JSValue>(d, error);
     if (!geoJSON) {
-        return Error { geoJSON.error().message };
+        error = { error.message };
+        return {};
     }
 
     return geoJSON;
@@ -46,8 +49,8 @@ Result<GeoJSON> convertGeoJSON(const mbgl::android::Value& value) {
 template <>
 struct Converter<GeoJSON> {
 
-    Result<GeoJSON> operator()(const mbgl::android::Value& value) const {
-        return convertGeoJSON(value);
+    optional<GeoJSON> operator()(const mbgl::android::Value& value, Error& error) const {
+        return convertGeoJSON(value, error);
     }
 
 };
diff --git a/platform/android/src/style/conversion/url_or_tileset.hpp b/platform/android/src/style/conversion/url_or_tileset.hpp
index 4e502324d0..00ef913d41 100644
--- a/platform/android/src/style/conversion/url_or_tileset.hpp
+++ b/platform/android/src/style/conversion/url_or_tileset.hpp
@@ -12,27 +12,25 @@
 #include <string>
 
 namespace mbgl {
-namespace style {
-namespace conversion {
-
-template <>
-struct Converter<variant<std::string, Tileset>> {
-
-    template <class V>
-    Result<variant<std::string, Tileset>> operator()(const V& value) const {
-        if (isObject(value)) {
-            Result<Tileset> tileset = convert<Tileset>(value);
-            if (!tileset) {
-                return tileset.error();
-            }
-            return *tileset;
-        } else {
-            return *toString(value);
+namespace android {
+
+// This conversion is expected not to fail because it's used only in contexts where
+// the value was originally a String or TileSet object on the Java side. If it fails
+// to convert, it's a bug in our serialization or Java-side static typing.
+inline variant<std::string, Tileset> convertURLOrTileset(const Value& value) {
+    using namespace mbgl::style::conversion;
+
+    if (isObject(value)) {
+        Error error;
+        optional<Tileset> tileset = convert<Tileset>(value, error);
+        if (!tileset) {
+            throw std::logic_error(error.message);
         }
+        return { *tileset };
+    } else {
+        return { *toString(value) };
     }
-
-};
-
 }
+
 }
 }
diff --git a/platform/android/src/style/layers/layer.cpp b/platform/android/src/style/layers/layer.cpp
index dbf71fd2af..6e93cf4daf 100644
--- a/platform/android/src/style/layers/layer.cpp
+++ b/platform/android/src/style/layers/layer.cpp
@@ -104,9 +104,10 @@ namespace android {
         Value wrapped(env, jfilter);
         Filter filter;
 
-        Result<Filter> converted = convert<Filter>(wrapped);
+        Error error;
+        optional<Filter> converted = convert<Filter>(wrapped, error);
         if (!converted) {
-            mbgl::Log::Error(mbgl::Event::JNI, "Error setting filter: " + converted.error().message);
+            mbgl::Log::Error(mbgl::Event::JNI, "Error setting filter: " + error.message);
             return;
         }
         filter = std::move(*converted);
diff --git a/platform/android/src/style/sources/geojson_source.cpp b/platform/android/src/style/sources/geojson_source.cpp
index 7ab98e47c0..0c2d25f9fc 100644
--- a/platform/android/src/style/sources/geojson_source.cpp
+++ b/platform/android/src/style/sources/geojson_source.cpp
@@ -9,7 +9,7 @@
 #include "../../conversion/conversion.hpp"
 #include "../../conversion/collection.hpp"
 #include "../../geometry/conversion/feature.hpp"
-#include "../conversion/url_or_tileset.hpp"
+
 #include <mbgl/style/conversion.hpp>
 #include <mbgl/style/conversion/geojson_options.hpp>
 
@@ -18,11 +18,26 @@
 namespace mbgl {
 namespace android {
 
+    // This conversion is expected not to fail because it's used only in contexts where
+    // the value was originally a GeoJsonOptions object on the Java side. If it fails
+    // to convert, it's a bug in our serialization or Java-side static typing.
+    static style::GeoJSONOptions convertGeoJSONOptions(jni::JNIEnv& env, jni::Object<> options) {
+        using namespace mbgl::style::conversion;
+        if (!options) {
+            return style::GeoJSONOptions();
+        }
+        Error error;
+        optional<style::GeoJSONOptions> result = convert<style::GeoJSONOptions>(Value(env, options), error);
+        if (!result) {
+            throw std::logic_error(error.message);
+        }
+        return *result;
+    }
+
     GeoJSONSource::GeoJSONSource(jni::JNIEnv& env, jni::String sourceId, jni::Object<> options)
         : Source(env, std::make_unique<mbgl::style::GeoJSONSource>(
                 jni::Make<std::string>(env, sourceId),
-                options ? *style::conversion::convert<style::GeoJSONOptions>(Value(env, options)) : style::GeoJSONOptions()
-                )
+                convertGeoJSONOptions(env, options))
             ) {
     }
 
@@ -36,9 +51,10 @@ namespace android {
         using namespace mbgl::style::conversion;
 
         // Convert the jni object
-        Result<GeoJSON> converted = convert<GeoJSON>(Value(env, json));
+        Error error;
+        optional<GeoJSON> converted = convert<GeoJSON>(Value(env, json), error);
         if(!converted) {
-            mbgl::Log::Error(mbgl::Event::JNI, "Error setting geo json: " + converted.error().message);
+            mbgl::Log::Error(mbgl::Event::JNI, "Error setting geo json: " + error.message);
             return;
         }
 
diff --git a/platform/android/src/style/sources/raster_source.cpp b/platform/android/src/style/sources/raster_source.cpp
index 42ac4cda99..0234901a77 100644
--- a/platform/android/src/style/sources/raster_source.cpp
+++ b/platform/android/src/style/sources/raster_source.cpp
@@ -16,7 +16,7 @@ namespace android {
             env,
             std::make_unique<mbgl::style::RasterSource>(
                 jni::Make<std::string>(env, sourceId),
-                *style::conversion::convert<variant<std::string, Tileset>>(Value(env, urlOrTileSet)),
+                convertURLOrTileset(Value(env, urlOrTileSet)),
                 tileSize
             )
         ) {
diff --git a/platform/android/src/style/sources/vector_source.cpp b/platform/android/src/style/sources/vector_source.cpp
index 67777f50d4..4852a9b84f 100644
--- a/platform/android/src/style/sources/vector_source.cpp
+++ b/platform/android/src/style/sources/vector_source.cpp
@@ -23,7 +23,7 @@ namespace android {
             env,
             std::make_unique<mbgl::style::VectorSource>(
                 jni::Make<std::string>(env, sourceId),
-                *style::conversion::convert<variant<std::string, Tileset>>(Value(env, urlOrTileSet))
+                convertURLOrTileset(Value(env, urlOrTileSet))
             )
         ) {
     }