]> source.dussan.org Git - sonarqube.git/commitdiff
Fix quality flaws
authorJulien Lancelot <julien.lancelot@sonarsource.com>
Tue, 3 Dec 2013 16:57:54 +0000 (17:57 +0100)
committerJulien Lancelot <julien.lancelot@sonarsource.com>
Tue, 3 Dec 2013 16:57:54 +0000 (17:57 +0100)
15 files changed:
plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/CorePlugin.java
plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/technicaldebt/TechnicalDebtCalculator.java [deleted file]
plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/technicaldebt/TechnicalDebtCalculatorTest.java [deleted file]
sonar-batch/src/main/java/org/sonar/batch/index/MeasurePersister.java
sonar-batch/src/main/java/org/sonar/batch/technicaldebt/TechnicalDebtModelLoader.java
sonar-core/src/main/java/org/sonar/core/technicaldebt/TechnicalDebtModelSynchronizer.java
sonar-core/src/main/java/org/sonar/core/technicaldebt/TechnicalDebtRuleCache.java
sonar-core/src/main/java/org/sonar/core/technicaldebt/TechnicalDebtXMLImporter.java
sonar-core/src/test/java/org/sonar/core/technicaldebt/TechnicalDebtRuleCacheTest.java
sonar-plugin-api/src/main/java/org/sonar/api/measures/MeasuresFilters.java
sonar-plugin-api/src/main/java/org/sonar/api/utils/WorkUnit.java
sonar-plugin-api/src/test/java/org/sonar/api/utils/WorkUnitTest.java [new file with mode: 0644]
sonar-server/src/main/java/org/sonar/server/user/UserSession.java
sonar-server/src/test/java/org/sonar/server/group/InternalGroupMembershipQueryServiceTest.java
sonar-ws-client/src/main/java/org/sonar/wsclient/services/ResourceQuery.java

index e266df1057506949b4a12eddafffc7a22cd0e2a6..9d69cb3247aedff42abc5f76b21d1587e2a902fc 100644 (file)
@@ -43,7 +43,6 @@ import org.sonar.plugins.core.notifications.alerts.NewAlerts;
 import org.sonar.plugins.core.security.ApplyProjectRolesDecorator;
 import org.sonar.plugins.core.sensors.*;
 import org.sonar.plugins.core.technicaldebt.NewTechnicalDebtDecorator;
-import org.sonar.plugins.core.technicaldebt.TechnicalDebtCalculator;
 import org.sonar.plugins.core.technicaldebt.TechnicalDebtDecorator;
 import org.sonar.plugins.core.timemachine.*;
 import org.sonar.plugins.core.web.TestsViewer;
@@ -288,7 +287,6 @@ public final class CorePlugin extends SonarPlugin {
 
       // technical debt
       TechnicalDebtConverter.class,
-      TechnicalDebtCalculator.class,
       TechnicalDebtDecorator.class,
       NewTechnicalDebtDecorator.class,
 
@@ -389,7 +387,7 @@ public final class CorePlugin extends SonarPlugin {
         .type(PropertyType.BOOLEAN)
         .category(CoreProperties.CATEGORY_SECURITY)
         .build()
-      );
+    );
   }
 
 }
diff --git a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/technicaldebt/TechnicalDebtCalculator.java b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/technicaldebt/TechnicalDebtCalculator.java
deleted file mode 100644 (file)
index 5e11c13..0000000
+++ /dev/null
@@ -1,63 +0,0 @@
-/*
- * SonarQube, open source software quality management tool.
- * Copyright (C) 2008-2013 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.plugins.core.technicaldebt;
-
-import com.google.common.base.Objects;
-import org.sonar.api.BatchExtension;
-import org.sonar.api.issue.Issue;
-import org.sonar.api.issue.internal.WorkDayDuration;
-import org.sonar.api.technicaldebt.batch.Requirement;
-import org.sonar.api.technicaldebt.batch.TechnicalDebtModel;
-import org.sonar.api.utils.WorkUnit;
-import org.sonar.core.technicaldebt.TechnicalDebtConverter;
-
-/**
- * Computes the remediation cost based on the quality and analysis models.
- */
-public class TechnicalDebtCalculator implements BatchExtension {
-
-  private final TechnicalDebtConverter converter;
-  private TechnicalDebtModel model;
-
-  public TechnicalDebtCalculator(TechnicalDebtModel model, TechnicalDebtConverter converter) {
-    this.model = model;
-    this.converter = converter;
-  }
-
-  public WorkDayDuration calculTechnicalDebt(Issue issue) {
-    Requirement requirement = model.requirementsByRule(issue.ruleKey());
-    if (requirement != null) {
-      return converter.fromMinutes(calculTechnicalDebt(requirement, issue));
-    }
-    return null;
-  }
-
-  private long calculTechnicalDebt(Requirement requirement, Issue issue) {
-    long effortToFix = Objects.firstNonNull(issue.effortToFix(), 1L).longValue();
-
-    WorkUnit factorUnit = requirement.factor();
-    long factor = factorUnit != null ? converter.toMinutes(factorUnit) : 0L;
-
-    WorkUnit offsetUnit = requirement.offset();
-    long offset = offsetUnit != null ? converter.toMinutes(offsetUnit) : 0L;
-
-    return effortToFix * factor + offset;
-  }
-}
diff --git a/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/technicaldebt/TechnicalDebtCalculatorTest.java b/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/technicaldebt/TechnicalDebtCalculatorTest.java
deleted file mode 100644 (file)
index 18e0a68..0000000
+++ /dev/null
@@ -1,131 +0,0 @@
-/*
- * SonarQube, open source software quality management tool.
- * Copyright (C) 2008-2013 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.plugins.core.technicaldebt;
-
-import org.junit.Before;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.mockito.Mock;
-import org.mockito.Mockito;
-import org.mockito.runners.MockitoJUnitRunner;
-import org.sonar.api.issue.internal.DefaultIssue;
-import org.sonar.api.rule.RuleKey;
-import org.sonar.api.technicaldebt.batch.TechnicalDebtModel;
-import org.sonar.api.technicaldebt.batch.internal.DefaultRequirement;
-import org.sonar.api.utils.WorkUnit;
-import org.sonar.core.technicaldebt.TechnicalDebtConverter;
-
-import static org.fest.assertions.Assertions.assertThat;
-import static org.mockito.Mockito.*;
-
-@RunWith(MockitoJUnitRunner.class)
-public class TechnicalDebtCalculatorTest {
-
-  @Mock
-  TechnicalDebtModel model;
-
-  @Mock
-  TechnicalDebtConverter converter;
-
-  WorkUnit tenMinutes = WorkUnit.create(10d, WorkUnit.MINUTES);
-  WorkUnit fiveMinutes = WorkUnit.create(5d, WorkUnit.MINUTES);
-
-  TechnicalDebtCalculator remediationCostCalculator;
-
-  @Before
-  public void before() {
-    when(converter.toMinutes(tenMinutes)).thenReturn(10l);
-    when(converter.toMinutes(fiveMinutes)).thenReturn(5l);
-
-    remediationCostCalculator = new TechnicalDebtCalculator(model, converter);
-  }
-
-  @Test
-  public void calcul_technical_debt() throws Exception {
-    RuleKey ruleKey = RuleKey.of("squid", "AvoidCycle");
-    DefaultIssue issue = new DefaultIssue().setKey("ABCDE").setRuleKey(ruleKey);
-
-    DefaultRequirement requirement = mock(DefaultRequirement.class);
-    Mockito.when(requirement.factor()).thenReturn(tenMinutes);
-    Mockito.when(requirement.offset()).thenReturn(fiveMinutes);
-    when(model.requirementsByRule(ruleKey)).thenReturn(requirement);
-
-    remediationCostCalculator.calculTechnicalDebt(issue);
-
-    verify(converter).fromMinutes(10l + 5l);
-  }
-
-  @Test
-  public void calcul_technical_debt_with_effort_to_fix() throws Exception {
-    RuleKey ruleKey = RuleKey.of("squid", "AvoidCycle");
-    DefaultIssue issue = new DefaultIssue().setKey("ABCDE").setRuleKey(ruleKey).setEffortToFix(2d);
-
-    DefaultRequirement requirement = mock(DefaultRequirement.class);
-    Mockito.when(requirement.factor()).thenReturn(tenMinutes);
-    Mockito.when(requirement.offset()).thenReturn(fiveMinutes);
-    when(model.requirementsByRule(ruleKey)).thenReturn(requirement);
-
-    remediationCostCalculator.calculTechnicalDebt(issue);
-
-    verify(converter).fromMinutes(10l * 2 + 5l);
-  }
-
-  @Test
-  public void calcul_technical_debt_with_no_offset() throws Exception {
-    RuleKey ruleKey = RuleKey.of("squid", "AvoidCycle");
-    DefaultIssue issue = new DefaultIssue().setKey("ABCDE").setRuleKey(ruleKey).setEffortToFix(2d);
-
-    DefaultRequirement requirement = mock(DefaultRequirement.class);
-    Mockito.when(requirement.factor()).thenReturn(tenMinutes);
-    Mockito.when(requirement.offset()).thenReturn(null);
-    when(model.requirementsByRule(ruleKey)).thenReturn(requirement);
-
-    remediationCostCalculator.calculTechnicalDebt(issue);
-
-    verify(converter).fromMinutes(10l * 2 + 0l);
-  }
-
-  @Test
-  public void calcul_technical_debt_with_no_factor() throws Exception {
-    RuleKey ruleKey = RuleKey.of("squid", "AvoidCycle");
-    DefaultIssue issue = new DefaultIssue().setKey("ABCDE").setRuleKey(ruleKey).setEffortToFix(2d);
-
-    DefaultRequirement requirement = mock(DefaultRequirement.class);
-    Mockito.when(requirement.factor()).thenReturn(null);
-    Mockito.when(requirement.offset()).thenReturn(fiveMinutes);
-    when(model.requirementsByRule(ruleKey)).thenReturn(requirement);
-
-    remediationCostCalculator.calculTechnicalDebt(issue);
-
-    verify(converter).fromMinutes(0l * 2 + 5l);
-  }
-
-  @Test
-  public void no_technical_debt_if_requirement_not_found() throws Exception {
-    RuleKey ruleKey = RuleKey.of("squid", "AvoidCycle");
-    DefaultIssue issue = new DefaultIssue().setKey("ABCDE").setRuleKey(ruleKey);
-    when(model.requirementsByRule(ruleKey)).thenReturn(null);
-
-    assertThat(remediationCostCalculator.calculTechnicalDebt(issue)).isNull();
-    verify(converter, never()).fromMinutes(anyLong());
-  }
-
-}
-
index fe17bf369e573de1fc371e3768df821272ea5a47..c0590c7f8e30b51f8ed0c12858b0a821ebb2ed05 100644 (file)
@@ -34,6 +34,8 @@ import org.sonar.api.resources.Resource;
 import org.sonar.api.resources.ResourceUtils;
 import org.sonar.api.rules.Rule;
 import org.sonar.api.rules.RuleFinder;
+import org.sonar.api.technicaldebt.batch.Characteristic;
+import org.sonar.api.technicaldebt.batch.Requirement;
 import org.sonar.api.utils.SonarException;
 import org.sonar.core.persistence.MyBatis;
 
@@ -152,22 +154,26 @@ public final class MeasurePersister {
     model.setVariationValue4(measure.getVariation4());
     model.setVariationValue5(measure.getVariation5());
     model.setUrl(measure.getUrl());
-    if (measure.getCharacteristic() != null) {
-      model.setCharacteristicId(measure.getCharacteristic().id());
-    } else if (measure.getRequirement() != null) {
-      model.setCharacteristicId(measure.getRequirement().id());
+    Characteristic characteristic = measure.getCharacteristic();
+    Requirement requirement = measure.getRequirement();
+    if (characteristic != null) {
+      model.setCharacteristicId(characteristic.id());
+    } else if (requirement != null) {
+      model.setCharacteristicId(requirement.id());
     }
     model.setPersonId(measure.getPersonId());
-    if (measure.getValue() != null) {
-      model.setValue(measure.getValue().doubleValue());
+    Double value = measure.getValue();
+    if (value != null) {
+      model.setValue(value.doubleValue());
     } else {
       model.setValue(null);
     }
     if (measure instanceof RuleMeasure) {
       RuleMeasure ruleMeasure = (RuleMeasure) measure;
       model.setRulePriority(ruleMeasure.getSeverity());
-      if (ruleMeasure.getRule() != null) {
-        Rule ruleWithId = ruleFinder.findByKey(ruleMeasure.getRule().getRepositoryKey(), ruleMeasure.getRule().getKey());
+      Rule rule = ruleMeasure.getRule();
+      if (rule != null) {
+        Rule ruleWithId = ruleFinder.findByKey(rule.getRepositoryKey(), rule.getKey());
         if (ruleWithId == null) {
           throw new SonarException("Can not save a measure with unknown rule " + ruleMeasure);
         }
index ba46f1805af52b1647e3a041196f2ab6788f32f6..cf11256bac94fc978da8396825d25fa0391be3fe 100644 (file)
@@ -54,7 +54,7 @@ public class TechnicalDebtModelLoader implements BatchComponent {
 
     addRootCharacteristics(model, dtos, characteristicsById);
     addCharacteristics(dtos, characteristicsById);
-    addRequirements(model, dtos, characteristicsById);
+    addRequirements(dtos, characteristicsById);
     return model;
   }
 
@@ -78,7 +78,7 @@ public class TechnicalDebtModelLoader implements BatchComponent {
     }
   }
 
-  private void addRequirements(DefaultTechnicalDebtModel model, List<CharacteristicDto> dtos, Map<Integer, DefaultCharacteristic> characteristicsById) {
+  private void addRequirements(List<CharacteristicDto> dtos, Map<Integer, DefaultCharacteristic> characteristicsById) {
     Map<Integer, Rule> rulesById = rulesById(ruleFinder.findAll(RuleQuery.create()));
     for (CharacteristicDto dto : dtos) {
       Integer ruleId = dto.getRuleId();
index 251caee0286ab9a0c9535d82cc9755d2ea5b3bc9..23a9f83967ed905ba990244e1d633d4b26dd5d63 100644 (file)
@@ -108,7 +108,8 @@ public class TechnicalDebtModelSynchronizer implements ServerExtension {
     return characteristics;
   }
 
-  private void mergePlugins(List<CharacteristicDto> existingModel, DefaultTechnicalDebtModel defaultModel, ValidationMessages messages, TechnicalDebtRuleCache rulesCache, SqlSession session) {
+  private void mergePlugins(List<CharacteristicDto> existingModel, DefaultTechnicalDebtModel defaultModel, ValidationMessages messages, TechnicalDebtRuleCache rulesCache,
+                            SqlSession session) {
     for (String pluginKey : getContributingPluginListWithoutSqale()) {
       DefaultTechnicalDebtModel pluginModel = loadModelFromXml(pluginKey, messages, rulesCache);
       checkPluginDoNotAddNewCharacteristic(pluginModel, defaultModel);
@@ -116,7 +117,8 @@ public class TechnicalDebtModelSynchronizer implements ServerExtension {
     }
   }
 
-  private void mergePlugin(DefaultTechnicalDebtModel pluginModel, List<CharacteristicDto> existingModel, ValidationMessages messages, TechnicalDebtRuleCache rulesCache, SqlSession session) {
+  private void mergePlugin(DefaultTechnicalDebtModel pluginModel, List<CharacteristicDto> existingModel, ValidationMessages messages, TechnicalDebtRuleCache rulesCache,
+                           SqlSession session) {
     if (!messages.hasErrors()) {
       for (DefaultRequirement pluginRequirement : pluginModel.requirements()) {
         Rule rule = rulesCache.getByRuleKey(pluginRequirement.ruleKey());
@@ -171,7 +173,7 @@ public class TechnicalDebtModelSynchronizer implements ServerExtension {
     return Iterables.find(existingModel, new Predicate<CharacteristicDto>() {
       @Override
       public boolean apply(CharacteristicDto input) {
-        return input.getRuleId() == null && input.getKey().equals(key);
+        return input.getRuleId() == null && input.getKey() != null && input.getKey().equals(key);
       }
     });
   }
index e435fe6a02254faacd26cc34665f0ac90464e095..d5a098a2e457601883c51607d8db5406e41d5563 100644 (file)
@@ -42,12 +42,6 @@ public class TechnicalDebtRuleCache {
     this.ruleFinder = ruleFinder;
   }
 
-  @CheckForNull
-  public Rule getRule(String repository, String ruleKey) {
-    initRules();
-    return lookUpRuleInCache(repository, ruleKey);
-  }
-
   @CheckForNull
   public Rule getByRuleKey(RuleKey ruleKey) {
     initRules();
@@ -60,10 +54,6 @@ public class TechnicalDebtRuleCache {
     return cachedRulesId.get(ruleId);
   }
 
-  public boolean exists(Rule rule) {
-    return getRule(rule.getRepositoryKey(), rule.getKey()) != null;
-  }
-
   public boolean exists(Integer ruleId) {
     initRules();
     return getByRuleId(ruleId) != null;
index ebffcd1fbdc74d477a8d68365d288df883a796c7..33ea98e5b7d37919f99f5a848bf6fce945ca1083 100644 (file)
@@ -124,12 +124,7 @@ public class TechnicalDebtXMLImporter implements ServerExtension {
       } else if (StringUtils.equals(node, REPOSITORY_KEY)) {
         DefaultRequirement requirement = processRequirement(cursor, messages, technicalDebtRuleCache);
         if (requirement != null) {
-          if (parent.parent() == null) {
-            messages.addWarningText("Requirement '" + requirement.ruleKey()  + "' is ignored because it's defined directly under a root characteristic.");
-          } else {
-            requirement.setCharacteristic(parent);
-            requirement.setRootCharacteristic(parent.parent());
-          }
+          addRequirement(requirement, parent, messages);
         }
       }
     }
@@ -142,6 +137,16 @@ public class TechnicalDebtXMLImporter implements ServerExtension {
     return null;
   }
 
+  private void addRequirement(DefaultRequirement requirement, DefaultCharacteristic parent, ValidationMessages messages){
+    DefaultCharacteristic root = parent.parent();
+    if (root == null) {
+      messages.addWarningText("Requirement '" + requirement.ruleKey()  + "' is ignored because it's defined directly under a root characteristic.");
+    } else {
+      requirement.setCharacteristic(parent);
+      requirement.setRootCharacteristic(root);
+    }
+  }
+
   private DefaultRequirement processRequirement(SMInputCursor cursor, ValidationMessages messages, TechnicalDebtRuleCache technicalDebtRuleCache)
     throws XMLStreamException {
 
index bd13002c77141b0a8fcee80b7c5d7aa6fc62139c..7185f7910c1261ac3e14617b6181a2d90329a0a4 100644 (file)
@@ -24,6 +24,7 @@ import org.fest.assertions.Assertions;
 import org.junit.Test;
 import org.mockito.Matchers;
 import org.mockito.Mockito;
+import org.sonar.api.rule.RuleKey;
 import org.sonar.api.rules.Rule;
 import org.sonar.api.rules.RuleFinder;
 import org.sonar.api.rules.RuleQuery;
@@ -39,8 +40,8 @@ public class TechnicalDebtRuleCacheTest {
     Mockito.when(ruleFinder.findAll(Matchers.any(RuleQuery.class))).thenReturn(Collections.EMPTY_LIST);
 
     TechnicalDebtRuleCache technicalDebtRuleCache = new TechnicalDebtRuleCache(ruleFinder);
-    technicalDebtRuleCache.getRule("", "");
-    technicalDebtRuleCache.getRule("", "");
+    technicalDebtRuleCache.getByRuleKey(RuleKey.of("repo1", "rule1"));
+    technicalDebtRuleCache.getByRuleKey(RuleKey.of("repo1", "rule1"));
 
     Mockito.verify(ruleFinder, Mockito.times(1)).findAll(Matchers.any(RuleQuery.class));
   }
@@ -55,8 +56,8 @@ public class TechnicalDebtRuleCacheTest {
     Mockito.when(ruleFinder.findAll(Matchers.any(RuleQuery.class))).thenReturn(Lists.newArrayList(rule1, rule2));
 
     TechnicalDebtRuleCache technicalDebtRuleCache = new TechnicalDebtRuleCache(ruleFinder);
-    Rule actualRule1 = technicalDebtRuleCache.getRule("repo1", "rule1");
-    Rule actualRule2 = technicalDebtRuleCache.getRule("repo2", "rule2");
+    Rule actualRule1 = technicalDebtRuleCache.getByRuleKey(RuleKey.of("repo1", "rule1"));
+    Rule actualRule2 = technicalDebtRuleCache.getByRuleKey(RuleKey.of("repo2", "rule2"));
 
     Assertions.assertThat(actualRule1).isEqualTo(rule1);
     Assertions.assertThat(actualRule2).isEqualTo(rule2);
@@ -66,14 +67,28 @@ public class TechnicalDebtRuleCacheTest {
   public void return_if_rule_exists() throws Exception {
 
     Rule rule1 = Rule.create("repo1", "rule1");
-    Rule rule2 = Rule.create("repo2", "rule2");
 
     RuleFinder ruleFinder = Mockito.mock(RuleFinder.class);
     Mockito.when(ruleFinder.findAll(Matchers.any(RuleQuery.class))).thenReturn(Lists.newArrayList(rule1));
 
     TechnicalDebtRuleCache technicalDebtRuleCache = new TechnicalDebtRuleCache(ruleFinder);
 
-    Assertions.assertThat(technicalDebtRuleCache.exists(rule1)).isTrue();
-    Assertions.assertThat(technicalDebtRuleCache.exists(rule2)).isFalse();
+    Assertions.assertThat(technicalDebtRuleCache.exists(RuleKey.of("repo1", "rule1"))).isTrue();
+    Assertions.assertThat(technicalDebtRuleCache.exists(RuleKey.of("repo2", "rule2"))).isFalse();
+  }
+
+  @Test
+  public void return_if_rule_id_exists() throws Exception {
+
+    Rule rule1 = Rule.create("repo1", "rule1");
+    rule1.setId(1);
+
+    RuleFinder ruleFinder = Mockito.mock(RuleFinder.class);
+    Mockito.when(ruleFinder.findAll(Matchers.any(RuleQuery.class))).thenReturn(Lists.newArrayList(rule1));
+
+    TechnicalDebtRuleCache technicalDebtRuleCache = new TechnicalDebtRuleCache(ruleFinder);
+
+    Assertions.assertThat(technicalDebtRuleCache.exists(1)).isTrue();
+    Assertions.assertThat(technicalDebtRuleCache.exists(2)).isFalse();
   }
 }
index d42f3c329f3e0d7480c5c19305196513bc0e1dde..510806b5c459be7bd412c0084fd4aaafdf3693a6 100644 (file)
@@ -76,12 +76,10 @@ public final class MeasuresFilters {
           return null;
         }
         for (Measure measure : measures) {
-          Characteristic measureCharacteristic = measure.getCharacteristic();
           if (measure.getClass().equals(Measure.class) &&
             measure.getMetric().equals(metric) &&
             measure.getPersonId() == null &&
-            measureCharacteristic != null &&
-            measureCharacteristic.equals(characteristic)) {
+            isSameCharacteristic(measure, characteristic)) {
             return measure;
           }
         }
@@ -90,6 +88,12 @@ public final class MeasuresFilters {
     };
   }
 
+  private static boolean isSameCharacteristic(Measure measure, final Characteristic characteristic){
+    Characteristic measureCharacteristic = measure.getCharacteristic();
+    return measureCharacteristic != null &&
+      measureCharacteristic.equals(characteristic);
+  }
+
   public static MeasuresFilter<Measure> requirement(final Metric metric, final Requirement requirement) {
     return new MetricFilter<Measure>(metric) {
 
@@ -101,8 +105,7 @@ public final class MeasuresFilters {
           if (measure.getClass().equals(Measure.class) &&
             measure.getMetric().equals(metric) &&
             measure.getPersonId() == null &&
-            measure.getRequirement() != null &&
-            measure.getRequirement().equals(requirement)) {
+            isSameRequirement(measure, requirement)) {
             return measure;
           }
         }
@@ -111,6 +114,12 @@ public final class MeasuresFilters {
     };
   }
 
+  private static boolean isSameRequirement(Measure measure, final Requirement requirement){
+    Requirement measureRequirement = measure.getRequirement();
+    return measureRequirement != null &&
+      measureRequirement.equals(requirement);
+  }
+
   /**
    * @since 2.0
    */
index 417666ce2828bfe5fd38261f985c2f26209152c8..61a5aeec2a7f4c9356210a0099dd483dadd761cc 100644 (file)
@@ -53,15 +53,15 @@ public final class WorkUnit {
   }
 
   public static WorkUnit create(@Nullable Double value, @Nullable String unit) {
-    unit = StringUtils.defaultIfEmpty(unit, DEFAULT_UNIT);
-    if (!ArrayUtils.contains(UNITS, unit)) {
-      throw new IllegalArgumentException("Unit can not be: " + unit + ". Possible values are " + ArrayUtils.toString(UNITS));
+    String defaultIfEmptyUnit = StringUtils.defaultIfEmpty(unit, DEFAULT_UNIT);
+    if (!ArrayUtils.contains(UNITS, defaultIfEmptyUnit)) {
+      throw new IllegalArgumentException("Unit can not be: " + defaultIfEmptyUnit + ". Possible values are " + ArrayUtils.toString(UNITS));
     }
     double d = value != null ? value : DEFAULT_VALUE;
     if (d < 0.0) {
       throw new IllegalArgumentException("Value can not be negative: " + d);
     }
-    return new WorkUnit(d, unit);
+    return new WorkUnit(d, defaultIfEmptyUnit);
   }
 
   public static WorkUnit create() {
diff --git a/sonar-plugin-api/src/test/java/org/sonar/api/utils/WorkUnitTest.java b/sonar-plugin-api/src/test/java/org/sonar/api/utils/WorkUnitTest.java
new file mode 100644 (file)
index 0000000..108f7aa
--- /dev/null
@@ -0,0 +1,68 @@
+/*
+ * SonarQube, open source software quality management tool.
+ * Copyright (C) 2008-2013 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.api.utils;
+
+import org.junit.Test;
+
+import static org.fest.assertions.Assertions.assertThat;
+
+public class WorkUnitTest {
+
+  @Test
+  public void create() throws Exception {
+    WorkUnit workUnit = WorkUnit.create(2.0, "mn");
+    assertThat(workUnit.getUnit()).isEqualTo("mn");
+    assertThat(workUnit.getValue()).isEqualTo(2.0);
+  }
+
+  @Test
+  public void create_default() throws Exception {
+    WorkUnit workUnit = WorkUnit.create();
+    assertThat(workUnit.getUnit()).isEqualTo("d");
+    assertThat(workUnit.getValue()).isEqualTo(0.0);
+  }
+
+  @Test
+  public void test_equals() throws Exception {
+    assertThat(WorkUnit.create(2.0, "mn")).isEqualTo(WorkUnit.create(2.0, "mn"));
+    assertThat(WorkUnit.create(3.0, "mn")).isNotEqualTo(WorkUnit.create(2.0, "mn"));
+    assertThat(WorkUnit.create(2.0, "h")).isNotEqualTo(WorkUnit.create(2.0, "mn"));
+  }
+
+  @Test
+  public void fail_with_bad_unit() throws Exception {
+    try {
+      WorkUnit.create(2.0, "z");
+    } catch (Exception e) {
+      assertThat(e).isInstanceOf(IllegalArgumentException.class);
+    }
+  }
+
+  @Test
+  public void fail_with_bad_value() throws Exception {
+    try {
+      WorkUnit.create(-2.0, "mn");
+    } catch (Exception e) {
+      assertThat(e).isInstanceOf(IllegalArgumentException.class);
+    }
+  }
+
+}
index 580ef13536210d459d671176da2b59ef69550197..b28e5d65fbdfea93558e5714f8b8419558d66d40 100644 (file)
@@ -139,7 +139,7 @@ public class UserSession {
    */
   public UserSession checkProjectPermission(String projectPermission, String projectKey) {
     if (!hasProjectPermission(projectPermission, projectKey)) {
-      throw new ForbiddenException("Insufficient privileges");
+      throw new ForbiddenException(INSUFFICIENT_PRIVILEGES_MESSAGE);
     }
     return this;
   }
index 18e5b0647783c5ba97a8aa81784a9ec28ebfd38c..49ab0d3c17bc077384fa5823a6f34531a62add5b 100644 (file)
@@ -32,6 +32,7 @@ import org.sonar.server.exceptions.NotFoundException;
 import java.util.List;
 
 import static org.fest.assertions.Assertions.assertThat;
+import static org.fest.assertions.Fail.fail;
 
 /**
  * Use Test with DB because there's no IT on this feature for the moment
@@ -91,6 +92,7 @@ public class InternalGroupMembershipQueryServiceTest extends AbstractDaoTestCase
       service.find(ImmutableMap.of(
         "user", "user_not_existing",
         "selected", "all"));
+      fail();
     } catch (Exception e) {
       assertThat(e).isInstanceOf(NotFoundException.class).hasMessage("User 'user_not_existing' does not exists.");
     }
index 162c485ca651e975fbe336ec81a06d260e61ab4a..087b5a4ad15a7e71e910d1e40effea78c9bcd6cb 100644 (file)
@@ -20,6 +20,7 @@
 package org.sonar.wsclient.services;
 
 public class ResourceQuery extends Query<Resource> {
+
   public static final String BASE_URL = "/api/resources";
 
   public static final int DEPTH_UNLIMITED = -1;
@@ -77,15 +78,6 @@ public class ResourceQuery extends Query<Resource> {
     return this;
   }
 
-  /**
-   * @see #setCharacteristics
-   */
-  @Deprecated
-  public ResourceQuery setCharacteristicKeys(String model, String... keys) {
-    this.characteristicKeys = keys;
-    return this;
-  }
-
   public ResourceQuery setCharacteristics(String... keys) {
     this.characteristicKeys = keys;
     return this;