From 95e5c47c69006e06086ec43b3401e0674f98e59c Mon Sep 17 00:00:00 2001 From: Rohan Sircar Date: Thu, 20 May 2021 18:39:20 +0530 Subject: [PATCH] remove unbounded get users api endpoint --- Cargo.toml | 4 --- README.md | 15 +----------- src/actions/users.rs | 23 +++--------------- src/lib.rs | 11 ++------- src/main.rs | 10 +++++--- src/models/users.rs | 8 +++--- src/routes/users.rs | 50 +++++++++----------------------------- static/dummy.js | 0 tests/integration/users.rs | 17 ++++++------- 9 files changed, 36 insertions(+), 102 deletions(-) create mode 100644 static/dummy.js diff --git a/Cargo.toml b/Cargo.toml index 0869869..1727b3f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -12,18 +12,14 @@ actix-http = "2.2.0" bytes = "1.0.1" futures = "0.3.14" serde_json = "1.0.64" -# json = "0.12.4" -# listenfd = "0.3.3" dotenv = "0.15.0" r2d2 = "0.8.9" -# jsonwebtoken = "7.2.0" actix-identity = "0.3.1" actix-web-httpauth = "0.5.1" rand = "0.8.3" nanoid = "0.4.0" bcrypt = "0.9.0" timeago = "0.3.0" -# comp = "0.2.1" regex = "1.4.5" lazy_static = "1.4.0" lazy-regex = "0.1.4" diff --git a/README.md b/README.md index 3e9c0cd..2fda24e 100644 --- a/README.md +++ b/README.md @@ -40,7 +40,7 @@ curl -X GET http://localhost:7800/api/users ``` curl -H "content-type: application/json" \ --X POST \ +-X PUT \ -i http://localhost:7800/api/users \ --data '{"name":"user4","password":"test"}' ``` @@ -66,19 +66,6 @@ curl -H "content-type: application/json" \ ] ``` -### DTO Validation - -``` -curl -H "content-type: application/json" \ --X POST \ --i http://localhost:7800/api/users \ ---data '{"name":"abc","password":"test"}' # min length for name is 4 -``` - -``` -ValidationErrors({"name": Field([ValidationError { code: "length", message: None, params: {"value": String("abc"), "min": Number(4), "max": Number(10)} }])}) -``` - ## Memory Usage Memory usage as compared to interpreted languages was my primary motivation for looking into rust as a backend language. As of writing, the demo app uses less than 50MB of memory. diff --git a/src/actions/users.rs b/src/actions/users.rs index 3ba715d..dd05e84 100644 --- a/src/actions/users.rs +++ b/src/actions/users.rs @@ -1,6 +1,6 @@ use diesel::prelude::*; -use crate::models::{self, Pagination, UserId}; +use crate::models::{self, Pagination, UserId, Username}; use crate::{errors, models::Password}; use bcrypt::{hash, verify, DEFAULT_COST}; use validators::prelude::*; @@ -21,7 +21,7 @@ pub fn find_user_by_uid( } pub fn _find_user_by_name( - user_name: String, + user_name: Username, conn: &impl diesel::Connection, ) -> Result, errors::DomainError> { use crate::schema::users::dsl::*; @@ -33,15 +33,6 @@ pub fn _find_user_by_name( Ok(maybe_user?) } -pub fn get_all( - conn: &impl diesel::Connection, -) -> Result, errors::DomainError> { - use crate::schema::users::dsl::*; - Ok(users - .select(users::all_columns()) - .load::(conn)?) -} - // def findAll(userId: Long, limit: Int, offset: Int) = db.run { // for { // comments <- query.filter(_.creatorId === userId) @@ -56,12 +47,10 @@ pub fn get_all( // ) // } -pub fn get_users_paginated( - // user_id: UserId, +pub fn get_all_users( pagination: &Pagination, conn: &impl diesel::Connection, ) -> Result, errors::DomainError> { - // use crate::schema::users::dsl::*; Ok(query::_paginate_result(&pagination).load::(conn)?) } @@ -71,12 +60,6 @@ pub fn search_users( conn: &impl diesel::Connection, ) -> Result, errors::DomainError> { use crate::schema::users::dsl::*; - // Ok(users - // .filter(name.like(format!("%{}%", query))) - // .order_by(created_at) - // .offset(pagination.calc_offset().as_uint().into()) - // .limit(pagination.limit.as_uint().into()) - // .load::(conn)?) Ok(query::_paginate_result(&pagination) .filter(name.like(format!("%{}%", query))) .load::(conn)?) diff --git a/src/lib.rs b/src/lib.rs index 8edbb0b..28e0948 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -69,24 +69,17 @@ pub fn configure_app(app_data: AppData) -> Box { web::scope("/api") .service( web::scope("/users") - .route( - "", - web::get().to(routes::users::get_all_users), - ) + .route("", web::get().to(routes::users::get_users)) .route( "/search", web::get().to(routes::users::search_users), ) - .route("", web::post().to(routes::users::add_user)) + .route("", web::put().to(routes::users::add_user)) .route( "/{user_id}", web::get().to(routes::users::get_user), ), ) - .route( - "/pagination", - web::get().to(routes::users::get_users_paginated), - ) .route( "/build-info", web::get().to(routes::misc::build_info_req), diff --git a/src/main.rs b/src/main.rs index 6d53319..4d49ed2 100755 --- a/src/main.rs +++ b/src/main.rs @@ -5,6 +5,7 @@ use diesel_tracing::sqlite::InstrumentedSqliteConnection; use io::ErrorKind; use std::io; use tracing::subscriber::set_global_default; +use tracing_appender::non_blocking::WorkerGuard; use tracing_bunyan_formatter::{BunyanFormattingLayer, JsonStorageLayer}; use tracing_log::LogTracer; use tracing_subscriber::fmt::format::FmtSpan; @@ -30,7 +31,8 @@ async fn main() -> io::Result<()> { ) })?; - let _ = setup_logger(env_config.logger_format)?; + //bind guard to variable instead of _ + let _guard = setup_logger(env_config.logger_format)?; let connspec = &env_config.database_url; let manager = @@ -69,7 +71,7 @@ async fn main() -> io::Result<()> { actix_demo::run(format!("{}:7800", env_config.http_host), app_data).await } -pub fn setup_logger(format: LoggerFormat) -> io::Result<()> { +pub fn setup_logger(format: LoggerFormat) -> io::Result { let env_filter = EnvFilter::try_from_env("ACTIX_DEMO_RUST_LOG").map_err(|err| { io::Error::new( @@ -115,6 +117,8 @@ pub fn setup_logger(format: LoggerFormat) -> io::Result<()> { .with_span_events(FmtSpan::NEW) .with_span_events(FmtSpan::CLOSE) .with_env_filter(env_filter) + .with_writer(non_blocking) + .with_thread_names(true) .finish(); let _ = set_global_default(subscriber).map_err(|err| { io::Error::new( @@ -124,5 +128,5 @@ pub fn setup_logger(format: LoggerFormat) -> io::Result<()> { })?; } }; - Ok(()) + Ok(_guard) } diff --git a/src/models/users.rs b/src/models/users.rs index 318ee3f..412d03e 100644 --- a/src/models/users.rs +++ b/src/models/users.rs @@ -167,17 +167,17 @@ impl Pagination { } #[derive(Debug, Clone, Deserialize)] -pub struct SearchQuery(String); +pub struct SearchQueryString(String); -impl SearchQuery { +impl SearchQueryString { pub fn as_str(&self) -> &str { &self.0 } } #[derive(Debug, Clone, Deserialize)] -pub struct UserSearchRequest { - pub q: SearchQuery, +pub struct SearchQuery { + pub q: SearchQueryString, // pub pagination: Pagination } diff --git a/src/routes/users.rs b/src/routes/users.rs index 1fcc88c..fa33401 100644 --- a/src/routes/users.rs +++ b/src/routes/users.rs @@ -2,10 +2,9 @@ use actix_web::{web, HttpResponse}; use crate::{ actions, - models::{self, ApiResponse, Pagination, UserId, UserSearchRequest}, + models::{self, ApiResponse, Pagination, SearchQuery, UserId}, }; use crate::{errors::DomainError, AppData}; -use actix_web::error::ResponseError; /// Finds user by UID. #[tracing::instrument( @@ -60,33 +59,8 @@ pub async fn get_user( // } // } -///List all users #[tracing::instrument(level = "debug", skip(app_data))] -pub async fn get_all_users( - app_data: web::Data, -) -> Result { - // use web::block to offload blocking Diesel code without blocking server thread - let users = web::block(move || { - let pool = &app_data.pool; - let conn = pool.get()?; - actions::get_all(&conn) - }) - .await - .map_err(|err| DomainError::new_thread_pool_error(err.to_string()))?; - - let _ = tracing::trace!("{:?}", users); - - if !users.is_empty() { - Ok(HttpResponse::Ok().json(ApiResponse::successful(users))) - } else { - Err(DomainError::new_entity_does_not_exist_error( - "No users available".to_owned(), - )) - } -} - -#[tracing::instrument(level = "debug", skip(app_data))] -pub async fn get_users_paginated( +pub async fn get_users( app_data: web::Data, pagination: web::Query, ) -> Result { @@ -95,7 +69,7 @@ pub async fn get_users_paginated( let pool = &app_data.pool; let conn = pool.get()?; let p: Pagination = pagination.into_inner(); - actions::get_users_paginated(&p, &conn) + actions::get_all_users(&p, &conn) }) .await .map_err(|err| DomainError::new_thread_pool_error(err.to_string()))?; @@ -108,7 +82,7 @@ pub async fn get_users_paginated( #[tracing::instrument(level = "debug", skip(app_data))] pub async fn search_users( app_data: web::Data, - query: web::Query, + query: web::Query, pagination: web::Query, ) -> Result { let _ = tracing::info!("Search users request"); @@ -126,21 +100,21 @@ pub async fn search_users( Ok(HttpResponse::Ok().json(ApiResponse::successful(users))) } -/// Inserts new user with name defined in form. +/// Inserts a new user #[tracing::instrument(level = "debug", skip(app_data))] pub async fn add_user( app_data: web::Data, form: web::Json, -) -> Result { - // use web::block to offload blocking Diesel code without blocking server thread - web::block(move || { +) -> Result { + let user = web::block(move || { let pool = &app_data.pool; let conn = pool.get()?; actions::insert_new_user(form.0, &conn, Some(app_data.config.hash_cost)) }) .await - .map(|user| { - let _ = tracing::trace!("{:?}", user); - HttpResponse::Created().json(ApiResponse::successful(user)) - }) + .map_err(|err| DomainError::new_thread_pool_error(err.to_string()))?; + + let _ = tracing::trace!("{:?}", user); + + Ok(HttpResponse::Created().json(ApiResponse::successful(user))) } diff --git a/static/dummy.js b/static/dummy.js new file mode 100644 index 0000000..e69de29 diff --git a/tests/integration/users.rs b/tests/integration/users.rs index 1813f13..0bdb557 100644 --- a/tests/integration/users.rs +++ b/tests/integration/users.rs @@ -12,19 +12,16 @@ mod tests { use super::*; #[actix_rt::test] - async fn should_return_error_message_if_no_users_exist() { - let req = test::TestRequest::get().uri("/api/users").to_request(); + async fn should_return_empty_array_if_no_users_exist() { + let req = test::TestRequest::get() + .uri("/api/users?page=0&limit=2") + .to_request(); let resp = common::test_app().await.unwrap().call(req).await.unwrap(); - assert_eq!(resp.status(), StatusCode::NOT_FOUND); - let body: ApiResponse = test::read_body_json(resp).await; + assert_eq!(resp.status(), StatusCode::OK); + let body: ApiResponse> = test::read_body_json(resp).await; let _ = tracing::debug!("{:?}", body); - assert_eq!( - body, - ApiResponse::failure( - "Entity does not exist - No users available".to_owned() - ) - ); + assert_eq!(body, ApiResponse::successful(vec![1; 0])); } #[actix_rt::test]