From 86c3482cbd008a58d549a2efff2bc5842058b192 Mon Sep 17 00:00:00 2001 From: qdequele Date: Thu, 5 Mar 2020 18:29:10 +0100 Subject: [PATCH] review the internal schema to allow to create schema without identifier; fix #513 --- meilisearch-core/src/database.rs | 2 + .../src/update/documents_addition.rs | 4 +- .../src/update/documents_deletion.rs | 2 +- meilisearch-http/src/routes/document.rs | 17 +- meilisearch-http/src/routes/index.rs | 49 ++-- meilisearch-http/src/routes/setting.rs | 12 +- meilisearch-http/tests/common.rs | 2 +- meilisearch-http/tests/index.rs | 244 ++++++++++++------ meilisearch-http/tests/settings.rs | 70 +++++ meilisearch-schema/src/error.rs | 2 + meilisearch-schema/src/schema.rs | 37 ++- 11 files changed, 312 insertions(+), 129 deletions(-) diff --git a/meilisearch-core/src/database.rs b/meilisearch-core/src/database.rs index 650cadd5b..31fc28654 100644 --- a/meilisearch-core/src/database.rs +++ b/meilisearch-core/src/database.rs @@ -8,6 +8,7 @@ use crossbeam_channel::{Receiver, Sender}; use heed::types::{Str, Unit}; use heed::{CompactionOption, Result as ZResult}; use log::debug; +use meilisearch_schema::Schema; use crate::{store, update, Index, MResult}; @@ -242,6 +243,7 @@ impl Database { index.main.put_name(&mut writer, name)?; index.main.put_created_at(&mut writer)?; index.main.put_updated_at(&mut writer)?; + index.main.put_schema(&mut writer, &Schema::new())?; let env_clone = self.env.clone(); let update_env_clone = self.update_env.clone(); diff --git a/meilisearch-core/src/update/documents_addition.rs b/meilisearch-core/src/update/documents_addition.rs index 77275b552..c85836694 100644 --- a/meilisearch-core/src/update/documents_addition.rs +++ b/meilisearch-core/src/update/documents_addition.rs @@ -115,7 +115,7 @@ pub fn apply_documents_addition<'a, 'b>( None => return Err(Error::SchemaMissing), }; - let identifier = schema.identifier(); + let identifier = schema.identifier().ok_or(Error::MissingIdentifier)?; // 1. store documents ids for future deletion for document in addition { @@ -184,7 +184,7 @@ pub fn apply_documents_partial_addition<'a, 'b>( None => return Err(Error::SchemaMissing), }; - let identifier = schema.identifier(); + let identifier = schema.identifier().ok_or(Error::MissingIdentifier)?; // 1. store documents ids for future deletion for mut document in addition { diff --git a/meilisearch-core/src/update/documents_deletion.rs b/meilisearch-core/src/update/documents_deletion.rs index 314632db3..4ae211591 100644 --- a/meilisearch-core/src/update/documents_deletion.rs +++ b/meilisearch-core/src/update/documents_deletion.rs @@ -40,7 +40,7 @@ impl DocumentsDeletion { where D: serde::Serialize, { - let identifier = schema.identifier(); + let identifier = schema.identifier().ok_or(Error::MissingIdentifier)?; let document_id = match extract_document_id(&identifier, &document)? { Some(id) => id, None => return Err(Error::MissingDocumentId), diff --git a/meilisearch-http/src/routes/document.rs b/meilisearch-http/src/routes/document.rs index c1745d78f..df5421dfd 100644 --- a/meilisearch-http/src/routes/document.rs +++ b/meilisearch-http/src/routes/document.rs @@ -1,7 +1,6 @@ use std::collections::{BTreeSet, HashSet}; use indexmap::IndexMap; -use meilisearch_schema::Schema; use serde::{Deserialize, Serialize}; use serde_json::Value; use tide::{Request, Response}; @@ -134,10 +133,11 @@ async fn update_multiple_documents(mut ctx: Request, is_partial: bool) -> let query: UpdateDocumentsQuery = ctx.query().unwrap_or_default(); let db = &ctx.state().db; - let reader = db.main_read_txn()?; - let current_schema = index.main.schema(&reader)?; - if current_schema.is_none() { + let reader = db.main_read_txn()?; + let mut schema = index.main.schema(&reader)?.ok_or(ResponseError::internal("schema not found"))?; + + if schema.identifier().is_none() { let id = match query.identifier { Some(id) => id, None => match data.first().and_then(|docs| find_identifier(docs)) { @@ -145,9 +145,12 @@ async fn update_multiple_documents(mut ctx: Request, is_partial: bool) -> None => return Err(ResponseError::bad_request("Could not infer a schema")), }, }; - let mut writer = db.main_write_txn()?; - index.main.put_schema(&mut writer, &Schema::with_identifier(&id))?; - writer.commit()?; + + if schema.set_identifier(&id).is_ok() { + let mut writer = db.main_write_txn()?; + index.main.put_schema(&mut writer, &schema)?; + writer.commit()?; + } } let mut document_addition = if is_partial { diff --git a/meilisearch-http/src/routes/index.rs b/meilisearch-http/src/routes/index.rs index b8adbb531..924d22389 100644 --- a/meilisearch-http/src/routes/index.rs +++ b/meilisearch-http/src/routes/index.rs @@ -1,7 +1,6 @@ use chrono::{DateTime, Utc}; use log::error; use meilisearch_core::ProcessedUpdateResult; -use meilisearch_schema::Schema; use rand::seq::SliceRandom; use serde::{Deserialize, Serialize}; use serde_json::json; @@ -41,7 +40,10 @@ pub async fn list_indexes(ctx: Request) -> SResult { let updated_at = index.main.updated_at(&reader)?.into_internal_error()?; let identifier = match index.main.schema(&reader) { - Ok(Some(schema)) => Some(schema.identifier().to_owned()), + Ok(Some(schema)) => match schema.identifier() { + Some(identifier) => Some(identifier.to_owned()), + None => None + }, _ => None, }; @@ -88,7 +90,10 @@ pub async fn get_index(ctx: Request) -> SResult { let updated_at = index.main.updated_at(&reader)?.into_internal_error()?; let identifier = match index.main.schema(&reader) { - Ok(Some(schema)) => Some(schema.identifier().to_owned()), + Ok(Some(schema)) => match schema.identifier() { + Some(identifier) => Some(identifier.to_owned()), + None => None + }, _ => None, }; @@ -171,9 +176,11 @@ pub async fn create_index(mut ctx: Request) -> SResult { .into_internal_error()?; if let Some(id) = body.identifier.clone() { - created_index - .main - .put_schema(&mut writer, &Schema::with_identifier(&id))?; + if let Some(mut schema) = created_index.main.schema(&mut writer)? { + if let Ok(_) = schema.set_identifier(&id) { + created_index.main.put_schema(&mut writer, &schema)?; + } + } } writer.commit()?; @@ -224,15 +231,19 @@ pub async fn update_index(mut ctx: Request) -> SResult { index.main.put_name(&mut writer, &name)?; } - if let Some(identifier) = body.identifier { - if let Ok(Some(_)) = index.main.schema(&writer) { - return Err(ResponseError::bad_request( - "The index identifier cannot be updated", - )); + if let Some(id) = body.identifier.clone() { + if let Some(mut schema) = index.main.schema(&mut writer)? { + match schema.identifier() { + Some(_) => { + return Err(ResponseError::bad_request("The index identifier cannot be updated")); + }, + None => { + if let Ok(_) = schema.set_identifier(&id) { + index.main.put_schema(&mut writer, &schema)?; + } + } + } } - index - .main - .put_schema(&mut writer, &Schema::with_identifier(&identifier))?; } index.main.put_updated_at(&mut writer)?; @@ -244,7 +255,15 @@ pub async fn update_index(mut ctx: Request) -> SResult { let updated_at = index.main.updated_at(&reader)?.into_internal_error()?; let identifier = match index.main.schema(&reader) { - Ok(Some(schema)) => Some(schema.identifier().to_owned()), + Ok(Some(schema)) => { + match schema.identifier() { + Some(identifier) => { + Some(identifier.to_owned()) + }, + None => None + } + + }, _ => None, }; diff --git a/meilisearch-http/src/routes/setting.rs b/meilisearch-http/src/routes/setting.rs index ecb97f233..2aad834e4 100644 --- a/meilisearch-http/src/routes/setting.rs +++ b/meilisearch-http/src/routes/setting.rs @@ -50,11 +50,7 @@ pub async fn get_all(ctx: Request) -> SResult { .iter() .map(|s| (*s).to_string()) .collect::>(); - if attrs.is_empty() { - None - } else { - Some(attrs) - } + Some(attrs) }); let displayed_attributes = schema.clone().map(|s| { @@ -63,11 +59,7 @@ pub async fn get_all(ctx: Request) -> SResult { .iter() .map(|s| (*s).to_string()) .collect::>(); - if attrs.is_empty() { - None - } else { - Some(attrs) - } + Some(attrs) }); let accept_new_fields = schema.map(|s| s.accept_new_fields()); diff --git a/meilisearch-http/tests/common.rs b/meilisearch-http/tests/common.rs index 95e59a1d3..a76af5bba 100644 --- a/meilisearch-http/tests/common.rs +++ b/meilisearch-http/tests/common.rs @@ -57,7 +57,7 @@ impl Server { block_on(res.into_body().read_to_end(&mut buf)).unwrap(); let response: Value = serde_json::from_slice(&buf).unwrap(); - if response["status"] == "processed" { + if response["status"] == "processed" || response["status"] == "error" { eprintln!("{:#?}", response); return; } diff --git a/meilisearch-http/tests/index.rs b/meilisearch-http/tests/index.rs index 4bb173e5e..e376900d1 100644 --- a/meilisearch-http/tests/index.rs +++ b/meilisearch-http/tests/index.rs @@ -8,31 +8,28 @@ fn create_index_with_name() { let mut server = common::Server::with_uid("movies"); // 1 - Create a new index - // Index with only a name "movies" - // POST: /indexes let body = json!({ "name": "movies", }); let (res1_value, status_code) = server.create_index(body); + assert_eq!(status_code, 201); assert_eq!(res1_value.as_object().unwrap().len(), 5); let r1_name = res1_value["name"].as_str().unwrap(); let r1_uid = res1_value["uid"].as_str().unwrap(); let r1_created_at = res1_value["createdAt"].as_str().unwrap(); let r1_updated_at = res1_value["updatedAt"].as_str().unwrap(); - assert_eq!(r1_name, "movies"); assert_eq!(r1_uid.len(), 8); assert!(r1_created_at.len() > 1); assert!(r1_updated_at.len() > 1); // 2 - Check the list of indexes - // Must have 1 index with the exact same content that the request 1 - // GET: /indexes let (res2_value, status_code) = server.list_indexes(); + assert_eq!(status_code, 200); assert_eq!(res2_value.as_array().unwrap().len(), 1); assert_eq!(res2_value[0].as_object().unwrap().len(), 5); @@ -40,7 +37,6 @@ fn create_index_with_name() { let r2_uid = res2_value[0]["uid"].as_str().unwrap(); let r2_created_at = res2_value[0]["createdAt"].as_str().unwrap(); let r2_updated_at = res2_value[0]["updatedAt"].as_str().unwrap(); - assert_eq!(r2_name, r1_name); assert_eq!(r2_uid.len(), r1_uid.len()); assert_eq!(r2_created_at.len(), r1_created_at.len()); @@ -52,31 +48,28 @@ fn create_index_with_uid() { let mut server = common::Server::with_uid("movies"); // 1 - Create a new index - // Index with only an uid "movies" - // POST: /indexes let body = json!({ "uid": "movies", }); let (res1_value, status_code) = server.create_index(body); + assert_eq!(status_code, 201); assert_eq!(res1_value.as_object().unwrap().len(), 5); let r1_name = res1_value["name"].as_str().unwrap(); let r1_uid = res1_value["uid"].as_str().unwrap(); let r1_created_at = res1_value["createdAt"].as_str().unwrap(); let r1_updated_at = res1_value["updatedAt"].as_str().unwrap(); - assert_eq!(r1_name, "movies"); assert_eq!(r1_uid, "movies"); assert!(r1_created_at.len() > 1); assert!(r1_updated_at.len() > 1); // 2 - Check the list of indexes - // Must have 1 index with the exact same content that the request 1 - // GET: /indexes let (res2_value, status_code) = server.list_indexes(); + assert_eq!(status_code, 200); assert_eq!(res2_value.as_array().unwrap().len(), 1); assert_eq!(res2_value[0].as_object().unwrap().len(), 5); @@ -84,7 +77,6 @@ fn create_index_with_uid() { let r2_uid = res2_value[0]["uid"].as_str().unwrap(); let r2_created_at = res2_value[0]["createdAt"].as_str().unwrap(); let r2_updated_at = res2_value[0]["updatedAt"].as_str().unwrap(); - assert_eq!(r2_name, r1_name); assert_eq!(r2_uid, r1_uid); assert_eq!(r2_created_at.len(), r1_created_at.len()); @@ -96,41 +88,35 @@ fn create_index_with_name_and_uid() { let mut server = common::Server::with_uid("movies"); // 1 - Create a new index - // Index with a name "Films" and an uid "fn_movies" - // POST: /indexes let body = json!({ "name": "Films", "uid": "fr_movies", }); let (res1_value, status_code) = server.create_index(body); - assert_eq!(status_code, 201); + assert_eq!(status_code, 201); assert_eq!(res1_value.as_object().unwrap().len(), 5); let r1_name = res1_value["name"].as_str().unwrap(); let r1_uid = res1_value["uid"].as_str().unwrap(); let r1_created_at = res1_value["createdAt"].as_str().unwrap(); let r1_updated_at = res1_value["updatedAt"].as_str().unwrap(); - assert_eq!(r1_name, "Films"); assert_eq!(r1_uid, "fr_movies"); assert!(r1_created_at.len() > 1); assert!(r1_updated_at.len() > 1); // 2 - Check the list of indexes - // Must have 1 index with the exact same content that the request 1 - // GET: /indexes let (res2_value, status_code) = server.list_indexes(); - assert_eq!(status_code, 200); + assert_eq!(status_code, 200); assert_eq!(res2_value.as_array().unwrap().len(), 1); assert_eq!(res2_value[0].as_object().unwrap().len(), 5); let r2_name = res2_value[0]["name"].as_str().unwrap(); let r2_uid = res2_value[0]["uid"].as_str().unwrap(); let r2_created_at = res2_value[0]["createdAt"].as_str().unwrap(); let r2_updated_at = res2_value[0]["updatedAt"].as_str().unwrap(); - assert_eq!(r2_name, r1_name); assert_eq!(r2_uid, r1_uid); assert_eq!(r2_created_at.len(), r1_created_at.len()); @@ -140,9 +126,8 @@ fn create_index_with_name_and_uid() { #[test] fn rename_index() { let mut server = common::Server::with_uid("movies"); + // 1 - Create a new index - // Index with only a name "movies" - // POST: /indexes let body = json!({ "name": "movies", @@ -150,55 +135,48 @@ fn rename_index() { }); let (res1_value, status_code) = server.create_index(body); - assert_eq!(status_code, 201); + assert_eq!(status_code, 201); assert_eq!(res1_value.as_object().unwrap().len(), 5); let r1_name = res1_value["name"].as_str().unwrap(); let r1_uid = res1_value["uid"].as_str().unwrap(); let r1_created_at = res1_value["createdAt"].as_str().unwrap(); let r1_updated_at = res1_value["updatedAt"].as_str().unwrap(); - assert_eq!(r1_name, "movies"); assert_eq!(r1_uid.len(), 6); assert!(r1_created_at.len() > 1); assert!(r1_updated_at.len() > 1); // 2 - Update an index name - // Update "movies" to "TV Shows" - // PUT: /indexes/:uid let body = json!({ "name": "TV Shows", }); let (res2_value, status_code) = server.update_index(body); - assert_eq!(status_code, 200); + assert_eq!(status_code, 200); assert_eq!(res2_value.as_object().unwrap().len(), 5); let r2_name = res2_value["name"].as_str().unwrap(); let r2_uid = res2_value["uid"].as_str().unwrap(); let r2_created_at = res2_value["createdAt"].as_str().unwrap(); let r2_updated_at = res2_value["updatedAt"].as_str().unwrap(); - assert_eq!(r2_name, "TV Shows"); assert_eq!(r2_uid, r1_uid); assert_eq!(r2_created_at, r1_created_at); assert!(r2_updated_at.len() > 1); // 3 - Check the list of indexes - // Must have 1 index with the exact same content that the request 2 - // GET: /indexes let (res3_value, status_code) = server.list_indexes(); - assert_eq!(status_code, 200); + assert_eq!(status_code, 200); assert_eq!(res3_value.as_array().unwrap().len(), 1); assert_eq!(res3_value[0].as_object().unwrap().len(), 5); let r3_name = res3_value[0]["name"].as_str().unwrap(); let r3_uid = res3_value[0]["uid"].as_str().unwrap(); let r3_created_at = res3_value[0]["createdAt"].as_str().unwrap(); let r3_updated_at = res3_value[0]["updatedAt"].as_str().unwrap(); - assert_eq!(r3_name, r2_name); assert_eq!(r3_uid.len(), r1_uid.len()); assert_eq!(r3_created_at.len(), r1_created_at.len()); @@ -210,8 +188,6 @@ fn delete_index_and_recreate_it() { let mut server = common::Server::with_uid("movies"); // 1 - Create a new index - // Index with only a name "movies" - // POST: /indexes let body = json!({ "name": "movies", @@ -219,90 +195,76 @@ fn delete_index_and_recreate_it() { }); let (res1_value, status_code) = server.create_index(body); - assert_eq!(status_code, 201); + assert_eq!(status_code, 201); assert_eq!(res1_value.as_object().unwrap().len(), 5); let r1_name = res1_value["name"].as_str().unwrap(); let r1_uid = res1_value["uid"].as_str().unwrap(); let r1_created_at = res1_value["createdAt"].as_str().unwrap(); let r1_updated_at = res1_value["updatedAt"].as_str().unwrap(); - assert_eq!(r1_name, "movies"); assert_eq!(r1_uid.len(), 6); assert!(r1_created_at.len() > 1); assert!(r1_updated_at.len() > 1); // 2 - Check the list of indexes - // Must have 1 index with the exact same content that the request 1 - // GET: /indexes let (res2_value, status_code) = server.list_indexes(); - assert_eq!(status_code, 200); + assert_eq!(status_code, 200); assert_eq!(res2_value.as_array().unwrap().len(), 1); assert_eq!(res2_value[0].as_object().unwrap().len(), 5); let r2_name = res2_value[0]["name"].as_str().unwrap(); let r2_uid = res2_value[0]["uid"].as_str().unwrap(); let r2_created_at = res2_value[0]["createdAt"].as_str().unwrap(); let r2_updated_at = res2_value[0]["updatedAt"].as_str().unwrap(); - assert_eq!(r2_name, r1_name); assert_eq!(r2_uid.len(), r1_uid.len()); assert_eq!(r2_created_at.len(), r1_created_at.len()); assert_eq!(r2_updated_at.len(), r1_updated_at.len()); // 3- Delete an index - // Update "movies" to "TV Shows" - // DELETE: /indexes/:uid let (_res2_value, status_code) = server.delete_index(); + assert_eq!(status_code, 204); // 4 - Check the list of indexes - // Must have 0 index - // GET: /indexes let (res2_value, status_code) = server.list_indexes(); - assert_eq!(status_code, 200); + assert_eq!(status_code, 200); assert_eq!(res2_value.as_array().unwrap().len(), 0); // 5 - Create a new index - // Index with only a name "movies" - // POST: /indexes let body = json!({ "name": "movies", }); let (res1_value, status_code) = server.create_index(body); - assert_eq!(status_code, 201); + assert_eq!(status_code, 201); assert_eq!(res1_value.as_object().unwrap().len(), 5); let r1_name = res1_value["name"].as_str().unwrap(); let r1_uid = res1_value["uid"].as_str().unwrap(); let r1_created_at = res1_value["createdAt"].as_str().unwrap(); let r1_updated_at = res1_value["updatedAt"].as_str().unwrap(); - assert_eq!(r1_name, "movies"); assert_eq!(r1_uid.len(), 8); assert!(r1_created_at.len() > 1); assert!(r1_updated_at.len() > 1); // 6 - Check the list of indexes - // Must have 1 index with the exact same content that the request 1 - // GET: /indexes let (res2_value, status_code) = server.list_indexes(); assert_eq!(status_code, 200); - assert_eq!(res2_value.as_array().unwrap().len(), 1); assert_eq!(res2_value[0].as_object().unwrap().len(), 5); let r2_name = res2_value[0]["name"].as_str().unwrap(); let r2_uid = res2_value[0]["uid"].as_str().unwrap(); let r2_created_at = res2_value[0]["createdAt"].as_str().unwrap(); let r2_updated_at = res2_value[0]["updatedAt"].as_str().unwrap(); - assert_eq!(r2_name, r1_name); assert_eq!(r2_uid.len(), r1_uid.len()); assert_eq!(r2_created_at.len(), r1_created_at.len()); @@ -314,89 +276,75 @@ fn check_multiples_indexes() { let mut server = common::Server::with_uid("movies"); // 1 - Create a new index - // Index with only a name "movies" - // POST: /indexes let body = json!({ "name": "movies", }); let (res1_value, status_code) = server.create_index(body); - assert_eq!(status_code, 201); + assert_eq!(status_code, 201); assert_eq!(res1_value.as_object().unwrap().len(), 5); let r1_name = res1_value["name"].as_str().unwrap(); let r1_uid = res1_value["uid"].as_str().unwrap(); let r1_created_at = res1_value["createdAt"].as_str().unwrap(); let r1_updated_at = res1_value["updatedAt"].as_str().unwrap(); - assert_eq!(r1_name, "movies"); assert_eq!(r1_uid.len(), 8); assert!(r1_created_at.len() > 1); assert!(r1_updated_at.len() > 1); // 2 - Check the list of indexes - // Must have 1 index with the exact same content that the request 1 - // GET: /indexes let (res2_value, status_code) = server.list_indexes(); - assert_eq!(status_code, 200); + assert_eq!(status_code, 200); assert_eq!(res2_value.as_array().unwrap().len(), 1); assert_eq!(res2_value[0].as_object().unwrap().len(), 5); let r2_0_name = res2_value[0]["name"].as_str().unwrap(); let r2_0_uid = res2_value[0]["uid"].as_str().unwrap(); let r2_0_created_at = res2_value[0]["createdAt"].as_str().unwrap(); let r2_0_updated_at = res2_value[0]["updatedAt"].as_str().unwrap(); - assert_eq!(r2_0_name, r1_name); assert_eq!(r2_0_uid.len(), r1_uid.len()); assert_eq!(r2_0_created_at.len(), r1_created_at.len()); assert_eq!(r2_0_updated_at.len(), r1_updated_at.len()); // 3 - Create a new index - // Index with only a name "films" - // POST: /indexes let body = json!({ "name": "films", }); let (res3_value, status_code) = server.create_index(body); - assert_eq!(status_code, 201); + assert_eq!(status_code, 201); assert_eq!(res3_value.as_object().unwrap().len(), 5); let r3_name = res3_value["name"].as_str().unwrap(); let r3_uid = res3_value["uid"].as_str().unwrap(); let r3_created_at = res3_value["createdAt"].as_str().unwrap(); let r3_updated_at = res3_value["updatedAt"].as_str().unwrap(); - assert_eq!(r3_name, "films"); assert_eq!(r3_uid.len(), 8); assert!(r3_created_at.len() > 1); assert!(r3_updated_at.len() > 1); // 4 - Check the list of indexes - // Must have 2 index with the exact same content that the request 1 and 3 - // GET: /indexes let (res4_value, status_code) = server.list_indexes(); + assert_eq!(status_code, 200); - assert_eq!(res4_value.as_array().unwrap().len(), 2); - assert_eq!(res4_value[0].as_object().unwrap().len(), 5); let r4_0_name = res4_value[0]["name"].as_str().unwrap(); let r4_0_uid = res4_value[0]["uid"].as_str().unwrap(); let r4_0_created_at = res4_value[0]["createdAt"].as_str().unwrap(); let r4_0_updated_at = res4_value[0]["updatedAt"].as_str().unwrap(); - assert_eq!(res4_value[1].as_object().unwrap().len(), 5); let r4_1_name = res4_value[1]["name"].as_str().unwrap(); let r4_1_uid = res4_value[1]["uid"].as_str().unwrap(); let r4_1_created_at = res4_value[1]["createdAt"].as_str().unwrap(); let r4_1_updated_at = res4_value[1]["updatedAt"].as_str().unwrap(); - if r4_0_name == r1_name { assert_eq!(r4_0_name, r1_name); assert_eq!(r4_0_uid.len(), r1_uid.len()); @@ -408,7 +356,6 @@ fn check_multiples_indexes() { assert_eq!(r4_0_created_at.len(), r3_created_at.len()); assert_eq!(r4_0_updated_at.len(), r3_updated_at.len()); } - if r4_1_name == r1_name { assert_eq!(r4_1_name, r1_name); assert_eq!(r4_1_uid.len(), r1_uid.len()); @@ -427,19 +374,17 @@ fn create_index_failed() { let mut server = common::Server::with_uid("movies"); // 2 - Push index creation with empty json body - // POST: /indexes let body = json!({}); let (res_value, status_code) = server.create_index(body); - assert_eq!(status_code, 400); + assert_eq!(status_code, 400); let message = res_value["message"].as_str().unwrap(); assert_eq!(res_value.as_object().unwrap().len(), 1); assert_eq!(message, "Index creation must have an uid"); // 3 - Create a index with extra data - // POST: /indexes let body = json!({ "name": "movies", @@ -447,14 +392,13 @@ fn create_index_failed() { }); let (res_value, status_code) = server.create_index(body); - assert_eq!(status_code, 400); + assert_eq!(status_code, 400); let message = res_value["message"].as_str().unwrap(); assert_eq!(res_value.as_object().unwrap().len(), 1); assert_eq!(message, "invalid data"); // 3 - Create a index with wrong data type - // POST: /indexes let body = json!({ "name": "movies", @@ -462,8 +406,8 @@ fn create_index_failed() { }); let (res_value, status_code) = server.create_index(body); - assert_eq!(status_code, 400); + assert_eq!(status_code, 400); let message = res_value["message"].as_str().unwrap(); assert_eq!(res_value.as_object().unwrap().len(), 1); assert_eq!(message, "invalid data"); @@ -476,6 +420,8 @@ fn create_index_failed() { fn create_index_with_identifier_and_index() { let mut server = common::Server::with_uid("movies"); + // 1 - Create the index + let body = json!({ "uid": "movies", "identifier": "id", @@ -484,6 +430,8 @@ fn create_index_with_identifier_and_index() { let (_response, status_code) = server.create_index(body); assert_eq!(status_code, 201); + // 2 - Add content + let body = json!([{ "id": 123, "text": "The mask" @@ -491,6 +439,8 @@ fn create_index_with_identifier_and_index() { server.add_or_replace_multiple_documents(body.clone()); + // 3 - Retreive document + let (response, _status_code) = server.get_document(123); let expect = json!({ @@ -502,51 +452,181 @@ fn create_index_with_identifier_and_index() { } // Resolve issue https://github.com/meilisearch/MeiliSearch/issues/497 +// Test when the given index uid is not valid +// Should have a 400 status code +// Should have the right error message #[test] fn create_index_with_invalid_uid() { let mut server = common::Server::with_uid(""); + // 1 - Create the index with invalid uid + let body = json!({ "uid": "the movies" }); let (response, status_code) = server.create_index(body); - assert_eq!(status_code, 400); + assert_eq!(status_code, 400); let message = response["message"].as_str().unwrap(); assert_eq!(response.as_object().unwrap().len(), 1); assert_eq!(message, "Index must have a valid uid; Index uid can be of type integer or string only composed of alphanumeric characters, hyphens (-) and underscores (_)."); + // 2 - Create the index with invalid uid + let body = json!({ "uid": "%$#" }); let (response, status_code) = server.create_index(body); - assert_eq!(status_code, 400); + assert_eq!(status_code, 400); let message = response["message"].as_str().unwrap(); assert_eq!(response.as_object().unwrap().len(), 1); assert_eq!(message, "Index must have a valid uid; Index uid can be of type integer or string only composed of alphanumeric characters, hyphens (-) and underscores (_)."); + // 3 - Create the index with invalid uid + let body = json!({ "uid": "the~movies" }); let (response, status_code) = server.create_index(body); - assert_eq!(status_code, 400); + assert_eq!(status_code, 400); let message = response["message"].as_str().unwrap(); assert_eq!(response.as_object().unwrap().len(), 1); assert_eq!(message, "Index must have a valid uid; Index uid can be of type integer or string only composed of alphanumeric characters, hyphens (-) and underscores (_)."); + // 4 - Create the index with invalid uid + let body = json!({ "uid": "🎉" }); let (response, status_code) = server.create_index(body); - assert_eq!(status_code, 400); + assert_eq!(status_code, 400); let message = response["message"].as_str().unwrap(); assert_eq!(response.as_object().unwrap().len(), 1); assert_eq!(message, "Index must have a valid uid; Index uid can be of type integer or string only composed of alphanumeric characters, hyphens (-) and underscores (_)."); } + +// Test that it's possible to add identifier if it's not already set on index creation +#[test] +fn create_index_and_add_indentifier_after() { + let mut server = common::Server::with_uid("movies"); + + // 1 - Create the index with no identifier + + let body = json!({ + "uid": "movies", + }); + let (response, status_code) = server.create_index(body); + assert_eq!(status_code, 201); + assert_eq!(response["identifier"], json!(null)); + + // 2 - Update the index and add an identifier. + + let body = json!({ + "identifier": "id", + }); + + let (response, status_code) = server.update_index(body); + assert_eq!(status_code, 200); + eprintln!("response: {:#?}", response); + assert_eq!(response["identifier"].as_str().unwrap(), "id"); + + // 3 - Get index to verify if the identifier is good + + let (response, status_code) = server.get_index(); + assert_eq!(status_code, 200); + assert_eq!(response["identifier"].as_str().unwrap(), "id"); + +} + +// Test that it's impossible to change the identifier +#[test] +fn create_index_and_update_indentifier_after() { + let mut server = common::Server::with_uid("movies"); + + // 1 - Create the index with no identifier + + let body = json!({ + "uid": "movies", + "identifier": "id", + }); + let (response, status_code) = server.create_index(body); + assert_eq!(status_code, 201); + assert_eq!(response["identifier"].as_str().unwrap(), "id"); + + // 2 - Update the index and add an identifier. + + let body = json!({ + "identifier": "skuid", + }); + + let (_response, status_code) = server.update_index(body); + assert_eq!(status_code, 400); + + // 3 - Get index to verify if the identifier still the first one + + let (response, status_code) = server.get_index(); + assert_eq!(status_code, 200); + assert_eq!(response["identifier"].as_str().unwrap(), "id"); +} + + +// Test that schema inference work well +#[test] +fn create_index_without_identifier_and_add_document() { + let mut server = common::Server::with_uid("movies"); + + // 1 - Create the index with no identifier + + let body = json!({ + "uid": "movies", + }); + let (response, status_code) = server.create_index(body); + assert_eq!(status_code, 201); + assert_eq!(response["identifier"], json!(null)); + + // 2 - Add a document + + let body = json!([{ + "id": 123, + "title": "I'm a legend", + }]); + + server.add_or_update_multiple_documents(body); + + // 3 - Get index to verify if the identifier is good + + let (response, status_code) = server.get_index(); + assert_eq!(status_code, 200); + assert_eq!(response["identifier"].as_str().unwrap(), "id"); +} + + +// Test search with no identifier +#[test] +fn create_index_without_identifier_and_search() { + let mut server = common::Server::with_uid("movies"); + + // 1 - Create the index with no identifier + + let body = json!({ + "uid": "movies", + }); + let (response, status_code) = server.create_index(body); + assert_eq!(status_code, 201); + assert_eq!(response["identifier"], json!(null)); + + // 2 - Search + + let query = "q=captain&limit=3"; + + let (response, status_code) = server.search(&query); + assert_eq!(status_code, 200); + assert_eq!(response["hits"].as_array().unwrap().len(), 0); +} diff --git a/meilisearch-http/tests/settings.rs b/meilisearch-http/tests/settings.rs index 2f9c7c43c..fc8af7c89 100644 --- a/meilisearch-http/tests/settings.rs +++ b/meilisearch-http/tests/settings.rs @@ -252,3 +252,73 @@ fn write_all_and_update() { assert_json_eq!(expected, response, ordered: false); } + + +#[test] +fn test_default_settings() { + let mut server = common::Server::with_uid("movies"); + let body = json!({ + "uid": "movies", + }); + server.create_index(body); + + // 1 - Get all settings and compare to the previous one + + let body = json!({ + "rankingRules": [ + "typo", + "words", + "proximity", + "attribute", + "wordsPosition", + "exactness" + ], + "distinctAttribute": null, + "searchableAttributes": [], + "displayedAttributes": [], + "stopWords": [], + "synonyms": {}, + "acceptNewFields": true, + }); + + let (response, _status_code) = server.get_all_settings(); + + assert_json_eq!(body, response, ordered: false); +} + +#[test] +fn test_default_settings_2() { + let mut server = common::Server::with_uid("movies"); + let body = json!({ + "uid": "movies", + "identifier": "id", + }); + server.create_index(body); + + // 1 - Get all settings and compare to the previous one + + let body = json!({ + "rankingRules": [ + "typo", + "words", + "proximity", + "attribute", + "wordsPosition", + "exactness" + ], + "distinctAttribute": null, + "searchableAttributes": [ + "id" + ], + "displayedAttributes": [ + "id" + ], + "stopWords": [], + "synonyms": {}, + "acceptNewFields": true, + }); + + let (response, _status_code) = server.get_all_settings(); + + assert_json_eq!(body, response, ordered: false); +} diff --git a/meilisearch-schema/src/error.rs b/meilisearch-schema/src/error.rs index 29c4dd035..839ad1816 100644 --- a/meilisearch-schema/src/error.rs +++ b/meilisearch-schema/src/error.rs @@ -6,6 +6,7 @@ pub type SResult = Result; #[derive(Debug)] pub enum Error { FieldNameNotFound(String), + IdentifierAlreadyPresent, MaxFieldsLimitExceeded, } @@ -14,6 +15,7 @@ impl fmt::Display for Error { use self::Error::*; match self { FieldNameNotFound(field) => write!(f, "The field {:?} doesn't exist", field), + IdentifierAlreadyPresent => write!(f, "The schema already have an identifier. It's impossible to update it"), MaxFieldsLimitExceeded => write!(f, "The maximum of possible reattributed field id has been reached"), } } diff --git a/meilisearch-schema/src/schema.rs b/meilisearch-schema/src/schema.rs index 2f9eee4f5..79e2d3fad 100644 --- a/meilisearch-schema/src/schema.rs +++ b/meilisearch-schema/src/schema.rs @@ -6,7 +6,7 @@ use std::collections::{HashMap, HashSet}; pub struct Schema { fields_map: FieldsMap, - identifier: FieldId, + identifier: Option, ranked: HashSet, displayed: HashSet, @@ -17,6 +17,18 @@ pub struct Schema { } impl Schema { + pub fn new() -> Schema { + Schema { + fields_map: FieldsMap::default(), + identifier: None, + ranked: HashSet::new(), + displayed: HashSet::new(), + indexed: Vec::new(), + indexed_map: HashMap::new(), + accept_new_fields: true, + } + } + pub fn with_identifier(name: &str) -> Schema { let mut fields_map = FieldsMap::default(); let field_id = fields_map.insert(name).unwrap(); @@ -31,7 +43,7 @@ impl Schema { Schema { fields_map, - identifier: field_id, + identifier: Some(field_id), ranked: HashSet::new(), displayed, indexed, @@ -40,18 +52,21 @@ impl Schema { } } - pub fn identifier(&self) -> &str { - self.fields_map.name(self.identifier).unwrap() + pub fn identifier(&self) -> Option<&str> { + self.identifier.map(|id| self.fields_map.name(id).unwrap()) } - pub fn set_identifier(&mut self, id: &str) -> SResult<()> { - match self.id(id) { - Some(id) => { - self.identifier = id; - Ok(()) - }, - None => Err(Error::FieldNameNotFound(id.to_string())) + pub fn set_identifier(&mut self, name: &str) -> SResult { + if self.identifier.is_some() { + return Err(Error::IdentifierAlreadyPresent) } + + let id = self.insert(name)?; + self.identifier = Some(id); + self.set_indexed(name)?; + self.set_displayed(name)?; + + Ok(id) } pub fn id(&self, name: &str) -> Option {