Simplify the API and add graphqljava subpackage to express our dependency to graphql-java
diff --git a/graphql-scripting/src/main/java/org/apache/sling/graphql/api/DataFetcherDefinition.java b/graphql-scripting/src/main/java/org/apache/sling/graphql/api/DataFetcherDefinition.java
deleted file mode 100644
index bfe5c31..0000000
--- a/graphql-scripting/src/main/java/org/apache/sling/graphql/api/DataFetcherDefinition.java
+++ /dev/null
@@ -1,35 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements. See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership. The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied. See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-
-package org.apache.sling.graphql.api;
-
-import aQute.bnd.annotation.ProviderType;
-
-@ProviderType
-public interface DataFetcherDefinition {
-
- String getFetcherNamespace();
-
- String getFetcherName();
-
- String getFetcherOptions();
-
- String getFetcherSourceExpression();
-
-}
diff --git a/graphql-scripting/src/main/java/org/apache/sling/graphql/api/DataFetcherProvider.java b/graphql-scripting/src/main/java/org/apache/sling/graphql/api/graphqljava/DataFetcherProvider.java
similarity index 61%
copy from graphql-scripting/src/main/java/org/apache/sling/graphql/api/DataFetcherProvider.java
copy to graphql-scripting/src/main/java/org/apache/sling/graphql/api/graphqljava/DataFetcherProvider.java
index 8c37d79..9b5b28a 100644
--- a/graphql-scripting/src/main/java/org/apache/sling/graphql/api/DataFetcherProvider.java
+++ b/graphql-scripting/src/main/java/org/apache/sling/graphql/api/graphqljava/DataFetcherProvider.java
@@ -17,7 +17,7 @@
* under the License.
*/
-package org.apache.sling.graphql.api;
+package org.apache.sling.graphql.api.graphqljava;
import aQute.bnd.annotation.ConsumerType;
import graphql.schema.DataFetcher;
@@ -25,11 +25,19 @@
@ConsumerType
public interface DataFetcherProvider {
-
+ // TODO this should be a service property
String getNamespace();
+ // TODO this is probably not needed as clients
+ // can query all (ranked) services that belong to the desired
+ // namespace by calling createDataFetcher and stopping
+ // as soon as one returns non-null.
String getName();
- DataFetcher<Object> createDataFetcher(DataFetcherDefinition fetcherDef, Resource r);
-
+ /* Create a DataFetcher according to the supplied options.
+ * The implementation can decide to reuse the same object
+ * multiple times, new one every time or anything in
+ * between, no assumptions should be made around that.
+ */
+ DataFetcher<Object> createDataFetcher(Resource r, String name, String options, String source);
}
diff --git a/graphql-scripting/src/main/java/org/apache/sling/graphql/api/DataFetcherProvider.java b/graphql-scripting/src/main/java/org/apache/sling/graphql/api/graphqljava/package-info.java
similarity index 63%
rename from graphql-scripting/src/main/java/org/apache/sling/graphql/api/DataFetcherProvider.java
rename to graphql-scripting/src/main/java/org/apache/sling/graphql/api/graphqljava/package-info.java
index 8c37d79..e41bb85 100644
--- a/graphql-scripting/src/main/java/org/apache/sling/graphql/api/DataFetcherProvider.java
+++ b/graphql-scripting/src/main/java/org/apache/sling/graphql/api/graphqljava/package-info.java
@@ -17,19 +17,15 @@
* under the License.
*/
-package org.apache.sling.graphql.api;
-
-import aQute.bnd.annotation.ConsumerType;
-import graphql.schema.DataFetcher;
-import org.apache.sling.api.resource.Resource;
-
-@ConsumerType
-public interface DataFetcherProvider {
-
- String getNamespace();
-
- String getName();
-
- DataFetcher<Object> createDataFetcher(DataFetcherDefinition fetcherDef, Resource r);
-
-}
+ /**
+ * This package contains APIs which are specific to the
+ * Sling GraphQL core implementation that's based on the
+ * com.graphql-java:graphql-java library.
+ *
+ * If we later want our API to be independent of that, we
+ * can deprecate this package in favor of generic implementations
+ * that would then be in the parent package.
+ */
+@Version("1.0.0")
+package org.apache.sling.graphql.api.graphqljava;
+import org.osgi.annotation.versioning.Version;
\ No newline at end of file
diff --git a/graphql-scripting/src/main/java/org/apache/sling/graphql/api/package-info.java b/graphql-scripting/src/main/java/org/apache/sling/graphql/api/package-info.java
index dc831ff..3a32ca4 100644
--- a/graphql-scripting/src/main/java/org/apache/sling/graphql/api/package-info.java
+++ b/graphql-scripting/src/main/java/org/apache/sling/graphql/api/package-info.java
@@ -17,7 +17,10 @@
* under the License.
*/
-@Version("1.0.0")
+ /**
+ * This package contains APIs which are independent of
+ * a specific implementation of the underlying graphQL engine.
+ */
+@Version("2.0.0")
package org.apache.sling.graphql.api;
-
import org.osgi.annotation.versioning.Version;
\ No newline at end of file
diff --git a/graphql-scripting/src/main/java/org/apache/sling/scripting/gql/engine/GraphQLResourceQuery.java b/graphql-scripting/src/main/java/org/apache/sling/scripting/gql/engine/GraphQLResourceQuery.java
index e0782cc..fbada31 100644
--- a/graphql-scripting/src/main/java/org/apache/sling/scripting/gql/engine/GraphQLResourceQuery.java
+++ b/graphql-scripting/src/main/java/org/apache/sling/scripting/gql/engine/GraphQLResourceQuery.java
@@ -27,7 +27,7 @@
import graphql.language.ObjectTypeDefinition;
import org.apache.sling.api.resource.Resource;
import org.apache.sling.graphql.api.SchemaProvider;
-import org.apache.sling.scripting.gql.schema.DataFetcherDefinitionImpl;
+import org.apache.sling.scripting.gql.schema.DataFetcherDefinition;
import org.apache.sling.scripting.gql.schema.DataFetcherSelector;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -129,7 +129,7 @@
commentStr = commentStr.substring(1).trim();
try {
- DataFetcherDefinitionImpl def = new DataFetcherDefinitionImpl(commentStr);
+ DataFetcherDefinition def = new DataFetcherDefinition(commentStr);
DataFetcher<Object> fetcher = fetchers.getDataFetcherForType(def, r);
if (fetcher != null) {
return fetcher;
diff --git a/graphql-scripting/src/main/java/org/apache/sling/scripting/gql/schema/DataFetcherDefinitionImpl.java b/graphql-scripting/src/main/java/org/apache/sling/scripting/gql/schema/DataFetcherDefinition.java
similarity index 90%
rename from graphql-scripting/src/main/java/org/apache/sling/scripting/gql/schema/DataFetcherDefinitionImpl.java
rename to graphql-scripting/src/main/java/org/apache/sling/scripting/gql/schema/DataFetcherDefinition.java
index 23d2404..ebae583 100644
--- a/graphql-scripting/src/main/java/org/apache/sling/scripting/gql/schema/DataFetcherDefinitionImpl.java
+++ b/graphql-scripting/src/main/java/org/apache/sling/scripting/gql/schema/DataFetcherDefinition.java
@@ -20,12 +20,10 @@
package org.apache.sling.scripting.gql.schema;
-import org.apache.sling.graphql.api.DataFetcherDefinition;
-
import java.util.regex.Matcher;
import java.util.regex.Pattern;
-public class DataFetcherDefinitionImpl implements DataFetcherDefinition {
+public class DataFetcherDefinition {
public final String fetcherNamespace;
public final String fetcherName;
public final String fetcherOptions;
@@ -39,7 +37,7 @@
/** Creates a definition from a formatted String like
*
*/
- public DataFetcherDefinitionImpl(String fetcherDef) {
+ public DataFetcherDefinition(String fetcherDef) {
if(fetcherDef == null) {
throw new IllegalArgumentException("Invalid input: " + fetcherDef);
}
@@ -57,22 +55,18 @@
return input == null ? "" : input.trim();
}
- @Override
public String getFetcherNamespace() {
return fetcherNamespace;
}
- @Override
public String getFetcherName() {
return fetcherName;
}
- @Override
public String getFetcherOptions() {
return fetcherOptions;
}
- @Override
public String getFetcherSourceExpression() {
return fetcherSourceExpression;
}
diff --git a/graphql-scripting/src/main/java/org/apache/sling/scripting/gql/schema/DataFetcherSelector.java b/graphql-scripting/src/main/java/org/apache/sling/scripting/gql/schema/DataFetcherSelector.java
index 4caa635..a5cc9be 100644
--- a/graphql-scripting/src/main/java/org/apache/sling/scripting/gql/schema/DataFetcherSelector.java
+++ b/graphql-scripting/src/main/java/org/apache/sling/scripting/gql/schema/DataFetcherSelector.java
@@ -21,8 +21,7 @@
import graphql.schema.DataFetcher;
import org.apache.sling.api.resource.Resource;
-import org.apache.sling.graphql.api.DataFetcherProvider;
-import org.apache.sling.graphql.api.DataFetcherDefinition;
+import org.apache.sling.graphql.api.graphqljava.DataFetcherProvider;
import org.osgi.framework.Constants;
import org.osgi.service.component.annotations.Component;
import org.osgi.service.component.annotations.Reference;
@@ -58,7 +57,7 @@
for (DataFetcherProvider factory : factories) {
if (factory.getNamespace().equals(ns) && factory.getName().equals(name)) {
- return factory.createDataFetcher(def, r);
+ return factory.createDataFetcher(r, def.getFetcherName(), def.getFetcherOptions(), def.getFetcherSourceExpression());
}
}
return null;
diff --git a/graphql-scripting/src/test/java/org/apache/sling/scripting/gql/engine/GraphQLResourceQueryTest.java b/graphql-scripting/src/test/java/org/apache/sling/scripting/gql/engine/GraphQLResourceQueryTest.java
index b6e1241..ce3e539 100644
--- a/graphql-scripting/src/test/java/org/apache/sling/scripting/gql/engine/GraphQLResourceQueryTest.java
+++ b/graphql-scripting/src/test/java/org/apache/sling/scripting/gql/engine/GraphQLResourceQueryTest.java
@@ -22,7 +22,7 @@
import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue;
-
+import static org.junit.Assert.fail;
import static org.hamcrest.Matchers.equalTo;
import java.util.UUID;
@@ -34,6 +34,7 @@
import org.apache.sling.scripting.gql.schema.MockDataFetcherSelector;
import org.apache.sling.graphql.api.SchemaProvider;
import org.junit.Before;
+import org.junit.Ignore;
import org.junit.Test;
import org.mockito.Mockito;
@@ -88,4 +89,11 @@
assertThat(json, hasJsonPath("$.data.currentResource.pathSHA256", equalTo("sha-256#path#" + pathSHA256)));
assertThat(json, hasJsonPath("$.data.currentResource.resourceTypeMD5", equalTo("md5#resourceType#" + resourceTypeMD5)));
}
-}
+
+ @Test
+ @Ignore
+ public void nullValuesTest() {
+ // GsonBuilder().serializeNulls().create() used to take care of that
+ fail("TODO - add a test that verifies that null values are included in the generated JSON");
+ }
+}
\ No newline at end of file
diff --git a/graphql-scripting/src/test/java/org/apache/sling/scripting/gql/schema/DataFetcherDefinitionTest.java b/graphql-scripting/src/test/java/org/apache/sling/scripting/gql/schema/DataFetcherDefinitionTest.java
index da0b483..07eb31a 100644
--- a/graphql-scripting/src/test/java/org/apache/sling/scripting/gql/schema/DataFetcherDefinitionTest.java
+++ b/graphql-scripting/src/test/java/org/apache/sling/scripting/gql/schema/DataFetcherDefinitionTest.java
@@ -67,11 +67,11 @@
@Test
public void testMatch() throws Exception {
if(failureClass == null) {
- final DataFetcherDefinitionImpl d = new DataFetcherDefinitionImpl(input);
- assertEquals("DataFetcherDefinitionImpl#" + expected, d.toString());
+ final DataFetcherDefinition d = new DataFetcherDefinition(input);
+ assertEquals("DataFetcherDefinition#" + expected, d.toString());
} else {
try {
- new DataFetcherDefinitionImpl(input);
+ new DataFetcherDefinition(input);
fail("Expecting a " + failureClass.getName());
} catch(Throwable t) {
assertEquals("Expecting a " + failureClass.getName(), failureClass, t.getClass());
diff --git a/graphql-scripting/src/test/java/org/apache/sling/scripting/gql/schema/DataFetcherSelectorTest.java b/graphql-scripting/src/test/java/org/apache/sling/scripting/gql/schema/DataFetcherSelectorTest.java
index cdcd814..50f5178 100644
--- a/graphql-scripting/src/test/java/org/apache/sling/scripting/gql/schema/DataFetcherSelectorTest.java
+++ b/graphql-scripting/src/test/java/org/apache/sling/scripting/gql/schema/DataFetcherSelectorTest.java
@@ -20,7 +20,7 @@
package org.apache.sling.scripting.gql.schema;
import graphql.schema.DataFetcher;
-import org.apache.sling.graphql.api.DataFetcherProvider;
+import org.apache.sling.graphql.api.graphqljava.DataFetcherProvider;
import org.junit.Test;
import org.mockito.Mockito;
@@ -37,31 +37,31 @@
DataFetcherProvider fetcher1 = Mockito.mock(DataFetcherProvider.class);
Mockito.when(fetcher1.getNamespace()).thenReturn("ns1");
Mockito.when(fetcher1.getName()).thenReturn("name1");
- Mockito.when(fetcher1.createDataFetcher(Mockito.any(), Mockito.any()))
+ Mockito.when(fetcher1.createDataFetcher(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any()))
.thenReturn(gqlFetcher1);
DataFetcherProvider fetcher2 = Mockito.mock(DataFetcherProvider.class);
Mockito.when(fetcher2.getNamespace()).thenReturn("ns2");
Mockito.when(fetcher2.getName()).thenReturn("name2");
- Mockito.when(fetcher2.createDataFetcher(Mockito.any(), Mockito.any()))
+ Mockito.when(fetcher2.createDataFetcher(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any()))
.thenReturn(gqlFetcher2);
DataFetcherProvider fetcher3 = Mockito.mock(DataFetcherProvider.class);
Mockito.when(fetcher3.getNamespace()).thenReturn("ns1");
Mockito.when(fetcher3.getName()).thenReturn("name2");
- Mockito.when(fetcher3.createDataFetcher(Mockito.any(), Mockito.any()))
+ Mockito.when(fetcher3.createDataFetcher(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any()))
.thenReturn(gqlFetcher3);
DataFetcherSelector fetchers = new DataFetcherSelector(fetcher1, fetcher2, fetcher3);
assertEquals(
gqlFetcher1,
- fetchers.getDataFetcherForType(new DataFetcherDefinitionImpl("fetch:ns1/name1"), null));
+ fetchers.getDataFetcherForType(new DataFetcherDefinition("fetch:ns1/name1"), null));
assertEquals(
gqlFetcher2,
- fetchers.getDataFetcherForType(new DataFetcherDefinitionImpl("fetch:ns2/name2"), null));
+ fetchers.getDataFetcherForType(new DataFetcherDefinition("fetch:ns2/name2"), null));
assertEquals(
gqlFetcher3,
- fetchers.getDataFetcherForType(new DataFetcherDefinitionImpl("fetch:ns1/name2"), null));
+ fetchers.getDataFetcherForType(new DataFetcherDefinition("fetch:ns1/name2"), null));
assertNull(
- fetchers.getDataFetcherForType(new DataFetcherDefinitionImpl("fetch:ns2/name1"), null));
+ fetchers.getDataFetcherForType(new DataFetcherDefinition("fetch:ns2/name1"), null));
}
}
diff --git a/graphql-scripting/src/test/java/org/apache/sling/scripting/gql/schema/MockDataFetcherSelector.java b/graphql-scripting/src/test/java/org/apache/sling/scripting/gql/schema/MockDataFetcherSelector.java
index b6da1bc..9a7c7c0 100644
--- a/graphql-scripting/src/test/java/org/apache/sling/scripting/gql/schema/MockDataFetcherSelector.java
+++ b/graphql-scripting/src/test/java/org/apache/sling/scripting/gql/schema/MockDataFetcherSelector.java
@@ -23,8 +23,7 @@
import graphql.schema.DataFetchingEnvironment;
import org.apache.sling.api.resource.Resource;
-import org.apache.sling.graphql.api.DataFetcherProvider;
-import org.apache.sling.graphql.api.DataFetcherDefinition;
+import org.apache.sling.graphql.api.graphqljava.DataFetcherProvider;
import java.io.UnsupportedEncodingException;
import java.security.MessageDigest;
@@ -66,7 +65,7 @@
}
@Override
- public DataFetcher<Object> createDataFetcher(DataFetcherDefinition fetcherDef, Resource r) {
+ public DataFetcher<Object> createDataFetcher(Resource r, String name, String options, String source) {
return new EchoDataFetcher(r);
}
}
@@ -99,7 +98,7 @@
}
@Override
- public DataFetcher<Object> createDataFetcher(DataFetcherDefinition fetcherDef, Resource r) {
+ public DataFetcher<Object> createDataFetcher(Resource r, String name, String options, String source) {
Map<String, Object> data = new LinkedHashMap<>(4);
data.put("test", true);
return new StaticDataFetcher(data);
@@ -110,20 +109,20 @@
private final Resource r;
private final String algorithm;
- private final String propertyName;
+ private final String source;
- DigestDataFetcher(Resource r, DataFetcherDefinition fetcherDef) {
+ DigestDataFetcher(Resource r, String options, String source) {
this.r = r;
- this.algorithm = fetcherDef.getFetcherOptions();
- this.propertyName = fetcherDef.getFetcherSourceExpression();
+ this.algorithm = options;
+ this.source = source;
}
@Override
public Object get(DataFetchingEnvironment environment) {
String rawValue = null;
- if ("path".equals(propertyName)) {
+ if ("path".equals(source)) {
rawValue = r.getPath();
- } else if("resourceType".equals(propertyName)) {
+ } else if("resourceType".equals(source)) {
rawValue = r.getResourceType();
}
@@ -134,7 +133,7 @@
throw new RuntimeException("Error computing digest:" + e, e);
}
- return algorithm + "#" + propertyName + "#" + digest;
+ return algorithm + "#" + source + "#" + digest;
}
}
@@ -166,8 +165,8 @@
}
@Override
- public DataFetcher<Object> createDataFetcher(DataFetcherDefinition fetcherDef, Resource r) {
- return new DigestDataFetcher(r, fetcherDef);
+ public DataFetcher<Object> createDataFetcher(Resource r, String name, String options, String source) {
+ return new DigestDataFetcher(r, options, source);
}
}
}
diff --git a/graphql-scripting/src/test/java/org/apache/sling/scripting/graphql/it/GraphQLScriptingTestSupport.java b/graphql-scripting/src/test/java/org/apache/sling/scripting/graphql/it/GraphQLScriptingTestSupport.java
index f4061ef..0a35f43 100644
--- a/graphql-scripting/src/test/java/org/apache/sling/scripting/graphql/it/GraphQLScriptingTestSupport.java
+++ b/graphql-scripting/src/test/java/org/apache/sling/scripting/graphql/it/GraphQLScriptingTestSupport.java
@@ -25,7 +25,7 @@
import org.apache.sling.api.resource.ResourceResolver;
import org.apache.sling.api.resource.ResourceResolverFactory;
-import org.apache.sling.graphql.api.DataFetcherProvider;
+import org.apache.sling.graphql.api.graphqljava.DataFetcherProvider;
import org.apache.sling.servlethelpers.MockSlingHttpServletRequest;
import org.apache.sling.servlethelpers.MockSlingHttpServletResponse;
import org.apache.sling.testing.paxexam.TestSupport;
diff --git a/graphql-scripting/src/test/java/org/apache/sling/scripting/graphql/it/PipeDataFetcherFactory.java b/graphql-scripting/src/test/java/org/apache/sling/scripting/graphql/it/PipeDataFetcherFactory.java
index 0c1120a..e9fe618 100644
--- a/graphql-scripting/src/test/java/org/apache/sling/scripting/graphql/it/PipeDataFetcherFactory.java
+++ b/graphql-scripting/src/test/java/org/apache/sling/scripting/graphql/it/PipeDataFetcherFactory.java
@@ -22,25 +22,22 @@
import graphql.schema.DataFetcher;
import graphql.schema.DataFetchingEnvironment;
import org.apache.sling.api.resource.Resource;
-import org.apache.sling.graphql.api.DataFetcherDefinition;
-import org.apache.sling.graphql.api.DataFetcherProvider;
+import org.apache.sling.graphql.api.graphqljava.DataFetcherProvider;
public class PipeDataFetcherFactory implements DataFetcherProvider {
static class PipeDataFetcher implements DataFetcher<Object> {
- private final DataFetcherDefinition fetcherDef;
-
private final Resource r;
+ private final String options;
- PipeDataFetcher(DataFetcherDefinition fetcherDef, Resource r) {
- this.fetcherDef = fetcherDef;
+ PipeDataFetcher(Resource r, String name, String options, String source) {
this.r = r;
+ this.options = options;
}
@Override
public Object get(DataFetchingEnvironment environment) throws Exception {
- String options = fetcherDef.getFetcherOptions();
if (!options.isEmpty() && !options.equals("$")) {
throw new IllegalArgumentException("Invalid fetcher options: " + options);
}
@@ -61,7 +58,7 @@
}
@Override
- public DataFetcher<Object> createDataFetcher(DataFetcherDefinition fetcherDef, Resource r) {
- return new PipeDataFetcher(fetcherDef, r);
+ public DataFetcher<Object> createDataFetcher(Resource r, String name, String options, String source) {
+ return new PipeDataFetcher(r, name, options, source);
}
}
