diff --git a/src/BlueprintParser.h b/src/BlueprintParser.h index 51c8a0f4..ba688489 100644 --- 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 index d9601605..6f976cd8 100644 --- 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::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::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 index 2604ee0f..bf3eabdf 100644 --- 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 index 6f04661b..ea47aea8 100644 --- 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 index ff1b8c10..426cbb69 100644 --- 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 index 490ee1d8..98e6025b 100644 --- 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 §ion, const BlockIterator& begin, const BlockIterator& end, @@ -225,6 +234,7 @@ 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; @@ -232,24 +242,81 @@ namespace snowcrash { 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) { diff --git a/src/PayloadParser.h b/src/PayloadParser.h index 9c296237..be476826 100644 --- a/src/PayloadParser.h +++ b/src/PayloadParser.h @@ -19,39 +19,36 @@ #include "AssetParser.h" #include "HeaderParser.h" -// Request matching regex +/** Request matching regex */ static const std::string RequestRegex("^[ \\t]*[Rr]equest([ \\t]+" SYMBOL_IDENTIFIER ")?([ \\t]\\(([^\\)]*)\\))?[ \\t]*$"); -// Response matching regex +/** Response matching regex */ static const std::string ResponseRegex("^[ \\t]*[Rr]esponse([ \\t]+([0-9_])*)?([ \\t]\\(([^\\)]*)\\))?[ \\t]*$"); -// Object matching regex +/** Object matching regex */ static const std::string ObjectRegex("^[ \\t]*(" SYMBOL_IDENTIFIER ")[ \\t][Oo]bject([ \\t]\\(([^\\)]*)\\))?[ \\t]*$"); namespace snowcrash { - FORCEINLINE Collection::const_iterator FindRequest(const Method& method, const Request& request) { - return std::find_if(method.requests.begin(), - method.requests.end(), - std::bind2nd(MatchPayload(), request)); - } - - FORCEINLINE Collection::const_iterator FindResponse(const Method& method, const Response& response) { - return std::find_if(method.responses.begin(), - method.responses.end(), - std::bind2nd(MatchPayload(), response)); - } - - // Payload signature + /** + * Payload signature + */ enum PayloadSignature { - UndefinedPayloadSignature, - NoPayloadSignature, - RequestPayloadSignature, - ResponsePayloadSignature, - ObjectPayloadSignature + UndefinedPayloadSignature, /// < Undefined payload. + NoPayloadSignature, /// < Not a payload. + RequestPayloadSignature, /// < Request payload. + ResponsePayloadSignature, /// < Response payload. + ObjectPayloadSignature /// < Resource object payload. }; - // Query payload signature a of given block + /** + * \brief Query the payload signature of a given block. + * \param begin The begin of the block to be queried. + * \param end The end of the markdown block buffer. + * \param name A buffer to retrieve payload name into. + * \param mediaType A buffer to retrieve payload media type into. + * \return The %PayloadSignature of the given block. + */ FORCEINLINE PayloadSignature GetPayloadSignature(const BlockIterator& begin, const BlockIterator& end, Name& name, @@ -93,6 +90,9 @@ namespace snowcrash { return NoPayloadSignature; } + /** + * Returns true if given block has any payload signature, false otherwise. + */ FORCEINLINE bool HasPayloadSignature(const BlockIterator& begin, const BlockIterator& end) { Name name; @@ -101,6 +101,10 @@ namespace snowcrash { return signature != NoPayloadSignature; } + /** + * Retruns true if given block has any payload signature and + * is written in the abbreviated form. False otherwise. + */ FORCEINLINE bool HasPayloadAssetSignature(const BlockIterator& begin, const BlockIterator& end) { if (!HasPayloadSignature(begin, end)) @@ -109,9 +113,9 @@ namespace snowcrash { return !HasNestedListBlock(begin, end); } - // - // Classifier of internal list items, Payload context - // + /** + * Classifier of internal list items, payload context. + */ template <> FORCEINLINE Section ClassifyInternaListBlock(const BlockIterator& begin, const BlockIterator& end) { @@ -128,9 +132,9 @@ namespace snowcrash { return UndefinedSection; } - // - // Block Classifier, Payload Context - // + /** + * Block Classifier, payload context. + */ template <> FORCEINLINE Section ClassifyBlock(const BlockIterator& begin, const BlockIterator& end, @@ -194,9 +198,9 @@ namespace snowcrash { context == ObjectSection) ? context : UndefinedSection; } - // - // Payload Section Parser - // + /** + * Payload section parser. + */ template<> struct SectionParser { @@ -246,6 +250,15 @@ namespace snowcrash { return result; } + /** + * \brief Parse Payload's overview blocks. + * \param section The current section's signature. + * \param cur The actual position within Markdown block buffer. + * \param bound Boundaries of Markdown block buffer. + * \param parser A parser's instance. + * \param payload An output buffer to write overview description into. + * \return A block parser section result. + */ static ParseSectionResult HandleOverviewSectionBlock(const Section& section, const BlockIterator& cur, const SectionBounds& bounds, @@ -291,6 +304,15 @@ namespace snowcrash { return result; } + /** + * \brief Parse an asset. + * \param section The current section's signature. + * \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 payload An output buffer to save the parsed asset to. + * \return A block parser section result. + */ static ParseSectionResult HandleAsset(const Section& section, const BlockIterator& begin, const BlockIterator& end, @@ -300,17 +322,17 @@ namespace snowcrash { ParseSectionResult result = AssetParser::Parse(begin, end, parser, asset); if (result.first.error.code != Error::OK) return result; - - if (asset.empty()) { - // WARN: empty asset - BlockIterator nameBlock = ListItemNameBlock(begin, end); - std::stringstream ss; - ss << "empty " << SectionName(section) << " asset"; - result.first.warnings.push_back(Warning(ss.str(), - EmptyDefinitionWarnign, - nameBlock->sourceMap)); - } - + + // TODO: remove +// if (asset.empty()) { +// // WARN: empty asset +// BlockIterator nameBlock = ListItemNameBlock(begin, end); +// std::stringstream ss; +// ss << "empty " << SectionName(section) << " asset"; +// result.first.warnings.push_back(Warning(ss.str(), +// EmptyDefinitionWarning, +// nameBlock->sourceMap)); +// } if (!SetAsset(section, asset, payload)) { // WARN: asset already set @@ -325,7 +347,15 @@ namespace snowcrash { return result; } - // Handle payload + body asset abbreviation + /** + * \brief Parse payload and abbreviated asset. + * \param section The current section's signature. + * \param begin The parsed of the block to be queried. + * \param end The end of the markdown block buffer. + * \param parser A parser's instance. + * \param payload An output buffer to save the parsed paylod and asset into. + * \return A block parser section result. + */ static ParseSectionResult HandlePayloadAsset(const Section& section, const BlockIterator& begin, const BlockIterator& end, @@ -366,7 +396,15 @@ namespace snowcrash { return result; } - // Try to parse a symbol reference + /** + * \brief Parse a symbol reference. + * \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 symbolName Output buffer to put parsed symbol's name into. + * \param symbolSourceMap Source map of the parsed symbol reference. + * \return A block parser section result. + */ static ParseSectionResult ParseSymbolReference(const BlockIterator& begin, const BlockIterator& end, BlueprintParserCore& parser, @@ -433,7 +471,9 @@ namespace snowcrash { return result; } - // Retrieves & process payload name - signature + /** + * Retrieve and process payload signature. + */ static void ProcessSignature(const Section& section, const BlockIterator& begin, const BlockIterator& end, @@ -460,7 +500,7 @@ namespace snowcrash { (section == ResponseSection || section == ResponseBodySection)) { BlockIterator nameBlock = ListItemNameBlock(begin, end); result.warnings.push_back(Warning("missing response HTTP status code, assuming `Response 200`", - EmptyDefinitionWarnign, + EmptyDefinitionWarning, nameBlock->sourceMap)); payload.name = "200"; } @@ -472,7 +512,10 @@ namespace snowcrash { } } - // Sets payload section asset. Returns true on success, false when asset is already set. + /** + * \brief Set payload's asset. + * \return True on success, false when an asset is already set. + */ static bool SetAsset(const Section& section, const Asset& asset, Payload& payload) { if (section == BodySection) { @@ -492,6 +535,7 @@ namespace snowcrash { } }; + /** Payload Parser */ typedef BlockParser > PayloadParser; } diff --git a/src/ResourceGroupParser.h b/src/ResourceGroupParser.h index 85296716..b85510bb 100644 --- a/src/ResourceGroupParser.h +++ b/src/ResourceGroupParser.h @@ -125,7 +125,7 @@ namespace snowcrash { // WARN: No Group name specified result.first.warnings.push_back(Warning("expected resource group name, e.g. `# `", - EmptyDefinitionWarnign, + EmptyDefinitionWarning, cur->sourceMap)); } @@ -168,7 +168,7 @@ namespace snowcrash { result.first.warnings.push_back(Warning("resource `" + resource.uriTemplate + "` is already defined", - DuplicateWarnign, + DuplicateWarning, begin->sourceMap)); } diff --git a/src/ResourceParser.h b/src/ResourceParser.h index 958e5b9c..44b850a0 100644 --- a/src/ResourceParser.h +++ b/src/ResourceParser.h @@ -266,7 +266,7 @@ namespace snowcrash { BlockIterator nameBlock = ListItemNameBlock(begin, end); result.first.warnings.push_back(Warning(ss.str(), - DuplicateWarnign, + DuplicateWarning, nameBlock->sourceMap)); } else { @@ -336,7 +336,7 @@ namespace snowcrash { "` already defined for resource `" + resource.uriTemplate + "`", - DuplicateWarnign, + DuplicateWarning, begin->sourceMap)); } @@ -349,7 +349,7 @@ namespace snowcrash { " " + resource.uriTemplate + "`", - EmptyDefinitionWarnign, + EmptyDefinitionWarning, begin->sourceMap)); } diff --git a/src/SourceAnnotation.h b/src/SourceAnnotation.h index e38b41cf..4fff3b7c 100644 --- a/src/SourceAnnotation.h +++ b/src/SourceAnnotation.h @@ -111,11 +111,12 @@ namespace snowcrash { enum WarningCode { NoWarning = 0, APINameWarning = 1, - DuplicateWarnign = 2, + DuplicateWarning = 2, FormattingWarning = 3, RedefinitionWarning = 4, IgnoringWarning = 5, - EmptyDefinitionWarnign = 6 + EmptyDefinitionWarning = 6, + NotEmptyDefinitionWarning = 7 }; /** diff --git a/test/test-Parser.cc b/test/test-Parser.cc index 5d548177..dbc6a743 100644 --- a/test/test-Parser.cc +++ b/test/test-Parser.cc @@ -146,13 +146,11 @@ TEST_CASE("Support description ending with an list item", "[parser][issue][#8]") // ... //"); const std::string bluerpintSource = \ -"\n\ -# GET /1\n\ -+ a description item\n\ -+ Response 200\n\ -\n\ - ...\n\ -"; + "# GET /1\n"\ + "+ a description item\n"\ + "+ Response 200\n"\ + "\n"\ + " ...\n"; Parser parser; Result result; @@ -170,3 +168,30 @@ TEST_CASE("Support description ending with an list item", "[parser][issue][#8]") REQUIRE(blueprint.resourceGroups[0].resources[0].methods[0].responses[0].body == "...\n"); } +TEST_CASE("Invalid ‘warning: empty body asset’ for certain status codes", "[parser][issue][#13]") +{ + // Blueprint in question: + //R"( + //# GET /1 + //+ Response 304 + //"); + const std::string bluerpintSource = \ + "# GET /1\n"\ + "+ Response 304\n"; + + Parser parser; + Result result; + Blueprint blueprint; + parser.parse(bluerpintSource, 0, result, blueprint); + REQUIRE(result.error.code == Error::OK); + REQUIRE(result.warnings.empty()); + + REQUIRE(blueprint.resourceGroups.size() == 1); + REQUIRE(blueprint.resourceGroups[0].resources.size() == 1); + REQUIRE(blueprint.resourceGroups[0].resources[0].methods.size() == 1); + REQUIRE(blueprint.resourceGroups[0].resources[0].methods[0].description.empty()); + REQUIRE(blueprint.resourceGroups[0].resources[0].methods[0].responses.size() == 1); + REQUIRE(blueprint.resourceGroups[0].resources[0].methods[0].responses[0].name == "304"); + REQUIRE(blueprint.resourceGroups[0].resources[0].methods[0].responses[0].body.empty()); +} + diff --git a/test/test-PayloadParser.cc b/test/test-PayloadParser.cc index c5a319c5..6e8573da 100644 --- a/test/test-PayloadParser.cc +++ b/test/test-PayloadParser.cc @@ -212,7 +212,7 @@ TEST_CASE("Parse just one payload in a list with multiple payloads", "[payload]" ParseSectionResult result = PayloadParser::Parse(markdown.begin(), markdown.end(), parser, payload); REQUIRE(result.first.error.code == Error::OK); - CHECK(result.first.warnings.size() == 1); // empty body asset + REQUIRE(result.first.warnings.empty()); const MarkdownBlock::Stack &blocks = markdown; REQUIRE(std::distance(blocks.begin(), result.second) == 3); @@ -247,7 +247,7 @@ TEST_CASE("Parse just one payload in a list with multiple items", "[payload]") ParseSectionResult result = PayloadParser::Parse(markdown.begin(), markdown.end(), parser, payload); REQUIRE(result.first.error.code == Error::OK); - CHECK(result.first.warnings.size() == 1); // empty body asset + REQUIRE(result.first.warnings.empty()); const MarkdownBlock::Stack &blocks = markdown; REQUIRE(std::distance(blocks.begin(), result.second) == 3);