Copilot commented on code in PR #2166:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2166#discussion_r3159726127
##########
minifi-api/include/minifi-c/minifi-c.h:
##########
@@ -232,6 +234,8 @@ MinifiBool
MinifiProcessContextHasNonEmptyProperty(MinifiProcessContext* context
MinifiStatus MinifiProcessContextGetControllerService(
MinifiProcessContext* process_context, MinifiStringView
controller_service_name, MinifiStringView controller_service_type,
MinifiControllerService** controller_service_out);
Review Comment:
`MinifiProcessContextGetDynamicProperties` currently does not accept a
`MinifiFlowFile*`, and the implementation calls
`ProcessContext::getDynamicProperties()` with the default `flow_file=nullptr`.
This means dynamic property values that use Expression Language and depend on
FlowFile attributes cannot be evaluated like many core processors do (they call
`getDynamicProperties(&flow_file)`). Consider adding an overload/parameter to
supply the FlowFile, or document that this API returns unevaluated/raw dynamic
properties.
```suggestion
MinifiProcessContext* process_context, MinifiStringView
controller_service_name, MinifiStringView controller_service_type,
MinifiControllerService** controller_service_out);
/**
* Retrieves the configured dynamic properties for the processor context.
*
* This API does not accept a `MinifiFlowFile*`, so dynamic property values
are
* not evaluated against FlowFile attributes. If a dynamic property uses
* Expression Language that depends on FlowFile attributes, the callback may
* receive the raw/unevaluated property value.
*
* To evaluate a specific property value against a FlowFile, use
* `MinifiProcessContextGetProperty()` and pass the relevant
`MinifiFlowFile*`.
*/
```
##########
minifi-api/include/minifi-c/minifi-c.h:
##########
@@ -41,6 +41,8 @@ extern "C" {
#define MINIFI_REGISTER_EXTENSION_FN MinifiRegisterExtension
#endif
+#define MINIFI_SSL_CONTEXT_SERVICE_API
"org.apache.nifi.minifi.controllers.SSLContextServiceInterface"
+
enum : uint32_t {
MINIFI_API_VERSION = 2
Review Comment:
The C extension API surface changed (new functions added and old exported
FlowFile attribute functions renamed/removed), but `MINIFI_API_VERSION` is
still `2`. Please bump the API version so extensions can reliably
detect/require this API level, and consider whether the min supported version
needs to be updated accordingly.
```suggestion
MINIFI_API_VERSION = 3
```
##########
extension-framework/cpp-extension-lib/src/core/ProcessSession.cpp:
##########
@@ -118,28 +118,28 @@ void ProcessSession::read(FlowFile& flow_file, const
io::InputStreamCallback& ca
void ProcessSession::setAttribute(FlowFile& ff, const std::string_view key,
std::string value) { // NOLINT(performance-unnecessary-value-param)
const MinifiStringView value_ref = utils::toStringView(value);
- if (MINIFI_STATUS_SUCCESS != MinifiFlowFileSetAttribute(impl_, ff.get(),
utils::toStringView(key), &value_ref)) {
+ if (MINIFI_STATUS_SUCCESS != MinifiProcessSessionSetFlowFileAttribute(impl_,
ff.get(), utils::toStringView(key), &value_ref)) {
throw minifi::Exception(minifi::FILE_OPERATION_EXCEPTION, "Failed to set
attribute");
}
}
void ProcessSession::removeAttribute(FlowFile& ff, const std::string_view key)
{
- if (MINIFI_STATUS_SUCCESS != MinifiFlowFileSetAttribute(impl_, ff.get(),
utils::toStringView(key), nullptr)) {
+ if (MINIFI_STATUS_SUCCESS != MinifiProcessSessionSetFlowFileAttribute(impl_,
ff.get(), utils::toStringView(key), nullptr)) {
throw minifi::Exception(minifi::FILE_OPERATION_EXCEPTION, "Failed to
remove attribute");
}
}
std::optional<std::string> ProcessSession::getAttribute(FlowFile& ff,
std::string_view key) {
std::optional<std::string> result;
- MinifiFlowFileGetAttribute(impl_, ff.get(), utils::toStringView(key), []
(void* user_ctx, MinifiStringView value) {
+ MinifiProcessSessionGetFlowFileAttribute(impl_, ff.get(),
utils::toStringView(key), [] (void* user_ctx, MinifiStringView value) {
*static_cast<std::optional<std::string>*>(user_ctx) =
std::string{value.data, value.length};
}, &result);
return result;
}
std::map<std::string, std::string> ProcessSession::getAttributes(FlowFile& ff)
{
std::map<std::string, std::string> result;
- MinifiFlowFileGetAttributes(impl_, ff.get(), [] (void* user_ctx,
MinifiStringView value, MinifiStringView key) {
+ MinifiProcessSessionGetFlowFileAttributes(impl_, ff.get(), [] (void*
user_ctx, MinifiStringView value, MinifiStringView key) {
static_cast<std::map<std::string,
std::string>*>(user_ctx)->insert({std::string{value.data, value.length},
std::string{key.data, key.length}});
Review Comment:
`ProcessSession::getAttributes()` passes a callback with the parameter order
swapped compared to `MinifiProcessSessionGetFlowFileAttributes` (which calls
cb(attribute_name, attribute_value)). As written, the resulting `std::map` will
contain values as keys and keys as values. Adjust the lambda parameter
naming/order and the `insert` call so the map is populated as attribute_name ->
attribute_value.
```suggestion
MinifiProcessSessionGetFlowFileAttributes(impl_, ff.get(), [] (void*
user_ctx, MinifiStringView key, MinifiStringView value) {
static_cast<std::map<std::string,
std::string>*>(user_ctx)->insert({std::string{key.data, key.length},
std::string{value.data, value.length}});
```
--
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]
