diff --git a/include/valijson/schema_parser.hpp b/include/valijson/schema_parser.hpp index 4b96838..63a09bd 100644 --- a/include/valijson/schema_parser.hpp +++ b/include/valijson/schema_parser.hpp @@ -57,10 +57,16 @@ public: * @brief Struct to contain templated function type for fetching documents */ template - struct FetchDocumentFunction { - /// Functor for dereferencing JSON References - typedef boost::function( - const std::string &uri)> Type; + struct FunctionPtrs + { + typedef typename adapters::AdapterTraits::DocumentType + DocumentType; + + /// Templated function pointer type for fetching remote documents + typedef const AdapterType * (*FetchDoc)(const std::string &uri); + + /// Templated function pointer type for freeing fetched documents + typedef void (*FreeDoc)(const AdapterType *); }; /** @@ -79,13 +85,26 @@ public: void populateSchema( const AdapterType &node, Schema &schema, - boost::optional::Type> - fetchDoc = boost::none) + typename FunctionPtrs::FetchDoc fetchDoc = NULL, + typename FunctionPtrs::FreeDoc freeDoc = NULL) { + if ((fetchDoc == NULL) ^ (freeDoc == NULL)) { + throw std::runtime_error( + "Remote document fetching cannot be enabled without both " + "fetch and free functions"); + } + typename DocumentCache::Type docCache; SchemaCache schemaCache; - populateSchema(schema, node, node, schema, boost::none, "", fetchDoc, - NULL, NULL, docCache, schemaCache); + try { + populateSchema(schema, node, node, schema, boost::none, "", + fetchDoc, NULL, NULL, docCache, schemaCache); + } catch (...) { + freeDocumentCache(docCache, freeDoc); + throw; + } + + freeDocumentCache(docCache, freeDoc); } private: @@ -96,11 +115,31 @@ private: typedef typename adapters::AdapterTraits::DocumentType DocumentType; - typedef std::map Type; + typedef std::map Type; }; typedef std::map > SchemaCache; + /** + * @brief Free memory used by fetched documents + * + * If a custom 'free' function has not been provided, then the default + * delete operator will be used. + * + * @param docCache collection of fetched documents to free + * @param freeDoc optional custom free function + */ + template + void freeDocumentCache(const typename DocumentCache::Type + &docCache, typename FunctionPtrs::FreeDoc freeDoc) + { + typedef typename DocumentCache::Type DocCacheType; + + BOOST_FOREACH( const typename DocCacheType::value_type &v, docCache ) { + freeDoc(v.second); + } + } + /** * @brief Populate a Schema object from JSON Schema document * @@ -135,8 +174,7 @@ private: const Subschema &subschema, const boost::optional currentScope, const std::string &nodePath, - const boost::optional::Type> - fetchDoc, + const typename FunctionPtrs::FetchDoc fetchDoc, const Subschema *parentSubschema, const std::string *ownName, typename DocumentCache::Type &docCache, @@ -473,8 +511,7 @@ private: const Subschema &subschema, const boost::optional currentScope, const std::string &nodePath, - const boost::optional::Type> - fetchDoc, + const typename FunctionPtrs::FetchDoc fetchDoc, const Subschema *parentSubschema, const std::string *ownName, typename DocumentCache::Type &docCache, @@ -496,16 +533,27 @@ private: "Support for JSON References not enabled."); } - // Returns a shared pointer to the remote document that was - // retrieved, or null if retrieval failed. The resulting document - // must remain in scope until populateSchema returns. - boost::shared_ptr docPtr = - (*fetchDoc)(*documentUri); + const AdapterType * docPtr = NULL; + const typename DocumentCache::Type::const_iterator + docCacheItr = docCache.find(*documentUri); + if (docCacheItr == docCache.end()) { + // Returns a shared pointer to the remote document that was + // retrieved, or null if retrieval failed. The resulting + // document must remain in scope until populateSchema returns. + docPtr = (*fetchDoc)(*documentUri); - // Can't proceed without the remote document - if (!docPtr) { - throw std::runtime_error( - "Failed to fetch referenced schema document."); + // Can't proceed without the remote document + if (!docPtr) { + throw std::runtime_error( + "Failed to fetch referenced schema document."); + } + + // TODO: If this fails, how would the document be freed? + docCache.insert( + typename DocumentCache::Type::value_type( + *documentUri, docPtr)); + } else { + docPtr = docCacheItr->second; } const AdapterType &ref = internal::json_pointer::resolveJsonPointer( @@ -552,8 +600,7 @@ private: const AdapterType &node, const boost::optional currentScope, const std::string &nodePath, - const boost::optional::Type> - fetchDoc, + const typename FunctionPtrs::FetchDoc fetchDoc, typename DocumentCache::Type &docCache, SchemaCache &schemaCache) { @@ -610,8 +657,7 @@ private: const AdapterType &node, const boost::optional currentScope, const std::string &nodePath, - const boost::optional::Type> - fetchDoc, + const typename FunctionPtrs::FetchDoc fetchDoc, typename DocumentCache::Type &docCache, SchemaCache &schemaCache) { @@ -686,8 +732,7 @@ private: const AdapterType &node, const boost::optional currentScope, const std::string &nodePath, - const boost::optional::Type> - fetchDoc, + const typename FunctionPtrs::FetchDoc fetchDoc, typename DocumentCache::Type &docCache, SchemaCache &schemaCache) { @@ -816,8 +861,7 @@ private: const boost::optional currentScope, const std::string &itemsPath, const std::string &additionalItemsPath, - const boost::optional::Type> - fetchDoc, + const typename FunctionPtrs::FetchDoc fetchDoc, typename DocumentCache::Type &docCache, SchemaCache &schemaCache) { @@ -923,8 +967,7 @@ private: const AdapterType &items, const boost::optional currentScope, const std::string &itemsPath, - const boost::optional::Type> - fetchDoc, + const typename FunctionPtrs::FetchDoc fetchDoc, typename DocumentCache::Type &docCache, SchemaCache &schemaCache) { @@ -1260,8 +1303,7 @@ private: const AdapterType &node, const boost::optional currentScope, const std::string &nodePath, - const boost::optional::Type> - fetchDoc, + const typename FunctionPtrs::FetchDoc fetchDoc, typename DocumentCache::Type &docCache, SchemaCache &schemaCache) { @@ -1303,8 +1345,7 @@ private: const AdapterType &node, const boost::optional currentScope, const std::string &nodePath, - const boost::optional::Type> - fetchDoc, + const typename FunctionPtrs::FetchDoc fetchDoc, typename DocumentCache::Type &docCache, SchemaCache &schemaCache) { @@ -1390,8 +1431,7 @@ private: const std::string &propertiesPath, const std::string &patternPropertiesPath, const std::string &additionalPropertiesPath, - const boost::optional::Type> - fetchDoc, + const typename FunctionPtrs::FetchDoc fetchDoc, const Subschema *parentSubschema, typename DocumentCache::Type &docCache, SchemaCache &schemaCache) @@ -1551,8 +1591,7 @@ private: const AdapterType &node, const boost::optional currentScope, const std::string &nodePath, - const boost::optional::Type> - fetchDoc, + const typename FunctionPtrs::FetchDoc fetchDoc, typename DocumentCache::Type &docCache, SchemaCache &schemaCache) { diff --git a/tests/test_fetch_document_callback.cpp b/tests/test_fetch_document_callback.cpp index 7b489f3..6f636e2 100644 --- a/tests/test_fetch_document_callback.cpp +++ b/tests/test_fetch_document_callback.cpp @@ -13,9 +13,6 @@ using valijson::SchemaParser; using valijson::adapters::RapidJsonAdapter; using valijson::Validator; -typedef SchemaParser::FetchDocumentFunction::Type - FetchDocumentFunction; - namespace { static rapidjson::MemoryPoolAllocator allocator; @@ -29,7 +26,7 @@ class TestFetchDocumentCallback : public ::testing::Test }; -boost::shared_ptr fetchDocument(const std::string &uri) +const RapidJsonAdapter * fetchDocument(const std::string &uri) { EXPECT_STREQ("test", uri.c_str()); @@ -49,7 +46,12 @@ boost::shared_ptr fetchDocument(const std::string &uri) // Have to ensure that fetchedRoot exists for at least as long as the // shared pointer that we return here - return boost::make_shared(fetchedRoot); + return new RapidJsonAdapter(fetchedRoot); +} + +void freeDocument(const RapidJsonAdapter *adapter) +{ + delete adapter; } TEST_F(TestFetchDocumentCallback, Basics) @@ -63,19 +65,21 @@ TEST_F(TestFetchDocumentCallback, Basics) // Parse schema document Schema schema; SchemaParser schemaParser; - schemaParser.populateSchema(schemaDocumentAdapter, schema, - boost::make_optional(fetchDocument)); + schemaParser.populateSchema(schemaDocumentAdapter, schema, fetchDocument, + freeDocument); // Test resulting schema with a valid document rapidjson::Document validDocument; validDocument.SetObject(); validDocument.AddMember("test", "valid", allocator); Validator validator; - EXPECT_TRUE(validator.validate(schema, RapidJsonAdapter(validDocument), NULL)); + EXPECT_TRUE(validator.validate(schema, RapidJsonAdapter(validDocument), + NULL)); // Test resulting schema with an invalid document rapidjson::Document invalidDocument; invalidDocument.SetObject(); invalidDocument.AddMember("test", 123, allocator); - EXPECT_FALSE(validator.validate(schema, RapidJsonAdapter(invalidDocument), NULL)); + EXPECT_FALSE(validator.validate(schema, RapidJsonAdapter(invalidDocument), + NULL)); }