Allow swappable lock identity providers #22
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Rationale
We often see orphaned locks left in Redis. The hostname, while useful, is not surfaced prominently in AWS tooling.
Having the ability to override this and use an AWS ARN or similar alongside each lock would make it far easier to determine if a lock has been orphaned and is safe to manually release and also sets the stage for an automated solution. Hostnames are also routinely recycled, which can hide orphaned locks.
Implementing swappable identity providers allows us to choose metadata that better serves us when debugging locks.
Current state
The only current option is to wrap
local_redislock\lock\redis_lock_factoryinside another plugin and override methods, or to use other brittle hacks to mutate the lock value. This kind of approach is not ideal as it's fragile across upgrades and tough to test.Implementation
Since modern Moodle has a DI container, and exposes the
di_configurationhook which allows plugins to override theidentity_providerbinding - using DI is the cleanest approach. A param has also been added to the constructor to inject the identity service, so that if Moodle ever adds lock factories to the DI container properly, we can use DI the intended way (instead of doing a "service locator-ish"DI::getin the constructor.