Closed mellowagain closed 2 years ago
Hey! I want to help with this issue, can you explain it a little bit more to me?
Thank you very much! Many routes contain repeating code such as:
let repo_owner = User::find_using_name(&uri.username, &mut transaction).await.ok_or_else(|| err!(NOT_FOUND, "Repository not found"))?;
let repo = Repository::open(repo_owner, &uri.repository, &mut transaction).await.ok_or_else(|| err!(NOT_FOUND, "Repository not found"))?;
if !privilege::check_access(&repo, web_user.as_ref(), &mut transaction).await? { die!(NOT_FOUND, "Not found"); }
* Branch existance check
```rust
let loose_ref = match gitoxide_repo.refs.find_loose(tree_name) {
Ok(loose_ref) => Ok(loose_ref),
Err(GitoxideFindError::Find(err)) => Err(err),
Err(GitoxideFindError::NotFound(_)) => die!(NOT_FOUND, "Not found")
}?; // Handle 404
I've been thinking about moving these checks into middlewares (I initially wrote guards but it turns out that middlewares are better for this use case than guards) so the routes can be decorated with it, thus removing the repeating code in most routes.
You can find more information on how to implement middlewares in actix-web here. There is also a very useful blog post on how to implement it.
If you require more information, feel free to comment again and I'll try providing it. If you need help, also feel free to reach out and I'll see what we can do.
Trying to implement this myself, I've hit a wall as path arguments are not accessible in middlewares, as they're not processed at that point (see actix/actix-web#2096). I've pushed my code into the repo-middleware
branch for now and putting this on halt.
I've still got an idea to add a FromRequest
to Repository
itself, which might end up working. I'll try it out at a later point.
We should move common operations in routes to guards to cleanup the actual route body to restrict to mostly stuff that is also route specific. I thought the following would be good to move the following checks to a guard: