From 3b240be549063b8adff7a2d3d0765b7565d04248 Mon Sep 17 00:00:00 2001 From: Siddhant Deshmukh Date: Tue, 2 Jul 2024 13:18:39 -0700 Subject: [PATCH] Refactoring FilterPath.parse by using an iterative approach instead of recursion. (#14200) * Refactor FilterPath parse function (#12067) Signed-off-by: Robin Friedmann * Implement unit tests for FilterPathTests (#12067) Signed-off-by: Robin Friedmann * Write warn log if Filter is empty; Add comments (#12067) Signed-off-by: Robin Friedmann * Add changelog Signed-off-by: Siddhant Deshmukh * Remove unnecessary log statement Signed-off-by: Siddhant Deshmukh * Remove unused logger Signed-off-by: Siddhant Deshmukh * Spotless apply Signed-off-by: Siddhant Deshmukh * Remove incorrect changelog Signed-off-by: Siddhant Deshmukh --------- Signed-off-by: Siddhant Deshmukh Co-authored-by: Robin Friedmann --- CHANGELOG.md | 1 + .../core/xcontent/filtering/FilterPath.java | 30 ++++++++----------- .../xcontent/filtering/FilterPathTests.java | 17 +++++++++++ 3 files changed, 31 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 425774aec6bb4..d259ceb2474f5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -72,6 +72,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Fix FuzzyQuery in keyword field will use IndexOrDocValuesQuery when both of index and doc_value are true ([#14378](https://github.com/opensearch-project/OpenSearch/pull/14378)) - Fix file cache initialization ([#14004](https://github.com/opensearch-project/OpenSearch/pull/14004)) - Handle NPE in GetResult if "found" field is missing ([#14552](https://github.com/opensearch-project/OpenSearch/pull/14552)) +- Refactoring FilterPath.parse by using an iterative approach ([#14200](https://github.com/opensearch-project/OpenSearch/pull/14200)) ### Security diff --git a/libs/core/src/main/java/org/opensearch/core/xcontent/filtering/FilterPath.java b/libs/core/src/main/java/org/opensearch/core/xcontent/filtering/FilterPath.java index 5389538a8c7dd..b8da9787165f8 100644 --- a/libs/core/src/main/java/org/opensearch/core/xcontent/filtering/FilterPath.java +++ b/libs/core/src/main/java/org/opensearch/core/xcontent/filtering/FilterPath.java @@ -46,7 +46,6 @@ public class FilterPath { static final FilterPath EMPTY = new FilterPath(); - private final String filter; private final String segment; private final FilterPath next; @@ -99,32 +98,29 @@ public static FilterPath[] compile(Set filters) { List paths = new ArrayList<>(); for (String filter : filters) { - if (filter != null) { + if (filter != null && !filter.isEmpty()) { filter = filter.trim(); if (filter.length() > 0) { - paths.add(parse(filter, filter)); + paths.add(parse(filter)); } } } return paths.toArray(new FilterPath[0]); } - private static FilterPath parse(final String filter, final String segment) { - int end = segment.length(); - - for (int i = 0; i < end;) { - char c = segment.charAt(i); + private static FilterPath parse(final String filter) { + // Split the filter into segments using a regex + // that avoids splitting escaped dots. + String[] segments = filter.split("(?= 0; i--) { + // Replace escaped dots with actual dots in the current segment. + String segment = segments[i].replaceAll("\\\\.", "."); + next = new FilterPath(filter, segment, next); } - return new FilterPath(filter, segment.replaceAll("\\\\.", "."), EMPTY); + + return next; } @Override diff --git a/libs/core/src/test/java/org/opensearch/core/xcontent/filtering/FilterPathTests.java b/libs/core/src/test/java/org/opensearch/core/xcontent/filtering/FilterPathTests.java index 0c5a17b70a956..d3191609f6119 100644 --- a/libs/core/src/test/java/org/opensearch/core/xcontent/filtering/FilterPathTests.java +++ b/libs/core/src/test/java/org/opensearch/core/xcontent/filtering/FilterPathTests.java @@ -35,6 +35,7 @@ import org.opensearch.common.util.set.Sets; import org.opensearch.test.OpenSearchTestCase; +import java.util.HashSet; import java.util.Set; import static java.util.Collections.singleton; @@ -369,4 +370,20 @@ public void testMultipleFilterPaths() { assertThat(filterPath.getSegment(), is(emptyString())); assertSame(filterPath, FilterPath.EMPTY); } + + public void testCompileWithEmptyString() { + Set filters = new HashSet<>(); + filters.add(""); + FilterPath[] filterPaths = FilterPath.compile(filters); + assertNotNull(filterPaths); + assertEquals(0, filterPaths.length); + } + + public void testCompileWithNull() { + Set filters = new HashSet<>(); + filters.add(null); + FilterPath[] filterPaths = FilterPath.compile(filters); + assertNotNull(filterPaths); + assertEquals(0, filterPaths.length); + } }