Prevent key :is_type_of not found error on interfaces

[maennchen]

Fix for #1077

Based on #1079, merge that one first.

[benwilson512]

Can you include a test case that would fail on master?

[maennchen]

@benwilson512 Where was the current functionality of the is_type_of tested?

[benwilson512]

test/absinthe/type/interface_test.exs mostly

[benwilson512]

I think it's actually this list which is wrong. The implementors list should be only concrete implementors. This change should occur in lib/absinthe/blueprint/schema/interface_type_definition.ex find_implementors/1

[benwilson512]

This way the filtering is accomplished at schema building time instead of every time the interface is evaluated at runtime.

[maennchen]

I'm running out of time now, I'll nave a look later 👍

[maennchen]

@benwilson512 If I change it in finde_implementors, I break those two tests:

  1) test interfaces are propagated across type imports (Absinthe.Schema.Rule.ObjectMustImplementInterfacesTest)
     test/absinthe/schema/rule/object_must_implement_interfaces_test.exs:88
     Assertion with == failed
     code:  assert %{named: [:cat, :dog, :user], favorite_foods: [:cat, :dog, :user], parented: [:named]} == Schema.__absinthe_interface_implementors__()
     left:  %{favorite_foods: [:cat, :dog, :user], named: [:cat, :dog, :user], parented: [:named]}
     right: %{favorite_foods: [:cat, :dog, :user], named: [:cat, :dog, :user], parented: []}
     stacktrace:
       test/absinthe/schema/rule/object_must_implement_interfaces_test.exs:89: (test)

.

  2) test interfaces are set from sdl (Absinthe.Schema.Rule.ObjectMustImplementInterfacesTest)
     test/absinthe/schema/rule/object_must_implement_interfaces_test.exs:122
     Assertion with == failed
     code:  assert %{image: [], node: [:image, :resource], resource: [:image]} == InterfaceImplementsInterfaces.__absinthe_interface_implementors__()
     left:  %{image: [], node: [:image, :resource], resource: [:image]}
     right: %{image: [], node: [], resource: []}
     stacktrace:
       test/absinthe/schema/rule/object_must_implement_interfaces_test.exs:123: (test)

Do we consider those tests correct or wrong?

[maennchen]

@benwilson512 In which direction should I continue?

[maennchen]

@tlvenn Now I finally get it. 😄

I'll try that way then

[maennchen]

@tlvenn

https://github.com/absinthe-graphql/absinthe/runs/4335029279?check_suite_focus=true

  1) test interfaces are set from sdl (Absinthe.Schema.Rule.ObjectMustImplementInterfacesTest)
Error:      test/absinthe/schema/rule/object_must_implement_interfaces_test.exs:132
     Assertion with == failed
     code:  assert %{image: [], node: [:image, :resource], resource: [:image]} == InterfaceImplementsInterfaces.__absinthe_interface_implementors__()
     left:  %{image: [], node: [:image, :resource], resource: [:image]}
     right: %{image: [], node: [], resource: []}
     stacktrace:
       test/absinthe/schema/rule/object_must_implement_interfaces_test.exs:133: (test)

.

  2) test interfaces are propagated across type imports (Absinthe.Schema.Rule.ObjectMustImplementInterfacesTest)
Error:      test/absinthe/schema/rule/object_must_implement_interfaces_test.exs:98
     Assertion with == failed
     code:  assert %{named: [:cat, :dog, :user], favorite_foods: [:cat, :dog, :user], parented: [:cat, :dog, :named, :user]} == Schema.__absinthe_interface_implementors__()
     left:  %{favorite_foods: [:cat, :dog, :user], named: [:cat, :dog, :user], parented: [:cat, :dog, :named, :user]}
     right: %{
              favorite_foods: [:cat, :dog, :user],
              named: [:cat, :dog, :user],
              parented: [:cat, :dog, :user]
            }
     stacktrace:
       test/absinthe/schema/rule/object_must_implement_interfaces_test.exs:99: (test)

You seem to know that code better. Should I just give you push access to my fork?

[tlvenn]

Ha great, thanks for this, I wanted to believe it would work but kinda suspected this could happen. Those 2 issues have been introduced by the original PR that added support for interface implementing interfaces because until now find_implementors/1 was incorrectly returning interfaces instead of just concrete types.

Given that:

• This test suite is purposely for testing objects that use interfaces.
• We have other tests against interfaces implementing interfaces in test/absinthe/schema/rule/object_interfaces_must_be_valid_test.exs.
• Transitively implemented interfaces must also be defined on implementing types.

We should simply get rid of interfaces implementing interfaces in those 2 tests and fix the assertions, we are mixing too many concerns otherwise and the purpose of those tests is lost.

We probably could / should introduce a specific test to assert that find_implementors/1 is only returning concrete types in another place. At the same time, the test you added indirectly address this so probably not so needed.

I am totally happy to take it from there if you want, totally up to you and in any case, thanks again for your time and effort on this. Have a great WE !

[tlvenn]

@maennchen can you please re send the invitation to collab on your fork, i did not see it initially and it has expired now, thanks.

[maennchen]

@tlvenn I sent it again :)

[maennchen]

@benwilson512 I added a test.

[maennchen]

@benwilson512 Ping :)

[maennchen]

@benwilson512 Iw ould really like to finish this PR. Can you give me some guidance on what to do to get it over the line?

[arturs678]

Any updates on this ?

[maennchen]

@tlvenn Thanks :)