]> source.dussan.org Git - sonarqube.git/commitdiff
SONAR-5056 Reindex rules in E/S when migrating requirements to rules
authorJulien Lancelot <julien.lancelot@sonarsource.com>
Mon, 31 Mar 2014 11:39:24 +0000 (13:39 +0200)
committerJulien Lancelot <julien.lancelot@sonarsource.com>
Mon, 31 Mar 2014 11:39:32 +0000 (13:39 +0200)
sonar-server/src/main/java/org/sonar/server/rule/RegisterRules.java
sonar-server/src/main/java/org/sonar/server/rule/RuleRegistry.java
sonar-server/src/main/java/org/sonar/server/startup/CopyRequirementsFromCharacteristicsToRules.java
sonar-server/src/test/java/org/sonar/server/qualityprofile/ESActiveRuleTest.java
sonar-server/src/test/java/org/sonar/server/qualityprofile/QProfileRuleLookupTest.java
sonar-server/src/test/java/org/sonar/server/rule/RuleRegistryTest.java
sonar-server/src/test/java/org/sonar/server/startup/CopyRequirementsFromCharacteristicsToRulesTest.java

index a8dcf16c09e95eef4e295b577bc8fc3b1e6ac8dd..41dfcd72a99f6ead77d304721e47512fca7e6050 100644 (file)
@@ -107,12 +107,8 @@ public class RegisterRules implements Startable {
     try {
       RulesDefinition.Context context = defLoader.load();
       Buffer buffer = new Buffer(system.now());
-      List<CharacteristicDto> characteristicDtos = characteristicDao.selectEnabledCharacteristics();
-      for (CharacteristicDto characteristicDto : characteristicDtos) {
-        buffer.add(characteristicDto);
-      }
       selectRulesFromDb(buffer, sqlSession);
-      enableRuleDefinitions(context, buffer, characteristicDtos, sqlSession);
+      enableRuleDefinitions(context, buffer, sqlSession);
       List<RuleDto> removedRules = processRemainingDbRules(buffer, sqlSession);
       removeActiveRulesOnStillExistingRepositories(removedRules, context);
       index(buffer);
@@ -144,29 +140,32 @@ public class RegisterRules implements Startable {
     for (RuleRuleTagDto tagDto : ruleDao.selectTags(sqlSession)) {
       buffer.add(tagDto);
     }
+    for (CharacteristicDto characteristicDto : characteristicDao.selectEnabledCharacteristics()) {
+      buffer.add(characteristicDto);
+    }
   }
 
-  private void enableRuleDefinitions(RulesDefinition.Context context, Buffer buffer, List<CharacteristicDto> characteristicDtos, SqlSession sqlSession) {
+  private void enableRuleDefinitions(RulesDefinition.Context context, Buffer buffer, SqlSession sqlSession) {
     for (RulesDefinition.Repository repoDef : context.repositories()) {
-      enableRepository(buffer, sqlSession, repoDef, characteristicDtos);
+      enableRepository(buffer, sqlSession, repoDef);
     }
     for (RulesDefinition.ExtendedRepository extendedRepoDef : context.extendedRepositories()) {
       if (context.repository(extendedRepoDef.key()) == null) {
         LOG.warn(String.format("Extension is ignored, repository %s does not exist", extendedRepoDef.key()));
       } else {
-        enableRepository(buffer, sqlSession, extendedRepoDef, characteristicDtos);
+        enableRepository(buffer, sqlSession, extendedRepoDef);
       }
     }
   }
 
-  private void enableRepository(Buffer buffer, SqlSession sqlSession, RulesDefinition.ExtendedRepository repoDef, List<CharacteristicDto> characteristicDtos) {
+  private void enableRepository(Buffer buffer, SqlSession sqlSession, RulesDefinition.ExtendedRepository repoDef) {
     int count = 0;
     for (RulesDefinition.Rule ruleDef : repoDef.rules()) {
       RuleDto dto = buffer.rule(RuleKey.of(ruleDef.repository().key(), ruleDef.key()));
       if (dto == null) {
-        dto = enableAndInsert(buffer, sqlSession, ruleDef, characteristicDtos);
+        dto = enableAndInsert(buffer, sqlSession, ruleDef);
       } else {
-        enableAndUpdate(buffer, sqlSession, ruleDef, dto, characteristicDtos);
+        enableAndUpdate(buffer, sqlSession, ruleDef, dto);
       }
       buffer.markProcessed(dto);
       count++;
@@ -177,7 +176,7 @@ public class RegisterRules implements Startable {
     sqlSession.commit();
   }
 
-  private RuleDto enableAndInsert(Buffer buffer, SqlSession sqlSession, RulesDefinition.Rule ruleDef, List<CharacteristicDto> characteristicDtos) {
+  private RuleDto enableAndInsert(Buffer buffer, SqlSession sqlSession, RulesDefinition.Rule ruleDef) {
     RuleDto ruleDto = new RuleDto()
       .setCardinality(ruleDef.template() ? Cardinality.MULTIPLE : Cardinality.SINGLE)
       .setConfigKey(ruleDef.internalKey())
@@ -191,7 +190,7 @@ public class RegisterRules implements Startable {
       .setUpdatedAt(buffer.now())
       .setStatus(ruleDef.status().name());
 
-    CharacteristicDto characteristic = findCharacteristic(ruleDef, null, characteristicDtos);
+    CharacteristicDto characteristic = buffer.characteristic(ruleDef.debtSubCharacteristic(), ruleDef.repository().key(), ruleDef.key(), null);
     DebtRemediationFunction remediationFunction = ruleDef.debtRemediationFunction();
     if (characteristic != null && remediationFunction != null) {
       ruleDto.setDefaultSubCharacteristicId(characteristic.getId())
@@ -218,8 +217,8 @@ public class RegisterRules implements Startable {
     return ruleDto;
   }
 
-  private void enableAndUpdate(Buffer buffer, SqlSession sqlSession, RulesDefinition.Rule ruleDef, RuleDto dto, List<CharacteristicDto> characteristicDtos) {
-    if (mergeRule(buffer, ruleDef, dto, characteristicDtos)) {
+  private void enableAndUpdate(Buffer buffer, SqlSession sqlSession, RulesDefinition.Rule ruleDef, RuleDto dto) {
+    if (mergeRule(buffer, ruleDef, dto)) {
       ruleDao.update(dto);
     }
     mergeParams(buffer, sqlSession, ruleDef, dto);
@@ -227,7 +226,7 @@ public class RegisterRules implements Startable {
     buffer.markProcessed(dto);
   }
 
-  private boolean mergeRule(Buffer buffer, RulesDefinition.Rule def, RuleDto dto, List<CharacteristicDto> characteristicDtos) {
+  private boolean mergeRule(Buffer buffer, RulesDefinition.Rule def, RuleDto dto) {
     boolean changed = false;
     if (!StringUtils.equals(dto.getName(), def.name())) {
       dto.setName(def.name());
@@ -260,17 +259,17 @@ public class RegisterRules implements Startable {
       dto.setLanguage(def.repository().language());
       changed = true;
     }
-    changed = mergeDebtDefinitions(def, dto, characteristicDtos) || changed;
+    CharacteristicDto characteristic = buffer.characteristic(def.debtSubCharacteristic(), def.repository().key(), def.key(), dto.getSubCharacteristicId());
+    changed = mergeDebtDefinitions(def, dto, characteristic) || changed;
     if (changed) {
       dto.setUpdatedAt(buffer.now());
     }
     return changed;
   }
 
-  private boolean mergeDebtDefinitions(RulesDefinition.Rule def, RuleDto dto, List<CharacteristicDto> characteristicDtos) {
+  private boolean mergeDebtDefinitions(RulesDefinition.Rule def, RuleDto dto, @Nullable CharacteristicDto characteristic) {
     boolean changed = false;
 
-    CharacteristicDto characteristic = findCharacteristic(def, dto.getSubCharacteristicId(), characteristicDtos);
     // Debt definitions are set to null if the characteristic is null or unknown
     boolean hasCharacteristic = characteristic != null;
     DebtRemediationFunction debtRemediationFunction = characteristic != null ? def.debtRemediationFunction() : null;
@@ -553,34 +552,32 @@ public class RegisterRules implements Startable {
     void markProcessed(RuleDto ruleDto) {
       unprocessedRuleIds.remove(ruleDto.getId());
     }
-  }
 
-  @CheckForNull
-  private CharacteristicDto findCharacteristic(RulesDefinition.Rule ruleDef, @Nullable Integer overridingCharacteristicId, List<CharacteristicDto> characteristicDtos) {
-    String key = ruleDef.debtSubCharacteristic();
-    // Rule is not linked to a default characteristic or characteristic has been disabled by user, nothing to do
-    if (key == null) {
-      return null;
-    }
-    CharacteristicDto characteristicDto = findCharacteristic(key, characteristicDtos);
-    if (characteristicDto == null) {
-      // Log a warning only if rule has not been overridden by user
-      if (overridingCharacteristicId == null) {
-        LOG.warn(String.format("Characteristic '%s' has not been found on rule '%s:%s'", key, ruleDef.repository().key(), ruleDef.key()));
+    CharacteristicDto characteristic(@Nullable String subCharacteristic, String repo, String ruleKey, @Nullable Integer overridingCharacteristicId){
+      // Rule is not linked to a default characteristic or characteristic has been disabled by user
+      if (subCharacteristic == null) {
+        return null;
       }
-    } else if (characteristicDto.getParentId() == null) {
-      throw MessageException.of(String.format("Rule '%s:%s' cannot be linked on the root characteristic '%s'", ruleDef.repository().key(), ruleDef.key(), key));
+      CharacteristicDto characteristicDto = findCharacteristic(subCharacteristic);
+      if (characteristicDto == null) {
+        // Log a warning only if rule has not been overridden by user
+        if (overridingCharacteristicId == null) {
+          LOG.warn(String.format("Characteristic '%s' has not been found on rule '%s:%s'", subCharacteristic, repo, ruleKey));
+        }
+      } else if (characteristicDto.getParentId() == null) {
+        throw MessageException.of(String.format("Rule '%s:%s' cannot be linked on the root characteristic '%s'", repo, ruleKey, subCharacteristic));
+      }
+      return characteristicDto;
     }
-    return characteristicDto;
-  }
 
-  @CheckForNull
-  private CharacteristicDto findCharacteristic(final String key, List<CharacteristicDto> characteristicDtos) {
-    return Iterables.find(characteristicDtos, new Predicate<CharacteristicDto>() {
-      @Override
-      public boolean apply(CharacteristicDto input) {
-        return key.equals(input.getKey());
-      }
-    }, null);
+    @CheckForNull
+    private CharacteristicDto findCharacteristic(final String key) {
+      return Iterables.find(characteristicsById.values(), new Predicate<CharacteristicDto>() {
+        @Override
+        public boolean apply(@Nullable CharacteristicDto input) {
+          return input != null && key.equals(input.getKey());
+        }
+      }, null);
+    }
   }
 }
index ce9c37ac918f3b11660b91dc4ec9e0e05693d8c4..ea7611f44ca69f424db13145120bf60f853e503b 100644 (file)
 
 package org.sonar.server.rule;
 
+import com.google.common.collect.ArrayListMultimap;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableList.Builder;
 import com.google.common.collect.Multimap;
 import org.apache.commons.lang.StringUtils;
+import org.apache.ibatis.session.SqlSession;
 import org.elasticsearch.ElasticSearchException;
 import org.elasticsearch.common.collect.Lists;
 import org.elasticsearch.common.collect.Maps;
@@ -40,7 +42,9 @@ import org.elasticsearch.search.SearchHits;
 import org.elasticsearch.search.sort.SortOrder;
 import org.sonar.api.rule.RuleKey;
 import org.sonar.api.utils.TimeProfiler;
+import org.sonar.core.persistence.MyBatis;
 import org.sonar.core.rule.*;
+import org.sonar.core.technicaldebt.db.CharacteristicDao;
 import org.sonar.core.technicaldebt.db.CharacteristicDto;
 import org.sonar.server.es.ESIndex;
 import org.sonar.server.es.SearchQuery;
@@ -59,6 +63,7 @@ import java.util.List;
 import java.util.Map;
 
 import static com.google.common.collect.Lists.newArrayList;
+import static com.google.common.collect.Maps.newHashMap;
 import static org.elasticsearch.index.query.FilterBuilders.*;
 import static org.sonar.api.rules.Rule.STATUS_REMOVED;
 
@@ -75,17 +80,48 @@ public class RuleRegistry {
   private static final String PARAM_STATUS = "status";
 
   private final ESIndex searchIndex;
+  private final MyBatis myBatis;
   private final RuleDao ruleDao;
+  private final CharacteristicDao characteristicDao;
 
-  public RuleRegistry(ESIndex searchIndex, RuleDao ruleDao) {
+  public RuleRegistry(ESIndex searchIndex, MyBatis myBatis, RuleDao ruleDao, CharacteristicDao characteristicDao) {
     this.searchIndex = searchIndex;
+    this.myBatis = myBatis;
     this.ruleDao = ruleDao;
+    this.characteristicDao = characteristicDao;
   }
 
   public void start() {
     searchIndex.addMappingFromClasspath(INDEX_RULES, TYPE_RULE, "/org/sonar/server/es/config/mappings/rule_mapping.json");
   }
 
+  public void reindexRules() {
+    SqlSession sqlSession = myBatis.openSession();
+    try {
+      Multimap<Integer, RuleParamDto> paramsByRuleId = ArrayListMultimap.create();
+      Multimap<Integer, RuleRuleTagDto> tagsByRuleId = ArrayListMultimap.create();
+      Map<Integer, CharacteristicDto> characteristicsById = newHashMap();
+
+      for (RuleParamDto paramDto : ruleDao.selectParameters(sqlSession)) {
+        paramsByRuleId.put(paramDto.getRuleId(), paramDto);
+      }
+      for (RuleRuleTagDto tagDto : ruleDao.selectTags(sqlSession)) {
+        tagsByRuleId.put(tagDto.getRuleId(), tagDto);
+      }
+      for (CharacteristicDto characteristicDto : characteristicDao.selectEnabledCharacteristics(sqlSession)) {
+        characteristicsById.put(characteristicDto.getId(), characteristicDto);
+      }
+
+      bulkIndexRules(
+        ruleDao.selectEnablesAndNonManual(sqlSession),
+        characteristicsById,
+        paramsByRuleId,
+        tagsByRuleId);
+    } finally {
+      sqlSession.close();
+    }
+  }
+
   public void bulkRegisterRules(Collection<RuleDto> rules, Map<Integer, CharacteristicDto> characteristicByRule, Multimap<Integer, RuleParamDto> paramsByRule,
                                 Multimap<Integer, RuleRuleTagDto> tagsByRule) {
     String[] ids = bulkIndexRules(rules, characteristicByRule, paramsByRule, tagsByRule);
index cbf1e521f5378b400e479cd6535ab0ab463f641a..195d873bd63a93adc8a8cfd6a0d468a94a866bb2 100644 (file)
@@ -40,6 +40,7 @@ import org.sonar.core.technicaldebt.db.RequirementDto;
 import org.sonar.server.db.migrations.MassUpdater;
 import org.sonar.server.db.migrations.SqlUtil;
 import org.sonar.server.rule.RegisterRules;
+import org.sonar.server.rule.RuleRegistry;
 
 import javax.annotation.CheckForNull;
 import javax.annotation.Nullable;
@@ -68,37 +69,44 @@ public class CopyRequirementsFromCharacteristicsToRules {
 
   private final RequirementDao requirementDao;
 
+  private final RuleRegistry ruleRegistry;
+
   /**
    * @param registerRules used only to be started after init of rules
    */
-  public CopyRequirementsFromCharacteristicsToRules(Database database, RequirementDao requirementDao, ServerUpgradeStatus status, RegisterRules registerRules) {
-    this(database, requirementDao, status, System2.INSTANCE);
+  public CopyRequirementsFromCharacteristicsToRules(Database database, RequirementDao requirementDao, ServerUpgradeStatus status, RuleRegistry ruleRegistry,
+                                                    RegisterRules registerRules) {
+    this(database, requirementDao, ruleRegistry, status, System2.INSTANCE);
   }
 
   @VisibleForTesting
-  CopyRequirementsFromCharacteristicsToRules(Database database, RequirementDao requirementDao, ServerUpgradeStatus status, System2 system2) {
+  CopyRequirementsFromCharacteristicsToRules(Database database, RequirementDao requirementDao, RuleRegistry ruleRegistry, ServerUpgradeStatus status, System2 system2) {
     this.db = database;
     this.system2 = system2;
     this.status = status;
     this.requirementDao = requirementDao;
+    this.ruleRegistry = ruleRegistry;
   }
 
   public void start() {
-    if (mustDoPurge()) {
-      doPurge();
+    if (mustDoExecute()) {
+      doExecute();
     }
   }
 
-  private boolean mustDoPurge() {
+  private boolean mustDoExecute() {
     return status.isUpgraded() && status.getInitialDbVersion() <= 520;
   }
 
-  private void doPurge() {
+  private void doExecute() {
     LOGGER.info("Copying requirement from characteristics to rules");
     copyRequirementsFromCharacteristicsToRules();
 
     LOGGER.info("Deleting requirements data");
     removeRequirementsDataFromCharacteristics();
+
+    LOGGER.info("Reindex rules in E/S");
+    ruleRegistry.reindexRules();
   }
 
   private void copyRequirementsFromCharacteristicsToRules() {
index d9f019634b2aa1d28e95c768b037edcae44f41ce..ad5e0a6616bc4d5037d2c5efef86edd32994e452 100644 (file)
@@ -92,7 +92,7 @@ public class ESActiveRuleTest {
     searchIndex = new ESIndex(node, profiling);
     searchIndex.start();
 
-    RuleRegistry esRule = new RuleRegistry(searchIndex, null);
+    RuleRegistry esRule = new RuleRegistry(searchIndex, null, null, null);
     esRule.start();
     esActiveRule = new ESActiveRule(searchIndex, activeRuleDao, myBatis, profiling);
     esActiveRule.start();
index c02b3708437f212c4555ea6639c3a720d06b1036..0e114e9e98461d705686226502a68d45fbd53628 100644 (file)
@@ -19,8 +19,6 @@
  */
 package org.sonar.server.qualityprofile;
 
-import org.sonar.server.paging.Paging;
-
 import com.github.tlrx.elasticsearch.test.EsSetup;
 import org.apache.commons.io.IOUtils;
 import org.elasticsearch.client.Requests;
@@ -33,6 +31,7 @@ import org.sonar.api.rule.Severity;
 import org.sonar.core.profiling.Profiling;
 import org.sonar.server.es.ESIndex;
 import org.sonar.server.es.ESNode;
+import org.sonar.server.paging.Paging;
 import org.sonar.server.rule.RuleRegistry;
 import org.sonar.test.TestUtils;
 
@@ -62,7 +61,7 @@ public class QProfileRuleLookupTest {
     settings.setProperty("sonar.log.profilingLevel", "FULL");
     ESIndex index = new ESIndex(searchNode, new Profiling(settings));
     index.start();
-    RuleRegistry registry = new RuleRegistry(index, null);
+    RuleRegistry registry = new RuleRegistry(index, null, null, null);
     registry.start();
     ESActiveRule esActiveRule = new ESActiveRule(index, null, null, null);
     esActiveRule.start();
index 43b9164b50792196118af03a1e8981697888214d..3fdab92d7901a428293d8cabda0d0854ad443306 100644 (file)
@@ -38,6 +38,7 @@ import org.sonar.api.rule.Severity;
 import org.sonar.core.persistence.MyBatis;
 import org.sonar.core.profiling.Profiling;
 import org.sonar.core.rule.*;
+import org.sonar.core.technicaldebt.db.CharacteristicDao;
 import org.sonar.core.technicaldebt.db.CharacteristicDto;
 import org.sonar.server.es.ESIndex;
 import org.sonar.server.es.ESNode;
@@ -51,8 +52,7 @@ import static com.google.common.collect.Lists.newArrayList;
 import static com.google.common.collect.Maps.newHashMap;
 import static org.fest.assertions.Assertions.assertThat;
 import static org.mockito.Matchers.any;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.when;
+import static org.mockito.Mockito.*;
 
 @RunWith(MockitoJUnitRunner.class)
 public class RuleRegistryTest {
@@ -61,11 +61,14 @@ public class RuleRegistryTest {
 
   ESIndex searchIndex;
 
+  @Mock
+  MyBatis myBatis;
+
   @Mock
   RuleDao ruleDao;
 
   @Mock
-  MyBatis myBatis;
+  CharacteristicDao characteristicDao;
 
   @Mock
   SqlSession session;
@@ -89,7 +92,7 @@ public class RuleRegistryTest {
     searchIndex = new ESIndex(node, profiling);
     searchIndex.start();
 
-    registry = new RuleRegistry(searchIndex, ruleDao);
+    registry = new RuleRegistry(searchIndex, myBatis, ruleDao, characteristicDao);
     registry.start();
 
     esSetup.execute(
@@ -231,6 +234,19 @@ public class RuleRegistryTest {
     assertThat((List<String>) rule2Document.get(RuleDocument.FIELD_ADMIN_TAGS)).hasSize(1);
   }
 
+  @Test
+  public void reindex_all_rules() {
+    SqlSession session = mock(SqlSession.class);
+    when(myBatis.openSession()).thenReturn(session);
+
+    registry.reindexRules();
+
+    verify(ruleDao).selectEnablesAndNonManual(session);
+    verify(ruleDao).selectParameters(session);
+    verify(ruleDao).selectTags(session);
+    verify(characteristicDao).selectEnabledCharacteristics(session);
+  }
+
   @Test
   public void index_and_reindex_single_rule() {
     RuleDto rule = new RuleDto();
index f7d48d21523cc8e73c21229c2a90303245a04257..2fda3e16e20ea67e91d321e6f1ed26701686b207 100644 (file)
@@ -31,8 +31,10 @@ import org.sonar.api.utils.System2;
 import org.sonar.core.persistence.AbstractDaoTestCase;
 import org.sonar.core.persistence.TestDatabase;
 import org.sonar.core.technicaldebt.db.RequirementDao;
+import org.sonar.server.rule.RuleRegistry;
 
 import static org.fest.assertions.Assertions.assertThat;
+import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
 @RunWith(MockitoJUnitRunner.class)
@@ -47,12 +49,15 @@ public class CopyRequirementsFromCharacteristicsToRulesTest extends AbstractDaoT
   @Mock
   System2 system2;
 
+  @Mock
+  RuleRegistry ruleRegistry;
+
   CopyRequirementsFromCharacteristicsToRules service;
 
   @Before
   public void setUp() throws Exception {
     when(system2.now()).thenReturn(DateUtils.parseDateTime("2014-03-13T19:10:03+0100").getTime());
-    service = new CopyRequirementsFromCharacteristicsToRules(db.database(), new RequirementDao(getMyBatis()), status, system2);
+    service = new CopyRequirementsFromCharacteristicsToRules(db.database(), new RequirementDao(getMyBatis()), ruleRegistry, status, system2);
   }
 
   @Test
@@ -66,6 +71,7 @@ public class CopyRequirementsFromCharacteristicsToRulesTest extends AbstractDaoT
     service.start();
 
     db.assertDbUnit(getClass(), "copy_requirements_from_characteristics_to_rules_result.xml", "rules");
+    verify(ruleRegistry).reindexRules();
   }
 
   @Test
@@ -78,6 +84,7 @@ public class CopyRequirementsFromCharacteristicsToRulesTest extends AbstractDaoT
     service.start();
 
     db.assertDbUnit(getClass(), "remove_requirements_data_from_characteristics_result.xml", "characteristics");
+    verify(ruleRegistry).reindexRules();
   }
 
   @Test