Merge pull request #5 from kderusso/kderusso/carlos_template_validation

Template parameters validation

x-pack/plugin/ent-search/build.gradle

@@ -16,6 +16,7 @@ dependencies {
   api project(':modules:lang-mustache')
 
   implementation "com.fasterxml.jackson.core:jackson-core:${versions.jackson}"
+
   implementation "com.networknt:json-schema-validator:${versions.networknt_json_schema_validator}"
   implementation "com.fasterxml.jackson.core:jackson-databind:${versions.jackson}"
 

x-pack/plugin/ent-search/qa/rest/src/yamlRestTest/resources/rest-api-spec/test/entsearch/55_search_application_search.yml

@@ -41,6 +41,11 @@ setup:
               params:
                 field_name: field1
                 field_value: value1
+            dictionary:
+              field_name:
+                type: string
+              field_value:
+                type: string
 
   - do:
       index:
@@ -122,6 +127,18 @@ teardown:
   - match: { hits.total.value: 1 }
   - match: { hits.hits.0._id: "doc2" }
 
+---
+"Query Search Application with invalid parameter validation":
+
+  - do:
+      catch: "bad_request"
+      search_application.search:
+        name: test-search-application
+        body:
+          params:
+            field_name: field3
+            field_value: 35
+
 ---
 "Query Search Application - not found":
 

x-pack/plugin/ent-search/src/main/java/module-info.java

@@ -6,6 +6,9 @@
  */
 
 module org.elasticsearch.application {
+    requires com.fasterxml.jackson.core;
+    requires com.fasterxml.jackson.databind;
+    requires com.fasterxml.jackson.annotation;
     requires org.apache.logging.log4j;
     requires org.apache.lucene.core;
 
@@ -15,6 +18,7 @@
     requires org.elasticsearch.server;
     requires org.elasticsearch.xcontent;
     requires org.elasticsearch.xcore;
+    requires json.schema.validator;
 
     exports org.elasticsearch.xpack.application.analytics;
     exports org.elasticsearch.xpack.application.analytics.action;

x-pack/plugin/ent-search/src/main/java/org/elasticsearch/xpack/application/search/SearchApplication.java

@@ -9,6 +9,7 @@
 
 import org.elasticsearch.ElasticsearchParseException;
 import org.elasticsearch.common.Strings;
+import org.elasticsearch.common.ValidationException;
 import org.elasticsearch.common.bytes.BytesReference;
 import org.elasticsearch.common.io.stream.ReleasableBytesStreamOutput;
 import org.elasticsearch.common.io.stream.StreamInput;
@@ -86,7 +87,7 @@ public SearchApplication(
         this.searchApplicationTemplate = searchApplicationTemplate;
     }
 
-    public SearchApplication(StreamInput in) throws IOException {
+    public SearchApplication(StreamInput in) throws IOException, ValidationException {
         this.name = in.readString();
         this.indices = in.readStringArray();
         this.analyticsCollectionName = in.readOptionalString();

x-pack/plugin/ent-search/src/main/java/org/elasticsearch/xpack/application/search/SearchApplicationIndexService.java

@@ -33,6 +33,7 @@
 import org.elasticsearch.cluster.metadata.IndexMetadata;
 import org.elasticsearch.cluster.metadata.Metadata;
 import org.elasticsearch.cluster.service.ClusterService;
+import org.elasticsearch.common.ValidationException;
 import org.elasticsearch.common.bytes.BytesReference;
 import org.elasticsearch.common.document.DocumentField;
 import org.elasticsearch.common.io.stream.InputStreamStreamInput;
@@ -186,24 +187,11 @@ private static XContentBuilder getIndexMappings() {
         }
     }
 
-    /**
-     * Gets the {@link SearchApplication} from the index if present, or delegate a {@link ResourceNotFoundException} failure to the provided
-     * listener if not.
-     *
-     * @param resourceName The resource name.
-     * @param listener The action listener to invoke on response/failure.
-     */
-    public void getSearchApplication(String resourceName, ActionListener<SearchApplication> listener) {
-        final GetRequest getRequest = new GetRequest(SEARCH_APPLICATION_ALIAS_NAME).id(resourceName).realtime(true);
-        clientWithOrigin.get(getRequest, new DelegatingIndexNotFoundActionListener<>(resourceName, listener, (l, getResponse) -> {
-            if (getResponse.isExists() == false) {
-                l.onFailure(new ResourceNotFoundException(resourceName));
-                return;
-            }
-            final BytesReference source = getResponse.getSourceInternal();
-            final SearchApplication res = parseSearchApplicationBinaryFromSource(source);
-            l.onResponse(res);
-        }));
+    static SearchApplication parseSearchApplicationBinaryWithVersion(StreamInput in) throws IOException, ValidationException {
+        TransportVersion version = TransportVersion.readVersion(in);
+        assert version.onOrBefore(TransportVersion.CURRENT) : version + " >= " + TransportVersion.CURRENT;
+        in.setTransportVersion(version);
+        return new SearchApplication(in);
     }
 
     private static String getSearchAliasName(SearchApplication app) {
@@ -423,6 +411,30 @@ private static SearchApplicationListItem hitToSearchApplicationListItem(SearchHi
         );
     }
 
+    /**
+     * Gets the {@link SearchApplication} from the index if present, or delegate a {@link ResourceNotFoundException} failure to the provided
+     * listener if not.
+     *
+     * @param resourceName The resource name.
+     * @param listener The action listener to invoke on response/failure.
+     */
+    public void getSearchApplication(String resourceName, ActionListener<SearchApplication> listener) {
+        final GetRequest getRequest = new GetRequest(SEARCH_APPLICATION_ALIAS_NAME).id(resourceName).realtime(true);
+        clientWithOrigin.get(getRequest, new DelegatingIndexNotFoundActionListener<>(resourceName, listener, (l, getResponse) -> {
+            if (getResponse.isExists() == false) {
+                l.onFailure(new ResourceNotFoundException(resourceName));
+                return;
+            }
+            try {
+                final BytesReference source = getResponse.getSourceInternal();
+                final SearchApplication res = parseSearchApplicationBinaryFromSource(source);
+                l.onResponse(res);
+            } catch (Exception e) {
+                l.onFailure(e);
+            }
+        }));
+    }
+
     private SearchApplication parseSearchApplicationBinaryFromSource(BytesReference source) {
         try (XContentParser parser = XContentHelper.createParser(XContentParserConfiguration.EMPTY, source, XContentType.JSON)) {
             ensureExpectedToken(parser.nextToken(), XContentParser.Token.START_OBJECT, parser);
@@ -453,16 +465,11 @@ public int read() {
             throw new ElasticsearchParseException("[" + SearchApplication.BINARY_CONTENT_FIELD.getPreferredName() + "] field is missing");
         } catch (IOException e) {
             throw new ElasticsearchParseException("Failed to parse: " + source.utf8ToString(), e);
+        } catch (ValidationException e) {
+            throw new ElasticsearchParseException("Invalid Search Application: " + source.utf8ToString(), e);
         }
     }
 
-    static SearchApplication parseSearchApplicationBinaryWithVersion(StreamInput in) throws IOException {
-        TransportVersion version = TransportVersion.readVersion(in);
-        assert version.onOrBefore(TransportVersion.CURRENT) : version + " >= " + TransportVersion.CURRENT;
-        in.setTransportVersion(version);
-        return new SearchApplication(in);
-    }
-
     static void writeSearchApplicationBinaryWithVersion(SearchApplication app, OutputStream os, Version minNodeVersion) throws IOException {
         // do not close the output
         os = Streams.noCloseStream(os);

x-pack/plugin/ent-search/src/main/java/org/elasticsearch/xpack/application/search/SearchApplicationTemplate.java

@@ -7,6 +7,7 @@
 
 package org.elasticsearch.xpack.application.search;
 
+import org.elasticsearch.common.ValidationException;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.common.io.stream.Writeable;
@@ -22,6 +23,7 @@
 import org.elasticsearch.xpack.application.search.action.QuerySearchApplicationAction.Request;
 
 import java.io.IOException;
+import java.util.Map;
 import java.util.Objects;
 
 import static org.elasticsearch.xcontent.ConstructingObjectParser.optionalConstructorArg;
@@ -44,14 +46,15 @@ public class SearchApplicationTemplate implements ToXContentObject, Writeable {
     static {
         PARSER.declareObject(optionalConstructorArg(), (p, c) -> Script.parse(p, Script.DEFAULT_TEMPLATE_LANG), TEMPLATE_SCRIPT_FIELD);
         PARSER.declareObject(optionalConstructorArg(), (p, c) -> {
-            XContentBuilder builder = XContentFactory.jsonBuilder();
-            return new TemplateParamValidator(builder.copyCurrentStructure(p));
+            try (XContentBuilder builder = XContentFactory.jsonBuilder()) {
+                return new TemplateParamValidator(builder.copyCurrentStructure(p));
+            }
         }, DICTIONARY_FIELD);
     }
 
     private final TemplateParamValidator templateParamValidator;
 
-    public SearchApplicationTemplate(StreamInput in) throws IOException {
+    public SearchApplicationTemplate(StreamInput in) throws IOException, ValidationException {
         this.script = in.readOptionalWriteable(Script::new);
         this.templateParamValidator = in.readOptionalWriteable(TemplateParamValidator::new);
     }
@@ -90,6 +93,18 @@ public void writeTo(StreamOutput out) throws IOException {
         out.writeOptionalWriteable(templateParamValidator);
     }
 
+    public static SearchApplicationTemplate parse(XContentParser parser) {
+        return PARSER.apply(parser, null);
+    }
+
+    static {
+        PARSER.declareObject(
+            optionalConstructorArg(),
+            (p, c) -> Script.parse(p, Script.DEFAULT_TEMPLATE_LANG),
+            SearchApplication.TEMPLATE_SCRIPT_FIELD
+        );
+    }
+
     @Override
     public int hashCode() {
         return Objects.hash(script, templateParamValidator);
@@ -103,6 +118,12 @@ public boolean equals(Object o) {
         return Objects.equals(script, template.script) && Objects.equals(templateParamValidator, template.templateParamValidator);
     }
 
+    public void validateTemplateParams(Map<String, Object> templateParams) throws ValidationException {
+        if (templateParamValidator != null) {
+            templateParamValidator.validate(templateParams);
+        }
+    }
+
     public Script script() {
         return script;
     }

x-pack/plugin/ent-search/src/main/java/org/elasticsearch/xpack/application/search/TemplateParamValidator.java

@@ -8,6 +8,7 @@
 package org.elasticsearch.xpack.application.search;
 
 import com.fasterxml.jackson.core.JsonProcessingException;
+import com.fasterxml.jackson.databind.JsonNode;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.fasterxml.jackson.databind.node.ObjectNode;
 import com.networknt.schema.JsonSchema;
@@ -26,6 +27,7 @@
 
 import java.io.IOException;
 import java.io.InputStream;
+import java.util.Map;
 import java.util.Objects;
 import java.util.Set;
 
@@ -54,16 +56,7 @@ public TemplateParamValidator(String dictionaryContent) throws ValidationExcepti
             // Create a new Schema with "properties" node based on the dictionary content
             final ObjectNode schemaJsonNode = OBJECT_MAPPER.createObjectNode();
             schemaJsonNode.set(PROPERTIES_NODE, OBJECT_MAPPER.readTree(dictionaryContent));
-            final Set<ValidationMessage> validationMessages = META_SCHEMA.validate(schemaJsonNode);
-
-            if (validationMessages.isEmpty() == false) {
-                ValidationException validationException = new ValidationException();
-                for (ValidationMessage validationMessage : validationMessages) {
-                    validationException.addValidationError(validationMessage.getMessage());
-                }
-
-                throw validationException;
-            }
+            validateWithSchema(META_SCHEMA, schemaJsonNode);
 
             this.jsonSchema = SCHEMA_FACTORY.getSchema(schemaJsonNode);
         } catch (JsonProcessingException e) {
@@ -75,6 +68,22 @@ public TemplateParamValidator(XContentBuilder xContentBuilder) {
         this(Strings.toString(xContentBuilder));
     }
 
+    private static void validateWithSchema(JsonSchema jsonSchema, JsonNode jsonNode) {
+        final Set<ValidationMessage> validationMessages = jsonSchema.validate(jsonNode);
+        if (validationMessages.isEmpty() == false) {
+            ValidationException validationException = new ValidationException();
+            for (ValidationMessage message : validationMessages) {
+                validationException.addValidationError(message.getMessage());
+            }
+
+            throw validationException;
+        }
+    }
+
+    public void validate(Map<String, Object> templateParams) throws ValidationException {
+        validateWithSchema(this.jsonSchema, OBJECT_MAPPER.valueToTree(templateParams));
+    }
+
     @Override
     public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
         try (InputStream stream = new BytesArray(getSchemaPropertiesAsString()).streamInput()) {

x-pack/plugin/ent-search/src/main/java/org/elasticsearch/xpack/application/search/action/TransportQuerySearchApplicationAction.java

@@ -16,6 +16,7 @@
 import org.elasticsearch.action.support.ActionFilters;
 import org.elasticsearch.client.internal.Client;
 import org.elasticsearch.cluster.service.ClusterService;
+import org.elasticsearch.common.ValidationException;
 import org.elasticsearch.common.inject.Inject;
 import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
 import org.elasticsearch.common.util.BigArrays;
@@ -31,6 +32,8 @@
 import org.elasticsearch.xcontent.XContentParser;
 import org.elasticsearch.xcontent.XContentParserConfiguration;
 import org.elasticsearch.xcontent.XContentType;
+import org.elasticsearch.xpack.application.search.SearchApplication;
+import org.elasticsearch.xpack.application.search.SearchApplicationTemplate;
 
 import java.io.IOException;
 import java.util.HashMap;
@@ -78,19 +81,17 @@ public TransportQuerySearchApplicationAction(
     @Override
     protected void doExecute(QuerySearchApplicationAction.Request request, ActionListener<SearchResponse> listener) {
         systemIndexService.getSearchApplication(request.name(), listener.delegateFailure((l, searchApplication) -> {
-            final Script script = searchApplication.searchApplicationTemplate().script();
-
             try {
-                final SearchSourceBuilder sourceBuilder = renderTemplate(script, mergeTemplateParams(request, script));
+                final SearchSourceBuilder sourceBuilder = renderTemplate(searchApplication, request);
                 SearchRequest searchRequest = new SearchRequest(searchApplication.indices()).source(sourceBuilder);
 
                 client.execute(
                     SearchAction.INSTANCE,
                     searchRequest,
                     listener.delegateFailure((l2, searchResponse) -> l2.onResponse(searchResponse))
                 );
-            } catch (IOException e) {
-                l.onFailure(e);
+            } catch (Exception e) {
+                listener.onFailure(e);
             }
         }));
     }
@@ -102,8 +103,17 @@ private static Map<String, Object> mergeTemplateParams(QuerySearchApplicationAct
         return mergedTemplateParams;
     }
 
-    private SearchSourceBuilder renderTemplate(Script script, Map<String, Object> templateParams) throws IOException {
-        TemplateScript compiledTemplate = scriptService.compile(script, TemplateScript.CONTEXT).newInstance(templateParams);
+    private SearchSourceBuilder renderTemplate(SearchApplication searchApplication, QuerySearchApplicationAction.Request request)
+        throws IOException, ValidationException {
+
+        final SearchApplicationTemplate template = searchApplication.searchApplicationTemplate();
+        final Map<String, Object> queryParams = request.queryParams();
+        final Script script = template.script();
+
+        template.validateTemplateParams(queryParams);
+
+        TemplateScript compiledTemplate = scriptService.compile(script, TemplateScript.CONTEXT)
+            .newInstance(mergeTemplateParams(request, script));
         String requestSource = compiledTemplate.execute();
 
         XContentParserConfiguration parserConfig = XContentParserConfiguration.EMPTY.withRegistry(xContentRegistry)