From 262741eabb8d7ac72f7aef6ec463387833417c1a Mon Sep 17 00:00:00 2001 From: Joel Verhagen Date: Mon, 5 Mar 2018 18:05:47 -0800 Subject: [PATCH] Add CopyFileAsync to ICoreFileStorageService (#5581) Progress on https://github.com/NuGet/Engineering/issues/1190 Progress on https://github.com/NuGet/Engineering/issues/1203 --- .../CloudBlobCoreFileStorageService.cs | 97 ++++++++ .../Services/CloudBlobWrapper.cs | 28 ++- .../Services/ICoreFileStorageService.cs | 12 + .../Services/ISimpleCloudBlob.cs | 3 + .../Services/FileSystemFileStorageService.cs | 40 ++++ .../Services/FileSystemService.cs | 5 + .../Services/IFileSystemService.cs | 2 + .../CloudBlobCoreFileStorageServiceFacts.cs | 218 ++++++++++++++++++ .../FileSystemFileStorageServiceFacts.cs | 96 ++++++++ 9 files changed, 499 insertions(+), 2 deletions(-) diff --git a/src/NuGetGallery.Core/Services/CloudBlobCoreFileStorageService.cs b/src/NuGetGallery.Core/Services/CloudBlobCoreFileStorageService.cs index 8755721894..b6bc3e2d04 100644 --- a/src/NuGetGallery.Core/Services/CloudBlobCoreFileStorageService.cs +++ b/src/NuGetGallery.Core/Services/CloudBlobCoreFileStorageService.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Concurrent; using System.Collections.Generic; +using System.Diagnostics; using System.Globalization; using System.IO; using System.Net; @@ -16,6 +17,15 @@ namespace NuGetGallery { public class CloudBlobCoreFileStorageService : ICoreFileStorageService { + /// + /// This is the maximum duration for to poll, + /// waiting for a package copy to complete. The value picked today is based off of the maximum duration we wait + /// when uploading files to Azure China blob storage. Note that in cases when the copy source and destination + /// are in the same container, the copy completed immediately and no polling is necessary. + /// + private static readonly TimeSpan MaxCopyDuration = TimeSpan.FromMinutes(10); + private static readonly TimeSpan CopyPollFrequency = TimeSpan.FromMilliseconds(500); + private static readonly HashSet KnownPublicFolders = new HashSet { CoreConstants.PackagesFolderName, CoreConstants.PackageBackupsFolderName, @@ -100,6 +110,93 @@ public async Task GetFileReferenceAsync(string folderName, strin } } + public async Task CopyFileAsync(string srcFolderName, string srcFileName, string destFolderName, string destFileName) + { + if (srcFolderName == null) + { + throw new ArgumentNullException(nameof(srcFolderName)); + } + + if (srcFileName == null) + { + throw new ArgumentNullException(nameof(srcFileName)); + } + + if (destFolderName == null) + { + throw new ArgumentNullException(nameof(destFolderName)); + } + + if (destFileName == null) + { + throw new ArgumentNullException(nameof(destFileName)); + } + + var srcContainer = await GetContainerAsync(srcFolderName); + var srcBlob = srcContainer.GetBlobReference(srcFileName); + + var destContainer = await GetContainerAsync(destFolderName); + var destBlob = destContainer.GetBlobReference(destFileName); + + // Determine the source blob etag. + await srcBlob.FetchAttributesAsync(); + var srcAccessCondition = AccessCondition.GenerateIfMatchCondition(srcBlob.ETag); + + // Check if the destination blob already exists and fetch attributes. + var destAccessCondition = AccessCondition.GenerateIfNotExistsCondition(); + if (await destBlob.ExistsAsync()) + { + if (destBlob.CopyState?.Status == CopyStatus.Failed) + { + // If the last copy failed, allow this copy to occur. + destAccessCondition = AccessCondition.GenerateIfMatchCondition(destBlob.ETag); + } + else if ((srcBlob.Properties.ContentMD5 != null + && srcBlob.Properties.ContentMD5 == destBlob.Properties.ContentMD5 + && srcBlob.Properties.Length == destBlob.Properties.Length)) + { + // If the blob hash is the same and the length is the same, no-op the copy. + return; + } + } + + // Start the server-side copy and wait for it to complete. + try + { + await destBlob.StartCopyAsync( + srcBlob, + srcAccessCondition, + destAccessCondition); + } + catch (StorageException ex) when (ex.RequestInformation?.HttpStatusCode == (int?)HttpStatusCode.Conflict) + { + throw new InvalidOperationException( + String.Format( + CultureInfo.CurrentCulture, + "There is already a blob with name {0} in container {1}.", + destFileName, + destFolderName), + ex); + } + + var stopwatch = Stopwatch.StartNew(); + while (destBlob.CopyState.Status == CopyStatus.Pending + && stopwatch.Elapsed < MaxCopyDuration) + { + await destBlob.FetchAttributesAsync(); + await Task.Delay(CopyPollFrequency); + } + + if (destBlob.CopyState.Status == CopyStatus.Pending) + { + throw new TimeoutException($"Waiting for the blob copy operation to complete timed out after {MaxCopyDuration.TotalSeconds} seconds."); + } + else if (destBlob.CopyState.Status != CopyStatus.Success) + { + throw new StorageException($"The blob copy operation had copy status {destBlob.CopyState.Status} ({destBlob.CopyState.StatusDescription})."); + } + } + public async Task SaveFileAsync(string folderName, string fileName, Stream packageFile, bool overwrite = true) { ICloudBlobContainer container = await GetContainerAsync(folderName); diff --git a/src/NuGetGallery.Core/Services/CloudBlobWrapper.cs b/src/NuGetGallery.Core/Services/CloudBlobWrapper.cs index 2bdf15f143..270e9de7b3 100644 --- a/src/NuGetGallery.Core/Services/CloudBlobWrapper.cs +++ b/src/NuGetGallery.Core/Services/CloudBlobWrapper.cs @@ -1,5 +1,6 @@ // 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 System; using System.IO; using System.Net; @@ -12,9 +13,9 @@ namespace NuGetGallery { public class CloudBlobWrapper : ISimpleCloudBlob { - private readonly ICloudBlob _blob; + private readonly CloudBlockBlob _blob; - public CloudBlobWrapper(ICloudBlob blob) + public CloudBlobWrapper(CloudBlockBlob blob) { _blob = blob; } @@ -24,6 +25,11 @@ public BlobProperties Properties get { return _blob.Properties; } } + public CopyState CopyState + { + get { return _blob.CopyState; } + } + public Uri Uri { get { return _blob.Uri; } @@ -137,6 +143,24 @@ public string GetSharedReadSignature(DateTimeOffset? endOfAccess) return signature; } + public async Task StartCopyAsync(ISimpleCloudBlob source, AccessCondition sourceAccessCondition, AccessCondition destAccessCondition) + { + // To avoid this we would need to somehow abstract away the primary and secondary storage locations. This + // is not worth the effort right now! + var sourceWrapper = source as CloudBlobWrapper; + if (sourceWrapper == null) + { + throw new ArgumentException($"The source blob must be a {nameof(CloudBlobWrapper)}."); + } + + await _blob.StartCopyAsync( + sourceWrapper._blob, + sourceAccessCondition: sourceAccessCondition, + destAccessCondition: destAccessCondition, + options: null, + operationContext: null); + } + // The default retry policy treats a 304 as an error that requires a retry. We don't want that! private class DontRetryOnNotModifiedPolicy : IRetryPolicy { diff --git a/src/NuGetGallery.Core/Services/ICoreFileStorageService.cs b/src/NuGetGallery.Core/Services/ICoreFileStorageService.cs index 392704a299..ee4670bb47 100644 --- a/src/NuGetGallery.Core/Services/ICoreFileStorageService.cs +++ b/src/NuGetGallery.Core/Services/ICoreFileStorageService.cs @@ -35,5 +35,17 @@ public interface ICoreFileStorageService Task GetFileReadUriAsync(string folderName, string fileName, DateTimeOffset? endOfAccess); Task SaveFileAsync(string folderName, string fileName, Stream packageFile, bool overwrite = true); + + /// + /// Copies the source file to the destination file. If the destination already exists and the content + /// is different, an exception should be thrown. If the file already exists, the implementation can choose to + /// no-op if the content is the same instead of throwing an exception. This method should throw if the source + /// file does not exist. + /// + /// The source folder. + /// The source file name or relative file path. + /// The destination folder. + /// The destination file name or relative file path. + Task CopyFileAsync(string srcFolderName, string srcFileName, string destFolderName, string destFileName); } } diff --git a/src/NuGetGallery.Core/Services/ISimpleCloudBlob.cs b/src/NuGetGallery.Core/Services/ISimpleCloudBlob.cs index 559827123d..9b366f0cee 100644 --- a/src/NuGetGallery.Core/Services/ISimpleCloudBlob.cs +++ b/src/NuGetGallery.Core/Services/ISimpleCloudBlob.cs @@ -12,6 +12,7 @@ namespace NuGetGallery public interface ISimpleCloudBlob { BlobProperties Properties { get; } + CopyState CopyState { get; } Uri Uri { get; } string Name { get; } DateTime LastModifiedUtc { get; } @@ -27,6 +28,8 @@ public interface ISimpleCloudBlob Task FetchAttributesAsync(); + Task StartCopyAsync(ISimpleCloudBlob source, AccessCondition sourceAccessCondition, AccessCondition destAccessCondition); + /// /// Generates the shared read signature that if appended to the blob URI /// would allow reading the contents of the blob using the produced URI diff --git a/src/NuGetGallery/Services/FileSystemFileStorageService.cs b/src/NuGetGallery/Services/FileSystemFileStorageService.cs index 3b694a942c..c2542b61b1 100644 --- a/src/NuGetGallery/Services/FileSystemFileStorageService.cs +++ b/src/NuGetGallery/Services/FileSystemFileStorageService.cs @@ -4,6 +4,7 @@ using System; using System.Globalization; using System.IO; +using System.Security.Cryptography; using System.Threading.Tasks; using System.Web.Hosting; using System.Web.Mvc; @@ -168,6 +169,45 @@ public Task SaveFileAsync(string folderName, string fileName, Stream packageFile return Task.FromResult(0); } + public Task CopyFileAsync(string srcFolderName, string srcFileName, string destFolderName, string destFileName) + { + if (srcFolderName == null) + { + throw new ArgumentNullException(nameof(srcFolderName)); + } + + if (srcFileName == null) + { + throw new ArgumentNullException(nameof(srcFileName)); + } + + if (destFolderName == null) + { + throw new ArgumentNullException(nameof(destFolderName)); + } + + if (destFileName == null) + { + throw new ArgumentNullException(nameof(destFileName)); + } + + var srcFilePath = BuildPath(_configuration.FileStorageDirectory, srcFolderName, srcFileName); + var destFilePath = BuildPath(_configuration.FileStorageDirectory, destFolderName, destFileName); + + _fileSystemService.CreateDirectory(Path.GetDirectoryName(destFilePath)); + + try + { + _fileSystemService.Copy(srcFilePath, destFilePath, overwrite: false); + } + catch (IOException e) + { + throw new InvalidOperationException("Could not copy because destination file already exists", e); + } + + return Task.CompletedTask; + } + public Task IsAvailableAsync() { return Task.FromResult(Directory.Exists(_configuration.FileStorageDirectory)); diff --git a/src/NuGetGallery/Services/FileSystemService.cs b/src/NuGetGallery/Services/FileSystemService.cs index 6a70691126..aaa4bd883c 100644 --- a/src/NuGetGallery/Services/FileSystemService.cs +++ b/src/NuGetGallery/Services/FileSystemService.cs @@ -51,5 +51,10 @@ public IFileReference GetFileReference(string path) var info = new FileInfo(path); return info.Exists ? new LocalFileReference(info) : null; } + + public virtual void Copy(string sourceFileName, string destFileName, bool overwrite) + { + File.Copy(sourceFileName, destFileName, overwrite); + } } } \ No newline at end of file diff --git a/src/NuGetGallery/Services/IFileSystemService.cs b/src/NuGetGallery/Services/IFileSystemService.cs index 59ced9a68e..723a031530 100644 --- a/src/NuGetGallery/Services/IFileSystemService.cs +++ b/src/NuGetGallery/Services/IFileSystemService.cs @@ -16,5 +16,7 @@ public interface IFileSystemService DateTimeOffset GetCreationTimeUtc(string path); IFileReference GetFileReference(string path); + + void Copy(string sourceFileName, string destFileName, bool overwrite); } } \ No newline at end of file diff --git a/tests/NuGetGallery.Core.Facts/Services/CloudBlobCoreFileStorageServiceFacts.cs b/tests/NuGetGallery.Core.Facts/Services/CloudBlobCoreFileStorageServiceFacts.cs index d2529020b0..1e21ae0266 100644 --- a/tests/NuGetGallery.Core.Facts/Services/CloudBlobCoreFileStorageServiceFacts.cs +++ b/tests/NuGetGallery.Core.Facts/Services/CloudBlobCoreFileStorageServiceFacts.cs @@ -625,5 +625,223 @@ private static Tuple, Mock, Uri> Setup( return Tuple.Create(fakeBlobClient, fakeBlob, blobUri); } } + + public class TheCopyFileAsyncMethod + { + private string _srcFolderName; + private string _srcFileName; + private string _srcETag; + private BlobProperties _srcProperties; + private string _destFolderName; + private string _destFileName; + private string _destETag; + private BlobProperties _destProperties; + private CopyState _destCopyState; + private Mock _blobClient; + private Mock _srcContainer; + private Mock _destContainer; + private Mock _srcBlobMock; + private Mock _destBlobMock; + private CloudBlobCoreFileStorageService _target; + + public TheCopyFileAsyncMethod() + { + _srcFolderName = "validation"; + _srcFileName = "4b6f16cc-7acd-45eb-ac21-33f0d927ec14/nuget.versioning.4.5.0.nupkg"; + _srcETag = "\"src-etag\""; + _srcProperties = new BlobProperties(); + _destFolderName = "packages"; + _destFileName = "nuget.versioning.4.5.0.nupkg"; + _destETag = "\"dest-etag\""; + _destProperties = new BlobProperties(); + _destCopyState = new CopyState(); + SetDestCopyStatus(CopyStatus.Success); + + _blobClient = new Mock(); + _srcContainer = new Mock(); + _destContainer = new Mock(); + _srcBlobMock = new Mock(); + _destBlobMock = new Mock(); + _blobClient + .Setup(x => x.GetContainerReference(_srcFolderName)) + .Returns(() => _srcContainer.Object); + _blobClient + .Setup(x => x.GetContainerReference(_destFolderName)) + .Returns(() => _destContainer.Object); + _srcContainer + .Setup(x => x.GetBlobReference(_srcFileName)) + .Returns(() => _srcBlobMock.Object); + _destContainer + .Setup(x => x.GetBlobReference(_destFileName)) + .Returns(() => _destBlobMock.Object); + _srcBlobMock + .Setup(x => x.Name) + .Returns(() => _srcFileName); + _srcBlobMock + .Setup(x => x.ETag) + .Returns(() => _srcETag); + _srcBlobMock + .Setup(x => x.Properties) + .Returns(() => _srcProperties); + _destBlobMock + .Setup(x => x.ETag) + .Returns(() => _destETag); + _destBlobMock + .Setup(x => x.Properties) + .Returns(() => _destProperties); + _destBlobMock + .Setup(x => x.CopyState) + .Returns(() => _destCopyState); + + _target = CreateService(fakeBlobClient: _blobClient); + } + + [Fact] + public async Task WillCopyTheFileIfDestinationDoesNotExist() + { + // Arrange + AccessCondition srcAccessCondition = null; + AccessCondition destAccessCondition = null; + ISimpleCloudBlob srcBlob = null; + + _destBlobMock + .Setup(x => x.StartCopyAsync(It.IsAny(), It.IsAny(), It.IsAny())) + .Returns(Task.FromResult(0)) + .Callback((b, s, d) => + { + srcBlob = b; + srcAccessCondition = s; + destAccessCondition = d; + }); + + // Act + await _target.CopyFileAsync(_srcFolderName, _srcFileName, _destFolderName, _destFileName); + + // Assert + _destBlobMock.Verify( + x => x.StartCopyAsync(It.IsAny(), It.IsAny(), It.IsAny()), + Times.Once); + Assert.Equal(_srcFileName, srcBlob.Name); + Assert.Equal(_srcETag, srcAccessCondition.IfMatchETag); + Assert.Equal("*", destAccessCondition.IfNoneMatchETag); + } + + [Fact] + public async Task WillThrowInvalidOperationExceptionForConflict() + { + // Arrange + _destBlobMock + .Setup(x => x.StartCopyAsync(It.IsAny(), It.IsAny(), It.IsAny())) + .Throws(new StorageException(new RequestResult { HttpStatusCode = (int)HttpStatusCode.Conflict }, "Conflict!", inner: null)); + + // Act & Assert + await Assert.ThrowsAsync( + () => _target.CopyFileAsync(_srcFolderName, _srcFileName, _destFolderName, _destFileName)); + } + + [Fact] + public async Task WillCopyTheFileIfDestinationHasFailedCopy() + { + // Arrange + AccessCondition srcAccessCondition = null; + AccessCondition destAccessCondition = null; + ISimpleCloudBlob srcBlob = null; + + SetDestCopyStatus(CopyStatus.Failed); + + _destBlobMock + .Setup(x => x.StartCopyAsync(It.IsAny(), It.IsAny(), It.IsAny())) + .Returns(Task.FromResult(0)) + .Callback((b, s, d) => + { + srcBlob = b; + srcAccessCondition = s; + destAccessCondition = d; + SetDestCopyStatus(CopyStatus.Pending); + }); + + _destBlobMock + .Setup(x => x.ExistsAsync()) + .ReturnsAsync(true); + + _destBlobMock + .Setup(x => x.FetchAttributesAsync()) + .Returns(Task.FromResult(0)) + .Callback(() => SetDestCopyStatus(CopyStatus.Success)); + + // Act + await _target.CopyFileAsync(_srcFolderName, _srcFileName, _destFolderName, _destFileName); + + // Assert + _destBlobMock.Verify( + x => x.StartCopyAsync(It.IsAny(), It.IsAny(), It.IsAny()), + Times.Once); + Assert.Equal(_srcFileName, srcBlob.Name); + Assert.Equal(_srcETag, srcAccessCondition.IfMatchETag); + Assert.Equal(_destETag, destAccessCondition.IfMatchETag); + } + + [Fact] + public async Task NoOpsIfPackageLengthAndHashMatch() + { + // Arrange + SetBlobContentMD5(_srcProperties, "mwgwUC0MwohHxgMmvQzO7A=="); + SetBlobLength(_srcProperties, 42); + SetBlobContentMD5(_destProperties, _srcProperties.ContentMD5); + SetBlobLength(_destProperties, _srcProperties.Length); + + _destBlobMock + .Setup(x => x.ExistsAsync()) + .ReturnsAsync(true); + + // Act + await _target.CopyFileAsync(_srcFolderName, _srcFileName, _destFolderName, _destFileName); + + // Assert + _destBlobMock.Verify( + x => x.StartCopyAsync(It.IsAny(), It.IsAny(), It.IsAny()), + Times.Never); + } + + [Fact] + public async Task ThrowsIfCopyOperationFails() + { + // Arrange + _destBlobMock + .Setup(x => x.StartCopyAsync(It.IsAny(), It.IsAny(), It.IsAny())) + .Returns(Task.FromResult(0)) + .Callback((_, __, ___) => + { + SetDestCopyStatus(CopyStatus.Failed); + }); + + // Act & Assert + var ex = await Assert.ThrowsAsync( + () => _target.CopyFileAsync(_srcFolderName, _srcFileName, _destFolderName, _destFileName)); + Assert.Contains("The blob copy operation had copy status Failed", ex.Message); + } + + private void SetDestCopyStatus(CopyStatus copyStatus) + { + // We have to use reflection because the setter is not public. + typeof(CopyState) + .GetProperty(nameof(CopyState.Status)) + .SetValue(_destCopyState, copyStatus, null); + } + + private void SetBlobLength(BlobProperties properties, long length) + { + typeof(BlobProperties) + .GetProperty(nameof(BlobProperties.Length)) + .SetValue(properties, length, null); + } + + private void SetBlobContentMD5(BlobProperties properties, string contentMD5) + { + typeof(BlobProperties) + .GetProperty(nameof(BlobProperties.ContentMD5)) + .SetValue(properties, contentMD5, null); + } + } } } diff --git a/tests/NuGetGallery.Facts/Services/FileSystemFileStorageServiceFacts.cs b/tests/NuGetGallery.Facts/Services/FileSystemFileStorageServiceFacts.cs index 7395046d1c..a95314dcc9 100644 --- a/tests/NuGetGallery.Facts/Services/FileSystemFileStorageServiceFacts.cs +++ b/tests/NuGetGallery.Facts/Services/FileSystemFileStorageServiceFacts.cs @@ -290,6 +290,102 @@ public async Task WillReturnNullWhenRequestedFileDoesNotExist() } } + public class TheCopyFileAsyncMethod : IDisposable + { + private readonly TestDirectory _directory; + private readonly string _srcFolderName; + private readonly string _srcFileName; + private readonly string _destFolderName; + private readonly string _destFileName; + private readonly Mock _appConfiguration; + private readonly Mock _fileSystemService; + private readonly FileSystemFileStorageService _target; + + public TheCopyFileAsyncMethod() + { + _directory = TestDirectory.Create(); + + _srcFolderName = "validation"; + _srcFileName = "4b6f16cc-7acd-45eb-ac21-33f0d927ec14/nuget.versioning.4.5.0.nupkg"; + _destFolderName = "packages"; + _destFileName = "nuget.versioning.4.5.0.nupkg"; + + _appConfiguration = new Mock(); + _appConfiguration + .Setup(x => x.FileStorageDirectory) + .Returns(() => _directory); + + _fileSystemService = new Mock { CallBase = true }; + + _target = new FileSystemFileStorageService( + _appConfiguration.Object, + _fileSystemService.Object); + } + + [Fact] + public async Task CopiesFileWhenDestinationDoesNotExist() + { + // Arrange + var content = "Hello, world!"; + await _target.SaveFileAsync( + _srcFolderName, + _srcFileName, + new MemoryStream(Encoding.ASCII.GetBytes(content))); + + // Act + await _target.CopyFileAsync(_srcFolderName, _srcFileName, _destFolderName, _destFileName); + + // Assert + using (var destStream = await _target.GetFileAsync(_destFolderName, _destFileName)) + using (var reader = new StreamReader(destStream)) + { + Assert.Equal(content, reader.ReadToEnd()); + } + } + + [Fact] + public async Task ThrowsWhenDestinationIsSame() + { + // Arrange + var content = "Hello, world!"; + await _target.SaveFileAsync( + _srcFolderName, + _srcFileName, + new MemoryStream(Encoding.ASCII.GetBytes(content))); + await _target.SaveFileAsync( + _destFolderName, + _destFileName, + new MemoryStream(Encoding.ASCII.GetBytes(content))); + + await Assert.ThrowsAsync( + () => _target.CopyFileAsync(_srcFolderName, _srcFileName, _destFolderName, _destFileName)); + } + + [Fact] + public async Task ThrowsWhenFileAlreadyExists() + { + // Arrange + var content = "Hello, world!"; + await _target.SaveFileAsync( + _srcFolderName, + _srcFileName, + new MemoryStream(Encoding.ASCII.GetBytes(content))); + await _target.SaveFileAsync( + _destFolderName, + _destFileName, + new MemoryStream(Encoding.ASCII.GetBytes("Something else."))); + + // Act & Assert + await Assert.ThrowsAsync( + () => _target.CopyFileAsync(_srcFolderName, _srcFileName, _destFolderName, _destFileName)); + } + + public void Dispose() + { + _directory?.Dispose(); + } + } + public class TheSaveFileMethod { private const string FolderName = "theFolderName";