]> source.dussan.org Git - sonarqube.git/commitdiff
Source code should only be read from the report 542/head
authorJulien Lancelot <julien.lancelot@sonarsource.com>
Mon, 28 Sep 2015 12:06:28 +0000 (14:06 +0200)
committerJulien Lancelot <julien.lancelot@sonarsource.com>
Tue, 29 Sep 2015 06:42:05 +0000 (08:42 +0200)
Revert of SONAR-6843

server/sonar-server-benchmarks/src/test/java/org/sonar/server/benchmark/PersistFileSourcesStepTest.java
server/sonar-server/src/main/java/org/sonar/server/computation/source/SourceLinesRepository.java
server/sonar-server/src/main/java/org/sonar/server/computation/source/SourceLinesRepositoryImpl.java
server/sonar-server/src/test/java/org/sonar/server/computation/issue/IntegrateIssuesVisitorTest.java
server/sonar-server/src/test/java/org/sonar/server/computation/issue/TrackerRawInputFactoryTest.java
server/sonar-server/src/test/java/org/sonar/server/computation/source/SourceLinesRepositoryImplTest.java [new file with mode: 0644]
server/sonar-server/src/test/java/org/sonar/server/computation/source/SourceLinesRepositoryRule.java
server/sonar-server/src/test/java/org/sonar/server/computation/source/SourceLinesRepositoryTest.java [deleted file]
server/sonar-server/src/test/java/org/sonar/server/computation/step/PersistFileSourcesStepTest.java

index c8d5df5374664914b5a44ae4ed2a81d36190885b..e57ba565c94eeeefb6ab4ebebd277fd3c9d7bf3b 100644 (file)
@@ -82,8 +82,7 @@ public class PersistFileSourcesStepTest {
     BatchReportDirectoryHolderImpl batchReportDirectoryHolder = new BatchReportDirectoryHolderImpl();
     batchReportDirectoryHolder.setDirectory(reportDir);
     org.sonar.server.computation.batch.BatchReportReader batchReportReader = new BatchReportReaderImpl(batchReportDirectoryHolder);
-    PersistFileSourcesStep step = new PersistFileSourcesStep(dbClient, System2.INSTANCE, treeRootHolder, batchReportReader,
-      new SourceLinesRepositoryImpl(dbClient, batchReportReader));
+    PersistFileSourcesStep step = new PersistFileSourcesStep(dbClient, System2.INSTANCE, treeRootHolder, batchReportReader, new SourceLinesRepositoryImpl(batchReportReader));
     step.execute();
 
     long end = System.currentTimeMillis();
index fc589bd0cf301ef168c32aff71dad7f3d0aaaf84..9cdfc910489cf8a3e39de8b764fbb3a9cff25caf 100644 (file)
@@ -26,11 +26,11 @@ import org.sonar.server.computation.component.Component;
 public interface SourceLinesRepository {
 
   /**
-   * Return lines from a given component. If file sources is not in the report then we read it from the database.
+   * Return lines from a given component from the report.
    *
    * @throws NullPointerException if argument is {@code null}
    * @throws IllegalArgumentException if component is not a {@link org.sonar.server.computation.component.Component.Type#FILE}
-   * @throws IllegalStateException if the file has no source code in the report and in the database
+   * @throws IllegalStateException if the file has no source code in the report
    */
   CloseableIterator<String> readLines(Component component);
 }
index 8238f0c1cd43d8d8d714d2d63ad6c7071d101e78..2d6e2d30a8cb05b56b4f4107d6f9e3802a888561 100644 (file)
 
 package org.sonar.server.computation.source;
 
-import com.google.common.base.Function;
 import com.google.common.base.Optional;
 import com.google.common.base.Preconditions;
-import javax.annotation.Nonnull;
 import org.sonar.core.util.CloseableIterator;
-import org.sonar.db.DbClient;
-import org.sonar.db.DbSession;
-import org.sonar.db.protobuf.DbFileSources;
-import org.sonar.db.source.FileSourceDto;
 import org.sonar.server.computation.batch.BatchReportReader;
 import org.sonar.server.computation.component.Component;
 
-import static com.google.common.collect.FluentIterable.from;
+import static com.google.common.base.Preconditions.checkState;
 import static org.sonar.server.computation.component.Component.Type.FILE;
 
 public class SourceLinesRepositoryImpl implements SourceLinesRepository {
 
-  private final DbClient dbClient;
   private final BatchReportReader reportReader;
 
-  public SourceLinesRepositoryImpl(DbClient dbClient, BatchReportReader reportReader) {
-    this.dbClient = dbClient;
+  public SourceLinesRepositoryImpl(BatchReportReader reportReader) {
     this.reportReader = reportReader;
   }
 
@@ -53,33 +45,7 @@ public class SourceLinesRepositoryImpl implements SourceLinesRepository {
     }
 
     Optional<CloseableIterator<String>> linesIteratorOptional = reportReader.readFileSource(component.getReportAttributes().getRef());
-    if (linesIteratorOptional.isPresent()) {
-      return linesIteratorOptional.get();
-    }
-    DbSession session = dbClient.openSession(false);
-    try {
-      return readLinesFromDb(session, component);
-    } finally {
-      dbClient.closeSession(session);
-    }
-  }
-
-  private CloseableIterator<String> readLinesFromDb(DbSession session, Component component) {
-    FileSourceDto dto = dbClient.fileSourceDao().selectSourceByFileUuid(session, component.getUuid());
-    if (dto == null) {
-      throw new IllegalStateException(String.format("The file '%s' has no source", component));
-    }
-    DbFileSources.Data data = dto.getSourceData();
-    return CloseableIterator.from(from(data.getLinesList()).transform(LineToRaw.INSTANCE).iterator());
+    checkState(linesIteratorOptional.isPresent(), String.format("File '%s' has no source code", component));
+    return linesIteratorOptional.get();
   }
-
-  private enum LineToRaw implements Function<DbFileSources.Line, String> {
-    INSTANCE;
-
-    @Override
-    public String apply(@Nonnull DbFileSources.Line line) {
-      return line.getSource();
-    }
-  }
-
 }
index df20c65ceb99514eb7dae0facb7ec62edd8e2ce8..c906425c58c2cfd4bf686ec10fa8a275d90c7738 100644 (file)
@@ -134,7 +134,7 @@ public class IntegrateIssuesVisitorTest {
       .setSeverity(Constants.Severity.BLOCKER)
       .build();
     reportReader.putIssues(FILE_REF, asList(reportIssue));
-    reportReader.putFileSourceLines(FILE_REF, "line1");
+    fileSourceRepository.addLine(FILE_REF, "line1");
 
     underTest.visitAny(FILE);
 
@@ -164,7 +164,7 @@ public class IntegrateIssuesVisitorTest {
       .setSeverity(Constants.Severity.BLOCKER)
       .build();
     reportReader.putIssues(FILE_REF, asList(reportIssue));
-    reportReader.putFileSourceLines(FILE_REF, "line1");
+    fileSourceRepository.addLine(FILE_REF, "line1");
 
     underTest.visitAny(FILE);
 
@@ -213,7 +213,7 @@ public class IntegrateIssuesVisitorTest {
       .setSeverity(Constants.Severity.BLOCKER)
       .build();
     reportReader.putIssues(FILE_REF, asList(reportIssue));
-    reportReader.putFileSourceLines(FILE_REF, "line1");
+    fileSourceRepository.addLine(FILE_REF, "line1");
 
     underTest.visitAny(FILE);
 
@@ -252,7 +252,7 @@ public class IntegrateIssuesVisitorTest {
       .setSeverity(Constants.Severity.BLOCKER)
       .build();
     reportReader.putIssues(FILE_REF, asList(reportIssue));
-    reportReader.putFileSourceLines(FILE_REF, "line1");
+    fileSourceRepository.addLine(FILE_REF, "line1");
 
     underTest.visitAny(FILE);
 
@@ -270,7 +270,7 @@ public class IntegrateIssuesVisitorTest {
       .setSeverity(Constants.Severity.BLOCKER)
       .build();
     reportReader.putIssues(FILE_REF, asList(reportIssue));
-    reportReader.putFileSourceLines(FILE_REF, "line1");
+    fileSourceRepository.addLine(FILE_REF, "line1");
 
     underTest.visitAny(FILE);
     assertThat(componentIssuesRepository.getIssues(FILE_REF)).hasSize(1);
index 70718ddd5a7e2c8293ccec8272ead767a86a059e..42e69c840b948c3a954695df3b38a881e32ab8f3 100644 (file)
@@ -45,8 +45,10 @@ import static org.mockito.Mockito.when;
 
 public class TrackerRawInputFactoryTest {
 
+  static int FILE_REF = 2;
+
   static ReportComponent PROJECT = ReportComponent.builder(Component.Type.PROJECT, 1).setKey("PROJECT_KEY_2").setUuid("PROJECT_UUID_1").build();
-  static ReportComponent FILE = ReportComponent.builder(Component.Type.FILE, 2).setKey("FILE_KEY_2").setUuid("FILE_UUID_2").build();
+  static ReportComponent FILE = ReportComponent.builder(Component.Type.FILE, FILE_REF).setKey("FILE_KEY_2").setUuid("FILE_UUID_2").build();
 
   @Rule
   public TreeRootHolderRule treeRootHolder = new TreeRootHolderRule().setRoot(PROJECT);
@@ -63,7 +65,7 @@ public class TrackerRawInputFactoryTest {
 
   @Test
   public void load_source_hash_sequences() throws Exception {
-    fileSourceRepository.addLine("line 1;").addLine("line 2;");
+    fileSourceRepository.addLines(FILE_REF, "line 1;", "line 2;");
     Input<DefaultIssue> input = underTest.create(FILE);
 
     assertThat(input.getLineHashSequence()).isNotNull();
@@ -84,7 +86,7 @@ public class TrackerRawInputFactoryTest {
 
   @Test
   public void load_issues() throws Exception {
-    fileSourceRepository.addLine("line 1;").addLine("line 2;");
+    fileSourceRepository.addLines(FILE_REF, "line 1;", "line 2;");
     BatchReport.Issue reportIssue = BatchReport.Issue.newBuilder()
       .setLine(2)
       .setMsg("the message")
@@ -115,7 +117,7 @@ public class TrackerRawInputFactoryTest {
 
   @Test
   public void ignore_report_issues_on_common_rules() throws Exception {
-    fileSourceRepository.addLine("line 1;").addLine("line 2;");
+    fileSourceRepository.addLines(FILE_REF, "line 1;", "line 2;");
     BatchReport.Issue reportIssue = BatchReport.Issue.newBuilder()
       .setMsg("the message")
       .setRuleRepository(CommonRuleKeys.commonRepositoryForLang("java"))
@@ -131,7 +133,7 @@ public class TrackerRawInputFactoryTest {
 
   @Test
   public void load_issues_of_compute_engine_common_rules() throws Exception {
-    fileSourceRepository.addLine("line 1;").addLine("line 2;");
+    fileSourceRepository.addLines(FILE_REF, "line 1;", "line 2;");
     DefaultIssue ceIssue = new DefaultIssue()
       .setRuleKey(RuleKey.of(CommonRuleKeys.commonRepositoryForLang("java"), "InsufficientCoverage"))
       .setMessage("not enough coverage")
diff --git a/server/sonar-server/src/test/java/org/sonar/server/computation/source/SourceLinesRepositoryImplTest.java b/server/sonar-server/src/test/java/org/sonar/server/computation/source/SourceLinesRepositoryImplTest.java
new file mode 100644 (file)
index 0000000..35c75a4
--- /dev/null
@@ -0,0 +1,91 @@
+/*
+ * SonarQube, open source software quality management tool.
+ * Copyright (C) 2008-2014 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.server.computation.source;
+
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.ExpectedException;
+import org.sonar.server.computation.batch.BatchReportReaderRule;
+import org.sonar.server.computation.component.Component;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.sonar.server.computation.component.ReportComponent.builder;
+
+public class SourceLinesRepositoryImplTest {
+
+  static final String FILE_UUID = "FILE_UUID";
+  static final String FILE_KEY = "FILE_KEY";
+  static final int FILE_REF = 2;
+
+  static final Component FILE = builder(Component.Type.FILE, FILE_REF)
+    .setKey(FILE_KEY)
+    .setUuid(FILE_UUID)
+    .build();
+
+  @Rule
+  public ExpectedException thrown = ExpectedException.none();
+
+  @Rule
+  public BatchReportReaderRule reportReader = new BatchReportReaderRule();
+
+  SourceLinesRepositoryImpl underTest = new SourceLinesRepositoryImpl(reportReader);
+
+  @Test
+  public void read_lines_from_report() throws Exception {
+    reportReader.putFileSourceLines(FILE_REF, "line1", "line2");
+
+    assertThat(underTest.readLines(FILE)).containsOnly("line1", "line2");
+  }
+
+  @Test
+  public void not_fail_to_read_lines_on_empty_file_from_report() throws Exception {
+    // File exist but there's no line
+    reportReader.putFileSourceLines(FILE_REF);
+
+    // Should not try to read source file from the db
+    assertThat(underTest.readLines(FILE)).isEmpty();
+  }
+
+  @Test
+  public void fail_with_ISE_when_file_has_no_source() throws Exception {
+    thrown.expect(IllegalStateException.class);
+    thrown.expectMessage("File 'ReportComponent{ref=2, key='FILE_KEY', type=FILE}' has no source code");
+
+    underTest.readLines(FILE);
+  }
+
+  @Test
+  public void fail_with_NPE_to_read_lines_on_null_component() throws Exception {
+    thrown.expect(NullPointerException.class);
+    thrown.expectMessage("Component should not be bull");
+
+    underTest.readLines(null);
+  }
+
+  @Test
+  public void fail_with_IAE_to_read_lines_on_not_file_component() throws Exception {
+    thrown.expect(IllegalArgumentException.class);
+    thrown.expectMessage("Component 'ReportComponent{ref=123, key='NotFile', type=PROJECT}' is not a file");
+
+    underTest.readLines(builder(Component.Type.PROJECT, 123).setKey("NotFile").build());
+  }
+
+}
index cb2c7b3cf326915b6a7cb7da8ee2cf129585e03a..bf24e242568c07e990adb4210b274934b2c17df6 100644 (file)
 
 package org.sonar.server.computation.source;
 
-import com.google.common.base.Preconditions;
-import java.util.ArrayList;
-import java.util.List;
+import com.google.common.collect.ArrayListMultimap;
+import com.google.common.collect.Multimap;
+import java.util.Arrays;
+import java.util.Collection;
 import org.junit.rules.ExternalResource;
 import org.sonar.core.util.CloseableIterator;
 import org.sonar.server.computation.component.Component;
 
+import static com.google.common.base.Preconditions.checkNotNull;
+import static com.google.common.base.Preconditions.checkState;
 import static org.sonar.server.computation.component.Component.Type.FILE;
 
 public class SourceLinesRepositoryRule extends ExternalResource implements SourceLinesRepository {
 
-  private List<String> lines = new ArrayList<>();
+  private Multimap<Integer, String> lines = ArrayListMultimap.create();
 
   @Override
   protected void after() {
@@ -40,15 +43,22 @@ public class SourceLinesRepositoryRule extends ExternalResource implements Sourc
 
   @Override
   public CloseableIterator<String> readLines(Component component) {
-    Preconditions.checkNotNull(component, "Component should not be bull");
+    checkNotNull(component, "Component should not be bull");
     if (!component.getType().equals(FILE)) {
       throw new IllegalArgumentException(String.format("Component '%s' is not a file", component));
     }
-    return CloseableIterator.from(lines.iterator());
+    Collection<String> componentLines = lines.get(component.getReportAttributes().getRef());
+    checkState(!componentLines.isEmpty(), String.format("File '%s' has no source code", component));
+    return CloseableIterator.from(componentLines.iterator());
   }
 
-  public SourceLinesRepositoryRule addLine(String line){
-    lines.add(line);
+  public SourceLinesRepositoryRule addLine(int componentRef, String line) {
+    this.lines.put(componentRef, line);
+    return this;
+  }
+
+  public SourceLinesRepositoryRule addLines(int componentRef, String... lines) {
+    this.lines.putAll(componentRef, Arrays.asList(lines));
     return this;
   }
 
diff --git a/server/sonar-server/src/test/java/org/sonar/server/computation/source/SourceLinesRepositoryTest.java b/server/sonar-server/src/test/java/org/sonar/server/computation/source/SourceLinesRepositoryTest.java
deleted file mode 100644 (file)
index 7da4260..0000000
+++ /dev/null
@@ -1,120 +0,0 @@
-/*
- * SonarQube, open source software quality management tool.
- * Copyright (C) 2008-2014 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.server.computation.source;
-
-import org.junit.Rule;
-import org.junit.Test;
-import org.junit.rules.ExpectedException;
-import org.sonar.api.utils.System2;
-import org.sonar.db.DbSession;
-import org.sonar.db.DbTester;
-import org.sonar.db.protobuf.DbFileSources;
-import org.sonar.db.source.FileSourceDto;
-import org.sonar.server.computation.batch.BatchReportReaderRule;
-import org.sonar.server.computation.component.Component;
-
-import static org.assertj.core.api.Assertions.assertThat;
-import static org.sonar.server.computation.component.ReportComponent.builder;
-
-public class SourceLinesRepositoryTest {
-
-  static final String FILE_UUID = "FILE_UUID";
-  static final String FILE_KEY = "FILE_KEY";
-  static final int FILE_REF = 2;
-
-  static final Component FILE = builder(Component.Type.FILE, FILE_REF)
-    .setKey(FILE_KEY)
-    .setUuid(FILE_UUID)
-    .build();
-
-  @Rule
-  public ExpectedException thrown = ExpectedException.none();
-
-  @Rule
-  public DbTester db = DbTester.create(System2.INSTANCE);
-
-  @Rule
-  public BatchReportReaderRule reportReader = new BatchReportReaderRule();
-
-  DbSession session = db.getSession();
-
-  SourceLinesRepositoryImpl underTest = new SourceLinesRepositoryImpl(db.getDbClient(), reportReader);
-
-  @Test
-  public void read_lines_from_report() throws Exception {
-    reportReader.putFileSourceLines(FILE_REF, "line1", "line2");
-
-    assertThat(underTest.readLines(FILE)).containsOnly("line1", "line2");
-  }
-
-  @Test
-  public void not_fail_to_read_lines_on_empty_file_from_report() throws Exception {
-    // File exist but there's no line
-    reportReader.putFileSourceLines(FILE_REF);
-
-    // Should not try to read source file from the db
-    assertThat(underTest.readLines(FILE)).isEmpty();
-  }
-
-  @Test
-  public void read_lines_from_database() throws Exception {
-    insertFileSourceInDb("line1", "line2");
-
-    assertThat(underTest.readLines(FILE)).containsOnly("line1", "line2");
-  }
-
-  @Test
-  public void read_from_report_even_if_source_exist_in_db() throws Exception {
-    reportReader.putFileSourceLines(FILE_REF, "report line1", "report line2");
-    insertFileSourceInDb("db line1", "db line2");
-
-    assertThat(underTest.readLines(FILE)).containsOnly("report line1", "report line2");
-  }
-
-  @Test
-  public void fail_with_NPE_to_read_lines_on_null_component() throws Exception {
-    thrown.expect(NullPointerException.class);
-    thrown.expectMessage("Component should not be bull");
-
-    underTest.readLines(null);
-  }
-
-  @Test
-  public void fail_with_IAE_to_read_lines_on_not_file_component() throws Exception {
-    thrown.expect(IllegalArgumentException.class);
-    thrown.expectMessage("Component 'ReportComponent{ref=123, key='NotFile', type=PROJECT}' is not a file");
-
-    underTest.readLines(builder(Component.Type.PROJECT, 123).setKey("NotFile").build());
-  }
-
-  private void insertFileSourceInDb(String... lines) {
-    DbFileSources.Data.Builder dataBuilder = DbFileSources.Data.newBuilder();
-    for (int i = 0; i < lines.length; i++) {
-      dataBuilder.addLinesBuilder().setLine(i + 1).setSource(lines[i]).build();
-    }
-    db.getDbClient().fileSourceDao().insert(session,
-      new FileSourceDto()
-        .setFileUuid(FILE_UUID).setProjectUuid("PROJECT_UUID")
-        .setSourceData(dataBuilder.build()));
-    session.commit();
-  }
-}
-
index 1d2c2e45e97de8344957c7730ad1010d591bea75..cd1bd5ee8847729bd569d1448e51be13b3e025ce 100644 (file)
 
 package org.sonar.server.computation.step;
 
-import com.google.common.base.Optional;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.experimental.categories.Category;
-import org.sonar.api.resources.Language;
 import org.sonar.api.utils.System2;
 import org.sonar.batch.protocol.Constants;
 import org.sonar.batch.protocol.output.BatchReport;
@@ -39,7 +37,6 @@ import org.sonar.server.computation.batch.BatchReportReaderRule;
 import org.sonar.server.computation.batch.TreeRootHolderRule;
 import org.sonar.server.computation.component.Component;
 import org.sonar.server.computation.component.ReportComponent;
-import org.sonar.server.computation.language.LanguageRepository;
 import org.sonar.server.computation.source.SourceLinesRepositoryRule;
 import org.sonar.test.DbTests;
 
@@ -129,10 +126,10 @@ public class PersistFileSourcesStepTest extends BaseStepTest {
     reportReader.putComponent(BatchReport.Component.newBuilder()
       .setRef(FILE_REF)
       .setType(Constants.ComponentType.FILE)
-        // Lines is set to 3 but only 2 lines are read from the file -> the last lines should be added
+      // Lines is set to 3 but only 2 lines are read from the file -> the last lines should be added
       .setLines(3)
       .build());
-    fileSourceRepository.addLine("line1").addLine("line2");
+    fileSourceRepository.addLines(FILE_REF, "line 1;", "line 2;");
 
     underTest.execute();
 
@@ -442,14 +439,8 @@ public class PersistFileSourcesStepTest extends BaseStepTest {
       .build());
 
     for (int i = 1; i <= numberOfLines; i++) {
-      fileSourceRepository.addLine("line" + i);
+      fileSourceRepository.addLine(FILE_REF, "line" + i);
     }
   }
 
-  private static class EmptyLanguageRepository implements LanguageRepository {
-    @Override
-    public Optional<Language> find(String languageKey) {
-      return Optional.absent();
-    }
-  }
 }