Skip to content

Commit 13b3d34

Browse files
joelverhagenjoelverhagen
committed
Add a feature flag for the atom feed and enable prerelease filtering (#6894)
Progress on #2992
1 parent 9eede69 commit 13b3d34

7 files changed

Lines changed: 176 additions & 21 deletions

File tree

‎src/NuGetGallery/App_Data/Files/Content/flags.json‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,14 @@
11
{
22
"Features": {
3-
"NuGetGallery.Typosquatting": "Enabled"
3+
"NuGetGallery.Typosquatting": "Enabled",
4+
"NuGetGallery.PackagesAtomFeed": "Enabled"
45
},
5-
66
"Flights": {
77
"NuGetGallery.TyposquattingFlight": {
8-
"All": true
8+
"All": true,
9+
"SiteAdmins": false,
10+
"Accounts": [],
11+
"Domains": []
912
}
1013
}
1114
}

‎src/NuGetGallery/Controllers/PackagesController.cs‎

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,7 @@ public partial class PackagesController
109109
private readonly IDiagnosticsSource _trace;
110110
private readonly ICoreLicenseFileService _coreLicenseFileService;
111111
private readonly ILicenseExpressionSplitter _licenseExpressionSplitter;
112+
private readonly IFeatureFlagService _featureFlagService;
112113

113114
public PackagesController(
114115
IPackageService packageService,
@@ -135,7 +136,8 @@ public PackagesController(
135136
ISymbolPackageUploadService symbolPackageUploadService,
136137
IDiagnosticsService diagnosticsService,
137138
ICoreLicenseFileService coreLicenseFileService,
138-
ILicenseExpressionSplitter licenseExpressionSplitter)
139+
ILicenseExpressionSplitter licenseExpressionSplitter,
140+
IFeatureFlagService featureFlagService)
139141
{
140142
_packageService = packageService;
141143
_uploadFileService = uploadFileService;
@@ -162,6 +164,7 @@ public PackagesController(
162164
_trace = diagnosticsService?.SafeGetSource(nameof(PackagesController)) ?? throw new ArgumentNullException(nameof(diagnosticsService));
163165
_coreLicenseFileService = coreLicenseFileService ?? throw new ArgumentNullException(nameof(coreLicenseFileService));
164166
_licenseExpressionSplitter = licenseExpressionSplitter ?? throw new ArgumentNullException(nameof(licenseExpressionSplitter));
167+
_featureFlagService = featureFlagService ?? throw new ArgumentNullException(nameof(featureFlagService));
165168
}
166169

167170
[HttpGet]
@@ -673,6 +676,7 @@ public virtual async Task<ActionResult> DisplayPackage(string id, string version
673676
model.PackageValidationIssues = _validationService.GetLatestPackageValidationIssues(package);
674677
model.SymbolsPackageValidationIssues = _validationService.GetLatestPackageValidationIssues(model.LatestSymbolsPackage);
675678
model.IsCertificatesUIEnabled = _contentObjectService.CertificatesConfiguration?.IsUIEnabledForUser(currentUser) ?? false;
679+
model.IsAtomFeedEnabled = _featureFlagService.IsPackagesAtomFeedEnabled();
676680

677681
model.ReadMeHtml = await _readMeService.GetReadMeHtmlAsync(package);
678682

@@ -737,18 +741,31 @@ public virtual async Task<ActionResult> DisplayPackage(string id, string version
737741
}
738742

739743
[HttpGet]
740-
public virtual ActionResult AtomFeed(string id, bool allowPrerelease = false)
744+
public virtual ActionResult AtomFeed(string id, bool prerel = true)
741745
{
746+
if (!_featureFlagService.IsPackagesAtomFeedEnabled())
747+
{
748+
return HttpNotFound();
749+
}
750+
742751
var packageRegistration = _packageService.FindPackageRegistrationById(id);
743752
if (packageRegistration == null)
744753
{
745754
return HttpNotFound();
746755
}
747756

748-
var packageVersions = packageRegistration.Packages
749-
.Where(x => x.Listed && x.PackageStatusKey == PackageStatus.Available)
750-
.OrderByDescending(p => NuGetVersion.Parse(p.NormalizedVersion))
751-
.ToList();
757+
IEnumerable<Package> packageVersionsQuery = packageRegistration
758+
.Packages
759+
.Where(x => x.Listed && x.PackageStatusKey == PackageStatus.Available)
760+
.OrderByDescending(p => NuGetVersion.Parse(p.NormalizedVersion));
761+
762+
if (!prerel)
763+
{
764+
packageVersionsQuery = packageVersionsQuery
765+
.Where(x => !x.IsPrerelease);
766+
}
767+
768+
var packageVersions = packageVersionsQuery.ToList();
752769

753770
if (packageVersions.Count == 0)
754771
{

‎src/NuGetGallery/Services/FeatureFlagService.cs‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@ public class FeatureFlagService : IFeatureFlagService
1616
private const string TyposquattingFeatureName = GalleryPrefix + "Typosquatting";
1717
private const string TyposquattingFlightName = GalleryPrefix + "TyposquattingFlight";
1818

19+
private const string PackagesAtomFeedFeatureName = GalleryPrefix + "PackagesAtomFeed";
20+
1921
private readonly IFeatureFlagClient _client;
2022

2123
public FeatureFlagService(IFeatureFlagClient client)
@@ -33,6 +35,11 @@ public bool IsTyposquattingEnabled(User user)
3335
return _client.IsEnabled(TyposquattingFlightName, user, defaultValue: false);
3436
}
3537

38+
public bool IsPackagesAtomFeedEnabled()
39+
{
40+
return _client.IsEnabled(PackagesAtomFeedFeatureName, defaultValue: false);
41+
}
42+
3643
private bool IsEnabled(string flight, User user, bool defaultValue)
3744
{
3845
return _client.IsEnabled(flight, user, defaultValue);

‎src/NuGetGallery/Services/IFeatureFlagService.cs‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,5 +21,12 @@ public interface IFeatureFlagService
2121
/// <param name="user"></param>
2222
/// <returns></returns>
2323
bool IsTyposquattingEnabled(User user);
24+
25+
/// <summary>
26+
/// Whether the packages Atom feed is enabled. If true, users can subscribe to new package versions using a
27+
/// feed reader on a per package ID basis.
28+
/// </summary>
29+
/// <returns></returns>
30+
bool IsPackagesAtomFeedEnabled();
2431
}
2532
}

‎src/NuGetGallery/ViewModels/DisplayPackageViewModel.cs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,7 @@ private DisplayPackageViewModel(Package package, User currentUser, string pushed
9999
public bool HasSemVer2Version { get; }
100100
public bool HasSemVer2Dependency { get; }
101101
public bool IsDotnetToolPackageType { get; set; }
102+
public bool IsAtomFeedEnabled { get; set; }
102103

103104
public bool HasNewerPrerelease
104105
{

‎src/NuGetGallery/Views/Packages/DisplayPackage.cshtml‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,10 @@
8686
<meta property="og:determiner" content="a" />
8787
<meta property="og:image" content="@(PackageHelper.ShouldRenderUrl(Model.IconUrl) ? Model.IconUrl : Url.Absolute("~/Content/gallery/img/default-package-icon-256x256.png"))" />
8888

89-
<link rel="alternate" type="application/atom+xml" title="Subscribe to @Model.Id updates" href="@Url.PackageAtomFeed(Model.Id)" />
89+
@if (Model.IsAtomFeedEnabled)
90+
{
91+
<link rel="alternate" type="application/atom+xml" title="Subscribe to @Model.Id updates" href="@Url.PackageAtomFeed(Model.Id)" />
92+
}
9093
}
9194

9295
@helper VersionListDivider(int rowCount, bool versionsExpanded)
@@ -862,11 +865,14 @@ foreach (var owner in Model.Owners)
862865
src="@Url.Absolute("~/Content/gallery/img/twitter.svg")"
863866
@ViewHelpers.ImageFallback(Url.Absolute("~/Content/gallery/img/twitter-24x24.png")) />
864867
</a>
865-
<a href="@Url.PackageAtomFeed(Model.Id)" data-track="atom-feed">
866-
<img width="24" height="24" alt="Use the Atom feed to subscribe to new versions of @Model.Id"
867-
src="@Url.Absolute("~/Content/gallery/img/rss.svg")"
868-
@ViewHelpers.ImageFallback(Url.Absolute("~/Content/gallery/img/rss-24x24.png")) />
869-
</a>
868+
@if (Model.IsAtomFeedEnabled)
869+
{
870+
<a href="@Url.PackageAtomFeed(Model.Id)" data-track="atom-feed">
871+
<img width="24" height="24" alt="Use the Atom feed to subscribe to new versions of @Model.Id"
872+
src="@Url.Absolute("~/Content/gallery/img/rss.svg")"
873+
@ViewHelpers.ImageFallback(Url.Absolute("~/Content/gallery/img/rss-24x24.png")) />
874+
</a>
875+
}
870876
</p>
871877
}
872878
</aside>

‎tests/NuGetGallery.Facts/Controllers/PackagesControllerFacts.cs‎

Lines changed: 120 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,8 @@ private static PackagesController CreateController(
6969
Mock<IContentObjectService> contentObjectService = null,
7070
Mock<ISymbolPackageUploadService> symbolPackageUploadService = null,
7171
Mock<ICoreLicenseFileService> coreLicenseFileService = null,
72-
Mock<ILicenseExpressionSplitter> licenseExpressionSplitter = null)
72+
Mock<ILicenseExpressionSplitter> licenseExpressionSplitter = null,
73+
Mock<IFeatureFlagService> featureFlagService = null)
7374
{
7475
packageService = packageService ?? new Mock<IPackageService>();
7576
if (uploadFileService == null)
@@ -181,6 +182,12 @@ private static PackagesController CreateController(
181182

182183
licenseExpressionSplitter = licenseExpressionSplitter ?? new Mock<ILicenseExpressionSplitter>();
183184

185+
if (featureFlagService == null)
186+
{
187+
featureFlagService = new Mock<IFeatureFlagService>();
188+
featureFlagService.SetReturnsDefault<bool>(true);
189+
}
190+
184191
var diagnosticsService = new Mock<IDiagnosticsService>();
185192
var controller = new Mock<PackagesController>(
186193
packageService.Object,
@@ -207,7 +214,8 @@ private static PackagesController CreateController(
207214
symbolPackageUploadService.Object,
208215
diagnosticsService.Object,
209216
coreLicenseFileService.Object,
210-
licenseExpressionSplitter.Object);
217+
licenseExpressionSplitter.Object,
218+
featureFlagService.Object);
211219

212220
controller.CallBase = true;
213221
controller.Object.SetOwinContextOverride(Fakes.CreateOwinContext());
@@ -966,6 +974,111 @@ public void GivenAExistentPackageWithUnlistedAvailablePackagesIt404s()
966974
ResultAssert.IsNotFound(result);
967975
}
968976

977+
[Theory]
978+
[InlineData(false)]
979+
[InlineData(true)]
980+
public void GivenAnExistentPackageTheFeatureFlagHidesTheFeed(bool enabled)
981+
{
982+
// Arrange
983+
var httpContext = new Mock<HttpContextBase>();
984+
var packageService = new Mock<IPackageService>();
985+
var configurationService = GetConfigurationService();
986+
configurationService.Current.Brand = "Test Gallery";
987+
var featureFlagService = new Mock<IFeatureFlagService>();
988+
featureFlagService
989+
.Setup(x => x.IsPackagesAtomFeedEnabled())
990+
.Returns(enabled);
991+
992+
var controller = CreateController(
993+
configurationService,
994+
packageService: packageService,
995+
featureFlagService: featureFlagService,
996+
httpContext: httpContext);
997+
998+
var packageRegistration = new PackageRegistration();
999+
packageRegistration.Id = "Foo";
1000+
1001+
var onlyVersion = new Package
1002+
{
1003+
Listed = true,
1004+
PackageStatusKey = PackageStatus.Available,
1005+
NormalizedVersion = "2.0.0",
1006+
Version = "2.0.0",
1007+
IsPrerelease = true,
1008+
PackageRegistration = new PackageRegistration()
1009+
{
1010+
Id = "Foo"
1011+
}
1012+
};
1013+
packageRegistration.Packages.Add(onlyVersion);
1014+
1015+
packageService.Setup(p => p.FindPackageRegistrationById("Foo"))
1016+
.Returns(packageRegistration);
1017+
1018+
// Act
1019+
var result = controller.AtomFeed("Foo");
1020+
1021+
// Assert
1022+
if (enabled)
1023+
{
1024+
Assert.IsType<SyndicationAtomActionResult>(result);
1025+
}
1026+
else
1027+
{
1028+
ResultAssert.IsNotFound(result);
1029+
}
1030+
}
1031+
1032+
[Theory]
1033+
[InlineData(false)]
1034+
[InlineData(true)]
1035+
public void GivenAExistentPackagePrereleaseVersionsCanBeFilteredOut(bool includePrerelease)
1036+
{
1037+
// Arrange
1038+
var httpContext = new Mock<HttpContextBase>();
1039+
var packageService = new Mock<IPackageService>();
1040+
var configurationService = GetConfigurationService();
1041+
configurationService.Current.Brand = "Test Gallery";
1042+
1043+
var controller = CreateController(
1044+
configurationService,
1045+
packageService: packageService,
1046+
httpContext: httpContext);
1047+
1048+
var packageRegistration = new PackageRegistration();
1049+
packageRegistration.Id = "Foo";
1050+
1051+
var onlyVersion = new Package
1052+
{
1053+
Listed = true,
1054+
PackageStatusKey = PackageStatus.Available,
1055+
NormalizedVersion = "2.0.0-beta",
1056+
Version = "2.0.0-beta",
1057+
IsPrerelease = true,
1058+
PackageRegistration = new PackageRegistration()
1059+
{
1060+
Id = "Foo"
1061+
}
1062+
};
1063+
packageRegistration.Packages.Add(onlyVersion);
1064+
1065+
packageService.Setup(p => p.FindPackageRegistrationById("Foo"))
1066+
.Returns(packageRegistration);
1067+
1068+
// Act
1069+
var result = controller.AtomFeed("Foo", includePrerelease);
1070+
1071+
// Assert
1072+
if (includePrerelease)
1073+
{
1074+
Assert.IsType<SyndicationAtomActionResult>(result);
1075+
}
1076+
else
1077+
{
1078+
ResultAssert.IsNotFound(result);
1079+
}
1080+
}
1081+
9691082
[Fact]
9701083
public void GivenAExistentPackageWithListedUnavailablePackagesIt404s()
9711084
{
@@ -1034,8 +1147,9 @@ public void GivenAExistentPackageWithListedAvailablePackagesItReturnsSyndication
10341147
{
10351148
Listed = true,
10361149
PackageStatusKey = PackageStatus.Available,
1037-
NormalizedVersion = "2.0.0",
1038-
Version = "2.0.0",
1150+
NormalizedVersion = "2.0.0-beta",
1151+
Version = "2.0.0-beta",
1152+
IsPrerelease = true,
10391153
Description = "Most recent version: Test Package",
10401154
Created = dateTimeYesterDay,
10411155
PackageRegistration = new PackageRegistration()
@@ -1078,8 +1192,8 @@ public void GivenAExistentPackageWithListedAvailablePackagesItReturnsSyndication
10781192

10791193
Assert.Equal(3, syndicationFeedItems.Count);
10801194

1081-
Assert.Equal("https://localhost/packages/Foo/2.0.0", syndicationFeedItems[0].Id);
1082-
Assert.Equal("Foo 2.0.0", syndicationFeedItems[0].Title.Text);
1195+
Assert.Equal("https://localhost/packages/Foo/2.0.0-beta", syndicationFeedItems[0].Id);
1196+
Assert.Equal("Foo 2.0.0-beta", syndicationFeedItems[0].Title.Text);
10831197
Assert.Equal("Most recent version: Test Package", (syndicationFeedItems[0].Content as System.ServiceModel.Syndication.TextSyndicationContent).Text);
10841198
Assert.Equal(dateTimeYesterDay, syndicationFeedItems[0].PublishDate);
10851199
Assert.Equal(dateTimeYesterDay, syndicationFeedItems[0].LastUpdatedTime);

0 commit comments

Comments
 (0)