From a5ae3c2a46b7426ad0463a9882a5adb9b6e091ef Mon Sep 17 00:00:00 2001 From: Eric Giffon Date: Thu, 7 Dec 2023 14:39:23 +0100 Subject: [PATCH] SONAR-21131 Add rule controller unit tests --- .../server/common/rule/RuleCreatorIT.java | 38 +++++ .../sonar/server/common/rule/RuleCreator.java | 3 + .../controller/DefaultRuleController.java | 4 +- .../server/v2/api/rule/request/Impact.java | 37 ----- .../controller/DefaultRuleControllerTest.java | 156 ++++++++++++++++-- .../src/test/resources/logback-test.xml | 17 ++ 6 files changed, 198 insertions(+), 57 deletions(-) delete mode 100644 server/sonar-webserver-webapi-v2/src/main/java/org/sonar/server/v2/api/rule/request/Impact.java create mode 100644 server/sonar-webserver-webapi-v2/src/test/resources/logback-test.xml diff --git a/server/sonar-webserver-common/src/it/java/org/sonar/server/common/rule/RuleCreatorIT.java b/server/sonar-webserver-common/src/it/java/org/sonar/server/common/rule/RuleCreatorIT.java index 6b36fa642ac..1d32224d360 100644 --- a/server/sonar-webserver-common/src/it/java/org/sonar/server/common/rule/RuleCreatorIT.java +++ b/server/sonar-webserver-common/src/it/java/org/sonar/server/common/rule/RuleCreatorIT.java @@ -55,6 +55,7 @@ import org.sonar.server.rule.index.RuleIndexer; import org.sonar.server.rule.index.RuleQuery; import static java.util.Collections.singletonList; +import static java.util.Objects.requireNonNull; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; import static org.assertj.core.api.Assertions.assertThatThrownBy; @@ -66,6 +67,8 @@ import static org.sonar.api.issue.impact.Severity.MEDIUM; import static org.sonar.api.issue.impact.SoftwareQuality.MAINTAINABILITY; import static org.sonar.api.issue.impact.SoftwareQuality.RELIABILITY; import static org.sonar.api.issue.impact.SoftwareQuality.SECURITY; +import static org.sonar.api.server.rule.internal.ImpactMapper.convertToImpactSeverity; +import static org.sonar.api.server.rule.internal.ImpactMapper.convertToSoftwareQuality; import static org.sonar.db.rule.RuleTesting.newCustomRule; import static org.sonar.db.rule.RuleTesting.newRule; import static org.sonar.server.util.TypeValidationsTesting.newFullTypeValidations; @@ -174,6 +177,26 @@ public class RuleCreatorIT { assertThat(rule.getSeverityString()).isEqualTo(Severity.MINOR); } + @Test + public void create_whenFieldsNotSpecified_shouldSetDefaultValues() { + // insert template rule + RuleDto templateRule = createTemplateRule(); + // Create custom rule + NewCustomRule newRule = NewCustomRule.createForCustomRule(CUSTOM_RULE_KEY, templateRule.getKey()) + .setName("My custom") + .setMarkdownDescription("Some description"); + RuleKey customRuleKey = underTest.create(dbSession, newRule).getKey(); + + RuleDto rule = dbTester.getDbClient().ruleDao().selectOrFailByKey(dbSession, customRuleKey); + assertThat(rule).isNotNull(); + assertThat(rule.getStatus()).isEqualTo(RuleStatus.READY); + assertThat(rule.getSeverityString()).isEqualTo(Severity.MAJOR); + assertThat(rule.getCleanCodeAttribute()).isEqualTo(CleanCodeAttribute.CONVENTIONAL); + assertThat(rule.getDefaultImpacts()).extracting(ImpactDto::getSoftwareQuality, ImpactDto::getSeverity).containsExactly(tuple( + convertToSoftwareQuality(RuleType.valueOf(templateRule.getType())), + convertToImpactSeverity(requireNonNull(Severity.MAJOR)))); + } + @Test public void create_whenImpactsAndTypeAreSet_shouldFail() { // insert template rule @@ -227,6 +250,21 @@ public class RuleCreatorIT { .hasMessage("Custom and template keys must be in the same repository"); } + @Test + public void create_whenRuleStatusRemoved_shouldFail() { + // insert template rule + RuleDto templateRule = createTemplateRule(); + // Create custom rule + NewCustomRule newRule = NewCustomRule.createForCustomRule(CUSTOM_RULE_KEY, templateRule.getKey()) + .setName("My custom") + .setMarkdownDescription("Some description") + .setStatus(RuleStatus.REMOVED); + + assertThatThrownBy(() -> underTest.create(dbSession, newRule)) + .isInstanceOf(BadRequestException.class) + .hasMessage("Rule status 'REMOVED' is not allowed"); + } + @Test public void create_custom_rule_with_empty_parameter_value() { // insert template rule diff --git a/server/sonar-webserver-common/src/main/java/org/sonar/server/common/rule/RuleCreator.java b/server/sonar-webserver-common/src/main/java/org/sonar/server/common/rule/RuleCreator.java index bcae391b0e2..9fabf677093 100644 --- a/server/sonar-webserver-common/src/main/java/org/sonar/server/common/rule/RuleCreator.java +++ b/server/sonar-webserver-common/src/main/java/org/sonar/server/common/rule/RuleCreator.java @@ -131,6 +131,9 @@ public class RuleCreator { validateName(errors, newRule); validateDescription(errors, newRule); + if (newRule.status() == RuleStatus.REMOVED) { + errors.add(format("Rule status '%s' is not allowed", RuleStatus.REMOVED)); + } String severity = newRule.severity(); if (severity != null && !Severity.ALL.contains(severity)) { errors.add(format("Severity \"%s\" is invalid", severity)); diff --git a/server/sonar-webserver-webapi-v2/src/main/java/org/sonar/server/v2/api/rule/controller/DefaultRuleController.java b/server/sonar-webserver-webapi-v2/src/main/java/org/sonar/server/v2/api/rule/controller/DefaultRuleController.java index c5fe2dcb123..89b50a583de 100644 --- a/server/sonar-webserver-webapi-v2/src/main/java/org/sonar/server/v2/api/rule/controller/DefaultRuleController.java +++ b/server/sonar-webserver-webapi-v2/src/main/java/org/sonar/server/v2/api/rule/controller/DefaultRuleController.java @@ -26,6 +26,7 @@ import org.sonar.server.common.rule.ReactivationException; import org.sonar.server.common.rule.service.NewCustomRule; import org.sonar.server.common.rule.service.RuleInformation; import org.sonar.server.common.rule.service.RuleService; +import org.sonar.server.exceptions.ServerException; import org.sonar.server.user.UserSession; import org.sonar.server.v2.api.rule.converter.RuleRestResponseGenerator; import org.sonar.server.v2.api.rule.enums.RuleStatusRestEnum; @@ -33,7 +34,6 @@ import org.sonar.server.v2.api.rule.request.RuleCreateRestRequest; import org.sonar.server.v2.api.rule.resource.Impact; import org.sonar.server.v2.api.rule.response.RuleRestResponse; import org.springframework.http.HttpStatus; -import org.springframework.web.server.ResponseStatusException; import static java.util.Optional.ofNullable; import static org.sonar.db.permission.GlobalPermission.ADMINISTER_QUALITY_PROFILES; @@ -58,7 +58,7 @@ public class DefaultRuleController implements RuleController { RuleInformation ruleInformation = ruleService.createCustomRule(toNewCustomRule(request)); return ruleRestResponseGenerator.toRuleRestResponse(ruleInformation); } catch (ReactivationException e) { - throw new ResponseStatusException(HttpStatus.CONFLICT, e.getMessage(), e); + throw new ServerException(HttpStatus.CONFLICT.value(), e.getMessage()); } } diff --git a/server/sonar-webserver-webapi-v2/src/main/java/org/sonar/server/v2/api/rule/request/Impact.java b/server/sonar-webserver-webapi-v2/src/main/java/org/sonar/server/v2/api/rule/request/Impact.java deleted file mode 100644 index 623721b7343..00000000000 --- a/server/sonar-webserver-webapi-v2/src/main/java/org/sonar/server/v2/api/rule/request/Impact.java +++ /dev/null @@ -1,37 +0,0 @@ -/* - * SonarQube - * Copyright (C) 2009-2023 SonarSource SA - * mailto:info 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 org.sonar.server.v2.api.rule.request; - -import io.swagger.v3.oas.annotations.media.Schema; -import javax.validation.constraints.NotNull; -import org.sonar.api.issue.impact.Severity; -import org.sonar.api.issue.impact.SoftwareQuality; - -public record Impact( - - @NotNull - @Schema(description = "Software quality") - SoftwareQuality softwareQuality, - - @NotNull - @Schema(description = "Severity") - Severity severity -) { -} diff --git a/server/sonar-webserver-webapi-v2/src/test/java/org/sonar/server/v2/api/rule/controller/DefaultRuleControllerTest.java b/server/sonar-webserver-webapi-v2/src/test/java/org/sonar/server/v2/api/rule/controller/DefaultRuleControllerTest.java index bf9bbd829e3..4a7e98ab6b5 100644 --- a/server/sonar-webserver-webapi-v2/src/test/java/org/sonar/server/v2/api/rule/controller/DefaultRuleControllerTest.java +++ b/server/sonar-webserver-webapi-v2/src/test/java/org/sonar/server/v2/api/rule/controller/DefaultRuleControllerTest.java @@ -19,22 +19,47 @@ */ package org.sonar.server.v2.api.rule.controller; -import org.junit.Ignore; +import com.google.gson.Gson; +import com.google.gson.GsonBuilder; +import java.util.List; +import javax.annotation.Nullable; +import org.junit.Before; import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.junit.MockitoJUnitRunner; +import org.sonar.api.resources.Languages; +import org.sonar.api.rule.RuleKey; +import org.sonar.db.rule.RuleDescriptionSectionDto; +import org.sonar.db.rule.RuleDto; +import org.sonar.db.rule.RuleTesting; +import org.sonar.server.common.rule.ReactivationException; +import org.sonar.server.common.rule.service.RuleInformation; import org.sonar.server.common.rule.service.RuleService; +import org.sonar.server.common.text.MacroInterpreter; +import org.sonar.server.language.LanguageTesting; +import org.sonar.server.rule.RuleDescriptionFormatter; import org.sonar.server.tester.UserSessionRule; import org.sonar.server.v2.api.ControllerTester; import org.sonar.server.v2.api.rule.converter.RuleRestResponseGenerator; +import org.sonar.server.v2.api.rule.enums.CleanCodeAttributeRestEnum; +import org.sonar.server.v2.api.rule.enums.ImpactSeverityRestEnum; +import org.sonar.server.v2.api.rule.enums.RuleStatusRestEnum; +import org.sonar.server.v2.api.rule.enums.SoftwareQualityRestEnum; +import org.sonar.server.v2.api.rule.request.RuleCreateRestRequest; +import org.sonar.server.v2.api.rule.resource.Impact; +import org.sonar.server.v2.api.rule.resource.Parameter; import org.sonar.server.v2.api.rule.response.RuleRestResponse; import org.springframework.http.MediaType; import org.springframework.test.web.servlet.MockMvc; +import org.springframework.test.web.servlet.MvcResult; +import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; +import static org.sonar.db.permission.GlobalPermission.ADMINISTER_QUALITY_PROFILES; import static org.sonar.server.v2.WebApiEndpoints.RULES_ENDPOINT; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.content; @@ -46,29 +71,124 @@ public class DefaultRuleControllerTest { @Rule public UserSessionRule userSession = UserSessionRule.standalone(); - private final RuleService ruleService = mock(); + private final RuleService ruleService = mock(RuleService.class); + private final Languages languages = mock(Languages.class); + private final MacroInterpreter macroInterpreter = mock(MacroInterpreter.class); + private final RuleDescriptionFormatter ruleDescriptionFormatter = mock(RuleDescriptionFormatter.class); + private final RuleRestResponseGenerator ruleRestResponseGenerator = new RuleRestResponseGenerator(languages, macroInterpreter, + ruleDescriptionFormatter); - private final RuleRestResponseGenerator ruleRestResponseGenerator = mock(); - private final MockMvc mockMvc = ControllerTester - .getMockMvc(new DefaultRuleController(userSession, ruleService, ruleRestResponseGenerator)); + private final MockMvc mockMvc = ControllerTester.getMockMvc(new DefaultRuleController(userSession, ruleService, + ruleRestResponseGenerator)); + private static final Gson gson = new GsonBuilder().create(); + @Before + public void setUp() { + when(macroInterpreter.interpret(anyString())).thenAnswer(invocation -> "interpreted" + invocation.getArgument(0)); + when(ruleDescriptionFormatter.toHtml(any(), any())).thenAnswer(invocation -> "html" + ((RuleDescriptionSectionDto) invocation.getArgument(1)).getContent()); + when(languages.get(anyString())).thenAnswer(invocation -> LanguageTesting.newLanguage(invocation.getArgument(0, String.class), + "languageName")); + } + + @Test + public void create_shouldReturnRule() throws Exception { + userSession.logIn().addPermission(ADMINISTER_QUALITY_PROFILES); + + RuleInformation ruleInformation = generateRuleInformation(); + when(ruleService.createCustomRule(any())).thenReturn(ruleInformation); + + MvcResult mvcResult = mockMvc.perform( + post(RULES_ENDPOINT) + .contentType(MediaType.APPLICATION_JSON_VALUE) + .content(gson.toJson(newRequest()))) + .andExpect(status().isOk()) + .andReturn(); + + RuleRestResponse response = gson.fromJson(mvcResult.getResponse().getContentAsString(), RuleRestResponse.class); + assertThat(response).isEqualTo(ruleRestResponseGenerator.toRuleRestResponse(ruleInformation)); + } @Test - @Ignore - public void create() throws Exception { - mockMvc.perform(post(RULES_ENDPOINT).contentType(MediaType.APPLICATION_JSON_VALUE).content("{}")) - .andExpectAll( - status().isOk()); + public void create_whenNotLoggedIn_shouldFailWithUnauthorized() throws Exception { + mockMvc.perform( + post(RULES_ENDPOINT) + .contentType(MediaType.APPLICATION_JSON_VALUE) + .content(gson.toJson(newRequest()))) + .andExpectAll(status().isUnauthorized(), content().json("{message:'Authentication is required'}")); } @Test - @Ignore - public void create_shouldReturnExpectedBody() throws Exception { - when(ruleRestResponseGenerator.toRuleRestResponse(any())).thenReturn(RuleRestResponse.Builder.builder().setId("id").build()); - - mockMvc.perform(post(RULES_ENDPOINT).contentType(MediaType.APPLICATION_JSON_VALUE).content("{}")) - .andExpectAll( - status().isOk(), - content().json("{id: 'id'}")); + public void create_whenNoPermission_shouldFailWithForbidden() throws Exception { + userSession.logIn(); + + mockMvc.perform( + post(RULES_ENDPOINT) + .contentType(MediaType.APPLICATION_JSON_VALUE) + .content(gson.toJson(newRequest()))) + .andExpectAll(status().isForbidden(), content().json("{message:'Insufficient privileges'}")); + } + + @Test + public void create_whenReactivationExceptionThrown_shouldFailWithConflict() throws Exception { + userSession.logIn().addPermission(ADMINISTER_QUALITY_PROFILES); + + String errorMessage = "reactivation_exception"; + when(ruleService.createCustomRule(any())).thenThrow(new ReactivationException(errorMessage, null)); + + mockMvc.perform( + post(RULES_ENDPOINT) + .contentType(MediaType.APPLICATION_JSON_VALUE) + .content(gson.toJson(newRequest()))) + .andExpectAll(status().isConflict(), content().json(String.format("{message:'%s'}", errorMessage))); + } + + @Test + public void create_whenMissingBodyField_shouldFailWithBadRequest() throws Exception { + userSession.logIn().addPermission(ADMINISTER_QUALITY_PROFILES); + + RuleCreateRestRequest request = newRequest(null); + + mockMvc.perform( + post(RULES_ENDPOINT) + .contentType(MediaType.APPLICATION_JSON_VALUE) + .content(gson.toJson(request))) + .andExpectAll(status().isBadRequest(), content().json("{message:'Value {} for field name was rejected. Error: must not be null.'}")); + } + + @Test + public void create_whenInvalidParam_shouldFailWithBadRequest() throws Exception { + userSession.logIn().addPermission(ADMINISTER_QUALITY_PROFILES); + + RuleCreateRestRequest request = newRequest("a".repeat(201)); + + mockMvc.perform( + post(RULES_ENDPOINT) + .contentType(MediaType.APPLICATION_JSON_VALUE) + .content(gson.toJson(request))) + .andExpectAll(status().isBadRequest(), + content().json(String.format("{message:'Value %s for field name was rejected. Error: size must be between 0 and 200.'}", + request.name()))); + } + + private static RuleCreateRestRequest newRequest() { + return newRequest("custom rule name"); + } + + private static RuleCreateRestRequest newRequest(@Nullable String name) { + return new RuleCreateRestRequest( + "java:custom_rule", + "java:template_rule", + name, + "some desc", + RuleStatusRestEnum.BETA, + List.of(new Parameter("key1", "desc", "value1", "text")), + CleanCodeAttributeRestEnum.MODULAR, + List.of(new Impact(SoftwareQualityRestEnum.MAINTAINABILITY, ImpactSeverityRestEnum.LOW))); + } + + private RuleInformation generateRuleInformation() { + RuleDto ruleDto = RuleTesting.newCustomRule(RuleTesting.newTemplateRule(RuleKey.parse("java:template_rule"))); + RuleTesting.newRuleParam(ruleDto); + return new RuleInformation(ruleDto, List.of(RuleTesting.newRuleParam(ruleDto), RuleTesting.newRuleParam(ruleDto))); } } diff --git a/server/sonar-webserver-webapi-v2/src/test/resources/logback-test.xml b/server/sonar-webserver-webapi-v2/src/test/resources/logback-test.xml new file mode 100644 index 00000000000..a5353e4a922 --- /dev/null +++ b/server/sonar-webserver-webapi-v2/src/test/resources/logback-test.xml @@ -0,0 +1,17 @@ + + + + + + + %d{HH:mm:ss.SSS} %-5level %logger{36} - %msg%n + + + + + + + + + -- 2.39.5