diff --git a/proctor-common/src/main/java/com/indeed/proctor/common/Proctor.java b/proctor-common/src/main/java/com/indeed/proctor/common/Proctor.java index c19008d2..a8bfd94c 100644 --- a/proctor-common/src/main/java/com/indeed/proctor/common/Proctor.java +++ b/proctor-common/src/main/java/com/indeed/proctor/common/Proctor.java @@ -43,7 +43,6 @@ import java.util.stream.IntStream; import static com.indeed.proctor.common.ProctorUtils.UNITLESS_ALLOCATION_IDENTIFIER; -import static com.indeed.proctor.common.model.PayloadExperimentConfig.isHigherPriority; /** * The sole entry point for client applications determining the test buckets for a particular @@ -562,7 +561,8 @@ private void attemptStoringProperty( testChoosers.get(testName).getTestDefinition().getPayloadExperimentConfig(); // store property if it has higher priority than the currently stored property try { - if (isHigherPriority(currPayloadConfig, newPayloadConfig)) { + if (currPayloadConfig == null + || currPayloadConfig.isHigherPriorityThan(newPayloadConfig)) { testProperties.put( field.getKey(), PayloadProperty.builder() diff --git a/proctor-common/src/main/java/com/indeed/proctor/common/model/PayloadExperimentConfig.java b/proctor-common/src/main/java/com/indeed/proctor/common/model/PayloadExperimentConfig.java index 1e35b918..83e54019 100644 --- a/proctor-common/src/main/java/com/indeed/proctor/common/model/PayloadExperimentConfig.java +++ b/proctor-common/src/main/java/com/indeed/proctor/common/model/PayloadExperimentConfig.java @@ -31,14 +31,13 @@ public String toString() { + '}'; } - public static boolean isHigherPriority( - final PayloadExperimentConfig payloadConfig, - final PayloadExperimentConfig otherPayloadConfig) { - return payloadConfig != null - && payloadConfig.getPriority() != null - && otherPayloadConfig != null - && otherPayloadConfig.getPriority() != null - && Long.parseLong(payloadConfig.getPriority()) - < Long.parseLong(otherPayloadConfig.getPriority()); + public boolean isHigherPriorityThan(final PayloadExperimentConfig otherPayloadConfig) { + final long currPriority = + this.getPriority() == null ? Long.MIN_VALUE : Long.parseLong(this.getPriority()); + final long otherPriority = + otherPayloadConfig == null || otherPayloadConfig.getPriority() == null + ? Long.MIN_VALUE + : Long.parseLong(otherPayloadConfig.getPriority()); + return currPriority < otherPriority; } } diff --git a/proctor-common/src/test/java/com/indeed/proctor/common/dynamic/TestNamespacesFilter.java b/proctor-common/src/test/java/com/indeed/proctor/common/dynamic/TestNamespacesFilter.java index 6916c74b..748da5ad 100644 --- a/proctor-common/src/test/java/com/indeed/proctor/common/dynamic/TestNamespacesFilter.java +++ b/proctor-common/src/test/java/com/indeed/proctor/common/dynamic/TestNamespacesFilter.java @@ -12,7 +12,10 @@ import java.util.List; -import static java.util.Collections.*; +import static java.util.Collections.EMPTY_LIST; +import static java.util.Collections.emptyList; +import static java.util.Collections.emptyMap; +import static java.util.Collections.emptySet; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.junit.Assert.assertEquals;