From 4bff85b2f02227ff1b401636b5dde9d20ecf4d64 Mon Sep 17 00:00:00 2001 From: Serge Huber Date: Thu, 13 Apr 2017 15:57:55 +0200 Subject: [PATCH 1/6] Fix OSGi service registration. --- src/main/java/graphql/servlet/OsgiGraphQLServlet.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/main/java/graphql/servlet/OsgiGraphQLServlet.java b/src/main/java/graphql/servlet/OsgiGraphQLServlet.java index 19c7cbdd..6e8d3474 100644 --- a/src/main/java/graphql/servlet/OsgiGraphQLServlet.java +++ b/src/main/java/graphql/servlet/OsgiGraphQLServlet.java @@ -38,7 +38,10 @@ import static graphql.schema.GraphQLSchema.newSchema; @Slf4j -@Component(property = {"alias=/graphql", "jmx.objectname=graphql.servlet:type=graphql"}) +@Component( + service=javax.servlet.http.HttpServlet.class, + property = {"alias=/graphql", "jmx.objectname=graphql.servlet:type=graphql"} +) public class OsgiGraphQLServlet extends GraphQLServlet { private List queryProviders = new ArrayList<>(); From e4e89ba90b40ef8506ad7a0d3ce598a359aa38c0 Mon Sep 17 00:00:00 2001 From: Serge Huber Date: Thu, 20 Apr 2017 14:30:29 +0200 Subject: [PATCH 2/6] Breaking changes: - Renaming the root query and mutation types from "query" and "mutation" to "Query" and "Mutation" to follow object type naming conventions - Modified the GraphQLQueryProvider implementation to make the old API deprecated and replace with a collection of field definitions, making this aligned with the GraphQLMutationProvider interface. This makes it possible to define data fetcher for the root object type fields. - Added JavaDoc documentation to the GraphQLQueryProvider interface --- .../graphql/servlet/GraphQLQueryProvider.java | 30 +++++++++++++++++-- .../graphql/servlet/OsgiGraphQLServlet.java | 24 ++++++++++----- 2 files changed, 44 insertions(+), 10 deletions(-) diff --git a/src/main/java/graphql/servlet/GraphQLQueryProvider.java b/src/main/java/graphql/servlet/GraphQLQueryProvider.java index c8cc8a13..2eefb055 100644 --- a/src/main/java/graphql/servlet/GraphQLQueryProvider.java +++ b/src/main/java/graphql/servlet/GraphQLQueryProvider.java @@ -14,11 +14,37 @@ */ package graphql.servlet; +import graphql.schema.GraphQLFieldDefinition; import graphql.schema.GraphQLObjectType; +import java.util.Collection; + +/** + * This interface is used by OSGi bundles to plugin new field into the root query type + */ public interface GraphQLQueryProvider { - GraphQLObjectType getQuery(); - Object context(); + + /** + * @return a collection of field definitions that will be added to the root query type. + */ + Collection getQueryFieldDefinitions(); + + /** + * @deprecated use query field definitions instead + * @return a GraphQL object type to add to the root query type + */ + default GraphQLObjectType getQuery() { return null; } + + /** + * @deprecated use query field definitions instead + * @return an object that will be used as a staticValue for the root query type field + */ + default Object context() { return null; } + + /** + * @deprecated use query field definitions instead + * @return the name to use for the field for this query provider in the root query type + */ default String getName() { return getQuery().getName(); } diff --git a/src/main/java/graphql/servlet/OsgiGraphQLServlet.java b/src/main/java/graphql/servlet/OsgiGraphQLServlet.java index 6e8d3474..cc9b408d 100644 --- a/src/main/java/graphql/servlet/OsgiGraphQLServlet.java +++ b/src/main/java/graphql/servlet/OsgiGraphQLServlet.java @@ -15,6 +15,7 @@ package graphql.servlet; import graphql.execution.ExecutionStrategy; +import graphql.schema.GraphQLFieldDefinition; import graphql.schema.GraphQLObjectType; import graphql.schema.GraphQLSchema; import graphql.schema.GraphQLType; @@ -52,16 +53,23 @@ public class OsgiGraphQLServlet extends GraphQLServlet { private GraphQLSchema readOnlySchema; protected void updateSchema() { - GraphQLObjectType.Builder object = newObject().name("query"); + GraphQLObjectType.Builder object = newObject().name("Query").description("Root query type"); for (GraphQLQueryProvider provider : queryProviders) { GraphQLObjectType query = provider.getQuery(); - object.field(newFieldDefinition(). - type(query). - staticValue(provider.context()). - name(provider.getName()). - description(query.getDescription()). - build()); + if (query != null) { + object.field(newFieldDefinition(). + type(query). + staticValue(provider.context()). + name(provider.getName()). + description(query.getDescription()). + build()); + } + if (provider.getQueryFieldDefinitions() != null && provider.getQueryFieldDefinitions().size() > 0) { + for (GraphQLFieldDefinition graphQLFieldDefinition : provider.getQueryFieldDefinitions()) { + object.field(graphQLFieldDefinition); + } + } } Set types = new HashSet<>(); @@ -74,7 +82,7 @@ protected void updateSchema() { if (mutationProviders.isEmpty()) { schema = readOnlySchema; } else { - GraphQLObjectType.Builder mutationObject = newObject().name("mutation"); + GraphQLObjectType.Builder mutationObject = newObject().name("Mutation").description("Root mutation type"); for (GraphQLMutationProvider provider : mutationProviders) { provider.getMutations().forEach(mutationObject::field); From c070f2196355ad2eefa6781c0d05d5fe645f3c4e Mon Sep 17 00:00:00 2001 From: Serge Huber Date: Thu, 20 Apr 2017 14:33:09 +0200 Subject: [PATCH 3/6] Improve migration documentation --- src/main/java/graphql/servlet/GraphQLQueryProvider.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/main/java/graphql/servlet/GraphQLQueryProvider.java b/src/main/java/graphql/servlet/GraphQLQueryProvider.java index 2eefb055..b0628876 100644 --- a/src/main/java/graphql/servlet/GraphQLQueryProvider.java +++ b/src/main/java/graphql/servlet/GraphQLQueryProvider.java @@ -25,7 +25,8 @@ public interface GraphQLQueryProvider { /** - * @return a collection of field definitions that will be added to the root query type. + * @return a collection of field definitions that will be added to the root query type. It is allowed to return null + * or an empty array (for example for migration purposes from the old API to this one). */ Collection getQueryFieldDefinitions(); From ad199cc2ee121f2757bc848aad7e1b347a7f68c1 Mon Sep 17 00:00:00 2001 From: Serge Huber Date: Thu, 20 Apr 2017 14:46:53 +0200 Subject: [PATCH 4/6] Make new method optional as this breaks automated tests. --- src/main/java/graphql/servlet/GraphQLQueryProvider.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/main/java/graphql/servlet/GraphQLQueryProvider.java b/src/main/java/graphql/servlet/GraphQLQueryProvider.java index b0628876..133ce54d 100644 --- a/src/main/java/graphql/servlet/GraphQLQueryProvider.java +++ b/src/main/java/graphql/servlet/GraphQLQueryProvider.java @@ -25,10 +25,9 @@ public interface GraphQLQueryProvider { /** - * @return a collection of field definitions that will be added to the root query type. It is allowed to return null - * or an empty array (for example for migration purposes from the old API to this one). + * @return a collection of field definitions that will be added to the root query type. */ - Collection getQueryFieldDefinitions(); + default Collection getQueryFieldDefinitions() { return null; } /** * @deprecated use query field definitions instead From 5d4dc1487d6260581016dcdcb85a8896b61e7e96 Mon Sep 17 00:00:00 2001 From: Serge Huber Date: Thu, 20 Apr 2017 17:25:18 +0200 Subject: [PATCH 5/6] - Remove all deprecated methods - Modify tests to use new field definition method --- .../graphql/servlet/GraphQLQueryProvider.java | 22 +---------------- .../graphql/servlet/OsgiGraphQLServlet.java | 10 -------- .../servlet/GraphQLVariablesSpec.groovy | 24 +++++++++++-------- .../servlet/OsgiGraphQLServletSpec.groovy | 21 ++++++++-------- 4 files changed, 26 insertions(+), 51 deletions(-) diff --git a/src/main/java/graphql/servlet/GraphQLQueryProvider.java b/src/main/java/graphql/servlet/GraphQLQueryProvider.java index 133ce54d..1b07da25 100644 --- a/src/main/java/graphql/servlet/GraphQLQueryProvider.java +++ b/src/main/java/graphql/servlet/GraphQLQueryProvider.java @@ -15,7 +15,6 @@ package graphql.servlet; import graphql.schema.GraphQLFieldDefinition; -import graphql.schema.GraphQLObjectType; import java.util.Collection; @@ -27,25 +26,6 @@ public interface GraphQLQueryProvider { /** * @return a collection of field definitions that will be added to the root query type. */ - default Collection getQueryFieldDefinitions() { return null; } + Collection getQueryFieldDefinitions(); - /** - * @deprecated use query field definitions instead - * @return a GraphQL object type to add to the root query type - */ - default GraphQLObjectType getQuery() { return null; } - - /** - * @deprecated use query field definitions instead - * @return an object that will be used as a staticValue for the root query type field - */ - default Object context() { return null; } - - /** - * @deprecated use query field definitions instead - * @return the name to use for the field for this query provider in the root query type - */ - default String getName() { - return getQuery().getName(); - } } diff --git a/src/main/java/graphql/servlet/OsgiGraphQLServlet.java b/src/main/java/graphql/servlet/OsgiGraphQLServlet.java index cc9b408d..7ea0534d 100644 --- a/src/main/java/graphql/servlet/OsgiGraphQLServlet.java +++ b/src/main/java/graphql/servlet/OsgiGraphQLServlet.java @@ -34,7 +34,6 @@ import java.util.Optional; import java.util.Set; -import static graphql.schema.GraphQLFieldDefinition.newFieldDefinition; import static graphql.schema.GraphQLObjectType.newObject; import static graphql.schema.GraphQLSchema.newSchema; @@ -56,15 +55,6 @@ protected void updateSchema() { GraphQLObjectType.Builder object = newObject().name("Query").description("Root query type"); for (GraphQLQueryProvider provider : queryProviders) { - GraphQLObjectType query = provider.getQuery(); - if (query != null) { - object.field(newFieldDefinition(). - type(query). - staticValue(provider.context()). - name(provider.getName()). - description(query.getDescription()). - build()); - } if (provider.getQueryFieldDefinitions() != null && provider.getQueryFieldDefinitions().size() > 0) { for (GraphQLFieldDefinition graphQLFieldDefinition : provider.getQueryFieldDefinitions()) { object.field(graphQLFieldDefinition); diff --git a/src/test/groovy/graphql/servlet/GraphQLVariablesSpec.groovy b/src/test/groovy/graphql/servlet/GraphQLVariablesSpec.groovy index a30aea54..895e5eb5 100644 --- a/src/test/groovy/graphql/servlet/GraphQLVariablesSpec.groovy +++ b/src/test/groovy/graphql/servlet/GraphQLVariablesSpec.groovy @@ -17,15 +17,29 @@ package graphql.servlet import graphql.annotations.GraphQLAnnotations import graphql.annotations.GraphQLField import graphql.annotations.GraphQLName +import graphql.schema.GraphQLFieldDefinition import graphql.schema.GraphQLObjectType import graphql.schema.GraphQLSchema import lombok.SneakyThrows import spock.lang.Specification +import static graphql.schema.GraphQLFieldDefinition.newFieldDefinition + class GraphQLVariablesSpec extends Specification { static class ComplexQueryProvider implements GraphQLQueryProvider { + @Override + Collection getQueryFieldDefinitions() { + List fieldDefinitions = new ArrayList<>(); + fieldDefinitions.add(newFieldDefinition() + .name("data") + .type(GraphQLAnnotations.object(DataQuery.class)) + .staticValue(new DataQuery()) + .build()); + return fieldDefinitions; + } + static class Data { @GraphQLField String field1 @@ -48,16 +62,6 @@ class GraphQLVariablesSpec extends Specification { } } - @Override - @SneakyThrows - GraphQLObjectType getQuery() { - return GraphQLAnnotations.object(DataQuery.class) - } - - @Override - Object context() { - return new DataQuery() - } } GraphQLSchema schema diff --git a/src/test/groovy/graphql/servlet/OsgiGraphQLServletSpec.groovy b/src/test/groovy/graphql/servlet/OsgiGraphQLServletSpec.groovy index 454ca199..ed13c449 100644 --- a/src/test/groovy/graphql/servlet/OsgiGraphQLServletSpec.groovy +++ b/src/test/groovy/graphql/servlet/OsgiGraphQLServletSpec.groovy @@ -29,22 +29,23 @@ class OsgiGraphQLServletSpec extends Specification { static class TestQueryProvider implements GraphQLQueryProvider { + @Override + Collection getQueryFieldDefinitions() { + List fieldDefinitions = new ArrayList<>(); + fieldDefinitions.add(newFieldDefinition() + .name("query") + .type(GraphQLAnnotations.object(Query.class)) + .staticValue(new Query()) + .build()); + return fieldDefinitions; + } + @GraphQLName("query") static class Query { @GraphQLField public String field; } - @Override - @SneakyThrows - GraphQLObjectType getQuery() { - return GraphQLAnnotations.object(Query.class); - } - - @Override - Object context() { - return new Query(); - } } def "query provider adds query objects"() { From c7fa1d5d894cb1d4eb8db6244d248f3fcd4955ef Mon Sep 17 00:00:00 2001 From: Serge Huber Date: Thu, 20 Apr 2017 17:33:04 +0200 Subject: [PATCH 6/6] Rename the method, for consistency with the mutation provider --- src/main/java/graphql/servlet/GraphQLQueryProvider.java | 2 +- src/main/java/graphql/servlet/OsgiGraphQLServlet.java | 4 ++-- src/test/groovy/graphql/servlet/GraphQLVariablesSpec.groovy | 2 +- src/test/groovy/graphql/servlet/OsgiGraphQLServletSpec.groovy | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/main/java/graphql/servlet/GraphQLQueryProvider.java b/src/main/java/graphql/servlet/GraphQLQueryProvider.java index 1b07da25..54f46e10 100644 --- a/src/main/java/graphql/servlet/GraphQLQueryProvider.java +++ b/src/main/java/graphql/servlet/GraphQLQueryProvider.java @@ -26,6 +26,6 @@ public interface GraphQLQueryProvider { /** * @return a collection of field definitions that will be added to the root query type. */ - Collection getQueryFieldDefinitions(); + Collection getQueries(); } diff --git a/src/main/java/graphql/servlet/OsgiGraphQLServlet.java b/src/main/java/graphql/servlet/OsgiGraphQLServlet.java index 7ea0534d..85063e7e 100644 --- a/src/main/java/graphql/servlet/OsgiGraphQLServlet.java +++ b/src/main/java/graphql/servlet/OsgiGraphQLServlet.java @@ -55,8 +55,8 @@ protected void updateSchema() { GraphQLObjectType.Builder object = newObject().name("Query").description("Root query type"); for (GraphQLQueryProvider provider : queryProviders) { - if (provider.getQueryFieldDefinitions() != null && provider.getQueryFieldDefinitions().size() > 0) { - for (GraphQLFieldDefinition graphQLFieldDefinition : provider.getQueryFieldDefinitions()) { + if (provider.getQueries() != null && provider.getQueries().size() > 0) { + for (GraphQLFieldDefinition graphQLFieldDefinition : provider.getQueries()) { object.field(graphQLFieldDefinition); } } diff --git a/src/test/groovy/graphql/servlet/GraphQLVariablesSpec.groovy b/src/test/groovy/graphql/servlet/GraphQLVariablesSpec.groovy index 895e5eb5..985c216d 100644 --- a/src/test/groovy/graphql/servlet/GraphQLVariablesSpec.groovy +++ b/src/test/groovy/graphql/servlet/GraphQLVariablesSpec.groovy @@ -30,7 +30,7 @@ class GraphQLVariablesSpec extends Specification { static class ComplexQueryProvider implements GraphQLQueryProvider { @Override - Collection getQueryFieldDefinitions() { + Collection getQueries() { List fieldDefinitions = new ArrayList<>(); fieldDefinitions.add(newFieldDefinition() .name("data") diff --git a/src/test/groovy/graphql/servlet/OsgiGraphQLServletSpec.groovy b/src/test/groovy/graphql/servlet/OsgiGraphQLServletSpec.groovy index ed13c449..5e9084ad 100644 --- a/src/test/groovy/graphql/servlet/OsgiGraphQLServletSpec.groovy +++ b/src/test/groovy/graphql/servlet/OsgiGraphQLServletSpec.groovy @@ -30,7 +30,7 @@ class OsgiGraphQLServletSpec extends Specification { static class TestQueryProvider implements GraphQLQueryProvider { @Override - Collection getQueryFieldDefinitions() { + Collection getQueries() { List fieldDefinitions = new ArrayList<>(); fieldDefinitions.add(newFieldDefinition() .name("query")