Fix quality flaws

SonarSource · Mar 5, 2015 · 33e7b48 · 33e7b48
1 parent 29f7b15
commit 33e7b48
Show file tree

Hide file tree

Showing 17 changed files with 82 additions and 48 deletions.
diff --git a/server/sonar-server/src/main/java/org/sonar/server/activity/Activity.java b/server/sonar-server/src/main/java/org/sonar/server/activity/Activity.java
@@ -19,6 +19,7 @@
  */
 package org.sonar.server.activity;
 
+import javax.annotation.CheckForNull;
 import javax.annotation.Nullable;
 
 import java.util.LinkedHashMap;
@@ -51,6 +52,7 @@ public void setAction(String action) {
     this.action = action;
   }
 
+  @CheckForNull
   public String getMessage() {
     return message;
   }

diff --git a/server/sonar-server/src/main/java/org/sonar/server/activity/index/ActivityDoc.java b/server/sonar-server/src/main/java/org/sonar/server/activity/index/ActivityDoc.java
@@ -67,7 +67,7 @@ public void setLogin(@Nullable String s) {
   }
 
   public String getType() {
-    return ((String) getField(ActivityIndexDefinition.FIELD_TYPE));
+    return (String) getField(ActivityIndexDefinition.FIELD_TYPE);
   }
 
   public void setType(String s) {
@@ -83,9 +83,8 @@ public void setAction(@Nullable String s) {
     setField(ActivityIndexDefinition.FIELD_ACTION, s);
   }
 
-  @CheckForNull
   public Map<String, String> getDetails() {
-    return this.getNullableField(ActivityIndexDefinition.FIELD_DETAILS);
+    return this.getField(ActivityIndexDefinition.FIELD_DETAILS);
   }
 
   public void setDetails(Map<String, String> details) {
@@ -101,6 +100,7 @@ public void setMessage(@Nullable String s) {
     setField(ActivityIndexDefinition.FIELD_MESSAGE, s);
   }
 
+  @Override
   public String toString() {
     return ReflectionToStringBuilder.toString(this);
   }

diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/step/ParseReportStep.java b/server/sonar-server/src/main/java/org/sonar/server/computation/step/ParseReportStep.java
@@ -52,12 +52,10 @@ public void execute(ComputationContext context) {
   private void recursivelyProcessComponent(ComputationContext context, int componentRef) {
     BatchReportReader reportReader = context.getReportReader();
     BatchReport.Component component = reportReader.readComponent(componentRef);
-    if (component != null) {
+    List<BatchReport.Issue> issues = reportReader.readComponentIssues(componentRef);
-      List<BatchReport.Issue> issues = reportReader.readComponentIssues(componentRef);
+    issueComputation.processComponentIssues(context, component.getUuid(), issues);
-      issueComputation.processComponentIssues(context, component.getUuid(), issues);
+    for (Integer childRef : component.getChildRefsList()) {
-      for (Integer childRef : component.getChildRefsList()) {
+      recursivelyProcessComponent(context, childRef);
-        recursivelyProcessComponent(context, childRef);
-      }
     }
   }
 

diff --git a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileActivityQuery.java b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileActivityQuery.java
@@ -42,7 +42,9 @@ public String getQprofileKey() {
   }
 
   public QProfileActivityQuery setQprofileKey(@Nullable String qprofileKey) {
-    addDataOrFilter("profileKey", qprofileKey);
+    if (qprofileKey != null) {
+      addDataOrFilter("profileKey", qprofileKey);
+    }
     return this;
   }
 }
diff --git a/server/sonar-server/src/test/java/org/sonar/server/computation/ComputationServiceTest.java b/server/sonar-server/src/test/java/org/sonar/server/computation/ComputationServiceTest.java
@@ -50,7 +50,7 @@
 import org.sonar.api.utils.internal.JUnitTempFolder;
 import org.sonar.api.utils.log.LogTester;
 import org.sonar.api.utils.log.LoggerLevel;
-import org.sonar.batch.protocol.output.BatchOutputWriter;
+import org.sonar.batch.protocol.output.BatchReportWriter;
 import org.sonar.batch.protocol.output.BatchReport;
 import org.sonar.core.computation.db.AnalysisReportDto;
 import org.sonar.core.persistence.DbTester;
@@ -178,7 +178,7 @@ private ComputationStep mockStep(String... qualifiers) {
 
   private File generateZip() throws IOException {
     File dir = tempFolder.newDir();
-    BatchOutputWriter writer = new BatchOutputWriter(dir);
+    BatchReportWriter writer = new BatchReportWriter(dir);
     writer.writeMetadata(BatchReport.Metadata.newBuilder()
       .setRootComponentRef(1)
       .setProjectKey("PROJECT_KEY")

diff --git a/server/sonar-server/src/test/java/org/sonar/server/computation/step/ParseReportStepTest.java b/server/sonar-server/src/test/java/org/sonar/server/computation/step/ParseReportStepTest.java
@@ -24,7 +24,7 @@
 import org.junit.Test;
 import org.junit.rules.TemporaryFolder;
 import org.sonar.batch.protocol.Constants;
-import org.sonar.batch.protocol.output.BatchOutputWriter;
+import org.sonar.batch.protocol.output.BatchReportWriter;
 import org.sonar.batch.protocol.output.BatchReport;
 import org.sonar.batch.protocol.output.BatchReportReader;
 import org.sonar.core.component.ComponentDto;
@@ -78,7 +78,7 @@ public void extract_report_from_db_and_browse_components() throws Exception {
   private File generateReport() throws IOException {
     File dir = temp.newFolder();
     // project and 2 files
-    BatchOutputWriter writer = new BatchOutputWriter(dir);
+    BatchReportWriter writer = new BatchReportWriter(dir);
     writer.writeMetadata(BatchReport.Metadata.newBuilder()
       .setRootComponentRef(1)
       .setProjectKey("PROJECT_KEY")

diff --git a/sonar-batch-protocol/src/main/java/org/sonar/batch/protocol/output/BatchReportReader.java b/sonar-batch-protocol/src/main/java/org/sonar/batch/protocol/output/BatchReportReader.java
@@ -22,8 +22,6 @@
 import org.sonar.batch.protocol.ProtobufUtil;
 import org.sonar.batch.protocol.output.BatchReport.Issues;
 
-import javax.annotation.CheckForNull;
-
 import java.io.File;
 import java.util.Collections;
 import java.util.List;
@@ -44,13 +42,12 @@ public BatchReport.Metadata readMetadata() {
     return ProtobufUtil.readFile(file, BatchReport.Metadata.PARSER);
   }
 
-  @CheckForNull
   public BatchReport.Component readComponent(int componentRef) {
     File file = fileStructure.fileFor(FileStructure.Domain.COMPONENT, componentRef);
-    if (file.exists() && file.isFile()) {
+    if (!file.exists() || !file.isFile()) {
-      return ProtobufUtil.readFile(file, BatchReport.Component.PARSER);
+      throw new IllegalStateException("Unable to find report for component #" + componentRef + ". File does not exist: " + file);
     }
-    return null;
+    return ProtobufUtil.readFile(file, BatchReport.Component.PARSER);
   }
 
   public List<BatchReport.Issue> readComponentIssues(int componentRef) {

diff --git a/...ch/protocol/output/BatchOutputWriter.java → ...ch/protocol/output/BatchReportWriter.java b/...ch/protocol/output/BatchOutputWriter.java → ...ch/protocol/output/BatchReportWriter.java
@@ -23,11 +23,11 @@
 
 import java.io.File;
 
-public class BatchOutputWriter {
+public class BatchReportWriter {
 
   private final FileStructure fileStructure;
 
-  public BatchOutputWriter(File dir) {
+  public BatchReportWriter(File dir) {
     if (!dir.exists() && !dir.mkdirs()) {
       throw new IllegalStateException("Unable to create directory: " + dir);
     }

diff --git a/...r-batch-protocol/src/test/java/org/sonar/batch/protocol/output/BatchReportReaderTest.java b/...r-batch-protocol/src/test/java/org/sonar/batch/protocol/output/BatchReportReaderTest.java
@@ -48,15 +48,48 @@ public void create_dir_if_does_not_exist() throws Exception {
     assertThat(reader.readComponentIssues(1)).hasSize(1);
     assertThat(reader.readComponentIssues(200)).isEmpty();
     assertThat(reader.readComponent(1).getUuid()).isEqualTo("UUID_A");
-    assertThat(reader.readComponent(200)).isNull();
     Issues deletedComponentIssues = reader.readDeletedComponentIssues(1);
     assertThat(deletedComponentIssues.getComponentUuid()).isEqualTo("compUuid");
     assertThat(deletedComponentIssues.getListList()).hasSize(1);
+  }
+
+  @Test(expected = IllegalStateException.class)
+  public void fail_if_missing_metadata_file() throws Exception {
+    File dir = temp.newFolder();
+
+    BatchReportReader reader = new BatchReportReader(dir);
+    reader.readMetadata();
+  }
+
+  @Test(expected = IllegalStateException.class)
+   public void fail_if_missing_file_on_deleted_component() throws Exception {
+    File dir = temp.newFolder();
+
+    BatchReportReader reader = new BatchReportReader(dir);
+    reader.readDeletedComponentIssues(666);
+  }
+
+  @Test(expected = IllegalStateException.class)
+  public void fail_if_missing_file_on_component() throws Exception {
+    File dir = temp.newFolder();
+
+    BatchReportReader reader = new BatchReportReader(dir);
+    reader.readComponent(666);
+  }
 
+  /**
+   * no file if no issues
+   */
+  @Test
+  public void ignore_missing_file_on_component_issues() throws Exception {
+    File dir = temp.newFolder();
+
+    BatchReportReader reader = new BatchReportReader(dir);
+    assertThat(reader.readComponentIssues(666)).isEmpty();
   }
 
   private void initFiles(File dir) {
-    BatchOutputWriter writer = new BatchOutputWriter(dir);
+    BatchReportWriter writer = new BatchReportWriter(dir);
 
     BatchReport.Metadata.Builder metadata = BatchReport.Metadata.newBuilder()
       .setAnalysisDate(15000000L)

diff --git a/...rotocol/output/BatchOutputWriterTest.java → ...rotocol/output/BatchReportWriterTest.java b/...rotocol/output/BatchOutputWriterTest.java → ...rotocol/output/BatchReportWriterTest.java
@@ -31,7 +31,7 @@
 
 import static org.assertj.core.api.Assertions.assertThat;
 
-public class BatchOutputWriterTest {
+public class BatchReportWriterTest {
 
   @Rule
   public TemporaryFolder temp = new TemporaryFolder();
@@ -41,15 +41,15 @@ public void create_dir_if_does_not_exist() throws Exception {
     File dir = temp.newFolder();
     FileUtils.deleteQuietly(dir);
 
-    new BatchOutputWriter(dir);
+    new BatchReportWriter(dir);
 
     assertThat(dir).isDirectory().exists();
   }
 
   @Test
   public void write_metadata() throws Exception {
     File dir = temp.newFolder();
-    BatchOutputWriter writer = new BatchOutputWriter(dir);
+    BatchReportWriter writer = new BatchReportWriter(dir);
     BatchReport.Metadata.Builder metadata = BatchReport.Metadata.newBuilder()
       .setAnalysisDate(15000000L)
       .setProjectKey("PROJECT_A")
@@ -65,7 +65,7 @@ public void write_metadata() throws Exception {
   @Test
   public void write_component() throws Exception {
     File dir = temp.newFolder();
-    BatchOutputWriter writer = new BatchOutputWriter(dir);
+    BatchReportWriter writer = new BatchReportWriter(dir);
 
     // no data yet
     assertThat(writer.hasComponentData(FileStructure.Domain.COMPONENT, 1)).isFalse();
@@ -96,7 +96,7 @@ public void write_component() throws Exception {
   @Test
   public void write_issues() throws Exception {
     File dir = temp.newFolder();
-    BatchOutputWriter writer = new BatchOutputWriter(dir);
+    BatchReportWriter writer = new BatchReportWriter(dir);
 
     // no data yet
     assertThat(writer.hasComponentData(FileStructure.Domain.ISSUES, 1)).isFalse();
@@ -122,7 +122,7 @@ public void write_issues() throws Exception {
   @Test
   public void write_issues_of_deleted_component() throws Exception {
     File dir = temp.newFolder();
-    BatchOutputWriter writer = new BatchOutputWriter(dir);
+    BatchReportWriter writer = new BatchReportWriter(dir);
 
     // no data yet
     assertThat(writer.hasComponentData(FileStructure.Domain.ISSUES_ON_DELETED, 1)).isFalse();

diff --git a/sonar-batch/src/main/java/org/sonar/batch/report/ComponentsPublisher.java b/sonar-batch/src/main/java/org/sonar/batch/report/ComponentsPublisher.java
@@ -26,7 +26,7 @@
 import org.sonar.batch.index.BatchResource;
 import org.sonar.batch.index.ResourceCache;
 import org.sonar.batch.protocol.Constants;
-import org.sonar.batch.protocol.output.BatchOutputWriter;
+import org.sonar.batch.protocol.output.BatchReportWriter;
 import org.sonar.batch.protocol.output.BatchReport;
 
 import javax.annotation.CheckForNull;
@@ -45,12 +45,12 @@ public ComponentsPublisher(ProjectReactor reactor, ResourceCache resourceCache)
   }
 
   @Override
-  public void publish(BatchOutputWriter writer) {
+  public void publish(BatchReportWriter writer) {
     BatchResource rootProject = resourceCache.get(reactor.getRoot().getKeyWithBranch());
     recursiveWriteComponent(rootProject, writer);
   }
 
-  private void recursiveWriteComponent(BatchResource batchResource, BatchOutputWriter writer) {
+  private void recursiveWriteComponent(BatchResource batchResource, BatchReportWriter writer) {
     Resource r = batchResource.resource();
     BatchReport.Component.Builder builder = BatchReport.Component.newBuilder();
 

diff --git a/sonar-batch/src/main/java/org/sonar/batch/report/IssuesPublisher.java b/sonar-batch/src/main/java/org/sonar/batch/report/IssuesPublisher.java
@@ -30,7 +30,7 @@
 import org.sonar.batch.index.ResourceCache;
 import org.sonar.batch.issue.IssueCache;
 import org.sonar.batch.protocol.Constants;
-import org.sonar.batch.protocol.output.BatchOutputWriter;
+import org.sonar.batch.protocol.output.BatchReportWriter;
 import org.sonar.batch.protocol.output.BatchReport;
 
 import javax.annotation.Nullable;
@@ -52,7 +52,7 @@ public IssuesPublisher(ProjectReactor reactor, ResourceCache resourceCache, Issu
   }
 
   @Override
-  public void publish(BatchOutputWriter writer) {
+  public void publish(BatchReportWriter writer) {
     Collection<Object> deletedComponentKeys = issueCache.componentKeys();
     for (BatchResource resource : resourceCache.all()) {
       String componentKey = resource.resource().getEffectiveKey();
@@ -71,7 +71,7 @@ public BatchReport.Issue apply(DefaultIssue input) {
     exportMetadata(writer, count);
   }
 
-  private void exportMetadata(BatchOutputWriter writer, int count) {
+  private void exportMetadata(BatchReportWriter writer, int count) {
     BatchResource rootProject = resourceCache.get(reactor.getRoot().getKeyWithBranch());
     BatchReport.Metadata.Builder builder = BatchReport.Metadata.newBuilder()
       .setAnalysisDate(((Project) rootProject.resource()).getAnalysisDate().getTime())
@@ -85,7 +85,7 @@ private void exportMetadata(BatchOutputWriter writer, int count) {
     writer.writeMetadata(builder.build());
   }
 
-  private int exportIssuesOfDeletedComponents(Collection<Object> deletedComponentKeys, BatchOutputWriter writer) {
+  private int exportIssuesOfDeletedComponents(Collection<Object> deletedComponentKeys, BatchReportWriter writer) {
     int deletedComponentCount = 0;
     for (Object componentKey : deletedComponentKeys) {
       deletedComponentCount++;

diff --git a/sonar-batch/src/main/java/org/sonar/batch/report/PublishReportJob.java b/sonar-batch/src/main/java/org/sonar/batch/report/PublishReportJob.java
@@ -33,7 +33,7 @@
 import org.sonar.api.utils.ZipUtils;
 import org.sonar.batch.bootstrap.DefaultAnalysisMode;
 import org.sonar.batch.bootstrap.ServerClient;
-import org.sonar.batch.protocol.output.BatchOutputWriter;
+import org.sonar.batch.protocol.output.BatchReportWriter;
 
 import java.io.File;
 import java.io.IOException;
@@ -84,7 +84,7 @@ private File prepareReport() {
     try {
       long startTime = System.currentTimeMillis();
       File reportDir = temp.newDir("batch-report");
-      BatchOutputWriter writer = new BatchOutputWriter(reportDir);
+      BatchReportWriter writer = new BatchReportWriter(reportDir);
       for (ReportPublisher publisher : publishers) {
         publisher.publish(writer);
       }

diff --git a/sonar-batch/src/main/java/org/sonar/batch/report/ReportPublisher.java b/sonar-batch/src/main/java/org/sonar/batch/report/ReportPublisher.java
@@ -19,13 +19,13 @@
  */
 package org.sonar.batch.report;
 
-import org.sonar.batch.protocol.output.BatchOutputWriter;
+import org.sonar.batch.protocol.output.BatchReportWriter;
 
 /**
  * Adds a sub-part of data to output report
  */
 public interface ReportPublisher {
 
-  void publish(BatchOutputWriter writer);
+  void publish(BatchReportWriter writer);
 
 }
diff --git a/sonar-batch/src/test/java/org/sonar/batch/report/ComponentsPublisherTest.java b/sonar-batch/src/test/java/org/sonar/batch/report/ComponentsPublisherTest.java
@@ -30,7 +30,7 @@
 import org.sonar.api.resources.Project;
 import org.sonar.api.utils.DateUtils;
 import org.sonar.batch.index.ResourceCache;
-import org.sonar.batch.protocol.output.BatchOutputWriter;
+import org.sonar.batch.protocol.output.BatchReportWriter;
 import org.sonar.batch.protocol.output.FileStructure;
 
 import static org.assertj.core.api.Assertions.assertThat;
@@ -77,7 +77,7 @@ public void add_components_to_report() throws Exception {
     testFile.setId(6).setUuid("TEST_FILE_UUID");
     resourceCache.add(testFile, dir).setSnapshot(new Snapshot().setId(16));
 
-    BatchOutputWriter writer = new BatchOutputWriter(temp.newFolder());
+    BatchReportWriter writer = new BatchReportWriter(temp.newFolder());
     publisher.publish(writer);
 
     assertThat(writer.hasComponentData(FileStructure.Domain.COMPONENT, 1)).isTrue();

diff --git a/sonar-batch/src/test/java/org/sonar/batch/report/IssuesPublisherTest.java b/sonar-batch/src/test/java/org/sonar/batch/report/IssuesPublisherTest.java
@@ -33,7 +33,7 @@
 import org.sonar.api.utils.Duration;
 import org.sonar.batch.index.ResourceCache;
 import org.sonar.batch.issue.IssueCache;
-import org.sonar.batch.protocol.output.BatchOutputWriter;
+import org.sonar.batch.protocol.output.BatchReportWriter;
 import org.sonar.batch.protocol.output.BatchReport.Metadata;
 import org.sonar.batch.protocol.output.BatchReportReader;
 
@@ -98,7 +98,7 @@ public void publishIssuesAndMetadata() throws Exception {
     when(issueCache.byComponent("foo:src/Foo.php")).thenReturn(Arrays.asList(issue1, issue2));
 
     File outputDir = temp.newFolder();
-    BatchOutputWriter writer = new BatchOutputWriter(outputDir);
+    BatchReportWriter writer = new BatchReportWriter(outputDir);
 
     publisher.publish(writer);
 
@@ -149,7 +149,7 @@ public void publishIssuesOfDeletedComponents() throws Exception {
     when(issueCache.componentKeys()).thenReturn(Arrays.<Object>asList("foo:deleted.php"));
 
     File outputDir = temp.newFolder();
-    BatchOutputWriter writer = new BatchOutputWriter(outputDir);
+    BatchReportWriter writer = new BatchReportWriter(outputDir);
 
     publisher.publish(writer);