Skip to content

Commit

Permalink
Add Validation rule for unique directives per location. Closes #179
Browse files Browse the repository at this point in the history
  • Loading branch information
OlegIlyenko committed Nov 19, 2016
1 parent b56b57a commit e32d23f
Show file tree
Hide file tree
Showing 31 changed files with 222 additions and 86 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
## v1.0.0-RC4 (2016-XX-XX)

* Validation rule for unique directives per location (#179).
* Enforces input coercion rules (#177).
* `MiddlewareQueryContext` and `ExecutionResult` now contain information about validation and query reducers' execution time (`validationTiming`, `queryReducerTiming`)
* `Middleware.fieldError` was not called in all possible exceptional situations (#184).
Expand Down
6 changes: 6 additions & 0 deletions src/main/scala/sangria/ast/AstVisitor.scala
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package sangria.ast

import sangria.ast
import sangria.validation.Violation

import scala.util.control.Breaks._

object AstVisitor {
Expand Down Expand Up @@ -281,4 +283,8 @@ object AstVisitor {

object AstVisitorCommand extends Enumeration {
val Skip, Continue, Break = Value

val RightContinue: Either[Vector[Violation], AstVisitorCommand.Value] = Right(Continue)
val RightSkip: Either[Vector[Violation], AstVisitorCommand.Value] = Right(Skip)
val RightBreak: Either[Vector[Violation], AstVisitorCommand.Value] = Right(Break)
}
13 changes: 6 additions & 7 deletions src/main/scala/sangria/ast/QueryAst.scala
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ sealed trait NameValue extends AstNode with WithComments {
def value: Value
}

sealed trait WithDirectives {
sealed trait WithDirectives extends AstNode {
def directives: List[Directive]
}

Expand Down Expand Up @@ -209,15 +209,15 @@ case class FieldDefinition(
arguments: List[InputValueDefinition],
directives: List[Directive] = Nil,
comments: List[Comment] = Nil,
position: Option[Position] = None) extends SchemaAstNode
position: Option[Position] = None) extends SchemaAstNode with WithDirectives

case class InputValueDefinition(
name: String,
valueType: Type,
defaultValue: Option[Value],
directives: List[Directive] = Nil,
comments: List[Comment] = Nil,
position: Option[Position] = None) extends SchemaAstNode
position: Option[Position] = None) extends SchemaAstNode with WithDirectives

case class ObjectTypeDefinition(
name: String,
Expand Down Expand Up @@ -255,7 +255,7 @@ case class EnumValueDefinition(
name: String,
directives: List[Directive] = Nil,
comments: List[Comment] = Nil,
position: Option[Position] = None) extends SchemaAstNode
position: Option[Position] = None) extends SchemaAstNode with WithDirectives

case class InputObjectTypeDefinition(
name: String,
Expand Down Expand Up @@ -287,7 +287,7 @@ case class SchemaDefinition(
directives: List[Directive] = Nil,
comments: List[Comment] = Nil,
trailingComments: List[Comment] = Nil,
position: Option[Position] = None) extends TypeSystemDefinition with WithTrailingComments
position: Option[Position] = None) extends TypeSystemDefinition with WithTrailingComments with WithDirectives

case class OperationTypeDefinition(
operation: OperationType,
Expand All @@ -305,9 +305,8 @@ sealed trait AstNode {

sealed trait SchemaAstNode extends AstNode with WithComments
sealed trait TypeSystemDefinition extends SchemaAstNode with Definition
sealed trait TypeDefinition extends TypeSystemDefinition {
sealed trait TypeDefinition extends TypeSystemDefinition with WithDirectives {
def name: String
def directives: List[Directive]
}

object AstNode {
Expand Down
3 changes: 2 additions & 1 deletion src/main/scala/sangria/validation/QueryValidator.scala
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ object QueryValidator {
new ProvidedNonNullArguments,
new ScalarLeafs,
new UniqueArgumentNames,
new UniqueDirectivesPerLocation,
new UniqueFragmentNames,
new UniqueInputFieldNames,
new UniqueOperationNames,
Expand Down Expand Up @@ -91,7 +92,7 @@ class RuleBasedQueryValidator(rules: List[ValidationRule]) extends QueryValidato
visitRes match {
case Left(violation)
ctx.addViolations(violation)
case Right(Skip)
case AstVisitorCommand.RightSkip
ctx.skips(visitor) = node
case Right(Break)
ctx.ignoredVisitors += visitor
Expand Down
4 changes: 4 additions & 0 deletions src/main/scala/sangria/validation/Violation.scala
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,10 @@ case class RequiredSubselectionViolation(fieldName: String, typeName: String, so
lazy val simpleErrorMessage = s"Field '$fieldName' of type '$typeName' must have a sub selection."
}

case class DuplicateDirectiveViolation(directiveName: String, sourceMapper: Option[SourceMapper], positions: List[Position]) extends AstNodeViolation {
lazy val simpleErrorMessage = s"The directive '$directiveName' can only be used once at this location."
}

case class TypeIncompatibleAnonSpreadViolation(parentTypeName: String, fragTypeName: String, sourceMapper: Option[SourceMapper], positions: List[Position]) extends AstNodeViolation {
lazy val simpleErrorMessage = s"Fragment cannot be spread here as objects of type '$parentTypeName' can never be of type '$fragTypeName'."
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
package sangria.validation.rules

import sangria.renderer.{SchemaRenderer, QueryRenderer}
import sangria.renderer.{QueryRenderer, SchemaRenderer}
import sangria.validation.ValidationContext.isValidLiteralValue
import sangria.validation.{BadValueViolation, ValidationContext, ValidationRule}
import sangria.ast
import sangria.ast.AstVisitorCommand._
import sangria.ast.AstVisitorCommand

/**
* Argument values of correct type
Expand All @@ -28,8 +28,8 @@ class ArgumentsOfCorrectType extends ValidationRule {
ctx.sourceMapper,
value.position.toList)))
else
Right(Continue)
} getOrElse Right(Continue)
AstVisitorCommand.RightContinue
} getOrElse AstVisitorCommand.RightContinue
}
}
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package sangria.validation.rules

import sangria.ast
import sangria.ast.AstVisitorCommand._
import sangria.ast.AstVisitorCommand
import sangria.renderer.{QueryRenderer, SchemaRenderer}
import sangria.schema.OptionInputType
import sangria.validation.ValidationContext._
Expand Down Expand Up @@ -38,9 +38,9 @@ class DefaultValuesOfCorrectType extends ValidationRule {
ctx.sourceMapper,
defaultValue.position.toList)))
else
Right(Continue)
AstVisitorCommand.RightContinue
case _
Right(Continue)
AstVisitorCommand.RightContinue
}
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
package sangria.validation.rules

import sangria.ast
import sangria.ast.AstVisitorCommand._
import sangria.renderer.{QueryRenderer, SchemaRenderer}
import sangria.ast.AstVisitorCommand
import sangria.renderer.SchemaRenderer
import sangria.schema._
import sangria.util.StringUtil
import sangria.validation.ValidationContext._
import sangria.validation._

/**
Expand Down Expand Up @@ -33,7 +32,7 @@ class FieldsOnCorrectType extends ValidationRule {
ctx.sourceMapper,
pos.toList)))
case _
Right(Continue)
AstVisitorCommand.RightContinue
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package sangria.validation.rules

import sangria.ast
import sangria.ast.AstVisitorCommand._
import sangria.ast.AstVisitorCommand
import sangria.renderer.SchemaRenderer
import sangria.schema.CompositeType
import sangria.validation.{InlineFragmentOnNonCompositeErrorViolation, FragmentOnNonCompositeErrorViolation, ValidationContext, ValidationRule}
Expand All @@ -21,14 +21,14 @@ class FragmentsOnCompositeType extends ValidationRule {
case Some(tpe) if !tpe.isInstanceOf[CompositeType[_]]
Left(Vector(InlineFragmentOnNonCompositeErrorViolation(cond.name, ctx.sourceMapper, cond.position.toList)))
case _
Right(Continue)
AstVisitorCommand.RightContinue
}
case ast.FragmentDefinition(name, cond, _, _, _, _, pos)
ctx.typeInfo.tpe match {
case Some(tpe) if !tpe.isInstanceOf[CompositeType[_]]
Left(Vector(FragmentOnNonCompositeErrorViolation(name, cond.name, ctx.sourceMapper, cond.position.toList)))
case _
Right(Continue)
AstVisitorCommand.RightContinue
}
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package sangria.validation.rules

import sangria.ast
import sangria.ast.AstVisitorCommand._
import sangria.ast.AstVisitorCommand
import sangria.renderer.{QueryRenderer, SchemaRenderer}
import sangria.util.StringUtil
import sangria.validation._
Expand All @@ -28,7 +28,7 @@ class KnownArgumentNames extends ValidationRule {
ctx.sourceMapper,
pos.toList)))
case _
Right(Continue)
AstVisitorCommand.RightContinue
}

case _: ast.Directive
Expand All @@ -41,11 +41,11 @@ class KnownArgumentNames extends ValidationRule {
ctx.sourceMapper,
pos.toList)))
case _
Right(Continue)
AstVisitorCommand.RightContinue
}

case _
Right(Continue)
AstVisitorCommand.RightContinue
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/main/scala/sangria/validation/rules/KnownDirectives.scala
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import sangria.ast.{AstNode, OperationType}
import sangria.schema.DirectiveLocation

import sangria.ast
import sangria.ast.AstVisitorCommand._
import sangria.ast.AstVisitorCommand
import sangria.validation._

/**
Expand All @@ -28,7 +28,7 @@ class KnownDirectives extends ValidationRule {
Left(Vector(MisplacedDirectiveViolation(name, None, ctx.sourceMapper, pos.toList)))
case Some((correctLocation, hint)) if !dir.locations.contains(correctLocation)
Left(Vector(MisplacedDirectiveViolation(name, Some(hint), ctx.sourceMapper, pos.toList)))
case _ Right(Continue)
case _ AstVisitorCommand.RightContinue
}
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package sangria.validation.rules

import sangria.ast
import sangria.ast.AstVisitorCommand._
import sangria.ast.AstVisitorCommand
import sangria.validation._

import scala.language.postfixOps
Expand All @@ -18,7 +18,7 @@ class KnownFragmentNames extends ValidationRule {
case ast.FragmentSpread(name, _, _, pos)
ctx.fragments.get(name) match {
case None Left(Vector(UnknownFragmentViolation(name, ctx.sourceMapper, pos.toList)))
case _ Right(Continue)
case _ AstVisitorCommand.RightContinue
}
}
}
Expand Down
6 changes: 3 additions & 3 deletions src/main/scala/sangria/validation/rules/KnownTypeNames.scala
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package sangria.validation.rules

import sangria.ast
import sangria.ast.AstVisitorCommand._
import sangria.ast.AstVisitorCommand
import sangria.util.StringUtil
import sangria.validation._

Expand All @@ -20,7 +20,7 @@ class KnownTypeNames extends ValidationRule {
// TODO: when validating IDL, re-enable these. Experimental version does not
// add unreferenced types, resulting in false-positive errors. Squelched
// errors for now.
Right(Skip)
AstVisitorCommand.RightSkip
case ast.NamedType(name, pos)
if (!ctx.schema.allTypes.contains(name))
Left(Vector(UnknownTypeViolation(
Expand All @@ -29,7 +29,7 @@ class KnownTypeNames extends ValidationRule {
ctx.sourceMapper,
pos.toList)))
else
Right(Continue)
AstVisitorCommand.RightContinue
}
}
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package sangria.validation.rules

import sangria.ast
import sangria.ast.AstVisitorCommand._
import sangria.ast.AstVisitorCommand
import sangria.validation._

import scala.language.postfixOps
Expand All @@ -19,12 +19,12 @@ class LoneAnonymousOperation extends ValidationRule {
override val onEnter: ValidationVisit = {
case ast.Document(definitions, _, _, _)
operationCount = definitions.count(_.isInstanceOf[ast.OperationDefinition])
Right(Continue)
AstVisitorCommand.RightContinue
case op: ast.OperationDefinition
if (op.name.isEmpty && operationCount > 1)
Left(Vector(AnonOperationNotAloneViolation(ctx.sourceMapper, op.position.toList)))
else
Right(Continue)
AstVisitorCommand.RightContinue
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -57,15 +57,15 @@ class NoFragmentCycles extends ValidationRule {

override val onEnter: ValidationVisit = {
case fragmentDef @ ast.FragmentDefinition(fragmentName, _, _, _, _, _, _)
if (visitedFrags.contains(fragmentName)) Right(Skip)
if (visitedFrags.contains(fragmentName)) AstVisitorCommand.RightSkip
else {
val errors = detectCycleRecursive(fragmentDef)

if (errors.nonEmpty) Left(errors)
else Right(Continue)
else AstVisitorCommand.RightContinue
}

case _: ast.OperationDefinition Right(Skip)
case _: ast.OperationDefinition AstVisitorCommand.RightSkip
}
}
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package sangria.validation.rules

import sangria.ast
import sangria.ast.AstVisitorCommand._
import sangria.ast.AstVisitorCommand
import sangria.validation._
import scala.collection.mutable.{Set MutableSet}

Expand All @@ -20,11 +20,11 @@ class NoUndefinedVariables extends ValidationRule {
override val onEnter: ValidationVisit = {
case _: ast.OperationDefinition
variableNameDefined.clear()
Right(Continue)
AstVisitorCommand.RightContinue

case varDef: ast.VariableDefinition
variableNameDefined += varDef.name
Right(Continue)
AstVisitorCommand.RightContinue
}

override def onLeave: ValidationVisit = {
Expand All @@ -40,7 +40,7 @@ class NoUndefinedVariables extends ValidationRule {
}
}

if (errors.nonEmpty) Left(errors.distinct) else Right(Continue)
if (errors.nonEmpty) Left(errors.distinct) else AstVisitorCommand.RightContinue
}
}
}
Loading

0 comments on commit e32d23f

Please sign in to comment.