Change LRUDiskCache to allow subclasses to override functionality #554
Conversation
| return this.lruStorage | ||
| } | ||
|
|
||
| public has (key: string): boolean { |
There was a problem hiding this comment.
I changed the has/get/set methods syntax because the previous syntax doesn't work properly when a subclass overrides, losing the access to super.
No changes were made in this one besides the syntax though.
| @@ -85,23 +103,18 @@ export class LRUDiskCache<V> implements CacheLayer<string, V>{ | |||
| }) | |||
|
|
|||
| // if it is an outdated file when stale=true | |||
| if (timeOfDeath < Date.now()) { | |||
| if (lruData.timeOfDeath < Date.now()) { | |||
There was a problem hiding this comment.
The timeOfDeath is now part of the data retrieved from the memomery cache (lruStorage), instead of being the value itself.
mairatma
force-pushed
the
feat/cacheOptions
branch
2 times, most recently
from
8c42d10
to
d9594b4
Compare
mairatma
force-pushed
the
perf/partialCache
branch
from
c037086
to
7f62e70
Compare
mairatma
requested review from
filipewl,
arturpimentel,
danzanzini and
filafb
| /** | ||
| * Builds the data object that will be stored at the LRU memory storage. | ||
| * Subclasses that need to store more than just the time of death should | ||
| * override this. | ||
| */ | ||
| protected buildLRUData(timeOfDeath: number, localCacheOptions?: LocalCacheOptions): LRUData { | ||
| return { timeOfDeath } | ||
| } |
There was a problem hiding this comment.
The data stored in the memory part of the cache was only the timeOfDeath. This new method will allow storing more data there, for any logic that needs to run before deciding to fetch the file from disk.
In our use case this extra data will be the list of fields that we have stored from the original object.
| } | ||
| public async set (key: string, value: V, maxAge?: number, localCacheOptions?: LocalCacheOptions): Promise<boolean> { | ||
| const timeOfDeath = maxAge ? maxAge + Date.now() : NaN | ||
| const lruData = this.buildLRUData(timeOfDeath, localCacheOptions) |
There was a problem hiding this comment.
This just changed the way we build the data to be stored in the memory cache to use the new buildLRUData function.
mairatma
force-pushed
the
feat/cacheOptions
branch
from
d9594b4
to
c49223e
Compare
mairatma
force-pushed
the
perf/partialCache
branch
from
8e58d42
to
9c1ca53
Compare
What is the purpose of this pull request?
This changes LRUDiskCache so that a subclass can properly override some of its functionality (such as the has/get/set methods, stats calculations, etc).
What problem is this solving?
We need to build a new disk cache implementation with extra features. To avoid reimplementing everything again, we can instead extend from LRUDiskCache and override some of its methods to achieve what we need. But the way LRUDiskCache was created didn't allow for these extensions, so this PR is changing it to allow better usage by subclasses.
How should this be manually tested?
Types of changes