Skip to content

Conversation

@florensie
Copy link

Fixes #497

Checklist

  • Pull requests follows the contribution guide
  • New or modified functionality is covered by tests

Description

Made necessary changes to class scanning logic to support using supertypes in resolvers.

Copy link
Collaborator

@oryan-block oryan-block left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution 👍
I have some questions.

val resolverInfoList = this.resolverInfos.filter { it.dataClassType == item.clazz }
val resolverInfo: ResolverInfo = (if (resolverInfoList.size > 1) {
MultiResolverInfo(resolverInfoList)
val resolverInfoList = this.resolverInfos.filter { it.dataClassType.unwrap().isAssignableFrom(item.clazz.unwrap()) }
Copy link
Collaborator

Choose a reason for hiding this comment

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

Lets extract item.clazz.unwrap() to a variable.

* Checks if all `ResolverInfo` instances are related to the same data type
*/
private fun findDataClass(): Class<out Any> {
val dataClass = resolverInfoList.asSequence().map { it.dataClassType }.distinct().singleOrNull()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we fix this to find the correct type instead of deleting it?

}
}

class ItemInterfaceResolver : GraphQLResolver<ItemInterface> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Mind creating a separate test class with an example closer to the original issue instead of adding to this one?

} else {
resolverInfosByDataClass[item.clazz] ?: DataClassResolverInfo(item.clazz)
if (resolverInfoList.size == 1) {
MultiResolverInfo(resolverInfoList, item.clazz.unwrap())
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why multi if there's only one?

val appropriateFirstParameter = if (search.requiredFirstParameterType != null) {
method.genericParameterTypes.firstOrNull()?.let {
it == search.requiredFirstParameterType || method.declaringClass.typeParameters.contains(it)
it == search.requiredFirstParameterType || (it is Class<*> && it.unwrap().isAssignableFrom(search.requiredFirstParameterType))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we need the cast to Class here?

@florensie
Copy link
Author

I'm going to redo this some other time.

@florensie florensie closed this Jun 15, 2022
@swollner
Copy link

Hi is there any progress on this?

@florensie
Copy link
Author

@swollner None beyond this PR. Best to keep discussion to #497, feel free to use this PR as a reference/base if you wish to contribute yourself.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants