Merge pull request #2179 from guwirth/other-repository

Make the loading of other rules more robust.

Author: guwirth

cxx-sensors/src/main/java/org/sonar/cxx/sensors/other/CxxOtherRepository.java

@@ -54,14 +54,23 @@ public CxxOtherRepository(Configuration config, RulesDefinitionXmlLoader xmlRule
   public void define(Context context) {
     var repository = context.createRepository(KEY, "cxx")
       .setName(NAME);
+    var validate = context.createRepository("Validate", "cxx")
+      .setName("validate");
 
     xmlRuleLoader.load(repository, getClass().getResourceAsStream("/external-rule.xml"), StandardCharsets.UTF_8.name());
     for (var ruleDefs : this.config.getStringArray(CxxOtherSensor.RULES_KEY)) {
       if (ruleDefs != null && !ruleDefs.trim().isEmpty()) {
         try {
+          // read rules first into dummy repository to check if there are errors
+          xmlRuleLoader.load(validate, new StringReader(ruleDefs));
+          // in case of no errors read again into real repository
           xmlRuleLoader.load(repository, new StringReader(ruleDefs));
         } catch (IllegalStateException e) {
-          LOG.error("Cannot load Rules Definions '{}'", e.getMessage());
+          // In case of an error, ignore the whole XML block. The loading happens during the server start,
+          // errors are critical and can cause that the server cannot be started anymore.
+          var xml = ruleDefs.substring(0, Math.min(ruleDefs.length(), 120))
+            .replaceAll("\\R", "").replaceAll(">[ ]+<", "><");
+          LOG.error("Cannot load rule definions for 'sonar.cxx.other.rules', '{}', XML '{}...'", e.getMessage(), xml);
         }
       }
     }

cxx-sensors/src/test/java/org/sonar/cxx/sensors/other/CxxOtherRepositoryTest.java

@@ -46,11 +46,33 @@ public class CxxOtherRepositoryTest {
 
   private final String profile2 = "<?xml version=\"1.0\" encoding=\"ASCII\"?>\n"
                                     + "<rules>\n"
-                                    + "    <rule key=\"key\">\n"
-                                    + "        <name><![CDATA[name]]></name>\n"
-                                    + "        <configKey><![CDATA[configKey]]></configKey>\n"
-                                    + "        <category name=\"category\" />\n"
-                                    + "        <description><![CDATA[description]]></description>\n"
+                                    + "    <rule key=\"key1\">\n"
+                                    + "        <name><![CDATA[name1]]></name>\n"
+                                    + "        <configKey><![CDATA[configKey1]]></configKey>\n"
+                                    + "        <category name=\"category1\" />\n"
+                                    + "        <description><![CDATA[description1]]></description>\n"
+                                    + "    </rule>\n"
+                                    + "    <rule key=\"key2\">\n"
+                                    + "        <name><![CDATA[name2]]></name>\n"
+                                    + "        <configKey><![CDATA[configKey2]]></configKey>\n"
+                                    + "        <category name=\"category2\" />\n"
+                                    + "        <description><![CDATA[description2]]></description>\n"
+                                    + "    </rule>\n"
+                                    + "</rules>";
+
+  private final String profile3 = "<?xml version=\"1.0\" encoding=\"ASCII\"?>\n"
+                                    + "<rules>\n"
+                                    + "    <rule key=\"key1\">\n"
+                                    + "        <name><![CDATA[name1]]></name>\n"
+                                    + "        <configKey><![CDATA[configKey1]]></configKey>\n"
+                                    + "        <category name=\"category1\" />\n"
+                                    + "        <description><![CDATA[description1]]></description>\n"
+                                    + "    </rule>\n"
+                                    + "    <rule key=\"key3\">\n"
+                                    + "        <name><![CDATA[name3]]></name>\n"
+                                    + "        <configKey><![CDATA[configKey3]]></configKey>\n"
+                                    + "        <category name=\"category3\" />\n"
+                                    + "        <description><![CDATA[description3]]></description>\n"
                                     + "    </rule>\n"
                                     + "</rules>";
 
@@ -99,15 +121,41 @@ public void verifyRuleValuesTest() {
     def.define(context);
 
     RulesDefinition.Repository repo = context.repository(CxxOtherRepository.KEY);
-    var rule = repo.rule("key");
+    var rule = repo.rule("key1");
     assertThat(rule).isNotNull();
 
-    // from rule.xml
-    assertThat(rule.key()).isEqualTo("key");
-    assertThat(rule.name()).isEqualTo("name");
-    assertThat(rule.internalKey()).isEqualTo("configKey");
-    assertThat(rule.htmlDescription()).isEqualTo("description");
+    assertThat(rule.key()).isEqualTo("key1");
+    assertThat(rule.name()).isEqualTo("name1");
+    assertThat(rule.internalKey()).isEqualTo("configKey1");
+    assertThat(rule.htmlDescription()).isEqualTo("description1");
+  }
+
+  @Test
+  public void verifyRulesWithSameKey() {
+    settings.setProperty(CxxOtherSensor.RULES_KEY, profile2 + "," + profile3);
+    var def = new CxxOtherRepository(settings.asConfig(), new RulesDefinitionXmlLoader());
+
+    var context = new RulesDefinition.Context();
+    def.define(context);
+
+    RulesDefinition.Repository repo = context.repository(CxxOtherRepository.KEY);
+    assertThat(repo.rules()).hasSize(3);
+
+    var rule = repo.rule("key1");
+    assertThat(rule).isNotNull();
+
+    assertThat(rule.key()).isEqualTo("key1");
+    assertThat(rule.name()).isEqualTo("name1");
+    assertThat(rule.internalKey()).isEqualTo("configKey1");
+    assertThat(rule.htmlDescription()).isEqualTo("description1");
+
+    rule = repo.rule("key2");
+    assertThat(rule).isNotNull();
 
+    assertThat(rule.key()).isEqualTo("key2");
+    assertThat(rule.name()).isEqualTo("name2");
+    assertThat(rule.internalKey()).isEqualTo("configKey2");
+    assertThat(rule.htmlDescription()).isEqualTo("description2");
   }
 
 }