NIFI-3785: Moving controller now fails if there is a referencing comp…

…onent scope conflict.

nifi-framework-bundle/nifi-framework/nifi-framework-components/src/main/java/org/apache/nifi/controller/service/StandardControllerServiceNode.java

@@ -115,6 +115,7 @@ public class StandardControllerServiceNode extends AbstractComponentNode impleme
     private volatile LogLevel bulletinLevel = LogLevel.WARN;
 
     private final AtomicBoolean active;
+    private final AtomicBoolean moving;
 
     public StandardControllerServiceNode(final LoggableComponent<ControllerService> implementation, final LoggableComponent<ControllerService> proxiedControllerService,
                                          final ControllerServiceInvocationHandler invocationHandler, final String id, final ValidationContextFactory validationContextFactory,
@@ -134,6 +135,7 @@ public StandardControllerServiceNode(final LoggableComponent<ControllerService>
         super(id, validationContextFactory, serviceProvider, componentType, componentCanonicalClass, reloadComponent, extensionManager, validationTrigger, isExtensionMissing);
         this.serviceProvider = serviceProvider;
         this.active = new AtomicBoolean();
+        this.moving = new AtomicBoolean(false);
         setControllerServiceAndProxy(implementation, proxiedControllerService, invocationHandler);
         stateTransition = new ServiceStateTransition(this);
         this.comment = "";
@@ -445,7 +447,14 @@ public ControllerServiceState getState() {
     public boolean isActive() {
         return this.active.get();
     }
-
+    @Override
+    public boolean isMoving() {
+        return this.moving.get();
+    }
+    @Override
+    public void setMoving(boolean moving) {
+        this.moving.set(moving);
+    }
     @Override
     public boolean awaitEnabled(final long timePeriod, final TimeUnit timeUnit) throws InterruptedException {
         LOG.debug("Waiting up to {} {} for {} to be enabled", timePeriod, timeUnit, this);

nifi-framework-bundle/nifi-framework/nifi-framework-components/src/main/java/org/apache/nifi/groups/StandardProcessGroup.java

@@ -2592,7 +2592,9 @@ public void removeControllerService(final ControllerServiceNode service) {
                 throw new IllegalStateException("ControllerService " + service.getIdentifier() + " is not a member of this Process Group");
             }
 
-            service.verifyCanDelete();
+            if (!service.isMoving()) {
+                service.verifyCanDelete();
+            }
 
             try (final NarCloseable x = NarCloseable.withComponentNarLoader(extensionManager, service.getControllerServiceImplementation().getClass(), service.getIdentifier())) {
                 final ConfigurationContext configurationContext = new StandardConfigurationContext(service, controllerServiceProvider, null);
@@ -2639,6 +2641,7 @@ public void removeControllerService(final ControllerServiceNode service) {
                 } catch (Throwable t) {
                 }
             }
+            service.setMoving(false);
             writeLock.unlock();
         }
     }

nifi-framework-bundle/nifi-framework/nifi-framework-core-api/src/main/java/org/apache/nifi/controller/service/ControllerServiceNode.java

@@ -206,6 +206,10 @@ public interface ControllerServiceNode extends ComponentNode, VersionedComponent
      */
     boolean isActive();
 
+    boolean isMoving();
+
+    void setMoving(boolean moving);
+
     /**
      * Waits up to the given amount of time for the Controller Service to transition to an ENABLED state.
      * @param timePeriod maximum amount of time to wait

nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/StandardNiFiServiceFacade.java

@@ -2952,9 +2952,20 @@ public ControllerServiceEntity moveControllerService(final Revision revision, fi
         final ControllerServiceNode controllerService = controllerServiceDAO.getControllerService(controllerServiceDTO.getId());
         final RevisionUpdate<ControllerServiceDTO> snapshot = updateComponent(revision,
                 controllerService,
-                () -> moveControllerServiceWork(controllerService, newProcessGroupID),
+                () -> {
+                    final ProcessGroup oldParentGroup = controllerService.getProcessGroup();
+                    controllerService.setMoving(true);
+                    oldParentGroup.removeControllerService(controllerService);
+                    if (!oldParentGroup.isRootGroup() && oldParentGroup.getParent().getIdentifier().equals(newProcessGroupID)) {
+                        // move to parent process group
+                        oldParentGroup.getParent().addControllerService(controllerService);
+                    } else {
+                        // move to child process group
+                        oldParentGroup.getProcessGroup(newProcessGroupID).addControllerService(controllerService);
+                    }
+                    return controllerService;
+                },
                 cs -> {
-                    awaitValidationCompletion(cs);
                     final ControllerServiceDTO dto = dtoFactory.createControllerServiceDto(cs);
                     final ControllerServiceReference ref = controllerService.getReferences();
                     final ControllerServiceReferencingComponentsEntity referencingComponentsEntity = createControllerServiceReferencingComponentsEntity(ref);
@@ -2966,95 +2977,10 @@ public ControllerServiceEntity moveControllerService(final Revision revision, fi
         final PermissionsDTO operatePermissions = dtoFactory.createPermissionsDto(new OperationAuthorizable(controllerService));
         final List<BulletinDTO> bulletins = dtoFactory.createBulletinDtos(bulletinRepository.findBulletinsForSource(controllerServiceDTO.getId()));
         final List<BulletinEntity> bulletinEntities = bulletins.stream().map(bulletin -> entityFactory.createBulletinEntity(bulletin, permissions.getCanRead())).collect(Collectors.toList());
+        controllerService.performValidation();
         return entityFactory.createControllerServiceEntity(snapshot.getComponent(), dtoFactory.createRevisionDTO(snapshot.getLastModification()), permissions, operatePermissions, bulletinEntities);
     }
 
-    private ControllerServiceNode moveControllerServiceWork(final ControllerServiceNode controllerService, final String newProcessGroupID) {
-        final ProcessGroup oldParentGroup = controllerService.getProcessGroup();
-        Set<ComponentNode> referencedComponents = controllerService.getReferences().getReferencingComponents();
-        isReferencesDisabled(referencedComponents);
-        oldParentGroup.removeControllerService(controllerService);
-        ProcessGroup newParent;
-        if (!oldParentGroup.isRootGroup() && oldParentGroup.getParent().getIdentifier().equals(newProcessGroupID)) {
-            // move to parent process group
-            newParent = oldParentGroup.getParent();
-            newParent.addControllerService(controllerService);
-
-            // unset any references the controller services has to other controller services that are now out of scope
-            Map<String, String> updatedProps = new HashMap<>();
-            Set<Map.Entry<PropertyDescriptor, PropertyConfiguration>> properties = controllerService.getProperties().entrySet();
-            for (var prop : properties) {
-                var value = prop.getValue();
-                if (value !=null) {
-                    ControllerServiceNode controller;
-                    try {
-                        controller = controllerServiceDAO.getControllerService((value.getRawValue()));
-                    } catch (Exception e){
-                        continue;
-                    }
-                    if (controller != null) {
-                        if (!hasProcessGroup(controller.getProcessGroup(), newParent.getIdentifier())) {
-                            controller.removeReference(controllerService, prop.getKey());
-                            updatedProps.put(prop.getKey().getName(), null);
-                        }
-                    }
-                }
-            }
-            if (!updatedProps.isEmpty())
-                controllerService.setProperties(updatedProps, true, Collections.emptySet());
-
-        } else {
-            // move to child process group
-            newParent = oldParentGroup.getProcessGroup(newProcessGroupID);
-            newParent.addControllerService(controllerService);
-
-            // unset any references for processors that are outside the new scope
-            for (ComponentNode node : referencedComponents) {
-                if (!hasProcessGroup(newParent, node.getProcessGroupIdentifier())) {
-                    Set<Map.Entry<PropertyDescriptor, PropertyConfiguration>> properties = node.getProperties().entrySet();
-                    Map<String, String> updatedProps = new HashMap<>();
-                    for (Map.Entry<PropertyDescriptor, PropertyConfiguration> prop : properties) {
-                        final PropertyConfiguration value = prop.getValue();
-                        if (value != null && value.getRawValue().equals(controllerService.getIdentifier())) {
-                            controllerService.removeReference(node, prop.getKey());
-                            node.getComponent().onPropertyModified(prop.getKey(), controllerService.getIdentifier(), null);
-                            updatedProps.put(prop.getKey().getName(), null);
-                        }
-                    }
-                    node.setProperties(updatedProps, true, Collections.emptySet());
-                }
-            }
-        }
-
-        return controllerService;
-    }
-    private boolean hasProcessGroup(ProcessGroup root, String id){
-        if (root.getProcessGroup(id) != null || root.getIdentifier().equals(id)) {
-            return true;
-        }
-
-        for (ProcessGroup child : root.getProcessGroups()) {
-            if (hasProcessGroup(child, id)) {
-                return true;
-            }
-        }
-
-        return false;
-    }
-
-    private void isReferencesDisabled(Set<ComponentNode> nodes) {
-        for (ComponentNode node : nodes) {
-            if (node instanceof ProcessorNode processorNode) {
-                if (processorNode.isRunning())
-                    throw new IllegalStateException("Cannot move controller service. Referenced processor " + processorNode + " is running");
-            }
-            if (node instanceof ControllerServiceNode controllerNode) {
-                if (controllerNode.isActive())
-                    throw new IllegalStateException("Cannot move controller service. Referenced controller services " + controllerNode + " is enabled");
-            }
-        }
-    }
-
     @Override
     public List<ConfigVerificationResultDTO> performControllerServiceConfigVerification(final String controllerServiceId, final Map<String, String> properties, final Map<String, String> variables) {
         return controllerServiceDAO.verifyConfiguration(controllerServiceId, properties, variables);

nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/ControllerServiceResource.java

@@ -16,6 +16,7 @@
  */
 package org.apache.nifi.web.api;
 
+import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashSet;
 import java.util.List;
@@ -735,23 +736,6 @@ public Response moveControllerServices(
         }
 
         final ControllerServiceDTO requestControllerServiceDTO = serviceFacade.getControllerService(id, true).getComponent();
-        ControllerServiceState requestControllerServiceState = null;
-        try {
-            requestControllerServiceState = ControllerServiceState.valueOf(requestControllerServiceDTO.getState());
-        } catch (final IllegalArgumentException iae) {
-            // ignore
-        }
-
-        // ensure an action has been specified
-        if (requestControllerServiceState == null) {
-            throw new IllegalArgumentException("Must specify the updated state. To update the referencing Controller Services the "
-                    + "state should be DISABLED.");
-        }
-
-        // ensure the controller service state is not DISABLING
-        if (!ControllerServiceState.DISABLED.equals(requestControllerServiceState) ) {
-            throw new IllegalArgumentException("Cannot set the referencing services to ENABLING or DISABLING");
-        }
 
         final Revision requestRevision = getRevision(requestControllerServiceEntity, id);
         return withWriteLock(
@@ -770,16 +754,34 @@ public Response moveControllerServices(
                     final ProcessGroupAuthorizable authorizableProcessGroupOld = lookup.getProcessGroup(requestControllerServiceDTO.getParentGroupId());
                     authorizableProcessGroupOld.getAuthorizable().authorize(authorizer, RequestAction.WRITE, NiFiUserUtils.getNiFiUser());
 
-                    // authorize any referenced services
-                    AuthorizeControllerServiceReference.authorizeControllerServiceReferences(requestControllerServiceDTO.getProperties(), authorizable, authorizer, lookup);
-                    AuthorizeParameterReference.authorizeParameterReferences(requestControllerServiceDTO.getProperties(), authorizer, authorizable.getParameterContext(),
-                            NiFiUserUtils.getNiFiUser());
-
-                    // authorize referencing components
+                    // Verify all referencing and referenced components are still within scope
+                    List<String> conflictingComponents = new ArrayList<>();
                     requestControllerServiceDTO.getReferencingComponents().forEach(e -> {
-                        final Authorizable controllerService = lookup.getControllerServiceReferencingComponent(requestControllerServiceDTO.getId(), e.getId());
-                        OperationAuthorizable.authorizeOperation(controllerService, authorizer, NiFiUserUtils.getNiFiUser());
+                        if (authorizableProcessGroupNew.getProcessGroup().findProcessor(e.getId()) == null
+                            && authorizableProcessGroupNew.getProcessGroup().findControllerService(e.getId(), true, false) == null) {
+                            conflictingComponents.add("[" + e.getComponent().getName() + "]");
+                        }
+
+                        final Authorizable referencingComponent = lookup.getControllerServiceReferencingComponent(requestControllerServiceDTO.getId(), e.getId());
+                        OperationAuthorizable.authorizeOperation(referencingComponent, authorizer, NiFiUserUtils.getNiFiUser());
+                    });
+
+                    requestControllerServiceDTO.getProperties().forEach((key, value) -> {
+                        try {
+                            ControllerServiceEntity refControllerService = serviceFacade.getControllerService(value, false);
+                            if (refControllerService != null) {
+                                if (authorizableProcessGroupNew.getProcessGroup().findControllerService(value, false, true) == null) {
+                                    conflictingComponents.add("[" + refControllerService.getComponent().getName() + "]");
+                                }
+                            }
+                        } catch(Exception ignored) {}
                     });
+
+                    if (!conflictingComponents.isEmpty()) {
+                        String errorMessage = "Could not move controller service because the following components would be out of scope: ";
+                        errorMessage += String.join(" ", conflictingComponents);
+                        throw new IllegalStateException(errorMessage);
+                    }
                 },
                 null,
                 (revision, controllerServiceEntity) -> {

nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-ui/src/main/webapp/js/nf/canvas/nf-controller-service.js

@@ -1570,7 +1570,7 @@
             var flow = response.processGroupFlow;
             if(nfCommon.isDefinedAndNotNull(flow.breadcrumb.parentBreadcrumb)) {
                 if(flow.breadcrumb.parentBreadcrumb.permissions.canRead
-                    && flow.breadcrumb.parentBreadcrumb.permissions.canWrite){
+                    && flow.breadcrumb.parentBreadcrumb.permissions.canWrite) {
                     processGroups.push({
                         text: flow.breadcrumb.parentBreadcrumb.breadcrumb.name + ' (Parent)',
                         value: flow.breadcrumb.parentBreadcrumb.breadcrumb.id
@@ -1604,7 +1604,7 @@
            },
            handler: {
                click: function () {
-                    nfControllerService.promptToMoveController(serviceTable, controllerServiceEntity);
+                    moveToProcessGroup(serviceTable, controllerServiceEntity, $('#move-controller-service-scope').combo('getSelectedOption').value);
                }
            }
        }, {
@@ -1644,13 +1644,15 @@
             revision: nfClient.getRevision(controllerServiceEntity)
         };
 
-        $.ajax({
+        var request = $.ajax({
             type: 'PUT',
             url: controllerServiceEntity.uri + '/move',
             data: JSON.stringify(controllerServiceRequest),
             dataType: 'json',
             contentType: 'application/json'
-        }).done(function (newControllerServiceEntity) {
+        });
+
+        request.done(function (newControllerServiceEntity) {
            var controllerServicesUri;
            if (nfCommon.isDefinedAndNotNull(controllerServiceEntity.parentGroupId)) {
                controllerServicesUri = config.urls.api + '/flow/process-groups/' + encodeURIComponent(controllerServiceEntity.parentGroupId) + '/controller-services';
@@ -1660,9 +1662,16 @@
 
            // load the controller services grid
            nfControllerServices.loadControllerServices(controllerServicesUri, serviceTable);
+
+           nfDialog.showOkDialog({
+               headerText: 'Controller Service',
+               dialogContent: 'The controller service was moved successfully'
+           });
+
            $('#move-controller-service-dialog').modal('hide');
         });
 
+        request.fail(nfErrorHandler.handleAjaxError);
     };
 
     /**
@@ -2788,26 +2797,6 @@
             });
         },
 
-        /**
-         * Prompts the user before attempting to move the specified controller service.
-         *
-         * @param {jQuery} serviceTable
-         * @param {object} controllerServiceEntity
-         */
-        promptToMoveController: function (serviceTable, controllerServiceEntity) {
-            // prompt for move
-            nfDialog.showYesNoDialog({
-                headerText: 'Move Controller Service',
-                dialogContent: 'Move controller service \'' + nfCommon.escapeHtml(controllerServiceEntity.component.name) + '\' '
-                                + 'to \'' + nfCommon.escapeHtml($('#move-controller-service-scope').combo('getSelectedOption').text) + '\'?'
-                                + '<br /><br /><i class="invalid"></i> &nbsp'
-                                + 'Any reference to this controller service by processors outside the new scope will be removed.',
-                yesHandler: function () {
-                    moveToProcessGroup(serviceTable, controllerServiceEntity, $('#move-controller-service-scope').combo('getSelectedOption').value);
-                }
-            });
-        },
-
         /**
          * Prompts the user before attempting to delete the specified controller service.
          *

nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-ui/src/main/webapp/js/nf/canvas/nf-controller-services.js

@@ -946,12 +946,13 @@
                 if (canRead) {
                     if (canWrite && isDisabled) {
                         markup += '<div class="pointer edit-controller-service fa fa-gear" title="Configure"></div>';
-                        if (canWriteControllerServiceParent(dataContext)) {
-                            markup += '<div class="pointer move-controller-service fa  fa-arrows" title="Move to Parent/Child"></div>';
-                        }
                     } else {
                         markup += '<div class="pointer view-controller-service fa fa-gear" title="View Configuration"></div>';
                     }
+
+                    if (canWrite && canWriteControllerServiceParent(dataContext)) {
+                        markup += '<div class="pointer move-controller-service fa  fa-arrows" title="Move to Parent/Child"></div>';
+                    }
                 }
 
                 if (canOperate) {