Skip to content

Commit 426a2cc

Browse files
andimarekandimarek
authored
Merge pull request #4287 from graphql-java/claude/fix-graphql-java-4278-v6Z3y
Fix findPubliclyAccessibleMethod to search interfaces for accessible methods
2 parents 7dac6ab + e718c23 commit 426a2cc

3 files changed

Lines changed: 296 additions & 0 deletions

File tree

‎src/main/java/graphql/schema/PropertyFetchingImpl.java‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -244,12 +244,51 @@ private Method findPubliclyAccessibleMethod(CacheKey cacheKey, Class<?> rootClas
244244
return method;
245245
}
246246
}
247+
// Check public interfaces implemented by this class (handles non-public classes
248+
// like TreeMap.Entry that implement public interfaces like Map.Entry)
249+
Method method = findMethodOnPublicInterfaces(cacheKey, currentClass.getInterfaces(), methodName, dfeInUse, allowStaticMethods);
250+
if (method != null) {
251+
return method;
252+
}
247253
currentClass = currentClass.getSuperclass();
248254
}
249255
assert rootClass != null;
250256
return rootClass.getMethod(methodName);
251257
}
252258

259+
private Method findMethodOnPublicInterfaces(CacheKey cacheKey, Class<?>[] interfaces, String methodName, boolean dfeInUse, boolean allowStaticMethods) {
260+
for (Class<?> iface : interfaces) {
261+
if (Modifier.isPublic(iface.getModifiers())) {
262+
if (dfeInUse) {
263+
try {
264+
Method method = iface.getMethod(methodName, singleArgumentType);
265+
if (isSuitablePublicMethod(method, allowStaticMethods)) {
266+
METHOD_CACHE.putIfAbsent(cacheKey, new CachedMethod(method));
267+
return method;
268+
}
269+
} catch (NoSuchMethodException e) {
270+
// ok try the next approach
271+
}
272+
}
273+
try {
274+
Method method = iface.getMethod(methodName);
275+
if (isSuitablePublicMethod(method, allowStaticMethods)) {
276+
METHOD_CACHE.putIfAbsent(cacheKey, new CachedMethod(method));
277+
return method;
278+
}
279+
} catch (NoSuchMethodException e) {
280+
// continue searching
281+
}
282+
}
283+
// Also search super-interfaces of non-public interfaces
284+
Method method = findMethodOnPublicInterfaces(cacheKey, iface.getInterfaces(), methodName, dfeInUse, allowStaticMethods);
285+
if (method != null) {
286+
return method;
287+
}
288+
}
289+
return null;
290+
}
291+
253292
private boolean isSuitablePublicMethod(Method method, boolean allowStaticMethods) {
254293
int methodModifiers = method.getModifiers();
255294
if (Modifier.isPublic(methodModifiers)) {

‎src/test/groovy/graphql/schema/PropertyDataFetcherTest.groovy‎

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import graphql.schema.fetching.ConfusedPojo
77
import graphql.schema.somepackage.ClassWithDFEMethods
88
import graphql.schema.somepackage.ClassWithInterfaces
99
import graphql.schema.somepackage.ClassWithInteritanceAndInterfaces
10+
import graphql.schema.somepackage.InterfaceInheritanceHolder
1011
import graphql.schema.somepackage.RecordLikeClass
1112
import graphql.schema.somepackage.RecordLikeTwoClassesDown
1213
import graphql.schema.somepackage.TestClass
@@ -788,6 +789,124 @@ class PropertyDataFetcherTest extends Specification {
788789

789790
class OtherObject extends BaseObject {}
790791

792+
def "fetch via public interface method on non-public class - issue 4278"() {
793+
given:
794+
// TreeMap.Entry is a package-private class implementing the public Map.Entry interface
795+
// On Java 16+, setAccessible fails on JDK internal classes, so the only way to invoke
796+
// getValue() is by finding it through the public Map.Entry interface
797+
PropertyDataFetcherHelper.setUseLambdaFactory(false)
798+
PropertyDataFetcher.clearReflectionCache()
799+
800+
def treeMap = new TreeMap<String, String>()
801+
treeMap.put("testKey", "testValue")
802+
def entry = treeMap.entrySet().iterator().next()
803+
def environment = env("value", entry)
804+
805+
when:
806+
def result = fetcher.get(environment)
807+
808+
then:
809+
result == "testValue"
810+
811+
where:
812+
fetcher | _
813+
new PropertyDataFetcher("value") | _
814+
SingletonPropertyDataFetcher.singleton() | _
815+
}
816+
817+
def "fetch via public interface method on non-public class for key - issue 4278"() {
818+
given:
819+
PropertyDataFetcherHelper.setUseLambdaFactory(false)
820+
PropertyDataFetcher.clearReflectionCache()
821+
822+
def treeMap = new TreeMap<String, String>()
823+
treeMap.put("testKey", "testValue")
824+
def entry = treeMap.entrySet().iterator().next()
825+
def environment = env("key", entry)
826+
827+
when:
828+
def result = fetcher.get(environment)
829+
830+
then:
831+
result == "testKey"
832+
833+
where:
834+
fetcher | _
835+
new PropertyDataFetcher("key") | _
836+
SingletonPropertyDataFetcher.singleton() | _
837+
}
838+
839+
def "fetch method from public interface through package-private interface chain"() {
840+
given:
841+
// PackagePrivateChainImpl (package-private) implements PackagePrivateMiddleInterface (package-private)
842+
// which extends PublicBaseInterface (public) — defines getBaseValue()
843+
// The recursive interface search must traverse through the package-private middle interface
844+
PropertyDataFetcherHelper.setUseLambdaFactory(false)
845+
PropertyDataFetcher.clearReflectionCache()
846+
847+
def obj = InterfaceInheritanceHolder.createChainImpl()
848+
def environment = env("baseValue", obj)
849+
850+
when:
851+
def result = new PropertyDataFetcher("baseValue").get(environment)
852+
853+
then:
854+
result == "baseValue"
855+
}
856+
857+
def "fetch method through diamond interface inheritance"() {
858+
given:
859+
// DiamondImpl (package-private) implements both PackagePrivateBranchA and PackagePrivateBranchB
860+
// Both are package-private interfaces extending PublicBaseInterface (public) — defines getBaseValue()
861+
// The search must find getBaseValue() through either branch
862+
PropertyDataFetcherHelper.setUseLambdaFactory(false)
863+
PropertyDataFetcher.clearReflectionCache()
864+
865+
def obj = InterfaceInheritanceHolder.createDiamondImpl()
866+
867+
expect:
868+
new PropertyDataFetcher(property).get(env(property, obj)) == expected
869+
870+
where:
871+
property | expected
872+
"baseValue" | "diamondBaseValue"
873+
}
874+
875+
def "fetch via public interface method with DataFetchingEnvironment parameter on non-public class"() {
876+
given:
877+
// PackagePrivateDfeImpl implements PublicDfeInterface which declares getDfeValue(DataFetchingEnvironment)
878+
// This exercises the dfeInUse path in findMethodOnPublicInterfaces (lines 262-267)
879+
PropertyDataFetcherHelper.setUseLambdaFactory(false)
880+
PropertyDataFetcher.clearReflectionCache()
881+
882+
def obj = InterfaceInheritanceHolder.createDfeImpl()
883+
def environment = env("dfeValue", obj)
884+
885+
when:
886+
def result = new PropertyDataFetcher("dfeValue").get(environment)
887+
888+
then:
889+
result == "dfeValue"
890+
}
891+
892+
def "fetch via interface search hits NoSuchMethodException and continues to next interface"() {
893+
given:
894+
// PackagePrivateMultiInterfaceImpl implements PublicInterfaceWithoutTarget (no getBaseValue)
895+
// and PublicBaseInterface (has getBaseValue). The search must hit NoSuchMethodException
896+
// on the first interface and continue to find it on the second.
897+
PropertyDataFetcherHelper.setUseLambdaFactory(false)
898+
PropertyDataFetcher.clearReflectionCache()
899+
900+
def obj = InterfaceInheritanceHolder.createMultiInterfaceImpl()
901+
def environment = env("baseValue", obj)
902+
903+
when:
904+
def result = new PropertyDataFetcher("baseValue").get(environment)
905+
906+
then:
907+
result == "foundViaSecondInterface"
908+
}
909+
791910
def "Can access private property from base class that starts with i in Turkish"() {
792911
// see https://github.com/graphql-java/graphql-java/issues/3385
793912
given:

‎src/test/groovy/graphql/schema/somepackage/InterfaceInheritanceHolder.java‎

Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,138 @@
1+
package graphql.schema.somepackage;
2+
3+
import graphql.schema.DataFetchingEnvironment;
4+
5+
/**
6+
* Test fixtures for interface-extends-interface method resolution.
7+
* <p>
8+
* Tests the recursive interface search in findMethodOnPublicInterfaces:
9+
* a package-private class implements a package-private interface that extends
10+
* a public interface. The method is only declared on the public grandparent,
11+
* so the algorithm must recursively traverse through the package-private
12+
* interface to find it.
13+
* <p>
14+
* Linear chain:
15+
* <pre>
16+
* PublicBaseInterface (public) — defines getBaseValue()
17+
* |
18+
* PackagePrivateMiddleInterface — extends PublicBaseInterface (adds nothing new)
19+
* |
20+
* PackagePrivateChainImpl (package-private) — implements PackagePrivateMiddleInterface
21+
* </pre>
22+
* <p>
23+
* Diamond pattern:
24+
* <pre>
25+
* PublicBaseInterface (public) — defines getBaseValue()
26+
* / \
27+
* PackagePrivateBranchA PackagePrivateBranchB — each adds own method
28+
* \ /
29+
* DiamondImpl (package-private)
30+
* </pre>
31+
*/
32+
public class InterfaceInheritanceHolder {
33+
34+
// --- Linear interface chain ---
35+
36+
public interface PublicBaseInterface {
37+
String getBaseValue();
38+
}
39+
40+
// Package-private interface extending a public interface — adds no new methods
41+
interface PackagePrivateMiddleInterface extends PublicBaseInterface {
42+
}
43+
44+
// Package-private class: only implements the package-private interface.
45+
// getBaseValue() is declared on PublicBaseInterface — the recursive search
46+
// must traverse PackagePrivateMiddleInterface -> PublicBaseInterface to find it.
47+
static class PackagePrivateChainImpl implements PackagePrivateMiddleInterface {
48+
@Override
49+
public String getBaseValue() {
50+
return "baseValue";
51+
}
52+
}
53+
54+
// --- Diamond pattern ---
55+
56+
// Two package-private interfaces both extending PublicBaseInterface
57+
interface PackagePrivateBranchA extends PublicBaseInterface {
58+
String getBranchAValue();
59+
}
60+
61+
interface PackagePrivateBranchB extends PublicBaseInterface {
62+
String getBranchBValue();
63+
}
64+
65+
// Package-private class implementing both branches (diamond).
66+
// getBaseValue() is only on PublicBaseInterface — must be found through either branch.
67+
static class DiamondImpl implements PackagePrivateBranchA, PackagePrivateBranchB {
68+
@Override
69+
public String getBaseValue() {
70+
return "diamondBaseValue";
71+
}
72+
73+
@Override
74+
public String getBranchAValue() {
75+
return "branchAValue";
76+
}
77+
78+
@Override
79+
public String getBranchBValue() {
80+
return "branchBValue";
81+
}
82+
}
83+
84+
// --- DFE interface: public interface with a method accepting DataFetchingEnvironment ---
85+
86+
public interface PublicDfeInterface {
87+
String getDfeValue(DataFetchingEnvironment dfe);
88+
}
89+
90+
// Package-private class implementing the public DFE interface.
91+
// Exercises the dfeInUse path in findMethodOnPublicInterfaces.
92+
static class PackagePrivateDfeImpl implements PublicDfeInterface {
93+
@Override
94+
public String getDfeValue(DataFetchingEnvironment dfe) {
95+
return "dfeValue";
96+
}
97+
}
98+
99+
// --- Interface with multiple methods: one exists, one doesn't ---
100+
// Used to exercise the NoSuchMethodException catch path in findMethodOnPublicInterfaces.
101+
102+
public interface PublicInterfaceWithoutTarget {
103+
String getUnrelatedValue();
104+
}
105+
106+
// Package-private class implementing an interface that does NOT have the fetched property.
107+
// Also implements PublicBaseInterface which DOES have it.
108+
// The search hits NoSuchMethodException on PublicInterfaceWithoutTarget, then finds it on PublicBaseInterface.
109+
static class PackagePrivateMultiInterfaceImpl implements PublicInterfaceWithoutTarget, PublicBaseInterface {
110+
@Override
111+
public String getUnrelatedValue() {
112+
return "unrelated";
113+
}
114+
115+
@Override
116+
public String getBaseValue() {
117+
return "foundViaSecondInterface";
118+
}
119+
}
120+
121+
// --- Factory methods (public entry points for tests) ---
122+
123+
public static Object createChainImpl() {
124+
return new PackagePrivateChainImpl();
125+
}
126+
127+
public static Object createDiamondImpl() {
128+
return new DiamondImpl();
129+
}
130+
131+
public static Object createDfeImpl() {
132+
return new PackagePrivateDfeImpl();
133+
}
134+
135+
public static Object createMultiInterfaceImpl() {
136+
return new PackagePrivateMultiInterfaceImpl();
137+
}
138+
}

0 commit comments

Comments
 (0)