Skip to content
Merged
Show file tree
Hide file tree
Changes from 37 commits
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
a57aabf
Copied initial files over and addressred some PR comments
jgonz120 Dec 20, 2023
05ed3db
Test updates
jgonz120 Dec 21, 2023
c285f0f
optimized string split
jgonz120 Dec 21, 2023
c06e3f5
cleanup
jgonz120 Dec 21, 2023
ef90a51
switch to stream
jgonz120 Dec 21, 2023
4c31cbf
fix typo
jgonz120 Dec 21, 2023
b787826
fix typo
jgonz120 Dec 21, 2023
c627b25
create static json reader state
jgonz120 Dec 21, 2023
ef35381
typo
jgonz120 Dec 21, 2023
084986d
typo
jgonz120 Dec 21, 2023
5850754
added lazy string split
jgonz120 Dec 22, 2023
014739f
using
jgonz120 Dec 22, 2023
d28a482
Update unit tests
jgonz120 Dec 22, 2023
c9cb53f
unit tests
jgonz120 Dec 22, 2023
d083e23
fix references
jgonz120 Dec 22, 2023
0b22126
Fix typo
jgonz120 Dec 22, 2023
e81974f
Added test for invalid logs
jgonz120 Jan 4, 2024
d2cc5d6
removed extra name assignment from package spec reader
jgonz120 Jan 4, 2024
2de68c4
use array empty
jgonz120 Jan 4, 2024
79e2b71
move public method up
jgonz120 Jan 4, 2024
ccd6fe1
remove uneeded string list
jgonz120 Jan 4, 2024
8d719c6
use false string
jgonz120 Jan 4, 2024
212b7bd
add is final block to test
jgonz120 Jan 4, 2024
b5a82d7
rename test
jgonz120 Jan 4, 2024
82363ee
style
jgonz120 Jan 4, 2024
0cc4e51
style
jgonz120 Jan 4, 2024
c525dc8
add tests for validating empty streams on creationg of utf8jsonstream…
jgonz120 Jan 4, 2024
4e5112a
Added test and implemented string split in two
jgonz120 Jan 5, 2024
2398980
fix validation for lazy string split
jgonz120 Jan 8, 2024
1693913
reduce methods in LikeFileFormat
jgonz120 Jan 8, 2024
e0ef66d
set the list values with the results directly
jgonz120 Jan 8, 2024
b4d0169
store environment variable to avoid calling GetEnvironmentVariable se…
jgonz120 Jan 8, 2024
f060670
switch to splitintwo
jgonz120 Jan 9, 2024
4602b44
Update conditional for framework
jgonz120 Jan 9, 2024
a240062
Caching the parsed NugetVersion and VersionRange objects.
jgonz120 Jan 11, 2024
0883567
Fixes from PR
jgonz120 Jan 11, 2024
c4adaf7
Avoid creating empty lists
jgonz120 Jan 11, 2024
bda30bb
Revert "Caching the parsed NugetVersion and VersionRange objects."
jgonz120 Jan 17, 2024
fb7ebd5
Fix netwonsoft json parsing
jgonz120 Jan 17, 2024
32aba9a
Add missing reference
jgonz120 Jan 17, 2024
8853734
added some examples of the json to be parsed
jgonz120 Jan 17, 2024
1b29455
Fix references in comments
jgonz120 Jan 17, 2024
1877448
Fixes from PR
jgonz120 Jan 22, 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
Original file line number Diff line number Diff line change
Expand Up @@ -112,19 +112,7 @@ public partial class JsonPackageSpecReader
internal static PackageSpec GetPackageSpecUtf8JsonStreamReader(Stream stream, string name, string packageSpecPath, string snapshotValue)
{
var reader = new Utf8JsonStreamReader(stream);
PackageSpec packageSpec;
packageSpec = GetPackageSpec(ref reader, name, packageSpecPath, snapshotValue);

if (!string.IsNullOrEmpty(name))
{
packageSpec.Name = name;
if (!string.IsNullOrEmpty(packageSpecPath))
{
packageSpec.FilePath = Path.GetFullPath(packageSpecPath);

}
}
return packageSpec;
return GetPackageSpec(ref reader, name, packageSpecPath, snapshotValue);
}

internal static PackageSpec GetPackageSpec(ref Utf8JsonStreamReader jsonReader, string name, string packageSpecPath, string snapshotValue)
Expand Down Expand Up @@ -277,9 +265,10 @@ internal static PackageSpec GetPackageSpec(ref Utf8JsonStreamReader jsonReader,

internal static void ReadCentralTransitiveDependencyGroup(
ref Utf8JsonStreamReader jsonReader,
IList<LibraryDependency> results,
out IList<LibraryDependency> results,
string packageSpecPath)
{
results = null;
if (jsonReader.Read() && jsonReader.TokenType == JsonTokenType.StartObject)
{
while (jsonReader.Read() && jsonReader.TokenType == JsonTokenType.PropertyName)
Expand All @@ -295,10 +284,12 @@ internal static void ReadCentralTransitiveDependencyGroup(
if (jsonReader.Read())
{
var libraryDependency = ReadLibraryDependency(ref jsonReader, packageSpecPath, libraryName);
results ??= [];
results.Add(libraryDependency);
}
}
}
results ??= Array.Empty<LibraryDependency>();
}

private static LibraryDependency ReadLibraryDependency(ref Utf8JsonStreamReader jsonReader, string packageSpecPath, string libraryName)
Expand Down Expand Up @@ -328,7 +319,7 @@ private static LibraryDependency ReadLibraryDependency(ref Utf8JsonStreamReader
{
try
{
dependencyVersionRange = VersionRange.Parse(dependencyVersionValue);
dependencyVersionRange = JsonUtility.ParseVersionRange(dependencyVersionValue);
}
catch (Exception ex)
{
Expand Down Expand Up @@ -498,7 +489,7 @@ private static void ReadDependencies(
var versionPropValue = jsonReader.GetString();
try
{
versionOverride = VersionRange.Parse(versionPropValue);
versionOverride = JsonUtility.ParseVersionRange(versionPropValue);
}
catch (Exception ex)
{
Expand Down Expand Up @@ -527,7 +518,7 @@ private static void ReadDependencies(
{
try
{
dependencyVersionRange = VersionRange.Parse(dependencyVersionValue);
dependencyVersionRange = JsonUtility.ParseVersionRange(dependencyVersionValue);
}
catch (Exception ex)
{
Expand Down Expand Up @@ -637,7 +628,7 @@ private static void ReadCentralPackageVersions(
throw FileFormatException.Create("The version cannot be null or empty.", filePath);
}

centralPackageVersions[propertyName] = new CentralPackageVersion(propertyName, VersionRange.Parse(version));
centralPackageVersions[propertyName] = new CentralPackageVersion(propertyName, JsonUtility.ParseVersionRange(version));
}
}
}
Expand Down Expand Up @@ -733,13 +724,18 @@ private static void ReadDownloadDependencies(
packageSpecPath);
}

string[] versions = versionValue.Split(VersionSeparators, StringSplitOptions.RemoveEmptyEntries);
var versions = new LazyStringSplit(versionValue, VersionSeparator);

foreach (string singleVersionValue in versions)
{
if (string.IsNullOrEmpty(singleVersionValue))
{
continue;
}

try
{
VersionRange version = VersionRange.Parse(singleVersionValue);
VersionRange version = JsonUtility.ParseVersionRange(singleVersionValue);

downloadDependencies.Add(new DownloadDependency(name, version));
}
Expand Down Expand Up @@ -938,7 +934,7 @@ private static void ReadMSBuildMetadata(ref Utf8JsonStreamReader jsonReader, Pac
RestoreLockProperties restoreLockProperties = null;
var skipContentFileWrite = false;
List<PackageSource> sources = null;
List<ProjectRestoreMetadataFrameworkInfo> targetFrameworks = null;
IList<ProjectRestoreMetadataFrameworkInfo> targetFrameworks = null;
var validateRuntimeAssets = false;
WarningProperties warningProperties = null;
RestoreAuditProperties auditProperties = null;
Expand Down Expand Up @@ -1294,7 +1290,7 @@ private static void ReadPackageTypes(PackageSpec packageSpec, ref Utf8JsonStream
packageTypes = new[] { packageType };
break;
case JsonTokenType.StartArray:
var types = new List<PackageType>();
List<PackageType> types = null;

while (jsonReader.Read() && jsonReader.TokenType != JsonTokenType.EndArray)
{
Expand All @@ -1309,8 +1305,10 @@ private static void ReadPackageTypes(PackageSpec packageSpec, ref Utf8JsonStream
}

packageType = CreatePackageType(ref jsonReader);
types ??= [];
types.Add(packageType);
}

packageTypes = types;
break;
case JsonTokenType.Null:
Expand Down Expand Up @@ -1496,7 +1494,7 @@ private static RuntimeDependencySet ReadRuntimeDependencySet(ref Utf8JsonStreamR
var propertyName = jsonReader.GetString();
dependencies ??= [];

var dependency = new RuntimePackageDependency(propertyName, VersionRange.Parse(jsonReader.ReadNextTokenAsString()));
var dependency = new RuntimePackageDependency(propertyName, JsonUtility.ParseVersionRange(jsonReader.ReadNextTokenAsString()));

dependencies.Add(dependency);
}
Expand Down Expand Up @@ -1541,14 +1539,14 @@ private static RuntimeDescription ReadRuntimeDescription(ref Utf8JsonStreamReade

private static List<RuntimeDescription> ReadRuntimes(ref Utf8JsonStreamReader jsonReader)
{
var runtimeDescriptions = new List<RuntimeDescription>();
List<RuntimeDescription> runtimeDescriptions = null;

if (jsonReader.Read() && jsonReader.TokenType == JsonTokenType.StartObject)
{
while (jsonReader.Read() && jsonReader.TokenType == JsonTokenType.PropertyName)
{
RuntimeDescription runtimeDescription = ReadRuntimeDescription(ref jsonReader, jsonReader.GetString());

runtimeDescriptions ??= [];
runtimeDescriptions.Add(runtimeDescription);
}
}
Expand All @@ -1572,14 +1570,15 @@ private static void ReadScripts(ref Utf8JsonStreamReader jsonReader, PackageSpec
}
else if (jsonReader.TokenType == JsonTokenType.StartArray)
{
var list = new List<string>();
IList<string> list = null;

while (jsonReader.Read() && jsonReader.TokenType == JsonTokenType.String)
{
list ??= [];
list.Add(jsonReader.GetString());
}

packageSpec.Scripts[propertyName] = list;
packageSpec.Scripts[propertyName] = list ?? Enumerable.Empty<string>();
}
else
{
Expand All @@ -1594,15 +1593,15 @@ private static void ReadScripts(ref Utf8JsonStreamReader jsonReader, PackageSpec

private static List<CompatibilityProfile> ReadSupports(ref Utf8JsonStreamReader jsonReader)
{
var compatibilityProfiles = new List<CompatibilityProfile>();
List<CompatibilityProfile> compatibilityProfiles = null;

if (jsonReader.Read() && jsonReader.TokenType == JsonTokenType.StartObject)
{
while (jsonReader.Read() && jsonReader.TokenType == JsonTokenType.PropertyName)
{
var propertyName = jsonReader.GetString();
CompatibilityProfile compatibilityProfile = ReadCompatibilityProfile(ref jsonReader, propertyName);

compatibilityProfiles ??= [];
compatibilityProfiles.Add(compatibilityProfile);
}
}
Expand Down Expand Up @@ -1640,7 +1639,7 @@ private static LibraryDependencyTarget ReadTarget(

private static List<ProjectRestoreMetadataFrameworkInfo> ReadTargetFrameworks(ref Utf8JsonStreamReader jsonReader)
{
var targetFrameworks = new List<ProjectRestoreMetadataFrameworkInfo>();
List<ProjectRestoreMetadataFrameworkInfo> targetFrameworks = null;

if (jsonReader.Read() && jsonReader.TokenType == JsonTokenType.StartObject)
{
Expand Down Expand Up @@ -1723,7 +1722,7 @@ private static List<ProjectRestoreMetadataFrameworkInfo> ReadTargetFrameworks(re
jsonReader.Skip();
}
}

targetFrameworks ??= [];
targetFrameworks.Add(frameworkGroup);
}
}
Expand Down
18 changes: 9 additions & 9 deletions src/NuGet.Core/NuGet.ProjectModel/JsonPackageSpecReader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ public static partial class JsonPackageSpecReader
{
private static readonly char[] DelimitedStringSeparators = { ' ', ',' };
private static readonly char[] VersionSeparators = new[] { ';' };
private const char VersionSeparator = ';';
public static readonly string RestoreOptions = "restore";
public static readonly string RestoreSettings = "restoreSettings";
public static readonly string HideWarningsAndErrors = "hideWarningsAndErrors";
Expand Down Expand Up @@ -75,10 +76,9 @@ internal static PackageSpec GetPackageSpec(JsonTextReader jsonReader, string pac
return GetPackageSpec(jsonReader, name: null, packageSpecPath, snapshotValue: null);
}

internal static PackageSpec GetPackageSpec(Stream stream, string name, string packageSpecPath, string snapshotValue, IEnvironmentVariableReader environmentVariableReader)
internal static PackageSpec GetPackageSpec(Stream stream, string name, string packageSpecPath, string snapshotValue, IEnvironmentVariableReader environmentVariableReader, bool bypassCache = false)
{
var useNj = environmentVariableReader.GetEnvironmentVariable("NUGET_EXPERIMENTAL_USE_NJ_FOR_FILE_PARSING");
if (string.IsNullOrEmpty(useNj) || useNj.Equals("false", StringComparison.OrdinalIgnoreCase))
if (!JsonUtility.UseNewstonSoftJsonForParsing(environmentVariableReader, bypassCache))
{
return GetPackageSpecUtf8JsonStreamReader(stream, name, packageSpecPath, snapshotValue);
}
Expand Down Expand Up @@ -306,7 +306,7 @@ private static void ReadCentralPackageVersions(
filePath);
}

centralPackageVersions[propertyName] = new CentralPackageVersion(propertyName, VersionRange.Parse(version));
centralPackageVersions[propertyName] = new CentralPackageVersion(propertyName, JsonUtility.ParseVersionRange(version));
});
}

Expand Down Expand Up @@ -449,7 +449,7 @@ private static void ReadDependencies(
{
try
{
versionOverride = VersionRange.Parse((string)jsonReader.Value);
versionOverride = JsonUtility.ParseVersionRange((string)jsonReader.Value);
}
catch (Exception ex)
{
Expand Down Expand Up @@ -478,7 +478,7 @@ private static void ReadDependencies(
{
try
{
dependencyVersionRange = VersionRange.Parse(dependencyVersionValue);
dependencyVersionRange = JsonUtility.ParseVersionRange(dependencyVersionValue);
}
catch (Exception ex)
{
Expand Down Expand Up @@ -620,7 +620,7 @@ internal static void ReadCentralTransitiveDependencyGroup(
{
try
{
dependencyVersionRange = VersionRange.Parse(dependencyVersionValue);
dependencyVersionRange = JsonUtility.ParseVersionRange(dependencyVersionValue);
}
catch (Exception ex)
{
Expand Down Expand Up @@ -737,7 +737,7 @@ private static void ReadDownloadDependencies(
{
try
{
VersionRange version = VersionRange.Parse(singleVersionValue);
VersionRange version = JsonUtility.ParseVersionRange(singleVersionValue);

downloadDependencies.Add(new DownloadDependency(name, version));
}
Expand Down Expand Up @@ -1502,7 +1502,7 @@ static RuntimeDependencySet ReadRuntimeDependencySet(JsonTextReader jsonReader,
{
dependencies ??= new List<RuntimePackageDependency>();

var dependency = new RuntimePackageDependency(propertyName, VersionRange.Parse(jsonReader.ReadNextTokenAsString()));
var dependency = new RuntimePackageDependency(propertyName, JsonUtility.ParseVersionRange(jsonReader.ReadNextTokenAsString()));

dependencies.Add(dependency);
});
Expand Down
62 changes: 61 additions & 1 deletion src/NuGet.Core/NuGet.ProjectModel/JsonUtility.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,19 @@
using System.IO;
using Newtonsoft.Json;
using Newtonsoft.Json.Linq;
using NuGet.Common;
using NuGet.Packaging.Core;
using NuGet.Versioning;

namespace NuGet.ProjectModel
{
internal static class JsonUtility
{
private static readonly Dictionary<string, NuGetVersion> NuGetVersionCache = new();
private static readonly Dictionary<string, VersionRange> VersionRangeCache = new();

internal const string NUGET_EXPERIMENTAL_USE_NJ_FOR_FILE_PARSING = nameof(NUGET_EXPERIMENTAL_USE_NJ_FOR_FILE_PARSING);
internal static bool? UseNewtonsoftJson = null;
internal static readonly char[] PathSplitChars = new[] { LockFile.DirectorySeparatorChar };

/// <summary>
Expand Down Expand Up @@ -43,12 +49,35 @@ internal static JObject LoadJson(TextReader reader)
}
}

internal static T LoadJson<T>(Stream stream, IUtf8JsonStreamReaderConverter<T> converter)
{
var streamingJsonReader = new Utf8JsonStreamReader(stream);
return converter.Read(ref streamingJsonReader);
}

internal static PackageDependency ReadPackageDependency(string property, JToken json)
{
var versionStr = json.Value<string>();
return new PackageDependency(
property,
versionStr == null ? null : VersionRange.Parse(versionStr));
versionStr == null ? null : JsonUtility.ParseVersionRange(versionStr));
}

internal static bool UseNewstonSoftJsonForParsing(IEnvironmentVariableReader environmentVariableReader, bool bypassCache)
{
if (!UseNewtonsoftJson.HasValue || bypassCache)
{
if (bool.TryParse(environmentVariableReader.GetEnvironmentVariable(NUGET_EXPERIMENTAL_USE_NJ_FOR_FILE_PARSING), out var useNj))
{
UseNewtonsoftJson = useNj;
}
else
{
UseNewtonsoftJson = false;
}
}

return UseNewtonsoftJson.Value;
}

internal static JProperty WritePackageDependencyWithLegacyString(PackageDependency item)
Expand Down Expand Up @@ -141,5 +170,36 @@ internal static JToken WriteString(string item)
{
return item != null ? new JValue(item) : JValue.CreateNull();
}

internal static NuGetVersion ParseNugetVersion(string value)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

#5529 (comment)

@davkean @nkolev92 continuing this conversation here. Would we want the cache of version/version range to live only during the parsing of the file?

Copy link
Member

Choose a reason for hiding this comment

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

I think defining a great lifetime for these caches would be challenging.

I'm comfortable if this change starts with the assets file read scope, but we should look into expanding it.

Copy link
Member

Choose a reason for hiding this comment

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

The title of this PR is to migrate to System.Text.Json, and the existing Newtonsoft.Json code doesn't do caching, so can you bump caching work to a different PR?

I'm behind on my other priorities, but ideally I should start work on NuGet/Home#12124 in the next sprint or two. For that work, I need this PR merged into the feature branch, and the feature branch merged into the dev branch.

If merging the assets file System.Text.Json work is delayed, then that will delay my ability to start on this other work. Therefore, I'd like to minimize scope creep that blocks other work.

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 love that, so I went ahead and undid the caching stuff and saved a stash of it.

{
if (!NuGetVersionCache.ContainsKey(value))
{
var result = NuGetVersion.Parse(value);
NuGetVersionCache[value] = result;
}
return NuGetVersionCache[value];
}

internal static bool TryParseNugetVersion(string value, out NuGetVersion version)
{
if (!NuGetVersionCache.ContainsKey(value))
{
_ = NuGetVersion.TryParse(value, out version);
NuGetVersionCache[value] = version;
}
version = NuGetVersionCache[value];
return version is not null;
}

internal static VersionRange ParseVersionRange(string value)
{
if (!VersionRangeCache.ContainsKey(value))
{
var result = VersionRange.Parse(value);
VersionRangeCache[value] = result;
}
return VersionRangeCache[value];
}
}
}
Loading