diff --git a/build.sbt b/build.sbt index 9f96ab2c..32ec9b49 100644 --- a/build.sbt +++ b/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.*"), + 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 diff --git a/modules/ast/src/main/scala/sangria/ast/QueryAst.scala b/modules/ast/src/main/scala/sangria/ast/QueryAst.scala index ee5e861a..80b81563 100644 --- a/modules/ast/src/main/scala/sangria/ast/QueryAst.scala +++ b/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 diff --git a/modules/core/src/main/scala/sangria/ast/AstVisitor.scala b/modules/core/src/main/scala/sangria/ast/AstVisitor.scala index a44a1951..583a9e4e 100644 --- a/modules/core/src/main/scala/sangria/ast/AstVisitor.scala +++ b/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)) diff --git a/modules/core/src/main/scala/sangria/introspection/IntrospectionParser.scala b/modules/core/src/main/scala/sangria/introspection/IntrospectionParser.scala index 4b4dfdf0..bebff47b 100644 --- a/modules/core/src/main/scala/sangria/introspection/IntrospectionParser.scala +++ b/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) diff --git a/modules/core/src/main/scala/sangria/introspection/model.scala b/modules/core/src/main/scala/sangria/introspection/model.scala index c08a8f94..48c4805a 100644 --- a/modules/core/src/main/scala/sangria/introspection/model.scala +++ b/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) diff --git a/modules/core/src/main/scala/sangria/introspection/package.scala b/modules/core/src/main/scala/sangria/introspection/package.scala index 35d8bda8..ecc2af9b 100644 --- a/modules/core/src/main/scala/sangria/introspection/package.scala +++ b/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 ""} | } diff --git a/modules/core/src/main/scala/sangria/macros/AstLiftable.scala b/modules/core/src/main/scala/sangria/macros/AstLiftable.scala index 5edab4c4..b0b3f9ee 100644 --- a/modules/core/src/main/scala/sangria/macros/AstLiftable.scala +++ b/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)" diff --git a/modules/core/src/main/scala/sangria/renderer/QueryRenderer.scala b/modules/core/src/main/scala/sangria/renderer/QueryRenderer.scala index 33127d68..45939fb6 100644 --- a/modules/core/src/main/scala/sangria/renderer/QueryRenderer.scala +++ b/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 + "|") diff --git a/modules/core/src/main/scala/sangria/renderer/SchemaRenderer.scala b/modules/core/src/main/scala/sangria/renderer/SchemaRenderer.scala index ae85323a..6835a145 100644 --- a/modules/core/src/main/scala/sangria/renderer/SchemaRenderer.scala +++ b/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, diff --git a/modules/core/src/main/scala/sangria/schema/AstSchemaBuilder.scala b/modules/core/src/main/scala/sangria/schema/AstSchemaBuilder.scala index c5b05ae7..3527ddf1 100644 --- a/modules/core/src/main/scala/sangria/schema/AstSchemaBuilder.scala +++ b/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) )) diff --git a/modules/core/src/main/scala/sangria/schema/IntrospectionSchemaBuilder.scala b/modules/core/src/main/scala/sangria/schema/IntrospectionSchemaBuilder.scala index 35916d79..47a9e3e6 100644 --- a/modules/core/src/main/scala/sangria/schema/IntrospectionSchemaBuilder.scala +++ b/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) )) diff --git a/modules/core/src/main/scala/sangria/schema/ResolverBasedAstSchemaBuilder.scala b/modules/core/src/main/scala/sangria/schema/ResolverBasedAstSchemaBuilder.scala index b80cb2dc..5d894417 100644 --- a/modules/core/src/main/scala/sangria/schema/ResolverBasedAstSchemaBuilder.scala +++ b/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, _]], diff --git a/modules/core/src/main/scala/sangria/schema/Schema.scala b/modules/core/src/main/scala/sangria/schema/Schema.scala index e850378d..8e2e39f6 100644 --- a/modules/core/src/main/scala/sangria/schema/Schema.scala +++ b/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 { diff --git a/modules/core/src/main/scala/sangria/schema/SchemaComparator.scala b/modules/core/src/main/scala/sangria/schema/SchemaComparator.scala index e29f9d9e..17542b1b 100644 --- a/modules/core/src/main/scala/sangria/schema/SchemaComparator.scala +++ b/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[_], diff --git a/modules/core/src/main/scala/sangria/schema/SchemaValidationRule.scala b/modules/core/src/main/scala/sangria/schema/SchemaValidationRule.scala index 5ee02f5f..b8754011 100644 --- a/modules/core/src/main/scala/sangria/schema/SchemaValidationRule.scala +++ b/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) diff --git a/modules/core/src/main/scala/sangria/validation/rules/UniqueDirectivesPerLocation.scala b/modules/core/src/main/scala/sangria/validation/rules/UniqueDirectivesPerLocation.scala index 45e1bfd1..f76a5966 100644 --- a/modules/core/src/main/scala/sangria/validation/rules/UniqueDirectivesPerLocation.scala +++ b/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) diff --git a/modules/core/src/test/resources/queries/schema-kitchen-sink-pretty.graphql b/modules/core/src/test/resources/queries/schema-kitchen-sink-pretty.graphql index edfe9b79..1ec3f5df 100644 --- a/modules/core/src/test/resources/queries/schema-kitchen-sink-pretty.graphql +++ b/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 \ No newline at end of file +directive @myRepeatableDir(name: String!) repeatable on OBJECT | INTERFACE + +directive @include(if: Boolean!) on FIELD | FRAGMENT_SPREAD | INLINE_FRAGMENT diff --git a/modules/core/src/test/resources/queries/schema-kitchen-sink.graphql b/modules/core/src/test/resources/queries/schema-kitchen-sink.graphql index 5b6c0211..425c9489 100644 --- a/modules/core/src/test/resources/queries/schema-kitchen-sink.graphql +++ b/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 diff --git a/modules/core/src/test/scala/sangria/introspection/IntrospectionSpec.scala b/modules/core/src/test/scala/sangria/introspection/IntrospectionSpec.scala index 677ded38..313192bc 100644 --- a/modules/core/src/test/scala/sangria/introspection/IntrospectionSpec.scala +++ b/modules/core/src/test/scala/sangria/introspection/IntrospectionSpec.scala @@ -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, @@ -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", @@ -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", @@ -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 diff --git a/modules/core/src/test/scala/sangria/macros/LiteralMacroSpec.scala b/modules/core/src/test/scala/sangria/macros/LiteralMacroSpec.scala index 9e76252e..4164a07a 100644 --- a/modules/core/src/test/scala/sangria/macros/LiteralMacroSpec.scala +++ b/modules/core/src/test/scala/sangria/macros/LiteralMacroSpec.scala @@ -1270,6 +1270,7 @@ class LiteralMacroSpec extends AnyWordSpec with Matchers { DirectiveLocation("INLINE_FRAGMENT", Vector.empty, None) ), None, + false, Vector.empty, None ), @@ -1290,6 +1291,7 @@ class LiteralMacroSpec extends AnyWordSpec with Matchers { DirectiveLocation("INLINE_FRAGMENT", Vector.empty, None) ), None, + false, Vector.empty, None ) diff --git a/modules/core/src/test/scala/sangria/renderer/QueryRendererSpec.scala b/modules/core/src/test/scala/sangria/renderer/QueryRendererSpec.scala index 2f15c07f..7d234a7c 100644 --- a/modules/core/src/test/scala/sangria/renderer/QueryRendererSpec.scala +++ b/modules/core/src/test/scala/sangria/renderer/QueryRendererSpec.scala @@ -728,10 +728,12 @@ class QueryRendererSpec extends AnyWordSpec with Matchers with StringMatchers { |extend type Foo {seven(argument:[String]):Type} |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|INLINE_FRAGMENT""".stripMargin)( after.being(strippedOfCarriageReturns)) - prettyRendered should equal(FileUtil.loadQuery("schema-kitchen-sink-pretty.graphql")) + (prettyRendered + "\n") should equal( + FileUtil.loadQuery("schema-kitchen-sink-pretty.graphql")) } "renders schema with comments correctly" in { diff --git a/modules/core/src/test/scala/sangria/renderer/SchemaRenderSpec.scala b/modules/core/src/test/scala/sangria/renderer/SchemaRenderSpec.scala index f64777e8..77cc420e 100644 --- a/modules/core/src/test/scala/sangria/renderer/SchemaRenderSpec.scala +++ b/modules/core/src/test/scala/sangria/renderer/SchemaRenderSpec.scala @@ -797,6 +797,9 @@ class SchemaRenderSpec | description: String | locations: [__DirectiveLocation!]! | args: [__InputValue!]! + | + | "Permits using the directive multiple times at the same location." + | isRepeatable: Boolean! |} | |"A Directive can be adjacent to many parts of the GraphQL language, a __DirectiveLocation describes one such possible adjacencies." diff --git a/modules/core/src/test/scala/sangria/schema/AstSchemaMaterializerSpec.scala b/modules/core/src/test/scala/sangria/schema/AstSchemaMaterializerSpec.scala index dd3c255f..8b58c1f6 100644 --- a/modules/core/src/test/scala/sangria/schema/AstSchemaMaterializerSpec.scala +++ b/modules/core/src/test/scala/sangria/schema/AstSchemaMaterializerSpec.scala @@ -441,6 +441,21 @@ class AstSchemaMaterializerSpec cycleOutput(schema) should equal(schema)(after.being(strippedOfCarriageReturns)) } + + "Supports repeatable directives" in { + val schemaStr = + """type Query { + | str: String + |} + | + |directive @foo(arg: Int) repeatable on FIELD""".stripMargin + + cycleOutput(schemaStr) should equal(schemaStr)(after.being(strippedOfCarriageReturns)) + + val schema = Schema.buildFromAst(QueryParser.parse(schemaStr).get) + + schema.directivesByName("foo").repeatable should be(true) + } } "Failures" should { diff --git a/modules/core/src/test/scala/sangria/schema/IntrospectionSchemaMaterializerSpec.scala b/modules/core/src/test/scala/sangria/schema/IntrospectionSchemaMaterializerSpec.scala index c99e33fe..f20eb58b 100644 --- a/modules/core/src/test/scala/sangria/schema/IntrospectionSchemaMaterializerSpec.scala +++ b/modules/core/src/test/scala/sangria/schema/IntrospectionSchemaMaterializerSpec.scala @@ -387,7 +387,15 @@ class IntrospectionSchemaMaterializerSpec "customDirective", Some("This is a custom directive"), shouldInclude = _ => true, - locations = Set(DirectiveLocation.Field))) + locations = Set(DirectiveLocation.Field)), + Directive( + "customRepeatableDirective", + Some("This is a custom repeatable directive"), + shouldInclude = _ => true, + repeatable = true, + locations = Set(DirectiveLocation.Field) + ) + ) )) "builds a schema aware of deprecation" in testSchema( diff --git a/modules/core/src/test/scala/sangria/schema/ResolverBasedAstSchemaBuilderSpec.scala b/modules/core/src/test/scala/sangria/schema/ResolverBasedAstSchemaBuilderSpec.scala index 94b1335b..0ee009ce 100644 --- a/modules/core/src/test/scala/sangria/schema/ResolverBasedAstSchemaBuilderSpec.scala +++ b/modules/core/src/test/scala/sangria/schema/ResolverBasedAstSchemaBuilderSpec.scala @@ -904,5 +904,120 @@ class ResolverBasedAstSchemaBuilderSpec extends AnyWordSpec with Matchers with F } """.parseJson) } + + "validate unique directives on extensions" in { + val TestDir = Directive( + "dir", + locations = + Set(DL.Object, DL.Interface, DL.Union, DL.InputObject, DL.Scalar, DL.Schema, DL.Enum)) + + val TestRepDir = Directive( + "dir", + repeatable = true, + locations = + Set(DL.Object, DL.Interface, DL.Union, DL.InputObject, DL.Scalar, DL.Schema, DL.Enum)) + + val builder = resolverBased[Unit](AdditionalDirectives(Seq(TestDir))) + val builderRep = resolverBased[Unit](AdditionalDirectives(Seq(TestRepDir))) + + def testForUnique(schemaAst: ast.Document) = { + val error = intercept[SchemaValidationException]( + Schema.buildFromAst(schemaAst, builder.validateSchemaWithException(schemaAst))) + + error.violations should have size 1 + + error.violations.head.errorMessage should include( + "The directive 'dir' can only be used once at this location.") + + // should be successful + Schema.buildFromAst(schemaAst, builderRep.validateSchemaWithException(schemaAst)) + } + + testForUnique(gql""" + type Query @dir {field: String} + extend type Query @dir + """) + + testForUnique(gql""" + type Query {field: String} + interface Foo @dir {field: String} + extend interface Foo @dir + """) + + testForUnique(gql""" + type Query @dir {filed: String} + type Cat {field: String} + type Dog {field: String} + + union Foo @dir = Cat | Dog + + extend union Foo @dir + """) + + testForUnique(gql""" + type Query @dir {filed: String} + enum Foo @dir {RED, BLUE} + extend enum Foo @dir + """) + + testForUnique(gql""" + type Query @dir {filed: String} + scalar Foo @dir + extend scalar Foo @dir + """) + + testForUnique(gql""" + type Query @dir {filed: String} + input Foo @dir {field: String} + extend input Foo @dir + """) + + testForUnique(gql""" + type Query @dir {filed: String} + schema @dir {query: Query} + extend schema @dir + """) + } + + "validate unique directives on extensions with schema extension" in { + val TestDir = Directive( + "dir", + locations = + Set(DL.Object, DL.Interface, DL.Union, DL.InputObject, DL.Scalar, DL.Schema, DL.Enum)) + + val builder = resolverBased[Unit](AdditionalDirectives(Seq(TestDir))) + + val mainSchemaAst = gql"type Query @dir {field: String}" + val extSchemaAst = gql"extend type Query @dir" + + val schema = + Schema.buildFromAst(mainSchemaAst, builder.validateSchemaWithException(mainSchemaAst)) + + val error = intercept[SchemaValidationException]( + schema.extend(extSchemaAst, builder.validateSchemaWithException(extSchemaAst))) + + error.violations should have size 1 + error.violations.head.errorMessage should include( + "The directive 'dir' can only be used once at this location.") + } + + "allow empty field list on types as long as provider contributes new fields" in { + val TestDir = Directive("dir", locations = Set(DL.Object)) + + val builder = resolverBased[Any](DirectiveFieldProvider( + TestDir, + c => + List(MaterializedField( + c.origin, + Field("hello", c.scalarType("String"), resolve = (_: Context[Any, Any]) => "world"))))) + + val schemaAst = gql"type Query @dir" + + val schema = Schema.buildFromAst(schemaAst, builder.validateSchemaWithException(schemaAst)) + + val query = gql"{hello}" + + Executor.execute(schema, query).await should be(Map("data" -> Map("hello" -> "world"))) + } } } diff --git a/modules/core/src/test/scala/sangria/schema/SchemaComparatorSpec.scala b/modules/core/src/test/scala/sangria/schema/SchemaComparatorSpec.scala index 31340aa2..85396af8 100644 --- a/modules/core/src/test/scala/sangria/schema/SchemaComparatorSpec.scala +++ b/modules/core/src/test/scala/sangria/schema/SchemaComparatorSpec.scala @@ -157,6 +157,20 @@ class SchemaComparatorSpec extends AnyWordSpec with Matchers { nonBreakingChange[DirectiveArgumentAdded]("Argument `c` was added to `foo` directive") ) + "detect changes repeatable directive definition" in checkChanges( + gql""" + directive @a repeatable on OBJECT + directive @b(foo: Int) on OBJECT + """, + gql""" + directive @a on OBJECT + directive @b(foo: Int) repeatable on OBJECT + """, + breakingChange[DirectiveRepeatableChanged]("Directive `a` was made unique per location"), + nonBreakingChange[DirectiveRepeatableChanged]( + "Directive `b` was made repeatable per location") + ) + "should detect changes in input types" in checkChanges( graphql""" input Sort {dir: Int} diff --git a/modules/core/src/test/scala/sangria/util/ValidationSupport.scala b/modules/core/src/test/scala/sangria/util/ValidationSupport.scala index c2d27de1..9587d99b 100644 --- a/modules/core/src/test/scala/sangria/util/ValidationSupport.scala +++ b/modules/core/src/test/scala/sangria/util/ValidationSupport.scala @@ -280,6 +280,21 @@ trait ValidationSupport extends Matchers { "onVariableDefinition", locations = Set(DirectiveLocation.VariableDefinition), shouldInclude = alwaysInclude), + Directive( + "genericDirectiveA", + locations = Set(DirectiveLocation.FragmentDefinition, DirectiveLocation.Field), + shouldInclude = alwaysInclude), + Directive( + "genericDirectiveB", + locations = Set(DirectiveLocation.FragmentDefinition, DirectiveLocation.Field), + shouldInclude = alwaysInclude), + Directive( + "repeatableDirective", + repeatable = true, + arguments = Argument("id", IntType, "Some generic ID") :: Nil, + locations = Set(DirectiveLocation.Object), + shouldInclude = alwaysInclude + ), Directive( "onSchema", locations = Set(DirectiveLocation.Schema), diff --git a/modules/core/src/test/scala/sangria/validation/rules/UniqueDirectivesPerLocationSpec.scala b/modules/core/src/test/scala/sangria/validation/rules/UniqueDirectivesPerLocationSpec.scala index 8b76c662..afe5dd9b 100644 --- a/modules/core/src/test/scala/sangria/validation/rules/UniqueDirectivesPerLocationSpec.scala +++ b/modules/core/src/test/scala/sangria/validation/rules/UniqueDirectivesPerLocationSpec.scala @@ -15,88 +15,113 @@ class UniqueDirectivesPerLocationSpec extends AnyWordSpec with ValidationSupport """) "unique directives in different locations" in expectPasses(""" - fragment Test on Type @directiveA { - field @directiveB + fragment Test on Type @genericDirectiveA { + field @genericDirectiveB } """) "unique directives in same locations" in expectPasses(""" - fragment Test on Type @directiveA @directiveB { - field @directiveA @directiveB + fragment Test on Type @genericDirectiveA @genericDirectiveB { + field @genericDirectiveA @genericDirectiveB } """) "same directives in different locations" in expectPasses(""" - fragment Test on Type @directiveA { - field @directiveA + fragment Test on Type @genericDirectiveA { + field @genericDirectiveA } """) "same directives in similar locations" in expectPasses(""" fragment Test on Type { - field @directive - field @directive + field @genericDirectiveA + field @genericDirectiveA + } + """) + + "repeatable directives in same location" in expectPasses(""" + type Test @repeatableDirective(id: 1) @repeatableDirective(id: 2) { + field: String! + } + """) + + "repeatable directives in similar locations" in expectPasses(""" + type Test @repeatableDirective(id: 1) { + field: String! + } + extend type Test @repeatableDirective(id: 2) { + anotherField: String! + } + """) + + "unknown directives must be ignored" in expectPasses(""" + type Test @unknownDirective @unknownDirective { + field: String! + } + + extend type Test @unknownDirective { + anotherField: String! } """) "duplicate directives in one location" in expectFailsPosList( """ fragment Test on Type { - field @directive @directive + field @genericDirectiveA @genericDirectiveA } """, List( - "The directive 'directive' can only be used once at this location." -> List( + "The directive 'genericDirectiveA' can only be used once at this location." -> List( Pos(3, 17), - Pos(3, 28)) + Pos(3, 36)) ) ) "many duplicate directives in one location" in expectFailsPosList( """ fragment Test on Type { - field @directive @directive @directive + field @genericDirectiveA @genericDirectiveA @genericDirectiveA } """, List( - "The directive 'directive' can only be used once at this location." -> List( + "The directive 'genericDirectiveA' can only be used once at this location." -> List( Pos(3, 17), - Pos(3, 28)), - "The directive 'directive' can only be used once at this location." -> List( + Pos(3, 36)), + "The directive 'genericDirectiveA' can only be used once at this location." -> List( Pos(3, 17), - Pos(3, 39)) + Pos(3, 55)) ) ) "different duplicate directives in one location" in expectFailsPosList( """ fragment Test on Type { - field @directiveA @directiveB @directiveA @directiveB + field @genericDirectiveA @genericDirectiveB @genericDirectiveA @genericDirectiveB } """, List( - "The directive 'directiveA' can only be used once at this location." -> List( + "The directive 'genericDirectiveA' can only be used once at this location." -> List( Pos(3, 17), - Pos(3, 41)), - "The directive 'directiveB' can only be used once at this location." -> List( - Pos(3, 29), - Pos(3, 53)) + Pos(3, 55)), + "The directive 'genericDirectiveB' can only be used once at this location." -> List( + Pos(3, 36), + Pos(3, 74)) ) ) "duplicate directives in many locations" in expectFailsPosList( """ - fragment Test on Type @directive @directive { - field @directive @directive + fragment Test on Type @genericDirectiveA @genericDirectiveA { + field @genericDirectiveA @genericDirectiveA } """, List( - "The directive 'directive' can only be used once at this location." -> List( + "The directive 'genericDirectiveA' can only be used once at this location." -> List( Pos(2, 31), - Pos(2, 42)), - "The directive 'directive' can only be used once at this location." -> List( + Pos(2, 50)), + "The directive 'genericDirectiveA' can only be used once at this location." -> List( Pos(3, 17), - Pos(3, 28)) + Pos(3, 36)) ) ) } diff --git a/modules/parser/src/main/scala/sangria/parser/QueryParser.scala b/modules/parser/src/main/scala/sangria/parser/QueryParser.scala index a881b48c..c8e791ec 100644 --- a/modules/parser/src/main/scala/sangria/parser/QueryParser.scala +++ b/modules/parser/src/main/scala/sangria/parser/QueryParser.scala @@ -471,11 +471,13 @@ private[parser] sealed trait TypeSystemDefinitions { wsNoComment('{') ~ InputValueDefinition.+ ~ Comments ~ wsNoComment('}') ~> (_ -> _) } + private def repeatable = rule(capture(Keyword("repeatable")).? ~> (_.isDefined)) + private[this] def DirectiveDefinition = rule { Description ~ Comments ~ trackPos ~ directive ~ '@' ~ NameStrict ~ (ArgumentsDefinition.? ~> (_.getOrElse( - Vector.empty))) ~ on ~ DirectiveLocations ~> ( - (descr, comment, location, name, args, locations) => - ast.DirectiveDefinition(name, args, locations, descr, comment, location)) + Vector.empty))) ~ repeatable ~ on ~ DirectiveLocations ~> ( + (descr, comment, location, name, args, rep, locations) => + ast.DirectiveDefinition(name, args, locations, descr, rep, comment, location)) } private[this] def DirectiveLocations = rule { diff --git a/modules/parser/src/test/scala/sangria/parser/QueryParserSpec.scala b/modules/parser/src/test/scala/sangria/parser/QueryParserSpec.scala index dd601b61..7d54fcf4 100644 --- a/modules/parser/src/test/scala/sangria/parser/QueryParserSpec.scala +++ b/modules/parser/src/test/scala/sangria/parser/QueryParserSpec.scala @@ -1507,6 +1507,7 @@ class QueryParserSpec extends AnyWordSpec with Matchers with StringMatchers { DirectiveLocation("INLINE_FRAGMENT", Vector.empty, Some(AstLocation(104, 5, 13))) ), None, + false, Vector.empty, Some(AstLocation(9, 2, 9)) )), diff --git a/modules/parser/src/test/scala/sangria/parser/SchemaParserSpec.scala b/modules/parser/src/test/scala/sangria/parser/SchemaParserSpec.scala index f01160d8..3509c41b 100644 --- a/modules/parser/src/test/scala/sangria/parser/SchemaParserSpec.scala +++ b/modules/parser/src/test/scala/sangria/parser/SchemaParserSpec.scala @@ -610,6 +610,7 @@ class SchemaParserSpec extends AnyWordSpec with Matchers with StringMatchers { None, Vector.empty, Some(AstLocation(1545, 83, 1)))), + false, Vector.empty, Some(AstLocation(1557, 84, 1)) ), @@ -633,6 +634,7 @@ class SchemaParserSpec extends AnyWordSpec with Matchers with StringMatchers { DirectiveLocation("INLINE_FRAGMENT", Vector.empty, Some(AstLocation(1703, 89, 6))) ), None, + false, Vector.empty, Some(AstLocation(1633, 86, 1)) )