Merge branch 'feature/SEARCH-1745_WrapNPEForShardKey' into 'master'

SEARCH-1745 Better handling of NPE in MetadataTracker.

See merge request search_discovery/insightengine!117

.gitignore

@@ -4,6 +4,9 @@ target
 .project
 .idea/
 *.iml
+.vscode
+overlays
+*.orig
 .metadata
 .vscode
 overlays

search-services/alfresco-search/src/main/java/org/alfresco/solr/tracker/MetadataTracker.java

@@ -52,15 +52,18 @@ import org.alfresco.solr.client.SOLRAPIClient;
 import org.alfresco.solr.client.Transaction;
 import org.alfresco.solr.client.Transactions;
 import org.apache.commons.codec.EncoderException;
+import org.apache.commons.lang3.StringUtils;
 import org.json.JSONException;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import static java.util.Optional.of;
 
+import static org.alfresco.solr.tracker.DocRouterFactory.SHARD_KEY_KEY;
+
 /*
  * This tracks two things: transactions and metadata nodes
- * @author Ahmed Owianå
+ * @author Ahmed Owian
  */
 public class MetadataTracker extends AbstractTracker implements Tracker
 {
@@ -89,7 +92,7 @@ public class MetadataTracker extends AbstractTracker implements Tracker
         super(p, client, coreName, informationServer, Tracker.Type.MetaData);
         transactionDocsBatchSize = Integer.parseInt(p.getProperty("alfresco.transactionDocsBatchSize", "100"));
         shardMethod = p.getProperty("shard.method", SHARD_METHOD_DBID);
-        shardKey = p.getProperty(DocRouterFactory.SHARD_KEY_KEY);
+        shardKey = p.getProperty(SHARD_KEY_KEY);
         updateShardProperty();
         docRouter = DocRouterFactory.getRouter(p, ShardMethodEnum.getShardMethod(shardMethod));
         nodeBatchSize = Integer.parseInt(p.getProperty("alfresco.nodeBatchSize", "10"));
@@ -1202,6 +1205,10 @@ public class MetadataTracker extends AbstractTracker implements Tracker
 
     public static QName getShardProperty(String field)
     {
+        if (StringUtils.isBlank(field))
+        {
+            throw new IllegalArgumentException("Sharding property " + SHARD_KEY_KEY + " has not been set.");
+        }
         AlfrescoSolrDataModel dataModel = AlfrescoSolrDataModel.getInstance();
         NamespaceDAO namespaceDAO = dataModel.getNamespaceDAO();
         DictionaryService dictionaryService = dataModel.getDictionaryService(CMISStrictDictionaryService.DEFAULT);
@@ -1209,11 +1216,9 @@ public class MetadataTracker extends AbstractTracker implements Tracker
                 namespaceDAO,
                 dictionaryService,
                 field);
-
         if (propertyDef == null)
         {
-            log.error("Sharding property not found: {}", field);
+            throw new IllegalStateException("Sharding property " + SHARD_KEY_KEY + " was set to " + field + ", but no such property was found.");
-            return null;
         }
         return propertyDef.getName();
     }

search-services/alfresco-search/src/test/java/org/alfresco/solr/tracker/DistributedExplicitShardRoutingTrackerTest.java

@@ -52,6 +52,7 @@ import static org.alfresco.solr.AlfrescoSolrUtils.getNodeMetaData;
 import static org.alfresco.solr.AlfrescoSolrUtils.getTransaction;
 import static org.alfresco.solr.AlfrescoSolrUtils.indexAclChangeSet;
 import static org.alfresco.solr.AlfrescoSolrUtils.list;
+import static org.alfresco.solr.tracker.DocRouterFactory.SHARD_KEY_KEY;
 
 /**
  * Test Routes based on an explicit shard
@@ -179,7 +180,7 @@ public class DistributedExplicitShardRoutingTrackerTest extends AbstractAlfresco
         prop.put("shard.method", ShardMethodEnum.EXPLICIT_ID.toString());
         //Normally this would be used by the Solr client which will automatically add the property to the node.shardPropertyValue
         //For testing this doesn't work like that so I setShardPropertyValue explicitly above.
-        prop.put("shard.key", ContentModel.PROP_SKYPE.toString());
+        prop.put(SHARD_KEY_KEY, ContentModel.PROP_SKYPE.toString());
         return prop;
     }
 }

search-services/alfresco-search/src/test/java/org/alfresco/solr/tracker/DistributedPropertyBasedAlfrescoSolrTrackerTest.java

@@ -18,7 +18,27 @@
  */
 package org.alfresco.solr.tracker;
 
+import static java.util.Collections.singletonList;
+
+import static org.alfresco.repo.search.adaptor.lucene.QueryConstants.FIELD_DOC_TYPE;
+import static org.alfresco.solr.AlfrescoSolrUtils.getAcl;
+import static org.alfresco.solr.AlfrescoSolrUtils.getAclChangeSet;
+import static org.alfresco.solr.AlfrescoSolrUtils.getAclReaders;
+import static org.alfresco.solr.AlfrescoSolrUtils.getNode;
+import static org.alfresco.solr.AlfrescoSolrUtils.getNodeMetaData;
+import static org.alfresco.solr.AlfrescoSolrUtils.getTransaction;
+import static org.alfresco.solr.AlfrescoSolrUtils.indexAclChangeSet;
+import static org.alfresco.solr.tracker.DocRouterFactory.SHARD_KEY_KEY;
+
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Properties;
+import java.util.Random;
+
 import com.carrotsearch.randomizedtesting.RandomizedContext;
+
 import org.alfresco.model.ContentModel;
 import org.alfresco.repo.index.shard.ShardMethodEnum;
 import org.alfresco.solr.AbstractAlfrescoDistributedTest;
@@ -38,26 +58,6 @@ import org.junit.AfterClass;
 import org.junit.BeforeClass;
 import org.junit.Test;
 
-import java.util.ArrayList;
-import java.util.Collections;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
-import java.util.Properties;
-import java.util.Random;
-
-import static java.util.Collections.singleton;
-import static java.util.Collections.singletonList;
-import static org.alfresco.repo.search.adaptor.lucene.QueryConstants.FIELD_DOC_TYPE;
-import static org.alfresco.solr.AlfrescoSolrUtils.getAcl;
-import static org.alfresco.solr.AlfrescoSolrUtils.getAclChangeSet;
-import static org.alfresco.solr.AlfrescoSolrUtils.getAclReaders;
-import static org.alfresco.solr.AlfrescoSolrUtils.getNode;
-import static org.alfresco.solr.AlfrescoSolrUtils.getNodeMetaData;
-import static org.alfresco.solr.AlfrescoSolrUtils.getTransaction;
-import static org.alfresco.solr.AlfrescoSolrUtils.indexAclChangeSet;
-import static org.alfresco.solr.AlfrescoSolrUtils.list;
-
 /**
  * Test Routes based on a text property field.
  *
@@ -182,7 +182,7 @@ public class DistributedPropertyBasedAlfrescoSolrTrackerTest extends AbstractAlf
         Properties prop = new Properties();
         prop.put("shard.method", ShardMethodEnum.PROPERTY.toString());
         prop.put("shard.regex", "^[A-Za-z0-9._%+-]+@([A-Za-z0-9.-]+\\.[A-Za-z]{2,6})$");
-        prop.put("shard.key", ContentModel.PROP_EMAIL.toString());
+        prop.put(SHARD_KEY_KEY, ContentModel.PROP_EMAIL.toString());
         return prop;
     }
 }