absinthe-graphql / absinthe_plug

Plug support for Absinthe, the GraphQL toolkit for Elixir
https://hex.pm/packages/absinthe_plug
MIT License
260 stars 163 forks source link

Allow using a struct as the absinthe context #258

Closed coryodaniel closed 3 years ago

coryodaniel commented 3 years ago

My application has a few different authentication contexts. I tried using a struct instead of a map when assigning a context to Abinsthe.Plug, but it results in an error.

Is there any reason why structs shouldn't be a valid value? I wouldn't mind making a PR if it makes sense for the project.

       (elixir 1.11.4) lib/access.ex:286: Access.get/3
       (absinthe_plug 1.5.5) lib/absinthe/plug.ex:332: Absinthe.Plug.update_config/3
       (absinthe_plug 1.5.5) lib/absinthe/plug.ex:289: Absinthe.Plug.call/2
       (phoenix 1.5.8) lib/phoenix/router/route.ex:41: Phoenix.Router.Route.call/2
       (phoenix 1.5.8) lib/phoenix/router.ex:352: Phoenix.Router.__call__/2
       (massdriver 0.1.0) lib/massdriver_web/endpoint.ex:1: MassdriverWeb.Endpoint.plug_builder_call/2
       (massdriver 0.1.0) lib/massdriver_web/endpoint.ex:1: MassdriverWeb.Endpoint.call/2
       (phoenix 1.5.8) lib/phoenix/test/conn_test.ex:225: Phoenix.ConnTest.dispatch/5
       test/massdriver_web/schema_test.exs:103: (test)
benwilson512 commented 3 years ago

Hi @coryodaniel . This is an issue because your specific struct does not implement the Access behavior. Absinthe is not explicitly disallowing it, but it does require that it behaves as a map.

coryodaniel commented 3 years ago

Thanks for the quick reply!

I get that it’s because the access behavior isn’t implemented, I guess my question is, if I sent a PR to use Map functions instead of using the access behavior so structs would be supported, would that be a welcome feature? Or do you prefer users impl the access behavior should they need it.

benwilson512 commented 3 years ago

@coryodaniel I think a PR that used map functions is fine, BUT we need to make sure that a nil context is still supported for backwards compatibility reasons.

In theory this could break compatibility for anyone using a novel KV struct that implemented its own access behavior but the use of access wasn't really intended to support that anyway.