This is an automated email from the ASF dual-hosted git repository.
rombert pushed a commit to branch master
in repository
https://gitbox.apache.org/repos/asf/sling-org-apache-sling-graphql-core.git
The following commit(s) were added to refs/heads/master by this push:
new f7b4826 SLING-13152 Fix SelectedFieldWrapper overwriting sub-field
selections for aliased duplicate fields (#45)
f7b4826 is described below
commit f7b4826f33ade8c55e624f49e8774fd1c84cfcbd
Author: Martin Schulze <[email protected]>
AuthorDate: Tue Mar 31 16:09:40 2026 +0200
SLING-13152 Fix SelectedFieldWrapper overwriting sub-field selections for
aliased duplicate fields (#45)
---------
Co-authored-by: Martin Schulze <[email protected]>
---
.../graphql/core/engine/SelectedFieldWrapper.java | 35 ++++-
.../graphql/core/engine/SelectionSetWrapper.java | 7 +
.../core/engine/SelectedFieldWrapperTest.java | 157 +++++++++++++++++++++
3 files changed, 196 insertions(+), 3 deletions(-)
diff --git
a/src/main/java/org/apache/sling/graphql/core/engine/SelectedFieldWrapper.java
b/src/main/java/org/apache/sling/graphql/core/engine/SelectedFieldWrapper.java
index 15399ee..9f399fb 100644
---
a/src/main/java/org/apache/sling/graphql/core/engine/SelectedFieldWrapper.java
+++
b/src/main/java/org/apache/sling/graphql/core/engine/SelectedFieldWrapper.java
@@ -36,7 +36,9 @@ import java.util.Map;
/**
* Implement a wrapper for GraphQL SelectedField.
*
- * ATTENTION: here we are assuming that fields added are unqiue by the Fully
Qualified Name (FQN).
+ * ATTENTION: here we are assuming that fields added are unique by the Fully
Qualified Name (FQN).
+ * When multiple children share the same FQN (e.g. aliased selections with
different inline fragments),
+ * their sub-fields are merged into a single wrapper rather than kept as
separate entries.
*
* This updated version is keeping duplicate fields by field's simple name.
Use getFirstSubSelectedFieldByName() if
* you are sure that there is only one field with that simple name otherwise
use hasDuplicateFieldByName() to determine
@@ -69,14 +71,41 @@ public class SelectedFieldWrapper implements SelectedField {
if (selectionSet != null) {
selectionSet.getImmediateFields().forEach(sf -> {
SelectedFieldWrapper selectedChildField = new
SelectedFieldWrapper(sf);
- subFieldMap.put(sf.getName(), selectedChildField);
- subFQNFieldMap.put(sf.getFullyQualifiedName(),
selectedChildField);
+ String fqn = sf.getFullyQualifiedName();
+ SelectedField existing = subFQNFieldMap.get(fqn);
+ if (existing instanceof SelectedFieldWrapper) {
+ // Merge sub-fields from duplicate entries (e.g. aliased
selections
+ // of the same field with different inline fragments)
+ ((SelectedFieldWrapper)
existing).mergeSubFields(selectedChildField);
+ } else {
+ subFQNFieldMap.put(fqn, selectedChildField);
+ subFieldMap.put(sf.getName(), selectedChildField);
+ }
});
}
// Fields are not taken from the FQN Map to avoid dropping fields with
the same name
subFields = new ArrayList<>(subFQNFieldMap.values());
}
+ /**
+ * Merge sub-fields from another SelectedFieldWrapper into this one.
+ * This handles the case where the same field is selected multiple times
+ * (e.g. via aliases with different inline fragments), and the
sub-selections
+ * need to be combined.
+ */
+ void mergeSubFields(SelectedFieldWrapper other) {
+ for (SelectedField otherSub : other.getSubSelectedFields()) {
+ String fqn = otherSub.getFullyQualifiedName();
+ if (fqn != null) {
+ subFQNFieldMap.computeIfAbsent(fqn, k -> {
+ subFieldMap.put(otherSub.getName(), otherSub);
+ subFields.add(otherSub);
+ return otherSub;
+ });
+ }
+ }
+ }
+
@Override
public String getName() {
return name;
diff --git
a/src/main/java/org/apache/sling/graphql/core/engine/SelectionSetWrapper.java
b/src/main/java/org/apache/sling/graphql/core/engine/SelectionSetWrapper.java
index 0a97453..7e187e8 100644
---
a/src/main/java/org/apache/sling/graphql/core/engine/SelectionSetWrapper.java
+++
b/src/main/java/org/apache/sling/graphql/core/engine/SelectionSetWrapper.java
@@ -49,6 +49,13 @@ public class SelectionSetWrapper implements SelectionSet {
if (!name.contains("/")) {
fields.add(selectedField);
}
+ } else {
+ // Merge sub-fields from duplicate field entries (e.g.
aliased
+ // selections of the same field with different inline
fragments)
+ SelectedField existing = fieldsMap.get(name);
+ if (existing instanceof SelectedFieldWrapper) {
+ ((SelectedFieldWrapper)
existing).mergeSubFields(selectedField);
+ }
}
});
initFlatMap(fields, "");
diff --git
a/src/test/java/org/apache/sling/graphql/core/engine/SelectedFieldWrapperTest.java
b/src/test/java/org/apache/sling/graphql/core/engine/SelectedFieldWrapperTest.java
index 2dc1efe..66f1e53 100644
---
a/src/test/java/org/apache/sling/graphql/core/engine/SelectedFieldWrapperTest.java
+++
b/src/test/java/org/apache/sling/graphql/core/engine/SelectedFieldWrapperTest.java
@@ -179,4 +179,161 @@ public class SelectedFieldWrapperTest {
assertNull("Second Field unexpectedly found by FQN",
targetParent.getSubSelectedField(FIELD_SUB_SIMPLE_NAME_2));
assertNotNull("Second Field not found by FQN",
targetParent.getSubSelectedField(FIELD_SUB_FULLY_QUALIFIED_NAME_2));
}
+
+ /**
+ * Tests the mergeSubFields behavior (SITES-42449): when graphql-java
returns
+ * two immediate children with the same FQN (e.g. two "items" entries from
aliased
+ * inline fragments), their sub-fields must be merged rather than the
first being
+ * overwritten by the second.
+ *
+ * Simulates: { parentList { items { ... on ModelA { fieldA } ... on
ModelB { fieldB } } } }
+ * where graphql-java produces two "items" entries with FQN
"Parent.items", each
+ * carrying different sub-fields (ModelA.fieldA vs ModelB.fieldB).
+ */
+ @Test
+ public void testMergeSubFieldsForDuplicateFQN() {
+ // FQN shared by both "items" entries
+ String itemsFqn = "Parent.items";
+ String itemsName = "items";
+
+ // Sub-fields from the first inline fragment (ModelA)
+ String subFqnA = "ModelA.fieldA";
+ String subNameA = "fieldA";
+ graphql.schema.SelectedField sourceSubA =
mock(graphql.schema.SelectedField.class);
+ doReturn(subNameA).when(sourceSubA).getName();
+ doReturn(subNameA).when(sourceSubA).getQualifiedName();
+ doReturn(subFqnA).when(sourceSubA).getFullyQualifiedName();
+
+ DataFetchingFieldSelectionSet selSetItems1 =
mock(DataFetchingFieldSelectionSet.class);
+
doReturn(Arrays.asList(sourceSubA)).when(selSetItems1).getImmediateFields();
+
+ graphql.schema.SelectedField sourceItems1 =
mock(graphql.schema.SelectedField.class);
+ doReturn(itemsName).when(sourceItems1).getName();
+ doReturn(itemsName).when(sourceItems1).getQualifiedName();
+ doReturn(itemsFqn).when(sourceItems1).getFullyQualifiedName();
+ doReturn(selSetItems1).when(sourceItems1).getSelectionSet();
+
+ // Sub-fields from the second inline fragment (ModelB)
+ String subFqnB = "ModelB.fieldB";
+ String subNameB = "fieldB";
+ graphql.schema.SelectedField sourceSubB =
mock(graphql.schema.SelectedField.class);
+ doReturn(subNameB).when(sourceSubB).getName();
+ doReturn(subNameB).when(sourceSubB).getQualifiedName();
+ doReturn(subFqnB).when(sourceSubB).getFullyQualifiedName();
+
+ DataFetchingFieldSelectionSet selSetItems2 =
mock(DataFetchingFieldSelectionSet.class);
+
doReturn(Arrays.asList(sourceSubB)).when(selSetItems2).getImmediateFields();
+
+ graphql.schema.SelectedField sourceItems2 =
mock(graphql.schema.SelectedField.class);
+ doReturn(itemsName).when(sourceItems2).getName();
+ doReturn(itemsName).when(sourceItems2).getQualifiedName();
+ doReturn(itemsFqn).when(sourceItems2).getFullyQualifiedName();
+ doReturn(selSetItems2).when(sourceItems2).getSelectionSet();
+
+ // Parent field with both "items" entries as immediate children
+ graphql.schema.SelectedField sourceParent =
mock(graphql.schema.SelectedField.class);
+ doReturn("parent").when(sourceParent).getName();
+ doReturn("parent").when(sourceParent).getQualifiedName();
+ doReturn("Query.parent").when(sourceParent).getFullyQualifiedName();
+
+ DataFetchingFieldSelectionSet parentSelSet =
mock(DataFetchingFieldSelectionSet.class);
+ doReturn(Arrays.asList(sourceItems1,
sourceItems2)).when(parentSelSet).getImmediateFields();
+ doReturn(parentSelSet).when(sourceParent).getSelectionSet();
+
+ SelectedFieldWrapper parent = new SelectedFieldWrapper(sourceParent);
+
+ // The parent should have exactly one "items" child (merged, not
duplicated)
+ SelectedField itemsField = parent.getSubSelectedFieldByFQN(itemsFqn);
+ assertNotNull("Items field not found by FQN", itemsField);
+
+ // The merged "items" field should contain sub-fields from BOTH inline
fragments
+ assertTrue("Sub-field from first inline fragment (ModelA.fieldA)
missing",
+ itemsField.hasSubSelectedFieldsByFQN(subFqnA));
+ assertTrue("Sub-field from second inline fragment (ModelB.fieldB)
missing",
+ itemsField.hasSubSelectedFieldsByFQN(subFqnB));
+
+ // Verify by direct lookup
+ SelectedField foundA = itemsField.getSubSelectedFieldByFQN(subFqnA);
+ assertNotNull("fieldA not found by FQN", foundA);
+ assertEquals("Wrong name for fieldA", subNameA, foundA.getName());
+
+ SelectedField foundB = itemsField.getSubSelectedFieldByFQN(subFqnB);
+ assertNotNull("fieldB not found by FQN", foundB);
+ assertEquals("Wrong name for fieldB", subNameB, foundB.getName());
+
+ // Total sub-fields count should be 2 (one from each inline fragment)
+ assertEquals("Expected 2 merged sub-fields", 2,
itemsField.getSubSelectedFields().size());
+ }
+
+ /**
+ * Verifies that when two immediate children share the same FQN (and
simple name),
+ * the merged-away duplicate does NOT remain in subFieldMap.
+ *
+ * Before the fix, subFieldMap.put() ran unconditionally before the FQN
dedup merge,
+ * leaving a stale wrapper in the map. This caused
hasDuplicateFieldByName() to report
+ * a false positive and getSubSelectedFieldByName() to return both the
merged wrapper
+ * and the stale one.
+ */
+ @Test
+ public void testNoDuplicateInSubFieldMapAfterFQNMerge() {
+ String itemsFqn = "Parent.items";
+ String itemsName = "items";
+
+ // First "items" child with one sub-field
+ graphql.schema.SelectedField sourceSubA =
mock(graphql.schema.SelectedField.class);
+ doReturn("fieldA").when(sourceSubA).getName();
+ doReturn("fieldA").when(sourceSubA).getQualifiedName();
+ doReturn("ModelA.fieldA").when(sourceSubA).getFullyQualifiedName();
+
+ DataFetchingFieldSelectionSet selSetItems1 =
mock(DataFetchingFieldSelectionSet.class);
+
doReturn(Arrays.asList(sourceSubA)).when(selSetItems1).getImmediateFields();
+
+ graphql.schema.SelectedField sourceItems1 =
mock(graphql.schema.SelectedField.class);
+ doReturn(itemsName).when(sourceItems1).getName();
+ doReturn(itemsName).when(sourceItems1).getQualifiedName();
+ doReturn(itemsFqn).when(sourceItems1).getFullyQualifiedName();
+ doReturn(selSetItems1).when(sourceItems1).getSelectionSet();
+
+ // Second "items" child (same name + same FQN) with a different
sub-field
+ graphql.schema.SelectedField sourceSubB =
mock(graphql.schema.SelectedField.class);
+ doReturn("fieldB").when(sourceSubB).getName();
+ doReturn("fieldB").when(sourceSubB).getQualifiedName();
+ doReturn("ModelB.fieldB").when(sourceSubB).getFullyQualifiedName();
+
+ DataFetchingFieldSelectionSet selSetItems2 =
mock(DataFetchingFieldSelectionSet.class);
+
doReturn(Arrays.asList(sourceSubB)).when(selSetItems2).getImmediateFields();
+
+ graphql.schema.SelectedField sourceItems2 =
mock(graphql.schema.SelectedField.class);
+ doReturn(itemsName).when(sourceItems2).getName();
+ doReturn(itemsName).when(sourceItems2).getQualifiedName();
+ doReturn(itemsFqn).when(sourceItems2).getFullyQualifiedName();
+ doReturn(selSetItems2).when(sourceItems2).getSelectionSet();
+
+ // Parent with both "items" entries
+ graphql.schema.SelectedField sourceParent =
mock(graphql.schema.SelectedField.class);
+ doReturn("parent").when(sourceParent).getName();
+ doReturn("parent").when(sourceParent).getQualifiedName();
+ doReturn("Query.parent").when(sourceParent).getFullyQualifiedName();
+
+ DataFetchingFieldSelectionSet parentSelSet =
mock(DataFetchingFieldSelectionSet.class);
+ doReturn(Arrays.asList(sourceItems1,
sourceItems2)).when(parentSelSet).getImmediateFields();
+ doReturn(parentSelSet).when(sourceParent).getSelectionSet();
+
+ SelectedFieldWrapper parent = new SelectedFieldWrapper(sourceParent);
+
+ // subFieldMap must contain exactly one entry for "items" (the merged
wrapper),
+ // NOT two (merged + stale)
+ assertFalse("Same-FQN children must not be reported as duplicates by
simple name",
+ parent.hasDuplicateFieldByName(itemsName));
+ Collection<SelectedField> byName =
parent.getSubSelectedFieldByName(itemsName);
+ assertEquals("Expected exactly 1 entry in subFieldMap for same-FQN
children", 1, byName.size());
+
+ // The single entry must be the merged wrapper containing sub-fields
from both fragments
+ SelectedField merged = byName.iterator().next();
+ assertEquals("Expected 2 sub-fields in merged wrapper", 2,
merged.getSubSelectedFields().size());
+ assertTrue("fieldA missing from merged wrapper",
+ merged.hasSubSelectedFieldsByFQN("ModelA.fieldA"));
+ assertTrue("fieldB missing from merged wrapper",
+ merged.hasSubSelectedFieldsByFQN("ModelB.fieldB"));
+ }
}
