From 556e9154a8c28ab742d8c0dc7024336ff60ef41f Mon Sep 17 00:00:00 2001 From: Shishir H Date: Thu, 8 Nov 2018 10:26:43 -0800 Subject: [PATCH] Add cache control for nupkgs/snupks in public containers (#6634) --- .../CloudBlobCoreFileStorageService.cs | 46 ++++++++++++---- .../CloudBlobCoreFileStorageServiceFacts.cs | 52 ++++++++++++++++++- 2 files changed, 86 insertions(+), 12 deletions(-) diff --git a/src/NuGetGallery.Core/Services/CloudBlobCoreFileStorageService.cs b/src/NuGetGallery.Core/Services/CloudBlobCoreFileStorageService.cs index 7e39752420..bf97614c13 100644 --- a/src/NuGetGallery.Core/Services/CloudBlobCoreFileStorageService.cs +++ b/src/NuGetGallery.Core/Services/CloudBlobCoreFileStorageService.cs @@ -71,12 +71,12 @@ public async Task FileExistsAsync(string folderName, string fileName) public async Task 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 GetFileAsync(string folderName, string fileName) public async Task 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 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)); } } diff --git a/tests/NuGetGallery.Core.Facts/Services/CloudBlobCoreFileStorageServiceFacts.cs b/tests/NuGetGallery.Core.Facts/Services/CloudBlobCoreFileStorageServiceFacts.cs index 25a211b407..4ecb750ef1 100644 --- a/tests/NuGetGallery.Core.Facts/Services/CloudBlobCoreFileStorageServiceFacts.cs +++ b/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())) + .Returns(instance._srcBlobMock.Object); + instance._blobClient + .Setup(x => x.GetContainerReference(folderName)) + .Returns(() => instance._destContainer.Object); + + instance._destBlobMock + .Setup(x => x.StartCopyAsync(It.IsAny(), It.IsAny(), It.IsAny())) + .Returns(Task.FromResult(0)) + .Callback((_, __, ___) => + { + 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(), It.IsAny()), + Times.Once); + instance._destBlobMock.Verify( + x => x.StartCopyAsync(It.IsAny(), It.IsAny(), It.IsAny()), + Times.Once); + instance._destBlobMock.Verify( + x => x.SetPropertiesAsync(), + Times.Once); + instance._destBlobMock.Verify( + x => x.StartCopyAsync(It.IsAny(), It.IsAny(), It.IsAny()), + Times.Once); + Assert.NotNull(instance._destProperties.CacheControl); + instance._blobClient.Verify( + x => x.GetBlobFromUri(instance._srcUri), + Times.Once); + } + [Fact] public async Task WillCopyTheFileIfDestinationDoesNotExist() {