diff --git a/nifi-api/src/main/java/org/apache/nifi/components/PropertyDescriptor.java b/nifi-api/src/main/java/org/apache/nifi/components/PropertyDescriptor.java index acda6c4bfa..77c349a31d 100644 --- a/nifi-api/src/main/java/org/apache/nifi/components/PropertyDescriptor.java +++ b/nifi-api/src/main/java/org/apache/nifi/components/PropertyDescriptor.java @@ -150,8 +150,7 @@ public final class PropertyDescriptor implements Comparable } final String serviceId = controllerService.getIdentifier(); - if (!context.getControllerServiceLookup().isControllerServiceEnabled(serviceId) - && !context.getControllerServiceLookup().isControllerServiceEnabling(serviceId)) { + if (!isDependentServiceEnableable(context, serviceId)) { return new ValidationResult.Builder() .input(context.getControllerServiceLookup().getControllerServiceName(serviceId)) .subject(getName()) @@ -197,6 +196,28 @@ public final class PropertyDescriptor implements Comparable return lastResult; } + /** + * Will validate if the dependent service (service identified with the + * 'serviceId') is 'enableable' which means that the dependent service is + * either in ENABLING or ENABLED state. The important issue here is to + * understand the order in which states are assigned: + * + * - Upon the initialization of the service its state is set to ENABLING. + * + * - Transition to ENABLED will happen asynchronously. + * + * So we check first for ENABLING state and if it succeeds we skip the check + * for ENABLED state even though by the time this method returns the + * dependent service's state could be fully ENABLED. + */ + private boolean isDependentServiceEnableable(ValidationContext context, String serviceId) { + boolean enableable = context.getControllerServiceLookup().isControllerServiceEnabling(serviceId); + if (!enableable) { + enableable = context.getControllerServiceLookup().isControllerServiceEnabled(serviceId); + } + return enableable; + } + public static final class Builder { private String displayName = null; diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/test/java/org/apache/nifi/controller/service/TestStandardControllerServiceProvider.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/test/java/org/apache/nifi/controller/service/TestStandardControllerServiceProvider.java index a3589fae0d..bca13eb566 100644 --- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/test/java/org/apache/nifi/controller/service/TestStandardControllerServiceProvider.java +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/test/java/org/apache/nifi/controller/service/TestStandardControllerServiceProvider.java @@ -20,6 +20,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; +import java.beans.PropertyDescriptor; import java.util.HashSet; import java.util.LinkedHashMap; import java.util.List; @@ -110,9 +111,21 @@ public class TestStandardControllerServiceProvider { } } - @Test(timeout=10000) - public void testEnableReferencingServicesGraph() { + /** + * We run the same test 1000 times and prior to bug fix (see NIFI-1143) it + * would fail on some iteration. For more details please see + * {@link PropertyDescriptor}.isDependentServiceEnableable() as well as + * https://issues.apache.org/jira/browse/NIFI-1143 + */ + @Test(timeout = 10000) + public void testConcurrencyWithEnablingReferencingServicesGraph() { final ProcessScheduler scheduler = createScheduler(); + for (int i = 0; i < 1000; i++) { + testEnableReferencingServicesGraph(scheduler); + } + } + + public void testEnableReferencingServicesGraph(ProcessScheduler scheduler) { final StandardControllerServiceProvider provider = new StandardControllerServiceProvider(scheduler, null); // build a graph of controller services with dependencies as such: