You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-commits@jackrabbit.apache.org by ch...@apache.org on 2014/11/21 08:47:41 UTC

svn commit: r1640872 - in /jackrabbit/oak/trunk/oak-lucene/src: main/java/org/apache/jackrabbit/oak/plugins/index/lucene/ test/java/org/apache/jackrabbit/oak/plugins/index/lucene/

Author: chetanm
Date: Fri Nov 21 07:47:41 2014
New Revision: 1640872

URL: http://svn.apache.org/r1640872
Log:
OAK-2283 - Fix inconsistent handling of includedPropertyTypes

Adding experimental support for propertyType restriction in property definition. This is mostly for existing testcase. In general fulltext index case rule level setting for 'includePropertyTypes' would be sufficient

LuceneIndexEditor
* Fixed the editor to not evaluate property type restriction on its
   own and instead rely in IndexingRule
* Removed TODO related to Analyzed field should be stored against
   different field name as that has already been done with OAK-2280

Modified:
    jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/IndexDefinition.java
    jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexConstants.java
    jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexEditor.java
    jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/PropertyDefinition.java
    jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexTest.java

Modified: jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/IndexDefinition.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/IndexDefinition.java?rev=1640872&r1=1640871&r2=1640872&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/IndexDefinition.java (original)
+++ jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/IndexDefinition.java Fri Nov 21 07:47:41 2014
@@ -103,9 +103,9 @@ class IndexDefinition {
 
     private static String TYPES_ALLOW_ALL_NAME = "all";
 
-    private static final int TYPES_ALLOW_NONE = PropertyType.UNDEFINED;
+    static final int TYPES_ALLOW_NONE = PropertyType.UNDEFINED;
 
-    private static final int TYPES_ALLOW_ALL = -1;
+    static final int TYPES_ALLOW_ALL = -1;
 
     private final boolean fullTextEnabled;
 
@@ -464,7 +464,7 @@ class IndexDefinition {
             this.defaultFulltextEnabled = getOptionalValue(config, LuceneIndexConstants.FULL_TEXT_ENABLED, false);
             //TODO Provide a new proper propertyName for enabling storage
             this.defaultStorageEnabled = getOptionalValue(config, LuceneIndexConstants.EXPERIMENTAL_STORAGE, false);
-            this.propertyTypes = getSupportedTypes(config, TYPES_ALLOW_ALL);
+            this.propertyTypes = getSupportedTypes(config, INCLUDE_PROPERTY_TYPES, TYPES_ALLOW_ALL);
 
             List<NamePattern> namePatterns = newArrayList();
             Map<String,RelativeProperty> relativeProps = newHashMap();
@@ -585,17 +585,7 @@ class IndexDefinition {
         }
 
         public boolean includePropertyType(int type){
-            //TODO Rules related to type inclusion need to be synced
-            //Login IndexEditor evaluates differently compared to IndexDefinition
-            if(propertyTypes == TYPES_ALLOW_ALL){
-                return true;
-            }
-
-            if (propertyTypes == TYPES_ALLOW_NONE){
-                return false;
-            }
-
-            return (propertyTypes & (1 << type)) != 0;
+           return IndexDefinition.includePropertyType(propertyTypes, type);
         }
 
         public Collection<RelativeProperty> getRelativeProps() {
@@ -925,8 +915,8 @@ class IndexDefinition {
         return state.hasProperty(OAK_CHILD_ORDER);
     }
 
-    private static int getSupportedTypes(NodeState defn, int defaultVal) {
-        PropertyState pst = defn.getProperty(INCLUDE_PROPERTY_TYPES);
+    static int getSupportedTypes(NodeState defn, String typePropertyName, int defaultVal) {
+        PropertyState pst = defn.getProperty(typePropertyName);
         if (pst != null) {
             int types = 0;
             for (String inc : pst.getValue(Type.STRINGS)) {
@@ -945,6 +935,18 @@ class IndexDefinition {
         return defaultVal;
     }
 
+    static boolean includePropertyType(int includedPropertyTypes, int type){
+        if(includedPropertyTypes == TYPES_ALLOW_ALL){
+            return true;
+        }
+
+        if (includedPropertyTypes == TYPES_ALLOW_NONE){
+            return false;
+        }
+
+        return (includedPropertyTypes & (1 << type)) != 0;
+    }
+
     private static boolean hasFulltextEnabledIndexRule(List<IndexingRule> rules) {
         for (IndexingRule rule : rules){
             if (rule.fulltextEnabled){

Modified: jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexConstants.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexConstants.java?rev=1640872&r1=1640871&r2=1640872&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexConstants.java (original)
+++ jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexConstants.java Fri Nov 21 07:47:41 2014
@@ -142,4 +142,11 @@ public interface LuceneIndexConstants {
     String TEST_MODE = "testMode";
 
     String EVALUATE_PATH_RESTRICTION = "oak.experimental.evaluatePathRestrictions";
+
+    /**
+     * Experimental config to restrict which property type gets indexed at
+     * property definition level. Mostly index rule level #INCLUDE_PROPERTY_TYPES
+     * should be sufficient
+     */
+    String PROP_INCLUDED_TYPE = "oak.experimental.includePropertyTypes";
 }

Modified: jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexEditor.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexEditor.java?rev=1640872&r1=1640871&r2=1640872&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexEditor.java (original)
+++ jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexEditor.java Fri Nov 21 07:47:41 2014
@@ -324,12 +324,7 @@ public class LuceneIndexEditor implement
                                   PropertyState property,
                                   String pname,
                                   PropertyDefinition pd) throws CommitFailedException {
-        //In case of fulltext we also check if given type is enabled for indexing
-        //TODO Use context.includePropertyType however that cause issue. Need
-        //to make filtering based on type consistent both on indexing side and
-        //query side
-        //TODO Replace with indexRule.includeType
-        boolean includeTypeForFullText = (indexingRule.propertyTypes & (1 << property.getType().tag())) != 0;
+        boolean includeTypeForFullText = indexingRule.includePropertyType(property.getType().tag());
         if (Type.BINARY.tag() == property.getType().tag()
                 && includeTypeForFullText) {
             this.context.indexUpdate();
@@ -338,21 +333,16 @@ public class LuceneIndexEditor implement
         }  else {
             boolean dirty = false;
 
-            if (pd.propertyIndex){
+            if (pd.propertyIndex && pd.includePropertyType(property.getType().tag())){
                 dirty |= addTypedFields(fields, property, pname);
             }
 
             if (pd.fulltextEnabled() && includeTypeForFullText) {
                 for (String value : property.getValue(Type.STRINGS)) {
                     this.context.indexUpdate();
-                    //TODO Analyzed field should be stored against different field name
-                    //as term field use the same name. For compatibility this should be done
-                    //for newer index versions only
-                    if (pd.analyzed) {
+                    if (pd.analyzed && pd.includePropertyType(property.getType().tag())) {
                         String analyzedPropName = constructAnalyzedPropertyName(pname);
                         fields.add(newPropertyField(analyzedPropName, value, !pd.skipTokenization(pname), pd.stored));
-                        //TODO Property field uses OakType which has omitNorms set hence
-                        //cannot be boosted
                     }
 
                     if (pd.nodeScopeIndex) {

Modified: jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/PropertyDefinition.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/PropertyDefinition.java?rev=1640872&r1=1640871&r2=1640872&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/PropertyDefinition.java (original)
+++ jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/PropertyDefinition.java Fri Nov 21 07:47:41 2014
@@ -68,6 +68,8 @@ class PropertyDefinition {
 
     final boolean ordered;
 
+    final int includedPropertyTypes;
+
     public PropertyDefinition(IndexingRule idxDefn, String name, NodeState defn) {
         this.isRegexp = getOptionalValue(defn, PROP_IS_REGEX, false);
         this.name = getName(defn, name);
@@ -82,6 +84,8 @@ class PropertyDefinition {
         //If node is not set for full text then a property definition indicates that definition is for property index
         this.propertyIndex = getOptionalValue(defn, LuceneIndexConstants.PROP_PROPERTY_INDEX, !idxDefn.defaultFulltextEnabled);
         this.ordered = getOptionalValue(defn, LuceneIndexConstants.PROP_ORDERED, false);
+        this.includedPropertyTypes = IndexDefinition.getSupportedTypes(defn, LuceneIndexConstants.PROP_INCLUDED_TYPE,
+                IndexDefinition.TYPES_ALLOW_ALL);
 
         //TODO Add test case for above cases
 
@@ -128,6 +132,10 @@ class PropertyDefinition {
         return isTypeDefined() ? propertyType : PropertyType.STRING;
     }
 
+    public boolean includePropertyType(int type){
+        return IndexDefinition.includePropertyType(includedPropertyTypes, type);
+    }
+
     @Override
     public String toString() {
         return "PropertyDefinition{" +

Modified: jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexTest.java?rev=1640872&r1=1640871&r2=1640872&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexTest.java (original)
+++ jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexTest.java Fri Nov 21 07:47:41 2014
@@ -22,6 +22,7 @@ import java.util.Set;
 
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
+import javax.jcr.PropertyType;
 
 import static com.google.common.collect.ImmutableList.copyOf;
 import static com.google.common.collect.Iterators.transform;
@@ -34,6 +35,7 @@ import static org.apache.jackrabbit.JcrC
 import static org.apache.jackrabbit.JcrConstants.NT_BASE;
 import static org.apache.jackrabbit.oak.plugins.index.IndexConstants.INDEX_DEFINITIONS_NAME;
 import static org.apache.jackrabbit.oak.plugins.index.IndexConstants.REINDEX_PROPERTY_NAME;
+import static org.apache.jackrabbit.oak.plugins.index.lucene.LuceneIndexConstants.INDEX_RULES;
 import static org.apache.jackrabbit.oak.plugins.index.lucene.LuceneIndexConstants.PERSISTENCE_FILE;
 import static org.apache.jackrabbit.oak.plugins.index.lucene.LuceneIndexConstants.PERSISTENCE_NAME;
 import static org.apache.jackrabbit.oak.plugins.index.lucene.LuceneIndexConstants.PERSISTENCE_PATH;
@@ -66,7 +68,6 @@ import org.apache.jackrabbit.oak.spi.sta
 import org.apache.jackrabbit.oak.spi.state.NodeStore;
 import org.apache.lucene.analysis.Analyzer;
 import org.junit.After;
-import org.junit.Ignore;
 import org.junit.Test;
 
 import com.google.common.collect.ImmutableList;
@@ -178,11 +179,15 @@ public class LuceneIndexTest {
         assertFalse(cursor.hasNext());
     }
 
-    @Ignore("OAK-2283")
+
     @Test
     public void testLucene3() throws Exception {
-        NodeBuilder index = builder.child(INDEX_DEFINITIONS_NAME);
-        newLucenePropertyIndexDefinition(index, "lucene", ImmutableSet.of("foo"), null);
+        NodeBuilder index = newLucenePropertyIndexDefinition(builder.child(INDEX_DEFINITIONS_NAME),
+                "lucene", ImmutableSet.of("foo"), null);
+        NodeBuilder rules = index.child(INDEX_RULES);
+        NodeBuilder fooProp = rules.child("nt:base").child(LuceneIndexConstants.PROP_NODE).child("foo");
+        fooProp.setProperty(LuceneIndexConstants.PROP_PROPERTY_INDEX, true);
+        fooProp.setProperty(LuceneIndexConstants.PROP_INCLUDED_TYPE, PropertyType.TYPENAME_STRING);
 
         NodeState before = builder.getNodeState();
         builder.setProperty("foo", "bar");