HDDS-10745. Do not use BitSet for OzoneAcl.aclBitSet.

hadoop-hdds/common/src/main/java/com/google/protobuf/Proto2Utils.java

@@ -0,0 +1,32 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.google.protobuf;
+
+/** Utilities for protobuf v2. */
+public final class Proto2Utils {
+  /**
+   * Similar to {@link ByteString#copyFrom(byte[])} except that this method does not copy.
+   * This method is safe only if the content of the array remains unchanged.
+   * Otherwise, it violates the immutability of {@link ByteString}.
+   */
+  public static ByteString unsafeByteString(byte[] array) {
+    return new LiteralByteString(array);
+  }
+
+  private Proto2Utils() { }
+}

hadoop-hdds/common/src/main/java/com/google/protobuf/package-info.java

@@ -0,0 +1,22 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+/**
+ * Classes using the protobuf internal APIs.
+ */
+package com.google.protobuf;

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

@@ -22,20 +22,24 @@
 import com.fasterxml.jackson.annotation.JsonIgnore;
 import com.google.common.annotations.VisibleForTesting;
 import com.google.protobuf.ByteString;
+import com.google.protobuf.Proto2Utils;
 import net.jcip.annotations.Immutable;
 import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OzoneAclInfo;
 import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OzoneAclInfo.OzoneAclScope;
 import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OzoneAclInfo.OzoneAclType;
 import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer.ACLIdentityType;
 import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer.ACLType;
+import org.apache.ratis.util.MemoizedSupplier;
 
 import java.util.ArrayList;
 import java.util.BitSet;
+import java.util.Collections;
 import java.util.EnumSet;
 import java.util.List;
 import java.util.Objects;
-import java.util.function.Consumer;
-import java.util.stream.Collectors;
+import java.util.function.Function;
+import java.util.function.IntFunction;
+import java.util.function.Supplier;
 
 import static org.apache.hadoop.ozone.security.acl.IAccessAuthorizer.ACLType.ALL;
 import static org.apache.hadoop.ozone.security.acl.IAccessAuthorizer.ACLType.NONE;
@@ -57,48 +61,63 @@ public class OzoneAcl {
   private final ACLIdentityType type;
   private final String name;
   @JsonIgnore
-  private final BitSet aclBitSet;
+  private final int aclBits;
   private final AclScope aclScope;
-  private static final List<ACLType> EMPTY_LIST = new ArrayList<>(0);
+
+  @JsonIgnore
+  private final Supplier<String> toStringMethod;
+  @JsonIgnore
+  private final Supplier<Integer> hashCodeMethod;
 
   public OzoneAcl(ACLIdentityType type, String name, AclScope scope, ACLType... acls) {
-    this(type, name, scope, bitSetOf(acls));
+    this(type, name, scope, toInt(acls));
   }
 
   public OzoneAcl(ACLIdentityType type, String name, AclScope scope, EnumSet<ACLType> acls) {
-    this(type, name, scope, bitSetOf(acls.toArray(new ACLType[0])));
+    this(type, name, scope, toInt(acls));
   }
 
-  private OzoneAcl(ACLIdentityType type, String name, AclScope scope, BitSet acls) {
+  private OzoneAcl(ACLIdentityType type, String name, AclScope scope, int acls) {
     this.name = validateNameAndType(type, name);
     this.type = type;
     this.aclScope = scope;
-    this.aclBitSet = acls;
+    this.aclBits = acls;
+
+    this.toStringMethod = MemoizedSupplier.valueOf(() -> getType() + ":" + getName() + ":"
+        + ACLType.getACLString(BitSet.valueOf(getAclByteString().asReadOnlyByteBuffer())) + "[" + getAclScope() + "]");
+    this.hashCodeMethod = MemoizedSupplier.valueOf(() -> Objects.hash(getName(),
+        BitSet.valueOf(getAclByteString().asReadOnlyByteBuffer()), getType().toString(), getAclScope()));
   }
 
-  private static BitSet bitSetOf(ACLType... acls) {
-    BitSet bits = new BitSet();
-    if (acls != null && acls.length > 0) {
-      for (ACLType acl : acls) {
-        bits.set(acl.ordinal());
-      }
-    }
-    return bits;
+
+  private static int toInt(int aclTypeOrdinal) {
+    return 1 << aclTypeOrdinal;
   }
 
-  private static BitSet validateAndCopy(BitSet acls) {
-    Objects.requireNonNull(acls);
+  private static int toInt(ACLType acl) {
+    return toInt(acl.ordinal());
+  }
 
-    if (acls.cardinality() > ACLType.getNoOfAcls()) {
-      throw new IllegalArgumentException("Acl bitset passed has unexpected " +
-          "size. bitset size:" + acls.cardinality() + ", bitset:" + acls);
+  private static int toInt(ACLType[] acls) {
+    if (acls == null) {
+      return 0;
     }
-
-    return copyBitSet(acls);
+    int value = 0;
+    for (ACLType acl : acls) {
+      value |= toInt(acl);
+    }
+    return value;
   }
 
-  private static BitSet copyBitSet(BitSet acls) {
-    return (BitSet) acls.clone();
+  private static int toInt(Iterable<ACLType> acls) {
+    if (acls == null) {
+      return 0;
+    }
+    int value = 0;
+    for (ACLType acl : acls) {
+      value |= toInt(acl);
+    }
+    return value;
   }
 
   private static String validateNameAndType(ACLIdentityType type, String name) {
@@ -124,7 +143,7 @@ private static String validateNameAndType(ACLIdentityType type, String name) {
 
   public OzoneAcl withScope(final AclScope scope) {
     return scope == aclScope ? this
-        : new OzoneAcl(type, name, scope, copyBitSet(aclBitSet));
+        : new OzoneAcl(type, name, scope, aclBits);
   }
 
   /**
@@ -199,14 +218,21 @@ public static OzoneAclInfo toProtobuf(OzoneAcl acl) {
         .setName(acl.getName())
         .setType(OzoneAclType.valueOf(acl.getType().name()))
         .setAclScope(OzoneAclScope.valueOf(acl.getAclScope().name()))
-        .setRights(ByteString.copyFrom(acl.getAclByteArray()));
+        .setRights(acl.getAclByteString());
     return builder.build();
   }
 
   public static OzoneAcl fromProtobuf(OzoneAclInfo protoAcl) {
-    BitSet aclRights = BitSet.valueOf(protoAcl.getRights().toByteArray());
+    final byte[] bytes = protoAcl.getRights().toByteArray();
+    if (bytes.length > 4) {
+      throw new AssertionError("Expected at most 4 bytes but got " + bytes.length);
+    }
+    int aclRights = 0;
+    for (int i = 0; i < bytes.length; i++) {
+      aclRights |= (bytes[i] & 0xff) << (i * 8);
+    }
     return new OzoneAcl(ACLIdentityType.valueOf(protoAcl.getType().name()), protoAcl.getName(),
-        AclScope.valueOf(protoAcl.getAclScope().name()), validateAndCopy(aclRights));
+        AclScope.valueOf(protoAcl.getAclScope().name()), aclRights);
   }
 
   public AclScope getAclScope() {
@@ -215,8 +241,7 @@ public AclScope getAclScope() {
 
   @Override
   public String toString() {
-    return type + ":" + name + ":" + ACLType.getACLString(aclBitSet)
-        + "[" + aclScope + "]";
+    return toStringMethod.get();
   }
 
   /**
@@ -230,8 +255,7 @@ public String toString() {
    */
   @Override
   public int hashCode() {
-    return Objects.hash(this.getName(), aclBitSet,
-                        this.getType().toString(), this.getAclScope());
+    return hashCodeMethod.get();
   }
 
   /**
@@ -245,47 +269,62 @@ public String getName() {
 
   @JsonIgnore
   public boolean isEmpty() {
-    return aclBitSet.isEmpty();
+    return aclBits == 0;
   }
 
   @VisibleForTesting
   public boolean isSet(ACLType acl) {
-    return aclBitSet.get(acl.ordinal());
+    return (aclBits & toInt(acl)) != 0;
   }
 
   public boolean checkAccess(ACLType acl) {
     return (isSet(acl) || isSet(ALL)) && !isSet(NONE);
   }
 
   public OzoneAcl add(OzoneAcl other) {
-    return apply(bits -> bits.or(other.aclBitSet));
+    return apply(bits -> bits | other.aclBits);
   }
 
   public OzoneAcl remove(OzoneAcl other) {
-    return apply(bits -> bits.andNot(other.aclBitSet));
+    return apply(bits -> bits & ~other.aclBits);
   }
 
-  /** @return copy of this {@code OzoneAcl} after applying the given {@code op},
-   * or this instance if {@code op} makes no difference */
-  private OzoneAcl apply(Consumer<BitSet> op) {
-    final BitSet cloneBits = copyBitSet(aclBitSet);
-    op.accept(cloneBits);
-    return cloneBits.equals(aclBitSet)
+  private OzoneAcl apply(IntFunction<Integer> op) {
+    int applied = op.apply(aclBits);
+    return applied == aclBits
         ? this
-        : new OzoneAcl(type, name, aclScope, cloneBits);
+        : new OzoneAcl(type, name, aclScope, applied);
+  }
+
+  @JsonIgnore
+  public ByteString getAclByteString() {
+    // only first 9 bits are used currently
+    final byte first = (byte) aclBits;
+    final byte second = (byte) (aclBits >>> 8);
+    final byte[] bytes = second != 0 ? new byte[]{first, second} : new byte[]{first};
+    return Proto2Utils.unsafeByteString(bytes);
   }
 
   @JsonIgnore
-  public byte[] getAclByteArray() {
-    return aclBitSet.toByteArray();
+  public List<String> getAclStringList() {
+    return getAclList(aclBits, ACLType::name);
   }
 
   public List<ACLType> getAclList() {
-    if (aclBitSet != null) {
-      return aclBitSet.stream().mapToObj(a ->
-          ACLType.values()[a]).collect(Collectors.toList());
+    return getAclList(aclBits, Function.identity());
+  }
+
+  private static <T> List<T> getAclList(int aclBits, Function<ACLType, T> converter) {
+    if (aclBits == 0) {
+      return Collections.emptyList();
+    }
+    final List<T> toReturn = new ArrayList<>(Integer.bitCount(aclBits));
+    for (int i = 0; i < ACLType.values().length; i++) {
+      if ((toInt(i) & aclBits) != 0) {
+        toReturn.add(converter.apply(ACLType.values()[i]));
+      }
     }
-    return EMPTY_LIST;
+    return Collections.unmodifiableList(toReturn);
   }
 
   /**
@@ -316,7 +355,7 @@ public boolean equals(Object obj) {
     OzoneAcl otherAcl = (OzoneAcl) obj;
     return otherAcl.getName().equals(this.getName()) &&
         otherAcl.getType().equals(this.getType()) &&
-        Objects.equals(aclBitSet, otherAcl.aclBitSet) &&
+        this.aclBits == otherAcl.aclBits &&
         otherAcl.getAclScope().equals(this.getAclScope());
   }
 

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

@@ -1,4 +1,4 @@
-/**
+/*
  * Licensed to the Apache Software Foundation (ASF) under one or more
  * contributor license agreements.  See the NOTICE file distributed with this
  * work for additional information regarding copyright ownership.  The ASF
@@ -63,6 +63,13 @@ enum ACLType {
     ALL,
     NONE;
     private static int length = ACLType.values().length;
+    static {
+      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() {

hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/helpers/TestOzoneAclUtil.java

@@ -34,7 +34,6 @@
 import static org.apache.hadoop.ozone.OzoneAcl.AclScope.DEFAULT;
 import static org.apache.hadoop.ozone.security.acl.IAccessAuthorizer.ACLIdentityType.GROUP;
 import static org.apache.hadoop.ozone.security.acl.IAccessAuthorizer.ACLIdentityType.USER;
-import static org.junit.jupiter.api.Assertions.assertArrayEquals;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNotEquals;
@@ -232,7 +231,6 @@ public void testAddDefaultAcl() {
     assertEquals(2, ozoneAcls.size());
     assertNotEquals(ozoneAcls.get(0).getAclScope(),
         ozoneAcls.get(1).getAclScope());
-    assertArrayEquals(ozoneAcls.get(0).getAclByteArray(),
-        ozoneAcls.get(1).getAclByteArray());
+    assertEquals(ozoneAcls.get(0).getAclByteString(), ozoneAcls.get(1).getAclByteString());
   }
 }

hadoop-ozone/interface-storage/src/main/java/org/apache/hadoop/ozone/om/helpers/OzoneAclStorage.java

@@ -17,8 +17,6 @@
  */
 package org.apache.hadoop.ozone.om.helpers;
 
-import com.google.protobuf.ByteString;
-
 import java.util.BitSet;
 import java.util.EnumSet;
 import java.util.List;
@@ -55,7 +53,7 @@ public static OzoneAclInfo toProtobuf(OzoneAcl acl) {
         .setName(acl.getName())
         .setType(OzoneAclType.valueOf(acl.getType().name()))
         .setAclScope(OzoneAclScope.valueOf(acl.getAclScope().name()))
-        .setRights(ByteString.copyFrom(acl.getAclByteArray()));
+        .setRights(acl.getAclByteString());
     return builder.build();
   }
 

hadoop-ozone/interface-storage/src/test/java/org/apache/hadoop/ozone/om/helpers/TestOmPrefixInfo.java

@@ -40,7 +40,7 @@ public class TestOmPrefixInfo {
   private static OzoneManagerStorageProtos.OzoneAclInfo buildTestOzoneAclInfo(
       String aclString) {
     OzoneAcl oacl = OzoneAcl.parseAcl(aclString);
-    ByteString rights = ByteString.copyFrom(oacl.getAclByteArray());
+    final ByteString rights = oacl.getAclByteString();
     return OzoneManagerStorageProtos.OzoneAclInfo.newBuilder()
         .setType(OzoneManagerStorageProtos.OzoneAclInfo.OzoneAclType.USER)
         .setName(oacl.getName())

hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/types/AclMetadata.java

@@ -130,9 +130,7 @@ public static AclMetadata fromOzoneAcl(OzoneAcl ozoneAcl) {
     return builder.withType(ozoneAcl.getType().toString().toUpperCase())
         .withName(ozoneAcl.getName())
         .withScope(ozoneAcl.getAclScope().toString().toUpperCase())
-        .withAclList(ozoneAcl.getAclList().stream().map(Enum::toString)
-            .map(String::toUpperCase)
-            .collect(Collectors.toList()))
+        .withAclList(ozoneAcl.getAclStringList())
         .build();
   }
 }