Skip to content

Commit 4bfc031

Browse files
dkalinowskidkalinowski
committed
post review
1 parent 4246051 commit 4bfc031

11 files changed

+143
-103
lines changed

‎src/BUILD‎

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -617,7 +617,7 @@ cc_library(
617617
}) + select({
618618
"//:enable_drogon": [
619619
"libdrogon_http_server",
620-
"libovmsmulti_part_parser_drogon_impl",
620+
"libmultipart_parser_drogon_impl",
621621
],
622622
"//conditions:default" : [
623623
"libnet_http_server",
@@ -817,7 +817,7 @@ cc_library(
817817
deps = [
818818
"@com_github_tencent_rapidjson//:rapidjson",
819819
"libovmsclient_connection",
820-
"libovmsmulti_part_parser",
820+
"libmultipart_parser",
821821
],
822822
visibility = ["//visibility:public"],
823823
copts = COPTS_ADJUSTED,
@@ -852,30 +852,28 @@ cc_library(
852852
)
853853

854854
cc_library(
855-
name = "libovmsmulti_part_parser",
855+
name = "libmultipart_parser",
856856
hdrs = ["multi_part_parser.hpp"],
857857
deps = [
858858
],
859859
visibility = ["//visibility:public",],
860860
local_defines = COMMON_LOCAL_DEFINES,
861861
copts = COPTS_ADJUSTED,
862862
linkopts = LINKOPTS_ADJUSTED,
863-
alwayslink = 1,
864863
)
865864

866865
cc_library(
867-
name = "libovmsmulti_part_parser_drogon_impl",
866+
name = "libmultipart_parser_drogon_impl",
868867
hdrs = ["http_frontend/multi_part_parser_drogon_impl.hpp"],
869868
srcs = ["http_frontend/multi_part_parser_drogon_impl.cpp"],
870869
deps = [
871-
"libovmsmulti_part_parser",
870+
"libmultipart_parser",
872871
"@drogon//:drogon_cmake",
873872
],
874873
visibility = ["//visibility:public",],
875874
local_defines = COMMON_LOCAL_DEFINES,
876875
copts = COPTS_ADJUSTED,
877876
linkopts = LINKOPTS_ADJUSTED,
878-
alwayslink = 1,
879877
)
880878

881879
cc_library(

‎src/http_frontend/multi_part_parser_drogon_impl.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ std::string DrogonMultiPartParser::getFieldByName(const std::string& name) const
3030
return this->parser->getParameter<std::string>(name);
3131
}
3232

33-
std::string_view DrogonMultiPartParser::getFileContentByName(const std::string& name) const {
33+
std::string_view DrogonMultiPartParser::getFileContentByFieldName(const std::string& name) const {
3434
auto fileMap = this->parser->getFilesMap();
3535

3636
auto it = fileMap.find(name);

‎src/http_frontend/multi_part_parser_drogon_impl.hpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
//*****************************************************************************
2-
// Copyright 2024 Intel Corporation
2+
// Copyright 2025 Intel Corporation
33
//
44
// Licensed under the Apache License, Version 2.0 (the "License");
55
// you may not use this file except in compliance with the License.
@@ -43,7 +43,7 @@ class DrogonMultiPartParser : public MultiPartParser {
4343
bool hasParseError() const override;
4444

4545
std::string getFieldByName(const std::string& name) const override;
46-
std::string_view getFileContentByName(const std::string& name) const override;
46+
std::string_view getFileContentByFieldName(const std::string& name) const override;
4747
};
4848

4949
} // namespace ovms

‎src/http_rest_api_handler.cpp‎

Lines changed: 109 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -453,99 +453,128 @@ Status HttpRestApiHandler::dispatchToProcessor(
453453
return StatusCode::UNKNOWN_REQUEST_COMPONENTS_TYPE;
454454
}
455455

456-
Status HttpRestApiHandler::processV3(const std::string_view uri, const HttpRequestComponents& request_components, std::string& response, const std::string& request_body, std::shared_ptr<HttpAsyncWriter> serverReaderWriter, std::shared_ptr<MultiPartParser> multiPartParser) {
457-
#if (MEDIAPIPE_DISABLE == 0)
458-
OVMS_PROFILE_FUNCTION();
459-
HttpPayload request;
460-
std::shared_ptr<Document> doc = std::make_shared<Document>();
461-
std::shared_ptr<MediapipeGraphExecutor> executor;
462-
bool streamFieldVal = false;
463-
{
464-
auto it = request_components.headers.find("content-type");
465-
bool isApplicationJson = it != request_components.headers.end() && it->second.find("application/json") != std::string::npos;
466-
bool isMultiPart = it != request_components.headers.end() && it->second.find("multipart/form-data") != std::string::npos;
467-
bool isDefault = !isApplicationJson && !isMultiPart;
468-
469-
std::string model_name;
470-
471-
if (isMultiPart) {
472-
OVMS_PROFILE_SCOPE("multipart parse");
473-
if (!multiPartParser->parse()) {
474-
return StatusCode::REST_INVALID_URL;
475-
}
476-
model_name = multiPartParser->getFieldByName("model");
477-
if (model_name.empty()) {
478-
isDefault = true;
479-
} else {
480-
SPDLOG_DEBUG("Model name from deduced from MultiPart field: {}", model_name);
481-
}
482-
// Set json parser in invalid state in order to get HasParseError to respond with true
483-
doc->Parse("error");
484-
} else if (isApplicationJson) {
485-
{
486-
OVMS_PROFILE_SCOPE("rapidjson parse");
487-
doc->Parse(request_body.c_str());
488-
}
489-
OVMS_PROFILE_SCOPE("rapidjson validate");
490-
if (doc->HasParseError()) {
491-
return Status(StatusCode::JSON_INVALID, "Cannot parse JSON body");
492-
}
456+
static void ensureJsonParserInErrorState(std::shared_ptr<Document>& parsedJson) {
457+
// Hack to set json parser in invalid state in order to get HasParseError to respond with true
458+
parsedJson->Parse("error");
459+
}
493460

494-
if (!doc->IsObject()) {
495-
return Status(StatusCode::JSON_INVALID, "JSON body must be an object");
496-
}
461+
static Status createV3HttpPayload(
462+
const std::string_view uri,
463+
const HttpRequestComponents& request_components,
464+
std::string& response,
465+
const std::string& request_body,
466+
std::shared_ptr<HttpAsyncWriter> serverReaderWriter,
467+
std::shared_ptr<MultiPartParser> multiPartParser,
468+
HttpPayload& request,
469+
std::string& modelName,
470+
bool& streamFieldVal) {
471+
OVMS_PROFILE_SCOPE("createV3HttpPayload");
472+
473+
std::shared_ptr<Document> parsedJson = std::make_shared<Document>();
474+
475+
auto it = request_components.headers.find("content-type");
476+
bool isApplicationJson = it != request_components.headers.end() && it->second.find("application/json") != std::string::npos;
477+
bool isMultiPart = it != request_components.headers.end() && it->second.find("multipart/form-data") != std::string::npos;
478+
bool isUriBasedRouting = !isApplicationJson && !isMultiPart; // For content types other than "application/json" and "multipart/form-data", we look for model information in the URI
479+
480+
if (isMultiPart) {
481+
OVMS_PROFILE_SCOPE("multipart parse");
482+
if (!multiPartParser->parse()) {
483+
SPDLOG_DEBUG("Failed to parse multipart content type request");
484+
return StatusCode::FAILED_TO_PARSE_MULTIPART_CONTENT_TYPE;
485+
}
486+
modelName = multiPartParser->getFieldByName("model");
487+
if (modelName.empty()) {
488+
isUriBasedRouting = true;
489+
} else {
490+
SPDLOG_DEBUG("Model name from deduced from MultiPart field: {}", modelName);
491+
}
492+
ensureJsonParserInErrorState(parsedJson);
493+
} else if (isApplicationJson) {
494+
{
495+
OVMS_PROFILE_SCOPE("rapidjson parse");
496+
parsedJson->Parse(request_body.c_str());
497+
}
498+
OVMS_PROFILE_SCOPE("rapidjson validate");
499+
if (parsedJson->HasParseError()) {
500+
return Status(StatusCode::JSON_INVALID, "Cannot parse JSON body");
501+
}
497502

498-
auto modelNameIt = doc->FindMember("model");
499-
if (modelNameIt == doc->MemberEnd()) {
500-
return Status(StatusCode::JSON_INVALID, "model field is missing in JSON body");
501-
}
503+
if (!parsedJson->IsObject()) {
504+
return Status(StatusCode::JSON_INVALID, "JSON body must be an object");
505+
}
502506

503-
if (!modelNameIt->value.IsString()) {
504-
return Status(StatusCode::JSON_INVALID, "model field is not a string");
505-
}
507+
auto modelNameIt = parsedJson->FindMember("model");
508+
if (modelNameIt == parsedJson->MemberEnd()) {
509+
return Status(StatusCode::JSON_INVALID, "model field is missing in JSON body");
510+
}
506511

507-
bool isTextGenerationEndpoint = uri.find("completions") != std::string_view::npos;
508-
if (isTextGenerationEndpoint) {
509-
auto streamIt = doc->FindMember("stream");
510-
if (streamIt != doc->MemberEnd()) {
511-
if (!streamIt->value.IsBool()) {
512-
return Status(StatusCode::JSON_INVALID, "stream field is not a boolean");
513-
}
514-
streamFieldVal = streamIt->value.GetBool();
515-
}
516-
}
512+
if (!modelNameIt->value.IsString()) {
513+
return Status(StatusCode::JSON_INVALID, "model field is not a string");
514+
}
517515

518-
model_name = modelNameIt->value.GetString();
519-
if (model_name.empty()) {
520-
isDefault = true;
521-
} else {
522-
SPDLOG_DEBUG("Model name from deduced from JSON: {}", model_name);
516+
bool isTextGenerationEndpoint = uri.find("completions") != std::string_view::npos;
517+
if (isTextGenerationEndpoint) {
518+
auto streamIt = parsedJson->FindMember("stream");
519+
if (streamIt != parsedJson->MemberEnd()) {
520+
if (!streamIt->value.IsBool()) {
521+
return Status(StatusCode::JSON_INVALID, "stream field is not a boolean");
522+
}
523+
streamFieldVal = streamIt->value.GetBool();
523524
}
524525
}
525526

526-
// Deduce Graph Name from URI since there is no info in JSON or MultiPart
527-
if (isDefault) {
528-
if (uri.size() <= 4) { // nothing after "/v3/..."
529-
return StatusCode::REST_INVALID_URL;
530-
}
531-
model_name = std::string(uri.substr(4));
532-
SPDLOG_DEBUG("Model name from deduced from URI: {}", model_name);
533-
// Set json parser in invalid state in order to get HasParseError to respond with true
534-
doc->Parse("error");
527+
modelName = modelNameIt->value.GetString();
528+
if (modelName.empty()) {
529+
isUriBasedRouting = true;
530+
} else {
531+
SPDLOG_DEBUG("Model name from deduced from JSON: {}", modelName);
535532
}
533+
}
536534

537-
auto status = this->modelManager.createPipeline(executor, model_name);
538-
if (!status.ok()) {
539-
return status;
535+
// Deduce Graph Name from URI since there is no info in JSON or MultiPart
536+
if (isUriBasedRouting) {
537+
if (uri.size() <= 4) { // nothing after "/v3/..."
538+
SPDLOG_DEBUG("Failed to deduce model name from URI");
539+
return StatusCode::FAILED_TO_DEDUCE_MODEL_NAME_FROM_URI;
540540
}
541+
modelName = std::string(uri.substr(4));
542+
SPDLOG_DEBUG("Model name from deduced from URI: {}", modelName);
543+
// Set json parser in invalid state in order to get HasParseError to respond with true
544+
ensureJsonParserInErrorState(parsedJson);
545+
}
546+
547+
request.headers = request_components.headers;
548+
request.body = request_body;
549+
request.parsedJson = std::move(parsedJson);
550+
request.uri = std::string(uri);
551+
request.client = std::make_shared<HttpClientConnection>(serverReaderWriter);
552+
request.multipartParser = std::move(multiPartParser);
553+
554+
return StatusCode::OK;
555+
}
556+
557+
Status HttpRestApiHandler::processV3(const std::string_view uri, const HttpRequestComponents& request_components, std::string& response, const std::string& request_body, std::shared_ptr<HttpAsyncWriter> serverReaderWriter, std::shared_ptr<MultiPartParser> multiPartParser) {
558+
#if (MEDIAPIPE_DISABLE == 0)
559+
OVMS_PROFILE_FUNCTION();
560+
561+
HttpPayload request;
562+
std::string modelName;
563+
bool streamFieldVal = false;
564+
565+
auto status = createV3HttpPayload(uri, request_components, response, request_body, serverReaderWriter, std::move(multiPartParser), request, modelName, streamFieldVal);
566+
if (!status.ok()) {
567+
// TODO: Add logger
568+
SPDLOG_DEBUG("Failed to create V3 payload");
569+
return status;
570+
}
541571

542-
request.headers = request_components.headers;
543-
request.body = request_body;
544-
request.parsedJson = std::move(doc);
545-
request.uri = std::string(uri);
546-
request.client = std::make_shared<HttpClientConnection>(serverReaderWriter);
547-
request.multipartParser = std::move(multiPartParser);
572+
std::shared_ptr<MediapipeGraphExecutor> executor;
573+
status = this->modelManager.createPipeline(executor, modelName);
574+
if (!status.ok()) {
575+
return status;
548576
}
577+
549578
if (streamFieldVal == false) {
550579
ExecutionContext executionContext{ExecutionContext::Interface::REST, ExecutionContext::Method::V3Unary};
551580
return executor->infer(&request, &response, executionContext);

‎src/multi_part_parser.hpp‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,20 @@ namespace ovms {
2020

2121
class MultiPartParser {
2222
public:
23+
24+
// Called by ovms core during initial request processing to deduce servable name for routing
2325
virtual bool parse() = 0;
2426

27+
// API for MP calculators to check whether request was an actual multipart request
2528
virtual bool hasParseError() const = 0;
2629

30+
// API for MP calculators to get the multipart field content by field name.
31+
// Returns empty string if field is not found.
2732
virtual std::string getFieldByName(const std::string& name) const = 0;
28-
virtual std::string_view getFileContentByName(const std::string& name) const = 0;
33+
34+
// API for MP calculators to get the multipart file content by field name.
35+
// Returns empty string if file is not found.
36+
virtual std::string_view getFileContentByFieldName(const std::string& name) const = 0;
2937
};
3038

3139
} // namespace ovms

‎src/status.cpp‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,8 @@ const std::unordered_map<StatusCode, std::string> Status::statusMessageMap = {
130130
{StatusCode::REST_INVALID_URL, "Invalid request URL"},
131131
{StatusCode::REST_UNSUPPORTED_METHOD, "Unsupported method"},
132132
{StatusCode::UNKNOWN_REQUEST_COMPONENTS_TYPE, "Request components type not recognized"},
133+
{StatusCode::FAILED_TO_PARSE_MULTIPART_CONTENT_TYPE, "Request of multipart type but failed to parse"},
134+
{StatusCode::FAILED_TO_DEDUCE_MODEL_NAME_FROM_URI, "Failed to deduce model name from all possible ways"},
133135

134136
// Rest parser failure
135137
{StatusCode::REST_BODY_IS_NOT_AN_OBJECT, "Request body should be JSON object"},

‎src/status.hpp‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -167,11 +167,14 @@ enum class StatusCode {
167167
AS_INCORRECT_REQUESTED_OBJECT_TYPE,
168168

169169
// REST handler
170-
REST_NOT_FOUND, /*!< Requested REST resource not found */
171-
REST_COULD_NOT_PARSE_VERSION, /*!< Could not parse model version in request */
172-
REST_INVALID_URL, /*!< Malformed REST request url */
173-
REST_UNSUPPORTED_METHOD, /*!< Request sent with unsupported method */
174-
UNKNOWN_REQUEST_COMPONENTS_TYPE, /*!< Components type not recognized */
170+
REST_NOT_FOUND, /*!< Requested REST resource not found */
171+
REST_COULD_NOT_PARSE_VERSION, /*!< Could not parse model version in request */
172+
REST_INVALID_URL, /*!< Malformed REST request url */
173+
REST_UNSUPPORTED_METHOD, /*!< Request sent with unsupported method */
174+
UNKNOWN_REQUEST_COMPONENTS_TYPE, /*!< Components type not recognized */
175+
FAILED_TO_PARSE_MULTIPART_CONTENT_TYPE, /*!< Request of multipart type but failed to parse */
176+
FAILED_TO_DEDUCE_MODEL_NAME_FROM_URI, /*!< Failed to deduce model name from all possible ways */
177+
175178

176179
// REST Parse
177180
REST_BODY_IS_NOT_AN_OBJECT, /*!< REST body should be JSON object */

‎src/test/mediapipe/calculators/multipart_accepting_calculator.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ class MultipartAcceptingCalculator : public CalculatorBase {
5050
RET_CHECK(payload.multipartParser != nullptr);
5151
std::string email = payload.multipartParser->getFieldByName("email");
5252
std::string username = payload.multipartParser->getFieldByName("username");
53-
std::string_view fileContent = payload.multipartParser->getFileContentByName("file");
53+
std::string_view fileContent = payload.multipartParser->getFileContentByFieldName("file");
5454

5555
cc->Outputs().Tag(OUTPUT_TAG_NAME).Add(new std::string{email + std::string{"+"} + username + std::string{"\n"} + std::string(fileContent)}, cc->InputTimestamp());
5656
return absl::OkStatus();

‎src/test/multi_part_parser_drogon_test.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,6 @@ TEST(MultiPartParserDrogonImpl, GetFileContentByName) {
5454
ovms::DrogonMultiPartParser parser(req);
5555
ASSERT_TRUE(parser.parse());
5656
ASSERT_FALSE(parser.hasParseError());
57-
std::string val = std::string(parser.getFileContentByName("somekey"));
57+
std::string val = std::string(parser.getFileContentByFieldName("somekey"));
5858
EXPECT_EQ(val, "Hello; World");
5959
}

‎src/test/multipart_calculator_test.cpp‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ It has two lines.
9292
EXPECT_CALL(*multiPartParser, getFieldByName(::testing::Eq("model"))).WillOnce(::testing::Return("multipart"));
9393
EXPECT_CALL(*multiPartParser, getFieldByName(::testing::Eq("email"))).WillOnce(::testing::Return("[email protected]"));
9494
EXPECT_CALL(*multiPartParser, getFieldByName(::testing::Eq("username"))).WillOnce(::testing::Return("john_doe"));
95-
EXPECT_CALL(*multiPartParser, getFileContentByName(::testing::Eq("file"))).WillOnce([](const std::string& name) {
95+
EXPECT_CALL(*multiPartParser, getFileContentByFieldName(::testing::Eq("file"))).WillOnce([](const std::string& name) {
9696
static std::string retval{"this is file content\nIt has two lines."};
9797
return std::string_view(retval);
9898
});
@@ -134,7 +134,7 @@ It has two lines.
134134
EXPECT_CALL(*multiPartParser, getFieldByName(::testing::Eq("model"))).WillOnce(::testing::Return(""));
135135
EXPECT_CALL(*multiPartParser, getFieldByName(::testing::Eq("email"))).WillOnce(::testing::Return("[email protected]"));
136136
EXPECT_CALL(*multiPartParser, getFieldByName(::testing::Eq("username"))).WillOnce(::testing::Return("john_doe"));
137-
EXPECT_CALL(*multiPartParser, getFileContentByName(::testing::Eq("file"))).WillOnce([](const std::string& name) {
137+
EXPECT_CALL(*multiPartParser, getFileContentByFieldName(::testing::Eq("file"))).WillOnce([](const std::string& name) {
138138
static std::string retval{"this is file content\nIt has two lines."};
139139
return std::string_view(retval);
140140
});
@@ -178,7 +178,7 @@ It has two lines.
178178
EXPECT_CALL(*multiPartParser, parse()).WillOnce(::testing::Return(true));
179179
EXPECT_CALL(*multiPartParser, getFieldByName(::testing::_)).Times(0);
180180
EXPECT_CALL(*multiPartParser, getFieldByName(::testing::Eq("model"))).WillOnce(::testing::Return(""));
181-
EXPECT_CALL(*multiPartParser, getFileContentByName(::testing::_)).Times(0);
181+
EXPECT_CALL(*multiPartParser, getFileContentByFieldName(::testing::_)).Times(0);
182182

183183
// Default routing uses everything that comes after /v3/ as graph name
184184
const std::string URI = "/v3/NON_EXISTENT";
@@ -214,7 +214,7 @@ It has two lines.
214214
EXPECT_CALL(*multiPartParser, parse()).WillOnce(::testing::Return(true));
215215
EXPECT_CALL(*multiPartParser, getFieldByName(::testing::_)).Times(0);
216216
EXPECT_CALL(*multiPartParser, getFieldByName(::testing::Eq("model"))).WillOnce(::testing::Return(""));
217-
EXPECT_CALL(*multiPartParser, getFileContentByName(::testing::_)).Times(0);
217+
EXPECT_CALL(*multiPartParser, getFileContentByFieldName(::testing::_)).Times(0);
218218

219219
// Default routing uses everything that comes after /v3/ as graph name
220220
const std::string URI = "/v3/";

0 commit comments

Comments
 (0)