946: Sort displayedAttributes field r=MarinPostma a=gorogoroumaru

Fix #943

displayedAttributes use the HashSet struct which is an unsorted structure, so I changed the implementation from HashSet into BTreeSet.

Co-authored-by: gorogoroumaru <zokutyou2@gmail.com>
This commit is contained in:
bors[bot] 2020-10-13 14:37:47 +00:00 committed by GitHub
commit f359b64d59
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 96 additions and 11 deletions

View File

@ -10,6 +10,7 @@
## v0.14.0 ## v0.14.0
- Sort displayedAttributes (#943)
- Fix facet distribution case (#797) - Fix facet distribution case (#797)
- Snapshotting (#839) - Snapshotting (#839)
- Fix bucket-sort unwrap bug (#915) - Fix bucket-sort unwrap bug (#915)

View File

@ -1,4 +1,5 @@
use std::collections::hash_map::{Entry, HashMap}; use std::collections::hash_map::{Entry, HashMap};
use std::collections::BTreeMap;
use std::fs::File; use std::fs::File;
use std::path::Path; use std::path::Path;
use std::sync::{Arc, RwLock}; use std::sync::{Arc, RwLock};
@ -577,7 +578,7 @@ impl Database {
} }
// convert attributes to their names // convert attributes to their names
let frequency: HashMap<_, _> = fields_frequency let frequency: BTreeMap<_, _> = fields_frequency
.into_iter() .into_iter()
.filter_map(|(a, c)| schema.name(a).map(|name| (name.to_string(), c))) .filter_map(|(a, c)| schema.name(a).map(|name| (name.to_string(), c)))
.collect(); .collect();

View File

@ -1,4 +1,4 @@
use std::collections::{BTreeMap, BTreeSet, HashSet}; use std::collections::{BTreeMap, BTreeSet};
use std::str::FromStr; use std::str::FromStr;
use std::iter::IntoIterator; use std::iter::IntoIterator;
@ -23,7 +23,7 @@ pub struct Settings {
#[serde(default, deserialize_with = "deserialize_some")] #[serde(default, deserialize_with = "deserialize_some")]
pub searchable_attributes: Option<Option<Vec<String>>>, pub searchable_attributes: Option<Option<Vec<String>>>,
#[serde(default, deserialize_with = "deserialize_some")] #[serde(default, deserialize_with = "deserialize_some")]
pub displayed_attributes: Option<Option<HashSet<String>>>, pub displayed_attributes: Option<Option<BTreeSet<String>>>,
#[serde(default, deserialize_with = "deserialize_some")] #[serde(default, deserialize_with = "deserialize_some")]
pub stop_words: Option<Option<BTreeSet<String>>>, pub stop_words: Option<Option<BTreeSet<String>>>,
#[serde(default, deserialize_with = "deserialize_some")] #[serde(default, deserialize_with = "deserialize_some")]
@ -161,7 +161,7 @@ pub struct SettingsUpdate {
pub distinct_attribute: UpdateState<String>, pub distinct_attribute: UpdateState<String>,
pub primary_key: UpdateState<String>, pub primary_key: UpdateState<String>,
pub searchable_attributes: UpdateState<Vec<String>>, pub searchable_attributes: UpdateState<Vec<String>>,
pub displayed_attributes: UpdateState<HashSet<String>>, pub displayed_attributes: UpdateState<BTreeSet<String>>,
pub stop_words: UpdateState<BTreeSet<String>>, pub stop_words: UpdateState<BTreeSet<String>>,
pub synonyms: UpdateState<BTreeMap<String, Vec<String>>>, pub synonyms: UpdateState<BTreeMap<String, Vec<String>>>,
pub attributes_for_faceting: UpdateState<Vec<String>>, pub attributes_for_faceting: UpdateState<Vec<String>>,

View File

@ -1,5 +1,5 @@
use std::borrow::Cow; use std::borrow::Cow;
use std::collections::HashMap; use std::collections::BTreeMap;
use chrono::{DateTime, Utc}; use chrono::{DateTime, Utc};
use heed::types::{ByteSlice, OwnedType, SerdeBincode, Str, CowSlice}; use heed::types::{ByteSlice, OwnedType, SerdeBincode, Str, CowSlice};
@ -31,7 +31,7 @@ const SYNONYMS_KEY: &str = "synonyms";
const UPDATED_AT_KEY: &str = "updated-at"; const UPDATED_AT_KEY: &str = "updated-at";
const WORDS_KEY: &str = "words"; const WORDS_KEY: &str = "words";
pub type FreqsMap = HashMap<String, usize>; pub type FreqsMap = BTreeMap<String, usize>;
type SerdeFreqsMap = SerdeBincode<FreqsMap>; type SerdeFreqsMap = SerdeBincode<FreqsMap>;
type SerdeDatetime = SerdeBincode<DateTime<Utc>>; type SerdeDatetime = SerdeBincode<DateTime<Utc>>;

View File

@ -1,4 +1,4 @@
use std::collections::{BTreeMap, BTreeSet, HashSet}; use std::collections::{BTreeMap, BTreeSet};
use actix_web::{delete, get, post}; use actix_web::{delete, get, post};
use actix_web::{web, HttpResponse}; use actix_web::{web, HttpResponse};
@ -405,7 +405,7 @@ async fn get_displayed(
async fn update_displayed( async fn update_displayed(
data: web::Data<Data>, data: web::Data<Data>,
path: web::Path<IndexParam>, path: web::Path<IndexParam>,
body: web::Json<Option<HashSet<String>>>, body: web::Json<Option<BTreeSet<String>>>,
) -> Result<HttpResponse, ResponseError> { ) -> Result<HttpResponse, ResponseError> {
let index = data let index = data
.db .db
@ -539,7 +539,7 @@ fn get_indexed_attributes(schema: &Schema) -> Vec<String> {
} }
} }
fn get_displayed_attributes(schema: &Schema) -> HashSet<String> { fn get_displayed_attributes(schema: &Schema) -> BTreeSet<String> {
if schema.is_displayed_all() { if schema.is_displayed_all() {
["*"].iter().map(|s| s.to_string()).collect() ["*"].iter().map(|s| s.to_string()).collect()
} else { } else {

View File

@ -1,4 +1,4 @@
use std::collections::HashMap; use std::collections::{HashMap, BTreeMap};
use actix_web::web; use actix_web::web;
use actix_web::HttpResponse; use actix_web::HttpResponse;
@ -24,7 +24,7 @@ pub fn services(cfg: &mut web::ServiceConfig) {
struct IndexStatsResponse { struct IndexStatsResponse {
number_of_documents: u64, number_of_documents: u64,
is_indexing: bool, is_indexing: bool,
fields_distribution: HashMap<String, usize>, fields_distribution: BTreeMap<String, usize>,
} }
#[get("/indexes/{index_uid}/stats", wrap = "Authentication::Private")] #[get("/indexes/{index_uid}/stats", wrap = "Authentication::Private")]

View File

@ -777,3 +777,33 @@ async fn update_existing_primary_key_is_error() {
assert_eq!(response["errorCode"], "primary_key_already_present"); assert_eq!(response["errorCode"], "primary_key_already_present");
assert_eq!(response["errorType"], "invalid_request_error"); assert_eq!(response["errorType"], "invalid_request_error");
} }
#[actix_rt::test]
async fn test_facets_distribution_attribute() {
let mut server = common::Server::test_server().await;
let (response, _status_code) = server.get_index_stats().await;
let expected = json!({
"isIndexing": false,
"numberOfDocuments":77,
"fieldsDistribution":{
"age":77,
"gender":77,
"phone":77,
"name":77,
"registered":77,
"latitude":77,
"email":77,
"tags":77,
"longitude":77,
"color":77,
"address":77,
"balance":77,
"about":77,
"picture":77,
},
});
assert_json_eq!(expected, response, ordered: true);
}

View File

@ -468,3 +468,56 @@ async fn settings_that_contains_wildcard_is_wildcard() {
assert_eq!(response["searchableAttributes"].as_array().unwrap()[0], "*"); assert_eq!(response["searchableAttributes"].as_array().unwrap()[0], "*");
assert_eq!(response["displayedAttributes"].as_array().unwrap()[0], "*"); assert_eq!(response["displayedAttributes"].as_array().unwrap()[0], "*");
} }
#[actix_rt::test]
async fn test_displayed_attributes_field() {
let mut server = common::Server::test_server().await;
let body = json!({
"rankingRules": [
"typo",
"words",
"proximity",
"attribute",
"wordsPosition",
"exactness",
"desc(registered)",
"desc(age)",
],
"distinctAttribute": "id",
"searchableAttributes": [
"id",
"name",
"color",
"gender",
"email",
"phone",
"address",
"registered",
"about"
],
"displayedAttributes": [
"age",
"email",
"gender",
"name",
"registered",
],
"stopWords": [
"ad",
"in",
"ut",
],
"synonyms": {
"road": ["avenue", "street"],
"street": ["avenue"],
},
"attributesForFaceting": ["name"],
});
server.update_all_settings(body.clone()).await;
let (response, _status_code) = server.get_all_settings().await;
assert_json_eq!(body, response, ordered: true);
}