]> source.dussan.org Git - sonarqube.git/commitdiff
SONAR-4360 OufOfMemory error when too many issues
authorSimon Brandhof <simon.brandhof@gmail.com>
Wed, 29 May 2013 12:27:38 +0000 (14:27 +0200)
committerSimon Brandhof <simon.brandhof@gmail.com>
Wed, 29 May 2013 14:26:35 +0000 (16:26 +0200)
15 files changed:
plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/IssueTrackingDecoratorTest.java
pom.xml
sonar-batch/src/main/java/org/sonar/batch/index/Cache.java
sonar-batch/src/main/java/org/sonar/batch/issue/DefaultIssuable.java
sonar-batch/src/main/java/org/sonar/batch/issue/DeprecatedViolations.java
sonar-batch/src/main/java/org/sonar/batch/issue/IssueCache.java
sonar-batch/src/main/java/org/sonar/batch/scan/DeprecatedJsonReport.java
sonar-batch/src/main/java/org/sonar/batch/scan/JsonReport.java
sonar-batch/src/test/java/org/sonar/batch/index/CacheTest.java
sonar-batch/src/test/java/org/sonar/batch/issue/IssuableFactoryTest.java
sonar-batch/src/test/java/org/sonar/batch/issue/IssueCacheTest.java
sonar-batch/src/test/java/org/sonar/batch/source/HighlightableBuilderTest.java
sonar-core/src/main/java/org/sonar/core/component/ResourceComponent.java
sonar-core/src/test/java/org/sonar/core/component/ResourceComponentTest.java
sonar-plugin-api/src/main/java/org/sonar/api/issue/Issuable.java

index 4b4a56832f4adaef4ef95b65f83e49a5420d3520..9944887b07bed676ebf72a2c72ea68f1e98281ed 100644 (file)
@@ -49,7 +49,7 @@ import static org.mockito.Mockito.*;
 public class IssueTrackingDecoratorTest extends AbstractDaoTestCase {
 
   IssueTrackingDecorator decorator;
-  IssueCache issueCache = mock(IssueCache.class);
+  IssueCache issueCache = mock(IssueCache.class, RETURNS_MOCKS);
   InitialOpenIssuesStack initialOpenIssues = mock(InitialOpenIssuesStack.class);
   IssueTracking tracking = mock(IssueTracking.class, RETURNS_MOCKS);
   IssueHandlers handlers = mock(IssueHandlers.class);
diff --git a/pom.xml b/pom.xml
index 031b94c60a701871e7bbcbba2280f4208711c083..d4ba34cca8e15b1dbfa767d2cbdad97f57a38302 100644 (file)
--- a/pom.xml
+++ b/pom.xml
       <dependency>
         <groupId>com.google.code.gson</groupId>
         <artifactId>gson</artifactId>
-        <version>2.2.2</version>
+        <version>2.2.4</version>
       </dependency>
       <dependency>
         <groupId>com.akiban</groupId>
index 399dafb665f7d9424f270b35cd2f092efff5bdb1..f68292c94af9445e7141280551f5da59a4a8b91f 100644 (file)
@@ -19,7 +19,6 @@
  */
 package org.sonar.batch.index;
 
-import com.google.common.collect.Lists;
 import com.google.common.collect.Sets;
 import com.persistit.Exchange;
 import com.persistit.Key;
@@ -28,9 +27,7 @@ import org.apache.commons.lang.builder.ToStringBuilder;
 
 import javax.annotation.CheckForNull;
 import java.io.Serializable;
-import java.util.Collection;
 import java.util.Iterator;
-import java.util.List;
 import java.util.Set;
 
 /**
@@ -41,7 +38,6 @@ import java.util.Set;
 public class Cache<K, V extends Serializable> {
 
   private static final String DEFAULT_GROUP = "_";
-  // TODO improve exception messages by using this cache name
   private final String name;
   private final Exchange exchange;
 
@@ -62,7 +58,7 @@ public class Cache<K, V extends Serializable> {
       exchange.store();
       return this;
     } catch (Exception e) {
-      throw new IllegalStateException("Fail to put element in cache", e);
+      throw new IllegalStateException("Fail to put element in the cache " + name, e);
     }
   }
 
@@ -84,7 +80,7 @@ public class Cache<K, V extends Serializable> {
       }
       return (V) exchange.getValue().get();
     } catch (Exception e) {
-      throw new IllegalStateException("Fail to get element from cache", e);
+      throw new IllegalStateException("Fail to get element from cache " + name, e);
     }
   }
 
@@ -106,7 +102,7 @@ public class Cache<K, V extends Serializable> {
       exchange.append(group).append(key);
       return exchange.remove();
     } catch (Exception e) {
-      throw new IllegalStateException("Fail to get element from cache", e);
+      throw new IllegalStateException("Fail to get element from cache " + name, e);
     }
   }
 
@@ -128,7 +124,7 @@ public class Cache<K, V extends Serializable> {
       exchange.removeKeyRange(exchange.getKey(), key);
       return this;
     } catch (Exception e) {
-      throw new IllegalStateException("Fail to clear cache group: " + group, e);
+      throw new IllegalStateException("Fail to clear group '" + group + "' from cache " + name, e);
     }
   }
 
@@ -173,7 +169,7 @@ public class Cache<K, V extends Serializable> {
       }
       return keys;
     } catch (Exception e) {
-      throw new IllegalStateException("Fail to get cache keys", e);
+      throw new IllegalStateException("Fail to get keys from cache " + name, e);
     }
   }
 
@@ -187,40 +183,38 @@ public class Cache<K, V extends Serializable> {
     return keySet(DEFAULT_GROUP);
   }
 
-  // TODO implement a lazy-loading equivalent with Iterator/Iterable
-  public Collection<V> values(String group) {
+  /**
+   * Lazy-loading values for a given group
+   */
+  public Iterable<V> values(String group) {
     try {
-      List<V> values = Lists.newArrayList();
       exchange.clear();
       Exchange iteratorExchange = new Exchange(exchange);
       iteratorExchange.append(group).append(Key.BEFORE);
-      while (iteratorExchange.next(false)) {
-        if (iteratorExchange.getValue().isDefined()) {
-          values.add((V) iteratorExchange.getValue().get());
-        }
-      }
-      return values;
+      return new ValueIterable<V>(iteratorExchange, false);
     } catch (Exception e) {
-      throw new IllegalStateException("Fail to get cache values", e);
+      throw new IllegalStateException("Fail to get values from cache " + name, e);
     }
   }
 
+  /**
+   * Lazy-loading values
+   */
   public Iterable<V> values() {
     return values(DEFAULT_GROUP);
   }
 
-  public Collection<V> allValues() {
+  /**
+   * Lazy-loading values of all groups
+   */
+  public Iterable<V> allValues() {
     try {
-      List<V> values = Lists.newArrayList();
       exchange.clear();
       Exchange iteratorExchange = new Exchange(exchange);
       iteratorExchange.append(Key.BEFORE);
-      while (iteratorExchange.next(true)) {
-        values.add((V) iteratorExchange.getValue().get());
-      }
-      return values;
+      return new ValueIterable<V>(iteratorExchange, true);
     } catch (Exception e) {
-      throw new IllegalStateException("Fail to get cache values", e);
+      throw new IllegalStateException("Fail to get values from cache " + name, e);
     }
   }
 
@@ -235,7 +229,7 @@ public class Cache<K, V extends Serializable> {
       }
       return groups;
     } catch (Exception e) {
-      throw new IllegalStateException("Fail to get cache values", e);
+      throw new IllegalStateException("Fail to get values from cache " + name, e);
     }
   }
 
@@ -249,6 +243,56 @@ public class Cache<K, V extends Serializable> {
     return new EntryIterable(new Exchange(exchange), false);
   }
 
+
+  //
+  // LAZY ITERATORS AND ITERABLES
+  //
+
+  private static class ValueIterable<T extends Serializable> implements Iterable<T> {
+    private final Iterator<T> iterator;
+
+    private ValueIterable(Exchange exchange, boolean deep) {
+      this.iterator = new ValueIterator<T>(exchange, deep);
+    }
+
+    @Override
+    public Iterator<T> iterator() {
+      return iterator;
+    }
+  }
+
+  private static class ValueIterator<T extends Serializable> implements Iterator<T> {
+    private final Exchange exchange;
+    private final boolean deep;
+
+    private ValueIterator(Exchange exchange, boolean deep) {
+      this.exchange = exchange;
+      this.deep = deep;
+    }
+
+    @Override
+    public boolean hasNext() {
+      try {
+        return exchange.next(deep);
+      } catch (PersistitException e) {
+        throw new IllegalStateException(e);
+      }
+    }
+
+    @Override
+    public T next() {
+      T value = null;
+      if (exchange.getValue().isDefined()) {
+        value = (T) exchange.getValue().get();
+      }
+      return value;
+    }
+
+    @Override
+    public void remove() {
+    }
+  }
+
   private static class EntryIterable<T extends Serializable> implements Iterable<Entry<T>> {
     private final EntryIterator<T> it;
 
index 151e1783b3e2fab4bd175d4391c263c5833b7f08..06a0c3df42d00f5bf0b414fedd1b97e70fc0e068 100644 (file)
  */
 package org.sonar.batch.issue;
 
+import com.google.common.collect.Lists;
 import org.sonar.api.component.Component;
 import org.sonar.api.issue.Issuable;
 import org.sonar.api.issue.Issue;
 import org.sonar.core.issue.DefaultIssue;
 import org.sonar.core.issue.DefaultIssueBuilder;
 
-import java.util.Collection;
+import java.util.List;
 
 /**
  * @since 3.6
@@ -49,13 +50,14 @@ public class DefaultIssuable implements Issuable {
 
   @Override
   public boolean addIssue(Issue issue) {
-    return scanIssues.initAndAddIssue(((DefaultIssue)issue));
+    return scanIssues.initAndAddIssue(((DefaultIssue) issue));
   }
 
   @SuppressWarnings("unchecked")
   @Override
-  public Collection<Issue> issues() {
-    return (Collection)cache.byComponent(component.key());
+  public List<Issue> issues() {
+    Iterable<DefaultIssue> elements = cache.byComponent(component.key());
+    return Lists.<Issue>newArrayList(elements);
   }
 
   @Override
index 936165eae69846f6589d15e13a1912c0360d7149..b0c0c39b66aa1985ec5cc6a5ab78b7dff7a3aa6a 100644 (file)
@@ -50,7 +50,7 @@ public class DeprecatedViolations implements BatchComponent {
   }
 
   public List<Violation> get(String componentKey) {
-    Collection<DefaultIssue> issues = issueCache.byComponent(componentKey);
+    Iterable<DefaultIssue> issues = issueCache.byComponent(componentKey);
     List<Violation> violations = Lists.newArrayList();
     for (DefaultIssue issue : issues) {
       violations.add(toViolation(issue));
index b136805f81584720974f1560ad03992247e8a820..0c28eaaa3937f5c668409e4b108b51d994d948cd 100644 (file)
@@ -25,10 +25,6 @@ import org.sonar.batch.index.Cache;
 import org.sonar.batch.index.Caches;
 import org.sonar.core.issue.DefaultIssue;
 
-import java.util.Collection;
-
-import static com.google.common.collect.Lists.newArrayList;
-
 /**
  * Shared issues among all project modules
  */
@@ -41,12 +37,12 @@ public class IssueCache implements BatchComponent {
     cache = caches.createCache("issues");
   }
 
-  public Collection<DefaultIssue> byComponent(String componentKey) {
+  public Iterable<DefaultIssue> byComponent(String componentKey) {
     return cache.values(componentKey);
   }
 
-  public Collection<DefaultIssue> all() {
-    return newArrayList(cache.allValues());
+  public Iterable<DefaultIssue> all() {
+    return cache.allValues();
   }
 
   public IssueCache put(DefaultIssue issue) {
index 410725d86db8eebc973f63041f00278ff7d78d52..a3f35db68d10e66a6743172d41d27c73f626e19d 100644 (file)
@@ -91,10 +91,10 @@ public class DeprecatedJsonReport implements BatchComponent {
   }
 
   @VisibleForTesting
-  void writeJson(Collection<Resource> resources, Writer output) {
+  void writeJson(Collection<Resource> resources, Writer writer) {
     JsonWriter json = null;
     try {
-      json = new JsonWriter(output);
+      json = new JsonWriter(writer);
       json.setSerializeNulls(false);
 
       json.beginObject()
@@ -103,16 +103,15 @@ public class DeprecatedJsonReport implements BatchComponent {
         .beginObject();
 
       for (Resource resource : resources) {
-        Collection<DefaultIssue> issues = getIssues(resource);
-        if (issues.isEmpty()) {
-          continue;
-        }
-
-        json.name(resource.getKey())
-          .beginArray();
-
+        Iterable<DefaultIssue> issues = getIssues(resource);
+        boolean hasViolation = false;
         for (DefaultIssue issue : issues) {
-          json.beginObject()
+          if (!hasViolation) {
+            json.name(resource.getKey()).beginArray();
+            hasViolation = true;
+          }
+          json
+            .beginObject()
             .name("line").value(issue.line())
             .name("message").value(issue.message())
             .name("severity").value(issue.severity())
@@ -120,19 +119,16 @@ public class DeprecatedJsonReport implements BatchComponent {
             .name("rule_repository").value(issue.ruleKey().repository())
             .name("rule_name").value(ruleName(issue.ruleKey()))
             .name("switched_off").value(Issue.RESOLUTION_FALSE_POSITIVE.equals(issue.resolution()))
-            .name("is_new").value((issue.isNew()));
-          if (issue.creationDate() != null) {
-            json.name("created_at").value(DateUtils.formatDateTime(issue.creationDate()));
-          }
-          json.endObject();
+            .name("is_new").value((issue.isNew()))
+            .name("created_at").value(DateUtils.formatDateTime(issue.creationDate()))
+            .endObject();
+        }
+        if (hasViolation) {
+          json.endArray();
         }
-
-        json.endArray();
       }
 
-      json.endObject()
-        .endObject()
-        .flush();
+      json.endObject().endObject().flush();
     } catch (IOException e) {
       throw new SonarException("Unable to export results", e);
     } finally {
@@ -145,7 +141,7 @@ public class DeprecatedJsonReport implements BatchComponent {
   }
 
   @VisibleForTesting
-  Collection<DefaultIssue> getIssues(Resource resource) {
+  Iterable<DefaultIssue> getIssues(Resource resource) {
     return issueCache.byComponent(resource.getEffectiveKey());
   }
 }
index d37a139d8ad28c833094b8da792bf974a352c0bb..996950f99acdf40039c2315fe4d8ef5679bf8696 100644 (file)
@@ -175,7 +175,7 @@ public class JsonReport implements BatchComponent {
   }
 
   @VisibleForTesting
-  Collection<DefaultIssue> getIssues() {
+  Iterable<DefaultIssue> getIssues() {
     return issueCache.all();
   }
 }
index 385a798d1c79f758adf6b39c070fdb34f8710979..b4acfe4cb638a83e799b78208fed0d053077b765 100644 (file)
@@ -124,6 +124,7 @@ public class CacheTest {
     cache.put("org/apache/struts/Filter.java", "lines", 500f);
     assertThat(cache.values("org/apache/struts/Action.java")).containsOnly(123f, 200f);
     assertThat(cache.values("org/apache/struts/Filter.java")).containsOnly(500f);
+    assertThat(cache.values("other")).isEmpty();
   }
 
   @Test
index f661546cb323b4728e9e0c09351bd719a6bd711e..9bc450ac5fd904945f12589027dbc0568cba4110 100644 (file)
@@ -20,6 +20,7 @@
 package org.sonar.batch.issue;
 
 import org.junit.Test;
+import org.mockito.Mockito;
 import org.sonar.api.component.Component;
 import org.sonar.api.issue.Issuable;
 import org.sonar.api.resources.File;
@@ -33,13 +34,13 @@ import static org.mockito.Mockito.mock;
 
 public class IssuableFactoryTest {
 
-  ScanIssues issueFactory = mock(ScanIssues.class);
-  IssueCache cache = mock(IssueCache.class);
+  ScanIssues scanIssues = mock(ScanIssues.class);
+  IssueCache cache = mock(IssueCache.class, Mockito.RETURNS_MOCKS);
 
   @Test
   public void file_should_be_issuable() throws Exception {
-    IssuableFactory factory = new IssuableFactory(issueFactory, cache);
-    Component component = new ResourceComponent(new File("foo/bar.c"));
+    IssuableFactory factory = new IssuableFactory(scanIssues, cache);
+    Component component = new ResourceComponent(new File("foo/bar.c").setEffectiveKey("foo/bar.c"));
     Issuable issuable = factory.loadPerspective(Issuable.class, component);
 
     assertThat(issuable).isNotNull();
@@ -49,8 +50,8 @@ public class IssuableFactoryTest {
 
   @Test
   public void project_should_be_issuable() throws Exception {
-    IssuableFactory factory = new IssuableFactory(issueFactory, cache);
-    Component component = new ResourceComponent(new Project("Foo"));
+    IssuableFactory factory = new IssuableFactory(scanIssues, cache);
+    Component component = new ResourceComponent(new Project("Foo").setEffectiveKey("foo"));
     Issuable issuable = factory.loadPerspective(Issuable.class, component);
 
     assertThat(issuable).isNotNull();
@@ -60,8 +61,8 @@ public class IssuableFactoryTest {
 
   @Test
   public void java_file_should_be_issuable() throws Exception {
-    IssuableFactory factory = new IssuableFactory(issueFactory, cache);
-    Component component = new ResourceComponent(new JavaFile("bar.Foo"));
+    IssuableFactory factory = new IssuableFactory(scanIssues, cache);
+    Component component = new ResourceComponent(new JavaFile("org.apache.Action").setEffectiveKey("struts:org.apache.Action"));
     Issuable issuable = factory.loadPerspective(Issuable.class, component);
 
     assertThat(issuable).isNotNull();
@@ -71,8 +72,8 @@ public class IssuableFactoryTest {
 
   @Test
   public void java_class_should_not_be_issuable() throws Exception {
-    IssuableFactory factory = new IssuableFactory(issueFactory, cache);
-    Component component = new ResourceComponent(JavaClass.create("bar", "Foo"));
+    IssuableFactory factory = new IssuableFactory(scanIssues, cache);
+    Component component = new ResourceComponent(JavaClass.create("org.apache.Action").setEffectiveKey("struts:org.apache.Action"));
     Issuable issuable = factory.loadPerspective(Issuable.class, component);
 
     assertThat(issuable).isNull();
index 7d0102afef6dd817c5ef8901b7b39d02506406e6..15f286f6ee394e9e97d825dc005fd2741ad76b16 100644 (file)
@@ -21,6 +21,7 @@ package org.sonar.batch.issue;
 
 import com.google.common.base.Function;
 import com.google.common.collect.Collections2;
+import com.google.common.collect.ImmutableList;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
@@ -31,6 +32,7 @@ import org.sonar.core.issue.DefaultIssue;
 
 import javax.annotation.Nullable;
 import java.util.Collection;
+import java.util.List;
 
 import static org.fest.assertions.Assertions.assertThat;
 
@@ -69,7 +71,7 @@ public class IssueCacheTest {
     issue.setSeverity(Severity.MINOR);
     cache.put(issue);
 
-    Collection<DefaultIssue> issues = cache.byComponent("org.struts.Action");
+    List<DefaultIssue> issues = ImmutableList.copyOf(cache.byComponent("org.struts.Action"));
     assertThat(issues).hasSize(1);
     Issue reloaded = issues.iterator().next();
     assertThat(reloaded.key()).isEqualTo("111");
@@ -83,12 +85,12 @@ public class IssueCacheTest {
     DefaultIssue issue2 = new DefaultIssue().setKey("222").setComponentKey("org.struts.Filter").setSeverity(Severity.INFO);
     cache.put(issue1).put(issue2);
 
-    Collection<DefaultIssue> issues = cache.all();
-    assertThat(issues).hasSize(2).containsOnly(issue1, issue2);
+    List<DefaultIssue> issues = ImmutableList.copyOf(cache.all());
+    assertThat(issues).containsOnly(issue1, issue2);
   }
 
-  private Collection<String> issueKeys(Collection<DefaultIssue> issues) {
-    return Collections2.transform(issues, new Function<DefaultIssue, String>() {
+  private Collection<String> issueKeys(Iterable<DefaultIssue> issues) {
+    return Collections2.transform(ImmutableList.copyOf(issues), new Function<DefaultIssue, String>() {
       @Override
       public String apply(@Nullable DefaultIssue issue) {
         return issue.key();
index e535d30f5af0c8a17f78372241a13f58f06997ee..757b0e0cf5533d602f20fd98daea348a2b9be692 100644 (file)
@@ -23,6 +23,7 @@ import org.junit.Test;
 import org.sonar.api.component.Component;
 import org.sonar.api.resources.File;
 import org.sonar.api.resources.Project;
+import org.sonar.api.resources.Resource;
 import org.sonar.api.source.Highlightable;
 import org.sonar.batch.index.ComponentDataCache;
 import org.sonar.core.component.ResourceComponent;
@@ -37,7 +38,8 @@ public class HighlightableBuilderTest {
 
   @Test
   public void should_load_default_perspective() throws Exception {
-    Component component = new ResourceComponent(new File("foo/bar.c"));
+    Resource file = new File("foo.c").setEffectiveKey("myproject:path/to/foo.c");
+    Component component = new ResourceComponent(file);
 
     HighlightableBuilder builder = new HighlightableBuilder(cache);
     Highlightable perspective = builder.loadPerspective(Highlightable.class, component);
@@ -48,7 +50,7 @@ public class HighlightableBuilderTest {
 
   @Test
   public void project_should_not_be_highlightable() {
-    Component component = new ResourceComponent(new Project("Foo"));
+    Component component = new ResourceComponent(new Project("struts").setEffectiveKey("org.struts"));
 
     HighlightableBuilder builder = new HighlightableBuilder(cache);
     Highlightable perspective = builder.loadPerspective(Highlightable.class, component);
@@ -58,7 +60,7 @@ public class HighlightableBuilderTest {
 
   @Test
   public void java_class_should_not_be_highlightable() {
-    Component component = new ResourceComponent(JavaClass.create("foo", "Bar"));
+    Component component = new ResourceComponent(JavaClass.create("org.struts.Action").setEffectiveKey("struts:org.struts.Action"));
 
     HighlightableBuilder builder = new HighlightableBuilder(cache);
     Highlightable perspective = builder.loadPerspective(Highlightable.class, component);
index 33195157a6f070bef624a89ef20436a8b38fb5d6..339cc935d7d2540f106de9e989a93d7bb9560a59 100644 (file)
@@ -19,6 +19,7 @@
  */
 package org.sonar.core.component;
 
+import com.google.common.base.Strings;
 import org.sonar.api.component.Component;
 import org.sonar.api.database.model.Snapshot;
 import org.sonar.api.resources.Resource;
@@ -36,6 +37,9 @@ public class ResourceComponent implements Component {
 
   public ResourceComponent(Resource resource, @Nullable Snapshot snapshot) {
     this.key = resource.getEffectiveKey();
+    if (Strings.isNullOrEmpty(key)) {
+      throw new IllegalArgumentException("Missing component key");
+    }
     this.name = resource.getName();
     this.longName = resource.getLongName();
     this.qualifier = resource.getQualifier();
index e0e278b62b1d807154e836ab3b9e2c1d3d042e05..a3e87afd5b92a453a71f411114aa0968f59263e1 100644 (file)
@@ -22,13 +22,18 @@ package org.sonar.core.component;
 import org.junit.Test;
 import org.sonar.api.database.model.Snapshot;
 import org.sonar.api.resources.File;
+import org.sonar.api.resources.Resource;
 
 import static org.fest.assertions.Assertions.assertThat;
+import static org.fest.assertions.Fail.fail;
 
 public class ResourceComponentTest {
+
+  Resource file = new File("foo.c").setEffectiveKey("myproject:path/to/foo.c");
+
   @Test
   public void db_ids_should_be_optional() {
-    ResourceComponent component = new ResourceComponent(new File("foo.c"), new Snapshot());
+    ResourceComponent component = new ResourceComponent(file, new Snapshot());
 
     assertThat(component.snapshotId()).isNull();
     assertThat(component.resourceId()).isNull();
@@ -39,7 +44,7 @@ public class ResourceComponentTest {
     Snapshot snapshot = new Snapshot();
     snapshot.setId(123);
     snapshot.setResourceId(456);
-    ResourceComponent component = new ResourceComponent(new File("foo.c"), snapshot);
+    ResourceComponent component = new ResourceComponent(file, snapshot);
 
     assertThat(component.snapshotId()).isEqualTo(123);
     assertThat(component.resourceId()).isEqualTo(456);
@@ -47,10 +52,18 @@ public class ResourceComponentTest {
 
   @Test
   public void should_use_effective_key() {
-    File file = new File("foo.c");
-    file.setEffectiveKey("myproject:path/to/foo.c");
     ResourceComponent component = new ResourceComponent(file);
-
     assertThat(component.key()).isEqualTo("myproject:path/to/foo.c");
   }
+
+  @Test
+  public void effective_key_should_be_set() {
+    try {
+      File file = new File("foo.c");
+      new ResourceComponent(file);
+      fail();
+    } catch (IllegalArgumentException e) {
+      assertThat(e).hasMessage("Missing component key");
+    }
+  }
 }
index 3dd8d7412622f9cdcfe4f228aaea010a9f5e6e88..bd6005916197397e919d5c8270e6206cb966225b 100644 (file)
@@ -24,7 +24,7 @@ import org.sonar.api.component.Perspective;
 import org.sonar.api.rule.RuleKey;
 
 import javax.annotation.Nullable;
-import java.util.Collection;
+import java.util.List;
 
 /**
  * @since 3.6
@@ -56,5 +56,5 @@ public interface Issuable extends Perspective {
    */
   boolean addIssue(Issue issue);
 
-  Collection<Issue> issues();
+  List<Issue> issues();
 }