Preserve :context init option if conn.private[:absinthe][:context] is set

[mruoss]

Hi there. I've stumbled upon a problem in absinthe_plug I'd like to propose a fix for.

Description of the Problem

If Absinthe.Plug is initialized with a :context option and some other plug puts something inside conn.private[:absinthe][:context], either directly or by using the helper functions Absinthe.Plug.assign_context/2 or Absinthe.Plug.put_options/2, the values from the initialization are overwritten by the latter. This is a result from Absinthe.Plug.update_config(:init_options, conn) merging the options, but not doing a deep merge.

Proposed Solution

In fact to me it looks like the :context config field is merged in Absinthe.Plug.Request.parse/2, so there's no need to also do that in Absinthe.Plug. I've therefore removed :context from the @init_options list.

Tests

My pull request comes with two tests. The first one fails without my fix. The second one is a sanitary check that asserts the two representations are actually merged.

[mruoss]

Just found an old PR that sounds somewhat related: #248

[mruoss]

Any thoughts on this?

[binaryseed]

It's not really clear to me what's going on in the tests, perhaps you could push this up in 2 commits so that your tests demonstrate the failure in CI?

[mruoss]

@binaryseed absolutely. I have committed the test alone now and am about to push the fix in a minute. I hope this brings some clarity together with the description above? Otherwise let me know.

[binaryseed]

Is this the error you intended to demonstrate?

Error:      test/lib/absinthe/plug_test.exs:491
     ** (FunctionClauseError) no function clause matching in anonymous fn/2 in Absinthe.Plug.TestSchema.__absinthe_function__/2

[mruoss]

Alright I can see how this is hard to understand because no resolver matches with the user missing in theh context. I extended the user resolver in the TestSchema to return an error if the user is missing in the context. Does this make things clearer?

[mruoss]

Fixed the formatting error and pushed again.

[binaryseed]

Ahh, I think I know why I was confused by the new tests, they were masking that the context is actually getting written over with the logic change here.

I've created a new PR that extends this with logic to intentionally merge the contexts: #257

[mruoss]

Thanks!