fgerlits commented on code in PR #1383:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1383#discussion_r954762504
##########
extensions/script/ExecuteScript.h:
##########
@@ -185,23 +124,11 @@ class ExecuteScript : public core::Processor {
ScriptEngineFactory engine_factory_;
#ifdef LUA_SUPPORT
- std::unique_ptr<ScriptEngineQueue<lua::LuaScriptEngine>> script_engine_q_;
+ std::shared_ptr<utils::ResourceQueue<lua::LuaScriptEngine>>
lua_script_engine_queue_;
Review Comment:
why did `lua_script_engine_queue_` change from a `unique_ptr` to a
`shared_ptr`?
##########
libminifi/test/unit/ResourceQueueTests.cpp:
##########
@@ -0,0 +1,90 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include <chrono>
+#include <set>
+
+#include "ResourceQueue.h"
+#include "../TestBase.h"
+#include "../Catch.h"
+#include "logging/LoggerConfiguration.h"
+
+using namespace std::literals::chrono_literals;
+
+namespace org::apache::nifi::minifi::utils::testing {
+
+TEST_CASE("maximum_number_of_creatable_resources", "[utils::ResourceQueue]") {
+ std::shared_ptr<core::logging::Logger>
logger_{core::logging::LoggerFactory<ResourceQueue<int>>::getLogger()};
+ LogTestController::getInstance().setTrace<ResourceQueue<int>>();
+
+ std::set<int> resources_created;
+
+ auto worker = [&](int value, const std::shared_ptr<ResourceQueue<int>>&
resource_queue) {
+ auto resource = resource_queue->getResource([value]{return
std::make_unique<int>(value);});
+ std::this_thread::sleep_for(10ms);
+ resources_created.emplace(*resource);
+ };
+
+ SECTION("Maximum 2 resources") {
+ auto resource_queue = ResourceQueue<int>::create(2, logger_);
+ std::thread thread_one{[&] { worker(1, resource_queue); }};
+ std::thread thread_two{[&] { worker(2, resource_queue); }};
+ std::thread thread_three{[&] { worker(3, resource_queue); }};
+
+ thread_one.join();
+ thread_two.join();
+ thread_three.join();
+
+ CHECK(!resources_created.empty());
+ CHECK(resources_created.size() <= 2);
+ }
+
+
+ SECTION("No Maximum resources") {
+ auto resource_queue = ResourceQueue<int>::create(2, logger_);
Review Comment:
typo:
```suggestion
auto resource_queue = ResourceQueue<int>::create(std::nullopt, logger_);
```
##########
libminifi/test/unit/ResourceQueueTests.cpp:
##########
@@ -0,0 +1,90 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include <chrono>
+#include <set>
+
+#include "ResourceQueue.h"
+#include "../TestBase.h"
+#include "../Catch.h"
+#include "logging/LoggerConfiguration.h"
+
+using namespace std::literals::chrono_literals;
+
+namespace org::apache::nifi::minifi::utils::testing {
+
+TEST_CASE("maximum_number_of_creatable_resources", "[utils::ResourceQueue]") {
+ std::shared_ptr<core::logging::Logger>
logger_{core::logging::LoggerFactory<ResourceQueue<int>>::getLogger()};
+ LogTestController::getInstance().setTrace<ResourceQueue<int>>();
+
+ std::set<int> resources_created;
+
+ auto worker = [&](int value, const std::shared_ptr<ResourceQueue<int>>&
resource_queue) {
+ auto resource = resource_queue->getResource([value]{return
std::make_unique<int>(value);});
+ std::this_thread::sleep_for(10ms);
+ resources_created.emplace(*resource);
+ };
+
+ SECTION("Maximum 2 resources") {
+ auto resource_queue = ResourceQueue<int>::create(2, logger_);
+ std::thread thread_one{[&] { worker(1, resource_queue); }};
+ std::thread thread_two{[&] { worker(2, resource_queue); }};
+ std::thread thread_three{[&] { worker(3, resource_queue); }};
+
+ thread_one.join();
+ thread_two.join();
+ thread_three.join();
+
+ CHECK(!resources_created.empty());
+ CHECK(resources_created.size() <= 2);
+ }
+
+
+ SECTION("No Maximum resources") {
+ auto resource_queue = ResourceQueue<int>::create(2, logger_);
+ std::thread thread_one{[&] { worker(1, resource_queue); }};
+ std::thread thread_two{[&] { worker(2, resource_queue); }};
+ std::thread thread_three{[&] { worker(3, resource_queue); }};
+
+ thread_one.join();
+ thread_two.join();
+ thread_three.join();
+
+ CHECK(!resources_created.empty());
+ CHECK(resources_created.size() <= 3);
Review Comment:
This is a weak check, as it was passing in spite of the typo. Maybe split
the two sections into separate test cases, and save the resource wrapper
instead of the resource in the unlimited case? There is probably a better way.
##########
extensions/http-curl/tests/unit/ConnectionCountingServer.h:
##########
@@ -0,0 +1,149 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#pragma once
+
+#include <array>
+#include <vector>
+#include <string>
+#include <set>
+#include "CivetServer.h"
+
+namespace org::apache::nifi::minifi::extensions::curl::testing {
+
+namespace details {
+
+class NumberedMethodResponder : public CivetHandler {
+ public:
+ explicit NumberedMethodResponder(std::set<utils::SmallString<36>>&
connections) : connections_(connections) {}
+
+ bool handleGet(CivetServer*, struct mg_connection* conn) override {
+ sendNumberedMessage("GET", conn);
+ return true;
+ }
+
+ bool handlePost(CivetServer*, struct mg_connection* conn) override {
+ sendNumberedMessage("POST", conn);
+ return true;
+ }
+
+ bool handlePut(CivetServer*, struct mg_connection* conn) override {
+ sendNumberedMessage("PUT", conn);
+ return true;
+ }
+
+ bool handleHead(CivetServer*, struct mg_connection* conn) override {
+ sendNumberedMessage("HEAD", conn);
+ return true;
+ }
+
+ private:
+ void sendNumberedMessage(std::string body, struct mg_connection* conn) {
+ saveConnectionId(conn);
+ body.append(std::to_string(response_id_));
+ mg_printf(conn, "HTTP/1.1 200 OK\r\n");
+ mg_printf(conn, "Content-length: %lu\r\n", body.length());
+ mg_printf(conn, "Response-number: %" PRIu64 "\r\n", response_id_);
+ mg_printf(conn, "\r\n");
+ mg_printf(conn, body.data(), body.length());
+ ++response_id_;
+ }
+
+ void saveConnectionId(struct mg_connection* conn) {
+ auto user_connection_data =
reinterpret_cast<utils::SmallString<36>*>(mg_get_user_connection_data(conn));
+ connections_.emplace(*user_connection_data);
Review Comment:
an assertion here that `user_connection_data` is not null would make me
sleep better
##########
extensions/http-curl/tests/unit/ConnectionCountingServer.h:
##########
@@ -0,0 +1,149 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#pragma once
+
+#include <array>
+#include <vector>
+#include <string>
+#include <set>
+#include "CivetServer.h"
+
+namespace org::apache::nifi::minifi::extensions::curl::testing {
+
+namespace details {
+
+class NumberedMethodResponder : public CivetHandler {
+ public:
+ explicit NumberedMethodResponder(std::set<utils::SmallString<36>>&
connections) : connections_(connections) {}
+
+ bool handleGet(CivetServer*, struct mg_connection* conn) override {
+ sendNumberedMessage("GET", conn);
+ return true;
+ }
+
+ bool handlePost(CivetServer*, struct mg_connection* conn) override {
+ sendNumberedMessage("POST", conn);
+ return true;
+ }
+
+ bool handlePut(CivetServer*, struct mg_connection* conn) override {
+ sendNumberedMessage("PUT", conn);
+ return true;
+ }
+
+ bool handleHead(CivetServer*, struct mg_connection* conn) override {
+ sendNumberedMessage("HEAD", conn);
+ return true;
+ }
+
+ private:
+ void sendNumberedMessage(std::string body, struct mg_connection* conn) {
+ saveConnectionId(conn);
+ body.append(std::to_string(response_id_));
+ mg_printf(conn, "HTTP/1.1 200 OK\r\n");
+ mg_printf(conn, "Content-length: %lu\r\n", body.length());
+ mg_printf(conn, "Response-number: %" PRIu64 "\r\n", response_id_);
+ mg_printf(conn, "\r\n");
+ mg_printf(conn, body.data(), body.length());
+ ++response_id_;
+ }
+
+ void saveConnectionId(struct mg_connection* conn) {
+ auto user_connection_data =
reinterpret_cast<utils::SmallString<36>*>(mg_get_user_connection_data(conn));
+ connections_.emplace(*user_connection_data);
+ }
+
+ uint64_t response_id_ = 0;
+ std::set<utils::SmallString<36>>& connections_;
+};
+
+class ReverseBodyPostHandler : public CivetHandler {
+ public:
+ explicit ReverseBodyPostHandler(std::set<utils::SmallString<36>>&
connections) : connections_(connections) {}
+
+ bool handlePost(CivetServer* /*server*/, struct mg_connection* conn)
override {
+ saveConnectionId(conn);
+ std::vector<char> request_body;
+ request_body.reserve(2048);
+ size_t read_size = mg_read(conn, request_body.data(), 2048);
Review Comment:
we could add an assertion that `read_size` is less than 2048, as that should
be true in all tests
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
