You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by mattyb149 <gi...@git.apache.org> on 2016/08/22 15:13:13 UTC

[GitHub] nifi pull request #907: NIFI-2619: Added logic to ClassLoaderUtils to trim m...

GitHub user mattyb149 opened a pull request:

    https://github.com/apache/nifi/pull/907

    NIFI-2619: Added logic to ClassLoaderUtils to trim module paths and accept URLs

    

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/mattyb149/nifi NIFI-2619

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/nifi/pull/907.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #907
    
----
commit 663ada18ff39deb261b6923c47f1f422b859e302
Author: Matt Burgess <ma...@apache.org>
Date:   2016-08-22T15:09:14Z

    NIFI-2619: Added unit test showing bugs

commit 623d41affe5e24142a2fd8a48d97e682e9245af7
Author: Matt Burgess <ma...@apache.org>
Date:   2016-08-22T15:11:52Z

    NIFI-2619: Added logic to ClassLoaderUtils to trim module paths and accept URLs

----


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request #907: NIFI-2619: Added logic to ClassLoaderUtils to trim m...

Posted by mattyb149 <gi...@git.apache.org>.
Github user mattyb149 commented on a diff in the pull request:

    https://github.com/apache/nifi/pull/907#discussion_r75730484
  
    --- Diff: nifi-commons/nifi-utils/src/main/java/org/apache/nifi/util/file/classloader/ClassLoaderUtils.java ---
    @@ -21,45 +21,59 @@
     import java.net.MalformedURLException;
     import java.net.URL;
     import java.net.URLClassLoader;
    +import java.util.Arrays;
     import java.util.LinkedList;
     import java.util.List;
    +import java.util.stream.Collectors;
     
     public class ClassLoaderUtils {
     
         public static ClassLoader getCustomClassLoader(String modulePath, ClassLoader parentClassLoader, FilenameFilter filenameFilter) throws MalformedURLException {
    -        String[] modules = modulePath != null? modulePath.split(",") : null;
    -        URL[] classpaths = getURLsForClasspath(modules,filenameFilter);
    -        return createModuleClassLoader(classpaths,parentClassLoader);
    +        // Split and trim the module path(s)
    +        List<String> modules = (modulePath == null)
    +                ? null
    +                : Arrays.stream(modulePath.split(",")).map((path) -> path == null ? null : path.trim()).filter((item) -> item != null).collect(Collectors.toList());
    --- End diff --
    
    Good point. I changed to use yours (replacing some things with method references) and added a unit test around the errors you found.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi issue #907: NIFI-2619: Added logic to ClassLoaderUtils to trim module p...

Posted by YolandaMDavis <gi...@git.apache.org>.
Github user YolandaMDavis commented on the issue:

    https://github.com/apache/nifi/pull/907
  
    @mattyb149 thanks for applying that change.  I tested on standalone node with Jolt Transform processor using a list of customjars (included extra commas, spaces etc) and works as expected with no exceptions thrown.
    
    +1
    
    Will squash and merge shortly


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi issue #907: NIFI-2619: Added logic to ClassLoaderUtils to trim module p...

Posted by YolandaMDavis <gi...@git.apache.org>.
Github user YolandaMDavis commented on the issue:

    https://github.com/apache/nifi/pull/907
  
    @mattyb149 will take a look


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request #907: NIFI-2619: Added logic to ClassLoaderUtils to trim m...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/nifi/pull/907


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request #907: NIFI-2619: Added logic to ClassLoaderUtils to trim m...

Posted by YolandaMDavis <gi...@git.apache.org>.
Github user YolandaMDavis commented on a diff in the pull request:

    https://github.com/apache/nifi/pull/907#discussion_r75727555
  
    --- Diff: nifi-commons/nifi-utils/src/main/java/org/apache/nifi/util/file/classloader/ClassLoaderUtils.java ---
    @@ -21,45 +21,59 @@
     import java.net.MalformedURLException;
     import java.net.URL;
     import java.net.URLClassLoader;
    +import java.util.Arrays;
     import java.util.LinkedList;
     import java.util.List;
    +import java.util.stream.Collectors;
     
     public class ClassLoaderUtils {
     
         public static ClassLoader getCustomClassLoader(String modulePath, ClassLoader parentClassLoader, FilenameFilter filenameFilter) throws MalformedURLException {
    -        String[] modules = modulePath != null? modulePath.split(",") : null;
    -        URL[] classpaths = getURLsForClasspath(modules,filenameFilter);
    -        return createModuleClassLoader(classpaths,parentClassLoader);
    +        // Split and trim the module path(s)
    +        List<String> modules = (modulePath == null)
    +                ? null
    +                : Arrays.stream(modulePath.split(",")).map((path) -> path == null ? null : path.trim()).filter((item) -> item != null).collect(Collectors.toList());
    --- End diff --
    
    Executed tests and realized doesn't quite cover for mistakenly added leading commas or leading/trailing commas with blanks before or after them (respectively) e.g. "/src/folder/one.jar1, /src/folder/two.jar, " . Edge case but I think changing the lambda around to blank values in array are filtered out should prevent error:
    
    //Maybe something like this using StringUtils.isNotBlank ?
    Arrays.stream(modulePath.split(",")).filter((item) -> StringUtils.isNotBlank(item)).map((path) -> path.trim()).collect(Collectors.toList());


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---