From 0450e6d1b89a9ca30220dfb4accd14528e872e02 Mon Sep 17 00:00:00 2001 From: shubh586 Date: Mon, 12 Jan 2026 03:27:20 +0530 Subject: [PATCH 1/2] Refactor: Migrate matcher implementations to Java records - Convert AllOfMatcher, AnyOfMatcher, ConstantMatcher, IdMatcher, InstanceOfMatcher, NotMatcher, ScopeMatcher, and UsernameMatcher from classes to Java records - Keep serialVersionUID consistent with prior class-based implementation - Use pattern matching instanceof in IdMatcher and UsernameMatcher - Ensure mutable collections (Lists, Sets) in AllOfMatcher, AnyOfMatcher and ScopeMatcher are defensively copied and wrapped as unmodifiable - Retain binary-compatible constructors for ScopeMatcher - Update CredentialsMatcher javadoc to recommend records - Add MatchersTest covering compact constructors, pattern matching branches, and alternative constructors Fixes #1002 --- .../credentials/CredentialsMatcher.java | 15 +- .../credentials/matchers/AllOfMatcher.java | 53 +--- .../credentials/matchers/AnyOfMatcher.java | 53 +--- .../credentials/matchers/ConstantMatcher.java | 51 +-- .../credentials/matchers/IdMatcher.java | 51 +-- .../matchers/InstanceOfMatcher.java | 46 +-- .../credentials/matchers/NotMatcher.java | 46 +-- .../credentials/matchers/ScopeMatcher.java | 59 +--- .../credentials/matchers/UsernameMatcher.java | 51 +-- .../credentials/matchers/MatchersTest.java | 290 ++++++++++++++++++ 10 files changed, 346 insertions(+), 369 deletions(-) create mode 100644 src/test/java/com/cloudbees/plugins/credentials/matchers/MatchersTest.java diff --git a/src/main/java/com/cloudbees/plugins/credentials/CredentialsMatcher.java b/src/main/java/com/cloudbees/plugins/credentials/CredentialsMatcher.java index 1e1fe936..fb79ce7e 100644 --- a/src/main/java/com/cloudbees/plugins/credentials/CredentialsMatcher.java +++ b/src/main/java/com/cloudbees/plugins/credentials/CredentialsMatcher.java @@ -27,11 +27,20 @@ import java.io.Serializable; /** - * Something that matches credentials. Best practice is to + * Something that matches credentials. Best practice is to use Java records, + * which automatically provide + * {@code equals()}, {@code hashCode()}, and {@code toString()} implementations. + * When using records, ensure + * the canonical constructor maintains binary compatibility with existing code + * that may instantiate matchers directly. + *

+ * Implementations should define a {@code serialVersionUID} field to ensure consistent serialization. + *

+ * For legacy implementations using classes instead of records: *

* * @since 1.5 diff --git a/src/main/java/com/cloudbees/plugins/credentials/matchers/AllOfMatcher.java b/src/main/java/com/cloudbees/plugins/credentials/matchers/AllOfMatcher.java index bef2e19d..bb90c12c 100644 --- a/src/main/java/com/cloudbees/plugins/credentials/matchers/AllOfMatcher.java +++ b/src/main/java/com/cloudbees/plugins/credentials/matchers/AllOfMatcher.java @@ -25,7 +25,6 @@ import com.cloudbees.plugins.credentials.Credentials; import com.cloudbees.plugins.credentials.CredentialsMatcher; -import edu.umd.cs.findbugs.annotations.CheckForNull; import edu.umd.cs.findbugs.annotations.NonNull; import java.util.ArrayList; import java.util.Collections; @@ -36,7 +35,7 @@ * * @since 1.5 */ -public class AllOfMatcher implements CredentialsMatcher { +public record AllOfMatcher(@NonNull List matchers) implements CredentialsMatcher { /** * Standardize serialization. * @@ -45,59 +44,17 @@ public class AllOfMatcher implements CredentialsMatcher { private static final long serialVersionUID = 2161005681083022432L; /** - * The matchers to match. + * Compact constructor that normalizes the list. */ - @NonNull - private final List matchers; - - /** - * Creates a new instance. - * - * @param matchers the matchers to match. - */ - public AllOfMatcher(@CheckForNull List matchers) { - this.matchers = new ArrayList<>(matchers == null ? Collections.emptyList() : matchers); + public AllOfMatcher { + matchers = matchers == null ? Collections.emptyList() : Collections.unmodifiableList(new ArrayList<>(matchers)); } /** * {@inheritDoc} */ + @Override public boolean matches(@NonNull Credentials item) { return matchers.stream().allMatch(matcher -> matcher.matches(item)); } - - /** - * {@inheritDoc} - */ - @Override - public int hashCode() { - return matchers.hashCode(); - } - - /** - * {@inheritDoc} - */ - @Override - public boolean equals(Object o) { - if (this == o) { - return true; - } - if (o == null || getClass() != o.getClass()) { - return false; - } - - AllOfMatcher that = (AllOfMatcher) o; - - return matchers.equals(that.matchers); - - } - - /** - * {@inheritDoc} - */ - @Override - public String toString() { - return "AllMatcher{" + "matchers=" + matchers + - '}'; - } } diff --git a/src/main/java/com/cloudbees/plugins/credentials/matchers/AnyOfMatcher.java b/src/main/java/com/cloudbees/plugins/credentials/matchers/AnyOfMatcher.java index 342ce691..00d40999 100644 --- a/src/main/java/com/cloudbees/plugins/credentials/matchers/AnyOfMatcher.java +++ b/src/main/java/com/cloudbees/plugins/credentials/matchers/AnyOfMatcher.java @@ -25,7 +25,6 @@ import com.cloudbees.plugins.credentials.Credentials; import com.cloudbees.plugins.credentials.CredentialsMatcher; -import edu.umd.cs.findbugs.annotations.CheckForNull; import edu.umd.cs.findbugs.annotations.NonNull; import java.util.ArrayList; import java.util.Collections; @@ -36,68 +35,26 @@ * * @since 1.5 */ -public class AnyOfMatcher implements CredentialsMatcher { +public record AnyOfMatcher(@NonNull List matchers) implements CredentialsMatcher { /** * Standardize serialization. * * @since 2.1.0 */ private static final long serialVersionUID = 8214348092732916263L; - /** - * The matchers to match. - */ - @NonNull - private final List matchers; /** - * Creates a new instance. - * - * @param matchers the matchers to match. + * Compact constructor that normalizes the list. */ - public AnyOfMatcher(@CheckForNull List matchers) { - this.matchers = new ArrayList<>( - matchers == null ? Collections.emptyList() : matchers); + public AnyOfMatcher { + matchers = matchers == null ? Collections.emptyList() : Collections.unmodifiableList(new ArrayList<>(matchers)); } /** * {@inheritDoc} */ + @Override public boolean matches(@NonNull Credentials item) { return matchers.stream().anyMatch(matcher -> matcher.matches(item)); } - - /** - * {@inheritDoc} - */ - @Override - public int hashCode() { - return matchers.hashCode(); - } - - /** - * {@inheritDoc} - */ - @Override - public boolean equals(Object o) { - if (this == o) { - return true; - } - if (o == null || getClass() != o.getClass()) { - return false; - } - - AnyOfMatcher that = (AnyOfMatcher) o; - - return matchers.equals(that.matchers); - - } - - /** - * {@inheritDoc} - */ - @Override - public String toString() { - return "AnyMatcher{" + "matchers=" + matchers + - '}'; - } } diff --git a/src/main/java/com/cloudbees/plugins/credentials/matchers/ConstantMatcher.java b/src/main/java/com/cloudbees/plugins/credentials/matchers/ConstantMatcher.java index 0f152335..d7632b75 100644 --- a/src/main/java/com/cloudbees/plugins/credentials/matchers/ConstantMatcher.java +++ b/src/main/java/com/cloudbees/plugins/credentials/matchers/ConstantMatcher.java @@ -32,66 +32,19 @@ * * @since 1.5 */ -public class ConstantMatcher implements CredentialsMatcher { +public record ConstantMatcher(boolean match) implements CredentialsMatcher { /** * Standardize serialization. * * @since 2.1.0 */ private static final long serialVersionUID = 8270819649776908382L; - /** - * Whether to match. - */ - private final boolean match; - - /** - * Constructs a new instance. - * - * @param match whether to match or not. - */ - public ConstantMatcher(boolean match) { - this.match = match; - } /** * {@inheritDoc} */ + @Override public boolean matches(@NonNull Credentials item) { return match; } - - /** - * {@inheritDoc} - */ - @Override - public int hashCode() { - return (match ? 1 : 0); - } - - /** - * {@inheritDoc} - */ - @Override - public boolean equals(Object o) { - if (this == o) { - return true; - } - if (o == null || getClass() != o.getClass()) { - return false; - } - - ConstantMatcher that = (ConstantMatcher) o; - - return match == that.match; - - } - - /** - * {@inheritDoc} - */ - @Override - public String toString() { - return "ConstantMatcher{" + "match=" + match + - '}'; - } } diff --git a/src/main/java/com/cloudbees/plugins/credentials/matchers/IdMatcher.java b/src/main/java/com/cloudbees/plugins/credentials/matchers/IdMatcher.java index 06fdce87..3a70db83 100644 --- a/src/main/java/com/cloudbees/plugins/credentials/matchers/IdMatcher.java +++ b/src/main/java/com/cloudbees/plugins/credentials/matchers/IdMatcher.java @@ -31,72 +31,33 @@ import java.util.Objects; /** - * Matches credentials that are {@link IdCredentials} and have the specified {@link IdCredentials#getId()}. + * Matches credentials that are {@link IdCredentials} and have the specified + * {@link IdCredentials#getId()}. * * @since 1.5 */ -public class IdMatcher implements CredentialsMatcher { +public record IdMatcher(@NonNull String id) implements CredentialsMatcher { /** * Standardize serialization. * * @since 2.1.0 */ private static final long serialVersionUID = -2400504567993839126L; - /** - * The id to match. - */ - @NonNull - private final String id; /** * Constructs a new instance. * * @param id the id to match. */ - public IdMatcher(@NonNull String id) { + public IdMatcher { Objects.requireNonNull(id); - this.id = id; - } - - /** - * {@inheritDoc} - */ - public boolean matches(@NonNull Credentials item) { - return item instanceof IdCredentials && id.equals(((IdCredentials) item).getId()); - } - - /** - * {@inheritDoc} - */ - @Override - public int hashCode() { - return id.hashCode(); - } - - /** - * {@inheritDoc} - */ - @Override - public boolean equals(Object o) { - if (this == o) { - return true; - } - if (o == null || getClass() != o.getClass()) { - return false; - } - - IdMatcher idMatcher = (IdMatcher) o; - - return id.equals(idMatcher.id); - } /** * {@inheritDoc} */ @Override - public String toString() { - return "IdMatcher{" + "id='" + id + '\'' + - '}'; + public boolean matches(@NonNull Credentials item) { + return item instanceof IdCredentials idc && id.equals(idc.getId()); } } diff --git a/src/main/java/com/cloudbees/plugins/credentials/matchers/InstanceOfMatcher.java b/src/main/java/com/cloudbees/plugins/credentials/matchers/InstanceOfMatcher.java index 682caf8c..234f5ead 100644 --- a/src/main/java/com/cloudbees/plugins/credentials/matchers/InstanceOfMatcher.java +++ b/src/main/java/com/cloudbees/plugins/credentials/matchers/InstanceOfMatcher.java @@ -34,68 +34,28 @@ * * @since 1.5 */ -public class InstanceOfMatcher implements CredentialsMatcher { +public record InstanceOfMatcher(@NonNull Class clazz) implements CredentialsMatcher { /** * Standardize serialization. * * @since 2.1.0 */ private static final long serialVersionUID = 7841840317353807524L; - /** - * The type that the credentials must implement - */ - @NonNull - private final Class clazz; /** * Constructs a new instance. * * @param clazz the type that credentials must implement. */ - public InstanceOfMatcher(@NonNull Class clazz) { + public InstanceOfMatcher { Objects.requireNonNull(clazz); - this.clazz = clazz; } /** * {@inheritDoc} */ + @Override public boolean matches(@NonNull Credentials item) { return clazz.isInstance(item); } - - /** - * {@inheritDoc} - */ - @Override - public int hashCode() { - return clazz.hashCode(); - } - - /** - * {@inheritDoc} - */ - @Override - public boolean equals(Object o) { - if (this == o) { - return true; - } - if (o == null || getClass() != o.getClass()) { - return false; - } - - InstanceOfMatcher that = (InstanceOfMatcher) o; - - return clazz.equals(that.clazz); - - } - - /** - * {@inheritDoc} - */ - @Override - public String toString() { - return "InstanceOfMatcher{" + "clazz=" + clazz + - '}'; - } } diff --git a/src/main/java/com/cloudbees/plugins/credentials/matchers/NotMatcher.java b/src/main/java/com/cloudbees/plugins/credentials/matchers/NotMatcher.java index adfcbc0e..2bab5ab7 100644 --- a/src/main/java/com/cloudbees/plugins/credentials/matchers/NotMatcher.java +++ b/src/main/java/com/cloudbees/plugins/credentials/matchers/NotMatcher.java @@ -34,68 +34,28 @@ * * @since 1.5 */ -public class NotMatcher implements CredentialsMatcher { +public record NotMatcher(@NonNull CredentialsMatcher matcher) implements CredentialsMatcher { /** * Standardize serialization. * * @since 2.1.0 */ private static final long serialVersionUID = 3301127941013284754L; - /** - * The matchers to match. - */ - @NonNull - private final CredentialsMatcher matcher; /** * Creates a new instance. * * @param matcher the matcher to invert the match of. */ - public NotMatcher(@NonNull CredentialsMatcher matcher) { + public NotMatcher { Objects.requireNonNull(matcher); - this.matcher = matcher; } /** * {@inheritDoc} */ + @Override public boolean matches(@NonNull Credentials item) { return !matcher.matches(item); } - - /** - * {@inheritDoc} - */ - @Override - public int hashCode() { - return matcher.hashCode(); - } - - /** - * {@inheritDoc} - */ - @Override - public boolean equals(Object o) { - if (this == o) { - return true; - } - if (o == null || getClass() != o.getClass()) { - return false; - } - - NotMatcher that = (NotMatcher) o; - - return matcher.equals(that.matcher); - - } - - /** - * {@inheritDoc} - */ - @Override - public String toString() { - return "NotMatcher{" + "matcher=" + matcher + - '}'; - } } diff --git a/src/main/java/com/cloudbees/plugins/credentials/matchers/ScopeMatcher.java b/src/main/java/com/cloudbees/plugins/credentials/matchers/ScopeMatcher.java index 04e6a3d8..24026e34 100644 --- a/src/main/java/com/cloudbees/plugins/credentials/matchers/ScopeMatcher.java +++ b/src/main/java/com/cloudbees/plugins/credentials/matchers/ScopeMatcher.java @@ -26,7 +26,6 @@ import com.cloudbees.plugins.credentials.Credentials; import com.cloudbees.plugins.credentials.CredentialsMatcher; import com.cloudbees.plugins.credentials.CredentialsScope; -import com.cloudbees.plugins.credentials.common.IdCredentials; import edu.umd.cs.findbugs.annotations.NonNull; import java.util.Arrays; import java.util.Collection; @@ -36,22 +35,27 @@ import java.util.Set; /** - * Matches credentials that are {@link IdCredentials} and have the specified {@link CredentialsScope}(s). + * Matches credentials that have the specified {@link CredentialsScope}(s). * * @since 1.5 */ -public class ScopeMatcher implements CredentialsMatcher { +public record ScopeMatcher(@NonNull Set scopes) implements CredentialsMatcher { /** * Standardize serialization. * * @since 2.1.0 */ private static final long serialVersionUID = -7786779595366393177L; + /** - * The scopes to match. + * Constructs a new instance. + * + * @param scopes the scopes to match. */ - @NonNull - private final Set scopes; + public ScopeMatcher { + Objects.requireNonNull(scopes); + scopes = scopes.isEmpty() ? Collections.emptySet() : Collections.unmodifiableSet(EnumSet.copyOf(scopes)); + } /** * Constructs a new instance. @@ -59,8 +63,7 @@ public class ScopeMatcher implements CredentialsMatcher { * @param scope the scope to match. */ public ScopeMatcher(@NonNull CredentialsScope scope) { - Objects.requireNonNull(scope); - this.scopes = Collections.singleton(scope); + this(Collections.singleton(Objects.requireNonNull(scope))); } /** @@ -69,7 +72,7 @@ public ScopeMatcher(@NonNull CredentialsScope scope) { * @param scopes the scopes to match. */ public ScopeMatcher(@NonNull CredentialsScope... scopes) { - this.scopes = EnumSet.copyOf(Arrays.asList(scopes)); + this(EnumSet.copyOf(Arrays.asList(scopes))); } /** @@ -78,48 +81,14 @@ public ScopeMatcher(@NonNull CredentialsScope... scopes) { * @param scopes the scopes to match. */ public ScopeMatcher(@NonNull Collection scopes) { - this.scopes = EnumSet.copyOf(scopes); + this(EnumSet.copyOf(scopes)); } /** * {@inheritDoc} */ + @Override public boolean matches(@NonNull Credentials item) { return scopes.contains(item.getScope()); } - - /** - * {@inheritDoc} - */ - @Override - public int hashCode() { - return scopes.hashCode(); - } - - /** - * {@inheritDoc} - */ - @Override - public boolean equals(Object o) { - if (this == o) { - return true; - } - if (o == null || getClass() != o.getClass()) { - return false; - } - - ScopeMatcher that = (ScopeMatcher) o; - - return scopes.equals(that.scopes); - - } - - /** - * {@inheritDoc} - */ - @Override - public String toString() { - return "ScopeMatcher{" + "scopes=" + scopes + - '}'; - } } diff --git a/src/main/java/com/cloudbees/plugins/credentials/matchers/UsernameMatcher.java b/src/main/java/com/cloudbees/plugins/credentials/matchers/UsernameMatcher.java index 70ac077c..4d3d6f86 100644 --- a/src/main/java/com/cloudbees/plugins/credentials/matchers/UsernameMatcher.java +++ b/src/main/java/com/cloudbees/plugins/credentials/matchers/UsernameMatcher.java @@ -31,73 +31,34 @@ import java.util.Objects; /** - * Matches credentials that are {@link UsernameCredentials} and have the specified {@link + * Matches credentials that are {@link UsernameCredentials} and have the + * specified {@link * UsernameCredentials#getUsername()} * * @since 1.5 */ -public class UsernameMatcher implements CredentialsMatcher { +public record UsernameMatcher(@NonNull String username) implements CredentialsMatcher { /** * Standardize serialization. * * @since 2.1.0 */ private static final long serialVersionUID = -2166795904091485580L; - /** - * The username to match. - */ - @NonNull - private final String username; /** * Constructs a new instance. * * @param username the username to match. */ - public UsernameMatcher(@NonNull String username) { + public UsernameMatcher { Objects.requireNonNull(username); - this.username = username; - } - - /** - * {@inheritDoc} - */ - public boolean matches(@NonNull Credentials item) { - return item instanceof UsernameCredentials && username.equals(((UsernameCredentials) item).getUsername()); - } - - /** - * {@inheritDoc} - */ - @Override - public int hashCode() { - return username.hashCode(); - } - - /** - * {@inheritDoc} - */ - @Override - public boolean equals(Object o) { - if (this == o) { - return true; - } - if (o == null || getClass() != o.getClass()) { - return false; - } - - UsernameMatcher that = (UsernameMatcher) o; - - return username.equals(that.username); - } /** * {@inheritDoc} */ @Override - public String toString() { - return "UsernameMatcher{" + "username='" + username + '\'' + - '}'; + public boolean matches(@NonNull Credentials item) { + return item instanceof UsernameCredentials uc && username.equals(uc.getUsername()); } } diff --git a/src/test/java/com/cloudbees/plugins/credentials/matchers/MatchersTest.java b/src/test/java/com/cloudbees/plugins/credentials/matchers/MatchersTest.java new file mode 100644 index 00000000..d9697d96 --- /dev/null +++ b/src/test/java/com/cloudbees/plugins/credentials/matchers/MatchersTest.java @@ -0,0 +1,290 @@ +/* + * The MIT License + * + * Copyright (c) 2011-2012, CloudBees, Inc., Stephen Connolly. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ +package com.cloudbees.plugins.credentials.matchers; + +import com.cloudbees.plugins.credentials.Credentials; +import com.cloudbees.plugins.credentials.CredentialsMatcher; +import com.cloudbees.plugins.credentials.CredentialsScope; +import com.cloudbees.plugins.credentials.common.IdCredentials; +import com.cloudbees.plugins.credentials.common.UsernameCredentials; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +/** + * Tests for the record-based matcher implementations, covering compact + * constructors, pattern matching branches, and alternative constructors. + */ +class MatchersTest { + + // ---- AllOfMatcher ---- + + @Test + void allOfMatcherHandlesNullList() { + AllOfMatcher matcher = new AllOfMatcher(null); + assertNotNull(matcher.matchers()); + assertTrue(matcher.matchers().isEmpty()); + } + + @Test + void allOfMatcherDefensiveCopy() { + CredentialsMatcher always = new ConstantMatcher(true); + List original = new java.util.ArrayList<>(); + original.add(always); + AllOfMatcher matcher = new AllOfMatcher(original); + original.clear(); + assertFalse(matcher.matchers().isEmpty(), "Should not be affected by mutation of original list"); + } + + @Test + void allOfMatcherMatchesWhenAllMatch() { + Credentials cred = mock(Credentials.class); + AllOfMatcher matcher = new AllOfMatcher(List.of( + new ConstantMatcher(true), + new ConstantMatcher(true))); + assertTrue(matcher.matches(cred)); + } + + @Test + void allOfMatcherFailsWhenOneDoesNotMatch() { + Credentials cred = mock(Credentials.class); + AllOfMatcher matcher = new AllOfMatcher(List.of( + new ConstantMatcher(true), + new ConstantMatcher(false))); + assertFalse(matcher.matches(cred)); + } + + // ---- AnyOfMatcher ---- + + @Test + void anyOfMatcherHandlesNullList() { + AnyOfMatcher matcher = new AnyOfMatcher(null); + assertNotNull(matcher.matchers()); + assertTrue(matcher.matchers().isEmpty()); + } + + @Test + void anyOfMatcherDefensiveCopy() { + CredentialsMatcher always = new ConstantMatcher(true); + List original = new java.util.ArrayList<>(); + original.add(always); + AnyOfMatcher matcher = new AnyOfMatcher(original); + original.clear(); + assertFalse(matcher.matchers().isEmpty(), "Should not be affected by mutation of original list"); + } + + @Test + void anyOfMatcherMatchesWhenOneMatches() { + Credentials cred = mock(Credentials.class); + AnyOfMatcher matcher = new AnyOfMatcher(List.of( + new ConstantMatcher(false), + new ConstantMatcher(true))); + assertTrue(matcher.matches(cred)); + } + + @Test + void anyOfMatcherFailsWhenNoneMatch() { + Credentials cred = mock(Credentials.class); + AnyOfMatcher matcher = new AnyOfMatcher(List.of( + new ConstantMatcher(false), + new ConstantMatcher(false))); + assertFalse(matcher.matches(cred)); + } + + // ---- IdMatcher ---- + + @Test + void idMatcherRejectsNonIdCredentials() { + Credentials cred = mock(Credentials.class); + IdMatcher matcher = new IdMatcher("test-id"); + assertFalse(matcher.matches(cred), "Should not match non-IdCredentials"); + } + + @Test + void idMatcherMatchesCorrectId() { + IdCredentials cred = mock(IdCredentials.class); + when(cred.getId()).thenReturn("test-id"); + IdMatcher matcher = new IdMatcher("test-id"); + assertTrue(matcher.matches(cred)); + } + + @Test + void idMatcherRejectsMismatchedId() { + IdCredentials cred = mock(IdCredentials.class); + when(cred.getId()).thenReturn("other-id"); + IdMatcher matcher = new IdMatcher("test-id"); + assertFalse(matcher.matches(cred)); + } + + @Test + void idMatcherRejectsNullId() { + assertThrows(NullPointerException.class, () -> new IdMatcher(null)); + } + + // ---- UsernameMatcher ---- + + @Test + void usernameMatcherCompactConstructorRejectsNull() { + assertThrows(NullPointerException.class, () -> new UsernameMatcher(null)); + } + + @Test + void usernameMatcherRejectsNonUsernameCredentials() { + Credentials cred = mock(Credentials.class); + UsernameMatcher matcher = new UsernameMatcher("admin"); + assertFalse(matcher.matches(cred), "Should not match non-UsernameCredentials"); + } + + @Test + void usernameMatcherMatchesCorrectUsername() { + UsernameCredentials cred = mock(UsernameCredentials.class); + when(cred.getUsername()).thenReturn("admin"); + UsernameMatcher matcher = new UsernameMatcher("admin"); + assertTrue(matcher.matches(cred)); + } + + @Test + void usernameMatcherRejectsMismatchedUsername() { + UsernameCredentials cred = mock(UsernameCredentials.class); + when(cred.getUsername()).thenReturn("user"); + UsernameMatcher matcher = new UsernameMatcher("admin"); + assertFalse(matcher.matches(cred)); + } + + // ---- InstanceOfMatcher ---- + + @Test + void instanceOfMatcherCompactConstructorRejectsNull() { + assertThrows(NullPointerException.class, () -> new InstanceOfMatcher(null)); + } + + @Test + void instanceOfMatcherMatchesCorrectType() { + IdCredentials cred = mock(IdCredentials.class); + InstanceOfMatcher matcher = new InstanceOfMatcher(IdCredentials.class); + assertTrue(matcher.matches(cred)); + } + + @Test + void instanceOfMatcherRejectsWrongType() { + Credentials cred = mock(Credentials.class); + InstanceOfMatcher matcher = new InstanceOfMatcher(IdCredentials.class); + assertFalse(matcher.matches(cred)); + } + + // ---- ScopeMatcher ---- + + @Test + void scopeMatcherVarargsConstructor() { + ScopeMatcher matcher = new ScopeMatcher(CredentialsScope.GLOBAL, CredentialsScope.SYSTEM); + assertTrue(matcher.scopes().contains(CredentialsScope.GLOBAL)); + assertTrue(matcher.scopes().contains(CredentialsScope.SYSTEM)); + } + + @Test + void scopeMatcherCollectionConstructor() { + ScopeMatcher matcher = new ScopeMatcher( + Arrays.asList(CredentialsScope.GLOBAL, CredentialsScope.USER)); + assertTrue(matcher.scopes().contains(CredentialsScope.GLOBAL)); + assertTrue(matcher.scopes().contains(CredentialsScope.USER)); + } + + @Test + void scopeMatcherSingleScopeConstructor() { + ScopeMatcher matcher = new ScopeMatcher(CredentialsScope.SYSTEM); + assertEquals(Collections.singleton(CredentialsScope.SYSTEM), matcher.scopes()); + } + + @Test + void scopeMatcherMatchesCredentialWithMatchingScope() { + Credentials cred = mock(Credentials.class); + when(cred.getScope()).thenReturn(CredentialsScope.GLOBAL); + ScopeMatcher matcher = new ScopeMatcher(CredentialsScope.GLOBAL, CredentialsScope.SYSTEM); + assertTrue(matcher.matches(cred)); + } + + @Test + void scopeMatcherRejectsCredentialWithNonMatchingScope() { + Credentials cred = mock(Credentials.class); + when(cred.getScope()).thenReturn(CredentialsScope.USER); + ScopeMatcher matcher = new ScopeMatcher(CredentialsScope.GLOBAL, CredentialsScope.SYSTEM); + assertFalse(matcher.matches(cred)); + } + + @Test + void scopeMatcherCanonicalConstructorRejectsNull() { + assertThrows(NullPointerException.class, () -> new ScopeMatcher((java.util.Set) null)); + } + + // ---- NotMatcher ---- + + @Test + void notMatcherCompactConstructorRejectsNull() { + assertThrows(NullPointerException.class, () -> new NotMatcher(null)); + } + + @Test + void notMatcherInvertsResult() { + Credentials cred = mock(Credentials.class); + NotMatcher matcher = new NotMatcher(new ConstantMatcher(true)); + assertFalse(matcher.matches(cred)); + + NotMatcher matcher2 = new NotMatcher(new ConstantMatcher(false)); + assertTrue(matcher2.matches(cred)); + } + + // ---- ConstantMatcher ---- + + @Test + void constantMatcherAlwaysReturnsConfiguredValue() { + Credentials cred = mock(Credentials.class); + assertTrue(new ConstantMatcher(true).matches(cred)); + assertFalse(new ConstantMatcher(false).matches(cred)); + } + + // ---- Record equality/toString (auto-generated by records) ---- + + @Test + void recordEqualityAndHashCode() { + IdMatcher a = new IdMatcher("id1"); + IdMatcher b = new IdMatcher("id1"); + IdMatcher c = new IdMatcher("id2"); + + assertEquals(a, b); + assertEquals(a.hashCode(), b.hashCode()); + assertNotEquals(a, c); + } + + @Test + void recordToStringIsReadable() { + IdMatcher matcher = new IdMatcher("my-id"); + String str = matcher.toString(); + assertTrue(str.contains("my-id"), "toString should contain the id value"); + } +} From 7efe7bb579c4d631a6cc0c77f5f0e2bbcaf3ea19 Mon Sep 17 00:00:00 2001 From: shubh586 Date: Thu, 14 May 2026 17:28:54 +0530 Subject: [PATCH 2/2] Fix @NonNull contract: reject null in AllOfMatcher and AnyOfMatcher MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace null-tolerant compact constructors with Objects.requireNonNull() to align with the @NonNull annotation on the record component. This makes the API contract consistent — no production caller passes null. Update tests to assert NullPointerException instead of null-tolerance. --- .../plugins/credentials/matchers/AllOfMatcher.java | 6 ++++-- .../plugins/credentials/matchers/AnyOfMatcher.java | 6 ++++-- .../plugins/credentials/matchers/MatchersTest.java | 12 ++++-------- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/main/java/com/cloudbees/plugins/credentials/matchers/AllOfMatcher.java b/src/main/java/com/cloudbees/plugins/credentials/matchers/AllOfMatcher.java index bb90c12c..cfd8219b 100644 --- a/src/main/java/com/cloudbees/plugins/credentials/matchers/AllOfMatcher.java +++ b/src/main/java/com/cloudbees/plugins/credentials/matchers/AllOfMatcher.java @@ -29,6 +29,7 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; +import java.util.Objects; /** * Matches all of the supplied matchers. @@ -44,10 +45,11 @@ public record AllOfMatcher(@NonNull List matchers) implement private static final long serialVersionUID = 2161005681083022432L; /** - * Compact constructor that normalizes the list. + * Compact constructor that defensively copies and makes the list unmodifiable. */ public AllOfMatcher { - matchers = matchers == null ? Collections.emptyList() : Collections.unmodifiableList(new ArrayList<>(matchers)); + Objects.requireNonNull(matchers); + matchers = Collections.unmodifiableList(new ArrayList<>(matchers)); } /** diff --git a/src/main/java/com/cloudbees/plugins/credentials/matchers/AnyOfMatcher.java b/src/main/java/com/cloudbees/plugins/credentials/matchers/AnyOfMatcher.java index 00d40999..ac6872aa 100644 --- a/src/main/java/com/cloudbees/plugins/credentials/matchers/AnyOfMatcher.java +++ b/src/main/java/com/cloudbees/plugins/credentials/matchers/AnyOfMatcher.java @@ -29,6 +29,7 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; +import java.util.Objects; /** * Matches any of the supplied matchers. @@ -44,10 +45,11 @@ public record AnyOfMatcher(@NonNull List matchers) implement private static final long serialVersionUID = 8214348092732916263L; /** - * Compact constructor that normalizes the list. + * Compact constructor that defensively copies and makes the list unmodifiable. */ public AnyOfMatcher { - matchers = matchers == null ? Collections.emptyList() : Collections.unmodifiableList(new ArrayList<>(matchers)); + Objects.requireNonNull(matchers); + matchers = Collections.unmodifiableList(new ArrayList<>(matchers)); } /** diff --git a/src/test/java/com/cloudbees/plugins/credentials/matchers/MatchersTest.java b/src/test/java/com/cloudbees/plugins/credentials/matchers/MatchersTest.java index d9697d96..0f04e8a6 100644 --- a/src/test/java/com/cloudbees/plugins/credentials/matchers/MatchersTest.java +++ b/src/test/java/com/cloudbees/plugins/credentials/matchers/MatchersTest.java @@ -46,10 +46,8 @@ class MatchersTest { // ---- AllOfMatcher ---- @Test - void allOfMatcherHandlesNullList() { - AllOfMatcher matcher = new AllOfMatcher(null); - assertNotNull(matcher.matchers()); - assertTrue(matcher.matchers().isEmpty()); + void allOfMatcherRejectsNullList() { + assertThrows(NullPointerException.class, () -> new AllOfMatcher(null)); } @Test @@ -83,10 +81,8 @@ void allOfMatcherFailsWhenOneDoesNotMatch() { // ---- AnyOfMatcher ---- @Test - void anyOfMatcherHandlesNullList() { - AnyOfMatcher matcher = new AnyOfMatcher(null); - assertNotNull(matcher.matchers()); - assertTrue(matcher.matchers().isEmpty()); + void anyOfMatcherRejectsNullList() { + assertThrows(NullPointerException.class, () -> new AnyOfMatcher(null)); } @Test