From ca11f3783cf4c3c4df13f4829031c0495ccb5bce Mon Sep 17 00:00:00 2001 From: eliaporciani Date: Tue, 10 Sep 2019 10:12:53 +0200 Subject: [PATCH] [SEARCH-1829] core review --- .../solr/tracker/MetadataTracker.java | 64 ++++++++++--------- ...istributedDateAbstractSolrTrackerTest.java | 9 ++- ...butedDateMonthAlfrescoSolrTrackerTest.java | 9 ++- 3 files changed, 49 insertions(+), 33 deletions(-) diff --git a/search-services/alfresco-search/src/main/java/org/alfresco/solr/tracker/MetadataTracker.java b/search-services/alfresco-search/src/main/java/org/alfresco/solr/tracker/MetadataTracker.java index 7922ae675..58778d865 100644 --- a/search-services/alfresco-search/src/main/java/org/alfresco/solr/tracker/MetadataTracker.java +++ b/search-services/alfresco-search/src/main/java/org/alfresco/solr/tracker/MetadataTracker.java @@ -19,7 +19,13 @@ package org.alfresco.solr.tracker; import java.io.IOException; -import java.util.*; +import java.util.ArrayList; +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; import org.alfresco.error.AlfrescoRuntimeException; @@ -52,10 +58,9 @@ import org.json.JSONException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import javax.swing.text.html.Option; - import static java.util.Optional.of; +import static java.util.Optional.ofNullable; import static org.alfresco.solr.tracker.DocRouterFactory.SHARD_KEY_KEY; /* @@ -79,7 +84,7 @@ public class MetadataTracker extends AbstractTracker implements Tracker private ConcurrentLinkedQueue queriesToReindex = new ConcurrentLinkedQueue(); private DocRouter docRouter; /** The string representation of the shard key. */ - private String shardKey; + private Optional shardKey; /** The property to use for determining the shard. */ private Optional shardProperty = Optional.empty(); @@ -89,7 +94,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(SHARD_KEY_KEY); + shardKey = ofNullable(p.getProperty(SHARD_KEY_KEY)); firstUpdateShardProperty(); docRouter = DocRouterFactory.getRouter(p, ShardMethodEnum.getShardMethod(shardMethod)); nodeBatchSize = Integer.parseInt(p.getProperty("alfresco.nodeBatchSize", "10")); @@ -98,42 +103,38 @@ public class MetadataTracker extends AbstractTracker implements Tracker /** * Set the shard property using the shard key. - * The property has to be update ad each iteration because the model could be deactivated or changes. */ private void updateShardProperty() { - if (shardKey != null) - { - Optional updatedShardProperty = getShardProperty(shardKey); + shardKey.ifPresent(shardKeyName -> { + Optional updatedShardProperty = getShardProperty(shardKeyName); if (!shardProperty.equals(updatedShardProperty)) { if (updatedShardProperty.isEmpty()) { - log.warn("The model defining " + shardKey + " property has been disabled"); + log.warn("The model defining " + shardKeyName + " property has been disabled"); } else { - log.info("New SHARD_KEY_KEY property found for " + shardKey); + log.info("New " + SHARD_KEY_KEY + " property found for " + shardKeyName); } } shardProperty = updatedShardProperty; - } + }); } private void firstUpdateShardProperty() { - if (shardKey != null) - { + shardKey.ifPresent( shardKeyName -> { updateShardProperty(); if (shardProperty.isEmpty()) { - log.warn("Sharding property " + SHARD_KEY_KEY + " was set to " + shardKey + ", but no such property was found."); + log.warn("Sharding property " + SHARD_KEY_KEY + " was set to " + shardKeyName + ", but no such property was found."); } - } + }); } - MetadataTracker() { super(Tracker.Type.MetaData); @@ -264,10 +265,7 @@ public class MetadataTracker extends AbstractTracker implements Tracker HashMap extendedPropertyBag = new HashMap<>(propertyBag); updateShardProperty(); - if (shardProperty.isPresent()) - { - extendedPropertyBag.putAll(docRouter.getProperties(shardProperty.get())); - } + shardProperty.ifPresent(p -> extendedPropertyBag.putAll(docRouter.getProperties(p))); return ShardStateBuilder.shardState() .withMaster(isMaster) @@ -410,10 +408,8 @@ public class MetadataTracker extends AbstractTracker implements Tracker gnp.setStoreIdentifier(storeRef.getIdentifier()); updateShardProperty(); - if (shardProperty.isPresent()) - { - gnp.setShardProperty(shardProperty.get()); - } + shardProperty.ifPresent(p -> gnp.setShardProperty(p)); + gnp.setCoreName(coreName); List nodes = client.getNodes(gnp, (int) info.getUpdates()); @@ -509,7 +505,7 @@ public class MetadataTracker extends AbstractTracker implements Tracker gnp.setStoreProtocol(storeRef.getProtocol()); gnp.setStoreIdentifier(storeRef.getIdentifier()); gnp.setCoreName(coreName); - List nodes = client.getNodes(gnp, (int) info.getUpdates()); + List nodes = client.getNodes(gnp, (int) info.getUpdates()); for (Node node : nodes) { docCount++; @@ -934,10 +930,8 @@ public class MetadataTracker extends AbstractTracker implements Tracker gnp.setStoreProtocol(storeRef.getProtocol()); gnp.setStoreIdentifier(storeRef.getIdentifier()); updateShardProperty(); - if (shardProperty.isPresent()) - { - gnp.setShardProperty(shardProperty.get()); - } + shardProperty.ifPresent(p -> gnp.setShardProperty(p)); + gnp.setCoreName(coreName); List nodes = client.getNodes(gnp, Integer.MAX_VALUE); @@ -1237,6 +1231,16 @@ public class MetadataTracker extends AbstractTracker implements Tracker this.queriesToReindex.offer(query); } + + /** + * 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 getShardProperty(String field) { if (StringUtils.isBlank(field)) diff --git a/search-services/alfresco-search/src/test/java/org/alfresco/solr/tracker/DistributedDateAbstractSolrTrackerTest.java b/search-services/alfresco-search/src/test/java/org/alfresco/solr/tracker/DistributedDateAbstractSolrTrackerTest.java index d91a8d5c4..6aacd0282 100644 --- a/search-services/alfresco-search/src/test/java/org/alfresco/solr/tracker/DistributedDateAbstractSolrTrackerTest.java +++ b/search-services/alfresco-search/src/test/java/org/alfresco/solr/tracker/DistributedDateAbstractSolrTrackerTest.java @@ -39,7 +39,12 @@ import org.apache.solr.client.solrj.SolrQuery; import org.junit.Test; import java.text.SimpleDateFormat; -import java.util.*; +import java.util.ArrayList; +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; @@ -118,7 +123,7 @@ public abstract class DistributedDateAbstractSolrTrackerTest extends AbstractAlf waitForDocCount(new TermQuery(new Term("content@s___t@{http://www.alfresco.org/model/content/1.0}content", "world")), numNodes, 100000); Optional shardProperty = MetadataTracker.getShardProperty("created"); - assertTrue(shardProperty.isPresent()); + assertTrue("'created' field is expected to be found in data model", shardProperty.isPresent()); List fieldInstanceList = AlfrescoSolrDataModel.getInstance().getIndexedFieldNamesForProperty(shardProperty.get()).getFields(); diff --git a/search-services/alfresco-search/src/test/java/org/alfresco/solr/tracker/DistributedDateMonthAlfrescoSolrTrackerTest.java b/search-services/alfresco-search/src/test/java/org/alfresco/solr/tracker/DistributedDateMonthAlfrescoSolrTrackerTest.java index f11a5833c..e0a569b4a 100644 --- a/search-services/alfresco-search/src/test/java/org/alfresco/solr/tracker/DistributedDateMonthAlfrescoSolrTrackerTest.java +++ b/search-services/alfresco-search/src/test/java/org/alfresco/solr/tracker/DistributedDateMonthAlfrescoSolrTrackerTest.java @@ -53,7 +53,14 @@ import org.junit.BeforeClass; import org.junit.Test; import java.text.SimpleDateFormat; -import java.util.*; +import java.util.ArrayList; +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; @SolrTestCaseJ4.SuppressSSL public class DistributedDateMonthAlfrescoSolrTrackerTest extends AbstractAlfrescoDistributedTest