Merge pull request #899 from sangria-graphql/repeatable-directives-v2

Support for repeatable directives

build.sbt

@@ -1,6 +1,8 @@
 import sbt.Developer
 import sbt.Keys._
 
+import com.typesafe.tools.mima.core._
+
 // sbt-github-actions needs configuration in `ThisBuild`
 ThisBuild / crossScalaVersions := Seq("2.12.16", "2.13.8")
 ThisBuild / scalaVersion := crossScalaVersions.value.last
@@ -44,6 +46,13 @@ lazy val ast = project
     name := "sangria-ast",
     description := "Scala GraphQL AST representation",
     mimaPreviousArtifacts := Set("org.sangria-graphql" %% "sangria-ast" % "3.0.0"),
+    mimaBinaryIssueFilters ++= Seq(
+      ProblemFilters.exclude[IncompatibleResultTypeProblem]("sangria.ast.DirectiveDefinition.*"),
+      ProblemFilters.exclude[DirectMissingMethodProblem]("sangria.ast.DirectiveDefinition.apply"),
+      ProblemFilters.exclude[DirectMissingMethodProblem]("sangria.ast.DirectiveDefinition.copy"),
+      ProblemFilters.exclude[DirectMissingMethodProblem]("sangria.ast.DirectiveDefinition.this"),
+      ProblemFilters.exclude[MissingTypesProblem]("sangria.ast.DirectiveDefinition$")
+    ),
     apiURL := {
       val ver = CrossVersion.binaryScalaVersion(scalaVersion.value)
       Some(url(s"https://www.javadoc.io/doc/org.sangria-graphql/sangria-ast_$ver/latest/"))
@@ -79,6 +88,29 @@ lazy val core = project
     name := "sangria-core",
     description := "Scala GraphQL implementation",
     mimaPreviousArtifacts := Set("org.sangria-graphql" %% "sangria-core" % "3.0.0"),
+    mimaBinaryIssueFilters ++= Seq(
+      ProblemFilters.exclude[DirectMissingMethodProblem](
+        "sangria.introspection.IntrospectionDirective.apply"),
+      ProblemFilters.exclude[DirectMissingMethodProblem](
+        "sangria.introspection.IntrospectionDirective.copy"),
+      ProblemFilters.exclude[DirectMissingMethodProblem](
+        "sangria.introspection.IntrospectionDirective.this"),
+      ProblemFilters.exclude[MissingTypesProblem]("sangria.introspection.IntrospectionDirective$"),
+      ProblemFilters.exclude[DirectMissingMethodProblem](
+        "sangria.introspection.IntrospectionDirective.apply"),
+      ProblemFilters.exclude[DirectMissingMethodProblem](
+        "sangria.introspection.package.introspectionQueryString"),
+      ProblemFilters.exclude[DirectMissingMethodProblem](
+        "sangria.introspection.package.introspectionQuery"),
+      ProblemFilters.exclude[IncompatibleResultTypeProblem]("sangria.schema.Directive.<init>*"),
+      ProblemFilters.exclude[IncompatibleResultTypeProblem]("sangria.schema.Directive.apply*"),
+      ProblemFilters.exclude[DirectMissingMethodProblem]("sangria.schema.Directive.copy"),
+      ProblemFilters.exclude[IncompatibleResultTypeProblem](
+        "sangria.schema.Directive.copy$default$*"),
+      ProblemFilters.exclude[DirectMissingMethodProblem]("sangria.schema.Directive.apply"),
+      ProblemFilters.exclude[DirectMissingMethodProblem]("sangria.schema.Directive.this"),
+      ProblemFilters.exclude[MissingTypesProblem]("sangria.schema.Directive$")
+    ),
     Test / testOptions += Tests.Argument(TestFrameworks.ScalaTest, "-oF"),
     libraryDependencies ++= Seq(
       // AST Visitor

modules/ast/src/main/scala/sangria/ast/QueryAst.scala

@@ -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

modules/core/src/main/scala/sangria/ast/AstVisitor.scala

@@ -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))

modules/core/src/main/scala/sangria/introspection/IntrospectionParser.scala

@@ -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]) =
@@ -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)
 

modules/core/src/main/scala/sangria/introspection/model.scala

@@ -124,4 +124,5 @@ case class IntrospectionDirective(
     name: String,
     description: Option[String],
     locations: Set[DirectiveLocation.Value],
-    args: Seq[IntrospectionInputValue])
+    args: Seq[IntrospectionInputValue],
+    repeatable: Boolean)

modules/core/src/main/scala/sangria/introspection/package.scala

@@ -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)
     )
   )
 
@@ -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 }
@@ -463,6 +472,7 @@ package object introspection {
        |      args {
        |        ...InputValue
        |      }
+       |      ${if (directiveRepeatableFlag) "isRepeatable" else ""}
        |    }
        |    ${if (schemaDescription) "description" else ""}
        |  }

modules/core/src/main/scala/sangria/macros/AstLiftable.scala

@@ -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)"
 

modules/core/src/main/scala/sangria/renderer/QueryRenderer.scala

@@ -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 "") +
@@ -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 + "|")

modules/core/src/main/scala/sangria/renderer/SchemaRenderer.scala

@@ -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,

modules/core/src/main/scala/sangria/schema/AstSchemaBuilder.scala

@@ -708,6 +708,7 @@ class DefaultAstSchemaBuilder[Ctx] extends AstSchemaBuilder[Ctx] {
         description = directiveDescription(definition),
         locations = locations,
         arguments = arguments,
+        repeatable = definition.repeatable,
         shouldInclude = directiveShouldInclude(definition)
       ))
 

modules/core/src/main/scala/sangria/schema/IntrospectionSchemaBuilder.scala

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

modules/core/src/main/scala/sangria/schema/ResolverBasedAstSchemaBuilder.scala

@@ -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, _]],

modules/core/src/main/scala/sangria/schema/Schema.scala

@@ -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 {

modules/core/src/main/scala/sangria/schema/SchemaComparator.scala

@@ -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,
@@ -77,7 +88,7 @@ object SchemaComparator {
       dirRemoved = SchemaChange.DirectiveArgumentAstDirectiveRemoved(newDir, _, _)
     )
 
-    locationChanges ++ fieldChanges
+    repeatableChanged ++ locationChanges ++ fieldChanges
   }
 
   private def findInDirectiveLocations(
@@ -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[_],

modules/core/src/main/scala/sangria/schema/SchemaValidationRule.scala

@@ -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)

modules/core/src/main/scala/sangria/validation/rules/UniqueDirectivesPerLocation.scala

@@ -12,6 +12,10 @@ import scala.collection.mutable.{Map => MutableMap}
   */
 class UniqueDirectivesPerLocation extends ValidationRule {
   override def visitor(ctx: ValidationContext) = new AstValidatingVisitor {
+    private val repeatableDirectives = ctx.schema.directivesByName.map { case (n, d) =>
+      (n, 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
@@ -20,14 +24,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)

modules/core/src/test/resources/queries/schema-kitchen-sink-pretty.graphql

@@ -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

modules/core/src/test/resources/queries/schema-kitchen-sink.graphql

@@ -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