Merge branch 'fix/SEARCH-1829' into 'master'

Fix/search 1829

See merge request search_discovery/insightengine!152

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

@@ -24,6 +24,7 @@ import java.util.HashMap;
 import java.util.HashSet;
 import java.util.LinkedHashSet;
 import java.util.List;
+import java.util.Optional;
 import java.util.Properties;
 import java.util.concurrent.ConcurrentLinkedQueue;
 
@@ -59,6 +60,7 @@ import org.slf4j.LoggerFactory;
 
 import static java.util.Optional.of;
 
+import static java.util.Optional.ofNullable;
 import static org.alfresco.solr.tracker.DocRouterFactory.SHARD_KEY_KEY;
 
 /*
@@ -82,9 +84,9 @@ public class MetadataTracker extends AbstractTracker implements Tracker
     private ConcurrentLinkedQueue<String> queriesToReindex = new ConcurrentLinkedQueue<String>();
     private DocRouter docRouter;
     /** The string representation of the shard key. */
-    private String shardKey;
+    private Optional<String> shardKey;
     /** The property to use for determining the shard. */
-    private QName shardProperty;
+    private Optional<QName> shardProperty = Optional.empty();
 
     public MetadataTracker(Properties p, SOLRAPIClient client, String coreName,
                 InformationServer informationServer)
@@ -92,8 +94,8 @@ 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(SHARD_KEY_KEY);
+        shardKey = ofNullable(p.getProperty(SHARD_KEY_KEY));
-        updateShardProperty();
+        firstUpdateShardProperty();
         docRouter = DocRouterFactory.getRouter(p, ShardMethodEnum.getShardMethod(shardMethod));
         nodeBatchSize = Integer.parseInt(p.getProperty("alfresco.nodeBatchSize", "10"));
         threadHandler = new ThreadHandler(p, coreName, "MetadataTracker");
@@ -104,12 +106,35 @@ public class MetadataTracker extends AbstractTracker implements Tracker
      */
     private void updateShardProperty()
     {
-        if(shardProperty == null && shardKey != null)
+        shardKey.ifPresent(shardKeyName -> {
-        {
+            Optional<QName> updatedShardProperty = getShardProperty(shardKeyName);
-            shardProperty = getShardProperty(shardKey);
+            if (!shardProperty.equals(updatedShardProperty))
-        }
+            {
+                if (updatedShardProperty.isEmpty())
+                {
+                    log.warn("The model defining " + shardKeyName + " property has been disabled");
+                }
+                else
+                {
+                    log.info("New " + SHARD_KEY_KEY + " property found for " + shardKeyName);
+                }
+            }
+            shardProperty = updatedShardProperty;
+        });
     }
 
+    private void firstUpdateShardProperty()
+    {
+        shardKey.ifPresent( shardKeyName -> {
+            updateShardProperty();
+            if (shardProperty.isEmpty())
+            {
+                log.warn("Sharding property " + SHARD_KEY_KEY + " was set to " + shardKeyName + ", but no such property was found.");
+            }
+        });
+    }
+
+
     MetadataTracker()
     {
         super(Tracker.Type.MetaData);
@@ -174,7 +199,6 @@ public class MetadataTracker extends AbstractTracker implements Tracker
             * will pull its data from a "tracking" Solr node using Solr's master/slave replication, rather then tracking the repository.
             *
             */
-            
             ShardState shardstate = getShardState();
             client.getTransactions(0L, null, 0L, null, 0, shardstate);
             return;
@@ -240,7 +264,8 @@ public class MetadataTracker extends AbstractTracker implements Tracker
         propertyBag.put("coreName", coreName);
         HashMap<String, String> extendedPropertyBag = new HashMap<>(propertyBag);
         updateShardProperty();
-        extendedPropertyBag.putAll(docRouter.getProperties(shardProperty));
+
+        shardProperty.ifPresent(p -> extendedPropertyBag.putAll(docRouter.getProperties(p)));
 
         return ShardStateBuilder.shardState()
                 .withMaster(isMaster)
@@ -382,7 +407,9 @@ public class MetadataTracker extends AbstractTracker implements Tracker
                     gnp.setStoreProtocol(storeRef.getProtocol());
                     gnp.setStoreIdentifier(storeRef.getIdentifier());
                     updateShardProperty();
-                    gnp.setShardProperty(shardProperty);
+
+                    shardProperty.ifPresent(p -> gnp.setShardProperty(p));
+
                     gnp.setCoreName(coreName);
 
                     List<Node> nodes = client.getNodes(gnp, (int) info.getUpdates());
@@ -478,7 +505,7 @@ public class MetadataTracker extends AbstractTracker implements Tracker
                     gnp.setStoreProtocol(storeRef.getProtocol());
                     gnp.setStoreIdentifier(storeRef.getIdentifier());
                     gnp.setCoreName(coreName);
-                    List<Node> nodes = client.getNodes(gnp, (int) info.getUpdates());
+                    List<Node> nodes =   client.getNodes(gnp, (int) info.getUpdates());
                     for (Node node : nodes)
                     {
                         docCount++;
@@ -903,7 +930,8 @@ public class MetadataTracker extends AbstractTracker implements Tracker
         gnp.setStoreProtocol(storeRef.getProtocol());
         gnp.setStoreIdentifier(storeRef.getIdentifier());
         updateShardProperty();
-        gnp.setShardProperty(shardProperty);
+        shardProperty.ifPresent(p -> gnp.setShardProperty(p));
+
         gnp.setCoreName(coreName);
         List<Node> nodes = client.getNodes(gnp, Integer.MAX_VALUE);
         
@@ -1203,7 +1231,17 @@ public class MetadataTracker extends AbstractTracker implements Tracker
         this.queriesToReindex.offer(query);
     }
 
-    public static QName getShardProperty(String field)
+
+    /**
+     * Given the field name, returns the name of the property definition.
+     * If the property definition is not found, Empty optional is returned.
+     *
+     * @param field
+     *
+     * @return the name of the associated property definition if present, Optional.Empty() otherwise
+     *
+     */
+    public static Optional<QName> getShardProperty(String field)
     {
         if (StringUtils.isBlank(field))
         {
@@ -1218,8 +1256,8 @@ public class MetadataTracker extends AbstractTracker implements Tracker
                 field);
         if (propertyDef == null)
         {
-            throw new IllegalStateException("Sharding property " + SHARD_KEY_KEY + " was set to " + field + ", but no such property was found.");
+            return Optional.empty();
         }
-        return propertyDef.getName();
+        return of(propertyDef.getName());
     }
 }

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

@@ -20,6 +20,7 @@ package org.alfresco.solr.tracker;
 
 import org.alfresco.model.ContentModel;
 import org.alfresco.service.cmr.repository.datatype.DefaultTypeConverter;
+import org.alfresco.service.namespace.QName;
 import org.alfresco.solr.AbstractAlfrescoDistributedTest;
 import org.alfresco.solr.AlfrescoSolrDataModel;
 import org.alfresco.solr.client.Acl;
@@ -43,6 +44,7 @@ import java.util.Calendar;
 import java.util.Date;
 import java.util.GregorianCalendar;
 import java.util.List;
+import java.util.Optional;
 
 import static java.util.Collections.singletonList;
 import static java.util.stream.IntStream.range;
@@ -120,8 +122,11 @@ public abstract class DistributedDateAbstractSolrTrackerTest extends AbstractAlf
         indexTransaction(bigTxn, nodes, nodeMetaDatas);
         waitForDocCount(new TermQuery(new Term("content@s___t@{http://www.alfresco.org/model/content/1.0}content", "world")), numNodes, 100000);
 
+        Optional<QName> shardProperty = MetadataTracker.getShardProperty("created");
+        assertTrue("'created' field is expected to be found in data model", shardProperty.isPresent());
+
         List<AlfrescoSolrDataModel.FieldInstance> fieldInstanceList =
-                AlfrescoSolrDataModel.getInstance().getIndexedFieldNamesForProperty(MetadataTracker.getShardProperty("created")).getFields();
+                AlfrescoSolrDataModel.getInstance().getIndexedFieldNamesForProperty(shardProperty.get()).getFields();
 
         AlfrescoSolrDataModel.FieldInstance fieldInstance = fieldInstanceList.get(0);
         String fieldName = fieldInstance.getField();

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

@@ -32,6 +32,7 @@ import static org.alfresco.solr.AlfrescoSolrUtils.indexAclChangeSet;
 import org.alfresco.model.ContentModel;
 import org.alfresco.repo.index.shard.ShardMethodEnum;
 import org.alfresco.service.cmr.repository.datatype.DefaultTypeConverter;
+import org.alfresco.service.namespace.QName;
 import org.alfresco.solr.AbstractAlfrescoDistributedTest;
 import org.alfresco.solr.AlfrescoSolrDataModel;
 import org.alfresco.solr.SolrInformationServer;
@@ -57,6 +58,7 @@ import java.util.Calendar;
 import java.util.Date;
 import java.util.GregorianCalendar;
 import java.util.List;
+import java.util.Optional;
 import java.util.Properties;
 import java.util.TimeZone;
 
@@ -139,7 +141,10 @@ public class DistributedDateMonthAlfrescoSolrTrackerTest extends AbstractAlfresc
         waitForDocCount(new TermQuery(new Term("content@s___t@{http://www.alfresco.org/model/content/1.0}content", "world")), numNodes, 100000);
         waitForDocCountAllCores(new TermQuery(new Term(FIELD_DOC_TYPE, SolrInformationServer.DOC_TYPE_ACL)), numAcls, 100000);
 
-        List<AlfrescoSolrDataModel.FieldInstance> fieldInstanceList = AlfrescoSolrDataModel.getInstance().getIndexedFieldNamesForProperty(MetadataTracker.getShardProperty("created")).getFields();
+        Optional<QName> shardProperty = MetadataTracker.getShardProperty("created");
+        assertTrue(shardProperty.isPresent());
+
+        List<AlfrescoSolrDataModel.FieldInstance> fieldInstanceList = AlfrescoSolrDataModel.getInstance().getIndexedFieldNamesForProperty(shardProperty.get()).getFields();
         AlfrescoSolrDataModel.FieldInstance fieldInstance = fieldInstanceList.get(0);
         String fieldName = fieldInstance.getField();