m-hogue commented on a change in pull request #3961: NIFI-6915 Jms Durable non 
shared subscription is broken
URL: https://github.com/apache/nifi/pull/3961#discussion_r364347248
 
 

 ##########
 File path: 
nifi-nar-bundles/nifi-jms-bundle/nifi-jms-processors/src/main/java/org/apache/nifi/jms/processors/ConsumeJMS.java
 ##########
 @@ -173,6 +175,23 @@
         relationships = Collections.unmodifiableSet(_relationships);
     }
 
+    private static boolean isDurableSubscriber(final ProcessContext context) {
+        final Boolean durableBoolean = 
context.getProperty(DURABLE_SUBSCRIBER).evaluateAttributeExpressions().asBoolean();
+        return durableBoolean == null ? false : durableBoolean;
+    }
+
+    private static boolean isShared(final ProcessContext context) {
+        final Boolean sharedBoolean = 
context.getProperty(SHARED_SUBSCRIBER).evaluateAttributeExpressions().asBoolean();
+        return sharedBoolean == null ? false : sharedBoolean;
+    }
+
+    @OnScheduled
+    public void onSchedule(ProcessContext context) {
 
 Review comment:
   I believe you have a typo here. I think the logic should be 
`context.getMaxConcurrentTasks() > 1 && isDurableSubscriber(context) && 
!isShared(context)`. As is, this condition would never be satisfied.
   
   Can you please add a unit test verifying this new behavior? Also, is it 
possible to add unit test(s) to verify that the issue you're trying to address 
(changing client IDs for durable non-shared clients on processor restart) is 
resolved?

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

Reply via email to