#949 Create expiring map type + integrate it into recovery code service

src/main/java/fr/xephi/authme/service/RecoveryCodeService.java

@@ -1,38 +1,38 @@
 package fr.xephi.authme.service;
 
-import com.google.common.annotations.VisibleForTesting;
+import fr.xephi.authme.initialization.HasCleanup;
 import fr.xephi.authme.initialization.SettingsDependent;
-import fr.xephi.authme.util.RandomStringUtils;
 import fr.xephi.authme.settings.Settings;
 import fr.xephi.authme.settings.properties.SecuritySettings;
+import fr.xephi.authme.util.ExpiringMap;
+import fr.xephi.authme.util.RandomStringUtils;
 
 import javax.inject.Inject;
-import java.util.Map;
+import java.util.concurrent.TimeUnit;
-import java.util.concurrent.ConcurrentHashMap;
 
 import static fr.xephi.authme.settings.properties.SecuritySettings.RECOVERY_CODE_HOURS_VALID;
-import static fr.xephi.authme.util.Utils.MILLIS_PER_HOUR;
 
 /**
  * Manager for recovery codes.
  */
-public class RecoveryCodeService implements SettingsDependent {
+public class RecoveryCodeService implements SettingsDependent, HasCleanup {
-
-    private Map<String, ExpiringEntry> recoveryCodes = new ConcurrentHashMap<>();
 
+    private final ExpiringMap<String, String> recoveryCodes;
     private int recoveryCodeLength;
-    private long recoveryCodeExpirationMillis;
+    private int recoveryCodeExpiration;
 
     @Inject
     RecoveryCodeService(Settings settings) {
-        reload(settings);
+        recoveryCodeLength = settings.getProperty(SecuritySettings.RECOVERY_CODE_LENGTH);
+        recoveryCodeExpiration = settings.getProperty(SecuritySettings.RECOVERY_CODE_HOURS_VALID);
+        recoveryCodes = new ExpiringMap<>(recoveryCodeExpiration, TimeUnit.HOURS);
     }
 
     /**
      * @return whether recovery codes are enabled or not
      */
     public boolean isRecoveryCodeNeeded() {
-        return recoveryCodeLength > 0 && recoveryCodeExpirationMillis > 0;
+        return recoveryCodeLength > 0 && recoveryCodeExpiration > 0;
     }
 
     /**
@@ -43,7 +43,7 @@ public class RecoveryCodeService implements SettingsDependent {
      */
     public String generateCode(String player) {
         String code = RandomStringUtils.generateHex(recoveryCodeLength);
-        recoveryCodes.put(player, new ExpiringEntry(code, System.currentTimeMillis() + recoveryCodeExpirationMillis));
+        recoveryCodes.put(player, code);
         return code;
     }
 
@@ -55,11 +55,8 @@ public class RecoveryCodeService implements SettingsDependent {
      * @return true if the code matches and has not expired, false otherwise
      */
     public boolean isCodeValid(String player, String code) {
-        ExpiringEntry entry = recoveryCodes.get(player);
+        String storedCode = recoveryCodes.get(player);
-        if (entry != null) {
+        return storedCode != null && storedCode.equals(code);
-            return code != null && code.equals(entry.getCode());
-        }
-        return false;
     }
 
     /**
@@ -74,26 +71,12 @@ public class RecoveryCodeService implements SettingsDependent {
     @Override
     public void reload(Settings settings) {
         recoveryCodeLength = settings.getProperty(SecuritySettings.RECOVERY_CODE_LENGTH);
-        recoveryCodeExpirationMillis = settings.getProperty(RECOVERY_CODE_HOURS_VALID) * MILLIS_PER_HOUR;
+        recoveryCodeExpiration = settings.getProperty(RECOVERY_CODE_HOURS_VALID);
+        recoveryCodes.setExpiration(recoveryCodeExpiration, TimeUnit.HOURS);
     }
 
-    /**
+    @Override
-     * Entry with an expiration.
+    public void performCleanup() {
-     */
+        recoveryCodes.removeExpiredEntries();
-    @VisibleForTesting
-    static final class ExpiringEntry {
-
-        private final String code;
-        private final long expiration;
-
-        ExpiringEntry(String code, long expiration) {
-            this.code = code;
-            this.expiration = expiration;
-        }
-
-        String getCode() {
-            return System.currentTimeMillis() < expiration ? code : null;
-        }
     }
-
 }

src/main/java/fr/xephi/authme/util/ExpiringMap.java

@@ -0,0 +1,114 @@
+package fr.xephi.authme.util;
+
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.TimeUnit;
+
+/**
+ * Map with expiring entries. Following a configured amount of time after
+ * an entry has been inserted, the map will act as if the entry does not
+ * exist.
+ * <p>
+ * Time starts counting directly after insertion. Inserting a new entry with
+ * a key that already has a value will "reset" the expiration. Although the
+ * expiration can be redefined later on, only entries which are inserted
+ * afterwards will use the new expiration.
+ * <p>
+ * An expiration of {@code <= 0} will make the map expire all entries
+ * immediately after insertion. Note that the map does not remove expired
+ * entries automatically; this is only done when calling
+ * {@link #removeExpiredEntries()}.
+ *
+ * @param <K> the key type
+ * @param <V> the value type
+ */
+public class ExpiringMap<K, V> {
+
+    private final Map<K, ExpiringEntry<V>> entries = new ConcurrentHashMap<>();
+    private long expirationMillis;
+
+    /**
+     * Constructor.
+     *
+     * @param duration the duration of time after which entries expire
+     * @param unit the time unit in which {@code duration} is expressed
+     */
+    public ExpiringMap(long duration, TimeUnit unit) {
+        setExpiration(duration, unit);
+    }
+
+    /**
+     * Returns the value associated with the given key,
+     * if available and not expired.
+     *
+     * @param key the key to look up
+     * @return the associated value, or {@code null} if not available
+     */
+    public V get(K key) {
+        ExpiringEntry<V> value = entries.get(key);
+        return value == null ? null : value.getValue();
+    }
+
+    /**
+     * Inserts a value for the given key. Overwrites a previous value
+     * for the key if it exists.
+     *
+     * @param key the key to insert a value for
+     * @param value the value to insert
+     */
+    public void put(K key, V value) {
+        long expiration = System.currentTimeMillis() + expirationMillis;
+        entries.put(key, new ExpiringEntry<>(value, expiration));
+    }
+
+    /**
+     * Removes the value for the given key, if available.
+     *
+     * @param key the key to remove the value for
+     */
+    public void remove(K key) {
+        entries.remove(key);
+    }
+
+    /**
+     * Removes all entries which have expired from the internal structure.
+     */
+    public void removeExpiredEntries() {
+        entries.entrySet().removeIf(entry -> System.currentTimeMillis() > entry.getValue().getExpiration());
+    }
+
+    /**
+     * Sets a new expiration duration. Note that already present entries
+     * will still make use of the old expiration.
+     *
+     * @param duration the duration of time after which entries expire
+     * @param unit the time unit in which {@code duration} is expressed
+     */
+    public void setExpiration(long duration, TimeUnit unit) {
+        this.expirationMillis = unit.toMillis(duration);
+    }
+
+    /**
+     * Class holding a value paired with an expiration timestamp.
+     *
+     * @param <V> the value type
+     */
+    private static final class ExpiringEntry<V> {
+
+        private final V value;
+        private final long expiration;
+
+        ExpiringEntry(V value, long expiration) {
+            this.value = value;
+            this.expiration = expiration;
+        }
+
+        V getValue() {
+            return System.currentTimeMillis() > expiration ? null : value;
+        }
+
+        long getExpiration() {
+            return expiration;
+        }
+    }
+}

src/test/java/fr/xephi/authme/service/RecoveryCodeServiceTest.java

@@ -4,15 +4,13 @@ import ch.jalu.injector.testing.BeforeInjecting;
 import ch.jalu.injector.testing.DelayedInjectionRunner;
 import ch.jalu.injector.testing.InjectDelayed;
 import fr.xephi.authme.ReflectionTestUtils;
-import fr.xephi.authme.service.RecoveryCodeService.ExpiringEntry;
 import fr.xephi.authme.settings.Settings;
 import fr.xephi.authme.settings.properties.SecuritySettings;
+import fr.xephi.authme.util.ExpiringMap;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mockito.Mock;
 
-import java.util.Map;
-
 import static fr.xephi.authme.AuthMeMatchers.stringWithLength;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.nullValue;
@@ -60,22 +58,8 @@ public class RecoveryCodeServiceTest {
         recoveryCodeService.generateCode(name);
 
         // then
-        ExpiringEntry entry = getCodeMap().get(name);
+        String code = getCodeMap().get(name);
-        assertThat(entry.getCode(), stringWithLength(5));
+        assertThat(code, stringWithLength(5));
-    }
-
-    @Test
-    public void shouldNotConsiderExpiredCode() {
-        // given
-        String player = "Cat";
-        String code = "11F235";
-        setCodeInMap(player, code, System.currentTimeMillis() - 500);
-
-        // when
-        boolean result = recoveryCodeService.isCodeValid(player, code);
-
-        // then
-        assertThat(result, equalTo(false));
     }
 
     @Test
@@ -106,12 +90,7 @@ public class RecoveryCodeServiceTest {
     }
 
 
-    private Map<String, ExpiringEntry> getCodeMap() {
+    private ExpiringMap<String, String> getCodeMap() {
         return ReflectionTestUtils.getFieldValue(RecoveryCodeService.class, recoveryCodeService, "recoveryCodes");
     }
-
-    private void setCodeInMap(String player, String code, long expiration) {
-        Map<String, ExpiringEntry> map = getCodeMap();
-        map.put(player, new ExpiringEntry(code, expiration));
-    }
 }

src/test/java/fr/xephi/authme/util/ExpiringMapTest.java

@@ -0,0 +1,94 @@
+package fr.xephi.authme.util;
+
+import fr.xephi.authme.ReflectionTestUtils;
+import org.junit.Test;
+
+import java.util.Map;
+import java.util.concurrent.TimeUnit;
+
+import static org.hamcrest.Matchers.containsInAnyOrder;
+import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.nullValue;
+import static org.junit.Assert.assertThat;
+
+/**
+ * Test for {@link ExpiringMap}.
+ */
+public class ExpiringMapTest {
+
+    @Test
+    public void shouldAddAndRetrieveEntries() {
+        // given
+        ExpiringMap<String, Double> map = new ExpiringMap<>(3, TimeUnit.MINUTES);
+
+        // when / then
+        map.put("three", 3.0);
+        map.put("treefiddy", 3.50);
+
+        assertThat(map.get("three"), equalTo(3.0));
+        assertThat(map.get("treefiddy"), equalTo(3.50));
+    }
+
+    @Test
+    public void shouldRemoveEntry() {
+        // given
+        ExpiringMap<String, Boolean> map = new ExpiringMap<>(1, TimeUnit.HOURS);
+        map.put("hi", true);
+        map.put("ha", false);
+
+        // when
+        map.remove("ha");
+
+        // then
+        assertThat(map.get("ha"), nullValue());
+        assertThat(map.get("hi"), equalTo(true));
+    }
+
+    @Test
+    public void shouldUpdateExpiration() {
+        // given
+        ExpiringMap<Integer, Integer> map = new ExpiringMap<>(2, TimeUnit.DAYS);
+        map.put(2, 4);
+        map.put(3, 9);
+
+        // when
+        map.setExpiration(0, TimeUnit.SECONDS);
+
+        // then
+        map.put(5, 25);
+        assertThat(map.get(2), equalTo(4));
+        assertThat(map.get(3), equalTo(9));
+        assertThat(map.get(5), nullValue());
+    }
+
+    @Test
+    public void shouldAcceptNegativeExpiration() {
+        // given / when
+        ExpiringMap<Integer, String> map = new ExpiringMap<>(-3, TimeUnit.MINUTES);
+        map.put(3, "trois");
+
+        // then
+        assertThat(map.get(3), nullValue());
+    }
+
+    @Test
+    public void shouldCleanUpExpiredEntries() throws InterruptedException {
+        // given
+        ExpiringMap<Integer, Integer> map = new ExpiringMap<>(400, TimeUnit.MILLISECONDS);
+        map.put(144, 12);
+        map.put(121, 11);
+        map.put(81, 9);
+        map.setExpiration(900, TimeUnit.MILLISECONDS);
+        map.put(64, 8);
+        map.put(25, 5);
+
+        // when
+        Thread.sleep(400);
+        map.removeExpiredEntries();
+
+        // then
+        Map<Integer, ?> internalMap = ReflectionTestUtils.getFieldValue(ExpiringMap.class, map, "entries");
+        assertThat(internalMap.keySet(), containsInAnyOrder(64, 25));
+    }
+
+}