I wrote a lot but I didn't see any big problems in your code, you could honestly ignore all of this and be totally fine from what I saw.
You can use assert_eq!(x, y) instead of assert!(x == y).
I write my unit tests in the same file where the thing being tested is, but if you want a separate module for them, you can just make the whole module #[cfg(test)]:
#[cfg(test)]
mod testing;
instead of having to put it in each test file.
You can nest imports:
Instead of
use crate::domain::model::SessionData;
use crate::domain::model::User;
use crate::domain::repository::ResultSession;
use crate::domain::repository::ResultUser;
use crate::domain::repository::SessionError;
use crate::domain::repository::SessionRepository;
use crate::domain::repository::UserRegistrationError;
you can write
use crate::domain::{
model::{SessionData, User},
repository::{
ResultSession, ResultUser, SessionError, SessionRepository, UserRegistrationError,
},
};
use chrono::NaiveDateTime;
use mysql::{error::Error::MySqlError, params, prelude::Queryable, Pool};
use mysql_common::Row;
or whatever other scheme you prefer. Of course, preferring the explicit one line per import style is fine as well. You can enforce your preferred import formatting with rustfmt configuration options, though you will need to use the nightly toolchain to format with them (cargo +nightly fmt, you can probably configure your editor to do this on save; I've done so on vscode). I use
imports_granularity = "Crate"
group_imports = "One"
in all my projects, but that's just my personal preference.
I prefer using a library like diesel or sqlx for database stuff. Both will enable compile time checks for your queries to make maintaining your code easier, though they employ very different strategies to do so. I personally prefer diesel, but both have their pros and cons and sqlx is certainly easier to get started with. SeaORM is another option but I haven't tried it.
You can use a library like thiserror or anyhow to make the error handling more convenient. thiserror enables you to have an enum with variants for each error that can occur, whereas anyhow lets you turn every error into the library's error type and attach some error message with context. The common advice is to use thiserror for libraries (to allow consumers of the library to match on the error type and choose what to do about each kind of error) and anyhow for binaries (because it's often not important to know the exact underlying error type in an application).
The modules are a little too nested for my taste. For example, src/domain/repository/mod.rs is just
mod sessionrepository;
pub use sessionrepository::ResultSession;
pub use sessionrepository::ResultUser;
pub use sessionrepository::SessionError;
pub use sessionrepository::SessionRepository;
pub use sessionrepository::UserRegistrationError;
Instead of this, I would just have src/domain/sessionrepository.rs.
I don't think the #[serde(crate = "rocket::serde")] attributes are necessary.
There's some ownership related stuff that could be handled more idiomatically, for example
impl UserResponse {
pub fn new(user: &User) -> Self {
Self {
name: user.name.to_owned(),
email: user.email.to_owned(),
id: user.id,
created: user.created,
}
}
}
could take ownership of the argument to avoid having to clone the name and email, since you never need the user after converting it into a response
impl UserResponse {
pub fn new(user: User) -> Self {
Self {
name: user.name,
email: user.email,
id: user.id,
created: user.created,
}
}
}
but it's also totally fine as is. Ownership is one of the hardest parts of Rust to learn, and it's a good idea to use .clone() to make things work especially when starting out, rather than get stuck trying to micro-optimize things that probably won't matter. I just figured I'd give one example.
, but I do prefer them nested too.
).