SchemaDiff EnforceDirectives Option Does Not Identify Removal of Directive From Type or Field #3343
Description
Describe the bug
The enforceDirectives option for SchemaDiff does not properly detect if a directive was removed from a field or type. This is because it relies on an introspection result as input. The introspection query does not return directives on fields or types so they are not present when the diff of the document is ran.
To Reproduce
Schema 1:
directive @someDirective on FIELD_DEFINITION
type test {
version: String! @someDirective
}
type Query {
getTests: [test]!
}
Schema 2:
directive @someDirective on FIELD_DEFINITION
type test {
version: String!
}
type Query {
getTests: [test]!
}
When schema diff is run on these two schemas it will detect no changes even though removal of a directive could be a breaking change especially for composite schemas.
Potential solution would be providing the ability for DiffSet to provide the SchemaDefinition directly so that it can be overridden to use an approach that does not require introspection:
public interface DiffSet {
Document getOldSchemaDefinition();
Document getNewSchemaDefinition();
}
public class IntrospectionDiffSet implements DiffSet {
....
@Override
public Document getOldSchemaDefinition() {
return new IntrospectionResultToSchema().createSchemaDefinition(this.introspectionOld);
}
@Override
public Document getNewSchemaDefinition() {
return new IntrospectionResultToSchema().createSchemaDefinition(this.introspectionNew);
}
}
The SchemaDiff class can then use this hook rather than requiring the introspection result method.
public class SchemaDiff {
...
private void diffSchemaImpl(DiffSet diffSet, DifferenceReporter reporter) {
Document oldDoc = diffSet.getOldSchemaDefinition();
Document newDoc = diffSet.getNewSchemaDefinition();
...
}
}
We would then be able to override this implementation with something like this:
public class SDLDiffSet implements DiffSet {
....
@Override
public Document getOldSchemaDefinition() {
return Parser.parse(this.oldSchemaSdl);
}
@Override
public Document getNewSchemaDefinition() {
return Parser.parse(this.newSchemaSdl);
}
}
I am willing to help with a PR if there is some agreement on direction here.