]> source.dussan.org Git - sonarqube.git/commitdiff
SONAR-10257 apply feedback
authorDuarte Meneses <duarte.meneses@sonarsource.com>
Mon, 5 Feb 2018 16:49:04 +0000 (17:49 +0100)
committerDuarte Meneses <duarte.meneses@sonarsource.com>
Wed, 7 Feb 2018 13:33:55 +0000 (14:33 +0100)
server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/scm/ScmInfo.java
server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/scm/ScmInfoImpl.java
server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/scm/ScmInfoRepositoryImpl.java
server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/source/SourceLinesDiff.java
server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/source/SourceLinesDiffFinder.java
server/sonar-server/src/main/java/org/sonar/server/issue/index/IssueIndex.java
server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/source/SourceLinesDiffFinderTest.java
tests/src/test/java/org/sonarqube/tests/source/NoScmTest.java
tests/src/test/java/org/sonarqube/tests/source/NoScmThenScmTest.java
tests/src/test/java/org/sonarqube/tests/source/ScmThenNoScmTest.java
tests/src/test/java/org/sonarqube/tests/source/SourceScmWS.java

index 1297b674699ddbee96c993e4f262d42c8ab86aa1..387d88c1ead18909914a38523369429f7e83d361 100644 (file)
@@ -22,7 +22,8 @@ package org.sonar.server.computation.task.projectanalysis.scm;
 import java.util.Map;
 
 /**
- * Represents the Scm information for a specific file.
+ * Represents changeset information for a file. If SCM information is present, it will be the author, revision and date fetched from SCM
+ * for every line. Otherwise, it's a date that corresponds the the analysis date in which the line was modified. 
  */
 public interface ScmInfo {
 
index ca116018d67d460518d217c7bcf19f72a8585727..feb4ea4e8a38c4335196c618fa5f636dcfbc7ce4 100644 (file)
@@ -22,15 +22,11 @@ package org.sonar.server.computation.task.projectanalysis.scm;
 import java.util.Collections;
 import java.util.Comparator;
 import java.util.Map;
-import java.util.stream.Collectors;
-import javax.annotation.CheckForNull;
 import javax.annotation.concurrent.Immutable;
 import static com.google.common.base.Preconditions.checkState;
 
 @Immutable
 public class ScmInfoImpl implements ScmInfo {
-
-  @CheckForNull
   private final Changeset latestChangeset;
   private final Map<Integer, Changeset> lineChangesets;
 
@@ -41,9 +37,8 @@ public class ScmInfoImpl implements ScmInfo {
   }
 
   private static Changeset computeLatestChangeset(Map<Integer, Changeset> lineChangesets) {
-    return lineChangesets.values().stream()
-      .collect(Collectors.maxBy(Comparator.comparing(Changeset::getDate)))
-      .orElse(null);
+    return lineChangesets.values().stream().max(Comparator.comparingLong(Changeset::getDate))
+      .orElseThrow(() -> new IllegalStateException("Expecting at least one Changeset to be present"));
   }
 
   @Override
index 5dfdf4b7a7ba6832d967e5c87006cc8364401859..24e714a5683705a798070078b905513fb4dd0e7d 100644 (file)
@@ -75,15 +75,17 @@ public class ScmInfoRepositoryImpl implements ScmInfoRepository {
   private Optional<ScmInfo> getScmInfoForComponent(Component component) {
     ScannerReport.Changesets changesets = scannerReportReader.readChangesets(component.getReportAttributes().getRef());
 
-    if (changesets != null) {
-      if (changesets.getChangesetCount() == 0) {
-        return generateAndMergeDb(component, changesets.getCopyFromPrevious());
-      }
-      return getScmInfoFromReport(component, changesets);
+    if (changesets == null) {
+      LOGGER.trace("No SCM info for file '{}'", component.getKey());
+      // SCM not available. It might have been available before - don't keep author and revision.
+      return generateAndMergeDb(component, false);
     }
 
-    LOGGER.trace("No SCM info for file '{}'", component.getKey());
-    return generateAndMergeDb(component, false);
+    // will be empty if the flag "copy from previous" is set, or if the file is empty.
+    if (changesets.getChangesetCount() == 0) {
+      return generateAndMergeDb(component, changesets.getCopyFromPrevious());
+    }
+    return getScmInfoFromReport(component, changesets);
   }
 
   private static Optional<ScmInfo> getScmInfoFromReport(Component file, ScannerReport.Changesets changesets) {
@@ -92,6 +94,9 @@ public class ScmInfoRepositoryImpl implements ScmInfoRepository {
   }
 
   private Optional<ScmInfo> generateScmInfoForAllFile(Component file) {
+    if (file.getFileAttributes().getLines() == 0) {
+      return Optional.empty();
+    }
     Set<Integer> newOrChangedLines = IntStream.rangeClosed(1, file.getFileAttributes().getLines()).boxed().collect(Collectors.toSet());
     return Optional.of(GeneratedScmInfo.create(analysisMetadata.getAnalysisDate(), newOrChangedLines));
   }
@@ -106,13 +111,13 @@ public class ScmInfoRepositoryImpl implements ScmInfoRepository {
     return Changeset.newChangesetBuilder().setDate(changeset.getDate()).build();
   }
 
-  private Optional<ScmInfo> generateAndMergeDb(Component file, boolean copyFromPrevious) {
+  private Optional<ScmInfo> generateAndMergeDb(Component file, boolean keepAuthorAndRevision) {
     Optional<DbScmInfo> dbInfoOpt = scmInfoDbLoader.getScmInfo(file);
     if (!dbInfoOpt.isPresent()) {
       return generateScmInfoForAllFile(file);
     }
 
-    ScmInfo scmInfo = copyFromPrevious ? dbInfoOpt.get() : removeAuthorAndRevision(dbInfoOpt.get());
+    ScmInfo scmInfo = keepAuthorAndRevision ? dbInfoOpt.get() : removeAuthorAndRevision(dbInfoOpt.get());
     boolean fileUnchanged = file.getStatus() == Status.SAME && sourceHashRepository.getRawSourceHash(file).equals(dbInfoOpt.get().fileHash());
 
     if (fileUnchanged) {
index 4490afd1a4fc148de36b806a1228e463495e5d8f..61b6763df88a878d062a2d765868f8549a123645 100644 (file)
@@ -22,5 +22,11 @@ package org.sonar.server.computation.task.projectanalysis.source;
 import org.sonar.server.computation.task.projectanalysis.component.Component;
 
 public interface SourceLinesDiff {
+  /**
+   * Creates a diff between the file in the database and the file in the report using Myers' algorithm, and links matching lines between
+   * both files.
+   * @return an array with one entry for each line in the left side. Those entries point either to a line in the right side, or to 0, 
+   * in which case it means the line was added.
+   */
   int[] getMatchingLines(Component component);
 }
index e60fa0b62fedbcc025b3470b572405c4f80c69b5..f0bc058c6959237ced6a2422a4b711fc4f3d4af0 100644 (file)
  */
 package org.sonar.server.computation.task.projectanalysis.source;
 
+import difflib.myers.DifferentiationFailedException;
 import difflib.myers.MyersDiff;
 import difflib.myers.PathNode;
 import java.util.List;
+import org.sonar.api.utils.log.Logger;
+import org.sonar.api.utils.log.Loggers;
 
 public class SourceLinesDiffFinder {
+  private static final Logger LOG = Loggers.get(SourceLinesDiffFinder.class);
+  private final List<String> left;
+  private final List<String> right;
 
-  private final List<String> database;
-  private final List<String> report;
-
-  public SourceLinesDiffFinder(List<String> database, List<String> report) {
-    this.database = database;
-    this.report = report;
+  public SourceLinesDiffFinder(List<String> left, List<String> right) {
+    this.left = left;
+    this.right = right;
   }
 
-  /**
-   * Creates a diff between the file in the database and the file in the report using Myers' algorithm, and links matching lines between
-   * both files.
-   * @return an array with one entry for each line in the report. Those entries point either to a line in the database, or to 0, 
-   * in which case it means the line was added.
-   */
   public int[] findMatchingLines() {
-    int[] index = new int[report.size()];
+    int[] index = new int[right.size()];
 
-    int dbLine = database.size();
-    int reportLine = report.size();
+    int dbLine = left.size();
+    int reportLine = right.size();
     try {
-      PathNode node = MyersDiff.buildPath(database.toArray(), report.toArray());
+      PathNode node = MyersDiff.buildPath(left.toArray(), right.toArray());
 
       while (node.prev != null) {
         PathNode prevNode = node.prev;
@@ -65,7 +62,8 @@ public class SourceLinesDiffFinder {
         }
         node = prevNode;
       }
-    } catch (Exception e) {
+    } catch (DifferentiationFailedException e) {
+      LOG.error("Error finding matching lines", e);
       return index;
     }
     return index;
index 7e2a89645e89c41953ac1d8de8ca1547121d197c..2b23343f6ddc71d771463ec7c825182fb6e0fe6b 100644 (file)
@@ -42,7 +42,6 @@ import org.elasticsearch.action.search.SearchResponse;
 import org.elasticsearch.index.query.BoolQueryBuilder;
 import org.elasticsearch.index.query.QueryBuilder;
 import org.elasticsearch.index.query.QueryBuilders;
-import org.elasticsearch.index.query.RangeQueryBuilder;
 import org.elasticsearch.indices.TermsLookup;
 import org.elasticsearch.search.aggregations.AggregationBuilder;
 import org.elasticsearch.search.aggregations.AggregationBuilders;
@@ -548,7 +547,7 @@ public class IssueIndex {
       .format(DateUtils.DATETIME_FORMAT)
       .timeZone(DateTimeZone.forOffsetMillis(system.getDefaultTimeZone().getRawOffset()))
       // ES dateHistogram bounds are inclusive while createdBefore parameter is exclusive
-      .extendedBounds(new ExtendedBounds(startInclusive ? startTime : startTime + 1, endTime - 1L));
+      .extendedBounds(new ExtendedBounds(startInclusive ? startTime : (startTime + 1), endTime - 1L));
     addEffortAggregationIfNeeded(query, dateHistogram);
     return Optional.of(dateHistogram);
   }
index 06c110aeb030a738d453ef52c50984c79f70be4b..99394391a66281326bc9b8e210f3b9b5d498c613 100644 (file)
@@ -29,7 +29,6 @@ public class SourceLinesDiffFinderTest {
 
   @Test
   public void shouldFindNothingWhenContentAreIdentical() {
-
     List<String> database = new ArrayList<>();
     database.add("line - 0");
     database.add("line - 1");
@@ -47,12 +46,10 @@ public class SourceLinesDiffFinderTest {
     int[] diff = new SourceLinesDiffFinder(database, report).findMatchingLines();
 
     assertThat(diff).containsExactly(1, 2, 3, 4, 5);
-
   }
 
   @Test
   public void shouldFindNothingWhenContentAreIdentical2() {
-
     List<String> database = new ArrayList<>();
     database.add("package sample;\n");
     database.add("\n");
@@ -79,12 +76,10 @@ public class SourceLinesDiffFinderTest {
 
     int[] diff = new SourceLinesDiffFinder(database, report).findMatchingLines();
     assertThat(diff).containsExactly(1, 2, 3, 4, 0, 0, 0, 0, 0, 0, 5, 6, 7);
-
   }
 
   @Test
   public void shouldDetectWhenStartingWithModifiedLines() {
-
     List<String> database = new ArrayList<>();
     database.add("line - 0");
     database.add("line - 1");
@@ -100,12 +95,10 @@ public class SourceLinesDiffFinderTest {
     int[] diff = new SourceLinesDiffFinder(database, report).findMatchingLines();
 
     assertThat(diff).containsExactly(0, 0, 3, 4);
-
   }
 
   @Test
   public void shouldDetectWhenEndingWithModifiedLines() {
-
     List<String> database = new ArrayList<>();
     database.add("line - 0");
     database.add("line - 1");
@@ -121,12 +114,10 @@ public class SourceLinesDiffFinderTest {
     int[] diff = new SourceLinesDiffFinder(database, report).findMatchingLines();
 
     assertThat(diff).containsExactly(1, 2, 0, 0);
-
   }
 
   @Test
   public void shouldDetectModifiedLinesInMiddleOfTheFile() {
-
     List<String> database = new ArrayList<>();
     database.add("line - 0");
     database.add("line - 1");
@@ -146,12 +137,10 @@ public class SourceLinesDiffFinderTest {
     int[] diff = new SourceLinesDiffFinder(database, report).findMatchingLines();
 
     assertThat(diff).containsExactly(1, 2, 0, 0, 5, 6);
-
   }
 
   @Test
   public void shouldDetectNewLinesAtBeginningOfFile() {
-
     List<String> database = new ArrayList<>();
     database.add("line - 0");
     database.add("line - 1");
@@ -167,12 +156,10 @@ public class SourceLinesDiffFinderTest {
     int[] diff = new SourceLinesDiffFinder(database, report).findMatchingLines();
 
     assertThat(diff).containsExactly(0, 0, 1, 2, 3);
-
   }
 
   @Test
   public void shouldDetectNewLinesInMiddleOfFile() {
-
     List<String> database = new ArrayList<>();
     database.add("line - 0");
     database.add("line - 1");
@@ -190,12 +177,10 @@ public class SourceLinesDiffFinderTest {
     int[] diff = new SourceLinesDiffFinder(database, report).findMatchingLines();
 
     assertThat(diff).containsExactly(1, 2, 0, 0, 3, 4);
-
   }
 
   @Test
   public void shouldDetectNewLinesAtEndOfFile() {
-
     List<String> database = new ArrayList<>();
     database.add("line - 0");
     database.add("line - 1");
@@ -211,12 +196,10 @@ public class SourceLinesDiffFinderTest {
     int[] diff = new SourceLinesDiffFinder(database, report).findMatchingLines();
 
     assertThat(diff).containsExactly(1, 2, 3, 0, 0);
-
   }
 
   @Test
   public void shouldIgnoreDeletedLinesAtEndOfFile() {
-
     List<String> database = new ArrayList<>();
     database.add("line - 0");
     database.add("line - 1");
@@ -232,12 +215,10 @@ public class SourceLinesDiffFinderTest {
     int[] diff = new SourceLinesDiffFinder(database, report).findMatchingLines();
 
     assertThat(diff).containsExactly(1, 2, 3);
-
   }
 
   @Test
   public void shouldIgnoreDeletedLinesInTheMiddleOfFile() {
-
     List<String> database = new ArrayList<>();
     database.add("line - 0");
     database.add("line - 1");
@@ -259,7 +240,6 @@ public class SourceLinesDiffFinderTest {
 
   @Test
   public void shouldIgnoreDeletedLinesAtTheStartOfTheFile() {
-
     List<String> database = new ArrayList<>();
     database.add("line - 0");
     database.add("line - 1");
@@ -273,7 +253,5 @@ public class SourceLinesDiffFinderTest {
     int[] diff = new SourceLinesDiffFinder(database, report).findMatchingLines();
 
     assertThat(diff).containsExactly(3, 4);
-
   }
-
 }
index de23ed8a23e3849ffc3f55dce48500329e379e7f..0add9b91c9ab6a637812ebe71e4620735b66c459 100644 (file)
@@ -26,6 +26,7 @@ import java.io.IOException;
 import java.text.ParseException;
 import java.util.Date;
 import java.util.Map;
+import org.junit.Before;
 import org.junit.ClassRule;
 import org.junit.Rule;
 import org.junit.Test;
@@ -49,7 +50,7 @@ public class NoScmTest {
   @ClassRule
   public static Orchestrator orchestrator = ORCHESTRATOR;
 
-  private SourceScmWS ws = new SourceScmWS(orchestrator);
+  private SourceScmWS ws;
 
   @Rule
   public TemporaryFolder temporaryFolder = new TemporaryFolder();
@@ -57,6 +58,11 @@ public class NoScmTest {
   @Rule
   public Tester tester = new Tester(orchestrator);
 
+  @Before
+  public void setUp() {
+    ws = new SourceScmWS(tester);
+  }
+  
   @Test
   public void two_analysis_without_scm_on_same_file() throws ParseException, IOException {
 
index 8da6f589b67531f49bcabd9ad19a11764b04cd13..f44fc83c68a5843e757b4cc9d9a5526b8079a01b 100644 (file)
@@ -26,6 +26,7 @@ import java.io.IOException;
 import java.text.ParseException;
 import java.util.Date;
 import java.util.Map;
+import org.junit.Before;
 import org.junit.ClassRule;
 import org.junit.Rule;
 import org.junit.Test;
@@ -48,7 +49,7 @@ public class NoScmThenScmTest {
 
   @ClassRule
   public static Orchestrator orchestrator = ORCHESTRATOR;
-  private SourceScmWS ws = new SourceScmWS(orchestrator);
+  private SourceScmWS ws;
 
   @Rule
   public TemporaryFolder temporaryFolder = new TemporaryFolder();
@@ -56,6 +57,11 @@ public class NoScmThenScmTest {
   @Rule
   public Tester tester = new Tester(orchestrator);
 
+  @Before
+  public void setUp() {
+    ws = new SourceScmWS(tester);
+  }
+
   @Test
   public void without_and_then_with_scm_on_same_file() throws ParseException, IOException {
 
index b53fbcd63001dd2d1864b0198c3be2c3f494ee5e..2a61e74dd3720002ee90eaa6ac2580a81983af88 100644 (file)
@@ -28,6 +28,7 @@ import java.time.OffsetDateTime;
 import java.time.ZoneOffset;
 import java.util.Date;
 import java.util.Map;
+import org.junit.Before;
 import org.junit.ClassRule;
 import org.junit.Rule;
 import org.junit.Test;
@@ -46,7 +47,7 @@ public class ScmThenNoScmTest {
 
   @ClassRule
   public static Orchestrator orchestrator = ORCHESTRATOR;
-  private SourceScmWS ws = new SourceScmWS(orchestrator);
+  private SourceScmWS ws;
 
   @Rule
   public TemporaryFolder temporaryFolder = new TemporaryFolder();
@@ -54,6 +55,11 @@ public class ScmThenNoScmTest {
   @Rule
   public Tester tester = new Tester(orchestrator);
 
+  @Before
+  public void setUp() {
+    ws = new SourceScmWS(tester);
+  }
+
   @Test
   public void with_and_then_without_scm_on_same_file() throws ParseException, IOException {
 
index 77472ed4be04a7c00195d4e570340a50f75e341b..c61aeca894cb9f42ab96b376d1f359b9cbbd3471 100644 (file)
  */
 package org.sonarqube.tests.source;
 
-import com.sonar.orchestrator.Orchestrator;
 import java.text.ParseException;
 import java.util.HashMap;
 import java.util.Map;
 import org.sonar.wsclient.jsonsimple.JSONArray;
 import org.sonar.wsclient.jsonsimple.JSONObject;
 import org.sonar.wsclient.jsonsimple.JSONValue;
+import org.sonarqube.qa.util.Tester;
+import org.sonarqube.ws.client.GetRequest;
 
 public class SourceScmWS {
+  public final Tester tester;
 
-  private final Orchestrator orchestrator;
-
-  public SourceScmWS(Orchestrator orchestrator) {
-    this.orchestrator = orchestrator;
+  public SourceScmWS(Tester tester) {
+    this.tester = tester;
   }
 
   public Map<Integer, LineData> getScmData(String fileKey) throws ParseException {
     Map<Integer, LineData> result = new HashMap<>();
-    String json = orchestrator.getServer().adminWsClient().get("api/sources/scm", "key", fileKey);
+    String json = tester.asAnonymous().wsClient().wsConnector().call(new GetRequest("api/sources/scm").setParam("key", fileKey)).content();
     JSONObject obj = (JSONObject) JSONValue.parse(json);
     JSONArray array = (JSONArray) obj.get("scm");
     for (Object anArray : array) {
@@ -48,5 +48,4 @@ public class SourceScmWS {
     return result;
   }
 
-
 }