This is an automated email from the ASF dual-hosted git repository.
adoroszlai pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/ozone.git
The following commit(s) were added to refs/heads/master by this push:
new 991798c5925 HDDS-15804. Use Set instead of List for ACLs in
OMKeyRequest (#10727)
991798c5925 is described below
commit 991798c5925542d851015e5691194ed3688598d0
Author: Tsz-Wo Nicholas Sze <[email protected]>
AuthorDate: Sat Jul 11 07:05:49 2026 -0700
HDDS-15804. Use Set instead of List for ACLs in OMKeyRequest (#10727)
---
.../java/org/apache/hadoop/ozone/OzoneAcl.java | 22 ++++++---
.../hadoop/ozone/om/helpers/AclListBuilder.java | 7 +--
.../hadoop/ozone/om/helpers/OmDirectoryInfo.java | 4 +-
.../apache/hadoop/ozone/om/helpers/OmKeyInfo.java | 3 +-
.../hadoop/ozone/om/helpers/OzoneAclUtil.java | 55 +++++++---------------
.../ozone/security/acl/IAccessAuthorizer.java | 16 +------
.../hadoop/ozone/om/request/key/OMKeyRequest.java | 16 ++-----
.../hadoop/ozone/s3/endpoint/BucketAclHandler.java | 7 +--
8 files changed, 51 insertions(+), 79 deletions(-)
diff --git
a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/OzoneAcl.java
b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/OzoneAcl.java
index 695b85afcdf..2d22c4879e1 100644
--- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/OzoneAcl.java
+++ b/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);
}
/**
diff --git
a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/AclListBuilder.java
b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/AclListBuilder.java
index 5e097ff1663..23d00cc4bdc 100644
---
a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/AclListBuilder.java
+++
b/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;
diff --git
a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmDirectoryInfo.java
b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmDirectoryInfo.java
index 3657d5ee68b..7f489b528f2 100644
---
a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmDirectoryInfo.java
+++
b/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;
}
diff --git
a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmKeyInfo.java
b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmKeyInfo.java
index dcb4b1baafb..4d8d3d4c7eb 100644
---
a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmKeyInfo.java
+++
b/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);
}
diff --git
a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OzoneAclUtil.java
b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OzoneAclUtil.java
index 3dae69f7110..6cfb905a68e 100644
---
a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OzoneAclUtil.java
+++
b/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;
}
diff --git
a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/security/acl/IAccessAuthorizer.java
b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/security/acl/IAccessAuthorizer.java
index 8a07bab606b..a9d59c63591 100644
---
a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/security/acl/IAccessAuthorizer.java
+++
b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/security/acl/IAccessAuthorizer.java
@@ -91,8 +91,8 @@ 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
@@ -100,20 +100,6 @@ enum ACLType {
}
}
- 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.
*
diff --git
a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequest.java
b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequest.java
index 115eacd8712..d12b8fa0525 100644
---
a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequest.java
+++
b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequest.java
@@ -42,6 +42,7 @@
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;
@@ -324,12 +325,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()));
}
@@ -348,7 +348,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;
}
}
@@ -359,7 +358,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;
}
}
@@ -368,12 +366,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;
}
@@ -385,12 +381,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) {
@@ -407,7 +402,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;
}
diff --git
a/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketAclHandler.java
b/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketAclHandler.java
index ace1110791a..bc9e9dca281 100644
---
a/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketAclHandler.java
+++
b/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) {
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
