Crystal: Extensible HTTP::Server::Context for libraries

[andrewhamon]

The current state of HTTP:Server::Context isn't the greatest when it comes to libraries/frameworks/middlewares that need to add additional context (prototypical example would be adding a params method).

Currently it seems like the most common route is to simply monkey patch HTTP::Server::Context and add whatever is needed, but this can lead to namespace collisions very easily.

It would be great if there was a way to safely register extensions with HTTP context. There was some brief discussion in gitter: https://gitter.im/crystal-lang/crystal?at=5873e020cbcb28177049b76a

The purpose of this issue is to throw around some ideas to see what we could cook up and determine if its worthwhile. I believe its worth noting that Rack has done just fine so far using a shared namespace... but I hope we can do better :)

[maiha]

This is a nice issue. I think this problem is not limited in HTTP but in language specification.

In Ruby, that problem has not been solved for a long time since it was born. Unfortunely, we have same problem in Crystal too. Updating gems or shards easily breaks our apps in Ruby and Crystal. I feel this feature prevents many users from using in production.

It's time to change the negative legacy. For example, Scala solved this by using override modifier. This feature has helped me many times. I hope that such collision safety will be implemented in Crystal. :smile:

[andrewhamon]

An override modifier would definitely be a welcome addition, but I don't think its enough on its own to solve this issue.

[asterite]

I believe its worth noting that Rack has done just fine so far using a shared namespace

So why do you need to "improve" this?

The same is true for Ruby namespaces: if I declare a module Foo, then no one else can do it. But Ruby's ecosystem just does fine with that. Plus, in Crystal you'll immediately get a compile error in the case where I use the same property name as yours.

I personally don't think there's anything we can improve here without drastically changing the language.

[ysbaddaden]

Yes, simply having Context#kemal or Context#myapp accessors for framework specifics seems enough to me. Furthermore, any conflict should quickly raise compilation errors

[andrewhamon]

Well, technically in Rack people add to the request hash, not monkey patch. But it's still a shared namespace with the same risks.

Some people in the gitter were suggesting that a hash keyed by classes would be cool, so like context.extensions[MyLib::Params] # instance of MyLib::Params

[asterite]

But what if I choose MyLib::Params for my library too?

There's really no difference between the chance of a clash between:

context.extensions[MyLib::Params]

and

context.extensions_my_lib_params

But there is a difference: the later is probably implemented by adding a class variable to HTTP::Context, while the former will go through a hash lookup, which is slightly slower. So using a Hash with a type as a key (which, by the way, can't be expressed right now in the language) has the same chances of a clash, is more verbose to write and is slower.

[andrewhamon]

Yes, but the point of doing that would be to encourage namespacing.

I'm not worried that there will be two context.kemal_params. I'm worried because Kemal is using context.params with no namespacing of any sort _right now_.

Edit: ~Err... I thought they were? Can't seem to find it now~ Here it is.

[asterite]

But that's OK, params represents the URL params. There won't be an extension for Kemal that will also use params as a name because that's already covered. There's also a session property which provides access to the session values, and nobody else will want to use session unless it's a different implementation of a session, in which case you won't use kemal-session (so you'll chose one of those).

In fact, if I remember correctly, the whole discussion appeared because of crux, which is a router. But since it's a router you won't be using kemal's router (which also handles routing), so it's fine if you name your property params because there won't be a clash there. Combining crux with kemal doesn't make sense.

[RX14]

This problem would come up if you used two routers which modified HTTP::Server::Context. For example if you decided to switch out your routing library to another incrementally you would have two routers in the app for a while, and hit this problem.

[andrewhamon]

@asterite a really common example of two frameworks being used at the same time in Ruby is when people mount the admin panel of Resque (which is a Sinatra app) in to their Rails app.

Also, a very important distinction between the Rack hash and Crystal context is that even if two Ruby libs used the exact same name, "params", that field would most likely just be overwritten at runtime once the context passes from one app to another. That can't happen in Crystal.

[andrewhamon]

Btw, I'm not a huge fan of the idea of changing the language to support this either :) but I do think that there is room for improvement, either by establishing some conventions around this and maybe by proving some generic context helpers in the standard lib. Defining a Session interface and maybe adding a JSON::Any hash for additional params would go a long way. Rack has a fair number of standard helpers and whatnot which pretty much everyone uses.

[luislavena]

I've encountered the issue described but however decided is a co-sharing and design issue and not specifically something that Crystal has to solve.

In my case, I've decided against exposing these router library specific details in Context and instead provide my own extension to it using my namespace. For example Beryl exposes beryl which allow you to access params within it (if you're interfacing with Context directly).

I've also wrapped the original HTTP Context and use delegate to give direct access to params without having to use beryl.params always.

But that way, I keep things isolated from the Context, allowing me to run by router and other router library that might be extending Context without being affected.

Cheers.