Skip to content

Commit

Permalink
SONAR-8798 fix quality flaws
Browse files Browse the repository at this point in the history
  • Loading branch information
@bartfastiel
Daniel Schwarz authored and bartfastiel committed Aug 9, 2017
1 parent 21faf01 commit fbbb1a0
Show file tree
Hide file tree
Showing 15 changed files with 121 additions and 158 deletions.
2 changes: 1 addition & 1 deletion ...r-process-monitor/src/main/java/org/sonar/application/process/AbstractProcessMonitor.java
View file
Expand Up @@ -30,7 +30,7 @@

abstract class AbstractProcessMonitor implements ProcessMonitor {

private static final Logger LOG = LoggerFactory.getLogger(EsProcessMonitor.class);
private static final Logger LOG = LoggerFactory.getLogger(AbstractProcessMonitor.class);
private static final int EXPECTED_EXIT_VALUE = 0;

protected final Process process;
Expand Down
8 changes: 0 additions & 8 deletions ...sonar-process-monitor/src/main/java/org/sonar/application/process/CommandFactoryImpl.java
View file
Expand Up @@ -85,14 +85,6 @@ public EsCommand createEsCommand(AppSettings settings) {
settingsMap.forEach((key, value) -> res.addEsOption("-E" + key + "=" + value));

return res;

// FIXME quid of proxy settings and sonar.search.javaOpts/javaAdditionalOpts
// defaults of HTTPS are the same than HTTP defaults
// setSystemPropertyToDefaultIfNotSet(command, HTTPS_PROXY_HOST, HTTP_PROXY_HOST);
// setSystemPropertyToDefaultIfNotSet(command, HTTPS_PROXY_PORT, HTTP_PROXY_PORT);
// command
// .addJavaOptions(settings.getProps().nonNullValue(ProcessProperties.SEARCH_JAVA_OPTS))
// .addJavaOptions(settings.getProps().nonNullValue(ProcessProperties.SEARCH_JAVA_ADDITIONAL_OPTS));
}

private Properties buildLog4j2Properties(File logDir) {
Expand Down
5 changes: 2 additions & 3 deletions ...r/sonar-process-monitor/src/main/java/org/sonar/application/process/EsProcessMonitor.java
View file
Expand Up @@ -23,7 +23,6 @@
import io.netty.util.ThreadDeathWatcher;
import io.netty.util.concurrent.GlobalEventExecutor;
import java.net.InetAddress;
import java.net.MalformedURLException;
import java.net.UnknownHostException;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicBoolean;
Expand Down Expand Up @@ -62,7 +61,7 @@ public class EsProcessMonitor extends AbstractProcessMonitor {
private final EsCommand esCommand;
private AtomicReference<TransportClient> transportClient = new AtomicReference<>(null);

public EsProcessMonitor(Process process, ProcessId processId, EsCommand esCommand) throws MalformedURLException {
public EsProcessMonitor(Process process, ProcessId processId, EsCommand esCommand) {
super(process, processId);
this.esCommand = esCommand;
}
Expand Down Expand Up @@ -165,7 +164,7 @@ private TransportClient getTransportClient() {
}

private TransportClient buildTransportClient() {
org.elasticsearch.common.settings.Settings.Builder esSettings = org.elasticsearch.common.settings.Settings.builder();
Settings.Builder esSettings = Settings.builder();

// mandatory property defined by bootstrap process
esSettings.put("cluster.name", esCommand.getClusterName());
Expand Down
2 changes: 1 addition & 1 deletion server/sonar-process-monitor/src/main/java/org/sonar/application/process/EsSettings.java
View file
Expand Up @@ -173,7 +173,7 @@ private void configureMarvel(Map<String, String> builder) {
}
}

private void configureAction(Map<String, String> builder) {
private static void configureAction(Map<String, String> builder) {
builder.put("action.auto_create_index", String.valueOf(false));
}
}
18 changes: 11 additions & 7 deletions ...onar-process-monitor/src/main/java/org/sonar/application/process/ProcessLauncherImpl.java
View file
Expand Up @@ -81,7 +81,9 @@ public ProcessMonitor launch(EsCommand esCommand) {
try {
writeConfFiles(esCommand);
ProcessBuilder processBuilder = create(esCommand);
LOG.info("Launch process[{}]: {}", processId.getKey(), String.join(" ", processBuilder.command()));
if (LOG.isInfoEnabled()) {
LOG.info("Launch process[{}]: {}", processId.getKey(), String.join(" ", processBuilder.command()));
}

process = processBuilder.start();

Expand All @@ -108,7 +110,6 @@ private void writeConfFiles(EsCommand esCommand) {
IOUtils.copy(getClass().getResourceAsStream("jvm.options"), new FileOutputStream(new File(confDir, "jvm.options")));
esCommand.getLog4j2Properties().store(new FileOutputStream(new File(confDir, "log4j2.properties")), "log42 properties file for ES bundled in SonarQube");
} catch (IOException e) {
e.printStackTrace();
throw new IllegalStateException("Failed to write ES configuration files", e);
}
}
Expand All @@ -121,7 +122,9 @@ public ProcessMonitor launch(JavaCommand javaCommand) {
ProcessCommands commands = allProcessesCommands.createAfterClean(processId.getIpcIndex());

ProcessBuilder processBuilder = create(javaCommand);
LOG.info("Launch process[{}]: {}", processId.getKey(), String.join(" ", processBuilder.command()));
if (LOG.isInfoEnabled()) {
LOG.info("Launch process[{}]: {}", processId.getKey(), String.join(" ", processBuilder.command()));
}
process = processBuilder.start();
return new ProcessCommandsProcessMonitor(process, processId, commands);
} catch (Exception e) {
Expand All @@ -143,18 +146,20 @@ private ProcessBuilder create(EsCommand esCommand) {
return create(esCommand, commands);
}

private void writeJvmOptions(EsCommand esCommand) {
private static void writeJvmOptions(EsCommand esCommand) {
Path jvmOptionsFile = esCommand.getJvmOptionsFile();
String jvmOptions = esCommand.getJvmOptions()
.stream()
.map(s -> s.split(" (?=-)"))// FIXME this pattern does not allow escaping

// we do not expect the user to use parameters containing " -"
.map(s -> s.split(" (?=-)"))
.flatMap(Arrays::stream)
.collect(Collectors.joining("\n"));
String jvmOptionsContent = ELASTICSEARCH_JVM_OPTIONS_HEADER + jvmOptions;
try {
Files.write(jvmOptionsFile, jvmOptionsContent.getBytes(Charset.forName("UTF-8")));
} catch (IOException e) {
throw new RuntimeException("Cannot write Elasticsearch jvm options file", e);
throw new IllegalStateException("Cannot write Elasticsearch jvm options file", e);
}
}

Expand Down Expand Up @@ -199,7 +204,6 @@ private File buildPropertiesFile(JavaCommand javaCommand) {
props.putAll(javaCommand.getArguments());
props.setProperty(PROPERTY_PROCESS_KEY, javaCommand.getProcessId().getKey());
props.setProperty(PROPERTY_PROCESS_INDEX, Integer.toString(javaCommand.getProcessId().getIpcIndex()));
// FIXME is it the responsibility of child process to have this timeout (too) ?
props.setProperty(PROPERTY_TERMINATION_TIMEOUT, "60000");
props.setProperty(PROPERTY_SHARED_PATH, tempDir.getAbsolutePath());
try (OutputStream out = new FileOutputStream(propertiesFile)) {
Expand Down
3 changes: 1 addition & 2 deletions server/sonar-server/src/main/java/org/sonar/server/component/index/ComponentIndex.java
View file
Expand Up @@ -32,7 +32,6 @@
import org.elasticsearch.search.SearchHits;
import org.elasticsearch.search.aggregations.AggregationBuilders;
import org.elasticsearch.search.aggregations.bucket.filters.FiltersAggregationBuilder;
import org.elasticsearch.search.aggregations.bucket.filters.FiltersAggregator;
import org.elasticsearch.search.aggregations.bucket.filters.FiltersAggregator.KeyedFilter;
import org.elasticsearch.search.aggregations.bucket.filters.InternalFilters;
import org.elasticsearch.search.aggregations.bucket.filters.InternalFilters.InternalBucket;
Expand Down Expand Up @@ -156,6 +155,6 @@ private static ComponentHitsPerQualifier bucketToQualifier(InternalBucket bucket
SearchHits hitList = docs.getHits();
SearchHit[] hits = hitList.getHits();

return new ComponentHitsPerQualifier(bucket.getKey(), ComponentHit.fromSearchHits(hits), hitList.totalHits());
return new ComponentHitsPerQualifier(bucket.getKey(), ComponentHit.fromSearchHits(hits), hitList.getTotalHits());
}
}
4 changes: 2 additions & 2 deletions server/sonar-server/src/main/java/org/sonar/server/es/BulkIndexer.java
View file
Expand Up @@ -151,8 +151,8 @@ public void addDeletion(SearchRequestBuilder searchRequest) {
while (true) {
SearchHit[] hits = searchResponse.getHits().getHits();
for (SearchHit hit : hits) {
SearchHitField routing = hit.field("_routing");
DeleteRequestBuilder deleteRequestBuilder = client.prepareDelete(hit.index(), hit.type(), hit.getId());
SearchHitField routing = hit.getField("_routing");
DeleteRequestBuilder deleteRequestBuilder = client.prepareDelete(hit.getIndex(), hit.getType(), hit.getId());
if (routing != null) {
deleteRequestBuilder.setRouting(routing.getValue());
}
Expand Down
51 changes: 6 additions & 45 deletions server/sonar-server/src/main/java/org/sonar/server/es/EsUtils.java
View file
Expand Up @@ -65,7 +65,7 @@ public static <D extends BaseDoc> List<D> convertToDocs(SearchHits hits, Functio
return docs;
}

public static LinkedHashMap<String, Long> termsToMap(Terms terms) {
public static Map<String, Long> termsToMap(Terms terms) {
LinkedHashMap<String, Long> map = new LinkedHashMap<>();
List<? extends Terms.Bucket> buckets = terms.getBuckets();
for (Terms.Bucket bucket : buckets) {
Expand Down Expand Up @@ -115,58 +115,19 @@ public static String escapeSpecialRegexChars(String str) {
return SPECIAL_REGEX_CHARS.matcher(str).replaceAll("\\\\$0");
}

private static class DocScrollIterator<D extends BaseDoc> implements Iterator<D> {

private final EsClient esClient;
private final String scrollId;
private final Function<Map<String, Object>, D> docConverter;

private final Queue<SearchHit> hits = new ArrayDeque<>();

private DocScrollIterator(EsClient esClient, SearchResponse scrollResponse, Function<Map<String, Object>, D> docConverter) {
this.esClient = esClient;
this.scrollId = scrollResponse.getScrollId();
this.docConverter = docConverter;
Collections.addAll(hits, scrollResponse.getHits().getHits());
}

@Override
public boolean hasNext() {
if (hits.isEmpty()) {
SearchScrollRequestBuilder esRequest = esClient.prepareSearchScroll(scrollId)
.setScroll(TimeValue.timeValueMinutes(SCROLL_TIME_IN_MINUTES));
Collections.addAll(hits, esRequest.get().getHits().getHits());
}
return !hits.isEmpty();
}

@Override
public D next() {
if (!hasNext()) {
throw new NoSuchElementException();
}
return docConverter.apply(hits.poll().getSource());
}

@Override
public void remove() {
throw new UnsupportedOperationException("Cannot remove item when scrolling");
}
}

public static <ID> Iterator<ID> scrollIds(EsClient esClient, SearchResponse scrollResponse, Function<String, ID> idConverter) {
public static <I> Iterator<I> scrollIds(EsClient esClient, SearchResponse scrollResponse, Function<String, I> idConverter) {
return new IdScrollIterator<>(esClient, scrollResponse, idConverter);
}

private static class IdScrollIterator<ID> implements Iterator<ID> {
private static class IdScrollIterator<I> implements Iterator<I> {

private final EsClient esClient;
private final String scrollId;
private final Function<String, ID> idConverter;
private final Function<String, I> idConverter;

private final Queue<SearchHit> hits = new ArrayDeque<>();

private IdScrollIterator(EsClient esClient, SearchResponse scrollResponse, Function<String, ID> idConverter) {
private IdScrollIterator(EsClient esClient, SearchResponse scrollResponse, Function<String, I> idConverter) {
this.esClient = esClient;
this.scrollId = scrollResponse.getScrollId();
this.idConverter = idConverter;
Expand All @@ -184,7 +145,7 @@ public boolean hasNext() {
}

@Override
public ID next() {
public I next() {
if (!hasNext()) {
throw new NoSuchElementException();
}
Expand Down
102 changes: 55 additions & 47 deletions server/sonar-server/src/main/java/org/sonar/server/es/NewIndex.java
View file
Expand Up @@ -338,55 +338,63 @@ public StringFieldBuilder store() {
}

public NewIndexType build() {
Map<String, Object> hash = new TreeMap<>();
if (subFields.isEmpty()) {
hash.putAll(ImmutableMap.of(
"type", getFieldType(),
"index", disableSearch ? INDEX_NOT_SEARCHABLE : INDEX_SEARCHABLE,
"norms", valueOf(!disableNorms),
"store", valueOf(store)));
if (getFieldData()) {
hash.put(FIELD_FIELDDATA, FIELDDATA_ENABLED);
}
} else {
hash.put("type", getFieldType());

Map<String, Object> multiFields = new TreeMap<>(subFields);

if (termVectorWithPositionOffsets) {
multiFields.entrySet().forEach(entry -> {
Object subFieldMapping = entry.getValue();
if (subFieldMapping instanceof Map) {
entry.setValue(
addFieldToMapping(
(Map<String, String>) subFieldMapping,
FIELD_TERM_VECTOR, "with_positions_offsets"));
}
});
hash.put(FIELD_TERM_VECTOR, "with_positions_offsets");
}
if (getFieldData()) {
multiFields.entrySet().forEach(entry -> {
Object subFieldMapping = entry.getValue();
if (subFieldMapping instanceof Map) {
entry.setValue(
addFieldToMapping(
(Map<String, String>) subFieldMapping,
FIELD_FIELDDATA, FIELDDATA_ENABLED));
}
});
hash.put(FIELD_FIELDDATA, FIELDDATA_ENABLED);
}

multiFields.put(fieldName, ImmutableMap.of(
"type", getFieldType(),
"index", INDEX_SEARCHABLE,
"norms", "false",
"store", valueOf(store)));

hash.put("fields", multiFields);
return buildWithoutSubfields();
}
return buildWithSubfields();
}

private NewIndexType buildWithoutSubfields() {
Map<String, Object> hash = new TreeMap<>();
hash.putAll(ImmutableMap.of(
"type", getFieldType(),
INDEX, disableSearch ? INDEX_NOT_SEARCHABLE : INDEX_SEARCHABLE,
"norms", valueOf(!disableNorms),
"store", valueOf(store)));
if (getFieldData()) {
hash.put(FIELD_FIELDDATA, FIELDDATA_ENABLED);
}
return indexType.setProperty(fieldName, hash);
}

private NewIndexType buildWithSubfields() {
Map<String, Object> hash = new TreeMap<>();
hash.put("type", getFieldType());

Map<String, Object> multiFields = new TreeMap<>(subFields);

if (termVectorWithPositionOffsets) {
multiFields.entrySet().forEach(entry -> {
Object subFieldMapping = entry.getValue();
if (subFieldMapping instanceof Map) {
entry.setValue(
addFieldToMapping(
(Map<String, String>) subFieldMapping,
FIELD_TERM_VECTOR, "with_positions_offsets"));
}
});
hash.put(FIELD_TERM_VECTOR, "with_positions_offsets");
}
if (getFieldData()) {
multiFields.entrySet().forEach(entry -> {
Object subFieldMapping = entry.getValue();
if (subFieldMapping instanceof Map) {
entry.setValue(
addFieldToMapping(
(Map<String, String>) subFieldMapping,
FIELD_FIELDDATA, FIELDDATA_ENABLED));
}
});
hash.put(FIELD_FIELDDATA, FIELDDATA_ENABLED);
}

multiFields.put(fieldName, ImmutableMap.of(
"type", getFieldType(),
INDEX, INDEX_SEARCHABLE,
"norms", "false",
"store", valueOf(store)));
hash.put("fields", multiFields);

return indexType.setProperty(fieldName, hash);
}

Expand Down Expand Up @@ -463,7 +471,7 @@ private NestedFieldBuilder setProperty(String fieldName, Object value) {
public NestedFieldBuilder addKeywordField(String fieldName) {
return setProperty(fieldName, ImmutableMap.of(
"type", FIELD_TYPE_KEYWORD,
"index", INDEX_SEARCHABLE));
INDEX, INDEX_SEARCHABLE));
}

public NestedFieldBuilder addDoubleField(String fieldName) {
Expand Down
3 changes: 2 additions & 1 deletion server/sonar-server/src/main/java/org/sonar/server/es/StickyFacetBuilder.java
View file
Expand Up @@ -81,7 +81,8 @@ public AggregationBuilder buildStickyFacet(String fieldName, String facetName, O
* @param selected the terms, that the user already has selected
* @return the (global) aggregation, that can be added on top level of the elasticsearch request
*/
public AggregationBuilder buildStickyFacet(String fieldName, String facetName, Function<TermsAggregationBuilder, AggregationBuilder> additionalAggregationFilter, Object... selected) {
public AggregationBuilder buildStickyFacet(String fieldName, String facetName, Function<TermsAggregationBuilder, AggregationBuilder> additionalAggregationFilter,
Object... selected) {
return buildStickyFacet(fieldName, facetName, FACET_DEFAULT_SIZE, additionalAggregationFilter, selected);
}

Expand Down
18 changes: 6 additions & 12 deletions server/sonar-server/src/main/java/org/sonar/server/measure/index/ProjectMeasuresIndex.java
View file
Expand Up @@ -284,13 +284,13 @@ private Map<String, QueryBuilder> createFilters(ProjectMeasuresQuery query) {
filters.put("__authorization", authorizationTypeSupport.createQueryFilter());
Multimap<String, MetricCriterion> metricCriterionMultimap = ArrayListMultimap.create();
query.getMetricCriteria().forEach(metricCriterion -> metricCriterionMultimap.put(metricCriterion.getMetricKey(), metricCriterion));
metricCriterionMultimap.asMap().entrySet().forEach(entry -> {
metricCriterionMultimap.asMap().forEach((key, value) -> {
BoolQueryBuilder metricFilters = boolQuery();
entry.getValue()
value
.stream()
.map(ProjectMeasuresIndex::toQuery)
.forEach(metricFilters::must);
filters.put(entry.getKey(), metricFilters);
filters.put(key, metricFilters);
});

query.getQualityGateStatus()
Expand All @@ -308,18 +308,12 @@ private Map<String, QueryBuilder> createFilters(ProjectMeasuresQuery query) {
query.getTags()
.ifPresent(tags -> filters.put(FIELD_TAGS, termsQuery(FIELD_TAGS, tags)));

createTextQueryFilter(query).ifPresent(queryBuilder -> filters.put("textQuery", queryBuilder));
query.getQueryText()
.map(ProjectsTextSearchQueryFactory::createQuery)
.ifPresent(queryBuilder -> filters.put("textQuery", queryBuilder));
return filters;
}

private static Optional<QueryBuilder> createTextQueryFilter(ProjectMeasuresQuery query) {
Optional<String> queryText = query.getQueryText();
if (!queryText.isPresent()) {
return Optional.empty();
}
return Optional.of(ProjectsTextSearchQueryFactory.createQuery(queryText.get()));
}

private static QueryBuilder toQuery(MetricCriterion criterion) {
if (criterion.isNoData()) {
return boolQuery().mustNot(
Expand Down

0 comments on commit fbbb1a0

Please sign in to comment.