Skip to content

Commit

Permalink
removed unwraps in project,
Browse files Browse the repository at this point in the history
however, some expect's are still left. but they should be removed
  • Loading branch information
Uewotm90 committed Dec 6, 2023
1 parent eec668b commit 8f5a0e1
Show file tree
Hide file tree
Showing 8 changed files with 76 additions and 38 deletions.
2 changes: 2 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,11 @@ serde_json = "1.0.108"
[build-dependencies]
tonic-build = "0.10.2"


[lints.clippy]
complexity = "deny"
correctness = "deny"
perf = "deny"
suspicious = "warn"
enum_variant_names = "allow"
unwrap_used = "warn"
14 changes: 8 additions & 6 deletions src/api/auth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ pub fn validation_interceptor(mut req: Request<()>) -> Result<Request<()>, Statu
Ok(token_data) => {

Check warning on line 18 in src/api/auth.rs

View workflow job for this annotation

GitHub Actions / cargo fmt

Diff in /home/runner/work/Ecdar-API/Ecdar-API/src/api/auth.rs
req.metadata_mut().insert(
"uid",
metadata::MetadataValue::from_str(&token_data.claims.sub).unwrap(),
metadata::MetadataValue::from_str(&token_data.claims.sub).map_err(|err| Status::internal(err.to_string()))?,
);
Ok(req)
}
Expand Down Expand Up @@ -260,27 +260,29 @@ impl<T> RequestExt for Request<T> {
self.metadata().get("authorization").map(|token| {

Check warning on line 260 in src/api/auth.rs

View workflow job for this annotation

GitHub Actions / cargo fmt

Diff in /home/runner/work/Ecdar-API/Ecdar-API/src/api/auth.rs
token
.to_str()
.unwrap()
.expect("failed to parse token string")//TODO better error handling
.trim_start_matches("Bearer ")
.to_string()
})
}
/// Returns the token string slice from the request metadata.
fn token_str(&self) -> Option<&str> {

Check warning on line 269 in src/api/auth.rs

View workflow job for this annotation

GitHub Actions / cargo fmt

Diff in /home/runner/work/Ecdar-API/Ecdar-API/src/api/auth.rs
match self.metadata().get("authorization") {
Some(token) => Some(token.to_str().unwrap().trim_start_matches("Bearer ")),
//TODO better error handling
Some(token) => Some(token.to_str().expect("failed to parse token string").trim_start_matches("Bearer ")),
None => None,
}
}

/// Returns the uid from the request metadata.

Check warning on line 277 in src/api/auth.rs

View workflow job for this annotation

GitHub Actions / cargo fmt

Diff in /home/runner/work/Ecdar-API/Ecdar-API/src/api/auth.rs
fn uid(&self) -> Option<i32> {
let uid = match self.metadata().get("uid").unwrap().to_str() {
//TODO better error handling
let uid = match self.metadata().get("uid").expect("failed to parse user id").to_str() {
Ok(uid) => uid,
Err(_) => return None,
};

Some(uid.parse().unwrap())
//TODO better error handling
Some(uid.parse().expect("failed to parse user id"))
}
}

Expand Down
69 changes: 52 additions & 17 deletions src/api/ecdar_api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use crate::database::{session_context::SessionContextTrait, user_context::UserCo
use crate::entities::{access, in_use, model, query, session, user};
use chrono::{Duration, Utc};
use regex::Regex;
use sea_orm::SqlErr;
use sea_orm::{DbErr, SqlErr};

Check warning on line 22 in src/api/ecdar_api.rs

View workflow job for this annotation

GitHub Actions / Clippy lint and check

unused import: `DbErr`

warning: unused import: `DbErr` --> src/api/ecdar_api.rs:22:15 | 22 | use sea_orm::{DbErr, SqlErr}; | ^^^^^ | = note: `#[warn(unused_imports)]` on by default

Check warning on line 22 in src/api/ecdar_api.rs

View workflow job for this annotation

GitHub Actions / Clippy lint and check

unused import: `DbErr`

warning: unused import: `DbErr` --> src/api/ecdar_api.rs:22:15 | 22 | use sea_orm::{DbErr, SqlErr}; | ^^^^^ | = note: `#[warn(unused_imports)]` on by default
use serde_json;
use std::sync::Arc;
use tonic::{Code, Request, Response, Status};
Expand All @@ -30,7 +30,39 @@ const IN_USE_DURATION_MINUTES: i64 = 10;
pub struct ConcreteEcdarApi {
contexts: ContextCollection,
}

/// Maps a `DbErr` to a `Status`
// fn to_status(db_err: DbErr) -> Status {
// //TODO Probably a bad idea to return DbErr messages, oh well.
// match db_err.sql_err() {
// Some(serr) => match serr {
// SqlErr::UniqueConstraintViolation(mes) => return Status::new(Code::AlreadyExists, mes),
// SqlErr::ForeignKeyConstraintViolation(mes) => {
// return Status::new(Code::InvalidArgument, mes)
// }
// _ => unreachable!(),
// },
// None => {}
// }
// match db_err {
// DbErr::ConnectionAcquire(err) => Status::from_error(Box::new(err)),
// DbErr::TryIntoErr { from, into, source } => todo!(),
// DbErr::Conn(err) => Status::new(Code::FailedPrecondition, err.to_string()),
// DbErr::Exec(err) => Status::new(Code::Internal, err.to_string()),
// DbErr::Query(err) => Status::new(Code::Internal, err.to_string()),
// DbErr::ConvertFromU64(mes) => todo!(),
// DbErr::UnpackInsertId => todo!(),
// DbErr::UpdateGetPrimaryKey => panic!("unknown error"),
// DbErr::RecordNotFound(mes) => Status::new(Code::NotFound, mes),
// DbErr::AttrNotSet(mes) => Status::new(Code::Internal, mes),
// DbErr::Custom(mes) => Status::new(Code::Unknown, mes),
// DbErr::Type(mes) => Status::new(Code::Internal, mes),
// DbErr::Json(mes) => Status::new(Code::InvalidArgument, mes),
// DbErr::Migration(mes) => todo!(),
// DbErr::RecordNotInserted => todo!(),
// DbErr::RecordNotUpdated => Status::new(Code::NotFound, "No record updated"),
// }
// // todo!()
// }
/// Updates or creates a session in the database for a given user.
///
///
Expand All @@ -52,13 +84,13 @@ pub async fn handle_session(
access_token: access_token.clone(),

Check warning on line 84 in src/api/ecdar_api.rs

View workflow job for this annotation

GitHub Actions / cargo fmt

Diff in /home/runner/work/Ecdar-API/Ecdar-API/src/api/ecdar_api.rs
refresh_token: refresh_token.clone(),
updated_at: Default::default(),
user_id: uid.parse().unwrap(),
user_id: uid.parse().map_err(|err| Status::internal(format!("failed to parse user id (uid) ({err})")))?,
})
.await
.map_err(|err| Status::new(Code::Internal, err.to_string()))?;
} else {

Check warning on line 91 in src/api/ecdar_api.rs

View workflow job for this annotation

GitHub Actions / cargo fmt

Diff in /home/runner/work/Ecdar-API/Ecdar-API/src/api/ecdar_api.rs
let mut session = match session_context
.get_by_token(TokenType::RefreshToken, request.token_string().unwrap())
.get_by_token(TokenType::RefreshToken, request.token_string().ok_or(Status::internal("failed to get token from request metadata"))?)
.await
{
Ok(Some(session)) => session,
Expand All @@ -84,13 +116,13 @@ pub async fn handle_session(

fn is_valid_email(email: &str) -> bool {
Regex::new(r"^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,}$")
.unwrap()
.expect("failed to compile regex")
.is_match(email)
}

fn is_valid_username(username: &str) -> bool {
Regex::new(r"^[a-zA-Z0-9_]{3,32}$")
.unwrap()
.expect("failed to compile regex")
.is_match(username)
}

Expand Down Expand Up @@ -139,7 +171,7 @@ impl EcdarApi for ConcreteEcdarApi {
let model = Model {

Check warning on line 171 in src/api/ecdar_api.rs

View workflow job for this annotation

GitHub Actions / cargo fmt

Diff in /home/runner/work/Ecdar-API/Ecdar-API/src/api/ecdar_api.rs
id: model.id,
name: model.name,
components_info: serde_json::from_value(model.components_info).unwrap(),
components_info: serde_json::from_value(model.components_info).map_err(|err| Status::internal(err.to_string()))?,
owner_id: model.owner_id,
};

Expand All @@ -156,7 +188,7 @@ impl EcdarApi for ConcreteEcdarApi {
let session = self

Check warning on line 188 in src/api/ecdar_api.rs

View workflow job for this annotation

GitHub Actions / cargo fmt

Diff in /home/runner/work/Ecdar-API/Ecdar-API/src/api/ecdar_api.rs
.contexts
.session_context
.get_by_token(TokenType::AccessToken, request.token_string().unwrap())
.get_by_token(TokenType::AccessToken, request.token_string().ok_or(Status::internal("failed to get token from request metadata"))?)
.await
.map_err(|err| Status::new(Code::Internal, err.to_string()))?
.ok_or_else(|| {
Expand Down Expand Up @@ -198,7 +230,7 @@ impl EcdarApi for ConcreteEcdarApi {
model_id: query.model_id,

Check warning on line 230 in src/api/ecdar_api.rs

View workflow job for this annotation

GitHub Actions / cargo fmt

Diff in /home/runner/work/Ecdar-API/Ecdar-API/src/api/ecdar_api.rs
query: query.string,
result: match query.result {
Some(result) => serde_json::from_value(result).unwrap(),
Some(result) => serde_json::from_value(result).expect("failed to parse message"), //TODO better error handling
None => "".to_owned(),
},
outdated: query.outdated,
Expand All @@ -222,7 +254,7 @@ impl EcdarApi for ConcreteEcdarApi {
.ok_or(Status::internal("Could not get uid from request metadata"))?;

Check warning on line 254 in src/api/ecdar_api.rs

View workflow job for this annotation

GitHub Actions / cargo fmt

Diff in /home/runner/work/Ecdar-API/Ecdar-API/src/api/ecdar_api.rs

let components_info = match message.clone().components_info {
Some(components_info) => serde_json::to_value(components_info).unwrap(),
Some(components_info) => serde_json::to_value(components_info).map_err(|err| Status::internal(err.to_string()))?,
None => return Err(Status::invalid_argument("No components info provided")),
};

Expand Down Expand Up @@ -268,19 +300,22 @@ impl EcdarApi for ConcreteEcdarApi {
let session = self
.contexts
.session_context
.get_by_token(TokenType::AccessToken, request.token_string().unwrap())
.get_by_token(TokenType::AccessToken, request.token_string().ok_or(Status::internal("Failed to get token from request metadata"))?)
.await
.unwrap()
.unwrap();
.map_err(|_err| Status::internal("failed to query database"))? //TODO better error message
.ok_or(Status::not_found("token not found"))?;

let in_use = in_use::Model {
model_id: model.clone().id,
session_id: session.id,
latest_activity: Default::default(),
};

self.contexts.in_use_context.create(in_use).await.unwrap();
self.contexts.access_context.create(access).await.unwrap();
self.contexts.in_use_context.create(in_use).await
.map_err(|_err| Status::new(Code::Internal, "failed to create entity"))?;

self.contexts.access_context.create(access).await
.map_err(|_err| Status::new(Code::Internal, "failed to create entity"))?;

Ok(Response::new(CreateModelResponse { id: model.id }))
}
Expand Down Expand Up @@ -336,7 +371,7 @@ impl EcdarApi for ConcreteEcdarApi {
let session = match self
.contexts
.session_context
.get_by_token(TokenType::AccessToken, request.token_string().unwrap())
.get_by_token(TokenType::AccessToken, request.token_string().ok_or(Status::new(Code::Internal,"Failed to get token from request metadata"))?) //? better error message?
.await
{
Ok(Some(session)) => session,
Expand Down Expand Up @@ -383,7 +418,7 @@ impl EcdarApi for ConcreteEcdarApi {
None => model.name,
},
components_info: match message.clone().components_info {
Some(components_info) => serde_json::to_value(components_info).unwrap(),
Some(components_info) => serde_json::to_value(components_info).map_err(|err| Status::new(Code::Internal,err.to_string()))?,
None => model.components_info,
},
owner_id: match message.clone().owner_id {
Expand Down
5 changes: 3 additions & 2 deletions src/api/hashing_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,12 @@ pub trait HashingContextTrait: Send + Sync {
pub struct HashingContext;

impl HashingContextTrait for HashingContext {
//! Methods should not panic, but propogate their result to the caller
fn hash_password(&self, password: String) -> String {
hash(password, DEFAULT_COST).unwrap()
hash(password, DEFAULT_COST).expect("failed to hash password")
}

fn verify_password(&self, password: String, hash: &str) -> bool {
verify(password, hash).unwrap()
verify(password, hash).expect("failed to verify password")
}
}
16 changes: 7 additions & 9 deletions src/api/reveaal_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ pub struct ReveaalContext;
impl ReveaalContext {
async fn get_connection() -> EcdarBackendClient<Channel> {
let url = env::var("REVEAAL_ADDRESS").expect("Expected REVEAAL_ADDRESS to be set.");
EcdarBackendClient::connect(url).await.unwrap()
EcdarBackendClient::connect(url).await.expect("failed to connect to Ecdar backend") //? Perhaps the error handling should be more graceful
}
}

Expand All @@ -25,43 +25,41 @@ impl EcdarBackend for ReveaalContext {
&self,
request: Request<()>,
) -> Result<Response<UserTokenResponse>, Status> {
Ok(ReveaalContext::get_connection()
ReveaalContext::get_connection()
.await
.get_user_token(request)
.await
.unwrap())
}

async fn send_query(
&self,
request: Request<QueryRequest>,
) -> Result<Response<QueryResponse>, Status> {
Ok(ReveaalContext::get_connection()
ReveaalContext::get_connection()
.await
.send_query(request)
.await
.unwrap())

}

async fn start_simulation(
&self,
request: Request<SimulationStartRequest>,
) -> Result<Response<SimulationStepResponse>, Status> {
Ok(ReveaalContext::get_connection()
ReveaalContext::get_connection()
.await
.start_simulation(request)
.await
.unwrap())

}

async fn take_simulation_step(
&self,
request: Request<SimulationStepRequest>,
) -> Result<Response<SimulationStepResponse>, Status> {
Ok(ReveaalContext::get_connection()
ReveaalContext::get_connection()
.await
.take_simulation_step(request)
.await
.unwrap())
}
}
2 changes: 1 addition & 1 deletion src/api/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ pub async fn start_grpc_server(
let addr = env::var("API_ADDRESS")
.expect("Expected API_ADDRESS to be set.")
.parse()
.unwrap();
.expect("Failed to parse IP address");

println!("Starting grpc server on '{}'", addr);

Expand Down
4 changes: 2 additions & 2 deletions src/database/database_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ impl PostgresDatabaseContext {
#[async_trait]
impl DatabaseContextTrait for PostgresDatabaseContext {
async fn reset(&self) -> Result<Arc<dyn DatabaseContextTrait>, DbErr> {
Migrator::fresh(&self.db_connection).await.unwrap();
Migrator::fresh(&self.db_connection).await.expect("failed to connect to database");

Ok(Arc::new(PostgresDatabaseContext {
db_connection: self.get_connection(),
Expand Down Expand Up @@ -64,7 +64,7 @@ impl SQLiteDatabaseContext {
#[async_trait]
impl DatabaseContextTrait for SQLiteDatabaseContext {
async fn reset(&self) -> Result<Arc<dyn DatabaseContextTrait>, DbErr> {
Migrator::fresh(&self.db_connection).await.unwrap();
Migrator::fresh(&self.db_connection).await.expect("failed to connect to database");

Ok(Arc::new(SQLiteDatabaseContext {
db_connection: self.get_connection(),
Expand Down
2 changes: 1 addition & 1 deletion src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ async fn main() -> Result<(), Box<dyn Error>> {
hashing_context: Arc::new(HashingContext),
};

start_grpc_server(contexts).await.unwrap();
start_grpc_server(contexts).await.expect("failed to start grpc server");

Ok(())
}

0 comments on commit 8f5a0e1

Please sign in to comment.