Uploaded image for project: 'Jenkins'
  1. Jenkins
  2. JENKINS-39324

AbstractIdCredentialsListBoxModel signature changes to avoid Item/ItemGroup ambiguity (was: take ModelObject)

XMLWordPrintable

    • credentials-plugin 1305.v04f5ec1f3743

      Background

      Currently there are pairs of methods differentiated by Item vs. ItemGroup, which is clumsy. For use from JENKINS-38048 it is desirable to let the Item overloads accept null, so that you can populate global credentials when in a global configuration screen.

      The suggestion is to deprecate all these, replacing them with methods¹ taking @Nonnull ModelObject, a common supertype. Call sites could either use @AncestorInPath ModelObject context, or continue to search for Item and pass context != null ? context : Jenkins.getInstance().

      For a context which is neither Item nor ItemGroup, to start with just fall back to Jenkins.getInstance(). Later these could provide special handling for User, Node, etc.

      To simplify passing an @NonNull Authentication argument, there should be a helper method along the lines of

      public static @NonNull Authentication authenticationOf(@Nonnull ModelObject context, @NonNull Authentication fallback) {
          return context instanceof Queue.Task ? Tasks.getAuthenticationOf((Queue.Task) context) : fallback;
      }
      

      which call sites could call something like:

      diff --git a/src/main/java/hudson/plugins/git/UserRemoteConfig.java b/src/main/java/hudson/plugins/git/UserRemoteConfig.java
      index 92bd778..92ff0dc 100644
      --- a/src/main/java/hudson/plugins/git/UserRemoteConfig.java
      +++ b/src/main/java/hudson/plugins/git/UserRemoteConfig.java
      @@ -83,7 +83,6 @@ public class UserRemoteConfig extends AbstractDescribableImpl<UserRemoteConfig>
           @Extension
           public static class DescriptorImpl extends Descriptor<UserRemoteConfig> {
       
      -        @SuppressFBWarnings(value="NP_NULL_PARAM_DEREF", justification="pending https://github.com/jenkinsci/credentials-plugin/pull/68")
               public ListBoxModel doFillCredentialsIdItems(@AncestorInPath Item project,
                                                            @QueryParameter String url,
                                                            @QueryParameter String credentialsId) {
      @@ -94,10 +93,8 @@ public class UserRemoteConfig extends AbstractDescribableImpl<UserRemoteConfig>
                   return new StandardListBoxModel()
                           .includeEmptyValue()
                           .includeMatchingAs(
      -                            project instanceof Queue.Task
      -                                    ? Tasks.getAuthenticationOf((Queue.Task) project)
      -                                    : ACL.SYSTEM,
                                   project,
      +                            AbstractIdCredentialsListBoxModel.authenticationOf(project, ACL.SYSTEM),
                                   StandardUsernameCredentials.class,
                                   GitURIRequirementsBuilder.fromUri(url).build(),
                                   GitClient.CREDENTIALS_MATCHER)
      

      ¹Perhaps methods with new names rather than overloads. Or perhaps overloads with different argument ordering. Overloads with the same argument ordering would not work, since existing overloads would be considered more specific by the Java compiler unless you disambiguated with an explicit cast.

            vlatombe Vincent Latombe
            jglick Jesse Glick
            Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

              Created:
              Updated:
              Resolved: