Move Utf8JsonStreamReader & extensions to NuGet Shared #6834

donnie-msft · Oct 3, 2025

Bug

Fixes: NuGet/Home#14579

Description

Moves the Utf8Json helper classes into NuGet Shared.

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.~~

src/NuGet.Core/NuGet.ProjectModel/LockFile/Utf8JsonStreamLockFileConverter.cs

NuGet.sln

...Get.Clients/NuGet.PackageManagement.VisualStudio/NuGet.PackageManagement.VisualStudio.csproj

kartheekp-ms · Oct 13, 2025

build/Shared/Utf8JsonStreamReader.cs

        }

-        internal bool ReadNextTokenAsBoolOrThrowAnException(byte[] propertyName)
+        internal bool ReadNextTokenAsBoolOrThrowAnException(byte[] propertyName, string invalidAttributeString)


I know that you are simply moving the file to NuGet Shared in this PR, but out of curiosity, I am wondering why this method only throws an error when reading the boolean value from the JSON object and the other methods don't. When I checked the callers of this method, they are not handling the exception as a special case. That makes me think: do we really need this method? Can we simply call the ReadNextTokenAsBoolOrFalse method instead and delete this method?

It's a good point. I see that it was discussed in the PR introducing it and the decision was to rename it to be distinct from ReadNextTokenAsBoolOrFalse.
I agree it's unclear why this is important, but there are 2 tests explicitly checking for an exception. Therefore, this could be a behavior change which I don't want to take on in this refactoring. See:

NuGet.Client/test/NuGet.Clients.Tests/NuGet.PackageManagement.VisualStudio.Test/ProjectSystems/LegacyPackageReferenceProjectTests.cs

Lines 1737 to 1790 in 32dcbfb

public async Task GetPackageSpec_WithInvalidSdkAnalysisLevel_ThrowsAnException()

{

await NuGetUIThreadHelper.JoinableTaskFactory.SwitchToMainThreadAsync();

// Arrange

using var testDirectory = TestDirectory.Create();

var projectBuildProperties = new Mock<IVsProjectBuildProperties>();

projectBuildProperties.Setup(b => b.GetPropertyValue(ProjectBuildProperties.SdkAnalysisLevel))

.Returns("9.ainvlaid");

var projectAdapter = CreateProjectAdapter(testDirectory, projectBuildProperties);

Mock<IVsProjectAdapter> projectAdapterMock = Mock.Get(projectAdapter);

var projectServices = new TestProjectSystemServices();

var testProject = new LegacyPackageReferenceProject(

projectAdapter,

Guid.NewGuid().ToString(),

projectServices,

_threadingService);

var settings = NullSettings.Instance;

var testDependencyGraphCacheContext = new DependencyGraphCacheContext(NullLogger.Instance, settings);

// Act & Assert

await Assert.ThrowsAsync<ArgumentException>(async () => await testProject.GetPackageSpecsAsync(testDependencyGraphCacheContext));

}

[Fact]

public async Task GetPackageSpec_WithInvalidUsingMicrosoftNetSdk_ThrowsAnException()

{

await NuGetUIThreadHelper.JoinableTaskFactory.SwitchToMainThreadAsync();

// Arrange

using var testDirectory = TestDirectory.Create();

var projectBuildProperties = new Mock<IVsProjectBuildProperties>();

projectBuildProperties.Setup(b => b.GetPropertyValue(ProjectBuildProperties.UsingMicrosoftNETSdk))

.Returns("falsetrue");

var projectAdapter = CreateProjectAdapter(testDirectory, projectBuildProperties);

Mock<IVsProjectAdapter> projectAdapterMock = Mock.Get(projectAdapter);

var projectServices = new TestProjectSystemServices();

var testProject = new LegacyPackageReferenceProject(

projectAdapter,

Guid.NewGuid().ToString(),

projectServices,

_threadingService);

var settings = NullSettings.Instance;

var testDependencyGraphCacheContext = new DependencyGraphCacheContext(NullLogger.Instance, settings);

// Act & Assert

await Assert.ThrowsAsync<ArgumentException>(async () => await testProject.GetPackageSpecsAsync(testDependencyGraphCacheContext));

}

donnie-msft mentioned this pull request Oct 3, 2025

Move Visual Studio's Utf8JsonStreamReader & extensions to NuGet Shared #6833

Closed

3 tasks

dotnet-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 10, 2025

donnie-msft changed the title ~~[Draft] Move Utf8JsonStreamReader & extensions to NuGet Shared~~ Move Utf8JsonStreamReader & extensions to NuGet Shared Oct 13, 2025

dotnet-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 13, 2025

donnie-msft marked this pull request as ready for review October 13, 2025 21:17

donnie-msft requested a review from a team as a code owner October 13, 2025 21:17

donnie-msft requested review from jeffkl, kartheekp-ms and martinrrm October 13, 2025 21:17

kartheekp-ms reviewed Oct 13, 2025

View reviewed changes

src/NuGet.Core/NuGet.ProjectModel/LockFile/Utf8JsonStreamLockFileConverter.cs Outdated Show resolved Hide resolved

kartheekp-ms reviewed Oct 13, 2025

View reviewed changes

NuGet.sln Outdated Show resolved Hide resolved

kartheekp-ms reviewed Oct 13, 2025

View reviewed changes

...Get.Clients/NuGet.PackageManagement.VisualStudio/NuGet.PackageManagement.VisualStudio.csproj Outdated Show resolved Hide resolved

kartheekp-ms reviewed Oct 13, 2025

View reviewed changes

donnie-msft and others added 6 commits October 14, 2025 10:34

Move Utf8JsonStreamReader & extensions to NuGet Shared

19733a1

cleanup

2028de8

WIP - Move NuGet.ProjectModel to shared Utf8JsonStreamReader

2c37d1d

Update tests

cbee774

One Utf8JsonStreamReader to rule them all!

b7146ed

Remove unused String Resx

0edef0c

donnie-msft force-pushed the dev-donnie-msft-pjRefactorHelperAll branch from c453148 to 0edef0c Compare October 14, 2025 18:03

Remove static

6bb1267

donnie-msft requested a review from kartheekp-ms October 14, 2025 19:57

jeffkl approved these changes Oct 14, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Move Utf8JsonStreamReader & extensions to NuGet Shared #6834

Move Utf8JsonStreamReader & extensions to NuGet Shared #6834

donnie-msft commented Oct 3, 2025 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kartheekp-ms Oct 13, 2025 •

edited

Loading

Uh oh!

donnie-msft Oct 14, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	public async Task GetPackageSpec_WithInvalidSdkAnalysisLevel_ThrowsAnException()
	{
	await NuGetUIThreadHelper.JoinableTaskFactory.SwitchToMainThreadAsync();

	// Arrange
	using var testDirectory = TestDirectory.Create();
	var projectBuildProperties = new Mock<IVsProjectBuildProperties>();
	projectBuildProperties.Setup(b => b.GetPropertyValue(ProjectBuildProperties.SdkAnalysisLevel))
	.Returns("9.ainvlaid");
	var projectAdapter = CreateProjectAdapter(testDirectory, projectBuildProperties);

	Mock<IVsProjectAdapter> projectAdapterMock = Mock.Get(projectAdapter);

	var projectServices = new TestProjectSystemServices();
	var testProject = new LegacyPackageReferenceProject(
	projectAdapter,
	Guid.NewGuid().ToString(),
	projectServices,
	_threadingService);

	var settings = NullSettings.Instance;
	var testDependencyGraphCacheContext = new DependencyGraphCacheContext(NullLogger.Instance, settings);

	// Act & Assert
	await Assert.ThrowsAsync<ArgumentException>(async () => await testProject.GetPackageSpecsAsync(testDependencyGraphCacheContext));
	}

	[Fact]
	public async Task GetPackageSpec_WithInvalidUsingMicrosoftNetSdk_ThrowsAnException()
	{
	await NuGetUIThreadHelper.JoinableTaskFactory.SwitchToMainThreadAsync();

	// Arrange
	using var testDirectory = TestDirectory.Create();
	var projectBuildProperties = new Mock<IVsProjectBuildProperties>();
	projectBuildProperties.Setup(b => b.GetPropertyValue(ProjectBuildProperties.UsingMicrosoftNETSdk))
	.Returns("falsetrue");
	var projectAdapter = CreateProjectAdapter(testDirectory, projectBuildProperties);

	Mock<IVsProjectAdapter> projectAdapterMock = Mock.Get(projectAdapter);

	var projectServices = new TestProjectSystemServices();
	var testProject = new LegacyPackageReferenceProject(
	projectAdapter,
	Guid.NewGuid().ToString(),
	projectServices,
	_threadingService);

	var settings = NullSettings.Instance;
	var testDependencyGraphCacheContext = new DependencyGraphCacheContext(NullLogger.Instance, settings);

	// Act & Assert
	await Assert.ThrowsAsync<ArgumentException>(async () => await testProject.GetPackageSpecsAsync(testDependencyGraphCacheContext));
	}

Search code, repositories, users, issues, pull requests...

Move Utf8JsonStreamReader & extensions to NuGet Shared #6834

Are you sure you want to change the base?

Move Utf8JsonStreamReader & extensions to NuGet Shared #6834

Conversation

donnie-msft commented Oct 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Bug

Description

PR Checklist

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kartheekp-ms Oct 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

donnie-msft Oct 14, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

donnie-msft commented Oct 3, 2025 •

edited

Loading

kartheekp-ms Oct 13, 2025 •

edited

Loading