Primary and hash key index warn about non-bindable field/type at init…

… time and fail at query time

hollow/src/main/java/com/netflix/hollow/core/index/FieldPaths.java

@@ -29,12 +29,16 @@
 import java.util.Collections;
 import java.util.List;
 import java.util.Objects;
+import java.util.logging.Level;
+import java.util.logging.Logger;
 
 /**
  * Functionality for processing field paths.
  */
 public final class FieldPaths {
 
+    private static final Logger LOG = Logger.getLogger(FieldPaths.class.getName());
+
     /**
      * Creates an object-based field path given a data set and the field path in symbolic form conforming to paths
      * associated with a primary key.
@@ -124,8 +128,9 @@ static FieldPath<FieldSegment> createFieldPath(
             HollowSchema schema = dataset.getSchema(segmentType);
             // @@@ Can this only occur for anything other than the root `type`?
             if (schema == null) {
-                throw new FieldPathException(FieldPathException.ErrorKind.NOT_BINDABLE, dataset, type, segments,
-                        fieldSegments, null, i);
+                LOG.log(Level.WARNING, FieldPathException.message(FieldPathException.ErrorKind.NOT_BINDABLE, dataset,
+                        type, segments, fieldSegments, null, i));
+                throw new FieldPathException(FieldPathException.ErrorKind.NOT_BINDABLE, dataset, type, segments, fieldSegments, null, i);
             }
 
             String segment = segments[i];

hollow/src/main/java/com/netflix/hollow/core/index/HollowHashIndex.java

@@ -16,6 +16,7 @@
  */
 package com.netflix.hollow.core.index;
 
+import static com.netflix.hollow.core.index.FieldPaths.FieldPathException.ErrorKind.NOT_BINDABLE;
 import static java.util.Objects.requireNonNull;
 
 import com.netflix.hollow.core.HollowConstants;
@@ -28,6 +29,9 @@
 import com.netflix.hollow.core.read.engine.HollowReadStateEngine;
 import com.netflix.hollow.core.read.engine.HollowTypeStateListener;
 import com.netflix.hollow.core.read.engine.object.HollowObjectTypeReadState;
+import java.util.Arrays;
+import java.util.logging.Level;
+import java.util.logging.Logger;
 
 /**
  * A HollowHashIndex is used for indexing non-primary-key data.  This type of index can map multiple keys to a single matching record, and/or
@@ -38,6 +42,7 @@
  * <i>actors</i>, each elements contained therein, and finally each actors <i>actorId</i> field.
  */
 public class HollowHashIndex implements HollowTypeStateListener {
+    private static final Logger LOG = Logger.getLogger(HollowHashIndex.class.getName());
 
     private volatile HollowHashIndexState hashStateVolatile;
 
@@ -81,17 +86,34 @@ public HollowHashIndex(HollowDataAccess hollowDataAccess, String type, String se
         this.selectField = selectField;
         this.matchFields = matchFields;
 
+        if (typeState == null) {
+            LOG.log(Level.WARNING, "Index initialization for " + this + " failed because type "
+                    + type + " was not found in read state");
+            return;
+        }
         reindexHashIndex();
     }
 
     /**
      * Recreate the hash index entirely
      */
     private void reindexHashIndex() {
-        HollowHashIndexBuilder builder = new HollowHashIndexBuilder(hollowDataAccess, type, selectField, matchFields);
+        HollowHashIndexBuilder builder;
+        try {
+            builder = new HollowHashIndexBuilder(hollowDataAccess, type, selectField, matchFields);
+        } catch (FieldPaths.FieldPathException e) {
+            if (e.error == NOT_BINDABLE) {
+                LOG.log(Level.WARNING, "Index initialization for " + this
+                        + " failed because one of the match fields could not be bound to a type in"
+                        + " the read state");
+                this.hashStateVolatile = null;
+                return;
+            } else {
+                throw e;
+            }
+        }
 
         builder.buildIndex();
-
         this.hashStateVolatile = new HollowHashIndexState(builder);
     }
 
@@ -104,6 +126,9 @@ private void reindexHashIndex() {
      * found.
      */
     public HollowHashIndexResult findMatches(Object... query) {
+        if (hashStateVolatile == null) {
+            throw new IllegalStateException(this + " wasn't initialized");
+        }
         int hashCode = 0;
 
         for(int i=0;i<query.length;i++) {
@@ -202,6 +227,9 @@ private boolean matchIsEqual(FixedLengthElementArray matchHashTable, long hashBu
      * discarding the index.
      */
     public void listenForDeltaUpdates() {
+        if (typeState == null) {
+            return;
+        }
         if (!(typeState instanceof HollowObjectTypeReadState))
             throw new IllegalStateException("Cannot listen for delta updates when objectTypeDataAccess is a " + typeState.getClass().getSimpleName() + ". Is this index participating in object longevity?");
 
@@ -214,6 +242,9 @@ public void listenForDeltaUpdates() {
      * Call this method before discarding indexes which are currently listening for delta updates.
      */
     public void detachFromDeltaUpdates() {
+        if (typeState == null) {
+            return;
+        }
         if ((typeState instanceof HollowObjectTypeReadState))
             ((HollowObjectTypeReadState) typeState).removeListener(this);
     }
@@ -229,7 +260,10 @@ public void removedOrdinal(int ordinal) { }
 
     @Override
     public void endUpdate() {
-       reindexHashIndex();
+        if (hashStateVolatile == null) {
+            return;
+        }
+        reindexHashIndex();
     }
 
     /**
@@ -329,4 +363,9 @@ public int getBitsPerSelectTablePointer() {
             return bitsPerSelectTablePointer;
         }
     }
+
+    @Override
+    public String toString() {
+        return "HollowHashIndex [type=" + type + ", selectField=" + selectField + ", matchFields=" + Arrays.toString(matchFields) + "]";
+    }
 }

hollow/src/main/java/com/netflix/hollow/core/index/HollowPrimaryKeyIndex.java

@@ -17,6 +17,7 @@
 package com.netflix.hollow.core.index;
 
 import static com.netflix.hollow.core.HollowConstants.ORDINAL_NONE;
+import static com.netflix.hollow.core.index.FieldPaths.FieldPathException.ErrorKind.NOT_BINDABLE;
 import static java.util.Objects.requireNonNull;
 
 import com.netflix.hollow.core.index.key.HollowPrimaryKeyValueDeriver;
@@ -100,20 +101,37 @@ public HollowPrimaryKeyIndex(HollowReadStateEngine stateEngine, PrimaryKey prima
                 + "] failed because read state wasn't initialized");
 
         this.primaryKey = primaryKey;
-        this.typeState = (HollowObjectTypeReadState) stateEngine.getTypeState(primaryKey.getType());
         this.fieldPathIndexes = new int[primaryKey.numFields()][];
         this.fieldTypes = new FieldType[primaryKey.numFields()];
 
         this.memoryRecycler = memoryRecycler;
+        this.specificOrdinalsToIndex = specificOrdinalsToIndex;
+        this.typeState = (HollowObjectTypeReadState) stateEngine.getTypeState(primaryKey.getType());
 
-        for(int i=0;i<primaryKey.numFields();i++) {
-            fieldPathIndexes[i] = primaryKey.getFieldPathIndex(stateEngine, i);
-            fieldTypes[i] = primaryKey.getFieldType(stateEngine, i);
+        if (typeState == null) {
+            LOG.log(Level.WARNING, "Hollow Primary Key Index creation for type [" + primaryKey.getType()
+                    + "] failed because type was not found in read state");
+            this.keyDeriver = null;
+            return;
         }
 
+        int i=0;
+        try {
+            for(i=0;i<primaryKey.numFields();i++) {
+                fieldPathIndexes[i] = primaryKey.getFieldPathIndex(stateEngine, i);
+                fieldTypes[i] = primaryKey.getFieldType(stateEngine, i);
+            }
+        } catch (FieldPaths.FieldPathException e) {
+            if (e.error == NOT_BINDABLE) {
+                LOG.log(Level.WARNING, "Index initialization for " + primaryKey + " failed because field " +
+                        primaryKey.getFieldPath(i) + " could not be bound to a type in the read state");
+                this.keyDeriver = null;
+                return;
+            } else {
+                throw e;
+            }
+        }
         this.keyDeriver = new HollowPrimaryKeyValueDeriver(typeState, fieldPathIndexes, fieldTypes);
-        this.specificOrdinalsToIndex = specificOrdinalsToIndex;
-
         reindex();
     }
 
@@ -124,8 +142,15 @@ public HollowPrimaryKeyIndex(HollowReadStateEngine stateEngine, PrimaryKey prima
      * <p>
      * In order to prevent memory leaks, if this method is called and the index is no longer needed, call detachFromDeltaUpdates() before
      * discarding the index.
+     * <p>
+     * Note that this index does not listen on snapshot update. If a snapshot update occurs this index will
+     * NOT return the latest data in the consumer. Callers must detach this index instance and initialize a new
+     * one on snapshot update, or disable double snapshot update for the consumer altogether.
      */
     public void listenForDeltaUpdates() {
+        if (typeState == null) {
+            return;
+        }
         if(specificOrdinalsToIndex != null)
             throw new IllegalStateException("Cannot listen for delta updates when indexing only specified ordinals!");
 
@@ -138,6 +163,9 @@ public void listenForDeltaUpdates() {
      * Call this method before discarding indexes which are currently listening for delta updates.
      */
     public void detachFromDeltaUpdates() {
+        if (typeState == null) {
+            return;
+        }
         typeState.removeListener(this);
     }
 
@@ -162,6 +190,10 @@ public List<FieldType> getFieldTypes() {
      * @return the matching ordinal for the key, otherwise -1 if the key is not present
      */
     public int getMatchingOrdinal(Object key) {
+        if (hashTableVolatile == null) {
+            throw new IllegalStateException("Index " + primaryKey.toString()  + " wasn't initialized");
+        }
+
         PrimaryKeyIndexHashTable hashTable = hashTableVolatile;
         if(fieldPathIndexes.length != 1 || hashTable.bitsPerElement == 0)
             return -1;
@@ -197,6 +229,10 @@ public int getMatchingOrdinal(Object key) {
      * @return the matching ordinal for the two keys, otherwise -1 if the key is not present
      */
     public int getMatchingOrdinal(Object key1, Object key2) {
+        if (hashTableVolatile == null) {
+            throw new IllegalStateException("Index " + primaryKey.toString()  + " wasn't initialized");
+        }
+
         PrimaryKeyIndexHashTable hashTable = hashTableVolatile;
         if(fieldPathIndexes.length != 2 || hashTable.bitsPerElement == 0)
             return -1;
@@ -234,6 +270,10 @@ public int getMatchingOrdinal(Object key1, Object key2) {
      * @return the matching ordinal for the three keys, otherwise -1 if the key is not present
      */
     public int getMatchingOrdinal(Object key1, Object key2, Object key3) {
+        if (hashTableVolatile == null) {
+            throw new IllegalStateException("Index " + primaryKey.toString()  + " wasn't initialized");
+        }
+
         PrimaryKeyIndexHashTable hashTable = hashTableVolatile;
         if(fieldPathIndexes.length != 3 || hashTable.bitsPerElement == 0)
             return -1;
@@ -270,6 +310,10 @@ public int getMatchingOrdinal(Object key1, Object key2, Object key3) {
      * @return the matching ordinal for the keys, otherwise -1 if the key is not present
      */
     public int getMatchingOrdinal(Object... keys) {
+        if (hashTableVolatile == null) {
+            throw new IllegalStateException("Index " + primaryKey.toString()  + " wasn't initialized");
+        }
+
         PrimaryKeyIndexHashTable hashTable = hashTableVolatile;
         if(fieldPathIndexes.length != keys.length || hashTable.bitsPerElement == 0)
             return -1;
@@ -378,6 +422,10 @@ public void removedOrdinal(int ordinal) { }
 
     @Override
     public synchronized void endUpdate() {
+        if (hashTableVolatile == null) {
+            return;
+        }
+
         BitSet ordinals = typeState.getListener(PopulatedOrdinalListener.class).getPopulatedOrdinals();
 
         int hashTableSize = HashCodes.hashTableSize(ordinals.cardinality());

hollow/src/test/java/com/netflix/hollow/core/index/HollowHashIndexTest.java

@@ -19,11 +19,19 @@
 import static java.util.Arrays.stream;
 import static java.util.stream.Collectors.toList;
 import static java.util.stream.Collectors.toSet;
+import static org.junit.Assert.fail;
 
 import com.netflix.hollow.core.AbstractStateEngineTest;
+import com.netflix.hollow.core.read.engine.HollowReadStateEngine;
 import com.netflix.hollow.core.read.iterator.HollowOrdinalIterator;
+import com.netflix.hollow.core.schema.HollowObjectSchema;
+import com.netflix.hollow.core.util.StateEngineRoundTripper;
+import com.netflix.hollow.core.write.HollowObjectTypeWriteState;
+import com.netflix.hollow.core.write.HollowObjectWriteRecord;
+import com.netflix.hollow.core.write.HollowWriteStateEngine;
 import com.netflix.hollow.core.write.objectmapper.HollowInline;
 import com.netflix.hollow.core.write.objectmapper.HollowObjectMapper;
+import java.io.IOException;
 import java.util.Arrays;
 import java.util.HashSet;
 import java.util.List;
@@ -261,7 +269,7 @@ public void testFindingMatchForNullQueryValue() throws Exception {
         roundTripSnapshot();
         HollowHashIndex index = new HollowHashIndex(readStateEngine, "TypeB", "", "b1.value");
         index.findMatches(new Object[]{null});
-        Assert.fail("exception expected");
+        fail("exception expected");
     }
 
     @Test
@@ -321,6 +329,44 @@ public void testGettingPropertiesValues() throws Exception {
         Assert.assertEquals(index.getSelectField(), "");
     }
 
+    @Test
+    public void testNotBindable() throws IOException {
+        HollowWriteStateEngine writeEngine = new HollowWriteStateEngine();
+        HollowObjectSchema movieSchema = new HollowObjectSchema("Movie", 3);
+        movieSchema.addField("id", HollowObjectSchema.FieldType.LONG);
+        movieSchema.addField("title", HollowObjectSchema.FieldType.REFERENCE, "String");
+        movieSchema.addField("releaseYear", HollowObjectSchema.FieldType.INT);
+        HollowObjectTypeWriteState movieState = new HollowObjectTypeWriteState(movieSchema);
+        writeEngine.addTypeState(movieState);
+
+        HollowObjectWriteRecord movieRec = new HollowObjectWriteRecord(movieSchema);
+        movieRec.setLong("id", 1);
+        movieRec.setReference("title", 0);  // NOTE that String type wasn't added
+        movieRec.setInt("releaseYear", 1999);
+        writeEngine.add("Movie", movieRec);
+
+        HollowReadStateEngine readEngine = new HollowReadStateEngine();
+        StateEngineRoundTripper.roundTripSnapshot(writeEngine, readEngine);
+
+        // invalid because root type doesn't exist
+        HollowHashIndex invalidHashIndex1 = new HollowHashIndex(readEngine, "String", "value", "value");
+        try {
+            invalidHashIndex1.findMatches("test");
+            fail("Index on root type not bound is expected to fail hard at query time");
+        } catch (IllegalStateException e) {}
+
+        // invalid because a type in the field paths doesn't exist
+        HollowHashIndex invalidHashIndex2 = new HollowHashIndex(readEngine, "Movie", "id", "title.value");
+        try {
+            invalidHashIndex2.findMatches("test");
+            fail("Index on field path not bound is expected to fail hard at query time");
+        } catch (IllegalStateException e) {}
+
+        // valid index despite a non-indexed field (title) not bindable to a type (String)
+        HollowPrimaryKeyIndex validPki = new HollowPrimaryKeyIndex(readEngine, "Movie", "id");
+        Assert.assertEquals(0, validPki.getMatchingOrdinal(1L));
+    }
+
     private void assertIteratorContainsAll(HollowOrdinalIterator iter, int... expectedOrdinals) {
         Set<Integer> ordinalSet = new HashSet<>();
         int ordinal = iter.next();