Settings validation for bloom extension

neo4j · Sep 14, 2017 · 53131d8 · 53131d8
1 parent aa42079
commit 53131d8
Show file tree

Hide file tree

Showing 7 changed files with 99 additions and 14 deletions.
diff --git a/community/kernel/src/main/java/org/neo4j/kernel/configuration/Settings.java b/community/kernel/src/main/java/org/neo4j/kernel/configuration/Settings.java
@@ -940,6 +940,54 @@ public String toString()
         };
     }
 
+    public static BiFunction<List<String>,Function<String,String>,List<String>> nonEmptyList =
+            new BiFunction<List<String>,Function<String,String>,List<String>>()
+            {
+                @Override
+                public List<String> apply( List<String> values, Function<String,String> settings )
+                {
+                    if ( values.isEmpty() )
+                    {
+                        throw new IllegalArgumentException( "setting must not be empty" );
+                    }
+                    return values;
+                }
+
+                @Override
+                public String toString()
+                {
+                    return "Non empty list";
+                }
+            };
+
+    public static BiFunction<List<String>,Function<String,String>,List<String>> matchesAny( final String regex )
+    {
+        final Pattern pattern = Pattern.compile( regex );
+
+        return new BiFunction<List<String>,Function<String,String>,List<String>>()
+        {
+            @Override
+            public List<String> apply( List<String> values, Function<String,String> settings )
+            {
+                for ( String value : values )
+                {
+                    if ( !pattern.matcher( value ).matches() )
+                    {
+                        throw new IllegalArgumentException( "value does not match expression:" + regex );
+                    }
+                }
+
+                return values;
+            }
+
+            @Override
+            public String toString()
+            {
+                return String.format( MATCHES_PATTERN_MESSAGE, regex );
+            }
+        };
+    }
+
     public static <T extends Comparable<T>> BiFunction<T, Function<String, String>, T> min( final T min )
     {
         return new BiFunction<T, Function<String, String>, T>()

diff --git a/enterprise/bloom-index/src/main/java/org/neo4j/kernel/api/impl/bloom/BloomIndex.java b/enterprise/bloom-index/src/main/java/org/neo4j/kernel/api/impl/bloom/BloomIndex.java
diff --git a/...ise/fulltext-addon/src/main/java/org/neo4j/kernel/api/impl/fulltext/FulltextProvider.java b/...ise/fulltext-addon/src/main/java/org/neo4j/kernel/api/impl/fulltext/FulltextProvider.java
@@ -34,6 +34,7 @@
  */
 public class FulltextProvider implements AutoCloseable
 {
+    public static final String LUCENE_FULLTEXT_ADDON_INTERNAL_ID = "__lucene__fulltext__addon__internal__id__";
     private static FulltextProvider instance;
     private final GraphDatabaseService db;
     private final Log log;

diff --git a/...don/src/main/java/org/neo4j/kernel/api/impl/fulltext/LuceneFulltextDocumentStructure.java b/...don/src/main/java/org/neo4j/kernel/api/impl/fulltext/LuceneFulltextDocumentStructure.java
@@ -36,7 +36,6 @@
 
 class LuceneFulltextDocumentStructure
 {
-    static final String ID_KEY = "id";
 
     private static final ThreadLocal<DocWithId> perThreadDocument = ThreadLocal.withInitial( DocWithId::new );
 
@@ -72,8 +71,8 @@ private static class DocWithId
 
         private DocWithId()
         {
-            idField = new StringField( ID_KEY, "", NO );
-            idValueField = new NumericDocValuesField( ID_KEY, 0L );
+            idField = new StringField( FulltextProvider.LUCENE_FULLTEXT_ADDON_INTERNAL_ID, "", NO );
+            idValueField = new NumericDocValuesField( FulltextProvider.LUCENE_FULLTEXT_ADDON_INTERNAL_ID, 0L );
             document = new Document();
             document.add( idField );
             document.add( idValueField );
@@ -102,7 +101,7 @@ private void removeAllValueFields()
             {
                 IndexableField field = it.next();
                 String fieldName = field.name();
-                if ( !fieldName.equals( ID_KEY ) )
+                if ( !fieldName.equals( FulltextProvider.LUCENE_FULLTEXT_ADDON_INTERNAL_ID ) )
                 {
                     it.remove();
                 }
@@ -113,7 +112,7 @@ private void removeAllValueFields()
 
     static Term newTermForChangeOrRemove( long id )
     {
-        return new Term( ID_KEY, "" + id );
+        return new Term( FulltextProvider.LUCENE_FULLTEXT_ADDON_INTERNAL_ID, "" + id );
     }
 
 }
diff --git a/...fulltext-addon/src/main/java/org/neo4j/kernel/api/impl/fulltext/SimpleFulltextReader.java b/...fulltext-addon/src/main/java/org/neo4j/kernel/api/impl/fulltext/SimpleFulltextReader.java
@@ -35,7 +35,7 @@
 import org.neo4j.kernel.api.impl.index.partition.PartitionSearcher;
 import org.neo4j.kernel.api.impl.schema.reader.IndexReaderCloseException;
 
-import static org.neo4j.kernel.api.impl.schema.LuceneDocumentStructure.NODE_ID_KEY;
+import static org.neo4j.kernel.api.impl.fulltext.FulltextProvider.LUCENE_FULLTEXT_ADDON_INTERNAL_ID;
 
 /**
  * Lucene index reader that is able to read/sample a single partition of a partitioned Lucene index.
@@ -105,7 +105,7 @@ private PrimitiveLongIterator indexQuery( Query query )
         {
             DocValuesCollector docValuesCollector = new DocValuesCollector( true );
             getIndexSearcher().search( query, docValuesCollector );
-            return docValuesCollector.getSortedValuesIterator( NODE_ID_KEY, Sort.RELEVANCE );
+            return docValuesCollector.getSortedValuesIterator( LUCENE_FULLTEXT_ADDON_INTERNAL_ID, Sort.RELEVANCE );
         }
         catch ( IOException e )
         {

diff --git a/...va/org/neo4j/kernel/api/impl/fulltext/integrations/bloom/LoadableBloomFulltextConfig.java b/...va/org/neo4j/kernel/api/impl/fulltext/integrations/bloom/LoadableBloomFulltextConfig.java
@@ -20,15 +20,21 @@
 package org.neo4j.kernel.api.impl.fulltext.integrations.bloom;
 
 import java.util.List;
+import java.util.function.BiFunction;
+import java.util.function.Function;
 
 import org.neo4j.configuration.Description;
 import org.neo4j.configuration.Internal;
 import org.neo4j.configuration.LoadableConfig;
 import org.neo4j.graphdb.config.Setting;
+import org.neo4j.kernel.configuration.Settings;
 
-import static org.neo4j.kernel.configuration.Settings.NO_DEFAULT;
+import static org.neo4j.kernel.api.impl.fulltext.FulltextProvider.LUCENE_FULLTEXT_ADDON_INTERNAL_ID;
 import static org.neo4j.kernel.configuration.Settings.STRING;
 import static org.neo4j.kernel.configuration.Settings.STRING_LIST;
+import static org.neo4j.kernel.configuration.Settings.buildSetting;
+import static org.neo4j.kernel.configuration.Settings.illegalValueMessage;
+import static org.neo4j.kernel.configuration.Settings.matchesAny;
 import static org.neo4j.kernel.configuration.Settings.setting;
 
 /**
@@ -37,13 +43,18 @@
 public class LoadableBloomFulltextConfig implements LoadableConfig
 {
 
-    // Bloom index
+    public static final String UNSUPPORTED_PROPERY_KEY_REGEX = "^(?!" + LUCENE_FULLTEXT_ADDON_INTERNAL_ID + ").+$";
+    public static final BiFunction<List<String>,Function<String,String>,List<String>> ILLEGAL_VALUE_CONSTRAINT =
+            illegalValueMessage( "Must not contain '" + LUCENE_FULLTEXT_ADDON_INTERNAL_ID + "'", matchesAny( UNSUPPORTED_PROPERY_KEY_REGEX ) );
+    public static final BiFunction<List<String>,Function<String,String>,List<String>> MUST_NOT_BE_EMPTY_CONSTRAINT =
+            illegalValueMessage( "Must not be empty", Settings.nonEmptyList );
+
     @Internal
-    static final Setting<List<String>> bloom_indexed_properties = setting( "unsupported.dbms.bloom_indexed_properties", STRING_LIST, NO_DEFAULT );
+    static final Setting<List<String>> bloom_indexed_properties =
+            buildSetting( "unsupported.dbms.bloom_indexed_properties", STRING_LIST, "" ).constraint( ILLEGAL_VALUE_CONSTRAINT ).constraint(
+                    MUST_NOT_BE_EMPTY_CONSTRAINT ).build();
 
     @Description( "Define the analyzer to use for the bloom index. Expects the fully qualified classname of the analyzer to use" )
     @Internal
-    static final Setting<String> bloom_analyzer =
-            setting( "unsupported.dbms.bloom_analyzer", STRING, "org.apache.lucene.analysis.standard.StandardAnalyzer" );
-
+    static final Setting<String> bloom_analyzer = setting( "unsupported.dbms.bloom_analyzer", STRING, "org.apache.lucene.analysis.standard.StandardAnalyzer" );
 }
diff --git a/...xt-addon/src/test/java/org/neo4j/kernel/api/impl/fulltext/integrations/bloom/BloomIT.java b/...xt-addon/src/test/java/org/neo4j/kernel/api/impl/fulltext/integrations/bloom/BloomIT.java
@@ -23,6 +23,7 @@
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.ExpectedException;
 
 import java.util.Collections;
 import java.util.HashMap;
@@ -34,8 +35,11 @@
 import org.neo4j.graphdb.RelationshipType;
 import org.neo4j.graphdb.Result;
 import org.neo4j.graphdb.Transaction;
+import org.neo4j.graphdb.config.InvalidSettingException;
 import org.neo4j.graphdb.config.Setting;
+import org.neo4j.kernel.api.impl.fulltext.FulltextProvider;
 import org.neo4j.test.TestGraphDatabaseFactory;
+import org.neo4j.test.mockito.matcher.RootCauseMatcher;
 import org.neo4j.test.rule.TestDirectory;
 import org.neo4j.test.rule.fs.DefaultFileSystemRule;
 import org.neo4j.test.rule.fs.FileSystemRule;
@@ -52,6 +56,8 @@ public class BloomIT
     public final FileSystemRule fs = new DefaultFileSystemRule();
     @Rule
     public final TestDirectory testDirectory = TestDirectory.testDirectory();
+    @Rule
+    public final ExpectedException expectedException = ExpectedException.none();
 
     private TestGraphDatabaseFactory factory;
     private GraphDatabaseService db;
@@ -117,9 +123,29 @@ public void shouldBeAbleToConfigureAnalyzer() throws Exception
         assertFalse( result.hasNext() );
     }
 
+    @Test
+    public void shouldNotBeAbleToStartWithoutConfiguringProperties() throws Exception
+    {
+        Map<Setting<?>,String> config = new HashMap<>();
+        expectedException.expect( new RootCauseMatcher<>( InvalidSettingException.class, "Bad value" ) );
+        db = factory.newImpermanentDatabase( testDirectory.graphDbDir(), config );
+    }
+
+    @Test
+    public void shouldNotBeAbleToStartWithIllegalPropertyKey() throws Exception
+    {
+        Map<Setting<?>,String> config = new HashMap<>();
+        config.put( LoadableBloomFulltextConfig.bloom_indexed_properties, "prop, " + FulltextProvider.LUCENE_FULLTEXT_ADDON_INTERNAL_ID + ", hello" );
+        expectedException.expect( InvalidSettingException.class );
+        db = factory.newImpermanentDatabase( testDirectory.graphDbDir(), config );
+    }
+
     @After
     public void after() throws Exception
     {
-        db.shutdown();
+        if ( db != null )
+        {
+            db.shutdown();
+        }
     }
 }