Find breaking directive changes

ref: graphql/graphql-js#1152
This commit is contained in:
Daniel Tschinder 2018-02-16 00:15:19 +01:00
parent ddfeee314c
commit d71b45d60e
2 changed files with 419 additions and 50 deletions

View File

@ -5,10 +5,12 @@
namespace GraphQL\Utils;
use GraphQL\Type\Definition\Directive;
use GraphQL\Type\Definition\EnumType;
use GraphQL\Type\Definition\InputObjectType;
use GraphQL\Type\Definition\InterfaceType;
use GraphQL\Type\Definition\ListOfType;
use GraphQL\Type\Definition\NamedType;
use GraphQL\Type\Definition\NonNull;
use GraphQL\Type\Definition\ObjectType;
use GraphQL\Type\Definition\ScalarType;
@ -30,6 +32,10 @@ class FindBreakingChanges
const BREAKING_CHANGE_NON_NULL_ARG_ADDED = 'NON_NULL_ARG_ADDED';
const BREAKING_CHANGE_NON_NULL_INPUT_FIELD_ADDED = 'NON_NULL_INPUT_FIELD_ADDED';
const BREAKING_CHANGE_INTERFACE_REMOVED_FROM_OBJECT = 'INTERFACE_REMOVED_FROM_OBJECT';
const BREAKING_CHANGE_DIRECTIVE_REMOVED = 'DIRECTIVE_REMOVED';
const BREAKING_CHANGE_DIRECTIVE_ARG_REMOVED = 'DIRECTIVE_ARG_REMOVED';
const BREAKING_CHANGE_DIRECTIVE_LOCATION_REMOVED = 'DIRECTIVE_LOCATION_REMOVED';
const BREAKING_CHANGE_NON_NULL_DIRECTIVE_ARG_ADDED = 'NON_NULL_DIRECTIVE_ARG_ADDED';
const DANGEROUS_CHANGE_ARG_DEFAULT_VALUE = 'ARG_DEFAULT_VALUE_CHANGE';
const DANGEROUS_CHANGE_VALUE_ADDED_TO_ENUM = 'VALUE_ADDED_TO_ENUM';
@ -53,7 +59,11 @@ class FindBreakingChanges
self::findTypesRemovedFromUnions($oldSchema, $newSchema),
self::findValuesRemovedFromEnums($oldSchema, $newSchema),
self::findArgChanges($oldSchema, $newSchema)['breakingChanges'],
self::findInterfacesRemovedFromObjectTypes($oldSchema, $newSchema)
self::findInterfacesRemovedFromObjectTypes($oldSchema, $newSchema),
self::findRemovedDirectives($oldSchema, $newSchema),
self::findRemovedDirectiveArgs($oldSchema, $newSchema),
self::findAddedNonNullDirectiveArgs($oldSchema, $newSchema),
self::findRemovedDirectiveLocations($oldSchema, $newSchema)
);
}
@ -283,8 +293,8 @@ class FindBreakingChanges
$isSafe = self::isChangeSafeForObjectOrInterfaceField($oldFieldType, $newfieldType);
if (!$isSafe) {
$oldFieldTypeString = self::isNamedType($oldFieldType) ? $oldFieldType->name : $oldFieldType;
$newFieldTypeString = self::isNamedType($newfieldType) ? $newfieldType->name : $newfieldType;
$oldFieldTypeString = $oldFieldType instanceof NamedType ? $oldFieldType->name : $oldFieldType;
$newFieldTypeString = $newfieldType instanceof NamedType ? $newfieldType->name : $newfieldType;
$breakingChanges[] = ['type' => self::BREAKING_CHANGE_FIELD_CHANGED, 'description' => "${typeName}->${fieldName} changed type from ${oldFieldTypeString} to ${newFieldTypeString}."];
}
}
@ -323,11 +333,11 @@ class FindBreakingChanges
];
} else {
$oldFieldType = $oldTypeFieldsDef[$fieldName]->getType();
$newfieldType = $newTypeFieldsDef[$fieldName]->getType();
$isSafe = self::isChangeSafeForInputObjectFieldOrFieldArg($oldFieldType, $newfieldType);
$newFieldType = $newTypeFieldsDef[$fieldName]->getType();
$isSafe = self::isChangeSafeForInputObjectFieldOrFieldArg($oldFieldType, $newFieldType);
if (!$isSafe) {
$oldFieldTypeString = self::isNamedType($oldFieldType) ? $oldFieldType->name : $oldFieldType;
$newFieldTypeString = self::isNamedType($newfieldType) ? $newfieldType->name : $newfieldType;
$oldFieldTypeString = $oldFieldType instanceof NamedType ? $oldFieldType->name : $oldFieldType;
$newFieldTypeString = $newFieldType instanceof NamedType ? $newFieldType->name : $newFieldType;
$breakingChanges[] = [
'type' => self::BREAKING_CHANGE_FIELD_CHANGED,
'description' => "${typeName}->${fieldName} changed type from ${oldFieldTypeString} to ${newFieldTypeString}."];
@ -361,9 +371,9 @@ class FindBreakingChanges
Type $oldType, Type $newType
)
{
if (self::isNamedType($oldType)) {
if ($oldType instanceof NamedType) {
// if they're both named types, see if their names are equivalent
return (self::isNamedType($newType) && $oldType->name === $newType->name)
return ($newType instanceof NamedType && $oldType->name === $newType->name)
// moving from nullable to non-null of the same underlying type is safe
|| ($newType instanceof NonNull
&& self::isChangeSafeForObjectOrInterfaceField(
@ -387,16 +397,16 @@ class FindBreakingChanges
/**
* @param Type $oldType
* @param Schema $newSchema
* @param Type $newType
*
* @return bool
*/
private static function isChangeSafeForInputObjectFieldOrFieldArg(
Type $oldType, Type $newType
)
{
if (self::isNamedType($oldType)) {
return self::isNamedType($newType) && $oldType->name === $newType->name;
Type $oldType,
Type $newType
) {
if ($oldType instanceof NamedType) {
return $newType instanceof NamedType && $oldType->name === $newType->name;
} elseif ($oldType instanceof ListOfType) {
return $newType instanceof ListOfType && self::isChangeSafeForInputObjectFieldOrFieldArg($oldType->getWrappedType(), $newType->getWrappedType());
} elseif ($oldType instanceof NonNull) {
@ -583,20 +593,135 @@ class FindBreakingChanges
return $breakingChanges;
}
/**
* @param Type $type
*
* @return bool
*/
private static function isNamedType(Type $type)
public static function findRemovedDirectives(Schema $oldSchema, Schema $newSchema)
{
return (
$type instanceof ScalarType ||
$type instanceof ObjectType ||
$type instanceof InterfaceType ||
$type instanceof UnionType ||
$type instanceof EnumType ||
$type instanceof InputObjectType
);
$removedDirectives = [];
$newSchemaDirectiveMap = self::getDirectiveMapForSchema($newSchema);
foreach($oldSchema->getDirectives() as $directive) {
if (!isset($newSchemaDirectiveMap[$directive->name])) {
$removedDirectives[] = [
'type' => self::BREAKING_CHANGE_DIRECTIVE_REMOVED,
'description' => "{$directive->name} was removed",
];
}
}
return $removedDirectives;
}
public static function findRemovedArgsForDirectives(Directive $oldDirective, Directive $newDirective)
{
$removedArgs = [];
$newArgMap = self::getArgumentMapForDirective($newDirective);
foreach((array) $oldDirective->args as $arg) {
if (!isset($newArgMap[$arg->name])) {
$removedArgs[] = $arg;
}
}
return $removedArgs;
}
public static function findRemovedDirectiveArgs(Schema $oldSchema, Schema $newSchema)
{
$removedDirectiveArgs = [];
$oldSchemaDirectiveMap = self::getDirectiveMapForSchema($oldSchema);
foreach($newSchema->getDirectives() as $newDirective) {
if (!isset($oldSchemaDirectiveMap[$newDirective->name])) {
continue;
}
foreach(self::findRemovedArgsForDirectives($oldSchemaDirectiveMap[$newDirective->name], $newDirective) as $arg) {
$removedDirectiveArgs[] = [
'type' => self::BREAKING_CHANGE_DIRECTIVE_ARG_REMOVED,
'description' => "{$arg->name} was removed from {$newDirective->name}",
];
}
}
return $removedDirectiveArgs;
}
public static function findAddedArgsForDirective(Directive $oldDirective, Directive $newDirective)
{
$addedArgs = [];
$oldArgMap = self::getArgumentMapForDirective($oldDirective);
foreach((array) $newDirective->args as $arg) {
if (!isset($oldArgMap[$arg->name])) {
$addedArgs[] = $arg;
}
}
return $addedArgs;
}
public static function findAddedNonNullDirectiveArgs(Schema $oldSchema, Schema $newSchema)
{
$addedNonNullableArgs = [];
$oldSchemaDirectiveMap = self::getDirectiveMapForSchema($oldSchema);
foreach($newSchema->getDirectives() as $newDirective) {
if (!isset($oldSchemaDirectiveMap[$newDirective->name])) {
continue;
}
foreach(self::findAddedArgsForDirective($oldSchemaDirectiveMap[$newDirective->name], $newDirective) as $arg) {
if (!$arg->getType() instanceof NonNull) {
continue;
}
$addedNonNullableArgs[] = [
'type' => self::BREAKING_CHANGE_NON_NULL_DIRECTIVE_ARG_ADDED,
'description' => "A non-null arg {$arg->name} on directive {$newDirective->name} was added",
];
}
}
return $addedNonNullableArgs;
}
public static function findRemovedLocationsForDirective(Directive $oldDirective, Directive $newDirective)
{
$removedLocations = [];
$newLocationSet = array_flip($newDirective->locations);
foreach($oldDirective->locations as $oldLocation) {
if (!array_key_exists($oldLocation, $newLocationSet)) {
$removedLocations[] = $oldLocation;
}
}
return $removedLocations;
}
public static function findRemovedDirectiveLocations(Schema $oldSchema, Schema $newSchema)
{
$removedLocations = [];
$oldSchemaDirectiveMap = self::getDirectiveMapForSchema($oldSchema);
foreach($newSchema->getDirectives() as $newDirective) {
if (!isset($oldSchemaDirectiveMap[$newDirective->name])) {
continue;
}
foreach(self::findRemovedLocationsForDirective($oldSchemaDirectiveMap[$newDirective->name], $newDirective) as $location) {
$removedLocations[] = [
'type' => self::BREAKING_CHANGE_DIRECTIVE_LOCATION_REMOVED,
'description' => "{$location} was removed from {$newDirective->name}",
];
}
}
return $removedLocations;
}
private static function getDirectiveMapForSchema(Schema $schema)
{
return Utils::keyMap($schema->getDirectives(), function ($dir) { return $dir->name; });
}
private static function getArgumentMapForDirective(Directive $directive)
{
return Utils::keyMap($directive->args ?: [], function ($arg) { return $arg->name; });
}
}

View File

@ -1,7 +1,10 @@
<?php
namespace GraphQL\Tests\Utils;
use GraphQL\Language\DirectiveLocation;
use GraphQL\Type\Definition\Directive;
use GraphQL\Type\Definition\EnumType;
use GraphQL\Type\Definition\FieldArgument;
use GraphQL\Type\Definition\InputObjectType;
use GraphQL\Type\Definition\InterfaceType;
use GraphQL\Type\Definition\ObjectType;
@ -1134,32 +1137,78 @@ class FindBreakingChangesTest extends \PHPUnit_Framework_TestCase
]
]);
$directiveThatIsRemoved = Directive::skipDirective();
$directiveThatRemovesArgOld = new Directive([
'name' => 'DirectiveThatRemovesArg',
'locations' => [DirectiveLocation::FIELD_DEFINITION],
'args' => FieldArgument::createMap([
'arg1' => [
'name' => 'arg1',
],
]),
]);
$directiveThatRemovesArgNew = new Directive([
'name' => 'DirectiveThatRemovesArg',
'locations' => [DirectiveLocation::FIELD_DEFINITION],
]);
$nonNullDirectiveAddedOld = new Directive([
'name' => 'NonNullDirectiveAdded',
'locations' => [DirectiveLocation::FIELD_DEFINITION],
]);
$nonNullDirectiveAddedNew = new Directive([
'name' => 'NonNullDirectiveAdded',
'locations' => [DirectiveLocation::FIELD_DEFINITION],
'args' => FieldArgument::createMap([
'arg1' => [
'name' => 'arg1',
'type' => Type::nonNull(Type::boolean()),
],
]),
]);
$directiveRemovedLocationOld = new Directive([
'name' => 'Directive Name',
'locations' => [DirectiveLocation::FIELD_DEFINITION, DirectiveLocation::QUERY],
]);
$directiveRemovedLocationNew = new Directive([
'name' => 'Directive Name',
'locations' => [DirectiveLocation::FIELD_DEFINITION],
]);
$oldSchema = new Schema([
'query' => $this->queryType,
'types' =>
[
'TypeThatGetsRemoved' => $typeThatGetsRemoved,
'TypeThatChangesType' => $typeThatChangesTypeOld,
'TypeThatHasBreakingFieldChanges' => $typeThatHasBreakingFieldChangesOld,
'UnionTypeThatLosesAType' => $unionTypeThatLosesATypeOld,
'EnumTypeThatLosesAValue' => $enumTypeThatLosesAValueOld,
'ArgThatChanges' => $argThatChanges,
'TypeThatLosesInterface' => $typeThatLosesInterfaceOld
]
'types' => [
'TypeThatGetsRemoved' => $typeThatGetsRemoved,
'TypeThatChangesType' => $typeThatChangesTypeOld,
'TypeThatHasBreakingFieldChanges' => $typeThatHasBreakingFieldChangesOld,
'UnionTypeThatLosesAType' => $unionTypeThatLosesATypeOld,
'EnumTypeThatLosesAValue' => $enumTypeThatLosesAValueOld,
'ArgThatChanges' => $argThatChanges,
'TypeThatLosesInterface' => $typeThatLosesInterfaceOld
],
'directives' => [
$directiveThatIsRemoved,
$directiveThatRemovesArgOld,
$nonNullDirectiveAddedOld,
$directiveRemovedLocationOld,
]
]);
$newSchema = new Schema([
'query' => $this->queryType,
'types' =>
[
'TypeThatChangesType' => $typeThatChangesTypeNew,
'TypeThatHasBreakingFieldChanges' => $typeThatHasBreakingFieldChangesNew,
'UnionTypeThatLosesAType' => $unionTypeThatLosesATypeNew,
'EnumTypeThatLosesAValue' => $enumTypeThatLosesAValueNew,
'ArgThatChanges' => $argChanged,
'TypeThatLosesInterface' => $typeThatLosesInterfaceNew,
'Interface1' => $interface1
]
'types' => [
'TypeThatChangesType' => $typeThatChangesTypeNew,
'TypeThatHasBreakingFieldChanges' => $typeThatHasBreakingFieldChangesNew,
'UnionTypeThatLosesAType' => $unionTypeThatLosesATypeNew,
'EnumTypeThatLosesAValue' => $enumTypeThatLosesAValueNew,
'ArgThatChanges' => $argChanged,
'TypeThatLosesInterface' => $typeThatLosesInterfaceNew,
'Interface1' => $interface1
],
'directives' => [
$directiveThatRemovesArgNew,
$nonNullDirectiveAddedNew,
$directiveRemovedLocationNew,
]
]);
$expectedBreakingChanges = [
@ -1206,13 +1255,208 @@ class FindBreakingChangesTest extends \PHPUnit_Framework_TestCase
[
'type' => FindBreakingChanges::BREAKING_CHANGE_INTERFACE_REMOVED_FROM_OBJECT,
'description' => 'TypeThatLosesInterface1 no longer implements interface Interface1.',
],
[
'type' => FindBreakingChanges::BREAKING_CHANGE_DIRECTIVE_REMOVED,
'description' => 'skip was removed',
],
[
'type' => FindBreakingChanges::BREAKING_CHANGE_DIRECTIVE_ARG_REMOVED,
'description' => 'arg1 was removed from DirectiveThatRemovesArg',
],
[
'type' => FindBreakingChanges::BREAKING_CHANGE_NON_NULL_DIRECTIVE_ARG_ADDED,
'description' => 'A non-null arg arg1 on directive NonNullDirectiveAdded was added',
],
[
'type' => FindBreakingChanges::BREAKING_CHANGE_DIRECTIVE_LOCATION_REMOVED,
'description' => 'QUERY was removed from Directive Name',
]
];
$this->assertEquals($expectedBreakingChanges, FindBreakingChanges::findBreakingChanges($oldSchema, $newSchema));
}
// findDangerousChanges tests below here
/**
* @it should detect if a directive was explicitly removed
*/
public function testShouldDetectIfADirectiveWasExplicitlyRemoved()
{
$oldSchema = new Schema([
'directives' => [Directive::skipDirective(), Directive::includeDirective()],
]);
$newSchema = new Schema([
'directives' => [Directive::skipDirective()],
]);
$includeDirective = Directive::includeDirective();
$expectedBreakingChanges = [
[
'type' => FindBreakingChanges::BREAKING_CHANGE_DIRECTIVE_REMOVED,
'description' => "{$includeDirective->name} was removed",
]
];
$this->assertEquals($expectedBreakingChanges, FindBreakingChanges::findRemovedDirectives($oldSchema, $newSchema));
}
/**
* @it should detect if a directive was implicitly removed
*/
public function testShouldDetectIfADirectiveWasImplicitlyRemoved()
{
$oldSchema = new Schema([]);
$newSchema = new Schema([
'directives' => [Directive::skipDirective(), Directive::includeDirective()],
]);
$deprecatedDirective = Directive::deprecatedDirective();
$expectedBreakingChanges = [
[
'type' => FindBreakingChanges::BREAKING_CHANGE_DIRECTIVE_REMOVED,
'description' => "{$deprecatedDirective->name} was removed",
]
];
$this->assertEquals($expectedBreakingChanges, FindBreakingChanges::findRemovedDirectives($oldSchema, $newSchema));
}
/**
* @it should detect if a directive argument was removed
*/
public function testShouldDetectIfADirectiveArgumentWasRemoved()
{
$oldSchema = new Schema([
'directives' => [
new Directive([
'name' => 'DirectiveWithArg',
'locations' => [DirectiveLocation::FIELD_DEFINITION],
'args' => FieldArgument::createMap([
'arg1' => [
'name' => 'arg1',
],
]),
])
],
]);
$newSchema = new Schema([
'directives' => [
new Directive([
'name' => 'DirectiveWithArg',
'locations' => [DirectiveLocation::FIELD_DEFINITION],
])
],
]);
$expectedBreakingChanges = [
[
'type' => FindBreakingChanges::BREAKING_CHANGE_DIRECTIVE_ARG_REMOVED,
'description' => "arg1 was removed from DirectiveWithArg",
]
];
$this->assertEquals($expectedBreakingChanges, FindBreakingChanges::findRemovedDirectiveArgs($oldSchema, $newSchema));
}
/**
* @it should detect if a non-nullable directive argument was added
*/
public function testShouldDetectIfANonNullableDirectiveArgumentWasAdded()
{
$oldSchema = new Schema([
'directives' => [
new Directive([
'name' => 'DirectiveName',
'locations' => [DirectiveLocation::FIELD_DEFINITION],
])
],
]);
$newSchema = new Schema([
'directives' => [
new Directive([
'name' => 'DirectiveName',
'locations' => [DirectiveLocation::FIELD_DEFINITION],
'args' => FieldArgument::createMap([
'arg1' => [
'name' => 'arg1',
'type' => Type::nonNull(Type::boolean()),
],
]),
])
],
]);
$expectedBreakingChanges = [
[
'type' => FindBreakingChanges::BREAKING_CHANGE_NON_NULL_DIRECTIVE_ARG_ADDED,
'description' => "A non-null arg arg1 on directive DirectiveName was added",
]
];
$this->assertEquals($expectedBreakingChanges, FindBreakingChanges::findAddedNonNullDirectiveArgs($oldSchema, $newSchema));
}
/**
* @it should detect locations removed from a directive
*/
public function testShouldDetectLocationsRemovedFromADirective()
{
$d1 = new Directive([
'name' => 'Directive Name',
'locations' => [DirectiveLocation::FIELD_DEFINITION, DirectiveLocation::QUERY],
]);
$d2 = new Directive([
'name' => 'Directive Name',
'locations' => [DirectiveLocation::FIELD_DEFINITION],
]);
$this->assertEquals([DirectiveLocation::QUERY], FindBreakingChanges::findRemovedLocationsForDirective($d1, $d2));
}
/**
* @it should detect locations removed directives within a schema
*/
public function testShouldDetectLocationsRemovedDirectiveWithinASchema()
{
$oldSchema = new Schema([
'directives' => [
new Directive([
'name' => 'Directive Name',
'locations' => [
DirectiveLocation::FIELD_DEFINITION,
DirectiveLocation::QUERY
],
])
],
]);
$newSchema = new Schema([
'directives' => [
new Directive([
'name' => 'Directive Name',
'locations' => [DirectiveLocation::FIELD_DEFINITION],
])
],
]);
$expectedBreakingChanges = [
[
'type' => FindBreakingChanges::BREAKING_CHANGE_DIRECTIVE_LOCATION_REMOVED,
'description' => "QUERY was removed from Directive Name",
]
];
$this->assertEquals($expectedBreakingChanges, FindBreakingChanges::findRemovedDirectiveLocations($oldSchema, $newSchema));
}
// DESCRIBE: findDangerousChanges
public function testFindDangerousArgChanges()
{