From: Julien HENRY Date: Thu, 24 Apr 2014 12:39:35 +0000 (+0200) Subject: SONAR-5189 Fix regression when searching measures in cache X-Git-Tag: 4.4-RC1~1408 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=25d72e70d97e0985b9594dd2dc525758b56d0ef3;p=sonarqube.git SONAR-5189 Fix regression when searching measures in cache --- diff --git a/sonar-batch/src/main/java/org/sonar/batch/index/Cache.java b/sonar-batch/src/main/java/org/sonar/batch/index/Cache.java index 13e238fce5b..39815dcfcb3 100644 --- a/sonar-batch/src/main/java/org/sonar/batch/index/Cache.java +++ b/sonar-batch/src/main/java/org/sonar/batch/index/Cache.java @@ -321,20 +321,6 @@ public class Cache { } } - /** - * Lazy-loading values for a given key - */ - public Iterable allValues(Object key) { - try { - exchange.clear(); - exchange.append(key).append(Key.BEFORE); - Exchange iteratorExchange = new Exchange(exchange); - return new ValueIterable(iteratorExchange, true); - } catch (Exception e) { - throw new IllegalStateException("Fail to get values from cache " + name, e); - } - } - /** * Lazy-loading values */ diff --git a/sonar-batch/src/main/java/org/sonar/batch/index/DefaultIndex.java b/sonar-batch/src/main/java/org/sonar/batch/index/DefaultIndex.java index cea7aa71729..5a9c3148b23 100644 --- a/sonar-batch/src/main/java/org/sonar/batch/index/DefaultIndex.java +++ b/sonar-batch/src/main/java/org/sonar/batch/index/DefaultIndex.java @@ -184,12 +184,7 @@ public class DefaultIndex extends SonarIndex { public M getMeasures(Resource resource, MeasuresFilter filter) { // Reload resource so that effective key is populated Resource indexedResource = getResource(resource); - Iterable unfiltered; - if (filter instanceof MeasuresFilters.MetricFilter) { - unfiltered = measureCache.byMetric(indexedResource, ((MeasuresFilters.MetricFilter) filter).filterOnMetricKey()); - } else { - unfiltered = measureCache.byResource(indexedResource); - } + Iterable unfiltered = measureCache.byResource(indexedResource); return filter.filter(unfiltered); } diff --git a/sonar-batch/src/main/java/org/sonar/batch/scan/measure/MeasureCache.java b/sonar-batch/src/main/java/org/sonar/batch/scan/measure/MeasureCache.java index 1f668614d8f..5068a4734be 100644 --- a/sonar-batch/src/main/java/org/sonar/batch/scan/measure/MeasureCache.java +++ b/sonar-batch/src/main/java/org/sonar/batch/scan/measure/MeasureCache.java @@ -22,6 +22,7 @@ package org.sonar.batch.scan.measure; import com.google.common.base.Preconditions; import org.sonar.api.BatchComponent; import org.sonar.api.measures.Measure; +import org.sonar.api.measures.RuleMeasure; import org.sonar.api.resources.Resource; import org.sonar.batch.index.Cache; import org.sonar.batch.index.Cache.Entry; @@ -43,19 +44,33 @@ public class MeasureCache implements BatchComponent { } public Iterable byResource(Resource r) { - return cache.allValues(r.getEffectiveKey()); + return cache.values(r.getEffectiveKey()); } public MeasureCache put(Resource resource, Measure measure) { Preconditions.checkNotNull(resource.getEffectiveKey()); Preconditions.checkNotNull(measure.getMetricKey()); - cache.put(resource.getEffectiveKey(), measure.getMetricKey(), measure.hashCode(), measure); + cache.put(resource.getEffectiveKey(), computeMeasureKey(measure), measure); return this; } - public Iterable byMetric(Resource resource, String metricKey) { - Preconditions.checkNotNull(resource.getEffectiveKey()); - Preconditions.checkNotNull(metricKey); - return cache.values(resource.getEffectiveKey(), metricKey); + private static String computeMeasureKey(Measure m) { + StringBuilder sb = new StringBuilder(); + if (m.getMetricKey() != null) { + sb.append(m.getMetricKey()); + } + sb.append("|"); + if (m.getCharacteristic() != null) { + sb.append(m.getCharacteristic().key()); + } + sb.append("|"); + if (m.getPersonId() != null) { + sb.append(m.getPersonId()); + } + if (m instanceof RuleMeasure) { + sb.append("|"); + sb.append(((RuleMeasure) m).ruleKey()); + } + return sb.toString(); } } diff --git a/sonar-batch/src/test/java/org/sonar/batch/scan/measure/MeasureCacheTest.java b/sonar-batch/src/test/java/org/sonar/batch/scan/measure/MeasureCacheTest.java index a1791ff676d..0b1db5e1d75 100644 --- a/sonar-batch/src/test/java/org/sonar/batch/scan/measure/MeasureCacheTest.java +++ b/sonar-batch/src/test/java/org/sonar/batch/scan/measure/MeasureCacheTest.java @@ -27,11 +27,19 @@ import org.junit.rules.TemporaryFolder; import org.sonar.api.measures.CoreMetrics; import org.sonar.api.measures.Measure; import org.sonar.api.measures.RuleMeasure; +import org.sonar.api.resources.Directory; +import org.sonar.api.resources.File; import org.sonar.api.resources.Project; +import org.sonar.api.resources.Resource; +import org.sonar.api.rule.RuleKey; import org.sonar.api.rules.RulePriority; +import org.sonar.api.technicaldebt.batch.internal.DefaultCharacteristic; +import org.sonar.batch.index.Cache.Entry; import org.sonar.batch.index.Caches; import org.sonar.batch.index.CachesTest; +import java.util.Iterator; + import static org.fest.assertions.Assertions.assertThat; public class MeasureCacheTest { @@ -60,15 +68,17 @@ public class MeasureCacheTest { assertThat(cache.entries()).hasSize(0); assertThat(cache.byResource(p)).hasSize(0); - assertThat(cache.byMetric(p, "ncloc")).hasSize(0); Measure m = new Measure(CoreMetrics.NCLOC, 1.0); cache.put(p, m); assertThat(cache.entries()).hasSize(1); + Iterator> iterator = cache.entries().iterator(); + iterator.hasNext(); + Entry next = iterator.next(); + assertThat(next.value()).isEqualTo(m); + assertThat(next.key()[0]).isEqualTo("struts"); - assertThat(cache.byMetric(p, "ncloc")).hasSize(1); - assertThat(cache.byMetric(p, "ncloc").iterator().next()).isEqualTo(m); assertThat(cache.byResource(p)).hasSize(1); assertThat(cache.byResource(p).iterator().next()).isEqualTo(m); @@ -78,6 +88,67 @@ public class MeasureCacheTest { assertThat(cache.entries()).hasSize(2); assertThat(cache.byResource(p)).hasSize(2); - assertThat(cache.byMetric(p, "ncloc")).hasSize(1); + } + + @Test + public void should_add_measure_with_same_metric() throws Exception { + MeasureCache cache = new MeasureCache(caches); + Project p = new Project("struts"); + + assertThat(cache.entries()).hasSize(0); + assertThat(cache.byResource(p)).hasSize(0); + + Measure m1 = new Measure(CoreMetrics.NCLOC, 1.0); + Measure m2 = new Measure(CoreMetrics.NCLOC, 1.0).setCharacteristic(new DefaultCharacteristic().setKey("charac")); + Measure m3 = new Measure(CoreMetrics.NCLOC, 1.0).setPersonId(2); + Measure m4 = new RuleMeasure(CoreMetrics.NCLOC, RuleKey.of("repo", "rule"), RulePriority.BLOCKER, null); + cache.put(p, m1); + cache.put(p, m2); + cache.put(p, m3); + cache.put(p, m4); + + assertThat(cache.entries()).hasSize(4); + + assertThat(cache.byResource(p)).hasSize(4); + } + + @Test + public void should_get_measures() throws Exception { + MeasureCache cache = new MeasureCache(caches); + Project p = new Project("struts"); + Resource dir = new Directory("foo/bar").setEffectiveKey("struts:foo/bar"); + Resource file1 = new File("foo/bar/File1.txt").setEffectiveKey("struts:foo/bar/File1.txt"); + Resource file2 = new File("foo/bar/File2.txt").setEffectiveKey("struts:foo/bar/File2.txt"); + + assertThat(cache.entries()).hasSize(0); + + assertThat(cache.byResource(p)).hasSize(0); + assertThat(cache.byResource(dir)).hasSize(0); + + Measure mFile1 = new Measure(CoreMetrics.NCLOC, 1.0); + cache.put(file1, mFile1); + Measure mFile2 = new Measure(CoreMetrics.NCLOC, 3.0); + cache.put(file2, mFile2); + + assertThat(cache.entries()).hasSize(2); + assertThat(cache.byResource(p)).hasSize(0); + assertThat(cache.byResource(dir)).hasSize(0); + + Measure mDir = new Measure(CoreMetrics.NCLOC, 4.0); + cache.put(dir, mDir); + + assertThat(cache.entries()).hasSize(3); + assertThat(cache.byResource(p)).hasSize(0); + assertThat(cache.byResource(dir)).hasSize(1); + assertThat(cache.byResource(dir).iterator().next()).isEqualTo(mDir); + + Measure mProj = new Measure(CoreMetrics.NCLOC, 4.0); + cache.put(p, mProj); + + assertThat(cache.entries()).hasSize(4); + assertThat(cache.byResource(p)).hasSize(1); + assertThat(cache.byResource(p).iterator().next()).isEqualTo(mProj); + assertThat(cache.byResource(dir)).hasSize(1); + assertThat(cache.byResource(dir).iterator().next()).isEqualTo(mDir); } }