Operation name not parsed from query string if operationName JSON field omitted from POST request

[yonigibbs]

Description
When Actuator is used in a Spring Boot app using graphql-spring-boot-starter, metrics data is created whenever a GraphQL endpoint is hit. One of the tags is operationName. This should be the name of the GraphQL operation. If the GraphQL call is as follows, then this works correctly:

{
	"operationName": "MyOperationName"
	"query": "query MyOperationName { ... }"
}

However, it doesn't work if the operationName JSON field is omitted (but the operation name is still included in the text of the query JSON field), i.e. the JSON posted is as follows:

{
	"query": "query MyOperationName { ... }"
}

In this case, the operationName tag is set to "unknown".

The GraphQL spec seems to suggest that the operationName JSON field is only required if there is more than one query. If there is only query, it should be read from the text of the query JSON field: https://spec.graphql.org/June2018/#GetOperation(). So I think maybe that is missing from the code in this library?

I logged this in this repo as, although it manifested for me when using the Spring Boot dependency, I believe the issue might be in this library, as it is here that the HTTP request is read, in GraphQLObjectMapper, I think. If this is incorrect let me know and I'll log it in the graphql-spring-boot-starter repo. (The use of Actuator is itself irrelevant, but it's the way I discovered that the operation name didn't seem to be read correctly.)

To Reproduce
There's a repro here: https://github.com/yonigibbs/gqlopname. This contains a unit test that demonstrates the error. Happy to provide more info if required.

[yonigibbs]

Also see discussion here: https://spectrum.chat/graphql-java/general/reading-operationname-from-query-text~30cc09d5-0a55-4406-88da-7e8d0b91141a. There, one suggestion was to do something like this:

public class OperationNameExtractor {
    public static String extractOperationName(String gqlQuery, String requestedOperationName, String defaultIfNotFound) {
        if (StringUtils.isNotEmpty(requestedOperationName)) {
            return requestedOperationName;
        }
        if (StringUtils.isNotEmpty(gqlQuery)) {
            return parseForOperationName(gqlQuery, defaultIfNotFound);
        }
        return defaultIfNotFound;
    }
    private static String parseForOperationName(String gqlQuery, String defaultIfNotFound) {
        try {
            Document document = new Parser().parseDocument(gqlQuery);
            List<OperationDefinition> operations = document.getDefinitionsOfType(OperationDefinition.class);
            if (operations.size() == 1) {
                String name = operations.get(0).getName();
                if (StringUtils.isNotEmpty(name)) {
                    return name;
                }
            }
        } catch (InvalidSyntaxException ignored) {
            // ignored
        }
        return defaultIfNotFound;
    }
}

Happy to submit a PR if you think that's the way to go? Would you have a preference as to where exactly that logic should go (e.g. whether to put it into GraphQLObjectMapper itself, or at the level above that which calls this class)?

[oliemansm]

@yonigibbs Nice find! PR would be very welcome and think the GraphQLObjectMapper itself makes sense in this case.

[yonigibbs]

Great stuff, thanks @oliemansm. Leave it with me and I'll try to get a PR together. Thanks!

[yonigibbs]

Thanks for resolving this, @oliemansm. Sorry I didn't get a chance to put together a PR myself. Been busy on a bunch of other stuff. Appreciate you fixing this!