From: Julien Lancelot Date: Thu, 16 Jan 2014 09:49:57 +0000 (+0100) Subject: SONAR-4923 Fix issue where active rule parent id where not updated in ES when setting... X-Git-Tag: 4.2~658 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=cd44bc8dd3a6035abdf30ba4ab3e011c7f0a18ec;p=sonarqube.git SONAR-4923 Fix issue where active rule parent id where not updated in ES when setting parent profile --- diff --git a/sonar-core/src/main/java/org/sonar/core/qualityprofile/db/ActiveRuleDto.java b/sonar-core/src/main/java/org/sonar/core/qualityprofile/db/ActiveRuleDto.java index d5a4786caa0..1f83e8e52f4 100644 --- a/sonar-core/src/main/java/org/sonar/core/qualityprofile/db/ActiveRuleDto.java +++ b/sonar-core/src/main/java/org/sonar/core/qualityprofile/db/ActiveRuleDto.java @@ -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() { diff --git a/sonar-core/src/main/resources/org/sonar/core/qualityprofile/db/ActiveRuleMapper.xml b/sonar-core/src/main/resources/org/sonar/core/qualityprofile/db/ActiveRuleMapper.xml index f33b6a950b6..5b3352e6e7f 100644 --- a/sonar-core/src/main/resources/org/sonar/core/qualityprofile/db/ActiveRuleMapper.xml +++ b/sonar-core/src/main/resources/org/sonar/core/qualityprofile/db/ActiveRuleMapper.xml @@ -62,7 +62,7 @@ select - ,active_rule_parent.id as parentId + from active_rules a diff --git a/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileOperations.java b/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileOperations.java index aae23b1fb5f..a67e3c0a59a 100644 --- a/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileOperations.java +++ b/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileOperations.java @@ -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); } diff --git a/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileRule.java b/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileRule.java index e41139d706e..6ecfcc19c0e 100644 --- a/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileRule.java +++ b/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileRule.java @@ -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 ruleNoteDocument = (Map) 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 params() { @@ -245,11 +245,6 @@ public class QProfileRule { return activeRuleNote; } - @Override - public String toString() { - return new ReflectionToStringBuilder(this).toString(); - } - public List systemTags() { return systemTags; } @@ -258,4 +253,9 @@ public class QProfileRule { return adminTags; } + @Override + public String toString() { + return new ReflectionToStringBuilder(this).toString(); + } + } diff --git a/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileRuleParam.java b/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileRuleParam.java index e6a32a9c7c1..0749a5c0d1c 100644 --- a/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileRuleParam.java +++ b/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileRuleParam.java @@ -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(); + } } diff --git a/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfiles.java b/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfiles.java index b0cc11a853e..516b21d9047 100644 --- a/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfiles.java +++ b/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfiles.java @@ -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); } diff --git a/sonar-server/src/main/java/org/sonar/server/rule/ActiveRuleDocument.java b/sonar-server/src/main/java/org/sonar/server/rule/ActiveRuleDocument.java index 9e8ca6aa0f3..523a4249ae6 100644 --- a/sonar-server/src/main/java/org/sonar/server/rule/ActiveRuleDocument.java +++ b/sonar-server/src/main/java/org/sonar/server/rule/ActiveRuleDocument.java @@ -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"; diff --git a/sonar-server/src/main/java/org/sonar/server/rule/RuleDocument.java b/sonar-server/src/main/java/org/sonar/server/rule/RuleDocument.java index 8d77cc061b1..e19b489cec4 100644 --- a/sonar-server/src/main/java/org/sonar/server/rule/RuleDocument.java +++ b/sonar-server/src/main/java/org/sonar/server/rule/RuleDocument.java @@ -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"; diff --git a/sonar-server/src/main/java/org/sonar/server/rule/RuleRegistry.java b/sonar-server/src/main/java/org/sonar/server/rule/RuleRegistry.java index aba815f509e..3c9f4695273 100644 --- a/sonar-server/src/main/java/org/sonar/server/rule/RuleRegistry.java +++ b/sonar-server/src/main/java/org/sonar/server/rule/RuleRegistry.java @@ -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()); diff --git a/sonar-server/src/main/resources/org/sonar/server/es/config/mappings/active_rule_mapping.json b/sonar-server/src/main/resources/org/sonar/server/es/config/mappings/active_rule_mapping.json index 8fef621f2e6..b471e1c88e7 100644 --- a/sonar-server/src/main/resources/org/sonar/server/es/config/mappings/active_rule_mapping.json +++ b/sonar-server/src/main/resources/org/sonar/server/es/config/mappings/active_rule_mapping.json @@ -19,7 +19,7 @@ "type": "integer", "index": "not_analyzed" }, - "parentId": { + "activeRuleParentId": { "type": "integer", "index": "not_analyzed" }, diff --git a/sonar-server/src/main/resources/org/sonar/server/es/config/mappings/rule_mapping.json b/sonar-server/src/main/resources/org/sonar/server/es/config/mappings/rule_mapping.json index 564dfc1044a..53f87b54e6d 100644 --- a/sonar-server/src/main/resources/org/sonar/server/es/config/mappings/rule_mapping.json +++ b/sonar-server/src/main/resources/org/sonar/server/es/config/mappings/rule_mapping.json @@ -27,7 +27,7 @@ "type": "string", "index": "no" }, - "parentKey": { + "templateId": { "type": "string", "index": "not_analyzed" }, diff --git a/sonar-server/src/main/webapp/WEB-INF/app/controllers/rules_configuration_controller.rb b/sonar-server/src/main/webapp/WEB-INF/app/controllers/rules_configuration_controller.rb index 74ab0442349..1f482f46e6a 100644 --- a/sonar-server/src/main/webapp/WEB-INF/app/controllers/rules_configuration_controller.rb +++ b/sonar-server/src/main/webapp/WEB-INF/app/controllers/rules_configuration_controller.rb @@ -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 diff --git a/sonar-server/src/main/webapp/WEB-INF/app/views/rules_configuration/_rule.html.erb b/sonar-server/src/main/webapp/WEB-INF/app/views/rules_configuration/_rule.html.erb index 9b118572919..5611800f023 100644 --- a/sonar-server/src/main/webapp/WEB-INF/app/views/rules_configuration/_rule.html.erb +++ b/sonar-server/src/main/webapp/WEB-INF/app/views/rules_configuration/_rule.html.erb @@ -55,14 +55,14 @@ %> <% + 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 %>
- <%= 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 %> Overrides parent definition <%= message('rules_configuration.original_severity') -%> diff --git a/sonar-server/src/test/java/org/sonar/server/qualityprofile/QProfilesTest.java b/sonar-server/src/test/java/org/sonar/server/qualityprofile/QProfilesTest.java index 6314f457079..ab6346e2d87 100644 --- a/sonar-server/src/test/java/org/sonar/server/qualityprofile/QProfilesTest.java +++ b/sonar-server/src/test/java/org/sonar/server/qualityprofile/QProfilesTest.java @@ -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()); diff --git a/sonar-server/src/test/java/org/sonar/server/rule/ProfileRulesTest.java b/sonar-server/src/test/java/org/sonar/server/rule/ProfileRulesTest.java index 89bbb167541..60ff732ac69 100644 --- a/sonar-server/src/test/java/org/sonar/server/rule/ProfileRulesTest.java +++ b/sonar-server/src/test/java/org/sonar/server/rule/ProfileRulesTest.java @@ -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 index 00000000000..c38e4a347f9 --- /dev/null +++ b/sonar-server/src/test/resources/org/sonar/server/rule/ProfileRulesTest/find_active_rules_with_inheritance/active_rule25.json @@ -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 index 00000000000..fb70c854fde --- /dev/null +++ b/sonar-server/src/test/resources/org/sonar/server/rule/ProfileRulesTest/find_active_rules_with_inheritance/active_rule391.json @@ -0,0 +1,7 @@ +{ + "id": 391, + "severity": "MINOR", + "profileId": 2, + "inheritance": "INHERITED", + "activeRuleParentId": 25 +}