From a3b9539ab71148b09eef302315af2e127e7a1a69 Mon Sep 17 00:00:00 2001 From: Benjamin Falk Date: Sat, 21 Jan 2023 16:20:37 -0500 Subject: [PATCH] Fix `empty bool` Malformed Query Bug (#37) Empty `bool` query building bug is fixed. When attempting to build a search if you apply an empty criteria set, such as an `AllMatch` it would generate a malformed query where the `bool` node had an empty filter. Now criterion are checked before being applied to a search to prevent this going forward. --- CHANGELOG.md | 10 ++++++ Cargo.toml | 2 +- README.md | 2 +- examples/sample_code.rs | 1 + src/request/search/criterion.rs | 11 +++++++ src/request/search/criterion/all_match.rs | 6 ++++ src/request/search/criterion/any_match.rs | 4 +++ src/request/search/criterion/builder_trait.rs | 5 ++- src/request/search/criterion/not_all.rs | 6 ++++ tests/search_tests.rs | 33 +++++++++++++++++++ 10 files changed, 77 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c2a570a..e42ebe2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,16 @@ All notable changes to this project will be documented in this file. +## [0.1.7] - 2023-01-2021 + +### Fixed + +- Empty `bool` query building bug is fixed. When attempting to build + a search if you apply an empty criteria set, such as an `AllMatch` + it would generate a malformed query where the `bool` node had an + empty filter. Now criterion are checked before being applied to a + search to prevent this going forward. + ## [0.1.6] - 2023-01-20 ### Added diff --git a/Cargo.toml b/Cargo.toml index 22be6be..276f869 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "elastic_lens" -version = "0.1.6" +version = "0.1.7" edition = "2021" authors = [ "Ben Falk " diff --git a/README.md b/README.md index cc264d3..9a1d926 100644 --- a/README.md +++ b/README.md @@ -25,7 +25,7 @@ In your `Cargo.toml` file: # You must pick one of the currently two supported adapters # - "official_es7" # - "official_es8" -elastic_lens = { version = "0.1.6", features = ["official_es7"] } +elastic_lens = { version = "0.1.7", features = ["official_es7"] } tokio = { version = "1", features = ["full"] } serde = { version = "1.0", features = ["derive"] } ``` diff --git a/examples/sample_code.rs b/examples/sample_code.rs index 468d5d2..dbc1d40 100644 --- a/examples/sample_code.rs +++ b/examples/sample_code.rs @@ -252,6 +252,7 @@ pub mod without_clone_or_debug { #[derive(Deserialize)] pub struct User { + #[allow(dead_code)] name: String, } diff --git a/src/request/search/criterion.rs b/src/request/search/criterion.rs index 8fea06a..ec62784 100644 --- a/src/request/search/criterion.rs +++ b/src/request/search/criterion.rs @@ -52,6 +52,17 @@ pub enum Criterion { AllMatch(AllMatch), } +impl Criterion { + pub(crate) fn is_usable(&self) -> bool { + match self { + Self::AllMatch(all) => all.has_data(), + Self::NotAll(not_all) => not_all.has_data(), + Self::AnyMatch(any) => any.has_data(), + _ => true, + } + } +} + impl Serialize for Criterion { fn serialize(&self, serializer: S) -> Result where diff --git a/src/request/search/criterion/all_match.rs b/src/request/search/criterion/all_match.rs index 2a29449..e0a9d85 100644 --- a/src/request/search/criterion/all_match.rs +++ b/src/request/search/criterion/all_match.rs @@ -15,6 +15,12 @@ pub struct AllMatch { negative_criteria: Vec, } +impl AllMatch { + pub(crate) fn has_data(&self) -> bool { + !(self.negative_criteria.is_empty() && self.positive_criteria.is_empty()) + } +} + /// Selects if all criteria given select pub fn if_all_match(mut func: F) -> AllMatch where diff --git a/src/request/search/criterion/any_match.rs b/src/request/search/criterion/any_match.rs index 157dd87..83dfa8f 100644 --- a/src/request/search/criterion/any_match.rs +++ b/src/request/search/criterion/any_match.rs @@ -31,6 +31,10 @@ impl AnyMatch { pub fn add>(&mut self, criterion: C) { self.criteria.push(criterion.into()); } + + pub(crate) fn has_data(&self) -> bool { + !self.criteria.is_empty() + } } impl Serialize for AnyMatch { diff --git a/src/request/search/criterion/builder_trait.rs b/src/request/search/criterion/builder_trait.rs index 2c11849..faf4788 100644 --- a/src/request/search/criterion/builder_trait.rs +++ b/src/request/search/criterion/builder_trait.rs @@ -23,6 +23,9 @@ pub trait CriteriaBuilder: Sized { /// Having the provided condition fn with>(&mut self, condition: SC) { let condition = condition.into(); - ::push(self, condition.criterion, condition.tag); + + if condition.criterion.is_usable() { + ::push(self, condition.criterion, condition.tag); + } } } diff --git a/src/request/search/criterion/not_all.rs b/src/request/search/criterion/not_all.rs index 18a8536..2fc52f0 100644 --- a/src/request/search/criterion/not_all.rs +++ b/src/request/search/criterion/not_all.rs @@ -10,6 +10,12 @@ pub struct NotAll { criteria: Vec, } +impl NotAll { + pub(crate) fn has_data(&self) -> bool { + !self.criteria.is_empty() + } +} + impl NotAll { /// Consumes a criterion and negates it pub fn single>(criterion: C) -> Self { diff --git a/tests/search_tests.rs b/tests/search_tests.rs index 925911e..8e443dd 100644 --- a/tests/search_tests.rs +++ b/tests/search_tests.rs @@ -638,3 +638,36 @@ fn building_a_search_with_a_script_sort_and_params() { }) ); } + +#[test] +fn building_a_search_with_an_empty_all_match() { + use elastic_lens::request::search::AllMatch; + + let mut search = Search::default(); + let all = AllMatch::default(); + search.with(all); + + assert_eq!(search_to_json(search), json!({})); +} + +#[test] +fn building_a_search_with_an_empty_any_match() { + use elastic_lens::request::search::AnyMatch; + + let mut search = Search::default(); + let any = AnyMatch::default(); + search.with(any); + + assert_eq!(search_to_json(search), json!({})); +} + +#[test] +fn building_a_search_with_an_empty_not_all_match() { + use elastic_lens::request::search::NotAll; + + let mut search = Search::default(); + let not_all = NotAll::default(); + search.with(not_all); + + assert_eq!(search_to_json(search), json!({})); +}