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.

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 edge case with bad argument renamed #3164

Merged
merged 1 commit into from
Apr 3, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
31 changes: 18 additions & 13 deletions src/main/java/graphql/schema/diffing/ana/EditOperationAnalyzer.java
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ public EditOperationAnalysisResult analyzeEdits(List<EditOperation> editOperatio
if (editOperation.getTargetVertex().isOfType(SchemaGraph.FIELD)) {
fieldChanged(editOperation);
} else if (editOperation.getTargetVertex().isOfType(SchemaGraph.ARGUMENT)) {
handleArgumentChange(editOperation);
handleArgumentChange(editOperation, mapping);
} else if (editOperation.getTargetVertex().isOfType(SchemaGraph.INPUT_FIELD)) {
handleInputFieldChange(editOperation);
}
Expand Down Expand Up @@ -656,16 +656,21 @@ private void handleInputFieldChange(EditOperation editOperation) {
getInputObjectModification(newName).getDetails().add(new InputObjectFieldRename(oldName, inputField.getName()));
}

private void handleArgumentChange(EditOperation editOperation) {
private void handleArgumentChange(EditOperation editOperation, Mapping mapping) {
Vertex oldArgument = editOperation.getSourceVertex();
Vertex argument = editOperation.getTargetVertex();

if (!doesArgumentChangeMakeSense(oldArgument, argument, mapping)) {
return;
}

Vertex fieldOrDirective = newSchemaGraph.getFieldOrDirectiveForArgument(argument);
if (fieldOrDirective.isOfType(SchemaGraph.DIRECTIVE)) {
Vertex directive = fieldOrDirective;
DirectiveModification directiveModification = getDirectiveModification(directive.getName());
String oldName = editOperation.getSourceVertex().getName();
String oldName = oldArgument.getName();
String newName = argument.getName();
directiveModification.getDetails().add(new DirectiveArgumentRename(oldName, newName));

} else {
assertTrue(fieldOrDirective.isOfType(SchemaGraph.FIELD));
Vertex field = fieldOrDirective;
Expand All @@ -674,17 +679,16 @@ private void handleArgumentChange(EditOperation editOperation) {
if (fieldsContainerForField.isOfType(SchemaGraph.OBJECT)) {
Vertex object = fieldsContainerForField;
ObjectModification objectModification = getObjectModification(object.getName());
String oldName = editOperation.getSourceVertex().getName();
String oldName = oldArgument.getName();
String newName = argument.getName();
objectModification.getDetails().add(new ObjectFieldArgumentRename(fieldName, oldName, newName));
} else {
assertTrue(fieldsContainerForField.isOfType(SchemaGraph.INTERFACE));
Vertex interfaze = fieldsContainerForField;
InterfaceModification interfaceModification = getInterfaceModification(interfaze.getName());
String oldName = editOperation.getSourceVertex().getName();
String oldName = oldArgument.getName();
String newName = argument.getName();
interfaceModification.getDetails().add(new InterfaceFieldArgumentRename(fieldName, oldName, newName));

}
}
}
Expand Down Expand Up @@ -1162,12 +1166,13 @@ private void inputFieldTypeOrDefaultValueChanged(EditOperation editOperation) {
}

private void argumentTypeOrDefaultValueChanged(EditOperation editOperation, Mapping mapping) {
if (!doesArgumentChangeMakeSense(editOperation, mapping)) {
Vertex oldArgument = editOperation.getSourceEdge().getFrom();
Vertex argument = editOperation.getTargetEdge().getFrom();

if (!doesArgumentChangeMakeSense(oldArgument, argument, mapping)) {
return;
}

Edge targetEdge = editOperation.getTargetEdge();
Vertex argument = targetEdge.getFrom();
Vertex fieldOrDirective = newSchemaGraph.getFieldOrDirectiveForArgument(argument);
if (fieldOrDirective.isOfType(SchemaGraph.FIELD)) {
Vertex field = fieldOrDirective;
Expand Down Expand Up @@ -1231,10 +1236,10 @@ private void argumentTypeOrDefaultValueChanged(EditOperation editOperation, Mapp
* <p>
* We only want to report argument type changes if it makes sense i.e. if the argument container was the same.
*/
private boolean doesArgumentChangeMakeSense(EditOperation editOperation, Mapping mapping) {
private boolean doesArgumentChangeMakeSense(Vertex oldArgument, Vertex newArgument, Mapping mapping) {
// Container for an argument in this case should be a field or directive
Vertex oldContainer = oldSchemaGraph.getFieldOrDirectiveForArgument(editOperation.getSourceEdge().getFrom());
Vertex newContainer = newSchemaGraph.getFieldOrDirectiveForArgument(editOperation.getTargetEdge().getFrom());
Vertex oldContainer = oldSchemaGraph.getFieldOrDirectiveForArgument(oldArgument);
Vertex newContainer = newSchemaGraph.getFieldOrDirectiveForArgument(newArgument);

// Make sure the container is the same
return mapping.getTarget(oldContainer) == newContainer;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2557,6 +2557,130 @@ class EditOperationAnalyzerTest extends Specification {
defaultValueModification[0].newValue == '1000'
}

def "object field with argument removed and similarly named argument added"() {
given:
def oldSdl = """
type Query {
issues: IssueQuery
}
type IssueQuery {
issues(id: [ID!]): [Issue]
issuesById: [Issue]
}
type Issue {
id: ID!
}
"""
def newSdl = '''
type Query {
issues: IssueQuery
}
type IssueQuery {
issuesById(ids: [ID!]!): [Issue]
}
type Issue {
id: ID!
}
'''

when:
def changes = calcDiff(oldSdl, newSdl)

then:
changes.objectDifferences["IssueQuery"] instanceof ObjectModification
def issueQueryChanges = changes.objectDifferences["IssueQuery"] as ObjectModification
issueQueryChanges.details.size() == 2

def fieldDeletion = issueQueryChanges.getDetails(ObjectFieldDeletion)
fieldDeletion.size() == 1
fieldDeletion[0].name == "issues"

def fieldArgumentAddition = issueQueryChanges.getDetails(ObjectFieldArgumentAddition)
fieldArgumentAddition.size() == 1
fieldArgumentAddition[0].fieldName == "issuesById"
fieldArgumentAddition[0].name == "ids"
}

def "interface field with argument removed and similarly named argument added"() {
given:
def oldSdl = """
type Query {
issues: IssueQuery
}
interface IssueQuery {
issues(id: [ID!]): [Issue]
issuesById: [Issue]
}
type Issue {
id: ID!
}
"""
def newSdl = '''
type Query {
issues: IssueQuery
}
interface IssueQuery {
issuesById(ids: [ID!]!): [Issue]
}
type Issue {
id: ID!
}
'''

when:
def changes = calcDiff(oldSdl, newSdl)

then:
changes.interfaceDifferences["IssueQuery"] instanceof InterfaceModification
def issueQueryChanges = changes.interfaceDifferences["IssueQuery"] as InterfaceModification
issueQueryChanges.details.size() == 2

def fieldDeletion = issueQueryChanges.getDetails(InterfaceFieldDeletion)
fieldDeletion.size() == 1
fieldDeletion[0].name == "issues"

def fieldArgumentAddition = issueQueryChanges.getDetails(InterfaceFieldArgumentAddition)
fieldArgumentAddition.size() == 1
fieldArgumentAddition[0].fieldName == "issuesById"
fieldArgumentAddition[0].name == "ids"
}

def "directive removed and similarly named argument added"() {
given:
def oldSdl = '''
directive @dog(name: String) on FIELD_DEFINITION
directive @cat on FIELD_DEFINITION
type Query {
pet: String
}
'''
def newSdl = '''
directive @cat(names: String) on FIELD_DEFINITION
type Query {
pet: String
}
'''

when:
def changes = calcDiff(oldSdl, newSdl)

then:
changes.directiveDifferences["dog"] instanceof DirectiveDeletion
def dogChanges = changes.directiveDifferences["dog"] as DirectiveDeletion
dogChanges.name == "dog"

changes.directiveDifferences["cat"] instanceof DirectiveModification
def catChanges = changes.directiveDifferences["cat"] as DirectiveModification
!catChanges.isNameChanged()
catChanges.oldName == catChanges.newName
catChanges.newName == "cat"
catChanges.details.size() == 1

def argumentAddition = catChanges.getDetails(DirectiveArgumentAddition)
argumentAddition.size() == 1
argumentAddition[0].name == "names"
}

EditOperationAnalysisResult calcDiff(
String oldSdl,
String newSdl
Expand Down