diff --git a/src/Utils/FindBreakingChanges.php b/src/Utils/FindBreakingChanges.php index c747a71..63acbef 100644 --- a/src/Utils/FindBreakingChanges.php +++ b/src/Utils/FindBreakingChanges.php @@ -34,20 +34,8 @@ class FindBreakingChanges const DANGEROUS_CHANGE_ARG_DEFAULT_VALUE = 'ARG_DEFAULT_VALUE_CHANGE'; const DANGEROUS_CHANGE_VALUE_ADDED_TO_ENUM = 'VALUE_ADDED_TO_ENUM'; const DANGEROUS_CHANGE_TYPE_ADDED_TO_UNION = 'TYPE_ADDED_TO_UNION'; - - /** - * Given two schemas, returns an Array containing descriptions of all the types - * of potentially dangerous changes covered by the other functions down below. - * - * @return array - */ - public static function findDangerousChanges(Schema $oldSchema, Schema $newSchema) - { - return array_merge(self::findArgChanges($oldSchema, $newSchema)['dangerousChanges'], - self::findValuesAddedToEnums($oldSchema, $newSchema), - self::findTypesAddedToUnions($oldSchema, $newSchema) - ); - } + const DANGEROUS_CHANGE_NULLABLE_INPUT_FIELD_ADDED = 'NULLABLE_INPUT_FIELD_ADDED'; + const DANGEROUS_CHANGE_NULLABLE_ARG_ADDED = 'NULLABLE_ARG_ADDED'; /** * Given two schemas, returns an Array containing descriptions of all the types @@ -60,7 +48,8 @@ class FindBreakingChanges return array_merge( self::findRemovedTypes($oldSchema, $newSchema), self::findTypesThatChangedKind($oldSchema, $newSchema), - self::findFieldsThatChangedType($oldSchema, $newSchema), + self::findFieldsThatChangedTypeOnObjectOrInterfaceTypes($oldSchema, $newSchema), + self::findFieldsThatChangedTypeOnInputObjectTypes($oldSchema, $newSchema)['breakingChanges'], self::findTypesRemovedFromUnions($oldSchema, $newSchema), self::findValuesRemovedFromEnums($oldSchema, $newSchema), self::findArgChanges($oldSchema, $newSchema)['breakingChanges'], @@ -68,6 +57,22 @@ class FindBreakingChanges ); } + /** + * Given two schemas, returns an Array containing descriptions of all the types + * of potentially dangerous changes covered by the other functions down below. + * + * @return array + */ + public static function findDangerousChanges(Schema $oldSchema, Schema $newSchema) + { + return array_merge( + self::findArgChanges($oldSchema, $newSchema)['dangerousChanges'], + self::findValuesAddedToEnums($oldSchema, $newSchema), + self::findTypesAddedToUnions($oldSchema, $newSchema), + self::findFieldsThatChangedTypeOnInputObjectTypes($oldSchema, $newSchema)['dangerousChanges'] + ); + } + /** * Given two schemas, returns an Array containing descriptions of any breaking * changes in the newSchema related to removing an entire type. @@ -191,17 +196,24 @@ class FindBreakingChanges $oldArgs = $oldTypeFields[$fieldName]->args; $oldArgDef = Utils::find( $oldArgs, function ($arg) use ($newArgDef) { - return $arg->name === $newArgDef->name; - } + return $arg->name === $newArgDef->name; + } ); - if (!$oldArgDef && $newArgDef->getType() instanceof NonNull) { + if (!$oldArgDef) { $newTypeName = $newTypeDefinition->name; $newArgName = $newArgDef->name; - $breakingChanges[] = [ - 'type' => self::BREAKING_CHANGE_NON_NULL_ARG_ADDED, - 'description' => "A non-null arg ${newArgName} on ${newTypeName}->${fieldName} was added." - ]; + if ($newArgDef->getType() instanceof NonNull) { + $breakingChanges[] = [ + 'type' => self::BREAKING_CHANGE_NON_NULL_ARG_ADDED, + 'description' => "A non-null arg ${newArgName} on ${newTypeName}->${fieldName} was added." + ]; + } else { + $dangerousChanges[] = [ + 'type' => self::DANGEROUS_CHANGE_NULLABLE_ARG_ADDED, + 'description' => "A nullable arg ${newArgName} on ${newTypeName}->${fieldName} was added." + ]; + } } } } @@ -236,36 +248,18 @@ class FindBreakingChanges throw new \TypeError('unknown type ' . $type->name); } - /** - * Given two schemas, returns an Array containing descriptions of any breaking - * changes in the newSchema related to the fields on a type. This includes if - * a field has been removed from a type, if a field has changed type, or if - * a non-null field is added to an input type. - * - * @return array - */ - public static function findFieldsThatChangedType( - Schema $oldSchema, Schema $newSchema - ) - { - return array_merge( - self::findFieldsThatChangedTypeOnObjectOrInterfaceTypes($oldSchema, $newSchema), - self::findFieldsThatChangedTypeOnInputObjectTypes($oldSchema, $newSchema) - ); - } - /** * @param Schema $oldSchema * @param Schema $newSchema * * @return array */ - private static function findFieldsThatChangedTypeOnObjectOrInterfaceTypes(Schema $oldSchema, Schema $newSchema) + public static function findFieldsThatChangedTypeOnObjectOrInterfaceTypes(Schema $oldSchema, Schema $newSchema) { $oldTypeMap = $oldSchema->getTypeMap(); $newTypeMap = $newSchema->getTypeMap(); - $breakingFieldChanges = []; + $breakingChanges = []; foreach ($oldTypeMap as $typeName => $oldType) { $newType = isset($newTypeMap[$typeName]) ? $newTypeMap[$typeName] : null; if (!($oldType instanceof ObjectType || $oldType instanceof InterfaceType) || !($newType instanceof $oldType)) { @@ -275,7 +269,7 @@ class FindBreakingChanges $newTypeFieldsDef = $newType->getFields(); foreach ($oldTypeFieldsDef as $fieldName => $fieldDefinition) { if (!isset($newTypeFieldsDef[$fieldName])) { - $breakingFieldChanges[] = ['type' => self::BREAKING_CHANGE_FIELD_REMOVED, 'description' => "${typeName}->${fieldName} was removed."]; + $breakingChanges[] = ['type' => self::BREAKING_CHANGE_FIELD_REMOVED, 'description' => "${typeName}->${fieldName} was removed."]; } else { $oldFieldType = $oldTypeFieldsDef[$fieldName]->getType(); $newfieldType = $newTypeFieldsDef[$fieldName]->getType(); @@ -284,12 +278,12 @@ class FindBreakingChanges $oldFieldTypeString = self::isNamedType($oldFieldType) ? $oldFieldType->name : $oldFieldType; $newFieldTypeString = self::isNamedType($newfieldType) ? $newfieldType->name : $newfieldType; - $breakingFieldChanges[] = ['type' => self::BREAKING_CHANGE_FIELD_CHANGED, 'description' => "${typeName}->${fieldName} changed type from ${oldFieldTypeString} to ${newFieldTypeString}."]; + $breakingChanges[] = ['type' => self::BREAKING_CHANGE_FIELD_CHANGED, 'description' => "${typeName}->${fieldName} changed type from ${oldFieldTypeString} to ${newFieldTypeString}."]; } } } } - return $breakingFieldChanges; + return $breakingChanges; } /** @@ -305,7 +299,8 @@ class FindBreakingChanges $oldTypeMap = $oldSchema->getTypeMap(); $newTypeMap = $newSchema->getTypeMap(); - $breakingFieldChanges = []; + $breakingChanges = []; + $dangerousChanges = []; foreach ($oldTypeMap as $typeName => $oldType) { $newType = isset($newTypeMap[$typeName]) ? $newTypeMap[$typeName] : null; if (!($oldType instanceof InputObjectType) || !($newType instanceof InputObjectType)) { @@ -315,7 +310,10 @@ class FindBreakingChanges $newTypeFieldsDef = $newType->getFields(); foreach ($oldTypeFieldsDef as $fieldName => $fieldDefinition) { if (!isset($newTypeFieldsDef[$fieldName])) { - $breakingFieldChanges[] = ['type' => self::BREAKING_CHANGE_FIELD_REMOVED, 'description' => "${typeName}->${fieldName} was removed."]; + $breakingChanges[] = [ + 'type' => self::BREAKING_CHANGE_FIELD_REMOVED, + 'description' => "${typeName}->${fieldName} was removed." + ]; } else { $oldFieldType = $oldTypeFieldsDef[$fieldName]->getType(); $newfieldType = $newTypeFieldsDef[$fieldName]->getType(); @@ -323,18 +321,32 @@ class FindBreakingChanges if (!$isSafe) { $oldFieldTypeString = self::isNamedType($oldFieldType) ? $oldFieldType->name : $oldFieldType; $newFieldTypeString = self::isNamedType($newfieldType) ? $newfieldType->name : $newfieldType; - $breakingFieldChanges[] = ['type' => self::BREAKING_CHANGE_FIELD_CHANGED, 'description' => "${typeName}->${fieldName} changed type from ${oldFieldTypeString} to ${newFieldTypeString}."]; + $breakingChanges[] = [ + 'type' => self::BREAKING_CHANGE_FIELD_CHANGED, + 'description' => "${typeName}->${fieldName} changed type from ${oldFieldTypeString} to ${newFieldTypeString}."]; } } } + // Check if a field was added to the input object type foreach ($newTypeFieldsDef as $fieldName => $fieldDef) { - if (!isset($oldTypeFieldsDef[$fieldName]) && $fieldDef->getType() instanceof NonNull) { + if (!isset($oldTypeFieldsDef[$fieldName])) { $newTypeName = $newType->name; - $breakingFieldChanges[] = ['type' => self::BREAKING_CHANGE_NON_NULL_INPUT_FIELD_ADDED, 'description' => "A non-null field ${fieldName} on input type ${newTypeName} was added."]; + if ($fieldDef->getType() instanceof NonNull) { + $breakingChanges[] = [ + 'type' => self::BREAKING_CHANGE_NON_NULL_INPUT_FIELD_ADDED, + 'description' => "A non-null field ${fieldName} on input type ${newTypeName} was added." + ]; + } else { + $dangerousChanges[] = [ + 'type' => self::DANGEROUS_CHANGE_NULLABLE_INPUT_FIELD_ADDED, + 'description' => "A nullable field ${fieldName} on input type ${newTypeName} was added." + ]; + } } } } - return $breakingFieldChanges; + + return ['breakingChanges' => $breakingChanges, 'dangerousChanges' => $dangerousChanges]; } @@ -580,4 +592,4 @@ class FindBreakingChanges $type instanceof InputObjectType ); } -} \ No newline at end of file +} diff --git a/tests/Utils/FindBreakingChangesTest.php b/tests/Utils/FindBreakingChangesTest.php index b48f961..bfc79a9 100644 --- a/tests/Utils/FindBreakingChangesTest.php +++ b/tests/Utils/FindBreakingChangesTest.php @@ -254,7 +254,7 @@ class FindBreakingChangesTest extends \PHPUnit_Framework_TestCase ]) ]); - $this->assertEquals($expectedFieldChanges, FindBreakingChanges::findFieldsThatChangedType($oldSchema, $newSchema)); + $this->assertEquals($expectedFieldChanges, FindBreakingChanges::findFieldsThatChangedTypeOnObjectOrInterfaceTypes($oldSchema, $newSchema)); } @@ -424,7 +424,7 @@ class FindBreakingChangesTest extends \PHPUnit_Framework_TestCase ], ]; - $this->assertEquals($expectedFieldChanges, FindBreakingChanges::findFieldsThatChangedType($oldSchema, $newSchema)); + $this->assertEquals($expectedFieldChanges, FindBreakingChanges::findFieldsThatChangedTypeOnInputObjectTypes($oldSchema, $newSchema)['breakingChanges']); } public function testDetectsNonNullFieldAddedToInputType() @@ -468,7 +468,7 @@ class FindBreakingChangesTest extends \PHPUnit_Framework_TestCase 'type' => FindBreakingChanges::BREAKING_CHANGE_NON_NULL_INPUT_FIELD_ADDED, 'description' => 'A non-null field requiredField on input type InputType1 was added.' ], - FindBreakingChanges::findFieldsThatChangedType($oldSchema, $newSchema)[0] + FindBreakingChanges::findFieldsThatChangedTypeOnInputObjectTypes($oldSchema, $newSchema)['breakingChanges'][0] ); } @@ -1366,6 +1366,55 @@ class FindBreakingChangesTest extends \PHPUnit_Framework_TestCase ); } + /** + * @it should detect if a nullable field was added to an input + */ + public function testShouldDetectIfANullableFieldWasAddedToAnInput() + { + $oldInputType = new InputObjectType([ + 'name' => 'InputType1', + 'fields' => [ + 'field1' => [ + 'type' => Type::string(), + ], + ], + ]); + $newInputType = new InputObjectType([ + 'name' => 'InputType1', + 'fields' => [ + 'field1' => [ + 'type' => Type::string(), + ], + 'field2' => [ + 'type' => Type::int(), + ], + ], + ]); + + $oldSchema = new Schema([ + 'query' => $this->queryType, + 'types' => [ + $oldInputType, + ] + ]); + + $newSchema = new Schema([ + 'query' => $this->queryType, + 'types' => [ + $newInputType, + ] + ]); + + $expectedFieldChanges = [ + [ + 'description' => 'A nullable field field2 on input type InputType1 was added.', + 'type' => FindBreakingChanges::DANGEROUS_CHANGE_NULLABLE_INPUT_FIELD_ADDED + ], + ]; + + $this->assertEquals($expectedFieldChanges, FindBreakingChanges::findFieldsThatChangedTypeOnInputObjectTypes($oldSchema, $newSchema)['dangerousChanges']); + } + public function testFindsAllDangerousChanges() { $enumThatGainsAValueOld = new EnumType([ @@ -1473,4 +1522,64 @@ class FindBreakingChangesTest extends \PHPUnit_Framework_TestCase $this->assertEquals($expectedDangerousChanges, FindBreakingChanges::findDangerousChanges($oldSchema, $newSchema)); } + + /** + * @it should detect if a nullable field argument was added + */ + public function testShouldDetectIfANullableFieldArgumentWasAdded() + { + $oldType = new ObjectType([ + 'name' => 'Type1', + 'fields' => [ + 'field1' => [ + 'type' => Type::string(), + 'args' => [ + 'arg1' => [ + 'type' => Type::string(), + ], + ], + ], + ], + ]); + + $newType = new ObjectType([ + 'name' => 'Type1', + 'fields' => [ + 'field1' => [ + 'type' => Type::string(), + 'args' => [ + 'arg1' => [ + 'type' => Type::string(), + ], + 'arg2' => [ + 'type' => Type::string(), + ], + ], + ], + ], + ]); + + $oldSchema = new Schema([ + 'query' => $this->queryType, + 'types' => [ + $oldType, + ] + ]); + + $newSchema = new Schema([ + 'query' => $this->queryType, + 'types' => [ + $newType, + ] + ]); + + $expectedFieldChanges = [ + [ + 'description' => 'A nullable arg arg2 on Type1->field1 was added.', + 'type' => FindBreakingChanges::DANGEROUS_CHANGE_NULLABLE_ARG_ADDED + ], + ]; + + $this->assertEquals($expectedFieldChanges, FindBreakingChanges::findArgChanges($oldSchema, $newSchema)['dangerousChanges']); + } }