Skip to content

Commit

Permalink
File I/O Abstraction (Part I) (#15568)
Browse files Browse the repository at this point in the history
This is the first PR in a series of changes to introduce a file I/O
abstraction layer to the Bicep project. Currently, `Bicep.Core` heavily
relies on the large `IFileSystem` interface, `System.Uri`. This tight
coupling introduces several challenges, such as making it difficult for
the Bicep resource provider to supply a mock implementation of
`IFileSystem`. Additionally, it prevents support for VS Code virtual
workspaces because `System.Uri` is surprisingly not fully RFC-compliant
(see: #11467).

To address these issues, a higher-level abstraction for file I/O is
required. Given the broad scope of this refactoring, this PR focuses on
laying the groundwork by introducing the new abstraction layer and using
it to load configuration. Subsequent PRs will replace the usage of
`IFileSystem` and `System.Uri` throughout Bicep.Core and tidy up the
`IFileResolver` interface with the new APIs.

## Key Changes in this PR
The core of this PR is the introduction of a new `Bicep.IO` project.
This project defines the new file I/O abstraction layer and includes:

- `Bicep.IO.Abstraction` types:
  - `IFileExplorer`: Encapsulates directory and file discovery.
- `IFileHandle`: A pointer to a file, abstracting operations like
reading or writing.
- `IDirectoryHandle`: A pointer to a directory for managing directory
traversal.
- `ResourceIdentifier`: An RFC3986 compliant URI (without Query and
Fragment) which is going to replace `System.Uri` in `Bicep.Core`.
- `Bicep.IO.FileSystem` types:
- Concrete implementations of the above interfaces based on
`IFileSystem`.

The ultimate objective is to phase out direct usage of
`Bicep.IO.FileSystem` within `Bicep.Core` and restrict it to referencing
only `Bicep.IO.Abstraction` via `BannedSymbols.txt`. This design ensures
cleaner separation of concerns and enables better testing, security, and
support for virtual environments.

Below is an architecture diagram illustrating the new design and how the
layers interact:

<img width="1446" alt="Untitled"
src="https://github.com/user-attachments/assets/a68a8c73-31ca-4b54-bb2e-d4dc221108bf">

###### Microsoft Reviewers: [Open in
CodeFlow](https://microsoft.github.io/open-pr/?codeflow=https://github.com/Azure/bicep/pull/15568)
  • Loading branch information
shenglol authored Nov 14, 2024
1 parent 9e3757b commit 4b190e2
Show file tree
Hide file tree
Showing 92 changed files with 2,077 additions and 564 deletions.
478 changes: 247 additions & 231 deletions Bicep.sln

Large diffs are not rendered by default.

45 changes: 23 additions & 22 deletions src/Bicep.Cli.IntegrationTests/JsonRpcCommandTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -191,35 +191,36 @@ await RunServerTest(
[TestMethod]
public async Task GetFileReferences_returns_all_referenced_files()
{
var fileSystem = new MockFileSystem(new Dictionary<string, MockFileData>
{
["/main.bicepparam"] = """
using 'main.bicep'

param foo = 'foo'
""",
["/main.bicep"] = """
param foo string

var test = loadTextContent('invalid.txt')
var test2 = loadTextContent('valid.txt')
""",
["/valid.txt"] = """
hello!
""",
["/bicepconfig.json"] = """
{}
""",
});
var fileSystem = new MockFileSystem(
new Dictionary<string, MockFileData>
{
["/main.bicepparam"] = """
using 'main.bicep'

param foo = 'foo'
""",
["/main.bicep"] = """
param foo string

var test = loadTextContent('invalid.txt')
var test2 = loadTextContent('valid.txt')
""",
["/valid.txt"] = """
hello!
""",
["/bicepconfig.json"] = """
{}
""",
});

await RunServerTest(
services => services.WithFileSystem(fileSystem),
async (client, token) =>
{
var response = await client.GetFileReferences(new("/main.bicepparam"), token);
response.FilePaths.Should().Equal([
"/bicepconfig.json",
response.FilePaths.Should().BeEquivalentTo([
fileSystem.Path.GetFullPath("/bicepconfig.json"),
"/invalid.txt",
"/main.bicep",
"/main.bicepparam",
Expand Down
7 changes: 7 additions & 0 deletions src/Bicep.Cli.IntegrationTests/packages.lock.json
Original file line number Diff line number Diff line change
Expand Up @@ -1585,6 +1585,7 @@
"Azure.Deployments.Templates": "[1.195.0, )",
"Azure.Identity": "[1.13.1, )",
"Azure.ResourceManager.Resources": "[1.9.0, )",
"Bicep.IO": "[1.0.0, )",
"JsonPatch.Net": "[3.1.1, )",
"JsonPath.Net": "[1.1.6, )",
"Microsoft.Extensions.Configuration": "[8.0.0, )",
Expand Down Expand Up @@ -1674,6 +1675,12 @@
"System.IO.Abstractions.TestingHelpers": "[21.0.29, )"
}
},
"bicep.io": {
"type": "Project",
"dependencies": {
"System.IO.Abstractions": "[21.0.29, )"
}
},
"bicep.langserver": {
"type": "Project",
"dependencies": {
Expand Down
7 changes: 7 additions & 0 deletions src/Bicep.Cli.UnitTests/packages.lock.json
Original file line number Diff line number Diff line change
Expand Up @@ -1457,6 +1457,7 @@
"Azure.Deployments.Templates": "[1.195.0, )",
"Azure.Identity": "[1.13.1, )",
"Azure.ResourceManager.Resources": "[1.9.0, )",
"Bicep.IO": "[1.0.0, )",
"JsonPatch.Net": "[3.1.1, )",
"JsonPath.Net": "[1.1.6, )",
"Microsoft.Extensions.Configuration": "[8.0.0, )",
Expand Down Expand Up @@ -1507,6 +1508,12 @@
"Sarif.Sdk": "[4.5.4, )",
"StreamJsonRpc": "[2.19.27, )"
}
},
"bicep.io": {
"type": "Project",
"dependencies": {
"System.IO.Abstractions": "[21.0.29, )"
}
}
},
"net8.0/linux-arm64": {
Expand Down
7 changes: 5 additions & 2 deletions src/Bicep.Cli/Helpers/ServiceCollectionExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,11 @@
using Bicep.Core.TypeSystem.Providers;
using Bicep.Core.Utils;
using Bicep.Decompiler;
using Bicep.IO.Abstraction;
using Bicep.IO.FileSystem;
using Microsoft.Extensions.DependencyInjection;
using Environment = Bicep.Core.Utils.Environment;
using IOFileSystem = System.IO.Abstractions.FileSystem;
using LocalFileSystem = System.IO.Abstractions.FileSystem;

namespace Bicep.Cli.Helpers;

Expand Down Expand Up @@ -67,7 +69,8 @@ public static IServiceCollection AddBicepCore(this IServiceCollection services)
.AddSingleton<ITokenCredentialFactory, TokenCredentialFactory>()
.AddSingleton<IFileResolver, FileResolver>()
.AddSingleton<IEnvironment, Environment>()
.AddSingleton<IFileSystem, IOFileSystem>()
.AddSingleton<IFileSystem, LocalFileSystem>()
.AddSingleton<IFileExplorer, FileSystemFileExplorer>()
.AddSingleton<IConfigurationManager, ConfigurationManager>()
.AddSingleton<IBicepAnalyzer, LinterAnalyzer>()
.AddSingleton<IFeatureProviderFactory, FeatureProviderFactory>()
Expand Down
11 changes: 9 additions & 2 deletions src/Bicep.Cli/Rpc/CliJsonRpcServer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -98,9 +98,16 @@ public async Task<GetFileReferencesResponse> GetFileReferences(GetFileReferences
{
fileUris.Add(otherModel.SourceFile.FileUri);
fileUris.UnionWith(otherModel.GetAuxiliaryFileReferences());
if (otherModel.Configuration.ConfigFileUri is { } configFileUri)
if (otherModel.Configuration.ConfigFileIdentifier is { } configFileIdentifier)
{
fileUris.Add(configFileUri);
var uri = new UriBuilder
{
Scheme = configFileIdentifier.Scheme,
Host = configFileIdentifier.Authority,
Path = configFileIdentifier.Path,
}.Uri;

fileUris.Add(uri);
}
}

Expand Down
7 changes: 7 additions & 0 deletions src/Bicep.Cli/packages.lock.json
Original file line number Diff line number Diff line change
Expand Up @@ -1354,6 +1354,7 @@
"Azure.Deployments.Templates": "[1.195.0, )",
"Azure.Identity": "[1.13.1, )",
"Azure.ResourceManager.Resources": "[1.9.0, )",
"Bicep.IO": "[1.0.0, )",
"JsonPatch.Net": "[3.1.1, )",
"JsonPath.Net": "[1.1.6, )",
"Microsoft.Extensions.Configuration": "[8.0.0, )",
Expand Down Expand Up @@ -1393,6 +1394,12 @@
"Google.Protobuf": "[3.28.3, )",
"Grpc.Net.Client": "[2.66.0, )"
}
},
"bicep.io": {
"type": "Project",
"dependencies": {
"System.IO.Abstractions": "[21.0.29, )"
}
}
},
"net8.0/linux-arm64": {
Expand Down
2 changes: 1 addition & 1 deletion src/Bicep.Core.IntegrationTests/ExtensionRegistryTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
using FluentAssertions.Execution;
using Microsoft.VisualStudio.TestTools.UnitTesting;
using Newtonsoft.Json.Linq;
using IOFileSystem = System.IO.Abstractions.FileSystem;
using LocalFileSystem = System.IO.Abstractions.FileSystem;

namespace Bicep.Core.IntegrationTests;

Expand Down
7 changes: 7 additions & 0 deletions src/Bicep.Core.IntegrationTests/packages.lock.json
Original file line number Diff line number Diff line change
Expand Up @@ -1505,6 +1505,7 @@
"Azure.Deployments.Templates": "[1.195.0, )",
"Azure.Identity": "[1.13.1, )",
"Azure.ResourceManager.Resources": "[1.9.0, )",
"Bicep.IO": "[1.0.0, )",
"JsonPatch.Net": "[3.1.1, )",
"JsonPath.Net": "[1.1.6, )",
"Microsoft.Extensions.Configuration": "[8.0.0, )",
Expand Down Expand Up @@ -1573,6 +1574,12 @@
"System.IO.Abstractions.TestingHelpers": "[21.0.29, )"
}
},
"bicep.io": {
"type": "Project",
"dependencies": {
"System.IO.Abstractions": "[21.0.29, )"
}
},
"bicep.langserver": {
"type": "Project",
"dependencies": {
Expand Down
7 changes: 7 additions & 0 deletions src/Bicep.Core.Samples/packages.lock.json
Original file line number Diff line number Diff line change
Expand Up @@ -1505,6 +1505,7 @@
"Azure.Deployments.Templates": "[1.195.0, )",
"Azure.Identity": "[1.13.1, )",
"Azure.ResourceManager.Resources": "[1.9.0, )",
"Bicep.IO": "[1.0.0, )",
"JsonPatch.Net": "[3.1.1, )",
"JsonPath.Net": "[1.1.6, )",
"Microsoft.Extensions.Configuration": "[8.0.0, )",
Expand Down Expand Up @@ -1562,6 +1563,12 @@
"System.IO.Abstractions.TestingHelpers": "[21.0.29, )"
}
},
"bicep.io": {
"type": "Project",
"dependencies": {
"System.IO.Abstractions": "[21.0.29, )"
}
},
"bicep.langserver": {
"type": "Project",
"dependencies": {
Expand Down
18 changes: 13 additions & 5 deletions src/Bicep.Core.UnitTests/BicepTestConstants.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System.Collections.Immutable;
using System.IO.Abstractions;
using System.IO.Abstractions.TestingHelpers;
using Azure.Containers.ContainerRegistry;
using Bicep.Core.Analyzers.Linter;
using Bicep.Core.Analyzers.Linter.Rules;
Expand All @@ -22,10 +23,12 @@
using Bicep.Core.UnitTests.Utils;
using Bicep.Core.Utils;
using Bicep.Core.Workspaces;
using Bicep.IO.Abstraction;
using Bicep.IO.FileSystem;
using Bicep.LanguageServer.Registry;
using Bicep.LanguageServer.Telemetry;
using Moq;
using IOFileSystem = System.IO.Abstractions.FileSystem;
using OnDiskFileSystem = System.IO.Abstractions.FileSystem;

namespace Bicep.Core.UnitTests
{
Expand All @@ -37,9 +40,12 @@ public static class BicepTestConstants

public const string GeneratorTemplateHashPath = "metadata._generator.templateHash";

public static readonly IFileSystem FileSystem = new IOFileSystem();
public static readonly IFileSystem FileSystem = new OnDiskFileSystem();

public static readonly FileResolver FileResolver = new(FileSystem);

public static readonly IFileExplorer fileExplorer = new FileSystemFileExplorer(FileSystem);

public static readonly FeatureProviderOverrides FeatureOverrides = new();

public static readonly ConfigurationManager ConfigurationManager = CreateFilesystemConfigurationManager();
Expand Down Expand Up @@ -89,7 +95,7 @@ public static IModuleDispatcher CreateModuleDispatcher(IServiceProvider services

public static readonly IModuleRestoreScheduler ModuleRestoreScheduler = CreateMockModuleRestoreScheduler();

public static RootConfiguration CreateMockConfiguration(Dictionary<string, object>? customConfigurationData = null, Uri? configFileUri = null)
public static RootConfiguration CreateMockConfiguration(Dictionary<string, object>? customConfigurationData = null, string? configFilePath = null)
{
var configurationData = new Dictionary<string, object>
{
Expand Down Expand Up @@ -120,10 +126,12 @@ public static RootConfiguration CreateMockConfiguration(Dictionary<string, objec
element = element.SetPropertyByPath(path, value);
}

return RootConfiguration.Bind(element, configFileUri);
ResourceIdentifier? configFileIdentifier = configFilePath is not null ? new ResourceIdentifier("file", "", configFilePath) : null;

return RootConfiguration.Bind(element, configFileIdentifier);
}

public static ConfigurationManager CreateFilesystemConfigurationManager() => new(new IOFileSystem());
public static ConfigurationManager CreateFilesystemConfigurationManager() => new(new FileSystemFileExplorer(new OnDiskFileSystem()));

public static IFeatureProviderFactory CreateFeatureProviderFactory(FeatureProviderOverrides featureOverrides, IConfigurationManager? configurationManager = null)
=> new OverriddenFeatureProviderFactory(new FeatureProviderFactory(configurationManager ?? CreateFilesystemConfigurationManager()), featureOverrides);
Expand Down
Loading

0 comments on commit 4b190e2

Please sign in to comment.