You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by ra...@apache.org on 2020/09/16 14:51:19 UTC

[sling-org-apache-sling-graphql-core] branch master updated (4789c4a -> e675ad7)

This is an automated email from the ASF dual-hosted git repository.

radu pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-graphql-core.git.


    from 4789c4a  trivial: reformatted some README.md sections
     new f3f9414  SLING-9724 - Validate queries before persisting
     new e675ad7  SLING-9724 - Validate queries before persisting

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 pom.xml                                            |  14 +--
 .../graphql/core/engine/GraphQLResourceQuery.java  | 135 ++++++++++++---------
 .../graphql/core/scalars/SlingScalarsProvider.java |   2 +-
 .../core/scripting/GraphQLScriptEngine.java        |   6 +-
 .../sling/graphql/core/servlet/GraphQLServlet.java |  34 ++++--
 .../graphql/core/engine/CustomScalarsTest.java     |   4 +-
 .../core/engine/GraphQLResourceQueryTest.java      |  11 +-
 .../graphql/core/engine/ResourceQueryTestBase.java |  10 +-
 .../graphql/core/it/GraphQLCoreTestSupport.java    |   2 +-
 .../sling/graphql/core/it/GraphQLServletIT.java    |  24 ++++
 .../core/schema/SchemaDescriptionsTest.java        |   7 +-
 11 files changed, 155 insertions(+), 94 deletions(-)


[sling-org-apache-sling-graphql-core] 02/02: SLING-9724 - Validate queries before persisting

Posted by ra...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

radu pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-graphql-core.git

commit e675ad723fbb1f2718f1913a17c805df50525260
Author: Radu Cotescu <co...@adobe.com>
AuthorDate: Wed Sep 16 16:34:56 2020 +0200

    SLING-9724 - Validate queries before persisting
    
    * validate queries before persisting
    * made GraphQLResourceQuery a class with static methods, since there was no instance behaviour to begin with
---
 pom.xml                                            |  12 +-
 .../graphql/core/engine/GraphQLResourceQuery.java  | 135 ++++++++++++---------
 .../core/scripting/GraphQLScriptEngine.java        |   6 +-
 .../sling/graphql/core/servlet/GraphQLServlet.java |  34 ++++--
 .../graphql/core/engine/CustomScalarsTest.java     |   4 +-
 .../core/engine/GraphQLResourceQueryTest.java      |  11 +-
 .../graphql/core/engine/ResourceQueryTestBase.java |  10 +-
 .../graphql/core/it/GraphQLCoreTestSupport.java    |   2 +-
 .../sling/graphql/core/it/GraphQLServletIT.java    |  24 ++++
 .../core/schema/SchemaDescriptionsTest.java        |   7 +-
 10 files changed, 153 insertions(+), 92 deletions(-)

diff --git a/pom.xml b/pom.xml
index 3e1cc03..6f36856 100644
--- a/pom.xml
+++ b/pom.xml
@@ -181,12 +181,6 @@
       <scope>provided</scope>
     </dependency>
     <dependency>
-      <groupId>org.apache.sling</groupId>
-      <artifactId>org.apache.sling.testing.paxexam</artifactId>
-      <version>3.1.0</version>
-      <scope>test</scope>
-    </dependency>
-    <dependency>
       <groupId>commons-io</groupId>
       <artifactId>commons-io</artifactId>
       <version>2.4</version>
@@ -243,6 +237,12 @@
       <scope>test</scope>
     </dependency>
     <dependency>
+      <groupId>org.apache.sling</groupId>
+      <artifactId>org.apache.sling.testing.paxexam</artifactId>
+      <version>3.1.0</version>
+      <scope>test</scope>
+    </dependency>
+    <dependency>
       <groupId>org.ops4j.pax.exam</groupId>
       <artifactId>pax-exam</artifactId>
       <version>${org.ops4j.pax.exam.version}</version>
diff --git a/src/main/java/org/apache/sling/graphql/core/engine/GraphQLResourceQuery.java b/src/main/java/org/apache/sling/graphql/core/engine/GraphQLResourceQuery.java
index 711561d..2175f9d 100644
--- a/src/main/java/org/apache/sling/graphql/core/engine/GraphQLResourceQuery.java
+++ b/src/main/java/org/apache/sling/graphql/core/engine/GraphQLResourceQuery.java
@@ -19,39 +19,42 @@
 
 package org.apache.sling.graphql.core.engine;
 
-import javax.script.ScriptException;
+import java.io.IOException;
+import java.util.List;
+import java.util.Map;
+import java.util.regex.Pattern;
 
-import graphql.ExecutionInput;
-import graphql.language.Argument;
-import graphql.language.Directive;
-import graphql.language.FieldDefinition;
-import graphql.language.ObjectTypeDefinition;
-import graphql.language.StringValue;
+import javax.script.ScriptException;
 
 import org.apache.sling.api.resource.Resource;
 import org.apache.sling.graphql.api.SchemaProvider;
 import org.apache.sling.graphql.api.SlingDataFetcher;
 import org.apache.sling.graphql.core.scalars.SlingScalarsProvider;
+import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import graphql.ExecutionInput;
 import graphql.ExecutionResult;
 import graphql.GraphQL;
+import graphql.ParseAndValidate;
+import graphql.language.Argument;
+import graphql.language.Directive;
+import graphql.language.FieldDefinition;
+import graphql.language.ObjectTypeDefinition;
+import graphql.language.StringValue;
 import graphql.schema.DataFetcher;
+import graphql.schema.GraphQLScalarType;
 import graphql.schema.GraphQLSchema;
 import graphql.schema.idl.RuntimeWiring;
 import graphql.schema.idl.SchemaGenerator;
 import graphql.schema.idl.SchemaParser;
 import graphql.schema.idl.TypeDefinitionRegistry;
-import graphql.schema.GraphQLScalarType;
-
-import java.io.IOException;
-import java.util.Collections;
-import java.util.List;
-import java.util.Map;
-import java.util.regex.Pattern;
 
-/** Run a GraphQL query in the context of a Sling Resource */
+/**
+ * Run a GraphQL query in the context of a Sling Resource
+ */
 public class GraphQLResourceQuery {
 
     public static final String FETCHER_DIRECTIVE = "fetcher";
@@ -60,61 +63,66 @@ public class GraphQLResourceQuery {
     public static final String FETCHER_SOURCE = "source";
     private static final Pattern FETCHER_NAME_PATTERN = Pattern.compile("\\w+(/\\w+)+");
 
-    private final Logger log = LoggerFactory.getLogger(getClass());
+    private static final Logger LOGGER = LoggerFactory.getLogger(GraphQLResourceQuery.class);
 
-    public ExecutionResult executeQuery(SchemaProvider schemaProvider, 
-                                        SlingDataFetcherSelector fetchersSelector,
-                                        SlingScalarsProvider scalarsProvider,
-                                        Resource r,
-                                        String [] requestSelectors,
-                                        String query, Map<String, Object> variables) throws ScriptException {
-        if(r == null) {
-            throw new ScriptException("Resource is null");
-        }
-        if(query == null) {
-            throw new ScriptException("Query is null");
-        }
-        if(schemaProvider == null) {
-            throw new ScriptException("SchemaProvider is null");
-        }
-        if(fetchersSelector == null) {
-            throw new ScriptException("DataFetcherSelector is null");
-        }
-        if(variables == null) {
-            variables = Collections.emptyMap();
-        }
+    private GraphQLResourceQuery() {}
 
+    public static ExecutionResult executeQuery(@NotNull SchemaProvider schemaProvider,
+                                               @NotNull SlingDataFetcherSelector fetchersSelector,
+                                               @NotNull SlingScalarsProvider scalarsProvider,
+                                               @NotNull Resource r,
+                                               @NotNull String[] requestSelectors,
+                                               @NotNull String query, @NotNull Map<String, Object> variables) throws ScriptException {
         String schemaDef = null;
         try {
-            schemaDef = schemaProvider.getSchema(r, requestSelectors);
-        } catch(Exception e) {
-            final ScriptException up = new ScriptException("Schema provider failed");
-            up.initCause(e);
-            log.info("Schema provider Exception", up);
-            throw up;
-        }
-        log.debug("Resource {} maps to GQL schema {}", r.getPath(), schemaDef);
-        try {
+            schemaDef = prepareSchemaDefinition(schemaProvider, r, requestSelectors);
+            LOGGER.debug("Resource {} maps to GQL schema {}", r.getPath(), schemaDef);
             final GraphQLSchema schema = buildSchema(schemaDef, fetchersSelector, scalarsProvider, r);
             final GraphQL graphQL = GraphQL.newGraphQL(schema).build();
-            log.debug("Executing query\n[{}]\nat [{}] with variables [{}]", query, r.getPath(), variables);
+            LOGGER.debug("Executing query\n[{}]\nat [{}] with variables [{}]", query, r.getPath(), variables);
             ExecutionInput ei = ExecutionInput.newExecutionInput()
-                .query(query)
-                .variables(variables)
-                .build();
+                    .query(query)
+                    .variables(variables)
+                    .build();
             final ExecutionResult result = graphQL.execute(ei);
-            log.debug("ExecutionResult.isDataPresent={}", result.isDataPresent());
+            LOGGER.debug("ExecutionResult.isDataPresent={}", result.isDataPresent());
             return result;
+        } catch (ScriptException e) {
+            throw e;
         } catch(Exception e) {
             final ScriptException up = new ScriptException(
                 String.format("Query failed for Resource %s: schema=%s, query=%s", r.getPath(), schemaDef, query));
             up.initCause(e);
-            log.info("GraphQL Query Exception", up);
+            LOGGER.info("GraphQL Query Exception", up);
             throw up;                
         }
     }
 
-    private GraphQLSchema buildSchema(String sdl, SlingDataFetcherSelector fetchers, SlingScalarsProvider scalarsProvider, Resource currentResource) throws IOException {
+    public static boolean isQueryValid(@NotNull SchemaProvider schemaProvider,
+                                       @NotNull SlingDataFetcherSelector fetchersSelector,
+                                       @NotNull SlingScalarsProvider scalarsProvider,
+                                       @NotNull Resource r,
+                                       @NotNull String[] requestSelectors,
+                                       @NotNull String query, Map<String, Object> variables) {
+
+        try {
+            String schemaDef = prepareSchemaDefinition(schemaProvider, r, requestSelectors);
+            LOGGER.debug("Resource {} maps to GQL schema {}", r.getPath(), schemaDef);
+            final GraphQLSchema schema =
+                    buildSchema(schemaDef, fetchersSelector, scalarsProvider, r);
+            ExecutionInput executionInput = ExecutionInput.newExecutionInput()
+                    .query(query)
+                    .variables(variables)
+                    .build();
+            return !ParseAndValidate.parseAndValidate(schema, executionInput).isFailure();
+        } catch (Exception e) {
+            LOGGER.error(String.format("Invalid query: %s.", query), e);
+            return false;
+        }
+    }
+
+    private static GraphQLSchema buildSchema(String sdl, SlingDataFetcherSelector fetchers, SlingScalarsProvider scalarsProvider,
+                                             Resource currentResource) {
         TypeDefinitionRegistry typeRegistry = new SchemaParser().parse(sdl);
         Iterable<GraphQLScalarType> scalars = scalarsProvider.getCustomScalars(typeRegistry.scalars());
         RuntimeWiring runtimeWiring = buildWiring(typeRegistry, fetchers, scalars, currentResource);
@@ -122,8 +130,8 @@ public class GraphQLResourceQuery {
         return schemaGenerator.makeExecutableSchema(typeRegistry, runtimeWiring);
     }
 
-    private RuntimeWiring buildWiring(TypeDefinitionRegistry typeRegistry, SlingDataFetcherSelector fetchers, Iterable<GraphQLScalarType> scalars, Resource r)
-        throws IOException {
+    private static RuntimeWiring buildWiring(TypeDefinitionRegistry typeRegistry, SlingDataFetcherSelector fetchers,
+                                       Iterable<GraphQLScalarType> scalars, Resource r) {
         List<ObjectTypeDefinition> types = typeRegistry.getTypes(ObjectTypeDefinition.class);
         RuntimeWiring.Builder builder = RuntimeWiring.newRuntimeWiring();
         for (ObjectTypeDefinition type : types) {
@@ -146,7 +154,7 @@ public class GraphQLResourceQuery {
         return builder.build();
     }
 
-    private String getDirectiveArgumentValue(Directive d, String name) {
+    private static String getDirectiveArgumentValue(Directive d, String name) {
         final Argument a = d.getArgument(name);
         if(a != null && a.getValue() instanceof StringValue) {
             return ((StringValue)a.getValue()).getValue();
@@ -165,7 +173,7 @@ public class GraphQLResourceQuery {
         return name;
     }
 
-    private DataFetcher<Object> getDataFetcher(FieldDefinition field, 
+    private static DataFetcher<Object> getDataFetcher(FieldDefinition field,
         SlingDataFetcherSelector fetchers, Resource currentResource) throws IOException {
         DataFetcher<Object> result = null;
         final Directive d =field.getDirective(FETCHER_DIRECTIVE);
@@ -180,4 +188,17 @@ public class GraphQLResourceQuery {
         }
         return result;
     }
+
+    private static @Nullable String prepareSchemaDefinition(@NotNull SchemaProvider schemaProvider,
+                                                            @NotNull Resource resource,
+                                                            @NotNull String[] selectors) throws ScriptException {
+        try {
+            return schemaProvider.getSchema(resource, selectors);
+        } catch (Exception e) {
+            final ScriptException up = new ScriptException("Schema provider failed");
+            up.initCause(e);
+            LOGGER.info("Schema provider Exception", up);
+            throw up;
+        }
+    }
 }
diff --git a/src/main/java/org/apache/sling/graphql/core/scripting/GraphQLScriptEngine.java b/src/main/java/org/apache/sling/graphql/core/scripting/GraphQLScriptEngine.java
index b7eaa57..b569260 100644
--- a/src/main/java/org/apache/sling/graphql/core/scripting/GraphQLScriptEngine.java
+++ b/src/main/java/org/apache/sling/graphql/core/scripting/GraphQLScriptEngine.java
@@ -25,6 +25,7 @@ import java.io.Reader;
 import java.io.StringReader;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.List;
 
 import javax.script.AbstractScriptEngine;
@@ -59,13 +60,12 @@ public class GraphQLScriptEngine extends AbstractScriptEngine {
     @Override
     public Object eval(Reader reader, ScriptContext context) throws ScriptException {
         try {
-            final GraphQLResourceQuery q = new GraphQLResourceQuery();
 
             final Resource resource = (Resource) context.getBindings(ScriptContext.ENGINE_SCOPE)
                     .get(SlingBindings.RESOURCE);
             final String [] selectors = getRequestSelectors(resource);
-            final ExecutionResult result = q.executeQuery(factory.getSchemaProviders(), factory.getdataFetcherSelector(),
-                    factory.getScalarsProvider(), resource, selectors, IOUtils.toString(reader), null);
+            final ExecutionResult result = GraphQLResourceQuery.executeQuery(factory.getSchemaProviders(), factory.getdataFetcherSelector(),
+                    factory.getScalarsProvider(), resource, selectors, IOUtils.toString(reader), Collections.emptyMap());
             final PrintWriter out = (PrintWriter) context.getBindings(ScriptContext.ENGINE_SCOPE).get(SlingBindings.OUT);
             jsonSerializer.sendJSON(out, result);
         } catch(Exception e) {
diff --git a/src/main/java/org/apache/sling/graphql/core/servlet/GraphQLServlet.java b/src/main/java/org/apache/sling/graphql/core/servlet/GraphQLServlet.java
index 775bb33..640e5c0 100644
--- a/src/main/java/org/apache/sling/graphql/core/servlet/GraphQLServlet.java
+++ b/src/main/java/org/apache/sling/graphql/core/servlet/GraphQLServlet.java
@@ -53,11 +53,11 @@ import graphql.ExecutionResult;
 
 /** Servlet that can be activated to implement the standard
  *  GraphQL "protocol" as per https://graphql.org/learn/serving-over-http/
- * 
+ *
  *  This servlet is only active if the corresponding OSGi configurations
  *  are created. This allows is to be mounted either on a path to support
  *  the "traditional" GraphQL single-endpoint mode, or on specific resource
- *  types and selectors to turn specific Sling Resources into GraphQL 
+ *  types and selectors to turn specific Sling Resources into GraphQL
  *  endpoints.
  */
 
@@ -201,15 +201,11 @@ public class GraphQLServlet extends SlingAllMethodsServlet {
     }
 
     @Override
-    public void doPost(SlingHttpServletRequest request, SlingHttpServletResponse response) throws IOException {
+    public void doPost(@NotNull SlingHttpServletRequest request, @NotNull SlingHttpServletResponse response) throws IOException {
         String suffix = request.getRequestPathInfo().getSuffix();
         if (suffix != null) {
             if (StringUtils.isNotEmpty(suffixPersisted) && suffix.equals(suffixPersisted)) {
-                String query = IOUtils.toString(request.getReader());
-                String hash = cacheProvider.cacheQuery(query, request.getResource().getResourceType(),
-                        request.getRequestPathInfo().getSelectorString());
-                response.addHeader("Location", getLocationHeaderValue(request, hash));
-                response.setStatus(HttpServletResponse.SC_CREATED);
+                doPostPersistedQuery(request, response);
             } else {
                 response.sendError(HttpServletResponse.SC_BAD_REQUEST);
             }
@@ -218,6 +214,22 @@ public class GraphQLServlet extends SlingAllMethodsServlet {
         }
     }
 
+    private void doPostPersistedQuery(@NotNull SlingHttpServletRequest request, @NotNull SlingHttpServletResponse response)
+            throws IOException {
+        String rawQuery = IOUtils.toString(request.getReader());
+        QueryParser.Result query = QueryParser.fromJSON(rawQuery);
+        if (GraphQLResourceQuery.isQueryValid(schemaProviders, dataFetcherSelector, scalarsProvider, request.getResource(),
+                request.getRequestPathInfo().getSelectors(), query.getQuery(), query.getVariables())) {
+
+            String hash = cacheProvider.cacheQuery(rawQuery, request.getResource().getResourceType(),
+                    request.getRequestPathInfo().getSelectorString());
+            response.addHeader("Location", getLocationHeaderValue(request, hash));
+            response.setStatus(HttpServletResponse.SC_CREATED);
+        } else {
+            response.sendError(HttpServletResponse.SC_BAD_REQUEST, "Invalid GraphQL query.");
+        }
+    }
+
     private void execute(Resource resource, SlingHttpServletRequest request, SlingHttpServletResponse response) throws IOException {
         response.setContentType("application/json");
         response.setCharacterEncoding("UTF-8");
@@ -233,8 +245,7 @@ public class GraphQLServlet extends SlingAllMethodsServlet {
         }
 
         try {
-            final GraphQLResourceQuery q = new GraphQLResourceQuery();
-            final ExecutionResult executionResult = q.executeQuery(schemaProviders, dataFetcherSelector, scalarsProvider,
+            final ExecutionResult executionResult = GraphQLResourceQuery.executeQuery(schemaProviders, dataFetcherSelector, scalarsProvider,
                 resource, request.getRequestPathInfo().getSelectors(), query, result.getVariables());
             jsonSerializer.sendJSON(response.getWriter(), executionResult);
         } catch(Exception ex) {
@@ -247,8 +258,7 @@ public class GraphQLServlet extends SlingAllMethodsServlet {
         response.setCharacterEncoding("UTF-8");
         try {
             final QueryParser.Result result = QueryParser.fromJSON(persistedQuery);
-            final GraphQLResourceQuery q = new GraphQLResourceQuery();
-            final ExecutionResult executionResult = q.executeQuery(schemaProviders, dataFetcherSelector, scalarsProvider,
+            final ExecutionResult executionResult = GraphQLResourceQuery.executeQuery(schemaProviders, dataFetcherSelector, scalarsProvider,
                     request.getResource(), request.getRequestPathInfo().getSelectors(), result.getQuery(), result.getVariables());
             jsonSerializer.sendJSON(response.getWriter(), executionResult);
         } catch(Exception ex) {
diff --git a/src/test/java/org/apache/sling/graphql/core/engine/CustomScalarsTest.java b/src/test/java/org/apache/sling/graphql/core/engine/CustomScalarsTest.java
index ad524b3..a0a1354 100644
--- a/src/test/java/org/apache/sling/graphql/core/engine/CustomScalarsTest.java
+++ b/src/test/java/org/apache/sling/graphql/core/engine/CustomScalarsTest.java
@@ -52,7 +52,7 @@ public class CustomScalarsTest extends ResourceQueryTestBase {
     }
 
     @Test
-    public void urlSyntaxError() throws Exception {
+    public void urlSyntaxError() {
         final String url = "This is not an URL!";
         final String query = String.format("{ address (url: \"%s\") { url hostname } }", url);
         try {
@@ -63,4 +63,4 @@ public class CustomScalarsTest extends ResourceQueryTestBase {
         }
     }
 
-}
\ No newline at end of file
+}
diff --git a/src/test/java/org/apache/sling/graphql/core/engine/GraphQLResourceQueryTest.java b/src/test/java/org/apache/sling/graphql/core/engine/GraphQLResourceQueryTest.java
index a11ad64..251e9d2 100644
--- a/src/test/java/org/apache/sling/graphql/core/engine/GraphQLResourceQueryTest.java
+++ b/src/test/java/org/apache/sling/graphql/core/engine/GraphQLResourceQueryTest.java
@@ -27,6 +27,7 @@ import static org.hamcrest.Matchers.is;
 import static org.hamcrest.Matchers.nullValue;
 
 import java.io.IOException;
+import java.util.Collections;
 import java.util.Dictionary;
 import java.util.Hashtable;
 
@@ -92,7 +93,8 @@ public class GraphQLResourceQueryTest extends ResourceQueryTestBase {
     public void dataFetcherFailureTest() throws Exception {
         try {
             final String stmt = "{ currentResource { failure } }";
-            new GraphQLResourceQuery().executeQuery(schemaProvider, dataFetchersSelector, scalarsProvider, resource, null, stmt, null);
+            GraphQLResourceQuery.executeQuery(schemaProvider, dataFetchersSelector, scalarsProvider, resource, new String[] {}, stmt,
+                    Collections.emptyMap());
         } catch(RuntimeException rex) {
             assertThat(rex.getMessage(), equalTo("FailureDataFetcher"));
         }
@@ -113,7 +115,7 @@ public class GraphQLResourceQueryTest extends ResourceQueryTestBase {
         schemaProvider = new MockSchemaProvider("failing-schema");
         final ServiceRegistration<?> reg = TestUtil.registerSlingDataFetcher(context.bundleContext(), "missingSlash", new EchoDataFetcher(42));
         try {
-            queryJSON("{ currentResource { missingSlash } }", null);
+            queryJSON("{ currentResource { missingSlash } }", new String[] {});
             fail("Expected query to fail");
         } catch(Exception e) {
             TestUtil.assertNestedException(e, IOException.class, "does not match");
@@ -124,10 +126,11 @@ public class GraphQLResourceQueryTest extends ResourceQueryTestBase {
 
     @Test
     public void scriptedFetcherProviderTest() throws Exception {
-        final String json = queryJSON("{ currentResource { path } scriptedFetcher (testing: \"1, 2, 3\") { boolValue resourcePath testingArgument } }", null);
+        final String json = queryJSON("{ currentResource { path } scriptedFetcher (testing: \"1, 2, 3\") { boolValue resourcePath " +
+                "testingArgument } }", new String[] {});
         assertThat(json, hasJsonPath("$.data.currentResource.path", equalTo(resource.getPath())));
         assertThat(json, hasJsonPath("$.data.scriptedFetcher.boolValue", equalTo(true)));
         assertThat(json, hasJsonPath("$.data.scriptedFetcher.resourcePath", equalTo(resource.getPath())));
         assertThat(json, hasJsonPath("$.data.scriptedFetcher.testingArgument", equalTo("1, 2, 3")));
     }
-}
\ No newline at end of file
+}
diff --git a/src/test/java/org/apache/sling/graphql/core/engine/ResourceQueryTestBase.java b/src/test/java/org/apache/sling/graphql/core/engine/ResourceQueryTestBase.java
index 5c6504d..b36174b 100644
--- a/src/test/java/org/apache/sling/graphql/core/engine/ResourceQueryTestBase.java
+++ b/src/test/java/org/apache/sling/graphql/core/engine/ResourceQueryTestBase.java
@@ -19,6 +19,8 @@
 package org.apache.sling.graphql.core.engine;
 
 import static org.junit.Assert.assertTrue;
+
+import java.util.Collections;
 import java.util.Dictionary;
 import java.util.Hashtable;
 import java.util.UUID;
@@ -76,12 +78,12 @@ public abstract class ResourceQueryTestBase {
     }
 
     protected String queryJSON(String stmt) throws Exception {
-        return queryJSON(stmt, null);
+        return queryJSON(stmt, new String[]{});
     }
 
     protected String queryJSON(String stmt, String [] selectors) throws Exception {
-        final ExecutionResult result = new GraphQLResourceQuery().executeQuery(schemaProvider,
-            dataFetchersSelector, scalarsProvider, resource, selectors, stmt, null);
+        final ExecutionResult result = GraphQLResourceQuery.executeQuery(schemaProvider,
+            dataFetchersSelector, scalarsProvider, resource, selectors, stmt, Collections.emptyMap());
         assertTrue("Expecting no errors: " + result.getErrors(), result.getErrors().isEmpty());
         return new JsonSerializer().toJSON(result);
     }
@@ -92,4 +94,4 @@ public abstract class ResourceQueryTestBase {
     protected String getTestSchemaName() {
         return "test-schema";
     }
-}
\ No newline at end of file
+}
diff --git a/src/test/java/org/apache/sling/graphql/core/it/GraphQLCoreTestSupport.java b/src/test/java/org/apache/sling/graphql/core/it/GraphQLCoreTestSupport.java
index 60d10f0..1ebe0dc 100644
--- a/src/test/java/org/apache/sling/graphql/core/it/GraphQLCoreTestSupport.java
+++ b/src/test/java/org/apache/sling/graphql/core/it/GraphQLCoreTestSupport.java
@@ -215,7 +215,7 @@ public abstract class GraphQLCoreTestSupport extends TestSupport {
             body.put("variables", variables);
         }
 
-        return executeRequest("POST", path + "/persisted", null, "application/json", new StringReader(toJSON(body)), 201);
+        return executeRequest("POST", path + "/persisted", null, "application/json", new StringReader(toJSON(body)), -1);
     }
 
     protected String toJSON(Object source) {
diff --git a/src/test/java/org/apache/sling/graphql/core/it/GraphQLServletIT.java b/src/test/java/org/apache/sling/graphql/core/it/GraphQLServletIT.java
index ced345b..2eb1b6a 100644
--- a/src/test/java/org/apache/sling/graphql/core/it/GraphQLServletIT.java
+++ b/src/test/java/org/apache/sling/graphql/core/it/GraphQLServletIT.java
@@ -26,6 +26,7 @@ import java.util.Map;
 import javax.inject.Inject;
 
 import org.apache.commons.io.IOUtils;
+import org.apache.commons.lang3.StringUtils;
 import org.apache.http.Header;
 import org.apache.http.HttpHeaders;
 import org.apache.http.HttpHost;
@@ -64,6 +65,7 @@ import static org.hamcrest.Matchers.equalTo;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertThat;
+import static org.junit.Assert.assertTrue;
 import static org.ops4j.pax.exam.cm.ConfigurationAdminOptions.factoryConfiguration;
 
 @RunWith(PaxExam.class)
@@ -141,6 +143,28 @@ public class GraphQLServletIT extends GraphQLCoreTestSupport {
     }
 
     @Test
+    public void testPersistingInvalidQueries() throws Exception {
+        HttpHost targetHost = new HttpHost("localhost", httpPort(), "http");
+        HttpClientContext context = HttpClientContext.create();
+        Map<String, Object> queryMap = new HashMap<>();
+        queryMap.put("query", "{ current { resourceType name } }");
+        queryMap.put("variables", Collections.emptyMap());
+        String json = toJSON(queryMap);
+        try (CloseableHttpClient client = HttpClients.createDefault()) {
+
+            HttpPost post = new HttpPost("http://localhost:" + httpPort() + "/graphql/two.gql/persisted");
+            post.setEntity(new ByteArrayEntity(json.getBytes(), ContentType.APPLICATION_JSON));
+
+            try (CloseableHttpResponse postResponse = client.execute(targetHost, post, context)) {
+                assertEquals("Did not expect to persist an invalid query.", 400, postResponse.getStatusLine().getStatusCode());
+                String content = IOUtils.toString(postResponse.getEntity().getContent());
+                assertTrue("Expected a Sling error page.", StringUtils.isNotEmpty(content));
+                assertTrue("Expected to find the failure reason in the Sling error page.", content.contains("400 Invalid GraphQL query."));
+            }
+        }
+    }
+
+    @Test
     public void testPersistedQueriesWithAuthorization() throws Exception {
         HttpHost targetHost = new HttpHost("localhost", httpPort(), "http");
         CredentialsProvider credsProvider = new BasicCredentialsProvider();
diff --git a/src/test/java/org/apache/sling/graphql/core/schema/SchemaDescriptionsTest.java b/src/test/java/org/apache/sling/graphql/core/schema/SchemaDescriptionsTest.java
index f72fdb7..82cbf66 100644
--- a/src/test/java/org/apache/sling/graphql/core/schema/SchemaDescriptionsTest.java
+++ b/src/test/java/org/apache/sling/graphql/core/schema/SchemaDescriptionsTest.java
@@ -23,6 +23,7 @@ import static com.jayway.jsonpath.matchers.JsonPathMatchers.hasJsonPath;
 import static org.junit.Assert.assertThat;
 import static org.junit.Assert.assertTrue;
 
+import java.util.Collections;
 import java.util.UUID;
 
 import org.apache.sling.api.resource.Resource;
@@ -113,8 +114,8 @@ public class SchemaDescriptionsTest {
     }
 
     private String queryJSON(String stmt) throws Exception {
-        final ExecutionResult result = new GraphQLResourceQuery().executeQuery(schemaProvider,
-            dataFetchersSelector, scalarsProvider, resource, null, stmt, null);
+        final ExecutionResult result = GraphQLResourceQuery.executeQuery(schemaProvider,
+            dataFetchersSelector, scalarsProvider, resource, new String[] {}, stmt, Collections.emptyMap());
         assertTrue("Expecting no errors: " + result.getErrors(), result.getErrors().isEmpty());
         return new JsonSerializer().toJSON(result);
     }
@@ -147,4 +148,4 @@ public class SchemaDescriptionsTest {
         assertFieldDescription("SlingResource", "failure", "Failure message");
         assertFieldDescription("Test", "test", "null");
     }
-}
\ No newline at end of file
+}


[sling-org-apache-sling-graphql-core] 01/02: SLING-9724 - Validate queries before persisting

Posted by ra...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

radu pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-graphql-core.git

commit f3f9414b4d0bd9c482393b94fe00468aaa48a295
Author: Radu Cotescu <co...@adobe.com>
AuthorDate: Wed Sep 16 16:30:29 2020 +0200

    SLING-9724 - Validate queries before persisting
    
    * updated to graphql-java 15 for the new graphql.ParseAndValidate class
---
 pom.xml                                                                 | 2 +-
 .../org/apache/sling/graphql/core/scalars/SlingScalarsProvider.java     | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/pom.xml b/pom.xml
index 9c52ff7..3e1cc03 100644
--- a/pom.xml
+++ b/pom.xml
@@ -135,7 +135,7 @@
     <dependency>
       <groupId>com.graphql-java</groupId>
       <artifactId>graphql-java</artifactId>
-      <version>14.0</version>
+      <version>15.0</version>
       <scope>provided</scope>
     </dependency>
     <dependency>
diff --git a/src/main/java/org/apache/sling/graphql/core/scalars/SlingScalarsProvider.java b/src/main/java/org/apache/sling/graphql/core/scalars/SlingScalarsProvider.java
index 8ffde2e..a6771c6 100644
--- a/src/main/java/org/apache/sling/graphql/core/scalars/SlingScalarsProvider.java
+++ b/src/main/java/org/apache/sling/graphql/core/scalars/SlingScalarsProvider.java
@@ -57,7 +57,7 @@ public class SlingScalarsProvider {
     private GraphQLScalarType getScalar(String name) {
 
         // Ignore standard scalars
-        if(ScalarInfo.STANDARD_SCALAR_DEFINITIONS.containsKey(name)) {
+        if(ScalarInfo.isGraphqlSpecifiedScalar(name)) {
             return null;
         }