Merge pull request #1 from jtnord/JENKINS-28843

[FIXED JENKINS-28843] No longer stored the unique value in the items configuration.

.gitignore

@@ -0,0 +1,5 @@
+/target/
+/work/
+/.classpath
+/.project
+/.settings

pom.xml

@@ -8,7 +8,7 @@
 
   <groupId>org.jenkins-ci.plugins</groupId>
   <artifactId>unique-id</artifactId>
-  <version>1.3-SNAPSHOT</version>
+  <version>2.0-SNAPSHOT</version>
   <packaging>hpi</packaging>
   <name>Unique ID Library Plugin</name>
   <url>http://wiki.jenkins-ci.org/display/JENKINS/Unique+Id+Plugin</url>
@@ -17,16 +17,38 @@
     <connection>scm:git:ssh://github.com/jenkinsci/unique-id-plugin.git</connection>
     <developerConnection>scm:git:ssh://git@github.com/jenkinsci/unique-id-plugin.git</developerConnection>
     <url>https://github.com/jenkinsci/unique-id-plugin</url>
-    <tag>unique-id-1.2</tag>
   </scm>
 
+  <properties>
+     <maven.compiler.source>1.7</maven.compiler.source>
+     <maven.compiler.target>1.7</maven.compiler.target>
+  </properties>
+
   <dependencies>
     <dependency>
       <groupId>org.jenkins-ci.plugins</groupId>
       <artifactId>cloudbees-folder</artifactId>
       <version>4.0</version>
       <optional>true</optional>
     </dependency>
+    <dependency>
+      <groupId>org.jenkins-ci</groupId>
+      <artifactId>test-annotations</artifactId>
+      <version>1.2</version>
+      <scope>test</scope>
+    </dependency>
+    <dependency>
+      <groupId>junit</groupId>
+      <artifactId>junit</artifactId>
+      <version>4.12</version>
+      <scope>test</scope>
+    </dependency>
+    <dependency>
+      <groupId>org.hamcrest</groupId>
+      <artifactId>hamcrest-core</artifactId>
+      <version>1.3</version>
+      <scope>test</scope>
+    </dependency>
     <dependency>
       <groupId>org.jenkins-ci.plugins</groupId>
       <artifactId>credentials</artifactId>
@@ -54,7 +76,15 @@
     <plugins>
       <plugin>
         <artifactId>maven-release-plugin</artifactId>
-        <version>2.4</version>
+        <version>2.5.1</version>
+      </plugin>
+      <plugin>
+        <groupId>org.jenkins-ci.tools</groupId>
+        <artifactId>maven-hpi-plugin</artifactId>
+        <extensions>true</extensions>
+        <configuration>
+          <compatibleSinceVersion>2.0</compatibleSinceVersion>
+        </configuration>
       </plugin>
     </plugins>
   </build>

src/main/java/org/jenkinsci/plugins/uniqueid/IdStore.java

@@ -1,16 +1,28 @@
 package org.jenkinsci.plugins.uniqueid;
 
 import hudson.ExtensionPoint;
+
 import jenkins.model.Jenkins;
 
+import java.io.UnsupportedEncodingException;
+import java.nio.charset.StandardCharsets;
+import java.util.UUID;
+
 import javax.annotation.Nullable;
 
+import org.apache.commons.codec.binary.Base64;
+
 /**
  * An abstraction to persistently store and retrieve unique id's
  * for various Jenkins model objects.
  *
  * These keys are guaranteed to be unique with a Jenkins
  * and immutable across the lifetime of the given object.
+ * 
+ * Implementations should not store the ID inside any specific item configuration as it is
+ * common for users top copy items either through the UI or manually and this will cause the
+ * IDs to become non-unique.
+ * 
  *
  * @param <T>
  */
@@ -26,14 +38,14 @@ public IdStore (Class<T> forType) {
      * Creates an unique id for the given object.
      * Subsequent calls are idempotent.
      *
-     * @param object
+     * @param object the object to make the id for.
      */
     public abstract void make(T object);
 
     /**
      * Get the id for this given object.
      * @param object
-     * @return the id or null if none assigned.
+     * @return the id or {@code null} if none assigned.
      */
     @Nullable
     public abstract String get(T object);
@@ -63,7 +75,7 @@ public static <C> IdStore<C> forClass(Class<C> clazz) {
      *
      * @throws java.lang.IllegalArgumentException if the type is not supported.
      */
-    public static void makeId(Object object) {
+    public static void makeId(Object object) throws IllegalArgumentException {
         IdStore store = forClass(object.getClass());
         if (store == null) {
             throw new IllegalArgumentException("Unsupported type: " + object.getClass().getName());
@@ -77,7 +89,7 @@ public static void makeId(Object object) {
      *
      * @throws java.lang.IllegalArgumentException if the type is not supported.
      */
-    public static String getId(Object object) {
+    public static String getId(Object object) throws IllegalArgumentException {
         IdStore store = forClass(object.getClass());
         if (store == null) {
             throw new IllegalArgumentException("Unsupported type: " + object.getClass().getName());
@@ -86,4 +98,13 @@ public static String getId(Object object) {
         }
     }
 
+    /**
+     * Generates a new unique ID.
+     * Subclasses do not need to use this to create unique IDs and are free to create IDs by other methods.
+     * @return a string that should be unique against all jenkins instances.
+     */
+    protected static String generateUniqueID() {
+        return Base64.encodeBase64String(UUID.randomUUID().toString().getBytes(StandardCharsets.UTF_8)).substring(0, 30);
+    }
+
 }

src/main/java/org/jenkinsci/plugins/uniqueid/impl/FolderIdStore.java

@@ -1,37 +1,47 @@
 package org.jenkinsci.plugins.uniqueid.impl;
 
-import com.cloudbees.hudson.plugins.folder.Folder;
-import com.cloudbees.hudson.plugins.folder.FolderProperty;
-import com.cloudbees.hudson.plugins.folder.FolderPropertyDescriptor;
 import hudson.Extension;
 import hudson.model.Action;
 import hudson.model.Actionable;
-import org.jenkinsci.plugins.uniqueid.IdStore;
+import hudson.util.DescribableList;
 
 import java.io.IOException;
 import java.util.Collection;
 import java.util.Collections;
-import java.util.logging.Level;
+import java.util.Iterator;
 import java.util.logging.Logger;
 
+import org.kohsuke.accmod.Restricted;
+import org.kohsuke.accmod.restrictions.NoExternalUse;
+
+import com.cloudbees.hudson.plugins.folder.FolderProperty;
+import com.cloudbees.hudson.plugins.folder.FolderPropertyDescriptor;
+import com.cloudbees.hudson.plugins.folder.Folder;
+
 /**
  * Stores ids for folders as a {@link FolderIdProperty}
+ * @deprecated {@see PersistenceRootIdStore}
  */
 @Extension(optional = true)
-public class FolderIdStore extends IdStore<Folder> {
+@Deprecated
+@Restricted(NoExternalUse.class)
+public class FolderIdStore extends LegacyIdStore<Folder> {
     public FolderIdStore() {
         super(Folder.class);
     }
 
     @Override
-    public void make(Folder folder) {
-        if (folder.getProperties().get(FolderIdProperty.class) == null) {
-            try {
-                folder.addProperty(new FolderIdProperty());
-            } catch (IOException e) {
-                LOGGER.log(Level.SEVERE, "Failed to add property",e);
+    public void remove(Folder folder) throws IOException {
+        DescribableList<FolderProperty<?>,FolderPropertyDescriptor> properties = folder.getProperties();
+
+        for (Iterator<FolderProperty<?>> itr = properties.iterator(); itr.hasNext(); ) {
+            FolderProperty<?> prop = itr.next();
+
+            if (prop instanceof FolderIdProperty) {
+                itr.remove();
             }
         }
+        folder.save();
     }
 
     @Override

src/main/java/org/jenkinsci/plugins/uniqueid/impl/Id.java

@@ -1,18 +1,29 @@
 package org.jenkinsci.plugins.uniqueid.impl;
 
+import jenkins.model.RunAction2;
+
 import hudson.model.Action;
 import hudson.model.Actionable;
+import hudson.model.Run;
+
 import org.apache.commons.codec.binary.Base64;
+import org.kohsuke.accmod.Restricted;
+import org.kohsuke.accmod.restrictions.NoExternalUse;
 
 import javax.annotation.Nullable;
+
 import java.util.UUID;
 import java.util.logging.Logger;
 
 /**
- * An action which stores an id.
+ * DO NOT USE
+ * @deprecated users should not use this as it ties the ID explicitly to the item and as will not work when copying items (for example). 
  */
-class Id implements Action {
+@Deprecated
+@Restricted(NoExternalUse.class)
+class Id implements Action , RunAction2 {
 
+    private final static Logger LOGGER = Logger.getLogger(Id.class.getName());
 
     private final String id;
 
@@ -36,7 +47,13 @@ public String getId() {
         return id;
     }
 
+
+    /**
+     * @deprecated Sub classes should not use this as it stores the ID in the actionable item.
+     * @return
+     */
     @Nullable
+    @Deprecated
     protected static String getId(Actionable actionable) {
         Id id = actionable.getAction(Id.class);
         if (id != null) {
@@ -47,5 +64,15 @@ protected static String getId(Actionable actionable) {
     }
 
 
-    private final static Logger LOGGER = Logger.getLogger(Id.class.getName());
+
+    public void onAttached(Run<?, ?> r) {
+        // NO-OP
+    }
+
+    /** 
+     * Migrates the run away from using this Action.
+     */
+    public void onLoad(Run<?, ?> r) {
+        IdStoreMigratorV1ToV2.migrate(r);
+    }
 }