Repository navigation
Proposed spec change: support immutable data structures #40
Description
The existing specification for OWIN makes it difficult to leverage immutable data structures for the required IDictionary<string, object> environment and IDictionary<string, string[]> headers dictionaries. While it's unlikely that we will ever move away from Stream, a simple spec change would allow those of us who want immutable IDictionary types to do so: type AppFunc = Func<IDictionary<string, object>, Task<IDictionary<string, object>>>.
This signature could be used by existing implementations rather easily by simply returning the environment dictionary instance. However, for those using immutable data structures, this signature allows such implementations to return the new instance as it is passed along through the middleware chain or modified by the running application.
No changes would be required for the other existing or proposed signatures, which already return instances of one thing or another.
Activity
haf commentedon Aug 30, 2015
+1
horsdal commentedon Aug 30, 2015
Sounds reasonable. Supporting immutable dictionaries would be nice.
Tratcher commentedon Aug 31, 2015
-1 That sounds really inconvenient (and inefficient) for anybody else. The OWIN environment mutability is one of it's strengths, what is the motivation for disabling it? Immutable is also not opt-in friendly, every downstream middleware has to be aware of it and re-written to duplicate the dictionary for any possible modification.
I think only a very simple app/pipeline could process a request without modifying the original dictionary, even setting things like owin.ResponseStatusCode would become non-trivial. I've seen common middleware for modifying five of the standard request keys, several request headers, two of the response keys, and at least a dozen custom keys.
haf commentedon Aug 31, 2015
@Tratcher Why does it sound inconvenient or inefficient? It's just a signature, it doesn't ensure the underlying implementation of the interface is immutable, so you can just keep passing around the same dictionary in and out. So this is not about disabling it. But yes, if they write to the IDictionary, they'd have to update their used reference to what comes out (which is the same reference in your case).
However, it's absolutely not one of OWIN's strengths, it's a clear and present weakness. I could give you the same justification for this statement as you do of its opposite (i.e. nothing), but instead I wonder what makes you think that update-in-place is a strength in the light of the required thread synchronisation and excessive copying needed to guarantee server invariants?
That something is immutable doesn't mean that you don't produce more values with changed inner contents like you seem to think; what it means is that you can share the dictionary between threads easily without having to worry about concurrent updates violating invariants. If you want examples of this, I invite you to have a browse through Suave's source code.
Please take the above at face value, I am interested in hearing your view on this.
Tratcher commentedon Aug 31, 2015
Maybe we should start with some simple code samples like a 404 response. If the incoming environment can be immutable how does the following code need to change? Do I need to detect if it is immutable? Do I need to copy the dictionary to make modifications?
haf commentedon Aug 31, 2015
If we can, like we've been discussing in gitter, get really good public test suites, the client could show that it uses immutable data structures via an opt-in flag on C# and an opt-out flag on F# (for example).
Tratcher commentedon Aug 31, 2015
Sorry, there's apparently more magic going on here than I'm making sense of. I'm going to tap out until I figure out what's going on.
panesofglass commentedon Aug 31, 2015
@Tratcher, there's no magic. The idea is that OWIN currently only supports mutable dictionaries. The request is that it extend support to immutable dictionaries, not to require immutability. The overall change would require
AppFuncs to return their dictionary with the completedTaskrather than simply mutate the dictionary. For those wishing to continue using mutable dictionaries, the change is no different than allowing OWIN to follow the builder pattern as seen inStringBuilder.panesofglass commentedon Aug 31, 2015
@haf, that example should be:
panesofglass commentedon Aug 31, 2015
@Tratcher, oh, I see now what you meant about magic.
@haf, I think a flag is probably not a good idea. I also now see the problem that by supporting immutable dictionaries, other middleware and apps are going to be penalized by having to know whether or not that dictionary is immutable. At design time, everything will play nicely; at runtime, calls to mutate or
.Addwill suddenly throw exceptions.This is enough of a breaking change that everything would have to essentially be rewritten and become far more complicated. Unless, of course, the immutable dictionary had to provide a mutable fallback to support existing implementations, at which point the signature change is unnecessary.
panesofglass commentedon Aug 31, 2015
@haf is it possible we could create an adapter layer that supports immutable data structures and wraps older middleware?
damianh commentedon Aug 31, 2015
Under what scenarios would something like this occur? I've never found my self in a place where I'm mutating the env dictionary concurrently. (Sorry if dumb question).
haf commentedon Sep 1, 2015
@damianh Let's say you wanted to do more than a single transaction during the request you could fork-join each transaction and join it back before replying.
Pseudo-code:
Where
joinwould merge the changes of eachHttpRequestvalue that comes from each of logToDb (writes X-Last-Tx header), sendWelcomeEmail (writes X-RepeatSignup header) and renderAcq which writes to the headers (setting Vary header and stream).@panesofglass Penalized? If it's opt-in then there's no penalisation over what's already present. Here's an example from another project: https://facebook.github.io/react/docs/pure-render-mixin.html
Adapter that takes a stream of reads-and-writes and at the end outputs a value? Yes, e.g. DeltaDictionary is that.
Sorry, I don't get it, why is that? What's complicated?
serialseb commentedon Sep 1, 2015
👎
Changing AppFunc at this stage would be a huge undertaking leading to OWIN 2.0, invariably leading to breaking everything that currently exist.
I understand the sentiment, but with where we are currently, this looks to me like compat wins.
Unless i'm mistaken, the immutability of the env dictionary could be achieved in middleware, as it is a Func<AppFunc,AppFunc>. It would be trivial to write a MidFactory that allows for alternative implementations of what the request/response looks like. including making those immutable should one wish middlewares to be implemented this way.
That said, it would also break all things that are wrapping dictionary interfaces to detect who wrote what when and why in an injectable fashion, by composing wrappers around env, MidFactory and BuildFunc.
serialseb commentedon Sep 1, 2015
Could someone with the correct rights put a 2.0 tag on this?