]> source.dussan.org Git - sonarqube.git/commitdiff
SONAR-5007 keep error messages when bulk activating rules
authorSimon Brandhof <simon.brandhof@sonarsource.com>
Thu, 12 Jun 2014 22:38:15 +0000 (00:38 +0200)
committerSimon Brandhof <simon.brandhof@sonarsource.com>
Thu, 12 Jun 2014 22:38:15 +0000 (00:38 +0200)
21 files changed:
sonar-core/src/main/java/org/sonar/core/log/Loggable.java
sonar-server/src/main/java/org/sonar/server/exceptions/Message.java [new file with mode: 0644]
sonar-server/src/main/java/org/sonar/server/exceptions/Messages.java [new file with mode: 0644]
sonar-server/src/main/java/org/sonar/server/issue/IssueService.java
sonar-server/src/main/java/org/sonar/server/qualityprofile/ActiveRuleChange.java
sonar-server/src/main/java/org/sonar/server/qualityprofile/BulkActivationResult.java [deleted file]
sonar-server/src/main/java/org/sonar/server/qualityprofile/BulkChangeResult.java [new file with mode: 0644]
sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileBackuper.java
sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileReset.java
sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileService.java
sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivator.java
sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivatorContext.java
sonar-server/src/main/java/org/sonar/server/qualityprofile/ws/BulkRuleActivationActions.java
sonar-server/src/main/java/org/sonar/server/rule/RegisterRules.java
sonar-server/src/main/java/org/sonar/server/rule/Rule.java
sonar-server/src/main/java/org/sonar/server/search/BaseIndex.java
sonar-server/src/test/java/org/sonar/server/log/LogBackendMediumTest.java
sonar-server/src/test/java/org/sonar/server/log/db/LogDaoTest.java
sonar-server/src/test/java/org/sonar/server/log/db/TestLog.java [deleted file]
sonar-server/src/test/java/org/sonar/server/qualityprofile/RuleActivatorMediumTest.java
sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/QProfilesWsMediumTest.java

index 16597a57be7a16afa75728c3d61d9b89f3173a09..e8ad9e0be816f9e1c25575be99046ec05afa2b3c 100644 (file)
@@ -28,6 +28,6 @@ public interface Loggable {
 
   Map<String, String> 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 (file)
index 0000000..5a33261
--- /dev/null
@@ -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<String, Object> 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<String, Object> getParams() {
+    return params;
+  }
+
+  public Message setParams(Map<String, Object> 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 (file)
index 0000000..89a68ca
--- /dev/null
@@ -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<Message.Level, Message> messagesByLevel = ArrayListMultimap.create();
+
+  public Messages add(Message message) {
+    messagesByLevel.put(message.getLevel(), message);
+    return this;
+  }
+
+  public Collection<Message> all() {
+    return messagesByLevel.values();
+  }
+
+  public List<Message> 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<Message> messages = messagesByLevel.get(level);
+    if (!messages.isEmpty()) {
+      json.name(label).beginArray();
+      for (Message message : messages) {
+        json.beginObject().prop("msg", message.getLabel()).endObject();
+      }
+      json.endArray();
+    }
+  }
+}
index 5c97fc8f07030c19d390a3d85dd69b9a0e2f1e72..3d2dded5fbf772cd55e781c4efd3baafaee1705a 100644 (file)
@@ -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);
index 893a6009f6508af87e3eb58683dc281d01e38871..685e7a7c1c435b700cf45c776bf27fbb4fdb617c 100644 (file)
@@ -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<String, String> 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<String,String> m) {
+  public ActiveRuleChange setParameters(Map<String, String> m) {
     parameters.clear();
     parameters.putAll(m);
     return this;
@@ -112,22 +94,20 @@ public class ActiveRuleChange implements Loggable {
 
   @Override
   public Map<String, String> getDetails() {
-    ImmutableMap.Builder<String, String> details = ImmutableMap.<String, String>builder();
-
-    if (this.getType() != null) {
-      details.put("type", this.getType().name());
+    ImmutableMap.Builder<String, String> 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/BulkActivationResult.java
deleted file mode 100644 (file)
index a5f933d..0000000
+++ /dev/null
@@ -1,29 +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.qualityprofile;
-
-import com.google.common.collect.Lists;
-
-import java.util.List;
-
-public class BulkActivationResult {
-
-  private final List<String> errors = Lists.newArrayList();
-}
diff --git a/sonar-server/src/main/java/org/sonar/server/qualityprofile/BulkChangeResult.java b/sonar-server/src/main/java/org/sonar/server/qualityprofile/BulkChangeResult.java
new file mode 100644 (file)
index 0000000..e44abb4
--- /dev/null
@@ -0,0 +1,63 @@
+/*
+ * 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.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 BulkChangeResult {
+
+  private final Messages messages = new Messages();
+  private int succeeded = 0, failed = 0;
+  private final List<ActiveRuleChange> 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<ActiveRuleChange> c) {
+    this.changes.addAll(c);
+  }
+}
index 0f5a972797b8c1cbb40d68aea0c591cb98c115eb..e5af3f3bf54ed34175c1c8a54abada5eb6d5f2ca 100644 (file)
@@ -102,7 +102,7 @@ public class QProfileBackuper implements ServerComponent {
    * @param profileKey the target profile. If <code>null</code>, 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<RuleActivation> 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);
     }
index b7302c3031140e7460ac08b9bf8f2eaebd784d02..ac11fe27dc56e7e39683acbc3bd7c639e135405f 100644 (file)
@@ -94,7 +94,8 @@ public class QProfileReset implements ServerComponent {
   /**
    * Create the profile if needed.
    */
-  void reset(QualityProfileKey profileKey, Collection<RuleActivation> activations) {
+  BulkChangeResult reset(QualityProfileKey profileKey, Collection<RuleActivation> activations) {
+    BulkChangeResult result = new BulkChangeResult();
     Set<RuleKey> 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<ActiveRuleChange> 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();
index cd6ae02fbb98a32276302f207a2c7ecfd9fa24ff..a07ac3892592e4cbbd07ea5b8605d7fb5c52087c 100644 (file)
@@ -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<String, String> 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<String, String> bulkDeactivate(RuleQuery ruleQuery, QualityProfileKey profile) {
+  public BulkChangeResult bulkDeactivate(RuleQuery ruleQuery, QualityProfileKey profile) {
     verifyAdminPermission();
     return ruleActivator.bulkDeactivate(ruleQuery, profile);
   }
index 9d3b5eaa9a51d541cb4c7da4897bc2a50bd27031..c0d31a0198e15e16605fdc4541cd78c9e417e297 100644 (file)
 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<ActiveRuleChange> deactivate(ActiveRuleKey key) {
     DbSession dbSession = db.openSession(false);
     try {
-      return deactivate(dbSession, key);
-    } finally {
+      List<ActiveRuleChange> 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<ActiveRuleChange> 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<String, String> 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<String, String> 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<Rule> rules = result.scroll();
+      RuleResult ruleSearchResult = ruleIndex.search(ruleQuery, new QueryOptions().setScroll(true));
+      Iterator<Rule> 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<ActiveRuleChange> 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<String, String> bulkDeactivate(RuleQuery ruleQuery, QualityProfileKey profile) {
-    RuleIndex ruleIndex = index.get(RuleIndex.class);
-    Multimap<String, String> 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<Rule> rules = result.scroll();
+      RuleIndex ruleIndex = index.get(RuleIndex.class);
+      BulkChangeResult result = new BulkChangeResult();
+      RuleResult ruleSearchResult = ruleIndex.search(ruleQuery, new QueryOptions().setScroll(true));
+      Iterator<Rule> 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<ActiveRuleChange> 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()));
     }
index 0cf00d4f41b80d86b0d1d07a58e76d779467917e..1ab2b1ef8c356bca48a2480798e3b817651329d7 100644 (file)
@@ -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())) {
index 37a0cd5d791e0c0fee28d674200d04a85f279c36..06e5be6b99181a9e77f34ca6fcf4fd539ac0b067 100644 (file)
@@ -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<String, String> 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<String, String> results = profileService.bulkDeactivate(
+    BulkChangeResult result = profileService.bulkDeactivate(
       SearchAction.createRuleQuery(ruleService.newRuleQuery(), request),
       readKey(request));
-    writeResponse(results, response);
+    writeResponse(result, response);
   }
 
-  private void writeResponse(Multimap<String, String> 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();
   }
 
index 8a3f3793be7e9d9f1cf9c6498074846036fd3917..5e759e1f974a75dc6885df749c45961145552a0c 100644 (file)
@@ -119,7 +119,7 @@ public class RegisterRules implements Startable {
         session.commit();
       }
       List<RuleDto> 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<RuleDto> removedRules, RulesDefinition.Context context) {
+  private void removeActiveRulesOnStillExistingRepositories(DbSession session, Collection<RuleDto> removedRules, RulesDefinition.Context context) {
     List<String> repositoryKeys = newArrayList(Iterables.transform(context.repositories(), new Function<RulesDefinition.Repository, String>() {
         @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);
       }
     }
   }
index cb3008783d3fe6277df4a562dad9ecbe147bfa04..c918f8c3cda5951d858b79cdd8e77a7f2d2135ca 100644 (file)
@@ -33,8 +33,6 @@ import java.util.List;
  */
 public interface Rule {
 
-  public static final String MANUAL_REPOSITORY_KEY = "manual";
-
   RuleKey key();
 
   String language();
index 0fcf9d30d576aeae5eeb185b601b02eac7dd1ff6..aa05a8bda4433dd9e9bac483095d245bafaf93af 100644 (file)
@@ -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<DOMAIN, DTO extends Dto<KEY>, 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);
         }
index 55a1b187dec86fb2cf22c47920791d978b376a46..055d5ef65efb415a0138d2556323e0e5039e17d8 100644 (file)
@@ -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
+}
index 846dd27decb88012f654f7272f0780ec211186b4..37b6d974c10eb2582346cb119d94b3639b89ea51 100644 (file)
@@ -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 (file)
index 5e96729..0000000
+++ /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<String, String> getDetails() {
-    return null;
-  }
-
-  @Override
-  public Integer getExecutionTime() {
-    return null;
-  }
-}
\ No newline at end of file
index ff8ec304c9f18403850279627220167235932d88..74a3d835be72acc799710d670c5ce5daa61edae5 100644 (file)
@@ -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<ActiveRuleChange> 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<ActiveRuleChange> 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.<String, String>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"
index 216ca56cd4082e5fc56245e7eb03dc3c14e3c3e6..dbe449c8da3a782c962e99ef03a148be74c241bd 100644 (file)
@@ -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");