Implement LogRotatorProperty for logRotator logic and unit test for property conversion

Signed-off-by: Winston Prakash <winston.prakash@gmail.com>

10 files changed, 103 insertions, 42 deletions

diff --git a/hudson-core/src/main/java/hudson/model/AbstractProject.java b/hudson-core/src/main/java/hudson/model/AbstractProject.java
index 35d1f6d..38135a4 100644
--- a/hudson-core/src/main/java/hudson/model/AbstractProject.java
+++ b/hudson-core/src/main/java/hudson/model/AbstractProject.java
@@ -38,7 +38,7 @@ import hudson.model.Queue.WaitingItem;
 import hudson.model.RunMap.Constructor;
 import hudson.model.labels.LabelAtom;
 import hudson.model.labels.LabelExpression;
-import hudson.model.project.property.IntegerProjectProperty;
+import org.eclipse.hudson.api.model.project.property.IntegerProjectProperty;
 import hudson.model.queue.CauseOfBlockage;
 import hudson.model.queue.SubTask;
 import hudson.model.queue.SubTaskContributor;
diff --git a/hudson-core/src/main/java/hudson/model/Items.java b/hudson-core/src/main/java/hudson/model/Items.java
index 28e5b81..130e35b 100644
--- a/hudson-core/src/main/java/hudson/model/Items.java
+++ b/hudson-core/src/main/java/hudson/model/Items.java
@@ -19,9 +19,9 @@ package hudson.model;
 import com.thoughtworks.xstream.XStream;
 import hudson.DescriptorExtensionList;
 import hudson.Extension;
-import hudson.model.project.property.BooleanProjectProperty;
-import hudson.model.project.property.IntegerProjectProperty;
-import hudson.model.project.property.StringProjectProperty;
+import org.eclipse.hudson.api.model.project.property.BooleanProjectProperty;
+import org.eclipse.hudson.api.model.project.property.IntegerProjectProperty;
+import org.eclipse.hudson.api.model.project.property.StringProjectProperty;
 import hudson.matrix.MatrixProject;
 import hudson.matrix.MatrixConfiguration;
 import hudson.XmlFile;
diff --git a/hudson-core/src/main/java/hudson/model/Job.java b/hudson-core/src/main/java/hudson/model/Job.java
index 1fca2d4..7cb5ef3 100644
--- a/hudson-core/src/main/java/hudson/model/Job.java
+++ b/hudson-core/src/main/java/hudson/model/Job.java
@@ -18,15 +18,15 @@
 package hudson.model;
 
 import hudson.Functions;
-import hudson.model.project.property.BooleanProjectProperty;
-import hudson.model.project.property.IntegerProjectProperty;
-import hudson.model.project.property.StringProjectProperty;
+import org.eclipse.hudson.api.model.project.property.BooleanProjectProperty;
+import org.eclipse.hudson.api.model.project.property.IntegerProjectProperty;
+import org.eclipse.hudson.api.model.project.property.LogRotatorProjectProperty;
+import org.eclipse.hudson.api.model.project.property.StringProjectProperty;
 import hudson.util.graph.GraphSeries;
 import hudson.widgets.Widget;
 import java.lang.reflect.InvocationTargetException;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentMap;
-import org.apache.commons.lang3.ObjectUtils;
 import org.apache.commons.lang3.StringUtils;
 import com.google.common.collect.Sets;
 import com.infradna.tool.bridge_method_injector.WithBridgeMethods;
@@ -116,6 +116,8 @@ public abstract class Job<JobT extends Job<JobT, RunT>, RunT extends Run<JobT, R
         extends AbstractItem implements ExtensionPoint, StaplerOverridable, IJob {
     private static transient final String HUDSON_BUILDS_PROPERTY_KEY = "HUDSON_BUILDS";
 
+    public static final String LOG_ROTATOR_PROPERTY_NAME = "logRotator";
+
     /**
      * Next build number. Kept in a separate file because this is the only
      * information that gets updated often. This allows the rest of the
@@ -130,6 +132,12 @@ public abstract class Job<JobT extends Job<JobT, RunT>, RunT extends Run<JobT, R
      * is saved once.
      */
     private transient volatile boolean holdOffBuildUntilSave;
+
+    /**
+     * @deprecated as of 2.2.0
+     *             don't use this field directly, logic was moved to {@link org.eclipse.hudson.api.model.IProjectProperty}.
+     *             Use getter/setter for accessing to this field.
+     */
     private volatile LogRotator logRotator;
 
     private ConcurrentMap<String, IProjectProperty> jobProperties = new ConcurrentHashMap<String, IProjectProperty>();
@@ -314,6 +322,10 @@ public abstract class Job<JobT extends Job<JobT, RunT>, RunT extends Run<JobT, R
             property.setKey(entry.getKey());
             property.setJob(this);
         }
+        if (null == getProperty(LOG_ROTATOR_PROPERTY_NAME)) {
+            setLogRotator(logRotator);
+            logRotator = null;
+        }
     }
 
     /**
@@ -471,7 +483,7 @@ public abstract class Job<JobT extends Job<JobT, RunT>, RunT extends Run<JobT, R
      * Returns the log rotator for this job, or null if none.
      */
     public LogRotator getLogRotator() {
-        return logRotator!= null ? logRotator : (hasCascadingProject()? getCascadingProject().getLogRotator() : null);
+        return (LogRotator) getProperty(LOG_ROTATOR_PROPERTY_NAME, LogRotatorProjectProperty.class).getValue();
     }
 
     /**
@@ -479,12 +491,9 @@ public abstract class Job<JobT extends Job<JobT, RunT>, RunT extends Run<JobT, R
      *
      * @param logRotator log rotator.
      */
+    @SuppressWarnings("unchecked")
     public void setLogRotator(LogRotator logRotator) {
-        if (!(hasCascadingProject() && ObjectUtils.equals(getCascadingProject().getLogRotator(), logRotator))) {
-            this.logRotator = logRotator;
-        } else {
-            this.logRotator = null;
-        }
+        getProperty(LOG_ROTATOR_PROPERTY_NAME, LogRotatorProjectProperty.class).setValue(logRotator);
     }
 
     /**
@@ -1350,10 +1359,14 @@ public abstract class Job<JobT extends Job<JobT, RunT>, RunT extends Run<JobT, R
      * @param cascadingProjectName cascadingProject name.
      */
     @SuppressWarnings("unchecked")
-    public void setCascadingProjectName(String cascadingProjectName) {
+    public synchronized void setCascadingProjectName(String cascadingProjectName) {
         this.cascadingProjectName = cascadingProjectName;
-        this.cascadingProject = (JobT)Functions.getItemByName(Hudson.getInstance().getAllItems(this.getClass()),
-            cascadingProjectName);
+        if (StringUtils.isBlank(cascadingProjectName)) {
+            this.cascadingProject = null;
+        } else {
+            this.cascadingProject = (JobT) Functions.getItemByName(Hudson.getInstance().getAllItems(this.getClass()),
+                cascadingProjectName);
+        }
     }
 
     /**
diff --git a/hudson-core/src/main/java/hudson/model/project/property/BaseProjectProperty.java b/hudson-core/src/main/java/org/eclipse/hudson/api/model/project/property/BaseProjectProperty.java
index af6e0ea..4cd693c 100644
--- a/hudson-core/src/main/java/hudson/model/project/property/BaseProjectProperty.java
+++ b/hudson-core/src/main/java/org/eclipse/hudson/api/model/project/property/BaseProjectProperty.java
@@ -13,7 +13,7 @@
  *
  *******************************************************************************/
 
-package hudson.model.project.property;
+package org.eclipse.hudson.api.model.project.property;
 
 import org.apache.commons.lang3.ObjectUtils;
 import org.eclipse.hudson.api.model.IJob;
diff --git a/hudson-core/src/main/java/hudson/model/project/property/BooleanProjectProperty.java b/hudson-core/src/main/java/org/eclipse/hudson/api/model/project/property/BooleanProjectProperty.java
index fba0c33..73a2750 100644
--- a/hudson-core/src/main/java/hudson/model/project/property/BooleanProjectProperty.java
+++ b/hudson-core/src/main/java/org/eclipse/hudson/api/model/project/property/BooleanProjectProperty.java
@@ -12,9 +12,10 @@
  *    Nikita Levyankov
  *
  *******************************************************************************/
-package hudson.model.project.property;
+package org.eclipse.hudson.api.model.project.property;
 
 import org.eclipse.hudson.api.model.IJob;
+import org.eclipse.hudson.api.model.project.property.BaseProjectProperty;
 
 /**
  * Represents boolean property.
diff --git a/hudson-core/src/main/java/hudson/model/project/property/IntegerProjectProperty.java b/hudson-core/src/main/java/org/eclipse/hudson/api/model/project/property/IntegerProjectProperty.java
index 1e8e18e..482f231 100644
--- a/hudson-core/src/main/java/hudson/model/project/property/IntegerProjectProperty.java
+++ b/hudson-core/src/main/java/org/eclipse/hudson/api/model/project/property/IntegerProjectProperty.java
@@ -12,9 +12,10 @@
  *    Nikita Levyankov
  *
  *******************************************************************************/
-package hudson.model.project.property;
+package org.eclipse.hudson.api.model.project.property;
 
 import org.eclipse.hudson.api.model.IJob;
+import org.eclipse.hudson.api.model.project.property.BaseProjectProperty;
 
 /**
  * Represents integer property.
diff --git a/hudson-core/src/main/java/hudson/model/project/property/IntegerProjectProperty.java b/hudson-core/src/main/java/org/eclipse/hudson/api/model/project/property/LogRotatorProjectProperty.java
index 1e8e18e..3d3262d 100644
--- a/hudson-core/src/main/java/hudson/model/project/property/IntegerProjectProperty.java
+++ b/hudson-core/src/main/java/org/eclipse/hudson/api/model/project/property/LogRotatorProjectProperty.java
@@ -12,28 +12,20 @@
  *    Nikita Levyankov
  *
  *******************************************************************************/
-package hudson.model.project.property;
+package org.eclipse.hudson.api.model.project.property;
 
+import hudson.tasks.LogRotator;
 import org.eclipse.hudson.api.model.IJob;
 
 /**
- * Represents integer property.
+ * Represents LogRotator property.
  * <p/>
- * Date: 9/22/11
+ * Date: 9/27/11
  *
  * @author Nikita Levyankov
  */
-public class IntegerProjectProperty extends BaseProjectProperty<Integer> {
-
-    public IntegerProjectProperty(IJob job) {
+public class LogRotatorProjectProperty extends BaseProjectProperty<LogRotator> {
+    public LogRotatorProjectProperty(IJob job) {
         super(job);
     }
-
-    /**
-     * {@inheritDoc}
-     */
-    @Override
-    public Integer getDefaultValue() {
-        return 0;
-    }
-}
+}
\ No newline at end of file
diff --git a/hudson-core/src/main/java/hudson/model/project/property/StringProjectProperty.java b/hudson-core/src/main/java/org/eclipse/hudson/api/model/project/property/StringProjectProperty.java
index dffae46..299d3f8 100644
--- a/hudson-core/src/main/java/hudson/model/project/property/StringProjectProperty.java
+++ b/hudson-core/src/main/java/org/eclipse/hudson/api/model/project/property/StringProjectProperty.java
@@ -13,10 +13,11 @@
  *
  *******************************************************************************/
 
-package hudson.model.project.property;
+package org.eclipse.hudson.api.model.project.property;
 
 import org.apache.commons.lang3.StringUtils;
 import org.eclipse.hudson.api.model.IJob;
+import org.eclipse.hudson.api.model.project.property.BaseProjectProperty;
 
 /**
  * String property for project.
diff --git a/hudson-core/src/test/java/hudson/model/LegacyProjectTest.java b/hudson-core/src/test/java/hudson/model/LegacyProjectTest.java
index 3e7790a..2a51588 100644
--- a/hudson-core/src/test/java/hudson/model/LegacyProjectTest.java
+++ b/hudson-core/src/test/java/hudson/model/LegacyProjectTest.java
@@ -15,6 +15,8 @@
 package hudson.model;
 
 import java.io.File;
+import java.net.URISyntaxException;
+import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.powermock.core.classloader.annotations.PrepareForTest;
@@ -34,6 +36,12 @@ import static junit.framework.Assert.assertNull;
 @PrepareForTest({Hudson.class})
 public class LegacyProjectTest {
 
+    private File config;
+    @Before
+    public void setUp() throws URISyntaxException {
+        config = new File(FreeStyleProject.class.getResource("/hudson/model/freestyle").toURI());
+    }
+
     /**
      * Tests unmarshalls FreeStyleProject configuration and checks whether properties are configured based
      * on legacy values,
@@ -42,8 +50,7 @@ public class LegacyProjectTest {
      */
     @Test
     public void testLoadLegacyFreeStyleProject() throws Exception {
-        File freeStyleProjectConfig = new File(FreeStyleProject.class.getResource("/hudson/model/freestyle").toURI());
-        FreeStyleProject project = (FreeStyleProject) Items.getConfigFile(freeStyleProjectConfig).read();
+        FreeStyleProject project = (FreeStyleProject) Items.getConfigFile(config).read();
         project.setAllowSave(false);
         project.initProjectProperties();
         //Checks customWorkspace value
@@ -60,8 +67,7 @@ public class LegacyProjectTest {
      */
     @Test
     public void testLoadLegacyAbstractProject() throws Exception {
-        File freeStyleProjectConfig = new File(FreeStyleProject.class.getResource("/hudson/model/freestyle").toURI());
-        AbstractProject project = (AbstractProject) Items.getConfigFile(freeStyleProjectConfig).read();
+        AbstractProject project = (AbstractProject) Items.getConfigFile(config).read();
         project.setAllowSave(false);
         project.initProjectProperties();
         assertNull(project.getProperty(AbstractProject.BLOCK_BUILD_WHEN_UPSTREAM_BUILDING_PROPERTY_NAME));
@@ -80,4 +86,20 @@ public class LegacyProjectTest {
         assertNotNull(project.getProperty(AbstractProject.SCM_CHECKOUT_RETRY_COUNT_PROPERTY_NAME));
         assertNotNull(project.getProperty(AbstractProject.JDK_PROPERTY_NAME));
     }
+
+    /**
+     * Tests unmarshalls FreeStyleProject configuration and checks whether properties
+     * from Job are configured
+     *
+     * @throws Exception if any.
+     */
+    @Test
+    public void testLoadLegacyJob() throws Exception {
+        Job project = (Job) Items.getConfigFile(config).read();
+        project.setAllowSave(false);
+        project.initProjectProperties();
+        assertNull(project.getProperty(Job.LOG_ROTATOR_PROPERTY_NAME));
+        project.buildProjectProperties();
+        assertNotNull(project.getProperty(Job.LOG_ROTATOR_PROPERTY_NAME));
+    }
 }
diff --git a/hudson-core/src/test/java/hudson/model/project/property/ProjectPropertyTest.java b/hudson-core/src/test/java/org/eclipse/hudson/api/model/project/property/ProjectPropertyTest.java
index 775b1cf..a1ffecc 100644
--- a/hudson-core/src/test/java/hudson/model/project/property/ProjectPropertyTest.java
+++ b/hudson-core/src/test/java/org/eclipse/hudson/api/model/project/property/ProjectPropertyTest.java
@@ -13,9 +13,15 @@
  *
  *******************************************************************************/
 
-package hudson.model.project.property;
+package org.eclipse.hudson.api.model.project.property;
 
 import hudson.model.FreeStyleProjectMock;
+import hudson.tasks.LogRotator;
+import org.eclipse.hudson.api.model.project.property.BaseProjectProperty;
+import org.eclipse.hudson.api.model.project.property.BooleanProjectProperty;
+import org.eclipse.hudson.api.model.project.property.IntegerProjectProperty;
+import org.eclipse.hudson.api.model.project.property.LogRotatorProjectProperty;
+import org.eclipse.hudson.api.model.project.property.StringProjectProperty;
 import org.junit.Before;
 import org.junit.Test;
 
@@ -74,6 +80,12 @@ public class ProjectPropertyTest {
         } catch (Exception e) {
             assertEquals(BaseProjectProperty.INVALID_JOB_EXCEPTION, e.getMessage());
         }
+        try {
+            new LogRotatorProjectProperty(null);
+            fail("Null should be handled by ProjectProperty constructor.");
+        } catch (Exception e) {
+            assertEquals(BaseProjectProperty.INVALID_JOB_EXCEPTION, e.getMessage());
+        }
         BaseProjectProperty property = new BaseProjectProperty(project);
         assertNotNull(property.getJob());
         assertEquals(project, property.getJob());
@@ -96,6 +108,12 @@ public class ProjectPropertyTest {
         assertFalse((Boolean) property.prepareValue(false));
         assertTrue((Boolean) property.prepareValue(true));
 
+        //Boolean property acts as BaseProperty
+        property = new LogRotatorProjectProperty(project);
+        assertNull(property.prepareValue(null));
+        value = new LogRotator(1, 1, 1, 1);
+        assertEquals(value, property.prepareValue(value));
+
         //Integer property acts as BaseProperty
         property = new IntegerProjectProperty(project);
         assertNull(property.prepareValue(null));
@@ -124,7 +142,8 @@ public class ProjectPropertyTest {
         assertEquals(0, property.getDefaultValue());
         property = new BooleanProjectProperty(project);
         assertFalse((Boolean) property.getDefaultValue());
-
+        property = new LogRotatorProjectProperty(project);
+        assertNull(property.getDefaultValue());
     }
 
     /**
@@ -162,6 +181,12 @@ public class ProjectPropertyTest {
         assertTrue(property.allowOverrideValue(null, "abc"));
         assertTrue(property.allowOverrideValue("abc", null));
         assertTrue(property.allowOverrideValue("abc", "abcd"));
+
+        property = new LogRotatorProjectProperty(project);
+        assertFalse(property.allowOverrideValue(null, null));
+        assertTrue(property.allowOverrideValue(new LogRotator(1, 1, 1, 1), null));
+        assertTrue(property.allowOverrideValue(null, new LogRotator(1, 1, 1, 1)));
+        assertTrue(property.allowOverrideValue(new LogRotator(1, 1, 1, 2), new LogRotator(1, 1, 1, 1)));
     }
 
     /**
@@ -228,6 +253,12 @@ public class ProjectPropertyTest {
         assertEquals(value, property.getOriginalValue());
         property.setValue(null);
         assertFalse((Boolean) property.getOriginalValue());
+
+        value = new LogRotator(1, 1, 1, 1);
+        property = new LogRotatorProjectProperty(project);
+        property.setKey(propertyKey);
+        property.setValue(value);
+        assertEquals(value, property.getOriginalValue());
     }
 
     /**