Merge #2452

2452: Change http verbs r=ManyTheFish a=Kerollmops

This PR fixes #2419 by updating the HTTP verbs used to update the settings and every single setting parameter.

- [x] `PATCH /indexes/{indexUid}` instead of `PUT`
- [x] `PATCH /indexes/{indexUid}/settings`  instead of `POST`
- [x] `PATCH /indexes/{indexUid}/settings/typo-tolerance`  instead of `POST`
- [x] `PUT /indexes/{indexUid}/settings/displayed-attributes`  instead of `POST`
- [x] `PUT /indexes/{indexUid}/settings/distinct-attribute`  instead of `POST`
- [x] `PUT /indexes/{indexUid}/settings/filterable-attributes`  instead of `POST`
- [x] `PUT /indexes/{indexUid}/settings/ranking-rules`  instead of `POST`
- [x] `PUT /indexes/{indexUid}/settings/searchable-attributes`  instead of `POST`
- [x] `PUT /indexes/{indexUid}/settings/sortable-attributes`  instead of `POST`
- [x] `PUT /indexes/{indexUid}/settings/stop-words`  instead of `POST`
- [x] `PUT /indexes/{indexUid}/settings/synonyms`  instead of `POST`


Co-authored-by: Kerollmops <clement@meilisearch.com>

Cargo.lock

@@ -2036,7 +2036,6 @@ dependencies = [
  "obkv",
  "once_cell",
  "parking_lot",
- "paste",
  "pin-project-lite",
  "platform-dirs",
  "rand",

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"
 

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))))

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) => {
+    ($route:literal, $update_verb:ident, $type:ty, $attr:ident, $camelcase_attr:literal) => {
-        make_setting_route!($route, $type, $attr, $camelcase_attr, _analytics, |_, _| {});
+        make_setting_route!(
+            $route,
+            $update_verb,
+            $type,
+            $attr,
+            $camelcase_attr,
+            _analytics,
+            |_, _| {}
+        );
     };
 }
 
 make_setting_route!(
     "/filterable-attributes",
+    put,
     std::collections::BTreeSet<String>,
     filterable_attributes,
     "filterableAttributes",
@@ -134,6 +143,7 @@ make_setting_route!(
 
 make_setting_route!(
     "/sortable-attributes",
+    put,
     std::collections::BTreeSet<String>,
     sortable_attributes,
     "sortableAttributes",
@@ -156,6 +166,7 @@ make_setting_route!(
 
 make_setting_route!(
     "/displayed-attributes",
+    put,
     Vec<String>,
     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<String>,
     searchable_attributes,
     "searchableAttributes",
@@ -225,6 +238,7 @@ make_setting_route!(
 
 make_setting_route!(
     "/stop-words",
+    put,
     std::collections::BTreeSet<String>,
     stop_words,
     "stopWords"
@@ -232,6 +246,7 @@ make_setting_route!(
 
 make_setting_route!(
     "/synonyms",
+    put,
     std::collections::BTreeMap<String, Vec<String>>,
     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<String>,
     ranking_rules,
     "rankingRules",
@@ -271,7 +288,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()))*;

meilisearch-http/tests/auth/authorization.rs

@@ -18,7 +18,7 @@ pub static AUTHORIZATIONS: Lazy<HashMap<(&'static str, &'static str), HashSet<&'
             ("GET",     "/tasks") =>                                           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,16 @@ pub static AUTHORIZATIONS: Lazy<HashMap<(&'static str, &'static str), HashSet<&'
             ("GET",     "/indexes/products/settings/stop-words") =>            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", "*"},
+            ("PATCH",   "/indexes/products/settings") =>                       hashset!{"settings.update", "*"},
-            ("POST",    "/indexes/products/settings/displayed-attributes") =>  hashset!{"settings.update", "*"},
+            ("PATCH",   "/indexes/products/settings/typo-tolerance") =>        hashset!{"settings.update", "*"},
-            ("POST",    "/indexes/products/settings/distinct-attribute") =>    hashset!{"settings.update", "*"},
+            ("PUT",     "/indexes/products/settings/displayed-attributes") =>  hashset!{"settings.update", "*"},
-            ("POST",    "/indexes/products/settings/filterable-attributes") => hashset!{"settings.update", "*"},
+            ("PUT",     "/indexes/products/settings/distinct-attribute") =>    hashset!{"settings.update", "*"},
-            ("POST",    "/indexes/products/settings/ranking-rules") =>         hashset!{"settings.update", "*"},
+            ("PUT",     "/indexes/products/settings/filterable-attributes") => hashset!{"settings.update", "*"},
-            ("POST",    "/indexes/products/settings/searchable-attributes") => hashset!{"settings.update", "*"},
+            ("PUT",     "/indexes/products/settings/ranking-rules") =>         hashset!{"settings.update", "*"},
-            ("POST",    "/indexes/products/settings/sortable-attributes") =>   hashset!{"settings.update", "*"},
+            ("PUT",     "/indexes/products/settings/searchable-attributes") => hashset!{"settings.update", "*"},
-            ("POST",    "/indexes/products/settings/stop-words") =>            hashset!{"settings.update", "*"},
+            ("PUT",     "/indexes/products/settings/sortable-attributes") =>   hashset!{"settings.update", "*"},
-            ("POST",    "/indexes/products/settings/synonyms") =>              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", "*"},

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 [<update_$name>](&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 [<get_$name>](&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,
@@ -65,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) {
@@ -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()),
+            "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()),
+            "distinct-attribute"
+        );
+        self.service.get(url).await
+    }
 }
 
 pub struct GetDocumentOptions {

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",
+        (Post, "/indexes"),
-        "/indexes/doggo/documents/delete-batch",
+        (Post, "/indexes/doggo/documents/delete-batch"),
-        "/indexes/doggo/search",
+        (Post, "/indexes/doggo/search"),
-        "/indexes/doggo/settings",
+        (Patch, "/indexes/doggo/settings"),
-        "/indexes/doggo/settings/displayed-attributes",
+        (Put, "/indexes/doggo/settings/displayed-attributes"),
-        "/indexes/doggo/settings/distinct-attribute",
+        (Put, "/indexes/doggo/settings/distinct-attribute"),
-        "/indexes/doggo/settings/filterable-attributes",
+        (Put, "/indexes/doggo/settings/filterable-attributes"),
-        "/indexes/doggo/settings/ranking-rules",
+        (Put, "/indexes/doggo/settings/ranking-rules"),
-        "/indexes/doggo/settings/searchable-attributes",
+        (Put, "/indexes/doggo/settings/searchable-attributes"),
-        "/indexes/doggo/settings/sortable-attributes",
+        (Put, "/indexes/doggo/settings/sortable-attributes"),
-        "/indexes/doggo/settings/stop-words",
+        (Put, "/indexes/doggo/settings/stop-words"),
-        "/indexes/doggo/settings/synonyms",
+        (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))

meilisearch-http/tests/settings/get_settings.rs

@@ -214,7 +214,7 @@ macro_rules! test_setting_routes {
                         .chars()
                         .map(|c| if c == '_' { '-' } else { c })
                         .collect::<String>());
-                    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;