SONAR-5954 Fix backward compatibility of componentRoots parameter

diff --git a/server/sonar-server/src/main/java/org/sonar/server/issue/IssueQueryService.java b/server/sonar-server/src/main/java/org/sonar/server/issue/IssueQueryService.java
index d5d6473e40aaaf96c2b3497ac66529c1b58d90e6..d321211288578b4d6926117b4f1ea1cc5ff75e2b 100644 (file)
--- a/server/sonar-server/src/main/java/org/sonar/server/issue/IssueQueryService.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/issue/IssueQueryService.java
@@ -26,6 +26,7 @@ import com.google.common.base.Splitter;
 import com.google.common.base.Strings;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.Lists;
+import com.google.common.collect.Sets;
 import org.apache.commons.lang.ObjectUtils;
 import org.sonar.api.ServerComponent;
 import org.sonar.api.rule.RuleKey;
@@ -43,6 +44,7 @@ import javax.annotation.Nullable;
 import java.util.Collection;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 
 import static com.google.common.collect.Lists.newArrayList;
 
@@ -96,20 +98,12 @@ public class IssueQueryService implements ServerComponent {
             params.get(IssueFilterParameters.COMPONENT_KEYS),
             params.get(IssueFilterParameters.COMPONENTS)
             )
-          ));
-      addComponentRootUuids(builder, session,
-        RubyUtils.toStrings(
-          ObjectUtils.defaultIfNull(
-            params.get(IssueFilterParameters.MODULE_UUIDS),
-            params.get(IssueFilterParameters.COMPONENT_ROOT_UUIDS)
-            )
           ),
-        RubyUtils.toStrings(
-          ObjectUtils.defaultIfNull(
-            params.get(IssueFilterParameters.MODULE_KEYS),
-            params.get(IssueFilterParameters.COMPONENT_ROOTS)
-            )
-          ));
+        RubyUtils.toStrings(params.get(IssueFilterParameters.COMPONENT_ROOT_UUIDS)),
+        RubyUtils.toStrings(params.get(IssueFilterParameters.COMPONENT_ROOTS)));
+      addModuleUuids(builder, session,
+        RubyUtils.toStrings(params.get(IssueFilterParameters.MODULE_UUIDS)),
+        RubyUtils.toStrings(params.get(IssueFilterParameters.MODULE_KEYS)));
       String sort = (String) params.get(IssueFilterParameters.SORT);
       if (!Strings.isNullOrEmpty(sort)) {
         builder.sort(sort);
@@ -151,8 +145,9 @@ public class IssueQueryService implements ServerComponent {
       addProjectUuids(builder, session,
         request.paramAsStrings(IssueFilterParameters.PROJECT_UUIDS), request.paramAsStrings(IssueFilterParameters.PROJECT_KEYS));
       addComponentUuids(builder, session,
-        request.paramAsStrings(IssueFilterParameters.COMPONENT_UUIDS), request.paramAsStrings(IssueFilterParameters.COMPONENT_KEYS));
-      addComponentRootUuids(builder, session,
+        request.paramAsStrings(IssueFilterParameters.COMPONENT_UUIDS), request.paramAsStrings(IssueFilterParameters.COMPONENT_KEYS),
+        request.paramAsStrings(IssueFilterParameters.COMPONENT_ROOT_UUIDS), request.paramAsStrings(IssueFilterParameters.COMPONENT_ROOTS));
+      addModuleUuids(builder, session,
         request.paramAsStrings(IssueFilterParameters.MODULE_UUIDS), request.paramAsStrings(IssueFilterParameters.MODULE_KEYS));
       String sort = request.param(SearchRequestHandler.PARAM_SORT);
       if (!Strings.isNullOrEmpty(sort)) {
@@ -177,18 +172,31 @@ public class IssueQueryService implements ServerComponent {
     }
   }
 
-  private void addComponentUuids(IssueQuery.Builder builder, DbSession session, @Nullable Collection<String> componentUuids, @Nullable Collection<String> components) {
-    if (componentUuids != null) {
-      if (components != null) {
+  private void addComponentUuids(IssueQuery.Builder builder, DbSession session,
+    @Nullable Collection<String> componentUuids, @Nullable Collection<String> components,
+    /*
+     * Since 5.1, search of issues is recursive by default (module + submodules),
+     * but "componentKeys" parameter already deprecates "components" parameter,
+     * so queries specifying "componentRoots" must be handled manually
+     */
+    @Nullable Collection<String> componentRootUuids, @Nullable Collection<String> componentRoots) {
+    if (componentUuids != null || componentRootUuids != null) {
+      if (components != null || componentRoots != null) {
         throw new IllegalArgumentException("components and componentUuids cannot be set simultaneously");
       }
-      builder.componentUuids(componentUuids);
+      Set<String> allComponentUuids = Sets.newHashSet();
+      allComponentUuids.addAll((Collection<String>) ObjectUtils.defaultIfNull(componentUuids, Sets.newHashSet()));
+      allComponentUuids.addAll((Collection<String>) ObjectUtils.defaultIfNull(componentRootUuids, Sets.newHashSet()));
+      builder.componentUuids(allComponentUuids);
     } else {
-      builder.componentUuids(componentUuids(session, components));
+      Set<String> allComponents = Sets.newHashSet();
+      allComponents.addAll((Collection<String>) ObjectUtils.defaultIfNull(components, Sets.newHashSet()));
+      allComponents.addAll((Collection<String>) ObjectUtils.defaultIfNull(componentRoots, Sets.newHashSet()));
+      builder.componentUuids(componentUuids(session, allComponents));
     }
   }
 
-  private void addComponentRootUuids(IssueQuery.Builder builder, DbSession session, @Nullable Collection<String> componentRootUuids, @Nullable Collection<String> componentRoots) {
+  private void addModuleUuids(IssueQuery.Builder builder, DbSession session, @Nullable Collection<String> componentRootUuids, @Nullable Collection<String> componentRoots) {
     if (componentRootUuids != null) {
       if (componentRoots != null) {
         throw new IllegalArgumentException("componentRoots and componentRootUuids cannot be set simultaneously");

diff --git a/server/sonar-server/src/main/java/org/sonar/server/issue/ws/SearchAction.java b/server/sonar-server/src/main/java/org/sonar/server/issue/ws/SearchAction.java
index 30ef6ae512b8bc01e52eb77935f7826e6bea4193..d21653b59555ea1d2065e175f639d7199143c2b4 100644 (file)
--- a/server/sonar-server/src/main/java/org/sonar/server/issue/ws/SearchAction.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/issue/ws/SearchAction.java
@@ -233,13 +233,13 @@ public class SearchAction extends SearchRequestHandler<IssueQuery, Issue> {
       .setExampleValue("7d8749e8-3070-4903-9188-bdd82933bb92");
 
     action.createParam(IssueFilterParameters.COMPONENT_ROOTS)
-      .setDescription("Deprecated since 5.1. See moduleKeys.");
+      .setDescription("Deprecated since 5.1. Use componentKeys instead, with onComponentOnly=false.");
     action.createParam(IssueFilterParameters.COMPONENT_ROOT_UUIDS)
-      .setDescription("Deprecated since 5.1. See moduleUuids.");
+      .setDescription("Deprecated since 5.1. Use componentUuids instead, with onComponentOnly=false.");
     action.createParam(IssueFilterParameters.MODULE_KEYS)
       .setDescription("To retrieve issues associated to a specific list of modules (comma-separated list of module keys). " +
         INTERNAL_PARAMETER_DISCLAIMER +
-        "Views are not supported. If this parameter is set, componentRootUuids must not be set.")
+        "Views are not supported. If this parameter is set, moduleUuids must not be set.")
       .setDeprecatedKey(IssueFilterParameters.COMPONENT_ROOTS)
       .setExampleValue("org.apache.struts:struts");
     action.createParam(IssueFilterParameters.MODULE_UUIDS)

diff --git a/server/sonar-server/src/test/java/org/sonar/server/issue/IssueQueryServiceTest.java b/server/sonar-server/src/test/java/org/sonar/server/issue/IssueQueryServiceTest.java
index bc1bb2f7fe39c118403a1a0f19851160c6afafd6..673b911e83fbc4c0385e8cf42d6c0e0c67c06843 100644 (file)
--- a/server/sonar-server/src/test/java/org/sonar/server/issue/IssueQueryServiceTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/issue/IssueQueryServiceTest.java
@@ -23,6 +23,7 @@ package org.sonar.server.issue;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
+import org.mockito.Matchers;
 import org.mockito.Mock;
 import org.mockito.invocation.InvocationOnMock;
 import org.mockito.runners.MockitoJUnitRunner;
@@ -92,7 +93,7 @@ public class IssueQueryServiceTest {
     ArrayList<String> componentKeys = newArrayList("org.apache");
     map.put("components", componentKeys);
     ArrayList<String> moduleKeys = newArrayList("org.sonar");
-    map.put("componentRoots", moduleKeys);
+    map.put("moduleKeys", moduleKeys);
     map.put("reporters", newArrayList("marilyn"));
     map.put("assignees", newArrayList("joanna"));
     map.put("languages", newArrayList("xoo"));
@@ -107,8 +108,21 @@ public class IssueQueryServiceTest {
     map.put("sort", "CREATION_DATE");
     map.put("asc", true);
 
-    when(componentService.componentUuids(session, componentKeys, true)).thenReturn(newArrayList("ABCD"));
-    when(componentService.componentUuids(session, moduleKeys, true)).thenReturn(newArrayList("BCDE"));
+    when(componentService.componentUuids(eq(session), Matchers.anyCollection(), eq(true))).thenAnswer(new Answer<Collection<String>>() {
+      @Override
+      public Collection<String> answer(InvocationOnMock invocation) throws Throwable {
+        Collection<String> components = (Collection<String>) invocation.getArguments()[1];
+        if (components == null) {
+          return newArrayList();
+        }
+        if (components.contains("org.apache")) {
+          return newArrayList("ABCD");
+        } else if (components.contains("org.sonar")) {
+          return newArrayList("BCDE");
+        }
+        return newArrayList();
+      }
+    });
 
     IssueQuery query = issueQueryService.createFromMap(map);
     assertThat(query.issueKeys()).containsOnly("ABCDE1234");
@@ -139,7 +153,12 @@ public class IssueQueryServiceTest {
     ArrayList<String> componentKeys = newArrayList("unknown");
     map.put("components", componentKeys);
 
-    when(componentService.componentUuids(any(DbSession.class), eq(componentKeys), eq(true))).thenReturn(Arrays.<String>asList());
+    when(componentService.componentUuids(eq(session), Matchers.anyCollection(), eq(true))).thenAnswer(new Answer<Collection<String>>() {
+      @Override
+      public Collection<String> answer(InvocationOnMock invocation) throws Throwable {
+        return newArrayList();
+      }
+    });
 
     IssueQuery query = issueQueryService.createFromMap(map);
     assertThat(query.componentUuids()).containsOnly("<UNKNOWN>");
@@ -193,7 +212,7 @@ public class IssueQueryServiceTest {
       issueQueryService.createFromMap(map);
       fail();
     } catch (Exception e) {
-      assertThat(e).isInstanceOf(IllegalArgumentException.class).hasMessage("componentRoots and componentRootUuids cannot be set simultaneously");
+      assertThat(e).isInstanceOf(IllegalArgumentException.class).hasMessage("components and componentUuids cannot be set simultaneously");
     }
   }