X Tutup
Skip to content

Correct diff when argument is "moved" and the type is changed #3156

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 31, 2023
Merged

Correct diff when argument is "moved" and the type is changed #3156

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
e0303dc
Correct diff when argument is "moved" and the type is changed
gnawf Mar 30, 2023
3210fbc
Fix cases where argument was already added or deleted
gnawf Mar 30, 2023
ae2dd10
Fix edge case for argument type modification on renamed container
gnawf Mar 31, 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
115 changes: 93 additions & 22 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 @@ -106,19 +106,19 @@
@Internal
public class EditOperationAnalyzer {

private GraphQLSchema oldSchema;
private GraphQLSchema newSchema;
private SchemaGraph oldSchemaGraph;
private SchemaGraph newSchemaGraph;
private final GraphQLSchema oldSchema;
private final GraphQLSchema newSchema;
private final SchemaGraph oldSchemaGraph;
private final SchemaGraph newSchemaGraph;

private Map<String, ObjectDifference> objectDifferences = new LinkedHashMap<>();
private Map<String, InterfaceDifference> interfaceDifferences = new LinkedHashMap<>();
private Map<String, UnionDifference> unionDifferences = new LinkedHashMap<>();
private Map<String, EnumDifference> enumDifferences = new LinkedHashMap<>();
private Map<String, InputObjectDifference> inputObjectDifferences = new LinkedHashMap<>();
private Map<String, ScalarDifference> scalarDifferences = new LinkedHashMap<>();
private final Map<String, ObjectDifference> objectDifferences = new LinkedHashMap<>();
private final Map<String, InterfaceDifference> interfaceDifferences = new LinkedHashMap<>();
private final Map<String, UnionDifference> unionDifferences = new LinkedHashMap<>();
private final Map<String, EnumDifference> enumDifferences = new LinkedHashMap<>();
private final Map<String, InputObjectDifference> inputObjectDifferences = new LinkedHashMap<>();
private final Map<String, ScalarDifference> scalarDifferences = new LinkedHashMap<>();

private Map<String, DirectiveDifference> directiveDifferences = new LinkedHashMap<>();
private final Map<String, DirectiveDifference> directiveDifferences = new LinkedHashMap<>();

public EditOperationAnalyzer(GraphQLSchema oldSchema,
GraphQLSchema newSchema,
Expand Down Expand Up @@ -169,6 +169,7 @@ public EditOperationAnalysisResult analyzeEdits(List<EditOperation> editOperatio
handleUnionMemberChanges(editOperations, mapping);
handleEnumValuesChanges(editOperations, mapping);
handleAppliedDirectives(editOperations, mapping);
handleArgumentChanges(editOperations, mapping);

return new EditOperationAnalysisResult(
objectDifferences,
Expand All @@ -180,6 +181,23 @@ public EditOperationAnalysisResult analyzeEdits(List<EditOperation> editOperatio
directiveDifferences);
}

private void handleArgumentChanges(List<EditOperation> editOperations, Mapping mapping) {
for (EditOperation editOperation : editOperations) {
switch (editOperation.getOperation()) {
case INSERT_EDGE:
if (editOperation.getTargetEdge().getTo().isOfType(SchemaGraph.ARGUMENT)) {
argumentAdded(editOperation);
}
break;
case DELETE_EDGE:
if (editOperation.getSourceEdge().getTo().isOfType(SchemaGraph.ARGUMENT)) {
argumentDeleted(editOperation);
}
break;
}
}
}


private void handleAppliedDirectives(List<EditOperation> editOperations, Mapping mapping) {

Expand Down Expand Up @@ -580,7 +598,7 @@ private void handleTypeChanges(List<EditOperation> editOperations, Mapping mappi
break;
case CHANGE_EDGE:
if (newEdge.getLabel().startsWith("type=")) {
typeEdgeChanged(editOperation);
typeEdgeChanged(editOperation, mapping);
}
break;
}
Expand Down Expand Up @@ -1087,13 +1105,13 @@ private EditOperation findDeletedEdge(Vertex targetVertexFrom,
}


private void typeEdgeChanged(EditOperation editOperation) {
private void typeEdgeChanged(EditOperation editOperation, Mapping mapping) {
Edge targetEdge = editOperation.getTargetEdge();
Vertex from = targetEdge.getFrom();
if (from.isOfType(SchemaGraph.FIELD)) {
outputFieldTypeChanged(editOperation);
} else if (from.isOfType(SchemaGraph.ARGUMENT)) {
argumentTypeOrDefaultValueChanged(editOperation);
argumentTypeOrDefaultValueChanged(editOperation, mapping);
} else if (from.isOfType(SchemaGraph.INPUT_FIELD)) {
inputFieldTypeOrDefaultValueChanged(editOperation);
}
Expand All @@ -1117,7 +1135,11 @@ private void inputFieldTypeOrDefaultValueChanged(EditOperation editOperation) {
}
}

private void argumentTypeOrDefaultValueChanged(EditOperation editOperation) {
private void argumentTypeOrDefaultValueChanged(EditOperation editOperation, Mapping mapping) {
if (!doesArgumentChangeMakeSense(editOperation, mapping)) {
return;
}

Edge targetEdge = editOperation.getTargetEdge();
Vertex argument = targetEdge.getFrom();
Vertex fieldOrDirective = newSchemaGraph.getFieldOrDirectiveForArgument(argument);
Expand Down Expand Up @@ -1158,7 +1180,6 @@ private void argumentTypeOrDefaultValueChanged(EditOperation editOperation) {
getInterfaceModification(objectOrInterface.getName()).getDetails().add(interfaceFieldArgumentTypeModification);
}
}

} else {
assertTrue(fieldOrDirective.isOfType(SchemaGraph.DIRECTIVE));
Vertex directive = fieldOrDirective;
Expand All @@ -1168,16 +1189,29 @@ private void argumentTypeOrDefaultValueChanged(EditOperation editOperation) {
if (!oldDefaultValue.equals(newDefaultValue)) {
getDirectiveModification(directive.getName()).getDetails().add(new DirectiveArgumentDefaultValueModification(argument.getName(), oldDefaultValue, newDefaultValue));
}

String oldType = getTypeFromEdgeLabel(editOperation.getSourceEdge());
String newType = getTypeFromEdgeLabel(editOperation.getTargetEdge());

if (!oldType.equals(newType)) {
getDirectiveModification(directive.getName()).getDetails().add(new DirectiveArgumentTypeModification(argument.getName(), oldType, newType));

}
}
}

/**
* Sometimes the diffing algorithm will give us an argument change when the argument container
* changed i.e. the argument was "moved" around because the deleted and newly added arguments
* look similar.
* <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) {
// 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());

// Make sure the container is the same
return mapping.getTarget(oldContainer) == newContainer;
}

private void outputFieldTypeChanged(EditOperation editOperation) {
Expand All @@ -1199,10 +1233,7 @@ private void outputFieldTypeChanged(EditOperation editOperation) {
String oldType = getTypeFromEdgeLabel(editOperation.getSourceEdge());
String newType = getTypeFromEdgeLabel(editOperation.getTargetEdge());
interfaceModification.getDetails().add(new InterfaceFieldTypeModification(fieldName, oldType, newType));

}


}

// TODO: this is not great, we should avoid parsing the label like that
Expand Down Expand Up @@ -1690,7 +1721,13 @@ private void deletedDirective(EditOperation editOperation) {
}

private void argumentDeleted(EditOperation editOperation) {
// Note: sometimes the edit operation is the argument vertex itself being deleted
// Other times, it is the edge to the argument type being deleted
Vertex deletedArgument = editOperation.getSourceVertex();
if (deletedArgument == null) {
deletedArgument = editOperation.getSourceEdge().getTo();
}

Vertex fieldOrDirective = oldSchemaGraph.getFieldOrDirectiveForArgument(deletedArgument);
if (fieldOrDirective.isOfType(SchemaGraph.FIELD)) {
Vertex field = fieldOrDirective;
Expand All @@ -1700,13 +1737,25 @@ private void argumentDeleted(EditOperation editOperation) {
if (isObjectDeleted(object.getName())) {
return;
}
if (isFieldDeletedFromExistingObject(object.getName(), field.getName())) {
return;
}
if (isArgumentDeletedFromExistingObjectField(object.getName(), field.getName(), deletedArgument.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;
}
if (isFieldDeletedFromExistingInterface(interfaze.getName(), field.getName())) {
return;
}
if (isArgumentDeletedFromExistingInterfaceField(interfaze.getName(), field.getName(), deletedArgument.getName())) {
return;
}
getInterfaceModification(interfaze.getName()).getDetails().add(new InterfaceFieldArgumentDeletion(field.getName(), deletedArgument.getName()));
}
} else {
Expand All @@ -1715,14 +1764,21 @@ private void argumentDeleted(EditOperation editOperation) {
if (isDirectiveDeleted(directive.getName())) {
return;
}
if (isArgumentDeletedFromExistingDirective(directive.getName(), deletedArgument.getName())) {
return;
}
getDirectiveModification(directive.getName()).getDetails().add(new DirectiveArgumentDeletion(deletedArgument.getName()));
}

}

private void argumentAdded(EditOperation editOperation) {
Vertex addedArgument = editOperation.getTargetVertex();
if (addedArgument == null) {
addedArgument = editOperation.getTargetEdge().getTo();
}

Vertex fieldOrDirective = newSchemaGraph.getFieldOrDirectiveForArgument(addedArgument);

if (fieldOrDirective.isOfType(SchemaGraph.FIELD)) {
Vertex field = fieldOrDirective;
Vertex fieldsContainerForField = newSchemaGraph.getFieldsContainerForField(field);
Expand All @@ -1731,13 +1787,25 @@ private void argumentAdded(EditOperation editOperation) {
if (isObjectAdded(object.getName())) {
return;
}
if (isFieldNewForExistingObject(object.getName(), field.getName())) {
return;
}
if (isArgumentNewForExistingObjectField(object.getName(), field.getName(), addedArgument.getName())) {
return;
}
getObjectModification(object.getName()).getDetails().add(new ObjectFieldArgumentAddition(field.getName(), addedArgument.getName()));
} else {
assertTrue(fieldsContainerForField.isOfType(SchemaGraph.INTERFACE));
Vertex interfaze = fieldsContainerForField;
if (isInterfaceAdded(interfaze.getName())) {
return;
}
if (isFieldNewForExistingInterface(interfaze.getName(), field.getName())) {
return;
}
if (isArgumentNewForExistingInterfaceField(interfaze.getName(), field.getName(), addedArgument.getName())) {
return;
}
getInterfaceModification(interfaze.getName()).getDetails().add(new InterfaceFieldArgumentAddition(field.getName(), addedArgument.getName()));
}
} else {
Expand All @@ -1746,6 +1814,9 @@ private void argumentAdded(EditOperation editOperation) {
if (isDirectiveAdded(directive.getName())) {
return;
}
if (isArgumentNewForExistingDirective(directive.getName(), addedArgument.getName())) {
return;
}
getDirectiveModification(directive.getName()).getDetails().add(new DirectiveArgumentAddition(addedArgument.getName()));
}
}
Expand Down
Loading
X Tutup