From 92d3517ba7af03c40e85970fecd099b7aa25f30b Mon Sep 17 00:00:00 2001 From: Julien Lancelot Date: Fri, 28 Mar 2014 10:36:17 +0100 Subject: [PATCH] SONAR-5174 Rules search by language and repositories --- .../technicaldebt/server/Characteristic.java | 2 +- .../main/java/org/sonar/server/rule/Rule.java | 75 ++++++++++++++++++- .../java/org/sonar/server/rule/RuleQuery.java | 46 ++++++++++-- .../org/sonar/server/rule/RuleRegistry.java | 17 ++++- .../sonar/server/rule/RuleRegistryTest.java | 28 +++++-- .../java/org/sonar/server/rule/RuleTest.java | 12 +++ .../rule/RuleRegistryTest/shared/rule2.json | 2 +- 7 files changed, 158 insertions(+), 24 deletions(-) diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/technicaldebt/server/Characteristic.java b/sonar-plugin-api/src/main/java/org/sonar/api/technicaldebt/server/Characteristic.java index 1e263a6fe20..ac5a5166c38 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/technicaldebt/server/Characteristic.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/technicaldebt/server/Characteristic.java @@ -27,7 +27,7 @@ import org.sonar.api.utils.internal.WorkDuration; import javax.annotation.CheckForNull; /** - * @since 4.1 * + * @since 4.1 * @deprecated since 4.3. */ @Deprecated diff --git a/sonar-server/src/main/java/org/sonar/server/rule/Rule.java b/sonar-server/src/main/java/org/sonar/server/rule/Rule.java index d4a1a34b391..de3f08034db 100644 --- a/sonar-server/src/main/java/org/sonar/server/rule/Rule.java +++ b/sonar-server/src/main/java/org/sonar/server/rule/Rule.java @@ -20,6 +20,7 @@ package org.sonar.server.rule; import org.sonar.api.rule.RuleKey; +import org.sonar.api.server.debt.DebtRemediationFunction; import org.sonar.check.Cardinality; import javax.annotation.CheckForNull; @@ -46,6 +47,11 @@ public class Rule { private Collection systemTags; private Collection adminTags; private Collection params; + private String debtCharacteristicKey; + private String debtCharacteristicName; + private String debtSubCharacteristicKey; + private String debtSubCharacteristicName; + private DebtRemediationFunction debtRemediationFunction; private Date createdAt; private Date updatedAt; @@ -63,6 +69,11 @@ public class Rule { this.systemTags = defaultCollection(builder.systemTags); this.adminTags = defaultCollection(builder.adminTags); this.params = defaultCollection(builder.params); + this.debtCharacteristicKey = builder.debtCharacteristicKey; + this.debtCharacteristicName = builder.debtCharacteristicName; + this.debtSubCharacteristicKey = builder.debtSubCharacteristicKey; + this.debtSubCharacteristicName = builder.debtSubCharacteristicName; + this.debtRemediationFunction = builder.debtRemediationFunction; this.createdAt = builder.createdAt; this.updatedAt = builder.updatedAt; } @@ -123,10 +134,40 @@ public class Rule { return params; } + @CheckForNull + public String debtCharacteristicKey() { + return debtCharacteristicKey; + } + + @CheckForNull + public String debtCharacteristicName() { + return debtCharacteristicName; + } + + @CheckForNull + public String debtSubCharacteristicKey() { + return debtSubCharacteristicKey; + } + + @CheckForNull + public String debtSubCharacteristicName() { + return debtSubCharacteristicName; + } + + @CheckForNull + public DebtRemediationFunction debtRemediationFunction() { + return debtRemediationFunction; + } + public Date createdAt() { return createdAt; } + @CheckForNull + public Date updatedAt() { + return updatedAt; + } + public boolean isTemplate() { return Cardinality.MULTIPLE.toString().equals(cardinality); } @@ -135,10 +176,6 @@ public class Rule { return templateId != null; } - @CheckForNull - public Date updatedAt() { - return updatedAt; - } public static class Builder { @@ -156,6 +193,11 @@ public class Rule { private Collection systemTags; private Collection adminTags; private Collection params; + private String debtCharacteristicKey; + private String debtCharacteristicName; + private String debtSubCharacteristicKey; + private String debtSubCharacteristicName; + private DebtRemediationFunction debtRemediationFunction; private Date createdAt; private Date updatedAt; @@ -232,6 +274,31 @@ public class Rule { return this; } + public Builder setDebtCharacteristicKey(@Nullable String debtCharacteristicKey) { + this.debtCharacteristicKey = debtCharacteristicKey; + return this; + } + + public Builder setDebtCharacteristicName(@Nullable String debtCharacteristicName) { + this.debtCharacteristicName = debtCharacteristicName; + return this; + } + + public Builder setDebtSubCharacteristicKey(@Nullable String debtSubCharacteristicKey) { + this.debtSubCharacteristicKey = debtSubCharacteristicKey; + return this; + } + + public Builder setDebtSubCharacteristicName(@Nullable String debtSubCharacteristicName) { + this.debtSubCharacteristicName = debtSubCharacteristicName; + return this; + } + + public Builder setDebtRemediationFunction(@Nullable DebtRemediationFunction debtRemediationFunction) { + this.debtRemediationFunction = debtRemediationFunction; + return this; + } + public Builder setCreatedAt(Date createdAt) { this.createdAt = createdAt; return this; diff --git a/sonar-server/src/main/java/org/sonar/server/rule/RuleQuery.java b/sonar-server/src/main/java/org/sonar/server/rule/RuleQuery.java index 5448f4f1364..8635d85fcbf 100644 --- a/sonar-server/src/main/java/org/sonar/server/rule/RuleQuery.java +++ b/sonar-server/src/main/java/org/sonar/server/rule/RuleQuery.java @@ -24,6 +24,9 @@ import com.google.common.base.Preconditions; import javax.annotation.CheckForNull; import javax.annotation.Nullable; +import java.util.Collection; +import java.util.Collections; + /** * @since 4.3 */ @@ -34,7 +37,9 @@ public class RuleQuery { private String key; private String query; - private String characteristicKey; + private String characteristic; + private String language; + private Collection repositories; private int pageSize; private int pageIndex; @@ -42,7 +47,9 @@ public class RuleQuery { private RuleQuery(Builder builder) { this.key = builder.key; this.query = builder.query; - this.characteristicKey = builder.characteristicKey; + this.language = builder.language; + this.repositories = defaultCollection(builder.repositories); + this.characteristic = builder.characteristic; this.pageSize = builder.pageSize; this.pageIndex = builder.pageIndex; } @@ -58,8 +65,17 @@ public class RuleQuery { } @CheckForNull - public String characteristicKey() { - return characteristicKey; + public String language() { + return language; + } + + public Collection repositories() { + return repositories; + } + + @CheckForNull + public String characteristic() { + return characteristic; } public int pageSize() { @@ -78,7 +94,9 @@ public class RuleQuery { private String key; private String query; - private String characteristicKey; + private String characteristic; + private String language; + private Collection repositories; private Integer pageSize; private Integer pageIndex; @@ -93,8 +111,18 @@ public class RuleQuery { return this; } - public Builder characteristicKey(@Nullable String characteristicKey) { - this.characteristicKey = characteristicKey; + public Builder language(String language) { + this.language = language; + return this; + } + + public Builder repositories(Collection repositories) { + this.repositories = repositories; + return this; + } + + public Builder characteristic(@Nullable String characteristic) { + this.characteristic = characteristic; return this; } @@ -127,4 +155,8 @@ public class RuleQuery { Preconditions.checkArgument(pageIndex > 0, "Page index must be greater than 0 (got " + pageIndex + ")"); } } + + private static Collection defaultCollection(@Nullable Collection c) { + return c == null ? Collections.emptyList() : Collections.unmodifiableCollection(c); + } } diff --git a/sonar-server/src/main/java/org/sonar/server/rule/RuleRegistry.java b/sonar-server/src/main/java/org/sonar/server/rule/RuleRegistry.java index eb422f5de33..ea653871e66 100644 --- a/sonar-server/src/main/java/org/sonar/server/rule/RuleRegistry.java +++ b/sonar-server/src/main/java/org/sonar/server/rule/RuleRegistry.java @@ -58,8 +58,7 @@ import java.util.List; import java.util.Map; import static com.google.common.collect.Lists.newArrayList; -import static org.elasticsearch.index.query.FilterBuilders.boolFilter; -import static org.elasticsearch.index.query.FilterBuilders.termFilter; +import static org.elasticsearch.index.query.FilterBuilders.*; import static org.sonar.api.rules.Rule.STATUS_REMOVED; /** @@ -138,11 +137,21 @@ public class RuleRegistry { mainFilter.must(FilterBuilders.queryFilter( QueryBuilders.multiMatchQuery(query.query(), RuleDocument.FIELD_NAME + ".search", RuleDocument.FIELD_KEY).operator(Operator.AND))); } - if (query.characteristicKey() != null) { + if (query.characteristic() != null) { mainFilter.must(FilterBuilders.queryFilter( - QueryBuilders.multiMatchQuery(query.characteristicKey(), RuleDocument.FIELD_CHARACTERISTIC_KEY, RuleDocument.FIELD_SUB_CHARACTERISTIC_KEY).operator(Operator.OR)) + QueryBuilders.multiMatchQuery(query.characteristic(), RuleDocument.FIELD_CHARACTERISTIC_KEY, RuleDocument.FIELD_SUB_CHARACTERISTIC_KEY).operator(Operator.OR)) ); } + if (query.language() != null) { + mainFilter.must(termFilter(RuleDocument.FIELD_LANGUAGE, query.language())); + } + if (!query.repositories().isEmpty()) { + if (query.repositories().size() == 1) { + mainFilter.must(termFilter(RuleDocument.FIELD_REPOSITORY_KEY, query.repositories().iterator().next())); + } else { + mainFilter.must(termsFilter(RuleDocument.FIELD_REPOSITORY_KEY, query.repositories().toArray())); + } + } Paging paging = Paging.create(query.pageSize(), query.pageIndex()); SearchHits hits = searchIndex.executeRequest( searchIndex.client().prepareSearch(INDEX_RULES).setTypes(TYPE_RULE) diff --git a/sonar-server/src/test/java/org/sonar/server/rule/RuleRegistryTest.java b/sonar-server/src/test/java/org/sonar/server/rule/RuleRegistryTest.java index 123cf8357de..b54a81f151c 100644 --- a/sonar-server/src/test/java/org/sonar/server/rule/RuleRegistryTest.java +++ b/sonar-server/src/test/java/org/sonar/server/rule/RuleRegistryTest.java @@ -47,6 +47,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import static com.google.common.collect.Lists.newArrayList; import static com.google.common.collect.Maps.newHashMap; import static org.fest.assertions.Assertions.assertThat; import static org.mockito.Matchers.any; @@ -279,16 +280,29 @@ public class RuleRegistryTest { @Test public void find_rules_by_name() { // Removed rule should not appear - assertThat(registry.find(RuleQuery.builder().pageIndex(1).pageSize(10).build()).results()).hasSize(2); + assertThat(registry.find(RuleQuery.builder().build()).results()).hasSize(2); // Search is case insensitive - assertThat(registry.find(RuleQuery.builder().pageIndex(1).pageSize(10).searchQuery("one issue per line").build()).results()).hasSize(1); + assertThat(registry.find(RuleQuery.builder().searchQuery("one issue per line").build()).results()).hasSize(1); // Search is ngram based - assertThat(registry.find(RuleQuery.builder().pageIndex(1).pageSize(10).searchQuery("with param").build()).results()).hasSize(1); + assertThat(registry.find(RuleQuery.builder().searchQuery("with param").build()).results()).hasSize(1); // Search works also with key - assertThat(registry.find(RuleQuery.builder().pageIndex(1).pageSize(10).searchQuery("OneIssuePerLine").build()).results()).hasSize(1); + assertThat(registry.find(RuleQuery.builder().searchQuery("OneIssuePerLine").build()).results()).hasSize(1); + } + + @Test + public void find_rules_by_language() { + assertThat(registry.find(RuleQuery.builder().language("xoo").build()).results()).hasSize(2); + assertThat(registry.find(RuleQuery.builder().language("unknown").build()).results()).isEmpty(); + } + + @Test + public void find_rules_by_rule_repositories() { + assertThat(registry.find(RuleQuery.builder().repositories(newArrayList("xoo")).build()).results()).hasSize(1); + assertThat(registry.find(RuleQuery.builder().repositories(newArrayList("xoo", "xoo2")).build()).results()).hasSize(2); + assertThat(registry.find(RuleQuery.builder().repositories(newArrayList("unknown")).build()).results()).isEmpty(); } @Test @@ -353,9 +367,9 @@ public class RuleRegistryTest { registry.bulkRegisterRules(rules, characteristics, ArrayListMultimap.create(), ArrayListMultimap.create()); - assertThat(registry.find(RuleQuery.builder().characteristicKey("MODULARITY").build()).results()).hasSize(1); - assertThat(registry.find(RuleQuery.builder().characteristicKey("REUSABILITY").build()).results()).hasSize(1); - assertThat(registry.find(RuleQuery.builder().characteristicKey("Unknown").build()).results()).isEmpty(); + assertThat(registry.find(RuleQuery.builder().characteristic("MODULARITY").build()).results()).hasSize(1); + assertThat(registry.find(RuleQuery.builder().characteristic("REUSABILITY").build()).results()).hasSize(1); + assertThat(registry.find(RuleQuery.builder().characteristic("Unknown").build()).results()).isEmpty(); } private String testFileAsString(String testFile) throws Exception { diff --git a/sonar-server/src/test/java/org/sonar/server/rule/RuleTest.java b/sonar-server/src/test/java/org/sonar/server/rule/RuleTest.java index 10758a9665d..0df5db82313 100644 --- a/sonar-server/src/test/java/org/sonar/server/rule/RuleTest.java +++ b/sonar-server/src/test/java/org/sonar/server/rule/RuleTest.java @@ -23,6 +23,8 @@ package org.sonar.server.rule; import org.junit.Test; import org.sonar.api.rule.RuleKey; import org.sonar.api.rule.Severity; +import org.sonar.api.server.debt.DebtRemediationFunction; +import org.sonar.api.server.debt.internal.DefaultDebtRemediationFunction; import org.sonar.api.server.rule.RuleParamType; import org.sonar.check.Cardinality; @@ -50,6 +52,11 @@ public class RuleTest { .setAdminTags(newArrayList("AdminTag")) .setSystemTags(newArrayList("SysTag")) .setParams(newArrayList(new RuleParam("key", "desc", "default", RuleParamType.STRING))) + .setDebtCharacteristicKey("REUSABILITY") + .setDebtCharacteristicName("Reusability") + .setDebtSubCharacteristicKey("MODULARITY") + .setDebtSubCharacteristicName("Modularity") + .setDebtRemediationFunction(new DefaultDebtRemediationFunction(DebtRemediationFunction.Type.LINEAR_OFFSET, "1h", "15min")) .setCreatedAt(new Date()) .setUpdatedAt(new Date()) .build(); @@ -67,6 +74,11 @@ public class RuleTest { assertThat(rule.adminTags()).hasSize(1); assertThat(rule.systemTags()).hasSize(1); assertThat(rule.params()).hasSize(1); + assertThat(rule.debtCharacteristicKey()).isEqualTo("REUSABILITY"); + assertThat(rule.debtCharacteristicName()).isEqualTo("Reusability"); + assertThat(rule.debtSubCharacteristicKey()).isEqualTo("MODULARITY"); + assertThat(rule.debtSubCharacteristicName()).isEqualTo("Modularity"); + assertThat(rule.debtRemediationFunction()).isEqualTo(new DefaultDebtRemediationFunction(DebtRemediationFunction.Type.LINEAR_OFFSET, "1h", "15min")); assertThat(rule.createdAt()).isNotNull(); assertThat(rule.updatedAt()).isNotNull(); } diff --git a/sonar-server/src/test/resources/org/sonar/server/rule/RuleRegistryTest/shared/rule2.json b/sonar-server/src/test/resources/org/sonar/server/rule/RuleRegistryTest/shared/rule2.json index f1adc5ecc65..3879ecccdfc 100644 --- a/sonar-server/src/test/resources/org/sonar/server/rule/RuleRegistryTest/shared/rule2.json +++ b/sonar-server/src/test/resources/org/sonar/server/rule/RuleRegistryTest/shared/rule2.json @@ -5,7 +5,7 @@ "name": "One Issue Per Line", "description": "Generate an issue on each line of a file. It requires the metric \"lines\".", "parentKey": null, - "repositoryKey": "xoo", + "repositoryKey": "xoo2", "severity": "MAJOR", "status": "BETA", "createdAt": "2013-10-28T13:07:26.339Z", -- 2.39.5