From 22600d84f370f18b3050e2e06eec9d9975117487 Mon Sep 17 00:00:00 2001 From: Simon Brandhof Date: Thu, 30 Mar 2017 20:13:38 +0200 Subject: [PATCH] SONAR-9052 Refactor IssueQueryFactory - stop using ComponentService - improve readability - stop loading directories twice - remove usage of Guava Predicate - remove ComponentService from Compute Engine --- .../container/ComputeEngineContainerImpl.java | 2 - .../ComputeEngineContainerImplTest.java | 2 +- .../server/component/ComponentService.java | 55 ------ .../org/sonar/server/issue/IssueQuery.java | 46 ----- .../sonar/server/issue/IssueQueryFactory.java | 179 +++++------------ .../issue/index/IssueIndexDefinition.java | 2 +- .../server/issue/ws/ComponentTagsAction.java | 19 +- .../sonar/server/issue/ws/SearchAction.java | 3 +- .../component/ComponentServiceTest.java | 10 - .../server/issue/IssueQueryFactoryTest.java | 180 +++++++++--------- .../sonar/server/issue/IssueQueryTest.java | 2 - .../issue/ws/ComponentTagsActionTest.java | 85 ++++----- .../server/issue/ws/SearchActionTest.java | 2 +- 13 files changed, 188 insertions(+), 399 deletions(-) diff --git a/server/sonar-ce/src/main/java/org/sonar/ce/container/ComputeEngineContainerImpl.java b/server/sonar-ce/src/main/java/org/sonar/ce/container/ComputeEngineContainerImpl.java index 3b05ff5ee5c..d0600dc7e0d 100644 --- a/server/sonar-ce/src/main/java/org/sonar/ce/container/ComputeEngineContainerImpl.java +++ b/server/sonar-ce/src/main/java/org/sonar/ce/container/ComputeEngineContainerImpl.java @@ -67,7 +67,6 @@ import org.sonar.process.Props; import org.sonar.process.logging.LogbackHelper; import org.sonar.server.component.ComponentCleanerService; import org.sonar.server.component.ComponentFinder; -import org.sonar.server.component.ComponentService; import org.sonar.server.component.index.ComponentIndexer; import org.sonar.server.computation.queue.PurgeCeActivities; import org.sonar.server.computation.task.projectanalysis.ProjectAnalysisTaskModule; @@ -329,7 +328,6 @@ public class ComputeEngineContainerImpl implements ComputeEngineContainer { // components ComponentFinder.class, // used in ComponentService - ComponentService.class, // used in ReportSubmitter NewAlerts.class, NewAlerts.newMetadata(), ComponentCleanerService.class, diff --git a/server/sonar-ce/src/test/java/org/sonar/ce/container/ComputeEngineContainerImplTest.java b/server/sonar-ce/src/test/java/org/sonar/ce/container/ComputeEngineContainerImplTest.java index 16e2c381e7c..9f11d1c9c6b 100644 --- a/server/sonar-ce/src/test/java/org/sonar/ce/container/ComputeEngineContainerImplTest.java +++ b/server/sonar-ce/src/test/java/org/sonar/ce/container/ComputeEngineContainerImplTest.java @@ -88,7 +88,7 @@ public class ComputeEngineContainerImplTest { assertThat(picoContainer.getComponentAdapters()) .hasSize( CONTAINER_ITSELF - + 74 // level 4 + + 73 // level 4 + 4 // content of CeConfigurationModule + 5 // content of CeQueueModule + 3 // content of CeHttpModule diff --git a/server/sonar-server/src/main/java/org/sonar/server/component/ComponentService.java b/server/sonar-server/src/main/java/org/sonar/server/component/ComponentService.java index 02642279a5c..a3ad4ebd213 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/component/ComponentService.java +++ b/server/sonar-server/src/main/java/org/sonar/server/component/ComponentService.java @@ -19,30 +19,19 @@ */ package org.sonar.server.component; -import com.google.common.base.Joiner; -import com.google.common.collect.Collections2; -import com.google.common.collect.Sets; -import java.util.Collection; -import java.util.List; -import java.util.Set; -import javax.annotation.Nullable; -import org.sonar.api.ce.ComputeEngineSide; import org.sonar.api.server.ServerSide; import org.sonar.api.web.UserRole; import org.sonar.db.DbClient; import org.sonar.db.DbSession; import org.sonar.db.component.ComponentDto; import org.sonar.server.es.ProjectIndexer; -import org.sonar.server.exceptions.NotFoundException; import org.sonar.server.user.UserSession; -import static com.google.common.collect.Lists.newArrayList; import static org.sonar.core.component.ComponentKeys.isValidModuleKey; import static org.sonar.db.component.ComponentKeyUpdaterDao.checkIsProjectOrModule; import static org.sonar.server.ws.WsUtils.checkRequest; @ServerSide -@ComputeEngineSide public class ComponentService { private final DbClient dbClient; private final UserSession userSession; @@ -77,50 +66,6 @@ public class ComponentService { } } - public Collection componentUuids(DbSession session, @Nullable Collection componentKeys, boolean ignoreMissingComponents) { - Collection componentUuids = newArrayList(); - if (componentKeys != null && !componentKeys.isEmpty()) { - List components = dbClient.componentDao().selectByKeys(session, componentKeys); - - if (!ignoreMissingComponents && components.size() < componentKeys.size()) { - Collection foundKeys = Collections2.transform(components, ComponentDto::getKey); - Set missingKeys = Sets.newHashSet(componentKeys); - missingKeys.removeAll(foundKeys); - throw new NotFoundException("The following component keys do not match any component:\n" + - Joiner.on('\n').join(missingKeys)); - } - - for (ComponentDto component : components) { - componentUuids.add(component.uuid()); - } - } - return componentUuids; - } - - public Set getDistinctQualifiers(DbSession session, @Nullable Collection componentUuids) { - Set componentQualifiers = Sets.newHashSet(); - if (componentUuids != null && !componentUuids.isEmpty()) { - List components = dbClient.componentDao().selectByUuids(session, componentUuids); - - for (ComponentDto component : components) { - componentQualifiers.add(component.qualifier()); - } - } - return componentQualifiers; - } - - public Collection getByUuids(DbSession session, @Nullable Collection componentUuids) { - Set directoryPaths = Sets.newHashSet(); - if (componentUuids != null && !componentUuids.isEmpty()) { - List components = dbClient.componentDao().selectByUuids(session, componentUuids); - - for (ComponentDto component : components) { - directoryPaths.add(component); - } - } - return directoryPaths; - } - private static void checkProjectOrModuleKeyFormat(String key) { checkRequest(isValidModuleKey(key), "Malformed key for '%s'. Allowed characters are alphanumeric, '-', '_', '.' and ':', with at least one non-digit.", key); } diff --git a/server/sonar-server/src/main/java/org/sonar/server/issue/IssueQuery.java b/server/sonar-server/src/main/java/org/sonar/server/issue/IssueQuery.java index ccd79145b12..be38d598ec9 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/issue/IssueQuery.java +++ b/server/sonar-server/src/main/java/org/sonar/server/issue/IssueQuery.java @@ -72,8 +72,6 @@ public class IssueQuery { private final Boolean onComponentOnly; private final Boolean assigned; private final Boolean resolved; - private final Boolean hideRules; - private final Boolean hideComments; private final Date createdAt; private final Date createdAfter; private final Date createdBefore; @@ -104,8 +102,6 @@ public class IssueQuery { this.onComponentOnly = builder.onComponentOnly; this.assigned = builder.assigned; this.resolved = builder.resolved; - this.hideRules = builder.hideRules; - this.hideComments = builder.hideComments; this.createdAt = builder.createdAt; this.createdAfter = builder.createdAfter; this.createdBefore = builder.createdBefore; @@ -199,22 +195,6 @@ public class IssueQuery { return resolved; } - /** - * @since 4.2 - */ - @CheckForNull - public Boolean hideRules() { - return hideRules; - } - - /** - * @since 5.1 - */ - @CheckForNull - public Boolean hideComments() { - return hideComments; - } - @CheckForNull public Date createdAfter() { return createdAfter == null ? null : new Date(createdAfter.getTime()); @@ -283,8 +263,6 @@ public class IssueQuery { private Boolean onComponentOnly = false; private Boolean assigned = null; private Boolean resolved = null; - private Boolean hideRules = false; - private Boolean hideComments = false; private Date createdAt; private Date createdAfter; private Date createdBefore; @@ -410,30 +388,6 @@ public class IssueQuery { return this; } - /** - * If true, rules will not be loaded - * If false, rules will be loaded - * - * @since 4.2 - * - */ - public Builder hideRules(@Nullable Boolean b) { - this.hideRules = b; - return this; - } - - /** - * If true, comments will not be loaded - * If false, comments will be loaded - * - * @since 5.1 - * - */ - public Builder hideComments(@Nullable Boolean b) { - this.hideComments = b; - return this; - } - public Builder createdAt(@Nullable Date d) { this.createdAt = d == null ? null : new Date(d.getTime()); return this; diff --git a/server/sonar-server/src/main/java/org/sonar/server/issue/IssueQueryFactory.java b/server/sonar-server/src/main/java/org/sonar/server/issue/IssueQueryFactory.java index a4e032ca448..1d18c7731f8 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/issue/IssueQueryFactory.java +++ b/server/sonar-server/src/main/java/org/sonar/server/issue/IssueQueryFactory.java @@ -21,27 +21,23 @@ package org.sonar.server.issue; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Joiner; -import com.google.common.base.Predicate; import com.google.common.base.Splitter; import com.google.common.base.Strings; import com.google.common.collect.Collections2; import com.google.common.collect.Lists; import com.google.common.collect.Sets; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.Date; import java.util.List; import java.util.Locale; -import java.util.Map; import java.util.Objects; import java.util.Optional; import java.util.Set; -import java.util.concurrent.atomic.AtomicInteger; import javax.annotation.CheckForNull; -import javax.annotation.Nonnull; import javax.annotation.Nullable; import org.apache.commons.lang.BooleanUtils; -import org.apache.commons.lang.ObjectUtils; import org.joda.time.DateTime; import org.joda.time.format.ISOPeriodFormat; import org.sonar.api.resources.Qualifiers; @@ -49,22 +45,19 @@ import org.sonar.api.rule.RuleKey; import org.sonar.api.server.ServerSide; import org.sonar.api.utils.System2; import org.sonar.api.web.UserRole; +import org.sonar.core.util.stream.Collectors; import org.sonar.db.DbClient; import org.sonar.db.DbSession; import org.sonar.db.component.ComponentDto; import org.sonar.db.component.SnapshotDto; import org.sonar.db.organization.OrganizationDto; -import org.sonar.server.component.ComponentService; import org.sonar.server.user.UserSession; -import org.sonar.server.util.RubyUtils; -import org.sonarqube.ws.client.issue.IssuesWsParameters; import org.sonarqube.ws.client.issue.SearchWsRequest; import static com.google.common.base.Preconditions.checkArgument; -import static com.google.common.base.Predicates.notNull; -import static com.google.common.collect.FluentIterable.from; import static com.google.common.collect.Lists.newArrayList; import static java.lang.String.format; +import static java.util.Collections.singletonList; import static org.sonar.api.utils.DateUtils.longToDate; import static org.sonar.api.utils.DateUtils.parseDateOrDateTime; import static org.sonar.api.utils.DateUtils.parseEndingDateOrDateTime; @@ -89,88 +82,16 @@ public class IssueQueryFactory { private static final String UNKNOWN = ""; private final DbClient dbClient; - private final ComponentService componentService; private final System2 system; private final UserSession userSession; - public IssueQueryFactory(DbClient dbClient, ComponentService componentService, System2 system, UserSession userSession) { + public IssueQueryFactory(DbClient dbClient, System2 system, UserSession userSession) { this.dbClient = dbClient; - this.componentService = componentService; this.system = system; this.userSession = userSession; } - public IssueQuery createFromMap(Map params) { - try (DbSession dbSession = dbClient.openSession(false)) { - IssueQuery.Builder builder = IssueQuery.builder() - .issueKeys(RubyUtils.toStrings(params.get(IssuesWsParameters.PARAM_ISSUES))) - .severities(RubyUtils.toStrings(params.get(IssuesWsParameters.PARAM_SEVERITIES))) - .statuses(RubyUtils.toStrings(params.get(IssuesWsParameters.PARAM_STATUSES))) - .resolutions(RubyUtils.toStrings(params.get(IssuesWsParameters.PARAM_RESOLUTIONS))) - .resolved(RubyUtils.toBoolean(params.get(IssuesWsParameters.PARAM_RESOLVED))) - .rules(toRules(params.get(IssuesWsParameters.PARAM_RULES))) - .assignees(buildAssignees(RubyUtils.toStrings(params.get(IssuesWsParameters.PARAM_ASSIGNEES)))) - .languages(RubyUtils.toStrings(params.get(IssuesWsParameters.PARAM_LANGUAGES))) - .tags(RubyUtils.toStrings(params.get(IssuesWsParameters.PARAM_TAGS))) - .types(RubyUtils.toStrings(params.get(IssuesWsParameters.PARAM_TYPES))) - .assigned(RubyUtils.toBoolean(params.get(IssuesWsParameters.PARAM_ASSIGNED))) - .hideRules(RubyUtils.toBoolean(params.get(IssuesWsParameters.PARAM_HIDE_RULES))) - .createdAt(RubyUtils.toDate(params.get(IssuesWsParameters.PARAM_CREATED_AT))) - .createdAfter(buildCreatedAfterFromDates(RubyUtils.toDate(params.get(PARAM_CREATED_AFTER)), (String) params.get(PARAM_CREATED_IN_LAST))) - .createdBefore(RubyUtils.toDate(parseEndingDateOrDateTime((String) params.get(IssuesWsParameters.PARAM_CREATED_BEFORE)))) - .organizationUuid(convertOrganizationKeyToUuid(dbSession, (String) params.get(IssuesWsParameters.PARAM_ORGANIZATION))); - - Set allComponentUuids = Sets.newHashSet(); - boolean effectiveOnComponentOnly = mergeDeprecatedComponentParameters(dbSession, - RubyUtils.toBoolean(params.get(IssuesWsParameters.PARAM_ON_COMPONENT_ONLY)), - RubyUtils.toStrings(params.get(IssuesWsParameters.PARAM_COMPONENTS)), - RubyUtils.toStrings(params.get(IssuesWsParameters.PARAM_COMPONENT_UUIDS)), - RubyUtils.toStrings(params.get(IssuesWsParameters.PARAM_COMPONENT_KEYS)), - RubyUtils.toStrings(params.get(IssuesWsParameters.PARAM_COMPONENT_ROOT_UUIDS)), - RubyUtils.toStrings(params.get(IssuesWsParameters.PARAM_COMPONENT_ROOTS)), - allComponentUuids); - - addComponentParameters(builder, dbSession, - effectiveOnComponentOnly, - allComponentUuids, - RubyUtils.toStrings(params.get(IssuesWsParameters.PARAM_PROJECT_UUIDS)), - RubyUtils.toStrings( - ObjectUtils.defaultIfNull( - params.get(IssuesWsParameters.PARAM_PROJECT_KEYS), - params.get(IssuesWsParameters.PARAM_PROJECTS))), - RubyUtils.toStrings(params.get(IssuesWsParameters.PARAM_MODULE_UUIDS)), - RubyUtils.toStrings(params.get(IssuesWsParameters.PARAM_DIRECTORIES)), - RubyUtils.toStrings(params.get(IssuesWsParameters.PARAM_FILE_UUIDS)), - RubyUtils.toStrings(params.get(IssuesWsParameters.PARAM_AUTHORS))); - - String sort = (String) params.get(IssuesWsParameters.PARAM_SORT); - if (!Strings.isNullOrEmpty(sort)) { - builder.sort(sort); - builder.asc(RubyUtils.toBoolean(params.get(IssuesWsParameters.PARAM_ASC))); - } - String facetMode = (String) params.get(IssuesWsParameters.FACET_MODE); - if (!Strings.isNullOrEmpty(facetMode)) { - builder.facetMode(facetMode); - } else { - builder.facetMode(IssuesWsParameters.FACET_MODE_COUNT); - } - return builder.build(); - } - } - - @CheckForNull - private Date buildCreatedAfterFromDates(@Nullable Date createdAfter, @Nullable String createdInLast) { - checkArgument(createdAfter == null || createdInLast == null, format("%s and %s cannot be set simultaneously", PARAM_CREATED_AFTER, PARAM_CREATED_IN_LAST)); - - Date actualCreatedAfter = createdAfter; - if (createdInLast != null) { - actualCreatedAfter = new DateTime(system.now()).minus( - ISOPeriodFormat.standard().parsePeriod("P" + createdInLast.toUpperCase(Locale.ENGLISH))).toDate(); - } - return actualCreatedAfter; - } - - public IssueQuery createFromRequest(SearchWsRequest request) { + public IssueQuery create(SearchWsRequest request) { try (DbSession dbSession = dbClient.openSession(false)) { IssueQuery.Builder builder = IssueQuery.builder() .issueKeys(request.getIssues()) @@ -221,6 +142,18 @@ public class IssueQueryFactory { } } + @CheckForNull + private Date buildCreatedAfterFromDates(@Nullable Date createdAfter, @Nullable String createdInLast) { + checkArgument(createdAfter == null || createdInLast == null, format("Parameters %s and %s cannot be set simultaneously", PARAM_CREATED_AFTER, PARAM_CREATED_IN_LAST)); + + Date actualCreatedAfter = createdAfter; + if (createdInLast != null) { + actualCreatedAfter = new DateTime(system.now()).minus( + ISOPeriodFormat.standard().parsePeriod("P" + createdInLast.toUpperCase(Locale.ENGLISH))).toDate(); + } + return actualCreatedAfter; + } + @CheckForNull private String convertOrganizationKeyToUuid(DbSession dbSession, @Nullable String organizationKey) { if (organizationKey == null) { @@ -238,7 +171,7 @@ public class IssueQueryFactory { return buildCreatedAfterFromDates(createdAfter, createdInLast); } - checkRequest(createdAfter == null, "'%s' and '%s' cannot be set simultaneously", PARAM_CREATED_AFTER, PARAM_SINCE_LEAK_PERIOD); + checkRequest(createdAfter == null, "Parameters '%s' and '%s' cannot be set simultaneously", PARAM_CREATED_AFTER, PARAM_SINCE_LEAK_PERIOD); checkArgument(componentUuids.size() == 1, "One and only one component must be provided when searching since leak period"); String uuid = componentUuids.iterator().next(); @@ -286,31 +219,31 @@ public class IssueQueryFactory { allComponentUuids.addAll(componentRootUuids); effectiveOnComponentOnly = false; } else if (componentRoots != null) { - allComponentUuids.addAll(componentUuids(session, componentRoots)); + allComponentUuids.addAll(convertComponentKeysToUuids(session, componentRoots)); effectiveOnComponentOnly = false; } else if (components != null) { - allComponentUuids.addAll(componentUuids(session, components)); + allComponentUuids.addAll(convertComponentKeysToUuids(session, components)); effectiveOnComponentOnly = true; } else if (componentUuids != null) { allComponentUuids.addAll(componentUuids); effectiveOnComponentOnly = BooleanUtils.isTrue(onComponentOnly); } else if (componentKeys != null) { - allComponentUuids.addAll(componentUuids(session, componentKeys)); + allComponentUuids.addAll(convertComponentKeysToUuids(session, componentKeys)); effectiveOnComponentOnly = BooleanUtils.isTrue(onComponentOnly); } return effectiveOnComponentOnly; } private static boolean atMostOneNonNullElement(Object... objects) { - return !from(Arrays.asList(objects)) - .filter(notNull()) - .anyMatch(new HasTwoOrMoreElements()); + return Arrays.stream(objects) + .filter(Objects::nonNull) + .count() <= 1; } private void addComponentParameters(IssueQuery.Builder builder, DbSession session, boolean onComponentOnly, Collection componentUuids, - @Nullable Collection projectUuids, @Nullable Collection projects, + @Nullable Collection projectUuids, @Nullable Collection projectKeys, @Nullable Collection moduleUuids, @Nullable Collection directories, @Nullable Collection fileUuids, @@ -323,11 +256,11 @@ public class IssueQueryFactory { } builder.authors(authors); - checkArgument(projectUuids == null || projects == null, "projects and projectUuids cannot be set simultaneously"); + checkArgument(projectUuids == null || projectKeys == null, "Parameters projects and projectUuids cannot be set simultaneously"); if (projectUuids != null) { builder.projectUuids(projectUuids); - } else { - builder.projectUuids(componentUuids(session, projects)); + } else if (projectKeys != null) { + builder.projectUuids(convertComponentKeysToUuids(session, projectKeys)); } builder.moduleUuids(moduleUuids); builder.directories(directories); @@ -339,21 +272,27 @@ public class IssueQueryFactory { } private void addComponentsBasedOnQualifier(IssueQuery.Builder builder, DbSession session, Collection componentUuids) { - Set qualifiers = componentService.getDistinctQualifiers(session, componentUuids); - if (qualifiers.isEmpty()) { - // Qualifier not found, defaulting to componentUuids (e.g ) + if (componentUuids.isEmpty()) { builder.componentUuids(componentUuids); return; } + + List components = dbClient.componentDao().selectByUuids(session, componentUuids); + if (components.isEmpty()) { + builder.componentUuids(componentUuids); + return; + } + + Set qualifiers = components.stream().map(ComponentDto::qualifier).collect(Collectors.toHashSet()); if (qualifiers.size() > 1) { throw new IllegalArgumentException("All components must have the same qualifier, found " + Joiner.on(',').join(qualifiers)); } - String uniqueQualifier = qualifiers.iterator().next(); - switch (uniqueQualifier) { + String qualifier = qualifiers.iterator().next(); + switch (qualifier) { case Qualifiers.VIEW: case Qualifiers.SUBVIEW: - addViewsOrSubViews(builder, componentUuids, uniqueQualifier); + addViewsOrSubViews(builder, componentUuids); break; case Qualifiers.PROJECT: builder.projectUuids(componentUuids); @@ -362,7 +301,7 @@ public class IssueQueryFactory { builder.moduleRootUuids(componentUuids); break; case Qualifiers.DIRECTORY: - addDirectories(builder, session, componentUuids); + addDirectories(builder, components); break; case Qualifiers.FILE: case Qualifiers.UNIT_TEST_FILE: @@ -373,11 +312,10 @@ public class IssueQueryFactory { } } - private void addViewsOrSubViews(IssueQuery.Builder builder, Collection componentUuids, String uniqueQualifier) { - List filteredViewUuids = newArrayList(); - for (String viewUuid : componentUuids) { - if ((Qualifiers.VIEW.equals(uniqueQualifier) && userSession.hasComponentUuidPermission(UserRole.USER, viewUuid)) - || (Qualifiers.SUBVIEW.equals(uniqueQualifier) && userSession.hasComponentUuidPermission(UserRole.USER, viewUuid))) { + private void addViewsOrSubViews(IssueQuery.Builder builder, Collection viewOrSubViewUuids) { + List filteredViewUuids = new ArrayList<>(); + for (String viewUuid : viewOrSubViewUuids) { + if (userSession.hasComponentUuidPermission(UserRole.USER, viewUuid)) { filteredViewUuids.add(viewUuid); } } @@ -387,10 +325,10 @@ public class IssueQueryFactory { builder.viewUuids(filteredViewUuids); } - private void addDirectories(IssueQuery.Builder builder, DbSession session, Collection componentUuids) { + private static void addDirectories(IssueQuery.Builder builder, List directories) { Collection directoryModuleUuids = Sets.newHashSet(); Collection directoryPaths = Sets.newHashSet(); - for (ComponentDto directory : componentService.getByUuids(session, componentUuids)) { + for (ComponentDto directory : directories) { directoryModuleUuids.add(directory.moduleUuid()); directoryPaths.add(directory.path()); } @@ -398,13 +336,12 @@ public class IssueQueryFactory { builder.directories(directoryPaths); } - private Collection componentUuids(DbSession session, @Nullable Collection componentKeys) { - Collection componentUuids = Lists.newArrayList(); - componentUuids.addAll(componentService.componentUuids(session, componentKeys, true)); + private Collection convertComponentKeysToUuids(DbSession dbSession, Collection componentKeys) { + List componentUuids = dbClient.componentDao().selectByKeys(dbSession, componentKeys).stream().map(ComponentDto::uuid).collect(Collectors.toList()); // If unknown components are given, but no components are found, then all issues will be returned, // so we add this hack in order to return no issue in this case. - if (componentKeys != null && !componentKeys.isEmpty() && componentUuids.isEmpty()) { - componentUuids.add(UNKNOWN); + if (!componentKeys.isEmpty() && componentUuids.isEmpty()) { + return singletonList(UNKNOWN); } return componentUuids; } @@ -430,18 +367,4 @@ public class IssueQueryFactory { } return null; } - - private static class HasTwoOrMoreElements implements Predicate { - private AtomicInteger counter; - - private HasTwoOrMoreElements() { - this.counter = new AtomicInteger(); - } - - @Override - public boolean apply(@Nonnull Object input) { - Objects.requireNonNull(input); - return counter.incrementAndGet() >= 2; - } - } } diff --git a/server/sonar-server/src/main/java/org/sonar/server/issue/index/IssueIndexDefinition.java b/server/sonar-server/src/main/java/org/sonar/server/issue/index/IssueIndexDefinition.java index 7c7cbe03d35..0a3dd891302 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/issue/index/IssueIndexDefinition.java +++ b/server/sonar-server/src/main/java/org/sonar/server/issue/index/IssueIndexDefinition.java @@ -106,7 +106,7 @@ public class IssueIndexDefinition implements IndexDefinition { type.stringFieldBuilder(FIELD_ISSUE_MESSAGE).disableNorms().build(); type.stringFieldBuilder(FIELD_ISSUE_MODULE_UUID).disableNorms().build(); type.createUuidPathField(FIELD_ISSUE_MODULE_PATH); - type.stringFieldBuilder(FIELD_ISSUE_ORGANIZATION_UUID).build(); + type.stringFieldBuilder(FIELD_ISSUE_ORGANIZATION_UUID).disableNorms().build(); type.stringFieldBuilder(FIELD_ISSUE_PROJECT_UUID).disableNorms().addSubFields(SORTABLE_ANALYZER).build(); type.stringFieldBuilder(FIELD_ISSUE_DIRECTORY_PATH).disableNorms().build(); type.stringFieldBuilder(FIELD_ISSUE_RESOLUTION).disableNorms().build(); diff --git a/server/sonar-server/src/main/java/org/sonar/server/issue/ws/ComponentTagsAction.java b/server/sonar-server/src/main/java/org/sonar/server/issue/ws/ComponentTagsAction.java index bb2df5a4e99..f3602ded9f1 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/issue/ws/ComponentTagsAction.java +++ b/server/sonar-server/src/main/java/org/sonar/server/issue/ws/ComponentTagsAction.java @@ -19,8 +19,6 @@ */ package org.sonar.server.issue.ws; -import com.google.common.collect.ImmutableMap; -import com.google.common.collect.ImmutableMap.Builder; import com.google.common.io.Resources; import java.util.Map; import org.sonar.api.server.ws.Request; @@ -31,8 +29,9 @@ import org.sonar.api.utils.text.JsonWriter; import org.sonar.server.issue.IssueQuery; import org.sonar.server.issue.IssueQueryFactory; import org.sonar.server.issue.IssueService; -import org.sonarqube.ws.client.issue.IssuesWsParameters; +import org.sonarqube.ws.client.issue.SearchWsRequest; +import static java.util.Collections.singletonList; import static org.sonar.api.server.ws.WebService.Param.PAGE_SIZE; import static org.sonarqube.ws.client.issue.IssuesWsParameters.ACTION_COMPONENT_TAGS; import static org.sonarqube.ws.client.issue.IssuesWsParameters.PARAM_COMPONENT_UUID; @@ -40,7 +39,6 @@ import static org.sonarqube.ws.client.issue.IssuesWsParameters.PARAM_CREATED_AFT /** * List issue tags matching a given query. - * @since 5.1 */ public class ComponentTagsAction implements IssuesWsAction { @@ -75,13 +73,12 @@ public class ComponentTagsAction implements IssuesWsAction { @Override public void handle(Request request, Response response) throws Exception { - Builder paramBuilder = ImmutableMap.builder() - .put(IssuesWsParameters.PARAM_COMPONENT_UUIDS, request.mandatoryParam(PARAM_COMPONENT_UUID)) - .put(IssuesWsParameters.PARAM_RESOLVED, false); - if (request.hasParam(PARAM_CREATED_AFTER)) { - paramBuilder.put(PARAM_CREATED_AFTER, request.param(PARAM_CREATED_AFTER)); - } - IssueQuery query = queryService.createFromMap(paramBuilder.build()); + SearchWsRequest searchWsRequest = new SearchWsRequest() + .setComponentUuids(singletonList(request.mandatoryParam(PARAM_COMPONENT_UUID))) + .setResolved(false) + .setCreatedAfter(request.param(PARAM_CREATED_AFTER)); + + IssueQuery query = queryService.create(searchWsRequest); int pageSize = request.mandatoryParamAsInt(PAGE_SIZE); try (JsonWriter json = response.newJsonWriter()) { json.beginObject().name("tags").beginArray(); 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 2436c9d5450..eb46fdecc6f 100644 --- 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 @@ -221,7 +221,6 @@ public class SearchAction implements IssuesWsAction { "If this parameter is set to a truthy value, createdAfter must not be set and one component id or key must be provided.") .setBooleanPossibleValues() .setDefaultValue("false"); - } private static void addComponentRelatedParams(WebService.NewAction action) { @@ -305,7 +304,7 @@ public class SearchAction implements IssuesWsAction { // prepare the Elasticsearch request SearchOptions options = createSearchOptionsFromRequest(request); EnumSet additionalFields = SearchAdditionalField.getFromRequest(request); - IssueQuery query = issueQueryFactory.createFromRequest(request); + IssueQuery query = issueQueryFactory.create(request); // execute request SearchResult result = issueIndex.search(query, options); diff --git a/server/sonar-server/src/test/java/org/sonar/server/component/ComponentServiceTest.java b/server/sonar-server/src/test/java/org/sonar/server/component/ComponentServiceTest.java index 193753d8627..2698ce65747 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/component/ComponentServiceTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/component/ComponentServiceTest.java @@ -19,7 +19,6 @@ */ package org.sonar.server.component; -import java.util.Arrays; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; @@ -32,7 +31,6 @@ import org.sonar.db.component.ComponentDto; import org.sonar.server.es.ProjectIndexer; import org.sonar.server.tester.UserSessionRule; -import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.guava.api.Assertions.assertThat; import static org.mockito.Mockito.mock; import static org.sonar.db.component.ComponentTesting.newFileDto; @@ -55,14 +53,6 @@ public class ComponentServiceTest { private ComponentService underTest = new ComponentService(dbClient, userSession, projectIndexer); - @Test - public void should_fail_silently_on_components_not_found_if_told_so() { - String moduleKey = "sample:root:module"; - String fileKey = "sample:root:module:Foo.xoo"; - - assertThat(underTest.componentUuids(dbSession, Arrays.asList(moduleKey, fileKey), true)).isEmpty(); - } - @Test public void bulk_update() { ComponentDto project = componentDb.insertComponent(newProjectDto(dbTester.organizations().insert()).setKey("my_project")); diff --git a/server/sonar-server/src/test/java/org/sonar/server/issue/IssueQueryFactoryTest.java b/server/sonar-server/src/test/java/org/sonar/server/issue/IssueQueryFactoryTest.java index 06eefe50c36..294ab90c868 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/issue/IssueQueryFactoryTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/issue/IssueQueryFactoryTest.java @@ -20,8 +20,6 @@ package org.sonar.server.issue; import java.util.Date; -import java.util.HashMap; -import java.util.Map; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; @@ -34,7 +32,6 @@ import org.sonar.db.DbTester; import org.sonar.db.component.ComponentDto; import org.sonar.db.component.ComponentTesting; import org.sonar.db.organization.OrganizationDto; -import org.sonar.server.component.ComponentService; import org.sonar.server.exceptions.BadRequestException; import org.sonar.server.tester.UserSessionRule; import org.sonarqube.ws.client.issue.SearchWsRequest; @@ -55,9 +52,8 @@ public class IssueQueryFactoryTest { @Rule public DbTester db = DbTester.create(); - private ComponentService componentService = new ComponentService(db.getDbClient(), userSession); private System2 system = mock(System2.class); - private IssueQueryFactory underTest = new IssueQueryFactory(db.getDbClient(), componentService, system, userSession); + private IssueQueryFactory underTest = new IssueQueryFactory(db.getDbClient(), system, userSession); @Test public void create_from_parameters() { @@ -66,30 +62,28 @@ public class IssueQueryFactoryTest { ComponentDto module = db.components().insertComponent(ComponentTesting.newModuleDto(project)); ComponentDto file = db.components().insertComponent(ComponentTesting.newFileDto(project)); - Map map = new HashMap<>(); - map.put("issues", newArrayList("anIssueKey")); - map.put("severities", newArrayList("MAJOR", "MINOR")); - map.put("statuses", newArrayList("CLOSED")); - map.put("resolutions", newArrayList("FALSE-POSITIVE")); - map.put("resolved", true); - map.put("projectKeys", newArrayList(project.key())); - map.put("moduleUuids", newArrayList(module.uuid())); - map.put("directories", newArrayList("aDirPath")); - map.put("fileUuids", newArrayList(file.uuid())); - map.put("assignees", newArrayList("joanna")); - map.put("languages", newArrayList("xoo")); - map.put("tags", newArrayList("tag1", "tag2")); - map.put("organization", organization.getKey()); - map.put("assigned", true); - map.put("planned", true); - map.put("hideRules", true); - map.put("createdAfter", "2013-04-16T09:08:24+0200"); - map.put("createdBefore", "2013-04-17T09:08:24+0200"); - map.put("rules", "squid:AvoidCycle,findbugs:NullReference"); - map.put("sort", "CREATION_DATE"); - map.put("asc", true); - - IssueQuery query = underTest.createFromMap(map); + SearchWsRequest request = new SearchWsRequest() + .setIssues(asList("anIssueKey")) + .setSeverities(asList("MAJOR", "MINOR")) + .setStatuses(asList("CLOSED")) + .setResolutions(asList("FALSE-POSITIVE")) + .setResolved(true) + .setProjectKeys(asList(project.key())) + .setModuleUuids(asList(module.uuid())) + .setDirectories(asList("aDirPath")) + .setFileUuids(asList(file.uuid())) + .setAssignees(asList("joanna")) + .setLanguages(asList("xoo")) + .setTags(asList("tag1", "tag2")) + .setOrganization(organization.getKey()) + .setAssigned(true) + .setCreatedAfter("2013-04-16T09:08:24+0200") + .setCreatedBefore("2013-04-17T09:08:24+0200") + .setRules(asList("squid:AvoidCycle", "findbugs:NullReference")) + .setSort("CREATION_DATE") + .setAsc(true); + + IssueQuery query = underTest.create(request); assertThat(query.issueKeys()).containsOnly("anIssueKey"); assertThat(query.severities()).containsOnly("MAJOR", "MINOR"); @@ -105,7 +99,6 @@ public class IssueQueryFactoryTest { assertThat(query.organizationUuid()).isEqualTo(organization.getUuid()); assertThat(query.onComponentOnly()).isFalse(); assertThat(query.assigned()).isTrue(); - assertThat(query.hideRules()).isTrue(); assertThat(query.rules()).hasSize(2); assertThat(query.directories()).containsOnly("aDirPath"); assertThat(query.createdAfter()).isEqualTo(DateUtils.parseDateTime("2013-04-16T09:08:24+0200")); @@ -120,7 +113,7 @@ public class IssueQueryFactoryTest { .setCreatedAfter("2013-04-16") .setCreatedBefore("2013-04-17"); - IssueQuery query = underTest.createFromRequest(request); + IssueQuery query = underTest.create(request); assertThat(query.createdAfter()).isEqualTo(DateUtils.parseDate("2013-04-16")); assertThat(query.createdBefore()).isEqualTo(DateUtils.parseDate("2013-04-18")); @@ -128,10 +121,11 @@ public class IssueQueryFactoryTest { @Test public void add_unknown_when_no_component_found() { - Map map = new HashMap<>(); - map.put("components", newArrayList("does_not_exist")); + SearchWsRequest request = new SearchWsRequest() + .setComponentKeys(asList("does_not_exist")); + + IssueQuery query = underTest.create(request); - IssueQuery query = underTest.createFromMap(map); assertThat(query.componentUuids()).containsOnly(""); } @@ -146,61 +140,61 @@ public class IssueQueryFactoryTest { @Test public void fail_if_components_and_components_uuid_params_are_set_at_the_same_time() { - Map map = new HashMap<>(); - map.put("components", newArrayList("foo")); - map.put("componentUuids", newArrayList("bar")); + SearchWsRequest request = new SearchWsRequest() + .setComponentKeys(asList("foo")) + .setComponentUuids(asList("bar")); expectedException.expect(IllegalArgumentException.class); expectedException.expectMessage("At most one of the following parameters can be provided: componentKeys, componentUuids, components, componentRoots, componentUuids"); - underTest.createFromMap(map); + underTest.create(request); } @Test public void fail_if_both_projects_and_projectUuids_params_are_set() { - Map map = new HashMap<>(); - map.put("projects", newArrayList("foo")); - map.put("projectUuids", newArrayList("bar")); + SearchWsRequest request = new SearchWsRequest() + .setProjectKeys(asList("foo")) + .setProjectUuids(asList("bar")); expectedException.expect(IllegalArgumentException.class); - expectedException.expectMessage("projects and projectUuids cannot be set simultaneously"); + expectedException.expectMessage("Parameters projects and projectUuids cannot be set simultaneously"); - underTest.createFromMap(map); + underTest.create(request); } @Test public void fail_if_both_componentRoots_and_componentRootUuids_params_are_set() { - Map map = new HashMap<>(); - map.put("componentRoots", newArrayList("foo")); - map.put("componentRootUuids", newArrayList("bar")); + SearchWsRequest request = new SearchWsRequest() + .setComponentRoots(asList("foo")) + .setComponentRootUuids(asList("bar")); expectedException.expect(IllegalArgumentException.class); expectedException.expectMessage("At most one of the following parameters can be provided: componentKeys, componentUuids, components, componentRoots, componentUuids"); - underTest.createFromMap(map); + underTest.create(request); } @Test public void fail_if_componentRoots_references_components_with_different_qualifier() { ComponentDto project = db.components().insertProject(); ComponentDto file = db.components().insertComponent(ComponentTesting.newFileDto(project)); - Map map = new HashMap<>(); - map.put("componentRoots", newArrayList(project.key(), file.key())); + SearchWsRequest request = new SearchWsRequest() + .setComponentRoots(asList(project.key(), file.key())); expectedException.expect(IllegalArgumentException.class); expectedException.expectMessage("All components must have the same qualifier, found FIL,TRK"); - underTest.createFromMap(map); + underTest.create(request); } @Test public void param_componentRootUuids_enables_search_in_view_tree_if_user_has_permission_on_view() { ComponentDto view = db.components().insertView(); - Map map = new HashMap<>(); - map.put("componentRootUuids", newArrayList(view.uuid())); + SearchWsRequest request = new SearchWsRequest() + .setComponentRootUuids(asList(view.uuid())); userSession.addProjectUuidPermissions(UserRole.USER, view.uuid()); - IssueQuery query = underTest.createFromMap(map); + IssueQuery query = underTest.create(request); assertThat(query.viewUuids()).containsOnly(view.uuid()); assertThat(query.onComponentOnly()).isFalse(); @@ -210,10 +204,10 @@ public class IssueQueryFactoryTest { public void return_empty_results_if_not_allowed_to_search_for_subview() { ComponentDto view = db.components().insertView(); ComponentDto subView = db.components().insertComponent(ComponentTesting.newSubView(view)); - Map map = new HashMap<>(); - map.put("componentRootUuids", newArrayList(subView.uuid())); + SearchWsRequest request = new SearchWsRequest() + .setComponentRootUuids(asList(subView.uuid())); - IssueQuery query = underTest.createFromMap(map); + IssueQuery query = underTest.create(request); assertThat(query.viewUuids()).containsOnly(""); } @@ -221,10 +215,10 @@ public class IssueQueryFactoryTest { @Test public void param_componentUuids_enables_search_on_project_tree_by_default() { ComponentDto project = db.components().insertProject(); - Map map = new HashMap<>(); - map.put("componentUuids", newArrayList(project.uuid())); + SearchWsRequest request = new SearchWsRequest() + .setComponentUuids(asList(project.uuid())); - IssueQuery query = underTest.createFromMap(map); + IssueQuery query = underTest.create(request); assertThat(query.projectUuids()).containsExactly(project.uuid()); assertThat(query.onComponentOnly()).isFalse(); } @@ -232,11 +226,11 @@ public class IssueQueryFactoryTest { @Test public void onComponentOnly_restricts_search_to_specified_componentKeys() { ComponentDto project = db.components().insertProject(); - Map map = new HashMap<>(); - map.put("componentKeys", newArrayList(project.key())); - map.put("onComponentOnly", true); + SearchWsRequest request = new SearchWsRequest() + .setComponentKeys(asList(project.key())) + .setOnComponentOnly(true); - IssueQuery query = underTest.createFromMap(map); + IssueQuery query = underTest.create(request); assertThat(query.projectUuids()).isEmpty(); assertThat(query.componentUuids()).containsExactly(project.uuid()); @@ -247,10 +241,10 @@ public class IssueQueryFactoryTest { public void should_search_in_tree_with_module_uuid() { ComponentDto project = db.components().insertProject(); ComponentDto module = db.components().insertComponent(ComponentTesting.newModuleDto(project)); - Map map = new HashMap<>(); - map.put("componentUuids", newArrayList(module.uuid())); + SearchWsRequest request = new SearchWsRequest() + .setComponentUuids(asList(module.uuid())); - IssueQuery query = underTest.createFromMap(map); + IssueQuery query = underTest.create(request); assertThat(query.moduleRootUuids()).containsExactly(module.uuid()); assertThat(query.onComponentOnly()).isFalse(); } @@ -259,10 +253,10 @@ public class IssueQueryFactoryTest { public void param_componentUuids_enables_search_in_directory_tree() { ComponentDto project = db.components().insertProject(); ComponentDto dir = db.components().insertComponent(ComponentTesting.newDirectory(project, "src/main/java/foo")); - Map map = new HashMap<>(); - map.put("componentUuids", newArrayList(dir.uuid())); + SearchWsRequest request = new SearchWsRequest() + .setComponentUuids(asList(dir.uuid())); - IssueQuery query = underTest.createFromMap(map); + IssueQuery query = underTest.create(request); assertThat(query.moduleUuids()).containsOnly(dir.moduleUuid()); assertThat(query.directories()).containsOnly(dir.path()); @@ -273,10 +267,10 @@ public class IssueQueryFactoryTest { public void param_componentUuids_enables_search_by_file() { ComponentDto project = db.components().insertProject(); ComponentDto file = db.components().insertComponent(ComponentTesting.newFileDto(project)); - Map map = new HashMap<>(); - map.put("componentUuids", newArrayList(file.uuid())); + SearchWsRequest request = new SearchWsRequest() + .setComponentUuids(asList(file.uuid())); - IssueQuery query = underTest.createFromMap(map); + IssueQuery query = underTest.create(request); assertThat(query.fileUuids()).containsExactly(file.uuid()); } @@ -285,25 +279,25 @@ public class IssueQueryFactoryTest { public void param_componentUuids_enables_search_by_test_file() { ComponentDto project = db.components().insertProject(); ComponentDto file = db.components().insertComponent(ComponentTesting.newFileDto(project).setQualifier(Qualifiers.UNIT_TEST_FILE)); - Map map = new HashMap<>(); - map.put("componentUuids", newArrayList(file.uuid())); + SearchWsRequest request = new SearchWsRequest() + .setComponentUuids(asList(file.uuid())); - IssueQuery query = underTest.createFromMap(map); + IssueQuery query = underTest.create(request); assertThat(query.fileUuids()).containsExactly(file.uuid()); } @Test public void fail_if_created_after_and_created_since_are_both_set() { - Map map = new HashMap<>(); - map.put("createdAfter", "2013-07-25T07:35:00+0100"); - map.put("createdInLast", "palap"); + SearchWsRequest request = new SearchWsRequest() + .setCreatedAfter("2013-07-25T07:35:00+0100") + .setCreatedInLast("palap"); try { - underTest.createFromMap(map); + underTest.create(request); fail(); } catch (Exception e) { - assertThat(e).isInstanceOf(IllegalArgumentException.class).hasMessage("createdAfter and createdInLast cannot be set simultaneously"); + assertThat(e).isInstanceOf(IllegalArgumentException.class).hasMessage("Parameters createdAfter and createdInLast cannot be set simultaneously"); } } @@ -311,18 +305,17 @@ public class IssueQueryFactoryTest { public void set_created_after_from_created_since() { Date now = DateUtils.parseDateTime("2013-07-25T07:35:00+0100"); when(system.now()).thenReturn(now.getTime()); - Map map = new HashMap<>(); - - map.put("createdInLast", "1y2m3w4d"); - assertThat(underTest.createFromMap(map).createdAfter()).isEqualTo(DateUtils.parseDateTime("2012-04-30T07:35:00+0100")); + SearchWsRequest request = new SearchWsRequest() + .setCreatedInLast("1y2m3w4d"); + assertThat(underTest.create(request).createdAfter()).isEqualTo(DateUtils.parseDateTime("2012-04-30T07:35:00+0100")); } @Test public void fail_if_since_leak_period_and_created_after_set_at_the_same_time() { expectedException.expect(BadRequestException.class); - expectedException.expectMessage("'createdAfter' and 'sinceLeakPeriod' cannot be set simultaneously"); + expectedException.expectMessage("Parameters 'createdAfter' and 'sinceLeakPeriod' cannot be set simultaneously"); - underTest.createFromRequest(new SearchWsRequest() + underTest.create(new SearchWsRequest() .setSinceLeakPeriod(true) .setCreatedAfter("2013-07-25T07:35:00+0100")); } @@ -332,7 +325,7 @@ public class IssueQueryFactoryTest { expectedException.expect(IllegalArgumentException.class); expectedException.expectMessage("One and only one component must be provided when searching since leak period"); - underTest.createFromRequest(new SearchWsRequest().setSinceLeakPeriod(true)); + underTest.create(new SearchWsRequest().setSinceLeakPeriod(true)); } @Test @@ -340,9 +333,9 @@ public class IssueQueryFactoryTest { expectedException.expect(IllegalArgumentException.class); expectedException.expectMessage("One and only one component must be provided when searching since leak period"); - underTest.createFromRequest(new SearchWsRequest() + underTest.create(new SearchWsRequest() .setSinceLeakPeriod(true) - .setComponentUuids(newArrayList("component-uuid", "project-uuid"))); + .setComponentUuids(newArrayList("foo", "bar"))); } @Test @@ -350,17 +343,18 @@ public class IssueQueryFactoryTest { expectedException.expect(IllegalArgumentException.class); expectedException.expectMessage("'unknown-date' cannot be parsed as either a date or date+time"); - underTest.createFromRequest(new SearchWsRequest() + underTest.create(new SearchWsRequest() .setCreatedAfter("unknown-date")); } @Test public void return_empty_results_if_organization_with_specified_key_does_not_exist() { - Map map = new HashMap<>(); - map.put("organization", "does_not_exist"); + SearchWsRequest request = new SearchWsRequest() + .setOrganization("does_not_exist"); - IssueQuery query = underTest.createFromMap(map); + IssueQuery query = underTest.create(request); assertThat(query.organizationUuid()).isEqualTo(""); } + } diff --git a/server/sonar-server/src/test/java/org/sonar/server/issue/IssueQueryTest.java b/server/sonar-server/src/test/java/org/sonar/server/issue/IssueQueryTest.java index 79a2361187c..cce5c009d9c 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/issue/IssueQueryTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/issue/IssueQueryTest.java @@ -46,7 +46,6 @@ public class IssueQueryTest { .tags(newArrayList("tag1", "tag2")) .types(newArrayList("RELIABILITY", "SECURITY")) .assigned(true) - .hideRules(true) .createdAfter(new Date()) .createdBefore(new Date()) .createdAt(new Date()) @@ -65,7 +64,6 @@ public class IssueQueryTest { assertThat(query.tags()).containsOnly("tag1", "tag2"); assertThat(query.types()).containsOnly("RELIABILITY", "SECURITY"); assertThat(query.assigned()).isTrue(); - assertThat(query.hideRules()).isTrue(); assertThat(query.rules()).containsOnly(RuleKey.of("squid", "AvoidCycle")); assertThat(query.createdAfter()).isNotNull(); assertThat(query.createdBefore()).isNotNull(); diff --git a/server/sonar-server/src/test/java/org/sonar/server/issue/ws/ComponentTagsActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/issue/ws/ComponentTagsActionTest.java index 6617dd0b7da..f02c927b3a1 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/issue/ws/ComponentTagsActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/issue/ws/ComponentTagsActionTest.java @@ -21,51 +21,40 @@ package org.sonar.server.issue.ws; import com.google.common.collect.ImmutableMap; import java.util.Map; -import org.junit.Before; import org.junit.Test; -import org.junit.runner.RunWith; import org.mockito.ArgumentCaptor; -import org.mockito.Mock; -import org.mockito.runners.MockitoJUnitRunner; +import org.mockito.Mockito; import org.sonar.api.server.ws.WebService.Action; import org.sonar.api.server.ws.WebService.Param; import org.sonar.server.issue.IssueQuery; import org.sonar.server.issue.IssueQueryFactory; import org.sonar.server.issue.IssueService; -import org.sonar.server.ws.WsTester; +import org.sonar.server.ws.TestResponse; +import org.sonar.server.ws.WsActionTester; +import org.sonarqube.ws.client.issue.SearchWsRequest; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Matchers.any; +import static org.mockito.Matchers.eq; import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import static org.sonar.test.JsonAssert.assertJson; -@RunWith(MockitoJUnitRunner.class) public class ComponentTagsActionTest { - @Mock - private IssueService service; - - @Mock - private IssueQueryFactory queryService; - - private ComponentTagsAction componentTagsAction; - - private WsTester tester; - - @Before - public void setUp() { - componentTagsAction = new ComponentTagsAction(service, queryService); - tester = new WsTester(new IssuesWs(componentTagsAction)); - } + private IssueService service = mock(IssueService.class); + private IssueQueryFactory issueQueryFactory = mock(IssueQueryFactory.class, Mockito.RETURNS_DEEP_STUBS); + private ComponentTagsAction underTest = new ComponentTagsAction(service, issueQueryFactory); + private WsActionTester tester = new WsActionTester(underTest); @Test public void should_define() { - Action action = tester.controller("api/issues").action("component_tags"); + Action action = tester.getDef(); assertThat(action.description()).isNotEmpty(); assertThat(action.responseExampleAsString()).isNotEmpty(); assertThat(action.isPost()).isFalse(); assertThat(action.isInternal()).isTrue(); - assertThat(action.handler()).isEqualTo(componentTagsAction); + assertThat(action.handler()).isEqualTo(underTest); assertThat(action.params()).hasSize(3); Param query = action.param("componentUuid"); @@ -85,7 +74,10 @@ public class ComponentTagsActionTest { @Test public void should_return_empty_list() throws Exception { - tester.newGetRequest("api/issues", "component_tags").setParam("componentUuid", "polop").execute().assertJson("{\"tags\":[]}"); + TestResponse response = tester.newRequest() + .setParam("componentUuid", "polop") + .execute(); + assertJson(response.getInput()).isSimilarTo("{\"tags\":[]}"); } @Test @@ -97,17 +89,19 @@ public class ComponentTagsActionTest { .put("bug", 32L) .put("cert", 2L) .build(); - IssueQuery query = mock(IssueQuery.class); - ArgumentCaptor captor = ArgumentCaptor.forClass(Map.class); - when(queryService.createFromMap(captor.capture())).thenReturn(query); - when(service.listTagsForComponent(query, 5)).thenReturn(tags); + ArgumentCaptor captor = ArgumentCaptor.forClass(SearchWsRequest.class); + when(issueQueryFactory.create(captor.capture())).thenReturn(mock(IssueQuery.class)); + when(service.listTagsForComponent(any(IssueQuery.class), eq(5))).thenReturn(tags); + + TestResponse response = tester.newRequest() + .setParam("componentUuid", "polop") + .setParam("ps", "5") + .execute(); + assertJson(response.getInput()).isSimilarTo(getClass().getResource("ComponentTagsActionTest/component-tags.json")); - tester.newGetRequest("api/issues", "component_tags").setParam("componentUuid", "polop").setParam("ps", "5").execute() - .assertJson(getClass(), "component-tags.json"); - assertThat(captor.getValue()) - .containsEntry("componentUuids", "polop") - .containsEntry("resolved", false); - verify(service).listTagsForComponent(query, 5); + assertThat(captor.getValue().getComponentUuids()).containsOnly("polop"); + assertThat(captor.getValue().getResolved()).isFalse(); + assertThat(captor.getValue().getCreatedAfter()).isNull(); } @Test @@ -119,23 +113,20 @@ public class ComponentTagsActionTest { .put("bug", 32L) .put("cert", 2L) .build(); - IssueQuery query = mock(IssueQuery.class); - ArgumentCaptor captor = ArgumentCaptor.forClass(Map.class); - when(queryService.createFromMap(captor.capture())).thenReturn(query); - when(service.listTagsForComponent(query, 5)).thenReturn(tags); + ArgumentCaptor captor = ArgumentCaptor.forClass(SearchWsRequest.class); + when(issueQueryFactory.create(captor.capture())).thenReturn(mock(IssueQuery.class)); + when(service.listTagsForComponent(any(IssueQuery.class), eq(5))).thenReturn(tags); String componentUuid = "polop"; String createdAfter = "2011-04-25"; - tester.newGetRequest("api/issues", "component_tags") + TestResponse response = tester.newRequest() .setParam("componentUuid", componentUuid) .setParam("createdAfter", createdAfter) .setParam("ps", "5") - .execute() - .assertJson(getClass(), "component-tags.json"); - assertThat(captor.getValue()) - .containsEntry("componentUuids", componentUuid) - .containsEntry("resolved", false) - .containsEntry("createdAfter", createdAfter); - verify(service).listTagsForComponent(query, 5); + .execute(); + assertJson(response.getInput()).isSimilarTo(getClass().getResource("ComponentTagsActionTest/component-tags.json")); + assertThat(captor.getValue().getComponentUuids()).containsOnly(componentUuid); + assertThat(captor.getValue().getResolved()).isFalse(); + assertThat(captor.getValue().getCreatedAfter()).isEqualTo(createdAfter); } } diff --git a/server/sonar-server/src/test/java/org/sonar/server/issue/ws/SearchActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/issue/ws/SearchActionTest.java index f2a4231b0a4..48a93e15876 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/issue/ws/SearchActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/issue/ws/SearchActionTest.java @@ -55,7 +55,7 @@ public class SearchActionTest { assertThat(def.since()).isEqualTo("3.6"); assertThat(def.responseExampleAsString()).isNotEmpty(); - assertThat(def.params()).extracting("key").containsOnly( + assertThat(def.params()).extracting("key").containsExactlyInAnyOrder( "actionPlans", "additionalFields", "asc", "assigned", "assignees", "authors", "componentKeys", "componentRootUuids", "componentRoots", "componentUuids", "components", "createdAfter", "createdAt", "createdBefore", "createdInLast", "directories", "facetMode", "facets", "fileUuids", "issues", "languages", "moduleUuids", "onComponentOnly", "organization", "p", "planned", "projectKeys", "projectUuids", "projects", "ps", "reporters", "resolutions", "resolved", "rules", "s", "severities", "sinceLeakPeriod", -- 2.39.5