CrowdHailer/raxx

Add a Middleware-aware Router

nietaki opened this issue · 5 comments

nietaki commented

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

  • allow attaching a list of middlewares to a "range" of routes (different routes need to have different treatment)
  • nice (convenient, hard to use wrong, transparent) interface
  • make it easy and intuitive to have the middlewares configured at runtime (it's been a thorn in my side and the sides of others with Plug)
  • maybe allow attaching a list of middlewares before the routes match (?)
  • anything else that I'm missing?

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

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

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:

  • routes are grouped in sections (there might be a better name), and middleware pipelines are set per section.
  • by the default middlewares list is an empty list
  • the middlewares can be passed either as a list that's established at compile time, or a function that will be executed at runtime to return them

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

CrowdHailer commented

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:

  • it is very verbose, as it needs to be applied to every route
  • Supports compiletime only, unless middlewares was a function returning a list but that would be even more verbose.

For the above proposal:

  • I like the introduction of sections, name is fine for me
  • I would not (yet) separate configurable from unconfigurable stacks (would call them stacks instead of pipelines.
    I would pass the options to all functions the same e.g. ignore when need be. 
     # def api_pipeline() do
def api_stack(_options) do
nietaki commented

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