From bc53d7d5e2626261336f8f78ff0757fab64dfafe Mon Sep 17 00:00:00 2001
From: "Stephen Weatherford (MSFT)" <stephwe@microsoft.com>
Date: Thu, 9 Jan 2025 15:19:11 -0800
Subject: [PATCH] work

---
 ...UseRecentModuleVersionsIntegrationTests.cs | 29 ++++---------------
 .../Rules/UseRecentModuleVersionsRule.cs      |  3 ++
 .../Registry/OciArtifactRegistry.cs           | 15 ++++++----
 3 files changed, 18 insertions(+), 29 deletions(-)

diff --git a/src/Bicep.Cli.IntegrationTests/UseRecentModuleVersionsIntegrationTests.cs b/src/Bicep.Cli.IntegrationTests/UseRecentModuleVersionsIntegrationTests.cs
index af26e8e322c..ceb75b95b21 100644
--- a/src/Bicep.Cli.IntegrationTests/UseRecentModuleVersionsIntegrationTests.cs
+++ b/src/Bicep.Cli.IntegrationTests/UseRecentModuleVersionsIntegrationTests.cs
@@ -111,33 +111,11 @@ private async Task<CliResult> Test(Options options)
         return await Bicep(settings, "lint", mainFile, options.NoRestore ? "--no-restore" : null);
     }
 
-    [TestMethod] //asdfgasdfgasdfg2 should be failing exception is swallowed
-    public async Task IfLevelIsOff_ShouldNotDownloadModuleMetadata()
-    {
-        var moduleIndexClientMock = PublicRegistryModuleIndexClientMock.CreateToThrow(new Exception("shouldn't try to download metadata if rule is off"));
-        var result = await Test(new Options(CacheRoot)
-        {
-            Bicep = """
-                module m1 '{PREFIX}/fake/avm/res/app/container-app:0.2.0' = {
-                  name: 'm1'
-                }
-                """.Replace("{PREFIX}", PREFIX),
-            DiagnosticLevel = "off",
-            PublishedModules = [$"{PREFIX}/fake/avm/res/app/container-app:0.2.0"],
-            MetadataClient = moduleIndexClientMock.Object,
-        });
-
-        result.Should().NotHaveStderr();
-        result.Should().HaveStdout("");
-        result.Should().Succeed();
-
-        moduleIndexClientMock.Verify(client => client.GetModuleIndexAsync(), Times.Never, "shouldn't try to download metadata if rule is off");
-    }
-
     [TestMethod]
     // We don't currently cache to disk, but rather on every check to restore modules.
-    public async Task IfNoRestoreSpecified_ThenShouldFailBecauseNoCache()
+    public async Task IfNoRestoreSpecified_ThenShouldNotDownloadMetadata_AndShouldFailBecauseNoCache()
     {
+        var moduleIndexClientMock = PublicRegistryModuleIndexClientMock.CreateToThrow(new Exception("shouldn't try to download metadata --no-restore is set"));
         var result = await Test(new Options(CacheRoot)
         {
             Bicep = """
@@ -147,6 +125,7 @@ public async Task IfNoRestoreSpecified_ThenShouldFailBecauseNoCache()
                 """.Replace("{PREFIX}", PREFIX),
             PublishedModules = [$"{PREFIX}/fake/avm/res/app/container-app:0.2.0"],
             ModulesMetadata = [("fake/avm/res/app/container-app", ["0.2.0"])],
+            MetadataClient = moduleIndexClientMock.Object,
             NoRestore = true,
         });
 
@@ -154,6 +133,8 @@ public async Task IfNoRestoreSpecified_ThenShouldFailBecauseNoCache()
         result.Should().HaveStderrMatch("*Warning use-recent-module-versions: Available module versions have not yet been downloaded. If running from the command line, be sure --no-restore is not specified.*");
         result.Should().HaveStdout("");
         result.Should().Fail();
+
+        moduleIndexClientMock.Verify(client => client.GetModuleIndexAsync(), Times.Never, "shouldn't try to download metadata --no-restore is set");
     }
 
     [TestMethod]
diff --git a/src/Bicep.Core/Analyzers/Linter/Rules/UseRecentModuleVersionsRule.cs b/src/Bicep.Core/Analyzers/Linter/Rules/UseRecentModuleVersionsRule.cs
index 470f5e03314..b63090c6b63 100644
--- a/src/Bicep.Core/Analyzers/Linter/Rules/UseRecentModuleVersionsRule.cs
+++ b/src/Bicep.Core/Analyzers/Linter/Rules/UseRecentModuleVersionsRule.cs
@@ -118,6 +118,9 @@ private static IEnumerable<Failure> GetFailures(SemanticModel model, IServicePro
 
         private static IEnumerable<Failure> AnalyzeBicepModule(IPublicRegistryModuleMetadataProvider publicRegistryModuleMetadataProvider, ModuleDeclarationSyntax moduleSyntax, TextSpan errorSpan, string tag, string publicModulePath)
         {
+            // NOTE: We don't want linter tests to download anything during analysis.  So metadata is loaded
+            //   and cached during module restore.  So don't use the Get*Async methods of IPublicRegistryModuleMetadataProvider,
+            //   just the GetCached* methods
             var availableVersions = publicRegistryModuleMetadataProvider.GetCachedModuleVersions($"{LanguageConstants.BicepPublicMcrPathPrefix}{publicModulePath}")
                 .Select(v => v.Version)
                 .ToArray();
diff --git a/src/Bicep.Core/Registry/OciArtifactRegistry.cs b/src/Bicep.Core/Registry/OciArtifactRegistry.cs
index 0ee0eebea34..34126d486e0 100644
--- a/src/Bicep.Core/Registry/OciArtifactRegistry.cs
+++ b/src/Bicep.Core/Registry/OciArtifactRegistry.cs
@@ -35,7 +35,7 @@ public sealed class OciArtifactRegistry : ExternalArtifactRegistry<OciArtifactRe
 
         private readonly IFeatureProvider features;
 
-        private readonly IPublicRegistryModuleMetadataProvider/*asdfg or indexer?*/ publicRegistryModuleMetadataProvider;
+        private readonly IPublicRegistryModuleMetadataProvider publicRegistryModuleMetadataProvider;
 
         public OciArtifactRegistry(
             IFileResolver FileResolver,
@@ -218,10 +218,15 @@ private OciManifest GetCachedManifest(OciArtifactReference ociArtifactModuleRefe
 
         public override async Task OnRestoreArtifacts(bool forceRestore)
         {
-            // We call this here because we don't want to run this code at all if we're running from the command line and --no-restore has been specified.
-            // So, if we're restoring artifacts, it's safe to also query for public module metadata
-            //asdfg use global options instead?
-            await publicRegistryModuleMetadataProvider.TryAwaitCache(forceRestore); //asdfg why are we waiting for the cache?   asdfg should we be using the indexer instead?
+            // We don't want linter tests to download anything during analysis.  So we are downloading
+            //   metadata here to avoid downloading during analysis, and tests can use cached data if it
+            //   exists (e.g. IRegistryModuleMetadataProvider.GetCached* methods).
+            // If --no-restore has been specified on the command ine, we don't want to download anything at all.
+            // Therefore we do the cache download here so that lint rules can have access to the cached metadata.
+            // CONSIDER: Revisit if it's okay to download metadata during analysis?  This will be more of a problem
+            //   when we extend the linter rules to include private registry modules.
+
+            await publicRegistryModuleMetadataProvider.TryAwaitCache(forceRestore);
         }
 
         public override async Task<IDictionary<ArtifactReference, DiagnosticBuilder.DiagnosticBuilderDelegate>> RestoreArtifacts(IEnumerable<OciArtifactReference> references)