Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

SCAN4NET-158 Make file indexing more resilient #2261

Merged
merged 3 commits into from
Nov 7, 2024
Merged

SCAN4NET-158 Make file indexing more resilient #2261

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9fbe67b
SCAN4NET-158 Make file indexing more resilient
costin-zaharia-sonarsource Nov 5, 2024
a40ee0f
Address comments
costin-zaharia-sonarsource Nov 7, 2024
7b1891e
Use class based sut
costin-zaharia-sonarsource Nov 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 67 additions & 1 deletion Tests/SonarScanner.MSBuild.Shim.Test/AdditionalFilesServiceTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
using FluentAssertions;
using Microsoft.VisualStudio.TestTools.UnitTesting;
using NSubstitute;
using NSubstitute.ExceptionExtensions;
using SonarScanner.MSBuild.Common;
using TestUtilities;

Expand All @@ -43,7 +44,7 @@ public AdditionalFilesServiceTest()
wrapper
.EnumerateDirectories(ProjectBaseDir, "*", SearchOption.AllDirectories)
.Returns([]);
sut = new(wrapper);
sut = new(wrapper, logger);
}

[TestMethod]
Expand Down Expand Up @@ -344,4 +345,69 @@ public void AdditionalFiles_ExtensionsFound_MultipleProperties_NoAdditionalParam
"file12.test.TSx");
logger.AssertNoWarningsLogged();
}

[TestMethod]
public void AdditionalFiles_DirectoryAccessFail()
{
var directoryWrapper = Substitute.For<IDirectoryWrapper>();
directoryWrapper.EnumerateDirectories(ProjectBaseDir, "*", SearchOption.AllDirectories).Throws(_ => new DirectoryNotFoundException("Error message"));
var analysisConfig = new AnalysisConfig
{
ScanAllAnalysis = true,
LocalSettings = [],
ServerSettings = [new("sonar.typescript.file.suffixes", ".ts,.tsx")]
};

var sut = new AdditionalFilesService(directoryWrapper, logger);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you don't need to create the directoryWrapper and the sut, you can re-use the wrapper and sut initialized in the constructor (called once every method).

It's going to look like this, which is a bit simpler. Same in the second test.

        wrapper.EnumerateDirectories(ProjectBaseDir, "*", SearchOption.AllDirectories).Throws(_ => new DirectoryNotFoundException("Error message"));
        var analysisConfig = new AnalysisConfig
        {
            ScanAllAnalysis = true,
            LocalSettings = [],
            ServerSettings = [new("sonar.typescript.file.suffixes", ".ts,.tsx")]
        };

        var files = sut.AdditionalFiles(analysisConfig, ProjectBaseDir);

        files.Sources.Should().BeEmpty();
        files.Tests.Should().BeEmpty();
        wrapper.Received(1).EnumerateDirectories(ProjectBaseDir, "*", SearchOption.AllDirectories);
        logger.DebugMessages[0].Should().Be($"Reading directories from: '{ProjectBaseDir}'.");
        logger.DebugMessages[1].Should().MatchEquivalentOf(@"HResult: -2147024893, Exception: System.IO.DirectoryNotFoundException: Error message
   at NSubstitute.ExceptionExtensions.ExceptionExtensions.<>c__DisplayClass2_0.<Throws>b__0(CallInfo ci) *");
        logger.DebugMessages[2].Should().Be($"Reading files from: '{ProjectBaseDir}'.");
        logger.DebugMessages[3].Should().Be($"Found 0 files in: '{ProjectBaseDir}'.");
        logger.AssertSingleWarningExists($"Failed to get directories from: '{ProjectBaseDir}'.");

var files = sut.AdditionalFiles(analysisConfig, ProjectBaseDir);

files.Sources.Should().BeEmpty();
files.Tests.Should().BeEmpty();
directoryWrapper.Received(1).EnumerateDirectories(ProjectBaseDir, "*", SearchOption.AllDirectories);
logger.DebugMessages[0].Should().Be($"Reading directories from: '{ProjectBaseDir}'.");
logger.DebugMessages[1].Should().MatchEquivalentOf(@"HResult: -2147024893, Exception: System.IO.DirectoryNotFoundException: Error message
at NSubstitute.ExceptionExtensions.ExceptionExtensions.<>c__DisplayClass2_0.<Throws>b__0(CallInfo ci) *");
logger.DebugMessages[2].Should().Be($"Reading files from: '{ProjectBaseDir}'.");
logger.DebugMessages[3].Should().Be($"Found 0 files in: '{ProjectBaseDir}'.");
logger.AssertSingleWarningExists($"Failed to get directories from: '{ProjectBaseDir}'.");
}

[TestMethod]
public void AdditionalFiles_FileAccessFail()
{
var firstDirectory = new DirectoryInfo(Path.Combine(ProjectBaseDir.FullName, "first directory"));
var secondDirectory = new DirectoryInfo(Path.Combine(ProjectBaseDir.FullName, "second directory"));
var directoryWrapper = Substitute.For<IDirectoryWrapper>();
directoryWrapper.EnumerateDirectories(ProjectBaseDir, "*", SearchOption.AllDirectories).Returns([firstDirectory, secondDirectory]);
directoryWrapper.EnumerateFiles(ProjectBaseDir, "*", SearchOption.TopDirectoryOnly).Returns([new FileInfo("file in base dir.ts")]);
directoryWrapper.EnumerateFiles(firstDirectory, "*", SearchOption.TopDirectoryOnly).Throws(_ => new PathTooLongException("Error message"));
directoryWrapper.EnumerateFiles(secondDirectory, "*", SearchOption.TopDirectoryOnly).Returns([new FileInfo("file in second dir.ts")]);
var analysisConfig = new AnalysisConfig
{
ScanAllAnalysis = true,
LocalSettings = [],
ServerSettings = [new("sonar.typescript.file.suffixes", ".ts,.tsx")]
};

var sut = new AdditionalFilesService(directoryWrapper, logger);
var files = sut.AdditionalFiles(analysisConfig, ProjectBaseDir);

files.Sources.Select(x => x.Name).Should().BeEquivalentTo("file in base dir.ts", "file in second dir.ts");
files.Tests.Should().BeEmpty();
directoryWrapper.Received(1).EnumerateDirectories(ProjectBaseDir, "*", SearchOption.AllDirectories);
directoryWrapper.Received(3).EnumerateFiles(Arg.Any<DirectoryInfo>(), "*", SearchOption.TopDirectoryOnly);

logger.DebugMessages.Should().HaveCount(8);
logger.DebugMessages[0].Should().Be(@"Reading directories from: 'C:\dev'.");
logger.DebugMessages[1].Should().Be(@"Found 2 directories in: 'C:\dev'.");
logger.DebugMessages[2].Should().Be(@"Reading files from: 'C:\dev\first directory'.");
logger.DebugMessages[3].Should().MatchEquivalentOf(@"HResult: -2147024690, Exception: System.IO.PathTooLongException: Error message
at NSubstitute.ExceptionExtensions.ExceptionExtensions.<>c__DisplayClass2_0.<Throws>b__0(CallInfo ci) *");
logger.DebugMessages[4].Should().Be(@"Reading files from: 'C:\dev\second directory'.");
logger.DebugMessages[5].Should().Be(@"Found 1 files in: 'C:\dev\second directory'.");
logger.DebugMessages[6].Should().Be(@"Reading files from: 'C:\dev'.");
logger.DebugMessages[7].Should().Be(@"Found 1 files in: 'C:\dev'.");

logger.AssertSingleWarningExists(@"Failed to get files from: 'C:\dev\first directory'.");
}
}
2 changes: 1 addition & 1 deletion Tests/SonarScanner.MSBuild.Shim.Test/PropertiesFileGeneratorTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1546,7 +1546,7 @@ private PropertiesFileGenerator CreateSut(
{
sarifFixer ??= new RoslynV1SarifFixer(logger);
runtimeInformationWrapper ??= new RuntimeInformationWrapper();
additionalFileService ??= new AdditionalFilesService(DirectoryWrapper.Instance);
additionalFileService ??= new AdditionalFilesService(DirectoryWrapper.Instance, logger);
return new(analysisConfig, logger, sarifFixer, runtimeInformationWrapper, additionalFileService);
}
}
27 changes: 22 additions & 5 deletions src/SonarScanner.MSBuild.Shim/AdditionalFilesService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,9 @@ namespace SonarScanner.MSBuild.Shim;
// https://github.com/SonarSource/sonar-scanner-engine/blob/0d222f01c0b3a15e95c5c7d335d29c40ddf5d628/sonarcloud/sonar-scanner-engine/src/main/java/org/sonar/scanner/scan/filesystem/ProjectFilePreprocessor.java#L96
// and
// https://github.com/SonarSource/sonar-scanner-engine/blob/0d222f01c0b3a15e95c5c7d335d29c40ddf5d628/sonarcloud/sonar-scanner-engine/src/main/java/org/sonar/scanner/scan/filesystem/LanguageDetection.java#L70
public class AdditionalFilesService(IDirectoryWrapper directoryWrapper) : IAdditionalFilesService
public class AdditionalFilesService(IDirectoryWrapper directoryWrapper, ILogger logger) : IAdditionalFilesService
{
private const string SearchPatternAll = "*";
private static readonly char[] Comma = [','];

private static readonly IReadOnlyList<string> ExcludedDirectories =
Expand Down Expand Up @@ -87,15 +88,31 @@ public AdditionalFiles AdditionalFiles(AnalysisConfig analysisConfig, DirectoryI
: PartitionAdditionalFiles(GetAllFiles(extensions, projectBaseDir), analysisConfig);
}

private FileInfo[] GetAllFiles(IEnumerable<string> extensions, DirectoryInfo projectBaseDir) =>
directoryWrapper
.EnumerateDirectories(projectBaseDir, "*", SearchOption.AllDirectories)
private FileInfo[] GetAllFiles(IReadOnlyList<string> extensions, DirectoryInfo projectBaseDir) =>
CallDirectoryQuerySafe(projectBaseDir, "directories", () => directoryWrapper.EnumerateDirectories(projectBaseDir, SearchPatternAll, SearchOption.AllDirectories))
gregory-paidis-sonarsource marked this conversation as resolved.
Show resolved Hide resolved
.Concat([projectBaseDir]) // also include the root directory
.Where(x => !IsExcludedDirectory(x))
.SelectMany(x => directoryWrapper.EnumerateFiles(x, "*", SearchOption.TopDirectoryOnly))
.SelectMany(x => CallDirectoryQuerySafe(x, "files", () => directoryWrapper.EnumerateFiles(x, SearchPatternAll, SearchOption.TopDirectoryOnly)))
.Where(x => !IsExcludedFile(x) && extensions.Any(e => x.Name.EndsWith(e, StringComparison.OrdinalIgnoreCase) && !x.Name.Equals(e, StringComparison.OrdinalIgnoreCase)))
.ToArray();

private IReadOnlyList<T> CallDirectoryQuerySafe<T>(DirectoryInfo path, string entryType, Func<IEnumerable<T>> query)
{
try
{
logger.LogDebug("Reading {0} from: '{1}'.", entryType, path.FullName);
var result = query().ToArray();
logger.LogDebug("Found {0} {1} in: '{2}'.", result.Length, entryType, path.FullName);
return result;
}
catch (Exception exception)
{
logger.LogWarning(Resources.WARN_DirectoryGetContentFailure, entryType, path.FullName);
logger.LogDebug("HResult: {0}, Exception: {1}", exception.HResult, exception);
}
return Array.Empty<T>();
}

private static bool IsExcludedDirectory(DirectoryInfo directory) =>
ExcludedDirectories.Any(x => Array.Exists(
directory.FullName.Split(Path.DirectorySeparatorChar), // split it so that we also exclude subdirectories like .sonarqube/conf.
Expand Down
2 changes: 1 addition & 1 deletion src/SonarScanner.MSBuild.Shim/PropertiesFileGenerator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ public class PropertiesFileGenerator : IPropertiesFileGenerator
private readonly StringComparison pathComparison;

public PropertiesFileGenerator(AnalysisConfig analysisConfig, ILogger logger)
: this(analysisConfig, logger, new RoslynV1SarifFixer(logger), new RuntimeInformationWrapper(), new AdditionalFilesService(DirectoryWrapper.Instance))
: this(analysisConfig, logger, new RoslynV1SarifFixer(logger), new RuntimeInformationWrapper(), new AdditionalFilesService(DirectoryWrapper.Instance, logger))
{
}

Expand Down
19 changes: 18 additions & 1 deletion src/SonarScanner.MSBuild.Shim/Resources.Designer.cs
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions src/SonarScanner.MSBuild.Shim/Resources.resx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,9 @@ Possible causes:
<data name="WARN_DirectoryIsOutsideBaseDir" xml:space="preserve">
<value>Directory '{0}' is not located under the base directory '{1}' and will not be analyzed.</value>
</data>
<data name="WARN_DirectoryGetContentFailure" xml:space="preserve">
<value>Failed to get {0} from: '{1}'.</value>
</data>
<data name="MSG_UsingWorkingDirectoryAsProjectBaseDir" xml:space="preserve">
<value>Using working directory as project base directory: '{0}'.</value>
</data>
Expand Down