Skip to content

Commit

Permalink
Fix for broken package delete (#6642)
Browse files Browse the repository at this point in the history
* Package delete was broken with recent update. This fixes it.

* Moved folder names into its own class.
Updated tests to include All folders.

* Added tests to verify all folders are covered by tests.
  • Loading branch information
agr authored and Scott Bommarito committed Nov 9, 2018
1 parent e43bd55 commit 2284a50
Show file tree
Hide file tree
Showing 22 changed files with 193 additions and 167 deletions.
31 changes: 17 additions & 14 deletions src/NuGetGallery.Core/CoreConstants.cs
Expand Up @@ -25,21 +25,24 @@ public static class CoreConstants

public const string DefaultCacheControl = "max-age=120";

public const string UserCertificatesFolderName = "user-certificates";
public const string ContentFolderName = "content";
public const string DownloadsFolderName = "downloads";
public const string PackageBackupsFolderName = "package-backups";
public const string PackageReadMesFolderName = "readmes";
public const string PackagesFolderName = "packages";
public const string PackagesContentFolderName = "packages-content";
public const string UploadsFolderName = "uploads";
public const string ValidationFolderName = "validation";
public const string RevalidationFolderName = "revalidation";
public const string StatusFolderName = "status";

public const string SymbolPackagesFolderName = "symbol-packages";
public static class Folders
{
public const string UserCertificatesFolderName = "user-certificates";
public const string ContentFolderName = "content";
public const string DownloadsFolderName = "downloads";
public const string PackageBackupsFolderName = "package-backups";
public const string PackageReadMesFolderName = "readmes";
public const string PackagesFolderName = "packages";
public const string PackagesContentFolderName = "packages-content";
public const string UploadsFolderName = "uploads";
public const string ValidationFolderName = "validation";
public const string RevalidationFolderName = "revalidation";
public const string StatusFolderName = "status";
public const string SymbolPackagesFolderName = "symbol-packages";
public const string SymbolPackageBackupsFolderName = "symbol-package-backups";
}

public const string NuGetSymbolPackageFileExtension = ".snupkg";
public const string SymbolPackageBackupsFolderName = "symbol-package-backups";

public const string UploadTracingKeyHeaderName = "upload-id";

Expand Down
77 changes: 39 additions & 38 deletions src/NuGetGallery.Core/Services/CloudBlobCoreFileStorageService.cs
Expand Up @@ -28,21 +28,22 @@ public class CloudBlobCoreFileStorageService : ICoreFileStorageService
private static readonly TimeSpan CopyPollFrequency = TimeSpan.FromMilliseconds(500);

private static readonly HashSet<string> KnownPublicFolders = new HashSet<string> {
CoreConstants.PackagesFolderName,
CoreConstants.PackageBackupsFolderName,
CoreConstants.DownloadsFolderName,
CoreConstants.SymbolPackagesFolderName,
CoreConstants.SymbolPackageBackupsFolderName
CoreConstants.Folders.PackagesFolderName,
CoreConstants.Folders.PackageBackupsFolderName,
CoreConstants.Folders.DownloadsFolderName,
CoreConstants.Folders.SymbolPackagesFolderName,
CoreConstants.Folders.SymbolPackageBackupsFolderName
};

private static readonly HashSet<string> KnownPrivateFolders = new HashSet<string> {
CoreConstants.ContentFolderName,
CoreConstants.UploadsFolderName,
CoreConstants.PackageReadMesFolderName,
CoreConstants.ValidationFolderName,
CoreConstants.UserCertificatesFolderName,
CoreConstants.RevalidationFolderName,
CoreConstants.StatusFolderName,
CoreConstants.Folders.ContentFolderName,
CoreConstants.Folders.UploadsFolderName,
CoreConstants.Folders.PackageReadMesFolderName,
CoreConstants.Folders.ValidationFolderName,
CoreConstants.Folders.UserCertificatesFolderName,
CoreConstants.Folders.RevalidationFolderName,
CoreConstants.Folders.StatusFolderName,
CoreConstants.Folders.PackagesContentFolderName,
};

protected readonly ICloudBlobClient _client;
Expand Down Expand Up @@ -613,29 +614,29 @@ private static string GetContentType(string folderName)
{
switch (folderName)
{
case CoreConstants.PackagesFolderName:
case CoreConstants.PackageBackupsFolderName:
case CoreConstants.UploadsFolderName:
case CoreConstants.ValidationFolderName:
case CoreConstants.SymbolPackagesFolderName:
case CoreConstants.SymbolPackageBackupsFolderName:
case CoreConstants.Folders.PackagesFolderName:
case CoreConstants.Folders.PackageBackupsFolderName:
case CoreConstants.Folders.UploadsFolderName:
case CoreConstants.Folders.ValidationFolderName:
case CoreConstants.Folders.SymbolPackagesFolderName:
case CoreConstants.Folders.SymbolPackageBackupsFolderName:
return CoreConstants.PackageContentType;

case CoreConstants.DownloadsFolderName:
case CoreConstants.Folders.DownloadsFolderName:
return CoreConstants.OctetStreamContentType;

case CoreConstants.PackageReadMesFolderName:
case CoreConstants.Folders.PackageReadMesFolderName:
return CoreConstants.TextContentType;

case CoreConstants.ContentFolderName:
case CoreConstants.RevalidationFolderName:
case CoreConstants.StatusFolderName:
case CoreConstants.Folders.ContentFolderName:
case CoreConstants.Folders.RevalidationFolderName:
case CoreConstants.Folders.StatusFolderName:
return CoreConstants.JsonContentType;

case CoreConstants.UserCertificatesFolderName:
case CoreConstants.Folders.UserCertificatesFolderName:
return CoreConstants.CertificateContentType;

case CoreConstants.PackagesContentFolderName:
case CoreConstants.Folders.PackagesContentFolderName:
return CoreConstants.OctetStreamContentType;

default:
Expand All @@ -648,21 +649,21 @@ private static string GetCacheControl(string folderName)
{
switch (folderName)
{
case CoreConstants.PackagesFolderName:
case CoreConstants.SymbolPackagesFolderName:
case CoreConstants.PackagesContentFolderName:
case CoreConstants.ValidationFolderName:
case CoreConstants.Folders.PackagesFolderName:
case CoreConstants.Folders.SymbolPackagesFolderName:
case CoreConstants.Folders.ValidationFolderName:
return CoreConstants.DefaultCacheControl;

case CoreConstants.PackageBackupsFolderName:
case CoreConstants.UploadsFolderName:
case CoreConstants.SymbolPackageBackupsFolderName:
case CoreConstants.DownloadsFolderName:
case CoreConstants.PackageReadMesFolderName:
case CoreConstants.ContentFolderName:
case CoreConstants.RevalidationFolderName:
case CoreConstants.StatusFolderName:
case CoreConstants.UserCertificatesFolderName:
case CoreConstants.Folders.PackageBackupsFolderName:
case CoreConstants.Folders.UploadsFolderName:
case CoreConstants.Folders.SymbolPackageBackupsFolderName:
case CoreConstants.Folders.DownloadsFolderName:
case CoreConstants.Folders.PackageReadMesFolderName:
case CoreConstants.Folders.ContentFolderName:
case CoreConstants.Folders.RevalidationFolderName:
case CoreConstants.Folders.StatusFolderName:
case CoreConstants.Folders.UserCertificatesFolderName:
case CoreConstants.Folders.PackagesContentFolderName:
return null;

default:
Expand Down
8 changes: 4 additions & 4 deletions src/NuGetGallery.Core/Services/PackageFileServiceMetadata.cs
Expand Up @@ -5,19 +5,19 @@ namespace NuGetGallery
{
public class PackageFileMetadataService : IFileMetadataService
{
public string FileFolderName => CoreConstants.PackagesFolderName;
public string FileFolderName => CoreConstants.Folders.PackagesFolderName;

public string PackageContentFolderName => CoreConstants.PackagesContentFolderName;
public string PackageContentFolderName => CoreConstants.Folders.PackagesContentFolderName;

public string PackageContentPathTemplate => CoreConstants.PackageContentFileSavePathTemplate;

public string FileSavePathTemplate => CoreConstants.PackageFileSavePathTemplate;

public string FileExtension => CoreConstants.NuGetPackageFileExtension;

public string ValidationFolderName => CoreConstants.ValidationFolderName;
public string ValidationFolderName => CoreConstants.Folders.ValidationFolderName;

public string FileBackupsFolderName => CoreConstants.PackageBackupsFolderName;
public string FileBackupsFolderName => CoreConstants.Folders.PackageBackupsFolderName;

public string FileBackupSavePathTemplate => CoreConstants.PackageFileBackupSavePathTemplate;
}
Expand Down
8 changes: 4 additions & 4 deletions src/NuGetGallery.Core/Services/RevalidationStateService.cs
Expand Up @@ -67,7 +67,7 @@ public async Task<RevalidationState> MaybeUpdateStateAsync(Func<RevalidationStat
jsonWriter.Flush();
stream.Position = 0;

await _storage.SaveFileAsync(CoreConstants.RevalidationFolderName, StateFileName, stream, accessCondition);
await _storage.SaveFileAsync(CoreConstants.Folders.RevalidationFolderName, StateFileName, stream, accessCondition);
}

return state;
Expand All @@ -80,11 +80,11 @@ public async Task<RevalidationState> MaybeUpdateStateAsync(Func<RevalidationStat

private async Task<InternalState> GetInternalStateAsync()
{
var fileReference = await _storage.GetFileReferenceAsync(CoreConstants.RevalidationFolderName, StateFileName);
var fileReference = await _storage.GetFileReferenceAsync(CoreConstants.Folders.RevalidationFolderName, StateFileName);

if (fileReference == null)
{
throw new InvalidOperationException($"Could not find file '{StateFileName}' in folder '{CoreConstants.RevalidationFolderName}'");
throw new InvalidOperationException($"Could not find file '{StateFileName}' in folder '{CoreConstants.Folders.RevalidationFolderName}'");
}

using (var fileStream = fileReference.OpenRead())
Expand All @@ -95,7 +95,7 @@ private async Task<InternalState> GetInternalStateAsync()

if (state == null)
{
throw new InvalidOperationException($"State blob '{StateFileName}' in folder '{CoreConstants.RevalidationFolderName}' is malformed");
throw new InvalidOperationException($"State blob '{StateFileName}' in folder '{CoreConstants.Folders.RevalidationFolderName}' is malformed");
}

return new InternalState(fileReference, state);
Expand Down
Expand Up @@ -7,7 +7,7 @@ namespace NuGetGallery
{
public class SymbolPackageFileMetadataService : IFileMetadataService
{
public string FileFolderName => CoreConstants.SymbolPackagesFolderName;
public string FileFolderName => CoreConstants.Folders.SymbolPackagesFolderName;

public string PackageContentFolderName => throw new NotImplementedException();
public string PackageContentPathTemplate => throw new NotImplementedException();
Expand All @@ -16,9 +16,9 @@ public class SymbolPackageFileMetadataService : IFileMetadataService

public string FileExtension => CoreConstants.NuGetSymbolPackageFileExtension;

public string ValidationFolderName => CoreConstants.ValidationFolderName;
public string ValidationFolderName => CoreConstants.Folders.ValidationFolderName;

public string FileBackupsFolderName => CoreConstants.SymbolPackageBackupsFolderName;
public string FileBackupsFolderName => CoreConstants.Folders.SymbolPackageBackupsFolderName;

public string FileBackupSavePathTemplate => CoreConstants.PackageFileBackupSavePathTemplate;
}
Expand Down
2 changes: 1 addition & 1 deletion src/NuGetGallery/Services/CertificateService.cs
Expand Up @@ -172,7 +172,7 @@ private async Task SaveToFileStorageAsync(CertificateFile certificateFile)
try
{
await _fileStorageService.SaveFileAsync(
CoreConstants.UserCertificatesFolderName,
CoreConstants.Folders.UserCertificatesFolderName,
filePath,
certificateFile.Stream,
overwrite: false);
Expand Down
2 changes: 1 addition & 1 deletion src/NuGetGallery/Services/CloudBlobFileStorageService.cs
Expand Up @@ -97,7 +97,7 @@ internal Uri GetRedirectUri(Uri requestUrl, Uri blobUri)

public async Task<bool> IsAvailableAsync()
{
var container = await GetContainerAsync(CoreConstants.PackagesFolderName);
var container = await GetContainerAsync(CoreConstants.Folders.PackagesFolderName);
return await container.ExistsAsync();
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/NuGetGallery/Services/ContentService.cs
Expand Up @@ -124,7 +124,7 @@ private async Task<ContentItem> RefreshContentFromFile(string fileName, ContentI
using (Trace.Activity("Downloading Content Item: " + fileName))
{
IFileReference reference = await FileStorage.GetFileReferenceAsync(
CoreConstants.ContentFolderName,
CoreConstants.Folders.ContentFolderName,
fileName,
ifNoneMatch: cachedItem?.ContentId);

Expand Down
6 changes: 3 additions & 3 deletions src/NuGetGallery/Services/FileSystemFileStorageService.cs
Expand Up @@ -299,11 +299,11 @@ private static string GetContentType(string folderName)
{
switch (folderName)
{
case CoreConstants.PackagesFolderName:
case CoreConstants.SymbolPackagesFolderName:
case CoreConstants.Folders.PackagesFolderName:
case CoreConstants.Folders.SymbolPackagesFolderName:
return CoreConstants.PackageContentType;

case CoreConstants.DownloadsFolderName:
case CoreConstants.Folders.DownloadsFolderName:
return CoreConstants.OctetStreamContentType;

default:
Expand Down
10 changes: 5 additions & 5 deletions src/NuGetGallery/Services/PackageFileService.cs
Expand Up @@ -28,13 +28,13 @@ public PackageFileService(IFileStorageService fileStorageService)
public Task<ActionResult> CreateDownloadPackageActionResultAsync(Uri requestUrl, Package package)
{
var fileName = BuildFileName(package, CoreConstants.PackageFileSavePathTemplate, CoreConstants.NuGetPackageFileExtension);
return _fileStorageService.CreateDownloadFileActionResultAsync(requestUrl, CoreConstants.PackagesFolderName, fileName);
return _fileStorageService.CreateDownloadFileActionResultAsync(requestUrl, CoreConstants.Folders.PackagesFolderName, fileName);
}

public Task<ActionResult> CreateDownloadPackageActionResultAsync(Uri requestUrl, string id, string version)
{
var fileName = BuildFileName(id, version, CoreConstants.PackageFileSavePathTemplate, CoreConstants.NuGetPackageFileExtension);
return _fileStorageService.CreateDownloadFileActionResultAsync(requestUrl, CoreConstants.PackagesFolderName, fileName);
return _fileStorageService.CreateDownloadFileActionResultAsync(requestUrl, CoreConstants.Folders.PackagesFolderName, fileName);
}

/// <summary>
Expand All @@ -50,7 +50,7 @@ public Task DeleteReadMeMdFileAsync(Package package)

var fileName = BuildFileName(package, ReadMeFilePathTemplateActive, GalleryConstants.MarkdownFileExtension);

return _fileStorageService.DeleteFileAsync(CoreConstants.PackageReadMesFolderName, fileName);
return _fileStorageService.DeleteFileAsync(CoreConstants.Folders.PackageReadMesFolderName, fileName);
}

/// <summary>
Expand All @@ -69,7 +69,7 @@ public async Task SaveReadMeMdFileAsync(Package package, string readMeMd)

using (var readMeMdStream = new MemoryStream(Encoding.UTF8.GetBytes(readMeMd)))
{
await _fileStorageService.SaveFileAsync(CoreConstants.PackageReadMesFolderName, fileName, readMeMdStream, overwrite: true);
await _fileStorageService.SaveFileAsync(CoreConstants.Folders.PackageReadMesFolderName, fileName, readMeMdStream, overwrite: true);
}
}

Expand All @@ -86,7 +86,7 @@ public async Task<string> DownloadReadMeMdFileAsync(Package package)

var fileName = BuildFileName(package, ReadMeFilePathTemplateActive, GalleryConstants.MarkdownFileExtension);

using (var readMeMdStream = await _fileStorageService.GetFileAsync(CoreConstants.PackageReadMesFolderName, fileName))
using (var readMeMdStream = await _fileStorageService.GetFileAsync(CoreConstants.Folders.PackageReadMesFolderName, fileName))
{
// Note that fileStorageService implementations return null if not found.
if (readMeMdStream != null)
Expand Down
4 changes: 2 additions & 2 deletions src/NuGetGallery/Services/SymbolPackageFileService.cs
Expand Up @@ -21,13 +21,13 @@ public SymbolPackageFileService(IFileStorageService fileStorageService)
public Task<ActionResult> CreateDownloadSymbolPackageActionResultAsync(Uri requestUrl, SymbolPackage symbolPackage)
{
var fileName = BuildFileName(symbolPackage.Package, CoreConstants.PackageFileSavePathTemplate, CoreConstants.NuGetSymbolPackageFileExtension);
return _fileStorageService.CreateDownloadFileActionResultAsync(requestUrl, CoreConstants.SymbolPackagesFolderName, fileName);
return _fileStorageService.CreateDownloadFileActionResultAsync(requestUrl, CoreConstants.Folders.SymbolPackagesFolderName, fileName);
}

public Task<ActionResult> CreateDownloadSymbolPackageActionResultAsync(Uri requestUrl, string id, string version)
{
var fileName = BuildFileName(id, version, CoreConstants.PackageFileSavePathTemplate, CoreConstants.NuGetSymbolPackageFileExtension);
return _fileStorageService.CreateDownloadFileActionResultAsync(requestUrl, CoreConstants.SymbolPackagesFolderName, fileName);
return _fileStorageService.CreateDownloadFileActionResultAsync(requestUrl, CoreConstants.Folders.SymbolPackagesFolderName, fileName);
}
}
}
6 changes: 3 additions & 3 deletions src/NuGetGallery/Services/UploadFileService.cs
Expand Up @@ -25,7 +25,7 @@ public Task DeleteUploadFileAsync(int userKey)

var uploadFileName = BuildFileName(userKey);

return _fileStorageService.DeleteFileAsync(CoreConstants.UploadsFolderName, uploadFileName);
return _fileStorageService.DeleteFileAsync(CoreConstants.Folders.UploadsFolderName, uploadFileName);
}

public Task<Stream> GetUploadFileAsync(int userKey)
Expand Down Expand Up @@ -54,7 +54,7 @@ public Task SaveUploadFileAsync(int userKey, Stream packageFileStream)
packageFileStream.Position = 0;

var uploadFileName = BuildFileName(userKey);
return _fileStorageService.SaveFileAsync(CoreConstants.UploadsFolderName, uploadFileName, packageFileStream);
return _fileStorageService.SaveFileAsync(CoreConstants.Folders.UploadsFolderName, uploadFileName, packageFileStream);
}

private static string BuildFileName(int userKey)
Expand All @@ -66,7 +66,7 @@ private static string BuildFileName(int userKey)
private async Task<Stream> GetUploadFileAsyncCore(int userKey)
{
var uploadFileName = BuildFileName(userKey);
return await _fileStorageService.GetFileAsync(CoreConstants.UploadsFolderName, uploadFileName);
return await _fileStorageService.GetFileAsync(CoreConstants.Folders.UploadsFolderName, uploadFileName);
}
}
}

0 comments on commit 2284a50

Please sign in to comment.