Updated Permission Service to simplify caching #1143
Conversation
darrelmiller
requested review from
irvinesunday,
andrueastman,
baywet,
ddyett,
MIchaelMainer,
nikithauc,
zengin,
MaggieKimani1,
fey101,
adhiambovivian,
danielmbaluka and
rkodev
as code owners
| @@ -161,6 +158,10 @@ private string SanitizeUrlQueryPath(string url) | |||
| return url; | |||
| } | |||
|
|
|||
| // use the service provider to lazily get an IPermissionsStore instance | |||
There was a problem hiding this comment.
I moved this after the URL check to avoid loading the uriTemplateMatcher from the cache if we are not going to use it.
| @@ -50,36 +48,68 @@ public class PermissionsStore : IPermissionsStore | |||
| private const string PermissionsContainerBlobConfig = "BlobStorage:Containers:Permissions"; | |||
| private const string NullValueError = "Value cannot be null"; | |||
|
|
|||
|
|
|||
| private class PermissionsDataInfo | |||
There was a problem hiding this comment.
I had to create this wrapper class so that LoadPermissionsData could return both the UriTemplateMatcher and the ScopesList. I want to avoid using class level variables because we are going to store this in the cache. This avoids having to keep track of booleans to tell us if the cache is fresh, and it reduces multi-threading issues.
| { | ||
| get | ||
| { | ||
| if (!_Cache.TryGetValue<PermissionsDataInfo>("PermissionsData", out var permissionsData)) |
There was a problem hiding this comment.
If anyone tries to get access to the PermissionsData we go get the reference from the cache. We don't hold it as a field. If we can't get it from the cache, we go load the data and put that in a lock so that a second caller will get blocked until the load is complete.
If the data expires it will be flushed from cache within a minute.
| { | ||
| _urlTemplateMatcher = new UriTemplateMatcher(); | ||
| _scopesListTable = new Dictionary<int, object>(); | ||
| var urlTemplateMatcher = new UriTemplateMatcher(); |
There was a problem hiding this comment.
Class level variables in singleton services are a real pain for multi-threaded apps. Avoid them where you can.
| private IDictionary<int, object> _scopesListTable; | ||
| private readonly IMemoryCache _permissionsCache; | ||
|
|
||
| private readonly IMemoryCache _Cache; |
There was a problem hiding this comment.
The cache can store anything we need, so let's just call it what it is, a cache.
|
Kudos, SonarCloud Quality Gate passed! |
Overview
To try and minimize memory usage, the caching of permission data has been simplified and no longer uses the recyclable memory stream.