From 5a45b040bc6c167de1579bbb478716d68a71a1b4 Mon Sep 17 00:00:00 2001 From: Tom Page Date: Tue, 27 Aug 2019 12:39:54 +0100 Subject: [PATCH 1/6] Revert "Revert "Merge branch 'fix/SEARCH-1827_BugsRankedD' into 'master'"" This reverts commit 8046ac187fabbeb5f6933005480b804bb2e9a4a8. --- .../solr/AlfrescoCoreAdminHandler.java | 2 +- .../alfresco/solr/AlfrescoSolrDataModel.java | 3 +- .../alfresco/solr/SolrInformationServer.java | 19 ++++-- .../component/AsyncBuildSuggestComponent.java | 6 +- .../RewriteFacetParametersComponent.java | 2 +- .../solr/component/TempFileWarningLogger.java | 9 ++- .../solr/query/AbstractSolrCachingScorer.java | 2 +- .../solr/query/MimetypeGroupingCollector.java | 6 +- .../alfresco/solr/query/Solr4QueryParser.java | 65 +++++++++---------- .../solr/tracker/DateQuarterRouter.java | 9 ++- .../AlfrescoSolrClusteringComponent.java | 4 +- .../java/org/alfresco/solr/TrackerState.java | 23 +++++-- .../alfresco/solr/client/SOLRAPIClient.java | 21 +++--- .../alfresco/solr/tracker/TrackerStats.java | 21 +++--- 14 files changed, 107 insertions(+), 85 deletions(-) diff --git a/search-services/alfresco-search/src/main/java/org/alfresco/solr/AlfrescoCoreAdminHandler.java b/search-services/alfresco-search/src/main/java/org/alfresco/solr/AlfrescoCoreAdminHandler.java index 559ec6855..f38eba6b3 100644 --- a/search-services/alfresco-search/src/main/java/org/alfresco/solr/AlfrescoCoreAdminHandler.java +++ b/search-services/alfresco-search/src/main/java/org/alfresco/solr/AlfrescoCoreAdminHandler.java @@ -951,7 +951,7 @@ public class AlfrescoCoreAdminHandler extends CoreAdminHandler { if (maxNodeId >= midpoint) { - if(density >= 1) + if(density >= 1 || density == 0) { //This is fully dense shard. I'm not sure if it's possible to have more nodes on the shards //then the offset, but if it does happen don't expand. diff --git a/search-services/alfresco-search/src/main/java/org/alfresco/solr/AlfrescoSolrDataModel.java b/search-services/alfresco-search/src/main/java/org/alfresco/solr/AlfrescoSolrDataModel.java index f5a556a6e..c28d7ae8b 100644 --- a/search-services/alfresco-search/src/main/java/org/alfresco/solr/AlfrescoSolrDataModel.java +++ b/search-services/alfresco-search/src/main/java/org/alfresco/solr/AlfrescoSolrDataModel.java @@ -1195,8 +1195,7 @@ public class AlfrescoSolrDataModel implements QueryConstants public void removeModel(QName modelQName) { - // FIXME: this has no effect. The method should be changed (SEARCH-1482) - modelErrors.remove(modelQName); + modelErrors.remove(getM2Model(modelQName).getName()); dictionaryDAO.removeModel(modelQName); } diff --git a/search-services/alfresco-search/src/main/java/org/alfresco/solr/SolrInformationServer.java b/search-services/alfresco-search/src/main/java/org/alfresco/solr/SolrInformationServer.java index c4287b22a..838604cef 100644 --- a/search-services/alfresco-search/src/main/java/org/alfresco/solr/SolrInformationServer.java +++ b/search-services/alfresco-search/src/main/java/org/alfresco/solr/SolrInformationServer.java @@ -957,9 +957,10 @@ public class SolrInformationServer implements InformationServer SolrIndexSearcher solrIndexSearcher = refCounted.get(); coreSummary.add("Searcher", solrIndexSearcher.getStatistics()); Map infoRegistry = core.getInfoRegistry(); - for (String key : infoRegistry.keySet()) + for (Entry infos : infoRegistry.entrySet()) { - SolrInfoMBean infoMBean = infoRegistry.get(key); + SolrInfoMBean infoMBean = infos.getValue(); + String key = infos.getKey(); if (key.equals("/alfresco")) { // TODO Do we really need to fixStats in solr4? @@ -2117,8 +2118,9 @@ public class SolrInformationServer implements InformationServer static void addPropertiesToDoc(Map properties, boolean isContentIndexedForNode, SolrInputDocument newDoc, SolrInputDocument cachedDoc, boolean transformContentFlag) { - for (QName propertyQName : properties.keySet()) + for (Entry property : properties.entrySet()) { + QName propertyQName = property.getKey(); newDoc.addField(FIELD_PROPERTIES, propertyQName.toString()); newDoc.addField(FIELD_PROPERTIES, propertyQName.getPrefixString()); @@ -3412,10 +3414,15 @@ public class SolrInformationServer implements InformationServer SolrQueryRequest request, UpdateRequestProcessor processor, LinkedHashSet stack) throws AuthenticationException, IOException, JSONException { - if ((skipDescendantDocsForSpecificTypes && typesForSkippingDescendantDocs.contains(parentNodeMetaData.getType())) || - (skipDescendantDocsForSpecificAspects && shouldBeIgnoredByAnyAspect(parentNodeMetaData.getAspects()))) + + // skipDescendantDocsForSpecificAspects is initialised on a synchronised method, so access must be also synchronised + synchronized (this) { - return; + if ((skipDescendantDocsForSpecificTypes && typesForSkippingDescendantDocs.contains(parentNodeMetaData.getType())) || + (skipDescendantDocsForSpecificAspects && shouldBeIgnoredByAnyAspect(parentNodeMetaData.getAspects()))) + { + return; + } } Set childIds = new HashSet<>(); diff --git a/search-services/alfresco-search/src/main/java/org/alfresco/solr/component/AsyncBuildSuggestComponent.java b/search-services/alfresco-search/src/main/java/org/alfresco/solr/component/AsyncBuildSuggestComponent.java index efcb7d38d..1440ec5a5 100644 --- a/search-services/alfresco-search/src/main/java/org/alfresco/solr/component/AsyncBuildSuggestComponent.java +++ b/search-services/alfresco-search/src/main/java/org/alfresco/solr/component/AsyncBuildSuggestComponent.java @@ -41,6 +41,7 @@ import java.util.Iterator; import java.util.LinkedList; import java.util.List; import java.util.Map; +import java.util.Map.Entry; import java.util.Set; import java.util.concurrent.BlockingQueue; import java.util.concurrent.ConcurrentHashMap; @@ -472,8 +473,9 @@ public class AsyncBuildSuggestComponent extends SearchComponent implements SolrC @Override public long ramBytesUsed() { long sizeInBytes = 0; - for (String key : suggesters.keySet()) { - sizeInBytes += suggesters.get(key).get(ASYNC_CACHE_KEY).ramBytesUsed(); + for (Entry suggester : suggesters.entrySet()) + { + sizeInBytes += suggester.getValue().get(ASYNC_CACHE_KEY).ramBytesUsed(); } return sizeInBytes; } diff --git a/search-services/alfresco-search/src/main/java/org/alfresco/solr/component/RewriteFacetParametersComponent.java b/search-services/alfresco-search/src/main/java/org/alfresco/solr/component/RewriteFacetParametersComponent.java index f52fe77f0..33de8a9fb 100644 --- a/search-services/alfresco-search/src/main/java/org/alfresco/solr/component/RewriteFacetParametersComponent.java +++ b/search-services/alfresco-search/src/main/java/org/alfresco/solr/component/RewriteFacetParametersComponent.java @@ -91,7 +91,7 @@ public class RewriteFacetParametersComponent extends SearchComponent String rows = params.get("rows"); if(rows != null && !rows.isEmpty()) { - Integer row = new Integer(rows); + Integer row = Integer.valueOf(rows); // Avoid +1 in SOLR code which produces null:java.lang.NegativeArraySizeException at at org.apache.lucene.util.PriorityQueue.(PriorityQueue.java:56) if(row > 1000000) { diff --git a/search-services/alfresco-search/src/main/java/org/alfresco/solr/component/TempFileWarningLogger.java b/search-services/alfresco-search/src/main/java/org/alfresco/solr/component/TempFileWarningLogger.java index b2b8d07f3..6f885c3e6 100644 --- a/search-services/alfresco-search/src/main/java/org/alfresco/solr/component/TempFileWarningLogger.java +++ b/search-services/alfresco-search/src/main/java/org/alfresco/solr/component/TempFileWarningLogger.java @@ -19,7 +19,6 @@ package org.alfresco.solr.component; import java.io.IOException; -import java.nio.file.DirectoryStream; import java.nio.file.Files; import java.nio.file.Path; @@ -53,9 +52,9 @@ public class TempFileWarningLogger log.debug("Looking for temp files matching " + glob + " in directory " + dir); } - try(DirectoryStream stream = Files.newDirectoryStream(dir, glob)) + try { - for (Path file : stream) + for (Path file : Files.newDirectoryStream(dir, glob)) { if (log.isDebugEnabled()) { @@ -74,9 +73,9 @@ public class TempFileWarningLogger public void removeFiles() { - try(DirectoryStream stream = Files.newDirectoryStream(dir, glob)) + try { - for (Path file : stream) + for (Path file : Files.newDirectoryStream(dir, glob)) { file.toFile().delete(); } diff --git a/search-services/alfresco-search/src/main/java/org/alfresco/solr/query/AbstractSolrCachingScorer.java b/search-services/alfresco-search/src/main/java/org/alfresco/solr/query/AbstractSolrCachingScorer.java index b008175f4..157e5d1c9 100644 --- a/search-services/alfresco-search/src/main/java/org/alfresco/solr/query/AbstractSolrCachingScorer.java +++ b/search-services/alfresco-search/src/main/java/org/alfresco/solr/query/AbstractSolrCachingScorer.java @@ -47,7 +47,7 @@ public abstract class AbstractSolrCachingScorer extends Scorer static { for(int i = 0; i < cache.length; i++) - cache[i] = new Long(i); + cache[i] = Long.valueOf(i); } } diff --git a/search-services/alfresco-search/src/main/java/org/alfresco/solr/query/MimetypeGroupingCollector.java b/search-services/alfresco-search/src/main/java/org/alfresco/solr/query/MimetypeGroupingCollector.java index 863d79a83..7253121fb 100644 --- a/search-services/alfresco-search/src/main/java/org/alfresco/solr/query/MimetypeGroupingCollector.java +++ b/search-services/alfresco-search/src/main/java/org/alfresco/solr/query/MimetypeGroupingCollector.java @@ -20,6 +20,7 @@ package org.alfresco.solr.query; import java.io.IOException; import java.util.HashMap; +import java.util.Map.Entry; import org.alfresco.solr.AlfrescoSolrDataModel; import org.alfresco.solr.AlfrescoSolrDataModel.FieldUse; @@ -111,10 +112,9 @@ public class MimetypeGroupingCollector extends DelegatingCollector rb.rsp.add("analytics", analytics); NamedList fieldCounts = new NamedList<>(); analytics.add("mimetype()", fieldCounts); - for(String key : counters.keySet()) + for (Entry counter : counters.entrySet()) { - Counter counter = counters.get(key); - fieldCounts.add(key, counter.get()); + fieldCounts.add(counter.getKey(), counter.getValue().get()); } if(this.delegate instanceof DelegatingCollector) { diff --git a/search-services/alfresco-search/src/main/java/org/alfresco/solr/query/Solr4QueryParser.java b/search-services/alfresco-search/src/main/java/org/alfresco/solr/query/Solr4QueryParser.java index 5b138c593..c7dc07382 100644 --- a/search-services/alfresco-search/src/main/java/org/alfresco/solr/query/Solr4QueryParser.java +++ b/search-services/alfresco-search/src/main/java/org/alfresco/solr/query/Solr4QueryParser.java @@ -3301,44 +3301,35 @@ public class Solr4QueryParser extends QueryParser implements QueryConstants protected String getToken(String field, String value, AnalysisMode analysisMode) throws ParseException { - try (TokenStream source = getAnalyzer().tokenStream(field, new StringReader(value))) - { - String tokenised = null; + + TokenStream source = getAnalyzer().tokenStream(field, new StringReader(value)); - while (source.incrementToken()) - { - CharTermAttribute cta = source.getAttribute(CharTermAttribute.class); - OffsetAttribute offsetAtt = source.getAttribute(OffsetAttribute.class); - TypeAttribute typeAtt = null; - if (source.hasAttribute(TypeAttribute.class)) - { - typeAtt = source.getAttribute(TypeAttribute.class); - } - PositionIncrementAttribute posIncAtt = null; - if (source.hasAttribute(PositionIncrementAttribute.class)) - { - posIncAtt = source.getAttribute(PositionIncrementAttribute.class); - } - PackedTokenAttributeImpl token = new PackedTokenAttributeImpl(); - token.setEmpty().copyBuffer(cta.buffer(), 0, cta.length()); - token.setOffset(offsetAtt.startOffset(), offsetAtt.endOffset()); - if (typeAtt != null) - { - token.setType(typeAtt.type()); - } - if (posIncAtt != null) - { - token.setPositionIncrement(posIncAtt.getPositionIncrement()); - } - - tokenised = token.toString(); - } - return tokenised; - } catch (IOException e) + CharTermAttribute cta = source.getAttribute(CharTermAttribute.class); + OffsetAttribute offsetAtt = source.getAttribute(OffsetAttribute.class); + TypeAttribute typeAtt = null; + if (source.hasAttribute(TypeAttribute.class)) { - throw new ParseException("IO" + e.getMessage()); + typeAtt = source.getAttribute(TypeAttribute.class); + } + PositionIncrementAttribute posIncAtt = null; + if (source.hasAttribute(PositionIncrementAttribute.class)) + { + posIncAtt = source.getAttribute(PositionIncrementAttribute.class); + } + PackedTokenAttributeImpl token = new PackedTokenAttributeImpl(); + token.setEmpty().copyBuffer(cta.buffer(), 0, cta.length()); + token.setOffset(offsetAtt.startOffset(), offsetAtt.endOffset()); + if (typeAtt != null) + { + token.setType(typeAtt.type()); + } + if (posIncAtt != null) + { + token.setPositionIncrement(posIncAtt.getPositionIncrement()); } + return token.toString(); + } @Override @@ -5472,11 +5463,13 @@ public class Solr4QueryParser extends QueryParser implements QueryConstants } protected BytesRef analyzeMultitermTerm(String field, String part, Analyzer analyzerIn) { + if (analyzerIn == null) analyzerIn = getAnalyzer(); - try (TokenStream source = analyzerIn.tokenStream(field, part)) { - source.reset(); + try + { + TokenStream source = analyzerIn.tokenStream(field, part); TermToBytesRefAttribute termAtt = source.getAttribute(TermToBytesRefAttribute.class); if (!source.incrementToken()) diff --git a/search-services/alfresco-search/src/main/java/org/alfresco/solr/tracker/DateQuarterRouter.java b/search-services/alfresco-search/src/main/java/org/alfresco/solr/tracker/DateQuarterRouter.java index 8a446c880..c3c32ad2e 100644 --- a/search-services/alfresco-search/src/main/java/org/alfresco/solr/tracker/DateQuarterRouter.java +++ b/search-services/alfresco-search/src/main/java/org/alfresco/solr/tracker/DateQuarterRouter.java @@ -55,6 +55,13 @@ public class DateQuarterRouter implements DocRouter calendar.setTime(date); int month = calendar.get(Calendar.MONTH); int year = calendar.get(Calendar.YEAR); - return Math.ceil(((year * 12) + (month+1)) / 3) % numShards == shardInstance; + + // Avoid using Math.ceil with Integer + int countMonths = ((year * 12) + (month+1)); + int grouping = 3; + int ceilGroupInstance = countMonths / grouping + ((countMonths % grouping == 0) ? 0 : 1); + + return ceilGroupInstance % numShards == shardInstance; + } } \ No newline at end of file diff --git a/search-services/alfresco-search/src/main/java/org/apache/solr/handler/component/AlfrescoSolrClusteringComponent.java b/search-services/alfresco-search/src/main/java/org/apache/solr/handler/component/AlfrescoSolrClusteringComponent.java index 62a3f47c3..417e03d42 100644 --- a/search-services/alfresco-search/src/main/java/org/apache/solr/handler/component/AlfrescoSolrClusteringComponent.java +++ b/search-services/alfresco-search/src/main/java/org/apache/solr/handler/component/AlfrescoSolrClusteringComponent.java @@ -280,7 +280,7 @@ public class AlfrescoSolrClusteringComponent extends SearchComponent implements list.add(doc); if (ids != null) { - ids.put(doc, new Integer(docid)); + ids.put(doc, Integer.valueOf(docid)); } } return list; @@ -356,7 +356,7 @@ public class AlfrescoSolrClusteringComponent extends SearchComponent implements /** * @return Expose for tests. */ - Map getSearchClusteringEngines() { + Map getSearchClusteringEnginesView() { return searchClusteringEnginesView; } diff --git a/search-services/alfresco-solrclient-lib/src/main/java/org/alfresco/solr/TrackerState.java b/search-services/alfresco-solrclient-lib/src/main/java/org/alfresco/solr/TrackerState.java index d585a0c0b..8efed9b1b 100644 --- a/search-services/alfresco-solrclient-lib/src/main/java/org/alfresco/solr/TrackerState.java +++ b/search-services/alfresco-solrclient-lib/src/main/java/org/alfresco/solr/TrackerState.java @@ -26,6 +26,8 @@ package org.alfresco.solr; +import java.util.concurrent.atomic.AtomicInteger; + import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -64,8 +66,21 @@ public class TrackerState private volatile boolean checkedLastAclTransactionTime = false; private volatile boolean checkedLastTransactionTime = false; - private volatile boolean check = false; - private volatile int trackerCycles; + private volatile boolean check = false; + // Handle Thread Safe operations + private volatile TrackerCyclesInteger trackerCycles; + class TrackerCyclesInteger + { + private AtomicInteger value = new AtomicInteger(0); + private void increase() + { + value.incrementAndGet(); + } + private int getValue() + { + return value.get(); + } + } private long timeToStopIndexing; private long lastGoodChangeSetCommitTimeInIndex; @@ -237,13 +252,13 @@ public class TrackerState public int getTrackerCycles() { - return this.trackerCycles; + return this.trackerCycles.getValue(); } public synchronized void incrementTrackerCycles() { log.debug("incrementTrackerCycles from :" + trackerCycles); - this.trackerCycles++; + this.trackerCycles.increase(); log.debug("incremented TrackerCycles to :" + trackerCycles); } diff --git a/search-services/alfresco-solrclient-lib/src/main/java/org/alfresco/solr/client/SOLRAPIClient.java b/search-services/alfresco-solrclient-lib/src/main/java/org/alfresco/solr/client/SOLRAPIClient.java index d1a19a51a..136d5ddde 100644 --- a/search-services/alfresco-solrclient-lib/src/main/java/org/alfresco/solr/client/SOLRAPIClient.java +++ b/search-services/alfresco-solrclient-lib/src/main/java/org/alfresco/solr/client/SOLRAPIClient.java @@ -38,6 +38,7 @@ import java.util.Iterator; import java.util.List; import java.util.Locale; import java.util.Map; +import java.util.Map.Entry; import java.util.Set; import org.alfresco.error.AlfrescoRuntimeException; @@ -741,7 +742,7 @@ public class SOLRAPIClient String localeStr = o.has("locale") && !o.isNull("locale") ? o.getString("locale") : null; Locale locale = (o.has("locale") && !o.isNull("locale") ? deserializer.deserializeValue(Locale.class, localeStr) : null); - Long size = o.has("size") && !o.isNull("size") ? o.getLong("size") : null; + long size = o.has("size") && !o.isNull("size") ? o.getLong("size") : 0; String encoding = o.has("encoding") && !o.isNull("encoding") ? o.getString("encoding") : null; String mimetype = o.has("mimetype") && !o.isNull("mimetype") ? o.getString("mimetype") : null; @@ -1247,17 +1248,13 @@ public class SOLRAPIClient this.namespaceDAO = namespaceDAO; // add all default converters to this converter - // TODO find a better way of doing this - Map, Map, Converter>> converters = DefaultTypeConverter.INSTANCE.getConverters(); - for(Class source : converters.keySet()) - { - Map, Converter> converters1 = converters.get(source); - for(Class dest : converters1.keySet()) - { - Converter converter = converters1.get(dest); - instance.addConverter(source, dest, converter); - } - } + for (Entry, Map, Converter>> source : DefaultTypeConverter.INSTANCE.getConverters().entrySet()) + { + for (Entry, Converter> dest : source.getValue().entrySet()) + { + instance.addConverter((Class) source.getKey(), (Class) dest.getKey(), dest.getValue()); + } + } // dates instance.addConverter(String.class, Date.class, new TypeConverter.Converter() diff --git a/search-services/alfresco-solrclient-lib/src/main/java/org/alfresco/solr/tracker/TrackerStats.java b/search-services/alfresco-solrclient-lib/src/main/java/org/alfresco/solr/tracker/TrackerStats.java index 6b5f925bf..93291cad9 100644 --- a/search-services/alfresco-solrclient-lib/src/main/java/org/alfresco/solr/tracker/TrackerStats.java +++ b/search-services/alfresco-solrclient-lib/src/main/java/org/alfresco/solr/tracker/TrackerStats.java @@ -29,9 +29,12 @@ import java.util.ArrayList; import java.util.Collections; import java.util.Date; import java.util.HashMap; -import java.util.List; -import java.util.concurrent.ConcurrentHashMap; - +import java.util.List; +import java.util.Map.Entry; +import java.util.concurrent.ConcurrentHashMap; + +import javax.annotation.concurrent.NotThreadSafe; + import org.alfresco.solr.InformationServerCollectionProvider; import org.alfresco.solr.adapters.ISimpleOrderedMap; import org.alfresco.util.Pair; @@ -283,11 +286,10 @@ public class TrackerStats map.add("StdDev", getStandardDeviation()); if (incdludeDetail) { - for (String key : copies.keySet()) - { - IncrementalStats value = copies.get(key); - map.add(key, value.getNamedList(includeHist, includeValues)); - } + for (Entry copy : copies.entrySet()) + { + map.add(copy.getKey(), copy.getValue().getNamedList(includeHist, includeValues)); + } } return map; @@ -382,6 +384,7 @@ public class TrackerStats } + @NotThreadSafe public static class IncrementalStats { Date start = new Date(); @@ -769,7 +772,7 @@ public class TrackerStats { IncrementalStats copy = new IncrementalStats(this.scale, this.buckets, this.server); copy.start = this.start; - copy.max = this.max; + copy.max = this.getMax(); copy.min = this.min; copy.moments[0] = this.moments[0]; copy.moments[1] = this.moments[1]; From eba5581355313960540ea0743322f6489d771f1a Mon Sep 17 00:00:00 2001 From: Angel Borroy Date: Wed, 28 Aug 2019 15:15:41 +0200 Subject: [PATCH 2/6] Remove class to use local AtomicInteger property. --- pom.xml | 9 +++ .../component/AsyncBuildSuggestComponent.java | 7 +- .../solr/component/TempFileWarningLogger.java | 16 +++- .../solr/query/AbstractSolrCachingScorer.java | 8 +- .../alfresco/solr/query/Solr4QueryParser.java | 75 +++++++++++-------- .../solr/tracker/DateQuarterRouter.java | 4 +- .../java/org/alfresco/solr/TrackerState.java | 19 +---- 7 files changed, 74 insertions(+), 64 deletions(-) diff --git a/pom.xml b/pom.xml index 3b2e0f8c0..d0f38aaa0 100644 --- a/pom.xml +++ b/pom.xml @@ -39,4 +39,13 @@ search-services insight-engine + + + + findbugs + annotations + 1.0.0 + provided + + diff --git a/search-services/alfresco-search/src/main/java/org/alfresco/solr/component/AsyncBuildSuggestComponent.java b/search-services/alfresco-search/src/main/java/org/alfresco/solr/component/AsyncBuildSuggestComponent.java index 1440ec5a5..184a2f161 100644 --- a/search-services/alfresco-search/src/main/java/org/alfresco/solr/component/AsyncBuildSuggestComponent.java +++ b/search-services/alfresco-search/src/main/java/org/alfresco/solr/component/AsyncBuildSuggestComponent.java @@ -472,12 +472,7 @@ public class AsyncBuildSuggestComponent extends SearchComponent implements SolrC @Override public long ramBytesUsed() { - long sizeInBytes = 0; - for (Entry suggester : suggesters.entrySet()) - { - sizeInBytes += suggester.getValue().get(ASYNC_CACHE_KEY).ramBytesUsed(); - } - return sizeInBytes; + return suggesters.values().stream().mapToLong(value -> value.get(ASYNC_CACHE_KEY).ramBytesUsed()).sum(); } private Set getSuggesters(SolrParams params) { diff --git a/search-services/alfresco-search/src/main/java/org/alfresco/solr/component/TempFileWarningLogger.java b/search-services/alfresco-search/src/main/java/org/alfresco/solr/component/TempFileWarningLogger.java index 6f885c3e6..357e167a2 100644 --- a/search-services/alfresco-search/src/main/java/org/alfresco/solr/component/TempFileWarningLogger.java +++ b/search-services/alfresco-search/src/main/java/org/alfresco/solr/component/TempFileWarningLogger.java @@ -19,12 +19,16 @@ package org.alfresco.solr.component; import java.io.IOException; + +import java.nio.file.DirectoryStream; import java.nio.file.Files; import java.nio.file.Path; import org.slf4j.Logger; import org.springframework.util.StringUtils; +import edu.umd.cs.findbugs.annotations.SuppressWarnings; + /** * Temp files may take up a lot of space, warn administrators * of their existence, giving them the chance to manage them. @@ -45,6 +49,8 @@ public class TempFileWarningLogger glob = prefix + ".{"+ StringUtils.arrayToCommaDelimitedString(extensions) + "}"; } + // Avoid FindBugs false positive (https://github.com/spotbugs/spotbugs/issues/756) + @SuppressWarnings("RCN_REDUNDANT_NULLCHECK_WOULD_HAVE_BEEN_A_NPE") public boolean checkFiles() { if (log.isDebugEnabled()) @@ -52,9 +58,9 @@ public class TempFileWarningLogger log.debug("Looking for temp files matching " + glob + " in directory " + dir); } - try + try(DirectoryStream stream = Files.newDirectoryStream(dir, glob)) { - for (Path file : Files.newDirectoryStream(dir, glob)) + for (Path file : stream) { if (log.isDebugEnabled()) { @@ -71,11 +77,13 @@ public class TempFileWarningLogger } } + // Avoid FindBugs false positive (https://github.com/spotbugs/spotbugs/issues/756) + @SuppressWarnings("RCN_REDUNDANT_NULLCHECK_WOULD_HAVE_BEEN_A_NPE") public void removeFiles() { - try + try(DirectoryStream stream = Files.newDirectoryStream(dir, glob)) { - for (Path file : Files.newDirectoryStream(dir, glob)) + for (Path file : stream) { file.toFile().delete(); } diff --git a/search-services/alfresco-search/src/main/java/org/alfresco/solr/query/AbstractSolrCachingScorer.java b/search-services/alfresco-search/src/main/java/org/alfresco/solr/query/AbstractSolrCachingScorer.java index 157e5d1c9..6102afd1b 100644 --- a/search-services/alfresco-search/src/main/java/org/alfresco/solr/query/AbstractSolrCachingScorer.java +++ b/search-services/alfresco-search/src/main/java/org/alfresco/solr/query/AbstractSolrCachingScorer.java @@ -19,6 +19,7 @@ package org.alfresco.solr.query; import java.io.IOException; +import java.util.stream.LongStream; import org.apache.lucene.index.LeafReaderContext; import org.apache.lucene.search.DocIdSetIterator; @@ -43,12 +44,7 @@ public abstract class AbstractSolrCachingScorer extends Scorer private LongCache(){} - static final Long cache[] = new Long[CACHE_SIZE]; - - static { - for(int i = 0; i < cache.length; i++) - cache[i] = Long.valueOf(i); - } + static final long cache[] = LongStream.range(0, CACHE_SIZE).toArray(); } protected static Long getLong(long l) { diff --git a/search-services/alfresco-search/src/main/java/org/alfresco/solr/query/Solr4QueryParser.java b/search-services/alfresco-search/src/main/java/org/alfresco/solr/query/Solr4QueryParser.java index c7dc07382..b27ec4299 100644 --- a/search-services/alfresco-search/src/main/java/org/alfresco/solr/query/Solr4QueryParser.java +++ b/search-services/alfresco-search/src/main/java/org/alfresco/solr/query/Solr4QueryParser.java @@ -144,6 +144,8 @@ import org.jaxen.saxpath.base.XPathReader; import org.json.JSONObject; import org.springframework.extensions.surf.util.I18NUtil; +import edu.umd.cs.findbugs.annotations.SuppressWarnings; + /** * @author Andy * @@ -3299,39 +3301,50 @@ public class Solr4QueryParser extends QueryParser implements QueryConstants namespacePrefixResolver, field.substring(1)); } + // Avoid FindBugs false positive (https://github.com/spotbugs/spotbugs/issues/756) + @SuppressWarnings("RCN_REDUNDANT_NULLCHECK_WOULD_HAVE_BEEN_A_NPE") protected String getToken(String field, String value, AnalysisMode analysisMode) throws ParseException { - - TokenStream source = getAnalyzer().tokenStream(field, new StringReader(value)); + try (TokenStream source = getAnalyzer().tokenStream(field, new StringReader(value))) + { + String tokenised = null; - CharTermAttribute cta = source.getAttribute(CharTermAttribute.class); - OffsetAttribute offsetAtt = source.getAttribute(OffsetAttribute.class); - TypeAttribute typeAtt = null; - if (source.hasAttribute(TypeAttribute.class)) + while (source.incrementToken()) + { + CharTermAttribute cta = source.getAttribute(CharTermAttribute.class); + OffsetAttribute offsetAtt = source.getAttribute(OffsetAttribute.class); + TypeAttribute typeAtt = null; + if (source.hasAttribute(TypeAttribute.class)) + { + typeAtt = source.getAttribute(TypeAttribute.class); + } + PositionIncrementAttribute posIncAtt = null; + if (source.hasAttribute(PositionIncrementAttribute.class)) + { + posIncAtt = source.getAttribute(PositionIncrementAttribute.class); + } + PackedTokenAttributeImpl token = new PackedTokenAttributeImpl(); + token.setEmpty().copyBuffer(cta.buffer(), 0, cta.length()); + token.setOffset(offsetAtt.startOffset(), offsetAtt.endOffset()); + if (typeAtt != null) + { + token.setType(typeAtt.type()); + } + if (posIncAtt != null) + { + token.setPositionIncrement(posIncAtt.getPositionIncrement()); + } + + tokenised = token.toString(); + } + return tokenised; + } catch (IOException e) { - typeAtt = source.getAttribute(TypeAttribute.class); - } - PositionIncrementAttribute posIncAtt = null; - if (source.hasAttribute(PositionIncrementAttribute.class)) - { - posIncAtt = source.getAttribute(PositionIncrementAttribute.class); - } - PackedTokenAttributeImpl token = new PackedTokenAttributeImpl(); - token.setEmpty().copyBuffer(cta.buffer(), 0, cta.length()); - token.setOffset(offsetAtt.startOffset(), offsetAtt.endOffset()); - if (typeAtt != null) - { - token.setType(typeAtt.type()); - } - if (posIncAtt != null) - { - token.setPositionIncrement(posIncAtt.getPositionIncrement()); + throw new ParseException("IO" + e.getMessage()); } - return token.toString(); - } - + @Override public Query getPrefixQuery(String field, String termStr) throws ParseException { @@ -5462,14 +5475,14 @@ public class Solr4QueryParser extends QueryParser implements QueryConstants return analyzeMultitermTerm(field, part, getAnalyzer()); } + // Avoid FindBugs false positive (https://github.com/spotbugs/spotbugs/issues/756) + @SuppressWarnings("RCN_REDUNDANT_NULLCHECK_WOULD_HAVE_BEEN_A_NPE") protected BytesRef analyzeMultitermTerm(String field, String part, Analyzer analyzerIn) { - if (analyzerIn == null) analyzerIn = getAnalyzer(); - try - { + try (TokenStream source = analyzerIn.tokenStream(field, part)) { + source.reset(); - TokenStream source = analyzerIn.tokenStream(field, part); TermToBytesRefAttribute termAtt = source.getAttribute(TermToBytesRefAttribute.class); if (!source.incrementToken()) @@ -5483,7 +5496,7 @@ public class Solr4QueryParser extends QueryParser implements QueryConstants throw new RuntimeException("Error analyzing multiTerm term: " + part, e); } } - + private boolean analyzeRangeTerms = true; protected Query newRangeQuery(String field, String part1, String part2, boolean startInclusive, boolean endInclusive) { diff --git a/search-services/alfresco-search/src/main/java/org/alfresco/solr/tracker/DateQuarterRouter.java b/search-services/alfresco-search/src/main/java/org/alfresco/solr/tracker/DateQuarterRouter.java index c3c32ad2e..8f57ed0ae 100644 --- a/search-services/alfresco-search/src/main/java/org/alfresco/solr/tracker/DateQuarterRouter.java +++ b/search-services/alfresco-search/src/main/java/org/alfresco/solr/tracker/DateQuarterRouter.java @@ -59,8 +59,8 @@ public class DateQuarterRouter implements DocRouter // Avoid using Math.ceil with Integer int countMonths = ((year * 12) + (month+1)); int grouping = 3; - int ceilGroupInstance = countMonths / grouping + ((countMonths % grouping == 0) ? 0 : 1); - + int ceilGroupInstance = (countMonths + grouping - 1) / grouping; + return ceilGroupInstance % numShards == shardInstance; } diff --git a/search-services/alfresco-solrclient-lib/src/main/java/org/alfresco/solr/TrackerState.java b/search-services/alfresco-solrclient-lib/src/main/java/org/alfresco/solr/TrackerState.java index 8efed9b1b..2859723d4 100644 --- a/search-services/alfresco-solrclient-lib/src/main/java/org/alfresco/solr/TrackerState.java +++ b/search-services/alfresco-solrclient-lib/src/main/java/org/alfresco/solr/TrackerState.java @@ -68,19 +68,8 @@ public class TrackerState private volatile boolean check = false; // Handle Thread Safe operations - private volatile TrackerCyclesInteger trackerCycles; - class TrackerCyclesInteger - { - private AtomicInteger value = new AtomicInteger(0); - private void increase() - { - value.incrementAndGet(); - } - private int getValue() - { - return value.get(); - } - } + private volatile AtomicInteger trackerCycles; + private long timeToStopIndexing; private long lastGoodChangeSetCommitTimeInIndex; @@ -252,13 +241,13 @@ public class TrackerState public int getTrackerCycles() { - return this.trackerCycles.getValue(); + return this.trackerCycles.get(); } public synchronized void incrementTrackerCycles() { log.debug("incrementTrackerCycles from :" + trackerCycles); - this.trackerCycles.increase(); + this.trackerCycles.incrementAndGet(); log.debug("incremented TrackerCycles to :" + trackerCycles); } From 4ef2befdcbad7cec91286a9bd2d1911dbd54fabf Mon Sep 17 00:00:00 2001 From: Angel Borroy Date: Wed, 28 Aug 2019 16:34:32 +0200 Subject: [PATCH 3/6] Fixes from review comments. --- .../src/main/java/org/alfresco/solr/SolrInformationServer.java | 2 +- .../src/main/java/org/alfresco/solr/TrackerState.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/search-services/alfresco-search/src/main/java/org/alfresco/solr/SolrInformationServer.java b/search-services/alfresco-search/src/main/java/org/alfresco/solr/SolrInformationServer.java index 838604cef..d4d81df1f 100644 --- a/search-services/alfresco-search/src/main/java/org/alfresco/solr/SolrInformationServer.java +++ b/search-services/alfresco-search/src/main/java/org/alfresco/solr/SolrInformationServer.java @@ -2124,7 +2124,7 @@ public class SolrInformationServer implements InformationServer newDoc.addField(FIELD_PROPERTIES, propertyQName.toString()); newDoc.addField(FIELD_PROPERTIES, propertyQName.getPrefixString()); - PropertyValue value = properties.get(propertyQName); + PropertyValue value = property.getValue(); if(value != null) { if (value instanceof StringPropertyValue) diff --git a/search-services/alfresco-solrclient-lib/src/main/java/org/alfresco/solr/TrackerState.java b/search-services/alfresco-solrclient-lib/src/main/java/org/alfresco/solr/TrackerState.java index 2859723d4..8c36b99bc 100644 --- a/search-services/alfresco-solrclient-lib/src/main/java/org/alfresco/solr/TrackerState.java +++ b/search-services/alfresco-solrclient-lib/src/main/java/org/alfresco/solr/TrackerState.java @@ -68,7 +68,7 @@ public class TrackerState private volatile boolean check = false; // Handle Thread Safe operations - private volatile AtomicInteger trackerCycles; + private volatile AtomicInteger trackerCycles = new AtomicInteger(0); private long timeToStopIndexing; From 6e70029ae14c804bc5909fb3449c4c9be5d5bda4 Mon Sep 17 00:00:00 2001 From: Angel Borroy Date: Thu, 29 Aug 2019 09:53:51 +0200 Subject: [PATCH 4/6] Add FindBugs license --- .../packaging/src/main/resources/licenses/notice.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/search-services/packaging/src/main/resources/licenses/notice.txt b/search-services/packaging/src/main/resources/licenses/notice.txt index 7c83242c4..f61825399 100644 --- a/search-services/packaging/src/main/resources/licenses/notice.txt +++ b/search-services/packaging/src/main/resources/licenses/notice.txt @@ -29,6 +29,9 @@ xpp3-1.1.3_8.jar http://www.extreme.indiana.edu/xgws/xsoap/xpp/ === JSON === json-20160212.jar http://code.google.com/p/json-simple/ +=== LGPL === +annotations-1.0.0.jar http://findbugs.sourceforge.net + === Apache 2.0 === xml-resolver-1.2.jar https://github.com/FasterXML/jackson From 2cb1bed475715387fbbf91a4e6b35bc210465b04 Mon Sep 17 00:00:00 2001 From: Angel Borroy Date: Thu, 29 Aug 2019 11:08:18 +0200 Subject: [PATCH 5/6] Remove FindBugs annotations from WEB-INF/lib, as it's only used to produce FindBugs reports with SonarQube --- search-services/packaging/pom.xml | 2 +- .../packaging/src/main/resources/licenses/notice.txt | 3 --- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/search-services/packaging/pom.xml b/search-services/packaging/pom.xml index d7ce0529a..dbf962489 100644 --- a/search-services/packaging/pom.xml +++ b/search-services/packaging/pom.xml @@ -124,7 +124,7 @@ ${project.version} libs ${project.build.directory}/solr-libs - **/jackson-dataformat-smile-*.jar,**/asm-3.3.1.jar,**/jackson-core-asl-*.jar,**/jackson-mapper-asl-*.jar,**/dom4j-1.6.1.jar + **/jackson-dataformat-smile-*.jar,**/asm-3.3.1.jar,**/jackson-core-asl-*.jar,**/jackson-mapper-asl-*.jar,**/dom4j-1.6.1.jar,**/annotations-1.0.0.jar diff --git a/search-services/packaging/src/main/resources/licenses/notice.txt b/search-services/packaging/src/main/resources/licenses/notice.txt index f61825399..7c83242c4 100644 --- a/search-services/packaging/src/main/resources/licenses/notice.txt +++ b/search-services/packaging/src/main/resources/licenses/notice.txt @@ -29,9 +29,6 @@ xpp3-1.1.3_8.jar http://www.extreme.indiana.edu/xgws/xsoap/xpp/ === JSON === json-20160212.jar http://code.google.com/p/json-simple/ -=== LGPL === -annotations-1.0.0.jar http://findbugs.sourceforge.net - === Apache 2.0 === xml-resolver-1.2.jar https://github.com/FasterXML/jackson From e9a3a0dd3553c4a3d87b5a46167c7894f75ec2e6 Mon Sep 17 00:00:00 2001 From: Angel Borroy Date: Mon, 2 Sep 2019 09:15:57 +0200 Subject: [PATCH 6/6] Clarify condition. --- .../main/java/org/alfresco/solr/AlfrescoCoreAdminHandler.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/search-services/alfresco-search/src/main/java/org/alfresco/solr/AlfrescoCoreAdminHandler.java b/search-services/alfresco-search/src/main/java/org/alfresco/solr/AlfrescoCoreAdminHandler.java index f38eba6b3..9255db2a0 100644 --- a/search-services/alfresco-search/src/main/java/org/alfresco/solr/AlfrescoCoreAdminHandler.java +++ b/search-services/alfresco-search/src/main/java/org/alfresco/solr/AlfrescoCoreAdminHandler.java @@ -953,8 +953,8 @@ public class AlfrescoCoreAdminHandler extends CoreAdminHandler { if(density >= 1 || density == 0) { - //This is fully dense shard. I'm not sure if it's possible to have more nodes on the shards - //then the offset, but if it does happen don't expand. + //This is fully dense shard or an empty shard. + // If it does happen, no expand is required. bestGuess=0; } else