Skip to content

Commit

Permalink
fix: Remove shared mutable state between database instances (#2777)
Browse files Browse the repository at this point in the history
## Relevant issue(s)

Resolves #2774

## Description

Removes shared mutable GQL state between database instances.

These global static GQL type variables are actually mutated by the GQL
library we use at different points during database init, and schema
update. This means that when multiple Defra instances are created in the
same process they were sharing state. As well as just being an
inherently bad thing, this also prevented us from running tests within
packages in parallel using `t.Parallel()` as the race detector would
complain.
  • Loading branch information
AndrewSisley authored Jun 26, 2024
1 parent 5855969 commit c6c2373
Show file tree
Hide file tree
Showing 11 changed files with 606 additions and 539 deletions.
2 changes: 1 addition & 1 deletion internal/request/graphql/schema/collection.go
Original file line number Diff line number Diff line change
Expand Up @@ -482,7 +482,7 @@ func setCRDTType(field *ast.FieldDefinition, kind client.FieldKind) (client.CTyp
switch arg.Name.Value {
case "type":
cTypeString := arg.Value.GetValue().(string)
cType, validCRDTEnum := types.CRDTEnum.ParseValue(cTypeString).(client.CType)
cType, validCRDTEnum := types.CRDTEnum().ParseValue(cTypeString).(client.CType)
if !validCRDTEnum {
return 0, client.NewErrInvalidCRDTType(field.Name.Value, cTypeString)
}
Expand Down
4 changes: 2 additions & 2 deletions internal/request/graphql/schema/descriptions.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ var (
client.FieldKind_NILLABLE_STRING: gql.String,
client.FieldKind_STRING_ARRAY: gql.NewList(gql.NewNonNull(gql.String)),
client.FieldKind_NILLABLE_STRING_ARRAY: gql.NewList(gql.String),
client.FieldKind_NILLABLE_BLOB: schemaTypes.BlobScalarType,
client.FieldKind_NILLABLE_JSON: schemaTypes.JSONScalarType,
client.FieldKind_NILLABLE_BLOB: schemaTypes.BlobScalarType(),
client.FieldKind_NILLABLE_JSON: schemaTypes.JSONScalarType(),
}

defaultCRDTForFieldKind = map[client.FieldKind]client.CType{
Expand Down
12 changes: 7 additions & 5 deletions internal/request/graphql/schema/generate.go
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ func (g *Generator) generate(ctx context.Context, collections []client.Collectio
}
}

appendCommitChildGroupField()
g.appendCommitChildGroupField()

// resolve types
if err := g.manager.ResolveTypes(); err != nil {
Expand Down Expand Up @@ -500,7 +500,7 @@ func (g *Generator) buildTypes(
// add _version field
fields[request.VersionFieldName] = &gql.Field{
Description: versionFieldDescription,
Type: gql.NewList(schemaTypes.CommitObject),
Type: gql.NewList(g.manager.schema.TypeMap()[request.CommitTypeName]),
}

// add _deleted field
Expand Down Expand Up @@ -987,11 +987,13 @@ func (g *Generator) genNumericAggregateBaseArgInputs(obj *gql.Object) *gql.Input
})
}

func appendCommitChildGroupField() {
schemaTypes.CommitObject.Fields()[request.GroupFieldName] = &gql.FieldDefinition{
func (g *Generator) appendCommitChildGroupField() {
commitObject := g.manager.schema.TypeMap()[request.CommitTypeName]

commitObject.(*gql.Object).Fields()[request.GroupFieldName] = &gql.FieldDefinition{
Name: request.GroupFieldName,
Description: groupFieldDescription,
Type: gql.NewList(schemaTypes.CommitObject),
Type: gql.NewList(commitObject),
}
}

Expand Down
100 changes: 65 additions & 35 deletions internal/request/graphql/schema/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,27 @@ type SchemaManager struct {
// with a new default type map
func NewSchemaManager() (*SchemaManager, error) {
sm := &SchemaManager{}

orderEnum := schemaTypes.OrderingEnum()
crdtEnum := schemaTypes.CRDTEnum()
explainEnum := schemaTypes.ExplainEnum()

commitLinkObject := schemaTypes.CommitLinkObject()
commitObject := schemaTypes.CommitObject(commitLinkObject)
commitsOrderArg := schemaTypes.CommitsOrderArg(orderEnum)

schema, err := gql.NewSchema(gql.SchemaConfig{
Types: defaultTypes(),
Query: defaultQueryType(),
Types: defaultTypes(
commitObject,
commitLinkObject,
commitsOrderArg,
orderEnum,
crdtEnum,
explainEnum,
),
Query: defaultQueryType(commitObject, commitsOrderArg),
Mutation: defaultMutationType(),
Directives: defaultDirectivesType(),
Directives: defaultDirectivesType(crdtEnum, explainEnum, orderEnum),
})
if err != nil {
return sm, err
Expand Down Expand Up @@ -80,7 +96,10 @@ func (s *SchemaManager) ResolveTypes() error {
}

// @todo: Use a better default Query type
func defaultQueryType() *gql.Object {
func defaultQueryType(commitObject *gql.Object, commitsOrderArg *gql.InputObject) *gql.Object {
queryCommits := schemaTypes.QueryCommits(commitObject, commitsOrderArg)
queryLatestCommits := schemaTypes.QueryLatestCommits(commitObject)

return gql.NewObject(gql.ObjectConfig{
Name: "Query",
Fields: gql.Fields{
Expand All @@ -90,8 +109,8 @@ func defaultQueryType() *gql.Object {
},

// database API queries
schemaTypes.QueryCommits.Name: schemaTypes.QueryCommits,
schemaTypes.QueryLatestCommits.Name: schemaTypes.QueryLatestCommits,
queryCommits.Name: queryCommits,
queryLatestCommits.Name: queryLatestCommits,
},
})
}
Expand All @@ -109,15 +128,19 @@ func defaultMutationType() *gql.Object {
}

// default directives type.
func defaultDirectivesType() []*gql.Directive {
func defaultDirectivesType(
crdtEnum *gql.Enum,
explainEnum *gql.Enum,
orderEnum *gql.Enum,
) []*gql.Directive {
return []*gql.Directive{
schemaTypes.CRDTFieldDirective,
schemaTypes.ExplainDirective,
schemaTypes.PolicyDirective,
schemaTypes.IndexDirective,
schemaTypes.IndexFieldDirective,
schemaTypes.PrimaryDirective,
schemaTypes.RelationDirective,
schemaTypes.CRDTFieldDirective(crdtEnum),
schemaTypes.ExplainDirective(explainEnum),
schemaTypes.PolicyDirective(),
schemaTypes.IndexDirective(orderEnum),
schemaTypes.IndexFieldDirective(orderEnum),
schemaTypes.PrimaryDirective(),
schemaTypes.RelationDirective(),
}
}

Expand All @@ -135,7 +158,14 @@ func inlineArrayTypes() []gql.Type {
}

// default type map includes all the native scalar types
func defaultTypes() []gql.Type {
func defaultTypes(
commitObject *gql.Object,
commitLinkObject *gql.Object,
commitsOrderArg *gql.InputObject,
orderEnum *gql.Enum,
crdtEnum *gql.Enum,
explainEnum *gql.Enum,
) []gql.Type {
return []gql.Type{
// Base Scalar types
gql.Boolean,
Expand All @@ -146,31 +176,31 @@ func defaultTypes() []gql.Type {
gql.String,

// Custom Scalar types
schemaTypes.BlobScalarType,
schemaTypes.JSONScalarType,
schemaTypes.BlobScalarType(),
schemaTypes.JSONScalarType(),

// Base Query types

// Sort/Order enum
schemaTypes.OrderingEnum,
orderEnum,

// Filter scalar blocks
schemaTypes.BooleanOperatorBlock,
schemaTypes.NotNullBooleanOperatorBlock,
schemaTypes.DateTimeOperatorBlock,
schemaTypes.FloatOperatorBlock,
schemaTypes.NotNullFloatOperatorBlock,
schemaTypes.IdOperatorBlock,
schemaTypes.IntOperatorBlock,
schemaTypes.NotNullIntOperatorBlock,
schemaTypes.StringOperatorBlock,
schemaTypes.NotNullstringOperatorBlock,

schemaTypes.CommitsOrderArg,
schemaTypes.CommitLinkObject,
schemaTypes.CommitObject,

schemaTypes.CRDTEnum,
schemaTypes.ExplainEnum,
schemaTypes.BooleanOperatorBlock(),
schemaTypes.NotNullBooleanOperatorBlock(),
schemaTypes.DateTimeOperatorBlock(),
schemaTypes.FloatOperatorBlock(),
schemaTypes.NotNullFloatOperatorBlock(),
schemaTypes.IdOperatorBlock(),
schemaTypes.IntOperatorBlock(),
schemaTypes.NotNullIntOperatorBlock(),
schemaTypes.StringOperatorBlock(),
schemaTypes.NotNullstringOperatorBlock(),

commitsOrderArg,
commitLinkObject,
commitObject,

crdtEnum,
explainEnum,
}
}
11 changes: 0 additions & 11 deletions internal/request/graphql/schema/manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,17 +21,6 @@ func Test_SchemaManager_NewNoErrs(t *testing.T) {
assert.NoError(t, err, "NewSchemaManager returned an error")
}

func Test_SchemaManager_HasDefaultTypes(t *testing.T) {
s, err := NewSchemaManager()
assert.NoError(t, err, "NewSchemaManager returned an error")

tm := s.schema.TypeMap()
for _, ty := range defaultTypes() {
_, ok := tm[ty.Name()]
assert.True(t, ok, "TypeMap missing default type %s", ty.Name())
}
}

func Test_SchemaManager_ResolveTypes(t *testing.T) {
s, _ := NewSchemaManager()
err := s.ResolveTypes()
Expand Down
Loading

0 comments on commit c6c2373

Please sign in to comment.