Skip to content

Commit

Permalink
Red 🔴, Green 🟢, Refactor (#13539)
Browse files Browse the repository at this point in the history 
Some minor refactorings, mostly renames

###### Microsoft Reviewers: [Open in
CodeFlow](https://microsoft.github.io/open-pr/?codeflow=https://github.com/Azure/bicep/pull/13539)

---------

Co-authored-by: Stephen Weatherford <Stephen.Weatherford.com>
  • Loading branch information
@StephenWeatherford
StephenWeatherford committed Mar 6, 2024
1 parent 6fe20a0 commit 9fd452e
Show file tree
Hide file tree
Showing 14 changed files with 105 additions and 116 deletions.
2 changes: 1 addition & 1 deletion src/Bicep.Cli.IntegrationTests/FormatCommandTests.cs
View file
Expand Up @@ -219,7 +219,7 @@ public async Task Format_WithOutdir_SavesFileToOutdir()
[DataTestMethod]
[DataRow(true)]
[DataRow(false)]
public async Task Format_WithInsertFinalNewlineOverride_SetsFinalNewlineAccordlingly(bool insertFinalNewline)
public async Task Format_WithInsertFinalNewlineOverride_SetsFinalNewlineAccordingly(bool insertFinalNewline)
{
var fileContentWithoutFinalNewline = """
var obj = {
Expand Down
16 changes: 8 additions & 8 deletions src/Bicep.Cli.IntegrationTests/RestoreCommandTests.cs
View file
Expand Up @@ -176,7 +176,7 @@ public async Task Restore_Artifacts_BackwardsAndForwardsCompatibility(string? me
mediaType,
artifactType,
configContents,
new (string, string)[] { (BicepModuleMediaTypes.BicepModuleLayerV1Json, "data") });
new (string, string)[] { (BicepMediaTypes.BicepModuleLayerV1Json, "data") });

client.Blobs.Should().HaveCount(2);
client.Manifests.Should().HaveCount(1);
Expand Down Expand Up @@ -215,12 +215,12 @@ public async Task Restore_Artifacts_BackwardsAndForwardsCompatibility(string? me

[DataTestMethod]
// *** Valid Cases ***
[DataRow(new string[] { BicepModuleMediaTypes.BicepModuleLayerV1Json }, null)]
[DataRow(new string[] { "unknown1", "unknown2", BicepModuleMediaTypes.BicepModuleLayerV1Json }, null)]
[DataRow(new string[] { "unknown1", BicepModuleMediaTypes.BicepModuleLayerV1Json, "unknown2" }, null)]
[DataRow(new string[] { BicepModuleMediaTypes.BicepModuleLayerV1Json, "unknown1", "unknown2" }, null)]
[DataRow(new string[] { BicepModuleMediaTypes.BicepModuleLayerV1Json, "unknown1", "unknown1", "unknown2", "unknown2" }, null)]
[DataRow(new string[] { BicepModuleMediaTypes.BicepModuleLayerV1Json, BicepMediaTypes.BicepProviderArtifactLayerV1TarGzip }, null)]
[DataRow(new string[] { BicepMediaTypes.BicepModuleLayerV1Json }, null)]
[DataRow(new string[] { "unknown1", "unknown2", BicepMediaTypes.BicepModuleLayerV1Json }, null)]
[DataRow(new string[] { "unknown1", BicepMediaTypes.BicepModuleLayerV1Json, "unknown2" }, null)]
[DataRow(new string[] { BicepMediaTypes.BicepModuleLayerV1Json, "unknown1", "unknown2" }, null)]
[DataRow(new string[] { BicepMediaTypes.BicepModuleLayerV1Json, "unknown1", "unknown1", "unknown2", "unknown2" }, null)]
[DataRow(new string[] { BicepMediaTypes.BicepModuleLayerV1Json, BicepMediaTypes.BicepProviderArtifactLayerV1TarGzip }, null)]
// *** Negative Cases ***
[DataRow(
new string[] { BicepMediaTypes.BicepProviderArtifactLayerV1TarGzip },
Expand All @@ -235,7 +235,7 @@ public async Task Restore_Artifacts_BackwardsAndForwardsCompatibility(string? me
new string[] { "unknown2", "unknown1" },
".*Expected to find a layer with media type application\\/vnd.ms.bicep.module.layer.v1\\+json, but found none.*")]
[DataRow(
new string[] { BicepModuleMediaTypes.BicepModuleLayerV1Json, BicepModuleMediaTypes.BicepModuleLayerV1Json },
new string[] { BicepMediaTypes.BicepModuleLayerV1Json, BicepMediaTypes.BicepModuleLayerV1Json },
$".*Did not expect to find multiple layer media types of application\\/vnd.ms.bicep.module.layer.v1\\+json")]
// TODO: doesn't work because provider error handling is still coupled with module error handling.
[DataRow(
Expand Down
2 changes: 1 addition & 1 deletion src/Bicep.Core.IntegrationTests/Emit/TemplateEmitterTests.cs
View file
Expand Up @@ -124,7 +124,7 @@ public async Task Source_map_generation_should_work(EmbeddedFile file)
emitResult.Status.Should().Be(EmitStatus.Succeeded);
emitResult.SourceMap.Should().NotBeNull();

// Here we simplfy verify that the format of the baseline file looks correct.
// Here we simply verify that the format of the baseline file looks correct.
var sourceMapJson = JToken.FromObject(emitResult.SourceMap!);
sourceMapFile.WriteToOutputFolder(sourceMapJson.ToString());
sourceMapFile.ShouldHaveExpectedJsonValue();
Expand Down
2 changes: 1 addition & 1 deletion src/Bicep.Core.UnitTests/Registry/MockRegistryBlobClient.cs
View file
Expand Up @@ -40,7 +40,7 @@ public MockRegistryBlobClient() : base()

public IDictionary<string, OciManifest> ModuleManifestObjects =>
ManifestObjects
.Where(kvp => kvp.Value.ArtifactType == BicepModuleMediaTypes.BicepModuleArtifactType)
.Where(kvp => kvp.Value.ArtifactType == BicepMediaTypes.BicepModuleArtifactType)
.ToDictionary(kvp => kvp.Key, kvp => kvp.Value);

public override async Task<Response<DownloadRegistryBlobResult>> DownloadBlobContentAsync(string digest, CancellationToken cancellationToken = default)
Expand Down
54 changes: 27 additions & 27 deletions ....UnitTests/Registry/SourceArchiveTests.cs → ...nitTests/SourceCode/SourceArchiveTests.cs
View file
Expand Up @@ -151,9 +151,9 @@ private SourceFileWithArtifactReference CreateSourceFile(MockFileSystem fs, Uri?
return new SourceFileWithArtifactReference(
sourceKind switch
{
SourceArchive.SourceKind_ArmTemplate => SourceFileFactory.CreateArmTemplateFile(uri, actualContents),
SourceArchive.SourceKind_Bicep => SourceFileFactory.CreateSourceFile(uri, actualContents),
SourceArchive.SourceKind_TemplateSpec => SourceFileFactory.CreateTemplateSpecFile(uri, actualContents),
SourceArchive.SourceKind.ArmTemplate => SourceFileFactory.CreateArmTemplateFile(uri, actualContents),
SourceArchive.SourceKind.Bicep => SourceFileFactory.CreateSourceFile(uri, actualContents),
SourceArchive.SourceKind.TemplateSpec => SourceFileFactory.CreateTemplateSpecFile(uri, actualContents),
_ => throw new Exception($"Unrecognized source kind: {sourceKind}")
},
artifactReference);
Expand All @@ -166,14 +166,14 @@ public void CanPackAndUnpackSourceFiles()
var fs = new MockFileSystem();
fs.AddDirectory(projectFolder.LocalPath);

var mainBicep = CreateSourceFile(fs, projectFolder, "main.bicep", SourceArchive.SourceKind_Bicep, MainDotBicepSource);
var mainJson = CreateSourceFile(fs, projectFolder, "main.json", SourceArchive.SourceKind_ArmTemplate, MainDotJsonSource);
var standaloneJson = CreateSourceFile(fs, projectFolder, "standalone.json", SourceArchive.SourceKind_ArmTemplate, StandaloneJsonSource);
var templateSpecMainJson = CreateSourceFile(fs, projectFolder, "Template spec 1.json", SourceArchive.SourceKind_TemplateSpec, TemplateSpecJsonSource);
var localModuleJson = CreateSourceFile(fs, projectFolder, "localModule.json", SourceArchive.SourceKind_ArmTemplate, LocalModuleDotJsonSource);
var templateSpecMainJson2 = CreateSourceFile(fs, projectFolder, "folder/template spec 2.json", SourceArchive.SourceKind_TemplateSpec, TemplateSpecJsonSource);
var mainBicep = CreateSourceFile(fs, projectFolder, "main.bicep", SourceArchive.SourceKind.Bicep, MainDotBicepSource);
var mainJson = CreateSourceFile(fs, projectFolder, "main.json", SourceArchive.SourceKind.ArmTemplate, MainDotJsonSource);
var standaloneJson = CreateSourceFile(fs, projectFolder, "standalone.json", SourceArchive.SourceKind.ArmTemplate, StandaloneJsonSource);
var templateSpecMainJson = CreateSourceFile(fs, projectFolder, "Template spec 1.json", SourceArchive.SourceKind.TemplateSpec, TemplateSpecJsonSource);
var localModuleJson = CreateSourceFile(fs, projectFolder, "localModule.json", SourceArchive.SourceKind.ArmTemplate, LocalModuleDotJsonSource);
var templateSpecMainJson2 = CreateSourceFile(fs, projectFolder, "folder/template spec 2.json", SourceArchive.SourceKind.TemplateSpec, TemplateSpecJsonSource);
var externalModuleJson = CreateSourceFile(fs, $"{CacheRoot}/br/mcr.microsoft.com/bicep$storage$storage-account/1.0.1$/main.json",
SourceArchive.SourceKind_ArmTemplate /* the actual source archived is the compiled JSON */, ExternalModuleDotJsonSource, "mcr.microsoft.com/bicep/storage/storage-account:1.0.1");
SourceArchive.SourceKind.ArmTemplate /* the actual source archived is the compiled JSON */, ExternalModuleDotJsonSource, "mcr.microsoft.com/bicep/storage/storage-account:1.0.1");


using var stream = SourceArchive.PackSourcesIntoStream(
Expand All @@ -189,12 +189,12 @@ public void CanPackAndUnpackSourceFiles()
archivedFiles.Should().BeEquivalentTo(
new SourceArchive.SourceFileInfo[] {
// Note: the template spec files will be filtered out
new ("main.bicep", "files/main.bicep", SourceArchive.SourceKind_Bicep, MainDotBicepSource, null),
new ("main.json", "files/main.json", SourceArchive.SourceKind_ArmTemplate, MainDotJsonSource, null),
new ("standalone.json", "files/standalone.json", SourceArchive.SourceKind_ArmTemplate, StandaloneJsonSource, null),
new ("localModule.json", "files/localModule.json", SourceArchive.SourceKind_ArmTemplate, LocalModuleDotJsonSource, null),
new ("main.bicep", "files/main.bicep", SourceArchive.SourceKind.Bicep, MainDotBicepSource, null),
new ("main.json", "files/main.json", SourceArchive.SourceKind.ArmTemplate, MainDotJsonSource, null),
new ("standalone.json", "files/standalone.json", SourceArchive.SourceKind.ArmTemplate, StandaloneJsonSource, null),
new ("localModule.json", "files/localModule.json", SourceArchive.SourceKind.ArmTemplate, LocalModuleDotJsonSource, null),
new ("<cache>/br/mcr.microsoft.com/bicep$storage$storage-account/1.0.1$/main.json", "files/_cache_/br/mcr.microsoft.com/bicep$storage$storage-account/1.0.1$/main.json",
SourceArchive.SourceKind_ArmTemplate, ExternalModuleDotJsonSource, OciRegistryHelper.ParseModuleReference("br:mcr.microsoft.com/bicep/storage/storage-account:1.0.1")),
SourceArchive.SourceKind.ArmTemplate, ExternalModuleDotJsonSource, OciRegistryHelper.ParseModuleReference("br:mcr.microsoft.com/bicep/storage/storage-account:1.0.1")),
});
}

Expand All @@ -204,14 +204,14 @@ public void CanPackAndUnpackDocumentLinks()
Uri projectFolder = PathHelper.FilePathToFileUrl($"{ROOT}my project/my sources/");
var fs = new MockFileSystem();
fs.AddDirectory(projectFolder.LocalPath);
var mainBicep = CreateSourceFile(fs, projectFolder, "main&.bicep", SourceArchive.SourceKind_Bicep, MainDotBicepSource);
var mainJson = CreateSourceFile(fs, projectFolder, "main.json", SourceArchive.SourceKind_ArmTemplate, MainDotJsonSource);
var standaloneJson = CreateSourceFile(fs, projectFolder, "standalone.json", SourceArchive.SourceKind_ArmTemplate, StandaloneJsonSource);
var templateSpecMainJson = CreateSourceFile(fs, projectFolder, "cache/wherever/template spec 1.json", SourceArchive.SourceKind_TemplateSpec, TemplateSpecJsonSource);
var localModuleJson = CreateSourceFile(fs, projectFolder, "modules/localJsonModule.json", SourceArchive.SourceKind_ArmTemplate, LocalModuleDotJsonSource);
var localModuleBicep = CreateSourceFile(fs, projectFolder, "modules/localBicepModule.bicep", SourceArchive.SourceKind_ArmTemplate, LocalModuleDotJsonSource);
var mainBicep = CreateSourceFile(fs, projectFolder, "main&.bicep", SourceArchive.SourceKind.Bicep, MainDotBicepSource);
var mainJson = CreateSourceFile(fs, projectFolder, "main.json", SourceArchive.SourceKind.ArmTemplate, MainDotJsonSource);
var standaloneJson = CreateSourceFile(fs, projectFolder, "standalone.json", SourceArchive.SourceKind.ArmTemplate, StandaloneJsonSource);
var templateSpecMainJson = CreateSourceFile(fs, projectFolder, "cache/wherever/template spec 1.json", SourceArchive.SourceKind.TemplateSpec, TemplateSpecJsonSource);
var localModuleJson = CreateSourceFile(fs, projectFolder, "modules/localJsonModule.json", SourceArchive.SourceKind.ArmTemplate, LocalModuleDotJsonSource);
var localModuleBicep = CreateSourceFile(fs, projectFolder, "modules/localBicepModule.bicep", SourceArchive.SourceKind.ArmTemplate, LocalModuleDotJsonSource);
var externalModuleJson = CreateSourceFile(fs, $"{CacheRoot}/br/mcr.microsoft.com/bicep$storage$storage-account/1.0.1$/main.json",
SourceArchive.SourceKind_ArmTemplate, ExternalModuleDotJsonSource, "mcr.microsoft.com/bicep/storage/storage-account:1.0.1");
SourceArchive.SourceKind.ArmTemplate, ExternalModuleDotJsonSource, "mcr.microsoft.com/bicep/storage/storage-account:1.0.1");

var linksInput = new Dictionary<Uri, SourceCodeDocumentUriLink[]>()
{
Expand Down Expand Up @@ -431,7 +431,7 @@ string[] expectedArchivePaths
{
entrypointFolder += Path.DirectorySeparatorChar;
}
var files = inputPaths.Select(path => CreateSourceFile(fs, path, SourceArchive.SourceKind_Bicep, $"// {path}")).ToArray();
var files = inputPaths.Select(path => CreateSourceFile(fs, path, SourceArchive.SourceKind.Bicep, $"// {path}")).ToArray();

using var stream = SourceArchive.PackSourcesIntoStream(files[0].SourceFile.FileUri, CacheRoot, files);
SourceArchive sourceArchive = SourceArchive.UnpackFromStream(stream).UnwrapOrThrow();
Expand Down Expand Up @@ -489,10 +489,10 @@ string[] expectedArchivePaths
var rootBicepFolder = new Uri(ROOT);
fs.AddDirectory(rootBicepFolder.LocalPath);

var entrypointFile = CreateSourceFile(fs, rootBicepFolder, entrypointPath, SourceArchive.SourceKind_Bicep, MainDotBicepSource);
var sutFile1 = CreateSourceFile(fs, rootBicepFolder, inputBicepPath1, SourceArchive.SourceKind_Bicep, SecondaryDotBicepSource);
var sutFile2 = CreateSourceFile(fs, rootBicepFolder, inputBicepPath2, SourceArchive.SourceKind_Bicep, SecondaryDotBicepSource);
var sutFile3 = inputBicepPath3 is null ? null : CreateSourceFile(fs, rootBicepFolder, inputBicepPath3, SourceArchive.SourceKind_Bicep, SecondaryDotBicepSource);
var entrypointFile = CreateSourceFile(fs, rootBicepFolder, entrypointPath, SourceArchive.SourceKind.Bicep, MainDotBicepSource);
var sutFile1 = CreateSourceFile(fs, rootBicepFolder, inputBicepPath1, SourceArchive.SourceKind.Bicep, SecondaryDotBicepSource);
var sutFile2 = CreateSourceFile(fs, rootBicepFolder, inputBicepPath2, SourceArchive.SourceKind.Bicep, SecondaryDotBicepSource);
var sutFile3 = inputBicepPath3 is null ? null : CreateSourceFile(fs, rootBicepFolder, inputBicepPath3, SourceArchive.SourceKind.Bicep, SecondaryDotBicepSource);

using var stream = sutFile3 is null ?
SourceArchive.PackSourcesIntoStream(entrypointFile.SourceFile.FileUri, CacheRoot, entrypointFile, sutFile1, sutFile2) :
Expand Down
2 changes: 1 addition & 1 deletion src/Bicep.Core.UnitTests/Utils/OciRegistryHelper.cs
View file
Expand Up @@ -120,7 +120,7 @@ public static async Task<(MockRegistryBlobClient, Mock<IContainerRegistryClientF
artifactReference: moduleReference,
mediaType: mediaType,
artifactType: artifactType,
config: new OciDescriptor(configContents ?? string.Empty, BicepModuleMediaTypes.BicepModuleConfigV1),
config: new OciDescriptor(configContents ?? string.Empty, BicepMediaTypes.BicepModuleConfigV1),
layers: layers.Select(layer => new OciDescriptor(layer.contents, layer.mediaType)),
new OciManifestAnnotationsBuilder()
);
Expand Down
18 changes: 0 additions & 18 deletions src/Bicep.Core/Modules/BicepModuleMediaTypes.cs
View file

This file was deleted.

8 changes: 4 additions & 4 deletions src/Bicep.Core/Modules/OciModuleArtifactResult.cs
View file
Expand Up @@ -15,19 +15,19 @@ public class OciModuleArtifactResult : OciArtifactResult
base(manifestBits, manifestDigest, layers)
{
var manifest = this.Manifest;
if (manifest.ArtifactType is not null && !manifest.ArtifactType.Equals(BicepModuleMediaTypes.BicepModuleArtifactType, MediaTypeComparison))
if (manifest.ArtifactType is not null && !manifest.ArtifactType.Equals(BicepMediaTypes.BicepModuleArtifactType, MediaTypeComparison))
{
throw new InvalidArtifactException(
$"Expected OCI manifest artifactType value of '{BicepModuleMediaTypes.BicepModuleArtifactType}' but found '{manifest.ArtifactType}'. {NewerVersionMightBeRequired}",
$"Expected OCI manifest artifactType value of '{BicepMediaTypes.BicepModuleArtifactType}' but found '{manifest.ArtifactType}'. {NewerVersionMightBeRequired}",
InvalidArtifactExceptionKind.WrongArtifactType);
}
if (manifest.Config.MediaType is not null && !manifest.Config.MediaType.Equals(BicepModuleMediaTypes.BicepModuleConfigV1, MediaTypeComparison))
if (manifest.Config.MediaType is not null && !manifest.Config.MediaType.Equals(BicepMediaTypes.BicepModuleConfigV1, MediaTypeComparison))
{
throw new InvalidArtifactException($"Did not expect config media type \"{manifest.Config.MediaType}\". {NewerVersionMightBeRequired}");
}

// Ignore layers we don't recognize for now.
var expectedLayerMediaType = BicepModuleMediaTypes.BicepModuleLayerV1Json;
var expectedLayerMediaType = BicepMediaTypes.BicepModuleLayerV1Json;
var mainLayers = this.Layers.Where(l => l.MediaType.Equals(expectedLayerMediaType, MediaTypeComparison));

this.mainLayer = mainLayers.Count() switch
Expand Down
2 changes: 1 addition & 1 deletion src/Bicep.Core/Registry/AzureContainerRegistryManager.cs
View file
Expand Up @@ -157,7 +157,7 @@ private static async Task<OciArtifactResult> DownloadManifestAndLayersAsync(IOci

return deserializedManifest.ArtifactType switch
{
BicepModuleMediaTypes.BicepModuleArtifactType or null => new OciModuleArtifactResult(manifestResponse.Value.Manifest, manifestResponse.Value.Digest, layers),
BicepMediaTypes.BicepModuleArtifactType or null => new OciModuleArtifactResult(manifestResponse.Value.Manifest, manifestResponse.Value.Digest, layers),
BicepMediaTypes.BicepProviderArtifactType => new OciProviderArtifactResult(manifestResponse.Value.Manifest, manifestResponse.Value.Digest, layers),
_ => throw new InvalidArtifactException($"artifacts of type: \'{deserializedManifest.ArtifactType}\' are not supported by this Bicep version. {OciModuleArtifactResult.NewerVersionMightBeRequired}")
};
Expand Down
2 changes: 1 addition & 1 deletion src/Bicep.Core/Registry/ModuleDispatcher.cs
View file
Expand Up @@ -292,7 +292,7 @@ private void SetRestoreFailure(ArtifactReference reference, RootConfiguration co
// as the user is typing, the modules will keep getting recompiled
// we can't keep retrying syntactically correct references to non-existent modules on every key press
// absolute expiration here will ensure that the next retry is delayed until the specified interval passes
// we're not not doing sliding expiration because we want a retry to happen eventually
// we're not doing sliding expiration because we want a retry to happen eventually
// (we may consider adding an ability to immediately retry to the UX in the future as well)
var expiration = DateTime.UtcNow.Add(FailureExpirationInterval);
this.restoreFailures.TryAdd(new(configuration.Cloud, reference), new RestoreFailureInfo(reference, failureBuilder, expiration));
Expand Down

0 comments on commit 9fd452e

Please sign in to comment.