5101: Fix index settings opt out r=Kerollmops a=ManyTheFish

# Pull Request

## Related issue
Fixes #5099 

## What does this PR do?
- Refactor the settings implementation ensuring the routes are configured
- Add a test checking if all the routes are tested
- Refactor the tests to ease the modifications


Co-authored-by: ManyTheFish <many@meilisearch.com>
This commit is contained in:
meili-bors[bot] 2024-11-28 14:23:33 +00:00 committed by GitHub
commit d49d127863
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 426 additions and 398 deletions

View File

@ -17,6 +17,32 @@ use crate::extractors::authentication::GuardedData;
use crate::routes::{get_task_id, is_dry_run, SummarizedTaskView}; use crate::routes::{get_task_id, is_dry_run, SummarizedTaskView};
use crate::Opt; use crate::Opt;
/// This macro generates the routes for the settings.
///
/// It takes a list of settings and generates a module for each setting.
/// Each module contains the `get`, `update` and `delete` routes for the setting.
///
/// It also generates a `configure` function that configures the routes for the settings.
macro_rules! make_setting_routes {
($({route: $route:literal, update_verb: $update_verb:ident, value_type: $type:ty, err_type: $err_ty:ty, attr: $attr:ident, camelcase_attr: $camelcase_attr:literal, analytics: $analytics:ident},)*) => {
$(
make_setting_route!($route, $update_verb, $type, $err_ty, $attr, $camelcase_attr, $analytics);
)*
pub fn configure(cfg: &mut web::ServiceConfig) {
use crate::extractors::sequential_extractor::SeqHandler;
cfg.service(
web::resource("")
.route(web::patch().to(SeqHandler(update_all)))
.route(web::get().to(SeqHandler(get_all)))
.route(web::delete().to(SeqHandler(delete_all))))
$(.service($attr::resources()))*;
}
pub const ALL_SETTINGS_NAMES: &[&str] = &[$(stringify!($attr)),*];
};
}
#[macro_export] #[macro_export]
macro_rules! make_setting_route { macro_rules! make_setting_route {
($route:literal, $update_verb:ident, $type:ty, $err_ty:ty, $attr:ident, $camelcase_attr:literal, $analytics:ident) => { ($route:literal, $update_verb:ident, $type:ty, $err_ty:ty, $attr:ident, $camelcase_attr:literal, $analytics:ident) => {
@ -153,279 +179,227 @@ macro_rules! make_setting_route {
}; };
} }
make_setting_route!( make_setting_routes!(
"/filterable-attributes", {
put, route: "/filterable-attributes",
std::collections::BTreeSet<String>, update_verb: put,
meilisearch_types::deserr::DeserrJsonError< value_type: std::collections::BTreeSet<String>,
err_type: meilisearch_types::deserr::DeserrJsonError<
meilisearch_types::error::deserr_codes::InvalidSettingsFilterableAttributes, meilisearch_types::error::deserr_codes::InvalidSettingsFilterableAttributes,
>, >,
filterable_attributes, attr: filterable_attributes,
"filterableAttributes", camelcase_attr: "filterableAttributes",
FilterableAttributesAnalytics analytics: FilterableAttributesAnalytics
); },
{
make_setting_route!( route: "/sortable-attributes",
"/sortable-attributes", update_verb: put,
put, value_type: std::collections::BTreeSet<String>,
std::collections::BTreeSet<String>, err_type: meilisearch_types::deserr::DeserrJsonError<
meilisearch_types::deserr::DeserrJsonError<
meilisearch_types::error::deserr_codes::InvalidSettingsSortableAttributes, meilisearch_types::error::deserr_codes::InvalidSettingsSortableAttributes,
>, >,
sortable_attributes, attr: sortable_attributes,
"sortableAttributes", camelcase_attr: "sortableAttributes",
SortableAttributesAnalytics analytics: SortableAttributesAnalytics
); },
{
make_setting_route!( route: "/displayed-attributes",
"/displayed-attributes", update_verb: put,
put, value_type: Vec<String>,
Vec<String>, err_type: meilisearch_types::deserr::DeserrJsonError<
meilisearch_types::deserr::DeserrJsonError<
meilisearch_types::error::deserr_codes::InvalidSettingsDisplayedAttributes, meilisearch_types::error::deserr_codes::InvalidSettingsDisplayedAttributes,
>, >,
displayed_attributes, attr: displayed_attributes,
"displayedAttributes", camelcase_attr: "displayedAttributes",
DisplayedAttributesAnalytics analytics: DisplayedAttributesAnalytics
); },
{
make_setting_route!( route: "/typo-tolerance",
"/typo-tolerance", update_verb: patch,
patch, value_type: meilisearch_types::settings::TypoSettings,
meilisearch_types::settings::TypoSettings, err_type: meilisearch_types::deserr::DeserrJsonError<
meilisearch_types::deserr::DeserrJsonError<
meilisearch_types::error::deserr_codes::InvalidSettingsTypoTolerance, meilisearch_types::error::deserr_codes::InvalidSettingsTypoTolerance,
>, >,
typo_tolerance, attr: typo_tolerance,
"typoTolerance", camelcase_attr: "typoTolerance",
TypoToleranceAnalytics analytics: TypoToleranceAnalytics
); },
{
make_setting_route!( route: "/searchable-attributes",
"/searchable-attributes", update_verb: put,
put, value_type: Vec<String>,
Vec<String>, err_type: meilisearch_types::deserr::DeserrJsonError<
meilisearch_types::deserr::DeserrJsonError<
meilisearch_types::error::deserr_codes::InvalidSettingsSearchableAttributes, meilisearch_types::error::deserr_codes::InvalidSettingsSearchableAttributes,
>, >,
searchable_attributes, attr: searchable_attributes,
"searchableAttributes", camelcase_attr: "searchableAttributes",
SearchableAttributesAnalytics analytics: SearchableAttributesAnalytics
); },
{
make_setting_route!( route: "/stop-words",
"/stop-words", update_verb: put,
put, value_type: std::collections::BTreeSet<String>,
std::collections::BTreeSet<String>, err_type: meilisearch_types::deserr::DeserrJsonError<
meilisearch_types::deserr::DeserrJsonError<
meilisearch_types::error::deserr_codes::InvalidSettingsStopWords, meilisearch_types::error::deserr_codes::InvalidSettingsStopWords,
>, >,
stop_words, attr: stop_words,
"stopWords", camelcase_attr: "stopWords",
StopWordsAnalytics analytics: StopWordsAnalytics
); },
{
make_setting_route!( route: "/non-separator-tokens",
"/non-separator-tokens", update_verb: put,
put, value_type: std::collections::BTreeSet<String>,
std::collections::BTreeSet<String>, err_type: meilisearch_types::deserr::DeserrJsonError<
meilisearch_types::deserr::DeserrJsonError<
meilisearch_types::error::deserr_codes::InvalidSettingsNonSeparatorTokens, meilisearch_types::error::deserr_codes::InvalidSettingsNonSeparatorTokens,
>, >,
non_separator_tokens, attr: non_separator_tokens,
"nonSeparatorTokens", camelcase_attr: "nonSeparatorTokens",
NonSeparatorTokensAnalytics analytics: NonSeparatorTokensAnalytics
); },
{
make_setting_route!( route: "/separator-tokens",
"/separator-tokens", update_verb: put,
put, value_type: std::collections::BTreeSet<String>,
std::collections::BTreeSet<String>, err_type: meilisearch_types::deserr::DeserrJsonError<
meilisearch_types::deserr::DeserrJsonError<
meilisearch_types::error::deserr_codes::InvalidSettingsSeparatorTokens, meilisearch_types::error::deserr_codes::InvalidSettingsSeparatorTokens,
>, >,
separator_tokens, attr: separator_tokens,
"separatorTokens", camelcase_attr: "separatorTokens",
SeparatorTokensAnalytics analytics: SeparatorTokensAnalytics
); },
{
make_setting_route!( route: "/dictionary",
"/dictionary", update_verb: put,
put, value_type: std::collections::BTreeSet<String>,
std::collections::BTreeSet<String>, err_type: meilisearch_types::deserr::DeserrJsonError<
meilisearch_types::deserr::DeserrJsonError<
meilisearch_types::error::deserr_codes::InvalidSettingsDictionary, meilisearch_types::error::deserr_codes::InvalidSettingsDictionary,
>, >,
dictionary, attr: dictionary,
"dictionary", camelcase_attr: "dictionary",
DictionaryAnalytics analytics: DictionaryAnalytics
); },
{
make_setting_route!( route: "/synonyms",
"/synonyms", update_verb: put,
put, value_type: std::collections::BTreeMap<String, Vec<String>>,
std::collections::BTreeMap<String, Vec<String>>, err_type: meilisearch_types::deserr::DeserrJsonError<
meilisearch_types::deserr::DeserrJsonError<
meilisearch_types::error::deserr_codes::InvalidSettingsSynonyms, meilisearch_types::error::deserr_codes::InvalidSettingsSynonyms,
>, >,
synonyms, attr: synonyms,
"synonyms", camelcase_attr: "synonyms",
SynonymsAnalytics analytics: SynonymsAnalytics
); },
{
make_setting_route!( route: "/distinct-attribute",
"/distinct-attribute", update_verb: put,
put, value_type: String,
String, err_type: meilisearch_types::deserr::DeserrJsonError<
meilisearch_types::deserr::DeserrJsonError<
meilisearch_types::error::deserr_codes::InvalidSettingsDistinctAttribute, meilisearch_types::error::deserr_codes::InvalidSettingsDistinctAttribute,
>, >,
distinct_attribute, attr: distinct_attribute,
"distinctAttribute", camelcase_attr: "distinctAttribute",
DistinctAttributeAnalytics analytics: DistinctAttributeAnalytics
); },
{
make_setting_route!( route: "/proximity-precision",
"/proximity-precision", update_verb: put,
put, value_type: meilisearch_types::settings::ProximityPrecisionView,
meilisearch_types::settings::ProximityPrecisionView, err_type: meilisearch_types::deserr::DeserrJsonError<
meilisearch_types::deserr::DeserrJsonError<
meilisearch_types::error::deserr_codes::InvalidSettingsProximityPrecision, meilisearch_types::error::deserr_codes::InvalidSettingsProximityPrecision,
>, >,
proximity_precision, attr: proximity_precision,
"proximityPrecision", camelcase_attr: "proximityPrecision",
ProximityPrecisionAnalytics analytics: ProximityPrecisionAnalytics
); },
{
make_setting_route!( route: "/localized-attributes",
"/localized-attributes", update_verb: put,
put, value_type: Vec<meilisearch_types::locales::LocalizedAttributesRuleView>,
Vec<meilisearch_types::locales::LocalizedAttributesRuleView>, err_type: meilisearch_types::deserr::DeserrJsonError<
meilisearch_types::deserr::DeserrJsonError<
meilisearch_types::error::deserr_codes::InvalidSettingsLocalizedAttributes, meilisearch_types::error::deserr_codes::InvalidSettingsLocalizedAttributes,
>, >,
localized_attributes, attr: localized_attributes,
"localizedAttributes", camelcase_attr: "localizedAttributes",
LocalesAnalytics analytics: LocalesAnalytics
); },
{
make_setting_route!( route: "/ranking-rules",
"/ranking-rules", update_verb: put,
put, value_type: Vec<meilisearch_types::settings::RankingRuleView>,
Vec<meilisearch_types::settings::RankingRuleView>, err_type: meilisearch_types::deserr::DeserrJsonError<
meilisearch_types::deserr::DeserrJsonError<
meilisearch_types::error::deserr_codes::InvalidSettingsRankingRules, meilisearch_types::error::deserr_codes::InvalidSettingsRankingRules,
>, >,
ranking_rules, attr: ranking_rules,
"rankingRules", camelcase_attr: "rankingRules",
RankingRulesAnalytics analytics: RankingRulesAnalytics
); },
{
make_setting_route!( route: "/faceting",
"/faceting", update_verb: patch,
patch, value_type: meilisearch_types::settings::FacetingSettings,
meilisearch_types::settings::FacetingSettings, err_type: meilisearch_types::deserr::DeserrJsonError<
meilisearch_types::deserr::DeserrJsonError<
meilisearch_types::error::deserr_codes::InvalidSettingsFaceting, meilisearch_types::error::deserr_codes::InvalidSettingsFaceting,
>, >,
faceting, attr: faceting,
"faceting", camelcase_attr: "faceting",
FacetingAnalytics analytics: FacetingAnalytics
); },
{
make_setting_route!( route: "/pagination",
"/pagination", update_verb: patch,
patch, value_type: meilisearch_types::settings::PaginationSettings,
meilisearch_types::settings::PaginationSettings, err_type: meilisearch_types::deserr::DeserrJsonError<
meilisearch_types::deserr::DeserrJsonError<
meilisearch_types::error::deserr_codes::InvalidSettingsPagination, meilisearch_types::error::deserr_codes::InvalidSettingsPagination,
>, >,
pagination, attr: pagination,
"pagination", camelcase_attr: "pagination",
PaginationAnalytics analytics: PaginationAnalytics
); },
{
make_setting_route!( route: "/embedders",
"/embedders", update_verb: patch,
patch, value_type: std::collections::BTreeMap<String, Setting<meilisearch_types::milli::vector::settings::EmbeddingSettings>>,
std::collections::BTreeMap<String, Setting<meilisearch_types::milli::vector::settings::EmbeddingSettings>>, err_type: meilisearch_types::deserr::DeserrJsonError<
meilisearch_types::deserr::DeserrJsonError<
meilisearch_types::error::deserr_codes::InvalidSettingsEmbedders, meilisearch_types::error::deserr_codes::InvalidSettingsEmbedders,
>, >,
embedders, attr: embedders,
"embedders", camelcase_attr: "embedders",
EmbeddersAnalytics analytics: EmbeddersAnalytics
); },
{
make_setting_route!( route: "/search-cutoff-ms",
"/search-cutoff-ms", update_verb: put,
put, value_type: u64,
u64, err_type: meilisearch_types::deserr::DeserrJsonError<
meilisearch_types::deserr::DeserrJsonError<
meilisearch_types::error::deserr_codes::InvalidSettingsSearchCutoffMs, meilisearch_types::error::deserr_codes::InvalidSettingsSearchCutoffMs,
>, >,
search_cutoff_ms, attr: search_cutoff_ms,
"searchCutoffMs", camelcase_attr: "searchCutoffMs",
SearchCutoffMsAnalytics analytics: SearchCutoffMsAnalytics
); },
{
make_setting_route!( route: "/facet-search",
"/facet-search", update_verb: put,
put, value_type: bool,
bool, err_type: meilisearch_types::deserr::DeserrJsonError<
meilisearch_types::deserr::DeserrJsonError<
meilisearch_types::error::deserr_codes::InvalidSettingsFacetSearch, meilisearch_types::error::deserr_codes::InvalidSettingsFacetSearch,
>, >,
facet_search, attr: facet_search,
"facetSearch", camelcase_attr: "facetSearch",
FacetSearchAnalytics analytics: FacetSearchAnalytics
); },
{
make_setting_route!( route: "/prefix-search",
"/prefix-search", update_verb: put,
put, value_type: meilisearch_types::settings::PrefixSearchSettings,
meilisearch_types::settings::PrefixSearchSettings, err_type: meilisearch_types::deserr::DeserrJsonError<
meilisearch_types::deserr::DeserrJsonError<
meilisearch_types::error::deserr_codes::InvalidSettingsPrefixSearch, meilisearch_types::error::deserr_codes::InvalidSettingsPrefixSearch,
>, >,
prefix_search, attr: prefix_search,
"prefixSearch", camelcase_attr: "prefixSearch",
PrefixSearchAnalytics analytics: PrefixSearchAnalytics
); },
macro_rules! generate_configure {
($($mod:ident),*) => {
pub fn configure(cfg: &mut web::ServiceConfig) {
use crate::extractors::sequential_extractor::SeqHandler;
cfg.service(
web::resource("")
.route(web::patch().to(SeqHandler(update_all)))
.route(web::get().to(SeqHandler(get_all)))
.route(web::delete().to(SeqHandler(delete_all))))
$(.service($mod::resources()))*;
}
};
}
generate_configure!(
filterable_attributes,
sortable_attributes,
displayed_attributes,
localized_attributes,
searchable_attributes,
distinct_attribute,
proximity_precision,
stop_words,
separator_tokens,
non_separator_tokens,
dictionary,
synonyms,
ranking_rules,
typo_tolerance,
pagination,
faceting,
embedders,
search_cutoff_ms
); );
pub async fn update_all( pub async fn update_all(

View File

@ -1,44 +1,185 @@
use std::collections::HashMap; use crate::common::Server;
use once_cell::sync::Lazy;
use crate::common::{Server, Value};
use crate::json; use crate::json;
static DEFAULT_SETTINGS_VALUES: Lazy<HashMap<&'static str, Value>> = Lazy::new(|| { macro_rules! test_setting_routes {
let mut map = HashMap::new(); ($({setting: $setting:ident, update_verb: $update_verb:ident, default_value: $default_value:tt},) *) => {
map.insert("displayed_attributes", json!(["*"])); $(
map.insert("searchable_attributes", json!(["*"])); mod $setting {
map.insert("localized_attributes", json!(null)); use crate::common::Server;
map.insert("filterable_attributes", json!([]));
map.insert("distinct_attribute", json!(null)); #[actix_rt::test]
map.insert( async fn get_unexisting_index() {
"ranking_rules", let server = Server::new().await;
json!(["words", "typo", "proximity", "attribute", "sort", "exactness"]), let url = format!("/indexes/test/settings/{}",
); stringify!($setting)
map.insert("stop_words", json!([])); .chars()
map.insert("non_separator_tokens", json!([])); .map(|c| if c == '_' { '-' } else { c })
map.insert("separator_tokens", json!([])); .collect::<String>());
map.insert("dictionary", json!([])); let (_response, code) = server.service.get(url).await;
map.insert("synonyms", json!({})); assert_eq!(code, 404);
map.insert(
"faceting",
json!({
"maxValuesPerFacet": json!(100),
"sortFacetValuesBy": {
"*": "alpha"
} }
}),
); #[actix_rt::test]
map.insert( async fn update_unexisting_index() {
"pagination", let server = Server::new().await;
json!({ let url = format!("/indexes/test/settings/{}",
"maxTotalHits": json!(1000), stringify!($setting)
}), .chars()
); .map(|c| if c == '_' { '-' } else { c })
map.insert("search_cutoff_ms", json!(null)); .collect::<String>());
map let (response, code) = server.service.$update_verb(url, serde_json::Value::Null.into()).await;
}); assert_eq!(code, 202, "{}", response);
server.index("").wait_task(0).await;
let (response, code) = server.index("test").get().await;
assert_eq!(code, 200, "{}", response);
}
#[actix_rt::test]
async fn delete_unexisting_index() {
let server = Server::new().await;
let url = format!("/indexes/test/settings/{}",
stringify!($setting)
.chars()
.map(|c| if c == '_' { '-' } else { c })
.collect::<String>());
let (_, code) = server.service.delete(url).await;
assert_eq!(code, 202);
let response = server.index("").wait_task(0).await;
assert_eq!(response["status"], "failed");
}
#[actix_rt::test]
async fn get_default() {
let server = Server::new().await;
let index = server.index("test");
let (response, code) = index.create(None).await;
assert_eq!(code, 202, "{}", response);
index.wait_task(0).await;
let url = format!("/indexes/test/settings/{}",
stringify!($setting)
.chars()
.map(|c| if c == '_' { '-' } else { c })
.collect::<String>());
let (response, code) = server.service.get(url).await;
assert_eq!(code, 200, "{}", response);
let expected = crate::json!($default_value);
assert_eq!(expected, response);
}
}
)*
#[actix_rt::test]
async fn all_setting_tested() {
let expected = std::collections::BTreeSet::from_iter(meilisearch::routes::indexes::settings::ALL_SETTINGS_NAMES.iter());
let tested = std::collections::BTreeSet::from_iter([$(stringify!($setting)),*].iter());
let diff: Vec<_> = expected.difference(&tested).collect();
assert!(diff.is_empty(), "Not all settings were tested, please add the following settings to the `test_setting_routes!` macro: {:?}", diff);
}
};
}
test_setting_routes!(
{
setting: filterable_attributes,
update_verb: put,
default_value: []
},
{
setting: displayed_attributes,
update_verb: put,
default_value: ["*"]
},
{
setting: localized_attributes,
update_verb: put,
default_value: null
},
{
setting: searchable_attributes,
update_verb: put,
default_value: ["*"]
},
{
setting: distinct_attribute,
update_verb: put,
default_value: null
},
{
setting: stop_words,
update_verb: put,
default_value: []
},
{
setting: separator_tokens,
update_verb: put,
default_value: []
},
{
setting: non_separator_tokens,
update_verb: put,
default_value: []
},
{
setting: dictionary,
update_verb: put,
default_value: []
},
{
setting: ranking_rules,
update_verb: put,
default_value: ["words", "typo", "proximity", "attribute", "sort", "exactness"]
},
{
setting: synonyms,
update_verb: put,
default_value: {}
},
{
setting: pagination,
update_verb: patch,
default_value: {"maxTotalHits": 1000}
},
{
setting: faceting,
update_verb: patch,
default_value: {"maxValuesPerFacet": 100, "sortFacetValuesBy": {"*": "alpha"}}
},
{
setting: search_cutoff_ms,
update_verb: put,
default_value: null
},
{
setting: embedders,
update_verb: patch,
default_value: null
},
{
setting: facet_search,
update_verb: put,
default_value: true
},
{
setting: prefix_search,
update_verb: put,
default_value: "indexingTime"
},
{
setting: proximity_precision,
update_verb: put,
default_value: "byWord"
},
{
setting: sortable_attributes,
update_verb: put,
default_value: []
},
{
setting: typo_tolerance,
update_verb: patch,
default_value: {"enabled": true, "minWordSizeForTypos": {"oneTypo": 5, "twoTypos": 9}, "disableOnWords": [], "disableOnAttributes": []}
},
);
#[actix_rt::test] #[actix_rt::test]
async fn get_settings_unexisting_index() { async fn get_settings_unexisting_index() {
@ -342,93 +483,6 @@ async fn error_update_setting_unexisting_index_invalid_uid() {
"###); "###);
} }
macro_rules! test_setting_routes {
($($setting:ident $write_method:ident), *) => {
$(
mod $setting {
use crate::common::Server;
use super::DEFAULT_SETTINGS_VALUES;
#[actix_rt::test]
async fn get_unexisting_index() {
let server = Server::new().await;
let url = format!("/indexes/test/settings/{}",
stringify!($setting)
.chars()
.map(|c| if c == '_' { '-' } else { c })
.collect::<String>());
let (_response, code) = server.service.get(url).await;
assert_eq!(code, 404);
}
#[actix_rt::test]
async fn update_unexisting_index() {
let server = Server::new().await;
let url = format!("/indexes/test/settings/{}",
stringify!($setting)
.chars()
.map(|c| if c == '_' { '-' } else { c })
.collect::<String>());
let (response, code) = server.service.$write_method(url, serde_json::Value::Null.into()).await;
assert_eq!(code, 202, "{}", response);
server.index("").wait_task(0).await;
let (response, code) = server.index("test").get().await;
assert_eq!(code, 200, "{}", response);
}
#[actix_rt::test]
async fn delete_unexisting_index() {
let server = Server::new().await;
let url = format!("/indexes/test/settings/{}",
stringify!($setting)
.chars()
.map(|c| if c == '_' { '-' } else { c })
.collect::<String>());
let (_, code) = server.service.delete(url).await;
assert_eq!(code, 202);
let response = server.index("").wait_task(0).await;
assert_eq!(response["status"], "failed");
}
#[actix_rt::test]
async fn get_default() {
let server = Server::new().await;
let index = server.index("test");
let (response, code) = index.create(None).await;
assert_eq!(code, 202, "{}", response);
index.wait_task(0).await;
let url = format!("/indexes/test/settings/{}",
stringify!($setting)
.chars()
.map(|c| if c == '_' { '-' } else { c })
.collect::<String>());
let (response, code) = server.service.get(url).await;
assert_eq!(code, 200, "{}", response);
let expected = DEFAULT_SETTINGS_VALUES.get(stringify!($setting)).unwrap();
assert_eq!(expected, &response);
}
}
)*
};
}
test_setting_routes!(
filterable_attributes put,
displayed_attributes put,
localized_attributes put,
searchable_attributes put,
distinct_attribute put,
stop_words put,
separator_tokens put,
non_separator_tokens put,
dictionary put,
ranking_rules put,
synonyms put,
pagination patch,
faceting patch,
search_cutoff_ms put
);
#[actix_rt::test] #[actix_rt::test]
async fn error_set_invalid_ranking_rules() { async fn error_set_invalid_ranking_rules() {
let server = Server::new().await; let server = Server::new().await;