Remove Checked/Unchecked type parameter from Settings type

This commit is contained in:
Loïc Lecrenier 2022-06-21 15:34:53 +02:00
parent b84ea036dd
commit 4c1f034d9f
13 changed files with 48 additions and 143 deletions

View File

@ -1,7 +1,7 @@
use log::debug; use log::debug;
use actix_web::{web, HttpRequest, HttpResponse}; use actix_web::{web, HttpRequest, HttpResponse};
use meilisearch_lib::index::{Settings, Unchecked}; use meilisearch_lib::index::Settings;
use meilisearch_lib::index_controller::Update; use meilisearch_lib::index_controller::Update;
use meilisearch_lib::MeiliSearch; use meilisearch_lib::MeiliSearch;
use meilisearch_types::error::{MeiliDeserError, ResponseError}; use meilisearch_types::error::{MeiliDeserError, ResponseError};
@ -356,7 +356,7 @@ generate_configure!(
pub async fn update_all( pub async fn update_all(
meilisearch: GuardedData<ActionPolicy<{ actions::SETTINGS_UPDATE }>, MeiliSearch>, meilisearch: GuardedData<ActionPolicy<{ actions::SETTINGS_UPDATE }>, MeiliSearch>,
index_uid: web::Path<String>, index_uid: web::Path<String>,
body: ValidatedJson<Settings<Unchecked>, MeiliDeserError>, body: ValidatedJson<Settings, MeiliDeserError>,
req: HttpRequest, req: HttpRequest,
analytics: web::Data<dyn Analytics>, analytics: web::Data<dyn Analytics>,
) -> Result<HttpResponse, ResponseError> { ) -> Result<HttpResponse, ResponseError> {
@ -442,7 +442,7 @@ pub async fn delete_all(
data: GuardedData<ActionPolicy<{ actions::SETTINGS_UPDATE }>, MeiliSearch>, data: GuardedData<ActionPolicy<{ actions::SETTINGS_UPDATE }>, MeiliSearch>,
index_uid: web::Path<String>, index_uid: web::Path<String>,
) -> Result<HttpResponse, ResponseError> { ) -> Result<HttpResponse, ResponseError> {
let settings = Settings::cleared().into_unchecked(); let settings = Settings::cleared();
let allow_index_creation = data.filters().allow_index_creation; let allow_index_creation = data.filters().allow_index_creation;
let update = Update::Settings { let update = Update::Settings {

View File

@ -4,7 +4,7 @@ use serde::{Deserialize, Serialize};
use time::OffsetDateTime; use time::OffsetDateTime;
use meilisearch_lib::index::{Settings, Unchecked}; use meilisearch_lib::index::Settings;
use meilisearch_lib::MeiliSearch; use meilisearch_lib::MeiliSearch;
use meilisearch_types::error::ResponseError; use meilisearch_types::error::ResponseError;
use meilisearch_types::star_or::StarOr; use meilisearch_types::star_or::StarOr;
@ -140,7 +140,7 @@ pub enum UpdateType {
number: Option<usize>, number: Option<usize>,
}, },
Settings { Settings {
settings: Settings<Unchecked>, settings: Settings,
}, },
} }

View File

@ -3,7 +3,7 @@ use std::fmt::{self, Write};
use std::str::FromStr; use std::str::FromStr;
use std::write; use std::write;
use meilisearch_lib::index::{Settings, Unchecked}; use meilisearch_lib::index::Settings;
use meilisearch_lib::tasks::batch::BatchId; use meilisearch_lib::tasks::batch::BatchId;
use meilisearch_lib::tasks::task::{ use meilisearch_lib::tasks::task::{
DocumentDeletion, Task, TaskContent, TaskEvent, TaskId, TaskResult, DocumentDeletion, Task, TaskContent, TaskEvent, TaskId, TaskResult,
@ -144,7 +144,7 @@ enum TaskDetails {
#[serde(rename_all = "camelCase")] #[serde(rename_all = "camelCase")]
Settings { Settings {
#[serde(flatten)] #[serde(flatten)]
settings: Settings<Unchecked>, settings: Settings,
}, },
#[serde(rename_all = "camelCase")] #[serde(rename_all = "camelCase")]
IndexInfo { primary_key: Option<String> }, IndexInfo { primary_key: Option<String> },

View File

@ -5,7 +5,7 @@ use serde::{Deserialize, Serialize};
use time::OffsetDateTime; use time::OffsetDateTime;
use uuid::Uuid; use uuid::Uuid;
use crate::index::{Settings, Unchecked}; use crate::index::Settings;
#[derive(Serialize, Deserialize)] #[derive(Serialize, Deserialize)]
pub struct UpdateEntry { pub struct UpdateEntry {
@ -43,7 +43,7 @@ pub enum UpdateMeta {
DeleteDocuments { DeleteDocuments {
ids: Vec<String>, ids: Vec<String>,
}, },
Settings(Settings<Unchecked>), Settings(Settings),
} }
#[derive(Debug, Serialize, Deserialize, Clone)] #[derive(Debug, Serialize, Deserialize, Clone)]

View File

@ -6,7 +6,7 @@ use time::OffsetDateTime;
use uuid::Uuid; use uuid::Uuid;
use super::v4::{Task, TaskContent, TaskEvent}; use super::v4::{Task, TaskContent, TaskEvent};
use crate::index::{Settings, Unchecked}; use crate::index::Settings;
use crate::tasks::task::{DocumentDeletion, TaskId, TaskResult}; use crate::tasks::task::{DocumentDeletion, TaskId, TaskResult};
use super::v2; use super::v2;
@ -55,7 +55,7 @@ pub enum Update {
method: IndexDocumentsMethod, method: IndexDocumentsMethod,
content_uuid: Uuid, content_uuid: Uuid,
}, },
Settings(Settings<Unchecked>), Settings(Settings),
ClearDocuments, ClearDocuments,
} }
@ -100,7 +100,7 @@ pub enum UpdateMeta {
DeleteDocuments { DeleteDocuments {
ids: Vec<String>, ids: Vec<String>,
}, },
Settings(Settings<Unchecked>), Settings(Settings),
} }
#[derive(Debug, Serialize, Deserialize, Clone)] #[derive(Debug, Serialize, Deserialize, Clone)]

View File

@ -5,7 +5,7 @@ use serde::{Deserialize, Serialize};
use time::OffsetDateTime; use time::OffsetDateTime;
use uuid::Uuid; use uuid::Uuid;
use crate::index::{Settings, Unchecked}; use crate::index::Settings;
use crate::tasks::batch::BatchId; use crate::tasks::batch::BatchId;
use crate::tasks::task::{ use crate::tasks::task::{
DocumentDeletion, TaskContent as NewTaskContent, TaskEvent as NewTaskEvent, TaskId, TaskResult, DocumentDeletion, TaskContent as NewTaskContent, TaskEvent as NewTaskEvent, TaskId, TaskResult,
@ -82,7 +82,7 @@ pub enum TaskContent {
}, },
DocumentDeletion(DocumentDeletion), DocumentDeletion(DocumentDeletion),
SettingsUpdate { SettingsUpdate {
settings: Settings<Unchecked>, settings: Settings,
/// Indicates whether the task was a deletion /// Indicates whether the task was a deletion
is_deletion: bool, is_deletion: bool,
allow_index_creation: bool, allow_index_creation: bool,

View File

@ -13,11 +13,11 @@ use crate::document_formats::read_ndjson;
use crate::index::updates::apply_settings_to_builder; use crate::index::updates::apply_settings_to_builder;
use super::error::Result; use super::error::Result;
use super::{index::Index, Settings, Unchecked}; use super::{index::Index, Settings};
#[derive(Serialize, Deserialize)] #[derive(Serialize, Deserialize)]
struct DumpMeta { struct DumpMeta {
settings: Settings<Unchecked>, settings: Settings,
primary_key: Option<String>, primary_key: Option<String>,
} }
@ -69,7 +69,7 @@ impl Index {
let meta_file_path = path.as_ref().join(META_FILE_NAME); let meta_file_path = path.as_ref().join(META_FILE_NAME);
let mut meta_file = File::create(&meta_file_path)?; let mut meta_file = File::create(&meta_file_path)?;
let settings = self.settings_txn(txn)?.into_unchecked(); let settings = self.settings_txn(txn)?;
let primary_key = self.primary_key(txn)?.map(String::from); let primary_key = self.primary_key(txn)?.map(String::from);
let meta = DumpMeta { let meta = DumpMeta {
settings, settings,
@ -101,7 +101,6 @@ impl Index {
settings, settings,
primary_key, primary_key,
} = serde_json::from_reader(meta_file)?; } = serde_json::from_reader(meta_file)?;
let settings = settings.check();
let mut options = EnvOpenOptions::new(); let mut options = EnvOpenOptions::new();
options.map_size(size); options.map_size(size);

View File

@ -1,6 +1,5 @@
use std::collections::BTreeSet; use std::collections::BTreeSet;
use std::fs::create_dir_all; use std::fs::create_dir_all;
use std::marker::PhantomData;
use std::ops::Deref; use std::ops::Deref;
use std::path::Path; use std::path::Path;
use std::sync::Arc; use std::sync::Arc;
@ -20,7 +19,7 @@ use crate::EnvSizer;
use super::error::IndexError; use super::error::IndexError;
use super::error::Result; use super::error::Result;
use super::updates::{FacetingSettings, MinWordSizeTyposSetting, PaginationSettings, TypoSettings}; use super::updates::{FacetingSettings, MinWordSizeTyposSetting, PaginationSettings, TypoSettings};
use super::{Checked, Settings}; use super::Settings;
pub type Document = Map<String, Value>; pub type Document = Map<String, Value>;
@ -121,7 +120,7 @@ impl Index {
pub fn meta(&self) -> Result<IndexMeta> { pub fn meta(&self) -> Result<IndexMeta> {
IndexMeta::new(self) IndexMeta::new(self)
} }
pub fn settings(&self) -> Result<Settings<Checked>> { pub fn settings(&self) -> Result<Settings> {
let txn = self.read_txn()?; let txn = self.read_txn()?;
self.settings_txn(&txn) self.settings_txn(&txn)
} }
@ -130,7 +129,7 @@ impl Index {
self.uuid self.uuid
} }
pub fn settings_txn(&self, txn: &RoTxn) -> Result<Settings<Checked>> { pub fn settings_txn(&self, txn: &RoTxn) -> Result<Settings> {
let displayed_attributes = self let displayed_attributes = self
.displayed_fields(txn)? .displayed_fields(txn)?
.map(|fields| fields.into_iter().map(String::from).collect()); .map(|fields| fields.into_iter().map(String::from).collect());
@ -225,7 +224,6 @@ impl Index {
typo_tolerance: Setting::Set(typo_tolerance), typo_tolerance: Setting::Set(typo_tolerance),
faceting: Setting::Set(faceting), faceting: Setting::Set(faceting),
pagination: Setting::Set(pagination), pagination: Setting::Set(pagination),
_kind: PhantomData,
}) })
} }

View File

@ -2,7 +2,7 @@ pub use search::{
SearchQuery, SearchResult, DEFAULT_CROP_LENGTH, DEFAULT_CROP_MARKER, SearchQuery, SearchResult, DEFAULT_CROP_LENGTH, DEFAULT_CROP_MARKER,
DEFAULT_HIGHLIGHT_POST_TAG, DEFAULT_HIGHLIGHT_PRE_TAG, DEFAULT_SEARCH_LIMIT, DEFAULT_HIGHLIGHT_POST_TAG, DEFAULT_HIGHLIGHT_PRE_TAG, DEFAULT_SEARCH_LIMIT,
}; };
pub use updates::{apply_settings_to_builder, Checked, Facets, Settings, Unchecked}; pub use updates::{apply_settings_to_builder, Facets, Settings};
mod dump; mod dump;
pub mod error; pub mod error;
@ -89,7 +89,7 @@ pub mod test {
MockIndex::Mock(_) => todo!(), MockIndex::Mock(_) => todo!(),
} }
} }
pub fn settings(&self) -> Result<Settings<Checked>> { pub fn settings(&self) -> Result<Settings> {
match self { match self {
MockIndex::Real(index) => index.settings(), MockIndex::Real(index) => index.settings(),
MockIndex::Mock(_) => todo!(), MockIndex::Mock(_) => todo!(),
@ -175,7 +175,7 @@ pub mod test {
} }
} }
pub fn update_settings(&self, settings: &Settings<Checked>) -> Result<()> { pub fn update_settings(&self, settings: &Settings) -> Result<()> {
match self { match self {
MockIndex::Real(index) => index.update_settings(settings), MockIndex::Real(index) => index.update_settings(settings),
MockIndex::Mock(m) => unsafe { m.get("update_settings").call(settings) }, MockIndex::Mock(m) => unsafe { m.get("update_settings").call(settings) },

View File

@ -8,7 +8,6 @@ use milli::update::{
use milli::Criterion; use milli::Criterion;
use serde::{Deserialize, Serialize, Serializer}; use serde::{Deserialize, Serialize, Serializer};
use std::collections::{BTreeMap, BTreeSet}; use std::collections::{BTreeMap, BTreeSet};
use std::marker::PhantomData;
use std::num::NonZeroUsize; use std::num::NonZeroUsize;
use uuid::Uuid; use uuid::Uuid;
@ -32,26 +31,6 @@ where
.serialize(s) .serialize(s)
} }
#[derive(Clone, Default, Debug, Serialize, PartialEq)]
pub struct Checked;
#[derive(Clone, Default, Debug, Serialize, Deserialize, PartialEq)]
pub struct Unchecked;
impl<E> DeserializeFromValue<E> for Unchecked
where
E: jayson::DeserializeError,
{
fn deserialize_from_value<V>(
_value: jayson::Value<V>,
_location: jayson::ValuePointerRef,
) -> std::result::Result<Self, E>
where
V: jayson::IntoValue,
{
Ok(Unchecked)
}
}
#[cfg_attr(test, derive(proptest_derive::Arbitrary))] #[cfg_attr(test, derive(proptest_derive::Arbitrary))]
#[derive(Debug, Clone, Default, Serialize, Deserialize, PartialEq, DeserializeFromValue)] #[derive(Debug, Clone, Default, Serialize, Deserialize, PartialEq, DeserializeFromValue)]
#[serde(deny_unknown_fields)] #[serde(deny_unknown_fields)]
@ -108,25 +87,23 @@ pub struct PaginationSettings {
pub limited_to: Setting<usize>, pub limited_to: Setting<usize>,
} }
/// Holds all the settings for an index. `T` can either be `Checked` if they represents settings /// Holds all the settings for an index. The settings are validated by the implementation of
/// whose validity is guaranteed, or `Unchecked` if they need to be validated. In the later case, a /// jayson::DeserializeFromValue.
/// call to `check` will return a `Settings<Checked>` from a `Settings<Unchecked>`.
#[derive( #[derive(
Debug, Clone, Default, Serialize, Deserialize, PartialEq, jayson::DeserializeFromValue, Debug, Clone, Default, Serialize, Deserialize, PartialEq, jayson::DeserializeFromValue,
)] )]
#[serde(deny_unknown_fields)] #[serde(deny_unknown_fields)]
#[serde(rename_all = "camelCase")] #[serde(rename_all = "camelCase")]
#[serde(bound(serialize = "T: Serialize", deserialize = "T: Deserialize<'static>"))]
#[cfg_attr(test, derive(proptest_derive::Arbitrary))] #[cfg_attr(test, derive(proptest_derive::Arbitrary))]
#[jayson(rename_all = camelCase, deny_unknown_fields)] #[jayson(rename_all = camelCase, deny_unknown_fields)]
pub struct Settings<T> { pub struct Settings {
#[serde( #[serde(
default, default,
serialize_with = "serialize_with_wildcard", serialize_with = "serialize_with_wildcard",
skip_serializing_if = "Setting::is_not_set" skip_serializing_if = "Setting::is_not_set"
)] )]
#[cfg_attr(test, proptest(strategy = "test::setting_strategy()"))] #[cfg_attr(test, proptest(strategy = "test::setting_strategy()"))]
#[jayson(map = map_searchable_or_displayed_attributes)]
pub displayed_attributes: Setting<Vec<String>>, pub displayed_attributes: Setting<Vec<String>>,
#[serde( #[serde(
@ -135,6 +112,7 @@ pub struct Settings<T> {
skip_serializing_if = "Setting::is_not_set" skip_serializing_if = "Setting::is_not_set"
)] )]
#[cfg_attr(test, proptest(strategy = "test::setting_strategy()"))] #[cfg_attr(test, proptest(strategy = "test::setting_strategy()"))]
#[jayson(map = map_searchable_or_displayed_attributes)]
pub searchable_attributes: Setting<Vec<String>>, pub searchable_attributes: Setting<Vec<String>>,
#[serde(default, skip_serializing_if = "Setting::is_not_set")] #[serde(default, skip_serializing_if = "Setting::is_not_set")]
@ -165,13 +143,10 @@ pub struct Settings<T> {
#[serde(default, skip_serializing_if = "Setting::is_not_set")] #[serde(default, skip_serializing_if = "Setting::is_not_set")]
#[cfg_attr(test, proptest(strategy = "test::setting_strategy()"))] #[cfg_attr(test, proptest(strategy = "test::setting_strategy()"))]
pub pagination: Setting<PaginationSettings>, pub pagination: Setting<PaginationSettings>,
#[serde(skip)]
pub _kind: PhantomData<T>,
} }
impl Settings<Checked> { impl Settings {
pub fn cleared() -> Settings<Checked> { pub fn cleared() -> Settings {
Settings { Settings {
displayed_attributes: Setting::Reset, displayed_attributes: Setting::Reset,
searchable_attributes: Setting::Reset, searchable_attributes: Setting::Reset,
@ -184,81 +159,21 @@ impl Settings<Checked> {
typo_tolerance: Setting::Reset, typo_tolerance: Setting::Reset,
faceting: Setting::Reset, faceting: Setting::Reset,
pagination: Setting::Reset, pagination: Setting::Reset,
_kind: PhantomData,
}
}
pub fn into_unchecked(self) -> Settings<Unchecked> {
let Self {
displayed_attributes,
searchable_attributes,
filterable_attributes,
sortable_attributes,
ranking_rules,
stop_words,
synonyms,
distinct_attribute,
typo_tolerance,
faceting,
pagination,
..
} = self;
Settings {
displayed_attributes,
searchable_attributes,
filterable_attributes,
sortable_attributes,
ranking_rules,
stop_words,
synonyms,
distinct_attribute,
typo_tolerance,
faceting,
pagination,
_kind: PhantomData,
} }
} }
} }
pub fn map_searchable_or_displayed_attributes(
impl Settings<Unchecked> { setting: Setting<Vec<String>>,
pub fn check(self) -> Settings<Checked> { ) -> Setting<Vec<String>> {
let displayed_attributes = match self.displayed_attributes { match setting {
Setting::Set(fields) => { Setting::Set(fields) => {
if fields.iter().any(|f| f == "*") { if fields.iter().any(|f| f == "*") {
Setting::Reset Setting::Reset
} else { } else {
Setting::Set(fields) Setting::Set(fields)
}
} }
otherwise => otherwise,
};
let searchable_attributes = match self.searchable_attributes {
Setting::Set(fields) => {
if fields.iter().any(|f| f == "*") {
Setting::Reset
} else {
Setting::Set(fields)
}
}
otherwise => otherwise,
};
Settings {
displayed_attributes,
searchable_attributes,
filterable_attributes: self.filterable_attributes,
sortable_attributes: self.sortable_attributes,
ranking_rules: self.ranking_rules,
stop_words: self.stop_words,
synonyms: self.synonyms,
distinct_attribute: self.distinct_attribute,
typo_tolerance: self.typo_tolerance,
faceting: self.faceting,
pagination: self.pagination,
_kind: PhantomData,
} }
otherwise => otherwise,
} }
} }
@ -362,7 +277,7 @@ impl Index {
Ok(addition) Ok(addition)
} }
pub fn update_settings(&self, settings: &Settings<Checked>) -> Result<()> { pub fn update_settings(&self, settings: &Settings) -> Result<()> {
// We must use the write transaction of the update here. // We must use the write transaction of the update here.
let mut txn = self.write_txn()?; let mut txn = self.write_txn()?;
let mut builder = let mut builder =
@ -378,10 +293,7 @@ impl Index {
} }
} }
pub fn apply_settings_to_builder( pub fn apply_settings_to_builder(settings: &Settings, builder: &mut milli::update::Settings) {
settings: &Settings<Checked>,
builder: &mut milli::update::Settings,
) {
match settings.searchable_attributes { match settings.searchable_attributes {
Setting::Set(ref names) => builder.set_searchable_fields(names.clone()), Setting::Set(ref names) => builder.set_searchable_fields(names.clone()),
Setting::Reset => builder.reset_searchable_fields(), Setting::Reset => builder.reset_searchable_fields(),
@ -537,7 +449,6 @@ pub(crate) mod test {
typo_tolerance: Setting::NotSet, typo_tolerance: Setting::NotSet,
faceting: Setting::NotSet, faceting: Setting::NotSet,
pagination: Setting::NotSet, pagination: Setting::NotSet,
_kind: PhantomData::<Unchecked>,
}; };
let checked = settings.clone().check(); let checked = settings.clone().check();
@ -561,7 +472,6 @@ pub(crate) mod test {
typo_tolerance: Setting::NotSet, typo_tolerance: Setting::NotSet,
faceting: Setting::NotSet, faceting: Setting::NotSet,
pagination: Setting::NotSet, pagination: Setting::NotSet,
_kind: PhantomData::<Unchecked>,
}; };
let checked = settings.check(); let checked = settings.check();

View File

@ -22,9 +22,7 @@ use uuid::Uuid;
use crate::document_formats::{read_csv, read_json, read_ndjson}; use crate::document_formats::{read_csv, read_json, read_ndjson};
use crate::dump::{self, load_dump, DumpHandler}; use crate::dump::{self, load_dump, DumpHandler};
use crate::index::{ use crate::index::{Document, IndexMeta, IndexStats, SearchQuery, SearchResult, Settings};
Checked, Document, IndexMeta, IndexStats, SearchQuery, SearchResult, Settings, Unchecked,
};
use crate::index_resolver::error::IndexResolverError; use crate::index_resolver::error::IndexResolverError;
use crate::options::{IndexerOpts, SchedulerConfig}; use crate::options::{IndexerOpts, SchedulerConfig};
use crate::snapshot::{load_snapshot, SnapshotService}; use crate::snapshot::{load_snapshot, SnapshotService};
@ -126,7 +124,7 @@ pub enum Update {
DeleteDocuments(Vec<String>), DeleteDocuments(Vec<String>),
ClearDocuments, ClearDocuments,
Settings { Settings {
settings: Settings<Unchecked>, settings: Settings,
/// Indicates whether the update was a deletion /// Indicates whether the update was a deletion
is_deletion: bool, is_deletion: bool,
allow_index_creation: bool, allow_index_creation: bool,
@ -530,7 +528,7 @@ where
Ok(ret) Ok(ret)
} }
pub async fn settings(&self, uid: String) -> Result<Settings<Checked>> { pub async fn settings(&self, uid: String) -> Result<Settings> {
let index = self.index_resolver.get_index(uid).await?; let index = self.index_resolver.get_index(uid).await?;
let settings = spawn_blocking(move || index.settings()).await??; let settings = spawn_blocking(move || index.settings()).await??;
Ok(settings) Ok(settings)

View File

@ -225,7 +225,7 @@ mod real {
}; };
let settings = settings.clone(); let settings = settings.clone();
spawn_blocking(move || index.update_settings(&settings.check())).await??; spawn_blocking(move || index.update_settings(&settings)).await??;
Ok(TaskResult::Other) Ok(TaskResult::Other)
} }

View File

@ -6,7 +6,7 @@ use time::OffsetDateTime;
use uuid::Uuid; use uuid::Uuid;
use super::batch::BatchId; use super::batch::BatchId;
use crate::index::{Settings, Unchecked}; use crate::index::Settings;
pub type TaskId = u32; pub type TaskId = u32;
@ -155,7 +155,7 @@ pub enum TaskContent {
}, },
SettingsUpdate { SettingsUpdate {
index_uid: IndexUid, index_uid: IndexUid,
settings: Settings<Unchecked>, settings: Settings,
/// Indicates whether the task was a deletion /// Indicates whether the task was a deletion
is_deletion: bool, is_deletion: bool,
allow_index_creation: bool, allow_index_creation: bool,