From f86f766b22cd9d0cecd1bc5b7a3037a07fc23b9d Mon Sep 17 00:00:00 2001 From: Simon Brandhof Date: Mon, 11 Apr 2016 15:54:35 +0200 Subject: [PATCH] SONAR-5476 Rule search by key should be case insensitive --- .../org/sonar/server/rule/index/RuleDoc.java | 12 ---------- .../sonar/server/rule/index/RuleIndex.java | 10 ++++---- .../rule/index/RuleIndexDefinition.java | 5 +--- .../sonar/server/rule/index/RuleIndexer.java | 13 +--------- .../rule/index/RuleResultSetIterator.java | 2 -- .../server/rule/index/RuleDocTesting.java | 4 ++-- .../server/rule/index/RuleIndexTest.java | 24 ++++++++++++++----- .../rule/index/RuleResultSetIteratorTest.java | 3 --- 8 files changed, 28 insertions(+), 45 deletions(-) diff --git a/server/sonar-server/src/main/java/org/sonar/server/rule/index/RuleDoc.java b/server/sonar-server/src/main/java/org/sonar/server/rule/index/RuleDoc.java index 93d33fc7ac8..a2908fb8e6f 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/rule/index/RuleDoc.java +++ b/server/sonar-server/src/main/java/org/sonar/server/rule/index/RuleDoc.java @@ -19,9 +19,7 @@ */ package org.sonar.server.rule.index; -import com.google.common.annotations.VisibleForTesting; import java.util.Collection; -import java.util.List; import java.util.Map; import javax.annotation.CheckForNull; import javax.annotation.Nullable; @@ -49,16 +47,6 @@ public class RuleDoc extends BaseDoc { return this; } - @VisibleForTesting - List keyAsList() { - return (List) getField(RuleIndexDefinition.FIELD_RULE_KEY_AS_LIST); - } - - public RuleDoc setKeyAsList(@Nullable List s) { - setField(RuleIndexDefinition.FIELD_RULE_KEY_AS_LIST, s); - return this; - } - @CheckForNull public String ruleKey() { return getNullableField(RuleIndexDefinition.FIELD_RULE_RULE_KEY); diff --git a/server/sonar-server/src/main/java/org/sonar/server/rule/index/RuleIndex.java b/server/sonar-server/src/main/java/org/sonar/server/rule/index/RuleIndex.java index 47ab72986d0..7e7fccfc38b 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/rule/index/RuleIndex.java +++ b/server/sonar-server/src/main/java/org/sonar/server/rule/index/RuleIndex.java @@ -66,6 +66,8 @@ import org.sonar.server.es.SearchIdResult; import org.sonar.server.es.SearchOptions; import org.sonar.server.search.StickyFacetBuilder; +import static org.elasticsearch.index.query.QueryBuilders.matchQuery; +import static org.elasticsearch.index.query.QueryBuilders.simpleQueryStringQuery; import static org.sonar.server.es.EsUtils.SCROLL_TIME_IN_MINUTES; import static org.sonar.server.es.EsUtils.scrollIds; import static org.sonar.server.rule.index.RuleIndexDefinition.FIELD_ACTIVE_RULE_INHERITANCE; @@ -77,7 +79,6 @@ import static org.sonar.server.rule.index.RuleIndexDefinition.FIELD_RULE_HTML_DE import static org.sonar.server.rule.index.RuleIndexDefinition.FIELD_RULE_INTERNAL_KEY; import static org.sonar.server.rule.index.RuleIndexDefinition.FIELD_RULE_IS_TEMPLATE; import static org.sonar.server.rule.index.RuleIndexDefinition.FIELD_RULE_KEY; -import static org.sonar.server.rule.index.RuleIndexDefinition.FIELD_RULE_KEY_AS_LIST; import static org.sonar.server.rule.index.RuleIndexDefinition.FIELD_RULE_LANGUAGE; import static org.sonar.server.rule.index.RuleIndexDefinition.FIELD_RULE_NAME; import static org.sonar.server.rule.index.RuleIndexDefinition.FIELD_RULE_REPOSITORY; @@ -177,15 +178,16 @@ public class RuleIndex extends BaseIndex { String queryString = query.getQueryText(); // Human readable type of querying - qb.should(QueryBuilders.simpleQueryStringQuery(query.getQueryText()) + qb.should(simpleQueryStringQuery(query.getQueryText()) .field(FIELD_RULE_NAME + "." + SEARCH_WORDS_SUFFIX, 20f) .field(FIELD_RULE_HTML_DESCRIPTION + "." + SEARCH_WORDS_SUFFIX, 3f) .defaultOperator(SimpleQueryStringBuilder.Operator.AND) ).boost(20f); // Match and partial Match queries - qb.should(termQuery(FIELD_RULE_KEY, queryString, 15f)); - qb.should(termQuery(FIELD_RULE_KEY_AS_LIST, queryString, 35f)); + // Search by key uses the "sortable" sub-field as it requires to be case-insensitive (lower-case filtering) + qb.should(matchQuery(FIELD_RULE_KEY + "." + SORT_SUFFIX, queryString).operator(MatchQueryBuilder.Operator.AND).boost(30f)); + qb.should(matchQuery(FIELD_RULE_RULE_KEY + "." + SORT_SUFFIX, queryString).operator(MatchQueryBuilder.Operator.AND).boost(15f)); qb.should(termQuery(FIELD_RULE_LANGUAGE, queryString, 3f)); qb.should(termQuery(FIELD_RULE_ALL_TAGS, queryString, 10f)); qb.should(termAnyQuery(FIELD_RULE_ALL_TAGS, queryString, 1f)); diff --git a/server/sonar-server/src/main/java/org/sonar/server/rule/index/RuleIndexDefinition.java b/server/sonar-server/src/main/java/org/sonar/server/rule/index/RuleIndexDefinition.java index d4a7fb99157..87ea1ae7a28 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/rule/index/RuleIndexDefinition.java +++ b/server/sonar-server/src/main/java/org/sonar/server/rule/index/RuleIndexDefinition.java @@ -35,8 +35,6 @@ public class RuleIndexDefinition implements IndexDefinition { public static final String TYPE_RULE = "rule"; public static final String FIELD_RULE_KEY = "key"; - // TODO find at what this field is useful ? - public static final String FIELD_RULE_KEY_AS_LIST = "_key"; public static final String FIELD_RULE_REPOSITORY = "repo"; public static final String FIELD_RULE_RULE_KEY = "ruleKey"; public static final String FIELD_RULE_INTERNAL_KEY = "internalKey"; @@ -92,8 +90,7 @@ public class RuleIndexDefinition implements IndexDefinition { ruleMapping.setEnableSource(false); ruleMapping.stringFieldBuilder(FIELD_RULE_KEY).enableSorting().build(); - ruleMapping.stringFieldBuilder(FIELD_RULE_KEY_AS_LIST).build(); - ruleMapping.stringFieldBuilder(FIELD_RULE_RULE_KEY).disableSearch().docValues().build(); + ruleMapping.stringFieldBuilder(FIELD_RULE_RULE_KEY).enableSorting().build(); ruleMapping.stringFieldBuilder(FIELD_RULE_REPOSITORY).docValues().build(); ruleMapping.stringFieldBuilder(FIELD_RULE_INTERNAL_KEY).disableSearch().docValues().build(); diff --git a/server/sonar-server/src/main/java/org/sonar/server/rule/index/RuleIndexer.java b/server/sonar-server/src/main/java/org/sonar/server/rule/index/RuleIndexer.java index 3cffbde5689..35da71cb5af 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/rule/index/RuleIndexer.java +++ b/server/sonar-server/src/main/java/org/sonar/server/rule/index/RuleIndexer.java @@ -58,7 +58,7 @@ public class RuleIndexer extends BaseIndexer { rowIt.close(); return maxDate; } finally { - dbSession.close(); + dbClient.closeSession(dbSession); } } @@ -67,11 +67,6 @@ public class RuleIndexer extends BaseIndexer { long maxDate = 0L; while (rules.hasNext()) { RuleDoc rule = rules.next(); - // TODO when active rule is not more DAO v2, restore deleting of REMOVED rules and also remove active rules linked to this rule - // if (rule.status() == RuleStatus.REMOVED) { - // bulk.add(newDeleteRequest(rule)); - // } else { - // } bulk.add(newIndexRequest(rule)); // it's more efficient to sort programmatically than in SQL on some databases (MySQL for instance) @@ -92,10 +87,4 @@ public class RuleIndexer extends BaseIndexer { .routing(rule.repository()) .source(rule.getFields()); } - - // private DeleteRequest newDeleteRequest(RuleDoc rule) { - // return new DeleteRequest(INDEX, TYPE_RULE, rule.key().toString()) - // .routing(rule.repository()); - // } - } diff --git a/server/sonar-server/src/main/java/org/sonar/server/rule/index/RuleResultSetIterator.java b/server/sonar-server/src/main/java/org/sonar/server/rule/index/RuleResultSetIterator.java index 09d2ad17a8e..1ac41b290a6 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/rule/index/RuleResultSetIterator.java +++ b/server/sonar-server/src/main/java/org/sonar/server/rule/index/RuleResultSetIterator.java @@ -20,7 +20,6 @@ package org.sonar.server.rule.index; import com.google.common.base.Splitter; -import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Maps; import com.google.common.collect.Sets; @@ -101,7 +100,6 @@ public class RuleResultSetIterator extends ResultSetIterator { // all the fields must be present, even if value is null doc.setKey(key.toString()); - doc.setKeyAsList(ImmutableList.of(repositoryKey, ruleKey)); doc.setRuleKey(ruleKey); doc.setRepository(repositoryKey); doc.setName(rs.getString(3)); diff --git a/server/sonar-server/src/test/java/org/sonar/server/rule/index/RuleDocTesting.java b/server/sonar-server/src/test/java/org/sonar/server/rule/index/RuleDocTesting.java index 57d7ba5b965..773e7d31cdd 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/rule/index/RuleDocTesting.java +++ b/server/sonar-server/src/test/java/org/sonar/server/rule/index/RuleDocTesting.java @@ -46,7 +46,7 @@ public class RuleDocTesting { .setIsTemplate(false) .setAllTags(Arrays.asList("spring", "performance")) .setType(RuleType.CODE_SMELL) - .setCreatedAt(150000000L) - .setUpdatedAt(160000000L); + .setCreatedAt(1_500_000_000L) + .setUpdatedAt(1_600_000_000L); } } diff --git a/server/sonar-server/src/test/java/org/sonar/server/rule/index/RuleIndexTest.java b/server/sonar-server/src/test/java/org/sonar/server/rule/index/RuleIndexTest.java index 3241bb115a8..6ac245f2339 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/rule/index/RuleIndexTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/rule/index/RuleIndexTest.java @@ -103,15 +103,18 @@ public class RuleIndexTest { } @Test - public void search_key_by_query() { + public void search_by_key() { + RuleKey js1 = RuleKey.of("javascript", "X001"); + RuleKey cobol1 = RuleKey.of("cobol", "X001"); + RuleKey php2 = RuleKey.of("php", "S002"); indexRules( - newDoc(RuleKey.of("javascript", "X001")), - newDoc(RuleKey.of("cobol", "X001")), - newDoc(RuleKey.of("php", "S002"))); + newDoc(js1).setName("First rule").setHtmlDescription("The first rule"), + newDoc(cobol1).setName("Second rule").setHtmlDescription("The second rule"), + newDoc(php2).setName("Third rule").setHtmlDescription("The third rule")); // key RuleQuery query = new RuleQuery().setQueryText("X001"); - assertThat(index.search(query, new SearchOptions()).getIds()).hasSize(2); + assertThat(index.search(query, new SearchOptions()).getIds()).containsOnly(js1, cobol1); // partial key does not match query = new RuleQuery().setQueryText("X00"); @@ -119,7 +122,16 @@ public class RuleIndexTest { // repo:key -> nice-to-have ! query = new RuleQuery().setQueryText("javascript:X001"); - assertThat(index.search(query, new SearchOptions()).getIds()).hasSize(1); + assertThat(index.search(query, new SearchOptions()).getIds()).containsOnly(js1); + } + + @Test + public void search_by_case_insensitive_key() { + RuleKey ruleKey = RuleKey.of("javascript", "X001"); + indexRules(newDoc(ruleKey).setName("Name without key").setHtmlDescription("Description without key")); + + RuleQuery query = new RuleQuery().setQueryText("x001"); + assertThat(index.search(query, new SearchOptions()).getIds()).containsOnly(ruleKey); } @Test diff --git a/server/sonar-server/src/test/java/org/sonar/server/rule/index/RuleResultSetIteratorTest.java b/server/sonar-server/src/test/java/org/sonar/server/rule/index/RuleResultSetIteratorTest.java index 48d6fe66c6f..7f8825a01f6 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/rule/index/RuleResultSetIteratorTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/rule/index/RuleResultSetIteratorTest.java @@ -99,7 +99,6 @@ public class RuleResultSetIteratorTest { RuleDoc rule = rulesByKey.get(templateRule.getRuleKey()); assertThat(rule).isNotNull(); assertThat(rule.key()).isEqualTo(RuleKey.of("xoo", "S001")); - assertThat(rule.keyAsList()).containsOnly("xoo", "S001"); assertThat(rule.ruleKey()).isEqualTo("S001"); assertThat(rule.repository()).isEqualTo("xoo"); assertThat(rule.internalKey()).isEqualTo("S1"); @@ -144,7 +143,6 @@ public class RuleResultSetIteratorTest { RuleDoc rule = rulesByKey.get(templateRule.getRuleKey()); assertThat(rule.key()).isEqualTo(RuleKey.of("xoo", "S001")); - assertThat(rule.keyAsList()).containsOnly("xoo", "S001"); assertThat(rule.ruleKey()).isEqualTo("S001"); assertThat(rule.repository()).isEqualTo("xoo"); assertThat(rule.internalKey()).isEqualTo("S1"); @@ -160,7 +158,6 @@ public class RuleResultSetIteratorTest { rule = rulesByKey.get(customRule.getRuleKey()); assertThat(rule.key()).isEqualTo(RuleKey.of("xoo", "S002")); - assertThat(rule.keyAsList()).containsOnly("xoo", "S002"); assertThat(rule.ruleKey()).isEqualTo("S002"); assertThat(rule.repository()).isEqualTo("xoo"); assertThat(rule.internalKey()).isEqualTo("S2"); -- 2.39.5