HADOOP-6396. Fix unhelpful exception message when unable to parse umask (jghoman)

git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/branches/branch-0.21@887474 13f79535-47bb-0310-9956-ffa450edef68

CHANGES.txt

@@ -85,6 +85,9 @@ Trunk (unreleased changes)
     and suggests using -skpTrash, when moving to trash fails.
     (Boris Shkolnik via suresh)
 
+    HADOOP-6396. Fix uninformative exception message when unable to parse
+    umask (jghoman)
+
   NEW FEATURES
 
     HADOOP-4268. Change fsck to use ClientProtocol methods so that the

src/java/org/apache/hadoop/fs/permission/FsPermission.java

@@ -21,8 +21,6 @@ import java.io.DataInput;
 import java.io.DataOutput;
 import java.io.IOException;
 
-import org.apache.commons.logging.Log;
-import org.apache.commons.logging.LogFactory;
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.fs.CommonConfigurationKeys;
 import org.apache.hadoop.io.Writable;
@@ -33,8 +31,6 @@ import org.apache.hadoop.io.WritableFactory;
  * A class for file/directory permissions.
  */
 public class FsPermission implements Writable {
-  private static final Log LOG = LogFactory.getLog(FsPermission.class);
-  
   static final WritableFactory FACTORY = new WritableFactory() {
     public Writable newInstance() { return new FsPermission(); }
   };
@@ -182,7 +178,8 @@ public class FsPermission implements Writable {
         otheraction.and(umask.otheraction.not()));
   }
 
-  /** umask property label Deprecated key may be removed in version .23 */
+  /** umask property label deprecated key and code in getUMask method
+   *  to accommodate it may be removed in version .23 */
   public static final String DEPRECATED_UMASK_LABEL = "dfs.umask"; 
   public static final String UMASK_LABEL = 
                   CommonConfigurationKeys.FS_PERMISSIONS_UMASK_KEY;
@@ -198,10 +195,19 @@ public class FsPermission implements Writable {
     if(conf != null) {
       String confUmask = conf.get(UMASK_LABEL);
       if(confUmask != null) { // UMASK_LABEL is set
-        if(conf.deprecatedKeyWasSet(DEPRECATED_UMASK_LABEL)) 
-          umask = Integer.parseInt(confUmask); // Evaluate as decimal value
-        else
-          umask = new UmaskParser(confUmask).getUMask();
+        try {
+          if(conf.deprecatedKeyWasSet(DEPRECATED_UMASK_LABEL)) 
+            umask = Integer.parseInt(confUmask); // Evaluate as decimal value
+          else
+            umask = new UmaskParser(confUmask).getUMask();
+        } catch(IllegalArgumentException iae) {
+          // Provide more explanation for user-facing message
+          String type = iae instanceof NumberFormatException ? "decimal" 
+                                                          : "octal or symbolic";
+          
+          throw new IllegalArgumentException("Unable to parse " + confUmask + 
+                                              " as " + type + " umask.");
+        }
       } 
     }
     

src/test/core/org/apache/hadoop/fs/permission/TestFsPermission.java

@@ -157,11 +157,18 @@ public class TestFsPermission extends TestCase {
         FsPermission.getUMask(conf);
         fail("Shouldn't have been able to parse bad umask");
       } catch(IllegalArgumentException iae) {
-        assertEquals(iae.getMessage(), b);
+        assertTrue("Exception should specify parsing error and invalid umask: " 
+            + iae.getMessage(), isCorrectExceptionMessage(iae.getMessage(), b));
       }
     }
   }
   
+  private boolean isCorrectExceptionMessage(String msg, String umask) {
+    return msg.contains("Unable to parse") &&
+           msg.contains(umask) &&
+           msg.contains("octal or symbolic");
+  }
+  
   // Ensure that when the deprecated decimal umask key is used, it is correctly
   // parsed as such and converted correctly to an FsPermission value
   public void testDeprecatedUmask() {