From c3d8fe2902a355fa940532adcca5f4430c5778a4 Mon Sep 17 00:00:00 2001 From: Will Sobel Date: Tue, 24 Mar 2026 19:22:15 +0100 Subject: [PATCH 01/15] Added CORS OPTIONS support --- src/mtconnect/sink/rest_sink/response.hpp | 4 ++ src/mtconnect/sink/rest_sink/routing.hpp | 9 +++++ src/mtconnect/sink/rest_sink/server.cpp | 37 +++++++++++++++++++ src/mtconnect/sink/rest_sink/server.hpp | 8 ++++ src/mtconnect/sink/rest_sink/session_impl.cpp | 8 +++- 5 files changed, 64 insertions(+), 2 deletions(-) diff --git a/src/mtconnect/sink/rest_sink/response.hpp b/src/mtconnect/sink/rest_sink/response.hpp index a380c609a..80bf2356b 100644 --- a/src/mtconnect/sink/rest_sink/response.hpp +++ b/src/mtconnect/sink/rest_sink/response.hpp @@ -21,6 +21,7 @@ #include #include +#include #include #include "cached_file.hpp" @@ -64,6 +65,9 @@ namespace mtconnect { std::optional m_requestId; ///< Request id from websocket sub CachedFilePtr m_file; ///< Cached file if a file is being returned + + /// @brief Additional per-response header fields (e.g. for CORS preflight) + std::list> m_fields; }; using ResponsePtr = std::unique_ptr; diff --git a/src/mtconnect/sink/rest_sink/routing.hpp b/src/mtconnect/sink/rest_sink/routing.hpp index f3ee2f37a..d5ae3adf8 100644 --- a/src/mtconnect/sink/rest_sink/routing.hpp +++ b/src/mtconnect/sink/rest_sink/routing.hpp @@ -295,6 +295,15 @@ namespace mtconnect::sink::rest_sink { return true; } + /// @brief check if the routing's path pattern matches a given path (ignoring verb) + /// @param[in] path the request path to test + /// @return `true` if the path matches this routing's pattern + bool matchesPath(const std::string &path) const + { + std::smatch m; + return std::regex_match(path, m, m_pattern); + } + /// @brief check if this is related to a swagger API /// @returns `true` if related to swagger auto isSwagger() const { return m_swagger; } diff --git a/src/mtconnect/sink/rest_sink/server.cpp b/src/mtconnect/sink/rest_sink/server.cpp index b05804866..e531c444b 100644 --- a/src/mtconnect/sink/rest_sink/server.cpp +++ b/src/mtconnect/sink/rest_sink/server.cpp @@ -27,6 +27,8 @@ #include #include #include +#include +#include #include @@ -413,4 +415,39 @@ namespace mtconnect::sink::rest_sink { // addRouting({boost::beast::http::verb::get, "/swagger.yaml", handler, true}); } + void Server::addOptionsRouting() + { + using namespace boost; + using namespace adaptors; + auto handler = [this](SessionPtr session, const RequestPtr request) -> bool { + // Collect the set of HTTP verbs supported at this path + set verbs; + for (const auto &r : m_routings) + { + if (!r.isSwagger() && r.matchesPath(request->m_path)) + verbs.insert(r.getVerb()); + } + + // OPTIONS is always allowed + verbs.insert(http::verb::options); + + // Build the Allow / Access-Control-Allow-Methods header value + string methods = algorithm::join( + verbs | transformed([](http::verb v) { return string(http::to_string(v)); }), ", "); + + auto response = std::make_unique(status::no_content, "", "text/plain"); + response->m_close = false; + response->m_fields.emplace_back("Allow", methods); + response->m_fields.emplace_back("Access-Control-Allow-Methods", methods); + response->m_fields.emplace_back("Access-Control-Allow-Headers", + "Content-Type, Accept, Accept-Encoding"); + response->m_fields.emplace_back("Access-Control-Max-Age", "86400"); + + session->writeResponse(std::move(response)); + return true; + }; + + addRouting({boost::beast::http::verb::options, std::regex("/.*"), handler, true}); + } + } // namespace mtconnect::sink::rest_sink diff --git a/src/mtconnect/sink/rest_sink/server.hpp b/src/mtconnect/sink/rest_sink/server.hpp index 2fdd8010e..acec63431 100644 --- a/src/mtconnect/sink/rest_sink/server.hpp +++ b/src/mtconnect/sink/rest_sink/server.hpp @@ -85,6 +85,7 @@ namespace mtconnect::sink::rest_sink { loadTlsCertificate(); addSwaggerRoutings(); + addOptionsRouting(); } /// @brief Start the http server @@ -289,6 +290,13 @@ namespace mtconnect::sink::rest_sink { /// /// @brief Add swagger routings to the Agent void addSwaggerRoutings(); + /// @} + + /// @name CORS Support + /// @{ + /// + /// @brief Add OPTIONS routing for CORS preflight requests + void addOptionsRouting(); /// @brief generate swagger API from routings /// @param[in] format The mime format of the response ("json" or "yaml") /// diff --git a/src/mtconnect/sink/rest_sink/session_impl.cpp b/src/mtconnect/sink/rest_sink/session_impl.cpp index e241d5cc0..a17cd04a9 100644 --- a/src/mtconnect/sink/rest_sink/session_impl.cpp +++ b/src/mtconnect/sink/rest_sink/session_impl.cpp @@ -199,8 +199,8 @@ namespace mtconnect::sink::rest_sink { auto &msg = m_parser->get(); const auto &remote = m_remote; - // Check for put, post, or delete - if (msg.method() != http::verb::get) + // Check for put, post, or delete (allow OPTIONS for CORS preflight) + if (msg.method() != http::verb::get && msg.method() != http::verb::options) { if (!m_allowPuts) { @@ -376,6 +376,10 @@ namespace mtconnect::sink::rest_sink { { res->set(http::field::location, *response.m_location); } + for (const auto &f : response.m_fields) + { + res->set(f.first, f.second); + } } template From 95919c6b3829cddb2b3f4d294f676314ab54c78f Mon Sep 17 00:00:00 2001 From: Will Sobel Date: Tue, 24 Mar 2026 19:39:54 +0100 Subject: [PATCH 02/15] Added unit tests --- test_package/http_server_test.cpp | 139 ++++++++++++++++++++++++++++++ test_package/routing_test.cpp | 57 ++++++++++++ 2 files changed, 196 insertions(+) diff --git a/test_package/http_server_test.cpp b/test_package/http_server_test.cpp index 81c0a1c56..e3c3eb5de 100644 --- a/test_package/http_server_test.cpp +++ b/test_package/http_server_test.cpp @@ -683,6 +683,145 @@ TEST_F(HttpServerTest, additional_header_fields) ASSERT_EQ("https://foo.example", f2->second); } +TEST_F(HttpServerTest, options_returns_allowed_methods_for_get_only_path) +{ + auto handler = [&](SessionPtr session, RequestPtr request) -> bool { + ResponsePtr resp = make_unique(status::ok, "Done"); + session->writeResponse(std::move(resp)); + return true; + }; + + m_server->addRouting({http::verb::get, "/probe", handler}); + + start(); + startClient(); + + m_client->spawnRequest(http::verb::options, "/probe"); + ASSERT_TRUE(m_client->m_done); + EXPECT_EQ(int(http::status::no_content), m_client->m_status); + + auto allow = m_client->m_fields.find("Allow"); + ASSERT_NE(m_client->m_fields.end(), allow); + EXPECT_NE(string::npos, allow->second.find("GET")); + EXPECT_NE(string::npos, allow->second.find("OPTIONS")); + EXPECT_EQ(string::npos, allow->second.find("PUT")); + EXPECT_EQ(string::npos, allow->second.find("POST")); + EXPECT_EQ(string::npos, allow->second.find("DELETE")); + + auto acam = m_client->m_fields.find("Access-Control-Allow-Methods"); + ASSERT_NE(m_client->m_fields.end(), acam); + EXPECT_EQ(allow->second, acam->second); + + auto acah = m_client->m_fields.find("Access-Control-Allow-Headers"); + ASSERT_NE(m_client->m_fields.end(), acah); + + auto acma = m_client->m_fields.find("Access-Control-Max-Age"); + ASSERT_NE(m_client->m_fields.end(), acma); + EXPECT_EQ("86400", acma->second); +} + +TEST_F(HttpServerTest, options_returns_get_put_and_delete_when_registered) +{ + auto getHandler = [&](SessionPtr session, RequestPtr request) -> bool { + ResponsePtr resp = make_unique(status::ok, "Done"); + session->writeResponse(std::move(resp)); + return true; + }; + auto putHandler = [&](SessionPtr session, RequestPtr request) -> bool { + ResponsePtr resp = make_unique(status::ok, "Put ok"); + session->writeResponse(std::move(resp)); + return true; + }; + auto deleteHandler = [&](SessionPtr session, RequestPtr request) -> bool { + ResponsePtr resp = make_unique(status::ok, "Deleted"); + session->writeResponse(std::move(resp)); + return true; + }; + + m_server->addRouting({http::verb::get, "/asset/{id}", getHandler}); + m_server->addRouting({http::verb::put, "/asset/{id}", putHandler}); + m_server->addRouting({http::verb::delete_, "/asset/{id}", deleteHandler}); + m_server->allowPuts(); + + start(); + startClient(); + + m_client->spawnRequest(http::verb::options, "/asset/123"); + ASSERT_TRUE(m_client->m_done); + EXPECT_EQ(int(http::status::no_content), m_client->m_status); + + auto allow = m_client->m_fields.find("Allow"); + ASSERT_NE(m_client->m_fields.end(), allow); + EXPECT_NE(string::npos, allow->second.find("GET")); + EXPECT_NE(string::npos, allow->second.find("OPTIONS")); + EXPECT_NE(string::npos, allow->second.find("PUT")); + EXPECT_EQ(string::npos, allow->second.find("POST")); + EXPECT_NE(string::npos, allow->second.find("DELETE")); +} + +TEST_F(HttpServerTest, options_allowed_even_when_puts_disabled) +{ + auto handler = [&](SessionPtr session, RequestPtr request) -> bool { + ResponsePtr resp = make_unique(status::ok, "Done"); + session->writeResponse(std::move(resp)); + return true; + }; + + m_server->addRouting({http::verb::get, "/probe", handler}); + // Note: puts are NOT enabled + + start(); + startClient(); + + m_client->spawnRequest(http::verb::options, "/probe"); + ASSERT_TRUE(m_client->m_done); + + // OPTIONS should succeed even though puts are disabled + EXPECT_EQ(int(http::status::no_content), m_client->m_status); + + auto allow = m_client->m_fields.find("Allow"); + ASSERT_NE(m_client->m_fields.end(), allow); + EXPECT_NE(string::npos, allow->second.find("GET")); + EXPECT_NE(string::npos, allow->second.find("OPTIONS")); + EXPECT_EQ(string::npos, allow->second.find("PUT")); + EXPECT_EQ(string::npos, allow->second.find("POST")); + EXPECT_EQ(string::npos, allow->second.find("DELETE")); +} + +TEST_F(HttpServerTest, options_includes_configured_cors_origin_header) +{ + m_server->setHttpHeaders({"Access-Control-Allow-Origin:*"}); + + auto handler = [&](SessionPtr session, RequestPtr request) -> bool { + ResponsePtr resp = make_unique(status::ok, "Done"); + session->writeResponse(std::move(resp)); + return true; + }; + + m_server->addRouting({http::verb::get, "/probe", handler}); + + start(); + startClient(); + + m_client->spawnRequest(http::verb::options, "/probe"); + ASSERT_TRUE(m_client->m_done); + EXPECT_EQ(int(http::status::no_content), m_client->m_status); + + // Access-Control-Allow-Origin comes from the configured HttpHeaders + auto acao = m_client->m_fields.find("Access-Control-Allow-Origin"); + ASSERT_NE(m_client->m_fields.end(), acao); + ASSERT_EQ("*", acao->second); + + // Access-Control-Allow-Methods comes from the OPTIONS handler + auto acam = m_client->m_fields.find("Access-Control-Allow-Methods"); + ASSERT_NE(m_client->m_fields.end(), acam); + EXPECT_NE(string::npos, acam->second.find("GET")); + EXPECT_NE(string::npos, acam->second.find("OPTIONS")); + EXPECT_EQ(string::npos, acam->second.find("PUT")); + EXPECT_EQ(string::npos, acam->second.find("POST")); + EXPECT_EQ(string::npos, acam->second.find("DELETE")); +} + const string CertFile(TEST_RESOURCE_DIR "/user.crt"); const string KeyFile {TEST_RESOURCE_DIR "/user.key"}; const string DhFile {TEST_RESOURCE_DIR "/dh2048.pem"}; diff --git a/test_package/routing_test.cpp b/test_package/routing_test.cpp index 3235cea32..d8fadf5ee 100644 --- a/test_package/routing_test.cpp +++ b/test_package/routing_test.cpp @@ -304,3 +304,60 @@ TEST_F(RoutingTest, simple_put_with_trailing_slash) ASSERT_TRUE(r.matches(0, request)); ASSERT_EQ("ADevice", get(request->m_parameters["device"])); } + +TEST_F(RoutingTest, matchesPath_matches_simple_path) +{ + Routing r(verb::get, "/probe", m_func); + + EXPECT_TRUE(r.matchesPath("/probe")); + EXPECT_TRUE(r.matchesPath("/probe/")); + EXPECT_FALSE(r.matchesPath("/sample")); + EXPECT_FALSE(r.matchesPath("/probe/extra")); +} + +TEST_F(RoutingTest, matchesPath_matches_path_with_parameter) +{ + Routing r(verb::get, "/{device}/probe", m_func); + + EXPECT_TRUE(r.matchesPath("/ABC123/probe")); + EXPECT_TRUE(r.matchesPath("/mydevice/probe")); + EXPECT_FALSE(r.matchesPath("/probe")); + EXPECT_FALSE(r.matchesPath("/dev/probe/extra")); +} + +TEST_F(RoutingTest, matchesPath_ignores_verb) +{ + Routing getRoute(verb::get, "/asset/{id}", m_func); + Routing putRoute(verb::put, "/asset/{id}", m_func); + Routing deleteRoute(verb::delete_, "/asset/{id}", m_func); + + // matchesPath should match regardless of the routing's verb + EXPECT_TRUE(getRoute.matchesPath("/asset/A1")); + EXPECT_TRUE(putRoute.matchesPath("/asset/A1")); + EXPECT_TRUE(deleteRoute.matchesPath("/asset/A1")); + + // Different paths should not match + EXPECT_FALSE(getRoute.matchesPath("/probe")); + EXPECT_FALSE(putRoute.matchesPath("/probe")); + EXPECT_FALSE(deleteRoute.matchesPath("/probe")); +} + +TEST_F(RoutingTest, matchesPath_works_with_regex_routing) +{ + Routing r(verb::get, regex("/.+"), m_func); + + EXPECT_TRUE(r.matchesPath("/anything")); + EXPECT_TRUE(r.matchesPath("/some/deep/path")); + EXPECT_FALSE(r.matchesPath("/")); +} + +TEST_F(RoutingTest, matchesPath_with_query_parameters_in_pattern) +{ + Routing r(verb::get, "/{device}/sample?from={unsigned_integer}&count={integer:100}", m_func); + + // matchesPath only checks the path component, query params in the pattern don't affect it + EXPECT_TRUE(r.matchesPath("/ABC123/sample")); + EXPECT_TRUE(r.matchesPath("/device1/sample/")); + EXPECT_FALSE(r.matchesPath("/sample")); +} + From fe1f4108ff3f29f0f18f091218e1b56aad45a28a Mon Sep 17 00:00:00 2001 From: Will Sobel Date: Thu, 26 Mar 2026 11:35:49 +0100 Subject: [PATCH 03/15] Fixed options with specific paths --- src/mtconnect/sink/rest_sink/parameter.hpp | 3 + src/mtconnect/sink/rest_sink/routing.hpp | 178 +++++++++++------- src/mtconnect/sink/rest_sink/server.cpp | 17 +- src/mtconnect/sink/rest_sink/session_impl.cpp | 3 +- test_package/http_server_test.cpp | 79 ++++++++ 5 files changed, 203 insertions(+), 77 deletions(-) diff --git a/src/mtconnect/sink/rest_sink/parameter.hpp b/src/mtconnect/sink/rest_sink/parameter.hpp index 541e4dcd0..e09c42d9a 100644 --- a/src/mtconnect/sink/rest_sink/parameter.hpp +++ b/src/mtconnect/sink/rest_sink/parameter.hpp @@ -64,6 +64,9 @@ namespace mtconnect::sink::rest_sink { Parameter(const std::string &n, ParameterType t = STRING, UrlPart p = PATH) : m_name(n), m_type(t), m_part(p) {} + Parameter(const std::string_view &n, ParameterType t = STRING, UrlPart p = PATH) + : m_name(n), m_type(t), m_part(p) + {} Parameter(const Parameter &o) = default; /// @brief to support std::set interface diff --git a/src/mtconnect/sink/rest_sink/routing.hpp b/src/mtconnect/sink/rest_sink/routing.hpp index d5ae3adf8..c5c7fe0ad 100644 --- a/src/mtconnect/sink/rest_sink/routing.hpp +++ b/src/mtconnect/sink/rest_sink/routing.hpp @@ -18,6 +18,8 @@ #pragma once #include +#include + #include #include @@ -26,6 +28,7 @@ #include #include #include +#include #include "mtconnect/config.hpp" #include "mtconnect/logging.hpp" @@ -36,14 +39,14 @@ namespace mtconnect::sink::rest_sink { class Session; using SessionPtr = std::shared_ptr; - + /// @brief A REST routing that parses a URI pattern and associates a lambda when it is matched /// against a request class AGENT_LIB_API Routing { public: using Function = std::function; - + Routing(const Routing &r) = default; /// @brief Create a routing with a string /// @@ -54,23 +57,23 @@ namespace mtconnect::sink::rest_sink { /// @param[in] swagger `true` if swagger related Routing(boost::beast::http::verb verb, const std::string &pattern, const Function function, bool swagger = false, std::optional request = std::nullopt) - : m_verb(verb), m_command(request), m_function(function), m_swagger(swagger) + : m_verb(verb), m_command(request), m_function(function), m_swagger(swagger) { std::string s(pattern); - + auto qp = s.find_first_of('?'); if (qp != std::string::npos) { auto query = s.substr(qp + 1); s.erase(qp); - + queryParameters(query); } - + m_path.emplace(s); pathParameters(s); } - + /// @brief Create a routing with a regular expression /// /// Creates a routing from the regular expression to match against the path @@ -80,13 +83,13 @@ namespace mtconnect::sink::rest_sink { /// @param[in] swagger `true` if swagger related Routing(boost::beast::http::verb verb, const std::regex &pattern, const Function function, bool swagger = false, std::optional request = std::nullopt) - : m_verb(verb), - m_pattern(pattern), - m_command(request), - m_function(function), - m_swagger(swagger) + : m_verb(verb), + m_pattern(pattern), + m_command(request), + m_function(function), + m_swagger(swagger) {} - + /// @brief Added summary and description to the routing /// @param[in] summary optional summary /// @param[in] description optional description of the routing @@ -97,7 +100,7 @@ namespace mtconnect::sink::rest_sink { m_description = description; return *this; } - + /// @brief Added summary and description to the routing /// @param[in] summary optional summary /// @param[in] description optional description of the routing @@ -127,13 +130,13 @@ namespace mtconnect::sink::rest_sink { } } } - + if (param != nullptr) param->m_description = description; - + return *this; } - + /// @brief Document using common parameter documentation /// @param[in] docs common documentation for parameters Routing &documentParameters(const ParameterDocList &docs) @@ -144,20 +147,20 @@ namespace mtconnect::sink::rest_sink { } return *this; } - + /// @brief Get the description of the REST call for Swagger /// @returns optional string if description is givem const auto &getDescription() const { return m_description; } /// @brief Get the brief summary fo the REST call for Swagger /// @returns optional string if summary is givem const auto &getSummary() const { return m_summary; } - + /// @brief Get the list of path position in order /// @return the parameter list const ParameterList &getPathParameters() const { return m_pathParameters; } /// @brief get the unordered set of query parameters const QuerySet &getQueryParameters() const { return m_queryParameters; } - + /// @brief run the session's request if this routing matches /// /// Call the associated lambda when matched @@ -172,7 +175,7 @@ namespace mtconnect::sink::rest_sink { else return false; } - + /// @brief check if the routing matches the request /// /// @param[in] session the session making the request to pass to the Routing if matched @@ -202,7 +205,7 @@ namespace mtconnect::sink::rest_sink { s++; } } - + entity::EntityList errors; for (auto &p : m_queryParameters) { @@ -217,7 +220,7 @@ namespace mtconnect::sink::rest_sink { catch (ParameterError &e) { std::string msg = std::string("query parameter '") + p.m_name + "': " + e.what(); - + LOG(warning) << "Parameter error: " << msg; auto error = InvalidParameterValue::make(p.m_name, q->second, p.getTypeName(), p.getTypeFormat(), msg); @@ -229,10 +232,10 @@ namespace mtconnect::sink::rest_sink { request->m_parameters.emplace(make_pair(p.m_name, p.m_default)); } } - + if (!errors.empty()) throw RestError(errors, request->m_accepts); - + return true; } else @@ -241,7 +244,7 @@ namespace mtconnect::sink::rest_sink { } } } - + /// @brief Validate the request parameters without matching the path /// @param[in] session the session making the request to pass to the Routing if matched /// @param[in,out] request the incoming request with a verb and a path @@ -259,7 +262,7 @@ namespace mtconnect::sink::rest_sink { if (!validateValueType(p.m_type, it->second)) { std::string msg = std::string("path parameter '") + p.m_name + - "': invalid type, expected " + p.getTypeFormat(); + "': invalid type, expected " + p.getTypeFormat(); LOG(warning) << "Parameter error: " << msg; auto error = InvalidParameterValue::make(p.m_name, Parameter::toString(it->second), p.getTypeName(), p.getTypeFormat(), msg); @@ -267,7 +270,7 @@ namespace mtconnect::sink::rest_sink { } } } - + for (auto &p : m_queryParameters) { auto it = request->m_parameters.find(p.m_name); @@ -276,7 +279,7 @@ namespace mtconnect::sink::rest_sink { if (!validateValueType(p.m_type, it->second)) { std::string msg = std::string("query parameter '") + p.m_name + - "': invalid type, expected " + p.getTypeFormat(); + "': invalid type, expected " + p.getTypeFormat(); LOG(warning) << "Parameter error: " << msg; auto error = InvalidParameterValue::make(p.m_name, Parameter::toString(it->second), p.getTypeName(), p.getTypeFormat(), msg); @@ -288,13 +291,13 @@ namespace mtconnect::sink::rest_sink { request->m_parameters.emplace(make_pair(p.m_name, p.m_default)); } } - + if (!errors.empty()) throw RestError(errors, request->m_accepts); - + return true; } - + /// @brief check if the routing's path pattern matches a given path (ignoring verb) /// @param[in] path the request path to test /// @return `true` if the path matches this routing's pattern @@ -303,20 +306,24 @@ namespace mtconnect::sink::rest_sink { std::smatch m; return std::regex_match(path, m, m_pattern); } - + /// @brief check if this is related to a swagger API /// @returns `true` if related to swagger auto isSwagger() const { return m_swagger; } - + /// @brief Get the path component of the routing pattern const auto &getPath() const { return m_path; } /// @brief Get the routing `verb` const auto &getVerb() const { return m_verb; } - + + /// @brief Check if the route is a catch-all (every path segment is a parameter) + /// @returns `true` if all path segments are parameters (e.g. `/{device}`) + auto isCatchAll() const { return m_catchAll; } + /// @brief Get the optional command associated with the routing /// @returns optional routing const auto &getCommand() const { return m_command; } - + /// @brief Sets the command associated with this routing for use with websockets /// @param command the command auto &command(const std::string &command) @@ -324,44 +331,68 @@ namespace mtconnect::sink::rest_sink { m_command = command; return *this; } - + protected: void pathParameters(std::string s) { - std::regex reg("\\{([^}]+)\\}"); - std::smatch match; std::stringstream pat; - - while (regex_search(s, match, reg)) + + using namespace boost::algorithm; + using SplitList = std::list>; + + SplitList parts; + auto pos = s.find_first_not_of('/'); + if (pos != std::string::npos) { - pat << match.prefix() << "([^/]+)"; - m_pathParameters.emplace_back(match[1]); - s = match.suffix().str(); + auto range = boost::make_iterator_range(s.begin() + pos, s.end()); + split(parts, range, [](char c) { return c == '/'; }); + } + + bool hasLiteral = false; + for (auto &p : parts) + { + auto start = p.begin(); + auto end = p.end(); + + pat << "/"; + if (*start == '{' && *(end - 1) == '}') + { + std::string_view param(start + 1, end - 1); + pat << "([^/]+)"; + m_pathParameters.emplace_back(param); + } + else + { + pat << std::string_view(start, end); + hasLiteral = true; + } } - pat << s; pat << "/?"; - + m_patternText = pat.str(); m_pattern = std::regex(m_patternText); + + // A route is catch-all if it has parameters but no literal path segments + m_catchAll = !m_pathParameters.empty() && !hasLiteral; } - + void queryParameters(std::string s) { std::regex reg("([^=]+)=\\{([^}]+)\\}&?"); std::smatch match; - + while (regex_search(s, match, reg)) { Parameter qp(match[1]); qp.m_part = QUERY; - + getTypeAndDefault(match[2], qp); - + m_queryParameters.emplace(qp); s = match.suffix().str(); } } - + void getTypeAndDefault(const std::string &type, Parameter &par) { std::string t(type); @@ -372,7 +403,7 @@ namespace mtconnect::sink::rest_sink { def = t.substr(dp + 1); t.erase(dp); } - + if (t == "string") { par.m_type = STRING; @@ -393,23 +424,23 @@ namespace mtconnect::sink::rest_sink { { par.m_type = BOOL; } - + if (!def.empty()) { par.m_default = convertValue(def, par.m_type); } } - + ParameterValue convertValue(const std::string &s, ParameterType t) const { switch (t) { case STRING: return s; - + case NONE: throw ParameterError("Cannot convert to NONE"); - + case DOUBLE: { char *ep = nullptr; @@ -419,7 +450,7 @@ namespace mtconnect::sink::rest_sink { throw ParameterError("cannot convert string '" + s + "' to double"); return r; } - + case INTEGER: { char *ep = nullptr; @@ -427,10 +458,10 @@ namespace mtconnect::sink::rest_sink { int32_t r = int32_t(strtoll(sp, &ep, 10)); if (ep == sp) throw ParameterError("cannot convert string '" + s + "' to integer"); - + return r; } - + case UNSIGNED_INTEGER: { char *ep = nullptr; @@ -438,39 +469,39 @@ namespace mtconnect::sink::rest_sink { uint64_t r = strtoull(sp, &ep, 10); if (ep == sp) throw ParameterError("cannot convert string '" + s + "' to unsigned integer"); - + return r; } - + case BOOL: { return bool(s == "true" || s == "yes"); } } - + throw ParameterError("Unknown type for conversion: " + std::to_string(int(t))); - + return ParameterValue(); } - + bool validateValueType(ParameterType t, ParameterValue &value) { switch (t) { case STRING: return std::holds_alternative(value); - + case NONE: return std::holds_alternative(value); - + case DOUBLE: if (std::holds_alternative(value)) value = double(std::get(value)); else if (std::holds_alternative(value)) value = double(std::get(value)); - + return std::holds_alternative(value); - + case INTEGER: if (std::holds_alternative(value)) { @@ -486,7 +517,7 @@ namespace mtconnect::sink::rest_sink { value = int32_t(v); } return std::holds_alternative(value); - + case UNSIGNED_INTEGER: if (std::holds_alternative(value)) { @@ -501,13 +532,13 @@ namespace mtconnect::sink::rest_sink { value = uint64_t(v); } return std::holds_alternative(value); - + case BOOL: return std::holds_alternative(value); } return false; } - + protected: boost::beast::http::verb m_verb; std::regex m_pattern; @@ -517,10 +548,11 @@ namespace mtconnect::sink::rest_sink { QuerySet m_queryParameters; std::optional m_command; Function m_function; - + std::optional m_summary; std::optional m_description; - + bool m_swagger = false; + bool m_catchAll = false; }; } // namespace mtconnect::sink::rest_sink diff --git a/src/mtconnect/sink/rest_sink/server.cpp b/src/mtconnect/sink/rest_sink/server.cpp index e531c444b..872224fe8 100644 --- a/src/mtconnect/sink/rest_sink/server.cpp +++ b/src/mtconnect/sink/rest_sink/server.cpp @@ -420,14 +420,25 @@ namespace mtconnect::sink::rest_sink { using namespace boost; using namespace adaptors; auto handler = [this](SessionPtr session, const RequestPtr request) -> bool { - // Collect the set of HTTP verbs supported at this path - set verbs; + // Collect the set of HTTP verbs supported at this path, preferring + // specific routes over catch-all ones (routes where every segment is + // a parameter, e.g. /{device}). + set specificVerbs; + set catchAllVerbs; for (const auto &r : m_routings) { if (!r.isSwagger() && r.matchesPath(request->m_path)) - verbs.insert(r.getVerb()); + { + if (r.isCatchAll()) + catchAllVerbs.insert(r.getVerb()); + else + specificVerbs.insert(r.getVerb()); + } } + // If any specific route matched, use only those; otherwise fall back to catch-alls + auto &verbs = specificVerbs.empty() ? catchAllVerbs : specificVerbs; + // OPTIONS is always allowed verbs.insert(http::verb::options); diff --git a/src/mtconnect/sink/rest_sink/session_impl.cpp b/src/mtconnect/sink/rest_sink/session_impl.cpp index a17cd04a9..f188dd4db 100644 --- a/src/mtconnect/sink/rest_sink/session_impl.cpp +++ b/src/mtconnect/sink/rest_sink/session_impl.cpp @@ -200,7 +200,8 @@ namespace mtconnect::sink::rest_sink { const auto &remote = m_remote; // Check for put, post, or delete (allow OPTIONS for CORS preflight) - if (msg.method() != http::verb::get && msg.method() != http::verb::options) + if (msg.method() == http::verb::put || msg.method() == http::verb::post || + msg.method() == http::verb::delete_) { if (!m_allowPuts) { diff --git a/test_package/http_server_test.cpp b/test_package/http_server_test.cpp index e3c3eb5de..3a125c00f 100644 --- a/test_package/http_server_test.cpp +++ b/test_package/http_server_test.cpp @@ -759,6 +759,85 @@ TEST_F(HttpServerTest, options_returns_get_put_and_delete_when_registered) EXPECT_NE(string::npos, allow->second.find("DELETE")); } +TEST_F(HttpServerTest, options_returns_get_when_a_specific_and_wildcard_route_are_given) +{ + auto getHandler = [&](SessionPtr session, RequestPtr request) -> bool { + ResponsePtr resp = make_unique(status::ok, "Done"); + session->writeResponse(std::move(resp)); + return true; + }; + auto putHandler = [&](SessionPtr session, RequestPtr request) -> bool { + ResponsePtr resp = make_unique(status::ok, "Put ok"); + session->writeResponse(std::move(resp)); + return true; + }; + auto deleteHandler = [&](SessionPtr session, RequestPtr request) -> bool { + ResponsePtr resp = make_unique(status::ok, "Deleted"); + session->writeResponse(std::move(resp)); + return true; + }; + + m_server->addRouting({http::verb::get, "/current", getHandler}); + m_server->addRouting({http::verb::put, "/{device}?timestamp={timestamp}", putHandler}); + m_server->addRouting({http::verb::delete_, "/{device}?timestamp={timestamp}", deleteHandler}); + m_server->allowPuts(); + + start(); + startClient(); + + m_client->spawnRequest(http::verb::options, "/current"); + ASSERT_TRUE(m_client->m_done); + EXPECT_EQ(int(http::status::no_content), m_client->m_status); + + auto allow = m_client->m_fields.find("Allow"); + ASSERT_NE(m_client->m_fields.end(), allow); + EXPECT_NE(string::npos, allow->second.find("GET")); + EXPECT_NE(string::npos, allow->second.find("OPTIONS")); + EXPECT_EQ(string::npos, allow->second.find("PUT")); + EXPECT_EQ(string::npos, allow->second.find("POST")); + EXPECT_EQ(string::npos, allow->second.find("DELETE")); +} + +TEST_F(HttpServerTest, options_returns_get_when_complex_path_route_are_given) +{ + auto getHandler = [&](SessionPtr session, RequestPtr request) -> bool { + ResponsePtr resp = make_unique(status::ok, "Done"); + session->writeResponse(std::move(resp)); + return true; + }; + auto putHandler = [&](SessionPtr session, RequestPtr request) -> bool { + ResponsePtr resp = make_unique(status::ok, "Put ok"); + session->writeResponse(std::move(resp)); + return true; + }; + auto deleteHandler = [&](SessionPtr session, RequestPtr request) -> bool { + ResponsePtr resp = make_unique(status::ok, "Deleted"); + session->writeResponse(std::move(resp)); + return true; + }; + + m_server->addRouting({http::verb::get, "/{device}/current", getHandler}); + m_server->addRouting({http::verb::put, "/{device}/{command}?timestamp={timestamp}", putHandler}); + m_server->addRouting({http::verb::delete_, "/{device}/{command}?timestamp={timestamp}", deleteHandler}); + m_server->allowPuts(); + + start(); + startClient(); + + m_client->spawnRequest(http::verb::options, "/mydevice/current"); + ASSERT_TRUE(m_client->m_done); + EXPECT_EQ(int(http::status::no_content), m_client->m_status); + + auto allow = m_client->m_fields.find("Allow"); + ASSERT_NE(m_client->m_fields.end(), allow); + EXPECT_NE(string::npos, allow->second.find("GET")); + EXPECT_NE(string::npos, allow->second.find("OPTIONS")); + EXPECT_EQ(string::npos, allow->second.find("PUT")); + EXPECT_EQ(string::npos, allow->second.find("POST")); + EXPECT_EQ(string::npos, allow->second.find("DELETE")); +} + + TEST_F(HttpServerTest, options_allowed_even_when_puts_disabled) { auto handler = [&](SessionPtr session, RequestPtr request) -> bool { From d4edd47cfccde507d36542bc825ff45601131579 Mon Sep 17 00:00:00 2001 From: Will Sobel Date: Thu, 26 Mar 2026 11:46:38 +0100 Subject: [PATCH 04/15] Fixed some of the error messages --- src/mtconnect/configuration/agent_config.cpp | 3 +- .../configuration/coordinate_systems.cpp | 3 +- .../device_model/configuration/motion.cpp | 3 +- .../configuration/solid_model.cpp | 11 +- src/mtconnect/pipeline/shdr_token_mapper.cpp | 4 +- src/mtconnect/sink/rest_sink/parameter.hpp | 2 +- src/mtconnect/sink/rest_sink/routing.hpp | 149 +++++++++--------- src/mtconnect/sink/rest_sink/server.cpp | 4 +- .../source/adapter/shdr/connector.cpp | 24 +-- test_package/http_server_test.cpp | 20 +-- test_package/routing_test.cpp | 1 - 11 files changed, 113 insertions(+), 111 deletions(-) diff --git a/src/mtconnect/configuration/agent_config.cpp b/src/mtconnect/configuration/agent_config.cpp index ab63fa019..1824dd36e 100644 --- a/src/mtconnect/configuration/agent_config.cpp +++ b/src/mtconnect/configuration/agent_config.cpp @@ -1248,7 +1248,8 @@ namespace mtconnect::configuration { } catch (exception &e) { - LOG(info) << "Cannot load plugin " << name << " from " << path << " Reason: " << e.what(); + LOG(debug) << "Plugin " << name << " from " << path << " not found, Reason: " << e.what() + << ", trying next path if available."; } } diff --git a/src/mtconnect/device_model/configuration/coordinate_systems.cpp b/src/mtconnect/device_model/configuration/coordinate_systems.cpp index dcd0007bf..b3fbd47fa 100644 --- a/src/mtconnect/device_model/configuration/coordinate_systems.cpp +++ b/src/mtconnect/device_model/configuration/coordinate_systems.cpp @@ -33,7 +33,8 @@ namespace mtconnect { Requirement("Rotation", ValueType::VECTOR, 3, false), Requirement("TranslationDataSet", ValueType::DATA_SET, false), Requirement("RotationDataSet", ValueType::DATA_SET, false)}); - transformation->setOrder({"Translation", "TranslationDataSet", "Rotation", "RotationDataSet"}); + transformation->setOrder( + {"Translation", "TranslationDataSet", "Rotation", "RotationDataSet"}); auto coordinateSystem = make_shared(Requirements { Requirement("id", true), Requirement("name", false), Requirement("nativeName", false), diff --git a/src/mtconnect/device_model/configuration/motion.cpp b/src/mtconnect/device_model/configuration/motion.cpp index bac6f33f3..7a3d60b22 100644 --- a/src/mtconnect/device_model/configuration/motion.cpp +++ b/src/mtconnect/device_model/configuration/motion.cpp @@ -30,7 +30,8 @@ namespace mtconnect { Requirement("Rotation", ValueType::VECTOR, 3, false), Requirement("TranslationDataSet", ValueType::DATA_SET, false), Requirement("RotationDataSet", ValueType::DATA_SET, false)}); - transformation->setOrder({"Translation", "TranslationDataSet", "Rotation", "RotationDataSet"}); + transformation->setOrder( + {"Translation", "TranslationDataSet", "Rotation", "RotationDataSet"}); static auto motion = make_shared(Requirements { Requirement("id", true), Requirement("parentIdRef", false), diff --git a/src/mtconnect/device_model/configuration/solid_model.cpp b/src/mtconnect/device_model/configuration/solid_model.cpp index 76a5fd662..8d3e4e36c 100644 --- a/src/mtconnect/device_model/configuration/solid_model.cpp +++ b/src/mtconnect/device_model/configuration/solid_model.cpp @@ -29,11 +29,12 @@ namespace mtconnect { if (!solidModel) { static auto transformation = make_shared( - Requirements {Requirement("Translation", ValueType::VECTOR, 3, false), - Requirement("Rotation", ValueType::VECTOR, 3, false), - Requirement("TranslationDataSet", ValueType::DATA_SET, false), - Requirement("RotationDataSet", ValueType::DATA_SET, false)}); - transformation->setOrder({"Translation", "TranslationDataSet", "Rotation", "RotationDataSet"}); + Requirements {Requirement("Translation", ValueType::VECTOR, 3, false), + Requirement("Rotation", ValueType::VECTOR, 3, false), + Requirement("TranslationDataSet", ValueType::DATA_SET, false), + Requirement("RotationDataSet", ValueType::DATA_SET, false)}); + transformation->setOrder( + {"Translation", "TranslationDataSet", "Rotation", "RotationDataSet"}); solidModel = make_shared( Requirements {{"id", true}, diff --git a/src/mtconnect/pipeline/shdr_token_mapper.cpp b/src/mtconnect/pipeline/shdr_token_mapper.cpp index efef18c9f..3baf14f3b 100644 --- a/src/mtconnect/pipeline/shdr_token_mapper.cpp +++ b/src/mtconnect/pipeline/shdr_token_mapper.cpp @@ -166,8 +166,8 @@ namespace mtconnect { } catch (entity::PropertyError &e) { - LOG(warning) << "Cannot convert value for data item id '" << dataItem->getId() - << "': " << *token << " - " << e.what(); + LOG(debug) << "Cannot convert value for data item id '" << dataItem->getId() + << "': " << *token << " - " << e.what(); if (schemaVersion >= SCHEMA_VERSION(2, 5) && validation) { props.insert_or_assign("quality", "INVALID"s); diff --git a/src/mtconnect/sink/rest_sink/parameter.hpp b/src/mtconnect/sink/rest_sink/parameter.hpp index e09c42d9a..e7348d641 100644 --- a/src/mtconnect/sink/rest_sink/parameter.hpp +++ b/src/mtconnect/sink/rest_sink/parameter.hpp @@ -65,7 +65,7 @@ namespace mtconnect::sink::rest_sink { : m_name(n), m_type(t), m_part(p) {} Parameter(const std::string_view &n, ParameterType t = STRING, UrlPart p = PATH) - : m_name(n), m_type(t), m_part(p) + : m_name(n), m_type(t), m_part(p) {} Parameter(const Parameter &o) = default; diff --git a/src/mtconnect/sink/rest_sink/routing.hpp b/src/mtconnect/sink/rest_sink/routing.hpp index c5c7fe0ad..d9aafc4f7 100644 --- a/src/mtconnect/sink/rest_sink/routing.hpp +++ b/src/mtconnect/sink/rest_sink/routing.hpp @@ -17,10 +17,10 @@ #pragma once -#include #include +#include - +#include #include #include #include @@ -28,7 +28,6 @@ #include #include #include -#include #include "mtconnect/config.hpp" #include "mtconnect/logging.hpp" @@ -39,14 +38,14 @@ namespace mtconnect::sink::rest_sink { class Session; using SessionPtr = std::shared_ptr; - + /// @brief A REST routing that parses a URI pattern and associates a lambda when it is matched /// against a request class AGENT_LIB_API Routing { public: using Function = std::function; - + Routing(const Routing &r) = default; /// @brief Create a routing with a string /// @@ -57,23 +56,23 @@ namespace mtconnect::sink::rest_sink { /// @param[in] swagger `true` if swagger related Routing(boost::beast::http::verb verb, const std::string &pattern, const Function function, bool swagger = false, std::optional request = std::nullopt) - : m_verb(verb), m_command(request), m_function(function), m_swagger(swagger) + : m_verb(verb), m_command(request), m_function(function), m_swagger(swagger) { std::string s(pattern); - + auto qp = s.find_first_of('?'); if (qp != std::string::npos) { auto query = s.substr(qp + 1); s.erase(qp); - + queryParameters(query); } - + m_path.emplace(s); pathParameters(s); } - + /// @brief Create a routing with a regular expression /// /// Creates a routing from the regular expression to match against the path @@ -83,13 +82,13 @@ namespace mtconnect::sink::rest_sink { /// @param[in] swagger `true` if swagger related Routing(boost::beast::http::verb verb, const std::regex &pattern, const Function function, bool swagger = false, std::optional request = std::nullopt) - : m_verb(verb), - m_pattern(pattern), - m_command(request), - m_function(function), - m_swagger(swagger) + : m_verb(verb), + m_pattern(pattern), + m_command(request), + m_function(function), + m_swagger(swagger) {} - + /// @brief Added summary and description to the routing /// @param[in] summary optional summary /// @param[in] description optional description of the routing @@ -100,7 +99,7 @@ namespace mtconnect::sink::rest_sink { m_description = description; return *this; } - + /// @brief Added summary and description to the routing /// @param[in] summary optional summary /// @param[in] description optional description of the routing @@ -130,13 +129,13 @@ namespace mtconnect::sink::rest_sink { } } } - + if (param != nullptr) param->m_description = description; - + return *this; } - + /// @brief Document using common parameter documentation /// @param[in] docs common documentation for parameters Routing &documentParameters(const ParameterDocList &docs) @@ -147,20 +146,20 @@ namespace mtconnect::sink::rest_sink { } return *this; } - + /// @brief Get the description of the REST call for Swagger /// @returns optional string if description is givem const auto &getDescription() const { return m_description; } /// @brief Get the brief summary fo the REST call for Swagger /// @returns optional string if summary is givem const auto &getSummary() const { return m_summary; } - + /// @brief Get the list of path position in order /// @return the parameter list const ParameterList &getPathParameters() const { return m_pathParameters; } /// @brief get the unordered set of query parameters const QuerySet &getQueryParameters() const { return m_queryParameters; } - + /// @brief run the session's request if this routing matches /// /// Call the associated lambda when matched @@ -175,7 +174,7 @@ namespace mtconnect::sink::rest_sink { else return false; } - + /// @brief check if the routing matches the request /// /// @param[in] session the session making the request to pass to the Routing if matched @@ -205,7 +204,7 @@ namespace mtconnect::sink::rest_sink { s++; } } - + entity::EntityList errors; for (auto &p : m_queryParameters) { @@ -220,7 +219,7 @@ namespace mtconnect::sink::rest_sink { catch (ParameterError &e) { std::string msg = std::string("query parameter '") + p.m_name + "': " + e.what(); - + LOG(warning) << "Parameter error: " << msg; auto error = InvalidParameterValue::make(p.m_name, q->second, p.getTypeName(), p.getTypeFormat(), msg); @@ -232,10 +231,10 @@ namespace mtconnect::sink::rest_sink { request->m_parameters.emplace(make_pair(p.m_name, p.m_default)); } } - + if (!errors.empty()) throw RestError(errors, request->m_accepts); - + return true; } else @@ -244,7 +243,7 @@ namespace mtconnect::sink::rest_sink { } } } - + /// @brief Validate the request parameters without matching the path /// @param[in] session the session making the request to pass to the Routing if matched /// @param[in,out] request the incoming request with a verb and a path @@ -262,7 +261,7 @@ namespace mtconnect::sink::rest_sink { if (!validateValueType(p.m_type, it->second)) { std::string msg = std::string("path parameter '") + p.m_name + - "': invalid type, expected " + p.getTypeFormat(); + "': invalid type, expected " + p.getTypeFormat(); LOG(warning) << "Parameter error: " << msg; auto error = InvalidParameterValue::make(p.m_name, Parameter::toString(it->second), p.getTypeName(), p.getTypeFormat(), msg); @@ -270,7 +269,7 @@ namespace mtconnect::sink::rest_sink { } } } - + for (auto &p : m_queryParameters) { auto it = request->m_parameters.find(p.m_name); @@ -279,7 +278,7 @@ namespace mtconnect::sink::rest_sink { if (!validateValueType(p.m_type, it->second)) { std::string msg = std::string("query parameter '") + p.m_name + - "': invalid type, expected " + p.getTypeFormat(); + "': invalid type, expected " + p.getTypeFormat(); LOG(warning) << "Parameter error: " << msg; auto error = InvalidParameterValue::make(p.m_name, Parameter::toString(it->second), p.getTypeName(), p.getTypeFormat(), msg); @@ -291,13 +290,13 @@ namespace mtconnect::sink::rest_sink { request->m_parameters.emplace(make_pair(p.m_name, p.m_default)); } } - + if (!errors.empty()) throw RestError(errors, request->m_accepts); - + return true; } - + /// @brief check if the routing's path pattern matches a given path (ignoring verb) /// @param[in] path the request path to test /// @return `true` if the path matches this routing's pattern @@ -306,24 +305,24 @@ namespace mtconnect::sink::rest_sink { std::smatch m; return std::regex_match(path, m, m_pattern); } - + /// @brief check if this is related to a swagger API /// @returns `true` if related to swagger auto isSwagger() const { return m_swagger; } - + /// @brief Get the path component of the routing pattern const auto &getPath() const { return m_path; } /// @brief Get the routing `verb` const auto &getVerb() const { return m_verb; } - + /// @brief Check if the route is a catch-all (every path segment is a parameter) /// @returns `true` if all path segments are parameters (e.g. `/{device}`) auto isCatchAll() const { return m_catchAll; } - + /// @brief Get the optional command associated with the routing /// @returns optional routing const auto &getCommand() const { return m_command; } - + /// @brief Sets the command associated with this routing for use with websockets /// @param command the command auto &command(const std::string &command) @@ -331,15 +330,15 @@ namespace mtconnect::sink::rest_sink { m_command = command; return *this; } - + protected: void pathParameters(std::string s) { std::stringstream pat; - + using namespace boost::algorithm; using SplitList = std::list>; - + SplitList parts; auto pos = s.find_first_not_of('/'); if (pos != std::string::npos) @@ -347,17 +346,17 @@ namespace mtconnect::sink::rest_sink { auto range = boost::make_iterator_range(s.begin() + pos, s.end()); split(parts, range, [](char c) { return c == '/'; }); } - + bool hasLiteral = false; for (auto &p : parts) { auto start = p.begin(); auto end = p.end(); - + pat << "/"; if (*start == '{' && *(end - 1) == '}') { - std::string_view param(start + 1, end - 1); + std::string_view param(start + 1, end - 1); pat << "([^/]+)"; m_pathParameters.emplace_back(param); } @@ -368,31 +367,31 @@ namespace mtconnect::sink::rest_sink { } } pat << "/?"; - + m_patternText = pat.str(); m_pattern = std::regex(m_patternText); - + // A route is catch-all if it has parameters but no literal path segments m_catchAll = !m_pathParameters.empty() && !hasLiteral; } - + void queryParameters(std::string s) { std::regex reg("([^=]+)=\\{([^}]+)\\}&?"); std::smatch match; - + while (regex_search(s, match, reg)) { Parameter qp(match[1]); qp.m_part = QUERY; - + getTypeAndDefault(match[2], qp); - + m_queryParameters.emplace(qp); s = match.suffix().str(); } } - + void getTypeAndDefault(const std::string &type, Parameter &par) { std::string t(type); @@ -403,7 +402,7 @@ namespace mtconnect::sink::rest_sink { def = t.substr(dp + 1); t.erase(dp); } - + if (t == "string") { par.m_type = STRING; @@ -424,23 +423,23 @@ namespace mtconnect::sink::rest_sink { { par.m_type = BOOL; } - + if (!def.empty()) { par.m_default = convertValue(def, par.m_type); } } - + ParameterValue convertValue(const std::string &s, ParameterType t) const { switch (t) { case STRING: return s; - + case NONE: throw ParameterError("Cannot convert to NONE"); - + case DOUBLE: { char *ep = nullptr; @@ -450,7 +449,7 @@ namespace mtconnect::sink::rest_sink { throw ParameterError("cannot convert string '" + s + "' to double"); return r; } - + case INTEGER: { char *ep = nullptr; @@ -458,10 +457,10 @@ namespace mtconnect::sink::rest_sink { int32_t r = int32_t(strtoll(sp, &ep, 10)); if (ep == sp) throw ParameterError("cannot convert string '" + s + "' to integer"); - + return r; } - + case UNSIGNED_INTEGER: { char *ep = nullptr; @@ -469,39 +468,39 @@ namespace mtconnect::sink::rest_sink { uint64_t r = strtoull(sp, &ep, 10); if (ep == sp) throw ParameterError("cannot convert string '" + s + "' to unsigned integer"); - + return r; } - + case BOOL: { return bool(s == "true" || s == "yes"); } } - + throw ParameterError("Unknown type for conversion: " + std::to_string(int(t))); - + return ParameterValue(); } - + bool validateValueType(ParameterType t, ParameterValue &value) { switch (t) { case STRING: return std::holds_alternative(value); - + case NONE: return std::holds_alternative(value); - + case DOUBLE: if (std::holds_alternative(value)) value = double(std::get(value)); else if (std::holds_alternative(value)) value = double(std::get(value)); - + return std::holds_alternative(value); - + case INTEGER: if (std::holds_alternative(value)) { @@ -517,7 +516,7 @@ namespace mtconnect::sink::rest_sink { value = int32_t(v); } return std::holds_alternative(value); - + case UNSIGNED_INTEGER: if (std::holds_alternative(value)) { @@ -532,13 +531,13 @@ namespace mtconnect::sink::rest_sink { value = uint64_t(v); } return std::holds_alternative(value); - + case BOOL: return std::holds_alternative(value); } return false; } - + protected: boost::beast::http::verb m_verb; std::regex m_pattern; @@ -548,10 +547,10 @@ namespace mtconnect::sink::rest_sink { QuerySet m_queryParameters; std::optional m_command; Function m_function; - + std::optional m_summary; std::optional m_description; - + bool m_swagger = false; bool m_catchAll = false; }; diff --git a/src/mtconnect/sink/rest_sink/server.cpp b/src/mtconnect/sink/rest_sink/server.cpp index 872224fe8..a6ec1570d 100644 --- a/src/mtconnect/sink/rest_sink/server.cpp +++ b/src/mtconnect/sink/rest_sink/server.cpp @@ -19,6 +19,7 @@ #include #include +#include #include #include #include @@ -26,9 +27,8 @@ #include #include #include -#include #include -#include +#include #include diff --git a/src/mtconnect/source/adapter/shdr/connector.cpp b/src/mtconnect/source/adapter/shdr/connector.cpp index 551b79a5f..012e422c1 100644 --- a/src/mtconnect/source/adapter/shdr/connector.cpp +++ b/src/mtconnect/source/adapter/shdr/connector.cpp @@ -98,9 +98,9 @@ namespace mtconnect::source::adapter::shdr { if (ec) { - LOG(error) << "Cannot resolve address: " << m_server << ":" << m_port; - LOG(error) << ec.category().message(ec.value()) << ": " << ec.message(); - LOG(error) << "Will retry resolution of " << m_server << " in " << m_reconnectInterval.count() + LOG(warning) << "Cannot resolve address: " << m_server << ":" << m_port; + LOG(warning) << ec.message(); + LOG(warning) << "Will retry resolution of " << m_server << " in " << m_reconnectInterval.count() << " milliseconds"; m_timer.expires_after(m_reconnectInterval); @@ -136,8 +136,9 @@ namespace mtconnect::source::adapter::shdr { return true; } - /// @brief Attempt to reconnect after a delay. If the server is a hostname, re-resolve it to get the current IP - /// address in case it has changed. If the server is a static IP address, just reconnect. + /// @brief Attempt to reconnect after a delay. If the server is a hostname, re-resolve it to get + /// the current IP address in case it has changed. If the server is a static IP address, just + /// reconnect. inline void Connector::asyncTryConnect() { NAMED_SCOPE("Connector::asyncTryConnect"); @@ -203,7 +204,7 @@ namespace mtconnect::source::adapter::shdr { auto remote = m_socket.remote_endpoint(rec); if (rec) { - LOG(error) << "Failed to get remote endpoint: " << rec.message(); + LOG(warning) << "Failed to get remote endpoint: " << rec.message(); } else { @@ -233,7 +234,7 @@ namespace mtconnect::source::adapter::shdr { if (ec) { - LOG(error) << ec.category().message(ec.value()) << ": " << ec.message(); + LOG(error) << ec.message(); reconnect(); } else @@ -271,7 +272,7 @@ namespace mtconnect::source::adapter::shdr { if (ec) { - LOG(error) << ec.category().message(ec.value()) << ": " << ec.message(); + LOG(error) << ec.message(); reconnect(); } } @@ -292,15 +293,14 @@ namespace mtconnect::source::adapter::shdr { m_receiveTimeout.async_wait([this](sys::error_code ec) { if (!ec) { - LOG(error) << "(Port:" << m_localPort << ")" + LOG(warning) << "(Port:" << m_localPort << ")" << " connect: Did not receive data for over: " << m_receiveTimeLimit.count() << " ms"; asio::dispatch(m_strand, boost::bind(&Connector::reconnect, this)); } else if (ec != boost::asio::error::operation_aborted) { - LOG(error) << "Receive timeout: " << ec.category().message(ec.value()) << ": " - << ec.message(); + LOG(error) << "Receive timeout: " << ec.message(); } }); } @@ -413,7 +413,7 @@ namespace mtconnect::source::adapter::shdr { } else if (ec != boost::asio::error::operation_aborted) { - LOG(error) << "heartbeat: " << ec.category().message(ec.value()) << ": " << ec.message(); + LOG(error) << "heartbeat: " << ec.message(); } } diff --git a/test_package/http_server_test.cpp b/test_package/http_server_test.cpp index 3a125c00f..67eeeb951 100644 --- a/test_package/http_server_test.cpp +++ b/test_package/http_server_test.cpp @@ -776,19 +776,19 @@ TEST_F(HttpServerTest, options_returns_get_when_a_specific_and_wildcard_route_ar session->writeResponse(std::move(resp)); return true; }; - + m_server->addRouting({http::verb::get, "/current", getHandler}); m_server->addRouting({http::verb::put, "/{device}?timestamp={timestamp}", putHandler}); m_server->addRouting({http::verb::delete_, "/{device}?timestamp={timestamp}", deleteHandler}); m_server->allowPuts(); - + start(); startClient(); - + m_client->spawnRequest(http::verb::options, "/current"); ASSERT_TRUE(m_client->m_done); EXPECT_EQ(int(http::status::no_content), m_client->m_status); - + auto allow = m_client->m_fields.find("Allow"); ASSERT_NE(m_client->m_fields.end(), allow); EXPECT_NE(string::npos, allow->second.find("GET")); @@ -815,19 +815,20 @@ TEST_F(HttpServerTest, options_returns_get_when_complex_path_route_are_given) session->writeResponse(std::move(resp)); return true; }; - + m_server->addRouting({http::verb::get, "/{device}/current", getHandler}); m_server->addRouting({http::verb::put, "/{device}/{command}?timestamp={timestamp}", putHandler}); - m_server->addRouting({http::verb::delete_, "/{device}/{command}?timestamp={timestamp}", deleteHandler}); + m_server->addRouting( + {http::verb::delete_, "/{device}/{command}?timestamp={timestamp}", deleteHandler}); m_server->allowPuts(); - + start(); startClient(); - + m_client->spawnRequest(http::verb::options, "/mydevice/current"); ASSERT_TRUE(m_client->m_done); EXPECT_EQ(int(http::status::no_content), m_client->m_status); - + auto allow = m_client->m_fields.find("Allow"); ASSERT_NE(m_client->m_fields.end(), allow); EXPECT_NE(string::npos, allow->second.find("GET")); @@ -837,7 +838,6 @@ TEST_F(HttpServerTest, options_returns_get_when_complex_path_route_are_given) EXPECT_EQ(string::npos, allow->second.find("DELETE")); } - TEST_F(HttpServerTest, options_allowed_even_when_puts_disabled) { auto handler = [&](SessionPtr session, RequestPtr request) -> bool { diff --git a/test_package/routing_test.cpp b/test_package/routing_test.cpp index d8fadf5ee..489458af7 100644 --- a/test_package/routing_test.cpp +++ b/test_package/routing_test.cpp @@ -360,4 +360,3 @@ TEST_F(RoutingTest, matchesPath_with_query_parameters_in_pattern) EXPECT_TRUE(r.matchesPath("/device1/sample/")); EXPECT_FALSE(r.matchesPath("/sample")); } - From 744c57716b9eb3a3a6e20522c9ea19c44f17c6c9 Mon Sep 17 00:00:00 2001 From: Will Sobel Date: Thu, 26 Mar 2026 12:55:12 +0100 Subject: [PATCH 05/15] Fixed issues with regex based paths --- src/mtconnect/sink/rest_sink/routing.hpp | 22 ++++++- src/mtconnect/sink/rest_sink/server.cpp | 62 +++++++++---------- src/mtconnect/sink/rest_sink/server.hpp | 22 ++++--- test_package/http_server_test.cpp | 77 ++++++++++++++++++++++++ 4 files changed, 138 insertions(+), 45 deletions(-) diff --git a/src/mtconnect/sink/rest_sink/routing.hpp b/src/mtconnect/sink/rest_sink/routing.hpp index d9aafc4f7..d1e5d8005 100644 --- a/src/mtconnect/sink/rest_sink/routing.hpp +++ b/src/mtconnect/sink/rest_sink/routing.hpp @@ -86,7 +86,8 @@ namespace mtconnect::sink::rest_sink { m_pattern(pattern), m_command(request), m_function(function), - m_swagger(swagger) + m_swagger(swagger), + m_catchAll(true) {} /// @brief Added summary and description to the routing @@ -352,12 +353,27 @@ namespace mtconnect::sink::rest_sink { { auto start = p.begin(); auto end = p.end(); + + auto openBrace = std::find(start, end, '{'); + decltype(openBrace) closeBrace { end }; + if (openBrace != end && std::distance(openBrace, end) > 2) + closeBrace = std::find(openBrace + 1, end, '}'); pat << "/"; - if (*start == '{' && *(end - 1) == '}') + if (openBrace != end && closeBrace != end) { - std::string_view param(start + 1, end - 1); + if (openBrace > start) + { + pat << std::string_view(start, openBrace); + hasLiteral = true; + } + std::string_view param(openBrace + 1, closeBrace); pat << "([^/]+)"; + if (closeBrace + 1 < end) + { + pat << std::string_view(closeBrace + 1, end); + hasLiteral = true; + } m_pathParameters.emplace_back(param); } else diff --git a/src/mtconnect/sink/rest_sink/server.cpp b/src/mtconnect/sink/rest_sink/server.cpp index a6ec1570d..333e73f80 100644 --- a/src/mtconnect/sink/rest_sink/server.cpp +++ b/src/mtconnect/sink/rest_sink/server.cpp @@ -415,50 +415,44 @@ namespace mtconnect::sink::rest_sink { // addRouting({boost::beast::http::verb::get, "/swagger.yaml", handler, true}); } - void Server::addOptionsRouting() + bool Server::handleOptionsRequest(SessionPtr session, const RequestPtr request) { using namespace boost; using namespace adaptors; - auto handler = [this](SessionPtr session, const RequestPtr request) -> bool { - // Collect the set of HTTP verbs supported at this path, preferring - // specific routes over catch-all ones (routes where every segment is - // a parameter, e.g. /{device}). - set specificVerbs; - set catchAllVerbs; - for (const auto &r : m_routings) + set specificVerbs; + set catchAllVerbs; + for (const auto &r : m_routings) + { + if (!r.isSwagger() && r.matchesPath(request->m_path)) { - if (!r.isSwagger() && r.matchesPath(request->m_path)) - { - if (r.isCatchAll()) - catchAllVerbs.insert(r.getVerb()); - else - specificVerbs.insert(r.getVerb()); - } + if (r.isCatchAll()) + catchAllVerbs.insert(r.getVerb()); + else + specificVerbs.insert(r.getVerb()); } + } - // If any specific route matched, use only those; otherwise fall back to catch-alls - auto &verbs = specificVerbs.empty() ? catchAllVerbs : specificVerbs; - - // OPTIONS is always allowed - verbs.insert(http::verb::options); + // If any specific route matched, use only those; otherwise fall back to catch-alls + auto &verbs = specificVerbs.empty() ? catchAllVerbs : specificVerbs; - // Build the Allow / Access-Control-Allow-Methods header value - string methods = algorithm::join( - verbs | transformed([](http::verb v) { return string(http::to_string(v)); }), ", "); + // OPTIONS is always allowed + verbs.insert(http::verb::options); - auto response = std::make_unique(status::no_content, "", "text/plain"); - response->m_close = false; - response->m_fields.emplace_back("Allow", methods); - response->m_fields.emplace_back("Access-Control-Allow-Methods", methods); - response->m_fields.emplace_back("Access-Control-Allow-Headers", - "Content-Type, Accept, Accept-Encoding"); - response->m_fields.emplace_back("Access-Control-Max-Age", "86400"); + // Build the Allow / Access-Control-Allow-Methods header value + string methods = algorithm::join( + verbs | transformed([](http::verb v) { return string(http::to_string(v)); }), ", "); - session->writeResponse(std::move(response)); - return true; - }; + auto response = std::make_unique(status::no_content, "", "text/plain"); + response->m_close = false; + response->m_fields.emplace_back("Allow", methods); + response->m_fields.emplace_back("Access-Control-Allow-Methods", methods); + response->m_fields.emplace_back("Access-Control-Allow-Headers", + "Content-Type, Accept, Accept-Encoding"); + response->m_fields.emplace_back("Access-Control-Max-Age", "86400"); - addRouting({boost::beast::http::verb::options, std::regex("/.*"), handler, true}); + session->writeResponse(std::move(response)); + + return true; } } // namespace mtconnect::sink::rest_sink diff --git a/src/mtconnect/sink/rest_sink/server.hpp b/src/mtconnect/sink/rest_sink/server.hpp index acec63431..73377b428 100644 --- a/src/mtconnect/sink/rest_sink/server.hpp +++ b/src/mtconnect/sink/rest_sink/server.hpp @@ -85,7 +85,6 @@ namespace mtconnect::sink::rest_sink { loadTlsCertificate(); addSwaggerRoutings(); - addOptionsRouting(); } /// @brief Start the http server @@ -174,6 +173,10 @@ namespace mtconnect::sink::rest_sink { else message = "Command failed: " + *request->m_command; } + else if (request->m_verb == boost::beast::http::verb::options) + { + success = handleOptionsRequest(session, request); + } else { for (auto &r : m_routings) @@ -290,13 +293,6 @@ namespace mtconnect::sink::rest_sink { /// /// @brief Add swagger routings to the Agent void addSwaggerRoutings(); - /// @} - - /// @name CORS Support - /// @{ - /// - /// @brief Add OPTIONS routing for CORS preflight requests - void addOptionsRouting(); /// @brief generate swagger API from routings /// @param[in] format The mime format of the response ("json" or "yaml") /// @@ -306,6 +302,16 @@ namespace mtconnect::sink::rest_sink { const void renderSwaggerResponse(T &format); /// @} + /// @name CORS Support + /// @{ + /// + /// @brief Handle OPTIONS request for CORS preflight requests + /// @param[in] session the client session + /// @param[in] request the incoming request + /// @return `true` if the request was handled, otherwise `false` and a 404 will be returned + bool handleOptionsRequest(SessionPtr session, const RequestPtr request); + /// @} + protected: boost::asio::io_context &m_context; diff --git a/test_package/http_server_test.cpp b/test_package/http_server_test.cpp index 67eeeb951..658cb58e2 100644 --- a/test_package/http_server_test.cpp +++ b/test_package/http_server_test.cpp @@ -901,6 +901,83 @@ TEST_F(HttpServerTest, options_includes_configured_cors_origin_header) EXPECT_EQ(string::npos, acam->second.find("DELETE")); } +TEST_F(HttpServerTest, options_returns_correctly_for_path_with_parameter_value) +{ + auto handler = [&](SessionPtr session, RequestPtr request) -> bool { + ResponsePtr resp = make_unique(status::ok, "Done"); + session->writeResponse(std::move(resp)); + return true; + }; + + m_server->addRouting({boost::beast::http::verb::get, "/cancel/id={string}", handler}) + .document("MTConnect WebServices Cancel Stream", "Cancels a streaming sample request") + .command("cancel"); + + start(); + startClient(); + + m_client->spawnRequest(http::verb::options, "/cancel/id=12345"); + ASSERT_TRUE(m_client->m_done); + EXPECT_EQ(int(http::status::no_content), m_client->m_status); + + auto allow = m_client->m_fields.find("Allow"); + ASSERT_NE(m_client->m_fields.end(), allow); + EXPECT_NE(string::npos, allow->second.find("GET")); + EXPECT_NE(string::npos, allow->second.find("OPTIONS")); + EXPECT_EQ(string::npos, allow->second.find("PUT")); + EXPECT_EQ(string::npos, allow->second.find("POST")); + EXPECT_EQ(string::npos, allow->second.find("DELETE")); + + auto acam = m_client->m_fields.find("Access-Control-Allow-Methods"); + ASSERT_NE(m_client->m_fields.end(), acam); + EXPECT_EQ(allow->second, acam->second); + + auto acah = m_client->m_fields.find("Access-Control-Allow-Headers"); + ASSERT_NE(m_client->m_fields.end(), acah); + + auto acma = m_client->m_fields.find("Access-Control-Max-Age"); + ASSERT_NE(m_client->m_fields.end(), acma); + EXPECT_EQ("86400", acma->second); +} + +TEST_F(HttpServerTest, should_handle_routings_with_just_a_regex) +{ + auto handler = [&](SessionPtr session, RequestPtr request) -> bool { + ResponsePtr resp = make_unique(status::ok, "Done"); + session->writeResponse(std::move(resp)); + return true; + }; + + m_server->addRouting({boost::beast::http::verb::get, regex("/.+"), handler}); + m_server->addRouting({http::verb::put, "/{device}?timestamp={timestamp}", handler}); + + start(); + startClient(); + + m_client->spawnRequest(http::verb::options, "/file.xsd"); + ASSERT_TRUE(m_client->m_done); + EXPECT_EQ(int(http::status::no_content), m_client->m_status); + + auto allow = m_client->m_fields.find("Allow"); + ASSERT_NE(m_client->m_fields.end(), allow); + EXPECT_NE(string::npos, allow->second.find("GET")); + EXPECT_NE(string::npos, allow->second.find("OPTIONS")); + EXPECT_NE(string::npos, allow->second.find("PUT")); + EXPECT_EQ(string::npos, allow->second.find("POST")); + EXPECT_EQ(string::npos, allow->second.find("DELETE")); + + auto acam = m_client->m_fields.find("Access-Control-Allow-Methods"); + ASSERT_NE(m_client->m_fields.end(), acam); + EXPECT_EQ(allow->second, acam->second); + + auto acah = m_client->m_fields.find("Access-Control-Allow-Headers"); + ASSERT_NE(m_client->m_fields.end(), acah); + + auto acma = m_client->m_fields.find("Access-Control-Max-Age"); + ASSERT_NE(m_client->m_fields.end(), acma); + EXPECT_EQ("86400", acma->second); +} + const string CertFile(TEST_RESOURCE_DIR "/user.crt"); const string KeyFile {TEST_RESOURCE_DIR "/user.key"}; const string DhFile {TEST_RESOURCE_DIR "/dh2048.pem"}; From 19beae11d840a685359ad55cb489841b68b13a59 Mon Sep 17 00:00:00 2001 From: Will Sobel Date: Fri, 27 Mar 2026 19:31:14 +0100 Subject: [PATCH 06/15] Changed logging for some error messages on load --- src/mtconnect/configuration/agent_config.hpp | 4 +-- src/mtconnect/entity/xml_parser.cpp | 2 +- src/mtconnect/parser/xml_parser.cpp | 16 ++++++++++- src/mtconnect/sink/rest_sink/routing.hpp | 4 +-- src/mtconnect/sink/rest_sink/server.cpp | 2 +- .../source/adapter/shdr/connector.cpp | 8 +++--- test_package/http_server_test.cpp | 28 +++++++++---------- 7 files changed, 39 insertions(+), 25 deletions(-) diff --git a/src/mtconnect/configuration/agent_config.hpp b/src/mtconnect/configuration/agent_config.hpp index 742d1261b..a5ba57a28 100644 --- a/src/mtconnect/configuration/agent_config.hpp +++ b/src/mtconnect/configuration/agent_config.hpp @@ -295,7 +295,7 @@ namespace mtconnect { else { LOG(debug) << "Cannot find file '" << file << "' " - << " in path " << path; + << " in path " << path << ", continuing..."; } } @@ -312,7 +312,7 @@ namespace mtconnect { if (!ec) paths.emplace_back(con); else - LOG(debug) << "Cannot file path: " << path << ", " << ec.message(); + LOG(debug) << "Cannot find path: " << path << ", " << ec.message() << ", skipping..."; } void addPathFront(std::list &paths, std::filesystem::path path) diff --git a/src/mtconnect/entity/xml_parser.cpp b/src/mtconnect/entity/xml_parser.cpp index 53cf80e45..a07877fd4 100644 --- a/src/mtconnect/entity/xml_parser.cpp +++ b/src/mtconnect/entity/xml_parser.cpp @@ -287,7 +287,7 @@ namespace mtconnect::entity { } else { - LOG(warning) << "Unexpected element: " << nodeQName(child); + // LOG(warning) << "Unexpected element: " << nodeQName(child); errors.emplace_back( new EntityError("Invalid element '" + nodeQName(child) + "'", qname)); } diff --git a/src/mtconnect/parser/xml_parser.cpp b/src/mtconnect/parser/xml_parser.cpp index 9734d0ad5..20185ceb2 100644 --- a/src/mtconnect/parser/xml_parser.cpp +++ b/src/mtconnect/parser/xml_parser.cpp @@ -202,13 +202,27 @@ namespace mtconnect::parser { { auto device = entity::XmlParser::parseXmlNode(Device::getRoot(), nodeset->nodeTab[i], errors); + if (device) + { deviceList.emplace_back(dynamic_pointer_cast(device)); + } + else + { + LOG(error) << "Failed to parse device, skipping"; + } if (!errors.empty()) { for (auto &e : errors) - LOG(warning) << "Error parsing device: " << e->what(); + { + if (device) + LOG(warning) << "When loading device " << device->get("name") + << ", A problem was skipped: " + << e->what(); + else + LOG(error) << "Failed to load device: " << e->what(); + } } } } diff --git a/src/mtconnect/sink/rest_sink/routing.hpp b/src/mtconnect/sink/rest_sink/routing.hpp index d1e5d8005..709d13e4f 100644 --- a/src/mtconnect/sink/rest_sink/routing.hpp +++ b/src/mtconnect/sink/rest_sink/routing.hpp @@ -353,9 +353,9 @@ namespace mtconnect::sink::rest_sink { { auto start = p.begin(); auto end = p.end(); - + auto openBrace = std::find(start, end, '{'); - decltype(openBrace) closeBrace { end }; + decltype(openBrace) closeBrace {end}; if (openBrace != end && std::distance(openBrace, end) > 2) closeBrace = std::find(openBrace + 1, end, '}'); diff --git a/src/mtconnect/sink/rest_sink/server.cpp b/src/mtconnect/sink/rest_sink/server.cpp index 333e73f80..4cb1b8d2f 100644 --- a/src/mtconnect/sink/rest_sink/server.cpp +++ b/src/mtconnect/sink/rest_sink/server.cpp @@ -451,7 +451,7 @@ namespace mtconnect::sink::rest_sink { response->m_fields.emplace_back("Access-Control-Max-Age", "86400"); session->writeResponse(std::move(response)); - + return true; } diff --git a/src/mtconnect/source/adapter/shdr/connector.cpp b/src/mtconnect/source/adapter/shdr/connector.cpp index 012e422c1..be774fe60 100644 --- a/src/mtconnect/source/adapter/shdr/connector.cpp +++ b/src/mtconnect/source/adapter/shdr/connector.cpp @@ -100,8 +100,8 @@ namespace mtconnect::source::adapter::shdr { { LOG(warning) << "Cannot resolve address: " << m_server << ":" << m_port; LOG(warning) << ec.message(); - LOG(warning) << "Will retry resolution of " << m_server << " in " << m_reconnectInterval.count() - << " milliseconds"; + LOG(warning) << "Will retry resolution of " << m_server << " in " + << m_reconnectInterval.count() << " milliseconds"; m_timer.expires_after(m_reconnectInterval); m_timer.async_wait([this](boost::system::error_code ec) { @@ -294,8 +294,8 @@ namespace mtconnect::source::adapter::shdr { if (!ec) { LOG(warning) << "(Port:" << m_localPort << ")" - << " connect: Did not receive data for over: " << m_receiveTimeLimit.count() - << " ms"; + << " connect: Did not receive data for over: " << m_receiveTimeLimit.count() + << " ms"; asio::dispatch(m_strand, boost::bind(&Connector::reconnect, this)); } else if (ec != boost::asio::error::operation_aborted) diff --git a/test_package/http_server_test.cpp b/test_package/http_server_test.cpp index 658cb58e2..88b0c5808 100644 --- a/test_package/http_server_test.cpp +++ b/test_package/http_server_test.cpp @@ -908,18 +908,18 @@ TEST_F(HttpServerTest, options_returns_correctly_for_path_with_parameter_value) session->writeResponse(std::move(resp)); return true; }; - + m_server->addRouting({boost::beast::http::verb::get, "/cancel/id={string}", handler}) - .document("MTConnect WebServices Cancel Stream", "Cancels a streaming sample request") - .command("cancel"); + .document("MTConnect WebServices Cancel Stream", "Cancels a streaming sample request") + .command("cancel"); start(); startClient(); - + m_client->spawnRequest(http::verb::options, "/cancel/id=12345"); ASSERT_TRUE(m_client->m_done); EXPECT_EQ(int(http::status::no_content), m_client->m_status); - + auto allow = m_client->m_fields.find("Allow"); ASSERT_NE(m_client->m_fields.end(), allow); EXPECT_NE(string::npos, allow->second.find("GET")); @@ -927,14 +927,14 @@ TEST_F(HttpServerTest, options_returns_correctly_for_path_with_parameter_value) EXPECT_EQ(string::npos, allow->second.find("PUT")); EXPECT_EQ(string::npos, allow->second.find("POST")); EXPECT_EQ(string::npos, allow->second.find("DELETE")); - + auto acam = m_client->m_fields.find("Access-Control-Allow-Methods"); ASSERT_NE(m_client->m_fields.end(), acam); EXPECT_EQ(allow->second, acam->second); - + auto acah = m_client->m_fields.find("Access-Control-Allow-Headers"); ASSERT_NE(m_client->m_fields.end(), acah); - + auto acma = m_client->m_fields.find("Access-Control-Max-Age"); ASSERT_NE(m_client->m_fields.end(), acma); EXPECT_EQ("86400", acma->second); @@ -947,17 +947,17 @@ TEST_F(HttpServerTest, should_handle_routings_with_just_a_regex) session->writeResponse(std::move(resp)); return true; }; - + m_server->addRouting({boost::beast::http::verb::get, regex("/.+"), handler}); m_server->addRouting({http::verb::put, "/{device}?timestamp={timestamp}", handler}); start(); startClient(); - + m_client->spawnRequest(http::verb::options, "/file.xsd"); ASSERT_TRUE(m_client->m_done); EXPECT_EQ(int(http::status::no_content), m_client->m_status); - + auto allow = m_client->m_fields.find("Allow"); ASSERT_NE(m_client->m_fields.end(), allow); EXPECT_NE(string::npos, allow->second.find("GET")); @@ -965,14 +965,14 @@ TEST_F(HttpServerTest, should_handle_routings_with_just_a_regex) EXPECT_NE(string::npos, allow->second.find("PUT")); EXPECT_EQ(string::npos, allow->second.find("POST")); EXPECT_EQ(string::npos, allow->second.find("DELETE")); - + auto acam = m_client->m_fields.find("Access-Control-Allow-Methods"); ASSERT_NE(m_client->m_fields.end(), acam); EXPECT_EQ(allow->second, acam->second); - + auto acah = m_client->m_fields.find("Access-Control-Allow-Headers"); ASSERT_NE(m_client->m_fields.end(), acah); - + auto acma = m_client->m_fields.find("Access-Control-Max-Age"); ASSERT_NE(m_client->m_fields.end(), acma); EXPECT_EQ("86400", acma->second); From 5f14e3d80736d7d6c35fe9ef0b04b64a897912b4 Mon Sep 17 00:00:00 2001 From: Will Sobel Date: Tue, 7 Apr 2026 15:40:43 +0200 Subject: [PATCH 07/15] Added some fixes for logging directory management. Issue #604 --- src/mtconnect/configuration/agent_config.cpp | 24 +++++++++++--------- src/mtconnect/configuration/agent_config.hpp | 1 + src/mtconnect/configuration/service.cpp | 2 +- 3 files changed, 15 insertions(+), 12 deletions(-) diff --git a/src/mtconnect/configuration/agent_config.cpp b/src/mtconnect/configuration/agent_config.cpp index 1824dd36e..6ae8293f4 100644 --- a/src/mtconnect/configuration/agent_config.cpp +++ b/src/mtconnect/configuration/agent_config.cpp @@ -176,8 +176,9 @@ namespace mtconnect::configuration { cerr << "Loading configuration from:" << *path << endl; m_configFile = fs::canonical(*path); - addPathFront(m_configPaths, m_configFile.parent_path()); - addPathBack(m_dataPaths, m_configFile.parent_path()); + m_configPath = m_configFile.parent_path(); + addPathFront(m_configPaths, m_configPath); + addPathBack(m_dataPaths, m_configPath); ifstream file(m_configFile.c_str()); std::stringstream buffer; @@ -617,7 +618,12 @@ namespace mtconnect::configuration { // Get file names and patterns from the options. auto file_name = *GetOption(options, "file_name"); auto archive_pattern = *GetOption(options, "archive_pattern"); - + + logDirectory = m_configPath; + logFileName = fs::path(file_name); + if (logFileName.is_absolute()) + logDirectory = logFileName.parent_path(); + logArchivePattern = fs::path(archive_pattern); if (!logArchivePattern.has_filename()) { @@ -625,18 +631,14 @@ namespace mtconnect::configuration { } if (logArchivePattern.is_relative()) - logArchivePattern = fs::current_path() / logArchivePattern; - - // Get the log directory from the archive path. - logDirectory = logArchivePattern.parent_path(); + logArchivePattern = logDirectory / logArchivePattern; + else + logDirectory = logArchivePattern.parent_path(); // If the file name does not specify a log directory, use the // archive directory - logFileName = fs::path(file_name); - if (!logFileName.has_parent_path()) + if (!logFileName.has_parent_path() || logFileName.is_relative()) logFileName = logDirectory / logFileName; - else if (logFileName.is_relative()) - logFileName = fs::current_path() / logFileName; // Create a text file sink auto sink = boost::make_shared( diff --git a/src/mtconnect/configuration/agent_config.hpp b/src/mtconnect/configuration/agent_config.hpp index a5ba57a28..a74907e8f 100644 --- a/src/mtconnect/configuration/agent_config.hpp +++ b/src/mtconnect/configuration/agent_config.hpp @@ -374,6 +374,7 @@ namespace mtconnect { std::string m_devicesFile; std::filesystem::path m_exePath; std::filesystem::path m_working; + std::filesystem::path m_configPath; std::list m_configPaths; std::list m_dataPaths; diff --git a/src/mtconnect/configuration/service.cpp b/src/mtconnect/configuration/service.cpp index 1709937e7..8c502aa14 100644 --- a/src/mtconnect/configuration/service.cpp +++ b/src/mtconnect/configuration/service.cpp @@ -258,7 +258,7 @@ namespace mtconnect { return true; HANDLE token = nullptr; - if (!OpenProcessToken(GetCurrentProcess(), TOKEN_QUERY, &token)) + if (!OpenProcessToken(GetdProcess(), TOKEN_QUERY, &token)) { std::cerr << "OpenProcessToken failed (" << GetLastError() << ")" << std::endl; LOG(error) << "OpenProcessToken (" << GetLastError() << ")"; From 9c0a84d907ccf749c28b28cf8aa3fef15a16a16c Mon Sep 17 00:00:00 2001 From: Will Sobel Date: Tue, 7 Apr 2026 15:40:43 +0200 Subject: [PATCH 08/15] Added some fixes for logging directory management. Issue #604 --- src/mtconnect/configuration/agent_config.cpp | 24 +++++++++++--------- src/mtconnect/configuration/agent_config.hpp | 1 + 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/src/mtconnect/configuration/agent_config.cpp b/src/mtconnect/configuration/agent_config.cpp index 1824dd36e..6ae8293f4 100644 --- a/src/mtconnect/configuration/agent_config.cpp +++ b/src/mtconnect/configuration/agent_config.cpp @@ -176,8 +176,9 @@ namespace mtconnect::configuration { cerr << "Loading configuration from:" << *path << endl; m_configFile = fs::canonical(*path); - addPathFront(m_configPaths, m_configFile.parent_path()); - addPathBack(m_dataPaths, m_configFile.parent_path()); + m_configPath = m_configFile.parent_path(); + addPathFront(m_configPaths, m_configPath); + addPathBack(m_dataPaths, m_configPath); ifstream file(m_configFile.c_str()); std::stringstream buffer; @@ -617,7 +618,12 @@ namespace mtconnect::configuration { // Get file names and patterns from the options. auto file_name = *GetOption(options, "file_name"); auto archive_pattern = *GetOption(options, "archive_pattern"); - + + logDirectory = m_configPath; + logFileName = fs::path(file_name); + if (logFileName.is_absolute()) + logDirectory = logFileName.parent_path(); + logArchivePattern = fs::path(archive_pattern); if (!logArchivePattern.has_filename()) { @@ -625,18 +631,14 @@ namespace mtconnect::configuration { } if (logArchivePattern.is_relative()) - logArchivePattern = fs::current_path() / logArchivePattern; - - // Get the log directory from the archive path. - logDirectory = logArchivePattern.parent_path(); + logArchivePattern = logDirectory / logArchivePattern; + else + logDirectory = logArchivePattern.parent_path(); // If the file name does not specify a log directory, use the // archive directory - logFileName = fs::path(file_name); - if (!logFileName.has_parent_path()) + if (!logFileName.has_parent_path() || logFileName.is_relative()) logFileName = logDirectory / logFileName; - else if (logFileName.is_relative()) - logFileName = fs::current_path() / logFileName; // Create a text file sink auto sink = boost::make_shared( diff --git a/src/mtconnect/configuration/agent_config.hpp b/src/mtconnect/configuration/agent_config.hpp index a5ba57a28..a74907e8f 100644 --- a/src/mtconnect/configuration/agent_config.hpp +++ b/src/mtconnect/configuration/agent_config.hpp @@ -374,6 +374,7 @@ namespace mtconnect { std::string m_devicesFile; std::filesystem::path m_exePath; std::filesystem::path m_working; + std::filesystem::path m_configPath; std::list m_configPaths; std::list m_dataPaths; From eb456ffc30721f9fe17f72483d2e3574c07bc2b7 Mon Sep 17 00:00:00 2001 From: Will Date: Tue, 7 Apr 2026 19:20:57 +0200 Subject: [PATCH 09/15] Fixed logging directory creation and placement of rotation files. --- src/mtconnect/configuration/agent_config.cpp | 42 +++++++++++++++----- src/mtconnect/configuration/service.cpp | 2 +- 2 files changed, 33 insertions(+), 11 deletions(-) diff --git a/src/mtconnect/configuration/agent_config.cpp b/src/mtconnect/configuration/agent_config.cpp index 6ae8293f4..0a4d2aafc 100644 --- a/src/mtconnect/configuration/agent_config.cpp +++ b/src/mtconnect/configuration/agent_config.cpp @@ -619,30 +619,51 @@ namespace mtconnect::configuration { auto file_name = *GetOption(options, "file_name"); auto archive_pattern = *GetOption(options, "archive_pattern"); + // Default the log directory to the configuration file path. logDirectory = m_configPath; logFileName = fs::path(file_name); + logArchivePattern = fs::path(archive_pattern); + + // Determine the log directory based on the provided file name and archive pattern if (logFileName.is_absolute()) logDirectory = logFileName.parent_path(); + else if (logArchivePattern.is_absolute()) + logDirectory = logArchivePattern.parent_path(); - logArchivePattern = fs::path(archive_pattern); - if (!logArchivePattern.has_filename()) + // If the log file name is relative and has a parent path, use it to determine the log directory + if (logFileName.is_relative() && logFileName.has_parent_path()) { - logArchivePattern = logArchivePattern / archiveFileName(get(options["file_name"])); + logDirectory = logDirectory / logFileName.parent_path(); + logFileName = logFileName.filename(); + } + else if (logArchivePattern.is_relative() && logArchivePattern.has_parent_path()) + { + logDirectory = logDirectory / logArchivePattern.parent_path(); + logArchivePattern = logArchivePattern.filename(); } + // Make sure the log archive pattern includes a file name, use the default file name as the base. + if (!logArchivePattern.has_filename()) + logArchivePattern = logArchivePattern / archiveFileName(get(options["file_name"])); + + // Make the logArchivePattern and logFileName absolute paths + logDirectory = logDirectory.lexically_normal(); + if (logArchivePattern.is_relative()) logArchivePattern = logDirectory / logArchivePattern; - else - logDirectory = logArchivePattern.parent_path(); + logArchivePattern = logArchivePattern.lexically_normal(); + auto archiveDir = logArchivePattern.parent_path(); + fs::create_directories(logArchivePattern.parent_path()); - // If the file name does not specify a log directory, use the - // archive directory - if (!logFileName.has_parent_path() || logFileName.is_relative()) + if (logFileName.is_relative()) logFileName = logDirectory / logFileName; + logFileName = logFileName.lexically_normal(); + fs::create_directories(logFileName.parent_path()); + // Create a text file sink auto sink = boost::make_shared( - kw::file_name = logFileName, kw::target_file_name = logArchivePattern.filename(), + kw::file_name = logFileName.string(), kw::target_file_name = logArchivePattern.string(), kw::auto_flush = true, kw::rotation_size = logRotationSize, kw::open_mode = ios_base::out | ios_base::app, kw::format = formatter); @@ -651,7 +672,8 @@ namespace mtconnect::configuration { // Set up where the rotated files will be stored sink->locked_backend()->set_file_collector(logr::sinks::file::make_collector( - kw::target = logDirectory, kw::max_size = maxLogFileSize, kw::max_files = max_index)); + kw::target = archiveDir.string(), kw::max_size = maxLogFileSize, + kw::max_files = max_index)); if (rotationLogInterval > 0) { diff --git a/src/mtconnect/configuration/service.cpp b/src/mtconnect/configuration/service.cpp index 8c502aa14..1709937e7 100644 --- a/src/mtconnect/configuration/service.cpp +++ b/src/mtconnect/configuration/service.cpp @@ -258,7 +258,7 @@ namespace mtconnect { return true; HANDLE token = nullptr; - if (!OpenProcessToken(GetdProcess(), TOKEN_QUERY, &token)) + if (!OpenProcessToken(GetCurrentProcess(), TOKEN_QUERY, &token)) { std::cerr << "OpenProcessToken failed (" << GetLastError() << ")" << std::endl; LOG(error) << "OpenProcessToken (" << GetLastError() << ")"; From e225df3ccbc4d045405206fb0089782119d2192c Mon Sep 17 00:00:00 2001 From: Will Sobel Date: Tue, 7 Apr 2026 20:37:59 +0200 Subject: [PATCH 10/15] Version 2.7.0.6 --- CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 9ef5906ab..3c7ecca69 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -2,7 +2,7 @@ set(AGENT_VERSION_MAJOR 2) set(AGENT_VERSION_MINOR 7) set(AGENT_VERSION_PATCH 0) -set(AGENT_VERSION_BUILD 5) +set(AGENT_VERSION_BUILD 6) set(AGENT_VERSION_RC "") # This minimum version is to support Visual Studio 2019 and C++ feature checking and FetchContent From ce2748e05a845119b0d6161fae65af36388ed997 Mon Sep 17 00:00:00 2001 From: Will Sobel Date: Wed, 8 Apr 2026 11:49:19 +0200 Subject: [PATCH 11/15] Fixed tests to set log directors as temp for log testing. --- src/mtconnect/configuration/agent_config.cpp | 7 +- src/mtconnect/configuration/agent_config.hpp | 13 ++++ test_package/config_test.cpp | 69 ++++++++++++-------- 3 files changed, 59 insertions(+), 30 deletions(-) diff --git a/src/mtconnect/configuration/agent_config.cpp b/src/mtconnect/configuration/agent_config.cpp index 9d62eb49d..375ed00bd 100644 --- a/src/mtconnect/configuration/agent_config.cpp +++ b/src/mtconnect/configuration/agent_config.cpp @@ -599,6 +599,7 @@ namespace mtconnect::configuration { auto &rotationLogInterval = logChannel.m_rotationLogInterval; auto &logArchivePattern = logChannel.m_logArchivePattern; auto &logDirectory = logChannel.m_logDirectory; + auto &archiveLogDirectory = logChannel.m_archiveLogDirectory; auto &logFileName = logChannel.m_logFileName; maxLogFileSize = ConvertFileSize(options, "max_size", maxLogFileSize); @@ -652,8 +653,8 @@ namespace mtconnect::configuration { if (logArchivePattern.is_relative()) logArchivePattern = logDirectory / logArchivePattern; logArchivePattern = logArchivePattern.lexically_normal(); - auto archiveDir = logArchivePattern.parent_path(); - fs::create_directories(logArchivePattern.parent_path()); + archiveLogDirectory = logArchivePattern.parent_path(); + fs::create_directories(archiveLogDirectory); if (logFileName.is_relative()) logFileName = logDirectory / logFileName; @@ -671,7 +672,7 @@ namespace mtconnect::configuration { // Set up where the rotated files will be stored sink->locked_backend()->set_file_collector(logr::sinks::file::make_collector( - kw::target = archiveDir.string(), kw::max_size = maxLogFileSize, + kw::target = archiveLogDirectory.string(), kw::max_size = maxLogFileSize, kw::max_files = max_index)); if (rotationLogInterval > 0) diff --git a/src/mtconnect/configuration/agent_config.hpp b/src/mtconnect/configuration/agent_config.hpp index a74907e8f..f2e3a955a 100644 --- a/src/mtconnect/configuration/agent_config.hpp +++ b/src/mtconnect/configuration/agent_config.hpp @@ -183,6 +183,14 @@ namespace mtconnect { { return m_logChannels[channelName].m_logArchivePattern; } + + /// @brief gets the archive log directory + /// @return log directory + const auto &getArchiveLogDirectory(const std::string &channelName = "agent") + { + return m_logChannels[channelName].m_archiveLogDirectory; + } + /// @brief Get the maximum size of all the log files /// @return the maximum size of all log files auto getMaxLogFileSize(const std::string &channelName = "agent") @@ -259,6 +267,10 @@ namespace mtconnect { /// @brief add a path to the plugin paths /// @param path the path to add void addPluginPath(const std::filesystem::path &path) { addPathBack(m_pluginPaths, path); } + + ///@brief set the config path for testing + ///@param path the path to set for the config file directory + void setConfigPath(const std::filesystem::path &path) { m_configPath = path; } protected: DevicePtr getDefaultDevice(); @@ -348,6 +360,7 @@ namespace mtconnect { { std::string m_channelName; std::filesystem::path m_logDirectory; + std::filesystem::path m_archiveLogDirectory; std::filesystem::path m_logArchivePattern; std::filesystem::path m_logFileName; diff --git a/test_package/config_test.cpp b/test_package/config_test.cpp index fde6e2f1b..7b2a85a7b 100644 --- a/test_package/config_test.cpp +++ b/test_package/config_test.cpp @@ -779,13 +779,15 @@ MaxCachedFileSize = 2g TEST_F(ConfigTest, log_output_should_set_archive_file_pattern) { + auto root {createTempDirectory("log_1")}; + m_config->setConfigPath(root); m_config->setDebug(false); - string str(R"( + string str = "Devices = " TEST_RESOURCE_DIR "/samples/min_config.xml" R"( logger_config { output = file agent.log } -)"); +)"; m_config->loadConfig(str); @@ -794,18 +796,20 @@ logger_config { EXPECT_EQ("agent_%Y-%m-%d_%H-%M-%S_%N.log", m_config->getLogArchivePattern().filename()); EXPECT_EQ("agent.log", m_config->getLogFileName().filename()); - EXPECT_PATH_EQ(TEST_BIN_ROOT_DIR, m_config->getLogDirectory()); + EXPECT_PATH_EQ(root, m_config->getLogDirectory()); } TEST_F(ConfigTest, log_output_should_configure_file_name) { + auto root {createTempDirectory("log_2")}; + m_config->setConfigPath(root); m_config->setDebug(false); - - string str(R"( + + string str = "Devices = " TEST_RESOURCE_DIR "/samples/min_config.xml" R"( logger_config { output = file logging.log logging_%N.log } -)"); +)"; m_config->loadConfig(str); @@ -814,19 +818,21 @@ logger_config { EXPECT_EQ("logging_%N.log", m_config->getLogArchivePattern().filename()); EXPECT_EQ("logging.log", m_config->getLogFileName().filename()); - EXPECT_PATH_EQ(TEST_BIN_ROOT_DIR, m_config->getLogDirectory()); + EXPECT_PATH_EQ(root, m_config->getLogDirectory()); } TEST_F(ConfigTest, log_should_configure_file_name) { + auto root {createTempDirectory("log_3")}; + m_config->setConfigPath(root); m_config->setDebug(false); - - string str(R"( + + string str = "Devices = " TEST_RESOURCE_DIR "/samples/min_config.xml" R"( logger_config { file_name = logging.log archive_pattern = logging_%N.log } -)"); +)"; m_config->loadConfig(str); @@ -835,26 +841,28 @@ logger_config { EXPECT_EQ("logging_%N.log", m_config->getLogArchivePattern().filename()); EXPECT_EQ("logging.log", m_config->getLogFileName().filename()); - EXPECT_PATH_EQ(TEST_BIN_ROOT_DIR, m_config->getLogDirectory()); + EXPECT_PATH_EQ(root, m_config->getLogDirectory()); } TEST_F(ConfigTest, log_should_specify_relative_directory) { + auto root {createTempDirectory("log_3")}; + m_config->setConfigPath(root); m_config->setDebug(false); - - string str(R"( + + string str = "Devices = " TEST_RESOURCE_DIR "/samples/min_config.xml" R"( logger_config { file_name = logging.log archive_pattern = logs/logging_%N.log } -)"); +)"; m_config->loadConfig(str); auto sink = m_config->getLoggerSink(); ASSERT_TRUE(sink); - fs::path path {std::filesystem::canonical(TEST_BIN_ROOT_DIR) / "logs"}; + fs::path path { root / "logs"}; EXPECT_PATH_EQ(path / "logging_%N.log", m_config->getLogArchivePattern()); EXPECT_PATH_EQ(path / "logging.log", m_config->getLogFileName()); @@ -863,38 +871,43 @@ logger_config { TEST_F(ConfigTest, log_should_specify_relative_directory_with_active_in_parent) { + auto root {createTempDirectory("log_4")}; + m_config->setConfigPath(root); m_config->setDebug(false); - - string str(R"( + + string str = "Devices = " TEST_RESOURCE_DIR "/samples/min_config.xml" R"( logger_config { file_name = ./logging.log archive_pattern = logs/logging_%N.log } -)"); +)"; m_config->loadConfig(str); auto sink = m_config->getLoggerSink(); ASSERT_TRUE(sink); - fs::path path {std::filesystem::canonical(TEST_BIN_ROOT_DIR)}; + fs::path path {std::filesystem::canonical(root)}; EXPECT_PATH_EQ(path / "logs" / "logging_%N.log", m_config->getLogArchivePattern()); EXPECT_PATH_EQ(path / "logging.log", m_config->getLogFileName()); - EXPECT_PATH_EQ(path / "logs", m_config->getLogDirectory()); + EXPECT_PATH_EQ(path / "logs", m_config->getArchiveLogDirectory()); } TEST_F(ConfigTest, log_should_specify_max_file_and_rotation_size) { - m_config->setDebug(false); using namespace boost::log::trivial; - string str(R"( + auto root {createTempDirectory("log_5")}; + m_config->setConfigPath(root); + m_config->setDebug(false); + + string str = "Devices = " TEST_RESOURCE_DIR "/samples/min_config.xml" R"( logger_config { max_size = 1gb rotation_size = 20gb } -)"); +)"; m_config->loadConfig(str); @@ -908,15 +921,17 @@ logger_config { TEST_F(ConfigTest, log_should_configure_logging_level) { - m_config->setDebug(false); - using namespace boost::log::trivial; - string str(R"( + auto root {createTempDirectory("log_6")}; + m_config->setConfigPath(root); + m_config->setDebug(false); + + string str = "Devices = " TEST_RESOURCE_DIR "/samples/min_config.xml" R"( logger_config { level = fatal } -)"); +)"; m_config->loadConfig(str); From 17f07ccddb78685793b599bf624cfd78a110dfa7 Mon Sep 17 00:00:00 2001 From: Will Sobel Date: Wed, 8 Apr 2026 12:08:40 +0200 Subject: [PATCH 12/15] Added rotation test at 1kb --- test_package/config_test.cpp | 43 ++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/test_package/config_test.cpp b/test_package/config_test.cpp index 7b2a85a7b..56a4acfbf 100644 --- a/test_package/config_test.cpp +++ b/test_package/config_test.cpp @@ -23,6 +23,7 @@ // Keep this comment to keep gtest.h above. (clang-format off/on is not working here!) #include +#include #include #include @@ -982,7 +983,49 @@ logger_config { m_config->setLoggingLevel("FATAL"); EXPECT_EQ(severity_level::fatal, m_config->getLogLevel()); } + + TEST_F(ConfigTest, log_should_rotate_log_file_when_it_reaches_limit) + { + auto root {createTempDirectory("log_7")}; + m_config->setConfigPath(root); + m_config->setDebug(false); + + string str = "Devices = " TEST_RESOURCE_DIR "/samples/min_config.xml" R"( +logger_config { + file_name = ./logging.log + archive_pattern = logs/logging_%N.log + # Make if very small + rotation_size = 1k +} +)"; + + m_config->loadConfig(str); + + auto sink = m_config->getLoggerSink(); + ASSERT_TRUE(sink); + + fs::path path {std::filesystem::canonical(root)}; + + EXPECT_PATH_EQ(path / "logs" / "logging_%N.log", m_config->getLogArchivePattern()); + EXPECT_PATH_EQ(path / "logging.log", m_config->getLogFileName()); + EXPECT_PATH_EQ(path / "logs", m_config->getArchiveLogDirectory()); + + // Write some data to the log file to trigger rotation + for (auto _ : boost::irange(11)) + LOG(info) << "This is a long 100 byte test log message to trigger rotation of file with some common text included."; + + auto logging = path / "logging.log"; + EXPECT_TRUE(fs::exists(logging)) << "Expected log file to exist: " << logging; + EXPECT_TRUE(fs::file_size(logging) < 1024) << "Expected log file to be less than 1KB: " << logging; + for (auto i : boost::irange(2)) + { + fs::path rotated = path / "logs" / ("logging_" + std::to_string(i) + ".log"); + EXPECT_TRUE(fs::exists(rotated)) << "Expected log file to be rotated: " << rotated; + EXPECT_TRUE(fs::file_size(rotated) < 1024) << "Expected rotated log file to be less than 1KB: " << rotated; + } + } + TEST_F(ConfigTest, should_reload_device_xml_file) { auto root {createTempDirectory("1")}; From 1a322cc8283493e43f1a5995a2bc92237d395a5d Mon Sep 17 00:00:00 2001 From: Will Sobel Date: Wed, 8 Apr 2026 12:35:08 +0200 Subject: [PATCH 13/15] Fixed chunk body to remove x86 build warnings. --- src/mtconnect/source/adapter/agent_adapter/session_impl.hpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/mtconnect/source/adapter/agent_adapter/session_impl.hpp b/src/mtconnect/source/adapter/agent_adapter/session_impl.hpp index c0811e129..8d822634f 100644 --- a/src/mtconnect/source/adapter/agent_adapter/session_impl.hpp +++ b/src/mtconnect/source/adapter/agent_adapter/session_impl.hpp @@ -524,7 +524,7 @@ namespace mtconnect::source::adapter::agent_adapter { void createChunkBodyHandler() { m_chunkHandler = [this](std::uint64_t remain, boost::string_view body, - boost::system::error_code &ev) -> unsigned long { + boost::system::error_code &ev) -> std::size_t { if (!m_request) { derived().lowestLayer().close(); @@ -607,7 +607,7 @@ namespace mtconnect::source::adapter::agent_adapter { asio::io_context::strand m_strand; url::Url m_url; - std::function + std::function m_chunkHandler; std::function m_chunkHeaderHandler; From 6cf2aea4b75cc74db5cb11ab3d14b97b207fac7b Mon Sep 17 00:00:00 2001 From: Will Sobel Date: Wed, 8 Apr 2026 12:39:25 +0200 Subject: [PATCH 14/15] Formatted code with clang --- src/mtconnect/configuration/agent_config.cpp | 7 +- src/mtconnect/configuration/agent_config.hpp | 2 +- src/mtconnect/parser/xml_parser.cpp | 3 +- test_package/config_test.cpp | 73 ++++++++++++-------- 4 files changed, 52 insertions(+), 33 deletions(-) diff --git a/src/mtconnect/configuration/agent_config.cpp b/src/mtconnect/configuration/agent_config.cpp index 375ed00bd..37cbe0c43 100644 --- a/src/mtconnect/configuration/agent_config.cpp +++ b/src/mtconnect/configuration/agent_config.cpp @@ -619,7 +619,7 @@ namespace mtconnect::configuration { // Get file names and patterns from the options. auto file_name = *GetOption(options, "file_name"); auto archive_pattern = *GetOption(options, "archive_pattern"); - + // Default the log directory to the configuration file path. logDirectory = m_configPath; logFileName = fs::path(file_name); @@ -630,7 +630,7 @@ namespace mtconnect::configuration { logDirectory = logFileName.parent_path(); else if (logArchivePattern.is_absolute()) logDirectory = logArchivePattern.parent_path(); - + // If the log file name is relative and has a parent path, use it to determine the log directory if (logFileName.is_relative() && logFileName.has_parent_path()) { @@ -643,7 +643,8 @@ namespace mtconnect::configuration { logArchivePattern = logArchivePattern.filename(); } - // Make sure the log archive pattern includes a file name, use the default file name as the base. + // Make sure the log archive pattern includes a file name, use the default file name as the + // base. if (!logArchivePattern.has_filename()) logArchivePattern = logArchivePattern / archiveFileName(get(options["file_name"])); diff --git a/src/mtconnect/configuration/agent_config.hpp b/src/mtconnect/configuration/agent_config.hpp index f2e3a955a..0f5c13748 100644 --- a/src/mtconnect/configuration/agent_config.hpp +++ b/src/mtconnect/configuration/agent_config.hpp @@ -267,7 +267,7 @@ namespace mtconnect { /// @brief add a path to the plugin paths /// @param path the path to add void addPluginPath(const std::filesystem::path &path) { addPathBack(m_pluginPaths, path); } - + ///@brief set the config path for testing ///@param path the path to set for the config file directory void setConfigPath(const std::filesystem::path &path) { m_configPath = path; } diff --git a/src/mtconnect/parser/xml_parser.cpp b/src/mtconnect/parser/xml_parser.cpp index 20185ceb2..3b2511c20 100644 --- a/src/mtconnect/parser/xml_parser.cpp +++ b/src/mtconnect/parser/xml_parser.cpp @@ -218,8 +218,7 @@ namespace mtconnect::parser { { if (device) LOG(warning) << "When loading device " << device->get("name") - << ", A problem was skipped: " - << e->what(); + << ", A problem was skipped: " << e->what(); else LOG(error) << "Failed to load device: " << e->what(); } diff --git a/test_package/config_test.cpp b/test_package/config_test.cpp index 56a4acfbf..92797151f 100644 --- a/test_package/config_test.cpp +++ b/test_package/config_test.cpp @@ -784,7 +784,9 @@ MaxCachedFileSize = 2g m_config->setConfigPath(root); m_config->setDebug(false); - string str = "Devices = " TEST_RESOURCE_DIR "/samples/min_config.xml" R"( + string str = "Devices = " TEST_RESOURCE_DIR + "/samples/min_config.xml" + R"( logger_config { output = file agent.log } @@ -805,8 +807,10 @@ logger_config { auto root {createTempDirectory("log_2")}; m_config->setConfigPath(root); m_config->setDebug(false); - - string str = "Devices = " TEST_RESOURCE_DIR "/samples/min_config.xml" R"( + + string str = "Devices = " TEST_RESOURCE_DIR + "/samples/min_config.xml" + R"( logger_config { output = file logging.log logging_%N.log } @@ -827,8 +831,10 @@ logger_config { auto root {createTempDirectory("log_3")}; m_config->setConfigPath(root); m_config->setDebug(false); - - string str = "Devices = " TEST_RESOURCE_DIR "/samples/min_config.xml" R"( + + string str = "Devices = " TEST_RESOURCE_DIR + "/samples/min_config.xml" + R"( logger_config { file_name = logging.log archive_pattern = logging_%N.log @@ -850,8 +856,10 @@ logger_config { auto root {createTempDirectory("log_3")}; m_config->setConfigPath(root); m_config->setDebug(false); - - string str = "Devices = " TEST_RESOURCE_DIR "/samples/min_config.xml" R"( + + string str = "Devices = " TEST_RESOURCE_DIR + "/samples/min_config.xml" + R"( logger_config { file_name = logging.log archive_pattern = logs/logging_%N.log @@ -863,7 +871,7 @@ logger_config { auto sink = m_config->getLoggerSink(); ASSERT_TRUE(sink); - fs::path path { root / "logs"}; + fs::path path {root / "logs"}; EXPECT_PATH_EQ(path / "logging_%N.log", m_config->getLogArchivePattern()); EXPECT_PATH_EQ(path / "logging.log", m_config->getLogFileName()); @@ -875,8 +883,10 @@ logger_config { auto root {createTempDirectory("log_4")}; m_config->setConfigPath(root); m_config->setDebug(false); - - string str = "Devices = " TEST_RESOURCE_DIR "/samples/min_config.xml" R"( + + string str = "Devices = " TEST_RESOURCE_DIR + "/samples/min_config.xml" + R"( logger_config { file_name = ./logging.log archive_pattern = logs/logging_%N.log @@ -902,8 +912,10 @@ logger_config { auto root {createTempDirectory("log_5")}; m_config->setConfigPath(root); m_config->setDebug(false); - - string str = "Devices = " TEST_RESOURCE_DIR "/samples/min_config.xml" R"( + + string str = "Devices = " TEST_RESOURCE_DIR + "/samples/min_config.xml" + R"( logger_config { max_size = 1gb rotation_size = 20gb @@ -927,8 +939,10 @@ logger_config { auto root {createTempDirectory("log_6")}; m_config->setConfigPath(root); m_config->setDebug(false); - - string str = "Devices = " TEST_RESOURCE_DIR "/samples/min_config.xml" R"( + + string str = "Devices = " TEST_RESOURCE_DIR + "/samples/min_config.xml" + R"( logger_config { level = fatal } @@ -983,14 +997,16 @@ logger_config { m_config->setLoggingLevel("FATAL"); EXPECT_EQ(severity_level::fatal, m_config->getLogLevel()); } - + TEST_F(ConfigTest, log_should_rotate_log_file_when_it_reaches_limit) { auto root {createTempDirectory("log_7")}; m_config->setConfigPath(root); m_config->setDebug(false); - - string str = "Devices = " TEST_RESOURCE_DIR "/samples/min_config.xml" R"( + + string str = "Devices = " TEST_RESOURCE_DIR + "/samples/min_config.xml" + R"( logger_config { file_name = ./logging.log archive_pattern = logs/logging_%N.log @@ -998,34 +1014,37 @@ logger_config { rotation_size = 1k } )"; - + m_config->loadConfig(str); - + auto sink = m_config->getLoggerSink(); ASSERT_TRUE(sink); - + fs::path path {std::filesystem::canonical(root)}; - + EXPECT_PATH_EQ(path / "logs" / "logging_%N.log", m_config->getLogArchivePattern()); EXPECT_PATH_EQ(path / "logging.log", m_config->getLogFileName()); EXPECT_PATH_EQ(path / "logs", m_config->getArchiveLogDirectory()); - + // Write some data to the log file to trigger rotation for (auto _ : boost::irange(11)) - LOG(info) << "This is a long 100 byte test log message to trigger rotation of file with some common text included."; - + LOG(info) << "This is a long 100 byte test log message to trigger rotation of file with some " + "common text included."; + auto logging = path / "logging.log"; EXPECT_TRUE(fs::exists(logging)) << "Expected log file to exist: " << logging; - EXPECT_TRUE(fs::file_size(logging) < 1024) << "Expected log file to be less than 1KB: " << logging; + EXPECT_TRUE(fs::file_size(logging) < 1024) + << "Expected log file to be less than 1KB: " << logging; for (auto i : boost::irange(2)) { fs::path rotated = path / "logs" / ("logging_" + std::to_string(i) + ".log"); EXPECT_TRUE(fs::exists(rotated)) << "Expected log file to be rotated: " << rotated; - EXPECT_TRUE(fs::file_size(rotated) < 1024) << "Expected rotated log file to be less than 1KB: " << rotated; + EXPECT_TRUE(fs::file_size(rotated) < 1024) + << "Expected rotated log file to be less than 1KB: " << rotated; } } - + TEST_F(ConfigTest, should_reload_device_xml_file) { auto root {createTempDirectory("1")}; From 131304b541dd692945b7509b2337a7be525e2b35 Mon Sep 17 00:00:00 2001 From: Will Sobel Date: Wed, 8 Apr 2026 13:34:05 +0200 Subject: [PATCH 15/15] Remove x86 warnings because of header chunk handler. --- test_package/http_server_test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test_package/http_server_test.cpp b/test_package/http_server_test.cpp index 88b0c5808..86f733045 100644 --- a/test_package/http_server_test.cpp +++ b/test_package/http_server_test.cpp @@ -275,7 +275,7 @@ class Client map m_fields; string m_contentType; - std::function + std::function m_chunkHandler; std::function m_headerHandler;