Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for repeatable directives #899

Merged
merged 1 commit into from
Aug 4, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 32 additions & 0 deletions build.sbt
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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/"))
Expand Down Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions modules/ast/src/main/scala/sangria/ast/QueryAst.scala
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
Expand Down
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Loading