Refactor: Migrate matcher implementations to Java records Fixes #1002 by shubh586 · Pull Request #1009 · jenkinsci/credentials-plugin

shubh586 · 2026-01-11T22:20:05Z

Description

Fixes #1002
This PR modernizes the credentials plugin matcher implementations by converting them from traditional classes to Java records (Java 14+).

Changes:

Converted 8 matcher classes to records (AllOfMatcher, AnyOfMatcher, ConstantMatcher, IdMatcher, InstanceOfMatcher, NotMatcher, ScopeMatcher, UsernameMatcher)
Removed manual equals(), hashCode(), toString() implementations (auto-generated by records)
Removed serialVersionUID fields (not needed for records)
Updated CredentialsMatcher.java documentation to emphasize records as best practice
Maintained binary compatibility with existing code
Reduced boilerplate code by ~40% per matcher

This change aligns with modern Java best practices while preserving backward compatibility for existing Jenkins plugins.

Testing done

✅ Compiled successfully with mvn compile
✅ All 8 matcher files compile without errors
✅ Binary compatibility verified - constructor signatures remain identical
✅ Existing code calling matchers directly (e.g., new IdMatcher("id")) continues to work

Submitter checklist

Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
Ensure that the pull request title represents the desired changelog entry
Please describe what you did
Link to relevant issues in GitHub or Jira (Related to Deleting unused credentials query language #1001)
Link to relevant pull requests, esp. upstream and downstream changes
Ensure you have provided tests that demonstrate the feature works or the issue is fixed

shubh586 · 2026-05-13T18:23:17Z

@jglick would you please review this pr

Copilot

Pull request overview

This PR refactors credential matcher implementations from boilerplate Java classes into records and adds tests covering matcher behavior and constructors.

Changes:

Converts matcher classes such as IdMatcher, ScopeMatcher, and composite matchers to Java records.
Updates CredentialsMatcher documentation to recommend records.
Adds MatchersTest coverage for constructors, matching logic, equality, and toString.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
`src/main/java/com/cloudbees/plugins/credentials/CredentialsMatcher.java`	Updates matcher implementation guidance for records.
`src/main/java/com/cloudbees/plugins/credentials/matchers/AllOfMatcher.java`	Converts all-of matcher to a record.
`src/main/java/com/cloudbees/plugins/credentials/matchers/AnyOfMatcher.java`	Converts any-of matcher to a record.
`src/main/java/com/cloudbees/plugins/credentials/matchers/ConstantMatcher.java`	Converts constant matcher to a record.
`src/main/java/com/cloudbees/plugins/credentials/matchers/IdMatcher.java`	Converts ID matcher to a record.
`src/main/java/com/cloudbees/plugins/credentials/matchers/InstanceOfMatcher.java`	Converts type matcher to a record.
`src/main/java/com/cloudbees/plugins/credentials/matchers/NotMatcher.java`	Converts negating matcher to a record.
`src/main/java/com/cloudbees/plugins/credentials/matchers/ScopeMatcher.java`	Converts scope matcher to a record.
`src/main/java/com/cloudbees/plugins/credentials/matchers/UsernameMatcher.java`	Converts username matcher to a record.
`src/test/java/com/cloudbees/plugins/credentials/matchers/MatchersTest.java`	Adds unit tests for record-based matcher behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

-    public AllOfMatcher(@CheckForNull List<CredentialsMatcher> matchers) {
-        this.matchers = new ArrayList<>(matchers == null ? Collections.emptyList() : matchers);
+    public AllOfMatcher {
+        matchers = matchers == null ? Collections.emptyList() : new ArrayList<>(matchers);


-        this.matchers = new ArrayList<>(
-                matchers == null ? Collections.emptyList() : matchers);
+    public AnyOfMatcher {
+        matchers = matchers == null ? Collections.emptyList() : new ArrayList<>(matchers);


shubh586 · 2026-05-13T19:08:56Z

-    @NonNull
-    private final Set<CredentialsScope> scopes;
+    public ScopeMatcher {
+        Objects.requireNonNull(scopes);


+ * For legacy implementations using classes instead of records:
 * <ul>
 * <li>Implement {@link Object#toString()}</li>
- * <li>Implement {@link Object#equals(Object)} and {@link Object#hashCode()}</li>
- * <li>Define a {@code serialVersionUID} field to ensure consistent serialization</li>
+ * <li>Implement {@link Object#equals(Object)} and
+ * {@link Object#hashCode()}</li>
+ * <li>Define a {@code serialVersionUID} field to ensure consistent
+ * serialization</li>


- 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 jenkinsci#1002

shubh586 · 2026-05-14T05:21:04Z

@github-copilot review

Copilot

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

 * @since 1.5
 */
-public class AllOfMatcher implements CredentialsMatcher {
+public record AllOfMatcher(@NonNull List<CredentialsMatcher> matchers) implements CredentialsMatcher {


 * @since 1.5
 */
-public class AnyOfMatcher implements CredentialsMatcher {
+public record AnyOfMatcher(@NonNull List<CredentialsMatcher> matchers) implements CredentialsMatcher {


shubh586 · 2026-05-14T10:41:48Z

@timja will you help me in this??

yes with records, @nonnull on the component propagates to both the accessor and the canonical constructor parameter, so accepting null contradicts the advertised contract.
should i do like this,

public AllOfMatcher {
Objects.requireNonNull(matchers);
matchers = Collections.unmodifiableList(new ArrayList<>(matchers));
}

as no production caller passes null they go through CredentialsMatchers which uses Arrays.asList(). The tests would be updated to assert NullPointerException instead of testing null-tolerance.

is this approach good?

@nonnull

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.

shubh586 requested a review from a team as a code owner January 11, 2026 22:20

shubh586 changed the title ~~Refactor: Migrate matcher implementations to Java records~~ Refactor: Migrate matcher implementations to Java records Fixes #1002 Jan 12, 2026

shubh586 mentioned this pull request May 13, 2026

Switch CredentialsMatcher implementations to be records #1002

Open

shubh586 force-pushed the feature/migrate-matchers-to-records branch from 5a797b5 to 6bbcca1 Compare May 13, 2026 17:33

timja requested a review from Copilot May 13, 2026 18:37

Copilot started reviewing on behalf of timja May 13, 2026 18:38 View session

Copilot AI reviewed May 13, 2026

View reviewed changes

shubh586 force-pushed the feature/migrate-matchers-to-records branch from 6bbcca1 to 0450e6d Compare May 14, 2026 04:53

timja requested a review from Copilot May 14, 2026 09:50

Copilot started reviewing on behalf of timja May 14, 2026 09:51 View session

Copilot AI reviewed May 14, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Refactor: Migrate matcher implementations to Java records Fixes #1002#1009

Refactor: Migrate matcher implementations to Java records Fixes #1002#1009
shubh586 wants to merge 2 commits into
jenkinsci:masterfrom
shubh586:feature/migrate-matchers-to-records

shubh586 commented Jan 11, 2026 •

edited

Loading

Uh oh!

shubh586 commented May 13, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

shubh586 May 13, 2026

Uh oh!

shubh586 commented May 14, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

shubh586 commented May 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

shubh586 commented Jan 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Testing done

Submitter checklist

Uh oh!

shubh586 commented May 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

shubh586 May 13, 2026

Choose a reason for hiding this comment

Uh oh!

shubh586 commented May 14, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

shubh586 commented May 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

shubh586 commented Jan 11, 2026 •

edited

Loading

shubh586 commented May 13, 2026 •

edited

Loading