Copilot commented on code in PR #2166:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2166#discussion_r3159991600
##########
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:
The previous FlowFile attribute C API symbols (e.g.,
`MinifiFlowFileSetAttribute` / `MinifiFlowFileGetAttribute` /
`MinifiFlowFileGetAttributes`) appear to have been removed from the export
list. That’s an ABI-breaking change for existing extensions, and on platforms
using `RTLD_NOW`/Windows import libs it can prevent older extensions from
loading at all. Consider keeping the old symbols exported as deprecated
wrappers that forward to the new `MinifiProcessSession*` names (or bump the API
version and still keep old exports if backward compatibility is intended).
##########
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:
This PR adds new exported C API entry points, but `MINIFI_API_VERSION`
remains `2`. With the current extension loader (`RTLD_NOW`) and the default
version gating (min supported == agent version), an extension built against
these new symbols can fail to load on an older agent with missing exports,
without a clean version-mismatch error. Please bump `MINIFI_API_VERSION` (and,
if backward compatibility is desired, adjust the agent’s min supported API
version separately so older extensions can still load).
```suggestion
MINIFI_API_VERSION = 3
```
##########
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);
Review Comment:
The FlowFile attribute APIs were renamed from `MinifiFlowFile*` to
`MinifiProcessSession*` and the old function declarations are no longer
present. That’s a source/ABI breaking change for existing extensions that
include older headers and link against the old symbol names. Consider
reintroducing the old function names as exported wrappers (marked deprecated)
so existing extensions keep loading, while steering new code to the new naming.
##########
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;
+ MinifiStringView ca_certificate;
+ MinifiStringView certificate_file;
+ MinifiStringView private_key_file;
+ MinifiStringView passphrase;
+};
Review Comment:
This API exposes the private key passphrase (`MinifiSslData.passphrase`) to
extensions. Since this is sensitive material, please consider whether returning
it in plaintext is strictly necessary for the Kafka migration; if not, prefer
an API that avoids disclosing secrets (e.g., provide only file paths, or
provide a helper that configures an SSL/TLS context without revealing the
passphrase). If it must be exposed, document it as sensitive and ensure callers
are expected to copy it immediately (it’s currently returned as a non-owning
view).
##########
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;
+ MinifiStringView ca_certificate;
Review Comment:
`MinifiSslData.ca_certificate` is populated from
`SSLContextServiceInterface::getCACertificate()`, which returns a *file path*
(see the interface). The field name reads like it contains the certificate
contents, which is misleading for API consumers. Consider renaming to something
like `ca_certificate_file`/`ca_cert_file` (and similarly ensure naming is
consistently “*_file” for all path fields).
```suggestion
MinifiStringView ca_certificate_file;
```
##########
libminifi/src/minifi-c.cpp:
##########
@@ -525,7 +537,7 @@ MinifiBool MinifiFlowFileGetAttribute(MinifiProcessSession*
session, MinifiFlowF
return true;
}
-void MinifiFlowFileGetAttributes(MinifiProcessSession* session,
MinifiFlowFile* flowfile,
+void MinifiProcessSessionGetFlowFileAttributes(MinifiProcessSession* session,
MinifiFlowFile* flowfile,
Review Comment:
The old C API entry points (`MinifiFlowFileSetAttribute` /
`MinifiFlowFileGetAttribute` / `MinifiFlowFileGetAttributes`) appear to have
been renamed rather than kept as wrappers. Removing these symbols is an ABI
break that can prevent already-built extensions from loading. Please consider
adding back the old function names here as thin forwarding wrappers to the new
`MinifiProcessSession*` functions (and keep them exported) to preserve binary
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
