Ensure NormalizedQueryTreeFactory handles mutations and subscriptions#2126
Ensure NormalizedQueryTreeFactory handles mutations and subscriptions#2126skevy wants to merge 1 commit into
Conversation
andimarek
left a comment
andimarek
left a comment
There was a problem hiding this comment.
Thanks a lot .. great PR. Please have a look at the comments.
| } else if (operationDefinition.getOperation() == OperationDefinition.Operation.SUBSCRIPTION) { | ||
| rootType = graphQLSchema.getSubscriptionType(); | ||
| } else { | ||
| throw new IllegalStateException("Operation type is not valid."); |
There was a problem hiding this comment.
We use normally Assert.assertShouldNeverHappen here.
There was a problem hiding this comment.
Hello Andreas,
Assert.assertShouldNeverHappen seems to not work here as compiler complains that rootType is not initialized. And it looks like a good compile time check.
Can we commit this (as original author seems to be not available) as is? And replace this exception with something like
// Don't use Assert.assertShouldNeverHappen to make sure compiler does not force
// rootType above to be initialized. That way if code structure changes, compiler will
// catch that. Otherwise, we'd get NullPointerException at runtime.
throw new AssertException(
String.format(
"Internal error: should never happen: Operation type '%s' is not valid.",
operationDefinition.getOperation()));
in a follow up PR?
There was a problem hiding this comment.
+1 to commit, I'm hitting this issue as well. How about using a return statement to get rid of the initialization issue:
return Assert.assertShouldNeverHappen("Operation type is not valid.");| coordinatesToNormalizedFields[coordinates("Foo", "subFoo")].size() == 2 | ||
| } | ||
|
|
||
| def "handles mutations"() { |
There was a problem hiding this comment.
Another test for Subscriptions would be awesome, but Mutation is good enough.
|
superseded by #2173 |
4 participants
Fixes #2117