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.

Sign up for GitHub

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

Fix TODOs in SchemaValidator #2415

Merged
merged 1 commit into from
Sep 23, 2024
Merged

Fix TODOs in SchemaValidator #2415

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 2 additions & 2 deletions core/src/main/scala/caliban/GraphQL.scala
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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