Fix the /swap-indexes route API

1. payload
2. error messages
3. auth errors

meilisearch-http/src/routes/indexes_swap.rs

@@ -1,70 +0,0 @@
-use std::collections::HashSet;
-
-use actix_web::web::Data;
-use actix_web::{web, HttpResponse};
-use index_scheduler::IndexScheduler;
-use meilisearch_types::error::{Code, ResponseError};
-use meilisearch_types::tasks::KindWithContent;
-use serde::Deserialize;
-
-use crate::extractors::authentication::policies::*;
-use crate::extractors::authentication::GuardedData;
-use crate::extractors::sequential_extractor::SeqHandler;
-use crate::routes::tasks::TaskView;
-
-pub fn configure(cfg: &mut web::ServiceConfig) {
-    cfg.service(web::resource("").route(web::post().to(SeqHandler(indexes_swap))));
-}
-
-// TODO: Lo: revisit this struct once we have decided on what the payload should be
-#[derive(Deserialize, Debug, Clone, PartialEq, Eq)]
-#[serde(rename_all = "camelCase", deny_unknown_fields)]
-pub struct IndexesSwapPayload {
-    indexes: (String, String),
-}
-
-pub async fn indexes_swap(
-    index_scheduler: GuardedData<ActionPolicy<{ actions::INDEXES_SWAP }>, Data<IndexScheduler>>,
-    params: web::Json<Vec<IndexesSwapPayload>>,
-) -> Result<HttpResponse, ResponseError> {
-    let search_rules = &index_scheduler.filters().search_rules;
-
-    // TODO: Lo: error when the params are empty
-    // TODO: Lo: error when the same index appears more than once
-    // TODO: Lo: error when not authorized to swap
-
-    let mut swaps = vec![];
-    let mut indexes_set = HashSet::<String>::default();
-    for IndexesSwapPayload { indexes: (lhs, rhs) } in params.into_inner().into_iter() {
-        if !search_rules.is_index_authorized(&lhs) || !search_rules.is_index_authorized(&rhs) {
-            return Err(ResponseError::from_msg(
-                "TODO: error message when we swap with an index were not allowed to access"
-                    .to_owned(),
-                Code::BadRequest,
-            ));
-        }
-        swaps.push((lhs.clone(), rhs.clone()));
-        // TODO: Lo: should this check be here or within the index scheduler?
-        let is_unique_index_lhs = indexes_set.insert(lhs);
-        if !is_unique_index_lhs {
-            return Err(ResponseError::from_msg(
-                "TODO: error message when same index is in more than one swap".to_owned(),
-                Code::BadRequest,
-            ));
-        }
-        let is_unique_index_rhs = indexes_set.insert(rhs);
-        if !is_unique_index_rhs {
-            return Err(ResponseError::from_msg(
-                "TODO: error message when same index is in more than one swap".to_owned(),
-                Code::BadRequest,
-            ));
-        }
-    }
-
-    let task = KindWithContent::IndexSwap { swaps };
-
-    let task = index_scheduler.register(task)?;
-    let task_view = TaskView::from_task(&task);
-
-    Ok(HttpResponse::Accepted().json(task_view))
-}

meilisearch-http/src/routes/mod.rs

@@ -20,7 +20,7 @@ use crate::extractors::authentication::GuardedData;
 mod api_key;
 mod dump;
 pub mod indexes;
-mod indexes_swap;
+mod swap_indexes;
 mod tasks;
 
 pub fn configure(cfg: &mut web::ServiceConfig) {
@@ -31,7 +31,7 @@ pub fn configure(cfg: &mut web::ServiceConfig) {
         .service(web::resource("/stats").route(web::get().to(get_stats)))
         .service(web::resource("/version").route(web::get().to(get_version)))
         .service(web::scope("/indexes").configure(indexes::configure))
-        .service(web::scope("indexes-swap").configure(indexes_swap::configure));
+        .service(web::scope("swap-indexes").configure(swap_indexes::configure));
 }
 
 /// Extracts the raw values from the `StarOr` types and

meilisearch-http/src/routes/swap_indexes.rs

@@ -0,0 +1,131 @@
+use std::collections::HashSet;
+
+use actix_web::web::Data;
+use actix_web::{web, HttpResponse};
+use index_scheduler::IndexScheduler;
+use meilisearch_types::error::ResponseError;
+use meilisearch_types::tasks::KindWithContent;
+use serde::Deserialize;
+
+use self::errors::{DuplicateSwappedIndexError, IndexesNotFoundError};
+use crate::extractors::authentication::policies::*;
+use crate::extractors::authentication::GuardedData;
+use crate::extractors::sequential_extractor::SeqHandler;
+use crate::routes::tasks::TaskView;
+
+pub fn configure(cfg: &mut web::ServiceConfig) {
+    cfg.service(web::resource("").route(web::post().to(SeqHandler(swap_indexes))));
+}
+
+#[derive(Deserialize, Debug, Clone, PartialEq, Eq)]
+#[serde(rename_all = "camelCase", deny_unknown_fields)]
+pub struct SwapIndexesPayload {
+    swap: (String, String),
+}
+
+pub async fn swap_indexes(
+    index_scheduler: GuardedData<ActionPolicy<{ actions::INDEXES_SWAP }>, Data<IndexScheduler>>,
+    params: web::Json<Vec<SwapIndexesPayload>>,
+) -> Result<HttpResponse, ResponseError> {
+    let search_rules = &index_scheduler.filters().search_rules;
+
+    let mut swaps = vec![];
+    let mut indexes_set = HashSet::<String>::default();
+    let mut unknown_indexes = HashSet::new();
+    let mut duplicate_indexes = HashSet::new();
+    for SwapIndexesPayload { swap: (lhs, rhs) } in params.into_inner().into_iter() {
+        if !search_rules.is_index_authorized(&lhs) {
+            unknown_indexes.insert(lhs.clone());
+        }
+        if !search_rules.is_index_authorized(&rhs) {
+            unknown_indexes.insert(rhs.clone());
+        }
+
+        swaps.push((lhs.clone(), rhs.clone()));
+
+        let is_unique_index_lhs = indexes_set.insert(lhs.clone());
+        if !is_unique_index_lhs {
+            duplicate_indexes.insert(lhs);
+        }
+        let is_unique_index_rhs = indexes_set.insert(rhs.clone());
+        if !is_unique_index_rhs {
+            duplicate_indexes.insert(rhs);
+        }
+    }
+    if !duplicate_indexes.is_empty() {
+        return Err(DuplicateSwappedIndexError {
+            indexes: duplicate_indexes.into_iter().collect(),
+        }
+        .into());
+    }
+    if !unknown_indexes.is_empty() {
+        return Err(IndexesNotFoundError { indexes: unknown_indexes.into_iter().collect() }.into());
+    }
+
+    let task = KindWithContent::IndexSwap { swaps };
+
+    let task = index_scheduler.register(task)?;
+    let task_view = TaskView::from_task(&task);
+
+    Ok(HttpResponse::Accepted().json(task_view))
+}
+
+pub mod errors {
+    use std::fmt::Display;
+
+    use meilisearch_types::error::{Code, ErrorCode};
+
+    #[derive(Debug)]
+    pub struct IndexesNotFoundError {
+        pub indexes: Vec<String>,
+    }
+    impl Display for IndexesNotFoundError {
+        fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+            if self.indexes.len() == 1 {
+                write!(f, "Index `{}` not found,", self.indexes[0])?;
+            } else {
+                write!(f, "Indexes `{}`", self.indexes[0])?;
+                for index in self.indexes.iter().skip(1) {
+                    write!(f, ", `{}`", index)?;
+                }
+                write!(f, "not found.")?;
+            }
+            Ok(())
+        }
+    }
+    impl std::error::Error for IndexesNotFoundError {}
+    impl ErrorCode for IndexesNotFoundError {
+        fn error_code(&self) -> Code {
+            Code::IndexNotFound
+        }
+    }
+    #[derive(Debug)]
+    pub struct DuplicateSwappedIndexError {
+        pub indexes: Vec<String>,
+    }
+    impl Display for DuplicateSwappedIndexError {
+        fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+            if self.indexes.len() == 1 {
+                write!(f, "Indexes must be declared only once during a swap. `{}` was specified several times.", self.indexes[0])?;
+            } else {
+                write!(
+                    f,
+                    "Indexes must be declared only once during a swap. `{}`",
+                    self.indexes[0]
+                )?;
+                for index in self.indexes.iter().skip(1) {
+                    write!(f, ", `{}`", index)?;
+                }
+                write!(f, "were specified several times.")?;
+            }
+
+            Ok(())
+        }
+    }
+    impl std::error::Error for DuplicateSwappedIndexError {}
+    impl ErrorCode for DuplicateSwappedIndexError {
+        fn error_code(&self) -> Code {
+            Code::DuplicateIndexFound
+        }
+    }
+}

meilisearch-http/tests/auth/authorization.rs

@@ -26,7 +26,7 @@ pub static AUTHORIZATIONS: Lazy<HashMap<(&'static str, &'static str), HashSet<&'
             ("DELETE",  "/indexes/products/") =>                               hashset!{"indexes.delete", "indexes.*", "*"},
             ("POST",    "/indexes") =>                                         hashset!{"indexes.create", "indexes.*", "*"},
             ("GET",     "/indexes") =>                                         hashset!{"indexes.get", "indexes.*", "*"},
-            // ("POST",    "/indexes-swap") =>                                    hashset!{"indexes.swap", "indexes.*", "*"}, // TODO: uncomment and fix this test
+            ("POST",    "/swap-indexes") =>                                    hashset!{"indexes.swap", "indexes.*", "*"},
             ("GET",     "/indexes/products/settings") =>                       hashset!{"settings.get", "settings.*", "*"},
             ("GET",     "/indexes/products/settings/displayed-attributes") =>  hashset!{"settings.get", "settings.*", "*"},
             ("GET",     "/indexes/products/settings/distinct-attribute") =>    hashset!{"settings.get", "settings.*", "*"},

meilisearch-types/src/error.rs

@@ -120,6 +120,8 @@ pub enum Code {
     InvalidIndexUid,
     InvalidMinWordLengthForTypo,
 
+    DuplicateIndexFound,
+
     // invalid state error
     InvalidState,
     MissingPrimaryKey,
@@ -298,6 +300,9 @@ impl Code {
             InvalidMinWordLengthForTypo => {
                 ErrCode::invalid("invalid_min_word_length_for_typo", StatusCode::BAD_REQUEST)
             }
+            DuplicateIndexFound => {
+                ErrCode::invalid("duplicate_index_found", StatusCode::BAD_REQUEST)
+            }
         }
     }
 

meilisearch-types/src/keys.rs

@@ -270,6 +270,7 @@ impl Action {
             INDEXES_GET => Some(Self::IndexesGet),
             INDEXES_UPDATE => Some(Self::IndexesUpdate),
             INDEXES_DELETE => Some(Self::IndexesDelete),
+            INDEXES_SWAP => Some(Self::IndexesSwap),
             TASKS_ALL => Some(Self::TasksAll),
             TASKS_CANCEL => Some(Self::TasksCancel),
             TASKS_DELETE => Some(Self::TasksDelete),