Merge pull request #55 from meilisearch/fix-settings-delete

fix settings delete
This commit is contained in:
marin 2021-03-05 19:57:43 +01:00 committed by GitHub
commit 3992d917ec
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 18 additions and 14 deletions

View File

@ -59,13 +59,14 @@ impl Data {
pub async fn update_settings( pub async fn update_settings(
&self, &self,
index: impl AsRef<str> + Send + Sync + 'static, index: impl AsRef<str> + Send + Sync + 'static,
settings: Settings settings: Settings,
create: bool,
) -> anyhow::Result<UpdateStatus> { ) -> anyhow::Result<UpdateStatus> {
if !is_index_uid_valid(index.as_ref()) { if !is_index_uid_valid(index.as_ref()) {
bail!("invalid index uid: {:?}", index.as_ref()) bail!("invalid index uid: {:?}", index.as_ref())
} }
let index_controller = self.index_controller.clone(); let index_controller = self.index_controller.clone();
let update = tokio::task::spawn_blocking(move || index_controller.update_settings(index, settings)).await??; let update = tokio::task::spawn_blocking(move || index_controller.update_settings(index, settings, create)).await??;
Ok(update.into()) Ok(update.into())
} }

View File

@ -5,7 +5,7 @@ mod update_handler;
use std::path::Path; use std::path::Path;
use std::sync::Arc; use std::sync::Arc;
use anyhow::{bail, Context}; use anyhow::{bail, Context, anyhow};
use itertools::Itertools; use itertools::Itertools;
use milli::Index; use milli::Index;
@ -51,9 +51,14 @@ impl IndexController for LocalIndexController {
fn update_settings<S: AsRef<str>>( fn update_settings<S: AsRef<str>>(
&self, &self,
index: S, index: S,
settings: super::Settings settings: super::Settings,
create: bool,
) -> anyhow::Result<UpdateStatus<UpdateMeta, UpdateResult, String>> { ) -> anyhow::Result<UpdateStatus<UpdateMeta, UpdateResult, String>> {
let (_, update_store) = self.indexes.get_or_create_index(&index, self.update_db_size, self.index_db_size)?; let (_, update_store) = if create {
self.indexes.get_or_create_index(&index, self.update_db_size, self.index_db_size)?
} else {
self.indexes.index(&index)?.ok_or_else(|| anyhow!("Index {:?} doesn't exist", index.as_ref()))?
};
let meta = UpdateMeta::Settings(settings); let meta = UpdateMeta::Settings(settings);
let pending = update_store.register_update(meta, &[])?; let pending = update_store.register_update(meta, &[])?;
Ok(pending.into()) Ok(pending.into())

View File

@ -142,8 +142,9 @@ pub trait IndexController {
fn delete_documents(&self, index: impl AsRef<str>, document_ids: Vec<String>) -> anyhow::Result<UpdateStatus>; fn delete_documents(&self, index: impl AsRef<str>, document_ids: Vec<String>) -> anyhow::Result<UpdateStatus>;
/// Updates an index settings. If the index does not exist, it will be created when the update /// Updates an index settings. If the index does not exist, it will be created when the update
/// is applied to the index. /// is applied to the index. `create` specifies whether an index should be created if not
fn update_settings<S: AsRef<str>>(&self, index_uid: S, settings: Settings) -> anyhow::Result<UpdateStatus>; /// existing.
fn update_settings<S: AsRef<str>>(&self, index_uid: S, settings: Settings, create: bool) -> anyhow::Result<UpdateStatus>;
/// Create an index with the given `index_uid`. /// Create an index with the given `index_uid`.
fn create_index(&self, index_settings: IndexSettings) -> Result<IndexMetadata>; fn create_index(&self, index_settings: IndexSettings) -> Result<IndexMetadata>;

View File

@ -26,7 +26,7 @@ macro_rules! make_setting_route {
$attr: Some(None), $attr: Some(None),
..Default::default() ..Default::default()
}; };
match data.update_settings(index_uid.into_inner(), settings).await { match data.update_settings(index_uid.into_inner(), settings, false).await {
Ok(update_status) => { Ok(update_status) => {
let json = serde_json::to_string(&update_status).unwrap(); let json = serde_json::to_string(&update_status).unwrap();
Ok(HttpResponse::Ok().body(json)) Ok(HttpResponse::Ok().body(json))
@ -48,7 +48,7 @@ macro_rules! make_setting_route {
..Default::default() ..Default::default()
}; };
match data.update_settings(index_uid.into_inner(), settings).await { match data.update_settings(index_uid.into_inner(), settings, true).await {
Ok(update_status) => { Ok(update_status) => {
let json = serde_json::to_string(&update_status).unwrap(); let json = serde_json::to_string(&update_status).unwrap();
Ok(HttpResponse::Ok().body(json)) Ok(HttpResponse::Ok().body(json))
@ -137,7 +137,7 @@ async fn update_all(
index_uid: web::Path<String>, index_uid: web::Path<String>,
body: web::Json<Settings>, body: web::Json<Settings>,
) -> Result<HttpResponse, ResponseError> { ) -> Result<HttpResponse, ResponseError> {
match data.update_settings(index_uid.into_inner(), body.into_inner()).await { match data.update_settings(index_uid.into_inner(), body.into_inner(), true).await {
Ok(update_result) => { Ok(update_result) => {
let json = serde_json::to_string(&update_result).unwrap(); let json = serde_json::to_string(&update_result).unwrap();
Ok(HttpResponse::Ok().body(json)) Ok(HttpResponse::Ok().body(json))
@ -170,7 +170,7 @@ async fn delete_all(
index_uid: web::Path<String>, index_uid: web::Path<String>,
) -> Result<HttpResponse, ResponseError> { ) -> Result<HttpResponse, ResponseError> {
let settings = Settings::cleared(); let settings = Settings::cleared();
match data.update_settings(index_uid.into_inner(), settings).await { match data.update_settings(index_uid.into_inner(), settings, false).await {
Ok(update_result) => { Ok(update_result) => {
let json = serde_json::to_string(&update_result).unwrap(); let json = serde_json::to_string(&update_result).unwrap();
Ok(HttpResponse::Ok().body(json)) Ok(HttpResponse::Ok().body(json))

View File

@ -51,8 +51,6 @@ async fn test_partial_update() {
} }
#[actix_rt::test] #[actix_rt::test]
#[ignore]
// need fix #54
async fn delete_settings_unexisting_index() { async fn delete_settings_unexisting_index() {
let server = Server::new().await; let server = Server::new().await;
let index = server.index("test"); let index = server.index("test");
@ -132,7 +130,6 @@ macro_rules! test_setting_routes {
} }
#[actix_rt::test] #[actix_rt::test]
#[ignore]
async fn delete_unexisting_index() { async fn delete_unexisting_index() {
let server = Server::new().await; let server = Server::new().await;
let url = format!("/indexes/test/settings/{}", let url = format!("/indexes/test/settings/{}",