From bb405aa729be1c6459cb92efdb9f7fe2cc2353e0 Mon Sep 17 00:00:00 2001 From: Kerollmops Date: Thu, 2 Jun 2022 11:48:59 +0200 Subject: [PATCH 1/7] Update the /indexes/{indexUid} verb from PUT to PATCH --- meilisearch-http/src/routes/indexes/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/meilisearch-http/src/routes/indexes/mod.rs b/meilisearch-http/src/routes/indexes/mod.rs index 37f4ee7b8..c1d2fabf1 100644 --- a/meilisearch-http/src/routes/indexes/mod.rs +++ b/meilisearch-http/src/routes/indexes/mod.rs @@ -27,7 +27,7 @@ pub fn configure(cfg: &mut web::ServiceConfig) { .service( web::resource("") .route(web::get().to(SeqHandler(get_index))) - .route(web::put().to(SeqHandler(update_index))) + .route(web::patch().to(SeqHandler(update_index))) .route(web::delete().to(SeqHandler(delete_index))), ) .service(web::resource("/stats").route(web::get().to(SeqHandler(get_index_stats)))) From f8d3f739ad2aaf69ef9c36910cab9139abdcc0f6 Mon Sep 17 00:00:00 2001 From: Kerollmops Date: Thu, 2 Jun 2022 11:49:46 +0200 Subject: [PATCH 2/7] Update the /indexes/{indexUid}/settings verb from POST to PATCH --- meilisearch-http/src/routes/indexes/settings.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/meilisearch-http/src/routes/indexes/settings.rs b/meilisearch-http/src/routes/indexes/settings.rs index 222aca580..59a965288 100644 --- a/meilisearch-http/src/routes/indexes/settings.rs +++ b/meilisearch-http/src/routes/indexes/settings.rs @@ -271,7 +271,7 @@ macro_rules! generate_configure { use crate::extractors::sequential_extractor::SeqHandler; cfg.service( web::resource("") - .route(web::post().to(SeqHandler(update_all))) + .route(web::patch().to(SeqHandler(update_all))) .route(web::get().to(SeqHandler(get_all))) .route(web::delete().to(SeqHandler(delete_all)))) $(.service($mod::resources()))*; From 10a71fdb102096fdce4d724136d23ab2dbdffda3 Mon Sep 17 00:00:00 2001 From: Kerollmops Date: Thu, 2 Jun 2022 11:53:16 +0200 Subject: [PATCH 3/7] Update the /indexes/{indexUid}/settings/* verbs by adding a macro parameter --- .../src/routes/indexes/settings.rs | 25 ++++++++++++++++--- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/meilisearch-http/src/routes/indexes/settings.rs b/meilisearch-http/src/routes/indexes/settings.rs index 59a965288..9efa825f8 100644 --- a/meilisearch-http/src/routes/indexes/settings.rs +++ b/meilisearch-http/src/routes/indexes/settings.rs @@ -13,7 +13,7 @@ use crate::task::SummarizedTaskView; #[macro_export] macro_rules! make_setting_route { - ($route:literal, $type:ty, $attr:ident, $camelcase_attr:literal, $analytics_var:ident, $analytics:expr) => { + ($route:literal, $update_verb:ident, $type:ty, $attr:ident, $camelcase_attr:literal, $analytics_var:ident, $analytics:expr) => { pub mod $attr { use actix_web::{web, HttpRequest, HttpResponse, Resource}; use log::debug; @@ -100,18 +100,27 @@ macro_rules! make_setting_route { pub fn resources() -> Resource { Resource::new($route) .route(web::get().to(SeqHandler(get))) - .route(web::post().to(SeqHandler(update))) + .route(web::$update_verb().to(SeqHandler(update))) .route(web::delete().to(SeqHandler(delete))) } } }; - ($route:literal, $type:ty, $attr:ident, $camelcase_attr:literal) => { - make_setting_route!($route, $type, $attr, $camelcase_attr, _analytics, |_, _| {}); + ($route:literal, $update_verb:ident, $type:ty, $attr:ident, $camelcase_attr:literal) => { + make_setting_route!( + $route, + $update_verb, + $type, + $attr, + $camelcase_attr, + _analytics, + |_, _| {} + ); }; } make_setting_route!( "/filterable-attributes", + put, std::collections::BTreeSet, filterable_attributes, "filterableAttributes", @@ -134,6 +143,7 @@ make_setting_route!( make_setting_route!( "/sortable-attributes", + put, std::collections::BTreeSet, sortable_attributes, "sortableAttributes", @@ -156,6 +166,7 @@ make_setting_route!( make_setting_route!( "/displayed-attributes", + put, Vec, displayed_attributes, "displayedAttributes" @@ -163,6 +174,7 @@ make_setting_route!( make_setting_route!( "/typo-tolerance", + patch, meilisearch_lib::index::updates::TypoSettings, typo_tolerance, "typoTolerance", @@ -204,6 +216,7 @@ make_setting_route!( make_setting_route!( "/searchable-attributes", + put, Vec, searchable_attributes, "searchableAttributes", @@ -225,6 +238,7 @@ make_setting_route!( make_setting_route!( "/stop-words", + put, std::collections::BTreeSet, stop_words, "stopWords" @@ -232,6 +246,7 @@ make_setting_route!( make_setting_route!( "/synonyms", + put, std::collections::BTreeMap>, synonyms, "synonyms" @@ -239,6 +254,7 @@ make_setting_route!( make_setting_route!( "/distinct-attribute", + put, String, distinct_attribute, "distinctAttribute" @@ -246,6 +262,7 @@ make_setting_route!( make_setting_route!( "/ranking-rules", + put, Vec, ranking_rules, "rankingRules", From bcb51905d7f468aea43ee85142aed8e19a9dc870 Mon Sep 17 00:00:00 2001 From: Kerollmops Date: Thu, 2 Jun 2022 12:16:46 +0200 Subject: [PATCH 4/7] Fix the authorization tests --- Cargo.lock | 1 - meilisearch-http/Cargo.toml | 1 - meilisearch-http/tests/auth/authorization.rs | 20 +++---- meilisearch-http/tests/common/index.rs | 37 ++++++------ meilisearch-http/tests/content_type.rs | 59 ++++++++++++++------ 5 files changed, 70 insertions(+), 48 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index ea115c459..1f7d93ed2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2036,7 +2036,6 @@ dependencies = [ "obkv", "once_cell", "parking_lot", - "paste", "pin-project-lite", "platform-dirs", "rand", diff --git a/meilisearch-http/Cargo.toml b/meilisearch-http/Cargo.toml index ba11b20e0..b9771afa2 100644 --- a/meilisearch-http/Cargo.toml +++ b/meilisearch-http/Cargo.toml @@ -83,7 +83,6 @@ actix-rt = "2.7.0" assert-json-diff = "2.0.1" manifest-dir-macros = "0.1.14" maplit = "1.0.2" -paste = "1.0.6" serde_url_params = "0.2.1" urlencoding = "2.1.0" diff --git a/meilisearch-http/tests/auth/authorization.rs b/meilisearch-http/tests/auth/authorization.rs index 2080e2990..5d5d53d52 100644 --- a/meilisearch-http/tests/auth/authorization.rs +++ b/meilisearch-http/tests/auth/authorization.rs @@ -18,7 +18,7 @@ pub static AUTHORIZATIONS: Lazy hashset!{"tasks.get", "*"}, ("GET", "/tasks?indexUid=products") => hashset!{"tasks.get", "*"}, ("GET", "/tasks/0") => hashset!{"tasks.get", "*"}, - ("PUT", "/indexes/products/") => hashset!{"indexes.update", "*"}, + ("PATCH", "/indexes/products/") => hashset!{"indexes.update", "*"}, ("GET", "/indexes/products/") => hashset!{"indexes.get", "*"}, ("DELETE", "/indexes/products/") => hashset!{"indexes.delete", "*"}, ("POST", "/indexes") => hashset!{"indexes.create", "*"}, @@ -33,15 +33,15 @@ pub static AUTHORIZATIONS: Lazy hashset!{"settings.get", "*"}, ("GET", "/indexes/products/settings/synonyms") => hashset!{"settings.get", "*"}, ("DELETE", "/indexes/products/settings") => hashset!{"settings.update", "*"}, - ("POST", "/indexes/products/settings") => hashset!{"settings.update", "*"}, - ("POST", "/indexes/products/settings/displayed-attributes") => hashset!{"settings.update", "*"}, - ("POST", "/indexes/products/settings/distinct-attribute") => hashset!{"settings.update", "*"}, - ("POST", "/indexes/products/settings/filterable-attributes") => hashset!{"settings.update", "*"}, - ("POST", "/indexes/products/settings/ranking-rules") => hashset!{"settings.update", "*"}, - ("POST", "/indexes/products/settings/searchable-attributes") => hashset!{"settings.update", "*"}, - ("POST", "/indexes/products/settings/sortable-attributes") => hashset!{"settings.update", "*"}, - ("POST", "/indexes/products/settings/stop-words") => hashset!{"settings.update", "*"}, - ("POST", "/indexes/products/settings/synonyms") => hashset!{"settings.update", "*"}, + ("PATCH", "/indexes/products/settings") => hashset!{"settings.update", "*"}, + ("PUT", "/indexes/products/settings/displayed-attributes") => hashset!{"settings.update", "*"}, + ("PUT", "/indexes/products/settings/distinct-attribute") => hashset!{"settings.update", "*"}, + ("PUT", "/indexes/products/settings/filterable-attributes") => hashset!{"settings.update", "*"}, + ("PUT", "/indexes/products/settings/ranking-rules") => hashset!{"settings.update", "*"}, + ("PUT", "/indexes/products/settings/searchable-attributes") => hashset!{"settings.update", "*"}, + ("PUT", "/indexes/products/settings/sortable-attributes") => hashset!{"settings.update", "*"}, + ("PUT", "/indexes/products/settings/stop-words") => hashset!{"settings.update", "*"}, + ("PUT", "/indexes/products/settings/synonyms") => hashset!{"settings.update", "*"}, ("GET", "/indexes/products/stats") => hashset!{"stats.get", "*"}, ("GET", "/stats") => hashset!{"stats.get", "*"}, ("POST", "/dumps") => hashset!{"dumps.create", "*"}, diff --git a/meilisearch-http/tests/common/index.rs b/meilisearch-http/tests/common/index.rs index 4be8ad873..edda799e0 100644 --- a/meilisearch-http/tests/common/index.rs +++ b/meilisearch-http/tests/common/index.rs @@ -4,29 +4,12 @@ use std::{ }; use actix_web::http::StatusCode; -use paste::paste; use serde_json::{json, Value}; use tokio::time::sleep; use urlencoding::encode; use super::service::Service; -macro_rules! make_settings_test_routes { - ($($name:ident),+) => { - $(paste! { - pub async fn [](&self, value: Value) -> (Value, StatusCode) { - let url = format!("/indexes/{}/settings/{}", encode(self.uid.as_ref()).to_string(), stringify!($name).replace("_", "-")); - self.service.post(url, value).await - } - - pub async fn [](&self) -> (Value, StatusCode) { - let url = format!("/indexes/{}/settings/{}", encode(self.uid.as_ref()).to_string(), stringify!($name).replace("_", "-")); - self.service.get(url).await - } - })* - }; -} - pub struct Index<'a> { pub uid: String, pub service: &'a Service, @@ -198,7 +181,7 @@ impl Index<'_> { pub async fn update_settings(&self, settings: Value) -> (Value, StatusCode) { let url = format!("/indexes/{}/settings", encode(self.uid.as_ref())); - self.service.post(url, settings).await + self.service.patch(url, settings).await } pub async fn delete_settings(&self) -> (Value, StatusCode) { @@ -242,7 +225,23 @@ impl Index<'_> { self.service.get(url).await } - make_settings_test_routes!(distinct_attribute); + pub async fn update_distinct_attribute(&self, value: Value) -> (Value, StatusCode) { + let url = format!( + "/indexes/{}/settings/{}", + encode(self.uid.as_ref()).to_string(), + "distinct-attribute" + ); + self.service.put(url, value).await + } + + pub async fn get_distinct_attribute(&self) -> (Value, StatusCode) { + let url = format!( + "/indexes/{}/settings/{}", + encode(self.uid.as_ref()).to_string(), + "distinct-attribute" + ); + self.service.get(url).await + } } pub struct GetDocumentOptions { diff --git a/meilisearch-http/tests/content_type.rs b/meilisearch-http/tests/content_type.rs index d6b4cbd78..eace67a08 100644 --- a/meilisearch-http/tests/content_type.rs +++ b/meilisearch-http/tests/content_type.rs @@ -7,23 +7,45 @@ use actix_web::test; use meilisearch_http::{analytics, create_app}; use serde_json::{json, Value}; +enum HttpVerb { + Put, + Patch, + Post, + Get, + Delete, +} + +impl HttpVerb { + fn test_request(&self) -> test::TestRequest { + match self { + HttpVerb::Put => test::TestRequest::put(), + HttpVerb::Patch => test::TestRequest::patch(), + HttpVerb::Post => test::TestRequest::post(), + HttpVerb::Get => test::TestRequest::get(), + HttpVerb::Delete => test::TestRequest::delete(), + } + } +} + #[actix_rt::test] async fn error_json_bad_content_type() { + use HttpVerb::{Patch, Post, Put}; + let routes = [ - // all the POST routes except the dumps that can be created without any body or content-type + // all the routes except the dumps that can be created without any body or content-type // and the search that is not a strict json - "/indexes", - "/indexes/doggo/documents/delete-batch", - "/indexes/doggo/search", - "/indexes/doggo/settings", - "/indexes/doggo/settings/displayed-attributes", - "/indexes/doggo/settings/distinct-attribute", - "/indexes/doggo/settings/filterable-attributes", - "/indexes/doggo/settings/ranking-rules", - "/indexes/doggo/settings/searchable-attributes", - "/indexes/doggo/settings/sortable-attributes", - "/indexes/doggo/settings/stop-words", - "/indexes/doggo/settings/synonyms", + (Post, "/indexes"), + (Post, "/indexes/doggo/documents/delete-batch"), + (Post, "/indexes/doggo/search"), + (Patch, "/indexes/doggo/settings"), + (Put, "/indexes/doggo/settings/displayed-attributes"), + (Put, "/indexes/doggo/settings/distinct-attribute"), + (Put, "/indexes/doggo/settings/filterable-attributes"), + (Put, "/indexes/doggo/settings/ranking-rules"), + (Put, "/indexes/doggo/settings/searchable-attributes"), + (Put, "/indexes/doggo/settings/sortable-attributes"), + (Put, "/indexes/doggo/settings/stop-words"), + (Put, "/indexes/doggo/settings/synonyms"), ]; let bad_content_types = [ "application/csv", @@ -45,10 +67,11 @@ async fn error_json_bad_content_type() { analytics::MockAnalytics::new(&server.service.options).0 )) .await; - for route in routes { + for (verb, route) in routes { // Good content-type, we probably have an error since we didn't send anything in the json // so we only ensure we didn't get a bad media type error. - let req = test::TestRequest::post() + let req = verb + .test_request() .uri(route) .set_payload(document) .insert_header(("content-type", "application/json")) @@ -59,7 +82,8 @@ async fn error_json_bad_content_type() { "calling the route `{}` with a content-type of json isn't supposed to throw a bad media type error", route); // No content-type. - let req = test::TestRequest::post() + let req = verb + .test_request() .uri(route) .set_payload(document) .to_request(); @@ -82,7 +106,8 @@ async fn error_json_bad_content_type() { for bad_content_type in bad_content_types { // Always bad content-type - let req = test::TestRequest::post() + let req = verb + .test_request() .uri(route) .set_payload(document.to_string()) .insert_header(("content-type", bad_content_type)) From ce37f53a1605eaa3de308adfec656f5eff222a24 Mon Sep 17 00:00:00 2001 From: Kerollmops Date: Thu, 2 Jun 2022 12:17:53 +0200 Subject: [PATCH 5/7] Add typo-tolerance to the authorization tests --- meilisearch-http/tests/auth/authorization.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/meilisearch-http/tests/auth/authorization.rs b/meilisearch-http/tests/auth/authorization.rs index 5d5d53d52..7846188fb 100644 --- a/meilisearch-http/tests/auth/authorization.rs +++ b/meilisearch-http/tests/auth/authorization.rs @@ -34,6 +34,7 @@ pub static AUTHORIZATIONS: Lazy hashset!{"settings.get", "*"}, ("DELETE", "/indexes/products/settings") => hashset!{"settings.update", "*"}, ("PATCH", "/indexes/products/settings") => hashset!{"settings.update", "*"}, + ("PATCH", "/indexes/products/settings/typo-tolerance") => hashset!{"settings.update", "*"}, ("PUT", "/indexes/products/settings/displayed-attributes") => hashset!{"settings.update", "*"}, ("PUT", "/indexes/products/settings/distinct-attribute") => hashset!{"settings.update", "*"}, ("PUT", "/indexes/products/settings/filterable-attributes") => hashset!{"settings.update", "*"}, From 0258659278820dcb9958c42e14cc4dbcb66cde1e Mon Sep 17 00:00:00 2001 From: Kerollmops Date: Thu, 2 Jun 2022 12:24:27 +0200 Subject: [PATCH 6/7] Fix the get_settings tests --- meilisearch-http/tests/common/index.rs | 2 +- meilisearch-http/tests/settings/get_settings.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/meilisearch-http/tests/common/index.rs b/meilisearch-http/tests/common/index.rs index edda799e0..0a9b61d25 100644 --- a/meilisearch-http/tests/common/index.rs +++ b/meilisearch-http/tests/common/index.rs @@ -48,7 +48,7 @@ impl Index<'_> { }); let url = format!("/indexes/{}", encode(self.uid.as_ref())); - self.service.put(url, body).await + self.service.patch(url, body).await } pub async fn delete(&self) -> (Value, StatusCode) { diff --git a/meilisearch-http/tests/settings/get_settings.rs b/meilisearch-http/tests/settings/get_settings.rs index 9b3c31b63..e79b3ed26 100644 --- a/meilisearch-http/tests/settings/get_settings.rs +++ b/meilisearch-http/tests/settings/get_settings.rs @@ -214,7 +214,7 @@ macro_rules! test_setting_routes { .chars() .map(|c| if c == '_' { '-' } else { c }) .collect::()); - let (response, code) = server.service.post(url, serde_json::Value::Null).await; + let (response, code) = server.service.put(url, serde_json::Value::Null).await; assert_eq!(code, 202, "{}", response); server.index("").wait_task(0).await; let (response, code) = server.index("test").get().await; From 419922e475777cd32bf2b47bd5d5e5db97a6d439 Mon Sep 17 00:00:00 2001 From: Kerollmops Date: Thu, 2 Jun 2022 13:38:23 +0200 Subject: [PATCH 7/7] Make clippy happy --- meilisearch-http/tests/common/index.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/meilisearch-http/tests/common/index.rs b/meilisearch-http/tests/common/index.rs index 0a9b61d25..275bec4cd 100644 --- a/meilisearch-http/tests/common/index.rs +++ b/meilisearch-http/tests/common/index.rs @@ -228,7 +228,7 @@ impl Index<'_> { pub async fn update_distinct_attribute(&self, value: Value) -> (Value, StatusCode) { let url = format!( "/indexes/{}/settings/{}", - encode(self.uid.as_ref()).to_string(), + encode(self.uid.as_ref()), "distinct-attribute" ); self.service.put(url, value).await @@ -237,7 +237,7 @@ impl Index<'_> { pub async fn get_distinct_attribute(&self) -> (Value, StatusCode) { let url = format!( "/indexes/{}/settings/{}", - encode(self.uid.as_ref()).to_string(), + encode(self.uid.as_ref()), "distinct-attribute" ); self.service.get(url).await