Skip to content

Commit ca17034

Browse files
erdembayarerdembayar
authored
Precalculate normalization for perf improvement (#9883)
1 parent e181f1e commit ca17034

6 files changed

Lines changed: 63 additions & 18 deletions

File tree

‎src/NuGetGallery.Core/Services/ITyposquattingServiceHelper.cs‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,5 +15,20 @@ public interface ITyposquattingServiceHelper
1515
/// <param name="packageId">Package Id compared to</param>
1616
/// <returns>Return true if distance is less than the threshold</returns>
1717
bool IsDistanceLessThanOrEqualToThreshold(string uploadedPackageId, string packageId);
18+
19+
/// <summary>
20+
/// This method is used to check if the distance between the currently uploaded package ID and another package ID is less than or equal to the threshold.
21+
/// </summary>
22+
/// <param name="uploadedPackageId">Uploaded package Id</param>
23+
/// <param name="normalizedPackageId">Normalized Package Id compared to</param>
24+
/// <returns>Return true if distance is less than the threshold</returns>
25+
bool IsDistanceLessThanOrEqualToThresholdWithNormalizedPackageId(string uploadedPackageId, string normalizedPackageId);
26+
27+
/// <summary>
28+
/// This method is used to normalize string.
29+
/// </summary>
30+
/// <param name="str">String to normalize</param>
31+
/// <returns>Normalized string</returns>
32+
string NormalizeString(string str);
1833
}
1934
}

‎src/NuGetGallery/Services/NullTyposquattingService.cs‎

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,30 @@
11
// Copyright (c) .NET Foundation. All rights reserved.
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

4+
using System;
5+
46
namespace NuGetGallery.Services
57
{
68
public class ExactMatchTyposquattingServiceHelper : ITyposquattingServiceHelper
79
{
810
public bool IsDistanceLessThanOrEqualToThreshold(string uploadedPackageId, string packageId)
911
{
10-
return uploadedPackageId.ToLowerInvariant() == packageId.ToLowerInvariant();
12+
return StringComparer.OrdinalIgnoreCase.Equals(uploadedPackageId, packageId);
13+
}
14+
15+
public bool IsDistanceLessThanOrEqualToThresholdWithNormalizedPackageId(string uploadedPackageId, string normalizedPackageId)
16+
{
17+
return StringComparer.OrdinalIgnoreCase.Equals(uploadedPackageId, normalizedPackageId);
18+
}
19+
20+
public string NormalizeString(string packageId)
21+
{
22+
if (string.IsNullOrEmpty(packageId))
23+
{
24+
return string.Empty;
25+
}
26+
27+
return packageId.ToLowerInvariant();
1128
}
1229
}
1330
}

‎src/NuGetGallery/Services/TyposquattingCheckListCacheService.cs‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,16 +10,18 @@ namespace NuGetGallery
1010
public class TyposquattingCheckListCacheService : ITyposquattingCheckListCacheService
1111
{
1212
private readonly object Locker = new object();
13+
private readonly ITyposquattingServiceHelper _typosquattingServiceHelper;
1314

1415
private List<string> Cache;
1516
private DateTime LastRefreshTime;
1617

1718
private int TyposquattingCheckListConfiguredLength;
1819

19-
public TyposquattingCheckListCacheService()
20+
public TyposquattingCheckListCacheService(ITyposquattingServiceHelper typosquattingServiceHelper)
2021
{
2122
TyposquattingCheckListConfiguredLength = -1;
2223
LastRefreshTime = DateTime.MinValue;
24+
_typosquattingServiceHelper = typosquattingServiceHelper;
2325
}
2426

2527
public IReadOnlyCollection<string> GetTyposquattingCheckList(int checkListConfiguredLength, TimeSpan checkListExpireTime, IPackageService packageService)
@@ -44,12 +46,14 @@ public IReadOnlyCollection<string> GetTyposquattingCheckList(int checkListConfig
4446
if (ShouldCacheBeUpdated(checkListConfiguredLength, checkListExpireTime))
4547
{
4648
TyposquattingCheckListConfiguredLength = checkListConfiguredLength;
47-
48-
Cache = packageService.GetAllPackageRegistrations()
49+
IQueryable<string> cacheQuery = packageService.GetAllPackageRegistrations()
4950
.OrderByDescending(pr => pr.IsVerified)
5051
.ThenByDescending(pr => pr.DownloadCount)
5152
.Select(pr => pr.Id)
52-
.Take(TyposquattingCheckListConfiguredLength)
53+
.Take(TyposquattingCheckListConfiguredLength);
54+
55+
Cache = cacheQuery
56+
.Select(pr => _typosquattingServiceHelper.NormalizeString(pr))
5357
.ToList();
5458

5559
LastRefreshTime = DateTime.UtcNow;

‎src/NuGetGallery/Services/TyposquattingService.cs‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -60,18 +60,20 @@ public bool IsUploadedPackageIdTyposquatting(string uploadedPackageId, User uplo
6060

6161
var totalTimeStopwatch = Stopwatch.StartNew();
6262
var checklistRetrievalStopwatch = Stopwatch.StartNew();
63-
var packageIdsCheckList = _typosquattingCheckListCacheService.GetTyposquattingCheckList(checkListConfiguredLength, checkListExpireTimeInHours, _packageService);
63+
64+
// It must be normalized during initial list creation
65+
var normalizedPackageIdsCheckList = _typosquattingCheckListCacheService.GetTyposquattingCheckList(checkListConfiguredLength, checkListExpireTimeInHours, _packageService);
6466
checklistRetrievalStopwatch.Stop();
6567

6668
_telemetryService.TrackMetricForTyposquattingChecklistRetrievalTime(uploadedPackageId, checklistRetrievalStopwatch.Elapsed);
6769

6870
var algorithmProcessingStopwatch = Stopwatch.StartNew();
6971
var collisionIds = new ConcurrentBag<string>();
70-
Parallel.ForEach(packageIdsCheckList, (packageId, loopState) =>
72+
Parallel.ForEach(normalizedPackageIdsCheckList, (normalizedPackageId, loopState) =>
7173
{
72-
if (_typosquattingServiceHelper.IsDistanceLessThanOrEqualToThreshold(uploadedPackageId, packageId))
74+
if (_typosquattingServiceHelper.IsDistanceLessThanOrEqualToThresholdWithNormalizedPackageId(uploadedPackageId, normalizedPackageId))
7375
{
74-
collisionIds.Add(packageId);
76+
collisionIds.Add(normalizedPackageId);
7577
}
7678
});
7779
algorithmProcessingStopwatch.Stop();
@@ -86,7 +88,7 @@ public bool IsUploadedPackageIdTyposquatting(string uploadedPackageId, User uplo
8688
totalTimeStopwatch.Elapsed,
8789
wasUploadBlocked,
8890
typosquattingCheckCollisionIds,
89-
packageIdsCheckList.Count,
91+
normalizedPackageIdsCheckList.Count,
9092
checkListExpireTimeInHours);
9193

9294
return false;
@@ -126,7 +128,7 @@ public bool IsUploadedPackageIdTyposquatting(string uploadedPackageId, User uplo
126128
totalTimeStopwatch.Elapsed,
127129
wasUploadBlocked,
128130
typosquattingCheckCollisionIds,
129-
packageIdsCheckList.Count,
131+
normalizedPackageIdsCheckList.Count,
130132
checkListExpireTimeInHours);
131133

132134
return wasUploadBlocked;

‎tests/NuGetGallery.Core.Facts/Frameworks/PackageFrameworkCompatibilityFactoryFacts.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -217,7 +217,7 @@ public void FrameworksInTableShouldBeOnAscendingOrder()
217217
foreach (var row in result.Table)
218218
{
219219
var compatibleFrameworks = row.Value;
220-
var expectedList = compatibleFrameworks.OrderBy(cf => cf.Framework, new NuGetFrameworkSorter());
220+
var expectedList = compatibleFrameworks.OrderBy(cf => cf.Framework, NuGetFrameworkSorter.Instance);
221221

222222
Assert.True(expectedList.SequenceEqual(row.Value));
223223
}

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

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,9 @@ public void WhenGettingCheckListFromMultipleThreadsItIsInitializedOnce()
4343
.Setup(x => x.GetAllPackageRegistrations())
4444
.Returns(PacakgeRegistrationsList);
4545

46-
var newService = new TyposquattingCheckListCacheService();
46+
var mockTyposquattingServiceHelper = new Mock<ITyposquattingServiceHelper>();
47+
48+
var newService = new TyposquattingCheckListCacheService(mockTyposquattingServiceHelper.Object);
4749

4850
// Act
4951
int tasksNum = 3;
@@ -73,7 +75,8 @@ public void WhenExceedExpireTimeRefreshCheckListCache()
7375
.Setup(x => x.GetAllPackageRegistrations())
7476
.Returns(PacakgeRegistrationsList);
7577

76-
var newService = new TyposquattingCheckListCacheService();
78+
var mockTyposquattingServiceHelper = new Mock<ITyposquattingServiceHelper>();
79+
var newService = new TyposquattingCheckListCacheService(mockTyposquattingServiceHelper.Object);
7780

7881
// Act
7982
newService.GetTyposquattingCheckList(_packageIds.Count, TimeSpan.FromHours(0), mockPackageService.Object);
@@ -95,7 +98,8 @@ public void WhenNotEqualConfiguredLengthRefreshCheckListCache()
9598
.Setup(x => x.GetAllPackageRegistrations())
9699
.Returns(PacakgeRegistrationsList);
97100

98-
var newService = new TyposquattingCheckListCacheService();
101+
var mockTyposquattingServiceHelper = new Mock<ITyposquattingServiceHelper>();
102+
var newService = new TyposquattingCheckListCacheService(mockTyposquattingServiceHelper.Object);
99103

100104
// Act
101105
newService.GetTyposquattingCheckList(_packageIds.Count, TimeSpan.FromHours(24), mockPackageService.Object);
@@ -117,7 +121,8 @@ public void CheckConfiguredLengthNotAllowed()
117121
.Returns(PacakgeRegistrationsList);
118122
var checkListConfiguredLength = -1;
119123

120-
var newService = new TyposquattingCheckListCacheService();
124+
var mockTyposquattingServiceHelper = new Mock<ITyposquattingServiceHelper>();
125+
var newService = new TyposquattingCheckListCacheService(mockTyposquattingServiceHelper.Object);
121126

122127
// Act
123128
var exception = Assert.Throws<ArgumentOutOfRangeException>(
@@ -137,7 +142,8 @@ public void CheckExpireTimeNotAllowed()
137142
.Setup(x => x.GetAllPackageRegistrations())
138143
.Returns(PacakgeRegistrationsList);
139144

140-
var newService = new TyposquattingCheckListCacheService();
145+
var mockTyposquattingServiceHelper = new Mock<ITyposquattingServiceHelper>();
146+
var newService = new TyposquattingCheckListCacheService(mockTyposquattingServiceHelper.Object);
141147

142148
// Act
143149
var exception = Assert.Throws<ArgumentOutOfRangeException>(
@@ -151,7 +157,8 @@ public void CheckExpireTimeNotAllowed()
151157
public void CheckNullPackageService()
152158
{
153159
// Arrange
154-
var newService = new TyposquattingCheckListCacheService();
160+
var mockTyposquattingServiceHelper = new Mock<ITyposquattingServiceHelper>();
161+
var newService = new TyposquattingCheckListCacheService(mockTyposquattingServiceHelper.Object);
155162

156163
// Act
157164
var exception = Assert.Throws<ArgumentNullException>(

0 commit comments

Comments
 (0)