Caching `ClaimsPrincipal` during authentication and re-caching `FileConfiguration` objects in Consul and Disk File configuration repos #1249

EngRajabi · 2020-05-30T17:11:08Z

New Feature

Cache response access token

Motivation for New Feature

Improve performance for cache identity response.
Improve performance for cache disk file repository.
In large projects and with many simultaneous requests, this cache is required.

EngRajabi · 2020-06-23T10:05:23Z

@TomPallister

Hossein-Edraki

That's Good

raman-m · 2023-07-19T17:38:18Z

Hi Mohsen!
Thanks for the PR!

What issue is this PR related to?

raman-m · 2023-07-19T17:57:26Z

@EngRajabi commented on May 30, 2020:

improve performance for cache identity response.
improve performance for cache disk file repository.

In comparison to what performance indicators was the current performance improved?
Could you rewrite the description of your feature request please?

EngRajabi · 2023-07-20T07:21:25Z

I was in an enterprise project that used the reference token type. I had a performance problem. Because in every request, it needs to request the identity of the server. The problem was solved with this cache.

EngRajabi

merge conflict

raman-m · 2023-07-20T13:21:36Z

@EngRajabi commented on July 20, 2023

Did you use Service Discovery + Identity Server setup, or classic setup + Identity Server Bearer Tokens?
🆗 We will talk more during code review...

raman-m · 2023-07-20T13:22:52Z

Unfortunately there are no builds for these commits!
So, I cannot start code review without having an actual valid/green build...

raman-m · 2023-08-01T13:29:35Z

Mohsen, congrats! 🎉
The feature branch has been rebased onto ThreeMammals:develop successfully!

Now we can go with code review...

And, once again,
What issue is this PR related to?

Wrap with braces

raman-m

The primary issue with this PR is the lack of a feature switcher❗

We suggest upgrading the middleware to enforce cache usage, which could be a breaking change for Ocelot applications in production. Therefore, we need to implement the upgrade with the cache turned off by default, and introduce a new JSON option to enable/disable caching as needed.

Additional suggestions include:

Implementing unit+acceptance tests to cover the new static CacheTtlSeconds property
Developing integration tests with usage of the DefaultMemoryCache{T} caching service

raman-m · 2024-11-10T14:25:29Z

src/Ocelot/Authentication/Middleware/AuthenticationMiddleware.cs

                await _next(httpContext);
                return;
            }

            Logger.LogInformation(() => $"The path '{path}' is an authenticated route! {MiddlewareName} checking if client is authenticated...");
+            var token = httpContext.Request.Headers.Authorization;
+            var cacheKey = $"{nameof(AuthenticationMiddleware)}.{nameof(IHeaderDictionary.Authorization)}:{token}";
+            if (!_memoryCache.TryGetValue(cacheKey, out ClaimsPrincipal principal))


We are upgrading with enforced use of the cache❗

raman-m · 2024-11-10T14:27:55Z

src/Ocelot/Configuration/Repository/DiskFileConfigurationRepository.cs

@@ -42,22 +53,29 @@ private void Initialize(string folder)

        public Task<Response<FileConfiguration>> Get()
        {
-            string jsonConfiguration;
+            var configuration = _cache.Get(CacheKey, region: CacheKey);


Enforced use of the cache❗

raman-m · 2024-11-10T14:29:25Z

test/Ocelot.UnitTests/Authentication/AuthenticationMiddlewareTests.cs

@@ -49,7 +51,7 @@ public void MiddlewareName_Cstor_ReturnsTypeName()
                isNextCalled = true;
                return Task.CompletedTask;
            };
-            _middleware = new AuthenticationMiddleware(_next, _factory.Object);
+            _middleware = new AuthenticationMiddleware(_next, _factory.Object, new MemoryCache(new MemoryCacheOptions()));


Enforcing the use of new MemoryCache turns the test into an integration test.

raman-m · 2024-11-10T14:30:02Z

test/Ocelot.UnitTests/Configuration/DiskFileConfigurationRepositoryTests.cs

-        }
-
+
+            // TODO Add integration tests: var aspMemoryCache = new DefaultMemoryCache<FileConfiguration>(new MemoryCache(new MemoryCacheOptions()));


Add integration tests!

Hossein-Edraki approved these changes Oct 7, 2020

View reviewed changes

raman-m assigned EngRajabi Jul 19, 2023

raman-m changed the base branch from master to develop July 19, 2023 17:38

raman-m self-requested a review July 19, 2023 17:58

raman-m added feature A new feature question Initially seen a question could become a new feature or bug or closed ;) needs feedback Issue is waiting on feedback before acceptance waiting Waiting for answer to question or feedback from issue raiser labels Jul 19, 2023

EngRajabi commented Jul 20, 2023

View reviewed changes

raman-m force-pushed the feat_accessToken_cache branch from cd5675c to 0561ad5 Compare August 1, 2023 13:21

raman-m removed question Initially seen a question could become a new feature or bug or closed ;) waiting Waiting for answer to question or feedback from issue raiser labels Aug 1, 2023

raman-m requested review from binarymash, jps, geffzhang, RaynaldM, philproctor, anktsrkr, sharpn, abooraja, wast, yangzhongke and thiagoloureiro August 1, 2023 13:31

raman-m force-pushed the feat_accessToken_cache branch from c8c0634 to 4c2b93e Compare October 13, 2023 16:55

raman-m removed request for binarymash, jps, geffzhang, philproctor, anktsrkr, sharpn, abooraja, yangzhongke, thiagoloureiro and Hossein-Edraki November 9, 2024 19:17

EngRajabi and others added 5 commits November 10, 2024 14:35

add cache

4324e1e

add cache file repository

b09bb55

SA1503 Braces should not be omitted.

efc955b

Wrap with braces

Add required namespaces

d0fca0f

Fix build after rebasing

a4e33f4

raman-m force-pushed the feat_accessToken_cache branch from 4c2b93e to a4e33f4 Compare November 10, 2024 12:21

raman-m added 5 commits November 10, 2024 15:32

EOL: src/Ocelot.Provider.Consul/ConsulFileConfigurationRepository.cs

6504dcc

Quick code review by @raman-m

d052f47

CacheTtlSeconds static value

ff3f5ab

Global static CacheTtlSeconds settings

36909d9

Fix unit tests

615a7c7

raman-m requested changes Nov 10, 2024

View reviewed changes

raman-m added Authentication Ocelot feature: Authentication Configuration Ocelot feature: Configuration and removed needs feedback Issue is waiting on feedback before acceptance labels Nov 10, 2024

raman-m requested a review from ggnaegi November 10, 2024 14:43

raman-m added the Caching Ocelot feature: Caching label Nov 10, 2024

raman-m changed the title ~~Feat access token cache~~ Caching ClaimsPrincipalduring authentication and re-caching FileConfiguration objects in Consul and Disk File repos Nov 10, 2024

raman-m changed the title ~~Caching ClaimsPrincipalduring authentication and re-caching FileConfiguration objects in Consul and Disk File repos~~ Caching ClaimsPrincipal during authentication and re-caching FileConfiguration objects in Consul and Disk File configuration repos Nov 10, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Caching `ClaimsPrincipal` during authentication and re-caching `FileConfiguration` objects in Consul and Disk File configuration repos #1249

Caching `ClaimsPrincipal` during authentication and re-caching `FileConfiguration` objects in Consul and Disk File configuration repos #1249

EngRajabi commented May 30, 2020 •

edited by raman-m

Loading

EngRajabi commented Jun 23, 2020

Hossein-Edraki left a comment

raman-m commented Jul 19, 2023

raman-m commented Jul 19, 2023

EngRajabi commented Jul 20, 2023

EngRajabi left a comment

raman-m commented Jul 20, 2023

raman-m commented Jul 20, 2023 •

edited

Loading

raman-m commented Aug 1, 2023

raman-m left a comment •

edited

Loading

raman-m Nov 10, 2024

raman-m Nov 10, 2024

raman-m Nov 10, 2024

raman-m Nov 10, 2024

		}


		// TODO Add integration tests: var aspMemoryCache = new DefaultMemoryCache<FileConfiguration>(new MemoryCache(new MemoryCacheOptions()));

Caching ClaimsPrincipal during authentication and re-caching FileConfiguration objects in Consul and Disk File configuration repos #1249

Are you sure you want to change the base?

Caching ClaimsPrincipal during authentication and re-caching FileConfiguration objects in Consul and Disk File configuration repos #1249

Conversation

EngRajabi commented May 30, 2020 • edited by raman-m Loading

New Feature

Motivation for New Feature

EngRajabi commented Jun 23, 2020

Hossein-Edraki left a comment

Choose a reason for hiding this comment

raman-m commented Jul 19, 2023

raman-m commented Jul 19, 2023

EngRajabi commented Jul 20, 2023

EngRajabi left a comment

Choose a reason for hiding this comment

raman-m commented Jul 20, 2023

raman-m commented Jul 20, 2023 • edited Loading

raman-m commented Aug 1, 2023

raman-m left a comment • edited Loading

Choose a reason for hiding this comment

The primary issue with this PR is the lack of a feature switcher❗

raman-m Nov 10, 2024

Choose a reason for hiding this comment

raman-m Nov 10, 2024

Choose a reason for hiding this comment

raman-m Nov 10, 2024

Choose a reason for hiding this comment

raman-m Nov 10, 2024

Choose a reason for hiding this comment

Caching `ClaimsPrincipal` during authentication and re-caching `FileConfiguration` objects in Consul and Disk File configuration repos #1249

Caching `ClaimsPrincipal` during authentication and re-caching `FileConfiguration` objects in Consul and Disk File configuration repos #1249

EngRajabi commented May 30, 2020 •

edited by raman-m

Loading

raman-m commented Jul 20, 2023 •

edited

Loading

raman-m left a comment •

edited

Loading