Skip to content
This repository was archived by the owner on Oct 16, 2020. It is now read-only.

Commit 5da6791

Browse files
committed
Fix race condition when searching for NuGet packages.
1 parent 72a5c5f commit 5da6791

8 files changed

Lines changed: 96 additions & 32 deletions

File tree

src/AddIns/Misc/PackageManagement/Project/PackageManagement.csproj

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -232,6 +232,7 @@
232232
<Compile Include="Src\PackageManagementServiceProvider.cs" />
233233
<Compile Include="Src\IPackageRepositoryExtensions.cs" />
234234
<Compile Include="Src\PackageRepositoryFactoryEventArgs.cs" />
235+
<Compile Include="Src\PackagesForSelectedPageQuery.cs" />
235236
<Compile Include="Src\ParentPackagesOperationEventArgs.cs" />
236237
<Compile Include="Src\ProjectBuilder.cs" />
237238
<Compile Include="Src\ProjectRootElementExtensions.cs" />

src/AddIns/Misc/PackageManagement/Project/Src/AvailablePackagesViewModel.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ protected override IQueryable<IPackage> GetAllPackages(string searchCriteria)
8484
.Where(package => package.IsAbsoluteLatestVersion);
8585
}
8686
return repository
87-
.Search(SearchTerms, IncludePrerelease)
87+
.Search(searchCriteria, IncludePrerelease)
8888
.Where(package => package.IsLatestVersion);
8989
}
9090

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
// Copyright (c) AlphaSierraPapa for the SharpDevelop Team (for details please see \doc\copyright.txt)
2+
// This code is distributed under the GNU LGPL (for details please see \doc\license.txt)
3+
4+
using System;
5+
using System.Collections.Generic;
6+
7+
using NuGet;
8+
9+
namespace ICSharpCode.PackageManagement
10+
{
11+
public class PackagesForSelectedPageQuery
12+
{
13+
public PackagesForSelectedPageQuery (
14+
PackagesViewModel viewModel,
15+
IEnumerable<IPackage> allPackages,
16+
string searchCriteria)
17+
{
18+
Skip = viewModel.ItemsBeforeFirstPage;
19+
Take = viewModel.PageSize;
20+
AllPackages = allPackages;
21+
SearchCriteria = searchCriteria;
22+
TotalPackages = viewModel.TotalItems;
23+
}
24+
25+
public int Skip { get; private set; }
26+
public int Take { get; private set; }
27+
public string SearchCriteria { get; private set; }
28+
29+
public int TotalPackages { get; set; }
30+
public IEnumerable<IPackage> AllPackages { get; set; }
31+
}
32+
}

src/AddIns/Misc/PackageManagement/Project/Src/PackagesForSelectedPageResult.cs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,15 +26,19 @@ namespace ICSharpCode.PackageManagement
2626
{
2727
public class PackagesForSelectedPageResult
2828
{
29-
public PackagesForSelectedPageResult(IEnumerable<IPackage> packages, int totalPackages)
29+
public PackagesForSelectedPageResult(IEnumerable<IPackage> packages, PackagesForSelectedPageQuery query)
3030
{
3131
this.Packages = packages;
3232
this.TotalPackagesOnPage = packages.Count();
33-
this.TotalPackages = totalPackages;
33+
this.TotalPackages = query.TotalPackages;
34+
this.AllPackages = query.AllPackages;
35+
this.Query = query;
3436
}
3537

38+
public PackagesForSelectedPageQuery Query { get; set; }
3639
public IEnumerable<IPackage> Packages { get; set; }
3740
public int TotalPackagesOnPage { get; set; }
3841
public int TotalPackages { get; set; }
42+
public IEnumerable<IPackage> AllPackages { get; set; }
3943
}
4044
}

src/AddIns/Misc/PackageManagement/Project/Src/PackagesViewModel.cs

Lines changed: 39 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ public abstract class PackagesViewModel : ViewModelBase<PackagesViewModel>, IDis
4141
IEnumerable<IPackage> allPackages;
4242
ITask<PackagesForSelectedPageResult> task;
4343
bool includePrerelease;
44+
PackagesForSelectedPageQuery packagesForSelectedPageQuery;
4445

4546
public PackagesViewModel(
4647
IPackageManagementSolution solution,
@@ -153,15 +154,18 @@ void CancelReadPackagesTask()
153154

154155
void CreateReadPackagesTask()
155156
{
157+
var query = new PackagesForSelectedPageQuery(this, allPackages, GetSearchCriteria());
158+
packagesForSelectedPageQuery = query;
159+
156160
task = taskFactory.CreateTask(
157-
() => GetPackagesForSelectedPageResult(),
161+
() => GetPackagesForSelectedPageResult(query),
158162
OnPackagesReadForSelectedPage);
159163
}
160164

161-
PackagesForSelectedPageResult GetPackagesForSelectedPageResult()
165+
PackagesForSelectedPageResult GetPackagesForSelectedPageResult(PackagesForSelectedPageQuery query)
162166
{
163-
IEnumerable<IPackage> packages = GetPackagesForSelectedPage();
164-
return new PackagesForSelectedPageResult(packages, TotalItems);
167+
IEnumerable<IPackage> packages = GetPackagesForSelectedPage(query);
168+
return new PackagesForSelectedPageResult(packages, query);
165169
}
166170

167171
void OnPackagesReadForSelectedPage(ITask<PackagesForSelectedPageResult> task)
@@ -171,12 +175,19 @@ void OnPackagesReadForSelectedPage(ITask<PackagesForSelectedPageResult> task)
171175
SaveError(task.Exception);
172176
} else if (task.IsCancelled) {
173177
// Ignore
178+
} else if (!IsCurrentQuery(task.Result)) {
179+
// Ignore.
174180
} else {
175181
UpdatePackagesForSelectedPage(task.Result);
176182
}
177183
base.OnPropertyChanged(null);
178184
}
179185

186+
bool IsCurrentQuery(PackagesForSelectedPageResult result)
187+
{
188+
return packagesForSelectedPageQuery == result.Query;
189+
}
190+
180191
void SaveError(AggregateException ex)
181192
{
182193
HasError = true;
@@ -194,6 +205,8 @@ void UpdatePackagesForSelectedPage(PackagesForSelectedPageResult result)
194205
{
195206
pages.TotalItems = result.TotalPackages;
196207
pages.TotalItemsOnSelectedPage = result.TotalPackagesOnPage;
208+
TotalItems = result.TotalPackages;
209+
allPackages = result.AllPackages;
197210
UpdatePackageViewModels(result.Packages);
198211
}
199212

@@ -203,28 +216,33 @@ void PagesChanged(object sender, NotifyCollectionChangedEventArgs e)
203216
base.OnPropertyChanged(null);
204217
}
205218

206-
IEnumerable<IPackage> GetPackagesForSelectedPage()
219+
IEnumerable<IPackage> GetPackagesForSelectedPage(PackagesForSelectedPageQuery query)
207220
{
208-
IEnumerable<IPackage> filteredPackages = GetFilteredPackagesBeforePagingResults();
209-
return GetPackagesForSelectedPage(filteredPackages);
221+
IEnumerable<IPackage> filteredPackages = GetFilteredPackagesBeforePagingResults(query);
222+
return GetPackagesForSelectedPage(filteredPackages, query);
210223
}
211224

212-
IEnumerable<IPackage> GetFilteredPackagesBeforePagingResults()
225+
IEnumerable<IPackage> GetFilteredPackagesBeforePagingResults(PackagesForSelectedPageQuery query)
213226
{
214-
if (allPackages == null) {
215-
IQueryable<IPackage> packages = GetPackagesFromPackageSource();
216-
TotalItems = packages.Count();
217-
allPackages = GetFilteredPackagesBeforePagingResults(packages);
227+
if (query.AllPackages == null) {
228+
IQueryable<IPackage> packages = GetPackagesFromPackageSource(query.SearchCriteria);
229+
query.TotalPackages = packages.Count();
230+
query.AllPackages = GetFilteredPackagesBeforePagingResults(packages);
218231
}
219-
return allPackages;
232+
return query.AllPackages;
220233
}
221234

222235
/// <summary>
223236
/// Returns the queryable object that will be used to query the NuGet online feed.
224237
/// </summary>
225238
public IQueryable<IPackage> GetPackagesFromPackageSource()
226239
{
227-
IQueryable<IPackage> packages = GetAllPackages(GetSearchCriteria());
240+
return GetPackagesFromPackageSource(GetSearchCriteria());
241+
}
242+
243+
IQueryable<IPackage> GetPackagesFromPackageSource(string searchCriteria)
244+
{
245+
IQueryable<IPackage> packages = GetAllPackages(searchCriteria);
228246
return OrderPackages(packages);
229247
}
230248

@@ -242,12 +260,11 @@ string GetSearchCriteria()
242260
return SearchTerms;
243261
}
244262

245-
IEnumerable<IPackage> GetPackagesForSelectedPage(IEnumerable<IPackage> allPackages)
263+
IEnumerable<IPackage> GetPackagesForSelectedPage(IEnumerable<IPackage> allPackages, PackagesForSelectedPageQuery query)
246264
{
247-
int packagesToSkip = pages.ItemsBeforeFirstPage;
248265
return allPackages
249-
.Skip(packagesToSkip)
250-
.Take(pages.PageSize);
266+
.Skip(query.Skip)
267+
.Take(query.Take);
251268
}
252269

253270
/// <summary>
@@ -325,6 +342,10 @@ public int PageSize {
325342
set { pages.PageSize = value; }
326343
}
327344

345+
public int ItemsBeforeFirstPage {
346+
get { return pages.ItemsBeforeFirstPage; }
347+
}
348+
328349
public bool IsPaged {
329350
get { return pages.IsPaged; }
330351
}

src/AddIns/Misc/PackageManagement/Test/Src/Helpers/FakeTaskFactory.cs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,12 @@ public void ExecuteAllFakeTasks()
5151
}
5252
}
5353

54+
public void ExecuteTask(int index)
55+
{
56+
var task = FakeTasksCreated[index] as FakeTask<PackagesForSelectedPageResult>;
57+
task.ExecuteTaskCompletely();
58+
}
59+
5460
public void ClearAllFakeTasks()
5561
{
5662
FakeTasksCreated.Clear();

src/AddIns/Misc/PackageManagement/Test/Src/Helpers/TestablePackagesViewModel.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,10 +64,11 @@ public void AddOneFakePackage()
6464
AddFakePackage("Test");
6565
}
6666

67-
public void AddFakePackage(string packageId)
67+
public FakePackage AddFakePackage(string packageId)
6868
{
6969
FakePackage package = CreateFakePackage(packageId);
7070
FakePackages.Add(package);
71+
return package;
7172
}
7273

7374
FakePackage CreateFakePackage(string packageId)

src/AddIns/Misc/PackageManagement/Test/Src/PackagesViewModelTests.cs

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -333,22 +333,20 @@ public void Pages_TenPackagesSelectedPageNumberIsFiveAndPageSizeIsTwoAndMaximumS
333333
}
334334

335335
[Test]
336-
public void ReadPackages_RepositoryHasThreePackagesWhenSelectedPageIsOneAndPageSizeIsTwo_TwoPackageViewModelsCreatedForFirstTwoPackages()
336+
public void ReadPackages_SecondQueryFinishesBeforeFirst_PackagesInViewModelAreForSecondQuery()
337337
{
338338
CreateViewModel();
339-
viewModel.PageSize = 2;
340-
viewModel.SelectedPageNumber = 1;
341339
viewModel.AddThreeFakePackages();
340+
FakePackage package = viewModel.AddFakePackage("MyTest");
342341
viewModel.ReadPackages();
343-
CompleteReadPackagesTask();
342+
viewModel.SearchTerms = "MyTest";
344343

345-
var expectedPackages = new List<FakePackage>();
346-
expectedPackages.Add(viewModel.FakePackages[0]);
347-
expectedPackages.Add(viewModel.FakePackages[1]);
344+
var expectedPackages = new FakePackage [] { package };
348345

349-
ClearReadPackagesTasks();
350346
viewModel.ReadPackages();
351-
CompleteReadPackagesTask();
347+
taskFactory.ExecuteTask(1);
348+
taskFactory.ExecuteTask(0);
349+
ClearReadPackagesTasks();
352350

353351
PackageCollectionAssert.AreEqual(expectedPackages, viewModel.PackageViewModels);
354352
}
@@ -1072,7 +1070,8 @@ public void ReadPackages_BackgroundTaskHasExceptionWhenItFinishes_PackagesNotUpd
10721070
CreateViewModel();
10731071
viewModel.AddSixFakePackages();
10741072
viewModel.ReadPackages();
1075-
taskFactory.FirstFakeTaskCreated.Result = new PackagesForSelectedPageResult(viewModel.FakePackages, 6);
1073+
var query = new PackagesForSelectedPageQuery(viewModel, null, null);
1074+
taskFactory.FirstFakeTaskCreated.Result = new PackagesForSelectedPageResult(viewModel.FakePackages, query);
10761075
taskFactory.FirstFakeTaskCreated.IsFaulted = true;
10771076
CompleteReadPackagesTask();
10781077

0 commit comments

Comments
 (0)