HDDS-1917. TestOzoneRpcClientAbstract is failing.

Signed-off-by: Anu Engineer <aengineer@apache.org>

hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneRpcClientAbstract.java

@@ -26,6 +26,7 @@ import java.util.HashMap;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
+import java.util.Optional;
 import java.util.TreeMap;
 import java.util.UUID;
 import java.util.concurrent.CountDownLatch;
@@ -2456,31 +2457,30 @@ public abstract class TestOzoneRpcClientAbstract {
           ACLType.READ_ACL, ACCESS);
       // Verify that operation successful.
       assertTrue(store.addAcl(ozObj, newAcl));
-      List<OzoneAcl> acls = store.getAcl(ozObj);
-
-      assertTrue(acls.size() == expectedAcls.size());
-      boolean aclVerified = false;
-      for(OzoneAcl acl: acls) {
-        if(acl.getName().equals(newAcl.getName())) {
-          assertTrue(acl.getAclList().contains(ACLType.READ_ACL));
-          aclVerified = true;
-        }
-      }
-      assertTrue("New acl expected but not found.", aclVerified);
-      aclVerified = false;
+
+      assertEquals(expectedAcls.size(), store.getAcl(ozObj).size());
+      final Optional<OzoneAcl> readAcl = store.getAcl(ozObj).stream()
+          .filter(acl -> acl.getName().equals(newAcl.getName())
+              && acl.getType().equals(newAcl.getType()))
+          .findFirst();
+      assertTrue("New acl expected but not found.", readAcl.isPresent());
+      assertTrue("READ_ACL should exist in current acls:"
+          + readAcl.get(),
+          readAcl.get().getAclList().contains(ACLType.READ_ACL));
+
 
       // Case:2 Remove newly added acl permission.
       assertTrue(store.removeAcl(ozObj, newAcl));
-      acls = store.getAcl(ozObj);
-      assertTrue(acls.size() == expectedAcls.size());
-      for(OzoneAcl acl: acls) {
-        if(acl.getName().equals(newAcl.getName())) {
-          assertFalse("READ_ACL should not exist in current acls:" +
-              acls, acl.getAclList().contains(ACLType.READ_ACL));
-          aclVerified = true;
-        }
-      }
-      assertTrue("New acl expected but not found.", aclVerified);
+
+      assertEquals(expectedAcls.size(), store.getAcl(ozObj).size());
+      final Optional<OzoneAcl> nonReadAcl = store.getAcl(ozObj).stream()
+          .filter(acl -> acl.getName().equals(newAcl.getName())
+              && acl.getType().equals(newAcl.getType()))
+          .findFirst();
+      assertTrue("New acl expected but not found.", nonReadAcl.isPresent());
+      assertFalse("READ_ACL should not exist in current acls:"
+              + nonReadAcl.get(),
+          nonReadAcl.get().getAclList().contains(ACLType.READ_ACL));
     } else {
       fail("Default acl should not be empty.");
     }
@@ -2493,17 +2493,17 @@ public abstract class TestOzoneRpcClientAbstract {
       store.removeAcl(ozObj, a);
     }
     List<OzoneAcl> newAcls = store.getAcl(ozObj);
-    assertTrue(newAcls.size() == 0);
+    assertEquals(0, newAcls.size());
 
     // Add acl's and then call getAcl.
     int aclCount = 0;
     for (OzoneAcl a : expectedAcls) {
       aclCount++;
       assertTrue(store.addAcl(ozObj, a));
-      assertTrue(store.getAcl(ozObj).size() == aclCount);
+      assertEquals(aclCount, store.getAcl(ozObj).size());
     }
     newAcls = store.getAcl(ozObj);
-    assertTrue(newAcls.size() == expectedAcls.size());
+    assertEquals(expectedAcls.size(), newAcls.size());
     List<OzoneAcl> finalNewAcls = newAcls;
     expectedAcls.forEach(a -> assertTrue(finalNewAcls.contains(a)));
 
@@ -2514,7 +2514,7 @@ public abstract class TestOzoneRpcClientAbstract {
         ACLType.ALL, ACCESS);
     store.setAcl(ozObj, Arrays.asList(ua, ug));
     newAcls = store.getAcl(ozObj);
-    assertTrue(newAcls.size() == 2);
+    assertEquals(2, newAcls.size());
     assertTrue(newAcls.contains(ua));
     assertTrue(newAcls.contains(ug));
   }

hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java

@@ -77,6 +77,7 @@ import org.apache.hadoop.ozone.om.helpers.OmPartInfo;
 import org.apache.hadoop.ozone.om.helpers.OpenKeySession;
 import org.apache.hadoop.ozone.om.helpers.OzoneFSUtils;
 import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OzoneAclInfo;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OzoneAclInfo.OzoneAclType;
 import org.apache.hadoop.ozone.security.OzoneBlockTokenSecretManager;
 import org.apache.hadoop.ozone.security.acl.OzoneObj;
 import org.apache.hadoop.ozone.security.acl.RequestContext;
@@ -1406,7 +1407,8 @@ public class KeyManagerImpl implements KeyManager {
       List<OzoneAclInfo> newAcls = new ArrayList<>(keyInfo.getAcls());
       OzoneAclInfo newAcl = null;
       for(OzoneAclInfo a: keyInfo.getAcls()) {
-        if(a.getName().equals(acl.getName())) {
+        if (a.getName().equals(acl.getName()) &&
+            a.getType().equals(OzoneAclType.valueOf(acl.getType().name()))) {
           BitSet currentAcls = BitSet.valueOf(a.getRights().toByteArray());
           currentAcls.or(acl.getAclBitSet());
 
@@ -1483,7 +1485,8 @@ public class KeyManagerImpl implements KeyManager {
       } else {
         // Acl to be removed might be a subset of existing acls.
         for(OzoneAclInfo a: keyInfo.getAcls()) {
-          if(a.getName().equals(acl.getName())) {
+          if (a.getName().equals(acl.getName()) &&
+              a.getType().equals(OzoneAclType.valueOf(acl.getType().name()))) {
             BitSet currentAcls = BitSet.valueOf(a.getRights().toByteArray());
             acl.getAclBitSet().xor(currentAcls);
             currentAcls.and(acl.getAclBitSet());