-
Notifications
You must be signed in to change notification settings - Fork 99
MINIFICPP-2669 - Reduce controller service api #2065
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
36ff46b to
b61e730
Compare
fd945e9 to
6b53a92
Compare
| .artifact = "minifi-system", | ||
| .group = "org.apache.nifi.minifi", | ||
| .type = "org.apache.nifi.minifi.core.RecordSetReader", | ||
| .version = "1.0.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the version of components shipped with minifi should always match the minifi version, what do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changed it to use the minifi version
| * Controller Service base class that contains some pure virtual methods. | ||
| * | ||
| * Design: OnEnable is executed when the controller service is being enabled. | ||
| * Note that keeping state here must be protected in this function. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand this sentence, could you elaborate or rephrase?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
outdated, updated it
| ControllerServiceDescriptor* descriptor_{nullptr}; | ||
| ControllerServiceContext* context_{nullptr}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A comment describing the lifetimes (what guarantees that these will remain alive throughout the ControllerService lifetime) would be nice here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added comment
| } | ||
| if (thread_manager_->isAboveMax(current_workers_)) { | ||
| auto max = thread_manager_->getMaxConcurrentTasks(); | ||
| auto max = 1; // thread_manager_->getMaxConcurrentTasks(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why did this change? leftover debug thing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed the whole thread management logic/controller service
a1401a3 to
8f3c38b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Refactors MiNiFi C++ controller service APIs to reduce/reshape the exposed surface area, introducing a new “API/implementation” split and updating call sites across core, extensions, and tests.
Changes:
- Introduces new controller-service API types (API, context, descriptor, factory, metadata) and reworks controller service registration/lookup.
- Replaces many direct
ControllerServiceImplusages withControllerServiceBase+ControllerServiceInterfaceand wraps implementations in a newcore::controller::ControllerService. - Updates thread pool/controller service wiring and adjusts numerous unit/integration tests to the new controller service access patterns.
Reviewed changes
Copilot reviewed 118 out of 118 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| minifi-api/include/minifi-cpp/core/controller/ControllerServiceProvider.h | Removed legacy API header for controller service provider. |
| minifi-api/include/minifi-cpp/core/controller/ControllerServiceMetadata.h | Replaced old node/service API types with a minimal metadata struct. |
| minifi-api/include/minifi-cpp/core/controller/ControllerServiceInterface.h | Reduced controller service interface to a marker-style interface. |
| minifi-api/include/minifi-cpp/core/controller/ControllerServiceFactory.h | Added factory interface for creating controller service API implementations. |
| minifi-api/include/minifi-cpp/core/controller/ControllerServiceDescriptor.h | Added descriptor API for declaring supported properties. |
| minifi-api/include/minifi-cpp/core/controller/ControllerServiceContext.h | Added context API for property access during enable. |
| minifi-api/include/minifi-cpp/core/controller/ControllerServiceApi.h | Added new controller-service API surface (initialize/onEnable/notifyStop). |
| minifi-api/include/minifi-cpp/core/ProcessContext.h | Switched controller service return type to ControllerServiceInterface. |
| minifi-api/include/minifi-cpp/core/ControllerServiceApiDefinition.h | Added version field to controller service API definition. |
| minifi-api/include/minifi-cpp/core/ClassLoader.h | Added overload to register controller service factories. |
| minifi-api/include/minifi-cpp/controllers/ThreadManagementService.h | Migrated to ControllerServiceInterface. |
| minifi-api/include/minifi-cpp/controllers/SSLContextServiceInterface.h | Migrated to ControllerServiceInterface and added API version. |
| minifi-api/include/minifi-cpp/controllers/RecordSetWriter.h | Migrated to ControllerServiceInterface and added API version. |
| minifi-api/include/minifi-cpp/controllers/RecordSetReader.h | Migrated to ControllerServiceInterface and added API version. |
| minifi-api/include/minifi-cpp/controllers/AttributeProviderService.h | Migrated to ControllerServiceInterface. |
| libminifi/test/unit/YamlFlowSerializerTests.cpp | Updated controller service accessors to new return types. |
| libminifi/test/unit/UpdatePolicyTests.cpp | Updated controller service creation and access to implementation. |
| libminifi/test/unit/ProcessorConfigUtilsTests.cpp | Updated controller-service parsing tests for new wrappers/interfaces. |
| libminifi/test/unit/NetworkPrioritizerServiceTests.cpp | Updated tests to use wrapped controller services and implementation accessors. |
| libminifi/test/unit/NetUtilsTest.cpp | Updated SSL context service retrieval and usage. |
| libminifi/test/unit/JsonFlowSerializerTests.cpp | Updated controller service accessors to new return types. |
| libminifi/test/unit/ComponentManifestTests.cpp | Updated example controller service type hierarchy for new APIs. |
| libminifi/test/libtest/unit/MockClasses.h | Updated mock controller service usage for new ControllerServiceInterface. |
| libminifi/test/libtest/unit/ControllerServiceUtils.h | Added test helper for constructing wrapped controller services. |
| libminifi/test/keyvalue-tests/VolatileMapStateStorageTest.cpp | Updated controller service implementation retrieval helper usage. |
| libminifi/test/keyvalue-tests/PersistentStateStorageTest.cpp | Updated controller service implementation retrieval helper usage. |
| libminifi/test/integration/ControllerServiceIntegrationTests.cpp | Updated controller service implementation retrieval helper usage. |
| libminifi/test/integration/C2ControllerEnableFailureTest.cpp | Updated dummy controller service to new base/interface split. |
| libminifi/src/core/state/nodes/ResponseNodeLoader.cpp | Updated UpdatePolicy controller retrieval to new implementation access path. |
| libminifi/src/core/logging/LoggerConfiguration.cpp | Switched SSLContextService creation to createAndEnable. |
| libminifi/src/core/controller/ControllerServiceProvider.cpp | Renamed provider impl usage to new provider type. |
| libminifi/src/core/controller/ControllerServiceNode.cpp | Updated node implementation accessors and signatures. |
| libminifi/src/core/ClassLoader.cpp | Added wrapper to adapt controller service factories into object factories. |
| libminifi/src/controllers/UpdatePolicyControllerService.cpp | Removed now-unneeded legacy lifecycle methods. |
| libminifi/src/controllers/SSLContextService.cpp | Added createAndEnable helper using new wrapper service type. |
| libminifi/src/controllers/NetworkPrioritizerService.cpp | Updated prioritizer token handling and linkage with new API model. |
| libminifi/src/c2/protocols/RESTSender.cpp | Switched SSLContextService creation to createAndEnable. |
| libminifi/src/c2/ControllerSocketProtocol.cpp | Switched SSLContextService creation to createAndEnable. |
| libminifi/src/c2/C2Agent.cpp | Updated UpdatePolicy controller retrieval to new implementation access path. |
| libminifi/src/RemoteProcessGroupPort.cpp | Updated controller service lookup return type and SSL fallback creation. |
| libminifi/src/FlowController.cpp | Updated thread pool controller service provider wiring. |
| libminifi/include/io/NetworkPrioritizer.h | Removed global prioritizer factory and API method from interface. |
| libminifi/include/core/controller/StandardControllerServiceProvider.h | Updated to new provider base type and base include. |
| libminifi/include/core/controller/StandardControllerServiceNode.h | Updated to new node base type and includes. |
| libminifi/include/core/controller/ForwardingControllerServiceProvider.h | Updated to new provider base and removed deprecated methods. |
| libminifi/include/core/controller/ControllerServiceProvider.h | Consolidated provider API/impl and removed linked-service state helpers. |
| libminifi/include/core/controller/ControllerServiceNodeMap.h | Updated include paths for new controller service node header. |
| libminifi/include/core/controller/ControllerServiceNode.h | Replaced old node interface with new concrete base and templated impl accessor. |
| libminifi/include/core/controller/ControllerServiceLookup.h | Removed enabled/enabling/name helpers; kept lookup methods only. |
| libminifi/include/core/controller/ControllerService.h | Added new wrapper controller service type around ControllerServiceApi. |
| libminifi/include/core/ProcessGroup.h | Updated include path for controller service node. |
| libminifi/include/core/ProcessContextImpl.h | Updated to return ControllerServiceInterface and use templated node accessor. |
| libminifi/include/core/FlowConfiguration.h | Updated include path for controller service node. |
| libminifi/include/controllers/UpdatePolicyControllerService.h | Migrated to ControllerServiceBase + ControllerServiceInterface. |
| libminifi/include/controllers/ThreadManagementService.h | Migrated to ControllerServiceBase and updated initialization. |
| libminifi/include/controllers/SSLContextService.h | Migrated to ControllerServiceBase and added createAndEnable. |
| libminifi/include/controllers/NetworkPrioritizerService.h | Migrated to ControllerServiceBase + ControllerServiceInterface and embedded prioritizer state. |
| libminifi/include/c2/HeartbeatReporter.h | Updated include path for controller service provider. |
| libminifi/include/c2/ControllerSocketProtocol.h | Updated include path for controller service provider. |
| libminifi/include/c2/C2Protocol.h | Updated include path for controller service provider. |
| libminifi/include/SchedulingAgent.h | Updated include path for controller service provider. |
| libminifi/include/FlowController.h | Updated include path for controller service provider. |
| extensions/standard-processors/tests/unit/XMLRecordSetWriterTests.cpp | Updated tests to use wrapped controller services and implementation accessors. |
| extensions/standard-processors/tests/unit/XMLReaderTests.cpp | Updated tests to use wrapped controller services and implementation accessors. |
| extensions/standard-processors/tests/unit/TailFileTests.cpp | Updated test controller service mock to match new interface expectations. |
| extensions/standard-processors/tests/unit/JsonRecordTests.cpp | Updated tests to use wrapped controller services and implementation accessors. |
| extensions/standard-processors/tests/unit/ControllerServiceTests.cpp | Updated tests to construct new wrapped controller service type. |
| extensions/standard-processors/controllers/XMLRecordSetWriter.h | Updated service construction style and removed legacy common macros. |
| extensions/standard-processors/controllers/XMLReader.h | Updated service construction style and removed legacy common macros. |
| extensions/standard-processors/controllers/VolatileMapStateStorage.h | Updated construction style and removed legacy common macros. |
| extensions/standard-processors/controllers/VolatileMapStateStorage.cpp | Updated base initialization call to ControllerServiceBase. |
| extensions/standard-processors/controllers/PersistentMapStateStorage.h | Updated construction style and removed legacy common macros. |
| extensions/standard-processors/controllers/PersistentMapStateStorage.cpp | Updated base initialization call to ControllerServiceBase. |
| extensions/standard-processors/controllers/JsonTreeReader.h | Updated construction style and removed legacy common macros. |
| extensions/standard-processors/controllers/JsonRecordSetWriter.h | Updated construction style and removed legacy common macros. |
| extensions/sql/tests/mocks/MockODBCService.h | Updated construction style and removed legacy common macros. |
| extensions/sql/services/ODBCConnector.h | Updated ODBCService construction style and removed legacy common macros. |
| extensions/sql/services/DatabaseService.h | Migrated to ControllerServiceBase + ControllerServiceInterface. |
| extensions/sql/services/DatabaseService.cpp | Updated base initialization call to ControllerServiceBase. |
| extensions/smb/tests/utils/MockSmbConnectionControllerService.h | Updated test mock to remove legacy controller-service macros. |
| extensions/smb/tests/SmbConnectionControllerServiceTests.cpp | Updated to use templated node implementation accessor. |
| extensions/smb/tests/PutSmbTests.cpp | Updated to use templated node implementation accessor. |
| extensions/smb/tests/ListSmbTests.cpp | Updated to use templated node implementation accessor. |
| extensions/smb/tests/ListAndFetchSmbTests.cpp | Updated to use templated node implementation accessor. |
| extensions/smb/tests/FetchSmbTests.cpp | Updated to use templated node implementation accessor. |
| extensions/smb/SmbConnectionControllerService.h | Migrated to ControllerServiceBase + ControllerServiceInterface. |
| extensions/rocksdb-repos/controllers/RocksDbStateStorage.h | Updated construction style and removed legacy common macros. |
| extensions/rocksdb-repos/controllers/RocksDbStateStorage.cpp | Updated base initialization call to ControllerServiceBase. |
| extensions/kubernetes/controllerservice/KubernetesControllerService.h | Updated construction style and removed legacy common macros. |
| extensions/kubernetes/controllerservice/KubernetesControllerService.cpp | Updated base initialization call to ControllerServiceBase. |
| extensions/gcp/tests/GCPCredentialsControllerServiceTests.cpp | Updated to use templated node implementation accessor. |
| extensions/gcp/controllerservices/GCPCredentialsControllerService.h | Migrated to ControllerServiceBase + ControllerServiceInterface. |
| extensions/elasticsearch/ElasticsearchCredentialsControllerService.h | Migrated to ControllerServiceBase + ControllerServiceInterface. |
| extensions/couchbase/tests/PutCouchbaseKeyTests.cpp | Updated to use templated node implementation accessor. |
| extensions/couchbase/tests/MockCouchbaseClusterService.h | Updated test mock to remove legacy controller-service macros. |
| extensions/couchbase/tests/GetCouchbaseKeyTests.cpp | Updated to use templated node implementation accessor. |
| extensions/couchbase/controllerservices/CouchbaseClusterService.h | Migrated to ControllerServiceBase + ControllerServiceInterface. |
| extensions/civetweb/tests/ListenHTTPTests.cpp | Switched SSLContextService creation to createAndEnable. |
| extensions/azure/processors/AzureStorageProcessorBase.cpp | Updated controller service lookup return type. |
| extensions/azure/controllerservices/AzureStorageCredentialsService.h | Migrated to ControllerServiceBase + ControllerServiceInterface. |
| extensions/aws/tests/MultipartUploadStateStorageTest.cpp | Updated state storage construction to use metadata-based constructor. |
| extensions/aws/tests/AWSCredentialsServiceTest.cpp | Updated to use templated node implementation accessor. |
| extensions/aws/controllerservices/AWSCredentialsService.h | Migrated to ControllerServiceBase + ControllerServiceInterface. |
| extension-framework/src/controllers/keyvalue/KeyValueStateStorage.cpp | Removed legacy constructor in favor of new base. |
| extension-framework/include/utils/ProcessorConfigUtils.h | Updated controller service parsing to use ControllerServiceInterface. |
| extension-framework/include/controllers/keyvalue/KeyValueStateStorage.h | Migrated to ControllerServiceBase + ControllerServiceInterface. |
| extension-framework/include/controllers/RecordSetWriter.h | Migrated to ControllerServiceBase + interface exposure. |
| extension-framework/include/controllers/RecordSetReader.h | Migrated to ControllerServiceBase + interface exposure. |
| extension-framework/include/controllers/AttributeProviderService.h | Migrated to ControllerServiceBase + interface exposure. |
| encrypt-config/FlowConfigEncryptor.cpp | Updated controller service implementation retrieval call site. |
| core-framework/src/utils/ThreadPool.cpp | Updated controller service provider API and thread manager lookup. |
| core-framework/include/utils/ThreadPool.h | Updated provider API from lookup pointer to function callback. |
| core-framework/include/core/controller/ControllerServiceFactoryImpl.h | Added templated factory implementation for controller services. |
| core-framework/include/core/controller/ControllerServiceBase.h | Added new base implementation for controller service APIs. |
| core-framework/include/core/controller/ControllerService.h | Removed legacy ControllerServiceImpl header. |
| core-framework/include/core/Resource.h | Updated resource registration to also register controller services via factories. |
| controller/tests/ControllerTests.cpp | Switched SSLContextService creation to createAndEnable. |
| controller/MiNiFiController.cpp | Switched SSLContextService creation to createAndEnable. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
minifi-api/include/minifi-cpp/core/controller/ControllerServiceContext.h
Show resolved
Hide resolved
8f3c38b to
d6bf004
Compare
martinzink
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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:
For documentation related changes:
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.