Skip to content

Commit

Permalink
Migrate .nupkg.metadata read/write to System.Text.Json (#6083)
Browse files Browse the repository at this point in the history
  • Loading branch information
@zivkan
zivkan authored Oct 10, 2024
1 parent 7c46398 commit def41b0
Show file tree
Hide file tree
Showing 3 changed files with 79 additions and 63 deletions.
1 change: 1 addition & 0 deletions src/NuGet.Core/NuGet.Packaging/NuGet.Packaging.csproj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
<ItemGroup>
<PackageReference Include="Newtonsoft.Json" />
<PackageReference Include="System.Memory" Condition="'$(TargetFramework)' == '$(NETFXTargetFramework)'" />
<PackageReference Include="System.Text.Json" Condition="'$(TargetFrameworkIdentifier)' != '.NETCoreApp'" />
</ItemGroup>

<ItemGroup Condition="'$(TargetFramework)' != '$(NETFXTargetFramework)'">
Expand Down
57 changes: 47 additions & 10 deletions src/NuGet.Core/NuGet.Packaging/NupkgMetadata/NupkgMetadataFileFormat.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
using System;
using System.Globalization;
using System.IO;
using System.Text.Json;
using System.Text.Json.Serialization;
using Newtonsoft.Json;
using NuGet.Common;

Expand All @@ -17,7 +19,17 @@ public static class NupkgMetadataFileFormat
private const string HashProperty = "contentHash";
private const string SourceProperty = "source";

private static readonly JsonSerializer JsonSerializer = JsonSerializer.Create(GetSerializerSettings());
private static readonly Newtonsoft.Json.JsonSerializer JsonSerializer = Newtonsoft.Json.JsonSerializer.Create(GetSerializerSettings());
private static readonly JsonSerializerOptions JsonSerializerOptions = new JsonSerializerOptions()
{
WriteIndented = true,
PropertyNameCaseInsensitive = true,
PropertyNamingPolicy = JsonNamingPolicy.CamelCase,
// This file is not intended to be parsed in javascript, and the contentHash property contains a Base64 encoded string that
// can contain characters such as + that has not been encoded in the past.
Encoder = System.Text.Encodings.Web.JavaScriptEncoder.UnsafeRelaxedJsonEscaping,
DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull,
};

public static NupkgMetadataFile Read(string filePath)
{
Expand All @@ -31,12 +43,37 @@ public static NupkgMetadataFile Read(string filePath, ILogger log)

public static NupkgMetadataFile Read(Stream stream, ILogger log, string path)
{
using (var textReader = new StreamReader(stream))
if (stream is null) { throw new ArgumentNullException(nameof(stream)); }

try
{
return Read(textReader, log, path);
NupkgMetadataFile nupkgMetadata = System.Text.Json.JsonSerializer.Deserialize<NupkgMetadataFile>(stream, JsonSerializerOptions);
if (nupkgMetadata == null)
{
throw new InvalidDataException();
}
return nupkgMetadata;
}
catch (System.Text.Json.JsonException ex)
{
LogMessage(log, path, ex);
throw new InvalidDataException(ex.Message, ex);
}
catch (Exception ex)
{
LogMessage(log, path, ex);
throw;
}

static void LogMessage(ILogger log, string path, Exception ex)
{
log.LogWarning(string.Format(CultureInfo.CurrentCulture,
Strings.Error_LoadingHashFile,
path, ex.Message));
}
}

[Obsolete("Use a different overload for better performance. This overload will get deleted in the future.")]
public static NupkgMetadataFile Read(TextReader reader, ILogger log, string path)
{
try
Expand Down Expand Up @@ -75,12 +112,12 @@ public static void Write(string filePath, NupkgMetadataFile hashFile)

public static void Write(Stream stream, NupkgMetadataFile hashFile)
{
using (var textWriter = new StreamWriter(stream))
{
Write(textWriter, hashFile);
}
if (stream is null) { throw new ArgumentNullException(nameof(stream)); }

System.Text.Json.JsonSerializer.Serialize(stream, hashFile, JsonSerializerOptions);
}

[Obsolete("Use a different overload for better performance. This overload will get deleted in the future.")]
public static void Write(TextWriter textWriter, NupkgMetadataFile hashFile)
{
using (var jsonWriter = new JsonTextWriter(textWriter))
Expand All @@ -101,7 +138,7 @@ private static JsonSerializerSettings GetSerializerSettings()
return settings;
}

private class NupkgMetadataConverter : JsonConverter
private class NupkgMetadataConverter : Newtonsoft.Json.JsonConverter
{
internal static NupkgMetadataConverter Default { get; } = new NupkgMetadataConverter();

Expand All @@ -111,7 +148,7 @@ public override bool CanConvert(Type objectType)
return objectType == TargetType;
}

public override object ReadJson(JsonReader reader, Type objectType, object existingValue, JsonSerializer serializer)
public override object ReadJson(JsonReader reader, Type objectType, object existingValue, Newtonsoft.Json.JsonSerializer serializer)
{
NupkgMetadataFile nupkgMetadataFile = existingValue as NupkgMetadataFile;
if (nupkgMetadataFile == null)
Expand Down Expand Up @@ -161,7 +198,7 @@ public override object ReadJson(JsonReader reader, Type objectType, object exist
throw new JsonReaderException();
}

public override void WriteJson(JsonWriter writer, object value, JsonSerializer serializer)
public override void WriteJson(JsonWriter writer, object value, Newtonsoft.Json.JsonSerializer serializer)
{
if (!(value is NupkgMetadataFile nupkgMetadataFile))
{
Expand Down
84 changes: 31 additions & 53 deletions test/NuGet.Core.Tests/NuGet.Packaging.Test/NupkgMetadataFileFormatTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System.IO;
using Newtonsoft.Json;
using Newtonsoft.Json.Linq;
using System.Text;
using NuGet.Common;
using NuGet.Test.Utility;
using Xunit;
Expand All @@ -22,11 +21,7 @@ public void Read_V1FileContents_ReturnsCorrectObject()
}";

// Act
NupkgMetadataFile file;
using (var reader = new StringReader(nupkgMetadataFileContent))
{
file = NupkgMetadataFileFormat.Read(reader, NullLogger.Instance, string.Empty);
}
NupkgMetadataFile file = Read(nupkgMetadataFileContent, NullLogger.Instance, string.Empty);

// Assert
Assert.NotNull(file);
Expand All @@ -46,11 +41,7 @@ public void Read_V2FileContents_ReturnsCorrectObject()
}";

// Act
NupkgMetadataFile file;
using (var reader = new StringReader(nupkgMetadataFileContent))
{
file = NupkgMetadataFileFormat.Read(reader, NullLogger.Instance, string.Empty);
}
NupkgMetadataFile file = Read(nupkgMetadataFileContent, NullLogger.Instance, string.Empty);

// Assert
Assert.NotNull(file);
Expand All @@ -63,58 +54,39 @@ public void Read_V2FileContents_ReturnsCorrectObject()
public void Write_WithObject_RoundTrips()
{
var nupkgMetadataFileContent = @"{
""version"": 2,
""contentHash"": ""NhfNp80eWq5ms7fMrjuRqpwhL1H56IVzXF9d+OIDcEfQ92m1DyE0c+ufUE1ogB09+sYLd58IO4eJ8jyn7AifbA=="",
""source"": ""https://source/v3/index.json""
}";
""version"": 2,
""contentHash"": ""NhfNp80eWq5ms7fMrjuRqpwhL1H56IVzXF9d+OIDcEfQ92m1DyE0c+ufUE1ogB09+sYLd58IO4eJ8jyn7AifbA=="",
""source"": ""https://source/v3/index.json""
}";

NupkgMetadataFile metadataFile = null;
NupkgMetadataFile metadataFile = Read(nupkgMetadataFileContent, NullLogger.Instance, string.Empty);

using (var reader = new StringReader(nupkgMetadataFileContent))
using (var stream = new MemoryStream())
{
metadataFile = NupkgMetadataFileFormat.Read(reader, NullLogger.Instance, string.Empty);
}

using (var writer = new StringWriter())
{
NupkgMetadataFileFormat.Write(writer, metadataFile);
var output = JObject.Parse(writer.ToString());
var expected = JObject.Parse(nupkgMetadataFileContent);

Assert.Equal(expected.ToString(), output.ToString());
NupkgMetadataFileFormat.Write(stream, metadataFile);
stream.Position = 0;
string content;
using (var reader = new StreamReader(stream))
{
content = reader.ReadToEnd();
}

Assert.Equal(nupkgMetadataFileContent, content);
}
}

[Theory]
[InlineData("")]
[InlineData("1")]
[InlineData("[]")]
public void Read_ContentsNotAnObject_ThrowsException(string contents)
{
// Arrange
var logger = new TestLogger();

// Act
using (var stringReader = new StringReader(contents))
{
Assert.Throws<InvalidDataException>(() => NupkgMetadataFileFormat.Read(stringReader, logger, "from memory"));
}

// Assert
Assert.Equal(1, logger.Messages.Count);
}

[Fact]
public void Read_ContentsInvalidJson_ThrowsJsonReaderException()
[InlineData(@"{""version"":")]
public void Read_ContentsInvalid_ThrowsException(string contents)
{
// Arrange
var logger = new TestLogger();

// Act
using (var stringReader = new StringReader(@"{""version"":"))
{
Assert.Throws<JsonReaderException>(() => NupkgMetadataFileFormat.Read(stringReader, logger, "from memory"));
}
Assert.Throws<InvalidDataException>(() => Read(contents, logger, "from memory"));

// Assert
Assert.Equal(1, logger.Messages.Count);
Expand All @@ -135,13 +107,19 @@ public void Read_ContentsWithUnexpectedValues_IgnoresUnexpectedValues()
}";

// Act
using (var stringReader = new StringReader(nupkgMetadataFileContent))
{
NupkgMetadataFileFormat.Read(stringReader, logger, "from memory");
}
Read(nupkgMetadataFileContent, logger, "from memory");

// Assert
Assert.Equal(0, logger.Messages.Count);
}

private NupkgMetadataFile Read(string contents, ILogger logger, string path)
{
byte[] utf8Bytes = Encoding.UTF8.GetBytes(contents);
using (var stream = new MemoryStream(utf8Bytes))
{
return NupkgMetadataFileFormat.Read(stream, logger, path);
}
}
}
}

0 comments on commit def41b0

Please sign in to comment.