]> source.dussan.org Git - sonarqube.git/commitdiff
SONAR-4923 Fix issue where active rule parent id where not updated in ES when setting...
authorJulien Lancelot <julien.lancelot@sonarsource.com>
Thu, 16 Jan 2014 09:49:57 +0000 (10:49 +0100)
committerJulien Lancelot <julien.lancelot@sonarsource.com>
Thu, 16 Jan 2014 09:50:07 +0000 (10:50 +0100)
17 files changed:
sonar-core/src/main/java/org/sonar/core/qualityprofile/db/ActiveRuleDto.java
sonar-core/src/main/resources/org/sonar/core/qualityprofile/db/ActiveRuleMapper.xml
sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileOperations.java
sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileRule.java
sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileRuleParam.java
sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfiles.java
sonar-server/src/main/java/org/sonar/server/rule/ActiveRuleDocument.java
sonar-server/src/main/java/org/sonar/server/rule/RuleDocument.java
sonar-server/src/main/java/org/sonar/server/rule/RuleRegistry.java
sonar-server/src/main/resources/org/sonar/server/es/config/mappings/active_rule_mapping.json
sonar-server/src/main/resources/org/sonar/server/es/config/mappings/rule_mapping.json
sonar-server/src/main/webapp/WEB-INF/app/controllers/rules_configuration_controller.rb
sonar-server/src/main/webapp/WEB-INF/app/views/rules_configuration/_rule.html.erb
sonar-server/src/test/java/org/sonar/server/qualityprofile/QProfilesTest.java
sonar-server/src/test/java/org/sonar/server/rule/ProfileRulesTest.java
sonar-server/src/test/resources/org/sonar/server/rule/ProfileRulesTest/find_active_rules_with_inheritance/active_rule25.json [new file with mode: 0644]
sonar-server/src/test/resources/org/sonar/server/rule/ProfileRulesTest/find_active_rules_with_inheritance/active_rule391.json [new file with mode: 0644]

index d5a4786caa03e9ec81fbaf3ccf408ef57c5672a5..1f83e8e52f4d942f48f217814073e1aa34ed1825 100644 (file)
@@ -24,6 +24,7 @@ import org.apache.commons.lang.StringUtils;
 
 import javax.annotation.CheckForNull;
 import javax.annotation.Nullable;
+import javax.persistence.Transient;
 
 import java.util.Date;
 
@@ -42,6 +43,8 @@ public class ActiveRuleDto {
   private String noteUserLogin;
   private String noteData;
 
+  // This field do not exists in db, it's only retrieve by joins
+  @Transient
   private Integer parentId;
 
   public Integer getId() {
index f33b6a950b67a0a0d9647d5f3e8a7b1402ac5c1e..5b3352e6e7fdf8f65ad09eb27c062bf6be9e6628 100644 (file)
@@ -62,7 +62,7 @@
 
   <select id="selectById" parameterType="Integer" resultType="ActiveRule">
     SELECT <include refid="activeRuleColumns"/>
-    FROM active_rules a
+    FROM active_rules a1
     <include refid="activeRuleJoin"/>
     <where>
       AND a.id=#{id}
@@ -99,7 +99,7 @@
 
   <select id="selectAll" parameterType="map" resultType="ActiveRule">
     select
-    <include refid="activeRuleColumns"/>,active_rule_parent.id as parentId
+    <include refid="activeRuleColumns"/>
     from active_rules a
     <include refid="activeRuleJoin"/>
   </select>
index aae23b1fb5ff805301abbcd43d65c006b3921689..a67e3c0a59ada2e9eccfa4a22060987195b2279f 100644 (file)
@@ -154,15 +154,13 @@ public class QProfileOperations implements ServerComponent {
         throw new BadRequestException("Please do not select a child profile as parent.");
       }
       String parentName = parentProfile != null ? parentProfile.getName() : null;
-
-      ProfilesManager.RuleInheritanceActions actions = profilesManager.profileParentChanged(profile.getId(), parentName, userSession.name());
-      ruleRegistry.deleteActiveRules(actions.idsToDelete());
-      ruleRegistry.bulkIndexActiveRuleIds(actions.idsToIndex(), session);
-
       profile.setParent(parentName);
       dao.update(profile, session);
       session.commit();
 
+      ProfilesManager.RuleInheritanceActions actions = profilesManager.profileParentChanged(profile.getId(), parentName, userSession.name());
+      ruleRegistry.deleteActiveRules(actions.idsToDelete());
+      ruleRegistry.bulkIndexActiveRuleIds(actions.idsToIndex(), session);
     } finally {
       MyBatis.closeQuietly(session);
     }
index e41139d706ed5182c5e0c76f58b2410c27d42127..6ecfcc19c0ea4430b2cfb5a8a6058d319679794d 100644 (file)
@@ -39,19 +39,19 @@ public class QProfileRule {
   public static final String OVERRIDES = "OVERRIDES";
 
   private final Integer id;
-  private final Integer parentId;
   private final String key;
   private final String repositoryKey;
   private final String name;
   private final String description;
   private final String status;
   private final String cardinality;
-  private final Integer ruleId;
+  private final Integer templateId;
   private final Date createdAt;
   private final Date updatedAt;
   private final QProfileRuleNote ruleNote;
 
   private final Integer activeRuleId;
+  private final Integer activeRuleParentId;
   private final String severity;
   private final String inheritance;
   private final QProfileRuleNote activeRuleNote;
@@ -68,7 +68,7 @@ public class QProfileRule {
     description = (String) ruleSource.get(RuleDocument.FIELD_DESCRIPTION);
     status = (String) ruleSource.get(RuleDocument.FIELD_STATUS);
     cardinality = (String) ruleSource.get("cardinality");
-    ruleId = (Integer) ruleSource.get(RuleDocument.FIELD_PARENT_KEY);
+    templateId = (Integer) ruleSource.get(RuleDocument.FIELD_TEMPLATE_ID);
     createdAt = parseOptionalDate(RuleDocument.FIELD_CREATED_AT, ruleSource);
     updatedAt = parseOptionalDate(RuleDocument.FIELD_UPDATED_AT, ruleSource);
 
@@ -89,12 +89,12 @@ public class QProfileRule {
       severity = (String) ruleSource.get(ActiveRuleDocument.FIELD_SEVERITY);
       inheritance = null;
       activeRuleNote = null;
-      parentId = null;
+      activeRuleParentId = null;
     } else {
       activeRuleId = (Integer) activeRuleSource.get(ActiveRuleDocument.FIELD_ID);
       severity = (String) activeRuleSource.get(ActiveRuleDocument.FIELD_SEVERITY);
       inheritance = (String) activeRuleSource.get(ActiveRuleDocument.FIELD_INHERITANCE);
-      parentId = (Integer) activeRuleSource.get(ActiveRuleDocument.FIELD_PARENT_ID);
+      activeRuleParentId = (Integer) activeRuleSource.get(ActiveRuleDocument.FIELD_ACTIVE_RULE_PARENT_ID);
 
       if (activeRuleSource.containsKey(ActiveRuleDocument.FIELD_NOTE)) {
         Map<String, Object> ruleNoteDocument = (Map<String, Object>) activeRuleSource.get(ActiveRuleDocument.FIELD_NOTE);
@@ -207,13 +207,13 @@ public class QProfileRule {
   }
 
   @CheckForNull
-  public Integer ruleId() {
-    return ruleId;
+  public Integer templateId() {
+    return templateId;
   }
 
   @CheckForNull
-  public Integer parentId() {
-    return parentId;
+  public Integer activeRuleParentId() {
+    return activeRuleParentId;
   }
 
   public boolean isInherited() {
@@ -229,7 +229,7 @@ public class QProfileRule {
   }
 
   public boolean isEditable() {
-    return ruleId != null;
+    return templateId != null;
   }
 
   public List<QProfileRuleParam> params() {
@@ -245,11 +245,6 @@ public class QProfileRule {
     return activeRuleNote;
   }
 
-  @Override
-  public String toString() {
-    return new ReflectionToStringBuilder(this).toString();
-  }
-
   public List<String> systemTags() {
     return systemTags;
   }
@@ -258,4 +253,9 @@ public class QProfileRule {
     return adminTags;
   }
 
+  @Override
+  public String toString() {
+    return new ReflectionToStringBuilder(this).toString();
+  }
+
 }
index e6a32a9c7c17cffe9a453ba45c0a0794e48b72a7..0749a5c0d1c46f46fab3e0f3556140ae90cec2b7 100644 (file)
@@ -19,6 +19,8 @@
  */
 package org.sonar.server.qualityprofile;
 
+import org.apache.commons.lang.builder.ReflectionToStringBuilder;
+
 import javax.annotation.CheckForNull;
 import javax.annotation.Nullable;
 
@@ -59,4 +61,9 @@ public class QProfileRuleParam {
   public String type() {
     return type;
   }
+
+  @Override
+  public String toString() {
+    return new ReflectionToStringBuilder(this).toString();
+  }
 }
index b0cc11a853e25e79f8265c1e2019d4a2ebd01755..516b21d904708a33ed91e9e20b9ccdcdd72fc94f 100644 (file)
@@ -282,7 +282,7 @@ public class QProfiles implements ServerComponent {
 
   @CheckForNull
   public QProfileRule parentProfileRule(QProfileRule rule) {
-    Integer parentId = rule.parentId();
+    Integer parentId = rule.activeRuleParentId();
     if (parentId != null) {
       return rules.findByActiveRuleId(parentId);
     }
index 9e8ca6aa0f3309c8e23c1119793b470633bcdb52..523a4249ae60876e3b5c90f0337e5e3fb10ae71b 100644 (file)
@@ -23,10 +23,10 @@ package org.sonar.server.rule;
 public class ActiveRuleDocument {
 
   public static final String FIELD_ID = "id";
-  public static final String FIELD_PARENT_ID = "parentId";
   public static final String FIELD_SEVERITY = "severity";
   public static final String FIELD_PROFILE_ID = "profileId";
   public static final String FIELD_INHERITANCE = "inheritance";
+  public static final String FIELD_ACTIVE_RULE_PARENT_ID = "activeRuleParentId";
   public static final String FIELD_PARAMS = "params";
 
   public static final String FIELD_NOTE = "note";
index 8d77cc061b13ac3792c21d3f79229988a8f5eefc..e19b489cec460d4cc273bf5828b3718e5eb20842 100644 (file)
@@ -30,7 +30,7 @@ public final class RuleDocument {
   public static final String FIELD_STATUS = "status";
   public static final String FIELD_SEVERITY = "severity";
   public static final String FIELD_REPOSITORY_KEY = "repositoryKey";
-  public static final String FIELD_PARENT_KEY = "parentKey";
+  public static final String FIELD_TEMPLATE_ID = "templateId";
   public static final String FIELD_DESCRIPTION = "description";
   public static final String FIELD_NAME = "name";
   public static final String FIELD_LANGUAGE = "language";
index aba815f509e87afd8f3c87cae4990e4e28dfe960..3c9f4695273aaf58a3c7a70b8ba6bbe786570c9d 100644 (file)
@@ -305,7 +305,7 @@ public class RuleRegistry {
       .field(RuleDocument.FIELD_LANGUAGE, rule.getLanguage())
       .field(RuleDocument.FIELD_NAME, rule.getName())
       .field(RuleDocument.FIELD_DESCRIPTION, rule.getDescription())
-      .field(RuleDocument.FIELD_PARENT_KEY, rule.getParentId() == null ? null : rule.getParentId())
+      .field(RuleDocument.FIELD_TEMPLATE_ID, rule.getParentId() == null ? null : rule.getParentId())
       .field(RuleDocument.FIELD_REPOSITORY_KEY, rule.getRepositoryKey())
       .field(RuleDocument.FIELD_SEVERITY, getSeverityFromOrdinal(rule.getSeverity()))
       .field(RuleDocument.FIELD_STATUS, rule.getStatus())
@@ -357,7 +357,7 @@ public class RuleRegistry {
     XContentBuilder document = XContentFactory.jsonBuilder()
       .startObject()
       .field(ActiveRuleDocument.FIELD_ID, activeRule.getId())
-      .field(ActiveRuleDocument.FIELD_PARENT_ID, activeRule.getParentId())
+      .field(ActiveRuleDocument.FIELD_ACTIVE_RULE_PARENT_ID, activeRule.getParentId())
       .field(ActiveRuleDocument.FIELD_SEVERITY, getSeverityFromOrdinal(activeRule.getSeverity()))
       .field(ActiveRuleDocument.FIELD_PROFILE_ID, activeRule.getProfileId())
       .field(ActiveRuleDocument.FIELD_INHERITANCE, activeRule.getInheritance());
index 8fef621f2e6fc6ee1f65f61d63e156df45cf3c11..b471e1c88e7ec86ecb71a18a6bf03b5a7a4b1408 100644 (file)
@@ -19,7 +19,7 @@
         "type": "integer",
         "index": "not_analyzed"
       },
-      "parentId": {
+      "activeRuleParentId": {
         "type": "integer",
         "index": "not_analyzed"
       },
index 564dfc1044a69ca403a71cecfbb6bb85f92a5817..53f87b54e6d0b008f6f5fbdf8305037f0479d4e3 100644 (file)
@@ -27,7 +27,7 @@
         "type": "string",
         "index": "no"
       },
-      "parentKey": {
+      "templateId": {
         "type": "string",
         "index": "not_analyzed"
       },
index 74ab0442349c922d0e5d9c4f5d491df47acd8412..1f482f46e6a2e3e276cfd253e5a56573c56ccfe7 100644 (file)
@@ -191,10 +191,10 @@ class RulesConfigurationController < ApplicationController
     call_backend do
       @profile = Internal.quality_profiles.profile(params[:id].to_i)
       @rule = Internal.quality_profiles.rule(params[:rule_id].to_i)
-      if @rule.ruleId().nil?
+      if @rule.templateId().nil?
         redirect_to :action => 'index', :id => params[:id]
       else
-        @parent_rule = Internal.quality_profiles.rule(@rule.ruleId())
+        @parent_rule = Internal.quality_profiles.rule(@rule.templateId())
         @active_rules = Internal.quality_profiles.countActiveRules(@rule)
       end
     end
index 9b1185729199aa0e15cf050826e16ab2bc5d417a..5611800f023e879fe9ca037731744a8022b5760c 100644 (file)
     %>
       <table width="100%" class="table spacer-bottom bordered background-gray">
         <%
+           puts "#### " + rule.to_s
            if parent_active_rule
              parent_active_rule_link = link_to parent_profile.name, :controller => 'rules_configuration', :action => 'index',
                                                  :id => parent_profile.id, :rule_id => rule.id, :anchor => 'rule' + rule.id.to_s
         %>
           <tr>
             <td colspan="2">
-              <%= message(rule.inherited? ? 'rules_configuration.rule_inherited_from_profile_x' : 'rules_configuration.rule_overriding_from_profile_x',
-                          :params => parent_active_rule_link) -%>
+              <%= message(rule.inherited? ? 'rules_configuration.rule_inherited_from_profile_x' : 'rules_configuration.rule_overriding_from_profile_x', :params => parent_active_rule_link) -%>
               <% if parent_active_rule.severity != rule.severity %>
                 <img src="<%= ApplicationController.root_context -%>/images/overrides.png" alt="Overrides parent definition" title="<%= message('rules_configuration.overrides_parent_definition') -%>" style="vertical-align: middle"/>
                 <span class="form-val-note" style="font-weight: bold"> <%= message('rules_configuration.original_severity') -%>
index 6314f45707956bc7260ec43648f3ddc5c95b1f2a..ab6346e2d87e3a30db188d587053558a037b52d0 100644 (file)
@@ -287,7 +287,7 @@ public class QProfilesTest {
   public void parent_active_rule() throws Exception {
     QProfileRule rule = mock(QProfileRule.class);
     when(rule.id()).thenReturn(10);
-    when(rule.parentId()).thenReturn(6);
+    when(rule.activeRuleParentId()).thenReturn(6);
 
     qProfiles.parentProfileRule(rule);
     verify(rules).findByActiveRuleId(6);
@@ -297,7 +297,7 @@ public class QProfilesTest {
   public void parent_active_rule_return_null_when_no_parent() throws Exception {
     QProfileRule rule = mock(QProfileRule.class);
     when(rule.id()).thenReturn(10);
-    when(rule.parentId()).thenReturn(null);
+    when(rule.activeRuleParentId()).thenReturn(null);
 
     assertThat(qProfiles.parentProfileRule(rule)).isNull();
     verify(rules, never()).findByActiveRuleId(anyInt());
index 89bbb16754188211021842544ac63804d79f3525..60ff732ac69ce442c3ef5af4bec96bc7ca756b41 100644 (file)
@@ -101,6 +101,21 @@ public class ProfileRulesTest {
     assertThat(profileRules.findByActiveRuleId(9999)).isNull();
   }
 
+  @Test
+  public void find_by_active_rule_id_with_inheritance() throws Exception {
+    esSetup.client().prepareBulk()
+      // Parent active rule
+      .add(Requests.indexRequest().index("rules").type("active_rule").parent("25").source(testFileAsString("find_active_rules_with_inheritance/active_rule25.json")))
+        // Child active rule
+      .add(Requests.indexRequest().index("rules").type("active_rule").parent("759").source(testFileAsString("find_active_rules_with_inheritance/active_rule391.json")))
+      .setRefresh(true)
+      .execute().actionGet();
+
+    QProfileRule child = profileRules.findByActiveRuleId(391);
+    assertThat(child.inheritance()).isEqualTo("INHERITED");
+    assertThat(child.activeRuleParentId()).isEqualTo(25);
+  }
+
   @Test
   public void find_by_profile_id_and_rule_id() {
     // Rules on profiles
diff --git a/sonar-server/src/test/resources/org/sonar/server/rule/ProfileRulesTest/find_active_rules_with_inheritance/active_rule25.json b/sonar-server/src/test/resources/org/sonar/server/rule/ProfileRulesTest/find_active_rules_with_inheritance/active_rule25.json
new file mode 100644 (file)
index 0000000..c38e4a3
--- /dev/null
@@ -0,0 +1,6 @@
+{
+  "id": 25,
+  "severity": "MINOR",
+  "profileId": 1,
+  "inheritance": null
+}
diff --git a/sonar-server/src/test/resources/org/sonar/server/rule/ProfileRulesTest/find_active_rules_with_inheritance/active_rule391.json b/sonar-server/src/test/resources/org/sonar/server/rule/ProfileRulesTest/find_active_rules_with_inheritance/active_rule391.json
new file mode 100644 (file)
index 0000000..fb70c85
--- /dev/null
@@ -0,0 +1,7 @@
+{
+  "id": 391,
+  "severity": "MINOR",
+  "profileId": 2,
+  "inheritance": "INHERITED",
+  "activeRuleParentId": 25
+}