Crystal: Allow HTTP::Server::Context carry external data

Created on 2 Nov 2019  Â·  11Comments  Â·  Source: crystal-lang/crystal

In web framework development, we usually re-open HTTP::Server::Context to add some property.

Open proper way would be to allow this class to handle a data POD

The class would be

class HTTP::Server
  # Instances of this class are passed to an `HTTP::Server` handler.
  class BaseContext(T)
    # The `HTTP::Request` to process.
    getter request : Request

    # The `HTTP::Server::Response` to configure and write to.
    getter response : Response

    getter resources : T

    # :nodoc:
    def initialize(@request : Request, @response : Response)
    end
  end

  class Context < BaseContext(Nil)
  end
end

This have two advantages :

  • Allow framework developers to expose only what they want
  • Avoid re-open standard lib
    If a user want to add the method current_user, he just have to remplace the class, or open the pod

Most helpful comment

Actually read an interesting perspective on this the other day http://twitchard.github.io/posts/2019-11-01-beware-middleware.html

All 11 comments

Actually read an interesting perspective on this the other day http://twitchard.github.io/posts/2019-11-01-beware-middleware.html

This was discussed in the past (I can't remember the issue). Reopening is fine. Reopening is a feature that we have in Crystal and we should it use when appropriate.

Open proper way

I believe the current way is a proper way: it's type-safe, efficient and simple.

The only prob I've had with it is that the HTTP::Context gets re-used (and you don't realize that so...you learn it the hard way LOL)

The cons of reopening for this is that it could be harder for the app to have two http servers that behave differently.

A related idea/issue with the http server i had was to decouple how the connection wrapper for ssl via some generic argument.

Another dimension would be to allow specialized context type.

And all this can be an implementation for advanced uses, leaving the current http server be one specific instantiation.

I always liked the idea of being able to attach components, almost like an ECS to the context. Just a Hash(Class, Reference) (okay, Hash(Class, Void*) + magic). So you'd use context[Kemal::Foo] to get kemal's special sauce.

I'm also sceptical of middleware, but we should support the pattern where we can.

On a proper way, I mean a way that not change the behavior without indicate it to the user.

This is a strength of Crystal that you know exactly what happen, and can verify things at compile time, but the user shouldn't be surprised of the result :)

I'm more in a philosophy of extend than modify, if a class is build in a way, there is no reason to modify it, but you should be able to extend the class (with dependency injection, inheritance, ...) to bend the code to your problem solving.

But, I can understand not everyone have this mindset :+1:

Depending on what data the user wants to store, it might be sufficient to simply
have like

getter attributes : Hash(String, Int32 | Bool | Nil | Float64 | String) { Hash(String, Int32 | Bool | Nil | Float64 | String).new }

At least for storing simple request specific data to share between the request/response life cycle.

EDIT: Could even be expanded to an internal type that wraps a hash so you could do like

continue = ctx.attributes.get_bool "some_key"
limit = ctx.attributes.get_int "other_key"

@Blacksmoke16 That completely avoids type safety for no apparent reason.

I'm not saying its a replacement for reopening and adding your custom stuff. More so as just a generic simple place to store scalar values for one off things.

Anything more important/complex it should be reopened and have that type added.

Yeah that doesn't seem very precise somehow...
One idea might be to at least document how to monkey patch it (for
instance, mentioning that the objects are re-used...)

On Sat, Nov 23, 2019 at 3:50 PM Blacksmoke16 notifications@github.com
wrote:

I'm not saying its a replacement for reopening and adding your custom
stuff. More so as just a generic simple place to store scalar values for
one off things.

Anything more important/complex it should be reopened and have that type
added.

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
https://github.com/crystal-lang/crystal/issues/8423?email_source=notifications&email_token=AAADBUD7WOGNWDX3LBPQ6WDQVGXSJA5CNFSM4JIERY62YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEE77NTQ#issuecomment-557840078,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AAADBUC3R5GPGRISPHAKQS3QVGXSJANCNFSM4JIERY6Q
.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

asterite picture asterite  Â·  3Comments

lbguilherme picture lbguilherme  Â·  3Comments

oprypin picture oprypin  Â·  3Comments

cjgajard picture cjgajard  Â·  3Comments

lgphp picture lgphp  Â·  3Comments