Browse Source

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
Martijn van Groningen 6 years ago
parent
commit
f14874ca47

+ 11 - 7
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;
     }

+ 7 - 4
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.List;
@@ -48,10 +49,12 @@ public class EnrichIT extends ESRestHighLevelClientTestCase {
         @SuppressWarnings("unchecked")
         List<Map<String, Object>> responsePolicies = (List<Map<String, Object>>) 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<String, Object> toMap(Response response) throws IOException {

+ 36 - 32
docs/reference/ingest/ingest-node.asciidoc

@@ -858,10 +858,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
@@ -990,10 +991,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
@@ -1029,18 +1031,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"
+                ]
+            }
         }
     ]
 }
@@ -1065,18 +1068,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"
+                ]
+            }
         }
     ]
 }

+ 72 - 21
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<EnrichPolicy, Void> PARSER = new ConstructingObjectParser<>("policy",
-        args -> new EnrichPolicy(
-            (String) args[0],
-            (QuerySource) args[1],
-            (List<String>) args[2],
-            (String) args[3],
-            (List<String>) args[4]
+    private static final ConstructingObjectParser<EnrichPolicy, String> PARSER = new ConstructingObjectParser<>(
+        "policy",
+        false,
+        (args, policyType) -> new EnrichPolicy(
+            policyType,
+            (QuerySource) args[0],
+            (List<String>) args[1],
+            (String) args[2],
+            (List<String>) 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<NamedPolicy, Void> PARSER = new ConstructingObjectParser<>("named_policy",
-            args -> new NamedPolicy(
+        static final ConstructingObjectParser<NamedPolicy, String> PARSER = new ConstructingObjectParser<>(
+            "named_policy",
+            false,
+            (args, policyType) -> new NamedPolicy(
                 (String) args[0],
-                new EnrichPolicy((String) args[1],
-                    (QuerySource) args[2],
-                    (List<String>) args[3],
-                    (String) args[4],
-                    (List<String>) args[5])
+                new EnrichPolicy(policyType,
+                    (QuerySource) args[1],
+                    (List<String>) args[2],
+                    (String) args[3],
+                    (List<String>) 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

+ 23 - 7
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<Map<?,?>> policies = (List<Map<?,?>>) 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<String, Object> toMap(Response response) throws IOException {
         return toMap(EntityUtils.toString(response.getEntity()));
     }

+ 3 - 3
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]"));
     }

+ 2 - 3
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]"));

+ 12 - 14
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: