From 8a2e60dc096617e5c1df8b142667b348a7b823e3 Mon Sep 17 00:00:00 2001 From: mpostma Date: Thu, 28 May 2020 16:12:24 +0200 Subject: [PATCH] requested changes --- meilisearch-core/examples/from_file.rs | 12 ++--- meilisearch-core/src/database.rs | 12 +++-- meilisearch-error/src/lib.rs | 18 ++++---- meilisearch-http/src/routes/document.rs | 21 ++------- meilisearch-http/src/routes/health.rs | 12 +---- meilisearch-http/src/routes/index.rs | 10 ++-- meilisearch-http/src/routes/setting.rs | 54 ++++------------------ meilisearch-http/src/routes/stop_words.rs | 10 +--- meilisearch-http/src/routes/synonym.rs | 10 +--- meilisearch-http/tests/documents_delete.rs | 1 - 10 files changed, 46 insertions(+), 114 deletions(-) diff --git a/meilisearch-core/examples/from_file.rs b/meilisearch-core/examples/from_file.rs index 561036c02..af41efb6c 100644 --- a/meilisearch-core/examples/from_file.rs +++ b/meilisearch-core/examples/from_file.rs @@ -12,7 +12,7 @@ use serde::{Deserialize, Serialize}; use structopt::StructOpt; use termcolor::{Color, ColorChoice, ColorSpec, StandardStream, WriteColor}; -use meilisearch_core::{Database, DatabaseOptions, Highlight, ProcessedUpdateResult, Error as MError}; +use meilisearch_core::{Database, DatabaseOptions, Highlight, ProcessedUpdateResult}; use meilisearch_core::settings::Settings; use meilisearch_schema::FieldId; @@ -126,10 +126,7 @@ fn index_command(command: IndexCommand, database: Database) -> Result<(), Box(|writer| { - index.settings_update(writer, settings)?; - Ok(()) - })?; + db.update_write(|w| index.settings_update(w, settings))?; let mut rdr = if command.csv_data_path.as_os_str() == "-" { csv::Reader::from_reader(Box::new(io::stdin()) as Box) @@ -176,10 +173,7 @@ fn index_command(command: IndexCommand, database: Database) -> Result<(), Box(|writer| { - let update_id = additions.finalize(writer)?; - Ok(update_id) - })?; + let update_id = db.update_write(|w| additions.finalize(w))?; println!("committing update..."); max_update_id = max_update_id.max(update_id); diff --git a/meilisearch-core/src/database.rs b/meilisearch-core/src/database.rs index 0b0cd3149..79a1e430c 100644 --- a/meilisearch-core/src/database.rs +++ b/meilisearch-core/src/database.rs @@ -363,11 +363,13 @@ impl Database { /// provides a context with a reader to the main database. experimental. pub fn main_read(&self, f: F) -> Result where - F: Fn(&MainReader) -> Result, + F: FnOnce(&MainReader) -> Result, E: From, { let reader = self.main_read_txn()?; - f(&reader) + let result = f(&reader)?; + reader.abort().map_err(Error::Heed)?; + Ok(result) } pub fn update_read_txn(&self) -> MResult { @@ -394,11 +396,13 @@ impl Database { /// provides a context with a reader to the update database. experimental. pub fn update_read(&self, f: F) -> Result where - F: Fn(&UpdateReader) -> Result, + F: FnOnce(&UpdateReader) -> Result, E: From, { let reader = self.update_read_txn()?; - f(&reader) + let result = f(&reader)?; + reader.abort().map_err(Error::Heed)?; + Ok(result) } pub fn copy_and_compact_to_path>(&self, path: P) -> MResult<(File, File)> { diff --git a/meilisearch-error/src/lib.rs b/meilisearch-error/src/lib.rs index b5dce37e8..1c8fc4e78 100644 --- a/meilisearch-error/src/lib.rs +++ b/meilisearch-error/src/lib.rs @@ -22,7 +22,7 @@ pub trait ErrorCode: std::error::Error { /// return the error type fn error_type(&self) -> String { - self.error_code().r#type() + self.error_code().type_() } } @@ -130,13 +130,13 @@ impl Code { } /// return the error type - fn r#type(&self) -> String { + fn type_(&self) -> String { self.err_code().error_type.to_string() } /// return the doc url ascociated with the error fn url(&self) -> String { - format!("docs.meilisearch.com/error/{}", self.name()) + format!("https://docs.meilisearch.com/error/{}", self.name()) } } @@ -148,26 +148,26 @@ struct ErrCode { } impl ErrCode { - fn authentication(err_name: &'static str, status_code: StatusCode) -> ErrCode { + fn authentication(error_name: &'static str, status_code: StatusCode) -> ErrCode { ErrCode { status_code, - error_name: err_name, + error_name, error_type: ErrorType::Authentication, } } - fn internal(err_name: &'static str, status_code: StatusCode) -> ErrCode { + fn internal(error_name: &'static str, status_code: StatusCode) -> ErrCode { ErrCode { status_code, - error_name: err_name, + error_name, error_type: ErrorType::InternalError, } } - fn invalid(err_name: &'static str, status_code: StatusCode) -> ErrCode { + fn invalid(error_name: &'static str, status_code: StatusCode) -> ErrCode { ErrCode { status_code, - error_name: err_name, + error_name, error_type: ErrorType::InvalidRequest, } } diff --git a/meilisearch-http/src/routes/document.rs b/meilisearch-http/src/routes/document.rs index 448d04562..e7ad3801b 100644 --- a/meilisearch-http/src/routes/document.rs +++ b/meilisearch-http/src/routes/document.rs @@ -72,10 +72,7 @@ async fn delete_document( let mut documents_deletion = index.documents_deletion(); documents_deletion.delete_document_by_external_docid(path.document_id.clone()); - let update_id = data.db.update_write::<_, _, ResponseError>(|writer| { - let update_id = documents_deletion.finalize(writer)?; - Ok(update_id) - })?; + let update_id = data.db.update_write(|w| documents_deletion.finalize(w))?; Ok(HttpResponse::Accepted().json(IndexUpdateResponse::with_id(update_id))) } @@ -174,10 +171,7 @@ async fn update_multiple_documents( .set_primary_key(&id) .map_err(Error::bad_request)?; - data.db.main_write::<_, _, ResponseError>(|mut writer| { - index.main.put_schema(&mut writer, &schema)?; - Ok(()) - })?; + data.db.main_write(|w| index.main.put_schema(w, &schema))?; } let mut document_addition = if is_partial { @@ -190,12 +184,7 @@ async fn update_multiple_documents( document_addition.update_document(document); } - let update_id = data - .db - .update_write::<_, _, ResponseError>(|writer| { - let update_id = document_addition.finalize(writer)?; - Ok(update_id) - })?; + let update_id = data.db.update_write(|w| document_addition.finalize(w))?; Ok(HttpResponse::Accepted().json(IndexUpdateResponse::with_id(update_id))) } @@ -242,7 +231,7 @@ async fn delete_documents( documents_deletion.delete_document_by_external_docid(document_id); } - let update_id = data.db.update_write::<_, _, ResponseError>(|writer| Ok(documents_deletion.finalize(writer)?))?; + let update_id = data.db.update_write(|w| documents_deletion.finalize(w))?; Ok(HttpResponse::Accepted().json(IndexUpdateResponse::with_id(update_id))) } @@ -257,7 +246,7 @@ async fn clear_all_documents( .open_index(&path.index_uid) .ok_or(Error::index_not_found(&path.index_uid))?; - let update_id = data.db.update_write::<_, _, ResponseError>(|writer| Ok(index.clear_all(writer)?))?; + let update_id = data.db.update_write(|w| index.clear_all(w))?; Ok(HttpResponse::Accepted().json(IndexUpdateResponse::with_id(update_id))) } diff --git a/meilisearch-http/src/routes/health.rs b/meilisearch-http/src/routes/health.rs index c6f33b6fb..0f0a3a203 100644 --- a/meilisearch-http/src/routes/health.rs +++ b/meilisearch-http/src/routes/health.rs @@ -20,20 +20,12 @@ async fn get_health(data: web::Data) -> Result) -> Result { - data.db.main_write::<_, _, ResponseError>(|writer| { - data.db.set_healthy(writer)?; - Ok(()) - })?; - + data.db.main_write(|w| data.db.set_healthy(w))?; Ok(HttpResponse::Ok().finish()) } async fn set_unhealthy(data: web::Data) -> Result { - data.db.main_write::<_, _, ResponseError>(|writer| { - data.db.set_unhealthy(writer)?; - Ok(()) - })?; - + data.db.main_write(|w| data.db.set_unhealthy(w))?; Ok(HttpResponse::Ok().finish()) } diff --git a/meilisearch-http/src/routes/index.rs b/meilisearch-http/src/routes/index.rs index 6d8d654a0..f12179902 100644 --- a/meilisearch-http/src/routes/index.rs +++ b/meilisearch-http/src/routes/index.rs @@ -243,13 +243,13 @@ async fn update_index( .open_index(&path.index_uid) .ok_or(Error::index_not_found(&path.index_uid))?; - data.db.main_write::<_, _, ResponseError>(|mut writer| { + data.db.main_write::<_, _, ResponseError>(|writer| { if let Some(name) = &body.name { - index.main.put_name(&mut writer, name)?; + index.main.put_name(writer, name)?; } if let Some(id) = body.primary_key.clone() { - if let Some(mut schema) = index.main.schema(&writer)? { + if let Some(mut schema) = index.main.schema(writer)? { match schema.primary_key() { Some(_) => { return Err(Error::bad_request( @@ -258,12 +258,12 @@ async fn update_index( } None => { schema.set_primary_key(&id)?; - index.main.put_schema(&mut writer, &schema)?; + index.main.put_schema(writer, &schema)?; } } } } - index.main.put_updated_at(&mut writer)?; + index.main.put_updated_at(writer)?; Ok(()) })?; diff --git a/meilisearch-http/src/routes/setting.rs b/meilisearch-http/src/routes/setting.rs index 6bb883ee8..c7abd6ce4 100644 --- a/meilisearch-http/src/routes/setting.rs +++ b/meilisearch-http/src/routes/setting.rs @@ -153,10 +153,7 @@ async fn delete_all( attributes_for_faceting: UpdateState::Clear, }; - let update_id = data.db.update_write::<_, _, ResponseError>(|writer| { - let update_id = index.settings_update(writer, settings)?; - Ok(update_id) - })?; + let update_id = data.db.update_write(|w| index.settings_update(w, settings))?; Ok(HttpResponse::Accepted().json(IndexUpdateResponse::with_id(update_id))) } @@ -206,11 +203,7 @@ async fn update_rules( }; let settings = settings.into_update().map_err(Error::bad_request)?; - - let update_id = data.db.update_write::<_, _, ResponseError>(|writer| { - let update_id = index.settings_update(writer, settings)?; - Ok(update_id) - })?; + let update_id = data.db.update_write(|w| index.settings_update(w, settings))?; Ok(HttpResponse::Accepted().json(IndexUpdateResponse::with_id(update_id))) } @@ -233,10 +226,7 @@ async fn delete_rules( ..SettingsUpdate::default() }; - let update_id = data.db.update_write::<_, _, ResponseError>(|writer| { - let update_id = index.settings_update(writer, settings)?; - Ok(update_id) - })?; + let update_id = data.db.update_write(|w| index.settings_update(w, settings))?; Ok(HttpResponse::Accepted().json(IndexUpdateResponse::with_id(update_id))) } @@ -279,11 +269,7 @@ async fn update_distinct( }; let settings = settings.into_update().map_err(Error::bad_request)?; - - let update_id = data.db.update_write::<_, _, ResponseError>(|writer| { - let update_id = index.settings_update(writer, settings)?; - Ok(update_id) - })?; + let update_id = data.db.update_write(|w| index.settings_update(w, settings))?; Ok(HttpResponse::Accepted().json(IndexUpdateResponse::with_id(update_id))) } @@ -306,10 +292,7 @@ async fn delete_distinct( ..SettingsUpdate::default() }; - let update_id = data.db.update_write::<_, _, ResponseError>(|writer| { - let update_id = index.settings_update(writer, settings)?; - Ok(update_id) - })?; + let update_id = data.db.update_write(|w| index.settings_update(w, settings))?; Ok(HttpResponse::Accepted().json(IndexUpdateResponse::with_id(update_id))) } @@ -355,10 +338,7 @@ async fn update_searchable( let settings = settings.into_update().map_err(Error::bad_request)?; - let update_id = data.db.update_write::<_, _, ResponseError>(|writer| { - let update_id = index.settings_update(writer, settings)?; - Ok(update_id) - })?; + let update_id = data.db.update_write(|w| index.settings_update(w, settings))?; Ok(HttpResponse::Accepted().json(IndexUpdateResponse::with_id(update_id))) } @@ -381,10 +361,7 @@ async fn delete_searchable( ..SettingsUpdate::default() }; - let update_id = data.db.update_write::<_, _, ResponseError>(|writer| { - let update_id = index.settings_update(writer, settings)?; - Ok(update_id) - })?; + let update_id = data.db.update_write(|w| index.settings_update(w, settings))?; Ok(HttpResponse::Accepted().json(IndexUpdateResponse::with_id(update_id))) } @@ -431,11 +408,7 @@ async fn update_displayed( }; let settings = settings.into_update().map_err(Error::bad_request)?; - - let update_id = data.db.update_write::<_, _, ResponseError>(|writer| { - let update_id = index.settings_update(writer, settings)?; - Ok(update_id) - })?; + let update_id = data.db.update_write(|w| index.settings_update(w, settings))?; Ok(HttpResponse::Accepted().json(IndexUpdateResponse::with_id(update_id))) } @@ -458,10 +431,7 @@ async fn delete_displayed( ..SettingsUpdate::default() }; - let update_id = data.db.update_write::<_, _, ResponseError>(|writer| { - let update_id = index.settings_update(writer, settings)?; - Ok(update_id) - })?; + let update_id = data.db.update_write(|w| index.settings_update(w, settings))?; Ok(HttpResponse::Accepted().json(IndexUpdateResponse::with_id(update_id))) } @@ -507,11 +477,7 @@ async fn update_accept_new_fields( }; let settings = settings.into_update().map_err(Error::bad_request)?; - - let update_id = data.db.update_write::<_, _, ResponseError>(|writer| { - let update_id = index.settings_update(writer, settings)?; - Ok(update_id) - })?; + let update_id = data.db.update_write(|w| index.settings_update(w, settings))?; Ok(HttpResponse::Accepted().json(IndexUpdateResponse::with_id(update_id))) } diff --git a/meilisearch-http/src/routes/stop_words.rs b/meilisearch-http/src/routes/stop_words.rs index 0c22cbd13..eaea22827 100644 --- a/meilisearch-http/src/routes/stop_words.rs +++ b/meilisearch-http/src/routes/stop_words.rs @@ -49,10 +49,7 @@ async fn update( ..SettingsUpdate::default() }; - let update_id = data.db.update_write::<_, _, ResponseError>(|writer| { - let update_id = index.settings_update(writer, settings)?; - Ok(update_id) - })?; + let update_id = data.db.update_write(|w| index.settings_update(w, settings))?; Ok(HttpResponse::Accepted().json(IndexUpdateResponse::with_id(update_id))) } @@ -75,10 +72,7 @@ async fn delete( ..SettingsUpdate::default() }; - let update_id = data.db.update_write::<_, _, ResponseError>(|writer| { - let update_id = index.settings_update(writer, settings)?; - Ok(update_id) - })?; + let update_id = data.db.update_write(|w| index.settings_update(w, settings))?; Ok(HttpResponse::Accepted().json(IndexUpdateResponse::with_id(update_id))) } diff --git a/meilisearch-http/src/routes/synonym.rs b/meilisearch-http/src/routes/synonym.rs index 169230adb..f047e9dd3 100644 --- a/meilisearch-http/src/routes/synonym.rs +++ b/meilisearch-http/src/routes/synonym.rs @@ -60,10 +60,7 @@ async fn update( ..SettingsUpdate::default() }; - let update_id = data.db.update_write::<_, _, ResponseError>(|writer| { - let update_id = index.settings_update(writer, settings)?; - Ok(update_id) - })?; + let update_id = data.db.update_write(|w| index.settings_update(w, settings))?; Ok(HttpResponse::Accepted().json(IndexUpdateResponse::with_id(update_id))) } @@ -86,10 +83,7 @@ async fn delete( ..SettingsUpdate::default() }; - let update_id = data.db.update_write::<_, _, ResponseError>(|writer| { - let update_id = index.settings_update(writer, settings)?; - Ok(update_id) - })?; + let update_id = data.db.update_write(|w| index.settings_update(w, settings))?; Ok(HttpResponse::Accepted().json(IndexUpdateResponse::with_id(update_id))) } diff --git a/meilisearch-http/tests/documents_delete.rs b/meilisearch-http/tests/documents_delete.rs index 5aa3e595e..cd0bfc142 100644 --- a/meilisearch-http/tests/documents_delete.rs +++ b/meilisearch-http/tests/documents_delete.rs @@ -21,7 +21,6 @@ async fn delete_batch() { server.populate_movies().await; let (_response, status_code) = server.get_document(419704).await; - println!("{:?}", _response); assert_eq!(status_code, 200); let body = serde_json::json!([419704, 512200, 181812]);