Mailing-List: contact common-commits-help@hadoop.apache.org; run by ezmlm
Precedence: bulk
Reply-To: common-dev@hadoop.apache.org
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: svn commit: r964993 - in /hadoop/common/trunk: ./
 src/java/org/apache/hadoop/ipc/ src/java/org/apache/hadoop/security/
 src/test/core/org/apache/hadoop/fs/ src/test/core/org/apache/hadoop/security/
Date: Sat, 17 Jul 2010 01:42:22 -0000
To: common-commits@hadoop.apache.org
From: ddas@apache.org
Message-Id: <20100717014222.F16102388999@eris.apache.org>

Author: ddas
Date: Sat Jul 17 01:42:22 2010
New Revision: 964993

URL: http://svn.apache.org/viewvc?rev=964993&view=rev
Log:
HADOOP-6670. Use the UserGroupInformation's Subject as the criteria for equals and hashCode. Contributed by Owen O'Malley and Kan Zhang.

Modified:
    hadoop/common/trunk/CHANGES.txt
    hadoop/common/trunk/src/java/org/apache/hadoop/ipc/Client.java
    hadoop/common/trunk/src/java/org/apache/hadoop/security/UserGroupInformation.java
    hadoop/common/trunk/src/test/core/org/apache/hadoop/fs/TestFileSystemCaching.java
    hadoop/common/trunk/src/test/core/org/apache/hadoop/security/TestUserGroupInformation.java

Modified: hadoop/common/trunk/CHANGES.txt
URL: http://svn.apache.org/viewvc/hadoop/common/trunk/CHANGES.txt?rev=964993&r1=964992&r2=964993&view=diff
==============================================================================
--- hadoop/common/trunk/CHANGES.txt (original)
+++ hadoop/common/trunk/CHANGES.txt Sat Jul 17 01:42:22 2010
@@ -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

Modified: hadoop/common/trunk/src/java/org/apache/hadoop/ipc/Client.java
URL: http://svn.apache.org/viewvc/hadoop/common/trunk/src/java/org/apache/hadoop/ipc/Client.java?rev=964993&r1=964992&r2=964993&view=diff
==============================================================================
--- hadoop/common/trunk/src/java/org/apache/hadoop/ipc/Client.java (original)
+++ hadoop/common/trunk/src/java/org/apache/hadoop/ipc/Client.java Sat Jul 17 01:42:22 2010
@@ -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());
     }
   }  
 }

Modified: hadoop/common/trunk/src/java/org/apache/hadoop/security/UserGroupInformation.java
URL: http://svn.apache.org/viewvc/hadoop/common/trunk/src/java/org/apache/hadoop/security/UserGroupInformation.java?rev=964993&r1=964992&r2=964993&view=diff
==============================================================================
--- hadoop/common/trunk/src/java/org/apache/hadoop/security/UserGroupInformation.java (original)
+++ hadoop/common/trunk/src/java/org/apache/hadoop/security/UserGroupInformation.java Sat Jul 17 01:42:22 2010
@@ -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);
   }
 
   /**

Modified: hadoop/common/trunk/src/test/core/org/apache/hadoop/fs/TestFileSystemCaching.java
URL: http://svn.apache.org/viewvc/hadoop/common/trunk/src/test/core/org/apache/hadoop/fs/TestFileSystemCaching.java?rev=964993&r1=964992&r2=964993&view=diff
==============================================================================
--- hadoop/common/trunk/src/test/core/org/apache/hadoop/fs/TestFileSystemCaching.java (original)
+++ hadoop/common/trunk/src/test/core/org/apache/hadoop/fs/TestFileSystemCaching.java Sat Jul 17 01:42:22 2010
@@ -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);
   }
   

Modified: hadoop/common/trunk/src/test/core/org/apache/hadoop/security/TestUserGroupInformation.java
URL: http://svn.apache.org/viewvc/hadoop/common/trunk/src/test/core/org/apache/hadoop/security/TestUserGroupInformation.java?rev=964993&r1=964992&r2=964993&view=diff
==============================================================================
--- hadoop/common/trunk/src/test/core/org/apache/hadoop/security/TestUserGroupInformation.java (original)
+++ hadoop/common/trunk/src/test/core/org/apache/hadoop/security/TestUserGroupInformation.java Sat Jul 17 01:42:22 2010
@@ -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);
   }