support for ActionAndUpdateContext

[marioosh]

Added action which update Ctx and make able to convert final result to another type after that. For example resolver provides ComplexModel, we can use data from that model and put it into model, after that step we can convert ComplexModel to another one and marshall it as response.
At the end we can get all info from Ctx with ExecutionScheme.Extended .

[OlegIlyenko]

Thanks for working on this PR! I would like to make more in-depth review a bit later, but after the first look I have a few questions:

(1) How is ActionAndUpdateCtx different from using UpdateCtx and them mapping the result? E.g. UpdateCtx(wrapped)(v ⇒ ctx.ctx.copy(meta = v.meta)).map(wrapped ⇒ wrapped.value)

(2) I have some concerns about the context propagation. For example, what will happen if multiple root or non-root fields update the context? In this example

query {
  fieldThatSetsMataToA {
    friends {
      fieldThatSetsMataToB
    }
  }

  fieldThatSetsMataToC
}

Which value should resulting context contain (when accessed from ExecutionResult): A, B, C or A + B + C? If I understand the new implementation correctly, it would be either A or C depending on which field's Future would finish last, is it correct?

Unlike mutation fields, query fields are not guaranteed to execute sequentially. This is why only for root mutation fields the updated context object is provided in extended execution result. For query fields it's always the original userContext object currently (to avoid ambiguity).

[marioosh]

Hm, seems like your example covers completely what I did. Actually I saw that UpdateCtx did pass Ctx state to executor so I tried to implement my own action, but at the end I changed general behaviour and both Actions do the same now. I'll remove the new Axction but let's talk about passing context to executor.

I think it would be helpful to get that info in executor in many cases. In example above fieldThatSetsMataToAand ..B used the same Ctx, so how you modify it is up to you, but yes you're right, on top level on Execution it would be only A or C

What do you think to make it in the way to get both: A and C? Myabe executor should get Ctx for each top level query? What do you think?

[OlegIlyenko]

I think it is helpful to look at it from a perspective of your original use-case for this feature. Assuming that fieldThatSetsMataTo* are loading data from external REST services and collect response headers in the context object. I feel that it is not good to put so much emphasis on the root query fields. From my previous example, fieldThatSetsMataToA is not much different from fieldThatSetsMataToB. They both may communicate with an external service and potentially might contribute response headers.

Looking at it from this perspective, I think we would need a new mechanism for exposing this header data via an extended ExecutionResult if we would like to do it in an immutable way (without mutating the context in any way). I feel that it is hard to rely on a context object in this scenario.

One thing we can do is, for example, introduce a new update action that wraps a normal Action and allows to return some additional list of meta-data which is execution engine collects as query is executed and then exposes it via ExecutionResult. In theory this might provide a convenient way to collect such things as headers from fields regardless whether it is a root field or field is deeply nested. I still have concerns about this approach though, i think it would not be enough. Even in my example, let's say friends field gives back 1000 elements. In this scenario fieldThatSetsMataToB probably would be implemented as a deferred value and the actual REST request would be done in the deferred resolver. if implemented as I described above, all 1000 resolutions of fieldThatSetsMataToB field will return the same list of identical headers. I think it would be suboptimal since it adds overhead and does not maps nicely to the actual use-case. To address this, the new feature probably also need to somehow integrate with the deferred value resolution mechanism.

[marioosh]

it has no sense to push it in current state, I would think about another possible solution.