FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Fix findPubliclyAccessibleMethod to search interfaces for accessible methods by andimarek · Pull Request #4287 · graphql-java/graphql-java · GitHub

39 changes: 39 additions & 0 deletions src/main/java/graphql/schema/PropertyFetchingImpl.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

You allowing interfaces implementing interfaces - fair enough - but there is not tests for that

I think you need to create the complicated structure that tests this

if (method != null) {
return method;
}
}
return null;
}

private boolean isSuitablePublicMethod(Method method, boolean allowStaticMethods) {
int methodModifiers = method.getModifiers();
if (Modifier.isPublic(methodModifiers)) {
Expand Down
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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<String, String>()
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<String, String>()
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:
Expand Down
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Original file line number Diff line number Diff line change
@@ -0,0 +1,138 @@
package graphql.schema.somepackage;

import graphql.schema.DataFetchingEnvironment;

/**
* Test fixtures for interface-extends-interface method resolution.
* <p>
* 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.
* <p>
* Linear chain:
* <pre>
* PublicBaseInterface (public) — defines getBaseValue()
* |
* PackagePrivateMiddleInterface — extends PublicBaseInterface (adds nothing new)
* |
* PackagePrivateChainImpl (package-private) — implements PackagePrivateMiddleInterface
* </pre>
* <p>
* Diamond pattern:
* <pre>
* PublicBaseInterface (public) — defines getBaseValue()
* / \
* PackagePrivateBranchA PackagePrivateBranchB — each adds own method
* \ /
* DiamondImpl (package-private)
* </pre>
*/
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();
}
}

Back | FazBrowse Home | New Git URL