Removing trailing slashes #80

This PR solves the issue of there being both "/" and "" paths, that in the builder-web context shouldn't mean different things.

The slashes are now removed using a Dream middleware, and the request is redirected using a permanent redirect (that doesn't change the method used): https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Location

Trailing slashes from the hardcoded link urls were removed, as unneccesary redirects are then avoided.

I wonder, since albatross-client and builder-server interact with the HTTP server whether it would be more suitable to keep the / and redirect e.g. /job/:job to /job/:job - so add a trailin slash if it is not present. The advantage would be that the diff is smaller, and the URL structure would be preserved.

Any thoughts @reynir?

I wrote https://github.com/reynir/dream-normalize-route which defines a function normalize that takes e.g. Dream.get, a pattern and a handler and then applies Dream.get on the pattern and handler and makes a permanent redirect to the url with a trailing slash if the pattern does not end in a trailing slash or otherwise a redirect to the url without the trailing slash.

That would be the behavior I would want, but that solution is somewhat complex and relies on the current behavior of Dream.get patterns etc. :/

I think always redirecting to the trailing slash version is acceptable, too. Although I it would be nicest to keep e.g. /job/:job/build/:build/script without the trailing slash.

To begin with I don't think it's too much of an issue that /job/:job leads to an error page.

Here is a list of reasons for why I chose to default to removing slashes instead of appending them:

Reason 1

A slash normally means that you list something, like with directories. If you access the thing itself, I don't think it should have a slash in the end.

E.g.
https://builds-staging.robur.coop/job/albatross/build/a75a5f8c-61f8-4d06-9ff0-f7244a986e46/
vs.
https://builds-staging.robur.coop/job/albatross/build/a75a5f8c-61f8-4d06-9ff0-f7244a986e46

Reason 2

The existing url-structure of builder-web already defines "list-pages" as named urls on top of using a strailing slash, e.g.:

/job/:job/failed/
/failed-builds/

.. so the usage of slash in these urls is double-information - the url without the slash already communicates that it's a list.

Reason 3
The root of the site is a list, and it already removes the trailing slash.

Reason 4
I think it feels confusing and messy to use both slash and non-slash urls. The user doesn't really have any way of guessing a correct url, as he doesn't know the existing list vs non-list semantics of the site.

Reason 5
I think that query parameters looks less aesthetic when given to a slash-adress (in the context of there not being a need for the address to have a trailing slash).

E.g.:
https://builds-staging.robur.coop/job/albatross/?platform=freebsd-12
vs.
https://builds-staging.robur.coop/job/albatross?platform=freebsd-12

.. also, most of the time, a page is both for a single entity of something, and a list of something else at the same time - as is the case for /job/:job

Pages often also change semantics over time, e.g. starting as a single thing, and then becoming a list of things. I don't think one should switch from no trailing slash to slash in this case.

So overall my ideals for this is simplification, consistency and avoiding deprecation of urls.

I wrote https://github.com/reynir/dream-normalize-route which defines a function normalize that takes e.g. Dream.get, a pattern and a handler and then applies Dream.get on the pattern and handler and makes a permanent redirect to the url with a trailing slash if the pattern does not end in a trailing slash or otherwise a redirect to the url without the trailing slash.

@reynir I looked at the implementation - it needs to pass the query params on to the redirect too, else a page like https://builds-staging.robur.coop/job/albatross/?platform=ubuntu-20.04 breaks

I pushed some minor changes. Looks fine to merge now I think.

     let path_matches_dreamroute ~path dreamroute =
       let is_match path_elem dpath_elem =
         (dpath_elem |> String.starts_with ~prefix:":")

Removing trailing slashes #80

Reviewers