]> source.dussan.org Git - sonarqube.git/commitdiff
SONAR-19532 PythonXUnitSensor takes too long due to non-optimized file query
authorDuarte Meneses <duarte.meneses@sonarsource.com>
Fri, 9 Jun 2023 09:30:21 +0000 (11:30 +0200)
committersonartech <sonartech@sonarsource.com>
Mon, 12 Jun 2023 20:02:49 +0000 (20:02 +0000)
sonar-plugin-api-impl/src/main/java/org/sonar/api/batch/fs/internal/predicates/DefaultFilePredicates.java
sonar-plugin-api-impl/src/main/java/org/sonar/api/batch/fs/internal/predicates/IsPredicate.java [deleted file]
sonar-plugin-api-impl/src/test/java/org/sonar/api/batch/fs/internal/predicates/DefaultFilePredicatesTest.java
sonar-scanner-engine/src/main/java/org/sonar/scanner/externalissue/sarif/LocationMapper.java
sonar-scanner-engine/src/test/java/org/sonar/scanner/externalissue/sarif/LocationMapperTest.java

index 37345e7d45504bc6afc3f3f85aefd201a8ab84e2..bc0a5dc87fb30cc6069f9d06d3514e17eef6b803 100644 (file)
@@ -133,7 +133,10 @@ public class DefaultFilePredicates implements FilePredicates {
 
   @Override
   public FilePredicate is(File ioFile) {
-    return new IsPredicate(ioFile.toPath());
+    if (ioFile.isAbsolute()) {
+      return hasAbsolutePath(ioFile.getAbsolutePath());
+    }
+    return hasRelativePath(ioFile.getPath());
   }
 
   @Override
diff --git a/sonar-plugin-api-impl/src/main/java/org/sonar/api/batch/fs/internal/predicates/IsPredicate.java b/sonar-plugin-api-impl/src/main/java/org/sonar/api/batch/fs/internal/predicates/IsPredicate.java
deleted file mode 100644 (file)
index 80b861e..0000000
+++ /dev/null
@@ -1,43 +0,0 @@
-/*
- * SonarQube
- * Copyright (C) 2009-2023 SonarSource SA
- * mailto:info AT sonarsource DOT com
- *
- * This program 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.
- *
- * This program 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.api.batch.fs.internal.predicates;
-
-import java.io.IOException;
-import java.nio.file.Files;
-import java.nio.file.Path;
-import org.sonar.api.batch.fs.InputFile;
-
-public class IsPredicate extends AbstractFilePredicate {
-
-  private final Path path;
-
-  public IsPredicate(Path path) {
-    this.path = path;
-  }
-
-  @Override
-  public boolean apply(InputFile inputFile) {
-    try {
-      return Files.isSameFile(path, inputFile.path());
-    } catch (IOException e) {
-      return false;
-    }
-  }
-}
index a9c81f2682c01ab2b40afda73ed6df66e18ad613..17c6a424915e8edebafd6d93828f8a5bfb83ffc4 100644 (file)
@@ -153,8 +153,7 @@ public class DefaultFilePredicatesTest {
     Files.touch(javaFile.file());
 
     // relative file
-    Path workingDir = Paths.get(System.getProperty("user.dir"));
-    Path relativePath = workingDir.relativize(javaFile.path());
+    Path relativePath = moduleBasePath.relativize(javaFile.path());
     assertThat(predicates.is(relativePath.toFile()).apply(javaFile)).isTrue();
 
     // absolute file
index 010ebabb3bbf9783e7868e47c21b3444e9c8bbe7..129d4b61d93c3baef5dbcd1a072eb6a891ab2a71 100644 (file)
  */
 package org.sonar.scanner.externalissue.sarif;
 
+import com.google.common.annotations.VisibleForTesting;
 import java.io.File;
+import java.io.IOException;
 import java.net.URI;
+import java.nio.file.Files;
+import java.nio.file.Path;
 import java.util.Optional;
 import javax.annotation.CheckForNull;
 import javax.annotation.Nullable;
-import org.sonar.api.batch.fs.FilePredicates;
 import org.sonar.api.batch.fs.InputFile;
+import org.sonar.api.batch.fs.internal.predicates.AbstractFilePredicate;
 import org.sonar.api.batch.sensor.SensorContext;
 import org.sonar.api.batch.sensor.issue.NewIssueLocation;
 import org.sonar.api.scanner.ScannerSide;
@@ -86,8 +90,8 @@ public class LocationMapper {
 
   @CheckForNull
   private static InputFile findFile(SensorContext context, String filePath) {
-    FilePredicates predicates = context.fileSystem().predicates();
-    return context.fileSystem().inputFile(predicates.is(getFileFromAbsoluteUriOrPath(filePath)));
+    // we use a custom predicate (which is not optimized) because fileSystem().predicates().is() doesn't handle symlinks correctly
+    return context.fileSystem().inputFile(new IsPredicate(getFileFromAbsoluteUriOrPath(filePath).toPath()));
   }
 
   private static File getFileFromAbsoluteUriOrPath(String filePath) {
@@ -98,4 +102,22 @@ public class LocationMapper {
       return new File(filePath);
     }
   }
+
+  @VisibleForTesting
+  static class IsPredicate extends AbstractFilePredicate {
+    private final Path path;
+
+    public IsPredicate(Path path) {
+      this.path = path;
+    }
+
+    @Override
+    public boolean apply(InputFile inputFile) {
+      try {
+        return Files.isSameFile(path, inputFile.path());
+      } catch (IOException e) {
+        return false;
+      }
+    }
+  }
 }
index 16f91663bd8aad3a9abf6364a366fafdccbed7cd..17508d90c34607ffc1d301e6760eea7e009a4187 100644 (file)
@@ -19,6 +19,8 @@
  */
 package org.sonar.scanner.externalissue.sarif;
 
+import java.nio.file.Path;
+import java.nio.file.Paths;
 import java.util.Optional;
 import org.junit.Before;
 import org.junit.Test;
@@ -82,23 +84,25 @@ public class LocationMapperTest {
     when(result.getMessage().getText()).thenReturn(TEST_MESSAGE);
 
     when(location.getPhysicalLocation().getArtifactLocation().getUri()).thenReturn(URI_TEST);
-
-    FilePredicate filePredicate = mock(FilePredicate.class);
-    FilePredicates predicates = sensorContext.fileSystem().predicates();
-    when(predicates.is(any())).thenReturn(filePredicate);
-
-    when(sensorContext.fileSystem().inputFile(filePredicate)).thenReturn(inputFile);
-
+    when(sensorContext.fileSystem().inputFile(any(FilePredicate.class))).thenReturn(inputFile);
   }
 
   @Test
-  public void fillIssueInProjectLocation_shouldFillRelevantFields() {
-    NewIssueLocation actualIssueLocation = locationMapper.fillIssueInProjectLocation(result, newIssueLocation);
+  public void isPredicate_whenDifferentFile_returnsFalse() {
+    Path path = Paths.get("file");
+    InputFile inputFile = mock(InputFile.class);
+    when((inputFile.path())).thenReturn(Paths.get("file2"));
+    LocationMapper.IsPredicate isPredicate = new LocationMapper.IsPredicate(path);
+    assertThat(isPredicate.apply(inputFile)).isFalse();
+  }
 
-    assertThat(actualIssueLocation).isEqualTo(newIssueLocation);
-    verify(newIssueLocation).message(TEST_MESSAGE);
-    verify(newIssueLocation).on(sensorContext.project());
-    verifyNoMoreInteractions(newIssueLocation);
+  @Test
+  public void isPredicate_whenSameFile_returnsTrue() {
+    Path path = Paths.get("file");
+    InputFile inputFile = mock(InputFile.class);
+    when((inputFile.path())).thenReturn(path);
+    LocationMapper.IsPredicate isPredicate = new LocationMapper.IsPredicate(path);
+    assertThat(isPredicate.apply(inputFile)).isTrue();
   }
 
   @Test