From 4317933751521aa35a47f6089770babf67eabaf2 Mon Sep 17 00:00:00 2001
From: Radu Cotescu <170911+raducotescu@users.noreply.github.com>
Date: Mon, 29 Aug 2022 17:58:48 +0200
Subject: [PATCH] SLING-10883 - Update the GraphQL implementation to use the
Builder API for internal requests
* refactored code to switch to the o.a.s.api.request.builder API
---
pom.xml | 58 +++++++---
.../core/schema/DefaultSchemaProvider.java | 31 ++++--
.../core/it/GraphQLCoreTestSupport.java | 103 ++++++++++++++----
.../graphql/core/it/GraphQLServletIT.java | 8 +-
.../core/it/GraphQLServletNoConfigIT.java | 4 +-
5 files changed, 153 insertions(+), 51 deletions(-)
diff --git a/pom.xml b/pom.xml
index 7bc39ef..567ebe4 100644
--- a/pom.xml
+++ b/pom.xml
@@ -136,13 +136,13 @@
org.apache.sling
org.apache.sling.api
- 2.18.4
+ 2.24.0
provided
org.apache.sling
org.apache.sling.engine
- 2.6.22
+ 2.7.10
provided
@@ -181,12 +181,6 @@
2.1.12
provided
-
- org.apache.sling
- org.apache.sling.scripting.core
- 2.0.30
- provided
-
org.apache.sling
org.apache.sling.commons.osgi
@@ -209,12 +203,6 @@
slf4j-api
provided
-
- org.apache.sling
- org.apache.sling.servlet-helpers
- 1.4.2
- provided
-
junit
@@ -315,6 +303,48 @@
1.2.3
test
+
+ org.apache.sling
+ org.apache.sling.jcr.jackrabbit.usermanager
+ 2.2.16
+ test
+
+
+ org.apache.sling
+ org.apache.sling.resourceresolver
+ 1.8.0
+ test
+
+
+ org.apache.sling
+ org.apache.sling.servlets.resolver
+ 2.9.0
+ test
+
+
+ org.apache.sling
+ org.apache.sling.commons.compiler
+ 2.4.0
+ test
+
+
+ org.apache.sling
+ org.apache.sling.scripting.spi
+ 1.0.2
+ test
+
+
+ org.apache.sling
+ org.apache.sling.scripting.core
+ 2.4.0
+ test
+
+
+ org.osgi
+ org.osgi.util.converter
+ 1.0.0
+ test
+
diff --git a/src/main/java/org/apache/sling/graphql/core/schema/DefaultSchemaProvider.java b/src/main/java/org/apache/sling/graphql/core/schema/DefaultSchemaProvider.java
index 2574303..d0318f2 100644
--- a/src/main/java/org/apache/sling/graphql/core/schema/DefaultSchemaProvider.java
+++ b/src/main/java/org/apache/sling/graphql/core/schema/DefaultSchemaProvider.java
@@ -22,13 +22,18 @@
import java.io.IOException;
+import javax.servlet.Servlet;
+import javax.servlet.ServletException;
import javax.servlet.http.HttpServletResponse;
+import org.apache.sling.api.SlingHttpServletRequest;
+import org.apache.sling.api.SlingHttpServletResponse;
+import org.apache.sling.api.request.builder.Builders;
+import org.apache.sling.api.request.builder.SlingHttpServletRequestBuilder;
+import org.apache.sling.api.request.builder.SlingHttpServletResponseResult;
import org.apache.sling.api.resource.Resource;
import org.apache.sling.api.servlets.ServletResolver;
import org.apache.sling.graphql.api.SchemaProvider;
-import org.apache.sling.servlethelpers.internalrequests.InternalRequest;
-import org.apache.sling.servlethelpers.internalrequests.ServletInternalRequest;
import org.osgi.framework.Constants;
import org.osgi.service.component.annotations.Component;
import org.osgi.service.component.annotations.Reference;
@@ -53,16 +58,20 @@ public class DefaultSchemaProvider implements SchemaProvider {
@Override
public String getSchema(Resource r, String [] selectors) throws IOException {
- final InternalRequest req =
- new ServletInternalRequest(servletResolver, r)
- .withSelectors(selectors)
- .withExtension(SCHEMA_EXTENSION)
- ;
-
+ final SlingHttpServletRequest req =
+ Builders.newRequestBuilder(r).withSelectors(selectors).withExtension(SCHEMA_EXTENSION).build();
+ final SlingHttpServletResponseResult response = Builders.newResponseBuilder().build();
+ try {
+ Servlet servlet = servletResolver.resolveServlet(req);
+ if (servlet != null) {
+ servlet.service(req, response);
+ }
+ } catch (ServletException e) {
+ LOGGER.error("Unable to retrieve a GraphQL Schema for {}.", r.getPath());
+ }
LOGGER.debug("Getting GraphQL Schema for {}: {}", r.getPath(), req);
-
- if(req.execute().getStatus() == HttpServletResponse.SC_OK) {
- return req.getResponseAsString();
+ if(response.getStatus() == HttpServletResponse.SC_OK) {
+ return response.getOutputAsString();
} else {
return DEFAULT_SCHEMA;
}
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 2d6cc7e..d1e77ee 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
@@ -23,24 +23,33 @@
import java.time.Duration;
import java.time.Instant;
import java.util.ArrayList;
+import java.util.Arrays;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
+import java.util.Objects;
import java.util.UUID;
import javax.inject.Inject;
+import org.apache.commons.io.IOUtils;
import org.apache.commons.lang3.StringUtils;
import org.apache.johnzon.mapper.Mapper;
import org.apache.johnzon.mapper.MapperBuilder;
+import org.apache.sling.api.SlingHttpServletRequest;
+import org.apache.sling.api.request.builder.Builders;
+import org.apache.sling.api.request.builder.SlingHttpServletResponseResult;
+import org.apache.sling.api.resource.Resource;
import org.apache.sling.api.resource.ResourceResolver;
import org.apache.sling.api.resource.ResourceResolverFactory;
+import org.apache.sling.api.resource.ResourceWrapper;
import org.apache.sling.engine.SlingRequestProcessor;
import org.apache.sling.graphql.core.mocks.QueryDataFetcherComponent;
import org.apache.sling.graphql.core.mocks.TestDataFetcherComponent;
-import org.apache.sling.servlethelpers.MockSlingHttpServletResponse;
-import org.apache.sling.servlethelpers.internalrequests.SlingInternalRequest;
+import org.apache.sling.testing.paxexam.SlingOptions;
+import org.apache.sling.testing.paxexam.SlingVersionResolver;
import org.apache.sling.testing.paxexam.TestSupport;
+import org.jetbrains.annotations.NotNull;
import org.junit.Before;
import org.ops4j.pax.exam.Option;
import org.ops4j.pax.exam.ProbeBuilder;
@@ -49,6 +58,8 @@
import org.ops4j.pax.exam.options.extra.VMOption;
import org.ops4j.pax.tinybundles.core.TinyBundle;
import org.osgi.framework.Constants;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
import static org.apache.sling.testing.paxexam.SlingOptions.slingCommonsMetrics;
import static org.apache.sling.testing.paxexam.SlingOptions.slingQuickstartOakTar;
@@ -65,6 +76,8 @@
public abstract class GraphQLCoreTestSupport extends TestSupport {
+ private static final Logger LOGGER = LoggerFactory.getLogger(GraphQLCoreTestSupport.class);
+
private final static int STARTUP_WAIT_SECONDS = 30;
@Inject
@@ -86,6 +99,15 @@ public ModifiableCompositeOption baseConfiguration() {
jacocoCommand = new VMOption(jacocoOpt);
}
+ SlingOptions.versionResolver.setVersionFromProject(SlingVersionResolver.SLING_GROUP_ID, "org.apache.sling.api");
+ SlingOptions.versionResolver.setVersionFromProject(SlingVersionResolver.SLING_GROUP_ID, "org.apache.sling.servlets.resolver");
+ SlingOptions.versionResolver.setVersionFromProject(SlingVersionResolver.SLING_GROUP_ID, "org.apache.sling.engine");
+ SlingOptions.versionResolver.setVersionFromProject(SlingVersionResolver.SLING_GROUP_ID, "org.apache.sling.resourceresolver");
+ SlingOptions.versionResolver.setVersionFromProject(SlingVersionResolver.SLING_GROUP_ID, "org.apache.sling.scripting.api");
+ SlingOptions.versionResolver.setVersionFromProject(SlingVersionResolver.SLING_GROUP_ID, "org.apache.sling.scripting.core");
+ SlingOptions.versionResolver.setVersionFromProject(SlingVersionResolver.SLING_GROUP_ID, "org.apache.sling.commons.compiler");
+ SlingOptions.versionResolver.setVersionFromProject(SlingVersionResolver.SLING_GROUP_ID, "org.apache.sling.jcr.jackrabbit.usermanager");
+
return composite(
when(vmOption != null).useOptions(vmOption),
when(jacocoCommand != null).useOptions(jacocoCommand),
@@ -98,6 +120,8 @@ public ModifiableCompositeOption baseConfiguration() {
.asOption(),
mavenBundle().groupId("org.apache.sling").artifactId("org.apache.sling.servlet-helpers").versionAsInProject(),
mavenBundle().groupId("org.apache.sling").artifactId("org.apache.sling.commons.johnzon").versionAsInProject(),
+ mavenBundle().groupId("org.osgi").artifactId("org.osgi.util.converter").versionAsInProject(), // required for the newer Sling API
+ mavenBundle().groupId(SlingVersionResolver.SLING_GROUP_ID).artifactId("org.apache.sling.scripting.spi").versionAsInProject(),
mavenBundle().groupId("org.apache.johnzon").artifactId("johnzon-mapper").versionAsInProject(),
slingResourcePresence(),
slingCommonsMetrics(),
@@ -168,26 +192,65 @@ public void waitForSling() throws Exception {
fail("Did not get a " + expectedStatus + " status at " + path + " got " + statuses);
}
- protected MockSlingHttpServletResponse executeRequest(final String method,
- final String path, Map params, String contentType,
- Reader body, final int expectedStatus) throws Exception {
+ protected SlingHttpServletResponseResult executeRequest(final String method,
+ final String path, Map params, String contentType,
+ Reader body, final int expectedStatus) throws Exception {
// Admin resolver is fine for testing
@SuppressWarnings("deprecation")
final ResourceResolver resourceResolver = resourceResolverFactory.getAdministrativeResourceResolver(null);
final int [] statusParam = expectedStatus == -1 ? null : new int[] { expectedStatus };
-
- return (MockSlingHttpServletResponse)
- new SlingInternalRequest(resourceResolver, requestProcessor, path)
- .withRequestMethod(method)
- .withParameters(params)
- .withContentType(contentType)
- .withBody(body)
- .execute()
- .checkStatus(statusParam)
- .getResponse()
- ;
+ Resource resource = resourceResolver.resolve(path);
+ String selectorsExtensionSuffix = resource.getResourceMetadata().getResolutionPathInfo();
+ String[] selectors = new String[0];
+ String extension = null;
+ String suffix = null;
+ int firstSlash = selectorsExtensionSuffix.indexOf('/');
+ if (firstSlash != -1) {
+ suffix = selectorsExtensionSuffix.substring(firstSlash);
+ selectorsExtensionSuffix = selectorsExtensionSuffix.substring(0, selectorsExtensionSuffix.length() - suffix.length());
+ }
+ if (selectorsExtensionSuffix.startsWith(".")) {
+ selectorsExtensionSuffix = selectorsExtensionSuffix.substring(1);
+ }
+ if (selectorsExtensionSuffix.length() > 0) {
+ String[] parts = selectorsExtensionSuffix.split("\\.");
+ if (parts.length > 1) {
+ selectors = Arrays.copyOfRange(parts, 0, parts.length - 1);
+ }
+ extension = parts[parts.length - 1];
+ }
+ String resolvedPath = resource.getPath();
+ if (selectors.length > 0) {
+ resolvedPath += "." + String.join(".", selectors);
+ }
+ if (extension != null) {
+ resolvedPath += "." + extension;
+ }
+ if (suffix != null) {
+ resolvedPath += suffix;
+ }
+ if (!path.equals(resolvedPath) && suffix != null) {
+ resource = new ResourceWrapper(resource) {
+ @Override
+ public @NotNull String getPath() {
+ return path;
+ }
+ };
+ }
+ final SlingHttpServletRequest request = Builders.newRequestBuilder(Objects.requireNonNull(resource))
+ .withRequestMethod(method)
+ .withParameters(params)
+ .withContentType(contentType)
+ .withBody(body == null ? null : IOUtils.toString(body))
+ .withSelectors(resource instanceof ResourceWrapper ? null : selectors)
+ .withExtension(resource instanceof ResourceWrapper ? null : extension)
+ .withSuffix(resource instanceof ResourceWrapper ? null : suffix)
+ .build();
+ final SlingHttpServletResponseResult response = Builders.newResponseBuilder().build();
+ requestProcessor.processRequest(request, response, resourceResolver);
+ return response;
}
protected String getContent(String path) throws Exception {
@@ -211,7 +274,7 @@ protected String getContentWithPost(String path, String query, Map variables) throws Exception {
+ protected SlingHttpServletResponseResult persistQuery(String path, String query, Map variables) throws Exception {
Map body = new HashMap<>();
if (query != null) {
String queryEncoded = query.replace("\n", "\\n");
@@ -229,10 +292,10 @@ protected String toJSON(Object source) {
return mapper.toStructure(source).toString();
}
- protected Map toMap(String ...keyValuePairs) {
- final Map result = new HashMap<>();
+ protected Map toMap(String ...keyValuePairs) {
+ final Map result = new HashMap<>();
for(int i=0 ; i < keyValuePairs.length; i+=2) {
- result.put(keyValuePairs[i], keyValuePairs[i+1]);
+ result.put(keyValuePairs[i], new String[] {keyValuePairs[i+1]});
}
return result;
}
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 e434b63..42818c3 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
@@ -45,10 +45,10 @@
import org.apache.http.impl.client.BasicCredentialsProvider;
import org.apache.http.impl.client.CloseableHttpClient;
import org.apache.http.impl.client.HttpClients;
+import org.apache.sling.api.request.builder.SlingHttpServletResponseResult;
import org.apache.sling.graphql.api.SchemaProvider;
import org.apache.sling.graphql.core.mocks.ReplacingSchemaProvider;
import org.apache.sling.resource.presence.ResourcePresence;
-import org.apache.sling.servlethelpers.MockSlingHttpServletResponse;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.ops4j.pax.exam.Configuration;
@@ -61,8 +61,8 @@
import static com.jayway.jsonpath.matchers.JsonPathMatchers.hasJsonPath;
import static com.jayway.jsonpath.matchers.JsonPathMatchers.hasNoJsonPath;
-import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.equalTo;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue;
@@ -126,7 +126,7 @@ public void testGqlExtWithPost() throws Exception {
@Test
public void testPersistedQueriesBasic() throws Exception {
String queryHash = "a16982712f6ecdeba5d950d42e3c13df0fc26d008c497f6bf012701b57e02a51";
- MockSlingHttpServletResponse response = persistQuery("/graphql/two.gql", "{ currentResource { resourceType name } }", null);
+ SlingHttpServletResponseResult response = persistQuery("/graphql/two.gql", "{ currentResource { resourceType name } }", null);
assertEquals("Expected to have stored a persisted query.", 201, response.getStatus());
assertEquals("The value of the Location header does not look correct.",
"http://localhost/graphql/two.gql/persisted/" + queryHash + ".gql",
@@ -227,7 +227,7 @@ public void testOtherExtAndOtherSelector() throws Exception {
@Test
public void testMissingQuery() throws Exception {
- MockSlingHttpServletResponse response = executeRequest("GET", "/graphql/two.gql", null, null, null, -1);
+ SlingHttpServletResponseResult response = executeRequest("GET", "/graphql/two.gql", null, null, null, -1);
assertEquals(400, response.getStatus());
}
diff --git a/src/test/java/org/apache/sling/graphql/core/it/GraphQLServletNoConfigIT.java b/src/test/java/org/apache/sling/graphql/core/it/GraphQLServletNoConfigIT.java
index 680fbc3..23a5ee7 100644
--- a/src/test/java/org/apache/sling/graphql/core/it/GraphQLServletNoConfigIT.java
+++ b/src/test/java/org/apache/sling/graphql/core/it/GraphQLServletNoConfigIT.java
@@ -20,8 +20,8 @@
import javax.inject.Inject;
+import org.apache.sling.api.request.builder.SlingHttpServletResponseResult;
import org.apache.sling.resource.presence.ResourcePresence;
-import org.apache.sling.servlethelpers.MockSlingHttpServletResponse;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.ops4j.pax.exam.Configuration;
@@ -55,7 +55,7 @@ public Option[] configuration() {
@Test
public void testServletDisabledByDefault() throws Exception {
final String path = "/graphql/one";
- MockSlingHttpServletResponse response = executeRequest("GET", path + ".json", null, null, null, -1);
+ SlingHttpServletResponseResult response = executeRequest("GET", path + ".json", null, null, null, -1);
assertEquals(200, response.getStatus());
response = executeRequest("GET", path + ".gql", null, null, null, -1);