Copilot commented on code in PR #2120:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2120#discussion_r2852764279
##########
libminifi/src/core/ProcessSession.cpp:
##########
@@ -256,7 +257,15 @@ void ProcessSessionImpl::write(core::FlowFile &flow, const
io::OutputStreamCallb
if (nullptr == stream) {
throw Exception(FILE_OPERATION_EXCEPTION, "Failed to open flowfile
content for write");
}
- if (callback(stream) < 0) {
+ const auto callback_result = callback(stream);
+ if (callback_result == MinifiIoStatus::MINIFI_IO_CANCEL) {
+ stream->close();
+ content_session_->remove(claim);
+ claim.reset();
+ return;
Review Comment:
`MinifiIoStatus` is declared as an unscoped enum in `minifi-c.h` (`typedef
enum MinifiIoStatus : int64_t { ... }`), so the enumerators are not members of
the type. `MinifiIoStatus::MINIFI_IO_CANCEL` will not compile; use
`MINIFI_IO_CANCEL` (unqualified) or change the enum to a scoped `enum class`
and adjust the comparison/casts accordingly.
##########
libminifi/src/core/ProcessSession.cpp:
##########
@@ -256,7 +257,15 @@ void ProcessSessionImpl::write(core::FlowFile &flow, const
io::OutputStreamCallb
if (nullptr == stream) {
throw Exception(FILE_OPERATION_EXCEPTION, "Failed to open flowfile
content for write");
}
- if (callback(stream) < 0) {
+ const auto callback_result = callback(stream);
+ if (callback_result == MinifiIoStatus::MINIFI_IO_CANCEL) {
+ stream->close();
+ content_session_->remove(claim);
+ claim.reset();
+ return;
+ }
Review Comment:
The new cancellation path (`callback_result == ...CANCEL`) changes write
semantics but doesn't appear to be covered by existing unit/integration tests.
Please add a test that returns the cancel status from the callback and asserts
that the FlowFile's original content/claim is preserved and the temporary claim
is released (no orphan content left behind).
--
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]
