From 4904d6ce1826c6d9daac6cd0eb46d691e0daf5ac Mon Sep 17 00:00:00 2001 From: shishirh Date: Thu, 8 Nov 2018 11:49:43 -0800 Subject: [PATCH 1/3] Add etag for properties update --- .../Services/CloudBlobCoreFileStorageService.cs | 10 +++++++++- src/NuGetGallery.Core/Services/CloudBlobWrapper.cs | 5 +++++ src/NuGetGallery.Core/Services/ISimpleCloudBlob.cs | 1 + .../Services/CloudBlobCoreFileStorageServiceFacts.cs | 2 +- 4 files changed, 16 insertions(+), 2 deletions(-) diff --git a/src/NuGetGallery.Core/Services/CloudBlobCoreFileStorageService.cs b/src/NuGetGallery.Core/Services/CloudBlobCoreFileStorageService.cs index bf97614c13..4af1a23599 100644 --- a/src/NuGetGallery.Core/Services/CloudBlobCoreFileStorageService.cs +++ b/src/NuGetGallery.Core/Services/CloudBlobCoreFileStorageService.cs @@ -280,10 +280,18 @@ await destBlob.StartCopyAsync( if (!string.IsNullOrEmpty(cacheControl)) { await destBlob.FetchAttributesAsync(); + if (string.IsNullOrEmpty(destBlob.Properties.CacheControl)) { + var accessCondition = AccessConditionWrapper.GenerateIfMatchCondition(destBlob.ETag); + var mappedAccessCondition = new AccessCondition + { + IfNoneMatchETag = accessCondition.IfNoneMatchETag, + IfMatchETag = accessCondition.IfMatchETag + }; + destBlob.Properties.CacheControl = cacheControl; - await destBlob.SetPropertiesAsync(); + await destBlob.SetPropertiesAsync(mappedAccessCondition); } } diff --git a/src/NuGetGallery.Core/Services/CloudBlobWrapper.cs b/src/NuGetGallery.Core/Services/CloudBlobWrapper.cs index cdb6d43e32..6af90a790b 100644 --- a/src/NuGetGallery.Core/Services/CloudBlobWrapper.cs +++ b/src/NuGetGallery.Core/Services/CloudBlobWrapper.cs @@ -66,6 +66,11 @@ public async Task SetPropertiesAsync() await _blob.SetPropertiesAsync(); } + public async Task SetPropertiesAsync(AccessCondition accessCondition) + { + await _blob.SetPropertiesAsync(accessCondition, options: null, operationContext: null); + } + public async Task SetMetadataAsync(AccessCondition accessCondition) { await _blob.SetMetadataAsync(accessCondition, options: null, operationContext: null); diff --git a/src/NuGetGallery.Core/Services/ISimpleCloudBlob.cs b/src/NuGetGallery.Core/Services/ISimpleCloudBlob.cs index d2ac6b3e96..c27108bae0 100644 --- a/src/NuGetGallery.Core/Services/ISimpleCloudBlob.cs +++ b/src/NuGetGallery.Core/Services/ISimpleCloudBlob.cs @@ -26,6 +26,7 @@ public interface ISimpleCloudBlob Task ExistsAsync(); Task SetPropertiesAsync(); + Task SetPropertiesAsync(AccessCondition accessCondition); Task SetMetadataAsync(AccessCondition accessCondition); Task UploadFromStreamAsync(Stream source, bool overwrite); Task UploadFromStreamAsync(Stream source, AccessCondition accessCondition); diff --git a/tests/NuGetGallery.Core.Facts/Services/CloudBlobCoreFileStorageServiceFacts.cs b/tests/NuGetGallery.Core.Facts/Services/CloudBlobCoreFileStorageServiceFacts.cs index 4ecb750ef1..aff01a315b 100644 --- a/tests/NuGetGallery.Core.Facts/Services/CloudBlobCoreFileStorageServiceFacts.cs +++ b/tests/NuGetGallery.Core.Facts/Services/CloudBlobCoreFileStorageServiceFacts.cs @@ -1104,7 +1104,7 @@ await instance._target.CopyFileAsync( x => x.StartCopyAsync(It.IsAny(), It.IsAny(), It.IsAny()), Times.Once); instance._destBlobMock.Verify( - x => x.SetPropertiesAsync(), + x => x.SetPropertiesAsync(It.IsAny()), Times.Once); instance._destBlobMock.Verify( x => x.StartCopyAsync(It.IsAny(), It.IsAny(), It.IsAny()), From 1d0b354dd3f6f69cf88cefcc01756d0c56155f52 Mon Sep 17 00:00:00 2001 From: shishirh Date: Thu, 8 Nov 2018 13:32:41 -0800 Subject: [PATCH 2/3] Add new method for updating properties --- .../CloudBlobCoreFileStorageService.cs | 81 ++++++---- .../Services/ICoreFileStorageService.cs | 13 ++ .../Services/FileSystemFileStorageService.cs | 9 ++ .../CloudBlobCoreFileStorageServiceFacts.cs | 152 ++++++++++++------ .../FileSystemFileStorageServiceFacts.cs | 11 ++ 5 files changed, 186 insertions(+), 80 deletions(-) diff --git a/src/NuGetGallery.Core/Services/CloudBlobCoreFileStorageService.cs b/src/NuGetGallery.Core/Services/CloudBlobCoreFileStorageService.cs index 4af1a23599..c846f05777 100644 --- a/src/NuGetGallery.Core/Services/CloudBlobCoreFileStorageService.cs +++ b/src/NuGetGallery.Core/Services/CloudBlobCoreFileStorageService.cs @@ -276,25 +276,6 @@ 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)) - { - var accessCondition = AccessConditionWrapper.GenerateIfMatchCondition(destBlob.ETag); - var mappedAccessCondition = new AccessCondition - { - IfNoneMatchETag = accessCondition.IfNoneMatchETag, - IfMatchETag = accessCondition.IfMatchETag - }; - - destBlob.Properties.CacheControl = cacheControl; - await destBlob.SetPropertiesAsync(mappedAccessCondition); - } - } - return srcBlob.ETag; } @@ -456,6 +437,55 @@ public async Task SetMetadataAsync( } } + /// + /// Asynchronously sets blob properties. + /// + /// The folder (container) name. + /// The blob file name. + /// A function which updates blob properties and returns true + /// for changes to be persisted or false for changes to be discarded. + /// A task that represents the asynchronous operation. + public async Task SetPropertiesAsync( + string folderName, + string fileName, + Func>, BlobProperties, Task> updatePropertiesAsync) + { + if (folderName == null) + { + throw new ArgumentNullException(nameof(folderName)); + } + + if (fileName == null) + { + throw new ArgumentNullException(nameof(fileName)); + } + + if (updatePropertiesAsync == null) + { + throw new ArgumentNullException(nameof(updatePropertiesAsync)); + } + + var container = await GetContainerAsync(folderName); + var blob = container.GetBlobReference(fileName); + + await blob.FetchAttributesAsync(); + + var lazyStream = new Lazy>(() => GetFileAsync(folderName, fileName)); + var wasUpdated = await updatePropertiesAsync(lazyStream, blob.Properties); + + if (wasUpdated) + { + var accessCondition = AccessConditionWrapper.GenerateIfMatchCondition(blob.ETag); + var mappedAccessCondition = new AccessCondition + { + IfNoneMatchETag = accessCondition.IfNoneMatchETag, + IfMatchETag = accessCondition.IfMatchETag + }; + + await blob.SetPropertiesAsync(mappedAccessCondition); + } + } + public async Task GetETagOrNullAsync( string folderName, string fileName) @@ -614,19 +644,6 @@ private static string GetContentType(string folderName) } } - private static string GetCacheControlForCopy(string folderName) - { - switch (folderName) - { - case CoreConstants.PackagesFolderName: - case CoreConstants.SymbolPackagesFolderName: - return CoreConstants.DefaultCacheControl; - - default: - return null; - } - } - private static string GetCacheControl(string folderName) { switch (folderName) diff --git a/src/NuGetGallery.Core/Services/ICoreFileStorageService.cs b/src/NuGetGallery.Core/Services/ICoreFileStorageService.cs index b4aa68653f..9c135a3322 100644 --- a/src/NuGetGallery.Core/Services/ICoreFileStorageService.cs +++ b/src/NuGetGallery.Core/Services/ICoreFileStorageService.cs @@ -1,6 +1,7 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +using Microsoft.WindowsAzure.Storage.Blob; using System; using System.Collections.Generic; using System.IO; @@ -136,6 +137,18 @@ Task SetMetadataAsync( string fileName, Func>, IDictionary, Task> updateMetadataAsync); + /// + /// Updates properties on the file. + /// + /// The folder name. + /// The file name. + /// A function that will update file properties. + /// A task that represents the asynchronous operation. + Task SetPropertiesAsync( + string folderName, + string fileName, + Func>, BlobProperties, Task> updatePropertiesAsync); + /// /// Returns the etag value for the specified blob. If the blob does not exists it will return null. /// diff --git a/src/NuGetGallery/Services/FileSystemFileStorageService.cs b/src/NuGetGallery/Services/FileSystemFileStorageService.cs index 36077374f6..509aa36ef9 100644 --- a/src/NuGetGallery/Services/FileSystemFileStorageService.cs +++ b/src/NuGetGallery/Services/FileSystemFileStorageService.cs @@ -8,6 +8,7 @@ using System.Threading.Tasks; using System.Web.Hosting; using System.Web.Mvc; +using Microsoft.WindowsAzure.Storage.Blob; using NuGetGallery.Configuration; namespace NuGetGallery @@ -262,6 +263,14 @@ public Task SetMetadataAsync( return Task.CompletedTask; } + public Task SetPropertiesAsync( + string folderName, + string fileName, + Func>, BlobProperties, Task> updatePropertiesAsync) + { + return Task.CompletedTask; + } + private static string BuildPath(string fileStorageDirectory, string folderName, string fileName) { // Resolve the file storage directory diff --git a/tests/NuGetGallery.Core.Facts/Services/CloudBlobCoreFileStorageServiceFacts.cs b/tests/NuGetGallery.Core.Facts/Services/CloudBlobCoreFileStorageServiceFacts.cs index aff01a315b..6594776d23 100644 --- a/tests/NuGetGallery.Core.Facts/Services/CloudBlobCoreFileStorageServiceFacts.cs +++ b/tests/NuGetGallery.Core.Facts/Services/CloudBlobCoreFileStorageServiceFacts.cs @@ -1067,54 +1067,6 @@ 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(It.IsAny()), - 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() { @@ -1461,6 +1413,110 @@ await _service.SetMetadataAsync( } } + public class TheSetPropertiesAsyncMethod + { + private const string _content = "peach"; + + private readonly Mock _blobClient; + private readonly Mock _blobContainer; + private readonly Mock _blob; + private readonly CloudBlobCoreFileStorageService _service; + + public TheSetPropertiesAsyncMethod() + { + _blobClient = new Mock(); + _blobContainer = new Mock(); + _blob = new Mock(); + + _blobClient.Setup(x => x.GetContainerReference(It.IsAny())) + .Returns(_blobContainer.Object); + _blobContainer.Setup(x => x.CreateIfNotExistAsync()) + .Returns(Task.FromResult(0)); + _blobContainer.Setup(x => x.SetPermissionsAsync(It.IsAny())) + .Returns(Task.FromResult(0)); + _blobContainer.Setup(x => x.GetBlobReference(It.IsAny())) + .Returns(_blob.Object); + + _service = CreateService(fakeBlobClient: _blobClient); + } + + [Fact] + public async Task WhenLazyStreamRead_ReturnsContent() + { + _blob.Setup(x => x.DownloadToStreamAsync(It.IsAny(), It.IsAny())) + .Callback((stream, _) => + { + using (var writer = new StreamWriter(stream, Encoding.UTF8, bufferSize: 4096, leaveOpen: true)) + { + writer.Write(_content); + } + }) + .Returns(Task.FromResult(0)); + + await _service.SetPropertiesAsync( + folderName: CoreConstants.PackagesFolderName, + fileName: "a", + updatePropertiesAsync: async (lazyStream, properties) => + { + using (var stream = await lazyStream.Value) + using (var reader = new StreamReader(stream)) + { + Assert.Equal(_content, reader.ReadToEnd()); + } + + return false; + }); + + _blob.VerifyAll(); + _blobContainer.VerifyAll(); + _blobClient.VerifyAll(); + } + + [Fact] + public async Task WhenReturnValueIsFalse_PropertyChangesAreNotPersisted() + { + _blob.SetupGet(x => x.Properties) + .Returns(new BlobProperties()); + + await _service.SetPropertiesAsync( + folderName: CoreConstants.PackagesFolderName, + fileName: "a", + updatePropertiesAsync: (lazyStream, properties) => + { + Assert.NotNull(properties); + + return Task.FromResult(false); + }); + + _blob.VerifyAll(); + _blobContainer.VerifyAll(); + _blobClient.VerifyAll(); + } + + [Fact] + public async Task WhenReturnValueIsTrue_PropertiesChangesArePersisted() + { + _blob.SetupGet(x => x.Properties) + .Returns(new BlobProperties()); + _blob.Setup(x => x.SetPropertiesAsync(It.IsNotNull())) + .Returns(Task.FromResult(0)); + + await _service.SetPropertiesAsync( + folderName: CoreConstants.PackagesFolderName, + fileName: "a", + updatePropertiesAsync: (lazyStream, properties) => + { + Assert.NotNull(properties); + + return Task.FromResult(true); + }); + + _blob.VerifyAll(); + _blobContainer.VerifyAll(); + _blobClient.VerifyAll(); + } + } + public class TheGetETagMethod { private const string _etag = "dummy_etag"; diff --git a/tests/NuGetGallery.Facts/Services/FileSystemFileStorageServiceFacts.cs b/tests/NuGetGallery.Facts/Services/FileSystemFileStorageServiceFacts.cs index bed2735084..5442a94e7a 100644 --- a/tests/NuGetGallery.Facts/Services/FileSystemFileStorageServiceFacts.cs +++ b/tests/NuGetGallery.Facts/Services/FileSystemFileStorageServiceFacts.cs @@ -733,5 +733,16 @@ public async Task NoOps() await service.SetMetadataAsync(folderName: null, fileName: null, updateMetadataAsync: null); } } + + public class TheSetPropertiesAsyncMethod + { + [Fact] + public async Task NoOps() + { + var service = CreateService(); + + await service.SetPropertiesAsync(folderName: null, fileName: null, updatePropertiesAsync: null); + } + } } } \ No newline at end of file From 743432396af0d68b19029b4e57ff9231ccf754b3 Mon Sep 17 00:00:00 2001 From: shishirh Date: Thu, 8 Nov 2018 14:11:16 -0800 Subject: [PATCH 3/3] nit fix --- src/NuGetGallery.Core/Services/ICoreFileStorageService.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/NuGetGallery.Core/Services/ICoreFileStorageService.cs b/src/NuGetGallery.Core/Services/ICoreFileStorageService.cs index 9c135a3322..ef11a3f666 100644 --- a/src/NuGetGallery.Core/Services/ICoreFileStorageService.cs +++ b/src/NuGetGallery.Core/Services/ICoreFileStorageService.cs @@ -1,11 +1,11 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. -using Microsoft.WindowsAzure.Storage.Blob; using System; using System.Collections.Generic; using System.IO; using System.Threading.Tasks; +using Microsoft.WindowsAzure.Storage.Blob; namespace NuGetGallery {