Skip to content

Commit b9d2054

Browse files
Lynn Dailyndaidaii
Lynn Dai
authored and
lyndaidaii
committed
rename & add more test cases
1 parent 7009097 commit b9d2054

9 files changed

Lines changed: 52 additions & 50 deletions

File tree

‎src/NuGetGallery.Services/Configuration/FeatureFlagService.cs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ public class FeatureFlagService : IFeatureFlagService
3838
private const string Get2FADismissFeedback = GalleryPrefix + "Get2FADismissFeedback";
3939
private const string UsabillaOnEveryPageFeatureName = GalleryPrefix + "UsabillaEveryPage";
4040
private const string PackageRenamesFeatureName = GalleryPrefix + "PackageRenames";
41-
private const string UploadEmbeddedReadmeFeatureName = GalleryPrefix + "UploadEmbeddedReadmeFeatureName";
41+
private const string EmbeddedReadmeFlightName = GalleryPrefix + "EmbeddedReadmes";
4242

4343
private readonly IFeatureFlagClient _client;
4444

@@ -193,9 +193,9 @@ public bool IsPackageRenamesEnabled(User user)
193193
return _client.IsEnabled(PackageRenamesFeatureName, user, defaultValue: false);
194194
}
195195

196-
public bool IsUploadEmbeddedReadmeEnabled(User user)
196+
public bool AreEmbeddedReadmesEnabled(User user)
197197
{
198-
return _client.IsEnabled(UploadEmbeddedReadmeFeatureName, user, defaultValue: false);
198+
return _client.IsEnabled(EmbeddedReadmeFlightName, user, defaultValue: false);
199199
}
200200
}
201201
}

‎src/NuGetGallery.Services/Configuration/IFeatureFlagService.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -148,8 +148,8 @@ public interface IFeatureFlagService
148148
bool IsPackageRenamesEnabled(User user);
149149

150150
/// <summary>
151-
/// Whether the user is able to the upload package with an embedded readme file.
151+
/// Whether the user is able to publish the package with an embedded readme file.
152152
/// </summary>
153-
bool IsUploadEmbeddedReadmeEnabled(User user);
153+
bool AreEmbeddedReadmesEnabled(User user);
154154
}
155155
}

‎src/NuGetGallery.Services/PackageManagement/PackageService.cs‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@ namespace NuGetGallery
2121
{
2222
public class PackageService : CorePackageService, IPackageService
2323
{
24+
private const string MarkdownFileExtension = ".md";
25+
2426
private readonly IAuditingService _auditingService;
2527
private readonly ITelemetryService _telemetryService;
2628
private readonly ISecurityPolicyService _securityPolicyService;
@@ -722,7 +724,6 @@ private string GetLicenseExpression(PackageMetadata packageMetadata)
722724

723725
private static EmbeddedLicenseFileType GetEmbeddedLicenseType(string licenseFileName)
724726
{
725-
const string MarkdownFileExtension = ".md";
726727
const string TextFileExtension = ".txt";
727728

728729
var extension = Path.GetExtension(licenseFileName);
@@ -742,21 +743,19 @@ private static EmbeddedLicenseFileType GetEmbeddedLicenseType(string licenseFile
742743

743744
private static EmbeddedReadmeFileType GetEmbeddedReadmeType(PackageMetadata packageMetadata)
744745
{
745-
const string MarkdownFileExtension = ".md";
746-
747746
if (packageMetadata.ReadmeFile == null)
748747
{
749748
return EmbeddedReadmeFileType.Absent;
750749
}
751750

752751
var extension = Path.GetExtension(packageMetadata.ReadmeFile);
753752

754-
if (MarkdownFileExtension.Equals(extension, StringComparison.OrdinalIgnoreCase))
753+
if (MarkdownFileExtension.Equals(extension, StringComparison.OrdinalIgnoreCase) || string.Empty == extension)
755754
{
756755
return EmbeddedReadmeFileType.Markdown;
757756
}
758757

759-
throw new ArgumentException($"Invalid file name: {packageMetadata.ReadmeFile}");
758+
throw new ArgumentException($"The file name for the package readme must have the \"md\" file extentsion: {packageMetadata.ReadmeFile}");
760759
}
761760

762761
private static void ValidateSupportedFrameworks(string[] supportedFrameworks)

‎src/NuGetGallery/App_Start/DefaultDependenciesModule.cs‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -340,7 +340,6 @@ protected override void Load(ContainerBuilder builder)
340340
.InstancePerLifetimeScope();
341341

342342
builder.RegisterType<PackageMetadataValidationService>()
343-
.AsSelf()
344343
.As<IPackageMetadataValidationService>()
345344
.InstancePerLifetimeScope();
346345

‎src/NuGetGallery/Services/PackageMetadataValidationService.cs‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ public class PackageMetadataValidationService : IPackageMetadataValidationServic
6262
private const string LicenseNodeName = "license";
6363
private const string IconNodeName = "icon";
6464
private const string AllowedLicenseVersion = "1.0.0";
65-
private const string ReadmeNode = "readme";
65+
private const string ReadmeNodeName = "readme";
6666
private const string Unlicensed = "UNLICENSED";
6767

6868
private readonly IPackageService _packageService;
@@ -152,7 +152,7 @@ public async Task<PackageValidationResult> ValidateMetadataBeforeUploadAsync(
152152
return result;
153153
}
154154

155-
result = await CheckReadmeMetadataAsync(nuGetPackage, warnings, currentUser);
155+
result = await CheckReadmeMetadataAsync(nuGetPackage, currentUser);
156156
if (result != null)
157157
{
158158
return result;
@@ -415,7 +415,7 @@ private async Task<PackageValidationResult> CheckIconMetadataAsync(PackageArchiv
415415
return null;
416416
}
417417

418-
private async Task<PackageValidationResult> CheckReadmeMetadataAsync(PackageArchiveReader nuGetPackage, List<IValidationMessage> warnings, User user)
418+
private async Task<PackageValidationResult> CheckReadmeMetadataAsync(PackageArchiveReader nuGetPackage, User user)
419419
{
420420
var nuspecReader = GetNuspecReader(nuGetPackage);
421421
var readmeElement = nuspecReader.ReadmeElement;
@@ -425,15 +425,15 @@ private async Task<PackageValidationResult> CheckReadmeMetadataAsync(PackageArch
425425
return null;
426426
}
427427

428-
var embeddedReadmeEnabled = _featureFlagService.IsUploadEmbeddedReadmeEnabled(user);
428+
var embeddedReadmeEnabled = _featureFlagService.AreEmbeddedReadmesEnabled(user);
429429
if (!embeddedReadmeEnabled)
430430
{
431431
return PackageValidationResult.Invalid(Strings.UploadPackage_EmbeddedReadmeNotAccepted);
432432
}
433433

434434
if (HasChildElements(readmeElement))
435435
{
436-
return PackageValidationResult.Invalid(string.Format(Strings.UploadPackage_NodeContainsChildren, ReadmeNode));
436+
return PackageValidationResult.Invalid(string.Format(Strings.UploadPackage_NodeContainsChildren, ReadmeNodeName));
437437
}
438438

439439
var readmeFilePath = FileNameHelper.GetZipEntryPath(readmeElement.Value);
@@ -640,7 +640,7 @@ public UserContentEnabledNuspecReader(Stream stream)
640640

641641
public XElement LicenseElement => MetadataNode.Element(MetadataNode.Name.Namespace + LicenseNodeName);
642642
public XElement IconElement => MetadataNode.Element(MetadataNode.Name.Namespace + IconNodeName);
643-
public XElement ReadmeElement => MetadataNode.Element(MetadataNode.Name.Namespace + ReadmeNode);
643+
public XElement ReadmeElement => MetadataNode.Element(MetadataNode.Name.Namespace + ReadmeNodeName);
644644
}
645645

646646
private async Task<PackageValidationResult> CheckPackageEntryCountAsync(

‎src/NuGetGallery/Services/PackageUploadService.cs‎

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2,30 +2,25 @@
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

44
using System;
5-
using System.Collections.Generic;
65
using System.Data.Entity.Infrastructure;
76
using System.Data.SqlClient;
87
using System.IO;
9-
using System.Linq;
10-
using System.Threading;
118
using System.Threading.Tasks;
129
using NuGet.Packaging;
1310
using NuGet.Services.Entities;
14-
using NuGetGallery.Configuration;
1511
using NuGetGallery.Diagnostics;
1612
using NuGetGallery.Packaging;
1713

1814
namespace NuGetGallery
1915
{
2016
public class PackageUploadService : IPackageUploadService
2117
{
18+
2219
private readonly IPackageService _packageService;
2320
private readonly IPackageFileService _packageFileService;
2421
private readonly IEntitiesContext _entitiesContext;
2522
private readonly IReservedNamespaceService _reservedNamespaceService;
2623
private readonly IValidationService _validationService;
27-
private readonly IAppConfiguration _config;
28-
private readonly ITyposquattingService _typosquattingService;
2924
private readonly ICoreLicenseFileService _coreLicenseFileService;
3025
private readonly IPackageVulnerabilityService _vulnerabilityService;
3126
private readonly IPackageMetadataValidationService _metadataValidationService;
@@ -36,8 +31,6 @@ public PackageUploadService(
3631
IEntitiesContext entitiesContext,
3732
IReservedNamespaceService reservedNamespaceService,
3833
IValidationService validationService,
39-
IAppConfiguration config,
40-
ITyposquattingService typosquattingService,
4134
ICoreLicenseFileService coreLicenseFileService,
4235
IDiagnosticsService diagnosticsService,
4336
IPackageVulnerabilityService vulnerabilityService,
@@ -48,8 +41,6 @@ public PackageUploadService(
4841
_entitiesContext = entitiesContext ?? throw new ArgumentNullException(nameof(entitiesContext));
4942
_reservedNamespaceService = reservedNamespaceService ?? throw new ArgumentNullException(nameof(reservedNamespaceService));
5043
_validationService = validationService ?? throw new ArgumentNullException(nameof(validationService));
51-
_config = config ?? throw new ArgumentNullException(nameof(config));
52-
_typosquattingService = typosquattingService ?? throw new ArgumentNullException(nameof(typosquattingService));
5344
_coreLicenseFileService = coreLicenseFileService ?? throw new ArgumentNullException(nameof(coreLicenseFileService));
5445
if (diagnosticsService == null)
5546
{

‎src/NuGetGallery/Strings.resx‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1030,14 +1030,14 @@ The {1} Team</value>
10301030
</data>
10311031
<data name="UploadPackage_FileDoesNotExist" xml:space="preserve">
10321032
<value>The {0} file '{1}' does not exist in the package.</value>
1033-
<comment>{0} is the type of file ('license', 'icon', etc.), {1} is the file name specified in the .nuspec</comment>
1033+
<comment>{0} is the type of file ('license', 'icon', 'readme', etc.), {1} is the file name specified in the .nuspec</comment>
10341034
</data>
10351035
<data name="UploadPackage_LicenseMustBePlainText" xml:space="preserve">
10361036
<value>The license file must be plain text using UTF-8 encoding.</value>
10371037
</data>
10381038
<data name="UploadPackage_FileTooLong" xml:space="preserve">
10391039
<value>The {0} file cannot be larger than {1}.</value>
1040-
<comment>{0} is the file type ('license', 'icon', etc.), {1} is the string representation max allowed license file size (in whatever units the caller chooses)</comment>
1040+
<comment>{0} is the file type ('license', 'icon', 'readme', etc.), {1} is the string representation max allowed license file size (in whatever units the caller chooses)</comment>
10411041
</data>
10421042
<data name="UploadPackage_NodeContainsChildren" xml:space="preserve">
10431043
<value>The {0} node cannot contain child nodes.</value>
@@ -1175,4 +1175,7 @@ The {1} Team</value>
11751175
<data name="UploadPackage_ReadmeMustBePlainText" xml:space="preserve">
11761176
<value>The readme file must be plain text using UTF-8 encoding.</value>
11771177
</data>
1178+
<data name="UploadPackage_InvalidReadmeName" xml:space="preserve">
1179+
<value>the name of &lt;readme&gt; element is case sensitive, must use the &lt;readme&gt;</value>
1180+
</data>
11781181
</root>

‎tests/NuGetGallery.Facts/Services/PackageMetadataValidationServiceFacts.cs‎

Lines changed: 31 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1381,7 +1381,7 @@ public async Task AcceptsPackagesWithEmbeddedReadmeForFlightedUsers()
13811381
licenseExpression: "MIT",
13821382
licenseUrl: new Uri("https://licenses.nuget.org/MIT"));
13831383
_featureFlagService
1384-
.Setup(ffs => ffs.IsUploadEmbeddedReadmeEnabled(_currentUser))
1384+
.Setup(ffs => ffs.AreEmbeddedReadmesEnabled(_currentUser))
13851385
.Returns(true);
13861386

13871387
var result = await _target.ValidateMetadataBeforeUploadAsync(
@@ -1395,16 +1395,21 @@ public async Task AcceptsPackagesWithEmbeddedReadmeForFlightedUsers()
13951395
}
13961396

13971397
[Theory]
1398-
[InlineData("<readme><something/></readme>")]
1399-
[InlineData("<readme><something>readme.md</something></readme>")]
1398+
[InlineData("<readme><someChildNode/></readme>")]
1399+
[InlineData("<readme><someChildNode/> </readme>")]
1400+
[InlineData("<readme><someChildNode>readme.md</someChildNode></readme>")]
1401+
[InlineData("<readme><someChildNode /></readme>")]
1402+
[InlineData("<readme>readme.<someChildNode>md</someChildNode></readme>")]
1403+
[InlineData("<readme><M>M</M><I>I</I><T>T</T></readme>")]
1404+
[InlineData("<readme>M<I>I</I>T</readme>")]
14001405
public async Task RejectsReadmeElementWithChildren(string readmeElement)
14011406
{
14021407
_nuGetPackage = GeneratePackageWithUserContent(
14031408
getCustomNuspecNodes: () => readmeElement,
14041409
licenseExpression: "MIT",
14051410
licenseUrl: new Uri("https://licenses.nuget.org/MIT"));
14061411
_featureFlagService
1407-
.Setup(ffs => ffs.IsUploadEmbeddedReadmeEnabled(_currentUser))
1412+
.Setup(ffs => ffs.AreEmbeddedReadmesEnabled(_currentUser))
14081413
.Returns(true);
14091414

14101415
var result = await _target.ValidateMetadataBeforeUploadAsync(
@@ -1439,7 +1444,7 @@ private async Task<PackageValidationResult> ValidatePackageWithReadme(string rea
14391444
licenseExpression: "MIT",
14401445
licenseUrl: new Uri("https://licenses.nuget.org/MIT"));
14411446
_featureFlagService
1442-
.Setup(ffs => ffs.IsUploadEmbeddedReadmeEnabled(_currentUser))
1447+
.Setup(ffs => ffs.AreEmbeddedReadmesEnabled(_currentUser))
14431448
.Returns(true);
14441449

14451450
return await _target.ValidateMetadataBeforeUploadAsync(
@@ -1479,29 +1484,40 @@ public async Task ChecksReadmeFileExtension(string extension, bool successExpect
14791484
Assert.Empty(result.Warnings);
14801485
}
14811486

1482-
[Fact]
1483-
public async Task RejectsLongReadme()
1487+
[Theory]
1488+
[InlineData(42, true)]
1489+
[InlineData(1024, true)]
1490+
[InlineData(1024 * 1024 - 1, true)]
1491+
[InlineData(1024 * 1024, true)]
1492+
[InlineData(1024 * 1024 + 1, false)]
1493+
public async Task RejectsLongReadme(int fileLength, bool expectedSuccess)
14841494
{
1485-
const int ExpectedMaxReadmeLength = 1024 * 1024;
14861495

1487-
var readmeText = new String('a', ExpectedMaxReadmeLength + 100);
1496+
var readmeText = new String('a', fileLength);
14881497

14891498
_nuGetPackage = GeneratePackageWithUserContent(
14901499
readmeFilename: "readme.md",
14911500
readmeFileContents: readmeText,
14921501
licenseExpression: "MIT",
14931502
licenseUrl: new Uri("https://licenses.nuget.org/MIT"));
14941503
_featureFlagService
1495-
.Setup(ffs => ffs.IsUploadEmbeddedReadmeEnabled(_currentUser))
1504+
.Setup(ffs => ffs.AreEmbeddedReadmesEnabled(_currentUser))
14961505
.Returns(true);
14971506

14981507
var result = await _target.ValidateMetadataBeforeUploadAsync(
14991508
_nuGetPackage.Object,
15001509
GetPackageMetadata(_nuGetPackage),
15011510
_currentUser);
15021511

1503-
Assert.Equal(PackageValidationResultType.Invalid, result.Type);
1504-
Assert.Contains("The readme file cannot be larger", result.Message.PlainTextMessage);
1512+
if (expectedSuccess)
1513+
{
1514+
Assert.Equal(PackageValidationResultType.Accepted, result.Type);
1515+
}
1516+
else
1517+
{
1518+
Assert.Equal(PackageValidationResultType.Invalid, result.Type);
1519+
Assert.Contains("The readme file cannot be larger", result.Message.PlainTextMessage);
1520+
}
15051521
Assert.Empty(result.Warnings);
15061522
}
15071523

@@ -1512,7 +1528,7 @@ public async Task RejectsNupkgsReportingIncorrectReadmeFileLengthforReadmeFile()
15121528
const string readmeFileContents = "readmedocumentation";
15131529

15141530
_featureFlagService
1515-
.Setup(ffs => ffs.IsUploadEmbeddedReadmeEnabled(_currentUser))
1531+
.Setup(ffs => ffs.AreEmbeddedReadmesEnabled(_currentUser))
15161532
.Returns(true);
15171533
// Arrange
15181534
var packageStream = GeneratePackageStream(
@@ -1548,7 +1564,7 @@ public async Task AcceptsReadmeFileInSubdirectories(string readmePath)
15481564
licenseExpression: "MIT",
15491565
licenseUrl: new Uri("https://licenses.nuget.org/MIT"));
15501566
_featureFlagService
1551-
.Setup(ffs => ffs.IsUploadEmbeddedReadmeEnabled(_currentUser))
1567+
.Setup(ffs => ffs.AreEmbeddedReadmesEnabled(_currentUser))
15521568
.Returns(true);
15531569
var result = await _target.ValidateMetadataBeforeUploadAsync(
15541570
_nuGetPackage.Object,
@@ -1570,7 +1586,7 @@ public async Task RejectsBinaryReadmeFiles(byte[] readmeFileContent, bool expect
15701586
licenseExpression: "MIT",
15711587
licenseUrl: new Uri("https://licenses.nuget.org/MIT"));
15721588
_featureFlagService
1573-
.Setup(ffs => ffs.IsUploadEmbeddedReadmeEnabled(_currentUser))
1589+
.Setup(ffs => ffs.AreEmbeddedReadmesEnabled(_currentUser))
15741590
.Returns(true);
15751591

15761592
var result = await _target.ValidateMetadataBeforeUploadAsync(

‎tests/NuGetGallery.Facts/Services/PackageUploadServiceFacts.cs‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,6 @@ private static PackageUploadService CreateService(
3333
Mock<IPackageService> packageService = null,
3434
Mock<IReservedNamespaceService> reservedNamespaceService = null,
3535
Mock<IValidationService> validationService = null,
36-
Mock<IAppConfiguration> config = null,
3736
Mock<IPackageVulnerabilityService> vulnerabilityService = null)
3837
{
3938
packageService = packageService ?? new Mock<IPackageService>();
@@ -70,7 +69,6 @@ private static PackageUploadService CreateService(
7069
}
7170

7271
validationService = validationService ?? new Mock<IValidationService>();
73-
config = config ?? new Mock<IAppConfiguration>();
7472
var diagnosticsService = new Mock<IDiagnosticsService>();
7573
diagnosticsService
7674
.Setup(ds => ds.GetSource(It.IsAny<string>()))
@@ -83,8 +81,6 @@ private static PackageUploadService CreateService(
8381
new Mock<IEntitiesContext>().Object,
8482
reservedNamespaceService.Object,
8583
validationService.Object,
86-
config.Object,
87-
new Mock<ITyposquattingService>().Object,
8884
Mock.Of<ICoreLicenseFileService>(),
8985
diagnosticsService.Object,
9086
vulnerabilityService.Object,
@@ -690,8 +686,6 @@ public FactsBase()
690686
_entitiesContext.Object,
691687
_reservedNamespaceService.Object,
692688
_validationService.Object,
693-
_config.Object,
694-
_typosquattingService.Object,
695689
_licenseFileService.Object,
696690
_diagnosticsService.Object,
697691
_vulnerabilityService.Object,

0 commit comments

Comments
 (0)