Repository: jwtk/jjwt (java)

Issue:
Jwts.builder - audience cannot be add without calling add()
**Describe the bug**
When using builder to create a token adding audience needs to be followed by `and()`. This behaviour is not documented and it is inconsistent (eg. claims do not need that).

**To Reproduce**
Try to add audience to a builder by calling `builder.audience().add("my-web")`;
Test case:
```
  @Test
  public void testJwtBuilder(){
      //working
      JwtBuilder builder = Jwts.builder();
      builder.audience().add("my-web").and();
      String json = builder.compact();

      String decoded = new String(Base64.getDecoder().decode(json.split("\\.")[1]));
      assertThat(decoded).contains("aud");

      // Failing test scenario
      JwtBuilder builder2 = Jwts.builder();
      // missing and()
      builder2.audience().add("my-web");
      String json2 = builder2.compact();

      String decoded2 = new String(Base64.getDecoder().decode(json2.split("\\.")[1]));
      assertThat(decoded2).contains("aud");
  }
```

**Expected behavior**
Adding audience to a builder does not need to be followed by `and()`


Pull request description:
NestedCollection changes without `.and()`
* Ensured `NestedCollection`s do not need their `.and()` method called to apply collection changes. Instead, changes are applied immediately as they occur (via `.add`, `.remove`, etc), and `.and()` is now purely for returning to the parent builder if necessary/desired.
* Updated associated JavaDoc with code examples to make the `.and()` method's purpose a little clearer.
* Updated CHANGELOG.md

Closes #916

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 33ce2fb..70c53ed 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,5 +1,31 @@
 ## Release Notes
 
+### 0.12.5
+
+This patch release:
+
+* Ensures that builders' `NestedCollection` changes are applied to the collection immediately as mutation methods are called, no longer
+  requiring application developers to call `.and()` to 'commit' or apply a change.  For example, prior to this release,
+  the following code did not apply changes:
+  ```java
+  JwtBuilder builder = Jwts.builder();
+  builder.audience().add("an-audience"); // no .and() call
+  builder.compact(); // would not keep 'an-audience'
+  ```
+  Now this code works as expected and all other `NestedCollection` instances like it apply changes immediately (e.g. when calling
+  `.add(value)`).
+  
+  However, standard fluent builder chains are still recommended for readability when feasible, e.g.
+  
+  ```java
+  Jwts.builder()
+      .audience().add("an-audience").and() // allows fluent chaining
+      .subject("Joe")
+      // etc...
+      .compact()
+  ```
+  See [Issue 916](https://github.com/jwtk/jjwt/issues/916).
+
 ### 0.12.4
 
 This patch release includes various changes listed below.
diff --git a/README.md b/README.md
index 794f95b..797e307 100644
--- a/README.md
+++ b/README.md
@@ -993,7 +993,7 @@ String jws = Jwts.builder()
 
     .issuer("me")
     .subject("Bob")
-    .audience("you")
+    .audience().add("you").and()    
     .expiration(expiration) //a java.util.Date
     .notBefore(notBefore) //a java.util.Date 
     .issuedAt(new Date()) // for example, now
diff --git a/api/src/main/java/io/jsonwebtoken/ClaimsMutator.java b/api/src/main/java/io/jsonwebtoken/ClaimsMutator.java
index 4db0950..1fdca1e 100644
--- a/api/src/main/java/io/jsonwebtoken/ClaimsMutator.java
+++ b/api/src/main/java/io/jsonwebtoken/ClaimsMutator.java
@@ -96,6 +96,17 @@ public interface ClaimsMutator<T extends ClaimsMutator<T>> {
      * <a href="https://www.rfc-editor.org/rfc/rfc7519.html#section-4.1.3"><code>aud</code></a> (audience) Claim
      * set, quietly ignoring any null, empty, whitespace-only, or existing value already in the set.
      *
+     * <p>When finished, the {@code audience} collection's {@link AudienceCollection#and() and()} method may be used
+     * to continue configuration. For example:</p>
+     * <blockquote><pre>
+     *  Jwts.builder() // or Jwts.claims()
+     *
+     *     .audience().add("anAudience")<b>.and() // return parent</b>
+     *
+     *  .subject("Joe") // resume configuration...
+     *  // etc...
+     * </pre></blockquote>
+     *
      * @return the {@link AudienceCollection AudienceCollection} to use for {@code aud} configuration.
      * @see AudienceCollection AudienceCollection
      * @see AudienceCollection#single(String) AudienceCollection.single(String)
@@ -221,6 +232,16 @@ public interface ClaimsMutator<T extends ClaimsMutator<T>> {
      * A {@code NestedCollection} for setting {@link #audience()} values that also allows overriding the collection
      * to be a {@link #single(String) single string value} for legacy JWT recipients if necessary.
      *
+     * <p>Because this interface extends {@link NestedCollection}, the {@link #and()} method may be used to continue
+     * parent configuration. For example:</p>
+     * <blockquote><pre>
+     *  Jwts.builder() // or Jwts.claims()
+     *
+     *     .audience().add("anAudience")<b>.and() // return parent</b>
+     *
+     *  .subject("Joe") // resume parent configuration...
+     *  // etc...</pre></blockquote>
+     *
      * @param <P> the type of ClaimsMutator to return for method chaining.
      * @see #single(String)
      * @since 0.12.0
diff --git a/api/src/main/java/io/jsonwebtoken/JwtParserBuilder.java b/api/src/main/java/io/jsonwebtoken/JwtParserBuilder.java
index d1a4755..7993669 100644
--- a/api/src/main/java/io/jsonwebtoken/JwtParserBuilder.java
+++ b/api/src/main/java/io/jsonwebtoken/JwtParserBuilder.java
@@ -101,12 +101,25 @@ public interface JwtParserBuilder extends Builder<JwtParser> {
      * the parser encounters a Protected JWT that {@link ProtectedHeader#getCritical() requires} extensions, and
      * those extensions' header names are not specified via this method, the parser will reject that JWT.
      *
+     * <p>The collection's {@link Conjunctor#and() and()} method returns to the builder for continued parser
+     * configuration, for example:</p>
+     * <blockquote><pre>
+     * parserBuilder.critical().add("headerName")<b>.{@link Conjunctor#and() and()} // etc...</b></pre></blockquote>
+     *
      * <p><b>Extension Behavior</b></p>
      *
      * <p>The {@code critical} collection only identifies header parameter names that are used in extensions supported
      * by the application. <b>Application developers, <em>not JJWT</em>, MUST perform the associated extension behavior
      * using the parsed JWT</b>.</p>
      *
+     * <p><b>Continued Parser Configuration</b></p>
+     * <p>When finished, use the collection's
+     * {@link Conjunctor#and() and()} method to continue parser configuration, for example:
+     * <blockquote><pre>
+     * Jwts.parser()
+     *     .critical().add("headerName").<b>{@link Conjunctor#and() and()} // return parent</b>
+     * // resume parser configuration...</pre></blockquote>
+     *
      * @return the {@link NestedCollection} to use for {@code crit} configuration.
      * @see ProtectedHeader#getCritical()
      * @since 0.12.0
@@ -557,7 +570,7 @@ public interface JwtParserBuilder extends Builder<JwtParser> {
      * <p>The collection's {@link Conjunctor#and() and()} method returns to the builder for continued parser
      * configuration, for example:</p>
      * <blockquote><pre>
-     * parserBuilder.enc().add(anAeadAlgorithm).{@link Conjunctor#and() and()} // etc...</pre></blockquote>
+     * parserBuilder.enc().add(anAeadAlgorithm)<b>.{@link Conjunctor#and() and()} // etc...</b></pre></blockquote>
      *
      * <p><b>Standard Algorithms and Overrides</b></p>
      *
@@ -597,7 +610,7 @@ public interface JwtParserBuilder extends Builder<JwtParser> {
      * <p>The collection's {@link Conjunctor#and() and()} method returns to the builder for continued parser
      * configuration, for example:</p>
      * <blockquote><pre>
-     * parserBuilder.key().add(aKeyAlgorithm).{@link Conjunctor#and() and()} // etc...</pre></blockquote>
+     * parserBuilder.key().add(aKeyAlgorithm)<b>.{@link Conjunctor#and() and()} // etc...</b></pre></blockquote>
      *
      * <p><b>Standard Algorithms and Overrides</b></p>
      *
@@ -639,7 +652,7 @@ public interface JwtParserBuilder extends Builder<JwtParser> {
      * <p>The collection's {@link Conjunctor#and() and()} method returns to the builder for continued parser
      * configuration, for example:</p>
      * <blockquote><pre>
-     * parserBuilder.sig().add(aSignatureAlgorithm).{@link Conjunctor#and() and()} // etc...</pre></blockquote>
+     * parserBuilder.sig().add(aSignatureAlgorithm)<b>.{@link Conjunctor#and() and()} // etc...</b></pre></blockquote>
      *
      * <p><b>Standard Algorithms and Overrides</b></p>
      *
@@ -680,7 +693,7 @@ public interface JwtParserBuilder extends Builder<JwtParser> {
      * <p>The collection's {@link Conjunctor#and() and()} method returns to the builder for continued parser
      * configuration, for example:</p>
      * <blockquote><pre>
-     * parserBuilder.zip().add(aCompressionAlgorithm).{@link Conjunctor#and() and()} // etc...</pre></blockquote>
+     * parserBuilder.zip().add(aCompressionAlgorithm)<b>.{@link Conjunctor#and() and()} // etc...</b></pre></blockquote>
      *
      * <p><b>Standard Algorithms and Overrides</b></p>
      *
diff --git a/api/src/main/java/io/jsonwebtoken/ProtectedHeaderMutator.java b/api/src/main/java/io/jsonwebtoken/ProtectedHeaderMutator.java
index dd38bca..0022506 100644
--- a/api/src/main/java/io/jsonwebtoken/ProtectedHeaderMutator.java
+++ b/api/src/main/java/io/jsonwebtoken/ProtectedHeaderMutator.java
@@ -33,9 +33,11 @@ public interface ProtectedHeaderMutator<T extends ProtectedHeaderMutator<T>> ext
     /**
      * Configures names of header parameters used by JWT or JWA specification extensions that <em>MUST</em> be
      * understood and supported by the JWT recipient. When finished, use the collection's
-     * {@link Conjunctor#and() and()} method to return to the header builder, for example:
+     * {@link Conjunctor#and() and()} method to continue header configuration, for example:
      * <blockquote><pre>
-     * builder.critical().add("headerName").{@link Conjunctor#and() and()} // etc...</pre></blockquote>
+     * headerBuilder
+     *     .critical().add("headerName").<b>{@link Conjunctor#and() and()} // return parent</b>
+     * // resume header configuration...</pre></blockquote>
      *
      * @return the {@link NestedCollection} to use for {@code crit} configuration.
      * @see <a href="https://www.rfc-editor.org/rfc/rfc7515.html#section-4.1.11">JWS <code>crit</code> (Critical) Header Parameter</a>
diff --git a/api/src/main/java/io/jsonwebtoken/lang/NestedCollection.java b/api/src/main/java/io/jsonwebtoken/lang/NestedCollection.java
index cf0118d..2fac66c 100644
--- a/api/src/main/java/io/jsonwebtoken/lang/NestedCollection.java
+++ b/api/src/main/java/io/jsonwebtoken/lang/NestedCollection.java
@@ -17,7 +17,12 @@ package io.jsonwebtoken.lang;
 
 /**
  * A {@link CollectionMutator} that can return access to its parent via the {@link Conjunctor#and() and()} method for
- * continued configuration.
+ * continued configuration.  For example:
+ * <blockquote><pre>
+ * builder
+ *     .aNestedCollection()// etc...
+ *     <b>.and() // return parent</b>
+ * // resume parent configuration...</pre></blockquote>
  *
  * @param <E> the type of elements in the collection
  * @param <P> the parent to return
diff --git a/api/src/main/java/io/jsonwebtoken/security/JwkBuilder.java b/api/src/main/java/io/jsonwebtoken/security/JwkBuilder.java
index 2133925..add7be0 100644
--- a/api/src/main/java/io/jsonwebtoken/security/JwkBuilder.java
+++ b/api/src/main/java/io/jsonwebtoken/security/JwkBuilder.java
@@ -109,9 +109,9 @@ public interface JwkBuilder<K extends Key, J extends Jwk<K>, T extends JwkBuilde
      * the key is intended to be used. When finished, use the collection's {@link Conjunctor#and() and()} method to
      * return to the JWK builder, for example:
      * <blockquote><pre>
-     * jwkBuilder.operations().add(aKeyOperation).{@link Conjunctor#and() and()} // etc...</pre></blockquote>
+     * jwkBuilder.operations().add(aKeyOperation)<b>.{@link Conjunctor#and() and()} // etc...</b></pre></blockquote>
      *
-     * <p>The {@code and()} method will throw an {@link IllegalArgumentException} if any of the specified
+     * <p>The {@code add()} method(s) will throw an {@link IllegalArgumentException} if any of the specified
      * {@code KeyOperation}s are not permitted by the JWK's
      * {@link #operationPolicy(KeyOperationPolicy) operationPolicy}. See that documentation for more
      * information on security vulnerabilities when using the same key with multiple algorithms.</p>
diff --git a/impl/src/main/java/io/jsonwebtoken/impl/DefaultJweHeaderMutator.java b/impl/src/main/java/io/jsonwebtoken/impl/DefaultJweHeaderMutator.java
index b35806f..fc7c1f1 100644
--- a/impl/src/main/java/io/jsonwebtoken/impl/DefaultJweHeaderMutator.java
+++ b/impl/src/main/java/io/jsonwebtoken/impl/DefaultJweHeaderMutator.java
@@ -107,9 +107,8 @@ public class DefaultJweHeaderMutator<T extends JweHeaderMutator<T>>
     public NestedCollection<String, T> critical() {
         return new DefaultNestedCollection<String, T>(self(), this.DELEGATE.get(DefaultProtectedHeader.CRIT)) {
             @Override
-            public T and() {
+            protected void changed() {
                 put(DefaultProtectedHeader.CRIT, Collections.asSet(getCollection()));
-                return super.and();
             }
         };
     }
diff --git a/impl/src/main/java/io/jsonwebtoken/impl/DefaultJwtParserBuilder.java b/impl/src/main/java/io/jsonwebtoken/impl/DefaultJwtParserBuilder.java
index 1929048..ddce12b 100644
--- a/impl/src/main/java/io/jsonwebtoken/impl/DefaultJwtParserBuilder.java
+++ b/impl/src/main/java/io/jsonwebtoken/impl/DefaultJwtParserBuilder.java
@@ -220,9 +220,8 @@ public class DefaultJwtParserBuilder implements JwtParserBuilder {
     public NestedCollection<String, JwtParserBuilder> critical() {
         return new DefaultNestedCollection<String, JwtParserBuilder>(this, this.critical) {
             @Override
-            public JwtParserBuilder and() {
+            protected void changed() {
                 critical = Collections.asSet(getCollection());
-                return super.and();
             }
         };
     }
@@ -304,9 +303,8 @@ public class DefaultJwtParserBuilder implements JwtParserBuilder {
     public NestedCollection<CompressionAlgorithm, JwtParserBuilder> zip() {
         return new DefaultNestedCollection<CompressionAlgorithm, JwtParserBuilder>(this, this.zipAlgs.values()) {
             @Override
-            public JwtParserBuilder and() {
+            protected void changed() {
                 zipAlgs = new IdRegistry<>(StandardCompressionAlgorithms.NAME, getCollection());
-                return super.and();
             }
         };
     }
@@ -315,9 +313,8 @@ public class DefaultJwtParserBuilder implements JwtParserBuilder {
     public NestedCollection<AeadAlgorithm, JwtParserBuilder> enc() {
         return new DefaultNestedCollection<AeadAlgorithm, JwtParserBuilder>(this, this.encAlgs.values()) {
             @Override
-            public JwtParserBuilder and() {
+            public void changed() {
                 encAlgs = new IdRegistry<>(StandardEncryptionAlgorithms.NAME, getCollection());
-                return super.and();
             }
         };
     }
@@ -326,9 +323,8 @@ public class DefaultJwtParserBuilder implements JwtParserBuilder {
     public NestedCollection<SecureDigestAlgorithm<?, ?>, JwtParserBuilder> sig() {
         return new DefaultNestedCollection<SecureDigestAlgorithm<?, ?>, JwtParserBuilder>(this, this.sigAlgs.values()) {
             @Override
-            public JwtParserBuilder and() {
+            public void changed() {
                 sigAlgs = new IdRegistry<>(StandardSecureDigestAlgorithms.NAME, getCollection());
-                return super.and();
             }
         };
     }
@@ -337,9 +333,8 @@ public class DefaultJwtParserBuilder implements JwtParserBuilder {
     public NestedCollection<KeyAlgorithm<?, ?>, JwtParserBuilder> key() {
         return new DefaultNestedCollection<KeyAlgorithm<?, ?>, JwtParserBuilder>(this, this.keyAlgs.values()) {
             @Override
-            public JwtParserBuilder and() {
+            public void changed() {
                 keyAlgs = new IdRegistry<>(StandardKeyAlgorithms.NAME, getCollection());
-                return super.and();
             }
         };
     }
diff --git a/impl/src/main/java/io/jsonwebtoken/impl/DelegatingClaimsMutator.java b/impl/src/main/java/io/jsonwebtoken/impl/DelegatingClaimsMutator.java
index 9c408a1..fd5c483 100644
--- a/impl/src/main/java/io/jsonwebtoken/impl/DelegatingClaimsMutator.java
+++ b/impl/src/main/java/io/jsonwebtoken/impl/DelegatingClaimsMutator.java
@@ -130,12 +130,12 @@ public class DelegatingClaimsMutator<T extends MapMutator<String, Object, T> & C
             @Override
             public T single(String audience) {
                 return audienceSingle(audience);
+                // DO NOT call changed() here - we don't want to replace the value with a collection
             }
 
             @Override
-            public T and() {
+            protected void changed() {
                 put(DefaultClaims.AUDIENCE, Collections.asSet(getCollection()));
-                return super.and();
             }
         };
     }
diff --git a/impl/src/main/java/io/jsonwebtoken/impl/lang/DefaultCollectionMutator.java b/impl/src/main/java/io/jsonwebtoken/impl/lang/DefaultCollectionMutator.java
index 9a64d7e..cdb7fd8 100644
--- a/impl/src/main/java/io/jsonwebtoken/impl/lang/DefaultCollectionMutator.java
+++ b/impl/src/main/java/io/jsonwebtoken/impl/lang/DefaultCollectionMutator.java
@@ -37,38 +37,52 @@ public class DefaultCollectionMutator<E, M extends CollectionMutator<E, M>> impl
         return (M) this;
     }
 
-    @Override
-    public M add(E e) {
-        if (Objects.isEmpty(e)) return self();
+    private boolean doAdd(E e) {
+        if (Objects.isEmpty(e)) return false;
         if (e instanceof Identifiable && !Strings.hasText(((Identifiable) e).getId())) {
             String msg = e.getClass() + " getId() value cannot be null or empty.";
             throw new IllegalArgumentException(msg);
         }
-        this.collection.remove(e);
-        this.collection.add(e);
+        return this.collection.add(e);
+    }
+
+    @Override
+    public M add(E e) {
+        if (doAdd(e)) changed();
         return self();
     }
 
     @Override
     public M remove(E e) {
-        this.collection.remove(e);
+        if (this.collection.remove(e)) changed();
         return self();
     }
 
     @Override
     public M add(Collection<? extends E> c) {
+        boolean changed = false;
         for (E element : Collections.nullSafe(c)) {
-            add(element);
+            changed = doAdd(element) || changed;
         }
+        if (changed) changed();
         return self();
     }
 
     @Override
     public M clear() {
+        boolean changed = !Collections.isEmpty(this.collection);
         this.collection.clear();
+        if (changed) changed();
         return self();
     }
 
+    /**
+     * Callback for subclasses that wish to be notified if the internal collection has changed via builder mutation
+     * methods.
+     */
+    protected void changed() {
+    }
+
     protected Collection<E> getCollection() {
         return Collections.immutable(this.collection);
     }
diff --git a/impl/src/main/java/io/jsonwebtoken/impl/security/AbstractJwkBuilder.java b/impl/src/main/java/io/jsonwebtoken/impl/security/AbstractJwkBuilder.java
index 5ac3b4e..e580b34 100644
--- a/impl/src/main/java/io/jsonwebtoken/impl/security/AbstractJwkBuilder.java
+++ b/impl/src/main/java/io/jsonwebtoken/impl/security/AbstractJwkBuilder.java
@@ -111,11 +111,10 @@ abstract class AbstractJwkBuilder<K extends Key, J extends Jwk<K>, T extends Jwk
     public NestedCollection<KeyOperation, T> operations() {
         return new DefaultNestedCollection<KeyOperation, T>(self(), this.DELEGATE.getOperations()) {
             @Override
-            public T and() {
+            protected void changed() {
                 Collection<? extends KeyOperation> c = getCollection();
                 opsPolicy.validate(c);
                 DELEGATE.setOperations(c);
-                return super.and();
             }
         };
     }


Test patch:
diff --git a/impl/src/test/groovy/io/jsonwebtoken/impl/DefaultJwtBuilderTest.groovy b/impl/src/test/groovy/io/jsonwebtoken/impl/DefaultJwtBuilderTest.groovy
index 591b4b0..c6bc8ab 100644
--- a/impl/src/test/groovy/io/jsonwebtoken/impl/DefaultJwtBuilderTest.groovy
+++ b/impl/src/test/groovy/io/jsonwebtoken/impl/DefaultJwtBuilderTest.groovy
@@ -791,4 +791,47 @@ class DefaultJwtBuilderTest {
         assertEquals three, claims.aud
     }
 
+    /**
+     * Asserts that if a .audience() builder is used, and its .and() method is not called, the change to the
+     * audience is still applied when building the JWT.
+     * @see <a href="https://github.com/jwtk/jjwt/issues/916">JJWT Issue 916</a>
+     * @since JJWT_RELEASE_VERSION
+     */
+    @Test
+    void testAudienceWithoutConjunction() {
+        def aud = 'my-web'
+        def builder = Jwts.builder()
+        builder.audience().add(aud) // no .and() call
+        def jwt = builder.compact()
+
+        // assert that the resulting claims has the audience array set as expected:
+        def parsed = Jwts.parser().unsecured().build().parseUnsecuredClaims(jwt)
+        assertEquals aud, parsed.payload.getAudience()[0]
+    }
+
+    /**
+     * Asserts that if a .header().critical() builder is used, and its .and() method is not called, the change to the
+     * crit collection is still applied when building the header.
+     * @see <a href="https://github.com/jwtk/jjwt/issues/916">JJWT Issue 916</a>
+     * @since JJWT_RELEASE_VERSION
+     */
+    @Test
+    void testCritWithoutConjunction() {
+        def crit = 'test'
+        def builder = Jwts.builder().issuer('me')
+        def headerBuilder = builder.header()
+        headerBuilder.critical().add(crit) // no .and() method
+        headerBuilder.add(crit, 'foo') // no .and() method
+        builder.signWith(TestKeys.HS256)
+        def jwt = builder.compact()
+
+        def headerBytes = Decoders.BASE64URL.decode(jwt.split('\\.')[0])
+        def headerMap = Services.get(Deserializer).deserialize(Streams.reader(headerBytes)) as Map<String, ?>
+
+        def expected = [crit] as Set<String>
+        def val = headerMap.get('crit') as Set<String>
+        assertNotNull val
+        assertEquals expected, val
+    }
+
 }
diff --git a/impl/src/test/groovy/io/jsonwebtoken/impl/DefaultJwtHeaderBuilderTest.groovy b/impl/src/test/groovy/io/jsonwebtoken/impl/DefaultJwtHeaderBuilderTest.groovy
index 2951928..5c9df8e 100644
--- a/impl/src/test/groovy/io/jsonwebtoken/impl/DefaultJwtHeaderBuilderTest.groovy
+++ b/impl/src/test/groovy/io/jsonwebtoken/impl/DefaultJwtHeaderBuilderTest.groovy
@@ -507,6 +507,22 @@ class DefaultJwtHeaderBuilderTest {
         assertEquals expected, header.getCritical()
     }
 
+    /**
+     * Asserts that if a .critical() builder is used, and its .and() method is not called, the change to the
+     * crit collection is still applied when building the header.
+     * @see <a href="https://github.com/jwtk/jjwt/issues/916">JJWT Issue 916</a>
+     * @since JJWT_RELEASE_VERSION
+     */
+    @Test
+    void testCritWithoutConjunction() {
+        def crit = 'test'
+        def builder = jws()
+        builder.add(crit, 'foo').critical().add(crit) // no .and() method
+        def header = builder.build() as ProtectedHeader
+        def expected = [crit] as Set<String>
+        assertEquals expected, header.getCritical()
+    }
+
     @Test
     void testCritSingleNullIgnored() {
         def crit = 'test'
diff --git a/impl/src/test/groovy/io/jsonwebtoken/impl/DefaultJwtParserBuilderTest.groovy b/impl/src/test/groovy/io/jsonwebtoken/impl/DefaultJwtParserBuilderTest.groovy
index 71479b3..9cb7ef7 100644
--- a/impl/src/test/groovy/io/jsonwebtoken/impl/DefaultJwtParserBuilderTest.groovy
+++ b/impl/src/test/groovy/io/jsonwebtoken/impl/DefaultJwtParserBuilderTest.groovy
@@ -48,6 +48,22 @@ class DefaultJwtParserBuilderTest {
         assertTrue builder.@critical.isEmpty()
     }
 
+    /**
+     * Asserts that if a .critical() builder is used, and its .and() method is not called, the change to the
+     * crit collection is still applied when building the parser.
+     * @see <a href="https://github.com/jwtk/jjwt/issues/916">JJWT Issue 916</a>
+     * @since JJWT_RELEASE_VERSION
+     */
+    @Test
+    void testCriticalWithoutConjunction() {
+        builder.critical().add('foo') // no .and() call
+        assertFalse builder.@critical.isEmpty()
+        assertTrue builder.@critical.contains('foo')
+        def parser = builder.build()
+        assertFalse parser.@critical.isEmpty()
+        assertTrue parser.@critical.contains('foo')
+    }
+
     @Test
     void testSetProvider() {
         Provider provider = createMock(Provider)
@@ -173,6 +189,21 @@ class DefaultJwtParserBuilderTest {
         assertSame codec, parser.zipAlgs.locate(header)
     }
 
+    /**
+     * Asserts that if a .zip() builder is used, and its .and() method is not called, the change to the
+     * compression algorithm collection is still applied when building the parser.
+     * @see <a href="https://github.com/jwtk/jjwt/issues/916">JJWT Issue 916</a>
+     * @since JJWT_RELEASE_VERSION
+     */
+    @Test
+    void testAddCompressionAlgorithmWithoutConjunction() {
+        def codec = new TestCompressionCodec(id: 'test')
+        builder.zip().add(codec) // no .and() call
+        def parser = builder.build()
+        def header = Jwts.header().add('zip', codec.getId()).build()
+        assertSame codec, parser.zipAlgs.locate(header)
+    }
+
     @Test
     void testAddCompressionAlgorithmsOverrideDefaults() {
         def header = Jwts.header().add('zip', 'DEF').build()
@@ -211,6 +242,21 @@ class DefaultJwtParserBuilderTest {
         assertSame custom, parser.encAlgs.apply(header) // custom one, not standard impl
     }
 
+    /**
+     * Asserts that if an .enc() builder is used, and its .and() method is not called, the change to the
+     * encryption algorithm collection is still applied when building the parser.
+     * @see <a href="https://github.com/jwtk/jjwt/issues/916">JJWT Issue 916</a>
+     * @since JJWT_RELEASE_VERSION
+     */
+    @Test
+    void testAddEncryptionAlgorithmWithoutConjunction() {
+        def alg = new TestAeadAlgorithm(id: 'test')
+        builder.enc().add(alg) // no .and() call
+        def parser = builder.build() as DefaultJwtParser
+        def header = Jwts.header().add('alg', 'foo').add('enc', alg.getId()).build() as JweHeader
+        assertSame alg, parser.encAlgs.apply(header)
+    }
+
     @Test
     void testCaseSensitiveEncryptionAlgorithm() {
         def alg = Jwts.ENC.A256GCM
@@ -239,6 +285,23 @@ class DefaultJwtParserBuilderTest {
         assertSame custom, parser.keyAlgs.apply(header) // custom one, not standard impl
     }
 
+    /**
+     * Asserts that if an .key() builder is used, and its .and() method is not called, the change to the
+     * key algorithm collection is still applied when building the parser.
+     * @see <a href="https://github.com/jwtk/jjwt/issues/916">JJWT Issue 916</a>
+     * @since JJWT_RELEASE_VERSION
+     */
+    @Test
+    void testAddKeyAlgorithmWithoutConjunction() {
+        def alg = new TestKeyAlgorithm(id: 'test')
+        builder.key().add(alg) // no .and() call
+        def parser = builder.build() as DefaultJwtParser
+        def header = Jwts.header()
+                .add('enc', 'foo')
+                .add('alg', alg.getId()).build() as JweHeader
+        assertSame alg, parser.keyAlgs.apply(header)
+    }
+
     @Test
     void testCaseSensitiveKeyAlgorithm() {
         def alg = Jwts.KEY.A256GCMKW
@@ -268,6 +331,21 @@ class DefaultJwtParserBuilderTest {
         assertSame custom, parser.sigAlgs.apply(header) // custom one, not standard impl
     }
 
+    /**
+     * Asserts that if an .sig() builder is used, and its .and() method is not called, the change to the
+     * signature algorithm collection is still applied when building the parser.
+     * @see <a href="https://github.com/jwtk/jjwt/issues/916">JJWT Issue 916</a>
+     * @since JJWT_RELEASE_VERSION
+     */
+    @Test
+    void testAddSignatureAlgorithmWithoutConjunction() {
+        def alg = new TestMacAlgorithm(id: 'test')
+        builder.sig().add(alg) // no .and() call
+        def parser = builder.build() as DefaultJwtParser
+        def header = Jwts.header().add('alg', alg.getId()).build() as JwsHeader
+        assertSame alg, parser.sigAlgs.apply(header)
+    }
+
     @Test
     void testCaseSensitiveSignatureAlgorithm() {
         def alg = Jwts.SIG.HS256
diff --git a/impl/src/test/groovy/io/jsonwebtoken/impl/lang/DefaultCollectionMutatorTest.groovy b/impl/src/test/groovy/io/jsonwebtoken/impl/lang/DefaultCollectionMutatorTest.groovy
index 213a39f..e1b6d40 100644
--- a/impl/src/test/groovy/io/jsonwebtoken/impl/lang/DefaultCollectionMutatorTest.groovy
+++ b/impl/src/test/groovy/io/jsonwebtoken/impl/lang/DefaultCollectionMutatorTest.groovy
@@ -27,11 +27,18 @@ import static org.junit.Assert.*
  */
 class DefaultCollectionMutatorTest {
 
+    private int changeCount
     private DefaultCollectionMutator m
 
     @Before
     void setUp() {
-        m = new DefaultCollectionMutator(null)
+        changeCount = 0
+        m = new DefaultCollectionMutator(null) {
+            @Override
+            protected void changed() {
+                changeCount++
+            }
+        }
     }
 
     @Test
@@ -51,9 +58,17 @@ class DefaultCollectionMutatorTest {
     void add() {
         def val = 'hello'
         m.add(val)
+        assertEquals 1, changeCount
         assertEquals Collections.singleton(val), m.getCollection()
     }
 
+    @Test
+    void addDuplicateDoesNotTriggerChange() {
+        m.add('hello')
+        m.add('hello') //already in the set, no change should be reflected
+        assertEquals 1, changeCount
+    }
+
     @Test
     void addCollection() {
         def vals = ['hello', 'world']
@@ -67,6 +82,17 @@ class DefaultCollectionMutatorTest {
         assertFalse i.hasNext()
     }
 
+    /**
+     * Asserts that if a collection is added, each internal addition to the collection doesn't call changed(); instead
+     * changed() is only called once after they've all been added to the collection
+     */
+    @Test
+    void addCollectionTriggersSingleChange() {
+        def c = ['hello', 'world']
+        m.add(c)
+        assertEquals 1, changeCount // only one change triggered, not c.size()
+    }
+
     @Test(expected = IllegalArgumentException)
     void addIdentifiableWithNullId() {
         def e = new Identifiable() {
@@ -96,6 +122,12 @@ class DefaultCollectionMutatorTest {
         assertEquals Collections.singleton('world'), m.getCollection()
     }
 
+    @Test
+    void removeMissingDoesNotTriggerChange() {
+        m.remove('foo') // not in the collection, no change should be registered
+        assertEquals 0, changeCount
+    }
+
     @Test
     void clear() {
         m.add('one').add('two').add(['three', 'four'])
diff --git a/impl/src/test/groovy/io/jsonwebtoken/impl/security/AbstractJwkBuilderTest.groovy b/impl/src/test/groovy/io/jsonwebtoken/impl/security/AbstractJwkBuilderTest.groovy
index ce73f8a..ee7a75d 100644
--- a/impl/src/test/groovy/io/jsonwebtoken/impl/security/AbstractJwkBuilderTest.groovy
+++ b/impl/src/test/groovy/io/jsonwebtoken/impl/security/AbstractJwkBuilderTest.groovy
@@ -239,7 +239,21 @@ class AbstractJwkBuilderTest {
                 .related(Jwks.OP.VERIFY.id).build()
         def builder = builder().operationPolicy(Jwks.OP.policy().add(op).build())
         def jwk = builder.operations().add(Collections.setOf(op, Jwks.OP.VERIFY)).and().build() as Jwk
-        assertSame op, jwk.getOperations().find({it.id == 'sign'})
+        assertSame op, jwk.getOperations().find({ it.id == 'sign' })
+    }
+
+    /**
+     * Asserts that if a .operations() builder is used, and its .and() method is not called, the change to the
+     * operations collection is still applied when building the JWK.
+     * @see <a href="https://github.com/jwtk/jjwt/issues/916">JJWT Issue 916</a>
+     * @since JJWT_RELEASE_VERSION
+     */
+    @Test
+    void testOperationsWithoutConjunction() {
+        def builder = builder()
+        builder.operations().clear().add(Jwks.OP.DERIVE_BITS) // no .and() call
+        def jwk = builder.build()
+        assertEquals(Jwks.OP.DERIVE_BITS, jwk.getOperations()[0])
     }
 
     @Test