Show compiled JSON first for now, and handle source code errors and c…

…ompatibility (#12814) CHANGES: 1) Error handling if the __metadata.json file format in a source code archive is incompatible with the current Bicep version 2) When pressing F12 on a module, for now show the main.json first instead of the Bicep, until we remove the experimental publish source flag. This makes it clear that if they click the "Show Bicep sources (experimental)" button they're hitting experimental territory. 3) UnpackFromStream now returns a ResultWithException<SourceArchive> instead of SourceArchive <img width="542" alt="image" src="https://github.com/Azure/bicep/assets/6913354/070d024a-7aec-4c3b-abdf-eaafa97c2724"> ###### Microsoft Reviewers: [Open in CodeFlow](https://microsoft.github.io/open-pr/?codeflow=https://github.com/Azure/bicep/pull/12814) --------- Co-authored-by: Stephen Weatherford <Stephen.Weatherford.com>
Azure · Jan 11, 2024 · 03d846a · 03d846a
1 parent 1389c59
commit 03d846a
Show file tree

Hide file tree

Showing 23 changed files with 299 additions and 109 deletions.
diff --git a/src/Bicep.Core.UnitTests/Assertions/CachedModuleExtensions.cs b/src/Bicep.Core.UnitTests/Assertions/CachedModuleExtensions.cs
@@ -1,6 +1,7 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT License.
 
+using System;
 using System.Collections.Generic;
 using System.Collections.Immutable;
 using System.IO;
@@ -10,6 +11,7 @@
 using Bicep.Core.Modules;
 using Bicep.Core.Registry;
 using Bicep.Core.Registry.Oci;
+using Bicep.Core.SourceCode;
 using Bicep.Core.UnitTests.Registry;
 using FluentAssertions;
 using FluentAssertions.Primitives;
@@ -31,19 +33,21 @@ public CachedModuleAssertions(CachedModule CachedModule) : base(CachedModule)
 
         protected override string Identifier => $"CachedModule at {Subject.ModuleCacheFolder}";
 
-        public AndConstraint<CachedModuleAssertions> HaveSource(bool f = true)
+        public AndConstraint<CachedModuleAssertions> HaveSource(bool shouldHaveSource = true)
         {
             Subject.Should().BeValid();
 
-            if (f)
+            if (shouldHaveSource)
             {
                 Subject.HasSourceLayer.Should().BeTrue();
-                Subject.TryGetSource().Should().NotBeNull();
+                Subject.TryGetSource().IsSuccess().Should().BeTrue();
             }
             else
             {
                 Subject.HasSourceLayer.Should().BeFalse();
-                Subject.TryGetSource().Should().BeNull();
+                Subject.TryGetSource().IsSuccess().Should().BeFalse();
+                Subject.TryGetSource().IsSuccess(out _, out var ex);
+                ex.Should().BeOfType<SourceNotAvailableException>();
             }
 
             return new(this);

diff --git a/src/Bicep.Core.UnitTests/Assertions/MockRegistryAssertions.cs b/src/Bicep.Core.UnitTests/Assertions/MockRegistryAssertions.cs
@@ -120,6 +120,7 @@ private AndConstraint<MockRegistryAssertions> HaveModuleCore(string tag, Stream
                                     expectedSourceContent!.Position = 0;
                                     byte[] expectedSourceBytes = new byte[expectedSourceContent.Length];
                                     expectedSourceContent.Read(expectedSourceBytes, 0, expectedSourceBytes.Length);
+                                    expectedSourceContent.Position = 0;
 
                                     actualSourcesBytes.Should().Equal(expectedSourceBytes, "module sources should match");
                                 }

diff --git a/src/Bicep.Core.UnitTests/Registry/CachedModules.cs b/src/Bicep.Core.UnitTests/Registry/CachedModules.cs
@@ -8,6 +8,7 @@
 using System.Text.Json;
 using System.Text.Json.Nodes;
 using System.Text.Json.Serialization;
+using Bicep.Core.Diagnostics;
 using Bicep.Core.Registry;
 using Bicep.Core.SourceCode;
 using FluentAssertions;
@@ -85,14 +86,14 @@ public string[] LayerMediaTypes
 
     public bool HasSourceLayer => LayerMediaTypes.Contains("application/vnd.ms.bicep.module.source.v1.tar+gzip");
 
-    public SourceArchive? TryGetSource()
+    public ResultWithException<SourceArchive> TryGetSource()
     {
         var sourceArchivePath = Path.Combine(ModuleCacheFolder, $"source.tar.gz");
         if (File.Exists(sourceArchivePath))
         {
-            return SourceArchive.FromStream(File.OpenRead(sourceArchivePath));
+            return SourceArchive.UnpackFromStream(File.OpenRead(sourceArchivePath));
         }
 
-        return null;
+        return new(new SourceNotAvailableException());
     }
 }
diff --git a/src/Bicep.Core.UnitTests/Registry/OciModuleRegistryTests.cs b/src/Bicep.Core.UnitTests/Registry/OciModuleRegistryTests.cs
@@ -678,16 +678,16 @@ public async Task RestoreModuleWithSource_ShouldRestoreSourceToDisk(bool publish
             await RestoreModule(ociRegistry, moduleReference);
 
             ociRegistry.Should().HaveValidCachedModules(withSource: publishSource);
-            var actualSource = ociRegistry.TryGetSource(moduleReference);
+            var actualSourceResult = ociRegistry.TryGetSource(moduleReference);
 
             if (sourceStream is { })
             {
-                actualSource.Should().NotBeNull();
-                actualSource.Should().BeEquivalentTo(SourceArchive.FromStream(sourceStream));
+                actualSourceResult.TryUnwrap().Should().NotBeNull();
+                actualSourceResult.Unwrap().Should().BeEquivalentTo(SourceArchive.UnpackFromStream(sourceStream).Unwrap());
             }
             else
             {
-                actualSource.Should().BeNull();
+                actualSourceResult.IsSuccess().Should().BeFalse();
             }
         }
 

diff --git a/src/Bicep.Core.UnitTests/Registry/SourceArchiveTests.cs b/src/Bicep.Core.UnitTests/Registry/SourceArchiveTests.cs
@@ -147,8 +147,9 @@ public void CanPackAndUnpackSourceFiles()
         using var stream = SourceArchive.PackSourcesIntoStream(mainBicep.FileUri, mainBicep, mainJson, standaloneJson, templateSpecMainJson, localModuleJson);
         stream.Length.Should().BeGreaterThan(0);
 
-        SourceArchive sourceArchive = SourceArchive.FromStream(stream);
-        sourceArchive.EntrypointRelativePath.Should().Be("main.bicep");
+        SourceArchive? sourceArchive = SourceArchive.UnpackFromStream(stream).TryUnwrap();
+        sourceArchive.Should().NotBeNull();
+        sourceArchive!.EntrypointRelativePath.Should().Be("main.bicep");
 
 
         var archivedFiles = sourceArchive.SourceFiles.ToArray();
@@ -210,9 +211,10 @@ public void HandlesPathsCorrectly(
 
         using var stream = SourceArchive.PackSourcesIntoStream(mainBicep.FileUri, mainBicep, testFile);
 
-        SourceArchive sourceArchive = SourceArchive.FromStream(stream);
+        SourceArchive? sourceArchive = SourceArchive.UnpackFromStream(stream).TryUnwrap();
 
-        sourceArchive.EntrypointRelativePath.Should().Be("my main.bicep");
+        sourceArchive.Should().NotBeNull();
+        sourceArchive!.EntrypointRelativePath.Should().Be("my main.bicep");
 
         var archivedTestFile = sourceArchive.SourceFiles.Single(f => f.Path != "my main.bicep");
         archivedTestFile.Path.Should().Be(expecteArchivedUri);
@@ -246,7 +248,7 @@ public void GetSourceFiles_ForwardsCompat_ShouldIgnoreUnrecognizedPropertiesInMe
             )
         );
 
-        var sut = SourceArchive.FromStream(zip);
+        var sut = SourceArchive.UnpackFromStream(zip).Unwrap();
         var file = sut.SourceFiles.Single();
 
         file.Kind.Should().Be("bicep");
@@ -280,7 +282,7 @@ public void GetSourceFiles_BackwardsCompat_ShouldBeAbleToReadOldFormats()
             )
         );
 
-        var sut = SourceArchive.FromStream(zip);
+        var sut = SourceArchive.UnpackFromStream(zip).Unwrap();
         var file = sut.SourceFiles.Single();
 
         file.Kind.Should().Be("bicep");
@@ -318,14 +320,78 @@ public void GetSourceFiles_ForwardsCompat_ShouldIgnoreFileEntriesNotInMetadata()
             )
         );
 
-        var sut = SourceArchive.FromStream(zip);
+        var sut = SourceArchive.UnpackFromStream(zip).Unwrap();
         var file = sut.SourceFiles.Single();
 
         file.Kind.Should().Be("bicep");
         file.Contents.Should().Be("bicep contents");
         file.Path.Should().Contain("main.bicep");
     }
 
+    [TestMethod]
+    public void GetSourceFiles_ShouldGiveError_ForIncompatibleOlderVersion()
+    {
+        var zip = CreateGzippedTarredFileStream(
+            (
+                "__metadata.json",
+                @"
+                {
+                  ""entryPoint"": ""file:///main.bicep"",
+                  ""metadataVersion"": <version>,
+                  ""bicepVersion"": ""0.whatever.0"",
+                  ""sourceFiles"": [
+                    {
+                      ""path"": ""file:///main.bicep"",
+                      ""archivePath"": ""main.bicep"",
+                      ""kind"": ""bicep""
+                    }
+                  ]
+                }".Replace("<version>", (SourceArchive.CurrentMetadataVersion - 1).ToString())
+            ),
+            (
+                "main.bicep",
+                @"bicep contents"
+            )
+        );
+
+        SourceArchive.UnpackFromStream(zip).IsSuccess(out var sourceArchive, out var ex);
+        sourceArchive.Should().BeNull();
+        ex.Should().NotBeNull();
+        ex!.Message.Should().StartWith("This source code was published with an older, incompatible version of Bicep (0.whatever.0). You are using version ");
+    }
+
+    [TestMethod]
+    public void GetSourceFiles_ShouldGiveError_ForIncompatibleNewerVersion()
+    {
+        var zip = CreateGzippedTarredFileStream(
+            (
+                "__metadata.json",
+                @"
+                {
+                  ""entryPoint"": ""file:///main.bicep"",
+                  ""metadataVersion"": <version>,
+                  ""bicepVersion"": ""0.whatever.0"",
+                  ""sourceFiles"": [
+                    {
+                      ""path"": ""file:///main.bicep"",
+                      ""archivePath"": ""main.bicep"",
+                      ""kind"": ""bicep""
+                    }
+                  ]
+                }".Replace("<version>", (SourceArchive.CurrentMetadataVersion + 1).ToString())
+            ),
+            (
+                "main.bicep",
+                @"bicep contents"
+            )
+        );
+
+        var success = SourceArchive.UnpackFromStream(zip).IsSuccess(out _, out var ex);
+        success.Should().BeFalse();
+        ex.Should().NotBeNull();
+        ex!.Message.Should().StartWith("This source code was published with a newer, incompatible version of Bicep (0.whatever.0). You are using version ");
+    }
+
     private Stream CreateGzippedTarredFileStream(params (string relativePath, string contents)[] files)
     {
         var outFolder = FileHelper.GetUniqueTestOutputPath(TestContext!);

diff --git a/src/Bicep.Core/Diagnostics/ResultWithException.cs b/src/Bicep.Core/Diagnostics/ResultWithException.cs
@@ -0,0 +1,15 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT License.
+
+using System;
+using Bicep.Core.Utils;
+
+namespace Bicep.Core.Diagnostics;
+
+public class ResultWithException<TSuccess> : Result<TSuccess, Exception>
+    where TSuccess : class
+{
+    public ResultWithException(TSuccess success) : base(success) { }
+
+    public ResultWithException(Exception exception) : base(exception) { }
+}
diff --git a/src/Bicep.Core/Registry/ArtifactRegistry.cs b/src/Bicep.Core/Registry/ArtifactRegistry.cs
@@ -8,6 +8,7 @@
 using System.Threading.Tasks;
 using Bicep.Core.Diagnostics;
 using Bicep.Core.SourceCode;
+using Bicep.Core.Utils;
 
 namespace Bicep.Core.Registry
 {
@@ -37,7 +38,7 @@ public abstract class ArtifactRegistry<T> : IArtifactRegistry where T : Artifact
 
         public abstract Task<string?> TryGetDescription(T reference);
 
-        public abstract SourceArchive? TryGetSource(T reference);
+        public abstract ResultWithException<SourceArchive> TryGetSource(T reference);
 
         public bool IsArtifactRestoreRequired(ArtifactReference reference) => this.IsArtifactRestoreRequired(ConvertReference(reference));
 
@@ -62,7 +63,7 @@ public ResultWithDiagnostic<Uri> TryGetLocalArtifactEntryPointUri(ArtifactRefere
 
         public async Task<string?> TryGetDescription(ArtifactReference reference) => await this.TryGetDescription(ConvertReference(reference));
 
-        public SourceArchive? TryGetSource(ArtifactReference reference) => this.TryGetSource(ConvertReference(reference));
+        public ResultWithException<SourceArchive> TryGetSource(ArtifactReference reference) => this.TryGetSource(ConvertReference(reference));
 
         public abstract RegistryCapabilities GetCapabilities(T reference);
 

diff --git a/src/Bicep.Core/Registry/IArtifactDispatcher.cs b/src/Bicep.Core/Registry/IArtifactDispatcher.cs
@@ -8,6 +8,7 @@
 using System.Threading.Tasks;
 using Bicep.Core.Diagnostics;
 using Bicep.Core.SourceCode;
+using Bicep.Core.Utils;
 
 namespace Bicep.Core.Registry
 {
@@ -32,6 +33,6 @@ public interface IModuleDispatcher : IArtifactReferenceFactory
         void PruneRestoreStatuses();
 
         // Retrieves the sources that have been restored along with the module into the cache (if available)
-        SourceArchive? TryGetModuleSources(ArtifactReference reference);
+        ResultWithException<SourceArchive> TryGetModuleSources(ArtifactReference reference);
     }
 }
diff --git a/src/Bicep.Core/Registry/IArtifactRegistry.cs b/src/Bicep.Core/Registry/IArtifactRegistry.cs
@@ -10,6 +10,7 @@
 using Bicep.Core.Modules;
 using Bicep.Core.Registry.Providers;
 using Bicep.Core.SourceCode;
+using Bicep.Core.Utils;
 
 namespace Bicep.Core.Registry
 {
@@ -102,6 +103,6 @@ public interface IArtifactRegistry
         /// </summary>
         /// <param name="reference">The module reference</param>
         /// <returns>A source archive</returns>
-        SourceArchive? TryGetSource(ArtifactReference reference);
+        ResultWithException<SourceArchive> TryGetSource(ArtifactReference reference);
     }
 }
diff --git a/src/Bicep.Core/Registry/LocalModuleRegistry.cs b/src/Bicep.Core/Registry/LocalModuleRegistry.cs
@@ -11,6 +11,7 @@
 using Bicep.Core.Modules;
 using Bicep.Core.Semantics;
 using Bicep.Core.SourceCode;
+using Bicep.Core.Utils;
 
 namespace Bicep.Core.Registry
 {
@@ -106,9 +107,9 @@ public override Task PublishProvider(LocalModuleReference reference, Stream type
             return null;
         }
 
-        public override SourceArchive? TryGetSource(LocalModuleReference reference)
+        public override ResultWithException<SourceArchive> TryGetSource(LocalModuleReference reference)
         {
-            return null;
+            return new(new SourceNotAvailableException());
         }
     }
 }
diff --git a/src/Bicep.Core/Registry/ModuleDispatcher.cs b/src/Bicep.Core/Registry/ModuleDispatcher.cs
@@ -16,6 +16,7 @@
 using Bicep.Core.Semantics;
 using Bicep.Core.SourceCode;
 using Bicep.Core.Syntax;
+using Bicep.Core.Utils;
 
 namespace Bicep.Core.Registry
 {
@@ -228,7 +229,7 @@ public void PruneRestoreStatuses()
         private IArtifactRegistry GetRegistry(ArtifactReference reference) =>
             Registries(reference.ParentModuleUri).TryGetValue(reference.Scheme, out var registry) ? registry : throw new InvalidOperationException($"Unexpected artifactDeclaration reference scheme '{reference.Scheme}'.");
 
-        public SourceArchive? TryGetModuleSources(ArtifactReference reference)
+        public ResultWithException<SourceArchive> TryGetModuleSources(ArtifactReference reference)
         {
             var registry = this.GetRegistry(reference);
             return registry.TryGetSource(reference);

diff --git a/src/Bicep.Core/Registry/OciArtifactRegistry.cs b/src/Bicep.Core/Registry/OciArtifactRegistry.cs
@@ -21,6 +21,7 @@
 using Bicep.Core.Semantics;
 using Bicep.Core.SourceCode;
 using Bicep.Core.Tracing;
+using Bicep.Core.Utils;
 using Newtonsoft.Json;
 
 namespace Bicep.Core.Registry
@@ -506,15 +507,16 @@ private string GetArtifactFilePath(OciArtifactReference reference, ArtifactFileT
             return fileSystem.Path.Combine(this.GetArtifactDirectoryPath(reference), fileName);
         }
 
-        public override SourceArchive? TryGetSource(OciArtifactReference reference)
+        public override ResultWithException<SourceArchive> TryGetSource(OciArtifactReference reference)
         {
             var zipPath = GetArtifactFilePath(reference, ArtifactFileType.Source);
             if (File.Exists(zipPath))
             {
-                return SourceArchive.FromStream(File.OpenRead(zipPath));
+                return SourceArchive.UnpackFromStream(File.OpenRead(zipPath));
             }
 
-            return null;
+            // No sources available (presumably they weren't published)
+            return new(new SourceNotAvailableException());
         }
 
         private enum ArtifactFileType

diff --git a/src/Bicep.Core/Registry/TemplateSpecModuleRegistry.cs b/src/Bicep.Core/Registry/TemplateSpecModuleRegistry.cs
@@ -14,6 +14,7 @@
 using Bicep.Core.Semantics;
 using Bicep.Core.SourceCode;
 using Bicep.Core.Tracing;
+using Bicep.Core.Utils;
 
 namespace Bicep.Core.Registry
 {
@@ -153,9 +154,9 @@ protected override string GetArtifactDirectoryPath(TemplateSpecModuleReference r
             return Task.FromResult<string?>(null);
         }
 
-        public override SourceArchive? TryGetSource(TemplateSpecModuleReference reference)
+        public override ResultWithException<SourceArchive> TryGetSource(TemplateSpecModuleReference reference)
         {
-            return null;
+            return new(new SourceNotAvailableException());
         }
     }
 }