exceptionfactory commented on code in PR #10245:
URL: https://github.com/apache/nifi/pull/10245#discussion_r2303959643
##########
nifi-mock/src/main/java/org/apache/nifi/util/MockProcessSession.java:
##########
@@ -1456,4 +1481,73 @@ boolean isFlowFileKnown(final FlowFile flowFile) {
final String providedUuid =
curFlowFile.getAttribute(CoreAttributes.UUID.key());
return curUuid.equals(providedUuid);
}
+
+ public static final class TestFailedCommitException extends
RuntimeException {
+ private TestFailedCommitException() {
+ super("Failing the commit, as requested by the test");
+ }
+ }
+
+ public static final class Builder {
+
+ private final SharedSessionState sharedState;
+ private final Processor processor;
+ private StateManager stateManager;
+
+ private boolean enforceStreamsClosed = true;
+ private boolean allowRecursiveReads = false;
+ private boolean allowSynchronousCommits = false;
+ private boolean shouldFailCommit = false;
Review Comment:
With the default value being false, setting these values to `false` is
unnecessary.
##########
nifi-framework-bundle/nifi-framework/nifi-site-to-site/src/test/java/org/apache/nifi/remote/protocol/http/TestHttpFlowFileServerProtocol.java:
##########
@@ -525,7 +524,9 @@ private void setupMockProcessSession() {
when(rootGroupPort.getIdentifier()).thenReturn("root-group-port-id");
sessionState = new SharedSessionState(rootGroupPort, new
AtomicLong(0));
- processSession = new MockProcessSession(sessionState, rootGroupPort,
true, new MockStateManager(rootGroupPort), true);
+ processSession = MockProcessSession.builder(sessionState,
rootGroupPort)
+ .allowSynchronousCommits()
+ .build();
Review Comment:
Recommend reverting this change to keep the scope of this PR focused on
`nifi-mock`.
##########
nifi-mock/src/main/java/org/apache/nifi/util/MockProcessSession.java:
##########
@@ -285,14 +296,23 @@ public void commit() {
"enabled by calling TestRunner.");
}
- commitInternal();
+ try {
+ commitInternal();
+ } catch (final Throwable t) {
+ rollback();
+ throw t;
+ }
}
private void commitInternal() {
if (!beingProcessed.isEmpty()) {
throw new FlowFileHandlingException("Cannot commit session because
the following FlowFiles have not been removed or transferred: " +
beingProcessed);
}
+ if (shouldFailCommit) {
+ throw new TestFailedCommitException();
Review Comment:
Recommend throwing a `FlowFileHandlingException` instead of creating a new
Exception class.
##########
nifi-mock/src/main/java/org/apache/nifi/util/MockProcessSession.java:
##########
@@ -88,6 +88,7 @@ public class MockProcessSession implements ProcessSession {
private final StateManager stateManager;
private final boolean allowSynchronousCommits;
private final boolean allowRecursiveReads;
+ private final boolean shouldFailCommit;
Review Comment:
Recommend naming this `failCommit` and updating references, since `should`
implies optional behavior, versus expected behavior.
```suggestion
private final boolean failCommit;
```
--
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]
