Skip to content

Commit

Permalink
Replace JObjects in PackageMetadataResourceV3 with strong type for be…
Browse files Browse the repository at this point in the history
…tter performance, lower memory consumption (#3462)

* No need dictionary, same thing can be with HashSet which takes lower overhead and space.

* Replacing slugish JsonObject with strong type.

* Revert back unneccessary change.

* Add descriptions for new methods added.

* Add back MetadataReferenceCache cache just in case.

* Replace get single package version method too.

* Minor code readability change.

* Improve MetadataReferenceCache implementation.

* Address code comment by Donnie.

* Address after Andy's code review.

* Address more code review comments.

* Git push minor comment text change.

* Address code review by Nkolche

* Fix space

* Address more code review comments.

* Address comment by Andy.

* Address more comments by Nkolche.

* Address new comment by Andy.

* Address another comment by Andy for ValueTuple.

* Forgot to delete file RegistrationIndexResult.cs which no longer need.

Co-authored-by: Erick Yondon <eryondon@microsoft.com>
  • Loading branch information
Erdembayar Yondon and erdembayar authored Jun 27, 2020
1 parent 84554f6 commit 7532d57
Show file tree
Hide file tree
Showing 10 changed files with 363 additions and 46 deletions.
2 changes: 1 addition & 1 deletion src/NuGet.Core/NuGet.Protocol/Converters/JsonExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ public static class JsonExtensions
},
};

private static readonly JsonSerializer JsonObjectSerializer = JsonSerializer.Create(ObjectSerializationSettings);
internal static readonly JsonSerializer JsonObjectSerializer = JsonSerializer.Create(ObjectSerializationSettings);

/// <summary>
/// Serialize object to the JSON.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,13 @@

using System;
using System.Collections.Generic;
using System.Globalization;
using System.IO;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
using Newtonsoft.Json.Linq;
using NuGet.Common;
using NuGet.Protocol.Core.Types;
using NuGet.Protocol.Extensions;
using NuGet.Versioning;

namespace NuGet.Protocol
Expand Down Expand Up @@ -70,7 +69,7 @@ public async static Task<IEnumerable<JObject>> LoadRanges(
var lower = NuGetVersion.Parse(item["lower"].ToString());
var upper = NuGetVersion.Parse(item["upper"].ToString());

if (IsItemRangeRequired(range, lower, upper))
if (range.DoesRangeSatisfy(lower, upper))
{
JToken items;
if (!item.TryGetValue("items", out items))
Expand Down Expand Up @@ -103,20 +102,5 @@ public async static Task<IEnumerable<JObject>> LoadRanges(

return rangeTasks.Select((t) => t.Result);
}

private static bool IsItemRangeRequired(VersionRange dependencyRange, NuGetVersion catalogItemLower, NuGetVersion catalogItemUpper)
{
var catalogItemVersionRange = new VersionRange(minVersion: catalogItemLower, includeMinVersion: true,
maxVersion: catalogItemUpper, includeMaxVersion: true);

if (dependencyRange.HasLowerAndUpperBounds) // Mainly to cover the '!dependencyRange.IsMaxInclusive && !dependencyRange.IsMinInclusive' case
{
return catalogItemVersionRange.Satisfies(dependencyRange.MinVersion) || catalogItemVersionRange.Satisfies(dependencyRange.MaxVersion);
}
else
{
return dependencyRange.Satisfies(catalogItemLower) || dependencyRange.Satisfies(catalogItemUpper);
}
}
}
}
25 changes: 25 additions & 0 deletions src/NuGet.Core/NuGet.Protocol/Extensions/VersionRangeExtensions.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using NuGet.Versioning;

namespace NuGet.Protocol.Extensions
{
internal static class VersionRangeExtensions
{
public static bool DoesRangeSatisfy(this VersionRange dependencyRange, NuGetVersion catalogItemLower, NuGetVersion catalogItemUpper)
{
if (dependencyRange.HasLowerAndUpperBounds) // Mainly to cover the '!dependencyRange.IsMaxInclusive && !dependencyRange.IsMinInclusive' case
{
var catalogItemVersionRange = new VersionRange(minVersion: catalogItemLower, includeMinVersion: true,
maxVersion: catalogItemUpper, includeMaxVersion: true);

return catalogItemVersionRange.Satisfies(dependencyRange.MinVersion) || catalogItemVersionRange.Satisfies(dependencyRange.MaxVersion);
}
else
{
return dependencyRange.Satisfies(catalogItemLower) || dependencyRange.Satisfies(catalogItemUpper);
}
}
}
}
2 changes: 2 additions & 0 deletions src/NuGet.Core/NuGet.Protocol/GlobalSuppressions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -301,3 +301,5 @@
[assembly: SuppressMessage("Build", "CA1001:Type 'ServiceIndexResourceV3Provider' owns disposable field(s) '_semaphore' but is not disposable", Justification = "<Pending>", Scope = "type", Target = "~T:NuGet.Protocol.ServiceIndexResourceV3Provider")]
[assembly: SuppressMessage("Build", "CA1052:Type 'V2FeedUtilities' is a static holder type but is neither static nor NotInheritable", Justification = "<Pending>", Scope = "type", Target = "~T:NuGet.Protocol.V2FeedUtilities")]
[assembly: SuppressMessage("Globalization", "CA1308:Normalize strings to uppercase", Justification = "We intentionally making to lower case because we're creating web api query.", Scope = "member", Target = "~M:NuGet.Protocol.PackageSearchResourceV3.SearchPage``1(System.Func{System.Uri,System.Threading.Tasks.Task{``0}},System.String,NuGet.Protocol.Core.Types.SearchFilter,System.Int32,System.Int32,NuGet.Common.ILogger,System.Threading.CancellationToken)~System.Threading.Tasks.Task{``0}")]
[assembly: SuppressMessage("Globalization", "CA1308:Normalize strings to uppercase", Justification = "We convert packageId to lower case for api call.", Scope = "member", Target = "~M:NuGet.Protocol.PackageMetadataResourceV3.GetRegistratioIndexPageAsync(NuGet.Protocol.HttpSource,System.String,System.String,NuGet.Versioning.NuGetVersion,NuGet.Versioning.NuGetVersion,NuGet.Protocol.Core.Types.HttpSourceCacheContext,NuGet.Common.ILogger,System.Threading.CancellationToken)~System.Threading.Tasks.Task{NuGet.Protocol.Model.RegistrationPage}")]
[assembly: SuppressMessage("Globalization", "CA1308:Normalize strings to uppercase", Justification = "We convert packageId to lower case for api call.", Scope = "member", Target = "~M:NuGet.Protocol.PackageMetadataResourceV3.LoadRegistrationIndexAsync(NuGet.Protocol.HttpSource,System.Uri,System.String,NuGet.Protocol.Core.Types.SourceCacheContext,System.Func{NuGet.Protocol.HttpSourceResult,System.Threading.Tasks.Task{NuGet.Protocol.Model.RegistrationIndex}},NuGet.Common.ILogger,System.Threading.CancellationToken)~System.Threading.Tasks.Task{System.ValueTuple{NuGet.Protocol.Model.RegistrationIndex,NuGet.Protocol.Core.Types.HttpSourceCacheContext}}")]
17 changes: 17 additions & 0 deletions src/NuGet.Core/NuGet.Protocol/Model/RegistrationIndex.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System.Collections.Generic;
using Newtonsoft.Json;

namespace NuGet.Protocol.Model
{
/// <summary>
/// Source: https://docs.microsoft.com/en-us/nuget/api/registration-base-url-resource#registration-index
/// </summary>
internal class RegistrationIndex
{
[JsonProperty("items")]
public List<RegistrationPage> Items { get; set; }
}
}
16 changes: 16 additions & 0 deletions src/NuGet.Core/NuGet.Protocol/Model/RegistrationLeafItem.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using Newtonsoft.Json;

namespace NuGet.Protocol.Model
{
/// <summary>
/// Source: https://docs.microsoft.com/en-us/nuget/api/registration-base-url-resource#registration-leaf-object-in-a-page
/// </summary>
internal class RegistrationLeafItem
{
[JsonProperty("catalogEntry")]
public PackageSearchMetadataRegistration CatalogEntry { get; set; }
}
}
33 changes: 33 additions & 0 deletions src/NuGet.Core/NuGet.Protocol/Model/RegistrationPage.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System.Collections.Generic;
using Newtonsoft.Json;
namespace NuGet.Protocol.Model
{
/// <summary>
/// This model is used for both the registration page item (found in a registration index) and for a registration
/// page fetched on its own.
/// Source: https://docs.microsoft.com/en-us/nuget/api/registration-base-url-resource#registration-page
/// Source: https://docs.microsoft.com/en-us/nuget/api/registration-base-url-resource#registration-page-object
/// </summary>
internal class RegistrationPage
{
[JsonProperty("@id")]
public string Url { get; set; }

/// <summary>
/// This property can be null when this model is used as an item in <see cref="RegistrationIndex.Items"/> when
/// the server decided not to inline the leaf items. In this case, the <see cref="Url"/> property can be used
/// fetch another <see cref="RegistrationPage"/> instance with the <see cref="Items"/> property filled in.
/// </summary>
[JsonProperty("items")]
public List<RegistrationLeafItem> Items { get; set; }

[JsonProperty("lower")]
public string Lower { get; set; }

[JsonProperty("upper")]
public string Upper { get; set; }
}
}
Loading

0 comments on commit 7532d57

Please sign in to comment.