Documented ADR for multi-segment slugs

This commit is contained in:
Alejandro Celaya 2022-08-05 16:18:53 +02:00
parent fc0d99be41
commit 8961191b2e
7 changed files with 48 additions and 5 deletions

View file

@ -43,7 +43,7 @@
"php-middleware/request-id": "^4.1",
"pugx/shortid-php": "^1.0",
"ramsey/uuid": "^4.3",
"shlinkio/shlink-common": "dev-main#b395e99 as 4.5",
"shlinkio/shlink-common": "^4.5",
"shlinkio/shlink-config": "^1.6",
"shlinkio/shlink-event-dispatcher": "^2.4",
"shlinkio/shlink-importer": "^3.0",

View file

@ -16,7 +16,7 @@ The intention is to implement a system that allows adding to API keys as many of
Supporting more restrictions in the future is also desirable.
## Considered option
## Considered options
* Using an ACL/RBAC library, and checking roles in a middleware.
* Using a service that, provided an API key, tells if certain resource is reachable while it also allows building queries dynamically.

View file

@ -11,7 +11,7 @@ However, it does not track visits to any of those, just to valid short URLs.
The intention is to change that, and allow users to track the cases mentioned above.
## Considered option
## Considered options
* Create a new table to track visits o this kind.
* Reuse the existing `visits` table, by making `short_url_id` nullable and adding a couple of other fields.

View file

@ -13,7 +13,7 @@ However, after the creation of the caching PSRs ([PSR-6 - Cache](https://www.php
Also, Shlink needs support for Redis clusters and Redis sentinels, which is not supported by `doctrine/cache` Redis adapters.
## Considered option
## Considered options
After some research, the only packages that seem to support the capabilities required by Shlink and also seem healthy, are these:

View file

@ -11,7 +11,7 @@ It is potentially possible to combine both, but if you do so, you will find out
A [Twitter survey](https://twitter.com/shlinkio/status/1480614855006732289) has also showed up all participants also found the behavior should be the opposite.
## Considered option
## Considered options
* Move the logic to read env vars to another config file which always overrides installer options.
* Move the logic to read env vars to a config post-processor which overrides config dynamically, only if the appropriate env var had been defined.

View file

@ -0,0 +1,42 @@
# Support multi-segment custom slugs
* Status: Accepted
* Date: 2022-08-05
## Context and problem statement
There's a new requirement to support multi-segment custom slugs (as in `https://exam.ple/foo/bar/baz`).
The internal router does not support this at the moment, as it only matches the shortCode in one of the segments.
## Considered options
* Tweak the internal router, so that it is capable of matching multiple segments for the slug, in every route that requires it.
* Define a new set of routes with a short prefix that allows configuring multi-segment in those, without touching the existing routes.
* Let the router fail, and use a middleware to fall back to the proper route (similar to what was done for the extra path forwarding feature).
## Decision outcome
Even though I was initially inclined to use a fallback middleware, that has turned out to be harder than anticipated, because there are several possible routes where the slug is used, and we would still need some kind of router to determine which one matches.
Because of that, the selected approach has been to tweak the existing router, so that it can match multiple segments, and moving the configuration of routes to a common place so that they can be defined in the proper order that prevents conflicts.
## Pros and Cons of the Options
### Tweaking the router
* Bad: It requires routes to be defined in a specific order, and remember it in the future if more routes are added.
* Good: It initially requires fewer changes.
* Good: Once routes are defined in the proper order, all the internal logic works out of the box.
### Defining new routes
* Bad: The end-user experience gets affected.
* Bad: Probably a lot of side effects would happen when it comes to assembling short URLs.
* Bad: Routing needs to be configured twice, resolving the same logic.
* Bad: It turns out to still conflict with some routes, even with the prefix, which defeats what looked like its main benefit.
### Let routing fail and fall back in middleware
* Good: Does not require changing routes configuration, which means less side effects.
* Bad: Since many routes can potentially end up in the middleware, there's still the need to have some kind of routing logic.

View file

@ -2,6 +2,7 @@
Here listed you will find the different architectural decisions taken in the project, including all the reasoning behind it, options considered, and final outcome.
* [2022-08-05 Support multi-segment custom slugs](2022-08-05-support-multi-segment-custom-slugs.md)
* [2022-01-15 Update env vars behavior to have precedence over installer options](2022-01-15-update-env-vars-behavior-to-have-precedence-over-installer-options.md)
* [2021-08-05 Migrate to a new caching library](2021-08-05-migrate-to-a-new-caching-library.md)
* [2021-02-07 Track visits to 'base_url', 'invalid_short_url' and 'regular_404'](2021-02-07-track-visits-to-base-url-invalid-short-url-and-regular-404.md)