From 362d39c55912ee01e9bec4b0155bee601111ad91 Mon Sep 17 00:00:00 2001 From: Duarte Meneses Date: Fri, 13 Apr 2018 15:44:26 +0200 Subject: [PATCH] SONAR-10586 External rules can't be assigned to QPs --- .../qualityprofile/QProfileBackuperImpl.java | 10 +++++ .../qualityprofile/ws/ActivateRuleAction.java | 3 +- .../ws/ActivateRulesAction.java | 5 ++- .../ws/DeactivateRulesAction.java | 5 ++- .../qualityprofile/ws/QProfileWsSupport.java | 4 +- .../QProfileBackuperImplTest.java | 24 ++++++++++++ .../ws/ActivateRuleActionTest.java | 32 +++++++++++---- .../ws/ActivateRulesActionTest.java | 2 +- .../ws/DeactivateRuleActionTest.java | 39 +++++++++++++------ .../ws/DeactivateRulesActionTest.java | 2 +- .../ws/QProfileWsSupportTest.java | 15 ++++++- 11 files changed, 114 insertions(+), 27 deletions(-) diff --git a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileBackuperImpl.java b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileBackuperImpl.java index 9e5267d49e5..75defe5ec58 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileBackuperImpl.java +++ b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileBackuperImpl.java @@ -33,6 +33,7 @@ import java.util.Map; import java.util.Objects; import java.util.Set; import java.util.function.Function; +import java.util.stream.Collectors; import javax.annotation.Nullable; import javax.xml.stream.XMLInputFactory; import javax.xml.stream.XMLStreamException; @@ -183,6 +184,15 @@ public class QProfileBackuperImpl implements QProfileBackuper { .stream() .collect(MoreCollectors.uniqueIndex(RuleDefinitionDto::getKey)); + List externalRules = ruleDefinitionsByKey.values().stream() + .filter(RuleDefinitionDto::isExternal) + .collect(Collectors.toList()); + + if (!externalRules.isEmpty()) { + throw new IllegalArgumentException("The quality profile cannot be restored as it contains rules from external rule engines: " + + externalRules.stream().map(r -> r.getKey().toString()).collect(Collectors.joining(", "))); + } + return rules.stream() .map(r -> { RuleDefinitionDto ruleDefinition = ruleDefinitionsByKey.get(r.ruleKey); diff --git a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/ws/ActivateRuleAction.java b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/ws/ActivateRuleAction.java index 6710bd63893..da7614f482f 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/ws/ActivateRuleAction.java +++ b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/ws/ActivateRuleAction.java @@ -114,8 +114,7 @@ public class ActivateRuleAction implements QProfileWsAction { private RuleActivation readActivation(DbSession dbSession, Request request) { RuleKey ruleKey = RuleKey.parse(request.mandatoryParam(PARAM_RULE)); - RuleDefinitionDto ruleDefinition = dbClient.ruleDao().selectDefinitionByKey(dbSession, ruleKey) - .orElseThrow(() -> new IllegalArgumentException(format("Rule '%s' not found", ruleKey))); + RuleDefinitionDto ruleDefinition = wsSupport.getRule(dbSession, ruleKey); boolean reset = Boolean.TRUE.equals(request.paramAsBoolean(PARAM_RESET)); if (reset) { return RuleActivation.createReset(ruleDefinition.getId()); diff --git a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/ws/ActivateRulesAction.java b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/ws/ActivateRulesAction.java index 8818dadff9b..bdc71f6fdb8 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/ws/ActivateRulesAction.java +++ b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/ws/ActivateRulesAction.java @@ -29,6 +29,7 @@ import org.sonar.db.organization.OrganizationDto; import org.sonar.db.qualityprofile.QProfileDto; import org.sonar.server.qualityprofile.BulkChangeResult; import org.sonar.server.qualityprofile.QProfileRules; +import org.sonar.server.rule.index.RuleQuery; import org.sonar.server.rule.ws.RuleQueryFactory; import org.sonar.server.user.UserSession; @@ -93,7 +94,9 @@ public class ActivateRulesAction implements QProfileWsAction { OrganizationDto organization = wsSupport.getOrganization(dbSession, profile); wsSupport.checkCanEdit(dbSession, organization, profile); wsSupport.checkNotBuiltInt(profile); - result = qProfileRules.bulkActivateAndCommit(dbSession, profile, ruleQueryFactory.createRuleQuery(dbSession, request), request.param(PARAM_TARGET_SEVERITY)); + RuleQuery ruleQuery = ruleQueryFactory.createRuleQuery(dbSession, request); + ruleQuery.setIsExternal(false); + result = qProfileRules.bulkActivateAndCommit(dbSession, profile, ruleQuery, request.param(PARAM_TARGET_SEVERITY)); } writeResponse(result, response); diff --git a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/ws/DeactivateRulesAction.java b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/ws/DeactivateRulesAction.java index 1a96919832e..f8ee01e8e49 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/ws/DeactivateRulesAction.java +++ b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/ws/DeactivateRulesAction.java @@ -28,6 +28,7 @@ import org.sonar.db.organization.OrganizationDto; import org.sonar.db.qualityprofile.QProfileDto; import org.sonar.server.qualityprofile.BulkChangeResult; import org.sonar.server.qualityprofile.QProfileRules; +import org.sonar.server.rule.index.RuleQuery; import org.sonar.server.rule.ws.RuleQueryFactory; import org.sonar.server.user.UserSession; @@ -85,7 +86,9 @@ public class DeactivateRulesAction implements QProfileWsAction { QProfileDto profile = wsSupport.getProfile(dbSession, QProfileReference.fromKey(qualityProfileKey)); OrganizationDto organization = wsSupport.getOrganization(dbSession, profile); wsSupport.checkCanEdit(dbSession, organization, profile); - result = ruleActivator.bulkDeactivateAndCommit(dbSession, profile, ruleQueryFactory.createRuleQuery(dbSession, request)); + RuleQuery ruleQuery = ruleQueryFactory.createRuleQuery(dbSession, request); + ruleQuery.setIsExternal(false); + result = ruleActivator.bulkDeactivateAndCommit(dbSession, profile, ruleQuery); } writeResponse(result, response); } diff --git a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/ws/QProfileWsSupport.java b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/ws/QProfileWsSupport.java index e1cabd8e22c..a75ce6198cc 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/ws/QProfileWsSupport.java +++ b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/ws/QProfileWsSupport.java @@ -85,7 +85,9 @@ public class QProfileWsSupport { public RuleDefinitionDto getRule(DbSession dbSession, RuleKey ruleKey) { Optional ruleDefinitionDto = dbClient.ruleDao().selectDefinitionByKey(dbSession, ruleKey); - return checkFoundWithOptional(ruleDefinitionDto, "Rule with key '%s' not found", ruleKey); + RuleDefinitionDto rule = checkFoundWithOptional(ruleDefinitionDto, "Rule with key '%s' not found", ruleKey); + checkRequest(!rule.isExternal(), "Operation forbidden for rule '%s' imported from an external rule engine.", ruleKey); + return rule; } /** diff --git a/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/QProfileBackuperImplTest.java b/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/QProfileBackuperImplTest.java index 9dacf34b11b..deea8443d0e 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/QProfileBackuperImplTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/QProfileBackuperImplTest.java @@ -220,6 +220,30 @@ public class QProfileBackuperImplTest { } } + @Test + public void fail_to_restore_external_rule() throws Exception { + db.rules().insert(RuleKey.of("sonarjs", "s001"), r -> r.setIsExternal(true)).getId(); + OrganizationDto organization = db.organizations().insert(); + Reader backup = new StringReader("" + + "foo" + + "js" + + "" + + "" + + "sonarjs" + + "s001" + + "BLOCKER" + + "" + + "barbaz" + + "" + + "" + + "" + + ""); + + expectedException.expect(IllegalArgumentException.class); + expectedException.expectMessage("The quality profile cannot be restored as it contains rules from external rule engines: sonarjs:s001"); + underTest.restore(db.getSession(), backup, organization, null); + } + private RuleDefinitionDto createRule() { return db.rules().insert(); } diff --git a/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/ActivateRuleActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/ActivateRuleActionTest.java index 9633863955d..22c636af716 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/ActivateRuleActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/ActivateRuleActionTest.java @@ -26,6 +26,8 @@ import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; import org.mockito.ArgumentCaptor; +import org.mockito.Captor; +import org.mockito.MockitoAnnotations; import org.sonar.api.rule.RuleKey; import org.sonar.api.rule.Severity; import org.sonar.api.server.ws.WebService; @@ -67,6 +69,8 @@ public class ActivateRuleActionTest { public UserSessionRule userSession = UserSessionRule.standalone(); @Rule public ExpectedException expectedException = ExpectedException.none(); + @Captor + public ArgumentCaptor> ruleActivationCaptor; private DbClient dbClient = db.getDbClient(); private QProfileRules qProfileRules = mock(QProfileRules.class); @@ -79,6 +83,7 @@ public class ActivateRuleActionTest { @Before public void before() { + MockitoAnnotations.initMocks(this); defaultOrganization = db.getDefaultOrganization(); organization = db.organizations().insert(); } @@ -103,7 +108,7 @@ public class ActivateRuleActionTest { .setParam(PARAM_KEY, randomAlphanumeric(UUID_SIZE)); expectedException.expect(UnauthorizedException.class); - + request.execute(); } @@ -113,7 +118,7 @@ public class ActivateRuleActionTest { QProfileDto qualityProfile = db.qualityProfiles().insert(organization); TestRequest request = ws.newRequest() .setMethod("POST") - .setParam(PARAM_RULE, RuleTesting.newRuleDto().getKey().toString()) + .setParam(PARAM_RULE, RuleTesting.newRule().getKey().toString()) .setParam(PARAM_KEY, qualityProfile.getKee()); expectedException.expect(ForbiddenException.class); @@ -128,7 +133,7 @@ public class ActivateRuleActionTest { QProfileDto qualityProfile = db.qualityProfiles().insert(defaultOrganization, profile -> profile.setIsBuiltIn(true).setName("Xoo profile").setLanguage("xoo")); TestRequest request = ws.newRequest() .setMethod("POST") - .setParam(PARAM_RULE, RuleTesting.newRuleDto().getKey().toString()) + .setParam(PARAM_RULE, RuleTesting.newRule().getKey().toString()) .setParam(PARAM_KEY, qualityProfile.getKee()); expectedException.expect(BadRequestException.class); @@ -137,6 +142,23 @@ public class ActivateRuleActionTest { request.execute(); } + @Test + public void fail_activate_external_rule() { + userSession.logIn(db.users().insertUser()).addPermission(OrganizationPermission.ADMINISTER_QUALITY_PROFILES, defaultOrganization); + QProfileDto qualityProfile = db.qualityProfiles().insert(defaultOrganization); + RuleDefinitionDto rule = db.rules().insert(r -> r.setIsExternal(true)); + + TestRequest request = ws.newRequest() + .setMethod("POST") + .setParam(PARAM_RULE, rule.getKey().toString()) + .setParam(PARAM_KEY, qualityProfile.getKee()); + + expectedException.expect(BadRequestException.class); + expectedException.expectMessage(String.format("Operation forbidden for rule '%s' imported from an external rule engine.", rule.getKey())); + + request.execute(); + } + @Test public void activate_rule_in_default_organization() { userSession.logIn().addPermission(OrganizationPermission.ADMINISTER_QUALITY_PROFILES, defaultOrganization); @@ -153,8 +175,6 @@ public class ActivateRuleActionTest { TestResponse response = request.execute(); assertThat(response.getStatus()).isEqualTo(HttpURLConnection.HTTP_NO_CONTENT); - Class> collectionClass = (Class>) (Class) Collection.class; - ArgumentCaptor> ruleActivationCaptor = ArgumentCaptor.forClass(collectionClass); verify(qProfileRules).activateAndCommit(any(DbSession.class), any(QProfileDto.class), ruleActivationCaptor.capture()); Collection activations = ruleActivationCaptor.getValue(); @@ -183,8 +203,6 @@ public class ActivateRuleActionTest { TestResponse response = request.execute(); assertThat(response.getStatus()).isEqualTo(HttpURLConnection.HTTP_NO_CONTENT); - Class> collectionClass = (Class>) (Class) Collection.class; - ArgumentCaptor> ruleActivationCaptor = ArgumentCaptor.forClass(collectionClass); verify(qProfileRules).activateAndCommit(any(DbSession.class), any(QProfileDto.class), ruleActivationCaptor.capture()); Collection activations = ruleActivationCaptor.getValue(); diff --git a/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/ActivateRulesActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/ActivateRulesActionTest.java index 909ea57ade3..5f064e95afc 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/ActivateRulesActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/ActivateRulesActionTest.java @@ -62,7 +62,7 @@ public class ActivateRulesActionTest { private DbClient dbClient = db.getDbClient(); private QProfileWsSupport wsSupport = new QProfileWsSupport(dbClient, userSession, TestDefaultOrganizationProvider.from(db)); - private RuleQueryFactory ruleQueryFactory = mock(RuleQueryFactory.class); + private RuleQueryFactory ruleQueryFactory = mock(RuleQueryFactory.class, Mockito.RETURNS_MOCKS); private QProfileRules qProfileRules = mock(QProfileRules.class, Mockito.RETURNS_DEEP_STUBS); private WsActionTester ws = new WsActionTester(new ActivateRulesAction(ruleQueryFactory, userSession, qProfileRules, wsSupport, dbClient)); diff --git a/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/DeactivateRuleActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/DeactivateRuleActionTest.java index 21902b4d72e..c8c2a8821a2 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/DeactivateRuleActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/DeactivateRuleActionTest.java @@ -26,6 +26,8 @@ import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; import org.mockito.ArgumentCaptor; +import org.mockito.Captor; +import org.mockito.MockitoAnnotations; import org.sonar.api.rule.RuleKey; import org.sonar.api.server.ws.WebService; import org.sonar.db.DbClient; @@ -65,6 +67,8 @@ public class DeactivateRuleActionTest { public UserSessionRule userSession = UserSessionRule.standalone(); @Rule public ExpectedException expectedException = ExpectedException.none(); + @Captor + public ArgumentCaptor> ruleIdsCaptor; private DbClient dbClient = db.getDbClient(); private QProfileRules qProfileRules = mock(QProfileRules.class); @@ -76,6 +80,7 @@ public class DeactivateRuleActionTest { @Before public void before() { + MockitoAnnotations.initMocks(this); defaultOrganization = db.getDefaultOrganization(); organization = db.organizations().insert(); } @@ -105,10 +110,9 @@ public class DeactivateRuleActionTest { TestResponse response = request.execute(); assertThat(response.getStatus()).isEqualTo(HttpURLConnection.HTTP_NO_CONTENT); - ArgumentCaptor> ruleIdCaptor = ruleIdCollectionCaptor(); ArgumentCaptor qProfileDtoCaptor = ArgumentCaptor.forClass(QProfileDto.class); - verify(qProfileRules).deactivateAndCommit(any(DbSession.class), qProfileDtoCaptor.capture(), ruleIdCaptor.capture()); - assertThat(ruleIdCaptor.getValue()).containsExactly(rule.getId()); + verify(qProfileRules).deactivateAndCommit(any(DbSession.class), qProfileDtoCaptor.capture(), ruleIdsCaptor.capture()); + assertThat(ruleIdsCaptor.getValue()).containsExactly(rule.getId()); assertThat(qProfileDtoCaptor.getValue().getKee()).isEqualTo(qualityProfile.getKee()); } @@ -126,10 +130,9 @@ public class DeactivateRuleActionTest { TestResponse response = request.execute(); assertThat(response.getStatus()).isEqualTo(HttpURLConnection.HTTP_NO_CONTENT); - ArgumentCaptor> ruleIdCaptor = ruleIdCollectionCaptor(); ArgumentCaptor qProfileDtoCaptor = ArgumentCaptor.forClass(QProfileDto.class); - verify(qProfileRules).deactivateAndCommit(any(DbSession.class), qProfileDtoCaptor.capture(), ruleIdCaptor.capture()); - assertThat(ruleIdCaptor.getValue()).containsExactly(rule.getId()); + verify(qProfileRules).deactivateAndCommit(any(DbSession.class), qProfileDtoCaptor.capture(), ruleIdsCaptor.capture()); + assertThat(ruleIdsCaptor.getValue()).containsExactly(rule.getId()); assertThat(qProfileDtoCaptor.getValue().getKee()).isEqualTo(qualityProfile.getKee()); } @@ -156,7 +159,7 @@ public class DeactivateRuleActionTest { public void fail_if_not_logged_in() { TestRequest request = ws.newRequest() .setMethod("POST") - .setParam(PARAM_RULE, RuleTesting.newRuleDto().getKey().toString()) + .setParam(PARAM_RULE, RuleTesting.newRule().getKey().toString()) .setParam(PARAM_KEY, randomAlphanumeric(UUID_SIZE)); expectedException.expect(UnauthorizedException.class); @@ -179,6 +182,23 @@ public class DeactivateRuleActionTest { request.execute(); } + @Test + public void fail_activate_external_rule() { + userSession.logIn(db.users().insertUser()).addPermission(OrganizationPermission.ADMINISTER_QUALITY_PROFILES, defaultOrganization); + QProfileDto qualityProfile = db.qualityProfiles().insert(defaultOrganization); + RuleDefinitionDto rule = db.rules().insert(r -> r.setIsExternal(true)); + + TestRequest request = ws.newRequest() + .setMethod("POST") + .setParam(PARAM_RULE, rule.getKey().toString()) + .setParam(PARAM_KEY, qualityProfile.getKee()); + + expectedException.expect(BadRequestException.class); + expectedException.expectMessage(String.format("Operation forbidden for rule '%s' imported from an external rule engine.", rule.getKey())); + + request.execute(); + } + @Test public void fail_deactivate_if_built_in_profile() { RuleDefinitionDto rule = db.rules().insert(); @@ -194,9 +214,4 @@ public class DeactivateRuleActionTest { request.execute(); } - - private static ArgumentCaptor> ruleIdCollectionCaptor() { - Class> collectionClass = (Class>) (Class) Collection.class; - return ArgumentCaptor.forClass(collectionClass); - } } diff --git a/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/DeactivateRulesActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/DeactivateRulesActionTest.java index c1a570d8d28..b6b4dc7de29 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/DeactivateRulesActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/DeactivateRulesActionTest.java @@ -63,7 +63,7 @@ public class DeactivateRulesActionTest { private DbClient dbClient = db.getDbClient(); private QProfileRules qProfileRules = mock(QProfileRules.class, RETURNS_DEEP_STUBS); private QProfileWsSupport wsSupport = new QProfileWsSupport(dbClient, userSession, TestDefaultOrganizationProvider.from(db)); - private RuleQueryFactory ruleQueryFactory = mock(RuleQueryFactory.class); + private RuleQueryFactory ruleQueryFactory = mock(RuleQueryFactory.class, RETURNS_DEEP_STUBS); private DeactivateRulesAction underTest = new DeactivateRulesAction(ruleQueryFactory, userSession, qProfileRules, wsSupport, dbClient); private WsActionTester ws = new WsActionTester(underTest); private OrganizationDto defaultOrganization; diff --git a/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/QProfileWsSupportTest.java b/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/QProfileWsSupportTest.java index b9e0fd848c3..9447a25660f 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/QProfileWsSupportTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/QProfileWsSupportTest.java @@ -26,6 +26,8 @@ import org.sonar.db.DbTester; import org.sonar.db.organization.OrganizationDto; import org.sonar.db.qualityprofile.QProfileDto; import org.sonar.db.qualityprofile.QualityProfileTesting; +import org.sonar.db.rule.RuleDefinitionDto; +import org.sonar.server.exceptions.BadRequestException; import org.sonar.server.exceptions.NotFoundException; import org.sonar.server.organization.DefaultOrganizationProvider; import org.sonar.server.organization.TestDefaultOrganizationProvider; @@ -90,6 +92,16 @@ public class QProfileWsSupportTest { underTest.getProfile(db.getSession(), QProfileReference.fromName(null, "java", "missing")); } + @Test + public void getRule_throws_BadRequest_if_rule_is_external() { + RuleDefinitionDto rule = db.rules().insert(r -> r.setIsExternal(true)); + + expectedException.expect(BadRequestException.class); + expectedException.expectMessage(format("Operation forbidden for rule '%s' imported from an external rule engine.", rule.getKey())); + + underTest.getRule(db.getSession(), rule.getKey()); + } + @Test public void getProfile_throws_NotFoundException_if_specified_name_does_not_exist_on_specified_organization() { OrganizationDto org1 = db.organizations().insert(); @@ -98,7 +110,8 @@ public class QProfileWsSupportTest { OrganizationDto org2 = db.organizations().insert(); expectedException.expect(NotFoundException.class); - expectedException.expectMessage(format("Quality Profile for language '%s' and name '%s' does not exist in organization '%s'", profile.getLanguage(), profile.getName(), org2.getKey())); + expectedException + .expectMessage(format("Quality Profile for language '%s' and name '%s' does not exist in organization '%s'", profile.getLanguage(), profile.getName(), org2.getKey())); underTest.getProfile(db.getSession(), QProfileReference.fromName(org2.getKey(), profile.getLanguage(), profile.getName())); } -- 2.39.5