From 27fb5c0e4fad811bae8028a4e13267fdc083f8b3 Mon Sep 17 00:00:00 2001 From: Wojtek Wajerowicz <115081248+wojciech-wajerowicz-sonarsource@users.noreply.github.com> Date: Thu, 15 Jun 2023 17:04:02 +0200 Subject: [PATCH] SONAR-19522 Fail-fast if SARIF file doesn't exist. (#8587) --- .../org/sonar/core/sarif/SarifSerializer.java | 3 +- .../sonar/core/sarif/SarifSerializerImpl.java | 7 +-- ...Test.java => SarifSerializerImplTest.java} | 8 ++-- .../sarif/SarifIssuesImportSensor.java | 8 +++- .../sarif/SarifIssuesImportSensorTest.java | 46 ++++++++++++++----- 5 files changed, 51 insertions(+), 21 deletions(-) rename sonar-core/src/test/java/org/sonar/core/sarif/{SarifSerializerTest.java => SarifSerializerImplTest.java} (96%) diff --git a/sonar-core/src/main/java/org/sonar/core/sarif/SarifSerializer.java b/sonar-core/src/main/java/org/sonar/core/sarif/SarifSerializer.java index fb9777a6bbc..a20b18f280a 100644 --- a/sonar-core/src/main/java/org/sonar/core/sarif/SarifSerializer.java +++ b/sonar-core/src/main/java/org/sonar/core/sarif/SarifSerializer.java @@ -19,11 +19,12 @@ */ package org.sonar.core.sarif; +import java.nio.file.NoSuchFileException; import java.nio.file.Path; public interface SarifSerializer { String serialize(Sarif210 sarif210); - Sarif210 deserialize(Path sarifPath); + Sarif210 deserialize(Path sarifPath) throws NoSuchFileException; } diff --git a/sonar-core/src/main/java/org/sonar/core/sarif/SarifSerializerImpl.java b/sonar-core/src/main/java/org/sonar/core/sarif/SarifSerializerImpl.java index 487f81cf10a..202dd59bad4 100644 --- a/sonar-core/src/main/java/org/sonar/core/sarif/SarifSerializerImpl.java +++ b/sonar-core/src/main/java/org/sonar/core/sarif/SarifSerializerImpl.java @@ -25,6 +25,7 @@ import com.google.gson.JsonIOException; import com.google.gson.JsonSyntaxException; import java.io.IOException; import java.io.Reader; +import java.nio.file.NoSuchFileException; import java.nio.file.Path; import javax.inject.Inject; import org.sonar.api.ce.ComputeEngineSide; @@ -58,17 +59,17 @@ public class SarifSerializerImpl implements SarifSerializer { } @Override - public Sarif210 deserialize(Path reportPath) { + public Sarif210 deserialize(Path reportPath) throws NoSuchFileException { try (Reader reader = newBufferedReader(reportPath, UTF_8)) { Sarif210 sarif = gson.fromJson(reader, Sarif210.class); SarifVersionValidator.validateSarifVersion(sarif.getVersion()); return sarif; + } catch (NoSuchFileException e) { + throw e; } catch (JsonIOException | IOException e) { throw new IllegalStateException(format(SARIF_REPORT_ERROR, reportPath), e); } catch (JsonSyntaxException e) { throw new IllegalStateException(format(SARIF_JSON_SYNTAX_ERROR, reportPath), e); - } catch (IllegalStateException e) { - throw new IllegalStateException(e.getMessage(), e); } } } diff --git a/sonar-core/src/test/java/org/sonar/core/sarif/SarifSerializerTest.java b/sonar-core/src/test/java/org/sonar/core/sarif/SarifSerializerImplTest.java similarity index 96% rename from sonar-core/src/test/java/org/sonar/core/sarif/SarifSerializerTest.java rename to sonar-core/src/test/java/org/sonar/core/sarif/SarifSerializerImplTest.java index 24532267883..3a240cfe73e 100644 --- a/sonar-core/src/test/java/org/sonar/core/sarif/SarifSerializerTest.java +++ b/sonar-core/src/test/java/org/sonar/core/sarif/SarifSerializerImplTest.java @@ -21,6 +21,7 @@ package org.sonar.core.sarif; import java.net.URISyntaxException; import java.net.URL; +import java.nio.file.NoSuchFileException; import java.nio.file.Path; import java.nio.file.Paths; import java.util.Set; @@ -36,7 +37,7 @@ import static org.assertj.core.api.Assertions.fail; import static org.sonar.core.sarif.SarifVersionValidator.UNSUPPORTED_VERSION_MESSAGE_TEMPLATE; @RunWith(MockitoJUnitRunner.class) -public class SarifSerializerTest { +public class SarifSerializerImplTest { private static final String SARIF_JSON = "{\"version\":\"2.1.0\",\"$schema\":\"http://json.schemastore.org/sarif-2.1.0-rtm.4\",\"runs\":[{\"results\":[]}]}"; @@ -53,7 +54,7 @@ public class SarifSerializerTest { } @Test - public void deserialize() throws URISyntaxException { + public void deserialize() throws URISyntaxException, NoSuchFileException { URL sarifResource = requireNonNull(getClass().getResource("eslint-sarif210.json")); Path sarif = Paths.get(sarifResource.toURI()); @@ -68,8 +69,7 @@ public class SarifSerializerTest { Path sarif = Paths.get(file); assertThatThrownBy(() -> serializer.deserialize(sarif)) - .isInstanceOf(IllegalStateException.class) - .hasMessage(format("Failed to read SARIF report at '%s'", file)); + .isInstanceOf(NoSuchFileException.class); } @Test diff --git a/sonar-scanner-engine/src/main/java/org/sonar/scanner/externalissue/sarif/SarifIssuesImportSensor.java b/sonar-scanner-engine/src/main/java/org/sonar/scanner/externalissue/sarif/SarifIssuesImportSensor.java index 346cb727f7d..53a2a651dde 100644 --- a/sonar-scanner-engine/src/main/java/org/sonar/scanner/externalissue/sarif/SarifIssuesImportSensor.java +++ b/sonar-scanner-engine/src/main/java/org/sonar/scanner/externalissue/sarif/SarifIssuesImportSensor.java @@ -19,6 +19,7 @@ */ package org.sonar.scanner.externalissue.sarif; +import java.nio.file.NoSuchFileException; import java.nio.file.Path; import java.util.Arrays; import java.util.Collections; @@ -37,9 +38,12 @@ import org.sonar.api.config.PropertyDefinition; import org.sonar.api.resources.Qualifiers; import org.sonar.api.scanner.ScannerSide; import org.sonar.api.scanner.sensor.ProjectSensor; +import org.sonar.api.utils.MessageException; import org.sonar.core.sarif.Sarif210; import org.sonar.core.sarif.SarifSerializer; +import static java.lang.String.format; + @ScannerSide public class SarifIssuesImportSensor implements ProjectSensor { @@ -81,6 +85,8 @@ public class SarifIssuesImportSensor implements ProjectSensor { try { SarifImportResults sarifImportResults = processReport(context, reportPath); filePathToImportResults.put(reportPath, sarifImportResults); + } catch (NoSuchFileException e) { + throw MessageException.of(format("SARIF report file not found: %s", e.getFile())); } catch (Exception exception) { LOG.warn("Failed to process SARIF report from file '{}', error: '{}'", reportPath, exception.getMessage()); } @@ -92,7 +98,7 @@ public class SarifIssuesImportSensor implements ProjectSensor { return Arrays.stream(config.getStringArray(SARIF_REPORT_PATHS_PROPERTY_KEY)).collect(Collectors.toSet()); } - private SarifImportResults processReport(SensorContext context, String reportPath) { + private SarifImportResults processReport(SensorContext context, String reportPath) throws NoSuchFileException { LOG.debug("Importing SARIF issues from '{}'", reportPath); Path reportFilePath = context.fileSystem().resolvePath(reportPath).toPath(); Sarif210 sarifReport = sarifSerializer.deserialize(reportFilePath); diff --git a/sonar-scanner-engine/src/test/java/org/sonar/scanner/externalissue/sarif/SarifIssuesImportSensorTest.java b/sonar-scanner-engine/src/test/java/org/sonar/scanner/externalissue/sarif/SarifIssuesImportSensorTest.java index 6a85b604a86..f3181825034 100644 --- a/sonar-scanner-engine/src/test/java/org/sonar/scanner/externalissue/sarif/SarifIssuesImportSensorTest.java +++ b/sonar-scanner-engine/src/test/java/org/sonar/scanner/externalissue/sarif/SarifIssuesImportSensorTest.java @@ -20,6 +20,7 @@ package org.sonar.scanner.externalissue.sarif; import com.google.common.collect.MoreCollectors; +import java.nio.file.NoSuchFileException; import java.nio.file.Path; import java.util.Optional; import org.junit.Before; @@ -33,11 +34,13 @@ import org.sonar.api.batch.sensor.internal.SensorContextTester; import org.sonar.api.config.internal.MapSettings; import org.sonar.api.testfixtures.log.LogAndArguments; import org.sonar.api.testfixtures.log.LogTester; +import org.sonar.api.utils.MessageException; import org.sonar.api.utils.log.LoggerLevel; import org.sonar.core.sarif.Sarif210; import org.sonar.core.sarif.SarifSerializer; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; @@ -68,7 +71,7 @@ public class SarifIssuesImportSensorTest { private final SensorContextTester sensorContext = SensorContextTester.create(Path.of(".")); @Test - public void execute_whenSingleFileIsSpecified_shouldImportResults() { + public void execute_whenSingleFileIsSpecified_shouldImportResults() throws NoSuchFileException { sensorSettings.setProperty("sonar.sarifReportPaths", FILE_1); ReportAndResults reportAndResults = mockSuccessfulReportAndResults(FILE_1); @@ -83,7 +86,7 @@ public class SarifIssuesImportSensorTest { } @Test - public void execute_whenMultipleFilesAreSpecified_shouldImportResults() { + public void execute_whenMultipleFilesAreSpecified_shouldImportResults() throws NoSuchFileException { sensorSettings.setProperty("sonar.sarifReportPaths", SARIF_REPORT_PATHS_PARAM); ReportAndResults reportAndResults1 = mockSuccessfulReportAndResults(FILE_1); ReportAndResults reportAndResults2 = mockSuccessfulReportAndResults(FILE_2); @@ -99,7 +102,7 @@ public class SarifIssuesImportSensorTest { } @Test - public void execute_whenFileContainsOnlySuccessfulRuns_shouldLogCorrectMessage() { + public void execute_whenFileContainsOnlySuccessfulRuns_shouldLogCorrectMessage() throws NoSuchFileException { sensorSettings.setProperty("sonar.sarifReportPaths", FILE_1); ReportAndResults reportAndResults = mockSuccessfulReportAndResults(FILE_1); @@ -110,7 +113,7 @@ public class SarifIssuesImportSensorTest { } @Test - public void execute_whenFileContainsOnlyFailedRuns_shouldLogCorrectMessage() { + public void execute_whenFileContainsOnlyFailedRuns_shouldLogCorrectMessage() throws NoSuchFileException { sensorSettings.setProperty("sonar.sarifReportPaths", FILE_1); ReportAndResults reportAndResults = mockFailedReportAndResults(FILE_1); @@ -122,7 +125,7 @@ public class SarifIssuesImportSensorTest { } @Test - public void execute_whenFileContainsFailedAndSuccessfulRuns_shouldLogCorrectMessage() { + public void execute_whenFileContainsFailedAndSuccessfulRuns_shouldLogCorrectMessage() throws NoSuchFileException { sensorSettings.setProperty("sonar.sarifReportPaths", FILE_1); @@ -137,7 +140,7 @@ public class SarifIssuesImportSensorTest { } @Test - public void execute_whenImportFails_shouldSkipReport() { + public void execute_whenImportFails_shouldSkipReport() throws NoSuchFileException { sensorSettings.setProperty("sonar.sarifReportPaths", SARIF_REPORT_PATHS_PARAM); ReportAndResults reportAndResults1 = mockFailedReportAndResults(FILE_1); @@ -154,7 +157,7 @@ public class SarifIssuesImportSensorTest { } @Test - public void execute_whenDeserializationFails_shouldSkipReport() { + public void execute_whenDeserializationFails_shouldSkipReport() throws NoSuchFileException { sensorSettings.setProperty("sonar.sarifReportPaths", SARIF_REPORT_PATHS_PARAM); failDeserializingReport(FILE_1); @@ -168,12 +171,31 @@ public class SarifIssuesImportSensorTest { assertSummaryIsCorrectlyDisplayedForSuccessfulFile(FILE_2, reportAndResults2.getSarifImportResults()); } - private void failDeserializingReport(String path) { + @Test + public void execute_whenDeserializationThrowsMessageException_shouldRethrow() throws NoSuchFileException { + sensorSettings.setProperty("sonar.sarifReportPaths", FILE_1); + + NoSuchFileException e = new NoSuchFileException("non-existent"); + failDeserializingReportWithException(FILE_1, e); + + SarifIssuesImportSensor sensor = new SarifIssuesImportSensor(sarifSerializer, sarifImporter, sensorSettings.asConfig()); + assertThatThrownBy(() -> sensor.execute(sensorContext)) + .isInstanceOf(MessageException.class) + .hasMessage("SARIF report file not found: non-existent"); + + } + + private void failDeserializingReport(String path) throws NoSuchFileException { Path reportFilePath = sensorContext.fileSystem().resolvePath(path).toPath(); when(sarifSerializer.deserialize(reportFilePath)).thenThrow(new NullPointerException("deserialization failed")); } - private ReportAndResults mockSuccessfulReportAndResults(String path) { + private void failDeserializingReportWithException(String path, Exception exception) throws NoSuchFileException { + Path reportFilePath = sensorContext.fileSystem().resolvePath(path).toPath(); + when(sarifSerializer.deserialize(reportFilePath)).thenThrow(exception); + } + + private ReportAndResults mockSuccessfulReportAndResults(String path) throws NoSuchFileException { Sarif210 report = mockSarifReport(path); SarifImportResults sarifImportResults = mock(SarifImportResults.class); @@ -185,14 +207,14 @@ public class SarifIssuesImportSensorTest { return new ReportAndResults(report, sarifImportResults); } - private Sarif210 mockSarifReport(String path) { + private Sarif210 mockSarifReport(String path) throws NoSuchFileException { Sarif210 report = mock(Sarif210.class); Path reportFilePath = sensorContext.fileSystem().resolvePath(path).toPath(); when(sarifSerializer.deserialize(reportFilePath)).thenReturn(report); return report; } - private ReportAndResults mockFailedReportAndResults(String path) { + private ReportAndResults mockFailedReportAndResults(String path) throws NoSuchFileException { Sarif210 report = mockSarifReport(path); SarifImportResults sarifImportResults = mock(SarifImportResults.class); @@ -203,7 +225,7 @@ public class SarifIssuesImportSensorTest { return new ReportAndResults(report, sarifImportResults); } - private ReportAndResults mockMixedReportAndResults(String path) { + private ReportAndResults mockMixedReportAndResults(String path) throws NoSuchFileException { Sarif210 report = mockSarifReport(path); SarifImportResults sarifImportResults = mock(SarifImportResults.class); -- 2.39.5