From cb42e19a321403b1b0327fbacea18eb62c7f4bed Mon Sep 17 00:00:00 2001 From: Martijn van Groningen Date: Fri, 23 Aug 2019 13:38:12 +0200 Subject: [PATCH] Change how type is stored in an enrich policy. (#45789) A policy type controls how the enrich index is created and the query executed against the match field. Currently there is a single policy type (`exact_match`). In the near future more policy types will be added and different policy may have different configuration options. For this reason type should be a json object instead of a string field: ``` { "exact_match": { ... } } ``` instead of: ``` { "type": "exact_match", ... } ``` This will make streaming parsing of enrich policies easier as in the new format, the parsing code can know ahead what configuration fields to expect. In the latter format that is not possible if the type field appears not as the first field. Relates to #32789 --- .../client/enrich/PutPolicyRequest.java | 18 ++-- .../org/elasticsearch/client/EnrichIT.java | 11 ++- docs/reference/ingest/ingest-node.asciidoc | 68 +++++++------- .../xpack/core/enrich/EnrichPolicy.java | 93 ++++++++++++++----- .../test/enrich/CommonEnrichRestTestCase.java | 30 ++++-- .../xpack/enrich/EnrichSecurityFailureIT.java | 6 +- .../xpack/enrich/EnrichSecurityIT.java | 5 +- .../rest-api-spec/test/enrich/10_basic.yml | 26 +++--- 8 files changed, 166 insertions(+), 91 deletions(-) diff --git a/client/rest-high-level/src/main/java/org/elasticsearch/client/enrich/PutPolicyRequest.java b/client/rest-high-level/src/main/java/org/elasticsearch/client/enrich/PutPolicyRequest.java index 60f048fe447..7385961ba59 100644 --- a/client/rest-high-level/src/main/java/org/elasticsearch/client/enrich/PutPolicyRequest.java +++ b/client/rest-high-level/src/main/java/org/elasticsearch/client/enrich/PutPolicyRequest.java @@ -36,7 +36,6 @@ import java.util.Objects; public class PutPolicyRequest implements Validatable, ToXContentObject { - static final ParseField TYPE_FIELD = new ParseField("type"); static final ParseField QUERY_FIELD = new ParseField("query"); static final ParseField INDICES_FIELD = new ParseField("indices"); static final ParseField MATCH_FIELD_FIELD = new ParseField("match_field"); @@ -108,13 +107,18 @@ public class PutPolicyRequest implements Validatable, ToXContentObject { @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { builder.startObject(); - builder.field(TYPE_FIELD.getPreferredName(), type); - builder.field(INDICES_FIELD.getPreferredName(), indices); - if (query != null) { - builder.field(QUERY_FIELD.getPreferredName(), asMap(query, builder.contentType())); + { + builder.startObject(type); + { + builder.field(INDICES_FIELD.getPreferredName(), indices); + if (query != null) { + builder.field(QUERY_FIELD.getPreferredName(), asMap(query, builder.contentType())); + } + builder.field(MATCH_FIELD_FIELD.getPreferredName(), matchField); + builder.field(ENRICH_FIELDS_FIELD.getPreferredName(), enrichFields); + } + builder.endObject(); } - builder.field(MATCH_FIELD_FIELD.getPreferredName(), matchField); - builder.field(ENRICH_FIELDS_FIELD.getPreferredName(), enrichFields); builder.endObject(); return builder; } diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/EnrichIT.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/EnrichIT.java index 57873e409f1..f63ac67a372 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/EnrichIT.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/EnrichIT.java @@ -23,6 +23,7 @@ import org.elasticsearch.client.core.AcknowledgedResponse; import org.elasticsearch.client.enrich.PutPolicyRequest; import org.elasticsearch.common.xcontent.XContentHelper; import org.elasticsearch.common.xcontent.json.JsonXContent; +import org.elasticsearch.common.xcontent.support.XContentMapValues; import java.io.IOException; import java.util.Collections; @@ -49,10 +50,12 @@ public class EnrichIT extends ESRestHighLevelClientTestCase { @SuppressWarnings("unchecked") List> responsePolicies = (List>) responseBody.get("policies"); assertThat(responsePolicies.size(), equalTo(1)); - assertThat(responsePolicies.get(0).get("type"), equalTo(putPolicyRequest.getType())); - assertThat(responsePolicies.get(0).get("indices"), equalTo(putPolicyRequest.getIndices())); - assertThat(responsePolicies.get(0).get("match_field"), equalTo(putPolicyRequest.getMatchField())); - assertThat(responsePolicies.get(0).get("enrich_fields"), equalTo(putPolicyRequest.getEnrichFields())); + assertThat(XContentMapValues.extractValue("exact_match.indices", responsePolicies.get(0)), + equalTo(putPolicyRequest.getIndices())); + assertThat(XContentMapValues.extractValue("exact_match.match_field", responsePolicies.get(0)), + equalTo(putPolicyRequest.getMatchField())); + assertThat(XContentMapValues.extractValue("exact_match.enrich_fields", responsePolicies.get(0)), + equalTo(putPolicyRequest.getEnrichFields())); } private static Map toMap(Response response) throws IOException { diff --git a/docs/reference/ingest/ingest-node.asciidoc b/docs/reference/ingest/ingest-node.asciidoc index 9bb6c001302..6c766a950ea 100644 --- a/docs/reference/ingest/ingest-node.asciidoc +++ b/docs/reference/ingest/ingest-node.asciidoc @@ -857,10 +857,11 @@ Create an enrich policy: -------------------------------------------------- PUT /_enrich/policy/users-policy { - "type": "exact_match", - "indices": "users", - "match_field": "email", - "enrich_fields": ["first_name", "last_name", "address", "city", "zip", "state"] + "exact_match": { + "indices": "users", + "match_field": "email", + "enrich_fields": ["first_name", "last_name", "address", "city", "zip", "state"] + } } -------------------------------------------------- // CONSOLE @@ -989,10 +990,11 @@ Request: -------------------------------------------------- PUT /_enrich/policy/my-policy { - "type": "exact_match", - "indices": "users", - "match_field": "email", - "enrich_fields": ["first_name", "last_name", "address", "city", "zip", "state"] + "exact_match": { + "indices": "users", + "match_field": "email", + "enrich_fields": ["first_name", "last_name", "address", "city", "zip", "state"] + } } -------------------------------------------------- // CONSOLE @@ -1028,18 +1030,19 @@ Response: { "policies": [ { - "name" : "my-policy", - "type" : "exact_match", - "indices" : ["users"], - "match_field" : "email", - "enrich_fields" : [ - "first_name", - "last_name", - "address", - "city", - "zip", - "state" - ] + "exact_match": { + "name" : "my-policy", + "indices" : ["users"], + "match_field" : "email", + "enrich_fields" : [ + "first_name", + "last_name", + "address", + "city", + "zip", + "state" + ] + } } ] } @@ -1064,18 +1067,19 @@ Response: { "policies": [ { - "name" : "my-policy", - "type" : "exact_match", - "indices" : ["users"], - "match_field" : "email", - "enrich_fields" : [ - "first_name", - "last_name", - "address", - "city", - "zip", - "state" - ] + "exact_match": { + "name" : "my-policy", + "indices" : ["users"], + "match_field" : "email", + "enrich_fields" : [ + "first_name", + "last_name", + "address", + "city", + "zip", + "state" + ] + } } ] } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/enrich/EnrichPolicy.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/enrich/EnrichPolicy.java index 929d2ff9cdb..ea83e068033 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/enrich/EnrichPolicy.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/enrich/EnrichPolicy.java @@ -6,6 +6,7 @@ package org.elasticsearch.xpack.core.enrich; import org.elasticsearch.common.ParseField; +import org.elasticsearch.common.ParsingException; import org.elasticsearch.common.Strings; import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.io.stream.StreamInput; @@ -17,6 +18,7 @@ import org.elasticsearch.common.xcontent.ToXContentFragment; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentHelper; import org.elasticsearch.common.xcontent.XContentParser; +import org.elasticsearch.common.xcontent.XContentParser.Token; import org.elasticsearch.common.xcontent.XContentType; import java.io.IOException; @@ -34,20 +36,21 @@ public final class EnrichPolicy implements Writeable, ToXContentFragment { public static final String EXACT_MATCH_TYPE = "exact_match"; public static final String[] SUPPORTED_POLICY_TYPES = new String[]{EXACT_MATCH_TYPE}; - private static final ParseField TYPE = new ParseField("type"); private static final ParseField QUERY = new ParseField("query"); private static final ParseField INDICES = new ParseField("indices"); private static final ParseField MATCH_FIELD = new ParseField("match_field"); private static final ParseField ENRICH_FIELDS = new ParseField("enrich_fields"); @SuppressWarnings("unchecked") - private static final ConstructingObjectParser PARSER = new ConstructingObjectParser<>("policy", - args -> new EnrichPolicy( - (String) args[0], - (QuerySource) args[1], - (List) args[2], - (String) args[3], - (List) args[4] + private static final ConstructingObjectParser PARSER = new ConstructingObjectParser<>( + "policy", + false, + (args, policyType) -> new EnrichPolicy( + policyType, + (QuerySource) args[0], + (List) args[1], + (String) args[2], + (List) args[3] ) ); @@ -56,7 +59,6 @@ public final class EnrichPolicy implements Writeable, ToXContentFragment { } private static void declareParserOptions(ConstructingObjectParser parser) { - parser.declareString(ConstructingObjectParser.constructorArg(), TYPE); parser.declareObject(ConstructingObjectParser.optionalConstructorArg(), (p, c) -> { XContentBuilder contentBuilder = XContentBuilder.builder(p.contentType().xContent()); contentBuilder.generator().copyCurrentStructure(p); @@ -68,7 +70,24 @@ public final class EnrichPolicy implements Writeable, ToXContentFragment { } public static EnrichPolicy fromXContent(XContentParser parser) throws IOException { - return PARSER.parse(parser, null); + Token token = parser.currentToken(); + if (token != Token.START_OBJECT) { + token = parser.nextToken(); + } + if (token != Token.START_OBJECT) { + throw new ParsingException(parser.getTokenLocation(), "unexpected token"); + } + token = parser.nextToken(); + if (token != Token.FIELD_NAME) { + throw new ParsingException(parser.getTokenLocation(), "unexpected token"); + } + String policyType = parser.currentName(); + EnrichPolicy policy = PARSER.parse(parser, policyType); + token = parser.nextToken(); + if (token != Token.END_OBJECT) { + throw new ParsingException(parser.getTokenLocation(), "unexpected token"); + } + return policy; } private final String type; @@ -134,14 +153,21 @@ public final class EnrichPolicy implements Writeable, ToXContentFragment { @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { - builder.field(TYPE.getPreferredName(), type); + builder.startObject(type); + { + toInnerXContent(builder); + } + builder.endObject(); + return builder; + } + + private void toInnerXContent(XContentBuilder builder) throws IOException { if (query != null) { builder.field(QUERY.getPreferredName(), query.getQueryAsMap()); } builder.array(INDICES.getPreferredName(), indices.toArray(new String[0])); builder.field(MATCH_FIELD.getPreferredName(), matchField); builder.array(ENRICH_FIELDS.getPreferredName(), enrichFields.toArray(new String[0])); - return builder; } @Override @@ -222,14 +248,16 @@ public final class EnrichPolicy implements Writeable, ToXContentFragment { static final ParseField NAME = new ParseField("name"); @SuppressWarnings("unchecked") - static final ConstructingObjectParser PARSER = new ConstructingObjectParser<>("named_policy", - args -> new NamedPolicy( + static final ConstructingObjectParser PARSER = new ConstructingObjectParser<>( + "named_policy", + false, + (args, policyType) -> new NamedPolicy( (String) args[0], - new EnrichPolicy((String) args[1], - (QuerySource) args[2], - (List) args[3], - (String) args[4], - (List) args[5]) + new EnrichPolicy(policyType, + (QuerySource) args[1], + (List) args[2], + (String) args[3], + (List) args[4]) ) ); @@ -268,16 +296,39 @@ public final class EnrichPolicy implements Writeable, ToXContentFragment { @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { builder.startObject(); + builder.startObject(policy.type); { builder.field(NAME.getPreferredName(), name); - policy.toXContent(builder, params); + policy.toInnerXContent(builder); } builder.endObject(); + builder.endObject(); return builder; } public static NamedPolicy fromXContent(XContentParser parser) throws IOException { - return PARSER.parse(parser, null); + Token token = parser.currentToken(); + if (token != Token.START_OBJECT) { + token = parser.nextToken(); + } + if (token != Token.START_OBJECT) { + throw new ParsingException(parser.getTokenLocation(), "unexpected token"); + } + token = parser.nextToken(); + if (token != Token.FIELD_NAME) { + throw new ParsingException(parser.getTokenLocation(), "unexpected token"); + } + String policyType = parser.currentName(); + token = parser.nextToken(); + if (token != Token.START_OBJECT) { + throw new ParsingException(parser.getTokenLocation(), "unexpected token"); + } + NamedPolicy policy = PARSER.parse(parser, policyType); + token = parser.nextToken(); + if (token != Token.END_OBJECT) { + throw new ParsingException(parser.getTokenLocation(), "unexpected token"); + } + return policy; } @Override diff --git a/x-pack/plugin/enrich/qa/common/src/main/java/org/elasticsearch/test/enrich/CommonEnrichRestTestCase.java b/x-pack/plugin/enrich/qa/common/src/main/java/org/elasticsearch/test/enrich/CommonEnrichRestTestCase.java index d84b02a3cc6..5a7b440ded3 100644 --- a/x-pack/plugin/enrich/qa/common/src/main/java/org/elasticsearch/test/enrich/CommonEnrichRestTestCase.java +++ b/x-pack/plugin/enrich/qa/common/src/main/java/org/elasticsearch/test/enrich/CommonEnrichRestTestCase.java @@ -9,8 +9,12 @@ import org.apache.http.util.EntityUtils; import org.elasticsearch.client.Request; import org.elasticsearch.client.Response; import org.elasticsearch.client.ResponseException; +import org.elasticsearch.common.Strings; +import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentHelper; import org.elasticsearch.common.xcontent.json.JsonXContent; +import org.elasticsearch.common.xcontent.support.XContentMapValues; +import org.elasticsearch.index.query.QueryBuilders; import org.elasticsearch.test.rest.ESRestTestCase; import org.junit.After; @@ -18,6 +22,7 @@ import java.io.IOException; import java.util.List; import java.util.Map; +import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder; import static org.hamcrest.Matchers.equalTo; public abstract class CommonEnrichRestTestCase extends ESRestTestCase { @@ -29,15 +34,14 @@ public abstract class CommonEnrichRestTestCase extends ESRestTestCase { List> policies = (List>) responseMap.get("policies"); for (Map entry: policies) { - client().performRequest(new Request("DELETE", "/_enrich/policy/" + entry.get("name"))); + client().performRequest(new Request("DELETE", "/_enrich/policy/" + XContentMapValues.extractValue("exact_match.name", entry))); } } private void setupGenericLifecycleTest(boolean deletePipeilne) throws Exception { // Create the policy: Request putPolicyRequest = new Request("PUT", "/_enrich/policy/my_policy"); - putPolicyRequest.setJsonEntity("{\"type\": \"exact_match\",\"indices\": [\"my-source-index\"], \"match_field\": \"host\", " + - "\"enrich_fields\": [\"globalRank\", \"tldRank\", \"tld\"]}"); + putPolicyRequest.setJsonEntity(generatePolicySource("my-source-index")); assertOK(client().performRequest(putPolicyRequest)); // Add entry to source index and then refresh: @@ -86,8 +90,7 @@ public abstract class CommonEnrichRestTestCase extends ESRestTestCase { public void testImmutablePolicy() throws IOException { Request putPolicyRequest = new Request("PUT", "/_enrich/policy/my_policy"); - putPolicyRequest.setJsonEntity("{\"type\": \"exact_match\",\"indices\": [\"my-source-index\"], \"match_field\": \"host\", " + - "\"enrich_fields\": [\"globalRank\", \"tldRank\", \"tld\"]}"); + putPolicyRequest.setJsonEntity(generatePolicySource("my-source-index")); assertOK(client().performRequest(putPolicyRequest)); ResponseException exc = expectThrows(ResponseException.class, () -> client().performRequest(putPolicyRequest)); @@ -96,8 +99,7 @@ public abstract class CommonEnrichRestTestCase extends ESRestTestCase { public void testDeleteIsCaseSensitive() throws Exception { Request putPolicyRequest = new Request("PUT", "/_enrich/policy/my_policy"); - putPolicyRequest.setJsonEntity("{\"type\": \"exact_match\",\"indices\": [\"my-source-index\"], \"match_field\": \"host\", " + - "\"enrich_fields\": [\"globalRank\", \"tldRank\", \"tld\"]}"); + putPolicyRequest.setJsonEntity(generatePolicySource("my-source-index")); assertOK(client().performRequest(putPolicyRequest)); ResponseException exc = expectThrows(ResponseException.class, @@ -129,6 +131,20 @@ public abstract class CommonEnrichRestTestCase extends ESRestTestCase { assertOK(client().performRequest(getRequest)); } + public static String generatePolicySource(String index) throws IOException { + XContentBuilder source = jsonBuilder().startObject().startObject("exact_match"); + { + source.field("indices", index); + if (randomBoolean()) { + source.field("query", QueryBuilders.matchAllQuery()); + } + source.field("match_field", "host"); + source.field("enrich_fields", new String[] {"globalRank", "tldRank", "tld"}); + } + source.endObject().endObject(); + return Strings.toString(source); + } + private static Map toMap(Response response) throws IOException { return toMap(EntityUtils.toString(response.getEntity())); } diff --git a/x-pack/plugin/enrich/qa/rest-with-security/src/test/java/org/elasticsearch/xpack/enrich/EnrichSecurityFailureIT.java b/x-pack/plugin/enrich/qa/rest-with-security/src/test/java/org/elasticsearch/xpack/enrich/EnrichSecurityFailureIT.java index fd4f1589eda..4129dcbf920 100644 --- a/x-pack/plugin/enrich/qa/rest-with-security/src/test/java/org/elasticsearch/xpack/enrich/EnrichSecurityFailureIT.java +++ b/x-pack/plugin/enrich/qa/rest-with-security/src/test/java/org/elasticsearch/xpack/enrich/EnrichSecurityFailureIT.java @@ -10,6 +10,7 @@ import org.elasticsearch.client.ResponseException; import org.elasticsearch.common.settings.SecureString; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.util.concurrent.ThreadContext; +import org.elasticsearch.test.enrich.CommonEnrichRestTestCase; import org.elasticsearch.test.rest.ESRestTestCase; import static org.elasticsearch.xpack.core.security.authc.support.UsernamePasswordToken.basicAuthHeaderValue; @@ -32,10 +33,9 @@ public class EnrichSecurityFailureIT extends ESRestTestCase { .build(); } - public void testFailure() { + public void testFailure() throws Exception { Request putPolicyRequest = new Request("PUT", "/_enrich/policy/my_policy"); - putPolicyRequest.setJsonEntity("{\"type\": \"exact_match\",\"indices\": [\"my-source-index\"], \"match_field\": \"host\", " + - "\"enrich_fields\": [\"globalRank\", \"tldRank\", \"tld\"]}"); + putPolicyRequest.setJsonEntity(CommonEnrichRestTestCase.generatePolicySource("my-source-index")); ResponseException exc = expectThrows(ResponseException.class, () -> client().performRequest(putPolicyRequest)); assertTrue(exc.getMessage().contains("action [cluster:admin/xpack/enrich/put] is unauthorized for user [test_enrich_no_privs]")); } diff --git a/x-pack/plugin/enrich/qa/rest-with-security/src/test/java/org/elasticsearch/xpack/enrich/EnrichSecurityIT.java b/x-pack/plugin/enrich/qa/rest-with-security/src/test/java/org/elasticsearch/xpack/enrich/EnrichSecurityIT.java index 86e94183e12..0f7838c4a45 100644 --- a/x-pack/plugin/enrich/qa/rest-with-security/src/test/java/org/elasticsearch/xpack/enrich/EnrichSecurityIT.java +++ b/x-pack/plugin/enrich/qa/rest-with-security/src/test/java/org/elasticsearch/xpack/enrich/EnrichSecurityIT.java @@ -33,12 +33,11 @@ public class EnrichSecurityIT extends CommonEnrichRestTestCase { .build(); } - public void testInsufficientPermissionsOnNonExistentIndex() { + public void testInsufficientPermissionsOnNonExistentIndex() throws Exception { // This test is here because it requires a valid user that has permission to execute policy PUTs but should fail if the user // does not have access to read the backing indices used to enrich the data. Request putPolicyRequest = new Request("PUT", "/_enrich/policy/my_policy"); - putPolicyRequest.setJsonEntity("{\"type\": \"exact_match\",\"indices\": [\"some-other-index\"], \"match_field\": \"host\", " + - "\"enrich_fields\": [\"globalRank\", \"tldRank\", \"tld\"]}"); + putPolicyRequest.setJsonEntity(generatePolicySource("some-other-index")); ResponseException exc = expectThrows(ResponseException.class, () -> client().performRequest(putPolicyRequest)); assertThat(exc.getMessage(), containsString("unable to store policy because no indices match with the specified index patterns [some-other-index]")); diff --git a/x-pack/plugin/enrich/qa/rest/src/test/resources/rest-api-spec/test/enrich/10_basic.yml b/x-pack/plugin/enrich/qa/rest/src/test/resources/rest-api-spec/test/enrich/10_basic.yml index e6a48d64134..173e0fe34ac 100644 --- a/x-pack/plugin/enrich/qa/rest/src/test/resources/rest-api-spec/test/enrich/10_basic.yml +++ b/x-pack/plugin/enrich/qa/rest/src/test/resources/rest-api-spec/test/enrich/10_basic.yml @@ -5,10 +5,10 @@ enrich.put_policy: name: policy-crud body: - type: exact_match - indices: ["bar*"] - match_field: baz - enrich_fields: ["a", "b"] + exact_match: + indices: ["bar*"] + match_field: baz + enrich_fields: ["a", "b"] - is_true: acknowledged - do: @@ -20,20 +20,18 @@ enrich.get_policy: name: policy-crud - length: { policies: 1 } - - match: { policies.0.name: policy-crud } - - match: { policies.0.type: exact_match } - - match: { policies.0.indices: ["bar*"] } - - match: { policies.0.match_field: baz } - - match: { policies.0.enrich_fields: ["a", "b"] } + - match: { policies.0.exact_match.name: policy-crud } + - match: { policies.0.exact_match.indices: ["bar*"] } + - match: { policies.0.exact_match.match_field: baz } + - match: { policies.0.exact_match.enrich_fields: ["a", "b"] } - do: enrich.get_policy: {} - length: { policies: 1 } - - match: { policies.0.name: policy-crud } - - match: { policies.0.type: exact_match } - - match: { policies.0.indices: ["bar*"] } - - match: { policies.0.match_field: baz } - - match: { policies.0.enrich_fields: ["a", "b"] } + - match: { policies.0.exact_match.name: policy-crud } + - match: { policies.0.exact_match.indices: ["bar*"] } + - match: { policies.0.exact_match.match_field: baz } + - match: { policies.0.exact_match.enrich_fields: ["a", "b"] } - do: enrich.delete_policy: