From 8c74791beac5efe9fc6ffc1556eead40fff5dce3 Mon Sep 17 00:00:00 2001 From: guwirth Date: Fri, 4 Jun 2021 16:31:31 +0200 Subject: [PATCH] Make the loading of other rules more robust. Loading rule descriptions in XML format happens once during server startup. Errors in the XML files can cause the server to fail to start. The loading has been made more robust. In case of an error, individual XML blocks are ignored. This should at least allow the server to be restarted. Afterwards the faulty XML description can be fixed. --- .../cxx/sensors/other/CxxOtherRepository.java | 11 ++- .../sensors/other/CxxOtherRepositoryTest.java | 70 ++++++++++++++++--- 2 files changed, 69 insertions(+), 12 deletions(-) diff --git a/cxx-sensors/src/main/java/org/sonar/cxx/sensors/other/CxxOtherRepository.java b/cxx-sensors/src/main/java/org/sonar/cxx/sensors/other/CxxOtherRepository.java index 5d50d70027..bc172d0c5d 100644 --- a/cxx-sensors/src/main/java/org/sonar/cxx/sensors/other/CxxOtherRepository.java +++ b/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); } } } diff --git a/cxx-sensors/src/test/java/org/sonar/cxx/sensors/other/CxxOtherRepositoryTest.java b/cxx-sensors/src/test/java/org/sonar/cxx/sensors/other/CxxOtherRepositoryTest.java index 22e75fd080..ab95da53f5 100644 --- a/cxx-sensors/src/test/java/org/sonar/cxx/sensors/other/CxxOtherRepositoryTest.java +++ b/cxx-sensors/src/test/java/org/sonar/cxx/sensors/other/CxxOtherRepositoryTest.java @@ -46,11 +46,33 @@ public class CxxOtherRepositoryTest { private final String profile2 = "\n" + "\n" - + " \n" - + " \n" - + " \n" - + " \n" - + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + ""; + + private final String profile3 = "\n" + + "\n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + " \n" + ""; @@ -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"); } }