Merge pull request #505 from apiaryio/klokane/fix-493

Fix #493 repeated item in JSON schema required

CHANGELOG.md

@@ -15,6 +15,10 @@
 - Instead of returning int, functions that may error return `drafter_error`
   type. This adds additional type-safety when handling errors.
 
+## Bug Fixes
+* Fix JSON Schema "required" for multiple defined members
+  [#493](https://github.com/apiaryio/drafter/issues/493)
+
 ## 3.2.7
 
 ## Bug Fixes

src/refract/JSONSchemaVisitor.cc

@@ -1,4 +1,5 @@
 //
+//
 //  refract/JSONSchemaVisitor.cc
 //  librefract
 //
@@ -11,11 +12,13 @@
 #include <sstream>
 #include <iostream>
 #include <map>
+#include <set>
 
 #include "RenderJSONVisitor.h"
 #include "JSONSchemaVisitor.h"
 #include "SerializeCompactVisitor.h"
 
+
 #include <assert.h>
 
 namespace refract
@@ -310,7 +313,7 @@ namespace refract
             fixedType = true;
         }
 
-        processMembers(val.begin(), val.end(), reqVals, varProps, oneOfMembers, o);
+        processMembers(val, reqVals, varProps, oneOfMembers, o);
 
         if (!varProps.empty()) {
             addVariableProps(varProps, o);
@@ -524,7 +527,7 @@ namespace refract
         RefractElements oneOfMembers;
         IncludeMembers(e, members);
 
-        processMembers(members.begin(), members.end(), reqVals, varProps, oneOfMembers, props);
+        processMembers(members, reqVals, varProps, oneOfMembers, props);
 
         pObj = new ObjectElement;
 
@@ -575,40 +578,42 @@ namespace refract
         return ss.str();
     }
 
-    void JSONSchemaVisitor::processMembers(std::vector<refract::IElement*>::const_iterator begin,
-        std::vector<refract::IElement*>::const_iterator end,
+    void JSONSchemaVisitor::processMembers(const std::vector<refract::IElement*>& members,
         ArrayElement::ValueType& reqVals,
         std::vector<MemberElement*>& varProps,
         RefractElements& oneOfMembers,
         ObjectElement* o)
     {
-        for (auto it = begin; it != end; ++it) {
-            if (!*it) {
+        std::set<std::string> required;
+
+        for (const auto& member: members) {
+            if (!member) {
                 continue;
             }
 
             TypeQueryVisitor type;
-            Visit(type, *(*it));
+            Visit(type, *member);
+
 
             switch (type.get()) {
                 case TypeQueryVisitor::Member: {
-                    MemberElement* mr = static_cast<MemberElement*>(*it);
+                    MemberElement* mr = static_cast<MemberElement*>(member);
 
-                    if (IsTypeAttribute(*(*it), "required") || IsTypeAttribute(*(*it), "fixed")
-                        || ((fixed || fixedType) && !IsTypeAttribute(*(*it), "optional"))) {
+                    if (IsTypeAttribute(*member, "required") || IsTypeAttribute(*member, "fixed")
+                        || ((fixed || fixedType) && !IsTypeAttribute(*member, "optional"))) {
 
                         StringElement* str = TypeQueryVisitor::as<StringElement>(mr->value.first);
 
                         if (str) {
-                            reqVals.push_back(IElement::Create(str->value));
+                            required.insert(str->value);
                         }
                     }
 
                     if (IsVariableProperty(*mr->value.first)) {
                         varProps.push_back(mr);
                     } else {
                         JSONSchemaVisitor renderer(pDefs, fixed);
-                        Visit(renderer, *(*it));
+                        Visit(renderer, *member);
                         ObjectElement* o1 = TypeQueryVisitor::as<ObjectElement>(renderer.get());
 
                         if (!o1->value.empty()) {
@@ -622,7 +627,7 @@ namespace refract
                 } break;
 
                 case TypeQueryVisitor::Select: {
-                    SelectElement* sel = static_cast<SelectElement*>(*it);
+                    SelectElement* sel = static_cast<SelectElement*>(member);
 
                     // FIXME: there is no valid solution for multiple "SelectElement" in one object.
 
@@ -637,5 +642,11 @@ namespace refract
                     throw LogicError("Invalid member type of object in MSON definition");
             }
         }
+
+        std::transform(required.begin(), required.end(),
+                std::back_inserter(reqVals), 
+                [](const std::string& value) {
+                    return IElement::Create(value);
+                });
     }
 }

src/refract/JSONSchemaVisitor.h

@@ -45,8 +45,7 @@ namespace refract
         template <typename T>
         void primitiveType(const T& e);
 
-        void processMembers(std::vector<refract::IElement*>::const_iterator begin,
-            std::vector<refract::IElement*>::const_iterator end,
+        void processMembers(const std::vector<refract::IElement*>& members,
             ArrayElement::ValueType& reqVals,
             std::vector<MemberElement*>& varProps,
             RefractElements& oneOfMembers,

test/fixtures/schema/array-fixed-samples-complex.json

@@ -249,7 +249,7 @@
                                   "content": "application/schema+json"
                                 }
                               },
-                              "content": "{\n  \"$schema\": \"http://json-schema.org/draft-04/schema#\",\n  \"type\": \"object\",\n  \"properties\": {\n    \"tags\": {\n      \"type\": \"array\",\n      \"items\": [\n        {\n          \"type\": \"string\",\n          \"enum\": [\n            \"hello\"\n          ]\n        },\n        {\n          \"type\": \"object\",\n          \"properties\": {\n            \"name\": {\n              \"type\": \"string\",\n              \"enum\": [\n                \"snow\"\n              ]\n            },\n            \"color\": {\n              \"type\": \"string\",\n              \"enum\": [\n                \"white\"\n              ]\n            },\n            \"description\": {\n              \"type\": \"string\",\n              \"enum\": [\n                \"\"\n              ]\n            }\n          },\n          \"required\": [\n            \"name\",\n            \"color\",\n            \"description\"\n          ],\n          \"additionalProperties\": false\n        },\n        {\n          \"type\": \"string\",\n          \"enum\": [\n            \"world\"\n          ]\n        }\n      ]\n    }\n  },\n  \"required\": [\n    \"tags\"\n  ]\n}"
+                              "content": "{\n  \"$schema\": \"http://json-schema.org/draft-04/schema#\",\n  \"type\": \"object\",\n  \"properties\": {\n    \"tags\": {\n      \"type\": \"array\",\n      \"items\": [\n        {\n          \"type\": \"string\",\n          \"enum\": [\n            \"hello\"\n          ]\n        },\n        {\n          \"type\": \"object\",\n          \"properties\": {\n            \"name\": {\n              \"type\": \"string\",\n              \"enum\": [\n                \"snow\"\n              ]\n            },\n            \"color\": {\n              \"type\": \"string\",\n              \"enum\": [\n                \"white\"\n              ]\n            },\n            \"description\": {\n              \"type\": \"string\",\n              \"enum\": [\n                \"\"\n              ]\n            }\n          },\n          \"required\": [\n            \"color\",\n            \"description\",\n            \"name\"\n          ],\n          \"additionalProperties\": false\n        },\n        {\n          \"type\": \"string\",\n          \"enum\": [\n            \"world\"\n          ]\n        }\n      ]\n    }\n  },\n  \"required\": [\n    \"tags\"\n  ]\n}"
                             }
                           ]
                         }

test/fixtures/schema/issue-493-multiple-same-required.apib

@@ -0,0 +1,11 @@
+# Data Structures
+
+## Struct (object)
++ field (required)
++ field (required)
+
+# test [GET /]
+
++ Response 200 (application/json)
+    + Attributes (object)
+        + data (required, array[Struct], fixed-type)