[Rust] Capability based DbContext#5307
Open
kistz wants to merge 4 commits into
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Description of Changes
An evolution of #4707 where i explored adding a
db_read_onlymethod toDbContext. (hence i would appreciate your thoughts @gefjon )This was the wrong appraoch in retrospect because there are still some annoyances with this.
Furthermore there is really no benefit to adding the associated type
DbViewand is just making the ergonomics worse.So here is the new approach which solved all my problems:
Making a trait for every capability which the various contexts are providing because the Databse is only one of them.
These capabilities are (and pretty much the whole relevant div for this pr because the impl blocks are trivial) and should be self explanatory:
Why is this relevant?
Lets look at an example building on the previous pr:
You have abstracted your code in a trait which you can call for every context e.g. authorization.
Now this does not work because the sender method is not available on
DbContext.Now this is really annoying since you now have to pass additional parameters to the method.
Instead we can now specific these capabilities in the type system:
Additonally there could be also a
+ CtxWithTimestampif you wanted to for example store a last logged in date or smth (you get the idea)Now this is far better because
.senderis available forViewContextfor example so you can authorize with the same method.Alternatives/Bikeshedding
I chose the names CtxWith because really the
Contextsare the common denominator and not theDatabase.Thats also the reason why its
CtxDbReadbecause you are expressing: "All context where i get read access to the databse (e.g. everything).Other names have felt worse.
Also the deprecation can be removed but i think this approach is strictly superior and i dont think there are currently many people relying on it.
API and ABI breaking changes
None. one deprecation for the old
DbContextbut this can also be removed if desired.Expected complexity level and risk
Testing