HDDS-10515. Reduce OzoneAcl constructor count (apache#6396)

myskov · Mar 20, 2024 · 8582214 · 8582214
1 parent e907316
commit 8582214
Show file tree

Hide file tree

Showing 27 changed files with 192 additions and 228 deletions.
diff --git a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java
@@ -156,7 +156,6 @@
 import java.security.PrivilegedExceptionAction;
 import java.util.ArrayList;
 import java.util.Arrays;
-import java.util.BitSet;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.LinkedHashMap;
@@ -432,12 +431,12 @@ public void createVolume(String volumeName, VolumeArgs volArgs)
     List<OzoneAcl> listOfAcls = new ArrayList<>();
     //User ACL
     listOfAcls.add(new OzoneAcl(ACLIdentityType.USER,
-        owner, userRights, ACCESS));
+        owner, ACCESS, userRights));
     //Group ACLs of the User
     List<String> userGroups = Arrays.asList(UserGroupInformation
         .createRemoteUser(owner).getGroupNames());
     userGroups.stream().forEach((group) -> listOfAcls.add(
-        new OzoneAcl(ACLIdentityType.GROUP, group, groupRights, ACCESS)));
+        new OzoneAcl(ACLIdentityType.GROUP, group, ACCESS, groupRights)));
     //ACLs from VolumeArgs
     List<OzoneAcl> volumeAcls = volArgs.getAcls();
     if (volumeAcls != null) {
@@ -757,10 +756,7 @@ private List<OzoneAcl> getAclList() {
    * @return OzoneAcl
    */
   private OzoneAcl linkBucketDefaultAcl() {
-    BitSet aclRights = new BitSet();
-    aclRights.set(READ.ordinal());
-    aclRights.set(WRITE.ordinal());
-    return new OzoneAcl(ACLIdentityType.WORLD, "", aclRights, ACCESS);
+    return new OzoneAcl(ACLIdentityType.WORLD, "", ACCESS, READ, WRITE);
   }
 
   /**

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
@@ -30,6 +30,7 @@
 
 import java.util.ArrayList;
 import java.util.BitSet;
+import java.util.EnumSet;
 import java.util.List;
 import java.util.Objects;
 import java.util.function.Consumer;
@@ -58,17 +59,12 @@ public class OzoneAcl {
   private final AclScope aclScope;
   private static final List<ACLType> EMPTY_LIST = new ArrayList<>(0);
 
-  // TODO use varargs constructor
-  public OzoneAcl(ACLIdentityType type, String name, ACLType acl, AclScope scope) {
-    this(type, name, scope, bitSetOf(acl));
-  }
-
   public OzoneAcl(ACLIdentityType type, String name, AclScope scope, ACLType... acls) {
     this(type, name, scope, bitSetOf(acls));
   }
 
-  public OzoneAcl(ACLIdentityType type, String name, BitSet acls, AclScope scope) {
-    this(type, name, scope, validateAndCopy(acls));
+  public OzoneAcl(ACLIdentityType type, String name, AclScope scope, EnumSet<ACLType> acls) {
+    this(type, name, scope, bitSetOf(acls.toArray(new ACLType[0])));
   }
 
   private OzoneAcl(ACLIdentityType type, String name, AclScope scope, BitSet acls) {
@@ -148,7 +144,6 @@ public static OzoneAcl parseAcl(String acl)
     }
 
     ACLIdentityType aclType = ACLIdentityType.valueOf(parts[0].toUpperCase());
-    BitSet acls = new BitSet(ACLType.getNoOfAcls());
 
     String bits = parts[2];
 
@@ -163,14 +158,14 @@ public static OzoneAcl parseAcl(String acl)
           parts[2].indexOf("]")));
     }
 
-    // Set all acl bits.
+    EnumSet<ACLType> acls = EnumSet.noneOf(ACLType.class);
     for (char ch : bits.toCharArray()) {
-      acls.set(ACLType.getACLRight(String.valueOf(ch)).ordinal());
+      acls.add(ACLType.getACLRight(String.valueOf(ch)));
     }
 
     // TODO : Support sanitation of these user names by calling into
     // userAuth Interface.
-    return new OzoneAcl(aclType, parts[1], acls, aclScope);
+    return new OzoneAcl(aclType, parts[1], aclScope, acls);
   }
 
   /**
@@ -208,9 +203,8 @@ public static OzoneAclInfo toProtobuf(OzoneAcl acl) {
 
   public static OzoneAcl fromProtobuf(OzoneAclInfo protoAcl) {
     BitSet aclRights = BitSet.valueOf(protoAcl.getRights().toByteArray());
-    return new OzoneAcl(ACLIdentityType.valueOf(protoAcl.getType().name()),
-        protoAcl.getName(), aclRights,
-        AclScope.valueOf(protoAcl.getAclScope().name()));
+    return new OzoneAcl(ACLIdentityType.valueOf(protoAcl.getType().name()), protoAcl.getName(),
+        AclScope.valueOf(protoAcl.getAclScope().name()), validateAndCopy(aclRights));
   }
 
   public AclScope getAclScope() {

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
@@ -60,11 +60,11 @@ public static List<OzoneAcl> getAclList(String userName,
     List<OzoneAcl> listOfAcls = new ArrayList<>();
 
     // User ACL.
-    listOfAcls.add(new OzoneAcl(USER, userName, userRights, ACCESS));
+    listOfAcls.add(new OzoneAcl(USER, userName, ACCESS, userRights));
     if (userGroups != null) {
       // Group ACLs of the User.
       Arrays.asList(userGroups).forEach((group) -> listOfAcls.add(
-          new OzoneAcl(GROUP, group, groupRights, ACCESS)));
+          new OzoneAcl(GROUP, group, ACCESS, groupRights)));
     }
     return listOfAcls;
   }

diff --git a/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/helpers/TestOmBucketInfo.java b/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/helpers/TestOmBucketInfo.java
@@ -81,8 +81,7 @@ public void testClone() {
         .setAcls(Collections.singletonList(new OzoneAcl(
             IAccessAuthorizer.ACLIdentityType.USER,
             "defaultUser",
-            IAccessAuthorizer.ACLType.WRITE_ACL,
-            OzoneAcl.AclScope.ACCESS
+            OzoneAcl.AclScope.ACCESS, IAccessAuthorizer.ACLType.WRITE_ACL
         )))
         .build();
 
@@ -97,8 +96,7 @@ public void testClone() {
     omBucketInfo.setAcls(Collections.singletonList(new OzoneAcl(
         IAccessAuthorizer.ACLIdentityType.USER,
         "newUser",
-        IAccessAuthorizer.ACLType.WRITE_ACL,
-        OzoneAcl.AclScope.ACCESS
+        OzoneAcl.AclScope.ACCESS, IAccessAuthorizer.ACLType.WRITE_ACL
     )));
     assertNotEquals(
         omBucketInfo.getAcls().get(0),
@@ -115,8 +113,7 @@ public void testClone() {
     omBucketInfo.removeAcl(new OzoneAcl(
         IAccessAuthorizer.ACLIdentityType.USER,
         "newUser",
-        IAccessAuthorizer.ACLType.WRITE_ACL,
-        OzoneAcl.AclScope.ACCESS
+        OzoneAcl.AclScope.ACCESS, IAccessAuthorizer.ACLType.WRITE_ACL
     ));
     assertEquals(0, omBucketInfo.getAcls().size());
     assertEquals(1, cloneBucketInfo.getAcls().size());
@@ -131,8 +128,8 @@ public void getProtobufMessageEC() {
             .setStorageType(StorageType.ARCHIVE).setAcls(Collections
                 .singletonList(new OzoneAcl(
                     IAccessAuthorizer.ACLIdentityType.USER,
-                    "defaultUser", IAccessAuthorizer.ACLType.WRITE_ACL,
-                    OzoneAcl.AclScope.ACCESS))).build();
+                    "defaultUser", OzoneAcl.AclScope.ACCESS, IAccessAuthorizer.ACLType.WRITE_ACL
+                ))).build();
     OzoneManagerProtocolProtos.BucketInfo protobuf = omBucketInfo.getProtobuf();
     // No EC Config
     assertFalse(protobuf.hasDefaultReplicationConfig());
@@ -150,8 +147,8 @@ public void getProtobufMessageEC() {
         .setStorageType(StorageType.ARCHIVE)
         .setAcls(Collections.singletonList(new OzoneAcl(
             IAccessAuthorizer.ACLIdentityType.USER,
-            "defaultUser", IAccessAuthorizer.ACLType.WRITE_ACL,
-            OzoneAcl.AclScope.ACCESS)))
+            "defaultUser", OzoneAcl.AclScope.ACCESS, IAccessAuthorizer.ACLType.WRITE_ACL
+        )))
         .setDefaultReplicationConfig(
             new DefaultReplicationConfig(
                 new ECReplicationConfig(3, 2))).build();

diff --git a/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/helpers/TestOmKeyInfo.java b/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/helpers/TestOmKeyInfo.java
@@ -182,7 +182,7 @@ private void createdAndTest(boolean isMPU) {
 
     key.setAcls(Arrays.asList(new OzoneAcl(
         IAccessAuthorizer.ACLIdentityType.USER, "user1",
-        IAccessAuthorizer.ACLType.WRITE, ACCESS)));
+        ACCESS, IAccessAuthorizer.ACLType.WRITE)));
 
     // Change acls and check.
     assertNotEquals(key, cloneKey);

diff --git a/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/helpers/TestOmVolumeArgs.java b/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/helpers/TestOmVolumeArgs.java
@@ -46,7 +46,7 @@ public void testClone() throws Exception {
         .addMetadata("key1", "value1").addMetadata("key2", "value2")
         .addOzoneAcls(
             new OzoneAcl(IAccessAuthorizer.ACLIdentityType.USER, "user1",
-                IAccessAuthorizer.ACLType.READ, ACCESS)).build();
+                ACCESS, IAccessAuthorizer.ACLType.READ)).build();
 
     OmVolumeArgs cloneVolumeArgs = omVolumeArgs.copyObject();
 
@@ -55,7 +55,7 @@ public void testClone() throws Exception {
     // add user acl to write.
     omVolumeArgs.addAcl(new OzoneAcl(
         IAccessAuthorizer.ACLIdentityType.USER, "user1",
-        IAccessAuthorizer.ACLType.WRITE, ACCESS));
+        ACCESS, IAccessAuthorizer.ACLType.WRITE));
 
     // Now check clone acl
     assertNotEquals(cloneVolumeArgs.getAcls().get(0),
@@ -64,7 +64,7 @@ public void testClone() throws Exception {
     // Set user acl to Write_ACL.
     omVolumeArgs.setAcls(Collections.singletonList(new OzoneAcl(
         IAccessAuthorizer.ACLIdentityType.USER, "user1",
-        IAccessAuthorizer.ACLType.WRITE_ACL, ACCESS)));
+        ACCESS, IAccessAuthorizer.ACLType.WRITE_ACL)));
 
     assertNotEquals(cloneVolumeArgs.getAcls().get(0),
         omVolumeArgs.getAcls().get(0));
@@ -78,7 +78,7 @@ public void testClone() throws Exception {
 
     omVolumeArgs.removeAcl(new OzoneAcl(
         IAccessAuthorizer.ACLIdentityType.USER, "user1",
-        IAccessAuthorizer.ACLType.WRITE_ACL, ACCESS));
+        ACCESS, IAccessAuthorizer.ACLType.WRITE_ACL));
 
     // Removing acl, in original omVolumeArgs it should have no acls.
     assertEquals(0, omVolumeArgs.getAcls().size());

diff --git a/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/helpers/TestOzoneAclUtil.java b/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/helpers/TestOzoneAclUtil.java
@@ -49,13 +49,13 @@ public class TestOzoneAclUtil {
       getDefaultAcls();
 
   private static final OzoneAcl USER1 = new OzoneAcl(USER, "user1",
-      ACLType.READ_ACL, ACCESS);
+      ACCESS, ACLType.READ_ACL);
 
   private static final OzoneAcl USER2 = new OzoneAcl(USER, "user2",
-      ACLType.WRITE, ACCESS);
+      ACCESS, ACLType.WRITE);
 
   private static final OzoneAcl GROUP1 = new OzoneAcl(GROUP, "group1",
-      ACLType.ALL, ACCESS);
+      ACCESS, ACLType.ALL);
 
   @Test
   public void testAddAcl() throws IOException {
@@ -65,7 +65,7 @@ public void testAddAcl() throws IOException {
     // Add new permission to existing acl entry.
     OzoneAcl oldAcl = currentAcls.get(0);
     OzoneAcl newAcl = new OzoneAcl(oldAcl.getType(), oldAcl.getName(),
-        ACLType.READ_ACL, ACCESS);
+        ACCESS, ACLType.READ_ACL);
 
     addAndVerifyAcl(currentAcls, newAcl, true, DEFAULT_ACLS.size());
     // Add same permission again and verify result
@@ -97,7 +97,7 @@ public void testRemoveAcl() {
     // Add new permission to existing acl entru.
     OzoneAcl oldAcl = currentAcls.get(0);
     OzoneAcl newAcl = new OzoneAcl(oldAcl.getType(), oldAcl.getName(),
-        ACLType.READ_ACL, ACCESS);
+        ACCESS, ACLType.READ_ACL);
 
     // Remove non existing acl entry
     removeAndVerifyAcl(currentAcls, USER1, false, DEFAULT_ACLS.size());
@@ -191,11 +191,11 @@ private static List<OzoneAcl> getDefaultAcls() {
     IAccessAuthorizer.ACLType groupRights = aclConfig.getGroupDefaultRights();
 
     OzoneAclUtil.addAcl(ozoneAcls, new OzoneAcl(USER,
-        ugi.getUserName(), userRights, ACCESS));
+        ugi.getUserName(), ACCESS, userRights));
     //Group ACLs of the User
     List<String> userGroups = Arrays.asList(ugi.getGroupNames());
     userGroups.stream().forEach((group) -> OzoneAclUtil.addAcl(ozoneAcls,
-        new OzoneAcl(GROUP, group, groupRights, ACCESS)));
+        new OzoneAcl(GROUP, group, ACCESS, groupRights)));
     return ozoneAcls;
   }
 

diff --git a/...tion-test/src/test/java/org/apache/hadoop/fs/ozone/AbstractRootedOzoneFileSystemTest.java b/...tion-test/src/test/java/org/apache/hadoop/fs/ozone/AbstractRootedOzoneFileSystemTest.java
@@ -94,9 +94,9 @@
 import java.security.PrivilegedExceptionAction;
 import java.util.ArrayList;
 import java.util.Arrays;
-import java.util.BitSet;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.EnumSet;
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.LinkedList;
@@ -1187,17 +1187,15 @@ void testSharedTmpDir() throws IOException {
     ACLType userRights = aclConfig.getUserDefaultRights();
     // Construct ACL for world access
     // ACL admin owner, world read+write
-    BitSet aclRights = new BitSet();
-    aclRights.set(READ.ordinal());
-    aclRights.set(WRITE.ordinal());
+    EnumSet<ACLType> aclRights = EnumSet.of(READ, WRITE);
     // volume acls have all access to admin and read+write access to world
 
     // Construct VolumeArgs
     VolumeArgs volumeArgs = VolumeArgs.newBuilder()
         .setAdmin("admin")
         .setOwner("admin")
-        .addAcl(new OzoneAcl(ACLIdentityType.WORLD, "", aclRights, ACCESS))
-        .addAcl(new OzoneAcl(ACLIdentityType.USER, "admin", userRights, ACCESS))
+        .addAcl(new OzoneAcl(ACLIdentityType.WORLD, "", ACCESS, aclRights))
+        .addAcl(new OzoneAcl(ACLIdentityType.USER, "admin", ACCESS, userRights))
         .setQuotaInNamespace(1000)
         .setQuotaInBytes(Long.MAX_VALUE).build();
     // Sanity check
@@ -1232,7 +1230,7 @@ void testSharedTmpDir() throws IOException {
     BucketArgs bucketArgs = new BucketArgs.Builder()
         .setOwner("admin")
         .addAcl(new OzoneAcl(ACLIdentityType.WORLD, "", ACCESS, READ, WRITE, LIST))
-        .addAcl(new OzoneAcl(ACLIdentityType.USER, "admin", userRights, ACCESS))
+        .addAcl(new OzoneAcl(ACLIdentityType.USER, "admin", ACCESS, userRights))
         .setQuotaInNamespace(1000)
         .setQuotaInBytes(Long.MAX_VALUE).build();
 
@@ -1292,7 +1290,7 @@ void testTempMount() throws IOException {
     ACLType userRights = aclConfig.getUserDefaultRights();
     // Construct ACL for world access
     OzoneAcl aclWorldAccess = new OzoneAcl(ACLIdentityType.WORLD, "",
-        userRights, ACCESS);
+        ACCESS, userRights);
     // Construct VolumeArgs
     VolumeArgs volumeArgs = VolumeArgs.newBuilder()
         .addAcl(aclWorldAccess)
@@ -2293,7 +2291,7 @@ void testNonPrivilegedUserMkdirCreateBucket() throws IOException {
     ACLType userRights = aclConfig.getUserDefaultRights();
     // Construct ACL for world access
     OzoneAcl aclWorldAccess = new OzoneAcl(ACLIdentityType.WORLD, "",
-        userRights, ACCESS);
+        ACCESS, userRights);
     // Construct VolumeArgs, set ACL to world access
     VolumeArgs volumeArgs = VolumeArgs.newBuilder()
         .addAcl(aclWorldAccess)