adam-markovics commented on a change in pull request #820:
URL: https://github.com/apache/nifi-minifi-cpp/pull/820#discussion_r444186928
##########
File path: extensions/http-curl/tests/C2DescribeCoreComponentStateTest.cpp
##########
@@ -58,16 +57,11 @@ class VerifyC2DescribeCoreComponentState : public
VerifyC2Describe {
class DescribeCoreComponentStateHandler: public HeartbeatHandler {
public:
-
- explicit DescribeCoreComponentStateHandler(bool isSecure)
- : HeartbeatHandler(isSecure) {
- }
-
- virtual void handleHeartbeat(const rapidjson::Document&, struct
mg_connection * conn) {
+ virtual void handleHeartbeat(const rapidjson::Document&, struct
mg_connection * conn) override {
Review comment:
Done, also in other files. Maybe we should also make usage of override
mandatory by compiler flag. What do you guys think?
##########
File path: extensions/http-curl/tests/C2UpdateAgentTest.cpp
##########
@@ -16,178 +16,24 @@
* limitations under the License.
*/
-#include <sys/stat.h>
#undef NDEBUG
-#include <cassert>
-#include <utility>
-#include <chrono>
-#include <fstream>
-#include <memory>
-#include <string>
-#include <thread>
-#include <type_traits>
-#include <vector>
-#include <iostream>
-#include <sstream>
-#include "HTTPClient.h"
-#include "InvokeHTTP.h"
-#include "TestBase.h"
-#include "utils/StringUtils.h"
-#include "core/Core.h"
-#include "core/logging/Logger.h"
-#include "core/ProcessGroup.h"
-#include "core/yaml/YamlConfiguration.h"
-#include "FlowController.h"
-#include "properties/Configure.h"
-#include "unit/ProvenanceTestHelper.h"
-#include "io/StreamFactory.h"
-#include "c2/C2Agent.h"
-#include "CivetServer.h"
-#include <cstring>
-#include "protocols/RESTSender.h"
-
-void waitToVerifyProcessor() {
- std::this_thread::sleep_for(std::chrono::seconds(10));
-}
-
-static std::vector<std::string> responses;
-
-class ConfigHandler : public CivetHandler {
- public:
- ConfigHandler() {
- calls_ = 0;
- }
- bool handlePost(CivetServer *server, struct mg_connection *conn) {
- calls_++;
- if (responses.size() > 0) {
- std::string top_str = responses.back();
- responses.pop_back();
- mg_printf(conn, "HTTP/1.1 200 OK\r\nContent-Type: "
- "text/plain\r\nContent-Length: %lu\r\nConnection:
close\r\n\r\n",
- top_str.length());
- mg_printf(conn, "%s", top_str.c_str());
- } else {
- mg_printf(conn, "HTTP/1.1 500 Internal Server Error\r\n");
- }
-
- return true;
- }
-
- bool handleGet(CivetServer *server, struct mg_connection *conn) {
- std::ifstream myfile(test_file_location_.c_str());
-
- if (myfile.is_open()) {
- std::stringstream buffer;
- buffer << myfile.rdbuf();
- std::string str = buffer.str();
- myfile.close();
- mg_printf(conn, "HTTP/1.1 200 OK\r\nContent-Type: "
- "text/plain\r\nContent-Length: %lu\r\nConnection:
close\r\n\r\n",
- str.length());
- mg_printf(conn, "%s", str.c_str());
- } else {
- mg_printf(conn, "HTTP/1.1 500 Internal Server Error\r\n");
- }
-
- return true;
- }
- std::string test_file_location_;
- std::atomic<size_t> calls_;
-};
+#include "HTTPIntegrationBase.h"
+#include "HTTPHandlers.h"
int main(int argc, char **argv) {
- mg_init_library(0);
- LogTestController::getInstance().setInfo<minifi::FlowController>();
- LogTestController::getInstance().setTrace<minifi::utils::HTTPClient>();
- LogTestController::getInstance().setDebug<minifi::c2::RESTSender>();
- LogTestController::getInstance().setTrace<minifi::c2::C2Agent>();
-
- const char *options[] = { "document_root", ".", "listening_ports", "0", 0 };
- std::vector<std::string> cpp_options;
- for (int i = 0; i < (sizeof(options) / sizeof(options[0]) - 1); i++) {
- cpp_options.push_back(options[i]);
- }
-
- CivetServer server(cpp_options);
-
- std::string port_str = std::to_string(server.getListeningPorts()[0]);
-
- ConfigHandler h_ex;
- server.addHandler("/update", h_ex);
- std::string key_dir, test_file_location;
- if (argc > 1) {
- h_ex.test_file_location_ = test_file_location = argv[1];
- key_dir = argv[2];
- }
- std::string heartbeat_response = "{\"operation\" :
\"heartbeat\",\"requested_operations\": [ {"
- "\"operation\" : \"update\", "
- "\"operationid\" : \"8675309\", "
- "\"name\": \"agent\""
- "}]}";
-
- responses.push_back(heartbeat_response);
+ const cmd_args args = parse_cmdline_args(argc, argv, "update");
+ C2UpdateHandler handler(args.test_file);
+ VerifyC2UpdateAgent harness(10000);
+ harness.setKeyDir(args.key_dir);
+ harness.setUrl(args.url, &handler);
+ handler.setC2RestResponse(harness.getC2RestUrl(), "agent");
- std::ifstream myfile(test_file_location.c_str());
-
- std::string c2_rest_url = "http://localhost:"; + port_str + "/update";
-
-
- if (myfile.is_open()) {
- std::stringstream buffer;
- buffer << myfile.rdbuf();
- std::string str = buffer.str();
- myfile.close();
- std::string response = "{\"operation\" :
\"heartbeat\",\"requested_operations\": [ {"
- "\"operation\" : \"update\", "
- "\"operationid\" : \"8675309\", "
- "\"name\": \"agent\", \"content\": { \"location\": \"" + c2_rest_url
+"\"}}]}";
- responses.push_back(response);
- }
-
- std::shared_ptr<minifi::Configure> configuration =
std::make_shared<minifi::Configure>();
-
- configuration->set("nifi.c2.agent.protocol.class", "RESTSender");
- configuration->set("nifi.c2.enable", "true");
- configuration->set("nifi.c2.agent.class", "test");
- configuration->set("nifi.c2.agent.update.allow","true");
- configuration->set("nifi.c2.rest.url", c2_rest_url);
- configuration->set("nifi.c2.agent.heartbeat.period", "1000");
-
- std::shared_ptr<core::Repository> test_repo =
std::make_shared<TestRepository>();
- std::shared_ptr<core::Repository> test_flow_repo =
std::make_shared<TestFlowRepository>();
-
- configuration->set(minifi::Configure::nifi_flow_configuration_file,
test_file_location);
- configuration->set("c2.agent.update.command", "echo \"verification
command\"");
-
- std::shared_ptr<minifi::io::StreamFactory> stream_factory =
minifi::io::StreamFactory::getInstance(configuration);
- std::shared_ptr<core::ContentRepository> content_repo =
std::make_shared<core::repository::VolatileContentRepository>();
- std::unique_ptr<core::FlowConfiguration> yaml_ptr =
std::unique_ptr<core::YamlConfiguration>(
- new core::YamlConfiguration(test_repo, test_repo, content_repo,
stream_factory, configuration, test_file_location));
- std::shared_ptr<TestRepository> repo =
std::static_pointer_cast<TestRepository>(test_repo);
-
- std::shared_ptr<minifi::FlowController> controller =
std::make_shared<minifi::FlowController>(test_repo, test_flow_repo,
configuration, std::move(yaml_ptr), content_repo, DEFAULT_ROOT_GROUP_NAME,
-
true);
-
- core::YamlConfiguration yaml_config(test_repo, test_repo, content_repo,
stream_factory, configuration, test_file_location);
-
- std::unique_ptr<core::ProcessGroup> ptr =
yaml_config.getRoot(test_file_location);
- std::shared_ptr<core::ProcessGroup> pg =
std::shared_ptr<core::ProcessGroup>(ptr.get());
- ptr.release();
auto start = std::chrono::system_clock::now();
- controller->load();
- controller->start();
- waitToVerifyProcessor();
+ harness.run(args.test_file);
- controller->waitUnload(60000);
auto then = std::chrono::system_clock::now();
-
auto milliseconds =
std::chrono::duration_cast<std::chrono::milliseconds>(then - start).count();
- std::string logs = LogTestController::getInstance().log_output.str();
- assert(logs.find("removing file") != std::string::npos);
- assert(logs.find("May not have command processor") != std::string::npos);
- LogTestController::getInstance().reset();
- assert(h_ex.calls_ <= (milliseconds / 1000) + 1);
-
+ assert(handler.calls_ <= (milliseconds / 1000) + 1);
Review comment:
I agree. I think in every place the type should reflect the dimension of
the variable. For time dimension we have std::chrono. For others, like length
or file size... we might need 3rd party, such as units library.
##########
File path: extensions/http-curl/tests/HttpGetIntegrationTest.cpp
##########
@@ -17,46 +17,31 @@
*/
#define CURLOPT_SSL_VERIFYPEER_DISABLE 1
-#include <sys/stat.h>
#undef NDEBUG
#include <cassert>
#include <utility>
#include <chrono>
-#include <fstream>
#include <memory>
#include <string>
#include <thread>
-#include <type_traits>
#include <vector>
#include "HTTPClient.h"
#include "InvokeHTTP.h"
#include "TestServer.h"
#include "TestBase.h"
#include "utils/StringUtils.h"
-#include "core/Core.h"
-#include "core/logging/Logger.h"
-#include "core/ProcessGroup.h"
#include "core/yaml/YamlConfiguration.h"
#include "FlowController.h"
#include "properties/Configure.h"
#include "unit/ProvenanceTestHelper.h"
#include "io/StreamFactory.h"
-#include "processors/InvokeHTTP.h"
-#include "processors/ListenHTTP.h"
#include "processors/LogAttribute.h"
+#include "HTTPIntegrationBase.h"
-void waitToVerifyProcessor() {
- std::this_thread::sleep_for(std::chrono::seconds(10));
-}
-
-int log_message(const struct mg_connection *conn, const char *message) {
- puts(message);
- return 1;
-}
-
-int ssl_enable(void* /*ssl_context*/, void* /*user_data*/) {
- puts("Enable ssl");
- return 0;
+namespace {
+ void waitToVerifyProcessor() {
+ std::this_thread::sleep_for(std::chrono::seconds(10));
Review comment:
I'm thinking about it, i hope there is a way to prevent using sleep at
all (condition?).
##########
File path: extensions/http-curl/tests/HttpGetIntegrationTest.cpp
##########
@@ -121,12 +104,12 @@ int main(int argc, char **argv) {
inv->getProperty(minifi::processors::InvokeHTTP::URL.getName(), url);
HttpResponder h_ex;
std::string port, scheme, path;
- CivetServer *server = nullptr;
+ std::unique_ptr<CivetServer> server = nullptr;
parse_http_components(url, port, scheme, path);
struct mg_callbacks callback{};
if (scheme == "https") {
Review comment:
Done.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
