feat: Improve changes detection to avoid unneeded recompilation

me-viper · Sep 27, 2023 · b767367 · b767367
1 parent 6bcd14f
commit b767367
Show file tree

Hide file tree

Showing 4 changed files with 36 additions and 10 deletions.
diff --git a/src/OpaDotNet.Extensions.AspNetCore/ConfigurationPolicySource.cs b/src/OpaDotNet.Extensions.AspNetCore/ConfigurationPolicySource.cs
@@ -9,10 +9,10 @@ namespace OpaDotNet.Extensions.AspNetCore;
 [UsedImplicitly]
 public class ConfigurationPolicySource : OpaPolicySource
 {
-    private readonly IOptionsMonitor<OpaPolicyOptions> _policy;
-
     private readonly IDisposable? _policyChangeMonitor;
 
+    private OpaPolicyOptions _opts;
+
     public ConfigurationPolicySource(
         IRegoCompiler compiler,
         IOptions<OpaAuthorizationOptions> authOptions,
@@ -21,12 +21,19 @@ public ConfigurationPolicySource(
     {
         ArgumentNullException.ThrowIfNull(policy);
 
-        _policy = policy;
-        _policyChangeMonitor = _policy.OnChange(
-            _ =>
+        _opts = policy.CurrentValue;
+        _policyChangeMonitor = policy.OnChange(
+            p =>
             {
                 try
                 {
+                    if (!HasChanged(p, _opts))
+                    {
+                        Logger.LogDebug("No changes in policies configuration");
+                        return;
+                    }
+
+                    _opts = p;
                     Task.Run(() => CompileBundle(true)).ConfigureAwait(false);
                 }
                 catch (Exception)
@@ -37,6 +44,26 @@ public ConfigurationPolicySource(
             );
     }
 
+    private static bool HasChanged(OpaPolicyOptions a, OpaPolicyOptions b)
+    {
+        if (ReferenceEquals(a, b))
+            return false;
+
+        if (a.Keys.Count != b.Keys.Count)
+            return true;
+
+        foreach (var (k, v) in a)
+        {
+            if (!b.TryGetValue(k, out var ov))
+                return true;
+
+            if (!v.Equals(ov))
+                return true;
+        }
+
+        return false;
+    }
+
     /// <inheritdoc />
     protected override async Task<Stream?> CompileBundleFromSource(bool recompiling, CancellationToken cancellationToken = default)
     {
@@ -46,7 +73,7 @@ public ConfigurationPolicySource(
 
         await using (bundleWriter.ConfigureAwait(false))
         {
-            foreach (var (name, policy) in _policy.CurrentValue)
+            foreach (var (name, policy) in _opts)
             {
                 if (!string.IsNullOrWhiteSpace(policy.DataJson))
                     bundleWriter.WriteEntry(policy.DataJson, $"/{policy.Package}/data.json");

diff --git a/src/OpaDotNet.Extensions.AspNetCore/OpaPolicyDefinitionOptions.cs b/src/OpaDotNet.Extensions.AspNetCore/OpaPolicyDefinitionOptions.cs
@@ -5,7 +5,7 @@
 namespace OpaDotNet.Extensions.AspNetCore;
 
 [UsedImplicitly]
-public class OpaPolicyDefinitionOptions
+public record OpaPolicyDefinitionOptions
 {
     [UsedImplicitly]
     public string? Package { get; set; }

diff --git a/src/OpaDotNet.Extensions.AspNetCore/PublicAPI.Shipped.txt b/src/OpaDotNet.Extensions.AspNetCore/PublicAPI.Shipped.txt
@@ -38,7 +38,6 @@ OpaDotNet.Extensions.AspNetCore.OpaPolicyDefinitionOptions.DataJson.get -> strin
 OpaDotNet.Extensions.AspNetCore.OpaPolicyDefinitionOptions.DataJson.set -> void
 OpaDotNet.Extensions.AspNetCore.OpaPolicyDefinitionOptions.DataYaml.get -> string?
 OpaDotNet.Extensions.AspNetCore.OpaPolicyDefinitionOptions.DataYaml.set -> void
-OpaDotNet.Extensions.AspNetCore.OpaPolicyDefinitionOptions.OpaPolicyDefinitionOptions() -> void
 OpaDotNet.Extensions.AspNetCore.OpaPolicyDefinitionOptions.Package.get -> string?
 OpaDotNet.Extensions.AspNetCore.OpaPolicyDefinitionOptions.Package.set -> void
 OpaDotNet.Extensions.AspNetCore.OpaPolicyDefinitionOptions.Source.get -> string?

diff --git a/tests/OpaDotNet.Extensions.AspNetCore.Tests/ConfigurationPolicySourceTests.cs b/tests/OpaDotNet.Extensions.AspNetCore.Tests/ConfigurationPolicySourceTests.cs
@@ -185,8 +185,8 @@ public async Task WatchChanges()
             _output.WriteLine($"Checking: u{i}");
             Assert.True(result.Result);
 
-            policyOptions["p1"].Source = Policy(i + 1);
-            optionsMonitor.Change(policyOptions);
+            var newOpts = new OpaPolicyOptions { { "p1", new() { Source = Policy(i + 1) } } };
+            optionsMonitor.Change(newOpts);
 
             await Task.Delay(TimeSpan.FromSeconds(5));
         }