From 71226feb74f21c99fbf8e74cf949c196e4a1ab38 Mon Sep 17 00:00:00 2001 From: marin postma Date: Wed, 23 Jun 2021 13:21:48 +0200 Subject: [PATCH 1/9] refactor create_app macro --- meilisearch-http/src/lib.rs | 149 +++++++++++++----------- meilisearch-http/src/main.rs | 8 +- meilisearch-http/src/routes/document.rs | 3 +- 3 files changed, 83 insertions(+), 77 deletions(-) diff --git a/meilisearch-http/src/lib.rs b/meilisearch-http/src/lib.rs index 6f4ce7e43..182742a10 100644 --- a/meilisearch-http/src/lib.rs +++ b/meilisearch-http/src/lib.rs @@ -13,73 +13,83 @@ pub mod analytics; pub use self::data::Data; pub use option::Opt; +use actix_web::{HttpResponse, web}; + +pub fn configure_data(config: &mut web::ServiceConfig, data: Data) { + let http_payload_size_limit = data.http_payload_size_limit(); + config + .data(data) + .app_data( + web::JsonConfig::default() + .limit(dbg!(http_payload_size_limit)) + .content_type(|_mime| true) // Accept all mime types + .error_handler(|err, _req| error::payload_error_handler(err).into()), + ) + .app_data(web::PayloadConfig::new(http_payload_size_limit)) + .app_data( + web::QueryConfig::default() + .error_handler(|err, _req| error::payload_error_handler(err).into()), + ); +} + +#[cfg(feature = "mini-dashboard")] +pub fn dashboard(config: &mut web::ServiceConfig, enable_frontend: bool) { + use actix_web_static_files::Resource; + + mod dashboard { + include!(concat!(env!("OUT_DIR"), "/generated.rs")); + } + + if enable_frontend { + let generated = dashboard::generate(); + let mut scope = web::scope("/"); + // Generate routes for mini-dashboard assets + for (path, resource) in generated.into_iter() { + let Resource {mime_type, data, ..} = resource; + // Redirect index.html to / + if path == "index.html" { + config.service(web::resource("/").route(web::get().to(move || { + HttpResponse::Ok().content_type(mime_type).body(data) + }))); + } else { + scope = scope.service(web::resource(path).route(web::get().to(move || { + HttpResponse::Ok().content_type(mime_type).body(data) + }))); + } + } + config.service(scope); + } else { + config.service(routes::running); + } +} + +#[cfg(not(feature = "mini-dashboard"))] +pub fn dashboard(config: &mut web::ServiceConfig, _enable_frontend: bool) { + config.service(routes::running); +} + #[macro_export] macro_rules! create_app { - ($data:expr, $enable_frontend:expr) => { - { - use actix_cors::Cors; - use actix_web::middleware::TrailingSlash; - use actix_web::{App, HttpResponse}; - use actix_web::{middleware, web}; - use meilisearch_http::error::payload_error_handler; - use meilisearch_http::routes::*; + ($data:expr, $enable_frontend:expr) => {{ + use actix_cors::Cors; + use actix_web::middleware::TrailingSlash; + use actix_web::App; + use actix_web::{middleware, web}; + use meilisearch_http::routes::*; + use meilisearch_http::{dashboard, configure_data}; - #[cfg(feature = "mini-dashboard")] - use actix_web_static_files::Resource; - - #[cfg(feature = "mini-dashboard")] - mod dashboard { - include!(concat!(env!("OUT_DIR"), "/generated.rs")); - } - - let app = App::new() - .data($data.clone()) - .app_data( - web::JsonConfig::default() - .limit($data.http_payload_size_limit()) - .content_type(|_mime| true) // Accept all mime types - .error_handler(|err, _req| payload_error_handler(err).into()), - ) - .app_data( - web::QueryConfig::default() - .error_handler(|err, _req| payload_error_handler(err).into()), - ) - .configure(document::services) - .configure(index::services) - .configure(search::services) - .configure(settings::services) - .configure(health::services) - .configure(stats::services) - .configure(key::services) - .configure(dump::services); - #[cfg(feature = "mini-dashboard")] - let app = if $enable_frontend { - let mut app = app; - let generated = dashboard::generate(); - let mut scope = web::scope("/"); - // Generate routes for mini-dashboard assets - for (path, resource) in generated.into_iter() { - let Resource {mime_type, data, ..} = resource; - // Redirect index.html to / - if path == "index.html" { - app = app.service(web::resource("/").route(web::get().to(move || { - HttpResponse::Ok().content_type(mime_type).body(data) - }))); - } else { - scope = scope.service(web::resource(path).route(web::get().to(move || { - HttpResponse::Ok().content_type(mime_type).body(data) - }))); - } - } - app.service(scope) - } else { - app.service(running) - }; - - #[cfg(not(feature = "mini-dashboard"))] - let app = app.service(running); - - app.wrap( + App::new() + .configure(|s| configure_data(s, $data.clone())) + .configure(document::services) + .configure(index::services) + .configure(search::services) + .configure(settings::services) + .configure(health::services) + .configure(stats::services) + .configure(key::services) + .configure(dump::services) + .configure(|s| dashboard(s, $enable_frontend)) + .wrap( Cors::default() .send_wildcard() .allowed_headers(vec!["content-type", "x-meili-api-key"]) @@ -87,11 +97,8 @@ macro_rules! create_app { .allow_any_method() .max_age(86_400), // 24h ) - .wrap(middleware::Logger::default()) - .wrap(middleware::Compress::default()) - .wrap(middleware::NormalizePath::new(TrailingSlash::Trim)) - .default_service( - web::route().to(|| HttpResponse::NotFound())) - } - }; + .wrap(middleware::Logger::default()) + .wrap(middleware::Compress::default()) + .wrap(middleware::NormalizePath::new(middleware::TrailingSlash::Trim)) + }}; } diff --git a/meilisearch-http/src/main.rs b/meilisearch-http/src/main.rs index 9f3e62356..c723a8ad5 100644 --- a/meilisearch-http/src/main.rs +++ b/meilisearch-http/src/main.rs @@ -2,7 +2,7 @@ use std::env; use actix_web::HttpServer; use main_error::MainError; -use meilisearch_http::{create_app, Data, Opt}; +use meilisearch_http::{Data, Opt, create_app}; use structopt::StructOpt; #[cfg(all(not(debug_assertions), feature = "analytics"))] @@ -74,7 +74,7 @@ async fn main() -> Result<(), MainError> { async fn run_http(data: Data, opt: Opt) -> Result<(), Box> { let _enable_dashboard = &opt.env == "development"; - let http_server = HttpServer::new(move || create_app!(&data, _enable_dashboard)) + let http_server = HttpServer::new(move || create_app!(data, _enable_dashboard)) // Disable signals allows the server to terminate immediately when a user enter CTRL-C .disable_signals(); @@ -83,8 +83,8 @@ async fn run_http(data: Data, opt: Opt) -> Result<(), Box .bind_rustls(opt.http_addr, config)? .run() .await?; - } else { - http_server.bind(opt.http_addr)?.run().await?; + } else { + http_server.bind(opt.http_addr)?.run().await?; } Ok(()) } diff --git a/meilisearch-http/src/routes/document.rs b/meilisearch-http/src/routes/document.rs index 9019bdb7c..75c950734 100644 --- a/meilisearch-http/src/routes/document.rs +++ b/meilisearch-http/src/routes/document.rs @@ -1,4 +1,3 @@ -use actix_web::web::Payload; use actix_web::{delete, get, post, put}; use actix_web::{web, HttpResponse}; use indexmap::IndexMap; @@ -130,7 +129,7 @@ async fn add_documents( data: web::Data, path: web::Path, params: web::Query, - body: Payload, + body: web::Payload, ) -> Result { let update_status = data .add_documents( From 1c13100948b708909191627ca64c67630c7955c0 Mon Sep 17 00:00:00 2001 From: marin postma Date: Wed, 23 Jun 2021 13:55:16 +0200 Subject: [PATCH 2/9] implement custom payload --- meilisearch-http/src/data/updates.rs | 3 +- meilisearch-http/src/index_controller/mod.rs | 3 +- meilisearch-http/src/lib.rs | 60 +++++++++++++++++++- meilisearch-http/src/routes/document.rs | 6 +- 4 files changed, 65 insertions(+), 7 deletions(-) diff --git a/meilisearch-http/src/data/updates.rs b/meilisearch-http/src/data/updates.rs index 6b29d46b1..ea47d5a74 100644 --- a/meilisearch-http/src/data/updates.rs +++ b/meilisearch-http/src/data/updates.rs @@ -1,7 +1,6 @@ -use actix_web::web::Payload; use milli::update::{IndexDocumentsMethod, UpdateFormat}; -use super::Data; +use crate::{Data, Payload}; use crate::index::{Checked, Settings}; use crate::index_controller::{error::Result, IndexMetadata, IndexSettings, UpdateStatus}; diff --git a/meilisearch-http/src/index_controller/mod.rs b/meilisearch-http/src/index_controller/mod.rs index c32c40345..ddee5fd1e 100644 --- a/meilisearch-http/src/index_controller/mod.rs +++ b/meilisearch-http/src/index_controller/mod.rs @@ -3,7 +3,7 @@ use std::path::Path; use std::sync::Arc; use std::time::Duration; -use actix_web::web::{Bytes, Payload}; +use actix_web::web::Bytes; use chrono::{DateTime, Utc}; use futures::stream::StreamExt; use log::error; @@ -25,6 +25,7 @@ use uuid_resolver::{error::UuidResolverError, UuidResolverHandle}; use crate::index::{Checked, Document, SearchQuery, SearchResult, Settings}; use crate::option::Opt; use error::Result; +use crate::Payload; use self::dump_actor::load_dump; use self::error::IndexControllerError; diff --git a/meilisearch-http/src/lib.rs b/meilisearch-http/src/lib.rs index 182742a10..2bdb4de8e 100644 --- a/meilisearch-http/src/lib.rs +++ b/meilisearch-http/src/lib.rs @@ -10,10 +10,13 @@ pub mod routes; #[cfg(all(not(debug_assertions), feature = "analytics"))] pub mod analytics; +use std::{pin::Pin, task::{Context, Poll}}; + pub use self::data::Data; +use futures::{Stream, future::{Ready, ready}}; pub use option::Opt; -use actix_web::{HttpResponse, web}; +use actix_web::{FromRequest, HttpRequest, dev, error::PayloadError, web}; pub fn configure_data(config: &mut web::ServiceConfig, data: Data) { let http_payload_size_limit = data.http_payload_size_limit(); @@ -35,6 +38,7 @@ pub fn configure_data(config: &mut web::ServiceConfig, data: Data) { #[cfg(feature = "mini-dashboard")] pub fn dashboard(config: &mut web::ServiceConfig, enable_frontend: bool) { use actix_web_static_files::Resource; + use actix_web::HttpResponse; mod dashboard { include!(concat!(env!("OUT_DIR"), "/generated.rs")); @@ -102,3 +106,57 @@ macro_rules! create_app { .wrap(middleware::NormalizePath::new(middleware::TrailingSlash::Trim)) }}; } + +pub struct Payload { + payload: dev::Payload, + limit: usize, +} + +pub struct PayloadConfig { + limit: usize, +} + +impl Default for PayloadConfig { + fn default() -> Self { + Self { limit: 256 * 1024 } + } +} + +impl FromRequest for Payload { + type Config = PayloadConfig; + + type Error = PayloadError; + + type Future = Ready>; + + #[inline] + fn from_request(req: &HttpRequest, payload: &mut dev::Payload) -> Self::Future { + let limit = req.app_data::().map(|c| c.limit).unwrap_or(Self::Config::default().limit); + ready(Ok(Payload { payload: payload.take(), limit })) + } +} + +impl Stream for Payload { + type Item = Result; + + #[inline] + fn poll_next(mut self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll> { + match Pin::new(&mut self.payload).poll_next(cx) { + Poll::Ready(Some(result)) => { + match result { + Ok(bytes) => { + match self.limit.checked_sub(bytes.len()) { + Some(new_limit) => { + self.limit = new_limit; + Poll::Ready(Some(Ok(bytes))) + } + None => Poll::Ready(Some(Err(PayloadError::Overflow))), + } + } + x => Poll::Ready(Some(x)), + } + }, + otherwise => otherwise, + } + } +} diff --git a/meilisearch-http/src/routes/document.rs b/meilisearch-http/src/routes/document.rs index 75c950734..3cc483334 100644 --- a/meilisearch-http/src/routes/document.rs +++ b/meilisearch-http/src/routes/document.rs @@ -9,7 +9,7 @@ use serde_json::Value; use crate::error::ResponseError; use crate::helpers::Authentication; use crate::routes::IndexParam; -use crate::Data; +use crate::{Data, Payload}; const DEFAULT_RETRIEVE_DOCUMENTS_OFFSET: usize = 0; const DEFAULT_RETRIEVE_DOCUMENTS_LIMIT: usize = 20; @@ -129,7 +129,7 @@ async fn add_documents( data: web::Data, path: web::Path, params: web::Query, - body: web::Payload, + body: Payload, ) -> Result { let update_status = data .add_documents( @@ -173,7 +173,7 @@ async fn update_documents( data: web::Data, path: web::Path, params: web::Query, - body: web::Payload, + body: Payload, ) -> Result { let update = data .add_documents( From 4b292c6e9b2638c8957e8ea5ba38dc3f4cdf4d9b Mon Sep 17 00:00:00 2001 From: marin postma Date: Wed, 23 Jun 2021 13:58:22 +0200 Subject: [PATCH 3/9] add payload limit to app config --- meilisearch-http/src/lib.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/meilisearch-http/src/lib.rs b/meilisearch-http/src/lib.rs index 2bdb4de8e..ad3d4b86b 100644 --- a/meilisearch-http/src/lib.rs +++ b/meilisearch-http/src/lib.rs @@ -28,7 +28,7 @@ pub fn configure_data(config: &mut web::ServiceConfig, data: Data) { .content_type(|_mime| true) // Accept all mime types .error_handler(|err, _req| error::payload_error_handler(err).into()), ) - .app_data(web::PayloadConfig::new(http_payload_size_limit)) + .app_data(PayloadConfig::new(http_payload_size_limit)) .app_data( web::QueryConfig::default() .error_handler(|err, _req| error::payload_error_handler(err).into()), @@ -116,6 +116,10 @@ pub struct PayloadConfig { limit: usize, } +impl PayloadConfig { + pub fn new(limit: usize) -> Self { Self { limit } } +} + impl Default for PayloadConfig { fn default() -> Self { Self { limit: 256 * 1024 } From f62779671bc6c4e443497b60dfcab48ff4fb3421 Mon Sep 17 00:00:00 2001 From: marin postma Date: Wed, 23 Jun 2021 14:21:20 +0200 Subject: [PATCH 4/9] change error message for payload size limit --- .../src/index_controller/update_actor/error.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/meilisearch-http/src/index_controller/update_actor/error.rs b/meilisearch-http/src/index_controller/update_actor/error.rs index a95f1eafa..ae0418033 100644 --- a/meilisearch-http/src/index_controller/update_actor/error.rs +++ b/meilisearch-http/src/index_controller/update_actor/error.rs @@ -21,6 +21,8 @@ pub enum UpdateActorError { FatalUpdateStoreError, #[error("invalid payload: {0}")] InvalidPayload(Box), + #[error("payload error: {0}")] + PayloadError(#[from] actix_web::error::PayloadError), } impl From> for UpdateActorError { @@ -39,7 +41,6 @@ internal_error!( UpdateActorError: heed::Error, std::io::Error, serde_json::Error, - actix_http::error::PayloadError, tokio::task::JoinError ); @@ -51,6 +52,12 @@ impl ErrorCode for UpdateActorError { UpdateActorError::IndexActor(e) => e.error_code(), UpdateActorError::FatalUpdateStoreError => Code::Internal, UpdateActorError::InvalidPayload(_) => Code::BadRequest, + UpdateActorError::PayloadError(error) => { + match error { + actix_http::error::PayloadError::Overflow => Code::PayloadTooLarge, + _ => Code::Internal, + } + }, } } } From b000ae76145bfe7df033ac92be691f6cba2a4875 Mon Sep 17 00:00:00 2001 From: marin postma Date: Wed, 23 Jun 2021 14:45:26 +0200 Subject: [PATCH 5/9] remove file if write to update file fails --- .../index_controller/update_actor/actor.rs | 42 ++++++++++++------- 1 file changed, 28 insertions(+), 14 deletions(-) diff --git a/meilisearch-http/src/index_controller/update_actor/actor.rs b/meilisearch-http/src/index_controller/update_actor/actor.rs index 41995727a..f9b886783 100644 --- a/meilisearch-http/src/index_controller/update_actor/actor.rs +++ b/meilisearch-http/src/index_controller/update_actor/actor.rs @@ -122,7 +122,7 @@ where &self, uuid: Uuid, meta: UpdateMeta, - mut payload: mpsc::Receiver>, + payload: mpsc::Receiver>, ) -> Result { let file_path = match meta { UpdateMeta::DocumentsAddition { .. } => { @@ -137,21 +137,35 @@ where .open(&path) .await?; - let mut file_len = 0; - while let Some(bytes) = payload.recv().await { - let bytes = bytes?; - file_len += bytes.as_ref().len(); - file.write_all(bytes.as_ref()).await?; + async fn write_to_file(file: &mut fs::File, mut payload: mpsc::Receiver>) -> Result + where + D: AsRef<[u8]> + Sized + 'static, + { + let mut file_len = 0; + while let Some(bytes) = payload.recv().await { + let bytes = bytes?; + file_len += bytes.as_ref().len(); + file.write_all(bytes.as_ref()).await?; + } + Ok(file_len) } - if file_len != 0 { - file.flush().await?; - let file = file.into_std().await; - Some((file, update_file_id)) - } else { - // empty update, delete the empty file. - fs::remove_file(&path).await?; - None + let file_len = write_to_file(&mut file, payload).await; + + match file_len { + Ok(len) if len > 0 => { + file.flush().await?; + let file = file.into_std().await; + Some((file, update_file_id)) + } + Err(e) => { + fs::remove_file(&path).await?; + return Err(e) + } + _ => { + fs::remove_file(&path).await?; + None + } } } _ => None, From 834995b13030bde56a1f0fbc753fcb76f574e39e Mon Sep 17 00:00:00 2001 From: marin postma Date: Wed, 23 Jun 2021 14:48:33 +0200 Subject: [PATCH 6/9] clippy + fmt --- meilisearch-http/src/data/updates.rs | 2 +- meilisearch-http/src/error.rs | 9 +-- meilisearch-http/src/index/dump.rs | 2 +- meilisearch-http/src/index/mod.rs | 2 +- meilisearch-http/src/index/search.rs | 9 ++- meilisearch-http/src/index_controller/mod.rs | 2 +- .../index_controller/update_actor/actor.rs | 7 +- .../index_controller/update_actor/error.rs | 8 +-- .../update_actor/store/mod.rs | 9 ++- .../src/index_controller/updates.rs | 5 +- meilisearch-http/src/lib.rs | 68 +++++++++++-------- meilisearch-http/src/main.rs | 6 +- meilisearch-http/src/routes/search.rs | 2 +- 13 files changed, 78 insertions(+), 53 deletions(-) diff --git a/meilisearch-http/src/data/updates.rs b/meilisearch-http/src/data/updates.rs index ea47d5a74..e729340a3 100644 --- a/meilisearch-http/src/data/updates.rs +++ b/meilisearch-http/src/data/updates.rs @@ -1,8 +1,8 @@ use milli::update::{IndexDocumentsMethod, UpdateFormat}; -use crate::{Data, Payload}; use crate::index::{Checked, Settings}; use crate::index_controller::{error::Result, IndexMetadata, IndexSettings, UpdateStatus}; +use crate::{Data, Payload}; impl Data { pub async fn add_documents( diff --git a/meilisearch-http/src/error.rs b/meilisearch-http/src/error.rs index 7d56de738..0b7093e80 100644 --- a/meilisearch-http/src/error.rs +++ b/meilisearch-http/src/error.rs @@ -103,7 +103,7 @@ impl ErrorCode for MilliError<'_> { milli::Error::UserError(ref error) => { match error { // TODO: wait for spec for new error codes. - | UserError::Csv(_) + UserError::Csv(_) | UserError::SerdeJson(_) | UserError::MaxDatabaseSizeReached | UserError::InvalidCriterionName { .. } @@ -148,9 +148,10 @@ impl ErrorCode for PayloadError { PayloadError::Json(err) => match err { JsonPayloadError::Overflow => Code::PayloadTooLarge, JsonPayloadError::ContentType => Code::UnsupportedMediaType, - JsonPayloadError::Payload(aweb::error::PayloadError::Overflow) => Code::PayloadTooLarge, - JsonPayloadError::Deserialize(_) - | JsonPayloadError::Payload(_) => Code::BadRequest, + JsonPayloadError::Payload(aweb::error::PayloadError::Overflow) => { + Code::PayloadTooLarge + } + JsonPayloadError::Deserialize(_) | JsonPayloadError::Payload(_) => Code::BadRequest, JsonPayloadError::Serialize(_) => Code::Internal, _ => Code::Internal, }, diff --git a/meilisearch-http/src/index/dump.rs b/meilisearch-http/src/index/dump.rs index 52a5cabd9..263e3bd52 100644 --- a/meilisearch-http/src/index/dump.rs +++ b/meilisearch-http/src/index/dump.rs @@ -3,7 +3,7 @@ use std::io::{BufRead, BufReader, Write}; use std::path::Path; use std::sync::Arc; -use anyhow::{Context, bail}; +use anyhow::{bail, Context}; use heed::RoTxn; use indexmap::IndexMap; use milli::update::{IndexDocumentsMethod, UpdateFormat::JsonStream}; diff --git a/meilisearch-http/src/index/mod.rs b/meilisearch-http/src/index/mod.rs index 5a005fdc0..7227f8d35 100644 --- a/meilisearch-http/src/index/mod.rs +++ b/meilisearch-http/src/index/mod.rs @@ -13,7 +13,7 @@ use serde_json::{Map, Value}; use crate::helpers::EnvSizer; use error::Result; -pub use search::{SearchQuery, SearchResult, DEFAULT_SEARCH_LIMIT, default_crop_length}; +pub use search::{default_crop_length, SearchQuery, SearchResult, DEFAULT_SEARCH_LIMIT}; pub use updates::{Checked, Facets, Settings, Unchecked}; use self::error::IndexError; diff --git a/meilisearch-http/src/index/search.rs b/meilisearch-http/src/index/search.rs index a1dc2ee55..bc6d2ae74 100644 --- a/meilisearch-http/src/index/search.rs +++ b/meilisearch-http/src/index/search.rs @@ -233,8 +233,8 @@ impl Index { fn compute_matches>( matcher: &impl Matcher, document: &Document, - analyzer: &Analyzer - ) -> MatchesInfo { + analyzer: &Analyzer, +) -> MatchesInfo { let mut matches = BTreeMap::new(); for (key, value) in document { @@ -1174,6 +1174,9 @@ mod test { let analyzer = Analyzer::new(config); let matches = compute_matches(&matcher, &value, &analyzer); - assert_eq!(format!("{:?}", matches), r##"{"about": [MatchInfo { start: 0, length: 6 }, MatchInfo { start: 31, length: 7 }, MatchInfo { start: 191, length: 7 }, MatchInfo { start: 225, length: 7 }, MatchInfo { start: 233, length: 6 }], "color": [MatchInfo { start: 0, length: 3 }]}"##); + assert_eq!( + format!("{:?}", matches), + r##"{"about": [MatchInfo { start: 0, length: 6 }, MatchInfo { start: 31, length: 7 }, MatchInfo { start: 191, length: 7 }, MatchInfo { start: 225, length: 7 }, MatchInfo { start: 233, length: 6 }], "color": [MatchInfo { start: 0, length: 3 }]}"## + ); } } diff --git a/meilisearch-http/src/index_controller/mod.rs b/meilisearch-http/src/index_controller/mod.rs index ddee5fd1e..b7dba1a8a 100644 --- a/meilisearch-http/src/index_controller/mod.rs +++ b/meilisearch-http/src/index_controller/mod.rs @@ -24,8 +24,8 @@ use uuid_resolver::{error::UuidResolverError, UuidResolverHandle}; use crate::index::{Checked, Document, SearchQuery, SearchResult, Settings}; use crate::option::Opt; -use error::Result; use crate::Payload; +use error::Result; use self::dump_actor::load_dump; use self::error::IndexControllerError; diff --git a/meilisearch-http/src/index_controller/update_actor/actor.rs b/meilisearch-http/src/index_controller/update_actor/actor.rs index f9b886783..5b269ea9a 100644 --- a/meilisearch-http/src/index_controller/update_actor/actor.rs +++ b/meilisearch-http/src/index_controller/update_actor/actor.rs @@ -137,7 +137,10 @@ where .open(&path) .await?; - async fn write_to_file(file: &mut fs::File, mut payload: mpsc::Receiver>) -> Result + async fn write_to_file( + file: &mut fs::File, + mut payload: mpsc::Receiver>, + ) -> Result where D: AsRef<[u8]> + Sized + 'static, { @@ -160,7 +163,7 @@ where } Err(e) => { fs::remove_file(&path).await?; - return Err(e) + return Err(e); } _ => { fs::remove_file(&path).await?; diff --git a/meilisearch-http/src/index_controller/update_actor/error.rs b/meilisearch-http/src/index_controller/update_actor/error.rs index ae0418033..6f0e848c3 100644 --- a/meilisearch-http/src/index_controller/update_actor/error.rs +++ b/meilisearch-http/src/index_controller/update_actor/error.rs @@ -52,11 +52,9 @@ impl ErrorCode for UpdateActorError { UpdateActorError::IndexActor(e) => e.error_code(), UpdateActorError::FatalUpdateStoreError => Code::Internal, UpdateActorError::InvalidPayload(_) => Code::BadRequest, - UpdateActorError::PayloadError(error) => { - match error { - actix_http::error::PayloadError::Overflow => Code::PayloadTooLarge, - _ => Code::Internal, - } + UpdateActorError::PayloadError(error) => match error { + actix_http::error::PayloadError::Overflow => Code::PayloadTooLarge, + _ => Code::Internal, }, } } diff --git a/meilisearch-http/src/index_controller/update_actor/store/mod.rs b/meilisearch-http/src/index_controller/update_actor/store/mod.rs index 1ee30c99d..21a320b26 100644 --- a/meilisearch-http/src/index_controller/update_actor/store/mod.rs +++ b/meilisearch-http/src/index_controller/update_actor/store/mod.rs @@ -572,7 +572,10 @@ fn update_uuid_to_file_path(root: impl AsRef, uuid: Uuid) -> PathBuf { #[cfg(test)] mod test { use super::*; - use crate::index_controller::{UpdateResult, index_actor::{MockIndexActorHandle, error::IndexActorError}}; + use crate::index_controller::{ + index_actor::{error::IndexActorError, MockIndexActorHandle}, + UpdateResult, + }; use futures::future::ok; @@ -651,7 +654,9 @@ mod test { if processing.id() == 0 { Box::pin(ok(Ok(processing.process(UpdateResult::Other)))) } else { - Box::pin(ok(Err(processing.fail(IndexActorError::ExistingPrimaryKey.into())))) + Box::pin(ok(Err( + processing.fail(IndexActorError::ExistingPrimaryKey.into()) + ))) } }); diff --git a/meilisearch-http/src/index_controller/updates.rs b/meilisearch-http/src/index_controller/updates.rs index 13ad9f7e5..d02438d3c 100644 --- a/meilisearch-http/src/index_controller/updates.rs +++ b/meilisearch-http/src/index_controller/updates.rs @@ -3,7 +3,10 @@ use milli::update::{DocumentAdditionResult, IndexDocumentsMethod, UpdateFormat}; use serde::{Deserialize, Serialize}; use uuid::Uuid; -use crate::{error::ResponseError, index::{Settings, Unchecked}}; +use crate::{ + error::ResponseError, + index::{Settings, Unchecked}, +}; #[derive(Debug, Clone, Serialize, Deserialize)] pub enum UpdateResult { diff --git a/meilisearch-http/src/lib.rs b/meilisearch-http/src/lib.rs index ad3d4b86b..0ca09234c 100644 --- a/meilisearch-http/src/lib.rs +++ b/meilisearch-http/src/lib.rs @@ -10,13 +10,19 @@ pub mod routes; #[cfg(all(not(debug_assertions), feature = "analytics"))] pub mod analytics; -use std::{pin::Pin, task::{Context, Poll}}; +use std::{ + pin::Pin, + task::{Context, Poll}, +}; pub use self::data::Data; -use futures::{Stream, future::{Ready, ready}}; +use futures::{ + future::{ready, Ready}, + Stream, +}; pub use option::Opt; -use actix_web::{FromRequest, HttpRequest, dev, error::PayloadError, web}; +use actix_web::{dev, error::PayloadError, web, FromRequest, HttpRequest}; pub fn configure_data(config: &mut web::ServiceConfig, data: Data) { let http_payload_size_limit = data.http_payload_size_limit(); @@ -40,12 +46,12 @@ pub fn dashboard(config: &mut web::ServiceConfig, enable_frontend: bool) { use actix_web_static_files::Resource; use actix_web::HttpResponse; - mod dashboard { + mod generated { include!(concat!(env!("OUT_DIR"), "/generated.rs")); } if enable_frontend { - let generated = dashboard::generate(); + let generated = generated::generate(); let mut scope = web::scope("/"); // Generate routes for mini-dashboard assets for (path, resource) in generated.into_iter() { @@ -80,7 +86,7 @@ macro_rules! create_app { use actix_web::App; use actix_web::{middleware, web}; use meilisearch_http::routes::*; - use meilisearch_http::{dashboard, configure_data}; + use meilisearch_http::{configure_data, dashboard}; App::new() .configure(|s| configure_data(s, $data.clone())) @@ -95,15 +101,17 @@ macro_rules! create_app { .configure(|s| dashboard(s, $enable_frontend)) .wrap( Cors::default() - .send_wildcard() - .allowed_headers(vec!["content-type", "x-meili-api-key"]) - .allow_any_origin() - .allow_any_method() - .max_age(86_400), // 24h + .send_wildcard() + .allowed_headers(vec!["content-type", "x-meili-api-key"]) + .allow_any_origin() + .allow_any_method() + .max_age(86_400), // 24h ) .wrap(middleware::Logger::default()) .wrap(middleware::Compress::default()) - .wrap(middleware::NormalizePath::new(middleware::TrailingSlash::Trim)) + .wrap(middleware::NormalizePath::new( + middleware::TrailingSlash::Trim, + )) }}; } @@ -117,12 +125,14 @@ pub struct PayloadConfig { } impl PayloadConfig { - pub fn new(limit: usize) -> Self { Self { limit } } + pub fn new(limit: usize) -> Self { + Self { limit } + } } impl Default for PayloadConfig { fn default() -> Self { - Self { limit: 256 * 1024 } + Self { limit: 256 * 1024 } } } @@ -135,8 +145,14 @@ impl FromRequest for Payload { #[inline] fn from_request(req: &HttpRequest, payload: &mut dev::Payload) -> Self::Future { - let limit = req.app_data::().map(|c| c.limit).unwrap_or(Self::Config::default().limit); - ready(Ok(Payload { payload: payload.take(), limit })) + let limit = req + .app_data::() + .map(|c| c.limit) + .unwrap_or(Self::Config::default().limit); + ready(Ok(Payload { + payload: payload.take(), + limit, + })) } } @@ -146,19 +162,15 @@ impl Stream for Payload { #[inline] fn poll_next(mut self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll> { match Pin::new(&mut self.payload).poll_next(cx) { - Poll::Ready(Some(result)) => { - match result { - Ok(bytes) => { - match self.limit.checked_sub(bytes.len()) { - Some(new_limit) => { - self.limit = new_limit; - Poll::Ready(Some(Ok(bytes))) - } - None => Poll::Ready(Some(Err(PayloadError::Overflow))), - } + Poll::Ready(Some(result)) => match result { + Ok(bytes) => match self.limit.checked_sub(bytes.len()) { + Some(new_limit) => { + self.limit = new_limit; + Poll::Ready(Some(Ok(bytes))) } - x => Poll::Ready(Some(x)), - } + None => Poll::Ready(Some(Err(PayloadError::Overflow))), + }, + x => Poll::Ready(Some(x)), }, otherwise => otherwise, } diff --git a/meilisearch-http/src/main.rs b/meilisearch-http/src/main.rs index c723a8ad5..50201d912 100644 --- a/meilisearch-http/src/main.rs +++ b/meilisearch-http/src/main.rs @@ -2,7 +2,7 @@ use std::env; use actix_web::HttpServer; use main_error::MainError; -use meilisearch_http::{Data, Opt, create_app}; +use meilisearch_http::{create_app, Data, Opt}; use structopt::StructOpt; #[cfg(all(not(debug_assertions), feature = "analytics"))] @@ -83,8 +83,8 @@ async fn run_http(data: Data, opt: Opt) -> Result<(), Box .bind_rustls(opt.http_addr, config)? .run() .await?; - } else { - http_server.bind(opt.http_addr)?.run().await?; + } else { + http_server.bind(opt.http_addr)?.run().await?; } Ok(()) } diff --git a/meilisearch-http/src/routes/search.rs b/meilisearch-http/src/routes/search.rs index ab5a2f407..f596f7e4c 100644 --- a/meilisearch-http/src/routes/search.rs +++ b/meilisearch-http/src/routes/search.rs @@ -6,7 +6,7 @@ use serde_json::Value; use crate::error::ResponseError; use crate::helpers::Authentication; -use crate::index::{SearchQuery, default_crop_length, DEFAULT_SEARCH_LIMIT}; +use crate::index::{default_crop_length, SearchQuery, DEFAULT_SEARCH_LIMIT}; use crate::routes::IndexParam; use crate::Data; From a838238a639c7ac1646d0f3656075df376b7de4d Mon Sep 17 00:00:00 2001 From: marin postma Date: Wed, 23 Jun 2021 14:56:02 +0200 Subject: [PATCH 7/9] move payload to own module --- meilisearch-http/src/data/updates.rs | 3 +- meilisearch-http/src/extractors/mod.rs | 1 + meilisearch-http/src/extractors/payload.rs | 69 ++++++++++++++++++ meilisearch-http/src/index_controller/mod.rs | 2 +- meilisearch-http/src/lib.rs | 76 ++------------------ meilisearch-http/src/routes/document.rs | 3 +- 6 files changed, 79 insertions(+), 75 deletions(-) create mode 100644 meilisearch-http/src/extractors/mod.rs create mode 100644 meilisearch-http/src/extractors/payload.rs diff --git a/meilisearch-http/src/data/updates.rs b/meilisearch-http/src/data/updates.rs index e729340a3..4e38294e9 100644 --- a/meilisearch-http/src/data/updates.rs +++ b/meilisearch-http/src/data/updates.rs @@ -1,8 +1,9 @@ use milli::update::{IndexDocumentsMethod, UpdateFormat}; +use crate::extractors::payload::Payload; use crate::index::{Checked, Settings}; use crate::index_controller::{error::Result, IndexMetadata, IndexSettings, UpdateStatus}; -use crate::{Data, Payload}; +use crate::Data; impl Data { pub async fn add_documents( diff --git a/meilisearch-http/src/extractors/mod.rs b/meilisearch-http/src/extractors/mod.rs new file mode 100644 index 000000000..fbb091fe2 --- /dev/null +++ b/meilisearch-http/src/extractors/mod.rs @@ -0,0 +1 @@ +pub mod payload; diff --git a/meilisearch-http/src/extractors/payload.rs b/meilisearch-http/src/extractors/payload.rs new file mode 100644 index 000000000..260561e40 --- /dev/null +++ b/meilisearch-http/src/extractors/payload.rs @@ -0,0 +1,69 @@ +use std::pin::Pin; +use std::task::{Context, Poll}; + +use actix_http::error::PayloadError; +use actix_web::{dev, web, FromRequest, HttpRequest}; +use futures::future::{ready, Ready}; +use futures::Stream; + +pub struct Payload { + payload: dev::Payload, + limit: usize, +} + +pub struct PayloadConfig { + limit: usize, +} + +impl PayloadConfig { + pub fn new(limit: usize) -> Self { + Self { limit } + } +} + +impl Default for PayloadConfig { + fn default() -> Self { + Self { limit: 256 * 1024 } + } +} + +impl FromRequest for Payload { + type Config = PayloadConfig; + + type Error = PayloadError; + + type Future = Ready>; + + #[inline] + fn from_request(req: &HttpRequest, payload: &mut dev::Payload) -> Self::Future { + let limit = req + .app_data::() + .map(|c| c.limit) + .unwrap_or(Self::Config::default().limit); + ready(Ok(Payload { + payload: payload.take(), + limit, + })) + } +} + +impl Stream for Payload { + type Item = Result; + + #[inline] + fn poll_next(mut self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll> { + match Pin::new(&mut self.payload).poll_next(cx) { + Poll::Ready(Some(result)) => match result { + Ok(bytes) => match self.limit.checked_sub(bytes.len()) { + Some(new_limit) => { + self.limit = new_limit; + Poll::Ready(Some(Ok(bytes))) + } + None => Poll::Ready(Some(Err(PayloadError::Overflow))), + }, + x => Poll::Ready(Some(x)), + }, + otherwise => otherwise, + } + } +} diff --git a/meilisearch-http/src/index_controller/mod.rs b/meilisearch-http/src/index_controller/mod.rs index b7dba1a8a..c223ead76 100644 --- a/meilisearch-http/src/index_controller/mod.rs +++ b/meilisearch-http/src/index_controller/mod.rs @@ -22,9 +22,9 @@ use update_actor::UpdateActorHandle; pub use updates::*; use uuid_resolver::{error::UuidResolverError, UuidResolverHandle}; +use crate::extractors::payload::Payload; use crate::index::{Checked, Document, SearchQuery, SearchResult, Settings}; use crate::option::Opt; -use crate::Payload; use error::Result; use self::dump_actor::load_dump; diff --git a/meilisearch-http/src/lib.rs b/meilisearch-http/src/lib.rs index 0ca09234c..de30e1782 100644 --- a/meilisearch-http/src/lib.rs +++ b/meilisearch-http/src/lib.rs @@ -1,6 +1,7 @@ pub mod data; #[macro_use] pub mod error; +pub mod extractors; pub mod helpers; mod index; mod index_controller; @@ -10,19 +11,12 @@ pub mod routes; #[cfg(all(not(debug_assertions), feature = "analytics"))] pub mod analytics; -use std::{ - pin::Pin, - task::{Context, Poll}, -}; - pub use self::data::Data; -use futures::{ - future::{ready, Ready}, - Stream, -}; pub use option::Opt; -use actix_web::{dev, error::PayloadError, web, FromRequest, HttpRequest}; +use actix_web::web; + +use extractors::payload::PayloadConfig; pub fn configure_data(config: &mut web::ServiceConfig, data: Data) { let http_payload_size_limit = data.http_payload_size_limit(); @@ -114,65 +108,3 @@ macro_rules! create_app { )) }}; } - -pub struct Payload { - payload: dev::Payload, - limit: usize, -} - -pub struct PayloadConfig { - limit: usize, -} - -impl PayloadConfig { - pub fn new(limit: usize) -> Self { - Self { limit } - } -} - -impl Default for PayloadConfig { - fn default() -> Self { - Self { limit: 256 * 1024 } - } -} - -impl FromRequest for Payload { - type Config = PayloadConfig; - - type Error = PayloadError; - - type Future = Ready>; - - #[inline] - fn from_request(req: &HttpRequest, payload: &mut dev::Payload) -> Self::Future { - let limit = req - .app_data::() - .map(|c| c.limit) - .unwrap_or(Self::Config::default().limit); - ready(Ok(Payload { - payload: payload.take(), - limit, - })) - } -} - -impl Stream for Payload { - type Item = Result; - - #[inline] - fn poll_next(mut self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll> { - match Pin::new(&mut self.payload).poll_next(cx) { - Poll::Ready(Some(result)) => match result { - Ok(bytes) => match self.limit.checked_sub(bytes.len()) { - Some(new_limit) => { - self.limit = new_limit; - Poll::Ready(Some(Ok(bytes))) - } - None => Poll::Ready(Some(Err(PayloadError::Overflow))), - }, - x => Poll::Ready(Some(x)), - }, - otherwise => otherwise, - } - } -} diff --git a/meilisearch-http/src/routes/document.rs b/meilisearch-http/src/routes/document.rs index 3cc483334..a9d6c0db8 100644 --- a/meilisearch-http/src/routes/document.rs +++ b/meilisearch-http/src/routes/document.rs @@ -7,9 +7,10 @@ use serde::Deserialize; use serde_json::Value; use crate::error::ResponseError; +use crate::extractors::payload::Payload; use crate::helpers::Authentication; use crate::routes::IndexParam; -use crate::{Data, Payload}; +use crate::Data; const DEFAULT_RETRIEVE_DOCUMENTS_OFFSET: usize = 0; const DEFAULT_RETRIEVE_DOCUMENTS_LIMIT: usize = 20; From 880fc069bd786bc00bf922dfdb98c0301f2c339f Mon Sep 17 00:00:00 2001 From: marin postma Date: Wed, 23 Jun 2021 15:02:00 +0200 Subject: [PATCH 8/9] remove dbg --- meilisearch-http/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/meilisearch-http/src/lib.rs b/meilisearch-http/src/lib.rs index de30e1782..46fea718c 100644 --- a/meilisearch-http/src/lib.rs +++ b/meilisearch-http/src/lib.rs @@ -24,7 +24,7 @@ pub fn configure_data(config: &mut web::ServiceConfig, data: Data) { .data(data) .app_data( web::JsonConfig::default() - .limit(dbg!(http_payload_size_limit)) + .limit(http_payload_size_limit) .content_type(|_mime| true) // Accept all mime types .error_handler(|err, _req| error::payload_error_handler(err).into()), ) From f68c2574521c5e7fec1e12c5587a7b5f2476c526 Mon Sep 17 00:00:00 2001 From: marin postma Date: Wed, 23 Jun 2021 16:34:07 +0200 Subject: [PATCH 9/9] move flush in write_to_file function --- meilisearch-http/src/index_controller/update_actor/actor.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/meilisearch-http/src/index_controller/update_actor/actor.rs b/meilisearch-http/src/index_controller/update_actor/actor.rs index 5b269ea9a..5bc2c727e 100644 --- a/meilisearch-http/src/index_controller/update_actor/actor.rs +++ b/meilisearch-http/src/index_controller/update_actor/actor.rs @@ -145,11 +145,15 @@ where D: AsRef<[u8]> + Sized + 'static, { let mut file_len = 0; + while let Some(bytes) = payload.recv().await { let bytes = bytes?; file_len += bytes.as_ref().len(); file.write_all(bytes.as_ref()).await?; } + + file.flush().await?; + Ok(file_len) } @@ -157,7 +161,6 @@ where match file_len { Ok(len) if len > 0 => { - file.flush().await?; let file = file.into_std().await; Some((file, update_file_id)) }