Skip to content

Commit

Permalink
Support for repeatable directives
Browse files Browse the repository at this point in the history
Replay of #412 based on main
  • Loading branch information
yanns committed Aug 3, 2022
1 parent 32f9dc8 commit ba1c86e
Show file tree
Hide file tree
Showing 30 changed files with 392 additions and 55 deletions.
1 change: 1 addition & 0 deletions modules/ast/src/main/scala/sangria/ast/QueryAst.scala
Expand Up @@ -698,6 +698,7 @@ case class DirectiveDefinition(
arguments: Vector[InputValueDefinition],
locations: Vector[DirectiveLocation],
description: Option[StringValue] = None,
repeatable: Boolean = false,
comments: Vector[Comment] = Vector.empty,
location: Option[AstLocation] = None)
extends TypeSystemDefinition
Expand Down
2 changes: 1 addition & 1 deletion modules/core/src/main/scala/sangria/ast/AstVisitor.scala
Expand Up @@ -405,7 +405,7 @@ object AstVisitor {
tc.foreach(c => loop(c))
breakOrSkip(onLeave(n))
}
case n @ DirectiveDefinition(_, args, locations, description, comment, _) =>
case n @ DirectiveDefinition(_, args, locations, description, _, comment, _) =>
if (breakOrSkip(onEnter(n))) {
args.foreach(d => loop(d))
locations.foreach(d => loop(d))
Expand Down
Expand Up @@ -121,7 +121,8 @@ object IntrospectionParser {
args = mapFieldOpt(directive, "args")
.map(um.getListValue)
.getOrElse(Vector.empty)
.map(arg => parseInputValue(arg, path :+ "args"))
.map(arg => parseInputValue(arg, path :+ "args")),
repeatable = mapBooleanFieldOpt(directive, "isRepeatable").getOrElse(false)
)

private def parseType[In: InputUnmarshaller](tpe: In, path: Vector[String]) =
Expand Down Expand Up @@ -209,6 +210,12 @@ object IntrospectionParser {
path: Vector[String]): Boolean =
booleanValue(mapField(map, name, path), path :+ name)

private def mapBooleanFieldOpt[In: InputUnmarshaller](
map: In,
name: String,
path: Vector[String] = Vector.empty): Option[Boolean] =
mapFieldOpt(map, name).filter(um.isDefined).map(booleanValue(_, path :+ name))

private def mapFieldOpt[In: InputUnmarshaller](map: In, name: String): Option[In] =
um.getMapValue(map, name).filter(um.isDefined)

Expand Down
Expand Up @@ -124,4 +124,5 @@ case class IntrospectionDirective(
name: String,
description: Option[String],
locations: Set[DirectiveLocation.Value],
args: Seq[IntrospectionInputValue])
args: Seq[IntrospectionInputValue],
repeatable: Boolean)
18 changes: 14 additions & 4 deletions modules/core/src/main/scala/sangria/introspection/package.scala
Expand Up @@ -368,7 +368,12 @@ package object introspection {
"locations",
ListType(__DirectiveLocation),
resolve = _.value.locations.toVector.sorted),
Field("args", ListType(__InputValue), resolve = _.value.arguments)
Field("args", ListType(__InputValue), resolve = _.value.arguments),
Field(
"isRepeatable",
BooleanType,
Some("Permits using the directive multiple times at the same location."),
resolve = _.value.repeatable)
)
)

Expand Down Expand Up @@ -444,10 +449,14 @@ package object introspection {

def introspectionQuery: ast.Document = introspectionQuery()

def introspectionQuery(schemaDescription: Boolean = true): ast.Document =
QueryParser.parse(introspectionQueryString(schemaDescription)).get
def introspectionQuery(
schemaDescription: Boolean = true,
directiveRepeatableFlag: Boolean = true): ast.Document =
QueryParser.parse(introspectionQueryString(schemaDescription, directiveRepeatableFlag)).get

def introspectionQueryString(schemaDescription: Boolean = true): String =
def introspectionQueryString(
schemaDescription: Boolean = true,
directiveRepeatableFlag: Boolean = true): String =
s"""query IntrospectionQuery {
| __schema {
| queryType { name }
Expand All @@ -463,6 +472,7 @@ package object introspection {
| args {
| ...InputValue
| }
| ${if (directiveRepeatableFlag) "isRepeatable" else ""}
| }
| ${if (schemaDescription) "description" else ""}
| }
Expand Down
4 changes: 2 additions & 2 deletions modules/core/src/main/scala/sangria/macros/AstLiftable.scala
Expand Up @@ -73,8 +73,8 @@ trait AstLiftable {
case FragmentDefinition(n, t, d, s, v, c, tc, p) =>
q"_root_.sangria.ast.FragmentDefinition($n, $t, $d, $s, $v, $c, $tc, $p)"

case DirectiveDefinition(n, a, l, desc, c, p) =>
q"_root_.sangria.ast.DirectiveDefinition($n, $a, $l, $desc, $c, $p)"
case DirectiveDefinition(n, a, l, desc, r, c, p) =>
q"_root_.sangria.ast.DirectiveDefinition($n, $a, $l, $desc, $r, $c, $p)"
case SchemaDefinition(o, d, desc, c, tc, p) =>
q"_root_.sangria.ast.SchemaDefinition($o, $d, $desc, $c, $tc, $p)"

Expand Down
Expand Up @@ -772,7 +772,7 @@ object QueryRenderer {
renderDirs(dirs, config, indent, frontSep = true) +
renderOperationTypeDefinitions(ops, ext, indent, config, frontSep = true)

case dd @ DirectiveDefinition(name, args, locations, description, _, _) =>
case dd @ DirectiveDefinition(name, args, locations, description, rep, _, _) =>
val locsRendered = locations.iterator.zipWithIndex
.map { case (l, idx) =>
(if (idx != 0 && shouldRenderComment(l, None, config)) config.lineBreak else "") +
Expand All @@ -790,6 +790,7 @@ object QueryRenderer {
indent.str + "directive" + config.separator + "@" + name +
renderInputValueDefs(args, indent, config) + (if (args.isEmpty) config.mandatorySeparator
else "") +
(if (rep) "repeatable" + config.mandatorySeparator else "") +
"on" + (if (shouldRenderComment(locations.head, None, config)) ""
else config.mandatorySeparator) +
locsRendered.mkString(config.separator + "|")
Expand Down
Expand Up @@ -317,14 +317,17 @@ object SchemaRenderer {
dir.name,
renderArgs(dir.arguments),
dir.locations.toVector.map(renderDirectiveLocation).sortBy(_.name),
renderDescription(dir.description))
renderDescription(dir.description),
dir.repeatable
)

def renderDirective(dir: IntrospectionDirective) =
ast.DirectiveDefinition(
dir.name,
renderArgsI(dir.args),
dir.locations.toVector.map(renderDirectiveLocation).sortBy(_.name),
renderDescription(dir.description))
renderDescription(dir.description),
dir.repeatable)

def schemaAstFromIntrospection(
introspectionSchema: IntrospectionSchema,
Expand Down
Expand Up @@ -708,6 +708,7 @@ class DefaultAstSchemaBuilder[Ctx] extends AstSchemaBuilder[Ctx] {
description = directiveDescription(definition),
locations = locations,
arguments = arguments,
repeatable = definition.repeatable,
shouldInclude = directiveShouldInclude(definition)
))

Expand Down
Expand Up @@ -279,6 +279,7 @@ class DefaultIntrospectionSchemaBuilder[Ctx] extends IntrospectionSchemaBuilder[
description = directiveDescription(definition),
locations = definition.locations,
arguments = arguments,
repeatable = definition.repeatable,
shouldInclude = directiveShouldInclude(definition)
))

Expand Down
Expand Up @@ -132,6 +132,30 @@ class ResolverBasedAstSchemaBuilder[Ctx](val resolvers: Seq[AstSchemaResolver[Ct
case r @ AnyFieldResolver(fn) if fn.isDefinedAt(origin) => r
}

override def buildSchema(
definition: Option[ast.SchemaDefinition],
extensions: List[ast.SchemaExtensionDefinition],
queryType: ObjectType[Ctx, Any],
mutationType: Option[ObjectType[Ctx, Any]],
subscriptionType: Option[ObjectType[Ctx, Any]],
additionalTypes: List[Type with Named],
directives: List[Directive],
mat: AstSchemaMaterializer[Ctx]) =
Schema[Ctx, Any](
query = queryType,
mutation = mutationType,
subscription = subscriptionType,
additionalTypes = additionalTypes,
description = definition.flatMap(_.description.map(_.value)),
directives = directives,
astDirectives =
definition.fold(Vector.empty[ast.Directive])(_.directives) ++ extensions.flatMap(
_.directives),
astNodes = Vector(mat.document) ++ extensions ++ definition.toVector,
validationRules = SchemaValidationRule.default :+ new ResolvedDirectiveValidationRule(
this.directives.filterNot(_.repeatable).map(_.name).toSet)
)

override def resolveField(
origin: MatOrigin,
typeDefinition: Either[ast.TypeDefinition, ObjectLikeType[Ctx, _]],
Expand Down
1 change: 1 addition & 0 deletions modules/core/src/main/scala/sangria/schema/Schema.scala
Expand Up @@ -1209,6 +1209,7 @@ case class Directive(
description: Option[String] = None,
arguments: List[Argument[_]] = Nil,
locations: Set[DirectiveLocation.Value] = Set.empty,
repeatable: Boolean = false,
shouldInclude: DirectiveContext => Boolean = _ => true)
extends HasArguments
with Named {
Expand Down
Expand Up @@ -64,6 +64,17 @@ object SchemaComparator {
}

private def findInDirective(oldDir: Directive, newDir: Directive): Vector[SchemaChange] = {
val repeatableChanged =
if (oldDir.repeatable != newDir.repeatable)
Vector(
SchemaChange.DirectiveRepeatableChanged(
newDir,
oldDir.repeatable,
newDir.repeatable,
!newDir.repeatable))
else
Vector.empty

val locationChanges = findInDirectiveLocations(oldDir, newDir)
val fieldChanges = findInArgs(
oldDir.arguments,
Expand All @@ -77,7 +88,7 @@ object SchemaComparator {
dirRemoved = SchemaChange.DirectiveArgumentAstDirectiveRemoved(newDir, _, _)
)

locationChanges ++ fieldChanges
repeatableChanged ++ locationChanges ++ fieldChanges
}

private def findInDirectiveLocations(
Expand Down Expand Up @@ -1045,6 +1056,16 @@ object SchemaChange {
s"Argument `${argument.name}` was added to `${directive.name}` directive",
breaking)

case class DirectiveRepeatableChanged(
directive: Directive,
oldRepeatable: Boolean,
newRepeatable: Boolean,
breaking: Boolean)
extends AbstractChange(
if (newRepeatable) s"Directive `${directive.name}` was made repeatable per location"
else s"Directive `${directive.name}` was made unique per location",
breaking)

case class InputFieldTypeChanged(
tpe: InputObjectType[_],
field: InputField[_],
Expand Down
Expand Up @@ -751,6 +751,44 @@ class FullSchemaTraversalValidationRule(validators: SchemaElementValidator*)
def validName(name: String): Boolean = !reservedNames.contains(name)
}

/** Validates uniqueness of directives on types and the schema definition.
*
* It is not fully covered by `UniqueDirectivesPerLocation` since it onl looks at one AST node at a
* time, so it does not cover type + type extension scenario.
*/
class ResolvedDirectiveValidationRule(knownUniqueDirectives: Set[String])
extends SchemaValidationRule {
def validate[Ctx, Val](schema: Schema[Ctx, Val]): List[Violation] = {
val uniqueDirectives =
knownUniqueDirectives ++ schema.directives.filterNot(_.repeatable).map(_.name)
val sourceMapper = SchemaElementValidator.sourceMapper(schema)

val schemaViolations = validateUniqueDirectives(schema, uniqueDirectives, sourceMapper)

val typeViolations =
schema.typeList.collect { case withDirs: HasAstInfo =>
validateUniqueDirectives(withDirs, uniqueDirectives, sourceMapper)
}

schemaViolations.toList ++ typeViolations.flatten
}

private def validateUniqueDirectives(
withDirs: HasAstInfo,
uniqueDirectives: Set[String],
sourceMapper: Option[SourceMapper]) = {
val duplicates = withDirs.astDirectives
.filter(d => uniqueDirectives.contains(d.name))
.groupBy(_.name)
.filter(_._2.size > 1)
.toVector

duplicates.map { case (dirName, dups) =>
DuplicateDirectiveViolation(dirName, sourceMapper, dups.flatMap(_.location).toList)
}
}
}

case class SchemaValidationException(
violations: Vector[Violation],
eh: ExceptionHandler = ExceptionHandler.empty)
Expand Down
Expand Up @@ -12,6 +12,8 @@ import scala.collection.mutable.{Map => MutableMap}
*/
class UniqueDirectivesPerLocation extends ValidationRule {
override def visitor(ctx: ValidationContext) = new AstValidatingVisitor {
private val repeatableDirectives = ctx.schema.directivesByName.mapValues(d => d.repeatable)

override val onEnter: ValidationVisit = {
// Many different AST nodes may contain directives. Rather than listing
// them all, just listen for entering any node, and check to see if it
Expand All @@ -20,14 +22,16 @@ class UniqueDirectivesPerLocation extends ValidationRule {
val knownDirectives = MutableMap[String, ast.Directive]()

val errors = node.directives.foldLeft(Vector.empty[Violation]) {
case (errors, d) if knownDirectives contains d.name =>
errors :+ DuplicateDirectiveViolation(
case (es, d) if repeatableDirectives.getOrElse(d.name, true) =>
es
case (es, d) if knownDirectives.contains(d.name) =>
es :+ DuplicateDirectiveViolation(
d.name,
ctx.sourceMapper,
knownDirectives(d.name).location.toList ++ d.location.toList)
case (errors, d) =>
case (es, d) =>
knownDirectives(d.name) = d
errors
es
}

if (errors.nonEmpty) Left(errors)
Expand Down
Expand Up @@ -84,4 +84,6 @@ extend type Foo @onType
"cool skip"
directive @skip(if: Boolean!) on FIELD | FRAGMENT_SPREAD | INLINE_FRAGMENT
directive @include(if: Boolean!) on FIELD | FRAGMENT_SPREAD | INLINE_FRAGMENT
directive @myRepeatableDir(name: String!) repeatable on OBJECT | INTERFACE
directive @include(if: Boolean!) on FIELD | FRAGMENT_SPREAD | INLINE_FRAGMENT
Expand Up @@ -78,11 +78,15 @@ extend type Foo {
seven(argument: [String]): Type
}
extend type Foo @onType
extend type Foo @onType
"cool skip"
directive @skip(if: Boolean!) on FIELD | FRAGMENT_SPREAD | INLINE_FRAGMENT
directive @myRepeatableDir(name: String!) repeatable on
| OBJECT
| INTERFACE
directive @include(if: Boolean!)
on FIELD
| FRAGMENT_SPREAD
Expand Down
Expand Up @@ -108,6 +108,17 @@ class IntrospectionSpec extends AnyWordSpec with Matchers with FutureResultSuppo
),
"isDeprecated" -> false,
"deprecationReason" -> null
),
Map(
"name" -> "isRepeatable",
"description" -> "Permits using the directive multiple times at the same location.",
"args" -> Vector.empty,
"type" -> Map(
"kind" -> "NON_NULL",
"name" -> null,
"ofType" -> Map("kind" -> "SCALAR", "name" -> "Boolean", "ofType" -> null)),
"isDeprecated" -> false,
"deprecationReason" -> null
)
),
"inputFields" -> null,
Expand Down Expand Up @@ -714,7 +725,8 @@ class IntrospectionSpec extends AnyWordSpec with Matchers with FutureResultSuppo
"name" -> null,
"ofType" -> Map("kind" -> "SCALAR", "name" -> "Boolean", "ofType" -> null)),
"defaultValue" -> null
))
)),
"isRepeatable" -> false
),
Map(
"name" -> "skip",
Expand All @@ -728,7 +740,8 @@ class IntrospectionSpec extends AnyWordSpec with Matchers with FutureResultSuppo
"name" -> null,
"ofType" -> Map("kind" -> "SCALAR", "name" -> "Boolean", "ofType" -> null)),
"defaultValue" -> null
))
)),
"isRepeatable" -> false
),
Map(
"name" -> "deprecated",
Expand All @@ -739,7 +752,8 @@ class IntrospectionSpec extends AnyWordSpec with Matchers with FutureResultSuppo
"description" -> "Explains why this element was deprecated, usually also including a suggestion for how to access supported similar data. Formatted in [Markdown](https://daringfireball.net/projects/markdown/).",
"type" -> Map("kind" -> "SCALAR", "name" -> "String", "ofType" -> null),
"defaultValue" -> "\"No longer supported\""
))
)),
"isRepeatable" -> false
)
),
"description" -> null
Expand Down
Expand Up @@ -1270,6 +1270,7 @@ class LiteralMacroSpec extends AnyWordSpec with Matchers {
DirectiveLocation("INLINE_FRAGMENT", Vector.empty, None)
),
None,
false,
Vector.empty,
None
),
Expand All @@ -1290,6 +1291,7 @@ class LiteralMacroSpec extends AnyWordSpec with Matchers {
DirectiveLocation("INLINE_FRAGMENT", Vector.empty, None)
),
None,
false,
Vector.empty,
None
)
Expand Down

0 comments on commit ba1c86e

Please sign in to comment.