Hashcode implementation proposal for DataClassificationSet (#4933)

dotnet · Feb 15, 2024 · a8e1751 · a8e1751
1 parent 2a6e51e
commit a8e1751
Show file tree

Hide file tree

Showing 4 changed files with 94 additions and 5 deletions.
diff --git a/...ries/Microsoft.Extensions.Compliance.Abstractions/Classification/DataClassificationSet.cs b/...ries/Microsoft.Extensions.Compliance.Abstractions/Classification/DataClassificationSet.cs
@@ -15,6 +15,7 @@ namespace Microsoft.Extensions.Compliance.Classification;
 public sealed class DataClassificationSet : IEquatable<DataClassificationSet>
 {
     private readonly HashSet<DataClassification> _classifications = [];
+    private readonly int _cachedHashCode;
 
     /// <summary>
     /// Initializes a new instance of the <see cref="DataClassificationSet"/> class.
@@ -23,6 +24,7 @@ public sealed class DataClassificationSet : IEquatable<DataClassificationSet>
     public DataClassificationSet(DataClassification classification)
     {
         _ = _classifications.Add(classification);
+        _cachedHashCode = ComputeHashCode();
     }
 
     /// <summary>
@@ -33,6 +35,7 @@ public DataClassificationSet(IEnumerable<DataClassification> classifications)
     {
         _ = Throw.IfNull(classifications);
         _classifications.UnionWith(classifications);
+        _cachedHashCode = ComputeHashCode();
     }
 
     /// <summary>
@@ -43,6 +46,7 @@ public DataClassificationSet(params DataClassification[] classifications)
     {
         _ = Throw.IfNull(classifications);
         _classifications.UnionWith(classifications);
+        _cachedHashCode = ComputeHashCode();
     }
 
     /// <summary>
@@ -73,17 +77,17 @@ public DataClassificationSet Union(DataClassificationSet other)
     {
         _ = Throw.IfNull(other);
 
-        var result = new DataClassificationSet(other._classifications);
-        result._classifications.UnionWith(_classifications);
+        var combinedClassifications = new HashSet<DataClassification>(_classifications);
+        combinedClassifications.UnionWith(other._classifications);
 
-        return result;
+        return new DataClassificationSet(combinedClassifications);
     }
 
     /// <summary>
     /// Gets a hash code for the current object instance.
     /// </summary>
     /// <returns>The hash code value.</returns>
-    public override int GetHashCode() => _classifications.GetHashCode();
+    public override int GetHashCode() => _cachedHashCode;
 
     /// <summary>
     /// Compares an object with the current instance to see if they contain the same classifications.
@@ -131,4 +135,29 @@ public override string ToString()
 
         return result;
     }
+
+    /// <summary>
+    /// Computes the hash code for the current object instance.
+    /// </summary>
+    /// <returns>The computed hash code.</returns>
+    private int ComputeHashCode()
+    {
+#if NETFRAMEWORK
+        int hash = 0;
+        foreach (var item in _classifications)
+        {
+            hash ^= item.GetHashCode();
+        }
+
+        return hash;
+#else
+        var hash = default(HashCode);
+        foreach (var item in _classifications)
+        {
+            hash.Add(item);
+        }
+
+        return hash.ToHashCode();
+#endif
+    }
 }
diff --git a/test/Generators/Microsoft.Gen.Logging/Generated/Utils.cs b/test/Generators/Microsoft.Gen.Logging/Generated/Utils.cs
@@ -3,6 +3,7 @@
 
 using System;
 using System.Collections.Generic;
+using Microsoft.Extensions.Compliance.Classification;
 using Microsoft.Extensions.Compliance.Redaction;
 using Microsoft.Extensions.Compliance.Testing;
 using Microsoft.Extensions.DependencyInjection;
@@ -82,7 +83,7 @@ public static TestLogger GetLogger()
         {
             builder.SetRedactor<PlusRedactor>(new PublicDataAttribute().Classification);
             builder.SetRedactor<MinusRedactor>(new PrivateDataAttribute().Classification);
-            builder.SetRedactor<HashRedactor>(new PrivateDataAttribute().Classification, new PublicDataAttribute().Classification);
+            builder.SetRedactor<HashRedactor>(new DataClassificationSet(new PrivateDataAttribute().Classification, new PublicDataAttribute().Classification));
             builder.SetFallbackRedactor<StarRedactor>();
         });
 

diff --git a/...oft.Extensions.Compliance.Abstractions.Tests/Classification/DataClassificationSetTests.cs b/...oft.Extensions.Compliance.Abstractions.Tests/Classification/DataClassificationSetTests.cs
@@ -30,4 +30,22 @@ public static void Basic()
         Assert.False(dc1.Equals(null));
 #pragma warning restore CA1508 // Avoid dead conditional code
     }
+
+    [Fact]
+    public static void TestHashCodes()
+    {
+        var dc1 = new DataClassificationSet(FakeTaxonomy.PublicData);
+        var dc2 = new DataClassificationSet(new[] { FakeTaxonomy.PublicData });
+        var dc3 = new DataClassificationSet(new List<DataClassification> { FakeTaxonomy.PublicData });
+        var dc4 = (DataClassificationSet)FakeTaxonomy.PublicData;
+        var dc5 = DataClassificationSet.FromDataClassification(FakeTaxonomy.PublicData);
+
+        Assert.Equal(dc1.GetHashCode(), dc2.GetHashCode());
+        Assert.Equal(dc1.GetHashCode(), dc3.GetHashCode());
+        Assert.Equal(dc1.GetHashCode(), dc4.GetHashCode());
+        Assert.Equal(dc1.GetHashCode(), dc5.GetHashCode());
+
+        var dc6 = dc1.Union(FakeTaxonomy.PrivateData);
+        Assert.NotEqual(dc1, dc6);
+    }
 }
diff --git a/test/Libraries/Microsoft.Extensions.Compliance.Redaction.Tests/RedactorProviderTests.cs b/test/Libraries/Microsoft.Extensions.Compliance.Redaction.Tests/RedactorProviderTests.cs
@@ -3,6 +3,7 @@
 
 using System;
 using Microsoft.Extensions.Compliance.Classification;
+using Microsoft.Extensions.DependencyInjection;
 using Xunit;
 
 namespace Microsoft.Extensions.Compliance.Redaction.Test;
@@ -46,6 +47,46 @@ public void RedactorProvider_Returns_Redactor_For_Data_Classifications()
         Assert.Equal(typeof(ErasingRedactor), r3.GetType());
     }
 
+    [Fact]
+    public void RedactorProvider_Returns_Same_Redactor_For_Logically_Same_Data_Classification()
+    {
+        var dc1 = new DataClassificationSet(new DataClassification("DummyTaxonomy", "Classification"));
+        var dc2 = new DataClassificationSet(new DataClassification("DummyTaxonomy", "Classification2"));
+        var dc3 = new DataClassificationSet(new DataClassification("DummyTaxonomy", "Classification3"));
+        var dc4 = new DataClassificationSet(new DataClassification("DummyTaxonomy", "Classification4"));
+        var dc5 = new DataClassificationSet(new DataClassification("DummyTaxonomy", "Classification5"));
+        var dc6 = new DataClassificationSet(new DataClassification("DummyTaxonomy", "Classification6"));
+        var dc7 = new DataClassificationSet(new DataClassification("DummyTaxonomy", "Classification7"));
+        var dc8 = new DataClassificationSet(new DataClassification("DummyTaxonomy", "Classification8"));
+
+        var dc9 = new DataClassification("DummyTaxonomy", "Classification9");
+
+        var dc1LogicalCopy = new DataClassificationSet(new[] { new DataClassification("DummyTaxonomy", "Classification") });
+
+        var redactorProvider = new ServiceCollection()
+        .AddRedaction(redaction =>
+        {
+            redaction.SetRedactor<NullRedactor>(dc1);
+            redaction.SetRedactor<NullRedactor>(dc2);
+            redaction.SetRedactor<NullRedactor>(dc3);
+            redaction.SetRedactor<NullRedactor>(dc4);
+            redaction.SetRedactor<NullRedactor>(dc5);
+            redaction.SetRedactor<NullRedactor>(dc6);
+            redaction.SetRedactor<NullRedactor>(dc7);
+            redaction.SetRedactor<NullRedactor>(dc8);
+        })
+        .BuildServiceProvider()
+        .GetRequiredService<IRedactorProvider>();
+
+        var r1 = redactorProvider.GetRedactor(dc1);
+        var r2 = redactorProvider.GetRedactor(dc1LogicalCopy);
+        var r3 = redactorProvider.GetRedactor(dc9);
+
+        Assert.Equal(typeof(NullRedactor), r1.GetType());
+        Assert.Equal(typeof(NullRedactor), r2.GetType());
+        Assert.Equal(typeof(ErasingRedactor), r3.GetType());
+    }
+
     [Fact]
     public void RedactorProvider_Throws_On_Ctor_When_Options_Come_As_Null()
     {