Update PropertiesConstraint class to use custom allocator, with better encapsulation

examples/custom_schema.cpp

@@ -90,7 +90,7 @@ using valijson::constraints::TypeConstraint;
 void addPropertiesConstraint(Schema &schema)
 {
 
-    PropertiesConstraint::PropertySchemaMap propertySchemaMap;
+    PropertiesConstraint propertiesConstraint;
 
     {
         // Prepare an enum constraint requires a document to be equal to at
@@ -106,7 +106,7 @@ void addPropertiesConstraint(Schema &schema)
         schema.addConstraintToSubschema(constraint, subschema);
 
         // Include subschema in properties constraint
-        propertySchemaMap["category"] = subschema;
+        propertiesConstraint.addPropertySubschema("category", subschema);
     }
 
     {
@@ -118,7 +118,7 @@ void addPropertiesConstraint(Schema &schema)
         schema.addConstraintToSubschema(typeConstraint, subschema);
 
         // Include subschema in properties constraint
-        propertySchemaMap["description"] = subschema;
+        propertiesConstraint.addPropertySubschema("description", subschema);
     }
 
     {
@@ -131,7 +131,7 @@ void addPropertiesConstraint(Schema &schema)
         schema.addConstraintToSubschema(typeConstraint, subschema);
 
         // Include subschema in properties constraint
-        propertySchemaMap["price"] = subschema;
+        propertiesConstraint.addPropertySubschema("price", subschema);
     }
 
     {
@@ -145,12 +145,11 @@ void addPropertiesConstraint(Schema &schema)
         schema.addConstraintToSubschema(typeConstraint, subschema);
 
         // Include subschema in properties constraint
-        propertySchemaMap["title"] = subschema;
+        propertiesConstraint.addPropertySubschema("title", subschema);
     }
 
-    // Add a PropertiesConstraint to the schema, with the properties defined
+    // Add a PropertiesConstraint to the root schema
-    // above, no pattern properties or additional property schemas
+    schema.addConstraint(propertiesConstraint);
-    schema.addConstraint(PropertiesConstraint(propertySchemaMap));
 }
 
 void addRequiredConstraint(Schema &schema)

include/valijson/constraints/concrete_constraints.hpp

@@ -558,43 +558,97 @@ private:
 };
 
 /**
- * @brief   Represents a tuple of 'properties', 'patternProperties' and
+ * @brief   Represents a combination of 'properties', 'patternProperties' and
- *          'additionalProperties' constraints.
+ *          'additionalProperties' constraints
  */
-struct PropertiesConstraint: BasicConstraint<PropertiesConstraint> {
+class PropertiesConstraint: public BasicConstraint<PropertiesConstraint>
-
+{
-    typedef std::map<std::string, const Subschema *> PropertySchemaMap;
+public:
-
+    PropertiesConstraint()
-    PropertiesConstraint(const PropertySchemaMap &properties)
+      : properties(std::less<String>(), allocator),
-      : properties(properties),
+        patternProperties(std::less<String>(), allocator),
         additionalProperties(NULL) { }
 
-    PropertiesConstraint(const PropertySchemaMap &properties,
+    PropertiesConstraint(CustomAlloc allocFn, CustomFree freeFn)
-                         const PropertySchemaMap &patternProperties)
+      : BasicConstraint(allocFn, freeFn),
-      : properties(properties),
+        properties(std::less<String>(), allocator),
-        patternProperties(patternProperties),
+        patternProperties(std::less<String>(), allocator),
         additionalProperties(NULL) { }
 
-    PropertiesConstraint(const PropertySchemaMap &properties,
+    bool addPatternPropertySubschema(const char *patternProperty,
-                         const PropertySchemaMap &patternProperties,
+            const Subschema *subschema)
-                         const Subschema *additionalProperties)
+    {
-      : properties(properties),
+        return patternProperties.insert(PropertySchemaMap::value_type(
-        patternProperties(patternProperties),
+                String(patternProperty, allocator), subschema)).second;
-        additionalProperties(additionalProperties) { }
+    }
 
-    PropertiesConstraint(const PropertiesConstraint &other)
+    template<typename AllocatorType>
-      : properties(other.properties),
+    bool addPatternPropertySubschema(const std::basic_string<char,
-        patternProperties(other.patternProperties),
+            std::char_traits<char>, AllocatorType> &patternProperty,
-        additionalProperties(other.additionalProperties) {}
+            const Subschema *subschema)
+    {
+        return addPatternPropertySubschema(patternProperty.c_str(), subschema);
+    }
+
+    bool addPropertySubschema(const char *propertyName,
+            const Subschema *subschema)
+    {
+        return properties.insert(PropertySchemaMap::value_type(
+                String(propertyName, allocator), subschema)).second;
+    }
+
+    template<typename AllocatorType>
+    bool addPropertySubschema(const std::basic_string<char,
+            std::char_traits<char>, AllocatorType> &propertyName,
+            const Subschema *subschema)
+    {
+        return addPropertySubschema(propertyName.c_str(), subschema);
+    }
+
+    template<typename FunctorType>
+    void applyToPatternProperties(const FunctorType &fn) const
+    {
+        typedef typename PropertySchemaMap::value_type ValueType;
+        BOOST_FOREACH( const ValueType &value, patternProperties ) {
+            if (!fn(value.first, value.second)) {
+                return;
+            }
+        }
+    }
+
+    template<typename FunctorType>
+    void applyToProperties(const FunctorType &fn) const
+    {
+        typedef typename PropertySchemaMap::value_type ValueType;
+        BOOST_FOREACH( const ValueType &value, properties ) {
+            if (!fn(value.first, value.second)) {
+                return;
+            }
+        }
+    }
+
+    const Subschema * getAdditionalPropertiesSubschema() const
+    {
+        return additionalProperties;
+    }
+
+    void setAdditionalPropertiesSubschema(const Subschema *subschema)
+    {
+        additionalProperties = subschema;
+    }
+
+private:
+    typedef std::map<String, const Subschema *, std::less<String>, Allocator>
+            PropertySchemaMap;
+
+    PropertySchemaMap properties;
+    PropertySchemaMap patternProperties;
 
-    const PropertySchemaMap properties;
-    const PropertySchemaMap patternProperties;
     const Subschema *additionalProperties;
-
 };
 
 /**
- * @brief   Represents a 'required' constraint.
+ * @brief   Represents a 'required' constraint
  */
 class RequiredConstraint: public BasicConstraint<RequiredConstraint>
 {

include/valijson/constraints/constraint_visitor.hpp

@@ -17,7 +17,6 @@ struct MinLengthConstraint;
 struct MinPropertiesConstraint;
 struct MultipleOfConstraint;
 struct PatternConstraint;
-struct PropertiesConstraint;
 
 class AllOfConstraint;
 class AnyOfConstraint;
@@ -26,6 +25,7 @@ class EnumConstraint;
 class LinearItemsConstraint;
 class NotConstraint;
 class OneOfConstraint;
+class PropertiesConstraint;
 class RequiredConstraint;
 class SingularItemsConstraint;
 class TypeConstraint;

include/valijson/schema_parser.hpp

@@ -1273,42 +1273,37 @@ private:
         const Subschema *parentSubschema)
     {
         typedef typename AdapterType::ObjectMember Member;
-        typedef constraints::PropertiesConstraint::PropertySchemaMap PSM;
 
-        // Populate a PropertySchemaMap for each of the properties defined by
+        constraints::PropertiesConstraint constraint;
-        // the 'properties' keyword.
+
-        PSM propertySchemas;
+        // Create subschemas for 'properties' constraint
         if (properties) {
             BOOST_FOREACH( const Member m, properties->getObject() ) {
-                const std::string &propertyName = m.first;
+                const std::string &property = m.first;
-                const std::string childPath = propertiesPath + "/" +
+                const std::string childPath = propertiesPath + "/" + property;
-                        propertyName;
+                const Subschema *subschema = rootSchema.createSubschema();
-                const Subschema *childSubschema = rootSchema.createSubschema();
+                constraint.addPropertySubschema(property, subschema);
-                propertySchemas[propertyName] = childSubschema;
                 populateSchema<AdapterType>(rootSchema, rootNode, m.second,
-                        *childSubschema, currentScope, childPath, fetchDoc,
+                        *subschema, currentScope, childPath, fetchDoc,
-                        parentSubschema, &propertyName);
+                        parentSubschema, &property);
             }
         }
 
-        // Populate a PropertySchemaMap for each of the properties defined by
+        // Create subschemas for 'patternProperties' constraint
-        // the 'patternProperties' keyword
-        PSM patternPropertySchemas;
         if (patternProperties) {
             BOOST_FOREACH( const Member m, patternProperties->getObject() ) {
-                const std::string &propertyName = m.first;
+                const std::string &pattern = m.first;
                 const std::string childPath = patternPropertiesPath + "/" +
-                        propertyName;
+                        pattern;
-                const Subschema *childSubschema = rootSchema.createSubschema();
+                const Subschema *subschema = rootSchema.createSubschema();
-                patternPropertySchemas[propertyName] = childSubschema;
+                constraint.addPatternPropertySubschema(pattern, subschema);
                 populateSchema<AdapterType>(rootSchema, rootNode, m.second,
-                        *childSubschema, currentScope, childPath, fetchDoc,
+                        *subschema, currentScope, childPath, fetchDoc,
-                        parentSubschema, &propertyName);
+                        parentSubschema, &pattern);
             }
         }
 
-        // Populate an additionalItems schema if required
+        // Create an additionalItems subschema if required
-        const Subschema *additionalPropertiesSchema = NULL;
         if (additionalProperties) {
             // If additionalProperties has been set, check for a boolean value.
             // Setting 'additionalProperties' to true allows the values of
@@ -1322,15 +1317,17 @@ private:
                 // If it has a boolean value that is 'true', then an empty
                 // schema should be used.
                 if (additionalProperties->asBool()) {
-                    additionalPropertiesSchema = rootSchema.createSubschema();
+                    constraint.setAdditionalPropertiesSubschema(
+                            rootSchema.emptySubschema());
                 }
             } else if (additionalProperties->isObject()) {
                 // If additionalProperties is an object, it should be used as
                 // a child schema.
-                additionalPropertiesSchema = rootSchema.createSubschema();
+                const Subschema *subschema = rootSchema.createSubschema();
+                constraint.setAdditionalPropertiesSubschema(subschema);
                 populateSchema<AdapterType>(rootSchema, rootNode,
-                        *additionalProperties, *additionalPropertiesSchema,
+                        *additionalProperties, *subschema, currentScope,
-                        currentScope, additionalPropertiesPath, fetchDoc);
+                        additionalPropertiesPath, fetchDoc);
             } else {
                 // All other types are invalid
                 throw std::runtime_error(
@@ -1339,18 +1336,11 @@ private:
         } else {
             // If an additionalProperties constraint is not provided, then the
             // default value is an empty schema.
-            additionalPropertiesSchema = rootSchema.emptySubschema();
+            constraint.setAdditionalPropertiesSubschema(
+                    rootSchema.emptySubschema());
         }
 
-        if (additionalPropertiesSchema) {
+        return constraint;
-            // If an additionalProperties schema has been created, construct a
-            // new PropertiesConstraint object using that schema.
-            return constraints::PropertiesConstraint(propertySchemas,
-                    patternPropertySchemas, additionalPropertiesSchema);
-        }
-
-        return constraints::PropertiesConstraint(propertySchemas,
-                patternPropertySchemas);
     }
 
     /**

include/valijson/validation_visitor.hpp

@@ -785,13 +785,28 @@ public:
     }
 
     /**
-     * @brief   Validate against the properties, patternProperties, and
+     * @brief   Validate a value against a PropertiesConstraint
-     *          additionalProperties constraints represented by a
-     *          PatternConstraint object.
      *
-     * @param   constraint  Constraint that the target must validate against.
+     * Validation of an object against a PropertiesConstraint proceeds in three
+     * stages. The first stage finds all properties in the object that have a
+     * corresponding subschema in the constraint, and validates those properties
+     * recursively.
      *
-     * @return  true if the constraint is satisfied, false otherwise.
+     * Next, the object's properties will be validated against the subschemas
+     * for any 'patternProperties' that match a given property name. A property
+     * is required to validate against the sub-schema for all patterns that it
+     * matches.
+     *
+     * Finally, any properties that have not yet been validated against at least
+     * one subschema will be validated against the 'additionalItems' subschema.
+     * If this subschema is not present, then all properties must have been
+     * validated at least once.
+     *
+     * Non-object values are always considered valid.
+     *
+     * @param   constraint  Constraint that the target must validate against
+     *
+     * @return  \c true if the constraint is satisfied; \c false otherwise
      */
     virtual bool visit(const PropertiesConstraint &constraint)
     {
@@ -801,83 +816,53 @@ public:
 
         bool validated = true;
 
-        const typename AdapterType::Object obj = target.asObject();
+        // Track which properties have already been validated
+        std::set<std::string> propertiesMatched;
 
-        // Validate each property in the target object
+        // Validate properties against subschemas for matching 'properties'
-        BOOST_FOREACH( const typename AdapterType::ObjectMember m, obj ) {
+        // constraints
+        const typename AdapterType::Object object = target.asObject();
+        constraint.applyToProperties(ValidatePropertySubschemas(object, context,
+                true, false, true, strictTypes, results, &propertiesMatched,
+                &validated));
 
-            const std::string propertyName = m.first;
+        // Exit early if validation failed, and we're not collecting exhaustive
-            bool propertyNameMatched = false;
+        // validation results
+        if (!validated && !results) {
+            return false;
+        }
 
-            std::vector<std::string> newContext = context;
+        // Validate properties against subschemas for matching patternProperties
-            newContext.push_back("[\"" + m.first + "\"]");
+        // constraints
+        constraint.applyToPatternProperties(ValidatePatternPropertySubschemas(
+                object, context, true, false, true, strictTypes, results,
+                &propertiesMatched, &validated));
 
-            ValidationVisitor<AdapterType> v(m.second,
+        // Validate against additionalProperties subschema for any properties
-                newContext, strictTypes, results);
+        // that have not yet been matched
+        const Subschema *additionalPropertiesSubschema =
+                constraint.getAdditionalPropertiesSubschema();
+        if (!additionalPropertiesSubschema) {
+            return propertiesMatched.size() == target.getObjectSize();
+        }
 
-            // Search for matching property name
+        BOOST_FOREACH( const typename AdapterType::ObjectMember m, object ) {
-            PropertiesConstraint::PropertySchemaMap::const_iterator itr =
+            if (propertiesMatched.find(m.first) == propertiesMatched.end()) {
-                constraint.properties.find(propertyName);
+                // Update context
-            if (itr != constraint.properties.end()) {
+                std::vector<std::string> newContext = context;
-                propertyNameMatched = true;
+                newContext.push_back("[" + m.first + "]");
-                if (!v.validateSchema(*itr->second)) {
+
+                // Create a validator to validate the property's value
+                ValidationVisitor validator(m.second, newContext, strictTypes,
+                        results);
+                if (!validator.validateSchema(*additionalPropertiesSubschema)) {
                     if (results) {
-                        results->pushError(context,
+                        results->pushError(context, "Failed to validate "
-                            "Failed to validate against schema associated with property name '" +
+                                "against additional properties schema");
-                            propertyName + "' in properties constraint.");
-                        validated = false;
-                    } else {
-                        return false;
                     }
-                }
-            }
 
-            // Search for a regex that matches the property name
-            for (itr = constraint.patternProperties.begin(); itr != constraint.patternProperties.end(); ++itr) {
-                const boost::regex r(itr->first, boost::regex::perl);
-                if (boost::regex_search(propertyName, r)) {
-                    propertyNameMatched = true;
-                    // Check schema
-                    if (!v.validateSchema(*itr->second)) {
-                        if (results) {
-                            results->pushError(context,
-                                "Failed to validate against schema associated with regex '" +
-                                itr->first + "' in patternProperties constraint.");
-                            validated = false;
-                        } else {
-                            return false;
-                        }
-                    }
-                }
-            }
-
-            // If the property name has been matched by a name in 'properties'
-            // or a regex in 'patternProperties', then it should not be
-            // validated against the 'additionalPatterns' schema.
-            if (propertyNameMatched) {
-                continue;
-            }
-
-            // If an additionalProperties schema has been provided, the values
-            // associated with unmatched property names should be validated
-            // against that schema.
-            if (constraint.additionalProperties) {
-                if (v.validateSchema(*constraint.additionalProperties)) {
-                    continue;
-                } else if (results) {
-                    results->pushError(context, "Failed to validate property '" +
-                        propertyName + "' against schema in additionalProperties constraint.");
                     validated = false;
-                } else {
-                    return false;
                 }
-            } else if (results) {
-                results->pushError(context, "Failed to match property name '" +
-                        propertyName + "' to any names in 'properties' or "
-                        "regexes in 'patternProperties'");
-                validated = false;
-            } else {
-                return false;
             }
         }
 
@@ -1351,6 +1336,179 @@ private:
         bool * const validated;
     };
 
+    /**
+     * @brief  Functor to validate object properties against sub-schemas
+     *         defined by a 'patternProperties' constraint
+     */
+    struct ValidatePatternPropertySubschemas
+    {
+        ValidatePatternPropertySubschemas(
+                const typename AdapterType::Object &object,
+                const std::vector<std::string> &context,
+                bool continueOnSuccess,
+                bool continueOnFailure,
+                bool continueIfUnmatched,
+                bool strictTypes,
+                ValidationResults *results,
+                std::set<std::string> *propertiesMatched,
+                bool *validated)
+          : object(object),
+            context(context),
+            continueOnSuccess(continueOnSuccess),
+            continueOnFailure(continueOnFailure),
+            continueIfUnmatched(continueIfUnmatched),
+            strictTypes(strictTypes),
+            results(results),
+            propertiesMatched(propertiesMatched),
+            validated(validated) { }
+
+        template<typename StringType>
+        bool operator()(const StringType &patternProperty,
+                const Subschema *subschema) const
+        {
+            const std::string patternPropertyStr(patternProperty.c_str());
+
+            // It would be nice to store pre-allocated regex objects in the
+            // PropertiesConstraint, but boost::regex does not currently support
+            // custom allocators. This isn't an issue here, because Valijson's
+            // JSON Scheme validator does not yet support custom allocators.
+            const boost::regex r(patternPropertyStr, boost::regex::perl);
+
+            bool matchFound = false;
+
+            // Recursively validate all matching properties
+            typedef const typename AdapterType::ObjectMember ObjectMember;
+            BOOST_FOREACH( const ObjectMember m, object ) {
+                if (boost::regex_search(m.first, r)) {
+                    matchFound = true;
+                    if (propertiesMatched) {
+                        propertiesMatched->insert(m.first);
+                    }
+
+                    // Update context
+                    std::vector<std::string> newContext = context;
+                    newContext.push_back("[" + m.first + "]");
+
+                    // Recursively validate property's value
+                    ValidationVisitor validator(m.second, newContext,
+                            strictTypes, results);
+                    if (validator.validateSchema(*subschema)) {
+                        continue;
+                    }
+
+                    if (results) {
+                        results->pushError(context, "Failed to validate "
+                                "against schema associated with pattern '" +
+                                patternPropertyStr + "'.");
+                    }
+
+                    if (validated) {
+                        *validated = false;
+                    }
+
+                    if (!continueOnFailure) {
+                        return false;
+                    }
+                }
+            }
+
+            // Allow iteration to terminate if there was not at least one match
+            if (!matchFound && !continueIfUnmatched) {
+                return false;
+            }
+
+            return continueOnSuccess;
+        }
+
+    private:
+        const typename AdapterType::Object &object;
+        const std::vector<std::string> &context;
+        const bool continueOnSuccess;
+        const bool continueOnFailure;
+        const bool continueIfUnmatched;
+        const bool strictTypes;
+        ValidationResults * const results;
+        std::set<std::string> * const propertiesMatched;
+        bool * const validated;
+    };
+
+    /**
+     * @brief  Functor to validate object properties against sub-schemas defined
+     *         by a 'properties' constraint
+     */
+    struct ValidatePropertySubschemas
+    {
+        ValidatePropertySubschemas(
+                const typename AdapterType::Object &object,
+                const std::vector<std::string> &context,
+                bool continueOnSuccess,
+                bool continueOnFailure,
+                bool continueIfUnmatched,
+                bool strictTypes,
+                ValidationResults *results,
+                std::set<std::string> *propertiesMatched,
+                bool *validated)
+          : object(object),
+            context(context),
+            continueOnSuccess(continueOnSuccess),
+            continueOnFailure(continueOnFailure),
+            continueIfUnmatched(continueIfUnmatched),
+            strictTypes(strictTypes),
+            results(results),
+            propertiesMatched(propertiesMatched),
+            validated(validated) { }
+
+        template<typename StringType>
+        bool operator()(const StringType &propertyName,
+                const Subschema *subschema) const
+        {
+            const std::string propertyNameKey(propertyName.c_str());
+            const typename AdapterType::Object::const_iterator itr =
+                    object.find(propertyNameKey);
+            if (itr == object.end()) {
+                return continueIfUnmatched;
+            }
+
+            if (propertiesMatched) {
+                propertiesMatched->insert(propertyNameKey);
+            }
+
+            // Update context
+            std::vector<std::string> newContext = context;
+            newContext.push_back("[" + propertyNameKey + "]");
+
+            // Recursively validate property's value
+            ValidationVisitor validator(itr->second, newContext, strictTypes,
+                    results);
+            if (validator.validateSchema(*subschema)) {
+                return continueOnSuccess;
+            }
+
+            if (results) {
+                results->pushError(context, "Failed to validate against "
+                        "schema associated with property name '" +
+                        propertyNameKey + "'.");
+            }
+
+            if (validated) {
+                *validated = false;
+            }
+
+            return continueOnFailure;
+        }
+
+    private:
+        const typename AdapterType::Object &object;
+        const std::vector<std::string> &context;
+        const bool continueOnSuccess;
+        const bool continueOnFailure;
+        const bool continueIfUnmatched;
+        const bool strictTypes;
+        ValidationResults * const results;
+        std::set<std::string> * const propertiesMatched;
+        bool * const validated;
+    };
+
     /**
      * @brief  Functor to validate schema-based dependencies
      */