From 4c1f034d9f3c3e5189b7355b3c21e3fb18ee39f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Lecrenier?= Date: Tue, 21 Jun 2022 15:34:53 +0200 Subject: [PATCH] Remove Checked/Unchecked type parameter from Settings type --- .../src/routes/indexes/settings.rs | 6 +- meilisearch-http/src/routes/mod.rs | 4 +- meilisearch-http/src/task.rs | 4 +- meilisearch-lib/src/dump/compat/v2.rs | 4 +- meilisearch-lib/src/dump/compat/v3.rs | 6 +- meilisearch-lib/src/dump/compat/v4.rs | 4 +- meilisearch-lib/src/index/dump.rs | 7 +- meilisearch-lib/src/index/index.rs | 8 +- meilisearch-lib/src/index/mod.rs | 6 +- meilisearch-lib/src/index/updates.rs | 128 +++--------------- meilisearch-lib/src/index_controller/mod.rs | 8 +- meilisearch-lib/src/index_resolver/mod.rs | 2 +- meilisearch-lib/src/tasks/task.rs | 4 +- 13 files changed, 48 insertions(+), 143 deletions(-) diff --git a/meilisearch-http/src/routes/indexes/settings.rs b/meilisearch-http/src/routes/indexes/settings.rs index 5b7e9d609..c7a4a9fcd 100644 --- a/meilisearch-http/src/routes/indexes/settings.rs +++ b/meilisearch-http/src/routes/indexes/settings.rs @@ -1,7 +1,7 @@ use log::debug; 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::MeiliSearch; use meilisearch_types::error::{MeiliDeserError, ResponseError}; @@ -356,7 +356,7 @@ generate_configure!( pub async fn update_all( meilisearch: GuardedData, MeiliSearch>, index_uid: web::Path, - body: ValidatedJson, MeiliDeserError>, + body: ValidatedJson, req: HttpRequest, analytics: web::Data, ) -> Result { @@ -442,7 +442,7 @@ pub async fn delete_all( data: GuardedData, MeiliSearch>, index_uid: web::Path, ) -> Result { - let settings = Settings::cleared().into_unchecked(); + let settings = Settings::cleared(); let allow_index_creation = data.filters().allow_index_creation; let update = Update::Settings { diff --git a/meilisearch-http/src/routes/mod.rs b/meilisearch-http/src/routes/mod.rs index f61854c48..d97c0409f 100644 --- a/meilisearch-http/src/routes/mod.rs +++ b/meilisearch-http/src/routes/mod.rs @@ -4,7 +4,7 @@ use serde::{Deserialize, Serialize}; use time::OffsetDateTime; -use meilisearch_lib::index::{Settings, Unchecked}; +use meilisearch_lib::index::Settings; use meilisearch_lib::MeiliSearch; use meilisearch_types::error::ResponseError; use meilisearch_types::star_or::StarOr; @@ -140,7 +140,7 @@ pub enum UpdateType { number: Option, }, Settings { - settings: Settings, + settings: Settings, }, } diff --git a/meilisearch-http/src/task.rs b/meilisearch-http/src/task.rs index 06bba1f76..6201e9e1a 100644 --- a/meilisearch-http/src/task.rs +++ b/meilisearch-http/src/task.rs @@ -3,7 +3,7 @@ use std::fmt::{self, Write}; use std::str::FromStr; use std::write; -use meilisearch_lib::index::{Settings, Unchecked}; +use meilisearch_lib::index::Settings; use meilisearch_lib::tasks::batch::BatchId; use meilisearch_lib::tasks::task::{ DocumentDeletion, Task, TaskContent, TaskEvent, TaskId, TaskResult, @@ -144,7 +144,7 @@ enum TaskDetails { #[serde(rename_all = "camelCase")] Settings { #[serde(flatten)] - settings: Settings, + settings: Settings, }, #[serde(rename_all = "camelCase")] IndexInfo { primary_key: Option }, diff --git a/meilisearch-lib/src/dump/compat/v2.rs b/meilisearch-lib/src/dump/compat/v2.rs index 364d894c4..95237e138 100644 --- a/meilisearch-lib/src/dump/compat/v2.rs +++ b/meilisearch-lib/src/dump/compat/v2.rs @@ -5,7 +5,7 @@ use serde::{Deserialize, Serialize}; use time::OffsetDateTime; use uuid::Uuid; -use crate::index::{Settings, Unchecked}; +use crate::index::Settings; #[derive(Serialize, Deserialize)] pub struct UpdateEntry { @@ -43,7 +43,7 @@ pub enum UpdateMeta { DeleteDocuments { ids: Vec, }, - Settings(Settings), + Settings(Settings), } #[derive(Debug, Serialize, Deserialize, Clone)] diff --git a/meilisearch-lib/src/dump/compat/v3.rs b/meilisearch-lib/src/dump/compat/v3.rs index 61e31eccd..e8abec730 100644 --- a/meilisearch-lib/src/dump/compat/v3.rs +++ b/meilisearch-lib/src/dump/compat/v3.rs @@ -6,7 +6,7 @@ use time::OffsetDateTime; use uuid::Uuid; use super::v4::{Task, TaskContent, TaskEvent}; -use crate::index::{Settings, Unchecked}; +use crate::index::Settings; use crate::tasks::task::{DocumentDeletion, TaskId, TaskResult}; use super::v2; @@ -55,7 +55,7 @@ pub enum Update { method: IndexDocumentsMethod, content_uuid: Uuid, }, - Settings(Settings), + Settings(Settings), ClearDocuments, } @@ -100,7 +100,7 @@ pub enum UpdateMeta { DeleteDocuments { ids: Vec, }, - Settings(Settings), + Settings(Settings), } #[derive(Debug, Serialize, Deserialize, Clone)] diff --git a/meilisearch-lib/src/dump/compat/v4.rs b/meilisearch-lib/src/dump/compat/v4.rs index c412e7f17..df8d93241 100644 --- a/meilisearch-lib/src/dump/compat/v4.rs +++ b/meilisearch-lib/src/dump/compat/v4.rs @@ -5,7 +5,7 @@ use serde::{Deserialize, Serialize}; use time::OffsetDateTime; use uuid::Uuid; -use crate::index::{Settings, Unchecked}; +use crate::index::Settings; use crate::tasks::batch::BatchId; use crate::tasks::task::{ DocumentDeletion, TaskContent as NewTaskContent, TaskEvent as NewTaskEvent, TaskId, TaskResult, @@ -82,7 +82,7 @@ pub enum TaskContent { }, DocumentDeletion(DocumentDeletion), SettingsUpdate { - settings: Settings, + settings: Settings, /// Indicates whether the task was a deletion is_deletion: bool, allow_index_creation: bool, diff --git a/meilisearch-lib/src/index/dump.rs b/meilisearch-lib/src/index/dump.rs index e201e738b..83ec821b4 100644 --- a/meilisearch-lib/src/index/dump.rs +++ b/meilisearch-lib/src/index/dump.rs @@ -13,11 +13,11 @@ use crate::document_formats::read_ndjson; use crate::index::updates::apply_settings_to_builder; use super::error::Result; -use super::{index::Index, Settings, Unchecked}; +use super::{index::Index, Settings}; #[derive(Serialize, Deserialize)] struct DumpMeta { - settings: Settings, + settings: Settings, primary_key: Option, } @@ -69,7 +69,7 @@ impl Index { let meta_file_path = path.as_ref().join(META_FILE_NAME); 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 meta = DumpMeta { settings, @@ -101,7 +101,6 @@ impl Index { settings, primary_key, } = serde_json::from_reader(meta_file)?; - let settings = settings.check(); let mut options = EnvOpenOptions::new(); options.map_size(size); diff --git a/meilisearch-lib/src/index/index.rs b/meilisearch-lib/src/index/index.rs index c79c7e8f1..e5bb3eae8 100644 --- a/meilisearch-lib/src/index/index.rs +++ b/meilisearch-lib/src/index/index.rs @@ -1,6 +1,5 @@ use std::collections::BTreeSet; use std::fs::create_dir_all; -use std::marker::PhantomData; use std::ops::Deref; use std::path::Path; use std::sync::Arc; @@ -20,7 +19,7 @@ use crate::EnvSizer; use super::error::IndexError; use super::error::Result; use super::updates::{FacetingSettings, MinWordSizeTyposSetting, PaginationSettings, TypoSettings}; -use super::{Checked, Settings}; +use super::Settings; pub type Document = Map; @@ -121,7 +120,7 @@ impl Index { pub fn meta(&self) -> Result { IndexMeta::new(self) } - pub fn settings(&self) -> Result> { + pub fn settings(&self) -> Result { let txn = self.read_txn()?; self.settings_txn(&txn) } @@ -130,7 +129,7 @@ impl Index { self.uuid } - pub fn settings_txn(&self, txn: &RoTxn) -> Result> { + pub fn settings_txn(&self, txn: &RoTxn) -> Result { let displayed_attributes = self .displayed_fields(txn)? .map(|fields| fields.into_iter().map(String::from).collect()); @@ -225,7 +224,6 @@ impl Index { typo_tolerance: Setting::Set(typo_tolerance), faceting: Setting::Set(faceting), pagination: Setting::Set(pagination), - _kind: PhantomData, }) } diff --git a/meilisearch-lib/src/index/mod.rs b/meilisearch-lib/src/index/mod.rs index e6c831a01..0250947c2 100644 --- a/meilisearch-lib/src/index/mod.rs +++ b/meilisearch-lib/src/index/mod.rs @@ -2,7 +2,7 @@ pub use search::{ SearchQuery, SearchResult, DEFAULT_CROP_LENGTH, DEFAULT_CROP_MARKER, 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; pub mod error; @@ -89,7 +89,7 @@ pub mod test { MockIndex::Mock(_) => todo!(), } } - pub fn settings(&self) -> Result> { + pub fn settings(&self) -> Result { match self { MockIndex::Real(index) => index.settings(), MockIndex::Mock(_) => todo!(), @@ -175,7 +175,7 @@ pub mod test { } } - pub fn update_settings(&self, settings: &Settings) -> Result<()> { + pub fn update_settings(&self, settings: &Settings) -> Result<()> { match self { MockIndex::Real(index) => index.update_settings(settings), MockIndex::Mock(m) => unsafe { m.get("update_settings").call(settings) }, diff --git a/meilisearch-lib/src/index/updates.rs b/meilisearch-lib/src/index/updates.rs index 011e59e11..d1bb11d40 100644 --- a/meilisearch-lib/src/index/updates.rs +++ b/meilisearch-lib/src/index/updates.rs @@ -8,7 +8,6 @@ use milli::update::{ use milli::Criterion; use serde::{Deserialize, Serialize, Serializer}; use std::collections::{BTreeMap, BTreeSet}; -use std::marker::PhantomData; use std::num::NonZeroUsize; use uuid::Uuid; @@ -32,26 +31,6 @@ where .serialize(s) } -#[derive(Clone, Default, Debug, Serialize, PartialEq)] -pub struct Checked; - -#[derive(Clone, Default, Debug, Serialize, Deserialize, PartialEq)] -pub struct Unchecked; -impl DeserializeFromValue for Unchecked -where - E: jayson::DeserializeError, -{ - fn deserialize_from_value( - _value: jayson::Value, - _location: jayson::ValuePointerRef, - ) -> std::result::Result - where - V: jayson::IntoValue, - { - Ok(Unchecked) - } -} - #[cfg_attr(test, derive(proptest_derive::Arbitrary))] #[derive(Debug, Clone, Default, Serialize, Deserialize, PartialEq, DeserializeFromValue)] #[serde(deny_unknown_fields)] @@ -108,25 +87,23 @@ pub struct PaginationSettings { pub limited_to: Setting, } -/// Holds all the settings for an index. `T` can either be `Checked` if they represents settings -/// whose validity is guaranteed, or `Unchecked` if they need to be validated. In the later case, a -/// call to `check` will return a `Settings` from a `Settings`. - +/// Holds all the settings for an index. The settings are validated by the implementation of +/// jayson::DeserializeFromValue. #[derive( Debug, Clone, Default, Serialize, Deserialize, PartialEq, jayson::DeserializeFromValue, )] #[serde(deny_unknown_fields)] #[serde(rename_all = "camelCase")] -#[serde(bound(serialize = "T: Serialize", deserialize = "T: Deserialize<'static>"))] #[cfg_attr(test, derive(proptest_derive::Arbitrary))] #[jayson(rename_all = camelCase, deny_unknown_fields)] -pub struct Settings { +pub struct Settings { #[serde( default, serialize_with = "serialize_with_wildcard", skip_serializing_if = "Setting::is_not_set" )] #[cfg_attr(test, proptest(strategy = "test::setting_strategy()"))] + #[jayson(map = map_searchable_or_displayed_attributes)] pub displayed_attributes: Setting>, #[serde( @@ -135,6 +112,7 @@ pub struct Settings { skip_serializing_if = "Setting::is_not_set" )] #[cfg_attr(test, proptest(strategy = "test::setting_strategy()"))] + #[jayson(map = map_searchable_or_displayed_attributes)] pub searchable_attributes: Setting>, #[serde(default, skip_serializing_if = "Setting::is_not_set")] @@ -165,13 +143,10 @@ pub struct Settings { #[serde(default, skip_serializing_if = "Setting::is_not_set")] #[cfg_attr(test, proptest(strategy = "test::setting_strategy()"))] pub pagination: Setting, - - #[serde(skip)] - pub _kind: PhantomData, } -impl Settings { - pub fn cleared() -> Settings { +impl Settings { + pub fn cleared() -> Settings { Settings { displayed_attributes: Setting::Reset, searchable_attributes: Setting::Reset, @@ -184,81 +159,21 @@ impl Settings { typo_tolerance: Setting::Reset, faceting: Setting::Reset, pagination: Setting::Reset, - _kind: PhantomData, - } - } - - pub fn into_unchecked(self) -> Settings { - 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, } } } - -impl Settings { - pub fn check(self) -> Settings { - let displayed_attributes = match self.displayed_attributes { - Setting::Set(fields) => { - if fields.iter().any(|f| f == "*") { - Setting::Reset - } else { - Setting::Set(fields) - } +pub fn map_searchable_or_displayed_attributes( + setting: Setting>, +) -> Setting> { + match setting { + Setting::Set(fields) => { + if fields.iter().any(|f| f == "*") { + Setting::Reset + } else { + 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) } - pub fn update_settings(&self, settings: &Settings) -> Result<()> { + pub fn update_settings(&self, settings: &Settings) -> Result<()> { // We must use the write transaction of the update here. let mut txn = self.write_txn()?; let mut builder = @@ -378,10 +293,7 @@ impl Index { } } -pub fn apply_settings_to_builder( - settings: &Settings, - builder: &mut milli::update::Settings, -) { +pub fn apply_settings_to_builder(settings: &Settings, builder: &mut milli::update::Settings) { match settings.searchable_attributes { Setting::Set(ref names) => builder.set_searchable_fields(names.clone()), Setting::Reset => builder.reset_searchable_fields(), @@ -537,7 +449,6 @@ pub(crate) mod test { typo_tolerance: Setting::NotSet, faceting: Setting::NotSet, pagination: Setting::NotSet, - _kind: PhantomData::, }; let checked = settings.clone().check(); @@ -561,7 +472,6 @@ pub(crate) mod test { typo_tolerance: Setting::NotSet, faceting: Setting::NotSet, pagination: Setting::NotSet, - _kind: PhantomData::, }; let checked = settings.check(); diff --git a/meilisearch-lib/src/index_controller/mod.rs b/meilisearch-lib/src/index_controller/mod.rs index 88782c5ea..4fddc12fe 100644 --- a/meilisearch-lib/src/index_controller/mod.rs +++ b/meilisearch-lib/src/index_controller/mod.rs @@ -22,9 +22,7 @@ use uuid::Uuid; use crate::document_formats::{read_csv, read_json, read_ndjson}; use crate::dump::{self, load_dump, DumpHandler}; -use crate::index::{ - Checked, Document, IndexMeta, IndexStats, SearchQuery, SearchResult, Settings, Unchecked, -}; +use crate::index::{Document, IndexMeta, IndexStats, SearchQuery, SearchResult, Settings}; use crate::index_resolver::error::IndexResolverError; use crate::options::{IndexerOpts, SchedulerConfig}; use crate::snapshot::{load_snapshot, SnapshotService}; @@ -126,7 +124,7 @@ pub enum Update { DeleteDocuments(Vec), ClearDocuments, Settings { - settings: Settings, + settings: Settings, /// Indicates whether the update was a deletion is_deletion: bool, allow_index_creation: bool, @@ -530,7 +528,7 @@ where Ok(ret) } - pub async fn settings(&self, uid: String) -> Result> { + pub async fn settings(&self, uid: String) -> Result { let index = self.index_resolver.get_index(uid).await?; let settings = spawn_blocking(move || index.settings()).await??; Ok(settings) diff --git a/meilisearch-lib/src/index_resolver/mod.rs b/meilisearch-lib/src/index_resolver/mod.rs index 686a549b9..035a4db9a 100644 --- a/meilisearch-lib/src/index_resolver/mod.rs +++ b/meilisearch-lib/src/index_resolver/mod.rs @@ -225,7 +225,7 @@ mod real { }; let settings = settings.clone(); - spawn_blocking(move || index.update_settings(&settings.check())).await??; + spawn_blocking(move || index.update_settings(&settings)).await??; Ok(TaskResult::Other) } diff --git a/meilisearch-lib/src/tasks/task.rs b/meilisearch-lib/src/tasks/task.rs index 42f75834f..9840fa623 100644 --- a/meilisearch-lib/src/tasks/task.rs +++ b/meilisearch-lib/src/tasks/task.rs @@ -6,7 +6,7 @@ use time::OffsetDateTime; use uuid::Uuid; use super::batch::BatchId; -use crate::index::{Settings, Unchecked}; +use crate::index::Settings; pub type TaskId = u32; @@ -155,7 +155,7 @@ pub enum TaskContent { }, SettingsUpdate { index_uid: IndexUid, - settings: Settings, + settings: Settings, /// Indicates whether the task was a deletion is_deletion: bool, allow_index_creation: bool,