Address some plugin tech debt (#112)

* Bump parent pom * Slightly raise jenkins.version (to 2.361.4) * Set up BOM * Get rid of JSR-305 annotations * JEP-227 migration
jenkinsci · Sep 21, 2023 · 9bd7fb2 · 9bd7fb2
1 parent 622025e
commit 9bd7fb2
Show file tree

Hide file tree

Showing 25 changed files with 183 additions and 131 deletions.
diff --git a/pom.xml b/pom.xml
@@ -4,7 +4,7 @@
   <parent>
     <groupId>org.jenkins-ci.plugins</groupId>
     <artifactId>plugin</artifactId>
-    <version>4.55</version>
+    <version>4.73</version>
     <relativePath />
   </parent>
 
@@ -20,8 +20,9 @@
     <revision>1.18</revision>
     <changelist>-SNAPSHOT</changelist>
     <gitHubRepo>jenkinsci/${project.artifactId}-plugin</gitHubRepo>
-    <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
-    <jenkins.version>2.361</jenkins.version>
+    <jenkins.version>2.361.4</jenkins.version>
+    <bom>2.361.x</bom>
+    <bom.version>2102.v854b_fec19c92</bom.version>
   </properties>
 
   <scm>
@@ -44,11 +45,22 @@
     </pluginRepository>
   </pluginRepositories>
 
+  <dependencyManagement>
+    <dependencies>
+      <dependency>
+        <groupId>io.jenkins.tools.bom</groupId>
+        <artifactId>bom-${bom}</artifactId>
+        <version>${bom.version}</version>
+        <scope>import</scope>
+        <type>pom</type>
+      </dependency>
+    </dependencies>
+  </dependencyManagement>
+
   <dependencies>
     <dependency>
       <groupId>org.jenkins-ci.modules</groupId>
       <artifactId>instance-identity</artifactId>
-      <version>116.vf8f487400980</version>
     </dependency>
   </dependencies>
 

diff --git a/src/main/java/org/jenkinsci/plugins/pubsub/AbstractChannelSubscriber.java b/src/main/java/org/jenkinsci/plugins/pubsub/AbstractChannelSubscriber.java
@@ -24,8 +24,9 @@
 package org.jenkinsci.plugins.pubsub;
 
 import hudson.ExtensionPoint;
+import hudson.Util;
 import hudson.security.ACL;
-import org.acegisecurity.Authentication;
+import org.springframework.security.core.Authentication;
 
 /**
  * Simple asynchronous {@link ChannelSubscriber} {@link ExtensionPoint} for Jenkins.
@@ -47,9 +48,26 @@ public abstract class AbstractChannelSubscriber implements ChannelSubscriber, Ex
      * Override to restrict. Default is {@link ACL#SYSTEM}.
      *
      * @return The {@link Authentication} used for listening for events on the channel.
+     * @deprecated Use {@link #getAuthentication2()} instead.
      */
-    public Authentication getAuthentication() {
-        return ACL.SYSTEM;
+    @Deprecated
+    public org.acegisecurity.Authentication getAuthentication() {
+        return org.acegisecurity.Authentication.fromSpring(getAuthentication2());
+    }
+
+    /**
+     * Get the {@link Authentication} used for listening for events on the channel.
+     * <p>
+     * Override to restrict. Default is {@link ACL#SYSTEM2}.
+     *
+     * @return The {@link Authentication} used for listening for events on the channel.
+     */
+    public Authentication getAuthentication2() {
+        if (Util.isOverridden(AbstractChannelSubscriber.class, getClass(), "getAuthentication")) {
+            return getAuthentication().toSpring();
+        } else {
+            return ACL.SYSTEM2;
+        }
     }
 
     /**

diff --git a/src/main/java/org/jenkinsci/plugins/pubsub/AccessControlledMessage.java b/src/main/java/org/jenkinsci/plugins/pubsub/AccessControlledMessage.java
@@ -23,15 +23,15 @@
  */
 package org.jenkinsci.plugins.pubsub;
 
+import edu.umd.cs.findbugs.annotations.CheckForNull;
+import edu.umd.cs.findbugs.annotations.NonNull;
 import hudson.security.ACL;
 import hudson.security.AccessControlled;
 import hudson.security.Permission;
 import jenkins.model.Jenkins;
-import org.acegisecurity.AccessDeniedException;
-import org.acegisecurity.Authentication;
 
-import javax.annotation.CheckForNull;
-import javax.annotation.Nonnull;
+import org.springframework.security.access.AccessDeniedException;
+import org.springframework.security.core.Authentication;
 
 /**
  * {@link AccessControlled} {@link PubsubBus} message instance.
@@ -49,7 +49,7 @@
  * <h1>PubsubBus implementations</h1>
  * {@link PubsubBus} implementations should watch for this message subtype,
  * calling the relevant Jenkins security APIs as appropriate
- * ({@link ACL#impersonate(Authentication)}, {@link AccessControlled} etc).
+ * ({@link ACL#as2(Authentication)}, {@link AccessControlled} etc).
  * See {@link GuavaPubsubBus} implementation as an example.
  * 
  * @author <a href="mailto:tom.fennelly@gmail.com">tom.fennelly@gmail.com</a>
@@ -66,7 +66,7 @@ abstract class AccessControlledMessage<T extends AccessControlledMessage> extend
      * Get the permission required to see the message.
      * @return The permission required to see the message.
      */
-    protected abstract @Nonnull Permission getRequiredPermission();
+    protected abstract @NonNull Permission getRequiredPermission();
 
     /**
      * Get the Jenkins {@link AccessControlled} object associated with this message.
@@ -79,7 +79,7 @@ abstract class AccessControlledMessage<T extends AccessControlledMessage> extend
     /**
      * {@inheritDoc}
      */
-    public @Nonnull
+    public @NonNull
     ACL getACL() {
         AccessControlled eventItem = getAccessControlled();
         if (eventItem != null) {
@@ -93,7 +93,7 @@ ACL getACL() {
     /**
      * {@inheritDoc}
      */
-    public void checkPermission(@Nonnull Permission permission) throws AccessDeniedException {
+    public void checkPermission(@NonNull Permission permission) throws AccessDeniedException {
         if (isUnknownToJenkins()) {
             throw new AccessDeniedException(String.format("Jenkins Object '%s' Unknown.", getObjectName()));
         }
@@ -103,7 +103,7 @@ public void checkPermission(@Nonnull Permission permission) throws AccessDeniedE
     /**
      * {@inheritDoc}
      */
-    public boolean hasPermission(@Nonnull Permission permission) {
+    public boolean hasPermission(@NonNull Permission permission) {
         if (isUnknownToJenkins()) {
             return false;
         }

diff --git a/src/main/java/org/jenkinsci/plugins/pubsub/ChannelPublisher.java b/src/main/java/org/jenkinsci/plugins/pubsub/ChannelPublisher.java
@@ -23,7 +23,7 @@
  */
 package org.jenkinsci.plugins.pubsub;
 
-import javax.annotation.Nonnull;
+import edu.umd.cs.findbugs.annotations.NonNull;
 
 /**
  * @author <a href="mailto:tom.fennelly@gmail.com">tom.fennelly@gmail.com</a>
@@ -34,5 +34,5 @@ public interface ChannelPublisher {
      * Publish a message on the channel.
      * @param message The message properties.
      */
-    void publish(@Nonnull Message message);
+    void publish(@NonNull Message message);
 }
diff --git a/src/main/java/org/jenkinsci/plugins/pubsub/ChannelSubscriber.java b/src/main/java/org/jenkinsci/plugins/pubsub/ChannelSubscriber.java
@@ -23,7 +23,7 @@
  */
 package org.jenkinsci.plugins.pubsub;
 
-import javax.annotation.Nonnull;
+import edu.umd.cs.findbugs.annotations.NonNull;
 
 /**
  * @author <a href="mailto:tom.fennelly@gmail.com">tom.fennelly@gmail.com</a>
@@ -35,5 +35,5 @@ public interface ChannelSubscriber {
      * 
      * @param message The message properties.
      */
-    void onMessage(@Nonnull Message message);
+    void onMessage(@NonNull Message message);
 }
diff --git a/src/main/java/org/jenkinsci/plugins/pubsub/GuavaPubsubBus.java b/src/main/java/org/jenkinsci/plugins/pubsub/GuavaPubsubBus.java
@@ -26,18 +26,18 @@
 import com.google.common.eventbus.AsyncEventBus;
 import com.google.common.eventbus.EventBus;
 import com.google.common.eventbus.Subscribe;
+import edu.umd.cs.findbugs.annotations.NonNull;
 import hudson.security.ACL;
 import hudson.util.CopyOnWriteMap;
 import jenkins.model.Jenkins;
-import org.acegisecurity.Authentication;
 
 import javax.annotation.CheckForNull;
-import javax.annotation.Nonnull;
 import java.util.Map;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.LinkedBlockingQueue;
 import java.util.concurrent.ThreadPoolExecutor;
 import java.util.concurrent.TimeUnit;
+import org.springframework.security.core.Authentication;
 
 /**
  * Default {@link PubsubBus} implementation.
@@ -60,23 +60,23 @@ public GuavaPubsubBus() {
         start();
     }
 
-    @Nonnull
+    @NonNull
     @Override
-    protected ChannelPublisher publisher(@Nonnull String channelName) {
+    protected ChannelPublisher publisher(@NonNull String channelName) {
         final EventBus channelBus = getChannelBus(channelName);
         return channelBus::post;
     }
 
     @Override
-    public void subscribe(@Nonnull String channelName, @Nonnull ChannelSubscriber subscriber, @Nonnull Authentication authentication, @CheckForNull EventFilter eventFilter) {
+    public void subscribe2(@NonNull String channelName, @NonNull ChannelSubscriber subscriber, @NonNull Authentication authentication, @CheckForNull EventFilter eventFilter) {
         GuavaSubscriber guavaSubscriber = new GuavaSubscriber(subscriber, authentication, eventFilter);
         EventBus channelBus = getChannelBus(channelName);
         channelBus.register(guavaSubscriber);
         subscribers.put(subscriber, guavaSubscriber);
     }
 
     @Override
-    public void unsubscribe(@Nonnull String channelName, @Nonnull ChannelSubscriber subscriber) {
+    public void unsubscribe(@NonNull String channelName, @NonNull ChannelSubscriber subscriber) {
         GuavaSubscriber guavaSubscriber = subscribers.remove(subscriber);
         if (guavaSubscriber != null) {
             EventBus channelBus = getChannelBus(channelName);
@@ -117,31 +117,30 @@ private static class GuavaSubscriber {
         private Authentication authentication;
         private final EventFilter eventFilter;
 
-        public GuavaSubscriber(@Nonnull ChannelSubscriber subscriber, Authentication authentication, EventFilter eventFilter) {
+        public GuavaSubscriber(@NonNull ChannelSubscriber subscriber, Authentication authentication, EventFilter eventFilter) {
             this.subscriber = subscriber;
             if (authentication != null) {
                 this.authentication = authentication;
             } else {
-                this.authentication = Jenkins.ANONYMOUS;
+                this.authentication = Jenkins.ANONYMOUS2;
             }
             this.eventFilter = eventFilter;
         }
 
         @Subscribe
-        public void onMessage(@Nonnull final Message message) {
+        public void onMessage(@NonNull final Message message) {
             if (eventFilter != null && !message.containsAll(eventFilter)) {
                 // Don't deliver the message.
                 return;
             }
             if (message instanceof AccessControlledMessage) {
                 if (authentication != null) {
                     final AccessControlledMessage accMessage = (AccessControlledMessage) message;
-                    ACL.impersonate(authentication,
-                        () -> {
-                            if (accMessage.hasPermission(accMessage.getRequiredPermission())) {
-                                subscriber.onMessage(message.clone());
-                            }
-                        });
+                    try (var ignored = ACL.as2(authentication)) {
+                        if (accMessage.hasPermission(accMessage.getRequiredPermission())) {
+                            subscriber.onMessage(message.clone());
+                        }
+                    }
                 }
             } else {
                 subscriber.onMessage(message.clone());

diff --git a/src/main/java/org/jenkinsci/plugins/pubsub/JobChannelMessage.java b/src/main/java/org/jenkinsci/plugins/pubsub/JobChannelMessage.java
@@ -23,15 +23,15 @@
  */
 package org.jenkinsci.plugins.pubsub;
 
+import edu.umd.cs.findbugs.annotations.CheckForNull;
+import edu.umd.cs.findbugs.annotations.NonNull;
 import hudson.model.Item;
 import hudson.model.Job;
 import hudson.security.AccessControlled;
 import hudson.security.Permission;
 import jenkins.model.Jenkins;
 import jenkins.model.ParameterizedJobMixIn;
 
-import javax.annotation.CheckForNull;
-import javax.annotation.Nonnull;
 import java.util.logging.Logger;
 
 /**
@@ -53,7 +53,7 @@ public abstract class JobChannelMessage<T extends JobChannelMessage> extends Acc
         setChannelName(Events.JobChannel.NAME);
     }
 
-    public JobChannelMessage(@Nonnull Item jobChannelItem) {
+    public JobChannelMessage(@NonNull Item jobChannelItem) {
         setJobChannelItem(jobChannelItem);
     }
 
@@ -103,6 +103,7 @@ public String getJobName() {
      * Jenkins {@link Job}.
      * @deprecated Use #getJobChannelItem.
      */
+    @Deprecated
     public synchronized @CheckForNull ParameterizedJobMixIn.ParameterizedJob getJob() {
         LOGGER.warning(String.format("Unexpected call to deprecated method: %s.getJob(). Switch to using getJobChannelItem().", JobChannelMessage.class.getName()));
         if (jobChannelItem == null) {
@@ -114,7 +115,7 @@ public String getJobName() {
         return null;
     }
 
-    private synchronized void setJobChannelItem(@Nonnull Item jobChannelItem) {
+    private synchronized void setJobChannelItem(@NonNull Item jobChannelItem) {
         this.jobChannelItem = jobChannelItem;
         super.setChannelName(Events.JobChannel.NAME);
         set(EventProps.Job.job_name, jobChannelItem.getFullName());
@@ -129,7 +130,7 @@ protected AccessControlled getAccessControlled() {
         return getJobChannelItem();
     }
 
-    @Nonnull
+    @NonNull
     @Override
     protected Permission getRequiredPermission() {
         return Item.READ;

diff --git a/src/main/java/org/jenkinsci/plugins/pubsub/JobMessage.java b/src/main/java/org/jenkinsci/plugins/pubsub/JobMessage.java
@@ -23,9 +23,9 @@
  */
 package org.jenkinsci.plugins.pubsub;
 
+import edu.umd.cs.findbugs.annotations.NonNull;
 import hudson.model.Item;
 
-import javax.annotation.Nonnull;
 
 /**
  * Basic Job channel event message.
@@ -37,7 +37,7 @@ public final class JobMessage extends JobChannelMessage<JobMessage> {
     public JobMessage() {
     }
 
-    public JobMessage(@Nonnull Item jobChannelItem) {
+    public JobMessage(@NonNull Item jobChannelItem) {
         super(jobChannelItem);
     }
 

diff --git a/src/main/java/org/jenkinsci/plugins/pubsub/Message.java b/src/main/java/org/jenkinsci/plugins/pubsub/Message.java
@@ -23,14 +23,14 @@
  */
 package org.jenkinsci.plugins.pubsub;
 
+import edu.umd.cs.findbugs.annotations.CheckForNull;
+import edu.umd.cs.findbugs.annotations.NonNull;
 import hudson.model.Item;
 import jenkins.model.Jenkins;
 import net.sf.json.JSONObject;
 import org.apache.commons.codec.binary.Base64;
 import org.jenkinsci.main.modules.instance_identity.InstanceIdentity;
 
-import javax.annotation.CheckForNull;
-import javax.annotation.Nonnull;
 import java.io.IOException;
 import java.io.StringWriter;
 import java.io.Writer;
@@ -314,7 +314,7 @@ public String getEventUUID() {
      * Set {@link Item} propertis on the message instance.
      * @param item The Jenkins {@link Item}.
      */
-    protected T setItemProps(@Nonnull Item item) {
+    protected T setItemProps(@NonNull Item item) {
         set(EventProps.Jenkins.jenkins_object_name, item.getFullName());
         set(EventProps.Jenkins.jenkins_object_url, item.getUrl());
         return (T) this;
@@ -342,7 +342,7 @@ public Message clone() {
      * @return {@code true} if this message contain all of the properties supplied in the properties
      * argument, otherwise {@code false}.
      */
-    public boolean containsAll(@Nonnull Properties properties) {
+    public boolean containsAll(@NonNull Properties properties) {
         if (!properties.isEmpty()) {
             Set<Map.Entry<Object, Object>> entries = properties.entrySet();
             for (Map.Entry<Object, Object> entry : entries) {
@@ -363,7 +363,7 @@ public boolean containsAll(@Nonnull Properties properties) {
      * Write the message properties to JSON.
      * @return The message properties as a String.
      */
-    public final @Nonnull String toJSON() {
+    public final @NonNull String toJSON() {
         StringWriter writer = new StringWriter();
         try {
             toJSON(writer);
@@ -378,7 +378,7 @@ public boolean containsAll(@Nonnull Properties properties) {
      * @param writer The {@link Writer} instance.
      * @throws IOException Error writing to the {@link Writer}.
      */
-    public final void toJSON(@Nonnull Writer writer) throws IOException {
+    public final void toJSON(@NonNull Writer writer) throws IOException {
         JSONObject json = JSONObject.fromObject(this);
         json.write(writer);
         writer.flush();