HADOOP-17009: Embrace Immutability of Java Collections

belugabehr · web-flow · commit 100ec8e8709e · 2020-06-19T10:23:43.000-07:00
diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/AbstractFileSystem.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/AbstractFileSystem.java
@@ -23,7 +23,6 @@
 import java.lang.reflect.InvocationTargetException;
 import java.net.URI;
 import java.net.URISyntaxException;
-import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.EnumSet;
@@ -1032,7 +1031,7 @@ public String getCanonicalServiceName() {
    */
   @InterfaceAudience.LimitedPrivate( { "HDFS", "MapReduce" })
   public List<Token<?>> getDelegationTokens(String renewer) throws IOException {
-    return new ArrayList<Token<?>>(0);
+    return Collections.emptyList();
   }
 
   /**
diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/Stat.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/Stat.java
@@ -20,8 +20,7 @@
 import java.io.BufferedReader;
 import java.io.FileNotFoundException;
 import java.io.IOException;
-import java.util.HashMap;
-import java.util.Map;
+import java.util.Collections;
 import java.util.NoSuchElementException;
 import java.util.StringTokenizer;
 
@@ -65,9 +64,7 @@ public Stat(Path path, long blockSize, boolean deref, FileSystem fs)
     this.blockSize = blockSize;
     this.dereference = deref;
     // LANG = C setting
-    Map<String, String> env = new HashMap<String, String>();
-    env.put("LANG", "C");
-    setEnvironment(env);
+    setEnvironment(Collections.singletonMap("LANG", "C"));
   }
 
   public FileStatus getFileStatus() throws IOException {
diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/Delete.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/Delete.java
@@ -20,6 +20,7 @@
 
 import java.io.FileNotFoundException;
 import java.io.IOException;
+import java.util.Collections;
 import java.util.LinkedList;
 import java.util.List;
 
@@ -97,7 +98,7 @@ protected List<PathData> expandArgument(String arg) throws IOException {
           throw e;
         }
         // prevent -f on a non-existent glob from failing
-        return new LinkedList<PathData>();
+        return Collections.emptyList();
       }
     }
 
diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ha/HAAdmin.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ha/HAAdmin.java
@@ -19,9 +19,9 @@
 
 import java.io.IOException;
 import java.io.PrintStream;
-import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.Map;
 
 import org.apache.commons.cli.Options;
@@ -107,8 +107,7 @@ protected HAAdmin(Configuration conf) {
   protected abstract HAServiceTarget resolveTarget(String string);
   
   protected Collection<String> getTargetIds(String targetNodeToActivate) {
-    return new ArrayList<String>(
-        Arrays.asList(new String[]{targetNodeToActivate}));
+    return Collections.singleton(targetNodeToActivate);
   }
 
   protected String getUsageString() {
@@ -188,8 +187,10 @@ private int transitionToActive(final CommandLine cmd)
   private boolean isOtherTargetNodeActive(String targetNodeToActivate, boolean forceActive)
       throws IOException  {
     Collection<String> targetIds = getTargetIds(targetNodeToActivate);
-    targetIds.remove(targetNodeToActivate);
-    for(String targetId : targetIds) {
+    for (String targetId : targetIds) {
+      if (targetNodeToActivate.equals(targetId)) {
+        continue;
+      }
       HAServiceTarget target = resolveTarget(targetId);
       if (!checkManualStateManagementOK(target)) {
         return true;
diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/lib/StaticUserWebFilter.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/lib/StaticUserWebFilter.java
@@ -19,7 +19,7 @@
 
 import java.io.IOException;
 import java.security.Principal;
-import java.util.HashMap;
+import java.util.Collections;
 
 import javax.servlet.FilterChain;
 import javax.servlet.FilterConfig;
@@ -121,14 +121,10 @@ public void init(FilterConfig conf) throws ServletException {
 
   @Override
   public void initFilter(FilterContainer container, Configuration conf) {
-    HashMap<String, String> options = new HashMap<String, String>();
-    
     String username = getUsernameFromConf(conf);
-    options.put(HADOOP_HTTP_STATIC_USER, username);
 
-    container.addFilter("static_user_filter", 
-                        StaticUserFilter.class.getName(), 
-                        options);
+    container.addFilter("static_user_filter", StaticUserFilter.class.getName(),
+        Collections.singletonMap(HADOOP_HTTP_STATIC_USER, username));
   }
 
   /**
diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/metrics2/util/MBeans.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/metrics2/util/MBeans.java
@@ -18,7 +18,7 @@
 package org.apache.hadoop.metrics2.util;
 
 import java.lang.management.ManagementFactory;
-import java.util.HashMap;
+import java.util.Collections;
 import java.util.Map;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
@@ -70,8 +70,7 @@ private MBeans() {
    */
   static public ObjectName register(String serviceName, String nameName,
                                     Object theMbean) {
-    return register(serviceName, nameName, new HashMap<String, String>(),
-        theMbean);
+    return register(serviceName, nameName, Collections.emptyMap(), theMbean);
   }
 
   /**
diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/net/CachedDNSToSwitchMapping.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/net/CachedDNSToSwitchMapping.java
@@ -130,8 +130,7 @@ public List<String> resolve(List<String> names) {
    */
   @Override
   public Map<String, String> getSwitchMap() {
-    Map<String, String > switchMap = new HashMap<String, String>(cache);
-    return switchMap;
+    return new HashMap<>(cache);
   }
 
 
diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/net/NetworkTopology.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/net/NetworkTopology.java
@@ -196,10 +196,8 @@ public List<Node> getDatanodesInRack(String loc) {
         loc = loc.substring(1);
       }
       InnerNode rack = (InnerNode) clusterMap.getLoc(loc);
-      if (rack == null) {
-        return null;
-      }
-      return new ArrayList<Node>(rack.getChildren());
+      return (rack == null) ? new ArrayList<>(0)
+          : new ArrayList<>(rack.getChildren());
     } finally {
       netlock.readLock().unlock();
     }
diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/net/TableMapping.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/net/TableMapping.java
@@ -25,6 +25,7 @@
 import java.nio.file.Files;
 import java.nio.file.Paths;
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -130,7 +131,7 @@ public synchronized List<String> resolve(List<String> names) {
         if (map == null) {
           LOG.warn("Failed to read topology table. " +
             NetworkTopology.DEFAULT_RACK + " will be used for all nodes.");
-          map = new HashMap<String, String>();
+          map = Collections.emptyMap();
         }
       }
       List<String> results = new ArrayList<String>(names.size());
diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/CompositeGroupsMapping.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/CompositeGroupsMapping.java
@@ -19,6 +19,7 @@
 
 import java.io.IOException;
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
@@ -69,26 +70,24 @@ public class CompositeGroupsMapping
   public synchronized List<String> getGroups(String user) throws IOException {
     Set<String> groupSet = new TreeSet<String>();
 
-    List<String> groups = null;
     for (GroupMappingServiceProvider provider : providersList) {
+      List<String> groups = Collections.emptyList();
       try {
         groups = provider.getGroups(user);
       } catch (Exception e) {
         LOG.warn("Unable to get groups for user {} via {} because: {}",
             user, provider.getClass().getSimpleName(), e.toString());
         LOG.debug("Stacktrace: ", e);
       }        
-      if (groups != null && ! groups.isEmpty()) {
+      if (!groups.isEmpty()) {
         groupSet.addAll(groups);
         if (!combined) break;
       }
     }
 
-    List<String> results = new ArrayList<String>(groupSet.size());
-    results.addAll(groupSet);
-    return results;
+    return new ArrayList<>(groupSet);
   }
-  
+
   /**
    * Caches groups, no need to do that for this provider
    */
diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/JniBasedUnixGroupsNetgroupMapping.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/JniBasedUnixGroupsNetgroupMapping.java
@@ -20,6 +20,7 @@
 
 import java.io.IOException;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.List;
 import java.util.LinkedList;
 
@@ -125,6 +126,6 @@ protected synchronized List<String> getUsersForNetgroup(String netgroup) {
     if (users != null && users.length != 0) {
       return Arrays.asList(users);
     }
-    return new LinkedList<String>();
+    return Collections.emptyList();
   }
 }
diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/LdapGroupsMapping.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/LdapGroupsMapping.java
@@ -518,11 +518,11 @@ List<String> doGetGroups(String user, int goUpHierarchy)
     if (!results.hasMoreElements()) {
       LOG.debug("doGetGroups({}) returned no groups because the " +
           "user is not found.", user);
-      return new ArrayList<>();
+      return Collections.emptyList();
     }
     SearchResult result = results.nextElement();
 
-    List<String> groups = null;
+    List<String> groups = Collections.emptyList();
     if (useOneQuery) {
       try {
         /**
@@ -548,7 +548,7 @@ List<String> doGetGroups(String user, int goUpHierarchy)
                 "the second LDAP query using the user's DN.", e);
       }
     }
-    if (groups == null || groups.isEmpty() || goUpHierarchy > 0) {
+    if (groups.isEmpty() || goUpHierarchy > 0) {
       groups = lookupGroup(result, c, goUpHierarchy);
     }
     LOG.debug("doGetGroups({}) returned {}", user, groups);
diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/NetgroupCache.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/NetgroupCache.java
@@ -17,9 +17,9 @@
  */
 package org.apache.hadoop.security;
 
+import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashSet;
-import java.util.LinkedList;
 import java.util.List;
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
@@ -61,7 +61,7 @@ public static void getNetgroups(final String user,
    * @return list of cached groups
    */
   public static List<String> getNetgroupNames() {
-    return new LinkedList<String>(getGroups());
+    return new ArrayList<>(getGroups());
   }
 
   private static Set<String> getGroups() {
diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/ShellBasedIdMapping.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/ShellBasedIdMapping.java
@@ -24,6 +24,7 @@
 import java.nio.charset.Charset;
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.regex.Matcher;
@@ -534,7 +535,7 @@ synchronized private void updateMapIncr(final int id,
   static final class PassThroughMap<K> extends HashMap<K, K> {
     
     public PassThroughMap() {
-      this(new HashMap<K, K>());
+      this(Collections.emptyMap());
     }
     
     public PassThroughMap(Map<K, K> mapping) {
diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/token/delegation/web/DelegationTokenAuthenticationHandler.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/token/delegation/web/DelegationTokenAuthenticationHandler.java
@@ -20,9 +20,8 @@
 import java.io.IOException;
 import java.io.Writer;
 import java.text.MessageFormat;
-import java.util.HashMap;
+import java.util.Collections;
 import java.util.HashSet;
-import java.util.LinkedHashMap;
 import java.util.Map;
 import java.util.Properties;
 import java.util.Set;
@@ -301,8 +300,7 @@ public boolean managementOperation(AuthenticationToken token,
                   dt.decodeFromUrlString(tokenToRenew);
                   long expirationTime = tokenManager.renewToken(dt,
                       requestUgi.getShortUserName());
-                  map = new HashMap();
-                  map.put("long", expirationTime);
+                  map = Collections.singletonMap("long", expirationTime);
                 } catch (IOException ex) {
                   throw new AuthenticationException(ex.toString(), ex);
                 }
@@ -358,13 +356,11 @@ public boolean managementOperation(AuthenticationToken token,
 
   @SuppressWarnings("unchecked")
   private static Map delegationTokenToJSON(Token token) throws IOException {
-    Map json = new LinkedHashMap();
-    json.put(
+    Map json = Collections.singletonMap(
         KerberosDelegationTokenAuthenticator.DELEGATION_TOKEN_URL_STRING_JSON,
         token.encodeToUrlString());
-    Map response = new LinkedHashMap();
-    response.put(KerberosDelegationTokenAuthenticator.DELEGATION_TOKEN_JSON,
-        json);
+    Map response = Collections.singletonMap(
+        KerberosDelegationTokenAuthenticator.DELEGATION_TOKEN_JSON, json);
     return response;
   }
 
diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/service/AbstractService.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/service/AbstractService.java
@@ -20,6 +20,7 @@
 
 import java.io.IOException;
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -426,7 +427,7 @@ private void recordLifecycleEvent() {
 
   @Override
   public synchronized List<LifecycleEvent> getLifecycleHistory() {
-    return new ArrayList<LifecycleEvent>(lifecycleHistory);
+    return Collections.unmodifiableList(new ArrayList<>(lifecycleHistory));
   }
 
   /**
@@ -483,8 +484,7 @@ public void removeBlocker(String name) {
   @Override
   public Map<String, String> getBlockers() {
     synchronized (blockerMap) {
-      Map<String, String> map = new HashMap<String, String>(blockerMap);
-      return map;
+      return Collections.unmodifiableMap(new HashMap<>(blockerMap));
     }
   }
 }
diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/service/CompositeService.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/service/CompositeService.java
@@ -19,6 +19,7 @@
 package org.apache.hadoop.service;
 
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.List;
 
 import org.apache.hadoop.classification.InterfaceAudience.Public;
@@ -60,7 +61,7 @@ public CompositeService(String name) {
    */
   public List<Service> getServices() {
     synchronized (serviceList) {
-      return new ArrayList<Service>(serviceList);
+      return Collections.unmodifiableList(new ArrayList<>(serviceList));
     }
   }
 
diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/service/launcher/ServiceLauncher.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/service/launcher/ServiceLauncher.java
@@ -23,6 +23,7 @@
 import java.net.URL;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.List;
 
 import com.google.common.annotations.VisibleForTesting;
@@ -894,7 +895,7 @@ public List<String> extractCommandOptions(Configuration conf,
       List<String> args) {
     int size = args.size();
     if (size <= 1) {
-      return new ArrayList<>(0);
+      return Collections.emptyList();
     }
     List<String> coreArgs = args.subList(1, size);
 
diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/HttpExceptionUtils.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/HttpExceptionUtils.java
diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/Shell.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/Shell.java

Original file line number	Diff line number	Diff line change
`@@ -20,6 +20,7 @@`
`20`	`20`
`21`	`21`	`import java.io.FileNotFoundException;`
`22`	`22`	`import java.io.IOException;`
	`23`	`+import java.util.Collections;`
`23`	`24`	`import java.util.LinkedList;`
`24`	`25`	`import java.util.List;`
`25`	`26`
`@@ -97,7 +98,7 @@ protected List<PathData> expandArgument(String arg) throws IOException {`
`97`	`98`	`throw e;`
`98`	`99`	`}`
`99`	`100`	`// prevent -f on a non-existent glob from failing`
`100`		`- return new LinkedList<PathData>();`
	`101`	`+ return Collections.emptyList();`
`101`	`102`	`}`
`102`	`103`	`}`
`103`	`104`
Original file line number	Diff line number	Diff line change
`@@ -130,8 +130,7 @@ public List<String> resolve(List<String> names) {`
`130`	`130`	`*/`
`131`	`131`	`@Override`
`132`	`132`	`public Map<String, String> getSwitchMap() {`
`133`		`- Map<String, String > switchMap = new HashMap<String, String>(cache);`
`134`		`- return switchMap;`
	`133`	`+ return new HashMap<>(cache);`
`135`	`134`	`}`
`136`	`135`
`137`	`136`
Original file line number	Diff line number	Diff line change
`@@ -196,10 +196,8 @@ public List<Node> getDatanodesInRack(String loc) {`
`196`	`196`	`loc = loc.substring(1);`
`197`	`197`	`}`
`198`	`198`	`InnerNode rack = (InnerNode) clusterMap.getLoc(loc);`
`199`		`- if (rack == null) {`
`200`		`- return null;`
`201`		`- }`
`202`		`- return new ArrayList<Node>(rack.getChildren());`
	`199`	`+ return (rack == null) ? new ArrayList<>(0)`
	`200`	`+ : new ArrayList<>(rack.getChildren());`
`203`	`201`	`} finally {`
`204`	`202`	`netlock.readLock().unlock();`
`205`	`203`	`}`