HDDS-15804. Use Set instead of List for ACLs in OMKeyRequest.

hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/OzoneAcl.java

@@ -76,7 +76,7 @@ public final class OzoneAcl {
   @JsonIgnore
   private final Supplier<String> toStringMethod;
   @JsonIgnore
-  private final Supplier<Integer> hashCodeMethod;
+  private final MemoizedSupplier<Integer> hashCodeMethod;
 
   public static OzoneAcl of(ACLIdentityType type, String name, AclScope scope, ACLType... acls) {
     return new OzoneAcl(type, name, scope, toInt(acls));
@@ -348,6 +348,13 @@ public ACLIdentityType getType() {
     return type;
   }
 
+  public boolean sameNameTypeScope(OzoneAcl that) {
+    return this.getType() == that.getType()
+        && this.getAclScope() == that.getAclScope()
+        // compare string at last since it is expensive
+        && this.getName().equals(that.getName());
+  }
+
   /**
    * Indicates whether some other object is "equal to" this one.
    *
@@ -364,11 +371,14 @@ public boolean equals(Object obj) {
     if (obj == null || getClass() != obj.getClass()) {
       return false;
     }
-    OzoneAcl otherAcl = (OzoneAcl) obj;
-    return otherAcl.getName().equals(this.getName()) &&
-        otherAcl.getType().equals(this.getType()) &&
-        this.aclBits == otherAcl.aclBits &&
-        otherAcl.getAclScope().equals(this.getAclScope());
+    final OzoneAcl that = (OzoneAcl) obj;
+    if (this.hashCodeMethod.isInitialized() && that.hashCodeMethod.isInitialized()) {
+      if (!Objects.equals(this.hashCodeMethod.get(), that.hashCodeMethod.get())) {
+        return false;
+      }
+    }
+    return this.aclBits == that.aclBits
+        && sameNameTypeScope(that);
   }
 
   /**

hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/AclListBuilder.java

@@ -21,6 +21,7 @@
 import jakarta.annotation.Nonnull;
 import jakarta.annotation.Nullable;
 import java.util.ArrayList;
+import java.util.Collection;
 import java.util.List;
 import java.util.Objects;
 import org.apache.hadoop.ozone.OzoneAcl;
@@ -31,7 +32,7 @@ public final class AclListBuilder {
   /** The original list being built from, used if no changes are made, to reduce copying. */
   private final ImmutableList<OzoneAcl> originalList;
   /** The updated list being built, created lazily on the first modification. */
-  private List<OzoneAcl> updatedList;
+  private Collection<OzoneAcl> updatedList;
   /** Whether any changes were made. */
   private boolean changed;
 
@@ -80,7 +81,7 @@ public boolean add(@Nonnull OzoneAcl acl) {
     return added;
   }
 
-  public boolean addAll(@Nullable List<OzoneAcl> newAcls) {
+  public boolean addAll(@Nullable Collection<OzoneAcl> newAcls) {
     if (newAcls == null || newAcls.isEmpty()) {
       return false;
     }
@@ -91,7 +92,7 @@ public boolean addAll(@Nullable List<OzoneAcl> newAcls) {
   }
 
   /** Set the list being built to {@code acls}.  For further mutations to work, it must be modifiable. */
-  public boolean set(@Nonnull List<OzoneAcl> acls) {
+  public boolean set(@Nonnull Collection<OzoneAcl> acls) {
     Objects.requireNonNull(acls, "acls == null");
     boolean set = !acls.equals(updatedList != null ? updatedList : originalList);
     changed |= set;

hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmDirectoryInfo.java

@@ -18,7 +18,7 @@
 package org.apache.hadoop.ozone.om.helpers;
 
 import com.google.common.collect.ImmutableList;
-import java.util.List;
+import java.util.Collection;
 import java.util.Map;
 import java.util.Objects;
 import net.jcip.annotations.Immutable;
@@ -154,7 +154,7 @@ public Builder setModificationTime(long newModificationTime) {
       return this;
     }
 
-    public Builder setAcls(List<OzoneAcl> listOfAcls) {
+    public Builder setAcls(Collection<OzoneAcl> listOfAcls) {
       this.acls.addAll(listOfAcls);
       return this;
     }

hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmKeyInfo.java

@@ -21,6 +21,7 @@
 import com.google.common.collect.ImmutableMap;
 import jakarta.annotation.Nullable;
 import java.util.ArrayList;
+import java.util.Collection;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -617,7 +618,7 @@ public Builder setFileEncryptionInfo(FileEncryptionInfo feInfo) {
       return this;
     }
 
-    public Builder setAcls(List<OzoneAcl> listOfAcls) {
+    public Builder setAcls(Collection<OzoneAcl> listOfAcls) {
       if (listOfAcls != null) {
         this.acls.set(listOfAcls);
       }

hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OzoneAclUtil.java

@@ -24,6 +24,8 @@
 
 import java.io.IOException;
 import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Iterator;
 import java.util.List;
 import java.util.Objects;
 import java.util.function.Predicate;
@@ -33,7 +35,6 @@
 import org.apache.hadoop.ozone.om.OmConfig;
 import org.apache.hadoop.ozone.om.exceptions.OMException;
 import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OzoneAclInfo;
-import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer;
 import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer.ACLType;
 import org.apache.hadoop.ozone.security.acl.RequestContext;
 import org.apache.hadoop.security.UserGroupInformation;
@@ -85,26 +86,6 @@ public static List<OzoneAcl> getAclList(UserGroupInformation ugi, ACLType userPr
     return listOfAcls;
   }
 
-  /**
-   * Helper function to get acl list for one user/group.
-   *
-   * @param identityName
-   * @param type
-   * @param aclList
-   * @return list of OzoneAcls
-   * */
-  public static List<OzoneAcl> filterAclList(String identityName,
-      IAccessAuthorizer.ACLIdentityType type, List<OzoneAcl> aclList) {
-
-    if (aclList == null || aclList.isEmpty()) {
-      return new ArrayList<>();
-    }
-
-    List retList = aclList.stream().filter(acl -> acl.getType() == type
-        && acl.getName().equals(identityName)).collect(Collectors.toList());
-    return retList;
-  }
-
   private static boolean checkAccessInAcl(OzoneAcl a, UserGroupInformation ugi,
       ACLType aclToCheck) {
     switch (a.getType()) {
@@ -163,7 +144,7 @@ public static boolean inheritDefaultAcls(AclListBuilder acls,
    * @param scope scope applied to inherited ACL
    * @return true if any ACL was inherited from parent, false otherwise
    */
-  public static boolean inheritDefaultAcls(List<OzoneAcl> acls,
+  public static boolean inheritDefaultAcls(Collection<OzoneAcl> acls,
       List<OzoneAcl> parentAcls, OzoneAcl.AclScope scope) {
     return inheritDefaultAcls(acl -> addAcl(acls, acl), parentAcls, scope);
   }
@@ -219,20 +200,19 @@ public static List<OzoneAclInfo> toProtobuf(List<OzoneAcl> protoAcls) {
    * Add an OzoneAcl to existing list of OzoneAcls.
    * @return true if current OzoneAcls are changed, false otherwise.
    */
-  public static boolean addAcl(List<OzoneAcl> existingAcls, OzoneAcl acl) {
+  public static boolean addAcl(Collection<OzoneAcl> existingAcls, OzoneAcl acl) {
     if (existingAcls == null || acl == null) {
       return false;
     }
 
-    for (int i = 0; i < existingAcls.size(); i++) {
-      final OzoneAcl a = existingAcls.get(i);
-      if (a.getName().equals(acl.getName()) &&
-          a.getType().equals(acl.getType()) &&
-          a.getAclScope().equals(acl.getAclScope())) {
+    for (Iterator<OzoneAcl> i = existingAcls.iterator(); i.hasNext();) {
+      final OzoneAcl a = i.next();
+      if (a.sameNameTypeScope(acl)) {
         final OzoneAcl updated = a.add(acl);
         final boolean changed = !Objects.equals(updated, a);
         if (changed) {
-          existingAcls.set(i, updated);
+          i.remove();
+          existingAcls.add(updated);
         }
         return changed;
       }
@@ -242,7 +222,7 @@ public static boolean addAcl(List<OzoneAcl> existingAcls, OzoneAcl acl) {
     return true;
   }
 
-  public static boolean addAllAcl(List<OzoneAcl> existingAcls, List<OzoneAcl> acls) {
+  public static boolean addAllAcl(Collection<OzoneAcl> existingAcls, Collection<OzoneAcl> acls) {
     // TOOD optimize
     boolean changed = false;
     for (OzoneAcl acl : acls) {
@@ -255,22 +235,21 @@ public static boolean addAllAcl(List<OzoneAcl> existingAcls, List<OzoneAcl> acls
    * remove OzoneAcl from existing list of OzoneAcls.
    * @return true if current OzoneAcls are changed, false otherwise.
    */
-  public static boolean removeAcl(List<OzoneAcl> existingAcls, OzoneAcl acl) {
+  static boolean removeAcl(Collection<OzoneAcl> existingAcls, OzoneAcl acl) {
     if (existingAcls == null || existingAcls.isEmpty() || acl == null) {
       return false;
     }
 
-    for (int i = 0; i < existingAcls.size(); i++) {
-      final OzoneAcl a = existingAcls.get(i);
-      if (a.getName().equals(acl.getName()) &&
-          a.getType().equals(acl.getType()) &&
-          a.getAclScope().equals(acl.getAclScope())) {
+    for (Iterator<OzoneAcl> i = existingAcls.iterator(); i.hasNext();) {
+      final OzoneAcl a = i.next();
+      if (a.sameNameTypeScope(acl)) {
         final OzoneAcl updated = a.remove(acl);
         final boolean changed = !Objects.equals(updated, a);
         if (updated.isEmpty()) {
-          existingAcls.remove(i);
+          i.remove();
         } else if (changed) {
-          existingAcls.set(i, updated);
+          i.remove();
+          existingAcls.add(updated);
         }
         return changed;
       }

hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/security/acl/IAccessAuthorizer.java

@@ -91,29 +91,15 @@ enum ACLType {
     NONE,
     ASSUME_ROLE;   // ability to create STS tokens
 
-    private static int length = ACLType.values().length;
     static {
+      final int length = values().length;
       if (length > 16) {
         // must update getAclBytes(..) and other code
         throw new AssertionError("BUG: Length = " + length
             + " > 16, check the commit of this change and update the code.");
       }
     }
 
-    private static ACLType[] vals = ACLType.values();
-
-    public static int getNoOfAcls() {
-      return length;
-    }
-
-    public static ACLType getAclTypeFromOrdinal(int ordinal) {
-      if (ordinal > length - 1 && ordinal > -1) {
-        throw new IllegalArgumentException("Ordinal greater than array length" +
-            ". ordinal:" + ordinal);
-      }
-      return vals[ordinal];
-    }
-
     /**
      * Returns the ACL rights based on passed in String.
      *

hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequest.java

@@ -42,10 +42,12 @@
 import java.util.EnumSet;
 import java.util.HashMap;
 import java.util.Iterator;
+import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
 import java.util.Optional;
+import java.util.Set;
 import java.util.function.Function;
 import java.util.stream.Collectors;
 import org.apache.commons.lang3.tuple.Pair;
@@ -328,12 +330,11 @@ public EncryptedKeyVersion run() throws IOException {
     return edek;
   }
 
-  protected List<OzoneAcl> getAclsForKey(KeyArgs keyArgs,
+  protected Set<OzoneAcl> getAclsForKey(KeyArgs keyArgs,
       OmBucketInfo bucketInfo, OMFileRequest.OMPathInfo omPathInfo,
       PrefixManager prefixManager, OmConfig config) throws OMException {
 
-    List<OzoneAcl> acls = new ArrayList<>();
-    acls.addAll(getDefaultAclList(createUGIForApi(), config));
+    final Set<OzoneAcl> acls = new LinkedHashSet<>(getDefaultAclList(createUGIForApi(), config));
     if (!keyArgs.getAclsList().isEmpty() && !config.ignoreClientACLs()) {
       acls.addAll(OzoneAclUtil.fromProtobuf(keyArgs.getAclsList()));
     }
@@ -352,7 +353,6 @@ protected List<OzoneAcl> getAclsForKey(KeyArgs keyArgs,
         if (prefixInfo  != null) {
           if (OzoneAclUtil.inheritDefaultAcls(acls, prefixInfo.getAcls(), ACCESS)) {
             // Remove the duplicates
-            acls = acls.stream().distinct().collect(Collectors.toList());
             return acls;
           }
         }
@@ -363,7 +363,6 @@ protected List<OzoneAcl> getAclsForKey(KeyArgs keyArgs,
     // prefix are not set
     if (omPathInfo != null) {
       if (OzoneAclUtil.inheritDefaultAcls(acls, omPathInfo.getAcls(), ACCESS)) {
-        acls = acls.stream().distinct().collect(Collectors.toList());
         return acls;
       }
     }
@@ -372,12 +371,10 @@ protected List<OzoneAcl> getAclsForKey(KeyArgs keyArgs,
     // parent-dir are not set.
     if (bucketInfo != null) {
       if (OzoneAclUtil.inheritDefaultAcls(acls, bucketInfo.getAcls(), ACCESS)) {
-        acls = acls.stream().distinct().collect(Collectors.toList());
         return acls;
       }
     }
 
-    acls = acls.stream().distinct().collect(Collectors.toList());
     return acls;
   }
 
@@ -389,12 +386,11 @@ protected List<OzoneAcl> getAclsForKey(KeyArgs keyArgs,
    * @param config
    * @return Acls which inherited parent DEFAULT and keyArgs ACCESS acls.
    */
-  protected List<OzoneAcl> getAclsForDir(KeyArgs keyArgs, OmBucketInfo bucketInfo,
+  protected Set<OzoneAcl> getAclsForDir(KeyArgs keyArgs, OmBucketInfo bucketInfo,
       OMFileRequest.OMPathInfo omPathInfo, OmConfig config) throws OMException {
     // Acls inherited from parent or bucket will convert to DEFAULT scope
-    List<OzoneAcl> acls = new ArrayList<>();
     // add default ACLs
-    acls.addAll(getDefaultAclList(createUGIForApi(), config));
+    final Set<OzoneAcl> acls = new LinkedHashSet<>(getDefaultAclList(createUGIForApi(), config));
 
     // Inherit DEFAULT acls from parent-dir
     if (omPathInfo != null) {
@@ -411,7 +407,6 @@ protected List<OzoneAcl> getAclsForDir(KeyArgs keyArgs, OmBucketInfo bucketInfo,
     if (!keyArgs.getAclsList().isEmpty() && !config.ignoreClientACLs()) {
       acls.addAll(OzoneAclUtil.fromProtobuf(keyArgs.getAclsList()));
     }
-    acls = acls.stream().distinct().collect(Collectors.toList());
     return acls;
   }
 

hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketAclHandler.java

@@ -38,7 +38,6 @@
 import org.apache.hadoop.ozone.client.OzoneBucket;
 import org.apache.hadoop.ozone.client.OzoneVolume;
 import org.apache.hadoop.ozone.om.exceptions.OMException;
-import org.apache.hadoop.ozone.om.helpers.OzoneAclUtil;
 import org.apache.hadoop.ozone.s3.endpoint.S3BucketAcl.Grant;
 import org.apache.hadoop.ozone.s3.exception.OS3Exception;
 import org.apache.hadoop.ozone.s3.exception.S3ErrorTable;
@@ -181,8 +180,10 @@ Response handlePutRequest(S3RequestContext context, String bucketName, InputStre
       if (!currentAclsOnVolume.isEmpty()) {
         for (OzoneAcl acl : acls) {
           if (acl.getAclScope() == ACCESS) {
-            aclsToRemoveOnVolume.addAll(OzoneAclUtil.filterAclList(
-                acl.getName(), acl.getType(), currentAclsOnVolume));
+            currentAclsOnVolume.stream()
+                .filter(a -> a.getType() == acl.getType())
+                .filter(a -> a.getName().equals(acl.getName()))
+                .forEach(aclsToRemoveOnVolume::add);
           }
         }
         for (OzoneAcl acl : aclsToRemoveOnVolume) {