Skip to content

Commit 21c6db6

Browse files
Nigusu-AllehuNigusu-Allehu
authored
AOT compatible: enable in NuGet.Protocol (#7193)
1 parent 105e1d3 commit 21c6db6

12 files changed

Lines changed: 128 additions & 183 deletions

‎src/NuGet.Core/NuGet.Protocol/LocalRepositories/LocalPackageMetadataResource.cs‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,11 +40,9 @@ public override Task<IEnumerable<IPackageSearchMetadata>> GetMetadataAsync(
4040

4141
return Task.Run<IEnumerable<IPackageSearchMetadata>>(() =>
4242
{
43-
var metadataCache = new MetadataReferenceCache();
4443
return _localResource.FindPackagesById(packageId, log, token)
4544
.Where(p => includePrerelease || !p.Identity.Version.IsPrerelease)
4645
.Select(GetPackageMetadata)
47-
.Select(p => metadataCache.GetObject(p))
4846
.ToList();
4947
},
5048
token);

‎src/NuGet.Core/NuGet.Protocol/Model/PackageSearchMetadata.cs‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -256,5 +256,18 @@ private static IList<string> GetNonStandardLicenseIdentifiers(NuGetLicenseExpres
256256
/// <inheritdoc cref="IPackageSearchMetadata.Vulnerabilities" />
257257
[JsonProperty(PropertyName = JsonProperties.Vulnerabilities)]
258258
public IEnumerable<PackageVulnerabilityMetadata> Vulnerabilities { get; private set; }
259+
260+
internal void CacheStrings(MetadataReferenceCache cache)
261+
{
262+
Authors = cache.GetString(Authors);
263+
Description = cache.GetString(Description);
264+
PackageId = cache.GetString(PackageId);
265+
ReadmeFileUrl = cache.GetString(ReadmeFileUrl);
266+
Tags = cache.GetString(Tags);
267+
Summary = cache.GetString(Summary);
268+
Title = cache.GetString(Title);
269+
LicenseExpression = cache.GetString(LicenseExpression);
270+
LicenseExpressionVersion = cache.GetString(LicenseExpressionVersion);
271+
}
259272
}
260273
}

‎src/NuGet.Core/NuGet.Protocol/Model/PackageSearchMetadataBuilder.cs‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,18 @@ public class ClonedPackageSearchMetadata : IPackageSearchMetadata
6262
[Obsolete("PackagePath is recommended in place of PackageReader")]
6363
public Func<PackageReaderBase> PackageReader { get; set; }
6464
public string PackagePath { get; set; }
65+
66+
internal void CacheStrings(MetadataReferenceCache cache)
67+
{
68+
Authors = cache.GetString(Authors);
69+
Description = cache.GetString(Description);
70+
Owners = cache.GetString(Owners);
71+
ReadmeFileUrl = cache.GetString(ReadmeFileUrl);
72+
Summary = cache.GetString(Summary);
73+
Tags = cache.GetString(Tags);
74+
Title = cache.GetString(Title);
75+
PackagePath = cache.GetString(PackagePath);
76+
}
6577
}
6678

6779
private PackageSearchMetadataBuilder(IPackageSearchMetadata metadata)

‎src/NuGet.Core/NuGet.Protocol/NuGet.Protocol.csproj‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010
<XPLATProject>true</XPLATProject>
1111
<Description>NuGet's implementation for interacting with feeds. Contains functionality for all feed types.</Description>
1212
<UsePublicApiAnalyzer>perTfm</UsePublicApiAnalyzer>
13-
<IsAotCompatible>false</IsAotCompatible>
1413
</PropertyGroup>
1514

1615
<PropertyGroup Condition=" '$(IsVsixBuild)' == 'true' ">

‎src/NuGet.Core/NuGet.Protocol/Resources/PackageMetadataResourceV3.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -286,7 +286,7 @@ private void ProcessRegistrationPage(
286286
{
287287
catalogEntry.ReadmeFileUrl = _readmeUriTemplateResource?.GetReadmeUrl(catalogEntry.PackageId, catalogEntry.Version);
288288
}
289-
catalogEntry = metadataCache.GetObject(catalogEntry);
289+
catalogEntry.CacheStrings(metadataCache);
290290
results.Add(catalogEntry);
291291
}
292292
}

‎src/NuGet.Core/NuGet.Protocol/Resources/PackageSearchResourceV3.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ public override async Task<IEnumerable<IPackageSearchMetadata>> SearchAsync(stri
7777

7878
var searchResults = searchResultMetadata
7979
.Select(m => m.WithVersions(() => GetVersions(m, filter)))
80-
.Select(m => metadataCache.GetObject((PackageSearchMetadataBuilder.ClonedPackageSearchMetadata)m))
80+
.Select(m => { ((PackageSearchMetadataBuilder.ClonedPackageSearchMetadata)m).CacheStrings(metadataCache); return m; })
8181
.ToArray();
8282

8383
return searchResults;

‎src/NuGet.Core/NuGet.Protocol/Utility/MetadataReferenceCache.cs‎

Lines changed: 3 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,6 @@
55

66
using System;
77
using System.Collections.Generic;
8-
using System.Linq;
9-
using System.Reflection;
108
using NuGet.Versioning;
119

1210
namespace NuGet.Protocol
@@ -17,9 +15,7 @@ namespace NuGet.Protocol
1715
public class MetadataReferenceCache
1816
{
1917
private readonly Dictionary<string, string> _stringCache = new Dictionary<string, string>(StringComparer.Ordinal);
20-
private readonly Dictionary<Type, PropertyInfo[]> _propertyCache = new Dictionary<Type, PropertyInfo[]>();
2118
private readonly Dictionary<string, NuGetVersion> _versionCache = new Dictionary<string, NuGetVersion>(StringComparer.Ordinal);
22-
private readonly Type _metadataReferenceCacheType = typeof(MetadataReferenceCache);
2319

2420
/// <summary>
2521
/// Checks if <paramref name="s"/> already exists in the cache.
@@ -70,72 +66,12 @@ public NuGetVersion GetVersion(string s)
7066
}
7167

7268
/// <summary>
73-
/// Mapping of input parameter type to caching method.
74-
/// </summary>
75-
private static readonly IDictionary<Type, string> CachableTypesMap = new Dictionary<Type, string>
76-
{
77-
{typeof(string), nameof(GetString)}
78-
};
79-
80-
/// <summary>
81-
/// <see cref="IEnumerable{Type}"/> containing all types that can be cached.
82-
/// </summary>
83-
internal static IEnumerable<Type> CachableTypes => CachableTypesMap.Keys;
84-
85-
/// <summary>
86-
/// <see cref="IEnumerable{Type}"/> containing string type methods can be cached.
87-
/// </summary>
88-
internal Dictionary<Type, MethodInfo> CachableMethodTypes { get; } = new Dictionary<Type, MethodInfo>();
89-
90-
/// <summary>
91-
/// Iterates through the properties of <paramref name="input"/> that are either <see cref="string"/>s, <see cref="DateTimeOffset"/>s, or <see cref="NuGetVersion"/>s and checks them against the cache.
69+
/// This method is a no-op retained for API compatibility.
9270
/// </summary>
71+
#pragma warning disable CA1822 // Member does not access instance data - retained as instance method for API compatibility
9372
public T GetObject<T>(T input)
73+
#pragma warning restore CA1822
9474
{
95-
// Get all properties that contain both a Get method and a Set method and can be cached.
96-
PropertyInfo[] properties;
97-
Type typeKey = typeof(T);
98-
99-
if (!_propertyCache.TryGetValue(typeKey, out properties))
100-
{
101-
properties = typeKey.GetTypeInfo()
102-
.DeclaredProperties.Where(
103-
p => CachableTypesMap.ContainsKey(p.PropertyType) && p.GetMethod != null && p.SetMethod != null)
104-
.ToArray();
105-
106-
_propertyCache.Add(typeKey, properties);
107-
}
108-
109-
if (!CachableMethodTypes.ContainsKey(typeof(MetadataReferenceCache)))
110-
{
111-
// Doing reflection everytime is expensive so cache it for string type which is all this MetadataReferenceCache about.
112-
Type stringPropertyType = typeof(string);
113-
MethodInfo method = _metadataReferenceCacheType.GetTypeInfo()
114-
.DeclaredMethods.FirstOrDefault(
115-
m =>
116-
m.Name == CachableTypesMap[stringPropertyType] &&
117-
m.GetParameters().Select(p => p.ParameterType).SequenceEqual(new Type[] { stringPropertyType }));
118-
CachableMethodTypes.Add(_metadataReferenceCacheType, method);
119-
}
120-
121-
for (var i = 0; i < properties.Length; i++)
122-
{
123-
PropertyInfo property = properties[i];
124-
object value = property.GetMethod.Invoke(input, null);
125-
126-
object cachedValue = property.PropertyType == typeof(string) ?
127-
CachableMethodTypes[_metadataReferenceCacheType]
128-
.Invoke(this, new[] { value })
129-
:
130-
typeof(MetadataReferenceCache).GetTypeInfo()
131-
.DeclaredMethods.FirstOrDefault(
132-
m =>
133-
m.Name == CachableTypesMap[property.PropertyType] &&
134-
m.GetParameters().Select(p => p.ParameterType).SequenceEqual(new Type[] { property.PropertyType }))
135-
.Invoke(this, new[] { value });
136-
property.SetMethod.Invoke(input, new[] { cachedValue });
137-
}
138-
13975
return input;
14076
}
14177
}

‎test/NuGet.Core.Tests/NuGet.Protocol.Tests/Core/Types/PackageSearchMetadataBuilderTests.cs‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
11
// Copyright (c) .NET Foundation. All rights reserved.
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

4+
using System.Linq;
5+
using System.Reflection;
6+
using System.Text;
7+
using FluentAssertions;
48
using NuGet.Test.Utility;
59
using Xunit;
610

@@ -34,5 +38,44 @@ public void LocalPackageInfo_NotNull()
3438
Assert.NotNull(clone2.PackagePath);
3539
Assert.Equal(clone1.PackagePath, clone2.PackagePath);
3640
}
41+
42+
[Fact]
43+
public void CacheStrings_DeduplicatesStringsOnClonedMetadata()
44+
{
45+
// Arrange
46+
var cache = new MetadataReferenceCache();
47+
var metadata1 = new PackageSearchMetadataBuilder.ClonedPackageSearchMetadata
48+
{
49+
Authors = new StringBuilder().Append("Microsoft").ToString(),
50+
Description = "desc",
51+
};
52+
var metadata2 = new PackageSearchMetadataBuilder.ClonedPackageSearchMetadata
53+
{
54+
Authors = new StringBuilder().Append("Microsoft").ToString(),
55+
Description = "other",
56+
};
57+
Assert.NotSame(metadata1.Authors, metadata2.Authors);
58+
59+
// Act
60+
metadata1.CacheStrings(cache);
61+
metadata2.CacheStrings(cache);
62+
63+
// Assert
64+
Assert.Same(metadata1.Authors, metadata2.Authors);
65+
}
66+
67+
[Fact]
68+
public void Verify_AllStringProperties_AccountedInCacheStrings()
69+
{
70+
var stringProperties = typeof(PackageSearchMetadataBuilder.ClonedPackageSearchMetadata)
71+
.GetProperties(BindingFlags.Instance | BindingFlags.Public)
72+
.Where(p => p.PropertyType == typeof(string))
73+
.ToArray();
74+
75+
stringProperties.Should().HaveCount(8,
76+
$"the number of string properties changed in ClonedPackageSearchMetadata " +
77+
$"[{string.Join(", ", stringProperties.Select(p => p.Name))}]. " +
78+
"Please make sure this change is accounted for in the CacheStrings method");
79+
}
3780
}
3881
}

‎test/NuGet.Core.Tests/NuGet.Protocol.Tests/Model/PackageSearchMetadataTests.cs‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
// Copyright (c) .NET Foundation. All rights reserved.
2+
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
3+
4+
using System.Linq;
5+
using System.Reflection;
6+
using System.Text;
7+
using FluentAssertions;
8+
using Newtonsoft.Json.Linq;
9+
using Xunit;
10+
11+
namespace NuGet.Protocol.Tests
12+
{
13+
public class PackageSearchMetadataTests
14+
{
15+
[Fact]
16+
public void CacheStrings_DeduplicatesStrings()
17+
{
18+
// Arrange
19+
var cache = new MetadataReferenceCache();
20+
var authors1 = new StringBuilder().Append("Contoso").ToString();
21+
var authors2 = new StringBuilder().Append("Contoso").ToString();
22+
var metadata1 = new JObject { ["authors"] = authors1, ["description"] = "desc" }.FromJToken<PackageSearchMetadata>();
23+
var metadata2 = new JObject { ["authors"] = authors2, ["description"] = "other" }.FromJToken<PackageSearchMetadata>();
24+
Assert.NotSame(metadata1.Authors, metadata2.Authors);
25+
26+
// Act
27+
metadata1.CacheStrings(cache);
28+
metadata2.CacheStrings(cache);
29+
30+
// Assert
31+
Assert.Same(metadata1.Authors, metadata2.Authors);
32+
}
33+
34+
[Fact]
35+
public void Verify_AllStringProperties_AccountedInCacheStrings()
36+
{
37+
var stringProperties = typeof(PackageSearchMetadata)
38+
.GetProperties(BindingFlags.Instance | BindingFlags.Public)
39+
.Where(p => p.PropertyType == typeof(string))
40+
.ToArray();
41+
42+
stringProperties.Should().HaveCount(10,
43+
$"the number of string properties changed in PackageSearchMetadata " +
44+
$"[{string.Join(", ", stringProperties.Select(p => p.Name))}]. " +
45+
"Please make sure this change is accounted for in the CacheStrings method");
46+
}
47+
}
48+
}

‎test/NuGet.Core.Tests/NuGet.Protocol.Tests/PackageSearchResourceV3Tests.cs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -127,16 +127,16 @@ public async Task PackageSearchResourceV3_UsesReferenceCache()
127127
var resource = await repo.GetResourceAsync<PackageSearchResource>(CancellationToken.None);
128128

129129
// Act
130-
var packages = (IEnumerable<PackageSearchMetadataBuilder.ClonedPackageSearchMetadata>)await resource.SearchAsync(
130+
var packages = await resource.SearchAsync(
131131
"entityframework",
132132
new SearchFilter(false),
133133
skip: 0,
134134
take: 2,
135135
log: NullLogger.Instance,
136136
cancellationToken: CancellationToken.None);
137137

138-
var first = packages.ElementAt(0);
139-
var second = packages.ElementAt(1);
138+
var first = (PackageSearchMetadataBuilder.ClonedPackageSearchMetadata)packages.ElementAt(0);
139+
var second = (PackageSearchMetadataBuilder.ClonedPackageSearchMetadata)packages.ElementAt(1);
140140

141141
// Assert
142142
MetadataReferenceCacheTestUtility.AssertPackagesHaveSameReferences(first, second);

0 commit comments

Comments
 (0)