Repository: vert-x3/vertx-web
Issue: ConcurrentLRUCache shouldn't be named concurrent or LRU?

Hey!

I happened to notice `io.vertx.ext.web.impl.ConcurrentLRUCache` class in this project. Is this still being maintained and/or used somewhere? I couldn't find any references to this class in the codebase except in the tests here, or any discussions related to this.

I was about to start using this for a simple cache, but after having done cursory review, it looks like this class doesn't do what the name implies.

First and foremost, it is based on ConcurrentHashMap which is thread-safe, but it also relies internally on ArrayDeque, which is _not_ thread-safe. There doesn't seem to be any synchronization happening either. The "concurrent" in name would imply that this would be thread-safe.

Second, it doesn't seem to be LRU either. If I read the code correctly, it deletes the earliest inserted item once reaching the max size, instead of the least recently used item.

Not necessarily anything that needs to be done about this - just heads-up that the class name is quite confusing as it sets an expectation for something completely else.. :)

Changed files:
vertx-web/src/test/java/io/vertx/ext/web/ConcurrentLRUCacheTest.java, vertx-web/src/main/java/io/vertx/ext/web/impl/ConcurrentLRUCache.java

diff --git a/vertx-web/src/main/java/io/vertx/ext/web/impl/ConcurrentLRUCache.java b/vertx-web/src/main/java/io/vertx/ext/web/impl/ConcurrentLRUCache.java
deleted file mode 100644
index e4b91f050..000000000
--- a/vertx-web/src/main/java/io/vertx/ext/web/impl/ConcurrentLRUCache.java
+++ /dev/null
@@ -1,163 +0,0 @@
-/*
- * Copyright 2014 Red Hat, Inc.
- *
- *  All rights reserved. This program and the accompanying materials
- *  are made available under the terms of the Eclipse Public License v1.0
- *  and Apache License v2.0 which accompanies this distribution.
- *
- *  The Eclipse Public License is available at
- *  http://www.eclipse.org/legal/epl-v10.html
- *
- *  The Apache License v2.0 is available at
- *  http://www.opensource.org/licenses/apache2.0.php
- *
- *  You may elect to redistribute this code under either of these licenses.
- */
-
-package io.vertx.ext.web.impl;
-
-import java.util.ArrayDeque;
-import java.util.Map;
-import java.util.Queue;
-import java.util.concurrent.ConcurrentHashMap;
-import java.util.function.BiFunction;
-
-/**
- * Concurrent LRU cache.
- *
- * Note that remove operation on this structure is SLOW! Avoid using it.
- *
- * @author <a href="http://tfox.org">Tim Fox</a>
- */
-public class ConcurrentLRUCache<K, V> extends ConcurrentHashMap<K, V> {
-
-  private int maxSize;
-  private final Queue<K> queue = new ArrayDeque<>();
-
-  public ConcurrentLRUCache(int maxSize) {
-    this.maxSize = maxSize;
-    checkSize();
-  }
-
-  public ConcurrentLRUCache(int initialCapacity, int maxSize) {
-    super(initialCapacity);
-    this.maxSize = maxSize;
-    checkSize();
-  }
-
-  public ConcurrentLRUCache(Map<? extends K, ? extends V> m, int maxSize) {
-    super(m);
-    this.maxSize = maxSize;
-    checkSize();
-    for (K k: m.keySet()) {
-      entryAdded(k);
-    }
-    checkRemoveOldest();
-  }
-
-  public ConcurrentLRUCache(int initialCapacity, float loadFactor, int maxSize) {
-    super(initialCapacity, loadFactor);
-    this.maxSize = maxSize;
-    checkSize();
-  }
-
-  public ConcurrentLRUCache(int initialCapacity, float loadFactor, int concurrencyLevel, int maxSize) {
-    super(initialCapacity, loadFactor, concurrencyLevel);
-    this.maxSize = maxSize;
-    checkSize();
-  }
-
-  public void setMaxSize(int maxSize) {
-    this.maxSize = maxSize;
-    checkRemoveOldest();
-  }
-
-  @Override
-  public V put(K key, V value) {
-    V v = super.put(key, value);
-    if (v == null) {
-      entryAdded(key);
-    }
-    checkRemoveOldest();
-    return v;
-  }
-
-  @Override
-  public void putAll(Map<? extends K, ? extends V> m) {
-    for (K k: m.keySet()) {
-      if (!super.containsKey(k)) {
-        entryAdded(k);
-      }
-    }
-    super.putAll(m);
-    checkRemoveOldest();
-  }
-
-  @Override
-  public V remove(Object key) {
-    V v = super.remove(key);
-    if (v != null) {
-      entryRemoved(key);
-    }
-    return v;
-  }
-
-  @Override
-  public void clear() {
-    super.clear();
-    queue.clear();
-  }
-
-  @Override
-  public V putIfAbsent(K key, V value) {
-    V v = super.putIfAbsent(key, value);
-    if (v == null) {
-      entryAdded(key);
-    }
-    checkRemoveOldest();
-    return v;
-  }
-
-  @Override
-  public boolean remove(Object key, Object value) {
-    boolean removed = super.remove(key, value);
-    if (removed) {
-      entryRemoved(value);
-    }
-    return removed;
-  }
-
-  @Override
-  public V merge(K key, V value, BiFunction<? super V, ? super V, ? extends V> remappingFunction) {
-    throw new UnsupportedOperationException();
-  }
-
-  private void entryAdded(K k) {
-    queue.add(k);
-  }
-
-  private void entryRemoved(Object o) {
-    if (!queue.remove(o)) {
-      throw new IllegalStateException("Failed to remove");
-    }
-  }
-  
-  private void checkRemoveOldest() {
-    while (queue.size() > maxSize) {
-      K k = queue.poll();
-      if (k != null) {
-        super.remove(k);
-      }
-    }
-  }
-
-  private void checkSize() {
-    if (maxSize < 1) {
-      throw new IllegalArgumentException("maxSize must be >= 1");
-    }
-  }
-
-  public int queueSize() {
-    return queue.size();
-  }
-}
diff --git a/vertx-web/src/test/java/io/vertx/ext/web/ConcurrentLRUCacheTest.java b/vertx-web/src/test/java/io/vertx/ext/web/ConcurrentLRUCacheTest.java
deleted file mode 100644
index 1ff39d68e..000000000
--- a/vertx-web/src/test/java/io/vertx/ext/web/ConcurrentLRUCacheTest.java
+++ /dev/null
@@ -1,72 +0,0 @@
-/*
- * Copyright 2014 Red Hat, Inc.
- *
- *  All rights reserved. This program and the accompanying materials
- *  are made available under the terms of the Eclipse Public License v1.0
- *  and Apache License v2.0 which accompanies this distribution.
- *
- *  The Eclipse Public License is available at
- *  http://www.eclipse.org/legal/epl-v10.html
- *
- *  The Apache License v2.0 is available at
- *  http://www.opensource.org/licenses/apache2.0.php
- *
- *  You may elect to redistribute this code under either of these licenses.
- */
-
-package io.vertx.ext.web;
-
-import io.vertx.ext.web.impl.ConcurrentLRUCache;
-import org.junit.Test;
-
-import java.util.Map;
-
-import static org.junit.Assert.*;
-
-/**
- * @author <a href="http://tfox.org">Tim Fox</a>
- */
-public class ConcurrentLRUCacheTest extends LRUCacheTestBase {
-
-  @Override
-  protected Map<String, String> createCache() {
-    return new ConcurrentLRUCache<>(maxSize);
-  }
-
-  @Test
-  public void testPut() {
-    super.testPut();
-    assertEquals(maxSize, ((ConcurrentLRUCache)cache).queueSize());
-  }
-
-  @Test
-  public void testRemove() {
-    super.testRemove();
-    assertEquals(0, ((ConcurrentLRUCache)cache).queueSize());
-  }
-
-  @Test(expected=IllegalArgumentException.class)
-  public void testCacheInvalidSize1() {
-    new ConcurrentLRUCache<>(0);
-  }
-
-  @Test(expected=IllegalArgumentException.class)
-  public void testCacheInvalidSize2() {
-    new ConcurrentLRUCache<>(-1);
-  }
-
-  @Test
-  public void testIncreaseMaxSize() {
-    for (int i = 0; i < maxSize; i++) {
-      cache.put("key" + i, "value" + i);
-    }
-    assertEquals(maxSize, cache.size());
-    ConcurrentLRUCache<String, String> ccache = (ConcurrentLRUCache<String, String>)cache;
-    ccache.setMaxSize(maxSize + 10);
-    for (int i = maxSize; i < maxSize + 10; i++) {
-      cache.put("key" + i, "value" + i);
-    }
-    assertEquals(maxSize + 10, cache.size());
-  }
-
-}