License storage related methods extracted into their own class (#6674)

* Extracted license-related storage code into its own class with its own interface, update the DI configuration.

* One test  fixed

* Updated storage tests

* Moved the RevalidationStateService into StorageDependent list, added its own connection string to the configuration.
Need to test that it still works.

* Renamed the revalidation connection string key.

* Updated the StorageDependentsHaveExpectedTypes test.

* Moved license file extraction code to CoreLicenseFileService.
Moved some test utilities to Core test project to enable testing moved code.

* Test fix, organized usings.

* Tests for the ExtractAndSaveLicenseFileAsync method.

* Extracted content-related file settings into their own interface/implementation and added it to DI.

* Added flatcontainer to the list of known folders.

* Allowing license files.

* Put back warnings about using the license URL and about not specifying any license at all.

* Enabled previously disabled tests and did some fixes to accomodate for wording and logic changes.

Author: agr

src/NuGetGallery.Core/CoreConstants.cs

@@ -40,6 +40,7 @@ public static class Folders
             public const string StatusFolderName = "status";
             public const string SymbolPackagesFolderName = "symbol-packages";
             public const string SymbolPackageBackupsFolderName = "symbol-package-backups";
+            public const string FlatContainerFolderName = "v3-flatcontainer";
         }
 
         public const string NuGetSymbolPackageFileExtension = ".snupkg";

src/NuGetGallery.Core/NuGetGallery.Core.csproj

@@ -166,10 +166,14 @@
     <Compile Include="Services\CloudBlobCoreFileStorageService.cs" />
     <Compile Include="Services\CloudBlobWrapper.cs" />
     <Compile Include="Services\CloudFileReference.cs" />
+    <Compile Include="Services\CoreLicenseFileService.cs" />
     <Compile Include="Services\CorePackageFileService.cs" />
     <Compile Include="Services\CoreSymbolPackageService.cs" />
     <Compile Include="Services\CorePackageService.cs" />
     <Compile Include="Services\CryptographyService.cs" />
+    <Compile Include="Services\FileNameHelper.cs" />
+    <Compile Include="Services\IContentFileMetadataService.cs" />
+    <Compile Include="Services\ICoreLicenseFileService.cs" />
     <Compile Include="Services\PackageAlreadyExistsException.cs" />
     <Compile Include="Services\FileAlreadyExistsException.cs" />
     <Compile Include="Services\FileUriPermissions.cs" />

src/NuGetGallery.Core/Services/CloudBlobCoreFileStorageService.cs

@@ -32,7 +32,8 @@ public class CloudBlobCoreFileStorageService : ICoreFileStorageService
             CoreConstants.Folders.PackageBackupsFolderName,
             CoreConstants.Folders.DownloadsFolderName,
             CoreConstants.Folders.SymbolPackagesFolderName,
-            CoreConstants.Folders.SymbolPackageBackupsFolderName
+            CoreConstants.Folders.SymbolPackageBackupsFolderName,
+            CoreConstants.Folders.FlatContainerFolderName,
         };
 
         private static readonly HashSet<string> KnownPrivateFolders = new HashSet<string> {
@@ -620,6 +621,7 @@ private static string GetContentType(string folderName)
                 case CoreConstants.Folders.ValidationFolderName:
                 case CoreConstants.Folders.SymbolPackagesFolderName:
                 case CoreConstants.Folders.SymbolPackageBackupsFolderName:
+                case CoreConstants.Folders.FlatContainerFolderName:
                     return CoreConstants.PackageContentType;
 
                 case CoreConstants.Folders.DownloadsFolderName:
@@ -664,6 +666,7 @@ private static string GetCacheControl(string folderName)
                 case CoreConstants.Folders.StatusFolderName:
                 case CoreConstants.Folders.UserCertificatesFolderName:
                 case CoreConstants.Folders.PackagesContentFolderName:
+                case CoreConstants.Folders.FlatContainerFolderName:
                     return null;
 
                 default:

src/NuGetGallery.Core/Services/CoreLicenseFileService.cs

@@ -0,0 +1,127 @@
+// 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.Threading.Tasks;
+using NuGet.Packaging;
+using NuGet.Services.Entities;
+using NuGetGallery.Packaging;
+
+namespace NuGetGallery
+{
+    public class CoreLicenseFileService : ICoreLicenseFileService
+    {
+        private const string LicenseFileName = "license";
+
+        private readonly ICoreFileStorageService _fileStorageService;
+        private readonly IContentFileMetadataService _metadata;
+
+        public CoreLicenseFileService(ICoreFileStorageService fileStorageService, IContentFileMetadataService metadata)
+        {
+            _fileStorageService = fileStorageService ?? throw new ArgumentNullException(nameof(fileStorageService));
+            _metadata = metadata ?? throw new ArgumentNullException(nameof(metadata));
+        }
+
+        public Task SaveLicenseFileAsync(Package package, Stream licenseFile)
+        {
+            if (package == null)
+            {
+                throw new ArgumentNullException(nameof(package));
+            }
+
+            if (licenseFile == null)
+            {
+                throw new ArgumentNullException(nameof(licenseFile));
+            }
+
+            if (package.EmbeddedLicenseType == EmbeddedLicenseFileType.Absent)
+            {
+                throw new ArgumentException("Package must have an embedded license", nameof(package));
+            }
+
+            var fileName = BuildLicenseFileName(package);
+
+            // Gallery will generally ignore the content type on license files and will use the value from the DB,
+            // but we'll be nice and try to specify correct content type for them.
+            var contentType = package.EmbeddedLicenseType == EmbeddedLicenseFileType.Markdown
+                ? CoreConstants.MarkdownContentType
+                : CoreConstants.TextContentType;
+
+            return _fileStorageService.SaveFileAsync(_metadata.PackageContentFolderName, fileName, contentType, licenseFile, overwrite: true);
+        }
+
+        public async Task ExtractAndSaveLicenseFileAsync(Package package, Stream packageStream)
+        {
+            if (package == null)
+            {
+                throw new ArgumentNullException(nameof(package));
+            }
+
+            if (packageStream == null)
+            {
+                throw new ArgumentNullException(nameof(packageStream));
+            }
+
+            packageStream.Seek(0, SeekOrigin.Begin);
+            using (var packageArchiveReader = new PackageArchiveReader(packageStream, leaveStreamOpen: true))
+            {
+                var packageMetadata = PackageMetadata.FromNuspecReader(packageArchiveReader.GetNuspecReader(), strict: true);
+                if (packageMetadata.LicenseMetadata == null || packageMetadata.LicenseMetadata.Type != LicenseType.File || string.IsNullOrWhiteSpace(packageMetadata.LicenseMetadata.License))
+                {
+                    throw new InvalidOperationException("No license file specified in the nuspec");
+                }
+
+                var filename = packageMetadata.LicenseMetadata.License;
+                var licenseFileEntry = packageArchiveReader.GetEntry(filename); // throws on non-existent file
+                using (var licenseFileStream = licenseFileEntry.Open())
+                {
+                    await SaveLicenseFileAsync(package, licenseFileStream);
+                }
+            }
+        }
+
+        public Task<Stream> DownloadLicenseFileAsync(Package package)
+        {
+            var fileName = BuildLicenseFileName(package);
+            return _fileStorageService.GetFileAsync(_metadata.PackageContentFolderName, fileName);
+        }
+
+        public Task DeleteLicenseFileAsync(string id, string version)
+        {
+            if (id == null)
+            {
+                throw new ArgumentNullException(nameof(id));
+            }
+
+            if (string.IsNullOrWhiteSpace(id))
+            {
+                throw new ArgumentException($"{nameof(id)} cannot be empty", nameof(id));
+            }
+
+            if (version == null)
+            {
+                throw new ArgumentNullException(nameof(version));
+            }
+
+            if (string.IsNullOrWhiteSpace(version))
+            {
+                throw new ArgumentException($"{nameof(version)} cannot be empty", nameof(version));
+            }
+
+            var normalizedVersion = NuGetVersionFormatter.Normalize(version);
+            var fileName = BuildLicenseFileName(id, normalizedVersion);
+
+            return _fileStorageService.DeleteFileAsync(_metadata.PackageContentFolderName, fileName);
+        }
+
+        private string LicensePathTemplate => $"{_metadata.PackageContentPathTemplate}/{LicenseFileName}";
+
+        private string BuildLicenseFileName(Package package)
+            => FileNameHelper.BuildFileName(package, LicensePathTemplate, string.Empty);
+
+        private string BuildLicenseFileName(string id, string version)
+            => FileNameHelper.BuildFileName(id, version, LicensePathTemplate, string.Empty);
+
+    }
+}

src/NuGetGallery.Core/Services/CorePackageFileService.cs

@@ -34,25 +34,25 @@ public Task SavePackageFileAsync(Package package, Stream packageFile, bool overw
                 throw new ArgumentNullException(nameof(packageFile));
             }
 
-            var fileName = BuildFileName(package, _metadata.FileSavePathTemplate, _metadata.FileExtension);
+            var fileName = FileNameHelper.BuildFileName(package, _metadata.FileSavePathTemplate, _metadata.FileExtension);
             return _fileStorageService.SaveFileAsync(_metadata.FileFolderName, fileName, packageFile, overwrite);
         }
 
         public Task<Stream> DownloadPackageFileAsync(Package package)
         {
-            var fileName = BuildFileName(package, _metadata.FileSavePathTemplate, _metadata.FileExtension);
+            var fileName = FileNameHelper.BuildFileName(package, _metadata.FileSavePathTemplate, _metadata.FileExtension);
             return _fileStorageService.GetFileAsync(_metadata.FileFolderName, fileName);
         }
 
         public Task<Uri> GetPackageReadUriAsync(Package package)
         {
-            var fileName = BuildFileName(package, _metadata.FileSavePathTemplate, _metadata.FileExtension);
+            var fileName = FileNameHelper.BuildFileName(package, _metadata.FileSavePathTemplate, _metadata.FileExtension);
             return _fileStorageService.GetFileReadUriAsync(_metadata.FileFolderName, fileName, endOfAccess: null);
         }
 
         public Task<bool> DoesPackageFileExistAsync(Package package)
         {
-            var fileName = BuildFileName(package, _metadata.FileSavePathTemplate, _metadata.FileExtension);
+            var fileName = FileNameHelper.BuildFileName(package, _metadata.FileSavePathTemplate, _metadata.FileExtension);
             return _fileStorageService.FileExistsAsync(_metadata.FileFolderName, fileName);
         }
 
@@ -63,7 +63,7 @@ public Task SaveValidationPackageFileAsync(Package package, Stream packageFile)
                 throw new ArgumentNullException(nameof(packageFile));
             }
 
-            var fileName = BuildFileName(
+            var fileName = FileNameHelper.BuildFileName(
                 package,
                 _metadata.FileSavePathTemplate,
                 _metadata.FileExtension);
@@ -77,7 +77,7 @@ public Task SaveValidationPackageFileAsync(Package package, Stream packageFile)
 
         public Task<Stream> DownloadValidationPackageFileAsync(Package package)
         {
-            var fileName = BuildFileName(
+            var fileName = FileNameHelper.BuildFileName(
                 package,
                 _metadata.FileSavePathTemplate,
                 _metadata.FileExtension);
@@ -93,7 +93,7 @@ public Task DeleteValidationPackageFileAsync(string id, string version)
             }
 
             var normalizedVersion = NuGetVersionFormatter.Normalize(version);
-            var fileName = BuildFileName(
+            var fileName = FileNameHelper.BuildFileName(
                 id,
                 normalizedVersion,
                 _metadata.FileSavePathTemplate,
@@ -116,15 +116,15 @@ public Task DeletePackageFileAsync(string id, string version)
 
             var normalizedVersion = NuGetVersionFormatter.Normalize(version);
 
-            var fileName = BuildFileName(id, normalizedVersion, _metadata.FileSavePathTemplate, _metadata.FileExtension);
+            var fileName = FileNameHelper.BuildFileName(id, normalizedVersion, _metadata.FileSavePathTemplate, _metadata.FileExtension);
             return _fileStorageService.DeleteFileAsync(_metadata.FileFolderName, fileName);
         }
 
         public Task<Uri> GetValidationPackageReadUriAsync(Package package, DateTimeOffset endOfAccess)
         {
             package = package ?? throw new ArgumentNullException(nameof(package));
 
-            var fileName = BuildFileName(
+            var fileName = FileNameHelper.BuildFileName(
                 package,
                 _metadata.FileSavePathTemplate,
                 _metadata.FileExtension);
@@ -134,7 +134,7 @@ public Task<Uri> GetValidationPackageReadUriAsync(Package package, DateTimeOffse
 
         public Task<bool> DoesValidationPackageFileExistAsync(Package package)
         {
-            var fileName = BuildFileName(package, _metadata.FileSavePathTemplate, _metadata.FileExtension);
+            var fileName = FileNameHelper.BuildFileName(package, _metadata.FileSavePathTemplate, _metadata.FileExtension);
             return _fileStorageService.FileExistsAsync(_metadata.ValidationFolderName, fileName);
         }
 
@@ -199,76 +199,6 @@ public async Task StorePackageFileInBackupLocationAsync(Package package, Stream
             }
         }
 
-        public Task SaveLicenseFileAsync(Package package, Stream licenseFile)
-        {
-            if (package == null)
-            {
-                throw new ArgumentNullException(nameof(package));
-            }
-
-            if (licenseFile == null)
-            {
-                throw new ArgumentNullException(nameof(licenseFile));
-            }
-
-            if (package.EmbeddedLicenseType == EmbeddedLicenseFileType.Absent)
-            {
-                throw new ArgumentException("Package must have an embedded license", nameof(package));
-            }
-
-            var fileName = BuildLicenseFileName(package);
-
-            // Gallery will generally ignore the content type on license files and will use the value from the DB,
-            // but we'll be nice and try to specify correct content type for them.
-            var contentType = package.EmbeddedLicenseType == EmbeddedLicenseFileType.Markdown
-                ? CoreConstants.MarkdownContentType
-                : CoreConstants.TextContentType;
-
-            return _fileStorageService.SaveFileAsync(_metadata.PackageContentFolderName, fileName, contentType, licenseFile, overwrite: true);
-        }
-
-        public Task<Stream> DownloadLicenseFileAsync(Package package)
-        {
-            var fileName = BuildLicenseFileName(package);
-            return _fileStorageService.GetFileAsync(_metadata.PackageContentFolderName, fileName);
-        }
-
-        public Task DeleteLicenseFileAsync(string id, string version)
-        {
-            if (id == null)
-            {
-                throw new ArgumentNullException(nameof(id));
-            }
-
-            if (string.IsNullOrWhiteSpace(id))
-            {
-                throw new ArgumentException($"{nameof(id)} cannot be empty", nameof(id));
-            }
-
-            if (version == null)
-            {
-                throw new ArgumentNullException(nameof(version));
-            }
-
-            if (string.IsNullOrWhiteSpace(version))
-            {
-                throw new ArgumentException($"{nameof(version)} cannot be empty", nameof(version));
-            }
-
-            var normalizedVersion = NuGetVersionFormatter.Normalize(version);
-            var fileName = BuildLicenseFileName(id, normalizedVersion);
-
-            return _fileStorageService.DeleteFileAsync(_metadata.PackageContentFolderName, fileName);
-        }
-
-        private string LicensePathTemplate => $"{_metadata.PackageContentPathTemplate}/{CoreConstants.LicenseFileName}";
-
-        private string BuildLicenseFileName(Package package)
-            => BuildFileName(package, LicensePathTemplate, string.Empty);
-
-        private string BuildLicenseFileName(string id, string version)
-            => BuildFileName(id, version, LicensePathTemplate, string.Empty);
-
         private static string BuildBackupFileName(string id, string version, string hash, string extension, string fileBackupSavePathTemplate)
         {
             if (id == null)
@@ -296,51 +226,5 @@ private static string BuildBackupFileName(string id, string version, string hash
                 HttpServerUtility.UrlTokenEncode(hashBytes),
                 extension);
         }
-
-        protected static string BuildFileName(Package package, string format, string extension)
-        {
-            if (package == null)
-            {
-                throw new ArgumentNullException(nameof(package));
-            }
-
-            if (package.PackageRegistration == null ||
-                String.IsNullOrWhiteSpace(package.PackageRegistration.Id) ||
-                (String.IsNullOrWhiteSpace(package.NormalizedVersion) && String.IsNullOrWhiteSpace(package.Version)))
-            {
-                throw new ArgumentException(CoreStrings.PackageIsMissingRequiredData, nameof(package));
-            }
-
-            return BuildFileName(
-                package.PackageRegistration.Id,
-                string.IsNullOrEmpty(package.NormalizedVersion) ?
-                    NuGetVersionFormatter.Normalize(package.Version) :
-                    package.NormalizedVersion, format, extension);
-        }
-
-        protected static string BuildFileName(string id, string version, string pathTemplate, string extension)
-        {
-            if (id == null)
-            {
-                throw new ArgumentNullException(nameof(id));
-            }
-
-            if (version == null)
-            {
-                throw new ArgumentNullException(nameof(version));
-            }
-
-            // Note: packages should be saved and retrieved in blob storage using the lower case version of their filename because
-            // a) package IDs can and did change case over time
-            // b) blob storage is case sensitive
-            // c) we don't want to hit the database just to look up the right case
-            // and remember - version can contain letters too.
-            return String.Format(
-                CultureInfo.InvariantCulture,
-                pathTemplate,
-                id.ToLowerInvariant(),
-                version.ToLowerInvariant(),
-                extension);
-        }
     }
 }