Refactor test and apply suggestions

Author: agilelab-tmnd1991

hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/Constants.java

@@ -1363,19 +1363,19 @@ private Constants() {
 
 
   /**
-   * To use the same classloader that loaded S3AFileSystem to al
-   * load the user extensions, such as {{fs.s3a.aws.credentials.provider}}.
+   * To use the same classloader that loaded S3AFileSystem to load
+   * user extensions, such as {{fs.s3a.aws.credentials.provider}}.
    * It is useful to turn this off for Apache Spark applications that
    * might load S3AFileSystem from the Spark distribution (Launcher classloader)
    * while users might want to provide custom extensions (loaded by Spark MutableClassloader).
    * Default value: true.
    */
-  public static final String AWS_S3_EXTENSIONS_ISOLATED_CLASSLOADER =
-      "fs.s3a.extensions.isolated.classloader";
+  public static final String AWS_S3_CLASSLOADER_ISOLATION =
+      "fs.s3a.classloader.isolation";
 
 
   /**
-   * Default value for {{fs.s3a.extensions.isolated.classloader}}.
+   * Default value for {{fs.s3a.classloader.isolation}}.
    */
-  public static final boolean DEFAULT_AWS_S3_EXTENSIONS_ISOLATED_CLASSLOADER = true;
+  public static final boolean DEFAULT_AWS_S3_CLASSLOADER_ISOLATION = true;
 }

hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java

@@ -718,9 +718,17 @@ public void initialize(URI name, Configuration originalConf)
     }
   }
 
-  void isolateClassloader(Configuration conf, ClassLoader classLoader) {
-    if (conf.getBoolean(Constants.AWS_S3_EXTENSIONS_ISOLATED_CLASSLOADER,
-        Constants.DEFAULT_AWS_S3_EXTENSIONS_ISOLATED_CLASSLOADER)) {
+  /**
+   * If classloader isolation is {{true}}
+   * (through {@link Constants#AWS_S3_EXTENSIONS_ISOLATED_CLASSLOADER}) or not
+   * explicitly set, then the classLoader of the input configuration object
+   * will be set to the input classloader, otherwise nothing will happen.
+   * @param conf configuration object.
+   * @param classLoader isolated classLoader.
+   */
+  private void isolateClassloader(Configuration conf, ClassLoader classLoader) {
+    if (conf.getBoolean(Constants.AWS_S3_CLASSLOADER_ISOLATION,
+        Constants.DEFAULT_AWS_S3_CLASSLOADER_ISOLATION)) {
       LOG.debug("Configuration classloader set to S3AFileSystem classloader: {}", classLoader);
       conf.setClassLoader(classLoader);
     } else {

hadoop-tools/hadoop-aws/src/site/markdown/tools/hadoop-aws/index.md

@@ -564,7 +564,8 @@ This AWS credential provider is enabled in S3A by default.
 ## Custom AWS Credential Providers and Apache Spark
 
 Apache Spark employs two class loaders, one that loads "distribution" (Spark + Hadoop) classes and one that
-loads custom user classes. If the user wants to load custom implementations of AWS Credential Providers
+loads custom user classes. If the user wants to load custom implementations of AWS credential providers, 
+custom signers, delegation token providers or any other dynamically loaded extension class
 through user provided jars will need to set the following configuration:
 
 ```xml
@@ -579,7 +580,7 @@ through user provided jars will need to set the following configuration:
 
 ```
 
-If the following property is not set or set to true, the following exception will be thrown:
+If the following property is not set or set to `true`, the following exception will be thrown:
 
 ```
 java.io.IOException: From option fs.s3a.aws.credentials.provider java.lang.ClassNotFoundException: Class CustomCredentialsProvider not found

hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3AFileSystemIsolatedClassloader.java

@@ -0,0 +1,85 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.fs.s3a;
+
+import java.io.IOException;
+import java.lang.reflect.Constructor;
+import java.lang.reflect.InvocationTargetException;
+
+import org.apache.hadoop.conf.Configuration;
+import static org.assertj.core.api.Assertions.*;
+import org.junit.Test;
+
+public class ITestS3AFileSystemIsolatedClassloader extends AbstractS3ATestBase {
+
+  public static class CustomClassLoader extends ClassLoader {
+  }
+
+  private final ClassLoader customClassLoader = new CustomClassLoader();
+
+  private <T> T createClass(ClassLoader clsLoader, Class<T> toLoad) {
+    try {
+      Class<?> cls = clsLoader.loadClass(toLoad.getCanonicalName());
+      Constructor<?> constructor = cls.getConstructor();
+      return (T) constructor.newInstance();
+    } catch(ClassNotFoundException | InstantiationException | IllegalAccessException |
+        IllegalArgumentException | InvocationTargetException | NoSuchMethodException |
+        SecurityException e) {
+      throw new RuntimeException(e);
+    }
+  }
+
+  @Test
+  public void isolatedClassloader() throws IOException {
+    try (S3AFileSystem fs = getFileSystem()) {
+      Configuration conf = createClass(customClassLoader, Configuration.class);
+      assertEquals(fs.getConf().getClassLoader(), customClassLoader);
+      conf.setBoolean(Constants.AWS_S3_CLASSLOADER_ISOLATION, true);
+      assertNotEquals(fs.getClass().getClassLoader(), customClassLoader);
+      fs.initialize(fs.getUri(), conf);
+      assertEquals(fs.getConf().getClassLoader(), fs.getClass().getClassLoader());
+      assertNotEquals(fs.getConf().getClassLoader(), customClassLoader);
+    }
+  }
+
+  @Test
+  public void isolatedClassloaderDefault() throws IOException {
+    try (S3AFileSystem fs = getFileSystem()) {
+      Configuration conf = createClass(customClassLoader, Configuration.class);
+      assertEquals(fs.getConf().getClassLoader(), customClassLoader);
+      assertNotEquals(fs.getClass().getClassLoader(), customClassLoader);
+      fs.initialize(fs.getUri(), conf);
+      assertEquals(fs.getConf().getClassLoader(), fs.getClass().getClassLoader());
+      assertNotEquals(fs.getConf().getClassLoader(), customClassLoader);
+    }
+  }
+
+  @Test
+  public void notIsolatedClassloader() throws IOException {
+    try (S3AFileSystem fs = getFileSystem()) {
+      Configuration conf = createClass(customClassLoader, Configuration.class);
+      assertEquals(fs.getConf().getClassLoader(), customClassLoader);
+      conf.setBoolean(Constants.AWS_S3_CLASSLOADER_ISOLATION, false);
+      assertNotEquals(fs.getClass().getClassLoader(), customClassLoader);
+      fs.initialize(fs.getUri(), conf);
+      assertNotEquals(fs.getConf().getClassLoader(), fs.getClass().getClassLoader());
+      assertEquals(fs.getConf().getClassLoader(), customClassLoader);
+    }
+  }
+}