Integrate replacement logic directly into add

to reduce code complexity.

All tests passed.

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

@@ -16,7 +16,6 @@
 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;
@@ -25,7 +24,6 @@
 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> {
 
@@ -49,59 +47,46 @@ 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);
-        }
+    @Override
+    public M add(E e) {
+        boolean doReplace = false;
 
-        // Replacement step 1: iterate until element to replace
+        // Replacement step 1: iterate until element to replace (if any)
+        E item;
         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
+            item = it.next();
 
-        changed(); // trigger changed()
+            // Same item, nothing to do
+            if (item.equals(e))
+                return self();
 
-        return self();
-    }
-
-    @Override
-    public M add(E e) {
-        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;
+                it.remove(); // step 2: remove existing item
+                doReplace = true;
                 break;
             }
         }
 
-        if (Objects.isEmpty(existing)) {
+        if (doReplace) {
+            // 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(e); // step 4: add replacer element (position will be at the existing item)
+            this.collection.addAll(elementsAfterExisting); // step 5: add back the elements found after existing item
+
+            changed(); // trigger changed()
+        }
+        else {
+            // No replacement, do add instead
             if (doAdd(e)) changed();
         }
-        else replace(existing, e);
 
         return self();
     }

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

@@ -52,9 +52,17 @@ class DefaultCollectionMutatorTest {
         assertTrue c.isEmpty()
     }
 
+    @Test
+    void addNull() {
+        m.add(null)
+        assertEquals 0, changeCount
+        assertTrue m.getCollection().isEmpty() // wasn't added
+    }
+
     @Test
     void addEmpty() {
         m.add(Strings.EMPTY)
+        assertEquals 0, changeCount
         assertTrue m.getCollection().isEmpty() // wasn't added
     }
 
@@ -116,6 +124,19 @@ class DefaultCollectionMutatorTest {
         assertEquals 'bar', ((IdentifiableObject) m.collection.toArray()[0]).obj
     }
 
+    @Test
+    void addIdentifiableWithSameIdMaintainsOrder() {
+        IdentifiableObject e1 = new IdentifiableObject('1', 'e1')
+        IdentifiableObject e2 = new IdentifiableObject('sameId', 'e2')
+        IdentifiableObject e3 = new IdentifiableObject('3', 'e3')
+        IdentifiableObject eB = new IdentifiableObject('sameId', 'eB')
+
+        m.add([e1, e2, e3])
+        m.add(eB) // replace e2 with eB
+        assertEquals 2, changeCount
+        assertArrayEquals(new Object[] {e1, eB, e3}, m.collection.toArray())
+    }
+
     @Test
     void addSecureDigestAlgorithmWithSameIdReplacesExisting() {
         Class<?> c = Class.forName("io.jsonwebtoken.impl.security.DefaultMacAlgorithm")
@@ -143,47 +164,6 @@ 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'])