From 5603bc48ca5209c93542bc51229b774c066fb346 Mon Sep 17 00:00:00 2001 From: Simon Brandhof Date: Fri, 13 Jun 2014 00:38:15 +0200 Subject: [PATCH] SONAR-5007 keep error messages when bulk activating rules --- .../java/org/sonar/core/log/Loggable.java | 2 +- .../org/sonar/server/exceptions/Message.java | 112 ++++++++++++++++++ .../org/sonar/server/exceptions/Messages.java | 66 +++++++++++ .../org/sonar/server/issue/IssueService.java | 9 +- .../qualityprofile/ActiveRuleChange.java | 42 ++----- ...ationResult.java => BulkChangeResult.java} | 38 +++++- .../qualityprofile/QProfileBackuper.java | 4 +- .../server/qualityprofile/QProfileReset.java | 18 ++- .../qualityprofile/QProfileService.java | 5 +- .../server/qualityprofile/RuleActivator.java | 82 +++++-------- .../qualityprofile/RuleActivatorContext.java | 6 +- .../ws/BulkRuleActivationActions.java | 25 ++-- .../org/sonar/server/rule/RegisterRules.java | 6 +- .../main/java/org/sonar/server/rule/Rule.java | 2 - .../org/sonar/server/search/BaseIndex.java | 5 +- .../server/log/LogBackendMediumTest.java | 4 +- .../org/sonar/server/log/db/LogDaoTest.java | 4 +- .../java/org/sonar/server/log/db/TestLog.java | 50 -------- .../RuleActivatorMediumTest.java | 54 +++++++-- .../ws/QProfilesWsMediumTest.java | 25 ---- 20 files changed, 341 insertions(+), 218 deletions(-) create mode 100644 sonar-server/src/main/java/org/sonar/server/exceptions/Message.java create mode 100644 sonar-server/src/main/java/org/sonar/server/exceptions/Messages.java rename sonar-server/src/main/java/org/sonar/server/qualityprofile/{BulkActivationResult.java => BulkChangeResult.java} (56%) delete mode 100644 sonar-server/src/test/java/org/sonar/server/log/db/TestLog.java diff --git a/sonar-core/src/main/java/org/sonar/core/log/Loggable.java b/sonar-core/src/main/java/org/sonar/core/log/Loggable.java index 16597a57be7..e8ad9e0be81 100644 --- a/sonar-core/src/main/java/org/sonar/core/log/Loggable.java +++ b/sonar-core/src/main/java/org/sonar/core/log/Loggable.java @@ -28,6 +28,6 @@ public interface Loggable { Map getDetails(); - Integer getExecutionTime(); + int getExecutionTime(); } diff --git a/sonar-server/src/main/java/org/sonar/server/exceptions/Message.java b/sonar-server/src/main/java/org/sonar/server/exceptions/Message.java new file mode 100644 index 00000000000..5a332610ba5 --- /dev/null +++ b/sonar-server/src/main/java/org/sonar/server/exceptions/Message.java @@ -0,0 +1,112 @@ +/* + * SonarQube, open source software quality management tool. + * Copyright (C) 2008-2014 SonarSource + * mailto:contact AT sonarsource DOT com + * + * SonarQube is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * SonarQube is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package org.sonar.server.exceptions; + +import com.google.common.base.Objects; + +import javax.annotation.CheckForNull; +import javax.annotation.Nullable; + +import java.util.Map; + +public class Message { + + public static enum Level { + INFO, WARNING, ERROR + } + + private final String code; + private String label; + private Object[] l10nParams = new Object[0]; + private Level level; + private Map params; + + private Message(String code) { + this.code = code; + } + + public String getCode() { + return code; + } +@CheckForNull + public String getLabel() { + return label; + } + + public Message setLabel(String s) { + this.label = s; + return this; + } + + public Object[] getL10nParams() { + return l10nParams; + } + + public Message setL10nParams(Object[] l10nParams) { + this.l10nParams = l10nParams; + return this; + } + + public Level getLevel() { + return level; + } + + public Message setLevel(Level level) { + this.level = level; + return this; + } + + public Map getParams() { + return params; + } + + public Message setParams(Map params) { + this.params = params; + return this; + } + + public Message setParam(String key, @Nullable Object value) { + this.params.put(key, value); + return this; + } + + public static Message newError(String code) { + return new Message(code).setLevel(Level.ERROR); + } + + public static Message newWarning(String code) { + return new Message(code).setLevel(Level.WARNING); + } + + public static Message newInfo(String code) { + return new Message(code).setLevel(Level.INFO); + } + + @Override + public String toString() { + return Objects.toStringHelper(this) + .add("code", code) + .add("label", label) + .add("l10nParams", l10nParams) + .add("level", level) + .add("params", params) + .toString(); + } +} diff --git a/sonar-server/src/main/java/org/sonar/server/exceptions/Messages.java b/sonar-server/src/main/java/org/sonar/server/exceptions/Messages.java new file mode 100644 index 00000000000..89a68ca5292 --- /dev/null +++ b/sonar-server/src/main/java/org/sonar/server/exceptions/Messages.java @@ -0,0 +1,66 @@ +/* + * SonarQube, open source software quality management tool. + * Copyright (C) 2008-2014 SonarSource + * mailto:contact AT sonarsource DOT com + * + * SonarQube is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * SonarQube is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package org.sonar.server.exceptions; + +import com.google.common.collect.ArrayListMultimap; +import com.google.common.collect.ListMultimap; +import org.sonar.api.utils.text.JsonWriter; + +import java.util.Collection; +import java.util.List; + +public class Messages { + + private ListMultimap messagesByLevel = ArrayListMultimap.create(); + + public Messages add(Message message) { + messagesByLevel.put(message.getLevel(), message); + return this; + } + + public Collection all() { + return messagesByLevel.values(); + } + + public List getByLevel(Message.Level level) { + return messagesByLevel.get(level); + } + + public boolean hasLevel(Message.Level level) { + return messagesByLevel.containsKey(level); + } + + public void writeJson(JsonWriter json) { + writeJson(json, "errors", Message.Level.ERROR); + writeJson(json, "warnings", Message.Level.WARNING); + writeJson(json, "infos", Message.Level.INFO); + } + + private void writeJson(JsonWriter json, String label, Message.Level level) { + List messages = messagesByLevel.get(level); + if (!messages.isEmpty()) { + json.name(label).beginArray(); + for (Message message : messages) { + json.beginObject().prop("msg", message.getLabel()).endObject(); + } + json.endArray(); + } + } +} diff --git a/sonar-server/src/main/java/org/sonar/server/issue/IssueService.java b/sonar-server/src/main/java/org/sonar/server/issue/IssueService.java index 5c97fc8f070..3d2dded5fbf 100644 --- a/sonar-server/src/main/java/org/sonar/server/issue/IssueService.java +++ b/sonar-server/src/main/java/org/sonar/server/issue/IssueService.java @@ -56,8 +56,11 @@ import org.sonar.server.issue.actionplan.ActionPlanService; import org.sonar.server.user.UserSession; import javax.annotation.Nullable; - -import java.util.*; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.Date; +import java.util.List; /** * @since 3.6 @@ -228,7 +231,7 @@ public class IssueService implements ServerComponent { // TODO throw unauthorized throw new IllegalStateException("User does not have the required role"); } - if (!org.sonar.server.rule.Rule.MANUAL_REPOSITORY_KEY.equals(ruleKey.repository())) { + if (!ruleKey.isManual()) { throw new IllegalArgumentException("Issues can be created only on rules marked as 'manual': " + ruleKey); } Rule rule = findRule(ruleKey); diff --git a/sonar-server/src/main/java/org/sonar/server/qualityprofile/ActiveRuleChange.java b/sonar-server/src/main/java/org/sonar/server/qualityprofile/ActiveRuleChange.java index 893a6009f65..685e7a7c1c4 100644 --- a/sonar-server/src/main/java/org/sonar/server/qualityprofile/ActiveRuleChange.java +++ b/sonar-server/src/main/java/org/sonar/server/qualityprofile/ActiveRuleChange.java @@ -36,9 +36,8 @@ public class ActiveRuleChange implements Loggable { private final Type type; private final ActiveRuleKey key; - private boolean inheritedChange = false; - private String previousSeverity = null, severity = null; - private ActiveRule.Inheritance previousInheritance = null, inheritance = null; + private String severity = null; + private ActiveRule.Inheritance inheritance = null; private Map parameters = Maps.newHashMap(); private long start; @@ -57,23 +56,6 @@ public class ActiveRuleChange implements Loggable { return type; } - public boolean isInheritedChange() { - return inheritedChange; - } - - public void setInheritedChange(boolean b) { - this.inheritedChange = b; - } - - @CheckForNull - public String getPreviousSeverity() { - return previousSeverity; - } - - public void setPreviousSeverity(@Nullable String s) { - this.previousSeverity = s; - } - @CheckForNull public String getSeverity() { return severity; @@ -104,7 +86,7 @@ public class ActiveRuleChange implements Loggable { return this; } - public ActiveRuleChange setParameters(Map m) { + public ActiveRuleChange setParameters(Map m) { parameters.clear(); parameters.putAll(m); return this; @@ -112,22 +94,20 @@ public class ActiveRuleChange implements Loggable { @Override public Map getDetails() { - ImmutableMap.Builder details = ImmutableMap.builder(); - - if (this.getType() != null) { - details.put("type", this.getType().name()); + ImmutableMap.Builder details = ImmutableMap.builder(); + if (getType() != null) { + details.put("type", getType().name()); } - - if (this.getKey() != null) { - details.put("key", this.getKey().toString()); - details.put("ruleKey", this.getKey().ruleKey().toString()); - details.put("profileKey", this.getKey().qProfile().toString()); + if (getKey() != null) { + details.put("key", getKey().toString()); + details.put("ruleKey", getKey().ruleKey().toString()); + details.put("profileKey", getKey().qProfile().toString()); } return details.build(); } @Override - public Integer getExecutionTime() { + public int getExecutionTime() { return (int) (System.currentTimeMillis() - start); } diff --git a/sonar-server/src/main/java/org/sonar/server/qualityprofile/BulkActivationResult.java b/sonar-server/src/main/java/org/sonar/server/qualityprofile/BulkChangeResult.java similarity index 56% rename from sonar-server/src/main/java/org/sonar/server/qualityprofile/BulkActivationResult.java rename to sonar-server/src/main/java/org/sonar/server/qualityprofile/BulkChangeResult.java index a5f933d41a7..e44abb4a86a 100644 --- a/sonar-server/src/main/java/org/sonar/server/qualityprofile/BulkActivationResult.java +++ b/sonar-server/src/main/java/org/sonar/server/qualityprofile/BulkChangeResult.java @@ -20,10 +20,44 @@ package org.sonar.server.qualityprofile; import com.google.common.collect.Lists; +import org.sonar.server.exceptions.Message; +import org.sonar.server.exceptions.Messages; +import java.util.Collection; import java.util.List; -public class BulkActivationResult { +public class BulkChangeResult { - private final List errors = Lists.newArrayList(); + private final Messages messages = new Messages(); + private int succeeded = 0, failed = 0; + private final List changes = Lists.newArrayList(); + + BulkChangeResult addMessage(Message message) { + this.messages.add(message); + return this; + } + + public Messages getMessages() { + return messages; + } + + public int countSucceeded() { + return succeeded; + } + + public int countFailed() { + return failed; + } + + void incrementSucceeded() { + succeeded++; + } + + void incrementFailed() { + failed++; + } + + void addChanges(Collection c) { + this.changes.addAll(c); + } } diff --git a/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileBackuper.java b/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileBackuper.java index 0f5a972797b..e5af3f3bf54 100644 --- a/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileBackuper.java +++ b/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileBackuper.java @@ -102,7 +102,7 @@ public class QProfileBackuper implements ServerComponent { * @param profileKey the target profile. If null, then use the * key declared in the backup */ - void restore(Reader reader, @Nullable QualityProfileKey profileKey) { + BulkChangeResult restore(Reader reader, @Nullable QualityProfileKey profileKey) { try { String profileLang = null, profileName = null; List ruleActivations = Lists.newArrayList(); @@ -131,7 +131,7 @@ public class QProfileBackuper implements ServerComponent { } targetKey = (QualityProfileKey) ObjectUtils.defaultIfNull(profileKey, QualityProfileKey.of(profileName, profileLang)); - reset.reset(targetKey, ruleActivations); + return reset.reset(targetKey, ruleActivations); } catch (XMLStreamException e) { throw new IllegalStateException("Fail to restore Quality profile backup", e); } diff --git a/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileReset.java b/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileReset.java index b7302c30311..ac11fe27dc5 100644 --- a/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileReset.java +++ b/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileReset.java @@ -94,7 +94,8 @@ public class QProfileReset implements ServerComponent { /** * Create the profile if needed. */ - void reset(QualityProfileKey profileKey, Collection activations) { + BulkChangeResult reset(QualityProfileKey profileKey, Collection activations) { + BulkChangeResult result = new BulkChangeResult(); Set rulesToDeactivate = Sets.newHashSet(); DbSession dbSession = db.openSession(false); try { @@ -114,18 +115,25 @@ public class QProfileReset implements ServerComponent { for (RuleActivation activation : activations) { try { - activator.activate(dbSession, activation); + List changes = activator.activate(dbSession, activation); rulesToDeactivate.remove(activation.getKey().ruleKey()); + result.incrementSucceeded(); + result.addChanges(changes); } catch (BadRequestException e) { - // TODO should return warnings instead of logging warnings - LoggerFactory.getLogger(getClass()).warn(e.getMessage()); + result.incrementFailed(); + //TODOresult.addMessage() } } for (RuleKey ruleKey : rulesToDeactivate) { - activator.deactivate(dbSession, ActiveRuleKey.of(profileKey, ruleKey)); + try { + activator.deactivate(dbSession, ActiveRuleKey.of(profileKey, ruleKey)); + } catch (BadRequestException e) { + // ignore, probably a rule inherited from parent that can't be deactivated + } } dbSession.commit(); + return result; } finally { dbSession.close(); diff --git a/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileService.java b/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileService.java index cd6ae02fbb9..a07ac389259 100644 --- a/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileService.java +++ b/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileService.java @@ -19,7 +19,6 @@ */ package org.sonar.server.qualityprofile; -import com.google.common.collect.Multimap; import org.sonar.api.ServerComponent; import org.sonar.api.rule.RuleKey; import org.sonar.core.permission.GlobalPermissions; @@ -110,12 +109,12 @@ public class QProfileService implements ServerComponent { return ruleActivator.deactivate(key); } - public Multimap bulkActivate(RuleQuery ruleQuery, QualityProfileKey profile, @Nullable String severity) { + public BulkChangeResult bulkActivate(RuleQuery ruleQuery, QualityProfileKey profile, @Nullable String severity) { verifyAdminPermission(); return ruleActivator.bulkActivate(ruleQuery, profile, severity); } - public Multimap bulkDeactivate(RuleQuery ruleQuery, QualityProfileKey profile) { + public BulkChangeResult bulkDeactivate(RuleQuery ruleQuery, QualityProfileKey profile) { verifyAdminPermission(); return ruleActivator.bulkDeactivate(ruleQuery, profile); } diff --git a/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivator.java b/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivator.java index 9d3b5eaa9a5..c0d31a0198e 100644 --- a/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivator.java +++ b/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivator.java @@ -20,10 +20,7 @@ package org.sonar.server.qualityprofile; import com.google.common.base.Splitter; -import com.google.common.collect.ArrayListMultimap; -import com.google.common.collect.ImmutableSet; import com.google.common.collect.Lists; -import com.google.common.collect.Multimap; import org.apache.commons.lang.StringUtils; import org.sonar.api.ServerComponent; import org.sonar.api.server.rule.RuleParamType; @@ -43,7 +40,6 @@ import org.sonar.server.log.LogService; import org.sonar.server.qualityprofile.db.ActiveRuleDao; import org.sonar.server.rule.Rule; import org.sonar.server.rule.index.RuleIndex; -import org.sonar.server.rule.index.RuleNormalizer; import org.sonar.server.rule.index.RuleQuery; import org.sonar.server.rule.index.RuleResult; import org.sonar.server.search.IndexClient; @@ -62,10 +58,6 @@ import static com.google.common.collect.Lists.newArrayList; */ public class RuleActivator implements ServerComponent { - public static final String ACTIVATED = "activated"; - public static final String IGNORED = "ignored"; - public static final String DEACTIVATED = "deactivated"; - private final DbClient db; private final TypeValidations typeValidations; private final RuleActivatorContextFactory contextFactory; @@ -209,7 +201,6 @@ public class RuleActivator implements ServerComponent { } else if (change.getType() == ActiveRuleChange.Type.DEACTIVATED) { dao.deleteByKey(dbSession, change.getKey()); - //activeRule = null; } else if (change.getType() == ActiveRuleChange.Type.UPDATED) { activeRule = context.activeRule(); @@ -237,11 +228,6 @@ public class RuleActivator implements ServerComponent { } } } - for (ActiveRuleParamDto activeRuleParamDto : context.activeRuleParams()) { - if (!change.getParameters().containsKey(activeRuleParamDto.getKey())) { - // TODO delete param - } - } } return activeRule; @@ -254,9 +240,10 @@ public class RuleActivator implements ServerComponent { List deactivate(ActiveRuleKey key) { DbSession dbSession = db.openSession(false); try { - return deactivate(dbSession, key); - } finally { + List changes = deactivate(dbSession, key); dbSession.commit(); + return changes; + } finally { dbSession.close(); } } @@ -268,16 +255,6 @@ public class RuleActivator implements ServerComponent { return deactivate(dbSession, key, false); } - public List deactivate(RuleDto ruleDto) { - DbSession dbSession = db.openSession(false); - try { - return deactivate(dbSession, ruleDto); - } finally { - dbSession.commit(); - dbSession.close(); - } - } - /** * Deactivate a rule on a Quality profile WITHOUT committing db session, WITHOUT checking permissions, and forcing removal of inherited rules */ @@ -340,58 +317,55 @@ public class RuleActivator implements ServerComponent { } } - Multimap bulkActivate(RuleQuery ruleQuery, QualityProfileKey profileKey, @Nullable String severity) { + BulkChangeResult bulkActivate(RuleQuery ruleQuery, QualityProfileKey profileKey, @Nullable String severity) { + BulkChangeResult result = new BulkChangeResult(); RuleIndex ruleIndex = index.get(RuleIndex.class); - Multimap results = ArrayListMultimap.create(); DbSession dbSession = db.openSession(false); - try { - RuleResult result = ruleIndex.search(ruleQuery, - new QueryOptions() - .setScroll(true) - .setFieldsToReturn(ImmutableSet.of(RuleNormalizer.RuleField.IS_TEMPLATE.field()))); - - Iterator rules = result.scroll(); + RuleResult ruleSearchResult = ruleIndex.search(ruleQuery, new QueryOptions().setScroll(true)); + Iterator rules = ruleSearchResult.scroll(); while (rules.hasNext()) { Rule rule = rules.next(); - if (!rule.isTemplate()) { + try { ActiveRuleKey key = ActiveRuleKey.of(profileKey, rule.key()); RuleActivation activation = new RuleActivation(key); activation.setSeverity(severity); - for (ActiveRuleChange active : activate(dbSession, activation)) { - results.put(ACTIVATED, active.getKey().ruleKey().toString()); - } - } else { - results.put(IGNORED, rule.key().toString()); + List changes = activate(dbSession, activation); + result.addChanges(changes); + result.incrementSucceeded(); + + } catch (BadRequestException e) { + // other exceptions stop the bulk activation + result.incrementFailed(); + // TODO result.addMessage } } dbSession.commit(); } finally { dbSession.close(); } - return results; + return result; } - Multimap bulkDeactivate(RuleQuery ruleQuery, QualityProfileKey profile) { - RuleIndex ruleIndex = index.get(RuleIndex.class); - Multimap results = ArrayListMultimap.create(); + BulkChangeResult bulkDeactivate(RuleQuery ruleQuery, QualityProfileKey profile) { DbSession dbSession = db.openSession(false); - try { - RuleResult result = ruleIndex.search(ruleQuery, new QueryOptions().setScroll(true)); - Iterator rules = result.scroll(); + RuleIndex ruleIndex = index.get(RuleIndex.class); + BulkChangeResult result = new BulkChangeResult(); + RuleResult ruleSearchResult = ruleIndex.search(ruleQuery, new QueryOptions().setScroll(true)); + Iterator rules = ruleSearchResult.scroll(); while (rules.hasNext()) { Rule rule = rules.next(); ActiveRuleKey key = ActiveRuleKey.of(profile, rule.key()); - for (ActiveRuleChange deActive : deactivate(dbSession, key)) { - results.put(DEACTIVATED, deActive.getKey().ruleKey().toString()); - } + List changes = deactivate(dbSession, key); + result.addChanges(changes); + result.incrementSucceeded(); } dbSession.commit(); + return result; } finally { dbSession.close(); } - return results; } void setParent(QualityProfileKey key, @Nullable QualityProfileKey parentKey) { @@ -414,7 +388,7 @@ public class RuleActivator implements ServerComponent { } else if (profile.getParentKey() == null || !profile.getParentKey().equals(parentKey)) { QualityProfileDto parentProfile = db.qualityProfileDao().getNonNullByKey(dbSession, parentKey); if (isDescendant(dbSession, profile, parentProfile)) { - throw new BadRequestException("Please do not select a child profile as parent."); + throw new BadRequestException(String.format("Descendant profile '%s' can not be selected as parent of '%s'", parentKey, key)); } removeParent(dbSession, profile); @@ -461,7 +435,7 @@ public class RuleActivator implements ServerComponent { return false; } - private void verifyParametersAreNotSetOnCustomRule(RuleActivatorContext context, RuleActivation activation, ActiveRuleChange change){ + private void verifyParametersAreNotSetOnCustomRule(RuleActivatorContext context, RuleActivation activation, ActiveRuleChange change) { if (!activation.getParameters().isEmpty() && context.rule().getTemplateId() != null) { throw new IllegalStateException(String.format("Parameters cannot be set when activating the custom rule '%s'", activation.getKey().ruleKey())); } diff --git a/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivatorContext.java b/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivatorContext.java index 0cf00d4f41b..1ab2b1ef8c3 100644 --- a/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivatorContext.java +++ b/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivatorContext.java @@ -28,11 +28,9 @@ import org.sonar.core.qualityprofile.db.QualityProfileDto; import org.sonar.core.rule.RuleDto; import org.sonar.core.rule.RuleParamDto; import org.sonar.server.exceptions.BadRequestException; -import org.sonar.server.rule.Rule; import javax.annotation.CheckForNull; import javax.annotation.Nullable; - import java.util.Collection; import java.util.Map; @@ -161,7 +159,7 @@ class RuleActivatorContext { } RuleParamDto ruleParam = ruleParams.get(paramKey); if (ruleParam == null) { - throw new IllegalStateException(String.format("Rule parameter %s does not exist", paramKey)); + throw new BadRequestException(String.format("Rule parameter %s does not exist", paramKey)); } return ruleParam.getDefaultValue(); } @@ -189,7 +187,7 @@ class RuleActivatorContext { if (rule.isTemplate()) { throw new BadRequestException("Rule template can't be activated on a Quality profile: " + rule.getKey()); } - if (Rule.MANUAL_REPOSITORY_KEY.equals(rule.getRepositoryKey())) { + if (rule.getKey().isManual()) { throw new BadRequestException("Manual rule can't be activated on a Quality profile: " + rule.getKey()); } if (!profile.getLanguage().equals(rule.getLanguage())) { diff --git a/sonar-server/src/main/java/org/sonar/server/qualityprofile/ws/BulkRuleActivationActions.java b/sonar-server/src/main/java/org/sonar/server/qualityprofile/ws/BulkRuleActivationActions.java index 37a0cd5d791..06e5be6b991 100644 --- a/sonar-server/src/main/java/org/sonar/server/qualityprofile/ws/BulkRuleActivationActions.java +++ b/sonar-server/src/main/java/org/sonar/server/qualityprofile/ws/BulkRuleActivationActions.java @@ -19,7 +19,6 @@ */ package org.sonar.server.qualityprofile.ws; -import com.google.common.collect.Multimap; import org.sonar.api.ServerComponent; import org.sonar.api.rule.Severity; import org.sonar.api.server.ws.Request; @@ -28,6 +27,7 @@ import org.sonar.api.server.ws.Response; import org.sonar.api.server.ws.WebService; import org.sonar.api.utils.text.JsonWriter; import org.sonar.core.qualityprofile.db.QualityProfileKey; +import org.sonar.server.qualityprofile.BulkChangeResult; import org.sonar.server.qualityprofile.QProfileService; import org.sonar.server.rule.RuleService; import org.sonar.server.rule.ws.SearchAction; @@ -99,32 +99,25 @@ public class BulkRuleActivationActions implements ServerComponent { } private void bulkActivate(Request request, Response response) throws Exception { - Multimap results = profileService.bulkActivate( + BulkChangeResult result = profileService.bulkActivate( SearchAction.createRuleQuery(ruleService.newRuleQuery(), request), readKey(request), request.param(SEVERITY)); - writeResponse(results, response); + writeResponse(result, response); } private void bulkDeactivate(Request request, Response response) throws Exception { - Multimap results = profileService.bulkDeactivate( + BulkChangeResult result = profileService.bulkDeactivate( SearchAction.createRuleQuery(ruleService.newRuleQuery(), request), readKey(request)); - writeResponse(results, response); + writeResponse(result, response); } - private void writeResponse(Multimap results, Response response) { + private void writeResponse(BulkChangeResult result, Response response) { JsonWriter json = response.newJsonWriter().beginObject(); - for (String action : results.keySet()) { - json.name(action).beginArray(); - for (String key : results.get(action)) { - json - .beginObject() - .prop("key", key) - .endObject(); - } - json.endArray(); - } + json.prop("succeeded", result.countSucceeded()); + json.prop("failed", result.countFailed()); + result.getMessages().writeJson(json); json.endObject().close(); } diff --git a/sonar-server/src/main/java/org/sonar/server/rule/RegisterRules.java b/sonar-server/src/main/java/org/sonar/server/rule/RegisterRules.java index 8a3f3793be7..5e759e1f974 100644 --- a/sonar-server/src/main/java/org/sonar/server/rule/RegisterRules.java +++ b/sonar-server/src/main/java/org/sonar/server/rule/RegisterRules.java @@ -119,7 +119,7 @@ public class RegisterRules implements Startable { session.commit(); } List activeRules = processRemainingDbRules(allRules.values(), session); - removeActiveRulesOnStillExistingRepositories(activeRules, context); + removeActiveRulesOnStillExistingRepositories(session, activeRules, context); session.commit(); } finally { @@ -370,7 +370,7 @@ public class RegisterRules implements Startable { * The side effect of this approach is that extended repositories will not be managed the same way. * If an extended repository do not exists anymore, then related active rules will be removed. */ - private void removeActiveRulesOnStillExistingRepositories(Collection removedRules, RulesDefinition.Context context) { + private void removeActiveRulesOnStillExistingRepositories(DbSession session, Collection removedRules, RulesDefinition.Context context) { List repositoryKeys = newArrayList(Iterables.transform(context.repositories(), new Function() { @Override public String apply(RulesDefinition.Repository input) { @@ -382,7 +382,7 @@ public class RegisterRules implements Startable { for (RuleDto rule : removedRules) { // SONAR-4642 Remove active rules only when repository still exists if (repositoryKeys.contains(rule.getRepositoryKey())) { - ruleActivator.deactivate(rule); + ruleActivator.deactivate(session, rule); } } } diff --git a/sonar-server/src/main/java/org/sonar/server/rule/Rule.java b/sonar-server/src/main/java/org/sonar/server/rule/Rule.java index cb3008783d3..c918f8c3cda 100644 --- a/sonar-server/src/main/java/org/sonar/server/rule/Rule.java +++ b/sonar-server/src/main/java/org/sonar/server/rule/Rule.java @@ -33,8 +33,6 @@ import java.util.List; */ public interface Rule { - public static final String MANUAL_REPOSITORY_KEY = "manual"; - RuleKey key(); String language(); diff --git a/sonar-server/src/main/java/org/sonar/server/search/BaseIndex.java b/sonar-server/src/main/java/org/sonar/server/search/BaseIndex.java index 0fcf9d30d57..aa05a8bda44 100644 --- a/sonar-server/src/main/java/org/sonar/server/search/BaseIndex.java +++ b/sonar-server/src/main/java/org/sonar/server/search/BaseIndex.java @@ -49,6 +49,7 @@ import java.io.Serializable; import java.util.ArrayDeque; import java.util.ArrayList; import java.util.Collection; +import java.util.Collections; import java.util.Date; import java.util.HashMap; import java.util.Iterator; @@ -117,9 +118,7 @@ public abstract class BaseIndex, KEY extends Serial try { SearchScrollRequestBuilder esRequest = getClient().prepareSearchScroll(scrollId) .setScroll(TimeValue.timeValueMinutes(3)); - for (SearchHit hit : esRequest.get().getHits().getHits()) { - hits.add(hit); - } + Collections.addAll(hits, esRequest.get().getHits().getHits()); } catch (Exception e) { throw new IllegalStateException("Error while filling in the scroll buffer", e); } diff --git a/sonar-server/src/test/java/org/sonar/server/log/LogBackendMediumTest.java b/sonar-server/src/test/java/org/sonar/server/log/LogBackendMediumTest.java index 55a1b187dec..055d5ef65ef 100644 --- a/sonar-server/src/test/java/org/sonar/server/log/LogBackendMediumTest.java +++ b/sonar-server/src/test/java/org/sonar/server/log/LogBackendMediumTest.java @@ -85,7 +85,7 @@ public class LogBackendMediumTest { } @Override - public Integer getExecutionTime() { + public int getExecutionTime() { return 12; } }); @@ -110,4 +110,4 @@ public class LogBackendMediumTest { dbSession.commit(); assertThat(dao.findAll(dbSession)).hasSize(max); } -} \ No newline at end of file +} diff --git a/sonar-server/src/test/java/org/sonar/server/log/db/LogDaoTest.java b/sonar-server/src/test/java/org/sonar/server/log/db/LogDaoTest.java index 846dd27decb..37b6d974c10 100644 --- a/sonar-server/src/test/java/org/sonar/server/log/db/LogDaoTest.java +++ b/sonar-server/src/test/java/org/sonar/server/log/db/LogDaoTest.java @@ -105,7 +105,7 @@ public class LogDaoTest extends AbstractDaoTestCase { } @Override - public Integer getExecutionTime() { + public int getExecutionTime() { return 12; } }) @@ -123,4 +123,4 @@ public class LogDaoTest extends AbstractDaoTestCase { assertThat(details.get(testKey)).isEqualTo(testValue); } -} \ No newline at end of file +} diff --git a/sonar-server/src/test/java/org/sonar/server/log/db/TestLog.java b/sonar-server/src/test/java/org/sonar/server/log/db/TestLog.java deleted file mode 100644 index 5e96729d32a..00000000000 --- a/sonar-server/src/test/java/org/sonar/server/log/db/TestLog.java +++ /dev/null @@ -1,50 +0,0 @@ -/* - * SonarQube, open source software quality management tool. - * Copyright (C) 2008-2014 SonarSource - * mailto:contact AT sonarsource DOT com - * - * SonarQube is free software; you can redistribute it and/or - * modify it under the terms of the GNU Lesser General Public - * License as published by the Free Software Foundation; either - * version 3 of the License, or (at your option) any later version. - * - * SonarQube is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * Lesser General Public License for more details. - * - * You should have received a copy of the GNU Lesser General Public License - * along with this program; if not, write to the Free Software Foundation, - * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. - */ -package org.sonar.server.log.db; - -import org.sonar.core.log.Loggable; - -import java.util.Map; - -/** - * @since 4.4 - */ -public class TestLog implements Loggable { - - public String test; - - public TestLog() { - } - - public TestLog(String test) { - this.test = test; - } - - - @Override - public Map getDetails() { - return null; - } - - @Override - public Integer getExecutionTime() { - return null; - } -} \ No newline at end of file diff --git a/sonar-server/src/test/java/org/sonar/server/qualityprofile/RuleActivatorMediumTest.java b/sonar-server/src/test/java/org/sonar/server/qualityprofile/RuleActivatorMediumTest.java index ff8ec304c9f..74a3d835be7 100644 --- a/sonar-server/src/test/java/org/sonar/server/qualityprofile/RuleActivatorMediumTest.java +++ b/sonar-server/src/test/java/org/sonar/server/qualityprofile/RuleActivatorMediumTest.java @@ -29,13 +29,16 @@ import org.sonar.api.rule.RuleStatus; import org.sonar.api.rule.Severity; import org.sonar.api.server.rule.RuleParamType; import org.sonar.core.persistence.DbSession; -import org.sonar.core.qualityprofile.db.*; +import org.sonar.core.qualityprofile.db.ActiveRuleDto; +import org.sonar.core.qualityprofile.db.ActiveRuleKey; +import org.sonar.core.qualityprofile.db.ActiveRuleParamDto; +import org.sonar.core.qualityprofile.db.QualityProfileDto; +import org.sonar.core.qualityprofile.db.QualityProfileKey; import org.sonar.core.rule.RuleDto; import org.sonar.core.rule.RuleParamDto; import org.sonar.server.db.DbClient; import org.sonar.server.exceptions.BadRequestException; import org.sonar.server.qualityprofile.index.ActiveRuleIndex; -import org.sonar.server.rule.Rule; import org.sonar.server.rule.RuleTesting; import org.sonar.server.rule.index.RuleIndex; import org.sonar.server.rule.index.RuleQuery; @@ -43,7 +46,6 @@ import org.sonar.server.search.QueryOptions; import org.sonar.server.tester.ServerTester; import javax.annotation.Nullable; - import java.util.Arrays; import java.util.Collections; import java.util.List; @@ -57,9 +59,9 @@ public class RuleActivatorMediumTest { static final QualityProfileKey XOO_PROFILE_KEY = QualityProfileKey.of("P1", "xoo"); static final QualityProfileKey XOO_CHILD_PROFILE_KEY = QualityProfileKey.of("P2", "xoo"); static final QualityProfileKey XOO_GRAND_CHILD_PROFILE_KEY = QualityProfileKey.of("P3", "xoo"); - static final RuleKey MANUAL_RULE_KEY = RuleKey.of(Rule.MANUAL_REPOSITORY_KEY, "m1"); + static final RuleKey MANUAL_RULE_KEY = RuleKey.of(RuleKey.MANUAL_REPOSITORY_KEY, "m1"); static final RuleKey TEMPLATE_RULE_KEY = RuleKey.of("xoo", "template1"); - static final RuleKey CUSTOM_RULE_KEY = RuleKey.of(TEMPLATE_RULE_KEY.repository(), "custom1"); + static final RuleKey CUSTOM_RULE_KEY = RuleKey.of("xoo", "custom1"); static final RuleKey XOO_RULE_1 = RuleKey.of("xoo", "x1"); static final RuleKey XOO_RULE_2 = RuleKey.of("xoo", "x2"); @@ -119,10 +121,12 @@ public class RuleActivatorMediumTest { RuleActivation activation = new RuleActivation(activeRuleKey); activation.setSeverity(Severity.BLOCKER); activation.setParameter("max", "7"); - ruleActivator.activate(activation); + List changes = ruleActivator.activate(activation); assertThat(countActiveRules(XOO_PROFILE_KEY)).isEqualTo(1); verifyHasActiveRule(activeRuleKey, Severity.BLOCKER, null, ImmutableMap.of("max", "7")); + assertThat(changes).hasSize(1); + assertThat(changes.get(0).getType()).isEqualTo(ActiveRuleChange.Type.ACTIVATED); } @Test @@ -158,10 +162,12 @@ public class RuleActivatorMediumTest { RuleActivation update = new RuleActivation(activeRuleKey); update.setSeverity(Severity.CRITICAL); update.setParameter("max", "42"); - ruleActivator.activate(update); + List changes = ruleActivator.activate(update); assertThat(countActiveRules(XOO_PROFILE_KEY)).isEqualTo(1); verifyHasActiveRule(activeRuleKey, Severity.CRITICAL, null, ImmutableMap.of("max", "42")); + assertThat(changes).hasSize(1); + assertThat(changes.get(0).getType()).isEqualTo(ActiveRuleChange.Type.UPDATED); } @Test @@ -701,7 +707,7 @@ public class RuleActivatorMediumTest { } @Test - public void mass_activation() { + public void bulk_activation() { // Generate more rules than the search's max limit int bulkSize = QueryOptions.MAX_LIMIT + 10; for (int i = 0; i < bulkSize; i++) { @@ -716,13 +722,29 @@ public class RuleActivatorMediumTest { .isEqualTo(bulkSize); // 1. bulk activate all the rules - ruleActivator.bulkActivate( + BulkChangeResult result = ruleActivator.bulkActivate( new RuleQuery().setRepositories(Arrays.asList("bulk")), XOO_PROFILE_KEY, "MINOR"); - // 2. assert that all activation has been commited to DB and ES + // 2. assert that all activation has been commit to DB and ES dbSession.clearCache(); assertThat(db.activeRuleDao().findByProfileKey(dbSession, XOO_PROFILE_KEY)).hasSize(bulkSize); assertThat(index.findByProfile(XOO_PROFILE_KEY)).hasSize(bulkSize); + assertThat(result.countSucceeded()).isEqualTo(bulkSize); + assertThat(result.countFailed()).isEqualTo(0); + } + + @Test + public void bulk_activation_ignores_errors() { + // 1. bulk activate all the rules, even non xoo-rules and xoo templates + BulkChangeResult result = ruleActivator.bulkActivate(new RuleQuery(), XOO_PROFILE_KEY, "MINOR"); + + // 2. assert that all activations have been commit to DB and ES + // -> xoo rules x1, x2 and custom1 + dbSession.clearCache(); + assertThat(db.activeRuleDao().findByProfileKey(dbSession, XOO_PROFILE_KEY)).hasSize(3); + assertThat(index.findByProfile(XOO_PROFILE_KEY)).hasSize(3); + assertThat(result.countSucceeded()).isEqualTo(3); + assertThat(result.countFailed()).isGreaterThan(0); } @@ -757,6 +779,18 @@ public class RuleActivatorMediumTest { verifyHasActiveRule(ActiveRuleKey.of(childKey, XOO_RULE_2), Severity.MAJOR, null, Collections.emptyMap()); } + @Test + public void fail_if_set_child_as_parent() { + createChildProfiles(); + + try { + ruleActivator.setParent(XOO_PROFILE_KEY, XOO_GRAND_CHILD_PROFILE_KEY); + fail(); + } catch (BadRequestException e) { + assertThat(e).hasMessage("Descendant profile 'P3:xoo' can not be selected as parent of 'P1:xoo'"); + } + } + @Test public void keep_overridden_rules_when_unsetting_parent() { // x1 is activated on the "future parent" diff --git a/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/QProfilesWsMediumTest.java b/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/QProfilesWsMediumTest.java index 216ca56cd40..dbe449c8da3 100644 --- a/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/QProfilesWsMediumTest.java +++ b/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/QProfilesWsMediumTest.java @@ -281,31 +281,6 @@ public class QProfilesWsMediumTest { assertThat(db.activeRuleDao().findByProfileKey(session, profile.getKey())).hasSize(4); } - @Test - public void bulk_activate_rule_with_template() throws Exception { - QualityProfileDto profile = getProfile("java"); - RuleDto rule0 = getRule(profile.getLanguage(), "toto") - .setIsTemplate(true); - RuleDto rule1 = getRule(profile.getLanguage(), "tata"); - session.commit(); - - // 0. Assert No Active Rule for profile - assertThat(db.activeRuleDao().findByProfileKey(session, profile.getKey())).isEmpty(); - - // 1. Activate Rule - WsTester.TestRequest request = wsTester.newGetRequest(QProfilesWs.API_ENDPOINT, BulkRuleActivationActions.BULK_ACTIVATE_ACTION); - request.setParam(RuleActivationActions.PROFILE_KEY, profile.getKey().toString()); - request.setParam(SearchAction.PARAM_LANGUAGES, "java"); - WsTester.Result result = request.execute(); - session.clearCache(); - - // 2. assert replied ignored list - result.assertJson("{\"ignored\":[{\"key\":\"blah:toto\"}],\"activated\":[{\"key\":\"blah:tata\"}]}"); - - // 3. Assert ActiveRule in DAO - assertThat(db.activeRuleDao().findByProfileKey(session, profile.getKey())).hasSize(1); - } - @Test public void bulk_activate_rule_not_all() throws Exception { QualityProfileDto java = getProfile("java"); -- 2.39.5