[Refactoring] Move DbConn out PlumeRocket #797

오픈
igalic4 년 전을 오픈 · 1개의 코멘트
igalic 코멘트됨, 4 년 전
소유자

Currently, we pass the struct PlumeRocket to most routes:

/// Common context needed by most routes and operations on models
pub struct PlumeRocket {
    pub conn: DbConn,
    pub intl: rocket_i18n::I18n,
    pub user: Option<users::User>,
    pub searcher: Arc<search::Searcher>,
    pub worker: Arc<ScheduledThreadPool>,
    pub flash_msg: Option<(String, String)>,
}

The reason for refactoring (#462) was clippy warning us about too many arguments to our route functions.
so we collected all the data that clumped together in the parameter lists, into a single Object.

This is presenting problems, when trying to migrate to async rocket.


I think we should extract DbConn again.
This should (could?) allow us to use #[database] guard, making our async migration slightly less pain-freer.

What do we need to do?

  • decorate DbConn with #[database]
  • identify all route functions that use rockets.conn, and
  • add a parameter dbconn, replacing it
  • once all uses are replaced, remove DbConn from PlumeRockets

Benefits? Drawbacks? Alternatives?
Discuss!

Currently, we pass the `struct` `PlumeRocket` to most routes: ```rust /// Common context needed by most routes and operations on models pub struct PlumeRocket { pub conn: DbConn, pub intl: rocket_i18n::I18n, pub user: Option<users::User>, pub searcher: Arc<search::Searcher>, pub worker: Arc<ScheduledThreadPool>, pub flash_msg: Option<(String, String)>, } ``` The reason for refactoring (https://git.joinplu.me/Plume/Plume/pulls/462) was clippy warning us about too many arguments to our route functions. so we collected all the [data that clumped together](https://refactoring.guru/smells/data-clumps) in the parameter lists, into a single Object. This is [presenting problems](https://git.joinplu.me/Plume/Plume/pulls/777), when trying to [migrate to async rocket](https://git.joinplu.me/Plume/Plume/pulls/730). ----- I think we should extract DbConn again. This should (could?) allow us to use [`#[database]` guard](https://github.com/SergioBenitez/Rocket/pull/1343), making our `async` migration slightly less pain-freer. What do we need to do? - [x] decorate `DbConn` with `#[database]` - [ ] identify all route functions that *use* `rockets.conn`, and - [ ] add a parameter `dbconn`, replacing it - [ ] once all uses are replaced, remove `DbConn` from `PlumeRockets` ------ Benefits? Drawbacks? Alternatives? Discuss!
kiwii 코멘트됨, 4 년 전
소유자

What I can tell out of my head is:

  • benefits:
    • we can clearly see which routes use the database, and which don't
    • we will probably have less custom code if we use the #[database] attribute that generates some code for us, I guess.
  • drawbacks:
    • not sure it is the case, but maybe we won't be able to migrate to sqlx with the #[database] attribute, I don't know what types SQLx supports, and if they are compatible with Rocket's attribute.
  • alternatives:
    • keeping it the way it is? but that makes the async migration harder, so it is not really an option

Clippy shouldn't be unhappy either, it is just one parameter, so I'm all for it!

What I can tell out of my head is: - benefits: - we can clearly see which routes use the database, and which don't - we will probably have less custom code if we use the `#[database]` attribute that generates some code for us, I guess. - drawbacks: - not sure it is the case, but maybe we won't be able to migrate to `sqlx` with the `#[database]` attribute, I don't know what types SQLx supports, and if they are compatible with Rocket's attribute. - alternatives: - keeping it the way it is? but that makes the async migration harder, so it is not really an option Clippy shouldn't be unhappy either, it is just one parameter, so I'm all for it!
kiwii " 커밋 4 년 전에서 이 이슈 언급"
igalic 자체적으로 할당됨 4 년 전
로그인하여 이 대화에 참여
마일스톤 없음
담당자 없음
참여자 2명
알림
마감일
기한이 올바르지 않거나 범위를 벗어났습니다. 'yyyy-mm-dd'형식을 사용해주십시오.

마감일이 설정되지 않았습니다.

의존성

No dependencies set.

Reference: Plume/Plume#797
불러오는 중...
아직 콘텐츠가 없습니다.