Eliminate unnecessary column visibility creation

warehouse/query-core/src/main/java/datawave/query/attributes/Attribute.java

@@ -1,8 +1,11 @@
 package datawave.query.attributes;
 
+import static datawave.query.Constants.EMPTY_BYTES;
+
 import java.io.DataInput;
 import java.io.DataOutput;
 import java.io.IOException;
+import java.util.Arrays;
 import java.util.Collection;
 
 import org.apache.accumulo.core.data.ArrayByteSequence;
@@ -28,6 +31,7 @@ public abstract class Attribute<T extends Comparable<T>> implements WritableComp
 
     private static final Logger log = Logger.getLogger(Attribute.class);
     private static final Text EMPTY_TEXT = new Text();
+    private static final ByteSequence EMPTY_BYTE_SEQUENCE = new ArrayByteSequence(new byte[0]);
 
     /**
      * The metadata for this attribute. Really only the column visibility and timestamp are preserved in this metadata when serializing and deserializing.
@@ -59,6 +63,20 @@ public ColumnVisibility getColumnVisibility() {
         return Constants.EMPTY_VISIBILITY;
     }
 
+    /**
+     * Get a copy byte array that backs the {@link ColumnVisibility}. This avoids the expensive parse call found in the default constructor for the
+     * ColumnVisibility.
+     *
+     * @return a copy of the byte array that backs the column visibility
+     */
+    public byte[] getColumnVisibilityBytes() {
+        if (isMetadataSet()) {
+            byte[] data = metadata.getColumnVisibilityData().toArray();
+            return Arrays.copyOf(data, data.length);
+        }
+        return EMPTY_BYTES;
+    }
+
     public void setColumnVisibility(ColumnVisibility columnVisibility) {
         if (isMetadataSet()) {
             metadata = new Key(metadata.getRow(), metadata.getColumnFamily(), metadata.getColumnQualifier(), columnVisibility, metadata.getTimestamp());
@@ -94,7 +112,22 @@ protected void setMetadata(ColumnVisibility vis, long ts) {
         }
     }
 
-    private static final ByteSequence EMPTY_BYTE_SEQUENCE = new ArrayByteSequence(new byte[0]);
+    /**
+     * Set the metadata for this attribute. This method allows a trusted caller to directly set the column visibility via a byte array. Assumes that the bytes
+     * came from a {@link ColumnVisibility} object which has already parsed, verified and flattened the visibility string.
+     *
+     * @param vis
+     *            the column visibility bytes
+     * @param ts
+     *            the timestamp
+     */
+    protected void setMetadata(byte[] vis, long ts) {
+        if (metadata != null) {
+            metadata = new Key(metadata.getRow().getBytes(), metadata.getColumnFamily().getBytes(), metadata.getColumnQualifier().getBytes(), vis, ts);
+        } else {
+            metadata = new Key(EMPTY_BYTES, EMPTY_BYTES, EMPTY_BYTES, vis, ts);
+        }
+    }
 
     /*
      * Given a key, set the metadata. Expected input keys can be an event key, an fi key, or a tf key. Expected metadata is row=shardid, cf = type\0uid; cq =
@@ -168,7 +201,7 @@ protected void clearMetadata() {
     protected void writeMetadata(DataOutput out) throws IOException {
         out.writeBoolean(isMetadataSet());
         if (isMetadataSet()) {
-            byte[] cvBytes = getColumnVisibility().getExpression();
+            byte[] cvBytes = getColumnVisibilityBytes();
 
             WritableUtils.writeVInt(out, cvBytes.length);
 
@@ -180,7 +213,7 @@ protected void writeMetadata(DataOutput out) throws IOException {
     protected void writeMetadata(Kryo kryo, Output output) {
         output.writeBoolean(isMetadataSet());
         if (isMetadataSet()) {
-            byte[] cvBytes = getColumnVisibility().getExpression();
+            byte[] cvBytes = getColumnVisibilityBytes();
             output.writeInt(cvBytes.length, true);
             output.writeBytes(cvBytes);
             output.writeLong(getTimestamp());
@@ -204,8 +237,10 @@ protected void readMetadata(DataInput in) throws IOException {
     protected void readMetadata(Kryo kryo, Input input) {
         if (input.readBoolean()) {
             int size = input.readInt(true);
-
-            this.setMetadata(new ColumnVisibility(input.readBytes(size)), input.readLong());
+            byte[] cvBytes = input.readBytes(size);
+            long timestamp = input.readLong();
+            // trust the column visibility from the payload
+            setMetadata(cvBytes, timestamp);
         } else {
             this.clearMetadata();
         }

warehouse/query-core/src/main/java/datawave/query/attributes/Cardinality.java

@@ -115,12 +115,12 @@ protected int compareMetadata(Attribute<Cardinality> other) {
                 return -1;
             }
         } else if (this.isMetadataSet()) {
-            byte[] cvBytes = this.getColumnVisibility().getExpression();
+            byte[] cvBytes = this.getColumnVisibilityBytes();
             if (null == cvBytes) {
                 cvBytes = Constants.EMPTY_BYTES;
             }
 
-            byte[] otherCVBytes = other.getColumnVisibility().getExpression();
+            byte[] otherCVBytes = other.getColumnVisibilityBytes();
             if (null == otherCVBytes) {
                 otherCVBytes = Constants.EMPTY_BYTES;
             }

warehouse/query-core/src/test/java/datawave/query/attributes/TypeAttributeTest.java

@@ -2,10 +2,13 @@
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertInstanceOf;
+import static org.junit.jupiter.api.Assertions.assertNotSame;
+import static org.junit.jupiter.api.Assertions.assertSame;
 
 import java.io.ByteArrayOutputStream;
 
 import org.apache.accumulo.core.data.Key;
+import org.apache.accumulo.core.security.ColumnVisibility;
 import org.junit.Test;
 
 import com.esotericsoftware.kryo.Kryo;
@@ -50,4 +53,26 @@ public void testExceptionLeadsToNoOpType() {
         assertInstanceOf(NoOpType.class, type.getType());
         assertEquals("delegate value as string", type.getData().toString());
     }
+
+    @Test
+    public void testColumnVisibilityImmutability() {
+        NoOpType type = new NoOpType("no op value");
+        Key docKey = new Key("shard", "datatype\0uid", "", "VIZ-A");
+
+        TypeAttribute<?> attr = new TypeAttribute<>(type, docKey, false);
+
+        ColumnVisibility first = attr.getColumnVisibility();
+        ColumnVisibility second = attr.getColumnVisibility();
+        // verify we didn't accidentally go down the empty CV path
+        assertEquals("VIZ-A", new String(first.getExpression()));
+        // turns out 'getColumnVisibility' is not immutable
+        assertSame(first.getExpression(), second.getExpression());
+
+        // verify that the following method call is immutable and returns the correct visibility
+        byte[] left = attr.getColumnVisibilityBytes();
+        byte[] right = attr.getColumnVisibilityBytes();
+        assertEquals("VIZ-A", new String(left));
+        assertEquals("VIZ-A", new String(right));
+        assertNotSame(left, right);
+    }
 }

warehouse/query-core/src/test/java/datawave/query/function/serializer/KryoDocumentSerDeTest.java

@@ -2,10 +2,13 @@
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.api.Assertions.fail;
 
 import java.io.ByteArrayInputStream;
+import java.io.IOException;
 
 import org.apache.accumulo.core.data.Key;
+import org.apache.accumulo.core.security.ColumnVisibility;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
@@ -24,6 +27,7 @@
 import datawave.query.attributes.Attribute;
 import datawave.query.attributes.AttributeFactory;
 import datawave.query.attributes.Document;
+import datawave.query.attributes.DocumentKey;
 import datawave.query.function.deserializer.KryoDocumentDeserializer;
 import datawave.query.util.TypeMetadata;
 
@@ -35,6 +39,7 @@ public abstract class KryoDocumentSerDeTest {
     protected final KryoDocumentSerializer serializer = new KryoDocumentSerializer();
     protected final KryoDocumentDeserializer deserializer = new KryoDocumentDeserializer();
 
+    private final ColumnVisibility visibility = new ColumnVisibility("PUBLIC&(FOO|BAR)");
     private final String datatype = "datatype";
     private final Key documentKey = new Key("row", "datatype\0uid");
 
@@ -60,6 +65,9 @@ public void setup() {
         d.put("NUM", createAttribute("NUM", "25"));
         d.put("NUM_LIST", createAttribute("NUM_LIST", "22,23,24"));
         d.put("POINT", createAttribute("POINT", "POINT(10 10)"));
+
+        Key metadata = new Key("row", "datatype\0uid", "", visibility, 0L);
+        d.put(Document.DOCKEY_FIELD_NAME, new DocumentKey(metadata, true));
     }
 
     private TypeMetadata getTypeMetadata() {
@@ -89,7 +97,7 @@ public void testBulkSerialization() {
             int max = 1_000_000;
             for (int i = 1; i <= max; i++) {
                 byte[] bytes = serializer.serialize(d);
-                assertTrue(450 < bytes.length && bytes.length <= 460);
+                assertTrue(500 < bytes.length && bytes.length <= 510, "data length is: " + bytes.length);
             }
         }
     }
@@ -101,9 +109,13 @@ public void testBulkDeserialization() {
 
             int max = 1_000_000;
             for (int i = 1; i <= max; i++) {
-                ByteArrayInputStream bais = new ByteArrayInputStream(data);
-                Document d = deserializer.deserialize(bais);
-                assertEquals(12, d.size());
+                try (var bais = new ByteArrayInputStream(data)) {
+                    Document d = deserializer.deserialize(bais);
+                    assertEquals(13, d.size());
+                } catch (IOException e) {
+                    fail("failed to deserialize document");
+                    throw new RuntimeException(e);
+                }
             }
         }
     }