Skip to content

Diffing fixes 2 #3146

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.

Sign up for GitHub

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

Merged
andimarek merged 3 commits into from
Mar 23, 2023
Merged

Diffing fixes 2 #3146

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6b32c1e
Add more running checks in some loops
gnawf Mar 23, 2023
24de5c7
Fix argument deletion when type is deleted
gnawf Mar 23, 2023
3406441
Fix field rename on description change
gnawf Mar 23, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions src/main/java/graphql/schema/diffing/DiffImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,8 @@ private void addChildToQueue(MappingEntry parentEntry,
costMatrix[i - level].set(j, cost);
j++;
}

runningCheck.check();
}
HungarianAlgorithm hungarianAlgorithm = new HungarianAlgorithm(costMatrixForHungarianAlgo);

Expand Down Expand Up @@ -278,6 +280,8 @@ private void calculateRestOfChildren(List<Vertex> availableTargetVertices,
sibling.availableTargetVertices = availableTargetVertices;

siblings.add(sibling);

runningCheck.check();
}
siblings.add(LAST_ELEMENT);

Expand Down Expand Up @@ -390,6 +394,7 @@ private double calcLowerBoundMappingCost(Vertex v,
anchoredVerticesCost++;
}

runningCheck.check();
Copy link
Contributor Author

@gnawf gnawf Mar 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added more checks to places that the algorithm would get stuck on when the changes were large.

}

Multiset<String> intersection = Multisets.intersection(multisetLabelsV, multisetLabelsU);
Expand Down
21 changes: 17 additions & 4 deletions src/main/java/graphql/schema/diffing/ana/EditOperationAnalyzer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -740,18 +740,22 @@ private void handleEnumValueChanged(EditOperation editOperation) {
private void fieldChanged(EditOperation editOperation) {
Vertex field = editOperation.getTargetVertex();
Vertex fieldsContainerForField = newSchemaGraph.getFieldsContainerForField(field);

String oldName = editOperation.getSourceVertex().getName();
String newName = field.getName();
if (oldName.equals(newName)) {
// Something else like description could have changed
return;
}

if (fieldsContainerForField.isOfType(SchemaGraph.OBJECT)) {
Vertex object = fieldsContainerForField;
ObjectModification objectModification = getObjectModification(object.getName());
String oldName = editOperation.getSourceVertex().getName();
String newName = field.getName();
objectModification.getDetails().add(new ObjectFieldRename(oldName, newName));
} else {
assertTrue(fieldsContainerForField.isOfType(SchemaGraph.INTERFACE));
Vertex interfaze = fieldsContainerForField;
InterfaceModification interfaceModification = getInterfaceModification(interfaze.getName());
String oldName = editOperation.getSourceVertex().getName();
String newName = field.getName();
interfaceModification.getDetails().add(new InterfaceFieldRename(oldName, newName));
}
}
Expand Down Expand Up @@ -1688,15 +1692,24 @@ private void argumentDeleted(EditOperation editOperation) {
Vertex fieldsContainerForField = oldSchemaGraph.getFieldsContainerForField(field);
if (fieldsContainerForField.isOfType(SchemaGraph.OBJECT)) {
Vertex object = fieldsContainerForField;
if (isObjectDeleted(object.getName())) {
return;
}
getObjectModification(object.getName()).getDetails().add(new ObjectFieldArgumentDeletion(field.getName(), deletedArgument.getName()));
} else {
assertTrue(fieldsContainerForField.isOfType(SchemaGraph.INTERFACE));
Vertex interfaze = fieldsContainerForField;
if (isInterfaceDeleted(interfaze.getName())) {
return;
}
getInterfaceModification(interfaze.getName()).getDetails().add(new InterfaceFieldArgumentDeletion(field.getName(), deletedArgument.getName()));
}
} else {
assertTrue(fieldOrDirective.isOfType(SchemaGraph.DIRECTIVE));
Vertex directive = fieldOrDirective;
if (isDirectiveDeleted(directive.getName())) {
return;
}
getDirectiveModification(directive.getName()).getDetails().add(new DirectiveArgumentDeletion(deletedArgument.getName()));
}

Expand Down
104 changes: 101 additions & 3 deletions src/test/groovy/graphql/schema/diffing/ana/EditOperationAnalyzerTest.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@ import graphql.schema.diffing.SchemaDiffing
import spock.lang.Specification

import static graphql.schema.diffing.ana.SchemaDifference.AppliedDirectiveDeletion
import static graphql.schema.diffing.ana.SchemaDifference.AppliedDirectiveObjectFieldArgumentLocation
import static graphql.schema.diffing.ana.SchemaDifference.AppliedDirectiveDirectiveArgumentLocation
import static graphql.schema.diffing.ana.SchemaDifference.AppliedDirectiveObjectFieldLocation
import static graphql.schema.diffing.ana.SchemaDifference.AppliedDirectiveInterfaceFieldArgumentLocation
import static graphql.schema.diffing.ana.SchemaDifference.AppliedDirectiveObjectFieldArgumentLocation
import static graphql.schema.diffing.ana.SchemaDifference.AppliedDirectiveObjectFieldLocation
import static graphql.schema.diffing.ana.SchemaDifference.DirectiveAddition
import static graphql.schema.diffing.ana.SchemaDifference.DirectiveArgumentAddition
import static graphql.schema.diffing.ana.SchemaDifference.DirectiveArgumentDefaultValueModification
Expand All @@ -32,6 +32,7 @@ import static graphql.schema.diffing.ana.SchemaDifference.InputObjectFieldRename
import static graphql.schema.diffing.ana.SchemaDifference.InputObjectFieldTypeModification
import static graphql.schema.diffing.ana.SchemaDifference.InputObjectModification
import static graphql.schema.diffing.ana.SchemaDifference.InterfaceAddition
import static graphql.schema.diffing.ana.SchemaDifference.InterfaceDeletion
import static graphql.schema.diffing.ana.SchemaDifference.InterfaceFieldAddition
import static graphql.schema.diffing.ana.SchemaDifference.InterfaceFieldArgumentAddition
import static graphql.schema.diffing.ana.SchemaDifference.InterfaceFieldArgumentDefaultValueModification
Expand Down Expand Up @@ -1917,7 +1918,6 @@ class EditOperationAnalyzerTest extends Specification {
when:
def changes = calcDiff(oldSdl, newSdl)
then:
true
changes.inputObjectDifferences["Echo"] instanceof InputObjectModification
def diff = changes.inputObjectDifferences["Echo"] as InputObjectModification

Expand All @@ -1937,6 +1937,104 @@ class EditOperationAnalyzerTest extends Specification {
directiveDeletion[0].name == "d"
}

def "object field description changed"() {
given:
def oldSdl = '''
type Query {
" Hello"
echo: String
}
'''
def newSdl = '''
type Query {
"Test "
echo: String
}
'''

when:
def changes = calcDiff(oldSdl, newSdl)

then:
// no changes
changes.objectDifferences["Query"] == null
}

def "interface field description changed"() {
given:
def oldSdl = '''
type Query {
node: Node
}
interface Node {
" Hello"
echo: String
}
'''
def newSdl = '''
type Query {
node: Node
}
interface Node {
"World"
echo: String
}
'''

when:
def changes = calcDiff(oldSdl, newSdl)

then:
// no changes
changes.interfaceDifferences["Node"] == null
}

def "interface deleted with field argument"() {
given:
def oldSdl = '''
type Query {
node: Node
}
interface Node {
echo(test: String): String
}
'''
def newSdl = '''
type Query {
node: ID
}
'''

when:
def changes = calcDiff(oldSdl, newSdl)

then:
changes.interfaceDifferences["Node"] instanceof InterfaceDeletion
}

def "object deleted with field argument"() {
given:
def oldSdl = '''
type Query {
node: Node
}
type Node {
echo(test: String): String
}
'''
def newSdl = '''
type Query {
node: ID
}
'''

when:
def changes = calcDiff(oldSdl, newSdl)

then:
changes.objectDifferences["Node"] instanceof ObjectDeletion
}

EditOperationAnalysisResult calcDiff(
String oldSdl,
String newSdl
Expand Down