From 98b3ff0a5f05667b59723714d95bcfb1060d93c0 Mon Sep 17 00:00:00 2001 From: Julien Lancelot Date: Thu, 24 Mar 2016 11:33:44 +0100 Subject: [PATCH] SONAR-7472 Drop ability to create manual issues from WS --- .../src/test/java/it/Category2Suite.java | 2 - .../test/java/it/issue/ManualIssueTest.java | 358 ------------------ .../sonar/server/component/ws/AppAction.java | 1 - .../org/sonar/server/issue/IssueService.java | 70 +--- .../sonar/server/issue/ws/CreateAction.java | 25 +- .../sonar/server/source/SourceService.java | 17 - .../sonar/server/user/index/UserIndex.java | 13 - .../server/issue/IssueServiceMediumTest.java | 216 ----------- .../server/issue/ws/CreateActionTest.java | 43 +-- .../server/source/SourceServiceTest.java | 19 - .../server/user/index/UserIndexTest.java | 19 - .../component/ws/AppActionTest/app.json | 1 - .../ws/AppActionTest/app_with_it_measure.json | 1 - .../ws/AppActionTest/app_with_measures.json | 1 - .../app_with_overall_measure.json | 1 - .../ws/AppActionTest/app_with_ut_measure.json | 1 - sonar-ws/src/main/protobuf/ws-issues.proto | 4 +- 17 files changed, 10 insertions(+), 782 deletions(-) delete mode 100644 it/it-tests/src/test/java/it/issue/ManualIssueTest.java diff --git a/it/it-tests/src/test/java/it/Category2Suite.java b/it/it-tests/src/test/java/it/Category2Suite.java index bb7f7dead2c..23087b451c8 100644 --- a/it/it-tests/src/test/java/it/Category2Suite.java +++ b/it/it-tests/src/test/java/it/Category2Suite.java @@ -33,7 +33,6 @@ import it.issue.IssueSearchTest; import it.issue.IssueTrackingTest; import it.issue.IssueWorkflowTest; import it.issue.ManualIssueRelocationTest; -import it.issue.ManualIssueTest; import it.issue.NewIssuesMeasureTest; import it.qualityModel.MaintainabilityMeasureTest; import it.qualityModel.MaintainabilityRatingMeasureTest; @@ -73,7 +72,6 @@ import static util.ItUtils.xooPlugin; IssueTrackingTest.class, IssueWorkflowTest.class, ManualIssueRelocationTest.class, - ManualIssueTest.class, NewIssuesMeasureTest.class, // rule ManualRulesTest.class, diff --git a/it/it-tests/src/test/java/it/issue/ManualIssueTest.java b/it/it-tests/src/test/java/it/issue/ManualIssueTest.java deleted file mode 100644 index 25397379936..00000000000 --- a/it/it-tests/src/test/java/it/issue/ManualIssueTest.java +++ /dev/null @@ -1,358 +0,0 @@ -/* - * SonarQube - * Copyright (C) 2009-2016 SonarSource SA - * mailto:contact AT sonarsource DOT com - * - * This program is free software; you can redistribute it and/or - * modify it under the terms of the GNU Lesser General Public - * License as published by the Free Software Foundation; either - * version 3 of the License, or (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * Lesser General Public License for more details. - * - * You should have received a copy of the GNU Lesser General Public License - * along with this program; if not, write to the Free Software Foundation, - * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. - */ -package it.issue; - -import com.google.common.collect.ImmutableMap; -import java.util.List; -import org.junit.Before; -import org.junit.Test; -import org.junit.experimental.categories.Category; -import org.sonar.wsclient.issue.Issue; -import org.sonar.wsclient.issue.IssueComment; -import org.sonar.wsclient.issue.IssueQuery; -import org.sonar.wsclient.issue.Issues; -import org.sonar.wsclient.issue.NewIssue; -import util.QaOnly; - -import static org.assertj.core.api.Assertions.assertThat; -import static org.junit.Assert.fail; -import static util.ItUtils.runProjectAnalysis; -import static util.ItUtils.verifyHttpException; - -/** - * SONAR-4304 - */ -@Category(QaOnly.class) -public class ManualIssueTest extends AbstractIssueTest { - - private final static String COMPONENT_KEY = "sample:src/main/xoo/sample/Sample.xoo"; - - @Before - public void before() { - ORCHESTRATOR.resetData(); - analyzeProject(); - createManualRule(); - } - - @Test - public void create_manual_issue_through_ws() throws Exception { - // Create the manual issue - Issue newIssue = adminIssueClient().create(NewIssue.create().component(COMPONENT_KEY) - .rule("manual:invalidclassname") - .line(3) - .severity("CRITICAL") - .message("The name 'Sample' is too generic")); - assertThat(newIssue.key()).isNotNull(); - assertThat(newIssue.creationDate()).isNotNull(); - assertThat(newIssue.updateDate()).isNotNull(); - assertThat(newIssue.ruleKey()).isEqualTo("manual:invalidclassname"); - assertThat(newIssue.line()).isEqualTo(3); - assertThat(newIssue.severity()).isEqualTo(("CRITICAL")); - assertThat(newIssue.message()).isEqualTo(("The name 'Sample' is too generic")); - assertThat(newIssue.status()).isEqualTo("OPEN"); - assertThat(newIssue.resolution()).isNull(); - assertThat(newIssue.reporter()).isEqualTo("admin"); - - Issues issues = search(IssueQuery.create().issues(newIssue.key())); - assertThat(issues.list().get(0).reporter()).isEqualTo("admin"); - - // get the detail of the reporter - assertThat(issues.user("admin").name()).isEqualTo("Administrator"); - } - - @Test - public void scan_should_keep_manual_issues_open() throws Exception { - // Create the manual issue - Issue newIssue = adminIssueClient().create(NewIssue.create().component(COMPONENT_KEY) - .rule("manual:invalidclassname") - .line(3) - .severity("CRITICAL") - .message("The name 'Sample' is too generic")); - assertThat(newIssue.key()).isNotNull(); - assertThat(newIssue.creationDate()).isNotNull(); - assertThat(newIssue.updateDate()).isNotNull(); - - // the metric 'issues' is not up-to-date yet - assertThat(searchIssuesByComponent(COMPONENT_KEY)).hasSize(1); - - // re-inspect the project : the issue still exists - analyzeProject(); - - Issue issue = searchIssueByKey(newIssue.key()); - assertThat(issue.ruleKey()).isEqualTo("manual:invalidclassname"); - assertThat(issue.line()).isEqualTo(3); - assertThat(issue.severity()).isEqualTo(("CRITICAL")); - assertThat(issue.message()).isEqualTo(("The name 'Sample' is too generic")); - assertThat(issue.status()).isEqualTo("OPEN"); - assertThat(issue.resolution()).isNull(); - assertThat(issue.reporter()).isEqualTo("admin"); - assertThat(issue.creationDate()).isEqualTo(newIssue.creationDate()); - assertThat(issue.updateDate()).isEqualTo(newIssue.updateDate()); - } - - @Test - public void scan_should_close_issues_on_deleted_manual_rules() throws Exception { - // Create another manual rule - ORCHESTRATOR.getServer().adminWsClient().post("/api/rules/create", ImmutableMap.of( - "manual_key", "ruletoberemoved", - "name", "RuleToBeRemoved", - "markdown_description", "Rule to be removed" - )); - - // Create the manual issue - Issue newIssue = adminIssueClient().create(NewIssue.create().component(COMPONENT_KEY) - .rule("manual:ruletoberemoved") - .line(3) - .severity("CRITICAL") - .message("The name 'Sample' is too generic")); - assertThat(newIssue.status()).isEqualTo("OPEN"); - - // Delete the manual rule (will be in fact disabled in the db, not removed) - ORCHESTRATOR.getServer().adminWsClient().post("/api/rules/delete", ImmutableMap.of("key", "manual:ruletoberemoved")); - - analyzeProject(); - Issue closedIssue = searchIssueByKey(newIssue.key()); - assertThat(closedIssue.status()).isEqualTo("CLOSED"); - assertThat(closedIssue.resolution()).isEqualTo("REMOVED"); - assertThat(closedIssue.creationDate()).isEqualTo(newIssue.creationDate()); - assertThat(closedIssue.updateDate().before(newIssue.updateDate())).isFalse(); - assertThat(closedIssue.closeDate().before(closedIssue.creationDate())).isFalse(); - } - - @Test - public void scan_should_close_manual_resolved_issues() throws Exception { - // Create the manual issue - Issue newIssue = adminIssueClient().create(NewIssue.create().component(COMPONENT_KEY) - .rule("manual:invalidclassname") - .line(3) - .severity("CRITICAL") - .message("The name 'Sample' is too generic")); - assertThat(newIssue.status()).isEqualTo("OPEN"); - - // mark issue as resolved - adminIssueClient().doTransition(newIssue.key(), "resolve"); - - analyzeProject(); - Issue closedIssue = searchIssueByKey(newIssue.key()); - assertThat(closedIssue.status()).isEqualTo("CLOSED"); - assertThat(closedIssue.resolution()).isEqualTo("FIXED"); - assertThat(closedIssue.creationDate()).isEqualTo(newIssue.creationDate()); - assertThat(closedIssue.updateDate().before(newIssue.updateDate())).isFalse(); - assertThat(closedIssue.closeDate().before(closedIssue.creationDate())).isFalse(); - } - - @Test - public void add_comment_to_manual_issue() throws Exception { - // Create the manual issue - Issue manualIssue = adminIssueClient().create(NewIssue.create().component(COMPONENT_KEY) - .rule("manual:invalidclassname") - .line(3) - .severity("CRITICAL") - .message("The name 'Sample' is too generic")); - - // Add a comment on the manual issue - IssueComment comment = adminIssueClient().addComment(manualIssue.key(), "this is my *comment*"); - - // Reload manual issue - Issue reloaded = searchIssueWithComments(manualIssue.key()); - - assertThat(reloaded.comments()).hasSize(1); - assertThat(reloaded.comments().get(0).key()).isEqualTo(comment.key()); - } - - @Test - public void resolve_manual_issue() throws Exception { - // Create the manual issue - Issue manualIssue = adminIssueClient().create(NewIssue.create().component(COMPONENT_KEY) - .rule("manual:invalidclassname") - .line(3) - .severity("CRITICAL") - .message("The name 'Sample' is too generic")); - - // Resolve the manual issue - adminIssueClient().doTransition(manualIssue.key(), "resolve"); - - // Check the manual issue is well resolved - Issue reloaded = searchIssueByKey(manualIssue.key()); - assertThat(reloaded.status()).isEqualTo("RESOLVED"); - assertThat(reloaded.resolution()).isEqualTo("FIXED"); - assertThat(reloaded.creationDate()).isEqualTo(manualIssue.creationDate()); - assertThat(reloaded.updateDate().before(manualIssue.updateDate())).isFalse(); - - analyzeProject(); - - // Reload after analyse -> manual issue should be closed - reloaded = searchIssueByKey(manualIssue.key()); - assertThat(reloaded.status()).isEqualTo("CLOSED"); - assertThat(reloaded.resolution()).isEqualTo("FIXED"); - assertThat(reloaded.creationDate()).isEqualTo(manualIssue.creationDate()); - assertThat(reloaded.updateDate().before(manualIssue.updateDate())).isFalse(); - assertThat(reloaded.closeDate()).isNotNull(); - assertThat(reloaded.closeDate().before(reloaded.creationDate())).isFalse(); - } - - @Test - public void resolve_and_reopen_manual_issue() throws Exception { - // Create the manual issue - Issue issue = adminIssueClient().create(NewIssue.create().component(COMPONENT_KEY) - .rule("manual:invalidclassname") - .line(3) - .severity("CRITICAL") - .message("The name 'Sample' is too generic")); - - // Resolve the manual issue - adminIssueClient().doTransition(issue.key(), "resolve"); - - // Check the manual issue is well resolved - assertThat(searchIssueByKey(issue.key()).status()).isEqualTo("RESOLVED"); - - analyzeProject(); - // Reload after analyse -> manual issue is closed - assertThat(searchIssueByKey(issue.key()).status()).isEqualTo("CLOSED"); - - // Reopen the manual issue - adminIssueClient().doTransition(issue.key(), "reopen"); - - analyzeProject(); - // Reload after analyse -> manual issue is reopened - Issue reloaded = searchIssueByKey(issue.key()); - assertThat(reloaded.status()).isEqualTo("REOPENED"); - assertThat(reloaded.resolution()).isNull(); - assertThat(reloaded.creationDate()).isEqualTo(issue.creationDate()); - assertThat(reloaded.updateDate().before(issue.updateDate())).isFalse(); - } - - @Test - public void fail_if_unknown_rule() throws Exception { - try { - adminIssueClient().create(NewIssue.create().component(COMPONENT_KEY) - // this rule does not exist - .rule("manual:unknown-rule") - .line(3) - .severity("CRITICAL") - .message("The name 'Sample' is too generic")); - fail(); - } catch (Exception e) { - verifyHttpException(e, 400); - } - } - - @Test - public void fail_if_missing_rule() throws Exception { - try { - adminIssueClient().create(NewIssue.create().component(COMPONENT_KEY) - .line(3) - .severity("CRITICAL") - .message("The name 'Sample' is too generic")); - fail(); - } catch (Exception e) { - verifyHttpException(e, 400); - } - } - - @Test - public void fail_if_not_a_manual_rule() throws Exception { - try { - adminIssueClient().create(NewIssue.create().component(COMPONENT_KEY) - // Not a manual rule - .rule("xoo:OneIssuePerLine") - .line(3) - .severity("CRITICAL") - .message("The name 'Sample' is too generic")); - fail(); - } catch (Exception e) { - verifyHttpException(e, 400); - } - } - - @Test - public void fail_if_rule_is_disabled() throws Exception { - // Create and delete a manual rule - ORCHESTRATOR.getServer().adminWsClient().post("/api/rules/create", ImmutableMap.of( - "manual_key", "anotherinvalidclassname", - "name", "AnotherInvalidClassName", - "markdown_description", "Another invalid class name" - )); - ORCHESTRATOR.getServer().adminWsClient().post("/api/rules/delete", ImmutableMap.of( - "key", "manual:anotherinvalidclassname" - )); - - try { - adminIssueClient().create(NewIssue.create().component(COMPONENT_KEY) - // This rule is disabled - .rule("manual:anotherinvalidclassname") - .line(3) - .severity("CRITICAL") - .message("The name 'Sample' is too generic")); - fail(); - } catch (Exception e) { - verifyHttpException(e, 400); - } - } - - @Test - public void fail_if_component_does_not_exist() throws Exception { - try { - adminIssueClient().create(NewIssue.create().component("unknown component") - .rule("manual:invalidclassname") - .line(3) - .severity("CRITICAL") - .message("The name 'Sample' is too generic")); - fail(); - } catch (Exception e) { - verifyHttpException(e, 400); - } - } - - @Test - public void fail_if_not_logged_in() throws Exception { - try { - issueClient().create(NewIssue.create().component("unknown component") - .rule("manual:invalidclassname") - .line(3) - .severity("CRITICAL") - .message("The name 'Sample' is too generic")); - fail(); - } catch (Exception e) { - verifyHttpException(e, 401); - } - } - - private static void analyzeProject() { - // no active rules - runProjectAnalysis(ORCHESTRATOR, "shared/xoo-sample"); - } - - private static void createManualRule() { - ORCHESTRATOR.getServer().adminWsClient().post("/api/rules/create", ImmutableMap.of( - "manual_key", "invalidclassname", - "name", "InvalidClassName", - "markdown_description", "Invalid class name" - )); - } - - private static List searchIssuesByComponent(String componentKey) { - return search(IssueQuery.create().components(componentKey)).list(); - } - - private static Issue searchIssueWithComments(String issueKey) { - return searchIssue(issueKey, true); - } -} diff --git a/server/sonar-server/src/main/java/org/sonar/server/component/ws/AppAction.java b/server/sonar-server/src/main/java/org/sonar/server/component/ws/AppAction.java index 09e9169ca3d..71580811805 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/component/ws/AppAction.java +++ b/server/sonar-server/src/main/java/org/sonar/server/component/ws/AppAction.java @@ -150,7 +150,6 @@ public class AppAction implements RequestHandler { private static void appendPermissions(JsonWriter json, ComponentDto component, UserSession userSession) { boolean hasBrowsePermission = userSession.hasComponentPermission(UserRole.USER, component.key()); json.prop("canMarkAsFavourite", userSession.isLoggedIn() && hasBrowsePermission); - json.prop("canCreateManualIssue", userSession.isLoggedIn() && hasBrowsePermission); } private void appendMeasures(JsonWriter json, Map measuresByMetricKey) { diff --git a/server/sonar-server/src/main/java/org/sonar/server/issue/IssueService.java b/server/sonar-server/src/main/java/org/sonar/server/issue/IssueService.java index 190af4ebc68..e8f36a9e016 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/issue/IssueService.java +++ b/server/sonar-server/src/main/java/org/sonar/server/issue/IssueService.java @@ -19,7 +19,6 @@ */ package org.sonar.server.issue; -import com.google.common.base.Objects; import com.google.common.base.Optional; import com.google.common.base.Strings; import java.util.ArrayList; @@ -28,13 +27,11 @@ import java.util.Collections; import java.util.Date; import java.util.List; import java.util.Map; -import javax.annotation.CheckForNull; import javax.annotation.Nullable; import org.apache.commons.lang.StringUtils; import org.sonar.api.issue.Issue; import org.sonar.api.rule.RuleKey; import org.sonar.api.rule.RuleStatus; -import org.sonar.api.rule.Severity; import org.sonar.api.rules.RuleType; import org.sonar.api.ce.ComputeEngineSide; import org.sonar.api.server.ServerSide; @@ -42,13 +39,11 @@ import org.sonar.api.user.User; import org.sonar.api.user.UserFinder; import org.sonar.api.web.UserRole; import org.sonar.core.issue.DefaultIssue; -import org.sonar.core.issue.DefaultIssueBuilder; import org.sonar.core.issue.IssueChangeContext; import org.sonar.db.DbClient; import org.sonar.db.DbSession; import org.sonar.db.component.ComponentDto; import org.sonar.db.issue.IssueDto; -import org.sonar.db.protobuf.DbFileSources; import org.sonar.db.rule.RuleDto; import org.sonar.server.exceptions.BadRequestException; import org.sonar.server.issue.index.IssueIndex; @@ -56,10 +51,7 @@ import org.sonar.server.issue.notification.IssueChangeNotification; import org.sonar.server.issue.workflow.IssueWorkflow; import org.sonar.server.issue.workflow.Transition; import org.sonar.server.notification.NotificationManager; -import org.sonar.server.source.SourceService; import org.sonar.server.user.UserSession; -import org.sonar.server.user.index.UserDoc; -import org.sonar.server.user.index.UserIndex; @ServerSide @ComputeEngineSide @@ -73,8 +65,6 @@ public class IssueService { private final IssueStorage issueStorage; private final NotificationManager notificationService; private final UserFinder userFinder; - private final UserIndex userIndex; - private final SourceService sourceService; private final UserSession userSession; public IssueService(DbClient dbClient, IssueIndex issueIndex, @@ -83,7 +73,7 @@ public class IssueService { IssueUpdater issueUpdater, NotificationManager notificationService, UserFinder userFinder, - UserIndex userIndex, SourceService sourceService, UserSession userSession) { + UserSession userSession) { this.dbClient = dbClient; this.issueIndex = issueIndex; this.workflow = workflow; @@ -91,8 +81,6 @@ public class IssueService { this.issueUpdater = issueUpdater; this.notificationService = notificationService; this.userFinder = userFinder; - this.userIndex = userIndex; - this.sourceService = sourceService; this.userSession = userSession; } @@ -218,49 +206,6 @@ public class IssueService { } } - public Issue createManualIssue(String componentKey, RuleKey ruleKey, @Nullable Integer line, @Nullable String message, @Nullable String severity) { - userSession.checkLoggedIn(); - - DbSession dbSession = dbClient.openSession(false); - try { - Optional componentOptional = dbClient.componentDao().selectByKey(dbSession, componentKey); - if (!componentOptional.isPresent()) { - throw new BadRequestException(String.format("Component with key '%s' not found", componentKey)); - } - ComponentDto component = componentOptional.get(); - ComponentDto project = dbClient.componentDao().selectOrFailByUuid(dbSession, component.projectUuid()); - - userSession.checkComponentPermission(UserRole.USER, project.getKey()); - if (!ruleKey.isManual()) { - throw new IllegalArgumentException("Issues can be created only on rules marked as 'manual': " + ruleKey); - } - Optional rule = getRuleByKey(dbSession, ruleKey); - if (!rule.isPresent()) { - throw new IllegalArgumentException("Unknown rule: " + ruleKey); - } - - DefaultIssue issue = new DefaultIssueBuilder() - .componentKey(component.getKey()) - .type(RuleType.valueOf(rule.get().getType())) - .projectKey(project.getKey()) - .line(line) - .message(!Strings.isNullOrEmpty(message) ? message : rule.get().getName()) - .severity(Objects.firstNonNull(severity, Severity.MAJOR)) - .ruleKey(ruleKey) - .reporter(userSession.getLogin()) - .assignee(findSourceLineUser(dbSession, component.uuid(), line)) - .build(); - - Date now = new Date(); - issue.setCreationDate(now); - issue.setUpdateDate(now); - issueStorage.save(issue); - return issue; - } finally { - dbSession.close(); - } - } - public Issue getByKey(String key) { return issueIndex.getByKey(key); } @@ -335,17 +280,4 @@ public class IssueService { return issueIndex.countTags(query, pageSize); } - @CheckForNull - private String findSourceLineUser(DbSession dbSession, String fileUuid, @Nullable Integer line) { - if (line != null) { - Optional sourceLine = sourceService.getLine(dbSession, fileUuid, line); - if (sourceLine.isPresent() && sourceLine.get().hasScmAuthor()) { - UserDoc userDoc = userIndex.getNullableByScmAccount(sourceLine.get().getScmAuthor()); - if (userDoc != null) { - return userDoc.login(); - } - } - } - return null; - } } diff --git a/server/sonar-server/src/main/java/org/sonar/server/issue/ws/CreateAction.java b/server/sonar-server/src/main/java/org/sonar/server/issue/ws/CreateAction.java index fb3bf745ddc..a54b2628fd6 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/issue/ws/CreateAction.java +++ b/server/sonar-server/src/main/java/org/sonar/server/issue/ws/CreateAction.java @@ -19,13 +19,10 @@ */ package org.sonar.server.issue.ws; -import org.sonar.api.issue.Issue; -import org.sonar.api.rule.RuleKey; import org.sonar.api.rule.Severity; import org.sonar.api.server.ws.Request; import org.sonar.api.server.ws.Response; import org.sonar.api.server.ws.WebService; -import org.sonar.server.issue.IssueService; import static org.sonar.server.ws.KeyExamples.KEY_PROJECT_EXAMPLE_001; @@ -33,19 +30,12 @@ public class CreateAction implements IssuesWsAction { public static final String ACTION = "create"; - private final IssueService issueService; - private final OperationResponseWriter responseWriter; - - public CreateAction(IssueService issueService, OperationResponseWriter responseWriter) { - this.issueService = issueService; - this.responseWriter = responseWriter; - } - @Override public void define(WebService.NewController controller) { WebService.NewAction action = controller.createAction(ACTION) - .setDescription("Create a manual issue. Requires authentication and Browse permission on project") + .setDescription("Deprecated web service to do create a manual issue. Manual issue feature has been removed in 5.5. This web service has no effect.") .setSince("3.6") + .setDeprecatedSince("5.5") .setHandler(this) .setPost(true); @@ -72,15 +62,6 @@ public class CreateAction implements IssuesWsAction { @Override public void handle(Request request, Response response) throws Exception { - // required parameters - String componentKey = request.mandatoryParam("component"); - RuleKey ruleKey = RuleKey.parse(request.mandatoryParam("rule")); - - Issue issue = issueService.createManualIssue(componentKey, ruleKey, - request.paramAsInt("line"), - request.param("message"), - request.param("severity")); - - responseWriter.write(issue.key(), request, response); + response.noContent(); } } diff --git a/server/sonar-server/src/main/java/org/sonar/server/source/SourceService.java b/server/sonar-server/src/main/java/org/sonar/server/source/SourceService.java index 0d9034461a4..57689404bc4 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/source/SourceService.java +++ b/server/sonar-server/src/main/java/org/sonar/server/source/SourceService.java @@ -62,23 +62,6 @@ public class SourceService { return getLines(dbSession, fileUuid, from, toInclusive, lineToHtml()); } - /** - * Returns a single line from a source file. {@code Optional.absent()} is returned if the - * file or the line do not exist. - * @param line starts from 1 - */ - public Optional getLine(DbSession dbSession, String fileUuid, int line) { - verifyLine(line); - FileSourceDto dto = dbClient.fileSourceDao().selectSourceByFileUuid(dbSession, fileUuid); - if (dto == null) { - return Optional.absent(); - } - DbFileSources.Data data = dto.getSourceData(); - return FluentIterable.from(data.getLinesList()) - .filter(new IsGreaterOrEqualThanLine(line)) - .first(); - } - private Optional> getLines(DbSession dbSession, String fileUuid, int from, int toInclusive, Function function) { verifyLine(from); Preconditions.checkArgument(toInclusive >= from, String.format("Line number must greater than or equal to %d, got %d", from, toInclusive)); diff --git a/server/sonar-server/src/main/java/org/sonar/server/user/index/UserIndex.java b/server/sonar-server/src/main/java/org/sonar/server/user/index/UserIndex.java index ab5d886f254..fafc0648833 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/user/index/UserIndex.java +++ b/server/sonar-server/src/main/java/org/sonar/server/user/index/UserIndex.java @@ -85,19 +85,6 @@ public class UserIndex { return null; } - /** - * Returns the user associated with the given SCM account. If multiple users have the same - * SCM account, then result is null. - */ - @CheckForNull - public UserDoc getNullableByScmAccount(String scmAccount) { - List users = getAtMostThreeActiveUsersForScmAccount(scmAccount); - if (users.size() == 1) { - return users.get(0); - } - return null; - } - public UserDoc getByLogin(String login) { UserDoc userDoc = getNullableByLogin(login); if (userDoc == null) { diff --git a/server/sonar-server/src/test/java/org/sonar/server/issue/IssueServiceMediumTest.java b/server/sonar-server/src/test/java/org/sonar/server/issue/IssueServiceMediumTest.java index d8009ea2ac8..5a6d8846cda 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/issue/IssueServiceMediumTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/issue/IssueServiceMediumTest.java @@ -28,12 +28,8 @@ import org.junit.Before; import org.junit.ClassRule; import org.junit.Rule; import org.junit.Test; -import org.sonar.api.CoreProperties; -import org.sonar.api.config.Settings; import org.sonar.api.issue.DefaultTransitions; import org.sonar.api.issue.Issue; -import org.sonar.api.rule.RuleKey; -import org.sonar.api.rule.RuleStatus; import org.sonar.api.rule.Severity; import org.sonar.api.rules.RuleType; import org.sonar.api.security.DefaultGroups; @@ -46,18 +42,11 @@ import org.sonar.db.component.ComponentDto; import org.sonar.db.component.ComponentTesting; import org.sonar.db.issue.IssueDao; import org.sonar.db.issue.IssueDto; -import org.sonar.db.protobuf.DbFileSources; import org.sonar.db.rule.RuleDao; import org.sonar.db.rule.RuleDto; import org.sonar.db.rule.RuleTesting; -import org.sonar.db.source.FileSourceDao; -import org.sonar.db.source.FileSourceDto; -import org.sonar.db.user.GroupDao; -import org.sonar.db.user.GroupDto; import org.sonar.db.user.UserDto; import org.sonar.server.exceptions.BadRequestException; -import org.sonar.server.exceptions.ForbiddenException; -import org.sonar.server.issue.index.IssueDoc; import org.sonar.server.issue.index.IssueIndex; import org.sonar.server.issue.index.IssueIndexer; import org.sonar.server.issue.workflow.Transition; @@ -66,8 +55,6 @@ import org.sonar.server.permission.PermissionUpdater; import org.sonar.server.rule.index.RuleIndexer; import org.sonar.server.tester.ServerTester; import org.sonar.server.tester.UserSessionRule; -import org.sonar.server.user.NewUser; -import org.sonar.server.user.UserUpdater; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.entry; @@ -241,185 +228,6 @@ public class IssueServiceMediumTest { assertThat(IssueIndex.getByKey(issue.getKey()).type()).isEqualTo(RuleType.BUG); } - @Test - public void create_manual_issue() { - ComponentDto project = newProject(); - ComponentDto file = newFile(project); - userSessionRule.login("john").addProjectPermissions(UserRole.USER, project.key()); - - RuleDto manualRule = RuleTesting.newManualRule("manualRuleKey"); - tester.get(RuleDao.class).insert(session, manualRule); - session.commit(); - - Issue result = service.createManualIssue(file.key(), manualRule.getKey(), null, "Fix it", Severity.MINOR); - - IssueDoc manualIssue = IssueIndex.getByKey(result.key()); - assertThat(manualIssue.componentUuid()).isEqualTo(file.uuid()); - assertThat(manualIssue.projectUuid()).isEqualTo(project.uuid()); - assertThat(manualIssue.ruleKey()).isEqualTo(manualRule.getKey()); - assertThat(manualIssue.message()).isEqualTo("Fix it"); - assertThat(manualIssue.line()).isNull(); - assertThat(manualIssue.severity()).isEqualTo(Severity.MINOR); - } - - @Test - public void create_manual_issue_on_line() { - ComponentDto project = newProject(); - ComponentDto file = newFile(project); - newSourceLine(file, 1, "arthur"); - createDefaultGroup(); - newUser("arthur"); - userSessionRule.login("john").addProjectPermissions(UserRole.USER, project.key()); - - RuleDto manualRule = RuleTesting.newManualRule("manualRuleKey"); - tester.get(RuleDao.class).insert(session, manualRule); - session.commit(); - - Issue result = service.createManualIssue(file.key(), manualRule.getKey(), 1, "Fix it", Severity.MINOR); - - IssueDoc manualIssue = IssueIndex.getByKey(result.key()); - assertThat(manualIssue.componentUuid()).isEqualTo(file.uuid()); - assertThat(manualIssue.projectUuid()).isEqualTo(project.uuid()); - assertThat(manualIssue.ruleKey()).isEqualTo(manualRule.getKey()); - assertThat(manualIssue.message()).isEqualTo("Fix it"); - assertThat(manualIssue.line()).isEqualTo(1); - assertThat(manualIssue.severity()).isEqualTo(Severity.MINOR); - assertThat(manualIssue.gap()).isNull(); - assertThat(manualIssue.assignee()).isEqualTo("arthur"); - } - - @Test - public void create_manual_issue_with_major_severity_when_no_severity() { - ComponentDto project = newProject(); - ComponentDto file = newFile(project); - userSessionRule.login("john").addProjectPermissions(UserRole.USER, project.key()); - - RuleDto manualRule = RuleTesting.newManualRule("manualRuleKey"); - tester.get(RuleDao.class).insert(session, manualRule); - session.commit(); - - Issue result = service.createManualIssue(file.key(), manualRule.getKey(), null, "Fix it", null); - - Issue manualIssue = IssueIndex.getByKey(result.key()); - assertThat(manualIssue.severity()).isEqualTo(Severity.MAJOR); - } - - @Test - public void create_manual_issue_with_rule_name_when_no_message() { - ComponentDto project = newProject(); - ComponentDto file = newFile(project); - userSessionRule.login("john").addProjectPermissions(UserRole.USER, project.key()); - - RuleDto manualRule = RuleTesting.newManualRule("manualRuleKey").setName("Manual rule name"); - tester.get(RuleDao.class).insert(session, manualRule); - session.commit(); - - Issue result = service.createManualIssue(file.key(), manualRule.getKey(), null, null, null); - - Issue manualIssue = IssueIndex.getByKey(result.key()); - assertThat(manualIssue.message()).isEqualTo("Manual rule name"); - } - - @Test - public void create_manual_issue_without_assignee_when_scm_author_do_not_match_user() { - ComponentDto project = newProject(); - ComponentDto file = newFile(project); - // Unknown SCM account - newSourceLine(file, 1, "unknown"); - createDefaultGroup(); - newUser("arthur"); - userSessionRule.login("john").addProjectPermissions(UserRole.USER, project.key()); - - RuleDto manualRule = RuleTesting.newManualRule("manualRuleKey"); - tester.get(RuleDao.class).insert(session, manualRule); - session.commit(); - - Issue result = service.createManualIssue(file.key(), manualRule.getKey(), 1, "Fix it", Severity.MINOR); - - IssueDoc manualIssue = IssueIndex.getByKey(result.key()); - assertThat(manualIssue.assignee()).isNull(); - } - - @Test - public void create_manual_issue_without_assignee_when_no_scm_author_on_line() { - ComponentDto project = newProject(); - ComponentDto file = newFile(project); - // No author on line 1 - newSourceLine(file, 1, ""); - userSessionRule.login("john").addProjectPermissions(UserRole.USER, project.key()); - - RuleDto manualRule = RuleTesting.newManualRule("manualRuleKey"); - tester.get(RuleDao.class).insert(session, manualRule); - session.commit(); - - Issue result = service.createManualIssue(file.key(), manualRule.getKey(), 1, "Fix it", Severity.MINOR); - - IssueDoc manualIssue = IssueIndex.getByKey(result.key()); - assertThat(manualIssue.assignee()).isNull(); - } - - @Test - public void fail_create_manual_issue_on_not_manual_rule() { - RuleDto rule = newRule(); - ComponentDto project = newProject(); - ComponentDto file = newFile(project); - userSessionRule.login("john").addProjectPermissions(UserRole.USER, project.key()); - - try { - service.createManualIssue(file.key(), rule.getKey(), null, "Fix it", null); - fail(); - } catch (Exception e) { - assertThat(e).isInstanceOf(IllegalArgumentException.class).hasMessage("Issues can be created only on rules marked as 'manual': xoo:x1"); - } - } - - @Test(expected = IllegalArgumentException.class) - public void fail_create_manual_issue_if_rule_does_not_exist() { - ComponentDto project = newProject(); - ComponentDto file = newFile(project); - userSessionRule.login("john").addProjectPermissions(UserRole.USER, project.key()); - - service.createManualIssue(file.key(), RuleKey.of("rule", "unknown"), 10, "Fix it", null); - } - - @Test(expected = ForbiddenException.class) - public void fail_create_manual_issue_if_not_having_required_role() { - RuleDto rule = newRule(); - ComponentDto project = newProject(); - ComponentDto file = newFile(project); - - // User has not the 'user' role on the project - userSessionRule.login("john").addProjectPermissions(UserRole.CODEVIEWER, project.key()); - - RuleDto manualRule = RuleTesting.newManualRule("manualRuleKey"); - tester.get(RuleDao.class).insert(session, manualRule); - session.commit(); - - service.createManualIssue(file.key(), rule.getKey(), 10, "Fix it", null); - } - - @Test(expected = BadRequestException.class) - public void fail_to_create_manual_issue_on_unknown_component() { - ComponentDto project = newProject(); - userSessionRule.login("john").addProjectPermissions(UserRole.USER, project.key()); - - RuleDto manualRule = RuleTesting.newManualRule("manualRuleKey"); - tester.get(RuleDao.class).insert(session, manualRule); - session.commit(); - - service.createManualIssue("UNKNOWN", manualRule.getKey(), null, "Fix it", Severity.MINOR); - } - - @Test(expected = IllegalArgumentException.class) - public void fail_create_manual_issue_on_removed_rule() { - RuleDto rule = newRule(RuleTesting.newXooX1().setStatus(RuleStatus.REMOVED)); - ComponentDto project = newProject(); - ComponentDto file = newFile(project); - userSessionRule.login("john").addProjectPermissions(UserRole.USER, project.key()); - - service.createManualIssue(file.key(), rule.getKey(), 10, "Fix it", null); - } - @Test public void list_tags() { RuleDto rule = newRule(); @@ -549,28 +357,4 @@ public class IssueServiceMediumTest { return issue; } - private void newSourceLine(ComponentDto file, int line, String scmAuthor) { - DbFileSources.Data.Builder dataBuilder = DbFileSources.Data.newBuilder(); - dataBuilder.addLinesBuilder() - .setLine(line) - .setScmAuthor(scmAuthor) - .build(); - FileSourceDto dto = new FileSourceDto(); - dto.setProjectUuid(file.projectUuid()); - dto.setFileUuid(file.uuid()); - dto.setCreatedAt(System.currentTimeMillis()); - dto.setSourceData(dataBuilder.build()); - dto.setDataType(FileSourceDto.Type.SOURCE); - tester.get(FileSourceDao.class).insert(dto); - } - - private void newUser(String login) { - tester.get(UserUpdater.class).create(NewUser.create().setLogin(login).setName(login).setPassword("test")); - } - - private void createDefaultGroup() { - tester.get(Settings.class).setProperty(CoreProperties.CORE_DEFAULT_GROUP, "sonar-users"); - tester.get(GroupDao.class).insert(session, new GroupDto().setName("sonar-users").setDescription("Sonar Users")); - session.commit(); - } } diff --git a/server/sonar-server/src/test/java/org/sonar/server/issue/ws/CreateActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/issue/ws/CreateActionTest.java index ca317cfa73b..cfccf1c9e54 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/issue/ws/CreateActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/issue/ws/CreateActionTest.java @@ -20,57 +20,20 @@ package org.sonar.server.issue.ws; import org.junit.Test; -import org.sonar.api.rule.RuleKey; -import org.sonar.api.server.ws.Request; -import org.sonar.api.server.ws.Response; -import org.sonar.core.issue.DefaultIssue; -import org.sonar.server.issue.IssueService; import org.sonar.server.ws.WsAction; import org.sonar.server.ws.WsActionTester; -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; - public class CreateActionTest { - IssueService issueService = mock(IssueService.class); - OperationResponseWriter responseWriter = mock(OperationResponseWriter.class); - WsAction underTest = new CreateAction(issueService, responseWriter); + WsAction underTest = new CreateAction(); WsActionTester tester = new WsActionTester(underTest); @Test - public void create_manual_issue_with_default_values() throws Exception { - RuleKey ruleKey = RuleKey.of(RuleKey.MANUAL_REPOSITORY_KEY, "S1"); - when(issueService.createManualIssue("FILE_KEY", ruleKey, null, null, null)) - .thenReturn(new DefaultIssue().setKey("ISSUE_KEY")); - + public void does_nothing() throws Exception { tester.newRequest() .setParam("component", "FILE_KEY") - .setParam("rule", ruleKey.toString()) + .setParam("rule", "ruleKey") .execute(); - - verify(issueService).createManualIssue("FILE_KEY", ruleKey, null, null, null); - verify(responseWriter).write(eq("ISSUE_KEY"), any(Request.class), any(Response.class)); } - @Test - public void create_manual_issue() throws Exception { - RuleKey ruleKey = RuleKey.of(RuleKey.MANUAL_REPOSITORY_KEY, "S1"); - when(issueService.createManualIssue("FILE_KEY", ruleKey, 13, "the msg", "BLOCKER")) - .thenReturn(new DefaultIssue().setKey("ISSUE_KEY")); - - tester.newRequest() - .setParam("component", "FILE_KEY") - .setParam("rule", ruleKey.toString()) - .setParam("severity", "BLOCKER") - .setParam("line", "13") - .setParam("message", "the msg") - .execute(); - - verify(issueService).createManualIssue("FILE_KEY", ruleKey, 13, "the msg", "BLOCKER"); - verify(responseWriter).write(eq("ISSUE_KEY"), any(Request.class), any(Response.class)); - } } diff --git a/server/sonar-server/src/test/java/org/sonar/server/source/SourceServiceTest.java b/server/sonar-server/src/test/java/org/sonar/server/source/SourceServiceTest.java index b05daf018fc..05c73508927 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/source/SourceServiceTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/source/SourceServiceTest.java @@ -119,23 +119,4 @@ public class SourceServiceTest { assertThat(lines.isPresent()).isFalse(); } - @Test - public void getLine() throws Exception { - Optional line = underTest.getLine(dbTester.getSession(), FILE_UUID, 4); - assertThat(line.isPresent()).isTrue(); - assertThat(line.get().getLine()).isEqualTo(4); - assertThat(line.get().getSource()).isEqualTo("SOURCE_4"); - } - - @Test - public void getLine_absent_line() throws Exception { - Optional line = underTest.getLine(dbTester.getSession(), FILE_UUID, 500); - assertThat(line.isPresent()).isFalse(); - } - - @Test - public void getLine_absent_file() throws Exception { - Optional line = underTest.getLine(dbTester.getSession(), "FILE_DOES_NOT_EXIST", 10); - assertThat(line.isPresent()).isFalse(); - } } diff --git a/server/sonar-server/src/test/java/org/sonar/server/user/index/UserIndexTest.java b/server/sonar-server/src/test/java/org/sonar/server/user/index/UserIndexTest.java index b0e58b69a89..cc5bce8e65c 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/user/index/UserIndexTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/user/index/UserIndexTest.java @@ -94,25 +94,6 @@ public class UserIndexTest { } } - @Test - public void get_nullable_by_scm_account() throws Exception { - esTester.putDocuments(UserIndexDefinition.INDEX, UserIndexDefinition.TYPE_USER, this.getClass(), "user1.json", "user2.json"); - - assertThat(index.getNullableByScmAccount("user_1").login()).isEqualTo("user1"); - assertThat(index.getNullableByScmAccount("user1@mail.com").login()).isEqualTo("user1"); - assertThat(index.getNullableByScmAccount("user1").login()).isEqualTo("user1"); - - assertThat(index.getNullableByScmAccount("")).isNull(); - assertThat(index.getNullableByScmAccount("unknown")).isNull(); - } - - @Test - public void get_nullable_by_scm_account_return_null_when_two_users_have_same_email() throws Exception { - esTester.putDocuments(UserIndexDefinition.INDEX, UserIndexDefinition.TYPE_USER, this.getClass(), "user1.json", "user3-with-same-email-as-user1.json"); - - assertThat(index.getNullableByScmAccount("user1@mail.com")).isNull(); - } - @Test public void getAtMostThreeActiveUsersForScmAccount() throws Exception { esTester.putDocuments(UserIndexDefinition.INDEX, UserIndexDefinition.TYPE_USER, this.getClass(), "user1.json", "user3-with-same-email-as-user1.json", "inactive-user.json"); diff --git a/server/sonar-server/src/test/resources/org/sonar/server/component/ws/AppActionTest/app.json b/server/sonar-server/src/test/resources/org/sonar/server/component/ws/AppActionTest/app.json index 201fc70537f..43319c9e2a3 100644 --- a/server/sonar-server/src/test/resources/org/sonar/server/component/ws/AppActionTest/app.json +++ b/server/sonar-server/src/test/resources/org/sonar/server/component/ws/AppActionTest/app.json @@ -11,6 +11,5 @@ "projectName": "SonarQube", "fav": true, "canMarkAsFavourite": true, - "canCreateManualIssue": true, "measures": {} } diff --git a/server/sonar-server/src/test/resources/org/sonar/server/component/ws/AppActionTest/app_with_it_measure.json b/server/sonar-server/src/test/resources/org/sonar/server/component/ws/AppActionTest/app_with_it_measure.json index 0b233ce08e2..56210aad540 100644 --- a/server/sonar-server/src/test/resources/org/sonar/server/component/ws/AppActionTest/app_with_it_measure.json +++ b/server/sonar-server/src/test/resources/org/sonar/server/component/ws/AppActionTest/app_with_it_measure.json @@ -11,6 +11,5 @@ "projectName": "SonarQube", "fav": false, "canMarkAsFavourite": false, - "canCreateManualIssue": false, "measures": {"coverage": "85.2%"} } diff --git a/server/sonar-server/src/test/resources/org/sonar/server/component/ws/AppActionTest/app_with_measures.json b/server/sonar-server/src/test/resources/org/sonar/server/component/ws/AppActionTest/app_with_measures.json index 606f6594f53..66d5394790a 100644 --- a/server/sonar-server/src/test/resources/org/sonar/server/component/ws/AppActionTest/app_with_measures.json +++ b/server/sonar-server/src/test/resources/org/sonar/server/component/ws/AppActionTest/app_with_measures.json @@ -11,7 +11,6 @@ "projectName": "SonarQube", "fav": false, "canMarkAsFavourite": false, - "canCreateManualIssue": false, "measures": { "lines": "200", "coverage": "95.4%", diff --git a/server/sonar-server/src/test/resources/org/sonar/server/component/ws/AppActionTest/app_with_overall_measure.json b/server/sonar-server/src/test/resources/org/sonar/server/component/ws/AppActionTest/app_with_overall_measure.json index 7cc5aedb715..ec96dd5d243 100644 --- a/server/sonar-server/src/test/resources/org/sonar/server/component/ws/AppActionTest/app_with_overall_measure.json +++ b/server/sonar-server/src/test/resources/org/sonar/server/component/ws/AppActionTest/app_with_overall_measure.json @@ -11,6 +11,5 @@ "projectName": "SonarQube", "fav": false, "canMarkAsFavourite": false, - "canCreateManualIssue": false, "measures": {"coverage": "90.1%"} } diff --git a/server/sonar-server/src/test/resources/org/sonar/server/component/ws/AppActionTest/app_with_ut_measure.json b/server/sonar-server/src/test/resources/org/sonar/server/component/ws/AppActionTest/app_with_ut_measure.json index c6ef590ad5d..ff6d9282226 100644 --- a/server/sonar-server/src/test/resources/org/sonar/server/component/ws/AppActionTest/app_with_ut_measure.json +++ b/server/sonar-server/src/test/resources/org/sonar/server/component/ws/AppActionTest/app_with_ut_measure.json @@ -11,6 +11,5 @@ "projectName": "SonarQube", "fav": false, "canMarkAsFavourite": false, - "canCreateManualIssue": false, "measures": {"coverage": "95.4%"} } diff --git a/sonar-ws/src/main/protobuf/ws-issues.proto b/sonar-ws/src/main/protobuf/ws-issues.proto index a253c424402..c5c76bcad78 100644 --- a/sonar-ws/src/main/protobuf/ws-issues.proto +++ b/sonar-ws/src/main/protobuf/ws-issues.proto @@ -79,7 +79,9 @@ message Issue { optional string debt = 14; optional string assignee = 15; - optional string reporter = 16; + + // Unused since 5.5, manual issues feature has been removed + optional string unusedReporter = 16; // SCM login of the committer who introduced the issue optional string author = 17; -- 2.39.5