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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
