-
Notifications
You must be signed in to change notification settings - Fork 374
Description
Currently, to keep track of custom per-session data (e.g. the user making a websocket request, the specific resource they're trying to subscribe to) we store it under Session.Keys, which is an arbitrary key-value store with type map[string]any. This works fine but lacks any type-safety guarantees; e.g., we could make a typo and accidentally store data under a "usrr" key and later try to access it under "user", and fail.
In practice, I think, the data we would want to store on each session has the same structure/shape, so it makes sense to have all Sessions in a given melody app/instance be constrained to the same Keys type. This is possible to implement now since the introduction of Go generics; I am suggesting something like
type Melody[Keys any] struct {
Config *Config
Upgrader *websocket.Upgrader
messageHandler handleMessageFunc[Keys]
messageHandlerBinary handleMessageFunc[Keys]
messageSentHandler handleMessageFunc[Keys]
messageSentHandlerBinary handleMessageFunc[Keys]
errorHandler handleErrorFunc[Keys]
closeHandler handleCloseFunc[Keys]
connectHandler handleSessionFunc[Keys]
disconnectHandler handleSessionFunc[Keys]
pongHandler handleSessionFunc[Keys]
hub *hub[Keys]
}
type Session[Keys any] struct {
Request *http.Request
Keys Keys
conn *websocket.Conn
output chan envelope[Keys]
outputDone chan struct{}
melody *Melody[Keys]
open bool
rwmutex sync.RWMutex
}In terms of functionality I think this is a relatively minor change, because the core behavior of melody doesn't really rely on anything about Keys anyway-- it's basically just a passthrough object for storing custom properties (I believe? correct me if I'm wrong). But this would be a breaking change anyway, because it would force library users to adopt the "strictly-typed" way of doing things instead of the original loosely-typed/dynamic API. In particular:
Melody.HandleRequestwould no longer be available, because sessions must be created with some non-nil data (or, the library user can explicitly choose a nil-able type forKeys, and explicitly initializeMethod.HandleRequestWithKeyspassing innil; in either case this would have to happen explicitly rather than implicitly)- It would also render convenience functions such as
Session.{Set,Get}obsolete, sinceKeyshas arbitrary type that may be (most commonly/usefully) astructrather than amap.
Anyway, I'm raising this for discussion. As an avid lover of strict typing this is the number one thing I find myself wishing existed when using this library-- everything else is quite delightful.