Skip to content

Commit

Permalink
Fix TODOs in SchemaValidator (#2415)
Browse files Browse the repository at this point in the history
  • Loading branch information
@kyri-petrou
kyri-petrou authored Sep 23, 2024
1 parent aaebd69 commit 3b51d27
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 30 deletions.
4 changes: 2 additions & 2 deletions core/src/main/scala/caliban/GraphQL.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import caliban.parsing.{ Parser, SourceMapper, VariablesCoercer }
import caliban.rendering.DocumentRenderer
import caliban.schema._
import caliban.transformers.Transformer
import caliban.validation.Validator
import caliban.validation.{ SchemaValidator, Validator }
import caliban.wrappers.Wrapper
import caliban.wrappers.Wrapper._
import zio.stacktracer.TracingImplicits.disableAutoTrace
Expand All @@ -32,7 +32,7 @@ trait GraphQL[-R] { self =>
protected val transformer: Transformer[R]

private[caliban] def validateRootSchema: Either[ValidationError, RootSchema[R]] =
Validator.validateSchema(schemaBuilder)
SchemaValidator.validateSchema(schemaBuilder)

/**
* Returns a string that renders the API types into the GraphQL SDL.
Expand Down
41 changes: 18 additions & 23 deletions core/src/main/scala/caliban/validation/SchemaValidator.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,7 @@ import caliban.schema.{ RootSchema, RootSchemaBuilder, Types }
import caliban.validation.Utils.isObjectType
import caliban.validation.ValidationOps._

/*
* TODO In the next major version:
* 1. Make `SchemaValidator` an object and remove inheritance from Validator
* 2. Make all methods private except `validateSchema` and `validateType`
*/
private[caliban] trait SchemaValidator {
private[caliban] object SchemaValidator {

/**
* Verifies that the given schema is valid. Fails with a [[caliban.CalibanError.ValidationError]] otherwise.
Expand Down Expand Up @@ -43,7 +38,7 @@ private[caliban] trait SchemaValidator {
case _ => unit
})

private[caliban] def validateClashingTypes(types: List[__Type]): Either[ValidationError, Unit] = {
private def validateClashingTypes(types: List[__Type]): Either[ValidationError, Unit] = {
val check = types.groupBy(_.name).collectFirst { case (Some(name), v) if v.size > 1 => (name, v) }
check match {
case None => unit
Expand Down Expand Up @@ -108,7 +103,7 @@ private[caliban] trait SchemaValidator {
}
}

private[caliban] def validateEnum(t: __Type): Either[ValidationError, Unit] =
private def validateEnum(t: __Type): Either[ValidationError, Unit] =
t.allEnumValues match {
case _ :: _ => unit
case Nil =>
Expand All @@ -118,7 +113,7 @@ private[caliban] trait SchemaValidator {
)
}

private[caliban] def validateUnion(t: __Type): Either[ValidationError, Unit] =
private def validateUnion(t: __Type): Either[ValidationError, Unit] =
t.possibleTypes match {
case None | Some(Nil) =>
failValidation(
Expand All @@ -134,7 +129,7 @@ private[caliban] trait SchemaValidator {
case _ => unit
}

private[caliban] def validateInputObject(t: __Type): Either[ValidationError, Unit] = {
private def validateInputObject(t: __Type): Either[ValidationError, Unit] = {
lazy val inputObjectContext = s"""${if (t._isOneOfInput) "OneOf " else ""}InputObject '${t.name.getOrElse("")}'"""

def noDuplicateInputValueName(
Expand Down Expand Up @@ -184,7 +179,7 @@ private[caliban] trait SchemaValidator {
}
}

private[caliban] def validateInputValue(
private def validateInputValue(
inputValue: __InputValue,
errorContext: => String
): Either[ValidationError, Unit] = {
Expand All @@ -196,7 +191,7 @@ private[caliban] trait SchemaValidator {
} yield ()
}

private[caliban] def validateInterface(t: __Type): Either[ValidationError, Unit] = {
private def validateInterface(t: __Type): Either[ValidationError, Unit] = {
lazy val interfaceContext = s"Interface '${t.name.getOrElse("")}'"

t.allFields match {
Expand All @@ -209,7 +204,7 @@ private[caliban] trait SchemaValidator {
}
}

def validateObject(obj: __Type): Either[ValidationError, Unit] = {
private def validateObject(obj: __Type): Either[ValidationError, Unit] = {
lazy val objectContext = s"Object '${obj.name.getOrElse("")}'"

def validateInterfaceFields(obj: __Type) = {
Expand Down Expand Up @@ -315,7 +310,7 @@ private[caliban] trait SchemaValidator {
private def isListField(field: __Field) =
field._type.kind == __TypeKind.LIST

private[caliban] def onlyInputType(`type`: __Type, errorContext: => String): Either[ValidationError, Unit] = {
private def onlyInputType(`type`: __Type, errorContext: => String): Either[ValidationError, Unit] = {
// https://spec.graphql.org/June2018/#IsInputType()
def isInputType(t: __Type): Either[__Type, Unit] = {
import __TypeKind._
Expand All @@ -336,7 +331,7 @@ private[caliban] trait SchemaValidator {
}
}

private[caliban] def validateFields(fields: List[__Field], context: => String): Either[ValidationError, Unit] =
private def validateFields(fields: List[__Field], context: => String): Either[ValidationError, Unit] =
noDuplicateFieldName(fields, context) *>
validateAllDiscard(fields) { field =>
lazy val fieldContext = s"Field '${field.name}' of $context"
Expand All @@ -347,14 +342,14 @@ private[caliban] trait SchemaValidator {
} yield ()
}

private[caliban] def noDuplicateFieldName(fields: List[__Field], errorContext: => String) = {
private def noDuplicateFieldName(fields: List[__Field], errorContext: => String) = {
val messageBuilder = (f: __Field) => s"$errorContext has repeated fields: ${f.name}"
def explanatory =
"The field must have a unique name within that Interface type; no two fields may share the same name"
noDuplicateName[__Field](fields, _.name, messageBuilder, explanatory)
}

private[caliban] def onlyOutputType(`type`: __Type, errorContext: => String): Either[ValidationError, Unit] = {
private def onlyOutputType(`type`: __Type, errorContext: => String): Either[ValidationError, Unit] = {
// https://spec.graphql.org/June2018/#IsOutputType()
def isOutputType(t: __Type): Either[__Type, Unit] = {
import __TypeKind._
Expand All @@ -375,7 +370,7 @@ private[caliban] trait SchemaValidator {
}
}

private[caliban] def noDuplicateName[T](
private def noDuplicateName[T](
listOfNamed: List[T],
nameExtractor: T => String,
messageBuilder: T => String,
Expand All @@ -388,7 +383,7 @@ private[caliban] trait SchemaValidator {
failValidation(messageBuilder(duplicate), explanatoryText)
)

private[caliban] def checkName(name: String, fieldContext: => String): Either[ValidationError, Unit] =
private def checkName(name: String, fieldContext: => String): Either[ValidationError, Unit] =
Parser
.parseName(name)
.left
Expand All @@ -399,7 +394,7 @@ private[caliban] trait SchemaValidator {
)
) *> doesNotStartWithUnderscore(name, fieldContext)

private[caliban] def doesNotStartWithUnderscore(
private def doesNotStartWithUnderscore(
name: String,
errorContext: => String
): Either[ValidationError, Unit] =
Expand All @@ -408,7 +403,7 @@ private[caliban] trait SchemaValidator {
"""Names can not begin with the characters "__" (two underscores)"""
)

private[caliban] def validateRootQuery[R](
private def validateRootQuery[R](
schema: RootSchemaBuilder[R]
): Either[ValidationError, RootSchema[R]] =
schema.query match {
Expand All @@ -427,7 +422,7 @@ private[caliban] trait SchemaValidator {
)
}

private[caliban] def validateRootMutation[R](schema: RootSchemaBuilder[R]): Either[ValidationError, Unit] =
private def validateRootMutation[R](schema: RootSchemaBuilder[R]): Either[ValidationError, Unit] =
schema.mutation match {
case Some(mutation) if mutation.opType.kind != __TypeKind.OBJECT =>
failValidation(
Expand All @@ -437,7 +432,7 @@ private[caliban] trait SchemaValidator {
case _ => unit
}

private[caliban] def validateRootSubscription[R](schema: RootSchemaBuilder[R]): Either[ValidationError, Unit] =
private def validateRootSubscription[R](schema: RootSchemaBuilder[R]): Either[ValidationError, Unit] =
schema.subscription match {
case Some(subscription) if subscription.opType.kind != __TypeKind.OBJECT =>
failValidation(
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/scala/caliban/validation/Validator.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import scala.collection.compat._
import scala.collection.mutable
import scala.collection.mutable.ListBuffer

object Validator extends SchemaValidator {
object Validator {
import ValidationOps._
import caliban.syntax._

Expand Down
8 changes: 4 additions & 4 deletions core/src/test/scala/caliban/validation/ValidationSchemaSpec.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,15 @@ object ValidationSchemaSpec extends ZIOSpecDefault {

def checkTypeError(t: __Type, expectedMessage: String): IO[ValidationError, TestResult] =
ZIO
.fromEither(Validator.validateType(t))
.fromEither(SchemaValidator.validateType(t))
.exit
.map(assert(_)(fails(hasField("msg", _.msg, equalTo(expectedMessage)))))

override def spec =
suite("ValidationSchemaSpec")(
suite("Enum")(
test("non-empty enum is ok") {
val res = Validator.validateType(
val res = SchemaValidator.validateType(
Types.makeEnum(
name = Some("nonEmptyEnum"),
description = None,
Expand All @@ -54,7 +54,7 @@ object ValidationSchemaSpec extends ZIOSpecDefault {
),
suite("Union")(
test("union containing object types is ok") {
val res = Validator.validateType(
val res = SchemaValidator.validateType(
Types.makeUnion(
name = Some("GoodUnion"),
description = None,
Expand Down Expand Up @@ -298,7 +298,7 @@ object ValidationSchemaSpec extends ZIOSpecDefault {
graphQL(resolverFieldWithArg).interpreter.exit.map(assert(_)(succeeds(anything)))
},
test("fields with additional nullable args are valid") {
assert(Validator.validateObject(nullableExtraArgsObject))(isRight)
assert(SchemaValidator.validateType(nullableExtraArgsObject))(isRight)
},
test("fields with additional non-nullable args are invalid") {
checkTypeError(
Expand Down

0 comments on commit 3b51d27

Please sign in to comment.