From 719436a3859b70e47caae4259e77e08916bbe728 Mon Sep 17 00:00:00 2001 From: Simon Brandhof Date: Wed, 13 Jul 2016 17:44:45 +0200 Subject: [PATCH] Fix Quality flaws --- .../java/org/sonar/server/es/BaseIndexer.java | 29 ++++++-------- .../java/org/sonar/server/es/BulkIndexer.java | 4 +- .../issue/index/IssueAuthorizationDao.java | 2 +- .../issue/index/IssueAuthorizationDoc.java | 2 +- .../index/IssueAuthorizationIndexer.java | 14 +++---- .../sonar/server/issue/index/IssueDoc.java | 2 +- .../sonar/server/issue/index/IssueIndex.java | 38 +++++++------------ .../server/issue/index/IssueIndexer.java | 19 +++------- .../issue/index/IssueResultSetIterator.java | 2 +- 9 files changed, 41 insertions(+), 71 deletions(-) diff --git a/server/sonar-server/src/main/java/org/sonar/server/es/BaseIndexer.java b/server/sonar-server/src/main/java/org/sonar/server/es/BaseIndexer.java index 9978f7d4536..43c9593ef0e 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/es/BaseIndexer.java +++ b/server/sonar-server/src/main/java/org/sonar/server/es/BaseIndexer.java @@ -57,23 +57,20 @@ public abstract class BaseIndexer implements Startable { this.dateFieldName = dateFieldName; this.esClient = client; this.executor = new ThreadPoolExecutor(0, 1, - threadKeepAliveSeconds, TimeUnit.SECONDS, new LinkedBlockingQueue()); + threadKeepAliveSeconds, TimeUnit.SECONDS, new LinkedBlockingQueue<>()); } public void index(final IndexerTask task) { if (enabled) { final long requestedAt = System.currentTimeMillis(); - Future submit = executor.submit(new Runnable() { - @Override - public void run() { - if (lastUpdatedAt == -1L) { - lastUpdatedAt = esClient.getMaxFieldValue(indexName, typeName, dateFieldName); - } - if (requestedAt > lastUpdatedAt) { - long l = task.index(lastUpdatedAt); - // l can be 0 if no documents were indexed - lastUpdatedAt = Math.max(l, lastUpdatedAt); - } + Future submit = executor.submit(() -> { + if (lastUpdatedAt == -1L) { + lastUpdatedAt = esClient.getMaxFieldValue(indexName, typeName, dateFieldName); + } + if (requestedAt > lastUpdatedAt) { + long l = task.index(lastUpdatedAt); + // l can be 0 if no documents were indexed + lastUpdatedAt = Math.max(l, lastUpdatedAt); } }); try { @@ -85,12 +82,7 @@ public abstract class BaseIndexer implements Startable { } public void index() { - index(new IndexerTask() { - @Override - public long index(long lastUpdatedAt) { - return doIndex(lastUpdatedAt); - } - }); + index(lastUpdatedAtParam -> doIndex(lastUpdatedAtParam)); } protected abstract long doIndex(long lastUpdatedAt); @@ -110,6 +102,7 @@ public abstract class BaseIndexer implements Startable { executor.shutdown(); } + @FunctionalInterface public interface IndexerTask { long index(long lastUpdatedAt); } diff --git a/server/sonar-server/src/main/java/org/sonar/server/es/BulkIndexer.java b/server/sonar-server/src/main/java/org/sonar/server/es/BulkIndexer.java index 86209815835..003ef866c86 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/es/BulkIndexer.java +++ b/server/sonar-server/src/main/java/org/sonar/server/es/BulkIndexer.java @@ -164,7 +164,7 @@ public class BulkIndexer implements Startable { DeleteRequestBuilder deleteRequestBuilder = client.prepareDelete(hit.index(), hit.type(), hit.getId()); SearchHitField routing = hit.field("_routing"); if (routing != null) { - deleteRequestBuilder.setRouting((String) routing.getValue()); + deleteRequestBuilder.setRouting(routing.getValue()); } add(deleteRequestBuilder.request()); } @@ -235,7 +235,7 @@ public class BulkIndexer implements Startable { private class BulkResponseActionListener implements ActionListener { private final BulkRequestBuilder req; - public BulkResponseActionListener(BulkRequestBuilder req) { + BulkResponseActionListener(BulkRequestBuilder req) { this.req = req; } diff --git a/server/sonar-server/src/main/java/org/sonar/server/issue/index/IssueAuthorizationDao.java b/server/sonar-server/src/main/java/org/sonar/server/issue/index/IssueAuthorizationDao.java index 96d06a875b1..fb82b3d86f5 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/issue/index/IssueAuthorizationDao.java +++ b/server/sonar-server/src/main/java/org/sonar/server/issue/index/IssueAuthorizationDao.java @@ -166,7 +166,7 @@ public class IssueAuthorizationDao { } } - private PreparedStatement createStatement(DbClient dbClient, DbSession session, long afterDate) throws SQLException { + private static PreparedStatement createStatement(DbClient dbClient, DbSession session, long afterDate) throws SQLException { String sql; if (afterDate > 0L) { sql = StringUtils.replace(SQL_TEMPLATE, "{dateCondition}", " AND projects.authorization_updated_at>? "); diff --git a/server/sonar-server/src/main/java/org/sonar/server/issue/index/IssueAuthorizationDoc.java b/server/sonar-server/src/main/java/org/sonar/server/issue/index/IssueAuthorizationDoc.java index 0ec8da3788a..e1c33c81c04 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/issue/index/IssueAuthorizationDoc.java +++ b/server/sonar-server/src/main/java/org/sonar/server/issue/index/IssueAuthorizationDoc.java @@ -25,7 +25,7 @@ import org.sonar.server.es.BaseDoc; public class IssueAuthorizationDoc extends BaseDoc { public IssueAuthorizationDoc() { - super(Maps.newHashMap()); + super(Maps.newHashMap()); } @Override diff --git a/server/sonar-server/src/main/java/org/sonar/server/issue/index/IssueAuthorizationIndexer.java b/server/sonar-server/src/main/java/org/sonar/server/issue/index/IssueAuthorizationIndexer.java index 676942313f3..71568f2e578 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/issue/index/IssueAuthorizationIndexer.java +++ b/server/sonar-server/src/main/java/org/sonar/server/issue/index/IssueAuthorizationIndexer.java @@ -51,18 +51,14 @@ public class IssueAuthorizationIndexer extends BaseIndexer { @Override protected long doIndex(long lastUpdatedAt) { - // warning - do not enable large mode, else disabling of replicas - // will impact the type "issue" which is much bigger than issueAuthorization - final BulkIndexer bulk = new BulkIndexer(esClient, IssueIndexDefinition.INDEX); + try (DbSession dbSession = dbClient.openSession(false)) { + // warning - do not enable large mode, else disabling of replicas + // will impact the type "issue" which is much bigger than issueAuthorization + BulkIndexer bulk = new BulkIndexer(esClient, IssueIndexDefinition.INDEX); - DbSession dbSession = dbClient.openSession(false); - try { IssueAuthorizationDao dao = new IssueAuthorizationDao(); Collection authorizations = dao.selectAfterDate(dbClient, dbSession, lastUpdatedAt); return doIndex(bulk, authorizations); - - } finally { - dbSession.close(); } } @@ -91,7 +87,7 @@ public class IssueAuthorizationIndexer extends BaseIndexer { .get(); } - private ActionRequest newUpdateRequest(IssueAuthorizationDao.Dto dto) { + private static ActionRequest newUpdateRequest(IssueAuthorizationDao.Dto dto) { Map doc = ImmutableMap.of( IssueIndexDefinition.FIELD_AUTHORIZATION_PROJECT_UUID, dto.getProjectUuid(), IssueIndexDefinition.FIELD_AUTHORIZATION_GROUPS, dto.getGroups(), diff --git a/server/sonar-server/src/main/java/org/sonar/server/issue/index/IssueDoc.java b/server/sonar-server/src/main/java/org/sonar/server/issue/index/IssueDoc.java index 8984c4e86cb..53484ff3e3f 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/issue/index/IssueDoc.java +++ b/server/sonar-server/src/main/java/org/sonar/server/issue/index/IssueDoc.java @@ -44,7 +44,7 @@ public class IssueDoc extends BaseDoc implements Issue { } public IssueDoc() { - super(Maps.newHashMap()); + super(Maps.newHashMap()); } @Override diff --git a/server/sonar-server/src/main/java/org/sonar/server/issue/index/IssueIndex.java b/server/sonar-server/src/main/java/org/sonar/server/issue/index/IssueIndex.java index c5ef3def811..937b6c47122 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/issue/index/IssueIndex.java +++ b/server/sonar-server/src/main/java/org/sonar/server/issue/index/IssueIndex.java @@ -21,18 +21,18 @@ package org.sonar.server.issue.index; import com.google.common.base.Function; import com.google.common.base.Preconditions; -import com.google.common.collect.Collections2; import com.google.common.collect.ImmutableList; import com.google.common.collect.Lists; import com.google.common.collect.Maps; import com.google.common.collect.Sets; -import java.util.Arrays; import java.util.Collection; +import java.util.Collections; import java.util.Date; import java.util.HashMap; import java.util.Iterator; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Optional; import java.util.Set; import java.util.SortedSet; @@ -66,6 +66,7 @@ import org.sonar.api.resources.Scopes; import org.sonar.api.utils.DateUtils; import org.sonar.api.utils.System2; import org.sonar.core.util.NonNullInputFunction; +import org.sonar.core.util.stream.Collectors; import org.sonar.db.component.ComponentDto; import org.sonar.server.es.BaseIndex; import org.sonar.server.es.EsClient; @@ -315,7 +316,7 @@ public class IssueIndex extends BaseIndex { } @CheckForNull - private QueryBuilder createViewFilter(Collection viewUuids) { + private static QueryBuilder createViewFilter(Collection viewUuids) { if (viewUuids.isEmpty()) { return null; } @@ -368,7 +369,7 @@ public class IssueIndex extends BaseIndex { } } - private void validateCreationDateBounds(Date createdBefore, Date createdAfter) { + private void validateCreationDateBounds(@Nullable Date createdBefore, @Nullable Date createdAfter) { Preconditions.checkArgument(createdAfter == null || createdAfter.before(new Date(system.now())), "Start bound cannot be in the future"); Preconditions.checkArgument(createdAfter == null || createdBefore == null || createdAfter.before(createdBefore), @@ -413,7 +414,7 @@ public class IssueIndex extends BaseIndex { } addAssignedToMeFacetIfNeeded(esSearch, options, query, filters, esQuery); if (options.getFacets().contains(CREATED_AT)) { - getCreatedAtFacet(query, filters, esQuery).ifPresent(a -> esSearch.addAggregation(a)); + getCreatedAtFacet(query, filters, esQuery).ifPresent(esSearch::addAggregation); } } @@ -495,11 +496,7 @@ public class IssueIndex extends BaseIndex { .setTypes(IssueIndexDefinition.TYPE_ISSUE) .setSize(0); BoolQueryBuilder esFilter = boolQuery(); - for (QueryBuilder filter : filters.values()) { - if (filter != null) { - esFilter.must(filter); - } - } + filters.values().stream().filter(Objects::nonNull).forEach(esFilter::must); if (esFilter.hasClauses()) { esRequest.setQuery(QueryBuilders.filteredQuery(esQuery, esFilter)); } else { @@ -509,14 +506,13 @@ public class IssueIndex extends BaseIndex { Min minValue = esRequest.get().getAggregations().get(facetNameAndField); Double actualValue = minValue.getValue(); - System.out.println("min=" + actualValue); if (actualValue.isInfinite()) { return Optional.empty(); } return Optional.of(actualValue.longValue()); } - private AggregationBuilder createAssigneesFacet(IssueQuery query, Map filters, QueryBuilder queryBuilder) { + private static AggregationBuilder createAssigneesFacet(IssueQuery query, Map filters, QueryBuilder queryBuilder) { String fieldName = IssueIndexDefinition.FIELD_ISSUE_ASSIGNEE; String facetName = ASSIGNEES; @@ -545,12 +541,10 @@ public class IssueIndex extends BaseIndex { } private static Collection escapeValuesForFacetInclusion(@Nullable Collection values) { - return values == null ? Arrays.asList() : Collections2.transform(values, new Function() { - @Override - public String apply(String input) { - return Pattern.quote(input); - } - }); + if (values == null) { + return Collections.emptyList(); + } + return values.stream().map(Pattern::quote).collect(Collectors.toList(values.size())); } private void addAssignedToMeFacetIfNeeded(SearchRequestBuilder builder, SearchOptions options, IssueQuery query, Map filters, QueryBuilder queryBuilder) { @@ -602,12 +596,8 @@ public class IssueIndex extends BaseIndex { } @CheckForNull - private QueryBuilder createTermsFilter(String field, Collection values) { - if (!values.isEmpty()) { - return termsQuery(field, values); - } else { - return null; - } + private static QueryBuilder createTermsFilter(String field, Collection values) { + return values.isEmpty() ? null : termsQuery(field, values); } public List listTags(IssueQuery query, @Nullable String textQuery, int maxNumberOfTags) { diff --git a/server/sonar-server/src/main/java/org/sonar/server/issue/index/IssueIndexer.java b/server/sonar-server/src/main/java/org/sonar/server/issue/index/IssueIndexer.java index d0e821c4f11..53fbae1bf02 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/issue/index/IssueIndexer.java +++ b/server/sonar-server/src/main/java/org/sonar/server/issue/index/IssueIndexer.java @@ -61,13 +61,8 @@ public class IssueIndexer extends BaseIndexer { doIndex(createBulkIndexer(true), 0L, null); } - public void index(final String projectUuid) { - super.index(new IndexerTask() { - @Override - public long index(long lastUpdatedAt) { - return doIndex(createBulkIndexer(false), lastUpdatedAt, projectUuid); - } - }); + public void index(String projectUuid) { + super.index(lastUpdatedAt -> doIndex(createBulkIndexer(false), lastUpdatedAt, projectUuid)); } /** @@ -78,15 +73,11 @@ public class IssueIndexer extends BaseIndexer { } private long doIndex(BulkIndexer bulk, long lastUpdatedAt, @Nullable String projectUuid) { - DbSession dbSession = dbClient.openSession(false); - long maxDate; - try { + try (DbSession dbSession = dbClient.openSession(false)) { IssueResultSetIterator rowIt = IssueResultSetIterator.create(dbClient, dbSession, lastUpdatedAt, projectUuid); - maxDate = doIndex(bulk, rowIt); + long maxDate = doIndex(bulk, rowIt); rowIt.close(); return maxDate; - } finally { - dbSession.close(); } } @@ -144,7 +135,7 @@ public class IssueIndexer extends BaseIndexer { return bulk; } - private IndexRequest newIndexRequest(IssueDoc issue) { + private static IndexRequest newIndexRequest(IssueDoc issue) { String projectUuid = issue.projectUuid(); return new IndexRequest(INDEX, TYPE_ISSUE, issue.key()) diff --git a/server/sonar-server/src/main/java/org/sonar/server/issue/index/IssueResultSetIterator.java b/server/sonar-server/src/main/java/org/sonar/server/issue/index/IssueResultSetIterator.java index 823647b60a2..597494c8fc9 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/issue/index/IssueResultSetIterator.java +++ b/server/sonar-server/src/main/java/org/sonar/server/issue/index/IssueResultSetIterator.java @@ -151,7 +151,7 @@ class IssueResultSetIterator extends ResultSetIterator { @Override protected IssueDoc read(ResultSet rs) throws SQLException { - IssueDoc doc = new IssueDoc(Maps.newHashMapWithExpectedSize(30)); + IssueDoc doc = new IssueDoc(Maps.newHashMapWithExpectedSize(30)); String key = rs.getString(1); String projectUuid = rs.getString(2); -- 2.39.5