Add cache control for nupkgs/snupks in public containers (#6634)

Author: shishirx34

src/NuGetGallery.Core/Services/CloudBlobCoreFileStorageService.cs

@@ -71,12 +71,12 @@ public async Task<bool> FileExistsAsync(string folderName, string fileName)
 
         public async Task<Stream> GetFileAsync(string folderName, string fileName)
         {
-            if (String.IsNullOrWhiteSpace(folderName))
+            if (string.IsNullOrWhiteSpace(folderName))
             {
                 throw new ArgumentNullException(nameof(folderName));
             }
 
-            if (String.IsNullOrWhiteSpace(fileName))
+            if (string.IsNullOrWhiteSpace(fileName))
             {
                 throw new ArgumentNullException(nameof(fileName));
             }
@@ -86,12 +86,12 @@ public async Task<Stream> GetFileAsync(string folderName, string fileName)
 
         public async Task<IFileReference> GetFileReferenceAsync(string folderName, string fileName, string ifNoneMatch = null)
         {
-            if (String.IsNullOrWhiteSpace(folderName))
+            if (string.IsNullOrWhiteSpace(folderName))
             {
                 throw new ArgumentNullException(nameof(folderName));
             }
 
-            if (String.IsNullOrWhiteSpace(fileName))
+            if (string.IsNullOrWhiteSpace(fileName))
             {
                 throw new ArgumentNullException(nameof(fileName));
             }
@@ -243,7 +243,7 @@ await destBlob.StartCopyAsync(
             catch (StorageException ex) when (ex.IsFileAlreadyExistsException())
             {
                 throw new FileAlreadyExistsException(
-                    String.Format(
+                    string.Format(
                         CultureInfo.CurrentCulture,
                         "There is already a blob with name {0} in container {1}.",
                         destFileName,
@@ -276,6 +276,17 @@ await destBlob.StartCopyAsync(
                 throw new StorageException($"The blob copy operation had copy status {destBlob.CopyState.Status} ({destBlob.CopyState.StatusDescription}).");
             }
 
+            var cacheControl = GetCacheControlForCopy(destFolderName);
+            if (!string.IsNullOrEmpty(cacheControl))
+            {
+                await destBlob.FetchAttributesAsync();
+                if (string.IsNullOrEmpty(destBlob.Properties.CacheControl))
+                {
+                    destBlob.Properties.CacheControl = cacheControl;
+                    await destBlob.SetPropertiesAsync();
+                }
+            }
+
             return srcBlob.ETag;
         }
 
@@ -316,7 +327,7 @@ public async Task SaveFileAsync(string folderName, string fileName, string conte
             catch (StorageException ex) when (ex.IsFileAlreadyExistsException())
             {
                 throw new FileAlreadyExistsException(
-                    String.Format(
+                    string.Format(
                         CultureInfo.CurrentCulture,
                         "There is already a blob with name {0} in container {1}.",
                         fileName,
@@ -349,7 +360,7 @@ public async Task SaveFileAsync(string folderName, string fileName, Stream file,
             catch (StorageException ex) when (ex.IsFileAlreadyExistsException())
             {
                 throw new FileAlreadyExistsException(
-                    String.Format(
+                    string.Format(
                         CultureInfo.CurrentCulture,
                         "There is already a blob with name {0} in container {1}.",
                         fileName,
@@ -508,7 +519,7 @@ private bool IsPublicContainer(string folderName)
             }
 
             throw new InvalidOperationException(
-                String.Format(CultureInfo.CurrentCulture, "The folder name {0} is not supported.", folderName));
+                string.Format(CultureInfo.CurrentCulture, "The folder name {0} is not supported.", folderName));
         }
 
         private async Task<StorageResult> GetBlobContentAsync(string folderName, string fileName, string ifNoneMatch = null)
@@ -591,7 +602,20 @@ private static string GetContentType(string folderName)
 
                 default:
                     throw new InvalidOperationException(
-                        String.Format(CultureInfo.CurrentCulture, "The folder name {0} is not supported.", folderName));
+                        string.Format(CultureInfo.CurrentCulture, "The folder name {0} is not supported.", folderName));
+            }
+        }
+
+        private static string GetCacheControlForCopy(string folderName)
+        {
+            switch (folderName)
+            {
+                case CoreConstants.PackagesFolderName:
+                case CoreConstants.SymbolPackagesFolderName:
+                    return CoreConstants.DefaultCacheControl;
+
+                default:
+                    return null;
             }
         }
 
@@ -602,11 +626,11 @@ private static string GetCacheControl(string folderName)
                 case CoreConstants.PackagesFolderName:
                 case CoreConstants.SymbolPackagesFolderName:
                 case CoreConstants.PackagesContentFolderName:
+                case CoreConstants.ValidationFolderName:
                     return CoreConstants.DefaultCacheControl;
 
                 case CoreConstants.PackageBackupsFolderName:
                 case CoreConstants.UploadsFolderName:
-                case CoreConstants.ValidationFolderName:
                 case CoreConstants.SymbolPackageBackupsFolderName:
                 case CoreConstants.DownloadsFolderName:
                 case CoreConstants.PackageReadMesFolderName:
@@ -618,7 +642,7 @@ private static string GetCacheControl(string folderName)
 
                 default:
                     throw new InvalidOperationException(
-                        String.Format(CultureInfo.CurrentCulture, "The folder name {0} is not supported.", folderName));
+                        string.Format(CultureInfo.CurrentCulture, "The folder name {0} is not supported.", folderName));
             }
         }
 

tests/NuGetGallery.Core.Facts/Services/CloudBlobCoreFileStorageServiceFacts.cs

@@ -536,7 +536,9 @@ public async Task WillSetTheBlobControlCacheOnPackagesFolder(string folderName)
 
                 fakeBlob.Verify();
 
-                if (folderName == CoreConstants.PackagesFolderName || folderName == CoreConstants.SymbolPackagesFolderName)
+                if (folderName == CoreConstants.PackagesFolderName 
+                    || folderName == CoreConstants.SymbolPackagesFolderName
+                    || folderName == CoreConstants.ValidationFolderName)
                 {
                     Assert.Equal(CoreConstants.DefaultCacheControl, fakeBlob.Object.Properties.CacheControl);
                 }
@@ -1065,6 +1067,54 @@ await _target.CopyFileAsync(
                     Times.Once);
             }
 
+            [Theory]
+            [InlineData(CoreConstants.PackagesFolderName)]
+            [InlineData(CoreConstants.SymbolPackagesFolderName)]
+            public async Task WillCopyAndSetCacheControlOnCopyForFolder(string folderName)
+            {
+                // Arrange
+                var instance = new TheCopyFileAsyncMethod();
+                instance._blobClient
+                    .Setup(x => x.GetBlobFromUri(It.IsAny<Uri>()))
+                    .Returns(instance._srcBlobMock.Object);
+                instance._blobClient
+                    .Setup(x => x.GetContainerReference(folderName))
+                    .Returns(() => instance._destContainer.Object);
+
+                instance._destBlobMock
+                    .Setup(x => x.StartCopyAsync(It.IsAny<ISimpleCloudBlob>(), It.IsAny<AccessCondition>(), It.IsAny<AccessCondition>()))
+                    .Returns(Task.FromResult(0))
+                    .Callback<ISimpleCloudBlob, AccessCondition, AccessCondition>((_, __, ___) =>
+                    {
+                        SetDestCopyStatus(CopyStatus.Success);
+                    });
+                
+                // Act
+                await instance._target.CopyFileAsync(
+                    instance._srcUri,
+                    folderName,
+                    instance._destFileName,
+                    AccessConditionWrapper.GenerateIfNotExistsCondition());
+
+                // Assert
+                instance._destBlobMock.Verify(
+                    x => x.StartCopyAsync(instance._srcBlobMock.Object, It.IsAny<AccessCondition>(), It.IsAny<AccessCondition>()),
+                    Times.Once);
+                instance._destBlobMock.Verify(
+                    x => x.StartCopyAsync(It.IsAny<ISimpleCloudBlob>(), It.IsAny<AccessCondition>(), It.IsAny<AccessCondition>()),
+                    Times.Once);
+                instance._destBlobMock.Verify(
+                    x => x.SetPropertiesAsync(),
+                    Times.Once);
+                instance._destBlobMock.Verify(
+                    x => x.StartCopyAsync(It.IsAny<ISimpleCloudBlob>(), It.IsAny<AccessCondition>(), It.IsAny<AccessCondition>()),
+                    Times.Once);
+                Assert.NotNull(instance._destProperties.CacheControl);
+                instance._blobClient.Verify(
+                    x => x.GetBlobFromUri(instance._srcUri),
+                    Times.Once);
+            }
+
             [Fact]
             public async Task WillCopyTheFileIfDestinationDoesNotExist()
             {