Commit 92bd4bb
[Rust] Capability based DbContext (#5307)
# Description of Changes
An evolution of #4707
where i explored adding a `db_read_only` method to `DbContext`. (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
`DbView` and 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:
```rust
pub trait CtxDbRead {
fn db_read_only(&self) -> &LocalReadOnly;
}
pub trait CtxDbWrite: CtxDbRead {
fn db(&self) -> &Local;
}
pub trait CtxWithSender {
fn sender(&self) -> Identity;
}
pub trait CtxWithTimestamp {
fn timestamp(&self) -> Timestamp;
}
pub trait CtxWithHttp {
fn http(&self) -> &HttpClient;
}
```
## 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`.
```rust
impl<Db: CtxDbRead> Authorization for Db {
fn test(&self,args:Args) {
self.db_read_only().do_i_have_perms().find(self.sender()); //ERROR: no sender method
}
```
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:
```rust
impl<Db: CtxDbRead+CtxWithSender> Authorization for Db {
fn test(&self,args:Args) {
self.db_read_only().do_i_have_perms().find(self.sender()); //WORKS NOW YAY
}
```
Additonally there could be also a `+ CtxWithTimestamp` if you wanted to
for example store a last logged in date or smth (you get the idea)
Now this is far better because `.sender` is available for `ViewContext`
for example so you can authorize with the same method.
## Alternatives/Bikeshedding
I chose the names CtxWith because really the `Contexts` are the common
denominator and not the `Database`.
Thats also the reason why its `CtxDbRead` because 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 `DbContext` but this can also be
removed if desired.
# Expected complexity level and risk
1. Additive change with extremly minimal surface
# Testing
- [x] Works for my project
---------
Signed-off-by: Kilian Strunz <93079615+kistz@users.noreply.github.com>
Co-authored-by: Phoebe Goldman <phoebe@goldman-tribe.org>1 parent b44156d commit 92bd4bb
5 files changed
Lines changed: 333 additions & 34 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
13 | 13 | | |
14 | 14 | | |
15 | 15 | | |
16 | | - | |
| 16 | + | |
17 | 17 | | |
18 | 18 | | |
19 | 19 | | |
| |||
22 | 22 | | |
23 | 23 | | |
24 | 24 | | |
25 | | - | |
| 25 | + | |
26 | 26 | | |
27 | 27 | | |
28 | | - | |
| 28 | + | |
29 | 29 | | |
30 | 30 | | |
31 | 31 | | |
| |||
72 | 72 | | |
73 | 73 | | |
74 | 74 | | |
75 | | - | |
| 75 | + | |
76 | 76 | | |
77 | 77 | | |
78 | 78 | | |
| |||
109 | 109 | | |
110 | 110 | | |
111 | 111 | | |
112 | | - | |
| 112 | + | |
113 | 113 | | |
114 | 114 | | |
115 | 115 | | |
| |||
121 | 121 | | |
122 | 122 | | |
123 | 123 | | |
124 | | - | |
| 124 | + | |
125 | 125 | | |
126 | 126 | | |
127 | 127 | | |
| |||
148 | 148 | | |
149 | 149 | | |
150 | 150 | | |
151 | | - | |
| 151 | + | |
152 | 152 | | |
153 | 153 | | |
154 | 154 | | |
155 | 155 | | |
156 | 156 | | |
157 | 157 | | |
158 | 158 | | |
159 | | - | |
| 159 | + | |
160 | 160 | | |
161 | 161 | | |
162 | 162 | | |
| |||
0 commit comments