Skip to content

Commit 909939f

Browse files
Craigacpjhalexand
Craigacp
authored and
jhalexand
committed
Fixing ProvenanceUtil.extractConfiguration so the NullConfiguredProvenances don't leak into the ConfigurationData.
1 parent 487dd71 commit 909939f

File tree

5 files changed

+112
-15
lines changed

5 files changed

+112
-15
lines changed

‎olcut-core/src/main/java/com/oracle/labs/mlrg/olcut/provenance/ProvenanceUtil.java

Lines changed: 26 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
import com.oracle.labs.mlrg.olcut.config.property.ListProperty;
3434
import com.oracle.labs.mlrg.olcut.config.property.MapProperty;
3535
import com.oracle.labs.mlrg.olcut.config.property.SimpleProperty;
36+
import com.oracle.labs.mlrg.olcut.provenance.impl.NullConfiguredProvenance;
3637
import com.oracle.labs.mlrg.olcut.provenance.io.FlatMarshalledProvenance;
3738
import com.oracle.labs.mlrg.olcut.provenance.io.ListMarshalledProvenance;
3839
import com.oracle.labs.mlrg.olcut.provenance.io.MapMarshalledProvenance;
@@ -490,14 +491,17 @@ public static List<ConfigurationData> extractConfiguration(ObjectProvenance prov
490491
processingQueue.add(provenance);
491492
while (!processingQueue.isEmpty()) {
492493
ObjectProvenance curProv = processingQueue.poll();
493-
if (curProv instanceof ConfiguredObjectProvenance) {
494-
if (!provenanceTracker.containsKey((ConfiguredObjectProvenance)curProv)) {
495-
provenanceTracker.put((ConfiguredObjectProvenance) curProv, counter);
496-
traversalOrder.add((ConfiguredObjectProvenance) curProv);
497-
counter++;
494+
// skip null provenances
495+
if (!(curProv instanceof NullConfiguredProvenance)) {
496+
if (curProv instanceof ConfiguredObjectProvenance) {
497+
if (!provenanceTracker.containsKey((ConfiguredObjectProvenance) curProv)) {
498+
provenanceTracker.put((ConfiguredObjectProvenance) curProv, counter);
499+
traversalOrder.add((ConfiguredObjectProvenance) curProv);
500+
counter++;
501+
}
498502
}
503+
extractProvenanceToQueue(processingQueue, curProv);
499504
}
500-
extractProvenanceToQueue(processingQueue, curProv);
501505
}
502506

503507
List<ConfigurationData> output = new ArrayList<>();
@@ -528,28 +532,37 @@ private static ConfigurationData extractSingleConfiguration(ConfiguredObjectProv
528532

529533
for (Provenance p : (ListProvenance<?>)prov) {
530534
if (p instanceof ConfiguredObjectProvenance) {
531-
list.add(new SimpleProperty(computeName((ConfiguredObjectProvenance)p,map.get(p))));
535+
// skip nulls
536+
if (!(p instanceof NullConfiguredProvenance)) {
537+
list.add(new SimpleProperty(computeName((ConfiguredObjectProvenance) p, map.get(p))));
538+
}
532539
} else {
533540
list.add(new SimpleProperty(p.toString()));
534541
}
535542
}
536543

537544
data.add(e.getKey(),new ListProperty(list));
538545
} else if (prov instanceof MapProvenance) {
539-
Map<String,SimpleProperty> propMap = new HashMap<>();
546+
Map<String, SimpleProperty> propMap = new HashMap<>();
540547

541-
for (Pair<String,? extends Provenance> pair : (MapProvenance<?>)prov) {
548+
for (Pair<String, ? extends Provenance> pair : (MapProvenance<?>) prov) {
542549
Provenance valueProv = pair.getB();
543550
if (valueProv instanceof ConfiguredObjectProvenance) {
544-
propMap.put(pair.getA(),new SimpleProperty(computeName((ConfiguredObjectProvenance)valueProv,map.get(valueProv))));
551+
// skip nulls
552+
if (!(valueProv instanceof NullConfiguredProvenance)) {
553+
propMap.put(pair.getA(), new SimpleProperty(computeName((ConfiguredObjectProvenance) valueProv, map.get(valueProv))));
554+
}
545555
} else {
546-
propMap.put(pair.getA(),new SimpleProperty(valueProv.toString()));
556+
propMap.put(pair.getA(), new SimpleProperty(valueProv.toString()));
547557
}
548558
}
549559

550-
data.add(e.getKey(),new MapProperty(propMap));
560+
data.add(e.getKey(), new MapProperty(propMap));
551561
} else if (prov instanceof ConfiguredObjectProvenance) {
552-
data.add(e.getKey(),new SimpleProperty(computeName((ConfiguredObjectProvenance)prov,map.get(prov))));
562+
// Skip nulls;
563+
if (!(prov instanceof NullConfiguredProvenance)) {
564+
data.add(e.getKey(), new SimpleProperty(computeName((ConfiguredObjectProvenance) prov, map.get(prov))));
565+
}
553566
} else {
554567
data.add(e.getKey(),new SimpleProperty(prov.toString()));
555568
}

‎olcut-core/src/main/java/com/oracle/labs/mlrg/olcut/provenance/impl/NullConfiguredProvenance.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,10 @@ public NullConfiguredProvenance(String className) {
5656
this.className = className;
5757
}
5858

59+
/**
60+
* Used to unmarshal a null provenance.
61+
* @param map The set of fields. Must only contain the class name the provenance came from.
62+
*/
5963
public NullConfiguredProvenance(Map<String,Provenance> map) {
6064
if (map.containsKey(ObjectProvenance.CLASS_NAME)) {
6165
this.className = map.get(ObjectProvenance.CLASS_NAME).toString();

‎olcut-core/src/test/java/com/oracle/labs/mlrg/olcut/provenance/ExampleProvenancableConfigurable.java

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@
3131
import com.oracle.labs.mlrg.olcut.config.Config;
3232
import com.oracle.labs.mlrg.olcut.config.Configurable;
3333
import com.oracle.labs.mlrg.olcut.provenance.ExampleProvenancableConfigurable.ExampleProvenance;
34-
import com.oracle.labs.mlrg.olcut.provenance.io.ObjectMarshalledProvenance;
3534
import com.oracle.labs.mlrg.olcut.provenance.primitives.DoubleProvenance;
3635
import com.oracle.labs.mlrg.olcut.provenance.primitives.IntProvenance;
3736
import com.oracle.labs.mlrg.olcut.provenance.primitives.StringProvenance;

‎olcut-core/src/test/java/com/oracle/labs/mlrg/olcut/provenance/ProvenanceUtilTest.java

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,22 @@
11
package com.oracle.labs.mlrg.olcut.provenance;
22

33
import static org.junit.jupiter.api.Assertions.assertEquals;
4+
import static org.junit.jupiter.api.Assertions.assertTrue;
45

56
import java.io.File;
67
import java.io.IOException;
78
import java.io.ObjectInputStream;
89
import java.io.ObjectOutputStream;
910
import java.io.Serializable;
11+
import java.util.ArrayList;
12+
import java.util.Arrays;
13+
import java.util.List;
1014
import java.util.logging.Level;
1115
import java.util.logging.Logger;
1216

17+
import com.oracle.labs.mlrg.olcut.config.ConfigurationData;
18+
import com.oracle.labs.mlrg.olcut.provenance.impl.NullConfiguredProvenance;
19+
import com.oracle.labs.mlrg.olcut.provenance.io.ObjectMarshalledProvenance;
1320
import org.junit.jupiter.api.BeforeAll;
1421
import org.junit.jupiter.api.Test;
1522

@@ -27,7 +34,29 @@ public static void setup() {
2734
}
2835

2936
@Test
30-
void testSerialize() throws Exception {
37+
public void testNullFields() {
38+
TestProvenancableConfigurable test = new TestProvenancableConfigurable(25, Arrays.asList(5,4,3,2,1));
39+
40+
ConfiguredObjectProvenance prov = test.getProvenance();
41+
42+
assertTrue(prov.getConfiguredParameters().get("nullField") instanceof NullConfiguredProvenance);
43+
44+
List<ConfigurationData> configList = ProvenanceUtil.extractConfiguration(prov);
45+
46+
// The NullConfiguredProvenance should be an empty field in the configuration object that holds it so there is
47+
// only a single object provenance to convert into configuration data.
48+
assertEquals(1,configList.size());
49+
50+
List<ObjectMarshalledProvenance> marshalledProvenances = ProvenanceUtil.marshalProvenance(prov);
51+
assertEquals(2,marshalledProvenances.size());
52+
53+
ObjectProvenance unmarshalledProvenance = ProvenanceUtil.unmarshalProvenance(marshalledProvenances);
54+
55+
assertEquals(prov,unmarshalledProvenance);
56+
}
57+
58+
@Test
59+
public void testSerialize() throws Exception {
3160
File tempFile = File.createTempFile("serialized-provenancable", ".ser", new File("target"));
3261
tempFile.deleteOnExit();
3362

‎olcut-core/src/test/java/com/oracle/labs/mlrg/olcut/provenance/TestProvenancableConfigurable.java

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
package com.oracle.labs.mlrg.olcut.provenance;
2+
3+
import com.oracle.labs.mlrg.olcut.config.Config;
4+
import com.oracle.labs.mlrg.olcut.config.Configurable;
5+
import com.oracle.labs.mlrg.olcut.provenance.impl.ConfiguredObjectProvenanceImpl;
6+
7+
import java.util.ArrayList;
8+
import java.util.List;
9+
import java.util.Objects;
10+
11+
/**
12+
* Test class for provenance null handling.
13+
*/
14+
public final class TestProvenancableConfigurable implements Configurable, Provenancable<ConfiguredObjectProvenance> {
15+
16+
// This field is always null to test the extraction of null fields.
17+
@Config
18+
public ExampleProvenancableConfigurable nullField = null;
19+
20+
@Config
21+
public int someIntValue = 5;
22+
23+
@Config
24+
public List<Integer> someListOfInts = new ArrayList<>();
25+
26+
public TestProvenancableConfigurable() {}
27+
28+
public TestProvenancableConfigurable(int intValue, List<Integer> listOfInts) {
29+
someIntValue = intValue;
30+
someListOfInts = new ArrayList<>(listOfInts);
31+
}
32+
33+
@Override
34+
public boolean equals(Object o) {
35+
if (this == o) return true;
36+
if (o == null || getClass() != o.getClass()) return false;
37+
TestProvenancableConfigurable that = (TestProvenancableConfigurable) o;
38+
return someIntValue == that.someIntValue &&
39+
Objects.equals(nullField, that.nullField) &&
40+
someListOfInts.equals(that.someListOfInts);
41+
}
42+
43+
@Override
44+
public int hashCode() {
45+
return Objects.hash(nullField, someIntValue, someListOfInts);
46+
}
47+
48+
@Override
49+
public ConfiguredObjectProvenance getProvenance() {
50+
return new ConfiguredObjectProvenanceImpl(this,"test-configurable-provenancable");
51+
}
52+
}

0 commit comments

Comments
 (0)