Add `Req.add_request_steps/3` and friends

[wojtekmach]

Currently we have have pairs of append and prepend functions for each type of steps, Req.Request.append_request_steps/2, Req.Request.prepend_request_steps/2, ...

My idea of Req and Req.Request separation is the former is what most users would use most of the time and the latter is for putting together very custom request pipelines but in practice most commonly for developing custom steps.

A crucial idea for Req is using custom steps so by that rationale we should have a function on Req, not Req.Request. See https://github.com/wojtekmach/req/issues/139#issuecomment-1314314060.

Eventually we also want to add add steps after specific steps. See https://github.com/wojtekmach/req/issues/148.

I think we can solve both concerns as well as get rid of append/prepend pairs with this API:

Req.new()
|> Req.add_request_steps([foo: &foo/1])         # by default we append
|> Req.add_request_steps([foo: &foo/1], at: -1) # same as above
|> Req.add_request_steps([foo: &foo/1], at: 0)  # prepend
|> Req.add_request_steps([foo: &bar/1], after: :foo)
|> Req.add_request_steps([foo: &bar/1], before: :foo)
|> Req.add_response_steps([foo: &bar/1])
|> Req.add_error_steps([foo: &bar/1])

I don't plan to support :after and :before just yet, still waiting for concrete use cases, but that's something I want to keep in mind.

One more note about naming. When introducing Req.update I had an option to call it Req.Request.update. (https://github.com/wojtekmach/req/issues/139#issuecomment-1313500753). In "regular" usage, I thought this was better:

  Req.new
- |> Req.Request.update(options)
+ |> Req.update(options)
  |> Req.get!()

but simultaneously this seemed slightly worse:

  def attach(%Req.Request{} = request, options \\ []) do
    request
    |> Req.Request.register_options([:print_headers])
-   |> Req.Request.update(options)
+   |> Req.update(options)
    |> Req.Request.append_request_steps(print_headers: &print_request_headers/1)
    |> Req.Request.prepend_response_steps(print_headers: &print_response_headers/1)
  end

so the same goes for this Req.add_request_steps vs Req.Request.add_request_steps. Maybe the Req vs Req.Request separation isn't such a good idea after all?

Just to be clear, I'd hate to make significant API changes but everything is on the table before v1.0. If we'd end up making significant API changes, we'll deprecate things and only remove them on v1.0 or v1.1.

[wojtekmach]

Regarding API, another idea is to have it both ways: Req.add_request_steps and Req.Request.add_request_steps. The former delegates to the latter.

[tanguilp]

I'd propose a change to the proposed API:

Req.add_request_steps([foo: &bar/1], before: [:foo, :bar])

This is related to https://github.com/wojtekmach/req/issues/229: in this case we'd need to insert caching response step before [:decompress_body, :decode_body] but there's no guarantee two step will always keep the same order (well except in this case, since we need first to unzip before decoding the body).

[wojtekmach]

Oh so we’d ensure we are before any of these given steps? That’s an interesting approach. I’d like to see more use cases for :before/:after. Ie if we allow a single atom now we can add support for the list of atoms later in a backwards compatible way.

[tanguilp]

Oh so we’d ensure we are before any of these given steps?

Yep that's the idea.

I’d like to see more use cases for :before/:after.

I can't think of other use-case so far.

[acalejos]

Just a thought, but having this API might make writing plugins which interact with other steps much easier. I could imagine more use cases where plugin authors might want more fine grained control of where their steps get injected.