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

[ SEARCH-2110 ] Shard additional parameters are always sent, regardless shard method

See merge request search_discovery/insightengine!397

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

@@ -40,6 +40,7 @@ import org.alfresco.service.cmr.dictionary.PropertyDefinition;
 import org.alfresco.service.namespace.QName;
 import org.alfresco.solr.AlfrescoCoreAdminHandler;
 import org.alfresco.solr.AlfrescoSolrDataModel;
+import org.alfresco.solr.BoundedDeque;
 import org.alfresco.solr.InformationServer;
 import org.alfresco.solr.NodeReport;
 import org.alfresco.solr.TrackerState;
@@ -52,8 +53,8 @@ import java.util.Properties;
 
 /**
  * Superclass for all components which are able to inform Alfresco about the hosting node state.
- * This has been introduced in SEARCH-1752 for splitting the dual responsibility of the {@link MetadataTracker}.
- * As consequence of that, this class contains all the members needed for obtaining a valid
+ * This has been introduced in SEARCH-1752 for splitting the dual responsibility of the {@link org.alfresco.solr.tracker.MetadataTracker}.
+ * As consequence of that, this class contains only the members needed for obtaining a valid
  * {@link org.alfresco.repo.index.shard.ShardState} that can be periodically communicated to Alfresco.
  *
  * @author Andrea Gazzarini
@@ -176,15 +177,15 @@ public abstract class CoreStatePublisher extends AbstractTracker
      * The {@link ShardState} is primarily used in two places:
      *
      * <ul>
-     *     <li>Transaction tracking: (see {@link MetadataTracker#trackTransactions()}): for pulling/tracking transactions from Alfresco</li>
+     *     <li>Transaction tracking: (see {@link MetadataTracker#getSomeTransactions(BoundedDeque, Long, long, int, long}): for pulling/tracking transactions from Alfresco</li>
      *     <li>
-     *         DynamicSharding: when the {@link MetadataTracker} is running on a slave instance it doesn't actually act
-     *         as a tracker, it calls Alfresco to register the state of the node (the shard) without pulling any transactions.
-     *         As consequence of that, Alfresco will be aware about the shard which will be included in subsequent queries.
+     *         DynamicSharding: the {@link MetadataTracker} is not running on a slave instances; in those cases a special
+     *         "tracker" ({@link SlaveCoreStatePublisher}) will be in charge to send the correspondin shard state to Alfresco.
      *     </li>
      * </ul>
      *
      * @return the {@link ShardState} instance which stores the current state of the hosting shard.
+     * @see SlaveCoreStatePublisher
      */
     ShardState getShardState()
     {
@@ -198,10 +199,11 @@ public abstract class CoreStatePublisher extends AbstractTracker
 
         HashMap<String, String> propertyBag = new HashMap<>();
         propertyBag.put("coreName", coreName);
+
         HashMap<String, String> extendedPropertyBag = new HashMap<>(propertyBag);
         updateShardProperty();
 
-        shardProperty.ifPresent(p -> extendedPropertyBag.putAll(docRouter.getProperties(p)));
+        extendedPropertyBag.putAll(docRouter.getProperties(shardProperty));
 
         return ShardStateBuilder.shardState()
                 .withMaster(isMaster)

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

@@ -19,6 +19,7 @@
 package org.alfresco.solr.tracker;
 
 import java.util.Map;
+import java.util.Optional;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicLong;
 
@@ -108,9 +109,8 @@ public class DBIDRangeRouter implements DocRouter
     }
 
     @Override
-    public Map<String, String> getProperties(QName shardProperty)
+    public Map<String, String> getProperties(Optional<QName> shardProperty)
     {
         return Map.of(DocRouterFactory.SHARD_RANGE_KEY, startRange + "-" + expandableRange);
     }
-    
 }

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

@@ -26,10 +26,12 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import java.util.Calendar;
-import java.util.Collections;
 import java.util.Date;
 import java.util.GregorianCalendar;
 import java.util.Map;
+import java.util.Optional;
+
+import static java.util.Collections.emptyMap;
 
 /**
  * The date-based sharding assigns dates sequentially through shards based on the month.
@@ -121,14 +123,13 @@ public class DateMonthRouter implements DocRouter
     }
     
     @Override
-    public Map<String, String> getProperties(QName shardProperty)
+    public Map<String, String> getProperties(Optional<QName> shardProperty)
     {
-        
-        return (shardProperty == null ? 
-                Collections.emptyMap() : 
-                Map.of(DocRouterFactory.SHARD_KEY_KEY, shardProperty.getPrefixString(),
-                       DocRouterFactory.SHARD_DATE_GROUPING_KEY, String.valueOf(grouping)));
-        
+        return shardProperty
+                .map(QName::getPrefixString)
+                .map(prefix -> Map.of(
+                            DocRouterFactory.SHARD_KEY_KEY, prefix,
+                            DocRouterFactory.SHARD_DATE_GROUPING_KEY, String.valueOf(grouping)))
+                .orElse(emptyMap());
     }
-    
 }

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

@@ -22,6 +22,7 @@ import org.alfresco.solr.client.Node;
 
 import java.util.Collections;
 import java.util.Map;
+import java.util.Optional;
 
 import org.alfresco.service.namespace.QName;
 import org.alfresco.solr.client.Acl;
@@ -65,13 +66,13 @@ public interface DocRouter
     Boolean routeNode(int shardCount, int shardInstance, Node node);
     
     /**
-     * Get additional properties to "shardProperty" depending on the Shard Method
-     * @param shardProperty custom property used to configure the Router
+     * Get additional properties to "shardProperty" depending on the Shard Method.
+     *
+     * @param shardProperty custom property used to configure the Router. Note not all routers need that.
      * @return pair of key, value
      */
-    default public Map<String, String> getProperties(QName shardProperty) {
+    default Map<String, String> getProperties(Optional<QName> shardProperty) {
         return Collections.emptyMap();
     }
-    
 }
 

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

@@ -9,6 +9,7 @@ import java.util.Objects;
 import static java.util.Optional.ofNullable;
 
 import java.util.Map;
+import java.util.Optional;
 
 /**
  * A composable {@link DocRouter} which consists of
@@ -47,9 +48,8 @@ public class DocRouterWithFallback implements DocRouter
     }
     
     @Override
-    public Map<String, String> getProperties(QName shardProperty)
+    public Map<String, String> getProperties(Optional<QName> shardProperty)
     {
         return primaryStrategy.getProperties(shardProperty);
     }
-
 }

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

@@ -18,13 +18,15 @@
  */
 package org.alfresco.solr.tracker;
 
-import java.util.Collections;
 import java.util.Map;
+import java.util.Optional;
 
 import org.alfresco.service.namespace.QName;
 import org.alfresco.solr.client.Acl;
 import org.alfresco.solr.client.Node;
 
+import static java.util.Collections.emptyMap;
+
 /**
  * Routes a document only if the shardInstance matches the provided shardId.
  * The access control information is duplicated in each shard.
@@ -78,11 +80,11 @@ public class ExplicitShardIdWithDynamicPropertyRouter extends ComposableDocRoute
     }
     
     @Override
-    public Map<String, String> getProperties(QName shardProperty)
+    public Map<String, String> getProperties(Optional<QName> shardProperty)
     {
-        return (shardProperty == null ? 
-                Collections.emptyMap() : 
-                Map.of(DocRouterFactory.SHARD_KEY_KEY, shardProperty.getPrefixString()));
+        return shardProperty
+                .map(QName::getPrefixString)
+                .map(prefix -> Map.of(DocRouterFactory.SHARD_KEY_KEY, prefix))
+                .orElse(emptyMap());
     }
-
 }

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

@@ -26,11 +26,13 @@ import org.apache.solr.common.util.Hash;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import java.util.Collections;
 import java.util.Map;
+import java.util.Optional;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
+import static java.util.Collections.emptyMap;
+
 /**
  * Routes based on a text property field.
  * In this method, the value of some property is hashed and this hash is used to assign the node to a random shard.
@@ -126,12 +128,13 @@ public class PropertyRouter implements DocRouter
     }
     
     @Override
-    public Map<String, String> getProperties(QName shardProperty)
+    public Map<String, String> getProperties(Optional<QName> shardProperty)
     {
-        return (shardProperty == null ? 
-                Collections.emptyMap() : 
-                Map.of(DocRouterFactory.SHARD_KEY_KEY, shardProperty.getPrefixString(),
-                       DocRouterFactory.SHARD_REGEX_KEY, propertyRegEx));
+        return shardProperty
+                .map(QName::getPrefixString)
+                .map(prefix -> Map.of(
+                        DocRouterFactory.SHARD_KEY_KEY, prefix,
+                        DocRouterFactory.SHARD_REGEX_KEY, propertyRegEx))
+                .orElse(emptyMap());
     }
-    
 }