Skip to content

Commit

Permalink
File I/O Abstraction: Replace IFileResolver.TryAcquireFileLock (#15826
Browse files Browse the repository at this point in the history
)

As part of the ongoing file I/O abstraction migration, this PR removes
`IFileResolver.TryAcquireFileLock` and transitions it to the new file
I/O API. The updates, including changes to `IFeatureProvider`,
`*ArtifactRegistry`, and several related tests, are inevitable side
effects of what initially seemed like a straightforward update.
###### Microsoft Reviewers: [Open in
CodeFlow](https://microsoft.github.io/open-pr/?codeflow=https://github.com/Azure/bicep/pull/15826)
  • Loading branch information
shenglol authored Dec 12, 2024
1 parent 3804703 commit c404992
Show file tree
Hide file tree
Showing 68 changed files with 620 additions and 364 deletions.
5 changes: 3 additions & 2 deletions src/Bicep.Cli.IntegrationTests/BuildCommandTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
using Bicep.Core.UnitTests.Mock;
using Bicep.Core.UnitTests.Registry;
using Bicep.Core.UnitTests.Utils;
using Bicep.IO.FileSystem;
using FluentAssertions;
using FluentAssertions.Execution;
using Microsoft.Extensions.DependencyInjection;
Expand Down Expand Up @@ -82,8 +83,8 @@ public async Task Build_Valid_SingleFile_WithTemplateSpecReference_ShouldSucceed
{
// ensure something got restored

Directory.Exists(settings.FeatureOverrides!.CacheRootDirectory).Should().BeTrue();
Directory.EnumerateFiles(settings.FeatureOverrides.CacheRootDirectory!, "*.json", SearchOption.AllDirectories).Should().NotBeEmpty();
settings.FeatureOverrides!.CacheRootDirectory!.Exists().Should().BeTrue();
Directory.EnumerateFiles(settings.FeatureOverrides.CacheRootDirectory!.Uri.GetFileSystemPath(), "*.json", SearchOption.AllDirectories).Should().NotBeEmpty();
}

var compiledFilePath = Path.Combine(outputDirectory, DataSet.TestFileMainCompiled);
Expand Down
23 changes: 13 additions & 10 deletions src/Bicep.Cli.IntegrationTests/RestoreCommandTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
using Bicep.Core.UnitTests.Mock;
using Bicep.Core.UnitTests.Registry;
using Bicep.Core.UnitTests.Utils;
using Bicep.IO.Abstraction;
using Bicep.IO.FileSystem;
using FluentAssertions;
using FluentAssertions.Execution;
using Microsoft.VisualStudio.TestTools.UnitTesting;
Expand Down Expand Up @@ -167,10 +169,9 @@ public async Task Restore_Artifacts_BackwardsAndForwardsCompatibility(string? me
var registry = "example.com";
var registryUri = new Uri("https://" + registry);
var repository = "hello/there";
var tempDirectory = FileHelper.GetUniqueTestOutputPath(TestContext);
var cacheRootDirectory = FileHelper.GetCacheRootDirectory(TestContext).EnsureExists();

var (client, clientFactory) = await OciRegistryHelper.PublishArtifactLayersToMockClient(
tempDirectory,
registry,
registryUri,
repository,
Expand All @@ -191,12 +192,13 @@ public async Task Restore_Artifacts_BackwardsAndForwardsCompatibility(string? me
}}
";

var restoreBicepFilePath = Path.Combine(tempDirectory, "restored.bicep");
File.WriteAllText(restoreBicepFilePath, bicep);
var restoredFile = cacheRootDirectory.GetFile("restored.bicep");
restoredFile.WriteAllText(bicep);

var restoredFilePath = restoredFile.Uri.GetFileSystemPath();
var settings = new InvocationSettings(new(TestContext, RegistryEnabled: true), clientFactory.Object, BicepTestConstants.TemplateSpecRepositoryFactory);

var (output, error, result) = await Bicep(settings, "restore", restoreBicepFilePath);
var (output, error, result) = await Bicep(settings, "restore", restoredFilePath);
using (new AssertionScope())
{
output.Should().BeEmpty();
Expand Down Expand Up @@ -248,10 +250,9 @@ public async Task Restore_Artifacts_LayerMediaTypes(string[] layerMediaTypes, st
var registryUri = new Uri("https://" + registry);
var repository = "hello/there";
var dataSet = DataSets.Empty;
var tempDirectory = FileHelper.GetUniqueTestOutputPath(TestContext);
var cacheRootDirectory = FileHelper.GetCacheRootDirectory(TestContext);

var (client, clientFactory) = await OciRegistryHelper.PublishArtifactLayersToMockClient(
tempDirectory,
registry,
registryUri,
repository,
Expand All @@ -271,12 +272,14 @@ public async Task Restore_Artifacts_LayerMediaTypes(string[] layerMediaTypes, st
}}
";

var restoreBicepFilePath = Path.Combine(tempDirectory, "restored.bicep");
File.WriteAllText(restoreBicepFilePath, bicep);
var restoredFile = cacheRootDirectory.GetFile("restored.bicep");
restoredFile.WriteAllText(bicep);

var restoredFilePath = restoredFile.Uri.GetFileSystemPath();

var settings = new InvocationSettings(new(TestContext, RegistryEnabled: true), clientFactory.Object, BicepTestConstants.TemplateSpecRepositoryFactory);

var (output, error, result) = await Bicep(settings, "restore", restoreBicepFilePath);
var (output, error, result) = await Bicep(settings, "restore", restoredFilePath);
using (new AssertionScope())
{
output.Should().BeEmpty();
Expand Down
3 changes: 2 additions & 1 deletion src/Bicep.Cli.IntegrationTests/packages.lock.json
Original file line number Diff line number Diff line change
Expand Up @@ -1691,7 +1691,8 @@
"bicep.io": {
"type": "Project",
"dependencies": {
"System.IO.Abstractions": "[21.1.3, )"
"System.IO.Abstractions": "[21.1.3, )",
"System.Memory.Data": "[6.0.0, )"
}
},
"bicep.langserver": {
Expand Down
3 changes: 2 additions & 1 deletion src/Bicep.Cli.UnitTests/packages.lock.json
Original file line number Diff line number Diff line change
Expand Up @@ -1525,7 +1525,8 @@
"bicep.io": {
"type": "Project",
"dependencies": {
"System.IO.Abstractions": "[21.1.3, )"
"System.IO.Abstractions": "[21.1.3, )",
"System.Memory.Data": "[6.0.0, )"
}
}
},
Expand Down
3 changes: 2 additions & 1 deletion src/Bicep.Cli/packages.lock.json
Original file line number Diff line number Diff line change
Expand Up @@ -1405,7 +1405,8 @@
"bicep.io": {
"type": "Project",
"dependencies": {
"System.IO.Abstractions": "[21.1.3, )"
"System.IO.Abstractions": "[21.1.3, )",
"System.Memory.Data": "[6.0.0, )"
}
}
},
Expand Down
7 changes: 2 additions & 5 deletions src/Bicep.Core.IntegrationTests/AzTypesViaRegistryTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,7 @@ private async Task<ServiceBuilder> GetServices()
{
var indexJson = FileHelper.SaveResultFile(TestContext, "types/index.json", EmptyIndexJson);

var cacheRoot = FileHelper.GetUniqueTestOutputPath(TestContext);
Directory.CreateDirectory(cacheRoot);

var cacheRoot = FileHelper.GetCacheRootDirectory(TestContext).EnsureExists();
var services = new ServiceBuilder()
.WithFeatureOverrides(new(ExtensibilityEnabled: true, CacheRootDirectory: cacheRoot))
.WithContainerRegistryClientFactory(RegistryHelper.CreateOciClientForAzExtension());
Expand All @@ -62,8 +60,7 @@ private async Task<ServiceBuilder> ServicesWithTestExtensionArtifact(ArtifactReg
var manifest = BicepTestConstants.GetBicepExtensionManifest(blobResult.Value, configResult.Value);
await client.SetManifestAsync(manifest, artifactRegistryAddress.ExtensionVersion);

var cacheRoot = FileHelper.GetUniqueTestOutputPath(TestContext);
Directory.CreateDirectory(cacheRoot);
var cacheRoot = FileHelper.GetCacheRootDirectory(TestContext).EnsureExists();

return new ServiceBuilder()
.WithFeatureOverrides(new(ExtensibilityEnabled: true, CacheRootDirectory: cacheRoot))
Expand Down
3 changes: 1 addition & 2 deletions src/Bicep.Core.IntegrationTests/ExtensionRegistryTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,7 @@ public async Task Extensions_published_to_a_registry_can_be_compiled()
[TestMethod]
public async Task Extensions_published_to_filesystem_can_be_compiled()
{
var cacheDirectory = FileHelper.GetCacheRootPath(TestContext);
Directory.CreateDirectory(cacheDirectory);
var cacheDirectory = FileHelper.GetCacheRootDirectory(TestContext).EnsureExists();
var services = new ServiceBuilder().WithFeatureOverrides(new(CacheRootDirectory: cacheDirectory, ExtensibilityEnabled: true));

var typesTgz = ThirdPartyTypeHelper.GetTestTypesTgz();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,7 @@ private async Task<ServiceBuilder> GetServices()
var indexJsonBeta = FileHelper.SaveResultFile(TestContext, "types/index-beta.json", EmptyIndexJsonBeta);
var indexJsonV10 = FileHelper.SaveResultFile(TestContext, "types/index-v1.0.json", EmptyIndexJsonV10);

var cacheRoot = FileHelper.GetUniqueTestOutputPath(TestContext);
Directory.CreateDirectory(cacheRoot);

var cacheRoot = FileHelper.GetCacheRootDirectory(TestContext).EnsureExists();
var services = new ServiceBuilder()
.WithFeatureOverrides(new(ExtensibilityEnabled: true, CacheRootDirectory: cacheRoot))
.WithContainerRegistryClientFactory(RegistryHelper.CreateOciClientForMsGraphExtension());
Expand All @@ -78,8 +76,7 @@ private async Task<ServiceBuilder> ServicesWithTestExtensionArtifact(ArtifactReg
var manifest = BicepTestConstants.GetBicepExtensionManifest(blobResult.Value, configResult.Value);
await client.SetManifestAsync(manifest, artifactRegistryAddress.ExtensionVersion);

var cacheRoot = FileHelper.GetUniqueTestOutputPath(TestContext);
Directory.CreateDirectory(cacheRoot);
var cacheRoot = FileHelper.GetCacheRootDirectory(TestContext).EnsureExists();

return new ServiceBuilder()
.WithFeatureOverrides(new(ExtensibilityEnabled: true, CacheRootDirectory: cacheRoot))
Expand Down
53 changes: 19 additions & 34 deletions src/Bicep.Core.IntegrationTests/RegistryTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
using Bicep.Core.UnitTests;
using Bicep.Core.UnitTests.Assertions;
using Bicep.Core.UnitTests.Utils;
using Bicep.IO.Abstraction;
using FluentAssertions;
using FluentAssertions.Execution;
using Microsoft.VisualStudio.TestTools.UnitTesting;
Expand Down Expand Up @@ -38,23 +39,21 @@ public async Task InvalidRootCachePathShouldProduceReasonableErrors()

var fileUri = PathHelper.FilePathToFileUrl(Path.Combine(outputDirectory, DataSet.TestFileMain));

var badCacheDirectory = FileHelper.GetCacheRootPath(TestContext);
Directory.CreateDirectory(badCacheDirectory);
var badCacheDirectory = FileHelper.GetCacheRootDirectory(TestContext).EnsureExists();

var badCachePath = Path.Combine(badCacheDirectory, "file.txt");
File.Create(badCachePath);
File.Exists(badCachePath).Should().BeTrue();
badCacheDirectory.GetFile("file.txt").EnsureExists();
badCacheDirectory = badCacheDirectory.GetDirectory("file.txt");

// cache root points to a file
var featureOverrides = BicepTestConstants.FeatureOverrides with
{
RegistryEnabled = true,
CacheRootDirectory = badCachePath
CacheRootDirectory = badCacheDirectory,
};
var featuresFactory = BicepTestConstants.CreateFeatureProviderFactory(featureOverrides);

var services = Services
.WithFeatureOverrides(new(RegistryEnabled: true, CacheRootDirectory: badCachePath))
.WithFeatureOverrides(new(RegistryEnabled: true, CacheRootDirectory: badCacheDirectory))
.WithContainerRegistryClientFactory(clientFactory)
.WithTemplateSpecRepositoryFactory(templateSpecRepositoryFactory)
.Build();
Expand Down Expand Up @@ -158,9 +157,7 @@ public async Task ModuleRestoreContentionShouldProduceConsistentState()
var templateSpecRepositoryFactory = dataSet.CreateMockTemplateSpecRepositoryFactory(TestContext);
await dataSet.PublishModulesToRegistryAsync(clientFactory, publishSource);

var cacheDirectory = FileHelper.GetCacheRootPath(TestContext);
Directory.CreateDirectory(cacheDirectory);

var cacheDirectory = FileHelper.GetCacheRootDirectory(TestContext).EnsureExists();
var services = Services
.WithFeatureOverrides(new(CacheRootDirectory: cacheDirectory))
.WithContainerRegistryClientFactory(clientFactory)
Expand Down Expand Up @@ -213,9 +210,7 @@ public async Task ModuleRestoreWithStuckFileLockShouldFailAfterTimeout(IEnumerab
var templateSpecRepositoryFactory = dataSet.CreateMockTemplateSpecRepositoryFactory(TestContext);
await dataSet.PublishModulesToRegistryAsync(clientFactory, publishSource: publishSource);

var cacheDirectory = FileHelper.GetCacheRootPath(TestContext);
Directory.CreateDirectory(cacheDirectory);

var cacheDirectory = FileHelper.GetCacheRootDirectory(TestContext).EnsureExists();
var fileResolver = BicepTestConstants.FileResolver;
var services = Services
.WithFeatureOverrides(new(CacheRootDirectory: cacheDirectory))
Expand Down Expand Up @@ -243,14 +238,11 @@ public async Task ModuleRestoreWithStuckFileLockShouldFailAfterTimeout(IEnumerab
dispatcher.TryGetLocalArtifactEntryPointUri(moduleReferences[0]).IsSuccess(out var moduleFileUri).Should().BeTrue();
moduleFileUri.Should().NotBeNull();

var moduleFilePath = moduleFileUri!.LocalPath;
var moduleDirectory = Path.GetDirectoryName(moduleFilePath)!;
Directory.CreateDirectory(moduleDirectory);

var lockFileName = Path.Combine(moduleDirectory, "lock");
var lockFileUri = new Uri(lockFileName);
var moduleFile = BicepTestConstants.FileExplorer.GetFile(IOUri.FromLocalFilePath(moduleFileUri!.LocalPath));
var moduleDirectory = moduleFile.GetParent().EnsureExists();
var lockFile = moduleDirectory.GetFile("lock");

var @lock = fileResolver.TryAcquireFileLock(lockFileUri);
var @lock = lockFile.TryLock();
@lock.Should().NotBeNull();

// let's try to restore a module while holding a lock
Expand Down Expand Up @@ -285,9 +277,7 @@ public async Task ForceModuleRestoreWithStuckFileLockShouldFailAfterTimeout(IEnu
var templateSpecRepositoryFactory = dataSet.CreateMockTemplateSpecRepositoryFactory(TestContext);
await dataSet.PublishModulesToRegistryAsync(clientFactory);

var cacheDirectory = FileHelper.GetCacheRootPath(TestContext);
Directory.CreateDirectory(cacheDirectory);

var cacheDirectory = FileHelper.GetCacheRootDirectory(TestContext).EnsureExists();
var fileResolver = BicepTestConstants.FileResolver;
var services = Services
.WithFeatureOverrides(new(CacheRootDirectory: cacheDirectory))
Expand All @@ -314,14 +304,11 @@ public async Task ForceModuleRestoreWithStuckFileLockShouldFailAfterTimeout(IEnu
dispatcher.TryGetLocalArtifactEntryPointUri(moduleReferences[0]).IsSuccess(out var moduleFileUri).Should().BeTrue();
moduleFileUri.Should().NotBeNull();

var moduleFilePath = moduleFileUri!.LocalPath;
var moduleDirectory = Path.GetDirectoryName(moduleFilePath)!;
Directory.CreateDirectory(moduleDirectory);

var lockFileName = Path.Combine(moduleDirectory, "lock");
var lockFileUri = new Uri(lockFileName);
var moduleFile = BicepTestConstants.FileExplorer.GetFile(IOUri.FromLocalFilePath(moduleFileUri!.LocalPath));
var moduleDirectory = moduleFile.GetParent().EnsureExists();
var lockFile = moduleDirectory.GetFile("lock");

var @lock = fileResolver.TryAcquireFileLock(lockFileUri);
var @lock = lockFile.TryLock();
@lock.Should().NotBeNull();

// let's try to restore a module while holding a lock
Expand All @@ -338,7 +325,7 @@ public async Task ForceModuleRestoreWithStuckFileLockShouldFailAfterTimeout(IEnu
dispatcher.GetArtifactRestoreStatus(moduleReferences[0], out var failureBuilder).Should().Be(ArtifactRestoreStatus.Failed);

failureBuilder!.Should().HaveCode("BCP233");
failureBuilder!.Should().HaveMessageStartWith($"Unable to delete the module with reference \"{moduleReferences[0].FullyQualifiedReference}\" from cache: Exceeded the timeout of \"00:00:05\" for the lock on file \"{lockFileUri}\" to be released.");
failureBuilder!.Should().HaveMessageStartWith($"Unable to delete the module with reference \"{moduleReferences[0].FullyQualifiedReference}\" from cache: Exceeded the timeout of \"00:00:05\" for the lock on file \"{lockFile.Uri}\" to be released.");
#else
dispatcher.GetArtifactRestoreStatus(moduleReferences[0], out _).Should().Be(ArtifactRestoreStatus.Succeeded);
#endif
Expand All @@ -363,9 +350,7 @@ public async Task ForceModuleRestoreShouldRestoreAllModules(IEnumerable<External
var templateSpecRepositoryFactory = dataSet.CreateMockTemplateSpecRepositoryFactory(TestContext);
await dataSet.PublishModulesToRegistryAsync(clientFactory, publishSource);

var cacheDirectory = FileHelper.GetCacheRootPath(TestContext);
Directory.CreateDirectory(cacheDirectory);

var cacheDirectory = FileHelper.GetCacheRootDirectory(TestContext).EnsureExists();
var fileResolver = BicepTestConstants.FileResolver;
var services = Services
.WithFeatureOverrides(new(CacheRootDirectory: cacheDirectory))
Expand Down
5 changes: 3 additions & 2 deletions src/Bicep.Core.IntegrationTests/SourceArchiveTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
using Bicep.Core.UnitTests.Features;
using Bicep.Core.UnitTests.Utils;
using Bicep.Core.Workspaces;
using Bicep.IO.Abstraction;
using Bicep.LanguageServer.Handlers;
using FluentAssertions;
using Microsoft.Extensions.DependencyInjection;
Expand All @@ -33,12 +34,12 @@ public class SourceArchiveTests : TestBase
#region Helpers

[NotNull]
private string? CacheRoot { get; set; }
private IDirectoryHandle? CacheRoot { get; set; }

[TestInitialize]
public void TestInitialize()
{
CacheRoot = FileHelper.GetUniqueTestOutputPath(TestContext);
CacheRoot = FileHelper.GetCacheRootDirectory(TestContext);
MockFileSystem = new(MockFiles);
}

Expand Down
3 changes: 2 additions & 1 deletion src/Bicep.Core.IntegrationTests/packages.lock.json
Original file line number Diff line number Diff line change
Expand Up @@ -1588,7 +1588,8 @@
"bicep.io": {
"type": "Project",
"dependencies": {
"System.IO.Abstractions": "[21.1.3, )"
"System.IO.Abstractions": "[21.1.3, )",
"System.Memory.Data": "[6.0.0, )"
}
},
"bicep.langserver": {
Expand Down
3 changes: 2 additions & 1 deletion src/Bicep.Core.Samples/packages.lock.json
Original file line number Diff line number Diff line change
Expand Up @@ -1577,7 +1577,8 @@
"bicep.io": {
"type": "Project",
"dependencies": {
"System.IO.Abstractions": "[21.1.3, )"
"System.IO.Abstractions": "[21.1.3, )",
"System.Memory.Data": "[6.0.0, )"
}
},
"bicep.langserver": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,7 @@ public void GetResourceTypeNames_SeparateScopes()

private ApiVersionProvider CreateDefaultApiVersionProvider(IEnumerable<ResourceTypeReference>? resourceTypeReferences = null)
=> new(
new FeatureProvider(
IConfigurationManager.GetBuiltInConfiguration()),
resourceTypeReferences ?? []);
new FeatureProvider(IConfigurationManager.GetBuiltInConfiguration(), BicepTestConstants.FileExplorer),
resourceTypeReferences ?? []);
}
}
Loading

0 comments on commit c404992

Please sign in to comment.