ソースを参照

refactor: change load_stream and associated functions to pass error messages up the stream to be handled

Sam Jaffe 3 週間 前
コミット
544f627cb8

+ 7 - 2
include/jvalidate/adapter.h

@@ -385,9 +385,14 @@ public:
   }
 };
 
-template <typename JSON> bool load_file(std::filesystem::path const & path, JSON & out) {
+template <typename JSON>
+bool load_file(std::filesystem::path const & path, JSON & out, std::string & error) noexcept {
   std::ifstream in(path);
-  return load_stream(in, out);
+  if (in.bad()) {
+    error = "file error";
+    return false;
+  }
+  return load_stream(in, out, error);
 }
 }
 

+ 3 - 5
include/jvalidate/adapters/jsoncpp.h

@@ -11,12 +11,10 @@
 #include <jvalidate/enum.h>
 
 namespace jvalidate::adapter {
-template <> inline bool load_stream(std::istream & in, Json::Value & out) {
+template <>
+inline bool load_stream(std::istream & in, Json::Value & out, std::string & error) noexcept {
   Json::CharReaderBuilder builder;
-  std::string error;
-  bool good = Json::parseFromStream(builder, in, &out, &error);
-  // EXPECT_M(good, "Failed to load JSON: " << error);
-  return good;
+  return Json::parseFromStream(builder, in, &out, &error);
 }
 
 template <typename JSON> class JsonCppAdapter;

+ 5 - 3
include/jvalidate/compat/curl.h

@@ -16,7 +16,8 @@ inline size_t transfer_to_buffer(char * data, size_t size, size_t nmemb, void *
   return actual_size;
 }
 
-template <typename JSON> bool curl_get(jvalidate::URI const & uri, JSON & out) {
+template <typename JSON>
+bool curl_get(jvalidate::URI const & uri, JSON & out, std::string & error) noexcept {
   using jvalidate::adapter::load_file;
   using jvalidate::adapter::load_stream;
   if (uri.scheme().starts_with("http")) {
@@ -31,13 +32,14 @@ template <typename JSON> bool curl_get(jvalidate::URI const & uri, JSON & out) {
       curl_easy_cleanup(curl);
 
       if (res == CURLE_OK) {
-        return load_stream(ss, out);
+        return load_stream(ss, out, error);
       }
     }
     return false;
   } else if (uri.scheme() == "file") {
-    return load_file(uri.resource(), out);
+    return load_file(uri.resource(), out, error);
   } else {
+    error = "unknown scheme";
     return false;
   }
 }

+ 12 - 5
include/jvalidate/detail/reference_manager.h

@@ -122,8 +122,10 @@ public:
       return it->second;
     }
 
-    std::optional<A> external = external_.try_load(schema);
-    EXPECT_M(external.has_value(), "Unable to load external meta-schema " << schema);
+    std::string error;
+    std::optional<A> external = external_.try_load(schema, error);
+    EXPECT_M(external.has_value(),
+             "Unable to load external meta-schema " << schema << ": " << error);
     EXPECT_M(external->type() == adapter::Type::Object, "meta-schema must be an object");
 
     auto metaschema = external->as_object();
@@ -185,12 +187,12 @@ public:
    * As long as ref contains a valid URI/Anchor, we will return an Adapter, even
    * if that adapter might point to a null JSON.
    */
-  std::optional<A> load(Reference const & ref, Vocabulary<A> const * vocab) {
+  std::optional<A> load(Reference const & ref, Vocabulary<A> const * vocab, std::string & error) {
     if (auto it = roots_.find(ref.root()); it != roots_.end()) {
       return ref.pointer().walk(it->second);
     }
 
-    std::optional<A> external = external_.try_load(ref.uri());
+    std::optional<A> external = external_.try_load(ref.uri(), error);
     if (not external) {
       return std::nullopt;
     }
@@ -467,7 +469,12 @@ private:
       vocab_docs.emplace(vocab.substr(n));
       vocab.replace(n, 7, "/meta/");
 
-      auto vocab_object = external_.try_load(URI(vocab));
+      std::string error;
+      auto vocab_object = external_.try_load(URI(vocab), error);
+      if (!vocab_object.has_value()) {
+        continue;
+      }
+
       auto it = vocab_object->as_object().find("properties");
       if (it == vocab_object->as_object().end()) {
         continue;

+ 2 - 2
include/jvalidate/document_cache.h

@@ -56,7 +56,7 @@ public:
 
   operator bool() const { return resolve_; }
 
-  std::optional<A> try_load(URI const & uri) {
+  std::optional<A> try_load(URI const & uri, std::string & error) {
     // Short circuit - without a URIResolver, we can always return nullopt,
     // because this library doesn't promise to know how to load external
     // schemas from any source (including files).
@@ -65,7 +65,7 @@ public:
     }
 
     auto [it, created] = cache_.try_emplace(uri);
-    if (created && not resolve_(uri, it->second)) {
+    if (created && not resolve_(uri, it->second, error)) {
       // Doing it this way skips out on a move operation for the JSON object,
       // which could be useful if someone is using a legacy JSON object type.
       // Since std::map promises stability we don't need to concern ourselves

+ 4 - 2
include/jvalidate/forward.h

@@ -33,7 +33,8 @@ template <typename> struct AdapterTraits;
 template <typename V> struct AdapterTraits<V const> : AdapterTraits<V> {};
 
 template <typename JSON> using AdapterFor = typename AdapterTraits<JSON>::template Adapter<JSON>;
-template <typename JSON> bool load_stream(std::istream & in, JSON & out);
+template <typename JSON>
+bool load_stream(std::istream & in, JSON & out, std::string & error) noexcept;
 }
 
 namespace jvalidate::schema {
@@ -187,7 +188,8 @@ template <RegexEngine RE, typename ExtensionVisitor> class ValidationVisitor;
 
 template <RegexEngine RE, typename ExtensionVisitor> class Validator;
 
-template <Adapter A> using URIResolver = bool (*)(URI const &, typename A::value_type &);
+template <Adapter A>
+using URIResolver = bool (*)(URI const &, typename A::value_type &, std::string &) noexcept;
 }
 
 namespace jvalidate::extension {

+ 6 - 4
include/jvalidate/schema.h

@@ -272,15 +272,17 @@ private:
       return *cached;
     }
 
-    if (std::optional root = context.ref.load(lexical, context.vocab)) {
+    std::string error;
+    if (std::optional root = context.ref.load(lexical, context.vocab, error)) {
       return fetch_schema(context.rebind(*root, lexical, dynamic));
     }
 
-    std::string error = "URIResolver could not resolve " + std::string(lexical.uri());
+    constexpr char const * prelude = "URIResolver could not find ";
 #ifdef JVALIDATE_LOAD_FAILURE_AS_FALSE_SCHEMA
-    return alias(dynamic, &cache_.try_emplace(dynamic, error).first->second);
+    return alias(dynamic,
+                 &cache_.try_emplace(dynamic, prelude + std::string(lexical.uri())).first->second);
 #else
-    JVALIDATE_THROW(std::runtime_error, error);
+    JVALIDATE_THROW(std::runtime_error, prelude << lexical.uri() << ": " << error);
 #endif
   }
 

+ 12 - 4
src/validate.cxx

@@ -61,7 +61,12 @@ int main(int argc, char const * const * argv) {
 
   Json::Value jschema;
   Json::Value jobject;
-  if (!load_file(args.schema, jschema) || !load_file(args.object, jobject)) {
+  if (std::string error; !load_file(args.schema, jschema, error)) {
+    std::cerr << "Error loading schema: " << error << std::endl;
+    return EXIT_FAILURE;
+  }
+  if (std::string error; !load_file(args.object, jobject, error)) {
+    std::cerr << "Error loading schema: " << error << std::endl;
     return EXIT_FAILURE;
   }
 
@@ -81,10 +86,13 @@ int main(int argc, char const * const * argv) {
     return EXIT_FAILURE;
   }
 
-  std::stringstream ss;
-  ss << result;
   Json::Value json;
-  load_stream(ss, json);
+  {
+    std::stringstream ss;
+    ss << result;
+    std::string _;
+    load_stream(ss, json, _);
+  }
 
   std::map<std::string, Json::Value> because;
   for (Json::Value const & elem : json["details"]) {

+ 6 - 4
tests/selfvalidate_test.cxx

@@ -28,13 +28,14 @@ using testing::TestWithParam;
 
 using jvalidate::adapter::load_file;
 
-bool load_external_for_test(jvalidate::URI const & uri, Json::Value & out) {
+bool load_external_for_test(jvalidate::URI const & uri, Json::Value & out,
+                            std::string & error) noexcept {
   constexpr std::string_view g_fake_url = "localhost:1234/";
   if (uri.scheme().starts_with("http") && uri.resource().starts_with(g_fake_url)) {
     std::string_view path = uri.resource().substr(g_fake_url.size());
-    return load_file(JSONSchemaTestSuiteDir() / "remotes" / path, out);
+    return load_file(JSONSchemaTestSuiteDir() / "remotes" / path, out, error);
   }
-  return jvalidate::curl_get(uri, out);
+  return jvalidate::curl_get(uri, out, error);
 }
 
 class JsonSchema : public TestWithParam<SchemaParams> {
@@ -97,7 +98,8 @@ TEST_P(JsonSchema, TestSuite) {
   auto const & [version, file] = GetParam();
   Json::Value spec;
 
-  EXPECT_TRUE(load_file(file, spec));
+  std::string error;
+  EXPECT_TRUE(load_file(file, spec, error)) << error;
 
   for (auto const & suite : spec) {
     if (skip_suite(suite["description"].asString())) {