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

AbstractIdCredentialsListBoxModel signature changes to take ModelObject

    XMLWordPrintable

    Details

    • Similar Issues:

      Description

      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.

        Attachments

          Issue Links

            Activity

            Hide
            stephenconnolly Stephen Connolly added a comment -

            Removing myself as assignee. My current work assignments do not provide sufficient bandwidth to review these issues and in the majority of cases I am only assigned by virtue of being the default assignee. For the credentials-api and scm-api related plugins I have permission to allocate time reviewing changes to these APIs themselves to ensure these APIs remain cohesive, but that can be handled through PR reviews rather than assigning issues in JIRA

            Show
            stephenconnolly Stephen Connolly added a comment - Removing myself as assignee. My current work assignments do not provide sufficient bandwidth to review these issues and in the majority of cases I am only assigned by virtue of being the default assignee. For the credentials-api and scm-api related plugins I have permission to allocate time reviewing changes to these APIs themselves to ensure these APIs remain cohesive, but that can be handled through PR reviews rather than assigning issues in JIRA

              People

              • Assignee:
                Unassigned
                Reporter:
                jglick Jesse Glick
              • Votes:
                0 Vote for this issue
                Watchers:
                3 Start watching this issue

                Dates

                • Created:
                  Updated: