Github user olegz commented on a diff in the pull request: https://github.com/apache/nifi/pull/1156#discussion_r86161443 --- Diff: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core-api/src/main/java/org/apache/nifi/controller/AbstractConfiguredComponent.java --- @@ -90,47 +107,80 @@ public void setAnnotationData(final String data) { } @Override - public void setProperty(final String name, final String value) { - if (null == name || null == value) { - throw new IllegalArgumentException(); + public void setProperties(Map<String, String> properties) { + if (properties == null) { + return; } lock.lock(); try { verifyModifiable(); - try (final NarCloseable narCloseable = NarCloseable.withComponentNarLoader(component.getClass())) { - final PropertyDescriptor descriptor = component.getPropertyDescriptor(name); - - final String oldValue = properties.put(descriptor, value); - if (!value.equals(oldValue)) { - - if (descriptor.getControllerServiceDefinition() != null) { - if (oldValue != null) { - final ControllerServiceNode oldNode = serviceProvider.getControllerServiceNode(oldValue); - if (oldNode != null) { - oldNode.removeReference(this); - } - } - - final ControllerServiceNode newNode = serviceProvider.getControllerServiceNode(value); - if (newNode != null) { - newNode.addReference(this); + try (final NarCloseable narCloseable = NarCloseable.withComponentNarLoader(component.getClass(), id)) { + final Set<String> modulePaths = new LinkedHashSet<>(); + for (final Map.Entry<String, String> entry : properties.entrySet()) { + if (entry.getKey() != null && entry.getValue() == null) { + removeProperty(entry.getKey()); + } else if (entry.getKey() != null) { + setProperty(entry.getKey(), entry.getValue()); + + // for any properties that dynamically modify the classpath, attempt to evaluate them for expression language + final PropertyDescriptor descriptor = component.getPropertyDescriptor(entry.getKey()); + if (descriptor.isDynamicClasspathModifier() && !StringUtils.isEmpty(entry.getValue())) { + final StandardPropertyValue propertyValue = new StandardPropertyValue(entry.getValue(), null, variableRegistry); + modulePaths.add(propertyValue.evaluateAttributeExpressions().getValue()); } } + } - try { - component.onPropertyModified(descriptor, oldValue, value); - } catch (final Exception e) { - // nothing really to do here... - } + final boolean requiresInstanceClassLoading = ExtensionManager.requiresInstanceClassLoading(component.getClass().getTypeName()); + if (requiresInstanceClassLoading) { + processClasspathModifiers(modulePaths); + } else if (!requiresInstanceClassLoading && modulePaths.size() > 0) { + // Component has property descriptors with dynamicallyModifiesClasspath set to true, but the class does not have @RequiresInstanceClassLoading + logger.warn("One or more property descriptors intend to modify the classpath, " + + "but component does not contain the {} annotation, classpath will not be modified", + new Object[] {RequiresInstanceClassLoading.class.getName()}); } } } finally { lock.unlock(); } } + // Keep setProperty/removeProperty private so that all calls go through setProperties + private void setProperty(final String name, final String value) { + if (null == name || null == value) { + throw new IllegalArgumentException(); + } --- End diff -- Hmm. . . No message in the exception? Also, should we even check for nulls here given that it's private? meaning we have full control of when/how/where it is invoked?
--- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---