From 580bc40c0c87b5bdd35d05e250256063c94c1d56 Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Wed, 18 Mar 2020 14:15:30 +0000 Subject: [PATCH] Make it possible to deprecate all variants of a ParseField with no replacement (#53722) Sometimes we want to deprecate and remove a ParseField entirely, without replacement; for example, the various places where we specify a _type field in 7x. Currently we can tell users only that a particular field name should not be used, and that another name should be used in its place. This commit adds the ability to say that a field should not be used at all. --- .../client/RestHighLevelClient.java | 7 +-- .../client/ml/job/config/RuleScope.java | 11 +--- .../DeleteRoleMappingResponseTests.java | 10 +--- .../security/ExpressionRoleMappingTests.java | 34 ++++------- .../security/GetPrivilegesResponseTests.java | 10 +--- .../GetRoleMappingsResponseTests.java | 60 ++++++++----------- .../security/GetRolesResponseTests.java | 10 +--- .../RoleMapperExpressionParserTests.java | 10 +--- .../privileges/ApplicationPrivilegeTests.java | 10 +--- .../org/elasticsearch/common/ParseField.java | 16 ++++- .../common/xcontent/DeprecationHandler.java | 35 ++++++++++- .../elasticsearch/common/ParseFieldTests.java | 15 +++++ .../xcontent/LoggingDeprecationHandler.java | 7 ++- ...LoggingDeprecationAccumulationHandler.java | 7 +++ .../xpack/security/authc/ApiKeyService.java | 6 ++ 15 files changed, 128 insertions(+), 120 deletions(-) diff --git a/client/rest-high-level/src/main/java/org/elasticsearch/client/RestHighLevelClient.java b/client/rest-high-level/src/main/java/org/elasticsearch/client/RestHighLevelClient.java index b464c2166f8..e1ca36f5528 100644 --- a/client/rest-high-level/src/main/java/org/elasticsearch/client/RestHighLevelClient.java +++ b/client/rest-high-level/src/main/java/org/elasticsearch/client/RestHighLevelClient.java @@ -1888,12 +1888,7 @@ public class RestHighLevelClient implements Closeable { * emitted there just mean that you are talking to an old version of * Elasticsearch. There isn't anything you can do about the deprecation. */ - private static final DeprecationHandler DEPRECATION_HANDLER = new DeprecationHandler() { - @Override - public void usedDeprecatedName(String usedName, String modernName) {} - @Override - public void usedDeprecatedField(String usedName, String replacedWith) {} - }; + private static final DeprecationHandler DEPRECATION_HANDLER = DeprecationHandler.IGNORE_DEPRECATIONS; static List getDefaultNamedXContents() { Map> map = new HashMap<>(); diff --git a/client/rest-high-level/src/main/java/org/elasticsearch/client/ml/job/config/RuleScope.java b/client/rest-high-level/src/main/java/org/elasticsearch/client/ml/job/config/RuleScope.java index 8b6886d5825..95e727b818a 100644 --- a/client/rest-high-level/src/main/java/org/elasticsearch/client/ml/job/config/RuleScope.java +++ b/client/rest-high-level/src/main/java/org/elasticsearch/client/ml/job/config/RuleScope.java @@ -50,7 +50,7 @@ public class RuleScope implements ToXContentObject { Map value = (Map) entry.getValue(); builder.map(value); try (XContentParser scopeParser = XContentFactory.xContent(builder.contentType()).createParser( - NamedXContentRegistry.EMPTY, DEPRECATION_HANDLER, Strings.toString(builder))) { + NamedXContentRegistry.EMPTY, DeprecationHandler.IGNORE_DEPRECATIONS, Strings.toString(builder))) { scope.put(entry.getKey(), FilterRef.PARSER.parse(scopeParser, null)); } } @@ -59,15 +59,6 @@ public class RuleScope implements ToXContentObject { }; } - private static final DeprecationHandler DEPRECATION_HANDLER = new DeprecationHandler() { - - @Override - public void usedDeprecatedName(String usedName, String modernName) {} - - @Override - public void usedDeprecatedField(String usedName, String replacedWith) {} - }; - private final Map scope; public RuleScope() { diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/security/DeleteRoleMappingResponseTests.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/security/DeleteRoleMappingResponseTests.java index d89deb44e9f..1eee41bc236 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/security/DeleteRoleMappingResponseTests.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/security/DeleteRoleMappingResponseTests.java @@ -35,15 +35,7 @@ public class DeleteRoleMappingResponseTests extends ESTestCase { public void testFromXContent() throws IOException { final String json = "{ \"found\" : \"true\" }"; final DeleteRoleMappingResponse response = DeleteRoleMappingResponse.fromXContent(XContentType.JSON.xContent().createParser( - new NamedXContentRegistry(Collections.emptyList()), new DeprecationHandler() { - @Override - public void usedDeprecatedName(String usedName, String modernName) { - } - - @Override - public void usedDeprecatedField(String usedName, String replacedWith) { - } - }, json)); + new NamedXContentRegistry(Collections.emptyList()), DeprecationHandler.IGNORE_DEPRECATIONS, json)); final DeleteRoleMappingResponse expectedResponse = new DeleteRoleMappingResponse(true); assertThat(response, equalTo(expectedResponse)); } diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/security/ExpressionRoleMappingTests.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/security/ExpressionRoleMappingTests.java index f30307ebde5..3dd9d3ca5ee 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/security/ExpressionRoleMappingTests.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/security/ExpressionRoleMappingTests.java @@ -37,29 +37,21 @@ import static org.hamcrest.Matchers.equalTo; public class ExpressionRoleMappingTests extends ESTestCase { public void testExpressionRoleMappingParser() throws IOException { - final String json = - "{\n" + - " \"enabled\" : true,\n" + - " \"roles\" : [\n" + - " \"superuser\"\n" + - " ],\n" + - " \"rules\" : {\n" + - " \"field\" : {\n" + - " \"realm.name\" : \"kerb1\"\n" + - " }\n" + - " },\n" + - " \"metadata\" : { }\n" + + final String json = + "{\n" + + " \"enabled\" : true,\n" + + " \"roles\" : [\n" + + " \"superuser\"\n" + + " ],\n" + + " \"rules\" : {\n" + + " \"field\" : {\n" + + " \"realm.name\" : \"kerb1\"\n" + + " }\n" + + " },\n" + + " \"metadata\" : { }\n" + " }"; final ExpressionRoleMapping expressionRoleMapping = ExpressionRoleMapping.PARSER.parse(XContentType.JSON.xContent().createParser( - new NamedXContentRegistry(Collections.emptyList()), new DeprecationHandler() { - @Override - public void usedDeprecatedName(String usedName, String modernName) { - } - - @Override - public void usedDeprecatedField(String usedName, String replacedWith) { - } - }, json), "example-role-mapping"); + new NamedXContentRegistry(Collections.emptyList()), DeprecationHandler.IGNORE_DEPRECATIONS, json), "example-role-mapping"); final ExpressionRoleMapping expectedRoleMapping = new ExpressionRoleMapping("example-role-mapping", FieldRoleMapperExpression.ofKeyValues("realm.name", "kerb1"), singletonList("superuser"), Collections.emptyList(), diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/security/GetPrivilegesResponseTests.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/security/GetPrivilegesResponseTests.java index 74211892a09..b789d08b4b9 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/security/GetPrivilegesResponseTests.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/security/GetPrivilegesResponseTests.java @@ -80,15 +80,7 @@ public class GetPrivilegesResponseTests extends ESTestCase { "}"; final GetPrivilegesResponse response = GetPrivilegesResponse.fromXContent(XContentType.JSON.xContent().createParser( - new NamedXContentRegistry(Collections.emptyList()), new DeprecationHandler() { - @Override - public void usedDeprecatedName(String usedName, String modernName) { - } - - @Override - public void usedDeprecatedField(String usedName, String replacedWith) { - } - }, json)); + new NamedXContentRegistry(Collections.emptyList()), DeprecationHandler.IGNORE_DEPRECATIONS, json)); final ApplicationPrivilege readTestappPrivilege = new ApplicationPrivilege("testapp", "read", Arrays.asList("action:login", "data:read/*"), null); diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/security/GetRoleMappingsResponseTests.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/security/GetRoleMappingsResponseTests.java index 20883b859f9..4fda57f2ff6 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/security/GetRoleMappingsResponseTests.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/security/GetRoleMappingsResponseTests.java @@ -36,42 +36,34 @@ import static org.hamcrest.Matchers.equalTo; public class GetRoleMappingsResponseTests extends ESTestCase { public void testFromXContent() throws IOException { - final String json = "{\n" + - " \"kerberosmapping\" : {\n" + - " \"enabled\" : true,\n" + - " \"roles\" : [\n" + - " \"superuser\"\n" + - " ],\n" + - " \"rules\" : {\n" + - " \"field\" : {\n" + - " \"realm.name\" : \"kerb1\"\n" + - " }\n" + - " },\n" + - " \"metadata\" : { }\n" + - " },\n" + - " \"ldapmapping\" : {\n" + - " \"enabled\" : false,\n" + - " \"roles\" : [\n" + - " \"monitoring\"\n" + - " ],\n" + - " \"rules\" : {\n" + - " \"field\" : {\n" + - " \"groups\" : \"cn=ipausers,cn=groups,cn=accounts,dc=ipademo,dc=local\"\n" + - " }\n" + - " },\n" + - " \"metadata\" : { }\n" + - " }\n" + + final String json = "{\n" + + " \"kerberosmapping\" : {\n" + + " \"enabled\" : true,\n" + + " \"roles\" : [\n" + + " \"superuser\"\n" + + " ],\n" + + " \"rules\" : {\n" + + " \"field\" : {\n" + + " \"realm.name\" : \"kerb1\"\n" + + " }\n" + + " },\n" + + " \"metadata\" : { }\n" + + " },\n" + + " \"ldapmapping\" : {\n" + + " \"enabled\" : false,\n" + + " \"roles\" : [\n" + + " \"monitoring\"\n" + + " ],\n" + + " \"rules\" : {\n" + + " \"field\" : {\n" + + " \"groups\" : \"cn=ipausers,cn=groups,cn=accounts,dc=ipademo,dc=local\"\n" + + " }\n" + + " },\n" + + " \"metadata\" : { }\n" + + " }\n" + "}"; final GetRoleMappingsResponse response = GetRoleMappingsResponse.fromXContent(XContentType.JSON.xContent().createParser( - new NamedXContentRegistry(Collections.emptyList()), new DeprecationHandler() { - @Override - public void usedDeprecatedName(String usedName, String modernName) { - } - - @Override - public void usedDeprecatedField(String usedName, String replacedWith) { - } - }, json)); + new NamedXContentRegistry(Collections.emptyList()), DeprecationHandler.IGNORE_DEPRECATIONS, json)); final List expectedRoleMappingsList = new ArrayList<>(); expectedRoleMappingsList.add(new ExpressionRoleMapping("kerberosmapping", FieldRoleMapperExpression.ofKeyValues("realm.name", "kerb1"), Collections.singletonList("superuser"), Collections.emptyList(), null, true)); diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/security/GetRolesResponseTests.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/security/GetRolesResponseTests.java index c4620fa1a2f..8be4320e4de 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/security/GetRolesResponseTests.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/security/GetRolesResponseTests.java @@ -64,15 +64,7 @@ public class GetRolesResponseTests extends ESTestCase { " }\n" + "}"; final GetRolesResponse response = GetRolesResponse.fromXContent((XContentType.JSON.xContent().createParser( - new NamedXContentRegistry(Collections.emptyList()), new DeprecationHandler() { - @Override - public void usedDeprecatedName(String usedName, String modernName) { - } - - @Override - public void usedDeprecatedField(String usedName, String replacedWith) { - } - }, json))); + new NamedXContentRegistry(Collections.emptyList()), DeprecationHandler.IGNORE_DEPRECATIONS, json))); assertThat(response.getRoles().size(), equalTo(1)); assertThat(response.getTransientMetadataMap().size(), equalTo(1)); final Role role = response.getRoles().get(0); diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/security/support/expressiondsl/parser/RoleMapperExpressionParserTests.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/security/support/expressiondsl/parser/RoleMapperExpressionParserTests.java index 24ed5684fa8..1c9ccaa2f74 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/security/support/expressiondsl/parser/RoleMapperExpressionParserTests.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/security/support/expressiondsl/parser/RoleMapperExpressionParserTests.java @@ -115,15 +115,7 @@ public class RoleMapperExpressionParserTests extends ESTestCase { private RoleMapperExpression parse(String json) throws IOException { return new RoleMapperExpressionParser().parse("rules", XContentType.JSON.xContent().createParser(new NamedXContentRegistry( - Collections.emptyList()), new DeprecationHandler() { - @Override - public void usedDeprecatedName(String usedName, String modernName) { - } - - @Override - public void usedDeprecatedField(String usedName, String replacedWith) { - } - }, json)); + Collections.emptyList()), DeprecationHandler.IGNORE_DEPRECATIONS, json)); } } diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/security/user/privileges/ApplicationPrivilegeTests.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/security/user/privileges/ApplicationPrivilegeTests.java index b7201876730..532ff612fed 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/security/user/privileges/ApplicationPrivilegeTests.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/security/user/privileges/ApplicationPrivilegeTests.java @@ -54,15 +54,7 @@ public class ApplicationPrivilegeTests extends ESTestCase { + " }\n" + "}"; final ApplicationPrivilege privilege = ApplicationPrivilege.fromXContent(XContentType.JSON.xContent().createParser( - new NamedXContentRegistry(Collections.emptyList()), new DeprecationHandler() { - @Override - public void usedDeprecatedName(String usedName, String modernName) { - } - - @Override - public void usedDeprecatedField(String usedName, String replacedWith) { - } - }, json)); + new NamedXContentRegistry(Collections.emptyList()), DeprecationHandler.IGNORE_DEPRECATIONS, json)); final Map metadata = new HashMap<>(); metadata.put("description", "Read access to myapp"); final ApplicationPrivilege expectedPrivilege = diff --git a/libs/x-content/src/main/java/org/elasticsearch/common/ParseField.java b/libs/x-content/src/main/java/org/elasticsearch/common/ParseField.java index 084d82372c0..f4f12ed4f4c 100644 --- a/libs/x-content/src/main/java/org/elasticsearch/common/ParseField.java +++ b/libs/x-content/src/main/java/org/elasticsearch/common/ParseField.java @@ -34,6 +34,7 @@ public class ParseField { private final String[] deprecatedNames; private String allReplacedWith = null; private final String[] allNames; + private boolean fullyDeprecated = false; private static final String[] EMPTY = new String[0]; @@ -96,6 +97,15 @@ public class ParseField { return parseField; } + /** + * Return a new ParseField where all field names are deprecated with no replacement + */ + public ParseField withAllDeprecated() { + ParseField parseField = this.withDeprecation(getAllNamesIncludedDeprecated()); + parseField.fullyDeprecated = true; + return parseField; + } + /** * Does {@code fieldName} match this field? * @param fieldName @@ -108,7 +118,7 @@ public class ParseField { Objects.requireNonNull(fieldName, "fieldName cannot be null"); // if this parse field has not been completely deprecated then try to // match the preferred name - if (allReplacedWith == null && fieldName.equals(name)) { + if (fullyDeprecated == false && allReplacedWith == null && fieldName.equals(name)) { return true; } // Now try to match against one of the deprecated names. Note that if @@ -116,7 +126,9 @@ public class ParseField { // fields will be in the deprecatedNames array for (String depName : deprecatedNames) { if (fieldName.equals(depName)) { - if (allReplacedWith == null) { + if (fullyDeprecated) { + deprecationHandler.usedDeprecatedField(fieldName); + } else if (allReplacedWith == null) { deprecationHandler.usedDeprecatedName(fieldName, name); } else { deprecationHandler.usedDeprecatedField(fieldName, allReplacedWith); diff --git a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/DeprecationHandler.java b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/DeprecationHandler.java index 1b0dcf45680..2cfda82eb4c 100644 --- a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/DeprecationHandler.java +++ b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/DeprecationHandler.java @@ -41,6 +41,32 @@ public interface DeprecationHandler { throw new UnsupportedOperationException("deprecated fields not supported here but got [" + usedName + "] which has been replaced with [" + modernName + "]"); } + + @Override + public void usedDeprecatedField(String usedName) { + throw new UnsupportedOperationException("deprecated fields not supported here but got [" + + usedName + "] which has been deprecated entirely"); + } + }; + + /** + * Ignores all deprecations + */ + DeprecationHandler IGNORE_DEPRECATIONS = new DeprecationHandler() { + @Override + public void usedDeprecatedName(String usedName, String modernName) { + + } + + @Override + public void usedDeprecatedField(String usedName, String replacedWith) { + + } + + @Override + public void usedDeprecatedField(String usedName) { + + } }; /** @@ -52,9 +78,16 @@ public interface DeprecationHandler { /** * Called when the provided field name matches the current field but the entire - * field has been marked as deprecated. + * field has been marked as deprecated and another field should be used * @param usedName the provided field name * @param replacedWith the name of the field that replaced this field */ void usedDeprecatedField(String usedName, String replacedWith); + + /** + * Called when the provided field name matches the current field but the entire + * field has been marked as deprecated with no replacement + * @param usedName the provided field name + */ + void usedDeprecatedField(String usedName); } diff --git a/libs/x-content/src/test/java/org/elasticsearch/common/ParseFieldTests.java b/libs/x-content/src/test/java/org/elasticsearch/common/ParseFieldTests.java index 72ba5578a49..381c7c3e959 100644 --- a/libs/x-content/src/test/java/org/elasticsearch/common/ParseFieldTests.java +++ b/libs/x-content/src/test/java/org/elasticsearch/common/ParseFieldTests.java @@ -60,6 +60,21 @@ public class ParseFieldTests extends ESTestCase { assertWarnings("Deprecated field [like_text] used, replaced by [like]"); } + public void testDeprecatedWithNoReplacement() { + String name = "dep"; + String[] alternatives = new String[]{"old_dep", "new_dep"}; + ParseField field = new ParseField(name).withDeprecation(alternatives).withAllDeprecated(); + assertFalse(field.match("not a field name", LoggingDeprecationHandler.INSTANCE)); + assertTrue(field.match("dep", LoggingDeprecationHandler.INSTANCE)); + assertWarnings("Deprecated field [dep] used, which has been removed entirely"); + assertTrue(field.match("old_dep", LoggingDeprecationHandler.INSTANCE)); + assertWarnings("Deprecated field [old_dep] used, which has been removed entirely"); + assertTrue(field.match("new_dep", LoggingDeprecationHandler.INSTANCE)); + assertWarnings("Deprecated field [new_dep] used, which has been removed entirely"); + + + } + public void testGetAllNamesIncludedDeprecated() { ParseField parseField = new ParseField("terms", "in"); assertThat(parseField.getAllNamesIncludedDeprecated(), arrayContainingInAnyOrder("terms", "in")); diff --git a/server/src/main/java/org/elasticsearch/common/xcontent/LoggingDeprecationHandler.java b/server/src/main/java/org/elasticsearch/common/xcontent/LoggingDeprecationHandler.java index 097093ccce5..9a885b55d14 100644 --- a/server/src/main/java/org/elasticsearch/common/xcontent/LoggingDeprecationHandler.java +++ b/server/src/main/java/org/elasticsearch/common/xcontent/LoggingDeprecationHandler.java @@ -33,7 +33,7 @@ import org.elasticsearch.common.logging.DeprecationLogger; * though the user sent them. */ public class LoggingDeprecationHandler implements DeprecationHandler { - public static LoggingDeprecationHandler INSTANCE = new LoggingDeprecationHandler(); + public static final LoggingDeprecationHandler INSTANCE = new LoggingDeprecationHandler(); /** * The logger to which to send deprecation messages. * @@ -57,4 +57,9 @@ public class LoggingDeprecationHandler implements DeprecationHandler { public void usedDeprecatedField(String usedName, String replacedWith) { deprecationLogger.deprecated("Deprecated field [{}] used, replaced by [{}]", usedName, replacedWith); } + + @Override + public void usedDeprecatedField(String usedName) { + deprecationLogger.deprecated("Deprecated field [{}] used, which has been removed entirely", usedName); + } } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/utils/LoggingDeprecationAccumulationHandler.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/utils/LoggingDeprecationAccumulationHandler.java index 2322b3d9026..5e2a4d7a253 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/utils/LoggingDeprecationAccumulationHandler.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/utils/LoggingDeprecationAccumulationHandler.java @@ -39,6 +39,13 @@ public class LoggingDeprecationAccumulationHandler implements DeprecationHandler new Object[] {usedName, replacedWith})); } + @Override + public void usedDeprecatedField(String usedName) { + LoggingDeprecationHandler.INSTANCE.usedDeprecatedField(usedName); + deprecations.add(LoggerMessageFormat.format("Deprecated field [{}] used, which has been deprecated entirely", + new Object[]{ usedName })); + } + /** * The collected deprecation warnings */ diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ApiKeyService.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ApiKeyService.java index 3c7b9a5b8df..5d450648650 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ApiKeyService.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ApiKeyService.java @@ -634,6 +634,12 @@ public class ApiKeyService { deprecationLogger.deprecated("Deprecated field [{}] used in api key [{}], replaced by [{}]", usedName, apiKeyId, replacedWith); } + + @Override + public void usedDeprecatedField(String usedName) { + deprecationLogger.deprecated("Deprecated field [{}] used in api key [{}], which has been removed entirely", + usedName); + } } /**