Copilot commented on code in PR #2166:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2166#discussion_r3159726074
##########
minifi-api/include/minifi-c/minifi-c.h:
##########
@@ -254,16 +259,30 @@ size_t MinifiInputStreamSize(MinifiInputStream*);
int64_t MinifiInputStreamRead(MinifiInputStream* stream, char* buffer, size_t
size);
int64_t MinifiOutputStreamWrite(MinifiOutputStream* stream, const char* data,
size_t size);
-MinifiStatus MinifiFlowFileSetAttribute(MinifiProcessSession* session,
MinifiFlowFile* flowfile, MinifiStringView attribute_name, const
MinifiStringView* attribute_value);
-MinifiBool MinifiFlowFileGetAttribute(MinifiProcessSession* session,
MinifiFlowFile* flowfile, MinifiStringView attribute_name,
+MinifiStatus MinifiProcessSessionSetFlowFileAttribute(MinifiProcessSession*
session, MinifiFlowFile* flowfile, MinifiStringView attribute_name, const
MinifiStringView* attribute_value);
+MinifiBool MinifiProcessSessionGetFlowFileAttribute(MinifiProcessSession*
session, MinifiFlowFile* flowfile, MinifiStringView attribute_name,
void(*cb)(void* user_ctx,
MinifiStringView attribute_value), void* user_ctx);
-void MinifiFlowFileGetAttributes(MinifiProcessSession* session,
MinifiFlowFile* flowfile, void(*cb)(void* user_ctx, MinifiStringView
attribute_name, MinifiStringView attribute_value), void* user_ctx);
+void MinifiProcessSessionGetFlowFileAttributes(MinifiProcessSession* session,
MinifiFlowFile* flowfile,
+ void (*cb)(void* user_ctx, MinifiStringView attribute_name,
MinifiStringView attribute_value), void* user_ctx);
+uint64_t MinifiProcessSessionGetFlowFileSize(MinifiProcessSession* session,
MinifiFlowFile* flowfile);
+MinifiStatus MinifiProcessSessionGetFlowFileId(MinifiProcessSession* session,
MinifiFlowFile* flowfile, void(*cb)(void* user_ctx, MinifiStringView
flow_file_id), void* user_ctx);
MinifiStatus
MinifiControllerServiceContextGetProperty(MinifiControllerServiceContext*
context,
MinifiStringView property_name,
void(*cb)(void* user_ctx, MinifiStringView property_value),
void* user_ctx);
+struct MinifiSslData {
+ uint8_t version = 1;
+ MinifiStringView ca_certificate{};
+ MinifiStringView certificate_file{};
+ MinifiStringView private_key_file{};
+ MinifiStringView passphrase{};
+};
+
+MinifiStatus MinifiProcessContextGetSslData(MinifiProcessContext*
process_context, MinifiStringView controller_service_name,
+ void (*cb)(void* user_ctx, const MinifiSslData& ssl_data), void* user_ctx);
Review Comment:
`MinifiProcessContextGetSslData` exposes a callback signature that takes
`const MinifiSslData&`. For a C API (and for stable cross-compiler ABI), avoid
C++ references in exported function pointer types; use a pointer (e.g., `const
MinifiSslData*`) or pass by value, and make `MinifiSslData` an explicitly
C-compatible POD/typedef if the header is intended for C/FFI consumers.
```suggestion
uint8_t version;
MinifiStringView ca_certificate;
MinifiStringView certificate_file;
MinifiStringView private_key_file;
MinifiStringView passphrase;
};
MinifiStatus MinifiProcessContextGetSslData(MinifiProcessContext*
process_context, MinifiStringView controller_service_name,
void (*cb)(void* user_ctx, const MinifiSslData* ssl_data), void*
user_ctx);
```
##########
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
```
##########
libminifi/src/minifi-c.cpp:
##########
@@ -578,5 +603,39 @@ MinifiStatus MinifiProcessContextGetControllerService(
return MINIFI_STATUS_VALIDATION_FAILED;
}
+void MinifiProcessContextGetDynamicProperties(MinifiProcessContext* context,
+ void (*cb)(void* user_ctx, MinifiStringView dynamic_property_name,
MinifiStringView dynamic_property_value), void* user_ctx) {
+ gsl_Assert(context != MINIFI_NULL);
+ for (auto& [key, value] :
reinterpret_cast<minifi::core::ProcessContext*>(context)->getDynamicProperties())
{
+ cb(user_ctx, minifiStringView(key), minifiStringView(value));
+ }
Review Comment:
`MinifiProcessContextGetDynamicProperties` iterates dynamic properties via
`ProcessContext::getDynamicProperties()` without a FlowFile parameter, which
can prevent Expression Language substitution based on FlowFile attributes (core
processors often call `getDynamicProperties(&flow_file)`). If this API is
intended for onTrigger-time evaluation, consider adding a `MinifiFlowFile*`
argument and forwarding it to `getDynamicProperties(flow_file)`; otherwise,
clarify that returned values are evaluated with `flow_file=nullptr`.
##########
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*`.
*/
```
##########
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}});
```
##########
minifi-api/minifi-c-api.def:
##########
@@ -16,12 +16,17 @@ EXPORTS
MinifiProcessSessionCreate
MinifiProcessSessionTransfer
MinifiProcessSessionRemove
+ MinifiProcessSessionPenalize
MinifiProcessSessionRead
MinifiProcessSessionWrite
MinifiConfigGet
MinifiInputStreamSize
MinifiInputStreamRead
MinifiOutputStreamWrite
- MinifiFlowFileSetAttribute
- MinifiFlowFileGetAttribute
- MinifiFlowFileGetAttributes
+ MinifiProcessSessionSetFlowFileAttribute
+ MinifiProcessSessionGetFlowFileAttribute
+ MinifiProcessSessionGetFlowFileAttributes
+ MinifiProcessSessionGetFlowFileSize
+ MinifiProcessSessionGetFlowFileId
+ MinifiProcessContextGetDynamicProperties
+ MinifiProcessContextGetSslData
Review Comment:
This .def export list removes the previously exported
`MinifiFlowFileSetAttribute/GetAttribute/GetAttributes` symbols and replaces
them with new `MinifiProcessSession*` names. On Windows this is an ABI-breaking
change for any existing extension DLLs linking against the old symbol names.
Consider keeping the old exports as backward-compatible wrappers/aliases (at
least for one major API version), or bumping the API/ABI appropriately and
documenting the breaking change.
--
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]
