HADOOP-6670. Use the UserGroupInformation's Subject as the criteria for equals and hashCode. Contributed by Owen O'Malley and Kan Zhang.

git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/trunk@964993 13f79535-47bb-0310-9956-ffa450edef68

CHANGES.txt

@@ -130,6 +130,9 @@ Trunk (unreleased changes)
     HADOOP-6834. TFile.append compares initial key against null lastKey
     (hong tang via mahadev)
 
+    HADOOP-6670. Use the UserGroupInformation's Subject as the criteria for
+    equals and hashCode. (Owen O'Malley and Kan Zhang via ddas)
+
 Release 0.21.0 - Unreleased
 
   INCOMPATIBLE CHANGES

src/java/org/apache/hadoop/ipc/Client.java

@@ -1074,8 +1074,8 @@ public class Client {
      if (obj instanceof ConnectionId) {
        ConnectionId id = (ConnectionId) obj;
        return address.equals(id.address) && protocol == id.protocol && 
-              ticket == id.ticket;
-       //Note : ticket is a ref comparision.
+              ((ticket != null && ticket.equals(id.ticket)) ||
+               (ticket == id.ticket));
      }
      return false;
     }
@@ -1083,7 +1083,7 @@ public class Client {
     @Override
     public int hashCode() {
       return (address.hashCode() + PRIME * System.identityHashCode(protocol)) ^ 
-             System.identityHashCode(ticket);
+             (ticket == null ? 0 : ticket.hashCode());
     }
   }  
 }

src/java/org/apache/hadoop/security/UserGroupInformation.java

@@ -804,7 +804,7 @@ public class UserGroupInformation {
     } else if (o == null || getClass() != o.getClass()) {
       return false;
     } else {
-      return subject.equals(((UserGroupInformation) o).subject);
+      return subject == ((UserGroupInformation) o).subject;
     }
   }
 
@@ -813,7 +813,7 @@ public class UserGroupInformation {
    */
   @Override
   public int hashCode() {
-    return subject.hashCode();
+    return System.identityHashCode(subject);
   }
 
   /**

src/test/core/org/apache/hadoop/fs/TestFileSystemCaching.java

@@ -130,28 +130,26 @@ public class TestFileSystemCaching {
     assertNotSame(fsA, fsB);
     
     Token<T> t1 = mock(Token.class);
-    ugiA = UserGroupInformation.createRemoteUser("foo");
-    ugiA.addToken(t1);
+    UserGroupInformation ugiA2 = UserGroupInformation.createRemoteUser("foo");
     
-    fsA = ugiA.doAs(new PrivilegedExceptionAction<FileSystem>() {
+    fsA = ugiA2.doAs(new PrivilegedExceptionAction<FileSystem>() {
       public FileSystem run() throws Exception {
         return FileSystem.get(new URI("cachedfile://a"), conf);
       }
     });
-    //Although the users in the UGI are same, ugiA has tokens in it, and
-    //we should end up with different filesystems corresponding to the two UGIs
+    // Although the users in the UGI are same, they have different subjects
+    // and so are different.
     assertNotSame(fsA, fsA1);
     
-    ugiA = UserGroupInformation.createRemoteUser("foo");
     ugiA.addToken(t1);
     
-    fsA1 = ugiA.doAs(new PrivilegedExceptionAction<FileSystem>() {
+    fsA = ugiA.doAs(new PrivilegedExceptionAction<FileSystem>() {
       public FileSystem run() throws Exception {
         return FileSystem.get(new URI("cachedfile://a"), conf);
       }
     });
-    //Now the users in the UGI are the same, and they also have the same token.
-    //We should have the same filesystem for both
+    // Make sure that different UGI's with the same subject lead to the same
+    // file system.
     assertSame(fsA, fsA1);
   }
   

src/test/core/org/apache/hadoop/security/TestUserGroupInformation.java

@@ -158,12 +158,16 @@ public class TestUserGroupInformation {
       UserGroupInformation.createUserForTesting(USER_NAME, GROUP_NAMES);
 
     assertEquals(uugi, uugi);
-    // The subjects should be equal, so this should work
-    assertTrue(uugi.equals(
-                 UserGroupInformation.createUserForTesting
-                   (USER_NAME, GROUP_NAMES)));
-    // ensure that different UGI with the same subject are equal
-    assertEquals(uugi, new UserGroupInformation(uugi.getSubject()));
+    // The subjects should be different, so this should fail
+    UserGroupInformation ugi2 = 
+      UserGroupInformation.createUserForTesting(USER_NAME, GROUP_NAMES);
+    assertFalse(uugi.equals(ugi2));
+    assertFalse(uugi.hashCode() == ugi2.hashCode());
+
+    // two ugi that have the same subject need to be equal
+    UserGroupInformation ugi3 = new UserGroupInformation(uugi.getSubject());
+    assertEquals(uugi, ugi3);
+    assertEquals(uugi.hashCode(), ugi3.hashCode());
   }
   
   @Test
@@ -174,8 +178,8 @@ public class TestUserGroupInformation {
         "RealUser", GROUP_NAMES);
     UserGroupInformation proxyUgi1 = UserGroupInformation.createProxyUser(
         USER_NAME, realUgi1);
-    UserGroupInformation proxyUgi2 = UserGroupInformation.createProxyUser(
-        USER_NAME, realUgi2);
+    UserGroupInformation proxyUgi2 =
+      new UserGroupInformation( proxyUgi1.getSubject());
     UserGroupInformation remoteUgi = UserGroupInformation.createRemoteUser(USER_NAME);
     assertEquals(proxyUgi1, proxyUgi2);
     assertFalse(remoteUgi.equals(proxyUgi1));
@@ -288,16 +292,16 @@ public class TestUserGroupInformation {
         return null;
       }
     });
-    UserGroupInformation proxyUgi2 = UserGroupInformation.createProxyUser(
-        "proxy", ugi);
+    UserGroupInformation proxyUgi2 = 
+      new UserGroupInformation(proxyUgi.getSubject());
     proxyUgi2.setAuthenticationMethod(AuthenticationMethod.PROXY);
     Assert.assertEquals(proxyUgi, proxyUgi2);
     // Equality should work if authMethod is null
     UserGroupInformation realugi = UserGroupInformation.getCurrentUser();
     UserGroupInformation proxyUgi3 = UserGroupInformation.createProxyUser(
         "proxyAnother", realugi);
-    UserGroupInformation proxyUgi4 = UserGroupInformation.createProxyUser(
-        "proxyAnother", realugi);
+    UserGroupInformation proxyUgi4 = 
+      new UserGroupInformation(proxyUgi3.getSubject());
     Assert.assertEquals(proxyUgi3, proxyUgi4);
   }