From c7aaeb5e536348a02bcf94881babbdf47a3175b6 Mon Sep 17 00:00:00 2001 From: Stephane Gamard Date: Fri, 9 May 2014 16:03:52 +0200 Subject: [PATCH] Added searchability/filter by Tag for RuleIndex & service --- .../java/org/sonar/core/rule/RuleDto.java | 19 ++++---- .../java/org/sonar/server/rule2/RuleDoc.java | 6 +++ .../org/sonar/server/rule2/RuleIndex.java | 5 ++- .../org/sonar/server/search/BaseIndex.java | 27 ++++++++++-- .../server/rule2/RuleIndexMediumTest.java | 44 ++++++++++++++++++- .../server/rule2/RuleServiceMediumTest.java | 5 ++- 6 files changed, 89 insertions(+), 17 deletions(-) diff --git a/sonar-core/src/main/java/org/sonar/core/rule/RuleDto.java b/sonar-core/src/main/java/org/sonar/core/rule/RuleDto.java index c547ac27d3d..532f04d484e 100644 --- a/sonar-core/src/main/java/org/sonar/core/rule/RuleDto.java +++ b/sonar-core/src/main/java/org/sonar/core/rule/RuleDto.java @@ -33,7 +33,8 @@ import javax.annotation.Nullable; import java.util.Arrays; import java.util.Collections; import java.util.Date; -import java.util.List; +import java.util.Set; +import java.util.TreeSet; public final class RuleDto implements Dto { @@ -313,16 +314,16 @@ public final class RuleDto implements Dto { return this; } - public List getTags() { + public Set getTags() { return tags == null ? - Collections.EMPTY_LIST : - Arrays.asList(StringUtils.split(tags, ',')); + Collections.EMPTY_SET : + new TreeSet(Arrays.asList(StringUtils.split(tags, ','))); } - public List getSystemTags() { + public Set getSystemTags() { return systemTags == null ? - Collections.EMPTY_LIST : - Arrays.asList(StringUtils.split(systemTags, ',')); + Collections.EMPTY_SET : + new TreeSet(Arrays.asList(StringUtils.split(systemTags, ','))); } private String getTagsField() { @@ -341,12 +342,12 @@ public final class RuleDto implements Dto { systemTags = s; } - public RuleDto setTags(String[] tags) { + public RuleDto setTags(Set tags) { this.tags = StringUtils.join(tags, ','); return this; } - public RuleDto setSystemTags(String[] tags) { + public RuleDto setSystemTags(Set tags) { this.systemTags = StringUtils.join(tags, ','); return this; } diff --git a/sonar-server/src/main/java/org/sonar/server/rule2/RuleDoc.java b/sonar-server/src/main/java/org/sonar/server/rule2/RuleDoc.java index 96d0648759b..86eebe40679 100644 --- a/sonar-server/src/main/java/org/sonar/server/rule2/RuleDoc.java +++ b/sonar-server/src/main/java/org/sonar/server/rule2/RuleDoc.java @@ -19,6 +19,7 @@ */ package org.sonar.server.rule2; +import org.apache.commons.lang.builder.ReflectionToStringBuilder; import org.sonar.api.rule.RuleKey; import org.sonar.api.rule.RuleStatus; import org.sonar.api.server.debt.DebtRemediationFunction; @@ -189,4 +190,9 @@ class RuleDoc implements Rule { throw new IllegalStateException("Cannot parse date", e); } } + + @Override + public String toString() { + return ReflectionToStringBuilder.toString(this); + } } diff --git a/sonar-server/src/main/java/org/sonar/server/rule2/RuleIndex.java b/sonar-server/src/main/java/org/sonar/server/rule2/RuleIndex.java index e263df3ee54..de064ba5ceb 100644 --- a/sonar-server/src/main/java/org/sonar/server/rule2/RuleIndex.java +++ b/sonar-server/src/main/java/org/sonar/server/rule2/RuleIndex.java @@ -258,11 +258,13 @@ public class RuleIndex extends BaseIndex { @Override protected FilterBuilder getFilter(RuleQuery query, QueryOptions options) { BoolFilterBuilder fb = FilterBuilders.boolFilter(); - boolean hasFilter = false; this.addTermFilter(RuleField.LANGUAGE.key(), query.getLanguages(), fb); this.addTermFilter(RuleField.REPOSITORY.key(), query.getRepositories(), fb); this.addTermFilter(RuleField.SEVERITY.key(), query.getSeverities(), fb); this.addTermFilter(RuleField.KEY.key(), query.getKey(), fb); + + this.addMultiFieldTermFilter(query.getTags(), fb, RuleField.TAGS.key(), RuleField.SYSTEM_TAGS.key()); + if(query.getStatuses() != null && !query.getStatuses().isEmpty()) { Collection stringStatus = new ArrayList(); for (RuleStatus status : query.getStatuses()) { @@ -274,6 +276,7 @@ public class RuleIndex extends BaseIndex { if((query.getLanguages() != null && !query.getLanguages().isEmpty()) || (query.getRepositories() != null && !query.getRepositories().isEmpty()) || (query.getSeverities() != null && !query.getSeverities().isEmpty()) || + (query.getTags() != null && !query.getTags().isEmpty()) || (query.getStatuses() != null && !query.getStatuses().isEmpty()) || (query.getKey() != null && !query.getKey().isEmpty())) { return fb; diff --git a/sonar-server/src/main/java/org/sonar/server/search/BaseIndex.java b/sonar-server/src/main/java/org/sonar/server/search/BaseIndex.java index c178ae57bcc..5c3b3be03dc 100644 --- a/sonar-server/src/main/java/org/sonar/server/search/BaseIndex.java +++ b/sonar-server/src/main/java/org/sonar/server/search/BaseIndex.java @@ -42,6 +42,7 @@ import org.sonar.server.es.ESNode; import java.io.IOException; import java.io.Serializable; +import java.util.ArrayList; import java.util.Collection; import java.util.HashMap; import java.util.Map; @@ -158,7 +159,7 @@ public abstract class BaseIndex, K extends Serializable> Result result = new Result(esResult); - if(esResult != null){ + if (esResult != null) { result .setTotal((int) esResult.getHits().totalHits()) .setTime(esResult.getTookInMillis()); @@ -175,10 +176,10 @@ public abstract class BaseIndex, K extends Serializable> protected abstract R getSearchResult(Map fields); - protected R getSearchResult(SearchHit hit){ + protected R getSearchResult(SearchHit hit) { Map fields = new HashMap(); - for (Map.Entry field:hit.getFields().entrySet()){ - fields.put(field.getKey(),field.getValue().getValue()); + for (Map.Entry field : hit.getFields().entrySet()) { + fields.put(field.getKey(), field.getValue().getValue()); } return this.getSearchResult(fields); } @@ -340,6 +341,24 @@ public abstract class BaseIndex, K extends Serializable> /* ES QueryHelper Methods */ + + protected BoolFilterBuilder addMultiFieldTermFilter(Collection values, BoolFilterBuilder filter, String... fields) { + if (values != null && !values.isEmpty()) { + BoolFilterBuilder valuesFilter = FilterBuilders.boolFilter(); + for (String value : values) { + Collection filterBuilders = new ArrayList(); + for(String field:fields) { + filterBuilders.add(FilterBuilders.termFilter(field, value)); + } + valuesFilter.should(FilterBuilders.orFilter(filterBuilders.toArray(new FilterBuilder[filterBuilders.size()]))); + } + filter.must(valuesFilter); + } + return filter; + } + + + protected BoolFilterBuilder addTermFilter(String field, Collection values, BoolFilterBuilder filter) { if (values != null && !values.isEmpty()) { BoolFilterBuilder valuesFilter = FilterBuilders.boolFilter(); diff --git a/sonar-server/src/test/java/org/sonar/server/rule2/RuleIndexMediumTest.java b/sonar-server/src/test/java/org/sonar/server/rule2/RuleIndexMediumTest.java index 5ee5f268616..b61f9ae8e7c 100644 --- a/sonar-server/src/test/java/org/sonar/server/rule2/RuleIndexMediumTest.java +++ b/sonar-server/src/test/java/org/sonar/server/rule2/RuleIndexMediumTest.java @@ -19,6 +19,7 @@ */ package org.sonar.server.rule2; +import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; import org.junit.After; import org.junit.Before; @@ -32,7 +33,6 @@ import org.sonar.check.Cardinality; import org.sonar.core.persistence.DbSession; import org.sonar.core.persistence.MyBatis; import org.sonar.core.rule.RuleDto; -import org.sonar.server.search.Hit; import org.sonar.server.search.QueryOptions; import org.sonar.server.search.Result; import org.sonar.server.tester.ServerTester; @@ -319,6 +319,48 @@ public class RuleIndexMediumTest { assertThat(Iterables.getFirst(results.getHits(), null).key().rule()).isEqualTo("S002"); assertThat(Iterables.getLast(results.getHits(), null).key().rule()).isEqualTo("S001"); } + @Test + public void search_by_tag() { + dao.insert(newRuleDto(RuleKey.of("java", "S001")).setTags(ImmutableSet.of("tag1")), dbSession); + dao.insert(newRuleDto(RuleKey.of("java", "S002")).setTags(ImmutableSet.of("tag2")), dbSession); + dbSession.commit(); + index.refresh(); + + // find all + RuleQuery query = new RuleQuery(); + assertThat(index.search(query, new QueryOptions()).getHits()).hasSize(2); + + // tag1 in query + query = new RuleQuery().setQueryText("tag1"); + assertThat(index.search(query, new QueryOptions()).getHits()).hasSize(1); + assertThat(Iterables.getFirst(index.search(query, new QueryOptions()).getHits(),null).tags()).containsExactly("tag1"); + + // tag1 and tag2 in query + query = new RuleQuery().setQueryText("tag1 tag2"); + assertThat(index.search(query, new QueryOptions()).getHits()).hasSize(2); + + // tag2 in filter + query = new RuleQuery().setTags(ImmutableSet.of("tag2")); + assertThat(index.search(query, new QueryOptions()).getHits()).hasSize(1); + assertThat(Iterables.getFirst(index.search(query, new QueryOptions()).getHits(),null).tags()).containsExactly("tag2"); + + // tag2 in filter and tag1 tag2 in query + query = new RuleQuery().setTags(ImmutableSet.of("tag2")).setQueryText("tag1"); + assertThat(index.search(query, new QueryOptions()).getHits()).hasSize(0); + + // tag2 in filter and tag1 in query + query = new RuleQuery().setTags(ImmutableSet.of("tag2")).setQueryText("tag1 tag2"); + assertThat(index.search(query, new QueryOptions()).getHits()).hasSize(1); + assertThat(Iterables.getFirst(index.search(query, new QueryOptions()).getHits(),null).tags()).containsExactly("tag2"); + + // null list => no filter + query = new RuleQuery().setTags(Collections.emptySet()); + assertThat(index.search(query, new QueryOptions()).getHits()).hasSize(2); + + // null list => no filter + query = new RuleQuery().setTags(null); + assertThat(index.search(query, new QueryOptions()).getHits()).hasSize(2); + } @Test public void paging() { diff --git a/sonar-server/src/test/java/org/sonar/server/rule2/RuleServiceMediumTest.java b/sonar-server/src/test/java/org/sonar/server/rule2/RuleServiceMediumTest.java index aabdc24308e..60a607955ba 100644 --- a/sonar-server/src/test/java/org/sonar/server/rule2/RuleServiceMediumTest.java +++ b/sonar-server/src/test/java/org/sonar/server/rule2/RuleServiceMediumTest.java @@ -19,6 +19,7 @@ */ package org.sonar.server.rule2; +import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; import org.junit.After; import org.junit.Before; @@ -153,6 +154,8 @@ public class RuleServiceMediumTest { .setSeverity(Severity.INFO) .setCardinality(Cardinality.SINGLE) .setLanguage("js") + .setTags(ImmutableSet.of("tag1", "tag2")) + .setSystemTags(ImmutableSet.of("systag1", "systag2")) .setRemediationFunction("linear") .setDefaultRemediationFunction("linear_offset") .setRemediationCoefficient("1h") @@ -160,8 +163,6 @@ public class RuleServiceMediumTest { .setRemediationOffset("5min") .setDefaultRemediationOffset("10h") .setEffortToFixDescription(ruleKey.repository() + "." + ruleKey.rule() + ".effortToFix") - .setTags(new String[]{"tag1", "tag2"}) - .setSystemTags(new String[]{"systag1", "systag2"}) .setCreatedAt(DateUtils.parseDate("2013-12-16")) .setUpdatedAt(DateUtils.parseDate("2013-12-17")); } -- 2.39.5