Skip to content
This repository has been archived by the owner on Jul 10, 2024. It is now read-only.

Commit

Permalink
PROC-1522: Use Priorty for populating aggregate payload properties
Browse files Browse the repository at this point in the history
  • Loading branch information
@zacharygoodwin
zacharygoodwin committed Jun 3, 2024
1 parent 8f89748 commit bbca4b5
Show file tree
Hide file tree
Showing 2 changed files with 157 additions and 8 deletions.
47 changes: 39 additions & 8 deletions proctor-common/src/main/java/com/indeed/proctor/common/Proctor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import com.indeed.proctor.common.model.Audit;
import com.indeed.proctor.common.model.ConsumableTestDefinition;
import com.indeed.proctor.common.model.Payload;
import com.indeed.proctor.common.model.PayloadExperimentConfig;
import com.indeed.proctor.common.model.TestBucket;
import com.indeed.proctor.common.model.TestMatrixArtifact;
import com.indeed.proctor.common.model.TestType;
Expand Down Expand Up @@ -403,7 +404,7 @@ public ProctorResult determineTestGroups(
if (chooseResult.getTestBucket() != null) {
testGroups.put(testName, chooseResult.getTestBucket());
if (testChooser.getTestDefinition().getPayloadExperimentConfig() != null) {
testProperties.putAll(getProperties(testName, chooseResult.getTestBucket()));
populateProperties(testName, chooseResult.getTestBucket(), testProperties);
}
}
if (chooseResult.getAllocation() != null) {
Expand Down Expand Up @@ -522,23 +523,53 @@ private boolean isIncognitoEnabled(@Nonnull final Map<String, Object> inputConte
.orElse(false);
}

private Map<String, PayloadProperty> getProperties(
final String testName, final TestBucket testBucket) {
private void populateProperties(
final String testName,
final TestBucket testBucket,
final Map<String, PayloadProperty> testProperties) {
final Payload payload = testBucket.getPayload();
if (payload != null && payload.getJson() != null) {
final SortedMap<String, PayloadProperty> testProperties = new TreeMap<>();
payload.getJson()
.fields()
.forEachRemaining(
field ->
field -> {
final PayloadProperty curr = testProperties.get(field.getKey());
if (curr == null) {
testProperties.put(
field.getKey(),
PayloadProperty.builder()
.value(field.getValue())
.testName(testName)
.build()));
return testProperties;
.build());
} else {
final PayloadExperimentConfig currPayloadConfig =
testChoosers
.get(curr.getTestName())
.getTestDefinition()
.getPayloadExperimentConfig();
final PayloadExperimentConfig newPayloadConfig =
testChoosers
.get(testName)
.getTestDefinition()
.getPayloadExperimentConfig();
if (currPayloadConfig != null
&& currPayloadConfig.getPriority() != null
&& newPayloadConfig != null
&& newPayloadConfig.getPriority() != null
&& currPayloadConfig
.getPriority()
.compareTo(
newPayloadConfig.getPriority())
< 0) {
testProperties.put(
field.getKey(),
PayloadProperty.builder()
.value(field.getValue())
.testName(testName)
.build());
}
}
});
}
return Collections.emptyMap();
}
}
118 changes: 118 additions & 0 deletions proctor-common/src/test/java/com/indeed/proctor/common/TestProctor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1010,6 +1010,124 @@ public void testDetermineTestGroups_withProperty() throws JsonProcessingExceptio
;
}

@Test
public void testDetermineTestGroups_withPropertyAndPriority() throws JsonProcessingException {
final JsonNode p1 = new ObjectMapper().readTree("{\"key1\": 1, \"key2\": \"abc\"}");
final JsonNode p2 =
new ObjectMapper()
.readTree(
"{\"key1\": 2, \"some.property\": {}, \"another.property\": [\"abc\"]}");
final JsonNode p3 = new ObjectMapper().readTree("{\"some.property\": {\"sub\": 123}}");
final TestBucket inactiveBucket = new TestBucket("inactive", -1, "", new Payload(p3));
final TestBucket controlBucket = new TestBucket("control", 0, "", new Payload(p2));
final TestBucket activeBucket = new TestBucket("active", 1, "", new Payload(p1));
final Allocation allocation1 = new Allocation("", ImmutableList.of(new Range(1, 1.0)));
final Allocation allocation2 = new Allocation("", ImmutableList.of(new Range(0, 1.0)));
final Allocation allocation3 = new Allocation("", ImmutableList.of(new Range(-1, 1.0)));

final ConsumableTestDefinition testDefinition1 =
ConsumableTestDefinition.fromTestDefinition(
TestDefinition.builder()
.setSalt("&X")
.setTestType(TestType.ANONYMOUS_USER)
.addBuckets(inactiveBucket, controlBucket, activeBucket)
.addAllocations(allocation1)
.setPayloadExperimentConfig(
PayloadExperimentConfig.builder()
.namespaces(ImmutableList.of("foobar"))
.priority("1")
.build())
.build());
final ConsumableTestDefinition testDefinition2 =
ConsumableTestDefinition.fromTestDefinition(
TestDefinition.builder()
.setSalt("&Y")
.setTestType(TestType.ANONYMOUS_USER)
.addBuckets(inactiveBucket, controlBucket, activeBucket)
.addAllocations(allocation2)
.setPayloadExperimentConfig(
PayloadExperimentConfig.builder()
.namespaces(ImmutableList.of("foobar"))
.priority("2")
.build())
.build());

final ConsumableTestDefinition testDefinition3 =
ConsumableTestDefinition.fromTestDefinition(
TestDefinition.builder()
.setSalt("&Y")
.setTestType(TestType.ANONYMOUS_USER)
.addBuckets(inactiveBucket, controlBucket, activeBucket)
.addAllocations(allocation3)
.setPayloadExperimentConfig(
PayloadExperimentConfig.builder()
.namespaces(ImmutableList.of("foobar"))
.priority("3")
.build())
.build());

final Map<String, ConsumableTestDefinition> tests =
ImmutableMap.of(
"foo", testDefinition1,
"bar", testDefinition2,
"baz", testDefinition3);
final TestMatrixArtifact matrix = new TestMatrixArtifact();
matrix.setTests(tests);
matrix.setAudit(new Audit());

final Proctor proctor =
Proctor.construct(
matrix,
ProctorLoadResult.emptyResult(),
RuleEvaluator.defaultFunctionMapperBuilder().build());

final ProctorResult proctorResult =
proctor.determineTestGroups(
Identifiers.of(TestType.ANONYMOUS_USER, "cookie"),
Collections.emptyMap(),
ForceGroupsOptions.builder().build(),
Collections.emptyList());

assertThat(proctorResult.getBuckets())
.containsOnlyKeys("foo", "bar", "baz")
.containsEntry("foo", activeBucket)
.containsEntry("bar", controlBucket)
.containsEntry("baz", inactiveBucket);
assertThat(proctorResult.getAllocations()).containsOnlyKeys("foo", "bar", "baz");
assertThat(proctorResult.getTestDefinitions())
.containsOnlyKeys("foo", "bar", "baz")
.containsEntry("foo", testDefinition1)
.containsEntry("bar", testDefinition2)
.containsEntry("baz", testDefinition3);
assertThat(proctorResult.getProperties())
.containsOnlyKeys("key1", "key2", "some.property", "another.property")
.containsEntry(
"key1",
PayloadProperty.builder()
.value(new ObjectMapper().readTree("2"))
.testName("bar")
.build())
.containsEntry(
"key2",
PayloadProperty.builder()
.value(new ObjectMapper().readTree("\"abc\""))
.testName("foo")
.build())
.containsEntry(
"some.property",
PayloadProperty.builder()
.value(new ObjectMapper().readTree("{\"sub\": 123}"))
.testName("baz")
.build())
.containsEntry(
"another.property",
PayloadProperty.builder()
.value(new ObjectMapper().readTree("[\"abc\"]"))
.testName("bar")
.build());
;
}

private static TestMatrixArtifact createTestMatrixWithOneRandomTest(final String testName) {
final TestMatrixArtifact matrix = new TestMatrixArtifact();
final ConsumableTestDefinition testDefinition = new ConsumableTestDefinition();
Expand Down

0 comments on commit bbca4b5

Please sign in to comment.