Skip to content

Commit 3144450

Browse files
IEvangelistCopilot
andcommitted
Address PR review feedback
Fix the AppHost workflow indentation, validate preview registrations against the configured repository, make registry writes durable, harden cleanup deletes, and align the Preview AppHost to the repo's current Aspire package versions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent f01db6b commit 3144450

File tree

5 files changed

+125
-39
lines changed

5 files changed

+125
-39
lines changed

.github/workflows/apphost-build.yml

Lines changed: 30 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -23,33 +23,33 @@ jobs:
2323
project_path: src/apphost/Aspire.Dev.Preview.AppHost
2424
artifact_name: preview-apphost-release
2525
steps:
26-
- uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
27-
28-
- uses: actions/setup-dotnet@c2fa09f4bde5ebb9d1777cf28262a3eb3db3ced7 # v5.2.0
29-
with:
30-
global-json-file: global.json
31-
32-
- name: Restore
33-
run: cd ${{ matrix.apphost.project_path }} && dotnet restore
34-
35-
- name: Build
36-
run: cd ${{ matrix.apphost.project_path }} && dotnet build --no-restore --configuration Release
37-
38-
- name: Verify output
39-
run: |
40-
APPHOST_DLL=$(ls -1 ${{ matrix.apphost.project_path }}/bin/Release/*/${{ matrix.apphost.project_name }}.dll 2>/dev/null | head -n 1)
41-
if [ -z "$APPHOST_DLL" ]; then
42-
echo "AppHost build failed - output assembly not found"
43-
ls -R ${{ matrix.apphost.project_path }}/bin/Release || true
44-
exit 1
45-
fi
46-
echo "Found $APPHOST_DLL"
47-
48-
- name: Upload artifact
49-
if: ${{ always() }}
50-
uses: actions/upload-artifact@bbbca2ddaa5d8feaa63e36b76fdaad77386f024f # v7.0.0
51-
with:
52-
name: ${{ matrix.apphost.artifact_name }}
53-
path: ${{ matrix.apphost.project_path }}/bin/Release/*/
54-
if-no-files-found: warn
55-
retention-days: 7
26+
- uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
27+
28+
- uses: actions/setup-dotnet@c2fa09f4bde5ebb9d1777cf28262a3eb3db3ced7 # v5.2.0
29+
with:
30+
global-json-file: global.json
31+
32+
- name: Restore
33+
run: cd ${{ matrix.apphost.project_path }} && dotnet restore
34+
35+
- name: Build
36+
run: cd ${{ matrix.apphost.project_path }} && dotnet build --no-restore --configuration Release
37+
38+
- name: Verify output
39+
run: |
40+
APPHOST_DLL=$(ls -1 ${{ matrix.apphost.project_path }}/bin/Release/*/${{ matrix.apphost.project_name }}.dll 2>/dev/null | head -n 1)
41+
if [ -z "$APPHOST_DLL" ]; then
42+
echo "AppHost build failed - output assembly not found"
43+
ls -R ${{ matrix.apphost.project_path }}/bin/Release || true
44+
exit 1
45+
fi
46+
echo "Found $APPHOST_DLL"
47+
48+
- name: Upload artifact
49+
if: ${{ always() }}
50+
uses: actions/upload-artifact@bbbca2ddaa5d8feaa63e36b76fdaad77386f024f # v7.0.0
51+
with:
52+
name: ${{ matrix.apphost.artifact_name }}
53+
path: ${{ matrix.apphost.project_path }}/bin/Release/*/
54+
if-no-files-found: warn
55+
retention-days: 7

src/apphost/Aspire.Dev.Preview.AppHost/Aspire.Dev.Preview.AppHost.csproj

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
<Project Sdk="Aspire.AppHost.Sdk/13.1.0">
1+
<Project Sdk="Aspire.AppHost.Sdk/13.2.0">
22

33
<PropertyGroup>
44
<OutputType>Exe</OutputType>
@@ -9,7 +9,7 @@
99
</PropertyGroup>
1010

1111
<ItemGroup>
12-
<PackageReference Include="Aspire.Hosting.Azure.AppService" Version="13.1.0-preview.1.25616.3" />
12+
<PackageReference Include="Aspire.Hosting.Azure.AppService" Version="13.2.0" />
1313
</ItemGroup>
1414

1515
<ItemGroup>

src/statichost/PreviewHost/Previews/PreviewCoordinator.cs

Lines changed: 38 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -810,19 +810,51 @@ private static string ResolveActivationSourceDirectory(string extractedRoot)
810810
return nestedDirectory?.Directory ?? extractedRoot;
811811
}
812812

813-
private static void DeleteFileIfPresent(string? path)
813+
private void DeleteFileIfPresent(string? path)
814814
{
815-
if (!string.IsNullOrWhiteSpace(path) && File.Exists(path))
815+
if (string.IsNullOrWhiteSpace(path))
816816
{
817-
File.Delete(path);
817+
return;
818+
}
819+
820+
try
821+
{
822+
if (File.Exists(path))
823+
{
824+
File.Delete(path);
825+
}
826+
}
827+
catch (IOException exception)
828+
{
829+
_logger.LogWarning(exception, "Failed to delete temporary preview file {Path}", path);
830+
}
831+
catch (UnauthorizedAccessException exception)
832+
{
833+
_logger.LogWarning(exception, "Failed to delete temporary preview file {Path}", path);
818834
}
819835
}
820836

821-
private static void DeleteDirectoryIfPresent(string? path)
837+
private void DeleteDirectoryIfPresent(string? path)
822838
{
823-
if (!string.IsNullOrWhiteSpace(path) && Directory.Exists(path))
839+
if (string.IsNullOrWhiteSpace(path))
840+
{
841+
return;
842+
}
843+
844+
try
845+
{
846+
if (Directory.Exists(path))
847+
{
848+
Directory.Delete(path, recursive: true);
849+
}
850+
}
851+
catch (IOException exception)
852+
{
853+
_logger.LogWarning(exception, "Failed to delete temporary preview directory {Directory}", path);
854+
}
855+
catch (UnauthorizedAccessException exception)
824856
{
825-
Directory.Delete(path, recursive: true);
857+
_logger.LogWarning(exception, "Failed to delete temporary preview directory {Directory}", path);
826858
}
827859
}
828860

src/statichost/PreviewHost/Previews/PreviewStateStore.cs

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -508,7 +508,19 @@ private async Task SaveLockedAsync(CancellationToken cancellationToken)
508508
{
509509
Directory.CreateDirectory(StateRoot);
510510
var json = JsonSerializer.Serialize(_records, JsonOptions);
511-
await File.WriteAllTextAsync(_registryPath, json, cancellationToken);
511+
var registryDirectory = Path.GetDirectoryName(_registryPath) ?? StateRoot;
512+
Directory.CreateDirectory(registryDirectory);
513+
514+
var tempFilePath = Path.Combine(registryDirectory, $"{Path.GetFileName(_registryPath)}.{Path.GetRandomFileName()}.tmp");
515+
try
516+
{
517+
await File.WriteAllTextAsync(tempFilePath, json, cancellationToken);
518+
File.Move(tempFilePath, _registryPath, overwrite: true);
519+
}
520+
finally
521+
{
522+
DeleteFileIfPresent(tempFilePath);
523+
}
512524
}
513525

514526
private bool NormalizeLoadedRecord(PreviewRecord record)
@@ -650,4 +662,23 @@ private void DeleteDirectoryIfPresent(string path)
650662
_logger.LogWarning(exception, "Failed to delete preview directory {Directory}", path);
651663
}
652664
}
665+
666+
private void DeleteFileIfPresent(string path)
667+
{
668+
try
669+
{
670+
if (File.Exists(path))
671+
{
672+
File.Delete(path);
673+
}
674+
}
675+
catch (IOException exception)
676+
{
677+
_logger.LogWarning(exception, "Failed to delete preview file {Path}", path);
678+
}
679+
catch (UnauthorizedAccessException exception)
680+
{
681+
_logger.LogWarning(exception, "Failed to delete preview file {Path}", path);
682+
}
683+
}
653684
}

src/statichost/PreviewHost/Program.cs

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -283,6 +283,11 @@
283283
return Results.ValidationProblem(validationErrors);
284284
}
285285

286+
if (!TryValidateRepository(request, options.Value, out var repositoryValidationErrors))
287+
{
288+
return Results.ValidationProblem(repositoryValidationErrors);
289+
}
290+
286291
var result = await stateStore.RegisterAsync(request, cancellationToken);
287292
return Results.Json(new
288293
{
@@ -331,6 +336,24 @@ static bool TryValidate(PreviewRegistrationRequest request, out Dictionary<strin
331336
return false;
332337
}
333338

339+
static bool TryValidateRepository(PreviewRegistrationRequest request, PreviewHostOptions options, out Dictionary<string, string[]> validationErrors)
340+
{
341+
if (string.Equals(request.RepositoryOwner, options.RepositoryOwner, StringComparison.OrdinalIgnoreCase)
342+
&& string.Equals(request.RepositoryName, options.RepositoryName, StringComparison.OrdinalIgnoreCase))
343+
{
344+
validationErrors = [];
345+
return true;
346+
}
347+
348+
var repositoryMessage = $"Preview registrations must target the configured repository '{options.RepositoryOwner}/{options.RepositoryName}'.";
349+
validationErrors = new Dictionary<string, string[]>(StringComparer.Ordinal)
350+
{
351+
[nameof(PreviewRegistrationRequest.RepositoryOwner)] = [repositoryMessage],
352+
[nameof(PreviewRegistrationRequest.RepositoryName)] = [repositoryMessage]
353+
};
354+
return false;
355+
}
356+
334357
static bool HasValidBearerToken(HttpRequest request, string expectedToken)
335358
{
336359
if (string.IsNullOrWhiteSpace(expectedToken))

0 commit comments

Comments
 (0)