adding oneOf is a breaking change

sangria-graphql · Jul 22, 2024 · 49b8b01 · 49b8b01
1 parent 50371d8
commit 49b8b01
Show file tree

Hide file tree

Showing 3 changed files with 106 additions and 20 deletions.
diff --git a/build.sbt b/build.sbt
@@ -199,7 +199,17 @@ lazy val core = project
       ProblemFilters.exclude[DirectMissingMethodProblem](
         "sangria.validation.RuleBasedQueryValidator.validateInputDocument"),
       ProblemFilters.exclude[DirectMissingMethodProblem](
-        "sangria.validation.RuleBasedQueryValidator.validateInputDocument")
+        "sangria.validation.RuleBasedQueryValidator.validateInputDocument"),
+      ProblemFilters.exclude[DirectMissingMethodProblem](
+        "sangria.schema.SchemaChange#AbstractAstDirectiveAdded.this"),
+      ProblemFilters.exclude[DirectMissingMethodProblem](
+        "sangria.schema.SchemaChange#InputObjectTypeAstDirectiveAdded.copy"),
+      ProblemFilters.exclude[DirectMissingMethodProblem](
+        "sangria.schema.SchemaChange#InputObjectTypeAstDirectiveAdded.this"),
+      ProblemFilters.exclude[MissingTypesProblem](
+        "sangria.schema.SchemaChange$InputObjectTypeAstDirectiveAdded$"),
+      ProblemFilters.exclude[DirectMissingMethodProblem](
+        "sangria.schema.SchemaChange#InputObjectTypeAstDirectiveAdded.apply")
     ),
     Test / testOptions += Tests.Argument(TestFrameworks.ScalaTest, "-oF"),
     libraryDependencies ++= Seq(

diff --git a/modules/core/src/main/scala/sangria/schema/SchemaComparator.scala b/modules/core/src/main/scala/sangria/schema/SchemaComparator.scala
@@ -280,7 +280,11 @@ object SchemaComparator {
     val directiveChanges = findInAstDirs(
       oldType.astDirectives,
       newType.astDirectives,
-      added = SchemaChange.InputObjectTypeAstDirectiveAdded(newType, _),
+      added = d =>
+        SchemaChange.InputObjectTypeAstDirectiveAdded(
+          newType,
+          d,
+          breaking = d.name == OneOfDirective.name),
       removed = SchemaChange.InputObjectTypeAstDirectiveRemoved(newType, _)
     )
 
@@ -900,9 +904,9 @@ object SchemaChange {
 
   abstract class AbstractAstDirectiveAdded(
       val description: String,
-      val location: DirectiveLocation.Value)
+      val location: DirectiveLocation.Value,
+      val breakingChange: Boolean)
       extends AstDirectiveAdded {
-    val breakingChange = false
     val dangerousChange = false
   }
 
@@ -920,7 +924,9 @@ object SchemaChange {
       directive: ast.Directive)
       extends AbstractAstDirectiveAdded(
         s"Directive `${QueryRenderer.renderCompact(directive)}` added on a field `${tpe.name}.${field.name}`",
-        DirectiveLocation.FieldDefinition)
+        DirectiveLocation.FieldDefinition,
+        breakingChange = false
+      )
 
   case class FieldAstDirectiveRemoved(
       tpe: ObjectLikeType[_, _],
@@ -937,7 +943,9 @@ object SchemaChange {
       extends AbstractAstDirectiveAdded(
         s"Directive `${QueryRenderer.renderCompact(
             directive)}` added on an enum value `${tpe.name}.${value.name}`",
-        DirectiveLocation.EnumValue)
+        DirectiveLocation.EnumValue,
+        breakingChange = false
+      )
 
   case class EnumValueAstDirectiveRemoved(
       tpe: EnumType[_],
@@ -955,7 +963,9 @@ object SchemaChange {
       extends AbstractAstDirectiveAdded(
         s"Directive `${QueryRenderer.renderCompact(
             directive)}` added on an input field `${tpe.name}.${field.name}`",
-        DirectiveLocation.InputFieldDefinition)
+        DirectiveLocation.InputFieldDefinition,
+        breakingChange = false
+      )
 
   case class InputFieldAstDirectiveRemoved(
       tpe: InputObjectType[_],
@@ -974,7 +984,8 @@ object SchemaChange {
       extends AbstractAstDirectiveAdded(
         s"Directive `${QueryRenderer.renderCompact(
             directive)}` added on a directive argument `${dir.name}.${argument.name}`",
-        DirectiveLocation.ArgumentDefinition
+        DirectiveLocation.ArgumentDefinition,
+        breakingChange = false
       )
 
   case class DirectiveArgumentAstDirectiveRemoved(
@@ -995,7 +1006,8 @@ object SchemaChange {
       extends AbstractAstDirectiveAdded(
         s"Directive `${QueryRenderer.renderCompact(
             directive)}` added on a field argument `${tpe.name}.${field.name}[${argument.name}]`",
-        DirectiveLocation.ArgumentDefinition
+        DirectiveLocation.ArgumentDefinition,
+        breakingChange = false
       )
 
   case class FieldArgumentAstDirectiveRemoved(
@@ -1012,7 +1024,8 @@ object SchemaChange {
   case class ObjectTypeAstDirectiveAdded(tpe: ObjectType[_, _], directive: ast.Directive)
       extends AbstractAstDirectiveAdded(
         s"Directive `${QueryRenderer.renderCompact(directive)}` added on an object type `${tpe.name}`",
-        DirectiveLocation.Object)
+        DirectiveLocation.Object,
+        breakingChange = false)
 
   case class ObjectTypeAstDirectiveRemoved(tpe: ObjectType[_, _], directive: ast.Directive)
       extends AbstractAstDirectiveRemoved(
@@ -1022,7 +1035,8 @@ object SchemaChange {
   case class InterfaceTypeAstDirectiveAdded(tpe: InterfaceType[_, _], directive: ast.Directive)
       extends AbstractAstDirectiveAdded(
         s"Directive `${QueryRenderer.renderCompact(directive)}` added on an interface type `${tpe.name}`",
-        DirectiveLocation.Interface)
+        DirectiveLocation.Interface,
+        breakingChange = false)
 
   case class InterfaceTypeAstDirectiveRemoved(tpe: InterfaceType[_, _], directive: ast.Directive)
       extends AbstractAstDirectiveRemoved(
@@ -1032,7 +1046,8 @@ object SchemaChange {
   case class UnionTypeAstDirectiveAdded(tpe: UnionType[_], directive: ast.Directive)
       extends AbstractAstDirectiveAdded(
         s"Directive `${QueryRenderer.renderCompact(directive)}` added on a union type `${tpe.name}`",
-        DirectiveLocation.Union)
+        DirectiveLocation.Union,
+        breakingChange = false)
 
   case class UnionTypeAstDirectiveRemoved(tpe: UnionType[_], directive: ast.Directive)
       extends AbstractAstDirectiveRemoved(
@@ -1042,7 +1057,8 @@ object SchemaChange {
   case class EnumTypeAstDirectiveAdded(tpe: EnumType[_], directive: ast.Directive)
       extends AbstractAstDirectiveAdded(
         s"Directive `${QueryRenderer.renderCompact(directive)}` added on an enum type `${tpe.name}`",
-        DirectiveLocation.Enum)
+        DirectiveLocation.Enum,
+        breakingChange = false)
 
   case class EnumTypeAstDirectiveRemoved(tpe: EnumType[_], directive: ast.Directive)
       extends AbstractAstDirectiveRemoved(
@@ -1052,18 +1068,14 @@ object SchemaChange {
   case class ScalarTypeAstDirectiveAdded(tpe: ScalarType[_], directive: ast.Directive)
       extends AbstractAstDirectiveAdded(
         s"Directive `${QueryRenderer.renderCompact(directive)}` added on a scalar type `${tpe.name}`",
-        DirectiveLocation.Scalar)
+        DirectiveLocation.Scalar,
+        breakingChange = false)
 
   case class ScalarTypeAstDirectiveRemoved(tpe: ScalarType[_], directive: ast.Directive)
       extends AbstractAstDirectiveRemoved(
         s"Directive `${QueryRenderer.renderCompact(directive)}` removed from a scalar type `${tpe.name}`",
         DirectiveLocation.Scalar)
 
-  case class InputObjectTypeAstDirectiveAdded(tpe: InputObjectType[_], directive: ast.Directive)
-      extends AbstractAstDirectiveAdded(
-        s"Directive `${QueryRenderer.renderCompact(directive)}` added on an input type `${tpe.name}`",
-        DirectiveLocation.InputObject)
-
   case class InputObjectTypeAstDirectiveRemoved(tpe: InputObjectType[_], directive: ast.Directive)
       extends AbstractAstDirectiveRemoved(
         s"Directive `${QueryRenderer.renderCompact(directive)}` removed from an input type `${tpe.name}`",
@@ -1072,7 +1084,8 @@ object SchemaChange {
   case class SchemaAstDirectiveAdded(schema: Schema[_, _], directive: ast.Directive)
       extends AbstractAstDirectiveAdded(
         s"Directive `${QueryRenderer.renderCompact(directive)}` added on a schema",
-        DirectiveLocation.Schema)
+        DirectiveLocation.Schema,
+        breakingChange = false)
 
   case class SchemaAstDirectiveRemoved(schema: Schema[_, _], directive: ast.Directive)
       extends AbstractAstDirectiveRemoved(
@@ -1081,6 +1094,15 @@ object SchemaChange {
 
   // May be a breaking change
 
+  case class InputObjectTypeAstDirectiveAdded(
+      tpe: InputObjectType[_],
+      directive: ast.Directive,
+      breaking: Boolean)
+      extends AbstractAstDirectiveAdded(
+        s"Directive `${QueryRenderer.renderCompact(directive)}` added on an input type `${tpe.name}`",
+        DirectiveLocation.InputObject,
+        breakingChange = breaking)
+
   case class InputFieldAdded(tpe: InputObjectType[_], field: InputField[_], breaking: Boolean)
       extends AbstractChange(
         s"Input field `${field.name}` was added to `${tpe.name}` type",

diff --git a/modules/core/src/test/scala/sangria/schema/SchemaComparatorSpec.scala b/modules/core/src/test/scala/sangria/schema/SchemaComparatorSpec.scala
@@ -668,6 +668,60 @@ class SchemaComparatorSpec extends AnyWordSpec with Matchers {
       nonBreakingChange[ScalarTypeAstDirectiveRemoved](
         "Directive `@bar(ids:[1,2])` removed from a scalar type `Foo5`")
     )
+
+    "detect removal of @oneOf" in checkChangesWithoutQueryType(
+      gql"""
+        input UserBy @oneOf {
+          id: ID
+          email: String
+          username: String
+          registrationNumber: Int
+        }
+        type Query {
+          user(by: UserBy!): String
+        }
+      """,
+      gql"""
+        input UserBy {
+          id: ID
+          email: String
+          username: String
+          registrationNumber: Int
+        }
+        type Query {
+          user(by: UserBy!): String
+        }
+      """,
+      nonBreakingChange[InputObjectTypeAstDirectiveRemoved](
+        "Directive `@oneOf` removed from an input type `UserBy`")
+    )
+
+    "detect add of @oneOf" in checkChangesWithoutQueryType(
+      gql"""
+        input UserBy {
+          id: ID
+          email: String
+          username: String
+          registrationNumber: Int
+        }
+        type Query {
+          user(by: UserBy!): String
+        }
+      """,
+      gql"""
+        input UserBy @oneOf {
+          id: ID
+          email: String
+          username: String
+          registrationNumber: Int
+        }
+        type Query {
+          user(by: UserBy!): String
+        }
+      """,
+      breakingChange[InputObjectTypeAstDirectiveAdded](
+        "Directive `@oneOf` added on an input type `UserBy`")
+    )
   }
 
   private[this] def breakingChange[T: ClassTag](description: String) =