From bd2504c8dfe9f4d486173143aa5256a99746a574 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Wed, 25 Feb 2026 11:36:05 -0500 Subject: [PATCH] feat: add SUB permission caching with generation invalidation (Gap 5.8) Extend PermissionLruCache with SetSub/TryGetSub (internal key prefix "S:") alongside existing PUB API ("P:" prefix, backward-compatible). Add Invalidate() and Generation property for generation-based cache invalidation. Add GenerationId/IncrementGeneration to Account for account-level change signalling. 10 new tests in SubPermissionCacheTests cover all paths. --- src/NATS.Server/Auth/Account.cs | 10 ++ src/NATS.Server/Auth/PermissionLruCache.cs | 123 ++++++++++--- .../Auth/SubPermissionCacheTests.cs | 161 ++++++++++++++++++ 3 files changed, 272 insertions(+), 22 deletions(-) create mode 100644 tests/NATS.Server.Tests/Auth/SubPermissionCacheTests.cs diff --git a/src/NATS.Server/Auth/Account.cs b/src/NATS.Server/Auth/Account.cs index 5fed016..ea5cad8 100644 --- a/src/NATS.Server/Auth/Account.cs +++ b/src/NATS.Server/Auth/Account.cs @@ -152,6 +152,16 @@ public sealed class Account : IDisposable return true; } + // Generation ID for permission cache invalidation. + // Callers hold a per-client copy; when it diverges from GenerationId they must flush their caches. + // Reference: Go server/accounts.go — account generation tracking for permission invalidation. + private long _generationId; + + public long GenerationId => Interlocked.Read(ref _generationId); + + /// Increments the generation counter, signalling that permission caches are stale. + public void IncrementGeneration() => Interlocked.Increment(ref _generationId); + // Slow consumer tracking // Go reference: server/client.go — handleSlowConsumer, markConnAsSlow, server/accounts.go slowConsumerCount private long _slowConsumerCount; diff --git a/src/NATS.Server/Auth/PermissionLruCache.cs b/src/NATS.Server/Auth/PermissionLruCache.cs index e0e5856..afd571b 100644 --- a/src/NATS.Server/Auth/PermissionLruCache.cs +++ b/src/NATS.Server/Auth/PermissionLruCache.cs @@ -1,7 +1,8 @@ namespace NATS.Server.Auth; /// -/// Fixed-capacity LRU cache for permission results. +/// Fixed-capacity LRU cache for permission results, supporting both PUB and SUB entries +/// with generation-based invalidation. /// Lock-protected (per-client, low contention). /// Reference: Go client.go maxPermCacheSize=128. /// @@ -12,17 +13,51 @@ public sealed class PermissionLruCache private readonly LinkedList<(string Key, bool Value)> _list = new(); private readonly object _lock = new(); + // Generation tracking: _generation is the authoritative counter (bumped by Invalidate), + // _cacheGeneration is what the cache was last synced to. A mismatch on access triggers a clear. + private long _generation; + private long _cacheGeneration; + public PermissionLruCache(int capacity = 128) { _capacity = capacity; _map = new Dictionary>(capacity, StringComparer.Ordinal); } + /// + /// The current generation counter. Incremented on each call. + /// + public long Generation => Interlocked.Read(ref _generation); + + /// + /// Bumps the generation counter so that the next TryGet or TryGetSub call detects + /// the staleness and clears all cached entries. + /// + public void Invalidate() => Interlocked.Increment(ref _generation); + + // Ensures the cache is cleared if Invalidate was called since the last access. + // Must be called inside _lock. + private void EnsureFresh() + { + var gen = Interlocked.Read(ref _generation); + if (gen != _cacheGeneration) + { + _map.Clear(); + _list.Clear(); + _cacheGeneration = gen; + } + } + + // ── PUB API (backward-compatible) ──────────────────────────────────────── + + /// Looks up a PUB permission for . public bool TryGet(string key, out bool value) { + var internalKey = "P:" + key; lock (_lock) { - if (_map.TryGetValue(key, out var node)) + EnsureFresh(); + if (_map.TryGetValue(internalKey, out var node)) { value = node.Value.Value; _list.Remove(node); @@ -35,6 +70,52 @@ public sealed class PermissionLruCache } } + /// Stores a PUB permission for . + public void Set(string key, bool value) + { + var internalKey = "P:" + key; + lock (_lock) + { + EnsureFresh(); + SetInternal(internalKey, value); + } + } + + // ── SUB API ─────────────────────────────────────────────────────────────── + + /// Looks up a SUB permission for . + public bool TryGetSub(string subject, out bool value) + { + var internalKey = "S:" + subject; + lock (_lock) + { + EnsureFresh(); + if (_map.TryGetValue(internalKey, out var node)) + { + value = node.Value.Value; + _list.Remove(node); + _list.AddFirst(node); + return true; + } + + value = default; + return false; + } + } + + /// Stores a SUB permission for . + public void SetSub(string subject, bool allowed) + { + var internalKey = "S:" + subject; + lock (_lock) + { + EnsureFresh(); + SetInternal(internalKey, allowed); + } + } + + // ── Shared ──────────────────────────────────────────────────────────────── + public int Count { get @@ -46,28 +127,26 @@ public sealed class PermissionLruCache } } - public void Set(string key, bool value) + // Must be called inside _lock. + private void SetInternal(string internalKey, bool value) { - lock (_lock) + if (_map.TryGetValue(internalKey, out var existing)) { - if (_map.TryGetValue(key, out var existing)) - { - _list.Remove(existing); - existing.Value = (key, value); - _list.AddFirst(existing); - return; - } - - if (_map.Count >= _capacity) - { - var last = _list.Last!; - _map.Remove(last.Value.Key); - _list.RemoveLast(); - } - - var node = new LinkedListNode<(string Key, bool Value)>((key, value)); - _list.AddFirst(node); - _map[key] = node; + _list.Remove(existing); + existing.Value = (internalKey, value); + _list.AddFirst(existing); + return; } + + if (_map.Count >= _capacity) + { + var last = _list.Last!; + _map.Remove(last.Value.Key); + _list.RemoveLast(); + } + + var node = new LinkedListNode<(string Key, bool Value)>((internalKey, value)); + _list.AddFirst(node); + _map[internalKey] = node; } } diff --git a/tests/NATS.Server.Tests/Auth/SubPermissionCacheTests.cs b/tests/NATS.Server.Tests/Auth/SubPermissionCacheTests.cs new file mode 100644 index 0000000..e526184 --- /dev/null +++ b/tests/NATS.Server.Tests/Auth/SubPermissionCacheTests.cs @@ -0,0 +1,161 @@ +using NATS.Server.Auth; +using Shouldly; + +namespace NATS.Server.Tests.Auth; + +/// +/// Tests for SUB permission caching and generation-based invalidation. +/// Reference: Go server/client.go — subPermCache, pubPermCache, perm cache invalidation on account update. +/// +public sealed class SubPermissionCacheTests +{ + // ── SUB API ─────────────────────────────────────────────────────────────── + + [Fact] + public void SetSub_and_TryGetSub_round_trips() + { + var cache = new PermissionLruCache(); + + cache.SetSub("foo.bar", true); + + cache.TryGetSub("foo.bar", out var result).ShouldBeTrue(); + result.ShouldBeTrue(); + } + + [Fact] + public void TryGetSub_returns_false_for_unknown() + { + var cache = new PermissionLruCache(); + + cache.TryGetSub("unknown.subject", out var result).ShouldBeFalse(); + result.ShouldBeFalse(); + } + + [Fact] + public void PUB_and_SUB_stored_independently() + { + var cache = new PermissionLruCache(); + + // Same logical subject, different PUB/SUB outcomes + cache.Set("orders.>", false); // PUB denied + cache.SetSub("orders.>", true); // SUB allowed + + cache.TryGet("orders.>", out var pubAllowed).ShouldBeTrue(); + pubAllowed.ShouldBeFalse(); + + cache.TryGetSub("orders.>", out var subAllowed).ShouldBeTrue(); + subAllowed.ShouldBeTrue(); + } + + // ── Invalidation ───────────────────────────────────────────────────────── + + [Fact] + public void Invalidate_clears_on_next_access() + { + var cache = new PermissionLruCache(); + + cache.Set("pub.subject", true); + cache.SetSub("sub.subject", true); + + cache.Invalidate(); + + // Both PUB and SUB lookups should miss after invalidation + cache.TryGet("pub.subject", out _).ShouldBeFalse(); + cache.TryGetSub("sub.subject", out _).ShouldBeFalse(); + } + + [Fact] + public void Generation_increments_on_invalidate() + { + var cache = new PermissionLruCache(); + + var before = cache.Generation; + cache.Invalidate(); + var afterOne = cache.Generation; + cache.Invalidate(); + var afterTwo = cache.Generation; + + afterOne.ShouldBe(before + 1); + afterTwo.ShouldBe(before + 2); + } + + // ── LRU eviction ───────────────────────────────────────────────────────── + + [Fact] + public void LRU_eviction_applies_to_SUB_entries() + { + // capacity = 4: fill with 4 SUB entries then add a 5th; the oldest should be evicted + var cache = new PermissionLruCache(capacity: 4); + + cache.SetSub("a", true); + cache.SetSub("b", true); + cache.SetSub("c", true); + cache.SetSub("d", true); + + // Touch "a" so it becomes MRU; "b" becomes LRU + cache.TryGetSub("a", out _); + + // Adding "e" should evict "b" (LRU) + cache.SetSub("e", true); + + cache.Count.ShouldBe(4); + cache.TryGetSub("b", out _).ShouldBeFalse("b should have been evicted"); + cache.TryGetSub("e", out _).ShouldBeTrue("e was just added"); + } + + // ── Backward compatibility ──────────────────────────────────────────────── + + [Fact] + public void Existing_PUB_API_still_works() + { + var cache = new PermissionLruCache(); + + cache.Set("pub.only", true); + + cache.TryGet("pub.only", out var value).ShouldBeTrue(); + value.ShouldBeTrue(); + + // Overwrite with false + cache.Set("pub.only", false); + cache.TryGet("pub.only", out value).ShouldBeTrue(); + value.ShouldBeFalse(); + } + + // ── Account.GenerationId ────────────────────────────────────────────────── + + [Fact] + public void Account_GenerationId_starts_at_zero() + { + var account = new Account("test"); + + account.GenerationId.ShouldBe(0L); + } + + [Fact] + public void Account_IncrementGeneration_increments() + { + var account = new Account("test"); + + account.IncrementGeneration(); + account.GenerationId.ShouldBe(1L); + + account.IncrementGeneration(); + account.GenerationId.ShouldBe(2L); + } + + // ── Mixed PUB + SUB count ───────────────────────────────────────────────── + + [Fact] + public void Mixed_PUB_SUB_count_includes_both() + { + var cache = new PermissionLruCache(); + + cache.Set("pub.a", true); + cache.Set("pub.b", false); + cache.SetSub("sub.a", true); + cache.SetSub("sub.b", false); + + // All four entries (stored under different internal keys) contribute to Count + cache.Count.ShouldBe(4); + } +}