[SPARK-11093] [CORE] ChildFirstURLClassLoader#getResources should return all found resources, not just those in the child classloader

Author: Adam Lewandowski <alewandowski@ipcoop.com>

Closes #9106 from alewando/childFirstFix.

2 files changed, 44 insertions, 9 deletions

diff --git a/core/src/main/scala/org/apache/spark/util/MutableURLClassLoader.scala b/core/src/main/scala/org/apache/spark/util/MutableURLClassLoader.scala
index a1c33212cd..945217203b 100644
--- a/core/src/main/scala/org/apache/spark/util/MutableURLClassLoader.scala
+++ b/core/src/main/scala/org/apache/spark/util/MutableURLClassLoader.scala
@@ -21,6 +21,8 @@ import java.net.{URLClassLoader, URL}
 import java.util.Enumeration
 import java.util.concurrent.ConcurrentHashMap
 
+import scala.collection.JavaConverters._
+
 /**
  * URL class loader that exposes the `addURL` and `getURLs` methods in URLClassLoader.
  */
@@ -82,14 +84,9 @@ private[spark] class ChildFirstURLClassLoader(urls: Array[URL], parent: ClassLoa
   }
 
   override def getResources(name: String): Enumeration[URL] = {
-    val urls = super.findResources(name)
-    val res =
-      if (urls != null && urls.hasMoreElements()) {
-        urls
-      } else {
-        parentClassLoader.getResources(name)
-      }
-    res
+    val childUrls = super.findResources(name).asScala
+    val parentUrls = parentClassLoader.getResources(name).asScala
+    (childUrls ++ parentUrls).asJavaEnumeration
   }
 
   override def addURL(url: URL) {
diff --git a/core/src/test/scala/org/apache/spark/util/MutableURLClassLoaderSuite.scala b/core/src/test/scala/org/apache/spark/util/MutableURLClassLoaderSuite.scala
index d3d464e84f..8b53d4f14a 100644
--- a/core/src/test/scala/org/apache/spark/util/MutableURLClassLoaderSuite.scala
+++ b/core/src/test/scala/org/apache/spark/util/MutableURLClassLoaderSuite.scala
@@ -19,9 +19,14 @@ package org.apache.spark.util
 
 import java.net.URLClassLoader
 
+import scala.collection.JavaConverters._
+
+import org.scalatest.Matchers
+import org.scalatest.Matchers._
+
 import org.apache.spark.{SparkContext, SparkException, SparkFunSuite, TestUtils}
 
-class MutableURLClassLoaderSuite extends SparkFunSuite {
+class MutableURLClassLoaderSuite extends SparkFunSuite with Matchers {
 
   val urls2 = List(TestUtils.createJarWithClasses(
       classNames = Seq("FakeClass1", "FakeClass2", "FakeClass3"),
@@ -32,6 +37,12 @@ class MutableURLClassLoaderSuite extends SparkFunSuite {
       toStringValue = "1",
       classpathUrls = urls2)).toArray
 
+  val fileUrlsChild = List(TestUtils.createJarWithFiles(Map(
+    "resource1" -> "resource1Contents-child",
+    "resource2" -> "resource2Contents"))).toArray
+  val fileUrlsParent = List(TestUtils.createJarWithFiles(Map(
+    "resource1" -> "resource1Contents-parent"))).toArray
+
   test("child first") {
     val parentLoader = new URLClassLoader(urls2, null)
     val classLoader = new ChildFirstURLClassLoader(urls, parentLoader)
@@ -68,6 +79,33 @@ class MutableURLClassLoaderSuite extends SparkFunSuite {
     }
   }
 
+  test("default JDK classloader get resources") {
+    val parentLoader = new URLClassLoader(fileUrlsParent, null)
+    val classLoader = new URLClassLoader(fileUrlsChild, parentLoader)
+    assert(classLoader.getResources("resource1").asScala.size === 2)
+    assert(classLoader.getResources("resource2").asScala.size === 1)
+  }
+
+  test("parent first get resources") {
+    val parentLoader = new URLClassLoader(fileUrlsParent, null)
+    val classLoader = new MutableURLClassLoader(fileUrlsChild, parentLoader)
+    assert(classLoader.getResources("resource1").asScala.size === 2)
+    assert(classLoader.getResources("resource2").asScala.size === 1)
+  }
+
+  test("child first get resources") {
+    val parentLoader = new URLClassLoader(fileUrlsParent, null)
+    val classLoader = new ChildFirstURLClassLoader(fileUrlsChild, parentLoader)
+
+    val res1 = classLoader.getResources("resource1").asScala.toList
+    assert(res1.size === 2)
+    assert(classLoader.getResources("resource2").asScala.size === 1)
+
+    res1.map(scala.io.Source.fromURL(_).mkString) should contain inOrderOnly
+      ("resource1Contents-child", "resource1Contents-parent")
+  }
+
+
   test("driver sets context class loader in local mode") {
     // Test the case where the driver program sets a context classloader and then runs a job
     // in local mode. This is what happens when ./spark-submit is called with "local" as the