Should not invalidate the cache of a Volatile Document. (#14720)

As it is never persisted and can't be retrieved from a given store, see PR comment.
OrchardCMS · Nov 20, 2023 · 7b92336 · 7b92336
1 parent b3fa6b7
commit 7b92336
Show file tree

Hide file tree

Showing 3 changed files with 22 additions and 45 deletions.
diff --git a/src/OrchardCore/OrchardCore.Data.YesSql/Documents/DocumentStore.cs b/src/OrchardCore/OrchardCore.Data.YesSql/Documents/DocumentStore.cs
@@ -21,7 +21,6 @@ public class DocumentStore : IDocumentStore
         private DocumentStoreCommitFailureDelegate _afterCommitFailure;
 
         private bool _canceled;
-        private bool _committed;
 
         public DocumentStore(ISession session)
         {
@@ -54,27 +53,10 @@ public DocumentStore(ISession session)
                 return (false, loaded as T);
             }
 
-            T document = null;
-
-            // For consistency checking a document may be queried after 'SaveChangesAsync()'.
-            if (_committed)
-            {
-                // So we create a new session to not get a cached version from 'YesSql'.
-                await using var session = _session.Store.CreateSession();
-                document = await session.Query<T>().FirstOrDefaultAsync();
-            }
-            else
+            var document = await _session.Query<T>().FirstOrDefaultAsync();
+            if (document is not null)
             {
-                document = await _session.Query<T>().FirstOrDefaultAsync();
-            }
-
-            if (document != null)
-            {
-                if (!_committed)
-                {
-                    _session.Detach(document);
-                }
-
+                _session.Detach(document);
                 return (true, document);
             }
 
@@ -131,7 +113,7 @@ public void AfterCommitFailure<T>(DocumentStoreCommitFailureDelegate afterCommit
         /// <inheritdoc />
         public async Task CommitAsync()
         {
-            if (_session == null)
+            if (_session is null)
             {
                 return;
             }
@@ -140,10 +122,9 @@ public async Task CommitAsync()
             {
                 await _session.SaveChangesAsync();
 
-                _committed = true;
                 _loaded.Clear();
 
-                if (!_canceled && _afterCommitSuccess != null)
+                if (!_canceled && _afterCommitSuccess is not null)
                 {
                     foreach (var d in _afterCommitSuccess.GetInvocationList())
                     {
@@ -153,7 +134,7 @@ public async Task CommitAsync()
             }
             catch (ConcurrencyException exception)
             {
-                if (_afterCommitFailure != null)
+                if (_afterCommitFailure is not null)
                 {
                     foreach (var d in _afterCommitFailure.GetInvocationList())
                     {

diff --git a/src/OrchardCore/OrchardCore.Infrastructure/Documents/DocumentManager.cs b/src/OrchardCore/OrchardCore.Infrastructure/Documents/DocumentManager.cs
@@ -52,7 +52,7 @@ public IDocumentStore DocumentStore
             get
             {
                 var documentStore = (IDocumentStore)ShellScope.Get(DocumentStoreServiceType);
-                if (documentStore == null)
+                if (documentStore is null)
                 {
                     documentStore = (IDocumentStore)ShellScope.Services.GetRequiredService(DocumentStoreServiceType);
                     ShellScope.Set(DocumentStoreServiceType, documentStore);
@@ -78,7 +78,7 @@ public async Task<TDocument> GetOrCreateMutableAsync(Func<Task<TDocument>> facto
             else
             {
                 var volatileCache = ShellScope.Get<TDocument>(typeof(TDocument));
-                if (volatileCache != null)
+                if (volatileCache is not null)
                 {
                     document = volatileCache;
                 }
@@ -133,7 +133,7 @@ public async Task<TDocument> GetOrCreateImmutableAsync(Func<Task<TDocument>> fac
                 });
             }
 
-            if (document == null)
+            if (document is null)
             {
                 var cacheable = true;
 
@@ -186,6 +186,7 @@ public async Task UpdateAsync(TDocument document, Func<TDocument, Task> afterUpd
             {
                 await DocumentStore.UpdateAsync(document, async document =>
                 {
+                    // A non volatile document can be invalidated.
                     await InvalidateInternalAsync(document);
 
                     if (afterUpdateAsync != null)
@@ -204,9 +205,10 @@ await DocumentStore.UpdateAsync(document, async document =>
             // But still update the shared cache after committing.
             DocumentStore.AfterCommitSuccess<TDocument>(async () =>
             {
-                await InvalidateInternalAsync(document);
+                // A volatile document can't be invalidated.
+                await SetInternalAsync(document);
 
-                if (afterUpdateAsync != null)
+                if (afterUpdateAsync is not null)
                 {
                     await afterUpdateAsync(document);
                 }
@@ -239,7 +241,7 @@ private async Task<TDocument> GetInternalAsync(bool failover = false)
                 id = _memoryCache.Get<string>(_options.CacheIdKey);
             }
 
-            if (id == null)
+            if (id is null)
             {
                 return null;
             }
@@ -279,7 +281,7 @@ private async Task<TDocument> GetInternalAsync(bool failover = false)
 
             document = await GetFromDistributedCacheAsync();
 
-            if (document == null)
+            if (document is null)
             {
                 return null;
             }
@@ -340,14 +342,7 @@ protected async Task SetInternalAsync(TDocument document, bool failover = false)
 
                 if (stored.Identifier != document.Identifier)
                 {
-                    if (_isDistributed)
-                    {
-                        await _distributedCache.RemoveAsync(_options.CacheIdKey);
-                    }
-                    else
-                    {
-                        _memoryCache.Remove(_options.CacheIdKey);
-                    }
+                    await InvalidateInternalAsync(document);
                 }
             }
         }

diff --git a/src/OrchardCore/OrchardCore.Infrastructure/Documents/VolatileDocumentManager.cs b/src/OrchardCore/OrchardCore.Infrastructure/Documents/VolatileDocumentManager.cs
@@ -63,7 +63,7 @@ public async Task UpdateAtomicAsync(Func<Task<TDocument>> updateAsync, Func<TDoc
                 delegates.UpdateDelegateAsync += updateDelegate;
             }
 
-            if (afterUpdateAsync != null)
+            if (afterUpdateAsync is not null)
             {
                 var afterUpdateDelegate = new AfterUpdateDelegate(afterUpdateAsync);
                 if (delegates.Targets.Add(afterUpdateDelegate.Target))
@@ -92,16 +92,17 @@ public async Task UpdateAtomicAsync(Func<Task<TDocument>> updateAsync, Func<TDoc
                     document = await ((UpdateDelegate)d)();
                 }
 
-                if (document == null)
+                if (document is null)
                 {
                     return;
                 }
 
                 document.Identifier ??= IdGenerator.GenerateId();
 
-                await InvalidateInternalAsync(document);
+                // A volatile document can't be invalidated.
+                await SetInternalAsync(document);
 
-                if (delegates.AfterUpdateDelegateAsync != null)
+                if (delegates.AfterUpdateDelegateAsync is not null)
                 {
                     foreach (var d in delegates.AfterUpdateDelegateAsync.GetInvocationList())
                     {
@@ -115,7 +116,7 @@ private sealed class UpdateDelegates
         {
             public UpdateDelegate UpdateDelegateAsync;
             public AfterUpdateDelegate AfterUpdateDelegateAsync;
-            public HashSet<object> Targets = new();
+            public HashSet<object> Targets = [];
         }
     }
 }