diff --git a/src/main/java/graphql/schema/PropertyFetchingImpl.java b/src/main/java/graphql/schema/PropertyFetchingImpl.java index c17ba706b6..f03726b387 100644 --- a/src/main/java/graphql/schema/PropertyFetchingImpl.java +++ b/src/main/java/graphql/schema/PropertyFetchingImpl.java @@ -244,12 +244,51 @@ private Method findPubliclyAccessibleMethod(CacheKey cacheKey, Class rootClas return method; } } + // Check public interfaces implemented by this class (handles non-public classes + // like TreeMap.Entry that implement public interfaces like Map.Entry) + Method method = findMethodOnPublicInterfaces(cacheKey, currentClass.getInterfaces(), methodName, dfeInUse, allowStaticMethods); + if (method != null) { + return method; + } currentClass = currentClass.getSuperclass(); } assert rootClass != null; return rootClass.getMethod(methodName); } + private Method findMethodOnPublicInterfaces(CacheKey cacheKey, Class[] interfaces, String methodName, boolean dfeInUse, boolean allowStaticMethods) { + for (Class iface : interfaces) { + if (Modifier.isPublic(iface.getModifiers())) { + if (dfeInUse) { + try { + Method method = iface.getMethod(methodName, singleArgumentType); + if (isSuitablePublicMethod(method, allowStaticMethods)) { + METHOD_CACHE.putIfAbsent(cacheKey, new CachedMethod(method)); + return method; + } + } catch (NoSuchMethodException e) { + // ok try the next approach + } + } + try { + Method method = iface.getMethod(methodName); + if (isSuitablePublicMethod(method, allowStaticMethods)) { + METHOD_CACHE.putIfAbsent(cacheKey, new CachedMethod(method)); + return method; + } + } catch (NoSuchMethodException e) { + // continue searching + } + } + // Also search super-interfaces of non-public interfaces + Method method = findMethodOnPublicInterfaces(cacheKey, iface.getInterfaces(), methodName, dfeInUse, allowStaticMethods); + if (method != null) { + return method; + } + } + return null; + } + private boolean isSuitablePublicMethod(Method method, boolean allowStaticMethods) { int methodModifiers = method.getModifiers(); if (Modifier.isPublic(methodModifiers)) { diff --git a/src/test/groovy/graphql/schema/PropertyDataFetcherTest.groovy b/src/test/groovy/graphql/schema/PropertyDataFetcherTest.groovy index 7903f14229..d1bdbc94b5 100644 --- a/src/test/groovy/graphql/schema/PropertyDataFetcherTest.groovy +++ b/src/test/groovy/graphql/schema/PropertyDataFetcherTest.groovy @@ -7,6 +7,7 @@ import graphql.schema.fetching.ConfusedPojo import graphql.schema.somepackage.ClassWithDFEMethods import graphql.schema.somepackage.ClassWithInterfaces import graphql.schema.somepackage.ClassWithInteritanceAndInterfaces +import graphql.schema.somepackage.InterfaceInheritanceHolder import graphql.schema.somepackage.RecordLikeClass import graphql.schema.somepackage.RecordLikeTwoClassesDown import graphql.schema.somepackage.TestClass @@ -788,6 +789,124 @@ class PropertyDataFetcherTest extends Specification { class OtherObject extends BaseObject {} + def "fetch via public interface method on non-public class - issue 4278"() { + given: + // TreeMap.Entry is a package-private class implementing the public Map.Entry interface + // On Java 16+, setAccessible fails on JDK internal classes, so the only way to invoke + // getValue() is by finding it through the public Map.Entry interface + PropertyDataFetcherHelper.setUseLambdaFactory(false) + PropertyDataFetcher.clearReflectionCache() + + def treeMap = new TreeMap() + treeMap.put("testKey", "testValue") + def entry = treeMap.entrySet().iterator().next() + def environment = env("value", entry) + + when: + def result = fetcher.get(environment) + + then: + result == "testValue" + + where: + fetcher | _ + new PropertyDataFetcher("value") | _ + SingletonPropertyDataFetcher.singleton() | _ + } + + def "fetch via public interface method on non-public class for key - issue 4278"() { + given: + PropertyDataFetcherHelper.setUseLambdaFactory(false) + PropertyDataFetcher.clearReflectionCache() + + def treeMap = new TreeMap() + treeMap.put("testKey", "testValue") + def entry = treeMap.entrySet().iterator().next() + def environment = env("key", entry) + + when: + def result = fetcher.get(environment) + + then: + result == "testKey" + + where: + fetcher | _ + new PropertyDataFetcher("key") | _ + SingletonPropertyDataFetcher.singleton() | _ + } + + def "fetch method from public interface through package-private interface chain"() { + given: + // PackagePrivateChainImpl (package-private) implements PackagePrivateMiddleInterface (package-private) + // which extends PublicBaseInterface (public) — defines getBaseValue() + // The recursive interface search must traverse through the package-private middle interface + PropertyDataFetcherHelper.setUseLambdaFactory(false) + PropertyDataFetcher.clearReflectionCache() + + def obj = InterfaceInheritanceHolder.createChainImpl() + def environment = env("baseValue", obj) + + when: + def result = new PropertyDataFetcher("baseValue").get(environment) + + then: + result == "baseValue" + } + + def "fetch method through diamond interface inheritance"() { + given: + // DiamondImpl (package-private) implements both PackagePrivateBranchA and PackagePrivateBranchB + // Both are package-private interfaces extending PublicBaseInterface (public) — defines getBaseValue() + // The search must find getBaseValue() through either branch + PropertyDataFetcherHelper.setUseLambdaFactory(false) + PropertyDataFetcher.clearReflectionCache() + + def obj = InterfaceInheritanceHolder.createDiamondImpl() + + expect: + new PropertyDataFetcher(property).get(env(property, obj)) == expected + + where: + property | expected + "baseValue" | "diamondBaseValue" + } + + def "fetch via public interface method with DataFetchingEnvironment parameter on non-public class"() { + given: + // PackagePrivateDfeImpl implements PublicDfeInterface which declares getDfeValue(DataFetchingEnvironment) + // This exercises the dfeInUse path in findMethodOnPublicInterfaces (lines 262-267) + PropertyDataFetcherHelper.setUseLambdaFactory(false) + PropertyDataFetcher.clearReflectionCache() + + def obj = InterfaceInheritanceHolder.createDfeImpl() + def environment = env("dfeValue", obj) + + when: + def result = new PropertyDataFetcher("dfeValue").get(environment) + + then: + result == "dfeValue" + } + + def "fetch via interface search hits NoSuchMethodException and continues to next interface"() { + given: + // PackagePrivateMultiInterfaceImpl implements PublicInterfaceWithoutTarget (no getBaseValue) + // and PublicBaseInterface (has getBaseValue). The search must hit NoSuchMethodException + // on the first interface and continue to find it on the second. + PropertyDataFetcherHelper.setUseLambdaFactory(false) + PropertyDataFetcher.clearReflectionCache() + + def obj = InterfaceInheritanceHolder.createMultiInterfaceImpl() + def environment = env("baseValue", obj) + + when: + def result = new PropertyDataFetcher("baseValue").get(environment) + + then: + result == "foundViaSecondInterface" + } + def "Can access private property from base class that starts with i in Turkish"() { // see https://github.com/graphql-java/graphql-java/issues/3385 given: diff --git a/src/test/groovy/graphql/schema/somepackage/InterfaceInheritanceHolder.java b/src/test/groovy/graphql/schema/somepackage/InterfaceInheritanceHolder.java new file mode 100644 index 0000000000..e5cd61bee0 --- /dev/null +++ b/src/test/groovy/graphql/schema/somepackage/InterfaceInheritanceHolder.java @@ -0,0 +1,138 @@ +package graphql.schema.somepackage; + +import graphql.schema.DataFetchingEnvironment; + +/** + * Test fixtures for interface-extends-interface method resolution. + *

+ * Tests the recursive interface search in findMethodOnPublicInterfaces: + * a package-private class implements a package-private interface that extends + * a public interface. The method is only declared on the public grandparent, + * so the algorithm must recursively traverse through the package-private + * interface to find it. + *

+ * Linear chain: + *

+ *   PublicBaseInterface (public)              — defines getBaseValue()
+ *        |
+ *   PackagePrivateMiddleInterface            — extends PublicBaseInterface (adds nothing new)
+ *        |
+ *   PackagePrivateChainImpl (package-private) — implements PackagePrivateMiddleInterface
+ * 
+ *

+ * Diamond pattern: + *

+ *        PublicBaseInterface (public)          — defines getBaseValue()
+ *             /          \
+ *  PackagePrivateBranchA   PackagePrivateBranchB  — each adds own method
+ *             \          /
+ *        DiamondImpl (package-private)
+ * 
+ */ +public class InterfaceInheritanceHolder { + + // --- Linear interface chain --- + + public interface PublicBaseInterface { + String getBaseValue(); + } + + // Package-private interface extending a public interface — adds no new methods + interface PackagePrivateMiddleInterface extends PublicBaseInterface { + } + + // Package-private class: only implements the package-private interface. + // getBaseValue() is declared on PublicBaseInterface — the recursive search + // must traverse PackagePrivateMiddleInterface -> PublicBaseInterface to find it. + static class PackagePrivateChainImpl implements PackagePrivateMiddleInterface { + @Override + public String getBaseValue() { + return "baseValue"; + } + } + + // --- Diamond pattern --- + + // Two package-private interfaces both extending PublicBaseInterface + interface PackagePrivateBranchA extends PublicBaseInterface { + String getBranchAValue(); + } + + interface PackagePrivateBranchB extends PublicBaseInterface { + String getBranchBValue(); + } + + // Package-private class implementing both branches (diamond). + // getBaseValue() is only on PublicBaseInterface — must be found through either branch. + static class DiamondImpl implements PackagePrivateBranchA, PackagePrivateBranchB { + @Override + public String getBaseValue() { + return "diamondBaseValue"; + } + + @Override + public String getBranchAValue() { + return "branchAValue"; + } + + @Override + public String getBranchBValue() { + return "branchBValue"; + } + } + + // --- DFE interface: public interface with a method accepting DataFetchingEnvironment --- + + public interface PublicDfeInterface { + String getDfeValue(DataFetchingEnvironment dfe); + } + + // Package-private class implementing the public DFE interface. + // Exercises the dfeInUse path in findMethodOnPublicInterfaces. + static class PackagePrivateDfeImpl implements PublicDfeInterface { + @Override + public String getDfeValue(DataFetchingEnvironment dfe) { + return "dfeValue"; + } + } + + // --- Interface with multiple methods: one exists, one doesn't --- + // Used to exercise the NoSuchMethodException catch path in findMethodOnPublicInterfaces. + + public interface PublicInterfaceWithoutTarget { + String getUnrelatedValue(); + } + + // Package-private class implementing an interface that does NOT have the fetched property. + // Also implements PublicBaseInterface which DOES have it. + // The search hits NoSuchMethodException on PublicInterfaceWithoutTarget, then finds it on PublicBaseInterface. + static class PackagePrivateMultiInterfaceImpl implements PublicInterfaceWithoutTarget, PublicBaseInterface { + @Override + public String getUnrelatedValue() { + return "unrelated"; + } + + @Override + public String getBaseValue() { + return "foundViaSecondInterface"; + } + } + + // --- Factory methods (public entry points for tests) --- + + public static Object createChainImpl() { + return new PackagePrivateChainImpl(); + } + + public static Object createDiamondImpl() { + return new DiamondImpl(); + } + + public static Object createDfeImpl() { + return new PackagePrivateDfeImpl(); + } + + public static Object createMultiInterfaceImpl() { + return new PackagePrivateMultiInterfaceImpl(); + } +}