[
https://issues.apache.org/jira/browse/NIFI-2909?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15630037#comment-15630037
]
ASF GitHub Bot commented on NIFI-2909:
--------------------------------------
Github user bbende commented on a diff in the pull request:
https://github.com/apache/nifi/pull/1156#discussion_r86216168
--- Diff:
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core-api/src/main/java/org/apache/nifi/controller/AbstractConfiguredComponent.java
---
@@ -141,48 +191,64 @@ public void setProperty(final String name, final
String value) {
* @return true if removed; false otherwise
* @throws java.lang.IllegalArgumentException if the name is null
*/
- @Override
- public boolean removeProperty(final String name) {
+ private boolean removeProperty(final String name) {
if (null == name) {
throw new IllegalArgumentException();
}
- lock.lock();
- try {
- verifyModifiable();
-
- try (final NarCloseable narCloseable =
NarCloseable.withComponentNarLoader(component.getClass())) {
- final PropertyDescriptor descriptor =
component.getPropertyDescriptor(name);
- String value = null;
- if (!descriptor.isRequired() && (value =
properties.remove(descriptor)) != null) {
-
- if (descriptor.getControllerServiceDefinition() !=
null) {
- if (value != null) {
- final ControllerServiceNode oldNode =
serviceProvider.getControllerServiceNode(value);
- if (oldNode != null) {
- oldNode.removeReference(this);
- }
- }
- }
+ final PropertyDescriptor descriptor =
component.getPropertyDescriptor(name);
+ String value = null;
+ if (!descriptor.isRequired() && (value =
properties.remove(descriptor)) != null) {
- try {
- component.onPropertyModified(descriptor, value,
null);
- } catch (final Exception e) {
- // nothing really to do here...
+ if (descriptor.getControllerServiceDefinition() != null) {
+ if (value != null) {
+ final ControllerServiceNode oldNode =
serviceProvider.getControllerServiceNode(value);
+ if (oldNode != null) {
+ oldNode.removeReference(this);
}
-
- return true;
}
}
- } finally {
- lock.unlock();
+
+ try {
+ component.onPropertyModified(descriptor, value, null);
+ } catch (final Exception e) {
+ // nothing really to do here...
+ }
+
+ return true;
}
+
return false;
}
+ /**
+ * Adds all of the modules identified by the given module paths to the
InstanceClassLoader for this component.
+ *
+ * @param modulePaths a list of module paths where each entry can be a
comma-separated list of multiple module paths
+ */
+ private void processClasspathModifiers(final Set<String> modulePaths) {
+ try {
+ final URL[] urls =
ClassLoaderUtils.getURLsForClasspath(modulePaths, null, true);
+
+ final ClassLoader classLoader =
Thread.currentThread().getContextClassLoader();
+ if (!(classLoader instanceof InstanceClassLoader)) {
+ // Really shouldn't happen, but if we somehow got here and
don't have an InstanceClassLoader then log a warning and move on
+ logger.warn("Unable to modify the classpath for {},
expected InstanceClassLoader, but found {}",
+ new Object[] { name,
classLoader.getClass().getName()});
+ return;
--- End diff --
I think there are two different scenarios to consider, and the important
thing to keep in mind is that this getting called from setProperties which is
when you hit Apply from the Configure screen of a processor, which it has to be
done this way because the classpath needs to be modified before any validation
calls which might rely on the classpath...
The logging statement above would indicate a bug in the code where somehow
`setProperties` got called and the framework didn't correctly set the context
ClassLoader prior to calling it. We could throw an exception here, but really
that doesn't do anything more than logger.warn/logger.error which both generate
a bulletin in the UI telling the user the classpath wasn't modified, at which
point they could choose not to start the processor.
The second scenario is when ClassLoaderUtils.getURLsForClasspath(...) is
called... right now it skips over resources it can't resolve. My thinking here
was that in the general case we should be more lenient and not blow up just
because 1 out of possibly many resources can't be found. If someone absolutely
needs every resource loaded, I believe they should implement a custom validator
for their property that could call getURLsForClasspath with suppression set to
false, so that it fails validation if it can't find something.
What do you think?
> Provide a framework mechanism for loading additional classpath resources
> ------------------------------------------------------------------------
>
> Key: NIFI-2909
> URL: https://issues.apache.org/jira/browse/NIFI-2909
> Project: Apache NiFi
> Issue Type: Improvement
> Reporter: Bryan Bende
> Assignee: Bryan Bende
> Fix For: 1.1.0
>
>
> We currently have several components with a property for specifying
> additional classpath resources (DBCP connection pool, scripting processors,
> JMS). Each of these components is responsible for handling this in its own
> way.
> The framework should provide a more integrated solution to make it easier for
> component developers to deal with this scenario. Some requirements that need
> to be met by this solution:
> - Multiple instances of the same component with different resources added to
> the classpath and not interfering with each other (i.e. two DBCP connection
> pools using different drivers)
> - Ability to modify the actual ClassLoader of the component to deal with
> frameworks that use Class.forName() without passing in a ClassLoader, meaning
> if a processor loads class A and class A calls Class.forName(classBName),
> then class B needs to be available in the ClassLoader that loaded the
> processor's class which in turn loaded class A
> - A component developer should be able to indicate that a given
> PropertyDescriptor represents a classpath resource and the framework should
> take care of the ClassLoader manipulation
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
