EXP-660: Make priority comparison a class method

indeedeng · Jun 13, 2024 · 0ae13b3 · 0ae13b3
1 parent a9ea605
commit 0ae13b3
Show file tree

Hide file tree

Showing 3 changed files with 14 additions and 12 deletions.
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
@@ -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
@@ -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
@@ -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;