Skip to content

Commit 8f3fcdf

Browse files
Copilotdavidfowl
Copilot
authored and
davidfowl
committed
Add IFileSystemService interfaces with adjusted APIs
Co-authored-by: davidfowl <[email protected]> Update DistributedApplicationBuilder to register and expose IFileSystemService Co-authored-by: davidfowl <[email protected]> Update AspireStore and Locations to use IFileSystemService Co-authored-by: davidfowl <[email protected]> Add implementation status document with remaining work Co-authored-by: davidfowl <[email protected]> Fix invalid XML doc comment syntax for TempFile.Dispose Co-authored-by: davidfowl <[email protected]> Simplify CreateTempFile API to take no parameters Co-authored-by: davidfowl <[email protected]> Fix test compilation errors - add experimental warnings and fix constructor calls Co-authored-by: davidfowl <[email protected]> Update high priority files to use IFileSystemService Co-authored-by: davidfowl <[email protected]> Add optional fileName parameter to CreateTempFile Co-authored-by: davidfowl <[email protected]> Rename shared TempDirectory helper class to TestTempDirectory Co-authored-by: davidfowl <[email protected]> Refactor file handling to use IFileSystemService for temporary file management in Maui environment annotations and MySql builder extensions Update implementation status to reflect IFileSystemService integration across MySQL and MAUI components
1 parent f3b59c4 commit 8f3fcdf

23 files changed

+513
-69
lines changed

‎IMPLEMENTATION_STATUS.md‎

Lines changed: 159 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,159 @@
1+
# Implementation Status for PR #13244 with Feedback Adjustments
2+
3+
This document tracks the implementation of IFileSystemService across the Aspire codebase.
4+
5+
## Completed Tasks
6+
7+
### Core API
8+
- ✅ Created `src/Aspire.Hosting/Utils/IFileSystemService.cs` with simplified API
9+
- `CreateTempFile(string? fileName = null)` method - optional fileName parameter for named temp files
10+
- `CreateTempSubdirectory(string? prefix = null)` for creating temp directories
11+
- Simplified XML documentation
12+
- ✅ Created `src/Aspire.Hosting/Utils/FileSystemService.cs` implementation
13+
- ✅ Updated `src/Aspire.Hosting/IDistributedApplicationBuilder.cs`
14+
- Added FileSystemService property
15+
- Removed misleading configuration override documentation
16+
- ✅ Updated `src/Aspire.Hosting/DistributedApplicationBuilder.cs`
17+
- Registered IFileSystemService in DI container
18+
- Exposed via FileSystemService property
19+
- Updated AspireStore DI registration
20+
- Updated Locations DI registration
21+
22+
### Aspire.Hosting Core Files
23+
- ✅ Updated `src/Aspire.Hosting/ApplicationModel/AspireStore.cs`
24+
- ✅ Updated `src/Aspire.Hosting/Dcp/Locations.cs`
25+
- ✅ Updated `src/Aspire.Hosting/Pipelines/PipelineOutputService.cs`
26+
- ✅ Updated `src/Aspire.Hosting/Dashboard/DashboardEventHandlers.cs`
27+
- ✅ Updated `src/Aspire.Hosting/ApplicationModel/ProjectResource.cs`
28+
- ✅ Updated `src/Aspire.Hosting/ContainerResourceBuilderExtensions.cs`
29+
30+
### MySQL Integration
31+
- ✅ Updated `src/Aspire.Hosting.MySql/MySqlBuilderExtensions.cs`
32+
- Added `#pragma warning disable ASPIREFILESYSTEM001`
33+
- Updated `WritePhpMyAdminConfiguration` to accept `IFileSystemService`
34+
- Replaced `Path.GetTempFileName()` with `fileSystemService.TempDirectory.CreateTempFile().Path`
35+
36+
### MAUI Support
37+
- ✅ Updated `src/Aspire.Hosting.Maui/Utilities/MauiEnvironmentHelper.cs`
38+
- Added `#pragma warning disable ASPIREFILESYSTEM001`
39+
- Updated both `CreateAndroidEnvironmentTargetsFileAsync` and `CreateiOSEnvironmentTargetsFileAsync` to accept `IFileSystemService`
40+
- Replaced `Path.Combine(Path.GetTempPath(), ...)` with `fileSystemService.TempDirectory.CreateTempSubdirectory(...).Path`
41+
- ✅ Updated `src/Aspire.Hosting.Maui/Utilities/MauiAndroidEnvironmentAnnotation.cs`
42+
- Added `#pragma warning disable ASPIREFILESYSTEM001`
43+
- Added `IFileSystemService` to `MauiAndroidEnvironmentSubscriber` constructor
44+
- ✅ Updated `src/Aspire.Hosting.Maui/Utilities/MauiiOSEnvironmentAnnotation.cs`
45+
- Added `#pragma warning disable ASPIREFILESYSTEM001`
46+
- Added `IFileSystemService` to `MauiiOSEnvironmentSubscriber` constructor
47+
48+
## Remaining Files to Update
49+
50+
### Aspire.Hosting (1 file)
51+
1. **UserSecretsManagerFactory.cs** (line 184)
52+
- Location: `src/Aspire.Hosting/UserSecrets/UserSecretsManagerFactory.cs`
53+
- Current: `Path.GetTempFileName()`
54+
- Change: Inject `IFileSystemService` and use `CreateTempFile()`
55+
56+
### Aspire.Hosting.Azure (2 files)
57+
2. **AzureBicepResource.cs** (line 162)
58+
- Location: `src/Aspire.Hosting.Azure/AzureBicepResource.cs`
59+
- Current: `Directory.CreateTempSubdirectory("aspire").FullName`
60+
- Challenge: Public method on resource class, not easy to inject services
61+
- Options:
62+
- Set internal `TempDirectory` property before calling
63+
- Add optional `IFileSystemService` parameter
64+
- Get from service provider in callers
65+
66+
3. **AzureProvisioningResource.cs** (line 83)
67+
- Location: `src/Aspire.Hosting.Azure/AzureProvisioningResource.cs`
68+
- Current: `Directory.CreateTempSubdirectory("aspire").FullName`
69+
- Same challenge as AzureBicepResource
70+
71+
### Aspire.Cli (4 files - Out of scope)
72+
The CLI project is separate from the hosting infrastructure. Consider creating a separate `ICliFileSystemService` if needed.
73+
74+
4. **InitCommand.cs** (line 265)
75+
- Current: `Path.Combine(Path.GetTempPath(), $"aspire-init-{Guid.NewGuid()}")`
76+
77+
5. **UpdateCommand.cs** (line 295)
78+
- Current: `Directory.CreateTempSubdirectory("aspire-cli-extract").FullName`
79+
80+
6. **CliDownloader.cs** (line 58)
81+
- Current: `Directory.CreateTempSubdirectory("aspire-cli-download").FullName`
82+
83+
7. **TemporaryNuGetConfig.cs** (line 22)
84+
- Current: `Path.Combine(Path.GetTempPath(), Path.GetRandomFileName())`
85+
86+
### Test Files (Many - Out of scope for this PR)
87+
Test files manage their own temp directories for isolation. Key files include:
88+
- `tests/Shared/TempDirectory.cs` - Shared test helper that creates temp directories
89+
- `tests/Shared/DistributedApplicationTestingBuilderExtensions.cs` - Test configuration
90+
- Various functional tests (Valkey, PostgreSQL, SqlServer, Oracle, MySQL, Redis, etc.)
91+
- Azure tests (AzureContainerAppsTests, AzureAppServiceTests, AzureManifestUtils)
92+
- CLI tests (ConsoleInteractionServiceTests, DiskCacheTests, etc.)
93+
94+
These tests typically use `Path.GetTempPath()` or `Directory.CreateTempSubdirectory()` directly for test isolation and cleanup
95+
96+
## Implementation Pattern
97+
98+
For each file, follow this pattern:
99+
100+
### 1. Add experimental warning suppression
101+
```csharp
102+
#pragma warning disable ASPIREFILESYSTEM001 // Type is for evaluation purposes only
103+
```
104+
105+
### 2. Inject or resolve IFileSystemService
106+
```csharp
107+
// Option A: Constructor injection (preferred)
108+
public MyClass(IFileSystemService fileSystemService)
109+
{
110+
_fileSystemService = fileSystemService;
111+
}
112+
113+
// Option B: Resolve from service provider
114+
var fileSystemService = serviceProvider.GetRequiredService<IFileSystemService>();
115+
116+
// Option C: Get from builder
117+
var fileSystemService = builder.ApplicationBuilder.FileSystemService;
118+
```
119+
120+
### 3. Replace temp file/directory APIs
121+
```csharp
122+
// OLD: Path.GetTempFileName()
123+
// NEW: fileSystemService.TempDirectory.CreateTempFile().Path
124+
125+
// OLD: Path.Combine(Path.GetTempPath(), "myfile.ext")
126+
// NEW: fileSystemService.TempDirectory.CreateTempFile("myfile.ext").Path
127+
128+
// OLD: Directory.CreateTempSubdirectory("prefix")
129+
// NEW: fileSystemService.TempDirectory.CreateTempSubdirectory("prefix").Path
130+
```
131+
132+
## Key Adjustments from Original PR #13244
133+
134+
1. ✅ **Simplified API with optional fileName**: `CreateTempFile(string? fileName = null)` - simple temp file creation with optional named file support
135+
2. ✅ **Removed Documentation**: Removed references to ASPIRE_TEMP_FOLDER and Aspire:TempDirectory configuration
136+
3. ✅ **Smart parent directory cleanup**: When fileName is provided, automatically cleans up parent directory on dispose
137+
4. ✅ **MySQL Integration**: Updated `MySqlBuilderExtensions.cs` to use IFileSystemService
138+
5. ✅ **MAUI Integration**: Updated all MAUI environment helper files to use IFileSystemService
139+
140+
## Testing Checklist
141+
142+
Before finalizing, ensure:
143+
- [x] All modified files build without warnings
144+
- [ ] Existing tests pass
145+
- [ ] New FileSystemServiceTests cover all scenarios
146+
- [ ] Test disposing TempDirectory and TempFile objects
147+
- [ ] Test the `.Path` property extraction pattern (common in codebase)
148+
- [ ] Integration tests with actual temp file operations
149+
- [ ] Verify no resource leaks (the .Path extraction pattern is intentional)
150+
- [ ] Test both CreateTempFile() and CreateTempFile("filename.ext") patterns
151+
152+
## Notes
153+
154+
- The pattern of extracting `.Path` and not disposing is **intentional** - many temp files/dirs persist for app lifetime
155+
- `CreateTempFile()` creates a simple temp file using Path.GetTempFileName()
156+
- `CreateTempFile("filename.ext")` creates a temp subdirectory with the named file inside, cleans up both on dispose
157+
- `CreateTempSubdirectory(prefix)` creates a temp directory that can be disposed to clean up
158+
- All APIs are marked `[Experimental("ASPIREFILESYSTEM001")]`
159+
- CLI files are out of scope for this PR as they don't use the hosting infrastructure

‎src/Aspire.Hosting.Maui/Utilities/MauiAndroidEnvironmentAnnotation.cs‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,12 @@
11
// Licensed to the .NET Foundation under one or more agreements.
22
// The .NET Foundation licenses this file to you under the MIT license.
33

4+
#pragma warning disable ASPIREFILESYSTEM001 // Type is for evaluation purposes only
5+
46
using Aspire.Hosting.ApplicationModel;
57
using Aspire.Hosting.Eventing;
68
using Aspire.Hosting.Lifecycle;
9+
using Microsoft.Extensions.DependencyInjection;
710
using Microsoft.Extensions.Logging;
811

912
namespace Aspire.Hosting.Maui.Utilities;
@@ -38,7 +41,8 @@ internal sealed class MauiAndroidEnvironmentProcessedAnnotation : IResourceAnnot
3841
internal sealed class MauiAndroidEnvironmentSubscriber(
3942
DistributedApplicationExecutionContext executionContext,
4043
ResourceLoggerService loggerService,
41-
ResourceNotificationService notificationService) : IDistributedApplicationEventingSubscriber
44+
ResourceNotificationService notificationService,
45+
IFileSystemService fileSystemService) : IDistributedApplicationEventingSubscriber
4246
{
4347
public Task SubscribeAsync(IDistributedApplicationEventing eventing, DistributedApplicationExecutionContext execContext, CancellationToken cancellationToken)
4448
{
@@ -83,6 +87,7 @@ private async Task OnBeforeResourceStartedAsync(BeforeResourceStartedEvent @even
8387
if (generatedFilePath is null)
8488
{
8589
generatedFilePath = await MauiEnvironmentHelper.CreateAndroidEnvironmentTargetsFileAsync(
90+
fileSystemService,
8691
resource,
8792
executionContext,
8893
logger,

‎src/Aspire.Hosting.Maui/Utilities/MauiEnvironmentHelper.cs‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
// Licensed to the .NET Foundation under one or more agreements.
22
// The .NET Foundation licenses this file to you under the MIT license.
33

4+
#pragma warning disable ASPIREFILESYSTEM001 // Type is for evaluation purposes only
5+
46
using System.Globalization;
57
using System.Text;
68
using System.Xml.Linq;
@@ -22,12 +24,14 @@ internal static class MauiEnvironmentHelper
2224
/// <summary>
2325
/// Creates an MSBuild targets file for Android that sets environment variables.
2426
/// </summary>
27+
/// <param name="fileSystemService">The file system service for managing temp files.</param>
2528
/// <param name="resource">The resource to collect environment variables from.</param>
2629
/// <param name="executionContext">The execution context.</param>
2730
/// <param name="logger">Logger for diagnostic output.</param>
2831
/// <param name="cancellationToken">Cancellation token.</param>
2932
/// <returns>The path to the generated targets file, or null if no environment variables are present.</returns>
3033
public static async Task<string?> CreateAndroidEnvironmentTargetsFileAsync(
34+
IFileSystemService fileSystemService,
3135
IResource resource,
3236
DistributedApplicationExecutionContext executionContext,
3337
ILogger logger,
@@ -62,8 +66,7 @@ internal static class MauiEnvironmentHelper
6266
}
6367

6468
// Create a temporary targets file
65-
var tempDirectory = Path.Combine(Path.GetTempPath(), "aspire", "maui", "android-env");
66-
Directory.CreateDirectory(tempDirectory);
69+
var tempDirectory = fileSystemService.TempDirectory.CreateTempSubdirectory("aspire-maui-android-env").Path;
6770

6871
// Prune old targets files
6972
PruneOldTargets(tempDirectory, logger);
@@ -209,12 +212,14 @@ private static string EncodeSemicolons(string value, out bool wasEncoded)
209212
/// <summary>
210213
/// Creates an MSBuild targets file for iOS that sets environment variables.
211214
/// </summary>
215+
/// <param name="fileSystemService">The file system service for managing temp files.</param>
212216
/// <param name="resource">The resource to collect environment variables from.</param>
213217
/// <param name="executionContext">The execution context.</param>
214218
/// <param name="logger">Logger for diagnostic output.</param>
215219
/// <param name="cancellationToken">Cancellation token.</param>
216220
/// <returns>The path to the generated targets file, or null if no environment variables are present.</returns>
217221
public static async Task<string?> CreateiOSEnvironmentTargetsFileAsync(
222+
IFileSystemService fileSystemService,
218223
IResource resource,
219224
DistributedApplicationExecutionContext executionContext,
220225
ILogger logger,
@@ -232,8 +237,7 @@ private static string EncodeSemicolons(string value, out bool wasEncoded)
232237
}
233238

234239
// Create a temporary targets file
235-
var tempDirectory = Path.Combine(Path.GetTempPath(), "aspire", "maui", "mlaunch-env");
236-
Directory.CreateDirectory(tempDirectory);
240+
var tempDirectory = fileSystemService.TempDirectory.CreateTempSubdirectory("aspire-maui-mlaunch-env").Path;
237241

238242
// Prune old targets files
239243
PruneOldTargetsiOS(tempDirectory, logger);

‎src/Aspire.Hosting.Maui/Utilities/MauiiOSEnvironmentAnnotation.cs‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,12 @@
11
// Licensed to the .NET Foundation under one or more agreements.
22
// The .NET Foundation licenses this file to you under the MIT license.
33

4+
#pragma warning disable ASPIREFILESYSTEM001 // Type is for evaluation purposes only
5+
46
using Aspire.Hosting.ApplicationModel;
57
using Aspire.Hosting.Eventing;
68
using Aspire.Hosting.Lifecycle;
9+
using Microsoft.Extensions.DependencyInjection;
710
using Microsoft.Extensions.Logging;
811

912
namespace Aspire.Hosting.Maui.Utilities;
@@ -38,7 +41,8 @@ internal sealed class MauiiOSEnvironmentProcessedAnnotation : IResourceAnnotatio
3841
internal sealed class MauiiOSEnvironmentSubscriber(
3942
DistributedApplicationExecutionContext executionContext,
4043
ResourceLoggerService loggerService,
41-
ResourceNotificationService notificationService) : IDistributedApplicationEventingSubscriber
44+
ResourceNotificationService notificationService,
45+
IFileSystemService fileSystemService) : IDistributedApplicationEventingSubscriber
4246
{
4347
public Task SubscribeAsync(IDistributedApplicationEventing eventing, DistributedApplicationExecutionContext execContext, CancellationToken cancellationToken)
4448
{
@@ -83,6 +87,7 @@ private async Task OnBeforeResourceStartedAsync(BeforeResourceStartedEvent @even
8387
if (generatedFilePath is null)
8488
{
8589
generatedFilePath = await MauiEnvironmentHelper.CreateiOSEnvironmentTargetsFileAsync(
90+
fileSystemService,
8691
resource,
8792
executionContext,
8893
logger,

‎src/Aspire.Hosting.MySql/MySqlBuilderExtensions.cs‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
// Licensed to the .NET Foundation under one or more agreements.
22
// The .NET Foundation licenses this file to you under the MIT license.
33

4+
#pragma warning disable ASPIREFILESYSTEM001 // Type is for evaluation purposes only
5+
46
using Aspire.Hosting;
57
using Aspire.Hosting.ApplicationModel;
68
using Aspire.Hosting.MySql;
@@ -260,7 +262,8 @@ public static IResourceBuilder<T> WithPhpMyAdmin<T>(this IResourceBuilder<T> bui
260262
}
261263
else
262264
{
263-
var tempConfigFile = await WritePhpMyAdminConfiguration(mySqlInstances, ct).ConfigureAwait(false);
265+
var fileSystemService = e.Services.GetRequiredService<IFileSystemService>();
266+
var tempConfigFile = await WritePhpMyAdminConfiguration(fileSystemService, mySqlInstances, ct).ConfigureAwait(false);
264267

265268
try
266269
{
@@ -374,10 +377,10 @@ public static IResourceBuilder<MySqlServerResource> WithInitFiles(this IResource
374377
return builder.WithContainerFiles(initPath, importFullPath);
375378
}
376379

377-
private static async Task<string> WritePhpMyAdminConfiguration(IEnumerable<MySqlServerResource> mySqlInstances, CancellationToken cancellationToken)
380+
private static async Task<string> WritePhpMyAdminConfiguration(IFileSystemService fileSystemService, IEnumerable<MySqlServerResource> mySqlInstances, CancellationToken cancellationToken)
378381
{
379382
// This temporary file is not used by the container, it will be copied and then deleted
380-
var filePath = Path.GetTempFileName();
383+
var filePath = fileSystemService.TempDirectory.CreateTempFile().Path;
381384

382385
using var writer = new StreamWriter(filePath);
383386

‎src/Aspire.Hosting/ApplicationModel/AspireStore.cs‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
// Licensed to the .NET Foundation under one or more agreements.
22
// The .NET Foundation licenses this file to you under the MIT license.
33

4+
#pragma warning disable ASPIREFILESYSTEM001 // Type is for evaluation purposes only
5+
46
using System.IO.Hashing;
57
using Aspire.Hosting.Utils;
68

@@ -11,22 +13,26 @@ internal sealed class AspireStore : IAspireStore
1113
internal const string AspireStorePathKeyName = "Aspire:Store:Path";
1214

1315
private readonly string _basePath;
16+
private readonly IFileSystemService _directoryService;
1417

1518
/// <summary>
1619
/// Initializes a new instance of the <see cref="AspireStore"/> class with the specified base path.
1720
/// </summary>
1821
/// <param name="basePath">The base path for the store.</param>
22+
/// <param name="directoryService">The directory service for creating temp directories.</param>
1923
/// <returns>A new instance of <see cref="AspireStore"/>.</returns>
20-
public AspireStore(string basePath)
24+
public AspireStore(string basePath, IFileSystemService directoryService)
2125
{
2226
ArgumentNullException.ThrowIfNull(basePath);
27+
ArgumentNullException.ThrowIfNull(directoryService);
2328

2429
if (!Path.IsPathRooted(basePath))
2530
{
2631
throw new ArgumentException($"An absolute path is required: '${basePath}'", nameof(basePath));
2732
}
2833

2934
_basePath = basePath;
35+
_directoryService = directoryService;
3036
EnsureDirectory();
3137
}
3238

@@ -43,7 +49,7 @@ public string GetFileNameWithContent(string filenameTemplate, Stream contentStre
4349
filenameTemplate = Path.GetFileName(filenameTemplate);
4450

4551
// Create a temporary file to write the content to.
46-
var tempFileName = Path.GetTempFileName();
52+
var tempFileName = _directoryService.TempDirectory.CreateTempFile().Path;
4753

4854
// Fast, non-cryptographic hash.
4955
var hash = new XxHash3();

‎src/Aspire.Hosting/ApplicationModel/ProjectResource.cs‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
#pragma warning disable ASPIREDOCKERFILEBUILDER001 // Type is for evaluation purposes only and is subject to change or removal in future updates. Suppress this diagnostic to proceed.
55
#pragma warning disable ASPIRECONTAINERRUNTIME001 // Type is for evaluation purposes only and is subject to change or removal in future updates. Suppress this diagnostic to proceed.
66
#pragma warning disable ASPIRECOMPUTE001 // Type is for evaluation purposes only and is subject to change or removal in future updates. Suppress this diagnostic to proceed.
7+
#pragma warning disable ASPIREFILESYSTEM001 // Type is for evaluation purposes only
78

89
// Licensed to the .NET Foundation under one or more agreements.
910
// The .NET Foundation licenses this file to you under the MIT license.
@@ -137,9 +138,10 @@ private async Task BuildProjectImage(PipelineStepContext ctx)
137138
// Add COPY --from: statements for each source
138139
stage.AddContainerFiles(this, containerWorkingDir, logger);
139140

140-
// Write the Dockerfile to a temporary location
141+
// Get the directory service to create temp Dockerfile
141142
var projectDir = Path.GetDirectoryName(projectMetadata.ProjectPath)!;
142-
var tempDockerfilePath = Path.GetTempFileName();
143+
var directoryService = ctx.Services.GetRequiredService<IFileSystemService>();
144+
var tempDockerfilePath = directoryService.TempDirectory.CreateTempFile().Path;
143145

144146
var builtSuccessfully = false;
145147
try

0 commit comments

Comments
 (0)