This is an automated email from the ASF dual-hosted git repository. andysch pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-graphql-core.git
commit 794abf3593814f2165f9313f20676b3bb2aa85c1 Author: Andreas Schaefer <[email protected]> AuthorDate: Tue Mar 28 11:41:03 2023 -0700 SLING-10900 - Upgrade to graphql-java 20.1 The difference between upgrading from 17.4 to 20.1 is minimal and so I moved to the latest code --- pom.xml | 17 +++- .../graphql/core/engine/DefaultQueryExecutor.java | 9 +- .../graphql/core/engine/SelectedFieldWrapper.java | 28 +++--- .../graphql/core/engine/SelectionSetWrapper.java | 13 ++- .../engine/DefaultQueryExecutorLoggingTest.java | 6 +- .../core/engine/DefaultQueryExecutorTest.java | 101 +++++++++++---------- 6 files changed, 99 insertions(+), 75 deletions(-) diff --git a/pom.xml b/pom.xml index 730e165..3c676a0 100644 --- a/pom.xml +++ b/pom.xml @@ -29,7 +29,7 @@ </parent> <artifactId>org.apache.sling.graphql.core</artifactId> - <version>0.0.15-SNAPSHOT</version> + <version>0.0.17-SNAPSHOT</version> <name>Apache Sling GraphQL Core</name> <description>Support for GraphQL queries</description> @@ -121,7 +121,20 @@ <dependency> <groupId>com.graphql-java</groupId> <artifactId>graphql-java</artifactId> - <version>15.0</version> + <version>20.1</version> + <scope>provided</scope> + </dependency> + <!-- The version 3.1.0, 3.1.1 are not valid Maven Bundles and so some IT tests fail --> + <dependency> + <groupId>com.graphql-java</groupId> + <artifactId>java-dataloader</artifactId> + <version>3.2.0</version> + <scope>provided</scope> + </dependency> + <dependency> + <groupId>org.antlr</groupId> + <artifactId>antlr4-runtime</artifactId> + <version>4.7.1</version> <scope>provided</scope> </dependency> <dependency> diff --git a/src/main/java/org/apache/sling/graphql/core/engine/DefaultQueryExecutor.java b/src/main/java/org/apache/sling/graphql/core/engine/DefaultQueryExecutor.java index 0a0cb41..421cec4 100644 --- a/src/main/java/org/apache/sling/graphql/core/engine/DefaultQueryExecutor.java +++ b/src/main/java/org/apache/sling/graphql/core/engine/DefaultQueryExecutor.java @@ -304,7 +304,8 @@ public class DefaultQueryExecutor implements QueryExecutor { private DataFetcher<Object> getDataFetcher(FieldDefinition field, Resource currentResource) { DataFetcher<Object> result = null; - final Directive d = field.getDirective(FETCHER_DIRECTIVE); +// final Directive d = field.getDirective(FETCHER_DIRECTIVE); + final Directive d = field.getDirectives().stream().filter( i -> FETCHER_DIRECTIVE.equals(i.getName())).findFirst().orElse(null); if (d != null) { final String name = validateFetcherName(getDirectiveArgumentValue(d, FETCHER_NAME)); final String options = getDirectiveArgumentValue(d, FETCHER_OPTIONS); @@ -319,7 +320,8 @@ public class DefaultQueryExecutor implements QueryExecutor { private <T extends TypeDefinition<T>> TypeResolver getTypeResolver(TypeDefinition<T> typeDefinition, Resource currentResource) { TypeResolver resolver = null; - final Directive d = typeDefinition.getDirective(RESOLVER_DIRECTIVE); +// final Directive d = typeDefinition.getDirective(RESOLVER_DIRECTIVE); + final Directive d = typeDefinition.getDirectives().stream().filter( i -> RESOLVER_DIRECTIVE.equals(i.getName())).findFirst().orElse(null); if (d != null) { final String name = validateResolverName(getDirectiveArgumentValue(d, RESOLVER_NAME)); final String options = getDirectiveArgumentValue(d, RESOLVER_OPTIONS); @@ -406,7 +408,8 @@ public class DefaultQueryExecutor implements QueryExecutor { private void handleConnectionTypes(ObjectTypeDefinition typeDefinition, TypeDefinitionRegistry typeRegistry) { for (FieldDefinition fieldDefinition : typeDefinition.getFieldDefinitions()) { - Directive directive = fieldDefinition.getDirective("connection"); +// Directive directive = fieldDefinition.getDirective("connection"); + Directive directive = fieldDefinition.getDirectives().stream().filter( i -> "connection".equals(i.getName())).findFirst().orElse(null); if (directive != null) { if (directive.getArgument(CONNECTION_FOR) != null) { String forType = ((StringValue) directive.getArgument(CONNECTION_FOR).getValue()).getValue(); diff --git a/src/main/java/org/apache/sling/graphql/core/engine/SelectedFieldWrapper.java b/src/main/java/org/apache/sling/graphql/core/engine/SelectedFieldWrapper.java index 6ca27a3..8b50e9e 100644 --- a/src/main/java/org/apache/sling/graphql/core/engine/SelectedFieldWrapper.java +++ b/src/main/java/org/apache/sling/graphql/core/engine/SelectedFieldWrapper.java @@ -22,6 +22,7 @@ import graphql.language.Field; import graphql.language.InlineFragment; import graphql.language.Selection; import graphql.language.SelectionSet; +import graphql.schema.DataFetchingFieldSelectionSet; import org.apache.sling.graphql.api.SelectedField; import java.util.Arrays; @@ -36,27 +37,22 @@ import java.util.stream.Collectors; public class SelectedFieldWrapper implements SelectedField { private String name; + @Deprecated private boolean isInline; + private boolean conditional; + private List<String> objectTypeBNames; private Map<String, SelectedField> subFieldMap = new HashMap<>(); private List<SelectedField> subFields; - public SelectedFieldWrapper(Selection selection) { - SelectionSet selectionSet = null; - if (selection instanceof InlineFragment) { - InlineFragment inline = (InlineFragment) selection; - this.name = inline.getTypeCondition().getName(); - this.isInline = true; - selectionSet = inline.getSelectionSet(); - } - if (selection instanceof Field) { - Field subField = (Field) selection; - this.name = subField.getName(); - selectionSet = subField.getSelectionSet(); - } + public SelectedFieldWrapper(graphql.schema.SelectedField selectedField) { + this.name = selectedField.getName(); + this.objectTypeBNames = selectedField.getObjectTypeNames(); + this.conditional = selectedField.isConditional(); + DataFetchingFieldSelectionSet selectionSet = selectedField.getSelectionSet(); if (selectionSet != null) { - selectionSet.getSelections().forEach(s -> { - SelectedFieldWrapper wrappedField = new SelectedFieldWrapper(s); - subFieldMap.put(wrappedField.getName(), wrappedField); + selectionSet.getImmediateFields().forEach(sf -> { + SelectedFieldWrapper selectedChildField = new SelectedFieldWrapper(sf); + subFieldMap.put(selectedChildField.getName(), selectedChildField); }); } subFields = subFieldMap.values().stream().collect(Collectors.toList()); diff --git a/src/main/java/org/apache/sling/graphql/core/engine/SelectionSetWrapper.java b/src/main/java/org/apache/sling/graphql/core/engine/SelectionSetWrapper.java index 7db2229..0a97453 100644 --- a/src/main/java/org/apache/sling/graphql/core/engine/SelectionSetWrapper.java +++ b/src/main/java/org/apache/sling/graphql/core/engine/SelectionSetWrapper.java @@ -41,11 +41,14 @@ public class SelectionSetWrapper implements SelectionSet { public SelectionSetWrapper(@Nullable DataFetchingFieldSelectionSet selectionSet) { if (selectionSet != null) { - selectionSet.get().getSubFields().forEach((k, v) -> { - SelectedFieldWrapper selectedField = new SelectedFieldWrapper(v.getSingleField()); - fieldsMap.put(k, selectedField); - if (!k.contains("/")) { - fields.add(selectedField); + selectionSet.getImmediateFields().forEach(sf -> { + String name = sf.getName(); + SelectedFieldWrapper selectedField = new SelectedFieldWrapper(sf); + if (!fieldsMap.containsKey(name)) { + fieldsMap.put(name, selectedField); + if (!name.contains("/")) { + fields.add(selectedField); + } } }); initFlatMap(fields, ""); diff --git a/src/test/java/org/apache/sling/graphql/core/engine/DefaultQueryExecutorLoggingTest.java b/src/test/java/org/apache/sling/graphql/core/engine/DefaultQueryExecutorLoggingTest.java index 136c73b..a0a74fd 100644 --- a/src/test/java/org/apache/sling/graphql/core/engine/DefaultQueryExecutorLoggingTest.java +++ b/src/test/java/org/apache/sling/graphql/core/engine/DefaultQueryExecutorLoggingTest.java @@ -75,14 +75,16 @@ public class DefaultQueryExecutorLoggingTest extends ResourceQueryTestBase { } private void assertQuerySyntaxError(String ... selectors) { - final String invalidQuery = "INVALID " + UUID.randomUUID(); +// This does not work anymore because the error message is based on the query string +// final String invalidQuery = "INVALID " + UUID.randomUUID(); + final String invalidQuery = "INVALID " + "4ecae67c-13c5-432e-b36c-7a29d35118c1"; queryJSON(invalidQuery, selectors); capture.assertContains( Level.ERROR, "Query failed for Resource " + resource.getPath(), "query=" + invalidQuery, "Errors:Error: type=InvalidSyntax", - "message=Invalid Syntax : offending token 'INVALID' at line 1 column 1", + "message=Invalid syntax with ANTLR error 'token recognition error at: '4ec'' at line 1 column 9", String.format("selectors=%s", Arrays.toString(selectors)) ); } diff --git a/src/test/java/org/apache/sling/graphql/core/engine/DefaultQueryExecutorTest.java b/src/test/java/org/apache/sling/graphql/core/engine/DefaultQueryExecutorTest.java index 19f9765..e6fdf95 100644 --- a/src/test/java/org/apache/sling/graphql/core/engine/DefaultQueryExecutorTest.java +++ b/src/test/java/org/apache/sling/graphql/core/engine/DefaultQueryExecutorTest.java @@ -165,7 +165,7 @@ public class DefaultQueryExecutorTest extends ResourceQueryTestBase { ValidationResult result = queryExecutor.validate(stmt, Collections.emptyMap(), resource, new String[] {}); assertFalse(result.isValid()); String errors = String.join("\n", result.getErrors()); - assertTrue(errors.contains("Error: type=ValidationError; message=Validation error of type FieldUndefined: Field 'currentRsrc' in type 'Query' is undefined @ 'currentRsrc'; location=1,3;")); + assertTrue("Wrong response, found errors: '" + errors + "'", errors.contains("Error: type=ValidationError; message=Validation error (FieldUndefined@[currentRsrc]) : Field 'currentRsrc' in type 'Query' is undefined; location=1,3;")); } @Test @@ -251,7 +251,7 @@ public class DefaultQueryExecutorTest extends ResourceQueryTestBase { }; final List<SelectedField> selectionSetFields = selectionSet.getFields(); for (String expectedFieldname : expectedFieldNames) { - assertTrue(selectionSetFields.stream().anyMatch(f -> expectedFieldname.equals(f.getName()))); + assertTrue("Failed to find expected field name: '" + expectedFieldname + "'", selectionSetFields.stream().anyMatch(f -> expectedFieldname.equals(f.getName()))); } // Assert it contains the expected results @@ -267,57 +267,63 @@ public class DefaultQueryExecutorTest extends ResourceQueryTestBase { "allTests/boolValue", "allTests/resourcePath", "unionTest", - "unionTest/Human", - "unionTest/Human/id", - "unionTest/Human/address", - "unionTest/Droid", - "unionTest/Droid/id", - "unionTest/Droid/primaryFunction", +// "unionTest/Human", +// "unionTest/Human/id", + "unionTest/id", +// "unionTest/Human/address", + "unionTest/address", +// "unionTest/Droid", +// "unionTest/Droid/id", +// "unionTest/Droid/primaryFunction", + "unionTest/primaryFunction", "interfaceTest", "interfaceTest/id", - "interfaceTest/Human", - "interfaceTest/Human/address", - "interfaceTest/Droid", - "interfaceTest/Droid/primaryFunction" +// "interfaceTest/Human", +// "interfaceTest/Human/address", + "interfaceTest/address", +// "interfaceTest/Droid", + "interfaceTest/primaryFunction" +// "interfaceTest/Droid/primaryFunction" }; for (String expectedQN : expectedQualifiedName) { - assertTrue(selectionSet.contains(expectedQN)); + assertTrue("Failed to find qualified field name: '" + expectedQN + "'", selectionSet.contains(expectedQN)); } - String[] expectedNonInlineQNs = new String[] { - "boolValue", - "resourcePath", - "aTest", - "aTest/test", - "aTest/boolValue", - "aTest/resourcePath", - "allTests", - "allTests/test", - "allTests/boolValue", - "allTests/resourcePath", - "unionTest", - "unionTest/Human/id", - "unionTest/Human/address", - "unionTest/Droid/id", - "unionTest/Droid/primaryFunction", - "interfaceTest", - "interfaceTest/id", - "interfaceTest/Human/address", - "interfaceTest/Droid/primaryFunction" - }; - for (String expectedNonInlineQN : expectedNonInlineQNs) { - assertFalse(Objects.requireNonNull(selectionSet.get(expectedNonInlineQN)).isInline()); - } - - String[] expectedInlineQNs = new String[] { - "unionTest/Human", - "unionTest/Droid", - "interfaceTest/Human", - "interfaceTest/Droid" - }; - for (String expectedInlineQN : expectedInlineQNs) { - assertTrue(Objects.requireNonNull(selectionSet.get(expectedInlineQN)).isInline()); - } +//TODO: No more inline fragments in 17.4 -> remove when done upgrading +// String[] expectedNonInlineQNs = new String[] { +// "boolValue", +// "resourcePath", +// "aTest", +// "aTest/test", +// "aTest/boolValue", +// "aTest/resourcePath", +// "allTests", +// "allTests/test", +// "allTests/boolValue", +// "allTests/resourcePath", +// "unionTest", +// "unionTest/Human/id", +// "unionTest/Human/address", +// "unionTest/Droid/id", +// "unionTest/Droid/primaryFunction", +// "interfaceTest", +// "interfaceTest/id", +// "interfaceTest/Human/address", +// "interfaceTest/Droid/primaryFunction" +// }; +// for (String expectedNonInlineQN : expectedNonInlineQNs) { +// assertFalse(Objects.requireNonNull(selectionSet.get(expectedNonInlineQN)).isInline()); +// } +// +// String[] expectedInlineQNs = new String[] { +// "unionTest/Human", +// "unionTest/Droid", +// "interfaceTest/Human", +// "interfaceTest/Droid" +// }; +// for (String expectedInlineQN : expectedInlineQNs) { +// assertTrue(Objects.requireNonNull(selectionSet.get(expectedInlineQN)).isInline()); +// } String[] expectedSubFieldNames = new String[] { "test", @@ -333,6 +339,7 @@ public class DefaultQueryExecutorTest extends ResourceQueryTestBase { } } + @Test public void testCachedTypeRegistry() throws IOException { // by default we'll get the test-schema
