Idea: Compile-time checking of missing projection fields in generated code

[Ernir]

When making GraphQL requests, selecting fields of object types is mandatory.

For example, given a schema

type Query {
    movie(id: ID!): Movie
}

type Movie {
    id: ID!
    title: String!
}

the following query is valid:

{
  movie(id: "tt0068646") {
    title
  }
}

We can construct such a valid query using the generated Java code:

var movieGraphQLQuery = MovieGraphQLQuery.newRequest().id("tt0068646").build();
var movieProjectionRoot = new MovieProjectionRoot<>().title();
var graphQLQueryRequest = new GraphQLQueryRequest(movieGraphQLQuery, movieProjectionRoot);

String title =
dgsQueryExecutor.executeAndExtractJsonPath(
    graphQLQueryRequest.serialize(), "data.movie.title");

However, we can also construct an invalid projection by not providing any fields:

var movieProjectionRoot = new MovieProjectionRoot<>();

which would fail at runtime when we attempt to execute the query:

com.netflix.graphql.dgs.exceptions.QueryException: Validation error (SubselectionRequired@[movie]) : Subselection required for type 'Movie' of field 'movie'

The idea

Could we perform this check at compile-time rather than runtime?

E.g. by introducing intermediate steps in the projection builder, so that a new MovieProjectionRoot<>() would not return a type that is recognized as a valid projection. Only after specifying at least one field in the projection would we get a type that can actually be passed to the GraphQLQueryRequest.

This might be easier to make explicit if the projections were created using a builder (then we could just not expose a .build() method on the steps representing invalid projections). Preventing invalid queries like this would already be a breaking change, using a real builder would be an even bigger one. But it would prevent this class of runtime errors.

[srinivasankavitha]

Thanks for the suggestion. I agree this would be a great feature to put in some safety checks. That said, as you already mentioned, we would need to make it a breaking change and as part of a major release. We have a lot of projects internally as well that need to be migrated before we can introduce breaking changes, which unfortunately adds a lot of overhead and friction for introducing these types of changes. I'll add this to our backlog of feature requests so we can tackle it when we have some bandwidth.

[srinivasankavitha]

Created this issue: Netflix/dgs-codegen#628

Alternatively, we could also provide the same check on the GraphQLQueryRequest's serialize method, which can simply validate the entire constructed query string and return an error if not. This would be a simpler change that would have the same effect of checking at compile time. And we may even not need it to be a breaking change.