Skip to content

Bugfix: Nested collection replacment #970

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 21 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
b83bd89
Implement DefaultCollectionMutator.replace(E e)
lhy-hoyin Oct 16, 2024
508227c
tests: Add tests for DefaultCollectionMutator.replace(E e)
lhy-hoyin Oct 16, 2024
01003bf
Implement overloaded Assert.notEmpty for asserting Object
lhy-hoyin Oct 17, 2024
da96e8b
Change method signature of replace
lhy-hoyin Oct 19, 2024
8e8fd6c
Implement DefaultMacAlgorithm.equals
lhy-hoyin Oct 19, 2024
fd8dc5e
Modify add(e) logic to replace existing with same ID
lhy-hoyin Oct 19, 2024
0c15467
Modify tests to explicitly test for situation mentioned in #961
lhy-hoyin Oct 19, 2024
57e4268
fix: Fix issue where replace does not do a in-place replacement
lhy-hoyin Oct 19, 2024
a480d1e
Integrate replacement logic directly into add
lhy-hoyin Oct 29, 2024
f973a3d
Changing JDK 17 build to use GitHub's 'setup-java' instead of Oracle'…
lhazlewood Mar 13, 2025
053855f
Use SPDX identifier in POMs (#979)
TheMrMilchmann Mar 13, 2025
4beef3a
Implement DefaultCollectionMutator.replace(E e)
lhy-hoyin Oct 16, 2024
3e407f6
tests: Add tests for DefaultCollectionMutator.replace(E e)
lhy-hoyin Oct 16, 2024
de9f3d9
Implement overloaded Assert.notEmpty for asserting Object
lhy-hoyin Oct 17, 2024
db865ee
Change method signature of replace
lhy-hoyin Oct 19, 2024
d0bad35
Implement DefaultMacAlgorithm.equals
lhy-hoyin Oct 19, 2024
6d7a100
Modify add(e) logic to replace existing with same ID
lhy-hoyin Oct 19, 2024
8e36dce
Modify tests to explicitly test for situation mentioned in #961
lhy-hoyin Oct 19, 2024
8ab77ec
fix: Fix issue where replace does not do a in-place replacement
lhy-hoyin Oct 19, 2024
d50700f
Integrate replacement logic directly into add
lhy-hoyin Oct 29, 2024
484872b
Merge branch 'nested-collection-replace' of https://github.com/lhy-ho…
lhazlewood Mar 13, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@

import java.util.Collection;
import java.util.LinkedHashSet;
import java.util.NoSuchElementException;

public class DefaultCollectionMutator<E, M extends CollectionMutator<E, M>> implements CollectionMutator<E, M> {

Expand All @@ -46,6 +47,33 @@ private boolean doAdd(E e) {
return this.collection.add(e);
}

public M replace(E e) {
if (Objects.isEmpty(e)) {
if (e == null) throw new NullPointerException("Cannot be null.");
else throw new IllegalArgumentException("Cannot be empty.");
}

// Keep a copy of the original, in case remove/add fails, and need to rollback
E old = null;
for (E element : this.collection) {
if (element.equals(e)) {
old = element;
break;
}
}

if (old == null || !this.collection.contains(e)) {
String msg = this.getClass() + " does not contain " + e + ".";
throw new NoSuchElementException(msg);
}

if (this.collection.remove(e))
if (doAdd(e)) changed(); // removed and add successfully, notify changed()
else this.collection.add(old); // remove successfully but add failed, add back old (not considered a change)

return self();
}

@Override
public M add(E e) {
if (doAdd(e)) changed();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.*

/**
Expand Down Expand Up @@ -128,6 +132,35 @@ class DefaultCollectionMutatorTest {
assertEquals 0, changeCount
}

@Test
void replace() {
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.replace(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(expected = NoSuchElementException)
void replaceMissing() {
m.replace('foo')
}

@Test(expected = NullPointerException)
void replaceNull() {
m.replace(null)
}

@Test(expected = IllegalArgumentException)
void replaceEmpty() {
m.replace(Strings.EMPTY)
}

@Test
void clear() {
m.add('one').add('two').add(['three', 'four'])
Expand Down