Conversation
|
| ]).then(async ([isHardDeleted, isTagInvalidated]) => { | ||
| if (isHardDeleted) { | ||
| // Immediately delete from all layers and return false | ||
| await this.#deleteFromAllLayers(item.entry.getKey()) |
There was a problem hiding this comment.
Not super happy about performing a side-effect in a method which main purpose is to return a boolean indicating whether an entry is still valid.
If you have a better idea let me know :)
There was a problem hiding this comment.
Not a huge problem imo. We are also doing this in isTagInvalidated. But agree with you that its semantically incorrect
|
Thanks a lot for the PR and sorry forlate reply, i have been pretty busy these last weeks... Just a quick ( and maybe naive) question: instead of doing a double round-trip to check hard and soft tag keys separately, what about storing some metadata inside the tag value? Something like: await this.stack.set('___bc:t:users', { timestamp, type: 'hard' | 'soft' }) |
|
@Julien-R44 Any thought on how we should solve the naming conflict of
|
As already outlined in #71, in my opinion the current naming of the
deleteByTagoperation is a bit misleading, since it doesn't hard delete cache entries by tag matching, but rather expires them. That's why I included a commit in this PR renaming the currentdeleteByTagoperation toexpireByTagto be compliant with the naming of the key based operations.I'm aware that this means a breaking change in the API and I'm open to discuss how to resolve this if a major release is to avoid.
The main concern of this PR is the implementation of a new operation
deleteByTag, which performs a "kind of" hard delete via a lazy deletion logic via invalidation timestamps similar to the approach of theexpireByTag(previouslydeleteByTag) operation.Here I'm also open to discuss the best approach for the hard deletion logic.