Merge pull request #155 from kohlschuetter/ck/Inode3

Opaque: Add support for mutable byte buffers and arrays

Author: kofemann

core/src/main/java/org/dcache/nfs/util/Opaque.java

@@ -22,24 +22,43 @@
 import java.nio.ByteBuffer;
 import java.util.Arrays;
 import java.util.Base64;
+import java.util.Objects;
 
 /**
- * Describes something that can be used as a key in {@link java.util.Map} and that can be converted to a {@code byte[]}
- * and a Base64 string representation.
+ * Describes something that can be used as a key for {@link java.util.HashMap} and that can be converted to a
+ * {@code byte[]} and a Base64 string representation.
+ * <p>
+ * Note that {@link Opaque}s that are <em>stored</em> in {@link java.util.HashMap} need to be immutable. Call
+ * {@link #toImmutableOpaque()} when necessary (e.g., when using {@link java.util.HashMap#put(Object, Object)},
+ * {@link java.util.HashMap#computeIfAbsent(Object, java.util.function.Function)}, etc.
  */
 public interface Opaque {
     /**
-     * Returns an {@link Opaque} instance based on a copy of the given bytes.
+     * Returns an immutable {@link Opaque} instance based on a copy of the given bytes.
      * 
      * @param bytes The bytes.
      * @return The {@link Opaque} instance.
      */
     static Opaque forBytes(byte[] bytes) {
-        return new OpaqueImpl(bytes.clone());
+        return new OpaqueImmutableImpl(bytes.clone());
     }
 
     /**
-     * Returns an {@link Opaque} instance based on a copy of the {@code length} bytes from the given {@link ByteBuffer}.
+     * Returns an mutable {@link Opaque} instance based on the given byte array.
+     * <p>
+     * Note that the returned {@link Opaque} is typically not suitable for <em>storing</em> in a
+     * {@link java.util.HashMap}, but merely for lookups. Call {@link #toImmutableOpaque()} when necessary.
+     * 
+     * @param bytes The bytes.
+     * @return The {@link Opaque} instance.
+     */
+    static Opaque forMutableByteArray(byte[] bytes) {
+        return new OpaqueImpl(bytes);
+    }
+
+    /**
+     * Returns an immutable {@link Opaque} instance based on a copy of the {@code length} bytes from the given
+     * {@link ByteBuffer}.
      * 
      * @param buf The buffer.
      * @param length The number of bytes.
@@ -49,7 +68,24 @@ static Opaque forBytes(ByteBuffer buf, int length) {
         byte[] bytes = new byte[length];
         buf.get(bytes);
 
-        return new OpaqueImpl(bytes);
+        return new OpaqueImmutableImpl(bytes);
+    }
+
+    /**
+     * Returns a <em>mutable</em> {@link Opaque} instance backed on the byte contents of the given {@link ByteBuffer},
+     * for the given number of bytes starting from the given absolute index.
+     * <p>
+     * Note that the returned {@link Opaque} is typically not suitable for <em>storing</em> in a
+     * {@link java.util.HashMap}, but merely for lookups. Call {@link #toImmutableOpaque()} when necessary.
+     * 
+     * @param buf The buffer backing the {@link Opaque}.
+     * @param index The absolute index to start from.
+     * @param length The number of bytes.
+     * @return The {@link Opaque} instance.
+     * @see #toImmutableOpaque()
+     */
+    static Opaque forMutableByteBuffer(ByteBuffer buf, int index, int length) {
+        return new OpaqueBufferImpl(buf, index, length);
     }
 
     /**
@@ -102,6 +138,13 @@ static boolean defaultEquals(Opaque obj, Object other) {
      */
     String toBase64();
 
+    /**
+     * Returns an immutable {@link Opaque}, which may be the instance itself if it is already immutable.
+     * 
+     * @return An immutable opaque.
+     */
+    Opaque toImmutableOpaque();
+
     /**
      * Writes the bytes of this {@link Opaque} to the given {@link ByteBuffer}.
      * 
@@ -131,11 +174,10 @@ default void putBytes(ByteBuffer buf) {
     @Override
     boolean equals(Object o);
 
-    final class OpaqueImpl implements Opaque {
-        private final byte[] _opaque;
-        private String base64 = null;
+    class OpaqueImpl implements Opaque {
+        final byte[] _opaque;
 
-        private OpaqueImpl(byte[] opaque) {
+        OpaqueImpl(byte[] opaque) {
             _opaque = opaque;
         }
 
@@ -145,21 +187,22 @@ public byte[] toBytes() {
         }
 
         @Override
-        public String toBase64() {
-            if (base64 == null) {
-                base64 = Base64.getEncoder().withoutPadding().encodeToString(_opaque);
-            }
-            return base64;
+        public int hashCode() {
+            return Arrays.hashCode(_opaque);
         }
 
         @Override
-        public void putBytes(ByteBuffer buf) {
-            buf.put(_opaque);
+        public String toBase64() {
+            return toBase64Impl();
+        }
+
+        protected String toBase64Impl() {
+            return Base64.getEncoder().withoutPadding().encodeToString(_opaque);
         }
 
         @Override
-        public int hashCode() {
-            return Arrays.hashCode(_opaque);
+        public void putBytes(ByteBuffer buf) {
+            buf.put(_opaque);
         }
 
         @Override
@@ -173,6 +216,19 @@ public boolean equals(Object o) {
 
             if (o instanceof OpaqueImpl) {
                 return Arrays.equals(_opaque, ((OpaqueImpl) o)._opaque);
+            } else if (o instanceof OpaqueBufferImpl) {
+                OpaqueBufferImpl other = (OpaqueBufferImpl) o;
+                if (other.numBytes() != _opaque.length) {
+                    return false;
+                }
+                ByteBuffer otherBuf = other.buf;
+                int otherIndex = other.index;
+                for (int i = 0, n = _opaque.length, oi = otherIndex; i < n; i++, oi++) {
+                    if (_opaque[i] != otherBuf.get(oi)) {
+                        return false;
+                    }
+                }
+                return true;
             } else {
                 return Arrays.equals(_opaque, ((Opaque) o).toBytes());
             }
@@ -192,5 +248,125 @@ public String toString() {
         public int numBytes() {
             return _opaque.length;
         }
+
+        @Override
+        public Opaque toImmutableOpaque() {
+            return Opaque.forBytes(_opaque);
+        }
+    }
+
+    final class OpaqueImmutableImpl extends OpaqueImpl {
+        private String base64 = null;
+        private int hashCode;
+
+        protected OpaqueImmutableImpl(byte[] opaque) {
+            super(opaque);
+        }
+
+        @Override
+        public int hashCode() {
+            if (hashCode == 0) {
+                hashCode = Arrays.hashCode(_opaque);
+            }
+            return hashCode;
+        }
+
+        @Override
+        public String toBase64() {
+            if (base64 == null) {
+                base64 = toBase64Impl();
+            }
+            return base64;
+        }
+
+        @Override
+        public Opaque toImmutableOpaque() {
+            return this;
+        }
+    }
+
+    final class OpaqueBufferImpl implements Opaque {
+        private final ByteBuffer buf;
+        private final int index;
+        private final int length;
+
+        private OpaqueBufferImpl(ByteBuffer buf, int index, int length) {
+            this.buf = Objects.requireNonNull(buf);
+            this.index = index;
+            this.length = length;
+        }
+
+        @Override
+        public byte[] toBytes() {
+            byte[] bytes = new byte[length];
+            buf.get(index, bytes);
+            return bytes;
+        }
+
+        @Override
+        public int numBytes() {
+            return length;
+        }
+
+        @Override
+        public String toBase64() {
+            return Base64.getEncoder().withoutPadding().encodeToString(toBytes());
+        }
+
+        @Override
+        public Opaque toImmutableOpaque() {
+            return Opaque.forBytes(toBytes());
+        }
+
+        @Override
+        public int hashCode() {
+            int result = 1;
+            for (int i = index, n = index + length; i < n; i++) {
+                byte element = buf.get(i);
+                result = 31 * result + element;
+            }
+
+            return result;
+        }
+
+        @Override
+        public boolean equals(Object o) {
+            if (o == this) {
+                return true;
+            }
+            if (!(o instanceof Opaque)) {
+                return false;
+            }
+            if (length != ((Opaque) o).numBytes()) {
+                return false;
+            }
+
+            if (o instanceof OpaqueImpl) {
+                byte[] otherBytes = ((OpaqueImpl) o)._opaque;
+                for (int i = index, n = index + length, oi = 0; i < n; i++, oi++) {
+                    if (buf.get(i) != otherBytes[oi]) {
+                        return false;
+                    }
+                }
+                return true;
+            } else if (o instanceof OpaqueBufferImpl) {
+                OpaqueBufferImpl other = (OpaqueBufferImpl) o;
+                ByteBuffer otherBuf = other.buf;
+                int otherIndex = other.index;
+                for (int i = index, n = index + length, oi = otherIndex; i < n; i++, oi++) {
+                    if (buf.get(i) != otherBuf.get(oi)) {
+                        return false;
+                    }
+                }
+                return true;
+            } else {
+                return toImmutableOpaque().equals(o);
+            }
+        }
+
+        @Override
+        public String toString() {
+            return super.toString() + "[" + toBase64() + "]";
+        }
     }
 }

core/src/main/java/org/dcache/nfs/v4/FileTracker.java

@@ -242,7 +242,7 @@ public OpenRecord addOpen(NFS4Client client, StateOwner owner, Inode inode, int
         // client explicitly requested write delegation
         boolean wantWriteDelegation = (shareAccess & nfs4_prot.OPEN4_SHARE_ACCESS_WANT_WRITE_DELEG) != 0;
 
-        Opaque fileId = inode.getFileIdKey();
+        final Opaque fileId = inode.getFileIdKey().toImmutableOpaque();
         Lock lock = filesLock.get(fileId);
         lock.lock();
         try {

core/src/main/java/org/dcache/nfs/v4/nlm/SimpleLm.java

@@ -58,62 +58,51 @@ public SimpleLm(int concurrency) {
     /**
      * Exclusive lock on objects locks.
      */
-    private final ConcurrentHashMap<String, List<NlmLock>> locks = new ConcurrentHashMap<>();
+    private final ConcurrentHashMap<Opaque, List<NlmLock>> locks = new ConcurrentHashMap<>();
 
     @Override
     protected Lock getObjectLock(Opaque objId) {
-        String key = toKey(objId);
-        return objLock.get(key);
+        return objLock.get(objId);
     }
 
     @Override
     protected Collection<NlmLock> getActiveLocks(Opaque objId) {
-        String key = toKey(objId);
-        return locks.getOrDefault(key, Collections.emptyList());
+        return locks.getOrDefault(objId, Collections.emptyList());
     }
 
     @Override
     protected void add(Opaque objId, NlmLock lock) {
-        String key = toKey(objId);
-        Collection<NlmLock> l = locks.computeIfAbsent(key, k -> new ArrayList<>());
+        Collection<NlmLock> l = locks.computeIfAbsent(objId.toImmutableOpaque(), k -> new ArrayList<>());
         l.add(lock);
     }
 
     @Override
     protected boolean remove(Opaque objId, NlmLock lock) {
-        String key = toKey(objId);
-        Collection<NlmLock> l = locks.get(key);
+        Collection<NlmLock> l = locks.get(objId);
         boolean isRemoved = false;
         if (l != null) {
             isRemoved = l.remove(lock);
             if (l.isEmpty()) {
-                locks.remove(key);
+                locks.remove(objId);
             }
         }
         return isRemoved;
     }
 
     @Override
     protected void addAll(Opaque objId, Collection<NlmLock> locks) {
-        String key = toKey(objId);
-        Collection<NlmLock> l = this.locks.computeIfAbsent(key, k -> new ArrayList<>());
+        Collection<NlmLock> l = this.locks.computeIfAbsent(objId.toImmutableOpaque(), k -> new ArrayList<>());
         l.addAll(locks);
     }
 
     @Override
     protected void removeAll(Opaque objId, Collection<NlmLock> locks) {
-        String key = toKey(objId);
-        Collection<NlmLock> l = this.locks.get(key);
+        Collection<NlmLock> l = this.locks.get(objId);
         if (l != null) {
             l.removeAll(locks);
             if (l.isEmpty()) {
-                this.locks.remove(key);
+                this.locks.remove(objId);
             }
         }
     }
-
-    private final String toKey(Opaque objId) {
-        return objId.toBase64();
-    }
-
 }