From 4667c9fe1a92858308a891ea2536059d76733e38 Mon Sep 17 00:00:00 2001 From: Irevoire Date: Wed, 25 May 2022 19:40:43 +0200 Subject: [PATCH] fix(http): Fix the query parameter in the Documents route --- .../tests/documents/get_documents.rs | 90 ++++++++++++++++++- meilisearch-lib/src/index/index.rs | 42 +++++---- 2 files changed, 106 insertions(+), 26 deletions(-) diff --git a/meilisearch-http/tests/documents/get_documents.rs b/meilisearch-http/tests/documents/get_documents.rs index cad656088..83e433b22 100644 --- a/meilisearch-http/tests/documents/get_documents.rs +++ b/meilisearch-http/tests/documents/get_documents.rs @@ -1,5 +1,4 @@ -use crate::common::Server; -use crate::common::{GetAllDocumentsOptions, GetDocumentOptions}; +use crate::common::{GetAllDocumentsOptions, GetDocumentOptions, Server}; use serde_json::json; @@ -71,7 +70,6 @@ async fn get_document() { }) ); - /* This currently doesn't work but should be fixed by #2433 let (response, code) = index .get_document( 0, @@ -87,7 +85,6 @@ async fn get_document() { "nested": { "content": "foobar" }, }) ); - */ } #[actix_rt::test] @@ -289,6 +286,91 @@ async fn test_get_all_documents_attributes_to_retrieve() { } } +#[actix_rt::test] +async fn get_document_s_nested_attributes_to_retrieve() { + let server = Server::new().await; + let index = server.index("test"); + index.create(None).await; + let documents = json!([ + { + "id": 0, + "content.truc": "foobar", + }, + { + "id": 1, + "content": { + "truc": "foobar", + "machin": "bidule", + }, + }, + ]); + let (_, code) = index.add_documents(documents, None).await; + assert_eq!(code, 202); + index.wait_task(0).await; + + let (response, code) = index + .get_document( + 0, + Some(GetDocumentOptions { + fields: Some(vec!["content"]), + }), + ) + .await; + assert_eq!(code, 200); + assert_eq!(response, json!({})); + let (response, code) = index + .get_document( + 1, + Some(GetDocumentOptions { + fields: Some(vec!["content"]), + }), + ) + .await; + assert_eq!(code, 200); + assert_eq!( + response, + json!({ + "content": { + "truc": "foobar", + "machin": "bidule", + }, + }) + ); + + let (response, code) = index + .get_document( + 0, + Some(GetDocumentOptions { + fields: Some(vec!["content.truc"]), + }), + ) + .await; + assert_eq!(code, 200); + assert_eq!( + response, + json!({ + "content.truc": "foobar", + }) + ); + let (response, code) = index + .get_document( + 1, + Some(GetDocumentOptions { + fields: Some(vec!["content.truc"]), + }), + ) + .await; + assert_eq!(code, 200); + assert_eq!( + response, + json!({ + "content": { + "truc": "foobar", + }, + }) + ); +} + #[actix_rt::test] async fn get_documents_displayed_attributes_is_ignored() { let server = Server::new().await; diff --git a/meilisearch-lib/src/index/index.rs b/meilisearch-lib/src/index/index.rs index bcf94bb0c..9c6150cfb 100644 --- a/meilisearch-lib/src/index/index.rs +++ b/meilisearch-lib/src/index/index.rs @@ -8,7 +8,7 @@ use std::sync::Arc; use fst::IntoStreamer; use milli::heed::{EnvOpenOptions, RoTxn}; use milli::update::{IndexerConfig, Setting}; -use milli::{obkv_to_json, FieldDistribution, FieldId}; +use milli::{obkv_to_json, FieldDistribution}; use serde::{Deserialize, Serialize}; use serde_json::{Map, Value}; use time::OffsetDateTime; @@ -228,7 +228,7 @@ impl Index { let txn = self.read_txn()?; let fields_ids_map = self.fields_ids_map(&txn)?; - let fields_to_display = self.fields_to_display(&attributes_to_retrieve, &fields_ids_map)?; + let all_fields: Vec<_> = fields_ids_map.iter().map(|(id, _)| id).collect(); let iter = self.documents.range(&txn, &(..))?.skip(offset).take(limit); @@ -236,8 +236,15 @@ impl Index { for entry in iter { let (_id, obkv) = entry?; - let object = obkv_to_json(&fields_to_display, &fields_ids_map, obkv)?; - documents.push(object); + let document = obkv_to_json(&all_fields, &fields_ids_map, obkv)?; + let document = match &attributes_to_retrieve { + Some(attributes_to_retrieve) => permissive_json_pointer::select_values( + &document, + attributes_to_retrieve.iter().map(|s| s.as_ref()), + ), + None => document, + }; + documents.push(document); } let number_of_documents = self.number_of_documents(&txn)?; @@ -253,7 +260,7 @@ impl Index { let txn = self.read_txn()?; let fields_ids_map = self.fields_ids_map(&txn)?; - let fields_to_display = self.fields_to_display(&attributes_to_retrieve, &fields_ids_map)?; + let all_fields: Vec<_> = fields_ids_map.iter().map(|(id, _)| id).collect(); let internal_id = self .external_documents_ids(&txn)? @@ -267,7 +274,14 @@ impl Index { .map(|(_, d)| d) .ok_or(IndexError::DocumentNotFound(doc_id))?; - let document = obkv_to_json(&fields_to_display, &fields_ids_map, document)?; + let document = obkv_to_json(&all_fields, &fields_ids_map, document)?; + let document = match &attributes_to_retrieve { + Some(attributes_to_retrieve) => permissive_json_pointer::select_values( + &document, + attributes_to_retrieve.iter().map(|s| s.as_ref()), + ), + None => document, + }; Ok(document) } @@ -276,22 +290,6 @@ impl Index { self.env.size() } - fn fields_to_display>( - &self, - attributes_to_retrieve: &Option>, - fields_ids_map: &milli::FieldsIdsMap, - ) -> Result> { - let attributes_to_retrieve_ids = match attributes_to_retrieve { - Some(attrs) => attrs - .iter() - .filter_map(|f| fields_ids_map.id(f.as_ref())) - .collect(), - None => fields_ids_map.iter().map(|(id, _)| id).collect(), - }; - - Ok(attributes_to_retrieve_ids) - } - pub fn snapshot(&self, path: impl AsRef) -> Result<()> { let mut dst = path.as_ref().join(format!("indexes/{}/", self.uuid)); create_dir_all(&dst)?;