]> source.dussan.org Git - sonarqube.git/commitdiff
SONAR-10586 External rules can't be assigned to QPs
authorDuarte Meneses <duarte.meneses@sonarsource.com>
Fri, 13 Apr 2018 13:44:26 +0000 (15:44 +0200)
committerSonarTech <sonartech@sonarsource.com>
Thu, 26 Apr 2018 18:20:50 +0000 (20:20 +0200)
server/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileBackuperImpl.java
server/sonar-server/src/main/java/org/sonar/server/qualityprofile/ws/ActivateRuleAction.java
server/sonar-server/src/main/java/org/sonar/server/qualityprofile/ws/ActivateRulesAction.java
server/sonar-server/src/main/java/org/sonar/server/qualityprofile/ws/DeactivateRulesAction.java
server/sonar-server/src/main/java/org/sonar/server/qualityprofile/ws/QProfileWsSupport.java
server/sonar-server/src/test/java/org/sonar/server/qualityprofile/QProfileBackuperImplTest.java
server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/ActivateRuleActionTest.java
server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/ActivateRulesActionTest.java
server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/DeactivateRuleActionTest.java
server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/DeactivateRulesActionTest.java
server/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/QProfileWsSupportTest.java

index 9e5267d49e5406eec3787643f27048c862032441..75defe5ec5856883f7c69232bb389526cd3219df 100644 (file)
@@ -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<RuleDefinitionDto> 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);
index 6710bd638933758095f0e4af2aad05295ca8bb5c..da7614f482f9af4e183a146b9ef549b80d40fa25 100644 (file)
@@ -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());
index 8818dadff9b720cb5a8ad05b6dce3f0c17b16356..bdc71f6fdb89c7b0e14f01bea0135ae7a24d7699 100644 (file)
@@ -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);
index 1a96919832eb4e16cbf645c7c891b288e3da2c84..f8ee01e8e49bbe7ea5376d9bcc7981519345fe87 100644 (file)
@@ -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);
   }
index e1cabd8e22c5fe8004c9fbf19bb138ad89551b04..a75ce6198cc7524b6188c342e20be9656275b2b7 100644 (file)
@@ -85,7 +85,9 @@ public class QProfileWsSupport {
 
   public RuleDefinitionDto getRule(DbSession dbSession, RuleKey ruleKey) {
     Optional<RuleDefinitionDto> 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;
   }
 
   /**
index 9dacf34b11bd88f938eea00083f515ec3b14f026..deea8443d0e1bf919056fe2507895f9e9c25ce6c 100644 (file)
@@ -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("<?xml version='1.0' encoding='UTF-8'?>" +
+      "<profile><name>foo</name>" +
+      "<language>js</language>" +
+      "<rules>" +
+      "<rule>" +
+      "<repositoryKey>sonarjs</repositoryKey>" +
+      "<key>s001</key>" +
+      "<priority>BLOCKER</priority>" +
+      "<parameters>" +
+      "<parameter><key>bar</key><value>baz</value></parameter>" +
+      "</parameters>" +
+      "</rule>" +
+      "</rules>" +
+      "</profile>");
+
+    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();
   }
index 9633863955d5d62708ecbf75ad4e967e6d5aaa70..22c636af71681ff46a3c346c72c5d106cd20e3eb 100644 (file)
@@ -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<Collection<RuleActivation>> 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<Collection<RuleActivation>> collectionClass = (Class<Collection<RuleActivation>>) (Class) Collection.class;
-    ArgumentCaptor<Collection<RuleActivation>> ruleActivationCaptor = ArgumentCaptor.forClass(collectionClass);
     verify(qProfileRules).activateAndCommit(any(DbSession.class), any(QProfileDto.class), ruleActivationCaptor.capture());
 
     Collection<RuleActivation> activations = ruleActivationCaptor.getValue();
@@ -183,8 +203,6 @@ public class ActivateRuleActionTest {
     TestResponse response = request.execute();
 
     assertThat(response.getStatus()).isEqualTo(HttpURLConnection.HTTP_NO_CONTENT);
-    Class<Collection<RuleActivation>> collectionClass = (Class<Collection<RuleActivation>>) (Class) Collection.class;
-    ArgumentCaptor<Collection<RuleActivation>> ruleActivationCaptor = ArgumentCaptor.forClass(collectionClass);
     verify(qProfileRules).activateAndCommit(any(DbSession.class), any(QProfileDto.class), ruleActivationCaptor.capture());
 
     Collection<RuleActivation> activations = ruleActivationCaptor.getValue();
index 909ea57ade3f28d0fb181c6c663b7ea8202aab8e..5f064e95afcdf6c814545ef6bd6e9bef2959e2c3 100644 (file)
@@ -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));
index 21902b4d72ef32ccf402b9e30961b9d2b5684ca9..c8c2a8821a2d451f39e32c263be3976982aef84d 100644 (file)
@@ -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<Collection<Integer>> 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<Collection<Integer>> ruleIdCaptor = ruleIdCollectionCaptor();
     ArgumentCaptor<QProfileDto> 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<Collection<Integer>> ruleIdCaptor = ruleIdCollectionCaptor();
     ArgumentCaptor<QProfileDto> 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<Collection<Integer>> ruleIdCollectionCaptor() {
-    Class<Collection<Integer>> collectionClass = (Class<Collection<Integer>>) (Class) Collection.class;
-    return ArgumentCaptor.forClass(collectionClass);
-  }
 }
index c1a570d8d286410e5bdd8dddbec485d3676eb472..b6b4dc7de29ce65f2fc2240b2f9b69cde97dc999 100644 (file)
@@ -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;
index b9e0fd848c399576b658c871d76faef0e66a2310..9447a25660fb18560e0a377af59535a923754e0e 100644 (file)
@@ -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()));
   }