Bugfix: Nested collection replacment

api/src/main/java/io/jsonwebtoken/lang/Assert.java

@@ -347,6 +347,33 @@ public static void notEmpty(Map map) {
         notEmpty(map, "[Assertion failed] - this map must not be empty; it must contain at least one entry");
     }
 
+    /**
+     * Assert that an Object is not <code>null</code> or empty.
+     * <pre class="code">Assert.notEmpty(object, "Object cannot be null or empty");</pre>
+     *
+     * @param object  the object to check
+     * @param message the exception message to use if the assertion fails
+     * @return the non-null, non-empty object
+     * @throws IllegalArgumentException if the object is <code>null</code> or empty
+     */
+    public static Object notEmpty(Object object, String message) {
+        if (Objects.isEmpty(object)) {
+            throw new IllegalArgumentException(message);
+        }
+        return object;
+    }
+
+    /**
+     * Assert that an Object is not <code>null</code> or empty.
+     * <pre class="code">Assert.notEmpty(object);</pre>
+     *
+     * @param object the object to check
+     * @throws IllegalArgumentException if the object is <code>null</code> or empty
+     */
+    public static void notEmpty(Object object) {
+        notEmpty(object, "[Assertion failed] - this object must not be null or empty");
+    }
+
 
     /**
      * Assert that the provided object is an instance of the provided class.

impl/src/main/java/io/jsonwebtoken/impl/lang/DefaultCollectionMutator.java

@@ -16,13 +16,16 @@
 package io.jsonwebtoken.impl.lang;
 
 import io.jsonwebtoken.Identifiable;
+import io.jsonwebtoken.lang.Assert;
 import io.jsonwebtoken.lang.CollectionMutator;
 import io.jsonwebtoken.lang.Collections;
 import io.jsonwebtoken.lang.Objects;
 import io.jsonwebtoken.lang.Strings;
 
 import java.util.Collection;
+import java.util.Iterator;
 import java.util.LinkedHashSet;
+import java.util.NoSuchElementException;
 
 public class DefaultCollectionMutator<E, M extends CollectionMutator<E, M>> implements CollectionMutator<E, M> {
 
@@ -46,9 +49,60 @@ private boolean doAdd(E e) {
         return this.collection.add(e);
     }
 
+    public M replace(E existingElement, E newElement) {
+        Assert.notEmpty(existingElement, "existingElement cannot be null or empty.");
+        Assert.notEmpty(newElement, "newElement cannot be null or empty.");
+
+        // Same item, nothing to do
+        if (existingElement.equals(newElement))
+            return self();
+
+        // Does not contain existingElement to replace
+        if (!this.collection.contains(existingElement)) {
+            String msg = this.getClass() + " does not contain " + existingElement + ".";
+            throw new NoSuchElementException(msg);
+        }
+
+        // Replacement step 1: iterate until element to replace
+        Iterator<E> it = this.collection.iterator();
+        while (it.hasNext())
+            if (it.next().equals(existingElement)) {
+                it.remove(); // step 2: remove existingElement
+                break;
+            }
+
+        // Replacement step 3: collect and remove elements after element to replace
+        Collection<E> elementsAfterExisting = new LinkedHashSet<>();
+        while (it.hasNext()) {
+            elementsAfterExisting.add(it.next());
+            it.remove();
+        }
+
+        this.doAdd(newElement); // step 4: add replacer element (position will be at the existingElement)
+        this.collection.addAll(elementsAfterExisting); // step 5: add back the elemnts found after existingElement
+
+        changed(); // trigger changed()
+
+        return self();
+    }
+
     @Override
     public M add(E e) {
-        if (doAdd(e)) changed();
+        E existing = null;
+        for (E item : collection) {
+            boolean bothIdentifiable = e instanceof Identifiable && item instanceof Identifiable;
+            boolean sameId = bothIdentifiable && ((Identifiable) item).getId().equals(((Identifiable) e).getId());
+            if (sameId) {
+                existing = item;
+                break;
+            }
+        }
+
+        if (Objects.isEmpty(existing)) {
+            if (doAdd(e)) changed();
+        }
+        else replace(existing, e);
+
         return self();
     }
 

impl/src/main/java/io/jsonwebtoken/impl/security/DefaultMacAlgorithm.java

@@ -222,4 +222,16 @@ protected boolean doVerify(VerifySecureDigestRequest<SecretKey> request) {
         byte[] computedSignature = digest(request);
         return MessageDigest.isEqual(providedSignature, computedSignature);
     }
+
+    @Override
+    public boolean equals(Object obj) {
+        if (this == obj) {
+            return true;
+        }
+        if (obj instanceof DefaultMacAlgorithm) {
+            DefaultMacAlgorithm other = (DefaultMacAlgorithm) obj;
+            return super.equals(obj) && this.minKeyBitLength == other.minKeyBitLength;
+        }
+        return false;
+    }
 }

impl/src/test/groovy/io/jsonwebtoken/impl/lang/DefaultCollectionMutatorTest.groovy

@@ -16,10 +16,14 @@
 package io.jsonwebtoken.impl.lang
 
 import io.jsonwebtoken.Identifiable
+import io.jsonwebtoken.Jwts
 import io.jsonwebtoken.lang.Strings
+import io.jsonwebtoken.security.MacAlgorithm
 import org.junit.Before
 import org.junit.Test
 
+import java.lang.reflect.Constructor
+
 import static org.junit.Assert.*
 
 /**
@@ -95,24 +99,35 @@ class DefaultCollectionMutatorTest {
 
     @Test(expected = IllegalArgumentException)
     void addIdentifiableWithNullId() {
-        def e = new Identifiable() {
-            @Override
-            String getId() {
-                return null
-            }
-        }
-        m.add(e)
+        m.add(new IdentifiableObject(null, null))
     }
 
     @Test(expected = IllegalArgumentException)
     void addIdentifiableWithEmptyId() {
-        def e = new Identifiable() {
-            @Override
-            String getId() {
-                return '  '
-            }
-        }
-        m.add(e)
+        m.add(new IdentifiableObject('  ', null))
+    }
+
+    @Test
+    void addIdentifiableWithSameIdEvictsExisting() {
+        m.add(new IdentifiableObject('sameId', 'foo'))
+        m.add(new IdentifiableObject('sameId', 'bar'))
+        assertEquals 2, changeCount
+        assertEquals 1, m.collection.size() // second 'add' should evict first
+        assertEquals 'bar', ((IdentifiableObject) m.collection.toArray()[0]).obj
+    }
+
+    @Test
+    void addSecureDigestAlgorithmWithSameIdReplacesExisting() {
+        Class<?> c = Class.forName("io.jsonwebtoken.impl.security.DefaultMacAlgorithm")
+        Constructor<?> ctor = c.getDeclaredConstructor(String.class, String.class, int.class)
+        ctor.setAccessible(true)
+        MacAlgorithm custom = (MacAlgorithm) ctor.newInstance('HS512', 'HmacSHA512', 80)
+
+        m.add(Jwts.SIG.HS512)
+        m.add(custom)
+        assertEquals 2, changeCount // replace is count as one change
+        assertEquals 1, m.getCollection().size() // existing is removed as part of replacement
+        assertEquals 80, ((MacAlgorithm) m.getCollection().toArray()[0]).getKeyBitLength()
     }
 
     @Test
@@ -128,11 +143,67 @@ class DefaultCollectionMutatorTest {
         assertEquals 0, changeCount
     }
 
+    @Test
+    void replace() {
+        def e1 = new IdentifiableObject('sameId', 'e1')
+        def e2 = new IdentifiableObject('sameId', 'e2')
+
+        m.add(e1)
+        m.replace(e1, e2)
+        assertEquals 2, changeCount // replace is count as one change
+        assertEquals 1, m.getCollection().size() // existing is removed as part of replacement
+        assertEquals 'e2', ((IdentifiableObject) m.getCollection().toArray()[0]).obj
+    }
+
+    @Test
+    void replaceSameObject() {
+        m.add('hello')
+        m.replace('hello', 'hello') // replace with the same object, no change should be reflected
+        assertEquals 1, changeCount
+    }
+
+    @Test(expected = NoSuchElementException)
+    void replaceMissing() {
+        m.replace('foo', 'bar')
+    }
+
+    @Test(expected = IllegalArgumentException)
+    void replaceNull() {
+        m.replace('foo', null)
+    }
+
+    @Test(expected = IllegalArgumentException)
+    void replaceEmpty() {
+        m.replace('foo', Strings.EMPTY)
+    }
+
+    @Test
+    void replaceMaintainsOrder() {
+        m.add(['1', '2', '3'])
+        m.replace('2', 'B')
+        assertArrayEquals(new Object[] {'1', 'B', '3'}, m.collection.toArray())
+    }
+
     @Test
     void clear() {
         m.add('one').add('two').add(['three', 'four'])
         assertEquals 4, m.getCollection().size()
         m.clear()
         assertTrue m.getCollection().isEmpty()
     }
+
+    private class IdentifiableObject implements Identifiable {
+        String id
+        Object obj
+
+        IdentifiableObject(String id, Object obj) {
+            this.id = id
+            this.obj = obj
+        }
+
+        @Override
+        String getId() {
+            return id
+        }
+    }
 }