Skip to content

Commit

Permalink
Fix empty bool Malformed Query Bug (#37)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
benfalk authored Jan 21, 2023
1 parent 5c7d2ce commit a3b9539
Show file tree
Hide file tree
Showing 10 changed files with 77 additions and 3 deletions.
10 changes: 10 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "elastic_lens"
version = "0.1.6"
version = "0.1.7"
edition = "2021"
authors = [
"Ben Falk <benjamin.falk@yahoo.com>"
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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"] }
```
Expand Down
1 change: 1 addition & 0 deletions examples/sample_code.rs
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,7 @@ pub mod without_clone_or_debug {

#[derive(Deserialize)]
pub struct User {
#[allow(dead_code)]
name: String,
}

Expand Down
11 changes: 11 additions & 0 deletions src/request/search/criterion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
where
Expand Down
6 changes: 6 additions & 0 deletions src/request/search/criterion/all_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,12 @@ pub struct AllMatch {
negative_criteria: Vec<Criterion>,
}

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<F>(mut func: F) -> AllMatch
where
Expand Down
4 changes: 4 additions & 0 deletions src/request/search/criterion/any_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,10 @@ impl AnyMatch {
pub fn add<C: Into<Criterion>>(&mut self, criterion: C) {
self.criteria.push(criterion.into());
}

pub(crate) fn has_data(&self) -> bool {
!self.criteria.is_empty()
}
}

impl Serialize for AnyMatch {
Expand Down
5 changes: 4 additions & 1 deletion src/request/search/criterion/builder_trait.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ pub trait CriteriaBuilder: Sized {
/// Having the provided condition
fn with<SC: Into<SearchCondition>>(&mut self, condition: SC) {
let condition = condition.into();
<Self::Bucket as BucketPlacer>::push(self, condition.criterion, condition.tag);

if condition.criterion.is_usable() {
<Self::Bucket as BucketPlacer>::push(self, condition.criterion, condition.tag);
}
}
}
6 changes: 6 additions & 0 deletions src/request/search/criterion/not_all.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,12 @@ pub struct NotAll {
criteria: Vec<Criterion>,
}

impl NotAll {
pub(crate) fn has_data(&self) -> bool {
!self.criteria.is_empty()
}
}

impl NotAll {
/// Consumes a criterion and negates it
pub fn single<C: Into<Criterion>>(criterion: C) -> Self {
Expand Down
33 changes: 33 additions & 0 deletions tests/search_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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!({}));
}

0 comments on commit a3b9539

Please sign in to comment.