Skip to content
Permalink
Browse files

Fixed issue with deriving parameters

This mainly involed reducing the complexity of deriving parameters,
by simplifying how the scope is generated. Additionally, ParameterDefinition
instances are now copied by the ISPD before the rootProperty is registered.

This should avoid cross-contamination and potential race-conditions.

Added extended logging to isBuildable() check.
  • Loading branch information...
HedAurabesh committed Jul 29, 2014
1 parent 0726da9 commit 7afc16641cc452e4d5294c72070615cd030d20d9
@@ -60,6 +60,7 @@
import hudson.plugins.project_inheritance.projects.creation.ProjectCreationEngine.CreationClass;
import hudson.plugins.project_inheritance.projects.inheritance.InheritanceGovernor;
import hudson.plugins.project_inheritance.projects.parameters.InheritableStringParameterDefinition;
import hudson.plugins.project_inheritance.projects.parameters.InheritableStringParameterReferenceDefinition;
import hudson.plugins.project_inheritance.projects.parameters.InheritableStringParameterDefinition.IModes;
import hudson.plugins.project_inheritance.projects.parameters.InheritanceParametersDefinitionProperty;
import hudson.plugins.project_inheritance.projects.parameters.InheritanceParametersDefinitionProperty.ScopeEntry;
@@ -68,6 +69,7 @@
import hudson.plugins.project_inheritance.projects.references.AbstractProjectReference;
import hudson.plugins.project_inheritance.projects.references.ParameterizedProjectReference;
import hudson.plugins.project_inheritance.projects.references.ProjectReference;
import hudson.plugins.project_inheritance.projects.references.ProjectReference.PrioComparator;
import hudson.plugins.project_inheritance.projects.references.ProjectReference.PrioComparator.SELECTOR;
import hudson.plugins.project_inheritance.projects.view.InheritanceViewAction;
import hudson.plugins.project_inheritance.util.Helpers;
@@ -116,6 +118,7 @@
import java.util.LinkedHashMap;
import java.util.LinkedList;
import java.util.List;
import java.util.ListIterator;
import java.util.Map;
import java.util.Set;
import java.util.TreeSet;
@@ -2610,15 +2613,53 @@ public void setScm(SCM scm) throws IOException {
* will almost always lead to an infinite recursion.
*
* @param sortKey the key specifying the order in which projects are returned.
* @return
* @return a list of all parent references
*/
private List<AbstractProjectReference> getAllParentReferences(
public List<AbstractProjectReference> getAllParentReferences(
ProjectReference.PrioComparator.SELECTOR sortKey) {
InheritanceGovernor<List<AbstractProjectReference>> gov =
this.getParentReferencesGovernor(sortKey);
return gov.retrieveFullyDerivedField(this, IMode.INHERIT_FORCED);
}

/**
* Wrapper for {@link #getAllParentReferences(SELECTOR)}, but will add
* a reference to this project too, if needed.
*
* @param sortKey the key specifying the order in which projects are returned.
* @param addSelf if true, add a self-reference in the correct spot
* @return a list of all parent references, including a self-reference if
* addSelf is true.
*/
public List<AbstractProjectReference> getAllParentReferences(
ProjectReference.PrioComparator.SELECTOR sortKey, boolean addSelf) {
List<AbstractProjectReference> lst = this.getAllParentReferences(sortKey);

if (addSelf) {
boolean hasAddedSelf = false;
ListIterator<AbstractProjectReference> iter = lst.listIterator();
while (iter.hasNext()) {
AbstractProjectReference ref = iter.next();
int prio;
if (ref instanceof ProjectReference) {
prio = PrioComparator.getPriorityFor(ref, sortKey);
} else {
//An anonymous ref is always at priority 0
prio = 0;
}
if (!hasAddedSelf && prio > 0) {
hasAddedSelf = true;
iter.add(new SimpleProjectReference(this.getFullName()));
}
}
//Check if we were able to add a self-reference at all
if (!hasAddedSelf) {
lst.add(new SimpleProjectReference(this.getFullName()));
}
}

return lst;
}

public List<AbstractProjectReference> getCompatibleProjects() {
return this.getCompatibleProjects(SELECTOR.MISC);
@@ -3069,7 +3110,7 @@ protected T reduceFromFullInheritance(Deque<T> list) {
Deque<List<JobProperty<? super InheritanceProject>>> list) {
//First, we add the variances for the root project
InheritanceParametersDefinitionProperty variance =
rootProject.getVarianceProperties();
rootProject.getVarianceParameters();
if (variance != null) {
List<JobProperty<? super InheritanceProject>> varLst =
new LinkedList<JobProperty<? super InheritanceProject>>();
@@ -3137,7 +3178,7 @@ protected T reduceFromFullInheritance(Deque<T> list) {
return out;
}

private InheritanceParametersDefinitionProperty getVarianceProperties() {
public InheritanceParametersDefinitionProperty getVarianceParameters() {
if (this.isTransient == false) {
//No variance is or can possibly be defined
return null;
@@ -3195,7 +3236,6 @@ private InheritanceParametersDefinitionProperty getVarianceProperties() {
new InheritanceParametersDefinitionProperty(
this, ppr.getParameters()
);
ipdp.addScopedParameterDefinitions(ipdp);
return ipdp;
}
}
@@ -3825,10 +3865,12 @@ public boolean isRawParameterized() {
@Override
public boolean isBuildable() {
if (!super.isBuildable()) {
log.warning(String.format("%s not buildable; super.isBuildable() is false", this.getFullName()));
return false;
}
//Then, we check if it's an abstract job
if (this.isAbstract) {
log.warning(String.format("%s not buildable; project is abstract", this.getFullName()));
return false;
}

@@ -3837,6 +3879,7 @@ public boolean isBuildable() {
AbstractMap.SimpleEntry<Boolean, String> paramCheck =
this.getParameterSanity();
if (paramCheck.getKey() == false) {
log.warning(String.format("%s not buildable; Parameter inconsistency: %s", this.getFullName(), paramCheck.getValue()));
return false;
}

@@ -4504,34 +4547,35 @@ public final boolean hasCyclicDependency(String... whenTheseProjectsAdded) {
if (pd instanceof InheritableStringParameterDefinition) {
InheritableStringParameterDefinition ispd =
(InheritableStringParameterDefinition) pd;

//Checking if the "force-default-value" flag is set by the new one
if (ispd.getMustHaveDefaultValue()) {
s.hasToHaveDefaultSet = true;
}
//Checking if the assignment flag was set by the new one
if (ispd.getMustBeAssigned()) {
s.hasToBeAssigned = true;
}

//Then, checking if the "force-default-value" flag was unset
if (s.hasToHaveDefaultSet && !ispd.getMustHaveDefaultValue()) {
return new AbstractMap.SimpleEntry<Boolean, String>(
false, "Parameter '" + pd.getName() +
"' may not unset the flag that ensures that a" +
" default value is set."
);
}
//Checking if the "must-be-assigned" flag was unset
if (s.hasToBeAssigned && !ispd.getMustBeAssigned()) {
return new AbstractMap.SimpleEntry<Boolean, String>(
false, "Parameter '" + pd.getName() +
"' may not unset the flag that ensures that a" +
" final value is set before execution."
);
//We ignore references, as they can never invalidate flags
//Otherwise, we check whether they unset values
if (!(pd instanceof InheritableStringParameterReferenceDefinition)) {
//Checking if the "force-default-value" flag is set by the new one
if (ispd.getMustHaveDefaultValue()) {
s.hasToHaveDefaultSet = true;
}
//Checking if the assignment flag was set by the new one
if (ispd.getMustBeAssigned()) {
s.hasToBeAssigned = true;
}
//Then, checking if the "force-default-value" flag was unset
if (s.hasToHaveDefaultSet && !ispd.getMustHaveDefaultValue()) {
return new AbstractMap.SimpleEntry<Boolean, String>(
false, "Parameter '" + pd.getName() +
"' may not unset the flag that ensures that a" +
" default value is set."
);
}
//Checking if the "must-be-assigned" flag was unset
if (s.hasToBeAssigned && !ispd.getMustBeAssigned()) {
return new AbstractMap.SimpleEntry<Boolean, String>(
false, "Parameter '" + pd.getName() +
"' may not unset the flag that ensures that a" +
" final value is set before execution."
);
}
}


//Checking if overwriting causes a previous default to be lost
String defVal = ispd.getDefaultValue();
boolean defValNewlySet = !(defVal == null || defVal.isEmpty());
@@ -413,13 +413,17 @@ private StringParameterValue produceDerivedValue(String userEnteredValue)
IModes currMode = IModes.OVERWRITABLE;
boolean mustHaveValueSet = false;

for (ScopeEntry scope : fullScope) {
Iterator<ScopeEntry> iter = fullScope.iterator();
while(iter.hasNext()) {
ScopeEntry scope = iter.next();
if (!(scope.param instanceof InheritableStringParameterDefinition)) {
continue;
}
InheritableStringParameterDefinition ispd =
(InheritableStringParameterDefinition) scope.param;
String ispdVal = (ispd == this)

//The last value from the scope gets overwritten by the user-entered value
String ispdVal = (!iter.hasNext())
? userEnteredValue
: ispd.getDefaultValue();

@@ -22,6 +22,8 @@

import hudson.Extension;
import hudson.model.ParameterDefinition;
import hudson.model.ParameterValue;
import hudson.model.StringParameterValue;
import hudson.plugins.project_inheritance.projects.InheritanceProject;
import hudson.plugins.project_inheritance.projects.InheritanceProject.IMode;
import hudson.plugins.project_inheritance.projects.parameters.InheritanceParametersDefinitionProperty.ScopeEntry;
@@ -61,7 +63,7 @@ public InheritableStringParameterDefinition getParent() {
}

//Fetch the owner of this reference
String selfOwner = ipdp.getOwner().getDisplayName();
String selfOwner = ipdp.getOwner().getFullName();

List<ScopeEntry> scope = ipdp.getAllScopedParameterDefinitions();
ListIterator<ScopeEntry> iter = scope.listIterator(scope.size());
@@ -81,6 +83,20 @@ public InheritableStringParameterDefinition getParent() {
return null;
}

@Override
public ParameterDefinition copyWithDefaultValue(ParameterValue defaultValue) {
if (!(defaultValue instanceof StringParameterValue)) {
//This should never happen
return super.copyWithDefaultValue(defaultValue);
}

StringParameterValue spv = ((StringParameterValue) defaultValue);
String value = spv.value;
return new InheritableStringParameterReferenceDefinition(
this.getName(), value
);
}


/**
* Will fetch that field from its nearest parent; using the rootProperty

0 comments on commit 7afc166

Please sign in to comment.
You can’t perform that action at this time.