Merge pull request #3164 from gnawf/fix-argument-renamed

Fix edge case with bad argument renamed

src/main/java/graphql/schema/diffing/ana/EditOperationAnalyzer.java

@@ -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);
                     }
@@ -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;
@@ -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));
-
             }
         }
     }
@@ -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;
@@ -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;

src/test/groovy/graphql/schema/diffing/ana/EditOperationAnalyzerTest.groovy

@@ -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