Skip to content

Commit 6f42601

Browse files
andimarekandimarek
committed
bugfix: identify same fields across hierarchy correctly
1 parent a9de9ce commit 6f42601

2 files changed

Lines changed: 82 additions & 37 deletions

File tree

‎src/main/java/graphql/util/Anonymizer.java‎

Lines changed: 50 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -61,9 +61,11 @@
6161
import java.util.ArrayList;
6262
import java.util.Collections;
6363
import java.util.LinkedHashMap;
64+
import java.util.LinkedHashSet;
6465
import java.util.List;
6566
import java.util.Map;
6667
import java.util.Optional;
68+
import java.util.Set;
6769
import java.util.concurrent.atomic.AtomicInteger;
6870

6971
import static graphql.Assert.assertNotNull;
@@ -314,7 +316,7 @@ public TraversalControl visitGraphQLArgument(GraphQLArgument graphQLArgument, Tr
314316
GraphQLFieldDefinition fieldDefinition = (GraphQLFieldDefinition) parentNode;
315317
String fieldName = fieldDefinition.getName();
316318
GraphQLImplementingType implementingType = (GraphQLImplementingType) context.getParentContext().getParentNode();
317-
List<GraphQLFieldDefinition> matchingInterfaceFieldDefinitions = getMatchingInterfaceFieldDefinitions(fieldName, implementingType.getInterfaces());
319+
Set<GraphQLFieldDefinition> matchingInterfaceFieldDefinitions = getSameFields(fieldName, implementingType.getName(), interfaceToImplementations, schema);
318320
String newName;
319321
if (matchingInterfaceFieldDefinitions.size() == 0) {
320322
newName = "argument" + argumentCounter.getAndIncrement();
@@ -367,17 +369,15 @@ public TraversalControl visitGraphQLEnumValueDefinition(GraphQLEnumValueDefiniti
367369

368370
@Override
369371
public TraversalControl visitGraphQLFieldDefinition(GraphQLFieldDefinition graphQLFieldDefinition, TraverserContext<GraphQLSchemaElement> context) {
370-
String curName = graphQLFieldDefinition.getName();
372+
String fieldName = graphQLFieldDefinition.getName();
371373
GraphQLImplementingType parentNode = (GraphQLImplementingType) context.getParentNode();
372-
List<GraphQLNamedOutputType> interfaces = parentNode.getInterfaces();
373-
List<GraphQLFieldDefinition> sameFields = getMatchingInterfaceFieldDefinitions(curName, interfaces);
374-
sameFields.addAll(getMatchingImplementationsFieldDefinitions(parentNode.getName(), curName, interfaceToImplementations));
374+
Set<GraphQLFieldDefinition> sameFields = getSameFields(fieldName, parentNode.getName(), interfaceToImplementations, schema);
375375
String newName;
376376
if (sameFields.size() == 0) {
377377
newName = "field" + fieldCounter.getAndIncrement();
378378
} else {
379-
if (newNameMap.containsKey(sameFields.get(0))) {
380-
newName = newNameMap.get(sameFields.get(0));
379+
if (newNameMap.containsKey(sameFields.iterator().next())) {
380+
newName = newNameMap.get(sameFields.iterator().next());
381381
} else {
382382
newName = "field" + fieldCounter.getAndIncrement();
383383
for (GraphQLFieldDefinition fieldDefinition : sameFields) {
@@ -453,39 +453,62 @@ public TraversalControl visitGraphQLUnionType(GraphQLUnionType graphQLUnionType,
453453
return newNameMap;
454454
}
455455

456-
private static List<GraphQLFieldDefinition> getMatchingInterfaceFieldDefinitions(
457-
String curName,
458-
List<GraphQLNamedOutputType> interfaces) {
459-
List<GraphQLFieldDefinition> matchingInterfaceFieldDefinitions = new ArrayList<>();
460-
for (GraphQLNamedOutputType iface : interfaces) {
461-
GraphQLInterfaceType interfaceType = (GraphQLInterfaceType) iface;
462-
if (interfaceType.getFieldDefinition(curName) != null) {
463-
matchingInterfaceFieldDefinitions.add(interfaceType.getFieldDefinition(curName));
464-
}
465-
}
466-
return matchingInterfaceFieldDefinitions;
456+
private static Set<GraphQLFieldDefinition> getSameFields(String fieldName,
457+
String objectOrInterfaceName,
458+
Map<String, List<GraphQLImplementingType>> interfaceToImplementations,
459+
GraphQLSchema schema
460+
) {
461+
Set<GraphQLFieldDefinition> result = new LinkedHashSet<>();
462+
Set<String> alreadyChecked = new LinkedHashSet<>();
463+
getSameFieldsImpl(fieldName, objectOrInterfaceName, interfaceToImplementations, schema, alreadyChecked, result);
464+
return result;
467465
}
468466

469-
private static List<GraphQLFieldDefinition> getMatchingImplementationsFieldDefinitions(
470-
String interfaceName,
471-
String fieldName,
472-
Map<String, List<GraphQLImplementingType>> interfaceToImplementations) {
473-
List<GraphQLFieldDefinition> result = new ArrayList<>();
474-
List<GraphQLImplementingType> implementations = interfaceToImplementations.get(interfaceName);
467+
private static void getSameFieldsImpl(String fieldName,
468+
String curObjectOrInterface,
469+
Map<String, List<GraphQLImplementingType>> interfaceToImplementations,
470+
GraphQLSchema schema,
471+
Set<String> alreadyChecked,
472+
Set<GraphQLFieldDefinition> result) {
473+
if (alreadyChecked.contains(curObjectOrInterface)) {
474+
return;
475+
}
476+
alreadyChecked.add(curObjectOrInterface);
477+
478+
// "up": get all Interfaces
479+
GraphQLImplementingType type = (GraphQLImplementingType) schema.getType(curObjectOrInterface);
480+
List<GraphQLNamedOutputType> interfaces = type.getInterfaces();
481+
getMatchingFieldDefinitions(fieldName, interfaces, result);
482+
for (GraphQLNamedOutputType interfaze : interfaces) {
483+
getSameFieldsImpl(fieldName, interfaze.getName(), interfaceToImplementations, schema, alreadyChecked, result);
484+
}
485+
486+
// "down": get all Object or Interfaces
487+
List<GraphQLImplementingType> implementations = interfaceToImplementations.get(curObjectOrInterface);
475488
if (implementations == null) {
476-
return Collections.emptyList();
489+
return;
477490
}
491+
getMatchingFieldDefinitions(fieldName, implementations, result);
478492
for (GraphQLImplementingType implementingType : implementations) {
493+
getSameFieldsImpl(fieldName, implementingType.getName(), interfaceToImplementations, schema, alreadyChecked, result);
494+
}
495+
}
496+
497+
private static void getMatchingFieldDefinitions(
498+
String fieldName,
499+
List<? extends GraphQLType> interfaces,
500+
Set<GraphQLFieldDefinition> result) {
501+
for (GraphQLType iface : interfaces) {
502+
GraphQLImplementingType implementingType = (GraphQLImplementingType) iface;
479503
if (implementingType.getFieldDefinition(fieldName) != null) {
480504
result.add(implementingType.getFieldDefinition(fieldName));
481505
}
482506
}
483-
return result;
484507
}
485508

486509
private static List<GraphQLArgument> getMatchingArgumentDefinitions(
487510
String name,
488-
List<GraphQLFieldDefinition> fieldDefinitions) {
511+
Set<GraphQLFieldDefinition> fieldDefinitions) {
489512
List<GraphQLArgument> result = new ArrayList<>();
490513
for (GraphQLFieldDefinition fieldDefinition : fieldDefinitions) {
491514
Optional.ofNullable(fieldDefinition.getArgument(name)).map(result::add);

‎src/test/groovy/graphql/util/AnonymizerTest.groovy‎

Lines changed: 32 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -545,23 +545,33 @@ type Object1 {
545545
def "same field across hierarchy"() {
546546
def schema = TestUtil.schema("""
547547
type Query {
548-
foo: Object
548+
foo: Interface2
549549
}
550550
interface Interface1 implements Interface2 & Interface3 {
551551
id: ID!
552552
}
553-
553+
interface Interface4 implements Interface1 & Interface2 & Interface3 {
554+
id: ID!
555+
}
556+
interface Interface5 implements Interface1 & Interface2 & Interface3 & Interface6{
557+
id: ID!
558+
}
554559
interface Interface2 {
555560
id: ID!
556561
}
557562
558563
interface Interface3 {
559564
id: ID!
560565
}
561-
type Object implements Interface1 & Interface2 & Interface3 {
566+
interface Interface6 {
562567
id: ID!
563568
}
564569
570+
interface Interface7 implements Interface6 {
571+
id: ID!
572+
}
573+
574+
565575
""")
566576
when:
567577
def result = Anonymizer.anonymizeSchema(schema)
@@ -573,23 +583,35 @@ type Object implements Interface1 & Interface2 & Interface3 {
573583
}
574584
575585
interface Interface1 implements Interface2 & Interface3 {
576-
field2: ID!
586+
field1: ID!
577587
}
578588
579589
interface Interface2 {
580-
field2: ID!
590+
field1: ID!
581591
}
582592
583593
interface Interface3 {
584-
field2: ID!
594+
field1: ID!
585595
}
586596
587-
type Object1 {
588-
field1: Object2
597+
interface Interface4 implements Interface1 & Interface2 & Interface3 {
598+
field1: ID!
589599
}
590600
591-
type Object2 implements Interface1 & Interface2 & Interface3 {
592-
field2: ID!
601+
interface Interface5 implements Interface1 & Interface2 & Interface3 & Interface6 {
602+
field1: ID!
603+
}
604+
605+
interface Interface6 {
606+
field1: ID!
607+
}
608+
609+
interface Interface7 implements Interface6 {
610+
field1: ID!
611+
}
612+
613+
type Object1 {
614+
field2: Interface2
593615
}
594616
"""
595617
}

0 commit comments

Comments
 (0)