From 09b33b23a4d08ba107572c8b2e8ca30302e41db1 Mon Sep 17 00:00:00 2001
From: Simon Brandhof <simon.brandhof@sonarsource.com>
Date: Tue, 1 Jul 2014 22:13:28 +0200
Subject: [PATCH] Fix some quality flaws

---
 .../core/plugins/PluginClassloaders.java      | 19 ++++++----
 .../measure/persistence/MeasureDao.java       |  6 ++--
 .../server/qualityprofile/RuleActivator.java  | 13 ++++---
 .../qualityprofile/db/ActiveRuleDao.java      | 36 ++++++++++++-------
 .../org/sonar/server/rule/RuleUpdater.java    | 17 +++++++--
 5 files changed, 61 insertions(+), 30 deletions(-)

diff --git a/sonar-core/src/main/java/org/sonar/core/plugins/PluginClassloaders.java b/sonar-core/src/main/java/org/sonar/core/plugins/PluginClassloaders.java
index 976505dc3c4..d00f8242c95 100644
--- a/sonar-core/src/main/java/org/sonar/core/plugins/PluginClassloaders.java
+++ b/sonar-core/src/main/java/org/sonar/core/plugins/PluginClassloaders.java
@@ -131,10 +131,11 @@ public class PluginClassloaders {
       }
       return realm;
     } catch (UnsupportedClassVersionError e) {
-      throw new SonarException("The plugin " + plugin.getKey() + " is not supported with Java " + SystemUtils.JAVA_VERSION_TRIMMED, e);
+      throw new SonarException(String.format("The plugin %s is not supported with Java %s", plugin.getKey(),
+        SystemUtils.JAVA_VERSION_TRIMMED), e);
 
     } catch (Exception e) {
-      throw new SonarException("Fail to build the classloader of " + plugin.getKey(), e);
+      throw new SonarException(String.format("Fail to build the classloader of %s", plugin.getKey()), e);
     }
   }
 
@@ -146,7 +147,8 @@ public class PluginClassloaders {
       ClassRealm base = world.getRealm(plugin.getBasePlugin());
       if (base == null) {
         // Ignored, because base plugin is not installed
-        LOG.warn("Plugin " + plugin.getKey() + " is ignored because base plugin is not installed: " + plugin.getBasePlugin());
+        LOG.warn(String.format("Plugin %s is ignored because base plugin is not installed: %s",
+          plugin.getKey(), plugin.getBasePlugin()));
         return false;
       }
       // we create new realm to be able to return it by key without conversion to baseKey
@@ -156,10 +158,12 @@ public class PluginClassloaders {
       }
       return true;
     } catch (UnsupportedClassVersionError e) {
-      throw new SonarException("The plugin " + plugin.getKey() + " is not supported with Java " + SystemUtils.JAVA_VERSION_TRIMMED, e);
+      throw new SonarException(String.format("The plugin %s is not supported with Java %s",
+        plugin.getKey(), SystemUtils.JAVA_VERSION_TRIMMED), e);
 
     } catch (Exception e) {
-      throw new SonarException("Fail to extend the plugin " + plugin.getBasePlugin() + " for " + plugin.getKey(), e);
+      throw new SonarException(String.format("Fail to extend the plugin %s for %s",
+        plugin.getBasePlugin(), plugin.getKey()), e);
     }
   }
 
@@ -223,10 +227,11 @@ public class PluginClassloaders {
       return (Plugin) clazz.newInstance();
 
     } catch (UnsupportedClassVersionError e) {
-      throw new SonarException("The plugin " + plugin.getKey() + " is not supported with Java " + SystemUtils.JAVA_VERSION_TRIMMED, e);
+      throw new SonarException(String.format("The plugin %s is not supported with Java %s",
+        plugin.getKey(), SystemUtils.JAVA_VERSION_TRIMMED), e);
 
     } catch (Exception e) {
-      throw new SonarException("Fail to load plugin " + plugin.getKey(), e);
+      throw new SonarException(String.format("Fail to load plugin %s", plugin.getKey()), e);
     }
   }
 
diff --git a/sonar-server/src/main/java/org/sonar/server/measure/persistence/MeasureDao.java b/sonar-server/src/main/java/org/sonar/server/measure/persistence/MeasureDao.java
index ef7663576ef..7fbed57dc0f 100644
--- a/sonar-server/src/main/java/org/sonar/server/measure/persistence/MeasureDao.java
+++ b/sonar-server/src/main/java/org/sonar/server/measure/persistence/MeasureDao.java
@@ -57,7 +57,7 @@ public class MeasureDao extends BaseDao<MeasureMapper, MeasureDto, MeasureKey> i
     return session.getMapper(MeasureMapper.class).countByKey(key) > 0;
   }
 
-  public List<MeasureDto> findByComponentKeyAndMetricKeys(String componentKey, List<String> metricKeys, DbSession session){
+  public List<MeasureDto> findByComponentKeyAndMetricKeys(String componentKey, List<String> metricKeys, DbSession session) {
     if (metricKeys.isEmpty()) {
       return Collections.emptyList();
     }
@@ -71,7 +71,7 @@ public class MeasureDao extends BaseDao<MeasureMapper, MeasureDto, MeasureKey> i
 
   @Override
   protected MeasureDto doInsert(DbSession session, MeasureDto item) {
-   throw notImplemented();
+    throw notImplemented();
   }
 
   @Override
@@ -89,7 +89,7 @@ public class MeasureDao extends BaseDao<MeasureMapper, MeasureDto, MeasureKey> i
     throw notImplemented();
   }
 
-  private static RuntimeException notImplemented(){
+  private static RuntimeException notImplemented() {
     throw new IllegalStateException("Not implemented yet");
   }
 }
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 394001c452a..9c6e935c196 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
@@ -247,9 +247,13 @@ public class RuleActivator implements ServerComponent {
     ActiveRuleDto activeRule;
     ActiveRuleDao dao = db.activeRuleDao();
     activeRule = ActiveRuleDto.createFor(context.profile(), context.rule());
-    activeRule.setSeverity(change.getSeverity());
-    if (change.getInheritance() != null) {
-      activeRule.setInheritance(change.getInheritance().name());
+    String severity = change.getSeverity();
+    if (severity != null) {
+      activeRule.setSeverity(severity);
+    }
+    ActiveRule.Inheritance inheritance = change.getInheritance();
+    if (inheritance != null) {
+      activeRule.setInheritance(inheritance.name());
     }
     dao.insert(dbSession, activeRule);
     for (Map.Entry<String, String> param : change.getParameters().entrySet()) {
@@ -263,9 +267,8 @@ public class RuleActivator implements ServerComponent {
   }
 
   private ActiveRuleDto doUpdate(ActiveRuleChange change, RuleActivatorContext context, DbSession dbSession) {
-    ActiveRuleDto activeRule;
     ActiveRuleDao dao = db.activeRuleDao();
-    activeRule = context.activeRule();
+    ActiveRuleDto activeRule = context.activeRule();
     String severity = change.getSeverity();
     if (severity != null) {
       activeRule.setSeverity(severity);
diff --git a/sonar-server/src/main/java/org/sonar/server/qualityprofile/db/ActiveRuleDao.java b/sonar-server/src/main/java/org/sonar/server/qualityprofile/db/ActiveRuleDao.java
index b8c49eae83d..a79a79a863b 100644
--- a/sonar-server/src/main/java/org/sonar/server/qualityprofile/db/ActiveRuleDao.java
+++ b/sonar-server/src/main/java/org/sonar/server/qualityprofile/db/ActiveRuleDao.java
@@ -32,6 +32,7 @@ import org.sonar.core.qualityprofile.db.ActiveRuleKey;
 import org.sonar.core.qualityprofile.db.ActiveRuleMapper;
 import org.sonar.core.qualityprofile.db.ActiveRuleParamDto;
 import org.sonar.core.qualityprofile.db.QualityProfileDao;
+import org.sonar.core.qualityprofile.db.QualityProfileDto;
 import org.sonar.core.rule.RuleDto;
 import org.sonar.server.db.BaseDao;
 import org.sonar.server.rule.db.RuleDao;
@@ -95,13 +96,15 @@ public class ActiveRuleDao extends BaseDao<ActiveRuleMapper, ActiveRuleDto, Acti
   @Deprecated
   public ActiveRuleDto getById(DbSession session, int activeRuleId) {
     ActiveRuleDto activeRule = mapper(session).selectById(activeRuleId);
-
     if (activeRule != null) {
-      activeRule.setKey(ActiveRuleKey.of(
-        profileDao.getById(activeRule.getProfileId(), session).getKey(),
-        ruleDao.getById(session, activeRule.getRulId()).getKey()));
+      QualityProfileDto profile = profileDao.getById(activeRule.getProfileId(), session);
+      RuleDto rule = ruleDao.getById(session, activeRule.getRulId());
+      if (profile != null && rule != null) {
+        activeRule.setKey(ActiveRuleKey.of(profile.getKey(), rule.getKey()));
+        return activeRule;
+      }
     }
-    return activeRule;
+    return null;
   }
 
   @Override
@@ -129,9 +132,11 @@ public class ActiveRuleDao extends BaseDao<ActiveRuleMapper, ActiveRuleDto, Acti
 
   @Override
   protected void doDeleteByKey(DbSession session, ActiveRuleKey key) {
-    ActiveRuleDto rule = this.getNullableByKey(session, key);
-    mapper(session).deleteParameters(rule.getId());
-    mapper(session).delete(rule.getId());
+    ActiveRuleDto activeRule = getNullableByKey(session, key);
+    if (activeRule != null) {
+      mapper(session).deleteParameters(activeRule.getId());
+      mapper(session).delete(activeRule.getId());
+    }
   }
 
   /**
@@ -169,9 +174,12 @@ public class ActiveRuleDao extends BaseDao<ActiveRuleMapper, ActiveRuleDto, Acti
   public void removeParamByKeyAndName(DbSession session, ActiveRuleKey key, String param) {
     //TODO SQL rewrite to delete by key
     ActiveRuleDto activeRule = getNullableByKey(session, key);
-    ActiveRuleParamDto activeRuleParam = mapper(session).selectParamByActiveRuleAndKey(activeRule.getId(), param);
-    Preconditions.checkNotNull(activeRuleParam.getId(), ACTIVE_RULE_PARAM_IS_NOT_PERSISTED);
-    mapper(session).deleteParameter(activeRuleParam.getId());
+    if (activeRule != null) {
+      ActiveRuleParamDto activeRuleParam = mapper(session).selectParamByActiveRuleAndKey(activeRule.getId(), param);
+      if (activeRuleParam != null) {
+        mapper(session).deleteParameter(activeRuleParam.getId());
+      }
+    }
   }
 
   public void updateParam(DbSession session, ActiveRuleDto activeRule, ActiveRuleParamDto activeRuleParam) {
@@ -209,10 +217,14 @@ public class ActiveRuleDao extends BaseDao<ActiveRuleMapper, ActiveRuleDto, Acti
     return mapper(session).selectParamsByActiveRuleId(activeRule.getId());
   }
 
+  @CheckForNull
   public ActiveRuleParamDto getParamByKeyAndName(ActiveRuleKey key, String name, DbSession session) {
     Preconditions.checkNotNull(key, ACTIVE_RULE_KEY_CANNOT_BE_NULL);
     Preconditions.checkNotNull(name, PARAMETER_NAME_CANNOT_BE_NULL);
     ActiveRuleDto activeRule = getNullableByKey(session, key);
-    return mapper(session).selectParamByActiveRuleAndKey(activeRule.getId(), name);
+    if (activeRule!=null) {
+      return mapper(session).selectParamByActiveRuleAndKey(activeRule.getId(), name);
+    }
+    return null;
   }
 }
diff --git a/sonar-server/src/main/java/org/sonar/server/rule/RuleUpdater.java b/sonar-server/src/main/java/org/sonar/server/rule/RuleUpdater.java
index 93fe4fbe9a3..eb3d240b62d 100644
--- a/sonar-server/src/main/java/org/sonar/server/rule/RuleUpdater.java
+++ b/sonar-server/src/main/java/org/sonar/server/rule/RuleUpdater.java
@@ -39,7 +39,11 @@ import org.sonar.core.technicaldebt.db.CharacteristicDto;
 import org.sonar.server.db.DbClient;
 import org.sonar.server.user.UserSession;
 
-import java.util.*;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.Date;
+import java.util.List;
+import java.util.Set;
 
 public class RuleUpdater implements ServerComponent {
 
@@ -51,6 +55,9 @@ public class RuleUpdater implements ServerComponent {
     this.system = system;
   }
 
+  /**
+   * Update manual rules and custom rules (rules instantiated from templates)
+   */
   public boolean update(RuleUpdate update, UserSession userSession) {
     if (update.isEmpty()) {
       return false;
@@ -258,6 +265,10 @@ public class RuleUpdater implements ServerComponent {
     if (update.isChangeParameters() && update.isCustomRule()) {
       RuleDto customRule = context.rule;
       RuleDto templateRule = dbClient.ruleDao().getTemplate(customRule, dbSession);
+      if (templateRule == null) {
+        throw new IllegalStateException(String.format("Template %s of rule %s does not exist",
+          customRule.getTemplateId(), customRule.getKey()));
+      }
       List<RuleParamDto> templateRuleParams = dbClient.ruleDao().findRuleParamsByRuleKey(dbSession, templateRule.getKey());
       List<String> paramKeys = new ArrayList<String>();
 
@@ -280,7 +291,7 @@ public class RuleUpdater implements ServerComponent {
   }
 
   private void deleteOrUpdateParameters(DbSession dbSession, RuleUpdate update, RuleDto customRule, List<String> paramKeys,
-                                   Multimap<RuleDto, ActiveRuleDto> activeRules, Multimap<ActiveRuleDto, ActiveRuleParamDto> activeRuleParams){
+    Multimap<RuleDto, ActiveRuleDto> activeRules, Multimap<ActiveRuleDto, ActiveRuleParamDto> activeRuleParams) {
     for (RuleParamDto ruleParamDto : dbClient.ruleDao().findRuleParamsByRuleKey(dbSession, update.getRuleKey())) {
       String key = ruleParamDto.getName();
       String value = update.parameter(key);
@@ -315,7 +326,7 @@ public class RuleUpdater implements ServerComponent {
   }
 
   private void createNewParameters(DbSession dbSession, RuleUpdate update, RuleDto customRule, List<RuleParamDto> templateRuleParams, List<String> paramKeys,
-                                   Multimap<RuleDto, ActiveRuleDto> activeRules){
+    Multimap<RuleDto, ActiveRuleDto> activeRules) {
     for (RuleParamDto templateRuleParam : templateRuleParams) {
       String key = templateRuleParam.getName();
       if (!paramKeys.contains(key)) {
-- 
2.39.5