Browse Source

Source code should only be read from the report

Revert of SONAR-6843
tags/5.2-RC1
Julien Lancelot 8 years ago
parent
commit
1775b82f78

+ 1
- 2
server/sonar-server-benchmarks/src/test/java/org/sonar/server/benchmark/PersistFileSourcesStepTest.java View 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();

+ 2
- 2
server/sonar-server/src/main/java/org/sonar/server/computation/source/SourceLinesRepository.java View 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);
}

+ 4
- 38
server/sonar-server/src/main/java/org/sonar/server/computation/source/SourceLinesRepositoryImpl.java View File

@@ -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<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();
}
}

}

+ 5
- 5
server/sonar-server/src/test/java/org/sonar/server/computation/issue/IntegrateIssuesVisitorTest.java View 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);

+ 7
- 5
server/sonar-server/src/test/java/org/sonar/server/computation/issue/TrackerRawInputFactoryTest.java View 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")

server/sonar-server/src/test/java/org/sonar/server/computation/source/SourceLinesRepositoryTest.java → server/sonar-server/src/test/java/org/sonar/server/computation/source/SourceLinesRepositoryImplTest.java View File

@@ -23,18 +23,13 @@ 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 {
public class SourceLinesRepositoryImplTest {

static final String FILE_UUID = "FILE_UUID";
static final String FILE_KEY = "FILE_KEY";
@@ -48,15 +43,10 @@ public class SourceLinesRepositoryTest {
@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);
SourceLinesRepositoryImpl underTest = new SourceLinesRepositoryImpl(reportReader);

@Test
public void read_lines_from_report() throws Exception {
@@ -75,18 +65,11 @@ public class SourceLinesRepositoryTest {
}

@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");
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");

assertThat(underTest.readLines(FILE)).containsOnly("report line1", "report line2");
underTest.readLines(FILE);
}

@Test
@@ -105,16 +88,4 @@ public class SourceLinesRepositoryTest {
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();
}
}


+ 18
- 8
server/sonar-server/src/test/java/org/sonar/server/computation/source/SourceLinesRepositoryRule.java View File

@@ -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<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;
}


+ 3
- 12
server/sonar-server/src/test/java/org/sonar/server/computation/step/PersistFileSourcesStepTest.java View File

@@ -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<Language> find(String languageKey) {
return Optional.absent();
}
}
}

Loading…
Cancel
Save