Refactor tests, impl set split/join test

hollow/src/main/java/com/netflix/hollow/core/read/engine/list/HollowListTypeDataElementsJoiner.java

@@ -84,7 +84,7 @@ public void copyRecords() {
                 }
             } // else: lopsided shards could result for consumers that skip type shards with no additions, that gets handled
               // by not writing anything to elementData, and writing the cached value of elementCounter to listPointerData
-                // SNAP: TODO: write a test for lopsided list shards
+                // SNAP: TODO: write a test for lopsided list shards. Theres one in object joiner tests.
 
             to.listPointerData.setElementValue(to.bitsPerListPointer * ordinal, to.bitsPerListPointer, elementCounter);
         }

hollow/src/main/java/com/netflix/hollow/core/read/engine/set/HollowSetTypeDataElementsJoiner.java

@@ -21,6 +21,7 @@ public HollowSetTypeDataElementsJoiner(HollowSetTypeDataElements[] from) {
     @Override
     public void init() {
         this.to = new HollowSetTypeDataElements(from[0].memoryMode, from[0].memoryRecycler);
+        to.bitsPerElement = 0;
     }
 
     @Override
@@ -47,7 +48,7 @@ public void populateStats() {
 
             HollowSetTypeDataElements source = from[fromIndex];
 
-            long startBucket = getAbsoluteBucketStart(source, ordinal);
+            long startBucket = getAbsoluteBucketStart(source, fromOrdinal);
             long endBucket = source.setPointerAndSizeData.getElementValue((long)fromOrdinal * source.bitsPerFixedLengthSetPortion, source.bitsPerSetPointer);
             long numBuckets = endBucket - startBucket;
 
@@ -83,8 +84,9 @@ public void copyRecords() {
                 }
             } // else: lopsided shards could result for consumers that skip type shards with no additions, that gets handled
             // by not writing anything to elementData, and writing the cached value of bucketCounter to listPointerData
-            // SNAP: TODO: write a test for lopsided list shards (similar to for list types)
+            // SNAP: TODO: write a test for lopsided list shards (similar to for list types). Theres one in object joiner tests.
 
+            to.setPointerAndSizeData.setElementValue((ordinal * to.bitsPerFixedLengthSetPortion), to.bitsPerSetPointer, bucketCounter);
             long setSize = source.setPointerAndSizeData.getElementValue((fromOrdinal * source.bitsPerFixedLengthSetPortion) + source.bitsPerSetPointer, source.bitsPerSetSizeValue);
             to.setPointerAndSizeData.setElementValue((ordinal * to.bitsPerFixedLengthSetPortion) + to.bitsPerSetPointer, to.bitsPerSetSizeValue, setSize);
         }

hollow/src/main/java/com/netflix/hollow/core/read/engine/set/HollowSetTypeDataElementsSplitter.java

@@ -84,6 +84,8 @@ public void copyRecords() {
                 target.elementData.setElementValue(bucketCounter[toIndex] * target.bitsPerElement, target.bitsPerElement, bucketVal);
                 bucketCounter[toIndex]++;
             }
+
+            target.setPointerAndSizeData.setElementValue((toOrdinal * target.bitsPerFixedLengthSetPortion), target.bitsPerSetPointer, bucketCounter[toIndex]);
             long setSize = from.setPointerAndSizeData.getElementValue((ordinal * from.bitsPerFixedLengthSetPortion) + from.bitsPerSetPointer, from.bitsPerSetSizeValue);
             target.setPointerAndSizeData.setElementValue((toOrdinal * target.bitsPerFixedLengthSetPortion) + target.bitsPerSetPointer, target.bitsPerSetSizeValue, setSize);
         }

hollow/src/main/java/com/netflix/hollow/core/read/engine/set/HollowSetTypeReadState.java

@@ -16,6 +16,9 @@
  */
 package com.netflix.hollow.core.read.engine.set;
 
+import static com.netflix.hollow.core.HollowConstants.ORDINAL_NONE;
+import static com.netflix.hollow.core.index.FieldPaths.FieldPathException.ErrorKind.NOT_BINDABLE;
+
 import com.netflix.hollow.api.sampling.DisabledSamplingDirector;
 import com.netflix.hollow.api.sampling.HollowSampler;
 import com.netflix.hollow.api.sampling.HollowSamplingDirector;
@@ -46,9 +49,6 @@
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
-import static com.netflix.hollow.core.HollowConstants.ORDINAL_NONE;
-import static com.netflix.hollow.core.index.FieldPaths.FieldPathException.ErrorKind.NOT_BINDABLE;
-
 /**
  * A {@link HollowTypeReadState} for OBJECT type records. 
  */
@@ -59,7 +59,7 @@ public class HollowSetTypeReadState extends HollowCollectionTypeReadState implem
 
     private final int shardNumberMask;
     private final int shardOrdinalShift;
-    private final HollowSetTypeReadStateShard shards[];
+    final HollowSetTypeReadStateShard shards[];    // SNAP: TODO: elevated from private access for testing
 
     private HollowPrimaryKeyValueDeriver keyDeriver;
 
@@ -86,7 +86,20 @@ public HollowSetTypeReadState(HollowReadStateEngine stateEngine, MemoryMode memo
 
     }
 
-    @Override
+    // SNAP: TODO: for testing
+    public HollowSetTypeReadState(HollowReadStateEngine stateEngine, MemoryMode memoryMode, HollowSetSchema schema, int numShards, HollowSetTypeReadStateShard[] shards) {
+        super(stateEngine, memoryMode, schema);
+        this.sampler = new HollowSetSampler(schema.getName(), DisabledSamplingDirector.INSTANCE);
+        this.shardNumberMask = numShards - 1;
+        this.shardOrdinalShift = 31 - Integer.numberOfLeadingZeros(numShards);
+
+        if(numShards < 1 || 1 << shardOrdinalShift != numShards)
+            throw new IllegalArgumentException("Number of shards must be a power of 2!");
+
+        this.shards = shards;
+    }
+
+        @Override
     public void readSnapshot(HollowBlobInput in, ArraySegmentRecycler memoryRecycler, int numShards) throws IOException {
         throw new UnsupportedOperationException("This type does not yet support numShards specification when reading snapshot");
     }

hollow/src/test/java/com/netflix/hollow/core/read/HollowBlobOptionalPartTest.java

@@ -34,6 +34,7 @@
 import com.netflix.hollow.core.write.objectmapper.TypeA;
 import com.netflix.hollow.core.write.objectmapper.TypeB;
 import com.netflix.hollow.core.write.objectmapper.TypeC;
+import com.netflix.hollow.test.InMemoryBlobStore;
 import java.io.ByteArrayInputStream;
 import java.io.ByteArrayOutputStream;
 import java.io.File;
@@ -43,8 +44,6 @@
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashSet;
-
-import com.netflix.hollow.test.InMemoryBlobStore;
 import org.junit.Assert;
 import org.junit.Test;
 

hollow/src/test/java/com/netflix/hollow/core/read/engine/AbstractHollowTypeDataElementsSplitJoinTest.java

@@ -0,0 +1,55 @@
+package com.netflix.hollow.core.read.engine;
+
+import com.netflix.hollow.core.AbstractStateEngineTest;
+import com.netflix.hollow.core.schema.HollowObjectSchema;
+import com.netflix.hollow.core.write.HollowObjectTypeWriteState;
+import com.netflix.hollow.core.write.HollowObjectWriteRecord;
+import org.junit.Before;
+
+public class AbstractHollowTypeDataElementsSplitJoinTest extends AbstractStateEngineTest {
+    protected HollowObjectSchema schema;
+
+    @Before
+    public void setUp() {
+        schema = new HollowObjectSchema("TestObject", 4);
+        schema.addField("longField", HollowObjectSchema.FieldType.LONG);
+        schema.addField("stringField", HollowObjectSchema.FieldType.STRING);
+        schema.addField("intField", HollowObjectSchema.FieldType.INT);
+        schema.addField("doubleField", HollowObjectSchema.FieldType.DOUBLE);
+
+        super.setUp();
+    }
+
+    @Override
+    protected void initializeTypeStates() {
+        writeStateEngine.addTypeState(new HollowObjectTypeWriteState(schema));
+    }
+
+    protected void populateWriteStateEngine(int numRecords) {
+        initWriteStateEngine();
+        HollowObjectWriteRecord rec = new HollowObjectWriteRecord(schema);
+        for(int i=0;i<numRecords;i++) {
+            rec.reset();
+            rec.setLong("longField", i);
+            rec.setString("stringField", "Value" + i);
+            rec.setInt("intField", i);
+            rec.setDouble("doubleField", i);
+
+            writeStateEngine.add("TestObject", rec);
+        }
+    }
+
+    protected void populateWriteStateEngine(int[] recordIds) {
+        initWriteStateEngine();
+        HollowObjectWriteRecord rec = new HollowObjectWriteRecord(schema);
+        for(int recordId : recordIds) {
+            rec.reset();
+            rec.setLong("longField", recordId);
+            rec.setString("stringField", "Value" + recordId);
+            rec.setInt("intField", recordId);
+            rec.setDouble("doubleField", recordId);
+
+            writeStateEngine.add("TestObject", rec);
+        }
+    }
+}

hollow/src/test/java/com/netflix/hollow/core/read/engine/list/AbstractHollowListTypeDataElementsSplitJoinTest.java

@@ -2,83 +2,43 @@
 
 import static org.mockito.Mockito.when;
 
-import com.netflix.hollow.core.AbstractStateEngineTest;
-import com.netflix.hollow.core.read.engine.HollowReadStateEngine;
-import com.netflix.hollow.core.read.filter.HollowFilterConfig;
+import com.netflix.hollow.core.read.engine.AbstractHollowTypeDataElementsSplitJoinTest;
 import com.netflix.hollow.core.read.iterator.HollowOrdinalIterator;
 import com.netflix.hollow.core.schema.HollowListSchema;
-import com.netflix.hollow.core.schema.HollowObjectSchema;
-import com.netflix.hollow.core.util.StateEngineRoundTripper;
 import com.netflix.hollow.core.write.HollowListTypeWriteState;
 import com.netflix.hollow.core.write.HollowListWriteRecord;
-import com.netflix.hollow.core.write.HollowObjectTypeWriteState;
-import com.netflix.hollow.core.write.HollowObjectWriteRecord;
 import java.io.IOException;
 import java.util.Arrays;
 import org.junit.Assert;
 import org.junit.Before;
 import org.mockito.Mock;
 import org.mockito.MockitoAnnotations;
 
-public class AbstractHollowListTypeDataElementsSplitJoinTest extends AbstractStateEngineTest {
-    protected HollowObjectSchema objectSchema;
+public class AbstractHollowListTypeDataElementsSplitJoinTest extends AbstractHollowTypeDataElementsSplitJoinTest {
     protected HollowListSchema listSchema;
 
     @Mock
     protected HollowListTypeReadState mockListTypeState;
 
     @Before
     public void setUp() {
-        this.objectSchema = new HollowObjectSchema("TestObject", 4);
-        this.objectSchema.addField("longField", HollowObjectSchema.FieldType.LONG);
-        this.objectSchema.addField("stringField", HollowObjectSchema.FieldType.STRING);
-        this.objectSchema.addField("intField", HollowObjectSchema.FieldType.INT);
-        this.objectSchema.addField("doubleField", HollowObjectSchema.FieldType.DOUBLE);
-
         this.listSchema = new HollowListSchema("TestList", "TestObject");
 
+        super.setUp();
+
         MockitoAnnotations.initMocks(this);
         HollowListTypeDataElements[] fakeDataElements = new HollowListTypeDataElements[5];
         when(mockListTypeState.currentDataElements()).thenReturn(fakeDataElements);
-        super.setUp();
     }
 
     @Override
     protected void initializeTypeStates() {
-        writeStateEngine.setTargetMaxTypeShardSize(4096);
-        writeStateEngine.addTypeState(new HollowObjectTypeWriteState(objectSchema));
+        super.initializeTypeStates();
         writeStateEngine.addTypeState(new HollowListTypeWriteState(listSchema));
+        writeStateEngine.setTargetMaxTypeShardSize(4 * 100 * 1000 * 1024);
     }
 
-    private void populateWriteStateEngine(int numRecords, int[][] listContents) {
-        initWriteStateEngine();
-        HollowObjectWriteRecord rec = new HollowObjectWriteRecord(objectSchema);
-        for(int i=0;i<numRecords;i++) {
-            rec.reset();
-            rec.setLong("longField", i);
-            rec.setString("stringField", "Value" + i);
-            rec.setInt("intField", i);
-            rec.setDouble("doubleField", i);
-
-            writeStateEngine.add("TestObject", rec);
-        }
-        for(int[] list : listContents) {
-            addRecord(Arrays.stream(list).toArray());
-        }
-    }
-
-    private void populateWriteStateEngine(int[] recordIds, int[][] listContents) {
-        initWriteStateEngine();
-        HollowObjectWriteRecord rec = new HollowObjectWriteRecord(objectSchema);
-        for(int recordId : recordIds) {
-            rec.reset();
-            rec.setLong("longField", recordId);
-            rec.setString("stringField", "Value" + recordId);
-            rec.setInt("intField", recordId);
-            rec.setDouble("doubleField", recordId);
-
-            writeStateEngine.add("TestObject", rec);
-        }
+    private void populateWriteStateEngine(int[][] listContents) {
         for(int[] list : listContents) {
             addRecord(Arrays.stream(list).toArray());
         }
@@ -96,21 +56,12 @@ private void addRecord(int... ordinals) {
 
 
     protected HollowListTypeReadState populateTypeStateWith(int numRecords, int[][] listContents) throws IOException {
-        populateWriteStateEngine(numRecords, listContents);
+        populateWriteStateEngine(numRecords);
+        populateWriteStateEngine(listContents);
         roundTripSnapshot();
         return (HollowListTypeReadState) readStateEngine.getTypeState("TestList");
     }
 
-    protected HollowListTypeReadState populateTypeStateWith(int[] recordIds, int[][] listContents) throws IOException {
-        populateWriteStateEngine(recordIds, listContents);
-        roundTripSnapshot();
-        return (HollowListTypeReadState) readStateEngine.getTypeState("TestList");
-    }
-
-    protected void assertDataUnchanged(int[][] listContents) {
-        assertDataUnchanged((HollowListTypeReadState) readStateEngine.getTypeState("TestList"), listContents);
-    }
-
     protected void assertDataUnchanged(HollowListTypeReadState typeState, int[][] listContents) {
         int numListRecords = listContents.length;
         for(int i=0;i<numListRecords;i++) {

hollow/src/test/java/com/netflix/hollow/core/read/engine/list/HollowListTypeDataElementsSplitJoinTest.java

@@ -6,9 +6,6 @@
 import com.netflix.hollow.api.consumer.fs.HollowFilesystemBlobRetriever;
 import com.netflix.hollow.core.memory.MemoryMode;
 import com.netflix.hollow.core.read.engine.HollowReadStateEngine;
-import com.netflix.hollow.core.schema.HollowSchema;
-import com.netflix.hollow.core.write.HollowListTypeWriteState;
-import com.netflix.hollow.core.write.HollowObjectTypeWriteState;
 import com.netflix.hollow.tools.checksum.HollowChecksum;
 import java.io.IOException;
 import java.nio.file.Paths;
@@ -17,13 +14,6 @@
 
 public class HollowListTypeDataElementsSplitJoinTest extends AbstractHollowListTypeDataElementsSplitJoinTest {
 
-    @Override
-    protected void initializeTypeStates() {
-        writeStateEngine.setTargetMaxTypeShardSize(4 * 100 * 1000 * 1024);
-        writeStateEngine.addTypeState(new HollowObjectTypeWriteState(objectSchema));
-        writeStateEngine.addTypeState(new HollowListTypeWriteState(listSchema));
-    }
-
     @Test
     public void testSplitThenJoin() throws IOException {
 
@@ -80,7 +70,7 @@ private void assertChecksumUnchanged(HollowListTypeReadState newTypeState, Hollo
 
         for(int i=0;i<origTypeState.numShards();i++) {
             origTypeState.shards[i].applyToChecksum(origCksum, populatedOrdinals, i, origTypeState.numShards());
-            // SNAP: TODO: this will be shardsVolatile
+            // SNAP: TODO: this will be shardsVolatile, and consider moving this method into base class
         }
 
         for(int i=0;i<newTypeState.numShards();i++) {
@@ -131,7 +121,6 @@ public void testSplittingAndJoiningWithSnapshotBlob() throws Exception {
                     throw new IllegalArgumentException("These arguments need to be specified");
                 }
                 HollowListTypeReadState typeState = (HollowListTypeReadState) readStateEngine.getTypeState(listTypeWithOneShard);
-                HollowSchema origSchema = typeState.getSchema();
 
                 assertEquals(1, typeState.numShards());
 
@@ -143,7 +132,7 @@ public void testSplittingAndJoiningWithSnapshotBlob() throws Exception {
 
                 HollowListTypeReadStateShard joinedShard = new HollowListTypeReadStateShard();
                 joinedShard.setCurrentData(joinedElements);
-                // SNAP: TODO: refactor when constructor changes (this one takes readStateEnginer which doesnt correspond to joinedShard)
+                // SNAP: TODO: refactor when constructor changes (this one takes readStateEngine which doesnt correspond to joinedShard)
                 HollowListTypeReadState resultTypeState = new HollowListTypeReadState(readStateEngine, MemoryMode.ON_HEAP, typeState.getSchema(), 1,
                         new HollowListTypeReadStateShard[]{joinedShard});