Fix #13: Invalid ‘warning: empty body asset’ for certain status codes

apiaryio · Jul 15, 2013 · a9bf977 · a9bf977
1 parent 0abaabd
commit a9bf977
Show file tree

Hide file tree

Showing 12 changed files with 284 additions and 76 deletions.
diff --git a/src/BlueprintParser.h b/src/BlueprintParser.h
@@ -172,7 +172,7 @@ namespace snowcrash {
                 }
                 ss << " is already defined";
 
-                result.first.warnings.push_back(Warning(ss.str(), DuplicateWarnign, begin->sourceMap));
+                result.first.warnings.push_back(Warning(ss.str(), DuplicateWarning, begin->sourceMap));
             }
 
             output.resourceGroups.push_back(resourceGroup); // FIXME: C++11 move
@@ -227,7 +227,7 @@ namespace snowcrash {
                         std::stringstream ss;
                         ss << "duplicate definition of `" << it->first << "`";
                         result.first.warnings.push_back(Warning(ss.str(),
-                                                                DuplicateWarnign,
+                                                                DuplicateWarning,
                                                                 cur->sourceMap));
                     }
                 }

diff --git a/src/BlueprintUtility.h b/src/BlueprintUtility.h
@@ -97,6 +97,30 @@ namespace snowcrash {
             return first.method == second.method;
         }
     };
+
+    /**
+     *  \brief  Find a request withing given method.
+     *  \param  method  The method to check.
+     *  \param  request A request to look for.
+     *  \return Iterator pointing to the matching request within given method requests.
+     */
+    FORCEINLINE Collection<Request>::const_iterator FindRequest(const Method& method, const Request& request) {
+        return std::find_if(method.requests.begin(),
+                            method.requests.end(),
+                            std::bind2nd(MatchPayload(), request));
+    }
+
+    /**
+     *  \brief  Find a response withing responses of a given method.
+     *  \param  method  The method to check.
+     *  \param  response A response to look for.
+     *  \return Iterator pointing to the matching response within given method requests.
+     */
+    FORCEINLINE Collection<Response>::const_iterator FindResponse(const Method& method, const Response& response) {
+        return std::find_if(method.responses.begin(),
+                            method.responses.end(),
+                            std::bind2nd(MatchPayload(), response));
+    }
 }
 
 #endif
diff --git a/src/HTTP.cc b/src/HTTP.cc
@@ -12,3 +12,16 @@ using namespace snowcrash;
 
 const std::string HTTPHeaderName::Accept = "Accept";
 const std::string HTTPHeaderName::ContentType = "Content-Type";
+
+StatusCodeTraits snowcrash::GetStatusCodeTrait(HTTPStatusCode code)
+{
+    StatusCodeTraits traits;
+    traits.code = code;
+
+    // Following status codes MUST NOT containt response body
+    if (code == 204 || code == 304) {
+        traits.allowBody = false;
+    }
+
+    return traits;
+}
diff --git a/src/HTTP.h b/src/HTTP.h
@@ -21,7 +21,7 @@
 #define HTTP_METHODS "GET|POST|PUT|DELETE|OPTIONS|PATCH|PROPPATCH|LOCK|UNLOCK|COPY|MOVE|MKCOL|HEAD"
 
 /** 
- *  \brief URI Template
+ *  \brief URI Template.
  *
  *  See previous technical note (using macro).
  */
@@ -30,12 +30,46 @@
 namespace snowcrash {
 
     /**
-     *  Selected HTTP Header names
+     *  Selected HTTP Header names.
      */
     struct HTTPHeaderName {
         static const std::string Accept;
         static const std::string ContentType;
     };
+
+    /**
+     *  A HTTP Status code.
+     */
+    typedef unsigned int HTTPStatusCode;
+
+    /**
+     *  Traits of a HTTP response.
+     */
+    struct HTTPResponseTraits {
+
+        bool allowBody; /// < Response body is allowed.
+
+        HTTPResponseTraits() : allowBody(true) {}
+    };
+
+    /**
+     *  Response traits for a HTTP status code.
+     *
+     *  Status-related response prescription.
+     *  Ref: http://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html
+     */
+    struct StatusCodeTraits : HTTPResponseTraits
+    {
+        HTTPStatusCode code;
+        StatusCodeTraits() : code(0) {}
+    };
+
+    /**
+     *  \brief  Retrieve response traits for given status code.
+     *  \param  code    A HTTP status code to retrieve traits for.
+     *  \return A %StatusCodeTraits for given code.
+     */
+    extern StatusCodeTraits GetStatusCodeTrait(HTTPStatusCode code);
 }
 
 #endif
diff --git a/src/HeaderParser.h b/src/HeaderParser.h
@@ -146,7 +146,7 @@ namespace snowcrash {
                         std::stringstream ss;
                         ss << "duplicate definition of `" << header.first << "` header";
                         result.first.warnings.push_back(Warning(ss.str(),
-                                                                DuplicateWarnign,
+                                                                DuplicateWarning,
                                                                 sourceMap));
 
                     }
@@ -196,7 +196,7 @@ namespace snowcrash {
     void CheckHeaderDuplicates(const T& left,
                                const R& right,
                                const SourceDataBlock& rightSourceMap,
-                                Result& result) {
+                               Result& result) {
 
         for (HeaderIterator it = right.headers.begin(); it != right.headers.end(); ++it) {
             if (FindHeader(left.headers, *it) != left.headers.end()) {

diff --git a/src/MethodParser.h b/src/MethodParser.h
@@ -14,6 +14,7 @@
 #include "RegexMatch.h"
 #include "PayloadParser.h"
 #include "HeaderParser.h"
+#include "HTTP.h"
 
 static const std::string MethodHeaderRegex("^(" HTTP_METHODS ")[ \\t]*(" URI_TEMPLATE ")?$");
 static const std::string NamedMethodHeaderRegex("^([^\\[]*)\\[(" HTTP_METHODS ")]$");
@@ -214,6 +215,14 @@ namespace snowcrash {
             return result;
         }
 
+        /**
+         *  \brief  Parse method payload
+         *  \param  begin   The begin of the block to be parsed.
+         *  \param  end     The end of the markdown block buffer.
+         *  \param  parser  A parser's instance.
+         *  \param  method  An output buffer to store parsed payload into.
+         *  \return A block parser section result.
+         */
         static ParseSectionResult HandlePayload(const Section &section,
                                                 const BlockIterator& begin,
                                                 const BlockIterator& end,
@@ -225,31 +234,89 @@ namespace snowcrash {
             if (result.first.error.code != Error::OK)
                 return result;
 
+            // Check for duplicate
             if (IsPayloadDuplicate(section, payload, method)) {
                 // WARN: duplicate payload
                 std::stringstream ss;
                 ss << SectionName(section) << " payload `" << payload.name << "`";
                 ss << " already defined for `" << method.method << "` method";
                 BlockIterator nameBlock = ListItemNameBlock(begin, end);
                 result.first.warnings.push_back(Warning(ss.str(),
-                                                        DuplicateWarnign,
+                                                        DuplicateWarning,
                                                         nameBlock->sourceMap));
-
             }
 
-            if (section == RequestSection)
+            BlockIterator nameBlock = ListItemNameBlock(begin, end);
+
+            // Check payload integrity
+            CheckPayload(section, payload, nameBlock->sourceMap, result.first);
+
+            // Inject parsed payload into method
+            if (section == RequestSection) {
                 method.requests.push_back(payload);
-            else if (section == ResponseSection)
+            }
+            else if (section == ResponseSection) {
                 method.responses.push_back(payload);
+            }
 
-            BlockIterator nameBlock = ListItemNameBlock(begin, end);
+            // Check header duplicates
             CheckHeaderDuplicates(method, payload, nameBlock->sourceMap, result.first);
 
             return result;
         }
 
-        // Checks whether given section payload has duplicate.
-        // Returns true when a duplicate is found, false otherwise.
+
+        /**
+         *  \brief  Check & report payload validity.
+         *  \param  section     A section of the payload.
+         *  \param  sourceMap   Payload signature source map.
+         *  \param  payload     The payload to be checked.
+         */
+        static void CheckPayload(const Section& section,
+                                 const Payload& payload,
+                                 const SourceDataBlock& sourceMap,
+                                 Result& result) {
+
+            bool warnEmptyBody = false;
+            if (section == RequestSection) {
+                warnEmptyBody = payload.body.empty();
+            }
+            else if (section == ResponseSection) {
+                // Check status code
+                HTTPStatusCode code = 0;
+                if (!payload.name.empty()) {
+                    std::stringstream(payload.name) >> code;
+                }
+                StatusCodeTraits traits = GetStatusCodeTrait(code);
+                if (traits.allowBody) {
+                    warnEmptyBody = payload.body.empty();
+                }
+                else if (!payload.body.empty()) {
+                    // WARN: not empty body
+                    std::stringstream ss;
+                    ss << "the " << code << " response MUST NOT include a message-body";
+                    result.warnings.push_back(Warning(ss.str(),
+                                                      EmptyDefinitionWarning,
+                                                      sourceMap));
+                    return;
+                }
+            }
+
+            // Issue the warning
+            if (warnEmptyBody) {
+                // WARN: empty body
+                std::stringstream ss;
+                ss << "empty " << SectionName(section) << " message-body";
+                result.warnings.push_back(Warning(ss.str(),
+                                                  EmptyDefinitionWarning,
+                                                  sourceMap));
+            }
+        }
+
+        /**
+         *  Checks whether given section payload has duplicate.
+         *  \return True when a duplicate is found, false otherwise.
+         */
         static bool IsPayloadDuplicate(const Section& section, const Payload& payload, Method& method) {
 
             if (section == RequestSection) {