TEZ-4396 Ensure utility classes have only private (default) construct…

…ors + several code refactors (#197) (Gergely Hanko reviewed by Laszlo Bodor)
apache · Nov 26, 2022 · 2fd7df4 · 2fd7df4
1 parent 65f9ee3
commit 2fd7df4
Show file tree

Hide file tree

Showing 82 changed files with 865 additions and 1,124 deletions.
diff --git a/tez-api/src/main/java/org/apache/tez/client/TezClientUtils.java b/tez-api/src/main/java/org/apache/tez/client/TezClientUtils.java
@@ -116,11 +116,13 @@
 import com.google.common.collect.Lists;
 
 @Private
-public class TezClientUtils {
+public final class TezClientUtils {
 
   private static Logger LOG = LoggerFactory.getLogger(TezClientUtils.class);
   private static final int UTF8_CHUNK_SIZE = 16 * 1024;
 
+  private TezClientUtils() {}
+
   private static FileStatus[] getLRFileStatus(String fileName, Configuration conf) throws
       IOException {
     URI uri;

diff --git a/tez-api/src/main/java/org/apache/tez/common/ATSConstants.java b/tez-api/src/main/java/org/apache/tez/common/ATSConstants.java
@@ -138,4 +138,5 @@ public class ATSConstants {
   public static final String CALLER_TYPE = "callerType";
   public static final String DESCRIPTION = "description";
 
+  protected ATSConstants() {}
 }
diff --git a/tez-api/src/main/java/org/apache/tez/common/RPCUtil.java b/tez-api/src/main/java/org/apache/tez/common/RPCUtil.java
@@ -29,7 +29,9 @@
 
 import com.google.protobuf.ServiceException;
 
-public class RPCUtil {
+public final class RPCUtil {
+
+  private RPCUtil() {}
 
   /**
    * Returns an instance of {@link TezException}
@@ -55,17 +57,8 @@ private static <T extends Throwable> T instantiateException(
       return ex;
       // RemoteException contains useful information as against the
       // java.lang.reflect exceptions.
-    } catch (NoSuchMethodException e) {
-      throw re;
-    } catch (IllegalArgumentException e) {
-      throw re;
-    } catch (SecurityException e) {
-      throw re;
-    } catch (InstantiationException e) {
-      throw re;
-    } catch (IllegalAccessException e) {
-      throw re;
-    } catch (InvocationTargetException e) {
+    } catch (NoSuchMethodException | IllegalArgumentException | SecurityException | InstantiationException
+            | IllegalAccessException | InvocationTargetException e) {
       throw re;
     }
   }
@@ -85,12 +78,6 @@ private static <T extends RuntimeException> T instantiateRuntimeException(
     return instantiateException(cls, re);
   }
 
-  private static <T extends SessionNotRunning> T instantiateSessionNotRunningException(
-      Class<? extends T> cls, RemoteException re) throws RemoteException {
-    return instantiateException(cls, re);
-  }
-
-
   /**
    * Utility method that unwraps and returns appropriate exceptions.
    *
@@ -109,7 +96,7 @@ public static Void unwrapAndThrowException(ServiceException se)
     } else {
       if (cause instanceof RemoteException) {
         RemoteException re = (RemoteException) cause;
-        Class<?> realClass = null;
+        Class<?> realClass;
         try {
           realClass = Class.forName(re.getClassName());
         } catch (ClassNotFoundException cnf) {

diff --git a/tez-api/src/main/java/org/apache/tez/common/ReflectionUtils.java b/tez-api/src/main/java/org/apache/tez/common/ReflectionUtils.java
@@ -29,9 +29,11 @@
 import org.apache.tez.dag.api.TezReflectionException;
 
 @Private
-public class ReflectionUtils {
+public final class ReflectionUtils {
 
-  private static final Map<String, Class<?>> CLAZZ_CACHE = new ConcurrentHashMap<String, Class<?>>();
+  private static final Map<String, Class<?>> CLAZZ_CACHE = new ConcurrentHashMap<>();
+
+  private ReflectionUtils() {}
 
   @Private
   public static Class<?> getClazz(String className) throws TezReflectionException {

diff --git a/tez-api/src/main/java/org/apache/tez/common/TezCommonUtils.java b/tez-api/src/main/java/org/apache/tez/common/TezCommonUtils.java
@@ -55,7 +55,7 @@
 import com.google.protobuf.ByteString;
 
 @Private
-public class TezCommonUtils {
+public final class TezCommonUtils {
   public static final FsPermission TEZ_AM_DIR_PERMISSION = FsPermission
       .createImmutable((short) 0700); // rwx--------
   public static final FsPermission TEZ_AM_FILE_PERMISSION = FsPermission
@@ -64,6 +64,8 @@ public class TezCommonUtils {
 
   public static final String TEZ_SYSTEM_SUB_DIR = ".tez";
 
+  private TezCommonUtils() {}
+
   /**
    * <p>
    * This function returns the staging directory defined in the config with
@@ -222,7 +224,6 @@ public static Path getTezTextPlanStagingPath(Path tezSysStagingPath, String strA
    * @param conf
    *          Tez configuration
    * @return App recovery path
-   * @throws IOException
    */
   @Private
   public static Path getRecoveryPath(Path tezSysStagingPath, Configuration conf)
@@ -288,7 +289,6 @@ public static Path getSummaryRecoveryPath(Path attemptRecoverPath) {
    *          Filesystem
    * @param dir
    *          directory to be created
-   * @throws IOException
    */
   public static void mkDirForAM(FileSystem fs, Path dir) throws IOException {
     FsPermission perm = new FsPermission(TEZ_AM_DIR_PERMISSION);
@@ -312,7 +312,6 @@ public static void mkDirForAM(FileSystem fs, Path dir) throws IOException {
    * @param filePath
    *          file path to create the file
    * @return FSDataOutputStream
-   * @throws IOException
    */
   public static FSDataOutputStream createFileForAM(FileSystem fs, Path filePath) throws IOException {
     return FileSystem.create(fs, filePath, new FsPermission(TEZ_AM_FILE_PERMISSION));
@@ -417,7 +416,7 @@ public static String getCredentialsInfo(Credentials credentials, String identifi
     }
 
     StringBuilder sb = new StringBuilder();
-    sb.append("Credentials: #" + identifier + "Tokens=").append(credentials.numberOfTokens());
+    sb.append("Credentials: #").append(identifier).append("Tokens=").append(credentials.numberOfTokens());
     if (credentials.numberOfTokens() > 0) {
       sb.append(", Services=");
       sb.append(credentials.getAllTokens().stream()
@@ -435,16 +434,14 @@ public static ByteBuffer convertJobTokenToBytes(
       Token<JobTokenIdentifier> jobToken) throws IOException {
     DataOutputBuffer dob = new DataOutputBuffer();
     jobToken.write(dob);
-    ByteBuffer bb = ByteBuffer.wrap(dob.getData(), 0, dob.getLength());
-    return bb;
+    return ByteBuffer.wrap(dob.getData(), 0, dob.getLength());
   }
 
   public static Credentials parseCredentialsBytes(byte[] credentialsBytes) throws IOException {
     Credentials credentials = new Credentials();
     DataInputBuffer dib = new DataInputBuffer();
     try {
-      byte[] tokenBytes = credentialsBytes;
-      dib.reset(tokenBytes, tokenBytes.length);
+      dib.reset(credentialsBytes, credentialsBytes.length);
       credentials.readTokenStorageStream(dib);
       return credentials;
     } finally {
@@ -459,7 +456,7 @@ public static void logCredentials(Logger log, Credentials credentials, String id
   }
 
   public static Collection<String> tokenizeString(String str, String delim) {
-    List<String> values = new ArrayList<String>();
+    List<String> values = new ArrayList<>();
     if (str == null || str.isEmpty())
       return values;
     StringTokenizer tokenizer = new StringTokenizer(str, delim);
@@ -533,7 +530,7 @@ public static long getAMClientHeartBeatTimeoutMillis(Configuration conf) {
     if (val > 0 && val < TezConstants.TEZ_AM_CLIENT_HEARTBEAT_TIMEOUT_SECS_MINIMUM) {
       return TezConstants.TEZ_AM_CLIENT_HEARTBEAT_TIMEOUT_SECS_MINIMUM * 1000;
     }
-    return val * 1000;
+    return val * 1000L;
   }
 
   /**
@@ -570,7 +567,7 @@ public static long getDAGSessionTimeout(Configuration conf) {
     if (timeoutSecs == 0) {
       timeoutSecs = 1;
     }
-    return 1000l * timeoutSecs;
+    return 1000L * timeoutSecs;
   }
 
   public static int getJavaVersion() {

diff --git a/tez-api/src/main/java/org/apache/tez/common/TezUtils.java b/tez-api/src/main/java/org/apache/tez/common/TezUtils.java
@@ -21,7 +21,6 @@
 import java.io.IOException;
 import java.io.OutputStream;
 import java.nio.ByteBuffer;
-import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
@@ -49,10 +48,12 @@
  * {@link org.apache.hadoop.conf.Configuration} to {@link org.apache.tez.dag.api.UserPayload} etc.
  */
 @InterfaceAudience.Public
-public class TezUtils {
+public final class TezUtils {
 
   private static final Logger LOG = LoggerFactory.getLogger(TezUtils.class);
 
+  private TezUtils() {}
+
   /**
    * Allows changing the log level for task / AM logging. </p>
    *
@@ -73,18 +74,12 @@ public static void addLog4jSystemProperties(String logLevel,
    * @param conf
    *          : Configuration to be converted
    * @return PB ByteString (compressed)
-   * @throws java.io.IOException
    */
   public static ByteString createByteStringFromConf(Configuration conf) throws IOException {
     Objects.requireNonNull(conf, "Configuration must be specified");
     ByteString.Output os = ByteString.newOutput();
-    SnappyOutputStream compressOs = new SnappyOutputStream(os);
-    try {
+    try (SnappyOutputStream compressOs = new SnappyOutputStream(os)) {
       writeConfInPB(compressOs, conf);
-    } finally {
-      if (compressOs != null) {
-        compressOs.close();
-      }
     }
     return os.toByteString();
   }
@@ -95,7 +90,6 @@ public static ByteString createByteStringFromConf(Configuration conf) throws IOE
    *
    * @param conf configuration to be converted
    * @return an instance of {@link org.apache.tez.dag.api.UserPayload}
-   * @throws java.io.IOException
    */
   public static UserPayload createUserPayloadFromConf(Configuration conf) throws IOException {
     return UserPayload.create(ByteBuffer.wrap(createByteStringFromConf(conf).toByteArray()));
@@ -113,11 +107,10 @@ private static DAGProtos.ConfigurationProto createConfProto(SnappyInputStream un
    * @param byteString byteString representation of the conf created using {@link
    *                   #createByteStringFromConf(org.apache.hadoop.conf.Configuration)}
    * @return Configuration
-   * @throws java.io.IOException
    */
   public static Configuration createConfFromByteString(ByteString byteString) throws IOException {
     Objects.requireNonNull(byteString, "ByteString must be specified");
-    try(SnappyInputStream uncompressIs = new SnappyInputStream(byteString.newInput());) {
+    try(SnappyInputStream uncompressIs = new SnappyInputStream(byteString.newInput())) {
       DAGProtos.ConfigurationProto confProto = createConfProto(uncompressIs);
       Configuration conf = new Configuration(false);
       readConfFromPB(confProto, conf);
@@ -156,7 +149,6 @@ public static void addToConfFromByteString(Configuration configuration, ByteStri
    * @param payload {@link org.apache.tez.dag.api.UserPayload} created using {@link
    *                #createUserPayloadFromConf(org.apache.hadoop.conf.Configuration)}
    * @return Configuration
-   * @throws java.io.IOException
    */
   public static Configuration createConfFromUserPayload(UserPayload payload) throws IOException {
     return createConfFromByteString(ByteString.copyFrom(payload.getPayload()));
@@ -186,12 +178,10 @@ public static String convertToHistoryText(String description, Configuration conf
       }
       if (conf != null) {
         JSONObject confJson = new JSONObject();
-        Iterator<Entry<String, String>> iter = conf.iterator();
-        while (iter.hasNext()) {
-          Entry<String, String> entry = iter.next();
+        for (Entry<String, String> entry : conf) {
           String key = entry.getKey();
           String val = conf.get(entry.getKey());
-          if(val != null) {
+          if (val != null) {
             confJson.put(key, val);
           } else {
             LOG.debug("null value in Configuration after replacement for key={}. Skipping.", key);

diff --git a/tez-api/src/main/java/org/apache/tez/common/TezYARNUtils.java b/tez-api/src/main/java/org/apache/tez/common/TezYARNUtils.java
@@ -35,7 +35,7 @@
 import org.apache.tez.dag.api.TezConstants;
 
 @Private
-public class TezYARNUtils {
+public final class TezYARNUtils {
   private static Logger LOG = LoggerFactory.getLogger(TezYARNUtils.class);
 
   public static final String ENV_NAME_REGEX = "[A-Za-z_][A-Za-z0-9_]*";
@@ -49,6 +49,8 @@ public class TezYARNUtils {
       + "([^,]*)"                             // val group
   );
 
+  private TezYARNUtils() {}
+
   public static String getFrameworkClasspath(Configuration conf, boolean usingArchive) {
     StringBuilder classpathBuilder = new StringBuilder();
     boolean userClassesTakesPrecedence =
@@ -126,9 +128,11 @@ private static void addUserSpecifiedClasspath(StringBuilder classpathBuilder,
 
     // Add PWD:PWD/*
     classpathBuilder.append(Environment.PWD.$())
-        .append(File.pathSeparator)
-        .append(Environment.PWD.$() + File.separator + "*")
-        .append(File.pathSeparator);
+            .append(File.pathSeparator)
+            .append(Environment.PWD.$())
+            .append(File.separator)
+            .append("*")
+            .append(File.pathSeparator);
   }
 
   public static void appendToEnvFromInputString(Map<String, String> env,
@@ -161,7 +165,7 @@ public static void appendToEnvFromInputString(Map<String, String> env,
   public static void setEnvIfAbsentFromInputString(Map<String, String> env,
       String envString) {
     if (envString != null && envString.length() > 0) {
-      String childEnvs[] = envString.split(",");
+      String[] childEnvs = envString.split(",");
       for (String cEnv : childEnvs) {
         String[] parts = cEnv.split("="); // split on '='
         Matcher m = VAR_SUBBER .matcher(parts[1]);

diff --git a/tez-api/src/main/java/org/apache/tez/common/security/Master.java b/tez-api/src/main/java/org/apache/tez/common/security/Master.java
@@ -29,12 +29,14 @@
 
 @Private
 @Unstable
-public class Master {
+public final class Master {
 
   public enum State {
-    INITIALIZING, RUNNING;
+    INITIALIZING, RUNNING
   }
 
+  private Master() {}
+
   public static String getMasterUserName(Configuration conf) {
     return conf.get(YarnConfiguration.RM_PRINCIPAL);
   }