feat: added new helper functions #56
Conversation
* readEntity - returns the entityTypeMap values for id * readEntityExpired - returns boolean as to whether entity has expired * readEntityExpirationTime - returns the expire time for the entity
|
Hey @danReynolds, do you have any initial feedback on the above? |
There was a problem hiding this comment.
Looking great @jwarykowski 👏 ! Thanks for taking this on. I left some thoughts, just some small implementation details. I'd also recommend we write some tests for these new APIs once the implementation is finalized. If you'd like some help with that let me know, I could always jump in and write some or an example if you're not sure or time constrained.
| const { cacheTime, typename } = entity; | ||
|
|
||
| const timeToLive = | ||
| this.invalidationPolicies?.types?.[typename]?.timeToLive || |
There was a problem hiding this comment.
Could we make a helper function for calculating the expiration time to be reused here:
and add that helper as a member to the InvalidationPolicyManager class that the cache here could then use like this.invalidationPolicyManager.readEntityExpirationTime(id)?
There was a problem hiding this comment.
Hey @danReynolds, sorry, a little confused on the above. If possible can you add more details here just to make sure I make he correct change?
There was a problem hiding this comment.
Hey just got back from holiday. Yep so I'm thinking we'd add a function called readEntityExpirationTime to the invalidation policy manager class:
It would look like this:
readEntityExpirationTime(id: string) {
const { entityTypeMap, policies } = this.config;
// This simple helper function would also be added to the entity type map class
const cacheTime = entityTypeMap.readEntityCacheTime(id);
const timeToLive =
this.getPolicy(typename)?.timeToLive || policies.timeToLive;
if (!isNumber(cacheTime) || !isNumber(timeToLive)) {
return null;
}
return cacheTime + timeToLive;
}
There was a problem hiding this comment.
Basically something very similar to what you had in the InvalidationPolicyCache but for the manager. That way we can then call it again in here:
to re-use the functionality for getting the expiration time. Does that make sense? Thanks again for looking into this! I know you're busy with other things too so if you want me to help out with some stuff here I can.| } | ||
|
|
||
| readEntityExpired(id: string) { | ||
| const entity = this._expire(true).find( |
There was a problem hiding this comment.
To prevent us having to iterate through all entities to check expirations, which is somewhat more heavy, could we re-use the readEntityExpirationTime API and check against the current time to determine if it's expired here?
There was a problem hiding this comment.
Given that this is a boolean check I'd also recommend we go with a name like isEntityExpired
| @@ -380,6 +381,38 @@ export default class InvalidationPolicyCache extends InMemoryCache { | |||
| return this._expire(true); | |||
| } | |||
|
|
|||
| readEntity(id: string) { | |||
There was a problem hiding this comment.
I believe we had talked about a readEntityCacheTime API in the issue as opposed to exposing the entire underlying entity which has some metadata that we can probably keep inaccessible. Thoughts on that? I would also suggest it be written in the entityTypeMap and then called as this.entityTypeMap.readEntityCacheTime(id);
|
Hey @danReynolds, sorry I haven't come back to this as of yet. Its still on my list but project took over at work 😓 |
|
You are under no obligation haha I welcome the help but you do you |
Overview
This PR raises helper functions as defined in issue #54.