rename a number of constructor arguments for the various uploads to m…

…ake them more approachable

src/main/java/com/google/jenkins/plugins/storage/AbstractBucketLifecycleManager.java

@@ -65,9 +65,9 @@ public abstract class AbstractBucketLifecycleManager extends AbstractUpload {
    * NOTE: this hides the "make public" and "for failed" options of the
    * base, which are mainly relevant for true uploads.
    */
-  public AbstractBucketLifecycleManager(String bucketNameWithVars,
+  public AbstractBucketLifecycleManager(String bucket,
       @Nullable UploadModule module) {
-    super(bucketNameWithVars, false /*sharedPublicly*/,
+    super(bucket, false /*sharedPublicly*/,
         false /*forFailedJobs*/, module);
   }
 

src/main/java/com/google/jenkins/plugins/storage/AbstractUpload.java

@@ -100,20 +100,20 @@ public abstract class AbstractUpload
   /**
    * Construct the base upload from a handful of universal properties.
    *
-   * @param bucketNameWithVars The unresolved name of the storage bucket within
+   * @param bucket The unresolved name of the storage bucket within
    * which to store the resulting objects.
    * @param sharedPublicly Whether to publicly share the objects being uploaded
    * @param forFailedJobs Whether to perform the upload regardless of the
    * build's outcome
    */
-  public AbstractUpload(String bucketNameWithVars, boolean sharedPublicly,
+  public AbstractUpload(String bucket, boolean sharedPublicly,
       boolean forFailedJobs, @Nullable UploadModule module) {
     if (module != null) {
       this.module = module;
     } else {
       this.module = getDescriptor().getModule();
     }
-    this.bucketNameWithVars = checkNotNull(bucketNameWithVars);
+    this.bucketNameWithVars = checkNotNull(bucket);
     this.sharedPublicly = sharedPublicly;
     this.forFailedJobs = forFailedJobs;
   }
@@ -135,7 +135,7 @@ public final void perform(GoogleRobotCredentials credentials,
       // Turn paths containing things like $BUILD_NUMBER and $JOB_NAME into
       // their fully resolved forms.
       String bucketNameResolvedVars = Util.replaceMacro(
-          getBucketNameWithVars(), build.getEnvironment(listener));
+          getBucket(), build.getEnvironment(listener));
 
       if (!bucketNameResolvedVars.startsWith(GCS_SCHEME)) {
         listener.error(module.prefix(
@@ -242,9 +242,10 @@ public boolean forResult(Result result) {
    * The bucket name specified by the user, which potentially contains
    * unresolved symbols, such as $JOB_NAME and $BUILD_NUMBER.
    */
-  public String getBucketNameWithVars() {
+  public String getBucket() {
     return bucketNameWithVars;
   }
+  /** NOTE: old name kept for deserialization */
   private final String bucketNameWithVars;
 
   /**

src/main/java/com/google/jenkins/plugins/storage/ClassicUpload.java

@@ -23,8 +23,7 @@
 import org.kohsuke.stapler.DataBoundConstructor;
 import org.kohsuke.stapler.QueryParameter;
 
-import static com.google.common.base.Preconditions.checkNotNull;
-
+import com.google.common.base.Objects;
 import com.google.jenkins.plugins.util.Resolve;
 
 import hudson.Extension;
@@ -46,19 +45,24 @@ public class ClassicUpload extends AbstractUpload {
    * and the glob for matching files.
    */
   @DataBoundConstructor
-  public ClassicUpload(String bucketNameWithVars, boolean sharedPublicly,
+  public ClassicUpload(String bucket, boolean sharedPublicly,
       boolean forFailedJobs, @Nullable UploadModule module,
-      String sourceGlobWithVars) {
-    super(bucketNameWithVars, sharedPublicly, forFailedJobs, module);
-    this.sourceGlobWithVars = checkNotNull(sourceGlobWithVars);
+      String pattern,
+      // Legacy arguments for backwards compatibility
+      @Deprecated @Nullable String bucketNameWithVars,
+      @Deprecated @Nullable String sourceGlobWithVars) {
+    super(Objects.firstNonNull(bucket, bucketNameWithVars), sharedPublicly,
+        forFailedJobs, module);
+    this.sourceGlobWithVars =
+        Objects.firstNonNull(pattern, sourceGlobWithVars);
   }
 
   /**
    * {@inheritDoc}
    */
   @Override
   public String getDetails() {
-    return getSourceGlobWithVars();
+    return getPattern();
   }
 
   /**
@@ -70,7 +74,7 @@ protected UploadSpec getInclusions(AbstractBuild<?, ?> build,
       FilePath workspace, TaskListener listener) throws UploadException {
     try {
       String globResolvedVars = Util.replaceMacro(
-          getSourceGlobWithVars(), build.getEnvironment(listener));
+          getPattern(), build.getEnvironment(listener));
       // In order to support absolute globs (e.g. /gagent/metaOutput/std*.txt)
       // we must identify absolute paths and rebase the "workspace" to be the
       // root directory and the glob to be relative to root.
@@ -98,7 +102,7 @@ protected UploadSpec getInclusions(AbstractBuild<?, ?> build,
       }
       listener.getLogger().println(module.prefix(
           Messages.ClassicUpload_FoundForPattern(
-              inclusions.length, getSourceGlobWithVars())));
+              inclusions.length, getPattern())));
       return new UploadSpec(workspace, Arrays.asList(inclusions));
     } catch (InterruptedException e) {
       throw new UploadException(Messages.AbstractUpload_IncludeException(), e);
@@ -124,9 +128,10 @@ private FilePath getRoot(final FilePath workspace) {
    * The glob of files to upload, which potentially contains unresolved
    * symbols, such as $JOB_NAME and $BUILD_NUMBER.
    */
-  public String getSourceGlobWithVars() {
+  public String getPattern() {
     return sourceGlobWithVars;
   }
+  /** NOTE: old name kept for deserialization */
   private final String sourceGlobWithVars;
 
 
@@ -153,13 +158,13 @@ public String getDisplayName() {
     }
 
     /**
-     * This callback validates the {@code sourceGlobWithVars} input field's
+     * This callback validates the {@code pattern} input field's
      * values.
      */
-    public FormValidation doCheckSourceGlobWithVars(
-        @QueryParameter final String sourceGlobWithVars)
+    public FormValidation doCheckPattern(
+        @QueryParameter final String pattern)
         throws IOException {
-      String resolvedInput = Resolve.resolveBuiltin(sourceGlobWithVars);
+      String resolvedInput = Resolve.resolveBuiltin(pattern);
       if (resolvedInput.isEmpty()) {
         return FormValidation.error(
             Messages.ClassicUpload_EmptyGlob());

src/main/java/com/google/jenkins/plugins/storage/ExpiringBucketLifecycleManager.java

@@ -25,6 +25,7 @@
 import org.kohsuke.stapler.DataBoundConstructor;
 
 import com.google.api.services.storage.model.Bucket;
+import com.google.common.base.Objects;
 import com.google.common.collect.Lists;
 
 import hudson.Extension;
@@ -43,11 +44,14 @@ public class ExpiringBucketLifecycleManager
    * common properties.
    */
   @DataBoundConstructor
-  public ExpiringBucketLifecycleManager(String bucketNameWithVars,
-      @Nullable UploadModule module, int bucketObjectTTL) {
-    super(bucketNameWithVars, module);
-
-    this.bucketObjectTTL = bucketObjectTTL;
+  public ExpiringBucketLifecycleManager(String bucket,
+      @Nullable UploadModule module, Integer ttl,
+      // Legacy arguments for backwards compatibility
+      @Deprecated @Nullable String bucketNameWithVars,
+      @Deprecated @Nullable Integer bucketObjectTTL) {
+    super(Objects.firstNonNull(bucket, bucketNameWithVars), module);
+
+    this.bucketObjectTTL = Objects.firstNonNull(ttl, bucketObjectTTL);
   }
 
   /**
@@ -56,7 +60,7 @@ public ExpiringBucketLifecycleManager(String bucketNameWithVars,
   @Override
   public String getDetails() {
     return Messages.ExpiringBucketLifecycleManager_DetailsMessage(
-        getBucketObjectTTL());
+        getTtl());
   }
 
   /**
@@ -92,7 +96,7 @@ protected Bucket checkBucket(Bucket bucket)
       if (age == null) {
         continue;
       }
-      if (age != getBucketObjectTTL()) {
+      if (age != getTtl()) {
         continue;
       }
 
@@ -110,7 +114,7 @@ protected Bucket checkBucket(Bucket bucket)
   protected Bucket decorateBucket(Bucket bucket) {
     Bucket.Lifecycle.Rule rule = new Bucket.Lifecycle.Rule()
         .setCondition(new Bucket.Lifecycle.Rule.Condition()
-            .setAge(getBucketObjectTTL())) // age is in days
+            .setAge(getTtl())) // age is in days
         .setAction(new Bucket.Lifecycle.Rule.Action()
             .setType(DELETE));
 
@@ -140,9 +144,10 @@ protected Bucket decorateBucket(Bucket bucket) {
    * Surface the TTL for objects contained within the bucket for roundtripping
    * to the jelly UI.
    */
-  public int getBucketObjectTTL() {
+  public int getTtl() {
     return bucketObjectTTL;
   }
+  /** NOTE: old name kept for deserialization */
   private final int bucketObjectTTL;
 
   private static final String DELETE = "Delete";

src/main/java/com/google/jenkins/plugins/storage/GoogleCloudStorageUploader.java

@@ -169,7 +169,8 @@ public List<AbstractUpload> getDefaultUploads() {
       return ImmutableList.<AbstractUpload>of(
           new StdoutUpload(GCS_SCHEME,
               false /* public? */, true /* for failed? */,
-              null /* module */, "build-log.txt" /* log name */));
+              null /* module */, "build-log.txt" /* log name */,
+              null /* legacy arg: bucketNameWithVars */));
     }
 
     public List<AbstractUploadDescriptor> getUploads() {

src/main/java/com/google/jenkins/plugins/storage/StdoutUpload.java

@@ -26,6 +26,7 @@
 import static com.google.common.base.Preconditions.checkNotNull;
 import static com.google.common.io.ByteStreams.copy;
 
+import com.google.common.base.Objects;
 import com.google.common.collect.ImmutableList;
 import com.google.common.io.Closeables;
 import com.google.jenkins.plugins.util.Resolve;
@@ -52,10 +53,13 @@ public class StdoutUpload extends AbstractUpload {
    * information about how to name the build log file.
    */
   @DataBoundConstructor
-  public StdoutUpload(String bucketNameWithVars, boolean sharedPublicly,
+  public StdoutUpload(@Nullable String bucket, boolean sharedPublicly,
       boolean forFailedJobs, @Nullable UploadModule module,
-      String logName) {
-    super(bucketNameWithVars, sharedPublicly, forFailedJobs, module);
+      String logName,
+      // Legacy arguments for backwards compatibility
+      @Deprecated @Nullable String bucketNameWithVars) {
+    super(Objects.firstNonNull(bucket, bucketNameWithVars), sharedPublicly,
+        forFailedJobs, module);
     this.logName = checkNotNull(logName);
   }
 

src/main/resources/com/google/jenkins/plugins/storage/AbstractUpload/config.jelly

@@ -23,7 +23,7 @@
 
   <!-- Allow the user to specify the name of a bucket containing build
   variables to store the uploaded files -->
-  <f:entry title="${%Storage Location}" field="bucketNameWithVars">
+  <f:entry title="${%Storage Location}" field="bucket">
     <f:textbox default="gs://" />
   </f:entry>
 

src/main/resources/com/google/jenkins/plugins/storage/ClassicUpload/elements.jelly

@@ -18,7 +18,7 @@
 	 xmlns:a="/lib/auth">
   <!-- Allow the user to specify a glob containing build variables
   to be uploaded to GCS -->
-  <f:entry title="${%File Pattern}" field="sourceGlobWithVars">
+  <f:entry title="${%File Pattern}" field="pattern">
     <f:textbox />
   </f:entry>
 </j:jelly>

src/main/resources/com/google/jenkins/plugins/storage/ExpiringBucketLifecycleManager/elements.jelly

@@ -18,7 +18,7 @@
 	 xmlns:a="/lib/auth">
   <!-- Allow the user to specify the amount of time (in days) that the objects
   contained within the given bucket should last.  -->
-  <f:entry title="${%Days to retain objects}" field="bucketObjectTTL">
+  <f:entry title="${%Days to retain objects}" field="ttl">
     <f:number clazz="positive-number" />
   </f:entry>
 </j:jelly>

src/test/java/com/google/jenkins/plugins/storage/AbstractBucketLifecycleManagerTest.java

@@ -214,7 +214,7 @@ public void testGetters() {
         new MockUploadModule(executor),
         FAKE_DETAILS, null /* bucket */);
 
-    assertEquals(BUCKET_URI, underTest.getBucketNameWithVars());
+    assertEquals(BUCKET_URI, underTest.getBucket());
     assertEquals(FAKE_DETAILS, underTest.getDetails());
   }
 

src/test/java/com/google/jenkins/plugins/storage/AbstractUploadTest.java

@@ -243,7 +243,7 @@ public void testGetters() {
         FAKE_DETAILS,
         null /* uploads */);
 
-    assertEquals(BUCKET_URI, underTest.getBucketNameWithVars());
+    assertEquals(BUCKET_URI, underTest.getBucket());
     assertEquals(sharedPublicly, underTest.isSharedPublicly());
     assertEquals(forFailedJobs, underTest.isForFailedJobs());
   }

src/test/java/com/google/jenkins/plugins/storage/ClassicUploadTest.java

@@ -126,27 +126,42 @@ public void setUp() throws Exception {
     sharedPublicly = false;
     forFailedJobs = false;
     underTest = new ClassicUpload(bucket, sharedPublicly, forFailedJobs,
-        new MockUploadModule(executor), glob);
+        new MockUploadModule(executor), glob,
+        null /* legacy arg */, null /* legacy arg */);
   }
 
   @Test
   @WithoutJenkins
   public void testGetters() {
-    assertEquals(glob, underTest.getSourceGlobWithVars());
+    assertEquals(glob, underTest.getPattern());
+  }
+
+  @Test
+  @WithoutJenkins
+  public void testLegacyArgs() {
+    ClassicUpload legacyVersion = new ClassicUpload(null /* bucket */,
+        sharedPublicly, forFailedJobs, new MockUploadModule(executor),
+        null /* glob */, bucket, glob);
+    assertEquals(underTest.getBucket(), legacyVersion.getBucket());
+    assertEquals(underTest.isSharedPublicly(),
+        legacyVersion.isSharedPublicly());
+    assertEquals(underTest.isForFailedJobs(), legacyVersion.isForFailedJobs());
+    assertEquals(underTest.getPattern(), legacyVersion.getPattern());
   }
 
   @Test(expected = NullPointerException.class)
   @WithoutJenkins
   public void testCheckNullGlob() throws Exception {
     new ClassicUpload(bucket, sharedPublicly, forFailedJobs,
-        new MockUploadModule(executor), null);
+        new MockUploadModule(executor), null,
+        null /* legacy arg */, null /* legacy arg */);
   }
 
   @Test
   public void testCheckNullOnNullables() throws Exception {
     // The upload should handle null for the other fields.
     new ClassicUpload(bucket, sharedPublicly, forFailedJobs,
-        null /* module */, glob);
+        null /* module */, glob, null /* legacy arg */, null /* legacy arg */);
   }
 
   @Test
@@ -155,21 +170,21 @@ public void doCheckGlobTest() throws IOException {
     DescriptorImpl descriptor = new DescriptorImpl();
 
     assertEquals(FormValidation.Kind.OK,
-        descriptor.doCheckSourceGlobWithVars("asdf").kind);
+        descriptor.doCheckPattern("asdf").kind);
     // Some good sample globs we should accept
     assertEquals(FormValidation.Kind.OK,
-        descriptor.doCheckSourceGlobWithVars("target/*.war").kind);
+        descriptor.doCheckPattern("target/*.war").kind);
     assertEquals(FormValidation.Kind.OK,
-        descriptor.doCheckSourceGlobWithVars("**/target/foo.*").kind);
+        descriptor.doCheckPattern("**/target/foo.*").kind);
     // Successfully resolved
     assertEquals(FormValidation.Kind.OK,
-        descriptor.doCheckSourceGlobWithVars("asdf$BUILD_NUMBER").kind);
+        descriptor.doCheckPattern("asdf$BUILD_NUMBER").kind);
     // UN-successfully resolved
     assertEquals(FormValidation.Kind.ERROR,
-        descriptor.doCheckSourceGlobWithVars("$foo").kind);
+        descriptor.doCheckPattern("$foo").kind);
     // Escaped $BUILD_NUMBER
     assertEquals(FormValidation.Kind.ERROR,
-        descriptor.doCheckSourceGlobWithVars("$$BUILD_NUMBER").kind);
+        descriptor.doCheckPattern("$$BUILD_NUMBER").kind);
   }
 
   private void dumpLog(Run<?, ?> run) throws IOException {

src/test/java/com/google/jenkins/plugins/storage/ExpiringBucketLifecycleManagerTest.java

@@ -136,14 +136,25 @@ public void setUp() throws Exception {
     forbiddenException = new ForbiddenException();
 
     underTest = new ExpiringBucketLifecycleManager(BUCKET_URI,
-        new MockUploadModule(executor), TTL);
+        new MockUploadModule(executor), TTL,
+        null /* legacy arg */, null /* legacy arg */);
   }
 
   @Test
   @WithoutJenkins
   public void testGetters() {
-    assertEquals(BUCKET_URI, underTest.getBucketNameWithVars());
-    assertEquals(TTL, underTest.getBucketObjectTTL());
+    assertEquals(BUCKET_URI, underTest.getBucket());
+    assertEquals(TTL, underTest.getTtl());
+  }
+
+  @Test
+  @WithoutJenkins
+  public void testGettersWithLegacy() {
+    underTest = new ExpiringBucketLifecycleManager(null /* bucket */,
+        new MockUploadModule(executor), null /* ttl */,
+        BUCKET_URI, TTL);
+    assertEquals(BUCKET_URI, underTest.getBucket());
+    assertEquals(TTL, underTest.getTtl());
   }
 
   @Test