Skip to content

Commit

Permalink
SONAR-5959 Change behavior of component-related issue search queries
Browse files Browse the repository at this point in the history
  • Loading branch information
jblievremont committed Jan 5, 2015
1 parent 37e1304 commit 928ab17
Show file tree
Hide file tree
Showing 7 changed files with 72 additions and 32 deletions.
Expand Up @@ -58,7 +58,7 @@ public class IssueQuery {
private final Collection<String> statuses; private final Collection<String> statuses;
private final Collection<String> resolutions; private final Collection<String> resolutions;
private final Collection<String> components; private final Collection<String> components;
private final Collection<String> componentRoots; private final Collection<String> modules;
private final Collection<String> projects; private final Collection<String> projects;
private final Collection<RuleKey> rules; private final Collection<RuleKey> rules;
private final Collection<String> actionPlans; private final Collection<String> actionPlans;
Expand All @@ -84,7 +84,7 @@ private IssueQuery(Builder builder) {
this.statuses = defaultCollection(builder.statuses); this.statuses = defaultCollection(builder.statuses);
this.resolutions = defaultCollection(builder.resolutions); this.resolutions = defaultCollection(builder.resolutions);
this.components = defaultCollection(builder.components); this.components = defaultCollection(builder.components);
this.componentRoots = defaultCollection(builder.componentRoots); this.modules = defaultCollection(builder.modules);
this.projects = defaultCollection(builder.projects); this.projects = defaultCollection(builder.projects);
this.rules = defaultCollection(builder.rules); this.rules = defaultCollection(builder.rules);
this.actionPlans = defaultCollection(builder.actionPlans); this.actionPlans = defaultCollection(builder.actionPlans);
Expand Down Expand Up @@ -125,8 +125,8 @@ public Collection<String> componentUuids() {
return components; return components;
} }


public Collection<String> componentRootUuids() { public Collection<String> moduleUuids() {
return componentRoots; return modules;
} }


public Collection<String> projectUuids() { public Collection<String> projectUuids() {
Expand Down Expand Up @@ -230,7 +230,7 @@ public static class Builder {
private Collection<String> statuses; private Collection<String> statuses;
private Collection<String> resolutions; private Collection<String> resolutions;
private Collection<String> components; private Collection<String> components;
private Collection<String> componentRoots; private Collection<String> modules;
private Collection<String> projects; private Collection<String> projects;
private Collection<RuleKey> rules; private Collection<RuleKey> rules;
private Collection<String> actionPlans; private Collection<String> actionPlans;
Expand Down Expand Up @@ -278,8 +278,8 @@ public Builder componentUuids(@Nullable Collection<String> l) {
return this; return this;
} }


public Builder componentRootUuids(@Nullable Collection<String> l) { public Builder moduleUuids(@Nullable Collection<String> l) {
this.componentRoots = l; this.modules = l;
return this; return this;
} }


Expand Down
Expand Up @@ -193,9 +193,9 @@ private void addComponentRootUuids(IssueQuery.Builder builder, DbSession session
if (componentRoots != null) { if (componentRoots != null) {
throw new IllegalArgumentException("componentRoots and componentRootUuids cannot be set simultaneously"); throw new IllegalArgumentException("componentRoots and componentRootUuids cannot be set simultaneously");
} }
builder.componentRootUuids(componentRootUuids); builder.moduleUuids(componentRootUuids);
} else { } else {
builder.componentRootUuids(componentUuids(session, componentRoots)); builder.moduleUuids(componentUuids(session, componentRoots));
} }
} }


Expand Down
Expand Up @@ -24,6 +24,7 @@
import com.google.common.collect.Collections2; import com.google.common.collect.Collections2;
import com.google.common.collect.Lists; import com.google.common.collect.Lists;
import com.google.common.collect.Maps; import com.google.common.collect.Maps;
import com.google.common.collect.Sets;
import org.apache.commons.lang.BooleanUtils; import org.apache.commons.lang.BooleanUtils;
import org.elasticsearch.action.search.SearchRequestBuilder; import org.elasticsearch.action.search.SearchRequestBuilder;
import org.elasticsearch.action.search.SearchResponse; import org.elasticsearch.action.search.SearchResponse;
Expand Down Expand Up @@ -254,9 +255,9 @@ protected Map<String, FilterBuilder> getFilters(IssueQuery query, QueryContext o
filters.put(IssueIndexDefinition.FIELD_ISSUE_KEY, matchFilter(IssueIndexDefinition.FIELD_ISSUE_KEY, query.issueKeys())); filters.put(IssueIndexDefinition.FIELD_ISSUE_KEY, matchFilter(IssueIndexDefinition.FIELD_ISSUE_KEY, query.issueKeys()));
filters.put(IssueIndexDefinition.FIELD_ISSUE_ACTION_PLAN, matchFilter(IssueIndexDefinition.FIELD_ISSUE_ACTION_PLAN, query.actionPlans())); filters.put(IssueIndexDefinition.FIELD_ISSUE_ACTION_PLAN, matchFilter(IssueIndexDefinition.FIELD_ISSUE_ACTION_PLAN, query.actionPlans()));
filters.put(IssueIndexDefinition.FIELD_ISSUE_ASSIGNEE, matchFilter(IssueIndexDefinition.FIELD_ISSUE_ASSIGNEE, query.assignees())); filters.put(IssueIndexDefinition.FIELD_ISSUE_ASSIGNEE, matchFilter(IssueIndexDefinition.FIELD_ISSUE_ASSIGNEE, query.assignees()));
filters.put(IssueIndexDefinition.FIELD_ISSUE_MODULE_PATH, matchFilter(IssueIndexDefinition.FIELD_ISSUE_MODULE_PATH, query.componentRootUuids()));
filters.put(IssueIndexDefinition.FIELD_ISSUE_COMPONENT_UUID, matchFilter(IssueIndexDefinition.FIELD_ISSUE_COMPONENT_UUID, query.componentUuids())); addComponentRelatedFilters(query, filters);
filters.put(IssueIndexDefinition.FIELD_ISSUE_PROJECT_UUID, matchFilter(IssueIndexDefinition.FIELD_ISSUE_PROJECT_UUID, query.projectUuids()));
filters.put(IssueIndexDefinition.FIELD_ISSUE_LANGUAGE, matchFilter(IssueIndexDefinition.FIELD_ISSUE_LANGUAGE, query.languages())); filters.put(IssueIndexDefinition.FIELD_ISSUE_LANGUAGE, matchFilter(IssueIndexDefinition.FIELD_ISSUE_LANGUAGE, query.languages()));
filters.put(IssueIndexDefinition.FIELD_ISSUE_TAGS, matchFilter(IssueIndexDefinition.FIELD_ISSUE_TAGS, query.tags())); filters.put(IssueIndexDefinition.FIELD_ISSUE_TAGS, matchFilter(IssueIndexDefinition.FIELD_ISSUE_TAGS, query.tags()));
filters.put(IssueIndexDefinition.FIELD_ISSUE_RESOLUTION, matchFilter(IssueIndexDefinition.FIELD_ISSUE_RESOLUTION, query.resolutions())); filters.put(IssueIndexDefinition.FIELD_ISSUE_RESOLUTION, matchFilter(IssueIndexDefinition.FIELD_ISSUE_RESOLUTION, query.resolutions()));
Expand All @@ -270,6 +271,33 @@ protected Map<String, FilterBuilder> getFilters(IssueQuery query, QueryContext o
return filters; return filters;
} }


private void addComponentRelatedFilters(IssueQuery query, Map<String, FilterBuilder> filters) {
if (query.onComponentOnly()) {
Set<String> allComponents = Sets.newHashSet();
allComponents.addAll(query.projectUuids());
allComponents.addAll(query.moduleUuids());
allComponents.addAll(query.componentUuids());
filters.put(IssueIndexDefinition.FIELD_ISSUE_COMPONENT_UUID, matchFilter(IssueIndexDefinition.FIELD_ISSUE_COMPONENT_UUID, allComponents));
} else {
filters.put(IssueIndexDefinition.FIELD_ISSUE_PROJECT_UUID, matchFilter(IssueIndexDefinition.FIELD_ISSUE_PROJECT_UUID, query.projectUuids()));
filters.put(IssueIndexDefinition.FIELD_ISSUE_MODULE_UUID, matchFilter(IssueIndexDefinition.FIELD_ISSUE_MODULE_UUID, query.moduleUuids()));

FilterBuilder componentFilter = matchFilter(IssueIndexDefinition.FIELD_ISSUE_COMPONENT_UUID, query.componentUuids());
FilterBuilder modulePathFilter = matchFilter(IssueIndexDefinition.FIELD_ISSUE_MODULE_PATH, query.componentUuids());
BoolFilterBuilder compositeFilter = null;
if (componentFilter != null || modulePathFilter != null) {
compositeFilter = FilterBuilders.boolFilter();
if (componentFilter != null) {
compositeFilter.should(componentFilter);
}
if (modulePathFilter != null) {
compositeFilter.should(modulePathFilter);
}
}
filters.put(IssueIndexDefinition.FIELD_ISSUE_COMPONENT_UUID, compositeFilter);
}
}

private FilterBuilder getAuthorizationFilter(QueryContext options) { private FilterBuilder getAuthorizationFilter(QueryContext options) {
String user = options.getUserLogin(); String user = options.getUserLogin();
Set<String> groups = options.getUserGroups(); Set<String> groups = options.getUserGroups();
Expand Down
Expand Up @@ -445,6 +445,7 @@ private void collectFacetsData(Request request, Result<Issue> result, Set<String
collectParameterValues(request, IssueFilterParameters.PROJECT_UUIDS, projectUuids); collectParameterValues(request, IssueFilterParameters.PROJECT_UUIDS, projectUuids);
collectFacetKeys(result, IssueFilterParameters.COMPONENT_UUIDS, componentUuids); collectFacetKeys(result, IssueFilterParameters.COMPONENT_UUIDS, componentUuids);
collectParameterValues(request, IssueFilterParameters.COMPONENT_UUIDS, componentUuids); collectParameterValues(request, IssueFilterParameters.COMPONENT_UUIDS, componentUuids);
collectParameterValues(request, IssueFilterParameters.MODULE_UUIDS, componentUuids);
collectParameterValues(request, IssueFilterParameters.COMPONENT_ROOT_UUIDS, componentUuids); collectParameterValues(request, IssueFilterParameters.COMPONENT_ROOT_UUIDS, componentUuids);
collectFacetKeys(result, IssueFilterParameters.ASSIGNEES, userLogins); collectFacetKeys(result, IssueFilterParameters.ASSIGNEES, userLogins);
collectParameterValues(request, IssueFilterParameters.ASSIGNEES, userLogins); collectParameterValues(request, IssueFilterParameters.ASSIGNEES, userLogins);
Expand Down
Expand Up @@ -117,7 +117,7 @@ public void create_query_from_parameters() {
assertThat(query.resolutions()).containsOnly("FALSE-POSITIVE"); assertThat(query.resolutions()).containsOnly("FALSE-POSITIVE");
assertThat(query.resolved()).isTrue(); assertThat(query.resolved()).isTrue();
assertThat(query.componentUuids()).containsOnly("ABCD"); assertThat(query.componentUuids()).containsOnly("ABCD");
assertThat(query.componentRootUuids()).containsOnly("BCDE"); assertThat(query.moduleUuids()).containsOnly("BCDE");
assertThat(query.reporters()).containsOnly("marilyn"); assertThat(query.reporters()).containsOnly("marilyn");
assertThat(query.assignees()).containsOnly("joanna"); assertThat(query.assignees()).containsOnly("joanna");
assertThat(query.languages()).containsOnly("xoo"); assertThat(query.languages()).containsOnly("xoo");
Expand Down
Expand Up @@ -40,7 +40,7 @@ public void build_query() throws Exception {
.statuses(Lists.newArrayList(Issue.STATUS_RESOLVED)) .statuses(Lists.newArrayList(Issue.STATUS_RESOLVED))
.resolutions(newArrayList(Issue.RESOLUTION_FALSE_POSITIVE)) .resolutions(newArrayList(Issue.RESOLUTION_FALSE_POSITIVE))
.componentUuids(newArrayList("org/struts/Action.java")) .componentUuids(newArrayList("org/struts/Action.java"))
.componentRootUuids(newArrayList("org.struts:core")) .moduleUuids(newArrayList("org.struts:core"))
.rules(newArrayList(RuleKey.of("squid", "AvoidCycle"))) .rules(newArrayList(RuleKey.of("squid", "AvoidCycle")))
.actionPlans(newArrayList("AP1", "AP2")) .actionPlans(newArrayList("AP1", "AP2"))
.reporters(newArrayList("crunky")) .reporters(newArrayList("crunky"))
Expand All @@ -62,7 +62,7 @@ public void build_query() throws Exception {
assertThat(query.statuses()).containsOnly(Issue.STATUS_RESOLVED); assertThat(query.statuses()).containsOnly(Issue.STATUS_RESOLVED);
assertThat(query.resolutions()).containsOnly(Issue.RESOLUTION_FALSE_POSITIVE); assertThat(query.resolutions()).containsOnly(Issue.RESOLUTION_FALSE_POSITIVE);
assertThat(query.componentUuids()).containsOnly("org/struts/Action.java"); assertThat(query.componentUuids()).containsOnly("org/struts/Action.java");
assertThat(query.componentRootUuids()).containsOnly("org.struts:core"); assertThat(query.moduleUuids()).containsOnly("org.struts:core");
assertThat(query.reporters()).containsOnly("crunky"); assertThat(query.reporters()).containsOnly("crunky");
assertThat(query.assignees()).containsOnly("gargantua"); assertThat(query.assignees()).containsOnly("gargantua");
assertThat(query.languages()).containsOnly("xoo"); assertThat(query.languages()).containsOnly("xoo");
Expand Down Expand Up @@ -111,7 +111,7 @@ public void collection_params_should_not_be_null_but_empty() throws Exception {
IssueQuery query = IssueQuery.builder() IssueQuery query = IssueQuery.builder()
.issueKeys(null) .issueKeys(null)
.componentUuids(null) .componentUuids(null)
.componentRootUuids(null) .moduleUuids(null)
.statuses(null) .statuses(null)
.actionPlans(null) .actionPlans(null)
.assignees(null) .assignees(null)
Expand All @@ -124,7 +124,7 @@ public void collection_params_should_not_be_null_but_empty() throws Exception {
.build(); .build();
assertThat(query.issueKeys()).isEmpty(); assertThat(query.issueKeys()).isEmpty();
assertThat(query.componentUuids()).isEmpty(); assertThat(query.componentUuids()).isEmpty();
assertThat(query.componentRootUuids()).isEmpty(); assertThat(query.moduleUuids()).isEmpty();
assertThat(query.statuses()).isEmpty(); assertThat(query.statuses()).isEmpty();
assertThat(query.actionPlans()).isEmpty(); assertThat(query.actionPlans()).isEmpty();
assertThat(query.assignees()).isEmpty(); assertThat(query.assignees()).isEmpty();
Expand All @@ -141,7 +141,7 @@ public void test_default_query() throws Exception {
IssueQuery query = IssueQuery.builder().build(); IssueQuery query = IssueQuery.builder().build();
assertThat(query.issueKeys()).isEmpty(); assertThat(query.issueKeys()).isEmpty();
assertThat(query.componentUuids()).isEmpty(); assertThat(query.componentUuids()).isEmpty();
assertThat(query.componentRootUuids()).isEmpty(); assertThat(query.moduleUuids()).isEmpty();
assertThat(query.statuses()).isEmpty(); assertThat(query.statuses()).isEmpty();
assertThat(query.actionPlans()).isEmpty(); assertThat(query.actionPlans()).isEmpty();
assertThat(query.assignees()).isEmpty(); assertThat(query.assignees()).isEmpty();
Expand Down
Expand Up @@ -176,41 +176,52 @@ public void filter_by_keys() throws Exception {
} }


@Test @Test
public void filter_by_component_roots() throws Exception { public void filter_by_modules() throws Exception {
ComponentDto module = ComponentTesting.newModuleDto(project); ComponentDto module = ComponentTesting.newModuleDto(project);
ComponentDto subModule = ComponentTesting.newModuleDto(module); ComponentDto subModule = ComponentTesting.newModuleDto(module);
ComponentDto file = ComponentTesting.newFileDto(subModule); ComponentDto file = ComponentTesting.newFileDto(subModule);
tester.get(ComponentDao.class).insert(session, module, subModule, file); tester.get(ComponentDao.class).insert(session, module, subModule, file);


db.issueDao().insert(session, db.issueDao().insert(session,
IssueTesting.newDto(rule, module, project),
IssueTesting.newDto(rule, subModule, project),
IssueTesting.newDto(rule, file, project)); IssueTesting.newDto(rule, file, project));
session.commit(); session.commit();
index(); index();


assertThat(index.search(IssueQuery.builder().componentRootUuids(newArrayList(file.uuid())).build(), new QueryContext()).getHits()).isEmpty(); assertThat(index.search(IssueQuery.builder().moduleUuids(newArrayList(file.uuid())).build(), new QueryContext()).getHits()).isEmpty();
assertThat(index.search(IssueQuery.builder().componentRootUuids(newArrayList(module.uuid())).build(), new QueryContext()).getHits()).hasSize(1); assertThat(index.search(IssueQuery.builder().moduleUuids(newArrayList(module.uuid())).build(), new QueryContext()).getHits()).hasSize(1);
assertThat(index.search(IssueQuery.builder().componentRootUuids(newArrayList(subModule.uuid())).build(), new QueryContext()).getHits()).hasSize(1); assertThat(index.search(IssueQuery.builder().moduleUuids(newArrayList(subModule.uuid())).build(), new QueryContext()).getHits()).hasSize(1);
// TODO assertThat(index.search(IssueQuery.builder().componentRootUuids(newArrayList(project.uuid())).build(), new assertThat(index.search(IssueQuery.builder().moduleUuids(newArrayList(project.uuid())).build(), new QueryContext()).getHits()).hasSize(1);
// QueryContext()).getHits()).hasSize(1); assertThat(index.search(IssueQuery.builder().moduleUuids(newArrayList("unknown")).build(), new QueryContext()).getHits()).isEmpty();
assertThat(index.search(IssueQuery.builder().componentRootUuids(newArrayList("unknown")).build(), new QueryContext()).getHits()).isEmpty();
} }


@Test @Test
public void filter_by_components() throws Exception { public void filter_by_components() throws Exception {
ComponentDto module = ComponentTesting.newModuleDto(project);
ComponentDto subModule = ComponentTesting.newModuleDto(module);
ComponentDto file1 = ComponentTesting.newFileDto(project); ComponentDto file1 = ComponentTesting.newFileDto(project);
ComponentDto file2 = ComponentTesting.newFileDto(project); ComponentDto file2 = ComponentTesting.newFileDto(module);
tester.get(ComponentDao.class).insert(session, file1, file2); ComponentDto file3 = ComponentTesting.newFileDto(subModule);
tester.get(ComponentDao.class).insert(session, module, subModule, file1, file2, file3);


db.issueDao().insert(session, db.issueDao().insert(session,
IssueTesting.newDto(rule, project, project),
IssueTesting.newDto(rule, file1, project), IssueTesting.newDto(rule, file1, project),
IssueTesting.newDto(rule, file2, project)); IssueTesting.newDto(rule, module, project),
IssueTesting.newDto(rule, file2, project),
IssueTesting.newDto(rule, subModule, project),
IssueTesting.newDto(rule, file3, project));
session.commit(); session.commit();
index(); index();


assertThat(index.search(IssueQuery.builder().componentUuids(newArrayList(file1.uuid(), file2.uuid())).build(), new QueryContext()).getHits()).hasSize(2); assertThat(index.search(IssueQuery.builder().componentUuids(newArrayList(file1.uuid(), file2.uuid(), file3.uuid())).build(), new QueryContext()).getHits()).hasSize(3);
assertThat(index.search(IssueQuery.builder().componentUuids(newArrayList(file1.uuid())).build(), new QueryContext()).getHits()).hasSize(1); assertThat(index.search(IssueQuery.builder().componentUuids(newArrayList(file1.uuid())).build(), new QueryContext()).getHits()).hasSize(1);
assertThat(index.search(IssueQuery.builder().componentUuids(newArrayList(subModule.uuid())).build(), new QueryContext()).getHits()).hasSize(2);
assertThat(index.search(IssueQuery.builder().componentUuids(newArrayList(subModule.uuid())).onComponentOnly(true).build(), new QueryContext()).getHits()).hasSize(1);
assertThat(index.search(IssueQuery.builder().componentUuids(newArrayList(module.uuid())).build(), new QueryContext()).getHits()).hasSize(4);
assertThat(index.search(IssueQuery.builder().componentUuids(newArrayList(module.uuid())).onComponentOnly(true).build(), new QueryContext()).getHits()).hasSize(1);
assertThat(index.search(IssueQuery.builder().componentUuids(newArrayList("unknown")).build(), new QueryContext()).getHits()).isEmpty(); assertThat(index.search(IssueQuery.builder().componentUuids(newArrayList("unknown")).build(), new QueryContext()).getHits()).isEmpty();
assertThat(index.search(IssueQuery.builder().componentUuids(newArrayList(project.uuid())).build(), new QueryContext()).getHits()).isEmpty();
} }


@Test @Test
Expand Down Expand Up @@ -663,7 +674,7 @@ public void authorized_issues_on_groups() throws Exception {
assertThat(index.search(query.build(), new QueryContext()).getHits()).hasSize(0); assertThat(index.search(query.build(), new QueryContext()).getHits()).hasSize(0);


MockUserSession.set().setUserGroups("sonar-users", "sonar-admins"); MockUserSession.set().setUserGroups("sonar-users", "sonar-admins");
assertThat(index.search(query.componentRootUuids(newArrayList(project3.key())).build(), new QueryContext()).getHits()).hasSize(0); assertThat(index.search(query.moduleUuids(newArrayList(project3.key())).build(), new QueryContext()).getHits()).hasSize(0);
} }


@Test @Test
Expand Down Expand Up @@ -711,7 +722,7 @@ public void authorized_issues_on_user() throws Exception {
assertThat(index.search(query.build(), new QueryContext()).getHits()).hasSize(0); assertThat(index.search(query.build(), new QueryContext()).getHits()).hasSize(0);


MockUserSession.set().setLogin("john"); MockUserSession.set().setLogin("john");
assertThat(index.search(query.componentRootUuids(newArrayList(project3.key())).build(), new QueryContext()).getHits()).hasSize(0); assertThat(index.search(query.moduleUuids(newArrayList(project3.key())).build(), new QueryContext()).getHits()).hasSize(0);
} }


@Test @Test
Expand Down

0 comments on commit 928ab17

Please sign in to comment.