Skip to content

Commit 0742453

Browse files
deshsiddrobinf95
andauthored
Refactoring FilterPath.parse by using an iterative approach instead of recursion. (#14200)
* Refactor FilterPath parse function (#12067) Signed-off-by: Robin Friedmann <robinfriedmann.rf@gmail.com> * Implement unit tests for FilterPathTests (#12067) Signed-off-by: Robin Friedmann <robinfriedmann.rf@gmail.com> * Write warn log if Filter is empty; Add comments (#12067) Signed-off-by: Robin Friedmann <robinfriedmann.rf@gmail.com> * Add changelog Signed-off-by: Siddhant Deshmukh <deshsid@amazon.com> * Remove unnecessary log statement Signed-off-by: Siddhant Deshmukh <deshsid@amazon.com> * Remove unused logger Signed-off-by: Siddhant Deshmukh <deshsid@amazon.com> * Spotless apply Signed-off-by: Siddhant Deshmukh <deshsid@amazon.com> * Remove incorrect changelog Signed-off-by: Siddhant Deshmukh <deshsid@amazon.com> --------- Signed-off-by: Siddhant Deshmukh <deshsid@amazon.com> Co-authored-by: Robin Friedmann <robinfriedmann.rf@gmail.com>
1 parent f1f4f89 commit 0742453

File tree

3 files changed

+31
-17
lines changed

3 files changed

+31
-17
lines changed

CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
5454
- 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))
5555
- Fix file cache initialization ([#14004](https://github.com/opensearch-project/OpenSearch/pull/14004))
5656
- Handle NPE in GetResult if "found" field is missing ([#14552](https://github.com/opensearch-project/OpenSearch/pull/14552))
57+
- Refactoring FilterPath.parse by using an iterative approach ([#14200](https://github.com/opensearch-project/OpenSearch/pull/14200))
5758

5859
### Security
5960

libs/core/src/main/java/org/opensearch/core/xcontent/filtering/FilterPath.java

+13-17
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,6 @@
4646
public class FilterPath {
4747

4848
static final FilterPath EMPTY = new FilterPath();
49-
5049
private final String filter;
5150
private final String segment;
5251
private final FilterPath next;
@@ -99,32 +98,29 @@ public static FilterPath[] compile(Set<String> filters) {
9998

10099
List<FilterPath> paths = new ArrayList<>();
101100
for (String filter : filters) {
102-
if (filter != null) {
101+
if (filter != null && !filter.isEmpty()) {
103102
filter = filter.trim();
104103
if (filter.length() > 0) {
105-
paths.add(parse(filter, filter));
104+
paths.add(parse(filter));
106105
}
107106
}
108107
}
109108
return paths.toArray(new FilterPath[0]);
110109
}
111110

112-
private static FilterPath parse(final String filter, final String segment) {
113-
int end = segment.length();
114-
115-
for (int i = 0; i < end;) {
116-
char c = segment.charAt(i);
111+
private static FilterPath parse(final String filter) {
112+
// Split the filter into segments using a regex
113+
// that avoids splitting escaped dots.
114+
String[] segments = filter.split("(?<!\\\\)\\.");
115+
FilterPath next = EMPTY;
117116

118-
if (c == '.') {
119-
String current = segment.substring(0, i).replaceAll("\\\\.", ".");
120-
return new FilterPath(filter, current, parse(filter, segment.substring(i + 1)));
121-
}
122-
++i;
123-
if ((c == '\\') && (i < end) && (segment.charAt(i) == '.')) {
124-
++i;
125-
}
117+
for (int i = segments.length - 1; i >= 0; i--) {
118+
// Replace escaped dots with actual dots in the current segment.
119+
String segment = segments[i].replaceAll("\\\\.", ".");
120+
next = new FilterPath(filter, segment, next);
126121
}
127-
return new FilterPath(filter, segment.replaceAll("\\\\.", "."), EMPTY);
122+
123+
return next;
128124
}
129125

130126
@Override

libs/core/src/test/java/org/opensearch/core/xcontent/filtering/FilterPathTests.java

+17
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
import org.opensearch.common.util.set.Sets;
3636
import org.opensearch.test.OpenSearchTestCase;
3737

38+
import java.util.HashSet;
3839
import java.util.Set;
3940

4041
import static java.util.Collections.singleton;
@@ -369,4 +370,20 @@ public void testMultipleFilterPaths() {
369370
assertThat(filterPath.getSegment(), is(emptyString()));
370371
assertSame(filterPath, FilterPath.EMPTY);
371372
}
373+
374+
public void testCompileWithEmptyString() {
375+
Set<String> filters = new HashSet<>();
376+
filters.add("");
377+
FilterPath[] filterPaths = FilterPath.compile(filters);
378+
assertNotNull(filterPaths);
379+
assertEquals(0, filterPaths.length);
380+
}
381+
382+
public void testCompileWithNull() {
383+
Set<String> filters = new HashSet<>();
384+
filters.add(null);
385+
FilterPath[] filterPaths = FilterPath.compile(filters);
386+
assertNotNull(filterPaths);
387+
assertEquals(0, filterPaths.length);
388+
}
372389
}

0 commit comments

Comments
 (0)