Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FieldResolverError with resolver for superclass or interface #497

Open
florensie opened this issue Apr 19, 2021 · 7 comments
Open

FieldResolverError with resolver for superclass or interface #497

florensie opened this issue Apr 19, 2021 · 7 comments
Labels

Comments

@florensie
Copy link

florensie commented Apr 19, 2021

Description

Consider the following dataclass hierarchy:

abstract class Base {
}

class Thing extends Base {
}

This is a custom resolver for the abstract class Base:

class BaseResolver implements GraphQLResolver<Base> {
    public boolean isActive(Base base) {
       return true;
   }
}

Root query resolver with datafetcher that returns our concrete class Thing:

class RootQueryResolver implements GraphQLQueryResolver {
    public Thing getThing() {
        return new Thing();
    }
}

Expected behavior

The following schema should work fine and return true for queries on the active field:

type Query {
    thing: Thing!
}

type Thing {
    active: Boolean!
}

Actual behavior

graphql.kickstart.tools.resolver.FieldResolverError: No method or field found as defined in schema [...]

Steps to reproduce the bug

  1. Create a resolver for a supertype (class or interface) with any datafetcher
  2. Have a query return a subclass of that type
  3. Try building the schema
@florensie florensie added the bug label Apr 19, 2021
@florensie
Copy link
Author

I have a working solution here but it probably needs some more work before it can be turned into a PR.

@KammererTob
Copy link
Contributor

KammererTob commented May 26, 2021

I am not saying that this is not a bug (or maybe should work), but i think you can work around that by doing this:

abstract class BaseResolver  {
    public boolean isActive(Base base) {
       return true;
   }
}

class ThinkResolver extends BaseResolver implements GraphQLResolver<Thing> {}

Although Thing should probably not be abstract in this case, since abstract classes can never be instantiated.

@florensie
Copy link
Author

florensie commented May 26, 2021

Right, but that kinda defeats the purpose of polymorphism here. If you have a ThingA, ThingB, ThingC, etc. you'll still have to create a resolver for each of those subclasses, just not the implementation. So it's only really half of a solution, since in some cases you don't exactly know what the subclasses are at compile time, making this very fragile.

Also yeah Thing shouldn't be abstract in my example, I've fixed it.

@KammererTob
Copy link
Contributor

since in some cases you don't exactly know what the subclasses are at compile time

How so?

Regarding the issue: I guess it make make sense to check for resolvers up the ancestor chain starting at the most concrete resolver (since it might override properties). Although i am not sure if this is really worth the effort. Most of the time the concrete classes will not be empty and you would need a resolver for those anyways, and this resolver could then just extend the BaseResolver. But maybe i am just not seeing the downside.

@florensie
Copy link
Author

In situations with dynamic linking or as part of a library.

@KammererTob
Copy link
Contributor

In situations with dynamic linking or as part of a library.

I see. Although those could probably still just write their own resolvers (extending your base resolver) when extending your base class, right? But i see how this could be more beneficial now. Thanks for explaining your use-case.

@pelletier197
Copy link

I'm coming a little late on this, but I faced the same issue. Actually, @KammererTob's proposition didn't work for me right out of the box, but I was able to make it work in a pretty similar way if anyone needs it

abstract class BaseResolver<T :  Base> : GraphQLResolver<T>  {
    fun isActive(base: T) : Boolean {
       return true
   }
}

class ThinkResolver : BaseResolver<Thing> 

That being said, I agree with @florensie that it would be great to be able to provide only the GraphQLResolver<T> for an interface / abstract class and have if work automatically with subclasses (or pick the most specific resolver if there is more than one).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants