Skip to content

Commit

Permalink
Merge pull request #635 from dackroyd/bugfix/fix-flaky-validation-err…
Browse files Browse the repository at this point in the history
…or-ordering-for-directives

Fix Flaky Order of Directive Validation
  • Loading branch information
pavelnikolov authored Apr 3, 2024
2 parents a3d0ced + 731ec86 commit 6029c30
Showing 1 changed file with 12 additions and 6 deletions.
18 changes: 12 additions & 6 deletions internal/validation/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -698,7 +698,15 @@ func validateDirectives(c *opContext, loc string, directives ast.DirectiveList)
)
}

for n := range directiveNames {
// Iterating in the declared order, rather than using the directiveNames ordering which is random
for _, d := range directives {
n := d.Name.Name

ds := directiveNames[n]
if len(ds) <= 1 {
continue
}

dd, ok := c.schema.Directives[n]
if !ok {
// Invalid directive will have been flagged already
Expand All @@ -709,11 +717,6 @@ func validateDirectives(c *opContext, loc string, directives ast.DirectiveList)
continue
}

ds := directiveNames[n]
if len(ds) <= 1 {
continue
}

for _, loc := range ds[1:] {
// Duplicate directive errors are inconsistent with the behaviour for other types in graphql-js
// Instead of reporting a single error with all locations, errors are reported for each duplicate after the first declaration
Expand All @@ -722,6 +725,9 @@ func validateDirectives(c *opContext, loc string, directives ast.DirectiveList)
return fmt.Sprintf("The directive %q can only be used once at this location.", "@"+n)
})
}

// drop the name from the set to prevent the same errors being re-added for duplicates
delete(directiveNames, n)
}
}

Expand Down

0 comments on commit 6029c30

Please sign in to comment.