~bigbes/sr-ht-spec

4cc9f4338cd63998279198db5ae32a53d9e6f64c — Eugene Blikh a day ago 944e3eb
graph: pin the introspection schema to what the executor can serve

The introspection schema comes from gqlparser's built-in prelude; the
code that executes it is generated by gqlgen. Nothing tied the two
together, and gqlgen's generated executor answers a field it does not
know with panic("unknown field"), recovered into "internal system
error". A gqlparser that has learned a newer edition of the
introspection schema than the pinned gqlgen can execute therefore leaves
/query advertising fields that fail when a consumer selects them —
which matters here, because this endpoint is meant to be federated.

The new test derives the field list from whichever prelude is in use
rather than hard-coding one, and checks every declared field of the six
introspection types is selectable through a path this schema really has.
Against gqlparser v2.5.36 it fails on __Type and __InputValue, the two
types v2.5.22 added fields to; at the pinned v2.5.21 it passes.

Also corrects generate.go. Its note explained why the directive resolves
gqlgen through the module graph, which is true, but read as though that
made codegen work. It does not: v0.17.36 re-emits an existing resolver's
doc comment without its // markers and leaves schema.resolvers.go as
invalid Go. v0.17.94 fixes that and is blocked on sr-ht-core, so the
note now says so and says where the bump belongs.
2 files changed, 136 insertions(+), 0 deletions(-)

M graph/generate.go
A graph/introspection_test.go
M graph/generate.go => graph/generate.go +18 -0
@@ 9,6 9,24 @@ package graph
// golang.org/x/tools v0.9.3, which does not compile under this toolchain, while
// the version this module selects does.
//
// Running it that way gets the generator to start, but not to finish: codegen
// is broken here and the module-graph form is not a way around that. gqlgen
// v0.17.36 re-emits an existing resolver's doc comment without its `//`
// markers, so the run ends with `gofmt failed on schema.resolvers.go:
// expected declaration, found It` and leaves that file as invalid Go — revert
// it after any attempt. The bug is fixed upstream: v0.17.94 regenerates this
// package into valid, gofmt-clean Go.
//
// Adopting v0.17.94 is blocked outside this repository, not by the directive
// below. It changes graphql.ExecutableSchema's Complexity method to take a
// context, and sourcecraft.dev/bigbes/sr-ht-core's webhooks/context.go still
// calls the three-argument complexity.Calculate, so the shared library stops
// compiling. spec.sr.ht is the only service of the family that imports
// core-go's webhooks package, which is why the five siblings could move to
// v0.17.94 and this one cannot. The bump belongs in sr-ht-core first; when it
// lands, pin the directive below to the version go.mod then requires, as the
// siblings do.
//
// The generator is deliberately not imported anywhere. A blank import under a
// `generate` build tag — which is how git.sr.ht's api/graph does it — would put
// the whole codegen dependency tree (a CLI framework, a markdown renderer, the

A graph/introspection_test.go => graph/introspection_test.go +118 -0
@@ 0,0 1,118 @@
package graph

import (
	"encoding/json"
	"fmt"
	"strings"
	"testing"

	"github.com/stretchr/testify/assert"
	"github.com/stretchr/testify/require"
)

// The introspection schema is not ours: it comes from gqlparser's built-in
// prelude, while the code that executes it is generated by gqlgen. The two are
// versioned separately, and gqlgen's generated executor answers a field it does
// not know with panic("unknown field") — recovered into "internal system error"
// rather than into anything a caller can act on. So a gqlparser that has
// learned a newer edition of the introspection schema than the pinned gqlgen
// can execute leaves this endpoint advertising fields that fail when selected.
//
// That is not hypothetical: gqlparser v2.5.22 added the @oneOf directive,
// __Type.isOneOf, __InputValue.isDeprecated and deprecationReason, and the
// includeDeprecated arguments, for the oneOf-input-objects and
// deprecated-arguments spec changes. Against gqlgen v0.17.36 a query selecting
// isDeprecated on an argument answers 422 and "internal system error", which is
// why go.mod holds gqlparser at v2.5.21 — see the note there.
//
// This test derives the field list from whichever prelude is in use instead of
// hard-coding one, so a future gqlparser bump that outpaces the pinned gqlgen
// fails here rather than in a consumer federating this /query.
func TestIntrospectionSchemaIsFullyExecutable(t *testing.T) {
	h := newHarness(t, false)

	// Only __Schema and __Type are reachable from the query root; the rest are
	// reachable only underneath it. Each path is anchored on something this
	// schema actually has, so the selection is executed rather than skipped
	// over an empty list.
	paths := []struct {
		typeName string
		query    string // one %s, for the selection set
	}{
		{"__Schema", `{ __schema { %s } }`},
		{"__Type", `{ __type(name: "Query") { %s } }`},
		{"__Field", `{ __type(name: "Query") { fields { %s } } }`},
		{"__InputValue", `{ __type(name: "Query") { fields { args { %s } } } }`},
		{"__EnumValue", `{ __type(name: "ProposalState") { enumValues { %s } } }`},
		{"__Directive", `{ __schema { directives { %s } } }`},
	}

	for _, p := range paths {
		t.Run(p.typeName, func(t *testing.T) {
			sel, leaves := introspectionSelection(t, h, p.typeName)
			require.NotEmpty(t, sel, "no fields discovered on %s: the check would pass vacuously", p.typeName)
			require.NotEmpty(t, leaves, "no leaf fields on %s, so nothing below proves execution", p.typeName)

			r := query(t, h, fmt.Sprintf(p.query, strings.Join(sel, " ")))

			assert.Equal(t, 200, r.status, "body: %s", r.body)
			assert.Empty(t, r.errText(), "selecting every declared field of %s failed; "+
				"gqlparser's prelude and gqlgen's generated executor have diverged", p.typeName)
			// An anchor that resolved to an empty list would report no error
			// while executing none of the fields under test, so require that at
			// least one of them actually came back.
			assert.Contains(t, r.body, fmt.Sprintf("%q:", leaves[0]),
				"%s: no %q key in the response, so the anchor executed none of its fields",
				p.typeName, leaves[0])
		})
	}
}

// introspectionSelection asks the endpoint which fields it declares on an
// introspection type, and renders them as a selection set: leaf fields bare,
// composite fields with a __typename subselection so the query is valid
// whatever the field's type turns out to be. It returns the selection and the
// names of the leaf fields, which is what the caller can look for in the
// response to know the selection was executed and not skipped.
func introspectionSelection(t *testing.T, h harness, typeName string) (sel, leaves []string) {
	t.Helper()

	r := query(t, h, fmt.Sprintf(`{ __type(name: %q) { fields(includeDeprecated: true) {
		name type { kind ofType { kind ofType { kind ofType { kind } } } }
	} } }`, typeName))
	require.Equal(t, 200, r.status, "discovering fields of %s: %s", typeName, r.body)
	require.Empty(t, r.errText(), "discovering fields of %s", typeName)

	var out struct {
		Type struct {
			Fields []struct {
				Name string `json:"name"`
				Type introspectedType
			} `json:"fields"`
		} `json:"__type"`
	}
	require.NoError(t, json.Unmarshal(r.Data, &out), "decode %s", r.body)

	for _, f := range out.Type.Fields {
		if k := f.Type.baseKind(); k == "SCALAR" || k == "ENUM" {
			sel = append(sel, f.Name)
			leaves = append(leaves, f.Name)
			continue
		}
		sel = append(sel, f.Name+" { __typename }")
	}
	return sel, leaves
}

type introspectedType struct {
	Kind   string            `json:"kind"`
	OfType *introspectedType `json:"ofType"`
}

// baseKind unwraps the NON_NULL and LIST wrappers to the underlying kind.
func (t introspectedType) baseKind() string {
	if (t.Kind == "NON_NULL" || t.Kind == "LIST") && t.OfType != nil {
		return t.OfType.baseKind()
	}
	return t.Kind
}