revert <FilterPathBasedFilter support dot in field name> and move it into a new PR elastic#83178

mushao999 · mushao999 · commit 37f19a517ecb · 2022-01-27T15:29:51.000+08:00
diff --git a/libs/x-content/src/main/java/org/elasticsearch/xcontent/XContentParserConfiguration.java b/libs/x-content/src/main/java/org/elasticsearch/xcontent/XContentParserConfiguration.java
@@ -32,38 +32,34 @@ public class XContentParserConfiguration {
         DeprecationHandler.THROW_UNSUPPORTED_OPERATION,
         RestApiVersion.current(),
         null,
-        null,
-        false
+        null
     );
 
     final NamedXContentRegistry registry;
     final DeprecationHandler deprecationHandler;
     final RestApiVersion restApiVersion;
     final FilterPath[] includes;
     final FilterPath[] excludes;
-    final boolean supportDotInFieldName;
 
     private XContentParserConfiguration(
         NamedXContentRegistry registry,
         DeprecationHandler deprecationHandler,
         RestApiVersion restApiVersion,
         FilterPath[] includes,
-        FilterPath[] excludes,
-        boolean supportDotInFieldName
+        FilterPath[] excludes
     ) {
         this.registry = registry;
         this.deprecationHandler = deprecationHandler;
         this.restApiVersion = restApiVersion;
         this.includes = includes;
         this.excludes = excludes;
-        this.supportDotInFieldName = supportDotInFieldName;
     }
 
     /**
      * Replace the registry backing {@link XContentParser#namedObject}.
      */
     public XContentParserConfiguration withRegistry(NamedXContentRegistry registry) {
-        return new XContentParserConfiguration(registry, deprecationHandler, restApiVersion, includes, excludes, supportDotInFieldName);
+        return new XContentParserConfiguration(registry, deprecationHandler, restApiVersion, includes, excludes);
     }
 
     public NamedXContentRegistry registry() {
@@ -75,7 +71,7 @@ public NamedXContentRegistry registry() {
      * a deprecated field.
      */
     public XContentParserConfiguration withDeprecationHandler(DeprecationHandler deprecationHandler) {
-        return new XContentParserConfiguration(registry, deprecationHandler, restApiVersion, includes, excludes, supportDotInFieldName);
+        return new XContentParserConfiguration(registry, deprecationHandler, restApiVersion, includes, excludes);
     }
 
     public DeprecationHandler deprecationHandler() {
@@ -87,7 +83,7 @@ public DeprecationHandler deprecationHandler() {
      * {@link RestApiVersion}.
      */
     public XContentParserConfiguration withRestApiVersion(RestApiVersion restApiVersion) {
-        return new XContentParserConfiguration(registry, deprecationHandler, restApiVersion, includes, excludes, supportDotInFieldName);
+        return new XContentParserConfiguration(registry, deprecationHandler, restApiVersion, includes, excludes);
     }
 
     public RestApiVersion restApiVersion() {
@@ -103,15 +99,10 @@ public XContentParserConfiguration withFiltering(Set<String> includeStrings, Set
             deprecationHandler,
             restApiVersion,
             FilterPath.compile(includeStrings),
-            FilterPath.compile(excludeStrings),
-            supportDotInFieldName
+            FilterPath.compile(excludeStrings)
         );
     }
 
-    public XContentParserConfiguration withSupportDotInFieldName(boolean supportDotInFieldName) {
-        return new XContentParserConfiguration(registry, deprecationHandler, restApiVersion, includes, excludes, supportDotInFieldName);
-    }
-
     public JsonParser filter(JsonParser parser) {
         JsonParser filtered = parser;
         if (excludes != null) {
@@ -121,20 +112,10 @@ public JsonParser filter(JsonParser parser) {
                     throw new UnsupportedOperationException("double wildcards are not supported in filtered excludes");
                 }
             }
-            filtered = new FilteringParserDelegate(
-                filtered,
-                new FilterPathBasedFilter(excludes, false).supportDotInFieldName(supportDotInFieldName),
-                true,
-                true
-            );
+            filtered = new FilteringParserDelegate(filtered, new FilterPathBasedFilter(excludes, false), true, true);
         }
         if (includes != null) {
-            filtered = new FilteringParserDelegate(
-                filtered,
-                new FilterPathBasedFilter(includes, true).supportDotInFieldName(supportDotInFieldName),
-                true,
-                true
-            );
+            filtered = new FilteringParserDelegate(filtered, new FilterPathBasedFilter(includes, true), true, true);
         }
         return filtered;
     }
diff --git a/libs/x-content/src/main/java/org/elasticsearch/xcontent/support/filtering/FilterPath.java b/libs/x-content/src/main/java/org/elasticsearch/xcontent/support/filtering/FilterPath.java
@@ -71,17 +71,6 @@ private boolean isFinalNode() {
      * @return true if the name equal a final node, otherwise return false
      */
     boolean matches(String name, List<FilterPath> nextFilters) {
-        return matches(name, nextFilters, false);
-    }
-
-    /**
-     * Similar to {@link #matches(String, List)}, but can set whether support dot in field name or not
-     * @param name the xcontent property name
-     * @param nextFilters nextFilters is a List, used to check the inner property of name
-     * @param supportDotInFieldName support dot in field name or not
-     * @return true if the name equal a final node, otherwise return false
-     */
-    boolean matches(String name, List<FilterPath> nextFilters, boolean supportDotInFieldName) {
         if (nextFilters == null) {
             return false;
         }
@@ -110,23 +99,6 @@ boolean matches(String name, List<FilterPath> nextFilters, boolean supportDotInF
             nextFilters.add(this);
         }
 
-        // support dot in path
-        if (supportDotInFieldName && nextFilters.isEmpty()) {
-            // contains dot and not the first or last char
-            int dotIndex = name.indexOf('.');
-            if ((dotIndex != -1) && (dotIndex != 0) && (dotIndex != name.length() - 1)) {
-                String prefixName = name.substring(0, dotIndex);
-                String subName = name.substring(dotIndex + 1);
-                termNode = termsChildren.get(prefixName);
-                if (termNode != null) {
-                    if (termNode.isFinalNode) {
-                        return true;
-                    } else {
-                        return termNode.matches(subName, nextFilters, true);
-                    }
-                }
-            }
-        }
         return false;
     }
 
diff --git a/libs/x-content/src/main/java/org/elasticsearch/xcontent/support/filtering/FilterPathBasedFilter.java b/libs/x-content/src/main/java/org/elasticsearch/xcontent/support/filtering/FilterPathBasedFilter.java
@@ -42,13 +42,6 @@ public String toString() {
 
     private final boolean inclusive;
 
-    private boolean supportDotInFieldName = false;
-
-    public FilterPathBasedFilter supportDotInFieldName(boolean supportDotInFieldName) {
-        this.supportDotInFieldName = supportDotInFieldName;
-        return this;
-    }
-
     public FilterPathBasedFilter(FilterPath[] filters, boolean inclusive) {
         if (filters == null || filters.length == 0) {
             throw new IllegalArgumentException("filters cannot be null or empty");
@@ -68,16 +61,14 @@ private TokenFilter evaluate(String name, FilterPath[] filterPaths) {
         if (filterPaths != null) {
             List<FilterPath> nextFilters = new ArrayList<>();
             for (FilterPath filter : filterPaths) {
-                boolean matches = filter.matches(name, nextFilters, supportDotInFieldName);
+                boolean matches = filter.matches(name, nextFilters);
                 if (matches) {
                     return MATCHING;
                 }
             }
 
             if (nextFilters.isEmpty() == false) {
-                return new FilterPathBasedFilter(nextFilters.toArray(new FilterPath[nextFilters.size()]), inclusive).supportDotInFieldName(
-                    supportDotInFieldName
-                );
+                return new FilterPathBasedFilter(nextFilters.toArray(new FilterPath[nextFilters.size()]), inclusive);
             }
         }
         return NO_MATCHING;
diff --git a/libs/x-content/src/test/java/org/elasticsearch/xcontent/support/filtering/FilterPathTests.java b/libs/x-content/src/test/java/org/elasticsearch/xcontent/support/filtering/FilterPathTests.java
@@ -338,33 +338,4 @@ public void testNoMatchesFilter() {
         assertFalse(filterPath.matches(randomAlphaOfLength(10), nextFilters));
         assertEquals(nextFilters.size(), 0);
     }
-
-    public void testDotInFieldName() {
-        // FilterPath match
-        FilterPath[] filterPaths = FilterPath.compile(singleton("foo"));
-        List<FilterPath> nextFilters = new ArrayList<>();
-        assertTrue(filterPaths[0].matches("foo.bar", nextFilters, true));
-        assertEquals(nextFilters.size(), 0);
-
-        // FilterPath not match
-        filterPaths = FilterPath.compile(singleton("bar"));
-        assertFalse(filterPaths[0].matches("foo.bar", nextFilters, true));
-        assertEquals(nextFilters.size(), 0);
-
-        // FilterPath equals to fieldName
-        filterPaths = FilterPath.compile(singleton("foo.bar"));
-        assertTrue(filterPaths[0].matches("foo.bar", nextFilters, true));
-        assertEquals(nextFilters.size(), 0);
-
-        // FilterPath longer than fieldName
-        filterPaths = FilterPath.compile(singleton("foo.bar.test"));
-        assertFalse(filterPaths[0].matches("foo.bar", nextFilters, true));
-        assertEquals(nextFilters.size(), 1);
-        nextFilters.clear();
-
-        // partial match
-        filterPaths = FilterPath.compile(singleton("foo.bar.test"));
-        assertFalse(filterPaths[0].matches("foo.bar.text", nextFilters, true));
-        assertEquals(nextFilters.size(), 0);
-    }
 }
diff --git a/server/src/main/java/org/elasticsearch/common/xcontent/XContentFieldFilter.java b/server/src/main/java/org/elasticsearch/common/xcontent/XContentFieldFilter.java
@@ -74,7 +74,7 @@ static XContentFieldFilter newFieldFilter(String[] includes, String[] excludes)
             final XContentParserConfiguration parserConfig = XContentParserConfiguration.EMPTY.withFiltering(
                 Set.of(includes),
                 Set.of(excludes)
-            ).withSupportDotInFieldName(true);
+            );
             return (originalSource, contentType) -> {
                 if (originalSource == null || originalSource.length() <= 0) {
                     if (contentType == null) {
diff --git a/server/src/test/java/org/elasticsearch/common/xcontent/support/XContentFiledFilterTests.java b/server/src/test/java/org/elasticsearch/common/xcontent/support/XContentFiledFilterTests.java
@@ -243,6 +243,7 @@ public void testIncludingObjectWithNestedIncludedObject() throws IOException {
         testFilter(actual, actual, singleton("*.obj2"), emptySet());
     }
 
+    @AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/pull/83178")
     public void testDotsInFieldNames() throws IOException {
         Builder actual = builder -> builder.startObject()
             .field("foo.bar", 2)

Original file line number	Diff line number	Diff line change
`@@ -243,6 +243,7 @@ public void testIncludingObjectWithNestedIncludedObject() throws IOException {`
`243`	`243`	`testFilter(actual, actual, singleton("*.obj2"), emptySet());`
`244`	`244`	`}`
`245`	`245`
	`246`	`+ @AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/pull/83178")`
`246`	`247`	`public void testDotsInFieldNames() throws IOException {`
`247`	`248`	`Builder actual = builder -> builder.startObject()`
`248`	`249`	`.field("foo.bar", 2)`