Dynamic cache #1624

paynecrl97 · 2018-03-17T19:10:52Z

There's still a few things to do, but I'd like to get feedback on progress so far.

To do:

Sort out features- what should live where? cache block is registered in liquid at the moment, this probably isn't right.
Dependencies- do we need them?
Tests
Remove changes to views used for testing

…dynamic_cache # Conflicts: # src/OrchardCore.Modules/OrchardCore.DynamicCache/Services/DynamicCacheShapeDisplayEvents.cs

… nested within each other

Updates documentation

…dynamic_cache # Conflicts: # src/OrchardCore.Themes/TheBlogTheme/Views/Menu.liquid

Liquid cache statement gracefully handles no available cache service

Adds liquid tags to alter current CacheContext

sebastienros · 2018-03-20T23:20:56Z

src/OrchardCore.Modules/OrchardCore.DynamicCache/README.md


 | Method | Description |
 | --------- | ----------- |
-| `During(Timespan)` | Cached the shape for the specific amount of time. |
+| `WithDuration(Timespan)` | Cache the shape for the specific amount of time. |
+| `WithSlidingExpiration(Timespan)` | Cache the shape for a specific amount of time with a sliding window. |
 | `AddContext(params string[])` | Varies the cached content on the specified context values. |
 | `RemoveContext(string)` | Removes the specified context. |
 | `AddDependency(params string[])` | Defines the context values that will invalidate the cache entry. |


…dynamic_cache

sebastienros · 2018-04-23T19:42:09Z

src/OrchardCore.Themes/TheBlogTheme/Views/Content-BlogPost.Summary.liquid

@@ -1,5 +1,5 @@
-{% cache "blog-post-summary" %}
-{% cache Model.ContentItem.ContentItemId %}
+{% assign cacheId = "blog-post-summary-" | append: Model.ContentItem.ContentItemId %}


Why both in the id and as a dependency?

The dependency causes the cached block to be invalidated when the content changes.

The cache id gives us a unique cache item for each blog post.

But you pass an id, not a content item to the dependency. How does it know it's a content item id?

sebastienros · 2018-04-23T21:27:21Z

src/OrchardCore.Modules/OrchardCore.DynamicCache/Services/IDynamicCacheService.cs

-        Task SetCachedValueAsync(CacheContext context, string value); 
-        Task<string> ProcessEdgeSideIncludesAsync(string value);
+        Task SetCachedValueAsync(CacheContext context, string value);
+        Task<string> ReplaceEdgeSideIncludeTokensAsync(string value);


All the calls to ReplaceEdgeSideIncludeTokensAsync have been removed?

…dynamic_cache

Other small tidyups for PR

paynecrl97 · 2018-05-15T12:10:53Z

...OrchardCore.Modules/OrchardCore.DynamicCache/EventHandlers/DynamicCacheShapeDisplayEvents.cs

+            // So, if the cache context is not present in the _cached collection, we need to insert the ChildContent value into the cache:
+            if (!_cached.Contains(cacheContext) && context.ChildContent != null)
+            {
+                _cacheScopeManager.ExitScope(); // todo: how can we guarantee that this is called, even on failures?


^^ Any suggestions on how to achieve this?

We need another method that will be called when the display failed, or that is called even if it failed. I am checking where IShapeDisplayEvents is called.

We can also implement IDisposable and call dispose on the event instances from the caller. Is it didn't go into DisplayedAsync but calls Dispose then we know there was an issue and can exit from the scope.

Make IShapeDisplayEvents implement IDisposable? I don't think this would work as these scopes are sensitive to the order that they are opened and closed, so you'd need a new instance of each IShapeDisplayEvents for every shape (so that they could be disposed immediately after each shape had been rendered).

I think the first suggestion could work though.

Looks like shape events are only used in one place - DefaultHtmlDisplay.

Suggest adding Task DisplayingFailedAsync(ShapeDisplayContext context); to interface, and we guarantee that one of DisplayingFailedAsync or DisplayedAsync is always called.

Removes testing markup from views

sebastienros · 2018-05-15T16:47:28Z

src/OrchardCore.Modules/OrchardCore.DynamicCache/Liquid/CacheStatement.cs

+                return Completion.Normal;
+            }
+
+            cacheScopeManager.EnterScope(cacheContext);


try/finally to ensure ExitScope is always called

sebastienros · 2018-05-15T16:48:51Z

...OrchardCore.Modules/OrchardCore.DynamicCache/EventHandlers/DynamicCacheShapeDisplayEvents.cs

+            // So, if the cache context is not present in the _cached collection, we need to insert the ChildContent value into the cache:
+            if (!_cached.Contains(cacheContext) && context.ChildContent != null)
+            {
+                _cacheScopeManager.ExitScope(); // todo: how can we guarantee that this is called, even on failures?


We can also implement IDisposable and call dispose on the event instances from the caller. Is it didn't go into DisplayedAsync but calls Dispose then we know there was an issue and can exit from the scope.

Fixes cache attribute name on menu tag Adds DisplayingFinalizedAsync to shape disply

…dynamic_cache # Conflicts: # src/OrchardCore.Themes/TheBlogTheme/Views/Content-BlogPost.Summary.liquid

sebastienros · 2018-05-21T19:44:00Z

src/OrchardCore.Modules/OrchardCore.DynamicCache/Liquid/CacheStatement.cs

+            }
+
+            var debugMode = Configuration.IsDebugModeEnabled;
+            var splitChars = new[] { ',', ' ' };


paynecrl97 added 8 commits March 9, 2018 22:30

Adds liquid cache block

96c2c21

Merge branch 'dev' of https://github.com/OrchardCMS/OrchardCore into …

45ae222

…dynamic_cache # Conflicts: # src/OrchardCore.Modules/OrchardCore.DynamicCache/Services/DynamicCacheShapeDisplayEvents.cs

Cache logic abstracted away from shape events

22d2955

Refactor to abstract cache logic

c0edeb2

A little tidyup

562c4b1

Cached shapes and cache blocks now play nicely together when they are…

5a2b45c

… nested within each other

General tidyup

7cf7ffb

Adds fixed and sliding duration attributes to shape helpers

4d50ab6

Updates documentation

paynecrl97 added the don't merge label Mar 17, 2018

paynecrl97 added 5 commits March 17, 2018 19:12

Readme table fix

6558634

Merge branch 'dev' of https://github.com/OrchardCMS/OrchardCore into …

e7d2931

…dynamic_cache # Conflicts: # src/OrchardCore.Themes/TheBlogTheme/Views/Menu.liquid

Adds automatic tagging when displaying content items

c3e1335

Removes concept of Dependencies

28b561d

Liquid cache statement gracefully handles no available cache service

Adds Expires On to CacheContext

bda0237

Adds liquid tags to alter current CacheContext

sebastienros reviewed Mar 20, 2018

View reviewed changes

paynecrl97 added 6 commits March 21, 2018 12:10

Adds debug cached shape wrapper

c00034e

Merge branch 'dev' of https://github.com/OrchardCMS/OrchardCore into …

157e42a

…dynamic_cache

Merge branch 'dev' of https://github.com/OrchardCMS/OrchardCore into …

92ba519

…dynamic_cache

Adds cache tag helpers to allow the scope to be altered

97ff456

Fixes bug where cache contexts could be ignored

5a62c7f

Removes ESIs

f18ef7b

sebastienros reviewed Apr 23, 2018

View reviewed changes

paynecrl97 added 2 commits May 14, 2018 22:47

Merge branch 'dev' of https://github.com/OrchardCMS/OrchardCore into …

6d33cc3

…dynamic_cache

Fully removes ESIs

bd923b6

Other small tidyups for PR

paynecrl97 commented May 15, 2018

View reviewed changes

Adds tests for cache scope manager

717cb55

Removes testing markup from views

sebastienros reviewed May 15, 2018

View reviewed changes

Fixes issue where debug wrapper could duplicate

444a47e

Fixes cache attribute name on menu tag Adds DisplayingFinalizedAsync to shape disply

paynecrl97 removed the don't merge label May 20, 2018

Merge branch 'dev' of https://github.com/OrchardCMS/OrchardCore into …

1533951

…dynamic_cache # Conflicts: # src/OrchardCore.Themes/TheBlogTheme/Views/Content-BlogPost.Summary.liquid

sebastienros reviewed May 21, 2018

View reviewed changes

Static array

3a9dfc2

sebastienros merged commit 7fb0838 into dev May 23, 2018

sebastienros deleted the dynamic_cache branch May 23, 2018 19:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Dynamic cache #1624

Dynamic cache #1624

paynecrl97 commented Mar 17, 2018 •

edited

sebastienros Mar 20, 2018

sebastienros Apr 23, 2018

paynecrl97 Apr 23, 2018

sebastienros Apr 23, 2018

sebastienros Apr 23, 2018

paynecrl97 May 15, 2018

sebastienros May 15, 2018

sebastienros May 15, 2018

paynecrl97 May 15, 2018

sebastienros May 15, 2018

paynecrl97 May 15, 2018

sebastienros May 15, 2018

sebastienros May 15, 2018

sebastienros May 21, 2018

Dynamic cache #1624

Dynamic cache #1624

Conversation

paynecrl97 commented Mar 17, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

paynecrl97 commented Mar 17, 2018 •

edited