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.

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

Update ProjectSystemCache to be case insensitive for _primaryCache #6055

Merged
merged 1 commit into from
Oct 8, 2024

Conversation

jgonz120
Copy link
Contributor

@jgonz120 jgonz120 commented Sep 25, 2024

Bug

Fixes: https://github.com/NuGet/Client.Engineering/issues/2829

Description

Projects information was being added/updated with different cases, causes our apex tests to fail. The primary key for the _primaryCache is projectNames.FullName which is the full path and filename of the project. Switching the dictionary to ignore case allows the project to be added to the single entry instead of creating a new one.

When we load an empty solution, there are no projects, and VsSolutionManager.EnsureNuGetAndVsProjectAdapterCacheAsync does not initialize the cache.

When we create a new project we call VsSolutionManager.OnEnvDTEProjectAdded which attempts to initialize the cache again then add the project.

EnsureNuGetAndVsProjectAdapterCacheAsync gets the project name from

public static string GetProjectPathForWebsiteProject(IVsHierarchy project)
{
Assumes.Present(project);
ThreadHelper.ThrowIfNotOnUIThread();
project.GetCanonicalName((uint)VSConstants.VSITEMID.Root, out string projectPath);
return projectPath;
}

Then later in OnEnVDTEProjectedAdded we get the full name from the EnvDTE.Project full name

PR Checklist

  • Meaningful title, helpful description and a linked NuGet/Home issue
  • Added tests
  • Link to an issue or pull request to update docs if this PR changes settings, environment variables, new feature, etc.

@jgonz120 jgonz120 requested a review from a team as a code owner September 25, 2024 22:59
@jgonz120 jgonz120 added the Merge next release PRs that should not be merged until the dev branch targets the next release label Sep 25, 2024
Nigusu-Allehu
Nigusu-Allehu previously approved these changes Sep 25, 2024
@jgonz120 jgonz120 marked this pull request as draft September 26, 2024 17:46
@nkolev92 nkolev92 removed the Merge next release PRs that should not be merged until the dev branch targets the next release label Sep 27, 2024
@microsoft-github-policy-service microsoft-github-policy-service bot added the Status:No recent activity PRs that have not had any recent activity and will be closed if the label is not removed label Oct 4, 2024
@microsoft-github-policy-service microsoft-github-policy-service bot removed the Status:No recent activity PRs that have not had any recent activity and will be closed if the label is not removed label Oct 7, 2024
@jgonz120 jgonz120 marked this pull request as ready for review October 7, 2024 16:31
@@ -22,7 +22,7 @@ internal class ProjectSystemCache : IProjectSystemCache, IDisposable
// 0 - Cache is clean
// 1 - Cache is dirty
private int _isCacheDirty = 0;
private readonly Dictionary<string, CacheEntry> _primaryCache = new Dictionary<string, CacheEntry>();
private readonly Dictionary<string, CacheEntry> _primaryCache = new Dictionary<string, CacheEntry>(StringComparer.OrdinalIgnoreCase);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was originally comparing ProjectNames. 3d091da#diff-5a34d358c305cf4b9fb65e2fba45c849406b2898a65aa0a75513e41370c2161aR19

ProjectNames.Equals does a case insensitive check over the full name, so this change restores the dictionary to be case insensitive.

@jgonz120 jgonz120 merged commit 7b42029 into dev Oct 8, 2024
28 checks passed
@jgonz120 jgonz120 deleted the dev-jongonza-nugetuitest branch October 8, 2024 15:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants