Skip to content

Commit

Permalink
Merge pull request #2799 from kilink/astprinter-bug
Browse files Browse the repository at this point in the history
Fix bug in AstPrinter preventing query short form from being used
  • Loading branch information
dondonz committed May 17, 2022
2 parents a3a13d8 + f7b6975 commit 271879b
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
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
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
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
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
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 @@ -583,7 +583,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 @@ -601,8 +601,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
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
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 271879b

Please sign in to comment.