diff --git a/pom.xml b/pom.xml index 27047faa4..c305c9aeb 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/AlfrescoCoreAdminHandler.java b/search-services/alfresco-search/src/main/java/org/alfresco/solr/AlfrescoCoreAdminHandler.java index 559ec6855..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 @@ -951,10 +951,10 @@ 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. + //This is fully dense shard or an empty shard. + // If it does happen, no expand is required. bestGuess=0; } else 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..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 @@ -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,12 +2118,13 @@ 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()); - PropertyValue value = properties.get(propertyQName); + PropertyValue value = property.getValue(); if(value != null) { if (value instanceof StringPropertyValue) @@ -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..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 @@ -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; @@ -471,11 +472,7 @@ 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(); - } - 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/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..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,6 +19,7 @@ package org.alfresco.solr.component; import java.io.IOException; + import java.nio.file.DirectoryStream; import java.nio.file.Files; import java.nio.file.Path; @@ -26,6 +27,8 @@ 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. @@ -46,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()) @@ -72,6 +77,8 @@ 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(DirectoryStream stream = Files.newDirectoryStream(dir, glob)) 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..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] = new Long(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/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..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,6 +3301,8 @@ 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 { try (TokenStream source = getAnalyzer().tokenStream(field, new StringReader(value))) @@ -3340,7 +3344,7 @@ public class Solr4QueryParser extends QueryParser implements QueryConstants } } - + @Override public Query getPrefixQuery(String field, String termStr) throws ParseException { @@ -5471,6 +5475,8 @@ 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(); @@ -5490,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 8a446c880..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 @@ -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 - 1) / grouping; + + 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..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 @@ -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,10 @@ 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 AtomicInteger trackerCycles = new AtomicInteger(0); + private long timeToStopIndexing; private long lastGoodChangeSetCommitTimeInIndex; @@ -237,13 +241,13 @@ public class TrackerState public int getTrackerCycles() { - return this.trackerCycles; + return this.trackerCycles.get(); } public synchronized void incrementTrackerCycles() { log.debug("incrementTrackerCycles from :" + trackerCycles); - this.trackerCycles++; + this.trackerCycles.incrementAndGet(); 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]; diff --git a/search-services/packaging/pom.xml b/search-services/packaging/pom.xml index b618f5365..3417b6e2e 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