Fixed policy cache to handle metadata

Author: sunghwan2789

src/HotChocolate/AspNetCore/src/AspNetCore.Authorization/AuthorizationPolicyCache.cs

@@ -6,19 +6,15 @@ namespace HotChocolate.AspNetCore.Authorization;
 
 internal sealed class AuthorizationPolicyCache
 {
-    private readonly ConcurrentDictionary<string, AuthorizationPolicy> _cache = new();
+    private readonly ConcurrentDictionary<AuthorizeDirective, AuthorizationPolicy> _cache = new();
 
     public AuthorizationPolicy? LookupPolicy(AuthorizeDirective directive)
     {
-        var cacheKey = directive.GetPolicyCacheKey();
-
-        return _cache.GetValueOrDefault(cacheKey);
+        return _cache.GetValueOrDefault(directive);
     }
 
     public void CachePolicy(AuthorizeDirective directive, AuthorizationPolicy policy)
     {
-        var cacheKey = directive.GetPolicyCacheKey();
-
-        _cache.TryAdd(cacheKey, policy);
+        _cache.TryAdd(directive, policy);
     }
 }

src/HotChocolate/AspNetCore/test/AspNetCore.Authorization.Tests/AuthorizeDirectiveTests.cs

@@ -80,105 +80,6 @@ public void CreateInstance_Roles_RolesHasItems()
             t => Assert.Equal("b", t));
     }
 
-    [Fact]
-    public void CacheKey_Policy_NoRoles()
-    {
-        // arrange
-        var authorizeDirective = new AuthorizeDirective(
-            policy: "policy");
-
-        // act
-        var cacheKey = authorizeDirective.GetPolicyCacheKey();
-
-        // assert
-        Assert.Equal("policy;", cacheKey);
-    }
-
-    [Fact]
-    public void CacheKey_NoPolicy_Roles()
-    {
-        // arrange
-        var authorizeDirective = new AuthorizeDirective(
-            policy: null,
-            roles: ["a", "b"]);
-
-        // act
-        var cacheKey = authorizeDirective.GetPolicyCacheKey();
-
-        // assert
-        Assert.Equal(";a,b", cacheKey);
-    }
-
-    [Fact]
-    public void CacheKey_Policy_And_Roles()
-    {
-        // arrange
-        var authorizeDirective = new AuthorizeDirective(
-            policy: "policy",
-            roles: ["a", "b"]);
-
-        // act
-        var cacheKey = authorizeDirective.GetPolicyCacheKey();
-
-        // assert
-        Assert.Equal("policy;a,b", cacheKey);
-    }
-
-    [Fact]
-    public void CacheKey_NoPolicy_NoRoles()
-    {
-        // arrange
-        var authorizeDirective = new AuthorizeDirective(
-            policy: null,
-            roles: null);
-
-        // act
-        var cacheKey = authorizeDirective.GetPolicyCacheKey();
-
-        // assert
-        Assert.Equal("", cacheKey);
-    }
-
-    [Fact]
-    public void CacheKey_Policy_And_Role_Naming_Does_Not_Conflict()
-    {
-        // arrange
-        var authorizeDirective1 = new AuthorizeDirective(
-            policy: "policy",
-            roles: null);
-
-        var authorizeDirective2 = new AuthorizeDirective(
-            policy: null,
-            roles: ["policy"]);
-
-        // act
-        var cacheKey1 = authorizeDirective1.GetPolicyCacheKey();
-        var cacheKey2 = authorizeDirective2.GetPolicyCacheKey();
-
-        // assert
-        Assert.NotEqual(cacheKey1, cacheKey2);
-    }
-
-    [Fact]
-    public void CacheKey_Same_Roles_Albeit_Sorted_Differently_Have_Same_Cache_Key()
-    {
-        // arrange
-        var authorizeDirective1 = new AuthorizeDirective(
-            policy: null,
-            roles: ["a", "c", "b"]);
-
-        var authorizeDirective2 = new AuthorizeDirective(
-            policy: null,
-            roles: ["c", "b", "a"]);
-
-        // act
-        var cacheKey1 = authorizeDirective1.GetPolicyCacheKey();
-        var cacheKey2 = authorizeDirective2.GetPolicyCacheKey();
-
-        // assert
-        Assert.Equal(cacheKey1, cacheKey2);
-    }
-
     [Fact]
     public void TypeAuth_DefaultPolicy()
     {

src/HotChocolate/Core/src/Authorization/AuthorizeDirective.cs

@@ -1,14 +1,10 @@
-using System.Text;
-
 namespace HotChocolate.Authorization;
 
 /// <summary>
 /// The authorize directive.
 /// </summary>
 public sealed class AuthorizeDirective
 {
-    private readonly string _cacheKey;
-
     /// <summary>
     /// Initializes a new instance of <see cref="AuthorizeDirective"/>.
     /// </summary>
@@ -57,8 +53,6 @@ public AuthorizeDirective(
         Roles = roles?.OrderBy(r => r).ToList();
         Apply = apply;
         Metadata = metadata;
-
-        _cacheKey = BuildCacheKey(Policy, Roles);
     }
 
     /// <summary>
@@ -88,42 +82,4 @@ public AuthorizeDirective(
     /// Gets the metadata that can be used for authorization.
     /// </summary>
     public IReadOnlyList<object>? Metadata { get; }
-
-    /// <summary>
-    /// Gets a cache key that uniquely identifies the combined authorization policy,
-    /// of the specified <see cref="Roles"/> and <see cref="Policy"/>.
-    /// </summary>
-    internal string GetPolicyCacheKey() => _cacheKey;
-
-    private static string BuildCacheKey(string? policy, IReadOnlyList<string>? roles)
-    {
-        if (string.IsNullOrEmpty(policy) && roles is null)
-        {
-            return string.Empty;
-        }
-
-        var sb = new StringBuilder();
-
-        if (!string.IsNullOrEmpty(policy))
-        {
-            sb.Append(policy);
-        }
-
-        sb.Append(";");
-
-        if (roles is not null)
-        {
-            for (var i = 0; i < roles.Count; i++)
-            {
-                sb.Append(roles[i]);
-
-                if (i < roles.Count - 1)
-                {
-                    sb.Append(",");
-                }
-            }
-        }
-
-        return sb.ToString();
-    }
 }