]> source.dussan.org Git - sonarqube.git/commitdiff
SONAR-23466 Partially revert changes made by SONAR-22914
authorJulien HENRY <julien.henry@sonarsource.com>
Wed, 23 Oct 2024 10:30:58 +0000 (12:30 +0200)
committersonartech <sonartech@sonarsource.com>
Thu, 24 Oct 2024 20:02:44 +0000 (20:02 +0000)
server/sonar-webserver-core/src/it/java/org/sonar/server/rule/registration/RulesRegistrantIT.java
server/sonar-webserver-core/src/main/java/org/sonar/server/rule/RuleDefinitionsLoader.java
server/sonar-webserver-core/src/main/java/org/sonar/server/rule/registration/RulesRegistrant.java
server/sonar-webserver-core/src/main/java/org/sonar/server/rule/registration/RulesRegistrationContext.java
server/sonar-webserver-core/src/test/java/org/sonar/server/rule/RuleDefinitionsLoaderTest.java
server/sonar-webserver/src/main/java/org/sonar/server/platform/platformlevel/PlatformLevelStartup.java

index 5dec9321df785238c26748ece391798cb2931705..06b55920c390ec4bfad09c2a692dab0de90a925a 100644 (file)
@@ -165,7 +165,6 @@ class RulesRegistrantIT {
   private final StartupRuleUpdater startupRuleUpdater = new StartupRuleUpdater(dbClient, system, uuidFactory, resolver);
   private final NewRuleCreator newRuleCreator = new NewRuleCreator(resolver, uuidFactory, system);
   private final QualityProfileChangesUpdater qualityProfileChangesUpdater = mock();
-  private final DetectPluginChange detectPluginChange = mock();
 
   @BeforeEach
   void before() {
@@ -1169,7 +1168,6 @@ class RulesRegistrantIT {
     when(pluginRepository.getPluginKey(builtInRepoV1)).thenReturn(null);
     when(pluginRepository.getPluginKey(pluginRepo)).thenReturn(FAKE_PLUGIN_KEY);
     RuleDefinitionsLoader loader1 = new RuleDefinitionsLoader(pluginRepository, new RulesDefinition[] {builtInRepoV1, pluginRepo});
-    when(detectPluginChange.anyPluginChanged()).thenReturn(true);
     execute(loader1);
 
     RuleDto builtInRulev1 = dbClient.ruleDao().selectOrFailByKey(db.getSession(), RuleKey.of("sca", "rule1"));
@@ -1178,7 +1176,6 @@ class RulesRegistrantIT {
     RulesDefinition builtInRepoV2 = context -> createRule(context, "builtin", "sca", "rule1", rule -> rule.setName("Name after"));
     when(pluginRepository.getPluginKey(builtInRepoV2)).thenReturn(null);
     RuleDefinitionsLoader loader2 = new RuleDefinitionsLoader(pluginRepository, new RulesDefinition[] {builtInRepoV2, pluginRepo});
-    when(detectPluginChange.anyPluginChanged()).thenReturn(false);
     execute(loader2);
 
     RuleDto builtInRulev2 = dbClient.ruleDao().selectOrFailByKey(db.getSession(), RuleKey.of("sca", "rule1"));
@@ -1320,7 +1317,6 @@ class RulesRegistrantIT {
     ServerPluginRepository pluginRepository = mock(ServerPluginRepository.class);
     when(pluginRepository.getPluginKey(any(RulesDefinition.class))).thenReturn(FAKE_PLUGIN_KEY);
     RuleDefinitionsLoader loader = new RuleDefinitionsLoader(pluginRepository, defs);
-    when(detectPluginChange.anyPluginChanged()).thenReturn(true);
     execute(loader);
   }
 
@@ -1328,7 +1324,7 @@ class RulesRegistrantIT {
     reset(webServerRuleFinder);
 
     RulesRegistrant task = new RulesRegistrant(loader, qProfileRules, dbClient, ruleIndexer, activeRuleIndexer, system, webServerRuleFinder, metadataIndex,
-      rulesKeyVerifier, startupRuleUpdater, newRuleCreator, qualityProfileChangesUpdater, sonarQubeVersion, detectPluginChange, activeRulesImpactInitializer);
+      rulesKeyVerifier, startupRuleUpdater, newRuleCreator, qualityProfileChangesUpdater, sonarQubeVersion, activeRulesImpactInitializer);
     task.start();
     // Execute a commit to refresh session state as the task is using its own session
     db.getSession().commit();
index 67c6d2ecedb94c9307ba6411d8065ed0cf4d7bfb..e0a99f56064813700fccadaf779c1b30354f1dbf 100644 (file)
@@ -19,8 +19,6 @@
  */
 package org.sonar.server.rule;
 
-import java.util.Objects;
-import java.util.function.Predicate;
 import org.sonar.api.impl.server.RulesDefinitionContext;
 import org.sonar.api.server.rule.RulesDefinition;
 import org.sonar.server.plugins.ServerPluginRepository;
@@ -49,22 +47,12 @@ public class RuleDefinitionsLoader {
     this(serverPluginRepository, new RulesDefinition[0]);
   }
 
-  public RulesDefinition.Context loadFromPlugins() {
-    return load(Predicate.not(Objects::isNull));
-  }
-
-  public RulesDefinition.Context loadBuiltIn() {
-    return load(Objects::isNull);
-  }
-
-  private RulesDefinition.Context load(Predicate<String> pluginKeyPredicate) {
+  public RulesDefinition.Context load() {
     RulesDefinition.Context context = new RulesDefinitionContext();
     for (RulesDefinition rulesDefinition : rulesDefinitions) {
       var pluginKey = serverPluginRepository.getPluginKey(rulesDefinition);
-      if (pluginKeyPredicate.test(pluginKey)) {
-        context.setCurrentPluginKey(pluginKey);
-        rulesDefinition.define(context);
-      }
+      context.setCurrentPluginKey(pluginKey);
+      rulesDefinition.define(context);
     }
     context.setCurrentPluginKey(null);
     return context;
index bafd97fda7712189a260a9a0c9833d954bb463ce..30da7595fbfacb4db3da6457c65281cb7ade3dd0 100644 (file)
@@ -48,7 +48,6 @@ import org.sonar.db.rule.RuleDescriptionSectionDto;
 import org.sonar.db.rule.RuleDto;
 import org.sonar.db.rule.RuleRepositoryDto;
 import org.sonar.server.es.metadata.MetadataIndex;
-import org.sonar.server.plugins.DetectPluginChange;
 import org.sonar.server.qualityprofile.ActiveRuleChange;
 import org.sonar.server.qualityprofile.QProfileRules;
 import org.sonar.server.qualityprofile.index.ActiveRuleIndexer;
@@ -82,14 +81,13 @@ public class RulesRegistrant implements Startable {
   private final NewRuleCreator newRuleCreator;
   private final QualityProfileChangesUpdater qualityProfileChangesUpdater;
   private final SonarQubeVersion sonarQubeVersion;
-  private final DetectPluginChange detectPluginChange;
   private final ActiveRulesImpactInitializer activeRulesImpactInitializer;
 
   public RulesRegistrant(RuleDefinitionsLoader defLoader, QProfileRules qProfileRules, DbClient dbClient, RuleIndexer ruleIndexer,
     ActiveRuleIndexer activeRuleIndexer, System2 system2, WebServerRuleFinder webServerRuleFinder,
     MetadataIndex metadataIndex, RulesKeyVerifier rulesKeyVerifier, StartupRuleUpdater startupRuleUpdater,
     NewRuleCreator newRuleCreator, QualityProfileChangesUpdater qualityProfileChangesUpdater, SonarQubeVersion sonarQubeVersion,
-    DetectPluginChange detectPluginChange, ActiveRulesImpactInitializer activeRulesImpactInitializer) {
+    ActiveRulesImpactInitializer activeRulesImpactInitializer) {
     this.defLoader = defLoader;
     this.qProfileRules = qProfileRules;
     this.dbClient = dbClient;
@@ -103,7 +101,6 @@ public class RulesRegistrant implements Startable {
     this.newRuleCreator = newRuleCreator;
     this.qualityProfileChangesUpdater = qualityProfileChangesUpdater;
     this.sonarQubeVersion = sonarQubeVersion;
-    this.detectPluginChange = detectPluginChange;
     this.activeRulesImpactInitializer = activeRulesImpactInitializer;
   }
 
@@ -111,24 +108,15 @@ public class RulesRegistrant implements Startable {
   public void start() {
     Profiler profiler = Profiler.create(LOG).startInfo("Register rules");
     try (DbSession dbSession = dbClient.openSession(true)) {
-      var anyPluginChanged = detectPluginChange.anyPluginChanged();
-      RulesRegistrationContext rulesRegistrationContext = RulesRegistrationContext.create(dbClient, dbSession, !anyPluginChanged);
+      List<RulesDefinition.Repository> repositories = defLoader.load().repositories();
+      RulesRegistrationContext rulesRegistrationContext = RulesRegistrationContext.create(dbClient, dbSession);
+      rulesKeyVerifier.verifyRuleKeyConsistency(repositories, rulesRegistrationContext);
 
-      List<RulesDefinition.Repository> rulesRepositories = new ArrayList<>(defLoader.loadBuiltIn().repositories());
-      if (anyPluginChanged) {
-        LOG.info("Some plugins have changed, triggering loading of rules from plugins");
-        rulesRepositories.addAll(defLoader.loadFromPlugins().repositories());
-      }
-      rulesKeyVerifier.verifyRuleKeyConsistency(rulesRepositories, rulesRegistrationContext);
-
-      persistRepositories(dbSession, rulesRepositories, anyPluginChanged);
-
-      for (RulesDefinition.Repository repoDef : rulesRepositories) {
+      for (RulesDefinition.Repository repoDef : repositories) {
         if (repoDef.language() == null) {
           throw new IllegalStateException("Language is mandatory for repository " + repoDef.key());
         }
         Set<PluginRuleUpdate> pluginRuleUpdates = registerRules(rulesRegistrationContext, repoDef.rules(), dbSession);
-
         if (!repoDef.isExternal()) {
           // External rules are not part of quality profiles
           activeRulesImpactInitializer.createImpactsOnActiveRules(rulesRegistrationContext, repoDef, dbSession);
@@ -138,9 +126,10 @@ public class RulesRegistrant implements Startable {
       }
       activeRulesImpactInitializer.markInitialPopulationDone();
       processRemainingDbRules(rulesRegistrationContext, dbSession);
-      List<ActiveRuleChange> changes = anyPluginChanged ? removeActiveRulesOnStillExistingRepositories(dbSession, rulesRegistrationContext, rulesRepositories) : List.of();
+      List<ActiveRuleChange> changes = removeActiveRulesOnStillExistingRepositories(dbSession, rulesRegistrationContext, repositories);
       dbSession.commit();
 
+      persistRepositories(dbSession, repositories);
       // FIXME lack of resiliency, active rules index is corrupted if rule index fails
       // to be updated. Only a single DB commit should be executed.
       ruleIndexer.commitAndIndex(dbSession, rulesRegistrationContext.getAllModified().map(RuleDto::getUuid).collect(Collectors.toSet()));
@@ -164,7 +153,7 @@ public class RulesRegistrant implements Startable {
     }
   }
 
-  private void persistRepositories(DbSession dbSession, List<RulesDefinition.Repository> repositories, boolean deleteMissing) {
+  private void persistRepositories(DbSession dbSession, List<RulesDefinition.Repository> repositories) {
     List<String> keys = repositories.stream().map(RulesDefinition.Repository::key).toList();
     Set<String> existingKeys = dbClient.ruleRepositoryDao().selectAllKeys(dbSession);
 
@@ -174,9 +163,7 @@ public class RulesRegistrant implements Startable {
 
     dbClient.ruleRepositoryDao().update(dbSession, dtos.getOrDefault(true, emptyList()));
     dbClient.ruleRepositoryDao().insert(dbSession, dtos.getOrDefault(false, emptyList()));
-    if (deleteMissing) {
-      dbClient.ruleRepositoryDao().deleteIfKeyNotIn(dbSession, keys);
-    }
+    dbClient.ruleRepositoryDao().deleteIfKeyNotIn(dbSession, keys);
     dbSession.commit();
   }
 
index 7c1496773a64b8575c5917251517235662138126..a5b3f360d45b4e1f9403470f2dc0383a9e5f1000 100644 (file)
@@ -84,7 +84,7 @@ class RulesRegistrationContext {
       RuleDto rule = dbRulesByRuleUuid.get(ruleUuid);
       if (rule == null) {
         LOG.warn("Could not retrieve rule with uuid %s referenced by a deprecated rule key. " +
-            "The following deprecated rule keys seem to be referencing a non-existing rule",
+          "The following deprecated rule keys seem to be referencing a non-existing rule",
           ruleUuid, entry.getValue());
       } else {
         entry.getValue().forEach(d -> rulesByKey.put(d.getOldRuleKeyAsRuleKey(), rule));
@@ -143,10 +143,10 @@ class RulesRegistrationContext {
 
   Stream<RuleDto> getAllModified() {
     return Stream.of(
-        created.stream(),
-        updated.stream(),
-        removed.stream(),
-        renamed.keySet().stream())
+      created.stream(),
+      updated.stream(),
+      removed.stream(),
+      renamed.keySet().stream())
       .flatMap(s -> s);
   }
 
@@ -191,9 +191,8 @@ class RulesRegistrationContext {
     checkState(known.contains(ruleDto), "unknown RuleDto");
   }
 
-  static RulesRegistrationContext create(DbClient dbClient, DbSession dbSession, boolean onlyBuiltIn) {
+  static RulesRegistrationContext create(DbClient dbClient, DbSession dbSession) {
     Map<RuleKey, RuleDto> allRules = dbClient.ruleDao().selectAll(dbSession).stream()
-      .filter(rule -> !onlyBuiltIn || rule.getPluginKey() == null)
       .collect(Collectors.toMap(RuleDto::getKey, Function.identity()));
     Map<String, Set<SingleDeprecatedRuleKey>> existingDeprecatedKeysById = loadDeprecatedRuleKeys(dbClient, dbSession);
     Map<String, List<RuleParamDto>> ruleParamsByRuleUuid = loadAllRuleParameters(dbClient, dbSession);
index 28b566f9eddfecfd22fddb3444487cc0fabe4062..12c503ff29c69d124624ee47cba436ad31a5a00b 100644 (file)
@@ -31,13 +31,13 @@ public class RuleDefinitionsLoaderTest {
 
   @Test
   public void no_definitions() {
-    RulesDefinition.Context context = new RuleDefinitionsLoader(mock(ServerPluginRepository.class)).loadFromPlugins();
+    RulesDefinition.Context context = new RuleDefinitionsLoader(mock(ServerPluginRepository.class)).load();
 
     assertThat(context.repositories()).isEmpty();
   }
 
   @Test
-  public void load_FromPlugins_definitions_only_returns_from_plugins() {
+  public void load_returns_definition() {
     var serverPluginRepository = mock(ServerPluginRepository.class);
     var findbugsDefinitions = new FindbugsDefinitions();
     var builtInJavaDefinitions = new JavaDefinitions();
@@ -46,25 +46,10 @@ public class RuleDefinitionsLoaderTest {
     RulesDefinition.Context context = new RuleDefinitionsLoader(serverPluginRepository,
       new RulesDefinition[] {
         findbugsDefinitions, builtInJavaDefinitions
-      }).loadFromPlugins();
+      }).load();
 
-    assertThat(context.repositories()).hasSize(1);
+    assertThat(context.repositories()).hasSize(2);
     assertThat(context.repository("findbugs")).isNotNull();
-  }
-
-  @Test
-  public void load_builtin_definitions_only_returns_builtin() {
-    var serverPluginRepository = mock(ServerPluginRepository.class);
-    var findbugsDefinitions = new FindbugsDefinitions();
-    var builtInJavaDefinitions = new JavaDefinitions();
-    when(serverPluginRepository.getPluginKey(findbugsDefinitions)).thenReturn("findbugs");
-    when(serverPluginRepository.getPluginKey(builtInJavaDefinitions)).thenReturn(null);
-    RulesDefinition.Context context = new RuleDefinitionsLoader(serverPluginRepository,
-      new RulesDefinition[] {
-        findbugsDefinitions, builtInJavaDefinitions
-      }).loadBuiltIn();
-
-    assertThat(context.repositories()).hasSize(1);
     assertThat(context.repository("java")).isNotNull();
   }
 
index e331d7d6551e48b81fb6dbd55b271bb10641d63d..fdafcfbe451b055c643a6463cda5ddcf0f8b523f 100644 (file)
@@ -68,7 +68,7 @@ public class PlatformLevelStartup extends PlatformLevel {
 
     addIfStartupLeader(
       IndexerStartupTask.class);
-    addIfStartupLeader(
+    addIfStartupLeaderAndPluginsChanged(
       RuleDescriptionSectionsGeneratorResolver.class,
       AdvancedRuleDescriptionSectionsGenerator.class,
       LegacyHotspotRuleDescriptionSectionsGenerator.class,
@@ -78,8 +78,7 @@ public class PlatformLevelStartup extends PlatformLevel {
       NewRuleCreator.class,
       RulesKeyVerifier.class,
       StartupRuleUpdater.class,
-      QualityProfileChangesUpdater.class);
-    addIfStartupLeaderAndPluginsChanged(
+      QualityProfileChangesUpdater.class,
       RegisterMetrics.class,
       RegisterQualityGates.class,
       BuiltInQProfileLoader.class);