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

SLVS-1887 Use RelativePathHelper in ClientFileDtoFactory, skip files that can't be relativized #6049

Draft
wants to merge 1 commit into
base: gb/relative-path-calculation
Choose a base branch
from
Draft

SLVS-1887 Use RelativePathHelper in ClientFileDtoFactory, skip files that can't be relativized #6049

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
84 changes: 74 additions & 10 deletions src/Integration.UnitTests/LocalServices/FileTrackerTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
using SonarLint.VisualStudio.SLCore.Common.Helpers;
using SonarLint.VisualStudio.SLCore.Common.Models;
using SonarLint.VisualStudio.SLCore.Core;
using SonarLint.VisualStudio.SLCore.Listener.Files.Models;
using SonarLint.VisualStudio.SLCore.Service.File;
using SonarLint.VisualStudio.TestInfrastructure;

Expand All @@ -33,6 +34,9 @@ namespace SonarLint.VisualStudio.Integration.UnitTests.LocalServices;
[TestClass]
public class FileTrackerTests
{
const string configScopeId = "CONFIG_SCOPE_ID";
const string rootPath = "C:\\";

[TestMethod]
public void MefCtor_CheckExports()
{
Expand Down Expand Up @@ -65,21 +69,56 @@ public void AddFiles_ServiceProviderFailed_LogsError()
[TestMethod]
public void AddFiles_ShouldForwardFilesToSlCore()
{
var testSubject = CreateTestSubject(out var fileRpcSlCoreService);
var testSubject = CreateTestSubject(out var fileRpcSlCoreService, out var factory);
const string filePath = "C:\\Users\\test\\TestProject\\AFile.cs";
DidUpdateFileSystemParams result = null;
var clientFileDto = CreateDefaultClientFileDto();
SetUpDtoFactory(factory, filePath, clientFileDto);
fileRpcSlCoreService.DidUpdateFileSystem(Arg.Do<DidUpdateFileSystemParams>(parameters => result = parameters));

testSubject.AddFiles(new SourceFile("C:\\Users\\test\\TestProject\\AFile.cs"));
testSubject.AddFiles(new SourceFile(filePath));

result.removedFiles.Should().BeEmpty();
result.addedFiles.Should().BeEmpty();
result.changedFiles.Should().ContainSingle();
result.changedFiles.Should().BeEquivalentTo([clientFileDto]);
}

[TestMethod]
public void AddFiles_CanNotConvertSomeDtos_SkipsNotConvertedDtos()
{
var testSubject = CreateTestSubject(out var fileRpcSlCoreService, out var factory);
var filePath1 = "C:\\Users\\test\\TestProject\\AFile.cs";
var clientFileDto1 = CreateDefaultClientFileDto();
var filePath2 = "C:\\Users\\test\\TestProject\\BFile.cs";
DidUpdateFileSystemParams result = null;
SetUpDtoFactory(factory, filePath1, clientFileDto1);
SetUpDtoFactory(factory, filePath2, null);
fileRpcSlCoreService.DidUpdateFileSystem(Arg.Do<DidUpdateFileSystemParams>(parameters => result = parameters));

testSubject.AddFiles(new SourceFile(filePath1), new SourceFile(filePath2));

result.removedFiles.Should().BeEmpty();
result.addedFiles.Should().BeEmpty();
result.changedFiles.Should().BeEquivalentTo([clientFileDto1]);
}

[TestMethod]
public void AddFiles_CanNotCreateDto_ShouldNotNotifySlCore()
{
var testSubject = CreateTestSubject(out var fileRpcSlCoreService, out var factory);
var filePath = "C:\\Users\\test\\TestProject\\AFile.cs";
var clientFileDto = (ClientFileDto)null;
factory.CreateOrNull(configScopeId, rootPath, Arg.Is<SourceFile>(x => x.FilePath == filePath)).Returns(clientFileDto);

testSubject.AddFiles(new SourceFile(filePath));

fileRpcSlCoreService.DidNotReceiveWithAnyArgs().DidUpdateFileSystem(default);
}

[TestMethod]
public void RemoveFiles_ShouldForwardFilesToSlCore()
{
var testSubject = CreateTestSubject(out var fileRpcSlCoreService);
var testSubject = CreateTestSubject(out var fileRpcSlCoreService, out _);
DidUpdateFileSystemParams result = null;
fileRpcSlCoreService.DidUpdateFileSystem(Arg.Do<DidUpdateFileSystemParams>(parameters => result = parameters));

Expand All @@ -94,20 +133,45 @@ public void RemoveFiles_ShouldForwardFilesToSlCore()
[TestMethod]
public void RenameFiles_ShouldForwardFilesToSlCore()
{
var testSubject = CreateTestSubject(out var fileRpcSlCoreService);
const string renamedFilePath = "C:\\Users\\test\\TestProject\\ARenamedFile.cs";
var clientFileDto = CreateDefaultClientFileDto();
var testSubject = CreateTestSubject(out var fileRpcSlCoreService, out var factory);
SetUpDtoFactory(factory, renamedFilePath, clientFileDto);
DidUpdateFileSystemParams result = null;
fileRpcSlCoreService.DidUpdateFileSystem(Arg.Do<DidUpdateFileSystemParams>(parameters => result = parameters));

testSubject.RenameFiles(["C:\\Users\\test\\TestProject\\AFile.cs"],
[new SourceFile("C:\\Users\\test\\TestProject\\ARenamedFile.cs")]);
[new SourceFile(renamedFilePath)]);

result.removedFiles.Should().ContainSingle();
result.removedFiles[0].Should().BeEquivalentTo(new FileUri("C:\\Users\\test\\TestProject\\AFile.cs"));
result.addedFiles.Should().BeEmpty();
result.changedFiles.Should().ContainSingle();
result.changedFiles.Should().BeEquivalentTo([clientFileDto]);
}

private static FileTracker CreateTestSubject(out IFileRpcSLCoreService slCoreService)
[TestMethod]
public void RenameFiles_CanNotCreateDto_ShouldForwardOnlyRemovedFilesToSlCore()
{
const string renamedFilePath = "C:\\Users\\test\\TestProject\\ARenamedFile.cs";
var testSubject = CreateTestSubject(out var fileRpcSlCoreService, out var factory);
SetUpDtoFactory(factory, renamedFilePath, null);
DidUpdateFileSystemParams result = null;
fileRpcSlCoreService.DidUpdateFileSystem(Arg.Do<DidUpdateFileSystemParams>(parameters => result = parameters));

testSubject.RenameFiles(["C:\\Users\\test\\TestProject\\AFile.cs"],
[new SourceFile(renamedFilePath)]);

result.removedFiles.Should().ContainSingle();
result.removedFiles[0].Should().BeEquivalentTo(new FileUri("C:\\Users\\test\\TestProject\\AFile.cs"));
result.addedFiles.Should().BeEmpty();
result.changedFiles.Should().BeEmpty();
}

private static void SetUpDtoFactory(IClientFileDtoFactory factory, string filePath, ClientFileDto clientFileDto) => factory.CreateOrNull(configScopeId, rootPath, Arg.Is<SourceFile>(x => x.FilePath == filePath)).Returns(clientFileDto);

private static ClientFileDto CreateDefaultClientFileDto() => new(default, default, default, default, default, default);

private static FileTracker CreateTestSubject(out IFileRpcSLCoreService slCoreService, out IClientFileDtoFactory clientFileDtoFactory)
{
var serviceProvider = Substitute.For<ISLCoreServiceProvider>();
var fileRpcSlCoreService = Substitute.For<IFileRpcSLCoreService>();
Expand All @@ -119,8 +183,8 @@ private static FileTracker CreateTestSubject(out IFileRpcSLCoreService slCoreSer
slCoreService = fileRpcSlCoreService;

var activeConfigScopeTracker = Substitute.For<IActiveConfigScopeTracker>();
activeConfigScopeTracker.Current.Returns(new ConfigurationScope("CONFIG_SCOPE_ID", RootPath: "C:\\"));
var clientFileDtoFactory = Substitute.For<IClientFileDtoFactory>();
activeConfigScopeTracker.Current.Returns(new ConfigurationScope(configScopeId, RootPath: rootPath));
clientFileDtoFactory = Substitute.For<IClientFileDtoFactory>();

var threadHandling = Substitute.For<IThreadHandling>();
threadHandling.RunOnBackgroundThread(Arg.Any<Func<Task<int>>>())
Expand Down
17 changes: 12 additions & 5 deletions src/Integration/LocalServices/FileTracker.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,12 @@ public class FileTracker : IFileTracker
private readonly ILogger logger;

[ImportingConstructor]
public FileTracker(ISLCoreServiceProvider serviceProvider, IActiveConfigScopeTracker activeConfigScopeTracker,
IThreadHandling threadHandling, IClientFileDtoFactory clientFileDtoFactory, ILogger logger)
public FileTracker(
ISLCoreServiceProvider serviceProvider,
IActiveConfigScopeTracker activeConfigScopeTracker,
IThreadHandling threadHandling,
IClientFileDtoFactory clientFileDtoFactory,
ILogger logger)
{
this.serviceProvider = serviceProvider;
this.activeConfigScopeTracker = activeConfigScopeTracker;
Expand All @@ -69,15 +73,18 @@ public void RenameFiles(string[] beforeRenameFiles, SourceFile[] afterRenameFile

private void NotifySlCoreFilesChanged(string[] removedFiles, SourceFile[] addedOrChangedFiles)
{
if (serviceProvider.TryGetTransientService(out IFileRpcSLCoreService fileRpcSlCoreService) && activeConfigScopeTracker.Current is {} configScope)
if (serviceProvider.TryGetTransientService(out IFileRpcSLCoreService fileRpcSlCoreService) && activeConfigScopeTracker.Current is { } configScope)
{
var clientFiles = addedOrChangedFiles.Select(sourceFile => clientFileDtoFactory.Create(configScope.Id, configScope.RootPath, sourceFile)).ToList();
var clientFiles = addedOrChangedFiles.Select(sourceFile => clientFileDtoFactory.CreateOrNull(configScope.Id, configScope.RootPath, sourceFile)).Where(x => x is not null).ToList();
var removedFileUris = removedFiles.Select(f => new FileUri(f)).ToList();

/* we're only sending changed files here as it is complicated to implement the proper tracking of added files
AND `changed` files that were actually added are recognized as added by SLCore
https://github.com/SonarSource/sonarlint-core/pull/1163/files#diff-070e6ef952d4a71245d92ea8f281c5a56050e8992179cde3955d4b1530dff664R152 */
fileRpcSlCoreService.DidUpdateFileSystem(new DidUpdateFileSystemParams(removedFileUris, [], clientFiles));
if (removedFileUris.Any() || clientFiles.Any())
{
fileRpcSlCoreService.DidUpdateFileSystem(new DidUpdateFileSystemParams(removedFileUris, [], clientFiles));
}
}
else
{
Expand Down
12 changes: 12 additions & 0 deletions src/SLCore.Listeners.UnitTests/Implementation/ListFilesListenerTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
using SonarLint.VisualStudio.SLCore.Common.Helpers;
using SonarLint.VisualStudio.SLCore.Core;
using SonarLint.VisualStudio.SLCore.Listener.Files;
using SonarLint.VisualStudio.SLCore.Listener.Files.Models;

namespace SonarLint.VisualStudio.SLCore.Listeners.UnitTests.Implementation
{
Expand Down Expand Up @@ -148,6 +149,17 @@ public async Task ListFilesAsync_ConfigScopeChanged_ReturnsEmpty()
files.Should().BeEmpty();
}

private static void SetUpDtoFactory(
IClientFileDtoFactory factory,
string filePath,
ClientFileDto clientFileDto,
string rootPath,
string configScopeId = ConfigScopeId) => factory.CreateOrNull(configScopeId, rootPath, Arg.Is<SourceFile>(x => x.FilePath == filePath)).Returns(clientFileDto);

private static ClientFileDto CreateDefaultClientFileDto() => new(default, default, default, default, default, default);



private IFolderWorkspaceService CreateFolderWorkSpaceService(string root, params string[] files)
{
var folderWorkspaceService = Substitute.For<IFolderWorkspaceService>();
Expand Down
13 changes: 12 additions & 1 deletion src/SLCore.Listeners/Implementation/ListFilesListener.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,13 +59,24 @@ public Task<ListFilesResponse> ListFilesAsync(ListFilesParams parameters)
var root = GetRoot(fullFilePathList.First());
if (activeConfigScopeTracker.TryUpdateRootOnCurrentConfigScope(parameters.configScopeId, root))
{
clientFileDtos.AddRange(fullFilePathList.Select(fp => clientFileDtoFactory.Create(parameters.configScopeId, root, new SourceFile(fp))));
AddWorkspaceFilesDtos(parameters, clientFileDtos, root, fullFilePathList);
}
}
}
return Task.FromResult(new ListFilesResponse(clientFileDtos));
}

private void AddWorkspaceFilesDtos(
ListFilesParams parameters,
List<ClientFileDto> clientFileDtos,
string root,
IReadOnlyCollection<string> fullFilePathList) =>
clientFileDtos.AddRange(
fullFilePathList
// bug
.Select(fp => clientFileDtoFactory.CreateOrNull(parameters.configScopeId, root, new SourceFile(fp)))
.Where(x => x is not null));

private string GetRoot(string filePath)
{
var root = folderWorkspaceService.FindRootDirectory();
Expand Down
49 changes: 37 additions & 12 deletions src/SLCore.UnitTests/Common/Helpers/ClientFileDtoFactoryTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,19 +43,19 @@ public void MefCtor_CheckIsSingleton()
[DataRow(@"\\servername\user\projectA\directoryA\file.cs", @"\\servername\user\projectA\directoryA\", @"file.cs")]
public void Create_CalculatesCorrectRelativePath(string filePath, string rootPath, string expectedRelativePath)
{
var testSubject = new ClientFileDtoFactory();
var testSubject = new ClientFileDtoFactory(new TestLogger());

var result = testSubject.Create("CONFIG_SCOPE_ID", rootPath, new SourceFile(filePath));
var result = testSubject.CreateOrNull("CONFIG_SCOPE_ID", rootPath, new SourceFile(filePath));

result.ideRelativePath.Should().BeEquivalentTo(expectedRelativePath);
}

[TestMethod]
public void Create_ConstructsValidDto()
{
var testSubject = new ClientFileDtoFactory();
var testSubject = new ClientFileDtoFactory(new TestLogger());

var result = testSubject.Create("CONFIG_SCOPE_ID", @"C:\", new SourceFile(@"C:\Code\Project\File1.js"));
var result = testSubject.CreateOrNull("CONFIG_SCOPE_ID", @"C:\", new SourceFile(@"C:\Code\Project\File1.js"));

ValidateDto(result, @"C:\Code\Project\File1.js", @"Code\Project\File1.js");
}
Expand All @@ -65,43 +65,68 @@ public void Create_WithContent_ConstructsValidDto()
{
const string content = "somecontent";

var testSubject = new ClientFileDtoFactory();
var testSubject = new ClientFileDtoFactory(new TestLogger());

var result = testSubject.Create("CONFIG_SCOPE_ID", @"C:\", new SourceFile(@"C:\Code\Project\File1.js", content: content));
var result = testSubject.CreateOrNull("CONFIG_SCOPE_ID", @"C:\", new SourceFile(@"C:\Code\Project\File1.js", content: content));

ValidateDto(result, @"C:\Code\Project\File1.js", @"Code\Project\File1.js", expectedContent: content);
}

[TestMethod]
public void Create_WithLocalizedPath_ConstructsValidDto()
{
var testSubject = new ClientFileDtoFactory();
var testSubject = new ClientFileDtoFactory(new TestLogger());

var result = testSubject.Create("CONFIG_SCOPE_ID", @"C:\", new SourceFile(@"C:\привет\project\file1.js"));
var result = testSubject.CreateOrNull("CONFIG_SCOPE_ID", @"C:\", new SourceFile(@"C:\привет\project\file1.js"));

ValidateDto(result, @"C:\привет\project\file1.js", @"привет\project\file1.js");
}

[TestMethod]
public void Create_WithUNCPath_ConstructsValidDto()
{
var testSubject = new ClientFileDtoFactory();
var testSubject = new ClientFileDtoFactory(new TestLogger());

var result = testSubject.Create("CONFIG_SCOPE_ID", @"\\servername\work\", new SourceFile(@"\\servername\work\project\file1.js"));
var result = testSubject.CreateOrNull("CONFIG_SCOPE_ID", @"\\servername\work\", new SourceFile(@"\\servername\work\project\file1.js"));

ValidateDto(result, @"\\servername\work\project\file1.js", @"project\file1.js");
}

[TestMethod]
public void Create_WithWhitespacesPath_ConstructsValidDto()
{
var testSubject = new ClientFileDtoFactory();
var testSubject = new ClientFileDtoFactory(new TestLogger());

var result = testSubject.Create("CONFIG_SCOPE_ID", @"C:\", new SourceFile(@"C:\Code\My Project\My Favorite File2.js"));
var result = testSubject.CreateOrNull("CONFIG_SCOPE_ID", @"C:\", new SourceFile(@"C:\Code\My Project\My Favorite File2.js"));

ValidateDto(result, @"C:\Code\My Project\My Favorite File2.js", @"Code\My Project\My Favorite File2.js");
}

[TestMethod]
public void Create_WithPathAboveRoot_ConstructsValidDto()
{
var testSubject = new ClientFileDtoFactory(new TestLogger());

var result = testSubject.CreateOrNull("CONFIG_SCOPE_ID", @"C:\Code\OtherProject\", new SourceFile(@"C:\Code\Project\File1.js"));

ValidateDto(result, @"C:\Code\Project\File1.js", @"..\Project\File1.js");
}


[TestMethod]
public void Create_WithNonRelativezablePath_ReturnsNullAndLogs()
{
var testLogger = new TestLogger();
var testSubject = new ClientFileDtoFactory(testLogger);
const string filePath = @"C:\folder\project\file1.js";
const string rootPath = @"D:\";

var result = testSubject.CreateOrNull("CONFIG_SCOPE_ID", rootPath, new SourceFile(filePath));

result.Should().BeNull();
testLogger.AssertPartialOutputStringExists(string.Format(SLCoreStrings.ClientFile_NotRelative_Skipped, filePath, rootPath));
}

private static void ValidateDto(ClientFileDto actual, string expectedFsPath, string expectedIdeRelativePath, string expectedContent = null)
{
actual.Should().BeEquivalentTo(new ClientFileDto(
Expand Down
23 changes: 15 additions & 8 deletions src/SLCore/Common/Helpers/ClientFileDtoFactory.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,17 +27,24 @@ namespace SonarLint.VisualStudio.SLCore.Common.Helpers;

[Export(typeof(IClientFileDtoFactory))]
[PartCreationPolicy(CreationPolicy.Shared)]
public class ClientFileDtoFactory : IClientFileDtoFactory
[method: ImportingConstructor]
public class ClientFileDtoFactory(ILogger logger) : IClientFileDtoFactory
{
public ClientFileDto Create(string configScopeId, string rootPath, SourceFile sourceFile)
private readonly ILogger logger = logger.ForVerboseContext(nameof(ClientFileDtoFactory));

public ClientFileDto CreateOrNull(string configScopeId, string rootPath, SourceFile sourceFile)
{
var ideRelativePath = GetRelativePath(rootPath, sourceFile.FilePath);
var ideRelativePath = RelativePathHelper.GetRelativePathToRootFolder(rootPath, sourceFile.FilePath);

if (ideRelativePath == null)
{
logger.WriteLine(
new MessageLevelContext {Context = [configScopeId]},
SLCoreStrings.ClientFile_NotRelative_Skipped, sourceFile.FilePath, rootPath);
return null;
}

var uri = new FileUri(sourceFile.FilePath);
return new ClientFileDto(uri, ideRelativePath, configScopeId, null, sourceFile.Encoding, sourceFile.FilePath, sourceFile.Content);
}

private static string GetRelativePath(string root, string fullPath)
{
return fullPath.Substring(root.Length);
}
}
2 changes: 1 addition & 1 deletion src/SLCore/Common/Helpers/IClientFileDtoFactory.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,5 +25,5 @@ namespace SonarLint.VisualStudio.SLCore.Common.Helpers;

public interface IClientFileDtoFactory
{
ClientFileDto Create(string configScopeId, string rootPath, SourceFile sourceFile);
ClientFileDto CreateOrNull(string configScopeId, string rootPath, SourceFile sourceFile);
}
9 changes: 9 additions & 0 deletions src/SLCore/SLCoreStrings.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/SLCore/SLCoreStrings.resx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,4 +201,7 @@
<data name="RelativePathHelper_RootDoesNotEndWithSeparator" xml:space="preserve">
<value>Root path must end with a {0} separator</value>
</data>
<data name="ClientFile_NotRelative_Skipped" xml:space="preserve">
<value>File '{0}' can't be relativized against root '{1}' and is skipped</value>
</data>
</root>