* Ensured NestedCollections 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

Author: lhazlewood

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.

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

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

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>
      *

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>

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

@@ -17,7 +17,12 @@
 
 /**
  * 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

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>

impl/src/main/java/io/jsonwebtoken/impl/DefaultJweHeaderMutator.java

@@ -107,9 +107,8 @@ public T setCompressionAlgorithm(String zip) {
     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();
             }
         };
     }

impl/src/main/java/io/jsonwebtoken/impl/DefaultJwtParserBuilder.java

@@ -220,9 +220,8 @@ public JwtParserBuilder clock(Clock clock) {
     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 @@ private JwtParserBuilder decryptWith(final Key key) {
     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 JwtParserBuilder and() {
     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 JwtParserBuilder and() {
     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 JwtParserBuilder and() {
     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();
             }
         };
     }

impl/src/main/java/io/jsonwebtoken/impl/DelegatingClaimsMutator.java

@@ -130,12 +130,12 @@ public AudienceCollection<T> audience() {
             @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();
             }
         };
     }

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

@@ -37,38 +37,52 @@ protected final M self() {
         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);
     }

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

@@ -111,11 +111,10 @@ public T idFromThumbprint(HashAlgorithm alg) {
     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();
             }
         };
     }

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
+    }
+
 }