Skip to content

Commit

Permalink
Fix bug in AstPrinter preventing query short form from being used
Browse files Browse the repository at this point in the history
AstPrinter's operationDefinition method had a String comparison that would
always fail when checking the node's operation; the String contained in
the op variable was already lowercased, while the constant "QUERY" was
used in the comparison. The result was that query short form was
never used when printing queries.
  • Loading branch information
kilink committed Apr 16, 2022
1 parent 66595fa commit f7b6975
Show file tree
Hide file tree
Showing 11 changed files with 93 additions and 93 deletions.
2 changes: 1 addition & 1 deletion src/main/java/graphql/language/AstPrinter.java
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,7 @@ private NodePrinter<OperationDefinition> operationDefinition() {

// Anonymous queries with no directives or variable definitions can use
// the query short form.
if (isEmpty(name) && isEmpty(directives) && isEmpty(varDefinitions) && op.equals("QUERY")) {
if (isEmpty(name) && isEmpty(directives) && isEmpty(varDefinitions) && op.equals("query")) {
out.append(selectionSet);
} else {
out.append(spaced(op, smooshed(name, varDefinitions), directives, selectionSet));
Expand Down
8 changes: 4 additions & 4 deletions src/test/groovy/graphql/analysis/QueryTransformerTest.groovy
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ class QueryTransformerTest extends Specification {

then:
printAstCompact(newDocument) ==
"query {root {fooA {midA-modified {leafA} midB {leafB}} fooB {midA-modified {leafA} midB {leafB}}}}"
"{root {fooA {midA-modified {leafA} midB {leafB}} fooB {midA-modified {leafA} midB {leafB}}}}"
}

def "transform query delete midA nodes"() {
Expand All @@ -102,7 +102,7 @@ class QueryTransformerTest extends Specification {

then:
printAstCompact(newDocument) ==
"query {root {fooA {midB {leafB}} fooB {midB {leafB}}}}"
"{root {fooA {midB {leafB}} fooB {midB {leafB}}}}"
}

def "transform query add midA sibling"() {
Expand All @@ -124,7 +124,7 @@ class QueryTransformerTest extends Specification {

then:
printAstCompact(newDocument) ==
"query {root {fooA {midA {leafA} addedField}}}"
"{root {fooA {midA {leafA} addedField}}}"
}

def "transform query delete fragment spread and inline fragment"() {
Expand Down Expand Up @@ -170,7 +170,7 @@ class QueryTransformerTest extends Specification {
then:

printAstCompact(newDocument) ==
"query {root {fooA {midB {leafB}} fooB {midB {leafB}}}} fragment frag on Foo {midA {leafA}}"
"{root {fooA {midB {leafB}} fooB {midB {leafB}}}} fragment frag on Foo {midA {leafA}}"
}

def "transform query does not traverse named fragments when started from query"() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ class ApolloPersistedQuerySupportTest extends Specification {
def documentEntry = apolloSupport.getDocument(ei, engineParser)
def doc = documentEntry.getDocument()
then:
printAstCompact(doc) == "query {oneTwoThree}"
printAstCompact(doc) == "{oneTwoThree}"
persistedQueryCache.keyCount[hashOne] == 1
persistedQueryCache.parseCount[hashOne] == 1

Expand All @@ -79,7 +79,7 @@ class ApolloPersistedQuerySupportTest extends Specification {
doc = documentEntry.getDocument()

then:
printAstCompact(doc) == "query {oneTwoThree}"
printAstCompact(doc) == "{oneTwoThree}"
persistedQueryCache.keyCount[hashOne] == 2
persistedQueryCache.parseCount[hashOne] == 1 // only compiled once cause we had it
}
Expand All @@ -94,7 +94,7 @@ class ApolloPersistedQuerySupportTest extends Specification {
def documentEntry = apolloSupport.getDocument(ei, engineParser)
def doc = documentEntry.getDocument()
then:
printAstCompact(doc) == "query {normal}"
printAstCompact(doc) == "{normal}"
persistedQueryCache.keyCount.size() == 0
persistedQueryCache.parseCount.size() == 0
}
Expand All @@ -104,11 +104,11 @@ class ApolloPersistedQuerySupportTest extends Specification {
def apolloSupport = new ApolloPersistedQuerySupport(persistedQueryCache)

when:
def ei = mkEI(hashOne, "query {normal}")
def ei = mkEI(hashOne, "{normal}")
def documentEntry = apolloSupport.getDocument(ei, engineParser)
def doc = documentEntry.getDocument()
then:
printAstCompact(doc) == "query {oneTwoThree}"
printAstCompact(doc) == "{oneTwoThree}"
persistedQueryCache.keyCount[hashOne] == 1
persistedQueryCache.parseCount[hashOne] == 1

Expand Down Expand Up @@ -140,14 +140,14 @@ class ApolloPersistedQuerySupportTest extends Specification {
def documentEntry = apolloSupport.getDocument(ei, engineParser)
def doc = documentEntry.getDocument()
then:
printAstCompact(doc) == "query {oneTwoThree}"
printAstCompact(doc) == "{oneTwoThree}"

when:
ei = mkEI(hashTwo, PERSISTED_QUERY_MARKER)
documentEntry = apolloSupport.getDocument(ei, engineParser)
doc = documentEntry.getDocument()
then:
printAstCompact(doc) == "query {fourFiveSix}"
printAstCompact(doc) == "{fourFiveSix}"

when:
ei = mkEI("nonExistent", PERSISTED_QUERY_MARKER)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,19 +40,19 @@ class InMemoryPersistedQueryCacheTest extends Specification {
def getDoc = inMemCache.getPersistedQueryDocument(hash, ei, onMiss)
def doc = getDoc.document
then:
printAstCompact(doc) == "query {oneTwoThreeFour}"
printAstCompact(doc) == "{oneTwoThreeFour}"
}

def "When there's a cache miss, uses the query from known queries if the execution input's query is the APQ Marker"() {
def hash = "somehash"
def inMemCache = InMemoryPersistedQueryCache.newInMemoryPersistedQueryCache()
.addQuery(hash, "query {foo bar baz}")
.addQuery(hash, "{foo bar baz}")
.build()
def ei = mkEI(hash, PersistedQuerySupport.PERSISTED_QUERY_MARKER)
when:
def getDoc = inMemCache.getPersistedQueryDocument(hash, ei, onMiss)
def doc = getDoc.document
then:
printAstCompact(doc) == "query {foo bar baz}"
printAstCompact(doc) == "{foo bar baz}"
}
}
10 changes: 5 additions & 5 deletions src/test/groovy/graphql/language/AstPrinterTest.groovy
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ scalar DateTime
String output = printAst(document)

expect:
output == """query {
output == """{
empireHero: hero(episode: EMPIRE) {
name
}
Expand Down Expand Up @@ -235,7 +235,7 @@ fragment comparisonFields on Character {
String output = printAst(document)

expect:
output == """query {
output == """{
leftComparison: hero(episode: EMPIRE) {
...comparisonFields
}
Expand Down Expand Up @@ -563,7 +563,7 @@ extend input Input @directive {
String output = AstPrinter.printAstCompact(document)

expect:
output == '''query {aliasOfFoo:foo(arg1:"val1",args2:"val2") @isCached {hello} world @neverCache @okThenCache} fragment FX on SomeType {aliased:field(withArgs:"argVal",andMoreArgs:"andMoreVals")}'''
output == '''{aliasOfFoo:foo(arg1:"val1",args2:"val2") @isCached {hello} world @neverCache @okThenCache} fragment FX on SomeType {aliased:field(withArgs:"argVal",andMoreArgs:"andMoreVals")}'''
}

def "print ast with inline fragment without type condition"() {
Expand All @@ -581,8 +581,8 @@ extend input Input @directive {
String outputFull = AstPrinter.printAst(document)

expect:
outputCompact == '''query {foo {... {hello}}}'''
outputFull == '''query {
outputCompact == '''{foo {... {hello}}}'''
outputFull == '''{
foo {
... {
hello
Expand Down
2 changes: 1 addition & 1 deletion src/test/groovy/graphql/language/AstSignatureTest.groovy
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ fragment X on SomeType {
}
}"""

def expectedQuery = """query {
def expectedQuery = """{
allIssues(arg1: "", arg2: 0) {
id
}
Expand Down
4 changes: 2 additions & 2 deletions src/test/groovy/graphql/language/AstSorterTest.groovy
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ class AstSorterTest extends Specification {
'''

def expectedQuery = '''query {
def expectedQuery = '''{
unamedQueryX
unamedQueryY
unamedQueryZ
Expand Down Expand Up @@ -340,7 +340,7 @@ input InputZ {
}
'''

def expectedQuery = '''query {
def expectedQuery = '''{
field(arg: {x : {a : "vala", b : "valb", c : "valc"}, y : "valy", z : "valz"})
}
'''
Expand Down

0 comments on commit f7b6975

Please sign in to comment.