-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Move find_all to entity_api #15
Changes from all commits
4e25708
820a507
4d6b653
92f9727
7a065c5
8d17395
5e0b8bb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,66 @@ | ||
use std::error::Error as StdError; | ||
use std::fmt; | ||
|
||
use sea_orm::error::DbErr; | ||
|
||
/// Errors while executing operations related to entities. | ||
/// The intent is to categorize errors into two major types: | ||
/// * Errors related to data. Ex DbError::RecordNotFound | ||
/// * Errors related to interactions with the database itself. Ex DbError::Conn | ||
#[derive(Debug)] | ||
pub struct Error { | ||
// Underlying error emitted from seaORM internals | ||
pub inner: DbErr, | ||
// Enum representing which category of error | ||
pub error_type: EntityApiError, | ||
} | ||
|
||
#[derive(Debug)] | ||
pub enum EntityApiError { | ||
DatabaseConnectionLost, | ||
// Record not found | ||
RecordNotFound, | ||
// Record not updated | ||
RecordNotUpdated, | ||
// Errors related to interactions with the database itself. Ex DbError::Conn | ||
SystemError, | ||
} | ||
|
||
impl fmt::Display for Error { | ||
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { | ||
write!(f, "Entity API Error: {:?}", self) | ||
} | ||
} | ||
|
||
impl StdError for Error {} | ||
|
||
impl From<DbErr> for Error { | ||
fn from(err: DbErr) -> Self { | ||
match err { | ||
DbErr::RecordNotFound(_) => Error { | ||
inner: err, | ||
error_type: EntityApiError::RecordNotFound, | ||
}, | ||
DbErr::RecordNotUpdated => Error { | ||
inner: err, | ||
error_type: EntityApiError::RecordNotUpdated, | ||
}, | ||
DbErr::ConnectionAcquire(_) => Error { | ||
inner: err, | ||
error_type: EntityApiError::SystemError, | ||
}, | ||
DbErr::Conn(_) => Error { | ||
inner: err, | ||
error_type: EntityApiError::DatabaseConnectionLost, | ||
}, | ||
DbErr::Exec(_) => Error { | ||
inner: err, | ||
error_type: EntityApiError::SystemError, | ||
}, | ||
_ => Error { | ||
inner: err, | ||
error_type: EntityApiError::SystemError, | ||
}, | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,8 @@ | ||
use sea_orm::DatabaseConnection; | ||
use service::AppState; | ||
|
||
pub mod error; | ||
pub mod organization; | ||
|
||
pub async fn seed_database(db: &DatabaseConnection) { | ||
organization::seed_database(db).await; | ||
pub async fn seed_database(app_state: &AppState) { | ||
organization::seed_database(app_state).await; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,6 +4,7 @@ use axum::response::IntoResponse; | |
use axum::Json; | ||
use entity::organization; | ||
use entity::organization::Entity as Organization; | ||
use entity_api::organization as OrganizationApi; | ||
use sea_orm::entity::EntityTrait; | ||
use sea_orm::ActiveModelTrait; | ||
use sea_orm::ActiveValue::{NotSet, Set}; | ||
|
@@ -21,10 +22,9 @@ impl OrganizationController { | |
/// --request GET \ | ||
/// http://localhost:4000/organizations | ||
pub async fn index(State(app_state): State<AppState>) -> impl IntoResponse { | ||
let organizations = organization::Entity::find() | ||
.all(&app_state.database_connection.unwrap()) | ||
let organizations = OrganizationApi::find_all(&app_state) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think I prefer keeping the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's ok, this was only recognizing that |
||
.await | ||
.unwrap_or(vec![]); | ||
.unwrap_or_default(); | ||
|
||
Json(organizations) | ||
} | ||
|
@@ -36,10 +36,10 @@ impl OrganizationController { | |
pub async fn read(State(app_state): State<AppState>, Path(id): Path<i32>) -> impl IntoResponse { | ||
debug!("GET Organization by id: {}", id); | ||
|
||
let organization: Option<organization::Model> = organization::Entity::find_by_id(id) | ||
.one(&app_state.database_connection.unwrap()) | ||
.await | ||
.unwrap_or_default(); | ||
let organization: Result<Option<organization::Model>, Error> = | ||
OrganizationApi::find_by_id(&app_state, id) | ||
.await | ||
.map_err(|err| err.into()); | ||
|
||
Json(organization) | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,27 +1,57 @@ | ||
use std::error::Error as StdError; | ||
|
||
use axum::http::StatusCode; | ||
use axum::response::{IntoResponse, Response}; | ||
use serde::Serialize; | ||
|
||
use entity_api::error::EntityApiError; | ||
use entity_api::error::Error as EntityApiErrorSuper; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's talk through this when we meet later today. I think you've definitely exposed a part of how this is set up that could use more thought. I'm not sure I love adding the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Indeed, I borrowed the |
||
|
||
pub type Result<T> = core::result::Result<T, Error>; | ||
|
||
#[derive(Debug)] | ||
#[derive(Debug, Serialize)] | ||
|
||
pub enum Error { | ||
DatabaseConnectionLost, | ||
InternalServer, | ||
EntityNotFound, | ||
UnprocessableEntity, | ||
} | ||
|
||
impl StdError for Error {} | ||
|
||
impl std::fmt::Display for Error { | ||
fn fmt(&self, fmt: &mut std::fmt::Formatter) -> core::result::Result<(), std::fmt::Error> { | ||
write!(fmt, "{self:?}") | ||
} | ||
} | ||
|
||
impl IntoResponse for Error { | ||
fn into_response(self) -> Response { | ||
match self { | ||
Error::DatabaseConnectionLost => ( | ||
StatusCode::INTERNAL_SERVER_ERROR, | ||
"DB CONNECTION LOST - INTERNAL SERVER ERROR", | ||
) | ||
.into_response(), | ||
Error::InternalServer => { | ||
(StatusCode::INTERNAL_SERVER_ERROR, "INTERNAL SERVER ERROR").into_response() | ||
} | ||
Error::EntityNotFound => (StatusCode::NOT_FOUND, "ENTITY NOT FOUND").into_response(), | ||
Error::UnprocessableEntity => { | ||
(StatusCode::UNPROCESSABLE_ENTITY, "UNPROCESSABLE ENTITY").into_response() | ||
} | ||
} | ||
} | ||
} | ||
|
||
impl std::fmt::Display for Error { | ||
fn fmt(&self, fmt: &mut std::fmt::Formatter) -> core::result::Result<(), std::fmt::Error> { | ||
write!(fmt, "{self:?}") | ||
impl From<EntityApiErrorSuper> for Error { | ||
fn from(err: EntityApiErrorSuper) -> Self { | ||
match err.error_type { | ||
EntityApiError::DatabaseConnectionLost => Error::DatabaseConnectionLost, | ||
EntityApiError::RecordNotFound => Error::EntityNotFound, | ||
EntityApiError::RecordNotUpdated => Error::UnprocessableEntity, | ||
EntityApiError::SystemError => Error::InternalServer, | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this a lot. I'm curious, though, if we're now only transforming the error up one level in the actual controller? I think we'd need to write a test to determine this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you figured this out better in your other branch. Wrapping in
Ok()
is a valid solution because theResult<>
is unwrapped from await(), but we need it to be in aResult<>
for both theOk
orErr
cases.But having a naked
?
operator is always going to be my favorite solution syntactically, so I'm excited to see what you came up with.