[JENKINS-28881] - Fix several issues with explicit JobOwnerProperty u…

…sage impacting the inheritance
jenkinsci · Jan 4, 2016 · 74b81be · 74b81be
1 parent 8207da5
commit 74b81be
Show file tree

Hide file tree

Showing 2 changed files with 14 additions and 12 deletions.
diff --git a/...opsys/arc/jenkins/plugins/ownership/security/rolestrategy/AbstractOwnershipRoleMacro.java b/...opsys/arc/jenkins/plugins/ownership/security/rolestrategy/AbstractOwnershipRoleMacro.java
@@ -23,21 +23,25 @@
  */
 package com.synopsys.arc.jenkins.plugins.ownership.security.rolestrategy;
 
+import com.synopsys.arc.jenkins.plugins.ownership.IOwnershipHelper;
 import com.synopsys.arc.jenkins.plugins.ownership.IOwnershipItem;
 import com.synopsys.arc.jenkins.plugins.ownership.OwnershipDescription;
 import com.synopsys.arc.jenkins.plugins.ownership.jobs.JobOwnerHelper;
 import com.synopsys.arc.jenkins.plugins.ownership.jobs.JobOwnerJobProperty;
+import com.synopsys.arc.jenkins.plugins.ownership.nodes.NodeOwnerHelper;
 import com.synopsys.arc.jenkins.plugins.ownership.nodes.OwnerNodeProperty;
 import com.synopsys.arc.jenkins.plugins.rolestrategy.Macro;
 import com.synopsys.arc.jenkins.plugins.rolestrategy.RoleMacroExtension;
 import com.synopsys.arc.jenkins.plugins.rolestrategy.RoleType;
 import static com.synopsys.arc.jenkins.plugins.rolestrategy.RoleType.Project;
 import static com.synopsys.arc.jenkins.plugins.rolestrategy.RoleType.Slave;
 import hudson.model.Computer;
+import hudson.model.Item;
 import hudson.model.Job;
 import hudson.model.Node;
 import hudson.model.User;
 import hudson.security.AccessControlled;
+import org.jenkinsci.plugins.ownership.model.OwnershipHelperLocator;
 
 /**
  * An abstract class for {@link RoleMacroExtension}s provided by the ownership plugin.
@@ -62,30 +66,29 @@ public boolean IsApplicable(RoleType roleType) {
     }
 
     public static OwnershipDescription getOwnership(RoleType type, AccessControlled item) {
-        IOwnershipItem ownership = null;
+        //TODO refactor the code to use OwnershipHelperLocator
         switch(type) {
             case Project:
-                if (item instanceof Job) { 
-                    Job prj = (Job)item;
-                    JobOwnerJobProperty prop = JobOwnerHelper.getOwnerProperty(prj);
-                    if (prop != null) {
-                        ownership = prop;
-                    }                 
+                if (item instanceof Item) { 
+                    IOwnershipHelper<AccessControlled> helper = OwnershipHelperLocator.locate(item);
+                    return helper != null ? helper.getOwnershipDescription(item) : OwnershipDescription.DISABLED_DESCR;
                 }
                 break;
             case Slave:
                 if (item instanceof Computer) {
                     Computer comp = (Computer)item;
                     Node node = comp.getNode();
                     if (node != null) {
-                        ownership = node.getNodeProperties().get(OwnerNodeProperty.class);
+                        return NodeOwnerHelper.Instance.getOwnershipDescription(node);
                     }
                 }
                 break;
             default:
                 //do nothing => Ownership is disabled
         }
-        return ownership != null ? ownership.getOwnership() : OwnershipDescription.DISABLED_DESCR;
+
+        // Fallback to the disabled Ownership description
+        return OwnershipDescription.DISABLED_DESCR;
     }
 
     public static boolean hasPermission(User user, RoleType type, AccessControlled item, Macro macro, boolean acceptCoowners) {

diff --git a/src/main/java/org/jenkinsci/plugins/ownership/model/runs/RunOwnershipHelper.java b/src/main/java/org/jenkinsci/plugins/ownership/model/runs/RunOwnershipHelper.java
@@ -85,9 +85,8 @@ public OwnershipDescription getOwnershipDescription(Run item) {
     public static void setUp (@Nonnull AbstractBuild build, @Nonnull Map<String, String> target, 
             @CheckForNull BuildListener listener, 
             boolean injectJobOwnership, boolean injectNodeOwnership) {
-        if (injectJobOwnership) {
-            JobOwnerJobProperty prop = JobOwnerHelper.getOwnerProperty(build.getParent());  
-            OwnershipDescription descr = prop != null ? prop.getOwnership() : OwnershipDescription.DISABLED_DESCR;
+        if (injectJobOwnership) { 
+            OwnershipDescription descr = JobOwnerHelper.Instance.getOwnershipDescription(build.getParent());
             getVariables(descr, target, "JOB");
         }