You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by "martinzink (via GitHub)" <gi...@apache.org> on 2026/05/15 11:03:58 UTC
[PR] MINIFICPP-2816 Add controller_service::api_implementations to C api [nifi-minifi-cpp]
martinzink opened a new pull request, #2176:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2176
Thank you for submitting a contribution to Apache NiFi - MiNiFi C++.
In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:
### For all changes:
- [ ] Is there a JIRA ticket associated with this PR? Is it referenced
in the commit message?
- [ ] Does your PR title start with MINIFICPP-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.
- [ ] Has your PR been rebased against the latest commit within the target branch (typically main)?
- [ ] Is your initial contribution a single, squashed commit?
### For code changes:
- [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)?
- [ ] If applicable, have you updated the LICENSE file?
- [ ] If applicable, have you updated the NOTICE file?
### For documentation related changes:
- [ ] Have you ensured that format looks appropriate for the output in which it is rendered?
### Note:
Please ensure that once the PR is submitted, you check GitHub Actions CI results for build issues and submit an update to your PR as soon as possible.
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2816 Add controller_service::api_implementations to C api [nifi-minifi-cpp]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #2176:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2176#discussion_r3248228437
##########
minifi-api/include/minifi-c/minifi-c.h:
##########
@@ -207,11 +208,18 @@ typedef struct MinifiProcessorClassDefinition {
MinifiProcessorCallbacks callbacks;
} MinifiProcessorClassDefinition;
+struct MinifiControllerServiceProvidedAPI {
+ MinifiStringView type;
+ void*(*cast_api)(void*);
+};
+
Review Comment:
The `MinifiControllerServiceProvidedAPI` struct is declared here but never used anywhere in the codebase. The `provided_interfaces_ptr` field added to `MinifiControllerServiceClassDefinition` below uses `const MinifiStringView*`, not this struct. The `cast_api` function pointer concept it carries is implemented via the unrelated `get_interface` callback on `MinifiControllerServiceCallbacks`. Either remove this dead struct or refactor `provided_interfaces_ptr` to use it (which would obviate the global `get_interface` callback).
##########
libminifi/src/minifi-c.cpp:
##########
@@ -277,15 +277,23 @@ void useCControllerServiceClassDescription(const MinifiControllerServiceClassDef
auto name_segments = minifi::utils::string::split(toStringView(class_description.full_name), "::");
gsl_Assert(!name_segments.empty());
- minifi::ClassDescription description{
- .type_ = minifi::ResourceType::ControllerService,
- .short_name_ = name_segments.back(),
- .full_name_ = minifi::utils::string::join(".", name_segments),
- .description_ = toString(class_description.description),
- .class_properties_ = properties,
+ std::vector<core::ControllerServiceType> implements_apis;
+ implements_apis.reserve(class_description.provided_interfaces_count);
+ for (size_t i = 0; i < class_description.provided_interfaces_count; ++i) {
+ auto api_segments = string::split(toStringView(class_description.provided_interfaces_ptr[i]), "::");
+ implements_apis.push_back(core::ControllerServiceType::minifiSystemControllerServiceType(string::join(".", api_segments)));
+ }
Review Comment:
All provided interfaces from any C-API extension are recorded with a hardcoded artifact `"minifi-system"` and group `"org.apache.nifi.minifi"`. C-API extensions are registered as their own bundles (e.g. via `register_c_api_extension`), so this loses the actual bundle/artifact identity of the extension that defines the API. As a result, `providedApiImplementations` reported in the agent manifest will not correctly identify which extension/bundle the API interface comes from, which is the main consumer of `api_implementations`. The artifact/group should be derived from the registering extension (or passed through the C API) rather than hardcoded.
##########
minifi-api/common/include/minifi-cpp/core/ProvidedControllerServiceInterface.h:
##########
@@ -0,0 +1,34 @@
+/**
+ * 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 <string_view>
+#include "core/ClassName.h"
+
+namespace org::apache::nifi::minifi::core {
Review Comment:
Header `core/ClassName.h` lives in `core-framework/common/include`, while this file lives in `minifi-api/common/include`. Including it here introduces a dependency from `minifi-api` on `core-framework`, which appears to invert the intended layering (other headers under `minifi-cpp/core/` avoid pulling in `core-framework`). Consider moving `ClassName.h` (or just the `className<>()` template) into `minifi-api` to keep the dependency direction clean.
##########
extensions/stable-api-sandbox/tests/ZooTests.cpp:
##########
@@ -0,0 +1,71 @@
+/**
+ *
+ * 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 "AnimalControllerServices.h"
+#include "CProcessorTestUtils.h"
+#include "ZooProcessor.h"
+#include "api/core/Resource.h"
+#include "minifi-cpp/core/FlowFile.h"
+#include "unit/Catch.h"
+#include "unit/SingleProcessorTestController.h"
+#include "unit/TestBase.h"
+#include "unit/TestUtils.h"
+#include "utils/CProcessor.h"
+
+namespace org::apache::nifi::minifi::api_sandbox::test {
+TEST_CASE("ZooTest") {
+ minifi::test::SingleProcessorTestController controller(minifi::test::utils::make_custom_c_processor<ZooProcessor>(
+ core::ProcessorMetadata{utils::Identifier{}, "ZooProcessor", logging::LoggerFactory<ZooProcessor>::getLogger()}));
+ const auto dog_with_jetpack = minifi::test::utils::make_custom_c_controller_service<DogController>(core::ControllerServiceMetadata{
+ utils::Identifier{},
+ "DogController",
+ logging::LoggerFactory<DogController>::getLogger()});
+ auto dog_with_jetpack_node = controller.plan->addController("dog_with_jetpack", dog_with_jetpack);
+ CHECK(dog_with_jetpack->setProperty(DogController::HasJetpack.name, "true"));
+
+ const auto duck = minifi::test::utils::make_custom_c_controller_service<DuckController>(core::ControllerServiceMetadata{utils::Identifier{},
+ "DuckController",
+ logging::LoggerFactory<DogController>::getLogger()});
Review Comment:
The `DuckController` logger is created via `LoggerFactory<DogController>::getLogger()` instead of `LoggerFactory<DuckController>::getLogger()`. This appears to be a copy-paste bug that will misattribute log output from the duck controller.
##########
extensions/stable-api-sandbox/tests/ZooTests.cpp:
##########
@@ -0,0 +1,71 @@
+/**
+ *
+ * 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 "AnimalControllerServices.h"
+#include "CProcessorTestUtils.h"
+#include "ZooProcessor.h"
+#include "api/core/Resource.h"
+#include "minifi-cpp/core/FlowFile.h"
+#include "unit/Catch.h"
+#include "unit/SingleProcessorTestController.h"
+#include "unit/TestBase.h"
+#include "unit/TestUtils.h"
+#include "utils/CProcessor.h"
+
+namespace org::apache::nifi::minifi::api_sandbox::test {
+TEST_CASE("ZooTest") {
+ minifi::test::SingleProcessorTestController controller(minifi::test::utils::make_custom_c_processor<ZooProcessor>(
+ core::ProcessorMetadata{utils::Identifier{}, "ZooProcessor", logging::LoggerFactory<ZooProcessor>::getLogger()}));
+ const auto dog_with_jetpack = minifi::test::utils::make_custom_c_controller_service<DogController>(core::ControllerServiceMetadata{
+ utils::Identifier{},
+ "DogController",
+ logging::LoggerFactory<DogController>::getLogger()});
+ auto dog_with_jetpack_node = controller.plan->addController("dog_with_jetpack", dog_with_jetpack);
+ CHECK(dog_with_jetpack->setProperty(DogController::HasJetpack.name, "true"));
+
+ const auto duck = minifi::test::utils::make_custom_c_controller_service<DuckController>(core::ControllerServiceMetadata{utils::Identifier{},
+ "DuckController",
+ logging::LoggerFactory<DogController>::getLogger()});
+ auto duck_node = controller.plan->addController("duck", duck);
+
+ {
+ CHECK(controller.getProcessor()->setProperty(ZooProcessor::CanFlyService.name, "dog_with_jetpack"));
+ CHECK(controller.getProcessor()->setProperty(ZooProcessor::NumberOfLegsService.name, "duck"));
+ const auto& result = controller.trigger();
+ CHECK(LogTestController::getInstance().contains("[org::apache::nifi::minifi::api_sandbox::ZooProcessor] [critical] Can fly? true"));
+ CHECK(LogTestController::getInstance().contains("[org::apache::nifi::minifi::api_sandbox::ZooProcessor] [critical] Num of legs? 2"));
+ }
+ {
+ LogTestController::getInstance().clear();
+ CHECK(controller.getProcessor()->setProperty(ZooProcessor::CanFlyService.name, "duck"));
+ CHECK(controller.getProcessor()->setProperty(ZooProcessor::NumberOfLegsService.name, "duck"));
+ const auto& result = controller.trigger();
+ CHECK(LogTestController::getInstance().contains("[org::apache::nifi::minifi::api_sandbox::ZooProcessor] [critical] Can fly? true"));
+ CHECK(LogTestController::getInstance().contains("[org::apache::nifi::minifi::api_sandbox::ZooProcessor] [critical] Num of legs? 2"));
+ }
+ {
+ LogTestController::getInstance().clear();
+ CHECK(controller.getProcessor()->setProperty(ZooProcessor::CanFlyService.name, "duck"));
+ CHECK(controller.getProcessor()->setProperty(ZooProcessor::NumberOfLegsService.name, "invalid"));
+ const auto& result = controller.trigger();
Review Comment:
The `result` returned by `controller.trigger()` is declared but never used in any of the three test blocks (lines 49, 57, 65). Either assert something on the trigger result (e.g. that it produced a flow file or routed to a specific relationship) or drop the binding to keep the test intent clear.
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2816 Add controller_service::api_implementations to C api [nifi-minifi-cpp]
Posted by "szaszm (via GitHub)" <gi...@apache.org>.
szaszm commented on code in PR #2176:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2176#discussion_r3414397354
##########
extensions/stable-api-sandbox/CMakeLists.txt:
##########
Review Comment:
I'm thinking we should call this stable-api-tests or something like that, so even at first glance it's clear that this is a test-only extension.
##########
extension-framework/cpp-extension-lib/include/api/core/Resource.h:
##########
@@ -181,6 +191,20 @@ void useControllerServiceClassDefinition(Fn&& fn) {
static_cast<Class*>(self)->disable();
} catch (...) {}
},
+ .get_interface = [](void* self, MinifiStringView interface_name) -> void* {
+ try {
+ const std::string_view name_view{interface_name.data, interface_name.length};
+
+ if constexpr (requires { Class::ProvidedInterfaces; }) {
Review Comment:
tiny thing, doesn't really matter, but I'd only create the view if it's needed.
```suggestion
if constexpr (requires { Class::ProvidedInterfaces; }) {
const std::string_view name_view{interface_name.data, interface_name.length};
```
##########
extensions/stable-api-sandbox/ZooProcessor.h:
##########
@@ -0,0 +1,63 @@
+/**
+ * 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 "AnimalControllerServiceApis.h"
+#include "api/core/ProcessorImpl.h"
+#include "api/utils/Export.h"
+#include "core/PropertyDefinitionBuilder.h"
+#include "minifi-cpp/core/Annotation.h"
+
+namespace org::apache::nifi::minifi::api_sandbox {
+
+class ZooProcessor : public api::core::ProcessorImpl {
+ public:
+ EXTENSIONAPI static constexpr const char* Description = "Test ZooProcessor";
+
+ EXTENSIONAPI static constexpr auto CanFlyService = core::PropertyDefinitionBuilder<>::createProperty("Can fly service")
+ .withDescription("Test CanFlyService")
+ .isRequired(true)
+ .withAllowedTypes<CanFlyControllerApi>()
+ .build();
+
+ EXTENSIONAPI static constexpr auto NumberOfLegsService = core::PropertyDefinitionBuilder<>::createProperty("Number of legs service")
+ .withDescription("Test NumberOfLegsService")
+ .isRequired(true)
+ .withAllowedTypes<NumberOfLegsControllerApi>()
+ .build();
+
Review Comment:
same here
##########
minifi-api/common/include/minifi-cpp/core/ProvidedControllerServiceInterface.h:
##########
@@ -0,0 +1,43 @@
+/**
+ * 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 <string_view>
+#include <type_traits>
+
+#include "core/ClassName.h"
+
+namespace org::apache::nifi::minifi::core {
+
+struct ProvidedInterface {
+ std::string_view name;
+ void* (*cast)(void* instance);
Review Comment:
```suggestion
/// fully-qualified class name of the implemented ControllerService interface
std::string_view name;
/// casts a pointer of the ControllerService from its dynamic type to a pointer of an implemented interface type. Needed to support cases where they are not the same, like multiple inheritance, virtual inheritance, or contained non-first member implementing the interface
void* (*cast)(void* instance);
```
##########
minifi-api/common/include/minifi-cpp/core/ProvidedControllerServiceInterface.h:
##########
@@ -0,0 +1,43 @@
+/**
+ * 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 <string_view>
+#include <type_traits>
+
+#include "core/ClassName.h"
+
+namespace org::apache::nifi::minifi::core {
+
+struct ProvidedInterface {
+ std::string_view name;
+ void* (*cast)(void* instance);
+};
+
+template<typename Interface, typename ConcreteService>
+void* castProvidedControllerServiceInterface(void* instance) {
+ auto* concrete = static_cast<ConcreteService*>(instance);
+ auto* interface_ptr = static_cast<Interface*>(concrete);
+ return interface_ptr;
+}
+
+template<typename Interface, typename ConcreteService>
+constexpr ProvidedInterface createProvidedInterface() {
+ return ProvidedInterface{className<Interface>(), &castProvidedControllerServiceInterface<Interface, ConcreteService>};
Review Comment:
```suggestion
return ProvidedInterface{className<Interface>(), &castProvidedControllerServiceToInterface<Interface, ConcreteService>};
```
##########
minifi-api/common/include/minifi-cpp/core/ProvidedControllerServiceInterface.h:
##########
@@ -0,0 +1,43 @@
+/**
+ * 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 <string_view>
+#include <type_traits>
+
+#include "core/ClassName.h"
+
+namespace org::apache::nifi::minifi::core {
+
+struct ProvidedInterface {
+ std::string_view name;
+ void* (*cast)(void* instance);
+};
+
+template<typename Interface, typename ConcreteService>
Review Comment:
```suggestion
/// casts a pointer of the ControllerService from its dynamic type to a pointer of an implemented interface type. Needed to support cases where they are not the same, like multiple inheritance, virtual inheritance, or contained non-first member implementing the interface
template<typename Interface, typename ConcreteService>
```
##########
extensions/stable-api-sandbox/ZooProcessor.h:
##########
@@ -0,0 +1,63 @@
+/**
+ * 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 "AnimalControllerServiceApis.h"
+#include "api/core/ProcessorImpl.h"
+#include "api/utils/Export.h"
+#include "core/PropertyDefinitionBuilder.h"
+#include "minifi-cpp/core/Annotation.h"
+
+namespace org::apache::nifi::minifi::api_sandbox {
+
+class ZooProcessor : public api::core::ProcessorImpl {
+ public:
+ EXTENSIONAPI static constexpr const char* Description = "Test ZooProcessor";
+
+ EXTENSIONAPI static constexpr auto CanFlyService = core::PropertyDefinitionBuilder<>::createProperty("Can fly service")
+ .withDescription("Test CanFlyService")
+ .isRequired(true)
+ .withAllowedTypes<CanFlyControllerApi>()
+ .build();
Review Comment:
Continuation indentation should be 4 spaces deeper. I'm against trying to maintain alignments, because we get right-heavy lines like these, plus renames and similar changes break alignment.
##########
extensions/stable-api-sandbox/ZooProcessor.cpp:
##########
@@ -0,0 +1,46 @@
+/**
+ * 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 "ZooProcessor.h"
+
+#include "AnimalControllerServices.h"
+#include "api/core/ProcessContext.h"
+
+namespace org::apache::nifi::minifi::api_sandbox {
+
+MinifiStatus ZooProcessor::onTriggerImpl(api::core::ProcessContext& process_context, api::core::ProcessSession& process_session) {
+ if (const auto can_fly_opaque = process_context.getControllerService(CanFlyService)) {
+ if (*can_fly_opaque) {
+ const auto can_fly_controller_name = process_context.getProperty(CanFlyService, nullptr) | utils::orThrow("Should be here");
+ const CanFlyControllerApi* can_fly = reinterpret_cast<CanFlyControllerApi*>(*can_fly_opaque);
+ logger_->log_critical("Can {} fly? {}", can_fly_controller_name, can_fly->canFly());
+ }
+ }
+ if (const auto num_of_legs_opaque = process_context.getControllerService(NumberOfLegsService)) {
+ if (*num_of_legs_opaque) {
+ const auto num_of_legs_name = process_context.getProperty(NumberOfLegsService, nullptr) | utils::orThrow("Should be here");
+ const NumberOfLegsControllerApi* num_legs = reinterpret_cast<NumberOfLegsControllerApi*>(*num_of_legs_opaque);
Review Comment:
```suggestion
const NumberOfLegsControllerApi* const num_legs = reinterpret_cast<NumberOfLegsControllerApi*>(*num_of_legs_opaque);
```
##########
libminifi/test/unit/ComponentManifestTests.cpp:
##########
@@ -38,6 +39,25 @@ SerializedResponseNode& get(SerializedResponseNode& node, const std::string& fie
throw std::logic_error("No field '" + field + "'");
}
+namespace org::apache::nifi::minifi::test::apple {
+class ExampleApacheService : public core::controller::ControllerServiceBase, public core::controller::ControllerServiceHandle {
+ public:
+ using ControllerServiceBase::ControllerServiceBase;
+
+ void initialize() override {}
+ void onEnable() override {}
+
+ [[nodiscard]] ControllerServiceHandle* getControllerServiceHandle() override { return this; }
Review Comment:
Maybe this should be added to the macro ADD_COMMON_VIRTUAL_FUNCTIONS_FOR_CONTROLLER_SERVICES, tho it doesn't strictly belong in the scope of this change, so you decide
##########
extensions/stable-api-sandbox/ZooProcessor.cpp:
##########
@@ -0,0 +1,46 @@
+/**
+ * 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 "ZooProcessor.h"
+
+#include "AnimalControllerServices.h"
+#include "api/core/ProcessContext.h"
+
+namespace org::apache::nifi::minifi::api_sandbox {
+
+MinifiStatus ZooProcessor::onTriggerImpl(api::core::ProcessContext& process_context, api::core::ProcessSession& process_session) {
+ if (const auto can_fly_opaque = process_context.getControllerService(CanFlyService)) {
+ if (*can_fly_opaque) {
+ const auto can_fly_controller_name = process_context.getProperty(CanFlyService, nullptr) | utils::orThrow("Should be here");
+ const CanFlyControllerApi* can_fly = reinterpret_cast<CanFlyControllerApi*>(*can_fly_opaque);
Review Comment:
```suggestion
const CanFlyControllerApi* const can_fly = reinterpret_cast<CanFlyControllerApi*>(*can_fly_opaque);
```
##########
minifi-api/common/include/minifi-cpp/core/ProvidedControllerServiceInterface.h:
##########
@@ -0,0 +1,43 @@
+/**
+ * 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 <string_view>
+#include <type_traits>
+
+#include "core/ClassName.h"
+
+namespace org::apache::nifi::minifi::core {
+
+struct ProvidedInterface {
+ std::string_view name;
+ void* (*cast)(void* instance);
+};
+
+template<typename Interface, typename ConcreteService>
+void* castProvidedControllerServiceInterface(void* instance) {
Review Comment:
```suggestion
void* castProvidedControllerServiceToInterface(void* instance) {
```
##########
extensions/stable-api-sandbox/tests/ZooTests.cpp:
##########
@@ -0,0 +1,71 @@
+/**
+ *
+ * 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 "../AnimalControllerServices.h"
+#include "CProcessorTestUtils.h"
+#include "ZooProcessor.h"
+#include "api/core/Resource.h"
+#include "minifi-cpp/core/FlowFile.h"
+#include "unit/Catch.h"
+#include "unit/SingleProcessorTestController.h"
+#include "unit/TestBase.h"
+#include "unit/TestUtils.h"
+#include "utils/CProcessor.h"
+
+namespace org::apache::nifi::minifi::api_sandbox::test {
+TEST_CASE("ZooTest") {
+ minifi::test::SingleProcessorTestController controller(minifi::test::utils::make_custom_c_processor<ZooProcessor>(
+ core::ProcessorMetadata{utils::Identifier{}, "ZooProcessor", logging::LoggerFactory<ZooProcessor>::getLogger()}));
+ const auto dog_with_jetpack = minifi::test::utils::make_custom_c_controller_service<DogController>(core::ControllerServiceMetadata{
+ utils::Identifier{},
+ "DogController",
+ logging::LoggerFactory<DogController>::getLogger()});
+ auto dog_with_jetpack_node = controller.plan->addController("dog_with_jetpack", dog_with_jetpack);
+ CHECK(dog_with_jetpack->setProperty(DogController::HasJetpack.name, "true"));
+
+ const auto duck = minifi::test::utils::make_custom_c_controller_service<DuckController>(core::ControllerServiceMetadata{utils::Identifier{},
+ "DuckController",
+ logging::LoggerFactory<DuckController>::getLogger()});
+ auto duck_node = controller.plan->addController("duck", duck);
+
+ {
+ CHECK(controller.getProcessor()->setProperty(ZooProcessor::CanFlyService.name, "dog_with_jetpack"));
+ CHECK(controller.getProcessor()->setProperty(ZooProcessor::NumberOfLegsService.name, "duck"));
+ controller.trigger();
+ CHECK(LogTestController::getInstance()
+ .contains("[org::apache::nifi::minifi::api_sandbox::ZooProcessor] [critical] Can dog_with_jetpack fly? true"));
+ CHECK(LogTestController::getInstance().contains("[org::apache::nifi::minifi::api_sandbox::ZooProcessor] [critical] duck has 2 legs"));
+ }
+ {
+ LogTestController::getInstance().clear();
+ CHECK(controller.getProcessor()->setProperty(ZooProcessor::CanFlyService.name, "duck"));
+ CHECK(controller.getProcessor()->setProperty(ZooProcessor::NumberOfLegsService.name, "dog_with_jetpack"));
+ controller.trigger();
+ CHECK(LogTestController::getInstance().contains("[org::apache::nifi::minifi::api_sandbox::ZooProcessor] [critical] Can duck fly? true"));
+ CHECK(LogTestController::getInstance().contains("[org::apache::nifi::minifi::api_sandbox::ZooProcessor] [critical] dog_with_jetpack has 4 legs"));
+ }
+ {
+ LogTestController::getInstance().clear();
+ CHECK(controller.getProcessor()->setProperty(ZooProcessor::CanFlyService.name, "duck"));
+ CHECK(controller.getProcessor()->setProperty(ZooProcessor::NumberOfLegsService.name, "invalid"));
Review Comment:
shouldn't this throw an error?
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2816 Add controller_service::api_implementations to C api [nifi-minifi-cpp]
Posted by "szaszm (via GitHub)" <gi...@apache.org>.
szaszm commented on code in PR #2176:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2176#discussion_r3461444247
##########
libminifi/src/core/ClassLoader.cpp:
##########
@@ -275,7 +275,7 @@ gsl::owner<CoreComponent*> ClassLoaderImpl::instantiateRaw(const std::string &cl
if (filter(obj)) {
return obj;
}
- delete gsl::owner<CoreComponent*>{obj};
+ delete obj; // NOLINT(cppcoreguidelines-owning-memory)
Review Comment:
it seems that if filter throws, obj is leaked. Bug not introduced here, and most likely not a problem in practice, since filter functions tend to not throw, I just noticed it.
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2816 Add controller_service::api_implementations to C api [nifi-minifi-cpp]
Posted by "martinzink (via GitHub)" <gi...@apache.org>.
martinzink commented on code in PR #2176:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2176#discussion_r3413416741
##########
libminifi/src/core/state/nodes/AgentInformation.cpp:
##########
@@ -17,46 +17,91 @@
*/
#include "core/state/nodes/AgentInformation.h"
-#include "minifi-cpp/agent/agent_version.h"
-#include "core/Resource.h"
#include "core/ClassLoader.h"
-#include "utils/OsUtils.h"
+#include "core/Resource.h"
#include "core/state/nodes/SchedulingNodes.h"
#include "core/state/nodes/SupportedOperations.h"
+#include "minifi-c/minifi-c.h"
+#include "minifi-cpp/agent/agent_version.h"
+#include "minifi-cpp/controllers/AttributeProviderService.h"
+#include "minifi-cpp/controllers/ProxyConfigurationServiceInterface.h"
+#include "minifi-cpp/controllers/RecordSetReader.h"
+#include "minifi-cpp/controllers/RecordSetWriter.h"
+#include "minifi-cpp/controllers/SSLContextServiceInterface.h"
+#include "utils/OsUtils.h"
namespace org::apache::nifi::minifi::state::response {
utils::ProcessCpuUsageTracker AgentStatus::cpu_load_tracker_;
std::mutex AgentStatus::cpu_load_tracker_mutex_;
namespace {
+
+std::string allowedTypeGroupName(const std::string_view allowed_type) {
+ constexpr std::string_view APACHE_GROUP_STR = "org.apache.nifi.minifi";
+ if (allowed_type.starts_with(APACHE_GROUP_STR)) {
+ return std::string{APACHE_GROUP_STR};
+ }
+ return std::string{allowed_type.substr(0, allowed_type.find_last_of('.'))};
+}
+
+constexpr std::string_view shortNameFromFullName(std::string_view sv) {
+ auto pos = sv.rfind('.');
+ return (pos == std::string_view::npos) ? sv : sv.substr(pos + 1);
+}
+
+bool minifiSystemAllowedType(const std::string_view allowed_type) {
+ return allowed_type == controllers::SSLContextServiceInterface::ProvidesApi.type
+ || allowed_type == core::RecordSetWriter::ProvidesApi.type
+ || allowed_type == core::RecordSetReader::ProvidesApi.type
+ || allowed_type == controllers::ProxyConfigurationServiceInterface::ProvidesApi.type
+ || allowed_type == controllers::AttributeProviderService::ProvidesApi.type;
Review Comment:
yeah the first one seems a good idea, but thats a whole new PR i think, I did the second one here [refactor minifiSystemAllowedType](https://github.com/apache/nifi-minifi-cpp/pull/2176/commits/5955a539da1399c685caa9a23f493ef5a92e6c48) not sure if thats what u had in mind
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2816 Add controller_service::api_implementations to C api [nifi-minifi-cpp]
Posted by "lordgamez (via GitHub)" <gi...@apache.org>.
lordgamez commented on code in PR #2176:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2176#discussion_r3362855876
##########
libminifi/src/core/state/nodes/AgentInformation.cpp:
##########
@@ -17,46 +17,91 @@
*/
#include "core/state/nodes/AgentInformation.h"
-#include "minifi-cpp/agent/agent_version.h"
-#include "core/Resource.h"
#include "core/ClassLoader.h"
-#include "utils/OsUtils.h"
+#include "core/Resource.h"
#include "core/state/nodes/SchedulingNodes.h"
#include "core/state/nodes/SupportedOperations.h"
+#include "minifi-c/minifi-c.h"
+#include "minifi-cpp/agent/agent_version.h"
+#include "minifi-cpp/controllers/AttributeProviderService.h"
+#include "minifi-cpp/controllers/ProxyConfigurationServiceInterface.h"
+#include "minifi-cpp/controllers/RecordSetReader.h"
+#include "minifi-cpp/controllers/RecordSetWriter.h"
+#include "minifi-cpp/controllers/SSLContextServiceInterface.h"
+#include "utils/OsUtils.h"
namespace org::apache::nifi::minifi::state::response {
utils::ProcessCpuUsageTracker AgentStatus::cpu_load_tracker_;
std::mutex AgentStatus::cpu_load_tracker_mutex_;
namespace {
+
+std::string allowedTypeGroupName(const std::string_view allowed_type) {
+ constexpr std::string_view APACHE_GROUP_STR = "org.apache.nifi.minifi";
+ if (allowed_type.starts_with(APACHE_GROUP_STR)) {
+ return std::string{APACHE_GROUP_STR};
+ }
+ return std::string{allowed_type.substr(0, allowed_type.find_last_of('.'))};
+}
+
+constexpr std::string_view shortNameFromFullName(std::string_view sv) {
+ auto pos = sv.rfind('.');
+ return (pos == std::string_view::npos) ? sv : sv.substr(pos + 1);
+}
+
+bool minifiSystemAllowedType(const std::string_view allowed_type) {
+ return allowed_type == controllers::SSLContextServiceInterface::ProvidesApi.type
+ || allowed_type == core::RecordSetWriter::ProvidesApi.type
+ || allowed_type == core::RecordSetReader::ProvidesApi.type
+ || allowed_type == controllers::ProxyConfigurationServiceInterface::ProvidesApi.type
+ || allowed_type == controllers::AttributeProviderService::ProvidesApi.type;
Review Comment:
Would it be possible to
1) create a static vector to store these types in and the controller classes could register themselves in at startup?
2) or if that's too complicated at least extract these to an array where they would be listed manually and check if the passed type exists there
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2816 Add controller_service::api_implementations to C api [nifi-minifi-cpp]
Posted by "martinzink (via GitHub)" <gi...@apache.org>.
martinzink commented on code in PR #2176:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2176#discussion_r3453591190
##########
libminifi/src/core/state/nodes/AgentInformation.cpp:
##########
@@ -17,46 +17,93 @@
*/
#include "core/state/nodes/AgentInformation.h"
-#include "minifi-cpp/agent/agent_version.h"
-#include "core/Resource.h"
#include "core/ClassLoader.h"
-#include "utils/OsUtils.h"
+#include "core/Resource.h"
#include "core/state/nodes/SchedulingNodes.h"
#include "core/state/nodes/SupportedOperations.h"
+#include "minifi-c/minifi-c.h"
+#include "minifi-cpp/agent/agent_version.h"
+#include "minifi-cpp/controllers/AttributeProviderService.h"
+#include "minifi-cpp/controllers/ProxyConfigurationServiceInterface.h"
+#include "minifi-cpp/controllers/RecordSetReader.h"
+#include "minifi-cpp/controllers/RecordSetWriter.h"
+#include "minifi-cpp/controllers/SSLContextServiceInterface.h"
+#include "utils/OsUtils.h"
+#include "range/v3/algorithm/contains.hpp"
namespace org::apache::nifi::minifi::state::response {
utils::ProcessCpuUsageTracker AgentStatus::cpu_load_tracker_;
std::mutex AgentStatus::cpu_load_tracker_mutex_;
namespace {
+
+std::string allowedTypeGroupName(const std::string_view allowed_type) {
+ constexpr std::string_view APACHE_GROUP_STR = "org.apache.nifi.minifi";
+ if (allowed_type.starts_with(APACHE_GROUP_STR)) {
+ return std::string{APACHE_GROUP_STR};
+ }
+ return std::string{allowed_type.substr(0, allowed_type.find_last_of('.'))};
+}
+
+constexpr std::string_view shortNameFromFullName(std::string_view sv) {
+ auto pos = sv.rfind('.');
+ return (pos == std::string_view::npos) ? sv : sv.substr(pos + 1);
+}
Review Comment:
good idea 👍 [small refactor in AgentInformation.cpp](https://github.com/apache/nifi-minifi-cpp/pull/2176/commits/c20b3341734e19cd6bfe338a2221ec0334d971e3)
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2816 Add controller_service::api_implementations to C api [nifi-minifi-cpp]
Posted by "lordgamez (via GitHub)" <gi...@apache.org>.
lordgamez commented on code in PR #2176:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2176#discussion_r3428469802
##########
libminifi/src/core/state/nodes/AgentInformation.cpp:
##########
@@ -17,46 +17,94 @@
*/
#include "core/state/nodes/AgentInformation.h"
-#include "minifi-cpp/agent/agent_version.h"
-#include "core/Resource.h"
#include "core/ClassLoader.h"
-#include "utils/OsUtils.h"
+#include "core/Resource.h"
#include "core/state/nodes/SchedulingNodes.h"
#include "core/state/nodes/SupportedOperations.h"
+#include "minifi-c/minifi-c.h"
+#include "minifi-cpp/agent/agent_version.h"
+#include "minifi-cpp/controllers/AttributeProviderService.h"
+#include "minifi-cpp/controllers/ProxyConfigurationServiceInterface.h"
+#include "minifi-cpp/controllers/RecordSetReader.h"
+#include "minifi-cpp/controllers/RecordSetWriter.h"
+#include "minifi-cpp/controllers/SSLContextServiceInterface.h"
+#include "utils/OsUtils.h"
+#include "range/v3/algorithm/contains.hpp"
namespace org::apache::nifi::minifi::state::response {
utils::ProcessCpuUsageTracker AgentStatus::cpu_load_tracker_;
std::mutex AgentStatus::cpu_load_tracker_mutex_;
namespace {
+
+std::string allowedTypeGroupName(const std::string_view allowed_type) {
+ constexpr std::string_view APACHE_GROUP_STR = "org.apache.nifi.minifi";
+ if (allowed_type.starts_with(APACHE_GROUP_STR)) {
+ return std::string{APACHE_GROUP_STR};
+ }
+ return std::string{allowed_type.substr(0, allowed_type.find_last_of('.'))};
+}
+
+constexpr std::string_view shortNameFromFullName(std::string_view sv) {
+ auto pos = sv.rfind('.');
+ return (pos == std::string_view::npos) ? sv : sv.substr(pos + 1);
+}
+
+bool minifiSystemAllowedType(const std::string_view allowed_type) {
+ constexpr auto system_allowed_types = std::to_array<std::string_view>({
+ controllers::SSLContextServiceInterface::ProvidesApi.type,
+ core::RecordSetWriter::ProvidesApi.type,
+ core::RecordSetReader::ProvidesApi.type,
+ controllers::ProxyConfigurationServiceInterface::ProvidesApi.type,
+ controllers::AttributeProviderService::ProvidesApi.type});
+ return ranges::contains(system_allowed_types, allowed_type);
Review Comment:
It's not really what I suggested, but we discussed that the originally suggested improvement should be a separate PR. Until that is implemented the same issue persists that when any new system allowed type is introduced it needs to be manually inserted here which is not trivial to find and easily missed, does not really matter if it's inclined or extracted to a container.
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2816 Add controller_service::api_implementations to C api [nifi-minifi-cpp]
Posted by "szaszm (via GitHub)" <gi...@apache.org>.
szaszm commented on code in PR #2176:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2176#discussion_r3428372440
##########
libminifi/src/core/state/nodes/AgentInformation.cpp:
##########
@@ -17,46 +17,94 @@
*/
#include "core/state/nodes/AgentInformation.h"
-#include "minifi-cpp/agent/agent_version.h"
-#include "core/Resource.h"
#include "core/ClassLoader.h"
-#include "utils/OsUtils.h"
+#include "core/Resource.h"
#include "core/state/nodes/SchedulingNodes.h"
#include "core/state/nodes/SupportedOperations.h"
+#include "minifi-c/minifi-c.h"
+#include "minifi-cpp/agent/agent_version.h"
+#include "minifi-cpp/controllers/AttributeProviderService.h"
+#include "minifi-cpp/controllers/ProxyConfigurationServiceInterface.h"
+#include "minifi-cpp/controllers/RecordSetReader.h"
+#include "minifi-cpp/controllers/RecordSetWriter.h"
+#include "minifi-cpp/controllers/SSLContextServiceInterface.h"
+#include "utils/OsUtils.h"
+#include "range/v3/algorithm/contains.hpp"
namespace org::apache::nifi::minifi::state::response {
utils::ProcessCpuUsageTracker AgentStatus::cpu_load_tracker_;
std::mutex AgentStatus::cpu_load_tracker_mutex_;
namespace {
+
+std::string allowedTypeGroupName(const std::string_view allowed_type) {
+ constexpr std::string_view APACHE_GROUP_STR = "org.apache.nifi.minifi";
+ if (allowed_type.starts_with(APACHE_GROUP_STR)) {
+ return std::string{APACHE_GROUP_STR};
+ }
+ return std::string{allowed_type.substr(0, allowed_type.find_last_of('.'))};
+}
+
+constexpr std::string_view shortNameFromFullName(std::string_view sv) {
+ auto pos = sv.rfind('.');
+ return (pos == std::string_view::npos) ? sv : sv.substr(pos + 1);
+}
+
+bool minifiSystemAllowedType(const std::string_view allowed_type) {
+ constexpr auto system_allowed_types = std::to_array<std::string_view>({
+ controllers::SSLContextServiceInterface::ProvidesApi.type,
+ core::RecordSetWriter::ProvidesApi.type,
+ core::RecordSetReader::ProvidesApi.type,
+ controllers::ProxyConfigurationServiceInterface::ProvidesApi.type,
+ controllers::AttributeProviderService::ProvidesApi.type});
+ return ranges::contains(system_allowed_types, allowed_type);
Review Comment:
I like the old version with inline comparisons better. No need to introduce an STL container and ranges when it doesn't have a benefit, and line patterns are easily readable, even if they are somewhat repetitive.
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2816 Add controller_service::api_implementations to C api [nifi-minifi-cpp]
Posted by "martinzink (via GitHub)" <gi...@apache.org>.
martinzink commented on code in PR #2176:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2176#discussion_r3322328028
##########
core-framework/include/core/controller/ControllerServiceFactoryImpl.h:
##########
@@ -34,13 +34,13 @@ class ControllerServiceFactoryImpl : public ControllerServiceFactory {
: class_name_(core::className<T>()) {
}
- explicit ControllerServiceFactoryImpl(std::string group_name)
- : group_name_(std::move(group_name)),
+ explicit ControllerServiceFactoryImpl(std::string module_name)
Review Comment:
Ive renamed it from group_name to module_name, because this groupname was not the groupname in C2 but rather the artifact name
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2816 Add controller_service::api_implementations to C api [nifi-minifi-cpp]
Posted by "lordgamez (via GitHub)" <gi...@apache.org>.
lordgamez commented on code in PR #2176:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2176#discussion_r3428469802
##########
libminifi/src/core/state/nodes/AgentInformation.cpp:
##########
@@ -17,46 +17,94 @@
*/
#include "core/state/nodes/AgentInformation.h"
-#include "minifi-cpp/agent/agent_version.h"
-#include "core/Resource.h"
#include "core/ClassLoader.h"
-#include "utils/OsUtils.h"
+#include "core/Resource.h"
#include "core/state/nodes/SchedulingNodes.h"
#include "core/state/nodes/SupportedOperations.h"
+#include "minifi-c/minifi-c.h"
+#include "minifi-cpp/agent/agent_version.h"
+#include "minifi-cpp/controllers/AttributeProviderService.h"
+#include "minifi-cpp/controllers/ProxyConfigurationServiceInterface.h"
+#include "minifi-cpp/controllers/RecordSetReader.h"
+#include "minifi-cpp/controllers/RecordSetWriter.h"
+#include "minifi-cpp/controllers/SSLContextServiceInterface.h"
+#include "utils/OsUtils.h"
+#include "range/v3/algorithm/contains.hpp"
namespace org::apache::nifi::minifi::state::response {
utils::ProcessCpuUsageTracker AgentStatus::cpu_load_tracker_;
std::mutex AgentStatus::cpu_load_tracker_mutex_;
namespace {
+
+std::string allowedTypeGroupName(const std::string_view allowed_type) {
+ constexpr std::string_view APACHE_GROUP_STR = "org.apache.nifi.minifi";
+ if (allowed_type.starts_with(APACHE_GROUP_STR)) {
+ return std::string{APACHE_GROUP_STR};
+ }
+ return std::string{allowed_type.substr(0, allowed_type.find_last_of('.'))};
+}
+
+constexpr std::string_view shortNameFromFullName(std::string_view sv) {
+ auto pos = sv.rfind('.');
+ return (pos == std::string_view::npos) ? sv : sv.substr(pos + 1);
+}
+
+bool minifiSystemAllowedType(const std::string_view allowed_type) {
+ constexpr auto system_allowed_types = std::to_array<std::string_view>({
+ controllers::SSLContextServiceInterface::ProvidesApi.type,
+ core::RecordSetWriter::ProvidesApi.type,
+ core::RecordSetReader::ProvidesApi.type,
+ controllers::ProxyConfigurationServiceInterface::ProvidesApi.type,
+ controllers::AttributeProviderService::ProvidesApi.type});
+ return ranges::contains(system_allowed_types, allowed_type);
Review Comment:
It's not really what I suggested, but we discussed that the originally suggested improvement should be a separate PR. Until that is implemented the same issue persists that when any new system allowed type is introduced it needs to be manually inserted here which is not trivial to find and easily missed, does not really matter if it's inlined or extracted to a container. My original intent to alternatively extract the constants was to move them to a more accessible and discoverable place when introducing a new option, but there's not really a viable place for it now, so it can be reverted.
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2816 Add controller_service::api_implementations to C api [nifi-minifi-cpp]
Posted by "szaszm (via GitHub)" <gi...@apache.org>.
szaszm commented on code in PR #2176:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2176#discussion_r3428581645
##########
libminifi/src/core/state/nodes/AgentInformation.cpp:
##########
@@ -17,46 +17,94 @@
*/
#include "core/state/nodes/AgentInformation.h"
-#include "minifi-cpp/agent/agent_version.h"
-#include "core/Resource.h"
#include "core/ClassLoader.h"
-#include "utils/OsUtils.h"
+#include "core/Resource.h"
#include "core/state/nodes/SchedulingNodes.h"
#include "core/state/nodes/SupportedOperations.h"
+#include "minifi-c/minifi-c.h"
+#include "minifi-cpp/agent/agent_version.h"
+#include "minifi-cpp/controllers/AttributeProviderService.h"
+#include "minifi-cpp/controllers/ProxyConfigurationServiceInterface.h"
+#include "minifi-cpp/controllers/RecordSetReader.h"
+#include "minifi-cpp/controllers/RecordSetWriter.h"
+#include "minifi-cpp/controllers/SSLContextServiceInterface.h"
+#include "utils/OsUtils.h"
+#include "range/v3/algorithm/contains.hpp"
namespace org::apache::nifi::minifi::state::response {
utils::ProcessCpuUsageTracker AgentStatus::cpu_load_tracker_;
std::mutex AgentStatus::cpu_load_tracker_mutex_;
namespace {
+
+std::string allowedTypeGroupName(const std::string_view allowed_type) {
+ constexpr std::string_view APACHE_GROUP_STR = "org.apache.nifi.minifi";
+ if (allowed_type.starts_with(APACHE_GROUP_STR)) {
+ return std::string{APACHE_GROUP_STR};
+ }
+ return std::string{allowed_type.substr(0, allowed_type.find_last_of('.'))};
+}
+
+constexpr std::string_view shortNameFromFullName(std::string_view sv) {
+ auto pos = sv.rfind('.');
+ return (pos == std::string_view::npos) ? sv : sv.substr(pos + 1);
+}
+
+bool minifiSystemAllowedType(const std::string_view allowed_type) {
+ constexpr auto system_allowed_types = std::to_array<std::string_view>({
+ controllers::SSLContextServiceInterface::ProvidesApi.type,
+ core::RecordSetWriter::ProvidesApi.type,
+ core::RecordSetReader::ProvidesApi.type,
+ controllers::ProxyConfigurationServiceInterface::ProvidesApi.type,
+ controllers::AttributeProviderService::ProvidesApi.type});
+ return ranges::contains(system_allowed_types, allowed_type);
Review Comment:
ah okay. If it's a temporary solution, I don't insist in changing it, but I would still prefer the simpler version.
##########
libminifi/src/core/state/nodes/AgentInformation.cpp:
##########
@@ -17,46 +17,94 @@
*/
#include "core/state/nodes/AgentInformation.h"
-#include "minifi-cpp/agent/agent_version.h"
-#include "core/Resource.h"
#include "core/ClassLoader.h"
-#include "utils/OsUtils.h"
+#include "core/Resource.h"
#include "core/state/nodes/SchedulingNodes.h"
#include "core/state/nodes/SupportedOperations.h"
+#include "minifi-c/minifi-c.h"
+#include "minifi-cpp/agent/agent_version.h"
+#include "minifi-cpp/controllers/AttributeProviderService.h"
+#include "minifi-cpp/controllers/ProxyConfigurationServiceInterface.h"
+#include "minifi-cpp/controllers/RecordSetReader.h"
+#include "minifi-cpp/controllers/RecordSetWriter.h"
+#include "minifi-cpp/controllers/SSLContextServiceInterface.h"
+#include "utils/OsUtils.h"
+#include "range/v3/algorithm/contains.hpp"
namespace org::apache::nifi::minifi::state::response {
utils::ProcessCpuUsageTracker AgentStatus::cpu_load_tracker_;
std::mutex AgentStatus::cpu_load_tracker_mutex_;
namespace {
+
+std::string allowedTypeGroupName(const std::string_view allowed_type) {
+ constexpr std::string_view APACHE_GROUP_STR = "org.apache.nifi.minifi";
+ if (allowed_type.starts_with(APACHE_GROUP_STR)) {
+ return std::string{APACHE_GROUP_STR};
+ }
+ return std::string{allowed_type.substr(0, allowed_type.find_last_of('.'))};
+}
+
+constexpr std::string_view shortNameFromFullName(std::string_view sv) {
+ auto pos = sv.rfind('.');
+ return (pos == std::string_view::npos) ? sv : sv.substr(pos + 1);
+}
+
+bool minifiSystemAllowedType(const std::string_view allowed_type) {
+ constexpr auto system_allowed_types = std::to_array<std::string_view>({
+ controllers::SSLContextServiceInterface::ProvidesApi.type,
+ core::RecordSetWriter::ProvidesApi.type,
+ core::RecordSetReader::ProvidesApi.type,
+ controllers::ProxyConfigurationServiceInterface::ProvidesApi.type,
+ controllers::AttributeProviderService::ProvidesApi.type});
+ return ranges::contains(system_allowed_types, allowed_type);
Review Comment:
ah okay. If it's a temporary solution, I don't insist on changing it, but I would still prefer the simpler version.
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2816 Add controller_service::api_implementations to C api [nifi-minifi-cpp]
Posted by "martinzink (via GitHub)" <gi...@apache.org>.
martinzink commented on code in PR #2176:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2176#discussion_r3458284701
##########
libminifi/src/core/state/nodes/AgentInformation.cpp:
##########
@@ -17,46 +17,93 @@
*/
#include "core/state/nodes/AgentInformation.h"
-#include "minifi-cpp/agent/agent_version.h"
-#include "core/Resource.h"
#include "core/ClassLoader.h"
-#include "utils/OsUtils.h"
+#include "core/Resource.h"
#include "core/state/nodes/SchedulingNodes.h"
#include "core/state/nodes/SupportedOperations.h"
+#include "minifi-c/minifi-c.h"
+#include "minifi-cpp/agent/agent_version.h"
+#include "minifi-cpp/controllers/AttributeProviderService.h"
+#include "minifi-cpp/controllers/ProxyConfigurationServiceInterface.h"
+#include "minifi-cpp/controllers/RecordSetReader.h"
+#include "minifi-cpp/controllers/RecordSetWriter.h"
+#include "minifi-cpp/controllers/SSLContextServiceInterface.h"
+#include "utils/OsUtils.h"
+#include "range/v3/algorithm/contains.hpp"
namespace org::apache::nifi::minifi::state::response {
utils::ProcessCpuUsageTracker AgentStatus::cpu_load_tracker_;
std::mutex AgentStatus::cpu_load_tracker_mutex_;
namespace {
+
+std::string allowedTypeGroupName(const std::string_view allowed_type) {
+ constexpr std::string_view APACHE_GROUP_STR = "org.apache.nifi.minifi";
+ if (allowed_type.starts_with(APACHE_GROUP_STR)) {
+ return std::string{APACHE_GROUP_STR};
+ }
+ return std::string{allowed_type.substr(0, allowed_type.find_last_of('.'))};
+}
+
+constexpr std::string_view shortNameFromFullName(std::string_view sv) {
+ auto pos = sv.rfind('.');
+ return (pos == std::string_view::npos) ? sv : sv.substr(pos + 1);
+}
Review Comment:
it turns out its not equivalent, the shortname gets from foo.bar.baz **baz** and we need **foo.bar** here so [Revert "small refactor in AgentInformation.cpp"](https://github.com/apache/nifi-minifi-cpp/pull/2176/commits/c3771a8a796e98b8d7f3eaa9e7c161b96f5ce958)
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2816 Add controller_service::api_implementations to C api [nifi-minifi-cpp]
Posted by "martinzink (via GitHub)" <gi...@apache.org>.
martinzink commented on code in PR #2176:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2176#discussion_r3451015154
##########
extensions/stable-api-sandbox/ZooProcessor.h:
##########
@@ -0,0 +1,63 @@
+/**
+ * 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 "AnimalControllerServiceApis.h"
+#include "api/core/ProcessorImpl.h"
+#include "api/utils/Export.h"
+#include "core/PropertyDefinitionBuilder.h"
+#include "minifi-cpp/core/Annotation.h"
+
+namespace org::apache::nifi::minifi::api_sandbox {
+
+class ZooProcessor : public api::core::ProcessorImpl {
+ public:
+ EXTENSIONAPI static constexpr const char* Description = "Test ZooProcessor";
+
+ EXTENSIONAPI static constexpr auto CanFlyService = core::PropertyDefinitionBuilder<>::createProperty("Can fly service")
+ .withDescription("Test CanFlyService")
+ .isRequired(true)
+ .withAllowedTypes<CanFlyControllerApi>()
+ .build();
Review Comment:
ive tweaked the clang format and reformatted the file
https://github.com/apache/nifi-minifi-cpp/pull/2176/changes/4fac593e0c7ed8871caab93bb93309974b3a4af1
##########
extensions/stable-api-sandbox/ZooProcessor.h:
##########
@@ -0,0 +1,63 @@
+/**
+ * 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 "AnimalControllerServiceApis.h"
+#include "api/core/ProcessorImpl.h"
+#include "api/utils/Export.h"
+#include "core/PropertyDefinitionBuilder.h"
+#include "minifi-cpp/core/Annotation.h"
+
+namespace org::apache::nifi::minifi::api_sandbox {
+
+class ZooProcessor : public api::core::ProcessorImpl {
+ public:
+ EXTENSIONAPI static constexpr const char* Description = "Test ZooProcessor";
+
+ EXTENSIONAPI static constexpr auto CanFlyService = core::PropertyDefinitionBuilder<>::createProperty("Can fly service")
+ .withDescription("Test CanFlyService")
+ .isRequired(true)
+ .withAllowedTypes<CanFlyControllerApi>()
+ .build();
+
+ EXTENSIONAPI static constexpr auto NumberOfLegsService = core::PropertyDefinitionBuilder<>::createProperty("Number of legs service")
+ .withDescription("Test NumberOfLegsService")
+ .isRequired(true)
+ .withAllowedTypes<NumberOfLegsControllerApi>()
+ .build();
+
Review Comment:
https://github.com/apache/nifi-minifi-cpp/pull/2176/changes/4fac593e0c7ed8871caab93bb93309974b3a4af1
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2816 Add controller_service::api_implementations to C api [nifi-minifi-cpp]
Posted by "martinzink (via GitHub)" <gi...@apache.org>.
martinzink commented on code in PR #2176:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2176#discussion_r3453590080
##########
extensions/stable-api-testing/AnimalControllerServices.h:
##########
@@ -0,0 +1,67 @@
+/**
+ * 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 "AnimalControllerServiceApis.h"
+#include "api/core/ControllerServiceImpl.h"
+#include "api/utils/Export.h"
+#include "core/PropertyDefinitionBuilder.h"
+#include "minifi-cpp/core/ProvidedControllerServiceInterface.h"
+
+namespace org::apache::nifi::minifi::api_testing {
+class DogController : public api::core::ControllerServiceImpl, public CanFlyControllerApi, public NumberOfLegsControllerApi {
+ public:
+ EXTENSIONAPI static constexpr const char* Description = "Test DogController";
+ EXTENSIONAPI static constexpr auto HasJetpack = core::PropertyDefinitionBuilder<>::createProperty("Has Jetpack")
+ .withDescription("Whether or not the dog has a jetpack")
+ .withDefaultValue("false")
+ .withValidator(core::StandardPropertyValidators::BOOLEAN_VALIDATOR)
+ .build();
Review Comment:
[reformatted entire stable-api-testing dir](https://github.com/apache/nifi-minifi-cpp/pull/2176/commits/215e3f0c938f4e8ff9a9ac7fef53daee7511fffe)
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2816 Add controller_service::api_implementations to C api [nifi-minifi-cpp]
Posted by "martinzink (via GitHub)" <gi...@apache.org>.
martinzink commented on code in PR #2176:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2176#discussion_r3436652010
##########
extensions/stable-api-sandbox/tests/ZooTests.cpp:
##########
@@ -0,0 +1,71 @@
+/**
+ *
+ * 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 "../AnimalControllerServices.h"
+#include "CProcessorTestUtils.h"
+#include "ZooProcessor.h"
+#include "api/core/Resource.h"
+#include "minifi-cpp/core/FlowFile.h"
+#include "unit/Catch.h"
+#include "unit/SingleProcessorTestController.h"
+#include "unit/TestBase.h"
+#include "unit/TestUtils.h"
+#include "utils/CProcessor.h"
+
+namespace org::apache::nifi::minifi::api_sandbox::test {
+TEST_CASE("ZooTest") {
+ minifi::test::SingleProcessorTestController controller(minifi::test::utils::make_custom_c_processor<ZooProcessor>(
+ core::ProcessorMetadata{utils::Identifier{}, "ZooProcessor", logging::LoggerFactory<ZooProcessor>::getLogger()}));
+ const auto dog_with_jetpack = minifi::test::utils::make_custom_c_controller_service<DogController>(core::ControllerServiceMetadata{
+ utils::Identifier{},
+ "DogController",
+ logging::LoggerFactory<DogController>::getLogger()});
+ auto dog_with_jetpack_node = controller.plan->addController("dog_with_jetpack", dog_with_jetpack);
+ CHECK(dog_with_jetpack->setProperty(DogController::HasJetpack.name, "true"));
+
+ const auto duck = minifi::test::utils::make_custom_c_controller_service<DuckController>(core::ControllerServiceMetadata{utils::Identifier{},
+ "DuckController",
+ logging::LoggerFactory<DuckController>::getLogger()});
+ auto duck_node = controller.plan->addController("duck", duck);
+
+ {
+ CHECK(controller.getProcessor()->setProperty(ZooProcessor::CanFlyService.name, "dog_with_jetpack"));
+ CHECK(controller.getProcessor()->setProperty(ZooProcessor::NumberOfLegsService.name, "duck"));
+ controller.trigger();
+ CHECK(LogTestController::getInstance()
+ .contains("[org::apache::nifi::minifi::api_sandbox::ZooProcessor] [critical] Can dog_with_jetpack fly? true"));
+ CHECK(LogTestController::getInstance().contains("[org::apache::nifi::minifi::api_sandbox::ZooProcessor] [critical] duck has 2 legs"));
+ }
+ {
+ LogTestController::getInstance().clear();
+ CHECK(controller.getProcessor()->setProperty(ZooProcessor::CanFlyService.name, "duck"));
+ CHECK(controller.getProcessor()->setProperty(ZooProcessor::NumberOfLegsService.name, "dog_with_jetpack"));
+ controller.trigger();
+ CHECK(LogTestController::getInstance().contains("[org::apache::nifi::minifi::api_sandbox::ZooProcessor] [critical] Can duck fly? true"));
+ CHECK(LogTestController::getInstance().contains("[org::apache::nifi::minifi::api_sandbox::ZooProcessor] [critical] dog_with_jetpack has 4 legs"));
+ }
+ {
+ LogTestController::getInstance().clear();
+ CHECK(controller.getProcessor()->setProperty(ZooProcessor::CanFlyService.name, "duck"));
+ CHECK(controller.getProcessor()->setProperty(ZooProcessor::NumberOfLegsService.name, "invalid"));
Review Comment:
no we dont validate here, thats only done on C2 side, we theoratically could so its might be a good feature
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2816 Add controller_service::api_implementations to C api [nifi-minifi-cpp]
Posted by "martinzink (via GitHub)" <gi...@apache.org>.
martinzink merged PR #2176:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2176
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2816 Add controller_service::api_implementations to C api [nifi-minifi-cpp]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #2176:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2176#discussion_r3247745572
##########
extensions/stable-api-sandbox/ExtensionInitializer.cpp:
##########
@@ -0,0 +1,39 @@
+/**
+ * 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 "../../extension-framework/cpp-extension-lib/include/api/core/Resource.h"
Review Comment:
This duplicate include of `Resource.h` via a relative path that escapes the source directory looks unintentional. The file already includes `api/core/Resource.h` on line 21 through the proper include path. The `../../extension-framework/...` path is fragile (depends on the file's exact location in the source tree, won't work from an out-of-tree extension that just links against the headers) and bypasses the configured include directories. Please remove the line 18 include.
##########
minifi-api/include/minifi-c/minifi-c.h:
##########
@@ -202,11 +203,18 @@ typedef struct MinifiProcessorClassDefinition {
MinifiProcessorCallbacks callbacks;
} MinifiProcessorClassDefinition;
+struct MinifiControllerServiceProvidedAPI {
+ MinifiStringView type;
+ void*(*cast_api)(void*);
+};
+
typedef struct MinifiControllerServiceClassDefinition {
MinifiStringView full_name; // '::'-delimited fully qualified name e.g. 'org::apache::nifi::minifi::extensions::gcp::GCPCredentialsControllerService
MinifiStringView description;
size_t class_properties_count;
const MinifiPropertyDefinition* class_properties_ptr;
+ size_t provided_interfaces_count;
+ const MinifiStringView* provided_interfaces_ptr;
Review Comment:
This change adds two new fields to `MinifiControllerServiceClassDefinition` (`provided_interfaces_count`, `provided_interfaces_ptr`) and a new function pointer `get_interface` to `MinifiControllerServiceCallbacks`, both of which alter the size and layout of structs that C extensions populate. However, `MINIFI_API_VERSION` (minifi-c.h:49) has not been bumped. Per the existing convention, C extensions must export `MinifiApiVersion == MINIFI_API_VERSION`, and the agent rejects mismatches; without bumping the version, an old extension compiled against API version 3 will pass the version check but provide a struct of the old size, leaving `provided_interfaces_*` and `get_interface` as garbage that the agent will then dereference (e.g. the unconditional call at minifi-c.cpp:611). Please bump `MINIFI_API_VERSION`.
##########
libminifi/src/minifi-c.cpp:
##########
@@ -600,6 +608,12 @@ MinifiStatus MinifiProcessContextGetControllerService(
*controller_service_out = static_cast<MinifiControllerService*>(c_controller_service->getImpl());
return MINIFI_STATUS_SUCCESS;
}
+ auto interface_res = class_description.callbacks.get_interface(c_controller_service->getImpl(), controller_service_type);
+ if (!interface_res) {
+ return MINIFI_STATUS_VALIDATION_FAILED;
+ }
+ *controller_service_out = static_cast<MinifiControllerService*>(interface_res);
+ return MINIFI_STATUS_SUCCESS;
Review Comment:
`get_interface` is invoked unconditionally without first checking that the function pointer is non-null. A C extension that does not declare any provided interfaces has no reason to populate this callback, and `MinifiControllerServiceCallbacks` is documented (by its other use) as being aggregate-initialized field-by-field, so callers will commonly leave new optional fields at their default-initialized value. If `get_interface` is null this will crash. Please null-check the pointer (and return `MINIFI_STATUS_VALIDATION_FAILED` when absent) before calling it.
##########
extensions/stable-api-sandbox/AnimalControllerServiceApis.h:
##########
@@ -0,0 +1,33 @@
+/**
+ * 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 <cinttypes>
+
+namespace org::apache::nifi::minifi::api_sandbox {
+class NumberOfLegsControllerApi {
+ public:
+ virtual uint8_t numberOfLegs() const = 0;
+};
+
+class CanFlyControllerApi {
+ public:
Review Comment:
`NumberOfLegsControllerApi` and `CanFlyControllerApi` are abstract interfaces meant to be inherited by concrete controller services and accessed polymorphically (the `cast` in `createProvidedInterface` returns an `Interface*`, and callers in `ZooProcessor.cpp` dereference it via that pointer). Without a virtual destructor in the interface, deleting through the base pointer would be undefined behavior. Even though current usage does not delete via the interface, please declare a `virtual ~NumberOfLegsControllerApi() = default;` (and likewise for `CanFlyControllerApi`) to make the contract safe and to match the polymorphic intent.
##########
minifi-api/common/include/minifi-cpp/core/ProvidedControllerServiceInterface.h:
##########
@@ -0,0 +1,33 @@
+/**
+ * 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 <string_view>
+
+namespace org::apache::nifi::minifi::core {
+struct ProvidedInterface {
+ std::string_view name;
+ void* (*cast)(void* instance);
+};
+
+template<typename Interface, typename ConcreteService>
+constexpr ProvidedInterface createProvidedInterface() {
+ return ProvidedInterface{className<Interface>(), [](void* instance) -> void* {
Review Comment:
This header uses `core::className<Interface>()` but only includes `<string_view>`. It currently compiles only because translation units that include this header (e.g. `AnimalControllerServices.h`) happen to also pull in `core/ClassName.h` transitively via other headers. To make this header self-contained and robust, please add `#include "core/ClassName.h"`.
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2816 Add controller_service::api_implementations to C api [nifi-minifi-cpp]
Posted by "adamdebreceni (via GitHub)" <gi...@apache.org>.
adamdebreceni commented on code in PR #2176:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2176#discussion_r3341667871
##########
libminifi/src/minifi-c.cpp:
##########
@@ -164,7 +164,7 @@ class CControllerServiceFactory : public minifi::core::controller::ControllerSer
return std::make_unique<minifi::utils::CControllerService>(class_description_, std::move(metadata));
}
- [[nodiscard]] std::string getGroupName() const override { return group_name_; }
+ [[nodiscard]] std::string getModuleName() const override { return group_name_; }
Review Comment:
we could also update the group_name_ as well here
##########
core-framework/include/core/ObjectFactory.h:
##########
@@ -31,7 +31,7 @@ class ObjectFactoryImpl : public ObjectFactory {
ObjectFactoryImpl() = default;
- std::string getGroupName() const override {
+ std::string getModuleName() const override {
return group_;
Review Comment:
we could also update the member
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2816 Add controller_service::api_implementations to C api [nifi-minifi-cpp]
Posted by "szaszm (via GitHub)" <gi...@apache.org>.
szaszm commented on code in PR #2176:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2176#discussion_r3453490720
##########
extensions/stable-api-testing/AnimalControllerServices.h:
##########
@@ -0,0 +1,67 @@
+/**
+ * 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 "AnimalControllerServiceApis.h"
+#include "api/core/ControllerServiceImpl.h"
+#include "api/utils/Export.h"
+#include "core/PropertyDefinitionBuilder.h"
+#include "minifi-cpp/core/ProvidedControllerServiceInterface.h"
+
+namespace org::apache::nifi::minifi::api_testing {
+class DogController : public api::core::ControllerServiceImpl, public CanFlyControllerApi, public NumberOfLegsControllerApi {
+ public:
+ EXTENSIONAPI static constexpr const char* Description = "Test DogController";
+ EXTENSIONAPI static constexpr auto HasJetpack = core::PropertyDefinitionBuilder<>::createProperty("Has Jetpack")
+ .withDescription("Whether or not the dog has a jetpack")
+ .withDefaultValue("false")
+ .withValidator(core::StandardPropertyValidators::BOOLEAN_VALIDATOR)
+ .build();
Review Comment:
```suggestion
EXTENSIONAPI static constexpr auto HasJetpack = core::PropertyDefinitionBuilder<>::createProperty("Has Jetpack")
.withDescription("Whether or not the dog has a jetpack")
.withDefaultValue("false")
.withValidator(core::StandardPropertyValidators::BOOLEAN_VALIDATOR)
.build();
```
##########
extensions/stable-api-testing/AnimalControllerServices.h:
##########
@@ -0,0 +1,67 @@
+/**
+ * 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 "AnimalControllerServiceApis.h"
+#include "api/core/ControllerServiceImpl.h"
+#include "api/utils/Export.h"
+#include "core/PropertyDefinitionBuilder.h"
+#include "minifi-cpp/core/ProvidedControllerServiceInterface.h"
+
+namespace org::apache::nifi::minifi::api_testing {
+class DogController : public api::core::ControllerServiceImpl, public CanFlyControllerApi, public NumberOfLegsControllerApi {
Review Comment:
```suggestion
/// DogController and DuckController are testing that one ControllerService type can implement multiple interfaces
class DogController : public api::core::ControllerServiceImpl, public CanFlyControllerApi, public NumberOfLegsControllerApi {
```
##########
libminifi/src/core/state/nodes/AgentInformation.cpp:
##########
@@ -17,46 +17,93 @@
*/
#include "core/state/nodes/AgentInformation.h"
-#include "minifi-cpp/agent/agent_version.h"
-#include "core/Resource.h"
#include "core/ClassLoader.h"
-#include "utils/OsUtils.h"
+#include "core/Resource.h"
#include "core/state/nodes/SchedulingNodes.h"
#include "core/state/nodes/SupportedOperations.h"
+#include "minifi-c/minifi-c.h"
+#include "minifi-cpp/agent/agent_version.h"
+#include "minifi-cpp/controllers/AttributeProviderService.h"
+#include "minifi-cpp/controllers/ProxyConfigurationServiceInterface.h"
+#include "minifi-cpp/controllers/RecordSetReader.h"
+#include "minifi-cpp/controllers/RecordSetWriter.h"
+#include "minifi-cpp/controllers/SSLContextServiceInterface.h"
+#include "utils/OsUtils.h"
+#include "range/v3/algorithm/contains.hpp"
namespace org::apache::nifi::minifi::state::response {
utils::ProcessCpuUsageTracker AgentStatus::cpu_load_tracker_;
std::mutex AgentStatus::cpu_load_tracker_mutex_;
namespace {
+
+std::string allowedTypeGroupName(const std::string_view allowed_type) {
+ constexpr std::string_view APACHE_GROUP_STR = "org.apache.nifi.minifi";
+ if (allowed_type.starts_with(APACHE_GROUP_STR)) {
+ return std::string{APACHE_GROUP_STR};
+ }
+ return std::string{allowed_type.substr(0, allowed_type.find_last_of('.'))};
+}
+
+constexpr std::string_view shortNameFromFullName(std::string_view sv) {
+ auto pos = sv.rfind('.');
+ return (pos == std::string_view::npos) ? sv : sv.substr(pos + 1);
+}
Review Comment:
you could switch these and call shortNameFromFullName from allowedTypeGroupName. Unless you would rather keep them conceptually separate, regardless of the type compatibility.
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2816 Add controller_service::api_implementations to C api [nifi-minifi-cpp]
Posted by "martinzink (via GitHub)" <gi...@apache.org>.
martinzink commented on code in PR #2176:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2176#discussion_r3458277061
##########
libminifi/src/core/state/nodes/AgentInformation.cpp:
##########
@@ -17,46 +17,94 @@
*/
#include "core/state/nodes/AgentInformation.h"
-#include "minifi-cpp/agent/agent_version.h"
-#include "core/Resource.h"
#include "core/ClassLoader.h"
-#include "utils/OsUtils.h"
+#include "core/Resource.h"
#include "core/state/nodes/SchedulingNodes.h"
#include "core/state/nodes/SupportedOperations.h"
+#include "minifi-c/minifi-c.h"
+#include "minifi-cpp/agent/agent_version.h"
+#include "minifi-cpp/controllers/AttributeProviderService.h"
+#include "minifi-cpp/controllers/ProxyConfigurationServiceInterface.h"
+#include "minifi-cpp/controllers/RecordSetReader.h"
+#include "minifi-cpp/controllers/RecordSetWriter.h"
+#include "minifi-cpp/controllers/SSLContextServiceInterface.h"
+#include "utils/OsUtils.h"
+#include "range/v3/algorithm/contains.hpp"
namespace org::apache::nifi::minifi::state::response {
utils::ProcessCpuUsageTracker AgentStatus::cpu_load_tracker_;
std::mutex AgentStatus::cpu_load_tracker_mutex_;
namespace {
+
+std::string allowedTypeGroupName(const std::string_view allowed_type) {
+ constexpr std::string_view APACHE_GROUP_STR = "org.apache.nifi.minifi";
+ if (allowed_type.starts_with(APACHE_GROUP_STR)) {
+ return std::string{APACHE_GROUP_STR};
+ }
+ return std::string{allowed_type.substr(0, allowed_type.find_last_of('.'))};
+}
+
+constexpr std::string_view shortNameFromFullName(std::string_view sv) {
+ auto pos = sv.rfind('.');
+ return (pos == std::string_view::npos) ? sv : sv.substr(pos + 1);
+}
+
+bool minifiSystemAllowedType(const std::string_view allowed_type) {
+ constexpr auto system_allowed_types = std::to_array<std::string_view>({
+ controllers::SSLContextServiceInterface::ProvidesApi.type,
+ core::RecordSetWriter::ProvidesApi.type,
+ core::RecordSetReader::ProvidesApi.type,
+ controllers::ProxyConfigurationServiceInterface::ProvidesApi.type,
+ controllers::AttributeProviderService::ProvidesApi.type});
+ return ranges::contains(system_allowed_types, allowed_type);
Review Comment:
yeah since the temp solution wasnt anyones favorite i've reverted that to the simpler version [Revert "refactor minifiSystemAllowedType"](https://github.com/apache/nifi-minifi-cpp/pull/2176/commits/35d49cab7e09b7cdc82bd5d87bcd7f4cd24646e7)
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2816 Add controller_service::api_implementations to C api [nifi-minifi-cpp]
Posted by "szaszm (via GitHub)" <gi...@apache.org>.
szaszm commented on code in PR #2176:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2176#discussion_r3461444247
##########
libminifi/src/core/ClassLoader.cpp:
##########
@@ -275,7 +275,7 @@ gsl::owner<CoreComponent*> ClassLoaderImpl::instantiateRaw(const std::string &cl
if (filter(obj)) {
return obj;
}
- delete gsl::owner<CoreComponent*>{obj};
+ delete obj; // NOLINT(cppcoreguidelines-owning-memory)
Review Comment:
it seems that if filter throws, obj is leaked. Bug not introduced here, and most likely not a problem in practice, since filter functions tend to not throw, I just noticed it.
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2816 Add controller_service::api_implementations to C api [nifi-minifi-cpp]
Posted by "szaszm (via GitHub)" <gi...@apache.org>.
szaszm commented on code in PR #2176:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2176#discussion_r3428385586
##########
libminifi/src/core/state/nodes/AgentInformation.cpp:
##########
@@ -17,46 +17,94 @@
*/
#include "core/state/nodes/AgentInformation.h"
-#include "minifi-cpp/agent/agent_version.h"
-#include "core/Resource.h"
#include "core/ClassLoader.h"
-#include "utils/OsUtils.h"
+#include "core/Resource.h"
#include "core/state/nodes/SchedulingNodes.h"
#include "core/state/nodes/SupportedOperations.h"
+#include "minifi-c/minifi-c.h"
+#include "minifi-cpp/agent/agent_version.h"
+#include "minifi-cpp/controllers/AttributeProviderService.h"
+#include "minifi-cpp/controllers/ProxyConfigurationServiceInterface.h"
+#include "minifi-cpp/controllers/RecordSetReader.h"
+#include "minifi-cpp/controllers/RecordSetWriter.h"
+#include "minifi-cpp/controllers/SSLContextServiceInterface.h"
+#include "utils/OsUtils.h"
+#include "range/v3/algorithm/contains.hpp"
namespace org::apache::nifi::minifi::state::response {
utils::ProcessCpuUsageTracker AgentStatus::cpu_load_tracker_;
std::mutex AgentStatus::cpu_load_tracker_mutex_;
namespace {
+
+std::string allowedTypeGroupName(const std::string_view allowed_type) {
+ constexpr std::string_view APACHE_GROUP_STR = "org.apache.nifi.minifi";
+ if (allowed_type.starts_with(APACHE_GROUP_STR)) {
+ return std::string{APACHE_GROUP_STR};
+ }
+ return std::string{allowed_type.substr(0, allowed_type.find_last_of('.'))};
+}
+
+constexpr std::string_view shortNameFromFullName(std::string_view sv) {
+ auto pos = sv.rfind('.');
+ return (pos == std::string_view::npos) ? sv : sv.substr(pos + 1);
+}
+
+bool minifiSystemAllowedType(const std::string_view allowed_type) {
+ constexpr auto system_allowed_types = std::to_array<std::string_view>({
+ controllers::SSLContextServiceInterface::ProvidesApi.type,
+ core::RecordSetWriter::ProvidesApi.type,
+ core::RecordSetReader::ProvidesApi.type,
+ controllers::ProxyConfigurationServiceInterface::ProvidesApi.type,
+ controllers::AttributeProviderService::ProvidesApi.type});
+ return ranges::contains(system_allowed_types, allowed_type);
Review Comment:
I just noticed that @lordgamez suggested the change, so CC.
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2816 Add controller_service::api_implementations to C api [nifi-minifi-cpp]
Posted by "lordgamez (via GitHub)" <gi...@apache.org>.
lordgamez commented on code in PR #2176:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2176#discussion_r3428469802
##########
libminifi/src/core/state/nodes/AgentInformation.cpp:
##########
@@ -17,46 +17,94 @@
*/
#include "core/state/nodes/AgentInformation.h"
-#include "minifi-cpp/agent/agent_version.h"
-#include "core/Resource.h"
#include "core/ClassLoader.h"
-#include "utils/OsUtils.h"
+#include "core/Resource.h"
#include "core/state/nodes/SchedulingNodes.h"
#include "core/state/nodes/SupportedOperations.h"
+#include "minifi-c/minifi-c.h"
+#include "minifi-cpp/agent/agent_version.h"
+#include "minifi-cpp/controllers/AttributeProviderService.h"
+#include "minifi-cpp/controllers/ProxyConfigurationServiceInterface.h"
+#include "minifi-cpp/controllers/RecordSetReader.h"
+#include "minifi-cpp/controllers/RecordSetWriter.h"
+#include "minifi-cpp/controllers/SSLContextServiceInterface.h"
+#include "utils/OsUtils.h"
+#include "range/v3/algorithm/contains.hpp"
namespace org::apache::nifi::minifi::state::response {
utils::ProcessCpuUsageTracker AgentStatus::cpu_load_tracker_;
std::mutex AgentStatus::cpu_load_tracker_mutex_;
namespace {
+
+std::string allowedTypeGroupName(const std::string_view allowed_type) {
+ constexpr std::string_view APACHE_GROUP_STR = "org.apache.nifi.minifi";
+ if (allowed_type.starts_with(APACHE_GROUP_STR)) {
+ return std::string{APACHE_GROUP_STR};
+ }
+ return std::string{allowed_type.substr(0, allowed_type.find_last_of('.'))};
+}
+
+constexpr std::string_view shortNameFromFullName(std::string_view sv) {
+ auto pos = sv.rfind('.');
+ return (pos == std::string_view::npos) ? sv : sv.substr(pos + 1);
+}
+
+bool minifiSystemAllowedType(const std::string_view allowed_type) {
+ constexpr auto system_allowed_types = std::to_array<std::string_view>({
+ controllers::SSLContextServiceInterface::ProvidesApi.type,
+ core::RecordSetWriter::ProvidesApi.type,
+ core::RecordSetReader::ProvidesApi.type,
+ controllers::ProxyConfigurationServiceInterface::ProvidesApi.type,
+ controllers::AttributeProviderService::ProvidesApi.type});
+ return ranges::contains(system_allowed_types, allowed_type);
Review Comment:
It's not really what I suggested, but we discussed that the originally suggested improvement should be a separate PR. Until that is implemented the same issue persists that when any new system allowed type is introduced it needs to be manually inserted here which is not trivial to find and easily missed, does not really matter if it's inlined or extracted to a container.
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org