CrowdHailer / raxx

Interface for HTTP webservers, frameworks and clients
https://hexdocs.pm/raxx
Apache License 2.0
402 stars 29 forks source link

Add a Middleware-aware Router #148

Closed nietaki closed 5 years ago

nietaki commented 5 years ago

The current Raxx.Router is good, but doesn't allow the users to use middlewares yet. The purpose of this issue is to modify it to start supporting middlewares or create a new one that does.

Requirements

I think we should be able to roughly figure out a good and implementable interface before coding anything too labour-intensive up. I think if we move away from a single use Raxx.Router opts invocation to use Raxx.Router and another macro or two, it shouldn't be impossible.

(All the requirements and questions mainly to document my thinking and start a discussion, feel free to disagree)

nietaki commented 5 years ago

Additional thoughts, based on the Slack discussions:

Is it important to be able to build and transform routers at runtime?

I don't think so - I don't see a use-case for it and I'm afraid if we wanted to do it we might end up with a weird implementation

Is there a better way of defining the routes than matching on the requests?

Maybe, but I like matching on requests a lot - it's very easy to understand and surprisingly powerful. The only thing I dislike about it is that it's so verbose that the code formatter sometimes ends up wrapping the lines with router entries, making them less readable. For example a route like

{%{method: :GET, path: ["users", _id, "contact_preferences"]}, UserPage},

could be replaced with a less verbose line with a very similar expressive power:

{{:GET, ["users", _id, "contact_preferences"]}, UserPage},
nietaki commented 5 years ago

My initial thoughts are along the lines of this:

defmodule Foo.WWW do
  use Raxx.Router

  # different ways to define the middlewares for different sections

  @compile_time_middlewares [{Foo.Static, "/some/path"}]

  def api_pipeline() do
    allowed_token = System.get_env("ALLOWED_TOKEN")
    [{Foo.TokenAuthentication, [allowed_token: allowed_token]}]
  end

  def configurable_pipeline(options_passed_to_current_module) do
    foo = Keyword.get(options_passed_to_current_module, :foo, :bar)
    [{Foo.BusinessSpecificMiddleware, [foo: foo]}]
  end

  # routes

  section &api_pipeline/0, [
    {%{method: :GET, path: ["users", _id, "contact_preferences"]}, UserPreferences},
  ]

  section [
    {%{method: :GET, path: ["dashboard", "user_statistics"]}, Dashboard},
  ]

  section @compile_time_middlewares, [
    {%{method: :GET, path: []}, Foo.HomePage},
    {_, Foo.NotFoundPage}
  ]
 end

So in general:

I'm not married to this idea, this is a jump-off point from my side.

CrowdHailer commented 5 years ago

My idea was to modify the tuple to include middlewares.

{%{method: :GET, path: ["users", _id, "contact_preferences"]}, UserPreferences, middlewares},

Although this is a very small change I no longer think it is a good idea:

For the above proposal:

nietaki commented 5 years ago

All good points. I like the standardisation of the middlewares creation functions to /1.

I need to get my mind away from the word "pipeline" in general :D

CrowdHailer commented 5 years ago

Released in https://hex.pm/packages/raxx/0.17.3