Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor to encapsulate Recommender logic in the RecommenderPackageFeed #6082

Merged
merged 6 commits into from
Oct 18, 2024
Merged

Refactor to encapsulate Recommender logic in the RecommenderPackageFeed #6082

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
028e710
WIP: Refactoring to encapsulate Recommender logic
jebriede Aug 12, 2024
2fd2ad5
Refactor the Recommender feed to decorate another feed and add the re…
jebriede Oct 4, 2024
857f311
Added tests and uncommented out code
jebriede Oct 17, 2024
422493e
Undo global.json change
jebriede Oct 17, 2024
b3a53f9
Fix whitespace error
jebriede Oct 17, 2024
057ba14
Make equality comparer static instance
jebriede Oct 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ...ts/NuGet.PackageManagement.VisualStudio/PackageFeeds/InstalledAndTransitivePackageFeed.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ private static IPackageSearchMetadata GetMetadataFromIdentityForPackage<T>(T ide
return PackageSearchMetadataBuilder.FromIdentity(identity).Build();
}

internal override async Task<IPackageSearchMetadata> GetPackageMetadataAsync<T>(T identity, bool includePrerelease, CancellationToken cancellationToken)
internal override async Task<IPackageSearchMetadata> GetPackageMetadataAsync(PackageIdentity identity, bool includePrerelease, CancellationToken cancellationToken)
{
cancellationToken.ThrowIfCancellationRequested();

Expand Down
4 changes: 2 additions & 2 deletions src/NuGet.Clients/NuGet.PackageManagement.VisualStudio/PackageFeeds/InstalledPackageFeed.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ public override async Task<SearchResult<IPackageSearchMetadata>> ContinueSearchA
return CreateResult(searchItems);
}

internal async Task<IPackageSearchMetadata[]> GetMetadataForPackagesAsync<T>(T[] packages, bool includePrerelease, CancellationToken cancellationToken) where T : PackageIdentity
internal async Task<IPackageSearchMetadata[]> GetMetadataForPackagesAsync(PackageIdentity[] packages, bool includePrerelease, CancellationToken cancellationToken)
{
cancellationToken.ThrowIfCancellationRequested();

Expand Down Expand Up @@ -88,7 +88,7 @@ internal static SearchResult<IPackageSearchMetadata> CreateResult(IPackageSearch
return result;
}

internal virtual async Task<IPackageSearchMetadata> GetPackageMetadataAsync<T>(T identity, bool includePrerelease, CancellationToken cancellationToken) where T : PackageIdentity
internal virtual async Task<IPackageSearchMetadata> GetPackageMetadataAsync(PackageIdentity identity, bool includePrerelease, CancellationToken cancellationToken)
{
cancellationToken.ThrowIfCancellationRequested();

Expand Down
43 changes: 35 additions & 8 deletions ...NuGet.Clients/NuGet.PackageManagement.VisualStudio/PackageFeeds/RecommenderPackageFeed.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,13 @@ namespace NuGet.PackageManagement.VisualStudio
{
/// <summary>
/// Represents a package feed which recommends packages based on currently loaded project info
/// Decorates any other feed and adds recommendations to the top of the list
/// </summary>
public class RecommenderPackageFeed : IPackageFeed
{
public bool IsMultiSource => false;
public bool IsMultiSource => _decoratedPackageFeed.IsMultiSource;

private readonly IPackageFeed _decoratedPackageFeed;
private readonly SourceRepository _sourceRepository;
private readonly List<string> _installedPackages;
private readonly List<string> _transitivePackages;
Expand All @@ -40,6 +42,7 @@ public class RecommenderPackageFeed : IPackageFeed
private const int MaxRecommended = 5;

public RecommenderPackageFeed(
IPackageFeed decoratedFeed,
IEnumerable<SourceRepository> sourceRepositories,
PackageCollection installedPackages,
PackageCollection transitivePackages,
Expand All @@ -59,6 +62,7 @@ public RecommenderPackageFeed(
{
throw new ArgumentNullException(nameof(transitivePackages));
}
_decoratedPackageFeed = decoratedFeed ?? throw new ArgumentNullException(nameof(decoratedFeed));
_targetFrameworks = targetFrameworks ?? throw new ArgumentNullException(nameof(targetFrameworks));
_metadataProvider = metadataProvider ?? throw new ArgumentNullException(nameof(metadataProvider));
_logger = logger ?? throw new ArgumentNullException(nameof(logger));
Expand Down Expand Up @@ -86,18 +90,27 @@ private class RecommendSearchToken : ContinuationToken
public SearchFilter SearchFilter { get; set; }
}

public Task<SearchResult<IPackageSearchMetadata>> SearchAsync(string searchText, SearchFilter searchFilter, CancellationToken cancellationToken)
public async Task<SearchResult<IPackageSearchMetadata>> SearchAsync(string searchText, SearchFilter searchFilter, CancellationToken cancellationToken)
{
var searchToken = new RecommendSearchToken
{
SearchString = searchText,
SearchFilter = searchFilter,
};

return RecommendPackagesAsync(searchToken, cancellationToken);
SearchResult<IPackageSearchMetadata> recommenderResults = await RecommendPackagesAsync(searchToken, cancellationToken);
SearchResult<IPackageSearchMetadata> decoratedResults = await _decoratedPackageFeed.SearchAsync(searchText, searchFilter, cancellationToken);

// Add the recommended results to the top of the decorated feed's results, deduplicating any packages in the decorated feed.
IReadOnlyList<IPackageSearchMetadata> combinedResults = recommenderResults.Items.Union(decoratedResults.Items, new IdentityIdEqualityComparer()).ToList();
jebriede marked this conversation as resolved.
Show resolved Hide resolved
SearchResult<IPackageSearchMetadata> result = SearchResult.FromItems(combinedResults);
// We want to make sure we can continue searching the decorated feed and its sources' search statuses are accurately represented.
result.NextToken = decoratedResults.NextToken;
result.SourceSearchStatus = decoratedResults.SourceSearchStatus;
return result;
}

public async Task<SearchResult<IPackageSearchMetadata>> RecommendPackagesAsync(ContinuationToken continuationToken, CancellationToken cancellationToken)
private async Task<SearchResult<IPackageSearchMetadata>> RecommendPackagesAsync(ContinuationToken continuationToken, CancellationToken cancellationToken)
{
var searchToken = continuationToken as RecommendSearchToken;
if (searchToken is null)
Expand Down Expand Up @@ -178,13 +191,15 @@ public async Task<SearchResult<IPackageSearchMetadata>> RecommendPackagesAsync(C
return result;
}

// Recommender has no ContinueSearch functionality, so delegate to the decorated feed
public Task<SearchResult<IPackageSearchMetadata>> ContinueSearchAsync(ContinuationToken continuationToken, CancellationToken cancellationToken)
=> System.Threading.Tasks.Task.FromResult(SearchResult.Empty<IPackageSearchMetadata>());
=> _decoratedPackageFeed.ContinueSearchAsync(continuationToken, cancellationToken);

// Recommender has no RefreshSearch functionality, so delegate to the decorated feed
public Task<SearchResult<IPackageSearchMetadata>> RefreshSearchAsync(RefreshToken refreshToken, CancellationToken cancellationToken)
=> System.Threading.Tasks.Task.FromResult(SearchResult.Empty<IPackageSearchMetadata>());
=> _decoratedPackageFeed.RefreshSearchAsync(refreshToken, cancellationToken);

public async Task<IPackageSearchMetadata> GetPackageMetadataAsync(PackageIdentity identity, bool includePrerelease, CancellationToken cancellationToken)
private async Task<IPackageSearchMetadata> GetPackageMetadataAsync(PackageIdentity identity, bool includePrerelease, CancellationToken cancellationToken)
{
// first we try and load the metadata from a local package
var packageMetadata = await _metadataProvider.GetLocalPackageMetadataAsync(identity, includePrerelease, cancellationToken);
Expand All @@ -193,8 +208,20 @@ public async Task<IPackageSearchMetadata> GetPackageMetadataAsync(PackageIdentit
// and failing that we go to the network
packageMetadata = await _metadataProvider.GetPackageMetadataAsync(identity, includePrerelease, cancellationToken);
}
return packageMetadata;
return new RecommendedPackageSearchMetadata(packageMetadata, VersionInfo);
}

private class IdentityIdEqualityComparer : IEqualityComparer<IPackageSearchMetadata>
{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A pattern I prefer is to make a private constructor with a static instance to ensure that no one makes extra instances of this

Suggested change
{
{
public static IdentityIdEqualityComparer Instance = new IdentityIdEqualityComparer();
private IdentityIdEqualityComparer()
{
}

Then line 26 can go away and line 107 can use the static singleton

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this a lot, thanks!

public bool Equals(IPackageSearchMetadata x, IPackageSearchMetadata y)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to be OrdinalIgnoreCase.

{
return x.Identity.Id.Equals(y.Identity.Id);
}

public int GetHashCode(IPackageSearchMetadata obj)
{
return obj.Identity.Id.GetHashCode();
}
}
}
}
56 changes: 27 additions & 29 deletions src/NuGet.Clients/NuGet.PackageManagement.VisualStudio/Services/NuGetPackageSearchService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,16 +80,15 @@ public async ValueTask<IReadOnlyCollection<PackageSearchMetadataContextInfo>> Ge

bool recommendPackages = false;
IReadOnlyCollection<SourceRepository> sourceRepositories = await _sharedServiceState.GetRepositoriesAsync(packageSources, cancellationToken);
(IPackageFeed? mainFeed, IPackageFeed? recommenderFeed) packageFeeds = await CreatePackageFeedAsync(
IPackageFeed packageFeed = await CreatePackageFeedAsync(
projectContextInfos,
targetFrameworks,
itemFilter,
isSolution,
recommendPackages,
sourceRepositories,
cancellationToken);

Assumes.NotNull(packageFeeds.mainFeed);
Assumes.NotNull(packageFeed);

SourceRepository packagesFolderSourceRepository = await _packagesFolderLocalRepositoryLazy.GetValueAsync(cancellationToken);
IEnumerable<SourceRepository> globalPackageFolderRepositories = await GetAllPackageFoldersAsync(projectContextInfos, cancellationToken);
Expand All @@ -99,7 +98,7 @@ public async ValueTask<IReadOnlyCollection<PackageSearchMetadataContextInfo>> Ge
globalPackageFolderRepositories,
new VisualStudioActivityLogger());

var searchObject = new SearchObject(packageFeeds.mainFeed, packageFeeds.recommenderFeed, metadataProvider, packageSources, PackageSearchMetadataMemoryCache);
var searchObject = new SearchObject(packageFeed, metadataProvider, packageSources, PackageSearchMetadataMemoryCache);
return await searchObject.GetAllPackagesAsync(searchFilter, cancellationToken);
}

Expand Down Expand Up @@ -264,15 +263,15 @@ public async ValueTask<SearchResultContextInfo> SearchAsync(
Assumes.NotNull(searchFilter);

IReadOnlyCollection<SourceRepository>? sourceRepositories = await _sharedServiceState.GetRepositoriesAsync(packageSources, cancellationToken);
(IPackageFeed? mainFeed, IPackageFeed? recommenderFeed) = await CreatePackageFeedAsync(
IPackageFeed packageFeed = await CreatePackageFeedAsync(
projectContextInfos,
targetFrameworks,
itemFilter,
isSolution,
useRecommender,
sourceRepositories,
cancellationToken);
Assumes.NotNull(mainFeed);
Assumes.NotNull(packageFeed);

SourceRepository packagesFolderSourceRepository = await _packagesFolderLocalRepositoryLazy.GetValueAsync(cancellationToken);
IEnumerable<SourceRepository> globalPackageFolderRepositories = await GetAllPackageFoldersAsync(projectContextInfos, cancellationToken);
Expand All @@ -282,7 +281,7 @@ public async ValueTask<SearchResultContextInfo> SearchAsync(
globalPackageFolderRepositories,
new VisualStudioActivityLogger());

_searchObject = new SearchObject(mainFeed, recommenderFeed, metadataProvider, packageSources, PackageSearchMetadataMemoryCache);
_searchObject = new SearchObject(packageFeed, metadataProvider, packageSources, PackageSearchMetadataMemoryCache);
return await _searchObject.SearchAsync(searchText, searchFilter, useRecommender, cancellationToken);
}

Expand All @@ -301,8 +300,8 @@ public async ValueTask<int> GetTotalCountAsync(
Assumes.NotNull(searchFilter);

IReadOnlyCollection<SourceRepository>? sourceRepositories = await _sharedServiceState.GetRepositoriesAsync(packageSources, cancellationToken);
(IPackageFeed? mainFeed, IPackageFeed? recommenderFeed) = await CreatePackageFeedAsync(projectContextInfos, targetFrameworks, itemFilter, isSolution, recommendPackages: false, sourceRepositories, cancellationToken);
Assumes.NotNull(mainFeed);
IPackageFeed packageFeed = await CreatePackageFeedAsync(projectContextInfos, targetFrameworks, itemFilter, isSolution, recommendPackages: false, sourceRepositories, cancellationToken);
Assumes.NotNull(packageFeed);

SourceRepository packagesFolderSourceRepository = await _packagesFolderLocalRepositoryLazy.GetValueAsync(cancellationToken);
IEnumerable<SourceRepository> globalPackageFolderRepositories = await GetAllPackageFoldersAsync(projectContextInfos, cancellationToken);
Expand All @@ -312,7 +311,7 @@ public async ValueTask<int> GetTotalCountAsync(
globalPackageFolderRepositories,
new VisualStudioActivityLogger());

var searchObject = new SearchObject(mainFeed, recommenderFeed, metadataProvider, packageSources, searchCache: null);
var searchObject = new SearchObject(packageFeed, metadataProvider, packageSources, searchCache: null);
return await searchObject.GetTotalCountAsync(maxCount, searchFilter, cancellationToken);
}

Expand Down Expand Up @@ -383,7 +382,7 @@ public async Task<IReadOnlyList<SourceRepository>> GetAllPackageFoldersAsync(
return allLocalFolders;
}

internal async Task<(IPackageFeed? mainFeed, IPackageFeed? recommenderFeed)> CreatePackageFeedAsync(
internal async Task<IPackageFeed> CreatePackageFeedAsync(
IReadOnlyCollection<IProjectContextInfo> projectContextInfos,
IReadOnlyCollection<string> targetFrameworks,
ItemFilter itemFilter,
Expand All @@ -394,12 +393,12 @@ public async Task<IReadOnlyList<SourceRepository>> GetAllPackageFoldersAsync(
{
var logger = new VisualStudioActivityLogger();
var uiLogger = await ServiceLocator.GetComponentModelServiceAsync<INuGetUILogger>();
var packageFeeds = (mainFeed: (IPackageFeed?)null, recommenderFeed: (IPackageFeed?)null);
IPackageFeed? packageFeed = null;

if (itemFilter == ItemFilter.All && recommendPackages == false)
{
packageFeeds.mainFeed = new MultiSourcePackageFeed(sourceRepositories, uiLogger, TelemetryActivity.NuGetTelemetryService);
return packageFeeds;
packageFeed = new MultiSourcePackageFeed(sourceRepositories, uiLogger, TelemetryActivity.NuGetTelemetryService);
return packageFeed;
}

IEnumerable<SourceRepository> globalPackageFolderRepositories = await GetAllPackageFoldersAsync(projectContextInfos, cancellationToken);
Expand All @@ -418,11 +417,12 @@ public async Task<IReadOnlyList<SourceRepository>> GetAllPackageFoldersAsync(
PackageCollection transitivePackageCollection = PackageCollection.FromPackageReferences(browseTabPackages.TransitivePackages);

// if we get here, recommendPackages == true
packageFeeds.mainFeed = new MultiSourcePackageFeed(sourceRepositories, uiLogger, TelemetryActivity.NuGetTelemetryService);
packageFeed = new MultiSourcePackageFeed(sourceRepositories, uiLogger, TelemetryActivity.NuGetTelemetryService);
try
{
// Recommender needs installed and transitive package lists, but it does not need transitive origins data.
packageFeeds.recommenderFeed = new RecommenderPackageFeed(
return new RecommenderPackageFeed(
packageFeed,
sourceRepositories,
installedPackageCollection,
transitivePackageCollection,
Expand All @@ -434,8 +434,8 @@ public async Task<IReadOnlyList<SourceRepository>> GetAllPackageFoldersAsync(
{
// This could happen if the user disables the recommender extension. Catching this
// exception allows the package manager to continue without recommendations.
return packageFeed;
}
return packageFeeds;
}

if (itemFilter == ItemFilter.Installed)
Expand All @@ -445,9 +445,7 @@ public async Task<IReadOnlyList<SourceRepository>> GetAllPackageFoldersAsync(
PackageCollection installedPackageCollection = PackageCollection.FromPackageReferences(installedTabWithTransitiveOrigins.InstalledPackages);
PackageCollection transitivePackageCollection = PackageCollection.FromPackageReferences(installedTabWithTransitiveOrigins.TransitivePackages);

packageFeeds.mainFeed = new InstalledAndTransitivePackageFeed(installedPackageCollection, transitivePackageCollection, metadataProvider);

return packageFeeds;
return new InstalledAndTransitivePackageFeed(installedPackageCollection, transitivePackageCollection, metadataProvider);
}

if (itemFilter == ItemFilter.Consolidate)
Expand All @@ -456,29 +454,29 @@ public async Task<IReadOnlyList<SourceRepository>> GetAllPackageFoldersAsync(
IReadOnlyCollection<IPackageReferenceContextInfo> installedTabPackages = await GetAllInstalledPackagesAsync(projectContextInfos, cancellationToken);
PackageCollection installedPackageCollection = PackageCollection.FromPackageReferences(installedTabPackages);

packageFeeds.mainFeed = new ConsolidatePackageFeed(installedPackageCollection, metadataProvider, logger);
return packageFeeds;
packageFeed = new ConsolidatePackageFeed(installedPackageCollection, metadataProvider, logger);
return packageFeed;
}

// Search all / updates available cannot work without a source repo
if (sourceRepositories == null)
{
return packageFeeds;
}
//// Search all / updates available cannot work without a source repo
//if (sourceRepositories == null)
//{
// return packageFeed;
//}

if (itemFilter == ItemFilter.UpdatesAvailable)
{
// Updates tab, Project or Solution View: only needs installed packages
IReadOnlyCollection<IPackageReferenceContextInfo> updatedTabPackages = await GetAllInstalledPackagesAsync(projectContextInfos, cancellationToken);
PackageCollection installedPackageCollection = PackageCollection.FromPackageReferences(updatedTabPackages);

packageFeeds.mainFeed = new UpdatePackageFeed(
packageFeed = new UpdatePackageFeed(
_serviceBroker,
installedPackageCollection,
metadataProvider,
projectContextInfos.ToArray());

return packageFeeds;
return packageFeed;
}

throw new InvalidOperationException(
Expand Down
Loading
Loading