From: Julien Lancelot Date: Mon, 28 Sep 2015 12:06:28 +0000 (+0200) Subject: Source code should only be read from the report X-Git-Tag: 5.2-RC1~219 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=refs%2Fpull%2F542%2Fhead;p=sonarqube.git Source code should only be read from the report Revert of SONAR-6843 --- diff --git a/server/sonar-server-benchmarks/src/test/java/org/sonar/server/benchmark/PersistFileSourcesStepTest.java b/server/sonar-server-benchmarks/src/test/java/org/sonar/server/benchmark/PersistFileSourcesStepTest.java index c8d5df53746..e57ba565c94 100644 --- a/server/sonar-server-benchmarks/src/test/java/org/sonar/server/benchmark/PersistFileSourcesStepTest.java +++ b/server/sonar-server-benchmarks/src/test/java/org/sonar/server/benchmark/PersistFileSourcesStepTest.java @@ -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(); diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/source/SourceLinesRepository.java b/server/sonar-server/src/main/java/org/sonar/server/computation/source/SourceLinesRepository.java index fc589bd0cf3..9cdfc910489 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/computation/source/SourceLinesRepository.java +++ b/server/sonar-server/src/main/java/org/sonar/server/computation/source/SourceLinesRepository.java @@ -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 readLines(Component component); } diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/source/SourceLinesRepositoryImpl.java b/server/sonar-server/src/main/java/org/sonar/server/computation/source/SourceLinesRepositoryImpl.java index 8238f0c1cd4..2d6e2d30a8c 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/computation/source/SourceLinesRepositoryImpl.java +++ b/server/sonar-server/src/main/java/org/sonar/server/computation/source/SourceLinesRepositoryImpl.java @@ -20,28 +20,20 @@ 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> 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 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 { - INSTANCE; - - @Override - public String apply(@Nonnull DbFileSources.Line line) { - return line.getSource(); - } - } - } diff --git a/server/sonar-server/src/test/java/org/sonar/server/computation/issue/IntegrateIssuesVisitorTest.java b/server/sonar-server/src/test/java/org/sonar/server/computation/issue/IntegrateIssuesVisitorTest.java index df20c65ceb9..c906425c58c 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/computation/issue/IntegrateIssuesVisitorTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/computation/issue/IntegrateIssuesVisitorTest.java @@ -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); diff --git a/server/sonar-server/src/test/java/org/sonar/server/computation/issue/TrackerRawInputFactoryTest.java b/server/sonar-server/src/test/java/org/sonar/server/computation/issue/TrackerRawInputFactoryTest.java index 70718ddd5a7..42e69c840b9 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/computation/issue/TrackerRawInputFactoryTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/computation/issue/TrackerRawInputFactoryTest.java @@ -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 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 index 00000000000..35c75a44f96 --- /dev/null +++ b/server/sonar-server/src/test/java/org/sonar/server/computation/source/SourceLinesRepositoryImplTest.java @@ -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()); + } + +} diff --git a/server/sonar-server/src/test/java/org/sonar/server/computation/source/SourceLinesRepositoryRule.java b/server/sonar-server/src/test/java/org/sonar/server/computation/source/SourceLinesRepositoryRule.java index cb2c7b3cf32..bf24e242568 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/computation/source/SourceLinesRepositoryRule.java +++ b/server/sonar-server/src/test/java/org/sonar/server/computation/source/SourceLinesRepositoryRule.java @@ -20,18 +20,21 @@ 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 lines = new ArrayList<>(); + private Multimap lines = ArrayListMultimap.create(); @Override protected void after() { @@ -40,15 +43,22 @@ public class SourceLinesRepositoryRule extends ExternalResource implements Sourc @Override public CloseableIterator 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 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 index 7da426075fb..00000000000 --- a/server/sonar-server/src/test/java/org/sonar/server/computation/source/SourceLinesRepositoryTest.java +++ /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(); - } -} - diff --git a/server/sonar-server/src/test/java/org/sonar/server/computation/step/PersistFileSourcesStepTest.java b/server/sonar-server/src/test/java/org/sonar/server/computation/step/PersistFileSourcesStepTest.java index 1d2c2e45e97..cd1bd5ee884 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/computation/step/PersistFileSourcesStepTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/computation/step/PersistFileSourcesStepTest.java @@ -20,12 +20,10 @@ 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 find(String languageKey) { - return Optional.absent(); - } - } }