Skip to content

Deprecate all variants of a ParseField with no replacement #53722

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 18, 2020
Merged

Deprecate all variants of a ParseField with no replacement #53722

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 1 addition & 6 deletions client/rest-high-level/src/main/java/org/elasticsearch/client/RestHighLevelClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1888,12 +1888,7 @@ protected static boolean convertExistsResponse(Response response) {
* 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<NamedXContentRegistry.Entry> getDefaultNamedXContents() {
Map<String, ContextParser<Object, ? extends Aggregation>> map = new HashMap<>();
Expand Down
11 changes: 1 addition & 10 deletions client/rest-high-level/src/main/java/org/elasticsearch/client/ml/job/config/RuleScope.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ public static ContextParser<Void, RuleScope> parser() {
Map<String, ?> value = (Map<String, ?>) 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));
}
}
Expand All @@ -59,15 +59,6 @@ public static ContextParser<Void, RuleScope> parser() {
};
}

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<String, FilterRef> scope;

public RuleScope() {
Expand Down
10 changes: 1 addition & 9 deletions ...level/src/test/java/org/elasticsearch/client/security/DeleteRoleMappingResponseTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
Expand Down
34 changes: 13 additions & 21 deletions ...igh-level/src/test/java/org/elasticsearch/client/security/ExpressionRoleMappingTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,29 +37,21 @@
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(),
Expand Down
10 changes: 1 addition & 9 deletions ...igh-level/src/test/java/org/elasticsearch/client/security/GetPrivilegesResponseTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,15 +81,7 @@ public void testFromXContent() throws IOException {
"}";

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);
Expand Down
60 changes: 26 additions & 34 deletions ...h-level/src/test/java/org/elasticsearch/client/security/GetRoleMappingsResponseTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,42 +36,34 @@
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<ExpressionRoleMapping> expectedRoleMappingsList = new ArrayList<>();
expectedRoleMappingsList.add(new ExpressionRoleMapping("kerberosmapping", FieldRoleMapperExpression.ofKeyValues("realm.name",
"kerb1"), Collections.singletonList("superuser"), Collections.emptyList(), null, true));
Expand Down
10 changes: 1 addition & 9 deletions ...est-high-level/src/test/java/org/elasticsearch/client/security/GetRolesResponseTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,15 +64,7 @@ public void testFromXContent() throws IOException {
" }\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);
Expand Down
10 changes: 1 addition & 9 deletions ...csearch/client/security/support/expressiondsl/parser/RoleMapperExpressionParserTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,15 +115,7 @@ private <T> T checkExpressionType(RoleMapperExpression expr, Class<T> type) {

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));
}

}
10 changes: 1 addition & 9 deletions ...est/java/org/elasticsearch/client/security/user/privileges/ApplicationPrivilegeTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,15 +54,7 @@ public void testFromXContentAndToXContent() throws IOException {
+ " }\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<String, Object> metadata = new HashMap<>();
metadata.put("description", "Read access to myapp");
final ApplicationPrivilege expectedPrivilege =
Expand Down
16 changes: 14 additions & 2 deletions libs/x-content/src/main/java/org/elasticsearch/common/ParseField.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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];

Expand Down Expand Up @@ -96,6 +97,15 @@ public ParseField withAllDeprecated(String allReplacedWith) {
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
Expand All @@ -108,15 +118,17 @@ public boolean match(String fieldName, DeprecationHandler deprecationHandler) {
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)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be simpler to add something like:

if (fullyDeprecated) {
  deprecationHandler.usedDeprecatedField(fieldName);
  return true;
}

above this if statement?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We still need to check that one of the possible names matches so it's not quite as simple as that, unfortunately

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair enough.

return true;
}
// Now try to match against one of the deprecated names. Note that if
// the parse field is entirely deprecated (allReplacedWith != null) all
// 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);
Expand Down
35 changes: 34 additions & 1 deletion libs/x-content/src/main/java/org/elasticsearch/common/xcontent/DeprecationHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,32 @@ public void usedDeprecatedName(String usedName, String modernName) {
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() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@Override
public void usedDeprecatedName(String usedName, String modernName) {

}

@Override
public void usedDeprecatedField(String usedName, String replacedWith) {

}

@Override
public void usedDeprecatedField(String usedName) {

}
};

/**
Expand All @@ -52,9 +78,16 @@ public void usedDeprecatedName(String usedName, String modernName) {

/**
* 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);
}
15 changes: 15 additions & 0 deletions libs/x-content/src/test/java/org/elasticsearch/common/ParseFieldTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,21 @@ public void testAllDeprecated() {
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"));
Expand Down
7 changes: 6 additions & 1 deletion server/src/main/java/org/elasticsearch/common/xcontent/LoggingDeprecationHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
* 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.
*
Expand All @@ -57,4 +57,9 @@ public void usedDeprecatedName(String usedName, String modernName) {
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);
}
}
7 changes: 7 additions & 0 deletions ...ain/java/org/elasticsearch/xpack/core/ml/utils/LoggingDeprecationAccumulationHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,13 @@ public void usedDeprecatedField(String usedName, String replacedWith) {
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
*/
Expand Down
6 changes: 6 additions & 0 deletions ...k/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ApiKeyService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -628,6 +628,12 @@ public void usedDeprecatedField(String usedName, String replacedWith) {
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);
}
}

/**
Expand Down